Re: [PATCH v5 06/14] vpci/header: implement guest BAR register handlers

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>]

On 31.01.22 17:50, Jan Beulich wrote:
> On 31.01.2022 16:06, Oleksandr Andrushchenko wrote:
>> Hi, Roger!
>>>>                rom->type = VPCI_BAR_EMPTY;
>>>>        }
>>>> diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
>>>> index ed127a08a953..0a73b14a92dc 100644
>>>> --- a/xen/include/xen/vpci.h
>>>> +++ b/xen/include/xen/vpci.h
>>>> @@ -68,7 +68,10 @@ struct vpci {
>>>>        struct vpci_header {
>>>>            /* Information about the PCI BARs of this device. */
>>>>            struct vpci_bar {
>>>> +            /* Physical view of the BAR. */
>>> No, that's not the physical view, it's the physical (host) address.
>>>
>>>>                uint64_t addr;
>>>> +            /* Guest view of the BAR: address and lower bits. */
>>>> +            uint64_t guest_reg;
>>> I continue to think it would be clearer if you store the guest address
>>> here (gaddr, without the low bits) and add those in guest_bar_read
>>> based on bar->{type,prefetchable}. Then it would be equivalent to the
>>> existing 'addr' field.
>>>
>> I agreed first to do such a change, but then recalled our discussion with 
>> Jan [1].
>> And then we decided that in order for it to be efficient it is better if we 
>> setup all the
>> things during the write phase (rare), rather then during the write phase 
>> (more often).
> Small correction: The 2nd "write" was likely meant to be "read".
Yes, this is correct.
>   But
> please recall that Roger is the maintainer of the code, so he gets
> the final say.
Agree, but would vote for the current approach as it still saves some
CPU cycles making the read operation really tiny
>
> Jan
>
Thank you,
Oleksandr