Re: CVS commit: src/sys/arch/amd64
On Sat, Jan 04, 2020 at 08:43:16AM +0100, Maxime Villard wrote: > +.section multiboot,"",@note > Why @note? It will be in the .text anyway. Also why no dot in the section > name? That's supposed to be the naming convention. The idea is that one day if ld gets more reasonable, it could go in non-loading note ection at the beginning of the binary, but if you prefer .text, let us go with that. On the section name, ELF specification says "Section names with a dot (.) prefix are reserved for the system" (TIS ELF specification version 1.2), section names without a dot are allowed, and we use plenty of them in our kernels (e.g.: link_set_* sections). Our naming convention is not obvious to me, nor what the specification means by "the system" here. My hunch would be to avoid using an abitratry name inside a reserved namespace, althought we already did it. If you have a strong opinion on it, I can stand a leading dot in the multiboot section name. > I don't know if you realize, but you landed a huge pile > of crap in the middle of the amd64 locore I have been working on this, but the priority was obviously the boot problem. Attached is my latest change set, including the locore cleanup you asked for. -- Emmanuel Dreyfus m...@netbsd.org Index: sys/arch/amd64/amd64/locore.S === RCS file: /cvsroot/src/sys/arch/amd64/amd64/locore.S,v retrieving revision 1.195 diff -U4 -r1.195 locore.S --- sys/arch/amd64/amd64/locore.S 15 Dec 2019 02:58:21 - 1.195 +++ sys/arch/amd64/amd64/locore.S 5 Jan 2020 00:41:18 - @@ -431,10 +431,10 @@ .size tmpstk, tmpstk - . .space 512 tmpstk: -.section multiboot,"a" #if defined(MULTIBOOT) +.section multiboot .align 8 .globl Multiboot2_Header _C_LABEL(Multiboot2_Header): .intMULTIBOOT2_HEADER_MAGIC @@ -473,9 +473,9 @@ .int8 /* sizeof(struct multiboot_tag) */ .align 8 .globl Multiboot2_Header_end _C_LABEL(Multiboot2_Header_end): -#endif /* MULTIBOOT */ +#endif /* MULTIBOOT */ /* * Some hackage to deal with 64bit symbols in 32 bit mode. * This may not be needed if things are cleaned up a little. @@ -544,109 +544,13 @@ mov $(KERNTEXTOFF - KERNBASE), %rdi /* dest */ mov %r8, %rsi sub $(start - kernel_text), %rsi/* src */ mov $(__kernel_end - kernel_text), %rcx /* size */ - mov %rcx, %r12 - movq%rdi, %r11 /* for misaligned check */ - -#if !defined(NO_OVERLAP) - movq%rdi, %r13 - subq%rsi, %r13 -#endif - - shrq$3, %rcx/* count for copy by words */ - jz 8f /* j if less than 8 bytes */ - - lea -8(%rdi, %r12), %r14/* target address of last 8 */ - mov -8(%rsi, %r12), %r15/* get last word */ -#if !defined(NO_OVERLAP) - cmpq%r12, %r13 /* overlapping? */ - jb 10f -#endif - -/* - * Non-overlaping, copy forwards. - * Newer Intel cpus (Nehalem) will do 16byte read/write transfers - * if %ecx is more than 76. - * AMD might do something similar some day. - */ - and $7, %r11/* destination misaligned ? */ - jnz 12f - rep - movsq - mov %r15, (%r14)/* write last word */ - jmp .Lcopy_done -/* - * Destination misaligned - * AMD say it is better to align the destination (not the source). - * This will also re-align copies if the source and dest are both - * misaligned by the same amount) - * (I think Nehalem will use its accelerated copy if the source - * and destination have the same alignment.) - */ -12: - lea -9(%r11, %r12), %rcx/* post re-alignment count */ - neg %r11/* now -1 .. -7 */ - mov (%rsi), %r12/* get first word */ - mov %rdi, %r13 /* target for first word */ - lea 8(%rsi, %r11), %rsi - lea 8(%rdi, %r11), %rdi - shr $3, %rcx - rep - movsq - mov %r12, (%r13)/* write first word */ - mov %r15, (%r14)/* write last word */ - jmp .Lcopy_done - -#if !defined(NO_OVERLAP) -/* Must copy backwards. - * Reverse copy is probably easy to code faster than 'rep movds' - * since that requires (IIRC) an extra clock every 3 iterations (AMD). - * However I don't suppose anything cares that much! - * The big cost is the std/cld pair - reputedly 50+ cycles on Netburst P4. - * The copy is aligned with the buffer start (more likely to - * be a multiple of 8 than the end). - */ -10: - lea -8(%rsi, %rcx, 8), %rsi - lea -8(%rdi, %rcx, 8), %rdi - std + /* Assume non overlap and aligned size */ + shrq$3, %rcx rep movsq - cld
Re: CVS commit: src/sys/arch/amd64
On Sat, Jan 04, 2020 at 08:43:16AM +0100, Maxime Villard wrote: > As said repeatedly, the option should be enabled only _after_ the garbage > has been cleaned up. This is not easy if you just call it that. To me it looks like Emanuel is trying very hard to address all technical issues brought up explicitly and clear. If this patch does not totaly "clean up the garbage", it would help if you raise specific issues. If it does, why not include the re-enabling in the patch? Martin
Re: CVS commit: src/sys/arch/zaurus/conf
> > The problem is caused by sys/arch/arm/conf/Makefile.arm. > > It defines "COPTS+= -mapcs-frame" in recent rev 1.52 > > > > http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/arch/arm/conf/Makefile.arm#rev1.52 > > but MI sys/conf/Makefile.kern.inc defines COPTS+=-O2 > > only if COPTS is empty. > > I don't see how as sys/arch/zaurus/conf/INSTALL doesn't define DDB. On zaurus GENERIC also has a size restriction (due to bootloader). > > This affects not only zaurus but all arm ports? > > Maybe this patch is better? I wonder if it will work as expected if COPTS+="-Os" is already specified with options DDB, if -mapcs-frame is mandatory for DDB. But if -fno-omit-frame-pointer in Makefile.amd64 may have the similar problem, it's also okay for arm, IMO. Maybe it's less probematic to add a new option (COPTS_MD etc.) and add it to COPTS after including Makefile.kern.inc, i.e. in "(11) Appending make options." in Makefile.arm? --- Izumi Tsutsui
Re: CVS commit: src/sys/arch/zaurus/conf
On 03/01/2020 18:42, Izumi Tsutsui wrote:
I wrote:
The missing COPTS was an accident or fallout from some other changes?
Isn't it specified in -current?
The problem is caused by sys/arch/arm/conf/Makefile.arm.
It defines "COPTS+= -mapcs-frame" in recent rev 1.52
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/arch/arm/conf/Makefile.arm#rev1.52
but MI sys/conf/Makefile.kern.inc defines COPTS+=-O2
only if COPTS is empty.
I don't see how as sys/arch/zaurus/conf/INSTALL doesn't define DDB.
This affects not only zaurus but all arm ports?
Maybe this patch is better?
Nick
Index: sys/arch/arm/conf/Makefile.arm
===
RCS file: /cvsroot/src/sys/arch/arm/conf/Makefile.arm,v
retrieving revision 1.52
diff -u -p -r1.52 Makefile.arm
--- sys/arch/arm/conf/Makefile.arm 2 Jan 2020 14:33:55 - 1.52
+++ sys/arch/arm/conf/Makefile.arm 4 Jan 2020 15:02:40 -
@@ -78,7 +78,7 @@ CFLAGS+= -mno-unaligned-access
OPT_DDB= %DDB%
.if !empty(OPT_DDB) && ${HAVE_GCC:U0} > 0
-COPTS+=-mapcs-frame
+DEFCOPTS= -O2 -mapcs-frame
.endif
##
Re: CVS commit: src/sys/arch/amd64
Le 04/01/2020 à 03:33, Emmanuel Dreyfus a écrit : On Tue, Dec 31, 2019 at 09:32:05AM +0100, Maxime Villard wrote: I think max-page-size=0x1000 is the right thing to do, but someone needs to verify that the resulting binary is correct and that the resulting in-memory layout is correct too. Attached is an updated patch with this approach. I tested at mine and it seems fine. Come on... "I tested and it seems fine"... Whatever. I have now verified that the resulting binary layout is correct, that the in-memory layout is correct, and that libsa doesn't do crazy things with this binary. As far as my concerns were concerned, the max-page-size change is good to go. The rest is confused: +.section multiboot,"",@note Why @note? It will be in the .text anyway. Also why no dot in the section name? That's supposed to be the naming convention. +EXTRA_LINKFLAGS= --split-by-file=0x10 -z max-page-size=0x1000 -r -d KASLR kernels to not have a PHDR, so this is not relevant -- there was a reason this parameter wasn't getting passed in the first place. +optionsMULTIBOOT # Multiboot support (see multiboot(8)) As said repeatedly, the option should be enabled only _after_ the garbage has been cleaned up. In fact, why don't you revert your change, fix it correctly locally, and then re-submit it? I don't know if you realize, but you landed a huge pile of crap in the middle of the amd64 locore, not only does this crap not work but it also breaks EFI boot, and for two weeks you've been wondering what's wrong in it and you have consistently proposed absurd workarounds. Please revert your change entirely and put back the code in a clean and functional state. Thanks. Maxime
Re: boottime
As discussed on IRC/ICB, while we don't claim compatability, in this case it doesn't hurt to provide it. On Sat, 4 Jan 2020, m...@netbsd.org wrote: - Forwarded message from Paul Goyette - Log Message: Resurrect boottime, but only in the compat_90 module (whether built-in or separately loaded). This will enable running of old vmstat(1) images on newer kernels. To generate a diff of this commit: cvs rdiff -u -r1.2 -r1.3 src/sys/compat/common/compat_90_mod.c Why? we don't claim compatibility for libkvm users. !DSPAM:5e105996108288244013799! ++--+---+ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com | | Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org | ++--+---+
Re: CVS commit: src/lib/libc/sys
On 04.01.2020 05:40, Kamil Rytarowski wrote:
> Module Name: src
> Committed By: kamil
> Date: Sat Jan 4 04:40:17 UTC 2020
>
> Modified Files:
> src/lib/libc/sys: ptrace.2
>
> Log Message:
> /tmp/cvsbigmGa
>
Document PT_LWPSTATUS and PT_LWPNEXT in ptrace(2)
Remove mentions of obsolete PT_LWPINFO.
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.82 -r1.83 src/lib/libc/sys/ptrace.2
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>
>
> Modified files:
>
> Index: src/lib/libc/sys/ptrace.2
> diff -u src/lib/libc/sys/ptrace.2:1.82 src/lib/libc/sys/ptrace.2:1.83
> --- src/lib/libc/sys/ptrace.2:1.82Wed Oct 9 14:20:47 2019
> +++ src/lib/libc/sys/ptrace.2 Sat Jan 4 04:40:17 2020
> @@ -1,7 +1,7 @@
> -.\" $NetBSD: ptrace.2,v 1.82 2019/10/09 14:20:47 wiz Exp $
> +.\" $NetBSD: ptrace.2,v 1.83 2020/01/04 04:40:17 kamil Exp $
> .\"
> .\" This file is in the public domain.
> -.Dd October 9, 2019
> +.Dd January 4, 2019
> .Dt PTRACE 2
> .Os
> .Sh NAME
> @@ -399,7 +399,7 @@ argument should contain the name of the
> and the
> .Fa data
> argument should contain the length of the core filename.
> -.It Dv PT_LWPINFO
> +.It Dv PT_LWPSTATUS
> Returns information about a thread from the list of threads for the
> process specified in the
> .Fa pid
> @@ -407,41 +407,50 @@ argument.
> The
> .Fa addr
> argument should contain a
> -.Vt struct ptrace_lwpinfo
> +.Vt struct ptrace_lwpstatus
> defined as:
> .Bd -literal -offset indent
> -struct ptrace_lwpinfo {
> +struct ptrace_lwpstatus {
> lwpid_t pl_lwpid;
> - int pl_event;
> + sigset_t pl_sigpend;
> + sigset_t pl_sigmask;
> + char pl_name[20];
> + void *pl_private;
> };
> .Ed
> .Pp
> where
> .Fa pl_lwpid
> contains a thread LWP ID.
> -Information is returned for the thread following the one with the
> +Information is returned for the thread specified in
> +.Fa pl_lwpid .
> +.Fa pl_sigpend
> +contains the signals pending on that LWP.
> +.Fa pl_sigmask
> +contains the signals masked on that LWP.
> +.Fa pl_name
> +contains printable name of the LWP.
> +The string is always NUL terminated.
> +.Fa pl_private
> +contains the pointer to TLS base.
> +.Pp
> +The
> +.Fa data
> +argument should contain
> +.Dq Li sizeof(struct ptrace_lwpinfo) .
> +.It Dv PT_LWPNEXT
> +Is the same as
> +.Dv PT_LWPSTATUS ,
> +except that information is returned for the thread following the one with the
> specified ID in the process thread list, or for the first thread
> if
> .Fa pl_lwpid
> is 0.
> +.Pp
> Upon return
> .Fa pl_lwpid
> contains the LWP ID of the thread that was found, or 0 if there is
> no thread after the one whose LWP ID was supplied in the call.
> -.Fa pl_event
> -contains the event that stopped the thread.
> -Possible values are:
> -.Pp
> -.Bl -tag -width 30n -offset indent -compact
> -.It Dv PL_EVENT_NONE
> -.It Dv PL_EVENT_SIGNAL
> -.It Dv PL_EVENT_SUSPENDED
> -.El
> -.Pp
> -The
> -.Fa data
> -argument should contain
> -.Dq Li sizeof(struct ptrace_lwpinfo) .
> .It Dv PT_SYSCALL
> Stops a process before and after executing each system call.
> Otherwise this operation is the same as
> @@ -987,10 +996,3 @@ to
> .Fn ptrace
> .Ec ,
> should be able to sidestep this.
> -.Pp
> -.Dv PT_SET_SIGINFO ,
> -.Dv PT_RESUME
> -and
> -.Dv PT_SUSPEND
> -can change the image of process returned by
> -.Dv PT_LWPINFO .
>
signature.asc
Description: OpenPGP digital signature
boottime
- Forwarded message from Paul Goyette - Log Message: Resurrect boottime, but only in the compat_90 module (whether built-in or separately loaded). This will enable running of old vmstat(1) images on newer kernels. To generate a diff of this commit: cvs rdiff -u -r1.2 -r1.3 src/sys/compat/common/compat_90_mod.c Why? we don't claim compatibility for libkvm users.
