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

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>]

On 31.01.22 11:47, Oleksandr Andrushchenko wrote:
> Hi, Roger!
>
> On 12.01.22 14:35, Roger Pau Monné wrote:
>>
>>> +static void guest_rom_write(const struct pci_dev *pdev, unsigned int reg,
>>> +                            uint32_t val, void *data)
>>> +{
>>> +}
>>> +
>>> +static uint32_t guest_rom_read(const struct pci_dev *pdev, unsigned int 
>>> reg,
>>> +                               void *data)
>>> +{
>>> +    return 0xffffffff;
>>> +}
>> There should be no need for those handlers. As said elsewhere: for
>> guests registers not explicitly handled should return ~0 for reads and
>> drop writes, which is what you are proposing here.
> Yes, you are right: I can see in vpci_read that we end up reading ~0 if no
> handler exists (which is what I do here with guest_rom_read). But I am not 
> that
> sure about the dropped writes:
>
> void vpci_write(pci_sbdf_t sbdf, unsigned int reg, unsigned int size,
>                   uint32_t data)
> {
>       unsigned int data_offset = 0;
>
> [snip]
>
>       if ( data_offset < size )
>           /* Tailing gap, write the remaining. */
>           vpci_write_hw(sbdf, reg + data_offset, size - data_offset,
>                         data >> (data_offset * 8));
>
> so it looks like for the un-handled writes we still reach the HW register.
> Could you please tell if the code above needs improvement (like checking
> if the write was handled) or I still need to provide a write handler, e.g.
> guest_rom_write here?
Hm, but the same applies to the reads as well... And this is no surprise,
as for the guests I can see that it accesses all the configuration space
registers that I don't handle. Without that I would have guests unable
to properly setup a PCI device being passed through... And this is why
I have a big TODO in this series describing unhandled registers.
So, it seems that I do need to provide those handlers which I need to
drop writes and return ~0 on reads.