Re: [PATCH v8 10/14] xen/common: move the memory_mapping DOMCTL hypercall to common code

[Julien Grall <julien.grall@linaro.org>]

Hi Arianna,

On 25/05/14 11:51, Arianna Avanzini wrote:
> +#define mfn_t unsigned long

You should use typedef rather than define a type with a define...

> +#include <xen/p2m-common.h>
> +

That would make more sense to include asm/p2m.h in xen/p2m-common.h 
rather than the invert.

This will avoid issue such as, a source file is trying to include 
xen/p2m-common.h directly without adding asm/p2m.h before. This will 
result to a compilation issue.

Anyway, if the maintainers are fine with this solution, let's stick

>   /*
> diff --git a/xen/include/xen/p2m-common.h b/xen/include/xen/p2m-common.h
> new file mode 100644
> index 0000000..8760b9d
> --- /dev/null
> +++ b/xen/include/xen/p2m-common.h
> @@ -0,0 +1,16 @@
> +#ifndef _XEN_P2M_COMMON_H
> +#define _XEN_P2M_COMMON_H
> +
> +/* Map MMIO regions in the p2m: start_gfn and nr_mfns describe the range
> + * in the guest physical address space to map, starting from the machine
> + * frame number mfn. */
> +int map_mmio_regions(struct domain *d,
> +                     unsigned long start_gfn,
> +                     unsigned long nr_mfns,
> +                     mfn_t mfn);
> +int unmap_mmio_regions(struct domain *d,
> +                       unsigned long start_gfn,
> +                       unsigned long nr_mfns,
> +                       mfn_t mfn);

I don't like the idea to expose mfn_t and _mfn to the common code. All 
the other interfaces (i.e guest_physmap_*) are using unsigned long here. 
Why shouldn't we use the same thing here?

Hence, this is not consistent with the other P2M interface.

Regards,

-- 
Julien Grall