Message ID | 1502280338-23002-24-git-send-email-Dave.Martin@arm.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 127885 invoked by alias); 9 Aug 2017 12:08:32 -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 127791 invoked by uid 89); 9 Aug 2017 12:08:31 -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=Hx-languages-length:1761 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 23/27] arm64/sve: KVM: Hide SVE from CPU features exposed to guests Date: Wed, 9 Aug 2017 13:05:29 +0100 Message-Id: <1502280338-23002-24-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
KVM guests cannot currently use SVE, because SVE is always
configured to trap to EL2.
However, a guest that sees SVE reported as present in
ID_AA64PFR0_EL1 may legitimately expect that SVE works and try to
use it. Instead of working, the guest will receive an injected
undef exception, which may cause the guest to oops or go into a
spin.
To avoid misleading the guest into believing that SVE will work,
this patch masks out the SVE field from ID_AA64PFR0_EL1 when a
guest attempts to read this register. No support is explicitly
added for ID_AA64ZFR0_EL1 either, so that is still emulated as
reading as zero, which is consistent with SVE not being
implemented.
This is a temporary measure, and will be removed in a later series
when full KVM support for SVE is implemented.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
arch/arm64/kvm/sys_regs.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
Comments
On 09/08/17 13:05, Dave Martin wrote: > KVM guests cannot currently use SVE, because SVE is always > configured to trap to EL2. > > However, a guest that sees SVE reported as present in > ID_AA64PFR0_EL1 may legitimately expect that SVE works and try to > use it. Instead of working, the guest will receive an injected > undef exception, which may cause the guest to oops or go into a > spin. > > To avoid misleading the guest into believing that SVE will work, > this patch masks out the SVE field from ID_AA64PFR0_EL1 when a > guest attempts to read this register. No support is explicitly > added for ID_AA64ZFR0_EL1 either, so that is still emulated as > reading as zero, which is consistent with SVE not being > implemented. > > This is a temporary measure, and will be removed in a later series > when full KVM support for SVE is implemented. > > Signed-off-by: Dave Martin <Dave.Martin@arm.com> > --- > arch/arm64/kvm/sys_regs.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 6583dd7..9e8c54e 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -897,8 +897,20 @@ static u64 read_id_reg(struct sys_reg_desc const *r, bool raz) > { > u32 id = sys_reg((u32)r->Op0, (u32)r->Op1, > (u32)r->CRn, (u32)r->CRm, (u32)r->Op2); > + u64 val = raz ? 0 : read_sanitised_ftr_reg(id); > > - return raz ? 0 : read_sanitised_ftr_reg(id); > + if (id == SYS_ID_AA64PFR0_EL1) { > + static bool printed; > + > + if ((val & (0xfUL << ID_AA64PFR0_SVE_SHIFT)) && !printed) { > + kvm_info("SVE unsupported for guests, suppressing\n"); > + printed = true; > + } Ideally, this should be a vcpu_unimpl_once(). But: - it doesn't exist - vcpu_unimpl looks hopelessly x86 specific How about turning it into a pr_err_once() instead? > + > + val &= ~(0xfUL << ID_AA64PFR0_SVE_SHIFT); > + } > + > + return val; > } > > /* cpufeature ID register access trap handlers */ > Thanks, M.
On Tue, Aug 15, 2017 at 05:37:55PM +0100, Marc Zyngier wrote: > On 09/08/17 13:05, Dave Martin wrote: > > KVM guests cannot currently use SVE, because SVE is always > > configured to trap to EL2. > > > > However, a guest that sees SVE reported as present in > > ID_AA64PFR0_EL1 may legitimately expect that SVE works and try to > > use it. Instead of working, the guest will receive an injected > > undef exception, which may cause the guest to oops or go into a > > spin. > > > > To avoid misleading the guest into believing that SVE will work, > > this patch masks out the SVE field from ID_AA64PFR0_EL1 when a > > guest attempts to read this register. No support is explicitly > > added for ID_AA64ZFR0_EL1 either, so that is still emulated as > > reading as zero, which is consistent with SVE not being > > implemented. > > > > This is a temporary measure, and will be removed in a later series > > when full KVM support for SVE is implemented. > > > > Signed-off-by: Dave Martin <Dave.Martin@arm.com> > > --- > > arch/arm64/kvm/sys_regs.c | 14 +++++++++++++- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > > index 6583dd7..9e8c54e 100644 > > --- a/arch/arm64/kvm/sys_regs.c > > +++ b/arch/arm64/kvm/sys_regs.c > > @@ -897,8 +897,20 @@ static u64 read_id_reg(struct sys_reg_desc const *r, bool raz) > > { > > u32 id = sys_reg((u32)r->Op0, (u32)r->Op1, > > (u32)r->CRn, (u32)r->CRm, (u32)r->Op2); > > + u64 val = raz ? 0 : read_sanitised_ftr_reg(id); > > > > - return raz ? 0 : read_sanitised_ftr_reg(id); > > + if (id == SYS_ID_AA64PFR0_EL1) { > > + static bool printed; > > + > > + if ((val & (0xfUL << ID_AA64PFR0_SVE_SHIFT)) && !printed) { > > + kvm_info("SVE unsupported for guests, suppressing\n"); > > + printed = true; > > + } > > Ideally, this should be a vcpu_unimpl_once(). But: > - it doesn't exist > - vcpu_unimpl looks hopelessly x86 specific Yeah, I looked for an appropriate function and didn't find one ... and writing one just for this seemed overkill. > How about turning it into a pr_err_once() instead? Can do, though should it be an err? No error has occurred here, rather I want people who discover that their guest mysteriously doesn't see SVE gets a clue about why. [...] Cheers ---Dave
On 16/08/17 11:54, Dave Martin wrote: > On Tue, Aug 15, 2017 at 05:37:55PM +0100, Marc Zyngier wrote: >> On 09/08/17 13:05, Dave Martin wrote: >>> KVM guests cannot currently use SVE, because SVE is always >>> configured to trap to EL2. >>> >>> However, a guest that sees SVE reported as present in >>> ID_AA64PFR0_EL1 may legitimately expect that SVE works and try to >>> use it. Instead of working, the guest will receive an injected >>> undef exception, which may cause the guest to oops or go into a >>> spin. >>> >>> To avoid misleading the guest into believing that SVE will work, >>> this patch masks out the SVE field from ID_AA64PFR0_EL1 when a >>> guest attempts to read this register. No support is explicitly >>> added for ID_AA64ZFR0_EL1 either, so that is still emulated as >>> reading as zero, which is consistent with SVE not being >>> implemented. >>> >>> This is a temporary measure, and will be removed in a later series >>> when full KVM support for SVE is implemented. >>> >>> Signed-off-by: Dave Martin <Dave.Martin@arm.com> >>> --- >>> arch/arm64/kvm/sys_regs.c | 14 +++++++++++++- >>> 1 file changed, 13 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c >>> index 6583dd7..9e8c54e 100644 >>> --- a/arch/arm64/kvm/sys_regs.c >>> +++ b/arch/arm64/kvm/sys_regs.c >>> @@ -897,8 +897,20 @@ static u64 read_id_reg(struct sys_reg_desc const *r, bool raz) >>> { >>> u32 id = sys_reg((u32)r->Op0, (u32)r->Op1, >>> (u32)r->CRn, (u32)r->CRm, (u32)r->Op2); >>> + u64 val = raz ? 0 : read_sanitised_ftr_reg(id); >>> >>> - return raz ? 0 : read_sanitised_ftr_reg(id); >>> + if (id == SYS_ID_AA64PFR0_EL1) { >>> + static bool printed; >>> + >>> + if ((val & (0xfUL << ID_AA64PFR0_SVE_SHIFT)) && !printed) { >>> + kvm_info("SVE unsupported for guests, suppressing\n"); >>> + printed = true; >>> + } >> >> Ideally, this should be a vcpu_unimpl_once(). But: >> - it doesn't exist >> - vcpu_unimpl looks hopelessly x86 specific > > Yeah, I looked for an appropriate function and didn't find one ... and > writing one just for this seemed overkill. > >> How about turning it into a pr_err_once() instead? > > Can do, though should it be an err? > > No error has occurred here, rather I want people who discover that their > guest mysteriously doesn't see SVE gets a clue about why. An "err" is a good way to make it appear on the console. If you want to make it noticed, that's the way. M.
On Wed, Aug 16, 2017 at 12:10:53PM +0100, Marc Zyngier wrote: > On 16/08/17 11:54, Dave Martin wrote: > > On Tue, Aug 15, 2017 at 05:37:55PM +0100, Marc Zyngier wrote: > >> On 09/08/17 13:05, Dave Martin wrote: [...] > >>> + if (id == SYS_ID_AA64PFR0_EL1) { > >>> + static bool printed; > >>> + > >>> + if ((val & (0xfUL << ID_AA64PFR0_SVE_SHIFT)) && !printed) { > >>> + kvm_info("SVE unsupported for guests, suppressing\n"); > >>> + printed = true; > >>> + } > >> > >> Ideally, this should be a vcpu_unimpl_once(). But: > >> - it doesn't exist > >> - vcpu_unimpl looks hopelessly x86 specific > > > > Yeah, I looked for an appropriate function and didn't find one ... and > > writing one just for this seemed overkill. > > > >> How about turning it into a pr_err_once() instead? > > > > Can do, though should it be an err? > > > > No error has occurred here, rather I want people who discover that their > > guest mysteriously doesn't see SVE gets a clue about why. > An "err" is a good way to make it appear on the console. If you want to > make it noticed, that's the way. OK, I was concerned that overuse of err might be frowned upon. If this is the preferred way to signal such things, I'm happy to that. Cheers ---Dave
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 6583dd7..9e8c54e 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -897,8 +897,20 @@ static u64 read_id_reg(struct sys_reg_desc const *r, bool raz) { u32 id = sys_reg((u32)r->Op0, (u32)r->Op1, (u32)r->CRn, (u32)r->CRm, (u32)r->Op2); + u64 val = raz ? 0 : read_sanitised_ftr_reg(id); - return raz ? 0 : read_sanitised_ftr_reg(id); + if (id == SYS_ID_AA64PFR0_EL1) { + static bool printed; + + if ((val & (0xfUL << ID_AA64PFR0_SVE_SHIFT)) && !printed) { + kvm_info("SVE unsupported for guests, suppressing\n"); + printed = true; + } + + val &= ~(0xfUL << ID_AA64PFR0_SVE_SHIFT); + } + + return val; } /* cpufeature ID register access trap handlers */