Re: [Xen-devel] [PATCH v2 01/10] xen: add a p2mt parameter to map_mmio_regions

["Jan Beulich" <JBeulich@suse.com>]

>>> On 30.04.19 at 23:02, <sstabellini@kernel.org> wrote:
> --- a/xen/arch/x86/hvm/dom0_build.c
> +++ b/xen/arch/x86/hvm/dom0_build.c
> @@ -79,8 +79,11 @@ static int __init modify_identity_mmio(struct domain *d, unsigned long pfn,
>  
>      for ( ; ; )
>      {
> -        rc = map ?   map_mmio_regions(d, _gfn(pfn), nr_pages, _mfn(pfn))
> -                 : unmap_mmio_regions(d, _gfn(pfn), nr_pages, _mfn(pfn));
> +        if ( map )
> +            rc = map_mmio_regions(d, _gfn(pfn), nr_pages, _mfn(pfn),
> +                                  p2m_mmio_direct);
> +        else
> +            rc = unmap_mmio_regions(d, _gfn(pfn), nr_pages, _mfn(pfn));

May I ask that you leave alone the use of the conditional
operator here, and _just_ add the new argument?

> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -2264,12 +2264,16 @@ static unsigned int mmio_order(const struct domain *d,
>  int map_mmio_regions(struct domain *d,
>                       gfn_t start_gfn,
>                       unsigned long nr,
> -                     mfn_t mfn)
> +                     mfn_t mfn,
> +                     p2m_type_t p2mt)
>  {
>      int ret = 0;
>      unsigned long i;
>      unsigned int iter, order;
>  
> +    if ( p2mt != p2m_mmio_direct )
> +        return -EOPNOTSUPP;

Considering this and ...

> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -927,6 +927,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>          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? */
> @@ -939,6 +940,10 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>          /* Must break hypercall up as this could take a while. */
>          if ( nr_mfns > 64 )
>              break;
> +
> +        p2mt = p2m_mmio_direct_dev;
> +#else
> +        p2mt = p2m_mmio_direct;
>  #endif

... this, is there really value in adding the new parameter for
x86? A wrapper macro of the same name could be used to
strip the new last argument at all call sites (current and future
ones).

Jan



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