| Message ID | 20241013105522.391414-4-sughosh.ganu@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Make EFI memory allocations synchronous with LMB | expand |
Hi Sughosh, On Sun, 13 Oct 2024 at 04:55, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > Use the LMB API's for allocating and freeing up memory. With this, the > LMB module becomes the common backend for managing non U-Boot image > memory that might be requested by other modules. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > --- > Changes since V2: > * Use map_to_sysmem() to get the user-visible address to be shared > with the lmb API's for sandbox. > > lib/efi_loader/Kconfig | 1 + > lib/efi_loader/efi_memory.c | 77 +++++++++++-------------------------- > 2 files changed, 24 insertions(+), 54 deletions(-) When efi_init_obj() is called, it should be able to add the lmb memory to its own tables. There is no need to worry about lmb after that, since no other images will be loaded, except under EFI's control. Then perhaps you don't need this patch? Regards, Simon
On Mon, Oct 14, 2024 at 09:50:37AM -0600, Simon Glass wrote: > Hi Sughosh, > > On Sun, 13 Oct 2024 at 04:55, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > Use the LMB API's for allocating and freeing up memory. With this, the > > LMB module becomes the common backend for managing non U-Boot image > > memory that might be requested by other modules. > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > --- > > Changes since V2: > > * Use map_to_sysmem() to get the user-visible address to be shared > > with the lmb API's for sandbox. > > > > lib/efi_loader/Kconfig | 1 + > > lib/efi_loader/efi_memory.c | 77 +++++++++++-------------------------- > > 2 files changed, 24 insertions(+), 54 deletions(-) > > When efi_init_obj() is called, it should be able to add the lmb memory > to its own tables. There is no need to worry about lmb after that, > since no other images will be loaded, except under EFI's control. Then > perhaps you don't need this patch? But that's not true. The EFI application can return us to the U-Boot prompt.
On 14.10.24 17:56, Tom Rini wrote: > On Mon, Oct 14, 2024 at 09:50:37AM -0600, Simon Glass wrote: >> Hi Sughosh, >> >> On Sun, 13 Oct 2024 at 04:55, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: >>> >>> Use the LMB API's for allocating and freeing up memory. With this, the >>> LMB module becomes the common backend for managing non U-Boot image >>> memory that might be requested by other modules. >>> >>> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> >>> --- >>> Changes since V2: >>> * Use map_to_sysmem() to get the user-visible address to be shared >>> with the lmb API's for sandbox. >>> >>> lib/efi_loader/Kconfig | 1 + >>> lib/efi_loader/efi_memory.c | 77 +++++++++++-------------------------- >>> 2 files changed, 24 insertions(+), 54 deletions(-) >> >> When efi_init_obj() is called, it should be able to add the lmb memory >> to its own tables. There is no need to worry about lmb after that, >> since no other images will be loaded, except under EFI's control. Then >> perhaps you don't need this patch? > > But that's not true. The EFI application can return us to the U-Boot > prompt. > The following commands invoke efi_init_obj(): * bootefi * eficonfig * efidebug * env All of these commands may return to the console. bootmeth_efi_mgr calls efi_init_obj() even if not booting anything. Best regards Heinrich
Hi Tom, On Mon, 14 Oct 2024 at 09:56, Tom Rini <trini@konsulko.com> wrote: > > On Mon, Oct 14, 2024 at 09:50:37AM -0600, Simon Glass wrote: > > Hi Sughosh, > > > > On Sun, 13 Oct 2024 at 04:55, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > Use the LMB API's for allocating and freeing up memory. With this, the > > > LMB module becomes the common backend for managing non U-Boot image > > > memory that might be requested by other modules. > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > --- > > > Changes since V2: > > > * Use map_to_sysmem() to get the user-visible address to be shared > > > with the lmb API's for sandbox. > > > > > > lib/efi_loader/Kconfig | 1 + > > > lib/efi_loader/efi_memory.c | 77 +++++++++++-------------------------- > > > 2 files changed, 24 insertions(+), 54 deletions(-) > > > > When efi_init_obj() is called, it should be able to add the lmb memory > > to its own tables. There is no need to worry about lmb after that, > > since no other images will be loaded, except under EFI's control. Then > > perhaps you don't need this patch? > > But that's not true. The EFI application can return us to the U-Boot > prompt. Can you finish that thought? What are you getting at? Regards, Simon
On Mon, Oct 14, 2024 at 01:13:17PM -0600, Simon Glass wrote: > Hi Tom, > > On Mon, 14 Oct 2024 at 09:56, Tom Rini <trini@konsulko.com> wrote: > > > > On Mon, Oct 14, 2024 at 09:50:37AM -0600, Simon Glass wrote: > > > Hi Sughosh, > > > > > > On Sun, 13 Oct 2024 at 04:55, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > > > Use the LMB API's for allocating and freeing up memory. With this, the > > > > LMB module becomes the common backend for managing non U-Boot image > > > > memory that might be requested by other modules. > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > > --- > > > > Changes since V2: > > > > * Use map_to_sysmem() to get the user-visible address to be shared > > > > with the lmb API's for sandbox. > > > > > > > > lib/efi_loader/Kconfig | 1 + > > > > lib/efi_loader/efi_memory.c | 77 +++++++++++-------------------------- > > > > 2 files changed, 24 insertions(+), 54 deletions(-) > > > > > > When efi_init_obj() is called, it should be able to add the lmb memory > > > to its own tables. There is no need to worry about lmb after that, > > > since no other images will be loaded, except under EFI's control. Then > > > perhaps you don't need this patch? > > > > But that's not true. The EFI application can return us to the U-Boot > > prompt. > > Can you finish that thought? What are you getting at? Your assumption is false. As Heinrich followed up with, there are plenty of common cases, including *env* where efi_init_obj() is called and then the user can still do whatever they like via bootm. There is a need to worry about it. Trying to make an artificial distinction here complicates matters. Much more so than adjusting our almost non-existent documentation about memory map / layout to say that this EFI related data *is* U-Boot.
Hi Tom, On Mon, 14 Oct 2024 at 15:04, Tom Rini <trini@konsulko.com> wrote: > > On Mon, Oct 14, 2024 at 01:13:17PM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Mon, 14 Oct 2024 at 09:56, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Mon, Oct 14, 2024 at 09:50:37AM -0600, Simon Glass wrote: > > > > Hi Sughosh, > > > > > > > > On Sun, 13 Oct 2024 at 04:55, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > > > > > Use the LMB API's for allocating and freeing up memory. With this, the > > > > > LMB module becomes the common backend for managing non U-Boot image > > > > > memory that might be requested by other modules. > > > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > > > --- > > > > > Changes since V2: > > > > > * Use map_to_sysmem() to get the user-visible address to be shared > > > > > with the lmb API's for sandbox. > > > > > > > > > > lib/efi_loader/Kconfig | 1 + > > > > > lib/efi_loader/efi_memory.c | 77 +++++++++++-------------------------- > > > > > 2 files changed, 24 insertions(+), 54 deletions(-) > > > > > > > > When efi_init_obj() is called, it should be able to add the lmb memory > > > > to its own tables. There is no need to worry about lmb after that, > > > > since no other images will be loaded, except under EFI's control. Then > > > > perhaps you don't need this patch? > > > > > > But that's not true. The EFI application can return us to the U-Boot > > > prompt. > > > > Can you finish that thought? What are you getting at? > > Your assumption is false. As Heinrich followed up with, there are plenty > of common cases, including *env* where efi_init_obj() is called and then > the user can still do whatever they like via bootm. There is a need to > worry about it. Trying to make an artificial distinction here > complicates matters. Much more so than adjusting our almost non-existent > documentation about memory map / layout to say that this EFI related > data *is* U-Boot. Which assumption? That it can add the lmb memory to its tables? But efi_init_obj() is called every time the EFI subsystem is used. That immediately puts us into 'EFI' mode. What is *env*? Once efi_init_obj() is called, we know we are booting an EFI app, so it is OK to go outside the U-Boot region. It has to be, since U-Boot can be told to allocate anything, under the control of the EFI app. But when U-Boot is just happily running and not using the EFI loader, we should not be doing this. That is my primary concern. Regards, Simon
On Mon, Oct 14, 2024 at 03:13:01PM -0600, Simon Glass wrote: > Hi Tom, > > On Mon, 14 Oct 2024 at 15:04, Tom Rini <trini@konsulko.com> wrote: > > > > On Mon, Oct 14, 2024 at 01:13:17PM -0600, Simon Glass wrote: > > > Hi Tom, > > > > > > On Mon, 14 Oct 2024 at 09:56, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Mon, Oct 14, 2024 at 09:50:37AM -0600, Simon Glass wrote: > > > > > Hi Sughosh, > > > > > > > > > > On Sun, 13 Oct 2024 at 04:55, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > > > > > > > Use the LMB API's for allocating and freeing up memory. With this, the > > > > > > LMB module becomes the common backend for managing non U-Boot image > > > > > > memory that might be requested by other modules. > > > > > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > > > > --- > > > > > > Changes since V2: > > > > > > * Use map_to_sysmem() to get the user-visible address to be shared > > > > > > with the lmb API's for sandbox. > > > > > > > > > > > > lib/efi_loader/Kconfig | 1 + > > > > > > lib/efi_loader/efi_memory.c | 77 +++++++++++-------------------------- > > > > > > 2 files changed, 24 insertions(+), 54 deletions(-) > > > > > > > > > > When efi_init_obj() is called, it should be able to add the lmb memory > > > > > to its own tables. There is no need to worry about lmb after that, Saying "There is no need to worry about lmb after that" is not true. Invoking the "env" command for example will have efi_init_obj() be called, among the others that Heinrich listed. And to possibly refute a next issue, that is intentional so that efivars, the standard mechanism used by an OS to talk with the firmware can be available to U-Boot, if I recall things correctly. My understanding of your assumption is that you believe that once the EFI_LOADER subsystem has started work on a payload we're just a few call chains away from the OS being started and runtime services aside U-Boot being done. My understanding of how things are used today is that this is incorrect.
Hi Tom, On Mon, 14 Oct 2024 at 15:27, Tom Rini <trini@konsulko.com> wrote: > > On Mon, Oct 14, 2024 at 03:13:01PM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Mon, 14 Oct 2024 at 15:04, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Mon, Oct 14, 2024 at 01:13:17PM -0600, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Mon, 14 Oct 2024 at 09:56, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > On Mon, Oct 14, 2024 at 09:50:37AM -0600, Simon Glass wrote: > > > > > > Hi Sughosh, > > > > > > > > > > > > On Sun, 13 Oct 2024 at 04:55, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > > > > > > > > > Use the LMB API's for allocating and freeing up memory. With this, the > > > > > > > LMB module becomes the common backend for managing non U-Boot image > > > > > > > memory that might be requested by other modules. > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > > > > > --- > > > > > > > Changes since V2: > > > > > > > * Use map_to_sysmem() to get the user-visible address to be shared > > > > > > > with the lmb API's for sandbox. > > > > > > > > > > > > > > lib/efi_loader/Kconfig | 1 + > > > > > > > lib/efi_loader/efi_memory.c | 77 +++++++++++-------------------------- > > > > > > > 2 files changed, 24 insertions(+), 54 deletions(-) > > > > > > > > > > > > When efi_init_obj() is called, it should be able to add the lmb memory > > > > > > to its own tables. There is no need to worry about lmb after that, > > Saying "There is no need to worry about lmb after that" is not true. > Invoking the "env" command for example will have efi_init_obj() be > called, among the others that Heinrich listed. And to possibly refute > a next issue, that is intentional so that efivars, the standard > mechanism used by an OS to talk with the firmware can be available to > U-Boot, if I recall things correctly. > > My understanding of your assumption is that you believe that once the > EFI_LOADER subsystem has started work on a payload we're just a few call > chains away from the OS being started and runtime services aside U-Boot > being done. > > My understanding of how things are used today is that this is incorrect. What I am getting at is that once we have called that function we know we are booting an EFI app or using an EFI feature in preparation for doing so. Let's start there. Is that correct? Regards, Simon
On Mon, Oct 14, 2024 at 03:35:12PM -0600, Simon Glass wrote: > Hi Tom, > > On Mon, 14 Oct 2024 at 15:27, Tom Rini <trini@konsulko.com> wrote: > > > > On Mon, Oct 14, 2024 at 03:13:01PM -0600, Simon Glass wrote: > > > Hi Tom, > > > > > > On Mon, 14 Oct 2024 at 15:04, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Mon, Oct 14, 2024 at 01:13:17PM -0600, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Mon, 14 Oct 2024 at 09:56, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > On Mon, Oct 14, 2024 at 09:50:37AM -0600, Simon Glass wrote: > > > > > > > Hi Sughosh, > > > > > > > > > > > > > > On Sun, 13 Oct 2024 at 04:55, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > > > > > > > > > > > Use the LMB API's for allocating and freeing up memory. With this, the > > > > > > > > LMB module becomes the common backend for managing non U-Boot image > > > > > > > > memory that might be requested by other modules. > > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > > > > > > --- > > > > > > > > Changes since V2: > > > > > > > > * Use map_to_sysmem() to get the user-visible address to be shared > > > > > > > > with the lmb API's for sandbox. > > > > > > > > > > > > > > > > lib/efi_loader/Kconfig | 1 + > > > > > > > > lib/efi_loader/efi_memory.c | 77 +++++++++++-------------------------- > > > > > > > > 2 files changed, 24 insertions(+), 54 deletions(-) > > > > > > > > > > > > > > When efi_init_obj() is called, it should be able to add the lmb memory > > > > > > > to its own tables. There is no need to worry about lmb after that, > > > > Saying "There is no need to worry about lmb after that" is not true. > > Invoking the "env" command for example will have efi_init_obj() be > > called, among the others that Heinrich listed. And to possibly refute > > a next issue, that is intentional so that efivars, the standard > > mechanism used by an OS to talk with the firmware can be available to > > U-Boot, if I recall things correctly. > > > > My understanding of your assumption is that you believe that once the > > EFI_LOADER subsystem has started work on a payload we're just a few call > > chains away from the OS being started and runtime services aside U-Boot > > being done. > > > > My understanding of how things are used today is that this is incorrect. > > What I am getting at is that once we have called that function we know > we are booting an EFI app or using an EFI feature in preparation for > doing so. Let's start there. Is that correct? No, it is not correct.
Hi Tom, On Mon, 14 Oct 2024 at 15:48, Tom Rini <trini@konsulko.com> wrote: > > On Mon, Oct 14, 2024 at 03:35:12PM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Mon, 14 Oct 2024 at 15:27, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Mon, Oct 14, 2024 at 03:13:01PM -0600, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Mon, 14 Oct 2024 at 15:04, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > On Mon, Oct 14, 2024 at 01:13:17PM -0600, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Mon, 14 Oct 2024 at 09:56, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > On Mon, Oct 14, 2024 at 09:50:37AM -0600, Simon Glass wrote: > > > > > > > > Hi Sughosh, > > > > > > > > > > > > > > > > On Sun, 13 Oct 2024 at 04:55, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > > > > > > > > > > > > > Use the LMB API's for allocating and freeing up memory. With this, the > > > > > > > > > LMB module becomes the common backend for managing non U-Boot image > > > > > > > > > memory that might be requested by other modules. > > > > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > > > > > > > --- > > > > > > > > > Changes since V2: > > > > > > > > > * Use map_to_sysmem() to get the user-visible address to be shared > > > > > > > > > with the lmb API's for sandbox. > > > > > > > > > > > > > > > > > > lib/efi_loader/Kconfig | 1 + > > > > > > > > > lib/efi_loader/efi_memory.c | 77 +++++++++++-------------------------- > > > > > > > > > 2 files changed, 24 insertions(+), 54 deletions(-) > > > > > > > > > > > > > > > > When efi_init_obj() is called, it should be able to add the lmb memory > > > > > > > > to its own tables. There is no need to worry about lmb after that, > > > > > > Saying "There is no need to worry about lmb after that" is not true. > > > Invoking the "env" command for example will have efi_init_obj() be > > > called, among the others that Heinrich listed. And to possibly refute > > > a next issue, that is intentional so that efivars, the standard > > > mechanism used by an OS to talk with the firmware can be available to > > > U-Boot, if I recall things correctly. > > > > > > My understanding of your assumption is that you believe that once the > > > EFI_LOADER subsystem has started work on a payload we're just a few call > > > chains away from the OS being started and runtime services aside U-Boot > > > being done. > > > > > > My understanding of how things are used today is that this is incorrect. > > > > What I am getting at is that once we have called that function we know > > we are booting an EFI app or using an EFI feature in preparation for > > doing so. Let's start there. Is that correct? > > No, it is not correct. Can you give me a code link? Regards, Simon
On Mon, Oct 14, 2024 at 03:55:05PM -0600, Simon Glass wrote: > Hi Tom, > > On Mon, 14 Oct 2024 at 15:48, Tom Rini <trini@konsulko.com> wrote: > > > > On Mon, Oct 14, 2024 at 03:35:12PM -0600, Simon Glass wrote: > > > Hi Tom, > > > > > > On Mon, 14 Oct 2024 at 15:27, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Mon, Oct 14, 2024 at 03:13:01PM -0600, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Mon, 14 Oct 2024 at 15:04, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > On Mon, Oct 14, 2024 at 01:13:17PM -0600, Simon Glass wrote: > > > > > > > Hi Tom, > > > > > > > > > > > > > > On Mon, 14 Oct 2024 at 09:56, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > On Mon, Oct 14, 2024 at 09:50:37AM -0600, Simon Glass wrote: > > > > > > > > > Hi Sughosh, > > > > > > > > > > > > > > > > > > On Sun, 13 Oct 2024 at 04:55, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > > > > > > > > > > > > > > > Use the LMB API's for allocating and freeing up memory. With this, the > > > > > > > > > > LMB module becomes the common backend for managing non U-Boot image > > > > > > > > > > memory that might be requested by other modules. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > > > > > > > > --- > > > > > > > > > > Changes since V2: > > > > > > > > > > * Use map_to_sysmem() to get the user-visible address to be shared > > > > > > > > > > with the lmb API's for sandbox. > > > > > > > > > > > > > > > > > > > > lib/efi_loader/Kconfig | 1 + > > > > > > > > > > lib/efi_loader/efi_memory.c | 77 +++++++++++-------------------------- > > > > > > > > > > 2 files changed, 24 insertions(+), 54 deletions(-) > > > > > > > > > > > > > > > > > > When efi_init_obj() is called, it should be able to add the lmb memory > > > > > > > > > to its own tables. There is no need to worry about lmb after that, > > > > > > > > Saying "There is no need to worry about lmb after that" is not true. > > > > Invoking the "env" command for example will have efi_init_obj() be > > > > called, among the others that Heinrich listed. And to possibly refute > > > > a next issue, that is intentional so that efivars, the standard > > > > mechanism used by an OS to talk with the firmware can be available to > > > > U-Boot, if I recall things correctly. > > > > > > > > My understanding of your assumption is that you believe that once the > > > > EFI_LOADER subsystem has started work on a payload we're just a few call > > > > chains away from the OS being started and runtime services aside U-Boot > > > > being done. > > > > > > > > My understanding of how things are used today is that this is incorrect. > > > > > > What I am getting at is that once we have called that function we know > > > we are booting an EFI app or using an EFI feature in preparation for > > > doing so. Let's start there. Is that correct? > > > > No, it is not correct. > > Can you give me a code link? env_set() -> env_do_env_set() -> do_env_set_efi() -> efi_init_obj_list()
Hi Tom, On Mon, 14 Oct 2024 at 17:07, Tom Rini <trini@konsulko.com> wrote: > > On Mon, Oct 14, 2024 at 03:55:05PM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Mon, 14 Oct 2024 at 15:48, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Mon, Oct 14, 2024 at 03:35:12PM -0600, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Mon, 14 Oct 2024 at 15:27, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > On Mon, Oct 14, 2024 at 03:13:01PM -0600, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Mon, 14 Oct 2024 at 15:04, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > On Mon, Oct 14, 2024 at 01:13:17PM -0600, Simon Glass wrote: > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > On Mon, 14 Oct 2024 at 09:56, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > > > On Mon, Oct 14, 2024 at 09:50:37AM -0600, Simon Glass wrote: > > > > > > > > > > Hi Sughosh, > > > > > > > > > > > > > > > > > > > > On Sun, 13 Oct 2024 at 04:55, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > > > > > > > > > > > > > > > > > Use the LMB API's for allocating and freeing up memory. With this, the > > > > > > > > > > > LMB module becomes the common backend for managing non U-Boot image > > > > > > > > > > > memory that might be requested by other modules. > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > > > > > > > > > --- > > > > > > > > > > > Changes since V2: > > > > > > > > > > > * Use map_to_sysmem() to get the user-visible address to be shared > > > > > > > > > > > with the lmb API's for sandbox. > > > > > > > > > > > > > > > > > > > > > > lib/efi_loader/Kconfig | 1 + > > > > > > > > > > > lib/efi_loader/efi_memory.c | 77 +++++++++++-------------------------- > > > > > > > > > > > 2 files changed, 24 insertions(+), 54 deletions(-) > > > > > > > > > > > > > > > > > > > > When efi_init_obj() is called, it should be able to add the lmb memory > > > > > > > > > > to its own tables. There is no need to worry about lmb after that, > > > > > > > > > > Saying "There is no need to worry about lmb after that" is not true. > > > > > Invoking the "env" command for example will have efi_init_obj() be > > > > > called, among the others that Heinrich listed. And to possibly refute > > > > > a next issue, that is intentional so that efivars, the standard > > > > > mechanism used by an OS to talk with the firmware can be available to > > > > > U-Boot, if I recall things correctly. > > > > > > > > > > My understanding of your assumption is that you believe that once the > > > > > EFI_LOADER subsystem has started work on a payload we're just a few call > > > > > chains away from the OS being started and runtime services aside U-Boot > > > > > being done. > > > > > > > > > > My understanding of how things are used today is that this is incorrect. > > > > > > > > What I am getting at is that once we have called that function we know > > > > we are booting an EFI app or using an EFI feature in preparation for > > > > doing so. Let's start there. Is that correct? > > > > > > No, it is not correct. > > > > Can you give me a code link? > > env_set() -> env_do_env_set() -> do_env_set_efi() -> efi_init_obj_list() That's with the '-e' option, though, so people doing that are specifically choosing EFI. What I am saying is that, if we are not booting an EFI app or using an EFI feature. Since 'env set -e' is an EFI feature, efi_init_obj_list() is called, and EFI is in the picture. Even if efi_init_obj_list(), I don't like treating U-Boot's memory as a stack to just grow into. But as a first step, I do want to ensure that non-EFI boot can be more like U-Boot. Regards, Simon
On Tue, Oct 15, 2024 at 07:10:27AM -0600, Simon Glass wrote: > Hi Tom, > > On Mon, 14 Oct 2024 at 17:07, Tom Rini <trini@konsulko.com> wrote: > > > > On Mon, Oct 14, 2024 at 03:55:05PM -0600, Simon Glass wrote: > > > Hi Tom, > > > > > > On Mon, 14 Oct 2024 at 15:48, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Mon, Oct 14, 2024 at 03:35:12PM -0600, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Mon, 14 Oct 2024 at 15:27, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > On Mon, Oct 14, 2024 at 03:13:01PM -0600, Simon Glass wrote: > > > > > > > Hi Tom, > > > > > > > > > > > > > > On Mon, 14 Oct 2024 at 15:04, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > On Mon, Oct 14, 2024 at 01:13:17PM -0600, Simon Glass wrote: > > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > > > On Mon, 14 Oct 2024 at 09:56, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > > > > > On Mon, Oct 14, 2024 at 09:50:37AM -0600, Simon Glass wrote: > > > > > > > > > > > Hi Sughosh, > > > > > > > > > > > > > > > > > > > > > > On Sun, 13 Oct 2024 at 04:55, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > > Use the LMB API's for allocating and freeing up memory. With this, the > > > > > > > > > > > > LMB module becomes the common backend for managing non U-Boot image > > > > > > > > > > > > memory that might be requested by other modules. > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > > > > > > > > > > --- > > > > > > > > > > > > Changes since V2: > > > > > > > > > > > > * Use map_to_sysmem() to get the user-visible address to be shared > > > > > > > > > > > > with the lmb API's for sandbox. > > > > > > > > > > > > > > > > > > > > > > > > lib/efi_loader/Kconfig | 1 + > > > > > > > > > > > > lib/efi_loader/efi_memory.c | 77 +++++++++++-------------------------- > > > > > > > > > > > > 2 files changed, 24 insertions(+), 54 deletions(-) > > > > > > > > > > > > > > > > > > > > > > When efi_init_obj() is called, it should be able to add the lmb memory > > > > > > > > > > > to its own tables. There is no need to worry about lmb after that, > > > > > > > > > > > > Saying "There is no need to worry about lmb after that" is not true. > > > > > > Invoking the "env" command for example will have efi_init_obj() be > > > > > > called, among the others that Heinrich listed. And to possibly refute > > > > > > a next issue, that is intentional so that efivars, the standard > > > > > > mechanism used by an OS to talk with the firmware can be available to > > > > > > U-Boot, if I recall things correctly. > > > > > > > > > > > > My understanding of your assumption is that you believe that once the > > > > > > EFI_LOADER subsystem has started work on a payload we're just a few call > > > > > > chains away from the OS being started and runtime services aside U-Boot > > > > > > being done. > > > > > > > > > > > > My understanding of how things are used today is that this is incorrect. > > > > > > > > > > What I am getting at is that once we have called that function we know > > > > > we are booting an EFI app or using an EFI feature in preparation for > > > > > doing so. Let's start there. Is that correct? > > > > > > > > No, it is not correct. > > > > > > Can you give me a code link? > > > > env_set() -> env_do_env_set() -> do_env_set_efi() -> efi_init_obj_list() > > That's with the '-e' option, though, so people doing that are > specifically choosing EFI. They're choosing a feature, yes and from there may not use any more, and can / will be using "load" to put things in to memory. > What I am saying is that, if we are not booting an EFI app or using an > EFI feature. Since 'env set -e' is an EFI feature, efi_init_obj_list() > is called, and EFI is in the picture. > > Even if efi_init_obj_list(), I don't like treating U-Boot's memory as > a stack to just grow into. But as a first step, I do want to ensure > that non-EFI boot can be more like U-Boot. I think you intended a negation somewhere in that last sentence. But among the points trying to be made here are that no, we didn't start an EFI app (a previous point of contention you had), even if we used an EFI feature. We may or may not use more. But saying "I guess now we can live with the EFI pool doing what it needs" is quite silly since we've potentially done almost nothing else. And so why did we make this special case anyways? This is why I keep saying various versions of, you need to re-think what you consider "U-Boot memory". If you have concerns about our stack smashing in to things, that's the problem to address (it could just as easily smash an initrd placed high in memory by a non-EFI bootmeth).
Hi Tom, On Tue, 15 Oct 2024 at 13:05, Tom Rini <trini@konsulko.com> wrote: > > On Tue, Oct 15, 2024 at 07:10:27AM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Mon, 14 Oct 2024 at 17:07, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Mon, Oct 14, 2024 at 03:55:05PM -0600, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Mon, 14 Oct 2024 at 15:48, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > On Mon, Oct 14, 2024 at 03:35:12PM -0600, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Mon, 14 Oct 2024 at 15:27, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > On Mon, Oct 14, 2024 at 03:13:01PM -0600, Simon Glass wrote: > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > On Mon, 14 Oct 2024 at 15:04, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > > > On Mon, Oct 14, 2024 at 01:13:17PM -0600, Simon Glass wrote: > > > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > > > > > On Mon, 14 Oct 2024 at 09:56, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > > > > > > > On Mon, Oct 14, 2024 at 09:50:37AM -0600, Simon Glass wrote: > > > > > > > > > > > > Hi Sughosh, > > > > > > > > > > > > > > > > > > > > > > > > On Sun, 13 Oct 2024 at 04:55, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > Use the LMB API's for allocating and freeing up memory. With this, the > > > > > > > > > > > > > LMB module becomes the common backend for managing non U-Boot image > > > > > > > > > > > > > memory that might be requested by other modules. > > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > > > > > > > > > > > --- > > > > > > > > > > > > > Changes since V2: > > > > > > > > > > > > > * Use map_to_sysmem() to get the user-visible address to be shared > > > > > > > > > > > > > with the lmb API's for sandbox. > > > > > > > > > > > > > > > > > > > > > > > > > > lib/efi_loader/Kconfig | 1 + > > > > > > > > > > > > > lib/efi_loader/efi_memory.c | 77 +++++++++++-------------------------- > > > > > > > > > > > > > 2 files changed, 24 insertions(+), 54 deletions(-) > > > > > > > > > > > > > > > > > > > > > > > > When efi_init_obj() is called, it should be able to add the lmb memory > > > > > > > > > > > > to its own tables. There is no need to worry about lmb after that, > > > > > > > > > > > > > > Saying "There is no need to worry about lmb after that" is not true. > > > > > > > Invoking the "env" command for example will have efi_init_obj() be > > > > > > > called, among the others that Heinrich listed. And to possibly refute > > > > > > > a next issue, that is intentional so that efivars, the standard > > > > > > > mechanism used by an OS to talk with the firmware can be available to > > > > > > > U-Boot, if I recall things correctly. > > > > > > > > > > > > > > My understanding of your assumption is that you believe that once the > > > > > > > EFI_LOADER subsystem has started work on a payload we're just a few call > > > > > > > chains away from the OS being started and runtime services aside U-Boot > > > > > > > being done. > > > > > > > > > > > > > > My understanding of how things are used today is that this is incorrect. > > > > > > > > > > > > What I am getting at is that once we have called that function we know > > > > > > we are booting an EFI app or using an EFI feature in preparation for > > > > > > doing so. Let's start there. Is that correct? > > > > > > > > > > No, it is not correct. > > > > > > > > Can you give me a code link? > > > > > > env_set() -> env_do_env_set() -> do_env_set_efi() -> efi_init_obj_list() > > > > That's with the '-e' option, though, so people doing that are > > specifically choosing EFI. > > They're choosing a feature, yes and from there may not use any more, and > can / will be using "load" to put things in to memory. > > > What I am saying is that, if we are not booting an EFI app or using an > > EFI feature. Since 'env set -e' is an EFI feature, efi_init_obj_list() > > is called, and EFI is in the picture. > > > > Even if efi_init_obj_list(), I don't like treating U-Boot's memory as > > a stack to just grow into. But as a first step, I do want to ensure > > that non-EFI boot can be more like U-Boot. > > I think you intended a negation somewhere in that last sentence. It looks right to me. > > But among the points trying to be made here are that no, we didn't start > an EFI app (a previous point of contention you had), even if we used an > EFI feature. We may or may not use more. Yes, we have ended up with what I feel is a dog's breakfast. I'd like to tidy it up. > But saying "I guess now we can > live with the EFI pool doing what it needs" is quite silly since we've > potentially done almost nothing else. Do you mean that it is inconsistent for me to say that when efi_init_obj_list() is called, any memory can be used? Yes it is inconsistent. In fact I may as well just say that we should use U-Boot's region until we can't, e.g. we exhaust the reserved space. By that time, the boot will be well-advanced, I suspect. I'll take a look at some Ubuntu boots and see what happens in practice. > And so why did we make this > special case anyways? Once an EFI feature is used, we know we are booting EFI, so it might start using memory outside the U-Boot region. That's all. > This is why I keep saying various versions of, you > need to re-think what you consider "U-Boot memory". If you have concerns > about our stack smashing in to things, that's the problem to address (it > could just as easily smash an initrd placed high in memory by a non-EFI > bootmeth). I am not yet ready to re-think that, because my intuition tells me that I have this right. I would like to get some of my ideas in, to solve this problem. To do that, we need to be willing to let U-Boot allocate memory (on EFI's behalf) as it wishes, within the spec, not necessarily based on just what the code does today. Regards, SImon
On Thu, Oct 17, 2024 at 05:23:08PM -0600, Simon Glass wrote: > Hi Tom, > > On Tue, 15 Oct 2024 at 13:05, Tom Rini <trini@konsulko.com> wrote: > > > > On Tue, Oct 15, 2024 at 07:10:27AM -0600, Simon Glass wrote: [snip] > > > Even if efi_init_obj_list(), I don't like treating U-Boot's memory as > > > a stack to just grow into. But as a first step, I do want to ensure > > > that non-EFI boot can be more like U-Boot. > > > > I think you intended a negation somewhere in that last sentence. > > It looks right to me. I cannot understand what you mean by "I do want to ensure that non-EFI boot can be more like U-Boot." then, sorry. > > But among the points trying to be made here are that no, we didn't start > > an EFI app (a previous point of contention you had), even if we used an > > EFI feature. We may or may not use more. > > Yes, we have ended up with what I feel is a dog's breakfast. I'd like > to tidy it up. Getting back to a point I just made elsewhere, this is further you talking like the whole of EFI_LOADER is bad. > > But saying "I guess now we can > > live with the EFI pool doing what it needs" is quite silly since we've > > potentially done almost nothing else. > > Do you mean that it is inconsistent for me to say that when > efi_init_obj_list() is called, any memory can be used? Yes it is > inconsistent. In fact I may as well just say that we should use > U-Boot's region until we can't, e.g. we exhaust the reserved space. By > that time, the boot will be well-advanced, I suspect. I'll take a look > at some Ubuntu boots and see what happens in practice. > > > And so why did we make this > > special case anyways? > > Once an EFI feature is used, we know we are booting EFI, so it might > start using memory outside the U-Boot region. That's all. No, we don't. As another of your threads noted, we print a warning message about not being able to use persistent UEFI variables. At that point we can further go and see that no, this is not a UEFI-boot capable image and move on. But we've now crossed your point where you contend that UEFI is unavoidable. > > This is why I keep saying various versions of, you > > need to re-think what you consider "U-Boot memory". If you have concerns > > about our stack smashing in to things, that's the problem to address (it > > could just as easily smash an initrd placed high in memory by a non-EFI > > bootmeth). > > I am not yet ready to re-think that, because my intuition tells me > that I have this right. I would like to get some of my ideas in, to > solve this problem. To do that, we need to be willing to let U-Boot > allocate memory (on EFI's behalf) as it wishes, within the spec, not > necessarily based on just what the code does today. Well, in that I believe Heinrich has suggested some further changes to how the EFI memory map is handled, now that the lmb refactor is in and largely (but not entirely, Caleb made some good points on IRC Friday) done, we'll see what comes of that. But I'm not sure who you have convinced that we need to make these changes for a conceptual, rather than functional problem. All of that very much said, this is one of the times I wish I could figure out how to introduce a technical steering committee structure of some sort, and how to have matters voted on in some manner.
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index 6f6fa8d629..69b2c9360d 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -20,6 +20,7 @@ config EFI_LOADER select DM_EVENT select EVENT_DYNAMIC select LIB_UUID + select LMB imply PARTITION_UUIDS select REGEX imply FAT diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index c6f1dd0945..aa1da21f9f 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -9,6 +9,7 @@ #include <efi_loader.h> #include <init.h> +#include <lmb.h> #include <log.h> #include <malloc.h> #include <mapmem.h> @@ -432,53 +433,6 @@ static efi_status_t efi_check_allocated(u64 addr, bool must_be_allocated) return EFI_NOT_FOUND; } -/** - * efi_find_free_memory() - find free memory pages - * - * @len: size of memory area needed - * @max_addr: highest address to allocate - * Return: pointer to free memory area or 0 - */ -static uint64_t efi_find_free_memory(uint64_t len, uint64_t max_addr) -{ - struct efi_mem_list *lmem; - - /* - * Prealign input max address, so we simplify our matching - * logic below and can just reuse it as return pointer. - */ - max_addr &= ~EFI_PAGE_MASK; - - list_for_each_entry(lmem, &efi_mem, link) { - struct efi_mem_desc *desc = &lmem->desc; - uint64_t desc_len = desc->num_pages << EFI_PAGE_SHIFT; - uint64_t desc_end = desc->physical_start + desc_len; - uint64_t curmax = min(max_addr, desc_end); - uint64_t ret = curmax - len; - - /* We only take memory from free RAM */ - if (desc->type != EFI_CONVENTIONAL_MEMORY) - continue; - - /* Out of bounds for max_addr */ - if ((ret + len) > max_addr) - continue; - - /* Out of bounds for upper map limit */ - if ((ret + len) > desc_end) - continue; - - /* Out of bounds for lower map limit */ - if (ret < desc->physical_start) - continue; - - /* Return the highest address in this map within bounds */ - return ret; - } - - return 0; -} - /** * efi_allocate_pages - allocate memory pages * @@ -493,8 +447,9 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type, efi_uintn_t pages, uint64_t *memory) { u64 len; + uint flags; efi_status_t ret; - uint64_t addr; + phys_addr_t addr; /* Check import parameters */ if (memory_type >= EFI_PERSISTENT_MEMORY_TYPE && @@ -508,33 +463,37 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type, (len >> EFI_PAGE_SHIFT) != (u64)pages) return EFI_OUT_OF_RESOURCES; + flags = LMB_NOOVERWRITE | LMB_NONOTIFY; switch (type) { case EFI_ALLOCATE_ANY_PAGES: /* Any page */ - addr = efi_find_free_memory(len, -1ULL); + addr = (u64)lmb_alloc_flags(len, EFI_PAGE_SIZE, flags); if (!addr) return EFI_OUT_OF_RESOURCES; break; case EFI_ALLOCATE_MAX_ADDRESS: /* Max address */ - addr = efi_find_free_memory(len, *memory); + addr = map_to_sysmem((void *)(uintptr_t)*memory); + addr = (u64)lmb_alloc_base_flags(len, EFI_PAGE_SIZE, addr, + flags); if (!addr) return EFI_OUT_OF_RESOURCES; break; case EFI_ALLOCATE_ADDRESS: if (*memory & EFI_PAGE_MASK) return EFI_NOT_FOUND; - /* Exact address, reserve it. The addr is already in *memory. */ - ret = efi_check_allocated(*memory, false); - if (ret != EFI_SUCCESS) + + addr = map_to_sysmem((void *)(uintptr_t)*memory); + addr = (u64)lmb_alloc_addr_flags(addr, len, flags); + if (!addr) return EFI_NOT_FOUND; - addr = *memory; break; default: /* UEFI doesn't specify other allocation types */ return EFI_INVALID_PARAMETER; } + addr = (u64)(uintptr_t)map_sysmem(addr, 0); /* Reserve that map in our memory maps */ ret = efi_add_memory_map_pg(addr, pages, memory_type, true); if (ret != EFI_SUCCESS) @@ -555,6 +514,9 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type, */ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages) { + u64 len; + uint flags; + long status; efi_status_t ret; ret = efi_check_allocated(memory, true); @@ -568,6 +530,13 @@ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages) return EFI_INVALID_PARAMETER; } + flags = LMB_NOOVERWRITE | LMB_NONOTIFY; + len = (u64)pages << EFI_PAGE_SHIFT; + status = lmb_free_flags(map_to_sysmem((void *)(uintptr_t)memory), len, + flags); + if (status) + return EFI_NOT_FOUND; + ret = efi_add_memory_map_pg(memory, pages, EFI_CONVENTIONAL_MEMORY, false); if (ret != EFI_SUCCESS)
Use the LMB API's for allocating and freeing up memory. With this, the LMB module becomes the common backend for managing non U-Boot image memory that might be requested by other modules. Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> --- Changes since V2: * Use map_to_sysmem() to get the user-visible address to be shared with the lmb API's for sandbox. lib/efi_loader/Kconfig | 1 + lib/efi_loader/efi_memory.c | 77 +++++++++++-------------------------- 2 files changed, 24 insertions(+), 54 deletions(-)