* [PATCH 1/1] ArmPkg/MmCommunicationDxe: fix logic bug in DXE driver entrypoint
@ 2020-02-26 12:41 Ard Biesheuvel
2020-03-02 12:57 ` Leif Lindholm
0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2020-02-26 12:41 UTC (permalink / raw)
To: devel; +Cc: leif, liming.gao, Ard Biesheuvel
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>
---
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] ArmPkg/MmCommunicationDxe: fix logic bug in DXE driver entrypoint
2020-02-26 12:41 [PATCH 1/1] ArmPkg/MmCommunicationDxe: fix logic bug in DXE driver entrypoint Ard Biesheuvel
@ 2020-03-02 12:57 ` Leif Lindholm
2020-03-02 13:00 ` Ard Biesheuvel
0 siblings, 1 reply; 5+ messages in thread
From: Leif Lindholm @ 2020-03-02 12:57 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: devel, liming.gao
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>
Are you looking for this one to make the stable tag? You're not
explicitly asking for it, but you're cc:ing Liming.
> ---
> 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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] ArmPkg/MmCommunicationDxe: fix logic bug in DXE driver entrypoint
2020-03-02 12:57 ` Leif Lindholm
@ 2020-03-02 13:00 ` Ard Biesheuvel
2020-03-02 13:12 ` Leif Lindholm
0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2020-03-02 13:00 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel-groups-io, Gao, Liming
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
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] ArmPkg/MmCommunicationDxe: fix logic bug in DXE driver entrypoint
2020-03-02 13:00 ` Ard Biesheuvel
@ 2020-03-02 13:12 ` Leif Lindholm
2020-03-04 17:44 ` Ard Biesheuvel
0 siblings, 1 reply; 5+ messages in thread
From: Leif Lindholm @ 2020-03-02 13:12 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel-groups-io, Gao, Liming
On Mon, Mar 02, 2020 at 14:00:54 +0100, Ard Biesheuvel wrote:
> 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.
Thanks for explanation.
I'd say leave it then - if tag gets pushed Wednesday as I think the
current plan is, you can push it then.
/
Leif
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] ArmPkg/MmCommunicationDxe: fix logic bug in DXE driver entrypoint
2020-03-02 13:12 ` Leif Lindholm
@ 2020-03-04 17:44 ` Ard Biesheuvel
0 siblings, 0 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2020-03-04 17:44 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel-groups-io, Gao, Liming
On Mon, 2 Mar 2020 at 14:12, Leif Lindholm <leif@nuviainc.com> wrote:
>
> On Mon, Mar 02, 2020 at 14:00:54 +0100, Ard Biesheuvel wrote:
> > 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.
>
> Thanks for explanation.
> I'd say leave it then - if tag gets pushed Wednesday as I think the
> current plan is, you can push it then.
>
Pushed as 3be909099cf66f26a5ace282e4cbbdc6bf7cb32b
Thanks
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-03-04 17:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-26 12:41 [PATCH 1/1] ArmPkg/MmCommunicationDxe: fix logic bug in DXE driver entrypoint Ard Biesheuvel
2020-03-02 12:57 ` Leif Lindholm
2020-03-02 13:00 ` Ard Biesheuvel
2020-03-02 13:12 ` Leif Lindholm
2020-03-04 17:44 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox