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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 22/4/24 23:02, Michael S. Tsirkin wrote:
> On Mon, Apr 22, 2024 at 04:20:56PM +0200, Philippe Mathieu-Daudé wrote:
>> 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>
> 
> 
> 
> This is all completely bogus. Virtio SND is from Virtio 1.0 only.
> It is unconditionally little endian.

Oh, then the fix is as simple as:

  -    as->endianness = target_words_bigendian() ? 1 : 0;
  +    as->endianness = 0; /* VIRTIO 1.0: always LE. */

> If it's not it's a guest bug pls just fix it there.
> 
> 
>> ---
>> v2: Use virtio_is_big_endian()
>> v3: Remove "hw/core/cpu.h
>> ---
>>   hw/audio/virtio-snd.c | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
>> index c80b58bf5d..939cd78026 100644
>> --- a/hw/audio/virtio-snd.c
>> +++ b/hw/audio/virtio-snd.c
>> @@ -24,7 +24,6 @@
>>   #include "trace.h"
>>   #include "qapi/error.h"
>>   #include "hw/audio/virtio-snd.h"
>> -#include "hw/core/cpu.h"
>>   
>>   #define VIRTIO_SOUND_VM_VERSION 1
>>   #define VIRTIO_SOUND_JACK_DEFAULT 0
>> @@ -395,13 +394,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 +465,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
>