On Thu, Jan 30, 2020 at 07:16:24AM -0800, Andy Lutomirski wrote: > > > On Jan 30, 2020, at 4:31 AM, David Laight <David.Laight@xxxxxxxxxx> wrote: > > > >> If split lock detect is enabled (warn/fatal), #AC handler calls die() > >> when split lock happens in kernel. > >> > >> A sane guest should never tigger emulation on a split-lock access, but > >> it cannot prevent malicous guest from doing this. So just emulating the > >> access as a write if it's a split-lock access to avoid malicous guest > >> polluting the kernel log. > > > > That doesn't seem right if, for example, the locked access is addx. > > ISTM it would be better to force an immediate fatal error of some > > kind than just corrupt the guest memory. > > The existing page-spanning case is just as wrong. Yes, it's a deliberate shortcut to handle a corner case that no real world workload will ever trigger[*]. The split-lock #AC case is the same. Actually, it's significantly less likely than the page-split case. With a sane, non-malicious guest, the emulator code in question only gets triggered if unrestricted guest is supported. Without unrestricted guest, there are certain modes, e.g. Big Real Mode, where VM-Enter will fail, in which case KVM needs to emulate the entire guest code stream until the guest transitions back to a valid mode (from VMX perspective). When unrestricted guest is enabled, the emulator is only invoked for MMIO, I/O strings, and for some instructions that are emulated on #UD to allow migrating VMs between hosts without heterogenous CPU capabilities. Unrestricted guest is supported on all Intel CPUs since Westmere, and will be supported on all CPUs that support split-lock #AC and VMX. Except for a few esoteric use cases where using shadow paging is more performant than using EPT, there is zero benefit to disabling unrestricted guest, whereas enabling unrestricted guest provides additional performance and security. In other words, the odds of a sane, non-malicious guest executing a split- lock instruction that needs to be emulated by KVM are basically zilch. The reason the emulator needs to handle this case at all is because a malicious guest could play TLB games to trick KVM into emulating a split- lock instruction, e.g. get the guest's translation for RIP pointing at a string I/O instruction to trigger VM-Exit, but have the host translation point at a completely different instruction. With split-lock #AC enable in the host, that would cause a kernel split-lock #AC and panic the whole system. Exiting to host userspace with "emulation failed" is the other reasonable alternative, but that's basically the same as killing the guest. We're arguing that, in the extremely unlikely event that there is a workload out there that hits this, it's preferable to *maybe* corrupt guest memory and log the anomaly in the kernel log, as opposed to outright killing the guest with a generic "emulation failed". Looking forward, the other reason for taking this shortcut is to easily handle the case where KVM adds support for exposing split-lock #AC to the guest. With this approach, we don't have to teach the emulator how to query for split-lock #AC enabling in the guest. Again, in the interest of not adding code to the emulator that is effectively useless. [*] https://lkml.kernel.org/r/c8b2219b-53d5-38d2-3407-2476b45500eb@xxxxxxxxxx
