Re: [PATCH v19 5/5] target-arm: kvm64: handle SIGBUS signal from kernel or KVM

2019-10-17 Thread Xiang Zheng



On 2019/10/15 22:48, Peter Maydell wrote:
> On Tue, 15 Oct 2019 at 15:02, Xiang Zheng  wrote:
>>
>> From: Dongjiu Geng 
>>
>> Add a SIGBUS signal handler. In this handler, it checks the SIGBUS type,
>> translates the host VA delivered by host to guest PA, then fills this PA
>> to guest APEI GHES memory, then notifies guest according to the SIGBUS
>> type.
>>
>> When guest accesses the poisoned memory, it will generate a Synchronous
>> External Abort(SEA). Then host kernel gets an APEI notification and calls
>> memory_failure() to unmapped the affected page in stage 2, finally
>> returns to guest.
>>
>> Guest continues to access the PG_hwpoison page, it will trap to KVM as
>> stage2 fault, then a SIGBUS_MCEERR_AR synchronous signal is delivered to
>> Qemu, Qemu records this error address into guest APEI GHES memory and
>> notifes guest using Synchronous-External-Abort(SEA).
>>
>> In order to inject a vSEA, we introduce the kvm_inject_arm_sea() function
>> in which we can setup the type of exception and the syndrome information.
>> When switching to guest, the target vcpu will jump to the synchronous
>> external abort vector table entry.
>>
>> The ESR_ELx.DFSC is set to synchronous external abort(0x10), and the
>> ESR_ELx.FnV is set to not valid(0x1), which will tell guest that FAR is
>> not valid and hold an UNKNOWN value. These values will be set to KVM
>> register structures through KVM_SET_ONE_REG IOCTL.
>>
>> Signed-off-by: Dongjiu Geng 
>> Signed-off-by: Xiang Zheng 
> 
>> +static int acpi_ghes_record_mem_error(uint64_t error_block_address,
>> +  uint64_t error_physical_addr,
>> +  uint32_t data_length)
>> +{
>> +GArray *block;
>> +uint64_t current_block_length;
>> +/* Memory Error Section Type */
>> +QemuUUID mem_section_id_le = UEFI_CPER_SEC_PLATFORM_MEM;
>> +QemuUUID fru_id = {0};
> 
> Hi; this makes at least some versions of clang complain
> (this is a clang bug, but it's present in shipped versions):
> 
> /home/petmay01/linaro/qemu-from-laptop/qemu/hw/acpi/acpi_ghes.c:135:24:
> error: suggest braces around
>   initialization of subobject [-Werror,-Wmissing-braces]
> QemuUUID fru_id = {0};
>^
>{}
> 
> We generally use "{}" as the generic zero-initializer for
> this reason (it's gcc/clang specific whereas "{0}" is
> in the standard, but all of the compilers we care about
> support it and don't warn about its use).
> 
>> +uint8_t fru_text[20] = {0};
> 
> Clang doesn't mind this one because it's not initializing
> a struct type, but you could use "{}" here too for consistency.
> 

OK, I will replace all the "{0}" with "{}".

> thanks
> -- PMM
> 
> .
> 

-- 

Thanks,
Xiang




Re: [PATCH v19 5/5] target-arm: kvm64: handle SIGBUS signal from kernel or KVM

2019-10-15 Thread Peter Maydell
On Tue, 15 Oct 2019 at 15:02, Xiang Zheng  wrote:
>
> From: Dongjiu Geng 
>
> Add a SIGBUS signal handler. In this handler, it checks the SIGBUS type,
> translates the host VA delivered by host to guest PA, then fills this PA
> to guest APEI GHES memory, then notifies guest according to the SIGBUS
> type.
>
> When guest accesses the poisoned memory, it will generate a Synchronous
> External Abort(SEA). Then host kernel gets an APEI notification and calls
> memory_failure() to unmapped the affected page in stage 2, finally
> returns to guest.
>
> Guest continues to access the PG_hwpoison page, it will trap to KVM as
> stage2 fault, then a SIGBUS_MCEERR_AR synchronous signal is delivered to
> Qemu, Qemu records this error address into guest APEI GHES memory and
> notifes guest using Synchronous-External-Abort(SEA).
>
> In order to inject a vSEA, we introduce the kvm_inject_arm_sea() function
> in which we can setup the type of exception and the syndrome information.
> When switching to guest, the target vcpu will jump to the synchronous
> external abort vector table entry.
>
> The ESR_ELx.DFSC is set to synchronous external abort(0x10), and the
> ESR_ELx.FnV is set to not valid(0x1), which will tell guest that FAR is
> not valid and hold an UNKNOWN value. These values will be set to KVM
> register structures through KVM_SET_ONE_REG IOCTL.
>
> Signed-off-by: Dongjiu Geng 
> Signed-off-by: Xiang Zheng 

> +static int acpi_ghes_record_mem_error(uint64_t error_block_address,
> +  uint64_t error_physical_addr,
> +  uint32_t data_length)
> +{
> +GArray *block;
> +uint64_t current_block_length;
> +/* Memory Error Section Type */
> +QemuUUID mem_section_id_le = UEFI_CPER_SEC_PLATFORM_MEM;
> +QemuUUID fru_id = {0};

Hi; this makes at least some versions of clang complain
(this is a clang bug, but it's present in shipped versions):

/home/petmay01/linaro/qemu-from-laptop/qemu/hw/acpi/acpi_ghes.c:135:24:
error: suggest braces around
  initialization of subobject [-Werror,-Wmissing-braces]
QemuUUID fru_id = {0};
   ^
   {}

We generally use "{}" as the generic zero-initializer for
this reason (it's gcc/clang specific whereas "{0}" is
in the standard, but all of the compilers we care about
support it and don't warn about its use).

> +uint8_t fru_text[20] = {0};

Clang doesn't mind this one because it's not initializing
a struct type, but you could use "{}" here too for consistency.

thanks
-- PMM