| Message ID | 1554886979-22890-3-git-send-email-patrick.delaunay@st.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | [U-Boot,1/3] efi: fix issue for 32bit targets with ram_top at 4G | expand |
On 4/10/19 11:02 AM, Patrick Delaunay wrote: > Handle virtual address in efi_mem_carve_out function > when a new region is created to avoid issue in EFI memory map. > > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> > --- > > Issue detected during test of test of UEFI on ev1/dk2 boards. > > For debug purpose, I add the dump of the map in efi_mem_sort: > > static void efi_mem_sort(void) > { > ..... > int i = 0; > printf("-- %s -------------------\n", __func__ ); > list_for_each(lhandle, &efi_mem) { > struct efi_mem_list *lmem; > struct efi_mem_desc *cur; > > lmem = list_entry(lhandle, struct efi_mem_list, link); > cur = &lmem->desc; > > printf("%02d: va %08x pa %08x size %08x (%04x pages) => va end %08x attribute = %llx type=%d\n", > i++, > (u32)cur->virtual_start, > (u32)cur->physical_start, > (u32)(cur->num_pages << EFI_PAGE_SHIFT), > (u32)cur->num_pages, > (u32) (cur->virtual_start + (cur->num_pages << EFI_PAGE_SHIFT)), > cur->attribute, > cur->type); > } > } > > I get the memory layout (for EV1): > > 00: va fcf87000 pa fff92000 size 0006e000 (006e pages) => va end fcff5000 attribute = 8 type=2 > 01: va fff91000 pa fff91000 size 00001000 (0001 pages) => va end fff92000 attribute = 8000000000000008 type=5 > 02: va fcf87000 pa fcf87000 size 0300a000 (300a pages) => va end fff91000 attribute = 8 type=2 > 03: va fcf86000 pa fcf86000 size 00001000 (0001 pages) => va end fcf87000 attribute = 8000000000000008 type=6 > 04: va fcf46000 pa fcf7d000 size 00009000 (0009 pages) => va end fcf4f000 attribute = 8 type=0 > 05: va fcf7c000 pa fcf7c000 size 00001000 (0001 pages) => va end fcf7d000 attribute = 8000000000000008 type=6 > 06: va fcf46000 pa fcf77000 size 00005000 (0005 pages) => va end fcf4b000 attribute = 8 type=0 > 07: va c0000000 pa c0000000 size 3cf77000 (3cf77 pages) => va end fcf77000 attribute = 8 type=7 > > But for some region virtual address (va) != physical address (pa) > > After check of the code, the virtual address is not correctly managed > when new region is created. > > I don't sure that could be a issue, but I solve the issue > with the proposed patch: > > 00: va fff92000 pa fff92000 size 0006e000 (006e pages) => va end 00000000 attribute = 8 type=2 > 01: va fff91000 pa fff91000 size 00001000 (0001 pages) => va end fff92000 attribute = 8000000000000008 type=5 > 02: va fcf87000 pa fcf87000 size 0300a000 (300a pages) => va end fff91000 attribute = 8 type=2 > 03: va fcf86000 pa fcf86000 size 00001000 (0001 pages) => va end fcf87000 attribute = 8000000000000008 type=6 > 04: va fcf7d000 pa fcf7d000 size 00009000 (0009 pages) => va end fcf86000 attribute = 8 type=0 > 05: va fcf7c000 pa fcf7c000 size 00001000 (0001 pages) => va end fcf7d000 attribute = 8000000000000008 type=6 > 06: va fcf77000 pa fcf77000 size 00005000 (0005 pages) => va end fcf7c000 attribute = 8 type=0 > 07: va c0000000 pa c0000000 size 3cf77000 (3cf77 pages) => va end fcf77000 attribute = 8 type=7 > > The virtual address are now correct. > > PS: I don't found more elegant code to update the virtual address > but I avoid to force virtual_address = physical address > in this function. > > lib/efi_loader/efi_memory.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > index 81dc5fc..a47adef 100644 > --- a/lib/efi_loader/efi_memory.c > +++ b/lib/efi_loader/efi_memory.c > @@ -168,6 +168,16 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, > free(map); > } else { > map->desc.physical_start = carve_end; Thanks for reporting the issue. I think you correctly identified the two places where the update of the virtual address is missing. The efi_mem_carve_out() function is called efi_add_memory_map() which is only called at boottime. By definition the physical and virtual addresses must be the same at boottime. So a single line map->desc.virtual_start = carve_end; should be enough here. > + if (carve_end == map_end) > + map->desc.virtual_start = > + map_desc->virtual_start + > + (map_desc->num_pages << EFI_PAGE_SHIFT); > + else > + map->desc.virtual_start = > + carve_desc->virtual_start + > + (carve_desc->num_pages << > + EFI_PAGE_SHIFT); > + > map->desc.num_pages = (map_end - carve_end) > >> EFI_PAGE_SHIFT; > } > @@ -186,6 +196,11 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, > newmap = calloc(1, sizeof(*newmap)); > newmap->desc = map->desc; > newmap->desc.physical_start = carve_start; newmap->desc.virtual_start = carve_start; should be ok here. Best regards Heinrich > + if (carve_start == map_start) > + newmap->desc.virtual_start = map_desc->virtual_start; > + else > + newmap->desc.virtual_start = carve_desc->virtual_start; > + > newmap->desc.num_pages = (map_end - carve_start) >> EFI_PAGE_SHIFT; > /* Insert before current entry (descending address order) */ > list_add_tail(&newmap->link, &map->link); >
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 81dc5fc..a47adef 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -168,6 +168,16 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, free(map); } else { map->desc.physical_start = carve_end; + if (carve_end == map_end) + map->desc.virtual_start = + map_desc->virtual_start + + (map_desc->num_pages << EFI_PAGE_SHIFT); + else + map->desc.virtual_start = + carve_desc->virtual_start + + (carve_desc->num_pages << + EFI_PAGE_SHIFT); + map->desc.num_pages = (map_end - carve_end) >> EFI_PAGE_SHIFT; } @@ -186,6 +196,11 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, newmap = calloc(1, sizeof(*newmap)); newmap->desc = map->desc; newmap->desc.physical_start = carve_start; + if (carve_start == map_start) + newmap->desc.virtual_start = map_desc->virtual_start; + else + newmap->desc.virtual_start = carve_desc->virtual_start; + newmap->desc.num_pages = (map_end - carve_start) >> EFI_PAGE_SHIFT; /* Insert before current entry (descending address order) */ list_add_tail(&newmap->link, &map->link);
Handle virtual address in efi_mem_carve_out function when a new region is created to avoid issue in EFI memory map. Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> --- Issue detected during test of test of UEFI on ev1/dk2 boards. For debug purpose, I add the dump of the map in efi_mem_sort: static void efi_mem_sort(void) { ..... int i = 0; printf("-- %s -------------------\n", __func__ ); list_for_each(lhandle, &efi_mem) { struct efi_mem_list *lmem; struct efi_mem_desc *cur; lmem = list_entry(lhandle, struct efi_mem_list, link); cur = &lmem->desc; printf("%02d: va %08x pa %08x size %08x (%04x pages) => va end %08x attribute = %llx type=%d\n", i++, (u32)cur->virtual_start, (u32)cur->physical_start, (u32)(cur->num_pages << EFI_PAGE_SHIFT), (u32)cur->num_pages, (u32) (cur->virtual_start + (cur->num_pages << EFI_PAGE_SHIFT)), cur->attribute, cur->type); } } I get the memory layout (for EV1): 00: va fcf87000 pa fff92000 size 0006e000 (006e pages) => va end fcff5000 attribute = 8 type=2 01: va fff91000 pa fff91000 size 00001000 (0001 pages) => va end fff92000 attribute = 8000000000000008 type=5 02: va fcf87000 pa fcf87000 size 0300a000 (300a pages) => va end fff91000 attribute = 8 type=2 03: va fcf86000 pa fcf86000 size 00001000 (0001 pages) => va end fcf87000 attribute = 8000000000000008 type=6 04: va fcf46000 pa fcf7d000 size 00009000 (0009 pages) => va end fcf4f000 attribute = 8 type=0 05: va fcf7c000 pa fcf7c000 size 00001000 (0001 pages) => va end fcf7d000 attribute = 8000000000000008 type=6 06: va fcf46000 pa fcf77000 size 00005000 (0005 pages) => va end fcf4b000 attribute = 8 type=0 07: va c0000000 pa c0000000 size 3cf77000 (3cf77 pages) => va end fcf77000 attribute = 8 type=7 But for some region virtual address (va) != physical address (pa) After check of the code, the virtual address is not correctly managed when new region is created. I don't sure that could be a issue, but I solve the issue with the proposed patch: 00: va fff92000 pa fff92000 size 0006e000 (006e pages) => va end 00000000 attribute = 8 type=2 01: va fff91000 pa fff91000 size 00001000 (0001 pages) => va end fff92000 attribute = 8000000000000008 type=5 02: va fcf87000 pa fcf87000 size 0300a000 (300a pages) => va end fff91000 attribute = 8 type=2 03: va fcf86000 pa fcf86000 size 00001000 (0001 pages) => va end fcf87000 attribute = 8000000000000008 type=6 04: va fcf7d000 pa fcf7d000 size 00009000 (0009 pages) => va end fcf86000 attribute = 8 type=0 05: va fcf7c000 pa fcf7c000 size 00001000 (0001 pages) => va end fcf7d000 attribute = 8000000000000008 type=6 06: va fcf77000 pa fcf77000 size 00005000 (0005 pages) => va end fcf7c000 attribute = 8 type=0 07: va c0000000 pa c0000000 size 3cf77000 (3cf77 pages) => va end fcf77000 attribute = 8 type=7 The virtual address are now correct. PS: I don't found more elegant code to update the virtual address but I avoid to force virtual_address = physical address in this function. lib/efi_loader/efi_memory.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)