ArmPkg/ArmScmiDxe: Fix SCMI param overwrite in multi-transaction scenario

Fix an issue where input parameters in SCMI messages may be overwritten by
return values during repeated transactions when retrieving large data sets.

This issue affects:
1. ClockDescribeRates: when the number of clock rates exceeds the transfer
   limit. According to the SCMI specification (Section 4.6.2.5), the first
   and second parameters are initially used to pass clock_id and rate_index.
   However, due to SCMI’s shared memory communication mechanism, these same
   memory locations are later reused to return status and num_rate_flags.

2. PerformanceDescribeLevels: when the number of performance levels is too
   large to return in a single response. As described in Section 4.5.3.5,
   the first and second parameters are initially used for domain_id and
   level_index, but are overwritten with status and num_levels in the return.

Because SCMI reuses the same shared memory buffer for both input and output,
the return values can override input parameters if the buffer is not properly
re-initialized before each request. This patch ensures that the first and
second parameters are correctly set before every transaction to preserve
input integrity and ensure correct protocol behavior.

Signed-off-by: jie.fu <jie.fu@cixtech.com>
This commit is contained in:
Johnny.Fan 2025-04-15 10:33:55 +08:00 committed by mergify[bot]
parent 8406e672e8
commit f96d38f432
2 changed files with 12 additions and 8 deletions

View file

@ -186,7 +186,8 @@ ClockDescribeRates (
UINT32 PayloadLength;
SCMI_COMMAND Cmd;
UINT32 *MessageParams;
UINT32 *MessageParams1;
UINT32 *MessageParams2;
CLOCK_DESCRIBE_RATES *DescribeRates;
CLOCK_RATE_DWORD *Rate;
@ -199,7 +200,7 @@ ClockDescribeRates (
RequiredArraySize = 0;
RateIndex = 0;
Status = ScmiCommandGetPayload (&MessageParams);
Status = ScmiCommandGetPayload (&MessageParams1);
if (EFI_ERROR (Status)) {
return Status;
}
@ -207,10 +208,11 @@ ClockDescribeRates (
Cmd.ProtocolId = ScmiProtocolIdClock;
Cmd.MessageId = ScmiMessageIdClockDescribeRates;
*MessageParams++ = ClockId;
MessageParams2 = MessageParams1 + 1;
do {
*MessageParams = RateIndex;
*MessageParams1 = ClockId;
*MessageParams2 = RateIndex;
// Set Payload length, note PayloadLength is a IN/OUT parameter.
PayloadLength = sizeof (ClockId) + sizeof (RateIndex);

View file

@ -160,7 +160,8 @@ PerformanceDescribeLevels (
EFI_STATUS Status;
UINT32 PayloadLength;
SCMI_COMMAND Cmd;
UINT32 *MessageParams;
UINT32 *MessageParams1;
UINT32 *MessageParams2;
UINT32 LevelIndex;
UINT32 RequiredSize;
UINT32 LevelNo;
@ -169,7 +170,7 @@ PerformanceDescribeLevels (
PERF_DESCRIBE_LEVELS *Levels;
Status = ScmiCommandGetPayload (&MessageParams);
Status = ScmiCommandGetPayload (&MessageParams1);
if (EFI_ERROR (Status)) {
return Status;
}
@ -177,13 +178,14 @@ PerformanceDescribeLevels (
LevelIndex = 0;
RequiredSize = 0;
*MessageParams++ = DomainId;
MessageParams2 = MessageParams1 + 1;
Cmd.ProtocolId = ScmiProtocolIdPerformance;
Cmd.MessageId = ScmiMessageIdPerformanceDescribeLevels;
do {
*MessageParams = LevelIndex;
*MessageParams1 = DomainId;
*MessageParams2 = LevelIndex;
// Note, PayloadLength is an IN/OUT parameter.
PayloadLength = sizeof (DomainId) + sizeof (LevelIndex);