2015-08-28 12:49:57

by Dennis Chen

[permalink] [raw]
Subject: [PATCH] ACPI / ARM64: Get configuration base address of ECAM via ACPI MCFG table

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 <[email protected]>
---
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


2015-08-28 14:39:43

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH] ACPI / ARM64: Get configuration base address of ECAM via ACPI MCFG table

Hi Dennis,

You should CC [email protected] 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 <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2015-08-31 05:51:53

by Dennis Chen

[permalink] [raw]
Subject: Re: [PATCH] ACPI / ARM64: Get configuration base address of ECAM via ACPI MCFG table

On Fri, Aug 28, 2015 at 03:39:43PM +0100, Lorenzo Pieralisi wrote:
> Hi Dennis,
>
> You should CC [email protected] 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 <[email protected]>
> > ---
> > 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 [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> >
>

Regards,
Dennis