An u8 was casted into an u32, then all 32 bits were set to zero, this
causing another variable - in my case, processor flags - to be corrupted.
Dominik
--- linux/drivers/acpi-original/ec.c Fri Jul 12 22:43:11 2002
+++ linux/drivers/acpi/ec.c Fri Jul 12 23:28:14 2002
@@ -134,7 +134,7 @@
acpi_ec_read (
struct acpi_ec *ec,
u8 address,
- u8 *data)
+ u32 *data)
{
acpi_status status = AE_OK;
int result = 0;
@@ -167,7 +167,7 @@
goto end;
- acpi_hw_low_level_read(8, (u32*) data, &ec->data_addr, 0);
+ acpi_hw_low_level_read(8, data, &ec->data_addr, 0);
ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Read [%02x] from address [%02x]\n",
*data, address));
@@ -237,7 +237,7 @@
static int
acpi_ec_query (
struct acpi_ec *ec,
- u8 *data)
+ u32 *data)
{
int result = 0;
acpi_status status = AE_OK;
@@ -269,7 +269,7 @@
if (result)
goto end;
- acpi_hw_low_level_read(8, (u32*) data, &ec->data_addr, 0);
+ acpi_hw_low_level_read(8, data, &ec->data_addr, 0);
if (!*data)
result = -ENODATA;
@@ -328,7 +328,7 @@
{
acpi_status status = AE_OK;
struct acpi_ec *ec = (struct acpi_ec *) data;
- u8 value = 0;
+ u32 value = 0;
unsigned long flags = 0;
struct acpi_ec_query_data *query_data = NULL;
@@ -336,7 +336,7 @@
return;
spin_lock_irqsave(&ec->lock, flags);
- acpi_hw_low_level_read(8, (u32*) &value, &ec->command_addr, 0);
+ acpi_hw_low_level_read(8, &value, &ec->command_addr, 0);
spin_unlock_irqrestore(&ec->lock, flags);
/* TBD: Implement asynch events!
@@ -398,6 +398,7 @@
{
int result = 0;
struct acpi_ec *ec = NULL;
+ u32 tmp = 0;
ACPI_FUNCTION_TRACE("acpi_ec_space_handler");
@@ -408,7 +409,8 @@
switch (function) {
case ACPI_READ:
- result = acpi_ec_read(ec, (u8) address, (u8*) value);
+ result = acpi_ec_read(ec, (u8) address, &tmp);
+ *value = (acpi_integer) tmp;
break;
case ACPI_WRITE:
result = acpi_ec_write(ec, (u8) address, (u8) *value);
On Thu, 18 Jul 2002, Dominik Brodowski wrote:
> An u8 was casted into an u32, then all 32 bits were set to zero, this
> causing another variable - in my case, processor flags - to be corrupted.
>
> Dominik
But that's not what's happening here, the pointer is being cast, if the
object of the pointer is not u32, then casting the pointer doesn't fix the
real problem. If the "data" pointer points to a u8, then no casting will
make it work right when you save 32 bits into an 8 bit space. If this
changes the problem it's because of alignment, perhaps.
You give neither the kernel version nor the architecture, so I can't be
sure what's happening or what the compiler might do. I don't find that
code in the kernel I have on this machine (2.4.19-pre7-jam6) but that
diesn't mean much. The routine in hardware/hwregs.c on my kernel would
seem to pass the width correctly, but clearly this is a very different
version.
I think the cast is just to avoid the compiler whining about types, the
version I have is actually type "(acpi_generic_address*)" not (u32*), I
would think the compiler would still complain, but maybe only with
-pedantic or whatever.
In any case only the number of bits requested should be written, the
problem may have been avoided rather than fixed.
> --- linux/drivers/acpi-original/ec.c Fri Jul 12 22:43:11 2002
> +++ linux/drivers/acpi/ec.c Fri Jul 12 23:28:14 2002
> @@ -134,7 +134,7 @@
> acpi_ec_read (
> struct acpi_ec *ec,
> u8 address,
> - u8 *data)
> + u32 *data)
> {
> acpi_status status = AE_OK;
> int result = 0;
> @@ -167,7 +167,7 @@
> goto end;
>
>
> - acpi_hw_low_level_read(8, (u32*) data, &ec->data_addr, 0);
> + acpi_hw_low_level_read(8, data, &ec->data_addr, 0);
>
> ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Read [%02x] from address [%02x]\n",
> *data, address));
--
bill davidsen <[email protected]>
CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.
On Tue, Jul 23, 2002 at 05:20:57PM -0400, Bill Davidsen wrote:
> On Thu, 18 Jul 2002, Dominik Brodowski wrote:
>
> > An u8 was casted into an u32, then all 32 bits were set to zero, this
> > causing another variable - in my case, processor flags - to be corrupted.
> >
> > Dominik
>
> But that's not what's happening here, the pointer is being cast, if the
> object of the pointer is not u32, then casting the pointer doesn't fix the
> real problem. If the "data" pointer points to a u8, then no casting will
> make it work right when you save 32 bits into an 8 bit space. If this
> changes the problem it's because of alignment, perhaps.
There is the argument "size" to acpi_hw_low_level_read which makes sure that
only the right data is being read. Problem is that a sort of
"segmentation fault" occurs when such a cast allows writing in different
memory than allocated for the value.
> You give neither the kernel version nor the architecture
kernel version 2.5.2[5,6] or 2.4.-kernels with acpi-patch 20020709,
architecture: all architectures that implement Embedded Controllers.
> I think the cast is just to avoid the compiler whining about types, the
> version I have is actually type "(acpi_generic_address*)" not (u32*), I
> would think the compiler would still complain, but maybe only with
> -pedantic or whatever.
The casts were probably introduced for that reason. Per se, they are not
critical - but if there is any assumption later on that the data structure
is indeed of the large size, there is a problem.
Dominik
> You give neither the kernel version nor the architecture, so I can't be
> sure what's happening or what the compiler might do. I don't find that
> code in the kernel I have on this machine (2.4.19-pre7-jam6) but that
> diesn't mean much. The routine in hardware/hwregs.c on my kernel would
> seem to pass the width correctly, but clearly this is a very different
> version.
Actually, I found this patch accidently since it didn't list the kernel
version but it exactly fixed my ACPI oops I was getting on boots of the
2.5.2[6-7] kernels. I posted the relevent oops about a week ago under the
subject "2.5.2[6-7] ACPI oops" but I can send it to you if you're interested
--
Matthew Harrell Any sufficiently advanced technology
Bit Twiddlers, Inc. is indistinguishable from a rigged
[email protected] demo.
On Wed, 24 Jul 2002, Dominik Brodowski wrote:
> The casts were probably introduced for that reason. Per se, they are not
> critical - but if there is any assumption later on that the data structure
> is indeed of the large size, there is a problem.
Perhaps the pointer chould be (void *) everywhere, and then cast to the
right size where and when it is used. Clearly casting the pointer to a
larger size will result in allignment issues on some machines.
--
bill davidsen <[email protected]>
CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.