Paolo, Thanks for the feedback. I've posted v2 with changes based on Igor's feedback. I've also included a qtest per your feedback. Eric
________________________________ From: Paolo Bonzini <pbonz...@redhat.com> Sent: Tuesday, November 3, 2020 3:16 PM To: Eric DeVolder <eric.devol...@oracle.com>; qemu-devel@nongnu.org <qemu-devel@nongnu.org> Cc: m...@redhat.com <m...@redhat.com>; imamm...@redhat.com <imamm...@redhat.com>; marcel.apfelb...@gmail.com <marcel.apfelb...@gmail.com>; r...@twiddle.net <r...@twiddle.net>; ehabk...@redhat.com <ehabk...@redhat.com>; konrad.w...@oracle.com <konrad.w...@oracle.com>; boris.ostrov...@oracle.com <boris.ostrov...@oracle.com> Subject: Re: [PATCH 1/1] acpi: Implement ACPI ERST support for guests On 26/10/20 21:19, Eric DeVolder wrote: > This changeset introduces support for the ACPI Error Record > Serialization Table, ERST. > > ERST is defined in [1], and the error records are defined > according to [2]. > > This changeset generates the ACPI ERST table, which OSPM > follows to program the associated ERST device. The ERST > device occupies 8KiB of address space, with the first 8 bytes > containing registers, and the remainder being an exchange > buffer for reading/writing error records. > > The ERST device contains two registers, ACTION and VALUE > registers, following closely the ERST operations. > > The ERST device only examines the record for the signature 'CPER', > its record identifier, and bounds-checks the length against the size > of the the CPER record header; otherwise all other record fields > and payload are ignored. > > There are two options for this device: > -global acpi-erst.size=X > -global acpi-erst.filename=Y > The size X, if not specified, defaults to 64KiB, and must be within > 64KiB and 1MiB. > The filename Y, if not specified, defaults to "acpi-erst.backing". > The ERST backing storage is not mapped into the guest address space, > just the 8KiB programming area is mapped into the guest. > > This has been utilized as a backend for the Linux pstore feature. > > [1] ACPI 6.3 Specification, 18.3 Error Serialization > https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf > > [2] UEFI 2.8 Specification, Appendix N Common Platform Error Record > https://uefi.org/sites/default/files/resources/UEFI_Spec_2_8_final.pdf In addition to what Igor said, this really needs a test for the device (using qtest). It doesn't have to be super fine-grained, but at least a smoke test that the action and value registers do what they mean for a sample error recording operation. Paolo