ARC Feedback Updates and other changes #80

pathakraul · 2024-12-06T04:29:45Z

No description provided.

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

pathakraul · 2024-12-06T04:31:33Z

lftan · 2024-12-06T06:42:41Z

src/service-groups.adoc

-The <<table_service_groups>> below list all standard RPMI service groups
-defined by this specification.
+The RPMI specification also provides experimental service group IDs space
+for development of service group until a standard service group ID is not


should remove "not"?

lftan · 2024-12-06T06:43:09Z

src/service-groups.adoc

+The RPMI specification also provides experimental service group IDs space
+for development of service group until a standard service group ID is not
+allocated. The platform vendors can provide implementation specific RPMI
+service groups The <<table_service_groups>> below list all standard RPMI


extra "The"

lftan · 2024-12-06T06:52:03Z

src/message-protocol.adoc

@@ -166,6 +166,36 @@ values as corresponding fields in the RPMI request message. The `DATALEN`
 field of the RPMI acknowledgement message must be set according to the data
 carried by this acknowledgement.

+NOTE: The message token will help the application processors to keep track of
+the origin of the request when it receives  a response. This is  useful when the


extra spaces after 'receives' and after 'is'?

lftan · 2024-12-06T06:53:34Z

src/message-protocol.adoc

+distinguish which response belongs to which request.
+
+NOTE: The RPMI specification recommends monotonically increasing token numbers
+and the token number can be initialized from any value without any constarints.


Typo 'constarints" --> constraints

lftan · 2024-12-06T06:59:27Z

src/message-protocol.adoc

+NOTE: The flags[3] bit can be used for a particular message or for the entire
+lifecycle of RPMI message communication. For example if the application
+processor and the platform microcontroller are capable for MSIs and the application
+processor has configured MSI details via defined service in Section 4.1.10 then


Shouldn't hardcoded "Section 4.1.10", create a label/tag at section Section 4.1.10 (BASE_SET_MSI), then reference it with <<BASE_SET_MSI-lable>>

lftan · 2024-12-06T07:01:05Z

src/transport.adoc

@@ -38,6 +38,11 @@ the platform microcontroller can avoid implementing the P2A channel.
 The current RPMI specification only defines a shared memory based transport but
 other transport types can be added in the future.

+NOTE: The shared memory for RPMI transport and fast-channels allocated
+in DRAM or in on-chip RAM will require memory attributes configuration. These
+memory attributes also called PMA(Physical Memory Attributes) are defined in


add space before (

lftan · 2024-12-06T07:05:19Z

src/transport.adoc

-and only a few service groups may support fast-channels. A service group
-that supports fast-channels:
+NOTE: If the platform supports PLIC, the platform need to provide a MMIO
+register to inject an edge-triggered interrupt.


Not very understand this.
edge-triggered interrupt is not a must for PLIC, right?
and this MMIO register is a hardcoded csr address?

Edge triggered interrupt is preferred in the Linux. And MMIO register is not a CSR address. Write to this MMIO register if going to trigger the interrupt to PLIC causing triggering an edge triggered interrupt into AP. What do you suggest?

lftan · 2024-12-06T07:07:56Z

src/transport.adoc

+
+The platform must setup the PMA for the shared memory used for the fast-channels.
+
+NOTE: Its possible that the application processor and the platform


lftan · 2024-12-06T07:11:10Z

src/transport.adoc

+others issues which may lead to race conditions. To avoid the caching
+side-effects, the platform can configure the memory attribute of the shared
+memory as non-cacheable or IO memory for both the application processor and the
+platform microcontroller and implementation can perform manual cache maintenance


Split the long sentence. Eg:
Additionally, the implementation can perform manual cache maintenance using cache flush and invalidate operations.

lftan · 2024-12-06T07:12:03Z

src/transport.adoc

-NOTE: To avoid the caching side-effects, the platform can configure the shared
+The platform must setup the PMA for the shared memory used for RPMI transport.
+
+NOTE: Its possible that the application processor and the platform


Repeat the same Note here?

That two notes are separately provided one for fastchannel and one for the RPMI transport shared memory

lftan · 2024-12-06T07:12:18Z

src/transport.adoc

 memory as non-cacheable or IO memory for both the application processor and the
-platform microcontroller.
+platform microcontroller and implementation can perform manual cache maintenance


Same comment as above.

lftan · 2024-12-06T07:16:08Z

src/srvgrp-base.adoc

@@ -65,15 +65,28 @@ The following table lists the services in the BASE service group:
 |===

 ==== RPMI Implementation IDs
+The RPMI specification defines space for standard implementation IDs and for
+experimental implementation IDs. The experimental implementation IDs can be used
+by the implementations until a standard implementation ID is not assigned to it.


Remove 'not'.

"...until a standard implementation ID is assigned to it."

- Update Transport chapter with improvements - Fix typos in the Intro and Service Group chapter - Update high level architecture diagram - Update Message Protocol chapter with improvements Signed-off-by: Rahul Pathak <[email protected]>

pathakraul · 2024-12-08T04:32:49Z

If there are no comments then I will merge this tomorrow Monday morning and release it to ARC for review

avpatel · 2024-12-08T14:19:57Z

Looks good to me.

lftan · 2024-12-09T02:30:52Z

Looks good, merged.

rpmi: Add Experimental service groups IDs space

69e78a3

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

pathakraul requested a review from lftan December 6, 2024 04:30

lftan reviewed Dec 6, 2024

View reviewed changes

pathakraul force-pushed the rpathak_arc_review_79 branch from b6eeea6 to 1387d5d Compare December 7, 2024 01:54

lftan merged commit 2674515 into riscv-non-isa:main Dec 9, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ARC Feedback Updates and other changes #80

ARC Feedback Updates and other changes #80

pathakraul commented Dec 6, 2024

pathakraul commented Dec 6, 2024

lftan Dec 6, 2024

lftan Dec 6, 2024

lftan Dec 6, 2024

lftan Dec 6, 2024

lftan Dec 6, 2024

lftan Dec 6, 2024

lftan Dec 6, 2024

pathakraul Dec 7, 2024 •

edited

Loading

lftan Dec 6, 2024

lftan Dec 6, 2024

lftan Dec 6, 2024

pathakraul Dec 7, 2024

lftan Dec 6, 2024

lftan Dec 6, 2024

pathakraul commented Dec 8, 2024 •

edited

Loading

avpatel commented Dec 8, 2024

lftan commented Dec 9, 2024


		The platform must setup the PMA for the shared memory used for the fast-channels.

		NOTE: Its possible that the application processor and the platform

ARC Feedback Updates and other changes #80

ARC Feedback Updates and other changes #80

Conversation

pathakraul commented Dec 6, 2024

pathakraul commented Dec 6, 2024

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

pathakraul Dec 7, 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

pathakraul commented Dec 8, 2024 • edited Loading

avpatel commented Dec 8, 2024

lftan commented Dec 9, 2024

pathakraul Dec 7, 2024 •

edited

Loading

pathakraul commented Dec 8, 2024 •

edited

Loading