| Message ID | ZPqQEHXgmak1LMNh@mattapan.m5p.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Revert "EDAC/mce_amd: Do not load edac_mce_amd module on guests" | expand |
On Thu, Sep 07, 2023 at 08:08:00PM -0700, Elliott Mitchell wrote: > This reverts commit 767f4b620edadac579c9b8b6660761d4285fa6f9. > > There are at least 3 valid reasons why a VM may see MCE events/registers. Hmm, so they all read like a bunch of handwaving to me, with those probable hypothetical "may" formulations. How about we cut to the chase and you explain what exactly is the concrete issue you're encountering and trying to solve? Thx.
On 9/7/23 11:59 PM, Borislav Petkov wrote: > On Thu, Sep 07, 2023 at 08:08:00PM -0700, Elliott Mitchell wrote: >> This reverts commit 767f4b620edadac579c9b8b6660761d4285fa6f9. >> >> There are at least 3 valid reasons why a VM may see MCE events/registers. > > Hmm, so they all read like a bunch of handwaving to me, with those > probable hypothetical "may" formulations. > > How about we cut to the chase and you explain what exactly is the > concrete issue you're encountering and trying to solve? Also, please note that the EDAC modules don't handle MCE events directly. They act on information passed from the MCE subsystem. Furthermore, there are other EDAC modules that have the same !hypervisor check, so why change only this one? Thanks, Yazen
> Also, please note that the EDAC modules don't handle MCE events > directly. They act on information passed from the MCE subsystem. > > Furthermore, there are other EDAC modules that have the same !hypervisor > check, so why change only this one? The older Intel EDAC drivers translated system physical addresses to DIMM addresses by digging around in the CONFIG and MMIO space of the memory controller devices. It would seem unwise for a VMM to give access to those addresses to a guest (in general ... perhaps OK for a Xen style "DOM0" guest that is handling many tasks for the VMM?). What system resources do AMD EDAC drivers need access to? Could they work inside a guest? -Tony
On 9/13/23 11:50 AM, Luck, Tony wrote: >> Also, please note that the EDAC modules don't handle MCE events >> directly. They act on information passed from the MCE subsystem. >> >> Furthermore, there are other EDAC modules that have the same !hypervisor >> check, so why change only this one? > > The older Intel EDAC drivers translated system physical addresses to DIMM > addresses by digging around in the CONFIG and MMIO space of the memory > controller devices. It would seem unwise for a VMM to give access to those > addresses to a guest (in general ... perhaps OK for a Xen style "DOM0" guest that is > handling many tasks for the VMM?). > > What system resources do AMD EDAC drivers need access to? Could they > work inside a guest? > The MCE decoder may access some newer MCA registers, or request info from the MCE subsystem. But this is for informational error decoding. It won't support any actions that a guest could take. The AMD64 EDAC module reads system-specific memory controller registers through non-architectural interfaces. So also unwise or not useful for a guest to access. Thanks, Yazen
On Fri, Sep 08, 2023 at 05:59:11AM +0200, Borislav Petkov wrote: > On Thu, Sep 07, 2023 at 08:08:00PM -0700, Elliott Mitchell wrote: > > This reverts commit 767f4b620edadac579c9b8b6660761d4285fa6f9. > > > > There are at least 3 valid reasons why a VM may see MCE events/registers. > > Hmm, so they all read like a bunch of handwaving to me, with those > probable hypothetical "may" formulations. Indeed. At what point is the lack of information and response long enough to simply commit a revert due to those lacks? Even with the commit message having been rewritten and the link to: https://lkml.kernel.org/r/20210628172740.245689-1-Smita.KoralahalliChannabasappa@amd.com added, this still reads as roughly: "A hypothetical bug on a hypothetivisor" I rather suspect a genuine issue was observed, but with absolutely no detail this is useless. I can make some guesses, but those guesses relation to reality is dubious. On Wed, Sep 13, 2023 at 03:50:12PM +0000, Luck, Tony wrote: > > Also, please note that the EDAC modules don't handle MCE events > > directly. They act on information passed from the MCE subsystem. > > > > Furthermore, there are other EDAC modules that have the same !hypervisor > > check, so why change only this one? > > The older Intel EDAC drivers translated system physical addresses to DIMM > addresses by digging around in the CONFIG and MMIO space of the memory > controller devices. It would seem unwise for a VMM to give access to those > addresses to a guest (in general ... perhaps OK for a Xen style "DOM0" guest that is > handling many tasks for the VMM?). Which seems oddly similar to: "the Linux kernel may be handling adminstrative duties/hardware for a hypervisor. In this case, the events need to be processed and potentially passed back through the hypervisor." On Wed, Sep 13, 2023 at 12:21:50PM -0400, Yazen Ghannam wrote: > The MCE decoder may access some newer MCA registers, or request info > from the MCE subsystem. But this is for informational error decoding. It > won't support any actions that a guest could take. > > The AMD64 EDAC module reads system-specific memory controller registers > through non-architectural interfaces. So also unwise or not useful for a > guest to access. This could be emulated. With it not being officially specified the emulation may not be too accurate, but it is possible. Admittedly VMware may have abandoned this level of perfect emulation accuracy, but one could do it. Which would be "full virtualization of MCE events." On Wed, Sep 13, 2023 at 10:36:50AM -0400, Yazen Ghannam wrote: > Furthermore, there are other EDAC modules that have the same !hypervisor > check, so why change only this one? Indeed. Those will also need similar treatment, but that wouldn't be a revert of 767f4b620eda. I found 767f4b620eda in the process of looking for the correct hook point. There are at least two, and possibly more, points of view with regards to MCE and virtualization. I keep noticing most implementers are strictly thinking of perfect, full virtualization of hardware, and missing what is actually desired. Full virtualization is where you are renting an actual physical slice of actual hardware, proper virtualization of CEs and UEs is desireable. In reality most clients merely want to rent the processing power the hardware provides and not deal with actually owning the hardware. To them, CEs are an annoyance since they clutter logs and they're not something they're in a position to deal with. Instead the owner of the hardware wants the CEs so they can monitor hardware health. What you want depends on your SLAs, but the most prominent authors keep missing that many clients (VM owners) don't actually want to deal with CEs. A SLA could also state a single UE means discarding current VM state and rolling back to the last known good checkpoint.
On Thu, Sep 14, 2023 at 10:02:05AM -0700, Elliott Mitchell wrote: > Indeed. At what point is the lack of information and response long > enough to simply commit a revert due to those lacks? At no point. > Even with the commit message having been rewritten and the link to: > https://lkml.kernel.org/r/20210628172740.245689-1-Smita.KoralahalliChannabasappa@amd.com > added, this still reads as roughly: > > "A hypothetical bug on a hypothetivisor" If "Hypervisors likely do not expose the SMCA feature to the guest" doesn't explain to you what the problem is this commit is fixing, then I can't help you.
On Fri, Sep 15, 2023 at 01:56:31PM +0200, Borislav Petkov wrote: > On Thu, Sep 14, 2023 at 10:02:05AM -0700, Elliott Mitchell wrote: > > Indeed. At what point is the lack of information and response long > > enough to simply commit a revert due to those lacks? > > At no point. > > > Even with the commit message having been rewritten and the link to: > > https://lkml.kernel.org/r/20210628172740.245689-1-Smita.KoralahalliChannabasappa@amd.com > > added, this still reads as roughly: > > > > "A hypothetical bug on a hypothetivisor" > > If "Hypervisors likely do not expose the SMCA feature to the guest" > doesn't explain to you what the problem is this commit is fixing, then > I can't help you. Problem is you were objecting to 'probable hypothetical "may" formulations' in what I wrote, yet the original patch message overtly uses that word. In order for the first patch to be correct, it is insufficient for the condition to be unlikely. Ideally it should be mathematically proven impossible. As such I was writing about known counter-examples from the real world. Mainly at least one hypervisor (Xen) does tend to allow a particular VM to access sensitive system registers. Also it is entirely possible some hypervisor could proxy access to the registers and thus properly simulate the events. Not only that, but in fact this very strategy was already actively deployed: https://bugs.debian.org/810964 I'm less than 100% certain this successfully retrieves EDAC events on Xen right now, so I had been taking a look at the situation only to find 767f4b620eda. Perhaps everyone should consult with large-scale system administrators when doing things which effect them?
diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c index 9215c06783df..1b7fccfbb654 100644 --- a/drivers/edac/mce_amd.c +++ b/drivers/edac/mce_amd.c @@ -1361,9 +1361,6 @@ static int __init mce_amd_init(void) c->x86_vendor != X86_VENDOR_HYGON) return -ENODEV; - if (cpu_feature_enabled(X86_FEATURE_HYPERVISOR)) - return -ENODEV; - if (boot_cpu_has(X86_FEATURE_SMCA)) { xec_mask = 0x3f; goto out;