Re: [PATCH v3 4/8] acpi/ghes: Extend acpi_ghes_memory_errors() to support multiple CPERs

[Gavin Shan <gshan@redhat.com>]

Hi Philippe,

On 11/11/25 9:38 AM, Gavin Shan wrote:
> On 11/11/25 12:43 AM, Philippe Mathieu-Daudé wrote:
>> On 5/11/25 12:44, Gavin Shan wrote:
>>> In the situation where host and guest has 64KiB and 4KiB page sizes,
>>> one problematic host page affects 16 guest pages. we need to send 16
>>> consective errors in this specific case.
>>>
>>> Extend acpi_ghes_memory_errors() to support multiple CPERs after the
>>> hunk of code to generate the GHES error status is pulled out from
>>> ghes_gen_err_data_uncorrectable_recoverable(). The status field of
>>> generic error status block is also updated accordingly if multiple
>>> error data entries are contained in the generic error status block.
>>>
>>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>>> ---
>>>   hw/acpi/ghes-stub.c    |  2 +-
>>>   hw/acpi/ghes.c         | 60 +++++++++++++++++++++++-------------------
>>>   include/hw/acpi/ghes.h |  2 +-
>>>   target/arm/kvm.c       |  4 ++-
>>>   4 files changed, 38 insertions(+), 30 deletions(-)
>>
>>
>>>   int acpi_ghes_memory_errors(AcpiGhesState *ags, uint16_t source_id,
>>> -                            uint64_t physical_address)
>>> +                            uint64_t *addresses, uint32_t num_of_addresses)
>>>   {
>>>       /* Memory Error Section Type */
>>>       const uint8_t guid[] =
>>>             UUID_LE(0xA5BC1114, 0x6F64, 0x4EDE, 0xB8, 0x63, 0x3E, 0x83, \
>>>                     0xED, 0x7C, 0x83, 0xB1);
>>> +    /*
>>> +     * invalid fru id: ACPI 4.0: 17.3.2.6.1 Generic Error Data,
>>> +     * Table 17-13 Generic Error Data Entry
>>> +     */
>>> +    QemuUUID fru_id = {};
>>>       Error *errp = NULL;
>>>       int data_length;
>>>       GArray *block;
>>> +    uint32_t block_status, i;
>>>       block = g_array_new(false, true /* clear */, 1);
>>> -    data_length = ACPI_GHES_DATA_LENGTH + ACPI_GHES_MEM_CPER_LENGTH;
>>> +    data_length = num_of_addresses *
>>> +                  (ACPI_GHES_DATA_LENGTH + ACPI_GHES_MEM_CPER_LENGTH);
>>
>> Should we check num_of_addresses is in range?
>>
> 
> The check is already done by the following assert().
> 
>     /*
>       * It should not run out of the preallocated memory if adding a new generic
>       * error data entry
>       */
>      assert((data_length + ACPI_GHES_GESB_SIZE) <=
>              ACPI_GHES_MAX_RAW_DATA_LENGTH);
> 

I may have misunderstood your point. We probably need to ensure @num_of_addresses
doesn't overflow the bit fields (Bit#0 - Bit#10), as Igor suggested in another
reply.

Thanks,
Gavin