Re: [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs

Hi Alex,

On Fri, Jun 11, 2021 at 1:43 PM Alexandru Elisei
[off-list ref] wrote:

Hi,

On 6/8/21 3:11 PM, Fuad Tabba wrote:

quoted

Hi,

This patch series adds support for restricting CPU features for protected VMs
in KVM [1].

Various feature configurations are allowed in KVM/arm64. Supporting all
these features in pKVM is difficult, as it either involves moving much of
the handling code to EL2, which adds bloat and results in a less verifiable
trusted code base. Or it involves leaving the code handling at EL1, which
risks having an untrusted host kernel feeding wrong information to the EL2
and to the protected guests.

This series attempts to mitigate this by reducing the configuration space,
providing a reduced amount of feature support at EL2 with the least amount of
compromise of protected guests' capabilities.

This is done by restricting CPU features exposed to protected guests through
feature registers. These restrictions are enforced by trapping register
accesses as well as instructions associated with these features, and injecting
an undefined exception into the guest if it attempts to use a restricted
feature.

The features being restricted (only for protected VMs in protected mode) are
the following:
- Debug, Trace, and DoubleLock
- Performance Monitoring (PMU)
- Statistical Profiling (SPE)
- Scalable Vector Extension (SVE)
- Memory Partitioning and Monitoring (MPAM)
- Activity Monitoring (AMU)
- Memory Tagging (MTE)
- Limited Ordering Regions (LOR)
- AArch32 State
- Generic Interrupt Controller (GIC) (depending on rVIC support)
- Nested Virtualization (NV)
- Reliability, Availability, and Serviceability (RAS) above V1
- Implementation-defined Features

This series is based on kvmarm/next and Will's patches for an Initial pKVM user
ABI [1]. You can find the applied series here [2].

Since this is implementing the kernel side of an RFC userspace ABI, I'm going to
treat the series as an RFC also and not go into the individual patches.

What strikes me as odd is the fact that, as far as I can tell, you're duplicating
part of the kvm/sys_regs.c and kvm/handle_exit.c functionality in the nvhe code.
Why was this approach chosen instead of reusing the existing functions and adding
extra code to handle the protected VM case?

Another example of this is detecting when a host dropped to 32bit EL0, the comment
says that you don't trust the host to make the check. What exactly do you trust
the host to do at what point? I don't see this explained anywhere, it's possible
I've missed it.

You're right. I haven't explained this in the patch series or provided
enough context other than a link to Will's presentation [1].

The idea is that protected VMs are protected from the host Linux
kernel (and from other VMs), where the host does not have access to
guest memory even if compromised. This patch series does not cover
that part yet. It is a part of, and builds on, other concurrent work
in order to get us there eventually [2].

Once everything falls into place, the host should not even have access
to a protected guest's state or anything that would enable it to
manipulate it (e.g., vcpu register context and el2 system registers),
only hyp would have that access. If the host could access that state,
then it might be able to get around the protection provided.
Therefore, anything that is sensitive and that would require such
access needs to happen at hyp, hence the code in nvhe running only at
hyp.

I also think that registers that mostly don't change during the lifetime of the VM
(HCR_EL2, CPTR_EL2, MDCR_EL2) can be set by host when the VM becomes protected,
instead of fiddling with them at each world switch. Was there a particular reason
for changing them in __activate_traps_pvm() or was this just an implementation choice?

You're right that they don't change during the lifetime of a VM, but
protected VMs can coexist with non-protected VMs. The values of these
registers are different between the two (different trapping to control
protection as well as enabled features). Thus the new
__activate_traps_pvm(), which would activate traps specifically for
protected VMs.

Thank you,
/fuad

[1]
https://mirrors.edge.kernel.org/pub/linux/kernel/people/will/slides/kvmforum-2020-edited.pdf

[2] Some of the work on protected KVM:
https://lore.kernel.org/kvmarm/20201202184122.26046-1-dbrazdil@google.com/ (local)
https://lore.kernel.org/kvmarm/20210602094347.3730846-1-qperret@google.com/ (local)
https://lore.kernel.org/kvmarm/20210608114518.748712-1-qperret@google.com/ (local)
https://lore.kernel.org/kvmarm/20210322175639.801566-1-maz@kernel.org/ (local)
https://lore.kernel.org/kvmarm/20210603183347.1695-1-will@kernel.org/ (local)

Thanks,

Alex

quoted

Cheers,
/fuad

[1] https://lore.kernel.org/kvmarm/20210603183347.1695-1-will@kernel.org/ (local)

For more details about pKVM, please refer to Will's talk at KVM Forum 2020:
https://www.youtube.com/watch?v=edqJSzsDRxk

[2] https://android-kvm.googlesource.com/linux/+/refs/heads/tabba/el2_fixed_feature_v1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel