Re: [edk2-devel] [edk2-staging][PATCH Kun,v1 0/7] Add TPM subclass definition

Re: [edk2-devel] [edk2-staging][PATCH Kun,v1 0/7] Add TPM subclass definition

[Michael D Kinney]

Hi Kun,

I would prefer to focus on the spec issue with this patch set and only
update DEC and .H file.

The idea of removing all the status code related PCDs and updating all the
source files to use the #defines is a different change.  And that change may
impact many downstream modules/libs that currently use the PCDs.  

Mike


> -----Original Message-----
> From: Kun Qin <kuqin12@gmail.com>
> Sent: Wednesday, July 13, 2022 5:39 PM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Zhang, Qi1 <qi1.zhang@intel.com>; Kumar, Rahul1
> <rahul1.kumar@intel.com>; Andrew Fish <afish@apple.com>; Leif Lindholm <leif@nuviainc.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
> Subject: Re: [edk2-devel] [edk2-staging][PATCH v1 0/7] Add TPM subclass definition
> 
> Hi Mike,
> 
> The intention was to encourage the usage of new definition in .H file
> once the definition
> is updated. The PCD is left as is only to serve as backwards
> compatibility purpose.
> 
> Do you think only updating the DEC and .H file is more ideal? I could
> drop the other changes
> if so desired.
> 
> Thanks,
> Kun
> 
> On 7/13/2022 5:30 PM, Michael D Kinney wrote:
> > Hi Kun,
> >
> > Why was the PCD usage not preserved and only the value updated in the DEC file and .H file?
> >
> > Mike
> >
> >> -----Original Message-----
> >> From: Kun Qin <kuqin12@gmail.com>
> >> Sent: Wednesday, July 13, 2022 5:21 PM
> >> To: devel@edk2.groups.io
> >> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Zhang, Qi1 <qi1.zhang@intel.com>; Kumar,
> Rahul1
> >> <rahul1.kumar@intel.com>; Andrew Fish <afish@apple.com>; Leif Lindholm <leif@nuviainc.com>; Kinney, Michael D
> >> <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
> >> Subject: Re: [edk2-devel] [edk2-staging][PATCH v1 0/7] Add TPM subclass definition
> >>
> >> Hi SecurityPkg maintainers & EDK2 stewards,
> >>
> >> I sent out this patch series intending to update/fix the PI spec through
> >> code first process.
> >>
> >> Could you please shed some light on it and let me know if any feedback?
> >>
> >> Thanks in advance!
> >>
> >> Regards,
> >> Kun
> >>
> >> On 7/5/2022 8:38 PM, Kun Qin via groups.io wrote:
> >>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3966
> >>>
> >>>   From PI Specification v1.7 Errata A, EFI_PERIPHERAL_DOCKING is defined as
> >>> 0xD0000 (as well as included in PiStatusCode.h).
> >>>
> >>> However, subclass employed as PCD for TPM peripheral in SecurityPkg is
> >>> also defined as 0xD0000. The TPM subclass code was used in TcgPei.c when
> >>> reporting error codes.
> >>>
> >>> The collision of subclass definition could cause the parsing of reported
> >>> errors being ambiguous.
> >>>
> >>> This patch series add EFI_PERIPHERAL_TPM as a spec-defined value and
> >>> removed potential usages in the SecurityPkg.
> >>>
> >>> Patch v1 branch: https://github.com/kuqin12/edk2/tree/BZ3966-add_tpm_subclass
> >>>
> >>> Cc: Jiewen Yao <jiewen.yao@intel.com>
> >>> Cc: Jian J Wang <jian.j.wang@intel.com>
> >>> Cc: Qi Zhang <qi1.zhang@intel.com>
> >>> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> >>> Cc: Andrew Fish <afish@apple.com>
> >>> Cc: Leif Lindholm <leif@nuviainc.com>
> >>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> >>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> >>> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> >>>
> >>> Kun Qin (7):
> >>>     EDK2 Code First: PI Specification: New peripheral subclass for TPM
> >>>     MdePkg: MmCommunication: Add TPM subclass definition to MdePkg
> >>>     SecurityPkg: Tcg2Dxe: Replace PcdStatusCodeSubClassTpmDevice
> >>>     SecurityPkg: Tcg2Pei: Replace PcdStatusCodeSubClassTpmDevice
> >>>     SecurityPkg: TcgDxe: Replace PcdStatusCodeSubClassTpmDevice
> >>>     SecurityPkg: TcgPei: Replace PcdStatusCodeSubClassTpmDevice
> >>>     SecurityPkg: SubClassTpm: Updated default value
> >>>
> >>>    SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c   |  4 +-
> >>>    SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c   |  4 +-
> >>>    SecurityPkg/Tcg/TcgDxe/TcgDxe.c     |  2 +-
> >>>    SecurityPkg/Tcg/TcgPei/TcgPei.c     |  4 +-
> >>>    CodeFirst/BZ3966-SpecChange.md      | 60 ++++++++++++++++++++
> >>>    MdePkg/Include/Pi/PiStatusCode.h    |  1 +
> >>>    SecurityPkg/SecurityPkg.dec         |  6 +-
> >>>    SecurityPkg/SecurityPkg.uni         |  2 +-
> >>>    SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf |  1 -
> >>>    SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf |  1 -
> >>>    SecurityPkg/Tcg/TcgDxe/TcgDxe.inf   |  1 -
> >>>    SecurityPkg/Tcg/TcgPei/TcgPei.inf   |  1 -
> >>>    12 files changed, 72 insertions(+), 15 deletions(-)
> >>>    create mode 100644 CodeFirst/BZ3966-SpecChange.md
> >>>
> >
> > 
> >
> >

Re: [edk2-devel] [edk2-staging][PATCH Kun,v1 0/7] Add TPM subclass definition

[Kun Qin]

Thanks, Mike. I will drop the applicable code change and send a v2 
series shortly.

Regards,
Kun

On 7/13/2022 5:47 PM, Michael D Kinney wrote:
> Hi Kun,
>
> I would prefer to focus on the spec issue with this patch set and only
> update DEC and .H file.
>
> The idea of removing all the status code related PCDs and updating all the
> source files to use the #defines is a different change.  And that change may
> impact many downstream modules/libs that currently use the PCDs.
>
> Mike
>
>
>> -----Original Message-----
>> From: Kun Qin <kuqin12@gmail.com>
>> Sent: Wednesday, July 13, 2022 5:39 PM
>> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
>> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Zhang, Qi1 <qi1.zhang@intel.com>; Kumar, Rahul1
>> <rahul1.kumar@intel.com>; Andrew Fish <afish@apple.com>; Leif Lindholm <leif@nuviainc.com>; Gao, Liming
>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
>> Subject: Re: [edk2-devel] [edk2-staging][PATCH v1 0/7] Add TPM subclass definition
>>
>> Hi Mike,
>>
>> The intention was to encourage the usage of new definition in .H file
>> once the definition
>> is updated. The PCD is left as is only to serve as backwards
>> compatibility purpose.
>>
>> Do you think only updating the DEC and .H file is more ideal? I could
>> drop the other changes
>> if so desired.
>>
>> Thanks,
>> Kun
>>
>> On 7/13/2022 5:30 PM, Michael D Kinney wrote:
>>> Hi Kun,
>>>
>>> Why was the PCD usage not preserved and only the value updated in the DEC file and .H file?
>>>
>>> Mike
>>>
>>>> -----Original Message-----
>>>> From: Kun Qin <kuqin12@gmail.com>
>>>> Sent: Wednesday, July 13, 2022 5:21 PM
>>>> To: devel@edk2.groups.io
>>>> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Zhang, Qi1 <qi1.zhang@intel.com>; Kumar,
>> Rahul1
>>>> <rahul1.kumar@intel.com>; Andrew Fish <afish@apple.com>; Leif Lindholm <leif@nuviainc.com>; Kinney, Michael D
>>>> <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
>>>> Subject: Re: [edk2-devel] [edk2-staging][PATCH v1 0/7] Add TPM subclass definition
>>>>
>>>> Hi SecurityPkg maintainers & EDK2 stewards,
>>>>
>>>> I sent out this patch series intending to update/fix the PI spec through
>>>> code first process.
>>>>
>>>> Could you please shed some light on it and let me know if any feedback?
>>>>
>>>> Thanks in advance!
>>>>
>>>> Regards,
>>>> Kun
>>>>
>>>> On 7/5/2022 8:38 PM, Kun Qin via groups.io wrote:
>>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3966
>>>>>
>>>>>    From PI Specification v1.7 Errata A, EFI_PERIPHERAL_DOCKING is defined as
>>>>> 0xD0000 (as well as included in PiStatusCode.h).
>>>>>
>>>>> However, subclass employed as PCD for TPM peripheral in SecurityPkg is
>>>>> also defined as 0xD0000. The TPM subclass code was used in TcgPei.c when
>>>>> reporting error codes.
>>>>>
>>>>> The collision of subclass definition could cause the parsing of reported
>>>>> errors being ambiguous.
>>>>>
>>>>> This patch series add EFI_PERIPHERAL_TPM as a spec-defined value and
>>>>> removed potential usages in the SecurityPkg.
>>>>>
>>>>> Patch v1 branch: https://github.com/kuqin12/edk2/tree/BZ3966-add_tpm_subclass
>>>>>
>>>>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>>>>> Cc: Jian J Wang <jian.j.wang@intel.com>
>>>>> Cc: Qi Zhang <qi1.zhang@intel.com>
>>>>> Cc: Rahul Kumar <rahul1.kumar@intel.com>
>>>>> Cc: Andrew Fish <afish@apple.com>
>>>>> Cc: Leif Lindholm <leif@nuviainc.com>
>>>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>>>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>>>>> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
>>>>>
>>>>> Kun Qin (7):
>>>>>      EDK2 Code First: PI Specification: New peripheral subclass for TPM
>>>>>      MdePkg: MmCommunication: Add TPM subclass definition to MdePkg
>>>>>      SecurityPkg: Tcg2Dxe: Replace PcdStatusCodeSubClassTpmDevice
>>>>>      SecurityPkg: Tcg2Pei: Replace PcdStatusCodeSubClassTpmDevice
>>>>>      SecurityPkg: TcgDxe: Replace PcdStatusCodeSubClassTpmDevice
>>>>>      SecurityPkg: TcgPei: Replace PcdStatusCodeSubClassTpmDevice
>>>>>      SecurityPkg: SubClassTpm: Updated default value
>>>>>
>>>>>     SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c   |  4 +-
>>>>>     SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c   |  4 +-
>>>>>     SecurityPkg/Tcg/TcgDxe/TcgDxe.c     |  2 +-
>>>>>     SecurityPkg/Tcg/TcgPei/TcgPei.c     |  4 +-
>>>>>     CodeFirst/BZ3966-SpecChange.md      | 60 ++++++++++++++++++++
>>>>>     MdePkg/Include/Pi/PiStatusCode.h    |  1 +
>>>>>     SecurityPkg/SecurityPkg.dec         |  6 +-
>>>>>     SecurityPkg/SecurityPkg.uni         |  2 +-
>>>>>     SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf |  1 -
>>>>>     SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf |  1 -
>>>>>     SecurityPkg/Tcg/TcgDxe/TcgDxe.inf   |  1 -
>>>>>     SecurityPkg/Tcg/TcgPei/TcgPei.inf   |  1 -
>>>>>     12 files changed, 72 insertions(+), 15 deletions(-)
>>>>>     create mode 100644 CodeFirst/BZ3966-SpecChange.md
>>>>>
>>>
>>>
>>>
>
> 
>
>

Re: [edk2-devel] [edk2-staging][PATCH Kun,v1 0/7] Add TPM subclass definition

[Kun Qin]

Hi SecurityPkg maintainers and EDK2 stewards,

The updated patch series was sent here: 
https://edk2.groups.io/g/devel/message/91330

Please let me know if there is any other feedback on them.

Thanks,
Kun

On 7/13/2022 6:19 PM, Kun Qin via groups.io wrote:
> Thanks, Mike. I will drop the applicable code change and send a v2 
> series shortly.
>
> Regards,
> Kun
>
> On 7/13/2022 5:47 PM, Michael D Kinney wrote:
>> Hi Kun,
>>
>> I would prefer to focus on the spec issue with this patch set and only
>> update DEC and .H file.
>>
>> The idea of removing all the status code related PCDs and updating 
>> all the
>> source files to use the #defines is a different change.  And that 
>> change may
>> impact many downstream modules/libs that currently use the PCDs.
>>
>> Mike
>>
>>
>>> -----Original Message-----
>>> From: Kun Qin <kuqin12@gmail.com>
>>> Sent: Wednesday, July 13, 2022 5:39 PM
>>> To: devel@edk2.groups.io; Kinney, Michael D 
>>> <michael.d.kinney@intel.com>
>>> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J 
>>> <jian.j.wang@intel.com>; Zhang, Qi1 <qi1.zhang@intel.com>; Kumar, 
>>> Rahul1
>>> <rahul1.kumar@intel.com>; Andrew Fish <afish@apple.com>; Leif 
>>> Lindholm <leif@nuviainc.com>; Gao, Liming
>>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
>>> Subject: Re: [edk2-devel] [edk2-staging][PATCH v1 0/7] Add TPM 
>>> subclass definition
>>>
>>> Hi Mike,
>>>
>>> The intention was to encourage the usage of new definition in .H file
>>> once the definition
>>> is updated. The PCD is left as is only to serve as backwards
>>> compatibility purpose.
>>>
>>> Do you think only updating the DEC and .H file is more ideal? I could
>>> drop the other changes
>>> if so desired.
>>>
>>> Thanks,
>>> Kun
>>>
>>> On 7/13/2022 5:30 PM, Michael D Kinney wrote:
>>>> Hi Kun,
>>>>
>>>> Why was the PCD usage not preserved and only the value updated in 
>>>> the DEC file and .H file?
>>>>
>>>> Mike
>>>>
>>>>> -----Original Message-----
>>>>> From: Kun Qin <kuqin12@gmail.com>
>>>>> Sent: Wednesday, July 13, 2022 5:21 PM
>>>>> To: devel@edk2.groups.io
>>>>> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J 
>>>>> <jian.j.wang@intel.com>; Zhang, Qi1 <qi1.zhang@intel.com>; Kumar,
>>> Rahul1
>>>>> <rahul1.kumar@intel.com>; Andrew Fish <afish@apple.com>; Leif 
>>>>> Lindholm <leif@nuviainc.com>; Kinney, Michael D
>>>>> <michael.d.kinney@intel.com>; Gao, Liming 
>>>>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
>>>>> Subject: Re: [edk2-devel] [edk2-staging][PATCH v1 0/7] Add TPM 
>>>>> subclass definition
>>>>>
>>>>> Hi SecurityPkg maintainers & EDK2 stewards,
>>>>>
>>>>> I sent out this patch series intending to update/fix the PI spec 
>>>>> through
>>>>> code first process.
>>>>>
>>>>> Could you please shed some light on it and let me know if any 
>>>>> feedback?
>>>>>
>>>>> Thanks in advance!
>>>>>
>>>>> Regards,
>>>>> Kun
>>>>>
>>>>> On 7/5/2022 8:38 PM, Kun Qin via groups.io wrote:
>>>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3966
>>>>>>
>>>>>>    From PI Specification v1.7 Errata A, EFI_PERIPHERAL_DOCKING is 
>>>>>> defined as
>>>>>> 0xD0000 (as well as included in PiStatusCode.h).
>>>>>>
>>>>>> However, subclass employed as PCD for TPM peripheral in 
>>>>>> SecurityPkg is
>>>>>> also defined as 0xD0000. The TPM subclass code was used in 
>>>>>> TcgPei.c when
>>>>>> reporting error codes.
>>>>>>
>>>>>> The collision of subclass definition could cause the parsing of 
>>>>>> reported
>>>>>> errors being ambiguous.
>>>>>>
>>>>>> This patch series add EFI_PERIPHERAL_TPM as a spec-defined value and
>>>>>> removed potential usages in the SecurityPkg.
>>>>>>
>>>>>> Patch v1 branch: 
>>>>>> https://github.com/kuqin12/edk2/tree/BZ3966-add_tpm_subclass
>>>>>>
>>>>>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>>>>>> Cc: Jian J Wang <jian.j.wang@intel.com>
>>>>>> Cc: Qi Zhang <qi1.zhang@intel.com>
>>>>>> Cc: Rahul Kumar <rahul1.kumar@intel.com>
>>>>>> Cc: Andrew Fish <afish@apple.com>
>>>>>> Cc: Leif Lindholm <leif@nuviainc.com>
>>>>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>>>>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>>>>>> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
>>>>>>
>>>>>> Kun Qin (7):
>>>>>>      EDK2 Code First: PI Specification: New peripheral subclass 
>>>>>> for TPM
>>>>>>      MdePkg: MmCommunication: Add TPM subclass definition to MdePkg
>>>>>>      SecurityPkg: Tcg2Dxe: Replace PcdStatusCodeSubClassTpmDevice
>>>>>>      SecurityPkg: Tcg2Pei: Replace PcdStatusCodeSubClassTpmDevice
>>>>>>      SecurityPkg: TcgDxe: Replace PcdStatusCodeSubClassTpmDevice
>>>>>>      SecurityPkg: TcgPei: Replace PcdStatusCodeSubClassTpmDevice
>>>>>>      SecurityPkg: SubClassTpm: Updated default value
>>>>>>
>>>>>>     SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c   |  4 +-
>>>>>>     SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c   |  4 +-
>>>>>>     SecurityPkg/Tcg/TcgDxe/TcgDxe.c     |  2 +-
>>>>>>     SecurityPkg/Tcg/TcgPei/TcgPei.c     |  4 +-
>>>>>>     CodeFirst/BZ3966-SpecChange.md      | 60 ++++++++++++++++++++
>>>>>>     MdePkg/Include/Pi/PiStatusCode.h    |  1 +
>>>>>>     SecurityPkg/SecurityPkg.dec         |  6 +-
>>>>>>     SecurityPkg/SecurityPkg.uni         |  2 +-
>>>>>>     SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf |  1 -
>>>>>>     SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf |  1 -
>>>>>>     SecurityPkg/Tcg/TcgDxe/TcgDxe.inf   |  1 -
>>>>>>     SecurityPkg/Tcg/TcgPei/TcgPei.inf   |  1 -
>>>>>>     12 files changed, 72 insertions(+), 15 deletions(-)
>>>>>>     create mode 100644 CodeFirst/BZ3966-SpecChange.md
>>>>>>
>>>>
>>>>
>>>>
>>
>>
>>
>>
>
>
> 
>
>