| Message ID | 20210222115007.75b7de9b@alex-virtual-machine (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] x86/mce: fix wrong no-return-ip logic in do_machine_check() | expand |
On Mon, Feb 22, 2021 at 11:50:07AM +0800, Aili Yao wrote: > From commit b2f9d678e28c ("x86/mce: Check for faults tagged in > EXTABLE_CLASS_FAULT exception table entries"), When there is a > memory MCE_AR_SEVERITY error with no return ip, What is a "no return ip" - MCG_STATUS_RIPV? How do you trigger this error?
On Mon, 22 Feb 2021 10:24:03 +0100 Borislav Petkov <bp@alien8.de> wrote: > On Mon, Feb 22, 2021 at 11:50:07AM +0800, Aili Yao wrote: > > From commit b2f9d678e28c ("x86/mce: Check for faults tagged in > > EXTABLE_CLASS_FAULT exception table entries"), When there is a > > memory MCE_AR_SEVERITY error with no return ip, > > What is a "no return ip" - MCG_STATUS_RIPV? yes > How do you trigger this error? you can inject a memory UE to a VM, it should always be MCG_STATUS_RIPV 0. Best Regard! Aili Yao
On Mon, Feb 22, 2021 at 05:31:09PM +0800, Aili Yao wrote:
> you can inject a memory UE to a VM, it should always be MCG_STATUS_RIPV 0.
So the signature you injected is not something the hardware would
generate - you just didn't set MCG_STATUS_RIPV.
If so, why should the code handle invalid signatures which the harware
cannot generate?
On Mon, 22 Feb 2021 11:03:56 +0100 Borislav Petkov <bp@alien8.de> wrote: > On Mon, Feb 22, 2021 at 05:31:09PM +0800, Aili Yao wrote: > > you can inject a memory UE to a VM, it should always be MCG_STATUS_RIPV 0. > > So the signature you injected is not something the hardware would > generate - you just didn't set MCG_STATUS_RIPV. > > If so, why should the code handle invalid signatures which the harware > cannot generate? > So why would intel provide this MCG_STATUS_RIPV flag, it's better to remove it as it will never be set, and all the related logic for this flag is really needed ?
On Mon, Feb 22, 2021 at 06:08:19PM +0800, Aili Yao wrote: > So why would intel provide this MCG_STATUS_RIPV flag, it's better to > remove it as it will never be set, and all the related logic for this > flag is really needed ? Why would it never be set - of course it will be. You don't set it. If you wanna inject errors, then make sure you inject *valid* errors which the hardware *actually* generates, not some random ones.
On Mon, 22 Feb 2021 11:22:06 +0100 Borislav Petkov <bp@alien8.de> wrote: > On Mon, Feb 22, 2021 at 06:08:19PM +0800, Aili Yao wrote: > > So why would intel provide this MCG_STATUS_RIPV flag, it's better to > > remove it as it will never be set, and all the related logic for this > > flag is really needed ? > > Why would it never be set - of course it will be. You don't set it. If > you wanna inject errors, then make sure you inject *valid* errors which > the hardware *actually* generates, not some random ones. > As far as I know, Most of RAS related tests are faked, not real errors, and it's really meaningful. You should better reproduce the issue I tried to fix, or at least read the code more detailly and you will know if it's random and invalid Best Regards! Aili Yao
On Mon, 22 Feb 2021 19:21:46 +0800 Aili Yao <yaoaili@kingsoft.com> wrote: > On Mon, 22 Feb 2021 11:22:06 +0100 > Borislav Petkov <bp@alien8.de> wrote: > > > On Mon, Feb 22, 2021 at 06:08:19PM +0800, Aili Yao wrote: > > > So why would intel provide this MCG_STATUS_RIPV flag, it's better to > > > remove it as it will never be set, and all the related logic for this > > > flag is really needed ? > > > > Why would it never be set - of course it will be. You don't set it. If > > you wanna inject errors, then make sure you inject *valid* errors which > > the hardware *actually* generates, not some random ones. > > > > As far as I know, Most of RAS related tests are faked, not real errors, and it's really meaningful. > > You should better reproduce the issue I tried to fix, or at least read the code more detailly and you will > know if it's random and invalid > I See this in sdm 325462: AR (Action Required) flag, bit 55 - Indicates (when set) that MCA error code specific recovery action must be performed by system software at the time this error was signaled. This recovery action must be completed successfully before any additional work is scheduled for this processor. ------------------- When the RIPV flag in the IA32_MCG_STATUS is clear, an alternative execution stream needs to be provided; ------------------ when the MCA error code specific recovery specific recovery action cannot be successfully completed, system software must shut down the system. When the AR flag in the IA32_MCi_STATUS register is clear, system software may still take MCA error code specific recovery action but this is optional; system software can safely resume program execution at the instruction pointer saved on the stack from the machine check exception when the RIPV flag in the IA32_MCG_STATUS register is set. Best Regards! Aili Yao
On Mon, Feb 22, 2021 at 08:17:23PM +0800, Aili Yao wrote: > AR (Action Required) flag, bit 55 - Indicates (when set) that MCA > error code specific recovery action must be... Give me the *exact* MCE signature you're injecting please. Thx.
On Mon, 22 Feb 2021 13:22:41 +0100 Borislav Petkov <bp@alien8.de> wrote: > On Mon, Feb 22, 2021 at 08:17:23PM +0800, Aili Yao wrote: > > AR (Action Required) flag, bit 55 - Indicates (when set) that MCA > > error code specific recovery action must be... > > Give me the *exact* MCE signature you're injecting please. > > Thx. > Guest VM, the qemu has no way to know the RIPV value, so always get it cleared. Hardware event. This is not a software error. MCE 0 CPU 9 BANK 9 TSC 103d511e68c RIP 33:401270 MISC 8c ADDR 10e91d000 TIME 1613974147 Mon Feb 22 01:09:07 2021 MCG status:EIPV MCIP LMCE MCi status: Uncorrected error Error enabled MCi_MISC register valid MCi_ADDR register valid SRAR MCA: Data CACHE Level-0 Data-Read Error STATUS bd80000000000134 MCGSTATUS e MCGCAP 900010a APICID 9 SOCKETID 9 MICROCODE 1 CPUID Vendor Intel Family 6 Model 85 Step 7 Host: Hardware event. This is not a software error. MCE 0 CPU 1 BANK 1 TSC 1ee4f074462 RIP 33:4013a6 MISC 86 ADDR 10ed608000 TIME 1613985132 Mon Feb 22 17:12:12 2021 MCG status:RIPV EIPV MCIP LMCE MCi status: Uncorrected error Error enabled MCi_MISC register valid MCi_ADDR register valid SRAR MCA: Data CACHE Level-0 Data-Read Error STATUS bd80000000100134 MCGSTATUS f MCGCAP f000c14 APICID 2 SOCKETID 0 MICROCODE 5000021 CPUID Vendor Intel Family 6 Model 85
On Mon, Feb 22, 2021 at 08:35:49PM +0800, Aili Yao wrote: > Guest VM, the qemu has no way to know the RIPV value, so always get it > cleared. What does that mean? The guest VM will get the MCE signature it gets from the host kernel so the host kernel most definitely knows the RIPV value. It looks like you're testing how guests will handle MCEs which the host has caught and wants to inject into the guest for further handling. What is your exact use case? Please explain in detail how I can reproduce it step-by-step locally. Thx.
On Mon, 22 Feb 2021 13:45:50 +0100 Borislav Petkov <bp@alien8.de> wrote: > On Mon, Feb 22, 2021 at 08:35:49PM +0800, Aili Yao wrote: > > Guest VM, the qemu has no way to know the RIPV value, so always get it > > cleared. > > What does that mean? > > The guest VM will get the MCE signature it gets from the host kernel so > the host kernel most definitely knows the RIPV value. When Guest access one address with UE error, it will exit guest mode, the host will do the recovery job, and then one SIGBUS is send to the VCPU and qemu will catch the signal, there is only address and error level no RIPV in signal, so qemu will assume RIPV is cleared and inject the error into guest OS. > It looks like you're testing how guests will handle MCEs which the host > has caught and wants to inject into the guest for further handling. What > is your exact use case? Please explain in detail how I can reproduce it > step-by-step locally. Yeah, there are multiple steps i do: 1. One small test code in guest OS access one address A which will be injected UC error, the address will be logged, and use vtop you can get the guest physical address. 2. Using "virsh qemu-monitor-command guest --hmp gpa2hvagpa2hva 0xxxxxx" to get the user virtual address, 3. Using vtop you can get host physical address from the above user address. 4. Inject 0x10 level error using einj module. 5. then when guest access the address, you will see what happens. Please using latest upstream kernel for guest OS, and you may change monarch_timeout to a bigger value, or you will see other issues not only talked one. Tks Best Regards! Aili Yao
On Tue, Feb 23, 2021 at 10:27:55AM +0800, Aili Yao wrote: > When Guest access one address with UE error, it will exit guest mode, > the host will do the recovery job, and then one SIGBUS is send to > the VCPU and qemu will catch the signal, there is only address and > error level no RIPV in signal, so qemu will assume RIPV is cleared and > inject the error into guest OS. Lemme see: void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr) /* If we get an action required MCE, it has been injected by KVM * while the VM was running. An action optional MCE instead should * be coming from the main thread, which qemu_init_sigbus identifies * as the "early kill" thread. */ assert(code == BUS_MCEERR_AR || code == BUS_MCEERR_AO); ... kvm_mce_inject(cpu, paddr, code); in that function: if (code == BUS_MCEERR_AR) { status |= MCI_STATUS_AR | 0x134; mcg_status |= MCG_STATUS_EIPV; } else { status |= 0xc0; mcg_status |= MCG_STATUS_RIPV; } That looks like a valid RIP bit to me. Then cpu_x86_inject_mce() gets that mcg_status and injects it into the guest. So I can't follow your claim - qemu does handle RIPV just fine, it seems.
On Tue, 23 Feb 2021 10:43:00 +0100 Borislav Petkov <bp@alien8.de> wrote: > On Tue, Feb 23, 2021 at 10:27:55AM +0800, Aili Yao wrote: > > When Guest access one address with UE error, it will exit guest mode, > > the host will do the recovery job, and then one SIGBUS is send to > > the VCPU and qemu will catch the signal, there is only address and > > error level no RIPV in signal, so qemu will assume RIPV is cleared and > > inject the error into guest OS. > > Lemme see: > > void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr) > > /* If we get an action required MCE, it has been injected by KVM > * while the VM was running. An action optional MCE instead should > * be coming from the main thread, which qemu_init_sigbus identifies > * as the "early kill" thread. > */ > assert(code == BUS_MCEERR_AR || code == BUS_MCEERR_AO); > > ... > > kvm_mce_inject(cpu, paddr, code); > > in that function: > > if (code == BUS_MCEERR_AR) { > status |= MCI_STATUS_AR | 0x134; > mcg_status |= MCG_STATUS_EIPV; > } else { > status |= 0xc0; > mcg_status |= MCG_STATUS_RIPV; > } > > That looks like a valid RIP bit to me. Then cpu_x86_inject_mce() gets > that mcg_status and injects it into the guest. What i inject is AR error, and I don't see MCG_STATUS_RIPV flag. Tks Aili Yao
On Tue, Feb 23, 2021 at 05:56:40PM +0800, Aili Yao wrote:
> What i inject is AR error, and I don't see MCG_STATUS_RIPV flag.
Then keep debugging qemu to figure out why that is.
On Tue, 23 Feb 2021 11:05:38 +0100 Borislav Petkov <bp@alien8.de> wrote: > On Tue, Feb 23, 2021 at 05:56:40PM +0800, Aili Yao wrote: > > What i inject is AR error, and I don't see MCG_STATUS_RIPV flag. > > Then keep debugging qemu to figure out why that is. > What I think is qemu has not an easy to get the MCE signature from host or currently no methods for this So qemu treat all AR will be No RIPV, Do more is better than do less. Thanks Aili Yao
> What I think is qemu has not an easy to get the MCE signature from host or currently no methods for this > So qemu treat all AR will be No RIPV, Do more is better than do less. RIPV would be important in the guest in the case where the guest can fix the problem that caused the machine check and return to the failed instruction to continue. I think the only case where this happens is a fault in a read-only page mapped from a file (typically code page, but could be a data page). In this case memory-failure() unmaps the page with the posion but Linux can recover by reading data from the file into a new page. Other cases we send SIGBUS (so go to the signal handler instead of to the faulting instruction). So it would be good if the state of RIPV could be added to the signal state sent to qemu. If that isn't possible, then this full recovery case turns into another SIGBUS case. -Tony
On Tue, 23 Feb 2021 16:12:43 +0000 "Luck, Tony" <tony.luck@intel.com> wrote: > > What I think is qemu has not an easy to get the MCE signature from host or currently no methods for this > > So qemu treat all AR will be No RIPV, Do more is better than do less. > > RIPV would be important in the guest in the case where the guest can fix the problem that caused > the machine check and return to the failed instruction to continue. > > I think the only case where this happens is a fault in a read-only page mapped from a file (typically > code page, but could be a data page). In this case memory-failure() unmaps the page with the posion > but Linux can recover by reading data from the file into a new page. > > Other cases we send SIGBUS (so go to the signal handler instead of to the faulting instruction). > > So it would be good if the state of RIPV could be added to the signal state sent to qemu. If that > isn't possible, then this full recovery case turns into another SIGBUS case. This KVM and VM case of failing recovery for SRAR is just one scenario I think, If Intel guarantee that when memory SRAR is triggered, RIPV will always be set, then it's the job of qemu to set the RIPV instead. Or if When SRAR is triggered with RIPV cleared, the same issue will be true for host. And I think it's better for VM to know the real RIPV value, It need more work in qemu and kernel if possible. Thanks Aili Yao
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c index e133ce1e562b..70380d7d98b3 100644 --- a/arch/x86/kernel/cpu/mce/core.c +++ b/arch/x86/kernel/cpu/mce/core.c @@ -1414,7 +1414,10 @@ noinstr void do_machine_check(struct pt_regs *regs) /* If this triggers there is no way to recover. Die hard. */ BUG_ON(!on_thread_stack() || !user_mode(regs)); - queue_task_work(&m, kill_current_task); + if (worst == MCE_AR_SEVERITY) + queue_task_work(&m, 0); + else if (kill_current_task) + queue_task_work(&m, kill_current_task); } else { /*
From commit b2f9d678e28c ("x86/mce: Check for faults tagged in EXTABLE_CLASS_FAULT exception table entries"), When there is a memory MCE_AR_SEVERITY error with no return ip, Only a SIGBUS signal is send to current. As the page is not poisoned, the SIGBUS process's coredump step in kernel will touch the error page again, which result to a fatal error. We need to poison the page and then kill current in memory-failure module. So fix it using the orinigal checking method. Signed-off-by: Aili Yao <yaoaili@kingsoft.com> --- arch/x86/kernel/cpu/mce/core.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)