REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Yun Lou <yun.lou@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
Silicon/Intel/IntelSiliconPkg/Include/Protocol/PlatformDeviceSecurityPolicy.h | 84 ++++++++++++++++++++
1 file changed, 84 insertions(+)
diff --git a/Silicon/Intel/IntelSiliconPkg/Include/Protocol/PlatformDeviceSecurityPolicy.h b/Silicon/Intel/IntelSiliconPkg/Include/Protocol/PlatformDeviceSecurityPolicy.h
new file mode 100644
index 0000000000..cb5a71ad41
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Include/Protocol/PlatformDeviceSecurityPolicy.h
@@ -0,0 +1,84 @@
+/** @file
+ Device Security Policy Protocol definition
+
+ Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+
+#ifndef __EDKII_DEVICE_SECURITY_POLICY_PROTOCOL_H__
+#define __EDKII_DEVICE_SECURITY_POLICY_PROTOCOL_H__
+
+#include <Uefi.h>
+#include <Protocol/DeviceSecurity.h>
+
+typedef struct _EDKII_DEVICE_SECURITY_POLICY_PROTOCOL EDKII_DEVICE_SECURITY_POLICY_PROTOCOL;
+
+typedef struct {
+ UINT32 Version; // 0x1
+ UINT32 MeasurementPolicy;
+ UINT32 AuthenticationPolicy;
+} EDKII_DEVICE_SECURITY_POLICY;
+
+// BIT0 means if the action is needed or NOT
+#define EDKII_DEVICE_MEASUREMENT_POLICY_REQUIRED BIT0
+#define EDKII_DEVICE_AUTHENTICATION_POLICY_REQUIRED BIT0
+
+typedef struct {
+ UINT32 Version; // 0x1
+ UINT32 MeasurementState;
+ UINT32 AuthenticationState;
+} EDKII_DEVICE_SECURITY_STATE;
+
+// All zero means success
+#define EDKII_DEVICE_SECURITY_STATE_SUCCESS 0
+#define EDKII_DEVICE_SECURITY_STATE_ERROR BIT31
+#define EDKII_DEVICE_SECURITY_STATE_ERROR_UEFI_UNSUPPORTED (EDKII_DEVICE_SECURITY_STATE_ERROR + 0x0)
+#define EDKII_DEVICE_SECURITY_STATE_ERROR_UEFI_GET_POLICY_PROTOCOL (EDKII_DEVICE_SECURITY_STATE_ERROR + 0x1)
+#define EDKII_DEVICE_SECURITY_STATE_ERROR_PCI_NO_CAPABILITIES (EDKII_DEVICE_SECURITY_STATE_ERROR + 0x10)
+#define EDKII_DEVICE_SECURITY_STATE_ERROR_TCG_EXTEND_TPM_PCR (EDKII_DEVICE_SECURITY_STATE_ERROR + 0x20)
+
+/**
+ This function returns the device security policy associated with the device.
+
+ @param[in] This The protocol instance pointer.
+ @param[in] DeviceId The Identifier for the device.
+ @param[out] DeviceSecurityPolicy The Device Security Policy associated with the device.
+
+ @retval EFI_SUCCESS The device security policy is returned
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_DEVICE_SECURITY_GET_DEVICE_POLICY) (
+ IN EDKII_DEVICE_SECURITY_POLICY_PROTOCOL *This,
+ IN EDKII_DEVICE_IDENTIFIER *DeviceId,
+ OUT EDKII_DEVICE_SECURITY_POLICY **DeviceSecurityPolicy
+ );
+
+/**
+ This function sets the device state based upon the authentication result.
+
+ @param[in] This The protocol instance pointer.
+ @param[in] DeviceId The Identifier for the device.
+ @param[in] DeviceSecurityState The Device Security state associated with the device.
+
+ @retval EFI_SUCCESS The device state is set
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_DEVICE_SECURITY_SET_DEVICE_STATE) (
+ IN EDKII_DEVICE_SECURITY_POLICY_PROTOCOL *This,
+ IN EDKII_DEVICE_IDENTIFIER *DeviceId,
+ IN EDKII_DEVICE_SECURITY_STATE *DeviceSecurityState
+ );
+
+struct _EDKII_DEVICE_SECURITY_POLICY_PROTOCOL {
+ UINT32 Version; // 0x1
+ EDKII_DEVICE_SECURITY_GET_DEVICE_POLICY GetDevicePolicy;
+ EDKII_DEVICE_SECURITY_SET_DEVICE_STATE SetDeviceState;
+};
+
+extern EFI_GUID gEdkiiDeviceSecurityPolicyProtocolGuid;
+
+#endif
--
2.19.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#49770): https://edk2.groups.io/g/devel/message/49770
Mute This Topic: https://groups.io/mt/40117802/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Same feedback as provided in patch1/6.
In addition, is there a reason to add "EDKII" as prefix in the internal data structure names? E.g.
+typedef struct {
+ UINT32 Version; // 0x1
+ UINT32 MeasurementPolicy;
+ UINT32 AuthenticationPolicy;
+} EDKII_DEVICE_SECURITY_POLICY; // this can be named simply as "DEVICE_SECURITY_POLICY" right?
Also, on the services "GetDevicePolicy" and "SetDeviceState", is it possible to have any other return states than EFI_SUCCESS?
Regards,
Sai
-----Original Message-----
From: Yao, Jiewen
Sent: Thursday, October 31, 2019 5:31 AM
To: devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Lou, Yun <yun.lou@intel.com>
Subject: [PATCH V2 2/6] IntelSiliconPkg/Include: Add Platform Device Security Policy protocol
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Yun Lou <yun.lou@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
Silicon/Intel/IntelSiliconPkg/Include/Protocol/PlatformDeviceSecurityPolicy.h | 84 ++++++++++++++++++++
1 file changed, 84 insertions(+)
diff --git a/Silicon/Intel/IntelSiliconPkg/Include/Protocol/PlatformDeviceSecurityPolicy.h b/Silicon/Intel/IntelSiliconPkg/Include/Protocol/PlatformDeviceSecurityPolicy.h
new file mode 100644
index 0000000000..cb5a71ad41
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Include/Protocol/PlatformDeviceSecurityPolicy.h
@@ -0,0 +1,84 @@
+/** @file
+ Device Security Policy Protocol definition
+
+ Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+
+#ifndef __EDKII_DEVICE_SECURITY_POLICY_PROTOCOL_H__
+#define __EDKII_DEVICE_SECURITY_POLICY_PROTOCOL_H__
+
+#include <Uefi.h>
+#include <Protocol/DeviceSecurity.h>
+
+typedef struct _EDKII_DEVICE_SECURITY_POLICY_PROTOCOL EDKII_DEVICE_SECURITY_POLICY_PROTOCOL;
+
+typedef struct {
+ UINT32 Version; // 0x1
+ UINT32 MeasurementPolicy;
+ UINT32 AuthenticationPolicy;
+} EDKII_DEVICE_SECURITY_POLICY;
+
+// BIT0 means if the action is needed or NOT
+#define EDKII_DEVICE_MEASUREMENT_POLICY_REQUIRED BIT0
+#define EDKII_DEVICE_AUTHENTICATION_POLICY_REQUIRED BIT0
+
+typedef struct {
+ UINT32 Version; // 0x1
+ UINT32 MeasurementState;
+ UINT32 AuthenticationState;
+} EDKII_DEVICE_SECURITY_STATE;
+
+// All zero means success
+#define EDKII_DEVICE_SECURITY_STATE_SUCCESS 0
+#define EDKII_DEVICE_SECURITY_STATE_ERROR BIT31
+#define EDKII_DEVICE_SECURITY_STATE_ERROR_UEFI_UNSUPPORTED (EDKII_DEVICE_SECURITY_STATE_ERROR + 0x0)
+#define EDKII_DEVICE_SECURITY_STATE_ERROR_UEFI_GET_POLICY_PROTOCOL (EDKII_DEVICE_SECURITY_STATE_ERROR + 0x1)
+#define EDKII_DEVICE_SECURITY_STATE_ERROR_PCI_NO_CAPABILITIES (EDKII_DEVICE_SECURITY_STATE_ERROR + 0x10)
+#define EDKII_DEVICE_SECURITY_STATE_ERROR_TCG_EXTEND_TPM_PCR (EDKII_DEVICE_SECURITY_STATE_ERROR + 0x20)
+
+/**
+ This function returns the device security policy associated with the device.
+
+ @param[in] This The protocol instance pointer.
+ @param[in] DeviceId The Identifier for the device.
+ @param[out] DeviceSecurityPolicy The Device Security Policy associated with the device.
+
+ @retval EFI_SUCCESS The device security policy is returned
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_DEVICE_SECURITY_GET_DEVICE_POLICY) (
+ IN EDKII_DEVICE_SECURITY_POLICY_PROTOCOL *This,
+ IN EDKII_DEVICE_IDENTIFIER *DeviceId,
+ OUT EDKII_DEVICE_SECURITY_POLICY **DeviceSecurityPolicy
+ );
+
+/**
+ This function sets the device state based upon the authentication result.
+
+ @param[in] This The protocol instance pointer.
+ @param[in] DeviceId The Identifier for the device.
+ @param[in] DeviceSecurityState The Device Security state associated with the device.
+
+ @retval EFI_SUCCESS The device state is set
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_DEVICE_SECURITY_SET_DEVICE_STATE) (
+ IN EDKII_DEVICE_SECURITY_POLICY_PROTOCOL *This,
+ IN EDKII_DEVICE_IDENTIFIER *DeviceId,
+ IN EDKII_DEVICE_SECURITY_STATE *DeviceSecurityState
+ );
+
+struct _EDKII_DEVICE_SECURITY_POLICY_PROTOCOL {
+ UINT32 Version; // 0x1
+ EDKII_DEVICE_SECURITY_GET_DEVICE_POLICY GetDevicePolicy;
+ EDKII_DEVICE_SECURITY_SET_DEVICE_STATE SetDeviceState;
+};
+
+extern EFI_GUID gEdkiiDeviceSecurityPolicyProtocolGuid;
+
+#endif
--
2.19.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#50073): https://edk2.groups.io/g/devel/message/50073
Mute This Topic: https://groups.io/mt/40117802/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Thanks. Comment below:
> -----Original Message-----
> From: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>
> Sent: Thursday, November 7, 2019 5:51 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Lou, Yun <yun.lou@intel.com>
> Subject: RE: [PATCH V2 2/6] IntelSiliconPkg/Include: Add Platform Device
> Security Policy protocol
>
> Same feedback as provided in patch1/6.
[Jiewen] Agree. See response in previous email.
I will add structure description in V3.
> In addition, is there a reason to add "EDKII" as prefix in the internal data
> structure names? E.g.
> +typedef struct {
> + UINT32 Version; // 0x1
> + UINT32 MeasurementPolicy;
> + UINT32 AuthenticationPolicy;
> +} EDKII_DEVICE_SECURITY_POLICY; // this can be named simply as
> "DEVICE_SECURITY_POLICY" right?
[Jiewen] I believe it is common practice to add EDKII_ prefix to all data structure.
I did a search in MdeModulePkg.
I do see some EDKII protocol adds EDKII_ prefix for the data structure.
But some of them does not.
I feel better if we add it to avoid namespace confusing.
I am open for discussion.
>
> Also, on the services "GetDevicePolicy" and "SetDeviceState", is it possible to
> have any other return states than EFI_SUCCESS?
[Jiewen] Good question. I can add something in V3.
>
> Regards,
> Sai
>
>
> -----Original Message-----
> From: Yao, Jiewen
> Sent: Thursday, October 31, 2019 5:31 AM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Lou, Yun <yun.lou@intel.com>
> Subject: [PATCH V2 2/6] IntelSiliconPkg/Include: Add Platform Device Security
> Policy protocol
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Yun Lou <yun.lou@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
> Silicon/Intel/IntelSiliconPkg/Include/Protocol/PlatformDeviceSecurityPolicy.h |
> 84 ++++++++++++++++++++
> 1 file changed, 84 insertions(+)
>
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Include/Protocol/PlatformDeviceSecurityPolicy.h
> b/Silicon/Intel/IntelSiliconPkg/Include/Protocol/PlatformDeviceSecurityPolicy.h
> new file mode 100644
> index 0000000000..cb5a71ad41
> --- /dev/null
> +++
> b/Silicon/Intel/IntelSiliconPkg/Include/Protocol/PlatformDeviceSecurityPolicy.h
> @@ -0,0 +1,84 @@
> +/** @file
> + Device Security Policy Protocol definition
> +
> + Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#ifndef __EDKII_DEVICE_SECURITY_POLICY_PROTOCOL_H__
> +#define __EDKII_DEVICE_SECURITY_POLICY_PROTOCOL_H__
> +
> +#include <Uefi.h>
> +#include <Protocol/DeviceSecurity.h>
> +
> +typedef struct _EDKII_DEVICE_SECURITY_POLICY_PROTOCOL
> EDKII_DEVICE_SECURITY_POLICY_PROTOCOL;
> +
> +typedef struct {
> + UINT32 Version; // 0x1
> + UINT32 MeasurementPolicy;
> + UINT32 AuthenticationPolicy;
> +} EDKII_DEVICE_SECURITY_POLICY;
> +
> +// BIT0 means if the action is needed or NOT
> +#define EDKII_DEVICE_MEASUREMENT_POLICY_REQUIRED BIT0
> +#define EDKII_DEVICE_AUTHENTICATION_POLICY_REQUIRED BIT0
> +
> +typedef struct {
> + UINT32 Version; // 0x1
> + UINT32 MeasurementState;
> + UINT32 AuthenticationState;
> +} EDKII_DEVICE_SECURITY_STATE;
> +
> +// All zero means success
> +#define EDKII_DEVICE_SECURITY_STATE_SUCCESS 0
> +#define EDKII_DEVICE_SECURITY_STATE_ERROR BIT31
> +#define EDKII_DEVICE_SECURITY_STATE_ERROR_UEFI_UNSUPPORTED
> (EDKII_DEVICE_SECURITY_STATE_ERROR + 0x0)
> +#define
> EDKII_DEVICE_SECURITY_STATE_ERROR_UEFI_GET_POLICY_PROTOCOL
> (EDKII_DEVICE_SECURITY_STATE_ERROR + 0x1)
> +#define EDKII_DEVICE_SECURITY_STATE_ERROR_PCI_NO_CAPABILITIES
> (EDKII_DEVICE_SECURITY_STATE_ERROR + 0x10)
> +#define EDKII_DEVICE_SECURITY_STATE_ERROR_TCG_EXTEND_TPM_PCR
> (EDKII_DEVICE_SECURITY_STATE_ERROR + 0x20)
> +
> +/**
> + This function returns the device security policy associated with the device.
> +
> + @param[in] This The protocol instance pointer.
> + @param[in] DeviceId The Identifier for the device.
> + @param[out] DeviceSecurityPolicy The Device Security Policy associated
> with the device.
> +
> + @retval EFI_SUCCESS The device security policy is returned
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EDKII_DEVICE_SECURITY_GET_DEVICE_POLICY) (
> + IN EDKII_DEVICE_SECURITY_POLICY_PROTOCOL *This,
> + IN EDKII_DEVICE_IDENTIFIER *DeviceId,
> + OUT EDKII_DEVICE_SECURITY_POLICY **DeviceSecurityPolicy
> + );
> +
> +/**
> + This function sets the device state based upon the authentication result.
> +
> + @param[in] This The protocol instance pointer.
> + @param[in] DeviceId The Identifier for the device.
> + @param[in] DeviceSecurityState The Device Security state associated with
> the device.
> +
> + @retval EFI_SUCCESS The device state is set
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EDKII_DEVICE_SECURITY_SET_DEVICE_STATE) (
> + IN EDKII_DEVICE_SECURITY_POLICY_PROTOCOL *This,
> + IN EDKII_DEVICE_IDENTIFIER *DeviceId,
> + IN EDKII_DEVICE_SECURITY_STATE *DeviceSecurityState
> + );
> +
> +struct _EDKII_DEVICE_SECURITY_POLICY_PROTOCOL {
> + UINT32 Version; // 0x1
> + EDKII_DEVICE_SECURITY_GET_DEVICE_POLICY GetDevicePolicy;
> + EDKII_DEVICE_SECURITY_SET_DEVICE_STATE SetDeviceState;
> +};
> +
> +extern EFI_GUID gEdkiiDeviceSecurityPolicyProtocolGuid;
> +
> +#endif
> --
> 2.19.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#50159): https://edk2.groups.io/g/devel/message/50159
Mute This Topic: https://groups.io/mt/40117802/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Jiewen,
I am fine with the 1/6 patch that doesn't contain enough comments to describe the meaning
of each field in each structure because I can reach out to the referenced spec to understand them.
This 2/6 patch introduces a new protocol but contains very few comments (almost none) for each
structure each field and I cannot reach out to any spec to understand them.
So can you please add comments to each field of structures like EDKII_DEVICE_SECURITY_POLICY and
EDKII_DEVICE_SECURITY_STATE?
Also can you please add comments to all the macros defined in this patch to explain the meaning and
more important how they are going to impact the logic.
In general, can you please add enough comments so that a PCI/USB BUS driver developer can understand
the whole flow how this protocol supports the device security?
Thanks,
Ray
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Thursday, October 31, 2019 8:31 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Lou, Yun <yun.lou@intel.com>
> Subject: [PATCH V2 2/6] IntelSiliconPkg/Include: Add Platform Device
> Security Policy protocol
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Yun Lou <yun.lou@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
>
> Silicon/Intel/IntelSiliconPkg/Include/Protocol/PlatformDeviceSecurityPolicy.h
> | 84 ++++++++++++++++++++
> 1 file changed, 84 insertions(+)
>
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Include/Protocol/PlatformDeviceSecurityPolic
> y.h
> b/Silicon/Intel/IntelSiliconPkg/Include/Protocol/PlatformDeviceSecurityPolic
> y.h
> new file mode 100644
> index 0000000000..cb5a71ad41
> --- /dev/null
> +++
> b/Silicon/Intel/IntelSiliconPkg/Include/Protocol/PlatformDeviceSecurityPolic
> y.h
> @@ -0,0 +1,84 @@
> +/** @file
> + Device Security Policy Protocol definition
> +
> + Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +
> +#ifndef __EDKII_DEVICE_SECURITY_POLICY_PROTOCOL_H__
> +#define __EDKII_DEVICE_SECURITY_POLICY_PROTOCOL_H__
> +
> +#include <Uefi.h>
> +#include <Protocol/DeviceSecurity.h>
> +
> +typedef struct _EDKII_DEVICE_SECURITY_POLICY_PROTOCOL
> EDKII_DEVICE_SECURITY_POLICY_PROTOCOL;
> +
> +typedef struct {
> + UINT32 Version; // 0x1
> + UINT32 MeasurementPolicy;
> + UINT32 AuthenticationPolicy;
> +} EDKII_DEVICE_SECURITY_POLICY;
> +
> +// BIT0 means if the action is needed or NOT
> +#define EDKII_DEVICE_MEASUREMENT_POLICY_REQUIRED BIT0
> +#define EDKII_DEVICE_AUTHENTICATION_POLICY_REQUIRED BIT0
> +
> +typedef struct {
> + UINT32 Version; // 0x1
> + UINT32 MeasurementState;
> + UINT32 AuthenticationState;
> +} EDKII_DEVICE_SECURITY_STATE;
> +
> +// All zero means success
> +#define EDKII_DEVICE_SECURITY_STATE_SUCCESS 0
> +#define EDKII_DEVICE_SECURITY_STATE_ERROR BIT31
> +#define EDKII_DEVICE_SECURITY_STATE_ERROR_UEFI_UNSUPPORTED
> (EDKII_DEVICE_SECURITY_STATE_ERROR + 0x0)
> +#define
> EDKII_DEVICE_SECURITY_STATE_ERROR_UEFI_GET_POLICY_PROTOCOL
> (EDKII_DEVICE_SECURITY_STATE_ERROR + 0x1)
> +#define EDKII_DEVICE_SECURITY_STATE_ERROR_PCI_NO_CAPABILITIES
> (EDKII_DEVICE_SECURITY_STATE_ERROR + 0x10)
> +#define EDKII_DEVICE_SECURITY_STATE_ERROR_TCG_EXTEND_TPM_PCR
> (EDKII_DEVICE_SECURITY_STATE_ERROR + 0x20)
> +
> +/**
> + This function returns the device security policy associated with the device.
> +
> + @param[in] This The protocol instance pointer.
> + @param[in] DeviceId The Identifier for the device.
> + @param[out] DeviceSecurityPolicy The Device Security Policy associated
> with the device.
> +
> + @retval EFI_SUCCESS The device security policy is returned
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EDKII_DEVICE_SECURITY_GET_DEVICE_POLICY) (
> + IN EDKII_DEVICE_SECURITY_POLICY_PROTOCOL *This,
> + IN EDKII_DEVICE_IDENTIFIER *DeviceId,
> + OUT EDKII_DEVICE_SECURITY_POLICY **DeviceSecurityPolicy
> + );
> +
> +/**
> + This function sets the device state based upon the authentication result.
> +
> + @param[in] This The protocol instance pointer.
> + @param[in] DeviceId The Identifier for the device.
> + @param[in] DeviceSecurityState The Device Security state associated
> with the device.
> +
> + @retval EFI_SUCCESS The device state is set
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EDKII_DEVICE_SECURITY_SET_DEVICE_STATE) (
> + IN EDKII_DEVICE_SECURITY_POLICY_PROTOCOL *This,
> + IN EDKII_DEVICE_IDENTIFIER *DeviceId,
> + IN EDKII_DEVICE_SECURITY_STATE *DeviceSecurityState
> + );
> +
> +struct _EDKII_DEVICE_SECURITY_POLICY_PROTOCOL {
> + UINT32 Version; // 0x1
> + EDKII_DEVICE_SECURITY_GET_DEVICE_POLICY GetDevicePolicy;
> + EDKII_DEVICE_SECURITY_SET_DEVICE_STATE SetDeviceState;
> +};
> +
> +extern EFI_GUID gEdkiiDeviceSecurityPolicyProtocolGuid;
> +
> +#endif
> --
> 2.19.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#50165): https://edk2.groups.io/g/devel/message/50165
Mute This Topic: https://groups.io/mt/40117802/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Sure.
1. add comments to each field of structures like EDKII_DEVICE_SECURITY_POLICY and EDKII_DEVICE_SECURITY_STATE.
2. add comments to all the macros defined in this patch to explain the meaning and more important how they are going to impact the logic.
I don't believe a PCI/USB BUS driver developer need understand this protocol.
The PCI/USB only need to understand MdeModulePkg/DEVICE_SECURITY_PROTOCOL.
The IntelSiliconPkg/DEVICE_SECURITY_POLICY_PROTOCOL is for the internal implementation of a device security driver. It should be unknown by device driver writer.
Thank you
Yao Jiewen
> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Thursday, November 7, 2019 12:55 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Lou, Yun
> <yun.lou@intel.com>
> Subject: RE: [PATCH V2 2/6] IntelSiliconPkg/Include: Add Platform Device
> Security Policy protocol
>
> Jiewen,
> I am fine with the 1/6 patch that doesn't contain enough comments to describe
> the meaning
> of each field in each structure because I can reach out to the referenced spec to
> understand them.
>
> This 2/6 patch introduces a new protocol but contains very few comments
> (almost none) for each
> structure each field and I cannot reach out to any spec to understand them.
>
> So can you please add comments to each field of structures like
> EDKII_DEVICE_SECURITY_POLICY and
> EDKII_DEVICE_SECURITY_STATE?
> Also can you please add comments to all the macros defined in this patch to
> explain the meaning and
> more important how they are going to impact the logic.
>
> In general, can you please add enough comments so that a PCI/USB BUS driver
> developer can understand
> the whole flow how this protocol supports the device security?
>
> Thanks,
> Ray
>
> > -----Original Message-----
> > From: Yao, Jiewen <jiewen.yao@intel.com>
> > Sent: Thursday, October 31, 2019 8:31 PM
> > To: devel@edk2.groups.io
> > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> > <rangasai.v.chaganty@intel.com>; Lou, Yun <yun.lou@intel.com>
> > Subject: [PATCH V2 2/6] IntelSiliconPkg/Include: Add Platform Device
> > Security Policy protocol
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
> >
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> > Cc: Yun Lou <yun.lou@intel.com>
> > Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> > ---
> >
> > Silicon/Intel/IntelSiliconPkg/Include/Protocol/PlatformDeviceSecurityPolicy.h
> > | 84 ++++++++++++++++++++
> > 1 file changed, 84 insertions(+)
> >
> > diff --git
> > a/Silicon/Intel/IntelSiliconPkg/Include/Protocol/PlatformDeviceSecurityPolic
> > y.h
> > b/Silicon/Intel/IntelSiliconPkg/Include/Protocol/PlatformDeviceSecurityPolic
> > y.h
> > new file mode 100644
> > index 0000000000..cb5a71ad41
> > --- /dev/null
> > +++
> > b/Silicon/Intel/IntelSiliconPkg/Include/Protocol/PlatformDeviceSecurityPolic
> > y.h
> > @@ -0,0 +1,84 @@
> > +/** @file
> > + Device Security Policy Protocol definition
> > +
> > + Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +
> > +#ifndef __EDKII_DEVICE_SECURITY_POLICY_PROTOCOL_H__
> > +#define __EDKII_DEVICE_SECURITY_POLICY_PROTOCOL_H__
> > +
> > +#include <Uefi.h>
> > +#include <Protocol/DeviceSecurity.h>
> > +
> > +typedef struct _EDKII_DEVICE_SECURITY_POLICY_PROTOCOL
> > EDKII_DEVICE_SECURITY_POLICY_PROTOCOL;
> > +
> > +typedef struct {
> > + UINT32 Version; // 0x1
> > + UINT32 MeasurementPolicy;
> > + UINT32 AuthenticationPolicy;
> > +} EDKII_DEVICE_SECURITY_POLICY;
> > +
> > +// BIT0 means if the action is needed or NOT
> > +#define EDKII_DEVICE_MEASUREMENT_POLICY_REQUIRED BIT0
> > +#define EDKII_DEVICE_AUTHENTICATION_POLICY_REQUIRED BIT0
> > +
> > +typedef struct {
> > + UINT32 Version; // 0x1
> > + UINT32 MeasurementState;
> > + UINT32 AuthenticationState;
> > +} EDKII_DEVICE_SECURITY_STATE;
> > +
> > +// All zero means success
> > +#define EDKII_DEVICE_SECURITY_STATE_SUCCESS 0
> > +#define EDKII_DEVICE_SECURITY_STATE_ERROR BIT31
> > +#define EDKII_DEVICE_SECURITY_STATE_ERROR_UEFI_UNSUPPORTED
> > (EDKII_DEVICE_SECURITY_STATE_ERROR + 0x0)
> > +#define
> > EDKII_DEVICE_SECURITY_STATE_ERROR_UEFI_GET_POLICY_PROTOCOL
> > (EDKII_DEVICE_SECURITY_STATE_ERROR + 0x1)
> > +#define EDKII_DEVICE_SECURITY_STATE_ERROR_PCI_NO_CAPABILITIES
> > (EDKII_DEVICE_SECURITY_STATE_ERROR + 0x10)
> > +#define EDKII_DEVICE_SECURITY_STATE_ERROR_TCG_EXTEND_TPM_PCR
> > (EDKII_DEVICE_SECURITY_STATE_ERROR + 0x20)
> > +
> > +/**
> > + This function returns the device security policy associated with the device.
> > +
> > + @param[in] This The protocol instance pointer.
> > + @param[in] DeviceId The Identifier for the device.
> > + @param[out] DeviceSecurityPolicy The Device Security Policy associated
> > with the device.
> > +
> > + @retval EFI_SUCCESS The device security policy is returned
> > +**/
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *EDKII_DEVICE_SECURITY_GET_DEVICE_POLICY) (
> > + IN EDKII_DEVICE_SECURITY_POLICY_PROTOCOL *This,
> > + IN EDKII_DEVICE_IDENTIFIER *DeviceId,
> > + OUT EDKII_DEVICE_SECURITY_POLICY **DeviceSecurityPolicy
> > + );
> > +
> > +/**
> > + This function sets the device state based upon the authentication result.
> > +
> > + @param[in] This The protocol instance pointer.
> > + @param[in] DeviceId The Identifier for the device.
> > + @param[in] DeviceSecurityState The Device Security state associated
> > with the device.
> > +
> > + @retval EFI_SUCCESS The device state is set
> > +**/
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *EDKII_DEVICE_SECURITY_SET_DEVICE_STATE) (
> > + IN EDKII_DEVICE_SECURITY_POLICY_PROTOCOL *This,
> > + IN EDKII_DEVICE_IDENTIFIER *DeviceId,
> > + IN EDKII_DEVICE_SECURITY_STATE *DeviceSecurityState
> > + );
> > +
> > +struct _EDKII_DEVICE_SECURITY_POLICY_PROTOCOL {
> > + UINT32 Version; // 0x1
> > + EDKII_DEVICE_SECURITY_GET_DEVICE_POLICY GetDevicePolicy;
> > + EDKII_DEVICE_SECURITY_SET_DEVICE_STATE SetDeviceState;
> > +};
> > +
> > +extern EFI_GUID gEdkiiDeviceSecurityPolicyProtocolGuid;
> > +
> > +#endif
> > --
> > 2.19.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#50185): https://edk2.groups.io/g/devel/message/50185
Mute This Topic: https://groups.io/mt/40117802/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.