Re: [PATCH v9 11/17] kvm/vmx: Emulate MSR TEST_CTL

From: Xiaoyao Li <xiaoyao.li@linux.intel.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Fenghua Yu <fenghua.yu@intel.com>, Ingo Molnar <mingo@redhat.com>,
	Borislav Petkov <bp@alien8.de>, H Peter Anvin <hpa@zytor.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Dave Hansen <dave.hansen@intel.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Radim Krcmar <rkrcmar@redhat.com>,
	Christopherson Sean J <sean.j.christopherson@intel.com>,
	Ashok Raj <ashok.raj@intel.com>, Tony Luck <tony.luck@intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Xiaoyao Li <xiaoyao.li@intel.com>,
	Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>,
	Ravi V Shankar <ravi.v.shankar@intel.com>,
	linux-kernel <linux-kernel@vger.kernel.org>, x86 <x86@kernel.org>,
	kvm@vger.kernel.org
Subject: Re: [PATCH v9 11/17] kvm/vmx: Emulate MSR TEST_CTL
Date: Thu, 27 Jun 2019 15:58:27 +0800	[thread overview]
Message-ID: <fa53c72c-b1af-7d77-d39c-a9401dc65e27@linux.intel.com> (raw)
In-Reply-To: <alpine.DEB.2.21.1906270901120.32342@nanos.tec.linutronix.de>

On 6/27/2019 3:12 PM, Thomas Gleixner wrote:
> 
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
> A: Top-posting.
> Q: What is the most annoying thing in e-mail?
> 
> A: No.
> Q: Should I include quotations after my reply?
> 
> http://daringfireball.net/2007/07/on_top
> 
> A: Yes
> Q: Should I trim all irrelevant context?
> 

Sorry about this.
Won't do it anymore.

> On Thu, 27 Jun 2019, Xiaoyao Li wrote:
>>
>> Do you have any comments on this one as the policy of how to expose split lock
>> detection (emulate TEST_CTL) for guest changed.
>>
>> This patch makes the implementation as below:
>>
>> Host	|Guest	|Actual value in guest	|split lock happen in guest
>> ------------------------------------------------------------------
>> on	|off	|	on		|report #AC to userspace
>> 	|on	|	on		|inject #AC back to guest
>> ------------------------------------------------------------------
>> off	|off	|	off		|No #AC
>> 	|on	|	on		|inject #AC back to guest
> 
> A: Because it's way better to provide implementation details and useless
>     references to the SDM.
> 
> Q: What's the reason that this table is _NOT_ part of the changelog?
> 

will add it in next version.

>> In case 2, when split lock detection of both host and guest on, if there is a
>> split lock is guest, it will inject #AC back to userspace. Then if #AC is from
>> guest userspace apps, guest kernel sends SIGBUS to userspace apps instead of
>> whole guest killed by host. If #AC is from guest kernel, guest kernel may
>> clear it's split lock bit in test_ctl msr and re-execute the instruction, then
>> it goes into case 1, the #AC will report to host userspace, e.g., QEMU.
> 
> The real interesting question is whether the #AC on split lock prevents the
> actual bus lock or not. If it does then the above is fine.
> 
> If not, then it would be trivial for a malicious guest to set the
> SPLIT_LOCK_ENABLE bit and "handle" the exception pro forma, return to the
> offending instruction and trigger another one. It lowers the rate, but that
> doesn't make it any better.
> 
> The SDM is as usual too vague to be useful. Please clarify.
>

This feature is to ensure no bus lock (due to split lock) in hardware, 
that to say, when bit 29 of TEST_CTL is set, there is no bus lock due to 
split lock can be acquired.

> Thanks,
> 
> 	tglx
> 