Update RPMI Message Protocol chapter #35

pathakraul · 2024-07-29T01:54:44Z

This PR contains improvements to message protocol chapter.

pathakraul · 2024-07-29T02:41:21Z

@lftan I suggest getting this PR merged first before others since it changes the message format which will affect all service groups

lftan · 2024-07-31T09:25:49Z

src/message-protocol.adoc

-message request being sent to the Platform Microcontroller for a specific
-purpose. Each service may have an associated response message which enables the 
-Platform Microcontroller to provide the status or information for that request.
+RPMI message protocol defines the message format and different messages


"The RPMI Message Protocol..."

missing 'The'

lftan · 2024-07-31T09:26:19Z

src/message-protocol.adoc

+designated to perform a specific task. Multiple similar services are grouped 
+logically into a *Service Group* like Clock, Voltage, Power management, etc. 
+A service call results in a request message being sent on a transport 
+channel from application processor to platform micro-controller or vice-versa.


"channel from the application processor to the Platform Microcontroller or vice-versa."

lftan · 2024-07-31T09:29:22Z

src/message-protocol.adoc

+logically into a *Service Group* like Clock, Voltage, Power management, etc. 
+A service call results in a request message being sent on a transport 
+channel from application processor to platform micro-controller or vice-versa.
+If the request has an associated response, the platform micro-controller after


we standardize to use "Platform Microcontroller" in other chapters.

I have done the suggested changes

lftan · 2024-07-31T12:28:18Z

src/message-protocol.adoc

+
+Not every request message needs an acknowledgement due the the nature of the
+action requested by the application processor when taken by the platform 
+microcontroller may leave the application processor in a state where it cannot


change to "Platform Microcontroller", can u search all in this adoc? Got 4 places need to update this.

Also for "Application Processor".
Other adoc files use "Application Processor" instead of 'application processor'

I was not expecting having first letter in caps in between the sentence for these names. Its will look odd, since these are not acronyms

Other chapters use these now. I am ok if not use capital letter for the first letter, just need to standardize for all.
Can have another commit to update them.

lftan · 2024-07-31T12:32:04Z

src/message-protocol.adoc

-to any previous message request. Acknowledgment is dependent on the service and 
-each service dictates if acknowledgment is mandatory or not.
+Messages which carry the status of the request message after its been processed
+successfully. Acknowledgement message may also carry the additional data


"The Acknowledgement message ..."

lftan · 2024-07-31T12:34:33Z

src/message-protocol.adoc

-acknowledgment must follow byte ordering defined by the RPMI transport.
+Size of message header is fixed, but size of data is implementation defined,
+based on the transport queues layout.  All fields message must follow byte
+ordering defined by the RPMI transport.


All message fields must follow the byte order defined by the RPMI transport.

lftan · 2024-07-31T12:35:23Z

src/message-protocol.adoc

-Message header has a fixed size of `12-byte` which further contains three 
-fields each of `4-byte`. Each message gets a unique identity in an RPMI instance
-through its header.
+image::message-format.png[700,900]


Add Figure title.

lftan · 2024-07-31T12:36:25Z

src/message-protocol.adoc


-Both REQUEST and ACKNOWLEDGEMENT messages have the same message format.
+==== Message layout tables


Message Layout Tables

use capital letter for first character of each word for section title

lftan · 2024-07-31T12:37:55Z

src/message-protocol.adoc


-image::message-format.png[700,900]
+[#table_message_layout_table_example]
+.Message layout table example


Message Layout Table Example

use capital letter for first character of each word for table title.

lftan · 2024-07-31T12:38:59Z

src/message-protocol.adoc

-Both REQUEST and ACKNOWLEDGEMENT messages have the same message format.
+==== Message layout tables
+Message layout which includes header and data is represented in the form of 
+tables like below. Some columns mentioned below may be omitted from some of the


"..... tables as shown below."

lftan · 2024-07-31T12:39:14Z

src/message-protocol.adoc


-Both REQUEST and ACKNOWLEDGEMENT messages have the same message format.
+==== Message layout tables
+Message layout which includes header and data is represented in the form of 


"The message layout ..."

lftan · 2024-07-31T12:41:18Z

src/message-protocol.adoc

+
+| Every message is indexed with *Word* starting from `0` and each index 
+represents a `4-byte` portion of a message.
+| Name of a `4-byte` size portion of a message. 


We have Name for a struct, so need change description here

I dont understand this comment

lftan · 2024-07-31T12:45:26Z

src/message-protocol.adoc

+|===
+
+==== Message Header
+Message header of `8-byte` has a fixed layout which consists of multiple fields.


can change to :

The 8-byte message header has a fixed layout composed of multiple fields. Each message is uniquely identified within an RPMI instance by its header.

lftan · 2024-07-31T12:47:52Z

src/message-protocol.adoc

+
+	FLAGS[3]: DOORBELL
+	0b0: Doorbell interrupt enabled.
+	0b1: Doorbell interrupt disabled. PuC will not ring the doorbell to AP.


The PuC will not ring the doorbell to the AP.

lftan · 2024-07-31T12:50:24Z

src/message-protocol.adoc

-`FLAGS` with notification `MESSAGE_TYPE` marked. Notification messages do not 
-require any acknowledgement and how data from notification messages is utilized 
-is dependent on the implementation.
+In case of normal request which require acknowledgement, platform


"In the case of normal ...."

lftan · 2024-07-31T12:50:46Z

src/message-protocol.adoc

-require any acknowledgement and how data from notification messages is utilized 
-is dependent on the implementation.
+In case of normal request which require acknowledgement, platform
+microcontroller must preserve the `TOKEN`, `SERVICEGROUP_ID`, `SERVICE_ID` 


"the Platform Microcontroller"

lftan · 2024-07-31T12:51:27Z

src/message-protocol.adoc

-is dependent on the implementation.
+In case of normal request which require acknowledgement, platform
+microcontroller must preserve the `TOKEN`, `SERVICEGROUP_ID`, `SERVICE_ID` 
+from the request message header and use these fields in acknowledgement message


" ... in the acknowledgement message ...."

lftan · 2024-07-31T12:52:00Z

src/message-protocol.adoc

+In case of normal request which require acknowledgement, platform
+microcontroller must preserve the `TOKEN`, `SERVICEGROUP_ID`, `SERVICE_ID` 
+from the request message header and use these fields in acknowledgement message
+header. Platform microcontroller must mark the message type as 


"The Platform Microcontroller ....."

lftan · 2024-07-31T12:52:50Z

src/message-protocol.adoc

+In case of normal request which require acknowledgement, platform
+microcontroller must preserve the `TOKEN`, `SERVICEGROUP_ID`, `SERVICE_ID` 
+from the request message header and use these fields in acknowledgement message
+header. Platform microcontroller must mark the message type as 


change 'mark' to 'set' is better

lftan · 2024-07-31T12:54:07Z

src/message-protocol.adoc

+from the request message header and use these fields in acknowledgement message
+header. Platform microcontroller must mark the message type as 
+`ACKNOWLEDGEMENT` in the `FLAGS` and update the `DATALEN` depending on the 
+data supported by that acknowledgement.


".... this acknowledgement."

lftan · 2024-07-31T12:54:50Z

src/message-protocol.adoc

+header. Platform microcontroller must mark the message type as 
+`ACKNOWLEDGEMENT` in the `FLAGS` and update the `DATALEN` depending on the 
+data supported by that acknowledgement.
+In case of notification, the platform microcontroller will generate the `TOKEN`


"In the case of a notification, ..."

lftan · 2024-07-31T12:56:59Z

src/message-protocol.adoc

+In case of notification, the platform microcontroller will generate the `TOKEN`
+and set the `SERVICEGROUP_ID` and fixed `SERVICE_ID=0x00` assigned for each
+notification message in each service group and set the 
+`FLAGS` with message type as `NOTIFICATION`. Notification messages do not 


" ... with the message type"

lftan · 2024-07-31T12:58:55Z

src/message-protocol.adoc


 ==== Message Data
-Request message data format and acknowledgement data format depends on each 
+Message data for any message type if present must be multiple of `4-byte`. Each


"... must be a multiple of 4-bytes."

lftan · 2024-07-31T12:59:50Z

src/message-protocol.adoc


 ==== Message Data
-Request message data format and acknowledgement data format depends on each 
+Message data for any message type if present must be multiple of `4-byte`. Each
+of that `4-byte` portion is called a *Word*. +


"Each of these 4-bytes .... "

lftan · 2024-07-31T13:01:39Z

src/message-protocol.adoc

-Request message data format and acknowledgement data format depends on each 
+Message data for any message type if present must be multiple of `4-byte`. Each
+of that `4-byte` portion is called a *Word*. +
+Data format for request and acknowledgement message depends on each 


"The data format for the request and .."

lftan · 2024-07-31T13:02:59Z

src/message-protocol.adoc

-
-Message data formats in this specification are tabulated with a list of `32 bits`
-wide Word in each of the service groups section as depicted below.
+respective service groups. Maximum size of data each message can accommodate 


"The maximum size .."

lftan · 2024-07-31T13:04:40Z

src/message-protocol.adoc

 .Return Status Codes
 [cols="4, 2, 6", width=100%, align="center", options="header"]
 |===
 | Name				| Status Code | Description
-| RPMI_SUCCESS 			| 0 | Successful operation
+| RPMI_SUCCESS 			| 0 |  Successful operation


Unnecessary change

lftan · 2024-07-31T13:07:45Z

src/message-protocol.adoc

-Apart from status code, an acknowledgement may encode more data as response to 
-the request message and details are subsequently present in each service section 
-below.
+Acknowledgement message data contains `32-bit` STATUS code which represents the


STATUS, so that align with STATUS in line 169

lftan · 2024-07-31T13:13:09Z

src/message-protocol.adoc

+notification messages. Notification message will only be sent for events which 
+the application processor has subscribed. When there are multiple events
+supported in each service group, the application processor has to subscribe to
+each event individually through reserved enable notification service in each


I think " reserved enable notification service" can be changed to "_ENABLE_NOTIFICATION" service"

lftan · 2024-07-31T13:15:25Z

src/message-protocol.adoc


 [#img-notification-format]
 .Notification Format
 image::notification-format.png[500,600]

+==== Events
+Event consists of header containing two fields to


An event consists of a header containing two fields to identify an event:

lftan · 2024-07-31T13:17:30Z

src/message-protocol.adoc

+The number of events which can be accommodated in the message data depends on 
+the message data field size. The `DATALEN` field in the message header 
+represents the data size in bytes present in the message which is the aggregate
+of all events size. Then the application processor must parse each event and


"The Application Processor must then parse each .... "

lftan · 2024-07-31T13:18:24Z

src/message-protocol.adoc

+of all events size. Then the application processor must parse each event and
+its data according to the event header.
+
+Event data and its format are specific to each service group and details are


Event data and its format are specific to each service group, and details are provided in the respective
Service Group sections.

lftan · 2024-07-31T13:22:28Z

images/event-header.png

Can add "EVENT_HDR" for Word 0.

"EVENT_DATALEN-byte" here means in byte, rights?

If yes, then change to "EVENT_DATALEN (bytes)" is better.

lftan · 2024-07-31T13:27:17Z

src/message-protocol.adoc

 !===
-| 1 : (*EVENT_DATALEN* - 1)	| *EVENT_DATA*	| Event Data
+| 1:(*EVENT_DATALEN/4*)	| *EVENT_DATA*	| Event Data
 |===

 Above table represents the format for one event with its data. Subsequent events


"The table above shows ....."

lftan · 2024-07-31T13:28:32Z

src/message-protocol.adoc

 !===
-| 1 : (*EVENT_DATALEN* - 1)	| *EVENT_DATA*	| Event Data
+| 1:(*EVENT_DATALEN/4*)	| *EVENT_DATA*	| Event Data
 |===

 Above table represents the format for one event with its data. Subsequent events
 will be packed in the same manner. This spec does not define any ordering of 


"This specification ....."

lftan · 2024-07-31T13:29:01Z

src/message-protocol.adoc

-probable error codes for each service are also provided in each service response
-message format. AP must check for each error code and action based on that is 
-dependent on the AP.
+Below table lists all the error codes which can be returned by any service in


"The following table lists .... "

lftan · 2024-07-31T13:30:43Z

src/message-protocol.adoc


-[#table_error_codes]
+[#table_rpmi_error_codes]


This change breaks the reference link from all the services.

lftan · 2024-07-31T13:32:39Z

src/message-protocol.adoc

 .Return Status Codes
 [cols="4, 2, 6", width=100%, align="center", options="header"]
 |===
 | Name				| Status Code | Description
-| RPMI_SUCCESS 			| 0 | Successful operation
+| RPMI_SUCCESS 			| 0 |  Successful operation
 | RPMI_ERROR_FAILED 		| -1 | Failed due to general error
 | RPMI_ERROR_NOT_SUPPORTED	| -2 | Service or feature not supported
 | RPMI_ERROR_INVALID_PARAMETER 	| -3 | One or more parameters passed are 


Actually, we can use ChatGPT to check the grammar or fine-tune any sentence. You can try. :)

or can use this website: https://www.deepl.com/en/write

lftan · 2024-08-01T08:45:45Z

images/message-format.png

same here, DATALEN (bytes)

lftan · 2024-08-07T08:56:35Z

src/message-protocol.adoc

-dependent on the AP.
+=== Return Error Codes
+The table below lists all the error codes that can be returned by any service
+in the in the `STATUS' word of the acknowledgement message.


after generating pdf doc, it shows word `STATUS'.

Should be STATUS, should be "`".

Signed-off-by: Rahul Pathak <[email protected]>

lftan reviewed Jul 31, 2024

View reviewed changes

pathakraul force-pushed the messageformat branch from edfd82c to 3af2531 Compare July 31, 2024 09:48

lftan reviewed Jul 31, 2024

View reviewed changes

lftan reviewed Aug 1, 2024

View reviewed changes

images/message-format.png Outdated

Copy link

Collaborator

lftan Aug 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, DATALEN (bytes)

pathakraul force-pushed the messageformat branch 5 times, most recently from a06b0c8 to a0acb19 Compare August 7, 2024 04:04

lftan reviewed Aug 7, 2024

View reviewed changes

pathakraul force-pushed the messageformat branch from a0acb19 to a85d6e6 Compare August 7, 2024 09:12

Update RPMI Message Protocol chapter

96c3d48

Signed-off-by: Rahul Pathak <[email protected]>

pathakraul force-pushed the messageformat branch from a85d6e6 to 96c3d48 Compare August 8, 2024 02:53

pathakraul mentioned this pull request Aug 8, 2024

[REVIEW] RPMI Message Protocol #6

Closed

lftan merged commit e8f9258 into riscv-non-isa:main Aug 8, 2024
1 check passed

pathakraul deleted the messageformat branch November 9, 2024 14:47


		Both REQUEST and ACKNOWLEDGEMENT messages have the same message format.
		==== Message layout tables


		[#table_error_codes]
		[#table_rpmi_error_codes]

Update RPMI Message Protocol chapter #35

Update RPMI Message Protocol chapter #35

Conversation

pathakraul commented Jul 29, 2024

pathakraul commented Jul 29, 2024

Choose a reason for hiding this comment

lftan Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lftan Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lftan Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lftan Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lftan Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lftan Jul 31, 2024 •

edited

Loading

lftan Jul 31, 2024 •

edited

Loading

lftan Jul 31, 2024 •

edited

Loading

lftan Jul 31, 2024 •

edited

Loading

lftan Jul 31, 2024 •

edited

Loading