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

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

[Sinan Kaya]

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

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 drivers/acpi/reboot.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/reboot.c b/drivers/acpi/reboot.c
index 6fa9c2a4cfe9..d75e637ee36a 100644
--- a/drivers/acpi/reboot.c
+++ b/drivers/acpi/reboot.c
@@ -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");
-- 
2.19.0

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

[Sinan Kaya]

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 <okaya@kernel.org>
---
 drivers/acpi/osl.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index b48874b8e1ea..524fd5f33ea4 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -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)
-- 
2.19.0

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

[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 <okaya@kernel.org>
---
 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(-)

diff --git a/arch/x86/include/asm/pci_x86.h b/arch/x86/include/asm/pci_x86.h
index 959d618dbb17..73bb404f4d2a 100644
--- a/arch/x86/include/asm/pci_x86.h
+++ b/arch/x86/include/asm/pci_x86.h
@@ -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);
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 7cea769c37df..a0abcb3bd673 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -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
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index edc039313cd6..7c6afc111d76 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -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
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 530a3f675490..b7060dae2789 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -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);
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index f2bda77a2df1..657d642fcc67 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -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
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index 14499757338f..de1804aeaf69 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -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 */
 
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index ed80f147bd50..eb1fdf4c196a 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -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);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 11c71c4ecf75..51a5a5217667 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -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);
-- 
2.19.0

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

[Bjorn Helgaas]

On Fri, Dec 14, 2018 at 04:33:16PM +0000, Sinan Kaya wrote:
> 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 <okaya@kernel.org>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>	# PCI parts

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

> ---
>  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(-)
> 
> diff --git a/arch/x86/include/asm/pci_x86.h b/arch/x86/include/asm/pci_x86.h
> index 959d618dbb17..73bb404f4d2a 100644
> --- a/arch/x86/include/asm/pci_x86.h
> +++ b/arch/x86/include/asm/pci_x86.h
> @@ -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);
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 7cea769c37df..a0abcb3bd673 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -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
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index edc039313cd6..7c6afc111d76 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -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
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index 530a3f675490..b7060dae2789 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -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);
> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> index f2bda77a2df1..657d642fcc67 100644
> --- a/drivers/pci/Makefile
> +++ b/drivers/pci/Makefile
> @@ -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
> diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
> index 14499757338f..de1804aeaf69 100644
> --- a/include/acpi/acpi_drivers.h
> +++ b/include/acpi/acpi_drivers.h
> @@ -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 */
>  
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index ed80f147bd50..eb1fdf4c196a 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -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);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 11c71c4ecf75..51a5a5217667 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -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);
> -- 
> 2.19.0
> 

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

[Sinan Kaya]

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

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 drivers/acpi/acpica/Makefile    | 2 +-
 drivers/acpi/acpica/achware.h   | 9 +++++++++
 include/acpi/platform/aclinux.h | 4 ++++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/acpica/Makefile b/drivers/acpi/acpica/Makefile
index b14621da5413..59700433a96e 100644
--- a/drivers/acpi/acpica/Makefile
+++ b/drivers/acpi/acpica/Makefile
@@ -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 +=		\
diff --git a/drivers/acpi/acpica/achware.h b/drivers/acpi/acpica/achware.h
index 43ce67a9da1f..ef99e2fc37f8 100644
--- a/drivers/acpi/acpica/achware.h
+++ b/drivers/acpi/acpica/achware.h
@@ -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__ */
diff --git a/include/acpi/platform/aclinux.h b/include/acpi/platform/aclinux.h
index 7451b3bca83a..e3d21d014fcc 100644
--- a/include/acpi/platform/aclinux.h
+++ b/include/acpi/platform/aclinux.h
@@ -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
-- 
2.19.0

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

[Sinan Kaya]

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

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 arch/arm64/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index ea2ab0330e3a..bcb6262044d8 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -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
-- 
2.19.0

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

[Sinan Kaya]

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 <okaya@kernel.org>
---
 include/linux/pci.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 51a5a5217667..f0f2f55ea93c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -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);
-- 
2.19.0

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

[Bjorn Helgaas]

[+cc Lorenzo, Robin, Logan]

On Fri, Dec 14, 2018 at 04:33:19PM +0000, Sinan Kaya wrote:
> 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().

> Signed-off-by: Sinan Kaya <okaya@kernel.org>
> ---
>  include/linux/pci.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 51a5a5217667..f0f2f55ea93c 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -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);
> -- 
> 2.19.0
> 

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

[Robin Murphy]

On 2018-12-14 10:26 pm, Bjorn Helgaas wrote:
> [+cc Lorenzo, Robin, Logan]
> 
> On Fri, Dec 14, 2018 at 04:33:19PM +0000, Sinan Kaya wrote:
>> 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().

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.

> 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.

> 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.

>> Signed-off-by: Sinan Kaya <okaya@kernel.org>
>> ---
>>   include/linux/pci.h | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 51a5a5217667..f0f2f55ea93c 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -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);
>> -- 
>> 2.19.0
>>