[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH v3 RFC 2/2] target: arm: Add support for VCPU even
From: |
gengdongjiu |
Subject: |
Re: [Qemu-arm] [PATCH v3 RFC 2/2] target: arm: Add support for VCPU event states |
Date: |
Thu, 21 Jun 2018 19:59:01 +0800 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
Hi peter,
Thanks for the review.
On 2018/6/21 0:56, Peter Maydell wrote:
> On 7 June 2018 at 21:31, Dongjiu Geng <address@hidden> wrote:
>> This patch extends the qemu-kvm state sync logic with support for
>> KVM_GET/SET_VCPU_EVENTS, giving access to yet missing SError exception.
>> And also it can support the exception state migration.
>>
>> Signed-off-by: Dongjiu Geng <address@hidden>
>> ---
>> target/arm/cpu.h | 5 ++++
>> target/arm/kvm64.c | 64
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> target/arm/machine.c | 3 +++
>> 3 files changed, 72 insertions(+)
>>
>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>> index 8488273..b3d6682 100644
>> --- a/target/arm/cpu.h
>> +++ b/target/arm/cpu.h
>> @@ -645,6 +645,11 @@ typedef struct CPUARMState {
>> const struct arm_boot_info *boot_info;
>> /* Store GICv3CPUState to access from this struct */
>> void *gicv3state;
>> + struct {
>> + uint32_t pending;
>> + uint32_t has_esr;
>> + uint64_t esr;
>> + } serror;
>
> What is this state? That is, what guest CPU architectural state
> is it supposed to correspond to?
The guest CPU architectural can be arm64 CPU with RAS extension. so before
kvm_put_vcpu_events() sets the vcpu events, it firstly checks whether CPU
architectural support
RAS extension through kvm_can_set_vcpu_esr().
---------------------------------------------------------------------
static int kvm_put_vcpu_events(ARMCPU *cpu)
{
........................................
if (kvm_can_set_vcpu_esr(cpu)) {
events.exception.serror_has_esr = env->serror.has_esr;
events.exception.serror_esr = env->serror.esr;
}
...............................................
}
------------------------------------------------------------------
>
>> diff --git a/target/arm/machine.c b/target/arm/machine.c
>> index 2e28d08..5a359f4 100644
>> --- a/target/arm/machine.c
>> +++ b/target/arm/machine.c
>> @@ -695,6 +695,9 @@ const VMStateDescription vmstate_arm_cpu = {
>> VMSTATE_UINT32(env.exception.syndrome, ARMCPU),
>> VMSTATE_UINT32(env.exception.fsr, ARMCPU),
>> VMSTATE_UINT64(env.exception.vaddress, ARMCPU),
>> + VMSTATE_UINT32(env.serror.pending, ARMCPU),
>> + VMSTATE_UINT32(env.serror.has_esr, ARMCPU),
>> + VMSTATE_UINT64(env.serror.esr, ARMCPU),
>> VMSTATE_TIMER_PTR(gt_timer[GTIMER_PHYS], ARMCPU),
>> VMSTATE_TIMER_PTR(gt_timer[GTIMER_VIRT], ARMCPU),
>> {
>
> You can't just add fields like this, as it breaks migration
> compatibility. If these need to be migrated then you need a
> new subsection with a suitable 'needed' function controlling
> whether it is present. But we should determine first
> whether this really is new guest CPU state...
I know your means. I will modify it according to the patch[1], which is used to
do SVE migration.
Now I use this API kvm_can_set_vcpu_esr() to check the new guest CPU state. The
kvm_can_set_vcpu_esr()
will check whether CPU architectural supports RAS extension.
-------------------------------------------------------------
static bool kvm_can_set_vcpu_esr(ARMCPU *cpu)
{
CPUState *cs = CPU(cpu);
int ret = kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_INJECT_SERROR_ESR);
return (ret) ? true : false;
}
-----------------------------------------------------------------
[1]:
https://git.qemu.org/?p=qemu.git;a=commitdiff;h=ef401601d5561f9805102695d5e65d72594f7020
>
> thanks
> -- PMM
>
> .
>