Yes, that is why I was saying to use AhciSetFeaturePuis as the function name.
Thanks,
Star
-----Original Message-----
From: Ni, Ruiyu
Sent: Tuesday, June 5, 2018 11:41 AM
To: Zeng, Star <star.z...@intel.com>; edk2-devel@lists.01.org
Cc: Chiu, Chasel <chasel.c...@intel.com>
Subject: RE: [PATCH v2 3/4] MdeModulePkg/AtaAtapiPassThru: enable/disable PUIS
per policy
The Set Feature cmd is used to enable/disable PUIS.
Thanks/Ray
> -----Original Message-----
> From: Zeng, Star
> Sent: Tuesday, June 5, 2018 11:35 AM
> To: Ni, Ruiyu <ruiyu...@intel.com>; edk2-devel@lists.01.org
> Cc: Chiu, Chasel <chasel.c...@intel.com>; Zeng, Star
> <star.z...@intel.com>
> Subject: RE: [PATCH v2 3/4] MdeModulePkg/AtaAtapiPassThru:
> enable/disable PUIS per policy
>
> Reviewed-by: Star Zeng <star.z...@intel.com>
>
> How about using function name AhciSetFeaturePuis instead of
> AhciPuisEnable as the function is not just to enable Puis?
>
>
> Thanks,
> Star
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Monday, June 4, 2018 3:04 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.z...@intel.com>; Chiu, Chasel
> <chasel.c...@intel.com>
> Subject: [PATCH v2 3/4] MdeModulePkg/AtaAtapiPassThru: enable/disable
> PUIS per policy
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu...@intel.com>
> Cc: Star Zeng <star.z...@intel.com>
> Cc: Chasel Chiu <chasel.c...@intel.com>
> ---
> MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 48
> ++++++++++++++++++++++
> .../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 19 ++++++++-
> .../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 3 ++
> .../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf | 3 +-
> 4 files changed, 71 insertions(+), 2 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> index 14578c0f94..841b6a0e60 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> @@ -2316,6 +2316,38 @@ AhciSpinUpDisk (
> return EFI_SUCCESS;
> }
>
> +/**
> + Enable/disable/skip PUIS of the disk according to policy.
> +
> + @param PciIo The PCI IO protocol instance.
> + @param AhciRegisters The pointer to the EFI_AHCI_REGISTERS.
> + @param Port The number of port.
> + @param PortMultiplier The multiplier of port.
> +
> +**/
> +EFI_STATUS
> +AhciPuisEnable (
> + IN EFI_PCI_IO_PROTOCOL *PciIo,
> + IN EFI_AHCI_REGISTERS *AhciRegisters,
> + IN UINT8 Port,
> + IN UINT8 PortMultiplier
> + )
> +{
> + EFI_STATUS Status;
> +
> + Status = EFI_SUCCESS;
> + if (mAtaAtapiPolicy->PuisEnable == 0) {
> + Status = AhciDeviceSetFeature (PciIo, AhciRegisters, Port,
> +PortMultiplier, ATA_SUB_CMD_DISABLE_PUIS, 0x00,
> ATA_ATAPI_TIMEOUT);
> + } else if (mAtaAtapiPolicy->PuisEnable == 1) {
> + Status = AhciDeviceSetFeature (PciIo, AhciRegisters, Port,
> +PortMultiplier, ATA_SUB_CMD_ENABLE_PUIS, 0x00,
> ATA_ATAPI_TIMEOUT);
> + }
> + DEBUG ((DEBUG_INFO, "%a PUIS feature at port [%d] PortMultiplier
> + [%d] -
> %r!\n",
> + (mAtaAtapiPolicy->PuisEnable == 0) ? "Disable" : (
> + (mAtaAtapiPolicy->PuisEnable == 1) ? "Enable" : "Skip"
> + ), Port, PortMultiplier, Status));
> + return Status;
> +}
> +
> /**
> Initialize ATA host controller at AHCI mode.
>
> @@ -2658,6 +2690,22 @@ AhciModeInitialization (
> if (DeviceType == EfiIdeHarddisk) {
> REPORT_STATUS_CODE (EFI_PROGRESS_CODE,
> (EFI_PERIPHERAL_FIXED_MEDIA | EFI_P_PC_ENABLE));
> }
> +
> + //
> + // Enable/disable PUIS according to policy setting if PUIS is
> + capable
> (Word[83].BIT5 is set).
> + //
> + if ((Buffer.AtaData.command_set_supported_83 & BIT5) != 0) {
> + Status = AhciPuisEnable (
> + PciIo,
> + AhciRegisters,
> + Port,
> + 0
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "PUIS enable/disable failed, Status =
> + %r\n",
> Status));
> + continue;
> + }
> + }
> }
> }
>
> diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> index a48b295d26..aab704bcd3 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> @@ -2,7 +2,7 @@
> This file implements ATA_PASSTHRU_PROCTOCOL and
> EXT_SCSI_PASSTHRU_PROTOCOL interfaces
> for managed ATA controllers.
>
> - Copyright (c) 2010 - 2016, Intel Corporation. All rights
> reserved.<BR>
> + Copyright (c) 2010 - 2018, Intel Corporation. All rights
> + reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of
> the BSD License
> which accompanies this distribution. The full text of the license
> may be found at @@ -142,6 +142,15 @@ UINT8 mScsiId[TARGET_MAX_BYTES] = {
> 0xFF, 0xFF, 0xFF, 0xFF
> };
>
> +EDKII_ATA_ATAPI_POLICY_PROTOCOL *mAtaAtapiPolicy;
> +EDKII_ATA_ATAPI_POLICY_PROTOCOL mDefaultAtaAtapiPolicy = {
> + EDKII_ATA_ATAPI_POLICY_VERSION,
> + 2, // PuisEnable
> + 0, // DeviceSleepEnable
> + 0, // AggressiveDeviceSleepEnable
> + 0 // Reserved
> +};
> +
> /**
> Sends an ATA command to an ATA device that is attached to the ATA
> controller. This function
> supports both blocking I/O and non-blocking I/O. The blocking I/O
> functionality is required, @@ -739,6 +748,14 @@ AtaAtapiPassThruStart (
> goto ErrorExit;
> }
>
> + Status = gBS->LocateProtocol (&gEdkiiAtaAtapiPolicyProtocolGuid,
> + NULL, (VOID **)&mAtaAtapiPolicy); if (EFI_ERROR (Status)) {
> + //
> + // If there is no AtaAtapiPolicy exposed, use the default policy.
> + //
> + mAtaAtapiPolicy = &mDefaultAtaAtapiPolicy; }
> +
> //
> // Allocate a buffer to store the ATA_ATAPI_PASS_THRU_INSTANCE data
> structure
> //
> diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> index 31b005f2f6..b07bcbbb3e 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> @@ -24,6 +24,7 @@
> #include <Protocol/IdeControllerInit.h> #include
> <Protocol/AtaPassThru.h> #include <Protocol/ScsiPassThruExt.h>
> +#include <Protocol/AtaAtapiPolicy.h>
>
> #include <Library/DebugLib.h>
> #include <Library/BaseLib.h>
> @@ -45,6 +46,8 @@ extern EFI_DRIVER_BINDING_PROTOCOL
> gAtaAtapiPassThruDriverBinding; extern EFI_COMPONENT_NAME_PROTOCOL
> gAtaAtapiPassThruComponentName; extern EFI_COMPONENT_NAME2_PROTOCOL
> gAtaAtapiPassThruComponentName2;
>
> +extern EDKII_ATA_ATAPI_POLICY_PROTOCOL *mAtaAtapiPolicy;
> +
> #define ATA_ATAPI_PASS_THRU_SIGNATURE SIGNATURE_32 ('a', 'a', 'p', 't')
> #define ATA_ATAPI_DEVICE_SIGNATURE SIGNATURE_32 ('a', 'd', 'e', 'v')
> #define ATA_NONBLOCKING_TASK_SIGNATURE SIGNATURE_32 ('a', 't', 's',
> 'k') diff --git
> a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
> index 82d5f7a46c..d1ce859091 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
> @@ -4,7 +4,7 @@
> # This driver installs AtaPassThru and ExtScsiPassThru protocol in
> each ide/sata controller # to access to all attached Ata/Atapi devices.
> #
> -# Copyright (c) 2010 - 2014, Intel Corporation. All rights
> reserved.<BR>
> +# Copyright (c) 2010 - 2018, Intel Corporation. All rights
> +reserved.<BR>
> #
> # This program and the accompanying materials # are licensed and
> made available under the terms and conditions of the BSD License @@
> -67,6 +67,7 @@ [Protocols]
> gEfiIdeControllerInitProtocolGuid ## TO_START
> gEfiDevicePathProtocolGuid ## TO_START
> gEfiPciIoProtocolGuid ## TO_START
> + gEdkiiAtaAtapiPolicyProtocolGuid ## CONSUMES
>
> [Pcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdAtaSmartEnable ##
> SOMETIMES_CONSUMES
> --
> 2.16.1.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
