On 17/05/2022 14:21, Roger Pau Monne wrote: > Add support for enabling Bus Lock Detection on Intel systems. Such > detection works by triggering a vmexit, which is enough of a pause to > prevent a guest from abusing of the Bus Lock.
"which is enough of a" is a bit firmer than ideal. "which Andy says will be ok" is perhaps more accurate. Perhaps "which ought to be enough" ? A buslock here or there is no problem, and non-malicious software appears to be devoid of buslocks (hardly surprising - it would be a hard error on other architectures), but a malicious piece of userspace can trivially cripple the system. Forcing a vmexit on every buslock limits an attacker to one buslock per however long a vmentry/exit cycle takes. > Add an extra perf counter to track the number of Bus Locks detected. extra Xen perf counter. Because other hypervisors use actual perf counters to emulate this ability on current hardware. Maybe something we should consider... > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index d03e78bf0d..02cc7a2023 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -4053,6 +4053,16 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs) > > if ( unlikely(exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY) ) > return vmx_failed_vmentry(exit_reason, regs); > + if ( unlikely(exit_reason & VMX_EXIT_REASONS_BUS_LOCK) ) > + { > + /* > + * Delivery of Bus Lock VM exit was pre-empted by a higher priority > VM > + * exit. > + */ > + exit_reason &= ~VMX_EXIT_REASONS_BUS_LOCK; > + if ( exit_reason != EXIT_REASON_BUS_LOCK ) > + perfc_incr(buslock); I'm pretty sure you can drop the if, and do the perfc_incr() unconditionally. You won't get EXIT_REASON_BUS_LOCK | VMX_EXIT_REASONS_BUS_LOCK given that wording in the ISE. To test, Intel has PENDING_DBG which interferes with most easy attempts to create the condition, but how about this. Load an LDT, misaligned across a cacheline boundary, and set #DB's %cs to LDT[0] with a clear access bit, then execute an `icebp` instruction. The atomic write to set the access bit is a 4-byte access typically. This should cause the #DB intercept to trigger on the same instantaneous boundary that generated the buslock. Otherwise, LGTM. ~Andrew