diff mbox

[v3,RFC,2/2] target: arm: Add support for VCPU event states

Message ID 1528403460-30526-3-git-send-email-gengdongjiu@huawei.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dongjiu Geng June 7, 2018, 8:31 p.m. UTC 
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 <gengdongjiu@huawei.com>
---
 target/arm/cpu.h     |  5 ++++
 target/arm/kvm64.c   | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 target/arm/machine.c |  3 +++
 3 files changed, 72 insertions(+)

Comments

Peter Maydell June 20, 2018, 4:56 p.m. UTC | #1 
On 7 June 2018 at 21:31, Dongjiu Geng <gengdongjiu@huawei.com> 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 <gengdongjiu@huawei.com>
> ---
>  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?

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

thanks
-- PMM
Dongjiu Geng June 21, 2018, 11:59 a.m. UTC | #2 
Hi peter,
   Thanks for the review.

On 2018/6/21 0:56, Peter Maydell wrote:
> On 7 June 2018 at 21:31, Dongjiu Geng <gengdongjiu@huawei.com> 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 <gengdongjiu@huawei.com>
>> ---
>>  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
> 
> .
>
diff mbox

Patch

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;
 } CPUARMState;
 
 /**
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index e0b8246..45b6911 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -600,6 +600,59 @@  int kvm_arm_cpreg_level(uint64_t regidx)
 #define AARCH64_SIMD_CTRL_REG(x)   (KVM_REG_ARM64 | KVM_REG_SIZE_U32 | \
                  KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
 
+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;
+}
+
+
+static int kvm_put_vcpu_events(ARMCPU *cpu)
+{
+    CPUARMState *env = &cpu->env;
+    struct kvm_vcpu_events events = {};
+
+    if (!kvm_has_vcpu_events()) {
+        return 0;
+    }
+
+    memset(&events, 0, sizeof(events));
+    events.exception.serror_pending = env->serror.pending;
+
+    if (kvm_can_set_vcpu_esr(cpu)) {
+        events.exception.serror_has_esr = env->serror.has_esr;
+        events.exception.serror_esr = env->serror.esr;
+    }
+
+    return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_VCPU_EVENTS, &events);
+}
+
+static int kvm_get_vcpu_events(ARMCPU *cpu)
+{
+    CPUARMState *env = &cpu->env;
+    struct kvm_vcpu_events events;
+    int ret;
+
+    if (!kvm_has_vcpu_events()) {
+        return 0;
+    }
+
+    memset(&events, 0, sizeof(events));
+    ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_VCPU_EVENTS, &events);
+
+    if (ret < 0) {
+        return ret;
+    }
+
+    env->serror.pending = events.exception.serror_pending;
+    env->serror.has_esr = events.exception.serror_has_esr;
+    env->serror.esr = events.exception.serror_esr;
+
+    return 0;
+}
+
 int kvm_arch_put_registers(CPUState *cs, int level)
 {
     struct kvm_one_reg reg;
@@ -727,6 +780,12 @@  int kvm_arch_put_registers(CPUState *cs, int level)
         return ret;
     }
 
+    ret = kvm_put_vcpu_events(cpu);
+    if (ret) {
+        printf("return error kvm_put_vcpu_events: %d\n", ret);
+        return ret;
+    }
+
     if (!write_list_to_kvmstate(cpu, level)) {
         return EINVAL;
     }
@@ -863,6 +922,11 @@  int kvm_arch_get_registers(CPUState *cs)
     }
     vfp_set_fpcr(env, fpr);
 
+    ret = kvm_get_vcpu_events(cpu);
+    if (ret) {
+        return ret;
+    }
+
     if (!write_kvmstate_to_list(cpu)) {
         return EINVAL;
     }
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),
         {