| Message ID | 20180627171720.27028-2-leif.lindholm@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | efi: arm linux loader unification and correctness | expand |
On Wed, Jun 27, 2018 at 06:17:15PM +0100, Leif Lindholm wrote: > There are several implementations of this function in the tree. > Add a central version in grub-core/efi/mm.c. I am happy with the code itself but I am not sure why you are not replacing these "several implementations of this function" in the existing code. I would like to see a patch which does that. Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
On Fri, Jul 06, 2018 at 05:00:45PM +0200, Daniel Kiper wrote: > On Wed, Jun 27, 2018 at 06:17:15PM +0100, Leif Lindholm wrote: > > There are several implementations of this function in the tree. > > Add a central version in grub-core/efi/mm.c. > > I am happy with the code itself but I am not sure why you are > not replacing these "several implementations of this function" > in the existing code. I would like to see a patch which does that. I am happy to submit further cleanup patches once this set gets in, but this is a fundamental change in behaviour compared to those other bits of code (which I do not exercise or use). It is also a fundamental change in behaviour compared to the previous version submitted. I feel a temporary duplication minimises risk of further disruption. Best Regards, Leif _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
On Fri, Jul 06, 2018 at 05:21:17PM +0100, Leif Lindholm wrote: > On Fri, Jul 06, 2018 at 05:00:45PM +0200, Daniel Kiper wrote: > > On Wed, Jun 27, 2018 at 06:17:15PM +0100, Leif Lindholm wrote: > > > There are several implementations of this function in the tree. > > > Add a central version in grub-core/efi/mm.c. > > > > I am happy with the code itself but I am not sure why you are > > not replacing these "several implementations of this function" > > in the existing code. I would like to see a patch which does that. > > I am happy to submit further cleanup patches once this set gets in, > but this is a fundamental change in behaviour compared to those other > bits of code (which I do not exercise or use). It is also a > fundamental change in behaviour compared to the previous version > submitted. I feel a temporary duplication minimises risk of further > disruption. OK, I will take this patch if you promise to fix this in separate patch series later. Later means weeks not years. Of course I am happy to help with this. Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
On Fri, Jul 06, 2018 at 06:55:51PM +0200, Daniel Kiper wrote: > On Fri, Jul 06, 2018 at 05:21:17PM +0100, Leif Lindholm wrote: > > On Fri, Jul 06, 2018 at 05:00:45PM +0200, Daniel Kiper wrote: > > > On Wed, Jun 27, 2018 at 06:17:15PM +0100, Leif Lindholm wrote: > > > > There are several implementations of this function in the tree. > > > > Add a central version in grub-core/efi/mm.c. > > > > > > I am happy with the code itself but I am not sure why you are > > > not replacing these "several implementations of this function" > > > in the existing code. I would like to see a patch which does that. > > > > I am happy to submit further cleanup patches once this set gets in, > > but this is a fundamental change in behaviour compared to those other > > bits of code (which I do not exercise or use). It is also a > > fundamental change in behaviour compared to the previous version > > submitted. I feel a temporary duplication minimises risk of further > > disruption. > > OK, I will take this patch if you promise to fix this in separate patch > series later. Later means weeks not years. Of course I am happy to help > with this. I promise to send an RFC out next week. After that, I'll be basically offline for two weeks. What I cannot promise is validating my patches outside of QEMU/Ovmf, or figuring out special workarounds if it turns out this new mechanism does not work on some (broken) UEFI implementations. If we come across anything like that, then I would suggest we keep the duplication. The RFC will remove all caching of the memory map, which always felt a bit dubious to me. / Leif _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
On Fri, Jul 06, 2018 at 06:13:30PM +0100, Leif Lindholm wrote: > On Fri, Jul 06, 2018 at 06:55:51PM +0200, Daniel Kiper wrote: > > On Fri, Jul 06, 2018 at 05:21:17PM +0100, Leif Lindholm wrote: > > > On Fri, Jul 06, 2018 at 05:00:45PM +0200, Daniel Kiper wrote: > > > > On Wed, Jun 27, 2018 at 06:17:15PM +0100, Leif Lindholm wrote: > > > > > There are several implementations of this function in the tree. > > > > > Add a central version in grub-core/efi/mm.c. > > > > > > > > I am happy with the code itself but I am not sure why you are > > > > not replacing these "several implementations of this function" > > > > in the existing code. I would like to see a patch which does that. > > > > > > I am happy to submit further cleanup patches once this set gets in, > > > but this is a fundamental change in behaviour compared to those other > > > bits of code (which I do not exercise or use). It is also a > > > fundamental change in behaviour compared to the previous version > > > submitted. I feel a temporary duplication minimises risk of further > > > disruption. > > > > OK, I will take this patch if you promise to fix this in separate patch > > series later. Later means weeks not years. Of course I am happy to help > > with this. > > I promise to send an RFC out next week. > After that, I'll be basically offline for two weeks. > > What I cannot promise is validating my patches outside of QEMU/Ovmf, > or figuring out special workarounds if it turns out this new mechanism > does not work on some (broken) UEFI implementations. If we come across > anything like that, then I would suggest we keep the duplication. > > The RFC will remove all caching of the memory map, which always felt a > bit dubious to me. That works for me. Thank you! Have a nice weekend, Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c index c48e9b5c7..fd39d23b4 100644 --- a/grub-core/kern/efi/mm.c +++ b/grub-core/kern/efi/mm.c @@ -286,6 +286,26 @@ grub_efi_finish_boot_services (grub_efi_uintn_t *outbuf_size, void *outbuf, return GRUB_ERR_NONE; } +/* To obtain the UEFI memory map, we must pass a buffer of sufficient size + to hold the entire map. This function returns a sane start value for + buffer size. */ +grub_efi_uintn_t +grub_efi_find_mmap_size (void) +{ + grub_efi_uintn_t mmap_size = 0; + grub_efi_uintn_t desc_size; + + if (grub_efi_get_memory_map (&mmap_size, NULL, NULL, &desc_size, 0) < 0) + { + grub_error (GRUB_ERR_IO, "cannot get EFI memory map size"); + return 0; + } + + /* Add an extra page, since UEFI can alter the memory map itself on + callbacks or explicit calls, including console output. */ + return ALIGN_UP (mmap_size + GRUB_EFI_PAGE_SIZE, GRUB_EFI_PAGE_SIZE); +} + /* Get the memory map as defined in the EFI spec. Return 1 if successful, return 0 if partial, or return -1 if an error occurs. */ int diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h index c996913e5..1021273c1 100644 --- a/include/grub/efi/efi.h +++ b/include/grub/efi/efi.h @@ -49,6 +49,7 @@ void * EXPORT_FUNC(grub_efi_allocate_any_pages) (grub_efi_uintn_t pages); void EXPORT_FUNC(grub_efi_free_pages) (grub_efi_physical_address_t address, grub_efi_uintn_t pages); +grub_efi_uintn_t EXPORT_FUNC(grub_efi_find_mmap_size) (void); int EXPORT_FUNC(grub_efi_get_memory_map) (grub_efi_uintn_t *memory_map_size, grub_efi_memory_descriptor_t *memory_map,
There are several implementations of this function in the tree. Add a central version in grub-core/efi/mm.c. Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> --- grub-core/kern/efi/mm.c | 20 ++++++++++++++++++++ include/grub/efi/efi.h | 1 + 2 files changed, 21 insertions(+) -- 2.11.0 _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel