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: 74+ 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:17 ` 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 ` 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 ` 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.