Re: [PATCH v2 4/6] xen: add capability to load initrd outside of initial mapping

["Vladimir 'φ-coder/phcoder' Serbinenko" <phcoder@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2473 bytes --]

On 11.02.2016 15:13, Juergen Gross wrote:
> On 11/02/16 13:33, Daniel Kiper wrote:
>> On Thu, Feb 11, 2016 at 08:53:24AM +0100, Juergen Gross wrote:
>>> Modern pvops linux kernels support an initrd not covered by the initial
>>> mapping. This capability is flagged by an elf-note.
>>>
>>> In case the elf-note is set by the kernel don't place the initrd into
>>> the initial mapping. This will allow to load larger initrds and/or
>>> support domains with larger memory, as the initial mapping is limited
>>> to 2GB and it is containing the p2m list.
>>>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>> ---
>>>  grub-core/loader/i386/xen.c        | 56
++++++++++++++++++++++++++++++--------
>>>  grub-core/loader/i386/xen_fileXX.c |  3 ++
>>>  include/grub/xen_file.h            |  1 +
>>>  3 files changed, 49 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
>>> index 65cec27..0f41048 100644
>>> --- a/grub-core/loader/i386/xen.c
>>> +++ b/grub-core/loader/i386/xen.c
>>> @@ -307,15 +307,14 @@ grub_xen_pt_alloc (void)
>>>  }
>>>
>>>  static grub_err_t
>>> -grub_xen_boot (void)
>>> +grub_xen_alloc_end (void)
>>>  {
>>>    grub_err_t err;
>>> -  grub_uint64_t nr_pages;
>>> -  struct gnttab_set_version gnttab_setver;
>>> -  grub_size_t i;
>>> +  static int called = 0;
>>>
>>> -  if (grub_xen_n_allocated_shared_pages)
>>> -    return grub_error (GRUB_ERR_BUG, "active grants");
>>> +  if (called)
>>> +    return GRUB_ERR_NONE;
>>
>> Why?
>
> Did you look at the function? grub_xen_alloc_end() (some parts of the
> original grub_xen_boot()) is new and may be called multiple times. It
> was much easier to just call it and let it do what must be done only
> at the first time called.
>
What if a boot fails and then fallback kernel is loaded?
>>> +  if (grub_xen_n_allocated_shared_pages)
>>> +    return grub_error (GRUB_ERR_BUG, "active grants");
>>
>> Why?
>
> That's how it has been before. git just decided to generate the diff
> that way.
>
This is also needed to avoid passing control when some drivers are still
active
>>> +	case 16:
>>
>> Could you define this a constant and use it here?
>
> This would be the only case with a constant. All others are numeric
> as well.
>
I'm ok with not insisisting on using constants given current state but
in general constants are preferable (yes, xen code isn't always clean)





[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]