RE: [ion] 19742: - Implement RtlPrefectMemoryNonTemporal.Patch by Patrick Baggett <[e-mail]> and myself.

[Ge van Geldorp]

> From: [e-mail]
>
> +@RtlPrefetchMemoryNonTemporal@8:
> ret         /* Overwritten by ntoskrnl/ke/i386/kernel.c if SSE is

supported (see Ki386SetProcessorFeatures() ) */

Is this routine so time-critical that we want to resort to self-modifying
code?

GvG

[Alex Ionescu]

Ge van Geldorp wrote:
> > From: [e-mail]
> >
> > +@RtlPrefetchMemoryNonTemporal@8:
> > ret         /* Overwritten by ntoskrnl/ke/i386/kernel.c if SSE is
>
> supported (see Ki386SetProcessorFeatures() ) */
>
> Is this routine so time-critical that we want to resort to self-modifying
> code?

Yup, and there are many more cases where NT does this:
- Syscall vs INT2e
- Cmpxch8 vs cmpxchg x2 + spinlock
- XMMI Page Zeroing vs regular Page Zeroing
- Prefetching

And I'm probably missing more. There are critical APIs that would be 
very slow should the check be done each time they are called.

Best regards,
Alex Ionescu
> GvG

[Ge van Geldorp]

> From: Alex Ionescu
>
> Ge van Geldorp wrote:
> > Is this routine so time-critical that we want to resort to 
> > self-modifying code?
>
> Yup, and there are many more cases where NT does this:
> Syscall vs INT2e
> Cmpxch8 vs cmpxchg x2 + spinlock
> XMMI Page Zeroing vs regular Page Zeroing
> Prefetching
>
> And I'm probably missing more. There are critical APIs that 
> would be very slow should the check be done each time they are called.

I'm not arguing that we should ban self-modifying code in every case (and I
certainly don't intend to start a vote on it ;-)), the SYSCALL v. INT2E case
is a prime example of where it is useful. However, I was talking about "THIS
routine". Frankly, I don't buy that a routine that is not even called right
now (it was UNIMPLEMENTED()) is so time-critical that we need this. It kills
ROMability of the code, effectively shutting us out of the embedded market
that some people had rather high hopes for. The fact that NT does this is of
little concern, we only need to be compatible, not identical.

GvG

[James Tabor]

Ge van Geldorp wrote:
> I'm not arguing that we should ban self-modifying code in every case (and I
> certainly don't intend to start a vote on it ;-))

Did I read the word "VOTE"!

I agree we should vote to ban voting and people who like to modify them selves.

Thanks,
James

[Ged Murphy]

James Tabor wrote:
> Ge van Geldorp wrote:
> > I'm not arguing that we should ban self-modifying code in every case 
> > (and I
> > certainly don't intend to start a vote on it ;-))
>
> I agree we should vote to ban voting and people who like to modify 
> them selves.

I disagree !! 
I think we should vote on it !!

[Alex Ionescu]

Ge van Geldorp wrote:
> > From: Alex Ionescu
> >
> > Ge van Geldorp wrote:
> > > Is this routine so time-critical that we want to resort to 
> > > self-modifying code?
> >
> > Yup, and there are many more cases where NT does this:
> > Syscall vs INT2e
> > Cmpxch8 vs cmpxchg x2 + spinlock
> > XMMI Page Zeroing vs regular Page Zeroing
> > Prefetching
> >
> > And I'm probably missing more. There are critical APIs that 
> > would be very slow should the check be done each time they are called.
>
> I'm not arguing that we should ban self-modifying code in every case (and I
> certainly don't intend to start a vote on it ;-)), the SYSCALL v. INT2E case
> is a prime example of where it is useful. However, I was talking about "THIS
> routine". Frankly, I don't buy that a routine that is not even called right
> now (it was UNIMPLEMENTED()) is so time-critical that we need this. It kills
> ROMability of the code, effectively shutting us out of the embedded market
> that some people had rather high hopes for.

This is a very good point, but I think there are much bigger problems 
then this to get NT loading on a ROM. And a machine with a ROM wouldn't 
support this function in either case, so it's a moot point in the end.

This routine is pretty new and that's why it's not called, but from what 
I've read on SSE, prefetching is supposed to be an ultra-time critical 
operation and the API can be called recursively a large number of times. 
It was my opinion that maximing the speed was a good choice. When I 
studied the routine under Windows and I saw that a team of over 2 500 
hardware and software experts with access to gigantic user bases, 
profiling equipment and connections with developers and Intel made the 
same decision, it was gratifying. (To be honest, my original idea was to 
use "nop" and overwrite it with "ret", but both Patrick (who wrote the 
patch) and MS were right in making it a "ret" by default and using "nop" 
to overwrite, since that's even faster).
> The fact that NT does this is of
> little concern, we only need to be compatible, not identical.

It was of concern to me not because of compatibility, but because it 
shows that a team with so many resources agreed with my implementation 
(although, like I said before, in an even more optimized way).

To see an example where I disagree with MS (as proof that I'm not 
obessesed with copying NT) on a similar self-modifying code issue, take 
cmpxch8. I think that the code which MS uses to overwrite entire chunks 
of the functions with new assembly code is horribly wrong. I would 
implement both functions and then use a function pointer to chose one.

Best regards,
Alex Ionescu
> GvG
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

[Thomas Weidenmueller]

Alex Ionescu wrote:
> (To be honest, my original idea was to
> use "nop" and overwrite it with "ret", but both Patrick (who wrote the
> patch) and MS were right in making it a "ret" by default and using "nop"
> to overwrite, since that's even faster).

I don't understand how a modified nop to a ret can be faster than a ret
generated at compile time, or vice versa. However, having a ret by
default makes more sense ;)

- Thomas

[Alex Ionescu]

Thomas Weidenmueller wrote:
> Alex Ionescu wrote:
> > (To be honest, my original idea was to
> > use "nop" and overwrite it with "ret", but both Patrick (who wrote the
> > patch) and MS were right in making it a "ret" by default and using "nop"
> > to overwrite, since that's even faster).
>
> I don't understand how a modified nop to a ret can be faster than a ret
> generated at compile time, or vice versa. However, having a ret by
> default makes more sense ;)
> Thomas

Simple:

If you have a "nop" by default, then your job is as follows:
- Detect support for the instruction
- If it's NOT supported, overwrite with ret.
Additionnally, the default state of the function is to WORK, which 
fundamentally speaking is wrong because you're assuming that the 
advanced functionality exists, and then you're disabling it. Since 
you're trying to be safe, then the whole point is to protect from the 
potential BSOD.
However, with a "ret" by default, you only overwrite with nop if the 
instruction IS supported. Apart from the design issue itself, I think 
the premise here is that more CPUs don't have the support for the 
function, then the ones that do.

Best regards,
Alex Ionescu

[Thomas Weidenmueller]

Alex Ionescu wrote:
> If you have a "nop" by default, then your job is as follows:
> Detect support for the instruction
> If it's NOT supported, overwrite with ret.
>
> Additionnally, the default state of the function is to WORK, which
> fundamentally speaking is wrong because you're assuming that the
> advanced functionality exists, and then you're disabling it. Since
> you're trying to be safe, then the whole point is to protect from the
> potential BSOD.
> However, with a "ret" by default, you only overwrite with nop if the
> instruction IS supported. Apart from the design issue itself, I think
> the premise here is that more CPUs don't have the support for the
> function, then the ones that do.

That depends on whether the machine supports it or not, it's not a
general optimization. But you made it sound like one ;)

- Thomas

[Ge van Geldorp]

> From: Alex Ionescu
>
> This is a very good point, but I think there are much bigger 
> problems then this to get NT loading on a ROM.

So that's an argument to introduce code which brings us further from the
goal?
> This routine is pretty new and that's why it's not called, 
> but from what I've read on SSE, prefetching is supposed to be 
> an ultra-time critical operation

Then inside the kernel it should be inlined, saves a CALL and RET. For use
by drivers it should be declared as __attribute__((dllimport)) in the
headers, saves a JMP. That is, if I buy the "ultra-time critical operation"
which I still do not, without any timing evidence.
> When I studied the routine under Windows and I saw that a 
> team of over 2 500 hardware and software experts with access 
> to gigantic user bases, profiling equipment and connections 
> with developers and Intel made the same decision, it was 
> gratifying.

How can we claim that ReactOS is an independent implementation when we start
to use "well, I reverse engineered Windows and Microsoft does it the same
way" as a implementation decision rationale? For that matter, why spent time
on ReactOS at all? Obviously, Microsoft with its huge teams knows better
anyway, so we should just stick to Windows.

GvG

[Steven Edwards]

Hi Ge,

On 11/30/05, Ge van Geldorp [e-mail] wrote:
> How can we claim that ReactOS is an independent implementation when we start
> to use "well, I reverse engineered Windows and Microsoft does it the same
> way" as a implementation decision rationale? For that matter, why spent time
> on ReactOS at all? Obviously, Microsoft with its huge teams knows better
> anyway, so we should just stick to Windows.

You are correct. We cannot claim ReactOS is a independent
implementation if this is the rationale. I think the only solution is
to call for a vote on the methods we use to gather information about
Windows behavior.

--
Steven Edwards - ReactOS and Wine developer

"There is one thing stronger than all the armies in the world, and
that is an idea whose time has come." - Victor Hugo

[Steven Edwards]

Hi,

On 11/30/05, Steven Edwards [e-mail] wrote:
> You are correct. We cannot claim ReactOS is a independent
> implementation if this is the rationale. I think the only solution is
> to call for a vote on the methods we use to gather information about
> Windows behavior.

Sorry I typed in a bit of a rush. I think we can claim ReactOS is a
independent work if this information cannot be gained via any other
method than reverse engineering and disassembly of object code. We
have a IP document that while never voted on was not objected to by
anyone.

http://www.reactos.org/xhtml/en/dev_legalreview.html

I think we should vote on it and I propose we amend it to state
something like the following:

"Reverse engineering and Disassembly of object code are allowed only
after all attempts to gather information via independent test cases
have failed."

So the vote would be something like this

Option 1 - Leave the IP document as it stands
Option 2 - Accept the IP document with the above amendment
Option 3 - Discuss and edit other areas of the document

--
Steven Edwards - ReactOS and Wine developer

"There is one thing stronger than all the armies in the world, and
that is an idea whose time has come." - Victor Hugo

[Ge van Geldorp]

> From: Steven Edwards
>
> Sorry I typed in a bit of a rush. I think we can claim 
> ReactOS is a independent work if this information cannot be 
> gained via any other method than reverse engineering and 
> disassembly of object code. We have a IP document that while 
> never voted on was not objected to by anyone.
>
> http://www.reactos.org/xhtml/en/dev_legalreview.html
>
> I think we should vote on it and I propose we amend it to 
> state something like the following:
>
> "Reverse engineering and Disassembly of object code are 
> allowed only after all attempts to gather information via 
> independent test cases have failed."

I don't have a problem with reverse engineering per se, but what we fail to
do at the moment is use the information obtained from that in a clean room
manner. I.e. at the moment the same person that looks at the object code
also provides the ReactOS implementation. I think that's dangerous, I
believe most (court) cases where reverse engineering was found acceptable
were clean room cases. So I'd like to amend the IP policy with something
like:

If there is no other option than to reverse engineer/disassemble object
code, the person who reverse engineered the code shall put his findings in a
document describing the interface. That person can not implement the
corresponding code in ReactOS, implementation shall be done by a different
person (or group of persons), working only from the interface description.

We can then keep the document in svn so we can present an audit trail if
asked to produce one.

GvG

[Royce Mitchell III]

Ge van Geldorp wrote:
> I don't have a problem with reverse engineering per se, but what we fail to
> do at the moment is use the information obtained from that in a clean room
> manner. I.e. at the moment the same person that looks at the object code
> also provides the ReactOS implementation. I think that's dangerous, I
> believe most (court) cases where reverse engineering was found acceptable
> were clean room cases. So I'd like to amend the IP policy with something
> like:
>
> If there is no other option than to reverse engineer/disassemble object
> code, the person who reverse engineered the code shall put his findings in a
> document describing the interface. That person can not implement the
> corresponding code in ReactOS, implementation shall be done by a different
> person (or group of persons), working only from the interface description.
>
> We can then keep the document in svn so we can present an audit trail if
> asked to produce one.
>
> GvG

+1

[Steven Edwards]

Hi,

On 12/1/05, Ge van Geldorp [e-mail] wrote:
> I don't have a problem with reverse engineering per se, but what we fail to
> do at the moment is use the information obtained from that in a clean room
> manner. I.e. at the moment the same person that looks at the object code
> also provides the ReactOS implementation. I think that's dangerous, I
> believe most (court) cases where reverse engineering was found acceptable
> were clean room cases. So I'd like to amend the IP policy with something
> like:
>
> If there is no other option than to reverse engineer/disassemble object
> code, the person who reverse engineered the code shall put his findings in a
> document describing the interface. That person can not implement the
> corresponding code in ReactOS, implementation shall be done by a different
> person (or group of persons), working only from the interface description.
>
> We can then keep the document in svn so we can present an audit trail if
> asked to produce one.

Ok can we start a vote on this? We also need to hold a vote for
accepting the IP policy so I still think we should have something like

[] - Accept the IP Policy as it stands
[] - Accept the IP Policy with Amendment
[] - Discuss IP Policy further

If no choice gets more than 50% of a vote then we should remove option
that got the lowest number of votes.

--
Steven Edwards - ReactOS and Wine developer

"There is one thing stronger than all the armies in the world, and
that is an idea whose time has come." - Victor Hugo

[James Tabor]

Steven Edwards wrote:
> Ok can we start a vote on this? We also need to hold a vote for
> accepting the IP policy so I still think we should have something like
>
> [] - Accept the IP Policy as it stands
> [] - Accept the IP Policy with Amendment
> [X] - Discuss IP Policy further

I want to talk more about it, okay? I need to see something from lawyers about
both or more IP policies. US law and European law too. Let's keep it open before
a vote.

Thanks,
James

[Brandon Turner]

I need to get more info...

Brandon

Steven Edwards wrote:
> Hi,
>
> On 12/1/05, Ge van Geldorp [e-mail] wrote:
> > <snip>
>
> Ok can we start a vote on this? We also need to hold a vote for
> accepting the IP policy so I still think we should have something like
>
> [] - Accept the IP Policy as it stands
> [] - Accept the IP Policy with Amendment
> [X] - Discuss IP Policy further
>
> If no choice gets more than 50% of a vote then we should remove option
> that got the lowest number of votes.
>
> --
> Steven Edwards - ReactOS and Wine developer
>
> "There is one thing stronger than all the armies in the world, and
> that is an idea whose time has come." - Victor Hugo
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev