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

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

On 12/16/2013 12:11 PM, Stefano Stabellini wrote:
> On Mon, 16 Dec 2013, Jan Beulich wrote:
>>>>> On 13.12.13 at 21:18, Julien Grall <julien.grall@linaro.org> wrote:
>>> --- a/xen/common/memory.c
>>> +++ b/xen/common/memory.c
>>> @@ -90,7 +90,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;
>>
>> Why?
>>
>>> @@ -122,7 +122,29 @@ static void populate_physmap(struct memop_args *a)
>>>           }
>>>           else
>>>           {
>>> -            page = alloc_domheap_pages(d, a->extent_order, a->memflags);
>>> +            if ( d == dom0 && is_dom0_mapped_11() )
>>> +            {
>>> +                mfn = gpfn;
>>> +                if (!mfn_valid(mfn))
>>
>> Coding style.
>>
>>> +                {
>>> +                    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);
>>> +            }
>>
>> I think this hack doesn't belong into a common file. Rather than
>> having the (anyway oddly named) is_dom0_mapped_11(), it
>> would seem more clean to have this implemented by an inline
>> function for ARM, returning the struct page_info * (and getting
>> #define-d to NULL for all other cases, perhaps not even in an
>> x86 header, but right in the source file).
>
> Considering that having a 1:1 mapping for dom0 is conceivable even on
> x86 (if you want PVH dom0 but your platform doesn't come with an IOMMU),
> I would prefer the approach taken here. However I find your alternative
> suggestion acceptable too.

I will stay on your solution, with is_dom0_mapped_11 modify to 
is_domain_direct_mapped().

-- 
Julien Grall