[PATCH] lmb: Correctly unmap memory after notifications

[Ilias Apalodimas]

Hi Heinrich

On Mon, 28 Oct 2024 at 08:37, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 10/24/24 12:46, Ilias Apalodimas wrote:
> > We never unmap the memory used to update the EFI memory map after
> > notifications
> >
> > Fixes: commit 2f6191526a13 ("lmb: notify of any changes to the LMB memory map")
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > ---
> >   lib/lmb.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/lmb.c b/lib/lmb.c
> > index 890e2cbfdf6b..38c6e1d5ba8d 100644
> > --- a/lib/lmb.c
> > +++ b/lib/lmb.c
> > @@ -65,6 +65,7 @@ static int __maybe_unused lmb_map_update_notify(phys_addr_t addr,
> >                       status & ~EFI_ERROR_MASK);
> >               return -1;
> >       }
> > +     unmap_sysmem((void *)(uintptr_t)efi_addr);
>
> If PCI memory was ever mapped via pci_map_physmem(), I don't think that
> we want to unmap it here.

Ok, so looking at it again, not calling unmap_sysmem() here won't
break anything, so I guess we can drop this patch.

>
> Why do we map the memory in this notification function first hand?

Because the memory was added as such in efi_allocate_pages(). So for
sandbox we need to find the correct virtual memory that was added.

> Can't
> we use len = 0 when calling map_sysmem()?

We are using len = 0. But I am not following up on how could this
help. All you will get is a warning for sanbox if a pci device mapped
more that 0.

Thanks
/Ilias

>
> I guess we have to critically review all map_sysmem() calls.
>
> Best regards
>
> Heinrich
>
> >
> >       return 0;
> >   }
>