Re: CVS commit: src/sys/dev/pci
On Wed, Aug 21, 2024 at 01:42:28AM -0700, John Nemeth wrote: > } Log Message: > } Add Areca ARC-1224 > > I noticed that you mentioned newer Areca devices on icb. Is > there a particular device that you're interested in. I have an > updated version of arcmsr(4) that I've been meaning to clean up > and commit. It has been tested with at least one device newer than > what the in-tree code has. It was mostly done with code that Areca > provided with some cleanup by myself (especially the error paths). I was interested in perhaps importing or integrating at least some of the Areca-provided driver sources - but actual work on that on my part has been limited to eyeballing the diffs and wondering whether there were a cleaner way to integrate the five different variants that it supports (and the accompanying 2x size increase in both the header and .c files). It sounds like you're way, way further along than I am - if you were willing and able to commit, I'd heartily encourage you to do so. (I do *not* have any of the newer devices in question, FWIW. I'd also note that the FreeBSD driver for the non-SAS cards looks radically different from arcmsr and also requires linking in a binary blob, alas.)
Re: CVS commit: src/sys/dev/pci
On Aug 20, 22:44, "Tom Spindler" wrote: } } Module Name: src } Committed By: dogcow } Date: Tue Aug 20 22:44:04 UTC 2024 } } Modified Files: } src/sys/dev/pci: pcidevs } } Log Message: } Add Areca ARC-1224 Hi Tom, I noticed that you mentioned newer Areca devices on icb. Is there a particular device that you're interested in. I have an updated version of arcmsr(4) that I've been meaning to clean up and commit. It has been tested with at least one device newer than what the in-tree code has. It was mostly done with code that Areca provided with some cleanup by myself (especially the error paths). }-- End of excerpt from "Tom Spindler"
Re: CVS commit: src/sys/dev/usb
> Module Name:src > Committed By: riastradh > Date: Sun Jun 30 16:35:19 UTC 2024 > > Modified Files: > src/sys/dev/usb: if_url.c > > Log Message: > url(4): uint32_t for 32-bit hash so h>>31 becomes 0/1, not +1/-1. That was supposed to read: url(4): uint32_t for 32-bit hash so h>>31 becomes 0/1, not 0/-1.
Re: CVS commit: src/sys/dev/fdt
Thank you, too, for clarification!
rin
On 2024/06/13 5:10, Nick Hudson wrote:
Thanks for the fix.
The bug affects enable and disable in the all (-1) indexes case and
references out of bounds data.
Nick
On 12/06/2024 08:36, Rin Okuyama wrote:
Hmm, there was a confusion for my side.
This bug affected cases where
(1) index >= 1 is explicitly specified for
fdtbus_powerdomain_enable_index(),
as well as
(2) all indices are implicitly specified by
fdtbus_powerdomain_enable().
s/enable/disable/ functions were affected also.
Thanks,
rin
On 2024/06/12 15:39, Rin Okuyama wrote:
Module Name: src
Committed By: rin
Date: Wed Jun 12 06:39:28 UTC 2024
Modified Files:
src/sys/dev/fdt: fdt_powerdomain.c
Log Message:
fdt_powerdomain: Fix bug by which pd index >= 1 couldn't be enabled
Length in bytes was mistakenly used as number of uint32_t variables.
To generate a diff of this commit:
cvs rdiff -u -r1.1 -r1.2 src/sys/dev/fdt/fdt_powerdomain.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/dev/fdt/fdt_powerdomain.c
diff -u src/sys/dev/fdt/fdt_powerdomain.c:1.1
src/sys/dev/fdt/fdt_powerdomain.c:1.2
--- src/sys/dev/fdt/fdt_powerdomain.c:1.1 Fri Mar 4 08:19:06 2022
+++ src/sys/dev/fdt/fdt_powerdomain.c Wed Jun 12 06:39:28 2024
@@ -1,4 +1,4 @@
-/* $NetBSD: fdt_powerdomain.c,v 1.1 2022/03/04 08:19:06 skrll Exp $ */
+/* $NetBSD: fdt_powerdomain.c,v 1.2 2024/06/12 06:39:28 rin Exp $ */
/*-
* Copyright (c) 2022 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
*/
#include
-__KERNEL_RCSID(0, "$NetBSD: fdt_powerdomain.c,v 1.1 2022/03/04
08:19:06 skrll Exp $");
+__KERNEL_RCSID(0, "$NetBSD: fdt_powerdomain.c,v 1.2 2024/06/12
06:39:28 rin Exp $");
#include
@@ -103,7 +103,7 @@ fdtbus_powerdomain_enable_internal(int p
if (pds == NULL)
return EINVAL;
- for (const uint32_t *pd = pds; pd < pds + len; index--) {
+ for (const uint32_t *pd = pds; pd < pds + len / sizeof(*pd);
index--) {
uint32_t pd_node =
fdtbus_get_phandle_from_native(be32toh(pd[0]));
struct fdtbus_powerdomain_controller *pdc =
Re: CVS commit: src/sys/dev/fdt
Thanks for the fix.
The bug affects enable and disable in the all (-1) indexes case and
references out of bounds data.
Nick
On 12/06/2024 08:36, Rin Okuyama wrote:
Hmm, there was a confusion for my side.
This bug affected cases where
(1) index >= 1 is explicitly specified for
fdtbus_powerdomain_enable_index(),
as well as
(2) all indices are implicitly specified by
fdtbus_powerdomain_enable().
s/enable/disable/ functions were affected also.
Thanks,
rin
On 2024/06/12 15:39, Rin Okuyama wrote:
Module Name: src
Committed By: rin
Date: Wed Jun 12 06:39:28 UTC 2024
Modified Files:
src/sys/dev/fdt: fdt_powerdomain.c
Log Message:
fdt_powerdomain: Fix bug by which pd index >= 1 couldn't be enabled
Length in bytes was mistakenly used as number of uint32_t variables.
To generate a diff of this commit:
cvs rdiff -u -r1.1 -r1.2 src/sys/dev/fdt/fdt_powerdomain.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/dev/fdt/fdt_powerdomain.c
diff -u src/sys/dev/fdt/fdt_powerdomain.c:1.1
src/sys/dev/fdt/fdt_powerdomain.c:1.2
--- src/sys/dev/fdt/fdt_powerdomain.c:1.1 Fri Mar 4 08:19:06 2022
+++ src/sys/dev/fdt/fdt_powerdomain.c Wed Jun 12 06:39:28 2024
@@ -1,4 +1,4 @@
-/* $NetBSD: fdt_powerdomain.c,v 1.1 2022/03/04 08:19:06 skrll Exp $ */
+/* $NetBSD: fdt_powerdomain.c,v 1.2 2024/06/12 06:39:28 rin Exp $ */
/*-
* Copyright (c) 2022 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
*/
#include
-__KERNEL_RCSID(0, "$NetBSD: fdt_powerdomain.c,v 1.1 2022/03/04
08:19:06 skrll Exp $");
+__KERNEL_RCSID(0, "$NetBSD: fdt_powerdomain.c,v 1.2 2024/06/12
06:39:28 rin Exp $");
#include
@@ -103,7 +103,7 @@ fdtbus_powerdomain_enable_internal(int p
if (pds == NULL)
return EINVAL;
- for (const uint32_t *pd = pds; pd < pds + len; index--) {
+ for (const uint32_t *pd = pds; pd < pds + len / sizeof(*pd);
index--) {
uint32_t pd_node =
fdtbus_get_phandle_from_native(be32toh(pd[0]));
struct fdtbus_powerdomain_controller *pdc =
Re: CVS commit: src/sys/dev/fdt
Hmm, there was a confusion for my side.
This bug affected cases where
(1) index >= 1 is explicitly specified for
fdtbus_powerdomain_enable_index(),
as well as
(2) all indices are implicitly specified by
fdtbus_powerdomain_enable().
s/enable/disable/ functions were affected also.
Thanks,
rin
On 2024/06/12 15:39, Rin Okuyama wrote:
Module Name:src
Committed By: rin
Date: Wed Jun 12 06:39:28 UTC 2024
Modified Files:
src/sys/dev/fdt: fdt_powerdomain.c
Log Message:
fdt_powerdomain: Fix bug by which pd index >= 1 couldn't be enabled
Length in bytes was mistakenly used as number of uint32_t variables.
To generate a diff of this commit:
cvs rdiff -u -r1.1 -r1.2 src/sys/dev/fdt/fdt_powerdomain.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/dev/fdt/fdt_powerdomain.c
diff -u src/sys/dev/fdt/fdt_powerdomain.c:1.1
src/sys/dev/fdt/fdt_powerdomain.c:1.2
--- src/sys/dev/fdt/fdt_powerdomain.c:1.1 Fri Mar 4 08:19:06 2022
+++ src/sys/dev/fdt/fdt_powerdomain.c Wed Jun 12 06:39:28 2024
@@ -1,4 +1,4 @@
-/* $NetBSD: fdt_powerdomain.c,v 1.1 2022/03/04 08:19:06 skrll Exp $ */
+/* $NetBSD: fdt_powerdomain.c,v 1.2 2024/06/12 06:39:28 rin Exp $ */
/*-
* Copyright (c) 2022 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
*/
#include
-__KERNEL_RCSID(0, "$NetBSD: fdt_powerdomain.c,v 1.1 2022/03/04 08:19:06 skrll Exp
$");
+__KERNEL_RCSID(0, "$NetBSD: fdt_powerdomain.c,v 1.2 2024/06/12 06:39:28 rin Exp
$");
#include
@@ -103,7 +103,7 @@ fdtbus_powerdomain_enable_internal(int p
if (pds == NULL)
return EINVAL;
- for (const uint32_t *pd = pds; pd < pds + len; index--) {
+ for (const uint32_t *pd = pds; pd < pds + len / sizeof(*pd); index--) {
uint32_t pd_node =
fdtbus_get_phandle_from_native(be32toh(pd[0]));
struct fdtbus_powerdomain_controller *pdc =
Re: CVS commit: src/sys/dev/acpi
> Module Name:src > Committed By: christos > Date: Fri Apr 26 18:19:18 UTC 2024 > > Modified Files: > src/sys/dev/acpi: acpi_bat.c > > Log Message: > PR/58201: Malte Dehling: re-order sysmon initialization before acpi > registration, to avoid needing to call to acpi_deregister_notify on sysmon > failure. This isn't really a bug: the detach function calls acpi_deregister_notify. Now, with this change, it will call acpi_deregister_notify even if acpi_register_notify was never called. Fortunately, that's mostly harmless in the current implementation -- just as it was harmless to leave the notifier there; it doesn't use any memory that would be leaked. (Really, if there's any bug here, it's that sysmon_envsys_register can fail at all. This creates vast swaths of never-tested error branches that waste maintainer and auditor time.)
re: CVS commit: src/sys/dev/usb
"Nick Hudson" writes: > Module Name: src > Committed By: skrll > Date: Tue Dec 19 07:05:36 UTC 2023 > > Modified Files: > src/sys/dev/usb: if_axen.c > > Log Message: > Add support for AX88179A. From sc.dying on current-users. > > > To generate a diff of this commit: > cvs rdiff -u -r1.94 -r1.95 src/sys/dev/usb/if_axen.c cool. this can probably go back to not having a device softc by using un_flags: /* * This section is for driver to use, not touched by usbnet. */ unsignedun_flags; in struct usbnet, and axen_softc becomes just usbnet again. .mrg.
Re: CVS commit: src/sys/dev/pci/ixgbe
On 2023/10/12 14:50, SAITOH Masanobu wrote: > Module Name: src > Committed By: msaitoh > Date: Thu Oct 12 05:50:56 UTC 2023 > > Modified Files: > src/sys/dev/pci/ixgbe: ixgbe.c > > Log Message: > ixg(4): Don't print wrong error message about ixgbe_num_queues. > > Don't override the ixgbe_num_queues global variable. It's the default > value of the number of queues and should not override it because it > will be referenced by later device attach. For example, the number of > MSI-X vector is 64 on X540 and 18 on 82599. When both cards are inserted > to a machine that the number of CPU is 24 and X540 is probed earlier, > ixgbe_num_queues is overridden to 24 and the following error message is > printed when attaching 82599: > > ixg2: autoconfiguration error: ixgbe_num_queues (24) is too large, > using reduced amount (17). > > Note that the number of queues is in sc->num_queuss and referenced > by hw.ixgN.num_queues sysctl. The commit message was incorrect. - s/82599/82598/ - Worse thing can happen if a smaller number of MSI-X vector's device is attached earlier. The small number is set as the default value and the number of queues of the next device is unintentionally limited to it. > To generate a diff of this commit: > cvs rdiff -u -r1.341 -r1.342 src/sys/dev/pci/ixgbe/ixgbe.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. > -- --- SAITOH Masanobu (msai...@execsw.org msai...@netbsd.org)
Re: CVS commit: src/sys/dev/pci/igc
On 2023-10-15 17.06, Joerg Sonnenberger wrote: On Sun, Oct 15, 2023 at 10:36:53PM +, Greg Oster wrote: Module Name:src Committed By: oster Date: Sun Oct 15 22:36:53 UTC 2023 Modified Files: src/sys/dev/pci/igc: if_igc.c Log Message: Fix build of the MODULAR kernel, which explicitly excludes vlans. Please fix the macro to not expand to nothing instead. I think you're referring to the change I made to src/sys/dev/sequencer.c instead of this one... Done! Later... Greg Oster
Re: CVS commit: src/sys/dev/pci/igc
On Sun, Oct 15, 2023 at 10:36:53PM +, Greg Oster wrote: > Module Name: src > Committed By: oster > Date: Sun Oct 15 22:36:53 UTC 2023 > > Modified Files: > src/sys/dev/pci/igc: if_igc.c > > Log Message: > Fix build of the MODULAR kernel, which explicitly excludes vlans. Please fix the macro to not expand to nothing instead. Joerg
Re: CVS commit: src/sys/dev/pci
> Date: Thu, 10 Aug 2023 17:42:35 +0900 > From: Kengo NAKAHARA > > Could you tell me how you test this fix for future reference? I asked skrll@ to boot a VM with vmxnet3 and hammer on it with a combination of: 1. dhcp 2. host$ nc -l 54321 /dev/null guest$ nc host 54321 /dev/null 3. for i in `jot 4`; do jot 100 | while read i; do ifconfig vmxnet0 down ifconfig vmxnet0 up done & done wait and then see if the network continued functioning. However, I didn't do anything to verify that we could trigger all the problems that I noticed by code inspection. I didn't do that mostly because I'd been sitting on this patch for a year, and I didn't want it to languish indefinitely with obvious bugs lurking in vmxnet(4). Here are a couple more things that it would be good to do: 1. Create a sysctl knob to simulate watchdog failure and trigger reset. I don't think we tested this path at all, but it's a common bug with an easy fix -- defer to workqueue. Some other drivers have a sysctl knob like this: wm(4), bge(4). 2. Verify that SIOCADDMULTI and SIOCDELMULTI don't deadlock when run concurrently with ifconfig up/down. vmxnet(4) still has a serious bug: the `core lock' can still lead to a deadlock with reset and softint. It should be removed from most uses, and be limited to cover only mii transactions like I did for usbnet(4) last year.
Re: CVS commit: src/sys/dev/pci
Hi, On 2023/08/10 18:07, Nick Hudson wrote: On 10/08/2023 09:42, Kengo NAKAHARA wrote: Hi, Could you tell me how you test this fix for future reference? He didn't - I did. :) Taylor suggested running with network traffic and doing ifconfig down/up. To generate network traffic Taylor suggested host$ nc -l 54321 /dev/null guest$ nc host 54321 /dev/null and I did for i in `jot 64`; do ifconfig vmx0 down sleep 1 ifconfig vmx0 up done Nick I see. I want to know the traffic generator and ioctl jobs, that is exactly what is. Thank you for your comment. Thanks, -- // Internet Initiative Japan Inc. Device Engineering Section, Core Product Development Department, Network Division, Technology Unit Kengo NAKAHARA
Re: CVS commit: src/sys/dev/pci
On 10/08/2023 09:42, Kengo NAKAHARA wrote: Hi, Could you tell me how you test this fix for future reference? He didn't - I did. :) Taylor suggested running with network traffic and doing ifconfig down/up. To generate network traffic Taylor suggested host$ nc -l 54321 /dev/null guest$ nc host 54321 /dev/null and I did for i in `jot 64`; do ifconfig vmx0 down sleep 1 ifconfig vmx0 up done Nick
Re: CVS commit: src/sys/dev/pci
Hi, Could you tell me how you test this fix for future reference? Thanks, On 2023/08/10 17:24, Taylor R Campbell wrote: Module Name:src Committed By: riastradh Date: Thu Aug 10 08:24:45 UTC 2023 Modified Files: src/sys/dev/pci: if_vmx.c Log Message: vmxnet(4): Fix various MP bugs. - Defer reset to workqueue. => vmxnet3_stop_locked is forbidden in softint. => XXX Problem: We still take the core lock in softint, and we still take the core lock around vmxnet3_stop_locked. TBD. - Touch if_flags only under IFNET_LOCK. => Cache ifp->if_flags & IFF_PROMISC in vmxnet3_ifflags_cb. => Don't call vmxnet3_set_rxfilter unless up and running; cache this as vmx_mcastactive. Use ENETRESET in vmxnet3_ifflags_cb instead of calling vmxnet3_set_rxfilter directly. . (The cache is currently serialized by the core lock, but it might reasonably be serialized by an independent lock like in usbnet(9).) - Fix vmxnet3_stop_rendezvous so it actually does something. => New vxtxq_stopping, vxrxq_stopping variables synchronize with Rx/Tx interrupt handlers. - Sprinkle IFNET_LOCK and core lock assertions. To generate a diff of this commit: cvs rdiff -u -r1.11 -r1.12 src/sys/dev/pci/if_vmx.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. -- // Internet Initiative Japan Inc. Device Engineering Section, Core Product Development Department, Network Division, Technology Unit Kengo NAKAHARA
Re: CVS commit: src/sys/dev/dkwedge
> Date: Tue, 11 Apr 2023 21:50:49 +0200
> From: Michael van Elst
>
> On Wed, Apr 12, 2023 at 01:10:40AM +0700, Robert Elz wrote:
> >
> > | In that state then decrementing dk_rawopens beyond zero will make
> > | dklastclose do the right thing: nothing.
> >
> > Except that if that happens, dk_rawopens will be left == ~0 and the next
> > open attempt will then increment it, back to 0 again, which is almost
> > certainly not what was wanted.
> >
> > dklastclose() used to have code in it like
> >
> > if (...->dk_rawopens > 0) {
> > if (--...->dk_rawopens == 0)
>
>
> Indeed, that part was simplified away.
Correct. I first added the assertion dk_rawopens > 0 in the following
change change because, as I wrote in the commit message:
It is not possible for us to be closing a wedge whose parent is not
open by at least this wedge.
https://mail-index.netbsd.org/source-changes-hg/2022/08/22/msg359438.html
See
https://mail-index.netbsd.org/source-changes-d/2023/04/11/msg013937.html
for more details of why I believe the condition is always true here.
Then, on the grounds that the condition is always true (and asserted
to be so), I removed the conditional in a separate change:
https://mail-index.netbsd.org/source-changes-hg/2022/08/22/msg359439.html
So if you actually hit the assertion, please share diagnostics, or if
you believe there is a way that you could hit the assertion, please
explain how and we can figure out how to address it.
But if not, please put the assertions back, or I will next week.
Re: CVS commit: src/sys/dev/usb
> Module Name:src > Committed By: mlelstv > Date: Mon Apr 10 15:26:57 UTC 2023 > > Modified Files: > src/sys/dev/usb: uvideo.c uvideoreg.h > > Log Message: > Better descriptor parsing. How is it better? What problems does it fix? > Add sanity check if no default format is found. > > @@ -442,12 +442,8 @@ static void print_vs_format_dv_descripto > const uvideo_vs_format_dv_descriptor_t *); > #endif /* !UVIDEO_DEBUG */ > > -#define GET(type, descp, field) > \ > - (KASSERT((descp)->bLength >= sizeof(type)), > \ > - ((const type *)(descp))->field) > -#define GETP(type, descp, field) > \ > - (KASSERT((descp)->bLength >= sizeof(type)), > \ > - &(((const type *)(descp))->field)) > +#define GET(type, descp, field) (((const type *)(descp))->field) > +#define GETP(type, descp, field) (&(((const type *)(descp))->field)) > [...] > @@ -1398,8 +1398,6 @@ uvideo_stream_init_frame_based_format(st > return USBD_INVAL; > } > > - KASSERT(subtypelen >= sizeof(*uvdesc)); Please restore these assertions, and adjust them if needed to make them work. > + uvideo_frame_interval_t uFrameInterval; > } UPACKED uvideo_vs_frame_uncompressed_descriptor_t; > -CTASSERT(sizeof(uvideo_vs_frame_uncompressed_descriptor_t) == 26); Please restore compile-time assertions of these structure sizes so that it is easy to verify they match the spec. Please also add a reference to the section/page/table number in the spec in a comment so it's easy to look up.
Re: CVS commit: src/sys/dev/dkwedge
On Wed, Apr 12, 2023 at 01:10:40AM +0700, Robert Elz wrote:
>
> | In that state then decrementing dk_rawopens beyond zero will make
> | dklastclose do the right thing: nothing.
>
> Except that if that happens, dk_rawopens will be left == ~0 and the next
> open attempt will then increment it, back to 0 again, which is almost
> certainly not what was wanted.
>
> dklastclose() used to have code in it like
>
> if (...->dk_rawopens > 0) {
> if (--...->dk_rawopens == 0)
Indeed, that part was simplified away.
--
Michael van Elst
Internet: mlel...@serpens.de
"A potential Snark may lurk in every tree."
Re: CVS commit: src/sys/dev/dkwedge
Date:Tue, 11 Apr 2023 17:21:17 +0200
From:Michael van Elst
Message-ID:
| In that state then decrementing dk_rawopens beyond zero will make
| dklastclose do the right thing: nothing.
Except that if that happens, dk_rawopens will be left == ~0 and the next
open attempt will then increment it, back to 0 again, which is almost
certainly not what was wanted.
dklastclose() used to have code in it like
if (...->dk_rawopens > 0) {
if (--...->dk_rawopens == 0)
so that the -- would never be performed if rawopens was 0 when entered.
| When you want to check for overflows of dk_rawopens (which is difficult
| to overflow as you had to create 2^32 wedges)
It wasn't the overflow that Taylor meant, but this underflow (from 0 -> ~0)
which might be a problem.
(Not really relevant, but it wouldn't be 2^32 wedges, but 2^32 simultaneous
opens of any single wedge, right ... but that's not the real issue, that one
probably can't happen on any normal system, the file table could never get
big enough to allow 2^32 simultaneous opens of everything, let alone one
device).
Either dklastclose() can be called when dk_rawopens == 0 (in which case
the current code is broken) or it cannot, in which case the assertion would
have just verified that.
kre
Re: CVS commit: src/sys/dev/dkwedge
On Tue, Apr 11, 2023 at 09:07:49AM +, Taylor R Campbell wrote: > > (a) how we can legitimately enter a state where the assertions are > violated, and dklastclose is called when the close operation ends in no more openers. There is nothing that guarantees that any open was successful before with the effect that dk_rawopens is > 0 and dk_rawvp is not NULL. In that state then decrementing dk_rawopens beyond zero will make dklastclose do the right thing: nothing. When you want to check for overflows of dk_rawopens (which is difficult to overflow as you had to create 2^32 wedges) you need to watch it being incremented (also temporarily). Crashing after the fact with an assertion in dklastclose doesn't help. -- Michael van Elst Internet: mlel...@serpens.de "A potential Snark may lurk in every tree."
Re: CVS commit: src/sys/dev/dkwedge
> Module Name:src
> Committed By: mlelstv
> Date: Tue Sep 27 17:04:52 UTC 2022
>
> Modified Files:
> src/sys/dev/dkwedge: dk.c
>
> Log Message:
> Remove bogus assertions.
>
> @@ -1221,8 +1221,6 @@ dklastclose(struct dkwedge_softc *sc)
>
> KASSERT(mutex_owned(&sc->sc_dk.dk_openlock));
> KASSERT(mutex_owned(&sc->sc_parent->dk_rawlock));
> - KASSERT(sc->sc_parent->dk_rawopens > 0);
> - KASSERT(sc->sc_parent->dk_rawvp != NULL);
>
> if (--sc->sc_parent->dk_rawopens == 0) {
> struct vnode *const vp = sc->sc_parent->dk_rawvp;
If these assertions are bogus, please add a comment explaining:
(a) how we can legitimately enter a state where the assertions are
violated, and
(b) how this logic is supposed to work when dk_rawopens wraps around
from 0 to UINT_MAX and we try to dk_close_parent(NULL, ...).
Otherwise, please restore these assertions.
Re: CVS commit: src/sys/dev/wscons
Date:Tue, 24 Jan 2023 14:51:03 - (UTC) From:chris...@astron.com (Christos Zoulas) Message-ID: | Once we add them older kernels will break, I doubt that they'd break any kernels - curses using programs using these sequences with an old kernel might break though. But I don't think they need to be added in any case - the point of them existing is so that when using some remote system, with a terminfo/termcap database that doesn't include wsvt25 (which probably means anything that isn't BSD based) it is possible to set TERM=xterm (which is likely to exist everywhere) and get reasonable performance, rather than TERM=vt100 (also likely to exist, but with poor performance). There's no real need for the new ones to be known for local use, they weren't added because anyone was demanding them for any other reason. kre
Re: CVS commit: src/sys/dev/wscons
On Tue, 24 Jan 2023 at 14:51, Christos Zoulas wrote: > > In article , > Valery Ushakov wrote: > >On Wed, Jan 18, 2023 at 12:02:17 -0500, Christos Zoulas wrote: > > > >> Module Name: src > >> Committed By:christos > >> Date:Wed Jan 18 17:02:17 UTC 2023 > >> > >> Modified Files: > >> src/sys/dev/wscons: wsemul_vt100_subr.c > >> > >> Log Message: > >> Add rin, indn, vpa, hpa, and cbt terminfo capabilities (Crystal Kolipe) > > > >They probably need to be added to the terminfo description too. > > I guess :-) Once we add them older kernels will break, but it is unlikely > that older kernels will have new terminfo. There would need to be a new terminfo entry (whether the capabilities are added to wsvt25, and an older compat added, or a new wsvt25plus entry added), as otherwise a remote session from a newer system into an older will have issues due to missing capabilities. Maybe just update the manpage to note when the new capabilities were added and leave it for a release or so :) David
Re: CVS commit: src/sys/dev/wscons
In article , Valery Ushakov wrote: >On Wed, Jan 18, 2023 at 12:02:17 -0500, Christos Zoulas wrote: > >> Module Name: src >> Committed By:christos >> Date:Wed Jan 18 17:02:17 UTC 2023 >> >> Modified Files: >> src/sys/dev/wscons: wsemul_vt100_subr.c >> >> Log Message: >> Add rin, indn, vpa, hpa, and cbt terminfo capabilities (Crystal Kolipe) > >They probably need to be added to the terminfo description too. I guess :-) Once we add them older kernels will break, but it is unlikely that older kernels will have new terminfo. christos
Re: CVS commit: src/sys/dev/wscons
On Wed, Jan 18, 2023 at 12:02:17 -0500, Christos Zoulas wrote: > Module Name: src > Committed By: christos > Date: Wed Jan 18 17:02:17 UTC 2023 > > Modified Files: > src/sys/dev/wscons: wsemul_vt100_subr.c > > Log Message: > Add rin, indn, vpa, hpa, and cbt terminfo capabilities (Crystal Kolipe) They probably need to be added to the terminfo description too. -uwe
Re: CVS commit: src/sys/dev/i2c
Taylor R Campbell writes:
[snip]
> The issue here isn't the duration of the delay -- just the mechanism.
> Sleeping for 35ms with kpause(9) is fine, but busy-waiting on the CPU
> for 35ms with delay(9) is not (unless HZ is set to something absurdly
> low like 10 instead of the usual 100, but I doubt that's an important
> use-case to consider here).
>
> In this case, you should use:
>
> kpause("bmx280", /*intr*/false, MAX(1, mstohz(35)), NULL);
All understood.. the above, however, was not quite right either. That
delay is related to the over sampling that is set and wasn't a fixed
value. I made adjustments to scale the amount of time actually needed
to wait with the over sampling setting. This is the proper thing that
needed to be done in this case. I do agree that kpause is a better
thing to use here too.. no doubt..
There wasn't any real documentation about this, but it clearly was
something that needed to be considered. The only reference was a single
table that listed some of the typical and max durations for just *SOME*
of the over sampling setting (worse, yet, this table was removed from
later versions of the docs). It left out a lot and did not include any
sort of formula. Though some experimentation I can up with one that
should be workable and allowed for some tuning if it wasn't quite right.
>> was, as one might say, a "surprising development" as the documentation
>> really does not hint that this sort of thing goes on (or was even
>> possible to do).
>
> Can you put a link to the documentation in the source code, and cite
> the sections where you get the complicated formulas like in
> bmx280_compensate_P_int64?
Ya, I can do that. The three compensation formulas are pulled mostly
literally from the docs. All I really did was changed the types to
match the kernel names and unglobal'ed some of the variables for the
factory calibration settings. Peeking at Adafruit's Ardunio code, they
did the same thing.
--
Brad Spencer - b...@anduin.eldar.org - KC8VKS - http://anduin.eldar.org
Re: CVS commit: src/sys/dev/i2c
> Date: Wed, 23 Nov 2022 01:42:13 -0500
> From: Brad Spencer
>
> Simon Burge writes:
>
> > + delay(35000);
> >
> > This will spin for 35 milliseconds (per sensor read if I read this
> > correctly). Can you please look at using kpause(9) for this delay?
> > See some other i2c drivers for examples of this.
>
> Probably possible, may be a couple of days before I can get to
> it (or not, depends on how holiday prep goes)...
>
> I have used some of the cv_timedwait stuff in the past and didn't know
> about kpause which seems to be suited to what I need to do. Almost all
> sensor chips require some sort of wait after a command is sent, but this
> one was particularly frustrating about it.
The issue here isn't the duration of the delay -- just the mechanism.
Sleeping for 35ms with kpause(9) is fine, but busy-waiting on the CPU
for 35ms with delay(9) is not (unless HZ is set to something absurdly
low like 10 instead of the usual 100, but I doubt that's an important
use-case to consider here).
In this case, you should use:
kpause("bmx280", /*intr*/false, MAX(1, mstohz(35)), NULL);
> This
> was, as one might say, a "surprising development" as the documentation
> really does not hint that this sort of thing goes on (or was even
> possible to do).
Can you put a link to the documentation in the source code, and cite
the sections where you get the complicated formulas like in
bmx280_compensate_P_int64?
Re: CVS commit: src/sys/dev/i2c
Simon Burge writes: > Hi Brad, > >> Module Name: src >> Committed By:brad >> Date:Tue Nov 22 19:40:31 UTC 2022 >> >> Modified Files: >> >> src/sys/dev/i2c: bmx280.c >> >> Log Message: >> >> Read the datasheet more closely and put in some delays. The chip will >> just return junk if the wait is not long enough to allow a measurement >> to start. > > > + /* Hmm... this delay is not documented well.. mostly just a guess... > +* If it is too short, you will get junk returned as it is possible > to try > +* to ask for the data before the chip has even started... it seems... > +*/ > + delay(35000); > > This will spin for 35 milliseconds (per sensor read if I read this > correctly). Can you please look at using kpause(9) for this delay? > See some other i2c drivers for examples of this. > > Cheers, > Simon. Probably possible, may be a couple of days before I can get to it (or not, depends on how holiday prep goes)... I have used some of the cv_timedwait stuff in the past and didn't know about kpause which seems to be suited to what I need to do. Almost all sensor chips require some sort of wait after a command is sent, but this one was particularly frustrating about it. To give out too much information, all of the other sensors I have worked with NACK the I2C bus during the measurement cycle. This one, and probably all of Bosch's chips, do not do that. You have to read back a status register and check to see if it is busy doing something or other. If you just go ahead and read the data registers when it is busy you get junk with no hint that it is junk. But a big "however" is that it is possible (even for a RPI) to read the status register BEFORE the chip has even started to do its measurements, get a positive response (i.e. not busy) and then read the data registers and get junk. This was, as one might say, a "surprising development" as the documentation really does not hint that this sort of thing goes on (or was even possible to do). -- Brad Spencer - b...@anduin.eldar.org - KC8VKS - http://anduin.eldar.org
Re: CVS commit: src/sys/dev/i2c
Hi Brad, > Module Name: src > Committed By: brad > Date: Tue Nov 22 19:40:31 UTC 2022 > > Modified Files: > > src/sys/dev/i2c: bmx280.c > > Log Message: > > Read the datasheet more closely and put in some delays. The chip will > just return junk if the wait is not long enough to allow a measurement > to start. + /* Hmm... this delay is not documented well.. mostly just a guess... +* If it is too short, you will get junk returned as it is possible to try +* to ask for the data before the chip has even started... it seems... +*/ + delay(35000); This will spin for 35 milliseconds (per sensor read if I read this correctly). Can you please look at using kpause(9) for this delay? See some other i2c drivers for examples of this. Cheers, Simon.
Re: CVS commit: src/sys/dev/tprof
>I think this is a bug in your device tree because the KASSERT was >intentional: > > - In the ACPI case, we probe for CPU PMU support before calling >armv8_pmu_init. > - In the FDT case, the PMU attaches to a node described in the device >tree. > >So if you hit this KASSERT, AFAICT it means your device tree is describing >a device that is not there. Unless I'm missing something here. I tried to fix it to work properly as a kernel module for debugging and improving tprof itself, but the current implement is difficult due to the acpi/fdt pmu interrupt and tprof, so I gave up :-P I'll revert it. thanks! -- ryo shimizu
Re: CVS commit: src/sys/dev/tprof
I think this is a bug in your device tree because the KASSERT was intentional: - In the ACPI case, we probe for CPU PMU support before calling armv8_pmu_init. - In the FDT case, the PMU attaches to a node described in the device tree. So if you hit this KASSERT, AFAICT it means your device tree is describing a device that is not there. Unless I'm missing something here. Take care, Jared On Wed, 9 Nov 2022, Ryo Shimizu wrote: Module Name:src Committed By: ryo Date: Wed Nov 9 19:06:46 UTC 2022 Modified Files: src/sys/dev/tprof: tprof_armv8.c Log Message: If the hardware does not support PMU, return an error instead of KASSERT. To generate a diff of this commit: cvs rdiff -u -r1.14 -r1.15 src/sys/dev/tprof/tprof_armv8.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Re: CVS commit: src/sys/dev
On 2022/10/04 19:22, Taylor R Campbell wrote: >> Date: Tue, 4 Oct 2022 17:16:35 +0900 >> From: Masanobu SAITOH >> >> Before reverting changes, one of my machine which use serial console >> didn't print the Copyright message. >> >> Revert two revert commit, i.e. >> http://mail-index.netbsd.org/source-changes/2022/10/04/msg141389.html >> http://mail-index.netbsd.org/source-changes/2022/10/04/msg141388.html >> and applied consokfix.patch. >> [...] >> The boot message is not printed and I can see messages after /sbin/init. >> dmesg(1) shows the kernel messages. > > Can you please try with consokfix.patch _and_ consprintfix.patch? > > consprintfix.patch should restore the kernel messages on the console. It worked! -- --- SAITOH Masanobu (msai...@execsw.org msai...@netbsd.org)
Re: CVS commit: src/sys/dev
> Date: Tue, 4 Oct 2022 17:16:35 +0900 > From: Masanobu SAITOH > > Before reverting changes, one of my machine which use serial console > didn't print the Copyright message. > > Revert two revert commit, i.e. > http://mail-index.netbsd.org/source-changes/2022/10/04/msg141389.html > http://mail-index.netbsd.org/source-changes/2022/10/04/msg141388.html > and applied consokfix.patch. > [...] > The boot message is not printed and I can see messages after /sbin/init. > dmesg(1) shows the kernel messages. Can you please try with consokfix.patch _and_ consprintfix.patch? consprintfix.patch should restore the kernel messages on the console. >From 0f058a0e89e3f545a9020a2fb79dadd7ad89029a Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Tue, 4 Oct 2022 05:48:39 + Subject: [PATCH] squash! constty(4): Make MP-safe. - Fix reversed sense of conditional. --- sys/kern/subr_prf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sys/kern/subr_prf.c b/sys/kern/subr_prf.c index e87e6efc8501..53fb20c1d393 100644 --- a/sys/kern/subr_prf.c +++ b/sys/kern/subr_prf.c @@ -425,7 +425,7 @@ putone(int c, int flags, struct tty *tp) if ((flags & TOLOG) && c != '\0' && c != '\r' && c != 0177) logputchar(c); - if ((flags & TOCONS) && ctp != NULL && c != '\0') + if ((flags & TOCONS) && ctp == NULL && c != '\0') (*v_putc)(c); pserialize_read_exit(s);
Re: CVS commit: src/sys/dev
Hi. On 2022/10/04 16:24, Ryo ONODERA wrote: > Hi, > > Taylor R Campbell writes: > >>> Date: Tue, 04 Oct 2022 15:53:58 +0900 >>> From: Ryo ONODERA >>> >>> With this patch, it works fine for me. >>> There is no stall after genfb(4). >>> And I do not find any other problem so far. >> >> Thanks! There probably is another problem which is that kernel >> console printfs might stop appearing after a certain point, which the >> following patch might fix too -- I inadvertently reversed the sense >> of a conditional in the subr_prf.c changes. > > I have not encountered another problem yet. However with your > consprintfix.patch, it works fine for me too. Before reverting changes, one of my machine which use serial console didn't print the Copyright message. Revert two revert commit, i.e. http://mail-index.netbsd.org/source-changes/2022/10/04/msg141389.html http://mail-index.netbsd.org/source-changes/2022/10/04/msg141388.html and applied consokfix.patch. > NetBSD MBR boot > > NetBSD/x86 ffsv2 Primary Bootstrap > > 0 seconds. > booting hd0a:netbsd (howto 0xa) > 72567816+35522344+2226392 [945568+1555416+1162803]=0x6d7f2a8 > Loading /var/db/entropy-file > Tue Oct 4 17:07:44 JST 2022 > Starting root file system check: > /dev/rwd0a: file system is clean; not checking > Setting sysctl variables: ... The boot message is not printed and I can see messages after /sbin/init. dmesg(1) shows the kernel messages. Regards. > Thank you. > >> From 0f058a0e89e3f545a9020a2fb79dadd7ad89029a Mon Sep 17 00:00:00 2001 >> From: Taylor R Campbell >> Date: Tue, 4 Oct 2022 05:48:39 + >> Subject: [PATCH] squash! constty(4): Make MP-safe. >> >> - Fix reversed sense of conditional. >> --- >> sys/kern/subr_prf.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/sys/kern/subr_prf.c b/sys/kern/subr_prf.c >> index e87e6efc8501..53fb20c1d393 100644 >> --- a/sys/kern/subr_prf.c >> +++ b/sys/kern/subr_prf.c >> @@ -425,7 +425,7 @@ putone(int c, int flags, struct tty *tp) >> if ((flags & TOLOG) && >> c != '\0' && c != '\r' && c != 0177) >> logputchar(c); >> -if ((flags & TOCONS) && ctp != NULL && c != '\0') >> +if ((flags & TOCONS) && ctp == NULL && c != '\0') >> (*v_putc)(c); >> >> pserialize_read_exit(s); > -- --- SAITOH Masanobu (msai...@execsw.org msai...@netbsd.org)
Re: CVS commit: src/sys/dev
Hi, Taylor R Campbell writes: >> Date: Tue, 04 Oct 2022 15:53:58 +0900 >> From: Ryo ONODERA >> >> With this patch, it works fine for me. >> There is no stall after genfb(4). >> And I do not find any other problem so far. > > Thanks! There probably is another problem which is that kernel > console printfs might stop appearing after a certain point, which the > following patch might fix too -- I inadvertently reversed the sense > of a conditional in the subr_prf.c changes. I have not encountered another problem yet. However with your consprintfix.patch, it works fine for me too. Thank you. > From 0f058a0e89e3f545a9020a2fb79dadd7ad89029a Mon Sep 17 00:00:00 2001 > From: Taylor R Campbell > Date: Tue, 4 Oct 2022 05:48:39 + > Subject: [PATCH] squash! constty(4): Make MP-safe. > > - Fix reversed sense of conditional. > --- > sys/kern/subr_prf.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sys/kern/subr_prf.c b/sys/kern/subr_prf.c > index e87e6efc8501..53fb20c1d393 100644 > --- a/sys/kern/subr_prf.c > +++ b/sys/kern/subr_prf.c > @@ -425,7 +425,7 @@ putone(int c, int flags, struct tty *tp) > if ((flags & TOLOG) && > c != '\0' && c != '\r' && c != 0177) > logputchar(c); > - if ((flags & TOCONS) && ctp != NULL && c != '\0') > + if ((flags & TOCONS) && ctp == NULL && c != '\0') > (*v_putc)(c); > > pserialize_read_exit(s); -- Ryo ONODERA // r...@tetera.org PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB FD1B F404 27FA C7D1 15F3
Re: CVS commit: src/sys/dev
> Date: Tue, 04 Oct 2022 15:53:58 +0900 > From: Ryo ONODERA > > With this patch, it works fine for me. > There is no stall after genfb(4). > And I do not find any other problem so far. Thanks! There probably is another problem which is that kernel console printfs might stop appearing after a certain point, which the following patch might fix too -- I inadvertently reversed the sense of a conditional in the subr_prf.c changes. >From 0f058a0e89e3f545a9020a2fb79dadd7ad89029a Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Tue, 4 Oct 2022 05:48:39 + Subject: [PATCH] squash! constty(4): Make MP-safe. - Fix reversed sense of conditional. --- sys/kern/subr_prf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sys/kern/subr_prf.c b/sys/kern/subr_prf.c index e87e6efc8501..53fb20c1d393 100644 --- a/sys/kern/subr_prf.c +++ b/sys/kern/subr_prf.c @@ -425,7 +425,7 @@ putone(int c, int flags, struct tty *tp) if ((flags & TOLOG) && c != '\0' && c != '\r' && c != 0177) logputchar(c); - if ((flags & TOCONS) && ctp != NULL && c != '\0') + if ((flags & TOCONS) && ctp == NULL && c != '\0') (*v_putc)(c); pserialize_read_exit(s);
Re: CVS commit: src/sys/dev
Hi,
Taylor R Campbell writes:
>> Date: Tue, 04 Oct 2022 12:12:15 +0900
>> From: Ryo ONODERA
>>
>> "Taylor R Campbell" writes:
>>
>> > console(4), constty(4): Rip off the kernel lock.
>>
>> After introduction of MP-safe console/constty, my kernel stopped
>> just after genfb(4) detection.
>> LOCKDEBUG, DIAGNOSTIC, DEBUG options does not provide any additional
>> information with me.
>> Could you take a look at my problem?
>
> Sorry about that -- I've reverted this change and the MP-safe cons(4)
> change for now, but let's try to figure out what's wrong with them so
> I can reapply them and get the console paths out of the kernel lock
> for good.
No problem. And thanks for your quick response.
> Can you try the attached patch on top?
With this patch, it works fine for me.
There is no stall after genfb(4).
And I do not find any other problem so far.
$ cd /usr/src
$ TZ=UTC cvs up -dP -D2022-10-04
$ patch -p1 < ~/consokfix.patch
$ ./build.sh ... kernel=...
Thank you very much!!!
> From 2de03f1efbe5b73d42dc2f59730c17b99c04b3b9 Mon Sep 17 00:00:00 2001
> From: Taylor R Campbell
> Date: Tue, 4 Oct 2022 05:24:49 +
> Subject: [PATCH] squash! constty(4): Make MP-safe.
>
> - Fix initialization of ok in cn_redirect.
> ---
> sys/dev/cons.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/sys/dev/cons.c b/sys/dev/cons.c
> index f4f9a1602221..e621292a6b4a 100644
> --- a/sys/dev/cons.c
> +++ b/sys/dev/cons.c
> @@ -463,7 +463,7 @@ cn_redirect(dev_t *devp, int is_read, int *error, struct
> tty **ctpp)
> dev_t dev = *devp;
> struct tty *ctp;
> int s;
> - bool ok;
> + bool ok = false;
>
> *error = ENXIO;
> *ctpp = NULL;
> @@ -472,18 +472,17 @@ cn_redirect(dev_t *devp, int is_read, int *error,
> struct tty **ctpp)
> (cn_tab == NULL || (cn_tab->cn_pri != CN_REMOTE))) {
> if (is_read) {
> *error = 0;
> - ok = false;
> goto out;
> }
> tty_acquire(ctp);
> *ctpp = ctp;
> dev = ctp->t_dev;
> } else if (cn_tab == NULL) {
> - ok = false;
> goto out;
> } else {
> dev = cn_tab->cn_dev;
> }
> + ok = true;
> *devp = dev;
> out: pserialize_read_exit(s);
> return ok;
--
Ryo ONODERA // r...@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB FD1B F404 27FA C7D1 15F3
Re: CVS commit: src/sys/dev
> Date: Tue, 04 Oct 2022 12:12:15 +0900
> From: Ryo ONODERA
>
> "Taylor R Campbell" writes:
>
> > console(4), constty(4): Rip off the kernel lock.
>
> After introduction of MP-safe console/constty, my kernel stopped
> just after genfb(4) detection.
> LOCKDEBUG, DIAGNOSTIC, DEBUG options does not provide any additional
> information with me.
> Could you take a look at my problem?
Sorry about that -- I've reverted this change and the MP-safe cons(4)
change for now, but let's try to figure out what's wrong with them so
I can reapply them and get the console paths out of the kernel lock
for good.
Can you try the attached patch on top?
>From 2de03f1efbe5b73d42dc2f59730c17b99c04b3b9 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell
Date: Tue, 4 Oct 2022 05:24:49 +
Subject: [PATCH] squash! constty(4): Make MP-safe.
- Fix initialization of ok in cn_redirect.
---
sys/dev/cons.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/sys/dev/cons.c b/sys/dev/cons.c
index f4f9a1602221..e621292a6b4a 100644
--- a/sys/dev/cons.c
+++ b/sys/dev/cons.c
@@ -463,7 +463,7 @@ cn_redirect(dev_t *devp, int is_read, int *error, struct
tty **ctpp)
dev_t dev = *devp;
struct tty *ctp;
int s;
- bool ok;
+ bool ok = false;
*error = ENXIO;
*ctpp = NULL;
@@ -472,18 +472,17 @@ cn_redirect(dev_t *devp, int is_read, int *error, struct
tty **ctpp)
(cn_tab == NULL || (cn_tab->cn_pri != CN_REMOTE))) {
if (is_read) {
*error = 0;
- ok = false;
goto out;
}
tty_acquire(ctp);
*ctpp = ctp;
dev = ctp->t_dev;
} else if (cn_tab == NULL) {
- ok = false;
goto out;
} else {
dev = cn_tab->cn_dev;
}
+ ok = true;
*devp = dev;
out: pserialize_read_exit(s);
return ok;
Re: CVS commit: src/sys/dev
Hi, "Taylor R Campbell" writes: > Module Name: src > Committed By: riastradh > Date: Mon Oct 3 19:57:25 UTC 2022 > > Modified Files: > src/sys/dev: cons.c > > Log Message: > console(4), constty(4): Rip off the kernel lock. After introduction of MP-safe console/constty, my kernel stopped just after genfb(4) detection. LOCKDEBUG, DIAGNOSTIC, DEBUG options does not provide any additional information with me. Could you take a look at my problem? My dmesg just before MP-safe console.constty is here: Copyright (c) 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020, 2021, 2022 The NetBSD Foundation, Inc. All rights reserved. Copyright (c) 1982, 1986, 1989, 1991, 1993 The Regents of the University of California. All rights reserved. NetBSD 9.99.100 (DTRACE8) #16: Tue Oct 4 09:36:04 JST 2022 ryoon@brownie:/usr/world/9.99/amd64/obj/sys/arch/amd64/compile/DTRACE8 total memory = 15680 MB avail memory = 15137 MB timecounter: Timecounters tick every 10.000 msec Kernelized RAIDframe activated pms* disabled timecounter: Timecounter "i8254" frequency 1193182 Hz quality 100 efi: systbl at pa c9f7e018 mainbus0 (root) ACPI: RSDP 0xCDFFE014 24 (v02 HPQOEM) ACPI: XSDT 0xCDFA4228 000174 (v01 HPQOEM SLIC-MPC 0002 HP 0113) ACPI: FACP 0xCDFE 00010C (v05 HPQOEM SLIC-MPC 0002 HP 0004) ACPI: DSDT 0xCDFD2000 009607 (v01 HPQOEM 8929 0002 HP 0004) ACPI: FACS 0xCDEB3000 40 ACPI: UEFI 0xCDF7E000 000236 (v01 HPQOEM 8929 0001 HP 0004) ACPI: SSDT 0xCDFFC000 00020D (v01 HPQOEM 8929 0001 HP 0004) ACPI: SSDT 0xCDFF4000 0072B0 (v02 HPQOEM 8929 0002 HP 0004) ACPI: IVRS 0xCDFF3000 0001A4 (v02 HPQOEM 8929 0001 HP 0004) ACPI: SSDT 0xCDFEF000 003A21 (v01 HPQOEM 8929 0001 HP 0004) ACPI: SSDT 0xCDFEE000 000472 (v02 HPQOEM 8929 1000 HP 0004) ACPI: TPM2 0xCDFED000 4C (v04 HPQOEM 8929 0002 HP 0004) ACPI: SSDT 0xCDFEC000 00017D (v01 HPQOEM 8929 1000 HP 0004) ACPI: SSDT 0xCDFE4000 007B3B (v01 HPQOEM 8929 1000 HP 0004) ACPI: MSDM 0xCDFE3000 55 (v03 HPQOEM SLIC-MPC 0001 HP 0004) ACPI: ASF! 0xCDFE2000 A5 (v32 HPQOEM 8929 0002 HP 0004) ACPI: BOOT 0xCDFE1000 28 (v01 HPQOEM 8929 0002 HP 0004) ACPI: HPET 0xCDFDF000 38 (v01 HPQOEM 8929 0002 HP 0004) ACPI: APIC 0xCDFDE000 000138 (v03 HPQOEM 8929 0002 HP 0004) ACPI: MCFG 0xCDFDD000 3C (v01 HPQOEM 8929 0002 HP 0004) ACPI: SSDT 0xCDFD1000 C1 (v01 HPQOEM 8929 0002 HP 0004) ACPI: SSDT 0xCDFD 80 (v01 HPQOEM 8929 0002 HP 0004) ACPI: VFCT 0xCDFC2000 00D884 (v01 HPQOEM 8929 0001 HP 0004) ACPI: SSDT 0xCDFFD000 F8 (v01 HPQOEM 8929 1000 HP 0004) ACPI: SSDT 0xCDFC 5C (v02 HPQOEM 8929 1000 HP 0004) ACPI: SSDT 0xCDFB9000 005354 (v02 HPQOEM 8929 0001 HP 0004) ACPI: CRAT 0xCDFB8000 000EE8 (v01 HPQOEM 8929 0001 HP 0004) ACPI: CDIT 0xCDFB7000 29 (v01 HPQOEM 8929 0001 HP 0004) ACPI: SSDT 0xCDFB6000 000139 (v01 HPQOEM 8929 0001 HP 0004) ACPI: SSDT 0xCDFB5000 00028D (v01 HPQOEM 8929 0001 HP 0004) ACPI: SSDT 0xCDFB4000 000372 (v01 HPQOEM 8929 0001 HP 0004) ACPI: SSDT 0xCDFB3000 00021F (v01 HPQOEM 8929 0001 HP 0004) ACPI: SSDT 0xCDFB2000 000D53 (v01 HPQOEM 8929 0001 HP 0004) ACPI: SSDT 0xCDFB 0010C5 (v01 HPQOEM 8929 0001 HP 0004) ACPI: SSDT 0xCDFAC000 00362F (v01 HPQOEM 8929 0001 HP 0004) ACPI: FPDT 0xCDFAB000 44 (v01 HPQOEM SLIC-MPC 0002 HP 0004) ACPI: WSMT 0xCDFA9000 28 (v01 HPQOEM 8929 0001 HP 0004) ACPI: SSDT 0xCDFA8000 42 (v01 HPQOEM 8929 0001 HP 0004) ACPI: SSDT 0xCDFA7000 00020A (v01 HPQOEM 8929 0001 HP 0004) ACPI: SSDT 0xCDFA6000 0005AD (v01 HPQOEM 8929 0001 HP 0004) ACPI: SSDT 0xCDFA5000 0002E9 (v01 HPQOEM 8929 0001 HP 0004) ACPI: SSDT 0xCDFC1000 7D (v01 HPQOEM 8929 0001 HP 0004) ACPI: SSDT 0xCDFA3000 C7 (v01 HPQOEM 8929 0001 HP 0004) ACPI: SSDT 0xCDFA2000 0004DB (v01 HPQOEM 8929 0001 HP 0004) ACPI: BGRT 0xCDFDC000 38 (v01 HPQOEM 8929 0001 HP 0004) ACPI: 26 ACPI AML tables successfully acquired and loaded ioapic0 at mainbus0 a
Re: CVS commit: src/sys/dev/pci
Hi,
On 21/09/2022 08:56, matthew green wrote:
this asserts for me. perhaps ryo@ didn't have LOCKDEBUG?
I had LOCKDEBUG on, but not DEBUG.
https://nxr.netbsd.org/xref/src/sys/sys/systm.h#760
I see that adding options DEBUG does indeed cause a panic with
ASSERT_SLEEPABLE()...
ah! that would explain it. nick asked me to test switching
sc_mutex to IPL_SOFTCLOCK mutex, and this works, though it
exposed another bug in reboot that i also needed a fix for
(ifnet locked), see patch below with both fixes.
there's another with the rev 1.32 and rev 1.33+patch driver
when doing "ifconfig aq0 down up", i get this shortly after
and packets no longer flow:
aq0: watchdog timeout -- resetting
aq0: aq_handle_reset_work: INTR_MASK/STATUS = 0001/
aq0: aq_handle_reset_work: TXring[0] HEAD/TAIL=26/51
aq0: aq_handle_reset_work: TXring[1] HEAD/TAIL=153/170
aq0: aq_handle_reset_work: TXring[2] HEAD/TAIL=0/0
aq0: aq_handle_reset_work: TXring[3] HEAD/TAIL=0/0
aq0: aq_handle_reset_work: TXring[4] HEAD/TAIL=0/0
aq0: aq_handle_reset_work: TXring[5] HEAD/TAIL=0/0
aq0: aq_handle_reset_work: TXring[6] HEAD/TAIL=0/0
aq0: aq_handle_reset_work: TXring[7] HEAD/TAIL=0/0
I think this diff is slightly better and might even fix the problem
you're seeing with "ifconfig aq0 down up"
Nick
Index: sys/dev/pci/if_aq.c
===
RCS file: /cvsroot/src/sys/dev/pci/if_aq.c,v
retrieving revision 1.33
diff -u -p -r1.33 if_aq.c
--- sys/dev/pci/if_aq.c 16 Sep 2022 03:55:53 - 1.33
+++ sys/dev/pci/if_aq.c 21 Sep 2022 08:15:26 -
@@ -1278,7 +1278,7 @@ aq_attach(device_t parent, device_t self
int error;
sc->sc_dev = self;
- mutex_init(&sc->sc_mutex, MUTEX_DEFAULT, IPL_NET);
+ mutex_init(&sc->sc_mutex, MUTEX_DEFAULT, IPL_SOFTCLOCK);
mutex_init(&sc->sc_mpi_mutex, MUTEX_DEFAULT, IPL_NET);
sc->sc_pc = pc = pa->pa_pc;
@@ -1588,7 +1588,9 @@ aq_detach(device_t self, int flags __unu
if (sc->sc_iosize != 0) {
if (ifp->if_softc != NULL) {
- aq_stop(ifp, 0);
+ IFNET_LOCK(ifp);
+ aq_stop(ifp, 1);
+ IFNET_UNLOCK(ifp);
}
for (i = 0; i < AQ_NINTR_MAX; i++) {
@@ -1616,8 +1618,6 @@ aq_detach(device_t self, int flags __unu
sc->sc_iosize = 0;
}
- callout_stop(&sc->sc_tick_ch);
-
#if NSYSMON_ENVSYS > 0
if (sc->sc_sme != NULL) {
/* all sensors associated with this will also be detached */
re: CVS commit: src/sys/dev/pci
> >this asserts for me. perhaps ryo@ didn't have LOCKDEBUG?
>
> I had LOCKDEBUG on, but not DEBUG.
> https://nxr.netbsd.org/xref/src/sys/sys/systm.h#760
> I see that adding options DEBUG does indeed cause a panic with
> ASSERT_SLEEPABLE()...
ah! that would explain it. nick asked me to test switching
sc_mutex to IPL_SOFTCLOCK mutex, and this works, though it
exposed another bug in reboot that i also needed a fix for
(ifnet locked), see patch below with both fixes.
there's another with the rev 1.32 and rev 1.33+patch driver
when doing "ifconfig aq0 down up", i get this shortly after
and packets no longer flow:
aq0: watchdog timeout -- resetting
aq0: aq_handle_reset_work: INTR_MASK/STATUS = 0001/
aq0: aq_handle_reset_work: TXring[0] HEAD/TAIL=26/51
aq0: aq_handle_reset_work: TXring[1] HEAD/TAIL=153/170
aq0: aq_handle_reset_work: TXring[2] HEAD/TAIL=0/0
aq0: aq_handle_reset_work: TXring[3] HEAD/TAIL=0/0
aq0: aq_handle_reset_work: TXring[4] HEAD/TAIL=0/0
aq0: aq_handle_reset_work: TXring[5] HEAD/TAIL=0/0
aq0: aq_handle_reset_work: TXring[6] HEAD/TAIL=0/0
aq0: aq_handle_reset_work: TXring[7] HEAD/TAIL=0/0
.mrg.
Index: if_aq.c
===
RCS file: /cvsroot/src/sys/dev/pci/if_aq.c,v
retrieving revision 1.33
diff -p -u -r1.33 if_aq.c
--- if_aq.c 16 Sep 2022 03:55:53 - 1.33
+++ if_aq.c 21 Sep 2022 07:52:59 -
@@ -1278,7 +1278,7 @@ aq_attach(device_t parent, device_t self
int error;
sc->sc_dev = self;
- mutex_init(&sc->sc_mutex, MUTEX_DEFAULT, IPL_NET);
+ mutex_init(&sc->sc_mutex, MUTEX_DEFAULT, IPL_SOFTCLOCK);
mutex_init(&sc->sc_mpi_mutex, MUTEX_DEFAULT, IPL_NET);
sc->sc_pc = pc = pa->pa_pc;
@@ -1588,7 +1588,9 @@ aq_detach(device_t self, int flags __unu
if (sc->sc_iosize != 0) {
if (ifp->if_softc != NULL) {
+ IFNET_LOCK(ifp);
aq_stop(ifp, 0);
+ IFNET_UNLOCK(ifp);
}
for (i = 0; i < AQ_NINTR_MAX; i++) {
Re: CVS commit: src/sys/dev/pci
>> Module Name: src >> Committed By:skrll >> Date:Fri Sep 16 03:55:53 UTC 2022 >> >> Modified Files: >> src/sys/dev/pci: if_aq.c >> >> Log Message: >> Some MP improvements >> >> - Remove use of IFF_OACTIVE >> >> - Remove use of if_timer and provide an MP safe multiqueue watchdog >> >> - Sprinkle some lock assertions. >> >> Tested by ryo@. Thanks. > >this asserts for me. perhaps ryo@ didn't have LOCKDEBUG? I had LOCKDEBUG on, but not DEBUG. https://nxr.netbsd.org/xref/src/sys/sys/systm.h#760 I see that adding options DEBUG does indeed cause a panic with ASSERT_SLEEPABLE()... -- ryo shimizu
re: CVS commit: src/sys/dev/pci
"Nick Hudson" writes: > Module Name: src > Committed By: skrll > Date: Fri Sep 16 03:55:53 UTC 2022 > > Modified Files: > src/sys/dev/pci: if_aq.c > > Log Message: > Some MP improvements > > - Remove use of IFF_OACTIVE > > - Remove use of if_timer and provide an MP safe multiqueue watchdog > > - Sprinkle some lock assertions. > > Tested by ryo@. Thanks. this asserts for me. perhaps ryo@ didn't have LOCKDEBUG? the problem is that aq_init() calls AQ_LOCK(sc) -- this is a spin mutex -- and then calls aq_init_locked(). however, aq_init_locked() calls ASSERT_SLEEPABLE() since it has a code path that calls callout_halt() (which wants to sleep.) ie, the function that expects to be called with a spin mutex held also calls ASSERT_SLEEPABLE(). even if i were to comment that call, the later call to callout_halt() is the real problem. the only way i saw to handle this without investing some other method to invoke the callout_halt() from another lwp was to change aq_stop_locked() to return a value that says that callout_halt is needed here. that needs to be passed upto aq_stop() as well as aq_init(), both of which call aq_stop_locked(). it needs a little re-arrange due to aq_init_locked() already returning a value for aq_init() to return directly (and aq_init() is where the mutex will be dropped, and it's safe to callout_halt().) for now i'm running with rev 1.32. thanks. .mrg.
Re: CVS commit: src/sys/dev/nvmm
Hi Taylor, Thanks for updating NVMM! Would it be a good idea to sync our code with DragonBSDs? AFAICS they kept the code compiling for NetBSD too. To have a common code base? With regards, Reinoud On Tue, Sep 13, 2022 at 08:10:04PM +, Taylor R Campbell wrote: > Module Name: src > Committed By: riastradh > Date: Tue Sep 13 20:10:04 UTC 2022 > > Modified Files: > src/sys/dev/nvmm: nvmm.c nvmm_internal.h > src/sys/dev/nvmm/x86: nvmm_x86_vmx.c > > Log Message: > nvmm(4): Add suspend/resume support. > > New MD nvmm_impl callbacks: > > - .suspend_interrupt forces all VMs on all physical CPUs to exit. > - .vcpu_suspend suspends an individual vCPU on a machine. > - .machine_suspend suspends an individual machine. > - .suspend suspends the whole system. > - .resume resumes the whole system. > - .machine_resume resumes an individual machine. > - .vcpu_resume resumes an indidivudal vCPU on a machine. > > Suspending nvmm: > > 1. causes new VM operations (ioctl and close) to block until resumed, > 2. uses .suspend_interrupt to interrupt any concurrent and force them >to return early, and then > 3. uses the various suspend callbacks to suspend all vCPUs, machines, >and the whole system -- all vCPUs before the machine they're on, >and all machines before the system. > > Resuming nvmm does the reverse of (3) -- resume system, resume each > machine and then the vCPUs on that machine -- and then unblocks > operations. > > Implemented only for x86-vmx for now: > > - suspend_interrupt triggers a TLB IPI to cause VM exits; > - vcpu_suspend issues VMCLEAR to force any in-CPU state to be written > to memory; > - machine_suspend does nothing; > - suspend does VMXOFF on all CPUs; > - resume does VMXON on all CPUs; > - machine_resume does nothing; and > - vcpu_resume just marks each vCPU as valid but inactive so > subsequent use will clear it and load it with vmptrld. > > x86-svm left as an exercise for the reader. > > > To generate a diff of this commit: > cvs rdiff -u -r1.46 -r1.47 src/sys/dev/nvmm/nvmm.c > cvs rdiff -u -r1.20 -r1.21 src/sys/dev/nvmm/nvmm_internal.h > cvs rdiff -u -r1.84 -r1.85 src/sys/dev/nvmm/x86/nvmm_x86_vmx.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files.
Re: CVS commit: src/sys/dev/pci
On 03/08/2022 07:26, Kengo NAKAHARA wrote: Hi, On 2022/08/03 14:23, Nick Hudson wrote: Module Name: src Committed By: skrll Date: Wed Aug 3 05:23:30 UTC 2022 Modified Files: src/sys/dev/pci: if_wm.c Log Message: Add some KASSERTs around the locking protocol. Discussed with msaitoh@, knakahara@ and riastradh@ To generate a diff of this commit: cvs rdiff -u -r1.749 -r1.750 src/sys/dev/pci/if_wm.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Will you add "KASSERT(IFNET_LOCKED(ifp))" to all ethernet device driver init routine? If not, why the code is added wm(4) only? Good questions. While I don't see the problem with documenting the locking protocol this way and flushing out bugs with the KASSERTs I don't proposed to (personally) add them to every driver at this time. My motivation here is is that I'm making bge(4) MP safe and using wm(4) as a reference. It's even mentioned as a reference for the if_percpuq framework. https://nxr.netbsd.org/xref/src/sys/net/if.c#814 Perhaps as a driver is made MP safe it can also have similar KASSERTs added? Nick
Re: CVS commit: src/sys/dev/pci
Hi, On 2022/08/03 14:23, Nick Hudson wrote: Module Name:src Committed By: skrll Date: Wed Aug 3 05:23:30 UTC 2022 Modified Files: src/sys/dev/pci: if_wm.c Log Message: Add some KASSERTs around the locking protocol. Discussed with msaitoh@, knakahara@ and riastradh@ To generate a diff of this commit: cvs rdiff -u -r1.749 -r1.750 src/sys/dev/pci/if_wm.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Will you add "KASSERT(IFNET_LOCKED(ifp))" to all ethernet device driver init routine? If not, why the code is added wm(4) only? Thanks, -- // Internet Initiative Japan Inc. Device Engineering Section, Product Division, Technology Unit Kengo NAKAHARA
Re: CVS commit: src/sys/dev/ic
Oops, never mind, I hadn't yet seen the follow-up revert. > On Aug 1, 2022, at 8:29 AM, Jason Thorpe wrote: > > > >> On Aug 1, 2022, at 12:34 AM, Michael van Elst wrote: >> >> Module Name: src >> Committed By: mlelstv >> Date: Mon Aug 1 07:34:28 UTC 2022 >> >> Modified Files: >> src/sys/dev/ic: ahcisata_core.c bcmgenet.c nslm7x.c nvmereg.h nvmevar.h >> rtl8169.c tulip.c tulipreg.h >> >> Log Message: >> Also fix shift values for SCT constants. > > ??? > > diff -u src/sys/dev/ic/tulip.c:1.205 src/sys/dev/ic/tulip.c:1.206 > --- src/sys/dev/ic/tulip.c:1.205Sat Jun 25 02:46:15 2022 > +++ src/sys/dev/ic/tulip.c Mon Aug 1 07:34:28 2022 > @@ -1,4 +1,4 @@ > -/* $NetBSD: tulip.c,v 1.205 2022/06/25 02:46:15 tsutsui Exp $ */ > +/* $NetBSD: tulip.c,v 1.206 2022/08/01 07:34:28 mlelstv Exp $ */ > > /*- > * Copyright (c) 1998, 1999, 2000, 2002 The NetBSD Foundation, Inc. > @@ -36,7 +36,7 @@ > */ > > #include > -__KERNEL_RCSID(0, "$NetBSD: tulip.c,v 1.205 2022/06/25 02:46:15 tsutsui Exp > $"); > +__KERNEL_RCSID(0, "$NetBSD: tulip.c,v 1.206 2022/08/01 07:34:28 mlelstv Exp > $"); > > > #include > @@ -4394,7 +4394,7 @@ > */ > >/* XXX This should be auto-sense. */ > - ifmedia_set(&mii->mii_media, IFM_ETHER | IFM_10_T); > + ifmedia_set(&mii->mii_media, IFM_ETHER | IFM_10_5); > >tlp_print_media(sc); > } > > >> >> >> To generate a diff of this commit: >> cvs rdiff -u -r1.105 -r1.106 src/sys/dev/ic/ahcisata_core.c >> cvs rdiff -u -r1.11 -r1.12 src/sys/dev/ic/bcmgenet.c >> cvs rdiff -u -r1.74 -r1.75 src/sys/dev/ic/nslm7x.c >> cvs rdiff -u -r1.17 -r1.18 src/sys/dev/ic/nvmereg.h >> cvs rdiff -u -r1.24 -r1.25 src/sys/dev/ic/nvmevar.h >> cvs rdiff -u -r1.172 -r1.173 src/sys/dev/ic/rtl8169.c >> cvs rdiff -u -r1.205 -r1.206 src/sys/dev/ic/tulip.c >> cvs rdiff -u -r1.41 -r1.42 src/sys/dev/ic/tulipreg.h >> >> Please note that diffs are not public domain; they are subject to the >> copyright notices on the relevant files. >> > > -- thorpej -- thorpej
Re: CVS commit: src/sys/dev/ic
> On Aug 1, 2022, at 12:34 AM, Michael van Elst wrote: > > Module Name: src > Committed By: mlelstv > Date: Mon Aug 1 07:34:28 UTC 2022 > > Modified Files: > src/sys/dev/ic: ahcisata_core.c bcmgenet.c nslm7x.c nvmereg.h nvmevar.h >rtl8169.c tulip.c tulipreg.h > > Log Message: > Also fix shift values for SCT constants. ??? diff -u src/sys/dev/ic/tulip.c:1.205 src/sys/dev/ic/tulip.c:1.206 --- src/sys/dev/ic/tulip.c:1.205Sat Jun 25 02:46:15 2022 +++ src/sys/dev/ic/tulip.c Mon Aug 1 07:34:28 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: tulip.c,v 1.205 2022/06/25 02:46:15 tsutsui Exp $ */ +/* $NetBSD: tulip.c,v 1.206 2022/08/01 07:34:28 mlelstv Exp $ */ /*- * Copyright (c) 1998, 1999, 2000, 2002 The NetBSD Foundation, Inc. @@ -36,7 +36,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: tulip.c,v 1.205 2022/06/25 02:46:15 tsutsui Exp $"); +__KERNEL_RCSID(0, "$NetBSD: tulip.c,v 1.206 2022/08/01 07:34:28 mlelstv Exp $"); #include @@ -4394,7 +4394,7 @@ */ /* XXX This should be auto-sense. */ - ifmedia_set(&mii->mii_media, IFM_ETHER | IFM_10_T); + ifmedia_set(&mii->mii_media, IFM_ETHER | IFM_10_5); tlp_print_media(sc); } > > > To generate a diff of this commit: > cvs rdiff -u -r1.105 -r1.106 src/sys/dev/ic/ahcisata_core.c > cvs rdiff -u -r1.11 -r1.12 src/sys/dev/ic/bcmgenet.c > cvs rdiff -u -r1.74 -r1.75 src/sys/dev/ic/nslm7x.c > cvs rdiff -u -r1.17 -r1.18 src/sys/dev/ic/nvmereg.h > cvs rdiff -u -r1.24 -r1.25 src/sys/dev/ic/nvmevar.h > cvs rdiff -u -r1.172 -r1.173 src/sys/dev/ic/rtl8169.c > cvs rdiff -u -r1.205 -r1.206 src/sys/dev/ic/tulip.c > cvs rdiff -u -r1.41 -r1.42 src/sys/dev/ic/tulipreg.h > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. > -- thorpej
Re: CVS commit: src/sys/dev/pci
Hi. On 2022/07/22 23:22, Hisashi T Fujinaka wrote: > On Fri, 22 Jul 2022, SAITOH Masanobu wrote: > >> Module Name: src >> Committed By: msaitoh >> Date: Fri Jul 22 05:23:50 UTC 2022 >> >> Modified Files: >> src/sys/dev/pci: if_wm.c if_wmreg.h >> >> Log Message: >> Add more statistics countes. >> >> - Add many statics counters that the chip has. > > Shouldn't you say which "the chip" you're talking about since wm seems > to handle so many? The current implementation is based on the FreeBSD and Linux. All counters I added to NetBSD are also counted on both FreeBSD and Linux. There are some difference between those two. I also noticed that some code are doubtful and wrote them the different way. To make the implementation perfect, I have to read (almost) all datasheets and test on some chips. It's not good to write only based on the datasheets. One of the reason is that, for example, a register is not listed in the statistics counters' table but the detail of the register is described later in the chapter. It's not clear whether the register is exist or not. The current implementation may not count some useful counters. The current implementation is not the final form but it's worth to have. I could have written the details of the current implementation when I committed, but I didn't really feel the need to write the details of the implementation in its half-way state. My apologies. Thanks. > I suppose this isn't git so you can't fix this easily. > >> - Attach event counters only if available. >> >> >> To generate a diff of this commit: >> cvs rdiff -u -r1.745 -r1.746 src/sys/dev/pci/if_wm.c >> cvs rdiff -u -r1.126 -r1.127 src/sys/dev/pci/if_wmreg.h >> >> Please note that diffs are not public domain; they are subject to the >> copyright notices on the relevant files. >> >> > -- --- SAITOH Masanobu (msai...@execsw.org msai...@netbsd.org)
Re: CVS commit: src/sys/dev/pci
On Fri, 22 Jul 2022, SAITOH Masanobu wrote: Module Name:src Committed By: msaitoh Date: Fri Jul 22 05:23:50 UTC 2022 Modified Files: src/sys/dev/pci: if_wm.c if_wmreg.h Log Message: Add more statistics countes. - Add many statics counters that the chip has. Shouldn't you say which "the chip" you're talking about since wm seems to handle so many? I suppose this isn't git so you can't fix this easily. - Attach event counters only if available. To generate a diff of this commit: cvs rdiff -u -r1.745 -r1.746 src/sys/dev/pci/if_wm.c cvs rdiff -u -r1.126 -r1.127 src/sys/dev/pci/if_wmreg.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. -- Hisashi T Fujinaka - ht...@twofifty.com BSEE + BSChem + BAEnglish + MSCS + $2.50 = coffee
re: CVS commit: src/sys/dev/pci
On Sat, 9 Jul 2022, matthew green wrote:
"Paul Goyette" writes:
Module Name:src
Committed By: pgoyette
Date: Fri Jul 8 17:32:19 UTC 2022
Modified Files:
src/sys/dev/pci: nvme_pci.c
Log Message:
devsw_ok needs to survive across invocations of nvme_modcmd() so
allocate it statically.
Should address remaining issues with kern/56914
if (error) {
+ devsw_ok = false;
shouldn't devsw_ok be "false" here already? seems more like
something to ASSERT() than assign.
Yeah, this is likely unnecessary now. It got there during a
debug iteration.
I will remove.
++--+--+
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:|
| (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com|
| Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org |
| & Network Engineer | | pgoyett...@gmail.com |
++--+--+
re: CVS commit: src/sys/dev/pci
"Paul Goyette" writes:
> Module Name: src
> Committed By: pgoyette
> Date: Fri Jul 8 17:32:19 UTC 2022
>
> Modified Files:
> src/sys/dev/pci: nvme_pci.c
>
> Log Message:
> devsw_ok needs to survive across invocations of nvme_modcmd() so
> allocate it statically.
>
> Should address remaining issues with kern/56914
if (error) {
+ devsw_ok = false;
shouldn't devsw_ok be "false" here already? seems more like
something to ASSERT() than assign.
.mrg.
Re: CVS commit: src/sys/dev/usb
Hi, On 2022/05/14 19:44, Taylor R Campbell wrote: > Module Name: src > Committed By: riastradh > Date: Sat May 14 19:44:37 UTC 2022 > > Modified Files: > src/sys/dev/usb: xhci.c > > Log Message: > xhci(4): Handle race between software abort and hardware stall. xhci_abortx is expected to stop given single xfer, but it actually stops all xfers in pipe. When usbd_ar_pipe stops the first xfer in up_queue of isoc pipe such as uvideo(4), HCI generates multiple Transfer Events (UVIDEO_NXFERS (3) for uvideo) in order xfers are posted. ux_status of first xfer is set to USBD_CANCELLED by usbd_xfer_abort, so usbd_xfer_trycomplete in xhci_event_transfer fails and usb_transfer_complete is not called (xhci_abortx does it instead). However, other two xfers has ux_status = USBD_IN_PROGRESS, depending on how quick events are generated, xhci_event_transfer may call usb_transfer_complete for them before xhci_abortx calls usb_transfer_complete. It may fire KASSERT failure "not start of queue."
Re: CVS commit: src/sys/dev/marvell
On 2022/05/21 19:27, Rin Okuyama wrote: Module Name:src Committed By: rin Date: Sat May 21 10:27:30 UTC 2022 Modified Files: src/sys/dev/marvell: if_mvgbe.c Log Message: m_freem() *after* bus_dmamap_sync() and bus_dmamap_load() for that mbuf. This is mandatory for some archs. To generate a diff of this commit: cvs rdiff -u -r1.64 -r1.65 src/sys/dev/marvell/if_mvgbe.c s/load/unload/ here. Thanks riastradh@ for pointing out. rin
Re: CVS commit: src/sys/dev/raidframe
> On 16. Apr 2022, at 18:40, Andrius Varanavicius wrote: > > Module Name: src > Committed By: andvar > Date: Sat Apr 16 16:40:54 UTC 2022 > > Modified Files: > src/sys/dev/raidframe: rf_netbsdkintf.c > > Log Message: > Fix mistake in error branch locking caused by previous changes. > vput(vp) also unlocks vp, thus unlocking happens twice in error flow > causing kernel to panic with failed assertion lktype != LK_NONE > in vfs_vnode.c#778. Thanks riastradh with finding the issue. > > > To generate a diff of this commit: > cvs rdiff -u -r1.406 -r1.407 src/sys/dev/raidframe/rf_netbsdkintf.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. Thanks, replacing vput() with vrele() would have been even better ... -- J. Hannken-Illjes - hann...@mailbox.org signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/sys/dev/pci
On Sun, Feb 27, 2022 at 11:50:15AM +0100, Martin Husemann wrote: > On Sun, Feb 27, 2022 at 12:04:58AM +0100, Joerg Sonnenberger wrote: > > Personally, I prefer the use of the C extension in cases like this. The > > "portable" version is less readable... > > I would prefer the type correct C++ style variant (and making lint deal > with it, if it doesn't yet). At least gcc is happy with it. Ah, never mind - the inline function is void already - so effectively I agree with Jörg. Martin
Re: CVS commit: src/sys/dev/pci
On Sun, Feb 27, 2022 at 12:04:58AM +0100, Joerg Sonnenberger wrote:
> Personally, I prefer the use of the C extension in cases like this. The
> "portable" version is less readable...
I would prefer the type correct C++ style variant (and making lint deal
with it, if it doesn't yet). At least gcc is happy with it.
Index: if_wm.c
===
RCS file: /cvsroot/src/sys/dev/pci/if_wm.c,v
retrieving revision 1.728
diff -u -p -r1.728 if_wm.c
--- if_wm.c 26 Feb 2022 14:53:05 - 1.728
+++ if_wm.c 27 Feb 2022 10:48:11 -
@@ -10025,7 +10025,7 @@ wm_txrxintr_disable(struct wm_queue *wmq
struct wm_softc *sc = wmq->wmq_txq.txq_sc;
if (__predict_false(!wm_is_using_msix(sc))) {
- return wm_legacy_intr_disable(sc);
+ return (void)wm_legacy_intr_disable(sc);
}
if (sc->sc_type == WM_T_82574)
@@ -10046,7 +10046,7 @@ wm_txrxintr_enable(struct wm_queue *wmq)
wm_itrs_calculate(sc, wmq);
if (__predict_false(!wm_is_using_msix(sc))) {
- return wm_legacy_intr_enable(sc);
+ return (void)wm_legacy_intr_enable(sc);
}
/*
Re: CVS commit: src/sys/dev/pci
Am Sat, Feb 26, 2022 at 03:04:40PM + schrieb Roland Illig: > Module Name: src > Committed By: rillig > Date: Sat Feb 26 15:04:40 UTC 2022 > > Modified Files: > src/sys/dev/pci: if_wm.c > > Log Message: > if_wm.c: fix value return from void function > > lint complained: > if_wm.c(10028): error: > void function wm_txrxintr_disable cannot return value [213] > if_wm.c(10049): error: > void function wm_txrxintr_enable cannot return value [213] > > No functional change. Personally, I prefer the use of the C extension in cases like this. The "portable" version is less readable... Joerg
Re: CVS commit: src/sys/dev/fdt
Should fix PR 56650 > Module Name: src > Committed By: macallan > Date: Fri Jan 21 21:00:26 UTC 2022 > > Modified Files: > src/sys/dev/fdt: fdt_port.c > > Log Message: > when enumerating ports and endpoints treat missing 'reg' properties as zero > ok jmcneill: > Looking at Linux. If port or endpoint are missing a 'reg' property it > defaults to 0. > Please make our code do the same. > > see > https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/graph.yaml > > with this my pinebook has a usable console again > > > To generate a diff of this commit: > cvs rdiff -u -r1.6 -r1.7 src/sys/dev/fdt/fdt_port.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. >
Re: CVS commit: src/sys/dev/pci
On 23/12/2021 17:05, Juergen Hannken-Illjes wrote: Module Name:src Committed By: hannken Date: Thu Dec 23 17:05:49 UTC 2021 Modified Files: src/sys/dev/pci: if_wm.c Log Message: Keep constants 32 bit, why does __BIT() return uintmax_t? PRIxBIT is available Nick
Re: CVS commit: src/sys/dev/tprof
On 26/11/2021 13:24, Christos Zoulas wrote: Module Name:src Committed By: christos Date: Fri Nov 26 13:24:28 UTC 2021 Modified Files: src/sys/dev/tprof: tprof_armv7.c tprof_armv8.c Log Message: declare xc Thanks! Nick
Re: CVS commit: src/sys/dev/sbus
>Module Name: src >Committed By: macallan >Date: Sat Oct 30 05:37:39 UTC 2021 > >Modified Files: > src/sys/dev/sbus: mgx.c mgxreg.h > >Log Message: >actually mmap() the blitter registers when asked to, while there do some >magic number reduction > > >To generate a diff of this commit: >cvs rdiff -u -r1.17 -r1.18 src/sys/dev/sbus/mgx.c >cvs rdiff -u -r1.5 -r1.6 src/sys/dev/sbus/mgxreg.h @@ -684,6 +684,8 @@ uint32_t fg, bg; int x, y, wi, he, rv; +if (sc->sc_mode != WSDISPLAYIO_MODE_EMUL) return; + wi = font->fontwidth; he = font->fontheight; @@ -731,6 +733,8 @@ uint32_t fg, bg, scratch = ((sc->sc_stride * sc->sc_height) + 7) & ~7; int x, y, wi, he, len, i; +if (sc->sc_mode != WSDISPLAYIO_MODE_EMUL) return; + wi = font->fontwidth; he = font->fontheight; This seems to be a strange indent. Or debugging code mixed in? -- ryo shimizu
Re: CVS commit: src/sys/dev/pci
On Thu, Oct 21, 2021 at 05:32:28AM +, Shoichi YAMAGUCHI wrote: > Module Name: src > Committed By: yamaguchi > Date: Thu Oct 21 05:32:27 UTC 2021 > > Modified Files: > src/sys/dev/pci: virtio.c virtio_pci.c virtiovar.h > > Log Message: > divide setup routine of virtio interrupts > into establishment and device configuration > > > To generate a diff of this commit: > cvs rdiff -u -r1.49 -r1.50 src/sys/dev/pci/virtio.c > cvs rdiff -u -r1.30 -r1.31 src/sys/dev/pci/virtio_pci.c > cvs rdiff -u -r1.20 -r1.21 src/sys/dev/pci/virtiovar.h This seems to have broken the virtio_mmio build.
Re: CVS commit: src/sys/dev/pci
Hi, Thank you for your quick commit! Thanks, On 2021/10/18 17:15, Juergen Hannken-Illjes wrote: Module Name:src Committed By: hannken Date: Mon Oct 18 08:15:00 UTC 2021 Modified Files: src/sys/dev/pci: xmm7360.c Log Message: Use a local static variable to hold "pktq_rps_hash_default" like the other devices do. Kernel ALL/amd64 compiles again. OK: Kengo NAKAHARA To generate a diff of this commit: cvs rdiff -u -r1.12 -r1.13 src/sys/dev/pci/xmm7360.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. -- // Internet Initiative Japan Inc. Device Engineering Section, Product Division, Technology Unit Kengo NAKAHARA
Re: CVS commit: src/sys/dev/wscons
On Sun, Oct 10, 2021 at 10:44:03PM +0900, Izumi Tsutsui wrote: > > Module Name:src > > Committed By: nia > > Date: Tue Sep 28 06:14:28 UTC 2021 > > > > Modified Files: > > src/sys/dev/wscons: wsconsio.h wsmouse.c wsmousevar.h > > > > Log Message: > > wsmouse: add support for "precision scrolling" events and (GET|SET)PARAMS > > Could you please also update wsmouse(4) and wsmouse(9) man pages? > Even the current version lacks various info (especially about ioctl(2)) > but no reason to make it worse. > > Thanks, > --- > Izumi Tsutsui Sure, I had been meaning to anyway but I clearly forgot to commit those files...
Re: CVS commit: src/sys/dev/wscons
> Module Name: src > Committed By: nia > Date: Tue Sep 28 06:14:28 UTC 2021 > > Modified Files: > src/sys/dev/wscons: wsconsio.h wsmouse.c wsmousevar.h > > Log Message: > wsmouse: add support for "precision scrolling" events and (GET|SET)PARAMS Could you please also update wsmouse(4) and wsmouse(9) man pages? Even the current version lacks various info (especially about ioctl(2)) but no reason to make it worse. Thanks, --- Izumi Tsutsui
Re: CVS commit: src/sys/dev/usb
On 15/07/2021 04:25, Tohru Nishimura wrote: Module Name:src Committed By: nisimura Date: Thu Jul 15 03:25:50 UTC 2021 Modified Files: src/sys/dev/usb: if_mue.c uchcom.c Log Message: explanation typo To generate a diff of this commit: cvs rdiff -u -r1.60 -r1.61 src/sys/dev/usb/if_mue.c cvs rdiff -u -r1.38 -r1.39 src/sys/dev/usb/uchcom.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/dev/usb/if_mue.c diff -u src/sys/dev/usb/if_mue.c:1.60 src/sys/dev/usb/if_mue.c:1.61 --- src/sys/dev/usb/if_mue.c:1.60 Sat Jun 27 13:33:26 2020 +++ src/sys/dev/usb/if_mue.cThu Jul 15 03:25:50 2021 [snip] did you mean to commit the if_mue.c change? Certainly the commit message doesn't match. Thanks, Nick
Re: CVS commit: src/sys/dev/pad
On 2021/06/16 19:23, Rin Okuyama wrote: On 2021/06/16 18:15, Taylor R Campbell wrote: Date: Wed, 16 Jun 2021 17:38:26 +0900 From: Rin Okuyama KASSERT added to pad_attach() by this commit fires on macppc with mixerctl(1): [...] panic: kernel diagnostic assertion "KERNEL_LOCKED_P()" failed: file "/usr/src/sys/dev/pad/pad.c", line 214 Can you share `ident netbsd | grep -e pad.c -e subr_autoconf.c' output? config_attach_pseudo takes the kernel lock, so this assertion should definitely not fire. I don't understand why this happens on macppc, while does not on majority of other machines. Can this behavior depend on underlying audio(4) driver? If so, what should I do to fix? I don't think so -- this happens before we even reach audio_attach_mi. Thanks for hints! Seems like kernel module bug for powerpc. pad(4) is not in GENERIC. If it is in kernel, panic does not take place. On the other hand, if it is supplied as module, even if its pad.c (and subr_autoconf.c in main kernel) is up to date, panic occurs. Another problem is kernel modules for powerpc is: http://mail-index.netbsd.org/port-powerpc/2020/07/07/msg003590.html I still haven't figured out why... This turned out to be an MI bug between UP kernel v.s. modules: http://mail-index.netbsd.org/source-changes/2021/06/16/msg130239.html Should be fixed now :). Thanks, rin
Re: CVS commit: src/sys/dev/pad
On 2021/06/16 18:15, Taylor R Campbell wrote: Date: Wed, 16 Jun 2021 17:38:26 +0900 From: Rin Okuyama KASSERT added to pad_attach() by this commit fires on macppc with mixerctl(1): [...] panic: kernel diagnostic assertion "KERNEL_LOCKED_P()" failed: file "/usr/src/sys/dev/pad/pad.c", line 214 Can you share `ident netbsd | grep -e pad.c -e subr_autoconf.c' output? config_attach_pseudo takes the kernel lock, so this assertion should definitely not fire. I don't understand why this happens on macppc, while does not on majority of other machines. Can this behavior depend on underlying audio(4) driver? If so, what should I do to fix? I don't think so -- this happens before we even reach audio_attach_mi. Thanks for hints! Seems like kernel module bug for powerpc. pad(4) is not in GENERIC. If it is in kernel, panic does not take place. On the other hand, if it is supplied as module, even if its pad.c (and subr_autoconf.c in main kernel) is up to date, panic occurs. Another problem is kernel modules for powerpc is: http://mail-index.netbsd.org/port-powerpc/2020/07/07/msg003590.html I still haven't figured out why... Thanks, rin
Re: CVS commit: src/sys/dev/pad
> Date: Wed, 16 Jun 2021 17:38:26 +0900 > From: Rin Okuyama > > KASSERT added to pad_attach() by this commit fires on macppc with mixerctl(1): > [...] panic: kernel diagnostic assertion "KERNEL_LOCKED_P()" failed: file > "/usr/src/sys/dev/pad/pad.c", line 214 Can you share `ident netbsd | grep -e pad.c -e subr_autoconf.c' output? config_attach_pseudo takes the kernel lock, so this assertion should definitely not fire. > I don't understand why this happens on macppc, while does not on > majority of other machines. Can this behavior depend on underlying > audio(4) driver? If so, what should I do to fix? I don't think so -- this happens before we even reach audio_attach_mi.
Re: CVS commit: src/sys/dev/pad
Hi, KASSERT added to pad_attach() by this commit fires on macppc with mixerctl(1): # cd /usr/tests/usr.bin/mixerctl && atf-run ... tc-start: ..., nflag [...] panic: kernel diagnostic assertion "KERNEL_LOCKED_P()" failed: file "/usr/src/sys/dev/pad/pad.c", line 214 [...] cpu0: Begin traceback... [...] ... vpanic ... [...] ... kern_assert ... [...] ... pad_attach ... [...] ... config_attach_pseudo ... [...] ... pad_open ... [...] ... spec_open ... [...] ... VOP_OPEN ... [...] ... vn_open ... [...] ... do_open ... [...] ... do_sys_openat ... [...] ... sys_open ... [...] ... syscall ... [...] user SC trap #5 by ... (copy from framebuffer console by hands) I don't understand why this happens on macppc, while does not on majority of other machines. Can this behavior depend on underlying audio(4) driver? If so, what should I do to fix? Thanks, rin On 2021/06/14 19:14, Taylor R Campbell wrote: Module Name:src Committed By: riastradh Date: Mon Jun 14 10:14:58 UTC 2021 Modified Files: src/sys/dev/pad: pad.c Log Message: pad(4): Make this exclusively a cloning device. padN numbering never corresponded with audioM numbering except by accident, so the non-cloning device never worked reliably for scripting. This simplifies the logic substantially. While here, fix drvctl detach race. To generate a diff of this commit: cvs rdiff -u -r1.70 -r1.71 src/sys/dev/pad/pad.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Re: CVS commit: src/sys/dev
On Wed, 9 Jun 2021, Paul Goyette wrote: Module Name:src Committed By: pgoyette Date: Wed Jun 9 23:22:51 UTC 2021 Modified Files: src/sys/dev: dev_verbose.h Log Message: Use the localcount(9)-based module_hook mechanism to prevent the verbose modules' code and data being unloaded while in use. Should prevent some crashes reported by Riastradh@ to occur during suspend/resume operation. FYI, this commit "rides the bump" introduced a few hours ago by martin@ ++--+-+ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com | | Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org | ++--+-+
Re: CVS commit: src/sys/dev/audio
On 2021/06/08 16:09, nia wrote: On Tue, Jun 01, 2021 at 09:12:24PM +, Taylor R Campbell wrote: audio(4): Set AUMODE_PLAY/RECORD only if asked _and_ supported. If one is requested and _not_ supported, fail; otherwise we might enter audio_write with a null play track and crash on KASSERT. It looks like this is an incompatible change. Sun says: - Attempts to open a device with FREAD set fail if the device is not capable of recording. (Likewise for FWRITE and playback.) https://github.com/illumos/illumos-gate/blob/9ecd05bdc59e4a1091c51ce68cce2028d5ba6fd1/usr/src/uts/common/io/audio/impl/audio_sun.c#L70 But in NetBSD 7... audio_open() does not return a clear failure: https://github.com/NetBSD/src/blob/netbsd-7/sys/dev/audio.c#L1652 EINVAL is returned if !audio_can_playback in audiostartp() https://github.com/NetBSD/src/blob/netbsd-7/sys/dev/audio.c#L2801 ... which is called from audio_write() So it looks to me like open() should succeed but write() should fail. This is important if you want to open an audio device just to test a few properties (i.e. AUDIO_GETPROPS, AUDIO_GETDEV...). Also, FYI, some tests for audio(4) have started to fail somewhen between audio.c revs 1.96 (this commit) to 1.102: https://releng.netbsd.org/b5reports/i386/commits-2021.06.html#2021.06.02.08.46.16 Thanks, rin
Re: CVS commit: src/sys/dev/audio
On Tue, Jun 01, 2021 at 09:12:24PM +, Taylor R Campbell wrote: > audio(4): Set AUMODE_PLAY/RECORD only if asked _and_ supported. > > If one is requested and _not_ supported, fail; otherwise we might > enter audio_write with a null play track and crash on KASSERT. It looks like this is an incompatible change. Sun says: - Attempts to open a device with FREAD set fail if the device is not capable of recording. (Likewise for FWRITE and playback.) https://github.com/illumos/illumos-gate/blob/9ecd05bdc59e4a1091c51ce68cce2028d5ba6fd1/usr/src/uts/common/io/audio/impl/audio_sun.c#L70 But in NetBSD 7... audio_open() does not return a clear failure: https://github.com/NetBSD/src/blob/netbsd-7/sys/dev/audio.c#L1652 EINVAL is returned if !audio_can_playback in audiostartp() https://github.com/NetBSD/src/blob/netbsd-7/sys/dev/audio.c#L2801 ... which is called from audio_write() So it looks to me like open() should succeed but write() should fail. This is important if you want to open an audio device just to test a few properties (i.e. AUDIO_GETPROPS, AUDIO_GETDEV...).
Re: CVS commit: src/sys/dev
On Sat, 24 Apr 2021, Robert Elz wrote:
Date:Sat, 24 Apr 2021 00:15:37 +
From:"Michael Lorenz"
Message-ID: <20210424001537.c5c83f...@cvs.netbsd.org>
| add an ioctl() to get a list of fonts currently available via wsfont
It seems to me it would be useful for that ioctl to copyout()
the fi_numentries field of the struct (if addr != NULL) from
wsdisplayio_listfonts() just before the ENOMEM check (so it is
updated, even if ENOMEM is returned). (Does it make any sense
for addr to be NULL, or should that be an error? EINVAL or something.)
Otherwise, there doesn't seem to be any easy way for the user of
the ioctl to know how many fonts were returned (checking which elements
of the array were modified is not "easy") or how big the buffer would
need to be to fetch all of them in the ENOMEM case.
In several other places, we return "total space needed" separately,
regardless of how much data was actually copied. The general paradigm
is (more or less)
buff = NULL;
size = 0;
err = func(..., buff, size, &need);
while (err == 0) {
if (need > size) {
free(buff);
buff = malloc(need);
if (buff == NULL)
err = ENOMEM;
}
}
For a real-life example look at the modctl(2) code for MODULE_STAT
++--+---+
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com |
| Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org |
++--+---+
Re: CVS commit: src/sys/dev
Oh, I see from your code change to wsfontload.c that you intended for the fi_numentries field to get copied out, it just doesn't seem to happen. I also see that the addr==NULL case happens if malloc() (in wsfontload.c) failed - going ahead with the ioctl() in that case seems like a mistake, optimising away the error checking that way looks fragile. (And the magic 4096 even moreso). Just check the malloc() result, and then in the ioctl code, test for it as well, and return an error from there, rather than doing all the work for no benefit in that case.Alternatively, you could define that case to be a "fetch the count" variant of the ioctl, where all that happens in that case is that the fi_numentries field of the struct is filled in and returned. kre kre
Re: CVS commit: src/sys/dev
Date:Sat, 24 Apr 2021 00:15:37 + From:"Michael Lorenz" Message-ID: <20210424001537.c5c83f...@cvs.netbsd.org> | add an ioctl() to get a list of fonts currently available via wsfont It seems to me it would be useful for that ioctl to copyout() the fi_numentries field of the struct (if addr != NULL) from wsdisplayio_listfonts() just before the ENOMEM check (so it is updated, even if ENOMEM is returned). (Does it make any sense for addr to be NULL, or should that be an error? EINVAL or something.) Otherwise, there doesn't seem to be any easy way for the user of the ioctl to know how many fonts were returned (checking which elements of the array were modified is not "easy") or how big the buffer would need to be to fetch all of them in the ENOMEM case. It might also be worth allowing fi_numentries to also be an input parameter, to indicate where in the set of fonts the fetch should start, to provide a mechanism to cope should the list size ever grow so big that it ends up bigger than a single ioctl can handle (that is, skip the first fi_numentries fonts, and then continue from there). That would mean a slightly more complex piece of internal code though. And this last bit is just style, but I'd change the struct wsdisplayio_fontdesc to be uint32_t fd_len; uint16_t fd_height; unit16_t fd_width; char fd_name[]; and then make the entries returned be (rounded up) just big enough for the actual font names. But that's just me, and not important. kre
Re: CVS commit: src/sys/dev/pci
> On 12. Jul 2020, at 21:05, Jaromir Dolecek wrote: > > Module Name: src > Committed By: jdolecek > Date: Sun Jul 12 19:05:32 UTC 2020 > > Modified Files: > src/sys/dev/pci: if_bnx.c if_bnxvar.h > > Log Message: > enable MSI/MSI-X if supported by adapter > > tested MSI-X with Broadcom NetXtreme II BCM5709 1000Base-T > > > To generate a diff of this commit: > cvs rdiff -u -r1.95 -r1.96 src/sys/dev/pci/if_bnx.c > cvs rdiff -u -r1.12 -r1.13 src/sys/dev/pci/if_bnxvar.h > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. With this change my Dell PowerEdge 2850 spits watchdog resets: [ 68.1828359] bnx0: Watchdog timeout -- resetting! [ 88.6042909] bnx0: Watchdog timeout -- resetting! [ 119.0265230] bnx0: Watchdog timeout -- resetting! [ 145.4484562] bnx0: Watchdog timeout -- resetting! Dmesg before is: [ 1.017306] pci4 at ppb3 bus 7 [ 1.017306] pci4: i/o space, memory space enabled, rd/line, wr/inv ok [ 1.017306] bnx0 at pci4 dev 0 function 0: Broadcom NetXtreme II BCM5708 1000Base-T [ 1.017306] bnx0: Ethernet address 00:24:e8:67:4b:db [ 1.017306] bnx0: ASIC BCM5708 B2 (0x57081020) [ 1.017306] bnx0: PCI-X 64bit 133MHz [ 1.017306] bnx0: B/C (4.6.0); Bufs (RX:2;TX:2); Flags (MFW); MFW (ipms 1.6.0) [ 1.017306] bnx0: Coal (RX:6,6,18,18; TX:20,20,80,80) [ 1.017306] brgphy0 at bnx0 phy 1: BCM5708C 1000BASE-T media interface, rev. 6 [ 1.017306] brgphy0: 10baseT, 10baseT-FDX, 100baseTX, 100baseTX-FDX, 1000baseT, 1000baseT-FDX, auto [ 1.017306] bnx0: interrupting at ioapic0 pin 16 while dmesg after is: [ 1.017262] pci4 at ppb3 bus 7 [ 1.017262] pci4: i/o space, memory space enabled, rd/line, wr/inv ok [ 1.017262] bnx0 at pci4 dev 0 function 0: Broadcom NetXtreme II BCM5708 1000Base-T [ 1.017262] bnx0: Ethernet address 00:24:e8:67:4b:db [ 1.017262] bnx0: ASIC BCM5708 B2 (0x57081020) [ 1.017262] bnx0: PCI-X 64bit 133MHz [ 1.017262] bnx0: B/C (4.6.0); Bufs (RX:2;TX:2); Flags (MFW); MFW (ipms 1.6.0) [ 1.017262] bnx0: Coal (RX:6,6,18,18; TX:20,20,80,80) [ 1.017262] brgphy0 at bnx0 phy 1: BCM5708C 1000BASE-T media interface, rev. 6 [ 1.017262] brgphy0: 10baseT, 10baseT-FDX, 100baseTX, 100baseTX-FDX, 1000baseT, 1000baseT-FDX, auto [ 1.017262] bnx0: interrupting at msi0 vec 0 Pcictl dump gives (in the MSI-X case): PCI Message Signaled Interrupt Message Control register: 0x0081 MSI Enabled: on Multiple Message Capable: no (1 vector) Multiple Message Enabled: off (1 vector) 64 Bit Address Capable: on Per-Vector Masking Capable: off Extended Message Data Capable: off Extended Message Data Enable: off Message Address (lower) register: 0xfee0 Message Address (upper) register: 0x Message Data register: 0x0064 Any ideas how to fix this issue? -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany) signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/sys/dev/pci
On Tue, Jan 26, 2021 at 05:51:42PM +0900, Rin Okuyama wrote: > Hi, > This seems not correct for me. Is the attached patch OK with you? Well you spotted a bug indeed int he freeing section. I'll fix and commit it. Thanks for reporting. Reinoud signature.asc Description: PGP signature
Re: CVS commit: src/sys/dev/pci
Hi,
On 2021/01/25 0:33, Reinoud Zandijk wrote:
Module Name:src
Committed By: reinoud
Date: Sun Jan 24 15:33:02 UTC 2021
Modified Files:
src/sys/dev/pci: virtio_pci.c
Log Message:
On error unmap the pci_mapreg_map()d regions using bus_space_unmap() as
suggested by jak@
To generate a diff of this commit:
cvs rdiff -u -r1.22 -r1.23 src/sys/dev/pci/virtio_pci.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
This seems not correct for me. Is the attached patch OK with you?
Thanks,
rin
Index: sys/dev/pci/virtio_pci.c
===
RCS file: /home/netbsd/src/sys/dev/pci/virtio_pci.c,v
retrieving revision 1.25
diff -p -u -r1.25 virtio_pci.c
--- sys/dev/pci/virtio_pci.c24 Jan 2021 15:59:35 - 1.25
+++ sys/dev/pci/virtio_pci.c26 Jan 2021 08:47:23 -
@@ -444,7 +444,7 @@ virtio_pci_attach_10(device_t self, void
bus_size_t bars[NMAPREG] = { 0 };
int bars_idx[NMAPREG] = { 0 };
struct virtio_pci_cap *caps[] = { &common, &isr, &device, ¬ify.cap };
- int i, j = 0, ret = 0;
+ int i, j, ret = 0;
if (virtio_pci_find_cap(psc, VIRTIO_PCI_CAP_COMMON_CFG,
&common, sizeof(common)))
@@ -471,7 +471,7 @@ virtio_pci_attach_10(device_t self, void
bars[bar] = len;
}
- for (i = 0; i < __arraycount(bars); i++) {
+ for (i = j = 0; i < __arraycount(bars); i++) {
int reg;
pcireg_t type;
if (bars[i] == 0)
@@ -550,11 +550,12 @@ virtio_pci_attach_10(device_t self, void
err:
/* undo our pci_mapreg_map()s */
- for (i = 0; i < __arraycount(bars); i++) {
+ for (i = j = 0; i < __arraycount(bars); i++) {
if (bars[i] == 0)
continue;
bus_space_unmap(psc->sc_bars_iot[j], psc->sc_bars_ioh[j],
psc->sc_bars_iosize[j]);
+ j++;
}
return ret;
}
Re: CVS commit: src/sys/dev/pci
> On Jan 24, 2021, at 10:20 PM, Martin Husemann wrote: > >> I kept getting a ?static after non-static declaration? error when building >> for aarch64. > > I guess that was with outdated arm/include/bus_funcs.h and > sys/bus_proto.h (or where was the previous declaration)? I did a “cvs update” just before, *shrug*. In any case, the problem was easily avoidable, and now it is avoided. -- thorpej
Re: CVS commit: src/sys/dev/pci
On Sun, Jan 24, 2021 at 09:45:14PM -0800, Jason Thorpe wrote: > > > On Jan 24, 2021, at 9:37 PM, Martin Husemann wrote: > > > > While I don't care for names, I would like to understand fallout in > > details before hiding it - what exactly did not compile correctly? > > At least the affected arm kernels worked for me in the state directly > > before your commit. > > I kept getting a ?static after non-static declaration? error when building > for aarch64. I guess that was with outdated arm/include/bus_funcs.h and sys/bus_proto.h (or where was the previous declaration)? Martin
Re: CVS commit: src/sys/dev/pci
> On Jan 24, 2021, at 9:37 PM, Martin Husemann wrote: > > While I don't care for names, I would like to understand fallout in > details before hiding it - what exactly did not compile correctly? > At least the affected arm kernels worked for me in the state directly > before your commit. I kept getting a “static after non-static declaration” error when building for aarch64. -- thorpej
Re: CVS commit: src/sys/dev/pci
On Sun, Jan 24, 2021 at 03:34:08PM +, Jason R Thorpe wrote: > Module Name: src > Committed By: thorpej > Date: Sun Jan 24 15:34:08 UTC 2021 > > Modified Files: > src/sys/dev/pci: virtio_pci.c > > Log Message: > Redefining bus_space functions in drivers is a bad idea, and we just > should't be in the habit of doing so. Besides, the previous "solutions" > still did not compile correctly, and this does, so let's be done with > this nonsense, shall we? While I don't care for names, I would like to understand fallout in details before hiding it - what exactly did not compile correctly? At least the affected arm kernels worked for me in the state directly before your commit. Martin
Re: CVS commit: src/sys/dev/pci
> On Jan 23, 2021, at 5:40 PM, Christos Zoulas wrote: > >> it's a bad example. someone might copy it into another >> driver that _doesn't_ work with this version, but may seem >> to fix a build error. >> >> that's why i wanted to wrap the usage to make it clear if >> someone were to copy it elsewhere. > > I will add a comment. Yah, I was gonna say, “big fat comment in order!" -- thorpej
Re: CVS commit: src/sys/dev/pci
In article <17141.1611452...@splode.eterna.com.au>, matthew green wrote: >Christos Zoulas writes: >> In article <20210123230600.52be160...@jupiter.mumble.net>, >> Taylor R Campbell wrote: >> > >> >Conversely, how do you know whether this hacked-up implementation >> >which tears the write into two will actually work? Maybe it works for >> >virtio but there are likely other devices out there for which it will >> >fail or have weird side effects if the architecture doesn't have >> >native 8-byte bus I/O. >> >> But it is a static function defined in virtio_pci.c. How will other >> devices use it? I must be missing something. > >it's a bad example. someone might copy it into another >driver that _doesn't_ work with this version, but may seem >to fix a build error. > >that's why i wanted to wrap the usage to make it clear if >someone were to copy it elsewhere. I will add a comment. christos
re: CVS commit: src/sys/dev/pci
Christos Zoulas writes: > In article <20210123230600.52be160...@jupiter.mumble.net>, > Taylor R Campbell wrote: > > > >Conversely, how do you know whether this hacked-up implementation > >which tears the write into two will actually work? Maybe it works for > >virtio but there are likely other devices out there for which it will > >fail or have weird side effects if the architecture doesn't have > >native 8-byte bus I/O. > > But it is a static function defined in virtio_pci.c. How will other > devices use it? I must be missing something. it's a bad example. someone might copy it into another driver that _doesn't_ work with this version, but may seem to fix a build error. that's why i wanted to wrap the usage to make it clear if someone were to copy it elsewhere. .mrg.
Re: CVS commit: src/sys/dev/pci
In article <20210123230600.52be160...@jupiter.mumble.net>, Taylor R Campbell wrote: > >Conversely, how do you know whether this hacked-up implementation >which tears the write into two will actually work? Maybe it works for >virtio but there are likely other devices out there for which it will >fail or have weird side effects if the architecture doesn't have >native 8-byte bus I/O. But it is a static function defined in virtio_pci.c. How will other devices use it? I must be missing something. christos
Re: CVS commit: src/sys/dev/pci
> Date: Sat, 23 Jan 2021 22:59:22 - (UTC)
> From: chris...@astron.com (Christos Zoulas)
>
> In article <23974.1611441...@splode.eterna.com.au>,
> matthew green wrote:
> >this seems dangerous to me. we don't define it on
> >some platforms because we can't, so having it faked
> >out here seems like someone later will be confused
> >and the wrong thing will happen.
> >
> >i would rather have something like
> >
> >virtio_write8(...)
> >{
> >#ifdef __HAVE_BUS_SPACE_8
> > just use the real thing
> >#else
> > use the dual-_4 version that is ok _for this device_
> >#endif
> >}
> >
> >and then use this wrapper in the rest of the code.
>
> This implementation is internal to virtio_pci and is guaranteed to work
> by the spec, how will someone else us it?
Conversely, how do you know whether this hacked-up implementation
which tears the write into two will actually work? Maybe it works for
virtio but there are likely other devices out there for which it will
fail or have weird side effects if the architecture doesn't have
native 8-byte bus I/O.
Re: CVS commit: src/sys/dev/pci
In article <23974.1611441...@splode.eterna.com.au>,
matthew green wrote:
>"Christos Zoulas" writes:
>> Module Name: src
>> Committed By:christos
>> Date:Sat Jan 23 20:00:19 UTC 2021
>>
>> Modified Files:
>> src/sys/dev/pci: virtio_pci.c
>>
>> Log Message:
>> Provide a generic bus_space_write_8 function that is bi-endian.
>
>this seems dangerous to me. we don't define it on
>some platforms because we can't, so having it faked
>out here seems like someone later will be confused
>and the wrong thing will happen.
>
>i would rather have something like
>
>virtio_write8(...)
>{
>#ifdef __HAVE_BUS_SPACE_8
> just use the real thing
>#else
> use the dual-_4 version that is ok _for this device_
>#endif
>}
>
>and then use this wrapper in the rest of the code.
This implementation is internal to virtio_pci and is guaranteed to work
by the spec, how will someone else us it?
christos
re: CVS commit: src/sys/dev/pci
"Christos Zoulas" writes:
> Module Name: src
> Committed By: christos
> Date: Sat Jan 23 20:00:19 UTC 2021
>
> Modified Files:
> src/sys/dev/pci: virtio_pci.c
>
> Log Message:
> Provide a generic bus_space_write_8 function that is bi-endian.
this seems dangerous to me. we don't define it on
some platforms because we can't, so having it faked
out here seems like someone later will be confused
and the wrong thing will happen.
i would rather have something like
virtio_write8(...)
{
#ifdef __HAVE_BUS_SPACE_8
just use the real thing
#else
use the dual-_4 version that is ok _for this device_
#endif
}
and then use this wrapper in the rest of the code.
.mrg.
Re: CVS commit: src/sys/dev/pci
In article , Reinoud Zandijk wrote: >On Fri, Jan 22, 2021 at 04:54:51PM +1100, matthew green wrote: >> > +#ifndef _LP64 >> >> _LP64 is a terrible way to make this choice. >> >> heaps of our 32 bit platforms implement the _8 variants. > >Can't we then just make sure they have the 8 bit variant? and set a define if >its atomic or not? > >This way drivers van use the _8 variant freely and for the few drivers that >NEED the atomicity, they can check the define and deal with it the way they >like. Perhaps. But still for the ones that don't have it should use the central implementation so that we don't duplicate code. christos
Re: CVS commit: src/sys/dev/pci
In article ,
Nick Hudson wrote:
>On 22/01/2021 04:48, Christos Zoulas wrote:
>> +#if _QUAD_HIGHWORD
>> +bus_space_write_4(iot, ioh, offset, (uint32_t)(value & 0x));
>> +bus_space_write_4(iot, ioh, offset + 4, (uint32_t)(value >> 32));
>> +#else
>> +bus_space_write_4(iot, ioh, offset, (uint32_t)(value >> 32));
>> +bus_space_write_4(iot, ioh, offset + 4, (uint32_t)(value & 0x));
>> +#endif
>> +}
>> +#endif /* !_LP64 */
>
>
>BUS_ADDR_{HI,LO}32 are also available for your convenience.
Will use those thanks!
christos
Re: CVS commit: src/sys/dev/pci
On Fri, Jan 22, 2021 at 04:54:51PM +1100, matthew green wrote: > > +#ifndef _LP64 > > _LP64 is a terrible way to make this choice. > > heaps of our 32 bit platforms implement the _8 variants. Can't we then just make sure they have the 8 bit variant? and set a define if its atomic or not? This way drivers van use the _8 variant freely and for the few drivers that NEED the atomicity, they can check the define and deal with it the way they like. Reinoud
Re: CVS commit: src/sys/dev/pci
On 22/01/2021 04:48, Christos Zoulas wrote:
+#if _QUAD_HIGHWORD
+ bus_space_write_4(iot, ioh, offset, (uint32_t)(value & 0x));
+ bus_space_write_4(iot, ioh, offset + 4, (uint32_t)(value >> 32));
+#else
+ bus_space_write_4(iot, ioh, offset, (uint32_t)(value >> 32));
+ bus_space_write_4(iot, ioh, offset + 4, (uint32_t)(value & 0x));
+#endif
+}
+#endif /* !_LP64 */
BUS_ADDR_{HI,LO}32 are also available for your convenience.
Nick
Re: CVS commit: src/sys/dev/pci
In article <27744.1611294...@splode.eterna.com.au>, matthew green wrote: >> +#ifndef _LP64 > >_LP64 is a terrible way to make this choice. > >heaps of our 32 bit platforms implement the _8 variants. Let's add a _HAVE_ variable then? christos
re: CVS commit: src/sys/dev/pci
> +#ifndef _LP64 _LP64 is a terrible way to make this choice. heaps of our 32 bit platforms implement the _8 variants. .mrg.
Re: CVS commit: src/sys/dev/pci
In article <20210121204833.9ebcff...@cvs.netbsd.org>,
Reinoud Zandijk wrote:
>-=-=-=-=-=-
>
>Module Name: src
>Committed By: reinoud
>Date: Thu Jan 21 20:48:33 UTC 2021
>
>Modified Files:
> src/sys/dev/pci: virtio_pci.c
>
>Log Message:
>Remove dependency on bus_space_write_8() for i386 and instead implement it as
>two bus_space_write_4()'s as allowed in the spec.
Isn't it better to do it this way so it always works (not just for little
endian)? We could even provide this in the MI bus.h if others need it
and don't care about the non-atomic transactions. I have not even compile
tested it.
christos
Index: virtio_pci.c
===
RCS file: /cvsroot/src/sys/dev/pci/virtio_pci.c,v
retrieving revision 1.18
diff -u -p -u -r1.18 virtio_pci.c
--- virtio_pci.c21 Jan 2021 20:48:33 - 1.18
+++ virtio_pci.c22 Jan 2021 04:46:24 -
@@ -34,6 +34,7 @@ __KERNEL_RCSID(0, "$NetBSD: virtio_pci.c
#include
#include
#include
+#include
#include
#include
@@ -731,9 +732,20 @@ virtio_pci_read_queue_size_10(struct vir
* By definition little endian only in v1.0 and 8 byters are allowed to be
* written as two 4 byters
*/
-#define bus_space_write_le_8(iot, ioh, reg, val) \
- bus_space_write_4(iot, ioh, reg, ((uint64_t) (val)) & 0x); \
- bus_space_write_4(iot, ioh, reg + 4, ((uint64_t) (val)) >> 32);
+#ifndef _LP64
+static __inline void
+bus_space_write_8(bus_space_tag_t iot, bus_space_handle_t ioh,
+ bus_size_t offset, uint64_t value)
+{
+#if _QUAD_HIGHWORD
+ bus_space_write_4(iot, ioh, offset, (uint32_t)(value & 0x));
+ bus_space_write_4(iot, ioh, offset + 4, (uint32_t)(value >> 32));
+#else
+ bus_space_write_4(iot, ioh, offset, (uint32_t)(value >> 32));
+ bus_space_write_4(iot, ioh, offset + 4, (uint32_t)(value & 0x));
+#endif
+}
+#endif /* !_LP64 */
static void
virtio_pci_setup_queue_10(struct virtio_softc *sc, uint16_t idx, uint64_t addr)
@@ -747,15 +759,15 @@ virtio_pci_setup_queue_10(struct virtio_
bus_space_write_2(iot, ioh, VIRTIO_CONFIG1_QUEUE_SELECT, vq->vq_index);
if (addr == 0) {
bus_space_write_2(iot, ioh, VIRTIO_CONFIG1_QUEUE_ENABLE, 0);
- bus_space_write_le_8(iot, ioh, VIRTIO_CONFIG1_QUEUE_DESC, 0);
- bus_space_write_le_8(iot, ioh, VIRTIO_CONFIG1_QUEUE_AVAIL, 0);
- bus_space_write_le_8(iot, ioh, VIRTIO_CONFIG1_QUEUE_USED, 0);
+ bus_space_write_8(iot, ioh, VIRTIO_CONFIG1_QUEUE_DESC, 0);
+ bus_space_write_8(iot, ioh, VIRTIO_CONFIG1_QUEUE_AVAIL, 0);
+ bus_space_write_8(iot, ioh, VIRTIO_CONFIG1_QUEUE_USED, 0);
} else {
- bus_space_write_le_8(iot, ioh,
+ bus_space_write_8(iot, ioh,
VIRTIO_CONFIG1_QUEUE_DESC, addr);
- bus_space_write_le_8(iot, ioh,
+ bus_space_write_8(iot, ioh,
VIRTIO_CONFIG1_QUEUE_AVAIL, addr + vq->vq_availoffset);
- bus_space_write_le_8(iot, ioh,
+ bus_space_write_8(iot, ioh,
VIRTIO_CONFIG1_QUEUE_USED, addr + vq->vq_usedoffset);
bus_space_write_2(iot, ioh,
VIRTIO_CONFIG1_QUEUE_ENABLE, 1);
@@ -771,7 +783,6 @@ virtio_pci_setup_queue_10(struct virtio_
VIRTIO_CONFIG1_QUEUE_MSIX_VECTOR, vec);
}
}
-#undef bus_space_write_le_8
static void
virtio_pci_set_status_10(struct virtio_softc *sc, int status)
Re: CVS commit: src/sys/dev
> On Jan 21, 2021, at 12:00 AM, Martin Husemann > wrote: > > On Wed, Jan 20, 2021 at 07:46:48PM +, Reinoud Zandijk wrote: >> Module Name: src >> Committed By:reinoud >> Date:Wed Jan 20 19:46:48 UTC 2021 >> > [..] >> Log Message: >> Add VirtIO PCI v1.0 attachments and fix the drivers affected. >> >> * virtio on sparc64 attaches but is it not functioning though not a >> regression. > > While not a regression by this commit, it *did* work in netbsd-8, > so overall a bad regression that we should fix. This could be a bug in Qemu … it does not work on Alpha, either, and Jonathan Kollasch tracked down to Qemu 5’s Virtio subsystem not considering the DMA window on the Alpha platform. -- thorpej
Re: CVS commit: src/sys/dev
On Wed, Jan 20, 2021 at 07:46:48PM +, Reinoud Zandijk wrote: > Module Name: src > Committed By: reinoud > Date: Wed Jan 20 19:46:48 UTC 2021 > [..] > Log Message: > Add VirtIO PCI v1.0 attachments and fix the drivers affected. > > * virtio on sparc64 attaches but is it not functioning though not a > regression. While not a regression by this commit, it *did* work in netbsd-8, so overall a bad regression that we should fix. Martin
