Re: [PATCH V1 3/5] accel/amd_vpci: Add Remote Management(RM) queue infrastructure

[Yidong Zhang <yidong.zhang@amd.com>]

On 11/11/25 01:44, Christophe JAILLET wrote:
> Caution: This message originated from an External Source. Use proper 
> caution when opening attachments, clicking links, or responding.
> 
> 
> Le 11/11/2025 à 02:15, David Zhang a écrit :
>> This patch introduces a Remote Management (RM) queue service, which
>> provides a way to communicate between the management PCIe function (PF0)
>> and the embedded firmware running on AMD Versal SoCs.
>>
>> The RM service implements a hardware-based ring buffer for bidirectional
>> command and response exchange between the host driver and the firmware.
>>
>> This patch adds the core infrastructure for:
>>    - Initializing and managing the RM queue
>>    - Submitting commands to the embedded firmware
>>    - Polling for command completion
>>
>> Subsequent patches will integrate the infrastructure with the firmware
>> management logic to enable firmware download, status query, and other
>> control operations.
>>
>> Co-developed-by: Nishad Saraf <nishads@amd.com>
>> Signed-off-by: Nishad Saraf <nishads@amd.com>
>> Signed-off-by: David Zhang <yidong.zhang@amd.com>
> 
> ...
> 
>> diff --git a/drivers/accel/amd_vpci/versal-pci-rm-service.h 
>> b/drivers/accel/amd_vpci/versal-pci-rm-service.h
>> new file mode 100644
>> index 000000000000..d2397a1a672c
>> --- /dev/null
>> +++ b/drivers/accel/amd_vpci/versal-pci-rm-service.h
> 
> ...
> 
>> +#define RM_CMD_ID_MIN                        1
>> +#define RM_CMD_ID_MAX                        (BIT(17) - 1)
>> +#define RM_CMD_SQ_HDR_OPS_MSK                GENMASK(15, 0)
>> +#define RM_CMD_SQ_HDR_SIZE_MSK               GENMASK(14, 0)
>> +#define RM_CMD_SQ_SLOT_SIZE          SZ_512
>> +#define RM_CMD_CQ_SLOT_SIZE          SZ_16
>> +#define RM_CMD_CQ_BUFFER_SIZE                (1024 * 1024)
> 
> SZ_1M to be consistent with other xx_SIZE macro?

Thank you so much. I will fix this.

> 
>> +#define RM_CMD_CQ_BUFFER_OFFSET              0x0
>> +#define RM_CMD_LOG_PAGE_TYPE_MASK    GENMASK(15, 0)
>> +#define RM_CMD_VMR_CONTROL_MSK               GENMASK(10, 8)
>> +#define RM_CMD_VMR_CONTROL_PS_MASK   BIT(9)
>> +
>> +#define RM_CMD_WAIT_CONFIG_TIMEOUT   msecs_to_jiffies(10 * 1000)
>> +#define RM_CMD_WAIT_DOWNLOAD_TIMEOUT msecs_to_jiffies(300 * 1000)
> 
> secs_to_jiffies() maybe to be less verbose?

I will fix this.

> 
>> +
>> +#define RM_COMPLETION_TIMER          (HZ / 10)
>> +#define RM_HEALTH_CHECK_TIMER                (HZ)
> 
> ...
> 
> CJ