Re: [MULTIBOOT2 DOC PATCH v2 05/11] multiboot2: Add description of support for EFI boot services

[Daniel Kiper]

On Tue, Nov 29, 2016 at 11:39:39AM +0200, Toomas Soome wrote:
> > On 29. nov 2016, at 11:08, Daniel Kiper <address@hidden> wrote:
> > On Mon, Nov 28, 2016 at 08:53:51PM +0300, Andrei Borzenkov wrote:
> >> 24.11.2016 23:40, Daniel Kiper ?????:
> >>> Signed-off-by: Daniel Kiper <address@hidden>
> >>> ---
> >>> v2 - suggestions/fixes:
> >>>   - clarify physical address meaning for EFI amd64
> >>>     mode with boot services enabled
> >>>     (suggested by Andrew Cooper).
> >>> ---
> >>> doc/multiboot.texi |  115 
> >>> +++++++++++++++++++++++++++++++++++++++++++++++++++-
> >>> doc/multiboot2.h   |    2 +
> >>> 2 files changed, 115 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/doc/multiboot.texi b/doc/multiboot.texi
> >>> index f0f167e..cc1edab 100644
> >>> --- a/doc/multiboot.texi
> >>> +++ b/doc/multiboot.texi
> >>> @@ -534,6 +534,66 @@ The physical address to which the boot loader should 
> >>> jump in order to
> >>> start running the operating system.
> >>> @end table
> >>>
> >>> address@hidden EFI i386 entry address tag of Multiboot2 header
> >>> +
> >>> address@hidden
> >>> address@hidden
> >>> +        +-------------------+
> >>> +u16     | type = 8          |
> >>> +u16     | flags             |
> >>> +u32     | size              |
> >>> +u_virt  | entry_addr        |
> >>> +        +-------------------+
> >>> address@hidden group
> >>> address@hidden example
> >>> +
> >>> +All of the address fields in this tag are physical addresses.
> >>
> >> Should not entry_addr be u_phys then? Otherwise what is exact difference
> >
> > Please look at my earlier email to Toomas.
> >
> >> between u_phys and u_virt? Also for type == 3?
> >
> > Short excerpt from multiboot.texi:
> >
> > @item u_phys
> > The type of unsigned data of the same size as target architecture
> > physical address size.
> >
> > @item u_virt
> > The type of unsigned data of the same size as target architecture
> > virtual address size.
> >
> > Anyway, I agree that entry_addr type should be changed here.
>
> Just to give an example about the scale of the mess here???
>
> We boot BIOS system and get 32bit addresses as grub is running in 32bit 
> protected mode.
> We boot UEFI32, and get 32bit addresses.
> We boot UEFI64, and get 64bit addresses.
>
> Now, what is ???target architecture physical/virtual address size??? in case 
> we start
> 64bit kernel from UEFI32? (or from BIOS for that matter). Also in fact, the 
> grub2 itself
> is *not* using this ???target architecture address size???, but is explicitly 
> using
> multiboot_uint32_t - which from practical point of view seems to work out 
> just fine,
> but does not match at all with documentation.
>
> So, suppose I need to review and verify the implementation??? ;)

Ugh... Then I think that we can safely replace u_phys/u_virt with u32. I have
checked spec and it looks that it requires less changes than I expected. So,
I can add separate patch(es) fixing that. Though I should take a look at MIPS
part in spec too to not break its stuff. Does it make sense?

Daniel