On 19.11.2021 13:46, Oleksandr Andrushchenko wrote:
> On 19.11.21 14:37, Jan Beulich wrote:
>> On 19.11.2021 13:10, Oleksandr Andrushchenko wrote:
>>> On 19.11.21 13:58, Jan Beulich wrote:
>>>> On 05.11.2021 07:56, Oleksandr Andrushchenko wrote:
>>>>> --- a/xen/drivers/vpci/header.c
>>>>> +++ b/xen/drivers/vpci/header.c
>>>>> @@ -408,6 +408,48 @@ static void bar_write(const struct pci_dev *pdev,
>>>>> unsigned int reg,
>>>>> pci_conf_write32(pdev->sbdf, reg, val);
>>>>> }
>>>>>
>>>>> +static void guest_bar_write(const struct pci_dev *pdev, unsigned int reg,
>>>>> + uint32_t val, void *data)
>>>>> +{
>>>>> + struct vpci_bar *bar = data;
>>>>> + bool hi = false;
>>>>> +
>>>>> + if ( bar->type == VPCI_BAR_MEM64_HI )
>>>>> + {
>>>>> + ASSERT(reg > PCI_BASE_ADDRESS_0);
>>>>> + bar--;
>>>>> + hi = true;
>>>>> + }
>>>>> + else
>>>>> + {
>>>>> + val &= PCI_BASE_ADDRESS_MEM_MASK;
>>>>> + val |= bar->type == VPCI_BAR_MEM32 ? PCI_BASE_ADDRESS_MEM_TYPE_32
>>>>> + :
>>>>> PCI_BASE_ADDRESS_MEM_TYPE_64;
>>>>> + val |= bar->prefetchable ? PCI_BASE_ADDRESS_MEM_PREFETCH : 0;
>>>>> + }
>>>>> +
>>>>> + bar->guest_addr &= ~(0xffffffffull << (hi ? 32 : 0));
>>>>> + bar->guest_addr |= (uint64_t)val << (hi ? 32 : 0);
>>>>> +
>>>>> + bar->guest_addr &= ~(bar->size - 1) | ~PCI_BASE_ADDRESS_MEM_MASK;
>>>>> +}
>>>>> +
>>>>> +static uint32_t guest_bar_read(const struct pci_dev *pdev, unsigned int
>>>>> reg,
>>>>> + void *data)
>>>>> +{
>>>>> + const struct vpci_bar *bar = data;
>>>>> + bool hi = false;
>>>>> +
>>>>> + if ( bar->type == VPCI_BAR_MEM64_HI )
>>>>> + {
>>>>> + ASSERT(reg > PCI_BASE_ADDRESS_0);
>>>>> + bar--;
>>>>> + hi = true;
>>>>> + }
>>>>> +
>>>>> + return bar->guest_addr >> (hi ? 32 : 0);
>>>> I'm afraid "guest_addr" then isn't the best name; maybe "guest_val"?
>>>> This would make more obvious that there is a meaningful difference
>>>> from "addr" besides the guest vs host aspect.
>>> I am not sure I can agree here:
>>> bar->addr and bar->guest_addr make it clear what are these while
>>> bar->addr and bar->guest_val would make someone go look for
>>> additional information about what that val is for.
>> Feel free to replace "val" with something more suitable. "guest_bar"
>> maybe? The value definitely is not an address, so "addr" seems
>> inappropriate / misleading to me.
> This is a guest's view on the BAR's address. So to me it is still guest_addr
It's a guest's view on the BAR, not just the address. Or else you couldn't
simply return the value here without folding in the correct low bits.
Jan
