[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v4 05/11] vpci/header: implement guest BAR register handlers



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




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.