Re: [PATCH v3 15/18] KVM: arm64: Reject host access to protected VM private state

[Will Deacon <will@kernel.org>]

Hi Marc, Fuad,

On Thu, Sep 17, 2026 at 09:06:25AM +0100, Marc Zyngier wrote:
> On Wed, 16 Sep 2026 20:07:28 +0100,
> Fuad Tabba <fuad.tabba@linux.dev> wrote:
> > On Wed, 16 Sept 2026 at 17:30, Marc Zyngier <maz@kernel.org> wrote:
> > [...]
> > > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> > [...]
> > > > +/*
> > > > + * Once a protected vCPU has run, the host copy is not the guest's state,
> > > > + * and EL2 has read mp_state, which it does only at hyp vCPU creation.
> > > > + */
> > > > +static long pkvm_filter_vcpu_ioctl(struct kvm_vcpu *vcpu, unsigned int ioctl)
> > > > +{
> > > > +        switch (ioctl) {
> > > > +        case KVM_ARM_VCPU_INIT:
> > > > +        case KVM_SET_ONE_REG:
> > > > +        case KVM_GET_ONE_REG:
> > > > +                if (vcpu_is_protected(vcpu) && vcpu_has_run_once(vcpu))
> > > > +                        return -EPERM;
> > > > +        }
> > > > +
> > > > +        return 0;
> > > > +}
> > > > +
> > >
> > > I'm not keen on returning -EPERM for the ONE_REG stuff. For a start,
> > > X0 *is* valid on MMIO, and when you want to support LD64B and co,
> > > you'll need to show the actual data there.
> > >
> > > I'd rather return what is in the host vcpu structure, as normal.

I just wanted to chuck my thoughts in here, as some of this is my fault
and it would be really helpful to discuss it on the list. As you probably
know, on Android, pKVM has the semantics you describe above: the ONE_REG
calls succeed, but the register state isn't propagated to the guest once
it's started running. I think we largely did it this way because we were
solving a million problems at once during the initial development and
having to pipe-clean VMMs wasn't particularly appealing at the time.
It's also worth adding that, to my knowledge, we've never had any issues
in Android because of this decision.

So, based on that, it really sounds like it's the best option. If it ain't
broke, don't fix it!

*However*, I can't think of another upstream interface that behaves like
that and, when we came to document it, it was quite difficult to explain
it in a way that could be extended in the future. If we say that ONE_REG
doesn't propagate to the vCPU after first run and returns whatever was
last written, then userspace could (even accidentally) rely on that. If
we wanted to extend it, I think we'd not only need a method to advertise
the new behaviour (which I think you probably want even if ONE_REG
returned an error before) but also an opt-in for userspace to say that
it's ok with the new behaviour. In some ways, it feels a bit like the
"unchecked flags" problem for syscall arguments.

> > I think we should keep the error, for the reason s390 returns -EINVAL
> > from ONE_REG on a protected vCPU and x86 does from the register ioctls
> > for protected VM types:
> 
> News flash, this is not x86, nor s390. I don't feel constrained by
> other architecture, and we deviate *everywhere* already.

Right, this wasn't the rationale.

> > once the vCPU has run, the copy is the VMM's
> > boot state plus what the exit handlers copy out, and the VMM can't
> > distinguish them. x0 on MMIO is one of those fields, but the VMM reads
> > it from kvm_run->mmio. For LD64B, EL2 would copy the operands out the
> > same way and the error could be relaxed to those registers. Loosening
> > later breaks nobody. The comment and the message do overstate it by
> > calling the copy "not the guest's state", and I'll reword both in v4.
> > More on the kvmtool thread [1].
> 
> A protected-aware VMM already knows it cannot obtain the registers.

I think you can use that argument both ways: if the VMM knows it cannot
obtain the registers, then it's fine to return an error if the VMM does
something wrong.

In our recent investigation, it turned out that both kvmtool and crosvm
were perfectly happy with ONE_REG returning an error in normal operation
but we needed a handful of fixes for kvmtool [1] to handle things like
the 'debug' command.

If it's helpful, we can ask the crosvm developers here if they have
opinions for/against the two behaviours?

> I don't want to have to revisit the userspace interface once you have
> to relax it, because I know for sure that you will have to.

I think we'll have to do that either way, no? The VMM needs a way to
know that ONE_REG works and the situations in which it works. If the old
behaviour wasn't to return an error, it's also going to need a way to
enable the new behaviour.

> As far as LD64B is concerned, there is no place to copy anything in
> the run structure, and the relaxation would require to cover all the
> GPRs, ESR, and FAR. At this stage, returning whatever is there is the
> correct thing to do IMO.

The nice thing about that is that we can presumably tie it in with LD64B
support for protected VMs.

Will