linux-kernel - Re: [PATCH 3/3] KVM SVM: Add Bus Lock Detect support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <59381f4f-94de-4933-9dbd-f0fbdc5d5e4a@amd.com>
Date: Wed, 5 Jun 2024 21:44:13 +0530
From: Ravi Bangoria <ravi.bangoria@....com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com, pbonzini@...hat.com, thomas.lendacky@....com,
 hpa@...or.com, rmk+kernel@...linux.org.uk, peterz@...radead.org,
 james.morse@....com, lukas.bulwahn@...il.com, arjan@...ux.intel.com,
 j.granados@...sung.com, sibs@...natelecom.cn, nik.borisov@...e.com,
 michael.roth@....com, nikunj.dadhania@....com, babu.moger@....com,
 x86@...nel.org, kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
 santosh.shukla@....com, ananth.narayan@....com, sandipan.das@....com,
 ravi.bangoria@....com
Subject: Re: [PATCH 3/3] KVM SVM: Add Bus Lock Detect support

On 6/5/2024 8:38 PM, Sean Christopherson wrote:
> On Wed, Jun 05, 2024, Ravi Bangoria wrote:
>> Hi Sean,
>>
>> On 6/4/2024 6:15 AM, Sean Christopherson wrote:
>>> On Mon, Apr 29, 2024, Ravi Bangoria wrote:
>>>> Upcoming AMD uarch will support Bus Lock Detect. Add support for it
>>>> in KVM. Bus Lock Detect is enabled through MSR_IA32_DEBUGCTLMSR and
>>>> MSR_IA32_DEBUGCTLMSR is virtualized only if LBR Virtualization is
>>>> enabled. Add this dependency in the KVM.
>>>
>>> This is woefully incomplete, e.g. db_interception() needs to be updated to decipher
>>> whether the #DB is the responsbility of the host or of the guest.
>>
>> Can you please elaborate. Are you referring to vcpu->guest_debug thingy?
> 
> Yes.  More broadly, all of db_interception().
> 
>>> Honestly, I don't see any point in virtualizing this in KVM.  As Jim alluded to,
>>> what's far, far more interesting for KVM is "Bus Lock Threshold".  Virtualizing
>>> this for the guest would have been nice to have during the initial split-lock #AC
>>> support, but now I'm skeptical the complexity is worth the payoff.
>>
>> This has a valid usecase of penalizing offending processes. I'm not sure
>> how much it's really used in the production though.
> 
> Yeah, but split-lock #AC and #DB have existed on Intel for years, and no one has
> put in the effort to land KVM support, despite the series getting as far as v9[*].

Split-Lock Detect through #AC and Bus Lock Detect through #DB are independent
features. AMD supports only Bus Lock Detect with #DB. I'm not sure about Split
Lock Detect but Intel supports Bus Lock Detect in the guest. These are the
relevant commits:

  https://git.kernel.org/torvalds/c/9a3ecd5e2aa10
  https://git.kernel.org/torvalds/c/e8ea85fb280ec
  https://git.kernel.org/torvalds/c/76ea438b4afcd

> Some of the problems on Intel were due to the awful FMS-based feature detection,
> but those weren't the only hiccups.  E.g. IIRC, we never sorted out what should
> happen if both the host and guest want bus-lock #DBs.

I've to check about vcpu->guest_debug part, but keeping that aside, host and
guest can use Bus Lock Detect in parallel because, DEBUG_CTL MSR and DR6
register are save/restored in VMCB, hardware cause a VMEXIT_EXCEPTION_1 for
guest #DB(when intercepted) and hardware raises #DB on host when it's for the
host.

Please correct me if I misunderstood your comment.

> Anyways, my point is that, except for SEV-ES+ where there's no good reason NOT to
> virtualize Bus Lock Detect, I'm not convinced that it's worth virtualizing bus-lock
> #DBs.
> 
> [*] https://lore.kernel.org/all/20200509110542.8159-1-xiaoyao.li@intel.com
> 
>>> I suppose we could allow it if #DB isn't interecepted, at which point the enabling
>>> required is minimal?
>>
>> The feature uses DEBUG_CTL MSR, #DB and DR6 register. Do you mean expose
>> it when all three are accelerated or just #DB?
> 
> I mean that if KVM isn't intercepting #DB, then there's no extra complexity needed
> to sort out whether the #DB "belongs" to the host or the guest.  See commit
> 90cbf6d914ad ("KVM: SEV-ES: Eliminate #DB intercept when DebugSwap enabled").