Re: CVS commit: src/sys/arch/aarch64/aarch64

2021-02-23 Thread Jared McNeill

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

2021-02-22 Thread Jason Thorpe


> 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

2021-02-22 Thread Ryo Shimizu


>> 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

2021-02-22 Thread Nick Hudson

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

2021-02-22 Thread Ryo Shimizu


>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

2021-02-04 Thread Michael
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

2021-02-04 Thread Tetsuya Isaki
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

2021-02-03 Thread Michael
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

2021-01-25 Thread Jason Thorpe


> 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

2021-01-25 Thread Kamil Rytarowski
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

2021-01-25 Thread Valery Ushakov
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

2021-01-25 Thread Robert Elz
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

2021-01-25 Thread Valery Ushakov
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

2021-01-25 Thread Jason Thorpe


> 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

2021-01-25 Thread Kamil Rytarowski
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

2021-01-22 Thread Jared McNeill
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

2021-01-21 Thread Jared McNeill
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

2021-01-20 Thread matthew green
"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

2021-01-20 Thread Martin Husemann
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

2021-01-19 Thread Martin Husemann
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

2021-01-17 Thread matthew green
> 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

2021-01-17 Thread Rin Okuyama

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

2021-01-17 Thread Rin Okuyama

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

2021-01-01 Thread Jared McNeill
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

2020-11-21 Thread Rin Okuyama

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

2020-11-11 Thread Rin Okuyama

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

2020-11-11 Thread matthew green
"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

2020-10-24 Thread Julian Coleman
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

2020-10-24 Thread Tobias Nygren
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

2020-10-13 Thread Martin Husemann
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

2020-10-13 Thread Rin Okuyama

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

2020-10-13 Thread Kamil Rytarowski
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

2020-10-06 Thread Thomas Klausner
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

2020-10-06 Thread Rin Okuyama

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

2020-10-06 Thread Nick Hudson



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

2020-10-05 Thread Rin Okuyama

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

2020-10-01 Thread Ryo Shimizu
>> 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

2020-10-01 Thread Nick Hudson

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

2020-09-24 Thread Ryo Shimizu


>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

2020-09-24 Thread Nick Hudson

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

2020-09-03 Thread Jason Thorpe


> 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

2020-09-03 Thread matthew green
"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

2020-08-16 Thread Rin Okuyama

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

2020-08-10 Thread matthew green
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

2020-08-08 Thread Rin Okuyama

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

2020-08-01 Thread Taylor R Campbell
> 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

2020-07-21 Thread matthew green
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

2020-07-06 Thread Rin Okuyama

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

2020-07-06 Thread Jason Thorpe



> 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

2020-07-02 Thread Jared McNeill
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

2020-07-02 Thread scole_mail
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

2020-07-02 Thread Martin Husemann
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

2020-07-02 Thread scole_mail
"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

2020-06-27 Thread Rin Okuyama

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

2020-06-27 Thread Jaromír Doleček
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

2020-06-26 Thread Simon Burge
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

2020-06-26 Thread Rin Okuyama

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

2020-06-24 Thread matthew green
"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

2020-06-24 Thread Joerg Sonnenberger
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

2020-06-24 Thread Taylor R Campbell
> 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

2020-06-22 Thread Joerg Sonnenberger
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

2020-06-21 Thread Rin Okuyama

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

2020-06-21 Thread Joerg Sonnenberger
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

2020-06-09 Thread Simon Burge
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

2020-06-09 Thread Simon Burge
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

2020-06-09 Thread Izumi Tsutsui
> 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

2020-06-06 Thread Christos Zoulas
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

2020-06-06 Thread Valery Ushakov
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

2020-06-06 Thread Kamil Rytarowski
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

2020-06-06 Thread Simon Burge
"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

2020-06-04 Thread Kamil Rytarowski
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

2020-06-04 Thread Andrew Doran
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

2020-06-03 Thread Kamil Rytarowski
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

2020-06-03 Thread Andrew Doran
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

2020-06-03 Thread Andrew Doran
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

2020-06-03 Thread Maxime Villard

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

2020-06-03 Thread Maxime Villard

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

2020-06-02 Thread Kamil Rytarowski
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

2020-06-02 Thread Andrew Doran
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

2020-06-02 Thread Maxime Villard

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

2020-05-28 Thread Maxime Villard

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

2020-05-28 Thread Andrew Doran
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

2020-05-28 Thread Maxime Villard

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

2020-05-28 Thread Maxime Villard

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

2020-05-27 Thread Maxime Villard

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

2020-05-23 Thread Ryo Shimizu


>> 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

2020-05-23 Thread maya
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

2020-05-23 Thread Jason Thorpe


> 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

2020-05-21 Thread Andrew Doran
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

2020-05-21 Thread Joerg Sonnenberger
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

2020-05-20 Thread Maxime Villard

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

2020-05-16 Thread Manuel Bouyer
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

2020-05-15 Thread matthew green
"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

2020-04-13 Thread Manuel Bouyer
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

2020-04-09 Thread Frank Kardel

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

2020-04-05 Thread Manuel Bouyer
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

2020-04-02 Thread Kengo NAKAHARA

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

2020-03-20 Thread Jared McNeill

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

2020-03-18 Thread Martin Husemann
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

2020-03-18 Thread Yorick Hardy
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


  1   2   3   4   5   6   7   8   9   10   >