Re: [PATCH] x86/svm: Don't use vmcb->tlb_control as if it is a boolean

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 14/04/2020 14:57, Jan Beulich wrote:
> On 14.04.2020 14:14, Andrew Cooper wrote:
>> @@ -44,19 +41,20 @@ void svm_asid_handle_vmrun(void)
>>      struct hvm_vcpu_asid *p_asid =
>>          nestedhvm_vcpu_in_guestmode(curr)
>>          ? &vcpu_nestedhvm(curr).nv_n2asid : &curr->arch.hvm.n1asid;
>> -    bool_t need_flush = hvm_asid_handle_vmenter(p_asid);
>> +    bool need_flush = hvm_asid_handle_vmenter(p_asid);
>>  
>>      /* ASID 0 indicates that ASIDs are disabled. */
>>      if ( p_asid->asid == 0 )
>>      {
>>          vmcb_set_guest_asid(vmcb, 1);
>> -        vmcb->tlb_control = 1;
>> +        vmcb->tlb_control = TLB_CTRL_FLUSH_ALL;
> While there ought to be no difference in behavior, use of
> TLB_CTRL_FLUSH_ASID would seem more logical to me here. Other
> than below we're no after flushing all ASIDs in this case
> afaict.
>
> Question of course is - did early CPUs treat this as boolean,
> accepting any non-zero value to mean "flush all"?

The spec states "When the VMM sets the TLB_CONTROL field to 1, ...",
which is fairly clear on the matter.

> Preferably with such an adjustment

I'd prefer not to.  There is a good chance that your suggestion will
suffer a vmentry failure, or not flush anything on old hardware.

A change like that should be in its own patch, ideally with finding some
old enough hardware to verify.  I do not know if I have anything that
old to hand.  (Failing real hardware, some conformation from AMD about
how the CPU behaves).

> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks,

~Andrew