From 7fb89e1d44cb6aec342e5cca6ed6371d818a428c Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Mon, 20 Apr 2020 11:27:53 +0200 Subject: ACPI/IORT: take _DMA methods into account for named components Where IORT nodes for named components can describe simple DMA limits expressed as the number of address bits a device can drive, _DMA methods in AML can express more complex topologies, involving DMA translation in particular. Currently, we only take this _DMA method into account if it appears on a ACPI device node describing a PCIe root complex, but it is perfectly acceptable to use them for named components as well, so let's ensure we take them into account in those cases too. Note that such named components are expected to reside under a pseudo-bus node such as the ACPI0004 container device, which should be providing the _DMA method as well as a _CRS (as mandated by the ACPI spec). This is not enforced by the code however. Reported-by: Andrei Warkentin Signed-off-by: Ard Biesheuvel Acked-by: Lorenzo Pieralisi Link: https://lore.kernel.org/r/20200420092753.9819-1-ardb@kernel.org Signed-off-by: Will Deacon --- drivers/acpi/arm64/iort.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index 7d04424189df..051b2ce03070 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -1148,13 +1148,10 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size) else size = 1ULL << 32; - if (dev_is_pci(dev)) { - ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size); - if (ret == -ENODEV) - ret = rc_dma_get_range(dev, &size); - } else { - ret = nc_dma_get_range(dev, &size); - } + ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size); + if (ret == -ENODEV) + ret = dev_is_pci(dev) ? rc_dma_get_range(dev, &size) + : nc_dma_get_range(dev, &size); if (!ret) { /* -- cgit v1.2.3 From 6d3b29d07c3c55532e09d004a1466358c71affa7 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Fri, 1 May 2020 18:10:13 +0200 Subject: Revert "ACPI/IORT: Fix 'Number of IDs' handling in iort_id_map()" This reverts commit 3c23b83a88d00383e1d498cfa515249aa2fe0238. Signed-off-by: Ard Biesheuvel Link: https://lore.kernel.org/r/20200501161014.5935-2-ardb@kernel.org Signed-off-by: Will Deacon --- drivers/acpi/arm64/iort.c | 57 ++--------------------------------------------- 1 file changed, 2 insertions(+), 55 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index 051b2ce03070..d2fb33a43652 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -299,59 +299,6 @@ out: return status; } -struct iort_workaround_oem_info { - char oem_id[ACPI_OEM_ID_SIZE + 1]; - char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1]; - u32 oem_revision; -}; - -static bool apply_id_count_workaround; - -static struct iort_workaround_oem_info wa_info[] __initdata = { - { - .oem_id = "HISI ", - .oem_table_id = "HIP07 ", - .oem_revision = 0, - }, { - .oem_id = "HISI ", - .oem_table_id = "HIP08 ", - .oem_revision = 0, - } -}; - -static void __init -iort_check_id_count_workaround(struct acpi_table_header *tbl) -{ - int i; - - for (i = 0; i < ARRAY_SIZE(wa_info); i++) { - if (!memcmp(wa_info[i].oem_id, tbl->oem_id, ACPI_OEM_ID_SIZE) && - !memcmp(wa_info[i].oem_table_id, tbl->oem_table_id, ACPI_OEM_TABLE_ID_SIZE) && - wa_info[i].oem_revision == tbl->oem_revision) { - apply_id_count_workaround = true; - pr_warn(FW_BUG "ID count for ID mapping entry is wrong, applying workaround\n"); - break; - } - } -} - -static inline u32 iort_get_map_max(struct acpi_iort_id_mapping *map) -{ - u32 map_max = map->input_base + map->id_count; - - /* - * The IORT specification revision D (Section 3, table 4, page 9) says - * Number of IDs = The number of IDs in the range minus one, but the - * IORT code ignored the "minus one", and some firmware did that too, - * so apply a workaround here to keep compatible with both the spec - * compliant and non-spec compliant firmwares. - */ - if (apply_id_count_workaround) - map_max--; - - return map_max; -} - static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in, u32 *rid_out) { @@ -368,7 +315,8 @@ static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in, return -ENXIO; } - if (rid_in < map->input_base || rid_in > iort_get_map_max(map)) + if (rid_in < map->input_base || + (rid_in >= map->input_base + map->id_count)) return -ENXIO; *rid_out = map->output_base + (rid_in - map->input_base); @@ -1700,6 +1648,5 @@ void __init acpi_iort_init(void) return; } - iort_check_id_count_workaround(iort_table); iort_init_platform_devices(); } -- cgit v1.2.3 From 539979b6ec62f7bba40b0452b0574a0f4ec4fe4e Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Fri, 1 May 2020 18:10:14 +0200 Subject: ACPI/IORT: work around num_ids ambiguity The ID mapping table structure of the IORT table describes the size of a range using a num_ids field carrying the number of IDs in the region minus one. This has been misinterpreted in the past in the parsing code, and firmware is known to have shipped where this results in an ambiguity, where regions that should be adjacent have an overlap of one value. So let's work around this by detecting this case specifically: when resolving an ID translation, allow one that matches right at the end of a multi-ID region to be superseded by a subsequent one. To prevent potential regressions on broken firmware that happened to work before, only take the subsequent match into account if it occurs at the start of a mapping region. Signed-off-by: Ard Biesheuvel Reviewed-by: Lorenzo Pieralisi Link: https://lore.kernel.org/r/20200501161014.5935-3-ardb@kernel.org Signed-off-by: Will Deacon --- drivers/acpi/arm64/iort.c | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index d2fb33a43652..b011d25af676 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -300,7 +300,7 @@ out: } static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in, - u32 *rid_out) + u32 *rid_out, bool check_overlap) { /* Single mapping does not care for input id */ if (map->flags & ACPI_IORT_ID_SINGLE_MAPPING) { @@ -316,10 +316,34 @@ static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in, } if (rid_in < map->input_base || - (rid_in >= map->input_base + map->id_count)) + (rid_in > map->input_base + map->id_count)) return -ENXIO; + if (check_overlap) { + /* + * We already found a mapping for this input ID at the end of + * another region. If it coincides with the start of this + * region, we assume the prior match was due to the off-by-1 + * issue mentioned below, and allow it to be superseded. + * Otherwise, things are *really* broken, and we just disregard + * duplicate matches entirely to retain compatibility. + */ + pr_err(FW_BUG "[map %p] conflicting mapping for input ID 0x%x\n", + map, rid_in); + if (rid_in != map->input_base) + return -ENXIO; + } + *rid_out = map->output_base + (rid_in - map->input_base); + + /* + * Due to confusion regarding the meaning of the id_count field (which + * carries the number of IDs *minus 1*), we may have to disregard this + * match if it is at the end of the range, and overlaps with the start + * of another one. + */ + if (map->id_count > 0 && rid_in == map->input_base + map->id_count) + return -EAGAIN; return 0; } @@ -404,7 +428,8 @@ static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node, /* Parse the ID mapping tree to find specified node type */ while (node) { struct acpi_iort_id_mapping *map; - int i, index; + int i, index, rc = 0; + u32 out_ref = 0, map_id = id; if (IORT_TYPE_MASK(node->type) & type_mask) { if (id_out) @@ -438,15 +463,18 @@ static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node, if (i == index) continue; - if (!iort_id_map(map, node->type, id, &id)) + rc = iort_id_map(map, node->type, map_id, &id, out_ref); + if (!rc) break; + if (rc == -EAGAIN) + out_ref = map->output_reference; } - if (i == node->mapping_count) + if (i == node->mapping_count && !out_ref) goto fail_map; node = ACPI_ADD_PTR(struct acpi_iort_node, iort_table, - map->output_reference); + rc ? out_ref : map->output_reference); } fail_map: -- cgit v1.2.3 From 44cdc7b16e0a6a69a170cf98006aba1c1359ee3b Mon Sep 17 00:00:00 2001 From: Hanjun Guo Date: Fri, 8 May 2020 11:56:38 +0800 Subject: ACPI: IORT: Add extra message "applying workaround" for off-by-1 issue As we already applied a workaround for the off-by-1 issue, it's good to add extra message "applying workaround" to make people less uneasy to see FW_BUG message in the boot log. Signed-off-by: Hanjun Guo Acked-by: Ard Biesheuvel Link: https://lore.kernel.org/r/1588910198-8348-1-git-send-email-guohanjun@huawei.com Signed-off-by: Will Deacon --- drivers/acpi/arm64/iort.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/acpi') diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index b011d25af676..6e445bc55537 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -332,6 +332,8 @@ static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in, map, rid_in); if (rid_in != map->input_base) return -ENXIO; + + pr_err(FW_BUG "applying workaround.\n"); } *rid_out = map->output_base + (rid_in - map->input_base); -- cgit v1.2.3 From 5ec605108ff4901aedd62ee1bdd4250f2f7cf978 Mon Sep 17 00:00:00 2001 From: Hanjun Guo Date: Fri, 8 May 2020 12:05:52 +0800 Subject: ACPI: GTDT: Put GTDT table after parsing The mapped GTDT table needs to be released after the driver init. Signed-off-by: Hanjun Guo Link: https://lore.kernel.org/r/1588910753-18543-1-git-send-email-guohanjun@huawei.com Signed-off-by: Will Deacon --- drivers/acpi/arm64/gtdt.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/arm64/gtdt.c b/drivers/acpi/arm64/gtdt.c index 01962c63a711..f2d0e5915dab 100644 --- a/drivers/acpi/arm64/gtdt.c +++ b/drivers/acpi/arm64/gtdt.c @@ -394,7 +394,7 @@ static int __init gtdt_sbsa_gwdt_init(void) */ ret = acpi_gtdt_init(table, &timer_count); if (ret || !timer_count) - return ret; + goto out_put_gtdt; for_each_platform_timer(platform_timer) { if (is_non_secure_watchdog(platform_timer)) { @@ -408,6 +408,8 @@ static int __init gtdt_sbsa_gwdt_init(void) if (gwdt_count) pr_info("found %d SBSA generic Watchdog(s).\n", gwdt_count); +out_put_gtdt: + acpi_put_table(table); return ret; } -- cgit v1.2.3 From 701dafe0670c736c0131328c0fd64c1190f0bb0c Mon Sep 17 00:00:00 2001 From: Hanjun Guo Date: Fri, 8 May 2020 12:05:53 +0800 Subject: ACPI: IORT: Add comments for not calling acpi_put_table() The iort_table will be used at runtime after acpi_iort_init(), so add some comments to clarify this to make it less confusing. Signed-off-by: Hanjun Guo Link: https://lore.kernel.org/r/1588910753-18543-2-git-send-email-guohanjun@huawei.com Signed-off-by: Will Deacon --- drivers/acpi/arm64/iort.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers/acpi') diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index 6e445bc55537..619a3e503346 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -1667,6 +1667,10 @@ void __init acpi_iort_init(void) { acpi_status status; + /* iort_table will be used at runtime after the iort init, + * so we don't need to call acpi_put_table() to release + * the IORT table mapping. + */ status = acpi_get_table(ACPI_SIG_IORT, 0, &iort_table); if (ACPI_FAILURE(status)) { if (status != AE_NOT_FOUND) { -- cgit v1.2.3 From 50c8ab8d9fbf5b18d5162a797ca26568afc0af1a Mon Sep 17 00:00:00 2001 From: Tuan Phan Date: Wed, 20 May 2020 10:13:07 -0700 Subject: ACPI/IORT: Fix PMCG node single ID mapping handling An IORT PMCG node can have no ID mapping if its overflow interrupt is wire based therefore the code that parses the PMCG node can not assume the node will always have a single mapping present at index 0. Fix iort_get_id_mapping_index() by checking for an overflow interrupt and mapping count. Fixes: 24e516049360 ("ACPI/IORT: Add support for PMCG") Signed-off-by: Tuan Phan Reviewed-by: Hanjun Guo Acked-by: Lorenzo Pieralisi Link: https://lore.kernel.org/r/1589994787-28637-1-git-send-email-tuanphan@os.amperecomputing.com Signed-off-by: Will Deacon --- drivers/acpi/arm64/iort.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'drivers/acpi') diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index 619a3e503346..9c40709c2f4e 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -388,6 +388,7 @@ static struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node, static int iort_get_id_mapping_index(struct acpi_iort_node *node) { struct acpi_iort_smmu_v3 *smmu; + struct acpi_iort_pmcg *pmcg; switch (node->type) { case ACPI_IORT_NODE_SMMU_V3: @@ -415,6 +416,10 @@ static int iort_get_id_mapping_index(struct acpi_iort_node *node) return smmu->id_mapping_index; case ACPI_IORT_NODE_PMCG: + pmcg = (struct acpi_iort_pmcg *)node->node_data; + if (pmcg->overflow_gsiv || node->mapping_count == 0) + return -EINVAL; + return 0; default: return -EINVAL; -- cgit v1.2.3 From 09cda9a71350e61d8803058470697b95f3d3b4cb Mon Sep 17 00:00:00 2001 From: Zenghui Yu Date: Sat, 9 May 2020 17:34:30 +0800 Subject: ACPI/IORT: Remove the unused __get_pci_rid() Since commit bc8648d49a95 ("ACPI/IORT: Handle PCI aliases properly for IOMMUs"), __get_pci_rid() has become actually unused and can be removed. Signed-off-by: Zenghui Yu Acked-by: Lorenzo Pieralisi Acked-by: Hanjun Guo Link: https://lore.kernel.org/r/20200509093430.1983-1-yuzenghui@huawei.com Signed-off-by: Will Deacon --- drivers/acpi/arm64/iort.c | 9 --------- 1 file changed, 9 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index 9c40709c2f4e..28a6b387e80e 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -772,15 +772,6 @@ void acpi_configure_pmsi_domain(struct device *dev) dev_set_msi_domain(dev, msi_domain); } -static int __maybe_unused __get_pci_rid(struct pci_dev *pdev, u16 alias, - void *data) -{ - u32 *rid = data; - - *rid = alias; - return 0; -} - #ifdef CONFIG_IOMMU_API static struct acpi_iort_node *iort_get_msi_resv_iommu(struct device *dev) { -- cgit v1.2.3