Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1700017 > unrolled thread

Re: [RFC PATCH v2 38/38] KVM: arm64: Respect the virtual CPTR_EL2.TCPAC setting

Started byChristoffer Dall <cdall@linaro.org>
First post2017-07-31 15:00 +0200
Last post2017-08-01 13:30 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [RFC PATCH v2 38/38] KVM: arm64: Respect the virtual  CPTR_EL2.TCPAC setting Christoffer Dall <cdall@linaro.org> - 2017-07-31 15:00 +0200
    Re: [RFC PATCH v2 38/38] KVM: arm64: Respect the virtual  CPTR_EL2.TCPAC setting Jintack Lim <jintack.lim@linaro.org> - 2017-08-01 13:10 +0200
      Re: [RFC PATCH v2 38/38] KVM: arm64: Respect the virtual  CPTR_EL2.TCPAC setting Christoffer Dall <cdall@linaro.org> - 2017-08-01 13:30 +0200

#1700017 — Re: [RFC PATCH v2 38/38] KVM: arm64: Respect the virtual CPTR_EL2.TCPAC setting

FromChristoffer Dall <cdall@linaro.org>
Date2017-07-31 15:00 +0200
SubjectRe: [RFC PATCH v2 38/38] KVM: arm64: Respect the virtual CPTR_EL2.TCPAC setting
Message-ID<u9isF-6oB-1@gated-at.bofh.it>
On Tue, Jul 18, 2017 at 11:59:04AM -0500, Jintack Lim wrote:
> Forward CPACR_EL1 traps to the virtual EL2 if virtual CPTR_EL2 is
> configured to trap CPACR_EL1 accesses from EL1.
> 
> This is for recursive nested virtualization.
> 
> Signed-off-by: Jintack Lim <jintack.lim@linaro.org>
> ---
>  arch/arm64/kvm/sys_regs.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 6f67666..ba2966d 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1091,6 +1091,11 @@ static bool access_cpacr(struct kvm_vcpu *vcpu,
>  	if (el12_reg(p) && forward_nv_traps(vcpu))
>  		return kvm_inject_nested_sync(vcpu, kvm_vcpu_get_hsr(vcpu));
>  
> +	/* Forward this trap to the virtual EL2 if CPTR_EL2.TCPAC is set*/
> +	if (!el12_reg(p) && !vcpu_mode_el2(vcpu) &&
> +	    (vcpu_sys_reg(vcpu, CPTR_EL2) & CPTR_EL2_TCPAC))
> +		return kvm_inject_nested_sync(vcpu, kvm_vcpu_get_hsr(vcpu));
> +

I'm trying to understand what should happen if the VM is in EL1 and
accesses CPACR_EL12, but the guest hypervisor did not set
CPTR_EL2.TCPAC, why would we get here, and if there's a good reason why
we god here, is the EL12 access not supposed to undef at EL1 as opposed
to actually work, like it seems your code does when it doesn't take the
branch?

>  	/*
>  	 * When the virtual HCR_EL2.E2H == 1, an access to CPACR_EL1
>  	 * in the virtual EL2 is to access CPTR_EL2.
> -- 
> 1.9.1
> 

Thanks,
-Christoffer

[toc] | [next] | [standalone]


#1700868

FromJintack Lim <jintack.lim@linaro.org>
Date2017-08-01 13:10 +0200
Message-ID<u9DdL-3gc-9@gated-at.bofh.it>
In reply to#1700017
Hi Christoffer,

On Mon, Jul 31, 2017 at 8:59 AM, Christoffer Dall <cdall@linaro.org> wrote:
> On Tue, Jul 18, 2017 at 11:59:04AM -0500, Jintack Lim wrote:
>> Forward CPACR_EL1 traps to the virtual EL2 if virtual CPTR_EL2 is
>> configured to trap CPACR_EL1 accesses from EL1.
>>
>> This is for recursive nested virtualization.
>>
>> Signed-off-by: Jintack Lim <jintack.lim@linaro.org>
>> ---
>>  arch/arm64/kvm/sys_regs.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>> index 6f67666..ba2966d 100644
>> --- a/arch/arm64/kvm/sys_regs.c
>> +++ b/arch/arm64/kvm/sys_regs.c
>> @@ -1091,6 +1091,11 @@ static bool access_cpacr(struct kvm_vcpu *vcpu,
>>       if (el12_reg(p) && forward_nv_traps(vcpu))
>>               return kvm_inject_nested_sync(vcpu, kvm_vcpu_get_hsr(vcpu));
>>
>> +     /* Forward this trap to the virtual EL2 if CPTR_EL2.TCPAC is set*/
>> +     if (!el12_reg(p) && !vcpu_mode_el2(vcpu) &&
>> +         (vcpu_sys_reg(vcpu, CPTR_EL2) & CPTR_EL2_TCPAC))
>> +             return kvm_inject_nested_sync(vcpu, kvm_vcpu_get_hsr(vcpu));
>> +
>
> I'm trying to understand what should happen if the VM is in EL1 and
> accesses CPACR_EL12, but the guest hypervisor did not set
> CPTR_EL2.TCPAC, why would we get here, and if there's a good reason why

I guess what you meant is HCR_EL2.NV bit?

> we god here, is the EL12 access not supposed to undef at EL1 as opposed
> to actually work, like it seems your code does when it doesn't take the
> branch?

IIUC, we need to have this logic

if (el12_reg() && virtual HCR_EL2.NV == 0)
   inject_undef();

This is a good point, and should be applied for all traps controlled by NV bit.

>
>>       /*
>>        * When the virtual HCR_EL2.E2H == 1, an access to CPACR_EL1
>>        * in the virtual EL2 is to access CPTR_EL2.
>> --
>> 1.9.1
>>
>
> Thanks,
> -Christoffer

[toc] | [prev] | [next] | [standalone]


#1700892

FromChristoffer Dall <cdall@linaro.org>
Date2017-08-01 13:30 +0200
Message-ID<u9Dx8-3mr-7@gated-at.bofh.it>
In reply to#1700868
On Tue, Aug 01, 2017 at 07:03:35AM -0400, Jintack Lim wrote:
> Hi Christoffer,
> 
> On Mon, Jul 31, 2017 at 8:59 AM, Christoffer Dall <cdall@linaro.org> wrote:
> > On Tue, Jul 18, 2017 at 11:59:04AM -0500, Jintack Lim wrote:
> >> Forward CPACR_EL1 traps to the virtual EL2 if virtual CPTR_EL2 is
> >> configured to trap CPACR_EL1 accesses from EL1.
> >>
> >> This is for recursive nested virtualization.
> >>
> >> Signed-off-by: Jintack Lim <jintack.lim@linaro.org>
> >> ---
> >>  arch/arm64/kvm/sys_regs.c | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> >> index 6f67666..ba2966d 100644
> >> --- a/arch/arm64/kvm/sys_regs.c
> >> +++ b/arch/arm64/kvm/sys_regs.c
> >> @@ -1091,6 +1091,11 @@ static bool access_cpacr(struct kvm_vcpu *vcpu,
> >>       if (el12_reg(p) && forward_nv_traps(vcpu))
> >>               return kvm_inject_nested_sync(vcpu, kvm_vcpu_get_hsr(vcpu));
> >>
> >> +     /* Forward this trap to the virtual EL2 if CPTR_EL2.TCPAC is set*/
> >> +     if (!el12_reg(p) && !vcpu_mode_el2(vcpu) &&
> >> +         (vcpu_sys_reg(vcpu, CPTR_EL2) & CPTR_EL2_TCPAC))
> >> +             return kvm_inject_nested_sync(vcpu, kvm_vcpu_get_hsr(vcpu));
> >> +
> >
> > I'm trying to understand what should happen if the VM is in EL1 and
> > accesses CPACR_EL12, but the guest hypervisor did not set
> > CPTR_EL2.TCPAC, why would we get here, and if there's a good reason why
> 
> I guess what you meant is HCR_EL2.NV bit?
> 

No, HCR_EL2.NV is set, then we obviously get here, due to traps on _EL12
registers.

But if that wasn't the case (that's the time you'd be avaluating this
if-statement), then you're checking as part of the if-statement if the
virtual CPTR_EL2.TCPAC is set.  My question is, if the virtual
CPTR_EL2.TCPAC is not set, why would the physical one be set, which must
be the case if we're running this code, right?

> > we god here, is the EL12 access not supposed to undef at EL1 as opposed

I obviously meant *got* here.

> > to actually work, like it seems your code does when it doesn't take the
> > branch?
> 
> IIUC, we need to have this logic
> 
> if (el12_reg() && virtual HCR_EL2.NV == 0)
>    inject_undef();
> 
> This is a good point, and should be applied for all traps controlled by NV bit.
> 

Yes, but can this ever happen?

Thanks,
-Christoffer

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web