* [U-Boot] [PATCH] efi_loader: make pool allocations cacheline aligned
@ 2017-08-01 20:00 Rob Clark
2017-08-02 1:21 ` Heinrich Schuchardt
[not found] ` <f0b2b064-d919-b437-3c8d-a76902665424@gmx.de>
0 siblings, 2 replies; 5+ messages in thread
From: Rob Clark @ 2017-08-01 20:00 UTC (permalink / raw)
To: u-boot
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@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);
if (size == 0) {
*buffer = NULL;
--
2.13.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [U-Boot] [PATCH] efi_loader: make pool allocations cacheline aligned
2017-08-01 20:00 [U-Boot] [PATCH] efi_loader: make pool allocations cacheline aligned Rob Clark
@ 2017-08-02 1:21 ` Heinrich Schuchardt
[not found] ` <f0b2b064-d919-b437-3c8d-a76902665424@gmx.de>
1 sibling, 0 replies; 5+ messages in thread
From: Heinrich Schuchardt @ 2017-08-02 1:21 UTC (permalink / raw)
To: u-boot
On 08/01/2017 10:00 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@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);
>
> if (size == 0) {
> *buffer = NULL;
>
NAK
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.
Could you, please, use
scripts/getmaintainer.pl
to determine the addressees of patches.
Best regards
Heinrich
^ permalink raw reply [flat|nested] 5+ messages in thread[parent not found: <f0b2b064-d919-b437-3c8d-a76902665424@gmx.de>]
* [U-Boot] [PATCH] efi_loader: make pool allocations cacheline aligned
[not found] ` <f0b2b064-d919-b437-3c8d-a76902665424@gmx.de>
@ 2017-08-02 9:28 ` Rob Clark
2017-08-02 15:24 ` Brüns, Stefan
0 siblings, 1 reply; 5+ messages in thread
From: Rob Clark @ 2017-08-02 9:28 UTC (permalink / raw)
To: u-boot
On Tue, Aug 1, 2017 at 9:10 PM, Heinrich Schuchardt
<xypron.debian@gmx.de> wrote:
> On 08/01/2017 10:00 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@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);
>>
>> if (size == 0) {
>> *buffer = NULL;
>>
> NAK
>
> 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.
The divisor is a constant power of two so compiler should turn this into a shift
> Please, use
> + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT;
> as in the original line.
>
This was actually incorrect (missing a "- 1"), which is why I decided
to stop open-coding DIV_ROUND_UP().
BR,
-R
^ permalink raw reply [flat|nested] 5+ messages in thread* [U-Boot] [PATCH] efi_loader: make pool allocations cacheline aligned
2017-08-02 9:28 ` Rob Clark
@ 2017-08-02 15:24 ` Brüns, Stefan
2017-08-02 16:24 ` Rob Clark
0 siblings, 1 reply; 5+ messages in thread
From: Brüns, Stefan @ 2017-08-02 15:24 UTC (permalink / raw)
To: u-boot
On Mittwoch, 2. August 2017 11:28:37 CEST Rob Clark wrote:
> On Tue, Aug 1, 2017 at 9:10 PM, Heinrich Schuchardt
[...]
> >>
> >> @@ -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);
> >>
> >> if (size == 0) {
> >>
> >> *buffer = NULL;
> >
> > NAK
> >
> > 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.
>
> The divisor is a constant power of two so compiler should turn this into a
> shift
> > Please, use
> > + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT;
> > as in the original line.
>
> This was actually incorrect (missing a "- 1"), which is why I decided
> to stop open-coding DIV_ROUND_UP().
EFI_PAGE_MASK == EFI_PAGE_SIZE - 1
Kind regards,
Stefan
^ permalink raw reply [flat|nested] 5+ messages in thread* [U-Boot] [PATCH] efi_loader: make pool allocations cacheline aligned
2017-08-02 15:24 ` Brüns, Stefan
@ 2017-08-02 16:24 ` Rob Clark
0 siblings, 0 replies; 5+ messages in thread
From: Rob Clark @ 2017-08-02 16:24 UTC (permalink / raw)
To: u-boot
On Wed, Aug 2, 2017 at 11:24 AM, Brüns, Stefan
<Stefan.Bruens@rwth-aachen.de> wrote:
> On Mittwoch, 2. August 2017 11:28:37 CEST Rob Clark wrote:
>> On Tue, Aug 1, 2017 at 9:10 PM, Heinrich Schuchardt
> [...]
>> >>
>> >> @@ -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);
>> >>
>> >> if (size == 0) {
>> >>
>> >> *buffer = NULL;
>> >
>> > NAK
>> >
>> > 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.
>>
>> The divisor is a constant power of two so compiler should turn this into a
>> shift
>> > Please, use
>> > + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT;
>> > as in the original line.
>>
>> This was actually incorrect (missing a "- 1"), which is why I decided
>> to stop open-coding DIV_ROUND_UP().
>
> EFI_PAGE_MASK == EFI_PAGE_SIZE - 1
>
Oh, you're right, I was reading that as PAGE_SIZE, not PAGE_MASK. All
the more reason we should use DIV_ROUND_UP().
BR,
-R
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-08-02 16:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-01 20:00 [U-Boot] [PATCH] efi_loader: make pool allocations cacheline aligned Rob Clark
2017-08-02 1:21 ` Heinrich Schuchardt
[not found] ` <f0b2b064-d919-b437-3c8d-a76902665424@gmx.de>
2017-08-02 9:28 ` Rob Clark
2017-08-02 15:24 ` Brüns, Stefan
2017-08-02 16:24 ` Rob Clark
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox