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

[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

[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: [stos, again] Re: CVS commit: src/sys/arch/amd64

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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: CVS commit: src/sys/arch/amd64

[Emmanuel Dreyfus]

Ryo ONODERA  wrote:

> However I need multiboot support for amd64.
> I am waiting well-tested implementation.

At this point the problems are more about code style and cleaning, as we
have a fix for the boot bugs that has been reported. 

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
m...@netbsd.org

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

[Ryo ONODERA]

Hi,

However I need multiboot support for amd64.
I am waiting well-tested implementation.

"Emmanuel Dreyfus"  writes:

> Module Name:  src
> Committed By: manu
> Date: Thu Jan  9 00:42:24 UTC 2020
>
> Modified Files:
>   src/sys/arch/amd64/amd64: locore.S machdep.c
>   src/sys/arch/amd64/conf: GENERIC files.amd64 kern.ldscript
>
> Log Message:
> Rollback multiboot2 for amd64, as requested by core
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.197 -r1.198 src/sys/arch/amd64/amd64/locore.S
> cvs rdiff -u -r1.344 -r1.345 src/sys/arch/amd64/amd64/machdep.c
> cvs rdiff -u -r1.553 -r1.554 src/sys/arch/amd64/conf/GENERIC
> cvs rdiff -u -r1.114 -r1.115 src/sys/arch/amd64/conf/files.amd64
> cvs rdiff -u -r1.30 -r1.31 src/sys/arch/amd64/conf/kern.ldscript
>
> 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/locore.S
> diff -u src/sys/arch/amd64/amd64/locore.S:1.197 
> src/sys/arch/amd64/amd64/locore.S:1.198
> --- src/sys/arch/amd64/amd64/locore.S:1.197   Wed Jan  8 20:59:18 2020
> +++ src/sys/arch/amd64/amd64/locore.S Thu Jan  9 00:42:24 2020
> @@ -1,4 +1,4 @@
> -/*   $NetBSD: locore.S,v 1.197 2020/01/08 20:59:18 skrll Exp $   */
> +/*   $NetBSD: locore.S,v 1.198 2020/01/09 00:42:24 manu Exp $*/
>  
>  /*
>   * Copyright-o-rama!
> @@ -158,7 +158,6 @@
>  
>  #include "opt_compat_netbsd.h"
>  #include "opt_compat_netbsd32.h"
> -#include "opt_multiboot.h"
>  #include "opt_xen.h"
>  #include "opt_svs.h"
>  
> @@ -178,13 +177,6 @@
>  #include 
>  #include 
>  
> -#ifndef XENPV
> -#include 
> -#endif 
> -
> -#define CODE_SEGMENT 0x08
> -#define DATA_SEGMENT 0x10
> -
>  #if NLAPIC > 0
>  #include 
>  #endif
> @@ -432,50 +424,6 @@ END(farjmp64)
>   .space  512
>  tmpstk:
>  
> -.section multiboot,"a"
> -#if defined(MULTIBOOT)
> - .align  8
> - .globl  Multiboot2_Header
> -_C_LABEL(Multiboot2_Header):
> - .intMULTIBOOT2_HEADER_MAGIC
> - .intMULTIBOOT2_ARCHITECTURE_I386
> - .intMultiboot2_Header_end - Multiboot2_Header
> - .int-(MULTIBOOT2_HEADER_MAGIC + MULTIBOOT2_ARCHITECTURE_I386 \
> - + (Multiboot2_Header_end - Multiboot2_Header))
> -
> - .int1   /* MULTIBOOT_HEADER_TAG_INFORMATION_REQUEST */
> - .int12  /* sizeof(multiboot_header_tag_information_request) */
> - /* + sizeof(uint32_t) * requests */
> - .int4   /* MULTIBOOT_TAG_TYPE_BASIC_MEMINFO */
> - .align  8
> -
> - .int3   /* MULTIBOOT_HEADER_TAG_ENTRY_ADDRESS */
> - .int16  /* sizeof(struct multiboot_tag_efi64) */
> - .quad   (multiboot2_entry - KERNBASE)
> - .align  8
> -
> - .int9   /* MULTIBOOT_HEADER_TAG_ENTRY_ADDRESS_EFI64 */
> - .int16  /* sizeof(struct multiboot_tag_efi64) */
> - .quad   (multiboot2_entry - KERNBASE)
> - .align  8
> -
> -#if notyet
> - /*
> -  * Could be used to get an early console for debug,
> -  * but this is broken.
> -  */
> - .int7   /* MULTIBOOT_HEADER_TAG_EFI_BS */
> - .int8   /* sizeof(struct multiboot_tag) */
> - .align  8
> -#endif
> -
> - .int0   /* MULTIBOOT_HEADER_TAG_END */
> - .int8   /* sizeof(struct multiboot_tag) */
> - .align  8
> - .globl  Multiboot2_Header_end
> -_C_LABEL(Multiboot2_Header_end):
> -#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.
> @@ -492,700 +440,6 @@ ENTRY(start)
>   /* Warm boot */
>   movw$0x1234,0x472
>  
> -#if defined(MULTIBOOT)
> - jmp .Lnative_loader
> -
> -
> -multiboot2_entry:
> - .code64
> - /*
> -  * multiboot2 entry point. We are left here without
> -  * stack and with no idea of where we were loaded in memory.
> -  * The only inputs are
> -  * %eax MULTIBOOT2_BOOTLOADER_MAGIC
> -  * %ebx pointer to multiboot_info
> -  *
> -  * Here we will:
> -  * - copy the kernel to 0x20 (KERNTEXTOFF - KERNBASE)
> -  *  as almost all the code in locore.S assume it is there. 
> -  *  This is derived from 
> -  *  src/sys/arch/i386/stand/efiboot/bootx64/startprog64.S
> -  * - copy multiboot_info, as done in multiboot_pre_reloc() from
> -  *  src/sys/arch/x86/x86/multiboot2.c
> -  *  Unfortunately we cannot call that function as there is 
> -  *  no simple way to build it as 32 bit code in a 64 bit kernel.
> -  * - Copy ELF symbols, also as in multiboot_pre_reloc()
> -  */
> -
> - cli
> -
> - /*
> -  * Discover our load address and use it to get start address
> -  */
> - mov $_RELOC(tmpstk),%rsp
> - callnext
> -next:pop %r8
> - sub $(next - start), %r8
> -
> - /*
> -  * Save multiboot_info for later. 

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

[Emmanuel Dreyfus]

On Sun, Jan 05, 2020 at 02:43:43PM +0100, Maxime Villard wrote:
> I have now requested to core@ that multiboot in amd64 be reverted entirely.

So far I privilegied working on a fix to the boot problem that was
reported, rather than spending time on a revert. This was not a futile
effort, since at this point we have a fix that we agree on. My idea was
to commit it and then to address the other points you raised, but if
you cannot stand that, feel free to do the revert on your own.

There is also this problem that seems worth spending time to me:
http://mail-index.netbsd.org/tech-kern/2020/01/02/msg025911.html

I hit this bug 100% reliabily with qemu/EFI, even with the multiboot
stuff removed, and it seems to match what Masanobu Saitoh reported. 
I suspect the multiboot changes just undercovered it.

-- 
Emmanuel Dreyfus
m...@netbsd.org

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

[Maxime Villard]

Le 05/01/2020 à 13:56, Maxime Villard a écrit :
> Le 05/01/2020 à 02:03, Emmanuel Dreyfus a écrit :
>> 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.
> 
> I think .text.multiboot is fine, and @note should be dropped
> 
>> Attached is my latest change set, including the locore cleanup you asked
>> for.
> 
> Notice how, after cleanup, the big copy crap comes down to literally just
> two instructions. Unfortunately that's not exactly it:
> 
>  - As I said more than three weeks ago [1], I think it's the whole
>MULTIBOOT block that should be moved in a separate file, not just the
>32bit copy function. Only the '.Lbegin' label (to be renamed) needs to
>be in locore.S, the rest can (and should) be outside.
>  - multiboot2_pre_reloc_would_be_built_as_ia32 should be removed.
>  - The code is still not entirely KNF, search for "\t\n".
>  - Local labels should begin with ".L".
>  - Now I'm wondering why KEEP() in the ldscript? Why doesn't
>"*(.text.multiboot)" suffice?
> 
> And also... Recovering from the heart attack I got after looking at
> multiboot2_copy_syms32, I'm a bit confused; did you just objdump the
> function and copy-paste it in the kernel? How did you obtain this
> code? [Is it normal that I am already worried about your next answer?]
> 
> Overall, I'm irritated, yes, because instead of reverting the change and
> taking just one peaceful hour to fix things correctly, you have decided to
> waste everybody's time with the breakage and absurd patch-work. I find
> myself having to _insist_ for you to clean up the junk, and now I'm even
> quoting emails from one month ago.
> 
> Maxime
> 
> [1] https://mail-index.netbsd.org/source-changes-d/2019/12/12/msg011882.html

Sorry, but not gonna waste more of my time.

I have now requested to core@ that multiboot in amd64 be reverted entirely.

The correct way to add multiboot can be discussed afterwards, when the patch
will have been shared and will have been agreed upon beforehand, in a way
that it at least isn't total junk and doesn't break other things.

Maxime

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

[Maxime Villard]

Le 05/01/2020 à 02:03, Emmanuel Dreyfus a écrit :
> 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.

I think .text.multiboot is fine, and @note should be dropped

> Attached is my latest change set, including the locore cleanup you asked
> for.

Notice how, after cleanup, the big copy crap comes down to literally just
two instructions. Unfortunately that's not exactly it:

 - As I said more than three weeks ago [1], I think it's the whole
   MULTIBOOT block that should be moved in a separate file, not just the
   32bit copy function. Only the '.Lbegin' label (to be renamed) needs to
   be in locore.S, the rest can (and should) be outside.
 - multiboot2_pre_reloc_would_be_built_as_ia32 should be removed.
 - The code is still not entirely KNF, search for "\t\n".
 - Local labels should begin with ".L".
 - Now I'm wondering why KEEP() in the ldscript? Why doesn't
   "*(.text.multiboot)" suffice?

And also... Recovering from the heart attack I got after looking at
multiboot2_copy_syms32, I'm a bit confused; did you just objdump the
function and copy-paste it in the kernel? How did you obtain this
code? [Is it normal that I am already worried about your next answer?]

Overall, I'm irritated, yes, because instead of reverting the change and
taking just one peaceful hour to fix things correctly, you have decided to
waste everybody's time with the breakage and absurd patch-work. I find
myself having to _insist_ for you to clean up the junk, and now I'm even
quoting emails from one month ago.

Maxime

[1] https://mail-index.netbsd.org/source-changes-d/2019/12/12/msg011882.html

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

[Emmanuel Dreyfus]

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
-   

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

[Martin Husemann]

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

[Maxime Villard]

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: CVS commit: src/sys/arch/amd64

[Emmanuel Dreyfus]

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.

I am especially interested by feedback from msaitoh@ who reported the
crash at cpu attacch that I suspect to be the (probably unrelated) 
problem describred here:
http://mail-index.netbsd.org/tech-kern/2020/01/02/msg025911.html

-- 
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   4 Jan 2020 01:48:11 -
@@ -431,10 +431,10 @@
.size   tmpstk, tmpstk - .
.space  512
 tmpstk:
 
-.section multiboot,"a"
 #if defined(MULTIBOOT)
+.section multiboot,"",@note
.align  8
.globl  Multiboot2_Header
 _C_LABEL(Multiboot2_Header):
.intMULTIBOOT2_HEADER_MAGIC
Index: sys/arch/amd64/conf/GENERIC
===
RCS file: /cvsroot/src/sys/arch/amd64/conf/GENERIC,v
retrieving revision 1.551
diff -U4 -r1.551 GENERIC
--- sys/arch/amd64/conf/GENERIC 14 Dec 2019 07:45:20 -  1.551
+++ sys/arch/amd64/conf/GENERIC 4 Jan 2020 01:48:11 -
@@ -25,9 +25,9 @@
 #ident "GENERIC-$Revision: 1.551 $"
 
 maxusers   64  # estimated number of users
 
-#options   MULTIBOOT   # Multiboot support (see multiboot(8)) 
+optionsMULTIBOOT   # Multiboot support (see multiboot(8)) 
 
 # delay between "rebooting ..." message and hardware reset, in milliseconds
 #options   CPURESET_DELAY=2000
 
Index: sys/arch/amd64/conf/Makefile.amd64
===
RCS file: /cvsroot/src/sys/arch/amd64/conf/Makefile.amd64,v
retrieving revision 1.80
diff -U4 -r1.80 Makefile.amd64
--- sys/arch/amd64/conf/Makefile.amd64  14 Nov 2019 16:23:52 -  1.80
+++ sys/arch/amd64/conf/Makefile.amd64  4 Jan 2020 01:48:11 -
@@ -90,12 +90,12 @@
 ## (5) link settings
 ##
 TEXTADDR?= 0x8020
 .if defined(KASLR)
-EXTRA_LINKFLAGS=   --split-by-file=0x10 -r -d
+EXTRA_LINKFLAGS=   --split-by-file=0x10 -z max-page-size=0x1000 -r -d
 KERNLDSCRIPT?= ${AMD64}/conf/kern.ldscript.kaslr
 .else
-EXTRA_LINKFLAGS=   -z max-page-size=0x20
+EXTRA_LINKFLAGS=   -z max-page-size=0x1000
 KERNLDSCRIPT?= ${AMD64}/conf/kern.ldscript
 .endif
 LINKFLAGS_NORMAL=  -X
 
Index: sys/arch/amd64/conf/kern.ldscript
===
RCS file: /cvsroot/src/sys/arch/amd64/conf/kern.ldscript,v
retrieving revision 1.30
diff -U4 -r1.30 kern.ldscript
--- sys/arch/amd64/conf/kern.ldscript   15 Dec 2019 02:56:40 -  1.30
+++ sys/arch/amd64/conf/kern.ldscript   4 Jan 2020 01:48:11 -
@@ -12,20 +12,11 @@
 
 ENTRY(_start)
 SECTIONS
 {
-   /*
-* multiboot (file_offset) : AT (load_address) 
-* file_offset must be below 32k for multiboot 2 specification
-* BIOS boot requires load_address above 0x20
-*/
-   multiboot 0x1000 : AT (0x20)
+   .text : AT (ADDR(.text) & 0x0fff)
{
-   . = ALIGN(8);
KEEP(*(multiboot));
-   }
-   .text : AT (0x20 + SIZEOF(multiboot))
-   {
. = ALIGN(__PAGE_SIZE);
__text_user_start = . ;
*(.text.user)
. = ALIGN(__PAGE_SIZE);

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

[Emmanuel Dreyfus]

Masanobu SAITOH  wrote:

>  I have a UEFI boot machine and it also doesn't boot well.
> 
>  - It hangs after attaching ioapic0, cpu0 or acpi0 (or something else).
>The possibility is about 65%
>  - It sometimes panic in acpi_attach(), acpimcfg_probe or something else.
>The possibility is about 10%
>  - It sometimes boot up.
>The possibility is about 25%

I suspect this is a second bug that was undercovered by the multiboot
change. I get crashes like the one you report 100% reproductible with
qemu UEFI boot. I described the thing here:
http://mail-index.netbsd.org/tech-kern/2020/01/02/msg025911.html

Could you check with ddb the physical address accessed? Here is the
relevant excerpt in the message I posted:

db{0}> x/i $rip
netbsd:kmem_intr_alloc+0x64:movq%r12,0(%rax)
db{0}> print $rax
92057868
db{0}> call vtophys(92057868)
108

If you can add a #define DEBUG_MEMMAP 1 at the beginning of
src/sys/arch/x86/x86/efi.c and x86_machdep.c you will also have the
memory map provided by UEFI.

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
m...@netbsd.org

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

[Maxime Villard]

Le 30/12/2019 à 16:15, Emmanuel Dreyfus a écrit :
> On Sat, Dec 28, 2019 at 02:22:21AM +, Emmanuel Dreyfus wrote:
>>> Regardless of whether it is needed in this specific case, cutting the 2MBs
>>> of zero in the binary is wanted. Unfortunately last I looked at this (two
>>> years ago) there were some non-obvious consequences, and it needs to be
>>> carefully done.
>>
>> Any hints about the problems you encountered? Perhaps we can work it
>> around with an . = ALIGN(__LARGEE_PAGE_SIZE); before including .text.user ?
> 
> No anwser here? It is difficult to address an unknown problem...

Sorry for the delay. I don't remember the specifics, but it had to do with
misaligned sections in the end. There is also some pretty retarded code in
loadfile_elf32.c that should like some investigation, at least for this
kind of change.

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.

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

[Emmanuel Dreyfus]

On Sat, Dec 28, 2019 at 02:22:21AM +, Emmanuel Dreyfus wrote:
> > Regardless of whether it is needed in this specific case, cutting the 2MBs
> > of zero in the binary is wanted. Unfortunately last I looked at this (two
> > years ago) there were some non-obvious consequences, and it needs to be
> > carefully done.
> 
> Any hints about the problems you encountered? Perhaps we can work it
> around with an . = ALIGN(__LARGEE_PAGE_SIZE); before including .text.user ?

No anwser here? It is difficult to address an unknown problem...

-- 
Emmanuel Dreyfus
m...@netbsd.org

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

[Emmanuel Dreyfus]

On Fri, Dec 27, 2019 at 06:24:07PM +0100, Maxime Villard wrote:
> Now that I'm looking at i386 I see you've indeed made the same nonsensical
> changes there, with all the unnecessary garbage in the code.

Here I assume you refer to the starting at efi_multiboot2_loader, since
most of the other significant  multiboot stuff has been there for 13 years.

It is copied from bootloader's startprog.S. How do you suggest to
improve it?


-- 
Emmanuel Dreyfus
m...@netbsd.org

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

[Emmanuel Dreyfus]

On Fri, Dec 27, 2019 at 06:24:07PM +0100, Maxime Villard wrote:
>   .text : AT (ADDR(.text) & 0x0fff)
>   {
> + *(.multiboot)
> +
>   . = ALIGN(__PAGE_SIZE);
>   __text_user_start = . ;
>   ...
> 
> This guarantees that the structure is at the beginning of text.

That works. We can even make the multiboot section a note, for the sake
on cleanliness. (see attached patch. MULTIBOOT is enabled for testing).

> Regardless of whether it is needed in this specific case, cutting the 2MBs
> of zero in the binary is wanted. Unfortunately last I looked at this (two
> years ago) there were some non-obvious consequences, and it needs to be
> carefully done.

Any hints about the problems you encountered? Perhaps we can work it
around with an . = ALIGN(__LARGEE_PAGE_SIZE); before including .text.user ?

> Also, my previous remarks haven't been addressed entirely, and still stand.

Sure, it's just next in the todo list.

-- 
Emmanuel Dreyfus
m...@netbsd.org
? sys/arch/amd64/compile/obj
? sys/arch/amd64/stand/prekern/obj
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   28 Dec 2019 01:41:03 -
@@ -431,10 +431,10 @@
.size   tmpstk, tmpstk - .
.space  512
 tmpstk:
 
-.section multiboot,"a"
 #if defined(MULTIBOOT)
+.section multiboot,"",@note
.align  8
.globl  Multiboot2_Header
 _C_LABEL(Multiboot2_Header):
.intMULTIBOOT2_HEADER_MAGIC
Index: sys/arch/amd64/conf/GENERIC
===
RCS file: /cvsroot/src/sys/arch/amd64/conf/GENERIC,v
retrieving revision 1.551
diff -U4 -r1.551 GENERIC
--- sys/arch/amd64/conf/GENERIC 14 Dec 2019 07:45:20 -  1.551
+++ sys/arch/amd64/conf/GENERIC 28 Dec 2019 01:41:03 -
@@ -25,9 +25,9 @@
 #ident "GENERIC-$Revision: 1.551 $"
 
 maxusers   64  # estimated number of users
 
-#options   MULTIBOOT   # Multiboot support (see multiboot(8)) 
+optionsMULTIBOOT   # Multiboot support (see multiboot(8)) 
 
 # delay between "rebooting ..." message and hardware reset, in milliseconds
 #options   CPURESET_DELAY=2000
 
Index: sys/arch/amd64/conf/Makefile.amd64
===
RCS file: /cvsroot/src/sys/arch/amd64/conf/Makefile.amd64,v
retrieving revision 1.80
diff -U4 -r1.80 Makefile.amd64
--- sys/arch/amd64/conf/Makefile.amd64  14 Nov 2019 16:23:52 -  1.80
+++ sys/arch/amd64/conf/Makefile.amd64  28 Dec 2019 01:41:03 -
@@ -90,12 +90,12 @@
 ## (5) link settings
 ##
 TEXTADDR?= 0x8020
 .if defined(KASLR)
-EXTRA_LINKFLAGS=   --split-by-file=0x10 -r -d
+EXTRA_LINKFLAGS=   --split-by-file=0x10 -r -d -n
 KERNLDSCRIPT?= ${AMD64}/conf/kern.ldscript.kaslr
 .else
-EXTRA_LINKFLAGS=   -z max-page-size=0x20
+EXTRA_LINKFLAGS=   -z max-page-size=0x20 -n
 KERNLDSCRIPT?= ${AMD64}/conf/kern.ldscript
 .endif
 LINKFLAGS_NORMAL=  -X
 
Index: sys/arch/amd64/conf/kern.ldscript
===
RCS file: /cvsroot/src/sys/arch/amd64/conf/kern.ldscript,v
retrieving revision 1.30
diff -U4 -r1.30 kern.ldscript
--- sys/arch/amd64/conf/kern.ldscript   15 Dec 2019 02:56:40 -  1.30
+++ sys/arch/amd64/conf/kern.ldscript   28 Dec 2019 01:41:03 -
@@ -12,20 +12,11 @@
 
 ENTRY(_start)
 SECTIONS
 {
-   /*
-* multiboot (file_offset) : AT (load_address) 
-* file_offset must be below 32k for multiboot 2 specification
-* BIOS boot requires load_address above 0x20
-*/
-   multiboot 0x1000 : AT (0x20)
+   .text : AT (ADDR(.text) & 0x0fff)
{
-   . = ALIGN(8);
KEEP(*(multiboot));
-   }
-   .text : AT (0x20 + SIZEOF(multiboot))
-   {
. = ALIGN(__PAGE_SIZE);
__text_user_start = . ;
*(.text.user)
. = ALIGN(__PAGE_SIZE);

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

[Maxime Villard]

Le 27/12/2019 à 17:45, Emmanuel Dreyfus a écrit :
> On Fri, Dec 27, 2019 at 09:02:17AM +0100, Maxime Villard wrote:
>> Please stop with the nonsense... In this patch you are making the multiboot
>> header executable, and putting it in a section shared with userland under
>> SVS. Neither should be required; more than that, both are absolutely _not_
>> wanted.
>
> What are the actual drawbacks?

You are moving the structure to an area where it does not belong _at all_.
We don't want to map things in userland for no reason.

Now that I'm looking at the thing closely, I see why we are forced to make
it executable. So, that's fine for that part.

> FWIW, this is in line with how it was done on i386: it is just stored
> at the beginning of .text. Xen does the same. Of course it seems more
> natural to store that in a note section this is not loaded, but after
> experimenting a lot, I am not sure it can be done, since ld really
> want to push notes at the end of the file.

Now that I'm looking at i386 I see you've indeed made the same nonsensical
changes there, with all the unnecessary garbage in the code.

To me, you only need

.section .multiboot,"ax",@progbits

and

.text : AT (ADDR(.text) & 0x0fff)
{
+   *(.multiboot)
+
. = ALIGN(__PAGE_SIZE);
__text_user_start = . ;
...

This guarantees that the structure is at the beginning of text.

Regardless of whether it is needed in this specific case, cutting the 2MBs
of zero in the binary is wanted. Unfortunately last I looked at this (two
years ago) there were some non-obvious consequences, and it needs to be
carefully done.

Also, my previous remarks haven't been addressed entirely, and still stand.

Maxime

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

[Emmanuel Dreyfus]

On Fri, Dec 27, 2019 at 09:02:17AM +0100, Maxime Villard wrote:
> Please stop with the nonsense... In this patch you are making the multiboot
> header executable, and putting it in a section shared with userland under
> SVS. Neither should be required; more than that, both are absolutely _not_
> wanted.

What are the actual drawbacks? 

FWIW, this is in line with how it was done on i386: it is just stored
at the beginning of .text. Xen does the same. Of course it seems more
natural to store that in a note section this is not loaded, but after
experimenting a lot, I am not sure it can be done, since ld really
want to push notes at the end of the file.

-- 
Emmanuel Dreyfus
m...@netbsd.org

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

[Ryo ONODERA]

Hi,

Your patch works fine for my laptop too.

Thank you.

Masanobu SAITOH  writes:

> On 2019/12/27 1:55, Emmanuel Dreyfus wrote:
>> On Wed, Dec 25, 2019 at 05:05:11PM +0900, Masanobu SAITOH wrote:
> After this change, amd64 kernel does not boot on my HP Spectre x360
> 13-inch ae019TU laptop with pure UEFI boot mode.
>>>  I have a UEFI boot machine and it also doesn't boot well.
>> 
>> Please try the attached patch.
>> 
>> It adds the -n flag to ld, which disable auto-alignment of sections
>> in the file. I undestand alignement is highly desirable for userland
>> programs that may be mapped from file, but useless for the kernel,
>> which is just readen once by the bootloader.
>> 
>> Without auto-alignement, the .text segment starts right after the
>> ELF headers. This means the multiboot header can go in .text and
>> stay below 32k (as required by the multiboot specification). There
>> is no need for a multiboot section for that, and therefore no 
>> need to modify the linker script.
>> 
>> A side effect is that the kernel file shrinks of 2 MB, because there
>> is not an alignement hole between ELF headers and the .text section
>> anymore.
>> 
>> My patch also enable the MULTIBOOT option so that we can check
>> nothing gets broken with it. You can also try with the option
>> disabled, of course.
>> 
>
> Both with and without MULTIBOOT works fine. No any hangup/panic.
>
>  Thanks.
>
> -- 
> ---
> SAITOH Masanobu (msai...@execsw.org
>  msai...@netbsd.org)

-- 
Ryo ONODERA // r...@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

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

[Maxime Villard]

Le 26/12/2019 à 17:55, Emmanuel Dreyfus a écrit :
> On Wed, Dec 25, 2019 at 05:05:11PM +0900, Masanobu SAITOH wrote:
 After this change, amd64 kernel does not boot on my HP Spectre x360
 13-inch ae019TU laptop with pure UEFI boot mode.
>>  I have a UEFI boot machine and it also doesn't boot well.
> 
> Please try the attached patch.
> 
> It adds the -n flag to ld, which disable auto-alignment of sections
> in the file. I undestand alignement is highly desirable for userland
> programs that may be mapped from file, but useless for the kernel,
> which is just readen once by the bootloader.
> 
> Without auto-alignement, the .text segment starts right after the
> ELF headers. This means the multiboot header can go in .text and
> stay below 32k (as required by the multiboot specification). There
> is no need for a multiboot section for that, and therefore no 
> need to modify the linker script.
> 
> A side effect is that the kernel file shrinks of 2 MB, because there
> is not an alignement hole between ELF headers and the .text section
> anymore.
> 
> My patch also enable the MULTIBOOT option so that we can check
> nothing gets broken with it. You can also try with the option
> disabled, of course.

Please stop with the nonsense... In this patch you are making the multiboot
header executable, and putting it in a section shared with userland under
SVS. Neither should be required; more than that, both are absolutely _not_
wanted.

Instead of trying to patch-work the thing over and over, you should
probably revert it all, take an hour to peacefully write it correctly, and
then submit it again. Given the way multiboot was written so far I don't
see how we can accept to enable it by default.

Maxime

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

[Masanobu SAITOH]

On 2019/12/27 1:55, Emmanuel Dreyfus wrote:
> On Wed, Dec 25, 2019 at 05:05:11PM +0900, Masanobu SAITOH wrote:
 After this change, amd64 kernel does not boot on my HP Spectre x360
 13-inch ae019TU laptop with pure UEFI boot mode.
>>  I have a UEFI boot machine and it also doesn't boot well.
> 
> Please try the attached patch.
> 
> It adds the -n flag to ld, which disable auto-alignment of sections
> in the file. I undestand alignement is highly desirable for userland
> programs that may be mapped from file, but useless for the kernel,
> which is just readen once by the bootloader.
> 
> Without auto-alignement, the .text segment starts right after the
> ELF headers. This means the multiboot header can go in .text and
> stay below 32k (as required by the multiboot specification). There
> is no need for a multiboot section for that, and therefore no 
> need to modify the linker script.
> 
> A side effect is that the kernel file shrinks of 2 MB, because there
> is not an alignement hole between ELF headers and the .text section
> anymore.
> 
> My patch also enable the MULTIBOOT option so that we can check
> nothing gets broken with it. You can also try with the option
> disabled, of course.
> 

Both with and without MULTIBOOT works fine. No any hangup/panic.

 Thanks.

-- 
---
SAITOH Masanobu (msai...@execsw.org
 msai...@netbsd.org)

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

[Emmanuel Dreyfus]

On Wed, Dec 25, 2019 at 05:05:11PM +0900, Masanobu SAITOH wrote:
> >> After this change, amd64 kernel does not boot on my HP Spectre x360
> >> 13-inch ae019TU laptop with pure UEFI boot mode.
>  I have a UEFI boot machine and it also doesn't boot well.

Please try the attached patch.

It adds the -n flag to ld, which disable auto-alignment of sections
in the file. I undestand alignement is highly desirable for userland
programs that may be mapped from file, but useless for the kernel,
which is just readen once by the bootloader.

Without auto-alignement, the .text segment starts right after the
ELF headers. This means the multiboot header can go in .text and
stay below 32k (as required by the multiboot specification). There
is no need for a multiboot section for that, and therefore no 
need to modify the linker script.

A side effect is that the kernel file shrinks of 2 MB, because there
is not an alignement hole between ELF headers and the .text section
anymore.

My patch also enable the MULTIBOOT option so that we can check
nothing gets broken with it. You can also try with the option
disabled, of course.

-- 
Emmanuel Dreyfus
m...@netbsd.org
? sys/arch/amd64/compile/obj
? sys/arch/amd64/stand/prekern/obj
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   26 Dec 2019 16:44:10 -
@@ -431,51 +431,8 @@
.size   tmpstk, tmpstk - .
.space  512
 tmpstk:
 
-.section multiboot,"a"
-#if defined(MULTIBOOT)
-   .align  8
-   .globl  Multiboot2_Header
-_C_LABEL(Multiboot2_Header):
-   .intMULTIBOOT2_HEADER_MAGIC
-   .intMULTIBOOT2_ARCHITECTURE_I386
-   .intMultiboot2_Header_end - Multiboot2_Header
-   .int-(MULTIBOOT2_HEADER_MAGIC + MULTIBOOT2_ARCHITECTURE_I386 \
-   + (Multiboot2_Header_end - Multiboot2_Header))
-
-   .int1   /* MULTIBOOT_HEADER_TAG_INFORMATION_REQUEST */
-   .int12  /* sizeof(multiboot_header_tag_information_request) */
-   /* + sizeof(uint32_t) * requests */
-   .int4   /* MULTIBOOT_TAG_TYPE_BASIC_MEMINFO */
-   .align  8
-
-   .int3   /* MULTIBOOT_HEADER_TAG_ENTRY_ADDRESS */
-   .int16  /* sizeof(struct multiboot_tag_efi64) */
-   .quad   (multiboot2_entry - KERNBASE)
-   .align  8
-
-   .int9   /* MULTIBOOT_HEADER_TAG_ENTRY_ADDRESS_EFI64 */
-   .int16  /* sizeof(struct multiboot_tag_efi64) */
-   .quad   (multiboot2_entry - KERNBASE)
-   .align  8
-
-#if notyet
-   /*
-* Could be used to get an early console for debug,
-* but this is broken.
-*/
-   .int7   /* MULTIBOOT_HEADER_TAG_EFI_BS */
-   .int8   /* sizeof(struct multiboot_tag) */
-   .align  8
-#endif
-
-   .int0   /* MULTIBOOT_HEADER_TAG_END */
-   .int8   /* sizeof(struct multiboot_tag) */
-   .align  8
-   .globl  Multiboot2_Header_end
-_C_LABEL(Multiboot2_Header_end):
-#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.
@@ -2179,8 +2136,50 @@
 SYSCALL_ENTRY  syscall,is_svs=0
 
TEXT_USER_BEGIN
 
+#if defined(MULTIBOOT)
+   .align  8
+   .globl  Multiboot2_Header
+_C_LABEL(Multiboot2_Header):
+   .intMULTIBOOT2_HEADER_MAGIC
+   .intMULTIBOOT2_ARCHITECTURE_I386
+   .intMultiboot2_Header_end - Multiboot2_Header
+   .int-(MULTIBOOT2_HEADER_MAGIC + MULTIBOOT2_ARCHITECTURE_I386 \
+   + (Multiboot2_Header_end - Multiboot2_Header))
+
+   .int1   /* MULTIBOOT_HEADER_TAG_INFORMATION_REQUEST */
+   .int12  /* sizeof(multiboot_header_tag_information_request) */
+   /* + sizeof(uint32_t) * requests */
+   .int4   /* MULTIBOOT_TAG_TYPE_BASIC_MEMINFO */
+   .align  8
+
+   .int3   /* MULTIBOOT_HEADER_TAG_ENTRY_ADDRESS */
+   .int16  /* sizeof(struct multiboot_tag_efi64) */
+   .quad   (multiboot2_entry - KERNBASE)
+   .align  8
+
+   .int9   /* MULTIBOOT_HEADER_TAG_ENTRY_ADDRESS_EFI64 */
+   .int16  /* sizeof(struct multiboot_tag_efi64) */
+   .quad   (multiboot2_entry - KERNBASE)
+   .align  8
+
+#if notyet
+   /*
+* Could be used to get an early console for debug,
+* but this is broken.
+*/
+   .int7   /* MULTIBOOT_HEADER_TAG_EFI_BS */
+   .int8   /* sizeof(struct multiboot_tag) */
+   .align  8
+#endif
+
+   .int0   /* MULTIBOOT_HEADER_TAG_END */
+   .int8   /* sizeof(struct multiboot_tag) */
+   .align  8
+   .globl  

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

[Emmanuel Dreyfus]

On Wed, Dec 25, 2019 at 05:05:11PM +0900, Masanobu SAITOH wrote:
>  - It hangs after attaching ioapic0, cpu0 or acpi0 (or something else).
>The possibility is about 65%

What is the backtace? Does it goes through svs_init?

-- 
Emmanuel Dreyfus
m...@netbsd.org

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

[Ryo ONODERA]

Sorry for confusing.

My attached patch does not improve my situation.
My mail is mistake. Sorry.

Reverting linker script fixes the kernel boot.

Thank you.

On December 26, 2019 1:23:34 AM GMT+09:00, Emmanuel Dreyfus  
wrote:
>On Wed, Dec 25, 2019 at 07:42:47PM +0900, Ryo ONODERA wrote:
>> The attached patch works for me.
>> However I have no idea about the meaning.
>
>It changes the multiboot section from DATA to CODE, which is
>odd but perfectly fine. I cannot understand how it can change
>the situation, though. Did it really fix the problem? Your 
>next message about reverted kern.ldscript  confuses me.

-- 
Ryo ONODERA // r...@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB FD1B F404 27FA C7D1 15F3

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

[Emmanuel Dreyfus]

On Wed, Dec 25, 2019 at 07:42:47PM +0900, Ryo ONODERA wrote:
> The attached patch works for me.
> However I have no idea about the meaning.

It changes the multiboot section from DATA to CODE, which is
odd but perfectly fine. I cannot understand how it can change
the situation, though. Did it really fix the problem? Your 
next message about reverted kern.ldscript  confuses me.

-- 
Emmanuel Dreyfus
m...@netbsd.org

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

[Ryo ONODERA]

Hi,

Sorry I have accidentally reverted kern.ldscript.
With current kern.ldscript, it stalls after cpu0.

Thank you.

Ryo ONODERA  writes:

> Hi,
>
> Emmanuel Dreyfus  writes:
>
>> On Tue, Dec 24, 2019 at 05:50:00PM +0900, Ryo ONODERA wrote:
>>> After this change, amd64 kernel does not boot on my HP Spectre x360
>>> 13-inch ae019TU laptop with pure UEFI boot mode.
>>
>> Hello
>>
>> Does the attached patch (crafted for port-amd64/54775) fix the
>> problem?
>
> Without your patch, the kernel does not boot at all,
> so after the kernel is loaded, only "_" character is displayed.
>
> And your patch still does not work properly, so the kernel hangs
> after cpu0 (as same as msaitoh@'s however it hits 100% for my
> laptop).
>
> The attached patch works for me.
> However I have no idea about the meaning.
>
> Index: sys/arch/amd64/amd64/locore.S
> ===
> RCS file: /cvsroot/src/sys/arch/amd64/amd64/locore.S,v
> retrieving revision 1.195
> diff -u -r1.195 locore.S
> --- sys/arch/amd64/amd64/locore.S 15 Dec 2019 02:58:21 -  1.195
> +++ sys/arch/amd64/amd64/locore.S 25 Dec 2019 10:36:18 -
> @@ -1,4 +1,4 @@
> -/*   $NetBSD: locore.S,v 1.195 2019/12/15 02:58:21 manu Exp $*/
> +/*   $NetBSD: locore.S,v 1.193 2019/12/10 02:06:07 manu Exp $*/
>  
>  /*
>   * Copyright-o-rama!
> @@ -432,9 +432,9 @@
>   .space  512
>  tmpstk:
>  
> -.section multiboot,"a"
> -#if defined(MULTIBOOT)
> +.section multiboot,"ax",@progbits
>   .align  8
> +#if defined(MULTIBOOT)
>   .globl  Multiboot2_Header
>  _C_LABEL(Multiboot2_Header):
>   .intMULTIBOOT2_HEADER_MAGIC
> @@ -474,6 +474,9 @@
>   .align  8
>   .globl  Multiboot2_Header_end
>  _C_LABEL(Multiboot2_Header_end):
> +#else /* MULTIBOOT */
> + .int0xdeadbeef  /* have some non empty content */
> + .align  8
>  #endif   /* MULTIBOOT */
>  
>  /*
>
> Thank you.
>
>> -- 
>> 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.S15 Dec 2019 02:58:21 -  1.195
>> +++ sys/arch/amd64/amd64/locore.S22 Dec 2019 02:23:11 -
>> @@ -432,10 +432,10 @@
>>  .space  512
>>  tmpstk:
>>  
>>  .section multiboot,"a"
>> -#if defined(MULTIBOOT)
>>  .align  8
>> +#if defined(MULTIBOOT)
>>  .globl  Multiboot2_Header
>>  _C_LABEL(Multiboot2_Header):
>>  .intMULTIBOOT2_HEADER_MAGIC
>>  .intMULTIBOOT2_ARCHITECTURE_I386
>> @@ -473,8 +473,11 @@
>>  .int8   /* sizeof(struct multiboot_tag) */
>>  .align  8
>>  .globl  Multiboot2_Header_end
>>  _C_LABEL(Multiboot2_Header_end):
>> +#else /* MULTIBOOT */
>> +.int0xdeadbeef  /* have some non empty content */
>> +.align  8
>>  #endif  /* MULTIBOOT */
>>  
>>  /*
>>   * Some hackage to deal with 64bit symbols in 32 bit mode.
>
> -- 
> Ryo ONODERA // r...@tetera.org
> PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

-- 
Ryo ONODERA // r...@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

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

[Ryo ONODERA]

Hi,

Emmanuel Dreyfus  writes:

> On Tue, Dec 24, 2019 at 05:50:00PM +0900, Ryo ONODERA wrote:
>> After this change, amd64 kernel does not boot on my HP Spectre x360
>> 13-inch ae019TU laptop with pure UEFI boot mode.
>
> Hello
>
> Does the attached patch (crafted for port-amd64/54775) fix the
> problem?

Without your patch, the kernel does not boot at all,
so after the kernel is loaded, only "_" character is displayed.

And your patch still does not work properly, so the kernel hangs
after cpu0 (as same as msaitoh@'s however it hits 100% for my
laptop).

The attached patch works for me.
However I have no idea about the meaning.

Index: sys/arch/amd64/amd64/locore.S
===
RCS file: /cvsroot/src/sys/arch/amd64/amd64/locore.S,v
retrieving revision 1.195
diff -u -r1.195 locore.S
--- sys/arch/amd64/amd64/locore.S	15 Dec 2019 02:58:21 -	1.195
+++ sys/arch/amd64/amd64/locore.S	25 Dec 2019 10:36:18 -
@@ -1,4 +1,4 @@
-/*	$NetBSD: locore.S,v 1.195 2019/12/15 02:58:21 manu Exp $	*/
+/*	$NetBSD: locore.S,v 1.193 2019/12/10 02:06:07 manu Exp $	*/
 
 /*
  * Copyright-o-rama!
@@ -432,9 +432,9 @@
 	.space	512
 tmpstk:
 
-.section multiboot,"a"
-#if defined(MULTIBOOT)
+.section multiboot,"ax",@progbits
 	.align	8
+#if defined(MULTIBOOT)
 	.globl	Multiboot2_Header
 _C_LABEL(Multiboot2_Header):
 	.int	MULTIBOOT2_HEADER_MAGIC
@@ -474,6 +474,9 @@
 	.align	8
 	.globl	Multiboot2_Header_end
 _C_LABEL(Multiboot2_Header_end):
+#else /* MULTIBOOT */
+	.int	0xdeadbeef	/* have some non empty content */
+	.align	8
 #endif	/* MULTIBOOT */
 
 /*

Thank you.

> -- 
> 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 22 Dec 2019 02:23:11 -
> @@ -432,10 +432,10 @@
>   .space  512
>  tmpstk:
>  
>  .section multiboot,"a"
> -#if defined(MULTIBOOT)
>   .align  8
> +#if defined(MULTIBOOT)
>   .globl  Multiboot2_Header
>  _C_LABEL(Multiboot2_Header):
>   .intMULTIBOOT2_HEADER_MAGIC
>   .intMULTIBOOT2_ARCHITECTURE_I386
> @@ -473,8 +473,11 @@
>   .int8   /* sizeof(struct multiboot_tag) */
>   .align  8
>   .globl  Multiboot2_Header_end
>  _C_LABEL(Multiboot2_Header_end):
> +#else /* MULTIBOOT */
> + .int0xdeadbeef  /* have some non empty content */
> + .align  8
>  #endif   /* MULTIBOOT */
>  
>  /*
>   * Some hackage to deal with 64bit symbols in 32 bit mode.

-- 
Ryo ONODERA // r...@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

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

[Masanobu SAITOH]

On 2019/12/25 17:05, Masanobu SAITOH wrote:
> On 2019/12/24 23:47, Emmanuel Dreyfus wrote:
>> On Tue, Dec 24, 2019 at 05:50:00PM +0900, Ryo ONODERA wrote:
>>> After this change, amd64 kernel does not boot on my HP Spectre x360
>>> 13-inch ae019TU laptop with pure UEFI boot mode.
> 
>  I have a UEFI boot machine and it also doesn't boot well.
> 
>  - It hangs after attaching ioapic0, cpu0 or acpi0 (or something else).
>The possibility is about 65%
>  - It sometimes panic in acpi_attach(), acpimcfg_probe or something else.
>The possibility is about 10%
>  - It sometimes boot up.
>The possibility is about 25%
> 
>> Hello
>>
>> Does the attached patch (crafted for port-amd64/54775) fix the
>> problem?
> 
>  I tried your patch(It can't apply cleanly, so a applied it by hand).
> I can't see any improvement.
> 
>  It seems for me that ACPI table can't decode correctly when it couldn't
> boot up.
> 

Boot:
http://mail-index.netbsd.org/source-changes/2019/12/15/msg111926.html

Hang or panic:
http://mail-index.netbsd.org/source-changes/2019/12/15/msg111928.html

-- 
---
SAITOH Masanobu (msai...@execsw.org
 msai...@netbsd.org)

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

[Masanobu SAITOH]

On 2019/12/24 23:47, Emmanuel Dreyfus wrote:
> On Tue, Dec 24, 2019 at 05:50:00PM +0900, Ryo ONODERA wrote:
>> After this change, amd64 kernel does not boot on my HP Spectre x360
>> 13-inch ae019TU laptop with pure UEFI boot mode.

 I have a UEFI boot machine and it also doesn't boot well.

 - It hangs after attaching ioapic0, cpu0 or acpi0 (or something else).
   The possibility is about 65%
 - It sometimes panic in acpi_attach(), acpimcfg_probe or something else.
   The possibility is about 10%
 - It sometimes boot up.
   The possibility is about 25%

> Hello
> 
> Does the attached patch (crafted for port-amd64/54775) fix the
> problem?

 I tried your patch(It can't apply cleanly, so a applied it by hand).
I can't see any improvement.

 It seems for me that ACPI table can't decode correctly when it couldn't
boot up.

-- 
---
SAITOH Masanobu (msai...@execsw.org
 msai...@netbsd.org)

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

[Emmanuel Dreyfus]

On Tue, Dec 24, 2019 at 05:50:00PM +0900, Ryo ONODERA wrote:
> After this change, amd64 kernel does not boot on my HP Spectre x360
> 13-inch ae019TU laptop with pure UEFI boot mode.

Hello

Does the attached patch (crafted for port-amd64/54775) fix the
problem?

-- 
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   22 Dec 2019 02:23:11 -
@@ -432,10 +432,10 @@
.space  512
 tmpstk:
 
 .section multiboot,"a"
-#if defined(MULTIBOOT)
.align  8
+#if defined(MULTIBOOT)
.globl  Multiboot2_Header
 _C_LABEL(Multiboot2_Header):
.intMULTIBOOT2_HEADER_MAGIC
.intMULTIBOOT2_ARCHITECTURE_I386
@@ -473,8 +473,11 @@
.int8   /* sizeof(struct multiboot_tag) */
.align  8
.globl  Multiboot2_Header_end
 _C_LABEL(Multiboot2_Header_end):
+#else /* MULTIBOOT */
+   .int0xdeadbeef  /* have some non empty content */
+   .align  8
 #endif /* MULTIBOOT */
 
 /*
  * Some hackage to deal with 64bit symbols in 32 bit mode.

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

[Ryo ONODERA]

Hi,

"Emmanuel Dreyfus"  writes:

> Module Name:  src
> Committed By: manu
> Date: Sun Dec 15 02:56:40 UTC 2019
>
> Modified Files:
>   src/sys/arch/amd64/amd64: locore.S
>   src/sys/arch/amd64/conf: kern.ldscript
>
> Log Message:
> Restore multiboot 2 header in amd64 kernel
>
> The header must appear below 32k offset in the kernel file, but we
> have to make sure it does not load at low addresses, otherwise we
> break BIOS boot.
>
> .text section used to load at 0x20, we just load multiboot section
> there, and have .text loaded just after.

After this change, amd64 kernel does not boot on my HP Spectre x360
13-inch ae019TU laptop with pure UEFI boot mode.

Reverting this change on 2019-12-23T08:00 (UTC) enables pure UEFI boot.

On another laptop, VAIO S13, the same kernel boots fine in UEFI mode.

I want to continue to use my HP Spectre x360.
Could you take a look at my problem?

Thank you.

> To generate a diff of this commit:
> cvs rdiff -u -r1.193 -r1.194 src/sys/arch/amd64/amd64/locore.S
> cvs rdiff -u -r1.29 -r1.30 src/sys/arch/amd64/conf/kern.ldscript
>
> 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/locore.S
> diff -u src/sys/arch/amd64/amd64/locore.S:1.193 
> src/sys/arch/amd64/amd64/locore.S:1.194
> --- src/sys/arch/amd64/amd64/locore.S:1.193   Tue Dec 10 02:06:07 2019
> +++ src/sys/arch/amd64/amd64/locore.S Sun Dec 15 02:56:40 2019
> @@ -1,4 +1,4 @@
> -/*   $NetBSD: locore.S,v 1.193 2019/12/10 02:06:07 manu Exp $*/
> +/*   $NetBSD: locore.S,v 1.194 2019/12/15 02:56:40 manu Exp $*/
>  
>  /*
>   * Copyright-o-rama!
> @@ -432,7 +432,7 @@ END(farjmp64)
>   .space  512
>  tmpstk:
>  
> -.section multiboot,"ax",@progbits
> +.section multiboot,"a"
>  #if defined(MULTIBOOT)
>   .align  8
>   .globl  Multiboot2_Header
>
> Index: src/sys/arch/amd64/conf/kern.ldscript
> diff -u src/sys/arch/amd64/conf/kern.ldscript:1.29 
> src/sys/arch/amd64/conf/kern.ldscript:1.30
> --- src/sys/arch/amd64/conf/kern.ldscript:1.29Wed Dec 11 02:31:44 2019
> +++ src/sys/arch/amd64/conf/kern.ldscript Sun Dec 15 02:56:40 2019
> @@ -1,4 +1,4 @@
> -/*   $NetBSD: kern.ldscript,v 1.29 2019/12/11 02:31:44 manu Exp $*/
> +/*   $NetBSD: kern.ldscript,v 1.30 2019/12/15 02:56:40 manu Exp $*/
>  
>  #include "assym.h"
>  
> @@ -13,7 +13,17 @@ __LARGE_PAGE_SIZE = 0x20 ;
>  ENTRY(_start)
>  SECTIONS
>  {
> - .text : AT (ADDR(.text) & 0x0fff)
> + /*
> +  * multiboot (file_offset) : AT (load_address) 
> +  * file_offset must be below 32k for multiboot 2 specification
> +  * BIOS boot requires load_address above 0x20
> +  */
> + multiboot 0x1000 : AT (0x20)
> + {
> + . = ALIGN(8);
> + KEEP(*(multiboot));
> + }
> + .text : AT (0x20 + SIZEOF(multiboot))
>   {
>   . = ALIGN(__PAGE_SIZE);
>   __text_user_start = . ;
>

-- 
Ryo ONODERA // r...@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

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

[Kamil Rytarowski]

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

[Maxime Villard]

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

[matthew green]

"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/conf

[Kamil Rytarowski]

On 07.08.2019 08:28, Maxime Villard wrote:
> Module Name:  src
> Committed By: maxv
> Date: Wed Aug  7 06:28:03 UTC 2019
> 
> Modified Files:
>   src/sys/arch/amd64/conf: GENERIC
> 
> Log Message:
> Sync with reality.
> 
> 

Can we enable USER_LDT by default in GENERIC?

> To generate a diff of this commit:
> cvs rdiff -u -r1.532 -r1.533 src/sys/arch/amd64/conf/GENERIC
> 
> 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/conf/GENERIC
> diff -u src/sys/arch/amd64/conf/GENERIC:1.532 
> src/sys/arch/amd64/conf/GENERIC:1.533
> --- src/sys/arch/amd64/conf/GENERIC:1.532 Thu Aug  1 13:48:50 2019
> +++ src/sys/arch/amd64/conf/GENERIC   Wed Aug  7 06:28:03 2019
> @@ -1,4 +1,4 @@
> -# $NetBSD: GENERIC,v 1.532 2019/08/01 13:48:50 msaitoh Exp $
> +# $NetBSD: GENERIC,v 1.533 2019/08/07 06:28:03 maxv Exp $
>  #
>  # GENERIC machine description file
>  #
> @@ -22,7 +22,7 @@ include "arch/amd64/conf/std.amd64"
>  
>  options  INCLUDE_CONFIG_FILE # embed config file in kernel binary
>  
> -#ident   "GENERIC-$Revision: 1.532 $"
> +#ident   "GENERIC-$Revision: 1.533 $"
>  
>  maxusers 64  # estimated number of users
>  
> @@ -74,15 +74,12 @@ options   USERCONF# userconf(4) support
>  options  SYSCTL_INCLUDE_DESCR# Include sysctl descriptions in kernel
>  
>  # CPU-related options
> +#options USER_LDT# User-settable LDT, used by Wine
>  options  SVS # Separate Virtual Space
>  makeoptions  SPECTRE_V2_GCC_MITIGATION=1 # GCC Spectre variant 2
>   # migitation
>  options  SPECTRE_V2_GCC_MITIGATION
>  
> -# USER_LDT. You need to disable SVS to use it.
> -#options USER_LDT# user-settable LDT; used by WINE
> -#no options  SVS
> -
>  # CPU features
>  acpicpu* at cpu? # ACPI CPU (including frequency scaling)
>  coretemp*at cpu? # Intel on-die thermal sensor
> 




signature.asc
Description: OpenPGP digital signature

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

[Christos Zoulas]

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

[Maxime Villard]

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

[Joerg Sonnenberger]

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

[Maxime Villard]

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

[Joerg Sonnenberger]

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

[Kamil Rytarowski]

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

[Maxime Villard]

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

[Maxime Villard]

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

[Joerg Sonnenberger]

On Tue, Mar 13, 2018 at 06:02:54PM +0100, Maxime Villard wrote:
> Le 11/03/2018 à 22:30, Joerg Sonnenberger a écrit :
> > Haswell, using VTx. It seems to hit a triple fault in db_panic according
> > to the vbox.log.
> 
> At which stage of the boot procedure does it happen? Does it happen right
> before init is launched? Or does the kernel fault right after the bootloader?

Right after the boot loader.

Joerg

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

[Maxime Villard]

Le 11/03/2018 à 22:30, Joerg Sonnenberger a écrit :

On Sat, Mar 10, 2018 at 06:58:39PM +0100, Maxime Villard wrote:

Le 09/03/2018 à 20:33, Joerg Sonnenberger a écrit :

On Fri, Mar 09, 2018 at 06:36:45PM +0100, Maxime Villard wrote:

Le 09/03/2018 à 18:14, Joerg Sonnenberger a écrit :

On Mon, Feb 26, 2018 at 05:52:50AM +, Maxime Villard wrote:

Module Name:src
Committed By:   maxv
Date:   Mon Feb 26 05:52:50 UTC 2018

Modified Files:
src/sys/arch/amd64/conf: GENERIC

Log Message:
Enable SVS by default.


This broke using VirtualBox and I wouldn't be surprised by other
virtualisation solutions to be affected as well.


I'm using VirtualBox with SVS enabled, and have been doing this for two
months already, everything works fine for me.


I get an instant guru meditation once the kernel is loaded and starting.


hardware-assisted or not? I use hardware-assisted, 4.x and 5.x.


Haswell, using VTx. It seems to hit a triple fault in db_panic according
to the vbox.log.


At which stage of the boot procedure does it happen? Does it happen right
before init is launched? Or does the kernel fault right after the bootloader?

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

[Joerg Sonnenberger]

On Sat, Mar 10, 2018 at 06:58:39PM +0100, Maxime Villard wrote:
> Le 09/03/2018 à 20:33, Joerg Sonnenberger a écrit :
> > On Fri, Mar 09, 2018 at 06:36:45PM +0100, Maxime Villard wrote:
> > > Le 09/03/2018 à 18:14, Joerg Sonnenberger a écrit :
> > > > On Mon, Feb 26, 2018 at 05:52:50AM +, Maxime Villard wrote:
> > > > > Module Name:  src
> > > > > Committed By: maxv
> > > > > Date: Mon Feb 26 05:52:50 UTC 2018
> > > > > 
> > > > > Modified Files:
> > > > >   src/sys/arch/amd64/conf: GENERIC
> > > > > 
> > > > > Log Message:
> > > > > Enable SVS by default.
> > > > 
> > > > This broke using VirtualBox and I wouldn't be surprised by other
> > > > virtualisation solutions to be affected as well.
> > > 
> > > I'm using VirtualBox with SVS enabled, and have been doing this for two
> > > months already, everything works fine for me.
> > 
> > I get an instant guru meditation once the kernel is loaded and starting.
> 
> hardware-assisted or not? I use hardware-assisted, 4.x and 5.x.

Haswell, using VTx. It seems to hit a triple fault in db_panic according
to the vbox.log.

Joerg

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

[Martin Husemann]

On Sat, Mar 10, 2018 at 06:58:39PM +0100, Maxime Villard wrote:
> hardware-assisted or not? I use hardware-assisted, 4.x and 5.x.

I guess also the host CPU will matter (at least in virtualbox).

Martin

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

[Maxime Villard]

Le 09/03/2018 à 20:33, Joerg Sonnenberger a écrit :

On Fri, Mar 09, 2018 at 06:36:45PM +0100, Maxime Villard wrote:

Le 09/03/2018 à 18:14, Joerg Sonnenberger a écrit :

On Mon, Feb 26, 2018 at 05:52:50AM +, Maxime Villard wrote:

Module Name:src
Committed By:   maxv
Date:   Mon Feb 26 05:52:50 UTC 2018

Modified Files:
src/sys/arch/amd64/conf: GENERIC

Log Message:
Enable SVS by default.


This broke using VirtualBox and I wouldn't be surprised by other
virtualisation solutions to be affected as well.


I'm using VirtualBox with SVS enabled, and have been doing this for two
months already, everything works fine for me.


I get an instant guru meditation once the kernel is loaded and starting.


hardware-assisted or not? I use hardware-assisted, 4.x and 5.x.

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

[Joerg Sonnenberger]

On Fri, Mar 09, 2018 at 06:36:45PM +0100, Maxime Villard wrote:
> Le 09/03/2018 à 18:14, Joerg Sonnenberger a écrit :
> > On Mon, Feb 26, 2018 at 05:52:50AM +, Maxime Villard wrote:
> > > Module Name:  src
> > > Committed By: maxv
> > > Date: Mon Feb 26 05:52:50 UTC 2018
> > > 
> > > Modified Files:
> > >   src/sys/arch/amd64/conf: GENERIC
> > > 
> > > Log Message:
> > > Enable SVS by default.
> > 
> > This broke using VirtualBox and I wouldn't be surprised by other
> > virtualisation solutions to be affected as well.
> 
> I'm using VirtualBox with SVS enabled, and have been doing this for two
> months already, everything works fine for me.

I get an instant guru meditation once the kernel is loaded and starting.

Joerg

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

[Maxime Villard]

Le 09/03/2018 à 18:36, Maxime Villard a écrit :

Le 09/03/2018 à 18:14, Joerg Sonnenberger a écrit :

On Mon, Feb 26, 2018 at 05:52:50AM +, Maxime Villard wrote:

Module Name:src
Committed By:maxv
Date:Mon Feb 26 05:52:50 UTC 2018

Modified Files:
src/sys/arch/amd64/conf: GENERIC

Log Message:
Enable SVS by default.


This broke using VirtualBox and I wouldn't be surprised by other
virtualisation solutions to be affected as well.


I'm using VirtualBox with SVS enabled, and have been doing this for two
months already, everything works fine for me.


and by the way I also booted NetBSD+SVS on Qemu several times with different
cpu configurations, everything works fine, there is no problem.

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

[Maxime Villard]

Le 09/03/2018 à 18:14, Joerg Sonnenberger a écrit :

On Mon, Feb 26, 2018 at 05:52:50AM +, Maxime Villard wrote:

Module Name:src
Committed By:   maxv
Date:   Mon Feb 26 05:52:50 UTC 2018

Modified Files:
src/sys/arch/amd64/conf: GENERIC

Log Message:
Enable SVS by default.


This broke using VirtualBox and I wouldn't be surprised by other
virtualisation solutions to be affected as well.


I'm using VirtualBox with SVS enabled, and have been doing this for two
months already, everything works fine for me.

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

[Joerg Sonnenberger]

On Mon, Feb 26, 2018 at 05:52:50AM +, Maxime Villard wrote:
> Module Name:  src
> Committed By: maxv
> Date: Mon Feb 26 05:52:50 UTC 2018
> 
> Modified Files:
>   src/sys/arch/amd64/conf: GENERIC
> 
> Log Message:
> Enable SVS by default.

This broke using VirtualBox and I wouldn't be surprised by other
virtualisation solutions to be affected as well.

Joerg

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

[Maxime Villard]

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

[Christos Zoulas]

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

[Maxime Villard]

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

[Martin Husemann]

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

[Maxime Villard]

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

[Martin Husemann]

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

[Maxime Villard]

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

[Christos Zoulas]

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

[Christos Zoulas]

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

[Maxime Villard]

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

[Christos Zoulas]

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

[Maxime Villard]

Le 08/12/2017 à 00:11, Christos Zoulas a écrit :

Module Name:src
Committed By:   christos
Date:   Thu Dec  7 23:11:50 UTC 2017

Modified Files:
src/sys/arch/amd64/amd64: netbsd32_machdep.c
src/sys/arch/amd64/conf: files.amd64
Added Files:
src/sys/arch/amd64/amd64: compat_13_machdep.c

Log Message:
Save maxv@ some work and put back the compat_13_sigreturn changes that allow
amd64 to run ancient i386 binaries.


To generate a diff of this commit:
cvs rdiff -u -r0 -r1.3 src/sys/arch/amd64/amd64/compat_13_machdep.c
cvs rdiff -u -r1.114 -r1.115 src/sys/arch/amd64/amd64/netbsd32_machdep.c
cvs rdiff -u -r1.95 -r1.96 src/sys/arch/amd64/conf/files.amd64

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.


In case you're wondering, the delay is because of my idiotic setup, I left my
build+compile VM in another house. I *would have reverted* my changes once
there (tomorrow).

I disagree on TRAP_SIGDEBUG. It is typically the kind of debug messages that
make the code harder to understand. Just like all the debugging junk I removed
from Xen. (That's almost why kernel modules didn't work there before. In one
place one page happened to overlap on another one. But surrounded by debug
messages, no one had seen that.)

Maxime

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

[David Holland]

On Sat, Dec 02, 2017 at 09:59:02AM +, Maxime Villard wrote:
 > Module Name: src
 > Committed By:maxv
 > Date:Sat Dec  2 09:59:02 UTC 2017
 > 
 > Modified Files:
 >  src/sys/arch/amd64/conf: ALL
 > 
 > Log Message:
 > Remove options that do not exist on amd64.

Compile-testing those is still useful.

-- 
David A. Holland
dholl...@netbsd.org

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

[matthew green]

> > [...]
> > + * Default pager_map of 16MB is awfully small.  There is have plenty
> > + * of VA so use it.
> > [...]
> > 
> > It's only in a comment but "...is have..."?
> 
> Thanks, fixed.

thanks!  i think an earlier version read "we have plenty", but i've
been trying to avoid using "we" in code comments.  :-)


.mrg.

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

[Thomas Klausner]

On Mon, Nov 13, 2017 at 11:16:38AM +1100, Geoff Wing wrote:
> On Sunday 2017-11-12 07:24 +1100, matthew green output:
> :Module Name: src
> :Committed By:mrg
> :Date:Sat Nov 11 20:23:49 UTC 2017
> :Modified Files:
> : src/sys/arch/amd64/include: vmparam.h
> :Log Message:
> :bump PAGER_MAP_DEFAULT_SIZE to 512MB.  this should allow more
> :concurrent IOs to be possible, and i'm unable to see pager_map
> :contention any more.
> :other larger platforms should probably do this too.
> [...]
> :cvs rdiff -u -r1.43 -r1.44 src/sys/arch/amd64/include/vmparam.h
> 
> [...]
> + * Default pager_map of 16MB is awfully small.  There is have plenty
> + * of VA so use it.
> [...]
> 
> It's only in a comment but "...is have..."?

Thanks, fixed.
 Thomas

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

[Geoff Wing]

On Sunday 2017-11-12 07:24 +1100, matthew green output:
:Module Name:   src
:Committed By:  mrg
:Date:  Sat Nov 11 20:23:49 UTC 2017
:Modified Files:
:   src/sys/arch/amd64/include: vmparam.h
:Log Message:
:bump PAGER_MAP_DEFAULT_SIZE to 512MB.  this should allow more
:concurrent IOs to be possible, and i'm unable to see pager_map
:contention any more.
:other larger platforms should probably do this too.
[...]
:cvs rdiff -u -r1.43 -r1.44 src/sys/arch/amd64/include/vmparam.h

[...]
+ * Default pager_map of 16MB is awfully small.  There is have plenty
+ * of VA so use it.
[...]

It's only in a comment but "...is have..."?

Regards,
Geoff

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

[Nick Hudson]

On 10/20/17 11:08, Manuel Bouyer wrote:

On Fri, Oct 20, 2017 at 07:06:18AM -0300, Jared McNeill wrote:

On Fri, 20 Oct 2017, Manuel Bouyer wrote:


Any reason to not add it to other arches (especially i386) too ?

I wasn't sure it would work everywhere because there are structures used to
talk to the firmware that are defined (by both the bwfm driver and the
firmware) w/o __packed.

Are you able to test i386?

no, I don't have such device ...


WFM - I added it.


Nick

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

[Manuel Bouyer]

On Fri, Oct 20, 2017 at 07:06:18AM -0300, Jared McNeill wrote:
> On Fri, 20 Oct 2017, Manuel Bouyer wrote:
> 
> > Any reason to not add it to other arches (especially i386) too ?
> 
> I wasn't sure it would work everywhere because there are structures used to
> talk to the firmware that are defined (by both the bwfm driver and the
> firmware) w/o __packed.
> 
> Are you able to test i386?

no, I don't have such device ...

-- 
Manuel Bouyer 
 NetBSD: 26 ans d'experience feront toujours la difference
--

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

[Jared McNeill]

On Fri, 20 Oct 2017, Manuel Bouyer wrote:


Any reason to not add it to other arches (especially i386) too ?


I wasn't sure it would work everywhere because there are structures used 
to talk to the firmware that are defined (by both the bwfm driver and the 
firmware) w/o __packed.


Are you able to test i386?

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

[Manuel Bouyer]

On Thu, Oct 19, 2017 at 11:59:56PM +, Jared D. McNeill wrote:
> Module Name:  src
> Committed By: jmcneill
> Date: Thu Oct 19 23:59:56 UTC 2017
> 
> Modified Files:
>   src/sys/arch/amd64/conf: GENERIC
> 
> Log Message:
> add bwfm

Any reason to not add it to other arches (especially i386) too ?

-- 
Manuel Bouyer 
 NetBSD: 26 ans d'experience feront toujours la difference
--

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

[Paul Goyette]

See PR kern/51597

There is some rtsock stuff that does not get included in the compat 
module.



On Sat, 19 Aug 2017, co...@sdf.org wrote:


try to run 7.1 ifconfig on 8.0. without COMPAT_70 in the kernel, it won't run.
Its non-compatibility is probably buried deep within a switch statement for a 
generic function.

The only way we can reasonably make it work is:
- lie about what compat is, and build it inot the kernel, which also hurts 
people who want to remove it because they are using weak archs
- make networking modular?
- kernel hot patching (runtime cost)

We can drop the pretense. it doesn't work.

!DSPAM:59979908113027780614962!




+--+--++
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:  |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee dot com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd dot org |
+--+--++

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

[coypu]

try to run 7.1 ifconfig on 8.0. without COMPAT_70 in the kernel, it won't run.
Its non-compatibility is probably buried deep within a switch statement for a 
generic function.

The only way we can reasonably make it work is:
- lie about what compat is, and build it inot the kernel, which also hurts 
people who want to remove it because they are using weak archs
- make networking modular?
- kernel hot patching (runtime cost)

We can drop the pretense. it doesn't work.

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

[Martin Husemann]

On Wed, Aug 16, 2017 at 05:46:29AM +0800, Paul Goyette wrote:
> On Tue, 15 Aug 2017, Martin Husemann wrote:
> 
> > On Tue, Aug 15, 2017 at 04:33:19PM +0200, Maxime Villard wrote:
> > > So we agree? Each compat should be independent.
> > 
> > Yes.
> 
> Well, not totally independent!  We have module dependencies to enable the
> use of common code.

Sure, but independend of each other (top level compat modules). The other
modules are just implementation details.

Btw: if some ifdefs can not be resolved, it would still be good to build
the .o files from a common source, and do other ifdef magic in the file
including the implementation (similar to what we do with Elf32 vs. Elf64)
or just Makefile magic to set the proper ifdefs and output names.

Martin

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

[Paul Goyette]

On Tue, 15 Aug 2017, Martin Husemann wrote:


On Tue, Aug 15, 2017 at 04:33:19PM +0200, Maxime Villard wrote:

So we agree? Each compat should be independent.


Yes.


Well, not totally independent!  We have module dependencies to enable 
the use of common code.



It seems to me that
re-implementing (copy-paste) a few functions for linux is a step towards
direction, isn't it?


No, it isn't (but it MAY be ok for real trivial ones).

Untangling the maze, renaming the common functions and fixing the
modularization for them is the way to go, IMO.


Definitely agree here, 110%


BTW, the "new" module we're recommending might better be called 
compat_utils rather than compat_common.




+--+--++
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:  |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee dot com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd dot org |
+--+--++

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

[Martin Husemann]

On Tue, Aug 15, 2017 at 04:33:19PM +0200, Maxime Villard wrote:
> So we agree? Each compat should be independent.

Yes.

> It seems to me that
> re-implementing (copy-paste) a few functions for linux is a step towards
> direction, isn't it?

No, it isn't (but it MAY be ok for real trivial ones).

Untangling the maze, renaming the common functions and fixing the 
modularization for them is the way to go, IMO.

Martin

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

[Maxime Villard]

Le 15/08/2017 à 14:50, Martin Husemann a écrit :

On Tue, Aug 15, 2017 at 02:48:39PM +0200, Maxime Villard wrote:

Why is it a bad idea re-implement the few compat_xx functions used in
compat_linux? This would eliminate the dependency, and a single modload
would suffice.


Move them into a common module required by all current consumers.


This module already exists, and it's modules/compat. The problem, again,
is that this module will register new syscalls, while we only want the
functions to be available. And it's more than that: if dynamically loaded,
this module may conflict with the kernel, since several COMPAT_xx options
are enabled in GENERIC by default. So you get the same functions twice.

Maxime

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

[Martin Husemann]

On Tue, Aug 15, 2017 at 02:48:39PM +0200, Maxime Villard wrote:
> Why is it a bad idea re-implement the few compat_xx functions used in
> compat_linux? This would eliminate the dependency, and a single modload
> would suffice.

Move them into a common module required by all current consumers.

Martin

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

[Maxime Villard]

Le 04/08/2017 à 10:00, matthew green a écrit :

the setup comes from before modules and it allows code sharing
because the old 43 version matches other systems.  so there's
a single implementation of this code for a large number of
consumers, and the name of it describes where it comes from.
this is entirely as designed and desired.


Thinking about this again; this may have been desired, but it's clearly
a wrong design - and it also was with config(5), regardless of the modules.

The user finds himself in a situation where if he wants to use a linux
binary, he has to enable a whole bunch of unrelated compat options, just
because our compat_linux borrows one function here, and another there, etc.

While I agree that deduplicating code is a good thing in general, it
certainly is not when the subsystems that share the code are this distant,
and when these subsystems end up being less user-friendly.

Why is it a bad idea re-implement the few compat_xx functions used in
compat_linux? This would eliminate the dependency, and a single modload
would suffice.

Maxime

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

[Maxime Villard]

Le 04/08/2017 à 10:00, matthew green a écrit :

Maxime Villard writes:

Yes, I saw that too a few days later when moving the compat_freebsd files and
trying to do a modload. I went "what the hell is this", but didn't do anything.

What I could see, is that many of our compat options are at some point using
at least one compat_43_* function, even if they have nothing to do with 4.3BSD.

It appears that people just chose to use them, because they were convenient,
instead of re-writing them on purpose. The assumption was that the compat_43
functions would always be there; that's obviously a wrong assumption.

As a quick fix, I can revert my change. But that does not fix this mess; we
would have to either rename compat_43_* to compat_common_* and compile them by
default (ie, without depending on an #ifdef COMPAT_43), or we would have to
implement the functions in all of the compat options with proper names (eg
compat_43_sys_lseek -> freebsd_sys_lseek).


please revert your change (for now?).  thanks.


Alright, I'll do that for now.


Frankly, this is a huge mess.


i don't agree.

the setup comes from before modules and it allows code sharing
because the old 43 version matches other systems. so there's
a single implementation of this code for a large number of
consumers, and the name of it describes where it comes from.


Yes, but the fact that the availability of the compat_43_* functions depends
on #ifdef COMPAT_43, while they are just common functions used in a lot of
places, is wrong. And it has nothing to do with the modules, it was already
wrong before they were introduced (even if it used to be harmless).

As a third (and simpler) solution, we could make sure that these functions are
compiled regardless of COMPAT_43, without renaming them.

Maxime

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

[matthew green]

Maxime Villard writes:
> Yes, I saw that too a few days later when moving the compat_freebsd files and
> trying to do a modload. I went "what the hell is this", but didn't do 
> anything.
> 
> What I could see, is that many of our compat options are at some point using
> at least one compat_43_* function, even if they have nothing to do with 
> 4.3BSD.
> 
> It appears that people just chose to use them, because they were convenient,
> instead of re-writing them on purpose. The assumption was that the compat_43
> functions would always be there; that's obviously a wrong assumption.
> 
> As a quick fix, I can revert my change. But that does not fix this mess; we
> would have to either rename compat_43_* to compat_common_* and compile them by
> default (ie, without depending on an #ifdef COMPAT_43), or we would have to
> implement the functions in all of the compat options with proper names (eg
> compat_43_sys_lseek -> freebsd_sys_lseek).

please revert your change (for now?).  thanks.

> Frankly, this is a huge mess.

i don't agree.

the setup comes from before modules and it allows code sharing
because the old 43 version matches other systems.  so there's
a single implementation of this code for a large number of 
consumers, and the name of it describes where it comes from.
this is entirely as designed and desired.

normal kernels handle this fine within the config(5) framework.

that modules don't know how to do this properly yet is not the
existing code's problem -- that's just other of the issues that
modules have that are why they're not yet an acceptible way to
replace functionality.


.mrg.

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

[Maxime Villard]

Le 03/08/2017 à 23:32, co...@sdf.org a écrit :

On Fri, Jul 28, 2017 at 04:10:29PM +, Maxime Villard wrote:

Module Name:src
Committed By:   maxv
Date:   Fri Jul 28 16:10:29 UTC 2017

Modified Files:
src/sys/arch/amd64/conf: GENERIC XEN3_DOM0 XEN3_DOMU

Log Message:
After a careful review, and all things considered, disable compat43 by
default on amd64. The use case is limited, the potential for damage too
high, and it is safer to run a BSD4.3 binary on i386 since the kernel does
not have to go through netbsd32 - which may not correctly reproduce i386.


To generate a diff of this commit:
cvs rdiff -u -r1.461 -r1.462 src/sys/arch/amd64/conf/GENERIC
cvs rdiff -u -r1.136 -r1.137 src/sys/arch/amd64/conf/XEN3_DOM0
cvs rdiff -u -r1.75 -r1.76 src/sys/arch/amd64/conf/XEN3_DOMU


This breaks running compat_linux as a kernel module:

Aug  4 00:19:19 loggy /netbsd: kobj_checksyms, 974: [compat_linux]: linker 
error: symbol `compat_43_sys_sethostname' not found
Aug  4 00:19:19 loggy /netbsd: kobj_checksyms, 974: [compat_linux]: linker 
error: symbol `compat_43_sys_lseek' not found
Aug  4 00:19:19 loggy /netbsd: WARNING: module error: unable to affix module 
`compat_linux', error 8

I believe paulg made PR kern/51597 for similar issues


Yes, I saw that too a few days later when moving the compat_freebsd files and
trying to do a modload. I went "what the hell is this", but didn't do anything.

What I could see, is that many of our compat options are at some point using
at least one compat_43_* function, even if they have nothing to do with 4.3BSD.

It appears that people just chose to use them, because they were convenient,
instead of re-writing them on purpose. The assumption was that the compat_43
functions would always be there; that's obviously a wrong assumption.

As a quick fix, I can revert my change. But that does not fix this mess; we
would have to either rename compat_43_* to compat_common_* and compile them by
default (ie, without depending on an #ifdef COMPAT_43), or we would have to
implement the functions in all of the compat options with proper names (eg
compat_43_sys_lseek -> freebsd_sys_lseek).

Frankly, this is a huge mess.

Maxime

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

[coypu]

paulg adds:

This is not making GENERIC fail because COMPAT_43 is not really removed
on it.

$ nm /home/fly/amd/sys/arch/amd64/compile/GENERIC/netbsd |grep compat_43
80403485 T compat_43_netbsd32_fstat43
8040371a T compat_43_netbsd32_killpg
80403431 T compat_43_netbsd32_lstat43
804037b5 T compat_43_netbsd32_oaccept
...

This is from:
sys/compat/netbsd32/files.netbsd32:

filecompat/netbsd32/netbsd32_compat_43.ccompat_netbsd32 & (compat_43 | 
compat_sunos | compat_linux32)

So with COMPAT_LINUX32, it still builds the COMPAT_43 code.

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

[Paul Goyette]

On Fri, 28 Jul 2017, Maxime Villard wrote:


Module Name:src
Committed By:   maxv
Date:   Fri Jul 28 16:10:29 UTC 2017

Modified Files:
src/sys/arch/amd64/conf: GENERIC XEN3_DOM0 XEN3_DOMU

Log Message:
After a careful review, and all things considered, disable compat43 by
default on amd64. The use case is limited, the potential for damage too
high, and it is safer to run a BSD4.3 binary on i386 since the kernel does
not have to go through netbsd32 - which may not correctly reproduce i386.


Should COMPAT43 also be removed from the default build of compat module?

See line #26 in src/sys/modules/netbsd/Makefile



+--+--++
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:  |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee dot com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd dot org |
+--+--++

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

[Maxime Villard]

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

[coypu]

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

[David Holland]

On Sat, Nov 26, 2016 at 09:21:17PM +1100, matthew green wrote:
 > > Modified Files:
 > >src/sys/arch/amd64/conf: GENERIC
 > > 
 > > Log Message:
 > > 1.6 was the first amd64 release. (although it didn't really work too well
 > > before -5)
 > 
 > dunno what you're talking about.  i ran it for many years before -5.

If you like :-) I'm just going by what I saw in gnats at the time.

-- 
David A. Holland
dholl...@netbsd.org

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

[matthew green]

"David A. Holland" writes:
> Module Name:  src
> Committed By: dholland
> Date: Sat Nov 26 01:09:33 UTC 2016
> 
> Modified Files:
>   src/sys/arch/amd64/conf: GENERIC
> 
> Log Message:
> 1.6 was the first amd64 release. (although it didn't really work too well
> before -5)

dunno what you're talking about.  i ran it for many years before -5.


.mrg.

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

[Christos Zoulas]

will do.

christos