Hi Laszlo,
   sorry for my late response.

On 2017/6/3 20:01, Laszlo Ersek wrote:

On 05/22/17 16:23, Laszlo Ersek wrote:

quoted

Keeping some context:

On 05/12/17 23:00, Laszlo Ersek wrote:

quoted

On 04/30/17 07:35, Dongjiu Geng wrote:

quoted

(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 at linaro.org

http://mid.mail-archive.com/c76b36de-ebf9-c662-d454-0a95b43901e8 at 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:

 Ok, it is no problem.

quoted

quoted

quoted

+
+        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.

quoted

quoted

quoted

+        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.)

I understand your idear. but I have a question:
how about we exchange the two function's place, such as shown below:
then it still meets ours needs, the change is easy.
For every loop:
(1)when patch address in ACPI_BUILD_TABLE_FILE to the "address registers", the address register is zero-filed.
(2)when patch address in GHES_ERRORS_FW_CFG_FILE to the error status data blocks, the error status data block is still zero-filed.

    for (i = 0; i < GHES_ACPI_HEST_NOTIFY_RESERVED; i++) {
        .....................................
        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));


        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);
	.........................................

     }

thanks

Thanks
Laszlo


.