Date: Sat, 9 Sep 2023 12:53:01 +0200 From: Vincenzo Maffione <vmaffione@freebsd.org> To: =?UTF-8?Q?Mina_Gali=C4=87?= <freebsd@igalic.co> Cc: freebsd-hackers <freebsd-hackers@freebsd.org>, "freebsd-virtualization@FreeBSD.org" <freebsd-virtualization@freebsd.org> Subject: Re: Question about virtio_alloc_virtqueues Message-ID: <CA%2B_eA9h_AAh95sJ7q1p3J4xCZcd73WWzHkA=BfR6dFYA=%2BG5uw@mail.gmail.com> In-Reply-To: <ia9RXdtQafOfdOfg9wml1ApOEb4ivdf8lbRf74J1XYzA8HIDoEWQ75ScEQY8DpmUHlfMraZmgY0XlgaMWid46UBEIOeCBiOQxsFI4TIoIW4=@igalic.co> References: <ia9RXdtQafOfdOfg9wml1ApOEb4ivdf8lbRf74J1XYzA8HIDoEWQ75ScEQY8DpmUHlfMraZmgY0XlgaMWid46UBEIOeCBiOQxsFI4TIoIW4=@igalic.co>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] 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ć > > [-- Attachment #2 --] <div dir="ltr"><div>Hi,</div><div> 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.<br></div><div><br></div><div>However:</div><div><ul><li>per-virtqueue MSIX interrupts are now available (e.g. look at vtpci_setup_interrupts()), and</li><li>virtiqueues interrupt setup happens in a separate API, i.e. virtio_setup_intr(), which takes care all the possible cases<br></li></ul><div>So as far as I can tell that comment is not relevant anymore and can be removed together with the flags.</div><div>Unless I'm wrong, ofc.<br></div><div><br></div><div>Cheers,</div><div> Vincenzo<br></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Il giorno ven 8 set 2023 alle ore 21:05 Mina Galić <<a href="mailto:freebsd@igalic.co">freebsd@igalic.co</a>> ha scritto:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi folks,<br> <br> for the past two or so weeks, I've been trying to document the<br> virtio functions: You can see some of my progress here:<br> <br> <a href="https://codeberg.org/meena/freebsd-src/commits/branch/improve/virtio" rel="noreferrer" target="_blank">https://codeberg.org/meena/freebsd-src/commits/branch/improve/virtio</a><br>; <br> I'm currently trying to document virtio_alloc_virtqueues.<br> The second argument, flags, is only ever called with 0, and<br> never passed on to anything. So I thought I'd remove it.<br> However, there *is* this comment in if_vtnet.c:<br> <br> /*<br> * TODO: Enable interrupt binding if this is multiqueue. This will<br> * only matter when per-virtqueue MSIX is available.<br> */<br> if (sc->vtnet_flags & VTNET_FLAG_MQ)<br> flags |= 0;<br> <br> <br> after which flags are still set to 0. for now??<br> What does this mean?<br> <br> Kind regards,<br> <br> Mina Galić<br> <br> </blockquote></div>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CA%2B_eA9h_AAh95sJ7q1p3J4xCZcd73WWzHkA=BfR6dFYA=%2BG5uw>