[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aJ-5XDXp1CxKB_7J@google.com>
Date: Fri, 15 Aug 2025 15:49:00 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Borislav Petkov <bp@...en8.de>
Cc: Mario Limonciello <mario.limonciello@....com>, Yazen Ghannam <yazen.ghannam@....com>, x86@...nel.org,
linux-kernel@...r.kernel.org, Libing He <libhe@...hat.com>,
David Arcari <darcari@...hat.com>
Subject: Re: [PATCH] x86/CPU/AMD: Ignore invalid reset reason value
On Sat, Aug 16, 2025, Borislav Petkov wrote:
> On Fri, Aug 15, 2025 at 02:39:50PM -0700, Sean Christopherson wrote:
> > Not all of those use cases will want to access "bare metal" features, but my
> > point is that inferring anything about the system setup based solely on the
> > HYPERVISOR flag is all but guaranteed to break sooner or later.
>
> No, that's *exactly* why I will want to check HYPERVISOR. Because I don't know
> what any of that virt zoo of paravisors, smaller layers, this and that layers
> are hiding from me.
>
> That's exactly why I cannot trust that register anymore.
>
> Or do you have a better suggestion how am I to know in the kernel whether the
> values I read from that register actually come from the hardware the feature
> support was added for? Or not some funky layer in-between.
I'm saying the kernel shouldn't care. This patch fixed a bug/flaw where the
kernel didn't check for valid data. Here's another in-progress bug where the
kernel isn't sufficiently paranoid/hardened and runs afoul of a VMM that is
operating within spec, but differently from bare metal systems:
https://lore.kernel.org/all/ZjEfVNwRnE1GUd1T@google.com
In both cases, the kernel can avoid badness without needing to know/care where
an unexpected value/result came from.
For things like zen2_zenbleed_check() where the kernel needs to check a ucode
revision and toggle a uarch specific chicken bit, yeah, go ahead and query
HYPERVISOR because there's no sane alternative. But I want HYPERVISOR to be
viewed as an absolute last resort, not as a way to paper over kernel flaws.
> Hell, I can't even trust HYPERVISOR. Because some sneaky layer might not even
> set it.
>
> Which makes this even worse.
That's pretty much my point: HYPERVISOR says that there is a hypervisor underneath,
and that's about it.
> > That seems like a gigantic waste of time/effort. E.g. imagine the pain/churn if
> > MONITOR/MWAIT had been limited to !HYPERVISOR simply because early VM setups
> > didn't support executing MONITOR/MWAIT natively.
>
> No, that's called enablement. You test the kernel on your new hw or HV env and
> then you send patches and claim support. And we support it.
That simply can't scale. If you really want to make everything off-by-default
for VMs, then the kernel would need hundreds of HYPERVISOR checks.
How would you draw the line, i.e. how would you choose which features get to be
automatically used when running as a guest and which features need explicit
enablement from someone? I think the only reasonable answer is that features
with any kind of detection mechanism should just use that and not check HYPERVISOR,
because checking HYPERVISOR would add an extra layer of "enablement" with no
meaningful benefit.
Which is what I'm suggesting here. If an MMIO load reads back -1u, then it's a
darn good signal that FCH_PM_S5_RESET_STATUS is either unsupported or malfunctioning.
I don't see any reason to drag HYPERVISOR into the mess.
> > I don't mind HYPERVISOR per se, what I don't like is disabling code (or going
> > down different paths) based solely on historical use cases, i.e. based on the
> > assumption that "no one will ever do xyz!". I have no objection to pivoting
> > on HYPERVISOR if there is an explicitly documented limitation, or for cases where
> > there's a very high probability of making a bad decision, e.g. using FMS to
> > enumerate features.
>
> I don't think you're reading me here: it is not "no one will ever do". It is
> "off until someone really says she will do it. And do it according to the hw spec."
I am 100% in favor in holding VMMs to hardware specs, but IMO this isn't a case
where the VMM is operating out of spec.
Powered by blists - more mailing lists