Re: [edk2-devel] [PATCH v2 02/11] ArmPkg/ArmScmiDxe: Add PERFORMANCE_DESCRIBE_FASTCHANNEL support

["PierreGondois" <pierre.gondois@arm.com>]

On 10/26/23 12:37, Leif Lindholm wrote:
> On Wed, Oct 25, 2023 at 13:25:31 +0200, PierreGondois wrote:
>> From: Pierre Gondois <pierre.gondois@arm.com>
> 
> Sidebar: I think if you correct your name in your git config, the
> above tag will drop out.

Ok I will check.

> 
>> The PERFORMANCE_DESCRIBE_FASTCHANNEL Scmi command is available
>> since SCMI v2.0 and allows to query information about the supported
>> fast-channels of the Scmi performance protocol.
>> Add support for this command.
>>
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> ---
>>   .../ArmScmiDxe/ScmiPerformanceProtocol.c      | 80 +++++++++++++++--
>>   .../Protocol/ArmScmiPerformanceProtocol.h     | 90 ++++++++++++++++---
>>   2 files changed, 155 insertions(+), 15 deletions(-)
>>
>> diff --git a/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c b/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
>> index 0f89808fbdf9..1d87339209fd 100644
>> --- a/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
>> +++ b/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
>> @@ -1,12 +1,12 @@
>>   /** @file
>>   
>> -  Copyright (c) 2017-2021, Arm Limited. All rights reserved.<BR>
>> +  Copyright (c) 2017-2023, Arm Limited. All rights reserved.<BR>
>>   
>>     SPDX-License-Identifier: BSD-2-Clause-Patent
>>   
>> -  System Control and Management Interface V1.0
>> -    http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/
>> -    DEN0056A_System_Control_and_Management_Interface.pdf
>> +  System Control and Management Interface, latest version:
>> +  - https://developer.arm.com/documentation/den0056/latest/
>> +
>>   **/
>>   
>>   #include <Library/BaseMemoryLib.h>
>> @@ -416,6 +416,75 @@ PerformanceLevelGet (
>>     return EFI_SUCCESS;
>>   }
>>   
>> +/** Discover the attributes of the FastChannel for the specified
>> +    performance domain and the specified message.
>> +
>> +  @param[in]  This        A Pointer to SCMI_PERFORMANCE_PROTOCOL Instance.
>> +  @param[in]  DomainId    Identifier for the performance domain.
>> +  @param[in]  MessageId   Message Id of the FastChannel to discover.
>> +                          Must be one of:
>> +                           - PERFORMANCE_LIMITS_SET
>> +                           - PERFORMANCE_LIMITS_GET
>> +                           - PERFORMANCE_LEVEL_SET
>> +                           - PERFORMANCE_LEVEL_GET
>> +  @param[out] FastChannel If success, contains the FastChannel description.
>> +
>> +  @retval EFI_SUCCESS             Performance level got successfully.
>> +  @retval EFI_DEVICE_ERROR        SCP returns an SCMI error.
>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>> +  @retval EFI_TIMEOUT             Time out.
>> +  @retval EFI_UNSUPPORTED         Unsupported.
>> +**/
>> +EFI_STATUS
>> +DescribeFastchannel (
>> +  IN  SCMI_PERFORMANCE_PROTOCOL     *This,
>> +  IN  UINT32                        DomainId,
>> +  IN  SCMI_MESSAGE_ID_PERFORMANCE   MessageId,
>> +  OUT SCMI_PERFORMANCE_FASTCHANNEL  *FastChannel
>> +  )
>> +{
>> +  EFI_STATUS    Status;
>> +  SCMI_COMMAND  Cmd;
>> +  UINT32        PayloadLength;
>> +  UINT32        *ReturnValues;
>> +  UINT32        *MessageParams;
>> +
>> +  if ((This == NULL)  ||
>> +      (FastChannel == NULL))
>> +  {
>> +    return EFI_INVALID_PARAMETER;
>> +  }
>> +
>> +  Status = ScmiCommandGetPayload (&MessageParams);
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  *MessageParams++ = DomainId;
>> +  *MessageParams   = MessageId;
>> +
>> +  Cmd.ProtocolId = ScmiProtocolIdPerformance;
>> +  Cmd.MessageId  = ScmiMessageIdPerformanceDescribeFastchannel;
>> +  PayloadLength  = sizeof (DomainId) + sizeof (MessageId);
>> +
>> +  Status = ScmiCommandExecute (
>> +             &Cmd,
>> +             &PayloadLength,
>> +             &ReturnValues
>> +             );
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  CopyMem (
>> +    FastChannel,
>> +    ReturnValues,
>> +    sizeof (SCMI_PERFORMANCE_FASTCHANNEL)
>> +    );
>> +
>> +  return Status;
>> +}
>> +
>>   // Instance of the SCMI performance management protocol.
>>   STATIC CONST SCMI_PERFORMANCE_PROTOCOL  PerformanceProtocol = {
>>     PerformanceGetVersion,
>> @@ -425,7 +494,8 @@ STATIC CONST SCMI_PERFORMANCE_PROTOCOL  PerformanceProtocol = {
>>     PerformanceLimitsSet,
>>     PerformanceLimitsGet,
>>     PerformanceLevelSet,
>> -  PerformanceLevelGet
>> +  PerformanceLevelGet,
>> +  DescribeFastchannel,
>>   };
>>   
>>   /** Initialize performance management protocol and install on a given Handle.
>> diff --git a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
>> index 8e8e05d5a5f6..088182945a06 100644
>> --- a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
>> +++ b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
>> @@ -14,7 +14,7 @@
>>   
>>   #include <Protocol/ArmScmi.h>
>>   
>> -/// Arm Scmi performance protocol versions.
>> +/// Arm SCMI performance protocol versions.
>>   #define PERFORMANCE_PROTOCOL_VERSION_V1  0x10000
>>   #define PERFORMANCE_PROTOCOL_VERSION_V2  0x20000
>>   #define PERFORMANCE_PROTOCOL_VERSION_V3  0x30000
>> @@ -79,6 +79,56 @@ typedef struct {
>>     UINT32    RangeMin;
>>   } SCMI_PERFORMANCE_LIMITS;
>>   
>> +/// Doorbell Support bit.
>> +#define SCMI_PERF_FC_ATTRIB_HAS_DOORBELL  BIT0
>> +
>> +/// Performance protocol describe fastchannel
>> +typedef struct {
>> +  /// Attributes.
>> +  UINT32    Attributes;
>> +
>> +  /// Rate limit.
>> +  UINT32    RateLimit;
>> +
>> +  /// Lower 32 bits of the FastChannel address.
>> +  UINT32    ChanAddrLow;
>> +
>> +  /// Higher 32 bits of the FastChannel address.
>> +  UINT32    ChanAddrHigh;
>> +
>> +  /// Size of the FastChannel in bytes.
>> +  UINT32    ChanSize;
>> +
>> +  /// Lower 32 bits of the doorbell address.
>> +  UINT32    DoorbellAddrLow;
>> +
>> +  /// Higher 32 bits of the doorbell address.
>> +  UINT32    DoorbellAddrHigh;
>> +
>> +  /// Mask of lower 32 bits to set when writing to the doorbell register.
>> +  UINT32    DoorbellSetMaskLow;
>> +
>> +  /// Mask of higher 32 bits to set when writing to the doorbell register.
>> +  UINT32    DoorbellSetMaskHigh;
>> +
>> +  /// Mask of lower 32 bits to preserve when writing to the doorbell register.
>> +  UINT32    DoorbellPreserveMaskLow;
>> +
>> +  /// Mask of higher 32 bits to preserve when writing to the doorbell register.
>> +  UINT32    DoorbellPreserveMaskHigh;
>> +} SCMI_PERFORMANCE_FASTCHANNEL;
>> +
>> +/// SCMI Message Ids for the Performance Protocol.
>> +typedef enum {
>> +  ScmiMessageIdPerformanceDomainAttributes    = 0x3,
>> +  ScmiMessageIdPerformanceDescribeLevels      = 0x4,
>> +  ScmiMessageIdPerformanceLimitsSet           = 0x5,
>> +  ScmiMessageIdPerformanceLimitsGet           = 0x6,
>> +  ScmiMessageIdPerformanceLevelSet            = 0x7,
>> +  ScmiMessageIdPerformanceLevelGet            = 0x8,
>> +  ScmiMessageIdPerformanceDescribeFastchannel = 0xB,
>> +} SCMI_MESSAGE_ID_PERFORMANCE;
> 
> This struct appears to move in the code at the same time as having an
> entry added to it. This seems superficially unmotivated.
> However it is also moved to inside the pack(1) block, which makes no
> sense for an enum.

Yes right, I will move it out of the pack(1) section.
The reason to move the SCMI_MESSAGE_ID_PERFORMANCE definition up in the file
was that the SCMI_PERFORMANCE_DESCRIBE_FASTCHANNEL interface added in this
patch takes a SCMI_MESSAGE_ID_PERFORMANCE parameter as an argument, so the
enum needs to be defined before.
I can add a comment about this in the commit message.

Regards,
Pierre

> 
> /
>      Leif
> 
>> +
>>   #pragma pack()
>>   
>>   /** Return version of the performance management protocol supported by SCP.
>> @@ -238,6 +288,34 @@ EFI_STATUS
>>     OUT UINT32                    *Level
>>     );
>>   
>> +/** Discover the attributes of the FastChannel for the specified
>> +    performance domain and the specified message.
>> +
>> +  @param[in]  This        A Pointer to SCMI_PERFORMANCE_PROTOCOL Instance.
>> +  @param[in]  DomainId    Identifier for the performance domain.
>> +  @param[in]  MessageId   Message Id of the FastChannel to discover.
>> +                          Must be one of:
>> +                           - PERFORMANCE_LIMITS_SET
>> +                           - PERFORMANCE_LIMITS_GET
>> +                           - PERFORMANCE_LEVEL_SET
>> +                           - PERFORMANCE_LEVEL_GET
>> +  @param[out] FastChannel If success, contains the FastChannel description.
>> +
>> +  @retval EFI_SUCCESS             Performance level got successfully.
>> +  @retval EFI_DEVICE_ERROR        SCP returns an SCMI error.
>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>> +  @retval EFI_TIMEOUT             Time out.
>> +  @retval EFI_UNSUPPORTED         Unsupported.
>> +**/
>> +typedef
>> +EFI_STATUS
>> +(EFIAPI *SCMI_PERFORMANCE_DESCRIBE_FASTCHANNEL)(
>> +  IN  SCMI_PERFORMANCE_PROTOCOL       *This,
>> +  IN  UINT32                          DomainId,
>> +  IN  SCMI_MESSAGE_ID_PERFORMANCE     MessageId,
>> +  OUT SCMI_PERFORMANCE_FASTCHANNEL    *FastChannel
>> +  );
>> +
>>   typedef struct _SCMI_PERFORMANCE_PROTOCOL {
>>     SCMI_PERFORMANCE_GET_VERSION              GetVersion;
>>     SCMI_PERFORMANCE_GET_ATTRIBUTES           GetProtocolAttributes;
>> @@ -247,15 +325,7 @@ typedef struct _SCMI_PERFORMANCE_PROTOCOL {
>>     SCMI_PERFORMANCE_LIMITS_GET               LimitsGet;
>>     SCMI_PERFORMANCE_LEVEL_SET                LevelSet;
>>     SCMI_PERFORMANCE_LEVEL_GET                LevelGet;
>> +  SCMI_PERFORMANCE_DESCRIBE_FASTCHANNEL     DescribeFastchannel;
>>   } SCMI_PERFORMANCE_PROTOCOL;
>>   
>> -typedef enum {
>> -  ScmiMessageIdPerformanceDomainAttributes = 0x3,
>> -  ScmiMessageIdPerformanceDescribeLevels   = 0x4,
>> -  ScmiMessageIdPerformanceLimitsSet        = 0x5,
>> -  ScmiMessageIdPerformanceLimitsGet        = 0x6,
>> -  ScmiMessageIdPerformanceLevelSet         = 0x7,
>> -  ScmiMessageIdPerformanceLevelGet         = 0x8,
>> -} SCMI_MESSAGE_ID_PERFORMANCE;
>> -
>>   #endif /* ARM_SCMI_PERFORMANCE_PROTOCOL_H_ */
>> -- 
>> 2.25.1
>>
>>
>>
>> 
>>
>>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110513): https://edk2.groups.io/g/devel/message/110513
Mute This Topic: https://groups.io/mt/102175812/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-