Re: [Xen-devel] [PATCH v4 4/7] xen, common: add the XEN_DOMCTL_memory_mapping hypercall

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.03.14 at 03:02, <avanzini.arianna@xxxxxxxxx> wrote:
> +int unmap_mmio_regions(struct domain *d,
> +                       unsigned long start_gfn,
> +                       unsigned long end_gfn,
> +                       unsigned long mfn)
> +{
> +    int ret = 0;
> +    unsigned long nr_mfns = end_gfn - start_gfn + 1;
> +    unsigned long i;
> +
> +    if ( !paging_mode_translate(d) )
> +        return 0;
> +
> +    for ( i = 0; i < nr_mfns; i++ )
> +        ret |= !clear_mmio_p2m_entry(d, start_gfn + i);
> +
> +    return ret;
> +}

Either you keep the function returning a boolean value (but then
please the more conventional 1=success 0=failure), in which case
the function return type should be bool_t, or you make the
function return a proper error code (and propagate that as
necessary in the caller).

Furthermore the mfn parameter is unused here (and effectively
unused also in the ARM variant - it's meaningful only when passing
INSERT to apply_p2m_changes()), so please drop it.

> +    case XEN_DOMCTL_memory_mapping:
> +    {
> +        unsigned long gfn = op->u.memory_mapping.first_gfn;
> +        unsigned long mfn = op->u.memory_mapping.first_mfn;
> +        unsigned long nr_mfns = op->u.memory_mapping.nr_mfns;
> +        unsigned long mfn_end = mfn + nr_mfns;
> +        unsigned long gfn_end = gfn + nr_mfns;

Afaics all uses of [gm]fn_end involve subtracting 1 - please do this
right here if you already introduce these helper variables.

> --- /dev/null
> +++ b/xen/include/xen/p2m.h
> @@ -0,0 +1,16 @@
> +#ifndef _XEN_P2M_COMMON_H
> +#define _XEN_P2M_COMMON_H

You're on the right track with the guard - please name the header
p2m-common.h, so that it won't violate the common inclusion
hierarchy of the common header including the corresponding arch
one. The alternative would be to keep the name, rename the guard,
and replace all relevant (not necessarily all) #include <asm/p2m.h>
with #include <xen/p2m.h>.

Jan


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