From: Xiang Zheng <zhengxiang9@huawei.com>
To: Igor Mammedov <imammedo@redhat.com>
Cc: <pbonzini@redhat.com>, <mst@redhat.com>,
<shannon.zhaosl@gmail.com>, <peter.maydell@linaro.org>,
<lersek@redhat.com>, <james.morse@arm.com>,
<gengdongjiu@huawei.com>, <mtosatti@redhat.com>,
<rth@twiddle.net>, <ehabkost@redhat.com>,
<jonathan.cameron@huawei.com>, <xuwei5@huawei.com>,
<kvm@vger.kernel.org>, <qemu-devel@nongnu.org>,
<qemu-arm@nongnu.org>, <linuxarm@huawei.com>,
<wanghaibin.wang@huawei.com>
Subject: Re: [RESEND PATCH v21 2/6] docs: APEI GHES generation and CPER record description
Date: Wed, 27 Nov 2019 09:37:57 +0800 [thread overview]
Message-ID: <05d2ba81-501f-bd7e-8da4-73e413169688@huawei.com> (raw)
In-Reply-To: <20191115104458.200a6231@redhat.com>
Hi Igor,
Thanks for your review!
Since the series of patches are going to be merged, we will address your comments by follow up patches.
On 2019/11/15 17:44, Igor Mammedov wrote:
> On Mon, 11 Nov 2019 09:40:44 +0800
> Xiang Zheng <zhengxiang9@huawei.com> wrote:
>
>> From: Dongjiu Geng <gengdongjiu@huawei.com>
>>
>> Add APEI/GHES detailed design document
>>
>> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
>> Signed-off-by: Xiang Zheng <zhengxiang9@huawei.com>
>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>> ---
>> docs/specs/acpi_hest_ghes.rst | 95 +++++++++++++++++++++++++++++++++++
>> docs/specs/index.rst | 1 +
>> 2 files changed, 96 insertions(+)
>> create mode 100644 docs/specs/acpi_hest_ghes.rst
>>
>> diff --git a/docs/specs/acpi_hest_ghes.rst b/docs/specs/acpi_hest_ghes.rst
>> new file mode 100644
>> index 0000000000..348825f9d3
>> --- /dev/null
>> +++ b/docs/specs/acpi_hest_ghes.rst
>> @@ -0,0 +1,95 @@
>> +APEI tables generating and CPER record
>> +======================================
>> +
>> +..
>> + Copyright (c) 2019 HUAWEI TECHNOLOGIES CO., LTD.
>> +
>> + This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + See the COPYING file in the top-level directory.
>> +
>> +Design Details
>> +--------------
>> +
>> +::
>> +
>> + etc/acpi/tables etc/hardware_errors
>> + ==================== ==========================================
>> + + +--------------------------+ +-----------------------+
>> + | | HEST | | address | +--------------+
>> + | +--------------------------+ | registers | | Error Status |
>> + | | GHES1 | | +---------------------+ | Data Block 1 |
>> + | +--------------------------+ +--------->| |error_block_address1 |----------->| +------------+
>> + | | ................. | | | +---------------------+ | | CPER |
>> + | | error_status_address-----+-+ +------->| |error_block_address2 |--------+ | | CPER |
>> + | | ................. | | | +---------------------+ | | | .... |
>> + | | read_ack_register--------+-+ | | | .............. | | | | CPER |
>> + | | read_ack_preserve | | | +-----------------------+ | | +------------+
>> + | | read_ack_write | | | +----->| |error_block_addressN |------+ | | Error Status |
>> + + +--------------------------+ | | | | +---------------------+ | | | Data Block 2 |
>> + | | GHES2 | +-+-+----->| |read_ack_register1 | | +-->| +------------+
>> + + +--------------------------+ | | | +---------------------+ | | | CPER |
>> + | | ................. | | | +--->| |read_ack_register2 | | | | CPER |
>> + | | error_status_address-----+---+ | | | +---------------------+ | | | .... |
>> + | | ................. | | | | | ............. | | | | CPER |
>> + | | read_ack_register--------+-----+-+ | +---------------------+ | +-+------------+
>> + | | read_ack_preserve | | +->| |read_ack_registerN | | | |.......... |
>> + | | read_ack_write | | | | +---------------------+ | | +------------+
>> + + +--------------------------| | | | | Error Status |
>> + | | ............... | | | | | Data Block N |
>> + + +--------------------------+ | | +---->| +------------+
>> + | | GHESN | | | | | CPER |
>> + + +--------------------------+ | | | | CPER |
>> + | | ................. | | | | | .... |
>> + | | error_status_address-----+-----+ | | | CPER |
>> + | | ................. | | +-+------------+
>> + | | read_ack_register--------+---------+
>> + | | read_ack_preserve |
>> + | | read_ack_write |
>> + + +--------------------------+
>
> I'd merge "Error Status Data Block" with "address registers", so it would be
> clear that "Error Status Data Block" is located after "read_ack_registerN"
Yes, this image doesn't demonstrate this point. We will make some changes on
this image.
>
>> +
>> +(1) QEMU generates the ACPI HEST table. This table goes in the current
>> + "etc/acpi/tables" fw_cfg blob. Each error source has different
>> + notification types.
>> +
>> +(2) A new fw_cfg blob called "etc/hardware_errors" is introduced. QEMU
>> + also needs to populate this blob. The "etc/hardware_errors" fw_cfg blob
>> + contains an address registers table and an Error Status Data Block table.
>> +
>> +(3) The address registers table contains N Error Block Address entries
>> + and N Read Ack Register entries. The size for each entry is 8-byte.
>> + The Error Status Data Block table contains N Error Status Data Block
>> + entries. The size for each entry is 4096(0x1000) bytes. The total size
>> + for the "etc/hardware_errors" fw_cfg blob is (N * 8 * 2 + N * 4096) bytes.
>> + N is the number of the kinds of hardware error sources.
>> +
>> +(4) QEMU generates the ACPI linker/loader script for the firmware. The
>> + firmware pre-allocates memory for "etc/acpi/tables", "etc/hardware_errors"
>> + and copies blob contents there.
>> +
>> +(5) QEMU generates N ADD_POINTER commands, which patch addresses in the
>> + "error_status_address" fields of the HEST table with a pointer to the
>> + corresponding "address registers" in the "etc/hardware_errors" blob.
>> +
>> +(6) QEMU generates N ADD_POINTER commands, which patch addresses in the
>> + "read_ack_register" fields of the HEST table with a pointer to the
>> + corresponding "address registers" in the "etc/hardware_errors" blob.
>
> s/"address registers" in/"read_ack_register" within/
OK.
>
>> +
>> +(7) QEMU generates N ADD_POINTER commands for the firmware, which patch
>> + addresses in the "error_block_address" fields with a pointer to the
>> + respective "Error Status Data Block" in the "etc/hardware_errors" blob.
>> +
>> +(8) QEMU defines a third and write-only fw_cfg blob which is called
>> + "etc/hardware_errors_addr". Through that blob, the firmware can send back
>> + the guest-side allocation addresses to QEMU. The "etc/hardware_errors_addr"
>> + blob contains a 8-byte entry. QEMU generates a single WRITE_POINTER command
>> + for the firmware. The firmware will write back the start address of
>> + "etc/hardware_errors" blob to the fw_cfg file "etc/hardware_errors_addr".
>> +
>
>> +(9) When QEMU gets a SIGBUS from the kernel, QEMU formats the CPER right into
>> + guest memory,
>
> s/
> QEMU formats the CPER right into guest memory
> /
> QEMU writes CPER into corresponding "Error Status Data Block"
> /
>
OK.
>> and then injects platform specific interrupt (in case of
>> + arm/virt machine it's Synchronous External Abort) as a notification which
>> + is necessary for notifying the guest.
>
>
>> +
>> +(10) This notification (in virtual hardware) will be handled by the guest
>> + kernel, guest APEI driver will read the CPER which is recorded by QEMU and
>> + do the recovery.
> Maybe better would be to say:
> "
> On receiving notification, guest APEI driver cold read the CPER error
> and take appropriate action
> "
OK.
>
>
> also in HEST patches there is implicit ABI, which probably should be documented here.
> More specifically kvm_arch_on_sigbus_vcpu() error injection
> uses source_id as index in "etc/hardware_errors" to find out "Error Status Data Block"
> entry corresponding to error source. So supported source_id values should be assigned
> here and not be changed afterwards to make sure that guest will write error into
> expected "Error Status Data Block" even if guest was migrated to a newer QEMU.
>
OK, I will add the descriptions of the implicit ABI.
--
Thanks,
Xiang
next prev parent reply other threads:[~2019-11-27 1:38 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-11 1:40 [RESEND PATCH v21 0/6] Add ARMv8 RAS virtualization support in QEMU Xiang Zheng
2019-11-11 1:40 ` [RESEND PATCH v21 1/6] hw/arm/virt: Introduce a RAS machine option Xiang Zheng
2019-12-02 18:22 ` Peter Maydell
2019-12-07 12:10 ` gengdongjiu
2019-11-11 1:40 ` [RESEND PATCH v21 2/6] docs: APEI GHES generation and CPER record description Xiang Zheng
2019-11-15 9:44 ` Igor Mammedov
2019-11-27 1:37 ` Xiang Zheng [this message]
2019-11-27 8:12 ` Igor Mammedov
2019-11-11 1:40 ` [RESEND PATCH v21 3/6] ACPI: Add APEI GHES table generation support Xiang Zheng
2019-11-15 9:38 ` Igor Mammedov
2019-11-18 12:49 ` gengdongjiu
2019-11-18 13:18 ` gengdongjiu
2019-11-18 13:21 ` Michael S. Tsirkin
2019-11-18 13:57 ` gengdongjiu
2019-11-25 9:48 ` Igor Mammedov
2019-11-27 11:16 ` gengdongjiu
2019-11-22 15:44 ` Beata Michalska
2019-11-22 15:42 ` Beata Michalska
2019-11-25 9:23 ` Igor Mammedov
2019-11-11 1:40 ` [RESEND PATCH v21 4/6] KVM: Move hwpoison page related functions into kvm-all.c Xiang Zheng
2019-12-02 18:23 ` Peter Maydell
2019-11-11 1:40 ` [RESEND PATCH v21 5/6] target-arm: kvm64: handle SIGBUS signal from kernel or KVM Xiang Zheng
2019-11-15 16:37 ` Igor Mammedov
2019-11-22 15:47 ` Beata Michalska
2019-11-25 9:37 ` Igor Mammedov
2019-11-27 1:40 ` Xiang Zheng
2019-11-27 10:43 ` Igor Mammedov
2019-12-21 12:35 ` gengdongjiu
2019-11-22 15:47 ` Beata Michalska
2019-11-27 12:47 ` Xiang Zheng
2019-11-27 13:02 ` Igor Mammedov
2019-11-27 14:17 ` Beata Michalska
2019-12-03 3:35 ` Xiang Zheng
2019-11-27 14:17 ` Beata Michalska
2019-12-03 3:35 ` Xiang Zheng
2019-12-07 9:33 ` gengdongjiu
2019-12-09 13:05 ` Beata Michalska
2019-12-09 14:12 ` gengdongjiu
2019-11-11 1:40 ` [RESEND PATCH v21 6/6] MAINTAINERS: Add APCI/APEI/GHES entries Xiang Zheng
2019-12-02 18:27 ` [RESEND PATCH v21 0/6] Add ARMv8 RAS virtualization support in QEMU Peter Maydell
2019-12-03 2:09 ` gengdongjiu
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=05d2ba81-501f-bd7e-8da4-73e413169688@huawei.com \
--to=zhengxiang9@huawei.com \
--cc=ehabkost@redhat.com \
--cc=gengdongjiu@huawei.com \
--cc=imammedo@redhat.com \
--cc=james.morse@arm.com \
--cc=jonathan.cameron@huawei.com \
--cc=kvm@vger.kernel.org \
--cc=lersek@redhat.com \
--cc=linuxarm@huawei.com \
--cc=mst@redhat.com \
--cc=mtosatti@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=rth@twiddle.net \
--cc=shannon.zhaosl@gmail.com \
--cc=wanghaibin.wang@huawei.com \
--cc=xuwei5@huawei.com \
/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).