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

From: "Jan Beulich" <JBeulich@suse.com>
To: Roger Pau Monne <roger.pau@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	WeiLiu <wei.liu2@citrix.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
	xen-devel@lists.xenproject.org, boris.ostrovsky@oracle.com
Subject: Re: [PATCH v3.1 11/15] xen/mm: introduce a function to map large chunks of MMIO
Date: Tue, 29 Nov 2016 06:00:47 -0700	[thread overview]
Message-ID: <583D8A0F0200007800123393@prv-mh.provo.novell.com> (raw)
In-Reply-To: <20161129124122.lqkhsjddpuej3dqp@MacBook-Pro-de-Roger.local>

>>> On 29.11.16 at 13:41, <roger.pau@citrix.com> wrote:
> On Fri, Nov 11, 2016 at 09:58:44AM -0700, Jan Beulich wrote:
>> >>> On 29.10.16 at 10:59, <roger.pau@citrix.com> 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 ...

Jan

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