Re: [kvm-unit-tests PATCH v6 4/4] x86: Add test case for INVVPID with LAM

[Binbin Wu <binbin.wu@linux.intel.com>]

On 6/18/2024 1:55 PM, Binbin Wu wrote:
>
>
> On 6/6/2024 2:57 AM, Sean Christopherson wrote:
>> On Mon, Jan 22, 2024, Binbin Wu wrote:
>>> +    if (this_cpu_has(X86_FEATURE_LA57) && read_cr4() & X86_CR4_LA57)
>> Checking for feature support seems superfluous, e.g. LA57 should 
>> never be set if
>> it's unsupported.  Then you can do
>>
>>     lam_mask = is_la57_enabled() ? LAM57_MASK : LAM48_MASK;
> OK, will drop the feature check.
>
>>
>>> +        lam_mask = LAM57_MASK;
>>> +
>>> +    vaddr = alloc_vpage();
>>> +    install_page(current_page_table(), virt_to_phys(alloc_page()), 
>>> vaddr);
>>> +    /*
>>> +     * Since the stack memory address in KUT doesn't follow kernel 
>>> address
>>> +     * space partition rule, reuse the memory address for 
>>> descriptor and
>>> +     * the target address in the descriptor of invvpid.
>>> +     */
>>> +    operand = (struct invvpid_operand *)vaddr;
>> Why bother backing the virtual address?  MOV needs a valid 
>> translation, but
>> INVVPID does not (ditto for INVLPG and INVPCID, though it might be 
>> simpler and
>> easier to just use the allocated address for those).
>
> OK, will remove the unnecessary code.

Sorry, the backing is still needed here.
The target address inside the invvpid descriptor (operand->gla) doesn't 
need a valid translation, but the invvpid descriptor itself needs a 
valid translation.

>
>>
>>> +    operand->vpid = 0xffff;
>>> +    operand->gla = (u64)vaddr;
>>> +    operand = (struct invvpid_operand 
>>> *)set_la_non_canonical((u64)operand,
>>> +                                 lam_mask);
>>> +    fault = test_for_exception(GP_VECTOR, ds_invvpid, operand);
>>> +    report(!fault, "INVVPID (LAM on): tagged operand");
>
>