From: Leon Romanovsky <[email protected]>

Direct Translated P2P does not weaken IOMMU isolation because a Translated
Request carries an address supplied by the IOMMU. Config-space read
failures, however, leave ACS state unknown and must not report isolation.

Exercise both cases with fake config-space operations. Also cover missing
and unrequested controls and a missing ACS capability.

Signed-off-by: Leon Romanovsky <[email protected]>
---
 drivers/pci/pci.c          |   4 +-
 drivers/pci/pci.h          |   1 +
 drivers/pci/pci_acs_test.c | 138 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 142 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index f7d94ecf9157..4a9ab3882aac 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3578,7 +3578,8 @@ void pci_configure_ari(struct pci_dev *dev)
        }
 }
 
-static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags)
+VISIBLE_IF_KUNIT
+bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags)
 {
        int pos;
        u16 ctrl;
@@ -3598,6 +3599,7 @@ static bool pci_acs_flags_enabled(struct pci_dev *pdev, 
u16 acs_flags)
                return false;
        return (ctrl & acs_flags) == acs_flags;
 }
+EXPORT_SYMBOL_IF_KUNIT(pci_acs_flags_enabled);
 
 /**
  * pci_acs_enabled - test ACS against required flags for a given device
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 56f821e40637..5bc703ff0c86 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -1104,6 +1104,7 @@ enum pci_acs_p2pdma_state {
 };
 
 #if IS_ENABLED(CONFIG_KUNIT)
+bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags);
 enum pci_acs_p2pdma_state pci_acs_p2pdma_request(u16 ctrl,
                                                unsigned int tlp_flags);
 enum pci_acs_p2pdma_state pci_acs_p2pdma_completion(u16 ctrl,
diff --git a/drivers/pci/pci_acs_test.c b/drivers/pci/pci_acs_test.c
index 28eced6dd672..880fc810080a 100644
--- a/drivers/pci/pci_acs_test.c
+++ b/drivers/pci/pci_acs_test.c
@@ -102,6 +102,140 @@ static void pci_acs_p2pdma_completion_test(struct kunit 
*test)
                        c->expect);
 }
 
+/* Flags an IOMMU asks for; see REQ_ACS_FLAGS in drivers/iommu/iommu.c. */
+#define ACS_REQ_FLAGS  (PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF)
+#define ACS_ALL_CAPS   (PCI_ACS_SV | PCI_ACS_TB | PCI_ACS_RR | PCI_ACS_CR | \
+                        PCI_ACS_UF | PCI_ACS_DT)
+#define ACS_TEST_CAP   0x100
+
+struct acs_ctrl_cfg {
+       unsigned int devfn;
+       u16 cap;        /* Offset where the ACS capability responds */
+       u16 ctrl;
+       bool fail_read;
+};
+
+static int acs_ctrl_read(struct pci_bus *bus, unsigned int devfn,
+                        int where, int size, u32 *val)
+{
+       struct acs_ctrl_cfg *cfg = bus->sysdata;
+
+       *val = 0;
+       if (cfg->fail_read)
+               return PCIBIOS_DEVICE_NOT_FOUND;
+
+       if (devfn == cfg->devfn && size == 2 &&
+           where == cfg->cap + PCI_ACS_CTRL)
+               *val = cfg->ctrl;
+       return PCIBIOS_SUCCESSFUL;
+}
+
+static int acs_ctrl_write(struct pci_bus *bus, unsigned int devfn,
+                         int where, int size, u32 val)
+{
+       return PCIBIOS_SUCCESSFUL;
+}
+
+static struct pci_ops acs_ctrl_ops = {
+       .read   = acs_ctrl_read,
+       .write  = acs_ctrl_write,
+};
+
+struct acs_isolation_case {
+       const char *desc;
+       u16 ctrl;
+       u16 req;
+       bool expect;
+};
+
+static const struct acs_isolation_case acs_isolation_cases[] = {
+       { "all_enabled", ACS_REQ_FLAGS, ACS_REQ_FLAGS, true },
+       /* Translated Requests remain isolated by their IOMMU translation. */
+       { "dt", ACS_REQ_FLAGS | PCI_ACS_DT, ACS_REQ_FLAGS, true },
+       { "rr_not_enabled", PCI_ACS_SV | PCI_ACS_CR | PCI_ACS_UF,
+         ACS_REQ_FLAGS, false },
+       { "rr_not_required", PCI_ACS_SV | PCI_ACS_CR | PCI_ACS_UF,
+         PCI_ACS_SV | PCI_ACS_CR | PCI_ACS_UF, true },
+};
+
+static void acs_isolation_desc(const struct acs_isolation_case *c, char *desc)
+{
+       strscpy(desc, c->desc, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(acs_isolation, acs_isolation_cases, acs_isolation_desc);
+
+static void pci_acs_flags_enabled_test(struct kunit *test)
+{
+       const struct acs_isolation_case *c = test->param_value;
+       struct acs_ctrl_cfg cfg = {
+               .devfn = PCI_DEVFN(0, 0),
+               .cap = ACS_TEST_CAP,
+               .ctrl = c->ctrl,
+       };
+       struct pci_bus *bus = kunit_kzalloc(test, sizeof(*bus), GFP_KERNEL);
+       struct pci_dev *pdev = kunit_kzalloc(test, sizeof(*pdev), GFP_KERNEL);
+
+       KUNIT_ASSERT_NOT_NULL(test, bus);
+       KUNIT_ASSERT_NOT_NULL(test, pdev);
+
+       bus->ops = &acs_ctrl_ops;
+       bus->sysdata = &cfg;
+
+       pdev->bus = bus;
+       pdev->devfn = cfg.devfn;
+       pdev->acs_cap = ACS_TEST_CAP;
+       pdev->acs_capabilities = ACS_ALL_CAPS;
+
+       KUNIT_EXPECT_EQ(test, pci_acs_flags_enabled(pdev, c->req), c->expect);
+}
+
+static bool acs_isolated(struct kunit *test, struct acs_ctrl_cfg *cfg,
+                        u16 acs_cap, u16 acs_flags)
+{
+       struct pci_bus *bus = kunit_kzalloc(test, sizeof(*bus), GFP_KERNEL);
+       struct pci_dev *pdev = kunit_kzalloc(test, sizeof(*pdev), GFP_KERNEL);
+
+       KUNIT_ASSERT_NOT_NULL(test, bus);
+       KUNIT_ASSERT_NOT_NULL(test, pdev);
+
+       bus->ops = &acs_ctrl_ops;
+       bus->sysdata = cfg;
+
+       pdev->bus = bus;
+       pdev->devfn = cfg->devfn;
+       pdev->acs_cap = acs_cap;
+       pdev->acs_capabilities = ACS_ALL_CAPS;
+
+       return pci_acs_flags_enabled(pdev, acs_flags);
+}
+
+static void pci_acs_flags_no_cap_test(struct kunit *test)
+{
+       struct acs_ctrl_cfg cfg = {
+               .devfn = PCI_DEVFN(0, 0),
+               .cap = 0,
+               .ctrl = ACS_REQ_FLAGS,
+       };
+
+       KUNIT_EXPECT_FALSE(test, acs_isolated(test, &cfg, 0, ACS_REQ_FLAGS));
+}
+
+static void pci_acs_flags_read_fails_test(struct kunit *test)
+{
+       u16 no_rr = ACS_REQ_FLAGS & ~PCI_ACS_RR;
+       struct acs_ctrl_cfg cfg = {
+               .devfn = PCI_DEVFN(0, 0),
+               .cap = ACS_TEST_CAP,
+               .ctrl = ACS_REQ_FLAGS,
+       };
+
+       KUNIT_EXPECT_TRUE(test, acs_isolated(test, &cfg, ACS_TEST_CAP, no_rr));
+
+       cfg.fail_read = true;
+       KUNIT_EXPECT_FALSE(test, acs_isolated(test, &cfg, ACS_TEST_CAP, no_rr));
+}
+
 /*
  * Drive calc_map_type_and_dist() over a fabricated PCIe fabric matching the
  * canonical topology of two devices below one switch:
@@ -445,6 +579,10 @@ static struct kunit_case pci_acs_test_cases[] = {
                         acs_request_gen_params),
        KUNIT_CASE_PARAM(pci_acs_p2pdma_completion_test,
                         acs_completion_gen_params),
+       KUNIT_CASE_PARAM(pci_acs_flags_enabled_test,
+                        acs_isolation_gen_params),
+       KUNIT_CASE(pci_acs_flags_no_cap_test),
+       KUNIT_CASE(pci_acs_flags_read_fails_test),
        KUNIT_CASE(acs_walk_bus_addr_test),
        KUNIT_CASE(acs_walk_request_redirect_test),
        KUNIT_CASE(acs_walk_completion_redirect_test),

-- 
2.55.0

Reply via email to