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

[Mathieu Poirier <mathieu.poirier@linaro.org>]

On Fri, Oct 02, 2026 at 10:53:58AM +1000, Gavin Shan wrote:
> 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?
>

It is redundant, at least at this time.  I have removed it in favour of
MACHINE(qdev_get_machine())->cgs where cpu->kvm_rme was used.  With that it was
possible to get rid of function kvm_arm_rme_vcpu_init().
 
> >       /* 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
>