Re: [Xen-devel] [PATCH v3.1 11/15] xen/mm: introduce a function to map large chunks of MMIO

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

On Tue, Nov 29, 2016 at 06:00:47AM -0700, Jan Beulich wrote:
> >>> On 29.11.16 at 13:41, <roger.pau@xxxxxxxxxx> wrote:
> > On Fri, Nov 11, 2016 at 09:58:44AM -0700, Jan Beulich wrote:
> >> >>> On 29.10.16 at 10:59, <roger.pau@xxxxxxxxxx> wrote:
> >> > Current {un}map_mmio_regions implementation has a maximum number of 
> >> > loops 
> > to
> >> > perform before giving up and returning to the caller. This is an issue 
> >> > when
> >> > mapping large MMIO regions when building the hardware domain. In order to
> >> > solve it, introduce a wrapper around {un}map_mmio_regions that takes 
> >> > care 
> > of
> >> > calling process_pending_softirqs between consecutive {un}map_mmio_regions
> >> > calls.
> >> 
> >> So is this something that's going to be needed for other than
> >> hwdom building? Because if not ...
> > 
> > Yes, something similar will also be used by PHYSDEVOP_pci_mmcfg_reserved, 
> > but 
> > that would require hypercall continuations instead of processing pending 
> > softirqs.
> >  
> >> > --- a/xen/common/memory.c
> >> > +++ b/xen/common/memory.c
> >> > @@ -1418,6 +1418,32 @@ int prepare_ring_for_helper(
> >> >      return 0;
> >> >  }
> >> >  
> >> > +int modify_identity_mmio(struct domain *d, unsigned long pfn,
> >> > +                         unsigned long nr_pages, bool map)
> >> 
> >> ... I don't think the function belongs here, and it should be
> >> marked __hwdom_init.
> > 
> > Were would you recommend adding it? Take into account that's also going to 
> > be 
> > used by other code apart from the ACPI Dom0 builder, like the PCI BAR 
> > mapping.
> 
> Hmm, if it's to be used by non-init code, then it staying here would
> make sense if it's also potentially useful to ARM. If it isn't, then
> moving it to e.g. xen/arch/x86/mm.c might be better. If it was init
> only, I would have wanted it to go into one of the more dedicated
> init files ...

Right, for the scope of this series this function is only used by domain build, 
so it makes sense to place it in domain_build.c under __init. If later on I 
need 
it for other purposes I will move it.

Roger.

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