Hi On Fri, Jul 10, 2026 at 6:56 PM Alexander Mikhalitsyn <[email protected]> wrote: > > Am Fr., 26. Juni 2026 um 16:12 Uhr schrieb <[email protected]>: > > > > > So far, only rudimentary checks have been made to ensure that > > > the guest only performs state transitions permitted in > > > virtio-v1.2-csd01 5.14.6.6.1 PCM Command Lifecycle. Add a state > > > variable per audio stream and check all state transitions. > > > > > > Because only permitted state transitions are possible, only one > > > copy of the audio stream parameters is required and these do not > > > need to be initialised with default values. > > > > > > The state variable will also make it easier to restore the audio > > > stream after migration. > > > > > > Signed-off-by: Volker Rümelin <[email protected]> > > > [AM: there were too many conflicts, I did `git checkout --ours -- <.>` > > > and then reimplemented the patch idea > > > /AM] > > > Signed-off-by: Alexander Mikhalitsyn > > > <[email protected]> > > > Message-ID: <[email protected]> > > > > > > diff --git a/include/hw/audio/virtio-snd.h b/include/hw/audio/virtio-snd.h > > > index 41c63b3f23b..72ef34e0976 100644 > > > --- a/include/hw/audio/virtio-snd.h > > > +++ b/include/hw/audio/virtio-snd.h > > > @@ -75,8 +75,6 @@ typedef struct VirtIOSoundPCMStream > > > VirtIOSoundPCMStream; > > > > > > typedef struct virtio_snd_ctrl_command virtio_snd_ctrl_command; > > > > > > -typedef struct VirtIOSoundPCM VirtIOSoundPCM; > > > - > > > typedef struct VirtIOSoundPCMBuffer VirtIOSoundPCMBuffer; > > > > > > /* > > > @@ -121,31 +119,18 @@ struct VirtIOSoundPCMBuffer { > > > uint8_t data[]; > > > }; > > > > > > -struct VirtIOSoundPCM { > > > - /* > > > - * PCM parameters are a separate field instead of a > > > VirtIOSoundPCMStream > > > - * field, because the operation of PCM control requests is first > > > - * VIRTIO_SND_R_PCM_SET_PARAMS and then VIRTIO_SND_R_PCM_PREPARE; > > > this > > > - * means that some times we get parameters without having an > > > allocated > > > - * stream yet. > > > - */ > > > - virtio_snd_pcm_set_params *pcm_params; > > > - VirtIOSoundPCMStream **streams; > > > -}; > > > - > > > struct VirtIOSoundPCMStream { > > > virtio_snd_pcm_info info; > > > virtio_snd_pcm_set_params params; > > > + uint32_t state; > > > /* channel position values (VIRTIO_SND_CHMAP_XXX) */ > > > uint8_t positions[VIRTIO_SND_CHMAP_MAX_SIZE]; > > > VirtIOSound *s; > > > - bool flushing; > > > audsettings as; > > > union { > > > SWVoiceIn *in; > > > SWVoiceOut *out; > > > } voice; > > > - bool active; > > > uint32_t latency_bytes; > > > QSIMPLEQ_HEAD(, VirtIOSoundPCMBuffer) queue; > > > }; > > > @@ -212,7 +197,6 @@ struct VirtIOSound { > > > > > > VirtQueue *queues[VIRTIO_SND_VQ_MAX]; > > > uint64_t features; > > > - VirtIOSoundPCM pcm; > > > VirtIOSoundPCMStream *streams; > > > AudioBackend *audio_be; > > > VMChangeStateEntry *vmstate; > > > diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c > > > index 300ba13ffeb..68d737478f5 100644 > > > --- a/hw/audio/virtio-snd.c > > > +++ b/hw/audio/virtio-snd.c > > > @@ -30,11 +30,30 @@ > > > #define VIRTIO_SOUND_CHMAP_DEFAULT 0 > > > #define VIRTIO_SOUND_HDA_FN_NID 0 > > > > > > +#define VSND_PCMSTREAM_STATE_F_PARAMS_SET 0x10000 > > > +#define VSND_PCMSTREAM_STATE_F_PREPARED 0x20000 > > > +#define VSND_PCMSTREAM_STATE_F_ACTIVE 0x40000 > > > + > > > +#define VSND_PCMSTREAM_STATE_UNINITIALIZED 0 > > > +#define VSND_PCMSTREAM_STATE_PARAMS_SET (1 \ > > > + | > > > VSND_PCMSTREAM_STATE_F_PARAMS_SET) > > > +#define VSND_PCMSTREAM_STATE_PREPARED (2 \ > > > + | > > > VSND_PCMSTREAM_STATE_F_PARAMS_SET \ > > > + | > > > VSND_PCMSTREAM_STATE_F_PREPARED) > > > +#define VSND_PCMSTREAM_STATE_STARTED (4 \ > > > + | > > > VSND_PCMSTREAM_STATE_F_PARAMS_SET \ > > > + | > > > VSND_PCMSTREAM_STATE_F_PREPARED \ > > > + | > > > VSND_PCMSTREAM_STATE_F_ACTIVE) > > > +#define VSND_PCMSTREAM_STATE_STOPPED (6 \ > > > + | > > > VSND_PCMSTREAM_STATE_F_PARAMS_SET \ > > > + | > > > VSND_PCMSTREAM_STATE_F_PREPARED) > > > +#define VSND_PCMSTREAM_STATE_RELEASED (7 \ > > > + | > > > VSND_PCMSTREAM_STATE_F_PARAMS_SET) > > > > Dear Marc-André, > > > Hmm, those 1,2,4,6,7 values matches the spec 5.14.6.6.1 states. Ok, but > > worth a > > comment imho. > > Manos made a point that we can simplify these definitions and instead > of using bitmask, > just have enum like: > > + enum virtio_snd_pcm_state { > + VIRTIO_SND_PCM_STATE_UNINIT = 0, > + VIRTIO_SND_PCM_STATE_PARAMS_SET, > + VIRTIO_SND_PCM_STATE_PREPARED, > + VIRTIO_SND_PCM_STATE_STARTED, > + VIRTIO_SND_PCM_STATE_STOPPED, > + VIRTIO_SND_PCM_STATE_RELEASED, > + } > > (see https://lore.kernel.org/qemu-devel/[email protected]/#t ) > > WDYT?
Sure thanks
