Received-SPF: pass (google.com: domain of linux-kernel+bounces-100490-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Tue, 12 Mar 2024 09:07:11 -0700
In-Reply-To: <BN9PR11MB527600EC915D668127B97E3C8C2B2@BN9PR11MB5276.namprd11.prod.outlook.com>
Precedence: bulk
Mime-Version: 1.0
References: <20240309010929.1403984-1-seanjc@google.com> <20240309010929.1403984-6-seanjc@google.com>
 <Ze5bee/qJ41IESdk@yzhao56-desk.sh.intel.com> <Ze-hC8NozVbOQQIT@google.com> <BN9PR11MB527600EC915D668127B97E3C8C2B2@BN9PR11MB5276.namprd11.prod.outlook.com>
Message-ID: <ZfB9rzqOWmbaOeHd@google.com>
Subject: Re: [PATCH 5/5] KVM: VMX: Always honor guest PAT on CPUs that support self-snoop
From: Sean Christopherson <seanjc@google.com>
To: Kevin Tian <kevin.tian@intel.com>
Cc: Yan Y Zhao <yan.y.zhao@intel.com>, Paolo Bonzini <pbonzini@redhat.com>, 
	Lai Jiangshan <jiangshanlai@gmail.com>, "Paul E. McKenney" <paulmck@kernel.org>, 
	Josh Triplett <josh@joshtriplett.org>, "kvm@vger.kernel.org" <kvm@vger.kernel.org>, 
	"rcu@vger.kernel.org" <rcu@vger.kernel.org>, 
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Yiwei Zhang <zzyiwei@google.com>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Mar 12, 2024, Kevin Tian wrote:
> > From: Sean Christopherson <seanjc@google.com>
> > Sent: Tuesday, March 12, 2024 8:26 AM
> >=20
> > On Mon, Mar 11, 2024, Yan Zhao wrote:
> > > For the case of !static_cpu_has(X86_FEATURE_SELFSNOOP) &&
> > > kvm_arch_has_noncoherent_dma(vcpu->kvm), I think we at least should w=
arn
> > > about unsafe before honoring guest memory type.
> >=20
> > I don't think it gains us enough to offset the potential pain such a
> > message would bring.  Assuming the warning isn't outright ignored, the =
most
> > likely scenario is that the warning will cause random end users to worr=
y
> > that the setup they've been running for years is broken, when in realit=
y
> > it's probably just fine for their
> > use case.
>=20
> Isn't the 'worry' necessary to allow end users evaluate whether "it's
> probably just fine for their use case"?

Realistically, outside of large scale deployments, no end user is going to =
be able
to make that evaluation, because practically speaking it requires someone w=
ith
quite low-level hardware knowledge to be able to make that judgment call.  =
And
counting by number of human end users (as opposed to number of VMs being ru=
n), I
am willing to bet that the overwhelming majority of KVM users aren't kernel=
 or
systems engineers.

Understandably, users tend to be alarmed by (or suspicious of) new warnings=
 that
show up.  E.g. see the ancient KVM_SET_TSS_ADDR pr_warn[*].  And recently, =
we had
an internal bug report filed against KVM because they observed a performanc=
e
regression when booting a KVM guest, and saw a new message about some CPU
vulnerability being mitigated on VM-Exit that showed up in their *guest* ke=
rnel.

In short, my concern is that adding a new pr_warn() will generate noise for=
 end
users *and* for KVM developers/maintainers, because even if we phrase the m=
essage
to talk specifically about "untrusted workloads", the majority of affected =
users
will not have the necessary knowledge to make an informed decision.

[*] https://lore.kernel.org/all/f1afa6c0-cde2-ab8b-ea71-bfa62a45b956@tarent=
de

> I saw the old comment already mentioned that doing so may lead to unexpec=
ted
> behaviors. But I'm not sure whether such code-level caveat has been visib=
le
> enough to end users.

Another point to consider: KVM is _always_ potentially broken on such CPUs,=
 as
KVM forces WB for guest accesses.  I.e. KVM will create memory aliasing if =
the
host has guest memory mapped as non-WB in the PAT, without non-coherent DMA
exposed to the guest.

> > I would be quite surprised if there are people running untrusted worklo=
ads
> > on 10+ year old silicon *and* have passthrough devices and non-coherent
> > IOMMUs/DMA.
>=20
> this is probably true.
>=20
> > And anyone exposing a device directly to an untrusted workload really
> > should have done their homework.
>=20
> or they run trusted workloads which might be tampered by virus to
> exceed the scope of their homework. =F0=9F=98=8A

If a workload is being run in a KVM guest for host isolation/security purpo=
ses,
and a device was exposed to said workload, then I would firmly consider ana=
lyzing
the impact of a compromised guest to be part of their homework.

> > And it's not like we're going to change KVM's historical behavior at th=
is point.
>=20
> I agree with your point of not breaking userspace. But still think a warn=
ing
> might be informative to let users evaluate their setup against a newly
> identified "unexpected behavior"  which has security implication beyond
> the guest, while the previous interpretation of "unexpected behavior"=20
> might be that the guest can at most shoot its own foot...

If this issue weren't limited to 10+ year old hardware, I would be more inc=
lined
to add a message.  But at this point, realistically the only thing KVM woul=
d be
saying is "you're running old hardware, that might be unsafe in today's wor=
ld".

For users that care about security, we'd be telling them something they alr=
eady
know (and if they don't know, they've got bigger problems).  And for everyo=
ne
else, it'd be scary noise without any meaningful benefit.