[U-Boot] [PATCH v1 13/15] efi_loader: make pool allocations cacheline aligned

Heinrich Schuchardt xypron.glpk at gmx.de
Fri Aug 11 17:47:07 UTC 2017


On 08/11/2017 07:27 PM, Rob Clark wrote:
> On Fri, Aug 11, 2017 at 12:58 PM, Heinrich Schuchardt
> <xypron.glpk at gmx.de> wrote:
>> On 08/10/2017 08:29 PM, Rob Clark wrote:
>>> This avoids printf() spam about file reads (such as loading an image)
>>> into unaligned buffers (and the associated memcpy()).  And generally
>>> seems like a good idea.
>>>
>>> Signed-off-by: Rob Clark <robdclark at gmail.com>
>>> ---
>>>  lib/efi_loader/efi_memory.c | 5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>>> index 9e079f1fa3..2ba8d8b42b 100644
>>> --- a/lib/efi_loader/efi_memory.c
>>> +++ b/lib/efi_loader/efi_memory.c
>>> @@ -43,7 +43,7 @@ void *efi_bounce_buffer;
>>>   */
>>>  struct efi_pool_allocation {
>>>       u64 num_pages;
>>> -     char data[];
>>> +     char data[] __attribute__((aligned(ARCH_DMA_MINALIGN)));
>>>  };
>>>
>>>  /*
>>> @@ -356,7 +356,8 @@ efi_status_t efi_allocate_pool(int pool_type, unsigned long size,
>>>  {
>>>       efi_status_t r;
>>>       efi_physical_addr_t t;
>>> -     u64 num_pages = (size + sizeof(u64) + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT;
>>> +     u64 num_pages = DIV_ROUND_UP(size + sizeof(struct efi_pool_allocation),
>>> +                                  EFI_PAGE_SIZE);
>>
>> It seems you missed my mail dated 2017-08-02T01:21Z:
>>
>> With DIV_ROUND_UP you introduce a 64bit division. Depending on the
>> architecture this is only available via stdlib which is not available in
>> U-Boot.
>>
>> Please, use
>> + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT;
>> as in the original line.
>>
> 
> I didn't miss it.. but I did disagree with it.  It is an unsigned
> division by a power-of-two.  The compiler turns this into a
> right-shift.  So in fact both ways generate the same code, but the
> DIV_ROUND_UP() is more clear.

This conversion is not enforced by the define in include/linux/kernel.h.

This is not required by ISO/IEC 9899. So you should not rely on it.
We surely do not want compiler specific coding in U-Boot.

Regards

Heinrich


More information about the U-Boot mailing list