kern/58555: [PATCH] Kernel panic during boot when using viocon with PCIe

[gorg%gorgnet.net@localhost]

>Number:         58555
>Category:       kern
>Synopsis:       Kernel panic during boot when using viocon with PCIe
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Aug 04 23:00:00 +0000 2024
>Originator:     gorg%gorgnet.net@localhost
>Release:        NetBSD 10.0
>Organization:
>Environment:
System: NetBSD 10.0 NetBSD 10.0 (TEST) #3: Sun Aug  4 04:34:22 MDT 2024 evbarm
Architecture: aarch64
Machine: evbarm
>Description:
Upon boot, the kernel panics with the following backtrace:

cpu0: Begin traceback...
trace fp ffffc00001007540
fp ffffc00001007570 vpanic() at ffffc000004f0128 netbsd:vpanic+0x178
fp ffffc000010075d0 panic() at ffffc000004f0234 netbsd:panic+0x44
fp ffffc00001007660 data_abort_handler() at ffffc000000a969c netbsd:data_abort_handler+0x1ec
tf ffffc000010076d0 el1_trap() at ffffc000000aaf84 netbsd:el1_vectors+0x784
---- Data Abort (EL1): trapframe 0xffffc000010076d0 (304 bytes) ----
pc=ffffc000001377e4,   spsr=0000000060400005
esr=0000000096000004,    far=00000000000000d0
x0=ffff0000bf299400,     x1=ffffc000deb77000
x2=0000000000000004,     x3=0000000000000000
x4=0000000000000000,     x5=ffffc00040184020
x6=ffff0000bf381100,     x7=ffff0000bf221b00
x8=0000000000001000,     x9=0000000000000004
x10=ffffc000000a0844,    x11=000000000000003f
x12=fffffc0002fc80c0,    x13=fffffc0002fc80d0
x14=0000000000000020,    x15=ffff0000bf203400
x16=ffffc000000a0844,    x17=c6ee25699db8685b
x18=0000000000001000,    x19=ffff0000bf299600
x20=ffff0000bf299400,    x21=0000000000000000
x22=ffff0000bf2996a0,    x23=0000000000000080
x24=ffffc00001007c70,    x25=ffff0000bf208100
x26=0000000000000001,    x27=ffffc000009a36f8
x28=0000000000000000, fp=x29=ffffc00001007a00
lr=x30=ffffc000006d9390,     sp=ffffc00001007a00
------------------------------------------------
fp ffffc00001007a00 virtio_pci_kick_10() at ffffc000001377e4 netbsd:virtio_pci_kick_10+0x30
fp ffffc00001007a40 viocon_rx_fill() at ffffc0000036c63c netbsd:viocon_rx_fill+0xdc
fp ffffc00001007a80 viocon_attach() at ffffc0000036c714 netbsd:viocon_attach+0xc4
fp ffffc00001007ab0 config_attach_internal() at ffffc000004d5988 netbsd:config_attach_internal+0x1f8
fp ffffc00001007b10 config_found() at ffffc000004d5b60 netbsd:config_found+0xf0
fp ffffc00001007b70 virtio_pci_attach() at ffffc00000138d08 netbsd:virtio_pci_attach+0x288
fp ffffc00001007ca0 config_attach_internal() at ffffc000004d5988 netbsd:config_attach_internal+0x1f8
fp ffffc00001007d00 config_found() at ffffc000004d5b60 netbsd:config_found+0xf0
fp ffffc00001007d60 pci_probe_device() at ffffc000000b9d74 netbsd:pci_probe_device+0x5b4
fp ffffc00001007ea0 pci_enumerate_bus() at ffffc000000b9f48 netbsd:pci_enumerate_bus+0x1b8
fp ffffc00001007f50 pciattach() at ffffc000000ba2c8 netbsd:pciattach+0x138
fp ffffc00001007f90 config_attach_internal() at ffffc000004d5988 netbsd:config_attach_internal+0x1f8
fp ffffc00001007ff0 config_found() at ffffc000004d5b60 netbsd:config_found+0xf0
fp ffffc00001008050 ppbattach() at ffffc000000d2da4 netbsd:ppbattach+0x294
fp ffffc00001008150 config_attach_internal() at ffffc000004d5988 netbsd:config_attach_internal+0x1f8
fp ffffc000010081b0 config_found() at ffffc000004d5b60 netbsd:config_found+0xf0
fp ffffc00001008210 pci_probe_device() at ffffc000000b9d74 netbsd:pci_probe_device+0x5b4
fp ffffc00001008350 pci_enumerate_bus() at ffffc000000b9f48 netbsd:pci_enumerate_bus+0x1b8
fp ffffc00001008400 pciattach() at ffffc000000ba2c8 netbsd:pciattach+0x138
fp ffffc00001008440 config_attach_internal() at ffffc000004d5988 netbsd:config_attach_internal+0x1f8
fp ffffc000010084a0 config_found() at ffffc000004d5b60 netbsd:config_found+0xf0
fp ffffc00001008500 acpipchb_attach() at ffffc0000000e670 netbsd:acpipchb_attach+0x1f0
fp ffffc000010085f0 config_attach_internal() at ffffc000004d5988 netbsd:config_attach_internal+0x1f8
fp ffffc00001008650 config_found() at ffffc000004d5b60 netbsd:config_found+0xf0
fp ffffc000010086b0 acpi_rescan() at ffffc000000721ac netbsd:acpi_rescan+0x2ec
fp ffffc000010087d0 acpi_attach() at ffffc000000726ec netbsd:acpi_attach+0x3bc
fp ffffc000010088a0 config_attach_internal() at ffffc000004d5988 netbsd:config_attach_internal+0x1f8
fp ffffc00001008900 config_found() at ffffc000004d5b60 netbsd:config_found+0xf0
fp ffffc00001008960 acpi_fdt_attach() at ffffc00000070560 netbsd:acpi_fdt_attach+0xb0
fp ffffc000010089e0 config_attach_internal() at ffffc000004d5988 netbsd:config_attach_internal+0x1f8
fp ffffc00001008a40 config_attach() at ffffc000004d5c00 netbsd:config_attach+0x50
fp ffffc00001008a80 fdt_scan() at ffffc000005d3144 netbsd:fdt_scan+0x164
fp ffffc00001008c10 fdt_rescan() at ffffc000005d3660 netbsd:fdt_rescan+0x50
fp ffffc00001008c40 config_attach_internal() at ffffc000004d5988 netbsd:config_attach_internal+0x1f8
fp ffffc00001008ca0 config_found() at ffffc000004d5b60 netbsd:config_found+0xf0
fp ffffc00001008d00 arm_fdt_attach() at ffffc0000006b258 netbsd:arm_fdt_attach+0x94
fp ffffc00001008d60 config_attach_internal() at ffffc000004d5988 netbsd:config_attach_internal+0x1f8
fp ffffc00001008dc0 config_rootfound() at ffffc000004d5c8c netbsd:config_rootfound+0x58
fp ffffc00001008e20 cpu_configure() at ffffc00000066d1c netbsd:cpu_configure+0x4c
fp ffffc00001008e50 main() at ffffc0000073d550 netbsd:main+0x260
fp 0000000000000000 aarch64_start() at ffffc0000000189c netbsd:aarch64_start+0x109c

The last frame is within the source file virtio_pci.c at the following lines:

   678		unsigned offset = sc->sc_vqs[idx].vq_notify_off *
   679			psc->sc_notify_off_multiplier;

Since revision 1.66 in virtio.c, sc_vqs has been set in
virtio_child_attach_finish rather than virtio_child_attach_start:

http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/pci/virtio.c?rev=1.66&content-type=text/x-cvsweb-markup
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/pci/virtio.c.diff?r1=1.66&r2=1.65

Therefore, viocon_rx_fill must be called after virtio_child_attach_finish. The
patch below remedies this. When not using virtio 1.0 (which seems to usually be
the case when using PCI rather than PCIe), virtio_pci_kick_09 is called instead
of virtio_pci_kick_10. virtio_pci_kick_09 does not dereference sc_vqs.

>How-To-Repeat:
Boot a kernel compiled with the viocon driver within a virtual machine with a
virtconsole device attached to a PCIe root port, like so:

qemu-system-aarch64 -M virt -cpu neoverse-n1 ... \
  -device pcie-root-port,bus=pcie.0,chassis=1,id=pcie.1
  -device virtio-serial-pci,bus=pcie.1 \
  -device virtconsole \
  ...

Such a configuration may also be found on some Hetzner VPSs.

>Fix:
Index: sys/dev/virtio/viocon.c
===================================================================
RCS file: /cvsroot/src/sys/dev/virtio/viocon.c,v
retrieving revision 1.5.4.1
diff -u -r1.5.4.1 viocon.c
--- sys/dev/virtio/viocon.c	13 May 2023 10:56:10 -0000	1.5.4.1
+++ sys/dev/virtio/viocon.c	1 Aug 2024 07:30:44 -0000
@@ -222,12 +222,13 @@
 		printf("\n%s: viocon_port_create failed\n", __func__);
 		goto err;
 	}
-	viocon_rx_fill(sc->sc_ports[0]);
 
 	if (virtio_child_attach_finish(vsc, sc->sc_vqs, nvqs,
 	    /*config_change*/NULL, /*req_flags*/0) != 0)
 		goto err;
 
+	viocon_rx_fill(sc->sc_ports[0]);
+
 	return;
 err:
 	kmem_free(sc->sc_vqs, nvqs * sizeof(sc->sc_vqs[0]));