Re: [Xen-devel] [PATCH 07/10] VMX: handle PML enabling in vmx_vcpu_initialise

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 27/03/15 02:35, Kai Huang wrote:
> It's possible domain has already been in log-dirty mode when creating vcpu, in
> which case we should enable PML for this vcpu if PML has been enabled for the
> domain.
>
> Signed-off-by: Kai Huang <kai.huang@xxxxxxxxxxxxxxx>
> ---
>  xen/arch/x86/hvm/vmx/vmx.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 453bcc5..fce3aa2 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -116,6 +116,30 @@ static int vmx_vcpu_initialise(struct vcpu *v)
>          return rc;
>      }
>  
> +    /*
> +     * It's rare but still possible that domain has already been in log-dirty
> +     * mode when vcpu is being created (commented by Tim), in which case we
> +     * should enable PML for this vcpu if PML has been enabled for the 
> domain,
> +     * and failure to enable results in failure of creating this vcpu.
> +     *
> +     * Note even there's no vcpu created for the domain, 
> vmx_domain_enable_pml
> +     * will return successful in which case vmx_domain_pml_enabled will also
> +     * return true. And even this is the first vcpu to be created with
> +     * vmx_domain_pml_enabled being true, failure of enabling PML still 
> results
> +     * in failure of creating vcpu, to avoid complicated logic to revert PML
> +     * style EPT table to non-PML style EPT table.
> +     */
> +    if ( vmx_domain_pml_enabled(v->domain) )
> +    {
> +        if ( (rc = vmx_vcpu_enable_pml(v)) != 0 )

Given the comment here, is the assertion in the top of
vmx_vcpu_enable_pml() liable to trip?

> +        {
> +            dprintk(XENLOG_ERR, "Failed to enable PML for vcpu %d\n",
> +                    v->vcpu_id);

Please use %pv to identify the domain as well as vcpu.

~Andrew

> +            vmx_destroy_vmcs(v);
> +            return rc;
> +        }
> +    }
> +
>      vpmu_initialise(v);
>  
>      vmx_install_vlapic_mapping(v);


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