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

From: Robin Murphy
Date: Fri Dec 14 2018 - 18:33:45 EST

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():

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

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

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.


Signed-off-by: Sinan Kaya <okaya@xxxxxxxxxx>
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);
+static inline void pci_request_acs(void) {}
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);