Re: [Xen-devel] [PATCH V4] x86/hvm: fix domain crash when CR3 has the noflush bit set

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.02.18 at 12:01, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> @@ -563,13 +563,19 @@ void svm_update_guest_cr(struct vcpu *v, unsigned int 
> cr)
>      case 3:
>          vmcb_set_cr3(vmcb, v->arch.hvm_vcpu.hw_cr[3]);
>          if ( !nestedhvm_enabled(v->domain) )
> -            hvm_asid_flush_vcpu(v);
> +        {
> +            if ( !(flags & HVM_UPDATE_GUEST_CR3_NO_FLUSH) )
> +                hvm_asid_flush_vcpu(v);
> +        }
>          else if ( nestedhvm_vmswitch_in_progress(v) )
>              ; /* CR3 switches during VMRUN/VMEXIT do not flush the TLB. */
>          else
> -            hvm_asid_flush_vcpu_asid(
> -                nestedhvm_vcpu_in_guestmode(v)
> -                ? &vcpu_nestedhvm(v).nv_n2asid : &v->arch.hvm_vcpu.n1asid);
> +        {
> +            if ( !(flags & HVM_UPDATE_GUEST_CR3_NO_FLUSH) )

Any reason you didn't make this an "else if()", reducing code churn?

> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -80,6 +80,9 @@ enum hvm_intblk {
>  #define HVM_EVENT_VECTOR_UNSET    (-1)
>  #define HVM_EVENT_VECTOR_UPDATING (-2)
>  
> +/* update_guest_cr() flags. */
> +#define HVM_UPDATE_GUEST_CR3_NO_FLUSH 0x00000001

I'd prefer if the naming was consistent with X86_CR3_NOFLUSH
(i.e. have or don't have an underscore between NO and FLUSH in
both cases).

> --- a/xen/include/asm-x86/x86-defns.h
> +++ b/xen/include/asm-x86/x86-defns.h
> @@ -43,6 +43,11 @@
>  #define X86_CR0_PG              0x80000000 /* Paging                   (RW) 
> */
>  
>  /*
> + * Intel CPU flags in CR3
> + */
> +#define X86_CR3_NOFLUSH    0x8000000000000000

Please add the ULL suffix, so the insn emulator could eventually
use this without breaking the 32-bit test harness build.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel