Re: [PATCH v2 2/8] vpci/header: Emulate legacy capability list for host

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

On Wed, Apr 16, 2025 at 02:51:18AM +0000, Chen, Jiqian wrote:
> On 2025/4/15 18:10, Roger Pau Monné wrote:
> > On Wed, Apr 09, 2025 at 02:45:22PM +0800, Jiqian Chen wrote:
> >> +        while ( next && ttl )
> >> +        {
> >> +            unsigned int pos = next;
> >> +
> >> +            next = pci_find_next_cap_ttl(pdev->sbdf, pos + 
> >> PCI_CAP_LIST_NEXT,
> >> +                                         caps, n, &ttl);
> >> +
> >> +            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,
> >> +                                   pos + PCI_CAP_LIST_NEXT, 1,
> >> +                                   (void *)(uintptr_t)next);
> >> +            if ( rc )
> >> +                return rc;
> >> +
> >> +            next &= ~3;
> >> +        }
> >> +    }
> >> +
> >> +    /* Utilize rsvdp_mask to hide PCI_STATUS_CAP_LIST from the guest. */
> >> +    rc = vpci_add_register_mask(pdev->vpci, vpci_hw_read16, 
> >> vpci_hw_write16,
> >> +                                PCI_STATUS, 2, NULL,
> >> +                                PCI_STATUS_RO_MASK &
> >> +                                    ~(mask_cap_list ? PCI_STATUS_CAP_LIST 
> >> : 0),
> >> +                                PCI_STATUS_RW1C_MASK,
> >> +                                mask_cap_list ? PCI_STATUS_CAP_LIST : 0,
> >> +                                PCI_STATUS_RSVDZ_MASK);
> > 
> > One further remark I've forgot to make on the previous reply: I'm
> > unsure we want to do this for dom0, as we in general allow dom0
> > unfiltered write access (unless it's for accesses mediated by Xen).
> This part is the original implementation before my patches.
> If you want to restrict this only for domUs, not for dom0.

Oh, indeed.  I was confused thinking it was inside the
!is_hardware_domain() conditional.  Leave it like that, your series is
already long enough.

Thanks, Roger.