This commit doesn't change semantics.
It is done as a preparation for future commits which will modify
PCI attributes.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2567
Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
Signed-off-by: Liran Alon <liran.alon@oracle.com>
---
 OvmfPkg/PvScsiDxe/PvScsi.c | 53 +++++++++++++++++++++++++++++++++++++-
 OvmfPkg/PvScsiDxe/PvScsi.h |  1 +
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c
index b6a83d73cead..92e0f4a98965 100644
--- a/OvmfPkg/PvScsiDxe/PvScsi.c
+++ b/OvmfPkg/PvScsiDxe/PvScsi.c
@@ -281,18 +281,59 @@ PvScsiGetNextTarget (
   return EFI_NOT_FOUND;
 }
 
+STATIC
+EFI_STATUS
+PvScsiSetPCIAttributes (
+  IN OUT PVSCSI_DEV *Dev
+  )
+{
+  EFI_STATUS Status;
+
+  //
+  // Set saved original PCI attirubtes to invalid value
+  // such that cleanup logic could determine if it should restore
+  // PCI attributes or not
+  //
+  Dev->OriginalPciAttributes = (UINT64)(-1);
+
+  //
+  // Backup original PCI Attributes
+  //
+  Status = Dev->PciIo->Attributes (
+                         Dev->PciIo,
+                         EfiPciIoAttributeOperationGet,
+                         0,
+                         &Dev->OriginalPciAttributes
+                         );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  return EFI_SUCCESS;
+}
+
 STATIC
 EFI_STATUS
 PvScsiInit (
   IN OUT PVSCSI_DEV *Dev
   )
 {
+  EFI_STATUS Status;
+
   //
   // Init configuration
   //
   Dev->MaxTarget = PcdGet8 (PcdPvScsiMaxTargetLimit);
   Dev->MaxLun = PcdGet8 (PcdPvScsiMaxLunLimit);
 
+  //
+  // Set PCI Attributes
+  //
+  Status = PvScsiSetPCIAttributes (Dev);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
   //
   // Populate the exported interface's attributes
   //
@@ -331,7 +372,17 @@ PvScsiUninit (
   IN OUT PVSCSI_DEV *Dev
   )
 {
-  // Currently nothing to do here
+  //
+  // Restore PCI Attributes
+  //
+  if (Dev->OriginalPciAttributes != (UINT64)(-1)) {
+    Dev->PciIo->Attributes (
+                  Dev->PciIo,
+                  EfiPciIoAttributeOperationSet,
+                  Dev->OriginalPciAttributes,
+                  NULL
+                  );
+  }
 }
 
 //
diff --git a/OvmfPkg/PvScsiDxe/PvScsi.h b/OvmfPkg/PvScsiDxe/PvScsi.h
index e1e5ae18ebf2..5f611dbbc98c 100644
--- a/OvmfPkg/PvScsiDxe/PvScsi.h
+++ b/OvmfPkg/PvScsiDxe/PvScsi.h
@@ -20,6 +20,7 @@
 typedef struct {
   UINT32                          Signature;
   EFI_PCI_IO_PROTOCOL             *PciIo;
+  UINT64                          OriginalPciAttributes;
   UINT8                           MaxTarget;
   UINT8                           MaxLun;
   EFI_EXT_SCSI_PASS_THRU_PROTOCOL PassThru;
-- 
2.20.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55876): https://edk2.groups.io/g/devel/message/55876
Mute This Topic: https://groups.io/mt/72001278/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 03/16/20 16:01, Liran Alon wrote:
> This commit doesn't change semantics.
> It is done as a preparation for future commits which will modify
> PCI attributes.
>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2567
> Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
> Signed-off-by: Liran Alon <liran.alon@oracle.com>
> ---
>  OvmfPkg/PvScsiDxe/PvScsi.c | 53 +++++++++++++++++++++++++++++++++++++-
>  OvmfPkg/PvScsiDxe/PvScsi.h |  1 +
>  2 files changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c
> index b6a83d73cead..92e0f4a98965 100644
> --- a/OvmfPkg/PvScsiDxe/PvScsi.c
> +++ b/OvmfPkg/PvScsiDxe/PvScsi.c
> @@ -281,18 +281,59 @@ PvScsiGetNextTarget (
>    return EFI_NOT_FOUND;
>  }
>
> +STATIC
> +EFI_STATUS
> +PvScsiSetPCIAttributes (

(1) Minor wart: should be spelled "PvScsiSetPciAttributes".

> +  IN OUT PVSCSI_DEV *Dev
> +  )
> +{
> +  EFI_STATUS Status;
> +
> +  //
> +  // Set saved original PCI attirubtes to invalid value
> +  // such that cleanup logic could determine if it should restore
> +  // PCI attributes or not
> +  //
> +  Dev->OriginalPciAttributes = (UINT64)(-1);
> +
> +  //
> +  // Backup original PCI Attributes
> +  //
> +  Status = Dev->PciIo->Attributes (
> +                         Dev->PciIo,
> +                         EfiPciIoAttributeOperationGet,
> +                         0,
> +                         &Dev->OriginalPciAttributes
> +                         );
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }

No, this is again going in the wrong direction.

(3) Even if we wanted to go in this direction (and we don't), we should
store the  "invalid value" *after* determining failure status. Because,
I can't find any guarantee in the spec that Attributes() does not modify
"Result" on error.

(4) I don't like the assumption that "all-bits-set" is going to be an
invalid attribute mask forever.

(5) Most importantly, we're again mixing error handling styles. This
problem will be more apparent in later patches in the series, but this
is where it starts.

Saving the original PCI attributes, setting the new attributes, and
restoring the original ones, are *not* optional actions. They are
mandatory. Therefore we do not need "tracker" variables or even tracker
values. Whenever any one of those actions can be performed, then it
*must* be performed -- and that fact is deducible purely from the
control flow.

Looking at the "PvScsi.c" source file at the end of this series, this is
the (partial) call tree that I see:

  PvScsiDriverBindingStart
   PvScsiInit
     PvScsiSetPCIAttributes
     PvScsiWriteCmdDesc (PVSCSI_CMD_ADAPTER_RESET)
     PvScsiInitRings
     PvScsiAllocateSharedPages

Unfortunately, the PvScsiInit() function fails to roll back any of those
construction steps on error. In particular:

- the shared pages are not released,
- the rings are not un-inited,
- the original attributes are not restored.

This causes a bunch of leaks. Also, confusion on the normal teardown
path (initiated from Stop()), as evidenced by the "sentinel" value for
"OriginalPciAttributes".

You need to follow the exact same cascading error handling pattern in
*every* construction function that you follow in
PvScsiDriverBindingStart(). PvScsiInit() *in itself* should be atomic,
meaning that *either* all of its actions should succeed, *or* it should
roll back all partically completed construction steps.

For example, if the PvScsiAllocateSharedPages() call fails in
PvScsiInit(), then the rings need to be uninited, and the original PCI
attributes need to be restored, before we exit PvScsiInit() with a
failure status. Because, in this case (i.e., when PvScsiInit() fails),
PvScsiDriverBindingStart() will also take the error path, and we will
never touch the PCI attributes or the rings ever again.

*Consequently* (and I'm getting to my main point here, at long last), in
the PvScsiUninit() function, you can *unconditionally* restore the
original PCI attributes, because you know that, at that location, the
original attributes will have been saved. Whenever that is not the case,
then PvScsiUninit() is not reached!

This is not complicated. Simply follow the pattern rigorously in *every*
function:

- construct and allocate resources gradually,

- if the very first such step fails, return directly,

- if any one of the subsequent steps fails, jump to an error handling
  label matching the last successful allocation / construction step,

- error handling labels should mirror the construction steps in reverse
  order, and they should release the corresponding (partially
  allocated/constructed) resources,

- every function should be atomic: fully successful, or completely
  rolled back,

- any given "teardown" function should be almost identical to the error
  path of the corresponding "init" function, with two small differences:
  (a) there should be no labels, (b) the final construction step of the
  "init" function should be rolled back too, namely as the first action
  in the "teardown" function.

The beauty of this approach is that you don't even have to *think* about
managing resources. It is entirely mechanical.

Of course there are exceptions to this pattern, such as:
- ownership transfer,
- optionally used resources.

They do need special handling, but they are exceptions, not the norm.

So, let me proceed to pin-pointing the problem in this specific patch --
see bullet (6) below:

> +
> +  return EFI_SUCCESS;
> +}
> +
>  STATIC
>  EFI_STATUS
>  PvScsiInit (
>    IN OUT PVSCSI_DEV *Dev
>    )
>  {
> +  EFI_STATUS Status;
> +
>    //
>    // Init configuration
>    //
>    Dev->MaxTarget = PcdGet8 (PcdPvScsiMaxTargetLimit);
>    Dev->MaxLun = PcdGet8 (PcdPvScsiMaxLunLimit);
>
> +  //
> +  // Set PCI Attributes
> +  //
> +  Status = PvScsiSetPCIAttributes (Dev);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +

Note that, if PvScsiSetPCIAttributes() fails -- and so we couldn't save
a valid value to the "OriginalPciAttributes" field --, we return from
PvScsiInit() with an error code, here.

That causes PvScsiDriverBindingStart() to jump to the "ClosePciIo"
label, and ultimately propagate the error status outwards. The driver's
interaction with the controller handle is done, then.

Subsequently, PvScsiUninit() will *never* be reached, because the system
is never going to invoke PvScsiDriverBindingStop(), given that Start()
failed in the first place. Therefore:

>    //
>    // Populate the exported interface's attributes
>    //
> @@ -331,7 +372,17 @@ PvScsiUninit (
>    IN OUT PVSCSI_DEV *Dev
>    )
>  {
> -  // Currently nothing to do here
> +  //
> +  // Restore PCI Attributes
> +  //
> +  if (Dev->OriginalPciAttributes != (UINT64)(-1)) {

(6) This condition is superfluous here. Whenever it is reached, it
always evaluates to 1.

Specifically:

- If PvScsiUninit() is being called from under the "UninitDev" label in
  PvScsiDriverBindingStart(), then PvScsiInit() has succeeded. That
  means PvScsiSetPCIAttributes() was successful too.

- If PvScsiUninit() is being called from PvScsiDriverBindingStop(), then
  PvScsiDriverBindingStart() succeeded. That implies PvScsiInit()
  succeeded too.


Additionally, I think you are needlessly complicating the driver by
pushing the PCI attribute manipulation down to a helper function
(PvScsiSetPCIAttributes) *without* mirroring that helper function with
another helper function on the "teardown" path.

Because this way, the PCI attribute restoration is flattened into
PvScsiUninit() -- and that *does* make things harder to reason about,
because you don't have a matching path to mirror, from PvScsiInit().

Let me emphasize: the needless complication is not that
PvScsiSetPciAttributes() exists. The complication is that a counterpart
function does not exist.

In summary, I recommend the following (incremental) updates to this
patch, expressed as code:

> diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c
> index 92e0f4a98965..2732aaa9b471 100644
> --- a/OvmfPkg/PvScsiDxe/PvScsi.c
> +++ b/OvmfPkg/PvScsiDxe/PvScsi.c
> @@ -283,19 +283,12 @@ PvScsiGetNextTarget (
>
>  STATIC
>  EFI_STATUS
> -PvScsiSetPCIAttributes (
> +PvScsiSetPciAttributes (
>    IN OUT PVSCSI_DEV *Dev
>    )
>  {
>    EFI_STATUS Status;
>
> -  //
> -  // Set saved original PCI attirubtes to invalid value
> -  // such that cleanup logic could determine if it should restore
> -  // PCI attributes or not
> -  //
> -  Dev->OriginalPciAttributes = (UINT64)(-1);
> -
>    //
>    // Backup original PCI Attributes
>    //
> @@ -312,6 +305,20 @@ PvScsiSetPCIAttributes (
>    return EFI_SUCCESS;
>  }
>
> +STATIC
> +VOID
> +PvScsiRestorePciAttributes (
> +  IN PVSCSI_DEV *Dev
> +  )
> +{
> +  Dev->PciIo->Attributes (
> +                Dev->PciIo,
> +                EfiPciIoAttributeOperationSet,
> +                Dev->OriginalPciAttributes,
> +                NULL
> +                );
> +}
> +
>  STATIC
>  EFI_STATUS
>  PvScsiInit (
> @@ -329,10 +336,15 @@ PvScsiInit (
>    //
>    // Set PCI Attributes
>    //
> -  Status = PvScsiSetPCIAttributes (Dev);
> +  Status = PvScsiSetPciAttributes (Dev);
>    if (EFI_ERROR (Status)) {
>      return Status;
>    }
> +  //
> +  // NOTE: if any steps fail below, we need to introduce a new error handling
> +  // label called "RestorePciAttributes", and call PvScsiRestorePciAttributes()
> +  // there.
> +  //
>
>    //
>    // Populate the exported interface's attributes
> @@ -372,17 +384,7 @@ PvScsiUninit (
>    IN OUT PVSCSI_DEV *Dev
>    )
>  {
> -  //
> -  // Restore PCI Attributes
> -  //
> -  if (Dev->OriginalPciAttributes != (UINT64)(-1)) {
> -    Dev->PciIo->Attributes (
> -                  Dev->PciIo,
> -                  EfiPciIoAttributeOperationSet,
> -                  Dev->OriginalPciAttributes,
> -                  NULL
> -                  );
> -  }
> +  PvScsiRestorePciAttributes (Dev);
>  }
>
>  //

Because, that will make for the following *cumulative* patch (replacing
the current patch):

> diff --git a/OvmfPkg/PvScsiDxe/PvScsi.h b/OvmfPkg/PvScsiDxe/PvScsi.h
> index e1e5ae18ebf2..5f611dbbc98c 100644
> --- a/OvmfPkg/PvScsiDxe/PvScsi.h
> +++ b/OvmfPkg/PvScsiDxe/PvScsi.h
> @@ -20,6 +20,7 @@
>  typedef struct {
>    UINT32                          Signature;
>    EFI_PCI_IO_PROTOCOL             *PciIo;
> +  UINT64                          OriginalPciAttributes;
>    UINT8                           MaxTarget;
>    UINT8                           MaxLun;
>    EFI_EXT_SCSI_PASS_THRU_PROTOCOL PassThru;
> diff --git a/OvmfPkg/PvScsiDxe/PvScsi.c b/OvmfPkg/PvScsiDxe/PvScsi.c
> index b6a83d73cead..2732aaa9b471 100644
> --- a/OvmfPkg/PvScsiDxe/PvScsi.c
> +++ b/OvmfPkg/PvScsiDxe/PvScsi.c
> @@ -281,18 +281,71 @@ PvScsiGetNextTarget (
>    return EFI_NOT_FOUND;
>  }
>
> +STATIC
> +EFI_STATUS
> +PvScsiSetPciAttributes (
> +  IN OUT PVSCSI_DEV *Dev
> +  )
> +{
> +  EFI_STATUS Status;
> +
> +  //
> +  // Backup original PCI Attributes
> +  //
> +  Status = Dev->PciIo->Attributes (
> +                         Dev->PciIo,
> +                         EfiPciIoAttributeOperationGet,
> +                         0,
> +                         &Dev->OriginalPciAttributes
> +                         );
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC
> +VOID
> +PvScsiRestorePciAttributes (
> +  IN PVSCSI_DEV *Dev
> +  )
> +{
> +  Dev->PciIo->Attributes (
> +                Dev->PciIo,
> +                EfiPciIoAttributeOperationSet,
> +                Dev->OriginalPciAttributes,
> +                NULL
> +                );
> +}
> +
>  STATIC
>  EFI_STATUS
>  PvScsiInit (
>    IN OUT PVSCSI_DEV *Dev
>    )
>  {
> +  EFI_STATUS Status;
> +
>    //
>    // Init configuration
>    //
>    Dev->MaxTarget = PcdGet8 (PcdPvScsiMaxTargetLimit);
>    Dev->MaxLun = PcdGet8 (PcdPvScsiMaxLunLimit);
>
> +  //
> +  // Set PCI Attributes
> +  //
> +  Status = PvScsiSetPciAttributes (Dev);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +  //
> +  // NOTE: if any steps fail below, we need to introduce a new error handling
> +  // label called "RestorePciAttributes", and call PvScsiRestorePciAttributes()
> +  // there.
> +  //
> +
>    //
>    // Populate the exported interface's attributes
>    //
> @@ -331,7 +384,7 @@ PvScsiUninit (
>    IN OUT PVSCSI_DEV *Dev
>    )
>  {
> -  // Currently nothing to do here
> +  PvScsiRestorePciAttributes (Dev);
>  }
>
>  //

Note the key pattern: whenever we add a new construction / allocation
step to any "init" function (such as PvScsiInit()), we add the matching
"undo" step in *two* spots:

- on the error path in the same "init" function, in case a subsequent
  step fails,

- in the "teardown" counterpart of the "init" function (such as
  PvScsiUninit()).

I'm going to stop reviewing this iteration now; please rework the rest
of the series for v2 with this resource management pattern.

Thanks!
Laszlo

On 03/16/20 16:01, Liran Alon wrote:

> +    Dev->PciIo->Attributes (
> +                  Dev->PciIo,
> +                  EfiPciIoAttributeOperationSet,
> +                  Dev->OriginalPciAttributes,
> +                  NULL
> +                  );
> +  }
>  }
>
>  //
> diff --git a/OvmfPkg/PvScsiDxe/PvScsi.h b/OvmfPkg/PvScsiDxe/PvScsi.h
> index e1e5ae18ebf2..5f611dbbc98c 100644
> --- a/OvmfPkg/PvScsiDxe/PvScsi.h
> +++ b/OvmfPkg/PvScsiDxe/PvScsi.h
> @@ -20,6 +20,7 @@
>  typedef struct {
>    UINT32                          Signature;
>    EFI_PCI_IO_PROTOCOL             *PciIo;
> +  UINT64                          OriginalPciAttributes;
>    UINT8                           MaxTarget;
>    UINT8                           MaxLun;
>    EFI_EXT_SCSI_PASS_THRU_PROTOCOL PassThru;
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#56155): https://edk2.groups.io/g/devel/message/56155
Mute This Topic: https://groups.io/mt/72001278/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 24/03/2020 17:14, Laszlo Ersek wrote:
> On 03/16/20 16:01, Liran Alon wrote:
> I'm going to stop reviewing this iteration now; please rework the rest
> of the series for v2 with this resource management pattern.
>
> Thanks!
> Laszlo

Thanks for the very detailed review!

I have read all your comments and understood them. I am working on 
creating a v2 patch-series with all these fixed.
As the Init/UnInit logic is the only place in which this resource 
management pattern is done, and as patch-series is split
to small well-defined quite independent patches, can I request that you 
will complete review of all v1 patches?

I will understand if you are too busy for this, but it will allow me to 
take into account all your aggregated comments on v1
to hopefully create a perfect v2 patch-series. As I've tried taking into 
account all the comments you gave on Nikita's series.

Thanks!
-Liran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#56158): https://edk2.groups.io/g/devel/message/56158
Mute This Topic: https://groups.io/mt/72001278/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 03/24/20 16:35, Liran Alon wrote:
> 
> On 24/03/2020 17:14, Laszlo Ersek wrote:
>> On 03/16/20 16:01, Liran Alon wrote:
>> I'm going to stop reviewing this iteration now; please rework the rest
>> of the series for v2 with this resource management pattern.
>>
>> Thanks!
>> Laszlo
> 
> Thanks for the very detailed review!
> 
> I have read all your comments and understood them. I am working on
> creating a v2 patch-series with all these fixed.
> As the Init/UnInit logic is the only place in which this resource
> management pattern is done, and as patch-series is split
> to small well-defined quite independent patches, can I request that you
> will complete review of all v1 patches?
> 
> I will understand if you are too busy for this, but it will allow me to
> take into account all your aggregated comments on v1
> to hopefully create a perfect v2 patch-series. As I've tried taking into
> account all the comments you gave on Nikita's series.

I ended up doing a superficial run over the rest of the series, before
arriving at this email of yours.

Unfortunately, in the most "meaty" patches, there are many style issues,
and they kept throwing me off. It's difficult to ping-pong between style
remarks and semantics. Basically, in those patches, I'm asking for style
fixes now, so I have a chance at a more substantial review with v2.

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#56232): https://edk2.groups.io/g/devel/message/56232
Mute This Topic: https://groups.io/mt/72001278/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 25/03/2020 3:48, Laszlo Ersek wrote:
> On 03/24/20 16:35, Liran Alon wrote:
>> On 24/03/2020 17:14, Laszlo Ersek wrote:
>>> On 03/16/20 16:01, Liran Alon wrote:
>>> I'm going to stop reviewing this iteration now; please rework the rest
>>> of the series for v2 with this resource management pattern.
>>>
>>> Thanks!
>>> Laszlo
>> Thanks for the very detailed review!
>>
>> I have read all your comments and understood them. I am working on
>> creating a v2 patch-series with all these fixed.
>> As the Init/UnInit logic is the only place in which this resource
>> management pattern is done, and as patch-series is split
>> to small well-defined quite independent patches, can I request that you
>> will complete review of all v1 patches?
>>
>> I will understand if you are too busy for this, but it will allow me to
>> take into account all your aggregated comments on v1
>> to hopefully create a perfect v2 patch-series. As I've tried taking into
>> account all the comments you gave on Nikita's series.
> I ended up doing a superficial run over the rest of the series, before
> arriving at this email of yours.
>
> Unfortunately, in the most "meaty" patches, there are many style issues,
> and they kept throwing me off. It's difficult to ping-pong between style
> remarks and semantics. Basically, in those patches, I'm asking for style
> fixes now, so I have a chance at a more substantial review with v2.
>
> Thanks
> Laszlo
>
Thanks for reviewing also the other patches for style issues as-well. 
This is what I requested.
This will assist me in submitting a much more ready v2 submission.
I'm currently working on it and hope to have a v2 submission later today 
with all your fixes and suggestions applied.

Highly appreciate your detailed review comments.

Thanks,
-Liran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#56273): https://edk2.groups.io/g/devel/message/56273
Mute This Topic: https://groups.io/mt/72001278/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-