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

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

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.

-- 
Julien Grall