Am 05.01.24 um 11:54 schrieb Marc-André Lureau:
> Hi
>
> On Fri, Jan 5, 2024 at 12:34 AM Volker Rümelin <vr_q...@t-online.de> wrote:
>> It is much easier to migrate an array of structs than individual
>> structs that are accessed via a pointer to a pointer to an array
>> of pointers to struct, where some pointers can also be NULL.
>>
>> For this reason, the audio streams are already allocated during
>> the realization phase and all stream variables that are constant
>> at runtime are initialised immediately after allocation. This is
>> a step towards being able to migrate the audio streams of the
>> virtio sound device after the next few patches.
>>
>> Signed-off-by: Volker Rümelin <vr_q...@t-online.de>
>> ---
>> hw/audio/virtio-snd.c | 35 ++++++++++++++++++++++-------------
>> include/hw/audio/virtio-snd.h | 1 +
>> 2 files changed, 23 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
>> index 8344f61c64..36b1bb502c 100644
>> --- a/hw/audio/virtio-snd.c
>> +++ b/hw/audio/virtio-snd.c
>> @@ -447,11 +447,9 @@ static uint32_t virtio_snd_pcm_prepare(VirtIOSound *s,
>> uint32_t stream_id)
>>
>> stream = virtio_snd_pcm_get_stream(s, stream_id);
>> if (stream == NULL) {
>> - stream = g_new0(VirtIOSoundPCMStream, 1);
>> + stream = &s->streams[stream_id];
>> stream->active = false;
>> - stream->id = stream_id;
>> stream->pcm = s->pcm;
>> - stream->s = s;
>> QSIMPLEQ_INIT(&stream->queue);
>> QSIMPLEQ_INIT(&stream->invalid);
> note: I can't find where s->pcm->streams[stream_id] is reset to NULL
> on pcm_release...
Hi Marc-André,
right, I'll have to remove the subordinate clause from the commit
message where I claim some pointers can be NULL. All streams get
allocated in virtio_snd_realize() with calls of virtio_snd_pcm_prepare()
and get freed in virtio_snd_unrealize().
>> @@ -463,14 +461,6 @@ static uint32_t virtio_snd_pcm_prepare(VirtIOSound *s,
>> uint32_t stream_id)
>> }
>>
>> virtio_snd_get_qemu_audsettings(&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;
>> - stream->info.features = 0;
>> - stream->info.channels_min = 1;
>> - stream->info.channels_max = as.nchannels;
>> - stream->info.formats = supported_formats;
>> - stream->info.rates = supported_rates;
>> stream->params = *params;
>>
>> stream->positions[0] = VIRTIO_SND_CHMAP_FL;
>> @@ -1074,6 +1064,24 @@ static void virtio_snd_realize(DeviceState *dev,
>> Error **errp)
>> vsnd->vmstate =
>> qemu_add_vm_change_state_handler(virtio_snd_vm_state_change, vsnd);
>>
>> + vsnd->streams = g_new0(VirtIOSoundPCMStream, vsnd->snd_conf.streams);
>> +
>> + for (uint32_t i = 0; i < vsnd->snd_conf.streams; i++) {
>> + VirtIOSoundPCMStream *stream = &vsnd->streams[i];
>> +
>> + stream->id = i;
>> + stream->s = vsnd;
>> + stream->info.hdr.hda_fn_nid = VIRTIO_SOUND_HDA_FN_NID;
>> + stream->info.features = 0;
>> + stream->info.formats = supported_formats;
>> + stream->info.rates = supported_rates;
>> + stream->info.direction =
>> + i < vsnd->snd_conf.streams / 2 + (vsnd->snd_conf.streams & 1)
>> + ? VIRTIO_SND_D_OUTPUT : VIRTIO_SND_D_INPUT;
>> + stream->info.channels_min = 1;
>> + stream->info.channels_max = 2;
> Fixed max channels set to 2.. ? before this was set to
> MIN(AUDIO_MAX_CHANNELS, params->channels)
Before my patch params->channels and stream->info.max_channels were also
2 at the time the guest driver queried stream info. They got initialized
in function virtio_snd_realize().
default_params.channels = 2;
...
status = virtio_snd_set_pcm_params(vsnd, i, &default_params);
...
status = virtio_snd_pcm_prepare(vsnd, i);
The guest may not update stream->info variables. They are configuration
information set when QEMU starts. This was wrong in
virtio_snd_pcm_prepare().
>
>> + }
>> +
>> vsnd->pcm = g_new0(VirtIOSoundPCM, 1);
>> vsnd->pcm->snd = vsnd;
>> vsnd->pcm->streams =
>> @@ -1314,14 +1322,13 @@ static void virtio_snd_unrealize(DeviceState *dev)
>> qemu_del_vm_change_state_handler(vsnd->vmstate);
>> trace_virtio_snd_unrealize(vsnd);
>>
>> - if (vsnd->pcm) {
>> + if (vsnd->streams) {
>> if (vsnd->pcm->streams) {
>> for (uint32_t i = 0; i < vsnd->snd_conf.streams; i++) {
>> stream = vsnd->pcm->streams[i];
>> if (stream) {
>> virtio_snd_process_cmdq(stream->s);
>> virtio_snd_pcm_close(stream);
>> - g_free(stream);
>> }
>> }
>> g_free(vsnd->pcm->streams);
>> @@ -1329,6 +1336,8 @@ static void virtio_snd_unrealize(DeviceState *dev)
>> g_free(vsnd->pcm->pcm_params);
>> g_free(vsnd->pcm);
>> vsnd->pcm = NULL;
>> + g_free(vsnd->streams);
>> + vsnd->streams = NULL;
>> }
>> AUD_remove_card(&vsnd->card);
>> virtio_delete_queue(vsnd->queues[VIRTIO_SND_VQ_CONTROL]);
>> diff --git a/include/hw/audio/virtio-snd.h b/include/hw/audio/virtio-snd.h
>> index ea6315f59b..05b4490488 100644
>> --- a/include/hw/audio/virtio-snd.h
>> +++ b/include/hw/audio/virtio-snd.h
>> @@ -216,6 +216,7 @@ struct VirtIOSound {
>> VirtQueue *queues[VIRTIO_SND_VQ_MAX];
>> uint64_t features;
>> VirtIOSoundPCM *pcm;
>> + VirtIOSoundPCMStream *streams;
>> QEMUSoundCard card;
>> VMChangeStateEntry *vmstate;
>> virtio_snd_config snd_conf;
>> --
>> 2.35.3
>>
