From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.120]) by mx.groups.io with SMTP id smtpd.web12.1803.1589908677854113008 for ; Tue, 19 May 2020 10:17:58 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=La6Fo0BJ; spf=pass (domain: redhat.com, ip: 207.211.31.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1589908677; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=S5GfHn71gViA6sOYUysEQGAm/7nqieD3lfK1uhk/vZU=; b=La6Fo0BJthxWGBfv+HRSenGgZa18vb55Rbqin4Zo+cV1wVzT76z2FlotKFJthP4kKqbjY5 IrRhwpMTKN47aa70naMFMQRhaA8UUei7rHa5I+FILv9ES5EUqrqQ/hnVX2iY3jPTv68zm9 7a++ZsFdfFCElekHTVzjPUYR+9x1gaQ= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-81-PFEiihKSMWeCS4aQ9-ZeCA-1; Tue, 19 May 2020 13:17:42 -0400 X-MC-Unique: PFEiihKSMWeCS4aQ9-ZeCA-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 245C1835B40; Tue, 19 May 2020 17:17:41 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-149.ams2.redhat.com [10.36.114.149]) by smtp.corp.redhat.com (Postfix) with ESMTP id B061D5D9C5; Tue, 19 May 2020 17:17:39 +0000 (UTC) Subject: Re: Patch List for 202005 stable tag To: Leif Lindholm , "Gao, Liming" Cc: "Kinney, Michael D" , "afish@apple.com" , "devel@edk2.groups.io" References: <20200519120415.GN10467@vanye> From: "Laszlo Ersek" Message-ID: <3e55d758-18f6-a811-b863-d8ba39b9e47f@redhat.com> Date: Tue, 19 May 2020 19:17:38 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200519120415.GN10467@vanye> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit 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: 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