mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: William Roche <william.roche@oracle.com>
To: Yazen Ghannam <yazen.ghannam@amd.com>
Cc: Tony Luck <tony.luck@intel.com>,
	bp@alien8.de, Thomas Gleixner <tglx@linutronix.de>,
	mingo@redhat.com, dave.hansen@linux.intel.com, x86@kernel.org,
	hpa@zytor.com, "Allen, John" <John.Allen@amd.com>,
	linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org,
	Jane Chu <jane.chu@oracle.com>
Subject: Re: [RFC] AMD VM crashing on deferred memory error injection
Date: Fri, 13 Feb 2026 17:55:26 +0100	[thread overview]
Message-ID: <c6ddad69-c87e-4665-a54b-4dc02c7045d5@oracle.com> (raw)
In-Reply-To: <20260212193058.GA315621@yaz-khff2.amd.com>

On 2/12/26 20:30, Yazen Ghannam wrote:
> On Thu, Feb 12, 2026 at 04:36:47PM +0100, William Roche wrote:
>> [...]
>> Thinking more about the problem introduced by your commit, I realized
>> that only SMCA systems have MCA_DESTAT registers. So we should not
>> allow access to this register from a non SMCA machine.
>>   And Qemu AMD VM is an example of a non SMCA machine !
>>
> 
> So the SMCA CPUID bit is not advertised in this model?

No it's not advertised -- only succor and overflow-recov.
As implemented by John Allen to support relaying deferred errors:
https://lore.kernel.org/qemu-devel/20240603193622.47156-1-john.allen@amd.com/


>> So according to me, modifying the hypervisor kvm to allow the access
>> to MCA_DESTAT is clearly not the right move.
>>
>> We probably should implement an entire SMCA stack for Qemu, but this
>> is another topic...
>> For the moment, Borislav Petklov was right when he said that kvm works
>> as advertised. The problem that your fix introduced is that is tries to
>> access SMCA only registers on non SMCA machine.
>>
>> Do you agree on this aspect ?
>>
> 
> Yes, I agree.

Thanks


> AMD systems generally have a Read-as-Zero/Writes-Ignored behavior when
> accessing unimplemented MCA registers. But this requires the system to
> recognize the register space.
> 
> In this case, the register space is totally unknown to the system, so it
> responds with a #GP.

I understand what you mean about the platform permissiveness. This could
be a valid change of the kvm layer, allowing kernel code that would
mistakenly access SMCA registers on non-SMCA virtual machine not to
panic. Of course if all AMD hardware would work like that, I agree that
aligning the virtual AMD platforms should be done -- but as you
indicated, recognizing the register space to consider for
Read-as-Zero/Writes-Ignored is still unclear (to me).
Even if we manage to do that for an updated KVM layer, a non-SMCA VM
running on top of an older host kvm layer would still panic if its
kernel accesses SMCA registers. So I'm convinced that making sure the
AMD kernel accessing SMCA registers only on SMCA machines is the best
approach.

> 
>> If yes, than the correct change is to test if we are on an SMCA machine
>> before accessing this register, like:

I'm preparing a PATCH proposal to submit in a moment.

>>
>> diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c
>> index 3f1dda355307..8664ba048a62 100644
>> --- a/arch/x86/kernel/cpu/mce/amd.c
>> +++ b/arch/x86/kernel/cpu/mce/amd.c
>> @@ -875,14 +875,18 @@ void amd_clear_bank(struct mce *m)
>>   {
>>          amd_reset_thr_limit(m->bank);
>>
>> -       /* Clear MCA_DESTAT for all deferred errors even those logged in
>> MCA_STATUS. */
>> -       if (m->status & MCI_STATUS_DEFERRED)
>> -               mce_wrmsrq(MSR_AMD64_SMCA_MCx_DESTAT(m->bank), 0);
>> -
>> -       /* Don't clear MCA_STATUS if MCA_DESTAT was used exclusively. */
>> -       if (m->kflags & MCE_CHECK_DFR_REGS)
>> -               return;
>> +       if (mce_flags.smca) {
>> +               /*
>> +                * Clear MCA_DESTAT for all deferred errors even those
>> +                * logged in MCA_STATUS.
>> +                */
>> +               if (m->status & MCI_STATUS_DEFERRED)
>> +                       mce_wrmsrq(MSR_AMD64_SMCA_MCx_DESTAT(m->bank), 0);
>>
>> +               /* Don't clear MCA_STATUS if MCA_DESTAT was used
>> exclusively. */
>> +               if (m->kflags & MCE_CHECK_DFR_REGS)
>> +                       return;
>> +       }
>>          mce_wrmsrq(mca_msr_reg(m->bank, MCA_STATUS), 0);
>>   }
>>
>>
>> I haven't noticed any obvious other non SMCA limitation in the other
>> changes of this series, but if you think about any other case, we can
>> probably fix all of them together.
>>
>> If you agree with this change I can submit it as a formal PATCH.
>>
> 
> I think this change is fair. It could be minimized further by adding the
> SMCA check to the status bit check for the WRMSR step.
> 
> 	if (mce_flags.smca && (m->status & MCI_STATUS_DEFERRED))
> 		mce_wrmsrq(MSR_AMD64_SMCA_MCx_DESTAT(m->bank), 0);


Yes this is the minimal fix in our case, but I think that more clearly
separating the SMCA operations better shows the differences between SMCA
and non-SMCA.

I'll copy you on the PATCH proposal, maybe you can review it.

Thanks again for your detailed feedback.

Best regards,
William.

      reply	other threads:[~2026-02-13 16:56 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-09 16:36 William Roche
2026-02-09 17:36 ` Borislav Petkov
2026-02-09 17:38   ` Luck, Tony
2026-02-09 17:53     ` Borislav Petkov
2026-02-09 21:08 ` Yazen Ghannam
2026-02-09 21:18   ` Yazen Ghannam
2026-02-11  1:42     ` William Roche
2026-02-11 16:34       ` Yazen Ghannam
2026-02-12 15:36         ` William Roche
2026-02-12 19:30           ` Yazen Ghannam
2026-02-13 16:55             ` William Roche [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c6ddad69-c87e-4665-a54b-4dc02c7045d5@oracle.com \
    --to=william.roche@oracle.com \
    --cc=John.Allen@amd.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=jane.chu@oracle.com \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    --cc=yazen.ghannam@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®