Re: [Minios-devel] [Xen-devel] [PATCH 1/2] mini-os: support keeping start_info structure conditionally

[Juergen Gross <jgross@xxxxxxxx>]

On 29/08/16 14:32, Andrew Cooper wrote:
> On 29/08/2016 13:28, Juergen Gross wrote:
>> On 29/08/16 13:47, Andrew Cooper wrote:
>>> On 29/08/2016 12:17, Juergen Gross wrote:
>>>> On 29/08/16 13:09, Wei Liu wrote:
>>>>> On Mon, Aug 29, 2016 at 01:01:08PM +0200, Juergen Gross wrote:
>>>>>> grub stubdom needs the start_info structure. Keep a copy of it in
>>>>>> pv mini-os if configured via "CONFIG_KEEP_STARTINFO". This should
>>>>>> default to "n" in order to have it enabled only when really needed.
>>>>>>
>>>>> I'm not sure I understand the rationale for this.
>>>>>
>>>>> Shouldn't start_info always be kept when mini-os is PV? Under what
>>>>> condition should it not be kept?
>>>> The application on top of Mini-OS shouldn't depend on Mini-OS being
>>>> paravirtualized or not in the "normal" case. grub-stubdom is a
>>>> special case, as it needs to kexec to a loaded kernel which in turn
>>>> needs the start_info, of course.
>>>>
>>>> ioemu-stubdom OTOH should not need start_info as it could work on
>>>> a HVMlite Mini-OS, too.
>>>>
>>>> The idea of removing start_info in my HVMlite series was driven by
>>>> that thought: any application using start_info should break as it
>>>> clearly wouldn't be agnostic of the mode (pv or HVMlite) of Mini-OS.
>>>> pv-grub was an oversight here.
>>>>
>>>> I'm planning to modify ioemu-stubdom in the future to not use
>>>> start_info and then let it run in HVMlite mode, too.
>>> There is an issue here between MiniOS itself, and "the stubdom application".
>> Correct.
>>
>>> There should be no circumstance where the stubdom application needs
>>> access (pv-grub being a special case, but maybe the kexec details can be
>>> hidden as well).
>> Indeed. I'll have a look if this is possible. In case I find a clean
>> solution I'll send patches including one removing CONFIG_KEEP_STARTINFO
>> again.
>>
>>> However, while there is still relevant information in start_info, the
>>> low level PV bits should still have access.  Moreover, it is necessary
>>> to always have access when doing suspend/resume.
>> The information from start_info is available inside Mini-OS. So this
>> should be no problem.
> 
> I have never understood MiniOS's decision to memcpy() it elsewhere.  It
> is just a plain page of RAM set up by the domain builder; copying it
> elsewhere is just a waste of space.
> 
> OTOH, you must pass a pointer to it in the suspend() hypercall, so the
> resume logic can re-modify part of it.

Hmm, interesting detail. It took me some time to locate the code where
the start_info parameter of the suspend call is being used.

So the correct way to deal with start_info is to save the pointer to it
and mark this page as being in use.

I think I'll modify my patch to drop the new CONFIG parameter. Later
I'll modify ioemu to no longer rely on start_info and pv-grub if
possible, too. Then I can handle the start_info page the way it should
be done.


Juergen

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/cgi-bin/mailman/listinfo/minios-devel