On 2/11/2020 9:34 PM, Paolo Bonzini wrote:
On 11/02/20 14:22, Thomas Gleixner wrote:
Paolo Bonzini <pbonzini@xxxxxxxxxx> writes:
On 03/02/20 16:16, Xiaoyao Li wrote:
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.
Saying that anything doing a split lock access is malicious makes little
sense.
Correct, but we also have to accept, that split lock access can be used
in a malicious way, aka. DoS.
Indeed, a more accurate emulation such as temporarily disabling
split-lock detection in the emulator would allow the guest to use split
lock access as a vehicle for DoS, but that's not what the commit message
says. If it were only about polluting the kernel log, there's
printk_ratelimited for that. (In fact, if we went for incorrect
emulation as in this patch, a rate-limited pr_warn would be a good idea).
It is much more convincing to say that since this is pretty much a
theoretical case, we can assume that it is only done with the purpose of
DoS-ing the host or something like that, and therefore we kill the guest.
So you think there is no need to emulate this feature and return #AC to
guest?
Anyway, I'm fine with killing the guest.
BTW, Can it really be used for DoS purpose by malicious guest? Since
it's in kvm emulator so it needs vm-exit first and won't the die() in
kernel handler kill KVM? (Actually I'm not clear about KVM after die())
Split lock detection is essentially a debugging feature, there's a
reason why the MSR is called "TEST_CTL". So you don't want to make the
The fact that it ended up in MSR_TEST_CTL does not say anything. That's
where they it ended up to be as it was hastily cobbled together for
whatever reason.
Or perhaps it was there all the time in test silicon or something like
that... That would be a very plausible reason for all the quirks behind it.
Alright, I don't know the history of TEST_CTRL, there is a bit 31 in it
which means "Disable LOCK# assertion for split locked access" when set.
Bit 31 exists for a long period, but linux seems not use it so I guess
it may be a testing purpose bit.
However, when it comes to bit 29, split lock #AC, the main purpose is to
prevent any split lock more than debugging.
BTW, I guess the reason putting it in MSR_TEST_CTRL is that it's related
with split lock as bit 31.
Paolo
