From: Christoffer Dall <cdall@linaro.org> To: Dave Martin <Dave.Martin@arm.com> Cc: linux-arch@vger.kernel.org, Okamoto Takayuki <tokamoto@jp.fujitsu.com>, libc-alpha@sourceware.org, Ard Biesheuvel <ard.biesheuvel@linaro.org>, Szabolcs Nagy <szabolcs.nagy@arm.com>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will.deacon@arm.com>, Marc Zyngier <marc.zyngier@arm.com>, Richard Sandiford <richard.sandiford@arm.com>, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v3 22/28] arm64/sve: KVM: Prevent guests from using SVE Date: Wed, 18 Oct 2017 15:23:23 +0200 [thread overview] Message-ID: <20171018132323.GG8900@cbox> (raw) In-Reply-To: <20171017143142.GX19485@e103592.cambridge.arm.com> On Tue, Oct 17, 2017 at 03:31:42PM +0100, Dave Martin wrote: > On Tue, Oct 17, 2017 at 01:50:24PM +0200, Christoffer Dall wrote: > > On Tue, Oct 10, 2017 at 07:38:39PM +0100, 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. > > > > I don't understand this paragraph, beginning from ", so this...". > > > > > > From reading the code, what I think is the reason for having to flush > > the SVE state (and mark the host state invalid) is that even though we > > disallow SVE usage in the guest, the guest can use the normal FP state, > > and while we always fully preserve the host state, this could still > > corrupt some additional SVE state not properly preserved for the host. > > Is that correct? > > Yes, that's right: the guest can't touch the SVE-specific registers > Pn/FFR, but FPSIMD accesses to Vn regs cause the high bits of the > corresponding SVE Zn registers to be clobbered. In any case, the > FPSIMD restore done by KVM after guest exit is sufficient to clobber > those bits even if the guest didn't do it. > > This is a band-aid for not making the KVM world switch code properly > SVE-aware yet. > > Does the following wording sound better: > > --8<-- > > On guest exit, high bits of the SVE Zn registers may have been > clobbered as a side-effect the execution of FPSIMD instructions in > the guest. The existing KVM host FPSIMD restore code is not > sufficient to restore these bits, so this patch explicitly marks > the CPU as not containing cached vector state for any task, this > forcing a reload on the next return to userspace. This is an > interim measure, in advance of adding full SVE awareness to KVM. > > Because of the duplication of this operation > (__this_cpu_write(fpsimd_last_state, NULL)), it is factored out as s/it is/is/ (I think) > a new helper fpsimd_flush_cpu_state() to make the purpose clearer. > > -->8-- > > > > > > > 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. > > > > Otherwise the patch itself looks good to me. > > Thanks, let me know about the above wording change though. > Yes, the wording is good and helps a lot. Thanks for writing that. Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
WARNING: multiple messages have this Message-ID (diff)
From: cdall@linaro.org (Christoffer Dall) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v3 22/28] arm64/sve: KVM: Prevent guests from using SVE Date: Wed, 18 Oct 2017 15:23:23 +0200 [thread overview] Message-ID: <20171018132323.GG8900@cbox> (raw) In-Reply-To: <20171017143142.GX19485@e103592.cambridge.arm.com> On Tue, Oct 17, 2017 at 03:31:42PM +0100, Dave Martin wrote: > On Tue, Oct 17, 2017 at 01:50:24PM +0200, Christoffer Dall wrote: > > On Tue, Oct 10, 2017 at 07:38:39PM +0100, 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. > > > > I don't understand this paragraph, beginning from ", so this...". > > > > > > From reading the code, what I think is the reason for having to flush > > the SVE state (and mark the host state invalid) is that even though we > > disallow SVE usage in the guest, the guest can use the normal FP state, > > and while we always fully preserve the host state, this could still > > corrupt some additional SVE state not properly preserved for the host. > > Is that correct? > > Yes, that's right: the guest can't touch the SVE-specific registers > Pn/FFR, but FPSIMD accesses to Vn regs cause the high bits of the > corresponding SVE Zn registers to be clobbered. In any case, the > FPSIMD restore done by KVM after guest exit is sufficient to clobber > those bits even if the guest didn't do it. > > This is a band-aid for not making the KVM world switch code properly > SVE-aware yet. > > Does the following wording sound better: > > --8<-- > > On guest exit, high bits of the SVE Zn registers may have been > clobbered as a side-effect the execution of FPSIMD instructions in > the guest. The existing KVM host FPSIMD restore code is not > sufficient to restore these bits, so this patch explicitly marks > the CPU as not containing cached vector state for any task, this > forcing a reload on the next return to userspace. This is an > interim measure, in advance of adding full SVE awareness to KVM. > > Because of the duplication of this operation > (__this_cpu_write(fpsimd_last_state, NULL)), it is factored out as s/it is/is/ (I think) > a new helper fpsimd_flush_cpu_state() to make the purpose clearer. > > -->8-- > > > > > > > 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. > > > > Otherwise the patch itself looks good to me. > > Thanks, let me know about the above wording change though. > Yes, the wording is good and helps a lot. Thanks for writing that. Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
next prev parent reply other threads:[~2017-10-18 13:23 UTC|newest] Thread overview: 253+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-10-10 18:38 [PATCH v3 00/28] ARM Scalable Vector Extension (SVE) Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-10 18:38 ` [PATCH v3 01/28] regset: Add support for dynamically sized regsets Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-11 14:14 ` Catalin Marinas 2017-10-11 14:14 ` Catalin Marinas 2017-10-11 14:14 ` Catalin Marinas 2017-10-11 14:45 ` Dave Martin 2017-10-11 14:45 ` Dave Martin 2017-10-11 14:45 ` Dave Martin 2017-10-11 14:45 ` Dave Martin 2017-10-10 18:38 ` [PATCH v3 02/28] arm64: KVM: Hide unsupported AArch64 CPU features from guests Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-11 14:14 ` Catalin Marinas 2017-10-11 14:14 ` Catalin Marinas 2017-10-11 16:21 ` Marc Zyngier 2017-10-11 16:21 ` Marc Zyngier 2017-10-17 13:51 ` Christoffer Dall 2017-10-17 13:51 ` Christoffer Dall 2017-10-17 14:08 ` Marc Zyngier 2017-10-17 14:08 ` Marc Zyngier 2017-10-18 13:20 ` Christoffer Dall 2017-10-18 13:20 ` Christoffer Dall 2017-10-18 14:45 ` Dave Martin 2017-10-18 14:45 ` Dave Martin 2017-10-18 19:19 ` Christoffer Dall 2017-10-18 19:19 ` Christoffer Dall 2017-10-10 18:38 ` [PATCH v3 03/28] arm64: efi: Add missing Kconfig dependency on KERNEL_MODE_NEON Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-11 14:16 ` Catalin Marinas 2017-10-11 14:16 ` Catalin Marinas 2017-10-11 14:35 ` Dave Martin 2017-10-11 14:35 ` Dave Martin 2017-10-10 18:38 ` [PATCH v3 04/28] arm64: Port deprecated instruction emulation to new sysctl interface Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-11 14:17 ` Catalin Marinas 2017-10-11 14:17 ` Catalin Marinas 2017-10-10 18:38 ` [PATCH v3 05/28] arm64: fpsimd: Simplify uses of {set, clear}_ti_thread_flag() Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-10 18:38 ` [PATCH v3 05/28] arm64: fpsimd: Simplify uses of {set,clear}_ti_thread_flag() Dave Martin 2017-10-11 14:19 ` [PATCH v3 05/28] arm64: fpsimd: Simplify uses of {set, clear}_ti_thread_flag() Catalin Marinas 2017-10-11 14:19 ` Catalin Marinas 2017-10-10 18:38 ` [PATCH v3 06/28] arm64/sve: System register and exception syndrome definitions Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-11 14:20 ` Catalin Marinas 2017-10-11 14:20 ` Catalin Marinas 2017-10-10 18:38 ` [PATCH v3 07/28] arm64/sve: Low-level SVE architectural state manipulation functions Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-11 14:28 ` Catalin Marinas 2017-10-11 14:28 ` Catalin Marinas 2017-10-11 14:28 ` Catalin Marinas 2017-10-11 14:39 ` Dave Martin 2017-10-11 14:39 ` Dave Martin 2017-10-10 18:38 ` [PATCH v3 08/28] arm64/sve: Kconfig update and conditional compilation support Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-11 14:29 ` Catalin Marinas 2017-10-11 14:29 ` Catalin Marinas 2017-10-11 14:29 ` Catalin Marinas 2017-10-10 18:38 ` [PATCH v3 09/28] arm64/sve: Signal frame and context structure definition Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-11 14:29 ` Catalin Marinas 2017-10-11 14:29 ` Catalin Marinas 2017-10-11 14:29 ` Catalin Marinas 2017-10-10 18:38 ` [PATCH v3 10/28] arm64/sve: Low-level CPU setup Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-11 14:30 ` Catalin Marinas 2017-10-11 14:30 ` Catalin Marinas 2017-10-10 18:38 ` [PATCH v3 11/28] arm64/sve: Core task context handling Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-11 16:15 ` Catalin Marinas 2017-10-11 16:15 ` Catalin Marinas 2017-10-12 16:05 ` Dave Martin 2017-10-12 16:05 ` Dave Martin 2017-10-13 13:57 ` Catalin Marinas 2017-10-13 13:57 ` Catalin Marinas 2017-10-13 17:53 ` Dave Martin 2017-10-13 17:53 ` Dave Martin 2017-10-10 18:38 ` [PATCH v3 12/28] arm64/sve: Support vector length resetting for new processes Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-11 16:16 ` Catalin Marinas 2017-10-11 16:16 ` Catalin Marinas 2017-10-11 16:16 ` Catalin Marinas 2017-10-10 18:38 ` [PATCH v3 13/28] arm64/sve: Signal handling support Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-11 16:40 ` Catalin Marinas 2017-10-11 16:40 ` Catalin Marinas 2017-10-12 16:11 ` Dave Martin 2017-10-12 16:11 ` Dave Martin 2017-10-13 11:17 ` Catalin Marinas 2017-10-13 11:17 ` Catalin Marinas 2017-10-13 11:17 ` Catalin Marinas 2017-10-13 14:26 ` Dave Martin 2017-10-13 14:26 ` Dave Martin 2017-10-10 18:38 ` [PATCH v3 14/28] arm64/sve: Backend logic for setting the vector length Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-11 16:43 ` Catalin Marinas 2017-10-11 16:43 ` Catalin Marinas 2017-10-10 18:38 ` [PATCH v3 15/28] arm64: cpufeature: Move sys_caps_initialised declarations Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-11 16:50 ` Catalin Marinas 2017-10-11 16:50 ` Catalin Marinas 2017-10-10 18:38 ` [PATCH v3 16/28] arm64/sve: Probe SVE capabilities and usable vector lengths Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-11 16:55 ` Catalin Marinas 2017-10-11 16:55 ` Catalin Marinas 2017-10-12 12:56 ` Suzuki K Poulose 2017-10-12 12:56 ` Suzuki K Poulose 2017-10-16 15:46 ` Dave Martin 2017-10-16 15:46 ` Dave Martin 2017-10-16 16:27 ` Suzuki K Poulose 2017-10-16 16:27 ` Suzuki K Poulose 2017-10-16 16:27 ` Suzuki K Poulose 2017-10-16 16:44 ` Dave Martin 2017-10-16 16:44 ` Dave Martin 2017-10-16 16:47 ` Suzuki K Poulose 2017-10-16 16:47 ` Suzuki K Poulose 2017-10-16 16:47 ` Suzuki K Poulose 2017-10-16 16:55 ` Dave Martin 2017-10-16 16:55 ` Dave Martin 2017-10-16 16:58 ` Suzuki K Poulose 2017-10-16 16:58 ` Suzuki K Poulose 2017-10-10 18:38 ` [PATCH v3 17/28] arm64/sve: Preserve SVE registers around kernel-mode NEON use Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-12 10:15 ` Catalin Marinas 2017-10-12 10:15 ` Catalin Marinas 2017-10-10 18:38 ` [PATCH v3 18/28] arm64/sve: Preserve SVE registers around EFI runtime service calls Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-12 10:57 ` Catalin Marinas 2017-10-12 10:57 ` Catalin Marinas 2017-10-12 10:57 ` Catalin Marinas 2017-10-10 18:38 ` [PATCH v3 19/28] arm64/sve: ptrace and ELF coredump support Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-12 17:06 ` Catalin Marinas 2017-10-12 17:06 ` Catalin Marinas 2017-10-13 16:16 ` Dave Martin 2017-10-13 16:16 ` Dave Martin 2017-10-13 16:16 ` Dave Martin 2017-10-18 10:32 ` Catalin Marinas 2017-10-18 10:32 ` Catalin Marinas 2017-10-18 16:02 ` Dave Martin 2017-10-18 16:02 ` Dave Martin 2017-10-10 18:38 ` [PATCH v3 20/28] arm64/sve: Add prctl controls for userspace vector length management Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-12 17:11 ` Catalin Marinas 2017-10-12 17:11 ` Catalin Marinas 2017-10-10 18:38 ` [PATCH v3 21/28] arm64/sve: Add sysctl to set the default vector length for new processes Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-12 17:11 ` Catalin Marinas 2017-10-12 17:11 ` Catalin Marinas 2017-10-10 18:38 ` [PATCH v3 22/28] arm64/sve: KVM: Prevent guests from using SVE Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-11 16:28 ` Marc Zyngier 2017-10-11 16:28 ` Marc Zyngier 2017-10-12 11:04 ` Dave Martin 2017-10-12 11:04 ` Dave Martin 2017-10-12 11:28 ` Marc Zyngier 2017-10-12 11:28 ` Marc Zyngier 2017-10-13 14:15 ` Dave Martin 2017-10-13 14:15 ` Dave Martin 2017-10-13 14:21 ` Marc Zyngier 2017-10-13 14:21 ` Marc Zyngier 2017-10-13 16:47 ` Dave Martin 2017-10-13 16:47 ` Dave Martin 2017-10-12 17:13 ` Catalin Marinas 2017-10-12 17:13 ` Catalin Marinas 2017-10-17 11:50 ` Christoffer Dall 2017-10-17 11:50 ` Christoffer Dall 2017-10-17 11:50 ` Christoffer Dall 2017-10-17 11:50 ` Christoffer Dall 2017-10-17 14:31 ` Dave Martin 2017-10-17 14:31 ` Dave Martin 2017-10-18 13:23 ` Christoffer Dall [this message] 2017-10-18 13:23 ` Christoffer Dall 2017-10-18 15:00 ` Dave Martin 2017-10-18 15:00 ` Dave Martin 2017-10-18 19:22 ` Christoffer Dall 2017-10-18 19:22 ` Christoffer Dall 2017-10-10 18:38 ` [PATCH v3 23/28] arm64/sve: KVM: Treat guest SVE use as undefined instruction execution Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-12 17:13 ` Catalin Marinas 2017-10-12 17:13 ` Catalin Marinas 2017-10-17 13:58 ` Christoffer Dall 2017-10-17 13:58 ` Christoffer Dall 2017-10-10 18:38 ` [PATCH v3 24/28] arm64/sve: KVM: Hide SVE from CPU features exposed to guests Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-11 16:31 ` Marc Zyngier 2017-10-11 16:31 ` Marc Zyngier 2017-10-12 17:13 ` Catalin Marinas 2017-10-12 17:13 ` Catalin Marinas 2017-10-17 13:58 ` Christoffer Dall 2017-10-17 13:58 ` Christoffer Dall 2017-10-17 14:07 ` Dave Martin 2017-10-17 14:07 ` Dave Martin 2017-10-17 14:29 ` Marc Zyngier 2017-10-17 14:29 ` Marc Zyngier 2017-10-17 14:29 ` Marc Zyngier 2017-10-17 15:47 ` Dave Martin 2017-10-17 15:47 ` Dave Martin 2017-10-18 13:21 ` Christoffer Dall 2017-10-18 13:21 ` Christoffer Dall 2017-10-18 15:01 ` Dave Martin 2017-10-18 15:01 ` Dave Martin 2017-10-18 16:49 ` Christoffer Dall 2017-10-18 16:49 ` Christoffer Dall 2017-10-10 18:38 ` [PATCH v3 25/28] arm64/sve: Detect SVE and activate runtime support Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-11 17:11 ` Suzuki K Poulose 2017-10-11 17:11 ` Suzuki K Poulose 2017-10-12 17:14 ` Catalin Marinas 2017-10-12 17:14 ` Catalin Marinas 2017-10-12 17:14 ` Catalin Marinas 2017-10-10 18:38 ` [PATCH v3 26/28] arm64/sve: Add documentation Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-11 9:50 ` Szabolcs Nagy 2017-10-11 9:50 ` Szabolcs Nagy [not found] ` <59DDE958.4080605-5wv7dgnIgG8@public.gmane.org> 2017-10-11 11:08 ` Dave Martin 2017-10-11 11:08 ` Dave Martin 2017-10-11 11:08 ` Dave Martin [not found] ` <20171011110811.GB19485-M5GwZQ6tE7x5pKCnmE3YQBJ8xKzm50AiAL8bYrjMMd8@public.gmane.org> 2017-10-11 11:30 ` Szabolcs Nagy 2017-10-11 11:30 ` Szabolcs Nagy 2017-10-11 11:30 ` Szabolcs Nagy 2017-10-13 14:24 ` Catalin Marinas 2017-10-13 14:24 ` Catalin Marinas 2017-10-13 17:17 ` Dave Martin 2017-10-13 17:17 ` Dave Martin [not found] ` <20171013171758.GO19485-M5GwZQ6tE7x5pKCnmE3YQBJ8xKzm50AiAL8bYrjMMd8@public.gmane.org> 2017-10-18 9:32 ` Catalin Marinas 2017-10-18 9:32 ` Catalin Marinas 2017-10-18 9:32 ` Catalin Marinas [not found] ` <20171013142421.j32jzisukewxtosx-+1aNUgJU5qkijLcmloz0ER/iLCjYCKR+VpNB7YpNyf8@public.gmane.org> 2017-10-13 17:35 ` Dave Martin 2017-10-13 17:35 ` Dave Martin 2017-10-13 17:35 ` Dave Martin 2017-10-10 18:38 ` [RFC PATCH v3 27/28] arm64: signal: Report signal frame size to userspace via auxv Dave Martin 2017-10-10 18:38 ` Dave Martin 2017-10-11 10:19 ` Szabolcs Nagy 2017-10-11 10:19 ` Szabolcs Nagy 2017-10-11 13:14 ` Dave P Martin 2017-10-11 13:14 ` Dave P Martin 2017-10-10 18:38 ` [RFC PATCH v3 28/28] arm64/sve: signal: Include SVE when computing AT_MINSIGSTKSZ Dave Martin 2017-10-10 18:38 ` Dave Martin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20171018132323.GG8900@cbox \ --to=cdall@linaro.org \ --cc=Dave.Martin@arm.com \ --cc=ard.biesheuvel@linaro.org \ --cc=catalin.marinas@arm.com \ --cc=kvmarm@lists.cs.columbia.edu \ --cc=libc-alpha@sourceware.org \ --cc=linux-arch@vger.kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=marc.zyngier@arm.com \ --cc=richard.sandiford@arm.com \ --cc=szabolcs.nagy@arm.com \ --cc=tokamoto@jp.fujitsu.com \ --cc=will.deacon@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.