Re: [Qemu-devel] [PATCH for-4.0 v4 4/4] i386: allow to load initrd below 4G for recent linux

[Stefano Garzarella]

Hi,

On Thu, Dec 27, 2018 at 9:32 PM Eduardo Habkost <address@hidden> wrote:
>
> On Fri, Dec 21, 2018 at 11:10:30AM -0500, Michael S. Tsirkin wrote:
> > On Thu, Dec 06, 2018 at 10:32:13AM +0800, Li Zhijian wrote:
> > > a new field xloadflags was added to recent x86 linux, and BIT 1:
> > > XLF_CAN_BE_LOADED_ABOVE_4G is used to tell bootload that where initrd can 
> > > be
> > > loaded safely.
> > >
> > > Current QEMU/BIOS always loads initrd below below_4g_mem_size which is 
> > > always
> > > less than 4G, so here limiting initrd_max to 4G - 1 simply is enough if
> > > this bit is set.
> > >
> > > CC: Paolo Bonzini <address@hidden>
> > > CC: Richard Henderson <address@hidden>
> > > CC: Eduardo Habkost <address@hidden>
> > > CC: "Michael S. Tsirkin" <address@hidden>
> > > CC: Marcel Apfelbaum <address@hidden>
> > > Signed-off-by: Li Zhijian <address@hidden>
> > >
> > > ---
> > > V3: correct grammar and check XLF_CAN_BE_LOADED_ABOVE_4G first (Michael 
> > > S. Tsirkin)
> > >
> > > Signed-off-by: Li Zhijian <address@hidden>
> > > ---
> > >  hw/i386/pc.c | 10 +++++++++-
> > >  1 file changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > > index 3b10726..baa99c0 100644
> > > --- a/hw/i386/pc.c
> > > +++ b/hw/i386/pc.c
> > > @@ -904,7 +904,15 @@ static void load_linux(PCMachineState *pcms,
> > >  #endif
> > >
> > >      /* highest address for loading the initrd */
> > > -    if (protocol >= 0x203) {
> > > +    if (protocol >= 0x20c &&
> > > +        lduw_p(header+0x236) & XLF_CAN_BE_LOADED_ABOVE_4G) {
> > > +        /*
> > > +         * Although kernel allows initrd loading to above 4G,
> > > +         * it just makes it as large as possible while still staying 
> > > below 4G
> > > +         * since current BIOS always loads initrd below 
> > > pcms->below_4g_mem_size
> > > +         */
> > > +        initrd_max = UINT32_MAX;
> > > +    } else if (protocol >= 0x203) {
> > >          initrd_max = ldl_p(header+0x22c);
> > >      } else {
> > >          initrd_max = 0x37ffffff;
> >
> >
> > I still have trouble understanding the above.
> > Anyone else wants to comment / help rephrase the comment
> > and commit log so it's readable?
>
>
> The comment seems to contradict what I see on the code:
>
> | Although kernel allows initrd loading to above 4G,
>
> Sounds correct.
>
>
> | it just makes it as large as possible while still staying below 4G
>
> I'm not a native English speaker, but I believe "it" here should
> be interpreted as "the kernel", which would be incorrect.  It's
> this QEMU function that limits initrd_max to a uint32 value, not
> the kernel.
>
>
> | since current BIOS always loads initrd below pcms->below_4g_mem_size
>
> I don't know why the BIOS is mentioned here.  The
> below_4g_mem_size limit comes from these 2 lines inside
> load_linux():
>
>     if (initrd_max >= pcms->below_4g_mem_size - pcmc->acpi_data_size) {
>         initrd_max = pcms->below_4g_mem_size - pcmc->acpi_data_size - 1;
>     }
> In addition to that, initrd_max is uint32_t simply because QEMU
> doesn't support the 64-bit boot protocol (specifically the
> ext_ramdisk_image field), so all talk about below_4g_mem_size
> seems to be just a distraction.
>
> All that said, I miss one piece of information here: is
> XLF_CAN_BE_LOADED_ABOVE_4G really supposed to override
> header+0x22c?  linux/Documentation/x86/boot.txt isn't clear about
> that.  Is there any reference that can help us confirm this?

Looking at the following patch seems that we can confirm the assumption:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=4bf7111f50167133a71c23530ca852a41355e739

Note: the patch was reverted due to bugs in some firmwares, but IMHO
the assumption is correct.
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=47226ad4f4cfd1e91ded7f2ec42f83ff1c624663

Cheers,
Stefano

>
> --
> Eduardo
>