All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: Mark Brown <broonie@kernel.org>
Cc: kernel-team@android.com, kvm@vger.kernel.org,
	Will Deacon <will@kernel.org>,
	stable@vger.kernel.org, kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 01/18] KVM: arm64: Always start with clearing SVE flag on load
Date: Mon, 06 Jun 2022 12:28:32 +0100	[thread overview]
Message-ID: <87y1ya3uan.wl-maz@kernel.org> (raw)
In-Reply-To: <YpTXsgd1MPpJEjUJ@sirena.org.uk>

On Mon, 30 May 2022 15:41:54 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> [1  <text/plain; us-ascii (quoted-printable)>]
> On Sat, May 28, 2022 at 12:38:11PM +0100, Marc Zyngier wrote:
> > On each vcpu load, we set the KVM_ARM64_HOST_SVE_ENABLED
> > flag if SVE is enabled for EL0 on the host. This is used to restore
> > the correct state on vpcu put.
> > 
> > However, it appears that nothing ever clears this flag. Once
> > set, it will stick until the vcpu is destroyed, which has the
> > potential to spuriously enable SVE for userspace.
> 
> Oh dear.
> 
> Reviewed-by: Mark Brown <broonie@kernel.org>
> 
> > We probably never saw the issue because no VMM uses SVE, but
> > that's still pretty bad. Unconditionally clearing the flag
> > on vcpu load addresses the issue.
> 
> Unless I'm missing something since we currently always disable
> SVE on syscall even if the VMM were using SVE for some reason
> (SVE memcpy()?) we should already have disabled SVE for EL0 in
> sve_user_discard() during kernel entry so EL0 access to SVE
> should be disabled in the system register by the time we get
> here.

Indeed. And this begs the question: what is this code actually doing?
Is there any way we can end-up running a guest with any valid host SVE
state?

I remember being >this< close to removing that code some time ago, and
only stopped because I vaguely remembered Dave Martin convincing me at
some point that it was necessary. I'm unable to piece the argument
together again though.

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: Mark Brown <broonie@kernel.org>
Cc: kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	James Morse <james.morse@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Oliver Upton <oupton@google.com>, Will Deacon <will@kernel.org>,
	Fuad Tabba <tabba@google.com>,
	Quentin Perret <qperret@google.com>,
	kernel-team@android.com, stable@vger.kernel.org
Subject: Re: [PATCH 01/18] KVM: arm64: Always start with clearing SVE flag on load
Date: Mon, 06 Jun 2022 12:28:32 +0100	[thread overview]
Message-ID: <87y1ya3uan.wl-maz@kernel.org> (raw)
In-Reply-To: <YpTXsgd1MPpJEjUJ@sirena.org.uk>

On Mon, 30 May 2022 15:41:54 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> [1  <text/plain; us-ascii (quoted-printable)>]
> On Sat, May 28, 2022 at 12:38:11PM +0100, Marc Zyngier wrote:
> > On each vcpu load, we set the KVM_ARM64_HOST_SVE_ENABLED
> > flag if SVE is enabled for EL0 on the host. This is used to restore
> > the correct state on vpcu put.
> > 
> > However, it appears that nothing ever clears this flag. Once
> > set, it will stick until the vcpu is destroyed, which has the
> > potential to spuriously enable SVE for userspace.
> 
> Oh dear.
> 
> Reviewed-by: Mark Brown <broonie@kernel.org>
> 
> > We probably never saw the issue because no VMM uses SVE, but
> > that's still pretty bad. Unconditionally clearing the flag
> > on vcpu load addresses the issue.
> 
> Unless I'm missing something since we currently always disable
> SVE on syscall even if the VMM were using SVE for some reason
> (SVE memcpy()?) we should already have disabled SVE for EL0 in
> sve_user_discard() during kernel entry so EL0 access to SVE
> should be disabled in the system register by the time we get
> here.

Indeed. And this begs the question: what is this code actually doing?
Is there any way we can end-up running a guest with any valid host SVE
state?

I remember being >this< close to removing that code some time ago, and
only stopped because I vaguely remembered Dave Martin convincing me at
some point that it was necessary. I'm unable to piece the argument
together again though.

	M.

-- 
Without deviation from the norm, progress is not possible.

WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: Mark Brown <broonie@kernel.org>
Cc: kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	James Morse <james.morse@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Oliver Upton <oupton@google.com>, Will Deacon <will@kernel.org>,
	Fuad Tabba <tabba@google.com>,
	Quentin Perret <qperret@google.com>,
	kernel-team@android.com, stable@vger.kernel.org
Subject: Re: [PATCH 01/18] KVM: arm64: Always start with clearing SVE flag on load
Date: Mon, 06 Jun 2022 12:28:32 +0100	[thread overview]
Message-ID: <87y1ya3uan.wl-maz@kernel.org> (raw)
In-Reply-To: <YpTXsgd1MPpJEjUJ@sirena.org.uk>

On Mon, 30 May 2022 15:41:54 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> [1  <text/plain; us-ascii (quoted-printable)>]
> On Sat, May 28, 2022 at 12:38:11PM +0100, Marc Zyngier wrote:
> > On each vcpu load, we set the KVM_ARM64_HOST_SVE_ENABLED
> > flag if SVE is enabled for EL0 on the host. This is used to restore
> > the correct state on vpcu put.
> > 
> > However, it appears that nothing ever clears this flag. Once
> > set, it will stick until the vcpu is destroyed, which has the
> > potential to spuriously enable SVE for userspace.
> 
> Oh dear.
> 
> Reviewed-by: Mark Brown <broonie@kernel.org>
> 
> > We probably never saw the issue because no VMM uses SVE, but
> > that's still pretty bad. Unconditionally clearing the flag
> > on vcpu load addresses the issue.
> 
> Unless I'm missing something since we currently always disable
> SVE on syscall even if the VMM were using SVE for some reason
> (SVE memcpy()?) we should already have disabled SVE for EL0 in
> sve_user_discard() during kernel entry so EL0 access to SVE
> should be disabled in the system register by the time we get
> here.

Indeed. And this begs the question: what is this code actually doing?
Is there any way we can end-up running a guest with any valid host SVE
state?

I remember being >this< close to removing that code some time ago, and
only stopped because I vaguely remembered Dave Martin convincing me at
some point that it was necessary. I'm unable to piece the argument
together again though.

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2022-06-06 11:28 UTC|newest]

Thread overview: 141+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-28 11:38 [PATCH 00/18] KVM/arm64: Refactoring the vcpu flags Marc Zyngier
2022-05-28 11:38 ` Marc Zyngier
2022-05-28 11:38 ` Marc Zyngier
2022-05-28 11:38 ` [PATCH 01/18] KVM: arm64: Always start with clearing SVE flag on load Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-30 14:41   ` Mark Brown
2022-05-30 14:41     ` Mark Brown
2022-05-30 14:41     ` Mark Brown
2022-06-06 11:28     ` Marc Zyngier [this message]
2022-06-06 11:28       ` Marc Zyngier
2022-06-06 11:28       ` Marc Zyngier
2022-06-06 12:16       ` Mark Brown
2022-06-06 12:16         ` Mark Brown
2022-06-06 12:16         ` Mark Brown
2022-05-28 11:38 ` [PATCH 02/18] KVM: arm64: Always start with clearing SME " Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-30 14:51   ` Mark Brown
2022-05-30 14:51     ` Mark Brown
2022-05-30 14:51     ` Mark Brown
2022-05-28 11:38 ` [PATCH 03/18] KVM: arm64: Drop FP_FOREIGN_STATE from the hypervisor code Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-06-03  5:23   ` Reiji Watanabe
2022-06-03  5:23     ` Reiji Watanabe
2022-06-03  5:23     ` Reiji Watanabe
2022-06-04  8:10     ` Marc Zyngier
2022-06-04  8:10       ` Marc Zyngier
2022-06-04  8:10       ` Marc Zyngier
2022-06-07  4:47       ` Reiji Watanabe
2022-06-07  4:47         ` Reiji Watanabe
2022-06-07  4:47         ` Reiji Watanabe
2022-06-03  9:09   ` Mark Brown
2022-06-03  9:09     ` Mark Brown
2022-06-03  9:09     ` Mark Brown
2022-05-28 11:38 ` [PATCH 04/18] KVM: arm64: Move FP state ownership from flag to a tristate Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-06-03  9:14   ` Mark Brown
2022-06-03  9:14     ` Mark Brown
2022-06-03  9:14     ` Mark Brown
2022-06-06  8:41     ` Marc Zyngier
2022-06-06  8:41       ` Marc Zyngier
2022-06-06  8:41       ` Marc Zyngier
2022-06-06 10:31       ` Mark Brown
2022-06-06 10:31         ` Mark Brown
2022-06-06 10:31         ` Mark Brown
2022-06-04  8:16   ` Reiji Watanabe
2022-06-04  8:16     ` Reiji Watanabe
2022-06-04  8:16     ` Reiji Watanabe
2022-05-28 11:38 ` [PATCH 05/18] KVM: arm64: Add helpers to manipulate vcpu flags among a set Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-06-08  5:26   ` Reiji Watanabe
2022-06-08  5:26     ` Reiji Watanabe
2022-06-08  5:26     ` Reiji Watanabe
2022-06-08  6:51     ` Marc Zyngier
2022-06-08  6:51       ` Marc Zyngier
2022-06-08  6:51       ` Marc Zyngier
2022-06-09  2:25       ` Reiji Watanabe
2022-06-09  2:25         ` Reiji Watanabe
2022-06-09  2:25         ` Reiji Watanabe
2022-05-28 11:38 ` [PATCH 06/18] KVM: arm64: Add three sets of flags to the vcpu state Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-06-08 15:23   ` Fuad Tabba
2022-06-08 15:23     ` Fuad Tabba
2022-06-08 15:23     ` Fuad Tabba
2022-06-09  6:10   ` Reiji Watanabe
2022-06-09  6:10     ` Reiji Watanabe
2022-06-09  6:10     ` Reiji Watanabe
2022-06-09  7:46     ` Marc Zyngier
2022-06-09  7:46       ` Marc Zyngier
2022-06-09  7:46       ` Marc Zyngier
2022-06-09 17:24       ` Reiji Watanabe
2022-06-09 17:24         ` Reiji Watanabe
2022-06-09 17:24         ` Reiji Watanabe
2022-06-10  7:48         ` Marc Zyngier
2022-06-10  7:48           ` Marc Zyngier
2022-06-10  7:48           ` Marc Zyngier
2022-05-28 11:38 ` [PATCH 07/18] KVM: arm64: Move vcpu configuration flags into their own set Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-06-09  6:15   ` Reiji Watanabe
2022-06-09  6:15     ` Reiji Watanabe
2022-06-09  6:15     ` Reiji Watanabe
2022-05-28 11:38 ` [PATCH 08/18] KVM: arm64: Move vcpu PC/Exception flags to the input flag set Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-06-10  6:13   ` Reiji Watanabe
2022-06-10  6:13     ` Reiji Watanabe
2022-06-10  6:13     ` Reiji Watanabe
2022-05-28 11:38 ` [PATCH 09/18] KVM: arm64: Move vcpu debug/SPE/TRBE " Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-06-08 15:16   ` Fuad Tabba
2022-06-08 15:16     ` Fuad Tabba
2022-06-08 15:16     ` Fuad Tabba
2022-06-08 16:01     ` Marc Zyngier
2022-06-08 16:01       ` Marc Zyngier
2022-06-08 16:01       ` Marc Zyngier
2022-05-28 11:38 ` [PATCH 10/18] KVM: arm64: Move vcpu SVE/SME flags to the state " Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38 ` [PATCH 11/18] KVM: arm64: Move vcpu ON_UNSUPPORTED_CPU flag " Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38 ` [PATCH 12/18] KVM: arm64: Move vcpu WFIT " Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38 ` [PATCH 13/18] KVM: arm64: Kill unused vcpu flags field Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38 ` [PATCH 14/18] KVM: arm64: Convert vcpu sysregs_loaded_on_cpu to a state flag Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38 ` [PATCH 15/18] KVM: arm64: Warn when PENDING_EXCEPTION and INCREMENT_PC are set together Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-06-08 15:16   ` Fuad Tabba
2022-06-08 15:16     ` Fuad Tabba
2022-06-08 15:16     ` Fuad Tabba
2022-06-08 16:42     ` Marc Zyngier
2022-06-08 16:42       ` Marc Zyngier
2022-06-08 16:42       ` Marc Zyngier
2022-05-28 11:38 ` [PATCH 16/18] KVM: arm64: Add build-time sanity checks for flags Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38 ` [PATCH 17/18] KVM: arm64: Reduce the size of the vcpu flag members Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38 ` [PATCH 18/18] KVM: arm64: Document why pause cannot be turned into a flag Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-28 11:38   ` Marc Zyngier
2022-05-30  8:28 ` [PATCH 00/18] KVM/arm64: Refactoring the vcpu flags Marc Zyngier
2022-05-30  8:28   ` Marc Zyngier
2022-05-30  8:28   ` Marc Zyngier
2022-06-07 13:43 ` Marc Zyngier
2022-06-07 13:43   ` Marc Zyngier
2022-06-07 13:43   ` Marc Zyngier

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=87y1ya3uan.wl-maz@kernel.org \
    --to=maz@kernel.org \
    --cc=broonie@kernel.org \
    --cc=kernel-team@android.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=stable@vger.kernel.org \
    --cc=will@kernel.org \
    /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: link
Be 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.