[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/7] acpi: add vmcoreinfo device
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/7] acpi: add vmcoreinfo device |
Date: |
Fri, 7 Jul 2017 15:13:11 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
Marc-André,
sorry about this, but I have another late comment (I shoudl have pointed
this out in v1). Regarding:
On 07/06/17 12:16, Marc-André Lureau wrote:
> +#define VMCOREINFO_OFFSET 40 /* allow space for
> + * OVMF SDT Header Probe Supressor
> + */
and
> + Method (ADDR, 0, NotSerialized)
> + {
> + Local0 = Package (0x02) {}
> + Local0 [Zero] = (VCIA + 0x28)
> + Local0 [One] = Zero
> + Return (Local0)
> + }
and
> +In order to implement an OVMF "SDT Header Probe Suppressor", the contents of
> +the vmcoreinfo blob has 40 bytes of padding:
> +
> ++-----------------------------------+
> +| SSDT with OEM Table ID = VMCOREIN |
> ++-----------------------------------+
> +| ... | TOP OF PAGE
> +| VCIA dword object ----------------|-----> +---------------------------+
> +| ... | | fw-allocated array for |
> +| _STA method referring to VCIA | | "etc/vmcoreinfo" |
> +| ... | +---------------------------+
> +| ADDR method referring to VCIA | | 0: OVMF SDT Header probe |
> +| ... | | suppressor |
> ++-----------------------------------+ | 40: uint32 version field |
> + | 44: info contents |
> + | .... |
> + +---------------------------+
> + END OF PAGE
Please define the VMCOREINFO_OFFSET macro like this:
#define VMCOREINFO_OFFSET sizeof(AcpiTableHeader)
and then please refresh the documentation as well:
- replace decimal "40" with "36",
- replace 0x28 in the dumped SSDT with 0x24.
Namely, in order to suppress the OVMF ACP Table Header Probe -- "SDT"
simply means "System Description Table" --, it's enough to add
zero-padding that *precisely* covers such a header.
Given that we have a typedef for this header in QEMU, we should use it
in the definition of VMCOREINFO_OFFSET.
Now, the reason why VMGENID uses 40 instead comes from another
requirement: the VMGENID GUID has to be aligned at 8 bytes. (See
requirement "R1a" in "docs/specs/vmgenid.txt".) Therefore the directly
necessary 36 bytes of padding are rounded up to 40. See again
"docs/specs/vmgenid.txt":
----------------------------------+
| SSDT with OEM Table ID = VMGENID |
+----------------------------------+
| ... | TOP OF PAGE
| VGIA dword object ---------------|-----> +---------------------------+
| ... | | fw-allocated array for |
| _STA method referring to VGIA | | "etc/vmgenid_guid" |
| ... | +---------------------------+
| ADDR method referring to VGIA | | 0: OVMF SDT Header probe |
| ... | | suppressor |
+----------------------------------+ | 36: padding for 8-byte |
| alignment |
| 40: GUID |
| 56: padding to page size |
+---------------------------+
END OF PAGE
At offset 36 of "etc/vmgenid_guid", it says "padding for 8-byte alignment".
For VMCOREINFO, you don't need this extra alignment to 8 bytes, before
the "version" field is listed. So please make the VMCOREINFO_OFFSET
reflect what's actually necessary.
The rest of the code doesn't have to be modified (including your
experimental guest kernel driver); changing VMCOREINFO_OFFSET will
update all necessary locations automatically, including the generated
AML. Only the docs have to be synced manually.
... In fact, there *is* yet another location to update: the test case. I
suggest to include "hw/acpi/vmcoreinfo.h" in "tests/vmcoreinfo-test.c",
rather than open-code VMCOREINFO_OFFSET there. ... Oh wait: in the test
case you don't even use VMCOREINFO_OFFSET for anything. So just delete
the macro definition from that patch.
Thank you (and sorry about the churn),
Laszlo
- [Qemu-devel] [PATCH v2 0/7] KASLR kernel dump support, Marc-André Lureau, 2017/07/06
- [Qemu-devel] [PATCH v2 1/7] vmgenid: replace x-write-pointer-available hack, Marc-André Lureau, 2017/07/06
- [Qemu-devel] [PATCH v2 2/7] acpi: add vmcoreinfo device, Marc-André Lureau, 2017/07/06
- [Qemu-devel] [PATCH v2 3/7] tests: add simple vmcoreinfo test, Marc-André Lureau, 2017/07/06
- [Qemu-devel] [PATCH v2 4/7] dump: add vmcoreinfo ELF note, Marc-André Lureau, 2017/07/06
- [Qemu-devel] [PATCH v2 6/7] scripts/dump-guest-memory.py: add vmcoreinfo, Marc-André Lureau, 2017/07/06