From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 118939 invoked by alias); 12 Oct 2017 11:04:29 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 118925 invoked by uid 89); 12 Oct 2017 11:04:28 -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 Date: Thu, 12 Oct 2017 11:04:00 -0000 From: Dave Martin To: Marc Zyngier Cc: linux-arm-kernel@lists.infradead.org, linux-arch@vger.kernel.org, Christoffer Dall , Okamoto Takayuki , libc-alpha@sourceware.org, Ard Biesheuvel , Szabolcs Nagy , Catalin Marinas , Will Deacon , Richard Sandiford , Alex =?iso-8859-1?Q?Benn=E9e?= , kvmarm@lists.cs.columbia.edu Subject: Re: [PATCH v3 22/28] arm64/sve: KVM: Prevent guests from using SVE Message-ID: <20171012110409.GG19485@e103592.cambridge.arm.com> References: <1507660725-7986-1-git-send-email-Dave.Martin@arm.com> <1507660725-7986-23-git-send-email-Dave.Martin@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-SW-Source: 2017-10/txt/msg00525.txt.bz2 On Wed, Oct 11, 2017 at 05:28:06PM +0100, Marc Zyngier wrote: > [+ Christoffer] > > On 10/10/17 19:38, 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. > > > > This patch introduces another instance of > > __this_cpu_write(fpsimd_last_state, NULL), so this flush operation > > is abstracted out as a separate helper fpsimd_flush_cpu_state(). > > Other instances are ported appropriately. > > > > As a side effect of this refactoring, a this_cpu_write() in > > fpsimd_cpu_pm_notifier() is changed to __this_cpu_write(). This > > should be fine, since cpu_pm_enter() is supposed to be called only > > with interrupts disabled. > > > > Signed-off-by: Dave Martin > > Reviewed-by: Alex Bennée > > Cc: Marc Zyngier > > Cc: Ard Biesheuvel > > --- [...] > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > > index e923b58..674912d 100644 > > --- a/arch/arm64/include/asm/kvm_host.h > > +++ b/arch/arm64/include/asm/kvm_host.h [...] > > @@ -384,4 +385,14 @@ static inline void __cpu_init_stage2(void) [...] > > +static inline void kvm_fpsimd_flush_cpu_state(void) > > +{ > > + if (system_supports_sve()) > > + sve_flush_cpu_state(); > > Hmmm. How does this work if... !IS_ENABLED(CONFIG_ARM64_SVE) implies !system_supports_sve(), so if CONFIG_ARM64_SVE is not set, the call is optimised away. [...] > > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > > index a9cb794..6ae3703 100644 > > --- a/arch/arm64/kernel/fpsimd.c > > +++ b/arch/arm64/kernel/fpsimd.c > > @@ -1073,6 +1073,33 @@ void fpsimd_flush_task_state(struct task_struct *t) [...] > > +#ifdef CONFIG_ARM64_SVE > > +void sve_flush_cpu_state(void) > > +{ > > + struct fpsimd_state *const fpstate = __this_cpu_read(fpsimd_last_state); > > + struct task_struct *tsk; > > + > > + if (!fpstate) > > + return; > > + > > + tsk = container_of(fpstate, struct task_struct, thread.fpsimd_state); > > + if (test_tsk_thread_flag(tsk, TIF_SVE)) > > + fpsimd_flush_cpu_state(); > > +} > > +#endif /* CONFIG_ARM64_SVE */ > > ... CONFIG_ARM64_SVE is not set? Fixing this should just be a matter of > moving the #ifdef/#endif inside the function... Because sve_flush_cpu_state() is not in the same compilation unit it can't be static, and that means the compiler won't remove it automatically if it's unused -- hence the #ifdef. Because the call site is optimised away, there is no link failure. Don't we rely on this sort of thing all over the place? Cheers ---Dave