From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH 3/4 RFC] x86/p2m: use large pages for MMIO mappings Date: Wed, 30 Sep 2015 11:15:07 +0100 Message-ID: <560BB62B.4070402@citrix.com> References: <55F70C9A02000078000A2A58@prv-mh.provo.novell.com> <55F7E13902000078000A2B7E@prv-mh.provo.novell.com> <55F7E13902000078000A2B7E@prv-mh.provo.novell.com> <55F7E63702000078000A2BD6@prv-mh.provo.novell.com> <560A7700.6010309@citrix.com> <560A95AA02000078000A69A5@prv-mh.provo.novell.com> <560A8131.7040901@citrix.com> <560AA45302000078000A6A47@prv-mh.provo.novell.com> <560A8993.2050601@citrix.com> <560AA78702000078000A6A90@prv-mh.provo.novell.com> <560A8CCC.5030805@citrix.com> <560AADE302000078000A6AE6@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZhER4-000721-R4 for xen-devel@lists.xenproject.org; Wed, 30 Sep 2015 10:16:30 +0000 In-Reply-To: <560AADE302000078000A6AE6@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Wei Liu , Stefano Stabellini , George Dunlap , Andrew Cooper , IanJackson , Ian Campbell , Tiejun Chen , xen-devel , MalcolmCrossley , Keir Fraser List-Id: xen-devel@lists.xenproject.org Hi Jan, On 29/09/15 14:27, Jan Beulich wrote: >>>> On 29.09.15 at 15:06, wrote: >> On 29/09/15 14:00, Jan Beulich wrote: >>>>>> On 29.09.15 at 14:52, wrote: >>>> On 29/09/15 13:46, Jan Beulich wrote: >>>>> Again, make map_mmio_regions() a wrapper around an ARM-specific >>>>> function with the extra argument. No need to alter common or x86 >>>>> code. >>>> >>>> TBH, extending the mapp_mmio_region is the best solution. >>>> >>>> The name map_mmio_region is very generic and there is no reason we can't >>>> use it in the hypervisor. Adding yet another wrapper will confuse people >>>> and it will be hard for both the reviewer and the developer to know >>>> which one to use. >>> >>> I disagree - the function was originally meant to exclusively support >>> the respective domctl. The fact that ARM gained (many) more uses >>> should not impact common code or x86. >> >> The expectation you described is neither documented nor explicit from >> the name... > > From history only, agreed. > >> As the interface is not set in stone, we could decide to extend the >> usage of the function to make a coherent interface and not adding new >> wrapper because we don't want to touch x86... > > One more thing - what meaning would you expect the new parameter > to carry? Number of frames mapped? Number of iterations done? In > the former case, the goal aimed at here won't be achievable. In the > latter case, would you mean to pass -1UL for it for the ARM callers > wanting the operation to run to completion? None of the both. I was thinking to add a boolean which indicate if the function is preemptible or not. A bit like p2m_pod_set_cache_target. Although, if I had to choose between your two suggestions, I would lean toward the later. Regards, -- Julien Grall