On 6/21/19 10:38 AM, Marc Zyngier wrote:
> From: Christoffer Dall <christoffer.dall@xxxxxxxxxx>
>
> When running in virtual EL2 mode, we actually run the hardware in EL1
> and therefore have to use the EL1 registers to ensure correct operation.
>
> By setting the HCR.TVM and HCR.TVRM we ensure that the virtual EL2 mode
> doesn't shoot itself in the foot when setting up what it believes to be
> a different mode's system register state (for example when preparing to
> switch to a VM).
A guest hypervisor with vhe enabled uses the _EL12 register names when preparing
to run a guest, and accesses to those registers are already trapped when setting
HCR_EL2.NV. This patch affects only non-vhe guest hypervisors, would you mind
updating the message to reflect that?
>
> We can leverage the existing sysregs infrastructure to support trapped
> accesses to these registers.
>
> Signed-off-by: Christoffer Dall <christoffer.dall@xxxxxxxxxx>
> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> ---
> arch/arm64/kvm/hyp/switch.c | 4 ++++
> arch/arm64/kvm/sys_regs.c | 7 ++++++-
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 7b55c11b30fb..791b26570347 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -135,6 +135,10 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu)
> {
> u64 hcr = vcpu->arch.hcr_el2;
>
> + /* Trap VM sysreg accesses if an EL2 guest is not using VHE. */
> + if (vcpu_mode_el2(vcpu) && !vcpu_el2_e2h_is_set(vcpu))
> + hcr |= HCR_TVM | HCR_TRVM;
> +
> write_sysreg(hcr, hcr_el2);
>
> if (cpus_have_const_cap(ARM64_HAS_RAS_EXTN) && (hcr & HCR_VSE))
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index e181359adadf..0464d8e29cba 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -440,7 +440,12 @@ static bool access_vm_reg(struct kvm_vcpu *vcpu,
> u64 val;
> int reg = r->reg;
>
> - BUG_ON(!p->is_write);
> + BUG_ON(!vcpu_mode_el2(vcpu) && !p->is_write);
> +
> + if (!p->is_write) {
> + p->regval = vcpu_read_sys_reg(vcpu, reg);
> + return true;
> + }
>
> /* See the 32bit mapping in kvm_host.h */
> if (p->is_aarch32)
For more context:
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index e181359adadf..0464d8e29cba 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -428,31 +428,36 @@ static bool access_dcsw(struct kvm_vcpu *vcpu,
}
/*
* Generic accessor for VM registers. Only called as long as HCR_TVM
* is set. If the guest enables the MMU, we stop trapping the VM
* sys_regs and leave it in complete control of the caches.
*/
static bool access_vm_reg(struct kvm_vcpu *vcpu,
struct sys_reg_params *p,
const struct sys_reg_desc *r)
{
bool was_enabled = vcpu_has_cache_enabled(vcpu);
u64 val;
int reg = r->reg;
- BUG_ON(!p->is_write);
+ BUG_ON(!vcpu_mode_el2(vcpu) && !p->is_write);
+
+ if (!p->is_write) {
+ p->regval = vcpu_read_sys_reg(vcpu, reg);
+ return true;
+ }
/* See the 32bit mapping in kvm_host.h */
if (p->is_aarch32)
reg = r->reg / 2;
if (!p->is_aarch32 || !p->is_32bit) {
val = p->regval;
} else {
val = vcpu_read_sys_reg(vcpu, reg);
if (r->reg % 2)
val = (p->regval << 32) | (u64)lower_32_bits(val);
else
val = ((u64)upper_32_bits(val) << 32) |
lower_32_bits(p->regval);
}
Perhaps the function comment should be updated to reflect that the function is
also used for VM register traps for a non-vhe guest hypervisor?
