Add documentation for the RPMSG Based Virtual Ethernet Driver (rpmsg-eth).
The documentation describes the driver's architecture, shared memory
layout, RPMSG communication protocol, and requirements for vendor firmware
to interoperate with the driver. It details the use of a magic number for
shared memory validation, outlines the information exchanged between the
host and remote processor, and provides a how-to guide for vendors to
implement compatible firmware.

Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 .../device_drivers/ethernet/index.rst         |   1 +
 .../device_drivers/ethernet/rpmsg_eth.rst     | 339 ++++++++++++++++++
 2 files changed, 340 insertions(+)
 create mode 100644 Documentation/networking/device_drivers/ethernet/rpmsg_eth.rst

diff --git a/Documentation/networking/device_drivers/ethernet/index.rst b/Documentation/networking/device_drivers/ethernet/index.rst
index 40ac552641a3..941f60585ee4 100644
--- a/Documentation/networking/device_drivers/ethernet/index.rst
+++ b/Documentation/networking/device_drivers/ethernet/index.rst
@@ -61,6 +61,7 @@ Contents:
    wangxun/txgbevf
    wangxun/ngbe
    wangxun/ngbevf
+   rpmsg_eth
 
 .. only::  subproject and html
 
diff --git a/Documentation/networking/device_drivers/ethernet/rpmsg_eth.rst b/Documentation/networking/device_drivers/ethernet/rpmsg_eth.rst
new file mode 100644
index 000000000000..70c13deb31ea
--- /dev/null
+++ b/Documentation/networking/device_drivers/ethernet/rpmsg_eth.rst
@@ -0,0 +1,339 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===================================
+RPMSG Based Virtual Ethernet Driver
+===================================
+
+Overview
+========
+
+The RPMSG Based Virtual Ethernet Driver provides a virtual Ethernet interface for
+communication between a host processor and a remote processor using the RPMSG
+framework. This driver enables Ethernet-like packet transmission and reception
+over shared memory, facilitating inter-core communication in systems with
+heterogeneous processors.
+
+The driver is designed to work with the RPMSG framework, which is part of the
+Linux Remote Processor (remoteproc) subsystem. It uses shared memory for data
+exchange and supports features like multicast address management, dynamic MAC
+address assignment, and efficient packet processing using NAPI.
+
+This driver is generic and can be used by any vendor. Vendors can develop their
+own firmware for the remote processor to make it compatible with this driver.
+The firmware must adhere to the shared memory layout, RPMSG communication
+protocol, and data exchange requirements described in this documentation.
+
+Key Features
+============
+
+- Virtual Ethernet interface using RPMSG.
+- Shared memory-based packet transmission and reception.
+- Support for multicast address management.
+- Dynamic MAC address assignment.
+- NAPI (New API) support for efficient packet processing.
+- State machine for managing interface states.
+- Workqueue-based asynchronous operations.
+- Support for notifications and responses from the remote processor.
+
+Magic Number
+============
+
+A **magic number** is used in the shared memory layout to validate that the
+memory region is correctly initialized and accessible by both the host and the
+remote processor. This value is a unique constant (for example,
+``0xABCDABCD``) that is written to specific locations (such as the head and
+tail structures) in the shared memory by the firmware and checked by the Linux
+driver during the handshake process.
+
+Purpose of the Magic Number
+---------------------------
+
+- **Validation:** Ensures that the shared memory region has been properly set up
+  and is not corrupted or uninitialized.
+- **Synchronization:** Both the host and remote processor must agree on the
+  magic number value, which helps detect mismatches in memory layout or protocol
+  version.
+- **Error Detection:** If the driver detects an incorrect magic number during
+  initialization or runtime, it can abort the handshake and report an error,
+  preventing undefined behavior.
+
+Implementation Details
+----------------------
+
+- The magic number is defined as a macro in the driver source (e.g.,
+  ``#define RPMSG_ETH_SHM_MAGIC_NUM 0xABCDABCD``).
+- The firmware must write this value to the ``magic_num`` field of the head and
+  tail structures in the shared memory region.
+- During the handshake, the Linux driver reads these fields and compares them to
+  the expected value. If any mismatch is detected, the driver will log an error
+  and refuse to proceed.
+
+Example Usage in Shared Memory
+------------------------------
+
+.. code-block:: text
+
+      Shared Memory Layout:
+      ---------------------------
+      |   MAGIC_NUM (0xABCDABCD) |   <-- rpmsg_eth_shm_head
+      |          HEAD            |
+      ---------------------------
+      |   MAGIC_NUM (0xABCDABCD) |   <-- rpmsg_eth_shm_tail
+      |          TAIL            |
+      ---------------------------
+
+The magic number must be present in both the head and tail structures for the
+handshake to succeed.
+
+Firmware developers must ensure that the correct magic number is written to the
+appropriate locations in shared memory before the Linux driver attempts to
+initialize the interface.
+
+Shared Memory Layout
+====================
+
+The RPMSG Based Virtual Ethernet Driver uses a shared memory region to exchange
+data between the host and the remote processor. The shared memory is divided
+into transmit and receive regions, each with its own `head` and `tail` pointers
+to track the buffer state.
+
+Shared Memory Parameters
+------------------------
+
+The following parameters are exchanged between the host and the firmware to
+configure the shared memory layout:
+
+1. **num_pkt_bufs**:
+
+   - The total number of packet buffers available in the shared memory.
+   - This determines the maximum number of packets that can be stored in the
+     shared memory at any given time.
+
+2. **buff_slot_size**:
+
+   - The size of each buffer slot in the shared memory.
+   - This includes space for the packet length, metadata, and the actual packet
+     data.
+
+3. **base_addr**:
+
+   - The base address of the shared memory region.
+   - This is the starting point for accessing the shared memory.
+
+4. **tx_offset**:
+
+   - The offset from the `base_addr` where the transmit buffers begin.
+   - This is used by the host to write packets for transmission.
+
+5. **rx_offset**:
+
+   - The offset from the `base_addr` where the receive buffers begin.
+   - This is used by the host to read packets received from the remote
+     processor.
+
+Shared Memory Structure
+-----------------------
+
+The shared memory layout is as follows:
+
+.. code-block:: text
+
+      Shared Memory Layout:
+      ---------------------------
+      |        MAGIC_NUM        |   rpmsg_eth_shm_head
+      |          HEAD           |
+      ---------------------------
+      |        MAGIC_NUM        |
+      |        PKT_1_LEN        |
+      |          PKT_1          |
+      ---------------------------
+      |           ...           |
+      ---------------------------
+      |        MAGIC_NUM        |
+      |          TAIL           |   rpmsg_eth_shm_tail
+      ---------------------------
+
+1. **MAGIC_NUM**:
+
+   - A unique identifier used to validate the shared memory region.
+   - Ensures that the memory region is correctly initialized and accessible.
+
+2. **HEAD Pointer**:
+
+   - Tracks the start of the buffer for packet transmission or reception.
+   - Updated by the producer (host or remote processor) after writing a packet.
+
+3. **TAIL Pointer**:
+
+   - Tracks the end of the buffer for packet transmission or reception.
+   - Updated by the consumer (host or remote processor) after reading a packet.
+
+4. **Packet Buffers**:
+
+   - Each packet buffer contains:
+
+      - **Packet Length**: A 4-byte field indicating the size of the packet.
+      - **Packet Data**: The actual Ethernet frame data.
+
+5. **Buffer Size**:
+
+   - Each buffer has a fixed size defined by `RPMSG_ETH_BUFFER_SIZE`, which
+     includes space for the packet length and data.
+
+Buffer Management
+-----------------
+
+- The host and remote processor use a circular buffer mechanism to manage the shared memory.
+- The `head` and `tail` pointers are used to determine the number of packets available for processing:
+
+   .. code-block:: c
+
+         num_pkts = head - tail;
+         num_pkts = num_pkts >= 0 ? num_pkts : (num_pkts + max_buffers);
+
+- The producer writes packets to the buffer and increments the `head` pointer.
+- The consumer reads packets from the buffer and increments the `tail` pointer.
+
+RPMSG Communication
+===================
+
+The driver uses RPMSG channels to exchange control messages between the host and
+the remote processor. These messages are used to manage the state of the
+Ethernet interface, configure settings, notify events, and exchange runtime
+information.
+
+Information Exchanged Between RPMSG Channels
+--------------------------------------------
+
+1. **Requests from Host to Remote Processor**:
+
+   - `RPMSG_ETH_REQ_SHM_INFO`: Request shared memory information, such as
+     ``num_pkt_bufs``, ``buff_slot_size``, ``base_addr``, ``tx_offset``, and
+     ``rx_offset``.
+   - `RPMSG_ETH_REQ_SET_MAC_ADDR`: Set the MAC address of the Ethernet
+     interface.
+   - `RPMSG_ETH_REQ_ADD_MC_ADDR`: Add a multicast address to the remote
+     processor's filter list.
+   - `RPMSG_ETH_REQ_DEL_MC_ADDR`: Remove a multicast address from the remote
+     processor's filter list.
+
+2. **Responses from Remote Processor to Host**:
+
+   - `RPMSG_ETH_RESP_SET_MAC_ADDR`: Acknowledge the MAC address configuration.
+   - `RPMSG_ETH_RESP_ADD_MC_ADDR`: Acknowledge the addition of a multicast
+     address.
+   - `RPMSG_ETH_RESP_DEL_MC_ADDR`: Acknowledge the removal of a multicast
+     address.
+   - `RPMSG_ETH_RESP_SHM_INFO`: Respond with shared memory information such as
+     ``num_pkt_bufs``, ``buff_slot_size``, ``base_addr``, ``tx_offset``, and
+     ``rx_offset``.
+
+3. **Notifications from Remote Processor to Host**:
+
+   - `RPMSG_ETH_NOTIFY_PORT_UP`: Notify that the Ethernet port is up and ready
+     for communication.
+   - `RPMSG_ETH_NOTIFY_PORT_DOWN`: Notify that the Ethernet port is down.
+   - `RPMSG_ETH_NOTIFY_PORT_READY`: Notify that the Ethernet port is ready for
+     configuration.
+   - `RPMSG_ETH_NOTIFY_REMOTE_READY`: Notify that the remote processor is ready
+     for communication.
+
+4. **Runtime Information Exchanged**:
+
+   - **Link State**: Notifications about link state changes (e.g., link up or
+     link down).
+   - **Statistics**: Runtime statistics such as transmitted/received packets,
+     errors, and dropped packets.
+   - **Error Notifications**: Notifications about errors like buffer overflows
+     or invalid packets.
+   - **Configuration Updates**: Notifications about changes in configuration,
+     such as updated MTU or VLAN settings.
+
+How-To Guide for Vendors
+========================
+
+This section provides a guide for vendors to develop firmware for the remote
+processor that is compatible with the RPMSG Based Virtual Ethernet Driver.
+
+1. **Implement Shared Memory Layout**:
+
+   - Allocate a shared memory region for packet transmission and reception.
+   - Initialize the `MAGIC_NUM`, `num_pkt_bufs`, `buff_slot_size`, `base_addr`,
+     `tx_offset`, and `rx_offset`.
+
+2. **Magic Number Requirements**
+
+   - The firmware must write a unique magic number (for example, ``0xABCDABCD``)
+     to the `magic_num` field of both the head and tail structures in the shared
+     memory region.
+   - This magic number is used by the Linux driver to validate that the shared
+     memory region is correctly initialized and accessible.
+   - If the driver detects an incorrect magic number during the handshake, it
+     will abort initialization and report an error.
+   - Vendors must ensure the magic number matches the value expected by the
+     Linux driver (see the `RPMSG_ETH_SHM_MAGIC_NUM` macro in the driver
+     source).
+
+3. **Handle RPMSG Requests**:
+
+   - Implement handlers for the following RPMSG requests:
+
+      - `RPMSG_ETH_REQ_SHM_INFO`
+      - `RPMSG_ETH_REQ_SET_MAC_ADDR`
+      - `RPMSG_ETH_REQ_ADD_MC_ADDR`
+      - `RPMSG_ETH_REQ_DEL_MC_ADDR`
+
+4. **Send RPMSG Notifications**:
+
+   - Notify the host about the state of the Ethernet interface using the
+     notifications described above.
+
+5. **Send Runtime Information**:
+
+   - Implement mechanisms to send runtime information such as link state
+     changes, statistics, and error notifications.
+
+6. **Implement Packet Processing**:
+
+   - Process packets in the shared memory transmit and receive buffers.
+
+7. **Test the Firmware**:
+
+   - Use the RPMSG Based Virtual Ethernet Driver on the host to test packet
+     transmission and reception.
+
+Configuration
+=============
+
+The driver relies on the device tree for configuration. The shared memory region
+is specified using the `virtual-eth-shm` node in the device tree.
+
+Example Device Tree Node
+------------------------
+
+.. code-block:: dts
+
+   virtual-eth-shm {
+           compatible = "rpmsg,virtual-eth-shm";
+           reg = <0x80000000 0x10000>; /* Base address and size of shared memory */
+   };
+
+Limitations
+===========
+
+- The driver assumes a specific shared memory layout and may not work with other
+  configurations.
+- Multicast address filtering is limited to the capabilities of the underlying
+  RPMSG framework.
+- The driver currently supports only one transmit and one receive queue.
+
+References
+==========
+
+- RPMSG Framework Documentation: https://www.kernel.org/doc/html/latest/rpmsg.html
+- Linux Networking Documentation: https://www.kernel.org/doc/html/latest/networking/index.html
+
+Authors
+=======
+
+- MD Danish Anwar <danishanwar@ti.com>
-- 
2.34.1

On Wed, Jul 23, 2025 at 01:33:18PM +0530, MD Danish Anwar wrote:
> +References
> +==========
> +
> +- RPMSG Framework Documentation: https://www.kernel.org/doc/html/latest/rpmsg.html
> +- Linux Networking Documentation: https://www.kernel.org/doc/html/latest/networking/index.html

Instead of using external link to kernel docs, you can also use internal
cross-references, simply by mentioning the docs path
(Documentation/staging/rpmsg.rst and Documentation/networking/index.rst in this
case).

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

> --- a/Documentation/networking/device_drivers/ethernet/index.rst
> +++ b/Documentation/networking/device_drivers/ethernet/index.rst
> @@ -61,6 +61,7 @@ Contents:
>     wangxun/txgbevf
>     wangxun/ngbe
>     wangxun/ngbevf
> +   rpmsg_eth

This list is sorted. Please insert at the right location. I made the
same comment to somebody else this week as well....

> +This driver is generic and can be used by any vendor. Vendors can develop their
> +own firmware for the remote processor to make it compatible with this driver.
> +The firmware must adhere to the shared memory layout, RPMSG communication
> +protocol, and data exchange requirements described in this documentation.

Could you add a link to TIs firmware? It would be a good reference
implementation. But i guess that needs to wait until the driver is
merged and the ABI is stable.

> +Implementation Details
> +----------------------
> +
> +- The magic number is defined as a macro in the driver source (e.g.,
> +  ``#define RPMSG_ETH_SHM_MAGIC_NUM 0xABCDABCD``).
> +- The firmware must write this value to the ``magic_num`` field of the head and
> +  tail structures in the shared memory region.
> +- During the handshake, the Linux driver reads these fields and compares them to
> +  the expected value. If any mismatch is detected, the driver will log an error
> +  and refuse to proceed.

So the firmware always takes the role of "primary" and Linux is
"secondary"? With the current implementation, you cannot have Linux on
both ends?

I don't see this as a problem, but maybe it is worth stating as a
current limitation.

> +Shared Memory Layout
> +====================
> +
> +The RPMSG Based Virtual Ethernet Driver uses a shared memory region to exchange
> +data between the host and the remote processor. The shared memory is divided
> +into transmit and receive regions, each with its own `head` and `tail` pointers
> +to track the buffer state.
> +
> +Shared Memory Parameters
> +------------------------
> +
> +The following parameters are exchanged between the host and the firmware to
> +configure the shared memory layout:

So the host tells the firmware this? Maybe this is explained later,
but is the flow something like:

Linux makes an RPC call to the firmware with the parameters you list
below. Upon receiving that RPC, the firmware puts the magic numbers in
place. It then ACKs the RPC call? Linux then checks the magic numbers?

> +1. **num_pkt_bufs**:
> +
> +   - The total number of packet buffers available in the shared memory.
> +   - This determines the maximum number of packets that can be stored in the
> +     shared memory at any given time.
> +
> +2. **buff_slot_size**:
> +
> +   - The size of each buffer slot in the shared memory.
> +   - This includes space for the packet length, metadata, and the actual packet
> +     data.
> +
> +3. **base_addr**:
> +
> +   - The base address of the shared memory region.
> +   - This is the starting point for accessing the shared memory.

So this is the base address in the Linux address space? How should the
firmware convert this into a base address in its address space?

> +4. **tx_offset**:
> +
> +   - The offset from the `base_addr` where the transmit buffers begin.
> +   - This is used by the host to write packets for transmission.
> +
> +5. **rx_offset**:
> +
> +   - The offset from the `base_addr` where the receive buffers begin.
> +   - This is used by the host to read packets received from the remote
> +     processor.

Maybe change 'host' to 'Linux'? Or some other name, 'primary' and
'secondary'. The naming should be consistent throughout the
documentation and driver.

Part of the issue here is that you pass this information from Linux to
the firmware. When the firmware receives it, it has the complete
opposite meaning. It uses "tx_offset" to receive packets, and
"rx_offset" to send packets. This can quickly get confusing. If you
used names like "linux_tx_offset", the added context with avoid
confusion.

> +Shared Memory Structure
> +-----------------------
> +
> +The shared memory layout is as follows:
> +
> +.. code-block:: text
> +
> +      Shared Memory Layout:
> +      ---------------------------
> +      |        MAGIC_NUM        |   rpmsg_eth_shm_head
> +      |          HEAD           |
> +      ---------------------------
> +      |        MAGIC_NUM        |
> +      |        PKT_1_LEN        |
> +      |          PKT_1          |
> +      ---------------------------
> +      |           ...           |
> +      ---------------------------
> +      |        MAGIC_NUM        |
> +      |          TAIL           |   rpmsg_eth_shm_tail
> +      ---------------------------
> +
> +1. **MAGIC_NUM**:
> +
> +   - A unique identifier used to validate the shared memory region.
> +   - Ensures that the memory region is correctly initialized and accessible.
> +
> +2. **HEAD Pointer**:
> +
> +   - Tracks the start of the buffer for packet transmission or reception.
> +   - Updated by the producer (host or remote processor) after writing a packet.

Is this a pointer, or an offset from the base address? Pointers get
messy when you have multiple address spaces involved. An offset is
simpler to work with. Given that the buffers are fixed size, it could
even be an index.

> +Information Exchanged Between RPMSG Channels
> +--------------------------------------------
> +
> +1. **Requests from Host to Remote Processor**:

Another place where consistent naming would be good. Here it is the
remote processor, not firmware used earlier.

> +
> +   - `RPMSG_ETH_REQ_SHM_INFO`: Request shared memory information, such as
> +     ``num_pkt_bufs``, ``buff_slot_size``, ``base_addr``, ``tx_offset``, and
> +     ``rx_offset``.

Is this requested, or telling? I suppose the text above uses "between"
which is ambiguous.

> +3. **Notifications from Remote Processor to Host**:
> +
> +   - `RPMSG_ETH_NOTIFY_PORT_UP`: Notify that the Ethernet port is up and ready
> +     for communication.
> +   - `RPMSG_ETH_NOTIFY_PORT_DOWN`: Notify that the Ethernet port is down.
> +   - `RPMSG_ETH_NOTIFY_PORT_READY`: Notify that the Ethernet port is ready for
> +     configuration.

That needs more explanation. Why would it not be ready? 

> +   - `RPMSG_ETH_NOTIFY_REMOTE_READY`: Notify that the remote processor is ready
> +     for communication.

How does this differ from PORT_READY?

> +How-To Guide for Vendors
> +========================
> +
> +This section provides a guide for vendors to develop firmware for the remote
> +processor that is compatible with the RPMSG Based Virtual Ethernet Driver.
> +
> +1. **Implement Shared Memory Layout**:
> +
> +   - Allocate a shared memory region for packet transmission and reception.
> +   - Initialize the `MAGIC_NUM`, `num_pkt_bufs`, `buff_slot_size`, `base_addr`,
> +     `tx_offset`, and `rx_offset`.
> +
> +2. **Magic Number Requirements**
> +
> +   - The firmware must write a unique magic number (for example, ``0xABCDABCD``)

Why "for example"? Do you have a use case where some other value
should be used? Or can we just make this magic value part of the
specification?

> +- The driver assumes a specific shared memory layout and may not work with other
> +  configurations.
> +- Multicast address filtering is limited to the capabilities of the underlying
> +  RPMSG framework.

I don't think there is anything special here. The network stack always
does perfect address filtering. The driver can help out, by also doing
perfect address filtering, or imperfect address filtering, and letting
more through than actually wanted. Or it can go into promiscuous mode.

> +- The driver currently supports only one transmit and one receive queue.
> +
> +References
> +==========
> +
> +- RPMSG Framework Documentation: https://www.kernel.org/doc/html/latest/rpmsg.html

This results in 404 Not Found.

     Andrew

Hi Andrew,

On 23/07/25 9:54 pm, Andrew Lunn wrote:
>> --- a/Documentation/networking/device_drivers/ethernet/index.rst
>> +++ b/Documentation/networking/device_drivers/ethernet/index.rst
>> @@ -61,6 +61,7 @@ Contents:
>>     wangxun/txgbevf
>>     wangxun/ngbe
>>     wangxun/ngbevf
>> +   rpmsg_eth
> 
> This list is sorted. Please insert at the right location. I made the
> same comment to somebody else this week as well....
> 

Sure. I will re-order this.

>> +This driver is generic and can be used by any vendor. Vendors can develop their
>> +own firmware for the remote processor to make it compatible with this driver.
>> +The firmware must adhere to the shared memory layout, RPMSG communication
>> +protocol, and data exchange requirements described in this documentation.
> 
> Could you add a link to TIs firmware? It would be a good reference
> implementation. But i guess that needs to wait until the driver is
> merged and the ABI is stable.
> 

Currently TIs firmware is not open source. Once the firmware is
available in open source I can update the documentation to have a link
to that.

>> +Implementation Details
>> +----------------------
>> +
>> +- The magic number is defined as a macro in the driver source (e.g.,
>> +  ``#define RPMSG_ETH_SHM_MAGIC_NUM 0xABCDABCD``).
>> +- The firmware must write this value to the ``magic_num`` field of the head and
>> +  tail structures in the shared memory region.
>> +- During the handshake, the Linux driver reads these fields and compares them to
>> +  the expected value. If any mismatch is detected, the driver will log an error
>> +  and refuse to proceed.
> 
> So the firmware always takes the role of "primary" and Linux is
> "secondary"? With the current implementation, you cannot have Linux on
> both ends?
> 

Yes the firmware is primary and Linux is secondary. Linux can not be at
both ends.

> I don't see this as a problem, but maybe it is worth stating as a
> current limitation.
> 

Sure, I will mention that in the documentation in v2.

>> +Shared Memory Layout
>> +====================
>> +
>> +The RPMSG Based Virtual Ethernet Driver uses a shared memory region to exchange
>> +data between the host and the remote processor. The shared memory is divided
>> +into transmit and receive regions, each with its own `head` and `tail` pointers
>> +to track the buffer state.
>> +
>> +Shared Memory Parameters
>> +------------------------
>> +
>> +The following parameters are exchanged between the host and the firmware to
>> +configure the shared memory layout:
> 
> So the host tells the firmware this? Maybe this is explained later,
> but is the flow something like:
> 
> Linux makes an RPC call to the firmware with the parameters you list
> below. Upon receiving that RPC, the firmware puts the magic numbers in
> place. It then ACKs the RPC call? Linux then checks the magic numbers?
> 

Let me explain the flow,

Linux first send a rpmsg request with msg type = RPMSG_ETH_REQ_SHM_INFO
i.e. requesting for the shared memory info.

Once firmware recieves this request it sends response with below fields,

	num_pkt_bufs, buff_slot_size, base_addr, tx_offset, rx_offset

In the device tree, while reserving the shared memory for rpmsg_eth
driver, the base address and the size of the shared memory block is
mentioned. I have mentioned that in the documentation as well

+Configuration
+=============
+
+The driver relies on the device tree for configuration. The shared
memory region
+is specified using the `virtual-eth-shm` node in the device tree.
+
+Example Device Tree Node
+------------------------
+
+.. code-block:: dts
+
+   virtual-eth-shm {
+           compatible = "rpmsg,virtual-eth-shm";
+           reg = <0x80000000 0x10000>; /* Base address and size of
shared memory */
+   };

Now once Linux recieves (num_pkt_bufs, buff_slot_size, base_addr,
tx_offset, rx_offset) from firmware, the driver does a handshake
validation rpmsg_eth_validate_handshake() where the driver checks if the
base address recieved from firmware matches the base address resevred in
DT, if the tx and rx offsets are within the range, if the magic number
in tx and rx buffers are same as the magic number defined in driver etc.

Based on this validation the callback function returns success / failure.

If needed, I can add this detail also in the documentation.

>> +1. **num_pkt_bufs**:
>> +
>> +   - The total number of packet buffers available in the shared memory.
>> +   - This determines the maximum number of packets that can be stored in the
>> +     shared memory at any given time.
>> +
>> +2. **buff_slot_size**:
>> +
>> +   - The size of each buffer slot in the shared memory.
>> +   - This includes space for the packet length, metadata, and the actual packet
>> +     data.
>> +
>> +3. **base_addr**:
>> +
>> +   - The base address of the shared memory region.
>> +   - This is the starting point for accessing the shared memory.
> 
> So this is the base address in the Linux address space? How should the
> firmware convert this into a base address in its address space?
> 

The `base_addr` is the physical address of the shared memory. This is
reserved in the Linux DT. I haven't added the DT patch in this series,
but if you want I can add in v2 for your reference.

The same `base_addr` is used by firmware for the shared memory. During
the rpmsg callback, firmware shares this `base_addr` and during
rpmsg_eth_validate_handshake() driver checks if the base_addr shared by
firmware is same as the one described in DT or not. Driver only proceeds
if it's same.

After that the driver maps this base_addr and the tx / rx offsets in the
Linux virtual address space and use the same virtual address for writing
/ reading.

The firmware also maps this base_addr into it's vitual address space.
How firmware maps it is upto the firmware. The only requiremnt is that
the physical base_addr used by firmware and driver should be the same.

>> +4. **tx_offset**:
>> +
>> +   - The offset from the `base_addr` where the transmit buffers begin.
>> +   - This is used by the host to write packets for transmission.
>> +
>> +5. **rx_offset**:
>> +
>> +   - The offset from the `base_addr` where the receive buffers begin.
>> +   - This is used by the host to read packets received from the remote
>> +     processor.
> 
> Maybe change 'host' to 'Linux'? Or some other name, 'primary' and
> 'secondary'. The naming should be consistent throughout the
> documentation and driver.
> 

Sure I will change 'host' to 'Linux' and keep it consistent throughout
driver and firmware.

> Part of the issue here is that you pass this information from Linux to
> the firmware. When the firmware receives it, it has the complete
> opposite meaning. It uses "tx_offset" to receive packets, and
> "rx_offset" to send packets. This can quickly get confusing. If you
> used names like "linux_tx_offset", the added context with avoid
> confusion.
> 

Sure. Will do this.

>> +Shared Memory Structure
>> +-----------------------
>> +
>> +The shared memory layout is as follows:
>> +
>> +.. code-block:: text
>> +
>> +      Shared Memory Layout:
>> +      ---------------------------
>> +      |        MAGIC_NUM        |   rpmsg_eth_shm_head
>> +      |          HEAD           |
>> +      ---------------------------
>> +      |        MAGIC_NUM        |
>> +      |        PKT_1_LEN        |
>> +      |          PKT_1          |
>> +      ---------------------------
>> +      |           ...           |
>> +      ---------------------------
>> +      |        MAGIC_NUM        |
>> +      |          TAIL           |   rpmsg_eth_shm_tail
>> +      ---------------------------
>> +
>> +1. **MAGIC_NUM**:
>> +
>> +   - A unique identifier used to validate the shared memory region.
>> +   - Ensures that the memory region is correctly initialized and accessible.
>> +
>> +2. **HEAD Pointer**:
>> +
>> +   - Tracks the start of the buffer for packet transmission or reception.
>> +   - Updated by the producer (host or remote processor) after writing a packet.
> 
> Is this a pointer, or an offset from the base address? Pointers get
> messy when you have multiple address spaces involved. An offset is
> simpler to work with. Given that the buffers are fixed size, it could
> even be an index.
> 

Below are the structure definitions.

struct rpmsg_eth_shared_mem {
	struct rpmsg_eth_shm_index *head;
	struct rpmsg_eth_shm_buf *buf;
	struct rpmsg_eth_shm_index *tail;
} __packed;

struct rpmsg_eth_shm_index {
	u32 magic_num;
	u32 index;
}  __packed;

Head is pointer and it is mapped as below based on the information
shared by firmware

	port->tx_buffer->head =
		(struct rpmsg_eth_shm_index __force *)
		 (ioremap(msg->resp_msg.shm_info.base_addr +
			  msg->resp_msg.shm_info.tx_offset,
			  sizeof(*port->tx_buffer->head)));

>> +Information Exchanged Between RPMSG Channels
>> +--------------------------------------------
>> +
>> +1. **Requests from Host to Remote Processor**:
> 
> Another place where consistent naming would be good. Here it is the
> remote processor, not firmware used earlier.
> 

Sure I will rename it.

>> +
>> +   - `RPMSG_ETH_REQ_SHM_INFO`: Request shared memory information, such as
>> +     ``num_pkt_bufs``, ``buff_slot_size``, ``base_addr``, ``tx_offset``, and
>> +     ``rx_offset``.
> 
> Is this requested, or telling? I suppose the text above uses "between"
> which is ambiguous.

It's requested. The Linux driver requests firmware for these info.

> 
>> +3. **Notifications from Remote Processor to Host**:
>> +
>> +   - `RPMSG_ETH_NOTIFY_PORT_UP`: Notify that the Ethernet port is up and ready
>> +     for communication.
>> +   - `RPMSG_ETH_NOTIFY_PORT_DOWN`: Notify that the Ethernet port is down.
>> +   - `RPMSG_ETH_NOTIFY_PORT_READY`: Notify that the Ethernet port is ready for
>> +     configuration.
> 
> That needs more explanation. Why would it not be ready? 
> 

This actually not needed RPMSG_ETH_NOTIFY_PORT_UP and
RPMSG_ETH_NOTIFY_PORT_DOWN are enough. I will drop
RPMSG_ETH_NOTIFY_PORT_READY

>> +   - `RPMSG_ETH_NOTIFY_REMOTE_READY`: Notify that the remote processor is ready
>> +     for communication.
> 
> How does this differ from PORT_READY?

RPMSG_ETH_NOTIFY_REMOTE_READY implies that the remote processor i.e. the
firmware is ready where as RPMSG_ETH_NOTIFY_PORT_UP implies that the
ethernet port (Linux driver) is ready.

PORT_READY is not needed and can be dropped.


> 
>> +How-To Guide for Vendors
>> +========================
>> +
>> +This section provides a guide for vendors to develop firmware for the remote
>> +processor that is compatible with the RPMSG Based Virtual Ethernet Driver.
>> +
>> +1. **Implement Shared Memory Layout**:
>> +
>> +   - Allocate a shared memory region for packet transmission and reception.
>> +   - Initialize the `MAGIC_NUM`, `num_pkt_bufs`, `buff_slot_size`, `base_addr`,
>> +     `tx_offset`, and `rx_offset`.
>> +
>> +2. **Magic Number Requirements**
>> +
>> +   - The firmware must write a unique magic number (for example, ``0xABCDABCD``)
> 
> Why "for example"? Do you have a use case where some other value
> should be used? Or can we just make this magic value part of the
> specification?

No the magic number is always the same. I will update the doucmentation
to state the same.
> 
>> +- The driver assumes a specific shared memory layout and may not work with other
>> +  configurations.
>> +- Multicast address filtering is limited to the capabilities of the underlying
>> +  RPMSG framework.
> 
> I don't think there is anything special here. The network stack always
> does perfect address filtering. The driver can help out, by also doing
> perfect address filtering, or imperfect address filtering, and letting
> more through than actually wanted. Or it can go into promiscuous mode.
> 

Sure. I will drop this.

>> +- The driver currently supports only one transmit and one receive queue.
>> +
>> +References
>> +==========
>> +
>> +- RPMSG Framework Documentation: https://www.kernel.org/doc/html/latest/rpmsg.html
> 
> This results in 404 Not Found.
> 

I see the url is wrong. I will update the correct url
https://docs.kernel.org/staging/rpmsg.html

>      Andrew

-- 
Thanks and Regards,
Danish

> Linux first send a rpmsg request with msg type = RPMSG_ETH_REQ_SHM_INFO
> i.e. requesting for the shared memory info.
> 
> Once firmware recieves this request it sends response with below fields,
> 
> 	num_pkt_bufs, buff_slot_size, base_addr, tx_offset, rx_offset
> 
> In the device tree, while reserving the shared memory for rpmsg_eth
> driver, the base address and the size of the shared memory block is
> mentioned. I have mentioned that in the documentation as well

If it is in device tree, why should Linux ask for the base address and
length? That just seems like a source of errors, and added complexity.

In general, we just trust DT. It is a source of truth. So i would
delete all this backwards and forwards and just use the values from
DT. Just check the magic numbers are in place.

> The same `base_addr` is used by firmware for the shared memory. During
> the rpmsg callback, firmware shares this `base_addr` and during
> rpmsg_eth_validate_handshake() driver checks if the base_addr shared by
> firmware is same as the one described in DT or not. Driver only proceeds
> if it's same.

So there is a big assumption here. That both are sharing the same MMU,
or maybe IOMMU. Or both CPUs have configured their MMU/IOMMU so that
the pages appear at the same physical address. I think this is a
problem, and the design should avoid anything which makes this
assumptions. The data structures within the share memory should only
refer to offsets from the base of the shared memory, not absolute
values. Or an index into the table of buffers, 0..N.

> >> +2. **HEAD Pointer**:
> >> +
> >> +   - Tracks the start of the buffer for packet transmission or reception.
> >> +   - Updated by the producer (host or remote processor) after writing a packet.
> > 
> > Is this a pointer, or an offset from the base address? Pointers get
> > messy when you have multiple address spaces involved. An offset is
> > simpler to work with. Given that the buffers are fixed size, it could
> > even be an index.
> > 
> 
> Below are the structure definitions.
> 
> struct rpmsg_eth_shared_mem {
> 	struct rpmsg_eth_shm_index *head;
> 	struct rpmsg_eth_shm_buf *buf;
> 	struct rpmsg_eth_shm_index *tail;
> } __packed;
> 
> struct rpmsg_eth_shm_index {
> 	u32 magic_num;
> 	u32 index;
> }  __packed;

So index is the index into the array of fixed size buffers. That is
fine, it is not a pointer, so you don't need to worry about address
spaces. However, head and tail are pointers, so for those you do need
to worry about address spaces. But why do you even need them? Just put
the indexes directly into rpmsg_eth_shared_mem. The four index values
can be in the first few words of the shared memory, fixed offset from
the beginning, KISS.

	Andrew

On 24/07/25 10:07 pm, Andrew Lunn wrote:
>> Linux first send a rpmsg request with msg type = RPMSG_ETH_REQ_SHM_INFO
>> i.e. requesting for the shared memory info.
>>
>> Once firmware recieves this request it sends response with below fields,
>>
>> 	num_pkt_bufs, buff_slot_size, base_addr, tx_offset, rx_offset
>>
>> In the device tree, while reserving the shared memory for rpmsg_eth
>> driver, the base address and the size of the shared memory block is
>> mentioned. I have mentioned that in the documentation as well
> 
> If it is in device tree, why should Linux ask for the base address and
> length? That just seems like a source of errors, and added complexity.
> 
> In general, we just trust DT. It is a source of truth. So i would
> delete all this backwards and forwards and just use the values from
> DT. Just check the magic numbers are in place.
> 

Sure. I will make this change in v2.

>> The same `base_addr` is used by firmware for the shared memory. During
>> the rpmsg callback, firmware shares this `base_addr` and during
>> rpmsg_eth_validate_handshake() driver checks if the base_addr shared by
>> firmware is same as the one described in DT or not. Driver only proceeds
>> if it's same.
> 
> So there is a big assumption here. That both are sharing the same MMU,
> or maybe IOMMU. Or both CPUs have configured their MMU/IOMMU so that
> the pages appear at the same physical address. I think this is a
> problem, and the design should avoid anything which makes this
> assumptions. The data structures within the share memory should only
> refer to offsets from the base of the shared memory, not absolute
> values. Or an index into the table of buffers, 0..N.
> 
>>>> +2. **HEAD Pointer**:
>>>> +
>>>> +   - Tracks the start of the buffer for packet transmission or reception.
>>>> +   - Updated by the producer (host or remote processor) after writing a packet.
>>>
>>> Is this a pointer, or an offset from the base address? Pointers get
>>> messy when you have multiple address spaces involved. An offset is
>>> simpler to work with. Given that the buffers are fixed size, it could
>>> even be an index.
>>>
>>
>> Below are the structure definitions.
>>
>> struct rpmsg_eth_shared_mem {
>> 	struct rpmsg_eth_shm_index *head;
>> 	struct rpmsg_eth_shm_buf *buf;
>> 	struct rpmsg_eth_shm_index *tail;
>> } __packed;
>>
>> struct rpmsg_eth_shm_index {
>> 	u32 magic_num;
>> 	u32 index;
>> }  __packed;
> 
> So index is the index into the array of fixed size buffers. That is
> fine, it is not a pointer, so you don't need to worry about address
> spaces. However, head and tail are pointers, so for those you do need
> to worry about address spaces. But why do you even need them? Just put
> the indexes directly into rpmsg_eth_shared_mem. The four index values
> can be in the first few words of the shared memory, fixed offset from
> the beginning, KISS.
> 

I will drop all these pointers and use a offset based approach in v2.

Thanks for the feedback.

-- 
Thanks and Regards,
Danish

Hi Andrew,

On 7/24/2025 10:07 PM, Andrew Lunn wrote:
>> Linux first send a rpmsg request with msg type = RPMSG_ETH_REQ_SHM_INFO
>> i.e. requesting for the shared memory info.
>>
>> Once firmware recieves this request it sends response with below fields,
>>
>> 	num_pkt_bufs, buff_slot_size, base_addr, tx_offset, rx_offset
>>
>> In the device tree, while reserving the shared memory for rpmsg_eth
>> driver, the base address and the size of the shared memory block is
>> mentioned. I have mentioned that in the documentation as well
> 
> If it is in device tree, why should Linux ask for the base address and
> length? That just seems like a source of errors, and added complexity.
> 
> In general, we just trust DT. It is a source of truth. So i would
> delete all this backwards and forwards and just use the values from
> DT. Just check the magic numbers are in place.
> 

Sure, I will not check the base_addr and trust the info we get from
device tree. Just check the offsets we are getting from firmware is
within the shared memory block or not (using base_addr + size)

>> The same `base_addr` is used by firmware for the shared memory. During
>> the rpmsg callback, firmware shares this `base_addr` and during
>> rpmsg_eth_validate_handshake() driver checks if the base_addr shared by
>> firmware is same as the one described in DT or not. Driver only proceeds
>> if it's same.
> 
> So there is a big assumption here. That both are sharing the same MMU,
> or maybe IOMMU. Or both CPUs have configured their MMU/IOMMU so that
> the pages appear at the same physical address. I think this is a
> problem, and the design should avoid anything which makes this
> assumptions. The data structures within the share memory should only
> refer to offsets from the base of the shared memory, not absolute
> values. Or an index into the table of buffers, 0..N.
> 

Sure I will try to do the same.

>>>> +2. **HEAD Pointer**:
>>>> +
>>>> +   - Tracks the start of the buffer for packet transmission or reception.
>>>> +   - Updated by the producer (host or remote processor) after writing a packet.
>>>
>>> Is this a pointer, or an offset from the base address? Pointers get
>>> messy when you have multiple address spaces involved. An offset is
>>> simpler to work with. Given that the buffers are fixed size, it could
>>> even be an index.
>>>
>>
>> Below are the structure definitions.
>>
>> struct rpmsg_eth_shared_mem {
>> 	struct rpmsg_eth_shm_index *head;
>> 	struct rpmsg_eth_shm_buf *buf;
>> 	struct rpmsg_eth_shm_index *tail;
>> } __packed;
>>
>> struct rpmsg_eth_shm_index {
>> 	u32 magic_num;
>> 	u32 index;
>> }  __packed;
> 
> So index is the index into the array of fixed size buffers. That is
> fine, it is not a pointer, so you don't need to worry about address
> spaces. However, head and tail are pointers, so for those you do need
> to worry about address spaces. But why do you even need them? Just put
> the indexes directly into rpmsg_eth_shared_mem. The four index values
> can be in the first few words of the shared memory, fixed offset from
> the beginning, KISS.
> 

Sure I will try to move everything to offsets and not use pointers.

> 	Andrew

-- 
Thanks and Regards,
Md Danish Anwar

On Wed, 23 Jul 2025 13:33:18 +0530 MD Danish Anwar wrote:
> +   - Vendors must ensure the magic number matches the value expected by the
> +     Linux driver (see the `RPMSG_ETH_SHM_MAGIC_NUM` macro in the driver
> +     source).

For some reason this trips up make coccicheck:

EXN: Failure("unexpected paren order") in /home/cocci/testing/Documentation/networking/device_drivers/ethernet/rpmsg_eth.rst

If I replace the brackets with a comma it works:

   - Vendors must ensure the magic number matches the value expected by the
     Linux driver, see the `RPMSG_ETH_SHM_MAGIC_NUM` macro in the driver
     source.

Could you make that change in the next revision to avoid the problem?

Julia, is there an easy way to make coccinelle ignore files which
don't end with .c or .h when using --use-patch-diff ?
-- 
pw-bot: cr

On Wed, 23 Jul 2025, Jakub Kicinski wrote:

> On Wed, 23 Jul 2025 13:33:18 +0530 MD Danish Anwar wrote:
> > +   - Vendors must ensure the magic number matches the value expected by the
> > +     Linux driver (see the `RPMSG_ETH_SHM_MAGIC_NUM` macro in the driver
> > +     source).
>
> For some reason this trips up make coccicheck:
>
> EXN: Failure("unexpected paren order") in /home/cocci/testing/Documentation/networking/device_drivers/ethernet/rpmsg_eth.rst
>
> If I replace the brackets with a comma it works:
>
>    - Vendors must ensure the magic number matches the value expected by the
>      Linux driver, see the `RPMSG_ETH_SHM_MAGIC_NUM` macro in the driver
>      source.
>
> Could you make that change in the next revision to avoid the problem?
>
> Julia, is there an easy way to make coccinelle ignore files which
> don't end with .c or .h when using --use-patch-diff ?

Perhaps not.  I can adjust it.

julia

Hi Jakub,

On 23/07/25 7:19 pm, Jakub Kicinski wrote:
> On Wed, 23 Jul 2025 13:33:18 +0530 MD Danish Anwar wrote:
>> +   - Vendors must ensure the magic number matches the value expected by the
>> +     Linux driver (see the `RPMSG_ETH_SHM_MAGIC_NUM` macro in the driver
>> +     source).
> 
> For some reason this trips up make coccicheck:
> 
> EXN: Failure("unexpected paren order") in /home/cocci/testing/Documentation/networking/device_drivers/ethernet/rpmsg_eth.rst
> 
> If I replace the brackets with a comma it works:
> 
>    - Vendors must ensure the magic number matches the value expected by the
>      Linux driver, see the `RPMSG_ETH_SHM_MAGIC_NUM` macro in the driver
>      source.
> 
> Could you make that change in the next revision to avoid the problem?
> 

Sure. I'll do this change in v2.

> Julia, is there an easy way to make coccinelle ignore files which
> don't end with .c or .h when using --use-patch-diff ?

-- 
Thanks and Regards,
Danish