Re: [Xen-devel] [PATCH 1/3] [RFC] x86/hvm: Permit HVM_PARAM_IDENT_PT to be set more than once

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.05.15 at 14:54, <andrew.cooper3@xxxxxxxxxx> wrote:
> There is no conceptual problem with setting this parameter more than once.
> Checkpointed migration streams will typically set it once per checkpoint to
> the same value.
> 
> The parameter is only actually needed on early-generation VT-x which lacked
> the unrestricted guest capability, although it could plausibly be used on
> newer VT-x with unusual execution control settings.  Short circuit the
> expensive operations on non VT-x hardware.
> 
> The parameter itself must always be latched to avoid issues if the VM
> eventually migrates to a host which needs to use the pagetable.

Reads all plausible, except that I think the way you carry out this
last aspect needs adjustment:

> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -5730,12 +5730,13 @@ static int hvmop_set_param(
>              rc = -EINVAL;
>          break;
>      case HVM_PARAM_IDENT_PT:
> -        rc = -EINVAL;
> -        if ( d->arch.hvm_domain.params[a.index] != 0 )
> -            break;
> -
>          rc = 0;
> -        if ( !paging_mode_hap(d) )
> +        d->arch.hvm_domain.params[a.index] = a.value;
> +        /*
> +         * Only actually required for VT-x lacking unrestricted_guest
> +         * capabilities.  Short circuit the pause if possible.
> +         */
> +        if ( !paging_mode_hap(d) || !cpu_has_vmx )
>              break;

You should latch the new value inside this if()'s body and ...

> @@ -5749,7 +5750,6 @@ static int hvmop_set_param(
>  
>          rc = 0;
>          domain_pause(d);
> -        d->arch.hvm_domain.params[a.index] = a.value;
>          for_each_vcpu ( d, v )
>              paging_update_cr3(v);
>          domain_unpause(d);

... leave this one alone so the use site in VMX code won't see the
new value prematurely.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel