kern/57981: ld@virtio confuses F_SEG_MAX negotiation

[isaki%pastel-flower.jp@localhost]

>Number:         57981
>Category:       kern
>Synopsis:       ld@virtio confuses F_SEG_MAX negotiation
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Mar 02 06:15:00 +0000 2024
>Originator:     Tetsuya Isaki
>Release:        NetBSD-current around 20240225
>Organization:
>Environment:
NetBSD 10.99.10 virt68k
>Description:
In ld_virtio_attach(), F_SEG_MAX negotiation is confused.
(Although I cannot find what F_SEG_MAX value means exactly in
virtio 1.0 spec)

According to line 330 and 333, maxnsegs should be the number of
data segments.  If so, the comparison at line 322 is strange.
It compares the number of data segments and the number of non-data
segments.  It should be "if (maxnsegs == 0) then error"?
Or, the name of VIRTIO_BLK_MIN_SEGMENTS = 2 is misleading to me.
I'm not sure but how about VIRTIO_BLK_CTRL_SEGMENTS = 2 ?

src/sys/dev/pci/ld_virtio.c:
   73 /*
   74  * Each block request uses at least two segments - one for the header
   75  * and one for the status.
   76 */
   77 #define VIRTIO_BLK_MIN_SEGMENTS 2
    :
  261 ld_virtio_attach(device_t parent, device_t self, void *aux)
  262 {
    :
  319     if (features & VIRTIO_BLK_F_SEG_MAX) {
  320         maxnsegs = virtio_read_device_config_4(vsc,
  321             VIRTIO_BLK_CONFIG_SEG_MAX);
  322         if (maxnsegs < VIRTIO_BLK_MIN_SEGMENTS) {
  323             aprint_error_dev(sc->sc_dev,
  324                 "Too small SEG_MAX %d minimum is %d\n",
  325                 maxnsegs, VIRTIO_BLK_MIN_SEGMENTS);
  326             maxnsegs = maxxfersize / NBPG;
  327             // goto err;
  328         }
  329     } else
  330         maxnsegs = maxxfersize / NBPG;
  331
  332     /* 2 for the minimum size */
  333     maxnsegs += VIRTIO_BLK_MIN_SEGMENTS;
>How-To-Repeat:
Code review
>Fix:
N/A