Re: [PATCH] xen/arm: Allow balooning working with 1:1 memory mapping

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

On 11/14/2013 04:18 PM, Julien Grall wrote:
> 
> 
> On 11/14/2013 03:34 PM, Stefano Stabellini wrote:
>> On Thu, 14 Nov 2013, Julien Grall wrote:
>>> With the lake of iommu, dom0 must have a 1:1 memory mapping for all
>>> these guest physical address. When the ballon decides to give back a
>>> page to the kernel, this page must have the same address as previously.
>>> Otherwise, we will loose the 1:1 mapping and will break DMA-capable
>>> device.
>>>
>>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>>> CC: Keir Fraser <keir@xen.org>
>>> CC: Jan Beulich <jbeulich@suse.com>
>>> ---
>>>   xen/common/memory.c | 42 ++++++++++++++++++++++++++++++++++++++++--
>>>   1 file changed, 40 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/xen/common/memory.c b/xen/common/memory.c
>>> index 50b740f..df36d43 100644
>>> --- a/xen/common/memory.c
>>> +++ b/xen/common/memory.c
>>> @@ -28,6 +28,9 @@
>>>   #include <public/memory.h>
>>>   #include <xsm/xsm.h>
>>>   #include <xen/trace.h>
>>> +#ifdef CONFIG_ARM
>>> +#include <asm/platform.h>
>>> +#endif
>>>
>>>   struct memop_args {
>>>       /* INPUT */
>>> @@ -90,7 +93,7 @@ static void increase_reservation(struct memop_args *a)
>>>
>>>   static void populate_physmap(struct memop_args *a)
>>>   {
>>> -    struct page_info *page;
>>> +    struct page_info *page = NULL;
>>>       unsigned long i, j;
>>>       xen_pfn_t gpfn, mfn;
>>>       struct domain *d = a->domain;
>>> @@ -122,7 +125,33 @@ static void populate_physmap(struct memop_args *a)
>>>           }
>>>           else
>>>           {
>>> -            page = alloc_domheap_pages(d, a->extent_order,
>>> a->memflags);
>>> +#ifdef CONFIG_ARM
>>> +            if ( d == dom0 &&
>>> platform_has_quirk(PLATFORM_QUIRK_DOM0_MAPPING_11) )
>>> +            {
>>> +                mfn = gpfn;
>>> +                if (!mfn_valid(mfn))
>>> +                {
>>> +                    gdprintk(XENLOG_INFO, "Invalid mfn
>>> 0x%"PRI_xen_pfn"\n",
>>> +                             mfn);
>>> +                    goto out;
>>> +                }
>>> +
>>> +                page = mfn_to_page(mfn);
>>> +                if ( !get_page(page, d) )
>>> +                {
>>> +                    gdprintk(XENLOG_INFO,
>>> +                             "mfn 0x%"PRI_xen_pfn" doesn't belong to
>>> dom0\n",
>>> +                             mfn);
>>> +                    goto out;
>>> +                }
>>> +                put_page(page);
>>> +            }
>>> +            else
>>> +#endif
>>
>> I don't know if the x86 maintainers would like that, but I would prefer
>> to remove the ifdef CONFIG_ARM.
>> You just need to define a new function called is_dom0_mapped_11, that
>> always returns false on x86 and on ARM is implemented by d == dom0 &&
>> platform_has_quirk(PLATFORM_QUIRK_DOM0_MAPPING_11).
>> Then you can use it on common code.
> 
> With this solution, it's less clear that it's ARM specific code. We are
> introducing a workaround that will never be used on x86.
> 
> I would wait an opinion from other maintainers before sending a new
> version of this patch.
> 

Ping, any input for X86 maintainers (Keir, Jan)?

-- 
Julien Grall