Re: [PATCH 1/1] ArmPkg/MmCommunicationDxe: fix logic bug in DXE driver entrypoint

["Ard Biesheuvel" <ard.biesheuvel@linaro.org>]

On Mon, 2 Mar 2020 at 13:57, Leif Lindholm <leif@nuviainc.com> wrote:
>
> On Wed, Feb 26, 2020 at 13:41:03 +0100, Ard Biesheuvel wrote:
> > Commit 2fe25a74d6fee3c2 ("ArmPkg/MmCommunicationDxe: relay architected PI
> > events to MM context") update the ARM specific standalone MM client
> > driver to register for certain events in the entrypoint code, but did
> > so in a way that makes the entrypoint always return with an error.
> >
> > Instead, return EFI_SUCCESS if registering for those events succeeds,
> > and back out the registrations that did succeed if one fails, and
> > return an error.
> >
> > Fixes: 2fe25a74d6fee3c2 ("ArmPkg/MmCommunicationDxe: relay architected PI events to MM context")
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Reviewed-by: Leif Lindholm <leif@nuviainc.com>
>

Thanks.

> Are you looking for this one to make the stable tag? You're not
> explicitly asking for it, but you're cc:ing Liming.
>

I don't think it is terribly useful to get this into the stable tag,
given that the standalone MM support is not complete yet, and stalled
on a PI spec update.
But by the same reasoning, it could go in as well, since it is an
obvious bugfix, and the risk is low.

I don't really care either way tbh, I just wanted to make Liming aware of it.



> > ---
> >  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > index a4ee4dcd864c..6c06b3574861 100644
> > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > @@ -385,8 +385,16 @@ MmCommunication2Initialize (
> >                      MmGuidedEventNotify, mGuidedEventGuid[Index],
> >                      mGuidedEventGuid[Index], &mGuidedEvent[Index]);
> >      ASSERT_EFI_ERROR (Status);
> > +    if (EFI_ERROR (Status)) {
> > +      while (Index-- > 0) {
> > +        gBS->CloseEvent (mGuidedEvent[Index]);
> > +      }
> > +      goto UninstallProtocol;
> > +    }
> >    }
> > +  return EFI_SUCCESS;
> >
> > +UninstallProtocol:
> >    gBS->UninstallProtocolInterface (
> >           mMmCommunicateHandle,
> >           &gEfiMmCommunication2ProtocolGuid,
> > --
> > 2.17.1
> >