lkml.org 
[lkml]   [2018]   [May]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH] efi/x86: Clean up the eboot code a bit

    So I looked at arch/x86/boot/compressed/eboot.c to improve a printk message and
    ended up with the cleanups below.

    Only build tested.

    Thanks,

    Ingo

    =================>
    Subject: efi/x86: Clean up the eboot code
    From: Ingo Molnar <mingo@kernel.org>
    Date: Mon May 14 08:33:40 CEST 2018

    Various small cleanups:

    - Standardize printk messages:

    'alloc' => 'allocate'
    'mem' => 'memory'

    also put variable names in printk messages between quotes.

    - Align mass-assignments vertically for better readability

    - Break multi-line function prototypes at the name where possible,
    not in the middle of the parameter list

    - Use a newline before return statements consistently.

    - Use curly braces in a balanced fashion.

    - Remove stray newlines.

    No change in functionality.

    Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
    Cc: Linus Torvalds <torvalds@linux-foundation.org>
    Cc: Matt Fleming <matt@codeblueprint.co.uk>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Thomas Gleixner <tglx@linutronix.de>
    Cc: linux-efi@vger.kernel.org
    Signed-off-by: Ingo Molnar <mingo@kernel.org>
    ---
    arch/x86/boot/compressed/eboot.c | 247 +++++++++++++++++++--------------------
    1 file changed, 126 insertions(+), 121 deletions(-)

    Index: tip/arch/x86/boot/compressed/eboot.c
    ===================================================================
    --- tip.orig/arch/x86/boot/compressed/eboot.c
    +++ tip/arch/x86/boot/compressed/eboot.c
    @@ -34,9 +34,9 @@ static void setup_boot_services##bits(st
    \
    table = (typeof(table))sys_table; \
    \
    - c->runtime_services = table->runtime; \
    - c->boot_services = table->boottime; \
    - c->text_output = table->con_out; \
    + c->runtime_services = table->runtime; \
    + c->boot_services = table->boottime; \
    + c->text_output = table->con_out; \
    }
    BOOT_SERVICES(32);
    BOOT_SERVICES(64);
    @@ -64,6 +64,7 @@ static inline efi_status_t __open_volume
    efi_printk(sys_table, "Failed to open volume\n");

    *__fh = fh;
    +
    return status;
    }

    @@ -90,6 +91,7 @@ static inline efi_status_t __open_volume
    efi_printk(sys_table, "Failed to open volume\n");

    *__fh = fh;
    +
    return status;
    }

    @@ -140,16 +142,16 @@ __setup_efi_pci(efi_pci_io_protocol_t *p

    status = efi_call_early(allocate_pool, EFI_LOADER_DATA, size, &rom);
    if (status != EFI_SUCCESS) {
    - efi_printk(sys_table, "Failed to alloc mem for rom\n");
    + efi_printk(sys_table, "Failed to allocate memory for 'rom'\n");
    return status;
    }

    memset(rom, 0, sizeof(*rom));

    - rom->data.type = SETUP_PCI;
    - rom->data.len = size - sizeof(struct setup_data);
    - rom->data.next = 0;
    - rom->pcilen = pci->romsize;
    + rom->data.type = SETUP_PCI;
    + rom->data.len = size - sizeof(struct setup_data);
    + rom->data.next = 0;
    + rom->pcilen = pci->romsize;
    *__rom = rom;

    status = efi_call_proto(efi_pci_io_protocol, pci.read, pci,
    @@ -186,8 +188,7 @@ free_struct:
    }

    static void
    -setup_efi_pci32(struct boot_params *params, void **pci_handle,
    - unsigned long size)
    +setup_efi_pci32(struct boot_params *params, void **pci_handle, unsigned long size)
    {
    efi_pci_io_protocol_t *pci = NULL;
    efi_guid_t pci_proto = EFI_PCI_IO_PROTOCOL_GUID;
    @@ -226,13 +227,11 @@ setup_efi_pci32(struct boot_params *para
    params->hdr.setup_data = (unsigned long)rom;

    data = (struct setup_data *)rom;
    -
    }
    }

    static void
    -setup_efi_pci64(struct boot_params *params, void **pci_handle,
    - unsigned long size)
    +setup_efi_pci64(struct boot_params *params, void **pci_handle, unsigned long size)
    {
    efi_pci_io_protocol_t *pci = NULL;
    efi_guid_t pci_proto = EFI_PCI_IO_PROTOCOL_GUID;
    @@ -271,7 +270,6 @@ setup_efi_pci64(struct boot_params *para
    params->hdr.setup_data = (unsigned long)rom;

    data = (struct setup_data *)rom;
    -
    }
    }

    @@ -301,7 +299,7 @@ static void setup_efi_pci(struct boot_pa
    size, (void **)&pci_handle);

    if (status != EFI_SUCCESS) {
    - efi_printk(sys_table, "Failed to alloc mem for pci_handle\n");
    + efi_printk(sys_table, "Failed to allocate memory for 'pci_handle'\n");
    return;
    }

    @@ -347,8 +345,7 @@ static void retrieve_apple_device_proper
    status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
    size + sizeof(struct setup_data), &new);
    if (status != EFI_SUCCESS) {
    - efi_printk(sys_table,
    - "Failed to alloc mem for properties\n");
    + efi_printk(sys_table, "Failed to allocate memory for 'properties'\n");
    return;
    }

    @@ -364,9 +361,9 @@ static void retrieve_apple_device_proper
    new->next = 0;

    data = (struct setup_data *)(unsigned long)boot_params->hdr.setup_data;
    - if (!data)
    + if (!data) {
    boot_params->hdr.setup_data = (unsigned long)new;
    - else {
    + } else {
    while (data->next)
    data = (struct setup_data *)(unsigned long)data->next;
    data->next = (unsigned long)new;
    @@ -479,8 +476,8 @@ setup_uga64(void **uga_handle, unsigned
    /*
    * See if we have Universal Graphics Adapter (UGA) protocol
    */
    -static efi_status_t setup_uga(struct screen_info *si, efi_guid_t *uga_proto,
    - unsigned long size)
    +static efi_status_t
    +setup_uga(struct screen_info *si, efi_guid_t *uga_proto, unsigned long size)
    {
    efi_status_t status;
    u32 width, height;
    @@ -509,23 +506,24 @@ static efi_status_t setup_uga(struct scr
    goto free_handle;

    /* EFI framebuffer */
    - si->orig_video_isVGA = VIDEO_TYPE_EFI;
    + si->orig_video_isVGA = VIDEO_TYPE_EFI;

    - si->lfb_depth = 32;
    - si->lfb_width = width;
    - si->lfb_height = height;
    -
    - si->red_size = 8;
    - si->red_pos = 16;
    - si->green_size = 8;
    - si->green_pos = 8;
    - si->blue_size = 8;
    - si->blue_pos = 0;
    - si->rsvd_size = 8;
    - si->rsvd_pos = 24;
    + si->lfb_depth = 32;
    + si->lfb_width = width;
    + si->lfb_height = height;
    +
    + si->red_size = 8;
    + si->red_pos = 16;
    + si->green_size = 8;
    + si->green_pos = 8;
    + si->blue_size = 8;
    + si->blue_pos = 0;
    + si->rsvd_size = 8;
    + si->rsvd_pos = 24;

    free_handle:
    efi_call_early(free_pool, uga_handle);
    +
    return status;
    }

    @@ -607,7 +605,7 @@ struct boot_params *make_boot_params(str
    status = efi_low_alloc(sys_table, 0x4000, 1,
    (unsigned long *)&boot_params);
    if (status != EFI_SUCCESS) {
    - efi_printk(sys_table, "Failed to alloc lowmem for boot params\n");
    + efi_printk(sys_table, "Failed to allocate lowmem for boot params\n");
    return NULL;
    }

    @@ -623,9 +621,9 @@ struct boot_params *make_boot_params(str
    * Fill out some of the header fields ourselves because the
    * EFI firmware loader doesn't load the first sector.
    */
    - hdr->root_flags = 1;
    - hdr->vid_mode = 0xffff;
    - hdr->boot_flag = 0xAA55;
    + hdr->root_flags = 1;
    + hdr->vid_mode = 0xffff;
    + hdr->boot_flag = 0xAA55;

    hdr->type_of_loader = 0x21;

    @@ -633,6 +631,7 @@ struct boot_params *make_boot_params(str
    cmdline_ptr = efi_convert_cmdline(sys_table, image, &options_size);
    if (!cmdline_ptr)
    goto fail;
    +
    hdr->cmd_line_ptr = (unsigned long)cmdline_ptr;
    /* Fill in upper bits of command line address, NOP on 32 bit */
    boot_params->ext_cmd_line_ptr = (u64)(unsigned long)cmdline_ptr >> 32;
    @@ -669,10 +668,12 @@ struct boot_params *make_boot_params(str
    boot_params->ext_ramdisk_size = (u64)ramdisk_size >> 32;

    return boot_params;
    +
    fail2:
    efi_free(sys_table, options_size, hdr->cmd_line_ptr);
    fail:
    efi_free(sys_table, 0x4000, (unsigned long)boot_params);
    +
    return NULL;
    }

    @@ -684,7 +685,7 @@ static void add_e820ext(struct boot_para
    unsigned long size;

    e820ext->type = SETUP_E820_EXT;
    - e820ext->len = nr_entries * sizeof(struct boot_e820_entry);
    + e820ext->len = nr_entries * sizeof(struct boot_e820_entry);
    e820ext->next = 0;

    data = (struct setup_data *)(unsigned long)params->hdr.setup_data;
    @@ -698,8 +699,8 @@ static void add_e820ext(struct boot_para
    params->hdr.setup_data = (unsigned long)e820ext;
    }

    -static efi_status_t setup_e820(struct boot_params *params,
    - struct setup_data *e820ext, u32 e820ext_size)
    +static efi_status_t
    +setup_e820(struct boot_params *params, struct setup_data *e820ext, u32 e820ext_size)
    {
    struct boot_e820_entry *entry = params->e820_table;
    struct efi_info *efi = &params->efi_info;
    @@ -820,11 +821,11 @@ static efi_status_t alloc_e820ext(u32 nr
    }

    struct exit_boot_struct {
    - struct boot_params *boot_params;
    - struct efi_info *efi;
    - struct setup_data *e820ext;
    - __u32 e820ext_size;
    - bool is64;
    + struct boot_params *boot_params;
    + struct efi_info *efi;
    + struct setup_data *e820ext;
    + __u32 e820ext_size;
    + bool is64;
    };

    static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
    @@ -854,15 +855,15 @@ static efi_status_t exit_boot_func(efi_s
    signature = p->is64 ? EFI64_LOADER_SIGNATURE : EFI32_LOADER_SIGNATURE;
    memcpy(&p->efi->efi_loader_signature, signature, sizeof(__u32));

    - p->efi->efi_systab = (unsigned long)sys_table_arg;
    - p->efi->efi_memdesc_size = *map->desc_size;
    - p->efi->efi_memdesc_version = *map->desc_ver;
    - p->efi->efi_memmap = (unsigned long)*map->map;
    - p->efi->efi_memmap_size = *map->map_size;
    + p->efi->efi_systab = (unsigned long)sys_table_arg;
    + p->efi->efi_memdesc_size = *map->desc_size;
    + p->efi->efi_memdesc_version = *map->desc_ver;
    + p->efi->efi_memmap = (unsigned long)*map->map;
    + p->efi->efi_memmap_size = *map->map_size;

    #ifdef CONFIG_X86_64
    - p->efi->efi_systab_hi = (unsigned long)sys_table_arg >> 32;
    - p->efi->efi_memmap_hi = (unsigned long)*map->map >> 32;
    + p->efi->efi_systab_hi = (unsigned long)sys_table_arg >> 32;
    + p->efi->efi_memmap_hi = (unsigned long)*map->map >> 32;
    #endif

    return EFI_SUCCESS;
    @@ -880,17 +881,17 @@ static efi_status_t exit_boot(struct boo
    struct efi_boot_memmap map;
    struct exit_boot_struct priv;

    - map.map = &mem_map;
    - map.map_size = &map_sz;
    - map.desc_size = &desc_size;
    - map.desc_ver = &desc_version;
    - map.key_ptr = &key;
    - map.buff_size = &buff_size;
    - priv.boot_params = boot_params;
    - priv.efi = &boot_params->efi_info;
    - priv.e820ext = NULL;
    - priv.e820ext_size = 0;
    - priv.is64 = is64;
    + map.map = &mem_map;
    + map.map_size = &map_sz;
    + map.desc_size = &desc_size;
    + map.desc_ver = &desc_version;
    + map.key_ptr = &key;
    + map.buff_size = &buff_size;
    + priv.boot_params = boot_params;
    + priv.efi = &boot_params->efi_info;
    + priv.e820ext = NULL;
    + priv.e820ext_size = 0;
    + priv.is64 = is64;

    /* Might as well exit boot services now */
    status = efi_exit_boot_services(sys_table, handle, &map, &priv,
    @@ -898,10 +899,11 @@ static efi_status_t exit_boot(struct boo
    if (status != EFI_SUCCESS)
    return status;

    - e820ext = priv.e820ext;
    - e820ext_size = priv.e820ext_size;
    + e820ext = priv.e820ext;
    + e820ext_size = priv.e820ext_size;
    +
    /* Historic? */
    - boot_params->alt_mem_k = 32 * 1024;
    + boot_params->alt_mem_k = 32 * 1024;

    status = setup_e820(boot_params, e820ext, e820ext_size);
    if (status != EFI_SUCCESS)
    @@ -914,8 +916,8 @@ static efi_status_t exit_boot(struct boo
    * On success we return a pointer to a boot_params structure, and NULL
    * on failure.
    */
    -struct boot_params *efi_main(struct efi_config *c,
    - struct boot_params *boot_params)
    +struct boot_params *
    +efi_main(struct efi_config *c, struct boot_params *boot_params)
    {
    struct desc_ptr *gdt = NULL;
    efi_loaded_image_t *image;
    @@ -963,7 +965,7 @@ struct boot_params *efi_main(struct efi_
    status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
    sizeof(*gdt), (void **)&gdt);
    if (status != EFI_SUCCESS) {
    - efi_printk(sys_table, "Failed to alloc mem for gdt structure\n");
    + efi_printk(sys_table, "Failed to allocate memory for 'gdt' structure\n");
    goto fail;
    }

    @@ -971,7 +973,7 @@ struct boot_params *efi_main(struct efi_
    status = efi_low_alloc(sys_table, gdt->size, 8,
    (unsigned long *)&gdt->address);
    if (status != EFI_SUCCESS) {
    - efi_printk(sys_table, "Failed to alloc mem for gdt\n");
    + efi_printk(sys_table, "Failed to allocate memory for 'gdt'\n");
    goto fail;
    }

    @@ -1008,19 +1010,20 @@ struct boot_params *efi_main(struct efi_

    if (IS_ENABLED(CONFIG_X86_64)) {
    /* __KERNEL32_CS */
    - desc->limit0 = 0xffff;
    - desc->base0 = 0x0000;
    - desc->base1 = 0x0000;
    - desc->type = SEG_TYPE_CODE | SEG_TYPE_EXEC_READ;
    - desc->s = DESC_TYPE_CODE_DATA;
    - desc->dpl = 0;
    - desc->p = 1;
    - desc->limit1 = 0xf;
    - desc->avl = 0;
    - desc->l = 0;
    - desc->d = SEG_OP_SIZE_32BIT;
    - desc->g = SEG_GRANULARITY_4KB;
    - desc->base2 = 0x00;
    + desc->limit0 = 0xffff;
    + desc->base0 = 0x0000;
    + desc->base1 = 0x0000;
    + desc->type = SEG_TYPE_CODE | SEG_TYPE_EXEC_READ;
    + desc->s = DESC_TYPE_CODE_DATA;
    + desc->dpl = 0;
    + desc->p = 1;
    + desc->limit1 = 0xf;
    + desc->avl = 0;
    + desc->l = 0;
    + desc->d = SEG_OP_SIZE_32BIT;
    + desc->g = SEG_GRANULARITY_4KB;
    + desc->base2 = 0x00;
    +
    desc++;
    } else {
    /* Second entry is unused on 32-bit */
    @@ -1028,15 +1031,16 @@ struct boot_params *efi_main(struct efi_
    }

    /* __KERNEL_CS */
    - desc->limit0 = 0xffff;
    - desc->base0 = 0x0000;
    - desc->base1 = 0x0000;
    - desc->type = SEG_TYPE_CODE | SEG_TYPE_EXEC_READ;
    - desc->s = DESC_TYPE_CODE_DATA;
    - desc->dpl = 0;
    - desc->p = 1;
    - desc->limit1 = 0xf;
    - desc->avl = 0;
    + desc->limit0 = 0xffff;
    + desc->base0 = 0x0000;
    + desc->base1 = 0x0000;
    + desc->type = SEG_TYPE_CODE | SEG_TYPE_EXEC_READ;
    + desc->s = DESC_TYPE_CODE_DATA;
    + desc->dpl = 0;
    + desc->p = 1;
    + desc->limit1 = 0xf;
    + desc->avl = 0;
    +
    if (IS_ENABLED(CONFIG_X86_64)) {
    desc->l = 1;
    desc->d = 0;
    @@ -1044,41 +1048,41 @@ struct boot_params *efi_main(struct efi_
    desc->l = 0;
    desc->d = SEG_OP_SIZE_32BIT;
    }
    - desc->g = SEG_GRANULARITY_4KB;
    - desc->base2 = 0x00;
    + desc->g = SEG_GRANULARITY_4KB;
    + desc->base2 = 0x00;
    desc++;

    /* __KERNEL_DS */
    - desc->limit0 = 0xffff;
    - desc->base0 = 0x0000;
    - desc->base1 = 0x0000;
    - desc->type = SEG_TYPE_DATA | SEG_TYPE_READ_WRITE;
    - desc->s = DESC_TYPE_CODE_DATA;
    - desc->dpl = 0;
    - desc->p = 1;
    - desc->limit1 = 0xf;
    - desc->avl = 0;
    - desc->l = 0;
    - desc->d = SEG_OP_SIZE_32BIT;
    - desc->g = SEG_GRANULARITY_4KB;
    - desc->base2 = 0x00;
    + desc->limit0 = 0xffff;
    + desc->base0 = 0x0000;
    + desc->base1 = 0x0000;
    + desc->type = SEG_TYPE_DATA | SEG_TYPE_READ_WRITE;
    + desc->s = DESC_TYPE_CODE_DATA;
    + desc->dpl = 0;
    + desc->p = 1;
    + desc->limit1 = 0xf;
    + desc->avl = 0;
    + desc->l = 0;
    + desc->d = SEG_OP_SIZE_32BIT;
    + desc->g = SEG_GRANULARITY_4KB;
    + desc->base2 = 0x00;
    desc++;

    if (IS_ENABLED(CONFIG_X86_64)) {
    /* Task segment value */
    - desc->limit0 = 0x0000;
    - desc->base0 = 0x0000;
    - desc->base1 = 0x0000;
    - desc->type = SEG_TYPE_TSS;
    - desc->s = 0;
    - desc->dpl = 0;
    - desc->p = 1;
    - desc->limit1 = 0x0;
    - desc->avl = 0;
    - desc->l = 0;
    - desc->d = 0;
    - desc->g = SEG_GRANULARITY_4KB;
    - desc->base2 = 0x00;
    + desc->limit0 = 0x0000;
    + desc->base0 = 0x0000;
    + desc->base1 = 0x0000;
    + desc->type = SEG_TYPE_TSS;
    + desc->s = 0;
    + desc->dpl = 0;
    + desc->p = 1;
    + desc->limit1 = 0x0;
    + desc->avl = 0;
    + desc->l = 0;
    + desc->d = 0;
    + desc->g = SEG_GRANULARITY_4KB;
    + desc->base2 = 0x00;
    desc++;
    }

    @@ -1088,5 +1092,6 @@ struct boot_params *efi_main(struct efi_
    return boot_params;
    fail:
    efi_printk(sys_table, "efi_main() failed!\n");
    +
    return NULL;
    }
    \
     
     \ /
      Last update: 2018-05-14 08:44    [W:4.415 / U:0.108 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site