Re: CVS commit: src/sys/arch

2021-09-23 Thread Izumi Tsutsui
> Module Name:  src
> Committed By: skrll
> Date: Thu Sep 23 06:34:00 UTC 2021
> 
> Modified Files:
>   src/sys/arch/aarch64/aarch64: cpufunc.c
>   src/sys/arch/arm/arm32: cpu.c
> 
> Log Message:
> Print the cache information in similar formats and arm and aarch64, e.g.
 :
> aarch64 before
> [   1.030] cpu1: L1 48KB/64B*256L*3W PIPT Instruction cache
> [   1.030] cpu1: L1 32KB/64B*256L*2W PIPT Data cache
> [   1.030] cpu1: L2 2048KB/64B*2048L*16W PIPT Unified cache
> 
> aarch64 after
> [   1.030] cpu1: L1 48KB/64B 3-way (256 set) PIPT Instruction cache
> [   1.030] cpu1: L1 32KB/64B 2-way (256 set) PIPT Data cache
> [   1.030] cpu1: L2 2048KB/64B 16-way (2048 set) PIPT Unified cache

I prefer "line(s)" as before rather than "set(s)" since
the latter implies "N-way set associative cache".

---
Izumi Tsutsui


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

2021-09-08 Thread Rin Okuyama

On 2021/09/08 20:47, Izumi Tsutsui wrote:

Module Name:src
Committed By:   rin
Date:   Wed Sep  8 07:22:56 UTC 2021

Modified Files:
src/sys/arch/sh3/sh3: pmap.c

Log Message:
Redo a part of rev 1.89:

- Remove redundant parentheses/braces/comments.
- Fix indents.

No binary changes confirmed this time.


---
-   if (kva) {
+   if (kva)
entry |= PG_V | PG_SH |
((prot & VM_PROT_WRITE) ?
(PG_PR_KRW | PG_D) : PG_PR_KRO);
-   } else {
+   else
entry |= PG_V |
((prot & VM_PROT_WRITE) ?
(PG_PR_URW | PG_D) : PG_PR_URO);
-   }
}
---

This part doesn't match KNF:
  http://cvsweb.netbsd.org/bsdweb.cgi/src/share/misc/style#rev1.58


Update style around single-line braces according to discussion.

https://mail-index.netbsd.org/tech-userlevel/2020/07/12/msg012536.html
https://mail-index.netbsd.org/tech-kern/2020/07/12/msg026594.html

Retain some examples of technically unnecessary braces that likely
aid legibility from the previous commit.


So I don't think removing existing ones per "redundant" is a valid reason.


Oops, I misread style. These and one more similar braces have been restored.
Thanks for pointing it out!

rin


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

2021-09-08 Thread Izumi Tsutsui
> Module Name:  src
> Committed By: rin
> Date: Wed Sep  8 07:22:56 UTC 2021
> 
> Modified Files:
>   src/sys/arch/sh3/sh3: pmap.c
> 
> Log Message:
> Redo a part of rev 1.89:
> 
> - Remove redundant parentheses/braces/comments.
> - Fix indents.
> 
> No binary changes confirmed this time.

---
-   if (kva) {
+   if (kva)
entry |= PG_V | PG_SH |
((prot & VM_PROT_WRITE) ?
(PG_PR_KRW | PG_D) : PG_PR_KRO);
-   } else {
+   else
entry |= PG_V |
((prot & VM_PROT_WRITE) ?
(PG_PR_URW | PG_D) : PG_PR_URO);
-   }
}
---

This part doesn't match KNF:
 http://cvsweb.netbsd.org/bsdweb.cgi/src/share/misc/style#rev1.58

> Update style around single-line braces according to discussion.
> 
> https://mail-index.netbsd.org/tech-userlevel/2020/07/12/msg012536.html
> https://mail-index.netbsd.org/tech-kern/2020/07/12/msg026594.html
> 
> Retain some examples of technically unnecessary braces that likely
> aid legibility from the previous commit.

So I don't think removing existing ones per "redundant" is a valid reason.

---
Izumi Tsutsui


Re: CVS commit: src/sys/arch/evbarm/conf

2021-08-31 Thread Rin Okuyama

Hi,

On 2021/08/30 18:20, matthew green wrote:

hi.


nice work on BE marvell :)


Thanks!


"Rin Okuyama" writes:

Module Name:src
Committed By:   rin
Date:   Mon Aug 30 00:12:15 UTC 2021

Modified Files:
src/sys/arch/evbarm/conf: MARVELL_NAS

Log Message:
Enable FFS_EI and DISKLABEL_EI as this SoC supports both endians now.


personally, i think we should do these everywhere that isn't
size constrained.  they're useful for accessing other platform
disks quite often on all netbsd systems.


Yeah, I agree. Probably, we can have these options in
sys/conf/filesystems.config. Most powerful platforms have
already used that file:


$ cd /usr/src/sys/arch && grep filesystems.conf */conf/*
amd64/conf/GENERIC:include "conf/filesystems.config"
amd64/conf/XEN3_DOM0:include "conf/filesystems.config"
amd64/conf/XEN3_DOMU:include "conf/filesystems.config"
evbarm/conf/GENERIC.common:include "conf/filesystems.config"
i386/conf/GENERIC:include "conf/filesystems.config"
macppc/conf/GENERIC:include "conf/filesystems.config"
riscv/conf/GENERIC:include  "conf/filesystems.config"
sgimips/conf/GENERIC32_IP2x:include "conf/filesystems.config"
sgimips/conf/GENERIC32_IP3x:include "conf/filesystems.config"
sgimips/conf/GENERIC32_IP12:include "conf/filesystems.config"
sparc64/conf/GENERIC:include "conf/filesystems.config"


And we can switch other machines to use it.

Thanks,
rin


re: CVS commit: src/sys/arch/evbarm/conf

2021-08-30 Thread matthew green
hi.


nice work on BE marvell :)

"Rin Okuyama" writes:
> Module Name:  src
> Committed By: rin
> Date: Mon Aug 30 00:12:15 UTC 2021
>
> Modified Files:
>   src/sys/arch/evbarm/conf: MARVELL_NAS
>
> Log Message:
> Enable FFS_EI and DISKLABEL_EI as this SoC supports both endians now.

personally, i think we should do these everywhere that isn't
size constrained.  they're useful for accessing other platform
disks quite often on all netbsd systems.


.mrg.


Re: CVS commit: src/sys/arch/arm/xscale

2021-08-06 Thread Kengo NAKAHARA

Hi,

On 2021/08/06 17:58, Rin Okuyama wrote:

Module Name:src
Committed By:   rin
Date:   Fri Aug  6 08:58:42 UTC 2021

Modified Files:
src/sys/arch/arm/xscale: i80321_intr.h

Log Message:
Do *NOT* lower IPL in i80321_splraise().

Fix various strange crashes for DIAGNOSTIC kernel on evbarm/HDL_G,
including one worked around by if_wm.c rev 1.706:

http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/pci/if_wm.c#rev1.706


if_wm.c:r1.706 also fixes a potential bug which causes infinite loop
when do "sysctl -w hw.wmX.txrx_workqueue=1" on single processor system.
So, I leave the code.


Thanks,


--
//
Internet Initiative Japan Inc.

Device Engineering Section,
Product Division,
Technology Unit

Kengo NAKAHARA 




Re: CVS commit: src/sys/arch/arm/include/arm32

2021-05-30 Thread Simon Burge
"Rin Okuyama" wrote:

> Module Name:  src
> Committed By: rin
> Date: Sun May 30 07:20:00 UTC 2021
>
> Modified Files:
>
>   src/sys/arch/arm/include/arm32: param.h
>
> Log Message:
>
> Include opt_param.h for MSGBUFSIZE ifdef _KERNEL_OPT.

Thanks Rin!  I thought I had checked all the ways MSGBUFSIZE
was pulled in.

Cheers,
Simon.


Re: CVS commit: src/sys/arch/powerpc/include/booke

2021-04-21 Thread Rin Okuyama

On 2021/04/18 0:38, Joerg Sonnenberger wrote:

On Sat, Apr 17, 2021 at 01:25:57PM +, Rin Okuyama wrote:

Module Name:src
Committed By:   rin
Date:   Sat Apr 17 13:25:57 UTC 2021

Modified Files:
src/sys/arch/powerpc/include/booke: vmparam.h

Log Message:
Sync MAXfoo params with oea:

   MAXTSIZ: 512MB -> 128MB
   MAXDSIZ: 3.25GB -> 1GB

There should be no particular reasons for having different values.


Is there an architectural reason for MAXTSIZ to be 128MB? Because we
have seen binaries larger than that.

Ah, I just forgot recent discussion on MAXTSIZ.

There's no limitation due to architecture. I've confirmed binary just
works fine, whose text is larger than 256MB segment on oea. It works
on booke and ibm4xx also.

Maybe we can leave MAXTSIZ undefined for powerpc.

Thanks,
rin


Re: CVS commit: src/sys/arch/powerpc/include/booke

2021-04-17 Thread Joerg Sonnenberger
On Sat, Apr 17, 2021 at 01:25:57PM +, Rin Okuyama wrote:
> Module Name:  src
> Committed By: rin
> Date: Sat Apr 17 13:25:57 UTC 2021
> 
> Modified Files:
>   src/sys/arch/powerpc/include/booke: vmparam.h
> 
> Log Message:
> Sync MAXfoo params with oea:
> 
>   MAXTSIZ: 512MB -> 128MB
>   MAXDSIZ: 3.25GB -> 1GB
> 
> There should be no particular reasons for having different values.

Is there an architectural reason for MAXTSIZ to be 128MB? Because we
have seen binaries larger than that.

Joerg


Re: CVS commit: src/sys/arch

2021-04-02 Thread Simon Burge
"Rin Okuyama" wrote:

> Module Name:  src
> Committed By: rin
> Date: Fri Apr  2 12:11:42 UTC 2021
>
> Log Message:
>
> For ports with __HAVE_LEGACY_INTRCNT, turn intrcnt[] and derived
> variables into u_int, to match with kern/subr_evcnt.c.

Thanks Rin!

Cheers,
Simon.


Re: CVS commit: src/sys/arch/powerpc/oea

2021-04-01 Thread Jason Thorpe
Ugh.  Can we please stop making these hacky one-offs in "shared by all PowerPC 
platforms" code?  This actually points to a deeper problem in the pmap code 
that needs to be addressed correctly.

> On Apr 1, 2021, at 3:02 PM, Michael Lorenz  wrote:
> 
> Module Name:  src
> Committed By: macallan
> Date: Thu Apr  1 22:02:20 UTC 2021
> 
> Modified Files:
>   src/sys/arch/powerpc/oea: ofwoea_machdep.c
> 
> Log Message:
> avoid mapping 0xf000 - my beige G3 DSIs on it
> with this my the machine boots again
> tested on a variety of G4 and G5 models with no problems
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.59 -r1.60 src/sys/arch/powerpc/oea/ofwoea_machdep.c
> 
> 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/evbmips/stand/sbmips

2021-03-16 Thread Christos Zoulas
Yes, the binary is mips64-n32. There is no n32 for mips32.
I moved the CPUFLAGS=-n32 assignment to the 64 bit
portion of the Makefile.

christos

> On Mar 16, 2021, at 12:14 AM, matthew green  wrote:
> 
> "Christos Zoulas" writes:
>> Module Name: src
>> Committed By:christos
>> Date:Mon Mar 15 18:13:54 UTC 2021
>> 
>> Modified Files:
>>  src/sys/arch/evbmips/stand/sbmips: Makefile.bootprogs
>> 
>> Log Message:
>> - 32 bit mips uses oabi, don't force it to n32.
>> - compile assembly code with soft-float to kill linker warnings
> 
> this doesn't make sense to me.  it should be compiled
> as if for 64 bit CPU (it is.)  it even has this:
> 
> CFLAGS+=   -mips64
> CFLAGS+=   -Werror ${CWARNFLAGS}
> -CPUFLAGS+= -mabi=n32
> 
> two lines above the removed -mabi=n32.
> 
> what was the real problem here?  (the soft-float part
> seems reasonable.)
> 
> 
> .mrg.



signature.asc
Description: Message signed with OpenPGP


re: CVS commit: src/sys/arch/evbmips/stand/sbmips

2021-03-15 Thread matthew green
"Christos Zoulas" writes:
> Module Name:  src
> Committed By: christos
> Date: Mon Mar 15 18:13:54 UTC 2021
>
> Modified Files:
>   src/sys/arch/evbmips/stand/sbmips: Makefile.bootprogs
>
> Log Message:
> - 32 bit mips uses oabi, don't force it to n32.
> - compile assembly code with soft-float to kill linker warnings

this doesn't make sense to me.  it should be compiled
as if for 64 bit CPU (it is.)  it even has this:

 CFLAGS+=   -mips64
 CFLAGS+=   -Werror ${CWARNFLAGS}
-CPUFLAGS+= -mabi=n32

two lines above the removed -mabi=n32.

what was the real problem here?  (the soft-float part
seems reasonable.)


.mrg.


Re: CVS commit: src/sys/arch/amd64/conf

2021-03-05 Thread Greg Troxel

matthew green  writes:

> could this be done with include and "no foo" statement?
> eg, like sys/arch/sparc/conf/INSTALL does.

Maybe, but I'm not sure it will end up working.  Right now we don't know
if any of the missing things will be trouble, and even if we do move to
include/no I'd like to do that via an intermediate step of a config with
small differences.

Also I think we should also consider extracting lots of things into
includable files, similar to how

  include "dev/usb/usbdevices.config"

is used now in GENERIC.   That will raise interesting cross-arch issues
about value vs kernel memory usage probably.

These include files would allow a simplification of XEN3_DOMU which as I
see it should have ~no drivers but all the rest of the options.


signature.asc
Description: PGP signature


re: CVS commit: src/sys/arch/amd64/conf

2021-03-05 Thread matthew green
"Greg Troxel" writes:
> Module Name:  src
> Committed By: gdt
> Date: Fri Mar  5 20:30:56 UTC 2021
>
> Modified Files:
>   src/sys/arch/amd64/conf: XEN3_DOM0
>
> Log Message:
> XEN3_DOM0: Approach GENERIC
>
> When processed to remove comments, blank lines, normalize whitespace,
> and sort/uniq (one line was previously duplicated), this file is
> identical to the previous version.  It has been reorganized to reduce
> diffs to GENERIC, and many missing lines from GENERIC have been added
> but commented out.

could this be done with include and "no foo" statement?
eg, like sys/arch/sparc/conf/INSTALL does.


.mrg.


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


  1   2   3   4   5   6   7   8   9   10   >