public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* Patch List for 202005 stable tag
@ 2020-05-19 11:09 Liming Gao
  2020-05-19 12:04 ` Leif Lindholm
  2020-05-20  9:47 ` [edk2-devel] " Maciej Rabeda
  0 siblings, 2 replies; 10+ messages in thread
From: Liming Gao @ 2020-05-19 11:09 UTC (permalink / raw)
  To: Leif Lindholm, Laszlo Ersek, Kinney, Michael D, afish@apple.com
  Cc: devel@edk2.groups.io

[-- Attachment #1: Type: text/plain, Size: 679 bytes --]

Hi Stewards and all:
  I collect current patch lists in devel mail list. Those patch contributors request to add them for 200205 stable tag. Because we have enter into Soft Feature Freeze, I want to collect your feedback for them. If any patches are missing, please reply this mail to add them.

Feature List (pass review):
https://edk2.groups.io/g/devel/message/59578 [PATCH V7 0/1] Disable safe string constraint assertions
https://edk2.groups.io/g/devel/message/59589 [PATCH V7 0/6] Add definitions introduced in UEFI 2.8a

Bug List (pass review):
https://edk2.groups.io/g/devel/message/59735 [PATCH v3] SecurityPkg: Change default value source

Thanks
Liming



[-- Attachment #2: Type: text/html, Size: 5326 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Patch List for 202005 stable tag
  2020-05-19 11:09 Patch List for 202005 stable tag Liming Gao
@ 2020-05-19 12:04 ` Leif Lindholm
  2020-05-19 17:17   ` Laszlo Ersek
  2020-05-20  9:47 ` [edk2-devel] " Maciej Rabeda
  1 sibling, 1 reply; 10+ messages in thread
From: Leif Lindholm @ 2020-05-19 12:04 UTC (permalink / raw)
  To: Gao, Liming
  Cc: Laszlo Ersek, Kinney, Michael D, afish@apple.com,
	devel@edk2.groups.io

On Tue, May 19, 2020 at 11:09:42 +0000, Gao, Liming wrote:
> Hi Stewards and all:
>   I collect current patch lists in devel mail list. Those patch contributors request to add them for 200205 stable tag. Because we have enter into Soft Feature Freeze, I want to collect your feedback for them. If any patches are missing, please reply this mail to add them.
> 
> Feature List (pass review):
> https://edk2.groups.io/g/devel/message/59578 [PATCH V7 0/1] Disable safe string constraint assertions

This one I might even be persuaded to wave through in hard
freeze. Yes, please include.

> https://edk2.groups.io/g/devel/message/59589 [PATCH V7 0/6] Add definitions introduced in UEFI 2.8a

1,3-4 only adds definitions - I'm fine with those.

2/6 looks out of place compared with the set description, but as it
falls within the scope of "align codebase with current spec", and it
is a correction, it can stay. (It should also not break anything, and
if it does, that's automatically a bug isn't it?)

5-6, I have no comment on. Someone else will have to make that call.

> Bug List (pass review):
> https://edk2.groups.io/g/devel/message/59735 [PATCH v3] SecurityPkg: Change default value source

Patch is pretty garbled in groups.io. Thankfully, it looks better in
gmail.

I have a minor concern in that neither the comments on v2 nor the
changelog in v3 indicates the (useful) addition of the bit-value
lookup table for PcdTcg2PhysicalPresenceFlags in SecurityPkg.dec
and its associated dropping of the line
"# Default setting is TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT | TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT".

But if SecurityPkg maintainers are happy they haven't missed any other
changes in v3, I'm OK with this going in.

Regards,

Leif

> 
> Thanks
> Liming
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Patch List for 202005 stable tag
  2020-05-19 12:04 ` Leif Lindholm
@ 2020-05-19 17:17   ` Laszlo Ersek
  2020-05-20 14:43     ` Liming Gao
  0 siblings, 1 reply; 10+ messages in thread
From: Laszlo Ersek @ 2020-05-19 17:17 UTC (permalink / raw)
  To: Leif Lindholm, Gao, Liming
  Cc: Kinney, Michael D, afish@apple.com, devel@edk2.groups.io

On 05/19/20 14:04, Leif Lindholm wrote:
> On Tue, May 19, 2020 at 11:09:42 +0000, Gao, Liming wrote:
>> Hi Stewards and all:
>>   I collect current patch lists in devel mail list. Those patch contributors request to add them for 200205 stable tag. Because we have enter into Soft Feature Freeze, I want to collect your feedback for them. If any patches are missing, please reply this mail to add them.
>>
>> Feature List (pass review):
>> https://edk2.groups.io/g/devel/message/59578 [PATCH V7 0/1] Disable safe string constraint assertions
> 
> This one I might even be persuaded to wave through in hard
> freeze. Yes, please include.

Agreed.

>> https://edk2.groups.io/g/devel/message/59589 [PATCH V7 0/6] Add definitions introduced in UEFI 2.8a
> 
> 1,3-4 only adds definitions - I'm fine with those.
> 
> 2/6 looks out of place compared with the set description, but as it
> falls within the scope of "align codebase with current spec", and it
> is a correction, it can stay. (It should also not break anything, and
> if it does, that's automatically a bug isn't it?)

Patches v7 #1 through #4 should be merged, because they were posted
before the SFF, and they already carried an R-b from Liming (MdePkg
co-maintainer).

> 5-6, I have no comment on. Someone else will have to make that call.

Patches #5 and #6 seem to have been superseded by the following separate
series:

  [edk2-devel] [PATCH 0/2] Add FMP Capsule Image Header extension
  https://edk2.groups.io/g/devel/message/59652
  http://mid.mail-archive.com/20200515073848.13920-1-wei6.xu@intel.com

Now, this separate series was also posted (even if barely: 22 minutes)
before the SFF. The patches also contained some R-b tags upon posting.

Patch #1 had an R-b from Liming, upon posting. The files it modifies are:
- MdeModulePkg/Application/CapsuleApp/CapsuleDump.c
- MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c

I've run "BaseTools/Scripts/GetMaintainer.py" with the "-l" option on
both files. Both invocations list Liming as a designated reviewer. (From
section "MdeModulePkg: Firmware Update modules", as far as I can tell.)

Patch #2 had an R-b from Liming and Chao Zhang, on posting. The patch
modifies the following file:
- SignedCapsulePkg/Universal/RecoveryModuleLoadPei/RecoveryModuleLoadPei.c

"GetMaintainer.py" does list "Chao Zhang" with the "-l" option. (See the
"SignedCapsulePkg" section in Maintainers.txt".)

So, assuming both patches included those R-b's *justifiedly* when there
were posted (that is, the R-b's were given earlier on the list), I think
this 2-part series too should be merged. Technically speaking, it was
fully reviewed as soon as it was posted, and it was posted in time.

> 
>> Bug List (pass review):
>> https://edk2.groups.io/g/devel/message/59735 [PATCH v3] SecurityPkg: Change default value source
> 
> Patch is pretty garbled in groups.io. Thankfully, it looks better in
> gmail.
> 
> I have a minor concern in that neither the comments on v2 nor the
> changelog in v3 indicates the (useful) addition of the bit-value
> lookup table for PcdTcg2PhysicalPresenceFlags in SecurityPkg.dec
> and its associated dropping of the line
> "# Default setting is TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT | TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT".
> 
> But if SecurityPkg maintainers are happy they haven't missed any other
> changes in v3, I'm OK with this going in.

I think even a v4 would be eligible for merging (with the commit message
improvements that you are suggesting). Even during the hard feature
freeze. (Possibly justifying an extension to the hard feature freeze.)

Here's why I think that:
<https://bugzilla.tianocore.org/show_bug.cgi?id=2713#c0> is a good issue
report. It explains that this patch is a bugfix. Namely, commit
710174e01174 ("SecurityPkg: Tcg2PhysicalPresence: Define TCG2 PP Flags
Initial Pcd", 2016-12-29) was incomplete; it created an inconsistency in
physical presence flag defaults, between different code paths. And the
new patch fixes that.

(On the negative side, three versions of the patch have been posted to
the list, and the bugzilla ticket has pointers to... none of those. The
last comment remains, at this time, "Still investigate the solution to
fix it". I find that really frustrating and sad.)

Thanks,
Laszlo


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] Patch List for 202005 stable tag
  2020-05-19 11:09 Patch List for 202005 stable tag Liming Gao
  2020-05-19 12:04 ` Leif Lindholm
@ 2020-05-20  9:47 ` Maciej Rabeda
  2020-05-20 11:43   ` Philippe Mathieu-Daudé
                     ` (2 more replies)
  1 sibling, 3 replies; 10+ messages in thread
From: Maciej Rabeda @ 2020-05-20  9:47 UTC (permalink / raw)
  To: devel, liming.gao, Leif Lindholm, Laszlo Ersek, Kinney, Michael D,
	afish@apple.com

[-- Attachment #1: Type: text/plain, Size: 862 bytes --]

Hi Liming,

I would like to pull this in.
https://edk2.groups.io/g/devel/message/59947

Thanks,
Maciej

On 19-May-20 13:09, Liming Gao wrote:
>
> Hi Stewards and all:
>
>   I collect current patch lists in devel mail list. Those patch 
> contributors request to add them for 200205 stable tag. Because we 
> have enter into Soft Feature Freeze, I want to collect your feedback 
> for them. If any patches are missing, please reply this mail to add them.
>
> Feature List (pass review):
>
> https://edk2.groups.io/g/devel/message/59578 [PATCH V7 0/1] Disable 
> safe string constraint assertions
>
> https://edk2.groups.io/g/devel/message/59589 [PATCH V7 0/6] Add 
> definitions introduced in UEFI 2.8a
>
> Bug List (pass review):
>
> https://edk2.groups.io/g/devel/message/59735 [PATCH v3] SecurityPkg: 
> Change default value source
>
> Thanks
>
> Liming
>
> 


[-- Attachment #2: Type: text/html, Size: 5989 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] Patch List for 202005 stable tag
  2020-05-20  9:47 ` [edk2-devel] " Maciej Rabeda
@ 2020-05-20 11:43   ` Philippe Mathieu-Daudé
  2020-05-20 12:13   ` Chiu, Chasel
  2020-05-20 18:38   ` Laszlo Ersek
  2 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-05-20 11:43 UTC (permalink / raw)
  To: devel, maciej.rabeda, liming.gao, Leif Lindholm, Laszlo Ersek,
	Kinney, Michael D, afish@apple.com

On 5/20/20 11:47 AM, Maciej Rabeda wrote:
> Hi Liming,
> 
> I would like to pull this in.
> https://edk2.groups.io/g/devel/message/59947

This patch is indeed worth for the stable tag.

> 
> Thanks,
> Maciej
> 
> On 19-May-20 13:09, Liming Gao wrote:
>>
>> Hi Stewards and all:
>>
>>   I collect current patch lists in devel mail list. Those patch 
>> contributors request to add them for 200205 stable tag. Because we 
>> have enter into Soft Feature Freeze, I want to collect your feedback 
>> for them. If any patches are missing, please reply this mail to add them.
>>
>> ÂÂ
>>
>> Feature List (pass review):
>>
>> https://edk2.groups.io/g/devel/message/59578 [PATCH V7 0/1] Disable 
>> safe string constraint assertions
>>
>> https://edk2.groups.io/g/devel/message/59589 [PATCH V7 0/6] Add 
>> definitions introduced in UEFI 2.8a
>>
>> ÂÂ
>>
>> Bug List (pass review):
>>
>> https://edk2.groups.io/g/devel/message/59735 [PATCH v3] SecurityPkg: 
>> Change default value source
>>
>> ÂÂ
>>
>> Thanks
>>
>> Liming
>>
>> ÂÂ
>>
>> ÂÂ
>>
> 
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] Patch List for 202005 stable tag
  2020-05-20  9:47 ` [edk2-devel] " Maciej Rabeda
  2020-05-20 11:43   ` Philippe Mathieu-Daudé
@ 2020-05-20 12:13   ` Chiu, Chasel
  2020-05-20 18:43     ` Laszlo Ersek
  2020-05-20 18:38   ` Laszlo Ersek
  2 siblings, 1 reply; 10+ messages in thread
From: Chiu, Chasel @ 2020-05-20 12:13 UTC (permalink / raw)
  To: devel@edk2.groups.io, maciej.rabeda@linux.intel.com, Gao, Liming,
	Leif Lindholm, Laszlo Ersek, Kinney, Michael D, afish@apple.com

[-- Attachment #1: Type: text/plain, Size: 1471 bytes --]


Hi Liming,

I would like to include https://edk2.groups.io/g/devel/message/59921 to 202005 stable tag.
We missed something in previous FSP enhancement commit (https://edk2.groups.io/g/devel/message/59438) and this patch fixed the bug.

Thanks,
Chasel


From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Maciej Rabeda
Sent: Wednesday, May 20, 2020 5:47 PM
To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; Leif Lindholm <leif@nuviainc.com>; Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>; afish@apple.com
Subject: Re: [edk2-devel] Patch List for 202005 stable tag

Hi Liming,

I would like to pull this in.
https://edk2.groups.io/g/devel/message/59947

Thanks,
Maciej
On 19-May-20 13:09, Liming Gao wrote:
Hi Stewards and all:
  I collect current patch lists in devel mail list. Those patch contributors request to add them for 200205 stable tag. Because we have enter into Soft Feature Freeze, I want to collect your feedback for them. If any patches are missing, please reply this mail to add them.

Feature List (pass review):
https://edk2.groups.io/g/devel/message/59578 [PATCH V7 0/1] Disable safe string constraint assertions
https://edk2.groups.io/g/devel/message/59589 [PATCH V7 0/6] Add definitions introduced in UEFI 2.8a

Bug List (pass review):
https://edk2.groups.io/g/devel/message/59735 [PATCH v3] SecurityPkg: Change default value source

Thanks
Liming





[-- Attachment #2: Type: text/html, Size: 8633 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Patch List for 202005 stable tag
  2020-05-19 17:17   ` Laszlo Ersek
@ 2020-05-20 14:43     ` Liming Gao
  2020-05-20 18:48       ` Laszlo Ersek
  0 siblings, 1 reply; 10+ messages in thread
From: Liming Gao @ 2020-05-20 14:43 UTC (permalink / raw)
  To: Laszlo Ersek, Leif Lindholm
  Cc: Kinney, Michael D, afish@apple.com, devel@edk2.groups.io,
	Gao, Liming

Leif and Laszlo:
  I add my comments for previous three. Besides, I have got several changes for edk2 202005 stable tag. 

Bug List (pass review)
2. https://edk2.groups.io/g/devel/message/59921 [PATCH] IntelFsp2Pkg: Add FunctionParametePtr to FspGlobalData
[Chiu, Chasel] We missed something in previous FSP enhancement commit (https://edk2.groups.io/g/devel/message/59438) and this patch fixed the bug.

3. https://edk2.groups.io/g/devel/message/59914 [PATCH v2] NetworkPkg/DxeNetLib: Change the order of conditions in IF statement
[Liming] This is a bug. It has been merged in https://github.com/tianocore/edk2/pull/634 by Package maintainer.

4. https://edk2.groups.io/g/devel/message/59937 [PATCH] MdePkg: add definitions for ACPI NVDIMM Device Path
[Liming] This is one missing definition in MdePkg. I think it is fine to catch this stable tag.

5. https://edk2.groups.io/g/devel/message/56943 [PATCH v1 1/1] BaseTools: Remove deprecated Visual Studio Option 
[Liming] This is the change in VS2017 and VS2019 tool chain. 

Thanks
Liming
> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Wednesday, May 20, 2020 1:18 AM
> To: Leif Lindholm <leif@nuviainc.com>; Gao, Liming <liming.gao@intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; afish@apple.com; devel@edk2.groups.io
> Subject: Re: Patch List for 202005 stable tag
> 
> On 05/19/20 14:04, Leif Lindholm wrote:
> > On Tue, May 19, 2020 at 11:09:42 +0000, Gao, Liming wrote:
> >> Hi Stewards and all:
> >>   I collect current patch lists in devel mail list. Those patch contributors request to add them for 200205 stable tag. Because we
> have enter into Soft Feature Freeze, I want to collect your feedback for them. If any patches are missing, please reply this mail to add
> them.
> >>
> >> Feature List (pass review):
> >> https://edk2.groups.io/g/devel/message/59578 [PATCH V7 0/1] Disable safe string constraint assertions
> >
> > This one I might even be persuaded to wave through in hard
> > freeze. Yes, please include.
> 
> Agreed.
> 

> >> https://edk2.groups.io/g/devel/message/59589 [PATCH V7 0/6] Add definitions introduced in UEFI 2.8a
> >
> > 1,3-4 only adds definitions - I'm fine with those.
> >
> > 2/6 looks out of place compared with the set description, but as it
> > falls within the scope of "align codebase with current spec", and it
> > is a correction, it can stay. (It should also not break anything, and
> > if it does, that's automatically a bug isn't it?)
> 
> Patches v7 #1 through #4 should be merged, because they were posted
> before the SFF, and they already carried an R-b from Liming (MdePkg
> co-maintainer).
> 
> > 5-6, I have no comment on. Someone else will have to make that call.
> 
> Patches #5 and #6 seem to have been superseded by the following separate
> series:
> 
>   [edk2-devel] [PATCH 0/2] Add FMP Capsule Image Header extension
>   https://edk2.groups.io/g/devel/message/59652
>   http://mid.mail-archive.com/20200515073848.13920-1-wei6.xu@intel.com
> 
> Now, this separate series was also posted (even if barely: 22 minutes)
> before the SFF. The patches also contained some R-b tags upon posting.
> 
> Patch #1 had an R-b from Liming, upon posting. The files it modifies are:
> - MdeModulePkg/Application/CapsuleApp/CapsuleDump.c
> - MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
> 
> I've run "BaseTools/Scripts/GetMaintainer.py" with the "-l" option on
> both files. Both invocations list Liming as a designated reviewer. (From
> section "MdeModulePkg: Firmware Update modules", as far as I can tell.)
> 
> Patch #2 had an R-b from Liming and Chao Zhang, on posting. The patch
> modifies the following file:
> - SignedCapsulePkg/Universal/RecoveryModuleLoadPei/RecoveryModuleLoadPei.c
> 
> "GetMaintainer.py" does list "Chao Zhang" with the "-l" option. (See the
> "SignedCapsulePkg" section in Maintainers.txt".)
> 
> So, assuming both patches included those R-b's *justifiedly* when there
> were posted (that is, the R-b's were given earlier on the list), I think
> this 2-part series too should be merged. Technically speaking, it was
> fully reviewed as soon as it was posted, and it was posted in time.
> 
[Liming] This patch set have been merged in https://github.com/tianocore/edk2/pull/635. 

> >
> >> Bug List (pass review):
> >> https://edk2.groups.io/g/devel/message/59735 [PATCH v3] SecurityPkg: Change default value source
> >
> > Patch is pretty garbled in groups.io. Thankfully, it looks better in
> > gmail.
> >
> > I have a minor concern in that neither the comments on v2 nor the
> > changelog in v3 indicates the (useful) addition of the bit-value
> > lookup table for PcdTcg2PhysicalPresenceFlags in SecurityPkg.dec
> > and its associated dropping of the line
> > "# Default setting is TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT | TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT".
> >
> > But if SecurityPkg maintainers are happy they haven't missed any other
> > changes in v3, I'm OK with this going in.
> 
> I think even a v4 would be eligible for merging (with the commit message
> improvements that you are suggesting). Even during the hard feature
> freeze. (Possibly justifying an extension to the hard feature freeze.)

[Liming] I see SecurityPkg maintainters have given reviewed-by. So, I think it is fine to merge this change. 
> 
> Here's why I think that:
> <https://bugzilla.tianocore.org/show_bug.cgi?id=2713#c0> is a good issue
> report. It explains that this patch is a bugfix. Namely, commit
> 710174e01174 ("SecurityPkg: Tcg2PhysicalPresence: Define TCG2 PP Flags
> Initial Pcd", 2016-12-29) was incomplete; it created an inconsistency in
> physical presence flag defaults, between different code paths. And the
> new patch fixes that.
> 
> (On the negative side, three versions of the patch have been posted to
> the list, and the bugzilla ticket has pointers to... none of those. The
> last comment remains, at this time, "Still investigate the solution to
> fix it". I find that really frustrating and sad.)
> 
> Thanks,
> Laszlo


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] Patch List for 202005 stable tag
  2020-05-20  9:47 ` [edk2-devel] " Maciej Rabeda
  2020-05-20 11:43   ` Philippe Mathieu-Daudé
  2020-05-20 12:13   ` Chiu, Chasel
@ 2020-05-20 18:38   ` Laszlo Ersek
  2 siblings, 0 replies; 10+ messages in thread
From: Laszlo Ersek @ 2020-05-20 18:38 UTC (permalink / raw)
  To: Rabeda, Maciej, devel, liming.gao, Leif Lindholm,
	Kinney, Michael D, afish@apple.com

On 05/20/20 11:47, Rabeda, Maciej wrote:
> Hi Liming,
> 
> I would like to pull this in.
> https://edk2.groups.io/g/devel/message/59947

Already upstream at this time; commit d3733188a216
("NetworkPkg/DxeNetLib: Change the order of conditions in IF statement",
2020-05-20). That's good, it does look like a bugfix to me.

Thanks,
laszlo


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] Patch List for 202005 stable tag
  2020-05-20 12:13   ` Chiu, Chasel
@ 2020-05-20 18:43     ` Laszlo Ersek
  0 siblings, 0 replies; 10+ messages in thread
From: Laszlo Ersek @ 2020-05-20 18:43 UTC (permalink / raw)
  To: Chiu, Chasel, devel@edk2.groups.io, maciej.rabeda@linux.intel.com,
	Gao, Liming, Leif Lindholm, Kinney, Michael D, afish@apple.com

On 05/20/20 14:13, Chiu, Chasel wrote:
> 
> Hi Liming,
> 
> I would like to include https://edk2.groups.io/g/devel/message/59921 to 202005 stable tag.
> We missed something in previous FSP enhancement commit (https://edk2.groups.io/g/devel/message/59438) and this patch fixed the bug.

I guess it qualifies.

Thanks
Laszlo


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Patch List for 202005 stable tag
  2020-05-20 14:43     ` Liming Gao
@ 2020-05-20 18:48       ` Laszlo Ersek
  0 siblings, 0 replies; 10+ messages in thread
From: Laszlo Ersek @ 2020-05-20 18:48 UTC (permalink / raw)
  To: Gao, Liming, Leif Lindholm
  Cc: Kinney, Michael D, afish@apple.com, devel@edk2.groups.io

On 05/20/20 16:43, Gao, Liming wrote:
> Leif and Laszlo:
>   I add my comments for previous three. Besides, I have got several changes for edk2 202005 stable tag. 
> 
> Bug List (pass review)
> 2. https://edk2.groups.io/g/devel/message/59921 [PATCH] IntelFsp2Pkg: Add FunctionParametePtr to FspGlobalData
> [Chiu, Chasel] We missed something in previous FSP enhancement commit (https://edk2.groups.io/g/devel/message/59438) and this patch fixed the bug.

See my response in that thread ("I guess it qualifies").

> 
> 3. https://edk2.groups.io/g/devel/message/59914 [PATCH v2] NetworkPkg/DxeNetLib: Change the order of conditions in IF statement
> [Liming] This is a bug. It has been merged in https://github.com/tianocore/edk2/pull/634 by Package maintainer.
> 
> 4. https://edk2.groups.io/g/devel/message/59937 [PATCH] MdePkg: add definitions for ACPI NVDIMM Device Path
> [Liming] This is one missing definition in MdePkg. I think it is fine to catch this stable tag.

I agree; minimally for consistency with how we've handled the first few
patches of the series "[PATCH V7 0/6] Add definitions introduced in UEFI
2.8a" (which also only add definitions).

> 
> 5. https://edk2.groups.io/g/devel/message/56943 [PATCH v1 1/1] BaseTools: Remove deprecated Visual Studio Option 
> [Liming] This is the change in VS2017 and VS2019 tool chain. 

I agree this is a bugfix (and therefore it would qualify even during the
HFF).

Thanks,
Laszlo

> 
> Thanks
> Liming
>> -----Original Message-----
>> From: Laszlo Ersek <lersek@redhat.com>
>> Sent: Wednesday, May 20, 2020 1:18 AM
>> To: Leif Lindholm <leif@nuviainc.com>; Gao, Liming <liming.gao@intel.com>
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; afish@apple.com; devel@edk2.groups.io
>> Subject: Re: Patch List for 202005 stable tag
>>
>> On 05/19/20 14:04, Leif Lindholm wrote:
>>> On Tue, May 19, 2020 at 11:09:42 +0000, Gao, Liming wrote:
>>>> Hi Stewards and all:
>>>>   I collect current patch lists in devel mail list. Those patch contributors request to add them for 200205 stable tag. Because we
>> have enter into Soft Feature Freeze, I want to collect your feedback for them. If any patches are missing, please reply this mail to add
>> them.
>>>>
>>>> Feature List (pass review):
>>>> https://edk2.groups.io/g/devel/message/59578 [PATCH V7 0/1] Disable safe string constraint assertions
>>>
>>> This one I might even be persuaded to wave through in hard
>>> freeze. Yes, please include.
>>
>> Agreed.
>>
> 
>>>> https://edk2.groups.io/g/devel/message/59589 [PATCH V7 0/6] Add definitions introduced in UEFI 2.8a
>>>
>>> 1,3-4 only adds definitions - I'm fine with those.
>>>
>>> 2/6 looks out of place compared with the set description, but as it
>>> falls within the scope of "align codebase with current spec", and it
>>> is a correction, it can stay. (It should also not break anything, and
>>> if it does, that's automatically a bug isn't it?)
>>
>> Patches v7 #1 through #4 should be merged, because they were posted
>> before the SFF, and they already carried an R-b from Liming (MdePkg
>> co-maintainer).
>>
>>> 5-6, I have no comment on. Someone else will have to make that call.
>>
>> Patches #5 and #6 seem to have been superseded by the following separate
>> series:
>>
>>   [edk2-devel] [PATCH 0/2] Add FMP Capsule Image Header extension
>>   https://edk2.groups.io/g/devel/message/59652
>>   http://mid.mail-archive.com/20200515073848.13920-1-wei6.xu@intel.com
>>
>> Now, this separate series was also posted (even if barely: 22 minutes)
>> before the SFF. The patches also contained some R-b tags upon posting.
>>
>> Patch #1 had an R-b from Liming, upon posting. The files it modifies are:
>> - MdeModulePkg/Application/CapsuleApp/CapsuleDump.c
>> - MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
>>
>> I've run "BaseTools/Scripts/GetMaintainer.py" with the "-l" option on
>> both files. Both invocations list Liming as a designated reviewer. (From
>> section "MdeModulePkg: Firmware Update modules", as far as I can tell.)
>>
>> Patch #2 had an R-b from Liming and Chao Zhang, on posting. The patch
>> modifies the following file:
>> - SignedCapsulePkg/Universal/RecoveryModuleLoadPei/RecoveryModuleLoadPei.c
>>
>> "GetMaintainer.py" does list "Chao Zhang" with the "-l" option. (See the
>> "SignedCapsulePkg" section in Maintainers.txt".)
>>
>> So, assuming both patches included those R-b's *justifiedly* when there
>> were posted (that is, the R-b's were given earlier on the list), I think
>> this 2-part series too should be merged. Technically speaking, it was
>> fully reviewed as soon as it was posted, and it was posted in time.
>>
> [Liming] This patch set have been merged in https://github.com/tianocore/edk2/pull/635. 
> 
>>>
>>>> Bug List (pass review):
>>>> https://edk2.groups.io/g/devel/message/59735 [PATCH v3] SecurityPkg: Change default value source
>>>
>>> Patch is pretty garbled in groups.io. Thankfully, it looks better in
>>> gmail.
>>>
>>> I have a minor concern in that neither the comments on v2 nor the
>>> changelog in v3 indicates the (useful) addition of the bit-value
>>> lookup table for PcdTcg2PhysicalPresenceFlags in SecurityPkg.dec
>>> and its associated dropping of the line
>>> "# Default setting is TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT | TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT".
>>>
>>> But if SecurityPkg maintainers are happy they haven't missed any other
>>> changes in v3, I'm OK with this going in.
>>
>> I think even a v4 would be eligible for merging (with the commit message
>> improvements that you are suggesting). Even during the hard feature
>> freeze. (Possibly justifying an extension to the hard feature freeze.)
> 
> [Liming] I see SecurityPkg maintainters have given reviewed-by. So, I think it is fine to merge this change. 
>>
>> Here's why I think that:
>> <https://bugzilla.tianocore.org/show_bug.cgi?id=2713#c0> is a good issue
>> report. It explains that this patch is a bugfix. Namely, commit
>> 710174e01174 ("SecurityPkg: Tcg2PhysicalPresence: Define TCG2 PP Flags
>> Initial Pcd", 2016-12-29) was incomplete; it created an inconsistency in
>> physical presence flag defaults, between different code paths. And the
>> new patch fixes that.
>>
>> (On the negative side, three versions of the patch have been posted to
>> the list, and the bugzilla ticket has pointers to... none of those. The
>> last comment remains, at this time, "Still investigate the solution to
>> fix it". I find that really frustrating and sad.)
>>
>> Thanks,
>> Laszlo
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-05-20 18:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-19 11:09 Patch List for 202005 stable tag Liming Gao
2020-05-19 12:04 ` Leif Lindholm
2020-05-19 17:17   ` Laszlo Ersek
2020-05-20 14:43     ` Liming Gao
2020-05-20 18:48       ` Laszlo Ersek
2020-05-20  9:47 ` [edk2-devel] " Maciej Rabeda
2020-05-20 11:43   ` Philippe Mathieu-Daudé
2020-05-20 12:13   ` Chiu, Chasel
2020-05-20 18:43     ` Laszlo Ersek
2020-05-20 18:38   ` Laszlo Ersek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox