[PATCH v2] PCI: vmd: Add feature to scan BIOS enumerated devices.

[PATCH v2] PCI: vmd: Add feature to scan BIOS enumerated devices.

[Nirmal Patel]

Newer VMD  with device ID 0x28c1 has unique settings compared to its
predecessor where BIOS enumerates the entire VMD device tree and
assigns respective configurations.

VMD configuration BAR0 carries over from GNR legacy VMD as the mechanism
to access the configuration space of the devices owned by VMD. The size
of this window is fixed at 256 MB, where each function consumes 4 KB and
every bus consumes 1 MB.

The shadow and scratchpad registers have been relocated from the VMD
configuration space to the VMD MMIO space in VMD BAR4/BAR5, otherwise
refers to as MEMBAR2 or MSI-X bar.

VMD MSI-X remapping enable/disable is no longer supported.

All the VMD driver code needs to do is to obtain bus hide range along
with shadow register values set by BIOS and perform a bus scan.

The patch also involves small refactoring of vmd_enable_domain function.

Signed-off-by: Nirmal Patel <nirmal.patel@linux.intel.com>
---
v2 : Using PCI features flag instead of devic ID and fixing corner cases
     for vmd_remove_irq.
---
 drivers/pci/controller/vmd.c | 173 +++++++++++++++++++++++++++--------
 include/linux/pci_ids.h      |   1 +
 2 files changed, 135 insertions(+), 39 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index d4ae250d4bc6..ca8738285659 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -37,6 +37,11 @@
 #define MB2_SHADOW_OFFSET	0x2000
 #define MB2_SHADOW_SIZE		16
 
+/* DMR BAR4 register offsets */
+#define SHADOW_MEMBAR1_28C1		0x2818 /* MEMBAR1 physical address */
+#define SHADOW_MEMBAR2_28C1		0x2820 /* MEMBAR2 physical address */
+#define BASE_ID_REG_28C1		0x2840
+
 enum vmd_features {
 	/*
 	 * Device may contain registers which hint the physical location of the
@@ -77,6 +82,15 @@ enum vmd_features {
 	 * proper power management of the SoC.
 	 */
 	VMD_FEAT_BIOS_PM_QUIRK		= (1 << 5),
+
+	/*
+	 * Newer VMD  with device ID 0x28c1 has unique settings compared to its
+	 * predecessor where BIOS enumerates the entire VMD device tree and
+	 * stores respective configurations including bus start range and
+	 * shadow registers in VMD MMIO space in VMD BAR4/BAR5, otherwise refers
+	 * to as MEMBAR2 or MSI-X bar.
+	 */
+	VMD_FEAT_USE_BIOS_INFO		= (1 << 6),
 };
 
 #define VMD_BIOS_PM_QUIRK_LTR	0x1003	/* 3145728 ns */
@@ -142,6 +156,7 @@ struct vmd_dev {
 	u8			first_vec;
 	char			*name;
 	int			instance;
+	unsigned long		features;
 };
 
 static inline struct vmd_dev *vmd_from_bus(struct pci_bus *bus)
@@ -374,6 +389,8 @@ static void vmd_set_msi_remapping(struct vmd_dev *vmd, bool enable)
 
 static void vmd_remove_irq_domain(struct vmd_dev *vmd)
 {
+	if (!!(vmd->features & VMD_FEAT_USE_BIOS_INFO))
+		return;
 	/*
 	 * Some production BIOS won't enable remapping between soft reboots.
 	 * Ensure remapping is restored before unloading the driver.
@@ -393,7 +410,12 @@ static void __iomem *vmd_cfg_addr(struct vmd_dev *vmd, struct pci_bus *bus,
 				  unsigned int devfn, int reg, int len)
 {
 	unsigned int busnr_ecam = bus->number - vmd->busn_start;
-	u32 offset = PCIE_ECAM_OFFSET(busnr_ecam, devfn, reg);
+	u32 offset;
+
+	if (!!(vmd->features & VMD_FEAT_USE_BIOS_INFO))
+		busnr_ecam = bus->number;
+
+	offset = PCIE_ECAM_OFFSET(busnr_ecam, devfn, reg);
 
 	if (offset + len >= resource_size(&vmd->dev->resource[VMD_CFGBAR]))
 		return NULL;
@@ -661,6 +683,46 @@ static int vmd_get_bus_number_start(struct vmd_dev *vmd)
 	return 0;
 }
 
+static int vmd_get_bus_info_from_bar4(struct vmd_dev *vmd,
+				       resource_size_t *offset1,
+				       resource_size_t *offset2)
+{
+	u64 phys1, phys2, bar4_2840;
+	void __iomem *bar4;
+	u32 base_id;
+	u8 base_bus;
+
+
+	bar4 = pci_ioremap_bar(vmd->dev, 4);
+	if (!bar4)
+		return -ENOMEM;
+
+	/* Read shadow registers for MEMBAR1 and MEMBAR2 physical addresses. */
+	phys1 = readq(bar4 + SHADOW_MEMBAR1_28C1);
+	phys2 = readq(bar4 + SHADOW_MEMBAR2_28C1);
+
+	/*
+	 * Read and set bus start number from Base ID register.
+	 * 24-bit Base ID register is part of 64-bit shadowed reqid hide
+	 * range register and holds segment, bus, device and function.
+	 */
+	bar4_2840 = readq(bar4 + BASE_ID_REG_28C1);
+	base_id = bar4_2840 & 0xFFFFFF;
+	base_bus = base_id >> 8;
+	vmd->busn_start = base_bus;
+
+	/* Calculate offsets like vmd_get_phys_offsets() does. */
+	if (phys1)
+		*offset1 = vmd->dev->resource[VMD_MEMBAR1].start -
+			(phys1 & PCI_BASE_ADDRESS_MEM_MASK);
+	if (phys2)
+		*offset2 = vmd->dev->resource[VMD_MEMBAR2].start -
+			(phys2 & PCI_BASE_ADDRESS_MEM_MASK);
+
+	pci_iounmap(vmd->dev, bar4);
+	return 0;
+}
+
 static irqreturn_t vmd_irq(int irq, void *data)
 {
 	struct vmd_irq_list *irqs = data;
@@ -711,6 +773,55 @@ static int vmd_alloc_irqs(struct vmd_dev *vmd)
 	return 0;
 }
 
+static int vmd_prepare_offsets_and_bus(struct vmd_dev *vmd,
+					unsigned long features,
+					resource_size_t *membar2_offset,
+					resource_size_t *offset1,
+					resource_size_t *offset2)
+{
+	int ret;
+
+	/*
+	 * Shadow registers may exist in certain VMD device ids which allow
+	 * guests to correctly assign host physical addresses to the root ports
+	 * and child devices. These registers will either return the host value
+	 * or 0, depending on an enable bit in the VMD device.
+	 */
+	/*
+	 * For certain VMD devices (i.e. 0x28C1), BIOS places device info
+	 * in BAR4 shadow registers to determine the base bus number and memory
+	 * offsets.
+	 */
+	if (features & VMD_FEAT_USE_BIOS_INFO) {
+		if (resource_type(&vmd->dev->resource[4]) == IORESOURCE_MEM) {
+			*membar2_offset = SHADOW_MEMBAR1_28C1 + MB2_SHADOW_SIZE;
+			ret = vmd_get_bus_info_from_bar4(vmd, offset1, offset2);
+			if (ret)
+				return ret;
+		}
+	} else if (features & VMD_FEAT_HAS_MEMBAR_SHADOW) {
+		*membar2_offset = MB2_SHADOW_OFFSET + MB2_SHADOW_SIZE;
+		ret = vmd_get_phys_offsets(vmd, true, offset1, offset2);
+		if (ret)
+			return ret;
+	} else if (features & VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP) {
+		ret = vmd_get_phys_offsets(vmd, false, offset1, offset2);
+		if (ret)
+			return ret;
+	}
+
+	/*
+	 * Certain VMD devices may have a root port configuration option which
+	 * limits the bus range to between 0-127, 128-255, or 224-255.
+	 */
+	if (features & VMD_FEAT_HAS_BUS_RESTRICTIONS) {
+		ret = vmd_get_bus_number_start(vmd);
+		if (ret)
+			return ret;
+	}
+	return 0;
+}
+
 /*
  * Since VMD is an aperture to regular PCIe root ports, only allow it to
  * control features that the OS is allowed to control on the physical PCI bus.
@@ -784,38 +895,16 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 	struct pci_dev *dev;
 	int ret;
 
-	/*
-	 * Shadow registers may exist in certain VMD device ids which allow
-	 * guests to correctly assign host physical addresses to the root ports
-	 * and child devices. These registers will either return the host value
-	 * or 0, depending on an enable bit in the VMD device.
-	 */
-	if (features & VMD_FEAT_HAS_MEMBAR_SHADOW) {
-		membar2_offset = MB2_SHADOW_OFFSET + MB2_SHADOW_SIZE;
-		ret = vmd_get_phys_offsets(vmd, true, &offset[0], &offset[1]);
-		if (ret)
-			return ret;
-	} else if (features & VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP) {
-		ret = vmd_get_phys_offsets(vmd, false, &offset[0], &offset[1]);
-		if (ret)
-			return ret;
-	}
-
-	/*
-	 * Certain VMD devices may have a root port configuration option which
-	 * limits the bus range to between 0-127, 128-255, or 224-255
-	 */
-	if (features & VMD_FEAT_HAS_BUS_RESTRICTIONS) {
-		ret = vmd_get_bus_number_start(vmd);
-		if (ret)
-			return ret;
-	}
+	ret = vmd_prepare_offsets_and_bus(vmd, features, &membar2_offset,
+					  &offset[0], &offset[1]);
+	if (ret)
+		return ret;
 
 	res = &vmd->dev->resource[VMD_CFGBAR];
 	vmd->resources[0] = (struct resource) {
 		.name  = "VMD CFGBAR",
 		.start = vmd->busn_start,
-		.end   = vmd->busn_start + (resource_size(res) >> 20) - 1,
+		.end   = (resource_size(res) >> 20) - 1,
 		.flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED,
 	};
 
@@ -868,19 +957,21 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 	 * acceptable because the guest is usually CPU-limited and MSI
 	 * remapping doesn't become a performance bottleneck.
 	 */
-	if (!(features & VMD_FEAT_CAN_BYPASS_MSI_REMAP) ||
-	    offset[0] || offset[1]) {
-		ret = vmd_alloc_irqs(vmd);
-		if (ret)
-			return ret;
+	if (!(features & VMD_FEAT_USE_BIOS_INFO)) {
+		if (!(features & VMD_FEAT_CAN_BYPASS_MSI_REMAP) ||
+		    offset[0] || offset[1]) {
+			ret = vmd_alloc_irqs(vmd);
+			if (ret)
+				return ret;
 
-		vmd_set_msi_remapping(vmd, true);
+			vmd_set_msi_remapping(vmd, true);
 
-		ret = vmd_create_irq_domain(vmd);
-		if (ret)
-			return ret;
-	} else {
-		vmd_set_msi_remapping(vmd, false);
+			ret = vmd_create_irq_domain(vmd);
+			if (ret)
+				return ret;
+		} else {
+			vmd_set_msi_remapping(vmd, false);
+		}
 	}
 
 	pci_add_resource(&resources, &vmd->resources[0]);
@@ -998,6 +1089,7 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	vmd->dev = dev;
 	vmd->sysdata.domain = PCI_DOMAIN_NR_NOT_SET;
+	vmd->features = features;
 	vmd->instance = ida_alloc(&vmd_instance_ida, GFP_KERNEL);
 	if (vmd->instance < 0)
 		return vmd->instance;
@@ -1114,6 +1206,9 @@ static const struct pci_device_id vmd_ids[] = {
 		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW |
 				VMD_FEAT_HAS_BUS_RESTRICTIONS |
 				VMD_FEAT_CAN_BYPASS_MSI_REMAP,},
+	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_VMD_28C1),
+		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW |
+				VMD_FEAT_USE_BIOS_INFO,},
 	{PCI_VDEVICE(INTEL, 0x467f),
 		.driver_data = VMD_FEATS_CLIENT,},
 	{PCI_VDEVICE(INTEL, 0x4c3d),
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 24cb42f66e4b..2a8ebe7df92e 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2889,6 +2889,7 @@
 #define PCI_DEVICE_ID_INTEL_HDA_ICH8	0x284b
 #define PCI_DEVICE_ID_INTEL_ICH8_6	0x2850
 #define PCI_DEVICE_ID_INTEL_VMD_28C0	0x28c0
+#define PCI_DEVICE_ID_INTEL_VMD_28C1    0x28c1
 #define PCI_DEVICE_ID_INTEL_ICH9_0	0x2910
 #define PCI_DEVICE_ID_INTEL_ICH9_2	0x2912
 #define PCI_DEVICE_ID_INTEL_ICH9_3	0x2913
-- 
2.52.0

Re: [PATCH v2] PCI: vmd: Add feature to scan BIOS enumerated devices.

[sashiko-bot]

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

New issues:
- [High] Invalid VMD CFGBAR resource range calculated for older hardware with restricted bus ranges.
- [High] VMD shadow register exposed to the child device resource pool, risking memory corruption.
- [Medium] Silent failure and continued execution with uninitialized variables if BAR4 is not an IORESOURCE_MEM.

Pre-existing issues:
- [High] Missing pci_free_irq_vectors() calls on error paths leaks IRQ resources.
--

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

PCI: vmd: Add feature to scan BIOS enumerated devices.

Newer VMD with device ID 0x28c1 has unique settings compared to its
predecessor where BIOS enumerates the entire VMD device tree and
assigns respective configurations. This patch adds support to obtain
the bus hide range and shadow registers set by BIOS.

> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index d4ae250d4bc6d..ca8738285659a 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
[ ... ]
> @@ -711,6 +773,55 @@ static int vmd_alloc_irqs(struct vmd_dev *vmd)
>  	return 0;
>  }
>  
> +static int vmd_prepare_offsets_and_bus(struct vmd_dev *vmd,
> +					unsigned long features,
> +					resource_size_t *membar2_offset,
> +					resource_size_t *offset1,
> +					resource_size_t *offset2)
> +{
> +	int ret;
[ ... ]
> +	if (features & VMD_FEAT_USE_BIOS_INFO) {
> +		if (resource_type(&vmd->dev->resource[4]) == IORESOURCE_MEM) {
> +			*membar2_offset = SHADOW_MEMBAR1_28C1 + MB2_SHADOW_SIZE;

[Severity: High]
Does this offset calculation expose VMD configuration registers to child
devices?

The calculation SHADOW_MEMBAR1_28C1 + MB2_SHADOW_SIZE resolves to 0x2828.
However, the BASE_ID_REG_28C1 register is at offset 0x2840, which falls
after 0x2828.

If a child device maps a BAR over this region, could it overwrite the
VMD shadow register at 0x2840 and cause hardware misbehavior?

Should the offset be set large enough to protect all internal VMD registers,
such as >= 0x2848?

> +			ret = vmd_get_bus_info_from_bar4(vmd, offset1, offset2);
> +			if (ret)
> +				return ret;
> +		}

[Severity: Medium]
What happens if VMD_FEAT_USE_BIOS_INFO is set, but resource 4 is not
of type IORESOURCE_MEM?

Because the subsequent fallback checks use else if, they are bypassed
entirely. The function will silently return 0 without retrieving valid
offsets.

Will this cause the driver to probe with uninitialized offset values,
failing to handle broken hardware or firmware configurations?

> +	} else if (features & VMD_FEAT_HAS_MEMBAR_SHADOW) {
> +		*membar2_offset = MB2_SHADOW_OFFSET + MB2_SHADOW_SIZE;
> +		ret = vmd_get_phys_offsets(vmd, true, offset1, offset2);
[ ... ]
> @@ -784,38 +895,16 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
>  	struct pci_dev *dev;
>  	int ret;
[ ... ]
> -	if (features & VMD_FEAT_HAS_BUS_RESTRICTIONS) {
> -		ret = vmd_get_bus_number_start(vmd);
> -		if (ret)
> -			return ret;
> -	}
> +	ret = vmd_prepare_offsets_and_bus(vmd, features, &membar2_offset,
> +					  &offset[0], &offset[1]);
> +	if (ret)
> +		return ret;
>  
>  	res = &vmd->dev->resource[VMD_CFGBAR];
>  	vmd->resources[0] = (struct resource) {
>  		.name  = "VMD CFGBAR",
>  		.start = vmd->busn_start,
> -		.end   = vmd->busn_start + (resource_size(res) >> 20) - 1,
> +		.end   = (resource_size(res) >> 20) - 1,

[Severity: High]
Does this change break bus enumeration for older hardware with restricted bus
ranges?

By unconditionally dropping vmd->busn_start from the .end calculation,
it works for the new 28C1 device with a busn_start of 0.

However, for an older device with busn_start = 128 and a 32 MB CFGBAR
size, .start would be 128, but .end would calculate to 31.

Could this create a malformed resource where .start > .end?

>  		.flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED,
>  	};
[ ... ]
> @@ -868,19 +957,21 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
>  	 * acceptable because the guest is usually CPU-limited and MSI
>  	 * remapping doesn't become a performance bottleneck.
>  	 */
> -	if (!(features & VMD_FEAT_CAN_BYPASS_MSI_REMAP) ||
> -	    offset[0] || offset[1]) {
> -		ret = vmd_alloc_irqs(vmd);
> -		if (ret)
> -			return ret;
> +	if (!(features & VMD_FEAT_USE_BIOS_INFO)) {
> +		if (!(features & VMD_FEAT_CAN_BYPASS_MSI_REMAP) ||
> +		    offset[0] || offset[1]) {
> +			ret = vmd_alloc_irqs(vmd);
> +			if (ret)
> +				return ret;

[Severity: High]
This is a pre-existing issue, but does this error path leak IRQ resources?

According to PCI subsystem guidelines, every error path after a successful
pci_alloc_irq_vectors() must call pci_free_irq_vectors() before
returning.

If vmd_enable_domain() fails after allocating IRQs, or if
vmd_alloc_irqs() fails internally (e.g., during devm_kcalloc(),
init_srcu_struct(), or devm_request_irq()), it appears to return
directly without freeing the allocated MSI-X vectors.

Could this eventually exhaust system IRQ capacity?

> +			vmd_set_msi_remapping(vmd, true);
>  
> -		ret = vmd_create_irq_domain(vmd);
> -		if (ret)
> -			return ret;
> -	} else {
> -		vmd_set_msi_remapping(vmd, false);
> +			ret = vmd_create_irq_domain(vmd);
> +			if (ret)
> +				return ret;
> +		} else {
> +			vmd_set_msi_remapping(vmd, false);
> +		}
>  	}
>

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

Re: [PATCH v2] PCI: vmd: Add feature to scan BIOS enumerated devices.

[Nirmal Patel]

On Tue, 19 May 2026 16:05:04 +0000
sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 4 potential
> issue(s) to consider:
> 
> New issues:
> - [High] Invalid VMD CFGBAR resource range calculated for older
> hardware with restricted bus ranges.
> - [High] VMD shadow register exposed to the child device resource
> pool, risking memory corruption.
> - [Medium] Silent failure and continued execution with uninitialized
> variables if BAR4 is not an IORESOURCE_MEM.
> 
> Pre-existing issues:
> - [High] Missing pci_free_irq_vectors() calls on error paths leaks
> IRQ resources. --
> 
> commit fb1f4c20650569f7ba38772b55e3d4757083bcf4
> Author: Nirmal Patel <nirmal.patel@linux.intel.com>
> 
> PCI: vmd: Add feature to scan BIOS enumerated devices.
> 
> Newer VMD with device ID 0x28c1 has unique settings compared to its
> predecessor where BIOS enumerates the entire VMD device tree and
> assigns respective configurations. This patch adds support to obtain
> the bus hide range and shadow registers set by BIOS.
> 
> > diff --git a/drivers/pci/controller/vmd.c
> > b/drivers/pci/controller/vmd.c index d4ae250d4bc6d..ca8738285659a
> > 100644 --- a/drivers/pci/controller/vmd.c
> > +++ b/drivers/pci/controller/vmd.c  
> [ ... ]
> > @@ -711,6 +773,55 @@ static int vmd_alloc_irqs(struct vmd_dev *vmd)
> >  	return 0;
> >  }
> >  
> > +static int vmd_prepare_offsets_and_bus(struct vmd_dev *vmd,
> > +					unsigned long features,
> > +					resource_size_t
> > *membar2_offset,
> > +					resource_size_t *offset1,
> > +					resource_size_t *offset2)
> > +{
> > +	int ret;  
> [ ... ]
> > +	if (features & VMD_FEAT_USE_BIOS_INFO) {
> > +		if (resource_type(&vmd->dev->resource[4]) ==
> > IORESOURCE_MEM) {
> > +			*membar2_offset = SHADOW_MEMBAR1_28C1 +
> > MB2_SHADOW_SIZE;  
> 
> [Severity: High]
> Does this offset calculation expose VMD configuration registers to
> child devices?
No. This is for vmd driver to read and assign bus start range and
location of shadow registers.
> 
> The calculation SHADOW_MEMBAR1_28C1 + MB2_SHADOW_SIZE resolves to
> 0x2828. However, the BASE_ID_REG_28C1 register is at offset 0x2840,
> which falls after 0x2828.
> 

These two hardware registers don't have dependency. BASE_ID_REG_28C1
holds bus start value. Since the hardware register is named as Base ID,
I kept the name as it is.

> If a child device maps a BAR over this region, could it overwrite the
> VMD shadow register at 0x2840 and cause hardware misbehavior?
> 
> Should the offset be set large enough to protect all internal VMD
> registers, such as >= 0x2848?
There is no overlap. All of the these registers are separate.
64-bit SHADOW_MEMBAR1_28C1 and SHADOW_MEMBAR2_28C1 are located at
0x2818 and 0x2820. So the 16 byte size MB2_SHADOW_SIZE includes both of
these registers.
The BASE_ID_REG_28C1 is located at 0x2840, much farther from shadow
registers.
> 
> > +			ret = vmd_get_bus_info_from_bar4(vmd,
> > offset1, offset2);
> > +			if (ret)
> > +				return ret;
> > +		}  
> 
> [Severity: Medium]
> What happens if VMD_FEAT_USE_BIOS_INFO is set, but resource 4 is not
> of type IORESOURCE_MEM?
> 
> Because the subsequent fallback checks use else if, they are bypassed
> entirely. The function will silently return 0 without retrieving valid
> offsets.
> 
> Will this cause the driver to probe with uninitialized offset values,
> failing to handle broken hardware or firmware configurations?

resource 4 is needed to obtain required information for bus scan. So
unassigned resource 4 should return an error and stop the enumeration/
probe. I can return an error in else part.

> 
> > +	} else if (features & VMD_FEAT_HAS_MEMBAR_SHADOW) {
> > +		*membar2_offset = MB2_SHADOW_OFFSET +
> > MB2_SHADOW_SIZE;
> > +		ret = vmd_get_phys_offsets(vmd, true, offset1,
> > offset2);  
> [ ... ]
> > @@ -784,38 +895,16 @@ static int vmd_enable_domain(struct vmd_dev
> > *vmd, unsigned long features) struct pci_dev *dev;
> >  	int ret;  
> [ ... ]
> > -	if (features & VMD_FEAT_HAS_BUS_RESTRICTIONS) {
> > -		ret = vmd_get_bus_number_start(vmd);
> > -		if (ret)
> > -			return ret;
> > -	}
> > +	ret = vmd_prepare_offsets_and_bus(vmd, features,
> > &membar2_offset,
> > +					  &offset[0], &offset[1]);
> > +	if (ret)
> > +		return ret;
> >  
> >  	res = &vmd->dev->resource[VMD_CFGBAR];
> >  	vmd->resources[0] = (struct resource) {
> >  		.name  = "VMD CFGBAR",
> >  		.start = vmd->busn_start,
> > -		.end   = vmd->busn_start + (resource_size(res) >>
> > 20) - 1,
> > +		.end   = (resource_size(res) >> 20) - 1,  
> 
> [Severity: High]
> Does this change break bus enumeration for older hardware with
> restricted bus ranges?
> 
> By unconditionally dropping vmd->busn_start from the .end calculation,
> it works for the new 28C1 device with a busn_start of 0.
> 
> However, for an older device with busn_start = 128 and a 32 MB CFGBAR
> size, .start would be 128, but .end would calculate to 31.
> 
> Could this create a malformed resource where .start > .end?

This is a good point. But in my v1 patch, I kept vmd->busn_start +
(resource_size(res) >> 20) - 1. it was flagged as an issue.
If the busn_start is 128 and ConfigBar is 256 MB, then .end will be set
to out of bound value. I can set hard coded value of 0xff same as
pci/probe.c

> 
> >  		.flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED,
> >  	};  
> [ ... ]
> > @@ -868,19 +957,21 @@ static int vmd_enable_domain(struct vmd_dev
> > *vmd, unsigned long features)
> >  	 * acceptable because the guest is usually CPU-limited and
> > MSI
> >  	 * remapping doesn't become a performance bottleneck.
> >  	 */
> > -	if (!(features & VMD_FEAT_CAN_BYPASS_MSI_REMAP) ||
> > -	    offset[0] || offset[1]) {
> > -		ret = vmd_alloc_irqs(vmd);
> > -		if (ret)
> > -			return ret;
> > +	if (!(features & VMD_FEAT_USE_BIOS_INFO)) {
> > +		if (!(features & VMD_FEAT_CAN_BYPASS_MSI_REMAP) ||
> > +		    offset[0] || offset[1]) {
> > +			ret = vmd_alloc_irqs(vmd);
> > +			if (ret)
> > +				return ret;  
> 
> [Severity: High]
> This is a pre-existing issue, but does this error path leak IRQ
> resources?
> 
> According to PCI subsystem guidelines, every error path after a
> successful pci_alloc_irq_vectors() must call pci_free_irq_vectors()
> before returning.
> 
> If vmd_enable_domain() fails after allocating IRQs, or if
> vmd_alloc_irqs() fails internally (e.g., during devm_kcalloc(),
> init_srcu_struct(), or devm_request_irq()), it appears to return
> directly without freeing the allocated MSI-X vectors.
> 
> Could this eventually exhaust system IRQ capacity?

I can address this issue in another patch series as a bug fix or
improvement. 

> 
> > +			vmd_set_msi_remapping(vmd, true);
> >  
> > -		ret = vmd_create_irq_domain(vmd);
> > -		if (ret)
> > -			return ret;
> > -	} else {
> > -		vmd_set_msi_remapping(vmd, false);
> > +			ret = vmd_create_irq_domain(vmd);
> > +			if (ret)
> > +				return ret;
> > +		} else {
> > +			vmd_set_msi_remapping(vmd, false);
> > +		}
> >  	}
> >  
>