[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v4 2/6] xen: add a p2mt parameter to map_mmio_regions


On 07/08/2019 01:23, Stefano Stabellini wrote:
Add a p2mt parameter to map_mmio_regions, pass p2m_mmio_direct_dev on
ARM and p2m_mmio_direct on x86 -- no changes in behavior.

On x86, introduce a macro to strip away the last parameter and rename
the existing implementation of map_mmio_regions to map_mmio_region.
Use map_mmio_region in vpci as it is x86-only today.

This feels quite wrong. You have a "plural" function calling a "singular" function. This is usually the other way around. This is also quite difficult for a user to understand why the 's' is been dropped/added (depending how you view it) because in both case you only deal with a single region.

The confusion is added because there are no unmap_mmio_region so the code looks strange to read:

> +        rc = map->map ? map_mmio_region(map->d, _gfn(s), size, _mfn(s))
>                         : unmap_mmio_regions(map->d, _gfn(s), size, _mfn(s));

Anyway, I realized that Jan suggested it and it is x86 code. So if he is happy with it so it be.


diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index b48e408583..2674caa005 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -919,6 +919,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
          unsigned long nr_mfns = op->u.memory_mapping.nr_mfns;
          unsigned long mfn_end = mfn + nr_mfns - 1;
          int add = op->u.memory_mapping.add_mapping;
+        p2m_type_t p2mt;
ret = -EINVAL;
          if ( mfn_end < mfn || /* wrap? */
@@ -931,6 +932,10 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
          /* Must break hypercall up as this could take a while. */
          if ( nr_mfns > 64 )
+        p2mt = p2m_mmio_direct_dev;
+        p2mt = p2m_mmio_direct;

I think it is a pretty bad idea to have arch specific code in common code. This is only to make more difficult to add new arch (such as RISCv). Instead we should provide helper in arch code.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.