Re: [RFC PATCH 06/16] target/arm/kvm-rme: Initialize vCPU

[Richard Henderson]

On 1/27/23 05:07, Jean-Philippe Brucker wrote:
> The target code calls kvm_arm_vcpu_init() to mark the vCPU as part of a
> realm. RME support does not use the register lists, because the host can
> only set the boot PC and registers x0-x7. The rest is private to the
> Realm and saved/restored by the RMM.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>   target/arm/cpu.h     |  3 ++
>   target/arm/kvm_arm.h |  1 +
>   target/arm/helper.c  |  8 ++++++
>   target/arm/kvm-rme.c | 10 +++++++
>   target/arm/kvm.c     | 12 ++++++++
>   target/arm/kvm64.c   | 65 ++++++++++++++++++++++++++++++++++++++++++--
>   6 files changed, 97 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 9aeed3c848..7d8397985f 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -937,6 +937,9 @@ struct ArchCPU {
>       /* KVM steal time */
>       OnOffAuto kvm_steal_time;
>   
> +    /* 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 00d3df8cac..e4dc7fbb8d 100644
> --- a/target/arm/kvm_arm.h
> +++ b/target/arm/kvm_arm.h
> @@ -373,6 +373,7 @@ int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error 
> **errp);
>   int kvm_arm_rme_vm_type(MachineState *ms);
>   
>   bool kvm_arm_rme_enabled(void);
> +int kvm_arm_rme_vcpu_init(CPUState *cs);
>   
>   #else
>   
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index d8c8223ec3..52360ae2ff 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -126,6 +126,10 @@ bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync)
>       int i;
>       bool ok = true;
>   
> +    if (cpu->kvm_rme) {
> +        return ok;
> +    }
> +
>       for (i = 0; i < cpu->cpreg_array_len; i++) {
>           uint32_t regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]);
>           const ARMCPRegInfo *ri;
> @@ -171,6 +175,10 @@ bool write_list_to_cpustate(ARMCPU *cpu)
>       int i;
>       bool ok = true;
>   
> +    if (cpu->kvm_rme) {
> +        return ok;
> +    }
> +
>       for (i = 0; i < cpu->cpreg_array_len; i++) {
>           uint32_t regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]);
>           uint64_t v = cpu->cpreg_values[i];
> diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c
> index d7cdca1cbf..3833b187f9 100644
> --- a/target/arm/kvm-rme.c
> +++ b/target/arm/kvm-rme.c
> @@ -118,6 +118,16 @@ int kvm_arm_rme_init(ConfidentialGuestSupport *cgs, Error 
> **errp)
>       return 0;
>   }
>   
> +int kvm_arm_rme_vcpu_init(CPUState *cs)
> +{
> +    ARMCPU *cpu = ARM_CPU(cs);
> +
> +    if (kvm_arm_rme_enabled()) {
> +        cpu->kvm_rme = true;
> +    }
> +    return 0;
> +}
> +
>   int kvm_arm_rme_vm_type(MachineState *ms)
>   {
>       if (cgs_to_rme(ms->cgs)) {
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index f022c644d2..fcddead4fe 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -449,6 +449,10 @@ int kvm_arm_init_cpreg_list(ARMCPU *cpu)
>       int i, ret, arraylen;
>       CPUState *cs = CPU(cpu);
>   
> +    if (cpu->kvm_rme) {
> +        return 0;
> +    }
> +
>       rl.n = 0;
>       ret = kvm_vcpu_ioctl(cs, KVM_GET_REG_LIST, &rl);
>       if (ret != -E2BIG) {
> @@ -521,6 +525,10 @@ bool write_kvmstate_to_list(ARMCPU *cpu)
>       int i;
>       bool ok = true;
>   
> +    if (cpu->kvm_rme) {
> +        return ok;
> +    }
> +
>       for (i = 0; i < cpu->cpreg_array_len; i++) {
>           struct kvm_one_reg r;
>           uint64_t regidx = cpu->cpreg_indexes[i];
> @@ -557,6 +565,10 @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level)
>       int i;
>       bool ok = true;
>   
> +    if (cpu->kvm_rme) {
> +        return ok;
> +    }

I don't think that simply returning "ok" is best.  We shouldn't be calling this function 
at all with rme enabled.


r~