Re: [PATCH v8 02/12] Add virtio-sound-pci device

[Volker Rümelin]

Am 04.09.23 um 12:26 schrieb Manos Pitsidianakis:
> On Mon, 04 Sep 2023 09:32, Volker Rümelin <vr_qemu@t-online.de> wrote:
> > > +static Property virtio_snd_pci_properties[] = {
> > > +    DEFINE_AUDIO_PROPERTIES(VirtIOSoundPCI, vdev.card),
> >
> > I think DEFINE_AUDIO_PROPERTIES should be moved back to virtio-snd.c. 
> > The audiodev property is a virtio-sound property and not a 
> > virtio-sound-pci property.
>
> Hm, is it? Can you instantiate a virtio-sound device without the PCI 
> wrapper? Under hw/audio, DEFINE_AUDIO_PROPERTIES is set in PCI devices 
> as well (e.g. ac97)

Creating a virtio-sound device without the PCI wrapper is possible.
./qemu-system-x86_64 -M microvm -accel kvm -cpu host -m 512m -smp 2 
-serial stdio -device virtio-sound,audiodev=audio0 -audiodev 
pipewire,id=audio0 -display gtk
qemu-system-x86_64: -device virtio-sound,audiodev=audio0: Property 
'virtio-sound.audiodev' not found

If you move DEFINE_AUDIO_PROPERTIES to virtio-snd.c you don't see this 
error message and you can see a virtio-mmio sound device if you type 
info qtree in the QEMU compat monitor.

Now that you asked this question I wonder if this should be #define 
TYPE_VIRTIO_SND "virtio-sound-device". Other virtio devices have the 
-device suffix.

With best regards,
Volker

> > > +    DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
> > > +                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
> > > +    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
> > > +                       DEV_NVECTORS_UNSPECIFIED),
> > > +    DEFINE_PROP_END_OF_LIST(),
> > > +};
> > > +
> > > +static void virtio_snd_pci_realize(VirtIOPCIProxy *vpci_dev, Error 
> > > **errp)
> > > +{
> > > +    VirtIOSoundPCI *dev = VIRTIO_SND_PCI(vpci_dev);
> > > +    DeviceState *vdev = DEVICE(&dev->vdev);
> > > +
> > > +    if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
> > > +        vpci_dev->nvectors = 2;
> > > +    }
> >
> > Why do you need that intermediate step with DEV_NVECTORS_UNSPECIFIED? 
> > Unlike e.g. virtio-scsi-pci and virtio-net-pci devices, the default 
> > value of nvectors is already known at compile time and can be 
> > specified in the property definition.
>
> I did not think this through properly, you are correct. Thank you!
>
> Manos