From: "Michael S. Tsirkin" <mst@redhat.com>
To: Igor Mammedov <imammedo@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [PATCH 00/53] acpi: refactor error prone build_header() and packed structures usage in ACPI tables
Date: Fri, 2 Jul 2021 10:47:20 -0400 [thread overview]
Message-ID: <20210702104548-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20210625091818.1047980-1-imammedo@redhat.com>
On Fri, Jun 25, 2021 at 05:17:24AM -0400, Igor Mammedov wrote:
> Highlights:
> * drop pointer arithmetic in ACPI tables code
> * use endian agnostic API
> * simplifies review of tables. /in most cases just line by line comparision with spec/
A hue amount of work, thank you!
To make it easier to merge, how about splitting it up a bit?
E.g. I think first 10-11 patches make sense on their own, right?
> Series replaces build_header() with acpi_init_table()/acpi_table_composed()
> API that hides pointer/offset arithmetic from user, to prevent
> errors caused by it [1].
> While doing above, it was necessary to split table header from
> packed structure that was describing the table, which is
> counter-productive since it still leaves packed structure drawbacks.
> So that sort of forced me to rewrite tables that were composed with
> help of packed structures to preferred build_append_int_noprefix() API.
> In cases where switch to build_append_int_noprefix() was small, it was
> combined with acpi_init_table()/acpi_table_composed() patch.
> Conversion reduced code size quite a bit despite me adding doc comments
> for every table row.
>
> Series also includes optional qtest patches that add missing acpi
> tests for tables that I'm touching to verify conversion changes.
> That includes an alternative build time based impl. of
> qtest_has_accel() API. So if we start bike-shedding this qtest_has_accel()
> we can safely drop all tests included, till the time discussion settles
> and some form of a qtest_has_accel() is merged, at which point I'd respin
> depended tests.
>
> 1) commits
> bb9feea43179 x86: acpi: use offset instead of pointer when using build_header()
> 4d027afeb3a9 Virt: ACPI: fix qemu assert due to re-assigned table data address
>
> Link to repo:
> https://gitlab.com/imammedo/qemu/-/commits/acpi_build_header_refactoring
>
> CC: mst@redhat.com
>
> Igor Mammedov (53):
> tests: acpi: dump table with failed checksum
> tests: qtest: add qtest_has_accel() to check if tested binary supports
> accelerator
> tests: acpi: whitelist expected tables for acpi/q35/numamem testcase
> tests: acpi: q35: test for x2APIC entries in SRAT
> tests: acpi: update expected tables blobs
> tests: acpi: whitelist new expected table
> tests/data/acpi/q35/DMAR.dmar
> tests: acpi: add testcase for intel_iommu (DMAR table)
> tests: acpi: add expected blob for DMAR table
> tests: acpi: whitelist expected blobs for new acpi/q35/ivrs testcase
> tests: acpi: add testcase for amd-iommu (IVRS table)
> tests: acpi: update expected blobs
> tests: acpi: arm/virt: drop redundant test_acpi_one() in
> test_acpi_virt_tcg()
> tests: acpi: whitelist expected tables for acpi/virt/iort testcase
> tests: acpi: arm/virt: use kvm to test IORT table
> tests: acpi: add expected IORT table blob
> tests: arm-cpu-features: use qtest_has_kvm() API
> tests: migration-test: use qtest_has_accel() API
> tests: bios-tables-test: use qtest_has_accel() API to register TCG
> only tests
> acpi: add helper routines to initialize ACPI tables
> acpi: build_rsdt: use acpi_init_table()/acpi_table_composed() instead
> of build_header()
> acpi: build_xsdt: use acpi_init_table()/acpi_table_composed() instead
> of build_header()
> acpi: build_slit: use acpi_init_table()/acpi_table_composed() instead
> of build_header()
> acpi: build_fadt: use acpi_init_table()/acpi_table_composed() instead
> of build_header()
> acpi: build_tpm2: use acpi_init_table()/acpi_table_composed() instead
> of build_header()
> acpi: acpi_build_hest: use acpi_init_table()/acpi_table_composed()
> instead of build_header()
> acpi: build_mcfg: use acpi_init_table()/acpi_table_composed() instead
> of build_header()
> acpi: build_hmat: use acpi_init_table()/acpi_table_composed() instead
> of build_header()
> acpi: nvdimm_build_nfit: use acpi_init_table()/acpi_table_composed()
> instead of build_header()
> acpi: nvdimm_build_ssdt: use acpi_init_table()/acpi_table_composed()
> instead of build_header()
> acpi: vmgenid_build_acpi: use acpi_init_table()/acpi_table_composed()
> instead of build_header()
> acpi: x86: build_dsdt: use acpi_init_table()/acpi_table_composed()
> instead of build_header()
> acpi: build_hpet: use acpi_init_table()/acpi_table_composed() instead
> of build_header()
> acpi: build_tpm_tcpa: use acpi_init_table()/acpi_table_composed()
> instead of build_header()
> acpi: arm/x86: build_srat: use acpi_init_table()/acpi_table_composed()
> instead of build_header()
> acpi: use build_append_int_noprefix() API to compose SRAT table
> acpi: build_dmar_q35: use acpi_init_table()/acpi_table_composed()
> instead of build_header()
> acpi: build_waet: use acpi_init_table()/acpi_table_composed() instead
> of build_header()
> acpi: build_amd_iommu: use acpi_init_table()/acpi_table_composed()
> instead of build_header()
> acpi: madt: arm/x86: use acpi_init_table()/acpi_table_composed()
> instead of build_header()
> acpi: x86: remove dead code
> acpi: x86: set enabled when composing _MAT entries
> acpi: x86: madt: use build_append_int_noprefix() API to compose MADT
> table
> acpi: arm/virt: madt: use build_append_int_noprefix() API to compose
> MADT table
> acpi: build_dsdt_microvm: use acpi_init_table()/acpi_table_composed()
> instead of build_header()
> acpi: arm: virt: build_dsdt: use
> acpi_init_table()/acpi_table_composed() instead of build_header()
> acpi: arm: virt: build_iort: use
> acpi_init_table()/acpi_table_composed() instead of build_header()
> acpi: arm/virt: convert build_iort() to endian agnostic
> build_append_FOO() API
> acpi: arm/virt: build_spcr: fix invalid cast
> acpi: arm/virt: build_spcr: use
> acpi_init_table()/acpi_table_composed() instead of build_header()
> acpi: arm/virt: build_gtdt: use
> acpi_init_table()/acpi_table_composed() instead of build_header()
> acpi: build_facs: use build_append_int_noprefix() API to compose table
> acpi: remove no longer used build_header()
> acpi: AcpiGenericAddress no longer used to map/access fields of MMIO,
> drop packed attribute
>
> include/hw/acpi/acpi-defs.h | 528 +------------------------
> include/hw/acpi/acpi_dev_interface.h | 3 +-
> include/hw/acpi/aml-build.h | 21 +-
> include/hw/i386/pc.h | 7 +-
> tests/qtest/libqos/libqtest.h | 8 +
> hw/acpi/acpi-x86-stub.c | 3 +-
> hw/acpi/aml-build.c | 188 +++++----
> hw/acpi/cpu.c | 17 +-
> hw/acpi/ghes.c | 10 +-
> hw/acpi/hmat.c | 14 +-
> hw/acpi/nvdimm.c | 64 ++--
> hw/acpi/pci.c | 18 +-
> hw/acpi/vmgenid.c | 16 +-
> hw/arm/virt-acpi-build.c | 550 +++++++++++++++------------
> hw/i386/acpi-build.c | 285 +++++++-------
> hw/i386/acpi-common.c | 158 ++++----
> hw/i386/acpi-microvm.c | 13 +-
> meson.build | 6 +
> tests/data/acpi/q35/APIC.numamem | Bin 0 -> 2686 bytes
> tests/data/acpi/q35/DMAR.dmar | Bin 0 -> 80 bytes
> tests/data/acpi/q35/DSDT.ivrs | Bin 0 -> 7877 bytes
> tests/data/acpi/q35/DSDT.numamem | Bin 7865 -> 35222 bytes
> tests/data/acpi/q35/FACP.numamem | Bin 0 -> 244 bytes
> tests/data/acpi/q35/IVRS.ivrs | Bin 0 -> 104 bytes
> tests/data/acpi/q35/SRAT.numamem | Bin 224 -> 5080 bytes
> tests/data/acpi/virt/IORT.iort | Bin 0 -> 124 bytes
> tests/qtest/acpi-utils.c | 14 +
> tests/qtest/arm-cpu-features.c | 29 +-
> tests/qtest/bios-tables-test.c | 85 ++++-
> tests/qtest/libqtest.c | 27 ++
> tests/qtest/migration-test.c | 15 +-
> 31 files changed, 873 insertions(+), 1206 deletions(-)
> create mode 100644 tests/data/acpi/q35/APIC.numamem
> create mode 100644 tests/data/acpi/q35/DMAR.dmar
> create mode 100644 tests/data/acpi/q35/DSDT.ivrs
> create mode 100644 tests/data/acpi/q35/FACP.numamem
> create mode 100644 tests/data/acpi/q35/IVRS.ivrs
> create mode 100644 tests/data/acpi/virt/IORT.iort
>
> --
> 2.27.0
next prev parent reply other threads:[~2021-07-02 14:48 UTC|newest]
Thread overview: 71+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-25 9:17 [PATCH 00/53] acpi: refactor error prone build_header() and packed structures usage in ACPI tables Igor Mammedov
2021-06-25 9:17 ` [PATCH 01/53] tests: acpi: dump table with failed checksum Igor Mammedov
2021-06-25 9:17 ` [PATCH 02/53] tests: qtest: add qtest_has_accel() to check if tested binary supports accelerator Igor Mammedov
2021-06-25 9:33 ` Philippe Mathieu-Daudé
2021-06-25 9:17 ` [PATCH 03/53] tests: acpi: whitelist expected tables for acpi/q35/numamem testcase Igor Mammedov
2021-06-25 9:17 ` [PATCH 04/53] tests: acpi: q35: test for x2APIC entries in SRAT Igor Mammedov
2021-07-02 14:45 ` Michael S. Tsirkin
2021-07-08 9:18 ` Igor Mammedov
2021-07-08 11:59 ` Michael S. Tsirkin
2021-06-25 9:17 ` [PATCH 05/53] tests: acpi: update expected tables blobs Igor Mammedov
2021-06-25 9:17 ` [PATCH 06/53] tests: acpi: whitelist new expected table tests/data/acpi/q35/DMAR.dmar Igor Mammedov
2021-06-25 9:17 ` [PATCH 07/53] tests: acpi: add testcase for intel_iommu (DMAR table) Igor Mammedov
2021-06-25 9:17 ` [PATCH 08/53] tests: acpi: add expected blob for DMAR table Igor Mammedov
2021-06-25 9:17 ` [PATCH 09/53] tests: acpi: whitelist expected blobs for new acpi/q35/ivrs testcase Igor Mammedov
2021-06-25 9:17 ` [PATCH 10/53] tests: acpi: add testcase for amd-iommu (IVRS table) Igor Mammedov
2021-06-25 9:17 ` [PATCH 11/53] tests: acpi: update expected blobs Igor Mammedov
2021-06-25 9:17 ` [PATCH 12/53] tests: acpi: arm/virt: drop redudant test_acpi_one() in test_acpi_virt_tcg() Igor Mammedov
2021-06-25 9:17 ` [PATCH 12/53] tests: acpi: arm/virt: drop redundant " Igor Mammedov
2021-06-25 9:17 ` [PATCH 13/53] tests: acpi: whitelist expected tables for acpi/virt/iort testcase Igor Mammedov
2021-06-25 9:17 ` [PATCH 14/53] tests: acpi: arm/virt: use kvm to test IORT table Igor Mammedov
2021-06-25 9:17 ` [PATCH 15/53] tests: acpi: add expected IORT table blob Igor Mammedov
2021-06-25 9:17 ` [PATCH 16/53] tests: arm-cpu-features: use qtest_has_kvm() API Igor Mammedov
2021-06-25 9:17 ` [PATCH 17/53] tests: migration-test: use qtest_has_accel() API Igor Mammedov
2021-06-25 9:17 ` [PATCH 18/53] tests: bios-tables-test: use qtest_has_accel() API to register TCG only tests Igor Mammedov
2021-06-25 9:17 ` [PATCH 19/53] acpi: add helper routines to initialize ACPI tables Igor Mammedov
2021-06-29 19:16 ` Michael S. Tsirkin
2021-06-30 6:32 ` Igor Mammedov
2021-06-25 9:17 ` [PATCH 20/53] acpi: build_rsdt: use acpi_init_table()/acpi_table_composed() instead of build_header() Igor Mammedov
2021-06-25 9:17 ` [PATCH 21/53] acpi: build_xsdt: " Igor Mammedov
2021-06-25 9:17 ` [PATCH 22/53] acpi: build_slit: " Igor Mammedov
2021-06-25 9:17 ` [PATCH 23/53] acpi: build_fadt: " Igor Mammedov
2021-06-25 9:17 ` [PATCH 24/53] acpi: build_tpm2: " Igor Mammedov
2021-06-29 14:56 ` Stefan Berger
2021-06-25 9:17 ` [PATCH 25/53] acpi: acpi_build_hest: " Igor Mammedov
2021-07-02 12:21 ` Dongjiu Geng
2021-06-25 9:17 ` [PATCH 26/53] acpi: build_mcfg: " Igor Mammedov
2021-06-25 9:17 ` [PATCH 27/53] acpi: build_hmat: " Igor Mammedov
2021-06-25 9:17 ` [PATCH 28/53] acpi: nvdimm_build_nfit: " Igor Mammedov
2021-06-25 9:17 ` [PATCH 29/53] acpi: nvdimm_build_ssdt: " Igor Mammedov
2021-06-25 9:17 ` [PATCH 30/53] acpi: vmgenid_build_acpi: " Igor Mammedov
2021-06-25 9:17 ` [PATCH 31/53] acpi: x86: build_dsdt: " Igor Mammedov
2021-06-25 9:17 ` [PATCH 32/53] acpi: build_hpet: " Igor Mammedov
2021-06-25 9:17 ` [PATCH 33/53] acpi: build_tpm_tcpa: " Igor Mammedov
2021-06-29 15:19 ` Stefan Berger
2021-06-30 7:22 ` Igor Mammedov
2021-06-30 7:25 ` [PATCH v2 " Igor Mammedov
2021-06-30 12:21 ` Stefan Berger
2021-06-25 9:17 ` [PATCH 34/53] acpi: arm/x86: build_srat: " Igor Mammedov
2021-06-25 9:18 ` [PATCH 35/53] acpi: use build_append_int_noprefix() API to compose SRAT table Igor Mammedov
2021-06-25 9:18 ` [PATCH 36/53] acpi: build_dmar_q35: use acpi_init_table()/acpi_table_composed() instead of build_header() Igor Mammedov
2021-06-25 9:18 ` [PATCH 37/53] acpi: build_waet: " Igor Mammedov
2021-06-25 9:18 ` [PATCH 38/53] acpi: build_amd_iommu: " Igor Mammedov
2021-06-25 9:18 ` [PATCH 39/53] acpi: madt: arm/x86: " Igor Mammedov
2021-06-25 9:18 ` [PATCH 40/53] acpi: x86: remove dead code Igor Mammedov
2021-06-25 9:18 ` [PATCH 41/53] acpi: x86: set enabled when composing _MAT entries Igor Mammedov
2021-06-25 9:18 ` [PATCH 42/53] acpi: x86: madt: use build_append_int_noprefix() API to compose MADT table Igor Mammedov
2021-06-25 9:18 ` [PATCH 43/53] acpi: arm/virt: " Igor Mammedov
2021-06-25 9:18 ` [PATCH 44/53] acpi: build_dsdt_microvm: use acpi_init_table()/acpi_table_composed() instead of build_header() Igor Mammedov
2021-06-25 9:18 ` [PATCH 45/53] acpi: arm: virt: build_dsdt: " Igor Mammedov
2021-06-25 9:18 ` [PATCH 46/53] acpi: arm: virt: build_iort: " Igor Mammedov
2021-06-25 9:18 ` [PATCH 47/53] acpi: arm/virt: convert build_iort() to endian agnostic build_append_FOO() API Igor Mammedov
2021-06-25 9:18 ` [PATCH 48/53] acpi: arm/virt: build_spcr: fix invalid cast Igor Mammedov
2021-06-25 9:18 ` [PATCH 49/53] acpi: arm/virt: build_spcr: use acpi_init_table()/acpi_table_composed() instead of build_header() Igor Mammedov
2021-06-25 9:18 ` [PATCH 50/53] acpi: arm/virt: build_gtdt: " Igor Mammedov
2021-06-25 9:18 ` [PATCH 51/53] acpi: build_facs: use build_append_int_noprefix() API to compose table Igor Mammedov
2021-06-25 9:18 ` [PATCH 52/53] acpi: remove no longer used build_header() Igor Mammedov
2021-06-25 9:18 ` [PATCH 53/53] acpi: AcpiGenericAddress no longer used to map/access fields of MMIO, drop packed attribute Igor Mammedov
2021-07-02 14:47 ` Michael S. Tsirkin [this message]
2021-07-05 7:56 ` [PATCH 00/53] acpi: refactor error prone build_header() and packed structures usage in ACPI tables Igor Mammedov
2021-07-05 18:00 ` Michael S. Tsirkin
2021-07-07 9:11 ` Igor Mammedov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210702104548-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=imammedo@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).