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

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

[Dennis Chen]

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

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

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

[Lorenzo Pieralisi]

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/
> 

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

[Dennis Chen]

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

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

[Lorenzo Pieralisi]

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