All of lore.kernel.org
 help / color / mirror / Atom feed
* [Devel] [patch 4/5] Fix aligment issues
@ 2010-07-05 12:03 malattia
  0 siblings, 0 replies; 4+ messages in thread
From: malattia @ 2010-07-05 12:03 UTC (permalink / raw)
  To: devel

[-- Attachment #1: Type: text/plain, Size: 5778 bytes --]

Add more platforms to the list of the ones requiring aligned memory access. Also
fix callsites where wrong assumptions where made in terms of aligment.

Signed-off-by: Mattia Dongili <malattia(a)linux.it>
---
 compiler/asltree.c  |   15 ++++++++++-----
 executer/exoparg2.c |   12 +++++++++---
 include/actypes.h   |   26 +++++++++++++-------------
 3 files changed, 32 insertions(+), 21 deletions(-)

Index: b/include/actypes.h
===================================================================
--- a/include/actypes.h	2010-07-02 21:42:26.465350281 +0900
+++ b/include/actypes.h	2010-07-04 11:18:22.506529460 +0900
@@ -205,6 +205,19 @@
 
 /*! [End] no source code translation !*/
 
+/*
+ * In the case of the Itanium Processor Family (IPF), the hardware does not
+ * support misaligned memory transfers. Set the MISALIGNMENT_NOT_SUPPORTED flag
+ * to indicate that special precautions must be taken to avoid alignment faults.
+ * (IA64 or ia64 is currently used by existing compilers to indicate IPF.)
+ *
+ * Note: EM64T and other X86-64 processors support misaligned transfers,
+ * so there is no need to define this flag.
+ */
+#if defined (__IA64__) || defined (__ia64__) | defined(__alpha__) || defined(__sparc__) || defined(__hppa__) || defined(__arm__)
+#define ACPI_MISALIGNMENT_NOT_SUPPORTED
+#endif
+
 
 /*******************************************************************************
  *
@@ -231,19 +244,6 @@
 #define ACPI_SIZE_MAX                   ACPI_UINT64_MAX
 #define ACPI_USE_NATIVE_DIVIDE          /* Has native 64-bit integer support */
 
-/*
- * In the case of the Itanium Processor Family (IPF), the hardware does not
- * support misaligned memory transfers. Set the MISALIGNMENT_NOT_SUPPORTED flag
- * to indicate that special precautions must be taken to avoid alignment faults.
- * (IA64 or ia64 is currently used by existing compilers to indicate IPF.)
- *
- * Note: EM64T and other X86-64 processors support misaligned transfers,
- * so there is no need to define this flag.
- */
-#if defined (__IA64__) || defined (__ia64__)
-#define ACPI_MISALIGNMENT_NOT_SUPPORTED
-#endif
-
 
 /*******************************************************************************
  *
Index: b/executer/exoparg2.c
===================================================================
--- a/executer/exoparg2.c	2010-07-02 21:42:26.477349196 +0900
+++ b/executer/exoparg2.c	2010-07-04 11:18:22.506529460 +0900
@@ -248,6 +248,8 @@
     ACPI_OPERAND_OBJECT     **Operand = &WalkState->Operands[0];
     ACPI_OPERAND_OBJECT     *ReturnDesc1 = NULL;
     ACPI_OPERAND_OBJECT     *ReturnDesc2 = NULL;
+    UINT64                  ReturnValue1 = 0;
+    UINT64                  ReturnValue2 = 0;
     ACPI_STATUS             Status;
 
 
@@ -281,8 +283,10 @@
 
         Status = AcpiUtDivide (Operand[0]->Integer.Value,
                                Operand[1]->Integer.Value,
-                               &ReturnDesc1->Integer.Value,
-                               &ReturnDesc2->Integer.Value);
+                               &ReturnValue1, &ReturnValue2);
+        ReturnDesc1->Integer.Value = ReturnValue1;
+        ReturnDesc2->Integer.Value = ReturnValue2;
+
         if (ACPI_FAILURE (Status))
         {
             goto Cleanup;
@@ -357,6 +361,7 @@
     ACPI_OPERAND_OBJECT     **Operand = &WalkState->Operands[0];
     ACPI_OPERAND_OBJECT     *ReturnDesc = NULL;
     UINT64                  Index;
+    UINT64                  ReturnValue = 0;
     ACPI_STATUS             Status = AE_OK;
     ACPI_SIZE               Length;
 
@@ -400,7 +405,8 @@
         Status = AcpiUtDivide (Operand[0]->Integer.Value,
                                Operand[1]->Integer.Value,
                                NULL,
-                               &ReturnDesc->Integer.Value);
+                               &ReturnValue);
+        ReturnDesc->Integer.Value = ReturnValue;
         break;
 
 
Index: b/compiler/asltree.c
===================================================================
--- a/compiler/asltree.c	2010-07-02 21:42:26.529349893 +0900
+++ b/compiler/asltree.c	2010-07-04 11:18:22.510539385 +0900
@@ -501,24 +501,27 @@
         "\nCreateValuedLeafNode  Ln/Col %u/%u NewNode %p  Op %s  Value %8.8X%8.8X  ",
         Op->Asl.LineNumber, Op->Asl.Column, Op, UtGetOpName(ParseOpcode),
         ACPI_FORMAT_UINT64 (Value));
-    Op->Asl.Value.Integer = Value;
 
     switch (ParseOpcode)
     {
     case PARSEOP_STRING_LITERAL:
-        DbgPrint (ASL_PARSE_OUTPUT, "STRING->%s", Value);
+        Op->Asl.Value.String = (ACPI_STRING) Value;
+        DbgPrint (ASL_PARSE_OUTPUT, "STRING->%s", (ACPI_STRING) Value);
         break;
 
     case PARSEOP_NAMESEG:
-        DbgPrint (ASL_PARSE_OUTPUT, "NAMESEG->%s", Value);
+        Op->Asl.Value.String = (ACPI_STRING) Value;
+        DbgPrint (ASL_PARSE_OUTPUT, "NAMESEG->%s", (ACPI_STRING) Value);
         break;
 
     case PARSEOP_NAMESTRING:
-        DbgPrint (ASL_PARSE_OUTPUT, "NAMESTRING->%s", Value);
+        Op->Asl.Value.String = (ACPI_STRING) Value;
+        DbgPrint (ASL_PARSE_OUTPUT, "NAMESTRING->%s", (ACPI_STRING) Value);
         break;
 
     case PARSEOP_EISAID:
-        DbgPrint (ASL_PARSE_OUTPUT, "EISAID->%s", Value);
+        Op->Asl.Value.String = (ACPI_STRING) Value;
+        DbgPrint (ASL_PARSE_OUTPUT, "EISAID->%s", (ACPI_STRING) Value);
         break;
 
     case PARSEOP_METHOD:
@@ -526,10 +529,12 @@
         break;
 
     case PARSEOP_INTEGER:
+        Op->Asl.Value.Integer = Value;
         DbgPrint (ASL_PARSE_OUTPUT, "INTEGER");
         break;
 
     default:
+        Op->Asl.Value.Integer = Value;
         break;
     }
 


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [Devel] [patch 4/5] Fix aligment issues
@ 2010-07-06  1:19 Lin Ming
  0 siblings, 0 replies; 4+ messages in thread
From: Lin Ming @ 2010-07-06  1:19 UTC (permalink / raw)
  To: devel

[-- Attachment #1: Type: text/plain, Size: 4330 bytes --]

On Mon, 2010-07-05 at 20:03 +0800, malattia(a)linux.it wrote:
> Add more platforms to the list of the ones requiring aligned memory access. Also
> fix callsites where wrong assumptions where made in terms of aligment.
> 
> Signed-off-by: Mattia Dongili <malattia(a)linux.it>
> ---
>  compiler/asltree.c  |   15 ++++++++++-----
>  executer/exoparg2.c |   12 +++++++++---
>  include/actypes.h   |   26 +++++++++++++-------------
>  3 files changed, 32 insertions(+), 21 deletions(-)
> 
> Index: b/include/actypes.h
> ===================================================================
> --- a/include/actypes.h	2010-07-02 21:42:26.465350281 +0900
> +++ b/include/actypes.h	2010-07-04 11:18:22.506529460 +0900
> @@ -205,6 +205,19 @@
>  
>  /*! [End] no source code translation !*/
>  
> +/*
> + * In the case of the Itanium Processor Family (IPF), the hardware does not
> + * support misaligned memory transfers. Set the MISALIGNMENT_NOT_SUPPORTED flag
> + * to indicate that special precautions must be taken to avoid alignment faults.
> + * (IA64 or ia64 is currently used by existing compilers to indicate IPF.)
> + *
> + * Note: EM64T and other X86-64 processors support misaligned transfers,
> + * so there is no need to define this flag.
> + */
> +#if defined (__IA64__) || defined (__ia64__) | defined(__alpha__) || defined(__sparc__) || defined(__hppa__) || defined(__arm__)
> +#define ACPI_MISALIGNMENT_NOT_SUPPORTED
> +#endif
> +
>  
>  /*******************************************************************************
>   *
> @@ -231,19 +244,6 @@
>  #define ACPI_SIZE_MAX                   ACPI_UINT64_MAX
>  #define ACPI_USE_NATIVE_DIVIDE          /* Has native 64-bit integer support */
>  
> -/*
> - * In the case of the Itanium Processor Family (IPF), the hardware does not
> - * support misaligned memory transfers. Set the MISALIGNMENT_NOT_SUPPORTED flag
> - * to indicate that special precautions must be taken to avoid alignment faults.
> - * (IA64 or ia64 is currently used by existing compilers to indicate IPF.)
> - *
> - * Note: EM64T and other X86-64 processors support misaligned transfers,
> - * so there is no need to define this flag.
> - */
> -#if defined (__IA64__) || defined (__ia64__)
> -#define ACPI_MISALIGNMENT_NOT_SUPPORTED
> -#endif
> -
>  
>  /*******************************************************************************
>   *
> Index: b/executer/exoparg2.c
> ===================================================================
> --- a/executer/exoparg2.c	2010-07-02 21:42:26.477349196 +0900
> +++ b/executer/exoparg2.c	2010-07-04 11:18:22.506529460 +0900
> @@ -248,6 +248,8 @@
>      ACPI_OPERAND_OBJECT     **Operand = &WalkState->Operands[0];
>      ACPI_OPERAND_OBJECT     *ReturnDesc1 = NULL;
>      ACPI_OPERAND_OBJECT     *ReturnDesc2 = NULL;
> +    UINT64                  ReturnValue1 = 0;
> +    UINT64                  ReturnValue2 = 0;
>      ACPI_STATUS             Status;
>  
> 
> @@ -281,8 +283,10 @@
>  
>          Status = AcpiUtDivide (Operand[0]->Integer.Value,
>                                 Operand[1]->Integer.Value,
> -                               &ReturnDesc1->Integer.Value,
> -                               &ReturnDesc2->Integer.Value);
> +                               &ReturnValue1, &ReturnValue2);

Why need to replace &ReturnDesc1->Integer.Value with &ReturnValue1?

> +        ReturnDesc1->Integer.Value = ReturnValue1;
> +        ReturnDesc2->Integer.Value = ReturnValue2;
> +
>          if (ACPI_FAILURE (Status))
>          {
>              goto Cleanup;
> @@ -357,6 +361,7 @@
>      ACPI_OPERAND_OBJECT     **Operand = &WalkState->Operands[0];
>      ACPI_OPERAND_OBJECT     *ReturnDesc = NULL;
>      UINT64                  Index;
> +    UINT64                  ReturnValue = 0;
>      ACPI_STATUS             Status = AE_OK;
>      ACPI_SIZE               Length;
>  
> @@ -400,7 +405,8 @@
>          Status = AcpiUtDivide (Operand[0]->Integer.Value,
>                                 Operand[1]->Integer.Value,
>                                 NULL,
> -                               &ReturnDesc->Integer.Value);
> +                               &ReturnValue);
> +        ReturnDesc->Integer.Value = ReturnValue;
>          break;

Ditto.


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [Devel] [patch 4/5] Fix aligment issues
@ 2010-07-06 12:24 malattia
  0 siblings, 0 replies; 4+ messages in thread
From: malattia @ 2010-07-06 12:24 UTC (permalink / raw)
  To: devel

[-- Attachment #1: Type: text/plain, Size: 2290 bytes --]

On Tue, Jul 06, 2010 at 09:19:47AM +0800, Lin Ming wrote:
> On Mon, 2010-07-05 at 20:03 +0800, malattia(a)linux.it wrote:
> > Add more platforms to the list of the ones requiring aligned memory access. Also
> > fix callsites where wrong assumptions where made in terms of aligment.
...
> > Index: b/executer/exoparg2.c
> > ===================================================================
> > --- a/executer/exoparg2.c	2010-07-02 21:42:26.477349196 +0900
> > +++ b/executer/exoparg2.c	2010-07-04 11:18:22.506529460 +0900
> > @@ -248,6 +248,8 @@
> >      ACPI_OPERAND_OBJECT     **Operand = &WalkState->Operands[0];
> >      ACPI_OPERAND_OBJECT     *ReturnDesc1 = NULL;
> >      ACPI_OPERAND_OBJECT     *ReturnDesc2 = NULL;
> > +    UINT64                  ReturnValue1 = 0;
> > +    UINT64                  ReturnValue2 = 0;
> >      ACPI_STATUS             Status;
> >  
> > 
> > @@ -281,8 +283,10 @@
> >  
> >          Status = AcpiUtDivide (Operand[0]->Integer.Value,
> >                                 Operand[1]->Integer.Value,
> > -                               &ReturnDesc1->Integer.Value,
> > -                               &ReturnDesc2->Integer.Value);
> > +                               &ReturnValue1, &ReturnValue2);
> 
> Why need to replace &ReturnDesc1->Integer.Value with &ReturnValue1?

I get this on sparc systems without these 2 hunks:

malattia(a)smetana:~/acpica-unix> ./compiler/iasl -tc -ps.hex tests/misc/grammar.asl

Intel ACPI Component Architecture
ASL Optimizing Compiler version 20100528 [Jul  6 2010]
Copyright (c) 2000 - 2010 Intel Corporation
Supports ACPI Specification Revision 4.0a

[1]    28389 bus error  ./compiler/iasl -tc -ps.hex tests/misc/grammar.asl

it is caused by the pointer to the 64bit integer being aligned to 4
bytes instead of 8 bytes as required by the 64bit assigment inside
AcpiUtDivide when computing the result of the division:

        *OutQuotient = InDividend / InDivisor;

or at least that is my understanding of the problem.

Anyway, just using a temporary variable correctly aligned on the stack
and then assigning the result to Integer.Value afterwards works around
the problem.

Hope the explanation is understandable... otherwise I'll try harder :)
-- 
mattia
:wq!

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [Devel] [patch 4/5] Fix aligment issues
@ 2010-07-07  0:46 Lin Ming
  0 siblings, 0 replies; 4+ messages in thread
From: Lin Ming @ 2010-07-07  0:46 UTC (permalink / raw)
  To: devel

[-- Attachment #1: Type: text/plain, Size: 2489 bytes --]

On Tue, 2010-07-06 at 20:24 +0800, malattia(a)linux.it wrote:
> On Tue, Jul 06, 2010 at 09:19:47AM +0800, Lin Ming wrote:
> > On Mon, 2010-07-05 at 20:03 +0800, malattia(a)linux.it wrote:
> > > Add more platforms to the list of the ones requiring aligned memory access. Also
> > > fix callsites where wrong assumptions where made in terms of aligment.
> ...
> > > Index: b/executer/exoparg2.c
> > > ===================================================================
> > > --- a/executer/exoparg2.c	2010-07-02 21:42:26.477349196 +0900
> > > +++ b/executer/exoparg2.c	2010-07-04 11:18:22.506529460 +0900
> > > @@ -248,6 +248,8 @@
> > >      ACPI_OPERAND_OBJECT     **Operand = &WalkState->Operands[0];
> > >      ACPI_OPERAND_OBJECT     *ReturnDesc1 = NULL;
> > >      ACPI_OPERAND_OBJECT     *ReturnDesc2 = NULL;
> > > +    UINT64                  ReturnValue1 = 0;
> > > +    UINT64                  ReturnValue2 = 0;
> > >      ACPI_STATUS             Status;
> > >  
> > > 
> > > @@ -281,8 +283,10 @@
> > >  
> > >          Status = AcpiUtDivide (Operand[0]->Integer.Value,
> > >                                 Operand[1]->Integer.Value,
> > > -                               &ReturnDesc1->Integer.Value,
> > > -                               &ReturnDesc2->Integer.Value);
> > > +                               &ReturnValue1, &ReturnValue2);
> > 
> > Why need to replace &ReturnDesc1->Integer.Value with &ReturnValue1?
> 
> I get this on sparc systems without these 2 hunks:
> 
> malattia(a)smetana:~/acpica-unix> ./compiler/iasl -tc -ps.hex tests/misc/grammar.asl
> 
> Intel ACPI Component Architecture
> ASL Optimizing Compiler version 20100528 [Jul  6 2010]
> Copyright (c) 2000 - 2010 Intel Corporation
> Supports ACPI Specification Revision 4.0a
> 
> [1]    28389 bus error  ./compiler/iasl -tc -ps.hex tests/misc/grammar.asl
> 
> it is caused by the pointer to the 64bit integer being aligned to 4
> bytes instead of 8 bytes as required by the 64bit assigment inside
> AcpiUtDivide when computing the result of the division:
> 
>         *OutQuotient = InDividend / InDivisor;
> 
> or at least that is my understanding of the problem.
> 
> Anyway, just using a temporary variable correctly aligned on the stack
> and then assigning the result to Integer.Value afterwards works around
> the problem.
> 
> Hope the explanation is understandable... otherwise I'll try harder :)

Thanks for the clear explanation.

Lin Ming


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2010-07-07  0:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-05 12:03 [Devel] [patch 4/5] Fix aligment issues malattia
  -- strict thread matches above, loose matches on Subject: below --
2010-07-06  1:19 Lin Ming
2010-07-06 12:24 malattia
2010-07-07  0:46 Lin Ming

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.