linux-kernel - Re: [RFC PATCH v2 38/38] KVM: arm64: Respect the virtual CPTR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170801112029.GD5176@cbox>
Date:   Tue, 1 Aug 2017 13:20:29 +0200
From:   Christoffer Dall <cdall@...aro.org>
To:     Jintack Lim <jintack.lim@...aro.org>
Cc:     kvmarm@...ts.cs.columbia.edu,
        Christoffer Dall <christoffer.dall@...aro.org>,
        Marc Zyngier <marc.zyngier@....com>,
        Jonathan Corbet <corbet@....net>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        linux@...linux.org.uk, Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>, akpm@...ux-foundation.org,
        mchehab@...nel.org, cov@...eaurora.org,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        david.daney@...ium.com, mark.rutland@....com,
        Suzuki K Poulose <suzuki.poulose@....com>,
        stefan@...lo-penguin.com, Andy Gross <andy.gross@...aro.org>,
        wcohen@...hat.com, Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        shankerd@...eaurora.org, vladimir.murzin@....com,
        james.morse@....com, linux-doc@...r.kernel.org,
        lkml - Kernel Mailing List <linux-kernel@...r.kernel.org>,
        KVM General <kvm@...r.kernel.org>,
        arm-mail-list <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [RFC PATCH v2 38/38] KVM: arm64: Respect the virtual
 CPTR_EL2.TCPAC setting

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@...aro.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@...aro.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