|
| From: | Corey Minyard |
| Subject: | Re: [Qemu-devel] [PATCH v3] Sort the fw_cfg file list |
| Date: | Thu, 17 Mar 2016 16:30:52 -0500 |
| User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 |
On 03/17/2016 02:58 PM, Michael S. Tsirkin wrote:
On Thu, Mar 17, 2016 at 11:41:45AM -0500, Corey Minyard wrote:On 03/17/2016 11:02 AM, Michael S. Tsirkin wrote:On Wed, Mar 16, 2016 at 01:47:08PM -0500, address@hidden wrote:From: Gerd Hoffmann <address@hidden> Entries are inserted in filename order instead of being appended to the end in case sorting is enabled. This will avoid any future issues of moving the file creation around, it doesn't matter what order they are created now, the will always be in filename order. Signed-off-by: Gerd Hoffmann <address@hidden> Added machine type handling for compatibility. This was a fairly complex change, this will preserve the order of fw_cfg for older versions no matter what order the firmware files actually come in. A list is kept of the correct legacy order and the entries will be inserted based upon their order in the list. Except that some entries are ordered (in a specific area of the list) based upon what order they appear on the command line. Special handling is added for those entries. Signed-off-by: Corey Minyard <address@hidden> --- A new version of the sorted fw_cfg, with new backwards compatibility code. This is bigger than I would like, but I can't think of a good way to split it up that would make much sense. This is more for review, anyway, I'm sure there are issues with this. I'm not too excited about fw_cfg_set_order_override(), but I can't think of a better way to handle this. You can't use filename or prefixes, the same filenames appear in different places for VGA ROMs depending on if they are PCI or ISA.I don't see another good solution either. However, I would like to make these NOPs with new machine types. This way down the road we can drop this mess.They should be NOPs with new machine types. These only have an effect if legacy_fw_cfg_order is set in the machine class, and that's only set for older machines. -coreyI didn't realize this. OK. From layering point of view, I think I would like machines to call rom_set_order_override(). And then the fw cfg one should get fw cfg argument.
Do you mean you want to add rom_set_order_override() function that gets called from most places that then calls fw_cfg_set_order_override() with the fw_cfg as an argument? Yeah, I can see that makes sense. Then you can put the override order variable in FWCfgState.
hw/i386/pc.c | 4 ++ hw/i386/pc_piix.c | 1 + hw/i386/pc_q35.c | 1 + hw/nvram/fw_cfg.c | 127 +++++++++++++++++++++++++++++++++++++++++++--- include/hw/boards.h | 3 +- include/hw/nvram/fw_cfg.h | 7 +++ vl.c | 4 ++ 7 files changed, 138 insertions(+), 9 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 56ec6cd..707753b 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1406,6 +1406,7 @@ DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus) { DeviceState *dev = NULL; + fw_cfg_set_order_override(FW_CFG_ORDER_OVERRIDE_VGA); if (pci_bus) { PCIDevice *pcidev = pci_vga_init(pci_bus); dev = pcidev ? &pcidev->qdev : NULL; @@ -1413,6 +1414,7 @@ DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus) ISADevice *isadev = isa_vga_init(isa_bus); dev = isadev ? DEVICE(isadev) : NULL; } + fw_cfg_reset_order_override(); return dev; } @@ -1541,6 +1543,7 @@ void pc_nic_init(ISABus *isa_bus, PCIBus *pci_bus) { int i; + fw_cfg_set_order_override(FW_CFG_ORDER_OVERRIDE_NIC); for (i = 0; i < nb_nics; i++) { NICInfo *nd = &nd_table[i]; @@ -1550,6 +1553,7 @@ void pc_nic_init(ISABus *isa_bus, PCIBus *pci_bus) pci_nic_init_nofail(nd, pci_bus, "e1000", NULL); } } + fw_cfg_reset_order_override(); } void pc_pci_device_init(PCIBus *pci_bus) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 6f8c2cd..447703b 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -429,6 +429,7 @@ static void pc_i440fx_2_5_machine_options(MachineClass *m) m->alias = NULL; m->is_default = 0; pcmc->save_tsc_khz = false; + m->legacy_fw_cfg_order = 1; SET_MACHINE_COMPAT(m, PC_COMPAT_2_5); } diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 46522c9..04f3609 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -291,6 +291,7 @@ static void pc_q35_2_5_machine_options(MachineClass *m) pc_q35_2_6_machine_options(m); m->alias = NULL; pcmc->save_tsc_khz = false; + m->legacy_fw_cfg_order = 1; SET_MACHINE_COMPAT(m, PC_COMPAT_2_5); } diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index 7866248..1693f26 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -28,6 +28,7 @@ #include "hw/isa/isa.h" #include "hw/nvram/fw_cfg.h" #include "hw/sysbus.h" +#include "hw/boards.h" #include "trace.h" #include "qemu/error-report.h" #include "qemu/config-file.h" @@ -68,6 +69,7 @@ struct FWCfgState { /*< public >*/ FWCfgEntry entries[2][FW_CFG_MAX_ENTRY]; + int entry_order[FW_CFG_MAX_ENTRY]; FWCfgFiles *files; uint16_t cur_entry; uint32_t cur_offset; @@ -664,12 +666,88 @@ void fw_cfg_add_i64(FWCfgState *s, uint16_t key, uint64_t value) fw_cfg_add_bytes(s, key, copy, sizeof(value)); } +static int fw_cfg_order_override = -1; + +void fw_cfg_set_order_override(int order) +{ + assert(fw_cfg_order_override == -1); + fw_cfg_order_override = order; +} + +void fw_cfg_reset_order_override(void) +{ + assert(fw_cfg_order_override != -1); + fw_cfg_order_override = -1; +} + +/* + * This is the legacy order list. For legacy systems, files are in + * the fw_cfg in the order defined below, by the "order" value. Note + * that some entries (VGA ROMs, NIC option ROMS, etc.) go into a + * specific area, but there may be more than one and they occur in the + * order that the user specifies them on the command line. Those are + * handled in a special manner, using the order override above. + * + * For non-legacy, the files are sorted by filename to avoid this kind + * of complexity in the future. + * + * This is only for x86, other arches don't implement versioning so + * they won't set legacy mode. + */ +static struct { + const char *name; + int order; +} fw_cfg_order[] = { + { "etc/boot-menu-wait", 10 }, + { "bootsplash.jpg", 11 }, + { "bootsplash.bmp", 12 }, + { "etc/boot-fail-wait", 15 }, + { "etc/smbios/smbios-tables", 20 }, + { "etc/smbios/smbios-anchor", 30 }, + { "etc/e820", 40 }, + { "etc/reserved-memory-end", 50 }, + { "genroms/linuxboot.bin", 60 }, + { }, /* VGA ROMs from pc_vga_init come here, 70. */ + { }, /* NIC option ROMs from pc_nic_init come here, 80. */ + { "etc/system-states", 90 }, + { }, /* User ROMs come here, 100. */ + { }, /* Device FW comes here, 110. */ + { "etc/extra-pci-roots", 120 }, + { "etc/acpi/tables", 130 }, + { "etc/table-loader", 140 }, + { "etc/tpm/log", 150 }, + { "etc/acpi/rsdp", 160 }, + { "bootorder", 170 }, + +#define FW_CFG_ORDER_OVERRIDE_LAST 200 +}; + +static int get_fw_cfg_order(const char *name) +{ + int i; + + if (fw_cfg_order_override >= 0) + return fw_cfg_order_override; + + for (i = 0; i < ARRAY_SIZE(fw_cfg_order); i++) { + if (fw_cfg_order[i].name == NULL) + continue; + if (strcmp(name, fw_cfg_order[i].name) == 0) + return fw_cfg_order[i].order; + } + /* Stick unknown stuff at the end. */ + error_report("warning: Unknown firmware file in legacy mode: %s\n", name); + return FW_CFG_ORDER_OVERRIDE_LAST; +} + void fw_cfg_add_file_callback(FWCfgState *s, const char *filename, FWCfgReadCallback callback, void *callback_opaque, void *data, size_t len) { - int i, index; + int i, index, count; size_t dsize; + MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); + int order = 0; if (!s->files) { dsize = sizeof(uint32_t) + sizeof(FWCfgFile) * FW_CFG_FILE_SLOTS; @@ -677,13 +755,45 @@ void fw_cfg_add_file_callback(FWCfgState *s, const char *filename, fw_cfg_add_bytes(s, FW_CFG_FILE_DIR, s->files, dsize); } - index = be32_to_cpu(s->files->count); - assert(index < FW_CFG_FILE_SLOTS); + count = be32_to_cpu(s->files->count); + assert(count < FW_CFG_FILE_SLOTS); - pstrcpy(s->files->f[index].name, sizeof(s->files->f[index].name), - filename); - for (i = 0; i < index; i++) { - if (strcmp(s->files->f[index].name, s->files->f[i].name) == 0) { + /* Find the insertion point. */ + if (mc->legacy_fw_cfg_order) { + /* Sort by order, and keep insertion order for the same order. */Pls rephrase for readability.
Yeah, reading that again, it's pretty hard to parse. -corey
+ order = get_fw_cfg_order(filename); + for (index = count; + index > 0 && order < s->entry_order[index - 1]; + index--); + } else { + /* Sort by file name. */ + for (index = count; + index > 0 && strcmp(filename, s->files->f[index - 1].name) < 0; + index--); + } + + /* + * Move all the entries from the index point and after down one + * to create a slot for the new entry. Because calculations are + * being done with the index, make it so that "i" is the current + * index and "i - 1" is the one being copied from, thus the + * unusual start and end in the for statement. + */ + for (i = count + 1; i > index; i--) { + s->files->f[i] = s->files->f[i - 1]; + s->files->f[i].select = cpu_to_be16(FW_CFG_FILE_FIRST + i); + s->entries[0][FW_CFG_FILE_FIRST + i] = + s->entries[0][FW_CFG_FILE_FIRST + i - 1]; + s->entry_order[i] = s->entry_order[i - 1]; + } + + memset(&s->files->f[index], 0, sizeof(FWCfgFile)); + memset(&s->entries[0][FW_CFG_FILE_FIRST + index], 0, sizeof(FWCfgEntry)); + + pstrcpy(s->files->f[index].name, sizeof(s->files->f[index].name), filename); + for (i = 0; i <= count; i++) { + if (i != index && + strcmp(s->files->f[index].name, s->files->f[i].name) == 0) { error_report("duplicate fw_cfg file name: %s", s->files->f[index].name); exit(1); @@ -695,9 +805,10 @@ void fw_cfg_add_file_callback(FWCfgState *s, const char *filename, s->files->f[index].size = cpu_to_be32(len); s->files->f[index].select = cpu_to_be16(FW_CFG_FILE_FIRST + index); + s->entry_order[index] = order; trace_fw_cfg_add_file(s, index, s->files->f[index].name, len); - s->files->count = cpu_to_be32(index+1); + s->files->count = cpu_to_be32(count+1); } void fw_cfg_add_file(FWCfgState *s, const char *filename, diff --git a/include/hw/boards.h b/include/hw/boards.h index b5d7eae..b6567f7 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -84,7 +84,8 @@ struct MachineClass { no_cdrom:1, no_sdcard:1, has_dynamic_sysbus:1, - pci_allow_0_address:1; + pci_allow_0_address:1, + legacy_fw_cfg_order:1; int is_default; const char *default_machine_opts; const char *default_boot_order; diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h index 4315f4e..38bbfca 100644 --- a/include/hw/nvram/fw_cfg.h +++ b/include/hw/nvram/fw_cfg.h @@ -57,6 +57,13 @@ typedef struct FWCfgFile { char name[FW_CFG_MAX_FILE_PATH]; } FWCfgFile; +#define FW_CFG_ORDER_OVERRIDE_VGA 70 +#define FW_CFG_ORDER_OVERRIDE_NIC 80 +#define FW_CFG_ORDER_OVERRIDE_USER 100 +#define FW_CFG_ORDER_OVERRIDE_DEVICE 110 +void fw_cfg_set_order_override(int order); +void fw_cfg_reset_order_override(void); + typedef struct FWCfgFiles { uint32_t count; FWCfgFile f[]; diff --git a/vl.c b/vl.c index 7a28982..6fad844 100644 --- a/vl.c +++ b/vl.c @@ -4520,10 +4520,12 @@ int main(int argc, char **argv, char **envp) numa_post_machine_init(); + fw_cfg_set_order_override(FW_CFG_ORDER_OVERRIDE_USER); if (qemu_opts_foreach(qemu_find_opts("fw_cfg"), parse_fw_cfg, fw_cfg_find(), NULL) != 0) { exit(1); } + fw_cfg_reset_order_override(); /* init USB devices */ if (usb_enabled()) { @@ -4535,10 +4537,12 @@ int main(int argc, char **argv, char **envp) igd_gfx_passthru(); /* init generic devices */ + fw_cfg_set_order_override(FW_CFG_ORDER_OVERRIDE_DEVICE); if (qemu_opts_foreach(qemu_find_opts("device"), device_init_func, NULL, NULL)) { exit(1); } + fw_cfg_reset_order_override(); /* Did we create any drives that we failed to create a device for? */ drive_check_orphaned(); -- 2.5.0
| [Prev in Thread] | Current Thread | [Next in Thread] |