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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.01.18 at 10:39, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -2324,6 +2324,9 @@ int hvm_set_cr3(unsigned long value, bool_t may_defer)
>          }
>      }
>  
> +    if ( hvm_pcid_enabled(v) ) /* Clear the noflush bit. */
> +        value &= ((1ull << 63) - 1);

This needs a proper X86_CR3_* constant defined.

Did you investigate what it would take to actually _act_ upon the
bit being set (besides clearing it for the value to be stored to be
sane)? Afaict it would require handing a boolean through
paging_update_cr3() -> ... -> {svm,vmx}_update_guest_cr() to
allow suppressing the hvm_asid_flush_vcpu() etc there.

> --- a/xen/arch/x86/hvm/monitor.c
> +++ b/xen/arch/x86/hvm/monitor.c
> @@ -36,6 +36,12 @@ bool hvm_monitor_cr(unsigned int index, unsigned long 
> value, unsigned long old)
>      struct arch_domain *ad = &curr->domain->arch;
>      unsigned int ctrlreg_bitmask = monitor_ctrlreg_bitmask(index);
>  
> +    if ( index == 3 && hvm_pcid_enabled(curr) ) /* Clear the noflush bit. */
> +    {
> +        value &= ((1ull << 63) - 1);
> +        old &= ((1ull << 63) - 1);

Why would "old" need clearing the bit? It being set during write
doesn't actually get stored into the register, aiui.

Jan


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