Re: [PATCH v1 0/7] KVM: arm64: Fix handling of host fpsimd/sve state in protected mode

From: Oliver Upton <oliver.upton@linux.dev>
To: Marc Zyngier <maz@kernel.org>
Cc: Fuad Tabba <tabba@google.com>, Mark Brown <broonie@kernel.org>,
	kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	will@kernel.org, qperret@google.com, seanjc@google.com,
	alexandru.elisei@arm.com, catalin.marinas@arm.com,
	philmd@linaro.org, james.morse@arm.com, suzuki.poulose@arm.com,
	mark.rutland@arm.com, joey.gouly@arm.com, rananta@google.com,
	yuzenghui@huawei.com
Subject: Re: [PATCH v1 0/7] KVM: arm64: Fix handling of host fpsimd/sve state in protected mode
Date: Mon, 20 May 2024 17:37:36 +0000	[thread overview]
Message-ID: <ZkuKYA6-u-eZmBOO@linux.dev> (raw)
In-Reply-To: <87pltg3ntq.wl-maz@kernel.org>

On Mon, May 20, 2024 at 09:11:13AM +0100, Marc Zyngier wrote:
> On Mon, 20 May 2024 08:35:47 +0100, Fuad Tabba <tabba@google.com> wrote:
> > The reason for that is that in pKVM we want to avoid leaking any
> > information about protected VM activity to the host, including whether
> > the VM might have performed fpsimd/sve operations. Therefore, we need
> > to ensure that the host SVE state looks the same after a protected
> > guest has run as it did before a protected guest has run.

Wouldn't it be equally valid to just zero the state that will not be
preserved regardless of whether or not the guest used fpsimd/sve?

> > It would be correct to only save/restore the host's fpsimd state
> > (i.e., first 128 bits of the vector registers), which is what KVM does
> > in other modes. However, unless we always zero out the rest of the
> > state, regardless whether the protected guest has used fpsimd/sve,
> > then the host would be able to find out that the guest has in fact
> > performed fpsimd/sve operations.
> > 
> > This isn't necessary for non-protected VMs, but Marc thought that for
> > now it would be better to simplify things and have pKVM behave the
> > same way for both protected and non-protected VMs. As a future
> > optimization for non-protected VMs, we could have them behave as VMs
> > in other modes.
> 
> And I stand by what I said. Having a hybrid mode is a maintenance
> burden, and it will absolutely lead to some sort of horrible bugs (it
> just take a look at the mailing list to see that we have no shortage
> of bugs related to lazy FP/SVE handling).

Agree, but I don't think the suggestion is in any way incompatible with
eager save/restore of FP/SVE state.

From the looks of it, we're *still* adding protected-mode specialization
to save/restore the host's SVE state, even though we decided in commit
8383741ab2e7 ("KVM: arm64: Get rid of host SVE tracking/saving") that
this was completely unnecessary in non-protected configurations.

What I'm instead suggesting is that we make it part of the __kvm_vcpu_run() API
that the non-overlapping SVE state gets discarded by the callee, which
would align with an expectation that the host kernel has already done
this upon syscall entry.

Then all of the FPSIMD/SVE save/restore logic we have in the hyp 'just
works' so long as we 0 the SVE registers before loading in the host's
FPSIMD state.

-- 
Thanks,
Oliver