Re: [PATCH v6 04/13] vpci: restrict unhandled read/write operations for guests

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>]

On 04.02.22 16:11, Jan Beulich wrote:
> On 04.02.2022 07:34, Oleksandr Andrushchenko wrote:
>> A guest can read and write those registers which are not emulated and
>> have no respective vPCI handlers, so it can access the HW directly.
> I don't think this describes the present situation. Or did I miss where
> devices can actually be exposed to guests already, despite much of the
> support logic still missing?
No, they are not exposed yet and you know that.
I will update the commit message
>
>> In order to prevent a guest from reads and writes from/to the unhandled
>> registers make sure only hardware domain can access HW directly and restrict
>> guests from doing so.
> Tangential question: Going over the titles of the remaining patches I
> notice patch 6 is going to deal with BAR accesses. But (going just
> from the titles) I can't spot anywhere that vendor and device IDs
> would be exposed to guests. Yet that's the first thing guests will need
> in order to actually recognize devices. As said before, allowing guests
> access to such r/o fields is quite likely going to be fine.
Agree, I was thinking about adding such a patch to allow IDs,
but finally decided not to add more to this series.
Again, the whole thing is not working yet and for the development
this patch can/needs to be reverted. So, either we implement IDs
or not this doesn't change anything with this respect
>
>> --- a/xen/drivers/vpci/vpci.c
>> +++ b/xen/drivers/vpci/vpci.c
>> @@ -215,11 +215,15 @@ int vpci_remove_register(struct vpci *vpci, unsigned 
>> int offset,
>>   }
>>   
>>   /* Wrappers for performing reads/writes to the underlying hardware. */
>> -static uint32_t vpci_read_hw(pci_sbdf_t sbdf, unsigned int reg,
>> +static uint32_t vpci_read_hw(bool is_hwdom, pci_sbdf_t sbdf, unsigned int 
>> reg,
>>                                unsigned int size)
> Was the passing around of a boolean the consensus which was reached?
Was this patch committed yet?
> Personally I'd fine it more natural if the two functions checked
> current->domain themselves.
This is also possible, but I would like to hear Roger's view on this as well
I am fine either way
>
> Jan
>
Thank you,
Oleksandr