Re: [PATCH v6 2/8] PCI: vmd: Add vmd_configure_cfgbar() helper function

Re: [PATCH v6 2/8] PCI: vmd: Add vmd_configure_cfgbar() helper function

[sashiko-bot]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but there is a missing explicit `pci_free_irq_vectors()` on error paths and during device removal after a successful `pci_alloc_irq_vectors()` call, violating PCI subsystem guidelines.
--

--- Patch [2]: [PATCH v6 2/8] PCI: vmd: Add vmd_configure_cfgbar() helper function ---
commit bf3d098aa64842079a8c08e82a92ab1a8a2d4175
Author: Szymon Durawa <szymon.durawa@linux.intel.com>

PCI: vmd: Add vmd_configure_cfgbar() helper function

Move the VMD CFGBAR initialization code to a new helper
vmd_configure_cfgbar(). No functional changes.

> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index 070b247b3fbbe..2c824660cabd5 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c

[ ... ]

> @@ -857,13 +869,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
>  			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,
> -		.flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED,
> -	};
> +	vmd_configure_cfgbar(vmd);

[Severity: High]
This isn't a bug introduced by this patch, but while looking at
vmd_enable_domain() I noticed we might be leaking IRQ resources on error paths.

Earlier during setup, vmd_alloc_irqs() successfully calls
pci_alloc_irq_vectors(). However, if an error occurs later in
vmd_enable_domain(), the error paths exit like this:

        pci_bus_release_emul_domain_nr(sd->domain);
        pci_free_resource_list(&resources);
        vmd_remove_irq_domain(vmd);
        return -ENODEV;
        
Should there be a call to pci_free_irq_vectors() before returning to avoid
leaking vectors and potentially exhausting system IRQ capacity?

Similarly, the device removal logic in vmd_remove() also appears to omit the
pci_free_irq_vectors() cleanup step entirely. Does the driver need a manual
cleanup step here as well?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806221351.38592-1-szymon.durawa@linux.intel.com?part=2

Re: [PATCH v6 6/8] PCI: vmd: Convert bus and busn_start to an array

[sashiko-bot]

> Convert bus and busn_start from scalar to an array to support
> multiple VMD buses in the future. No functional changes.
> 
> Suggested-by: Nirmal Patel <nirmal.patel@linux.intel.com>
> Signed-off-by: Szymon Durawa <szymon.durawa@linux.intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806221351.38592-1-szymon.durawa@linux.intel.com?part=6

Re: [PATCH v6 5/8] PCI: vmd: Replace hardcoded values with enum and defines

[sashiko-bot]

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

New issues:
- [Medium] The patch failed to completely replace the hardcoded resource indices 1 and 2 with VMD_RES_MBAR_1 and VMD_RES_MBAR_2 when calling vmd_configure_membar().

Pre-existing issues:
- [High] Missing pci_free_irq_vectors() in error paths and module removal, leaking IRQ vectors.
--

--- Patch [5]: [PATCH v6 5/8] PCI: vmd: Replace hardcoded values with enum and defines ---
commit d7c8cf241184af8f2b0d45fc458cfb88afbce3a5
Author: Szymon Durawa <szymon.durawa@linux.intel.com>

PCI: vmd: Replace hardcoded values with enum and defines

Add enum vmd_resource type to replace hardcoded values. Add defines for
vmd bus start number based on VMD restriction value. No functional
changes.

> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index f88a72b6309e7..78e8bdf7c3d79 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c

[ ... ]

> @@ -838,8 +850,8 @@ static int vmd_configure_membar1_membar2(struct vmd_dev *vmd,
>  
>  	ret = vmd_configure_membar(vmd, 2, VMD_MEMBAR2, mbar2_ofs, 0);

[Severity: Medium]
Since the cleanup paths now use the enum value VMD_RES_MBAR_1, should this call
and the preceding call to vmd_configure_membar() for MEMBAR1 use the new enum
instead of the hardcoded 1 and 2?

>  	if (ret) {
> -		devm_kfree(&vmd->dev->dev, (void *)vmd->resources[1].name);
> -		memset(&vmd->resources[1], 0, sizeof(vmd->resources[1]));
> +		devm_kfree(&vmd->dev->dev, (void *)vmd->resources[VMD_RES_MBAR_1].name);
> +		memset(&vmd->resources[VMD_RES_MBAR_1], 0, sizeof(vmd->resources[VMD_RES_MBAR_1]));
>  		return ret;
>  	}

[Severity: High]
This is a pre-existing issue, but I noticed a potential IRQ leak during review.

In vmd_alloc_irqs(), if devm_kcalloc() fails, does this leak the IRQ vectors
allocated by pci_alloc_irq_vectors() since pci_free_irq_vectors() is not called?

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);
	if (vmd->msix_count < 0)
		return vmd->msix_count;

	vmd->irqs = devm_kcalloc(&dev->dev, vmd->msix_count, sizeof(*vmd->irqs),
				 GFP_KERNEL);
	if (!vmd->irqs)
		return -ENOMEM;
    ...
}

Similarly, does vmd_remove() permanently leak IRQs on unbind since it tears
down other resources but fails to invoke pci_free_irq_vectors()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806221351.38592-1-szymon.durawa@linux.intel.com?part=5

Re: [PATCH v6 3/8] PCI: vmd: Add vmd_configure_membar() and vmd_configure_membar1_membar2()

[sashiko-bot]

> Move the MEMBAR1 and MEMBAR2 registry initialization code to new helpers
> vmd_configure_membar() and vmd_configure_membar1_membar2().
> The refactor preserves MEMBAR address/offset/flags programming, but
> resource name construction is now generated in the helper ("VMD MEMBAR%d")
> instead of using two separate string literals.
> 
> Suggested-by: Nirmal Patel <nirmal.patel@linux.intel.com>
> Signed-off-by: Szymon Durawa <szymon.durawa@linux.intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806221351.38592-1-szymon.durawa@linux.intel.com?part=3

Re: [PATCH v6 4/8] PCI: vmd: Add vmd_create_bus()

[sashiko-bot]

> Move the VMD bus initialization code to a new helper vmd_create_bus().
> No functional changes.
> 
> Suggested-by: Nirmal Patel <nirmal.patel@linux.intel.com>
> Signed-off-by: Szymon Durawa <szymon.durawa@linux.intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806221351.38592-1-szymon.durawa@linux.intel.com?part=4

Re: [PATCH v6 1/8] PCI: vmd: Add vmd_bus_enumeration() helper function

[sashiko-bot]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but concurrent VMD driver probes will corrupt the global ACPI companion lookup hook, causing silent ACPI enumeration failures.
- [High] This problem wasn't introduced by this patch, but SRCU teardown in vmd_remove() races with active devm-managed IRQs, risking a Use-After-Free in the interrupt handler.
--

--- Patch [1]: [PATCH v6 1/8] PCI: vmd: Add vmd_bus_enumeration() helper function ---
commit 773d34aa9783fc8b62fb9cb40eac288e4cc56317
Author: Szymon Durawa <szymon.durawa@linux.intel.com>

PCI: vmd: Add vmd_bus_enumeration() helper function

Move the vmd bus enumeration code to a new helper vmd_bus_enumeration().
No functional changes.

Suggested-by: Nirmal Patel <nirmal.patel@linux.intel.com>
Signed-off-by: Szymon Durawa <szymon.durawa@linux.intel.com>

> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index d4ae250d4bc6d..070b247b3fbbe 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
[ ... ]
> @@ -771,6 +771,54 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
>  	return 0;
>  }
>  
> +static void vmd_bus_enumeration(struct pci_bus *bus, unsigned long features)
> +{
> +	struct pci_bus *child;
> +	struct pci_dev *dev;
> +	int ret;
> +
> +	vmd_acpi_begin();

[Severity: High]
This isn't a bug introduced by this patch, but could concurrent VMD driver
probes corrupt the global ACPI companion lookup hook?

If two VMD devices are probed concurrently (e.g., via PCI hotplug or parallel
sysfs bind), one thread will successfully set the ACPI hook and set the global
static hook_installed = true.

The second thread gets -EBUSY from pci_acpi_set_companion_lookup_hook, returns
early from vmd_acpi_begin(), but hook_installed remains true.

When the first thread finishes bus enumeration and calls vmd_acpi_end(), it
clears the hook. The second thread, still enumerating its bus, will then fail
to find ACPI companions for its children, breaking features like power
management and IRQ routing.

> +
> +	pci_scan_child_bus(bus);
> +	vmd_domain_reset(vmd_from_bus(bus));

[ ... ]

[Severity: High]
This is also a pre-existing issue in this file, but does SRCU teardown in
vmd_remove() race with active devm-managed IRQs, risking a use-after-free in
the interrupt handler?

When the VMD driver is unbound (e.g., via sysfs) or removed via hotplug,
vmd_remove() calls vmd_cleanup_srcu(), which destroys the SRCU structs via
cleanup_srcu_struct().

However, because the driver's interrupts were requested with
devm_request_irq(), they are not freed until the devres release phase, which
executes after vmd_remove() returns.

If an interrupt fires between cleanup_srcu_struct() and devm_free_irq(), the
vmd_irq() handler will blindly invoke srcu_read_lock() on the freed SRCU
struct.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806221351.38592-1-szymon.durawa@linux.intel.com?part=1

Re: [PATCH v6 7/8] PCI: vmd: Add support for second rootbus under VMD

[sashiko-bot]

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

New issues:
- [High] Incorrect MEMBAR2 resizing logic causes overlapping memory windows for BUS0 and truncated memory windows for BUS1.
- [High] Double free of the PCI emulated domain IDA and redundant removal of the IRQ domain on the VMD_BUS_0 creation error path.
- [High] Lockless modification of global resource tree pointers can race with concurrent readers.
- [High] Incorrect busn_start configuration for restriction type 3 assigns wrong root bus number to BUS0.

Pre-existing issues:
- [High] Missing pci_free_irq_vectors() in error paths and during device removal causes a permanent IRQ vector leak.
--

--- Patch [7]: [PATCH v6 7/8] PCI: vmd: Add support for second rootbus under VMD ---
commit 59d38ef8140ce34f05ff3140bfb153eb7ddbc2c5
Author: Szymon Durawa <szymon.durawa@linux.intel.com>
Subject: PCI: vmd: Add support for second rootbus under VMD

Starting from Intel Arrow Lake VMD enhancement introduces second root bus
support with fixed root bus number (0x80). Add new BUS1 enumeration and
divide MMIO space to be shared between both root buses.

> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index 786374965f74a..7c4379d565eaf 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c

[ ... ]

> @@ -587,12 +619,24 @@ static void vmd_attach_resources(struct vmd_dev *vmd)
>  {
>  	vmd->dev->resource[VMD_MEMBAR1].child = &vmd->resources[VMD_RES_MBAR_1];
>  	vmd->dev->resource[VMD_MEMBAR2].child = &vmd->resources[VMD_RES_MBAR_2];
> +
> +	if (vmd->bus1_rootbus) {
> +		vmd->resources[VMD_RES_MBAR_1].sibling =
> +			&vmd->resources[VMD_RES_BUS1_MBAR_1];
> +		vmd->resources[VMD_RES_MBAR_2].sibling =
> +			&vmd->resources[VMD_RES_BUS1_MBAR_2];
> +	}
>  }

[Severity: High]
Does modifying the sibling pointers here race with concurrent readers?

Directly modifying the sibling pointers of resources that are part of the
global iomem_resource tree without acquiring the resource_lock rwlock can
cause data races.

Concurrent readers, such as /proc/iomem, traversing the resource tree
while the sibling pointers are being modified here in vmd_attach_resources()
or cleared in the vmd_enable_domain() fallback error path could experience
invalid memory access.

[ ... ]

> @@ -668,6 +712,19 @@ static int vmd_get_bus_number_start(struct vmd_dev *vmd)
>  		case 2:
>  			vmd->busn_start[VMD_BUS_0] = VMD_RESTRICT_2_BUS_START;
>  			break;
> +		case 3:
> +			if (!(features & VMD_FEAT_HAS_BUS1_ROOTBUS)) {
> +				pci_err(dev, "VMD Bus Restriction detected type %d, but BUS1 Rootbus is not supported, aborting.\n",
> +					BUS_RESTRICT_CFG(reg));
> +				return -ENODEV;
> +			}
> +
> +			/* VMD (on root bus 0) secondary bus start number */
> +			vmd->busn_start[VMD_BUS_0] = VMD_RESTRICT_2_BUS_START;

[Severity: High]
Should the BUS0 start number be VMD_RESTRICT_0_BUS_START (0x00) here?

By assigning VMD_RESTRICT_2_BUS_START (0xE0) to BUS0 for restriction
type 3, vmd_create_bus() is instructed to create the BUS0 root bus
with bus number 0xE0.

However, for Arrow Lake, the BUS0 root bus is physically expected to be
at 0x00, and its CFGBAR resource range is correctly parsed and adjusted
starting at 0x00. Passing 0xE0 to pci_create_root_bus() conflicts with
the CFGBAR start address and can break device discovery on the primary
VMD bus.

[ ... ]

> @@ -850,49 +962,135 @@ static int vmd_configure_membar1_membar2(struct vmd_dev *vmd,
>  {
>  	int ret;
>  
> -	ret = vmd_configure_membar(vmd, VMD_RES_MBAR_1, VMD_MEMBAR1, 0, 0);
> -	if (ret)
> -		return ret;

[ ... ]

> +		/*
> +		 * Resize BUS MEMBAR1 and MEMBAR2 ranges to make space
> +		 * for BUS1 owned devices by adjusting range end with values
> +		 * stored in VMD_MEMBAR1_OFFSET and VMD_MEMBAR2_OFFSET registers
> +		 */
> +		ret = vmd_configure_membar(vmd, VMD_RES_MBAR_1, VMD_MEMBAR1, 0,
> +					   mbar1_sz - bus1_mbar1_ofs);
> +		if (ret)
> +			return ret;
> +
> +		ret = vmd_configure_membar(vmd, VMD_RES_MBAR_2, VMD_MEMBAR2,
> +					   mbar2_ofs, mbar2_sz - (bus1_mbar2_ofs + mbar2_ofs));
> +		if (ret)
> +			return ret;
> +
> +		ret = vmd_configure_membar(vmd, VMD_RES_BUS1_MBAR_1, VMD_MEMBAR1,
> +					   bus1_mbar1_ofs, 0);
> +		if (ret)
> +			return ret;
> +
> +		ret = vmd_configure_membar(vmd, VMD_RES_BUS1_MBAR_2, VMD_MEMBAR2,
> +					   mbar2_ofs + bus1_mbar2_ofs, 0);

[Severity: High]
Does this logic create overlapping memory windows for BUS0 and truncated
memory windows for BUS1?

The hardware splits MEMBAR2 based on bus1_mbar2_ofs, which is the
absolute offset from the BAR's base address. However, when configuring
vmd_configure_membar1_membar2(), the software-reserved shadow size
(mbar2_ofs) is added to the split point.

This extends the BUS0 region up to bus1_mbar2_ofs + mbar2_ofs - 1, and
sets the BUS1 start offset to mbar2_ofs + bus1_mbar2_ofs. Since hardware
routes based solely on bus1_mbar2_ofs, the OS will assign an overlapping
address range to BUS0 devices that hardware will incorrectly route to BUS1,
potentially breaking MMIO in the overlap region.

[ ... ]

> @@ -1036,7 +1244,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
>  
>  	sd->node = pcibus_to_node(vmd->dev->bus);
>  
> -	ret = vmd_create_bus(vmd, sd, offset);
> +	ret = vmd_create_bus(vmd, VMD_BUS_0, sd, offset, VMD_PRIMARY_BUS0);
>  	if (ret) {
>  		pci_err(vmd->dev, "Can't create bus: %d\n", ret);
>  		pci_bus_release_emul_domain_nr(sd->domain);
>  		vmd_remove_irq_domain(vmd);
>  		return ret;
>  	}

[Severity: High]
Will this result in a double free of the emulated domain IDA?

When pci_create_root_bus() fails for VMD_BUS_0 inside vmd_create_bus(),
the error path within that function now cleans up by calling
pci_bus_release_emul_domain_nr(sd->domain) and vmd_remove_irq_domain(vmd).

Since vmd_enable_domain() also checks the return code and repeats both
calls here, pci_bus_release_emul_domain_nr() is called twice on the
same domain index. This will trigger a kernel WARNING and corrupt the
IDA state structure.

[ ... ]

> @@ -1147,6 +1381,13 @@ static void vmd_remove(struct pci_dev *dev)
>  	pci_stop_root_bus(vmd->bus[VMD_BUS_0]);
>  	sysfs_remove_link(&vmd->dev->dev.kobj, "domain");
>  	pci_remove_root_bus(vmd->bus[VMD_BUS_0]);
> +
> +	if (vmd->bus1_rootbus && vmd->bus[VMD_BUS_1]) {
> +		pci_stop_root_bus(vmd->bus[VMD_BUS_1]);
> +		sysfs_remove_link(&vmd->dev->dev.kobj, "domain1");
> +		pci_remove_root_bus(vmd->bus[VMD_BUS_1]);
> +	}
> +
>  	vmd_cleanup_srcu(vmd);
>  	vmd_detach_resources(vmd);
>  	vmd_remove_irq_domain(vmd);
>  }

[Severity: High]
This is a pre-existing issue, but do we leak PCI IRQ vectors during device
removal and on probe error paths?

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

The device removal function vmd_remove() lacks a call to
pci_free_irq_vectors(). Furthermore, early returns in vmd_alloc_irqs()
and probe error paths in vmd_enable_domain() omit this cleanup. Unbinding
and rebinding the driver or failing probe will permanently leak PCI IRQ
vectors.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806221351.38592-1-szymon.durawa@linux.intel.com?part=7

Re: [PATCH v6 8/8] PCI: vmd: Add workaround for bus number hardwired to fixed non-zero value

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Dynamic bus assignment (e.g., for hotplugged switches or unconfigured bridges) behind VMD BUS1 is broken due to incomplete logical-to-physical bus number translation, leading to configuration space access failures and broken ACPI matching.
- [High] Double free of IRQ domain and emulated domain number on VMD_BUS_0 creation failure.
--

--- Patch [8]: [PATCH v6 8/8] PCI: vmd: Add workaround for bus number hardwired to fixed non-zero value ---
commit 1dce6e841a4c83aa7c67d1d1cd42002341425bb7
Author: Szymon Durawa <szymon.durawa@linux.intel.com>

PCI: vmd: Add workaround for bus number hardwired to fixed non-zero value

This commit implements a workaround for the VMD BUS1 root bus number
being fixed to 0x80 in hardware. It deconfigures the root bus number in
the first pass of pci_scan_bridge() and translates configuration accesses
and ACPI matching to work around the fixed root bus number.

> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index 7c4379d565eaf..8da2e6727282b 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -440,8 +440,22 @@ static void vmd_remove_irq_domain(struct vmd_dev *vmd)
>  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[VMD_BUS_0];
> -	u32 offset = PCIE_ECAM_OFFSET(busnr_ecam, devfn, reg);
> +	unsigned char bus_number;
> +	unsigned int busnr_ecam;
> +	u32 offset;
> +
> +	/*
> +	 * BUS1 is registered with logical bus number 0x80 to avoid bridge
> +	 * reconfiguration, but cfg accesses must target the original BUS1
> +	 * restricted range start.
> +	 */
> +	if (vmd->bus1_rootbus && bus->number == VMD_PRIMARY_BUS1)
> +		bus_number = vmd->busn_start[VMD_BUS_1];
> +	else
> +		bus_number = bus->number;
> +
> +	busnr_ecam = bus_number - vmd->busn_start[VMD_BUS_0];
> +	offset = PCIE_ECAM_OFFSET(busnr_ecam, devfn, reg);

[Severity: High]
Does this logic correctly handle subordinate buses dynamically assigned behind
BUS1?

Since vmd_cfg_addr() only remaps the exact bus number 0x80 to its physical
restricted range (e.g., 0xE1), subordinate buses like 0x81 would fall through
to the else branch.

If bus->number is 0x81, subtracting vmd->busn_start[VMD_BUS_0] (which is
0xE0) would underflow the unsigned busnr_ecam integer. This results in a
massive ECAM offset that silently fails the VMD_CFGBAR bounds check below,
returning all FFs and breaking configuration space access for dynamically
assigned buses.

[ ... ]

> @@ -520,18 +534,37 @@ static struct pci_ops vmd_ops = {
>  static struct acpi_device *vmd_acpi_find_companion(struct pci_dev *pci_dev)
>  {
>  	struct pci_host_bridge *bridge;
> -	u32 busnr, addr;
> +	struct vmd_dev *vmd;
> +	u32 addr;
> +	int busnr;
> +	u8 pci_bus_number;
> +	u8 bridge_bus_number;
>  
>  	if (pci_dev->bus->ops != &vmd_ops)
>  		return NULL;
>  
> +	vmd = vmd_from_bus(pci_dev->bus);
>  	bridge = pci_find_host_bridge(pci_dev->bus);
> -	busnr = pci_dev->bus->number - bridge->bus->number;
> +	pci_bus_number = pci_dev->bus->number;
> +	bridge_bus_number = bridge->bus->number;
> +
> +	/*
> +	 * BUS1 is registered with logical root number 0x80. For ACPI companion
> +	 * matching, compute the relative bus number against the original BUS1
> +	 * restricted range base.
> +	 */
> +	if (vmd->bus1_rootbus && bridge->bus == vmd->bus[VMD_BUS_1]) {
> +		bridge_bus_number = vmd->busn_start[VMD_BUS_1];
> +		if (pci_bus_number == VMD_PRIMARY_BUS1)
> +			pci_bus_number = vmd->busn_start[VMD_BUS_1];
> +	}
> +
> +	busnr = pci_bus_number - bridge_bus_number;
>  	/*
>  	 * The address computation below is only applicable to relative bus
>  	 * numbers below 32.
>  	 */
> -	if (busnr > 31)
> +	if (busnr < 0 || busnr > 31)
>  		return NULL;

[Severity: High]
Can this relative bus depth calculation produce negative values for OS-assigned
subordinate buses?

For a subordinate bus like 0x81, pci_bus_number remains 0x81, while
bridge_bus_number is remapped to the physical base (e.g., 0xE1). This results
in busnr being 0x81 - 0xE1 = -0x60, which fails the new busnr < 0 check and
breaks ACPI companion matching.

[ ... ]

> @@ -1045,6 +1078,7 @@ static int vmd_create_bus(struct vmd_dev *vmd, enum vmd_rootbus bus_number,
>  			  struct pci_sysdata *sd, resource_size_t *offset,
>  			  u8 primary)
>  {
> +	u8 root_busnr;
>  	u8 cfgbar = bus_number * 3;
>  	u8 membar1 = cfgbar + 1;
>  	u8 membar2 = cfgbar + 2;
> @@ -1057,8 +1091,27 @@ static int vmd_create_bus(struct vmd_dev *vmd, enum vmd_rootbus bus_number,
>  	pci_add_resource_offset(&resources, &vmd->resources[membar2],
>  				offset[1]);
>  
> -	vmd_bus = pci_create_root_bus(&vmd->dev->dev,
> -				      vmd->busn_start[bus_number], &vmd_ops, sd,
> +	/*
> +	 * Register BUS1 with its logical root number (0x80) up front so PCI core
> +	 * bridge scanning does not see a post-registration bus-number mutation.
> +	 *
> +	 * This is a workaround for pci_scan_bridge_extend() code.
> +	 * It marks bridge invalid configuration when detecting a
> +	 * non-zero (0x80) the VMD BUS1 root bus number. Thus Primary Bus Number
> +	 * of Root Ports on BUS1 is deconfigured in the first pass of
> +	 * pci_scan_bridge() to be re-assigned to 0x0 in the second pass.
> +	 * As a result no subordinate bus number behind VMD BUS1 is found.
> +	 * Workaround: VMD_BUS_1 bus number shall be set to VMD_PRIMARY_BUS1 so it has
> +	 * the same value as vmd->bus[VMD_BUS_1]->primary, it will bypass bus number
> +	 * reconfiguration.
> +	 */
> +
> +	if (bus_number == VMD_BUS_1 && vmd->bus1_rootbus)
> +		root_busnr = VMD_PRIMARY_BUS1;
> +	else
> +		root_busnr = vmd->busn_start[bus_number];
> +
> +	vmd_bus = pci_create_root_bus(&vmd->dev->dev, root_busnr, &vmd_ops, sd,
>  				      &resources);
>  
>  	if (!vmd_bus) {

[Severity: High]
Is there a double free of the IRQ domain and emulated domain number when
VMD_BUS_0 creation fails?

Inside vmd_create_bus(), the error path for !vmd_bus explicitly cleans up:

    if (bus_number == VMD_BUS_0) {
        pci_bus_release_emul_domain_nr(sd->domain);
        vmd_remove_irq_domain(vmd);
    }
    return -ENODEV;

However, the caller vmd_enable_domain() intercepts this error return and
mistakenly executes the exact same cleanup operations:

    ret = vmd_create_bus(vmd, VMD_BUS_0, sd, offset, VMD_PRIMARY_BUS0);
    if (ret) {
        pci_err(vmd->dev, "Can't create bus: %d\n", ret);
        pci_bus_release_emul_domain_nr(sd->domain);
        vmd_remove_irq_domain(vmd);
        return ret;
    }

Because vmd_remove_irq_domain() does not set vmd->irq_domain to NULL, this
appears to trigger an immediate double free of the fwnode and irq domain
structure. The IDA structure also gets freed twice.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806221351.38592-1-szymon.durawa@linux.intel.com?part=8

[PATCH v6 0/8] VMD add second root bus support

[Szymon Durawa]

This patch series implements second rootbus support inside Intel VMD
module. Current implementation allows VMD to take ownership of devices
only on first bus (Rootbus0). Starting from Intel Arrow Lake, VMD exposes
second bus (Rootbus1) to allow VMD to own devices on this bus as well.
VMD MMIO BARs (CFGBAR. MEMBAR1 and MEMBAR2) are now shared between
Rootbus0 and Rootbus1. Reconfiguration of 3 MMIO BARs is required by
resizing current MMIO BARs ranges. It allows to find/register VMD Rootbus1
and discovers devices or root ports under it.

Patches 1 to 6 introduce code refactoring without functional changes.
Patch 7 implements VMD Rootbus1 support and patch 8 provides workaround
for rootbus number hardwired to fixed non-zero value. Patch 8 is necessary
for correct enumeration attached devices under VMD Rootbus1. Without it
user cannot access those devices as they are not visible in the system,
only drives under VMD Rootbus0 are available to the user.

Changes from v1:
- splitting series into more commits, requested by Bjorn
- adding helper functions, suggested by Bjorn
- minor typos and unclear wording updated, suggested by Bjorn

Changes from v2:
- wording update in commit logs, suggested by Bjorn

Changes from v3:
- using GENMASK() instead of manual bits shifting, suggested by Bjorn
- converting decimal number to hex representation, suggested by Bjorn
- wording update in commit logs, suggested by Bjorn

Changes from v4:
- Update Dan's email address
- Resending the whole series to the correct mailing list

Changes from v5:
- Addressed review feedback from Sashiko AI:
 * Critical correctness in bus/resource layout in patches 7 and 8
 * Lifetime and Use-After-Free risks in patches 3 and 7
 * Resource tree integrity issues in patch 7
 * Memory leak issues on error paths across patches 3, 4 and 5
 * Allocation failure handling in patch 3
- Note on Sashiko feedback: Pre-existing warnings (unrelated to 
  this feature series) were left untouched.


Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Dan Williams <djbw@kernel.org>
Cc: Lukas Wunner <lukas@wunner.de>
Cc: linux-pci@vger.kernel.org
Suggested-by: Nirmal Patel <nirmal.patel@linux.intel.com>
Signed-off-by: Szymon Durawa <szymon.durawa@linux.intel.com>

Szymon Durawa (8):
  PCI: vmd: Add vmd_bus_enumeration() helper function
  PCI: vmd: Add vmd_configure_cfgbar() helper function
  PCI: vmd: Add vmd_configure_membar() and
    vmd_configure_membar1_membar2()
  PCI: vmd: Add vmd_create_bus()
  PCI: vmd: Replace hardcoded values with enum and defines
  PCI: vmd: Convert bus and busn_start to an array
  PCI: vmd: Add support for second rootbus under VMD
  PCI: vmd: Add workaround for bus number hardwired to fixed non-zero
    value

 drivers/pci/controller/vmd.c | 612 ++++++++++++++++++++++++++++-------
 1 file changed, 497 insertions(+), 115 deletions(-)

-- 
2.43.0

[PATCH v6 1/8] PCI: vmd: Add vmd_bus_enumeration() helper function

[Szymon Durawa]

Move the vmd bus enumeration code to a new helper vmd_bus_enumeration().
No functional changes.

Suggested-by: Nirmal Patel <nirmal.patel@linux.intel.com>
Signed-off-by: Szymon Durawa <szymon.durawa@linux.intel.com>
---
 drivers/pci/controller/vmd.c | 89 ++++++++++++++++++++----------------
 1 file changed, 49 insertions(+), 40 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index d4ae250d4bc6..070b247b3fbb 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -771,6 +771,54 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
 	return 0;
 }
 
+static void vmd_bus_enumeration(struct pci_bus *bus, unsigned long features)
+{
+	struct pci_bus *child;
+	struct pci_dev *dev;
+	int ret;
+
+	vmd_acpi_begin();
+
+	pci_scan_child_bus(bus);
+	vmd_domain_reset(vmd_from_bus(bus));
+
+	/*
+	 * When Intel VMD is enabled, the OS does not discover the Root Ports
+	 * owned by Intel VMD within the MMCFG space. pci_reset_bus() applies
+	 * a reset to the parent of the PCI device supplied as argument. This
+	 * is why we pass a child device, so the reset can be triggered at
+	 * the Intel bridge level and propagated to all the children in the
+	 * hierarchy.
+	 */
+	list_for_each_entry(child, &bus->children, node) {
+		if (!list_empty(&child->devices)) {
+			dev = list_first_entry(&child->devices, struct pci_dev,
+					       bus_list);
+			ret = pci_reset_bus(dev);
+			if (ret)
+				pci_warn(dev, "can't reset device: %d\n", ret);
+
+			break;
+		}
+	}
+
+	pci_assign_unassigned_bus_resources(bus);
+
+	pci_walk_bus(bus, vmd_pm_enable_quirk, &features);
+
+	/*
+	 * VMD root buses are virtual and don't return true on pci_is_pcie()
+	 * and will fail pcie_bus_configure_settings() early. It can instead be
+	 * run on each of the real root ports.
+	 */
+	list_for_each_entry(child, &bus->children, node)
+		pcie_bus_configure_settings(child);
+
+	pci_bus_add_devices(bus);
+
+	vmd_acpi_end();
+}
+
 static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 {
 	struct pci_sysdata *sd = &vmd->sysdata;
@@ -780,8 +828,6 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 	LIST_HEAD(resources);
 	resource_size_t offset[2] = {0};
 	resource_size_t membar2_offset = 0x2000;
-	struct pci_bus *child;
-	struct pci_dev *dev;
 	int ret;
 
 	/*
@@ -923,45 +969,8 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 	WARN(sysfs_create_link(&vmd->dev->dev.kobj, &vmd->bus->dev.kobj,
 			       "domain"), "Can't create symlink to domain\n");
 
-	vmd_acpi_begin();
-
-	pci_scan_child_bus(vmd->bus);
-	vmd_domain_reset(vmd);
+	vmd_bus_enumeration(vmd->bus, features);
 
-	/* When Intel VMD is enabled, the OS does not discover the Root Ports
-	 * owned by Intel VMD within the MMCFG space. pci_reset_bus() applies
-	 * a reset to the parent of the PCI device supplied as argument. This
-	 * is why we pass a child device, so the reset can be triggered at
-	 * the Intel bridge level and propagated to all the children in the
-	 * hierarchy.
-	 */
-	list_for_each_entry(child, &vmd->bus->children, node) {
-		if (!list_empty(&child->devices)) {
-			dev = list_first_entry(&child->devices,
-					       struct pci_dev, bus_list);
-			ret = pci_reset_bus(dev);
-			if (ret)
-				pci_warn(dev, "can't reset device: %d\n", ret);
-
-			break;
-		}
-	}
-
-	pci_assign_unassigned_bus_resources(vmd->bus);
-
-	pci_walk_bus(vmd->bus, vmd_pm_enable_quirk, &features);
-
-	/*
-	 * VMD root buses are virtual and don't return true on pci_is_pcie()
-	 * and will fail pcie_bus_configure_settings() early. It can instead be
-	 * run on each of the real root ports.
-	 */
-	list_for_each_entry(child, &vmd->bus->children, node)
-		pcie_bus_configure_settings(child);
-
-	pci_bus_add_devices(vmd->bus);
-
-	vmd_acpi_end();
 	return 0;
 }
 
-- 
2.43.0

[PATCH v6 2/8] PCI: vmd: Add vmd_configure_cfgbar() helper function

[Szymon Durawa]

Move the VMD CFGBAR initialization code to a new helper
vmd_configure_cfgbar(). No functional changes.

Suggested-by: Nirmal Patel <nirmal.patel@linux.intel.com>
Signed-off-by: Szymon Durawa <szymon.durawa@linux.intel.com>
---
 drivers/pci/controller/vmd.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index 070b247b3fbb..2c824660cabd 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -771,6 +771,18 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
 	return 0;
 }
 
+static void vmd_configure_cfgbar(struct vmd_dev *vmd)
+{
+	struct resource *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,
+		.flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED,
+	};
+}
+
 static void vmd_bus_enumeration(struct pci_bus *bus, unsigned long features)
 {
 	struct pci_bus *child;
@@ -857,13 +869,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 			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,
-		.flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED,
-	};
+	vmd_configure_cfgbar(vmd);
 
 	/*
 	 * If the window is below 4GB, clear IORESOURCE_MEM_64 so we can
-- 
2.43.0

[PATCH v6 3/8] PCI: vmd: Add vmd_configure_membar() and vmd_configure_membar1_membar2()

[Szymon Durawa]

Move the MEMBAR1 and MEMBAR2 registry initialization code to new helpers
vmd_configure_membar() and vmd_configure_membar1_membar2().
The refactor preserves MEMBAR address/offset/flags programming, but
resource name construction is now generated in the helper ("VMD MEMBAR%d")
instead of using two separate string literals.

Suggested-by: Nirmal Patel <nirmal.patel@linux.intel.com>
Signed-off-by: Szymon Durawa <szymon.durawa@linux.intel.com>
---
 drivers/pci/controller/vmd.c | 98 +++++++++++++++++++++++++-----------
 1 file changed, 68 insertions(+), 30 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index 2c824660cabd..62bc9b91300b 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -783,6 +783,69 @@ static void vmd_configure_cfgbar(struct vmd_dev *vmd)
 	};
 }
 
+/*
+ * vmd_configure_membar - Configure VMD MemBAR register, which points
+ * to MMIO address assigned by the OS or BIOS.
+ * @vmd: the VMD device
+ * @resource_number: resource buffer number to be filled in
+ * @membar_number: number of the MemBAR
+ * @start_offset: 4K aligned offset applied to start of VMD’s MEMBAR MMIO space
+ * @end_offset: 4K aligned offset applied to end of VMD’s MEMBAR MMIO space
+ *
+ * Function fills resource buffer inside the VMD structure.
+ *
+ * Return: 0 on success, -ENOMEM on allocation failure.
+ */
+static int vmd_configure_membar(struct vmd_dev *vmd, u8 resource_number,
+				u8 membar_number, resource_size_t start_offset,
+				resource_size_t end_offset)
+{
+	char *name;
+	u32 upper_bits;
+	unsigned long flags;
+
+	struct resource *res = &vmd->dev->resource[membar_number];
+
+	upper_bits = upper_32_bits(res->end);
+	flags = res->flags & ~IORESOURCE_SIZEALIGN;
+	if (!upper_bits)
+		flags &= ~IORESOURCE_MEM_64;
+
+	name = devm_kasprintf(&vmd->dev->dev, GFP_KERNEL, "VMD MEMBAR%d",
+			      resource_number);
+	if (!name)
+		return -ENOMEM;
+
+	vmd->resources[resource_number] = (struct resource){
+		.name = name,
+		.start = res->start + start_offset,
+		.end = res->end - end_offset,
+		.flags = flags,
+		.parent = res,
+	};
+
+	return 0;
+}
+
+static int vmd_configure_membar1_membar2(struct vmd_dev *vmd,
+					 resource_size_t mbar2_ofs)
+{
+	int ret;
+
+	ret = vmd_configure_membar(vmd, 1, VMD_MEMBAR1, 0, 0);
+	if (ret)
+		return ret;
+
+	ret = vmd_configure_membar(vmd, 2, VMD_MEMBAR2, mbar2_ofs, 0);
+	if (ret) {
+		devm_kfree(&vmd->dev->dev, (void *)vmd->resources[1].name);
+		memset(&vmd->resources[1], 0, sizeof(vmd->resources[1]));
+		return ret;
+	}
+
+	return 0;
+}
+
 static void vmd_bus_enumeration(struct pci_bus *bus, unsigned long features)
 {
 	struct pci_bus *child;
@@ -834,9 +897,6 @@ static void vmd_bus_enumeration(struct pci_bus *bus, unsigned long features)
 static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 {
 	struct pci_sysdata *sd = &vmd->sysdata;
-	struct resource *res;
-	u32 upper_bits;
-	unsigned long flags;
 	LIST_HEAD(resources);
 	resource_size_t offset[2] = {0};
 	resource_size_t membar2_offset = 0x2000;
@@ -883,36 +943,14 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 	 *
 	 * The only way we could use a 64-bit non-prefetchable MEMBAR is
 	 * if its address is <4GB so that we can convert it to a 32-bit
-	 * resource.  To be visible to the host OS, all VMD endpoints must
+	 * resource. To be visible to the host OS, all VMD endpoints must
 	 * be initially configured by platform BIOS, which includes setting
-	 * up these resources.  We can assume the device is configured
+	 * up these resources. We can assume the device is configured
 	 * according to the platform needs.
 	 */
-	res = &vmd->dev->resource[VMD_MEMBAR1];
-	upper_bits = upper_32_bits(res->end);
-	flags = res->flags & ~IORESOURCE_SIZEALIGN;
-	if (!upper_bits)
-		flags &= ~IORESOURCE_MEM_64;
-	vmd->resources[1] = (struct resource) {
-		.name  = "VMD MEMBAR1",
-		.start = res->start,
-		.end   = res->end,
-		.flags = flags,
-		.parent = res,
-	};
-
-	res = &vmd->dev->resource[VMD_MEMBAR2];
-	upper_bits = upper_32_bits(res->end);
-	flags = res->flags & ~IORESOURCE_SIZEALIGN;
-	if (!upper_bits)
-		flags &= ~IORESOURCE_MEM_64;
-	vmd->resources[2] = (struct resource) {
-		.name  = "VMD MEMBAR2",
-		.start = res->start + membar2_offset,
-		.end   = res->end,
-		.flags = flags,
-		.parent = res,
-	};
+	ret = vmd_configure_membar1_membar2(vmd, membar2_offset);
+	if (ret)
+		return ret;
 
 	/*
 	 * Currently MSI remapping must be enabled in guest passthrough mode
-- 
2.43.0

[PATCH v6 4/8] PCI: vmd: Add vmd_create_bus()

[Szymon Durawa]

Move the VMD bus initialization code to a new helper vmd_create_bus().
No functional changes.

Suggested-by: Nirmal Patel <nirmal.patel@linux.intel.com>
Signed-off-by: Szymon Durawa <szymon.durawa@linux.intel.com>
---
 drivers/pci/controller/vmd.c | 53 ++++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 20 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index 62bc9b91300b..f88a72b6309e 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -846,6 +846,35 @@ static int vmd_configure_membar1_membar2(struct vmd_dev *vmd,
 	return 0;
 }
 
+static int vmd_create_bus(struct vmd_dev *vmd, struct pci_sysdata *sd,
+			  resource_size_t *offset)
+{
+	LIST_HEAD(resources);
+
+	pci_add_resource(&resources, &vmd->resources[0]);
+	pci_add_resource_offset(&resources, &vmd->resources[1], offset[0]);
+	pci_add_resource_offset(&resources, &vmd->resources[2], offset[1]);
+
+	vmd->bus = pci_create_root_bus(&vmd->dev->dev, vmd->busn_start,
+				       &vmd_ops, sd, &resources);
+	if (!vmd->bus) {
+		pci_free_resource_list(&resources);
+		return -ENODEV;
+	}
+
+	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)
+		dev_set_msi_domain(&vmd->bus->dev, vmd->irq_domain);
+	else
+		dev_set_msi_domain(&vmd->bus->dev,
+				   dev_get_msi_domain(&vmd->dev->dev));
+
+	return 0;
+}
+
 static void vmd_bus_enumeration(struct pci_bus *bus, unsigned long features)
 {
 	struct pci_bus *child;
@@ -897,7 +926,6 @@ static void vmd_bus_enumeration(struct pci_bus *bus, unsigned long features)
 static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 {
 	struct pci_sysdata *sd = &vmd->sysdata;
-	LIST_HEAD(resources);
 	resource_size_t offset[2] = {0};
 	resource_size_t membar2_offset = 0x2000;
 	int ret;
@@ -973,10 +1001,6 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 		vmd_set_msi_remapping(vmd, false);
 	}
 
-	pci_add_resource(&resources, &vmd->resources[0]);
-	pci_add_resource_offset(&resources, &vmd->resources[1], offset[0]);
-	pci_add_resource_offset(&resources, &vmd->resources[2], offset[1]);
-
 	sd->vmd_dev = vmd->dev;
 
 	/*
@@ -991,25 +1015,14 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 
 	sd->node = pcibus_to_node(vmd->dev->bus);
 
-	vmd->bus = pci_create_root_bus(&vmd->dev->dev, vmd->busn_start,
-				       &vmd_ops, sd, &resources);
-	if (!vmd->bus) {
+	ret = vmd_create_bus(vmd, sd, offset);
+	if (ret) {
+		pci_err(vmd->dev, "Can't create bus: %d\n", ret);
 		pci_bus_release_emul_domain_nr(sd->domain);
-		pci_free_resource_list(&resources);
 		vmd_remove_irq_domain(vmd);
-		return -ENODEV;
+		return ret;
 	}
 
-	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)
-		dev_set_msi_domain(&vmd->bus->dev, vmd->irq_domain);
-	else
-		dev_set_msi_domain(&vmd->bus->dev,
-				   dev_get_msi_domain(&vmd->dev->dev));
-
 	WARN(sysfs_create_link(&vmd->dev->dev.kobj, &vmd->bus->dev.kobj,
 			       "domain"), "Can't create symlink to domain\n");
 
-- 
2.43.0

[PATCH v6 5/8] PCI: vmd: Replace hardcoded values with enum and defines

[Szymon Durawa]

Add enum vmd_resource type to replace hardcoded values. Add defines for
vmd bus start number based on VMD restriction value. No functional
changes.

Suggested-by: Nirmal Patel <nirmal.patel@linux.intel.com>
Signed-off-by: Szymon Durawa <szymon.durawa@linux.intel.com>
---
 drivers/pci/controller/vmd.c | 40 ++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index f88a72b6309e..78e8bdf7c3d7 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -26,6 +26,11 @@
 #define VMD_MEMBAR1	2
 #define VMD_MEMBAR2	4
 
+/* VMD restriction value determines secondary start bus number */
+#define VMD_RESTRICT_0_BUS_START 0x0
+#define VMD_RESTRICT_1_BUS_START 0x80
+#define VMD_RESTRICT_2_BUS_START 0xE0
+
 #define PCI_REG_VMCAP		0x40
 #define BUS_RESTRICT_CAP(vmcap)	(vmcap & 0x1)
 #define PCI_REG_VMCONFIG	0x44
@@ -37,6 +42,13 @@
 #define MB2_SHADOW_OFFSET	0x2000
 #define MB2_SHADOW_SIZE		16
 
+enum vmd_resource {
+	VMD_RES_CFGBAR = 0, /* VMD Bus0 Config BAR */
+	VMD_RES_MBAR_1, /* VMD Bus0 Resource MemBAR 1 */
+	VMD_RES_MBAR_2, /* VMD Bus0 Resource MemBAR 2 */
+	VMD_RES_COUNT
+};
+
 enum vmd_features {
 	/*
 	 * Device may contain registers which hint the physical location of the
@@ -135,7 +147,7 @@ struct vmd_dev {
 	struct vmd_irq_list	*irqs;
 
 	struct pci_sysdata	sysdata;
-	struct resource		resources[3];
+	struct resource		resources[VMD_RES_COUNT];
 	struct irq_domain	*irq_domain;
 	struct pci_bus		*bus;
 	u8			busn_start;
@@ -520,7 +532,7 @@ static inline void vmd_acpi_end(void) { }
 
 static void vmd_domain_reset(struct vmd_dev *vmd)
 {
-	u16 bus, max_buses = resource_size(&vmd->resources[0]);
+	u16 bus, max_buses = resource_size(&vmd->resources[VMD_RES_CFGBAR]);
 	u8 dev, functions, fn, hdr_type;
 	char __iomem *base;
 
@@ -568,8 +580,8 @@ static void vmd_domain_reset(struct vmd_dev *vmd)
 
 static void vmd_attach_resources(struct vmd_dev *vmd)
 {
-	vmd->dev->resource[VMD_MEMBAR1].child = &vmd->resources[1];
-	vmd->dev->resource[VMD_MEMBAR2].child = &vmd->resources[2];
+	vmd->dev->resource[VMD_MEMBAR1].child = &vmd->resources[VMD_RES_MBAR_1];
+	vmd->dev->resource[VMD_MEMBAR2].child = &vmd->resources[VMD_RES_MBAR_2];
 }
 
 static void vmd_detach_resources(struct vmd_dev *vmd)
@@ -643,13 +655,13 @@ static int vmd_get_bus_number_start(struct vmd_dev *vmd)
 
 		switch (BUS_RESTRICT_CFG(reg)) {
 		case 0:
-			vmd->busn_start = 0;
+			vmd->busn_start = VMD_RESTRICT_0_BUS_START;
 			break;
 		case 1:
-			vmd->busn_start = 128;
+			vmd->busn_start = VMD_RESTRICT_1_BUS_START;
 			break;
 		case 2:
-			vmd->busn_start = 224;
+			vmd->busn_start = VMD_RESTRICT_2_BUS_START;
 			break;
 		default:
 			pci_err(dev, "Unknown Bus Offset Setting (%d)\n",
@@ -775,7 +787,7 @@ static void vmd_configure_cfgbar(struct vmd_dev *vmd)
 {
 	struct resource *res = &vmd->dev->resource[VMD_CFGBAR];
 
-	vmd->resources[0] = (struct resource){
+	vmd->resources[VMD_RES_CFGBAR] = (struct resource){
 		.name = "VMD CFGBAR",
 		.start = vmd->busn_start,
 		.end = vmd->busn_start + (resource_size(res) >> 20) - 1,
@@ -838,8 +850,8 @@ static int vmd_configure_membar1_membar2(struct vmd_dev *vmd,
 
 	ret = vmd_configure_membar(vmd, 2, VMD_MEMBAR2, mbar2_ofs, 0);
 	if (ret) {
-		devm_kfree(&vmd->dev->dev, (void *)vmd->resources[1].name);
-		memset(&vmd->resources[1], 0, sizeof(vmd->resources[1]));
+		devm_kfree(&vmd->dev->dev, (void *)vmd->resources[VMD_RES_MBAR_1].name);
+		memset(&vmd->resources[VMD_RES_MBAR_1], 0, sizeof(vmd->resources[VMD_RES_MBAR_1]));
 		return ret;
 	}
 
@@ -851,9 +863,11 @@ static int vmd_create_bus(struct vmd_dev *vmd, struct pci_sysdata *sd,
 {
 	LIST_HEAD(resources);
 
-	pci_add_resource(&resources, &vmd->resources[0]);
-	pci_add_resource_offset(&resources, &vmd->resources[1], offset[0]);
-	pci_add_resource_offset(&resources, &vmd->resources[2], offset[1]);
+	pci_add_resource(&resources, &vmd->resources[VMD_RES_CFGBAR]);
+	pci_add_resource_offset(&resources, &vmd->resources[VMD_RES_MBAR_1],
+				offset[0]);
+	pci_add_resource_offset(&resources, &vmd->resources[VMD_RES_MBAR_2],
+				offset[1]);
 
 	vmd->bus = pci_create_root_bus(&vmd->dev->dev, vmd->busn_start,
 				       &vmd_ops, sd, &resources);
-- 
2.43.0

[PATCH v6 6/8] PCI: vmd: Convert bus and busn_start to an array

[Szymon Durawa]

Convert bus and busn_start from scalar to an array to support
multiple VMD buses in the future. No functional changes.

Suggested-by: Nirmal Patel <nirmal.patel@linux.intel.com>
Signed-off-by: Szymon Durawa <szymon.durawa@linux.intel.com>
---
 drivers/pci/controller/vmd.c | 50 +++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 21 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index 78e8bdf7c3d7..786374965f74 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -49,6 +49,11 @@ enum vmd_resource {
 	VMD_RES_COUNT
 };
 
+enum vmd_rootbus {
+	VMD_BUS_0 = 0,
+	VMD_BUS_COUNT
+};
+
 enum vmd_features {
 	/*
 	 * Device may contain registers which hint the physical location of the
@@ -149,8 +154,8 @@ struct vmd_dev {
 	struct pci_sysdata	sysdata;
 	struct resource		resources[VMD_RES_COUNT];
 	struct irq_domain	*irq_domain;
-	struct pci_bus		*bus;
-	u8			busn_start;
+	struct pci_bus		*bus[VMD_BUS_COUNT];
+	u8			busn_start[VMD_BUS_COUNT];
 	u8			first_vec;
 	char			*name;
 	int			instance;
@@ -404,7 +409,7 @@ static void vmd_remove_irq_domain(struct vmd_dev *vmd)
 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;
+	unsigned int busnr_ecam = bus->number - vmd->busn_start[VMD_BUS_0];
 	u32 offset = PCIE_ECAM_OFFSET(busnr_ecam, devfn, reg);
 
 	if (offset + len >= resource_size(&vmd->dev->resource[VMD_CFGBAR]))
@@ -655,13 +660,13 @@ static int vmd_get_bus_number_start(struct vmd_dev *vmd)
 
 		switch (BUS_RESTRICT_CFG(reg)) {
 		case 0:
-			vmd->busn_start = VMD_RESTRICT_0_BUS_START;
+			vmd->busn_start[VMD_BUS_0] = VMD_RESTRICT_0_BUS_START;
 			break;
 		case 1:
-			vmd->busn_start = VMD_RESTRICT_1_BUS_START;
+			vmd->busn_start[VMD_BUS_0] = VMD_RESTRICT_1_BUS_START;
 			break;
 		case 2:
-			vmd->busn_start = VMD_RESTRICT_2_BUS_START;
+			vmd->busn_start[VMD_BUS_0] = VMD_RESTRICT_2_BUS_START;
 			break;
 		default:
 			pci_err(dev, "Unknown Bus Offset Setting (%d)\n",
@@ -789,8 +794,9 @@ static void vmd_configure_cfgbar(struct vmd_dev *vmd)
 
 	vmd->resources[VMD_RES_CFGBAR] = (struct resource){
 		.name = "VMD CFGBAR",
-		.start = vmd->busn_start,
-		.end = vmd->busn_start + (resource_size(res) >> 20) - 1,
+		.start = vmd->busn_start[VMD_BUS_0],
+		.end = vmd->busn_start[VMD_BUS_0] +
+		       (resource_size(res) >> 20) - 1,
 		.flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED,
 	};
 }
@@ -844,11 +850,11 @@ static int vmd_configure_membar1_membar2(struct vmd_dev *vmd,
 {
 	int ret;
 
-	ret = vmd_configure_membar(vmd, 1, VMD_MEMBAR1, 0, 0);
+	ret = vmd_configure_membar(vmd, VMD_RES_MBAR_1, VMD_MEMBAR1, 0, 0);
 	if (ret)
 		return ret;
 
-	ret = vmd_configure_membar(vmd, 2, VMD_MEMBAR2, mbar2_ofs, 0);
+	ret = vmd_configure_membar(vmd, VMD_RES_MBAR_2, VMD_MEMBAR2, mbar2_ofs, 0);
 	if (ret) {
 		devm_kfree(&vmd->dev->dev, (void *)vmd->resources[VMD_RES_MBAR_1].name);
 		memset(&vmd->resources[VMD_RES_MBAR_1], 0, sizeof(vmd->resources[VMD_RES_MBAR_1]));
@@ -869,21 +875,22 @@ static int vmd_create_bus(struct vmd_dev *vmd, struct pci_sysdata *sd,
 	pci_add_resource_offset(&resources, &vmd->resources[VMD_RES_MBAR_2],
 				offset[1]);
 
-	vmd->bus = pci_create_root_bus(&vmd->dev->dev, vmd->busn_start,
-				       &vmd_ops, sd, &resources);
-	if (!vmd->bus) {
+	vmd->bus[VMD_BUS_0] = pci_create_root_bus(&vmd->dev->dev,
+						  vmd->busn_start[VMD_BUS_0],
+						  &vmd_ops, sd, &resources);
+	if (!vmd->bus[VMD_BUS_0]) {
 		pci_free_resource_list(&resources);
 		return -ENODEV;
 	}
 
 	vmd_copy_host_bridge_flags(pci_find_host_bridge(vmd->dev->bus),
-				   to_pci_host_bridge(vmd->bus->bridge));
+				   to_pci_host_bridge(vmd->bus[VMD_BUS_0]->bridge));
 
 	vmd_attach_resources(vmd);
 	if (vmd->irq_domain)
-		dev_set_msi_domain(&vmd->bus->dev, vmd->irq_domain);
+		dev_set_msi_domain(&vmd->bus[VMD_BUS_0]->dev, vmd->irq_domain);
 	else
-		dev_set_msi_domain(&vmd->bus->dev,
+		dev_set_msi_domain(&vmd->bus[VMD_BUS_0]->dev,
 				   dev_get_msi_domain(&vmd->dev->dev));
 
 	return 0;
@@ -1037,10 +1044,11 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 		return ret;
 	}
 
-	WARN(sysfs_create_link(&vmd->dev->dev.kobj, &vmd->bus->dev.kobj,
-			       "domain"), "Can't create symlink to domain\n");
+	WARN(sysfs_create_link(&vmd->dev->dev.kobj,
+			       &vmd->bus[VMD_BUS_0]->dev.kobj, "domain"),
+	     "Can't create symlink to domain\n");
 
-	vmd_bus_enumeration(vmd->bus, features);
+	vmd_bus_enumeration(vmd->bus[VMD_BUS_0], features);
 
 	return 0;
 }
@@ -1136,9 +1144,9 @@ static void vmd_remove(struct pci_dev *dev)
 {
 	struct vmd_dev *vmd = pci_get_drvdata(dev);
 
-	pci_stop_root_bus(vmd->bus);
+	pci_stop_root_bus(vmd->bus[VMD_BUS_0]);
 	sysfs_remove_link(&vmd->dev->dev.kobj, "domain");
-	pci_remove_root_bus(vmd->bus);
+	pci_remove_root_bus(vmd->bus[VMD_BUS_0]);
 	vmd_cleanup_srcu(vmd);
 	vmd_detach_resources(vmd);
 	vmd_remove_irq_domain(vmd);
-- 
2.43.0

[PATCH v6 7/8] PCI: vmd: Add support for second rootbus under VMD

[Szymon Durawa]

Starting from Intel Arrow Lake VMD enhancement introduces second root bus
support with fixed root bus number (0x80). It means that all 3 MMIO BARs
exposed by VMD are shared now between both buses (current BUS0 and
new BUS1).

Add new BUS1 enumeration and divide MMIO space to be shared between
both root buses. Due to enumeration issues with root bus hardwired to a
fixed non-zero value, this patch will work with a workaround proposed
in next patch. Without workaround user won't see attached devices for BUS1
root bus.

Suggested-by: Nirmal Patel <nirmal.patel@linux.intel.com>
Signed-off-by: Szymon Durawa <szymon.durawa@linux.intel.com>
---
 drivers/pci/controller/vmd.c | 305 +++++++++++++++++++++++++++++++----
 1 file changed, 273 insertions(+), 32 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index 786374965f74..7c4379d565ea 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -4,6 +4,7 @@
  * Copyright (c) 2015, Intel Corporation.
  */
 
+#include <linux/bitfield.h>
 #include <linux/device.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
@@ -30,6 +31,7 @@
 #define VMD_RESTRICT_0_BUS_START 0x0
 #define VMD_RESTRICT_1_BUS_START 0x80
 #define VMD_RESTRICT_2_BUS_START 0xE0
+#define VMD_RESTRICT_3_BUS_START 0xE1
 
 #define PCI_REG_VMCAP		0x40
 #define BUS_RESTRICT_CAP(vmcap)	(vmcap & 0x1)
@@ -42,15 +44,36 @@
 #define MB2_SHADOW_OFFSET	0x2000
 #define MB2_SHADOW_SIZE		16
 
+/* Primary Bus Number for VMD devices on root bus 0 */
+#define VMD_PRIMARY_BUS0    0x00
+/* Primary Bus Number for VMD devices on root bus 1 */
+#define VMD_PRIMARY_BUS1    0x80
+
+#define VMD_BUSRANGE0       0xc8
+#define VMD_BUSRANGE1       0xcc
+#define VMD_MEMBAR1_OFFSET  0xd0
+#define VMD_MEMBAR2_OFFSET1 0xd8
+#define VMD_MEMBAR2_OFFSET2 0xdc
+#define VMD_BUS_END(busr) FIELD_GET(GENMASK(15, 8), busr)
+#define VMD_BUS_START(busr) FIELD_GET(GENMASK(7, 0), busr)
+
+/*
+ * Add VMD resources for BUS1, it will share the same MMIO space with
+ * previous VMD resources.
+ */
 enum vmd_resource {
-	VMD_RES_CFGBAR = 0, /* VMD Bus0 Config BAR */
-	VMD_RES_MBAR_1, /* VMD Bus0 Resource MemBAR 1 */
-	VMD_RES_MBAR_2, /* VMD Bus0 Resource MemBAR 2 */
+	VMD_RES_CFGBAR = 0,  /* VMD Bus0 Config BAR */
+	VMD_RES_MBAR_1,      /* VMD Bus0 Resource MemBAR 1 */
+	VMD_RES_MBAR_2,      /* VMD Bus0 Resource MemBAR 2 */
+	VMD_RES_BUS1_CFGBAR, /* VMD Bus1 Config BAR */
+	VMD_RES_BUS1_MBAR_1, /* VMD Bus1 Resource MemBAR 1 */
+	VMD_RES_BUS1_MBAR_2, /* VMD Bus1 Resource MemBAR 2 */
 	VMD_RES_COUNT
 };
 
 enum vmd_rootbus {
 	VMD_BUS_0 = 0,
+	VMD_BUS_1,
 	VMD_BUS_COUNT
 };
 
@@ -94,6 +117,12 @@ enum vmd_features {
 	 * proper power management of the SoC.
 	 */
 	VMD_FEAT_BIOS_PM_QUIRK		= (1 << 5),
+
+	/*
+	 * Starting from Intel Arrow Lake, VMD devices have their VMD root ports
+	 * on the additional BUS1 root bus.
+	 */
+	VMD_FEAT_HAS_BUS1_ROOTBUS	= (1 << 6)
 };
 
 #define VMD_BIOS_PM_QUIRK_LTR	0x1003	/* 3145728 ns */
@@ -101,7 +130,8 @@ enum vmd_features {
 #define VMD_FEATS_CLIENT	(VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |	\
 				 VMD_FEAT_HAS_BUS_RESTRICTIONS |	\
 				 VMD_FEAT_OFFSET_FIRST_VECTOR |		\
-				 VMD_FEAT_BIOS_PM_QUIRK)
+				 VMD_FEAT_BIOS_PM_QUIRK |		\
+				 VMD_FEAT_HAS_BUS1_ROOTBUS)
 
 static DEFINE_IDA(vmd_instance_ida);
 
@@ -159,6 +189,7 @@ struct vmd_dev {
 	u8			first_vec;
 	char			*name;
 	int			instance;
+	bool			bus1_rootbus;
 };
 
 static inline struct vmd_dev *vmd_from_bus(struct pci_bus *bus)
@@ -537,7 +568,8 @@ static inline void vmd_acpi_end(void) { }
 
 static void vmd_domain_reset(struct vmd_dev *vmd)
 {
-	u16 bus, max_buses = resource_size(&vmd->resources[VMD_RES_CFGBAR]);
+	/* One ECAM bus consumes 1MB of CFGBAR space. */
+	u16 bus, max_buses = resource_size(&vmd->dev->resource[VMD_CFGBAR]) >> 20;
 	u8 dev, functions, fn, hdr_type;
 	char __iomem *base;
 
@@ -587,12 +619,24 @@ static void vmd_attach_resources(struct vmd_dev *vmd)
 {
 	vmd->dev->resource[VMD_MEMBAR1].child = &vmd->resources[VMD_RES_MBAR_1];
 	vmd->dev->resource[VMD_MEMBAR2].child = &vmd->resources[VMD_RES_MBAR_2];
+
+	if (vmd->bus1_rootbus) {
+		vmd->resources[VMD_RES_MBAR_1].sibling =
+			&vmd->resources[VMD_RES_BUS1_MBAR_1];
+		vmd->resources[VMD_RES_MBAR_2].sibling =
+			&vmd->resources[VMD_RES_BUS1_MBAR_2];
+	}
 }
 
 static void vmd_detach_resources(struct vmd_dev *vmd)
 {
 	vmd->dev->resource[VMD_MEMBAR1].child = NULL;
 	vmd->dev->resource[VMD_MEMBAR2].child = NULL;
+
+	if (vmd->bus1_rootbus) {
+		vmd->resources[VMD_RES_MBAR_1].sibling = NULL;
+		vmd->resources[VMD_RES_MBAR_2].sibling = NULL;
+	}
 }
 
 static int vmd_get_phys_offsets(struct vmd_dev *vmd, bool native_hint,
@@ -649,7 +693,7 @@ static int vmd_get_phys_offsets(struct vmd_dev *vmd, bool native_hint,
 	return 0;
 }
 
-static int vmd_get_bus_number_start(struct vmd_dev *vmd)
+static int vmd_get_bus_number_start(struct vmd_dev *vmd, unsigned long features)
 {
 	struct pci_dev *dev = vmd->dev;
 	u16 reg;
@@ -668,6 +712,19 @@ static int vmd_get_bus_number_start(struct vmd_dev *vmd)
 		case 2:
 			vmd->busn_start[VMD_BUS_0] = VMD_RESTRICT_2_BUS_START;
 			break;
+		case 3:
+			if (!(features & VMD_FEAT_HAS_BUS1_ROOTBUS)) {
+				pci_err(dev, "VMD Bus Restriction detected type %d, but BUS1 Rootbus is not supported, aborting.\n",
+					BUS_RESTRICT_CFG(reg));
+				return -ENODEV;
+			}
+
+			/* VMD (on root bus 0) secondary bus start number */
+			vmd->busn_start[VMD_BUS_0] = VMD_RESTRICT_2_BUS_START;
+			/* VMD (on root bus 1) secondary bus start number */
+			vmd->busn_start[VMD_BUS_1] = VMD_RESTRICT_3_BUS_START;
+			vmd->bus1_rootbus = true;
+			break;
 		default:
 			pci_err(dev, "Unknown Bus Offset Setting (%d)\n",
 				BUS_RESTRICT_CFG(reg));
@@ -788,7 +845,7 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
 	return 0;
 }
 
-static void vmd_configure_cfgbar(struct vmd_dev *vmd)
+static int vmd_configure_cfgbar(struct vmd_dev *vmd)
 {
 	struct resource *res = &vmd->dev->resource[VMD_CFGBAR];
 
@@ -799,6 +856,61 @@ static void vmd_configure_cfgbar(struct vmd_dev *vmd)
 		       (resource_size(res) >> 20) - 1,
 		.flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED,
 	};
+
+	if (vmd->bus1_rootbus) {
+		int ret;
+		u16 bus0_range = 0;
+		u16 bus1_range = 0;
+		u8 bus0_start, bus0_end;
+		u8 bus1_start, bus1_end;
+
+		ret = pci_read_config_word(vmd->dev, VMD_BUSRANGE0, &bus0_range);
+		if (ret) {
+			pci_err(vmd->dev, "Failed to read VMD_BUSRANGE0: %d\n", ret);
+			return -EIO;
+		}
+
+		ret = pci_read_config_word(vmd->dev, VMD_BUSRANGE1, &bus1_range);
+		if (ret) {
+			pci_err(vmd->dev, "Failed to read VMD_BUSRANGE1: %d\n", ret);
+			return -EIO;
+		}
+
+		bus0_start = VMD_BUS_START(bus0_range);
+		bus0_end = VMD_BUS_END(bus0_range);
+		bus1_start = VMD_BUS_START(bus1_range);
+		bus1_end = VMD_BUS_END(bus1_range);
+
+		if (bus0_start > bus0_end || bus1_start > bus1_end) {
+			pci_err(vmd->dev,
+				"Invalid bus range(s): BUS0 [%02x-%02x], BUS1 [%02x-%02x]\n",
+				bus0_start, bus0_end, bus1_start, bus1_end);
+			return -EINVAL;
+		}
+
+		if (!(bus0_end < bus1_start || bus1_end < bus0_start)) {
+			pci_err(vmd->dev,
+				"Overlapping bus ranges: BUS0 [%02x-%02x], BUS1 [%02x-%02x]\n",
+				bus0_start, bus0_end, bus1_start, bus1_end);
+			return -EINVAL;
+		}
+
+		/*
+		 * Resize BUS0 CFGBAR range to make space for BUS1
+		 * owned devices by adjusting range end with value stored in
+		 * VMD_BUSRANGE0 register.
+		 */
+		vmd->resources[VMD_RES_CFGBAR].start = bus0_start;
+		vmd->resources[VMD_RES_CFGBAR].end = bus0_end;
+
+		vmd->resources[VMD_RES_BUS1_CFGBAR] = (struct resource){
+			.name = "VMD CFGBAR BUS1",
+			.start = bus1_start,
+			.end = bus1_end,
+			.flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED,
+		};
+	}
+	return 0;
 }
 
 /*
@@ -850,49 +962,135 @@ static int vmd_configure_membar1_membar2(struct vmd_dev *vmd,
 {
 	int ret;
 
-	ret = vmd_configure_membar(vmd, VMD_RES_MBAR_1, VMD_MEMBAR1, 0, 0);
-	if (ret)
-		return ret;
+	if (vmd->bus1_rootbus) {
+		u32 reg = 0;
+		u32 bus1_mbar1_ofs = 0;
+		u64 bus1_mbar2_ofs = 0;
+		resource_size_t mbar1_sz, mbar2_sz;
 
-	ret = vmd_configure_membar(vmd, VMD_RES_MBAR_2, VMD_MEMBAR2, mbar2_ofs, 0);
-	if (ret) {
-		devm_kfree(&vmd->dev->dev, (void *)vmd->resources[VMD_RES_MBAR_1].name);
-		memset(&vmd->resources[VMD_RES_MBAR_1], 0, sizeof(vmd->resources[VMD_RES_MBAR_1]));
-		return ret;
+		mbar1_sz = resource_size(&vmd->dev->resource[VMD_MEMBAR1]);
+		mbar2_sz = resource_size(&vmd->dev->resource[VMD_MEMBAR2]);
+
+		ret = pci_read_config_dword(vmd->dev, VMD_MEMBAR1_OFFSET,
+					    &bus1_mbar1_ofs);
+		if (ret)
+			return -EIO;
+
+		ret = pci_read_config_dword(vmd->dev, VMD_MEMBAR2_OFFSET1, &reg);
+		if (ret)
+			return -EIO;
+		bus1_mbar2_ofs = reg;
+
+		ret = pci_read_config_dword(vmd->dev, VMD_MEMBAR2_OFFSET2, &reg);
+		if (ret)
+			return -EIO;
+		bus1_mbar2_ofs |= (u64)reg << 32;
+
+		if (!bus1_mbar1_ofs || bus1_mbar1_ofs >= mbar1_sz) {
+			pci_err(vmd->dev, "Invalid MEMBAR1 offset %#llx (BAR size %#llx)\n",
+				(unsigned long long)bus1_mbar1_ofs,
+				(unsigned long long)mbar1_sz);
+			return -EINVAL;
+		}
+
+		if (!bus1_mbar2_ofs || mbar2_ofs >= mbar2_sz ||
+		    bus1_mbar2_ofs >= mbar2_sz - mbar2_ofs) {
+			pci_err(vmd->dev,
+				"Invalid MEMBAR2 offset %#llx (mbar2_ofs %#llx, BAR size %#llx)\n",
+				(unsigned long long)bus1_mbar2_ofs,
+				(unsigned long long)mbar2_ofs,
+				(unsigned long long)mbar2_sz);
+			return -EINVAL;
+		}
+
+		/*
+		 * Resize BUS MEMBAR1 and MEMBAR2 ranges to make space
+		 * for BUS1 owned devices by adjusting range end with values
+		 * stored in VMD_MEMBAR1_OFFSET and VMD_MEMBAR2_OFFSET registers
+		 */
+		ret = vmd_configure_membar(vmd, VMD_RES_MBAR_1, VMD_MEMBAR1, 0,
+					   mbar1_sz - bus1_mbar1_ofs);
+		if (ret)
+			return ret;
+
+		ret = vmd_configure_membar(vmd, VMD_RES_MBAR_2, VMD_MEMBAR2,
+					   mbar2_ofs, mbar2_sz - (bus1_mbar2_ofs + mbar2_ofs));
+		if (ret)
+			return ret;
+
+		ret = vmd_configure_membar(vmd, VMD_RES_BUS1_MBAR_1, VMD_MEMBAR1,
+					   bus1_mbar1_ofs, 0);
+		if (ret)
+			return ret;
+
+		ret = vmd_configure_membar(vmd, VMD_RES_BUS1_MBAR_2, VMD_MEMBAR2,
+					   mbar2_ofs + bus1_mbar2_ofs, 0);
+		if (ret)
+			return ret;
+	} else {
+		ret = vmd_configure_membar(vmd, VMD_RES_MBAR_1, VMD_MEMBAR1, 0, 0);
+		if (ret)
+			return ret;
+
+		ret = vmd_configure_membar(vmd, VMD_RES_MBAR_2, VMD_MEMBAR2,
+					   mbar2_ofs, 0);
+		if (ret)
+			return ret;
 	}
 
 	return 0;
 }
 
-static int vmd_create_bus(struct vmd_dev *vmd, struct pci_sysdata *sd,
-			  resource_size_t *offset)
+static int vmd_create_bus(struct vmd_dev *vmd, enum vmd_rootbus bus_number,
+			  struct pci_sysdata *sd, resource_size_t *offset,
+			  u8 primary)
 {
+	u8 cfgbar = bus_number * 3;
+	u8 membar1 = cfgbar + 1;
+	u8 membar2 = cfgbar + 2;
+	struct pci_bus *vmd_bus;
 	LIST_HEAD(resources);
 
-	pci_add_resource(&resources, &vmd->resources[VMD_RES_CFGBAR]);
-	pci_add_resource_offset(&resources, &vmd->resources[VMD_RES_MBAR_1],
+	pci_add_resource(&resources, &vmd->resources[cfgbar]);
+	pci_add_resource_offset(&resources, &vmd->resources[membar1],
 				offset[0]);
-	pci_add_resource_offset(&resources, &vmd->resources[VMD_RES_MBAR_2],
+	pci_add_resource_offset(&resources, &vmd->resources[membar2],
 				offset[1]);
 
-	vmd->bus[VMD_BUS_0] = pci_create_root_bus(&vmd->dev->dev,
-						  vmd->busn_start[VMD_BUS_0],
-						  &vmd_ops, sd, &resources);
-	if (!vmd->bus[VMD_BUS_0]) {
+	vmd_bus = pci_create_root_bus(&vmd->dev->dev,
+				      vmd->busn_start[bus_number], &vmd_ops, sd,
+				      &resources);
+
+	if (!vmd_bus) {
 		pci_free_resource_list(&resources);
+
+		/* Clean only when BUS0 root bus cannot be created */
+		if (bus_number == VMD_BUS_0) {
+			pci_bus_release_emul_domain_nr(sd->domain);
+			vmd_remove_irq_domain(vmd);
+		}
 		return -ENODEV;
 	}
 
+	/*
+	 * pci_create_root_bus() does not initialise bus->primary.
+	 * Set it here before any scanning so bridge traversal logic
+	 * sees the correct upstream bus number from the start.
+	 */
+	vmd_bus->primary = primary;
+
 	vmd_copy_host_bridge_flags(pci_find_host_bridge(vmd->dev->bus),
-				   to_pci_host_bridge(vmd->bus[VMD_BUS_0]->bridge));
+				   to_pci_host_bridge(vmd_bus->bridge));
 
 	vmd_attach_resources(vmd);
 	if (vmd->irq_domain)
-		dev_set_msi_domain(&vmd->bus[VMD_BUS_0]->dev, vmd->irq_domain);
+		dev_set_msi_domain(&vmd_bus->dev, vmd->irq_domain);
 	else
-		dev_set_msi_domain(&vmd->bus[VMD_BUS_0]->dev,
+		dev_set_msi_domain(&vmd_bus->dev,
 				   dev_get_msi_domain(&vmd->dev->dev));
 
+	vmd->bus[bus_number] = vmd_bus;
+
 	return 0;
 }
 
@@ -905,7 +1103,15 @@ static void vmd_bus_enumeration(struct pci_bus *bus, unsigned long features)
 	vmd_acpi_begin();
 
 	pci_scan_child_bus(bus);
-	vmd_domain_reset(vmd_from_bus(bus));
+
+	/*
+	 * vmd_domain_reset() walks the full VMD CFGBAR aperture, so a single
+	 * invocation from BUS0 resets bridge windows for the whole VMD domain,
+	 * including BUS1. Running it again during BUS1 enumeration would
+	 * re-clobber windows already assigned for BUS0.
+	 */
+	if (bus->primary == VMD_PRIMARY_BUS0)
+		vmd_domain_reset(vmd_from_bus(bus));
 
 	/*
 	 * When Intel VMD is enabled, the OS does not discover the Root Ports
@@ -973,12 +1179,14 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 	 * 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);
+		ret = vmd_get_bus_number_start(vmd, features);
 		if (ret)
 			return ret;
 	}
 
-	vmd_configure_cfgbar(vmd);
+	ret = vmd_configure_cfgbar(vmd);
+	if (ret)
+		return ret;
 
 	/*
 	 * If the window is below 4GB, clear IORESOURCE_MEM_64 so we can
@@ -1036,7 +1244,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 
 	sd->node = pcibus_to_node(vmd->dev->bus);
 
-	ret = vmd_create_bus(vmd, sd, offset);
+	ret = vmd_create_bus(vmd, VMD_BUS_0, sd, offset, VMD_PRIMARY_BUS0);
 	if (ret) {
 		pci_err(vmd->dev, "Can't create bus: %d\n", ret);
 		pci_bus_release_emul_domain_nr(sd->domain);
@@ -1048,8 +1256,34 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 			       &vmd->bus[VMD_BUS_0]->dev.kobj, "domain"),
 	     "Can't create symlink to domain\n");
 
+	if (vmd->bus1_rootbus) {
+		ret = vmd_create_bus(vmd, VMD_BUS_1, sd, offset, VMD_PRIMARY_BUS1);
+		if (ret) {
+			pci_warn(vmd->dev,
+				 "Can't create BUS1: %d, continuing with BUS0 only\n",
+				 ret);
+
+			vmd->resources[VMD_RES_MBAR_1].sibling = NULL;
+			vmd->resources[VMD_RES_MBAR_2].sibling = NULL;
+			vmd->resources[VMD_RES_BUS1_CFGBAR] = (struct resource){};
+			vmd->resources[VMD_RES_BUS1_MBAR_1] = (struct resource){};
+			vmd->resources[VMD_RES_BUS1_MBAR_2] = (struct resource){};
+			vmd->bus1_rootbus = false;
+			vmd->bus[VMD_BUS_1] = NULL;
+		}
+
+		if (vmd->bus1_rootbus)
+			WARN(sysfs_create_link(&vmd->dev->dev.kobj,
+					       &vmd->bus[VMD_BUS_1]->dev.kobj,
+					       "domain1"),
+			     "Can't create symlink to domain1\n");
+	}
+
 	vmd_bus_enumeration(vmd->bus[VMD_BUS_0], features);
 
+	if (vmd->bus1_rootbus)
+		vmd_bus_enumeration(vmd->bus[VMD_BUS_1], features);
+
 	return 0;
 }
 
@@ -1147,6 +1381,13 @@ static void vmd_remove(struct pci_dev *dev)
 	pci_stop_root_bus(vmd->bus[VMD_BUS_0]);
 	sysfs_remove_link(&vmd->dev->dev.kobj, "domain");
 	pci_remove_root_bus(vmd->bus[VMD_BUS_0]);
+
+	if (vmd->bus1_rootbus && vmd->bus[VMD_BUS_1]) {
+		pci_stop_root_bus(vmd->bus[VMD_BUS_1]);
+		sysfs_remove_link(&vmd->dev->dev.kobj, "domain1");
+		pci_remove_root_bus(vmd->bus[VMD_BUS_1]);
+	}
+
 	vmd_cleanup_srcu(vmd);
 	vmd_detach_resources(vmd);
 	vmd_remove_irq_domain(vmd);
@@ -1239,4 +1480,4 @@ module_pci_driver(vmd_drv);
 MODULE_AUTHOR("Intel Corporation");
 MODULE_DESCRIPTION("Volume Management Device driver");
 MODULE_LICENSE("GPL v2");
-MODULE_VERSION("0.6");
+MODULE_VERSION("0.7");
-- 
2.43.0

[PATCH v6 8/8] PCI: vmd: Add workaround for bus number hardwired to fixed non-zero value

[Szymon Durawa]

The VMD BUS1 root bus number is fixed in hardware to 0x80. It marks bridge
invalid configuration when detecting a non-zero (0x80) the VMD BUS1 root
bus number.
Thus root bus number is deconfigured in the first pass of pci_scan_bridge()
to be re-assigned to 0x0 in the second pass. As a result no subordinate bus
number behind VMD BUS1 is found.

To avoid bus number reconfiguration, BUS1 number has to be the same
as BUS1 primary number.

Log snippet without workaround:

[    3.752507] vmd 0000:00:0e.0: PCI host bridge to bus 10000:e1
[    3.752510] pci_bus 10000:e1: busn_res: can not insert [bus e1-ff] under domain [bus 00-ff] (conflicts with (null) [bus e0-f0])
[    3.752515] pci_bus 10000:e1: root bus resource [bus f1-ff]
[    3.752517] pci_bus 10000:e1: root bus resource [mem 0x8c800000-0x8cffffff]
[    3.752519] pci_bus 10000:e1: root bus resource [mem 0x701b802000-0x701bffffff 64bit]
[    3.752523] pci_bus 10000:e1: scanning bus
[    3.752732] pci (null): Looking for ACPI companion (address 0x80e0ffff)
[    3.752745] pci 10000:e1:1c.0: [8086:7f38] type 01 class 0x060400 PCIe Root Port
[    3.752779] pci 10000:e1:1c.0: PCI bridge to [bus f1]
[    3.752861] pci 10000:e1:1c.0: PME# supported from D0 D3hot D3cold
[    3.752864] pci 10000:e1:1c.0: PME# disabled
[    3.752909] pci 10000:e1:1c.0: PTM enabled (root), 4ns granularity
[    3.752981] pci 10000:e1:1c.0: vgaarb: pci_notify
[    3.752987] pci_bus 10000:e1: fixups for bus
[    3.752992] pci 10000:e1:1c.0: scanning [bus f1-f1] behind bridge, pass 0
[    3.752993] pci 10000:e1:1c.0: primary 80, bus->number e1.
[    3.752994] pci 10000:e1:1c.0: bridge configuration invalid ([bus f1-f1]), reconfiguring
[    3.753003] pci 10000:e1:1c.0: scanning [bus 00-00] behind bridge, pass 1
[    3.753004] pci 10000:e1:1c.0: primary 00, bus->number e1.

Suggested-by: Nirmal Patel <nirmal.patel@linux.intel.com>
Signed-off-by: Szymon Durawa <szymon.durawa@linux.intel.com>
---
 drivers/pci/controller/vmd.c | 67 ++++++++++++++++++++++++++++++++----
 1 file changed, 60 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index 7c4379d565ea..8da2e6727282 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -440,8 +440,22 @@ static void vmd_remove_irq_domain(struct vmd_dev *vmd)
 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[VMD_BUS_0];
-	u32 offset = PCIE_ECAM_OFFSET(busnr_ecam, devfn, reg);
+	unsigned char bus_number;
+	unsigned int busnr_ecam;
+	u32 offset;
+
+	/*
+	 * BUS1 is registered with logical bus number 0x80 to avoid bridge
+	 * reconfiguration, but cfg accesses must target the original BUS1
+	 * restricted range start.
+	 */
+	if (vmd->bus1_rootbus && bus->number == VMD_PRIMARY_BUS1)
+		bus_number = vmd->busn_start[VMD_BUS_1];
+	else
+		bus_number = bus->number;
+
+	busnr_ecam = bus_number - vmd->busn_start[VMD_BUS_0];
+	offset = PCIE_ECAM_OFFSET(busnr_ecam, devfn, reg);
 
 	if (offset + len >= resource_size(&vmd->dev->resource[VMD_CFGBAR]))
 		return NULL;
@@ -520,18 +534,37 @@ static struct pci_ops vmd_ops = {
 static struct acpi_device *vmd_acpi_find_companion(struct pci_dev *pci_dev)
 {
 	struct pci_host_bridge *bridge;
-	u32 busnr, addr;
+	struct vmd_dev *vmd;
+	u32 addr;
+	int busnr;
+	u8 pci_bus_number;
+	u8 bridge_bus_number;
 
 	if (pci_dev->bus->ops != &vmd_ops)
 		return NULL;
 
+	vmd = vmd_from_bus(pci_dev->bus);
 	bridge = pci_find_host_bridge(pci_dev->bus);
-	busnr = pci_dev->bus->number - bridge->bus->number;
+	pci_bus_number = pci_dev->bus->number;
+	bridge_bus_number = bridge->bus->number;
+
+	/*
+	 * BUS1 is registered with logical root number 0x80. For ACPI companion
+	 * matching, compute the relative bus number against the original BUS1
+	 * restricted range base.
+	 */
+	if (vmd->bus1_rootbus && bridge->bus == vmd->bus[VMD_BUS_1]) {
+		bridge_bus_number = vmd->busn_start[VMD_BUS_1];
+		if (pci_bus_number == VMD_PRIMARY_BUS1)
+			pci_bus_number = vmd->busn_start[VMD_BUS_1];
+	}
+
+	busnr = pci_bus_number - bridge_bus_number;
 	/*
 	 * The address computation below is only applicable to relative bus
 	 * numbers below 32.
 	 */
-	if (busnr > 31)
+	if (busnr < 0 || busnr > 31)
 		return NULL;
 
 	addr = (busnr << 24) | ((u32)pci_dev->devfn << 16) | 0x8000FFFFU;
@@ -1045,6 +1078,7 @@ static int vmd_create_bus(struct vmd_dev *vmd, enum vmd_rootbus bus_number,
 			  struct pci_sysdata *sd, resource_size_t *offset,
 			  u8 primary)
 {
+	u8 root_busnr;
 	u8 cfgbar = bus_number * 3;
 	u8 membar1 = cfgbar + 1;
 	u8 membar2 = cfgbar + 2;
@@ -1057,8 +1091,27 @@ static int vmd_create_bus(struct vmd_dev *vmd, enum vmd_rootbus bus_number,
 	pci_add_resource_offset(&resources, &vmd->resources[membar2],
 				offset[1]);
 
-	vmd_bus = pci_create_root_bus(&vmd->dev->dev,
-				      vmd->busn_start[bus_number], &vmd_ops, sd,
+	/*
+	 * Register BUS1 with its logical root number (0x80) up front so PCI core
+	 * bridge scanning does not see a post-registration bus-number mutation.
+	 *
+	 * This is a workaround for pci_scan_bridge_extend() code.
+	 * It marks bridge invalid configuration when detecting a
+	 * non-zero (0x80) the VMD BUS1 root bus number. Thus Primary Bus Number
+	 * of Root Ports on BUS1 is deconfigured in the first pass of
+	 * pci_scan_bridge() to be re-assigned to 0x0 in the second pass.
+	 * As a result no subordinate bus number behind VMD BUS1 is found.
+	 * Workaround: VMD_BUS_1 bus number shall be set to VMD_PRIMARY_BUS1 so it has
+	 * the same value as vmd->bus[VMD_BUS_1]->primary, it will bypass bus number
+	 * reconfiguration.
+	 */
+
+	if (bus_number == VMD_BUS_1 && vmd->bus1_rootbus)
+		root_busnr = VMD_PRIMARY_BUS1;
+	else
+		root_busnr = vmd->busn_start[bus_number];
+
+	vmd_bus = pci_create_root_bus(&vmd->dev->dev, root_busnr, &vmd_ops, sd,
 				      &resources);
 
 	if (!vmd_bus) {
-- 
2.43.0