Re: [PATCH v3 03/11] vpci/header: Emulate legacy capability list for dom0

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, May 07, 2025 at 02:46:52AM +0000, Chen, Jiqian wrote:
> On 2025/5/6 21:50, Roger Pau Monné wrote:
> > On Mon, Apr 21, 2025 at 02:18:55PM +0800, Jiqian Chen wrote:
> >> Current logic of emulating legacy capability list is only for domU.
> >> So, expand it to emulate for dom0 too. Then it will be easy to hide
> >> a capability whose initialization fails in a function.
> >>
> >> Signed-off-by: Jiqian Chen <Jiqian.Chen@xxxxxxx>
> > 
> > Sorry, one nit I've noticed while looking at the next patch.
> > 
> >> @@ -786,13 +787,15 @@ static int vpci_init_capability_list(struct pci_dev 
> >> *pdev)
> >>  
> >>              next = pci_find_next_cap_ttl(pdev->sbdf,
> >>                                           pos + PCI_CAP_LIST_NEXT,
> >> -                                         supported_caps,
> >> -                                         ARRAY_SIZE(supported_caps), 
> >> &ttl);
> >> +                                         caps, n, &ttl);
> >>  
> >> -            rc = vpci_add_register(pdev->vpci, vpci_hw_read8, NULL,
> The same here, NULL -> vpci_hw_write8, I think.

No, not here, since the PCI_CAP_LIST_ID handler is only added for
non-hardware domains, and in that case we do want to ignore writes to
the register.

> >> -                                   pos + PCI_CAP_LIST_ID, 1, NULL);
> >> -            if ( rc )
> >> -                return rc;
> >> +            if ( !is_hwdom )
> >> +            {
> >> +                rc = vpci_add_register(pdev->vpci, vpci_hw_read8, NULL,
> >> +                                       pos + PCI_CAP_LIST_ID, 1, NULL);
> >> +                if ( rc )
> >> +                    return rc;
> >> +            }
> >>  
> >>              rc = vpci_add_register(pdev->vpci, vpci_read_val, NULL,
> > 
> > For the hardware domain the write handler should be vpci_hw_write8
> > instead of NULL.
> OK, I think I need to add definition of vpci_hw_write8.
> But I have a question, if hardware domain write this register through 
> vpci_hw_write8,
> then the "next address data" of hardware will be in consistent with vpci.
> Is it fine? Or should I update vpci's cache?

According to the spec this field is read-only, so writes should be
ignored.  We allow hardware domain full access because for hardware
domain we aim to trap as little as possible to not diverge behavior
from native, and to allow possible device quirks to work.

It could be conceivable that some vendor has a hidden specific
functionality that somehow triggered by a write to this field.

Regards, Roger.