On 7/2/19 5:32 PM, Alexandru Elisei wrote:
> On 6/21/19 10:38 AM, Marc Zyngier wrote:
>> From: Jintack Lim <jintack@xxxxxxxxxxxxxxx>
>>
>> Forward ELR_EL1, SPSR_EL1 and VBAR_EL1 traps to the virtual EL2 if the
>> virtual HCR_EL2.NV bit is set.
> HCR_EL2.NV1?
>> This is for recursive nested virtualization.
>>
>> Signed-off-by: Jintack Lim <jintack@xxxxxxxxxxxxxxx>
>> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
>> ---
>> arch/arm64/include/asm/kvm_arm.h | 1 +
>> arch/arm64/kvm/sys_regs.c | 19 +++++++++++++++++--
>> 2 files changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
>> index d21486274eeb..55f4525c112c 100644
>> --- a/arch/arm64/include/asm/kvm_arm.h
>> +++ b/arch/arm64/include/asm/kvm_arm.h
>> @@ -24,6 +24,7 @@
>>
>> /* Hyp Configuration Register (HCR) bits */
>> #define HCR_FWB (UL(1) << 46)
>> +#define HCR_NV1 (UL(1) << 43)
>> #define HCR_NV (UL(1) << 42)
>> #define HCR_API (UL(1) << 41)
>> #define HCR_APK (UL(1) << 40)
>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>> index 0f74b9277a86..beadebcfc888 100644
>> --- a/arch/arm64/kvm/sys_regs.c
>> +++ b/arch/arm64/kvm/sys_regs.c
>> @@ -473,8 +473,10 @@ static bool access_vm_reg(struct kvm_vcpu *vcpu,
>> if (el12_reg(p) && forward_nv_traps(vcpu))
>> return false;
>>
>> - if (!el12_reg(p) && forward_vm_traps(vcpu, p))
>> - return kvm_inject_nested_sync(vcpu, kvm_vcpu_get_hsr(vcpu));
>> + if (!el12_reg(p) && forward_vm_traps(vcpu, p)) {
>> + kvm_inject_nested_sync(vcpu, kvm_vcpu_get_hsr(vcpu));
>> + return false;
>> + }
>>
>> BUG_ON(!vcpu_mode_el2(vcpu) && !p->is_write);
>>
>> @@ -1643,6 +1645,13 @@ static bool access_sp_el1(struct kvm_vcpu *vcpu,
>> return true;
>> }
>>
>> +
>> +/* This function is to support the recursive nested virtualization */
>> +static bool forward_nv1_traps(struct kvm_vcpu *vcpu, struct sys_reg_params *p)
> Why the struct sys_reg_params *p argument? It isn't used by the function.
>> +{
>> + return forward_traps(vcpu, HCR_NV1);
>> +}
>> +
>> static bool access_elr(struct kvm_vcpu *vcpu,
>> struct sys_reg_params *p,
>> const struct sys_reg_desc *r)
>> @@ -1650,6 +1659,9 @@ static bool access_elr(struct kvm_vcpu *vcpu,
>> if (el12_reg(p) && forward_nv_traps(vcpu))
>> return false;
>>
>> + if (!el12_reg(p) && forward_nv1_traps(vcpu, p))
>> + return false;
>> +
>> if (p->is_write)
>> vcpu->arch.ctxt.gp_regs.elr_el1 = p->regval;
>> else
>> @@ -1665,6 +1677,9 @@ static bool access_spsr(struct kvm_vcpu *vcpu,
>> if (el12_reg(p) && forward_nv_traps(vcpu))
>> return false;
>>
>> + if (!el12_reg(p) && forward_nv1_traps(vcpu, p))
>> + return false;
>> +
>> if (p->is_write)
>> vcpu->arch.ctxt.gp_regs.spsr[KVM_SPSR_EL1] = p->regval;
>> else
> The commit message mentions VBAR_EL1, but there's no change related to it.
> Parhaps you're missing this (build tested only):
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index bd21f0f45a86..082dc31ff533 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -401,6 +401,12 @@ static bool el12_reg(struct sys_reg_params *p)
> return (p->Op1 == 5);
> }
>
> +/* This function is to support the recursive nested virtualization */
> +static bool forward_nv1_traps(struct kvm_vcpu *vcpu, struct sys_reg_params *p)
> +{
> + return forward_traps(vcpu, HCR_NV1);
> +}
> +
> static bool access_rw(struct kvm_vcpu *vcpu,
> struct sys_reg_params *p,
> const struct sys_reg_desc *r)
> @@ -408,6 +414,10 @@ static bool access_rw(struct kvm_vcpu *vcpu,
> if (el12_reg(p) && forward_nv_traps(vcpu))
> return false;
>
> + if (sys_reg(p->Op0, p->Op1, p->CRn, p->CRm, p->Op2) == SYS_VBAR_EL1 &&
> + forward_nv1_traps(vcpu, p))
Ahem... this is probably better:
+ if (r->reg == VBAR_EL1 && forward_nv1_traps(vcpu, p))
> + return false;
> +
> if (p->is_write)
> vcpu_write_sys_reg(vcpu, p->regval, r->reg);
> else
> @@ -1794,12 +1804,6 @@ static bool forward_ttlb_traps(struct kvm_vcpu *vcpu)
> return forward_traps(vcpu, HCR_TTLB);
> }
>
> -/* This function is to support the recursive nested virtualization */
> -static bool forward_nv1_traps(struct kvm_vcpu *vcpu, struct sys_reg_params *p)
> -{
> - return forward_traps(vcpu, HCR_NV1);
> -}
> -
> static bool access_elr(struct kvm_vcpu *vcpu,
> struct sys_reg_params *p,
> const struct sys_reg_desc *r)
>
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
