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/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_mutex, MUTEX_DEFAULT, IPL_NET);
+ mutex_init(>sc_mutex, MUTEX_DEFAULT, IPL_SOFTCLOCK);
mutex_init(>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_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_mutex, MUTEX_DEFAULT, IPL_NET);
+ mutex_init(>sc_mutex, MUTEX_DEFAULT, IPL_SOFTCLOCK);
mutex_init(>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/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/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/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/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/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/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[] = { , , , };
- int i, j = 0, ret = 0;
+ int i, j, ret = 0;
if (virtio_pci_find_cap(psc, VIRTIO_PCI_CAP_COMMON_CFG,
, 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/pci/ixgbe
On 2020/12/11 14:01, SAITOH Masanobu wrote: Module Name:src Committed By: msaitoh Date: Fri Dec 11 05:01:19 UTC 2020 Modified Files: src/sys/dev/pci/ixgbe: ixgbe.c ixgbe_type.h Log Message: Don't use EIMC_OTHER bit because it's read only other than 82598. Documents say: 82598: All of bit 31(OTHER bit) of EIxx are reserved. In reality, at least EIMS_OTHER and EIMC_OTHER exist and the OTHER interrupt doesn't work without EIMS_OTHER. Other than 82598: + EICR's bit 31 is defined and other EIXX's bit 31 are reserved. + In reality, EIMS_OTHER is read only and EIMC_OTHER doesn't exist. If one of bit 29..16 is set, EIMS_OTHER is set to 1 (Note that bit 30(TCP timer isn't included)). Even if write bit 31 of EIMC to 1, it's ignored (EIMS_OTHER doesn't set). We introduced new spin mutex in ixgbe.c rev. 1.260, so it's OK to remove EIMC_OTHER stuff. We already set EIMS_OTHER in if_init(), so keep it for 82598. No functional change other than 82598. Another solution is to control bit 30..16 directly to mask/unmask interrupt instead of the mutex. TODO: Some MSI-X interrupt(LSC, module insertion/removal etc.)'s mask/unmask code between ixgbe_msix_admin() and ixgbe_handle_admin() may be wrong. It'll be fixed later. To generate a diff of this commit: cvs rdiff -u -r1.261 -r1.262 src/sys/dev/pci/ixgbe/ixgbe.c cvs rdiff -u -r1.45 -r1.46 src/sys/dev/pci/ixgbe/ixgbe_type.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 2020/10/16 14:53, SAITOH Masanobu wrote: Module Name:src Committed By: msaitoh Date: Fri Oct 16 05:53:40 UTC 2020 Modified Files: src/sys/dev/pci: if_wm.c Log Message: Fixes a problem that the attach function reported "wm_gmii_setup_phytype: Unknown PHY model. OUI=00, model=" and "PHY type is still unknown." This was dmesg only problem. The SGMII read/write functions were correctly set even though error message was printed. This problem was added in if_wm.c rev. 1.656 which added SFP support. Don't call wm_gmii_setup_phytype() three times if the interface uses SGMII with internal MDIO. Tested with I354(Rangeley(SGMII(MDIO))) and I350(SERDES(SFP), SGMII(SFP)). To generate a diff of this commit: cvs rdiff -u -r1.690 -r1.691 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. -- --- SAITOH Masanobu (msai...@execsw.org msai...@netbsd.org)
Re: CVS commit: src/sys/dev/pci
Hello, On Sun, 11 Oct 2020 21:41:57 + "Julian Coleman" wrote: > Module Name: src > Committed By: jdc > Date: Sun Oct 11 21:41:57 UTC 2020 > > Modified Files: > src/sys/dev/pci: radeonfb.c > > Log Message: ... > don't ignore the bottom 200 lines of the display (for no apparent reason)) The reason I have that in most of my drivers is so I can see a good part of the glyph cache, which starts right below the VRAM area used by wsdisplay. Most drivers use it only for anti-aliased fonts so you probably didn't see anything there... have fun Michael
Re: CVS commit: src/sys/dev/pci
One of the things which need to be done is calling the if_ioctl always with the IFNET_LOCK() held. Right now it sometimes is, and other times it is not, so it's not possible to rely on it and KASSERT(). As for bnx(4) I did just some basic fixes around making it work with MSI(-X), since I don't really have easy access to the hw for testing. This might change soon, then I might look into making it NET_MPSAFE, after some other bug fixes. Jaromir Le sam. 18 juil. 2020 à 00:54, Jason Thorpe a écrit : > > > > > On Jul 17, 2020, at 3:50 PM, matthew green wrote: > > > > any chance you can look at NET_MPSAFE here etc? :) > > I have a bunch of local changes for this in one of my trees, and I hope to > get back to it after netbsd-10 branches. > > -- thorpej >
Re: CVS commit: src/sys/dev/pci
> On Jul 17, 2020, at 3:50 PM, matthew green wrote: > > any chance you can look at NET_MPSAFE here etc? :) I have a bunch of local changes for this in one of my trees, and I hope to get back to it after netbsd-10 branches. -- thorpej
re: CVS commit: src/sys/dev/pci
"Jaromir Dolecek" writes: > Module Name: src > Committed By: jdolecek > Date: Fri Jul 17 09:48:21 UTC 2020 > > Modified Files: > src/sys/dev/pci: if_bnx.c > > Log Message: > re-enable MSI/MSI-X, the TX timeouts were caused by the IFF_OACTIVE handling, > which was fixed in previous revision "fixed IFF_OACTIVE" in modern netbsd means "removed IFF_OACTIVE and handled it in the driver", as the flag is not SMP friendly. any chance you can look at NET_MPSAFE here etc? :) thanks. .mrg.
Re: [virtio] Re: CVS commit: src/sys/dev/pci
Le 01/06/2020 à 03:23, Shoichi Yamaguchi a écrit : Hi, On Wed, May 27, 2020 at 8:47 PM Shoichi Yamaguchi wrote: I modified virtio(4) not to allocate unused memory. I guess it fixes the issue. Could you check this? I confirmed your closing the report on syzbot. https://syzkaller.appspot.com/bug?id=73f690e8a3d8e304407808555f1dacfa004685d1 Thank you for your response. Sorry for my lack of response -- I was waiting for the kMSan instance to sync, but it is currently down, and has been down for four days and a half now. The kMSan instance got the time to run 24h before it broke for unrelated reasons. 24h before your patch, it triggered the bug 6 times. 24h after your patch, it triggered the bug 0 times. So indeed, we can call it fixed, thanks for the fix
Re: [virtio] Re: CVS commit: src/sys/dev/pci
Hi, On Wed, May 27, 2020 at 8:47 PM Shoichi Yamaguchi wrote: > > I modified virtio(4) not to allocate unused memory. > I guess it fixes the issue. > > Could you check this? I confirmed your closing the report on syzbot. https://syzkaller.appspot.com/bug?id=73f690e8a3d8e304407808555f1dacfa004685d1 Thank you for your response. Regards, yamaguchi
Re: [virtio] Re: CVS commit: src/sys/dev/pci
Hi, On Wed, May 27, 2020 at 2:20 AM Maxime Villard wrote: > > Hi, > I don't know if this is related to your changes, but kMSan detected one uninit > variable in virtio 3h ago: > > https://syzkaller.appspot.com/text?tag=CrashReport=12084ef610 > > [ 153.4370851] panic: MSan: Uninitialized Kmem Memory From > virtio_pci_setup_interrupts() > [ 153.4448669] cpu0: Begin traceback... > [ 153.4448669] vpanic() at netbsd:vpanic+0x7c1 sys/kern/subr_prf.c:288 > [ 153.4632004] panic() at netbsd:panic+0x1ad sys/kern/subr_prf.c:209 > [ 153.4734357] __msan_warning() at netbsd:__msan_warning+0xe7 > kmsan_report_inline sys/kern/subr_msan.c:239 [inline] > [ 153.4734357] __msan_warning() at netbsd:__msan_warning+0xe7 > sys/kern/subr_msan.c:612 > [ 153.4931985] virtio_pci_free_interrupts() at > netbsd:virtio_pci_free_interrupts+0x1b4 sys/dev/pci/virtio_pci.c:740 > [ 153.5132006] virtio_child_detach() at > netbsd:virtio_child_detach+0x116 sys/dev/pci/virtio.c:924 > [ 153.5331982] vioscsi_detach() at netbsd:vioscsi_detach+0x40d > sys/dev/pci/vioscsi.c:244 > [ 153.5532009] config_detach() at netbsd:config_detach+0x7e3 > sys/kern/subr_autoconf.c:1760 > [ 153.5732017] config_detach_all() at netbsd:config_detach_all+0x29a > sys/kern/subr_autoconf.c:1906 > [ 153.5831984] cpu_reboot() at netbsd:cpu_reboot+0x290 > sys/arch/amd64/amd64/machdep.c:700 > [ 153.6031986] kern_reboot() at netbsd:kern_reboot+0x18f > sys/kern/kern_reboot.c:73 > [ 153.6231980] sys_reboot() at netbsd:sys_reboot+0x28d > > This means that some memory allocated by virtio_pci_setup_interrupts() on > the kmem allocator was not initialized, and later one access to it was made > by virtio_pci_free_interrupts() at l.740 of the file. Thank you for your pointed out. I modified virtio(4) not to allocate unused memory. I guess it fixes the issue. Could you check this? Thanks, yamaguchi
[virtio] Re: CVS commit: src/sys/dev/pci
Hi, I don't know if this is related to your changes, but kMSan detected one uninit variable in virtio 3h ago: https://syzkaller.appspot.com/text?tag=CrashReport=12084ef610 [ 153.4370851] panic: MSan: Uninitialized Kmem Memory From virtio_pci_setup_interrupts() [ 153.4448669] cpu0: Begin traceback... [ 153.4448669] vpanic() at netbsd:vpanic+0x7c1 sys/kern/subr_prf.c:288 [ 153.4632004] panic() at netbsd:panic+0x1ad sys/kern/subr_prf.c:209 [ 153.4734357] __msan_warning() at netbsd:__msan_warning+0xe7 kmsan_report_inline sys/kern/subr_msan.c:239 [inline] [ 153.4734357] __msan_warning() at netbsd:__msan_warning+0xe7 sys/kern/subr_msan.c:612 [ 153.4931985] virtio_pci_free_interrupts() at netbsd:virtio_pci_free_interrupts+0x1b4 sys/dev/pci/virtio_pci.c:740 [ 153.5132006] virtio_child_detach() at netbsd:virtio_child_detach+0x116 sys/dev/pci/virtio.c:924 [ 153.5331982] vioscsi_detach() at netbsd:vioscsi_detach+0x40d sys/dev/pci/vioscsi.c:244 [ 153.5532009] config_detach() at netbsd:config_detach+0x7e3 sys/kern/subr_autoconf.c:1760 [ 153.5732017] config_detach_all() at netbsd:config_detach_all+0x29a sys/kern/subr_autoconf.c:1906 [ 153.5831984] cpu_reboot() at netbsd:cpu_reboot+0x290 sys/arch/amd64/amd64/machdep.c:700 [ 153.6031986] kern_reboot() at netbsd:kern_reboot+0x18f sys/kern/kern_reboot.c:73 [ 153.6231980] sys_reboot() at netbsd:sys_reboot+0x28d This means that some memory allocated by virtio_pci_setup_interrupts() on the kmem allocator was not initialized, and later one access to it was made by virtio_pci_free_interrupts() at l.740 of the file. Can you have a look? Thanks, Maxime
Re: CVS commit: src/sys/dev/pci
On 20/05/2020 21:18, Sevan Janiyan wrote: > Bump rcs tag which was missed in r1.9 That should've been r1.10 Sevan
Re: CVS commit: src/sys/dev/pci
On 2020/01/10 15:59, Jason Thorpe wrote: > > >> On Jan 9, 2020, at 9:02 PM, Masanobu SAITOH wrote: >> >> The reason why I moved stge_softc to if_stgereg.h was that ipgphy.c >> required to check stge's chip revision. So, if there is no any objection, >> I'll make new if_stgevar.h and share it with if_stge.c and ipgphy.c. > > That seems fine. Although it might be preferable to set a property on the > parent dev_t and then query that from ipgphy, I've changed by this way. If it use if_stgevar.h and it includes pci related header file, some archs' kernel which use ipgphy and not use PCI can't be compiled. Some other MII PHY drivers do so. Thanks. > rather than accessing the softc. > > -- thorpej > -- --- SAITOH Masanobu (msai...@execsw.org msai...@netbsd.org)
Re: CVS commit: src/sys/dev/pci
> On Jan 9, 2020, at 9:02 PM, Masanobu SAITOH wrote: > > The reason why I moved stge_softc to if_stgereg.h was that ipgphy.c > required to check stge's chip revision. So, if there is no any objection, > I'll make new if_stgevar.h and share it with if_stge.c and ipgphy.c. That seems fine. Although it might be preferable to set a property on the parent dev_t and then query that from ipgphy, rather than accessing the softc. -- thorpej
Re: CVS commit: src/sys/dev/pci
On 2020/01/10 13:13, Jason Thorpe wrote: > > >> On Jan 9, 2020, at 6:11 PM, Masanobu SAITOH wrote: >> >> >> Before my change, struct stge_softc is already in if_stgereg.h, >> so I had thought it would be OK to move to it. > > Ah, I don't think I would have put it there when I wrote the driver > originally, so it must have been moved there at some point. Oh, it was me :) http://mail-index.netbsd.org/source-changes/2019/10/07/msg109768.html > In any case, moving it into if_stgereg.h was also an error. > >> >>> Please move them back to if_stge.c. Doing incorrect things simply to >>> reduce the diff against someone else's copy of the code is not something we >>> should be undertaking. >> Two options: >> >> a) Move some structs (including struct stge_softc) and defines >>to if_stge.c > > There is no reason to have if_stgevar.h, because no other modules need these > definitions, so it should all move to if_stge.c. The reason why I moved stge_softc to if_stgereg.h was that ipgphy.c required to check stge's chip revision. So, if there is no any objection, I'll make new if_stgevar.h and share it with if_stge.c and ipgphy.c. > Thanks! > >> >> b) Move them to new if_stgevar.h >> >> Which one is prefer? > > -- thorpej > -- --- SAITOH Masanobu (msai...@execsw.org msai...@netbsd.org)
Re: CVS commit: src/sys/dev/pci
> On Jan 9, 2020, at 6:11 PM, Masanobu SAITOH wrote: > > > Before my change, struct stge_softc is already in if_stgereg.h, > so I had thought it would be OK to move to it. Ah, I don't think I would have put it there when I wrote the driver originally, so it must have been moved there at some point. In any case, moving it into if_stgereg.h was also an error. > >> Please move them back to if_stge.c. Doing incorrect things simply to reduce >> the diff against someone else's copy of the code is not something we should >> be undertaking. > Two options: > > a) Move some structs (including struct stge_softc) and defines >to if_stge.c There is no reason to have if_stgevar.h, because no other modules need these definitions, so it should all move to if_stge.c. Thanks! > > b) Move them to new if_stgevar.h > > Which one is prefer? -- thorpej
re: CVS commit: src/sys/dev/pci
> Two options: > > a) Move some structs (including struct stge_softc) and defines > to if_stge.c > > b) Move them to new if_stgevar.h i've always preferred: - fooreg.h and foovar.h exist only when more than 1 file use them - fooreg.h ONLY has hardware constructs - foovar.h ONLY has software constructs though i tend to only delete single-use foovar.h, not fooreg.h. .mrg.
Re: CVS commit: src/sys/dev/pci
On 2020/01/09 23:08, Jason Thorpe wrote:
>
>
>> On Jan 9, 2020, at 2:54 AM, SAITOH Masanobu wrote:
>>
>> Module Name: src
>> Committed By:msaitoh
>> Date:Thu Jan 9 10:54:16 UTC 2020
>>
>> Modified Files:
>> src/sys/dev/pci: if_stge.c if_stgereg.h
>>
>> Log Message:
>> Reduce diff against OpenBSD. No functional change.
>>
>> - USE CSR_{READ,WRITE}_*() macro.
>> - Move some macros from if_stge.c to if_stgereg.h
>
> This diff is incorrect.
Yes. You're right.
> The macros that were moved to if_stgereg.h are not related to hardware /
> register definitions, but are purely software constructs.
Before my change, struct stge_softc is already in if_stgereg.h,
so I had thought it would be OK to move to it.
> Please move them back to if_stge.c. Doing incorrect things simply to reduce
> the diff against someone else's copy of the code is not something we should
> be undertaking.
Two options:
a) Move some structs (including struct stge_softc) and defines
to if_stge.c
b) Move them to new if_stgevar.h
Which one is prefer?
> -- thorpej
>
--
---
SAITOH Masanobu (msai...@execsw.org
msai...@netbsd.org)
Re: CVS commit: src/sys/dev/pci
> On Jan 9, 2020, at 2:54 AM, SAITOH Masanobu wrote:
>
> Module Name: src
> Committed By: msaitoh
> Date: Thu Jan 9 10:54:16 UTC 2020
>
> Modified Files:
> src/sys/dev/pci: if_stge.c if_stgereg.h
>
> Log Message:
> Reduce diff against OpenBSD. No functional change.
>
> - USE CSR_{READ,WRITE}_*() macro.
> - Move some macros from if_stge.c to if_stgereg.h
This diff is incorrect. The macros that were moved to if_stgereg.h are not
related to hardware / register definitions, but are purely software constructs.
Please move them back to if_stge.c. Doing incorrect things simply to reduce
the diff against someone else's copy of the code is not something we should be
undertaking.
-- thorpej
Re: CVS commit: src/sys/dev/pci
It only prevent null pointer dereference. On Mon, Nov 18, 2019 at 3:18 PM wrote: > > On Mon, Nov 18, 2019 at 06:15:27AM +, m...@netbsd.org wrote: > > > Modified files: > > > > > > Index: src/sys/dev/pci/if_mcx.c > > > diff -u src/sys/dev/pci/if_mcx.c:1.5 src/sys/dev/pci/if_mcx.c:1.6 > > > --- src/sys/dev/pci/if_mcx.c:1.5Thu Oct 17 15:57:56 2019 > > > +++ src/sys/dev/pci/if_mcx.cMon Nov 18 04:40:05 2019 > > > @@ -1,4 +1,4 @@ > > > -/* $NetBSD: if_mcx.c,v 1.5 2019/10/17 15:57:56 msaitoh Exp $ */ > > > +/* $NetBSD: if_mcx.c,v 1.6 2019/11/18 04:40:05 nonaka Exp $ */ > > > /* $OpenBSD: if_mcx.c,v 1.33 2019/09/12 04:23:59 jmatthew Exp $ */ > > > > > > /* > > > @@ -6347,7 +6347,7 @@ mcx_load_mbuf(struct mcx_softc *sc, stru > > > break; > > > > > > case EFBIG: > > > - if (m_defrag(m, M_DONTWAIT) == 0 && > > > + if (m_defrag(m, M_DONTWAIT) != NULL && > > > bus_dmamap_load_mbuf(sc->sc_dmat, ms->ms_map, m, > > > BUS_DMA_STREAMING | BUS_DMA_NOWAIT) == 0) > > > break; > > > > > > > Is this one of those "m_defrag misbehaves because it will not turn it > > into a chain of 1 packet, but 2"? > > > > (I think this will not work) > > Additional context: > http://mail-index.netbsd.org/tech-net/2018/09/01/msg007031.html
Re: CVS commit: src/sys/dev/pci
> Modified files: > > Index: src/sys/dev/pci/if_mcx.c > diff -u src/sys/dev/pci/if_mcx.c:1.5 src/sys/dev/pci/if_mcx.c:1.6 > --- src/sys/dev/pci/if_mcx.c:1.5 Thu Oct 17 15:57:56 2019 > +++ src/sys/dev/pci/if_mcx.c Mon Nov 18 04:40:05 2019 > @@ -1,4 +1,4 @@ > -/* $NetBSD: if_mcx.c,v 1.5 2019/10/17 15:57:56 msaitoh Exp $ */ > +/* $NetBSD: if_mcx.c,v 1.6 2019/11/18 04:40:05 nonaka Exp $ */ > /* $OpenBSD: if_mcx.c,v 1.33 2019/09/12 04:23:59 jmatthew Exp $ */ > > /* > @@ -6347,7 +6347,7 @@ mcx_load_mbuf(struct mcx_softc *sc, stru > break; > > case EFBIG: > - if (m_defrag(m, M_DONTWAIT) == 0 && > + if (m_defrag(m, M_DONTWAIT) != NULL && > bus_dmamap_load_mbuf(sc->sc_dmat, ms->ms_map, m, > BUS_DMA_STREAMING | BUS_DMA_NOWAIT) == 0) > break; > Is this one of those "m_defrag misbehaves because it will not turn it into a chain of 1 packet, but 2"? (I think this will not work)
Re: CVS commit: src/sys/dev/pci
Tobias Nygren writes: > Yep, MSI works now. According to the PCI Express Base Specification, > Revision 3.0, table 7-3, having the bus master bit set to 0 disables > MSI. Asmedia chip seems to have a bug which makes DMA work regardless ... Yup - it works just fine! Well done! :) ahcisata0: interrupting at irq 8192 (MSI vec 0) -tih -- Most people who graduate with CS degrees don't understand the significance of Lisp. Lisp is the most important idea in computer science. --Alan Kay
Re: CVS commit: src/sys/dev/pci
On Fri, 18 Oct 2019 20:32:53 +0200 Tom Ivar Helbekkmo wrote: > Tobias Nygren writes: > > > Module Name:src > > Committed By: tnn > > Date: Fri Oct 18 17:16:50 UTC 2019 > > > > Modified Files: > > src/sys/dev/pci: ahcisata_pci.c > > > > Log Message: > > ahcisata: make sure bus mastering and memory space are actually enabled > > > > This makes the "ROCKPro64 PCI-e to Dual SATA-II Interface Card" work. > > Um. I've been using that interface card for a while, on my Rockpro64, > which is, in fact, running with root on a raidframe mirror of two SATA > disks connected to it. I did have to make a small change to make it > work, though: I had to force it to not use MSI(X) interrupts. Do you > think I can revert that local modification with your change? Yep, MSI works now. According to the PCI Express Base Specification, Revision 3.0, table 7-3, having the bus master bit set to 0 disables MSI. Asmedia chip seems to have a bug which makes DMA work regardless ... -Tobias
Re: CVS commit: src/sys/dev/pci
Tobias Nygren writes: > Module Name: src > Committed By: tnn > Date: Fri Oct 18 17:16:50 UTC 2019 > > Modified Files: > src/sys/dev/pci: ahcisata_pci.c > > Log Message: > ahcisata: make sure bus mastering and memory space are actually enabled > > This makes the "ROCKPro64 PCI-e to Dual SATA-II Interface Card" work. Um. I've been using that interface card for a while, on my Rockpro64, which is, in fact, running with root on a raidframe mirror of two SATA disks connected to it. I did have to make a small change to make it work, though: I had to force it to not use MSI(X) interrupts. Do you think I can revert that local modification with your change? ahcisata0 at pci1 dev 0 function 0: vendor 1b21 product 0612 (rev. 0x01) ahcisata0: 64-bit DMA unavailable ahcisata0: AHCI revision 1.20, 2 ports, 32 slots, CAP 0xeb32ffa1 ahcisata0: interrupting at GICv3 irq 82 atabus0 at ahcisata0 channel 0 atabus1 at ahcisata0 channel 1 wd0 at atabus0 drive 0 wd0: wd0: drive supports 16-sector PIO transfers, LBA48 addressing wd0: 931 GB, 1938021 cyl, 16 head, 63 sec, 512 bytes/sect x 1953525168 sectors wd0: drive supports PIO mode 4, DMA mode 2, Ultra-DMA mode 6 (Ultra/133), NCQ (32 tags) w/PRIO wd0(ahcisata0:0:0): using PIO mode 4, DMA mode 2, Ultra-DMA mode 6 (Ultra/133) (using DMA), NCQ (31 tags) w/PRIO wd1 at atabus1 drive 0 wd1: wd1: drive supports 16-sector PIO transfers, LBA48 addressing wd1: 931 GB, 1938021 cyl, 16 head, 63 sec, 512 bytes/sect x 1953525168 sectors wd1: drive supports PIO mode 4, DMA mode 2, Ultra-DMA mode 6 (Ultra/133), NCQ (32 tags) w/PRIO wd1(ahcisata0:1:0): using PIO mode 4, DMA mode 2, Ultra-DMA mode 6 (Ultra/133) (using DMA), NCQ (31 tags) w/PRIO raid0: RAID Level 1 raid0: Components: /dev/wd0a /dev/wd1a raid0: Total Sectors: 1936551168 (945581 MB) root on raid0a dumps on wd1b root file system type: ffs -tih -- Most people who graduate with CS degrees don't understand the significance of Lisp. Lisp is the most important idea in computer science. --Alan Kay
Re: CVS commit: src/sys/dev/pci/ixgbe
On 2019/06/27 18:56, SAITOH Masanobu wrote: > Module Name: src > Committed By: msaitoh > Date: Thu Jun 27 09:56:39 UTC 2019 > > Modified Files: > src/sys/dev/pci/ixgbe: ixv.c > > Log Message: > Don't call set_vfta() if any VLAN is attached. s/is/is not/ > XXX pullup-8. > > > To generate a diff of this commit: > cvs rdiff -u -r1.115 -r1.116 src/sys/dev/pci/ixgbe/ixv.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
Hi, Has there been any progress on this since 2017-12-03? I've noticed that ips(4) is missing the man-page, only has a single revision on ips.c, and is not connected to the build (neither to ALL nor GENERIC). Is it intentional for this driver to remain in tree? (Does it even still compile?) C. On 2017-W48-7 14:26 +, Jaromir Dolecek wrote: Module Name:src Committed By: jdolecek Date: Sun Dec 3 14:26:38 UTC 2017 Modified Files: src/sys/dev/pci: files.pci Added Files: src/sys/dev/pci: ips.c Log Message: port ips(4) driver from OpenBSD; needs a lot more work, right now just compilable To generate a diff of this commit: cvs rdiff -u -r1.391 -r1.392 src/sys/dev/pci/files.pci cvs rdiff -u -r0 -r1.1 src/sys/dev/pci/ips.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
Hello,
On Fri, 22 Mar 2019 17:07:10 +
m...@netbsd.org wrote:
> On Fri, Mar 22, 2019 at 07:41:41AM +, Martin Husemann wrote:
> > @@ -1402,6 +1402,9 @@ radeonfb_loadbios(struct radeonfb_softc
> > pci_find_rom(pa, romt, romh, romsz, PCI_ROM_CODE_TYPE_X86, ,
> > >sc_biossz);
> >
> > + if (sc->sc_biossz == 0 || sc->sc_bios == NULL)
> > + return;
> > +
> > foundit:
> > if (sc->sc_biossz > 0) {
> ^^^ Now the else branch isn't possible?
...and we also need to re-enable video DMA and output, so this is
wrong, and I committed a fix earlier anyway.
have fun
Michael
Re: CVS commit: src/sys/dev/pci
On Fri, Mar 22, 2019 at 07:41:41AM +, Martin Husemann wrote:
> @@ -1402,6 +1402,9 @@ radeonfb_loadbios(struct radeonfb_softc
> pci_find_rom(pa, romt, romh, romsz, PCI_ROM_CODE_TYPE_X86, ,
> >sc_biossz);
>
> + if (sc->sc_biossz == 0 || sc->sc_bios == NULL)
> + return;
> +
> foundit:
> if (sc->sc_biossz > 0) {
^^^ Now the else branch isn't possible?
re: CVS commit: src/sys/dev/pci
"Michael Lorenz" writes:
> Module Name: src
> Committed By: macallan
> Date: Wed Mar 20 23:05:19 UTC 2019
>
> Modified Files:
> src/sys/dev/pci: radeonfb.c
>
> Log Message:
> add code to read disabled ROMs, adapted from xf86-video-radeon
> With this radeonfb does The Right Thing(tm) on my 2xDVI mac radeon with
> decidedly non-standard output wiring.
> ( apparently at least *some* mac radeons have a hidden x86 BIOS with valid
> connector tables )
this seems to potentially malloc(0). if the disabled rom is
not found, here:
if (sc->sc_biossz != 0) printf("found disabled BIOS\n");
foundit:
sc->sc_bios = malloc(sc->sc_biossz, M_DEVBUF, M_WAITOK);
i think the original code that does an early return needs to
reappear around here, and the added conditional at the end
here can go away.
thanks.
.mrg.
Re: CVS commit: src/sys/dev/pci/ixgbe
On 2019/03/13 19:02, SAITOH Masanobu wrote: Module Name:src Committed By: msaitoh Date: Wed Mar 13 10:02:13 UTC 2019 Modified Files: src/sys/dev/pci/ixgbe: ixgbe.c Log Message: - Fix a bug that the VLAN HW filter function is not correctly disabled s/VLAN HW filter/VLAN HW tagging/ when all vlan is detached. - Fix a bug that VLAN HW filter function is not correctly controlled on 82598. s/VLAN HW filter/VLAN HW tagging/ - Control VLAN HW filter function correctly. - Don't clear IXGBE_VLNCTRL_CFIEN bit When ETHERCAP_VLAN_HWFILTER is set. I think it's not required (and Linux doesn't do it). This change has no effect to NetBSD because ETHERCAP_VLAN_HWFILTER is not supported yet. To generate a diff of this commit: cvs rdiff -u -r1.176 -r1.177 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
On 2019/02/07 13:03, SAITOH Masanobu wrote: Module Name:src Committed By: msaitoh Date: Thu Feb 7 04:03:25 UTC 2019 Modified Files: src/sys/dev/pci: if_wm.c Log Message: Fix a bug that WOL didn't work on some chips since if_wm.c rev. 1.60. s/1.60/1.610/ Set WUC_APME bit older than PCH. Will fixes PR kern/53945 reported by kardel@. Tested with my own 82574 card. To generate a diff of this commit: cvs rdiff -u -r1.624 -r1.625 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. -- --- SAITOH Masanobu (msai...@execsw.org msai...@netbsd.org)
Re: CVS commit: src/sys/dev/pci
On Tue, Jan 29, 2019 at 04:16:30PM +0900, Masanobu SAITOH wrote: > On 2019/01/29 2:51, Christoph Badura wrote: > >Wouldn't it be better to prevent this kind of bug in the future by putting > >the decision which method to use into a switch-statement and have the > >compiler worry about ordering? (And duplicates and omissions.) > > I think so and I'm going to simplify such type of checks (I have unfinished > code > in locally). The reason why I committed the above change only is to make > the change understandable by separating from other changes. I wasn't aware that you were planning to do that. Excellent! Thank you for doing that. --chris
Re: CVS commit: src/sys/dev/pci
On 2019/01/28 15:33, Jason Thorpe wrote: On Jan 28, 2019, at 8:11 AM, Masanobu SAITOH wrote: On 2019/01/28 15:07, Jason Thorpe wrote: Doesn’t it seem a little dangerous to just blindly enable? You mean it should be enable only when the secondary bridge is configured correctly? Both FreeBSD and OpenBSD blindly enabled it. One of the difference between NetBSD and others is that they configure unconfigured bridge. Right, if the address decoders aren't programmed properly, it seems like you could get into all sorts of trouble. done: Module Name:src Committed By: msaitoh Date: Tue Jan 29 09:25:52 UTC 2019 Modified Files: src/sys/dev/pci: ppb.c Log Message: If the secondary bus is configured and the bus mastering is not enabled, enable it. Suggested by thorpej@. I really feel like the correct way to solve the problem is to fully configure the bus using information from ACPI. For ACPI, I'm not familiar with ACPI's PCI stuff, so please someone... Even if it's implemented, some BIOS/UEFI don't configure add-in PCI bridge behind bridge... And also please someone to write hotplug code (especially for hot insertion)... -- thorpej -- --- SAITOH Masanobu (msai...@execsw.org msai...@netbsd.org)
Re: CVS commit: src/sys/dev/pci
On 2019/01/29 2:51, Christoph Badura wrote: On Fri, Jan 25, 2019 at 08:04:07AM +, SAITOH Masanobu wrote: Modified Files: src/sys/dev/pci: if_wm.c Log Message: 80003's SERDES is not the same as 82575's but the same as legacy devices. Use the old methods on 80003. XXX The reason why this bug existed is that our order of WM_T_* was little different from FreeBSD's enum e1000_mac_type. From 80003 to PCH_CNP and from 82575 to I211 are swapped. Wouldn't it be better to prevent this kind of bug in the future by putting the decision which method to use into a switch-statement and have the compiler worry about ordering? (And duplicates and omissions.) I think so and I'm going to simplify such type of checks (I have unfinished code in locally). The reason why I committed the above change only is to make the change understandable by separating from other changes. --chris Thanks. -- --- SAITOH Masanobu (msai...@execsw.org msai...@netbsd.org)
Re: CVS commit: src/sys/dev/pci
On Fri, Jan 25, 2019 at 08:04:07AM +, SAITOH Masanobu wrote: > Modified Files: > src/sys/dev/pci: if_wm.c > > Log Message: > 80003's SERDES is not the same as 82575's but the same as legacy devices. > Use the old methods on 80003. > > XXX The reason why this bug existed is that our order of WM_T_* was little > different from FreeBSD's enum e1000_mac_type. From 80003 to PCH_CNP and from > 82575 to I211 are swapped. Wouldn't it be better to prevent this kind of bug in the future by putting the decision which method to use into a switch-statement and have the compiler worry about ordering? (And duplicates and omissions.) --chris
Re: CVS commit: src/sys/dev/pci
Hi! Great, that did it - no more immediately visible device deficiencies. com* work wm1 works radeon still spits errors [10.941427] kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! [12.002002] kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! graphics seem to work though with little flickering in glx windows. So except for the radeon startup (and possible minor flickering in glx windows) my system seems to work now when booted via UEFI. Maybe someone with more insight in the radeon reset code can give hints what could be missing. So the system looks usable now just leaving the radeon UVD reset issue as final topic. Thanks! Frank The left-over pci setup differences are: --- lspci-csm-201901272019-01-27 14:42:57.454117956 +0100 +++ lspci-uefi-20190127.12019-01-28 07:10:47.857701110 +0100 @@ -4,7 +4,7 @@ 00:00.2 IOMMU: Advanced Micro Devices, Inc. [AMD] Device 1451 Subsystem: Advanced Micro Devices, Inc. [AMD] Device 1451 -Flags: bus master, fast devsel, latency 0, IRQ 255 +Flags: fast devsel, IRQ 255 Capabilities: [40] Secure device Capabilities: [64] MSI: Enable- Count=1/4 Maskable- 64bit+ Capabilities: [74] HyperTransport: MSI Mapping Enable+ Fixed+ @@ -339,7 +339,7 @@ 21:00.0 Serial controller: MosChip Semiconductor Technology Ltd. 4-Port PCIe Serial Adapter (prog-if 02 [16550]) Subsystem: Device a000:1000 -Flags: bus master, fast devsel, latency 0, IRQ 11 +Flags: fast devsel, IRQ 11 I/O ports at b030 Memory at fcc07000 (32-bit, non-prefetchable) Memory at fcc06000 (32-bit, non-prefetchable) @@ -351,7 +351,7 @@ 21:00.1 Serial controller: MosChip Semiconductor Technology Ltd. 4-Port PCIe Serial Adapter (prog-if 02 [16550]) Subsystem: Device a000:1000 -Flags: bus master, fast devsel, latency 0, IRQ 11 +Flags: fast devsel, IRQ 11 I/O ports at b020 Memory at fcc05000 (32-bit, non-prefetchable) Memory at fcc04000 (32-bit, non-prefetchable) @@ -362,7 +362,7 @@ 21:00.2 Serial controller: MosChip Semiconductor Technology Ltd. 4-Port PCIe Serial Adapter (prog-if 02 [16550]) Subsystem: Device a000:1000 -Flags: bus master, fast devsel, latency 0, IRQ 10 +Flags: fast devsel, IRQ 10 I/O ports at b010 Memory at fcc03000 (32-bit, non-prefetchable) Memory at fcc02000 (32-bit, non-prefetchable) @@ -373,7 +373,7 @@ 21:00.3 Serial controller: MosChip Semiconductor Technology Ltd. 4-Port PCIe Serial Adapter (prog-if 02 [16550]) Subsystem: Device a000:1000 -Flags: bus master, fast devsel, latency 0, IRQ 5 +Flags: fast devsel, IRQ 5 I/O ports at b000 Memory at fcc01000 (32-bit, non-prefetchable) Memory at fcc0 (32-bit, non-prefetchable) @@ -384,7 +384,7 @@ 24:00.0 System peripheral: Meinberg Funkuhren GPS180PEX GPS Receiver (PCI Express) (rev 01) Subsystem: Meinberg Funkuhren GPS180PEX GPS Receiver (PCI Express) -Flags: bus master, fast devsel, latency 0, IRQ 5 +Flags: fast devsel, IRQ 5 I/O ports at a000 Memory at fcb0 (32-bit, non-prefetchable) Capabilities: [50] MSI: Enable- Count=1/4 Maskable- 64bit+ @@ -465,7 +465,7 @@ 29:00.0 Non-Essential Instrumentation [1300]: Advanced Micro Devices, Inc. [AMD] Device 145a Subsystem: Advanced Micro Devices, Inc. [AMD] Device 145a -Flags: bus master, fast devsel, latency 0, IRQ 255 +Flags: fast devsel, IRQ 255 Capabilities: [48] Vendor Specific Information: Len=08 Capabilities: [50] Power Management version 3 Capabilities: [64] Express Endpoint, MSI 00 @@ -476,9 +476,9 @@ 29:00.2 Encryption controller: Advanced Micro Devices, Inc. [AMD] Device 1456 Subsystem: Advanced Micro Devices, Inc. [AMD] Device 1456 -Flags: bus master, fast devsel, latency 0, IRQ 11 -Memory at fd10 (32-bit, non-prefetchable) -Memory at fd20 (32-bit, non-prefetchable) +Flags: fast devsel, IRQ 11 +Memory at fd10 (32-bit, non-prefetchable) [disabled] +Memory at fd20 (32-bit, non-prefetchable) [disabled] Capabilities: [48] Vendor Specific Information: Len=08 Capabilities: [50] Power Management version 3 Capabilities: [64] Express Endpoint, MSI 00 @@ -502,7 +502,7 @@ 2a:00.0 Non-Essential Instrumentation [1300]: Advanced Micro Devices, Inc. [AMD] Device 1455 Subsystem: Advanced Micro Devices, Inc. [AMD] Device 1455 -Flags: bus master, fast devsel, latency 0, IRQ 255 +Flags: fast devsel, IRQ 255 Capabilities: [48] Vendor Specific Information: Len=08 Capabilities: [50] Power Management version 3 Capabilities: [64] Express Endpoint, MSI 00 On 01/28/19 05:11, Masanobu SAITOH wrote: Hi. On 2019/01/28 4:05, Frank
Re: CVS commit: src/sys/dev/pci
> On Jan 28, 2019, at 8:11 AM, Masanobu SAITOH wrote: > > On 2019/01/28 15:07, Jason Thorpe wrote: >> Doesn’t it seem a little dangerous to just blindly enable? > > You mean it should be enable only when the secondary bridge is configured > correctly? > > Both FreeBSD and OpenBSD blindly enabled it. One of the difference between > NetBSD and others is that they configure unconfigured bridge. Right, if the address decoders aren't programmed properly, it seems like you could get into all sorts of trouble. I really feel like the correct way to solve the problem is to fully configure the bus using information from ACPI. -- thorpej
Re: CVS commit: src/sys/dev/pci
On 2019/01/28 15:07, Jason Thorpe wrote: Doesn’t it seem a little dangerous to just blindly enable? You mean it should be enable only when the secondary bridge is configured correctly? Both FreeBSD and OpenBSD blindly enabled it. One of the difference between NetBSD and others is that they configure unconfigured bridge. -- thorpej Sent from my iPhone. On Jan 28, 2019, at 6:09 AM, SAITOH Masanobu wrote: Module Name:src Committed By:msaitoh Date:Mon Jan 28 04:09:51 UTC 2019 Modified Files: src/sys/dev/pci: ppb.c Log Message: Explicitly enable bus masterling in case BIOS, UEFI or firmware don't enable it. Might fix PR kern/53811. To generate a diff of this commit: cvs rdiff -u -r1.65 -r1.66 src/sys/dev/pci/ppb.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
Doesn’t it seem a little dangerous to just blindly enable? -- thorpej Sent from my iPhone. > On Jan 28, 2019, at 6:09 AM, SAITOH Masanobu wrote: > > Module Name:src > Committed By:msaitoh > Date:Mon Jan 28 04:09:51 UTC 2019 > > Modified Files: >src/sys/dev/pci: ppb.c > > Log Message: > Explicitly enable bus masterling in case BIOS, UEFI or firmware don't enable > it. Might fix PR kern/53811. > > > To generate a diff of this commit: > cvs rdiff -u -r1.65 -r1.66 src/sys/dev/pci/ppb.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
Hi. On 2019/01/28 4:05, Frank Kardel wrote: Hi, that made all devices being recognized during boot with UEFI - good. We seem closer, but not there yet (for my main board at least). There is still an issue with interrupts (or dma - see missing bus master in pci differences). It seems device interrupts via ioapic don't get interrupts/data at all or not reliably. This affects following devices AFAICS on my system: com* wm1 (PCIe network card) radeon (see errors below) MSI/X using devices seem to do fine. Interrupt allocation is the same in both environments: UEFI & CSM interrupt id device name(s) ioapic0 pin 9 acpi SCI ioapic0 pin 1 pckbc1 kbd msix0 vec 0 nvme0 adminq msix0 vec 1 nvme0 ioq1 msix0 vec 2 nvme0 ioq2 msix0 vec 3 nvme0 ioq3 msix0 vec 4 nvme0 ioq4 msix0 vec 5 nvme0 ioq5 msix0 vec 6 nvme0 ioq6 msix0 vec 7 nvme0 ioq7 msi1 vec 0 xhci0 msi2 vec 0 ahcisata0 msix3 vec 0 wm0TXRX0 msix3 vec 1 wm0TXRX1 msix3 vec 2 wm0LINK ioapic1 pin 10 wm1, com5 ioapic1 pin 11 com2, ahd0 ioapic1 pin 8 com3 ioapic1 pin 9 com4 msi4 vec 0 hdaudio0 msi5 vec 0 mpii0 msi6 vec 0 xhci1 msi7 vec 0 ahcisata1 msi8 vec 0 hdaudio1 ioapic0 pin 4 com0 ioapic1 pin 30 radeon0 Other hickups seen: keyboard input (PS/2) is sometimes repeated glxgears regularly stalls for a seconds and does not really run smoothly. llinfo entries for wm1 fail, arp resolution on wm1 fail wm1 seems completely broken - no packets are received there dmesg differences are from efi presence, minor difference memory size, different usb detection sequence. nothing critical. The main difference is the radeon* fails to properly initialize giving these diagnostics: kern info: [drm] radeon: irq initialized. kern info: [drm] ring test on 0 succeeded in 0 usecs kern info: [drm] ring test on 3 succeeded in 3 usecs kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:354)uvd_v1_0_start] *ERROR* UVD not responding, giving up!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_evergreen.c:5688)evergreen_startup] *ERROR* radeon: error initializing UVD (-1). kern info: [drm] ib test on ring 0 succeeded in 0 usecs kern info: [drm] ib test on ring 3 succeeded in 0 usecs Differences between pci configs (lspic -v) --- lspci-csm-20190127 2019-01-27 14:42:57.454117956 +0100 +++ lspci-uefi-20190127 2019-01-27 14:51:27.003880544 +0100 @@ -4,7 +4,7 @@ 00:00.2 IOMMU: Advanced Micro Devices, Inc. [AMD] Device 1451 Subsystem: Advanced Micro Devices, Inc. [AMD] Device 1451 - Flags: bus master, fast devsel, latency 0, IRQ 255 + Flags: fast devsel, IRQ 255 Capabilities: [40] Secure device Capabilities: [64] MSI: Enable- Count=1/4 Maskable- 64bit+ Capabilities: [74] HyperTransport: MSI Mapping Enable+ Fixed+ @@ -213,7 +213,7 @@ Capabilities: [100] Advanced Error Reporting 1d:00.0 PCI bridge: Advanced Micro Devices, Inc. [AMD] Device 43b4 (rev 02) (prog-if 00 [Normal decode]) - Flags: bus master, fast devsel, latency 0, IRQ 11 + Flags: fast devsel, IRQ 11 Bus: primary=1d, secondary=1e, subordinate=1e, sec-latency=0
Re: CVS commit: src/sys/dev/pci
Hi, that made all devices being recognized during boot with UEFI - good. We seem closer, but not there yet (for my main board at least). There is still an issue with interrupts (or dma - see missing bus master in pci differences). It seems device interrupts via ioapic don't get interrupts/data at all or not reliably. This affects following devices AFAICS on my system: com* wm1 (PCIe network card) radeon (see errors below) MSI/X using devices seem to do fine. Interrupt allocation is the same in both environments: UEFI & CSM interrupt iddevice name(s) ioapic0 pin 9 acpi SCI ioapic0 pin 1 pckbc1 kbd msix0 vec 0 nvme0 adminq msix0 vec 1 nvme0 ioq1 msix0 vec 2 nvme0 ioq2 msix0 vec 3 nvme0 ioq3 msix0 vec 4 nvme0 ioq4 msix0 vec 5 nvme0 ioq5 msix0 vec 6 nvme0 ioq6 msix0 vec 7 nvme0 ioq7 msi1 vec 0 xhci0 msi2 vec 0 ahcisata0 msix3 vec 0 wm0TXRX0 msix3 vec 1 wm0TXRX1 msix3 vec 2 wm0LINK ioapic1 pin 10 wm1, com5 ioapic1 pin 11 com2, ahd0 ioapic1 pin 8 com3 ioapic1 pin 9 com4 msi4 vec 0 hdaudio0 msi5 vec 0 mpii0 msi6 vec 0 xhci1 msi7 vec 0 ahcisata1 msi8 vec 0 hdaudio1 ioapic0 pin 4 com0 ioapic1 pin 30 radeon0 Other hickups seen: keyboard input (PS/2) is sometimes repeated glxgears regularly stalls for a seconds and does not really run smoothly. llinfo entries for wm1 fail, arp resolution on wm1 fail wm1 seems completely broken - no packets are received there dmesg differences are from efi presence, minor difference memory size, different usb detection sequence. nothing critical. The main difference is the radeon* fails to properly initialize giving these diagnostics: kern info: [drm] radeon: irq initialized. kern info: [drm] ring test on 0 succeeded in 0 usecs kern info: [drm] ring test on 3 succeeded in 3 usecs kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:345)uvd_v1_0_start] *ERROR* UVD not responding, trying to reset the VCPU!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_uvd_v1_0.c:354)uvd_v1_0_start] *ERROR* UVD not responding, giving up!!! kern error: [drm:(/src/NetBSD/cur/src/sys/external/bsd/drm2/dist/drm/radeon/radeon_evergreen.c:5688)evergreen_startup] *ERROR* radeon: error initializing UVD (-1). kern info: [drm] ib test on ring 0 succeeded in 0 usecs kern info: [drm] ib test on ring 3 succeeded in 0 usecs Differences between pci configs (lspic -v) --- lspci-csm-20190127 2019-01-27 14:42:57.454117956 +0100 +++ lspci-uefi-20190127 2019-01-27 14:51:27.003880544 +0100 @@ -4,7 +4,7 @@ 00:00.2 IOMMU: Advanced Micro Devices, Inc. [AMD] Device 1451 Subsystem: Advanced Micro Devices, Inc. [AMD] Device 1451 - Flags: bus master, fast devsel, latency 0, IRQ 255 + Flags: fast devsel, IRQ 255 Capabilities: [40] Secure device Capabilities: [64] MSI: Enable- Count=1/4 Maskable- 64bit+ Capabilities: [74] HyperTransport: MSI Mapping Enable+ Fixed+ @@ -213,7 +213,7 @@ Capabilities: [100] Advanced Error Reporting 1d:00.0 PCI bridge: Advanced Micro Devices, Inc. [AMD] Device 43b4 (rev 02) (prog-if 00 [Normal decode]) - Flags: bus master, fast devsel, latency 0, IRQ 11 + Flags: fast devsel, IRQ 11 Bus: primary=1d, secondary=1e, subordinate=1e, sec-latency=0 I/O behind bridge:
Re: CVS commit: src/sys/dev/pci
In article , Masanobu SAITOH wrote: > > I didn't know it! I've changed with it now. > > Thanks. Thank you! christos
Re: CVS commit: src/sys/dev/pci
On 2019/01/25 3:08, Christos Zoulas wrote:
In article <20190124045004.c9f48f...@cvs.netbsd.org>,
SAITOH Masanobu wrote:
-=-=-=-=-=-
Module Name:src
Committed By: msaitoh
Date: Thu Jan 24 04:50:04 UTC 2019
Modified Files:
src/sys/dev/pci: if_wm.c
Log Message:
No functional change intended:
- Use "do {} while (/*CONSTCOND*/false)" for null DPRINTF().
- Reduce indent level of wm_linkintr_gmii().
There is __nothing
christos
I didn't know it! I've changed with it now.
Thanks.
--
---
SAITOH Masanobu (msai...@execsw.org
msai...@netbsd.org)
Re: CVS commit: src/sys/dev/pci
In article <20190124045004.c9f48f...@cvs.netbsd.org>,
SAITOH Masanobu wrote:
>-=-=-=-=-=-
>
>Module Name: src
>Committed By: msaitoh
>Date: Thu Jan 24 04:50:04 UTC 2019
>
>Modified Files:
> src/sys/dev/pci: if_wm.c
>
>Log Message:
>No functional change intended:
>- Use "do {} while (/*CONSTCOND*/false)" for null DPRINTF().
>- Reduce indent level of wm_linkintr_gmii().
There is __nothing
christos
Re: CVS commit: src/sys/dev/pci
On 30/11/2018 17:47, Jaromir Dolecek wrote:
Module Name:src
Committed By: jdolecek
Date: Fri Nov 30 17:47:54 UTC 2018
Modified Files:
src/sys/dev/pci: ahcisata_pci.c xhci_pci.c
Log Message:
simplify intr establish code - rely on pci_intr_alloc() to allow
also MSI-X, and to return interrupt types which are possible for
pci_intr_establish(); remove fallbacks to retry with MSI/MSI-X
explicitly disabled
discussed on tech-kern@
https://mail-index.netbsd.org/tech-kern/2018/11/27/msg024240.html
err, RIP *_DISABLE_MSI{,X}?
delete them from sys/dev/pci/files.pci?
Nick
Re: CVS commit: src/sys/dev/pci
On 2018/11/02 17:16, SAITOH Masanobu wrote: Module Name:src Committed By: msaitoh Date: Fri Nov 2 08:16:49 UTC 2018 Modified Files: src/sys/dev/pci: if_wm.c Log Message: - Add missing wm_gate_hw_phy_config_ich8lan(false) in wm_phy_post_reset() on PCH2. wm_gate_hw_phy_config_ich8lan(true) is called in wm_reset(), so wm_phy_post_reset(false) should be called after reset. s/wm_phy_post_reset(false) should be called after reset/wm_gate_hw_phy_config_ich8lan(false) should be called after reset in wm_phy_post_reset()/ - On PCH2, set the phy config counter to 50msec after (PHY) reset. To generate a diff of this commit: cvs rdiff -u -r1.593 -r1.594 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. -- --- SAITOH Masanobu (msai...@execsw.org msai...@netbsd.org)
Re: CVS commit: src/sys/dev/pci
On 2018/11/02 17:04, SAITOH Masanobu wrote:
Module Name:src
Committed By: msaitoh
Date: Fri Nov 2 08:04:42 UTC 2018
Modified Files:
src/sys/dev/pci: if_wm.c
Log Message:
Fix a PCH2 specific bug that wrong register value can be read when boot.
When a wrong value is read when boot, the read device ID was incorrect and
ukphy(3) is attached instead of ihphy(4). The bug might also result in
MDIC read/write error.
How to reproduce:
0) Boot Windows.
1) Leave some minutes.
2) Reboot to NetBSD.
To reproduce this problem, the PHY link should be down
(don't connect Ethernet cable).
To fix this problem, adding extra 100us delay at the end of
wm_gmii_mdic_{read,write}reg() on PCH2. Same as FreeBSD and linux.
Reported by David Brownlee a few days ago and also reported by jmcneill
a half year ago. Tested with my own Thinkpad X220.
XXX pullup-[78]
To generate a diff of this commit:
cvs rdiff -u -r1.591 -r1.592 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.
--
---
SAITOH Masanobu (msai...@execsw.org
msai...@netbsd.org)
Re: CVS commit: src/sys/dev/pci
On 2018/11/01 1:11, Jared D. McNeill wrote: Module Name:src Committed By: jmcneill Date: Wed Oct 31 16:11:29 UTC 2018 Modified Files: src/sys/dev/pci: xhci_pci.c Log Message: Add MSI-X support. Some xHCI chips support multi-vector MSI or MSI-X. Each vector can be assigned to different port. Please someone(TM) wrote the code to get the benefit. To generate a diff of this commit: cvs rdiff -u -r1.14 -r1.15 src/sys/dev/pci/xhci_pci.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
On Fri, Sep 14, 2018 at 06:46:47PM +, Jonathan A. Kollasch wrote: > Module Name: src > Committed By: jakllsch > Date: Fri Sep 14 18:46:47 UTC 2018 > > Modified Files: > src/sys/dev/pci: if_msk.c if_mskvar.h if_skreg.h > > Log Message: > msk(4): add 64-bit DMA support > > > To generate a diff of this commit: > cvs rdiff -u -r1.77 -r1.78 src/sys/dev/pci/if_msk.c > cvs rdiff -u -r1.18 -r1.19 src/sys/dev/pci/if_mskvar.h > cvs rdiff -u -r1.24 -r1.25 src/sys/dev/pci/if_skreg.h The commit message on these file-revisions have been adjusted to: msk(4): add 64-bit DMA support portions of this change set provided by mrg@
Re: CVS commit: src/sys/dev/pci/ixgbe
On 2018/07/03 13:02, SAITOH Masanobu wrote: Module Name:src Committed By: msaitoh Date: Tue Jul 3 04:02:07 UTC 2018 Modified Files: src/sys/dev/pci/ixgbe: ixgbe.c Log Message: Remove nmbclusters check. A big problem is that the number of RX descriptor unexpectedly decreased from 2048 to 1024 if the number of port and/or the number of CPU is high. a2sdihtp4f: sysctl -a | grep ixg | grep rx_desc hw.ixg0.num_rx_desc = 2048 hw.ixg1.num_rx_desc = 2048 hw.ixg2.num_rx_desc = 2048 hw.ixg3.num_rx_desc = 1024 We don't use the mbuf cluster. The old code also had a bug that ixgbe_total_ports adds two every port and never decrement in the detach path. Found by hikaru@. The code was removed in FreeBSD when it switched to use iflib and OpenBSD removed the code 8 years ago. To generate a diff of this commit: cvs rdiff -u -r1.161 -r1.162 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
Hi, From: Kengo NAKAHARA , Date: Fri, 8 Jun 2018 20:20:03 +0900 > Hi, > > Sorry, I checked it fixed panic only. iwm.c:r1.81 and r1.82 fix this problem > at my environment. > > Could you try latest kernel with these commits? Thank you very much for your quick fix. It works fine now. > Thanks, > > On 2018/06/09 3:14, Ryo ONODERA wrote: >> Hi, >> >> This change causes the following error and I cannot use iwm(4) device >> anymore. >> Could you take a look at my problem? >> >> $ ifconfig iwm0 >> ifconfig: SIOCGIFFLAGS iwm0: Device not configured >> >> dmesg is here: >> (snip) >> iwm0 at pci2 dev 0 function 0: vendor 8086 product 24fd (rev. 0x78) >> iwm0: interrupting at msi2 vec 0 >> (snip) >> iwm0: hw rev 0x230, fw ver 22.361476.0, address 74:e5:f9:69:6c:bb >> iwm0: 11a rates: 6Mbps 9Mbps 12Mbps 18Mbps 24Mbps 36Mbps 48Mbps 54Mbps >> iwm0: 11b rates: 1Mbps 2Mbps 5.5Mbps 11Mbps >> iwm0: 11g rates: 1Mbps 2Mbps 5.5Mbps 11Mbps 6Mbps 9Mbps 12Mbps 18Mbps 24Mbps >> 36Mbps 48Mbps 54Mbps >> >> >> From: "Kengo NAKAHARA" , Date: Tue, 5 Jun 2018 >> 12:17:18 + >> >>> Module Name:src >>> Committed By: knakahara >>> Date: Tue Jun 5 12:17:18 UTC 2018 >>> >>> Modified Files: >>> src/sys/dev/pci: if_iwm.c >>> >>> Log Message: >>> Fix panic on boot with iwm(4). Advised by nonaka@n.o, thanks. >>> >>> XXX pullup-8 >>> >>> >>> To generate a diff of this commit: >>> cvs rdiff -u -r1.78 -r1.79 src/sys/dev/pci/if_iwm.c >>> >>> Please note that diffs are not public domain; they are subject to the >>> copyright notices on the relevant files. >>> >> >> -- >> Ryo ONODERA // r...@tetera.org >> PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB FD1B F404 27FA C7D1 15F3 >> > > -- > // > Internet Initiative Japan Inc. > > Device Engineering Section, > IoT Platform Development Department, > Network Division, > Technology Unit > > Kengo NAKAHARA -- Ryo ONODERA // r...@tetera.org PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB FD1B F404 27FA C7D1 15F3
Re: CVS commit: src/sys/dev/pci
Hi, Sorry, I checked it fixed panic only. iwm.c:r1.81 and r1.82 fix this problem at my environment. Could you try latest kernel with these commits? Thanks, On 2018/06/09 3:14, Ryo ONODERA wrote: > Hi, > > This change causes the following error and I cannot use iwm(4) device > anymore. > Could you take a look at my problem? > > $ ifconfig iwm0 > ifconfig: SIOCGIFFLAGS iwm0: Device not configured > > dmesg is here: > (snip) > iwm0 at pci2 dev 0 function 0: vendor 8086 product 24fd (rev. 0x78) > iwm0: interrupting at msi2 vec 0 > (snip) > iwm0: hw rev 0x230, fw ver 22.361476.0, address 74:e5:f9:69:6c:bb > iwm0: 11a rates: 6Mbps 9Mbps 12Mbps 18Mbps 24Mbps 36Mbps 48Mbps 54Mbps > iwm0: 11b rates: 1Mbps 2Mbps 5.5Mbps 11Mbps > iwm0: 11g rates: 1Mbps 2Mbps 5.5Mbps 11Mbps 6Mbps 9Mbps 12Mbps 18Mbps 24Mbps > 36Mbps 48Mbps 54Mbps > > > From: "Kengo NAKAHARA" , Date: Tue, 5 Jun 2018 12:17:18 > + > >> Module Name: src >> Committed By:knakahara >> Date:Tue Jun 5 12:17:18 UTC 2018 >> >> Modified Files: >> src/sys/dev/pci: if_iwm.c >> >> Log Message: >> Fix panic on boot with iwm(4). Advised by nonaka@n.o, thanks. >> >> XXX pullup-8 >> >> >> To generate a diff of this commit: >> cvs rdiff -u -r1.78 -r1.79 src/sys/dev/pci/if_iwm.c >> >> Please note that diffs are not public domain; they are subject to the >> copyright notices on the relevant files. >> > > -- > Ryo ONODERA // r...@tetera.org > PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB FD1B F404 27FA C7D1 15F3 > -- // Internet Initiative Japan Inc. Device Engineering Section, IoT Platform Development Department, Network Division, Technology Unit Kengo NAKAHARA
Re: CVS commit: src/sys/dev/pci/ixgbe
On 2018/05/07 16:56, Masanobu SAITOH wrote:
Hi.
On 2018/05/03 17:38, Frank Kardel wrote:
Hi,
I am now seeing that packet reception stops. tcpdump will only show outgoing
packets and no incoming packets.
ifconfig ixgX down
ifconfig ixgX up
gets things going again.
Code from 2018-02-24 was fine and I think even code shortly before this commit
was fine, but there was the mbuf used-after-free issue that hit me pretty hard.
So the system wasn't running for too long anyway.
Does that ring a bell ?
Frank
IMHO, this change doesn't cause such type of problem...
Could you show me the following information?
0) dmesg
This is required to know which chip is used and what type of
interrupt is used and configured (INTX, MSI or MSI-X).
1) ifconfig -v ixg0
It's required to know offload status, the MTU size and some
error count.
2) sysctl hw.ixg0 (or sysctl hw |grep ixg (for all ixg(4)s))
This output has some information of TX/RX ring.
3) vmstat -e[v]
Only for ixg(4):
vmstat -e[v] |grep ixg
ixg(4) has a lot of counters. I added error counters at
all of error paths. (If you found a error path which has
no counter, it's my fault.)
Only to check checksum counters:
options INET_CSUM_COUNTERS (for ip_input())
options TCP_CSUM_COUNTERS (for tcp_input())
options UDP_CSUM_COUNTERS (for udp_input())
options WM_EVENT_COUNTERS (only for wm(4))
options BGE_EVENT_COUNTERS (only for bge(4))
(No special options for ixg(4) because those counters are enabled by
default)
vmstat -e[v] |grep csum
4) If you use a virtual interface like vlan(4) or age(4). Please
s/age/agr/
show the configuration.
5) Whether you use options NET_MPSAFE or not.
Thanks.
Regards.
On 04/25/18 10:46, SAITOH Masanobu wrote:
Module Name: src
Committed By: msaitoh
Date: Wed Apr 25 08:46:19 UTC 2018
Modified Files:
src/sys/dev/pci/ixgbe: ix_txrx.c ixgbe.h ixgbe_netbsd.c ixgbe_netbsd.h
ixgbe_osdep.h
Log Message:
Don't free and reallocate bus_dmamem when it's not required. Currently,
the watchdog timer is completely broken and never fire (it's from FreeBSD
(pre iflib)). If the problem is fixed and watchdog fired, ixgbe_init() always
calls ixgbe_jcl_reinit() and it causes panic. The reason is that
ixgbe_local_timer1(it includes watchdog function) is softint and
xgbe_jcl_reinit() calls bus_dmamem*() functions. bus_dmamem*() can't be called
from interrupt context.
One of the way to prevent panic is use worqueue for the timer, but it's
not a small change. (I'll do it in future).
Another way is not reallocate dmamem if it's not required. If both the MTU
(rx_mbuf_sz in reality) and the number of RX descriptors are not changed, it's
not required to call bus_dmamem_{unmap,free}(). Even if we use workque, this
change save time of ixgbe_init().
I have a code to fix broken watchdog timer but it sometime causes watchdog
timeout, so I don't commit it yet.
To generate a diff of this commit:
cvs rdiff -u -r1.40 -r1.41 src/sys/dev/pci/ixgbe/ix_txrx.c
cvs rdiff -u -r1.45 -r1.46 src/sys/dev/pci/ixgbe/ixgbe.h
cvs rdiff -u -r1.6 -r1.7 src/sys/dev/pci/ixgbe/ixgbe_netbsd.c
cvs rdiff -u -r1.7 -r1.8 src/sys/dev/pci/ixgbe/ixgbe_netbsd.h
cvs rdiff -u -r1.21 -r1.22 src/sys/dev/pci/ixgbe/ixgbe_osdep.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/ixgbe
Hi.
On 2018/05/03 17:38, Frank Kardel wrote:
Hi,
I am now seeing that packet reception stops. tcpdump will only show outgoing
packets and no incoming packets.
ifconfig ixgX down
ifconfig ixgX up
gets things going again.
Code from 2018-02-24 was fine and I think even code shortly before this commit
was fine, but there was the mbuf used-after-free issue that hit me pretty hard.
So the system wasn't running for too long anyway.
Does that ring a bell ?
Frank
IMHO, this change doesn't cause such type of problem...
Could you show me the following information?
0) dmesg
This is required to know which chip is used and what type of
interrupt is used and configured (INTX, MSI or MSI-X).
1) ifconfig -v ixg0
It's required to know offload status, the MTU size and some
error count.
2) sysctl hw.ixg0 (or sysctl hw |grep ixg (for all ixg(4)s))
This output has some information of TX/RX ring.
3) vmstat -e[v]
Only for ixg(4):
vmstat -e[v] |grep ixg
ixg(4) has a lot of counters. I added error counters at
all of error paths. (If you found a error path which has
no counter, it's my fault.)
Only to check checksum counters:
options INET_CSUM_COUNTERS (for ip_input())
options TCP_CSUM_COUNTERS (for tcp_input())
options UDP_CSUM_COUNTERS (for udp_input())
options WM_EVENT_COUNTERS (only for wm(4))
options BGE_EVENT_COUNTERS (only for bge(4))
(No special options for ixg(4) because those counters are enabled by
default)
vmstat -e[v] |grep csum
4) If you use a virtual interface like vlan(4) or age(4). Please
show the configuration.
Regards.
On 04/25/18 10:46, SAITOH Masanobu wrote:
Module Name: src
Committed By: msaitoh
Date: Wed Apr 25 08:46:19 UTC 2018
Modified Files:
src/sys/dev/pci/ixgbe: ix_txrx.c ixgbe.h ixgbe_netbsd.c ixgbe_netbsd.h
ixgbe_osdep.h
Log Message:
Don't free and reallocate bus_dmamem when it's not required. Currently,
the watchdog timer is completely broken and never fire (it's from FreeBSD
(pre iflib)). If the problem is fixed and watchdog fired, ixgbe_init() always
calls ixgbe_jcl_reinit() and it causes panic. The reason is that
ixgbe_local_timer1(it includes watchdog function) is softint and
xgbe_jcl_reinit() calls bus_dmamem*() functions. bus_dmamem*() can't be called
from interrupt context.
One of the way to prevent panic is use worqueue for the timer, but it's
not a small change. (I'll do it in future).
Another way is not reallocate dmamem if it's not required. If both the MTU
(rx_mbuf_sz in reality) and the number of RX descriptors are not changed, it's
not required to call bus_dmamem_{unmap,free}(). Even if we use workque, this
change save time of ixgbe_init().
I have a code to fix broken watchdog timer but it sometime causes watchdog
timeout, so I don't commit it yet.
To generate a diff of this commit:
cvs rdiff -u -r1.40 -r1.41 src/sys/dev/pci/ixgbe/ix_txrx.c
cvs rdiff -u -r1.45 -r1.46 src/sys/dev/pci/ixgbe/ixgbe.h
cvs rdiff -u -r1.6 -r1.7 src/sys/dev/pci/ixgbe/ixgbe_netbsd.c
cvs rdiff -u -r1.7 -r1.8 src/sys/dev/pci/ixgbe/ixgbe_netbsd.h
cvs rdiff -u -r1.21 -r1.22 src/sys/dev/pci/ixgbe/ixgbe_osdep.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/ixgbe
Hi,
I am now seeing that packet reception stops. tcpdump will only show
outgoing packets and no incoming packets.
ifconfig ixgX down
ifconfig ixgX up
gets things going again.
Code from 2018-02-24 was fine and I think even code shortly before this
commit was fine, but there was the mbuf used-after-free issue that hit
me pretty hard. So the system wasn't running for too long anyway.
Does that ring a bell ?
Frank
On 04/25/18 10:46, SAITOH Masanobu wrote:
Module Name:src
Committed By: msaitoh
Date: Wed Apr 25 08:46:19 UTC 2018
Modified Files:
src/sys/dev/pci/ixgbe: ix_txrx.c ixgbe.h ixgbe_netbsd.c ixgbe_netbsd.h
ixgbe_osdep.h
Log Message:
Don't free and reallocate bus_dmamem when it's not required. Currently,
the watchdog timer is completely broken and never fire (it's from FreeBSD
(pre iflib)). If the problem is fixed and watchdog fired, ixgbe_init() always
calls ixgbe_jcl_reinit() and it causes panic. The reason is that
ixgbe_local_timer1(it includes watchdog function) is softint and
xgbe_jcl_reinit() calls bus_dmamem*() functions. bus_dmamem*() can't be called
from interrupt context.
One of the way to prevent panic is use worqueue for the timer, but it's
not a small change. (I'll do it in future).
Another way is not reallocate dmamem if it's not required. If both the MTU
(rx_mbuf_sz in reality) and the number of RX descriptors are not changed, it's
not required to call bus_dmamem_{unmap,free}(). Even if we use workque, this
change save time of ixgbe_init().
I have a code to fix broken watchdog timer but it sometime causes watchdog
timeout, so I don't commit it yet.
To generate a diff of this commit:
cvs rdiff -u -r1.40 -r1.41 src/sys/dev/pci/ixgbe/ix_txrx.c
cvs rdiff -u -r1.45 -r1.46 src/sys/dev/pci/ixgbe/ixgbe.h
cvs rdiff -u -r1.6 -r1.7 src/sys/dev/pci/ixgbe/ixgbe_netbsd.c
cvs rdiff -u -r1.7 -r1.8 src/sys/dev/pci/ixgbe/ixgbe_netbsd.h
cvs rdiff -u -r1.21 -r1.22 src/sys/dev/pci/ixgbe/ixgbe_osdep.h
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/ixgbe
Hi,
I am now seeing that packet reception stops. tcpdump will only show
outgoing packets and no incoming packets.
ifconfig ixgX down
ifconfig ixgX up
gets things going again.
Code from 2018-02-24 was fine and I think even code shortly before this
commit was fine, but there was the mbuf used-after-free issue that hit
me pretty hard. So the system wasn't running for too long anyway.
Does that ring a bell ?
Frank
On 04/25/18 10:46, SAITOH Masanobu wrote:
Module Name:src
Committed By: msaitoh
Date: Wed Apr 25 08:46:19 UTC 2018
Modified Files:
src/sys/dev/pci/ixgbe: ix_txrx.c ixgbe.h ixgbe_netbsd.c ixgbe_netbsd.h
ixgbe_osdep.h
Log Message:
Don't free and reallocate bus_dmamem when it's not required. Currently,
the watchdog timer is completely broken and never fire (it's from FreeBSD
(pre iflib)). If the problem is fixed and watchdog fired, ixgbe_init() always
calls ixgbe_jcl_reinit() and it causes panic. The reason is that
ixgbe_local_timer1(it includes watchdog function) is softint and
xgbe_jcl_reinit() calls bus_dmamem*() functions. bus_dmamem*() can't be called
from interrupt context.
One of the way to prevent panic is use worqueue for the timer, but it's
not a small change. (I'll do it in future).
Another way is not reallocate dmamem if it's not required. If both the MTU
(rx_mbuf_sz in reality) and the number of RX descriptors are not changed, it's
not required to call bus_dmamem_{unmap,free}(). Even if we use workque, this
change save time of ixgbe_init().
I have a code to fix broken watchdog timer but it sometime causes watchdog
timeout, so I don't commit it yet.
To generate a diff of this commit:
cvs rdiff -u -r1.40 -r1.41 src/sys/dev/pci/ixgbe/ix_txrx.c
cvs rdiff -u -r1.45 -r1.46 src/sys/dev/pci/ixgbe/ixgbe.h
cvs rdiff -u -r1.6 -r1.7 src/sys/dev/pci/ixgbe/ixgbe_netbsd.c
cvs rdiff -u -r1.7 -r1.8 src/sys/dev/pci/ixgbe/ixgbe_netbsd.h
cvs rdiff -u -r1.21 -r1.22 src/sys/dev/pci/ixgbe/ixgbe_osdep.h
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/ixgbe
On 2018/04/04 17:13, SAITOH Masanobu wrote:
Module Name:src
Committed By: msaitoh
Date: Wed Apr 4 08:13:07 UTC 2018
Modified Files:
src/sys/dev/pci/ixgbe: if_bypass.c if_fdir.c if_sriov.c ix_txrx.c
ixgbe.c ixgbe.h ixgbe_82598.c ixgbe_82598.h ixgbe_82599.c
ixgbe_82599.h ixgbe_api.c ixgbe_api.h ixgbe_common.c ixgbe_common.h
ixgbe_dcb.c ixgbe_dcb.h ixgbe_dcb_82598.c ixgbe_dcb_82598.h
ixgbe_dcb_82599.c ixgbe_dcb_82599.h ixgbe_fdir.h ixgbe_mbx.c
ixgbe_mbx.h ixgbe_osdep.c ixgbe_osdep.h ixgbe_phy.c ixgbe_phy.h
ixgbe_rss.h ixgbe_sriov.h ixgbe_type.h ixgbe_vf.c ixgbe_vf.h
ixgbe_x540.c ixgbe_x540.h ixv.c
Log Message:
Sync with the remaining part of FreeBSD r328265 except sfp_reinit stuff:
- Always schedule module intterrupt in ixgbe_config_link() when a device is
SFP+ based.
- Use not loop index but txr->me in ixv_initialize_{transmit,receive}_units().
It's required for VMDQ but NetBSD doesn't use it, so it's not a bug in
NetBSD.
- Simplify ixgbe_bp_wd_set(). No functional change.
- Whitespace.
+ Call ixgbe_set_phy_power() in ixgbe_init_locked() to
make sure we are not in power save mode.
--
---
SAITOH Masanobu (msai...@execsw.org
msai...@netbsd.org)
