| Message ID | 20180808095433.230882-17-sjg@chromium.org |
|---|---|
| State | Superseded, archived |
| Delegated to: | Alexander Graf |
| Headers | show |
| Series | efi: Enable sandbox support for EFI loader | expand |
On 08/08/2018 11:54 AM, Simon Glass wrote: > Add some more verbose debugging when doing memory allocations. This might > help to find bugs later. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > Changes in v9: None > Changes in v8: None > Changes in v7: None > Changes in v6: None > Changes in v5: None > Changes in v4: None > Changes in v3: None > Changes in v2: None > > lib/efi_loader/efi_boottime.c | 15 +++++++++++++++ > lib/efi_loader/efi_memory.c | 22 +++++++++++++++++++++- > 2 files changed, 36 insertions(+), 1 deletion(-) > > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > index b9e54f551a4..851d282f79d 100644 > --- a/lib/efi_loader/efi_boottime.c > +++ b/lib/efi_loader/efi_boottime.c > @@ -330,6 +330,7 @@ static efi_status_t EFIAPI efi_allocate_pages_ext(int type, int memory_type, > > EFI_ENTRY("%d, %d, 0x%zx, %p", type, memory_type, pages, memory); > r = efi_allocate_pages(type, memory_type, pages, memory); > + > return EFI_EXIT(r); > } > > @@ -379,11 +380,25 @@ static efi_status_t EFIAPI efi_get_memory_map_ext( > uint32_t *descriptor_version) > { > efi_status_t r; > + int i, entries; > > EFI_ENTRY("%p, %p, %p, %p, %p", memory_map_size, memory_map, > map_key, descriptor_size, descriptor_version); > r = efi_get_memory_map(memory_map_size, memory_map, map_key, > descriptor_size, descriptor_version); > + entries = *memory_map_size / sizeof(struct efi_mem_desc); > + debug(" memory_map_size=%zx (%lx entries)\n", *memory_map_size, > + (ulong)(*memory_map_size / sizeof(struct efi_mem_desc))); > + if (memory_map) { > + for (i = 0; i < entries; i++) { > + struct efi_mem_desc *desc = &memory_map[i]; > + > + debug(" type %d, phys %lx, virt %lx, num_pages %lx, attribute %lx\n", > + desc->type, (ulong)desc->physical_start, > + (ulong)desc->virtual_start, > + (ulong)desc->num_pages, (ulong)desc->attribute); > + } > + } > return EFI_EXIT(r); > } > > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > index 967c3f733e4..607152bc4e7 100644 > --- a/lib/efi_loader/efi_memory.c > +++ b/lib/efi_loader/efi_memory.c > @@ -151,6 +151,24 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, > return EFI_CARVE_LOOP_AGAIN; > } > > +static void efi_mem_print(const char *name) > +{ > + struct list_head *lhandle; > + > + debug(" %s: memory map\n", name); > + list_for_each(lhandle, &efi_mem) { > + struct efi_mem_list *lmem = list_entry(lhandle, > + struct efi_mem_list, link); > + struct efi_mem_desc *desc = &lmem->desc; > + > + debug(" type %d, phys %lx, virt %lx, num_pages %lx, attribute %lx\n", For printing 64bit types, please, use PRIx64 and don't convert to ulong. I would prefer to see 0x at the beginning of hexadecimal output. Best regards Heinrich > + desc->type, (ulong)desc->physical_start, > + (ulong)desc->virtual_start, (ulong)desc->num_pages, > + (ulong)desc->attribute); > + } > + debug("\n"); > +} > + > uint64_t efi_add_memory_map(uint64_t start, uint64_t pages, int memory_type, > bool overlap_only_ram) > { > @@ -243,6 +261,7 @@ uint64_t efi_add_memory_map(uint64_t start, uint64_t pages, int memory_type, > > /* And make sure memory is listed in descending order */ > efi_mem_sort(); > + efi_mem_print(__func__); > > return start; > } > @@ -466,7 +485,8 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size, > map_entries++; > > map_size = map_entries * sizeof(struct efi_mem_desc); > - > + debug("%s: map_size %lx, provided_map_size %lx\n", __func__, > + (ulong)map_size, (ulong)provided_map_size); > *memory_map_size = map_size; > > if (provided_map_size < map_size) >
On 08.08.18 11:54, Simon Glass wrote: > Add some more verbose debugging when doing memory allocations. This might > help to find bugs later. > > Signed-off-by: Simon Glass <sjg@chromium.org> I think I asked in some earlier version whether you double checked that this patch does not increase the binary size of a non-debug enabled build. Please note so in the commit message, so that it's clear at first glance. Thanks, Alex > --- > > Changes in v9: None > Changes in v8: None > Changes in v7: None > Changes in v6: None > Changes in v5: None > Changes in v4: None > Changes in v3: None > Changes in v2: None > > lib/efi_loader/efi_boottime.c | 15 +++++++++++++++ > lib/efi_loader/efi_memory.c | 22 +++++++++++++++++++++- > 2 files changed, 36 insertions(+), 1 deletion(-) > > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > index b9e54f551a4..851d282f79d 100644 > --- a/lib/efi_loader/efi_boottime.c > +++ b/lib/efi_loader/efi_boottime.c > @@ -330,6 +330,7 @@ static efi_status_t EFIAPI efi_allocate_pages_ext(int type, int memory_type, > > EFI_ENTRY("%d, %d, 0x%zx, %p", type, memory_type, pages, memory); > r = efi_allocate_pages(type, memory_type, pages, memory); > + > return EFI_EXIT(r); > } > > @@ -379,11 +380,25 @@ static efi_status_t EFIAPI efi_get_memory_map_ext( > uint32_t *descriptor_version) > { > efi_status_t r; > + int i, entries; > > EFI_ENTRY("%p, %p, %p, %p, %p", memory_map_size, memory_map, > map_key, descriptor_size, descriptor_version); > r = efi_get_memory_map(memory_map_size, memory_map, map_key, > descriptor_size, descriptor_version); > + entries = *memory_map_size / sizeof(struct efi_mem_desc); > + debug(" memory_map_size=%zx (%lx entries)\n", *memory_map_size, > + (ulong)(*memory_map_size / sizeof(struct efi_mem_desc))); > + if (memory_map) { > + for (i = 0; i < entries; i++) { > + struct efi_mem_desc *desc = &memory_map[i]; > + > + debug(" type %d, phys %lx, virt %lx, num_pages %lx, attribute %lx\n", > + desc->type, (ulong)desc->physical_start, > + (ulong)desc->virtual_start, > + (ulong)desc->num_pages, (ulong)desc->attribute); > + } > + } > return EFI_EXIT(r); > } > > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > index 967c3f733e4..607152bc4e7 100644 > --- a/lib/efi_loader/efi_memory.c > +++ b/lib/efi_loader/efi_memory.c > @@ -151,6 +151,24 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, > return EFI_CARVE_LOOP_AGAIN; > } > > +static void efi_mem_print(const char *name) > +{ > + struct list_head *lhandle; > + > + debug(" %s: memory map\n", name); > + list_for_each(lhandle, &efi_mem) { > + struct efi_mem_list *lmem = list_entry(lhandle, > + struct efi_mem_list, link); > + struct efi_mem_desc *desc = &lmem->desc; > + > + debug(" type %d, phys %lx, virt %lx, num_pages %lx, attribute %lx\n", > + desc->type, (ulong)desc->physical_start, > + (ulong)desc->virtual_start, (ulong)desc->num_pages, > + (ulong)desc->attribute); > + } > + debug("\n"); > +} > + > uint64_t efi_add_memory_map(uint64_t start, uint64_t pages, int memory_type, > bool overlap_only_ram) > { > @@ -243,6 +261,7 @@ uint64_t efi_add_memory_map(uint64_t start, uint64_t pages, int memory_type, > > /* And make sure memory is listed in descending order */ > efi_mem_sort(); > + efi_mem_print(__func__); > > return start; > } > @@ -466,7 +485,8 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size, > map_entries++; > > map_size = map_entries * sizeof(struct efi_mem_desc); > - > + debug("%s: map_size %lx, provided_map_size %lx\n", __func__, > + (ulong)map_size, (ulong)provided_map_size); > *memory_map_size = map_size; > > if (provided_map_size < map_size) >
Hi Heinich, On 23 August 2018 at 14:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 08/08/2018 11:54 AM, Simon Glass wrote: > > Add some more verbose debugging when doing memory allocations. This might > > help to find bugs later. > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > > > Changes in v9: None > > Changes in v8: None > > Changes in v7: None > > Changes in v6: None > > Changes in v5: None > > Changes in v4: None > > Changes in v3: None > > Changes in v2: None > > > > lib/efi_loader/efi_boottime.c | 15 +++++++++++++++ > > lib/efi_loader/efi_memory.c | 22 +++++++++++++++++++++- > > 2 files changed, 36 insertions(+), 1 deletion(-) > > > > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > > index b9e54f551a4..851d282f79d 100644 > > --- a/lib/efi_loader/efi_boottime.c > > +++ b/lib/efi_loader/efi_boottime.c > > @@ -330,6 +330,7 @@ static efi_status_t EFIAPI efi_allocate_pages_ext(int type, int memory_type, > > > > EFI_ENTRY("%d, %d, 0x%zx, %p", type, memory_type, pages, memory); > > r = efi_allocate_pages(type, memory_type, pages, memory); > > + > > return EFI_EXIT(r); > > } > > > > @@ -379,11 +380,25 @@ static efi_status_t EFIAPI efi_get_memory_map_ext( > > uint32_t *descriptor_version) > > { > > efi_status_t r; > > + int i, entries; > > > > EFI_ENTRY("%p, %p, %p, %p, %p", memory_map_size, memory_map, > > map_key, descriptor_size, descriptor_version); > > r = efi_get_memory_map(memory_map_size, memory_map, map_key, > > descriptor_size, descriptor_version); > > + entries = *memory_map_size / sizeof(struct efi_mem_desc); > > + debug(" memory_map_size=%zx (%lx entries)\n", *memory_map_size, > > + (ulong)(*memory_map_size / sizeof(struct efi_mem_desc))); > > + if (memory_map) { > > + for (i = 0; i < entries; i++) { > > + struct efi_mem_desc *desc = &memory_map[i]; > > + > > + debug(" type %d, phys %lx, virt %lx, num_pages %lx, attribute %lx\n", > > + desc->type, (ulong)desc->physical_start, > > + (ulong)desc->virtual_start, > > + (ulong)desc->num_pages, (ulong)desc->attribute); > > + } > > + } > > return EFI_EXIT(r); > > } > > > > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > > index 967c3f733e4..607152bc4e7 100644 > > --- a/lib/efi_loader/efi_memory.c > > +++ b/lib/efi_loader/efi_memory.c > > @@ -151,6 +151,24 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, > > return EFI_CARVE_LOOP_AGAIN; > > } > > > > +static void efi_mem_print(const char *name) > > +{ > > + struct list_head *lhandle; > > + > > + debug(" %s: memory map\n", name); > > + list_for_each(lhandle, &efi_mem) { > > + struct efi_mem_list *lmem = list_entry(lhandle, > > + struct efi_mem_list, link); > > + struct efi_mem_desc *desc = &lmem->desc; > > + > > + debug(" type %d, phys %lx, virt %lx, num_pages %lx, attribute %lx\n", > > For printing 64bit types, please, use PRIx64 and don't convert to ulong. OK I can do that. Also please check out Masahiro's series which goes in the opposite direction. > I would prefer to see 0x at the beginning of hexadecimal output. Actually the U-Boot convention is to print hex values without 0x. U-Boot almost always uses hex. > > > Best regards > > Heinrich Regards, Simon
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index b9e54f551a4..851d282f79d 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -330,6 +330,7 @@ static efi_status_t EFIAPI efi_allocate_pages_ext(int type, int memory_type, EFI_ENTRY("%d, %d, 0x%zx, %p", type, memory_type, pages, memory); r = efi_allocate_pages(type, memory_type, pages, memory); + return EFI_EXIT(r); } @@ -379,11 +380,25 @@ static efi_status_t EFIAPI efi_get_memory_map_ext( uint32_t *descriptor_version) { efi_status_t r; + int i, entries; EFI_ENTRY("%p, %p, %p, %p, %p", memory_map_size, memory_map, map_key, descriptor_size, descriptor_version); r = efi_get_memory_map(memory_map_size, memory_map, map_key, descriptor_size, descriptor_version); + entries = *memory_map_size / sizeof(struct efi_mem_desc); + debug(" memory_map_size=%zx (%lx entries)\n", *memory_map_size, + (ulong)(*memory_map_size / sizeof(struct efi_mem_desc))); + if (memory_map) { + for (i = 0; i < entries; i++) { + struct efi_mem_desc *desc = &memory_map[i]; + + debug(" type %d, phys %lx, virt %lx, num_pages %lx, attribute %lx\n", + desc->type, (ulong)desc->physical_start, + (ulong)desc->virtual_start, + (ulong)desc->num_pages, (ulong)desc->attribute); + } + } return EFI_EXIT(r); } diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 967c3f733e4..607152bc4e7 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -151,6 +151,24 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, return EFI_CARVE_LOOP_AGAIN; } +static void efi_mem_print(const char *name) +{ + struct list_head *lhandle; + + debug(" %s: memory map\n", name); + list_for_each(lhandle, &efi_mem) { + struct efi_mem_list *lmem = list_entry(lhandle, + struct efi_mem_list, link); + struct efi_mem_desc *desc = &lmem->desc; + + debug(" type %d, phys %lx, virt %lx, num_pages %lx, attribute %lx\n", + desc->type, (ulong)desc->physical_start, + (ulong)desc->virtual_start, (ulong)desc->num_pages, + (ulong)desc->attribute); + } + debug("\n"); +} + uint64_t efi_add_memory_map(uint64_t start, uint64_t pages, int memory_type, bool overlap_only_ram) { @@ -243,6 +261,7 @@ uint64_t efi_add_memory_map(uint64_t start, uint64_t pages, int memory_type, /* And make sure memory is listed in descending order */ efi_mem_sort(); + efi_mem_print(__func__); return start; } @@ -466,7 +485,8 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size, map_entries++; map_size = map_entries * sizeof(struct efi_mem_desc); - + debug("%s: map_size %lx, provided_map_size %lx\n", __func__, + (ulong)map_size, (ulong)provided_map_size); *memory_map_size = map_size; if (provided_map_size < map_size)
Add some more verbose debugging when doing memory allocations. This might help to find bugs later. Signed-off-by: Simon Glass <sjg@chromium.org> --- Changes in v9: None Changes in v8: None Changes in v7: None Changes in v6: None Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: None lib/efi_loader/efi_boottime.c | 15 +++++++++++++++ lib/efi_loader/efi_memory.c | 22 +++++++++++++++++++++- 2 files changed, 36 insertions(+), 1 deletion(-)