Re: [PATCH v7 3/5] KVM: x86: Introduce untag_addr() in kvm_x86_ops

[Chao Gao <chao.gao@intel.com>]

On Fri, Apr 21, 2023 at 03:48:50PM +0800, Binbin Wu wrote:
>
>On 4/19/2023 11:08 AM, Binbin Wu wrote:
>> 
>> On 4/19/2023 10:30 AM, Chao Gao wrote:
>> > On Tue, Apr 04, 2023 at 09:09:21PM +0800, Binbin Wu wrote:
>> > > Introduce a new interface untag_addr() to kvm_x86_ops to untag
>> > > the metadata
>> > >from linear address. Implement LAM version in VMX and dummy version
>> > in SVM.
>> > > When enabled feature like Intel Linear Address Masking or AMD Upper
>> > > Address Ignore, linear address may be tagged with metadata. Linear
>> > > address should be checked for modified canonicality and untagged in
>> > > instrution emulations or vmexit handlings if LAM or UAI is applicable.
>> > > 
>> > > Introduce untag_addr() to kvm_x86_ops to hide the code related to
>> > > vendor
>> > > specific details.
>> > > - For VMX, LAM version is implemented.
>> > >   LAM has a modified canonical check when applicable:
>> > >   * LAM_S48                : [ 1 ][ metadata ][ 1 ]
>> > >                                63               47
>> > >   * LAM_U48                : [ 0 ][ metadata ][ 0 ]
>> > >                                63               47
>> > >   * LAM_S57                : [ 1 ][ metadata ][ 1 ]
>> > >                                63               56
>> > >   * LAM_U57 + 5-lvl paging : [ 0 ][ metadata ][ 0 ]
>> > >                                63               56
>> > >   * LAM_U57 + 4-lvl paging : [ 0 ][ metadata ][ 0...0 ]
>> > >                                63               56..47
>> > >   If LAM is applicable to certain address, untag the metadata bits and
>> > >   replace them with the value of bit 47 (LAM48) or bit 56
>> > > (LAM57). Later
>> > >   the untagged address will do legacy canonical check. So that
>> > > LAM canonical
>> > >   check and mask can be covered by "untag + legacy canonical check".
>> > > 
>> > >   For cases LAM is not applicable, 'flags' is passed to the interface
>> > >   to skip untag.
>> > The "flags" can be dropped. Callers can simply skip the call of
>> > .untag_addr().
>> 
>> OK.
>> 
>> The "flags" was added for proof of future if such kind of untag is also
>> adopted in svm for AMD.
>> 
>> The cases to skip untag are different on the two vendor platforms.
>> 
>> But still, it is able to get the information in __linearize(), I will
>> drop the parameter.
>
>Have a second thought, the flags is still needed to pass to vmx/svm.
>
>If both implementions set the skip untag flag (SKIP_UNTAG_VMX |
>SKIP_UNTAG_SVM)
>or neither sets the skip untag flag,  __linearize() can decide to call
>.untag_addr() or not.
>
>However, in some case, if only one of the implementation need to set the skip
>untag for itself,
>in __linearize(), there is no enough information to tell whether to skip the
>untag or not.

OK. I have no strong preference. An alternative is:

We have only one flag. If AMD and Intel differ in some cases, set the
flag according to the CPU vendor of vCPUs.

if LAM applies to case A while AMD UAI doesn't, do

	if guest CPU is Intel
		set SKIP_UNTAG

for case B to which UAI applies while LAM doens't, do
	
	if guest CPU is AMD
		set SKIP_UNTAG