Re: [PATCH 2/5] I2O: SPARC fixes

[Markus Lidel <Markus.Lidel@shadowconnect.com>]

Hello,

Al Viro wrote:
> On Sun, Nov 20, 2005 at 11:07:14PM +0000, Al Viro wrote:
>>	And here's another fun one:
>>                evt->size = size;
>>                evt->tcntxt = le32_to_cpu(msg->u.s.tcntxt);
>>                evt->event_indicator = le32_to_cpu(msg->body[0]);
>>                memcpy(&evt->tcntxt, &msg->u.s.tcntxt, size * 4);
>>in i2o_driver_dispatch().
> Gaaack...  Old code used to be
>                        evt = kmalloc(size * 4 + sizeof(*evt), GFP_ATOMIC);
>                        if (!evt)
>                                return -ENOMEM;
>                        memset(evt, 0, size * 4 + sizeof(*evt));
> 
>                        evt->size = size;
>                        memcpy_fromio(&evt->tcntxt, &msg->u.s.tcntxt,
>                                     (size + 2) * 4);
> Then it became
>                evt->size = size;
>                evt->tcntxt = readl(&msg->u.s.tcntxt);
>                evt->event_indicator = readl(&msg->body[0]);
>                memcpy_fromio(&evt->tcntxt, &msg->u.s.tcntxt, size * 4);
> See the problem with it?  The last copy should be from &msg->body[1] to
> evt->data.  As it is, we do not copy the last 8 bytes (which might or
> might not be a problem) *AND* we overwrite tcntxt and event_indicator

At the moment it's not a problem, because the event system is not used...

> with bus-endian values right after having host-endian ones carefully
> assigned to them.

Yep, you're right...  the memcpy_fromio is wrong... It should be:

memcpy_fromio(&evt->body[1], &msg->body[1], size * 4);

as you already mentioned...

Thanks for your help.


Best regards,


Markus Lidel
------------------------------------------
Markus Lidel (Senior IT Consultant)

Shadow Connect GmbH
Carl-Reisch-Weg 12
D-86381 Krumbach
Germany

Phone:  +49 82 82/99 51-0
Fax:    +49 82 82/99 51-11

E-Mail: Markus.Lidel@shadowconnect.com
URL:    http://www.shadowconnect.com