| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250402124227.GB23033@nxa18884-linux>
Date: Wed, 2 Apr 2025 20:42:27 +0800
From: Peng Fan <peng.fan@....nxp.com>
To: Cristian Marussi <cristian.marussi@....com>
Cc: Sudeep Holla <sudeep.holla@....com>, Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Dan Carpenter <dan.carpenter@...aro.org>,
linux-kernel@...r.kernel.org, arm-scmi@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, imx@...ts.linux.dev,
devicetree@...r.kernel.org, Peng Fan <peng.fan@....com>
Subject: Re: [PATCH v3 1/7] firmware: arm_scmi: imx: Add LMM and CPU
documentation
Hi Cristian,
Thanks for reviewing the patch.
On Tue, Apr 01, 2025 at 03:51:40PM +0100, Cristian Marussi wrote:
>On Mon, Mar 03, 2025 at 10:53:22AM +0800, Peng Fan (OSS) wrote:
>> From: Peng Fan <peng.fan@....com>
>>
>> Add i.MX95 Logical Machine Management and CPU Protocol documentation.
>>
>> Signed-off-by: Peng Fan <peng.fan@....com>
>> ---
>> drivers/firmware/arm_scmi/vendors/imx/imx95.rst | 801 ++++++++++++++++++++++++
>> 1 file changed, 801 insertions(+)
>>
>> diff --git a/drivers/firmware/arm_scmi/vendors/imx/imx95.rst b/drivers/firmware/arm_scmi/vendors/imx/imx95.rst
>> index b2dfd6c46ca2f5f12f0475c24cb54c060e9fa421..74326bf2ea8586282a735713e0ab7eb90ccce8ff 100644
>> --- a/drivers/firmware/arm_scmi/vendors/imx/imx95.rst
>> +++ b/drivers/firmware/arm_scmi/vendors/imx/imx95.rst
>> @@ -32,6 +32,501 @@ port, and deploy the SM on supported processors.
>> The SM implements an interface compliant with the Arm SCMI Specification
>> with additional vendor specific extensions.
>>
>> +SCMI_LMM: System Control and Management Logical Machine Management Vendor Protocol
>> +==================================================================================
>> +
>> +This protocol is intended for boot, shutdown, and reset of other logical
>> +machines (LM). It is usually used to allow one LM(e.g. OSPM) to manage
>> +another LM which is usually an offload or accelerator engine.. Notifications
>> +from this protocol can also be used to manage a communication link to another
>> +LM. The LMM protocol provides functions to:
>> +
>> +- Describe the protocol version.
>> +- Discover implementation attributes.
>> +- Discover the LMs defined in the system.
>> +- Boot an LM.
>> +- Shutdown an LM (gracefully or forcibly).
>> +- Reset an LM (gracefully or forcibly).
>> +- Wake an LM from suspend.
>> +- Suspend an LM (gracefully).
>> +- Read boot/shutdown/reset information for an LM.
>> +- Get notifications when an LM boots or shuts down (e.g. LM[X] requested
>> + notification of LM[Y] boots or shuts down, when LM[Y] boots or shuts down,
>> + SCMI firmware will send notification to LM[X]).
>> +
>> +'Graceful' means asking LM itself to shutdown/reset/etc (e.g. sending
>> +notification to Linux, Then Linux reboots or powers down itself). It is async
>> +command that the SUCCESS of the command just means the command successfully
>> +return, not means reboot/reset successfully finished.
>> +'Forceful' means the SM will force shutdown/reset/etc the LM. It is sync
>> +command that the SUCCESS of the command means the LM has been successfully
>> +shutdown/reset/etc.
>> +If the commands not have Graceful/Forceful flag settings, such as WAKE, SUSEND,
>> +it is async command.
>> +
>
>Hi,
>
>> +Commands:
>> +_________
>> +
>> +PROTOCOL_VERSION
>> +~~~~~~~~~~~~~~~~
>> +
>> +message_id: 0x0
>> +protocol_id: 0x80
>> +This command is mandatory.
>> +
>
>Good that you added the mandatory/optional description..
>
>> ++---------------+--------------------------------------------------------------+
>> +|Return values |
>> ++---------------+--------------------------------------------------------------+
>> +|Name |Description |
>> ++---------------+--------------------------------------------------------------+
>> +|int32 status | See ARM SCMI Specification for status code definitions. |
>> ++---------------+--------------------------------------------------------------+
>> +|uint32 version | For this revision of the specification, this value must be |
>> +| | 0x10000. |
>> ++---------------+--------------------------------------------------------------+
>> +
>> +PROTOCOL_ATTRIBUTES
>> +~~~~~~~~~~~~~~~~~~~
>> +
>> +message_id: 0x1
>> +protocol_id: 0x80
>> +This command is mandatory.
>> +
>> ++------------------+-----------------------------------------------------------+
>> +|Return values |
>> ++------------------+-----------------------------------------------------------+
>> +|Name |Description |
>> ++------------------+-----------------------------------------------------------+
>> +|int32 status | See ARM SCMI Specification for status code definitions. |
>> ++------------------+-----------------------------------------------------------+
>> +|uint32 attributes |Protocol attributes: |
>> +| |Bits[31:8] Reserved, must be zero. |
>> +| |Bits[7:0] Number of Logical Machines |
>> ++------------------+-----------------------------------------------------------+
>> +
>> +PROTOCOL_MESSAGE_ATTRIBUTES
>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> +
>> +message_id: 0x2
>> +protocol_id: 0x80
>> +This command is mandatory.
>> +
>> ++------------------+-----------------------------------------------------------+
>> +|Return values |
>> ++------------------+-----------------------------------------------------------+
>> +|Name |Description |
>> ++------------------+-----------------------------------------------------------+
>> +|int32 status |SUCCESS: in case the message is implemented and available |
>> +| |to use. |
>> +| |NOT_FOUND: if the message identified by message_id is |
>> +| |invalid or not implemented |
>> ++------------------+-----------------------------------------------------------+
>> +|uint32 attributes |Flags that are associated with a specific function in the |
>> +| |protocol. For all functions in this protocol, this |
>> +| |parameter has a value of 0 |
>> ++------------------+-----------------------------------------------------------+
>> +
>> +LMM_ATTRIBUTES
>> +~~~~~~~~~~~~~~
>> +
>> +message_id: 0x3
>> +protocol_id: 0x80
>> +This command is mandatory.
>> +
>> ++------------------+-----------------------------------------------------------+
>> +|Parameters |
>> ++------------------+-----------------------------------------------------------+
>> +|Name |Description |
>> ++------------------+-----------------------------------------------------------+
>> +|uint32 lmid |ID of the Logical Machine |
>> ++------------------+-----------------------------------------------------------+
>> +|Return values |
>> ++------------------+-----------------------------------------------------------+
>> +|Name |Description |
>> ++------------------+-----------------------------------------------------------+
>> +|int32 status |SUCCESS: if valid attributes are returned. |
>> +| |NOT_FOUND: if lmId not points to a valid logical machine. |
>> +| |DENIED: if the agent does not have permission to get info |
>> +| |for the LM specified by lmid. |
>
>..mmmmm... regardig this next field...
>
>> ++------------------+-----------------------------------------------------------+
>> +|uint32 lmid |Identifier of the LM whose identification is requested. |
>> +| |This field is: Populated with the lmId of the calling |
>> +| |agent, when the lmId parameter passed via the function is |
>> +| |0xFFFFFFFF. Identical to the lmId field passed via the |
>> +| |calling parameters, in all other cases |
>
>In V2 there was an issue with the description and you told me
>
> >> ++------------------+-----------------------------------------------------------+
> > >> +|Parameters |
> > >> ++------------------+-----------------------------------------------------------+
> > >> +|Name |Description |
> > >> ++------------------+-----------------------------------------------------------+
> > >> +|uint32 lmid |ID of the Logical Machine |
> > >> ++------------------+-----------------------------------------------------------+
> > >> +|Return values |
> > >> ++------------------+-----------------------------------------------------------+
> > >> +|Name |Description |
> > >> ++------------------+-----------------------------------------------------------+
> > >> +|int32 status |SUCCESS: if valid attributes are returned. |
> > >> +| |NOT_FOUND: if lmId not points to a valid logical machine. |
> > >> +| |DENIED: if the agent does not have permission to get info |
> > >> +| |for the LM specified by lmid. |
> > >> ++------------------+-----------------------------------------------------------+
> > >> +|uint32 attributes | Bits[31:8] reserved. |
> > >> +| | Bits[7:0] Number of Logical Machines. |
> > >
> > >...BUT this returns again the number of LMs while asking the attributes
> > >of a specific LM ? .... is it a typo or what ? ...if it is just as a
> > >sort of placeholder for when you'll have really LM's attributes to show,
> > >consider that once this is documented and supported in this version of
> > >your vendor protocol it will be needed to be kept and maintained...maybe
> > >better just to declare this as zero in this version of the protocol if
> > >you dont really have anything for this command in this version...(like
> > >many times are defined the attributes fields in PROTOCOL_MESSAGE_ATTRIBUTES
> > >above, if you really know you could want/need this command in the
> > >future...is it used now ?
> >
> > My bad. This should be updated with below
> > +------------------+-----------------------------------------------------------+
> > |uint32 attributes | Bits[31:0] reserved. must be zero |
> > +------------------+-----------------------------------------------------------+
> > |uint32 state | Current state of the LM |
> > +------------------+-----------------------------------------------------------+
> > |uint32 errStatus | Last error status recorded |
> > +------------------+-----------------------------------------------------------+
> > |char name[16] | A NULL terminated ASCII string with the LM name, of up |
> > | | to 16 bytes |
> > +------------------+-----------------------------------------------------------+
>
>...so it seems to me that the above lmid is a new addition that was not
>mentioned ... bad cut&paste ? it does NOT seem to make so much
>sense...or I am missing something
Sorry for not made it clear in V2. This V3 is correct.
In https://github.com/nxp-imx/imx-sm code: sm/rpc/scmi/rpc_scmi_lmm.c
There is lmId in return.
/* Response type for LmmAttributes() */
typedef struct
{
/* Header word */
uint32_t header;
/* Return status */
int32_t status;
/* Return LM */
uint32_t lmId;
/* LM attributes */
uint32_t attributes;
/* Current state of the LM */
uint32_t state;
/* Last error status recorded */
int32_t errStatus;
/* ASCII name string */
uint8_t name[LMM_MAX_NAME];
} msg_tlmm3_t;
>
>Other than this, LGTM.
>
>Reviewed-by: Cristian Marussi <cristian.marussi@....com>
Appreciate.
Thanks,
Peng
>
>Thanks,
>Cristian
>
Powered by blists - more mailing lists