Re: Question about virtio_alloc_virtqueues

that sounds like we can just remove the flags parameter from virtio_alloc_virtqueues() because it doesn't do anything

I'm surprised i haven't seen… noticed any warnings about the flags parameter being unused.

Kind regards,

Mina-------- Original Message --------
On 9 Sept 2023, 11:53, Vincenzo Maffione wrote:

> Hi,
> Looking at the code, it looks like that comment was written at a time where per-virtqueue MSIX interrupts were not available, and virtio_alloc_virtqueues() API was already designed to receive a flag to indicate that per-virtqueue MSIX interrupts were requested. The code you quoted would likely have been a simple placeholder, to be replaced with something like "flags |= VIRTIO_PERVQ_INTR" once the support had come. It's a very common pattern to start with setting flags = 0, and then bitwise or |= flags depending on the situation.
>
> However:
>
> - per-virtqueue MSIX interrupts are now available (e.g. look at vtpci_setup_interrupts()), and
> - virtiqueues interrupt setup happens in a separate API, i.e. virtio_setup_intr(), which takes care all the possible cases
>
> So as far as I can tell that comment is not relevant anymore and can be removed together with the flags.
> Unless I'm wrong, ofc.
>
> Cheers,
> Vincenzo
>
> Il giorno ven 8 set 2023 alle ore 21:05 Mina Galić <freebsd@igalic.co> ha scritto:
>
>> Hi folks,
>>
>> for the past two or so weeks, I've been trying to document the
>> virtio functions: You can see some of my progress here:
>>
>> https://codeberg.org/meena/freebsd-src/commits/branch/improve/virtio
>>
>> I'm currently trying to document virtio_alloc_virtqueues.
>> The second argument, flags, is only ever called with 0, and
>> never passed on to anything. So I thought I'd remove it.
>> However, there *is* this comment in if_vtnet.c:
>>
>> /*
>> * TODO: Enable interrupt binding if this is multiqueue. This will
>> * only matter when per-virtqueue MSIX is available.
>> */
>> if (sc->vtnet_flags & VTNET_FLAG_MQ)
>> flags |= 0;
>>
>> after which flags are still set to 0. for now??
>> What does this mean?
>>
>> Kind regards,
>>
>> Mina Galić