Re: [PATCH RFC 25/35] arm: acpi add helper functions to map memory regions

[Julien Grall <julien.grall@linaro.org>]

On 06/02/2015 00:21, Stefano Stabellini wrote:
> On Thu, 5 Feb 2015, Julien Grall wrote:
>> Hi parth,
>>
>> Title: this is not acpi specific.
>>
>> On 04/02/2015 14:02, parth.dixit@linaro.org wrote:
>>> From: Parth Dixit <parth.dixit@linaro.org>
>>>
>>> For passing ACPI tables to dom0, UEFI memory needs to be mapped
>>> by xen in dom0 address space. This patch adds helper functions for mapping.
>>
>> I believe that this is not ACPI/RAM specific. Any cached MMIO regions will
>> have same issue.
>>
>> This because Device memory is too strong and disallow unaligned access.
>>
>>> Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
>>> ---
>>>    xen/arch/arm/p2m.c        | 24 ++++++++++++++++++++++++
>>>    xen/include/asm-arm/p2m.h | 10 ++++++++++
>>>    2 files changed, 34 insertions(+)
>>>
>>> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
>>> index 8809f5a..5593a91 100644
>>> --- a/xen/arch/arm/p2m.c
>>> +++ b/xen/arch/arm/p2m.c
>>> @@ -943,6 +943,30 @@ int p2m_populate_ram(struct domain *d,
>>>                                 0, MATTR_MEM, p2m_ram_rw);
>>>    }
>>>
>>> +int map_ram_regions(struct domain *d,
>>> +                     unsigned long start_gfn,
>>> +                     unsigned long nr,
>>> +                     unsigned long mfn)
>>
>> I don't like the name of the function. It gives the impression that we map any
>> RAM region to the guest via this function.
>>
>> Which is obviously wrong and should never be done.
>
> map_mem_regions?

It's still unclear, what mem mean? is an MMIO region? Is it cached, 
uncached? Is it read-only/read-write.

We already have a function map_mmio_regions. Maybe it would make sense 
to extend it to give more control about the mapping (cached/uncached, 
read-only, read-write,....). But this function is common with x86. So 
I'm not sure about what to do.

Regards,

-- 
Julien Grall