Re: [Xen-devel] [PATCH] x86/hap: use the right cache attributes when MTRR is disabled

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.07.16 at 18:15, <roger.pau@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/hvm/mtrr.c
> +++ b/xen/arch/x86/hvm/mtrr.c
> @@ -814,10 +814,17 @@ int epte_get_entry_emt(struct domain *d, unsigned long 
> gfn, mfn_t mfn,
>      if ( gmtrr_mtype == -EADDRNOTAVAIL )
>          return -1;
>  
> -    gmtrr_mtype = is_hvm_domain(d) && v ?

Where did the is_hvm_domain() go? Let's not break PVHv1 just yet.

> -                  get_mtrr_type(&v->arch.hvm_vcpu.mtrr,
> -                                gfn << PAGE_SHIFT, order) :
> -                  MTRR_TYPE_WRBACK;
> +    if ( v && v->arch.hvm_vcpu.mtrr.enabled )
> +        /* MTRR is enabled, use MTRR */
> +        gmtrr_mtype = get_mtrr_type(&v->arch.hvm_vcpu.mtrr, gfn << 
> PAGE_SHIFT,
> +                                    order);
> +    else if ( v && !hvm_paging_enabled(v) )
> +        /* MTRR is not enabled and paging is disabled, force UC */
> +        gmtrr_mtype = MTRR_TYPE_UNCACHABLE;
> +    else
> +        /* MTRR is not enabled and paging is enabled, use PAT */
> +        gmtrr_mtype = MTRR_TYPE_WRBACK;

I think this would then better be

    if ( v )
        gmtrr_mtype = MTRR_TYPE_WRBACK;
    else if ( v->arch.hvm_vcpu.mtrr.enabled )
        /* MTRR is enabled, use MTRR */
        gmtrr_mtype = get_mtrr_type(&v->arch.hvm_vcpu.mtrr, gfn << PAGE_SHIFT,
                                    order);
    else if ( !hvm_paging_enabled(v) )
        /* MTRR is not enabled and paging is disabled, force UC */
        gmtrr_mtype = MTRR_TYPE_UNCACHABLE;
    else
        /* MTRR is not enabled and paging is enabled, use PAT */
        gmtrr_mtype = MTRR_TYPE_WRBACK;

albeit even then using vCPU 0 feels wrong when d != current->domain.
Plus v->arch.hvm_vcpu.mtrr.enabled isn't really a boolean, so I think
its use also needs refining.

And finally please fix the comment style.

Jan


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