[Qemu-devel] [PATCH v3 1/4] ACPI: Add APEI GHES Table Generation support

[gengdongjiu]

Laszlo,

  Thank you very much  for your carefully review again and again. I am
following your suggestion.


2017-06-03 20:01 GMT+08:00, Laszlo Ersek <lersek at redhat.com>:
> On 05/22/17 16:23, Laszlo Ersek wrote:
>> Keeping some context:
>>
>> On 05/12/17 23:00, Laszlo Ersek wrote:
>>> On 04/30/17 07:35, Dongjiu Geng wrote:
>
>> (68) In the code below, you are not taking an "OVMF header probe
>> suppressor" into account.
>>
>> But, we have already planned to replace that quirk with a separate,
>> dedicated allocation hint or command, so I'm not going to describe what
>> an "OVMF header probe suppressor" is; instead, I'll describe the
>> replacement for it.
>>
>> [...]
>
> So, the NOACPI allocation hint is a no-go at the moment, based on the
> discussion in the following threads:
>
> http://mid.mail-archive.com/20170601112241.2580-1-ard.biesheuvel@linaro.org
>
> http://mid.mail-archive.com/c76b36de-ebf9-c662-d454-0a95b43901e8@redhat.com
>
> Therefore the header probe suppression remains necessary.
>
> In this case, it is not hard to do, you just have to reorder the
> following two ADD_POINTER additions a bit:
>
>>>> +
>>>> +        bios_linker_loader_add_pointer(linker,
>>>> GHES_ERRORS_FW_CFG_FILE,
>>>> +                                sizeof(uint64_t) * i,
>>>> sizeof(uint64_t),
>>>> +                                GHES_ERRORS_FW_CFG_FILE,
>>>> +                                MAX_ERROR_SOURCE_COUNT_V6 *
>>>> sizeof(uint64_t) +
>>>> +                                i * MAX_RAW_DATA_LENGTH);
>
> This one should be moved out to a separate loop, after the current loop.
>
>>>> +        bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
>>>> +                    address_registers_offset
>>>> +                    + i * sizeof(AcpiGenericHardwareErrorSource),
>>>> +                    sizeof(uint32_t), GHES_ERRORS_FW_CFG_FILE,
>>>> +                    i * sizeof(uint64_t));
>
> This one should be kept in the first (i.e., current) loop.
>
> The idea is, when you first point the HEST/GHES_n entries in
> ACPI_BUILD_TABLE_FILE to the "address registers" in
> GHES_ERRORS_FW_CFG_FILE, all those address registers will still be
> zero-filled. This will fail the ACPI table header probe in
> OvmfPkg/AcpiPlatformDxe, which is what we want.
>
> After this is done, the address registers in GHES_ERRORS_FW_CFG_FILE
> should be pointed to the error status data blocks in the same fw_cfg
> blob, in a separate loop. (Those error status data blocks will again be
> zero-filled, so no ACPI table headers will be mistakenly recognized in
> them.)
>
> Thanks
> Laszlo
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
>