Re: [PATCH v3 14/18] KVM: arm64: Add per-EC entry/exit state marshalling for protected guests

[Fuad Tabba]

Hi Marc,

On Wed, 16 Sept 2026 at 17:27, Marc Zyngier <maz@xxxxxxxxxx> wrote:
[...]
> > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
[...]
> > +static void handle_pvm_entry_sys64(struct pkvm_hyp_vcpu *hyp_vcpu)
[...]
> > +        /* If the host emulated a read access, update the register */
> > +        if (pc_update &&
> > +            !pvm_sys64_is_write(hyp_vcpu->vcpu.arch.fault.esr_el2)) {
> > +                /* r0 as transfer register between the guest and the host. */
> > +                u64 rt_val = READ_ONCE(host_vcpu->arch.ctxt.regs.regs[0]);
>
> Why isn't this
>
>                 rt_val = READ_ONCE(vcpu_gp_regs(host_vcpu)->regs[0]);
>
> and similarly everywhere else?

Old code momentum. I'll use the accessors throughout in v4.

[...]
> > +static void handle_pvm_exit_sys64(struct pkvm_hyp_vcpu *hyp_vcpu)
> > +{
> > +        struct kvm_vcpu *host_vcpu = hyp_vcpu->host_vcpu;
> > +        u32 esr_el2 = hyp_vcpu->vcpu.arch.fault.esr_el2;
> > +
> > +        /* The mode is required for the host to emulate some sysregs */
> > +        host_vcpu->arch.ctxt.regs.pstate =
> > +                pvm_host_pstate(hyp_vcpu->vcpu.arch.ctxt.regs.pstate);
> > +
> > +        /* r0 as transfer register between the guest and the host. */
> > +        if (pvm_sys64_is_write(esr_el2)) {
> > +                int rt = kvm_vcpu_sys_get_rt(&hyp_vcpu->vcpu);
> > +                u64 rt_val = vcpu_get_reg(&hyp_vcpu->vcpu, rt);
> > +
> > +                host_vcpu->arch.ctxt.regs.regs[0] = rt_val;
>
> and this should be
>
>                 vcpu_set_reg(host_vcpu, 0, rt_val);
>
> assuming you don't need a WRITE_ONCE() to match the READ_ONCE() in the
> other direction.

No WRITE_ONCE() needed, EL2 is the only writer on the way out. I'll
switch to vcpu_set_reg().

[...]
> > +static void handle_pvm_exit_iabt(struct pkvm_hyp_vcpu *hyp_vcpu)
> > +{
> > +        hyp_vcpu->host_vcpu->arch.fault.hpfar_el2 =
> > +                hyp_vcpu->vcpu.arch.fault.hpfar_el2;
>
> Please keep assignments on a single line (everywhere).

Will do.

Cheers,
/fuad