jcs.org / qemu
qemu with hax to log dma reads & writes jcs.org/2018/11/12/vfio
fork atom

fw-cfg: turn FW_CFG_FILE_SLOTS into a device property

We'd like to raise the value of FW_CFG_FILE_SLOTS. Doing it naively could
lead to problems with backward migration: a more recent QEMU (running an
older machine type) would allow the guest, in fw_cfg_select(), to select a
high key value that is unavailable in the same machine type implemented by
the older (target) QEMU. On the target host, fw_cfg_data_read() for
example could dereference nonexistent entries.

As first step, size the FWCfgState.entries[*] and FWCfgState.entry_order
arrays dynamically. All three array sizes will be influenced by the new
field FWCfgState.file_slots (and matching device property).

Make the following changes:

- Replace the FW_CFG_FILE_SLOTS macro with FW_CFG_FILE_SLOTS_MIN (minimum
count of fw_cfg file slots) in the header file. The value remains 0x10.

- Replace all uses of FW_CFG_FILE_SLOTS with a helper function called
fw_cfg_file_slots(), returning the new property.

- Eliminate the macro FW_CFG_MAX_ENTRY, and replace all its uses with a
helper function called fw_cfg_max_entry().

- In the MMIO- and IO-mapped realize functions both, allocate all three
arrays dynamically, based on the new property.

- The new property defaults to FW_CFG_FILE_SLOTS_MIN. This is going to be
customized in the following patches.

Cc: "Gabriel L. Somlo" <somlo@cmu.edu>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Gabriel Somlo <somlo@cmu.edu>
Tested-by: Gabriel Somlo <somlo@cmu.edu>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

authored by

Laszlo Ersek and committed by
Michael S. Tsirkin e12f3a13 baf2d5bf

+65 -11
+1 -1
docs/specs/fw_cfg.txt
··· 156 156 0xc000 - 0xffff Arch. Specific (0x0000 - 0x3fff, RW, ignored in v2.4+) 157 157 158 158 In practice, the number of allowed firmware configuration items is given 159 - by the value of FW_CFG_MAX_ENTRY (see fw_cfg.h). 159 + by the value (FW_CFG_FILE_FIRST + FW_CFG_FILE_SLOTS_MIN) (see fw_cfg.h). 160 160 161 161 = Guest-side DMA Interface = 162 162
+63 -8
hw/nvram/fw_cfg.c
··· 33 33 #include "qemu/error-report.h" 34 34 #include "qemu/config-file.h" 35 35 #include "qemu/cutils.h" 36 + #include "qapi/error.h" 36 37 37 38 #define FW_CFG_NAME "fw_cfg" 38 39 #define FW_CFG_PATH "/machine/" FW_CFG_NAME ··· 71 72 SysBusDevice parent_obj; 72 73 /*< public >*/ 73 74 74 - FWCfgEntry entries[2][FW_CFG_MAX_ENTRY]; 75 - int entry_order[FW_CFG_MAX_ENTRY]; 75 + uint16_t file_slots; 76 + FWCfgEntry *entries[2]; 77 + int *entry_order; 76 78 FWCfgFiles *files; 77 79 uint16_t cur_entry; 78 80 uint32_t cur_offset; ··· 257 259 /* nothing, write support removed in QEMU v2.4+ */ 258 260 } 259 261 262 + static inline uint16_t fw_cfg_file_slots(const FWCfgState *s) 263 + { 264 + return s->file_slots; 265 + } 266 + 267 + /* Note: this function returns an exclusive limit. */ 268 + static inline uint32_t fw_cfg_max_entry(const FWCfgState *s) 269 + { 270 + return FW_CFG_FILE_FIRST + fw_cfg_file_slots(s); 271 + } 272 + 260 273 static int fw_cfg_select(FWCfgState *s, uint16_t key) 261 274 { 262 275 int arch, ret; 263 276 FWCfgEntry *e; 264 277 265 278 s->cur_offset = 0; 266 - if ((key & FW_CFG_ENTRY_MASK) >= FW_CFG_MAX_ENTRY) { 279 + if ((key & FW_CFG_ENTRY_MASK) >= fw_cfg_max_entry(s)) { 267 280 s->cur_entry = FW_CFG_INVALID; 268 281 ret = 0; 269 282 } else { ··· 610 623 611 624 key &= FW_CFG_ENTRY_MASK; 612 625 613 - assert(key < FW_CFG_MAX_ENTRY && len < UINT32_MAX); 626 + assert(key < fw_cfg_max_entry(s) && len < UINT32_MAX); 614 627 assert(s->entries[arch][key].data == NULL); /* avoid key conflict */ 615 628 616 629 s->entries[arch][key].data = data; ··· 628 641 629 642 key &= FW_CFG_ENTRY_MASK; 630 643 631 - assert(key < FW_CFG_MAX_ENTRY && len < UINT32_MAX); 644 + assert(key < fw_cfg_max_entry(s) && len < UINT32_MAX); 632 645 633 646 /* return the old data to the function caller, avoid memory leak */ 634 647 ptr = s->entries[arch][key].data; ··· 777 790 int order = 0; 778 791 779 792 if (!s->files) { 780 - dsize = sizeof(uint32_t) + sizeof(FWCfgFile) * FW_CFG_FILE_SLOTS; 793 + dsize = sizeof(uint32_t) + sizeof(FWCfgFile) * fw_cfg_file_slots(s); 781 794 s->files = g_malloc0(dsize); 782 795 fw_cfg_add_bytes(s, FW_CFG_FILE_DIR, s->files, dsize); 783 796 } 784 797 785 798 count = be32_to_cpu(s->files->count); 786 - assert(count < FW_CFG_FILE_SLOTS); 799 + assert(count < fw_cfg_file_slots(s)); 787 800 788 801 /* Find the insertion point. */ 789 802 if (mc->legacy_fw_cfg_order) { ··· 857 870 assert(s->files); 858 871 859 872 index = be32_to_cpu(s->files->count); 860 - assert(index < FW_CFG_FILE_SLOTS); 873 + assert(index < fw_cfg_file_slots(s)); 861 874 862 875 for (i = 0; i < index; i++) { 863 876 if (strcmp(filename, s->files->f[i].name) == 0) { ··· 1014 1027 .class_init = fw_cfg_class_init, 1015 1028 }; 1016 1029 1030 + static void fw_cfg_file_slots_allocate(FWCfgState *s, Error **errp) 1031 + { 1032 + uint16_t file_slots_max; 1033 + 1034 + if (fw_cfg_file_slots(s) < FW_CFG_FILE_SLOTS_MIN) { 1035 + error_setg(errp, "\"file_slots\" must be at least 0x%x", 1036 + FW_CFG_FILE_SLOTS_MIN); 1037 + return; 1038 + } 1039 + 1040 + /* (UINT16_MAX & FW_CFG_ENTRY_MASK) is the highest inclusive selector value 1041 + * that we permit. The actual (exclusive) value coming from the 1042 + * configuration is (FW_CFG_FILE_FIRST + fw_cfg_file_slots(s)). */ 1043 + file_slots_max = (UINT16_MAX & FW_CFG_ENTRY_MASK) - FW_CFG_FILE_FIRST + 1; 1044 + if (fw_cfg_file_slots(s) > file_slots_max) { 1045 + error_setg(errp, "\"file_slots\" must not exceed 0x%" PRIx16, 1046 + file_slots_max); 1047 + return; 1048 + } 1049 + 1050 + s->entries[0] = g_new0(FWCfgEntry, fw_cfg_max_entry(s)); 1051 + s->entries[1] = g_new0(FWCfgEntry, fw_cfg_max_entry(s)); 1052 + s->entry_order = g_new0(int, fw_cfg_max_entry(s)); 1053 + } 1017 1054 1018 1055 static Property fw_cfg_io_properties[] = { 1019 1056 DEFINE_PROP_UINT32("iobase", FWCfgIoState, iobase, -1), 1020 1057 DEFINE_PROP_UINT32("dma_iobase", FWCfgIoState, dma_iobase, -1), 1021 1058 DEFINE_PROP_BOOL("dma_enabled", FWCfgIoState, parent_obj.dma_enabled, 1022 1059 true), 1060 + DEFINE_PROP_UINT16("x-file-slots", FWCfgIoState, parent_obj.file_slots, 1061 + FW_CFG_FILE_SLOTS_MIN), 1023 1062 DEFINE_PROP_END_OF_LIST(), 1024 1063 }; 1025 1064 ··· 1027 1066 { 1028 1067 FWCfgIoState *s = FW_CFG_IO(dev); 1029 1068 SysBusDevice *sbd = SYS_BUS_DEVICE(dev); 1069 + Error *local_err = NULL; 1070 + 1071 + fw_cfg_file_slots_allocate(FW_CFG(s), &local_err); 1072 + if (local_err) { 1073 + error_propagate(errp, local_err); 1074 + return; 1075 + } 1030 1076 1031 1077 /* when using port i/o, the 8-bit data register ALWAYS overlaps 1032 1078 * with half of the 16-bit control register. Hence, the total size ··· 1063 1109 DEFINE_PROP_UINT32("data_width", FWCfgMemState, data_width, -1), 1064 1110 DEFINE_PROP_BOOL("dma_enabled", FWCfgMemState, parent_obj.dma_enabled, 1065 1111 true), 1112 + DEFINE_PROP_UINT16("x-file-slots", FWCfgMemState, parent_obj.file_slots, 1113 + FW_CFG_FILE_SLOTS_MIN), 1066 1114 DEFINE_PROP_END_OF_LIST(), 1067 1115 }; 1068 1116 ··· 1071 1119 FWCfgMemState *s = FW_CFG_MEM(dev); 1072 1120 SysBusDevice *sbd = SYS_BUS_DEVICE(dev); 1073 1121 const MemoryRegionOps *data_ops = &fw_cfg_data_mem_ops; 1122 + Error *local_err = NULL; 1123 + 1124 + fw_cfg_file_slots_allocate(FW_CFG(s), &local_err); 1125 + if (local_err) { 1126 + error_propagate(errp, local_err); 1127 + return; 1128 + } 1074 1129 1075 1130 memory_region_init_io(&s->ctl_iomem, OBJECT(s), &fw_cfg_ctl_mem_ops, 1076 1131 FW_CFG(s), "fwcfg.ctl", FW_CFG_CTL_SIZE);
+1 -2
include/hw/nvram/fw_cfg_keys.h
··· 29 29 #define FW_CFG_FILE_DIR 0x19 30 30 31 31 #define FW_CFG_FILE_FIRST 0x20 32 - #define FW_CFG_FILE_SLOTS 0x10 33 - #define FW_CFG_MAX_ENTRY (FW_CFG_FILE_FIRST + FW_CFG_FILE_SLOTS) 32 + #define FW_CFG_FILE_SLOTS_MIN 0x10 34 33 35 34 #define FW_CFG_WRITE_CHANNEL 0x4000 36 35 #define FW_CFG_ARCH_LOCAL 0x8000