On Fri, Aug 30, 2013 at 6:01 AM, Grant Likely <grant.likely@xxxxxxxxxxxx> wrote:
> On Fri, 9 Aug 2013 16:26:05 -0700, Roy Franz <roy.franz@xxxxxxxxxx> wrote:
>> This allows allocations to be made low in memory while
>> avoiding allocations at the base of memory.
>
> Your commit message should include /why/ the change is needed. From the
> above I understand what the patch does, but I don't understand why it is
> necessary.
This was used to avoid relocating the zImage so low in memory that it
would conflict
with memory range that it was decompressed into. This is now handled
by allocating
the memory for the decompressed kernel, so the lower limit is no
longer required.
I'll remove it, as it is not necessary any more.
>
> The patch looks fine to me, but it would be worth investigating merging
> alloc_low and alloc_high. It looks like they both do pretty close to the
> same calculations. A single core function could do both, could have both
> minimum and maximum constraints, and could have a flag to determine if
> low or high addresses should be preferred.
>
> g.
>
> Reviewed-by: Grant Likely <grant.likely@xxxxxxxxxx>
>
>>
>> Signed-off-by: Roy Franz <roy.franz@xxxxxxxxxx>
>> ---
>> arch/x86/boot/compressed/eboot.c | 11 ++++++-----
>> drivers/firmware/efi/efi-stub-helper.c | 7 +++++--
>> 2 files changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
>> index 2a4430a..f44ef2f 100644
>> --- a/arch/x86/boot/compressed/eboot.c
>> +++ b/arch/x86/boot/compressed/eboot.c
>> @@ -458,7 +458,7 @@ struct boot_params *make_boot_params(void *handle, efi_system_table_t *_table)
>> }
>>
>> status = efi_low_alloc(sys_table, 0x4000, 1,
>> - (unsigned long *)&boot_params);
>> + (unsigned long *)&boot_params, 0);
>> if (status != EFI_SUCCESS) {
>> efi_printk(sys_table, "Failed to alloc lowmem for boot params\n");
>> return NULL;
>> @@ -505,7 +505,7 @@ struct boot_params *make_boot_params(void *handle, efi_system_table_t *_table)
>> options_size++; /* NUL termination */
>>
>> status = efi_low_alloc(sys_table, options_size, 1,
>> - &cmdline);
>> + &cmdline, 0);
>> if (status != EFI_SUCCESS) {
>> efi_printk(sys_table, "Failed to alloc mem for cmdline\n");
>> goto fail;
>> @@ -563,7 +563,8 @@ static efi_status_t exit_boot(struct boot_params *boot_params,
>> again:
>> size += sizeof(*mem_map) * 2;
>> _size = size;
>> - status = efi_low_alloc(sys_table, size, 1, (unsigned long *)&mem_map);
>> + status = efi_low_alloc(sys_table, size, 1,
>> + (unsigned long *)&mem_map, 0);
>> if (status != EFI_SUCCESS)
>> return status;
>>
>> @@ -697,7 +698,7 @@ static efi_status_t relocate_kernel(struct setup_header *hdr)
>> nr_pages, &start);
>> if (status != EFI_SUCCESS) {
>> status = efi_low_alloc(sys_table, hdr->init_size,
>> - hdr->kernel_alignment, &start);
>> + hdr->kernel_alignment, &start, 0);
>> if (status != EFI_SUCCESS)
>> efi_printk(sys_table, "Failed to alloc mem for kernel\n");
>> }
>> @@ -745,7 +746,7 @@ struct boot_params *efi_main(void *handle, efi_system_table_t *_table,
>>
>> gdt->size = 0x800;
>> status = efi_low_alloc(sys_table, gdt->size, 8,
>> - (unsigned long *)&gdt->address);
>> + (unsigned long *)&gdt->address, 0);
>> if (status != EFI_SUCCESS) {
>> efi_printk(sys_table, "Failed to alloc mem for gdt\n");
>> goto fail;
>> diff --git a/drivers/firmware/efi/efi-stub-helper.c b/drivers/firmware/efi/efi-stub-helper.c
>> index 0218d535..40cd16e 100644
>> --- a/drivers/firmware/efi/efi-stub-helper.c
>> +++ b/drivers/firmware/efi/efi-stub-helper.c
>> @@ -163,11 +163,11 @@ fail:
>> }
>>
>> /*
>> - * Allocate at the lowest possible address.
>> + * Allocate at the lowest possible address, that is not below min.
>> */
>> static efi_status_t efi_low_alloc(efi_system_table_t *sys_table_arg,
>> unsigned long size, unsigned long align,
>> - unsigned long *addr)
>> + unsigned long *addr, unsigned long min)
>> {
>> unsigned long map_size, desc_size;
>> efi_memory_desc_t *map;
>> @@ -204,6 +204,9 @@ static efi_status_t efi_low_alloc(efi_system_table_t *sys_table_arg,
>> if (start == 0x0)
>> start += 8;
>>
>> + if (start < min)
>> + start = min;
>> +
>> start = round_up(start, align);
>> if ((start + size) > end)
>> continue;
>> --
>> 1.7.10.4
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
