Re: [PATCH v2 1/5] libxc: remove allocate member from struct xc_dom_image

[Juergen Gross <jgross@suse.com>]

On 10/02/2015 04:47 PM, Ian Campbell wrote:
> On Fri, 2015-10-02 at 16:25 +0200, Juergen Gross wrote:
>> On 10/02/2015 03:01 PM, Ian Campbell wrote:
>>> On Fri, 2015-10-02 at 07:49 +0200, Juergen Gross wrote:
>>>> The allocate() callback in struct xc_dom_image is never set. Remove
>>>> it.
>>>>
>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
>>>
>>> This breaks the stubdom build:
>>>
>>> kexec.c: In function ‘kexec’:
>>> kexec.c:221:78: warning: taking address of expression of type ‘void’
>>>        xen_pfn_t boot_page_mfn = virt_to_mfn(&_boot_page);
>>>
>>>          ^
>>> kexec.c:230:8: error: ‘struct xc_dom_image’ has no member named
>>> ‘allocate’
>>>        dom->allocate = kexec_allocate;
>>>           ^
>>> kexec.c:318:60: warning: taking address of expression of type ‘void’
>>>                virt_to_mfn(&_boot_page));
>>>                                                               ^
>>> Makefile:79: recipe for target '/local/scratch/ianc/devel/committer
>>> -amd64.git/stubdom/grub-x86_64/kexec.o' failed
>>>
>>> On i386 too.
>>>
>>> And in fact that hook looks useful in that context, so either it needs
>>> to
>>> stay of stubdom kexec needs changing to work some other way.
>>
>> Too bad.
>>
>> I wanted to remove the allocate callback as it will conflict with the
>> allocations of memory outside the initial default mapping.
>>
>> Just to make sure I understand this correctly:
>>
>> stubdom is used in this context to support grub running as a pv domain
>> capable to start another pv domain.
>
> Correct.
>
>> So as long as stubdom doesn't support mapping the p2m list outside the
>> default mapping it makes no sense to support this feature for any domain
>> started via stubdom/grub (the main reason to use this feature is the
>> support of huge memory causing the p2m list to exceed the available
>> virtual address space of the default mapping).
>
> By "default mapping" you mean the mapping established by the domain builder
> which made the domain, as distinct from any mapping which the guest kernel
> might establish by itself later, right?

Yes.

> I'm not sure that there is any link between the stubdomain's own p2m and
> the default mapping of that and the p2m which it is building for use of the
> domain it is going to kexec and the default mappings of that p2m-to-be from
> the PoV of the to-be-kexec'd guest kernel.

I just checked it again. Initially I thought stubdom would have the same
limitations as Linux regarding the p2m size. But this is not true, as
stubdom's virt_base is 0, while Linux is using ffffffff80000000.

> I'm not sure how kexec operates in this regard.
>
>> So the easy solution would be to not support initrd and p2m outside the
>> default mapping when the allocate callback is set. Do you think this
>> solution is okay?
>
> Irrespective of the above just not supporting this mode would be one way to
> avoid the issue. It would make domains using pvgrub1 have different
> limitations than domains built directly. IOW users will have to trade off
> the security benefits of pvgrub vs the size of the domain they wish to
> build, which is a shame.

Hmm, maybe it's possible to add the support to stubdom. With the limit
of the p2m size not hitting stubdom it might be rather easy. I'll try
it.

> On the other hand pvgrub2 now exists as a separate (out of tree, it's in
> grub.git) thing anyway which doesn't use this domain builder at all AFAIK
> and that's the one I expect people are using going forward anyway.

Hmm, another place where huge domain support is to be added, I guess.


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel