[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 11/35] acpi: nvdimm_build_ssdt: use acpi_table_begin()/acp
From: |
Igor Mammedov |
Subject: |
Re: [PATCH v3 11/35] acpi: nvdimm_build_ssdt: use acpi_table_begin()/acpi_table_end() instead of build_header() |
Date: |
Wed, 22 Sep 2021 16:38:06 +0200 |
On Mon, 20 Sep 2021 18:41:30 +0200
Eric Auger <eauger@redhat.com> wrote:
> Hi Igor,
>
> On 9/7/21 4:47 PM, Igor Mammedov wrote:
> > it replaces error-prone pointer arithmetic for build_header() API,
> > with 2 calls to start and finish table creation,
> > which hides offsets magic from API user.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > v3:
> > * s/acpi_init_table|acpi_table_composed/acpi_table_begin|acpi_table_end/
> >
> > CC: xiaoguangrong.eric@gmail.com
> > ---
> > hw/acpi/nvdimm.c | 18 ++++++++++--------
> > 1 file changed, 10 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> > index 15f6ca82ca..e050b47c2b 100644
> > --- a/hw/acpi/nvdimm.c
> > +++ b/hw/acpi/nvdimm.c
> > @@ -1274,14 +1274,15 @@ static void nvdimm_build_ssdt(GArray
> > *table_offsets, GArray *table_data,
> > NVDIMMState *nvdimm_state,
> > uint32_t ram_slots, const char *oem_id)
> > {
> > + int mem_addr_offset;
> > Aml *ssdt, *sb_scope, *dev;
> > - int mem_addr_offset, nvdimm_ssdt;
> > + AcpiTable table = { .sig = "SSDT", .rev = 1,
> > + .oem_id = oem_id, .oem_table_id = "NVDIMM" };
> >
> > acpi_add_table(table_offsets, table_data);
> >
> > + acpi_table_begin(&table, table_data);
> > ssdt = init_aml_allocator();
> > - acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
> > -
> > sb_scope = aml_scope("\\_SB");
> >
> > dev = aml_device("NVDR");
> > @@ -1310,8 +1311,6 @@ static void nvdimm_build_ssdt(GArray *table_offsets,
> > GArray *table_data,
> > aml_append(sb_scope, dev);
> > aml_append(ssdt, sb_scope);
> >
> > - nvdimm_ssdt = table_data->len;
> > -
> > /* copy AML table into ACPI tables blob and patch header there */
> > g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
> > mem_addr_offset = build_append_named_dword(table_data,
> > @@ -1323,10 +1322,13 @@ static void nvdimm_build_ssdt(GArray
> > *table_offsets, GArray *table_data,
> > bios_linker_loader_add_pointer(linker,
> > ACPI_BUILD_TABLE_FILE, mem_addr_offset, sizeof(uint32_t),
> > NVDIMM_DSM_MEM_FILE, 0);
> > - build_header(linker, table_data,
> > - (void *)(table_data->data + nvdimm_ssdt),
> > - "SSDT", table_data->len - nvdimm_ssdt, 1, oem_id,
> > "NVDIMM");
> > free_aml_allocator();
> > + /*
> > + * must be executed as the last so that pointer patching command above
> > + * would be executed by guest before it recalculates checksum which
> > were
> > + * scheduled by acpi_table_composed()
> s/acpi_table_composed/acpi_table_end.
fixed it up:
/*
* must be executed as the last so that pointer patching command above
* would be executed by guest before it recalculated checksum which were
* scheduled by acpi_table_end()
*/
> also the sentence may need some rewording.
>
> Wonder if that kind of comment still is useful. Maybe this should be put
> once in the doc comment of acpi_table_end() in [PATCH v3 01/35] acpi:
> add helper routines to initialize ACPI tables, for future users to pay
> attention that it shall be called at the very end.
I'll keep comment for now, and add patch to add something similar to
acpi_table_end() later or drop it if I come up with a way to actually
enforce expected linker order.
> Besides
>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
>
> Eric
>
>
> > + */
> > + acpi_table_end(linker, &table);
> > }
> >
> > void nvdimm_build_srat(GArray *table_data)
> >
>
- [PATCH v3 07/35] acpi: acpi_build_hest: use acpi_table_begin()/acpi_table_end() instead of build_header(), (continued)
- [PATCH v3 07/35] acpi: acpi_build_hest: use acpi_table_begin()/acpi_table_end() instead of build_header(), Igor Mammedov, 2021/09/07
- [PATCH v3 08/35] acpi: build_mcfg: use acpi_table_begin()/acpi_table_end() instead of build_header(), Igor Mammedov, 2021/09/07
- [PATCH v3 09/35] acpi: build_hmat: use acpi_table_begin()/acpi_table_end() instead of build_header(), Igor Mammedov, 2021/09/07
- [PATCH v3 10/35] acpi: nvdimm_build_nfit: use acpi_table_begin()/acpi_table_end() instead of build_header(), Igor Mammedov, 2021/09/07
- [PATCH v3 11/35] acpi: nvdimm_build_ssdt: use acpi_table_begin()/acpi_table_end() instead of build_header(), Igor Mammedov, 2021/09/07
- [PATCH v3 12/35] acpi: vmgenid_build_acpi: use acpi_table_begin()/acpi_table_end() instead of build_header(), Igor Mammedov, 2021/09/07
- [PATCH v3 13/35] acpi: x86: build_dsdt: use acpi_table_begin()/acpi_table_end() instead of build_header(), Igor Mammedov, 2021/09/07
- [PATCH v3 14/35] acpi: build_hpet: use acpi_table_begin()/acpi_table_end() instead of build_header(), Igor Mammedov, 2021/09/07
- [PATCH v3 16/35] acpi: arm/x86: build_srat: use acpi_table_begin()/acpi_table_end() instead of build_header(), Igor Mammedov, 2021/09/07
- [PATCH v3 15/35] acpi: build_tpm_tcpa: use acpi_table_begin()/acpi_table_end() instead of build_header(), Igor Mammedov, 2021/09/07