Re: [PATCH 2/7] Initial support for ARMv7 architecture

[Francesco Lavra]

On 04/01/2013 03:28 AM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>> +grub_err_t
>> +reloc_thm_call (grub_uint16_t *target, Elf32_Addr sym_addr)
>> +{
>> +  grub_int32_t offset, offset_low, offset_high;
>> +  grub_uint32_t sign, j1, j2, is_blx;
>> +  grub_uint32_t insword, insmask;
>> +
>> +  /* Extract instruction word in alignment-safe manner */
>> +  insword = (*target << 16) | (*(target + 1));
> 
> Why not use grub_get_unaligned32

It's not a simple unaligned access, it's actually about accessing two
naturally aligned 16-bit halfwords, and concatenating them by putting
the first value in the most significant bytes. Each halfword is encoded
in little endian, but the set of two halfwords is encoded "most
significant halfword first".
The problem here is in the GRUB_UTIL case, where accessing each of the
two halfwords could cause endianness issues. In this case two calls to
grub_target_to_host16() should be used to retrieve the instruction word.

>> +#ifdef GRUB_UTIL
>> +#pragma GCC diagnostic ignored "-Wcast-align"
>> +  grub_util_info ("    *target = 0x%08x", *((unsigned int *)target));
>> +#endif
> 
> grub_get_unaligned.

Ditto, it's not a simple unaligned access.

> 
>> +  *addr = insword >> 16;
>> +  *(addr + 1) = insword & 0xffff;
> 
> Use grub_set_unaligned+

The same applies here, in the util case a couple of
grub_host_to_target16() calls must be used to store back the instruction
word.

--
Francesco