Re: [Xen-devel] [PATCH 3/5] i386/relocator: Remove unused avoid_efi_bootservices argument

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

Hey,

Sorry for late reply but I was busy with our internal EFI stuff. We did
more tests and some fixes and it seems that everything is going in right
direction. Well, I hope... :-))) Right now I am going to continue my work
on upstream EFI + GRUB2 + Xen stuff. I think that I will be able to release
new patch series for GRUB2 and Xen in second or third week of June (next
week we have 2 days holiday in Poland). Stay tuned...

On Thu, May 07, 2015 at 06:04:21PM +0200, Vladimir 'Ï-coder/phcoder' Serbinenko 
wrote:
> Not unused:
>     grub_efi_mmap_iterate (grub_relocator_alloc_chunk_align_iter, &ctx,
>                          avoid_efi_boot_services);

In general I have a problem with avoid_efi_boot_services stuff in GRUB2.
By default it is disabled and GRUB2 assumes that boot services (BS) memory
regions (code and data) are free. This means that it may put there loaded image
and/or fills memory maps (MULTIBOOT_TAG_TYPE_BASIC_MEMINFO, 
MULTIBOOT_TAG_TYPE_MMAP)
with above mentioned regions marked as free. This is OK when BS are disabled
before loaded image exaction. However, we introduced new feature like
MULTIBOOT_TAG_TYPE_EFI_BS (I am going to improve/extend it) and this makes
this behavior more problematic. It means that now GRUB2 with 
MULTIBOOT_TAG_TYPE_EFI_BS
enabled may overwrite BS regions and/or memory maps are bogus. In turn, this 
means
that BS are potentially unusable by loaded image which asked for BS with above
mentioned tag. So, I think that we should find a solution for that issues.

The simplest fix for that problem seems total removal of avoid_efi_boot_services
and assumption that BS memory regions are not free. I did some tests with that
but not so many and not so deep. I discovered that at least linux loader (IIRC)
has some issues with this solution. Maybe it could be fixed easily but I did
not investigated this issue so long. Additionally, I realized that boot services
regions are quite big (dozens or even about 100 MiB) and maybe this is not very
nice idea to assume them used in all cases.

So, maybe we should just focus on multiboot2 loader only. In that case GRUB2 
should
at first assume that BS regions are used. Then it should check image header. If 
it
does not contain MULTIBOOT_TAG_TYPE_EFI_BS tag then starting from that point it 
should
assume that BS regions are free and behave as it behaves right now. However, if 
multiboot2
loader encounters MULTIBOOT_TAG_TYPE_EFI_BS tag then it should still assume 
that BS
regions are not free. Additionally, GRUB2 should not pass any memory map info 
(i.e,
MULTIBOOT_TAG_TYPE_BASIC_MEMINFO, MULTIBOOT_TAG_TYPE_MMAP, 
MULTIBOOT_TAG_TYPE_EFI_MMAP,
etc.) because they are bogus. In that case loaded image should take memory map 
itself
using relevant BS calls. Does it make sense?

Another questions is why grub_relocator_alloc_chunk_addr() does not consult EFI
memory map if grub_relocator_alloc_chunk_align() does. Should not we fix it?

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel