| Message ID | CAKv+Gu_=et=2zHBTYOr9thz2kS0cXHHYg96oWGRdD3D10fqXtw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Wednesday, November 2, 2016 9:37:13 AM CET Ard Biesheuvel wrote: > @@ -98,7 +100,7 @@ > efi_memory_desc_t *md = (void *)memory_map + map_offset; > unsigned long slots; > > - slots = get_entry_num_slots(md, size, align); > + slots = get_entry_num_slots(md, size, ilog2(align)); > MD_NUM_SLOTS(md) = slots; > total_slots += slots; > } > """ > > This is because ARM does not have a division routine in the > decompressor, and the fact that the division by 'align' should always > involve a power of 2 is not visible to the compiler. > > If nobody objects, I will fold this in when applying > > I'm getting a link error here when building with -Os: drivers/firmware/efi/libstub/random.stub.o: In function `efi_random_alloc': random.c:(.text.efi_random_alloc+0x264): undefined reference to `__aeabi_llsr' If I compile this with -O2, the ilog2 gets inlined and everything works. Arnd _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 15 November 2016 at 15:11, Arnd Bergmann <arnd@arndb.de> wrote: > On Wednesday, November 2, 2016 9:37:13 AM CET Ard Biesheuvel wrote: >> @@ -98,7 +100,7 @@ >> efi_memory_desc_t *md = (void *)memory_map + map_offset; >> unsigned long slots; >> >> - slots = get_entry_num_slots(md, size, align); >> + slots = get_entry_num_slots(md, size, ilog2(align)); >> MD_NUM_SLOTS(md) = slots; >> total_slots += slots; >> } >> """ >> >> This is because ARM does not have a division routine in the >> decompressor, and the fact that the division by 'align' should always >> involve a power of 2 is not visible to the compiler. >> >> If nobody objects, I will fold this in when applying >> >> > > I'm getting a link error here when building with -Os: > > drivers/firmware/efi/libstub/random.stub.o: In function `efi_random_alloc': > random.c:(.text.efi_random_alloc+0x264): undefined reference to `__aeabi_llsr' > > If I compile this with -O2, the ilog2 gets inlined and everything > works. > This is caused by the fact that 'start' and 'end are u64 rather than unsigned long, and the stub does not have the u64 logical shift right routines. But interestingly, it does cover another issue with this code, i.e., that you cannot do allocations over 4 GB in the ARM stub, even on LPAE capable hardware. I will send out a patch to fix this. Thanks, Ard. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--- a/drivers/firmware/efi/libstub/random.c +++ b/drivers/firmware/efi/libstub/random.c @@ -8,6 +8,7 @@ */ #include <linux/efi.h> +#include <linux/log2.h> #include <asm/efi.h> #include "efistub.h" @@ -41,8 +42,9 @@ */ static unsigned long get_entry_num_slots(efi_memory_desc_t *md, unsigned long size, - unsigned long align) + unsigned long align_shift) { + unsigned long align = 1UL << align_shift; u64 start, end; if (md->type != EFI_CONVENTIONAL_MEMORY) @@ -55,7 +57,7 @@ if (start > end) return 0; - return (end - start + 1) / align; + return (end - start + 1) >> align_shift; } /* @@ -98,7 +100,7 @@ efi_memory_desc_t *md = (void *)memory_map + map_offset; unsigned long slots; - slots = get_entry_num_slots(md, size, align); + slots = get_entry_num_slots(md, size, ilog2(align)); MD_NUM_SLOTS(md) = slots; total_slots += slots; }