diff mbox series

PCI/ACPI: Add extra slot register check for non-ACPI device

Message ID 1701065447-13963-1-git-send-email-wangdong202303@163.com
State New
Headers show
Series PCI/ACPI: Add extra slot register check for non-ACPI device | expand

Commit Message

wangdong28 Nov. 27, 2023, 6:10 a.m. UTC 
From: Dong Wang <wangdong28@lenovo.com>

When enabling VMD function in UEFI setup, the physical slot of the M.2
NVMe device connected to the VMD device cannot be detected. Here is
the result from lspci ("Physical Slot" field is NOT shown): 

 10001:01:00.0 Non-Volatile memory controller: Intel Corporation NVMe
 Datacenter SSD [3DNAND, Beta Rock Controller] (prog-if 02 [NVM Express])
   Subsystem: Intel Corporation NVMe Datacenter SSD [3DNAND] SE M.2 (P4511)

Generally, the physical slot (/sys/bus/pci/slots) will be created via
either ACPI walking path during kernel init or hotplug path:

ACPI walking path:
  pcibios_add_bus
    acpi_pci_add_bus
      acpi_pci_slot_enumerate  
        acpi_walk_namespace
          register_slot
            pci_create_slot

hotplug path:
  __pci_hp_initialize
    pci_create_slot

[M.2 NVMe Device]
A. VMD disabled
When VMD is disabled, NVMe will be discovered during bus scanning and
recognized as acpi device. In this case, the physical slot is created
via the ACPI walking path. 

B. VMD enabled
vmd_enable_domain() invokes pcibios_add_bus(). This means that it goes
through the ACPI walking path. However, acpi_pci_add_bus() returns
directly becase the statment "!ACPI_HANDLE(bus->bridge)" is true.
See the following code snippet:

  void acpi_pci_add_bus(struct pci_bus *bus)
  {
      ...
      if (acpi_pci_disabled || !bus->bridge || !ACPI_HANDLE(bus->bridge))
		return;
      ...
  }

Since VMD creates its own root bus and devices of VMD are attached to
the bus, those devices are non-ACPI devices. That's why
"!ACPI_HANDLE(bus->bridge)" returns true.

In addition, M.2 NVMe devices does not have the hotplug capability. 
Here is the quote from PCI Express M.2 Specification (Revision 5.0, 
Version 1.0):

  CAUTION: M.2 Add-in Cards are not designed or intended to support
  Hot-Swap or Hot-Plug connections. Performing Hot-Swap or Hot-Plug
  may pose danger to the M.2 Add-in Card, to the system Platform,
  and to the person performing this act.

M.2 NVMe devices (non-ACPI devices and no hotplug capability) connected
to the VMD device cannot meet the above-mentioned paths. The corresponding
slot info of the M.2 NVMe controller cannot be created in
/sys/bus/pci/slots.

Fix this issue by checking the available physical slot number in
slot capabilities register. If the physical slot number is available,
create the slot info accordingly. The following lspci output shows the
available slot info with applying this patch:

 10001:01:00.0 Non-Volatile memory controller: Intel Corporation NVMe
 Datacenter SSD [3DNAND, Beta Rock Controller] (prog-if 02 [NVM Express])
   Subsystem: Intel Corporation NVMe Datacenter SSD [3DNAND] SE M.2 (P4511)
   Physical Slot: 16

[U.2 NVMe device]
A. VMD disabled
Same as M.2 NVMe Device case "A".

B. VMD enabled
Same as M.2 NVMe Device case "B".

The hotplug of the U.2 device is optional (See "PCI Express SFF-8639 Module
Specification" for detail). The U.2 NVMe controller with hotplug capability
connected to the VMD device can meet the hotplug path, so the slot info can
be shown correctly via the lspci utility (without this patch):

 10000:82:00.0 Non-Volatile memory controller: Intel Corporation NVMe
 Datacenter SSD [3DNAND, Beta Rock Controller] (prog-if 02 [NVM Express])
   Subsystem: Lenovo Thinksystem U.2 P4610 NVMe SSD
   Physical Slot: 64

For U.2 NVMe controller without hotplug capability, this patch is needed
to fix the missing slot info.

Suggested-and-reviewed-by: Adrian Huang <ahuang12@lenovo.com> 
Signed-off-by: Dong Wang <wangdong28@lenovo.com>
---
 drivers/pci/pci-acpi.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

Comments

kernel test robot Nov. 27, 2023, 3:22 p.m. UTC | #1 
Hi wangdong28,

kernel test robot noticed the following build errors:

[auto build test ERROR on rafael-pm/linux-next]
[also build test ERROR on rafael-pm/acpi-bus rafael-pm/devprop linus/master v6.7-rc3 next-20231127]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/wangdong28/PCI-ACPI-Add-extra-slot-register-check-for-non-ACPI-device/20231127-141554
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/1701065447-13963-1-git-send-email-wangdong202303%40163.com
patch subject: [PATCH] PCI/ACPI: Add extra slot register check for non-ACPI device
config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20231127/202311272056.KtldqGiA-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231127/202311272056.KtldqGiA-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311272056.KtldqGiA-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/pci/pci-acpi.c: In function 'pci_check_extra_slot_register':
>> drivers/pci/pci-acpi.c:896:14: error: implicit declaration of function 'is_vmd' [-Werror=implicit-function-declaration]
     896 |         if (!is_vmd(bus) || !pdev || pcie_capability_read_dword(pdev, PCI_EXP_SLTCAP, &slot_cap))
         |              ^~~~~~
   cc1: some warnings being treated as errors


vim +/is_vmd +896 drivers/pci/pci-acpi.c

   888	
   889	static void pci_check_extra_slot_register(struct pci_bus *bus)
   890	{
   891		struct pci_dev *pdev = bus->self;
   892		char slot_name[SLOT_NAME_SIZE];
   893		struct pci_slot *pci_slot;
   894		u32 slot_cap, slot_nr;
   895	
 > 896		if (!is_vmd(bus) || !pdev || pcie_capability_read_dword(pdev, PCI_EXP_SLTCAP, &slot_cap))
   897			return;
   898	
   899		if (!(slot_cap & PCI_EXP_SLTCAP_HPC)) {
   900			slot_nr = (slot_cap & PCI_EXP_SLTCAP_PSN) >> 19;
   901			snprintf(slot_name, SLOT_NAME_SIZE, "%u", slot_nr);
   902			pci_slot = pci_create_slot(bus, 0, slot_name, NULL);
   903			if (IS_ERR(pci_slot))
   904				pr_err("pci_create_slot returned %ld\n", PTR_ERR(pci_slot));
   905		}
   906	}
   907
diff mbox series

Patch

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 0045750..d7b3462 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -884,6 +884,27 @@  acpi_status pci_acpi_add_pm_notifier(struct acpi_device *dev,
 	return acpi_add_pm_notifier(dev, &pci_dev->dev, pci_acpi_wake_dev);
 }
 
+#define SLOT_NAME_SIZE  5
+
+static void pci_check_extra_slot_register(struct pci_bus *bus)
+{
+	struct pci_dev *pdev = bus->self;
+	char slot_name[SLOT_NAME_SIZE];
+	struct pci_slot *pci_slot;
+	u32 slot_cap, slot_nr;
+
+	if (!is_vmd(bus) || !pdev || pcie_capability_read_dword(pdev, PCI_EXP_SLTCAP, &slot_cap))
+		return;
+
+	if (!(slot_cap & PCI_EXP_SLTCAP_HPC)) {
+		slot_nr = (slot_cap & PCI_EXP_SLTCAP_PSN) >> 19;
+		snprintf(slot_name, SLOT_NAME_SIZE, "%u", slot_nr);
+		pci_slot = pci_create_slot(bus, 0, slot_name, NULL);
+		if (IS_ERR(pci_slot))
+			pr_err("pci_create_slot returned %ld\n", PTR_ERR(pci_slot));
+	}
+}
+
 /*
  * _SxD returns the D-state with the highest power
  * (lowest D-state number) supported in the S-state "x".
@@ -1202,9 +1223,16 @@  void acpi_pci_add_bus(struct pci_bus *bus)
 	union acpi_object *obj;
 	struct pci_host_bridge *bridge;
 
-	if (acpi_pci_disabled || !bus->bridge || !ACPI_HANDLE(bus->bridge))
+	if (acpi_pci_disabled || !bus->bridge)
 		return;
 
+
+	if (!ACPI_HANDLE(bus->bridge)) {
+		pci_check_extra_slot_register(bus);
+		return;
+	}
+
+
 	acpi_pci_slot_enumerate(bus);
 	acpiphp_enumerate_slots(bus);