[PATCH v3 6/6] vPCI: re-init extended-capability lists when MMCFG availability changed

Jan Beulich posted 6 patches 1 week, 1 day ago
[PATCH v3 6/6] vPCI: re-init extended-capability lists when MMCFG availability changed
Posted by Jan Beulich 1 week, 1 day ago
When Dom0 informs up about MMCFG usability, this may change whether
extended capabilities are available (accessible) for devices. Zap what
might be on record, and re-initialize the list.

No synchronization is added for the case where devices may already be in
use. That'll need sorting when (a) DomU support was added and (b) DomU-s
may run already while Dom0 / hwdom still boots (dom0less, Hyperlaunch).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
vpci_reinit_ext_capability_list()'s return value isn't checked, as it
doesn't feel quite right to fail the hypercall because of this. At the
same time it also doesn't feel quite right to have the function return
"void". Thoughts?
---
v3: New.

--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -8,6 +8,8 @@
 #include <xen/guest_access.h>
 #include <xen/iocap.h>
 #include <xen/serial.h>
+#include <xen/vpci.h>
+
 #include <asm/current.h>
 #include <asm/io_apic.h>
 #include <asm/msi.h>
@@ -169,7 +171,10 @@ int cf_check physdev_check_pci_extcfg(st
 
     ASSERT(pdev->seg == info->segment);
     if ( pdev->bus >= info->start_bus && pdev->bus <= info->end_bus )
+    {
         pci_check_extcfg(pdev);
+        vpci_reinit_ext_capability_list(pdev);
+    }
 
     return 0;
 }
--- 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);
+}
+
 int vpci_init_header(struct pci_dev *pdev)
 {
     uint16_t cmd;
--- a/xen/include/xen/vpci.h
+++ b/xen/include/xen/vpci.h
@@ -45,6 +45,7 @@ typedef struct {
     REGISTER_VPCI_CAPABILITY(PCI_EXT_CAP_ID_##name, name, finit, fclean, true)
 
 int __must_check vpci_init_header(struct pci_dev *pdev);
+int vpci_reinit_ext_capability_list(const struct pci_dev *pdev);
 
 /* Assign vPCI to device by adding handlers. */
 int __must_check vpci_assign_device(struct pci_dev *pdev);
@@ -300,6 +301,11 @@ bool vpci_ecam_read(pci_sbdf_t sbdf, uns
 #else /* !CONFIG_HAS_VPCI */
 struct vpci_vcpu {};
 
+static inline int vpci_reinit_ext_capability_list(const struct pci_dev *pdev)
+{
+    return 0;
+}
+
 static inline int vpci_assign_device(struct pci_dev *pdev)
 {
     return 0;
Re: [PATCH v3 6/6] vPCI: re-init extended-capability lists when MMCFG availability changed
Posted by Roger Pau Monné 1 week, 1 day ago
On Thu, Jan 29, 2026 at 02:10:34PM +0100, Jan Beulich wrote:
> When Dom0 informs up about MMCFG usability, this may change whether
> extended capabilities are available (accessible) for devices. Zap what
> might be on record, and re-initialize the list.
> 
> No synchronization is added for the case where devices may already be in
> use. That'll need sorting when (a) DomU support was added and (b) DomU-s
> may run already while Dom0 / hwdom still boots (dom0less, Hyperlaunch).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> vpci_reinit_ext_capability_list()'s return value isn't checked, as it
> doesn't feel quite right to fail the hypercall because of this. At the
> same time it also doesn't feel quite right to have the function return
> "void". Thoughts?
> ---
> v3: New.
> 
> --- a/xen/arch/x86/physdev.c
> +++ b/xen/arch/x86/physdev.c
> @@ -8,6 +8,8 @@
>  #include <xen/guest_access.h>
>  #include <xen/iocap.h>
>  #include <xen/serial.h>
> +#include <xen/vpci.h>
> +
>  #include <asm/current.h>
>  #include <asm/io_apic.h>
>  #include <asm/msi.h>
> @@ -169,7 +171,10 @@ int cf_check physdev_check_pci_extcfg(st
>  
>      ASSERT(pdev->seg == info->segment);
>      if ( pdev->bus >= info->start_bus && pdev->bus <= info->end_bus )
> +    {
>          pci_check_extcfg(pdev);
> +        vpci_reinit_ext_capability_list(pdev);
> +    }
>  
>      return 0;
>  }
> --- 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?

Thanks, Roger.
Re: [PATCH v3 6/6] vPCI: re-init extended-capability lists when MMCFG availability changed
Posted by Jan Beulich 1 week, 1 day ago
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?

Jan

Re: [PATCH v3 6/6] vPCI: re-init extended-capability lists when MMCFG availability changed
Posted by Roger Pau Monné 1 week, 1 day ago
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.