Skip to content

Conversation

@sdurawa
Copy link
Owner

@sdurawa sdurawa commented Nov 7, 2024

New version with applied Bjorn remarks

Copy link

@mtkaczyk mtkaczyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patch1 :
PCI: vmd: Create bus enumeration helper

Please change to:
PCI: vmd: Add vmd_bus_enumeration()


if (vmd->bus1_rootbus)
max_buses +=
resource_size(&vmd->resources[VMD_RES_BUS1_CFGBAR]) + 2;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formatting

return -ENODEV;
}

/* BUS0 (IOC) start number */
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bjorn doesn't like our IOC, PCH nomenclature, please challenge that :)


/* BUS0 (IOC) start number */
vmd->busn_start[VMD_BUS_0] = VMD_RESTRICT_2_BUS_START;
/* BUS1 (PCH) start number */
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as above (PCH)

vmd->bus[VMD_BUS_0] = pci_create_root_bus(&vmd->dev->dev,
vmd->busn_start[VMD_BUS_0],
vmd->bus[bus_number] = pci_create_root_bus(&vmd->dev->dev,
vmd->busn_start[bus_number],
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formatting

"Can't create symlink to domain\n");
WARN(sysfs_create_link(
&vmd->dev->dev.kobj, &vmd->bus[bus_number]->dev.kobj,
kasprintf(GFP_KERNEL, "domain bus%d", bus_number)),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formatting here and line above

"Can't create symlink to domain\n");
WARN(sysfs_create_link(
&vmd->dev->dev.kobj, &vmd->bus[bus_number]->dev.kobj,
kasprintf(GFP_KERNEL, "domain bus%d", bus_number)),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't just change link. "domain" link is there and it must stay as is. For example, you are breaking this code:
https://github.com/md-raid-utilities/mdadm/blob/main/platform-intel.c#L126

Copy link

@mtkaczyk mtkaczyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bjorn asked what is happening if this workaround is not applied, please elaborate :)

u32 offset;

/*
* VMD WA: for PCH rootbus, bus number is set to VMD_PRIMARY_BUS1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please explain WA

@sdurawa sdurawa force-pushed the vmd_pch_rootbus_v3 branch 5 times, most recently from 3e93b76 to 55e2095 Compare November 15, 2024 07:57
Szymon Durawa added 8 commits November 15, 2024 09:39
Move the vmd bus enumeration code to a new helper. No
functional changes.

Suggested-by: Nirmal Patel <[email protected]>
Reviewed-by: Mariusz Tkaczyk <[email protected]>
Signed-off-by: Szymon Durawa <[email protected]>
Move the VMD CFGBAR initialization code to a new helper. No
functional changes.

Suggested-by: Nirmal Patel <[email protected]>
Reviewed-by: Mariusz Tkaczyk <[email protected]>
Signed-off-by: Szymon Durawa <[email protected]>
Move the MEMBAR1 and MEMBAR2 registry initialization code to a new helper
functions. No functional changes.

Suggested-by: Nirmal Patel <[email protected]>
Reviewed-by: Mariusz Tkaczyk <[email protected]>
Signed-off-by: Szymon Durawa <[email protected]>
Move the VMD bus initialization code to a new helper. No
functional changes.

Suggested-by: Nirmal Patel <[email protected]>
Reviewed-by: Mariusz Tkaczyk <[email protected]>
Signed-off-by: Szymon Durawa <[email protected]>
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 <[email protected]>
Reviewed-by: Mariusz Tkaczyk <[email protected]>
Signed-off-by: Szymon Durawa <[email protected]>
Bus and busn_start are converted from scalar to an array to support
multiple VMD buses in the future. No functional changes.

Suggested-by: Nirmal Patel <[email protected]>
Reviewed-by: Mariusz Tkaczyk <[email protected]>
Signed-off-by: Szymon Durawa <[email protected]>
Starting from Intel Arrow Lake VMD enhacement introduces second rootbus 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).
This patch add new BUS1 enumeration and divide MMIO space to be shared between
both rootbuses. Due to enumeration issues with rootbus hardwired to a
fixed non-zero value, this patch will work with a workaround proposed in next
patch. Without workaraund user won't see attached devices behind BUS1 rootbus.

Suggested-by: Nirmal Patel <[email protected]>
Reviewed-by: Mariusz Tkaczyk <[email protected]>
Signed-off-by: Szymon Durawa <[email protected]>
…ro value

VMD BUS1 rootbus primary number is 0x80 and pci_scan_bridge_extend() cannot
assign it as "hard-wired to 0" and marks setup as broken. To avoid falling
into bridge configuration invalid condition, BUS1 number has to be the same
as BUS1 primary number.

Suggested-by: Nirmal Patel <[email protected]>
Reviewed-by: Mariusz Tkaczyk <[email protected]>
Signed-off-by: Szymon Durawa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants