Message ID | 1502280338-23002-22-git-send-email-Dave.Martin@arm.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 126788 invoked by alias); 9 Aug 2017 12:08:16 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 126745 invoked by uid 89); 9 Aug 2017 12:08:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: foss.arm.com From: Dave Martin <Dave.Martin@arm.com> To: linux-arm-kernel@lists.infradead.org Cc: Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will.deacon@arm.com>, Ard Biesheuvel <ard.biesheuvel@linaro.org>, Szabolcs Nagy <szabolcs.nagy@arm.com>, Richard Sandiford <richard.sandiford@arm.com>, kvmarm@lists.cs.columbia.edu, libc-alpha@sourceware.org, linux-arch@vger.kernel.org, Christoffer Dall <christoffer.dall@linaro.org>, Marc Zyngier <marc.zyngier@arm.com> Subject: [PATCH 21/27] arm64/sve: KVM: Prevent guests from using SVE Date: Wed, 9 Aug 2017 13:05:27 +0100 Message-Id: <1502280338-23002-22-git-send-email-Dave.Martin@arm.com> In-Reply-To: <1502280338-23002-1-git-send-email-Dave.Martin@arm.com> References: <1502280338-23002-1-git-send-email-Dave.Martin@arm.com> |
Commit Message
Dave Martin
Aug. 9, 2017, 12:05 p.m. UTC
Until KVM has full SVE support, guests must not be allowed to
execute SVE instructions.
This patch enables the necessary traps, and also ensures that the
traps are disabled again on exit from the guest so that the host
can still use SVE if it wants to.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
arch/arm64/include/asm/kvm_arm.h | 3 ++-
arch/arm64/kvm/hyp/switch.c | 6 +++---
2 files changed, 5 insertions(+), 4 deletions(-)
Comments
On 09/08/17 13:05, Dave Martin wrote: > Until KVM has full SVE support, guests must not be allowed to > execute SVE instructions. > > This patch enables the necessary traps, and also ensures that the > traps are disabled again on exit from the guest so that the host > can still use SVE if it wants to. > > Signed-off-by: Dave Martin <Dave.Martin@arm.com> > --- > arch/arm64/include/asm/kvm_arm.h | 3 ++- > arch/arm64/kvm/hyp/switch.c | 6 +++--- > 2 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h > index dbf0537..8a19651 100644 > --- a/arch/arm64/include/asm/kvm_arm.h > +++ b/arch/arm64/include/asm/kvm_arm.h > @@ -186,7 +186,7 @@ > #define CPTR_EL2_TTA (1 << 20) > #define CPTR_EL2_TFP (1 << CPTR_EL2_TFP_SHIFT) > #define CPTR_EL2_TZ (1 << 8) > -#define CPTR_EL2_DEFAULT 0x000033ff > +#define CPTR_EL2_DEFAULT (0x000033ff & ~CPTR_EL2_TZ) I must say I'm not overly fond of this construct. I'd rather introduce a RES1 field that matches the v8.2 description, instead of this ugly constant and something that clears it. > > /* Hyp Debug Configuration Register bits */ > #define MDCR_EL2_TPMS (1 << 14) > @@ -237,5 +237,6 @@ > > #define CPACR_EL1_FPEN (3 << 20) > #define CPACR_EL1_TTA (1 << 28) > +#define CPACR_EL1_DEFAULT (CPACR_EL1_FPEN | CPACR_EL1_ZEN_EL1EN) > > #endif /* __ARM64_KVM_ARM_H__ */ > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c > index 35a90b8..951f3eb 100644 > --- a/arch/arm64/kvm/hyp/switch.c > +++ b/arch/arm64/kvm/hyp/switch.c > @@ -48,7 +48,7 @@ static void __hyp_text __activate_traps_vhe(void) > > val = read_sysreg(cpacr_el1); > val |= CPACR_EL1_TTA; > - val &= ~CPACR_EL1_FPEN; > + val &= ~(CPACR_EL1_FPEN | CPACR_EL1_ZEN); > write_sysreg(val, cpacr_el1); > > write_sysreg(__kvm_hyp_vector, vbar_el1); > @@ -59,7 +59,7 @@ static void __hyp_text __activate_traps_nvhe(void) > u64 val; > > val = CPTR_EL2_DEFAULT; > - val |= CPTR_EL2_TTA | CPTR_EL2_TFP; > + val |= CPTR_EL2_TTA | CPTR_EL2_TFP | CPTR_EL2_TZ; > write_sysreg(val, cptr_el2); > } > > @@ -117,7 +117,7 @@ static void __hyp_text __deactivate_traps_vhe(void) > > write_sysreg(mdcr_el2, mdcr_el2); > write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2); > - write_sysreg(CPACR_EL1_FPEN, cpacr_el1); > + write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1); > write_sysreg(vectors, vbar_el1); > } > > Thanks, M.
On Tue, Aug 15, 2017 at 05:33:15PM +0100, Marc Zyngier wrote: > On 09/08/17 13:05, Dave Martin wrote: > > Until KVM has full SVE support, guests must not be allowed to > > execute SVE instructions. > > > > This patch enables the necessary traps, and also ensures that the > > traps are disabled again on exit from the guest so that the host > > can still use SVE if it wants to. > > > > Signed-off-by: Dave Martin <Dave.Martin@arm.com> > > --- > > arch/arm64/include/asm/kvm_arm.h | 3 ++- > > arch/arm64/kvm/hyp/switch.c | 6 +++--- > > 2 files changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h > > index dbf0537..8a19651 100644 > > --- a/arch/arm64/include/asm/kvm_arm.h > > +++ b/arch/arm64/include/asm/kvm_arm.h > > @@ -186,7 +186,7 @@ > > #define CPTR_EL2_TTA (1 << 20) > > #define CPTR_EL2_TFP (1 << CPTR_EL2_TFP_SHIFT) > > #define CPTR_EL2_TZ (1 << 8) > > -#define CPTR_EL2_DEFAULT 0x000033ff > > +#define CPTR_EL2_DEFAULT (0x000033ff & ~CPTR_EL2_TZ) > > I must say I'm not overly fond of this construct. I'd rather introduce a > RES1 field that matches the v8.2 description, instead of this ugly > constant and something that clears it. Sorry, I don't get your meaning here. v8.2 neither immediately predates or postdates SVE. What are you propsing? I guess we could just write 0x000032ff now that the only meaning the architecture can ever assign to bit 8 is known. [...] Cheers ---Dave
On 16/08/17 11:50, Dave Martin wrote: > On Tue, Aug 15, 2017 at 05:33:15PM +0100, Marc Zyngier wrote: >> On 09/08/17 13:05, Dave Martin wrote: >>> Until KVM has full SVE support, guests must not be allowed to >>> execute SVE instructions. >>> >>> This patch enables the necessary traps, and also ensures that the >>> traps are disabled again on exit from the guest so that the host >>> can still use SVE if it wants to. >>> >>> Signed-off-by: Dave Martin <Dave.Martin@arm.com> >>> --- >>> arch/arm64/include/asm/kvm_arm.h | 3 ++- >>> arch/arm64/kvm/hyp/switch.c | 6 +++--- >>> 2 files changed, 5 insertions(+), 4 deletions(-) >>> >>> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h >>> index dbf0537..8a19651 100644 >>> --- a/arch/arm64/include/asm/kvm_arm.h >>> +++ b/arch/arm64/include/asm/kvm_arm.h >>> @@ -186,7 +186,7 @@ >>> #define CPTR_EL2_TTA (1 << 20) >>> #define CPTR_EL2_TFP (1 << CPTR_EL2_TFP_SHIFT) >>> #define CPTR_EL2_TZ (1 << 8) >>> -#define CPTR_EL2_DEFAULT 0x000033ff >>> +#define CPTR_EL2_DEFAULT (0x000033ff & ~CPTR_EL2_TZ) >> >> I must say I'm not overly fond of this construct. I'd rather introduce a >> RES1 field that matches the v8.2 description, instead of this ugly >> constant and something that clears it. > > Sorry, I don't get your meaning here. v8.2 neither immediately predates > or postdates SVE. The ARMv8 ARM (DDI406B_a, D7.2.19) says otherwise. This bit is only defined as having a possibility of being 0 on an v8.2 implementation if SVE is implemented. > What are you propsing? What I'm proposing is: #define CPTR_EL2_RES1 0x32ff #define CPTR_EL2_DEFAULT CPTR_EL2_RES1 and none of that pointless constant clearing. > I guess we could just write 0x000032ff now that the only meaning the > architecture can ever assign to bit 8 is known. Exactly. M.
On 16/08/17 12:20, Marc Zyngier wrote: > On 16/08/17 11:50, Dave Martin wrote: >> On Tue, Aug 15, 2017 at 05:33:15PM +0100, Marc Zyngier wrote: >>> On 09/08/17 13:05, Dave Martin wrote: >>>> Until KVM has full SVE support, guests must not be allowed to >>>> execute SVE instructions. >>>> >>>> This patch enables the necessary traps, and also ensures that the >>>> traps are disabled again on exit from the guest so that the host >>>> can still use SVE if it wants to. >>>> >>>> Signed-off-by: Dave Martin <Dave.Martin@arm.com> >>>> --- >>>> arch/arm64/include/asm/kvm_arm.h | 3 ++- >>>> arch/arm64/kvm/hyp/switch.c | 6 +++--- >>>> 2 files changed, 5 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h >>>> index dbf0537..8a19651 100644 >>>> --- a/arch/arm64/include/asm/kvm_arm.h >>>> +++ b/arch/arm64/include/asm/kvm_arm.h >>>> @@ -186,7 +186,7 @@ >>>> #define CPTR_EL2_TTA (1 << 20) >>>> #define CPTR_EL2_TFP (1 << CPTR_EL2_TFP_SHIFT) >>>> #define CPTR_EL2_TZ (1 << 8) >>>> -#define CPTR_EL2_DEFAULT 0x000033ff >>>> +#define CPTR_EL2_DEFAULT (0x000033ff & ~CPTR_EL2_TZ) >>> >>> I must say I'm not overly fond of this construct. I'd rather introduce a >>> RES1 field that matches the v8.2 description, instead of this ugly >>> constant and something that clears it. >> >> Sorry, I don't get your meaning here. v8.2 neither immediately predates >> or postdates SVE. > > The ARMv8 ARM (DDI406B_a, D7.2.19) says otherwise. This bit is only Of course, the days of the ARMv7 ARM are still haunting me. This should read DDI487B_a. Apologies for the confusion. M.
On Wed, Aug 16, 2017 at 12:20:41PM +0100, Marc Zyngier wrote: > On 16/08/17 11:50, Dave Martin wrote: > > On Tue, Aug 15, 2017 at 05:33:15PM +0100, Marc Zyngier wrote: > >> On 09/08/17 13:05, Dave Martin wrote: > >>> Until KVM has full SVE support, guests must not be allowed to > >>> execute SVE instructions. > >>> > >>> This patch enables the necessary traps, and also ensures that the > >>> traps are disabled again on exit from the guest so that the host > >>> can still use SVE if it wants to. > >>> > >>> Signed-off-by: Dave Martin <Dave.Martin@arm.com> > >>> --- > >>> arch/arm64/include/asm/kvm_arm.h | 3 ++- > >>> arch/arm64/kvm/hyp/switch.c | 6 +++--- > >>> 2 files changed, 5 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h > >>> index dbf0537..8a19651 100644 > >>> --- a/arch/arm64/include/asm/kvm_arm.h > >>> +++ b/arch/arm64/include/asm/kvm_arm.h > >>> @@ -186,7 +186,7 @@ > >>> #define CPTR_EL2_TTA (1 << 20) > >>> #define CPTR_EL2_TFP (1 << CPTR_EL2_TFP_SHIFT) > >>> #define CPTR_EL2_TZ (1 << 8) > >>> -#define CPTR_EL2_DEFAULT 0x000033ff > >>> +#define CPTR_EL2_DEFAULT (0x000033ff & ~CPTR_EL2_TZ) > >> > >> I must say I'm not overly fond of this construct. I'd rather introduce a > >> RES1 field that matches the v8.2 description, instead of this ugly > >> constant and something that clears it. > > > > Sorry, I don't get your meaning here. v8.2 neither immediately predates > > or postdates SVE. > > The ARMv8 ARM (DDI406B_a, D7.2.19) says otherwise. This bit is only > defined as having a possibility of being 0 on an v8.2 implementation if > SVE is implemented. Right. I was confused by the v8.2 reference, since if this weren't true for v8.0 of the architecture, we couldn't simply change a compile time constant here. In fact, there's a compatible retroactive change to all arch versions >= v8.0. > > What are you propsing? > > What I'm proposing is: > > #define CPTR_EL2_RES1 0x32ff > #define CPTR_EL2_DEFAULT CPTR_EL2_RES1 > > and none of that pointless constant clearing. > > > I guess we could just write 0x000032ff now that the only meaning the > > architecture can ever assign to bit 8 is known. > Exactly. OK, good -- I'll change that. I was trying to avoid magic numberness, but it's a bit futile when talking about RESx bits. Cheers ---Dave
diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h index dbf0537..8a19651 100644 --- a/arch/arm64/include/asm/kvm_arm.h +++ b/arch/arm64/include/asm/kvm_arm.h @@ -186,7 +186,7 @@ #define CPTR_EL2_TTA (1 << 20) #define CPTR_EL2_TFP (1 << CPTR_EL2_TFP_SHIFT) #define CPTR_EL2_TZ (1 << 8) -#define CPTR_EL2_DEFAULT 0x000033ff +#define CPTR_EL2_DEFAULT (0x000033ff & ~CPTR_EL2_TZ) /* Hyp Debug Configuration Register bits */ #define MDCR_EL2_TPMS (1 << 14) @@ -237,5 +237,6 @@ #define CPACR_EL1_FPEN (3 << 20) #define CPACR_EL1_TTA (1 << 28) +#define CPACR_EL1_DEFAULT (CPACR_EL1_FPEN | CPACR_EL1_ZEN_EL1EN) #endif /* __ARM64_KVM_ARM_H__ */ diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c index 35a90b8..951f3eb 100644 --- a/arch/arm64/kvm/hyp/switch.c +++ b/arch/arm64/kvm/hyp/switch.c @@ -48,7 +48,7 @@ static void __hyp_text __activate_traps_vhe(void) val = read_sysreg(cpacr_el1); val |= CPACR_EL1_TTA; - val &= ~CPACR_EL1_FPEN; + val &= ~(CPACR_EL1_FPEN | CPACR_EL1_ZEN); write_sysreg(val, cpacr_el1); write_sysreg(__kvm_hyp_vector, vbar_el1); @@ -59,7 +59,7 @@ static void __hyp_text __activate_traps_nvhe(void) u64 val; val = CPTR_EL2_DEFAULT; - val |= CPTR_EL2_TTA | CPTR_EL2_TFP; + val |= CPTR_EL2_TTA | CPTR_EL2_TFP | CPTR_EL2_TZ; write_sysreg(val, cptr_el2); } @@ -117,7 +117,7 @@ static void __hyp_text __deactivate_traps_vhe(void) write_sysreg(mdcr_el2, mdcr_el2); write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2); - write_sysreg(CPACR_EL1_FPEN, cpacr_el1); + write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1); write_sysreg(vectors, vbar_el1); }