Re: [PATCH v3 06/26] target/arm/kvm-rme: Initialize vCPU

[Gavin Shan <gshan@redhat.com>]

On 11/26/24 5:56 AM, Jean-Philippe Brucker wrote:
> The target code calls kvm_arm_vcpu_init() to mark the vCPU as part of a
> Realm. For a Realm vCPU, only x0-x7 can be set at runtime. Before boot,
> the PC can also be set, and is ignored at runtime. KVM also accepts a
> few system register changes during initial configuration, as returned by
> KVM_GET_REG_LIST.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>   target/arm/cpu.h     |  3 +++
>   target/arm/kvm_arm.h | 15 +++++++++++
>   target/arm/kvm-rme.c | 10 ++++++++
>   target/arm/kvm.c     | 61 ++++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 89 insertions(+)
> 
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index d86e641280..f617591921 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -961,6 +961,9 @@ struct ArchCPU {
>       OnOffAuto kvm_steal_time;
>   #endif /* CONFIG_KVM */
>   
> +    /* Realm Management Extension */
> +    bool kvm_rme;
> +
>       /* Uniprocessor system with MP extensions */
>       bool mp_is_up;
>   
> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> index 9d6a89f9b1..8b52a881b0 100644
> --- a/target/arm/kvm_arm.h
> +++ b/target/arm/kvm_arm.h
> @@ -245,6 +245,16 @@ int kvm_arm_rme_init(MachineState *ms);
>    */
>   int kvm_arm_rme_vm_type(MachineState *ms);
>   
> +/**
> + * kvm_arm_rme_vcpu_init
> + * @cs: the CPU
> + *
> + * If the user requested a Realm, setup the given vCPU accordingly. Realm vCPUs
> + * behave a little differently, for example most of their register state is
> + * hidden from the host.
> + */
> +int kvm_arm_rme_vcpu_init(CPUState *cs);
> +
>   #else
>   
>   /*
> @@ -339,6 +349,11 @@ static inline int kvm_arm_rme_vm_type(MachineState *ms)
>       g_assert_not_reached();
>   }
>   
> +static inline int kvm_arm_rme_vcpu_init(CPUState *cs)
> +{
> +    g_assert_not_reached();
> +}
> +
>   #endif
>   
>   #endif
> diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c
> index 60d967a842..e3cc37538a 100644
> --- a/target/arm/kvm-rme.c
> +++ b/target/arm/kvm-rme.c
> @@ -137,6 +137,16 @@ int kvm_arm_rme_init(MachineState *ms)
>       return 0;
>   }
>   
> +int kvm_arm_rme_vcpu_init(CPUState *cs)
> +{
> +    ARMCPU *cpu = ARM_CPU(cs);
> +
> +    if (rme_guest) {
> +        cpu->kvm_rme = true;
> +    }
> +    return 0;
> +}
> +
>   int kvm_arm_rme_vm_type(MachineState *ms)
>   {
>       if (rme_guest) {
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 0c80992f7c..a0de2efc41 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -1926,6 +1926,11 @@ int kvm_arch_init_vcpu(CPUState *cs)
>           return ret;
>       }
>   
> +    ret = kvm_arm_rme_vcpu_init(cs);
> +    if (ret) {
> +        return ret;
> +    }
> +
>       if (cpu_isar_feature(aa64_sve, cpu)) {
>           ret = kvm_arm_sve_set_vls(cpu);
>           if (ret) {
> @@ -2062,6 +2067,35 @@ static int kvm_arch_put_sve(CPUState *cs)
>       return 0;
>   }
>   
> +static int kvm_arm_rme_put_core_regs(CPUState *cs, Error **errp)
> +{
> +    int i, ret;
> +    struct kvm_one_reg reg;
> +    ARMCPU *cpu = ARM_CPU(cs);
> +    CPUARMState *env = &cpu->env;
> +
> +    /*
> +     * The RME ABI only allows us to set 8 GPRs and the PC
> +     */

Needn't to span for multiple lines.

> +    for (i = 0; i < 8; i++) {
> +        reg.id = AARCH64_CORE_REG(regs.regs[i]);
> +        reg.addr = (uintptr_t) &env->xregs[i];
> +        ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
> +        if (ret) {
> +            return ret;
> +        }
> +    }
> +
> +    reg.id = AARCH64_CORE_REG(regs.pc);
> +    reg.addr = (uintptr_t) &env->pc;
> +    ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
> +    if (ret) {
> +        return ret;
> +    }
> +
> +    return 0;
> +}
> +

Nice place to use kvm_set_one_reg(). With it, @reg can be dropped.

>   static int kvm_arm_put_core_regs(CPUState *cs, int level, Error **errp)
>   {
>       uint64_t val;
> @@ -2072,6 +2106,10 @@ static int kvm_arm_put_core_regs(CPUState *cs, int level, Error **errp)
>       ARMCPU *cpu = ARM_CPU(cs);
>       CPUARMState *env = &cpu->env;
>   
> +    if (cpu->kvm_rme) {
> +        return kvm_arm_rme_put_core_regs(cs, errp);
> +    }
> +
>       /* If we are in AArch32 mode then we need to copy the AArch32 regs to the
>        * AArch64 registers before pushing them out to 64-bit KVM.
>        */
> @@ -2259,6 +2297,25 @@ static int kvm_arch_get_sve(CPUState *cs)
>       return 0;
>   }
>   
> +static int kvm_arm_rme_get_core_regs(CPUState *cs, Error **errp)
> +{
> +    int i, ret;
> +    struct kvm_one_reg reg;
> +    ARMCPU *cpu = ARM_CPU(cs);
> +    CPUARMState *env = &cpu->env;
> +
> +    for (i = 0; i < 8; i++) {
> +        reg.id = AARCH64_CORE_REG(regs.regs[i]);
> +        reg.addr = (uintptr_t) &env->xregs[i];
> +        ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
> +        if (ret) {
> +            return ret;
> +        }
> +    }
> +
> +    return 0;
> +}
> +

Similiarly, kvm_get_one_reg() can be used.

>   static int kvm_arm_get_core_regs(CPUState *cs, Error **errp)
>   {
>       uint64_t val;
> @@ -2269,6 +2326,10 @@ static int kvm_arm_get_core_regs(CPUState *cs, Error **errp)
>       ARMCPU *cpu = ARM_CPU(cs);
>       CPUARMState *env = &cpu->env;
>   
> +    if (cpu->kvm_rme) {
> +        return kvm_arm_rme_get_core_regs(cs, errp);
> +    }
> +
>       for (i = 0; i < 31; i++) {
>           ret = kvm_get_one_reg(cs, AARCH64_CORE_REG(regs.regs[i]),
>                                 &env->xregs[i]);

Thanks,
Gavin