rpmsg: let rpmsg_virtio_get_tx_buffer() always return idx in host side

[CV-Bowen]

In rpmsg host side, the tx buffer index is not assigned when this buffer is obtained from the reclaimer list, this commit let rpmsg_virtio_get_tx_buffer() always gets correct idx.

Note: the idx in rpmsg host side is not used, so this commit is just an improvement and not fix any issue.

[arnopo]

I crosschecked the behavior. In rpmsg_virtio_return_buffer, we don't use the index for host side. So in theory not need to set it.

Do you have a concrete use case for which your are facing an issue?

[CV-Bowen]

@arnopo The index will be used in rpmsg_virtio_release_tx_buffer(): idx = rp_hdr->reserved;

static int rpmsg_virtio_release_tx_buffer(struct rpmsg_device *rdev, void *txbuf)
{
    struct rpmsg_virtio_device *rvdev;
    struct rpmsg_hdr *rp_hdr = RPMSG_LOCATE_HDR(txbuf);
    void *vbuff = rp_hdr;  /* only used to avoid warning on the cast of a packed structure */
    struct vbuff_reclaimer_t *r_desc = (struct vbuff_reclaimer_t *)vbuff;
    uint16_t idx;

    /*
     * Reuse the RPMsg buffer to temporary store the vbuff_reclaimer_t structure.
     * Stores the index locally before overwriting the RPMsg header.
     */
    idx = rp_hdr->reserved;

    rvdev = metal_container_of(rdev, struct rpmsg_virtio_device, rdev);

    metal_mutex_acquire(&rdev->lock);

    r_desc->idx = idx;
    metal_list_add_tail(&rvdev->reclaimer, &r_desc->node);

    metal_mutex_release(&rdev->lock);

    return RPMSG_SUCCESS;
}

And the rp_hdr->reserved is set to idx in rpmsg_virtio_get_tx_payload_buffer()

[arnopo]

@arnopo The index will be used in rpmsg_virtio_release_tx_buffer(): idx = rp_hdr->reserved;

static int rpmsg_virtio_release_tx_buffer(struct rpmsg_device *rdev, void *txbuf)
{
  struct rpmsg_virtio_device *rvdev;
  struct rpmsg_hdr *rp_hdr = RPMSG_LOCATE_HDR(txbuf);
  void *vbuff = rp_hdr;  /* only used to avoid warning on the cast of a packed structure */
  struct vbuff_reclaimer_t *r_desc = (struct vbuff_reclaimer_t *)vbuff;
  uint16_t idx;

  /*
   * Reuse the RPMsg buffer to temporary store the vbuff_reclaimer_t structure.
   * Stores the index locally before overwriting the RPMsg header.
   */
  idx = rp_hdr->reserved;

  rvdev = metal_container_of(rdev, struct rpmsg_virtio_device, rdev);

  metal_mutex_acquire(&rdev->lock);

  r_desc->idx = idx;
  metal_list_add_tail(&rvdev->reclaimer, &r_desc->node);

  metal_mutex_release(&rdev->lock);

  return RPMSG_SUCCESS;
}

And the rp_hdr->reserved is set to idx in rpmsg_virtio_get_tx_payload_buffer()

What I would like to figure out is whether this fix is for an existing bug or if it is intended to prevent a potential bug. Upon looking into the code, whenrole == RPMSG_HOST, the index is not used.

In the end, I think that we should set the idx as you propose. However, what I am trying to determine first is whether this fix is for an existing bug or if it is an improvement to the code's robustness."

[CV-Bowen]

@arnopo Yes, the index is not used in HOST side and after discussing with @GUIDINGLI, we confirm this commit is just an improvement to code's robustness.

[arnopo]

@arnopo Yes, the index is not used in HOST side and after discussing with @GUIDINGLI, we confirm this commit is just an improvement to code's robustness.

Thanks for the confirmation. Please update the commit in consequence, it is not a fix but an improvement

[CV-Bowen]

@arnopo Done

[arnopo]

@edmooring, @tnmysh ,

Please, could your review this PR?