Re: [RFC v4 06/24] target/arm/kvm-rme: Initialize vCPU

[Gavin Shan <gshan@redhat.com>]

On 9/4/26 5:35 AM, Mathieu Poirier wrote:
> From: Jean-Philippe Brucker <jean-philippe@linaro.org>
> 
> 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>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>   target/arm/cpu.h      |  3 +++
>   target/arm/kvm-rme.c  |  9 ++++++++
>   target/arm/kvm-stub.c |  5 ++++
>   target/arm/kvm.c      | 54 +++++++++++++++++++++++++++++++++++++++++--
>   target/arm/kvm_arm.h  | 10 ++++++++
>   5 files changed, 79 insertions(+), 2 deletions(-)
> 
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index e3f931dba26b..6bbc942f9a59 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -1054,6 +1054,9 @@ struct ArchCPU {
>       /* KVM steal time */
>       OnOffAuto kvm_steal_time;
>   
> +    /* Realm Management Extension */
> +    bool kvm_rme;
> +

This field seems redundant to MachineState::cgs since all vCPUs support RME
(Realm Management Extension) when MachineState::cgs points a valid instance.
So I'm wandering why we need to maintain the redundant information here?

>       /* Uniprocessor system with MP extensions */
>       bool mp_is_up;
>   
> diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c
> index a4a5e195c196..f7c2cc786e2c 100644
> --- a/target/arm/kvm-rme.c
> +++ b/target/arm/kvm-rme.c
> @@ -69,6 +69,15 @@ static int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error **errp)
>       return 0;
>   }
>   
> +void kvm_arm_rme_vcpu_init(ARMCPU *cpu)
> +{
> +    if (!rme_guest) {
> +        return;
> +    }
> +
> +    cpu->kvm_rme = true;
> +}
> +

This could be simplified to:

       cpu->kvm_rme = !!rme_guest;

>   static void rme_guest_class_init(ObjectClass *oc, const void *data)
>   {
>       ConfidentialGuestSupportClass *klass = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
> diff --git a/target/arm/kvm-stub.c b/target/arm/kvm-stub.c
> index 88cbe8d85c41..5fde96f9b281 100644
> --- a/target/arm/kvm-stub.c
> +++ b/target/arm/kvm-stub.c
> @@ -119,3 +119,8 @@ char *kvm_print_register_name(uint64_t regidx)
>   {
>       g_assert_not_reached();
>   }
> +
> +void kvm_arm_rme_vcpu_init(ARMCPU *cpu)
> +{
> +    g_assert_not_reached();
> +}
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 33c3763aebfa..b4c6da1fb9f3 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -2015,6 +2015,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
>           cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
>       }
>   
> +    kvm_arm_rme_vcpu_init(cpu);
> +
>       /* Do KVM_ARM_VCPU_INIT ioctl */
>       ret = kvm_arm_vcpu_init(cpu);
>       if (ret) {
> @@ -2169,6 +2171,29 @@ static int kvm_arch_put_sve(CPUState *cs, uint32_t vq, bool have_ffr)
>       return 0;
>   }
>   
> +static int kvm_arm_rme_put_core_regs(CPUState *cs, Error **errp)
> +{
> +    int i, ret;
> +    ARMCPU *cpu = ARM_CPU(cs);
> +    CPUARMState *env = &cpu->env;
> +
> +    /* The RME ABI only allows us to set 8 GPRs and the PC */
> +    for (i = 0; i < 8; i++) {
> +        ret = kvm_set_one_reg(cs, AARCH64_CORE_REG(regs.regs[i]),
> +                              &env->xregs[i]);
> +        if (ret) {
> +            return ret;
> +        }
> +    }
> +
> +    ret = kvm_set_one_reg(cs, AARCH64_CORE_REG(regs.pc), &env->pc);
> +    if (ret) {
> +        return ret;
> +    }
> +
> +    return 0;
> +}
> +
>   static int kvm_arm_put_core_regs(CPUState *cs, Error **errp)
>   {
>       uint64_t val;
> @@ -2280,7 +2305,11 @@ int kvm_arch_put_registers(CPUState *cs, KvmPutState level, Error **errp)
>       int ret;
>       ARMCPU *cpu = ARM_CPU(cs);
>   
> -    ret = kvm_arm_put_core_regs(cs, errp);
> +    if (cpu->kvm_rme) {
> +        ret = kvm_arm_rme_put_core_regs(cs, errp);
> +    } else {
> +        ret = kvm_arm_put_core_regs(cs, errp);
> +    }
>       if (ret) {
>           return ret;
>       }
> @@ -2367,6 +2396,23 @@ static int kvm_arch_get_sve(CPUState *cs, uint32_t vq, bool have_ffr)
>       return 0;
>   }
>   
> +static int kvm_arm_rme_get_core_regs(CPUState *cs, Error **errp)
> +{
> +    int i, ret;
> +    ARMCPU *cpu = ARM_CPU(cs);
> +    CPUARMState *env = &cpu->env;
> +
> +    for (i = 0; i < 8; i++) {
> +        ret = kvm_get_one_reg(cs, AARCH64_CORE_REG(regs.regs[i]),
> +                              &env->xregs[i]);
> +        if (ret) {
> +            return ret;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
>   static int kvm_arm_get_core_regs(CPUState *cs, Error **errp)
>   {
>       uint64_t val;
> @@ -2478,7 +2524,11 @@ int kvm_arch_get_registers(CPUState *cs, Error **errp)
>       int ret;
>       ARMCPU *cpu = ARM_CPU(cs);
>   
> -    ret = kvm_arm_get_core_regs(cs, errp);
> +    if (cpu->kvm_rme) {
> +        ret = kvm_arm_rme_get_core_regs(cs, errp);
> +    } else {
> +        ret = kvm_arm_get_core_regs(cs, errp);
> +    }
>       if (ret) {
>           return ret;
>       }
> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> index e7c40fb003e4..d95381c13afa 100644
> --- a/target/arm/kvm_arm.h
> +++ b/target/arm/kvm_arm.h
> @@ -240,4 +240,14 @@ void arm_gic_cap_kvm_probe(GICCapability *v2, GICCapability *v3);
>    */
>   char *kvm_print_register_name(uint64_t regidx);
>   
> +/**
> + * 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.
> + */
> +void kvm_arm_rme_vcpu_init(ARMCPU *cpu);
> +

The comments here look outdated. This function simply set ARMCPU::kvm_rme to indicate
RME should be supported for this given vCPU. So the comments would be something as
below.

     /*
      *
      * Mark that RME (Realm Management Extension) should be supported for the given
      * vCPU if user has requested it. Realm vCPUs behave a little differently, for
      * example most of their register states are confidential and invisible to us.
      */

>   #endif

Thanks,
Gavin