openamp: change rx buffer hold flag to count

[CV-Bowen]

This commit fix one issue, before this commit, if rpmsg_hold_rx_buffer() is called in the endpoint callback to hold current received buffer and callrpmsg_release_rx_buffer() to release this buffer immediately. This buffer will be returned to the virtqueue in rpmsg_virtio_release_rx_buffer() and returned to virtqueue again in rpmsg_virtio_return_buffer() after the ept->cb(). So same buffer returned to the virtqueue twice, error happened. Follow shows this process:

rpmsg_virtio_rx_callback()
  - get rp_hdr
  - ept->cb(data = RPMSG_LOCATE_DATA(rp_hdr))
    - rpsmg_hold_rx_buffer(data)
    - rpmsg_release_rx_buffer(data) here has returned buffer to virtqueue
  - rpmsg_virtio_return_buffer() return same buffer to virtqueue again

The root cause of this issue is that the RPMSG_BUF_HELD has been cleared in rpmsg_release_rx_buffer() so rpmsg_virtio_rx_callback() will treat this buffer is not HELD and call rpmsg_virtio_return_buffer() to return this buffer again.

This commit change the HELD flag in rp_hdr to count to fix this issue and also support hold/release rx buffer recursively.

[CV-Bowen]

Strange to me that you hold the buffer to release it in same function. I would say hold it at the end of the ept->cb if needed... That say I think it is interesting to use a counter instead of a flag, as we could have several consumers.

What I'm expecting here is that we have same hold mechanism for the RX and the TX buffers. So please update also the TX part

@arnopo Hi, I'm a little confused about the update also the TX part, do you mean add rpmsg_virtio_hold_tx_buffer() in rpmsg_virtio and rpmsg_hold_tx_buffer() in rpmsg, and also add hold_tx_buffer ops to the struct rpmsg_device_ops to make user can call this function?

[arnopo]

Strange to me that you hold the buffer to release it in same function. I would say hold it at the end of the ept->cb if needed... That say I think it is interesting to use a counter instead of a flag, as we could have several consumers. What I'm expecting here is that we have same hold mechanism for the RX and the TX buffers. So please update also the TX part

@arnopo Hi, I'm a little confused about the update also the TX part, do you mean add rpmsg_virtio_hold_tx_buffer() in rpmsg_virtio and rpmsg_hold_tx_buffer() in rpmsg, and also add hold_tx_buffer ops to the struct rpmsg_device_ops to make user can call this function?

My request was not cristal clear. No need to add user interface to hold TX buffer. my point was just to use RPMSG_BUF_HELD_INC() and RPMSG_BUF_HELD_DEC() internally for the TX buffers.

[CV-Bowen]

Strange to me that you hold the buffer to release it in same function. I would say hold it at the end of the ept->cb if needed... That say I think it is interesting to use a counter instead of a flag, as we could have several consumers. What I'm expecting here is that we have same hold mechanism for the RX and the TX buffers. So please update also the TX part

@arnopo Hi, I'm a little confused about the update also the TX part, do you mean add rpmsg_virtio_hold_tx_buffer() in rpmsg_virtio and rpmsg_hold_tx_buffer() in rpmsg, and also add hold_tx_buffer ops to the struct rpmsg_device_ops to make user can call this function?

My request was not cristal clear. No need to add user interface to hold TX buffer. my point was just to use RPMSG_BUF_HELD_INC() and RPMSG_BUF_HELD_DEC() internally for the TX buffers.

@arnopo Got, and I has deleted the user interface. Could you take a look again.

[xiaoxiang781216]

@arnopo do you have more suggestion?

[CV-Bowen]

@arnopo Thanks, all the comments have been addressed.