ACPI: Allow CONFIG_PCI to be unset for reboot

PATCH Superseded Sinan Kaya 3 participants 9 messages Dec 14, 2018 1 message not in the archive

Subsystems (5)

x86 ACPI PCI arm64 +1 more subsystems

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Sinan Kaya

author

2018-12-14 16:33 UTC #1 lore raw

PATCH v9 1/6 ACPI: Allow CONFIG_PCI to be unset for reboot

Make PCI reboot conditional on PCI support being present on the kernel configuration.

Signed-off-by Sinan Kaya <[email protected]>

---
 drivers/acpi/reboot.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

drivers/acpi/reboot.c +7 -3

index 6fa9c2a4cfe9..d75e637ee36a 100644
@@ -7,8 +7,6 @@
 void acpi_reboot(void)
 {
 	struct acpi_generic_address *rr;
-	struct pci_bus *bus0;
-	unsigned int devfn;
 	u8 reset_value;
 
 	if (acpi_disabled)

@@ -33,6 +31,11 @@ void acpi_reboot(void)
 	 * on a device on bus 0. */
 	switch (rr->space_id) {
 	case ACPI_ADR_SPACE_PCI_CONFIG:
+	{
+#ifdef CONFIG_PCI
+		unsigned int devfn;
+		struct pci_bus *bus0;
+
 		/* The reset register can only live on bus 0. */
 		bus0 = pci_find_bus(0, 0);
 		if (!bus0)

@@ -44,8 +47,9 @@ void acpi_reboot(void)
 		/* Write the value that resets us. */
 		pci_bus_write_config_byte(bus0, devfn,
 				(rr->address & 0xffff), reset_value);
+#endif
 		break;
-
+	}
 	case ACPI_ADR_SPACE_SYSTEM_MEMORY:
 	case ACPI_ADR_SPACE_SYSTEM_IO:
 		printk(KERN_DEBUG "ACPI MEMORY or I/O RESET_REG.\n");

Signature · 1 line

2.19.0

Sinan Kaya

author

2018-12-14 16:33 UTC #2 lore raw

PATCH v9 2/6 ACPI / OSL: Stub out acpi_os_(read/write)_pci_configurations()

In reply to a message not in the archive

Getting ready to allow CONFIG_PCI to be unset with ACPI enabled. Stub out acpi_os_read_pci_configuration and acpi_os_write_pci_configuration functions when CONFIG_PCI is not defined.

Signed-off-by Sinan Kaya <[email protected]>

---
 drivers/acpi/osl.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

drivers/acpi/osl.c +14 -0

index b48874b8e1ea..524fd5f33ea4 100644
@@ -773,6 +773,7 @@ acpi_status
 acpi_os_read_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
 			       u64 *value, u32 width)
 {
+#ifdef CONFIG_PCI
 	int result, size;
 	u32 value32;

@@ -799,12 +800,19 @@ acpi_os_read_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
 	*value = value32;
 
 	return (result ? AE_ERROR : AE_OK);
+#else
+	int rc;
+
+	rc = pr_warn_once("PCI configuration space access is not supported\n");
+	return rc ? AE_SUPPORT : AE_OK;
+#endif
 }
 
 acpi_status
 acpi_os_write_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
 				u64 value, u32 width)
 {
+#ifdef CONFIG_PCI
 	int result, size;
 
 	switch (width) {

@@ -826,6 +834,12 @@ acpi_os_write_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
 				reg, size, value);
 
 	return (result ? AE_ERROR : AE_OK);
+#else
+	int rc;
+
+	rc = pr_warn_once("PCI configuration space access is not supported\n");
+	return rc ? AE_SUPPORT : AE_OK;
+#endif
 }
 
 static void acpi_os_execute_deferred(struct work_struct *work)

Signature · 1 line

2.19.0

Sinan Kaya

author

2018-12-14 16:33 UTC #3 lore raw

PATCH v9 3/6 PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set

In reply to a message not in the archive

We are compiling PCI code today for systems with ACPI and no PCI device present. Remove the useless code and reduce the tight dependency.

Signed-off-by Sinan Kaya <[email protected]>

---
 arch/x86/include/asm/pci_x86.h | 7 +++++++
 drivers/acpi/Kconfig           | 1 -
 drivers/acpi/Makefile          | 2 +-
 drivers/acpi/internal.h        | 5 +++++
 drivers/pci/Makefile           | 2 +-
 include/acpi/acpi_drivers.h    | 7 +++++++
 include/linux/acpi.h           | 7 +++++++
 include/linux/pci.h            | 4 ++++
 8 files changed, 32 insertions(+), 3 deletions(-)

arch/x86/include/asm/pci_x86.h +7 -0

index 959d618dbb17..73bb404f4d2a 100644
@@ -121,7 +121,14 @@ extern void __init dmi_check_pciprobe(void);
 extern void __init dmi_check_skip_isa_align(void);
 
 /* some common used subsys_initcalls */
+#ifdef CONFIG_PCI
 extern int __init pci_acpi_init(void);
+#else
+static inline int  __init pci_acpi_init(void)
+{
+	return -EINVAL;
+}
+#endif
 extern void __init pcibios_irq_init(void);
 extern int __init pcibios_init(void);
 extern int pci_legacy_init(void);

drivers/acpi/Kconfig +0 -1

index 7cea769c37df..a0abcb3bd673 100644
@@ -9,7 +9,6 @@ config ARCH_SUPPORTS_ACPI
 menuconfig ACPI
 	bool "ACPI (Advanced Configuration and Power Interface) Support"
 	depends on ARCH_SUPPORTS_ACPI
-	depends on PCI
 	select PNP
 	default y if X86
 	help

drivers/acpi/Makefile +1 -1

index edc039313cd6..7c6afc111d76 100644
@@ -39,7 +39,7 @@ acpi-y				+= processor_core.o
 acpi-$(CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC) += processor_pdc.o
 acpi-y				+= ec.o
 acpi-$(CONFIG_ACPI_DOCK)	+= dock.o
-acpi-y				+= pci_root.o pci_link.o pci_irq.o
+acpi-$(CONFIG_PCI)		+= pci_root.o pci_link.o pci_irq.o
 obj-$(CONFIG_ACPI_MCFG)		+= pci_mcfg.o
 acpi-y				+= acpi_lpss.o acpi_apd.o
 acpi-y				+= acpi_platform.o

drivers/acpi/internal.h +5 -0

index 530a3f675490..b7060dae2789 100644
@@ -25,8 +25,13 @@ int acpi_osi_init(void);
 acpi_status acpi_os_initialize1(void);
 void init_acpi_device_notify(void);
 int acpi_scan_init(void);
+#ifdef CONFIG_PCI
 void acpi_pci_root_init(void);
 void acpi_pci_link_init(void);
+#else
+static inline void acpi_pci_root_init(void) {}
+static inline void acpi_pci_link_init(void) {}
+#endif
 void acpi_processor_init(void);
 void acpi_platform_init(void);
 void acpi_pnp_init(void);

drivers/pci/Makefile +1 -1

index f2bda77a2df1..657d642fcc67 100644
@@ -11,6 +11,7 @@ ifdef CONFIG_PCI
 obj-$(CONFIG_PROC_FS)		+= proc.o
 obj-$(CONFIG_SYSFS)		+= slot.o
 obj-$(CONFIG_OF)		+= of.o
+obj-$(CONFIG_ACPI)		+= pci-acpi.o
 endif
 
 obj-$(CONFIG_PCI_QUIRKS)	+= quirks.o

@@ -20,7 +21,6 @@ obj-$(CONFIG_PCI_MSI)		+= msi.o
 obj-$(CONFIG_PCI_ATS)		+= ats.o
 obj-$(CONFIG_PCI_IOV)		+= iov.o
 obj-$(CONFIG_PCI_BRIDGE_EMUL)	+= pci-bridge-emul.o
-obj-$(CONFIG_ACPI)		+= pci-acpi.o
 obj-$(CONFIG_PCI_LABEL)		+= pci-label.o
 obj-$(CONFIG_X86_INTEL_MID)	+= pci-mid.o
 obj-$(CONFIG_PCI_SYSCALL)	+= syscall.o

include/acpi/acpi_drivers.h +7 -0

index 14499757338f..de1804aeaf69 100644
@@ -88,7 +88,14 @@ int acpi_pci_link_free_irq(acpi_handle handle);
 
 struct pci_bus;
 
+#ifdef CONFIG_PCI
 struct pci_dev *acpi_get_pci_dev(acpi_handle);
+#else
+static inline struct pci_dev *acpi_get_pci_dev(acpi_handle handle)
+{
+	return NULL;
+}
+#endif
 
 /* Arch-defined function to add a bus to the system */

include/linux/acpi.h +7 -0

index ed80f147bd50..eb1fdf4c196a 100644
@@ -340,7 +340,14 @@ struct pci_dev;
 int acpi_pci_irq_enable (struct pci_dev *dev);
 void acpi_penalize_isa_irq(int irq, int active);
 bool acpi_isa_irq_available(int irq);
+#ifdef CONFIG_PCI
 void acpi_penalize_sci_irq(int irq, int trigger, int polarity);
+#else
+static inline void acpi_penalize_sci_irq(int irq, int trigger,
+					int polarity)
+{
+}
+#endif
 void acpi_pci_irq_disable (struct pci_dev *dev);
 
 extern int ec_read(u8 addr, u8 *val);

include/linux/pci.h +4 -0

index 11c71c4ecf75..51a5a5217667 100644
@@ -1960,7 +1960,11 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev,
 				 enum pcie_reset_state state);
 int pcibios_add_device(struct pci_dev *dev);
 void pcibios_release_device(struct pci_dev *dev);
+#ifdef CONFIG_PCI
 void pcibios_penalize_isa_irq(int irq, int active);
+#else
+static inline void pcibios_penalize_isa_irq(int irq, int active) {}
+#endif
 int pcibios_alloc_irq(struct pci_dev *dev);
 void pcibios_free_irq(struct pci_dev *dev);
 resource_size_t pcibios_default_alignment(void);

Signature · 1 line

2.19.0

Bjorn Helgaas

Acked-by on v9 3/6

2018-12-14 21:52 UTC #4 lore raw

Re: [PATCH v9 3/6] PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set

Quoted 5 lines · Sinan Kaya

We are compiling PCI code today for systems with ACPI and no PCI device present. Remove the useless code and reduce the tight dependency.
Signed-off-by Sinan Kaya <[email protected]>

Acked-by Bjorn Helgaas <[email protected]> # PCI parts

Looks like most of this series is ACPI-related, so I assume Rafael will pick this up.

Quoted 150 lines

---
 arch/x86/include/asm/pci_x86.h | 7 +++++++
 drivers/acpi/Kconfig           | 1 -
 drivers/acpi/Makefile          | 2 +-
 drivers/acpi/internal.h        | 5 +++++
 drivers/pci/Makefile           | 2 +-
 include/acpi/acpi_drivers.h    | 7 +++++++
 include/linux/acpi.h           | 7 +++++++
 include/linux/pci.h            | 4 ++++
 8 files changed, 32 insertions(+), 3 deletions(-)

arch/x86/include/asm/pci_x86.h +7 -0

index 959d618dbb17..73bb404f4d2a 100644
@@ -121,7 +121,14 @@ extern void __init dmi_check_pciprobe(void);
 extern void __init dmi_check_skip_isa_align(void);
 
 /* some common used subsys_initcalls */
+#ifdef CONFIG_PCI
 extern int __init pci_acpi_init(void);
+#else
+static inline int  __init pci_acpi_init(void)
+{
+	return -EINVAL;
+}
+#endif
 extern void __init pcibios_irq_init(void);
 extern int __init pcibios_init(void);
 extern int pci_legacy_init(void);

drivers/acpi/Kconfig +0 -1

index 7cea769c37df..a0abcb3bd673 100644
@@ -9,7 +9,6 @@ config ARCH_SUPPORTS_ACPI
 menuconfig ACPI
 	bool "ACPI (Advanced Configuration and Power Interface) Support"
 	depends on ARCH_SUPPORTS_ACPI
-	depends on PCI
 	select PNP
 	default y if X86
 	help

drivers/acpi/Makefile +1 -1

index edc039313cd6..7c6afc111d76 100644
@@ -39,7 +39,7 @@ acpi-y				+= processor_core.o
 acpi-$(CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC) += processor_pdc.o
 acpi-y				+= ec.o
 acpi-$(CONFIG_ACPI_DOCK)	+= dock.o
-acpi-y				+= pci_root.o pci_link.o pci_irq.o
+acpi-$(CONFIG_PCI)		+= pci_root.o pci_link.o pci_irq.o
 obj-$(CONFIG_ACPI_MCFG)		+= pci_mcfg.o
 acpi-y				+= acpi_lpss.o acpi_apd.o
 acpi-y				+= acpi_platform.o

drivers/acpi/internal.h +5 -0

index 530a3f675490..b7060dae2789 100644
@@ -25,8 +25,13 @@ int acpi_osi_init(void);
 acpi_status acpi_os_initialize1(void);
 void init_acpi_device_notify(void);
 int acpi_scan_init(void);
+#ifdef CONFIG_PCI
 void acpi_pci_root_init(void);
 void acpi_pci_link_init(void);
+#else
+static inline void acpi_pci_root_init(void) {}
+static inline void acpi_pci_link_init(void) {}
+#endif
 void acpi_processor_init(void);
 void acpi_platform_init(void);
 void acpi_pnp_init(void);

drivers/pci/Makefile +1 -1

index f2bda77a2df1..657d642fcc67 100644
@@ -11,6 +11,7 @@ ifdef CONFIG_PCI
 obj-$(CONFIG_PROC_FS)		+= proc.o
 obj-$(CONFIG_SYSFS)		+= slot.o
 obj-$(CONFIG_OF)		+= of.o
+obj-$(CONFIG_ACPI)		+= pci-acpi.o
 endif
 
 obj-$(CONFIG_PCI_QUIRKS)	+= quirks.o

@@ -20,7 +21,6 @@ obj-$(CONFIG_PCI_MSI)		+= msi.o
 obj-$(CONFIG_PCI_ATS)		+= ats.o
 obj-$(CONFIG_PCI_IOV)		+= iov.o
 obj-$(CONFIG_PCI_BRIDGE_EMUL)	+= pci-bridge-emul.o
-obj-$(CONFIG_ACPI)		+= pci-acpi.o
 obj-$(CONFIG_PCI_LABEL)		+= pci-label.o
 obj-$(CONFIG_X86_INTEL_MID)	+= pci-mid.o
 obj-$(CONFIG_PCI_SYSCALL)	+= syscall.o

include/acpi/acpi_drivers.h +7 -0

index 14499757338f..de1804aeaf69 100644
@@ -88,7 +88,14 @@ int acpi_pci_link_free_irq(acpi_handle handle);
 
 struct pci_bus;
 
+#ifdef CONFIG_PCI
 struct pci_dev *acpi_get_pci_dev(acpi_handle);
+#else
+static inline struct pci_dev *acpi_get_pci_dev(acpi_handle handle)
+{
+	return NULL;
+}
+#endif
 
 /* Arch-defined function to add a bus to the system */

include/linux/acpi.h +7 -0

index ed80f147bd50..eb1fdf4c196a 100644
@@ -340,7 +340,14 @@ struct pci_dev;
 int acpi_pci_irq_enable (struct pci_dev *dev);
 void acpi_penalize_isa_irq(int irq, int active);
 bool acpi_isa_irq_available(int irq);
+#ifdef CONFIG_PCI
 void acpi_penalize_sci_irq(int irq, int trigger, int polarity);
+#else
+static inline void acpi_penalize_sci_irq(int irq, int trigger,
+					int polarity)
+{
+}
+#endif
 void acpi_pci_irq_disable (struct pci_dev *dev);
 
 extern int ec_read(u8 addr, u8 *val);

include/linux/pci.h +4 -0

index 11c71c4ecf75..51a5a5217667 100644
@@ -1960,7 +1960,11 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev,
 				 enum pcie_reset_state state);
 int pcibios_add_device(struct pci_dev *dev);
 void pcibios_release_device(struct pci_dev *dev);
+#ifdef CONFIG_PCI
 void pcibios_penalize_isa_irq(int irq, int active);
+#else
+static inline void pcibios_penalize_isa_irq(int irq, int active) {}
+#endif
 int pcibios_alloc_irq(struct pci_dev *dev);
 void pcibios_free_irq(struct pci_dev *dev);
 resource_size_t pcibios_default_alignment(void);

Signature · 1 line

2.19.0

Sinan Kaya

author

2018-12-14 16:33 UTC #5 lore raw

PATCH v9 4/6 ACPICA: Remove PCI bits from ACPICA when CONFIG_PCI is unset

In reply to a message not in the archive

Now that we allow CONFIG_PCI to be unset, remove useless code from ACPICA too.

Signed-off-by Sinan Kaya <[email protected]>

---
 drivers/acpi/acpica/Makefile    | 2 +-
 drivers/acpi/acpica/achware.h   | 9 +++++++++
 include/acpi/platform/aclinux.h | 4 ++++
 3 files changed, 14 insertions(+), 1 deletion(-)

drivers/acpi/acpica/Makefile +1 -1

index b14621da5413..59700433a96e 100644
@@ -77,13 +77,13 @@ acpi-y +=		\
 	hwacpi.o	\
 	hwesleep.o	\
 	hwgpe.o		\
-	hwpci.o		\
 	hwregs.o	\
 	hwsleep.o	\
 	hwvalid.o	\
 	hwxface.o	\
 	hwxfsleep.o
 
+acpi-$(CONFIG_PCI) += hwpci.o
 acpi-$(ACPI_FUTURE_USAGE) += hwtimer.o
 
 acpi-y +=		\

drivers/acpi/acpica/achware.h +9 -0

index 43ce67a9da1f..ef99e2fc37f8 100644
@@ -106,11 +106,20 @@ acpi_hw_enable_runtime_gpe_block(struct acpi_gpe_xrupt_info *gpe_xrupt_info,
 				 struct acpi_gpe_block_info *gpe_block,
 				 void *context);
 
+#ifdef ACPI_PCI_CONFIGURED
 /*
  * hwpci - PCI configuration support
  */
 acpi_status
 acpi_hw_derive_pci_id(struct acpi_pci_id *pci_id,
 		      acpi_handle root_pci_device, acpi_handle pci_region);
+#else
+static inline acpi_status
+acpi_hw_derive_pci_id(struct acpi_pci_id *pci_id, acpi_handle root_pci_device,
+		      acpi_handle pci_region)
+{
+	return AE_SUPPORT;
+}
+#endif
 
 #endif				/* __ACHWARE_H__ */

include/acpi/platform/aclinux.h +4 -0

index 7451b3bca83a..e3d21d014fcc 100644
@@ -33,6 +33,10 @@
 
 /* Kernel specific ACPICA configuration */
 
+#ifdef CONFIG_PCI
+#define ACPI_PCI_CONFIGURED
+#endif
+
 #ifdef CONFIG_ACPI_REDUCED_HARDWARE_ONLY
 #define ACPI_REDUCED_HARDWARE 1
 #endif

Signature · 1 line

2.19.0

Sinan Kaya

author

2018-12-14 16:33 UTC #6 lore raw

PATCH v9 5/6 arm64: select ACPI PCI code only both features are enabled

In reply to a message not in the archive

ACPI and PCI are no longer coupled to each other. Specify requirements for both when pulling in code.

Signed-off-by Sinan Kaya <[email protected]>

---
 arch/arm64/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm64/Kconfig +2 -2

index ea2ab0330e3a..bcb6262044d8 100644
@@ -5,7 +5,7 @@ config ARM64
 	select ACPI_GTDT if ACPI
 	select ACPI_IORT if ACPI
 	select ACPI_REDUCED_HARDWARE_ONLY if ACPI
-	select ACPI_MCFG if ACPI
+	select ACPI_MCFG if (ACPI && PCI)
 	select ACPI_SPCR_TABLE if ACPI
 	select ACPI_PPTT if ACPI
 	select ARCH_CLOCKSOURCE_DATA

@@ -163,7 +163,7 @@ config ARM64
 	select OF
 	select OF_EARLY_FLATTREE
 	select OF_RESERVED_MEM
-	select PCI_ECAM if ACPI
+	select PCI_ECAM if (ACPI && PCI)
 	select POWER_RESET
 	select POWER_SUPPLY
 	select REFCOUNT_FULL

Signature · 1 line

2.19.0

Sinan Kaya

author

2018-12-14 16:33 UTC #7 lore raw

PATCH v9 6/6 PCI: Stub out pci_request_acs() when CONFIG_PCI is not set

In reply to a message not in the archive

ACPI IORT table code relies on pci_request_acs() to be present. Define a stub function when CONFI_PCI is not set.

Signed-off-by Sinan Kaya <[email protected]>

---
 include/linux/pci.h | 4 ++++
 1 file changed, 4 insertions(+)

include/linux/pci.h +4 -0

index 51a5a5217667..f0f2f55ea93c 100644
@@ -2101,7 +2101,11 @@ static inline struct pci_dev *pcie_find_root_port(struct pci_dev *dev)
 	return NULL;
 }
 
+#ifdef CONFIG_PCI
 void pci_request_acs(void);
+#else
+static inline void pci_request_acs(void) {}
+#endif
 bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags);
 bool pci_acs_path_enabled(struct pci_dev *start,
 			  struct pci_dev *end, u16 acs_flags);

Signature · 1 line

2.19.0

Bjorn Helgaas

2018-12-14 22:26 UTC #8 lore raw

Re: [PATCH v9 6/6] PCI: Stub out pci_request_acs() when CONFIG_PCI is not set

[+cc Lorenzo, Robin, Logan]

Quoted 2 lines · Sinan Kaya

ACPI IORT table code relies on pci_request_acs() to be present. Define a stub function when CONFI_PCI is not set.

This doesn't seem like the simplest approach to me, but I probably don't understand what's going on in IORT.

It looks like *all* of iort_enable_acs() (the caller of pci_request_acs()) is PCI-specific; at least, the whole thing is wrapped in a test for ACPI_IORT_NODE_PCI_ROOT_COMPLEX. So the whole function could be wrapped in #ifdef CONFIG_PCI.

Here's the caller of iort_enable_acs():

  iort_init_platform_devices
    acs_enabled = false
    for (i = 0; i < iort->node_count; i++) {
      if (!acs_enabled)
        acs_enabled = iort_enable_acs(iort_node);

It seems like the acs_enabled state could be encapsulated inside iort_enable_acs().

Today pci_request_acs() is a system-wide thing, but I don't know why that's the case. Isn't it conceivable that different PCI hierarchies could have different ACS policies, e.g., because of P2P DMA or something?

Bottom line, pci_request_acs() is being called from what looks like PCI-specific code in IORT, and it would make more sense to me to prune out that code in IORT than to make a stub pci_request_acs().

Quoted 24 lines

Signed-off-by Sinan Kaya <[email protected]>

---
 include/linux/pci.h | 4 ++++
 1 file changed, 4 insertions(+)

include/linux/pci.h +4 -0

index 51a5a5217667..f0f2f55ea93c 100644
@@ -2101,7 +2101,11 @@ static inline struct pci_dev *pcie_find_root_port(struct pci_dev *dev)
 	return NULL;
 }
 
+#ifdef CONFIG_PCI
 void pci_request_acs(void);
+#else
+static inline void pci_request_acs(void) {}
+#endif
 bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags);
 bool pci_acs_path_enabled(struct pci_dev *start,
 			  struct pci_dev *end, u16 acs_flags);

Signature · 1 line

2.19.0

Robin Murphy

2018-12-14 23:33 UTC #9 lore raw

Re: [PATCH v9 6/6] PCI: Stub out pci_request_acs() when CONFIG_PCI is not set

Quoted 16 lines · Bjorn Helgaas

[+cc Lorenzo, Robin, Logan]
Quoted 2 lines · Sinan Kaya
ACPI IORT table code relies on pci_request_acs() to be present. Define a stub function when CONFI_PCI is not set.
This doesn't seem like the simplest approach to me, but I probably don't understand what's going on in IORT.
It looks like *all* of iort_enable_acs() (the caller of pci_request_acs()) is PCI-specific; at least, the whole thing is wrapped in a test for ACPI_IORT_NODE_PCI_ROOT_COMPLEX. So the whole function could be wrapped in #ifdef CONFIG_PCI.
Here's the caller of iort_enable_acs():

Quoted 8 lines

   iort_init_platform_devices
     acs_enabled = false
     for (i = 0; i < iort->node_count; i++) {
       if (!acs_enabled)
         acs_enabled = iort_enable_acs(iort_node);
It seems like the acs_enabled state could be encapsulated inside iort_enable_acs().

It could, but with the tiny disadvantage of having to allocate static storage for it to maintain the "don't bother checking if we want ACS if we've already requested it once" logic for multiple root complexes.

Quoted 4 lines

Today pci_request_acs() is a system-wide thing, but I don't know why that's the case. Isn't it conceivable that different PCI hierarchies could have different ACS policies, e.g., because of P2P DMA or something?

I can certainly imagine systems using entirely separate PCI domains for, say, expansion slots vs. on-board/on-chip devices, where the former may be expected to be assigned to VMs or userspace drivers and the latter only ever controlled by the host kernel. Not forcing ACS overhead upon the "non-virtualisable" (or "trusted" vs. "untrusted") domain could perhaps be beneficial in some cases.

Quoted 3 lines

Bottom line, pci_request_acs() is being called from what looks like PCI-specific code in IORT, and it would make more sense to me to prune out that code in IORT than to make a stub pci_request_acs().

Agreed - without CONFIG_PCI the whole of iort_enable_acs() may as well be stubbed out, because it won't achieve anything anyway. The implication is that if we have a root complex whose inbound DMA is controlled by an SMMU, we should enable ACS to make VFIO useful (or in case we want to enforce strict DMA isolation in general), but if Linux is never going to touch any PCI devices there's no point even looking at the RC node(s). TBH I'm pretty dubious about having IORT at all without PCI, but I suppose there could potentially be embedded SoCs which have an ITS and platform MSIs, or use an SMMU for their video/display subsystem, and for some reason demand ACPI-based firmware.

Robin.

Quoted 24 lines

Signed-off-by Sinan Kaya <[email protected]>

---
  include/linux/pci.h | 4 ++++
  1 file changed, 4 insertions(+)

include/linux/pci.h +4 -0

index 51a5a5217667..f0f2f55ea93c 100644
@@ -2101,7 +2101,11 @@ static inline struct pci_dev *pcie_find_root_port(struct pci_dev *dev)
  	return NULL;
  }
 
+#ifdef CONFIG_PCI
  void pci_request_acs(void);
+#else
+static inline void pci_request_acs(void) {}
+#endif
  bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags);
  bool pci_acs_path_enabled(struct pci_dev *start,
  			  struct pci_dev *end, u16 acs_flags);

Signature · 1 line

2.19.0