Code in arch/arm/lib/image.c doing alignment incorrectly?

[Glotzer, John]

This is regarding the uboot code in:           
                                                     
cmd/booti.c : booti_start()                    (caller)                              
arch/arm/lib/image.c : booti_setup()   (callee)   

The concern is that the alignment calculation is not achieving the intended result.
    
To be brief I will just post small sections from each file/function.    
    
cmd/boot.c : function booti_start()
--------------------------------------------------------------------------------    
  ...

  ret = booti_setup(ld, &relocated_addr, &image_size, false);    
  if (ret != 0)    
    return 1;    
    
  /* Handle BOOTM_STATE_LOADOS */    
  if (relocated_addr != ld) {    
    printf("Moving Image from 0x%lx to 0x%lx, end=%lx\n", ld,    
           relocated_addr, relocated_addr + image_size);    
    memmove((void *)relocated_addr, (void *)ld, image_size);    
  }

   ...    
--------------------------------------------------------------------------------    
    

arch/arm/lib/image.c : function booti_setup() 
-------------------------------------------------------------------------------- 

...

  if (!force_reloc && (le64_to_cpu(ih->flags) & BIT(3)))    
    dst = image - text_offset;    
  else    
    dst = gd->bd->bi_dram[0].start;    
        
  *relocated_addr = ALIGN(dst, SZ_2M) + text_offset; 

... 
--------------------------------------------------------------------------------    
    
    
The intent seems to be to put the start of the linux text segment
on a 2MB aligned address. But this code doesn't do this.    
    
Taking a use case from a recent project - consider a case where the initial    
address, ld, is 0x8028_0000 and text offset is 0x8_0000. Also assuming for this discussion
the "if" branch of the logic rather than the "else" branch although a similar argument
would apply for that case as well.
    
This means that when initially loaded the linux text segment would start at    
0x8030_0000  (the start address plus the text offset) which is not 2MB aligned.
But this logic as currently coded will:                
                                                                                                
1. Subtract 0x8_0000 from 0x8028_0000 to get 0x8020_0000.                               
2. Determine that 0x8020_0000 is 2MB aligned.                                         
3. Not relocate the code.                                                               
4. The result is a text segment that starts at 0x8030_0000 (not aligned).               
                                                                                               
Or consider loading at 0x8018_0000 with text offset 0x8_0000.                               
Here the text segment does start on a 2MB boundary (0x8020_0000) but this code                               
would conclude that it does not - it would                                                     
                                                                                               
1. Subtract 0x8_0000 from 0x8018_0000 to get 0x8010_0000.                                      
2. Determine that 0x8010_0000 is not 2MB aligned.                                             
3. Relocate the code. (in this case to 0x8028_0000).    
4. The result is a text segment that (again) starts at 0x8030_0000 (not aligned).
    
    
Should not the order of operations here be reversed? That is to say instead of    
Subtract, Align, Add the code should be doing Add, Align, Subtract?    

Thanks,

John Glotzer
glotzer at amazon.com