Messages in this thread | | | Date | Fri, 28 Jul 2017 16:33:29 +0800 | From | Baoquan He <> | Subject | Re: [PATCH v7] x86/boot/KASLR: Restrict kernel to be randomized in mirror regions |
| |
Hi Ingo,
On 07/28/17 at 08:46am, Ingo Molnar wrote: > > > + pmap = e->efi_memmap; > > +#else > > + pmap = (e->efi_memmap | ((__u64)e->efi_memmap_hi << 32)); > > +#endif > > + > > + nr_desc = e->efi_memmap_size / e->efi_memdesc_size; > > + for (i = 0; i < nr_desc; i++) { > > + md = (efi_memory_desc_t *)(pmap + (i * e->efi_memdesc_size)); > > + if (md->attribute & EFI_MEMORY_MORE_RELIABLE) { > > + region.start = md->phys_addr; > > + region.size = md->num_pages << EFI_PAGE_SHIFT; > > + process_mem_region(®ion, minimum, image_size); > > + efi_mirror_found = true; > > + > > + if (slot_area_index == MAX_SLOT_AREA) { > > + debug_putstr("Aborted EFI scan (slot_areas full)!\n"); > > + break; > > + } > > + } > > + } > > So I suggested this before: if you treat 'md' as an array of elements (which it > is), then the type cast can be moved to a more natural point, where we do address > calculations anyway: > > md = (efi_memory_desc_t *)(e->efi_memmap | ((__u64)e->efi_memmap_hi << 32))); > > The 'pmap' variable can be eliminated and all places in the loop that use 'md->' > can use 'md[i].'.
Thanks for reviewing this patch. Before when I read EFI code and found almost all EFI codes take step of e->efi_memdesc_size long to forward to get each EFI memmap, that's why I use it the same way here. I guess EFI may not guarantee that each EFI memory map entry of 'efi_memory_desc_t' occupy the whole space of e->efi_memdesc_size. In v6 post, you questioned this, and Matt helped to confirm that EFI spec is suggesting that way. Sorry, I trimmed the unrelated code in v6 reply, that could make Matt's reply ignored.
Thanks Baoquan
| |