Re: [Xen-devel] [PATCH 20/34] x86/mtrr: move is_var_mtrr_overlapped

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.08.18 at 17:12, <wei.liu2@xxxxxxxxxx> wrote:
> Move it to x86 generic code. While at it, use proper boolean type.
> 
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>

Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
with a couple of cosmetic adjustments beyond the bool
conversion you've already done:

> +bool is_var_mtrr_overlapped(const struct mtrr_state *m)
> +{
> +    unsigned int seg, i;
> +    unsigned int num_var_ranges = MASK_EXTR(m->mtrr_cap, MTRRcap_VCNT);
> +
> +    for ( i = 0; i < num_var_ranges; i++ )
> +    {
> +        uint64_t base1 = m->var_ranges[i].base >> PAGE_SHIFT;
> +        uint64_t mask1 = m->var_ranges[i].mask >> PAGE_SHIFT;

Here and below I wonder whether PFN_DOWN() wouldn't be
appropriate to use. I'll leave that up to you though.

> +        if ( !(m->var_ranges[i].mask & MTRR_PHYSMASK_VALID) )
> +            continue;
> +
> +        for ( seg = i + 1; seg < num_var_ranges; seg ++ )

Stray blank before ++.

> +        {
> +            uint64_t base2 = m->var_ranges[seg].base >> PAGE_SHIFT;
> +            uint64_t mask2 = m->var_ranges[seg].mask >> PAGE_SHIFT;
> +
> +            if ( !(m->var_ranges[seg].mask & MTRR_PHYSMASK_VALID) )
> +                continue;
> +
> +            if ( (base1 & mask1 & mask2) == (base2 & mask2 & mask1) )
> +            {
> +                /* MTRR is overlapped. */

"MTRRs overlap" or some such.

> +                return true;
> +            }
> +        }
> +    }
> +    return false;
> +}

Blank line ahead of main "return" please.

Jan



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