Re: CVS commit: src/sys/arch/amd64/amd64
On 05.09.2019 14:57, Maxime Villard wrote: > Module Name: src > Committed By: maxv > Date: Thu Sep 5 12:57:30 UTC 2019 > > Modified Files: > src/sys/arch/amd64/amd64: lock_stubs.S > > Log Message: > Remove unused, and style. > > > To generate a diff of this commit: > cvs rdiff -u -r1.31 -r1.32 src/sys/arch/amd64/amd64/lock_stubs.S > > 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/amd64/amd64/lock_stubs.S > diff -u src/sys/arch/amd64/amd64/lock_stubs.S:1.31 > src/sys/arch/amd64/amd64/lock_stubs.S:1.32 > --- src/sys/arch/amd64/amd64/lock_stubs.S:1.31Mon Feb 11 14:59:32 2019 > +++ src/sys/arch/amd64/amd64/lock_stubs.S Thu Sep 5 12:57:30 2019 > @@ -1,6 +1,6 @@ > -/* $NetBSD: lock_stubs.S,v 1.31 2019/02/11 14:59:32 cherry Exp $ */ > +/* $NetBSD: lock_stubs.S,v 1.32 2019/09/05 12:57:30 maxv Exp $ */ > > -/*- > +/* > * Copyright (c) 2006, 2007, 2008, 2009 The NetBSD Foundation, Inc. > * All rights reserved. > * This is our style use /*- for comments that shall not be reformatted (originally indent(1) specific). signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/sys/arch/amd64/amd64
Le 21/08/2019 à 23:47, matthew green a écrit : "Maxime Villard" writes: Module Name:src Committed By: maxv Date: Wed Aug 21 16:35:10 UTC 2019 Modified Files: src/sys/arch/amd64/amd64: locore.S Log Message: Switch from printf to panic. These messages were notorious for being unreadable, and at least a clean panic allows the user to inspect the system via DDB. Also simplify the output, EAX gets overwritten with the error code so it indicates nothing meaningful. thanks for this. i'd been working on the same myself. do you have a reliable way to trigger this issue? i thought that returning to userland with a lock held would do it, but i wasn't able to get that to work reliably. there's more work related to crash dumps i'd like to work on but i got distracted by testing a change similar to this one and didn't get back to it yet. if you hard-code a splhigh() in a syscall and invoke it, you can see the message; to get the unreadable/garbage output you likely need to have two threads that invoke the syscall at the same time
re: CVS commit: src/sys/arch/amd64/amd64
"Maxime Villard" writes: > Module Name: src > Committed By: maxv > Date: Wed Aug 21 16:35:10 UTC 2019 > > Modified Files: > src/sys/arch/amd64/amd64: locore.S > > Log Message: > Switch from printf to panic. These messages were notorious for being > unreadable, and at least a clean panic allows the user to inspect the > system via DDB. Also simplify the output, EAX gets overwritten with > the error code so it indicates nothing meaningful. thanks for this. i'd been working on the same myself. do you have a reliable way to trigger this issue? i thought that returning to userland with a lock held would do it, but i wasn't able to get that to work reliably. there's more work related to crash dumps i'd like to work on but i got distracted by testing a change similar to this one and didn't get back to it yet. .mrg.
Re: CVS commit: src/sys/arch/amd64/amd64
In article , Maxime Villard wrote: > >This isn't correct, with USER_LDT the 32bit LWPs may have non-default segregs, >besides it is really dumb to mix 32 and 64bit code, part of the reasons why >I dropped the thing Yes, it is still missing the check that the compat_netbsd32 function had. Before you disabled the code it was possible to debug a 32 bit process with a 64 bit debugger. This is still useful because trying to debug a 32 bit process with a 32 bit debugger on a 64 system is extremely difficult to get it right because the 32 bit debugger needs to know somehow that it is running on a 64 bit system in order to mangle the paths properly and load the appropriate shared libraries. I think that the choice if we are going to let this work or not does not belong to the opinion of a single person, but to the developer base of NetBSD or the core group. christos
Re: CVS commit: src/sys/arch/amd64/amd64
Le 27/06/2019 à 04:00, Christos Zoulas a écrit : Module Name:src Committed By: christos Date: Thu Jun 27 02:00:31 UTC 2019 Modified Files: src/sys/arch/amd64/amd64: machdep.c Log Message: Although this is correct, I will let maxv commit it. Still waiting. To generate a diff of this commit: cvs rdiff -u -r1.333 -r1.334 src/sys/arch/amd64/amd64/machdep.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. This isn't correct, with USER_LDT the 32bit LWPs may have non-default segregs, besides it is really dumb to mix 32 and 64bit code, part of the reasons why I dropped the thing
Re: CVS commit: src/sys/arch/amd64/amd64
On Sun, Apr 22, 2018 at 09:09:40PM +0200, Maxime Villard wrote: > I recently told membership-exec that I would be less outspoken, and more > convivial, so here's a try: > > Le 22/04/2018 à 20:51, Joerg Sonnenberger a écrit : > > On Sun, Apr 22, 2018 at 12:36:36PM +0200, Maxime Villard wrote: > > > Where are they? I haven't been made aware of any issue related to > > > SVS+clang. > > > > Yes, I did make you aware that SVS killed VirtualBox. > > You are being dishonest. You did tell me that SVS didn't work with your > VirtualBox. At no point in time did you tell me that it was related to clang > or anything close to being a compiler issue, and not an implementation > issue. I didn't claim that now either. All I said is that SVS was known to be broken in my environment. Understanding the issue took a while as reproduction was annoying given that people continued to break the LLVM build every other day, so it was hard to use official images for testing. > In fact, if you want my point of view, you reported your "problem" in a way > that made me just unable to understand what it was about. I had to ask you > repeatedly, question after question, what is your virtualbox, what is your > cpu, is it hw-assisted, and so on. Shockingly, I would have included more data if I know whether any of the parameters are relevant. I originally ruled out LLVM since I thought it worked on a different (physical) machine. No longer sure I did, given that the machine is not supposed to use SVS for the obvious performance implications. Joerg
Re: CVS commit: src/sys/arch/amd64/amd64
I recently told membership-exec that I would be less outspoken, and more convivial, so here's a try: Le 22/04/2018 à 20:51, Joerg Sonnenberger a écrit : On Sun, Apr 22, 2018 at 12:36:36PM +0200, Maxime Villard wrote: Where are they? I haven't been made aware of any issue related to SVS+clang. Yes, I did make you aware that SVS killed VirtualBox. You are being dishonest. You did tell me that SVS didn't work with your VirtualBox. At no point in time did you tell me that it was related to clang or anything close to being a compiler issue, and not an implementation issue. In fact, if you want my point of view, you reported your "problem" in a way that made me just unable to understand what it was about. I had to ask you repeatedly, question after question, what is your virtualbox, what is your cpu, is it hw-assisted, and so on. In PR reports, we ask users to provide a minimal amount of information. If you can't provide a full answer at once, and if I always have to ask one more question all the time, you're just putting all the work on my side, and I'm not going to use my crystal ball to try to guess what your exact configuration or use-case is. Having said that, I did review SVS when you reported your problem, I found and fixed one issue, but it wasn't related to your problem. Maxime
Re: CVS commit: src/sys/arch/amd64/amd64
On Sun, Apr 22, 2018 at 12:36:36PM +0200, Maxime Villard wrote: > Where are they? I haven't been made aware of any issue related to SVS+clang. Yes, I did make you aware that SVS killed VirtualBox. Joerg
Re: CVS commit: src/sys/arch/amd64/amd64
On 22.04.2018 12:36, Maxime Villard wrote: > Le 22/04/2018 à 12:32, Kamil Rytarowski a écrit : >> On 22.04.2018 07:46, Maxime Villard wrote: >>> Le 22/04/2018 à 01:25, Joerg Sonnenberger a écrit : Module Name: src Committed By: joerg Date: Sat Apr 21 23:25:01 UTC 2018 Modified Files: src/sys/arch/amd64/amd64: locore.S Log Message: Do not use movq for loading arbitrary 64bit immediates. The ISA restricts it to 32bit immediates. To generate a diff of this commit: cvs rdiff -u -r1.163 -r1.164 src/sys/arch/amd64/amd64/locore.S Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. >>> >>> Mmh. Is there a compiler where this makes a difference? On NetBSD/GGG it >>> doesn't (because if it did, SVS would never have worked), but I see that >>> on MacOS the instruction indeed makes a difference, the encoding >>> becomes: >>> >>> movq 0x0, %rax >>> >>> Which is obviously not what we expect. >>> >>> Is this the problem you were having a few weeks ago? That is to say, the >>> kernel that was crashing at boot time, did you compile it on another >>> system/compiler that generated a "movq 0x0,%rax"? >>> >>> Anyway your change seems correct. >>> >>> Thanks, >>> Maxime >> >> There are reports that the SVS kernel built by Clang doesn't work. > > Where are they? I haven't been made aware of any issue related to > SVS+clang. > > (By the way, I sent [pullup-8 #786] this morning.) I'm only aware about notification about the problem from users on IRC. signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/sys/arch/amd64/amd64
Le 22/04/2018 à 12:32, Kamil Rytarowski a écrit : On 22.04.2018 07:46, Maxime Villard wrote: Le 22/04/2018 à 01:25, Joerg Sonnenberger a écrit : Module Name:src Committed By:joerg Date:Sat Apr 21 23:25:01 UTC 2018 Modified Files: src/sys/arch/amd64/amd64: locore.S Log Message: Do not use movq for loading arbitrary 64bit immediates. The ISA restricts it to 32bit immediates. To generate a diff of this commit: cvs rdiff -u -r1.163 -r1.164 src/sys/arch/amd64/amd64/locore.S Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Mmh. Is there a compiler where this makes a difference? On NetBSD/GGG it doesn't (because if it did, SVS would never have worked), but I see that on MacOS the instruction indeed makes a difference, the encoding becomes: movq0x0, %rax Which is obviously not what we expect. Is this the problem you were having a few weeks ago? That is to say, the kernel that was crashing at boot time, did you compile it on another system/compiler that generated a "movq 0x0,%rax"? Anyway your change seems correct. Thanks, Maxime There are reports that the SVS kernel built by Clang doesn't work. Where are they? I haven't been made aware of any issue related to SVS+clang. (By the way, I sent [pullup-8 #786] this morning.)
Re: CVS commit: src/sys/arch/amd64/amd64
On 22.04.2018 07:46, Maxime Villard wrote: > Le 22/04/2018 à 01:25, Joerg Sonnenberger a écrit : >> Module Name: src >> Committed By: joerg >> Date: Sat Apr 21 23:25:01 UTC 2018 >> >> Modified Files: >> src/sys/arch/amd64/amd64: locore.S >> >> Log Message: >> Do not use movq for loading arbitrary 64bit immediates. The ISA >> restricts it to 32bit immediates. >> >> >> To generate a diff of this commit: >> cvs rdiff -u -r1.163 -r1.164 src/sys/arch/amd64/amd64/locore.S >> >> Please note that diffs are not public domain; they are subject to the >> copyright notices on the relevant files. > > Mmh. Is there a compiler where this makes a difference? On NetBSD/GGG it > doesn't (because if it did, SVS would never have worked), but I see that > on MacOS the instruction indeed makes a difference, the encoding becomes: > > movq 0x0, %rax > > Which is obviously not what we expect. > > Is this the problem you were having a few weeks ago? That is to say, the > kernel that was crashing at boot time, did you compile it on another > system/compiler that generated a "movq 0x0,%rax"? > > Anyway your change seems correct. > > Thanks, > Maxime There are reports that the SVS kernel built by Clang doesn't work. signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/sys/arch/amd64/amd64
Le 22/04/2018 à 01:25, Joerg Sonnenberger a écrit : Module Name:src Committed By: joerg Date: Sat Apr 21 23:25:01 UTC 2018 Modified Files: src/sys/arch/amd64/amd64: locore.S Log Message: Do not use movq for loading arbitrary 64bit immediates. The ISA restricts it to 32bit immediates. To generate a diff of this commit: cvs rdiff -u -r1.163 -r1.164 src/sys/arch/amd64/amd64/locore.S Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Mmh. Is there a compiler where this makes a difference? On NetBSD/GGG it doesn't (because if it did, SVS would never have worked), but I see that on MacOS the instruction indeed makes a difference, the encoding becomes: movq0x0, %rax Which is obviously not what we expect. Is this the problem you were having a few weeks ago? That is to say, the kernel that was crashing at boot time, did you compile it on another system/compiler that generated a "movq 0x0,%rax"? Anyway your change seems correct. Thanks, Maxime
Re: CVS commit: src/sys/arch/amd64/amd64
Le 24/02/2018 à 17:30, Christos Zoulas a écrit : In article <18bc2a5a-f82d-91ba-5e52-b262c907b...@m00nbsd.net>, Maxime Villard wrote: Le 24/02/2018 à 11:54, Martin Husemann a écrit : On Sat, Feb 24, 2018 at 11:37:11AM +0100, Maxime Villard wrote: If the macro was defined as #if, you would need to do something like: SYSCALL_ENTRY(syscall) #define SYSCALL_ENTRY_SVS SYSCALL_ENTRY(syscall_svs) #undef SYSCALL_ENTRY_SVS Where SYSCALL_ENTRY would contain another macro that depends on whether SYSCALL_ENTRY_SVS is defined. Not sure I follow here. I would do something like: SYSCALL_ENTRY_PLAIN(syscall) SYSCALL_ENTRY_SVS(syscall_svs) and have the SYSCALL_ENTRY_SVS be defined empty ifndef SVS. But then you are duplicating the code that is shared between the two. Yes, I can see why you prefer macros here, but you are also duplicating the stack frame formation code just because in one branch you are using r15 and in the other rax. Why not simplify it? or use a macro for it? Actually I was unhappy about having two different branches too. But thinking about this, now that we have a dynamic detection for SVS, we can use %rax in both branches. I've fixed that in rev1.155, now there is no duplication. Maxime
Re: CVS commit: src/sys/arch/amd64/amd64
In article <18bc2a5a-f82d-91ba-5e52-b262c907b...@m00nbsd.net>, Maxime Villard wrote: >Le 24/02/2018 à 11:54, Martin Husemann a écrit : >> On Sat, Feb 24, 2018 at 11:37:11AM +0100, Maxime Villard wrote: >>> If the macro was defined as #if, you would need to do something like: >>> >>> SYSCALL_ENTRY(syscall) >>> #define SYSCALL_ENTRY_SVS >>> SYSCALL_ENTRY(syscall_svs) >>> #undef SYSCALL_ENTRY_SVS >>> >>> Where SYSCALL_ENTRY would contain another macro that depends on whether >>> SYSCALL_ENTRY_SVS is defined. >> >> Not sure I follow here. >> >> I would do something like: >> >> SYSCALL_ENTRY_PLAIN(syscall) >> SYSCALL_ENTRY_SVS(syscall_svs) >> >> and have the SYSCALL_ENTRY_SVS be defined empty ifndef SVS. > >But then you are duplicating the code that is shared between the two. Yes, I can see why you prefer macros here, but you are also duplicating the stack frame formation code just because in one branch you are using r15 and in the other rax. Why not simplify it? or use a macro for it? christos
Re: CVS commit: src/sys/arch/amd64/amd64
Le 24/02/2018 à 11:54, Martin Husemann a écrit : On Sat, Feb 24, 2018 at 11:37:11AM +0100, Maxime Villard wrote: If the macro was defined as #if, you would need to do something like: SYSCALL_ENTRY(syscall) #define SYSCALL_ENTRY_SVS SYSCALL_ENTRY(syscall_svs) #undef SYSCALL_ENTRY_SVS Where SYSCALL_ENTRY would contain another macro that depends on whether SYSCALL_ENTRY_SVS is defined. Not sure I follow here. I would do something like: SYSCALL_ENTRY_PLAIN(syscall) SYSCALL_ENTRY_SVS(syscall_svs) and have the SYSCALL_ENTRY_SVS be defined empty ifndef SVS. But then you are duplicating the code that is shared between the two.
Re: CVS commit: src/sys/arch/amd64/amd64
On Sat, Feb 24, 2018 at 11:37:11AM +0100, Maxime Villard wrote: > If the macro was defined as #if, you would need to do something like: > > SYSCALL_ENTRY(syscall) > #define SYSCALL_ENTRY_SVS > SYSCALL_ENTRY(syscall_svs) > #undef SYSCALL_ENTRY_SVS > > Where SYSCALL_ENTRY would contain another macro that depends on whether > SYSCALL_ENTRY_SVS is defined. Not sure I follow here. I would do something like: SYSCALL_ENTRY_PLAIN(syscall) SYSCALL_ENTRY_SVS(syscall_svs) and have the SYSCALL_ENTRY_SVS be defined empty ifndef SVS. Martin
Re: CVS commit: src/sys/arch/amd64/amd64
Le 24/02/2018 à 11:14, Martin Husemann a écrit : On Fri, Feb 23, 2018 at 08:09:09AM +0100, Maxime Villard wrote: ... And? There is only one place where we use .if instead of #if, because there is a good reason for doing so. Which reason is that? Well, look at the code. We want to control what gets compiled in the macro with an argument. SYSCALL_ENTRY syscall,is_svs=0 SYSCALL_ENTRY syscall_svs,is_svs=1 If the macro was defined as #if, you would need to do something like: SYSCALL_ENTRY(syscall) #define SYSCALL_ENTRY_SVS SYSCALL_ENTRY(syscall_svs) #undef SYSCALL_ENTRY_SVS Where SYSCALL_ENTRY would contain another macro that depends on whether SYSCALL_ENTRY_SVS is defined. The second approach is the one that complexifies the code. Maxime
Re: CVS commit: src/sys/arch/amd64/amd64
On Fri, Feb 23, 2018 at 08:09:09AM +0100, Maxime Villard wrote: > ... And? There is only one place where we use .if instead of #if, because > there > is a good reason for doing so. Which reason is that? Martin
Re: CVS commit: src/sys/arch/amd64/amd64
Le 22/02/2018 à 17:31, Christos Zoulas a écrit : In article <7f4de63c-e782-14e6-5554-9b9d23471...@m00nbsd.net>, Maxime Villard wrote: Le 22/02/2018 à 15:54, Christos Zoulas a écrit : In article <20180222140848.70e95f...@cvs.netbsd.org>, Martin Husemann wrote: -=-=-=-=-=- Module Name:src Committed By: martin Date: Thu Feb 22 14:08:48 UTC 2018 Modified Files: src/sys/arch/amd64/amd64: locore.S Log Message: Protect the SVS part of SYSCALL_ENTRY by #ifdef SVS to make non-SVS kernels compile again. The combination of "#ifdef" and ".if" makes the code more horrific. Can we use one and not the other? Preferrably "#ifdef" since we already use it extensively? In this case the ifdef just had to be put around the declaration. You can't replace .if by #ifdef, there are two SYSCALL_ENTRY declarations, and we give a different argument depending on whether we want the SVS code to be in the macro or not. The question is do we want to keep using both cpp and assembly macros. Why wouldn't we? I don't see the problem. The use of assembly macros is recent, the cpp one has always been there. I.e. until recently we were not using .macro or .if, now we are. ... And? There is only one place where we use .if instead of #if, because there is a good reason for doing so. It doesn't occur to me we need to replace all the other #ifs by .ifs as a result. Maxime
Re: CVS commit: src/sys/arch/amd64/amd64
On Feb 23, 8:09am, m...@m00nbsd.net (Maxime Villard) wrote: -- Subject: Re: CVS commit: src/sys/arch/amd64/amd64 | > The question is do we want to keep using both cpp and assembly macros. | | Why wouldn't we? I don't see the problem. Because it adds complexity. | ... And? There is only one place where we use .if instead of #if, because | there is a good reason for doing so. It doesn't occur to me we need to | replace all the other #ifs by .ifs as a result. Requiring macro support ties us more tightly to binutils and gas, since the syntax and implementation is typically assembler specific. For example does it work with the llvm assembler? The bottom line is I would not use it unless it simplified the code a lot and made it more readable (and easier to debug). christos
Re: CVS commit: src/sys/arch/amd64/amd64
In article <7f4de63c-e782-14e6-5554-9b9d23471...@m00nbsd.net>, Maxime Villard wrote: >Le 22/02/2018 à 15:54, Christos Zoulas a écrit : >> In article <20180222140848.70e95f...@cvs.netbsd.org>, >> Martin Husemann wrote: >>> -=-=-=-=-=- >>> >>> Module Name:src >>> Committed By: martin >>> Date: Thu Feb 22 14:08:48 UTC 2018 >>> >>> Modified Files: >>> src/sys/arch/amd64/amd64: locore.S >>> >>> Log Message: >>> Protect the SVS part of SYSCALL_ENTRY by #ifdef SVS to make non-SVS >>> kernels compile again. >> >> The combination of "#ifdef" and ".if" makes the code more horrific. >> Can we use one and not the other? Preferrably "#ifdef" since we already >> use it extensively? > >In this case the ifdef just had to be put around the declaration. > >You can't replace .if by #ifdef, there are two SYSCALL_ENTRY declarations, >and we give a different argument depending on whether we want the SVS code >to be in the macro or not. The question is do we want to keep using both cpp and assembly macros. The use of assembly macros is recent, the cpp one has always been there. I.e. until recently we were not using .macro or .if, now we are. christos
Re: CVS commit: src/sys/arch/amd64/amd64
Le 22/02/2018 à 15:54, Christos Zoulas a écrit : In article <20180222140848.70e95f...@cvs.netbsd.org>, Martin Husemann wrote: -=-=-=-=-=- Module Name:src Committed By: martin Date: Thu Feb 22 14:08:48 UTC 2018 Modified Files: src/sys/arch/amd64/amd64: locore.S Log Message: Protect the SVS part of SYSCALL_ENTRY by #ifdef SVS to make non-SVS kernels compile again. The combination of "#ifdef" and ".if" makes the code more horrific. Can we use one and not the other? Preferrably "#ifdef" since we already use it extensively? In this case the ifdef just had to be put around the declaration. You can't replace .if by #ifdef, there are two SYSCALL_ENTRY declarations, and we give a different argument depending on whether we want the SVS code to be in the macro or not. Maxime
Re: CVS commit: src/sys/arch/amd64/amd64
In article <20180222140848.70e95f...@cvs.netbsd.org>, Martin Husemann wrote: >-=-=-=-=-=- > >Module Name: src >Committed By: martin >Date: Thu Feb 22 14:08:48 UTC 2018 > >Modified Files: > src/sys/arch/amd64/amd64: locore.S > >Log Message: >Protect the SVS part of SYSCALL_ENTRY by #ifdef SVS to make non-SVS >kernels compile again. The combination of "#ifdef" and ".if" makes the code more horrific. Can we use one and not the other? Preferrably "#ifdef" since we already use it extensively? christos
Re: CVS commit: src/sys/arch/amd64/amd64
e 24/03/2017 à 21:32, co...@sdf.org a écrit : cool! I see in arch/i386/i386/locore.S that there is another call gate and there's: 1246 IDTVEC(osyscall) 1247 #ifndef XEN 1248 /* XXX we are in trouble! interrupts be off here. */ 1249 cli /* must be first instruction */ 1250 #endif 1251 pushfl /* set eflags in trap frame */ Is 'cli' as first instruction what should've been done here, if it wasn't been otherwise useless? can xen not do it? Yes, I saw that too. In fact, I didn't understand how putting 'cli' fixed the issue, since an interrupt can still happen before this instruction. Given that it was committed by ad@, he probably must have thought about this too; so it perhaps means that call gates on i386 disable interrupt for the first instruction or something like that, but I was unable to find any reference to this in the SDMs. For Xen, there is no documentation, so if you want to find out what happens you need to dig into the Xen source code. As far as I can test, it seems that Xen disables interrupts on call gates. There is still at least one bug here: now that pushfl is the second instruction, the first two single-steps should be ignored, and this [1] branch should be 'osyscall + 2', otherwise we may unintentionnally disable single-stepping when returing to userland. [1] https://nxr.netbsd.org/xref/src/sys/arch/i386/i386/trap.c#716
Re: CVS commit: src/sys/arch/amd64/amd64
On Thu, Mar 23, 2017 at 05:25:51PM +, Maxime Villard wrote: > Module Name: src > Committed By: maxv > Date: Thu Mar 23 17:25:51 UTC 2017 > > Modified Files: > src/sys/arch/amd64/amd64: locore.S machdep.c trap.c > > Log Message: > Remove this call gate on amd64, it is useless and vulnerable. > > Call gates do not modify %rflags, so interrupts are not disabled when > entering the gate. There is a small window where we are in kernel mode and > with a userland %gs, and if an interrupt happens here we will rejump into > the kernel but not switch to the kernel TLS. > > Userland can simply perform a gate call in a loop, and hope that at some > point an interrupt will be received in this window - which necessarily will > be the case. With a specially-crafted %gs it is certainly enough to > escalate privileges. > > > To generate a diff of this commit: > cvs rdiff -u -r1.121 -r1.122 src/sys/arch/amd64/amd64/locore.S > cvs rdiff -u -r1.253 -r1.254 src/sys/arch/amd64/amd64/machdep.c > cvs rdiff -u -r1.94 -r1.95 src/sys/arch/amd64/amd64/trap.c > cool! I see in arch/i386/i386/locore.S that there is another call gate and there's: 1246 IDTVEC(osyscall) 1247 #ifndef XEN 1248 /* XXX we are in trouble! interrupts be off here. */ 1249 cli /* must be first instruction */ 1250 #endif 1251 pushfl /* set eflags in trap frame */ Is 'cli' as first instruction what should've been done here, if it wasn't been otherwise useless? can xen not do it? thanks.
Re: CVS commit: src/sys/arch/amd64/amd64
Le 29/05/2016 à 11:04, Maxime Villard a écrit : Module Name:src Committed By: maxv Date: Sun May 29 09:04:20 UTC 2016 Modified Files: src/sys/arch/amd64/amd64: locore.S Log Message: Revert rev1.94. It apparently raises a page fault from SMEP. I need to investigate the whole kernel mappings anyway, so I'll recommit this patch later. I obviously meant rev1.95 To generate a diff of this commit: cvs rdiff -u -r1.96 -r1.97 src/sys/arch/amd64/amd64/locore.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/amd64/amd64
Le 07/05/2016 23:13, matthew green a écrit : Joerg Sonnenberger writes: On Sat, May 07, 2016 at 11:49:21AM +, Maxime Villard wrote: Module Name:src Committed By: maxv Date: Sat May 7 11:49:21 UTC 2016 Modified Files: src/sys/arch/amd64/amd64: locore.S Log Message: clarify WTH. Can you please not mix arbitrary stylistic changes with refactoring and whatever else you have hidden in this?! I don't like the "arbitrary". I wrote this months ago, and the patch I have for this file entails many more actual functional changes. I just committed the stylistic and idiotic parts yesterday, because I was busy doing something else. The other real changes will come separately soon. agreed. there is at least one functional change here: PROC0_STK_OFF has changed definition. could you please explain this part? It's rather simple: -#define PROC0_STK_OFF (PROC0_PML4_OFF + PAGE_SIZE) +#define PROC0_STK_OFF (PROC0_PML4_OFF + 1 * PAGE_SIZE) #define PROC0_PTP3_OFF(PROC0_STK_OFF + UPAGES * PAGE_SIZE) #define PROC0_PTP2_OFF(PROC0_PTP3_OFF + NKL4_KIMG_ENTRIES * PAGE_SIZE) #define PROC0_PTP1_OFF(PROC0_PTP2_OFF + TABLE_L3_ENTRIES * PAGE_SIZE) All the macros are in the format NUMBER_OF_PAGES * PAGE_SIZE, so I put 1* to make clear we are allocating one page. additionally, please revert killkpt macro -- it makes it harder to understand the assembly as it moves the 1: target into a macro so that people will mis-reaad branch/jumps thinking they'll go to the following 1:. No. You can see above that there is the fillkpt macro, that is in charge of setting up a set of pages. We now have a pair fillkpt/killkpt, which is way clearer than hard-coded loops. fillkpt too uses 1: loop 1b as well, and there is no problem with it. I see by the way that I could have used killkpt for the PML4 entries as well; I'll commit that right now. thanks. .mrg.
re: CVS commit: src/sys/arch/amd64/amd64
Joerg Sonnenberger writes: > On Sat, May 07, 2016 at 11:49:21AM +, Maxime Villard wrote: > > Module Name:src > > Committed By: maxv > > Date: Sat May 7 11:49:21 UTC 2016 > > > > Modified Files: > > src/sys/arch/amd64/amd64: locore.S > > > > Log Message: > > clarify > > WTH. Can you please not mix arbitrary stylistic changes with refactoring > and whatever else you have hidden in this?! agreed. there is at least one functional change here: PROC0_STK_OFF has changed definition. could you please explain this part? additionally, please revert killkpt macro -- it makes it harder to understand the assembly as it moves the 1: target into a macro so that people will mis-reaad branch/jumps thinking they'll go to the following 1:. thanks. .mrg.
Re: CVS commit: src/sys/arch/amd64/amd64
On Sat, May 07, 2016 at 11:49:21AM +, Maxime Villard wrote: > Module Name: src > Committed By: maxv > Date: Sat May 7 11:49:21 UTC 2016 > > Modified Files: > src/sys/arch/amd64/amd64: locore.S > > Log Message: > clarify WTH. Can you please not mix arbitrary stylistic changes with refactoring and whatever else you have hidden in this?! Joerg
Re: CVS commit: src/sys/arch/amd64/amd64
On Wed, Jul 01, 2015 at 02:04:43AM +, Christos Zoulas wrote: > In article <20150630233112.ga8...@britannica.bec.de>, > Joerg Sonnenberger wrote: > >On Tue, Jun 30, 2015 at 05:08:24PM -0400, Christos Zoulas wrote: > >> Module Name: src > >> Committed By: christos > >> Date: Tue Jun 30 21:08:24 UTC 2015 > >> > >> Modified Files: > >>src/sys/arch/amd64/amd64: cpu_in_cksum.S > >> > >> Log Message: > >> handle PIC compilation (if we are building a PIE system; this is used > >by tests) > > > >Isn't the leaq generally preferable as smaller? > > I believe leaq is 7 bytes, and movq is 5. But I am not sure which takes > more cycles. 'leaq' with %rip will have to be a long encoding since rip relative isn't a 386 addressing mode. My guess is that both take the same number of cycles on current cpus. Some old brain cells recall 'lea' using different hardware from the ALU (for adds) so happening at a different stage in the pipeline and having different result delay and/or concurrency rules - but I can't remember which particular cpu that applied to. David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/sys/arch/amd64/amd64
In article <20150630233112.ga8...@britannica.bec.de>, Joerg Sonnenberger wrote: >On Tue, Jun 30, 2015 at 05:08:24PM -0400, Christos Zoulas wrote: >> Module Name: src >> Committed By:christos >> Date:Tue Jun 30 21:08:24 UTC 2015 >> >> Modified Files: >> src/sys/arch/amd64/amd64: cpu_in_cksum.S >> >> Log Message: >> handle PIC compilation (if we are building a PIE system; this is used >by tests) > >Isn't the leaq generally preferable as smaller? I believe leaq is 7 bytes, and movq is 5. But I am not sure which takes more cycles. christos
Re: CVS commit: src/sys/arch/amd64/amd64
On Tue, Jun 30, 2015 at 05:08:24PM -0400, Christos Zoulas wrote: > Module Name: src > Committed By: christos > Date: Tue Jun 30 21:08:24 UTC 2015 > > Modified Files: > src/sys/arch/amd64/amd64: cpu_in_cksum.S > > Log Message: > handle PIC compilation (if we are building a PIE system; this is used by > tests) Isn't the leaq generally preferable as smaller? Joerg
Re: CVS commit: src/sys/arch/amd64/amd64
On Tue, May 13, 2014 at 2:28 AM, Jonathan A. Kollasch wrote: > On Mon, May 12, 2014 at 07:05:29PM +0200, Joerg Sonnenberger wrote: >> On Mon, May 12, 2014 at 01:49:24PM +, Masao Uebayashi wrote: >> > Module Name:src >> > Committed By: uebayasi >> > Date: Mon May 12 13:49:24 UTC 2014 >> > >> > Modified Files: >> > src/sys/arch/amd64/amd64: machdep.c >> > >> > Log Message: >> > Don't reserve space (128) on signal stack for unknown reasons; the actual >> > space for struct sigframe_siginfo (+ alignment) is allocated just below. >> >> AMD64 uses a redzone, so the compiler can put up to 128 Bytes on the >> stack without having to adjust RSP. Please revert immediately. > > Done. Thanks. I left a comment there.
Re: CVS commit: src/sys/arch/amd64/amd64
On Mon, May 12, 2014 at 07:05:29PM +0200, Joerg Sonnenberger wrote: > On Mon, May 12, 2014 at 01:49:24PM +, Masao Uebayashi wrote: > > Module Name:src > > Committed By: uebayasi > > Date: Mon May 12 13:49:24 UTC 2014 > > > > Modified Files: > > src/sys/arch/amd64/amd64: machdep.c > > > > Log Message: > > Don't reserve space (128) on signal stack for unknown reasons; the actual > > space for struct sigframe_siginfo (+ alignment) is allocated just below. > > AMD64 uses a redzone, so the compiler can put up to 128 Bytes on the > stack without having to adjust RSP. Please revert immediately. Done.
Re: CVS commit: src/sys/arch/amd64/amd64
On Mon, May 12, 2014 at 01:49:24PM +, Masao Uebayashi wrote: > Module Name: src > Committed By: uebayasi > Date: Mon May 12 13:49:24 UTC 2014 > > Modified Files: > src/sys/arch/amd64/amd64: machdep.c > > Log Message: > Don't reserve space (128) on signal stack for unknown reasons; the actual > space for struct sigframe_siginfo (+ alignment) is allocated just below. AMD64 uses a redzone, so the compiler can put up to 128 Bytes on the stack without having to adjust RSP. Please revert immediately. Joerg
Re: CVS commit: src/sys/arch/amd64/amd64
On Sat, Jun 16, 2012 at 08:31:42PM +0100, David Laight wrote: > On Sat, Jun 16, 2012 at 04:42:27PM +, Joerg Sonnenberger wrote: > > Module Name:src > > Committed By: joerg > > Date: Sat Jun 16 16:42:27 UTC 2012 > > > > Modified Files: > > src/sys/arch/amd64/amd64: machdep.c > > > > Log Message: > > Annotate tautological if, so that clang doesn't warn about the dt usage > > later on. > > There is something horribly wrong here! > if (seg != GUDATA_SEL || seg != GUDATA32_SEL) > return EINVAL; > I suspect that should be && not || > As it is, the function will never accept a non-zero segment apart > from ones in the LDT. > > Hmmm... those functions need making static and collapsing down to > remove the never-set parameters. I fully agree that there is something wrong going on here. Changing the || to && would make it reference an unset variable. This is more a bandaid to give someone a clue that there really is something fishy here... Joerg
Re: CVS commit: src/sys/arch/amd64/amd64
On Sat, Jun 16, 2012 at 04:42:27PM +, Joerg Sonnenberger wrote: > Module Name: src > Committed By: joerg > Date: Sat Jun 16 16:42:27 UTC 2012 > > Modified Files: > src/sys/arch/amd64/amd64: machdep.c > > Log Message: > Annotate tautological if, so that clang doesn't warn about the dt usage > later on. There is something horribly wrong here! if (seg != GUDATA_SEL || seg != GUDATA32_SEL) return EINVAL; I suspect that should be && not || As it is, the function will never accept a non-zero segment apart from ones in the LDT. Hmmm... those functions need making static and collapsing down to remove the never-set parameters. David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/sys/arch/amd64/amd64
On 21 April 2012 13:52, Christos Zoulas wrote: > Module Name: src > Committed By: christos > Date: Sat Apr 21 18:52:37 UTC 2012 > > Modified Files: > src/sys/arch/amd64/amd64: vector.S > > Log Message: > Alignment fault traps push the error code automatically, so don't use ZTRAP! > Have we vetted all exceptions ? The list is pretty standard. -- ~Cherry
Re: CVS commit: src/sys/arch/amd64/amd64
On Apr 22, 12:00am, jeanyves.mig...@free.fr (Jean-Yves Migeon) wrote: -- Subject: Re: CVS commit: src/sys/arch/amd64/amd64 | It's the other way around; the bug was rather harmless in VMs (kills the | process with a SIGILL), while it force-reboot the host on a native platform. I had the real host so I was experiencing the crash, so I wanted to fix it quickly. | I could not know that the fix works on real hardware, that's why I was | waiting for Paul's response. Ok. christos
Re: CVS commit: src/sys/arch/amd64/amd64
Le 21/04/12 23:25, Christos Zoulas a écrit : In article<4f930a8c.6040...@free.fr>, Jean-Yves Migeon wrote: Le 21/04/12 20:52, Christos Zoulas a écrit : Module Name:src Committed By: christos Date: Sat Apr 21 18:52:37 UTC 2012 Modified Files: src/sys/arch/amd64/amd64: vector.S Log Message: Alignment fault traps push the error code automatically, so don't use ZTRAP! Meh, the fix was awaiting Paul testing... Alright, so I guess this one is right. Even if Paul's testing discovered that the fix did not work for the emulator, wouldn't you commit it so that at least things work on real hardware? It's the other way around; the bug was rather harmless in VMs (kills the process with a SIGILL), while it force-reboot the host on a native platform. I could not know that the fix works on real hardware, that's why I was waiting for Paul's response. Do you want me to ask for a pull-up? Sure, thanks. Will do. -- jym@
Re: CVS commit: src/sys/arch/amd64/amd64
In article <4f930a8c.6040...@free.fr>, Jean-Yves Migeon wrote: >Le 21/04/12 20:52, Christos Zoulas a écrit : > > Module Name:src > > Committed By: christos > > Date: Sat Apr 21 18:52:37 UTC 2012 > > > > Modified Files: > > src/sys/arch/amd64/amd64: vector.S > > > > Log Message: > > Alignment fault traps push the error code automatically, so don't use >ZTRAP! > >Meh, the fix was awaiting Paul testing... Alright, so I guess this one >is right. Even if Paul's testing discovered that the fix did not work for the emulator, wouldn't you commit it so that at least things work on real hardware? >Do you want me to ask for a pull-up? Sure, thanks. christos
Re: CVS commit: src/sys/arch/amd64/amd64
I just tested with a new updated kernel. It no longer crashes. Instead, it reports an expected failure: x86 architecture does not correctly report the address where the unaligned access occurred: /build/netbsd-local/src/tests/lib/libc/gen/t_siginfo.c:427: info->si_addr != (void *)addr Much better! On Sat, 21 Apr 2012, Jean-Yves Migeon wrote: Le 21/04/12 20:52, Christos Zoulas a écrit : Module Name:src Committed By: christos Date: Sat Apr 21 18:52:37 UTC 2012 Modified Files: src/sys/arch/amd64/amd64: vector.S Log Message: Alignment fault traps push the error code automatically, so don't use ZTRAP! Meh, the fix was awaiting Paul testing... Alright, so I guess this one is right. Do you want me to ask for a pull-up? -- jym@ !DSPAM:4f930ab01981554950846! - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
Re: CVS commit: src/sys/arch/amd64/amd64
Le 21/04/12 20:52, Christos Zoulas a écrit : > Module Name: src > Committed By: christos > Date: Sat Apr 21 18:52:37 UTC 2012 > > Modified Files: >src/sys/arch/amd64/amd64: vector.S > > Log Message: > Alignment fault traps push the error code automatically, so don't use ZTRAP! Meh, the fix was awaiting Paul testing... Alright, so I guess this one is right. Do you want me to ask for a pull-up? -- jym@
Re: CVS commit: src/sys/arch/amd64/amd64
On Fri, Jul 17, 2009 at 11:35:31AM +, Christos Zoulas wrote: > >Shouldn't the non-DIAGNOSTIC default case set ksi_signo to something > >other than 0? I'm not entirely sure what trapsignal will do with > >signal 0, but I doubt it's the right thing. > > Well, if you fall in the default case in both switch statements it > is clearly a bug (because you added another case in the outside > switch and you forgot to add it in the inside). I could remove the > DIAGNOSTIC check, but that leads to bloat. True. N/m. (Maybe the cases should be sorted in the same order though.) -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/arch/amd64/amd64
In article <20090717053517.ga10...@netbsd.org>, David Holland wrote: >On Thu, Jul 16, 2009 at 10:18:09AM -0400, Christos Zoulas wrote: > > Modified Files: > > src/sys/arch/amd64/amd64: trap.c > > > > Log Message: > > handle protection fault properly. > >Shouldn't the non-DIAGNOSTIC default case set ksi_signo to something >other than 0? I'm not entirely sure what trapsignal will do with >signal 0, but I doubt it's the right thing. Well, if you fall in the default case in both switch statements it is clearly a bug (because you added another case in the outside switch and you forgot to add it in the inside). I could remove the DIAGNOSTIC check, but that leads to bloat. christos
Re: CVS commit: src/sys/arch/amd64/amd64
On Thu, Jul 16, 2009 at 10:18:09AM -0400, Christos Zoulas wrote: > Modified Files: > src/sys/arch/amd64/amd64: trap.c > > Log Message: > handle protection fault properly. Shouldn't the non-DIAGNOSTIC default case set ksi_signo to something other than 0? I'm not entirely sure what trapsignal will do with signal 0, but I doubt it's the right thing. -- David A. Holland dholl...@netbsd.org
