[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250816084218.GAaKBEaukeGa6b5UBj@fat_crate.local>
Date: Sat, 16 Aug 2025 10:42:18 +0200
From: Borislav Petkov <bp@...en8.de>
To: Sean Christopherson <seanjc@...gle.com>
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 Fri, Aug 15, 2025 at 03:49:00PM -0700, Sean Christopherson wrote:
> 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.
Ok, let's play through how you suggest it:
We do not check HYPERVISOR.
The machine boots on *some* guest and it says:
"x86/amd: Previous system reset reason [0x1]: "power button was pressed for 4 seconds".
If this were:
* a normal KVM guest: the machine is straight lying here. There's no power
button.
* "to improve system stability/uptime, e.g. restart the VM if the kernel
crashes as opposed to rebooting the entire host" - this is basically telling
the guest owner that the *host* got rebooted. I'm not sure you want to tell
that to guest owners if you're a cloud vendor.
* If this guest type is lying: same issue as above.
So, what guarantees that hypervisors will adhere to the spec and DTRT here?
Hell, hypervisors don't even care about that probably because they don't know
yet that this thing exists or if they do, they *definitely* want to return an
error here and not disclose to guest owners why they got rebooted.
So the only thing that is workable here is if all hypervisors either return an
error value we do handle or they implement this properly.
And because I don't trust hypervisors to do this right because, as I said,
there needs to be a concentrated effort to support it - otherwise no one
cares, the *least* we can do here is:
if (s5_reset_reason_txt[i]) {
pr_info("x86/amd: Previous system reset reason [0x%08lx]: %s%s\n",
value, s5_reset_reason_txt[i],
(cpu_feature_enabled(X86_FEATURE_HYPERVISOR) ? " - unreliable: running on a HV" : ""));
}
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Powered by blists - more mailing lists