* [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.