[PATCH v2] hw/audio/virtio-snd: Use device endianness instead of target one

[PATCH v2] hw/audio/virtio-snd: Use device endianness instead of target one

[Philippe Mathieu-Daudé]

Since VirtIO devices can change endianness at runtime,
we need to use the device endianness, not the target
one.

Cc: qemu-stable@nongnu.org
Fixes: eb9ad377bb ("virtio-sound: handle control messages and streams")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/audio/virtio-snd.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index c80b58bf5d..82c5647660 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -395,13 +395,15 @@ static uint32_t virtio_snd_get_qemu_freq(uint32_t rate)
  * Get QEMU Audiosystem compatible audsettings from virtio based pcm stream
  * params.
  */
-static void virtio_snd_get_qemu_audsettings(audsettings *as,
+static void virtio_snd_get_qemu_audsettings(VirtIOSound *s, audsettings *as,
                                             virtio_snd_pcm_set_params *params)
 {
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
     as->nchannels = MIN(AUDIO_MAX_CHANNELS, params->channels);
     as->fmt = virtio_snd_get_qemu_format(params->format);
     as->freq = virtio_snd_get_qemu_freq(params->rate);
-    as->endianness = target_words_bigendian() ? 1 : 0;
+    as->endianness = virtio_is_big_endian(vdev) ? 1 : 0;
 }
 
 /*
@@ -464,7 +466,7 @@ static uint32_t virtio_snd_pcm_prepare(VirtIOSound *s, uint32_t stream_id)
         s->pcm->streams[stream_id] = stream;
     }
 
-    virtio_snd_get_qemu_audsettings(&as, params);
+    virtio_snd_get_qemu_audsettings(s, &as, params);
     stream->info.direction = stream_id < s->snd_conf.streams / 2 +
         (s->snd_conf.streams & 1) ? VIRTIO_SND_D_OUTPUT : VIRTIO_SND_D_INPUT;
     stream->info.hdr.hda_fn_nid = VIRTIO_SOUND_HDA_FN_NID;
-- 
2.41.0

Re: [PATCH v2] hw/audio/virtio-snd: Use device endianness instead of target one

[Manos Pitsidianakis]

On Mon, 22 Apr 2024 16:13, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>Since VirtIO devices can change endianness at runtime,
>we need to use the device endianness, not the target
>one.

Hey Philippe, can you clarify what do you mean by they can change 
endianness at runtime?

The target's one is used because that's the one it will be using to do 
I/O with its kernel's audio interface.





>
>Cc: qemu-stable@nongnu.org
>Fixes: eb9ad377bb ("virtio-sound: handle control messages and streams")
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> hw/audio/virtio-snd.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
>diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
>index c80b58bf5d..82c5647660 100644
>--- a/hw/audio/virtio-snd.c
>+++ b/hw/audio/virtio-snd.c
>@@ -395,13 +395,15 @@ static uint32_t virtio_snd_get_qemu_freq(uint32_t rate)
>  * Get QEMU Audiosystem compatible audsettings from virtio based pcm stream
>  * params.
>  */
>-static void virtio_snd_get_qemu_audsettings(audsettings *as,
>+static void virtio_snd_get_qemu_audsettings(VirtIOSound *s, audsettings *as,
>                                             virtio_snd_pcm_set_params *params)
> {
>+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
>+
>     as->nchannels = MIN(AUDIO_MAX_CHANNELS, params->channels);
>     as->fmt = virtio_snd_get_qemu_format(params->format);
>     as->freq = virtio_snd_get_qemu_freq(params->rate);
>-    as->endianness = target_words_bigendian() ? 1 : 0;
>+    as->endianness = virtio_is_big_endian(vdev) ? 1 : 0;
> }
> 
> /*
>@@ -464,7 +466,7 @@ static uint32_t virtio_snd_pcm_prepare(VirtIOSound *s, uint32_t stream_id)
>         s->pcm->streams[stream_id] = stream;
>     }
> 
>-    virtio_snd_get_qemu_audsettings(&as, params);
>+    virtio_snd_get_qemu_audsettings(s, &as, params);
>     stream->info.direction = stream_id < s->snd_conf.streams / 2 +
>         (s->snd_conf.streams & 1) ? VIRTIO_SND_D_OUTPUT : VIRTIO_SND_D_INPUT;
>     stream->info.hdr.hda_fn_nid = VIRTIO_SOUND_HDA_FN_NID;
>-- 
>2.41.0
>

Re: [PATCH v2] hw/audio/virtio-snd: Use device endianness instead of target one

[Philippe Mathieu-Daudé]

On 22/4/24 15:45, Manos Pitsidianakis wrote:
> On Mon, 22 Apr 2024 16:13, Philippe Mathieu-Daudé <philmd@linaro.org> 
> wrote:
>> Since VirtIO devices can change endianness at runtime,
>> we need to use the device endianness, not the target
>> one.
> 
> Hey Philippe, can you clarify what do you mean by they can change 
> endianness at runtime?
> 
> The target's one is used because that's the one it will be using to do 
> I/O with its kernel's audio interface.

See for example in SysemuCPUOps:

     /**
      * @virtio_is_big_endian: Callback to return %true if a CPU which
      * supports runtime configurable endianness is currently big-endian.
      * Non-configurable CPUs can use the default implementation of this
      * method.
      * This method should not be used by any callers other than the
      * pre-1.0 virtio devices.
      */
     bool (*virtio_is_big_endian)(CPUState *cpu);

and:

/**
  * cpu_virtio_is_big_endian:
  * @cpu: CPU

  * Returns %true if a CPU which supports runtime configurable endianness
  * is currently big-endian.
  */
bool cpu_virtio_is_big_endian(CPUState *cpu);

 From commit 616a655219 ("virtio: add endian-ambivalent support to 
VirtIODevice"):

     Some CPU families can dynamically change their endianness. This
     means we can have little endian ppc or big endian arm guests for
     example. This has an impact on legacy virtio data structures since
     they are target endian.
     We hence introduce a new property to track the endianness of each
     virtio device. It is reasonnably assumed that endianness won't
     change while the device is in use : we hence capture the device
     endianness when it gets reset.

> 
>>
>> Cc: qemu-stable@nongnu.org
>> Fixes: eb9ad377bb ("virtio-sound: handle control messages and streams")
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/audio/virtio-snd.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
>> index c80b58bf5d..82c5647660 100644
>> --- a/hw/audio/virtio-snd.c
>> +++ b/hw/audio/virtio-snd.c
>> @@ -395,13 +395,15 @@ static uint32_t 
>> virtio_snd_get_qemu_freq(uint32_t rate)
>>  * Get QEMU Audiosystem compatible audsettings from virtio based pcm 
>> stream
>>  * params.
>>  */
>> -static void virtio_snd_get_qemu_audsettings(audsettings *as,
>> +static void virtio_snd_get_qemu_audsettings(VirtIOSound *s, 
>> audsettings *as,
>>                                             virtio_snd_pcm_set_params 
>> *params)
>> {
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(s);
>> +
>>     as->nchannels = MIN(AUDIO_MAX_CHANNELS, params->channels);
>>     as->fmt = virtio_snd_get_qemu_format(params->format);
>>     as->freq = virtio_snd_get_qemu_freq(params->rate);
>> -    as->endianness = target_words_bigendian() ? 1 : 0;
>> +    as->endianness = virtio_is_big_endian(vdev) ? 1 : 0;
>> }
>>
>> /*
>> @@ -464,7 +466,7 @@ static uint32_t virtio_snd_pcm_prepare(VirtIOSound 
>> *s, uint32_t stream_id)
>>         s->pcm->streams[stream_id] = stream;
>>     }
>>
>> -    virtio_snd_get_qemu_audsettings(&as, params);
>> +    virtio_snd_get_qemu_audsettings(s, &as, params);
>>     stream->info.direction = stream_id < s->snd_conf.streams / 2 +
>>         (s->snd_conf.streams & 1) ? VIRTIO_SND_D_OUTPUT : 
>> VIRTIO_SND_D_INPUT;
>>     stream->info.hdr.hda_fn_nid = VIRTIO_SOUND_HDA_FN_NID;
>> -- 
>> 2.41.0
>>