Re: [Xen-devel] [PATCH v2 6/8] dm_op: convert HVMOP_set_mem_type

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.12.16 at 14:46, <paul.durrant@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/hvm/dm.c
> +++ b/xen/arch/x86/hvm/dm.c
> @@ -199,6 +199,87 @@ static int modified_memory(struct domain *d, xen_pfn_t 
> *first_pfn,
>      return rc;
>  }
>  
> +static bool allow_p2m_type_change(p2m_type_t old, p2m_type_t new)
> +{
> +    return ( p2m_is_ram(old) ||
> +             (p2m_is_hole(old) && new == p2m_mmio_dm) ||
> +             (old == p2m_ioreq_server && new == p2m_ram_rw) );

Pointless outermost parentheses; at the very least the blanks
immediately inside them need to go away.

> +static int set_mem_type(struct domain *d, hvmmem_type_t mem_type,
> +                        xen_pfn_t *first_pfn, unsigned int *nr)
> +{
> +    xen_pfn_t last_pfn = *first_pfn + *nr - 1;
> +    unsigned int iter;
> +    int rc;
> +
> +    /* Interface types to internal p2m types */
> +    static const p2m_type_t memtype[] = {
> +        [HVMMEM_ram_rw]  = p2m_ram_rw,
> +        [HVMMEM_ram_ro]  = p2m_ram_ro,
> +        [HVMMEM_mmio_dm] = p2m_mmio_dm,
> +        [HVMMEM_unused] = p2m_invalid,
> +        [HVMMEM_ioreq_server] = p2m_ioreq_server
> +    };
> +
> +    if ( (*first_pfn > last_pfn) ||
> +         (last_pfn > domain_get_maximum_gpfn(d)) )
> +        return -EINVAL;
> +
> +    if ( mem_type >= ARRAY_SIZE(memtype) ||
> +         unlikely(mem_type == HVMMEM_unused) )
> +        return -EINVAL;
> +
> +    iter = 0;
> +    rc = 0;
> +    while ( iter < *nr )
> +    {
> +        unsigned long pfn = *first_pfn + iter;
> +        p2m_type_t t;
> +
> +        get_gfn_unshare(d, pfn, &t);
> +        if ( p2m_is_paging(t) )
> +        {
> +            put_gfn(d, pfn);
> +            p2m_mem_paging_populate(d, pfn);
> +            return -EAGAIN;
> +        }
> +
> +        if ( p2m_is_shared(t) )
> +            rc = -EAGAIN;
> +        else if ( !allow_p2m_type_change(t, memtype[mem_type]) )
> +            rc = -EINVAL;
> +        else
> +            rc = p2m_change_type_one(d, pfn, t, memtype[mem_type]);
> +
> +        put_gfn(d, pfn);
> +
> +        if ( rc )
> +            break;
> +
> +        iter++;
> +
> +        /*
> +         * Check for continuation every 256th iteration and if the
> +         * iteration is not the last.
> +         */
> +        if ( (iter < *nr) && ((iter & 0xff) == 0) &&
> +             hypercall_preempt_check() )
> +        {
> +            rc = -ERESTART;
> +            break;
> +        }
> +    }
> +
> +    if ( rc == -ERESTART )
> +    {
> +        *first_pfn += iter;
> +        *nr -= iter;
> +    }

I may have overlooked this in the previous patch - the body of this
if() could easily move into the one setting rc to -ERESTART,
eliminating the extra conditional.

Jan


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