| Message ID | 1458154028-4934-1-git-send-email-minyard@acm.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 16, 2016 at 01:47:08PM -0500, minyard@acm.org wrote: > From: Gerd Hoffmann <kraxel@redhat.com> > > 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 <kraxel@redhat.com> > > 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 <cminyard@mvista.com> > --- > > 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. > 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. */ > + 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 >
On 03/17/2016 11:02 AM, Michael S. Tsirkin wrote: > On Wed, Mar 16, 2016 at 01:47:08PM -0500, minyard@acm.org wrote: >> From: Gerd Hoffmann <kraxel@redhat.com> >> >> 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 <kraxel@redhat.com> >> >> 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 <cminyard@mvista.com> >> --- >> >> 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. -corey > >> 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. */ >> + 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 >>
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, minyard@acm.org wrote: > >>From: Gerd Hoffmann <kraxel@redhat.com> > >> > >>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 <kraxel@redhat.com> > >> > >>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 <cminyard@mvista.com> > >>--- > >> > >>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. > > -corey I 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. > > > >> 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. > >>+ 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 > >>
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, minyard@acm.org wrote: >>>> From: Gerd Hoffmann <kraxel@redhat.com> >>>> >>>> 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 <kraxel@redhat.com> >>>> >>>> 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 <cminyard@mvista.com> >>>> --- >>>> >>>> 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. >> >> -corey > I 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 >>>>
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. */ + 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();