This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v3 22/28] arm64/sve: KVM: Prevent guests from using SVE
- From: Christoffer Dall <cdall at linaro dot org>
- To: Dave Martin <Dave dot Martin at arm dot com>
- Cc: linux-arch at vger dot kernel dot org, Okamoto Takayuki <tokamoto at jp dot fujitsu dot com>, libc-alpha at sourceware dot org, Ard Biesheuvel <ard dot biesheuvel at linaro dot org>, Szabolcs Nagy <szabolcs dot nagy at arm dot com>, Catalin Marinas <catalin dot marinas at arm dot com>, Will Deacon <will dot deacon at arm dot com>, Marc Zyngier <marc dot zyngier at arm dot com>, Richard Sandiford <richard dot sandiford at arm dot com>, kvmarm at lists dot cs dot columbia dot edu, linux-arm-kernel at lists dot infradead dot org
- Date: Wed, 18 Oct 2017 15:23:23 +0200
- Subject: Re: [PATCH v3 22/28] arm64/sve: KVM: Prevent guests from using SVE
- Authentication-results: sourceware.org; auth=none
- References: <1507660725-7986-1-git-send-email-Dave.Martin@arm.com> <1507660725-7986-23-git-send-email-Dave.Martin@arm.com> <20171017115024.GS1845@lvm> <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>