Re: [Qemu-devel] TCP based PCIE request forwarding

[Stefan Hajnoczi <stefanha@gmail.com>]

On Wed, Nov 21, 2012 at 03:27:48PM +0100, lementec fabien wrote:
> usage
> -----
> PCIEFW devices are instanciated using the following QEMU options:
> -device \
>  pciefw,\
>  laddr=<local_addr>,\
>  lport=<local_port>,\
>  raddr=<remote_addr>,\
>  rport=<remote_port>

Take a look at qemu_socket.h:socket_parse().  It should allow you to
support TCP, UNIX domain sockets, and arbitrary file descriptors.

> implementation
> --------------
> PCIEFW is a PCIE accesses forwarding device added to the QEMU source tree. At
> initialization, this device opens a bidirectionnal point to point communication
> channel with an external process. This process actually implements the PCIE
> endpoint. That is, a PCIE access made by QEMU is forwarded to the process.
> Reciprocally, replies and interrupts messages from the process are forwarded
> to QEMU.
>
> The commnication currently relies on a bidirectionnal point to point TCP

s/commnication/communication/

> socket based channel. Byte ordering is little endian.
>
> PCIEFW initiates a request upon access from QEMU. It sends a message whose
> format is described by the pciefw_msg_t type:
> 
> typedef struct pciefw_msg
> {
> #define PCIEFW_MSG_MAX_SIZE (offsetof(pciefw_msg_t, data) + 0x1000)

The size field is uint16_t.  Do you really want to limit to 4 KB of
data?

>
>   pciefw_header_t header;
>
> #define PCIEFW_OP_READ_CONFIG 0
> #define PCIEFW_OP_WRITE_CONFIG 1
> #define PCIEFW_OP_READ_MEM 2
> #define PCIEFW_OP_WRITE_MEM 3
> #define PCIEFW_OP_READ_IO 4
> #define PCIEFW_OP_WRITE_IO 5
> #define PCIEFW_OP_INT 6
> #define PCIEFW_OP_MSI 7
> #define PCIEFW_OP_MSIX 8
>
>   uint8_t op; /* in PCIEFW_OP_XXX */
>   uint8_t bar; /* in [0:5] */
>   uint8_t width; /* access in 1, 2, 4, 8 */
>   uint64_t addr;
>   uint16_t size; /* data size, in bytes */

Why is are both width and size fields?  For read-type operations the
size field would indicate how many bytes to read.  For write-type
operations the size field would indicate how many bytes are included in
data[].

>   uint8_t data[1];
>
> } __attribute__((packed)) pciefw_msg_t;
>
> Note that data is a variable length field.
>
> The PCIE endpoint process replies with a pciefw_reply_t formatted message:
>
> typedef struct pciefw_reply
> {
>   pciefw_header_t header;
>   uint8_t status;

What values does this field take?

>   uint8_t data[8];
> } __attribute__((packed)) pciefw_reply_t;
>
> The PCIE endpoint process can initiate pciefw_msg_t to perform write operations
> of its own. This is used to perform data transfer (DMA engines ...) and send
> interrupts.

Any flow control rules?  For example, can the endpoint raise an
interrupt while processing a message (before it sends a reply)?

> Both types start with a pciefw_header containing the total size:
>
> typedef struct pciefw_header
> {
>   uint16_t size;
> } __attribute__((packed)) pciefw_header_t;

A "hello" message type would be useful so that you can extend the
protocol in the future.  The message would contain feature bits or a
version number.

Stefan