[edk2-devel] [PATCH V2 5/6] IntelSiliconPkg/SamplePlatformDevicePolicyDxe: Add sample policy.

Yao, Jiewen posted 6 patches 6 years, 3 months ago
There is a newer version of this series
[edk2-devel] [PATCH V2 5/6] IntelSiliconPkg/SamplePlatformDevicePolicyDxe: Add sample policy.
Posted by Yao, Jiewen 6 years, 3 months ago
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303

This driver provides the platform sample policy to measure
a NVMe card.

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/Feature/PcieSecurity/SamplePlatformDevicePolicyDxe/SamplePlatformDevicePolicyDxe.c   | 189 ++++++++++++++++++++
 Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformDevicePolicyDxe/SamplePlatformDevicePolicyDxe.inf |  40 +++++
 2 files changed, 229 insertions(+)

diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformDevicePolicyDxe/SamplePlatformDevicePolicyDxe.c b/Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformDevicePolicyDxe/SamplePlatformDevicePolicyDxe.c
new file mode 100644
index 0000000000..1f01b961a8
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformDevicePolicyDxe/SamplePlatformDevicePolicyDxe.c
@@ -0,0 +1,189 @@
+/** @file
+  EDKII Device Security library for PCI device
+
+Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Uefi.h>
+#include <IndustryStandard/Spdm.h>
+#include <IndustryStandard/Pci.h>
+#include <Protocol/PciIo.h>
+#include <Protocol/DeviceSecurity.h>
+#include <Protocol/PlatformDeviceSecurityPolicy.h>
+#include <Library/DebugLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/MemoryAllocationLib.h>
+
+EDKII_DEVICE_SECURITY_POLICY           mDeviceSecurityPolicyNone = {
+  0x1,
+  0,
+  0,
+};
+EDKII_DEVICE_SECURITY_POLICY           mDeviceSecurityPolicyMeasurement = {
+  0x1,
+  EDKII_DEVICE_MEASUREMENT_POLICY_REQUIRED,
+  0,
+};
+
+/**
+  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
+**/
+EFI_STATUS
+EFIAPI
+GetDevicePolicy (
+  IN  EDKII_DEVICE_SECURITY_POLICY_PROTOCOL  *This,
+  IN  EDKII_DEVICE_IDENTIFIER                *DeviceId,
+  OUT EDKII_DEVICE_SECURITY_POLICY           **DeviceSecurityPolicy
+  )
+{
+  EFI_STATUS                  Status;
+  EFI_PCI_IO_PROTOCOL         *PciIo;
+  UINT16                      PciVendorId;
+  UINT16                      PciDeviceId;
+
+  *DeviceSecurityPolicy = &mDeviceSecurityPolicyNone;
+
+  DEBUG ((DEBUG_INFO, "GetDevicePolicy - 0x%g\n", &DeviceId->DeviceType));
+
+  if (!CompareGuid (&DeviceId->DeviceType, &gEdkiiDeviceIdentifierTypePciGuid)) {
+    return EFI_SUCCESS;
+  }
+
+  Status = gBS->HandleProtocol (
+                  DeviceId->DeviceHandle,
+                  &gEdkiiDeviceIdentifierTypePciGuid,
+                  (VOID **)&PciIo
+                  );
+  if (EFI_ERROR(Status)) {
+    DEBUG ((DEBUG_ERROR, "Locate - DeviceIdentifierTypePci - %r\n", Status));
+    return EFI_SUCCESS;
+  }
+
+  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint16, PCI_VENDOR_ID_OFFSET, 1, &PciVendorId);
+  ASSERT_EFI_ERROR(Status);
+  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint16, PCI_DEVICE_ID_OFFSET, 1, &PciDeviceId);
+  ASSERT_EFI_ERROR(Status);
+  DEBUG ((DEBUG_INFO, "PCI Info - %04x:%04x\n", PciVendorId, PciDeviceId));
+
+  if ((PciVendorId == 0x8086) && (PciDeviceId == 0x0B60)) {
+    *DeviceSecurityPolicy = &mDeviceSecurityPolicyMeasurement;
+  }
+
+  return EFI_SUCCESS;
+}
+
+/**
+  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
+**/
+EFI_STATUS
+EFIAPI
+SetDeviceState (
+  IN  EDKII_DEVICE_SECURITY_POLICY_PROTOCOL  *This,
+  IN  EDKII_DEVICE_IDENTIFIER                *DeviceId,
+  IN  EDKII_DEVICE_SECURITY_STATE            *DeviceSecurityState
+  )
+{
+  EFI_STATUS                  Status;
+  EFI_PCI_IO_PROTOCOL         *PciIo;
+  UINT16                      PciVendorId;
+  UINT16                      PciDeviceId;
+  UINTN                       Segment;
+  UINTN                       Bus;
+  UINTN                       Device;
+  UINTN                       Function;
+
+  DEBUG ((DEBUG_INFO, "SetDeviceState - 0x%g\n", &DeviceId->DeviceType));
+
+  if (!CompareGuid (&DeviceId->DeviceType, &gEdkiiDeviceIdentifierTypePciGuid)) {
+    return EFI_SUCCESS;
+  }
+
+  Status = gBS->HandleProtocol (
+                  DeviceId->DeviceHandle,
+                  &gEdkiiDeviceIdentifierTypePciGuid,
+                  (VOID **)&PciIo
+                  );
+  if (EFI_ERROR(Status)) {
+    DEBUG ((DEBUG_ERROR, "Locate - DeviceIdentifierTypePci - %r\n", Status));
+    return EFI_SUCCESS;
+  }
+
+  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint16, PCI_VENDOR_ID_OFFSET, 1, &PciVendorId);
+  ASSERT_EFI_ERROR(Status);
+  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint16, PCI_DEVICE_ID_OFFSET, 1, &PciDeviceId);
+  ASSERT_EFI_ERROR(Status);
+  DEBUG ((DEBUG_INFO, "PCI Info - %04x:%04x\n", PciVendorId, PciDeviceId));
+
+  Status = PciIo->GetLocation (
+                    PciIo,
+                    &Segment,
+                    &Bus,
+                    &Device,
+                    &Function
+                    );
+  if (!EFI_ERROR(Status)) {
+    DEBUG ((DEBUG_INFO, "PCI Loc - %04x:%02x:%02x:%02x\n",
+      Segment, Bus, Device, Function));
+  }
+
+  DEBUG ((DEBUG_INFO, "State - Measurement - 0x%08x, Authentication - 0x%08x\n",
+    DeviceSecurityState->MeasurementState,
+    DeviceSecurityState->AuthenticationState
+    ));
+
+  return EFI_SUCCESS;
+}
+
+EDKII_DEVICE_SECURITY_POLICY_PROTOCOL  mDeviceSecurityPolicy = {
+  0x1,
+  GetDevicePolicy,
+  SetDeviceState,
+};
+
+/**
+  Entrypoint of the device security driver.
+
+  @param[in]  ImageHandle  ImageHandle of the loaded driver
+  @param[in]  SystemTable  Pointer to the System Table
+
+  @retval  EFI_SUCCESS           The Protocol is installed.
+  @retval  EFI_OUT_OF_RESOURCES  Not enough resources available to initialize driver.
+  @retval  EFI_DEVICE_ERROR      A device error occurred attempting to initialize the driver.
+
+**/
+EFI_STATUS
+EFIAPI
+MainEntryPoint (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_HANDLE  Handle;
+  EFI_STATUS  Status;
+
+  Handle = NULL;
+  Status = gBS->InstallProtocolInterface (
+                  &Handle,
+                  &gEdkiiDeviceSecurityPolicyProtocolGuid,
+                  EFI_NATIVE_INTERFACE,
+                  &mDeviceSecurityPolicy
+                  );
+  ASSERT_EFI_ERROR(Status);
+
+  return Status;
+}
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformDevicePolicyDxe/SamplePlatformDevicePolicyDxe.inf b/Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformDevicePolicyDxe/SamplePlatformDevicePolicyDxe.inf
new file mode 100644
index 0000000000..a9b77d8371
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformDevicePolicyDxe/SamplePlatformDevicePolicyDxe.inf
@@ -0,0 +1,40 @@
+## @file
+#  EDKII Device Security library for PCI device
+#
+# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = SamplePlatformDevicePolicyDxe
+  FILE_GUID                      = 7EA7AACF-7ED3-4166-8271-B21156523620
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = MainEntryPoint
+
+[Sources]
+  SamplePlatformDevicePolicyDxe.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  IntelSiliconPkg/IntelSiliconPkg.dec
+
+[LibraryClasses]
+  UefiRuntimeServicesTableLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+  MemoryAllocationLib
+  DevicePathLib
+  BaseMemoryLib
+  PrintLib
+  DebugLib
+
+[Protocols]
+  gEdkiiDeviceSecurityPolicyProtocolGuid  ## PRODUCES
+  gEdkiiDeviceIdentifierTypePciGuid       ## COMSUMES
+
+[Depex]
+  TRUE
-- 
2.19.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#49773): https://edk2.groups.io/g/devel/message/49773
Mute This Topic: https://groups.io/mt/40117827/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V2 5/6] IntelSiliconPkg/SamplePlatformDevicePolicyDxe: Add sample policy.
Posted by Ni, Ray 6 years, 3 months ago
1. I am sorry but I just realized GetDevicePolicy() returns a pointer of policy to caller,
    instead of copying the policy data to caller's buffer. It's a bit strange to me. Can you
    please change the prototype so that caller needs to pass in a policy structure and
    GetDevicePolicy() fills the structure buffer using CopyMem from mDeviceSecurityPolicyNone?
    "*DeviceSecurityPolicy = &mDeviceSecurityPolicyNone;"
    "DeviceSecurityPolicy = &mDeviceSecurityPolicyMeasurement;"

2. I thought SetDeviceState() in your sample will reset the EDKII_DEVICE_SECURITY_STATE_ERROR_UEFI_UNSUPPORTED
    to 0. Why didn't the sample do that?

3. I just realized you didn't define the version macro for EDKII_DEVICE_SECURITY_POLICY_PROTOCOL.Version.
    I suggest you define a macro for the version instead of let producer/consumer use hard-code number (0x1).
    And I am not sure if you will use the same version macro for the protocol.version, securitypolicy.version and
    securitystate.version. I assume you will. No matter yes or no, can you please state that through comments
    in the policy protocol header file clearly?



> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao,
> Jiewen
> 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: [edk2-devel] [PATCH V2 5/6]
> IntelSiliconPkg/SamplePlatformDevicePolicyDxe: Add sample policy.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
> 
> This driver provides the platform sample policy to measure a NVMe card.
> 
> 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/Feature/PcieSecurity/SamplePlatformDevicePoli
> cyDxe/SamplePlatformDevicePolicyDxe.c   | 189 ++++++++++++++++++++
> 
> Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformDevicePoli
> cyDxe/SamplePlatformDevicePolicyDxe.inf |  40 +++++
>  2 files changed, 229 insertions(+)
> 
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformDeviceP
> olicyDxe/SamplePlatformDevicePolicyDxe.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformDeviceP
> olicyDxe/SamplePlatformDevicePolicyDxe.c
> new file mode 100644
> index 0000000000..1f01b961a8
> --- /dev/null
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformD
> +++ evicePolicyDxe/SamplePlatformDevicePolicyDxe.c
> @@ -0,0 +1,189 @@
> +/** @file
> +  EDKII Device Security library for PCI device
> +
> +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Uefi.h>
> +#include <IndustryStandard/Spdm.h>
> +#include <IndustryStandard/Pci.h>
> +#include <Protocol/PciIo.h>
> +#include <Protocol/DeviceSecurity.h>
> +#include <Protocol/PlatformDeviceSecurityPolicy.h>
> +#include <Library/DebugLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +
> +EDKII_DEVICE_SECURITY_POLICY           mDeviceSecurityPolicyNone = {
> +  0x1,
> +  0,
> +  0,
> +};
> +EDKII_DEVICE_SECURITY_POLICY           mDeviceSecurityPolicyMeasurement
> = {
> +  0x1,
> +  EDKII_DEVICE_MEASUREMENT_POLICY_REQUIRED,
> +  0,
> +};
> +
> +/**
> +  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
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetDevicePolicy (
> +  IN  EDKII_DEVICE_SECURITY_POLICY_PROTOCOL  *This,
> +  IN  EDKII_DEVICE_IDENTIFIER                *DeviceId,
> +  OUT EDKII_DEVICE_SECURITY_POLICY           **DeviceSecurityPolicy
> +  )
> +{
> +  EFI_STATUS                  Status;
> +  EFI_PCI_IO_PROTOCOL         *PciIo;
> +  UINT16                      PciVendorId;
> +  UINT16                      PciDeviceId;
> +
> +  *DeviceSecurityPolicy = &mDeviceSecurityPolicyNone;
> +
> +  DEBUG ((DEBUG_INFO, "GetDevicePolicy - 0x%g\n",
> + &DeviceId->DeviceType));
> +
> +  if (!CompareGuid (&DeviceId->DeviceType,
> &gEdkiiDeviceIdentifierTypePciGuid)) {
> +    return EFI_SUCCESS;
> +  }
> +
> +  Status = gBS->HandleProtocol (
> +                  DeviceId->DeviceHandle,
> +                  &gEdkiiDeviceIdentifierTypePciGuid,
> +                  (VOID **)&PciIo
> +                  );
> +  if (EFI_ERROR(Status)) {
> +    DEBUG ((DEBUG_ERROR, "Locate - DeviceIdentifierTypePci - %r\n",
> Status));
> +    return EFI_SUCCESS;
> +  }
> +
> +  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint16,
> + PCI_VENDOR_ID_OFFSET, 1, &PciVendorId);  ASSERT_EFI_ERROR(Status);
> + Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint16,
> + PCI_DEVICE_ID_OFFSET, 1, &PciDeviceId);  ASSERT_EFI_ERROR(Status);
> + DEBUG ((DEBUG_INFO, "PCI Info - %04x:%04x\n", PciVendorId,
> + PciDeviceId));
> +
> +  if ((PciVendorId == 0x8086) && (PciDeviceId == 0x0B60)) {
> +    *DeviceSecurityPolicy = &mDeviceSecurityPolicyMeasurement;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  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
> +**/
> +EFI_STATUS
> +EFIAPI
> +SetDeviceState (
> +  IN  EDKII_DEVICE_SECURITY_POLICY_PROTOCOL  *This,
> +  IN  EDKII_DEVICE_IDENTIFIER                *DeviceId,
> +  IN  EDKII_DEVICE_SECURITY_STATE            *DeviceSecurityState
> +  )
> +{
> +  EFI_STATUS                  Status;
> +  EFI_PCI_IO_PROTOCOL         *PciIo;
> +  UINT16                      PciVendorId;
> +  UINT16                      PciDeviceId;
> +  UINTN                       Segment;
> +  UINTN                       Bus;
> +  UINTN                       Device;
> +  UINTN                       Function;
> +
> +  DEBUG ((DEBUG_INFO, "SetDeviceState - 0x%g\n",
> + &DeviceId->DeviceType));
> +
> +  if (!CompareGuid (&DeviceId->DeviceType,
> &gEdkiiDeviceIdentifierTypePciGuid)) {
> +    return EFI_SUCCESS;
> +  }
> +
> +  Status = gBS->HandleProtocol (
> +                  DeviceId->DeviceHandle,
> +                  &gEdkiiDeviceIdentifierTypePciGuid,
> +                  (VOID **)&PciIo
> +                  );
> +  if (EFI_ERROR(Status)) {
> +    DEBUG ((DEBUG_ERROR, "Locate - DeviceIdentifierTypePci - %r\n",
> Status));
> +    return EFI_SUCCESS;
> +  }
> +
> +  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint16,
> + PCI_VENDOR_ID_OFFSET, 1, &PciVendorId);  ASSERT_EFI_ERROR(Status);
> + Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint16,
> + PCI_DEVICE_ID_OFFSET, 1, &PciDeviceId);  ASSERT_EFI_ERROR(Status);
> + DEBUG ((DEBUG_INFO, "PCI Info - %04x:%04x\n", PciVendorId,
> + PciDeviceId));
> +
> +  Status = PciIo->GetLocation (
> +                    PciIo,
> +                    &Segment,
> +                    &Bus,
> +                    &Device,
> +                    &Function
> +                    );
> +  if (!EFI_ERROR(Status)) {
> +    DEBUG ((DEBUG_INFO, "PCI Loc - %04x:%02x:%02x:%02x\n",
> +      Segment, Bus, Device, Function));  }
> +
> +  DEBUG ((DEBUG_INFO, "State - Measurement - 0x%08x, Authentication -
> 0x%08x\n",
> +    DeviceSecurityState->MeasurementState,
> +    DeviceSecurityState->AuthenticationState
> +    ));
> +
> +  return EFI_SUCCESS;
> +}
> +
> +EDKII_DEVICE_SECURITY_POLICY_PROTOCOL  mDeviceSecurityPolicy = {
> +  0x1,
> +  GetDevicePolicy,
> +  SetDeviceState,
> +};
> +
> +/**
> +  Entrypoint of the device security driver.
> +
> +  @param[in]  ImageHandle  ImageHandle of the loaded driver  @param[in]
> + SystemTable  Pointer to the System Table
> +
> +  @retval  EFI_SUCCESS           The Protocol is installed.
> +  @retval  EFI_OUT_OF_RESOURCES  Not enough resources available to
> initialize driver.
> +  @retval  EFI_DEVICE_ERROR      A device error occurred attempting to
> initialize the driver.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +MainEntryPoint (
> +  IN EFI_HANDLE        ImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  EFI_HANDLE  Handle;
> +  EFI_STATUS  Status;
> +
> +  Handle = NULL;
> +  Status = gBS->InstallProtocolInterface (
> +                  &Handle,
> +                  &gEdkiiDeviceSecurityPolicyProtocolGuid,
> +                  EFI_NATIVE_INTERFACE,
> +                  &mDeviceSecurityPolicy
> +                  );
> +  ASSERT_EFI_ERROR(Status);
> +
> +  return Status;
> +}
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformDeviceP
> olicyDxe/SamplePlatformDevicePolicyDxe.inf
> b/Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformDeviceP
> olicyDxe/SamplePlatformDevicePolicyDxe.inf
> new file mode 100644
> index 0000000000..a9b77d8371
> --- /dev/null
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformD
> +++ evicePolicyDxe/SamplePlatformDevicePolicyDxe.inf
> @@ -0,0 +1,40 @@
> +## @file
> +#  EDKII Device Security library for PCI device # # Copyright (c) 2019,
> +Intel Corporation. All rights reserved.<BR> # SPDX-License-Identifier:
> +BSD-2-Clause-Patent # ##
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005
> +  BASE_NAME                      = SamplePlatformDevicePolicyDxe
> +  FILE_GUID                      = 7EA7AACF-7ED3-4166-8271-B21156523620
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = MainEntryPoint
> +
> +[Sources]
> +  SamplePlatformDevicePolicyDxe.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  IntelSiliconPkg/IntelSiliconPkg.dec
> +
> +[LibraryClasses]
> +  UefiRuntimeServicesTableLib
> +  UefiBootServicesTableLib
> +  UefiDriverEntryPoint
> +  MemoryAllocationLib
> +  DevicePathLib
> +  BaseMemoryLib
> +  PrintLib
> +  DebugLib
> +
> +[Protocols]
> +  gEdkiiDeviceSecurityPolicyProtocolGuid  ## PRODUCES
> +  gEdkiiDeviceIdentifierTypePciGuid       ## COMSUMES
> +
> +[Depex]
> +  TRUE
> --
> 2.19.2.windows.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#50174): https://edk2.groups.io/g/devel/message/50174
Mute This Topic: https://groups.io/mt/40117827/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V2 5/6] IntelSiliconPkg/SamplePlatformDevicePolicyDxe: Add sample policy.
Posted by Yao, Jiewen 6 years, 3 months ago
Good feedback.
Comment below:

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Thursday, November 7, 2019 2:56 PM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Lou, Yun
> <yun.lou@intel.com>
> Subject: RE: [edk2-devel] [PATCH V2 5/6]
> IntelSiliconPkg/SamplePlatformDevicePolicyDxe: Add sample policy.
> 
> 1. I am sorry but I just realized GetDevicePolicy() returns a pointer of policy to
> caller,
>     instead of copying the policy data to caller's buffer. It's a bit strange to me.
> Can you
>     please change the prototype so that caller needs to pass in a policy structure
> and
>     GetDevicePolicy() fills the structure buffer using CopyMem from
> mDeviceSecurityPolicyNone?
>     "*DeviceSecurityPolicy = &mDeviceSecurityPolicyNone;"
>     "DeviceSecurityPolicy = &mDeviceSecurityPolicyMeasurement;"
[Jiewen] Agree. Will update in V3.

> 
> 2. I thought SetDeviceState() in your sample will reset the
> EDKII_DEVICE_SECURITY_STATE_ERROR_UEFI_UNSUPPORTED
>     to 0. Why didn't the sample do that?
[Jiewen] I rename the function to NotifyDeviceState() in V3.
I think that is clear.
Also the state parameter is IN. The caller should not change it.

> 
> 3. I just realized you didn't define the version macro for
> EDKII_DEVICE_SECURITY_POLICY_PROTOCOL.Version.
>     I suggest you define a macro for the version instead of let producer/consumer
> use hard-code number (0x1).
>     And I am not sure if you will use the same version macro for the
> protocol.version, securitypolicy.version and
>     securitystate.version. I assume you will. No matter yes or no, can you please
> state that through comments
>     in the policy protocol header file clearly?
[Jiewen] Agree. Will update in V3.

> 
> 
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao,
> > Jiewen
> > 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: [edk2-devel] [PATCH V2 5/6]
> > IntelSiliconPkg/SamplePlatformDevicePolicyDxe: Add sample policy.
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2303
> >
> > This driver provides the platform sample policy to measure a NVMe card.
> >
> > 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/Feature/PcieSecurity/SamplePlatformDevicePoli
> > cyDxe/SamplePlatformDevicePolicyDxe.c   | 189 ++++++++++++++++++++
> >
> > Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformDevicePoli
> > cyDxe/SamplePlatformDevicePolicyDxe.inf |  40 +++++
> >  2 files changed, 229 insertions(+)
> >
> > diff --git
> > a/Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformDeviceP
> > olicyDxe/SamplePlatformDevicePolicyDxe.c
> > b/Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformDeviceP
> > olicyDxe/SamplePlatformDevicePolicyDxe.c
> > new file mode 100644
> > index 0000000000..1f01b961a8
> > --- /dev/null
> > +++ b/Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformD
> > +++ evicePolicyDxe/SamplePlatformDevicePolicyDxe.c
> > @@ -0,0 +1,189 @@
> > +/** @file
> > +  EDKII Device Security library for PCI device
> > +
> > +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> > +SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#include <Uefi.h>
> > +#include <IndustryStandard/Spdm.h>
> > +#include <IndustryStandard/Pci.h>
> > +#include <Protocol/PciIo.h>
> > +#include <Protocol/DeviceSecurity.h>
> > +#include <Protocol/PlatformDeviceSecurityPolicy.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/BaseMemoryLib.h>
> > +#include <Library/UefiBootServicesTableLib.h>
> > +#include <Library/MemoryAllocationLib.h>
> > +
> > +EDKII_DEVICE_SECURITY_POLICY           mDeviceSecurityPolicyNone = {
> > +  0x1,
> > +  0,
> > +  0,
> > +};
> > +EDKII_DEVICE_SECURITY_POLICY           mDeviceSecurityPolicyMeasurement
> > = {
> > +  0x1,
> > +  EDKII_DEVICE_MEASUREMENT_POLICY_REQUIRED,
> > +  0,
> > +};
> > +
> > +/**
> > +  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
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +GetDevicePolicy (
> > +  IN  EDKII_DEVICE_SECURITY_POLICY_PROTOCOL  *This,
> > +  IN  EDKII_DEVICE_IDENTIFIER                *DeviceId,
> > +  OUT EDKII_DEVICE_SECURITY_POLICY           **DeviceSecurityPolicy
> > +  )
> > +{
> > +  EFI_STATUS                  Status;
> > +  EFI_PCI_IO_PROTOCOL         *PciIo;
> > +  UINT16                      PciVendorId;
> > +  UINT16                      PciDeviceId;
> > +
> > +  *DeviceSecurityPolicy = &mDeviceSecurityPolicyNone;
> > +
> > +  DEBUG ((DEBUG_INFO, "GetDevicePolicy - 0x%g\n",
> > + &DeviceId->DeviceType));
> > +
> > +  if (!CompareGuid (&DeviceId->DeviceType,
> > &gEdkiiDeviceIdentifierTypePciGuid)) {
> > +    return EFI_SUCCESS;
> > +  }
> > +
> > +  Status = gBS->HandleProtocol (
> > +                  DeviceId->DeviceHandle,
> > +                  &gEdkiiDeviceIdentifierTypePciGuid,
> > +                  (VOID **)&PciIo
> > +                  );
> > +  if (EFI_ERROR(Status)) {
> > +    DEBUG ((DEBUG_ERROR, "Locate - DeviceIdentifierTypePci - %r\n",
> > Status));
> > +    return EFI_SUCCESS;
> > +  }
> > +
> > +  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint16,
> > + PCI_VENDOR_ID_OFFSET, 1, &PciVendorId);  ASSERT_EFI_ERROR(Status);
> > + Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint16,
> > + PCI_DEVICE_ID_OFFSET, 1, &PciDeviceId);  ASSERT_EFI_ERROR(Status);
> > + DEBUG ((DEBUG_INFO, "PCI Info - %04x:%04x\n", PciVendorId,
> > + PciDeviceId));
> > +
> > +  if ((PciVendorId == 0x8086) && (PciDeviceId == 0x0B60)) {
> > +    *DeviceSecurityPolicy = &mDeviceSecurityPolicyMeasurement;
> > +  }
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > +  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
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +SetDeviceState (
> > +  IN  EDKII_DEVICE_SECURITY_POLICY_PROTOCOL  *This,
> > +  IN  EDKII_DEVICE_IDENTIFIER                *DeviceId,
> > +  IN  EDKII_DEVICE_SECURITY_STATE            *DeviceSecurityState
> > +  )
> > +{
> > +  EFI_STATUS                  Status;
> > +  EFI_PCI_IO_PROTOCOL         *PciIo;
> > +  UINT16                      PciVendorId;
> > +  UINT16                      PciDeviceId;
> > +  UINTN                       Segment;
> > +  UINTN                       Bus;
> > +  UINTN                       Device;
> > +  UINTN                       Function;
> > +
> > +  DEBUG ((DEBUG_INFO, "SetDeviceState - 0x%g\n",
> > + &DeviceId->DeviceType));
> > +
> > +  if (!CompareGuid (&DeviceId->DeviceType,
> > &gEdkiiDeviceIdentifierTypePciGuid)) {
> > +    return EFI_SUCCESS;
> > +  }
> > +
> > +  Status = gBS->HandleProtocol (
> > +                  DeviceId->DeviceHandle,
> > +                  &gEdkiiDeviceIdentifierTypePciGuid,
> > +                  (VOID **)&PciIo
> > +                  );
> > +  if (EFI_ERROR(Status)) {
> > +    DEBUG ((DEBUG_ERROR, "Locate - DeviceIdentifierTypePci - %r\n",
> > Status));
> > +    return EFI_SUCCESS;
> > +  }
> > +
> > +  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint16,
> > + PCI_VENDOR_ID_OFFSET, 1, &PciVendorId);  ASSERT_EFI_ERROR(Status);
> > + Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint16,
> > + PCI_DEVICE_ID_OFFSET, 1, &PciDeviceId);  ASSERT_EFI_ERROR(Status);
> > + DEBUG ((DEBUG_INFO, "PCI Info - %04x:%04x\n", PciVendorId,
> > + PciDeviceId));
> > +
> > +  Status = PciIo->GetLocation (
> > +                    PciIo,
> > +                    &Segment,
> > +                    &Bus,
> > +                    &Device,
> > +                    &Function
> > +                    );
> > +  if (!EFI_ERROR(Status)) {
> > +    DEBUG ((DEBUG_INFO, "PCI Loc - %04x:%02x:%02x:%02x\n",
> > +      Segment, Bus, Device, Function));  }
> > +
> > +  DEBUG ((DEBUG_INFO, "State - Measurement - 0x%08x, Authentication -
> > 0x%08x\n",
> > +    DeviceSecurityState->MeasurementState,
> > +    DeviceSecurityState->AuthenticationState
> > +    ));
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +EDKII_DEVICE_SECURITY_POLICY_PROTOCOL  mDeviceSecurityPolicy = {
> > +  0x1,
> > +  GetDevicePolicy,
> > +  SetDeviceState,
> > +};
> > +
> > +/**
> > +  Entrypoint of the device security driver.
> > +
> > +  @param[in]  ImageHandle  ImageHandle of the loaded driver  @param[in]
> > + SystemTable  Pointer to the System Table
> > +
> > +  @retval  EFI_SUCCESS           The Protocol is installed.
> > +  @retval  EFI_OUT_OF_RESOURCES  Not enough resources available to
> > initialize driver.
> > +  @retval  EFI_DEVICE_ERROR      A device error occurred attempting to
> > initialize the driver.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +MainEntryPoint (
> > +  IN EFI_HANDLE        ImageHandle,
> > +  IN EFI_SYSTEM_TABLE  *SystemTable
> > +  )
> > +{
> > +  EFI_HANDLE  Handle;
> > +  EFI_STATUS  Status;
> > +
> > +  Handle = NULL;
> > +  Status = gBS->InstallProtocolInterface (
> > +                  &Handle,
> > +                  &gEdkiiDeviceSecurityPolicyProtocolGuid,
> > +                  EFI_NATIVE_INTERFACE,
> > +                  &mDeviceSecurityPolicy
> > +                  );
> > +  ASSERT_EFI_ERROR(Status);
> > +
> > +  return Status;
> > +}
> > diff --git
> > a/Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformDeviceP
> > olicyDxe/SamplePlatformDevicePolicyDxe.inf
> > b/Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformDeviceP
> > olicyDxe/SamplePlatformDevicePolicyDxe.inf
> > new file mode 100644
> > index 0000000000..a9b77d8371
> > --- /dev/null
> > +++ b/Silicon/Intel/IntelSiliconPkg/Feature/PcieSecurity/SamplePlatformD
> > +++ evicePolicyDxe/SamplePlatformDevicePolicyDxe.inf
> > @@ -0,0 +1,40 @@
> > +## @file
> > +#  EDKII Device Security library for PCI device # # Copyright (c) 2019,
> > +Intel Corporation. All rights reserved.<BR> # SPDX-License-Identifier:
> > +BSD-2-Clause-Patent # ##
> > +
> > +[Defines]
> > +  INF_VERSION                    = 0x00010005
> > +  BASE_NAME                      = SamplePlatformDevicePolicyDxe
> > +  FILE_GUID                      = 7EA7AACF-7ED3-4166-8271-B21156523620
> > +  MODULE_TYPE                    = DXE_DRIVER
> > +  VERSION_STRING                 = 1.0
> > +  ENTRY_POINT                    = MainEntryPoint
> > +
> > +[Sources]
> > +  SamplePlatformDevicePolicyDxe.c
> > +
> > +[Packages]
> > +  MdePkg/MdePkg.dec
> > +  MdeModulePkg/MdeModulePkg.dec
> > +  IntelSiliconPkg/IntelSiliconPkg.dec
> > +
> > +[LibraryClasses]
> > +  UefiRuntimeServicesTableLib
> > +  UefiBootServicesTableLib
> > +  UefiDriverEntryPoint
> > +  MemoryAllocationLib
> > +  DevicePathLib
> > +  BaseMemoryLib
> > +  PrintLib
> > +  DebugLib
> > +
> > +[Protocols]
> > +  gEdkiiDeviceSecurityPolicyProtocolGuid  ## PRODUCES
> > +  gEdkiiDeviceIdentifierTypePciGuid       ## COMSUMES
> > +
> > +[Depex]
> > +  TRUE
> > --
> > 2.19.2.windows.1
> >
> >
> > 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#50188): https://edk2.groups.io/g/devel/message/50188
Mute This Topic: https://groups.io/mt/40117827/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-