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

[sashiko-bot@kernel.org]

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