RE: Obvious bug in /sys/i386/include/bus.h (was: bus_at386.h)
No, it's a correct method to set/reset the zero flag: (X | X) == X just as (X X) == X Yes but it stores result in ecx(using or, really, is not a problem)... however jecxz just is 2 bytes sized while or + jmp is 4 bytes. It means lesser FDE latency time and the pseudocode is enough similar. greeting, rookie ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: Obvious bug in /sys/i386/include/bus.h (was: bus_at386.h)
Norbert Koch wrote: So how can I fix this in assembly. I am not an expert with inlined assembly, so maybe someone can correct me if I am wrong, but something like this needs to be added: or %ecx, %ecx jz 2 2: This is wrong beacause the result is stored in ecx. Better using JECXZ instruction before the loop. No, it's a correct method to set/reset the zero flag: (X | X) == X just as (X X) == X So, he could also write: and %ecx, %ecx. [...] Also, on pipelined processors like Intel Pentium and better you would use test %ecx, %ecx for this purpose which internally is an and, but since it drops the result and just sets the condition register it is faster than and or or. At least potentially. Depends on the surrounding code. Uwe -- Uwe Doering | EscapeBox - Managed On-Demand UNIX Servers [EMAIL PROTECTED] | http://www.escapebox.net ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to [EMAIL PROTECTED]
RE: Obvious bug in /sys/i386/include/bus.h (was: bus_at386.h)
http://www.freebsd.org/cgi/query-pr.cgi?pr=80980
In FreeBSD 6-current the code for bus_space_write_multi_1() says:
__asm __volatile( \n\
cld \n\
1: lodsb \n\
movb %%al,(%2) \n\
loop 1b:
=S (addr), =c (count) :
r (bsh + offset), 0 (addr), 1 (count) :
%eax, memory, cc);
This is equivalent to:
while(--count)
{
/* I/O */
}
which is obviously wrong, because it doesn't check for count equal to zero.
So
how can I fix this in assembly. I am not an expert with inlined assembly,
so
maybe someone can correct me if I am wrong, but something like this needs
to
be added:
or %ecx, %ecx
jz 2
2:
This is wrong beacause the result is stored in ecx. Better using JECXZ
instruction
before the loop.
Greeting,
rookie
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: Obvious bug in /sys/i386/include/bus.h (was: bus_at386.h)
On Mon, Jun 13, 2005 at 02:12:38PM +0200, Hans Petter Selasky wrote:
This is equivalent to:
while(--count)
{
/* I/O */
}
which is obviously wrong, because it doesn't check for count equal to
zero.
Why do you think it is a bug? It is part of the interface contract and
useful to avoid an unnecessary check in 99% of the cases.
Joerg
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]
RE: Obvious bug in /sys/i386/include/bus.h (was: bus_at386.h)
So how can I fix this in assembly. I am not an expert with inlined assembly, so maybe someone can correct me if I am wrong, but something like this needs to be added: or %ecx, %ecx jz 2 2: This is wrong beacause the result is stored in ecx. Better using JECXZ instruction before the loop. Greeting, rookie No, it's a correct method to set/reset the zero flag: (X | X) == X just as (X X) == X So, he could also write: and %ecx, %ecx. I may be wrong, but in the old 386/486 days the jecxz was even less efficient, wasn't it? history Twenty years ago, my z80 programs had a lot of lines like and a ret z Weren't there discussions, if an nmos cpu consumed more electric power with either and a or or a? ;-) /history Norbert ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: Obvious bug in /sys/i386/include/bus.h (was: bus_at386.h)
On Monday 13 June 2005 14:44, Joerg Sonnenberger wrote:
On Mon, Jun 13, 2005 at 02:12:38PM +0200, Hans Petter Selasky wrote:
This is equivalent to:
while(--count)
{
/* I/O */
}
which is obviously wrong, because it doesn't check for count equal to
zero.
Why do you think it is a bug? It is part of the interface contract and
useful to avoid an unnecessary check in 99% of the cases.
Where is it documented?
This is a bug, because it will break standard FIFO design. Consider the
following pseudo code:
static void
filter(struct fifo *f)
{
do {
if(!f-len)
{
if(f-m) ...;
f-m = get_mbuf();
if(!f-m) break;
f-len = m-m_len;
f-ptr = m-m_data;
}
/* f-Z_chip is the maximum transfer length */
io_len = min(f-len, f-Z_chip);
bus_space_write_multi_1(t,h,xxx,f-ptr,io_len);
f-len -= io_len;
f-Z_chip -= io_len;
f-ptr += io_len;
} while(Z_chip);
}
This code is going to crash if f-Z_chip or f-len is zero. That happens
when one is trying to send or receive a zero length frame or mbuf. So then
one can argue wether one should allow zero length frames or not. I argue that
zero length frames should be allowed, because it enables easy stream
synchronization:
For example to signal end of stream, send a frame less than 16 bytes. Maximum
frame length is 16 bytes. If a stream is exactly 16 bytes long, then one has
to send one 16 byte frame and one 0 byte frame.
Also zero length frames can be used as a kind of heart-beat.
Adding that extra check for zero transfer length is not going to affect
performance at all. If one does that using C, the compiler can optimize
away that if(count) ... when count is a constant. Besides the i386
machine instructions ins and outs already exhibit that behaviour.
--HPS
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: Obvious bug in /sys/i386/include/bus.h (was: bus_at386.h)
On Mon, Jun 13, 2005 at 05:58:24PM +0200, Hans Petter Selasky wrote:
static void
filter(struct fifo *f)
{
do {
if(!f-len)
{
if(f-m) ...;
f-m = get_mbuf();
if(!f-m) break;
f-len = m-m_len;
f-ptr = m-m_data;
}
/* f-Z_chip is the maximum transfer length */
io_len = min(f-len, f-Z_chip);
if (io_len == 0)
continue;
bus_space_write_multi_1(t,h,xxx,f-ptr,io_len);
f-len -= io_len;
f-Z_chip -= io_len;
f-ptr += io_len;
} while(Z_chip);
}
[...]
Adding that extra check for zero transfer length is not going to affect
performance at all. If one does that using C, the compiler can optimize
away that if(count) ... when count is a constant. Besides the i386
machine instructions ins and outs already exhibit that behaviour.
The compiler can only optimize it away if it is known to be a constant.
But thinking again about it, it should be documented at least whether
a count of 0 is allowed or not. I think it makes more sense to not allow
it, but others (you included) might disagree.
Joerg
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: Obvious bug in /sys/i386/include/bus.h (was: bus_at386.h)
On Monday 13 June 2005 18:27, Joerg Sonnenberger wrote:
On Mon, Jun 13, 2005 at 05:58:24PM +0200, Hans Petter Selasky wrote:
static void
filter(struct fifo *f)
{
do {
if(!f-len)
{
if(f-m) ...;
f-m = get_mbuf();
if(!f-m) break;
f-len = m-m_len;
f-ptr = m-m_data;
}
/* f-Z_chip is the maximum transfer length */
io_len = min(f-len, f-Z_chip);
if (io_len == 0)
continue;
bus_space_write_multi_1(t,h,xxx,f-ptr,io_len);
f-len -= io_len;
f-Z_chip -= io_len;
f-ptr += io_len;
} while(Z_chip);
}
[...]
Adding that extra check for zero transfer length is not going to affect
performance at all. If one does that using C, the compiler can optimize
away that if(count) ... when count is a constant. Besides the i386
machine instructions ins and outs already exhibit that behaviour.
The compiler can only optimize it away if it is known to be a constant.
But thinking again about it, it should be documented at least whether
a count of 0 is allowed or not. I think it makes more sense to not allow
it, but others (you included) might disagree.
Why?
If a count of zero is not allowed, then bus_space_xxx() should panic on
count equal to zero and not freeze the system, so that the user is able to
find out what is wrong:
#if defined(XXX)
if(count == 0) panic(not allowed);
#endif
But that is just stupid, why not just return:
#if defined(XXX)
if(count == 0) return;
#endif
And then I can put:
#define XXX
in my code before including bus.h. Instead of creating wrappers, just to be
sure:
#define bus_space_read_multi_1(t,h,off,ptr,count) \
{ if(count) bus_space_read_multi_1(t,h,off,ptr,count); }
Because this is really specific to i386 and amd64. If you look at alpha,
sparc64 and ia64 they all use while(count--) So I think the one
that wrote that code did a mistake.
My theory is that he or her copied:
static __inline void
insb(u_int port, void *addr, size_t cnt)
{
__asm __volatile(cld; rep; insb
: +D (addr), +c (cnt)
: d (port)
: memory);
}
And forgot to add that extra check for count equal to zero, when converting
rep into loop!
--HPS
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]
