Message ID | 1452827541-10537-2-git-send-email-yanmiaobest@gmail.com |
---|---|
State | Superseded |
Delegated to: | Bin Meng |
Headers | show |
Hi Miao, On Fri, Jan 15, 2016 at 11:12 AM, Miao Yan <yanmiaobest@gmail.com> wrote: > Re-write the logic in qemu_fwcfg_list_firmware(), add a function > qemu_cfg_read_firmware_list() to handle reading firmware list. qemu_fwcfg_read_firmware_list() > > Signed-off-by: Miao Yan <yanmiaobest@gmail.com> > --- > arch/x86/cpu/qemu/fw_cfg.c | 60 +++++++++++++++++++++++++++++++++---------- > arch/x86/include/asm/fw_cfg.h | 8 ++++++ > 2 files changed, 54 insertions(+), 14 deletions(-) > > diff --git a/arch/x86/cpu/qemu/fw_cfg.c b/arch/x86/cpu/qemu/fw_cfg.c > index 0599214..b22026c 100644 > --- a/arch/x86/cpu/qemu/fw_cfg.c > +++ b/arch/x86/cpu/qemu/fw_cfg.c > @@ -10,10 +10,13 @@ > #include <malloc.h> > #include <asm/io.h> > #include <asm/fw_cfg.h> > +#include <linux/list.h> > > static bool fwcfg_present; > static bool fwcfg_dma_present; > > +static LIST_HEAD(fw_list); > + > /* Read configuration item using fw_cfg PIO interface */ > static void qemu_fwcfg_read_entry_pio(uint16_t entry, > uint32_t size, void *address) > @@ -162,29 +165,58 @@ static int qemu_fwcfg_setup_kernel(void *load_addr, void *initrd_addr) > return 0; > } > > -static int qemu_fwcfg_list_firmware(void) > +static int qemu_fwcfg_read_firmware_list(void) > { > int i; > uint32_t count; > - struct fw_cfg_files *files; > + struct fw_file *file; > + struct list_head *entry; > + > + /* don't read it twice */ > + if (!list_empty(&fw_list)) > + return 0; > > qemu_fwcfg_read_entry(FW_CFG_FILE_DIR, 4, &count); > if (!count) > return 0; > > count = be32_to_cpu(count); > - files = malloc(count * sizeof(struct fw_cfg_file)); > - if (!files) > - return -ENOMEM; > - > - files->count = count; > - qemu_fwcfg_read_entry(FW_CFG_INVALID, > - count * sizeof(struct fw_cfg_file), > - files->files); > - > - for (i = 0; i < files->count; i++) > - printf("%-56s\n", files->files[i].name); > - free(files); > + for (i = 0; i < count; i++) { > + file = malloc(sizeof(*file)); > + if (!file) { > + printf("error: allocating resource\n"); > + goto err; > + } > + qemu_fwcfg_read_entry(FW_CFG_INVALID, > + sizeof(struct fw_cfg_file), &file->cfg); > + file->addr = 0; > + list_add_tail(&file->list, &fw_list); > + } nits: leave one blank line before return > + return 0; > + > +err: > + list_for_each(entry, &fw_list) { > + file = list_entry(entry, struct fw_file, list); > + free(file); > + } nits: leave one blank line before return > + return -ENOMEM; > +} > + > +static int qemu_fwcfg_list_firmware(void) > +{ > + int ret; > + struct list_head *entry; > + struct fw_file *file; > + > + /* make sure fw_list is loaded */ > + ret = qemu_fwcfg_read_firmware_list(); > + if (ret) > + return ret; > + > + list_for_each(entry, &fw_list) { > + file = list_entry(entry, struct fw_file, list); > + printf("%-56s\n", file->cfg.name); > + } nits: leave one blank line before return > return 0; > } > > diff --git a/arch/x86/include/asm/fw_cfg.h b/arch/x86/include/asm/fw_cfg.h > index fb110fa..285d805 100644 > --- a/arch/x86/include/asm/fw_cfg.h > +++ b/arch/x86/include/asm/fw_cfg.h > @@ -12,6 +12,8 @@ > #define FW_DMA_PORT_LOW 0x514 > #define FW_DMA_PORT_HIGH 0x518 > > +#include <linux/list.h> > + > enum qemu_fwcfg_items { > FW_CFG_SIGNATURE = 0x00, > FW_CFG_ID = 0x01, > @@ -67,6 +69,12 @@ struct fw_cfg_file { > char name[FW_CFG_MAX_FILE_PATH]; > }; > > +struct fw_file { > + struct fw_cfg_file cfg; > + unsigned long addr; > + struct list_head list; > +}; Can you please put a comment block above to describe what is each member for? > + > struct fw_cfg_files { Looks that this struct fw_cfg_files is no longer used by any codes. Please remove this. > __be32 count; > struct fw_cfg_file files[]; > -- Regards, Bin
diff --git a/arch/x86/cpu/qemu/fw_cfg.c b/arch/x86/cpu/qemu/fw_cfg.c index 0599214..b22026c 100644 --- a/arch/x86/cpu/qemu/fw_cfg.c +++ b/arch/x86/cpu/qemu/fw_cfg.c @@ -10,10 +10,13 @@ #include <malloc.h> #include <asm/io.h> #include <asm/fw_cfg.h> +#include <linux/list.h> static bool fwcfg_present; static bool fwcfg_dma_present; +static LIST_HEAD(fw_list); + /* Read configuration item using fw_cfg PIO interface */ static void qemu_fwcfg_read_entry_pio(uint16_t entry, uint32_t size, void *address) @@ -162,29 +165,58 @@ static int qemu_fwcfg_setup_kernel(void *load_addr, void *initrd_addr) return 0; } -static int qemu_fwcfg_list_firmware(void) +static int qemu_fwcfg_read_firmware_list(void) { int i; uint32_t count; - struct fw_cfg_files *files; + struct fw_file *file; + struct list_head *entry; + + /* don't read it twice */ + if (!list_empty(&fw_list)) + return 0; qemu_fwcfg_read_entry(FW_CFG_FILE_DIR, 4, &count); if (!count) return 0; count = be32_to_cpu(count); - files = malloc(count * sizeof(struct fw_cfg_file)); - if (!files) - return -ENOMEM; - - files->count = count; - qemu_fwcfg_read_entry(FW_CFG_INVALID, - count * sizeof(struct fw_cfg_file), - files->files); - - for (i = 0; i < files->count; i++) - printf("%-56s\n", files->files[i].name); - free(files); + for (i = 0; i < count; i++) { + file = malloc(sizeof(*file)); + if (!file) { + printf("error: allocating resource\n"); + goto err; + } + qemu_fwcfg_read_entry(FW_CFG_INVALID, + sizeof(struct fw_cfg_file), &file->cfg); + file->addr = 0; + list_add_tail(&file->list, &fw_list); + } + return 0; + +err: + list_for_each(entry, &fw_list) { + file = list_entry(entry, struct fw_file, list); + free(file); + } + return -ENOMEM; +} + +static int qemu_fwcfg_list_firmware(void) +{ + int ret; + struct list_head *entry; + struct fw_file *file; + + /* make sure fw_list is loaded */ + ret = qemu_fwcfg_read_firmware_list(); + if (ret) + return ret; + + list_for_each(entry, &fw_list) { + file = list_entry(entry, struct fw_file, list); + printf("%-56s\n", file->cfg.name); + } return 0; } diff --git a/arch/x86/include/asm/fw_cfg.h b/arch/x86/include/asm/fw_cfg.h index fb110fa..285d805 100644 --- a/arch/x86/include/asm/fw_cfg.h +++ b/arch/x86/include/asm/fw_cfg.h @@ -12,6 +12,8 @@ #define FW_DMA_PORT_LOW 0x514 #define FW_DMA_PORT_HIGH 0x518 +#include <linux/list.h> + enum qemu_fwcfg_items { FW_CFG_SIGNATURE = 0x00, FW_CFG_ID = 0x01, @@ -67,6 +69,12 @@ struct fw_cfg_file { char name[FW_CFG_MAX_FILE_PATH]; }; +struct fw_file { + struct fw_cfg_file cfg; + unsigned long addr; + struct list_head list; +}; + struct fw_cfg_files { __be32 count; struct fw_cfg_file files[];
Re-write the logic in qemu_fwcfg_list_firmware(), add a function qemu_cfg_read_firmware_list() to handle reading firmware list. Signed-off-by: Miao Yan <yanmiaobest@gmail.com> --- arch/x86/cpu/qemu/fw_cfg.c | 60 +++++++++++++++++++++++++++++++++---------- arch/x86/include/asm/fw_cfg.h | 8 ++++++ 2 files changed, 54 insertions(+), 14 deletions(-)