On Thu, Jan 29, 2026 at 04:54:59PM +0100, Jan Beulich wrote:
> On 29.01.2026 16:40, Roger Pau Monné wrote:
> > On Thu, Jan 29, 2026 at 02:10:34PM +0100, Jan Beulich wrote:
> >> --- a/xen/drivers/vpci/header.c
> >> +++ b/xen/drivers/vpci/header.c
> >> @@ -869,6 +869,18 @@ static int vpci_init_ext_capability_list
> >> return 0;
> >> }
> >>
> >> +int vpci_reinit_ext_capability_list(const struct pci_dev *pdev)
> >> +{
> >> + if ( !pdev->vpci )
> >> + return 0;
> >> +
> >> + if ( vpci_remove_registers(pdev->vpci, PCI_CFG_SPACE_SIZE,
> >> + PCI_CFG_SPACE_EXP_SIZE -
> >> PCI_CFG_SPACE_SIZE) )
> >> + ASSERT_UNREACHABLE();
> >> +
> >> + return vpci_init_ext_capability_list(pdev);
> >
> > Isn't this missing the possible addition or removal of managed
> > extended capabilities? IOW: on removal of access to the extended
> > space the vPCI managed capabilties that have is_ext == true should
> > call their ->cleanup() hooks, and on discovery of MMCFG access we
> > should call the ->init() hooks?
>
> Now I know why this felt too easy. Yet I wonder: Why is this done in two
> parts in the first place?
I think this boils down to us (me I would think) not planning ahead
that capabilities might appear _after_ the initial device assignation.
This is true for example when Xen doesn't have access to the MMCFG at
boot, and it's only made aware of such after starting the hardware
domain.
Right now vpci_init_{,ext_}capability_list() is called from
vpci_init_header(), but for the case where MMCFG is registered late we
don't really want to re-init the header, so calling that helper is not
an option.
We might want to create two new wrappers that encapsulate
vpci_init_{,ext_}capability_list() + the calling of the
vpci_init_capabilities(), provided that vpci_init_capabilities() is
also split between legacy and extended capabilities. We want to call
those new helpers from vpci_assign_device() instead of
vpci_init_header().
Thanks, Roger.
