* [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change default value.
2019-05-08 3:01 [Patch v2 0/3] SecurityPkg/Opal: Change BlockSid policy Dong, Eric
@ 2019-05-08 3:01 ` Dong, Eric
2019-05-09 3:03 ` [edk2-devel] " Wu, Hao A
2019-05-09 9:53 ` Laszlo Ersek
2019-05-08 3:01 ` [Patch v2 2/3] SecurityPkg/OpalPassword: Change send BlockSID policy Dong, Eric
2019-05-08 3:01 ` [Patch v2 3/3] SecurityPkg/OpalPassword: Fix "Enable Feature" Menu disappear issue Dong, Eric
2 siblings, 2 replies; 12+ messages in thread
From: Dong, Eric @ 2019-05-08 3:01 UTC (permalink / raw)
To: devel; +Cc: Hao Wu
https://bugzilla.tianocore.org/show_bug.cgi?id=1782
Change BlockSID default policy, default enable BlockSid.
Signed-off-by: Eric Dong <eric.dong@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
---
SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h | 3 ++-
SecurityPkg/SecurityPkg.dec | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
index d9eee7f3e8..8da3deaf86 100644
--- a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
+++ b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
@@ -51,7 +51,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
// Default value
//
#define TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT (TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_ENABLE_BLOCK_SID | \
- TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_BLOCK_SID)
+ TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_BLOCK_SID |\
+ TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID)
/**
Check and execute the pending TPM request.
diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
index 6e4c4c3a02..3314f1854b 100644
--- a/SecurityPkg/SecurityPkg.dec
+++ b/SecurityPkg/SecurityPkg.dec
@@ -410,7 +410,7 @@
# PCD can be configured for different settings in different scenarios
# Default setting is TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT | TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
# @Prompt Initial setting of TCG2 Persistent Firmware Management Flags
- gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x300E2|UINT32|0x0001001B
+ gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x700E2|UINT32|0x0001001B
## Indicate current TPM2 Interrupt Number reported by _CRS control method.<BR><BR>
# TPM2 Interrupt feature is disabled If the pcd is set to 0.<BR>
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change default value.
2019-05-08 3:01 ` [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change default value Dong, Eric
@ 2019-05-09 3:03 ` Wu, Hao A
2019-05-09 11:16 ` Laszlo Ersek
2019-05-09 9:53 ` Laszlo Ersek
1 sibling, 1 reply; 12+ messages in thread
From: Wu, Hao A @ 2019-05-09 3:03 UTC (permalink / raw)
To: devel@edk2.groups.io, Dong, Eric
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Dong,
> Eric
> Sent: Wednesday, May 08, 2019 11:02 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A
> Subject: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change
> default value.
Just one minor comment, how about changing the title to:
SecurityPkg/SecurityPkg.dec: Change BlockSID default policy
Other than that, the patch is good to me:
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Best Regards,
Hao Wu
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=1782
>
> Change BlockSID default policy, default enable BlockSid.
>
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> ---
> SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h | 3 ++-
> SecurityPkg/SecurityPkg.dec | 2 +-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> index d9eee7f3e8..8da3deaf86 100644
> --- a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> +++ b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> @@ -51,7 +51,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> // Default value
> //
> #define TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
> (TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_ENABLE_BL
> OCK_SID | \
> -
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_BL
> OCK_SID)
> +
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_BL
> OCK_SID |\
> +
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID)
>
> /**
> Check and execute the pending TPM request.
> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
> index 6e4c4c3a02..3314f1854b 100644
> --- a/SecurityPkg/SecurityPkg.dec
> +++ b/SecurityPkg/SecurityPkg.dec
> @@ -410,7 +410,7 @@
> # PCD can be configured for different settings in different scenarios
> # Default setting is TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT |
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
> # @Prompt Initial setting of TCG2 Persistent Firmware Management Flags
> -
> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x300E2|UINT3
> 2|0x0001001B
> +
> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x700E2|UINT3
> 2|0x0001001B
>
> ## Indicate current TPM2 Interrupt Number reported by _CRS control
> method.<BR><BR>
> # TPM2 Interrupt feature is disabled If the pcd is set to 0.<BR>
> --
> 2.21.0.windows.1
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change default value.
2019-05-09 3:03 ` [edk2-devel] " Wu, Hao A
@ 2019-05-09 11:16 ` Laszlo Ersek
0 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2019-05-09 11:16 UTC (permalink / raw)
To: devel, hao.a.wu, Dong, Eric
On 05/09/19 05:03, Wu, Hao A wrote:
>> -----Original Message-----
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Dong,
>> Eric
>> Sent: Wednesday, May 08, 2019 11:02 AM
>> To: devel@edk2.groups.io
>> Cc: Wu, Hao A
>> Subject: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change
>> default value.
>
> Just one minor comment, how about changing the title to:
> SecurityPkg/SecurityPkg.dec: Change BlockSID default policy
That's an improvement too, thanks.
Laszlo
>
> Other than that, the patch is good to me:
> Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
>
> Best Regards,
> Hao Wu
>
>>
>> https://bugzilla.tianocore.org/show_bug.cgi?id=1782
>>
>> Change BlockSID default policy, default enable BlockSid.
>>
>> Signed-off-by: Eric Dong <eric.dong@intel.com>
>> Cc: Hao Wu <hao.a.wu@intel.com>
>> ---
>> SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h | 3 ++-
>> SecurityPkg/SecurityPkg.dec | 2 +-
>> 2 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
>> b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
>> index d9eee7f3e8..8da3deaf86 100644
>> --- a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
>> +++ b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
>> @@ -51,7 +51,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>> // Default value
>> //
>> #define TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
>> (TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_ENABLE_BL
>> OCK_SID | \
>> -
>> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_BL
>> OCK_SID)
>> +
>> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_BL
>> OCK_SID |\
>> +
>> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID)
>>
>> /**
>> Check and execute the pending TPM request.
>> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
>> index 6e4c4c3a02..3314f1854b 100644
>> --- a/SecurityPkg/SecurityPkg.dec
>> +++ b/SecurityPkg/SecurityPkg.dec
>> @@ -410,7 +410,7 @@
>> # PCD can be configured for different settings in different scenarios
>> # Default setting is TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT |
>> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
>> # @Prompt Initial setting of TCG2 Persistent Firmware Management Flags
>> -
>> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x300E2|UINT3
>> 2|0x0001001B
>> +
>> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x700E2|UINT3
>> 2|0x0001001B
>>
>> ## Indicate current TPM2 Interrupt Number reported by _CRS control
>> method.<BR><BR>
>> # TPM2 Interrupt feature is disabled If the pcd is set to 0.<BR>
>> --
>> 2.21.0.windows.1
>>
>>
>>
>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change default value.
2019-05-08 3:01 ` [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change default value Dong, Eric
2019-05-09 3:03 ` [edk2-devel] " Wu, Hao A
@ 2019-05-09 9:53 ` Laszlo Ersek
2019-05-09 12:41 ` Yao, Jiewen
1 sibling, 1 reply; 12+ messages in thread
From: Laszlo Ersek @ 2019-05-09 9:53 UTC (permalink / raw)
To: devel, eric.dong; +Cc: Hao Wu
On 05/08/19 05:01, Dong, Eric wrote:
> https://bugzilla.tianocore.org/show_bug.cgi?id=1782
>
> Change BlockSID default policy, default enable BlockSid.
>
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> ---
> SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h | 3 ++-
> SecurityPkg/SecurityPkg.dec | 2 +-
> 2 files changed, 3 insertions(+), 2 deletions(-)
Please change the subject line as follows:
----
SecurityPkg: enable BlockSID in PcdTcg2PhysicalPresenceFlags DEC default
----
no need to resubmit just for this, just pls update the patch before you
push it.
Thanks
Laszlo
> diff --git a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> index d9eee7f3e8..8da3deaf86 100644
> --- a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> +++ b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> @@ -51,7 +51,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> // Default value
> //
> #define TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT (TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_ENABLE_BLOCK_SID | \
> - TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_BLOCK_SID)
> + TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_BLOCK_SID |\
> + TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID)
>
> /**
> Check and execute the pending TPM request.
> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
> index 6e4c4c3a02..3314f1854b 100644
> --- a/SecurityPkg/SecurityPkg.dec
> +++ b/SecurityPkg/SecurityPkg.dec
> @@ -410,7 +410,7 @@
> # PCD can be configured for different settings in different scenarios
> # Default setting is TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT | TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
> # @Prompt Initial setting of TCG2 Persistent Firmware Management Flags
> - gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x300E2|UINT32|0x0001001B
> + gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x700E2|UINT32|0x0001001B
>
> ## Indicate current TPM2 Interrupt Number reported by _CRS control method.<BR><BR>
> # TPM2 Interrupt feature is disabled If the pcd is set to 0.<BR>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change default value.
2019-05-09 9:53 ` Laszlo Ersek
@ 2019-05-09 12:41 ` Yao, Jiewen
2019-05-09 21:26 ` Laszlo Ersek
0 siblings, 1 reply; 12+ messages in thread
From: Yao, Jiewen @ 2019-05-09 12:41 UTC (permalink / raw)
To: devel@edk2.groups.io, lersek@redhat.com, Dong, Eric; +Cc: Wu, Hao A
Hey
When I read https://bugzilla.tianocore.org/show_bug.cgi?id=1782, it says: "Current opal driver send blockSid command at ReadyToBoot event, it should been update to EndOfDxe point. Submit this bz to update the code."
But this patch is to update the default value.
I am very confused.
May I know what is the relationship between this patch and Bugzilla?
Why we need change the default value?
Thank you
Yao Jiewen
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Laszlo Ersek
> Sent: Thursday, May 9, 2019 2:53 AM
> To: devel@edk2.groups.io; Dong, Eric <eric.dong@intel.com>
> Cc: Wu, Hao A <hao.a.wu@intel.com>
> Subject: Re: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec:
> Change default value.
>
> On 05/08/19 05:01, Dong, Eric wrote:
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1782
> >
> > Change BlockSID default policy, default enable BlockSid.
> >
> > Signed-off-by: Eric Dong <eric.dong@intel.com>
> > Cc: Hao Wu <hao.a.wu@intel.com>
> > ---
> > SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h | 3 ++-
> > SecurityPkg/SecurityPkg.dec | 2 +-
> > 2 files changed, 3 insertions(+), 2 deletions(-)
>
> Please change the subject line as follows:
>
> ----
> SecurityPkg: enable BlockSID in PcdTcg2PhysicalPresenceFlags DEC default
> ----
>
> no need to resubmit just for this, just pls update the patch before you
> push it.
>
> Thanks
> Laszlo
>
> > diff --git a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> > index d9eee7f3e8..8da3deaf86 100644
> > --- a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> > +++ b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> > @@ -51,7 +51,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > // Default value
> > //
> > #define TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
> (TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_ENABLE
> _BLOCK_SID | \
> > -
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_
> BLOCK_SID)
> > +
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_
> BLOCK_SID |\
> > +
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID)
> >
> > /**
> > Check and execute the pending TPM request.
> > diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
> > index 6e4c4c3a02..3314f1854b 100644
> > --- a/SecurityPkg/SecurityPkg.dec
> > +++ b/SecurityPkg/SecurityPkg.dec
> > @@ -410,7 +410,7 @@
> > # PCD can be configured for different settings in different scenarios
> > # Default setting is TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT
> | TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
> > # @Prompt Initial setting of TCG2 Persistent Firmware Management
> Flags
> > -
> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x300E2|UI
> NT32|0x0001001B
> > +
> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x700E2|UI
> NT32|0x0001001B
> >
> > ## Indicate current TPM2 Interrupt Number reported by _CRS control
> method.<BR><BR>
> > # TPM2 Interrupt feature is disabled If the pcd is set to 0.<BR>
> >
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change default value.
2019-05-09 12:41 ` Yao, Jiewen
@ 2019-05-09 21:26 ` Laszlo Ersek
2019-05-09 21:48 ` Yao, Jiewen
0 siblings, 1 reply; 12+ messages in thread
From: Laszlo Ersek @ 2019-05-09 21:26 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io, Dong, Eric; +Cc: Wu, Hao A
Hi Jiewen,
On 05/09/19 14:41, Yao, Jiewen wrote:
> Hey
> When I read https://bugzilla.tianocore.org/show_bug.cgi?id=1782, it says: "Current opal driver send blockSid command at ReadyToBoot event, it should been update to EndOfDxe point. Submit this bz to update the code."
>
> But this patch is to update the default value.
>
> I am very confused.
>
> May I know what is the relationship between this patch and Bugzilla?
> Why we need change the default value?
I have absolutely no clue -- I only commented because I prefer a patch
subject to state *specifically* what a patch does. "Change default
value" was too vague. (When I commented I hadn't seen Hao Wu's similar
feedback just yet.)
Now, *why* this change is necessary, is totally over my head; I didn't
even begin to think about that.
Thanks
Laszlo
>
>
> Thank you
> Yao Jiewen
>
>
>> -----Original Message-----
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>> Laszlo Ersek
>> Sent: Thursday, May 9, 2019 2:53 AM
>> To: devel@edk2.groups.io; Dong, Eric <eric.dong@intel.com>
>> Cc: Wu, Hao A <hao.a.wu@intel.com>
>> Subject: Re: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec:
>> Change default value.
>>
>> On 05/08/19 05:01, Dong, Eric wrote:
>>> https://bugzilla.tianocore.org/show_bug.cgi?id=1782
>>>
>>> Change BlockSID default policy, default enable BlockSid.
>>>
>>> Signed-off-by: Eric Dong <eric.dong@intel.com>
>>> Cc: Hao Wu <hao.a.wu@intel.com>
>>> ---
>>> SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h | 3 ++-
>>> SecurityPkg/SecurityPkg.dec | 2 +-
>>> 2 files changed, 3 insertions(+), 2 deletions(-)
>>
>> Please change the subject line as follows:
>>
>> ----
>> SecurityPkg: enable BlockSID in PcdTcg2PhysicalPresenceFlags DEC default
>> ----
>>
>> no need to resubmit just for this, just pls update the patch before you
>> push it.
>>
>> Thanks
>> Laszlo
>>
>>> diff --git a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
>> b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
>>> index d9eee7f3e8..8da3deaf86 100644
>>> --- a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
>>> +++ b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
>>> @@ -51,7 +51,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>>> // Default value
>>> //
>>> #define TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
>> (TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_ENABLE
>> _BLOCK_SID | \
>>> -
>> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_
>> BLOCK_SID)
>>> +
>> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_
>> BLOCK_SID |\
>>> +
>> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID)
>>>
>>> /**
>>> Check and execute the pending TPM request.
>>> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
>>> index 6e4c4c3a02..3314f1854b 100644
>>> --- a/SecurityPkg/SecurityPkg.dec
>>> +++ b/SecurityPkg/SecurityPkg.dec
>>> @@ -410,7 +410,7 @@
>>> # PCD can be configured for different settings in different scenarios
>>> # Default setting is TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT
>> | TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
>>> # @Prompt Initial setting of TCG2 Persistent Firmware Management
>> Flags
>>> -
>> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x300E2|UI
>> NT32|0x0001001B
>>> +
>> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x700E2|UI
>> NT32|0x0001001B
>>>
>>> ## Indicate current TPM2 Interrupt Number reported by _CRS control
>> method.<BR><BR>
>>> # TPM2 Interrupt feature is disabled If the pcd is set to 0.<BR>
>>>
>>
>>
>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change default value.
2019-05-09 21:26 ` Laszlo Ersek
@ 2019-05-09 21:48 ` Yao, Jiewen
0 siblings, 0 replies; 12+ messages in thread
From: Yao, Jiewen @ 2019-05-09 21:48 UTC (permalink / raw)
To: Laszlo Ersek, devel@edk2.groups.io, Dong, Eric; +Cc: Wu, Hao A
Thanks Laszlo.
Sorry, I do not mean to ask you.
The question is for the original patch submitter.
I just reply the last email in my mail box.
I agree with you that the title should describe *what* code does.
At same time, I prefer to see the commit message on *why* we need do the change.
Thank you
Yao Jiewen
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, May 9, 2019 2:27 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Dong, Eric
> <eric.dong@intel.com>
> Cc: Wu, Hao A <hao.a.wu@intel.com>
> Subject: Re: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec:
> Change default value.
>
> Hi Jiewen,
>
> On 05/09/19 14:41, Yao, Jiewen wrote:
> > Hey
> > When I read https://bugzilla.tianocore.org/show_bug.cgi?id=1782, it says:
> "Current opal driver send blockSid command at ReadyToBoot event, it
> should been update to EndOfDxe point. Submit this bz to update the code."
> >
> > But this patch is to update the default value.
> >
> > I am very confused.
> >
> > May I know what is the relationship between this patch and Bugzilla?
> > Why we need change the default value?
>
> I have absolutely no clue -- I only commented because I prefer a patch
> subject to state *specifically* what a patch does. "Change default
> value" was too vague. (When I commented I hadn't seen Hao Wu's similar
> feedback just yet.)
>
> Now, *why* this change is necessary, is totally over my head; I didn't
> even begin to think about that.
>
> Thanks
> Laszlo
>
> >
> >
> > Thank you
> > Yao Jiewen
> >
> >
> >> -----Original Message-----
> >> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf
> Of
> >> Laszlo Ersek
> >> Sent: Thursday, May 9, 2019 2:53 AM
> >> To: devel@edk2.groups.io; Dong, Eric <eric.dong@intel.com>
> >> Cc: Wu, Hao A <hao.a.wu@intel.com>
> >> Subject: Re: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec:
> >> Change default value.
> >>
> >> On 05/08/19 05:01, Dong, Eric wrote:
> >>> https://bugzilla.tianocore.org/show_bug.cgi?id=1782
> >>>
> >>> Change BlockSID default policy, default enable BlockSid.
> >>>
> >>> Signed-off-by: Eric Dong <eric.dong@intel.com>
> >>> Cc: Hao Wu <hao.a.wu@intel.com>
> >>> ---
> >>> SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h | 3 ++-
> >>> SecurityPkg/SecurityPkg.dec | 2 +-
> >>> 2 files changed, 3 insertions(+), 2 deletions(-)
> >>
> >> Please change the subject line as follows:
> >>
> >> ----
> >> SecurityPkg: enable BlockSID in PcdTcg2PhysicalPresenceFlags DEC
> default
> >> ----
> >>
> >> no need to resubmit just for this, just pls update the patch before you
> >> push it.
> >>
> >> Thanks
> >> Laszlo
> >>
> >>> diff --git a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> >> b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> >>> index d9eee7f3e8..8da3deaf86 100644
> >>> --- a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> >>> +++ b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> >>> @@ -51,7 +51,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >>> // Default value
> >>> //
> >>> #define TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
> >>
> (TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_ENABLE
> >> _BLOCK_SID | \
> >>> -
> >>
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_
> >> BLOCK_SID)
> >>> +
> >>
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_
> >> BLOCK_SID |\
> >>> +
> >> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID)
> >>>
> >>> /**
> >>> Check and execute the pending TPM request.
> >>> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
> >>> index 6e4c4c3a02..3314f1854b 100644
> >>> --- a/SecurityPkg/SecurityPkg.dec
> >>> +++ b/SecurityPkg/SecurityPkg.dec
> >>> @@ -410,7 +410,7 @@
> >>> # PCD can be configured for different settings in different scenarios
> >>> # Default setting is
> TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT
> >> | TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
> >>> # @Prompt Initial setting of TCG2 Persistent Firmware Management
> >> Flags
> >>> -
> >>
> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x300E2|UI
> >> NT32|0x0001001B
> >>> +
> >>
> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x700E2|UI
> >> NT32|0x0001001B
> >>>
> >>> ## Indicate current TPM2 Interrupt Number reported by _CRS
> control
> >> method.<BR><BR>
> >>> # TPM2 Interrupt feature is disabled If the pcd is set to 0.<BR>
> >>>
> >>
> >>
> >>
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Patch v2 2/3] SecurityPkg/OpalPassword: Change send BlockSID policy.
2019-05-08 3:01 [Patch v2 0/3] SecurityPkg/Opal: Change BlockSid policy Dong, Eric
2019-05-08 3:01 ` [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change default value Dong, Eric
@ 2019-05-08 3:01 ` Dong, Eric
2019-05-09 3:03 ` [edk2-devel] " Wu, Hao A
2019-05-08 3:01 ` [Patch v2 3/3] SecurityPkg/OpalPassword: Fix "Enable Feature" Menu disappear issue Dong, Eric
2 siblings, 1 reply; 12+ messages in thread
From: Dong, Eric @ 2019-05-08 3:01 UTC (permalink / raw)
To: devel; +Cc: Hao Wu
https://bugzilla.tianocore.org/show_bug.cgi?id=1782
Change Send BlockSID command time from ReadyToBoot to
EndOfDxe.
Signed-off-by: Eric Dong <eric.dong@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
---
.../Tcg/Opal/OpalPassword/OpalDriver.c | 104 ++++++++----------
1 file changed, 46 insertions(+), 58 deletions(-)
diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
index 42999c89f0..009a97f66f 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
@@ -424,6 +424,47 @@ BuildOpalDeviceInfo (
FreePool (S3InitDevices);
}
+/**
+
+ Send BlockSid command if needed.
+
+**/
+VOID
+SendBlockSidCommand (
+ VOID
+ )
+{
+ OPAL_DRIVER_DEVICE *Itr;
+ TCG_RESULT Result;
+ OPAL_SESSION Session;
+ UINT32 PpStorageFlag;
+
+ PpStorageFlag = Tcg2PhysicalPresenceLibGetManagementFlags ();
+ if ((PpStorageFlag & TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID) != 0) {
+ //
+ // Send BlockSID command to each Opal disk
+ //
+ Itr = mOpalDriver.DeviceList;
+ while (Itr != NULL) {
+ if (Itr->OpalDisk.SupportedAttributes.BlockSid) {
+ ZeroMem(&Session, sizeof(Session));
+ Session.Sscp = Itr->OpalDisk.Sscp;
+ Session.MediaId = Itr->OpalDisk.MediaId;
+ Session.OpalBaseComId = Itr->OpalDisk.OpalBaseComId;
+
+ DEBUG ((DEBUG_INFO, "OpalPassword: EndOfDxe point, send BlockSid command to device!\n"));
+ Result = OpalBlockSid (&Session, TRUE); // HardwareReset must always be TRUE
+ if (Result != TcgResultSuccess) {
+ DEBUG ((DEBUG_ERROR, "OpalBlockSid fail\n"));
+ break;
+ }
+ }
+
+ Itr = Itr->Next;
+ }
+ }
+}
+
/**
Notification function of EFI_END_OF_DXE_EVENT_GROUP_GUID event group.
@@ -475,6 +516,11 @@ OpalEndOfDxeEventNotify (
TmpDev = TmpDev->Next;
}
+ //
+ // Send BlockSid command if needed.
+ //
+ SendBlockSidCommand ();
+
DEBUG ((DEBUG_INFO, "%a() - exit\n", __FUNCTION__));
gBS->CloseEvent (Event);
@@ -2262,53 +2308,6 @@ OpalDriverGetDeviceList(
return mOpalDriver.DeviceList;
}
-/**
- ReadyToBoot callback to send BlockSid command.
-
- @param Event Pointer to this event
- @param Context Event handler private Data
-
-**/
-VOID
-EFIAPI
-ReadyToBootCallback (
- IN EFI_EVENT Event,
- IN VOID *Context
- )
-{
- OPAL_DRIVER_DEVICE *Itr;
- TCG_RESULT Result;
- OPAL_SESSION Session;
- UINT32 PpStorageFlag;
-
- gBS->CloseEvent (Event);
-
- PpStorageFlag = Tcg2PhysicalPresenceLibGetManagementFlags ();
- if ((PpStorageFlag & TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID) != 0) {
- //
- // Send BlockSID command to each Opal disk
- //
- Itr = mOpalDriver.DeviceList;
- while (Itr != NULL) {
- if (Itr->OpalDisk.SupportedAttributes.BlockSid) {
- ZeroMem(&Session, sizeof(Session));
- Session.Sscp = Itr->OpalDisk.Sscp;
- Session.MediaId = Itr->OpalDisk.MediaId;
- Session.OpalBaseComId = Itr->OpalDisk.OpalBaseComId;
-
- DEBUG ((DEBUG_INFO, "OpalPassword: ReadyToBoot point, send BlockSid command to device!\n"));
- Result = OpalBlockSid (&Session, TRUE); // HardwareReset must always be TRUE
- if (Result != TcgResultSuccess) {
- DEBUG ((DEBUG_ERROR, "OpalBlockSid fail\n"));
- break;
- }
- }
-
- Itr = Itr->Next;
- }
- }
-}
-
/**
Stop this Controller.
@@ -2571,7 +2570,6 @@ EfiDriverEntryPoint(
)
{
EFI_STATUS Status;
- EFI_EVENT ReadyToBootEvent;
EFI_EVENT EndOfDxeEvent;
Status = EfiLibInstallDriverBindingComponentName2 (
@@ -2604,16 +2602,6 @@ EfiDriverEntryPoint(
);
ASSERT_EFI_ERROR (Status);
- //
- // register a ReadyToBoot event callback for sending BlockSid command
- //
- Status = EfiCreateEventReadyToBootEx (
- TPL_CALLBACK,
- ReadyToBootCallback,
- (VOID *) &ImageHandle,
- &ReadyToBootEvent
- );
-
//
// Install Hii packages.
//
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [edk2-devel] [Patch v2 2/3] SecurityPkg/OpalPassword: Change send BlockSID policy.
2019-05-08 3:01 ` [Patch v2 2/3] SecurityPkg/OpalPassword: Change send BlockSID policy Dong, Eric
@ 2019-05-09 3:03 ` Wu, Hao A
0 siblings, 0 replies; 12+ messages in thread
From: Wu, Hao A @ 2019-05-09 3:03 UTC (permalink / raw)
To: devel@edk2.groups.io, Dong, Eric
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Dong,
> Eric
> Sent: Wednesday, May 08, 2019 11:02 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A
> Subject: [edk2-devel] [Patch v2 2/3] SecurityPkg/OpalPassword: Change send
> BlockSID policy.
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=1782
>
> Change Send BlockSID command time from ReadyToBoot to
> EndOfDxe.
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Best Regards,
Hao Wu
>
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> ---
> .../Tcg/Opal/OpalPassword/OpalDriver.c | 104 ++++++++----------
> 1 file changed, 46 insertions(+), 58 deletions(-)
>
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> index 42999c89f0..009a97f66f 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> @@ -424,6 +424,47 @@ BuildOpalDeviceInfo (
> FreePool (S3InitDevices);
> }
>
> +/**
> +
> + Send BlockSid command if needed.
> +
> +**/
> +VOID
> +SendBlockSidCommand (
> + VOID
> + )
> +{
> + OPAL_DRIVER_DEVICE *Itr;
> + TCG_RESULT Result;
> + OPAL_SESSION Session;
> + UINT32 PpStorageFlag;
> +
> + PpStorageFlag = Tcg2PhysicalPresenceLibGetManagementFlags ();
> + if ((PpStorageFlag &
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID) != 0) {
> + //
> + // Send BlockSID command to each Opal disk
> + //
> + Itr = mOpalDriver.DeviceList;
> + while (Itr != NULL) {
> + if (Itr->OpalDisk.SupportedAttributes.BlockSid) {
> + ZeroMem(&Session, sizeof(Session));
> + Session.Sscp = Itr->OpalDisk.Sscp;
> + Session.MediaId = Itr->OpalDisk.MediaId;
> + Session.OpalBaseComId = Itr->OpalDisk.OpalBaseComId;
> +
> + DEBUG ((DEBUG_INFO, "OpalPassword: EndOfDxe point, send BlockSid
> command to device!\n"));
> + Result = OpalBlockSid (&Session, TRUE); // HardwareReset must always
> be TRUE
> + if (Result != TcgResultSuccess) {
> + DEBUG ((DEBUG_ERROR, "OpalBlockSid fail\n"));
> + break;
> + }
> + }
> +
> + Itr = Itr->Next;
> + }
> + }
> +}
> +
> /**
> Notification function of EFI_END_OF_DXE_EVENT_GROUP_GUID event group.
>
> @@ -475,6 +516,11 @@ OpalEndOfDxeEventNotify (
> TmpDev = TmpDev->Next;
> }
>
> + //
> + // Send BlockSid command if needed.
> + //
> + SendBlockSidCommand ();
> +
> DEBUG ((DEBUG_INFO, "%a() - exit\n", __FUNCTION__));
>
> gBS->CloseEvent (Event);
> @@ -2262,53 +2308,6 @@ OpalDriverGetDeviceList(
> return mOpalDriver.DeviceList;
> }
>
> -/**
> - ReadyToBoot callback to send BlockSid command.
> -
> - @param Event Pointer to this event
> - @param Context Event handler private Data
> -
> -**/
> -VOID
> -EFIAPI
> -ReadyToBootCallback (
> - IN EFI_EVENT Event,
> - IN VOID *Context
> - )
> -{
> - OPAL_DRIVER_DEVICE *Itr;
> - TCG_RESULT Result;
> - OPAL_SESSION Session;
> - UINT32 PpStorageFlag;
> -
> - gBS->CloseEvent (Event);
> -
> - PpStorageFlag = Tcg2PhysicalPresenceLibGetManagementFlags ();
> - if ((PpStorageFlag &
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID) != 0) {
> - //
> - // Send BlockSID command to each Opal disk
> - //
> - Itr = mOpalDriver.DeviceList;
> - while (Itr != NULL) {
> - if (Itr->OpalDisk.SupportedAttributes.BlockSid) {
> - ZeroMem(&Session, sizeof(Session));
> - Session.Sscp = Itr->OpalDisk.Sscp;
> - Session.MediaId = Itr->OpalDisk.MediaId;
> - Session.OpalBaseComId = Itr->OpalDisk.OpalBaseComId;
> -
> - DEBUG ((DEBUG_INFO, "OpalPassword: ReadyToBoot point, send BlockSid
> command to device!\n"));
> - Result = OpalBlockSid (&Session, TRUE); // HardwareReset must always
> be TRUE
> - if (Result != TcgResultSuccess) {
> - DEBUG ((DEBUG_ERROR, "OpalBlockSid fail\n"));
> - break;
> - }
> - }
> -
> - Itr = Itr->Next;
> - }
> - }
> -}
> -
> /**
> Stop this Controller.
>
> @@ -2571,7 +2570,6 @@ EfiDriverEntryPoint(
> )
> {
> EFI_STATUS Status;
> - EFI_EVENT ReadyToBootEvent;
> EFI_EVENT EndOfDxeEvent;
>
> Status = EfiLibInstallDriverBindingComponentName2 (
> @@ -2604,16 +2602,6 @@ EfiDriverEntryPoint(
> );
> ASSERT_EFI_ERROR (Status);
>
> - //
> - // register a ReadyToBoot event callback for sending BlockSid command
> - //
> - Status = EfiCreateEventReadyToBootEx (
> - TPL_CALLBACK,
> - ReadyToBootCallback,
> - (VOID *) &ImageHandle,
> - &ReadyToBootEvent
> - );
> -
> //
> // Install Hii packages.
> //
> --
> 2.21.0.windows.1
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Patch v2 3/3] SecurityPkg/OpalPassword: Fix "Enable Feature" Menu disappear issue.
2019-05-08 3:01 [Patch v2 0/3] SecurityPkg/Opal: Change BlockSid policy Dong, Eric
2019-05-08 3:01 ` [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change default value Dong, Eric
2019-05-08 3:01 ` [Patch v2 2/3] SecurityPkg/OpalPassword: Change send BlockSID policy Dong, Eric
@ 2019-05-08 3:01 ` Dong, Eric
2019-05-09 3:03 ` [edk2-devel] " Wu, Hao A
2 siblings, 1 reply; 12+ messages in thread
From: Dong, Eric @ 2019-05-08 3:01 UTC (permalink / raw)
To: devel; +Cc: Hao Wu
https://bugzilla.tianocore.org/show_bug.cgi?id=1782
After change behavior to send BlockSid command at EndOfDxe point,
check device ownership command will return un-authority error, it
finally caused opal driver can't show "Enable Feature" menu.
Update the code logic to send detect device ownership command
before send BlockSID command.
Signed-off-by: Eric Dong <eric.dong@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
---
.../Tcg/Opal/OpalPassword/OpalDriver.c | 11 +++++
.../Tcg/Opal/OpalPassword/OpalDriver.h | 1 +
SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c | 46 +++++++++++++++----
SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h | 15 ++++++
4 files changed, 63 insertions(+), 10 deletions(-)
diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
index 009a97f66f..965205c0b2 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
@@ -458,6 +458,11 @@ SendBlockSidCommand (
DEBUG ((DEBUG_ERROR, "OpalBlockSid fail\n"));
break;
}
+
+ //
+ // Record BlockSID command has been sent.
+ //
+ Itr->OpalDisk.SentBlockSID = TRUE;
}
Itr = Itr->Next;
@@ -2204,6 +2209,12 @@ ProcessOpalRequest (
ProcessOpalRequestEnableFeature (Dev, L"Enable Feature:");
}
+ //
+ // Update Device ownership.
+ // Later BlockSID command may block the update.
+ //
+ OpalDiskUpdateOwnerShip (&Dev->OpalDisk);
+
break;
}
diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
index a056e06106..beeabb1c0a 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
@@ -143,6 +143,7 @@ typedef struct {
UINT8 Password[OPAL_MAX_PASSWORD_SIZE];
UINT32 EstimateTimeCost;
+ BOOLEAN SentBlockSID; // Check whether BlockSid command has been sent.
} OPAL_DISK;
//
diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
index d0f3eda1e8..f101ca1c20 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
@@ -1215,6 +1215,40 @@ OpalDiskInitialize (
return OpalDiskUpdateStatus (&Dev->OpalDisk);
}
+/**
+ Update the device ownship
+
+ @param OpalDisk The Opal device.
+
+ @retval EFI_SUCESS Get ownership success.
+ @retval EFI_ACCESS_DENIED Has send BlockSID command, can't change ownership.
+ @retval EFI_INVALID_PARAMETER Not get Msid info before get ownership info.
+
+**/
+EFI_STATUS
+OpalDiskUpdateOwnerShip (
+ OPAL_DISK *OpalDisk
+ )
+{
+ OPAL_SESSION Session;
+
+ if (OpalDisk->MsidLength == 0) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (OpalDisk->SentBlockSID) {
+ return EFI_ACCESS_DENIED;
+ }
+
+ ZeroMem(&Session, sizeof(Session));
+ Session.Sscp = OpalDisk->Sscp;
+ Session.MediaId = OpalDisk->MediaId;
+ Session.OpalBaseComId = OpalDisk->OpalBaseComId;
+
+ OpalDisk->Owner = OpalUtilDetermineOwnership(&Session, OpalDisk->Msid, OpalDisk->MsidLength);
+ return EFI_SUCCESS;
+}
+
/**
Update the device info.
@@ -1223,6 +1257,7 @@ OpalDiskInitialize (
@retval EFI_SUCESS Initialize the device success.
@retval EFI_DEVICE_ERROR Get info from device failed.
@retval EFI_INVALID_PARAMETER Not get Msid info before get ownership info.
+ @retval EFI_ACCESS_DENIED Has send BlockSID command, can't change ownership.
**/
EFI_STATUS
@@ -1243,15 +1278,6 @@ OpalDiskUpdateStatus (
return EFI_DEVICE_ERROR;
}
- if (OpalDisk->MsidLength == 0) {
- return EFI_INVALID_PARAMETER;
- } else {
- //
- // Base on the Msid info to get the ownership, so Msid info must get first.
- //
- OpalDisk->Owner = OpalUtilDetermineOwnership(&Session, OpalDisk->Msid, OpalDisk->MsidLength);
- }
-
- return EFI_SUCCESS;
+ return OpalDiskUpdateOwnerShip (OpalDisk);
}
diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
index d3e236e2fe..89c709df99 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
@@ -357,4 +357,19 @@ OpalDiskInitialize (
IN OPAL_DRIVER_DEVICE *Dev
);
+/**
+ Update the device ownership
+
+ @param OpalDisk The Opal device.
+
+ @retval EFI_SUCESS Get ownership success.
+ @retval EFI_ACCESS_DENIED Has send BlockSID command, can't change ownership.
+ @retval EFI_INVALID_PARAMETER Not get Msid info before get ownership info.
+
+**/
+EFI_STATUS
+OpalDiskUpdateOwnerShip (
+ OPAL_DISK *OpalDisk
+ );
+
#endif // _HII_H_
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [edk2-devel] [Patch v2 3/3] SecurityPkg/OpalPassword: Fix "Enable Feature" Menu disappear issue.
2019-05-08 3:01 ` [Patch v2 3/3] SecurityPkg/OpalPassword: Fix "Enable Feature" Menu disappear issue Dong, Eric
@ 2019-05-09 3:03 ` Wu, Hao A
0 siblings, 0 replies; 12+ messages in thread
From: Wu, Hao A @ 2019-05-09 3:03 UTC (permalink / raw)
To: devel@edk2.groups.io, Dong, Eric
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Dong,
> Eric
> Sent: Wednesday, May 08, 2019 11:02 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A
> Subject: [edk2-devel] [Patch v2 3/3] SecurityPkg/OpalPassword: Fix "Enable
> Feature" Menu disappear issue.
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=1782
>
> After change behavior to send BlockSid command at EndOfDxe point,
> check device ownership command will return un-authority error, it
> finally caused opal driver can't show "Enable Feature" menu.
>
> Update the code logic to send detect device ownership command
> before send BlockSID command.
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Best Regards,
Hao Wu
>
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> ---
> .../Tcg/Opal/OpalPassword/OpalDriver.c | 11 +++++
> .../Tcg/Opal/OpalPassword/OpalDriver.h | 1 +
> SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c | 46 +++++++++++++++----
> SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h | 15 ++++++
> 4 files changed, 63 insertions(+), 10 deletions(-)
>
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> index 009a97f66f..965205c0b2 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> @@ -458,6 +458,11 @@ SendBlockSidCommand (
> DEBUG ((DEBUG_ERROR, "OpalBlockSid fail\n"));
> break;
> }
> +
> + //
> + // Record BlockSID command has been sent.
> + //
> + Itr->OpalDisk.SentBlockSID = TRUE;
> }
>
> Itr = Itr->Next;
> @@ -2204,6 +2209,12 @@ ProcessOpalRequest (
> ProcessOpalRequestEnableFeature (Dev, L"Enable Feature:");
> }
>
> + //
> + // Update Device ownership.
> + // Later BlockSID command may block the update.
> + //
> + OpalDiskUpdateOwnerShip (&Dev->OpalDisk);
> +
> break;
> }
>
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> index a056e06106..beeabb1c0a 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> @@ -143,6 +143,7 @@ typedef struct {
> UINT8 Password[OPAL_MAX_PASSWORD_SIZE];
>
> UINT32 EstimateTimeCost;
> + BOOLEAN SentBlockSID; // Check whether
> BlockSid command has been sent.
> } OPAL_DISK;
>
> //
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> index d0f3eda1e8..f101ca1c20 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> @@ -1215,6 +1215,40 @@ OpalDiskInitialize (
> return OpalDiskUpdateStatus (&Dev->OpalDisk);
> }
>
> +/**
> + Update the device ownship
> +
> + @param OpalDisk The Opal device.
> +
> + @retval EFI_SUCESS Get ownership success.
> + @retval EFI_ACCESS_DENIED Has send BlockSID command, can't change
> ownership.
> + @retval EFI_INVALID_PARAMETER Not get Msid info before get ownership
> info.
> +
> +**/
> +EFI_STATUS
> +OpalDiskUpdateOwnerShip (
> + OPAL_DISK *OpalDisk
> + )
> +{
>
> + OPAL_SESSION Session;
> +
> + if (OpalDisk->MsidLength == 0) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + if (OpalDisk->SentBlockSID) {
> + return EFI_ACCESS_DENIED;
> + }
> +
> + ZeroMem(&Session, sizeof(Session));
> + Session.Sscp = OpalDisk->Sscp;
> + Session.MediaId = OpalDisk->MediaId;
> + Session.OpalBaseComId = OpalDisk->OpalBaseComId;
> +
> + OpalDisk->Owner = OpalUtilDetermineOwnership(&Session, OpalDisk->Msid,
> OpalDisk->MsidLength);
>
> + return EFI_SUCCESS;
> +}
> +
> /**
> Update the device info.
>
> @@ -1223,6 +1257,7 @@ OpalDiskInitialize (
> @retval EFI_SUCESS Initialize the device success.
> @retval EFI_DEVICE_ERROR Get info from device failed.
> @retval EFI_INVALID_PARAMETER Not get Msid info before get ownership
> info.
> + @retval EFI_ACCESS_DENIED Has send BlockSID command, can't change
> ownership.
>
> **/
> EFI_STATUS
> @@ -1243,15 +1278,6 @@ OpalDiskUpdateStatus (
> return EFI_DEVICE_ERROR;
> }
>
> - if (OpalDisk->MsidLength == 0) {
> - return EFI_INVALID_PARAMETER;
> - } else {
> - //
> - // Base on the Msid info to get the ownership, so Msid info must get first.
> - //
> - OpalDisk->Owner = OpalUtilDetermineOwnership(&Session, OpalDisk-
> >Msid, OpalDisk->MsidLength);
> - }
> -
> - return EFI_SUCCESS;
> + return OpalDiskUpdateOwnerShip (OpalDisk);
> }
>
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
> index d3e236e2fe..89c709df99 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
> @@ -357,4 +357,19 @@ OpalDiskInitialize (
> IN OPAL_DRIVER_DEVICE *Dev
> );
>
> +/**
> + Update the device ownership
> +
> + @param OpalDisk The Opal device.
> +
> + @retval EFI_SUCESS Get ownership success.
> + @retval EFI_ACCESS_DENIED Has send BlockSID command, can't change
> ownership.
> + @retval EFI_INVALID_PARAMETER Not get Msid info before get ownership
> info.
> +
> +**/
> +EFI_STATUS
> +OpalDiskUpdateOwnerShip (
> + OPAL_DISK *OpalDisk
> + );
> +
> #endif // _HII_H_
> --
> 2.21.0.windows.1
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread