Re: CVS commit: src/sys/arch/aarch64/aarch64
On Mon, 22 Feb 2021, Ryo Shimizu wrote: I think this condition is not necessary since cpu_idle() is just called from idle_loop(), and ci_intr_depth is always zero at this time. Ah yes, my mistake! Please feel free to revert this commit as part of your proposed change.
Re: CVS commit: src/sys/arch/aarch64/aarch64
> On Feb 22, 2021, at 11:49 AM, Ryo Shimizu wrote: > > Ah, You are quite right! > idle/# lwp is provided and assigned for each CPU, so curcpu() obtained from > idle lwp was always the same. > So, there's no need to move curcpu() to after DISABLE_INTERRUPT. Please make sure to add a comment explaining why! -- thorpej
Re: CVS commit: src/sys/arch/aarch64/aarch64
>> In addition, because of the possibility of kpreemption (but aarch64 has = >no KPREEMPT yet), >> the acquisition of curcpu() is moved to after DISABLE_INTERRUPT and got = >the following. >> >[snip] > > >> >> Is this ok? >> > >Looks good - I wonder if the fact that curcpu is an invariant for the >idlelwp helps here too? Ah, You are quite right! idle/# lwp is provided and assigned for each CPU, so curcpu() obtained from idle lwp was always the same. So, there's no need to move curcpu() to after DISABLE_INTERRUPT. Thanks -- ryo shimizu
Re: CVS commit: src/sys/arch/aarch64/aarch64
On 22/02/2021 10:40, Ryo Shimizu wrote: Module Name:src Committed By: jmcneill Date: Sun Feb 21 23:37:10 UTC 2021 Modified Files: src/sys/arch/aarch64/aarch64: idle_machdep.S Log Message: When waking from cpu_idle(), only call dosoftints if ci_intr_depth == 0 To generate a diff of this commit: cvs rdiff -u -r1.7 -r1.8 src/sys/arch/aarch64/aarch64/idle_machdep.S I think this condition is not necessary since cpu_idle() is just called from idle_loop(), and ci_intr_depth is always zero at this time. After thinking about it, I realized that there is no need to even increment intr_depth. curcpu()->ci_ntr_depth = 1; ARM_IRQ_HANDLER(); curcpu()->ci_ntr_depth = 0; In addition, because of the possibility of kpreemption (but aarch64 has no KPREEMPT yet), the acquisition of curcpu() is moved to after DISABLE_INTERRUPT and got the following. [snip] Is this ok? Looks good - I wonder if the fact that curcpu is an invariant for the idlelwp helps here too? Nick
Re: CVS commit: src/sys/arch/aarch64/aarch64
>Module Name: src >Committed By: jmcneill >Date: Sun Feb 21 23:37:10 UTC 2021 > >Modified Files: > src/sys/arch/aarch64/aarch64: idle_machdep.S > >Log Message: >When waking from cpu_idle(), only call dosoftints if ci_intr_depth == 0 > > >To generate a diff of this commit: >cvs rdiff -u -r1.7 -r1.8 src/sys/arch/aarch64/aarch64/idle_machdep.S I think this condition is not necessary since cpu_idle() is just called from idle_loop(), and ci_intr_depth is always zero at this time. After thinking about it, I realized that there is no need to even increment intr_depth. curcpu()->ci_ntr_depth = 1; ARM_IRQ_HANDLER(); curcpu()->ci_ntr_depth = 0; In addition, because of the possibility of kpreemption (but aarch64 has no KPREEMPT yet), the acquisition of curcpu() is moved to after DISABLE_INTERRUPT and got the following. cvs -q diff -aU10 -a -p idle_machdep.S Index: idle_machdep.S === RCS file: /src/cvs/cvsroot-netbsd/src/sys/arch/aarch64/aarch64/idle_machdep.S,v retrieving revision 1.8 diff -a -U 10 -a -p -r1.8 idle_machdep.S --- idle_machdep.S 21 Feb 2021 23:37:09 - 1.8 +++ idle_machdep.S 22 Feb 2021 10:16:25 - @@ -67,40 +67,36 @@ ENTRY(cpu_idle) stp x29, x30, [sp, #TF_X29] /* save x29,x30 */ #ifdef DDB add x29, sp, #TF_X29/* link frame for backtrace */ #endif /* fill the minimum required trapframe */ mov x2, #SPSR_M_EL1H/* what our spsr should be */ str x2, [sp, #TF_SPSR] adr x0, 1f str x0, [sp, #TF_PC]/* CLKF_PC refer to tf_pc */ - - mrs x1, tpidr_el1 /* get curlwp */ - ldr x1, [x1, #L_CPU]/* get curcpu */ - ldr w28, [x1, #CI_INTR_DEPTH] /* w28 = ci->ci_intr_depth */ - add w2, w28, #1 /* w2 = intr_depth + 1 */ - mov x0, sp /* get pointer to trapframe */ + mrs x1, tpidr_el1 /* get curlwp */ DISABLE_INTERRUPT - wfi + ldr x1, [x1, #L_CPU]/* get curcpu */ + mov w2, #1 + str w2, [x1, #CI_INTR_DEPTH]/* ci->ci_intr_depth = 1 */ - str w2, [x1, #CI_INTR_DEPTH]/* ci->ci_intr_depth++ */ + wfi bl ARM_IRQ_HANDLER /* irqhandler(trapframe) */ 1: mrs x1, tpidr_el1 /* get curlwp */ ldr x1, [x1, #L_CPU]/* get curcpu */ - str w28, [x1, #CI_INTR_DEPTH] /* ci->ci_intr_depth = old */ + str wzr, [x1, #CI_INTR_DEPTH] /* ci->ci_intr_depth = 0 */ #if defined(__HAVE_FAST_SOFTINTS) && !defined(__HAVE_PIC_FAST_SOFTINTS) - cbnzw28, 1f /* Skip if intr_depth > 0 */ ldr w3, [x1, #CI_SOFTINTS] /* Get pending softint mask */ /* CPL should be 0 */ ldr w2, [x1, #CI_CPL] /* Get current priority level */ lsr w3, w3, w2 /* shift mask by cpl */ cbz w3, 1f bl _C_LABEL(dosoftints)/* dosoftints() */ 1: #endif /* __HAVE_FAST_SOFTINTS && !__HAVE_PIC_FAST_SOFTINTS */ ldr x28, [sp, #TF_X28] /* restore x28 */ Is this ok? -- ryo shimizu
Re: CVS commit: src/sys/arch/hppa/gsc
Hello, On Fri, 05 Feb 2021 00:12:35 +0900 Tetsuya Isaki wrote: > At Wed, 3 Feb 2021 13:37:24 -0500, > Michael wrote: > > > Committed By: isaki > > > Date: Wed Feb 3 15:13:49 UTC 2021 > > > > > > Modified Files: > > > src/sys/arch/hppa/gsc: harmony.c > > > > > > Log Message: > > > Fix locking against myself. > > > trigger_output will be called with sc_intr_lock held. > > > From source code review, not tested. > > > > I just tested this on my C200 - playback works. > > Thank you for quick response! > > > The sample rate seems off - everything plays too slow, but that's > > probably completely unrelated. > > Oops, this was my mistake. > I hope that harmony.c,v 1.9 will fix this problem. Works perfectly now, thank you! have fun Michael
Re: CVS commit: src/sys/arch/hppa/gsc
Hello, At Wed, 3 Feb 2021 13:37:24 -0500, Michael wrote: > > Committed By: isaki > > Date: Wed Feb 3 15:13:49 UTC 2021 > > > > Modified Files: > > src/sys/arch/hppa/gsc: harmony.c > > > > Log Message: > > Fix locking against myself. > > trigger_output will be called with sc_intr_lock held. > > From source code review, not tested. > > I just tested this on my C200 - playback works. Thank you for quick response! > The sample rate seems off - everything plays too slow, but that's > probably completely unrelated. Oops, this was my mistake. I hope that harmony.c,v 1.9 will fix this problem. Sorry for breaking it. --- Tetsuya Isaki
Re: CVS commit: src/sys/arch/hppa/gsc
Hello, On Wed, 3 Feb 2021 15:13:49 + "Tetsuya Isaki" wrote: > Module Name: src > Committed By: isaki > Date: Wed Feb 3 15:13:49 UTC 2021 > > Modified Files: > src/sys/arch/hppa/gsc: harmony.c > > Log Message: > Fix locking against myself. > trigger_output will be called with sc_intr_lock held. > From source code review, not tested. I just tested this on my C200 - playback works. The sample rate seems off - everything plays too slow, but that's probably completely unrelated. have fun Michael
Re: CVS commit: src/sys/arch
> On Jan 25, 2021, at 9:45 AM, Robert Elz wrote:
>
>Date:Mon, 25 Jan 2021 08:19:44 -0800
>From:Jason Thorpe
>Message-ID:
>
> | Using { 0 } makes an assumption about the first member of the
> | structure which is not guaranteed to remain true.
>
> That's right, but you could explicitly init a named field, most likely
> the one that is tested to determine that this is the sentinel, eg: from
> one of the recent updates ...
>
> static const struct device_compatible_entry compat_data[] = {
>{ .compat = "pnpPNP,401" },
> - { 0 }
> + { }
> };
>
> that could instead be changed to
> { .compat = NULL }
>
> (or something similar to that).
I noticed this because of a different local change in my tree that makes the
first field another anonymous union.
Anyhow, I'll go ahead and define a standard sentinel macro that can be used for
the common { .compat = XXX } case, and fire up sed to fix up the tree.
-- thorpej
Re: CVS commit: src/sys/arch
On 25.01.2021 17:19, Jason Thorpe wrote:
>
>> On Jan 25, 2021, at 6:22 AM, Kamil Rytarowski wrote:
>>
>> I have no problem with this change but I am curious why should we use "{
>> }"? It's a C GNU extension and C++ syntax.
>
> Using { 0 } makes an assumption about the first member of the structure which
> is not guaranteed to remain true.
>
Both versions should work in the same way, except that {0} is the
standard variation and {} an extension.
I have got no preference for either.
> -- thorpej
>
signature.asc
Description: OpenPGP digital signature
Re: CVS commit: src/sys/arch
On Mon, Jan 25, 2021 at 20:28:52 +0300, Valery Ushakov wrote:
> On Mon, Jan 25, 2021 at 08:19:44 -0800, Jason Thorpe wrote:
>
> > > On Jan 25, 2021, at 6:22 AM, Kamil Rytarowski wrote:
> > >
> > > I have no problem with this change but I am curious why should we use "{
> > > }"? It's a C GNU extension and C++ syntax.
> >
> > Using { 0 } makes an assumption about the first member of the
> > structure which is not guaranteed to remain true.
>
> The commit message says:
>
> | Since we're using designated initialisers for compat data, we should
> | use a completely empty initializer for the sentinel.
>
> but that "should" is not true. The code that checks that sentinel
> uses some particular member to access it, so, pedantically speaking,
> the initialization must designate that member in the sentinel
> initialization. Yes, this is verbose and doesn't look as pretty, but
> that is what "should" happen here. Using non-standard {} extension
> makes it look nicer, but is not a "should" kind of necessity.
PS: Forgot to add that C++ doesn't have designated initializers (or at
least it didn't last time I looked), so they are in a different
situation here and need an empty initializer list. In C the only
difference it makes is, as far as I can tell, exactly this kind of an
array with a sentinel at the end and the difference is cosmetic.
-uwe
Re: CVS commit: src/sys/arch
Date:Mon, 25 Jan 2021 08:19:44 -0800
From:Jason Thorpe
Message-ID:
| Using { 0 } makes an assumption about the first member of the
| structure which is not guaranteed to remain true.
That's right, but you could explicitly init a named field, most likely
the one that is tested to determine that this is the sentinel, eg: from
one of the recent updates ...
static const struct device_compatible_entry compat_data[] = {
{ .compat = "pnpPNP,401" },
- { 0 }
+ { }
};
that could instead be changed to
{ .compat = NULL }
(or something similar to that).
kre
Re: CVS commit: src/sys/arch
On Mon, Jan 25, 2021 at 08:19:44 -0800, Jason Thorpe wrote:
> > On Jan 25, 2021, at 6:22 AM, Kamil Rytarowski wrote:
> >
> > I have no problem with this change but I am curious why should we use "{
> > }"? It's a C GNU extension and C++ syntax.
>
> Using { 0 } makes an assumption about the first member of the
> structure which is not guaranteed to remain true.
The commit message says:
| Since we're using designated initialisers for compat data, we should
| use a completely empty initializer for the sentinel.
but that "should" is not true. The code that checks that sentinel
uses some particular member to access it, so, pedantically speaking,
the initialization must designate that member in the sentinel
initialization. Yes, this is verbose and doesn't look as pretty, but
that is what "should" happen here. Using non-standard {} extension
makes it look nicer, but is not a "should" kind of necessity.
-uwe
Re: CVS commit: src/sys/arch
> On Jan 25, 2021, at 6:22 AM, Kamil Rytarowski wrote:
>
> I have no problem with this change but I am curious why should we use "{
> }"? It's a C GNU extension and C++ syntax.
Using { 0 } makes an assumption about the first member of the structure which
is not guaranteed to remain true.
-- thorpej
Re: CVS commit: src/sys/arch
On 25.01.2021 15:20, Jason R Thorpe wrote:
> Module Name: src
> Committed By: thorpej
> Date: Mon Jan 25 14:20:39 UTC 2021
>
> Modified Files:
> src/sys/arch/arm/altera: cycv_clkmgr.c
> src/sys/arch/arm/amlogic: meson_pinctrl.c meson_pwm.c meson_thermal.c
> meson_usbctrl.c meson_usbphy.c mesong12_clkc.c mesongx_mmc.c
> mesongxbb_clkc.c
> src/sys/arch/arm/broadcom: bcm2835_emmc.c bcm2835_intr.c
> src/sys/arch/arm/fdt: gicv3_fdt.c pcihost_fdt.c
> src/sys/arch/arm/nvidia: tegra_ahcisata.c tegra_nouveau.c tegra_pcie.c
> tegra_pinmux.c tegra_soctherm.c tegra_xusb.c
> src/sys/arch/arm/nxp: if_enet_imx.c imx6_pcie.c imx6_spi.c
> imx8mq_usbphy.c imx_sdhc.c
> src/sys/arch/arm/rockchip: rk3328_iomux.c rk3399_iomux.c rk_gmac.c
> rk_i2s.c rk_pwm.c rk_tsadc.c rk_usb.c rk_v1crypto.c rk_vop.c
> src/sys/arch/arm/samsung: exynos_dwcmmc.c exynos_pinctrl.c
> exynos_platform.c exynos_usbdrdphy.c exynos_usbphy.c
> src/sys/arch/arm/sociox: if_ave.c
> src/sys/arch/arm/sunxi: sun4i_a10_ccu.c sun4i_dma.c sun6i_dma.c
> sun8i_crypto.c sunxi_can.c sunxi_codec.c sunxi_de2_ccu.c
> sunxi_dep.c sunxi_gpio.c sunxi_hdmi.c sunxi_hdmiphy.c sunxi_i2s.c
> sunxi_lcdc.c sunxi_lradc.c sunxi_mixer.c sunxi_mmc.c sunxi_musb.c
> sunxi_nmi.c sunxi_pwm.c sunxi_rsb.c sunxi_rtc.c sunxi_sid.c
> sunxi_sramc.c sunxi_tcon.c sunxi_thermal.c sunxi_ts.c sunxi_twi.c
> sunxi_usb3phy.c sunxi_usbphy.c sunxi_wdt.c
> src/sys/arch/arm/ti: ti_dpll_clock.c ti_gpio.c ti_iic.c ti_omapintc.c
> ti_omaptimer.c ti_sdhc.c
> src/sys/arch/macppc/dev: deq.c lmu.c psoc.c smusat.c
> src/sys/arch/mips/cavium/dev: octeon_cib.c octeon_intc.c
> src/sys/arch/sparc64/dev: pcf8591_envctrl.c
>
> Log Message:
> Since we're using designated initialisers for compat data, we should
> use a completely empty initializer for the sentinel.
>
> static const struct device_compatible_entry compat_data[] = {
> { .compat = "ecadc" },
> -
> - { 0 }
> + { }
> };
>
> static int
>
I have no problem with this change but I am curious why should we use "{
}"? It's a C GNU extension and C++ syntax.
signature.asc
Description: OpenPGP digital signature
Re: CVS commit: src/sys/arch/evbarm/conf
I forgot that I added dma-ranges support back in Feb last year. All good, please ignore me :) On Thu, 21 Jan 2021, Jared McNeill wrote: This driver is not 64-bit safe and should not be enabled on aarch64 as-is. I think before turning it on we should restrict it to the lower 1GB of memory like the Pi3 models where we know it at least has a chance of working. You can do this easily by creating a restrictive tag using bus_dmatag_subregion(9). Take care, Jared On Thu, 21 Jan 2021, Nia Alarie wrote: Module Name:src Committed By: nia Date: Thu Jan 21 17:46:28 UTC 2021 Modified Files: src/sys/arch/evbarm/conf: GENERIC64 Log Message: add vcaudio (intentionally this time) gives working audio output on rpi3 without needing to run a 32-bit image. To generate a diff of this commit: cvs rdiff -u -r1.173 -r1.174 src/sys/arch/evbarm/conf/GENERIC64 Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Re: CVS commit: src/sys/arch/evbarm/conf
This driver is not 64-bit safe and should not be enabled on aarch64 as-is. I think before turning it on we should restrict it to the lower 1GB of memory like the Pi3 models where we know it at least has a chance of working. You can do this easily by creating a restrictive tag using bus_dmatag_subregion(9). Take care, Jared On Thu, 21 Jan 2021, Nia Alarie wrote: Module Name:src Committed By: nia Date: Thu Jan 21 17:46:28 UTC 2021 Modified Files: src/sys/arch/evbarm/conf: GENERIC64 Log Message: add vcaudio (intentionally this time) gives working audio output on rpi3 without needing to run a 32-bit image. To generate a diff of this commit: cvs rdiff -u -r1.173 -r1.174 src/sys/arch/evbarm/conf/GENERIC64 Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
re: CVS commit: src/sys/arch
"Nia Alarie" writes: > Module Name: src > Committed By: nia > Date: Wed Jan 20 13:22:08 UTC 2021 > > Modified Files: > src/sys/arch/amd64/conf: GENERIC MODULAR XEN3_DOM0 XEN3_DOMU > src/sys/arch/evbarm/conf: OPENBLOCKS_AX3 > src/sys/arch/i386/conf: GENERIC MODULAR XEN3PAE_DOM0 XEN3PAE_DOMU > src/sys/arch/landisk/conf: GENERIC > src/sys/arch/usermode/conf: GENERIC.common > src/sys/arch/zaurus/conf: GENERIC > > Log Message: > remove compat_ossaudio from kernel modules > > this is only useful with compat_linux and gets autoloaded when > compat_linux is loaded, so there's no reason to bake it into kernels > any more. not everyone uses COMPAT_LINUX as a module. this seems reasonable to have as a comment anywhere COMAPT_LINUX is commented. at least the GENERIC*s should have it, IMO. additionally, there are a few issues remaining. eg, the evbarm/conf/POGO kernel now has a useless "no options" for COMPAT_OSSAUDIO, and there are at least a handful of commented versions remaining. .mrg.
Re: CVS commit: src/sys/arch/arm/sunxi
On Tue, Jan 19, 2021 at 07:19:51PM +0100, Martin Husemann wrote: > On Tue, Jan 19, 2021 at 12:35:10AM +, Jason R Thorpe wrote: > > Module Name:src > > Committed By: thorpej > > Date: Tue Jan 19 00:35:10 UTC 2021 > > > > Modified Files: > > src/sys/arch/arm/sunxi: sunxi_sramc.c > > > > Log Message: > > Use device_compatible_entry / of_search_compatible() rather than matching > > against multiple sets of compatibility strings. > > > > > > To generate a diff of this commit: > > cvs rdiff -u -r1.5 -r1.6 src/sys/arch/arm/sunxi/sunxi_sramc.c > > This breaks cubietruck (fdt is: sun7i-a20-cubietruck.dtb): And it was fixed with $NetBSD: sunxi_sramc.c,v 1.7 2021/01/20 00:48:49 jmcneill Exp $ Martin
Re: CVS commit: src/sys/arch/arm/sunxi
On Tue, Jan 19, 2021 at 12:35:10AM +, Jason R Thorpe wrote:
> Module Name: src
> Committed By: thorpej
> Date: Tue Jan 19 00:35:10 UTC 2021
>
> Modified Files:
> src/sys/arch/arm/sunxi: sunxi_sramc.c
>
> Log Message:
> Use device_compatible_entry / of_search_compatible() rather than matching
> against multiple sets of compatibility strings.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.5 -r1.6 src/sys/arch/arm/sunxi/sunxi_sramc.c
This breaks cubietruck (fdt is: sun7i-a20-cubietruck.dtb):
[ 1.000] fclock0 at simplebus2: 2500 Hz fixed clock (mii_phy_tx)
[ 1.000] fclock1 at simplebus2: 12500 Hz fixed clock (gmac_int_tx)
[ 1.000] fclock2 at simplebus2: 2400 Hz fixed clock (osc24M)
[ 1.000] fclock3 at simplebus2: 32768 Hz fixed clock (osc32k)
[ 1.000] gtmr0 at simplebus0: Generic Timer
[ 1.000] gtmr0: interrupting on GIC irq 27
[ 1.000] armgtmr0 at gtmr0: Generic Timer (24000 kHz, virtual)
[ 1.030] sun4ia10ccu0 at simplebus1: A20 CCU
[ 1.030] sunxinmi0 at simplebus1: NMI
[ 1.030] sunxigmacclk0 at simplebus2: GMAC MII/RGMII clock mux
[ 1.030] sunxigpio0 at simplebus1: PIO
[ 1.030] gpio0 at sunxigpio0: 175 pins
[ 1.030] sunxigpio0: interrupting on GIC irq 60
[ 1.030] sun4idma0 at simplebus1: DMA controller
[ 1.030] sun4idma0: interrupting on GIC irq 59
[ 1.030] sunxisramc0 at simplebus1: SRAM Controller
[ 1.030] uvm_fault(0x80b92d68, 0, 1) -> e
[ 1.030] Fatal kernel mode data abort: 'Translation Fault (S)'
[ 1.030] trapframe: 0x80bc8cf0
[ 1.030] FSR=0005, FAR=, spsr=6353
[ 1.030] r0 =92cfd200, r1 =806b0910, r2 =, r3 =
[ 1.030] r4 =92a0cd00, r5 =10c4, r6 =92cfd200, r7 =0dd0
[ 1.030] r8 =806b0910, r9 =114c, r10=80634a80, r11=80bc8d94
[ 1.030] r12=92cf3988, ssp=80bc8d40, slr=804bc688, pc =80061fac
Stopped in pid 0.0 (system) at netbsd:sunxi_sramc_attach+0x16c:ldr
r2, [r2]
(gdb) list *(sunxi_sramc_attach+0x16c)
0x80061fac is in sunxi_sramc_attach
(../../../../arch/arm/sunxi/sunxi_sramc.c:135).
130 if (dce != NULL) {
131 node = kmem_alloc(sizeof(*node), KM_SLEEP);
132 node->phandle = child;
133 node->area = dce->data;
134 TAILQ_INSERT_TAIL(>sc_nodes, node, nodes);
135 aprint_verbose_dev(sc->sc_dev, "area: %s\n",
136 node->area->desc);
137 }
138 }
139 }
With the change backed out it boots fine (dmesg below).
Martin
--8<--
Copyright (c) 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005,
2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017,
2018, 2019, 2020, 2021 The NetBSD Foundation, Inc. All rights reserved.
Copyright (c) 1982, 1986, 1989, 1991, 1993
The Regents of the University of California. All rights reserved.
NetBSD 9.99.78 (GENERIC) #117: Tue Jan 19 19:14:10 CET 2021
mar...@seven-days-to-the-wolves.aprisoft.de:/work/src/sys/arch/evbarm/compile/GENERIC
total memory = 2045 MB
avail memory = 1989 MB
entropy: no seed from bootloader
timecounter: Timecounters tick every 10.000 msec
Kernelized RAIDframe activated
armfdt0 (root)
simplebus0 at armfdt0: Cubietech Cubietruck
simplebus1 at simplebus0
cpus0 at simplebus0
simplebus2 at simplebus0
simplebus3 at simplebus0
cpu0 at cpus0: Cortex-A7 r0p4 (Cortex V7A core)
cpu0: DC enabled IC enabled WB enabled LABT branch prediction enabled
cpu0: 32KB/32B 2-way L1 VIPT Instruction cache
cpu0: 32KB/64B 4-way write-back-locking-C L1 PIPT Data cache
cpu0: 256KB/64B 8-way write-through L2 PIPT Unified cache
vfp0 at cpu0: NEON MPE (VFP 3.0+), rounding, NaN propagation, denormals
cpufreqdt0 at cpu0
cpu1 at cpus0
cpufreqdt1 at cpu1
gic0 at simplebus1: GIC
armgic0 at gic0: Generic Interrupt Controller, 160 sources (150 valid)
armgic0: 16 Priorities, 128 SPIs, 7 PPIs, 15 SGIs
fclock0 at simplebus2: 2500 Hz fixed clock (mii_phy_tx)
fclock1 at simplebus2: 12500 Hz fixed clock (gmac_int_tx)
fclock2 at simplebus2: 2400 Hz fixed clock (osc24M)
fclock3 at simplebus2: 32768 Hz fixed clock (osc32k)
gtmr0 at simplebus0: Generic Timer
gtmr0: interrupting on GIC irq 27
armgtmr0 at gtmr0: Generic Timer (24000 kHz, virtual)
timecounter: Timecounter "armgtmr0" frequency 2400 Hz quality 500
sun4ia10ccu0 at simplebus1: A20 CCU
sunxinmi0 at simplebus1: NMI
sunxigmacclk0 at simplebus2: GMAC MII/RGMII clock mux
sunxigpio0 at simplebus1: PIO
gpio0 at sunxigpio0: 175 pins
sunxigpio0: interrupting on GIC irq 60
sun4idma0 at simplebus1: DMA controller
sun4idma0: interrupting on GIC irq 59
sunxisramc0 at simplebus1: SRAM Controller
sunxisramc0: area: SRAM A3/A4
sunxisramc0: area: SRAM D
sunxidebe0 at simplebus1: Display Engine Backend (display-backend@1e6)
re: CVS commit: src/sys/arch/aarch64/aarch64
> Log Message: > Fix build as crash(8); Protect db_md_meminfo_cmd() by defined(_KERNEL). thanks. surprised i never saw this as the change was in a tree for a few weeks, but i guess i was mostly doing kernels in that tree not full builds.. .mrg.
Re: CVS commit: src/sys/arch/powerpc/ibm4xx
On 2021/01/18 14:49, Rin Okuyama wrote:
(2) However, in clock.c rev 1.31 and prior, curcpu->ci_idepth was not
raised before calling {hard,stat}clock(). Therefore, cpu_intr_p()
wrongly returns false. As a result, callee functions misunderstood
that they are not running in the interrupt context.
1.31 --> 1.30
Sorry for bothering you many times...
rin
Re: CVS commit: src/sys/arch/powerpc/ibm4xx
On 2021/01/18 13:35, Rin Okuyama wrote:
Module Name:src
Committed By: rin
Date: Mon Jan 18 04:35:05 UTC 2021
Modified Files:
src/sys/arch/powerpc/ibm4xx: clock.c
Log Message:
Invoke hardclock() and statclock() in the interrupt context.
Otherwise, entropy_enter() is used instead of entropy_enter_intr() in
statclock(), which results in KASSERT() failure.
To generate a diff of this commit:
cvs rdiff -u -r1.30 -r1.31 src/sys/arch/powerpc/ibm4xx/clock.c
This message is somewhat misleading. I meant:
(1) Callers are interrupt handlers, therefore, {hard,stat}clock() are
apparently invoked in the interrupt context.
(2) However, in clock.c rev 1.31 and prior, curcpu->ci_idepth was not
raised before calling {hard,stat}clock(). Therefore, cpu_intr_p()
wrongly returns false. As a result, callee functions misunderstood
that they are not running in the interrupt context.
I have to improve my English writing skills ;-).
Thanks,
rin
Re: CVS commit: src/sys/arch/arm/rockchip
Oops. The change was to make sure that a devicetree node with a "rockchip,grf" property on a device type that doesn't provide a config struct doesn't deref a NULL ptr. On Fri, 1 Jan 2021, Jared D. McNeill wrote: Module Name:src Committed By: jmcneill Date: Fri Jan 1 11:44:41 UTC 2021 Modified Files: src/sys/arch/arm/rockchip: rk_i2s.c Log Message: rk_i2s.c To generate a diff of this commit: cvs rdiff -u -r1.5 -r1.6 src/sys/arch/arm/rockchip/rk_i2s.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/arch/arm/arm32
Excellent! Thank you so much for finding out and fixing this! Full ATF successfully completed for Raspberry Pi 2b, which formerly crashed due to "anon != NULL && anon->an_ref != 0" panic. Now, ATF is running on Cubietruck and Raspberry Pi Zero W. Thanks, rin On 2020/11/22 4:44, Nick Hudson wrote: Module Name:src Committed By: skrll Date: Sat Nov 21 19:44:52 UTC 2020 Modified Files: src/sys/arch/arm/arm32: cpuswitch.S Log Message: Ensure that r5 contains curlwp before DO_AST_AND_RESTORE_ALIGNMENT_FAULTS in lwp_trampoline as required by the move to make ASTs operate per-LWP rather than per-CPU. Thanks to martin@ for bisecting the amap corruption he was seeing and testing this fix. To generate a diff of this commit: cvs rdiff -u -r1.103 -r1.104 src/sys/arch/arm/arm32/cpuswitch.S Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Re: CVS commit: src/sys/arch/arm/arm
On 2020/11/12 6:52, matthew green wrote: "Rin Okuyama" writes: Module Name:src Committed By: rin Date: Tue Nov 10 21:40:07 UTC 2020 Modified Files: src/sys/arch/arm/arm: cpu_exec.c Log Message: Test (epp->ep_esch->es_emul != _netbsd) instead of nice, this is a step forward. an optimisation on it could be to remove this test entirely if neither MODULAR or COMAPT_NETBSD32 are set, as it will always be false there. Ah, yes. I will commit after some test. Thanks! rin
re: CVS commit: src/sys/arch/arm/arm
"Rin Okuyama" writes: > Module Name: src > Committed By: rin > Date: Tue Nov 10 21:40:07 UTC 2020 > > Modified Files: > src/sys/arch/arm/arm: cpu_exec.c > > Log Message: > Test (epp->ep_esch->es_emul != _netbsd) instead of nice, this is a step forward. an optimisation on it could be to remove this test entirely if neither MODULAR or COMAPT_NETBSD32 are set, as it will always be false there. thanks. .mrg.
Re: CVS commit: src/sys/arch/sparc64/dev
Hi Tobias, > If you're interested there is an older version[1] of envctrl in the > Attic that might be relevant to use for reference. It supported fan > speed controls on E450. IIRC I got some of the magic constants from > OpenSolaris. Sadly I don't own an E450 any more. > > [1] > cvsweb.netbsd.org/bsdweb.cgi/~checkout~/src/sys/arch/sparc64/dev/Attic/envctrl.c?rev=1.11=text/plain_with_tag=MAIN Thank you - that's useful information! Related to the magic offsets, I noticed that the value read from the sensor was always lower than the value written. However, it doesn't appear to be a constant difference (I see about 15 for most values, but only 5 for the minimum). I think that it's OK to keep the higher value - it'll mean that we run the fans slightly faster. The CPU temperature on my E250 doesn't reach the minimum threshold where the fan speed actually increases, so I don't think that this will have any impact. The GPIO values there helps to see what values I should be checking for too. It would be good to merge this with the current code, but I only have an E250 to test on, so I'll need to find a volunteer. Regards, Julian
Re: CVS commit: src/sys/arch/sparc64/dev
On Sat, 24 Oct 2020 15:16:39 + Julian Coleman wrote: > Module Name: src > Committed By: jdc > Date: Sat Oct 24 15:16:39 UTC 2020 > > Modified Files: > src/sys/arch/sparc64/dev: pcf8591_envctrl.c > > Log Message: > Add support for automatically changing the CPU fan speed on the E250 in a > similar way to the SB1000/SB2000. > The fan control information was determined by experiment, as it's only > partially available in OFW. > Hardcode the missing information for E250 fan control into the driver > (it should be possible to support the E450 in future too). If you're interested there is an older version[1] of envctrl in the Attic that might be relevant to use for reference. It supported fan speed controls on E450. IIRC I got some of the magic constants from OpenSolaris. Sadly I don't own an E450 any more. [1] cvsweb.netbsd.org/bsdweb.cgi/~checkout~/src/sys/arch/sparc64/dev/Attic/envctrl.c?rev=1.11=text/plain_with_tag=MAIN
Re: CVS commit: src/sys/arch/aarch64/aarch64
On Tue, Oct 13, 2020 at 12:57:44PM +0200, Kamil Rytarowski wrote: > > Log Message: > > BE32 binaries are no longer supported for ARMv7 and later, and > > therefore for aarch64eb. > > > > Reject them with ENOEXEC, rather than causing illegal instruction > > exceptions due to unexpected binary format. > Not supported in general or on NetBSD? Big Endian 32bit is supported on > Cavium ThunderX (at least on a selection of models if not all of them). The new supported format is BE-8 (BE-32 is the legacy format used for big endian arm upto v4). All newer arm either were LE only or supported BE-8 and little endian. Martin
Re: CVS commit: src/sys/arch/aarch64/aarch64
On 2020/10/13 19:57, Kamil Rytarowski wrote: On 13.10.2020 09:04, Rin Okuyama wrote: Module Name:src Committed By: rin Date: Tue Oct 13 07:04:49 UTC 2020 Modified Files: src/sys/arch/aarch64/aarch64: exec_machdep.c Log Message: BE32 binaries are no longer supported for ARMv7 and later, and therefore for aarch64eb. Reject them with ENOEXEC, rather than causing illegal instruction exceptions due to unexpected binary format. Not supported in general or on NetBSD? Big Endian 32bit is supported on Cavium ThunderX (at least on a selection of models if not all of them). BE32 does not mean "big endian 32bit binary". It is an old binary format for big endian 32-bit executables, that is supported up to ARMv6. ARMv7 and later only support a new binary format, BE8. BE8 binaries work without problem with COMPAT_NETBSD32 on aarch64eb. Thanks, rin
Re: CVS commit: src/sys/arch/aarch64/aarch64
On 13.10.2020 09:04, Rin Okuyama wrote: > Module Name: src > Committed By: rin > Date: Tue Oct 13 07:04:49 UTC 2020 > > Modified Files: > src/sys/arch/aarch64/aarch64: exec_machdep.c > > Log Message: > BE32 binaries are no longer supported for ARMv7 and later, and > therefore for aarch64eb. > > Reject them with ENOEXEC, rather than causing illegal instruction > exceptions due to unexpected binary format. > > Not supported in general or on NetBSD? Big Endian 32bit is supported on Cavium ThunderX (at least on a selection of models if not all of them). signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/sys/arch
Thank you very much! Thomas > On 06.10.2020, at 15:42, Christos Zoulas wrote: > > Module Name: src > Committed By: christos > Date: Tue Oct 6 13:42:03 UTC 2020 > > Modified Files: > src/sys/arch/aarch64/include: vmparam.h > src/sys/arch/amd64/include: vmparam.h > src/sys/arch/mips/include: vmparam.h > src/sys/arch/riscv/include: vmparam.h > src/sys/arch/sparc64/include: vmparam.h > > Log Message: > GC unused MAXTSIZ32 > > > To generate a diff of this commit: > cvs rdiff -u -r1.15 -r1.16 src/sys/arch/aarch64/include/vmparam.h > cvs rdiff -u -r1.52 -r1.53 src/sys/arch/amd64/include/vmparam.h > cvs rdiff -u -r1.63 -r1.64 src/sys/arch/mips/include/vmparam.h > cvs rdiff -u -r1.5 -r1.6 src/sys/arch/riscv/include/vmparam.h > cvs rdiff -u -r1.40 -r1.41 src/sys/arch/sparc64/include/vmparam.h > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. > > Modified files: > > Index: src/sys/arch/aarch64/include/vmparam.h > diff -u src/sys/arch/aarch64/include/vmparam.h:1.15 > src/sys/arch/aarch64/include/vmparam.h:1.16 > --- src/sys/arch/aarch64/include/vmparam.h:1.15 Wed Sep 23 01:02:27 2020 > +++ src/sys/arch/aarch64/include/vmparam.hTue Oct 6 09:42:03 2020 > @@ -1,4 +1,4 @@ > -/* $NetBSD: vmparam.h,v 1.15 2020/09/23 05:02:27 skrll Exp $ */ > +/* $NetBSD: vmparam.h,v 1.16 2020/10/06 13:42:03 christos Exp $ */ > > /*- > * Copyright (c) 2014 The NetBSD Foundation, Inc. > @@ -103,10 +103,6 @@ > > #define USRSTACK32VM_MAXUSER_ADDRESS32 > > -#ifndef MAXTSIZ32 > -#define MAXTSIZ32 (1L << 26) /* 32bit max text size (64MB) */ > -#endif > - > #ifndef MAXDSIZ32 > #define MAXDSIZ32 (3U*1024*1024*1024) /* max data size */ > #endif > > Index: src/sys/arch/amd64/include/vmparam.h > diff -u src/sys/arch/amd64/include/vmparam.h:1.52 > src/sys/arch/amd64/include/vmparam.h:1.53 > --- src/sys/arch/amd64/include/vmparam.h:1.52 Wed Jan 22 11:52:46 2020 > +++ src/sys/arch/amd64/include/vmparam.h Tue Oct 6 09:42:03 2020 > @@ -1,4 +1,4 @@ > -/* $NetBSD: vmparam.h,v 1.52 2020/01/22 16:52:46 ad Exp $ */ > +/* $NetBSD: vmparam.h,v 1.53 2020/10/06 13:42:03 christos Exp $*/ > > /*- > * Copyright (c) 1990 The Regents of the University of California. > @@ -106,7 +106,6 @@ > * 32bit memory related constants. > */ > > -#define MAXTSIZ32(256*1024*1024) > #ifndef DFLDSIZ32 > #define DFLDSIZ32 (256*1024*1024) /* initial data size > limit */ > #endif > > Index: src/sys/arch/mips/include/vmparam.h > diff -u src/sys/arch/mips/include/vmparam.h:1.63 > src/sys/arch/mips/include/vmparam.h:1.64 > --- src/sys/arch/mips/include/vmparam.h:1.63 Sun Jul 26 04:08:41 2020 > +++ src/sys/arch/mips/include/vmparam.h Tue Oct 6 09:42:03 2020 > @@ -1,4 +1,4 @@ > -/* $NetBSD: vmparam.h,v 1.63 2020/07/26 08:08:41 simonb Exp $ */ > +/* $NetBSD: vmparam.h,v 1.64 2020/10/06 13:42:03 christos Exp $*/ > > /* > * Copyright (c) 1988 University of Utah. > @@ -126,9 +126,6 @@ > /* > * Virtual memory related constants, all in bytes > */ > -#ifndef MAXTSIZ32 > -#define MAXTSIZ32 MAXTSIZ /* max text size */ > -#endif > #ifndef DFLDSIZ32 > #define DFLDSIZ32 DFLDSIZ /* initial data size > limit */ > #endif > > Index: src/sys/arch/riscv/include/vmparam.h > diff -u src/sys/arch/riscv/include/vmparam.h:1.5 > src/sys/arch/riscv/include/vmparam.h:1.6 > --- src/sys/arch/riscv/include/vmparam.h:1.5 Sat Jun 1 08:42:28 2019 > +++ src/sys/arch/riscv/include/vmparam.h Tue Oct 6 09:42:03 2020 > @@ -1,4 +1,4 @@ > -/* $NetBSD: vmparam.h,v 1.5 2019/06/01 12:42:28 maxv Exp $ */ > +/* $NetBSD: vmparam.h,v 1.6 2020/10/06 13:42:03 christos Exp $ */ > > /*- > * Copyright (c) 2014 The NetBSD Foundation, Inc. > @@ -79,9 +79,6 @@ > /* > * Virtual memory related constants, all in bytes > */ > -#ifndef MAXTSIZ32 > -#define MAXTSIZ32 MAXTSIZ /* max text size */ > -#endif > #ifndef DFLDSIZ32 > #define DFLDSIZ32 DFLDSIZ /* initial data size > limit */ > #endif > > Index: src/sys/arch/sparc64/include/vmparam.h > diff -u src/sys/arch/sparc64/include/vmparam.h:1.40 > src/sys/arch/sparc64/include/vmparam.h:1.41 > --- src/sys/arch/sparc64/include/vmparam.h:1.40 Wed Jan 22 11:59:37 2020 > +++ src/sys/arch/sparc64/include/vmparam.hTue Oct 6 09:42:03 2020 > @@ -1,4 +1,4 @@ > -/* $NetBSD: vmparam.h,v 1.40 2020/01/22 16:59:37 ad Exp $ */ > +/* $NetBSD: vmparam.h,v 1.41 2020/10/06 13:42:03 christos Exp $ */ > > /* > * Copyright (c) 1992, 1993 > @@ -155,9 +155,6 @@ > /* > * 32-bit emulation limits (same as sparc - we could go bigger) > */ > -#ifndef MAXTSIZ32 > -#define MAXTSIZ32 (64*1024*1024) /* max text size */ > -#endif > #ifndef DFLDSIZ32 > #define DFLDSIZ32 (64*1024*1024) /* initial data
Re: CVS commit: src/sys/arch/aarch64/aarch64
It works fine now. Thank you for quick fix!! rin On 2020/10/06 15:28, Nick Hudson wrote: On 06/10/2020 01:54, Rin Okuyama wrote: Hi, On 2020/10/01 1:35, Nick Hudson wrote: Module Name: src Committed By: skrll Date: Wed Sep 30 16:35:49 UTC 2020 Modified Files: src/sys/arch/aarch64/aarch64: cpuswitch.S vectors.S Log Message: Move el[01]_trap_exit into vectors.S where the callers exist To generate a diff of this commit: cvs rdiff -u -r1.27 -r1.28 src/sys/arch/aarch64/aarch64/cpuswitch.S cvs rdiff -u -r1.18 -r1.19 src/sys/arch/aarch64/aarch64/vectors.S Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. This commit seems to break COMPAT_NETBSD32. On RPI3, earmv7hf binaries get SIGSEGV: Hopefully fixed now with the commit below. Please let me know if you have any other problems. Sorry for the breakage. Nick Module Name: src Committed By: skrll Date: Tue Oct 6 06:26:46 UTC 2020 Modified Files: src/sys/arch/aarch64/aarch64: cpuswitch.S vectors.S Log Message: move #include "opt_compat_netbsd32.h" to where it's required To generate a diff of this commit: cvs rdiff -u -r1.28 -r1.29 src/sys/arch/aarch64/aarch64/cpuswitch.S cvs rdiff -u -r1.19 -r1.20 src/sys/arch/aarch64/aarch64/vectors.S
Re: CVS commit: src/sys/arch/aarch64/aarch64
On 06/10/2020 01:54, Rin Okuyama wrote: Hi, On 2020/10/01 1:35, Nick Hudson wrote: Module Name: src Committed By: skrll Date: Wed Sep 30 16:35:49 UTC 2020 Modified Files: src/sys/arch/aarch64/aarch64: cpuswitch.S vectors.S Log Message: Move el[01]_trap_exit into vectors.S where the callers exist To generate a diff of this commit: cvs rdiff -u -r1.27 -r1.28 src/sys/arch/aarch64/aarch64/cpuswitch.S cvs rdiff -u -r1.18 -r1.19 src/sys/arch/aarch64/aarch64/vectors.S Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. This commit seems to break COMPAT_NETBSD32. On RPI3, earmv7hf binaries get SIGSEGV: Hopefully fixed now with the commit below. Please let me know if you have any other problems. Sorry for the breakage. Nick Module Name:src Committed By: skrll Date: Tue Oct 6 06:26:46 UTC 2020 Modified Files: src/sys/arch/aarch64/aarch64: cpuswitch.S vectors.S Log Message: move #include "opt_compat_netbsd32.h" to where it's required To generate a diff of this commit: cvs rdiff -u -r1.28 -r1.29 src/sys/arch/aarch64/aarch64/cpuswitch.S cvs rdiff -u -r1.19 -r1.20 src/sys/arch/aarch64/aarch64/vectors.S
Re: CVS commit: src/sys/arch/aarch64/aarch64
Hi, On 2020/10/01 1:35, Nick Hudson wrote: Module Name:src Committed By: skrll Date: Wed Sep 30 16:35:49 UTC 2020 Modified Files: src/sys/arch/aarch64/aarch64: cpuswitch.S vectors.S Log Message: Move el[01]_trap_exit into vectors.S where the callers exist To generate a diff of this commit: cvs rdiff -u -r1.27 -r1.28 src/sys/arch/aarch64/aarch64/cpuswitch.S cvs rdiff -u -r1.18 -r1.19 src/sys/arch/aarch64/aarch64/vectors.S Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. This commit seems to break COMPAT_NETBSD32. On RPI3, earmv7hf binaries get SIGSEGV: --- # uname -a NetBSD 9.99.73 NetBSD 9.99.73 (GENERIC64) #33: Mon Oct 5 23:26:29 JST 2020 rin@latipes:/sys/arch/evbarm/compile/GENERIC64 evbarm # file /emul/netbsd32/bin/sh /emul/netbsd32/bin/sh: ELF 32-bit LSB pie executable, ARM, EABI5 version 1 (SYSV), dynamically linked, interpreter /libexec/ld.elf_so, for NetBSD 9.99.73, compiled for: earmv7hf, not stripped # /emul/netbsd32/bin/sh [1] Segmentation fault (core dumped) /emul/netbsd32/bin/sh # ktrace /emul/netbsd32/bin/sh [1] Segmentation fault (core dumped) ktrace /emul/netbsd32/bin/sh # kdump | tail 171171 sh CALL netbsd32_break(0xb74ffdc) 171171 sh RET netbsd32_break 0 171171 sh CALL netbsd32___clock_gettime50(3,0xfff997f8) 171171 sh RET netbsd32___clock_gettime50 0 171171 sh CALL netbsd32___clock_gettime50(3,0xfff997f8) 171171 sh RET netbsd32___clock_gettime50 0 171171 sh CALL netbsd32___clock_gettime50(3,0xfff99810) 171171 sh RET netbsd32___clock_gettime50 0 171171 sh PSIG SIGSEGV SIG_DFL: code=SEGV_MAPERR, addr=0x8, trap=-1845493754) 171171 sh NAMI "sh.core" --- Full kdump is provided here: http://www.netbsd.org/~rin/aarch64_netbsd32_kdump_20201006.txt By reverting this commit, arm32 binaries become working again. Can you take a look please? Thanks, rin
Re: CVS commit: src/sys/arch/aarch64
>> Index: src/sys/arch/aarch64/include/cpu.h >> diff -u src/sys/arch/aarch64/include/cpu.h:1.27 src/sys/arch/aarch64/inc= >lude/cpu.h:1.28 >> --- src/sys/arch/aarch64/include/cpu.h:1.27 Mon Sep 14 10:06:35 2020 >> +++ src/sys/arch/aarch64/include/cpu.h Thu Oct 1 06:40:16 2020 >> @@ -1,4 +1,4 @@ >> -/* $NetBSD: cpu.h,v 1.27 2020/09/14 10:06:35 ryo Exp $ */ >> +/* $NetBSD: cpu.h,v 1.28 2020/10/01 06:40:16 ryo Exp $ */ >> >> /*- >>* Copyright (c) 2014, 2020 The NetBSD Foundation, Inc. >> @@ -156,7 +156,7 @@ void cpu_hatch(struct cpu_info *); >> extern struct cpu_info *cpu_info[]; >> extern struct cpu_info cpu_info_store[]; >> >> -#define CPU_INFO_ITERATOR cpuid_t >> +#define CPU_INFO_ITERATOR int >> #if defined(MULTIPROCESSOR) || defined(_MODULE) >> #define cpu_number() (curcpu()->ci_index) >> #define CPU_IS_PRIMARY(ci) ((ci)->ci_index =3D=3D 0) >> > >I think this is the wrong way to go ultimately > >unsigned int at least In most arch, CPU_INFO_ITERATOR is an int. # grep CPU_INFO_ITERATOR /usr/src/sys/arch/*/include/cpu.h /usr/src/sys/arch/aarch64/include/cpu.h:#define CPU_INFO_ITERATOR int /usr/src/sys/arch/alpha/include/cpu.h:#define CPU_INFO_ITERATOR int __unused /usr/src/sys/arch/arm/include/cpu.h:#define CPU_INFO_ITERATOR int /usr/src/sys/arch/hppa/include/cpu.h:#define CPU_INFO_ITERATOR int /usr/src/sys/arch/ia64/include/cpu.h:#define CPU_INFO_ITERATOR int __unused /usr/src/sys/arch/mips/include/cpu.h:#define CPU_INFO_ITERATOR int /usr/src/sys/arch/mips/include/cpu.h:#define CPU_INFO_ITERATOR int __unused /usr/src/sys/arch/or1k/include/cpu.h:#define CPU_INFO_ITERATORcpuid_t /usr/src/sys/arch/powerpc/include/cpu.h:#define CPU_INFO_ITERATOR int /usr/src/sys/arch/powerpc/include/cpu.h:#define CPU_INFO_ITERATOR int /usr/src/sys/arch/riscv/include/cpu.h:#define CPU_INFO_ITERATOR cpuid_t /usr/src/sys/arch/sparc64/include/cpu.h:#define CPU_INFO_ITERATOR int __unused /usr/src/sys/arch/vax/include/cpu.h:#define CPU_INFO_ITERATOR int __unused /usr/src/sys/arch/x86/include/cpu.h:#define CPU_INFO_ITERATOR int __unused and, it is compared to "int ncpu", so it is matched to the type of 'ncpu'. (otherwise, clang warns) -- ryo shimizu
Re: CVS commit: src/sys/arch/aarch64
On 01/10/2020 07:40, Ryo Shimizu wrote: Module Name:src Committed By: ryo Date: Thu Oct 1 06:40:16 UTC 2020 Modified Files: src/sys/arch/aarch64/aarch64: procfs_machdep.c src/sys/arch/aarch64/include: cpu.h Log Message: fix build error with LLVM [...] Index: src/sys/arch/aarch64/include/cpu.h diff -u src/sys/arch/aarch64/include/cpu.h:1.27 src/sys/arch/aarch64/include/cpu.h:1.28 --- src/sys/arch/aarch64/include/cpu.h:1.27 Mon Sep 14 10:06:35 2020 +++ src/sys/arch/aarch64/include/cpu.h Thu Oct 1 06:40:16 2020 @@ -1,4 +1,4 @@ -/* $NetBSD: cpu.h,v 1.27 2020/09/14 10:06:35 ryo Exp $ */ +/* $NetBSD: cpu.h,v 1.28 2020/10/01 06:40:16 ryo Exp $ */ /*- * Copyright (c) 2014, 2020 The NetBSD Foundation, Inc. @@ -156,7 +156,7 @@ voidcpu_hatch(struct cpu_info *); extern struct cpu_info *cpu_info[]; extern struct cpu_info cpu_info_store[]; -#define CPU_INFO_ITERATOR cpuid_t +#define CPU_INFO_ITERATOR int #if defined(MULTIPROCESSOR) || defined(_MODULE) #define cpu_number() (curcpu()->ci_index) #define CPU_IS_PRIMARY(ci)((ci)->ci_index == 0) I think this is the wrong way to go ultimately unsigned int at least Nick
Re: CVS commit: src/sys/arch/aarch64/aarch64
>On 24/09/2020 10:04, Ryo Shimizu wrote: >> Module Name: src >> Committed By:ryo >> Date:Thu Sep 24 09:04:38 UTC 2020 >> >> Modified Files: >> src/sys/arch/aarch64/aarch64: bus_space_asm_generic.S >> >> Log Message: >> fix bugs in *_bs_rm_8_swap(). it was only reading 4 bytes, not 8 bytes. > >I think there's another one that needs fixing... > >Nick >Index: sys/arch/aarch64/aarch64/bus_space_asm_generic.S >=== >RCS file: /cvsroot/src/sys/arch/aarch64/aarch64/bus_space_asm_generic.S,v >retrieving revision 1.3 >diff -u -p -r1.3 bus_space_asm_generic.S >--- sys/arch/aarch64/aarch64/bus_space_asm_generic.S 24 Sep 2020 09:04:38 >- 1.3 >+++ sys/arch/aarch64/aarch64/bus_space_asm_generic.S 24 Sep 2020 10:11:18 >- >@@ -225,7 +225,7 @@ ENTRY_NP(\funcname\()_bs_rm_4_swap) > ldr w8, [x0, #BS_STRIDE] > lsl x8, x2, x8 /* offset <<= tag->bs_stride */ > 1: >- ldrhw9, [x1, x8] >+ ldr w9, [x1, x8] > subsx4, x4, #1 /* count-- */ > rev w9, w9 > str w9, [x3], #4 Ahhh, right. I fixed this too. Thanks again. -- ryo shimizu
Re: CVS commit: src/sys/arch/aarch64/aarch64
On 24/09/2020 10:04, Ryo Shimizu wrote: Module Name:src Committed By: ryo Date: Thu Sep 24 09:04:38 UTC 2020 Modified Files: src/sys/arch/aarch64/aarch64: bus_space_asm_generic.S Log Message: fix bugs in *_bs_rm_8_swap(). it was only reading 4 bytes, not 8 bytes. I think there's another one that needs fixing... Nick Index: sys/arch/aarch64/aarch64/bus_space_asm_generic.S === RCS file: /cvsroot/src/sys/arch/aarch64/aarch64/bus_space_asm_generic.S,v retrieving revision 1.3 diff -u -p -r1.3 bus_space_asm_generic.S --- sys/arch/aarch64/aarch64/bus_space_asm_generic.S 24 Sep 2020 09:04:38 - 1.3 +++ sys/arch/aarch64/aarch64/bus_space_asm_generic.S 24 Sep 2020 10:11:18 - @@ -225,7 +225,7 @@ ENTRY_NP(\funcname\()_bs_rm_4_swap) ldr w8, [x0, #BS_STRIDE] lsl x8, x2, x8 /* offset <<= tag->bs_stride */ 1: - ldrh w9, [x1, x8] + ldr w9, [x1, x8] subs x4, x4, #1 /* count-- */ rev w9, w9 str w9, [x3], #4
Re: CVS commit: src/sys/arch/alpha/include
> On Sep 3, 2020, at 1:14 PM, matthew green wrote: > > "Jason R Thorpe" writes: >> Module Name: src >> Committed By:thorpej >> Date:Thu Sep 3 04:20:54 UTC 2020 >> >> Modified Files: >> src/sys/arch/alpha/include: cpu.h >> >> Log Message: >> Garabage-collect curpcb / cpu_info::ci_curpcb. > > does alpha have modules? this may be a ABI change needing > a kernel version bump... "Sort of." They don't work (not all of the required relocations are handled correctly), so I'm not that concerned. -- thorpej
re: CVS commit: src/sys/arch/alpha/include
"Jason R Thorpe" writes: > Module Name: src > Committed By: thorpej > Date: Thu Sep 3 04:20:54 UTC 2020 > > Modified Files: > src/sys/arch/alpha/include: cpu.h > > Log Message: > Garabage-collect curpcb / cpu_info::ci_curpcb. does alpha have modules? this may be a ABI change needing a kernel version bump... .mrg.
Re: CVS commit: src/sys/arch
On 2020/08/11 10:24, matthew green wrote: XXX Apply similar fixes to other m68k ports. yes...but also, a long-term project to consolidate all the almost-identical m68k code copied into each port would avoid this probably :-) Agreed. Also, I've found many dead codes in amiga/locore.s. I'd like to clean them up little by little. Thanks, rin
re: CVS commit: src/sys/arch
thanks! > XXX > Apply similar fixes to other m68k ports. yes...but also, a long-term project to consolidate all the almost-identical m68k code copied into each port would avoid this probably :-)
Re: CVS commit: src/sys/arch/amiga/amiga
Hi, Sorry for the serious delay in my response. On 2020/07/22 13:37, matthew green wrote: thanks for getting more m68k working! Thanks! "Rin Okuyama" writes: Module Name:src Committed By: rin Date: Tue Jul 21 06:39:31 UTC 2020 Modified Files: src/sys/arch/amiga/amiga: locore.s Log Message: Align tmpstk to 4-byte boundary in the same manner as mac68k. However, unfortunately, this does not fix strange crashes of GCC8-compiled kernel, for which I cannot even enter DDB nor obtain crash dump. We need further investigation... boo. can you update the README.gcc8 file (external/gpl3/gcc), for the m68k: line on L87 or so, and add 'y' for the known working kernels, and 'n' for the failed ones. Finally, I managed GCC8 to work for amiga kernel! Can you please take a look into port-m68k/6 ? http://gnats.netbsd.org/6 I will update the table when the patch in the PR and related ones are committed. After that, I think m68k ports can be switched to GCC8. Thanks, rin
Re: CVS commit: src/sys/arch/x86
> Module Name:src
> Committed By: jdolecek
> Date: Sat Aug 1 12:36:36 UTC 2020
>
> Modified Files:
> src/sys/arch/x86/pci: pci_intr_machdep.c
> src/sys/arch/x86/x86: mainbus.c
>
> Log Message:
> reorder includes to pull __HAVE_PCI_MSI_MSIX properly via
>
If requires a file to be included
("opt_pci.h"?) in order for its definition of __HAVE_PCI_MSI_MSIX to
work, why doesn't include that file
directly?
Or, if I didn't follow exactly what's going on here, why is it
necessary to reorder the includes?
I think we should generally treat it as a bug if including two header
files in different orders gives different outcomes, and fix it by
fixing the header files rather than by adjusting the ordering of
includes in every file where they're used.
re: CVS commit: src/sys/arch/amiga/amiga
thanks for getting more m68k working! "Rin Okuyama" writes: > Module Name: src > Committed By: rin > Date: Tue Jul 21 06:39:31 UTC 2020 > > Modified Files: > src/sys/arch/amiga/amiga: locore.s > > Log Message: > Align tmpstk to 4-byte boundary in the same manner as mac68k. > > However, unfortunately, this does not fix strange crashes of GCC8-compiled > kernel, for which I cannot even enter DDB nor obtain crash dump. > > We need further investigation... boo. can you update the README.gcc8 file (external/gpl3/gcc), for the m68k: line on L87 or so, and add 'y' for the known working kernels, and 'n' for the failed ones. thanks. .mrg.
Re: CVS commit: src/sys/arch/powerpc/booke
On 2020/07/07 9:51, Jason Thorpe wrote: On Jul 6, 2020, at 5:28 PM, Rin Okuyama wrote: Module Name:src Committed By: rin Date: Tue Jul 7 00:28:31 UTC 2020 Modified Files: src/sys/arch/powerpc/booke: e500_tlb.c Log Message: Fix kernel panic due to tmpfs. pmap for booke assumes that the ``va'' argument for pmap_kenter_pa(9) is page-aligned. However, by recent changes, tmpfs became to use ``va'' with page offset via ubc_uiomove(9). So, truncate it to page boundary. This change seems wrong. I think it needs to be fixed in tmpfs. Thank you for your comment. OK, I will revert this and send PR. rin
Re: CVS commit: src/sys/arch/powerpc/booke
> On Jul 6, 2020, at 5:28 PM, Rin Okuyama wrote: > > Module Name: src > Committed By: rin > Date: Tue Jul 7 00:28:31 UTC 2020 > > Modified Files: > src/sys/arch/powerpc/booke: e500_tlb.c > > Log Message: > Fix kernel panic due to tmpfs. > > pmap for booke assumes that the ``va'' argument for pmap_kenter_pa(9) is > page-aligned. However, by recent changes, tmpfs became to use ``va'' with > page offset via ubc_uiomove(9). So, truncate it to page boundary. This change seems wrong. I think it needs to be fixed in tmpfs. -- thorpej
Re: CVS commit: src/sys/arch/aarch64/aarch64
I think this will have issues on some big.LITTLE configurations like Rockchip RK3399. In the RK3399 case cpu[0-3] is VIPT I$ and cpu[4-5] is PIPT I$. Boot order of secondaries is not guaranteed so it is possible to get different values of aarch64_cache_vindexsize from one boot to the next. Jared On Thu, 2 Jul 2020, Rin Okuyama wrote: Module Name:src Committed By: rin Date: Thu Jul 2 12:59:31 UTC 2020 Modified Files: src/sys/arch/aarch64/aarch64: pmap.c Log Message: Set uvmexp.ncolors appropriately, which is required for some CPU models with VIPT icache. Otherwise, alias in virtual address results in inconsistent results, at least for applications that rewrite text of other process, e.g., GDB for arm32. Also, this hopefully fixes other unexpected failures due to alias. Confirmed that there's no observable regression in performance; difference in ``time make -j8'' for GENERIC64 kernel on BCM2837 with and without setting uvmexp.ncolors is within 0.1%. Thanks to ryo@ for discussion. To generate a diff of this commit: cvs rdiff -u -r1.80 -r1.81 src/sys/arch/aarch64/aarch64/pmap.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/arch/ia64/stand/ia64/efi
Martin Husemann writes: > > That build pre-dates Luke's fix. > Ahh, you are correct and it built fine for me, apologies. Thanks
Re: CVS commit: src/sys/arch/ia64/stand/ia64/efi
On Thu, Jul 02, 2020 at 08:03:09AM -0700, scole_mail wrote: > That change didn't work: > > http://releng.netbsd.org/builds/HEAD/202007020210Z/ia64.build.failed > ... > nbmake[10]: nbmake[10]: don't know how to make loader.efi.c. Stop That build pre-dates Luke's fix. Martin
Re: CVS commit: src/sys/arch/ia64/stand/ia64/efi
"Luke Mewburn" writes: > Module Name: src > Committed By: lukem > Date: Thu Jul 2 09:07:25 UTC 2020 > > Modified Files: > src/sys/arch/ia64/stand/ia64/efi: Makefile > > Log Message: > loader.efi doesn't have source > > (Untested fix) > > > To generate a diff of this commit: > cvs rdiff -u -r1.6 -r1.7 src/sys/arch/ia64/stand/ia64/efi/Makefile > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. That change didn't work: http://releng.netbsd.org/builds/HEAD/202007020210Z/ia64.build.failed ... nbmake[10]: nbmake[10]: don't know how to make loader.efi.c. Stop
Re: CVS commit: src/sys/arch/powerpc/include
Perhaps yes, but I'm not sure. How do you mips guys think?
Thanks,
rin
On 2020/06/27 19:08, Jaromír Doleček wrote:
Perhaps we can use a similar technique for mips too? There also the
kernel actually always uses a compile-time fixed page size AFAICS.
Jaromir
Le sam. 27 juin 2020 à 04:51, Rin Okuyama a écrit :
Module Name:src
Committed By: rin
Date: Sat Jun 27 02:51:23 UTC 2020
Modified Files:
src/sys/arch/powerpc/include: vmparam.h
Log Message:
Restrict {MIN,MAX}_PAGE_SIZE for MODULAR || _MODULE, which makes
non-MODULAR kernel a little bit efficient.
They are also exposed to userland for jemalloc.
To generate a diff of this commit:
cvs rdiff -u -r1.22 -r1.23 src/sys/arch/powerpc/include/vmparam.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/arch/powerpc/include
Perhaps we can use a similar technique for mips too? There also the
kernel actually always uses a compile-time fixed page size AFAICS.
Jaromir
Le sam. 27 juin 2020 à 04:51, Rin Okuyama a écrit :
>
> Module Name:src
> Committed By: rin
> Date: Sat Jun 27 02:51:23 UTC 2020
>
> Modified Files:
> src/sys/arch/powerpc/include: vmparam.h
>
> Log Message:
> Restrict {MIN,MAX}_PAGE_SIZE for MODULAR || _MODULE, which makes
> non-MODULAR kernel a little bit efficient.
>
> They are also exposed to userland for jemalloc.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.22 -r1.23 src/sys/arch/powerpc/include/vmparam.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/arch/mips/cavium/dev
Hi Rin, Rin Okuyama wrote: > Hi, > > On 2020/06/23 14:18, Simon Burge wrote: > > Module Name:src > > Committed By: simonb > > Date: Tue Jun 23 05:18:43 UTC 2020 > > > > Modified Files: > > src/sys/arch/mips/cavium/dev: octeon_uart.c > > > > Log Message: > > Add support for a very simple output-only console so early printf() can > > work. > > Minor tweaks, remove some unused code. > > > > > > To generate a diff of this commit: > > cvs rdiff -u -r1.8 -r1.9 src/sys/arch/mips/cavium/dev/octeon_uart.c > > Didn't you forget to ``cvs add octeon_uartvar.h''? Periodic build fails as: Yep, indeed! Added, thanks for pointing this out. Cheers, Simon.
Re: CVS commit: src/sys/arch/mips/cavium/dev
Hi, On 2020/06/23 14:18, Simon Burge wrote: Module Name:src Committed By: simonb Date: Tue Jun 23 05:18:43 UTC 2020 Modified Files: src/sys/arch/mips/cavium/dev: octeon_uart.c Log Message: Add support for a very simple output-only console so early printf() can work. Minor tweaks, remove some unused code. To generate a diff of this commit: cvs rdiff -u -r1.8 -r1.9 src/sys/arch/mips/cavium/dev/octeon_uart.c Didn't you forget to ``cvs add octeon_uartvar.h''? Periodic build fails as: |/home/source/ab/HEAD/src/sys/arch/mips/cavium/dev/octeon_uart.c:48:10: fatal error: mips/cavium/dev/octeon_uartvar.h: No such file or directory | #include | ^~ Thanks, rin
re: CVS commit: src/sys/arch/x86/x86
"Jaromir Dolecek" writes: > Module Name: src > Committed By: jdolecek > Date: Wed Jun 24 22:28:08 UTC 2020 > > Modified Files: > src/sys/arch/x86/x86: multiboot2.c > > Log Message: > don't try allocating 16KB of scratch space on stack > > it's too early for kmem_alloc(), so use static variable in BSS; it's used > post reloc, so don't need to use the RELOC() macros > > XXX compile-tested only on i386 this one is quite a lot of lost wasted space. can you find a way to reuse these pages? or use the x86-specific ways to get early memory... thanks. .mrg.
Re: CVS commit: src/sys/arch/x86/x86
On Wed, Jun 24, 2020 at 10:28:08PM +, Jaromir Dolecek wrote: > Module Name: src > Committed By: jdolecek > Date: Wed Jun 24 22:28:08 UTC 2020 > > Modified Files: > src/sys/arch/x86/x86: multiboot2.c > > Log Message: > don't try allocating 16KB of scratch space on stack > > it's too early for kmem_alloc(), so use static variable in BSS; it's used > post reloc, so don't need to use the RELOC() macros Why? This is the initial early stack, not the normal LWP stack. Joerg
Re: CVS commit: src/sys/arch/xen/xen
> Module Name:src > Committed By: jdolecek > Date: Fri Apr 10 18:03:06 UTC 2020 > > Modified Files: > src/sys/arch/xen/xen: if_xennet_xenbus.c > > Log Message: > convert to bus_dma(9), remove now not necessary XENPVHVM redefines > [...] > + KASSERT(req->rxreq_gntref = GRANT_INVALID_REF); I don't think this does quite what you meant it to do! I can fix it by adding the missing `=', but it may cause the kassert to fire because I don't see anywhere in xennet_rx_free_req that sets it to GRANT_INVALID_REF and I don't know the surrounding logic well enough to be confident that just changing it there is correct.
Re: CVS commit: src/sys/arch/powerpc
On Mon, Jun 22, 2020 at 05:34:57AM +, Rin Okuyama wrote: > Module Name: src > Committed By: rin > Date: Mon Jun 22 05:34:57 UTC 2020 > > Modified Files: > src/sys/arch/powerpc/include: mcontext.h types.h > src/sys/arch/powerpc/powerpc: sig_machdep.c > > Log Message: > Fix previous; hide userland ABI details for kernel as suggested by joerg: Thanks! Joerg
Re: CVS commit: src/sys/arch/powerpc
On 2020/06/22 5:10, Joerg Sonnenberger wrote: On Sun, Jun 21, 2020 at 12:40:00AM +, Rin Okuyama wrote: - Obsolete __lwp_settcb() in order to let kernel know new TLS address via _lwp_setprivate(2). Alternatively, we can call _lwp_setprivate(2) within __lwp_settcb() like mips, but it is just double handling; we adjust %r2 appropriately in _lwp_setprivate(2) via cpu_lwp_setprivate(). If an architecture stores the TCB at an offset, it should provide __lwp_settcb to do the necessary adjust and then invoke _lwp_setprivate as appropiate. The MIPS variant is correct. This should *not* be done in the kernel, as it is an ABI detail of the userland TLS API. Thank you for your comment. I agree. Tt is cleaner not to bring in userland ABI details into kernel. OK, I will fix it differently. Thanks, rin
Re: CVS commit: src/sys/arch/powerpc
On Sun, Jun 21, 2020 at 12:40:00AM +, Rin Okuyama wrote: > - Obsolete __lwp_settcb() in order to let kernel know new TLS address via > _lwp_setprivate(2). Alternatively, we can call _lwp_setprivate(2) within > __lwp_settcb() like mips, but it is just double handling; we adjust %r2 > appropriately in _lwp_setprivate(2) via cpu_lwp_setprivate(). If an architecture stores the TCB at an offset, it should provide __lwp_settcb to do the necessary adjust and then invoke _lwp_setprivate as appropiate. The MIPS variant is correct. This should *not* be done in the kernel, as it is an ABI detail of the userland TLS API. Joerg
Re: CVS commit: src/sys/arch/mips/mips
Simon Burge wrote: > > > Module Name: src > > > Committed By: simonb > > > Date: Tue Jun 9 06:18:01 UTC 2020 > > > > > > Modified Files: > > > src/sys/arch/mips/mips: mips_machdep.c > > > > > > Log Message: > > > If we are on a SiByte or Cavium CPU with an FPU, report as "built-in FPU" > > > instead of saying it's an unknown FPU type. > > > > > > XXX - add any other CPUs to this list? > > > > This seems to cause build errors for non mipsNN: > > Oops, will fix. Thanks for reporting. Fixed, thanks! Cheers, Simon.
Re: CVS commit: src/sys/arch/mips/mips
Izumi Tsutsui wrote: > > Module Name:src > > Committed By: simonb > > Date: Tue Jun 9 06:18:01 UTC 2020 > > > > Modified Files: > > src/sys/arch/mips/mips: mips_machdep.c > > > > Log Message: > > If we are on a SiByte or Cavium CPU with an FPU, report as "built-in FPU" > > instead of saying it's an unknown FPU type. > > > > XXX - add any other CPUs to this list? > > This seems to cause build errors for non mipsNN: Oops, will fix. Thanks for reporting. Cheers, Simon.
Re: CVS commit: src/sys/arch/mips/mips
> Module Name: src > Committed By: simonb > Date: Tue Jun 9 06:18:01 UTC 2020 > > Modified Files: > src/sys/arch/mips/mips: mips_machdep.c > > Log Message: > If we are on a SiByte or Cavium CPU with an FPU, report as "built-in FPU" > instead of saying it's an unknown FPU type. > > XXX - add any other CPUs to this list? This seems to cause build errors for non mipsNN: --- # compile RAMDISK/mips_machdep.o /s/cvs/src/obj.ews4800mips/tooldir.NetBSD-9.0-i386/bin/mipseb--netbsd-gcc -G 0 -mno-abicalls -msoft-float -ffixed-24 -ffreestanding -fno-zero-initialized-in-bss -fno-delete-null-pointer-checks -Os -mmemcpy -fno-strict-aliasing -fno-common -std=gnu99 -Werror -Wall -Wno-main -Wno-format-zero-length -Wpointer-arith -Wmissing-prototypes -Wstrict-prototypes -Wold-style-definition -Wswitch -Wshadow -Wcast-qual -Wwrite-strings -Wno-unreachable-code -Wno-pointer-sign -Wno-attributes -Wno-sign-compare -march=r4400 -mabi=32 --sysroot=/s/cvs/src/obj.ews4800mips/destdir.ews4800mips -Dews4800mips -I. -I/s/cvs/src/sys/external/bsd/libnv/dist -I/s/cvs/src/sys/../common/lib/libx86emu -I/s/cvs/src/sys/../common/lib/libc/misc -I/s/cvs/src/sys/../common/include -I/s/cvs/src/sys/arch -I/s/cvs/src/sys -nostdinc -DCOMPAT_UTILS -DMIPS3 -DMIPS3_ENABLE_CLOCK_INTR -DCOMPAT_44 -D_KERNEL -D_KERNEL_OPT -std=gnu99 -I/s/cvs/src/sys/lib/libkern/../../../common/lib/libc/quad -I/s/cvs/src/sys/lib/libkern/../! ../../common/lib/libc/string -I/s/cvs/src/sys/lib/libkern/../../../common/lib/libc/arch/mips/string -I/s/cvs/src/sys/lib/libkern/../../../common/lib/libc/hash/sha3 -I/s/cvs/src/sys/external/bsd/libnv/dist -c /s/cvs/src/sys/arch/mips/mips/mips_machdep.c -o mips_machdep.o /s/cvs/src/sys/arch/mips/mips/mips_machdep.c: In function 'cpu_identify': /s/cvs/src/sys/arch/mips/mips/mips_machdep.c:1508:11: error: implicit declaration of function 'mipsNN_cp0_config1_read'; did you mean 'mips3_cp0_config_read'? [-Werror=implicit-function-declaration] cfg1 = mipsNN_cp0_config1_read(); ^~~ mips3_cp0_config_read /s/cvs/src/sys/arch/mips/mips/mips_machdep.c:1509:15: error: 'MIPSNN_CFG1_FP' undeclared (first use in this function); did you mean 'MIPS_CR_IP'? if (cfg1 & MIPSNN_CFG1_FP) ^~ MIPS_CR_IP /s/cvs/src/sys/arch/mips/mips/mips_machdep.c:1509:15: note: each undeclared identifier is reported only once for each function it appears in cc1: all warnings being treated as errors *** Failed target: mips_machdep.o *** Failed command: echo '# ' "compile RAMDISK/mips_machdep.o" && echo /s/cvs/src/obj.ews4800mips/tooldir.NetBSD-9.0-i386/bin/mipseb--netbsd-gcc -G 0 -mno-abicalls -msoft-float -ffixed-24 -ffreestanding -fno-zero-initialized-in-bss -fno-delete-null-pointer-checks -Os -mmemcpy -fno-strict-aliasing -fno-common -std=gnu99 -Werror -Wall -Wno-main -Wno-format-zero-length -Wpointer-arith -Wmissing-prototypes -Wstrict-prototypes -Wold-style-definition -Wswitch -Wshadow -Wcast-qual -Wwrite-strings -Wno-unreachable-code -Wno-pointer-sign -Wno-attributes -Wno-sign-compare -march=r4400 -mabi=32 --sysroot=/s/cvs/src/obj.ews4800mips/destdir.ews4800mips -Dews4800mips -I. -I/s/cvs/src/sys/external/bsd/libnv/dist -I/s/cvs/src/sys/../common/lib/libx86emu -I/s/cvs/src/sys/../common/lib/libc/misc -I/s/cvs/src/sys/../common/include -I/s/cvs/src/sys/arch -I/s/cvs/src/sys -nostdinc -DCOMPAT_UTILS -DMIPS3 -DMIPS3_ENABLE_CLOCK_INTR -DCOMPAT_44 -D_KERNEL -D_KERNEL_OPT -std=gnu99 -I/s/cvs/src/sys/lib! /libkern/../../../common/lib/libc/quad -I/s/cvs/src/sys/lib/libkern/../../../common/lib/libc/string -I/s/cvs/src/sys/lib/libkern/../../../common/lib/libc/arch/mips/string -I/s/cvs/src/sys/lib/libkern/../../../common/lib/libc/hash/sha3 -I/s/cvs/src/sys/external/bsd/libnv/dist -c /s/cvs/src/sys/arch/mips/mips/mips_machdep.c -o mips_machdep.o && /s/cvs/src/obj.ews4800mips/tooldir.NetBSD-9.0-i386/bin/mipseb--netbsd-gcc -G 0 -mno-abicalls -msoft-float -ffixed-24 -ffreestanding -fno-zero-initialized-in-bss -fno-delete-null-pointer-checks -Os -mmemcpy -fno-strict-aliasing -fno-common -std=gnu99 -Werror -Wall -Wno-main -Wno-format-zero-length -Wpointer-arith -Wmissing-prototypes -Wstrict-prototypes -Wold-style-definition -Wswitch -Wshadow -Wcast-qual -Wwrite-strings -Wno-unreachable-code -Wno-pointer-sign -Wno-attributes -Wno-sign-compare -march=r4400 -mabi=32 --sysroot=/s/cvs/src/obj.ews4800mips/destdir.ews4800mips -Dews4800mips -I. -I/s/cvs/src/sys/external/bsd/libnv/dist -I/s/cv! s/src/sys/../common/lib/libx86emu -I/s/cvs/src/sys/../common/l! ib/libc/misc -I/s/cvs/src/sys/../common/include -I/s/cvs/src/sys/arch -I/s/cvs/src/sys -nostdinc -DCOMPAT_UTILS -DMIPS3 -DMIPS3_ENABLE_CLOCK_INTR -DCOMPAT_44 -D_KERNEL -D_KERNEL_OPT -std=gnu99 -I/s/cvs/src/sys/lib/libkern/../../../common/lib/libc/quad -I/s/cvs/src/sys/lib/libkern/../../../common/lib/libc/string
Re: CVS commit: src/sys/arch/x86/x86
In article <20200606135850.ge14...@pony.stderr.spb.ru>, Valery Ushakov wrote: >On Sat, Jun 06, 2020 at 11:25:19 +0200, Kamil Rytarowski wrote: > >> On 06.06.2020 09:42, Simon Burge wrote: >> > "Kamil Rytarowski" wrote: >> > >> >> Module Name: src >> >> Committed By: kamil >> >> Date: Fri Jun 5 21:48:04 UTC 2020 >> >> >> >> Modified Files: >> >> >> >> src/sys/arch/x86/x86: cpu_rng.c >> >> >> >> Log Message: >> >> >> >> Change const unsigned to preprocessor define >> >> >> >> Fixes GCC -O0 build with the stack protector. >> > >> > Surely a gcc bug? This almost certainly needs an >> > /* XXX gcc stack protector -O0 bug */ comment and >> > possibly an entry in doc/HACKS as well otherwise >> > someone will come along later and de-uglify this >> > change. >> >> This is not really a GCC bug, as C const is not constexpr. It's >> also not the only place with such logic and such workaround. C++ >> fixed it and have real const. > >Doesn't -Wvla help catching these? Should we enable it? I think it might catch too much... But we can try it... christos
Re: CVS commit: src/sys/arch/x86/x86
On Sat, Jun 06, 2020 at 11:25:19 +0200, Kamil Rytarowski wrote: > On 06.06.2020 09:42, Simon Burge wrote: > > "Kamil Rytarowski" wrote: > > > >> Module Name: src > >> Committed By: kamil > >> Date: Fri Jun 5 21:48:04 UTC 2020 > >> > >> Modified Files: > >> > >>src/sys/arch/x86/x86: cpu_rng.c > >> > >> Log Message: > >> > >> Change const unsigned to preprocessor define > >> > >> Fixes GCC -O0 build with the stack protector. > > > > Surely a gcc bug? This almost certainly needs an > > /* XXX gcc stack protector -O0 bug */ comment and > > possibly an entry in doc/HACKS as well otherwise > > someone will come along later and de-uglify this > > change. > > This is not really a GCC bug, as C const is not constexpr. It's > also not the only place with such logic and such workaround. C++ > fixed it and have real const. Doesn't -Wvla help catching these? Should we enable it? -uwe
Re: CVS commit: src/sys/arch/x86/x86
On 06.06.2020 09:42, Simon Burge wrote: > "Kamil Rytarowski" wrote: > >> Module Name: src >> Committed By:kamil >> Date:Fri Jun 5 21:48:04 UTC 2020 >> >> Modified Files: >> >> src/sys/arch/x86/x86: cpu_rng.c >> >> Log Message: >> >> Change const unsigned to preprocessor define >> >> Fixes GCC -O0 build with the stack protector. > > Surely a gcc bug? This almost certainly needs an > /* XXX gcc stack protector -O0 bug */ comment and > possibly an entry in doc/HACKS as well otherwise > someone will come along later and de-uglify this > change. > > Cheers, > Simon. > This is not really a GCC bug, as C const is not constexpr. It's also not the only place with such logic and such workaround. C++ fixed it and have real const. signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/sys/arch/x86/x86
"Kamil Rytarowski" wrote: > Module Name: src > Committed By: kamil > Date: Fri Jun 5 21:48:04 UTC 2020 > > Modified Files: > > src/sys/arch/x86/x86: cpu_rng.c > > Log Message: > > Change const unsigned to preprocessor define > > Fixes GCC -O0 build with the stack protector. Surely a gcc bug? This almost certainly needs an /* XXX gcc stack protector -O0 bug */ comment and possibly an entry in doc/HACKS as well otherwise someone will come along later and de-uglify this change. Cheers, Simon.
Re: [stos, again] Re: CVS commit: src/sys/arch/amd64
On 04.06.2020 23:41, Andrew Doran wrote: > On Thu, Jun 04, 2020 at 02:35:17AM +0200, Kamil Rytarowski wrote: > >> On 04.06.2020 00:42, Andrew Doran wrote: >>> On Wed, Jun 03, 2020 at 02:03:22AM +0200, Kamil Rytarowski wrote: >>> On 03.06.2020 01:49, Andrew Doran wrote: > On the assembly thing recall that recently you expressed a desire to > remove > all of the amd64 assembly string functions from libc because of > sanitizers - > I invested my time to do up a little demo to try and show you why that's > not > a good idea: > > http://mail-index.netbsd.org/port-amd64/2020/04/19/msg003226.html Please note that interceptors for string functions are not just some extra burden, but also very useful approach to feedback a fuzzer through additional coverage. At least libFuzzer and honggfuzz wrap many kinds of string functions and use it for fuzzing. We should add a special mode in KCOV to feedback userland (syzkaller) with traces from string functions. https://github.com/google/honggfuzz/blob/bbb476eec95ad927d6d7d3d367d2b3e38eed3569/libhfuzz/memorycmp.c#L24 >>> >>> No argument from me there at all. I think that's a great idea and was >>> looking at the interceptors in TSAN recently to see how they work. >>> >>> Andrew >>> >> >> My note was not about switching away from ASM functions for certain >> functions, but rather giving an option to disable them under a sanitizer >> and adding an interceptor that can be useful for feedbacking a fuzzer. >> It's still not clear whether we will create such interface in KCOV as it >> has to be coordinated with Google+Linux as we would like to have a >> compatible interface for syzkaller. >> >> TSAN - do you mean the userland ones? > > Right, the userland one. > Great. We used to pass around 95% of upstream LLVM TSan tests. The remaining ones were hard and I had no time to look into them. >> BTW. There is a work-in-progress MKSANITIZER support for TSan, but it >> used to create unkillable processes (kernel bug). Basically when using a >> TSanitized userland applications, you will quickly end up with such >> processes (from AFAIR calling ls(1) and other simple applications are >> enough). >> >> If you are interested, I can share a reproducer. > > Yes please. Is the setup difficult? I plan to look at some of the > remaining issues noted on syzbot over the next while too. > ./build.sh -j8 -N0 -U -u -V MAKECONF=/dev/null -V MKCOMPAT=no -V MKDEBUGLIB=yes -V MKDEBUG=yes -V MKSANITIZER=yes -V USE_SANITIZER=thread -V MKLLVM=yes -V MKGCC=no -V HAVE_LLVM=yes -O /public/netbsd.fuzzer distribution Null mount /dev /dev/pts /tmp and chroot into it. Try to run some basic applications and see creation unkillable processes. Unless that was fixed by an accident/indirectly, you will quickly reproduce it. > Cheers, > Andrew >
Re: [stos, again] Re: CVS commit: src/sys/arch/amd64
On Thu, Jun 04, 2020 at 02:35:17AM +0200, Kamil Rytarowski wrote: > On 04.06.2020 00:42, Andrew Doran wrote: > > On Wed, Jun 03, 2020 at 02:03:22AM +0200, Kamil Rytarowski wrote: > > > >> On 03.06.2020 01:49, Andrew Doran wrote: > >>> On the assembly thing recall that recently you expressed a desire to > >>> remove > >>> all of the amd64 assembly string functions from libc because of > >>> sanitizers - > >>> I invested my time to do up a little demo to try and show you why that's > >>> not > >>> a good idea: > >>> > >>> http://mail-index.netbsd.org/port-amd64/2020/04/19/msg003226.html > >> > >> Please note that interceptors for string functions are not just some > >> extra burden, but also very useful approach to feedback a fuzzer through > >> additional coverage. > >> > >> At least libFuzzer and honggfuzz wrap many kinds of string functions and > >> use it for fuzzing. We should add a special mode in KCOV to feedback > >> userland (syzkaller) with traces from string functions. > >> > >> https://github.com/google/honggfuzz/blob/bbb476eec95ad927d6d7d3d367d2b3e38eed3569/libhfuzz/memorycmp.c#L24 > > > > No argument from me there at all. I think that's a great idea and was > > looking at the interceptors in TSAN recently to see how they work. > > > > Andrew > > > > My note was not about switching away from ASM functions for certain > functions, but rather giving an option to disable them under a sanitizer > and adding an interceptor that can be useful for feedbacking a fuzzer. > It's still not clear whether we will create such interface in KCOV as it > has to be coordinated with Google+Linux as we would like to have a > compatible interface for syzkaller. > > TSAN - do you mean the userland ones? Right, the userland one. > BTW. There is a work-in-progress MKSANITIZER support for TSan, but it > used to create unkillable processes (kernel bug). Basically when using a > TSanitized userland applications, you will quickly end up with such > processes (from AFAIR calling ls(1) and other simple applications are > enough). > > If you are interested, I can share a reproducer. Yes please. Is the setup difficult? I plan to look at some of the remaining issues noted on syzbot over the next while too. Cheers, Andrew
Re: [stos, again] Re: CVS commit: src/sys/arch/amd64
On 04.06.2020 00:42, Andrew Doran wrote: > On Wed, Jun 03, 2020 at 02:03:22AM +0200, Kamil Rytarowski wrote: > >> On 03.06.2020 01:49, Andrew Doran wrote: >>> On the assembly thing recall that recently you expressed a desire to remove >>> all of the amd64 assembly string functions from libc because of sanitizers - >>> I invested my time to do up a little demo to try and show you why that's not >>> a good idea: >>> >>> http://mail-index.netbsd.org/port-amd64/2020/04/19/msg003226.html >> >> Please note that interceptors for string functions are not just some >> extra burden, but also very useful approach to feedback a fuzzer through >> additional coverage. >> >> At least libFuzzer and honggfuzz wrap many kinds of string functions and >> use it for fuzzing. We should add a special mode in KCOV to feedback >> userland (syzkaller) with traces from string functions. >> >> https://github.com/google/honggfuzz/blob/bbb476eec95ad927d6d7d3d367d2b3e38eed3569/libhfuzz/memorycmp.c#L24 > > No argument from me there at all. I think that's a great idea and was > looking at the interceptors in TSAN recently to see how they work. > > Andrew > My note was not about switching away from ASM functions for certain functions, but rather giving an option to disable them under a sanitizer and adding an interceptor that can be useful for feedbacking a fuzzer. It's still not clear whether we will create such interface in KCOV as it has to be coordinated with Google+Linux as we would like to have a compatible interface for syzkaller. TSAN - do you mean the userland ones? BTW. There is a work-in-progress MKSANITIZER support for TSan, but it used to create unkillable processes (kernel bug). Basically when using a TSanitized userland applications, you will quickly end up with such processes (from AFAIR calling ls(1) and other simple applications are enough). If you are interested, I can share a reproducer.
Re: [stos, again] Re: CVS commit: src/sys/arch/amd64
Maxime, I read your e-mail carefully and conclude that the best way forward here is put this one to core@ for a technical decision. Cheers, Andrew On Wed, Jun 03, 2020 at 08:25:32AM +0200, Maxime Villard wrote: > Le 03/06/2020 ? 01:49, Andrew Doran a ?crit?: > > On Tue, Jun 02, 2020 at 08:41:53AM +0200, Maxime Villard wrote: > > > > > Le 02/06/2020 ? 00:58, Andrew Doran a ?crit?: > > > > Module Name:src > > > > Committed By: ad > > > > Date: Mon Jun 1 22:58:06 UTC 2020 > > > > > > > > Modified Files: > > > > src/sys/arch/amd64/amd64: cpufunc.S > > > > src/sys/arch/amd64/include: frameasm.h > > > > > > > > Log Message: > > > > Reported-by: syzbot+6dd5a230d19f0cbc7...@syzkaller.appspotmail.com > > > > > > > > Instrument STOS/MOVS for KMSAN to unbreak it. > > > > > > > > > > > > To generate a diff of this commit: > > > > cvs rdiff -u -r1.58 -r1.59 src/sys/arch/amd64/amd64/cpufunc.S > > > > cvs rdiff -u -r1.49 -r1.50 src/sys/arch/amd64/include/frameasm.h > > > > > > > > Please note that diffs are not public domain; they are subject to the > > > > copyright notices on the relevant files. > > > > > > Can you just stop ignoring the remarks that are made? > > > > That's up to you Maxime. If you habitually make it difficult for people to > > come to a reasonable compromise with you, then you're asking to not be taken > > seriously and will find yourself being ignored. > > You are confused. I asked for KMSAN to be unbroken, and proposed an > alternative, > which is atomic_load_relaxed. You were free to explain why my point was a bad > idea or why it didn't matter, but you refused, and just added a hack on top of > another. I'm afraid that's up to you. > > But whatever, it doesn't matter. Thanks for reverting the pmap.c changes, it > is more correct now than before. > > > > I said explicitly > > > that adding manual instrumentation here is _wrong_. > > > > I don't share your assessment in the general sense. It should be possible > > to instrument assembly code because KMSAN is useful AND we can't get by > > without assembly - there are some things that C just can't do (or do well > > enough). > > > > On the assembly thing recall that recently you expressed a desire to remove > > all of the amd64 assembly string functions from libc because of sanitizers - > > I invested my time to do up a little demo to try and show you why that's not > > a good idea: > > > > http://mail-index.netbsd.org/port-amd64/2020/04/19/msg003226.html > > I saw, yes. I answered explaining that a conversation with Ryo Shimizu had > changed my mind a bit, and seeing your results (which as far as I can tell > do not indicate a performance improvement significant enough to not be > considered as noise), I asked you whether it was that relevant. You didn't > follow up though. > > > The system is a balancing act. There are lots of factors to be taken into > > account: maintainability, tooling like KMSAN, user's varying needs, the > > capabilites of different machines, performance, feature set and so on, and > > dare I say it even the enjoyment of the people working on the project is > > important too. Myopic focus on one factor only to the detriment of others > > is no good. > > I am well aware of that. > > > > The kMSan ASM fixups > > > are limited to args/returns, and that is part of a sensical policy that > > > _should not_ be changed without a good reason. > > > > > > x86_movs/x86_stos have strictly _no reason_ to exist. Of the 18 > > > conversions > > > you made to them in pmap.c, not one is justified. memcpy/memset were all > > > correct. > > > > I introduced these functions as a compromise because you were unhappy with > > use of memcpy() to copy PDEs. See: > > > > http://mail-index.netbsd.org/port-amd64/2020/05/23/msg003280.html > > > > You wrote: > > > > In the [XXX] window, the PTEs could be used by userland. If you > > copied them using memcpy(), some parts of the bytes could contain > > stale values. > > Sure, I was explicitly referring to SVS, which has an unusual way of > accessing PTEs (contrary to pmap), which is why it needs special atomic > care that other places do not. > > > Live PDEs are also copied in pmap.c so I made a change there too. After > > that I decided "why not" and used the new functions everywhere PDEs/PTEs or > > pages are block zeroed / copied. But I'm also happy with memcpy()/memset(). > > Either way will work. In fairness I do work too fast sometimes. > > > > > The only reason you made these big unneeded changes is for SVS not to take > > > the bus lock, > > > > There is no bus lock on x86 (not since the 1990s anyway). > > > > > but as was said already, atomic_load_relaxed will do what > > > you want without the need for these functions. > > > > > > Please revert _both changes now_, this one and the previous one which > > > introduced both functions, and let's use atomic_load_relaxed. > > > > You're focusing on only
Re: [stos, again] Re: CVS commit: src/sys/arch/amd64
On Wed, Jun 03, 2020 at 02:03:22AM +0200, Kamil Rytarowski wrote: > On 03.06.2020 01:49, Andrew Doran wrote: > > On the assembly thing recall that recently you expressed a desire to remove > > all of the amd64 assembly string functions from libc because of sanitizers - > > I invested my time to do up a little demo to try and show you why that's not > > a good idea: > > > > http://mail-index.netbsd.org/port-amd64/2020/04/19/msg003226.html > > Please note that interceptors for string functions are not just some > extra burden, but also very useful approach to feedback a fuzzer through > additional coverage. > > At least libFuzzer and honggfuzz wrap many kinds of string functions and > use it for fuzzing. We should add a special mode in KCOV to feedback > userland (syzkaller) with traces from string functions. > > https://github.com/google/honggfuzz/blob/bbb476eec95ad927d6d7d3d367d2b3e38eed3569/libhfuzz/memorycmp.c#L24 No argument from me there at all. I think that's a great idea and was looking at the interceptors in TSAN recently to see how they work. Andrew
Re: [stos, again] Re: CVS commit: src/sys/arch/amd64
Le 03/06/2020 à 02:03, Kamil Rytarowski a écrit : On 03.06.2020 01:49, Andrew Doran wrote: On the assembly thing recall that recently you expressed a desire to remove all of the amd64 assembly string functions from libc because of sanitizers - I invested my time to do up a little demo to try and show you why that's not a good idea: http://mail-index.netbsd.org/port-amd64/2020/04/19/msg003226.html Please note that interceptors for string functions are not just some extra burden, but also very useful approach to feedback a fuzzer through additional coverage. At least libFuzzer and honggfuzz wrap many kinds of string functions and use it for fuzzing. We should add a special mode in KCOV to feedback userland (syzkaller) with traces from string functions. https://github.com/google/honggfuzz/blob/bbb476eec95ad927d6d7d3d367d2b3e38eed3569/libhfuzz/memorycmp.c#L24 Yes, and not just that either. When you use ASM instead of C, you basically prevent _any kind_ of useful transformation the compiler could make. It includes sanitizers, but also coverage as you said; and also retpoline, PAC, BTI, CET, SafeStack, and in short, a very big bunch of modern features. Favoring C rather than ASM in the general sense offers much bigger benefits than just "it accomodates kMSan". Maxime
Re: [stos, again] Re: CVS commit: src/sys/arch/amd64
Le 03/06/2020 à 01:49, Andrew Doran a écrit :
On Tue, Jun 02, 2020 at 08:41:53AM +0200, Maxime Villard wrote:
Le 02/06/2020 ? 00:58, Andrew Doran a ?crit?:
Module Name:src
Committed By: ad
Date: Mon Jun 1 22:58:06 UTC 2020
Modified Files:
src/sys/arch/amd64/amd64: cpufunc.S
src/sys/arch/amd64/include: frameasm.h
Log Message:
Reported-by: syzbot+6dd5a230d19f0cbc7...@syzkaller.appspotmail.com
Instrument STOS/MOVS for KMSAN to unbreak it.
To generate a diff of this commit:
cvs rdiff -u -r1.58 -r1.59 src/sys/arch/amd64/amd64/cpufunc.S
cvs rdiff -u -r1.49 -r1.50 src/sys/arch/amd64/include/frameasm.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Can you just stop ignoring the remarks that are made?
That's up to you Maxime. If you habitually make it difficult for people to
come to a reasonable compromise with you, then you're asking to not be taken
seriously and will find yourself being ignored.
You are confused. I asked for KMSAN to be unbroken, and proposed an alternative,
which is atomic_load_relaxed. You were free to explain why my point was a bad
idea or why it didn't matter, but you refused, and just added a hack on top of
another. I'm afraid that's up to you.
But whatever, it doesn't matter. Thanks for reverting the pmap.c changes, it
is more correct now than before.
I said explicitly
that adding manual instrumentation here is _wrong_.
I don't share your assessment in the general sense. It should be possible
to instrument assembly code because KMSAN is useful AND we can't get by
without assembly - there are some things that C just can't do (or do well
enough).
On the assembly thing recall that recently you expressed a desire to remove
all of the amd64 assembly string functions from libc because of sanitizers -
I invested my time to do up a little demo to try and show you why that's not
a good idea:
http://mail-index.netbsd.org/port-amd64/2020/04/19/msg003226.html
I saw, yes. I answered explaining that a conversation with Ryo Shimizu had
changed my mind a bit, and seeing your results (which as far as I can tell
do not indicate a performance improvement significant enough to not be
considered as noise), I asked you whether it was that relevant. You didn't
follow up though.
The system is a balancing act. There are lots of factors to be taken into
account: maintainability, tooling like KMSAN, user's varying needs, the
capabilites of different machines, performance, feature set and so on, and
dare I say it even the enjoyment of the people working on the project is
important too. Myopic focus on one factor only to the detriment of others
is no good.
I am well aware of that.
The kMSan ASM fixups
are limited to args/returns, and that is part of a sensical policy that
_should not_ be changed without a good reason.
x86_movs/x86_stos have strictly _no reason_ to exist. Of the 18 conversions
you made to them in pmap.c, not one is justified. memcpy/memset were all
correct.
I introduced these functions as a compromise because you were unhappy with
use of memcpy() to copy PDEs. See:
http://mail-index.netbsd.org/port-amd64/2020/05/23/msg003280.html
You wrote:
In the [XXX] window, the PTEs could be used by userland. If you
copied them using memcpy(), some parts of the bytes could contain
stale values.
Sure, I was explicitly referring to SVS, which has an unusual way of
accessing PTEs (contrary to pmap), which is why it needs special atomic
care that other places do not.
Live PDEs are also copied in pmap.c so I made a change there too. After
that I decided "why not" and used the new functions everywhere PDEs/PTEs or
pages are block zeroed / copied. But I'm also happy with memcpy()/memset().
Either way will work. In fairness I do work too fast sometimes.
The only reason you made these big unneeded changes is for SVS not to take
the bus lock,
There is no bus lock on x86 (not since the 1990s anyway).
but as was said already, atomic_load_relaxed will do what
you want without the need for these functions.
Please revert _both changes now_, this one and the previous one which
introduced both functions, and let's use atomic_load_relaxed.
You're focusing on only one factor. I'll explain in detail. Here is the
original code I replaced:
685 static inline pt_entry_t
686 svs_pte_atomic_read(struct pmap *pmap, size_t idx)
687 {
688/*
689 * XXX: We don't have a basic atomic_fetch_64 function?
690 */
691return atomic_cas_64(>pm_pdir[idx], 666, 666);
692 }
...
717/* User slots. */
718for (i = 0; i < PDIR_SLOT_USERLIM; i++) {
719pte = svs_pte_atomic_read(pmap, i);
720ci->ci_svs_updir[i] = pte;
721}
There's no need for an atomic op there because fetches on x86 are by
definition atomic, and it
Re: [stos, again] Re: CVS commit: src/sys/arch/amd64
On 03.06.2020 01:49, Andrew Doran wrote: > On the assembly thing recall that recently you expressed a desire to remove > all of the amd64 assembly string functions from libc because of sanitizers - > I invested my time to do up a little demo to try and show you why that's not > a good idea: > > http://mail-index.netbsd.org/port-amd64/2020/04/19/msg003226.html Please note that interceptors for string functions are not just some extra burden, but also very useful approach to feedback a fuzzer through additional coverage. At least libFuzzer and honggfuzz wrap many kinds of string functions and use it for fuzzing. We should add a special mode in KCOV to feedback userland (syzkaller) with traces from string functions. https://github.com/google/honggfuzz/blob/bbb476eec95ad927d6d7d3d367d2b3e38eed3569/libhfuzz/memorycmp.c#L24
Re: [stos, again] Re: CVS commit: src/sys/arch/amd64
On Tue, Jun 02, 2020 at 08:41:53AM +0200, Maxime Villard wrote:
> Le 02/06/2020 ? 00:58, Andrew Doran a ?crit?:
> > Module Name:src
> > Committed By: ad
> > Date: Mon Jun 1 22:58:06 UTC 2020
> >
> > Modified Files:
> > src/sys/arch/amd64/amd64: cpufunc.S
> > src/sys/arch/amd64/include: frameasm.h
> >
> > Log Message:
> > Reported-by: syzbot+6dd5a230d19f0cbc7...@syzkaller.appspotmail.com
> >
> > Instrument STOS/MOVS for KMSAN to unbreak it.
> >
> >
> > To generate a diff of this commit:
> > cvs rdiff -u -r1.58 -r1.59 src/sys/arch/amd64/amd64/cpufunc.S
> > cvs rdiff -u -r1.49 -r1.50 src/sys/arch/amd64/include/frameasm.h
> >
> > Please note that diffs are not public domain; they are subject to the
> > copyright notices on the relevant files.
>
> Can you just stop ignoring the remarks that are made?
That's up to you Maxime. If you habitually make it difficult for people to
come to a reasonable compromise with you, then you're asking to not be taken
seriously and will find yourself being ignored.
> I said explicitly
> that adding manual instrumentation here is _wrong_.
I don't share your assessment in the general sense. It should be possible
to instrument assembly code because KMSAN is useful AND we can't get by
without assembly - there are some things that C just can't do (or do well
enough).
On the assembly thing recall that recently you expressed a desire to remove
all of the amd64 assembly string functions from libc because of sanitizers -
I invested my time to do up a little demo to try and show you why that's not
a good idea:
http://mail-index.netbsd.org/port-amd64/2020/04/19/msg003226.html
The system is a balancing act. There are lots of factors to be taken into
account: maintainability, tooling like KMSAN, user's varying needs, the
capabilites of different machines, performance, feature set and so on, and
dare I say it even the enjoyment of the people working on the project is
important too. Myopic focus on one factor only to the detriment of others
is no good.
> The kMSan ASM fixups
> are limited to args/returns, and that is part of a sensical policy that
> _should not_ be changed without a good reason.
>
> x86_movs/x86_stos have strictly _no reason_ to exist. Of the 18 conversions
> you made to them in pmap.c, not one is justified. memcpy/memset were all
> correct.
I introduced these functions as a compromise because you were unhappy with
use of memcpy() to copy PDEs. See:
http://mail-index.netbsd.org/port-amd64/2020/05/23/msg003280.html
You wrote:
In the [XXX] window, the PTEs could be used by userland. If you
copied them using memcpy(), some parts of the bytes could contain
stale values.
Live PDEs are also copied in pmap.c so I made a change there too. After
that I decided "why not" and used the new functions everywhere PDEs/PTEs or
pages are block zeroed / copied. But I'm also happy with memcpy()/memset().
Either way will work. In fairness I do work too fast sometimes.
> The only reason you made these big unneeded changes is for SVS not to take
> the bus lock,
There is no bus lock on x86 (not since the 1990s anyway).
> but as was said already, atomic_load_relaxed will do what
> you want without the need for these functions.
>
> Please revert _both changes now_, this one and the previous one which
> introduced both functions, and let's use atomic_load_relaxed.
You're focusing on only one factor. I'll explain in detail. Here is the
original code I replaced:
685 static inline pt_entry_t
686 svs_pte_atomic_read(struct pmap *pmap, size_t idx)
687 {
688 /*
689 * XXX: We don't have a basic atomic_fetch_64 function?
690 */
691 return atomic_cas_64(>pm_pdir[idx], 666, 666);
692 }
...
717 /* User slots. */
718 for (i = 0; i < PDIR_SLOT_USERLIM; i++) {
719 pte = svs_pte_atomic_read(pmap, i);
720 ci->ci_svs_updir[i] = pte;
721 }
There's no need for an atomic op there because fetches on x86 are by
definition atomic, and it does nothing to alter the memory ordering in this
case. There are side effects to the atomic op: it's serializing and always
generates an unbuffered writeback, even in the failure case. So the source
is being copied into itself, as well as into the destination, and the CPU's
store buffer is rendered ineffective.
A cut-n-paste replacement to use the relaxed functions predictably ties the
compiler in knots and the generated code is bad.
/* User slots. */
for (i = 0; i < PDIR_SLOT_USERLIM; i++) {
pte = atomic_load_relaxed(>pm_pdir[i]);
atomic_store_relaxed(>ci_svs_updir[i], pte);
}
00400c9f :
400c9f: 48 8b 06mov(%rsi),%rax
400ca2: 48 8b 17mov(%rdi),%rdx
400ca5: 48 8d b0 00 00 40 06lea0x640(%rax),%rsi
[stos, again] Re: CVS commit: src/sys/arch/amd64
Le 02/06/2020 à 00:58, Andrew Doran a écrit : Module Name:src Committed By: ad Date: Mon Jun 1 22:58:06 UTC 2020 Modified Files: src/sys/arch/amd64/amd64: cpufunc.S src/sys/arch/amd64/include: frameasm.h Log Message: Reported-by: syzbot+6dd5a230d19f0cbc7...@syzkaller.appspotmail.com Instrument STOS/MOVS for KMSAN to unbreak it. To generate a diff of this commit: cvs rdiff -u -r1.58 -r1.59 src/sys/arch/amd64/amd64/cpufunc.S cvs rdiff -u -r1.49 -r1.50 src/sys/arch/amd64/include/frameasm.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Can you just stop ignoring the remarks that are made? I said explicitly that adding manual instrumentation here is _wrong_. The kMSan ASM fixups are limited to args/returns, and that is part of a sensical policy that _should not_ be changed without a good reason. x86_movs/x86_stos have strictly _no reason_ to exist. Of the 18 conversions you made to them in pmap.c, not one is justified. memcpy/memset were all correct. The only reason you made these big unneeded changes is for SVS not to take the bus lock, but as was said already, atomic_load_relaxed will do what you want without the need for these functions. Please revert _both changes now_, this one and the previous one which introduced both functions, and let's use atomic_load_relaxed. Maxime
Re: [stos] Re: CVS commit: src/sys/arch
Le 28/05/2020 à 23:58, Andrew Doran a écrit : On Thu, May 28, 2020 at 07:06:04PM +0200, Maxime Villard wrote: Le 27/05/2020 ? 21:58, Maxime Villard a ?crit?: Le 27/05/2020 ? 21:33, Andrew Doran a ?crit?: Module Name:??? src Committed By:??? ad Date:??? Wed May 27 19:33:40 UTC 2020 Modified Files: src/sys/arch/amd64/amd64: cpufunc.S locore.S src/sys/arch/i386/i386: cpufunc.S locore.S src/sys/arch/x86/include: pmap.h src/sys/arch/x86/x86: pmap.c Log Message: - Add a couple of wrapper functions around STOS and MOVS and use them to zero ?? and copy PTEs in preference to memset()/memcpy(). - Remove related SSE / pageidlezero stuff. To generate a diff of this commit: cvs rdiff -u -r1.56 -r1.57 src/sys/arch/amd64/amd64/cpufunc.S cvs rdiff -u -r1.208 -r1.209 src/sys/arch/amd64/amd64/locore.S cvs rdiff -u -r1.42 -r1.43 src/sys/arch/i386/i386/cpufunc.S cvs rdiff -u -r1.184 -r1.185 src/sys/arch/i386/i386/locore.S cvs rdiff -u -r1.121 -r1.122 src/sys/arch/x86/include/pmap.h cvs rdiff -u -r1.395 -r1.396 src/sys/arch/x86/x86/pmap.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. -END(x86_stos) +END(x86_movs) The naming convention should also be more explicit I think, because movs does not say if it is b/w/l/q. Don't have anything meaningful to suggest though I see that syzbot-kMSan is failing because of this change. Looking at it more closely: - Having MI ASM copy functions is unwanted, because the sanitizers won't be able to sanitize the accesses. kMSan misses the initialization and reports false positives. As well kASan will miss memory corruptions and kCSan will miss races. - It appears you replaced memcpy/memset by x86_movs/x86_stos in places where it is unnecessary and unwanted. Eg the majority of the changes in pmap.c are unwanted and should remain memcpy/memset. Please revert this change promptly in order to unbreak kMSan. We really need to have a clear policy on which function should be in ASM, and not introduce wild MI things like that which not only are rarely justified but also are a big PITA when sanitizers come into play. Very good. I see a function in subr_msan.c that looks like it does the needful here: void __msan_instrument_asm_store(void *addr, size_t size) I don't see any uses in tree. Is there a reason for that? The functions that begin by __msan_* are called from the compiler-generated instrumentation directly, not from the kernel (even though they could). Adding calls to this function from asm would be a big hack that I don't want in the kernel, and it wouldn't be addressing the real problem, which is, that the introduction of x86_movs/x86_stos is unnecessary in the first place, and the way they are used right now is just wrong -- memcpy/memset should have stayed in place. The whole change you made is for svs_pdir_switch() to use quads, but why not use atomic_load_relaxed? With that it should fetch quads without taking the bus lock, and we don't have to resort to ugly ASM functions. Thanks, Maxime
Re: [stos] Re: CVS commit: src/sys/arch
On Thu, May 28, 2020 at 07:06:04PM +0200, Maxime Villard wrote: > Le 27/05/2020 ? 21:58, Maxime Villard a ?crit?: > > Le 27/05/2020 ? 21:33, Andrew Doran a ?crit?: > > > Module Name:??? src > > > Committed By:??? ad > > > Date:??? Wed May 27 19:33:40 UTC 2020 > > > > > > Modified Files: > > > src/sys/arch/amd64/amd64: cpufunc.S locore.S > > > src/sys/arch/i386/i386: cpufunc.S locore.S > > > src/sys/arch/x86/include: pmap.h > > > src/sys/arch/x86/x86: pmap.c > > > > > > Log Message: > > > - Add a couple of wrapper functions around STOS and MOVS and use them to > > > zero > > > ?? and copy PTEs in preference to memset()/memcpy(). > > > > > > - Remove related SSE / pageidlezero stuff. > > > > > > > > > To generate a diff of this commit: > > > cvs rdiff -u -r1.56 -r1.57 src/sys/arch/amd64/amd64/cpufunc.S > > > cvs rdiff -u -r1.208 -r1.209 src/sys/arch/amd64/amd64/locore.S > > > cvs rdiff -u -r1.42 -r1.43 src/sys/arch/i386/i386/cpufunc.S > > > cvs rdiff -u -r1.184 -r1.185 src/sys/arch/i386/i386/locore.S > > > cvs rdiff -u -r1.121 -r1.122 src/sys/arch/x86/include/pmap.h > > > cvs rdiff -u -r1.395 -r1.396 src/sys/arch/x86/x86/pmap.c > > > > > > Please note that diffs are not public domain; they are subject to the > > > copyright notices on the relevant files. > > > > -END(x86_stos) > > +END(x86_movs) > > > > The naming convention should also be more explicit I think, because movs > > does not say if it is b/w/l/q. Don't have anything meaningful to suggest > > though > > I see that syzbot-kMSan is failing because of this change. Looking at it more > closely: > > - Having MI ASM copy functions is unwanted, because the sanitizers won't be >able to sanitize the accesses. kMSan misses the initialization and reports >false positives. As well kASan will miss memory corruptions and kCSan will >miss races. > > - It appears you replaced memcpy/memset by x86_movs/x86_stos in places where >it is unnecessary and unwanted. Eg the majority of the changes in pmap.c >are unwanted and should remain memcpy/memset. > > Please revert this change promptly in order to unbreak kMSan. We really need > to have a clear policy on which function should be in ASM, and not introduce > wild MI things like that which not only are rarely justified but also are a > big PITA when sanitizers come into play. Very good. I see a function in subr_msan.c that looks like it does the needful here: void __msan_instrument_asm_store(void *addr, size_t size) I don't see any uses in tree. Is there a reason for that? Thanks, Andrew
Re: [stos] Re: CVS commit: src/sys/arch
Le 28/05/2020 à 19:06, Maxime Villard a écrit : Le 27/05/2020 à 21:58, Maxime Villard a écrit : Le 27/05/2020 à 21:33, Andrew Doran a écrit : Module Name: src Committed By: ad Date: Wed May 27 19:33:40 UTC 2020 Modified Files: src/sys/arch/amd64/amd64: cpufunc.S locore.S src/sys/arch/i386/i386: cpufunc.S locore.S src/sys/arch/x86/include: pmap.h src/sys/arch/x86/x86: pmap.c Log Message: - Add a couple of wrapper functions around STOS and MOVS and use them to zero and copy PTEs in preference to memset()/memcpy(). - Remove related SSE / pageidlezero stuff. To generate a diff of this commit: cvs rdiff -u -r1.56 -r1.57 src/sys/arch/amd64/amd64/cpufunc.S cvs rdiff -u -r1.208 -r1.209 src/sys/arch/amd64/amd64/locore.S cvs rdiff -u -r1.42 -r1.43 src/sys/arch/i386/i386/cpufunc.S cvs rdiff -u -r1.184 -r1.185 src/sys/arch/i386/i386/locore.S cvs rdiff -u -r1.121 -r1.122 src/sys/arch/x86/include/pmap.h cvs rdiff -u -r1.395 -r1.396 src/sys/arch/x86/x86/pmap.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. -END(x86_stos) +END(x86_movs) The naming convention should also be more explicit I think, because movs does not say if it is b/w/l/q. Don't have anything meaningful to suggest though I see that syzbot-kMSan is failing because of this change. Looking at it more closely: - Having MI ASM copy functions is unwanted, because the sanitizers won't be able to sanitize the accesses. kMSan misses the initialization and reports false positives. As well kASan will miss memory corruptions and kCSan will miss races. - It appears you replaced memcpy/memset by x86_movs/x86_stos in places where it is unnecessary and unwanted. Eg the majority of the changes in pmap.c are unwanted and should remain memcpy/memset. Please revert this change promptly in order to unbreak kMSan. We really need to have a clear policy on which function should be in ASM, and not introduce wild MI things like that which not only are rarely justified but also are a big PITA when sanitizers come into play. Thanks, Maxime "MI" -> "MD"
Re: [stos] Re: CVS commit: src/sys/arch
Le 27/05/2020 à 21:58, Maxime Villard a écrit : Le 27/05/2020 à 21:33, Andrew Doran a écrit : Module Name: src Committed By: ad Date: Wed May 27 19:33:40 UTC 2020 Modified Files: src/sys/arch/amd64/amd64: cpufunc.S locore.S src/sys/arch/i386/i386: cpufunc.S locore.S src/sys/arch/x86/include: pmap.h src/sys/arch/x86/x86: pmap.c Log Message: - Add a couple of wrapper functions around STOS and MOVS and use them to zero and copy PTEs in preference to memset()/memcpy(). - Remove related SSE / pageidlezero stuff. To generate a diff of this commit: cvs rdiff -u -r1.56 -r1.57 src/sys/arch/amd64/amd64/cpufunc.S cvs rdiff -u -r1.208 -r1.209 src/sys/arch/amd64/amd64/locore.S cvs rdiff -u -r1.42 -r1.43 src/sys/arch/i386/i386/cpufunc.S cvs rdiff -u -r1.184 -r1.185 src/sys/arch/i386/i386/locore.S cvs rdiff -u -r1.121 -r1.122 src/sys/arch/x86/include/pmap.h cvs rdiff -u -r1.395 -r1.396 src/sys/arch/x86/x86/pmap.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. -END(x86_stos) +END(x86_movs) The naming convention should also be more explicit I think, because movs does not say if it is b/w/l/q. Don't have anything meaningful to suggest though I see that syzbot-kMSan is failing because of this change. Looking at it more closely: - Having MI ASM copy functions is unwanted, because the sanitizers won't be able to sanitize the accesses. kMSan misses the initialization and reports false positives. As well kASan will miss memory corruptions and kCSan will miss races. - It appears you replaced memcpy/memset by x86_movs/x86_stos in places where it is unnecessary and unwanted. Eg the majority of the changes in pmap.c are unwanted and should remain memcpy/memset. Please revert this change promptly in order to unbreak kMSan. We really need to have a clear policy on which function should be in ASM, and not introduce wild MI things like that which not only are rarely justified but also are a big PITA when sanitizers come into play. Thanks, Maxime
[stos] Re: CVS commit: src/sys/arch
Le 27/05/2020 à 21:33, Andrew Doran a écrit : Module Name:src Committed By: ad Date: Wed May 27 19:33:40 UTC 2020 Modified Files: src/sys/arch/amd64/amd64: cpufunc.S locore.S src/sys/arch/i386/i386: cpufunc.S locore.S src/sys/arch/x86/include: pmap.h src/sys/arch/x86/x86: pmap.c Log Message: - Add a couple of wrapper functions around STOS and MOVS and use them to zero and copy PTEs in preference to memset()/memcpy(). - Remove related SSE / pageidlezero stuff. To generate a diff of this commit: cvs rdiff -u -r1.56 -r1.57 src/sys/arch/amd64/amd64/cpufunc.S cvs rdiff -u -r1.208 -r1.209 src/sys/arch/amd64/amd64/locore.S cvs rdiff -u -r1.42 -r1.43 src/sys/arch/i386/i386/cpufunc.S cvs rdiff -u -r1.184 -r1.185 src/sys/arch/i386/i386/locore.S cvs rdiff -u -r1.121 -r1.122 src/sys/arch/x86/include/pmap.h cvs rdiff -u -r1.395 -r1.396 src/sys/arch/x86/x86/pmap.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. -END(x86_stos) +END(x86_movs) The naming convention should also be more explicit I think, because movs does not say if it is b/w/l/q. Don't have anything meaningful to suggest though
Re: CVS commit: src/sys/arch/aarch64
>> Has any consideration be given to perhaps creating a new MACHINE_ARCH for >> this, or somehow otherwise decorating the ELF files to indicate their >> exec-ability? > >I am under the impression that PAC was designed to be forewards >compatible, so older CPUs can execute code with this annotation. I don't >whether it works in practice though. Yes. That's right. Even binaries compiled with "gcc -msign-return-address=all" work fine on ARMv8.0-v8.2 CPUs. This is because the "paciasp" and "autiasp" instructions are treated as "nop" on ARMv8.0-v8.2 CPUs. Therefore, I don't think it's necessary to add detailed attributes to the ELF header. So, even if we default the PAC option right now, we won't have any problems. But we might as well enable PAC by default after real ARMv8.3 hardware becomes available easily. (now tested on "qemu-system-aarch64 -cpu max") -- ryo shimizu
Re: CVS commit: src/sys/arch/aarch64
On Sat, May 23, 2020 at 02:13:46PM -0700, Jason Thorpe wrote: > > > On May 23, 2020, at 11:08 AM, Ryo Shimizu wrote: > > > > Module Name:src > > Committed By: ryo > > Date: Sat May 23 18:08:59 UTC 2020 > > > > Modified Files: > > src/sys/arch/aarch64/aarch64: cpufunc.c cpuswitch.S exec_machdep.c > > genassym.cf netbsd32_machdep.c vectors.S vm_machdep.c > > src/sys/arch/aarch64/include: armreg.h machdep.h proc.h > > > > Log Message: > > Not only the kernel thread, but also the userland PAC keys > > (APIA,APIB,APDA,APDB,APGA) are now randomly initialized at exec, and > > switched > > when context switch. > > userland programs are able to perform pointer authentication on ARMv8.3+PAC > > cpu. > > Has any consideration be given to perhaps creating a new MACHINE_ARCH for > this, or somehow otherwise decorating the ELF files to indicate their > exec-ability? I am under the impression that PAC was designed to be forewards compatible, so older CPUs can execute code with this annotation. I don't whether it works in practice though.
Re: CVS commit: src/sys/arch/aarch64
> On May 23, 2020, at 11:08 AM, Ryo Shimizu wrote: > > Module Name: src > Committed By: ryo > Date: Sat May 23 18:08:59 UTC 2020 > > Modified Files: > src/sys/arch/aarch64/aarch64: cpufunc.c cpuswitch.S exec_machdep.c > genassym.cf netbsd32_machdep.c vectors.S vm_machdep.c > src/sys/arch/aarch64/include: armreg.h machdep.h proc.h > > Log Message: > Not only the kernel thread, but also the userland PAC keys > (APIA,APIB,APDA,APDB,APGA) are now randomly initialized at exec, and switched > when context switch. > userland programs are able to perform pointer authentication on ARMv8.3+PAC > cpu. Has any consideration be given to perhaps creating a new MACHINE_ARCH for this, or somehow otherwise decorating the ELF files to indicate their exec-ability? > > reviewd by maxv@, thanks. > > > To generate a diff of this commit: > cvs rdiff -u -r1.18 -r1.19 src/sys/arch/aarch64/aarch64/cpufunc.c > cvs rdiff -u -r1.20 -r1.21 src/sys/arch/aarch64/aarch64/cpuswitch.S > cvs rdiff -u -r1.6 -r1.7 src/sys/arch/aarch64/aarch64/exec_machdep.c > cvs rdiff -u -r1.24 -r1.25 src/sys/arch/aarch64/aarch64/genassym.cf > cvs rdiff -u -r1.12 -r1.13 src/sys/arch/aarch64/aarch64/netbsd32_machdep.c > cvs rdiff -u -r1.16 -r1.17 src/sys/arch/aarch64/aarch64/vectors.S > cvs rdiff -u -r1.7 -r1.8 src/sys/arch/aarch64/aarch64/vm_machdep.c > cvs rdiff -u -r1.44 -r1.45 src/sys/arch/aarch64/include/armreg.h > cvs rdiff -u -r1.10 -r1.11 src/sys/arch/aarch64/include/machdep.h > cvs rdiff -u -r1.6 -r1.7 src/sys/arch/aarch64/include/proc.h > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. > -- thorpej
Re: CVS commit: src/sys/arch
On Thu, May 21, 2020 at 11:19:49PM +0200, Joerg Sonnenberger wrote: > On Thu, May 21, 2020 at 09:12:31PM +, Andrew Doran wrote: > > Module Name:src > > Committed By: ad > > Date: Thu May 21 21:12:31 UTC 2020 > > > > Modified Files: > > src/sys/arch/x86/acpi: acpi_wakeup.c > > src/sys/arch/x86/include: i82489var.h > > src/sys/arch/x86/x86: cpu.c lapic.c x86_machdep.c > > src/sys/arch/xen/x86: cpu.c > > src/sys/arch/xen/xen: hypervisor.c xen_clock.c > > > > Log Message: > > - Recalibrate the APIC timer using the TSC, once the TSC has in turn been > > recalibrated using the HPET. This gets the clock interrupt firing more > > closely to HZ. > > Why using the TSC and not the HPET directly? For systems with HPET, same > question with the ACPI / PCI timer. The idea I'm going with is: The TSC is precise and very quick to read, far more so than the other timers where it's at least 0.5-1us to pull a value from the counter register. To get around slowness of the other timers the TSC is calibrated with respect to the HPET over a long span (seconds). I have no objection to revisiting it if we can do better, but this is at least much better than where we were before. kern.timecounter.choice = TSC(q=3000, f=10 Hz) lapic(q=-100, f=2 Hz) clockinterrupt(q=0, f=100 Hz) hpet0(q=2000, f=14318180 Hz) ACPI-Fast(q=1000, f=3579545 Hz) i8254(q=100, f=1193182 Hz) dummy(q=-100, f=100 Hz) kern.timecounter.choice = TSC(q=3000, f=25 Hz) lapic(q=-100, f=1 Hz) clockinterrupt(q=0, f=100 Hz) ichlpcib0(q=1000, f=3579545 Hz) hpet0(q=2000, f=14318180 Hz) ACPI-Fast(q=1000, f=3579545 Hz) i8254(q=100, f=1193182 Hz) dummy(q=-100, f=100 Hz) Andrew
Re: CVS commit: src/sys/arch
On Thu, May 21, 2020 at 09:12:31PM +, Andrew Doran wrote: > Module Name: src > Committed By: ad > Date: Thu May 21 21:12:31 UTC 2020 > > Modified Files: > src/sys/arch/x86/acpi: acpi_wakeup.c > src/sys/arch/x86/include: i82489var.h > src/sys/arch/x86/x86: cpu.c lapic.c x86_machdep.c > src/sys/arch/xen/x86: cpu.c > src/sys/arch/xen/xen: hypervisor.c xen_clock.c > > Log Message: > - Recalibrate the APIC timer using the TSC, once the TSC has in turn been > recalibrated using the HPET. This gets the clock interrupt firing more > closely to HZ. Why using the TSC and not the HPET directly? For systems with HPET, same question with the ACPI / PCI timer. Joerg
[cpufunc] Re: CVS commit: src/sys/arch
Module Name:src Committed By: ad Date: Tue May 19 21:40:55 UTC 2020 Modified Files: src/sys/arch/amd64/amd64: cpufunc.S src/sys/arch/i386/i386: cpufunc.S i386func.S Log Message: Make cpu_counter(), cpu_counter32() and tsc_get_timecount() into a single preemption-safe routine. To generate a diff of this commit: cvs rdiff -u -r1.52 -r1.53 src/sys/arch/amd64/amd64/cpufunc.S cvs rdiff -u -r1.40 -r1.41 src/sys/arch/i386/i386/cpufunc.S cvs rdiff -u -r1.21 -r1.22 src/sys/arch/i386/i386/i386func.S Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Aliasing functions with different prototypes is wrong. cpu_counter() returns a uint64_t, so it should have KMSAN_INIT_RET(8). The two other functions you aliased return uint32_t, so they should have KMSAN_INIT_RET(4). This can't be reconciled because of the alias. Please revert this change.
Re: CVS commit: src/sys/arch/xen
On Sat, May 16, 2020 at 02:51:43PM +1000, matthew green wrote: > "Jaromir Dolecek" writes: > > Module Name:src > > Committed By: jdolecek > > Date: Fri May 15 07:42:58 UTC 2020 > > > > Modified Files: > > src/sys/arch/xen/include: intr.h > > src/sys/arch/xen/x86: pintr.c > > > > Log Message: > > use short for irq2port[] to save memory (4KB), it only needs to store > > numbers <= NR_EVENT_CHANNELS (2048) > > not that they're necessarily related, but you can have a look > at PR#54837 which has problems with running out of interrrupts > on systems with lots of cpus and devices with lots of msix? It's not related in any way to Xen. In fact, a kernel running under Xen PV could avoid this issue. I have plans to update the per-cpu interrupt mask from 32 to 48 bits (it's not possible to go to 64 bits as suggested in the PR). Making the idtvect per-CPU is another topic. -- Manuel Bouyer NetBSD: 26 ans d'experience feront toujours la difference --
re: CVS commit: src/sys/arch/xen
"Jaromir Dolecek" writes: > Module Name: src > Committed By: jdolecek > Date: Fri May 15 07:42:58 UTC 2020 > > Modified Files: > src/sys/arch/xen/include: intr.h > src/sys/arch/xen/x86: pintr.c > > Log Message: > use short for irq2port[] to save memory (4KB), it only needs to store > numbers <= NR_EVENT_CHANNELS (2048) not that they're necessarily related, but you can have a look at PR#54837 which has problems with running out of interrrupts on systems with lots of cpus and devices with lots of msix? thanks. .mrg.
Re: CVS commit: src/sys/arch/xen/xen
On Mon, Apr 13, 2020 at 08:09:13PM +, Jaromir Dolecek wrote: > Module Name: src > Committed By: jdolecek > Date: Mon Apr 13 20:09:13 UTC 2020 > > Modified Files: > src/sys/arch/xen/xen: xbd_xenbus.c > > Log Message: > KASSERT() that requested I/O size is <= XBD_MAX_XFER - this can happen > e.g. with custom DomU kernel which doesn't have the value for MAXPHYS > reduced to 32k like the XEN3_DOMU config I though we were now splitting the requests ? -- Manuel Bouyer NetBSD: 26 ans d'experience feront toujours la difference --
Re: CVS commit: src/sys/arch/xen/xen
HI, I am not sur whether it is the commit below, but 2 out 4 times my xen-DOMU from today (20200409/9.99.55) panics with following locking botch: [ 29.9301379] panic: kernel diagnostic assertion "IFNET_LOCKED(ifp)" failed: file "/usr/src/sys/arch/xen/xen/if_xennet_xenbus.c", line 1120 [ 29.9301379] cpu2: Begin traceback... [ 29.9301379] vpanic() at netbsd:vpanic+0x146 [ 29.9301379] kern_assert() at netbsd:kern_assert+0x48 [ 29.9301379] xennet_ioctl() at netbsd:xennet_ioctl+0x6d [ 29.9301379] if_mcast_op() at netbsd:if_mcast_op+0x6a [ 29.9301379] in6_addmulti() at netbsd:in6_addmulti+0x153 [ 29.9301379] in6_joingroup() at netbsd:in6_joingroup+0x45 [ 29.9301379] ip6_ctloutput() at netbsd:ip6_ctloutput+0x141c [ 29.9301379] udp6_ctloutput() at netbsd:udp6_ctloutput+0xa2 [ 29.9301379] udp6_ctloutput_wrapper() at netbsd:udp6_ctloutput_wrapper+0x2c [ 29.9301379] sosetopt() at netbsd:sosetopt+0x5c [ 29.9301379] sys_setsockopt() at netbsd:sys_setsockopt+0x8e [ 29.9301379] syscall() at netbsd:syscall+0x9c [ 29.9301379] --- syscall (number 105) --- [ 29.9301379] 75d934d3469a: [ 29.9301379] cpu2: End traceback... [ 29.9301379] rebooting... Best regards, Frank On 04/06/20 20:23, Jaromir Dolecek wrote: Module Name:src Committed By: jdolecek Date: Mon Apr 6 18:23:21 UTC 2020 Modified Files: src/sys/arch/xen/xen: if_xennet_xenbus.c Log Message: convert to IFEF_MPSAFE, also enable interrupt handler without biglock no performance difference observed compared to version before change, for neither UP nor MP DomU To generate a diff of this commit: cvs rdiff -u -r1.105 -r1.106 src/sys/arch/xen/xen/if_xennet_xenbus.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/arch/xen/xen
On Sun, Apr 05, 2020 at 05:26:47PM +, Jaromir Dolecek wrote: > Module Name: src > Committed By: jdolecek > Date: Sun Apr 5 17:26:47 UTC 2020 > > Modified Files: > src/sys/arch/xen/xen: if_xennet_xenbus.c xennetback_xenbus.c > > Log Message: > remove support for legacy rx-flip mode for xennet(4)/xvif(4), making > rx-copy (first shipped in NetBSD 6.0 in 2012) the only supported > mode did you actually read my reply to your proposal to port-xen ? The example provided in the xen 4.11 sources still use rx-flip, so I'm not sure it's not supported any more by linux dom0. -- Manuel Bouyer NetBSD: 26 ans d'experience feront toujours la difference --
Re: CVS commit: src/sys/arch/x86/x86
Hi, Hmm, but TSC drift is still observed on recent (high clock) CPUs. I will have researched and fix later. On 2020/04/03 12:05, Kengo NAKAHARA wrote: Module Name:src Committed By: knakahara Date: Fri Apr 3 03:05:39 UTC 2020 Modified Files: src/sys/arch/x86/x86: tsc.c Log Message: Fix TSC drift is observed almost every time wrongly. Ths "TSC drift" in tsc_tc_init() means the cpu_cc_skew delta between first measurement (in cpu_start_secondary) and second measurement (in cpu_boot_secondary), that is, the TSC drift is expected to be almost zero. However, the second measument in current implementation is added extra cpu_cc_skew accidentally, so current delta value means almost cpu_cc_skew wrongly. tsc_sync_bp and tsc_sync_ap should use rdtsc() to get raw values. Advised by nonaka@n.o, thanks. To generate a diff of this commit: cvs rdiff -u -r1.38 -r1.39 src/sys/arch/x86/x86/tsc.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 Development Department, Product Division, Technology Unit Kengo NAKAHARA
Re: CVS commit: src/sys/arch/arm/sociox
Great work! One small remark:
+static int
+ave_fdt_match(device_t parent, cfdata_t cf, void *aux)
+{
+ static const char * compatible[] = {
+#ifdef _LP64
+ "socionext,unifier-ld20-ave4",
+#else
+ "socionext,unifier-pro4-ave4",
+ "socionext,unifier-pxs2-ave4",
+ "socionext,unifier-ld11-ave4",
+ "socionext,unifier-pxs3-ave4",
+#endif
+ NULL
+ };
Please do not use #ifdef here. Compatible strings are meant to describe
which hardware device the driver is compatible with, and can be shared
across multiple SoCs. In the case of IP licensing, possibly not even in
the same SoC family! Consider a hypothetical where Socionext could release
a new 32-bit SoC with an ave(4) that is functionally identical to the one
found in their previous 64-bit chip.
Re: CVS commit: src/sys/arch/x86/acpi
On Wed, Mar 18, 2020 at 09:44:03PM +0200, Yorick Hardy wrote: > Dear Andrew, > > On 2020-03-14, Andrew Doran wrote: > > Module Name:src > > Committed By: ad > > Date: Sat Mar 14 13:50:46 UTC 2020 > > > > Modified Files: > > src/sys/arch/x86/acpi: acpi_cpu_md.c > > > > Log Message: > > Put ACPI idle under ACPICPU_ENABLE_C3 until the wrinkles are ironed out. > > This seems well written and basically all good, but currently doesn't enter > > a low power state, and imposes a big performance penalty. Proposed on > > port-i386 & port-amd64. > > > > > > To generate a diff of this commit: > > cvs rdiff -u -r1.81 -r1.82 src/sys/arch/x86/acpi/acpi_cpu_md.c > > > > Please note that diffs are not public domain; they are subject to the > > copyright notices on the relevant files. > > This change seems to cause my machine to hang while booting. Reverting > the change seems to fix the problem. Here are the details: Same here, see kern/55080. Martin
Re: CVS commit: src/sys/arch/x86/acpi
Dear Andrew, On 2020-03-14, Andrew Doran wrote: > Module Name: src > Committed By: ad > Date: Sat Mar 14 13:50:46 UTC 2020 > > Modified Files: > src/sys/arch/x86/acpi: acpi_cpu_md.c > > Log Message: > Put ACPI idle under ACPICPU_ENABLE_C3 until the wrinkles are ironed out. > This seems well written and basically all good, but currently doesn't enter > a low power state, and imposes a big performance penalty. Proposed on > port-i386 & port-amd64. > > > To generate a diff of this commit: > cvs rdiff -u -r1.81 -r1.82 src/sys/arch/x86/acpi/acpi_cpu_md.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. This change seems to cause my machine to hang while booting. Reverting the change seems to fix the problem. Here are the details: $ cpuctl identify 0 cpu0: highest basic info 0005 cpu0: highest extended info 801b cpu0: "AMD Athlon(tm) II X3 450 Processor" cpu0: AMD Family 10h (686-class), 3200.22 MHz cpu0: family 0x10 model 0x5 stepping 0x3 (id 0x100f53) cpu0: features 0x178bfbff cpu0: features 0x178bfbff cpu0: features1 0x802009 cpu0: features2 0xefd3fbff cpu0: features2 0xefd3fbff cpu0: features3 0x37ff cpu0: features3 0x37ff cpu0: I-cache 64KB 64B/line 2-way, D-cache 64KB 64B/line 2-way cpu0: L2 cache 512KB 64B/line 16-way cpu0: ITLB 32 4KB entries fully associative, 16 2MB entries fully associative cpu0: DTLB 48 4KB entries fully associative, 48 2MB entries fully associative cpu0: L2 ITLB 512 4KB entries 4-way cpu0: L2 DTLB 512 4KB entries 4-way, 128 2MB entries 2-way cpu0: L1 1GB page DTLB 48 1GB entries fully associative cpu0: L2 1GB page DTLB 16 1GB entries 8-way cpu0: Initial APIC ID 0 cpu0: Cluster/Package ID 0 cpu0: Core ID 0 cpu0: SMT ID 0 cpu0: MONITOR/MWAIT extensions 0x3 cpu0: monitor-line size 64 cpu0: AMD Power Management features: 0x1f9 cpu0: SVM Rev. 1 cpu0: SVM NASID 64 cpu0: SVM features 0xf cpu0: UCode version: 0x1c8 $ dmesg | fgrep acpicpu [ 1.014781] acpicpu0 at cpu0: ACPI CPU [ 1.014781] acpicpu0: C1: HLT, lat 0 us, pow 0 mW [ 1.014781] acpicpu0: P0: FFH, lat 4 us, pow 34375 mW, 3200 MHz [ 1.014781] acpicpu0: P1: FFH, lat 4 us, pow 25245 mW, 2500 MHz [ 1.014781] acpicpu0: P2: FFH, lat 4 us, pow 22200 mW, 2000 MHz [ 1.014781] acpicpu0: P3: FFH, lat 4 us, pow 12095 mW, 800 MHz [ 1.014781] acpicpu0: T0: I/O, lat 1 us, pow 0 mW, 100 % [ 1.014781] acpicpu0: T1: I/O, lat 1 us, pow 0 mW, 88 % [ 1.014781] acpicpu0: T2: I/O, lat 1 us, pow 0 mW, 76 % [ 1.014781] acpicpu0: T3: I/O, lat 1 us, pow 0 mW, 64 % [ 1.014781] acpicpu0: T4: I/O, lat 1 us, pow 0 mW, 52 % [ 1.014781] acpicpu0: T5: I/O, lat 1 us, pow 0 mW, 40 % [ 1.014781] acpicpu0: T6: I/O, lat 1 us, pow 0 mW, 28 % [ 1.014781] acpicpu0: T7: I/O, lat 1 us, pow 0 mW, 16 % [ 1.014781] acpicpu1 at cpu1: ACPI CPU [ 1.014781] acpicpu2 at cpu2: ACPI CPU [ 1.017385] acpicpu0 at cpu0: ACPI CPU [ 1.017385] acpicpu0: C1: HLT, lat 0 us, pow 0 mW [ 1.017385] acpicpu0: P0: FFH, lat 4 us, pow 34375 mW, 3200 MHz [ 1.017385] acpicpu0: P1: FFH, lat 4 us, pow 25245 mW, 2500 MHz [ 1.017385] acpicpu0: P2: FFH, lat 4 us, pow 22200 mW, 2000 MHz [ 1.017385] acpicpu0: P3: FFH, lat 4 us, pow 12095 mW, 800 MHz [ 1.017385] acpicpu0: T0: I/O, lat 1 us, pow 0 mW, 100 % [ 1.017385] acpicpu0: T1: I/O, lat 1 us, pow 0 mW, 88 % [ 1.017385] acpicpu0: T2: I/O, lat 1 us, pow 0 mW, 76 % [ 1.017385] acpicpu0: T3: I/O, lat 1 us, pow 0 mW, 64 % [ 1.017385] acpicpu0: T4: I/O, lat 1 us, pow 0 mW, 52 % [ 1.017385] acpicpu0: T5: I/O, lat 1 us, pow 0 mW, 40 % [ 1.017385] acpicpu0: T6: I/O, lat 1 us, pow 0 mW, 28 % [ 1.017385] acpicpu0: T7: I/O, lat 1 us, pow 0 mW, 16 % [ 1.017385] acpicpu1 at cpu1: ACPI CPU [ 1.017385] acpicpu2 at cpu2: ACPI CPU -- Kind regards, Yorick Hardy
