[PATCH] PCI: vmd: Only assign _OSC features in bare metal OS.

[PATCH] PCI: vmd: Only assign _OSC features in bare metal OS.

[Nirmal Patel]

From: Nirmal Patel <nirmal.patel@intel.com>

Copy valid _OSC flags from root bridge only when OS can provide correct
values. In virtual machine, all of the control features are disabled as
hypervisor will not passthrough root bridge information to VM.

Signed-off-by: Nirmal Patel <nirmal.patel@linux.intel.com>
Reviewed-by:  Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/pci/controller/vmd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index d4ae250d4bc6..f9e38f2f926a 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -910,8 +910,10 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 		return -ENODEV;
 	}
 
-	vmd_copy_host_bridge_flags(pci_find_host_bridge(vmd->dev->bus),
-				   to_pci_host_bridge(vmd->bus->bridge));
+	/* Don't copy _OSC control flags in VM, it disables features.*/
+	if (!offset[0] || !offset[1])
+		vmd_copy_host_bridge_flags(pci_find_host_bridge(vmd->dev->bus),
+					 to_pci_host_bridge(vmd->bus->bridge));
 
 	vmd_attach_resources(vmd);
 	if (vmd->irq_domain)
-- 
2.43.0

Re: [PATCH] PCI: vmd: Only assign _OSC features in bare metal OS.

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Incorrect logical condition for bare-metal identification copies _OSC flags in VMs.

Pre-existing issues:
- [High] Missing `pci_free_irq_vectors()` causes permanent IRQ resource leak.
- [High] Use-After-Free of SRCU structures on device teardown.
--

commit e71e6e8c7000a74dc9ba1f30dbef4f5d9f13bf08
Author: Nirmal Patel <nirmal.patel@intel.com>

PCI: vmd: Only assign _OSC features in bare metal OS.

Copy valid _OSC flags from root bridge only when OS can provide correct
values. In virtual machine, all of the control features are disabled as
hypervisor will not passthrough root bridge information to VM.

> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index d4ae250d4bc6d..f9e38f2f926a7 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -910,8 +910,10 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
>  		return -ENODEV;
>  	}
>  
> -	vmd_copy_host_bridge_flags(pci_find_host_bridge(vmd->dev->bus),
> -				   to_pci_host_bridge(vmd->bus->bridge));
> +	/* Don't copy _OSC control flags in VM, it disables features.*/
> +	if (!offset[0] || !offset[1])

[Severity: High]
Does this logical condition correctly identify a bare-metal environment?

If a VM exposes only one non-zero MEMBAR offset, this evaluates to true,
which misidentifies the VM as bare metal. This results in copying the
host bridge flags and incorrectly disabling control features in the VM.

Should this use && instead of || to ensure both offsets are zero?

> +		vmd_copy_host_bridge_flags(pci_find_host_bridge(vmd->dev->bus),
> +					 to_pci_host_bridge(vmd->bus->bridge));
>  
>  	vmd_attach_resources(vmd);
>  	if (vmd->irq_domain)

[ ... ]

[Severity: High]
This is a pre-existing issue, but does vmd_remove() leak the IRQ vectors?

vmd_alloc_irqs() allocates vectors without a devm wrapper:

drivers/pci/controller/vmd.c:vmd_alloc_irqs() {
...
    vmd->msix_count = pci_alloc_irq_vectors(dev, vmd->first_vec + 1,
                                            vmd->msix_count, PCI_IRQ_MSIX);
...
}

However, the teardown function lacks a corresponding cleanup:

drivers/pci/controller/vmd.c:vmd_remove() {
...
    vmd_cleanup_srcu(vmd);
    vmd_detach_resources(vmd);
    vmd_remove_irq_domain(vmd);
...
}

Since pcim_enable_device() doesn't exempt explicit cleanup for this API,
will this cause a permanent IRQ resource leak on module unload?

[Severity: High]
This is a pre-existing issue, but is there a use-after-free risk with the
SRCU structures during device teardown?

In vmd_remove(), the driver manually cleans up SRCU primitives:

drivers/pci/controller/vmd.c:vmd_remove() {
...
    vmd_cleanup_srcu(vmd);
...
}

Because the device's IRQ handlers are managed by devm_request_irq(), they
are not freed or unmasked until after vmd_remove() returns.

If an MSI-X interrupt fires in the intermediate window, could vmd_irq()
access the destroyed srcu_struct?

drivers/pci/controller/vmd.c:vmd_irq() {
...
    idx = srcu_read_lock(&irqs->srcu);
...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629170117.269703-1-nirmal.patel@linux.intel.com?part=1

[PATCH] PCI: vmd: Only assign _OSC features in bare metal OS.

[Nirmal Patel]

From: Nirmal Patel <nirmal.patel@intel.com>

Copy valid _OSC flags from root bridge only when OS can provide correct
values. In virtual machine, all of the control features are disabled as
hypervisor will not passthrough root bridge information to VM.

Signed-off-by: Nirmal Patel <nirmal.patel@linux.intel.com>
Reviewed-by:  Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/pci/controller/vmd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index d4ae250d4bc6..f9e38f2f926a 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -910,8 +910,10 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 		return -ENODEV;
 	}
 
-	vmd_copy_host_bridge_flags(pci_find_host_bridge(vmd->dev->bus),
-				   to_pci_host_bridge(vmd->bus->bridge));
+	/* Don't copy _OSC control flags in VM, it disables features.*/
+	if (!offset[0] || !offset[1])
+		vmd_copy_host_bridge_flags(pci_find_host_bridge(vmd->dev->bus),
+					 to_pci_host_bridge(vmd->bus->bridge));
 
 	vmd_attach_resources(vmd);
 	if (vmd->irq_domain)
-- 
2.43.0

Re: [PATCH] PCI: vmd: Only assign _OSC features in bare metal OS.

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incorrect boolean logic for detecting virtual machine environments due to a De Morgan's Law violation, leading to erroneous `_OSC` flag copying in VMs.
- [High] The VM detection heuristic used for skipping `_OSC` flags fails to account for Xen domains and older VMD hardware lacking shadow registers.
--

commit 51d3c2c481556010415fe2cd5fa77be881005b5c
Author: Nirmal Patel <nirmal.patel@intel.com>

PCI: vmd: Only assign _OSC features in bare metal OS.

Copy valid _OSC flags from root bridge only when OS can provide correct
values. In virtual machine, all of the control features are disabled as
hypervisor will not passthrough root bridge information to VM.

> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index d4ae250d4bc6d..f9e38f2f926a7 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -910,8 +910,10 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
>  		return -ENODEV;
>  	}
>  
> -	vmd_copy_host_bridge_flags(pci_find_host_bridge(vmd->dev->bus),
> -				   to_pci_host_bridge(vmd->bus->bridge));
> +	/* Don't copy _OSC control flags in VM, it disables features.*/
> +	if (!offset[0] || !offset[1])

[Severity: High]
Does this logical condition correctly identify bare-metal environments?

The patch uses !offset[0] || !offset[1] to execute bare-metal logic. However,
if a hypervisor identity-maps exactly one MEMBAR (offset is 0) while
translating the other (offset is non-zero), this condition will evaluate to
true.

Since earlier in the function offset[0] || offset[1] is used to indicate a VM
for MSI remapping, should the inverse (bare metal) be
!offset[0] && !offset[1]?

[Severity: High]
Does this VM detection heuristic also cover Xen domains and older VMD hardware?

If running in a Xen domain or a VM with older VMD hardware lacking the
VMD_FEAT_HAS_MEMBAR_SHADOW capability, both offset variables remain 0.

This causes the condition to evaluate to true, meaning the driver will still
copy the _OSC flags in these VM environments, potentially breaking PCIe
features. Could this be avoided by checking for xen_domain() or hardware
capability constraints alongside the offsets?

> +		vmd_copy_host_bridge_flags(pci_find_host_bridge(vmd->dev->bus),
> +					 to_pci_host_bridge(vmd->bus->bridge));
>  
>  	vmd_attach_resources(vmd);
>  	if (vmd->irq_domain)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629170243.269830-1-nirmal.patel@linux.intel.com?part=1

Re: [PATCH] PCI: vmd: Only assign _OSC features in bare metal OS.

[Andy Shevchenko]

On Mon, Jun 29, 2026 at 05:02:43PM +0000, Nirmal Patel wrote:

> Copy valid _OSC flags from root bridge only when OS can provide correct
> values. In virtual machine, all of the control features are disabled as
> hypervisor will not passthrough root bridge information to VM.

> Reviewed-by:  Andy Shevchenko <andriy.shevchenko@intel.com>

I confirm that I reviewed this internally and agree on the main approach and
the implementation. I think it's a redundant period in the Subject slipped,
but it maybe tweaked whilst applying I think.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] PCI: vmd: Only assign _OSC features in bare metal OS.

[Bjorn Helgaas]

On Mon, Jun 29, 2026 at 05:02:43PM +0000, Nirmal Patel wrote:
> From: Nirmal Patel <nirmal.patel@intel.com>
> 
> Copy valid _OSC flags from root bridge only when OS can provide correct
> values. In virtual machine, all of the control features are disabled as
> hypervisor will not passthrough root bridge information to VM.
> 
> Signed-off-by: Nirmal Patel <nirmal.patel@linux.intel.com>
> Reviewed-by:  Andy Shevchenko <andriy.shevchenko@intel.com>
> ---
>  drivers/pci/controller/vmd.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index d4ae250d4bc6..f9e38f2f926a 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -910,8 +910,10 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
>  		return -ENODEV;
>  	}
>  
> -	vmd_copy_host_bridge_flags(pci_find_host_bridge(vmd->dev->bus),
> -				   to_pci_host_bridge(vmd->bus->bridge));
> +	/* Don't copy _OSC control flags in VM, it disables features.*/
> +	if (!offset[0] || !offset[1])

Drop period in subject line.

Replace period in comment with space.

Kind of a weird way to check for VM.  I see it's already done this way
earlier in the function, but maybe it's time to add a descriptive
boolean based on offset[0] and offset[1].

> +		vmd_copy_host_bridge_flags(pci_find_host_bridge(vmd->dev->bus),
> +					 to_pci_host_bridge(vmd->bus->bridge));
>  
>  	vmd_attach_resources(vmd);
>  	if (vmd->irq_domain)
> -- 
> 2.43.0
>