| Message ID | 1440766163-12101-1-git-send-email-dennis.chen@arm.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Hi Dennis, You should CC linux-pci@vger.kernel.org and the PCI subsystem maintainer next time. On Fri, Aug 28, 2015 at 01:49:23PM +0100, Dennis Chen wrote: > This patch will fall back to ACPI MCFG table if _CBA method fails > to get the configuration base address of ECAM. Firmware on ARM > platform uses MCFG table instead of _CBA method. This is needed > to scan the PCIe root complex for ARM SoC. Code to enumerate PCI with ACPI on ARM64 is under review: https://lkml.org/lkml/2015/6/8/443 Having said that, I do not think this patch will be needed, you can't add code in the kernel because it may be needed in the future, I do not see how it can be possibly useful at present on ARM64 unless you pulled some patch dependencies; if that's the case you should have listed them. This patch can't be review standalone since it has no use in the current kernel (at least for ARM64, it should be tested on x86 though). Thanks, Lorenzo > Signed-off-by: Dennis Chen <dennis.chen@arm.com> > --- > drivers/pci/pci-acpi.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 97 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c > index 314a625..211b9d9 100644 > --- a/drivers/pci/pci-acpi.c > +++ b/drivers/pci/pci-acpi.c > @@ -27,18 +27,110 @@ const u8 pci_acpi_dsm_uuid[] = { > 0x91, 0x17, 0xea, 0x4d, 0x19, 0xc3, 0x43, 0x4d > }; > > +static int __init acpi_mcfg_check_entry(struct acpi_table_mcfg *mcfg) > +{ > + if (mcfg->header.revision != 1) { > + pr_err("invalid header revision:%d in ACPI MCFG table\n", > + mcfg->header.revision); > + return -EINVAL; > + } > + > + return 0; > +} > + > +static phys_addr_t __init acpi_get_mcfg_cba(u16 segment, u8 bus) > +{ > + struct acpi_table_header *table = NULL; > + acpi_size tbl_size; > + struct acpi_table_mcfg *mcfg = NULL; > + struct acpi_mcfg_allocation *cfg_table, *cfg; > + acpi_status status; > + phys_addr_t cba = 0; > + unsigned long i; > + int entries; > + > + if (acpi_disabled) > + return 0; > + > + status = acpi_get_table_with_size(ACPI_SIG_MCFG, 0, &table, &tbl_size); > + if (ACPI_FAILURE(status)) { > + const char *msg = acpi_format_exception(status); > + > + pr_err("Failed to get MCFG table, %s\n", msg); > + return 0; > + } > + > + if (table) > + mcfg = (struct acpi_table_mcfg *)table; > + > + entries = 0; > + i = table->length - sizeof(struct acpi_table_mcfg); > + while (i >= sizeof(struct acpi_mcfg_allocation)) { > + entries++; > + i -= sizeof(struct acpi_mcfg_allocation); > + } > + if (entries == 0) { > + pr_err("ACPI MCFG table has no entries\n"); > + goto out; > + } > + > + cfg_table = (struct acpi_mcfg_allocation *) &mcfg[1]; > + for (i = 0; i < entries; i++) { > + cfg = &cfg_table[i]; > + if (acpi_mcfg_check_entry(mcfg)) > + goto out; > + > + if (cfg->pci_segment == segment && > + cfg->start_bus_number <= bus && > + bus <= cfg->end_bus_number) { > + cba = (phys_addr_t)cfg->address; > + goto out; > + } > + } > +out: > + /* > + * acpi_get_table_with_size() creates MCFG table mapping that > + * should be released after parsing and before resuming boot > + */ > + early_acpi_os_unmap_memory(table, tbl_size); > + return cba; > +} > + > phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle) > { > acpi_status status = AE_NOT_EXIST; > - unsigned long long mcfg_addr; > + unsigned long long mcfg_addr, mcfg_seg, mcfg_bus; > + u16 seg; > + u8 bus; > + > + if (!handle) > + return 0; > > - if (handle) > - status = acpi_evaluate_integer(handle, METHOD_NAME__CBA, > + status = acpi_evaluate_integer(handle, METHOD_NAME__CBA, > NULL, &mcfg_addr); > + if (ACPI_SUCCESS(status)) > + return (phys_addr_t)mcfg_addr; > + > + pr_info("ACPI: Falling back to acpi mcfg table\n"); > + > + /* > + * Fall back to MCFG table if _CBA failed: > + * get the mcfg_addr via ACPI MCFG table. PCI Firmware v3.2 spec: 4.1.2 > + */ > + status = acpi_evaluate_integer(handle, METHOD_NAME__SEG, > + NULL, &mcfg_seg); > if (ACPI_FAILURE(status)) > - return 0; > + mcfg_seg = 0; > + > + status = acpi_evaluate_integer(handle, METHOD_NAME__BBN, > + NULL, &mcfg_bus); > + if (ACPI_FAILURE(status)) > + mcfg_bus = 0; > + > + seg = mcfg_seg & 0xFFFF; > + bus = mcfg_bus & 0xFF; > > - return (phys_addr_t)mcfg_addr; > + return acpi_get_mcfg_cba(seg, bus); > } > > static acpi_status decode_type0_hpx_record(union acpi_object *record, > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 28, 2015 at 03:39:43PM +0100, Lorenzo Pieralisi wrote: > Hi Dennis, > > You should CC linux-pci@vger.kernel.org and the PCI subsystem > maintainer next time. > > Good reminder! Thanks, mate ;-) > On Fri, Aug 28, 2015 at 01:49:23PM +0100, Dennis Chen wrote: > > This patch will fall back to ACPI MCFG table if _CBA method fails > > to get the configuration base address of ECAM. Firmware on ARM > > platform uses MCFG table instead of _CBA method. This is needed > > to scan the PCIe root complex for ARM SoC. > > Code to enumerate PCI with ACPI on ARM64 is under review: > > https://lkml.org/lkml/2015/6/8/443 > Oops,seems I am late, just go through the code to scan the root: https://lkml.org/lkml/2015/5/26/215 a little bit complicated, to be honest. Maybe I can have some input then. > Having said that, I do not think this patch will be needed, > you can't add code in the kernel because it may be needed in > the future, I do not see how it can be possibly useful at present > on ARM64 unless you pulled some patch dependencies; if that's the > case you should have listed them. > > This patch can't be review standalone since it has no use in > the current kernel (at least for ARM64, it should be tested > on x86 though). I do have a patch set to enumerate all the downstream devices of the PCIe root bridge. With this patch, I can focus on the enabling/fixing of the drivers of those devices. As you can imagine, the patch have some redundant codes with PCI/ACPI codes now under x86 directory. It's reasonable to move those arch-agnostic codes to a common place. I am OK to keep them pending as private as a test code base for me As for this patch, it's used to get the ecam address from MCFG instead of _CBA which x86 is using, see acpi_pci_root_get_mcfg_addr() code. So, it's for ARM64 and can be tested with uefi definitely. I'm missing some context here? > Thanks, > Lorenzo > > > Signed-off-by: Dennis Chen <dennis.chen@arm.com> > > --- > > drivers/pci/pci-acpi.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++--- > > 1 file changed, 97 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c > > index 314a625..211b9d9 100644 > > --- a/drivers/pci/pci-acpi.c > > +++ b/drivers/pci/pci-acpi.c > > @@ -27,18 +27,110 @@ const u8 pci_acpi_dsm_uuid[] = { > > 0x91, 0x17, 0xea, 0x4d, 0x19, 0xc3, 0x43, 0x4d > > }; > > > > +static int __init acpi_mcfg_check_entry(struct acpi_table_mcfg *mcfg) > > +{ > > + if (mcfg->header.revision != 1) { > > + pr_err("invalid header revision:%d in ACPI MCFG table\n", > > + mcfg->header.revision); > > + return -EINVAL; > > + } > > + > > + return 0; > > +} > > + > > +static phys_addr_t __init acpi_get_mcfg_cba(u16 segment, u8 bus) > > +{ > > + struct acpi_table_header *table = NULL; > > + acpi_size tbl_size; > > + struct acpi_table_mcfg *mcfg = NULL; > > + struct acpi_mcfg_allocation *cfg_table, *cfg; > > + acpi_status status; > > + phys_addr_t cba = 0; > > + unsigned long i; > > + int entries; > > + > > + if (acpi_disabled) > > + return 0; > > + > > + status = acpi_get_table_with_size(ACPI_SIG_MCFG, 0, &table, &tbl_size); > > + if (ACPI_FAILURE(status)) { > > + const char *msg = acpi_format_exception(status); > > + > > + pr_err("Failed to get MCFG table, %s\n", msg); > > + return 0; > > + } > > + > > + if (table) > > + mcfg = (struct acpi_table_mcfg *)table; > > + > > + entries = 0; > > + i = table->length - sizeof(struct acpi_table_mcfg); > > + while (i >= sizeof(struct acpi_mcfg_allocation)) { > > + entries++; > > + i -= sizeof(struct acpi_mcfg_allocation); > > + } > > + if (entries == 0) { > > + pr_err("ACPI MCFG table has no entries\n"); > > + goto out; > > + } > > + > > + cfg_table = (struct acpi_mcfg_allocation *) &mcfg[1]; > > + for (i = 0; i < entries; i++) { > > + cfg = &cfg_table[i]; > > + if (acpi_mcfg_check_entry(mcfg)) > > + goto out; > > + > > + if (cfg->pci_segment == segment && > > + cfg->start_bus_number <= bus && > > + bus <= cfg->end_bus_number) { > > + cba = (phys_addr_t)cfg->address; > > + goto out; > > + } > > + } > > +out: > > + /* > > + * acpi_get_table_with_size() creates MCFG table mapping that > > + * should be released after parsing and before resuming boot > > + */ > > + early_acpi_os_unmap_memory(table, tbl_size); > > + return cba; > > +} > > + > > phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle) > > { > > acpi_status status = AE_NOT_EXIST; > > - unsigned long long mcfg_addr; > > + unsigned long long mcfg_addr, mcfg_seg, mcfg_bus; > > + u16 seg; > > + u8 bus; > > + > > + if (!handle) > > + return 0; > > > > - if (handle) > > - status = acpi_evaluate_integer(handle, METHOD_NAME__CBA, > > + status = acpi_evaluate_integer(handle, METHOD_NAME__CBA, > > NULL, &mcfg_addr); > > + if (ACPI_SUCCESS(status)) > > + return (phys_addr_t)mcfg_addr; > > + > > + pr_info("ACPI: Falling back to acpi mcfg table\n"); > > + > > + /* > > + * Fall back to MCFG table if _CBA failed: > > + * get the mcfg_addr via ACPI MCFG table. PCI Firmware v3.2 spec: 4.1.2 > > + */ > > + status = acpi_evaluate_integer(handle, METHOD_NAME__SEG, > > + NULL, &mcfg_seg); > > if (ACPI_FAILURE(status)) > > - return 0; > > + mcfg_seg = 0; > > + > > + status = acpi_evaluate_integer(handle, METHOD_NAME__BBN, > > + NULL, &mcfg_bus); > > + if (ACPI_FAILURE(status)) > > + mcfg_bus = 0; > > + > > + seg = mcfg_seg & 0xFFFF; > > + bus = mcfg_bus & 0xFF; > > > > - return (phys_addr_t)mcfg_addr; > > + return acpi_get_mcfg_cba(seg, bus); > > } > > > > static acpi_status decode_type0_hpx_record(union acpi_object *record, > > -- > > 1.9.1 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ > > > Regards, Dennis -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 31, 2015 at 06:51:58AM +0100, Dennis Chen wrote: > On Fri, Aug 28, 2015 at 03:39:43PM +0100, Lorenzo Pieralisi wrote: > > Hi Dennis, > > > > You should CC linux-pci@vger.kernel.org and the PCI subsystem > > maintainer next time. > > > > > > Good reminder! Thanks, mate ;-) > > > On Fri, Aug 28, 2015 at 01:49:23PM +0100, Dennis Chen wrote: > > > This patch will fall back to ACPI MCFG table if _CBA method fails > > > to get the configuration base address of ECAM. Firmware on ARM > > > platform uses MCFG table instead of _CBA method. This is needed > > > to scan the PCIe root complex for ARM SoC. > > > > Code to enumerate PCI with ACPI on ARM64 is under review: > > > > https://lkml.org/lkml/2015/6/8/443 > > > > Oops,seems I am late, just go through the code to scan the root: > https://lkml.org/lkml/2015/5/26/215 > a little bit complicated, to be honest. Maybe I can have some input then. > > > Having said that, I do not think this patch will be needed, > > you can't add code in the kernel because it may be needed in > > the future, I do not see how it can be possibly useful at present > > on ARM64 unless you pulled some patch dependencies; if that's the > > case you should have listed them. > > > > This patch can't be review standalone since it has no use in > > the current kernel (at least for ARM64, it should be tested > > on x86 though). > > I do have a patch set to enumerate all the downstream devices of > the PCIe root bridge. With this patch, I can focus on the enabling/fixing > of the drivers of those devices. As you can imagine, the patch have some > redundant codes with PCI/ACPI codes now under x86 directory. It's reasonable > to move those arch-agnostic codes to a common place. I am OK to keep > them pending as private as a test code base for me > > As for this patch, it's used to get the ecam address from MCFG instead of > _CBA which x86 is using, see acpi_pci_root_get_mcfg_addr() code. > So, it's for ARM64 and can be tested with uefi definitely. I'm missing some > context here? No, but as I said this patch is no use standalone (and again it has to be tested on x86), there is no reason for it to go upstream given the patch dependencies you mentioned. When the dust settles on the ACPI/PCI enablement we will have a look at this patch and see if it is still needed, there is no point in merging it as-is, it serves no purpose at present. Lorenzo -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index 314a625..211b9d9 100644 --- a/drivers/pci/pci-acpi.c +++ b/drivers/pci/pci-acpi.c @@ -27,18 +27,110 @@ const u8 pci_acpi_dsm_uuid[] = { 0x91, 0x17, 0xea, 0x4d, 0x19, 0xc3, 0x43, 0x4d }; +static int __init acpi_mcfg_check_entry(struct acpi_table_mcfg *mcfg) +{ + if (mcfg->header.revision != 1) { + pr_err("invalid header revision:%d in ACPI MCFG table\n", + mcfg->header.revision); + return -EINVAL; + } + + return 0; +} + +static phys_addr_t __init acpi_get_mcfg_cba(u16 segment, u8 bus) +{ + struct acpi_table_header *table = NULL; + acpi_size tbl_size; + struct acpi_table_mcfg *mcfg = NULL; + struct acpi_mcfg_allocation *cfg_table, *cfg; + acpi_status status; + phys_addr_t cba = 0; + unsigned long i; + int entries; + + if (acpi_disabled) + return 0; + + status = acpi_get_table_with_size(ACPI_SIG_MCFG, 0, &table, &tbl_size); + if (ACPI_FAILURE(status)) { + const char *msg = acpi_format_exception(status); + + pr_err("Failed to get MCFG table, %s\n", msg); + return 0; + } + + if (table) + mcfg = (struct acpi_table_mcfg *)table; + + entries = 0; + i = table->length - sizeof(struct acpi_table_mcfg); + while (i >= sizeof(struct acpi_mcfg_allocation)) { + entries++; + i -= sizeof(struct acpi_mcfg_allocation); + } + if (entries == 0) { + pr_err("ACPI MCFG table has no entries\n"); + goto out; + } + + cfg_table = (struct acpi_mcfg_allocation *) &mcfg[1]; + for (i = 0; i < entries; i++) { + cfg = &cfg_table[i]; + if (acpi_mcfg_check_entry(mcfg)) + goto out; + + if (cfg->pci_segment == segment && + cfg->start_bus_number <= bus && + bus <= cfg->end_bus_number) { + cba = (phys_addr_t)cfg->address; + goto out; + } + } +out: + /* + * acpi_get_table_with_size() creates MCFG table mapping that + * should be released after parsing and before resuming boot + */ + early_acpi_os_unmap_memory(table, tbl_size); + return cba; +} + phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle) { acpi_status status = AE_NOT_EXIST; - unsigned long long mcfg_addr; + unsigned long long mcfg_addr, mcfg_seg, mcfg_bus; + u16 seg; + u8 bus; + + if (!handle) + return 0; - if (handle) - status = acpi_evaluate_integer(handle, METHOD_NAME__CBA, + status = acpi_evaluate_integer(handle, METHOD_NAME__CBA, NULL, &mcfg_addr); + if (ACPI_SUCCESS(status)) + return (phys_addr_t)mcfg_addr; + + pr_info("ACPI: Falling back to acpi mcfg table\n"); + + /* + * Fall back to MCFG table if _CBA failed: + * get the mcfg_addr via ACPI MCFG table. PCI Firmware v3.2 spec: 4.1.2 + */ + status = acpi_evaluate_integer(handle, METHOD_NAME__SEG, + NULL, &mcfg_seg); if (ACPI_FAILURE(status)) - return 0; + mcfg_seg = 0; + + status = acpi_evaluate_integer(handle, METHOD_NAME__BBN, + NULL, &mcfg_bus); + if (ACPI_FAILURE(status)) + mcfg_bus = 0; + + seg = mcfg_seg & 0xFFFF; + bus = mcfg_bus & 0xFF; - return (phys_addr_t)mcfg_addr; + return acpi_get_mcfg_cba(seg, bus); } static acpi_status decode_type0_hpx_record(union acpi_object *record,
This patch will fall back to ACPI MCFG table if _CBA method fails to get the configuration base address of ECAM. Firmware on ARM platform uses MCFG table instead of _CBA method. This is needed to scan the PCIe root complex for ARM SoC. Signed-off-by: Dennis Chen <dennis.chen@arm.com> --- drivers/pci/pci-acpi.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 97 insertions(+), 5 deletions(-)