[PATCH 1/2] arch: x86: lib: acpi_table: Fix MCFG entries

[PATCH 1/2] arch: x86: lib: acpi_table: Fix MCFG entries

[Moritz Fischer]

Commit d953137526cc ("x86: Move SSDT table to a writer function")
introduced a bug where the actual MCFG entries are no longer generated.

Cc: Simon Glass <sjg@chromium.org>
Fixes: d953137526cc ("x86: Move SSDT table to a writer function")
Signed-off-by: Moritz Fischer <moritzf@google.com>
---

Hi Simon, Andriy,

it looks like this got dropped when moving stuff around, I don't have HW
to test this, but it seemed off.

- Moritz

---

 arch/x86/lib/acpi_table.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
index c0534343f1..753bf39619 100644
--- a/arch/x86/lib/acpi_table.c
+++ b/arch/x86/lib/acpi_table.c
@@ -499,6 +499,8 @@ int acpi_write_mcfg(struct acpi_ctx *ctx, const struct acpi_writer *entry)
 	header->length = sizeof(struct acpi_mcfg);
 	header->revision = 1;
 
+	current = acpi_fill_mcfg(current);
+
 	/* (Re)calculate length and checksum */
 	header->length = current - (u32)mcfg;
 	header->checksum = table_compute_checksum(mcfg, header->length);
-- 
2.35.0.263.gb82422642f-goog

[PATCH 2/2] acpi: Move MCFG implementation to common lib

[Moritz Fischer]

MCFG tables are used on multiple arches. Move to common ACPI lib.

Cc: Simon Glass <sjg@chromium.org>
Signed-off-by: Moritz Fischer <moritzf@google.com>
---

 arch/x86/cpu/intel_common/acpi.c  | 15 +++++---
 arch/x86/cpu/tangier/acpi.c       | 11 ++++--
 arch/x86/include/asm/acpi_table.h |  1 -
 arch/x86/lib/acpi_table.c         | 54 --------------------------
 lib/acpi/Makefile                 |  1 +
 lib/acpi/mcfg.c                   | 64 +++++++++++++++++++++++++++++++
 6 files changed, 81 insertions(+), 65 deletions(-)
 create mode 100644 lib/acpi/mcfg.c

diff --git a/arch/x86/cpu/intel_common/acpi.c b/arch/x86/cpu/intel_common/acpi.c
index 15f19da206..d94ec208f6 100644
--- a/arch/x86/cpu/intel_common/acpi.c
+++ b/arch/x86/cpu/intel_common/acpi.c
@@ -31,14 +31,17 @@
 #include <linux/err.h>
 #include <power/acpi_pmc.h>
 
-u32 acpi_fill_mcfg(u32 current)
+int acpi_fill_mcfg(struct acpi_ctx *ctx)
 {
+	size_t size;
+
 	/* PCI Segment Group 0, Start Bus Number 0, End Bus Number is 255 */
-	current += acpi_create_mcfg_mmconfig((void *)current,
-					     CONFIG_MMCONF_BASE_ADDRESS, 0, 0,
-					     (CONFIG_SA_PCIEX_LENGTH >> 20)
-					     - 1);
-	return current;
+	size = acpi_create_mcfg_mmconfig((void *)ctx->current,
+					 CONFIG_MMCONF_BASE_ADDRESS, 0, 0,
+					 (CONFIG_SA_PCIEX_LENGTH >> 20) - 1);
+	acpi_inc(ctx, size);
+
+	return 0;
 }
 
 static int acpi_sci_irq(void)
diff --git a/arch/x86/cpu/tangier/acpi.c b/arch/x86/cpu/tangier/acpi.c
index 12f9289612..e3a2fcea76 100644
--- a/arch/x86/cpu/tangier/acpi.c
+++ b/arch/x86/cpu/tangier/acpi.c
@@ -68,14 +68,17 @@ u32 acpi_fill_madt(u32 current)
 	return current;
 }
 
-u32 acpi_fill_mcfg(u32 current)
+int acpi_fill_mcfg(struct acpi_ctx *ctx)
 {
+	size_t size;
+
 	/* TODO: Derive parameters from SFI MCFG table */
-	current += acpi_create_mcfg_mmconfig
-		((struct acpi_mcfg_mmconfig *)current,
+	size = acpi_create_mcfg_mmconfig
+		((struct acpi_mcfg_mmconfig *)ctx->current,
 		MCFG_BASE_ADDRESS, 0x0, 0x0, 0x0);
+	acpi_inc(ctx, size);
 
-	return current;
+	return 0;
 }
 
 static u32 acpi_fill_csrt_dma(struct acpi_csrt_group *grp)
diff --git a/arch/x86/include/asm/acpi_table.h b/arch/x86/include/asm/acpi_table.h
index 0d07f7cad8..39547de0d4 100644
--- a/arch/x86/include/asm/acpi_table.h
+++ b/arch/x86/include/asm/acpi_table.h
@@ -34,7 +34,6 @@ int acpi_create_madt_lapic_nmi(struct acpi_madt_lapic_nmi *lapic_nmi,
 u32 acpi_fill_madt(u32 current);
 int acpi_create_mcfg_mmconfig(struct acpi_mcfg_mmconfig *mmconfig, u32 base,
 			      u16 seg_nr, u8 start, u8 end);
-u32 acpi_fill_mcfg(u32 current);
 
 /**
  * acpi_write_hpet() - Write out a HPET table
diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
index 753bf39619..c5b33dc65d 100644
--- a/arch/x86/lib/acpi_table.c
+++ b/arch/x86/lib/acpi_table.c
@@ -161,28 +161,6 @@ int acpi_write_madt(struct acpi_ctx *ctx, const struct acpi_writer *entry)
 }
 ACPI_WRITER(5x86, NULL, acpi_write_madt, 0);
 
-int acpi_create_mcfg_mmconfig(struct acpi_mcfg_mmconfig *mmconfig, u32 base,
-			      u16 seg_nr, u8 start, u8 end)
-{
-	memset(mmconfig, 0, sizeof(*mmconfig));
-	mmconfig->base_address_l = base;
-	mmconfig->base_address_h = 0;
-	mmconfig->pci_segment_group_number = seg_nr;
-	mmconfig->start_bus_number = start;
-	mmconfig->end_bus_number = end;
-
-	return sizeof(struct acpi_mcfg_mmconfig);
-}
-
-__weak u32 acpi_fill_mcfg(u32 current)
-{
-	current += acpi_create_mcfg_mmconfig
-		((struct acpi_mcfg_mmconfig *)current,
-		CONFIG_PCIE_ECAM_BASE, 0x0, 0x0, 255);
-
-	return current;
-}
-
 /**
  * acpi_create_tcpa() - Create a TCPA table
  *
@@ -480,38 +458,6 @@ int acpi_write_gnvs(struct acpi_ctx *ctx, const struct acpi_writer *entry)
 }
 ACPI_WRITER(4gnvs, "GNVS", acpi_write_gnvs, 0);
 
-/* MCFG is defined in the PCI Firmware Specification 3.0 */
-int acpi_write_mcfg(struct acpi_ctx *ctx, const struct acpi_writer *entry)
-{
-	struct acpi_table_header *header;
-	struct acpi_mcfg *mcfg;
-	u32 current;
-
-	mcfg = ctx->current;
-	header = &mcfg->header;
-
-	current = (u32)mcfg + sizeof(struct acpi_mcfg);
-
-	memset(mcfg, '\0', sizeof(struct acpi_mcfg));
-
-	/* Fill out header fields */
-	acpi_fill_header(header, "MCFG");
-	header->length = sizeof(struct acpi_mcfg);
-	header->revision = 1;
-
-	current = acpi_fill_mcfg(current);
-
-	/* (Re)calculate length and checksum */
-	header->length = current - (u32)mcfg;
-	header->checksum = table_compute_checksum(mcfg, header->length);
-
-	acpi_inc(ctx, mcfg->header.length);
-	acpi_add_table(ctx, mcfg);
-
-	return 0;
-}
-ACPI_WRITER(5mcfg, "MCFG", acpi_write_mcfg, 0);
-
 /**
  * acpi_write_hpet() - Write out a HPET table
  *
diff --git a/lib/acpi/Makefile b/lib/acpi/Makefile
index f9b504988f..956b5a0d72 100644
--- a/lib/acpi/Makefile
+++ b/lib/acpi/Makefile
@@ -11,6 +11,7 @@ obj-y += acpi_writer.o
 ifndef CONFIG_QEMU
 obj-y += base.o
 obj-y += csrt.o
+obj-y += mcfg.o
 
 # Sandbox does not build a .asl file
 ifndef CONFIG_SANDBOX
diff --git a/lib/acpi/mcfg.c b/lib/acpi/mcfg.c
new file mode 100644
index 0000000000..2231e25124
--- /dev/null
+++ b/lib/acpi/mcfg.c
@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Write an ACPI MCFG table
+ *
+ * Copyright 2022 Google LLC
+ */
+
+#define LOG_CATEGORY LOGC_ACPI
+
+#include <common.h>
+#include <mapmem.h>
+#include <tables_csum.h>
+#include <acpi/acpi_table.h>
+#include <dm/acpi.h>
+
+int acpi_create_mcfg_mmconfig(struct acpi_mcfg_mmconfig *mmconfig, u32 base,
+			      u16 seg_nr, u8 start, u8 end)
+{
+	memset(mmconfig, 0, sizeof(*mmconfig));
+	mmconfig->base_address_l = base;
+	mmconfig->base_address_h = 0;
+	mmconfig->pci_segment_group_number = seg_nr;
+	mmconfig->start_bus_number = start;
+	mmconfig->end_bus_number = end;
+
+	return sizeof(struct acpi_mcfg_mmconfig);
+}
+
+__weak int acpi_fill_mcfg(struct acpi_ctx *ctx)
+{
+	return -ENOENT;
+}
+
+/* MCFG is defined in the PCI Firmware Specification 3.0 */
+int acpi_write_mcfg(struct acpi_ctx *ctx, const struct acpi_writer *entry)
+{
+	struct acpi_table_header *header;
+	struct acpi_mcfg *mcfg;
+	int ret;
+
+	mcfg = ctx->current;
+	header = &mcfg->header;
+
+	memset(mcfg, '\0', sizeof(struct acpi_mcfg));
+
+	/* Fill out header fields */
+	acpi_fill_header(header, "MCFG");
+	header->length = sizeof(struct acpi_mcfg);
+	header->revision = 1;
+	acpi_inc(ctx, sizeof(*header));
+
+	ret = acpi_fill_mcfg(ctx);
+	if (ret)
+		return log_msg_ret("fill", ret);
+
+	/* (Re)calculate length and checksum */
+	header->length = (ulong)ctx->current - (ulong)mcfg;
+	header->checksum = table_compute_checksum(mcfg, header->length);
+
+	acpi_add_table(ctx, mcfg);
+
+	return 0;
+}
+ACPI_WRITER(5mcfg, "MCFG", acpi_write_mcfg, 0);
-- 
2.35.0.263.gb82422642f-goog

Re: [PATCH 2/2] acpi: Move MCFG implementation to common lib

[Simon Glass]

Hi Moritz,

On Sat, 5 Feb 2022 at 13:17, Moritz Fischer <moritzf@google.com> wrote:
>
> MCFG tables are used on multiple arches. Move to common ACPI lib.
>
> Cc: Simon Glass <sjg@chromium.org>
> Signed-off-by: Moritz Fischer <moritzf@google.com>
> ---
>
>  arch/x86/cpu/intel_common/acpi.c  | 15 +++++---
>  arch/x86/cpu/tangier/acpi.c       | 11 ++++--
>  arch/x86/include/asm/acpi_table.h |  1 -
>  arch/x86/lib/acpi_table.c         | 54 --------------------------
>  lib/acpi/Makefile                 |  1 +
>  lib/acpi/mcfg.c                   | 64 +++++++++++++++++++++++++++++++
>  6 files changed, 81 insertions(+), 65 deletions(-)
>  create mode 100644 lib/acpi/mcfg.c
>
> diff --git a/arch/x86/cpu/intel_common/acpi.c b/arch/x86/cpu/intel_common/acpi.c
> index 15f19da206..d94ec208f6 100644
> --- a/arch/x86/cpu/intel_common/acpi.c
> +++ b/arch/x86/cpu/intel_common/acpi.c
> @@ -31,14 +31,17 @@
>  #include <linux/err.h>
>  #include <power/acpi_pmc.h>
>
> -u32 acpi_fill_mcfg(u32 current)
> +int acpi_fill_mcfg(struct acpi_ctx *ctx)
>  {
> +       size_t size;
> +
>         /* PCI Segment Group 0, Start Bus Number 0, End Bus Number is 255 */
> -       current += acpi_create_mcfg_mmconfig((void *)current,
> -                                            CONFIG_MMCONF_BASE_ADDRESS, 0, 0,
> -                                            (CONFIG_SA_PCIEX_LENGTH >> 20)
> -                                            - 1);
> -       return current;
> +       size = acpi_create_mcfg_mmconfig((void *)ctx->current,
> +                                        CONFIG_MMCONF_BASE_ADDRESS, 0, 0,
> +                                        (CONFIG_SA_PCIEX_LENGTH >> 20) - 1);
> +       acpi_inc(ctx, size);
> +
> +       return 0;
>  }
>
>  static int acpi_sci_irq(void)
> diff --git a/arch/x86/cpu/tangier/acpi.c b/arch/x86/cpu/tangier/acpi.c
> index 12f9289612..e3a2fcea76 100644
> --- a/arch/x86/cpu/tangier/acpi.c
> +++ b/arch/x86/cpu/tangier/acpi.c
> @@ -68,14 +68,17 @@ u32 acpi_fill_madt(u32 current)
>         return current;
>  }
>
> -u32 acpi_fill_mcfg(u32 current)
> +int acpi_fill_mcfg(struct acpi_ctx *ctx)
>  {
> +       size_t size;
> +
>         /* TODO: Derive parameters from SFI MCFG table */
> -       current += acpi_create_mcfg_mmconfig
> -               ((struct acpi_mcfg_mmconfig *)current,
> +       size = acpi_create_mcfg_mmconfig
> +               ((struct acpi_mcfg_mmconfig *)ctx->current,
>                 MCFG_BASE_ADDRESS, 0x0, 0x0, 0x0);
> +       acpi_inc(ctx, size);
>
> -       return current;
> +       return 0;
>  }
>
>  static u32 acpi_fill_csrt_dma(struct acpi_csrt_group *grp)
> diff --git a/arch/x86/include/asm/acpi_table.h b/arch/x86/include/asm/acpi_table.h
> index 0d07f7cad8..39547de0d4 100644
> --- a/arch/x86/include/asm/acpi_table.h
> +++ b/arch/x86/include/asm/acpi_table.h
> @@ -34,7 +34,6 @@ int acpi_create_madt_lapic_nmi(struct acpi_madt_lapic_nmi *lapic_nmi,
>  u32 acpi_fill_madt(u32 current);
>  int acpi_create_mcfg_mmconfig(struct acpi_mcfg_mmconfig *mmconfig, u32 base,
>                               u16 seg_nr, u8 start, u8 end);
> -u32 acpi_fill_mcfg(u32 current);
>
>  /**
>   * acpi_write_hpet() - Write out a HPET table
> diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> index 753bf39619..c5b33dc65d 100644
> --- a/arch/x86/lib/acpi_table.c
> +++ b/arch/x86/lib/acpi_table.c
> @@ -161,28 +161,6 @@ int acpi_write_madt(struct acpi_ctx *ctx, const struct acpi_writer *entry)
>  }
>  ACPI_WRITER(5x86, NULL, acpi_write_madt, 0);
>
> -int acpi_create_mcfg_mmconfig(struct acpi_mcfg_mmconfig *mmconfig, u32 base,
> -                             u16 seg_nr, u8 start, u8 end)
> -{
> -       memset(mmconfig, 0, sizeof(*mmconfig));
> -       mmconfig->base_address_l = base;
> -       mmconfig->base_address_h = 0;
> -       mmconfig->pci_segment_group_number = seg_nr;
> -       mmconfig->start_bus_number = start;
> -       mmconfig->end_bus_number = end;
> -
> -       return sizeof(struct acpi_mcfg_mmconfig);
> -}
> -
> -__weak u32 acpi_fill_mcfg(u32 current)
> -{
> -       current += acpi_create_mcfg_mmconfig
> -               ((struct acpi_mcfg_mmconfig *)current,
> -               CONFIG_PCIE_ECAM_BASE, 0x0, 0x0, 255);
> -
> -       return current;
> -}
> -
>  /**
>   * acpi_create_tcpa() - Create a TCPA table
>   *
> @@ -480,38 +458,6 @@ int acpi_write_gnvs(struct acpi_ctx *ctx, const struct acpi_writer *entry)
>  }
>  ACPI_WRITER(4gnvs, "GNVS", acpi_write_gnvs, 0);
>
> -/* MCFG is defined in the PCI Firmware Specification 3.0 */
> -int acpi_write_mcfg(struct acpi_ctx *ctx, const struct acpi_writer *entry)
> -{
> -       struct acpi_table_header *header;
> -       struct acpi_mcfg *mcfg;
> -       u32 current;
> -
> -       mcfg = ctx->current;
> -       header = &mcfg->header;
> -
> -       current = (u32)mcfg + sizeof(struct acpi_mcfg);
> -
> -       memset(mcfg, '\0', sizeof(struct acpi_mcfg));
> -
> -       /* Fill out header fields */
> -       acpi_fill_header(header, "MCFG");
> -       header->length = sizeof(struct acpi_mcfg);
> -       header->revision = 1;
> -
> -       current = acpi_fill_mcfg(current);
> -
> -       /* (Re)calculate length and checksum */
> -       header->length = current - (u32)mcfg;
> -       header->checksum = table_compute_checksum(mcfg, header->length);
> -
> -       acpi_inc(ctx, mcfg->header.length);
> -       acpi_add_table(ctx, mcfg);
> -
> -       return 0;
> -}
> -ACPI_WRITER(5mcfg, "MCFG", acpi_write_mcfg, 0);
> -
>  /**
>   * acpi_write_hpet() - Write out a HPET table
>   *
> diff --git a/lib/acpi/Makefile b/lib/acpi/Makefile
> index f9b504988f..956b5a0d72 100644
> --- a/lib/acpi/Makefile
> +++ b/lib/acpi/Makefile
> @@ -11,6 +11,7 @@ obj-y += acpi_writer.o
>  ifndef CONFIG_QEMU
>  obj-y += base.o
>  obj-y += csrt.o
> +obj-y += mcfg.o
>
>  # Sandbox does not build a .asl file
>  ifndef CONFIG_SANDBOX
> diff --git a/lib/acpi/mcfg.c b/lib/acpi/mcfg.c
> new file mode 100644
> index 0000000000..2231e25124
> --- /dev/null
> +++ b/lib/acpi/mcfg.c
> @@ -0,0 +1,64 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Write an ACPI MCFG table
> + *
> + * Copyright 2022 Google LLC
> + */
> +
> +#define LOG_CATEGORY LOGC_ACPI
> +
> +#include <common.h>
> +#include <mapmem.h>
> +#include <tables_csum.h>
> +#include <acpi/acpi_table.h>
> +#include <dm/acpi.h>
> +
> +int acpi_create_mcfg_mmconfig(struct acpi_mcfg_mmconfig *mmconfig, u32 base,
> +                             u16 seg_nr, u8 start, u8 end)
> +{
> +       memset(mmconfig, 0, sizeof(*mmconfig));
> +       mmconfig->base_address_l = base;
> +       mmconfig->base_address_h = 0;
> +       mmconfig->pci_segment_group_number = seg_nr;
> +       mmconfig->start_bus_number = start;
> +       mmconfig->end_bus_number = end;
> +
> +       return sizeof(struct acpi_mcfg_mmconfig);
> +}
> +
> +__weak int acpi_fill_mcfg(struct acpi_ctx *ctx)
> +{
> +       return -ENOENT;
> +}
> +
> +/* MCFG is defined in the PCI Firmware Specification 3.0 */
> +int acpi_write_mcfg(struct acpi_ctx *ctx, const struct acpi_writer *entry)
> +{
> +       struct acpi_table_header *header;
> +       struct acpi_mcfg *mcfg;
> +       int ret;
> +
> +       mcfg = ctx->current;
> +       header = &mcfg->header;
> +
> +       memset(mcfg, '\0', sizeof(struct acpi_mcfg));
> +
> +       /* Fill out header fields */
> +       acpi_fill_header(header, "MCFG");
> +       header->length = sizeof(struct acpi_mcfg);
> +       header->revision = 1;
> +       acpi_inc(ctx, sizeof(*header));

sizeof(*mcfg)

I can fix when applying if you agre?

> +
> +       ret = acpi_fill_mcfg(ctx);
> +       if (ret)
> +               return log_msg_ret("fill", ret);
> +
> +       /* (Re)calculate length and checksum */
> +       header->length = (ulong)ctx->current - (ulong)mcfg;
> +       header->checksum = table_compute_checksum(mcfg, header->length);
> +
> +       acpi_add_table(ctx, mcfg);
> +
> +       return 0;
> +}
> +ACPI_WRITER(5mcfg, "MCFG", acpi_write_mcfg, 0);
> --
> 2.35.0.263.gb82422642f-goog
>

Regards,
Simon

Re: [PATCH 2/2] acpi: Move MCFG implementation to common lib

[Moritz Fischer]

Hi Simon,

On Mon, Feb 7, 2022 at 12:22 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Moritz,
>
> On Sat, 5 Feb 2022 at 13:17, Moritz Fischer <moritzf@google.com> wrote:
> >
> > MCFG tables are used on multiple arches. Move to common ACPI lib.
> >
> > Cc: Simon Glass <sjg@chromium.org>
> > Signed-off-by: Moritz Fischer <moritzf@google.com>
> > ---
> >
> >  arch/x86/cpu/intel_common/acpi.c  | 15 +++++---
> >  arch/x86/cpu/tangier/acpi.c       | 11 ++++--
> >  arch/x86/include/asm/acpi_table.h |  1 -
> >  arch/x86/lib/acpi_table.c         | 54 --------------------------
> >  lib/acpi/Makefile                 |  1 +
> >  lib/acpi/mcfg.c                   | 64 +++++++++++++++++++++++++++++++
> >  6 files changed, 81 insertions(+), 65 deletions(-)
> >  create mode 100644 lib/acpi/mcfg.c
> >
> > diff --git a/arch/x86/cpu/intel_common/acpi.c b/arch/x86/cpu/intel_common/acpi.c
> > index 15f19da206..d94ec208f6 100644
> > --- a/arch/x86/cpu/intel_common/acpi.c
> > +++ b/arch/x86/cpu/intel_common/acpi.c
> > @@ -31,14 +31,17 @@
> >  #include <linux/err.h>
> >  #include <power/acpi_pmc.h>
> >
> > -u32 acpi_fill_mcfg(u32 current)
> > +int acpi_fill_mcfg(struct acpi_ctx *ctx)
> >  {
> > +       size_t size;
> > +
> >         /* PCI Segment Group 0, Start Bus Number 0, End Bus Number is 255 */
> > -       current += acpi_create_mcfg_mmconfig((void *)current,
> > -                                            CONFIG_MMCONF_BASE_ADDRESS, 0, 0,
> > -                                            (CONFIG_SA_PCIEX_LENGTH >> 20)
> > -                                            - 1);
> > -       return current;
> > +       size = acpi_create_mcfg_mmconfig((void *)ctx->current,
> > +                                        CONFIG_MMCONF_BASE_ADDRESS, 0, 0,
> > +                                        (CONFIG_SA_PCIEX_LENGTH >> 20) - 1);
> > +       acpi_inc(ctx, size);
> > +
> > +       return 0;
> >  }
> >
> >  static int acpi_sci_irq(void)
> > diff --git a/arch/x86/cpu/tangier/acpi.c b/arch/x86/cpu/tangier/acpi.c
> > index 12f9289612..e3a2fcea76 100644
> > --- a/arch/x86/cpu/tangier/acpi.c
> > +++ b/arch/x86/cpu/tangier/acpi.c
> > @@ -68,14 +68,17 @@ u32 acpi_fill_madt(u32 current)
> >         return current;
> >  }
> >
> > -u32 acpi_fill_mcfg(u32 current)
> > +int acpi_fill_mcfg(struct acpi_ctx *ctx)
> >  {
> > +       size_t size;
> > +
> >         /* TODO: Derive parameters from SFI MCFG table */
> > -       current += acpi_create_mcfg_mmconfig
> > -               ((struct acpi_mcfg_mmconfig *)current,
> > +       size = acpi_create_mcfg_mmconfig
> > +               ((struct acpi_mcfg_mmconfig *)ctx->current,
> >                 MCFG_BASE_ADDRESS, 0x0, 0x0, 0x0);
> > +       acpi_inc(ctx, size);
> >
> > -       return current;
> > +       return 0;
> >  }
> >
> >  static u32 acpi_fill_csrt_dma(struct acpi_csrt_group *grp)
> > diff --git a/arch/x86/include/asm/acpi_table.h b/arch/x86/include/asm/acpi_table.h
> > index 0d07f7cad8..39547de0d4 100644
> > --- a/arch/x86/include/asm/acpi_table.h
> > +++ b/arch/x86/include/asm/acpi_table.h
> > @@ -34,7 +34,6 @@ int acpi_create_madt_lapic_nmi(struct acpi_madt_lapic_nmi *lapic_nmi,
> >  u32 acpi_fill_madt(u32 current);
> >  int acpi_create_mcfg_mmconfig(struct acpi_mcfg_mmconfig *mmconfig, u32 base,
> >                               u16 seg_nr, u8 start, u8 end);
> > -u32 acpi_fill_mcfg(u32 current);
> >
> >  /**
> >   * acpi_write_hpet() - Write out a HPET table
> > diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> > index 753bf39619..c5b33dc65d 100644
> > --- a/arch/x86/lib/acpi_table.c
> > +++ b/arch/x86/lib/acpi_table.c
> > @@ -161,28 +161,6 @@ int acpi_write_madt(struct acpi_ctx *ctx, const struct acpi_writer *entry)
> >  }
> >  ACPI_WRITER(5x86, NULL, acpi_write_madt, 0);
> >
> > -int acpi_create_mcfg_mmconfig(struct acpi_mcfg_mmconfig *mmconfig, u32 base,
> > -                             u16 seg_nr, u8 start, u8 end)
> > -{
> > -       memset(mmconfig, 0, sizeof(*mmconfig));
> > -       mmconfig->base_address_l = base;
> > -       mmconfig->base_address_h = 0;
> > -       mmconfig->pci_segment_group_number = seg_nr;
> > -       mmconfig->start_bus_number = start;
> > -       mmconfig->end_bus_number = end;
> > -
> > -       return sizeof(struct acpi_mcfg_mmconfig);
> > -}
> > -
> > -__weak u32 acpi_fill_mcfg(u32 current)
> > -{
> > -       current += acpi_create_mcfg_mmconfig
> > -               ((struct acpi_mcfg_mmconfig *)current,
> > -               CONFIG_PCIE_ECAM_BASE, 0x0, 0x0, 255);
> > -
> > -       return current;
> > -}
> > -
> >  /**
> >   * acpi_create_tcpa() - Create a TCPA table
> >   *
> > @@ -480,38 +458,6 @@ int acpi_write_gnvs(struct acpi_ctx *ctx, const struct acpi_writer *entry)
> >  }
> >  ACPI_WRITER(4gnvs, "GNVS", acpi_write_gnvs, 0);
> >
> > -/* MCFG is defined in the PCI Firmware Specification 3.0 */
> > -int acpi_write_mcfg(struct acpi_ctx *ctx, const struct acpi_writer *entry)
> > -{
> > -       struct acpi_table_header *header;
> > -       struct acpi_mcfg *mcfg;
> > -       u32 current;
> > -
> > -       mcfg = ctx->current;
> > -       header = &mcfg->header;
> > -
> > -       current = (u32)mcfg + sizeof(struct acpi_mcfg);
> > -
> > -       memset(mcfg, '\0', sizeof(struct acpi_mcfg));
> > -
> > -       /* Fill out header fields */
> > -       acpi_fill_header(header, "MCFG");
> > -       header->length = sizeof(struct acpi_mcfg);
> > -       header->revision = 1;
> > -
> > -       current = acpi_fill_mcfg(current);
> > -
> > -       /* (Re)calculate length and checksum */
> > -       header->length = current - (u32)mcfg;
> > -       header->checksum = table_compute_checksum(mcfg, header->length);
> > -
> > -       acpi_inc(ctx, mcfg->header.length);
> > -       acpi_add_table(ctx, mcfg);
> > -
> > -       return 0;
> > -}
> > -ACPI_WRITER(5mcfg, "MCFG", acpi_write_mcfg, 0);
> > -
> >  /**
> >   * acpi_write_hpet() - Write out a HPET table
> >   *
> > diff --git a/lib/acpi/Makefile b/lib/acpi/Makefile
> > index f9b504988f..956b5a0d72 100644
> > --- a/lib/acpi/Makefile
> > +++ b/lib/acpi/Makefile
> > @@ -11,6 +11,7 @@ obj-y += acpi_writer.o
> >  ifndef CONFIG_QEMU
> >  obj-y += base.o
> >  obj-y += csrt.o
> > +obj-y += mcfg.o
> >
> >  # Sandbox does not build a .asl file
> >  ifndef CONFIG_SANDBOX
> > diff --git a/lib/acpi/mcfg.c b/lib/acpi/mcfg.c
> > new file mode 100644
> > index 0000000000..2231e25124
> > --- /dev/null
> > +++ b/lib/acpi/mcfg.c
> > @@ -0,0 +1,64 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Write an ACPI MCFG table
> > + *
> > + * Copyright 2022 Google LLC
> > + */
> > +
> > +#define LOG_CATEGORY LOGC_ACPI
> > +
> > +#include <common.h>
> > +#include <mapmem.h>
> > +#include <tables_csum.h>
> > +#include <acpi/acpi_table.h>
> > +#include <dm/acpi.h>
> > +
> > +int acpi_create_mcfg_mmconfig(struct acpi_mcfg_mmconfig *mmconfig, u32 base,
> > +                             u16 seg_nr, u8 start, u8 end)
> > +{
> > +       memset(mmconfig, 0, sizeof(*mmconfig));
> > +       mmconfig->base_address_l = base;
> > +       mmconfig->base_address_h = 0;
> > +       mmconfig->pci_segment_group_number = seg_nr;
> > +       mmconfig->start_bus_number = start;
> > +       mmconfig->end_bus_number = end;
> > +
> > +       return sizeof(struct acpi_mcfg_mmconfig);
> > +}
> > +
> > +__weak int acpi_fill_mcfg(struct acpi_ctx *ctx)
> > +{
> > +       return -ENOENT;
> > +}
> > +
> > +/* MCFG is defined in the PCI Firmware Specification 3.0 */
> > +int acpi_write_mcfg(struct acpi_ctx *ctx, const struct acpi_writer *entry)
> > +{
> > +       struct acpi_table_header *header;
> > +       struct acpi_mcfg *mcfg;
> > +       int ret;
> > +
> > +       mcfg = ctx->current;
> > +       header = &mcfg->header;
> > +
> > +       memset(mcfg, '\0', sizeof(struct acpi_mcfg));
> > +
> > +       /* Fill out header fields */
> > +       acpi_fill_header(header, "MCFG");
> > +       header->length = sizeof(struct acpi_mcfg);
> > +       header->revision = 1;
> > +       acpi_inc(ctx, sizeof(*header));
>
> sizeof(*mcfg)
>
> I can fix when applying if you agre?

D'oh, yes please.

>
> > +
> > +       ret = acpi_fill_mcfg(ctx);
> > +       if (ret)
> > +               return log_msg_ret("fill", ret);
> > +
> > +       /* (Re)calculate length and checksum */
> > +       header->length = (ulong)ctx->current - (ulong)mcfg;
> > +       header->checksum = table_compute_checksum(mcfg, header->length);
> > +
> > +       acpi_add_table(ctx, mcfg);
> > +
> > +       return 0;
> > +}
> > +ACPI_WRITER(5mcfg, "MCFG", acpi_write_mcfg, 0);
> > --
> > 2.35.0.263.gb82422642f-goog
> >
>
> Regards,
> Simon

Cheers,
Moritz

Re: [PATCH 2/2] acpi: Move MCFG implementation to common lib

[Simon Glass]

Hi Moritz,

On Mon, 7 Feb 2022 at 13:49, Moritz Fischer <moritzf@google.com> wrote:
>
> Hi Simon,
>
> On Mon, Feb 7, 2022 at 12:22 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Moritz,
> >
> > On Sat, 5 Feb 2022 at 13:17, Moritz Fischer <moritzf@google.com> wrote:
> > >
> > > MCFG tables are used on multiple arches. Move to common ACPI lib.
> > >
> > > Cc: Simon Glass <sjg@chromium.org>
> > > Signed-off-by: Moritz Fischer <moritzf@google.com>
> > > ---
> > >
> > >  arch/x86/cpu/intel_common/acpi.c  | 15 +++++---
> > >  arch/x86/cpu/tangier/acpi.c       | 11 ++++--
> > >  arch/x86/include/asm/acpi_table.h |  1 -
> > >  arch/x86/lib/acpi_table.c         | 54 --------------------------
> > >  lib/acpi/Makefile                 |  1 +
> > >  lib/acpi/mcfg.c                   | 64 +++++++++++++++++++++++++++++++
> > >  6 files changed, 81 insertions(+), 65 deletions(-)
> > >  create mode 100644 lib/acpi/mcfg.c
> > >
> > > diff --git a/arch/x86/cpu/intel_common/acpi.c b/arch/x86/cpu/intel_common/acpi.c
> > > index 15f19da206..d94ec208f6 100644
> > > --- a/arch/x86/cpu/intel_common/acpi.c
> > > +++ b/arch/x86/cpu/intel_common/acpi.c
> > > @@ -31,14 +31,17 @@
> > >  #include <linux/err.h>
> > >  #include <power/acpi_pmc.h>
> > >
> > > -u32 acpi_fill_mcfg(u32 current)
> > > +int acpi_fill_mcfg(struct acpi_ctx *ctx)
> > >  {
> > > +       size_t size;
> > > +
> > >         /* PCI Segment Group 0, Start Bus Number 0, End Bus Number is 255 */
> > > -       current += acpi_create_mcfg_mmconfig((void *)current,
> > > -                                            CONFIG_MMCONF_BASE_ADDRESS, 0, 0,
> > > -                                            (CONFIG_SA_PCIEX_LENGTH >> 20)
> > > -                                            - 1);
> > > -       return current;
> > > +       size = acpi_create_mcfg_mmconfig((void *)ctx->current,
> > > +                                        CONFIG_MMCONF_BASE_ADDRESS, 0, 0,
> > > +                                        (CONFIG_SA_PCIEX_LENGTH >> 20) - 1);
> > > +       acpi_inc(ctx, size);
> > > +
> > > +       return 0;
> > >  }
> > >
> > >  static int acpi_sci_irq(void)
> > > diff --git a/arch/x86/cpu/tangier/acpi.c b/arch/x86/cpu/tangier/acpi.c
> > > index 12f9289612..e3a2fcea76 100644
> > > --- a/arch/x86/cpu/tangier/acpi.c
> > > +++ b/arch/x86/cpu/tangier/acpi.c
> > > @@ -68,14 +68,17 @@ u32 acpi_fill_madt(u32 current)
> > >         return current;
> > >  }
> > >
> > > -u32 acpi_fill_mcfg(u32 current)
> > > +int acpi_fill_mcfg(struct acpi_ctx *ctx)
> > >  {
> > > +       size_t size;
> > > +
> > >         /* TODO: Derive parameters from SFI MCFG table */
> > > -       current += acpi_create_mcfg_mmconfig
> > > -               ((struct acpi_mcfg_mmconfig *)current,
> > > +       size = acpi_create_mcfg_mmconfig
> > > +               ((struct acpi_mcfg_mmconfig *)ctx->current,
> > >                 MCFG_BASE_ADDRESS, 0x0, 0x0, 0x0);
> > > +       acpi_inc(ctx, size);
> > >
> > > -       return current;
> > > +       return 0;
> > >  }
> > >
> > >  static u32 acpi_fill_csrt_dma(struct acpi_csrt_group *grp)
> > > diff --git a/arch/x86/include/asm/acpi_table.h b/arch/x86/include/asm/acpi_table.h
> > > index 0d07f7cad8..39547de0d4 100644
> > > --- a/arch/x86/include/asm/acpi_table.h
> > > +++ b/arch/x86/include/asm/acpi_table.h
> > > @@ -34,7 +34,6 @@ int acpi_create_madt_lapic_nmi(struct acpi_madt_lapic_nmi *lapic_nmi,
> > >  u32 acpi_fill_madt(u32 current);
> > >  int acpi_create_mcfg_mmconfig(struct acpi_mcfg_mmconfig *mmconfig, u32 base,
> > >                               u16 seg_nr, u8 start, u8 end);
> > > -u32 acpi_fill_mcfg(u32 current);
> > >
> > >  /**
> > >   * acpi_write_hpet() - Write out a HPET table
> > > diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> > > index 753bf39619..c5b33dc65d 100644
> > > --- a/arch/x86/lib/acpi_table.c
> > > +++ b/arch/x86/lib/acpi_table.c
> > > @@ -161,28 +161,6 @@ int acpi_write_madt(struct acpi_ctx *ctx, const struct acpi_writer *entry)
> > >  }
> > >  ACPI_WRITER(5x86, NULL, acpi_write_madt, 0);
> > >
> > > -int acpi_create_mcfg_mmconfig(struct acpi_mcfg_mmconfig *mmconfig, u32 base,
> > > -                             u16 seg_nr, u8 start, u8 end)
> > > -{
> > > -       memset(mmconfig, 0, sizeof(*mmconfig));
> > > -       mmconfig->base_address_l = base;
> > > -       mmconfig->base_address_h = 0;
> > > -       mmconfig->pci_segment_group_number = seg_nr;
> > > -       mmconfig->start_bus_number = start;
> > > -       mmconfig->end_bus_number = end;
> > > -
> > > -       return sizeof(struct acpi_mcfg_mmconfig);
> > > -}
> > > -
> > > -__weak u32 acpi_fill_mcfg(u32 current)
> > > -{
> > > -       current += acpi_create_mcfg_mmconfig
> > > -               ((struct acpi_mcfg_mmconfig *)current,
> > > -               CONFIG_PCIE_ECAM_BASE, 0x0, 0x0, 255);
> > > -
> > > -       return current;
> > > -}
> > > -
> > >  /**
> > >   * acpi_create_tcpa() - Create a TCPA table
> > >   *
> > > @@ -480,38 +458,6 @@ int acpi_write_gnvs(struct acpi_ctx *ctx, const struct acpi_writer *entry)
> > >  }
> > >  ACPI_WRITER(4gnvs, "GNVS", acpi_write_gnvs, 0);
> > >
> > > -/* MCFG is defined in the PCI Firmware Specification 3.0 */
> > > -int acpi_write_mcfg(struct acpi_ctx *ctx, const struct acpi_writer *entry)
> > > -{
> > > -       struct acpi_table_header *header;
> > > -       struct acpi_mcfg *mcfg;
> > > -       u32 current;
> > > -
> > > -       mcfg = ctx->current;
> > > -       header = &mcfg->header;
> > > -
> > > -       current = (u32)mcfg + sizeof(struct acpi_mcfg);
> > > -
> > > -       memset(mcfg, '\0', sizeof(struct acpi_mcfg));
> > > -
> > > -       /* Fill out header fields */
> > > -       acpi_fill_header(header, "MCFG");
> > > -       header->length = sizeof(struct acpi_mcfg);
> > > -       header->revision = 1;
> > > -
> > > -       current = acpi_fill_mcfg(current);
> > > -
> > > -       /* (Re)calculate length and checksum */
> > > -       header->length = current - (u32)mcfg;
> > > -       header->checksum = table_compute_checksum(mcfg, header->length);
> > > -
> > > -       acpi_inc(ctx, mcfg->header.length);
> > > -       acpi_add_table(ctx, mcfg);
> > > -
> > > -       return 0;
> > > -}
> > > -ACPI_WRITER(5mcfg, "MCFG", acpi_write_mcfg, 0);
> > > -
> > >  /**
> > >   * acpi_write_hpet() - Write out a HPET table
> > >   *
> > > diff --git a/lib/acpi/Makefile b/lib/acpi/Makefile
> > > index f9b504988f..956b5a0d72 100644
> > > --- a/lib/acpi/Makefile
> > > +++ b/lib/acpi/Makefile
> > > @@ -11,6 +11,7 @@ obj-y += acpi_writer.o
> > >  ifndef CONFIG_QEMU
> > >  obj-y += base.o
> > >  obj-y += csrt.o
> > > +obj-y += mcfg.o
> > >
> > >  # Sandbox does not build a .asl file
> > >  ifndef CONFIG_SANDBOX
> > > diff --git a/lib/acpi/mcfg.c b/lib/acpi/mcfg.c
> > > new file mode 100644
> > > index 0000000000..2231e25124
> > > --- /dev/null
> > > +++ b/lib/acpi/mcfg.c
> > > @@ -0,0 +1,64 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Write an ACPI MCFG table
> > > + *
> > > + * Copyright 2022 Google LLC
> > > + */
> > > +
> > > +#define LOG_CATEGORY LOGC_ACPI
> > > +
> > > +#include <common.h>
> > > +#include <mapmem.h>
> > > +#include <tables_csum.h>
> > > +#include <acpi/acpi_table.h>
> > > +#include <dm/acpi.h>
> > > +
> > > +int acpi_create_mcfg_mmconfig(struct acpi_mcfg_mmconfig *mmconfig, u32 base,
> > > +                             u16 seg_nr, u8 start, u8 end)
> > > +{
> > > +       memset(mmconfig, 0, sizeof(*mmconfig));
> > > +       mmconfig->base_address_l = base;
> > > +       mmconfig->base_address_h = 0;
> > > +       mmconfig->pci_segment_group_number = seg_nr;
> > > +       mmconfig->start_bus_number = start;
> > > +       mmconfig->end_bus_number = end;
> > > +
> > > +       return sizeof(struct acpi_mcfg_mmconfig);
> > > +}
> > > +
> > > +__weak int acpi_fill_mcfg(struct acpi_ctx *ctx)
> > > +{
> > > +       return -ENOENT;
> > > +}
> > > +
> > > +/* MCFG is defined in the PCI Firmware Specification 3.0 */
> > > +int acpi_write_mcfg(struct acpi_ctx *ctx, const struct acpi_writer *entry)
> > > +{
> > > +       struct acpi_table_header *header;
> > > +       struct acpi_mcfg *mcfg;
> > > +       int ret;
> > > +
> > > +       mcfg = ctx->current;
> > > +       header = &mcfg->header;
> > > +
> > > +       memset(mcfg, '\0', sizeof(struct acpi_mcfg));
> > > +
> > > +       /* Fill out header fields */
> > > +       acpi_fill_header(header, "MCFG");
> > > +       header->length = sizeof(struct acpi_mcfg);
> > > +       header->revision = 1;
> > > +       acpi_inc(ctx, sizeof(*header));
> >
> > sizeof(*mcfg)
> >
> > I can fix when applying if you agre?
>
> D'oh, yes please.

OK, with that fixed:

Reviewed-by: Simon Glass <sjg@chromium.org>

>
> >
> > > +
> > > +       ret = acpi_fill_mcfg(ctx);
> > > +       if (ret)
> > > +               return log_msg_ret("fill", ret);
> > > +
> > > +       /* (Re)calculate length and checksum */
> > > +       header->length = (ulong)ctx->current - (ulong)mcfg;
> > > +       header->checksum = table_compute_checksum(mcfg, header->length);
> > > +
> > > +       acpi_add_table(ctx, mcfg);
> > > +
> > > +       return 0;
> > > +}
> > > +ACPI_WRITER(5mcfg, "MCFG", acpi_write_mcfg, 0);
> > > --
> > > 2.35.0.263.gb82422642f-goog
> > >
> >
> > Regards,
> > Simon
>
> Cheers,
> Moritz

Re: [PATCH 2/2] acpi: Move MCFG implementation to common lib

[Simon Glass]

Hi Moritz,

On Mon, 7 Feb 2022 at 13:49, Moritz Fischer <moritzf@google.com> wrote:
>
> Hi Simon,
>
> On Mon, Feb 7, 2022 at 12:22 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Moritz,
> >
> > On Sat, 5 Feb 2022 at 13:17, Moritz Fischer <moritzf@google.com> wrote:
> > >
> > > MCFG tables are used on multiple arches. Move to common ACPI lib.
> > >
> > > Cc: Simon Glass <sjg@chromium.org>
> > > Signed-off-by: Moritz Fischer <moritzf@google.com>
> > > ---
> > >
> > >  arch/x86/cpu/intel_common/acpi.c  | 15 +++++---
> > >  arch/x86/cpu/tangier/acpi.c       | 11 ++++--
> > >  arch/x86/include/asm/acpi_table.h |  1 -
> > >  arch/x86/lib/acpi_table.c         | 54 --------------------------
> > >  lib/acpi/Makefile                 |  1 +
> > >  lib/acpi/mcfg.c                   | 64 +++++++++++++++++++++++++++++++
> > >  6 files changed, 81 insertions(+), 65 deletions(-)
> > >  create mode 100644 lib/acpi/mcfg.c
> > >
Applied to u-boot-dm, thanks!

Re: [PATCH 1/2] arch: x86: lib: acpi_table: Fix MCFG entries

[Simon Glass]

Hi Moritz,

On Sat, 5 Feb 2022 at 13:17, Moritz Fischer <moritzf@google.com> wrote:
>
> Commit d953137526cc ("x86: Move SSDT table to a writer function")
> introduced a bug where the actual MCFG entries are no longer generated.
>
> Cc: Simon Glass <sjg@chromium.org>
> Fixes: d953137526cc ("x86: Move SSDT table to a writer function")
> Signed-off-by: Moritz Fischer <moritzf@google.com>
> ---
>
> Hi Simon, Andriy,
>
> it looks like this got dropped when moving stuff around, I don't have HW
> to test this, but it seemed off.
>
> - Moritz
>
> ---
>
>  arch/x86/lib/acpi_table.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>
Tested on: coral
Tested-by: Simon Glass <sjg@chromium.org>


>
> diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> index c0534343f1..753bf39619 100644
> --- a/arch/x86/lib/acpi_table.c
> +++ b/arch/x86/lib/acpi_table.c
> @@ -499,6 +499,8 @@ int acpi_write_mcfg(struct acpi_ctx *ctx, const struct acpi_writer *entry)
>         header->length = sizeof(struct acpi_mcfg);
>         header->revision = 1;
>
> +       current = acpi_fill_mcfg(current);
> +
>         /* (Re)calculate length and checksum */
>         header->length = current - (u32)mcfg;
>         header->checksum = table_compute_checksum(mcfg, header->length);
> --
> 2.35.0.263.gb82422642f-goog
>

Re: [PATCH 1/2] arch: x86: lib: acpi_table: Fix MCFG entries

[Simon Glass]

Hi Moritz,

On Sat, 5 Feb 2022 at 13:17, Moritz Fischer <moritzf@google.com> wrote:
>
> Commit d953137526cc ("x86: Move SSDT table to a writer function")
> introduced a bug where the actual MCFG entries are no longer generated.
>
> Cc: Simon Glass <sjg@chromium.org>
> Fixes: d953137526cc ("x86: Move SSDT table to a writer function")
> Signed-off-by: Moritz Fischer <moritzf@google.com>
> ---
>
> Hi Simon, Andriy,
>
> it looks like this got dropped when moving stuff around, I don't have HW
> to test this, but it seemed off.
>
> - Moritz
>
> ---
>
>  arch/x86/lib/acpi_table.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>
Tested on: coral
Tested-by: Simon Glass <sjg@chromium.org>


>
Applied to u-boot-dm, thanks!