Re: vmd: reset queue_size if queue_select is invalid

[Mike Larkin]

On Wed, Jul 26, 2017 at 09:37:30PM -0700, Nick Owens wrote:
> hello tech@,
> 
> here is a diff that will follow the virtio spec a little closer, and
> allows 9front's (http://9front.org) virtio-blk driver to correctly find
> the number of queues. i know that virtio-blk only has one queue, but
> the virtio probing code is shared between virtio-blk and virtio-scsi.
> 
> without this change, the size of the first queue is used for all
> subsequently probed queues.
> 
> for completeness i've changed rng and net to do the same as blk.
> 
> some bits from the spec:
> 
> 4.1.4.3.1 - "The device MUST present a 0 in queue_size if the virtqueue
> corresponding to the current queue_select is unavailable."
> 
> 4.1.5.1.3 - "Write the virtqueue index (first queue is 0) to
> queue_select. Read the virtqueue size from queue_size. This controls
> how big the virtqueue is (see 2.4 Virtqueues). If this field is 0, the
> virtqueue does not exist."
> 

Thanks. committed.

Sorry it took so long.

-ml

PS, the diff got mangled below, but I recreated it by hand.



> Index: virtio.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/virtio.c,v
> retrieving revision 1.49
> diff -u -p -u -p -r1.49 virtio.c
> --- virtio.c  30 May 2017 17:56:47 -  1.49
> +++ virtio.c  27 Jul 2017 04:35:46 -
> @@ -150,8 +150,10 @@ void
>  viornd_update_qs(void)
>  {
>   /* Invalid queue? */
> - if (viornd.cfg.queue_select > 0)
> + if (viornd.cfg.queue_select > 0) {
> + viornd.cfg.queue_size = 0;
>   return;
> + }
>  
>   /* Update queue address/size based on queue select */
>   viornd.cfg.queue_address =
> viornd.vq[viornd.cfg.queue_select].qa; @@ -324,8 +326,10 @@ void
>  vioblk_update_qs(struct vioblk_dev *dev)
>  {
>   /* Invalid queue? */
> - if (dev->cfg.queue_select > 0)
> + if (dev->cfg.queue_select > 0) {
> + dev->cfg.queue_size = 0;
>   return;
> + }
>  
>   /* Update queue address/size based on queue select */
>   dev->cfg.queue_address = dev->vq[dev->cfg.queue_select].qa;
> @@ -1037,8 +1041,10 @@ void
>  vionet_update_qs(struct vionet_dev *dev)
>  {
>   /* Invalid queue? */
> - if (dev->cfg.queue_select > 1)
> + if (dev->cfg.queue_select > 1) {
> + dev->cfg.queue_size = 0;
>   return;
> + }
>  
>   /* Update queue address/size based on queue select */
>   dev->cfg.queue_address = dev->vq[dev->cfg.queue_select].qa;
> 

Re: vmd: reset queue_size if queue_select is invalid

[Mike Larkin]

On Thu, Aug 03, 2017 at 04:18:54PM -0700, Nick Owens wrote:
> ping?
> 

Sorry this got buried. Should have time later today (yes, really) :)

-ml

> On Thu, Jul 27, 2017 at 10:20 AM, Mike Larkin  wrote:
> > On Wed, Jul 26, 2017 at 09:37:30PM -0700, Nick Owens wrote:
> >> hello tech@,
> >>
> >> here is a diff that will follow the virtio spec a little closer, and
> >> allows 9front's (http://9front.org) virtio-blk driver to correctly find
> >> the number of queues. i know that virtio-blk only has one queue, but
> >> the virtio probing code is shared between virtio-blk and virtio-scsi.
> >>
> >> without this change, the size of the first queue is used for all
> >> subsequently probed queues.
> >>
> >> for completeness i've changed rng and net to do the same as blk.
> >>
> >> some bits from the spec:
> >>
> >> 4.1.4.3.1 - "The device MUST present a 0 in queue_size if the virtqueue
> >> corresponding to the current queue_select is unavailable."
> >>
> >> 4.1.5.1.3 - "Write the virtqueue index (first queue is 0) to
> >> queue_select. Read the virtqueue size from queue_size. This controls
> >> how big the virtqueue is (see 2.4 Virtqueues). If this field is 0, the
> >> virtqueue does not exist."
> >>
> >
> > vmd diffs are always welcome, thanks. I'll take a look at this later today.
> >
> > -ml
> >
> >
> >> Index: virtio.c
> >> ===
> >> RCS file: /cvs/src/usr.sbin/vmd/virtio.c,v
> >> retrieving revision 1.49
> >> diff -u -p -u -p -r1.49 virtio.c
> >> --- virtio.c  30 May 2017 17:56:47 -  1.49
> >> +++ virtio.c  27 Jul 2017 04:35:46 -
> >> @@ -150,8 +150,10 @@ void
> >>  viornd_update_qs(void)
> >>  {
> >>   /* Invalid queue? */
> >> - if (viornd.cfg.queue_select > 0)
> >> + if (viornd.cfg.queue_select > 0) {
> >> + viornd.cfg.queue_size = 0;
> >>   return;
> >> + }
> >>
> >>   /* Update queue address/size based on queue select */
> >>   viornd.cfg.queue_address =
> >> viornd.vq[viornd.cfg.queue_select].qa; @@ -324,8 +326,10 @@ void
> >>  vioblk_update_qs(struct vioblk_dev *dev)
> >>  {
> >>   /* Invalid queue? */
> >> - if (dev->cfg.queue_select > 0)
> >> + if (dev->cfg.queue_select > 0) {
> >> + dev->cfg.queue_size = 0;
> >>   return;
> >> + }
> >>
> >>   /* Update queue address/size based on queue select */
> >>   dev->cfg.queue_address = dev->vq[dev->cfg.queue_select].qa;
> >> @@ -1037,8 +1041,10 @@ void
> >>  vionet_update_qs(struct vionet_dev *dev)
> >>  {
> >>   /* Invalid queue? */
> >> - if (dev->cfg.queue_select > 1)
> >> + if (dev->cfg.queue_select > 1) {
> >> + dev->cfg.queue_size = 0;
> >>   return;
> >> + }
> >>
> >>   /* Update queue address/size based on queue select */
> >>   dev->cfg.queue_address = dev->vq[dev->cfg.queue_select].qa;
> >>

Re: vmd: reset queue_size if queue_select is invalid

[Nick Owens]

ping?

On Thu, Jul 27, 2017 at 10:20 AM, Mike Larkin  wrote:
> On Wed, Jul 26, 2017 at 09:37:30PM -0700, Nick Owens wrote:
>> hello tech@,
>>
>> here is a diff that will follow the virtio spec a little closer, and
>> allows 9front's (http://9front.org) virtio-blk driver to correctly find
>> the number of queues. i know that virtio-blk only has one queue, but
>> the virtio probing code is shared between virtio-blk and virtio-scsi.
>>
>> without this change, the size of the first queue is used for all
>> subsequently probed queues.
>>
>> for completeness i've changed rng and net to do the same as blk.
>>
>> some bits from the spec:
>>
>> 4.1.4.3.1 - "The device MUST present a 0 in queue_size if the virtqueue
>> corresponding to the current queue_select is unavailable."
>>
>> 4.1.5.1.3 - "Write the virtqueue index (first queue is 0) to
>> queue_select. Read the virtqueue size from queue_size. This controls
>> how big the virtqueue is (see 2.4 Virtqueues). If this field is 0, the
>> virtqueue does not exist."
>>
>
> vmd diffs are always welcome, thanks. I'll take a look at this later today.
>
> -ml
>
>
>> Index: virtio.c
>> ===
>> RCS file: /cvs/src/usr.sbin/vmd/virtio.c,v
>> retrieving revision 1.49
>> diff -u -p -u -p -r1.49 virtio.c
>> --- virtio.c  30 May 2017 17:56:47 -  1.49
>> +++ virtio.c  27 Jul 2017 04:35:46 -
>> @@ -150,8 +150,10 @@ void
>>  viornd_update_qs(void)
>>  {
>>   /* Invalid queue? */
>> - if (viornd.cfg.queue_select > 0)
>> + if (viornd.cfg.queue_select > 0) {
>> + viornd.cfg.queue_size = 0;
>>   return;
>> + }
>>
>>   /* Update queue address/size based on queue select */
>>   viornd.cfg.queue_address =
>> viornd.vq[viornd.cfg.queue_select].qa; @@ -324,8 +326,10 @@ void
>>  vioblk_update_qs(struct vioblk_dev *dev)
>>  {
>>   /* Invalid queue? */
>> - if (dev->cfg.queue_select > 0)
>> + if (dev->cfg.queue_select > 0) {
>> + dev->cfg.queue_size = 0;
>>   return;
>> + }
>>
>>   /* Update queue address/size based on queue select */
>>   dev->cfg.queue_address = dev->vq[dev->cfg.queue_select].qa;
>> @@ -1037,8 +1041,10 @@ void
>>  vionet_update_qs(struct vionet_dev *dev)
>>  {
>>   /* Invalid queue? */
>> - if (dev->cfg.queue_select > 1)
>> + if (dev->cfg.queue_select > 1) {
>> + dev->cfg.queue_size = 0;
>>   return;
>> + }
>>
>>   /* Update queue address/size based on queue select */
>>   dev->cfg.queue_address = dev->vq[dev->cfg.queue_select].qa;
>>

Re: vmd: reset queue_size if queue_select is invalid

[Mike Larkin]

On Wed, Jul 26, 2017 at 09:37:30PM -0700, Nick Owens wrote:
> hello tech@,
> 
> here is a diff that will follow the virtio spec a little closer, and
> allows 9front's (http://9front.org) virtio-blk driver to correctly find
> the number of queues. i know that virtio-blk only has one queue, but
> the virtio probing code is shared between virtio-blk and virtio-scsi.
> 
> without this change, the size of the first queue is used for all
> subsequently probed queues.
> 
> for completeness i've changed rng and net to do the same as blk.
> 
> some bits from the spec:
> 
> 4.1.4.3.1 - "The device MUST present a 0 in queue_size if the virtqueue
> corresponding to the current queue_select is unavailable."
> 
> 4.1.5.1.3 - "Write the virtqueue index (first queue is 0) to
> queue_select. Read the virtqueue size from queue_size. This controls
> how big the virtqueue is (see 2.4 Virtqueues). If this field is 0, the
> virtqueue does not exist."
> 

vmd diffs are always welcome, thanks. I'll take a look at this later today.

-ml


> Index: virtio.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/virtio.c,v
> retrieving revision 1.49
> diff -u -p -u -p -r1.49 virtio.c
> --- virtio.c  30 May 2017 17:56:47 -  1.49
> +++ virtio.c  27 Jul 2017 04:35:46 -
> @@ -150,8 +150,10 @@ void
>  viornd_update_qs(void)
>  {
>   /* Invalid queue? */
> - if (viornd.cfg.queue_select > 0)
> + if (viornd.cfg.queue_select > 0) {
> + viornd.cfg.queue_size = 0;
>   return;
> + }
>  
>   /* Update queue address/size based on queue select */
>   viornd.cfg.queue_address =
> viornd.vq[viornd.cfg.queue_select].qa; @@ -324,8 +326,10 @@ void
>  vioblk_update_qs(struct vioblk_dev *dev)
>  {
>   /* Invalid queue? */
> - if (dev->cfg.queue_select > 0)
> + if (dev->cfg.queue_select > 0) {
> + dev->cfg.queue_size = 0;
>   return;
> + }
>  
>   /* Update queue address/size based on queue select */
>   dev->cfg.queue_address = dev->vq[dev->cfg.queue_select].qa;
> @@ -1037,8 +1041,10 @@ void
>  vionet_update_qs(struct vionet_dev *dev)
>  {
>   /* Invalid queue? */
> - if (dev->cfg.queue_select > 1)
> + if (dev->cfg.queue_select > 1) {
> + dev->cfg.queue_size = 0;
>   return;
> + }
>  
>   /* Update queue address/size based on queue select */
>   dev->cfg.queue_address = dev->vq[dev->cfg.queue_select].qa;
> 

vmd: reset queue_size if queue_select is invalid

[Nick Owens]

hello tech@,

here is a diff that will follow the virtio spec a little closer, and
allows 9front's (http://9front.org) virtio-blk driver to correctly find
the number of queues. i know that virtio-blk only has one queue, but
the virtio probing code is shared between virtio-blk and virtio-scsi.

without this change, the size of the first queue is used for all
subsequently probed queues.

for completeness i've changed rng and net to do the same as blk.

some bits from the spec:

4.1.4.3.1 - "The device MUST present a 0 in queue_size if the virtqueue
corresponding to the current queue_select is unavailable."

4.1.5.1.3 - "Write the virtqueue index (first queue is 0) to
queue_select. Read the virtqueue size from queue_size. This controls
how big the virtqueue is (see 2.4 Virtqueues). If this field is 0, the
virtqueue does not exist."

Index: virtio.c
===
RCS file: /cvs/src/usr.sbin/vmd/virtio.c,v
retrieving revision 1.49
diff -u -p -u -p -r1.49 virtio.c
--- virtio.c30 May 2017 17:56:47 -  1.49
+++ virtio.c27 Jul 2017 04:35:46 -
@@ -150,8 +150,10 @@ void
 viornd_update_qs(void)
 {
/* Invalid queue? */
-   if (viornd.cfg.queue_select > 0)
+   if (viornd.cfg.queue_select > 0) {
+   viornd.cfg.queue_size = 0;
return;
+   }
 
/* Update queue address/size based on queue select */
viornd.cfg.queue_address =
viornd.vq[viornd.cfg.queue_select].qa; @@ -324,8 +326,10 @@ void
 vioblk_update_qs(struct vioblk_dev *dev)
 {
/* Invalid queue? */
-   if (dev->cfg.queue_select > 0)
+   if (dev->cfg.queue_select > 0) {
+   dev->cfg.queue_size = 0;
return;
+   }
 
/* Update queue address/size based on queue select */
dev->cfg.queue_address = dev->vq[dev->cfg.queue_select].qa;
@@ -1037,8 +1041,10 @@ void
 vionet_update_qs(struct vionet_dev *dev)
 {
/* Invalid queue? */
-   if (dev->cfg.queue_select > 1)
+   if (dev->cfg.queue_select > 1) {
+   dev->cfg.queue_size = 0;
return;
+   }
 
/* Update queue address/size based on queue select */
dev->cfg.queue_address = dev->vq[dev->cfg.queue_select].qa;