diff mbox series

[v3,02/15] lmb: add a flag to allow suppressing memory map change notification

Message ID 20241013105522.391414-3-sughosh.ganu@linaro.org
State New
Headers show
Series Make EFI memory allocations synchronous with LMB | expand

Commit Message

Sughosh Ganu Oct. 13, 2024, 10:55 a.m. UTC
Add a flag LMB_NONOTIFY that can be passed to the LMB API's for
reserving memory. This will then result in no notification being sent
from the LMB module for the changes to the LMB's memory map.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
Changes since V2: None

 include/lmb.h | 1 +
 lib/lmb.c     | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

Comments

Tom Rini Oct. 13, 2024, 5:51 p.m. UTC | #1
On Sun, Oct 13, 2024 at 04:25:09PM +0530, Sughosh Ganu wrote:

> Add a flag LMB_NONOTIFY that can be passed to the LMB API's for
> reserving memory. This will then result in no notification being sent
> from the LMB module for the changes to the LMB's memory map.
> 
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>

Can you please elaborate on what we need this flag for, for example what
the valid situation for it is? We should be updating (or adding comments
such that updates are automatic..) to doc/api/lmb.rst as well. Thanks.
Sughosh Ganu Oct. 14, 2024, 6:56 a.m. UTC | #2
On Sun, 13 Oct 2024 at 23:21, Tom Rini <trini@konsulko.com> wrote:
>
> On Sun, Oct 13, 2024 at 04:25:09PM +0530, Sughosh Ganu wrote:
>
> > Add a flag LMB_NONOTIFY that can be passed to the LMB API's for
> > reserving memory. This will then result in no notification being sent
> > from the LMB module for the changes to the LMB's memory map.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
>
> Can you please elaborate on what we need this flag for, for example what
> the valid situation for it is? We should be updating (or adding comments
> such that updates are automatic..) to doc/api/lmb.rst as well. Thanks.

The flag is used to prevent notifying the EFI code about the change in
the LMB memory map when the alloc/free request originates in the EFI
memory module. The EFI memory module adds the memory region to its
memory map as part of the alloc/free operation. The idea is that the
LMB module should notify EFI about changes to its memory map when
these changes are initiated from non-EFI consumers of memory.

-sughosh

>
> --
> Tom
Tom Rini Oct. 14, 2024, 3:02 p.m. UTC | #3
On Mon, Oct 14, 2024 at 12:26:02PM +0530, Sughosh Ganu wrote:
> On Sun, 13 Oct 2024 at 23:21, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sun, Oct 13, 2024 at 04:25:09PM +0530, Sughosh Ganu wrote:
> >
> > > Add a flag LMB_NONOTIFY that can be passed to the LMB API's for
> > > reserving memory. This will then result in no notification being sent
> > > from the LMB module for the changes to the LMB's memory map.
> > >
> > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> >
> > Can you please elaborate on what we need this flag for, for example what
> > the valid situation for it is? We should be updating (or adding comments
> > such that updates are automatic..) to doc/api/lmb.rst as well. Thanks.
> 
> The flag is used to prevent notifying the EFI code about the change in
> the LMB memory map when the alloc/free request originates in the EFI
> memory module.

Simon, this is the loop you referred to on IRC the other day, and
instead you solve this with the flag about a point of cooperation?

> The EFI memory module adds the memory region to its
> memory map as part of the alloc/free operation. The idea is that the
> LMB module should notify EFI about changes to its memory map when
> these changes are initiated from non-EFI consumers of memory.

OK, so the first change is that since doc/api/lmb.rst is just a
kerneldoc of include/lmb.h we need to add the flag here, and we need to
first document LMB_NOOVERWRITE which is already undocumented. The next
change is that at the end of the series, update the rst with some
paragraphs about what and why, for clarity, after auditing include/lmb.h
for other items that are missing kerneldoc comments. I am willing to
make that second change a follow-up series, however.
Simon Glass Oct. 14, 2024, 3:51 p.m. UTC | #4
Hi Tom,

On Mon, 14 Oct 2024 at 09:02, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Oct 14, 2024 at 12:26:02PM +0530, Sughosh Ganu wrote:
> > On Sun, 13 Oct 2024 at 23:21, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Sun, Oct 13, 2024 at 04:25:09PM +0530, Sughosh Ganu wrote:
> > >
> > > > Add a flag LMB_NONOTIFY that can be passed to the LMB API's for
> > > > reserving memory. This will then result in no notification being sent
> > > > from the LMB module for the changes to the LMB's memory map.
> > > >
> > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > >
> > > Can you please elaborate on what we need this flag for, for example what
> > > the valid situation for it is? We should be updating (or adding comments
> > > such that updates are automatic..) to doc/api/lmb.rst as well. Thanks.
> >
> > The flag is used to prevent notifying the EFI code about the change in
> > the LMB memory map when the alloc/free request originates in the EFI
> > memory module.
>
> Simon, this is the loop you referred to on IRC the other day, and
> instead you solve this with the flag about a point of cooperation?

Yes, that's right. I'd like to have EFI allocations confined to the
EFI area until efi_init_obj_list() is called, then have EFI add the
lmb reservations to its own table, then own memory-allocation from
then on.

Basically if we apply my series[1] first, it will remove the need for
this EFI call and the associated complexity. Most of this series will
still be needed though.

>
> > The EFI memory module adds the memory region to its
> > memory map as part of the alloc/free operation. The idea is that the
> > LMB module should notify EFI about changes to its memory map when
> > these changes are initiated from non-EFI consumers of memory.
>
> OK, so the first change is that since doc/api/lmb.rst is just a
> kerneldoc of include/lmb.h we need to add the flag here, and we need to
> first document LMB_NOOVERWRITE which is already undocumented. The next
> change is that at the end of the series, update the rst with some
> paragraphs about what and why, for clarity, after auditing include/lmb.h
> for other items that are missing kerneldoc comments. I am willing to
> make that second change a follow-up series, however.

Regards,
Simon

[1] https://patchwork.ozlabs.org/project/uboot/list/?series=427729
Heinrich Schuchardt Oct. 14, 2024, 5:12 p.m. UTC | #5
On 14.10.24 17:02, Tom Rini wrote:
> On Mon, Oct 14, 2024 at 12:26:02PM +0530, Sughosh Ganu wrote:
>> On Sun, 13 Oct 2024 at 23:21, Tom Rini <trini@konsulko.com> wrote:
>>>
>>> On Sun, Oct 13, 2024 at 04:25:09PM +0530, Sughosh Ganu wrote:
>>>
>>>> Add a flag LMB_NONOTIFY that can be passed to the LMB API's for
>>>> reserving memory. This will then result in no notification being sent
>>>> from the LMB module for the changes to the LMB's memory map.
>>>>
>>>> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
>>>
>>> Can you please elaborate on what we need this flag for, for example what
>>> the valid situation for it is? We should be updating (or adding comments
>>> such that updates are automatic..) to doc/api/lmb.rst as well. Thanks.
>>
>> The flag is used to prevent notifying the EFI code about the change in
>> the LMB memory map when the alloc/free request originates in the EFI
>> memory module.
>
> Simon, this is the loop you referred to on IRC the other day, and
> instead you solve this with the flag about a point of cooperation?
>
>> The EFI memory module adds the memory region to its
>> memory map as part of the alloc/free operation. The idea is that the
>> LMB module should notify EFI about changes to its memory map when
>> these changes are initiated from non-EFI consumers of memory.
>
> OK, so the first change is that since doc/api/lmb.rst is just a
> kerneldoc of include/lmb.h we need to add the flag here, and we need to
> first document LMB_NOOVERWRITE which is already undocumented. The next
> change is that at the end of the series, update the rst with some
> paragraphs about what and why, for clarity, after auditing include/lmb.h
> for other items that are missing kerneldoc comments. I am willing to
> make that second change a follow-up series, however.
>

Cf.
[v2,1/1] lmb: add missing LMB_NOOVERWRITE description
https://patchwork.ozlabs.org/project/uboot/patch/20240919105523.179497-2-heinrich.schuchardt@canonical.com/

Best regards

Heinrich
diff mbox series

Patch

diff --git a/include/lmb.h b/include/lmb.h
index 0e8426f437..a76262d520 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -23,6 +23,7 @@  enum lmb_flags {
 	LMB_NONE		= 0,
 	LMB_NOMAP		= BIT(1),
 	LMB_NOOVERWRITE		= BIT(2),
+	LMB_NONOTIFY		= BIT(3),
 };
 
 /**
diff --git a/lib/lmb.c b/lib/lmb.c
index a38537af9c..e1e616679f 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -30,7 +30,7 @@  static struct lmb lmb;
 static void lmb_print_region_flags(enum lmb_flags flags)
 {
 	u64 bitpos;
-	const char *flag_str[] = { "none", "no-map", "no-overwrite" };
+	const char *flag_str[] = { "none", "no-map", "no-overwrite", "no-notify" };
 
 	do {
 		bitpos = flags ? fls(flags) - 1 : 0;