GCC ARM: aligned access

2014-08-31 Thread Peng Fan
Hi,

I am writing some code and found that system crashed. I found it was
unaligned access which causes `data abort` exception. I write a piece of code 
and objdump
it. I am not sure this is right or not.

command:
arm-poky-linux-gnueabi-gcc -marm -mno-thumb-interwork -mabi=aapcs-linux 
-mword-relocations -march=armv7-a -mno-unaligned-access -ffunction-sections 
-fdata-sections -fno-common -ffixed-r9 -msoft-float -pipe  -O2 -c 2.c -o 2.o

arch is armv7-a and used '-mno-unaligned access'

c code:
typedef unsigned char u8;   
int func(u8 *data)  
{   
return *(unsigned int *)data;   
}

The objdumped asm code is:
   
Disassembly of section .text.func:  

 :
   0: e590  ldr r0, [r0]
   4: e12fff1e  bx  lr

from the asm code, we can see that 'ldr r0, [r0]' corresponding to '*(unsigned 
int*)data'. is this correct?
we can not guarantee that pointer data is 4bytes aligned. If pointer data is 
not 4bytes aligned, and aligned 
access check is enabled by setting a hardware bit in arm coprocessor, then 
`data abort` may occur.


Regards,
Peng.


Re: GCC ARM: aligned access

2014-08-31 Thread Peng Fan


On 09/01/2014 08:09 AM, Matt Thomas wrote:
> 
> On Aug 31, 2014, at 11:32 AM, Joel Sherrill  wrote:
> 
>>> Hi,
>>>
>>> I am writing some code and found that system crashed. I found it was
>>> unaligned access which causes `data abort` exception. I write a piece
>>> of code and objdump
>>> it. I am not sure this is right or not.
>>>
>>> command:
>>> arm-poky-linux-gnueabi-gcc -marm -mno-thumb-interwork -mabi=aapcs-linux
>>> -mword-relocations -march=armv7-a -mno-unaligned-access
>>> -ffunction-sections -fdata-sections -fno-common -ffixed-r9 -msoft-float
>>> -pipe  -O2 -c 2.c -o 2.o
>>>
>>> arch is armv7-a and used '-mno-unaligned access'
>>
>> I think this is totally expected. You were passed a u8 pointer which is 
>> aligned for that type (no restrictions likely). You cast it to a type with 
>> stricter alignment requirements. The code is just flawed. Some CPUs handle 
>> unaligned accesses but not your ARM.
> 
armv7 and armv6 arch except armv6-m support unaligned access. a u8 pointer is 
casted to u32 pointer, should gcc take the align problem into consideration to 
avoid possible errors? because -mno-unaligned-access.
> While armv7 and armv6 supports unaligned access, that support has to be 
> enabled by the underlying O/S.  Not knowing the underlying environment, 
> I can't say whether that support is enabled.  One issue we had in NetBSD
> in moving to gcc4.8 was that the NetBSD/arm kernel didn't enable unaligned
> access for armv[67] CPUs.  We quickly changed things so unaligned access
> is supported.

Yeah. by set a hardware bit in arm coprocessor, unaligned access will not cause 
data abort exception.
I just wonder is the following correct? should gcc take the responsibility to 
take care possible unaligned pointer `u8 *data`? because -mno-unaligned-access 
is passed to gcc.

int func(u8 *data)  
{   
return *(unsigned int *)data;   
}

 :
   0: e590  ldr r0, [r0]
   4: e12fff1e  bx  lr

Regards,
Peng.
> 


Re: GCC ARM: aligned access

2014-09-02 Thread Peng Fan


On 09/02/2014 09:25 PM, Julian Brown wrote:
> On Mon, 1 Sep 2014 09:14:31 +0800
> Peng Fan  wrote:
> 
>> On 09/01/2014 08:09 AM, Matt Thomas wrote:
>>>
>>> On Aug 31, 2014, at 11:32 AM, Joel Sherrill
>>>  wrote:
>>>> I think this is totally expected. You were passed a u8 pointer
>>>> which is aligned for that type (no restrictions likely). You cast
>>>> it to a type with stricter alignment requirements. The code is
>>>> just flawed. Some CPUs handle unaligned accesses but not your ARM.
>>>
>> armv7 and armv6 arch except armv6-m support unaligned access. a u8
>> pointer is casted to u32 pointer, should gcc take the align problem
>> into consideration to avoid possible errors? because
>> -mno-unaligned-access.
> 
> Using -munaligned-access (or its inverse) isn't enough to make GCC
> generate code that can perform arbitrary unaligned accesses, because
> several instructions (e.g. VFP loads/stores or load/store multiple
> instructions IIRC) must still act on naturally-aligned data even when
> the hardware flag to enable unaligned accesses is on, and those
> instructions will still be generated by GCC when they are considered
> safe, i.e. when not doing explicitly-unaligned accesses in packed
> structures or similar.
> 
> It would be *possible* to add an option to the backend to allow
> arbitrary alignment for any access, I think, but it's not at all clear
> that it's a good idea, and would certainly negatively affect
> performance.
> 
> (If you need unaligned accesses, you can use e.g. memcpy, and that will
> probably generate good inline code.)

Thanks for the reply. I've tried memcpy and all is fine.

Regards,
Peng.
> 
> Julian
> 


Warning: unpredictable: identical transfer and status registers --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3

2019-02-12 Thread Peng Fan
Hi,

We met an issue when building a piece jailhouse hypervisor code, "stxr   %w0, 
%3, %2\n\t" is 
compiled as "stxr w4,x5,[x4]" which triggers the warning 
"Warning: unpredictable: identical transfer and status registers"
After folder the do while into asm code, it is compiled as "stxr w1, x4, [x2]", 
no warning.

aarch64 poky gcc 8.3 triggers warning, aarch64 poky gcc 7.3 does not trigger 
warning, but has generated
same machine code. Not sure this is gcc bug or not, is my patch the correct 
fix? please help.

Detailed info below, the jailhouse repo code: 
https://github.com/siemens/jailhouse/blob/master/hypervisor/arch/arm64/include/asm/bitops.h#L77

static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
{
u32 ret;
u64 test, tmp;

BITOPT_ALIGN(nr, addr);

/* AARCH64_TODO: using Inner Shareable DMB at the moment,
 * revisit when we will deal with shareability domains */

do {
asm volatile (
"ldxr   %3, %2\n\t"
"ands   %1, %3, %4\n\t"
"b.ne   1f\n\t"
"orr%3, %3, %4\n\t"
"1:\n\t"
"stxr   %w0, %3, %2\n\t"
"dmbish\n\t"
: "=r" (ret), "=&r" (test),
  "+Q" (*(volatile unsigned long *)addr),
  "=r" (tmp)
: "r" (1ul << nr));
} while (ret);
return !!(test);
}

void panic_printk(const char *fmt, ...)
{
unsigned long cpu_id = phys_processor_id();
va_list ap;

if (test_and_set_bit(0, &panic_in_progress) && panic_cpu != cpu_id)
return;
panic_cpu = cpu_id;

va_start(ap, fmt);

__vprintk(fmt, ap);

va_end(ap);
}

The asm code:
c0207a94 :
c0207a94:   a9b67bfdstp x29, x30, [sp, #-160]!
c0207a98:   910003fdmov x29, sp
c0207a9c:   f9000bf3str x19, [sp, #16]
c0207aa0:   aa0003f3mov x19, x0
c0207aa4:   a9068ba1stp x1, x2, [x29, #104]
c0207aa8:   a90793a3stp x3, x4, [x29, #120]
c0207aac:   a9089ba5stp x5, x6, [x29, #136]
c0207ab0:   f9004fa7str x7, [x29, #152]
c0207ab4:   97ffed27bl  c0202f50 
c0207ab8:   f081adrpx1, c021a000 
c0207abc:   d2800022mov x2, #0x1
// #1
c0207ac0:   91006024add x4, x1, #0x18
c0207ac4:   c85f7c85ldxrx5, [x4]
c0207ac8:   ea0200a3andsx3, x5, x2
c0207acc:   5441b.nec0207ad4 
  // b.any
c0207ad0:   aa0200a5orr x5, x5, x2
c0207ad4:   c8047c85stxrw4, x5, [x4]
c0207ad8:   d5033bbfdmb ish
c0207adc:   3524cbnzw4, c0207ac0 


Note pc c0207ad4 has instruction stxr w4, x5, [x4] which is triggers 
warning using aarch64 poky gcc 8.3

I did a fix to the code as below:
--- a/hypervisor/arch/arm64/include/asm/bitops.h
+++ b/hypervisor/arch/arm64/include/asm/bitops.h
@@ -83,21 +83,21 @@ static inline int test_and_set_bit(int nr, volatile 
unsigned long *addr)

/* AARCH64_TODO: using Inner Shareable DMB at the moment,
 * revisit when we will deal with shareability domains */
+   asm volatile (
+   "1:\n\t"
+   "ldxr   %3, %2\n\t"
+   "ands   %1, %3, %4\n\t"
+   "b.ne   2f\n\t"
+   "orr%3, %3, %4\n\t"
+   "2:\n\t"
+   "stxr   %w0, %3, %2\n\t"
+   "dmbish\n\t"
+   "cbnz   %w0, 1b\n\t"
+   : "=r" (ret), "=&r" (test),
+ "+Q" (*(volatile unsigned long *)addr),
+ "=r" (tmp)
+   : "r" (1ul << nr));

-   do {
-   asm volatile (
-   "ldxr   %3, %2\n\t"
-   "ands   %1, %3, %4\n\t"
-   "b.ne   1f\n\t"
-   "orr%3, %3, %4\n\t"
-   "1:\n\t"
-   "stxr   %w0, %3, %2\n\t"
-   "dmbish\n\t"
-   : "=r" (ret), "=&r" (test),
- "+Q" (*(volatile unsigned long *)addr),
- "=r" (tmp)
-   : "r" (1ul << nr));
-   } while (ret);
return !!(test);
 }

Then the asm code as below:
c02079e0 :
c02079e0:   a9b67bfdstp x29, x30, [sp, #-160]!
c02079e4:   910003fdmov x29, sp
c02079e8:   f9000bf3str x19, [sp, #16]
c02079ec:   aa0003f3mov x19, x0
c02079f0:   

RE: Warning: unpredictable: identical transfer and status registers --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3

2019-02-13 Thread Peng Fan


> -Original Message-
> From: jailhouse-...@googlegroups.com
> [mailto:jailhouse-...@googlegroups.com] On Behalf Of Segher Boessenkool
> Sent: 2019年2月13日 17:11
> To: Peng Fan 
> Cc: gcc@gcc.gnu.org; james.greenha...@arm.com; n...@arm.com;
> jailhouse-...@googlegroups.com; will.dea...@arm.com; Catalin Marinas
> 
> Subject: Re: Warning: unpredictable: identical transfer and status registers
> --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3
> 
> OneWed, Feb 13, 2019 at 07:13:21AM +, Peng Fan wrote:
> > We met an issue when building a piece jailhouse hypervisor code,
> "stxr   %w0, %3, %2\n\t" is
> > compiled as "stxr w4,x5,[x4]" which triggers the warning
> > "Warning: unpredictable: identical transfer and status registers"
> 
> This is not a GCC question.
> 
> The three registers, in order, are status, transfer, and base.  The warning
> claims transfer and status are identical, but in fact base and status are.
> The code (in binutils, gas/config/tc-aarch64.c) is
> 
> case ldstexcl:
>   /* It is unpredictable if the destination and status registers are the
>  same.  */
>   if ((aarch64_get_operand_class (opnds[0].type)
>== AARCH64_OPND_CLASS_INT_REG)
>   && (aarch64_get_operand_class (opnds[1].type)
>   == AARCH64_OPND_CLASS_INT_REG)
>   && (opnds[0].reg.regno == opnds[1].reg.regno
>   || opnds[0].reg.regno == opnds[2].reg.regno))
> as_warn (_("unpredictable: identical transfer and status registers"
>" --`%s'"),
>  str);
> 
> so either that op0 == op2 test is spurious, or the warning message is
> misleading.

From aarch64 spec, "stxr w4,x5,[x4]"'s behavior is chip implementation defined,
so I think the warning is correct.

> 
> Please ask on binut...@sourceware.org and/or file a bug at
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsour
> ceware.org%2Fbugzilla%2F&data=02%7C01%7Cpeng.fan%40nxp.com%
> 7C2463376de5aa417d3d6508d69197e4df%7C686ea1d3bc2b4c6fa92cd99c5c
> 301635%7C0%7C0%7C636856478900947075&sdata=pg%2FZmo91Cxfji
> ESlBiR5shmsdxoYWslpfZeAXk5TV30%3D&reserved=0 ?

Ok. I'll post questions to binut...@sourceware.org

Thanks,
Peng.

> 
> 
> Segher
> 
> --
> You received this message because you are subscribed to the Google Groups
> "Jailhouse" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to jailhouse-dev+unsubscr...@googlegroups.com.
> For more options, visit
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgrou
> ps.google.com%2Fd%2Foptout&data=02%7C01%7Cpeng.fan%40nxp.co
> m%7C2463376de5aa417d3d6508d69197e4df%7C686ea1d3bc2b4c6fa92cd9
> 9c5c301635%7C0%7C0%7C636856478900947075&sdata=oiNHshxQvku
> OjuTiRKlK%2F3em5DIGNysm1UbZZKuHcjg%3D&reserved=0.


RE: Warning: unpredictable: identical transfer and status registers --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3

2019-02-13 Thread Peng Fan


> -Original Message-
> From: Andreas Schwab [mailto:sch...@suse.de]
> Sent: 2019年2月13日 21:59
> To: Peng Fan 
> Cc: gcc@gcc.gnu.org; james.greenha...@arm.com; n...@arm.com;
> jailhouse-...@googlegroups.com; will.dea...@arm.com; Catalin Marinas
> 
> Subject: Re: Warning: unpredictable: identical transfer and status registers
> --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3
> 
> On Feb 13 2019, Peng Fan  wrote:
> 
> > static inline int test_and_set_bit(int nr, volatile unsigned long
> > *addr) {
> > u32 ret;
> > u64 test, tmp;
> >
> > BITOPT_ALIGN(nr, addr);
> >
> > /* AARCH64_TODO: using Inner Shareable DMB at the moment,
> >  * revisit when we will deal with shareability domains */
> >
> > do {
> > asm volatile (
> > "ldxr   %3, %2\n\t"
> > "ands   %1, %3, %4\n\t"
> > "b.ne   1f\n\t"
> > "orr%3, %3, %4\n\t"
> > "1:\n\t"
> > "stxr   %w0, %3, %2\n\t"
> > "dmbish\n\t"
> > : "=r" (ret), "=&r" (test),
> >   "+Q" (*(volatile unsigned long *)addr),
> >   "=r" (tmp)
> > : "r" (1ul << nr));
> 
> %3 is modified early, but not marked earlyclobber.

Thanks, I'll try add earlyclobber.

Peng

> 
> Andreas.
> 
> --
> Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196
> BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7 "And now for something
> completely different."


RE: Warning: unpredictable: identical transfer and status registers --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3

2019-02-13 Thread Peng Fan
Hi Michael,

> -Original Message-
> From: Michael Matz [mailto:m...@suse.de]
> Sent: 2019年2月13日 22:28
> To: Peng Fan 
> Cc: gcc@gcc.gnu.org; james.greenha...@arm.com; n...@arm.com;
> jailhouse-...@googlegroups.com; will.dea...@arm.com; Catalin Marinas
> 
> Subject: Re: Warning: unpredictable: identical transfer and status registers
> --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3
> 
> Hi,
> 
> On Wed, 13 Feb 2019, Peng Fan wrote:
> 
> > asm volatile (
> > "ldxr   %3, %2\n\t"
> > "ands   %1, %3, %4\n\t"
> > "b.ne   1f\n\t"
> > "orr%3, %3, %4\n\t"
> > "1:\n\t"
> > "stxr   %w0, %3, %2\n\t"
> > "dmbish\n\t"
> > : "=r" (ret), "=&r" (test),
> >   "+Q" (*(volatile unsigned long *)addr),
> >   "=r" (tmp)
> > : "r" (1ul << nr));
> 
> As Andreas says, you need to add an early-clobber for op3 for correctness (to
> force it into a different register from op4).  And you also need an
> early-clobber on op0 to force it into a different register from op2 (which for
> purposes of register assignment is an input operand holding an address).

So the fix should be the following, right?
do {
asm volatile (
"ldxr   %3, %2\n\t"
"ands   %1, %3, %4\n\t"
"b.ne   1f\n\t"
"orr%3, %3, %4\n\t"
"1:\n\t"
"stxr   %w0, %3, %2\n\t"
"dmbish\n\t"
: "=&r" (ret), "=&r" (test),
  "+Q" (*(volatile unsigned long *)addr),
  "=&r" (tmp)
: "r" (1ul << nr));
} while (ret);

Thanks,
Peng.

> 
> 
> Ciao,
> Michael.


RE: Warning: unpredictable: identical transfer and status registers --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3

2019-02-14 Thread Peng Fan


> -Original Message-
> From: Michael Matz [mailto:m...@suse.de]
> Sent: 2019年2月13日 22:45
> To: Peng Fan 
> Cc: gcc@gcc.gnu.org; james.greenha...@arm.com; n...@arm.com;
> jailhouse-...@googlegroups.com; will.dea...@arm.com; Catalin Marinas
> 
> Subject: RE: Warning: unpredictable: identical transfer and status registers
> --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3
> 
> Hi,
> 
> On Wed, 13 Feb 2019, Peng Fan wrote:
> 
> > So the fix should be the following, right?
> 
> Yup.

Thanks for your help.

Thanks,
Peng.

> 
> 
> Ciao,
> Michael.