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

[Jan Beulich <jbeulich@suse.com>]

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". But
please recall that Roger is the maintainer of the code, so he gets
the final say.

Jan