Re: [PATCH 3/4 RFC] x86/p2m: use large pages for MMIO mappings

[Julien Grall <julien.grall@citrix.com>]

Hi Jan,

On 29/09/15 14:27, Jan Beulich wrote:
>>>> On 29.09.15 at 15:06, <julien.grall@citrix.com> wrote:
>> On 29/09/15 14:00, Jan Beulich wrote:
>>>>>> On 29.09.15 at 14:52, <julien.grall@citrix.com> 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