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

[Li Zhijian]

On 12/28/2018 4:31 AM, Eduardo Habkost 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:
> > >   
> > >       /* 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),

Thanks for explaining this :), i'm not clear before.



> 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?

Good question.

Since i'm not familiar with boot protocol, at the beginning, i also CCed to 
LKML for helps
https://lkml.org/lkml/2018/11/10/82

Ingo said:
> > > If XLF_CAN_BE_LOADED_ABOVE_4G is not > set, then you most likely 
> > are on a 32-bit kernel and there are more > fundamental limits (even 
> > if you were to load it above the 2 GB mark, you > would be limited by 
> > the size of kernel memory.) > > So, in case you are wondering: the 
> > bootloader that broke when setting > the initrd_max field above 2 GB 
> > was, of course, Grub. > > So just use XLF_CAN_BE_LOADED_ABOVE_4G. 
> > There is no need for a new flag > or new field. That's nice, and 
> > that's the best solution!

that make me to believe that if XLF_CAN_BE_LOADED_ABOVE_4G is set, BELOW_4G is 
allowed too.

if above is credible, might be we can update the comments like:
-------
QEMU doesn't support the 64-bit boot protocol (specifically the
ext_ramdisk_image field).

In addition, kernel allows to load initrd above 4G if 
XLF_CAN_BE_LOADED_ABOVE_4G is set,
so we believe that load initrd below 4G is allowed too.

For simplicity, so just set initrd_max to UINT32_MAX is enough and safe.
-------
 
Thanks
Zhijian