Re: [PATCH v2 3/7] pc-bios/s390-ccw: Introduce virtio_tswap helpers

[Jared Rossi <jrossi@linux.ibm.com>]

On 9/3/26 12:24 PM, Zhuoying Cai wrote:
> Replace the repeated pattern:
>
>      be_ipl() ? x : bswapN(x)
>
> with helpers that conditionally byte-swap based on the virtio transport
> endianness.
>
> While only the 16-bit helper is used immediately, the 32-bit and 64-bit
> variants are added here for later use.
>
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
> ---

Reviewed-by: Jared Rossi <jrossi@linux.ibm.com>

>   pc-bios/s390-ccw/virtio.c | 27 +++++++++++++++++++++------
>   pc-bios/s390-ccw/virtio.h |  4 ++++
>   2 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
> index a0d249db24..037c00a3e1 100644
> --- a/pc-bios/s390-ccw/virtio.c
> +++ b/pc-bios/s390-ccw/virtio.c
> @@ -97,7 +97,7 @@ void vring_init(VRing *vr, VqInfo *info)
>       vr->avail->idx = 0;
>   
>       /* We're running with interrupts off anyways, so don't bother */
> -    vr->used->flags = be_ipl() ? VRING_USED_F_NO_NOTIFY : bswap16(VRING_USED_F_NO_NOTIFY);
> +    vr->used->flags = virtio_tswap16(VRING_USED_F_NO_NOTIFY);
>       vr->used->idx = 0;
>       vr->used_idx = 0;
>       vr->next_idx = 0;
> @@ -140,6 +140,22 @@ bool be_ipl(void)
>       }
>   }
>   
> +/* Conditionally byte-swap between virtio-endian and s390x native big-endian. */
> +uint16_t virtio_tswap16(uint16_t x)
> +{
> +    return be_ipl() ? x : bswap16(x);
> +}
> +
> +uint32_t virtio_tswap32(uint32_t x)
> +{
> +    return be_ipl() ? x : bswap32(x);
> +}
> +
> +uint64_t virtio_tswap64(uint64_t x)
> +{
> +    return be_ipl() ? x : bswap64(x);
> +}
> +
>   /*
>    * Format the virtio ring descriptor endianness
>    * Return the available index increment in the appropriate endianness
> @@ -156,12 +172,11 @@ void vring_send_buf(VRing *vr, void *p, int len, int flags)
>   {
>       uint16_t avail_idx;
>   
> -    avail_idx = be_ipl() ? vr->avail->idx : bswap16(vr->avail->idx);
> +    avail_idx = virtio_tswap16(vr->avail->idx);
>   
>       /* For follow-up chains we need to keep the first entry point */
>       if (!(flags & VRING_HIDDEN_IS_CHAIN)) {
> -        vr->avail->ring[avail_idx % vr->num] = be_ipl() ? vr->next_idx :
> -                                                          bswap16(vr->next_idx);
> +        vr->avail->ring[avail_idx % vr->num] = virtio_tswap16(vr->next_idx);
>       }
>   
>       vr->desc[vr->next_idx].addr = (unsigned long)p;
> @@ -179,7 +194,7 @@ void vring_send_buf(VRing *vr, void *p, int len, int flags)
>       /* Chains only have a single ID */
>       if (!(flags & VRING_DESC_F_NEXT)) {
>           avail_idx++;
> -        vr->avail->idx = be_ipl() ? avail_idx : bswap16(avail_idx);
> +        vr->avail->idx = virtio_tswap16(avail_idx);
>       }
>   }
>   
> @@ -187,7 +202,7 @@ int vr_poll(VRing *vr)
>   {
>       uint16_t used_idx;
>   
> -    used_idx = be_ipl() ? vr->used->idx : bswap16(vr->used->idx);
> +    used_idx = virtio_tswap16(vr->used->idx);
>       if (used_idx == vr->used_idx) {
>           vring_notify(vr);
>           yield();
> diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
> index aa307025e0..645dc288b1 100644
> --- a/pc-bios/s390-ccw/virtio.h
> +++ b/pc-bios/s390-ccw/virtio.h
> @@ -287,6 +287,10 @@ int virtio_run(VDev *vdev, int vqid, VirtioCmd *cmd);
>   int virtio_reset(VDev *vdev);
>   int virtio_setup_ccw(VDev *vdev);
>   
> +uint16_t virtio_tswap16(uint16_t x);
> +uint32_t virtio_tswap32(uint32_t x);
> +uint64_t virtio_tswap64(uint64_t x);
> +
>   /* virtio-net.c */
>   int virtio_net_init(void *mac_addr);
>   void virtio_net_deinit(void);