* [Qemu-arm] [PATCH v5 wave 1 0/4] fw-cfg: support writeable blobs and more files @ 2017-01-11 17:34 ` Laszlo Ersek 0 siblings, 0 replies; 20+ messages in thread From: Laszlo Ersek @ 2017-01-11 17:34 UTC (permalink / raw) To: qemu devel list Cc: Peter Maydell, Stefano Stabellini, Eduardo Habkost, Michael S. Tsirkin, Gabriel L. Somlo, Shannon Zhao, Mark Cave-Ayland, Alexander Graf, Michael Walle, qemu-arm, Gerd Hoffmann, Paolo Bonzini, Anthony Perard, Igor Mammedov, Artyom Tarasenko, David Gibson This is the first (fw_cfg) half of the v5 iteration of the series posted here: <http://lists.nongnu.org/archive/html/qemu-devel/2016-12/msg00129.html>. In this version, the fw_cfg patches have been separated into a standalone "wave", for helping review / maintenance, and also for enabling independent features on top of writeable blobs. More importantly, I've addressed Igor's v4 feedback. See the individual patches for the details. Patch #3 is included verbatim from Eduardo's pending series (see the patch notes for the archive URL), as a dependency for patch #4. If Eduardo's series is merged first, patch #3 can be dropped (in fact git-rebase should do it automatically). Please excuse the surprisingly long list of CC's, it's due to the fact that fw_cfg is quite widely used (see patch #4). Cc: "Gabriel L. Somlo" <somlo@cmu.edu> Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Alexander Graf <agraf@suse.de> Cc: Anthony Perard <anthony.perard@citrix.com> Cc: Artyom Tarasenko <atar4qemu@gmail.com> Cc: David Gibson <david@gibson.dropbear.id.au> Cc: Eduardo Habkost <ehabkost@redhat.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Igor Mammedov <imammedo@redhat.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Cc: Michael Walle <michael@walle.cc> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: Shannon Zhao <zhaoshenglong@huawei.com> Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: qemu-arm@nongnu.org Thanks Laszlo Eduardo Habkost (1): pc: Add 2.9 machine-types Laszlo Ersek (2): fw-cfg: turn FW_CFG_FILE_SLOTS into a device property fw-cfg: bump "file_slots" to 0x20 for 2.9+ machine types Michael S. Tsirkin (1): fw-cfg: support writeable blobs docs/specs/fw_cfg.txt | 36 ++++++++++---- hw/lm32/lm32_hwsetup.h | 2 +- include/hw/compat.h | 10 +++- include/hw/i386/pc.h | 2 + include/hw/loader.h | 7 +-- include/hw/nvram/fw_cfg.h | 3 +- include/hw/nvram/fw_cfg_keys.h | 3 +- hw/arm/virt-acpi-build.c | 2 +- hw/core/loader.c | 18 ++++--- hw/i386/acpi-build.c | 4 +- hw/i386/pc_piix.c | 15 ++++-- hw/i386/pc_q35.c | 13 ++++- hw/nvram/fw_cfg.c | 110 +++++++++++++++++++++++++++++++++++------ 13 files changed, 177 insertions(+), 48 deletions(-) -- 2.9.3 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v5 wave 1 0/4] fw-cfg: support writeable blobs and more files @ 2017-01-11 17:34 ` Laszlo Ersek 0 siblings, 0 replies; 20+ messages in thread From: Laszlo Ersek @ 2017-01-11 17:34 UTC (permalink / raw) To: qemu devel list Cc: Gabriel L. Somlo, Michael S. Tsirkin, Alexander Graf, Anthony Perard, Artyom Tarasenko, David Gibson, Eduardo Habkost, Gerd Hoffmann, Igor Mammedov, Mark Cave-Ayland, Michael Walle, Paolo Bonzini, Peter Maydell, Shannon Zhao, Stefano Stabellini, qemu-arm This is the first (fw_cfg) half of the v5 iteration of the series posted here: <http://lists.nongnu.org/archive/html/qemu-devel/2016-12/msg00129.html>. In this version, the fw_cfg patches have been separated into a standalone "wave", for helping review / maintenance, and also for enabling independent features on top of writeable blobs. More importantly, I've addressed Igor's v4 feedback. See the individual patches for the details. Patch #3 is included verbatim from Eduardo's pending series (see the patch notes for the archive URL), as a dependency for patch #4. If Eduardo's series is merged first, patch #3 can be dropped (in fact git-rebase should do it automatically). Please excuse the surprisingly long list of CC's, it's due to the fact that fw_cfg is quite widely used (see patch #4). Cc: "Gabriel L. Somlo" <somlo@cmu.edu> Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Alexander Graf <agraf@suse.de> Cc: Anthony Perard <anthony.perard@citrix.com> Cc: Artyom Tarasenko <atar4qemu@gmail.com> Cc: David Gibson <david@gibson.dropbear.id.au> Cc: Eduardo Habkost <ehabkost@redhat.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Igor Mammedov <imammedo@redhat.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Cc: Michael Walle <michael@walle.cc> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: Shannon Zhao <zhaoshenglong@huawei.com> Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: qemu-arm@nongnu.org Thanks Laszlo Eduardo Habkost (1): pc: Add 2.9 machine-types Laszlo Ersek (2): fw-cfg: turn FW_CFG_FILE_SLOTS into a device property fw-cfg: bump "file_slots" to 0x20 for 2.9+ machine types Michael S. Tsirkin (1): fw-cfg: support writeable blobs docs/specs/fw_cfg.txt | 36 ++++++++++---- hw/lm32/lm32_hwsetup.h | 2 +- include/hw/compat.h | 10 +++- include/hw/i386/pc.h | 2 + include/hw/loader.h | 7 +-- include/hw/nvram/fw_cfg.h | 3 +- include/hw/nvram/fw_cfg_keys.h | 3 +- hw/arm/virt-acpi-build.c | 2 +- hw/core/loader.c | 18 ++++--- hw/i386/acpi-build.c | 4 +- hw/i386/pc_piix.c | 15 ++++-- hw/i386/pc_q35.c | 13 ++++- hw/nvram/fw_cfg.c | 110 +++++++++++++++++++++++++++++++++++------ 13 files changed, 177 insertions(+), 48 deletions(-) -- 2.9.3 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-arm] [PATCH v5 wave 1 1/4] fw-cfg: support writeable blobs 2017-01-11 17:34 ` [Qemu-devel] " Laszlo Ersek @ 2017-01-11 17:34 ` Laszlo Ersek -1 siblings, 0 replies; 20+ messages in thread From: Laszlo Ersek @ 2017-01-11 17:34 UTC (permalink / raw) To: qemu devel list Cc: Peter Maydell, Michael S. Tsirkin, Gabriel L. Somlo, Shannon Zhao, Michael Walle, qemu-arm, Gerd Hoffmann, Paolo Bonzini, Igor Mammedov From: "Michael S. Tsirkin" <mst@redhat.com> Useful to send guest data back to QEMU. Changes from Laszlo Ersek <lersek@redhat.com>: - rebase the patch from Michael Tsirkin's original postings at [1] and [2] to the following patches: - loader: Allow a custom AddressSpace when loading ROMs - loader: Add AddressSpace loading support to uImages - loader: fix handling of custom address spaces when adding ROM blobs - reject such writes immediately that would exceed the end of the array, rather than performing a partial write before setting the error bit: see the (len != dma.length) condition - document the write interface [1] http://lists.nongnu.org/archive/html/qemu-devel/2016-02/msg04968.html [2] http://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg02735.html 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: Michael Walle <michael@walle.cc> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: Shannon Zhao <zhaoshenglong@huawei.com> Cc: qemu-arm@nongnu.org Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Marcel Apfelbaum <marcel@redhat.com> --- Notes: v5: - no changes, just picked up Marcel's R-b docs/specs/fw_cfg.txt | 32 +++++++++++++++++++++++++------- hw/lm32/lm32_hwsetup.h | 2 +- include/hw/loader.h | 7 ++++--- include/hw/nvram/fw_cfg.h | 3 ++- hw/arm/virt-acpi-build.c | 2 +- hw/core/loader.c | 18 +++++++++++------- hw/i386/acpi-build.c | 4 ++-- hw/nvram/fw_cfg.c | 37 +++++++++++++++++++++++++++++-------- 8 files changed, 75 insertions(+), 30 deletions(-) diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt index 7a5f8c7824ce..a19e2adbe1c6 100644 --- a/docs/specs/fw_cfg.txt +++ b/docs/specs/fw_cfg.txt @@ -33,6 +33,10 @@ the selector value is between 0x4000-0x7fff or 0xc000-0xffff. NOTE: As of QEMU v2.4, writes to the fw_cfg data register are no longer supported, and will be ignored (treated as no-ops)! +NOTE: As of QEMU v2.9, writes are reinstated, but only through the DMA + interface (see below). Furthermore, writeability of any specific item is + governed independently of Bit14 in the selector key value. + Bit15 of the selector register indicates whether the configuration setting is architecture specific. A value of 0 means the item is a generic configuration item. A value of 1 means the item is specific @@ -43,7 +47,7 @@ value between 0x8000-0xffff. == Data Register == -* Read/Write (writes ignored as of QEMU v2.4) +* Read/Write (writes ignored as of QEMU v2.4, but see the DMA interface) * Location: platform dependent (IOport [*] or MMIO) * Width: 8-bit (if IOport), 8/16/32/64-bit (if MMIO) * Endianness: string-preserving @@ -134,8 +138,8 @@ struct FWCfgFile { /* an individual file entry, 64 bytes total */ === All Other Data Items === -Please consult the QEMU source for the most up-to-date and authoritative -list of selector keys and their respective items' purpose and format. +Please consult the QEMU source for the most up-to-date and authoritative list +of selector keys and their respective items' purpose, format and writeability. === Ranges === @@ -144,9 +148,11 @@ items, and up to 0x4000 architecturally specific ones. Selector Reg. Range Usage --------------- ----------- -0x0000 - 0x3fff Generic (0x0000 - 0x3fff, RO) +0x0000 - 0x3fff Generic (0x0000 - 0x3fff, generally RO, possibly RW through + the DMA interface in QEMU v2.9+) 0x4000 - 0x7fff Generic (0x0000 - 0x3fff, RW, ignored in QEMU v2.4+) -0x8000 - 0xbfff Arch. Specific (0x0000 - 0x3fff, RO) +0x8000 - 0xbfff Arch. Specific (0x0000 - 0x3fff, generally RO, possibly RW + through the DMA interface in QEMU v2.9+) 0xc000 - 0xffff Arch. Specific (0x0000 - 0x3fff, RW, ignored in v2.4+) In practice, the number of allowed firmware configuration items is given @@ -182,6 +188,7 @@ The "control" field has the following bits: - Bit 1: Read - Bit 2: Skip - Bit 3: Select. The upper 16 bits are the selected index. + - Bit 4: Write When an operation is triggered, if the "control" field has bit 3 set, the upper 16 bits are interpreted as an index of a firmware configuration item. @@ -191,8 +198,17 @@ If the "control" field has bit 1 set, a read operation will be performed. "length" bytes for the current selector and offset will be copied into the physical RAM address specified by the "address" field. -If the "control" field has bit 2 set (and not bit 1), a skip operation will be -performed. The offset for the current selector will be advanced "length" bytes. +If the "control" field has bit 4 set (and not bit 1), a write operation will be +performed. "length" bytes will be copied from the physical RAM address +specified by the "address" field to the current selector and offset. QEMU +prevents starting or finishing the write beyond the end of the item associated +with the current selector (i.e., the item cannot be resized). Truncated writes +are dropped entirely. Writes to read-only items are also rejected. All of these +write errors set bit 0 (the error bit) in the "control" field. + +If the "control" field has bit 2 set (and neither bit 1 nor bit 4), a skip +operation will be performed. The offset for the current selector will be +advanced "length" bytes. To check the result, read the "control" field: error bit set -> something went wrong. @@ -234,3 +250,5 @@ Prefix "opt/org.qemu/" is reserved for QEMU itself. Use of names not beginning with "opt/" is potentially dangerous and entirely unsupported. QEMU will warn if you try. + +All externally provided fw_cfg items are read-only to the guest. diff --git a/hw/lm32/lm32_hwsetup.h b/hw/lm32/lm32_hwsetup.h index 23e18784dffd..a01f6bc5dfeb 100644 --- a/hw/lm32/lm32_hwsetup.h +++ b/hw/lm32/lm32_hwsetup.h @@ -75,7 +75,7 @@ static inline void hwsetup_create_rom(HWSetup *hw, hwaddr base) { rom_add_blob("hwsetup", hw->data, TARGET_PAGE_SIZE, - TARGET_PAGE_SIZE, base, NULL, NULL, NULL, NULL); + TARGET_PAGE_SIZE, base, NULL, NULL, NULL, NULL, true); } static inline void hwsetup_add_u8(HWSetup *hw, uint8_t u) diff --git a/include/hw/loader.h b/include/hw/loader.h index 0c864cfd6046..0dbd8d6bf37a 100644 --- a/include/hw/loader.h +++ b/include/hw/loader.h @@ -180,7 +180,8 @@ MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len, size_t max_len, hwaddr addr, const char *fw_file_name, FWCfgReadCallback fw_callback, - void *callback_opaque, AddressSpace *as); + void *callback_opaque, AddressSpace *as, + bool read_only); int rom_add_elf_program(const char *name, void *data, size_t datasize, size_t romsize, hwaddr addr, AddressSpace *as); int rom_check_and_register_reset(void); @@ -194,7 +195,7 @@ void hmp_info_roms(Monitor *mon, const QDict *qdict); #define rom_add_file_fixed(_f, _a, _i) \ rom_add_file(_f, NULL, _a, _i, false, NULL, NULL) #define rom_add_blob_fixed(_f, _b, _l, _a) \ - rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL, NULL) + rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL, NULL, true) #define rom_add_file_mr(_f, _mr, _i) \ rom_add_file(_f, NULL, 0, _i, false, _mr, NULL) #define rom_add_file_as(_f, _as, _i) \ @@ -202,7 +203,7 @@ void hmp_info_roms(Monitor *mon, const QDict *qdict); #define rom_add_file_fixed_as(_f, _a, _i, _as) \ rom_add_file(_f, NULL, _a, _i, false, NULL, _as) #define rom_add_blob_fixed_as(_f, _b, _l, _a, _as) \ - rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL, _as) + rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL, _as, true) #define PC_ROM_MIN_VGA 0xc0000 #define PC_ROM_MIN_OPTION 0xc8000 diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h index 5c27a1f0d50b..b980cbaebf43 100644 --- a/include/hw/nvram/fw_cfg.h +++ b/include/hw/nvram/fw_cfg.h @@ -136,6 +136,7 @@ void fw_cfg_add_file(FWCfgState *s, const char *filename, void *data, * @callback_opaque: argument to be passed into callback function * @data: pointer to start of item data * @len: size of item data + * @read_only: is file read only * * Add a new NAMED fw_cfg item as a raw "blob" of the given size. The data * referenced by the starting pointer is only linked, NOT copied, into the @@ -151,7 +152,7 @@ void fw_cfg_add_file(FWCfgState *s, const char *filename, void *data, */ void fw_cfg_add_file_callback(FWCfgState *s, const char *filename, FWCfgReadCallback callback, void *callback_opaque, - void *data, size_t len); + void *data, size_t len, bool read_only); /** * fw_cfg_modify_file: diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 085a61117378..205d6268e217 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -818,7 +818,7 @@ static MemoryRegion *acpi_add_rom_blob(AcpiBuildState *build_state, uint64_t max_size) { return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1, - name, virt_acpi_build_update, build_state, NULL); + name, virt_acpi_build_update, build_state, NULL, true); } static const VMStateDescription vmstate_virt_acpi_build = { diff --git a/hw/core/loader.c b/hw/core/loader.c index 45742494e6fd..ee5abd6eb7f4 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -853,7 +853,7 @@ static void fw_cfg_resized(const char *id, uint64_t length, void *host) } } -static void *rom_set_mr(Rom *rom, Object *owner, const char *name) +static void *rom_set_mr(Rom *rom, Object *owner, const char *name, bool ro) { void *data; @@ -862,7 +862,7 @@ static void *rom_set_mr(Rom *rom, Object *owner, const char *name) rom->datasize, rom->romsize, fw_cfg_resized, &error_fatal); - memory_region_set_readonly(rom->mr, true); + memory_region_set_readonly(rom->mr, ro); vmstate_register_ram_global(rom->mr); data = memory_region_get_ram_ptr(rom->mr); @@ -942,7 +942,7 @@ int rom_add_file(const char *file, const char *fw_dir, snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name); if ((!option_rom || mc->option_rom_has_mr) && mc->rom_file_has_mr) { - data = rom_set_mr(rom, OBJECT(fw_cfg), devpath); + data = rom_set_mr(rom, OBJECT(fw_cfg), devpath, true); } else { data = rom->data; } @@ -979,7 +979,7 @@ err: MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len, size_t max_len, hwaddr addr, const char *fw_file_name, FWCfgReadCallback fw_callback, void *callback_opaque, - AddressSpace *as) + AddressSpace *as, bool read_only) { MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); Rom *rom; @@ -998,10 +998,14 @@ MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len, char devpath[100]; void *data; - snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name); + if (read_only) { + snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name); + } else { + snprintf(devpath, sizeof(devpath), "/ram@%s", fw_file_name); + } if (mc->rom_file_has_mr) { - data = rom_set_mr(rom, OBJECT(fw_cfg), devpath); + data = rom_set_mr(rom, OBJECT(fw_cfg), devpath, read_only); mr = rom->mr; } else { data = rom->data; @@ -1009,7 +1013,7 @@ MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len, fw_cfg_add_file_callback(fw_cfg, fw_file_name, fw_callback, callback_opaque, - data, rom->datasize); + data, rom->datasize, read_only); } return mr; } diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 42ecf619d523..20d70fa114a7 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -2936,7 +2936,7 @@ static MemoryRegion *acpi_add_rom_blob(AcpiBuildState *build_state, uint64_t max_size) { return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1, - name, acpi_build_update, build_state, NULL); + name, acpi_build_update, build_state, NULL, true); } static const VMStateDescription vmstate_acpi_build = { @@ -3002,7 +3002,7 @@ void acpi_setup(void) build_state->rsdp = g_memdup(tables.rsdp->data, rsdp_size); fw_cfg_add_file_callback(pcms->fw_cfg, ACPI_BUILD_RSDP_FILE, acpi_build_update, build_state, - build_state->rsdp, rsdp_size); + build_state->rsdp, rsdp_size, true); build_state->rsdp_mr = NULL; } else { build_state->rsdp = NULL; diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index 3ebecb22606a..e0145c11a19b 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -54,11 +54,13 @@ #define FW_CFG_DMA_CTL_READ 0x02 #define FW_CFG_DMA_CTL_SKIP 0x04 #define FW_CFG_DMA_CTL_SELECT 0x08 +#define FW_CFG_DMA_CTL_WRITE 0x10 #define FW_CFG_DMA_SIGNATURE 0x51454d5520434647ULL /* "QEMU CFG" */ typedef struct FWCfgEntry { uint32_t len; + bool allow_write; uint8_t *data; void *callback_opaque; FWCfgReadCallback read_callback; @@ -326,7 +328,7 @@ static void fw_cfg_dma_transfer(FWCfgState *s) FWCfgDmaAccess dma; int arch; FWCfgEntry *e; - int read; + int read = 0, write = 0; dma_addr_t dma_addr; /* Reset the address before the next access */ @@ -353,8 +355,13 @@ static void fw_cfg_dma_transfer(FWCfgState *s) if (dma.control & FW_CFG_DMA_CTL_READ) { read = 1; + write = 0; + } else if (dma.control & FW_CFG_DMA_CTL_WRITE) { + read = 0; + write = 1; } else if (dma.control & FW_CFG_DMA_CTL_SKIP) { read = 0; + write = 0; } else { dma.length = 0; } @@ -374,7 +381,9 @@ static void fw_cfg_dma_transfer(FWCfgState *s) dma.control |= FW_CFG_DMA_CTL_ERROR; } } - + if (write) { + dma.control |= FW_CFG_DMA_CTL_ERROR; + } } else { if (dma.length <= (e->len - s->cur_offset)) { len = dma.length; @@ -391,6 +400,14 @@ static void fw_cfg_dma_transfer(FWCfgState *s) dma.control |= FW_CFG_DMA_CTL_ERROR; } } + if (write) { + if (!e->allow_write || + len != dma.length || + dma_memory_read(s->dma_as, dma.address, + &e->data[s->cur_offset], len)) { + dma.control |= FW_CFG_DMA_CTL_ERROR; + } + } s->cur_offset += len; } @@ -586,7 +603,8 @@ static const VMStateDescription vmstate_fw_cfg = { static void fw_cfg_add_bytes_read_callback(FWCfgState *s, uint16_t key, FWCfgReadCallback callback, void *callback_opaque, - void *data, size_t len) + void *data, size_t len, + bool read_only) { int arch = !!(key & FW_CFG_ARCH_LOCAL); @@ -599,6 +617,7 @@ static void fw_cfg_add_bytes_read_callback(FWCfgState *s, uint16_t key, s->entries[arch][key].len = (uint32_t)len; s->entries[arch][key].read_callback = callback; s->entries[arch][key].callback_opaque = callback_opaque; + s->entries[arch][key].allow_write = !read_only; } static void *fw_cfg_modify_bytes_read(FWCfgState *s, uint16_t key, @@ -616,13 +635,14 @@ static void *fw_cfg_modify_bytes_read(FWCfgState *s, uint16_t key, s->entries[arch][key].data = data; s->entries[arch][key].len = len; s->entries[arch][key].callback_opaque = NULL; + s->entries[arch][key].allow_write = false; return ptr; } void fw_cfg_add_bytes(FWCfgState *s, uint16_t key, void *data, size_t len) { - fw_cfg_add_bytes_read_callback(s, key, NULL, NULL, data, len); + fw_cfg_add_bytes_read_callback(s, key, NULL, NULL, data, len, true); } void fw_cfg_add_string(FWCfgState *s, uint16_t key, const char *value) @@ -749,7 +769,7 @@ static int get_fw_cfg_order(FWCfgState *s, const char *name) void fw_cfg_add_file_callback(FWCfgState *s, const char *filename, FWCfgReadCallback callback, void *callback_opaque, - void *data, size_t len) + void *data, size_t len, bool read_only) { int i, index, count; size_t dsize; @@ -811,7 +831,8 @@ void fw_cfg_add_file_callback(FWCfgState *s, const char *filename, } fw_cfg_add_bytes_read_callback(s, FW_CFG_FILE_FIRST + index, - callback, callback_opaque, data, len); + callback, callback_opaque, data, len, + read_only); s->files->f[index].size = cpu_to_be32(len); s->files->f[index].select = cpu_to_be16(FW_CFG_FILE_FIRST + index); @@ -824,7 +845,7 @@ void fw_cfg_add_file_callback(FWCfgState *s, const char *filename, void fw_cfg_add_file(FWCfgState *s, const char *filename, void *data, size_t len) { - fw_cfg_add_file_callback(s, filename, NULL, NULL, data, len); + fw_cfg_add_file_callback(s, filename, NULL, NULL, data, len, true); } void *fw_cfg_modify_file(FWCfgState *s, const char *filename, @@ -847,7 +868,7 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename, } } /* add new one */ - fw_cfg_add_file_callback(s, filename, NULL, NULL, data, len); + fw_cfg_add_file_callback(s, filename, NULL, NULL, data, len, true); return NULL; } -- 2.9.3 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v5 wave 1 1/4] fw-cfg: support writeable blobs @ 2017-01-11 17:34 ` Laszlo Ersek 0 siblings, 0 replies; 20+ messages in thread From: Laszlo Ersek @ 2017-01-11 17:34 UTC (permalink / raw) To: qemu devel list Cc: Gabriel L. Somlo, Michael S. Tsirkin, Gerd Hoffmann, Igor Mammedov, Michael Walle, Paolo Bonzini, Peter Maydell, Shannon Zhao, qemu-arm From: "Michael S. Tsirkin" <mst@redhat.com> Useful to send guest data back to QEMU. Changes from Laszlo Ersek <lersek@redhat.com>: - rebase the patch from Michael Tsirkin's original postings at [1] and [2] to the following patches: - loader: Allow a custom AddressSpace when loading ROMs - loader: Add AddressSpace loading support to uImages - loader: fix handling of custom address spaces when adding ROM blobs - reject such writes immediately that would exceed the end of the array, rather than performing a partial write before setting the error bit: see the (len != dma.length) condition - document the write interface [1] http://lists.nongnu.org/archive/html/qemu-devel/2016-02/msg04968.html [2] http://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg02735.html 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: Michael Walle <michael@walle.cc> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: Shannon Zhao <zhaoshenglong@huawei.com> Cc: qemu-arm@nongnu.org Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Marcel Apfelbaum <marcel@redhat.com> --- Notes: v5: - no changes, just picked up Marcel's R-b docs/specs/fw_cfg.txt | 32 +++++++++++++++++++++++++------- hw/lm32/lm32_hwsetup.h | 2 +- include/hw/loader.h | 7 ++++--- include/hw/nvram/fw_cfg.h | 3 ++- hw/arm/virt-acpi-build.c | 2 +- hw/core/loader.c | 18 +++++++++++------- hw/i386/acpi-build.c | 4 ++-- hw/nvram/fw_cfg.c | 37 +++++++++++++++++++++++++++++-------- 8 files changed, 75 insertions(+), 30 deletions(-) diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt index 7a5f8c7824ce..a19e2adbe1c6 100644 --- a/docs/specs/fw_cfg.txt +++ b/docs/specs/fw_cfg.txt @@ -33,6 +33,10 @@ the selector value is between 0x4000-0x7fff or 0xc000-0xffff. NOTE: As of QEMU v2.4, writes to the fw_cfg data register are no longer supported, and will be ignored (treated as no-ops)! +NOTE: As of QEMU v2.9, writes are reinstated, but only through the DMA + interface (see below). Furthermore, writeability of any specific item is + governed independently of Bit14 in the selector key value. + Bit15 of the selector register indicates whether the configuration setting is architecture specific. A value of 0 means the item is a generic configuration item. A value of 1 means the item is specific @@ -43,7 +47,7 @@ value between 0x8000-0xffff. == Data Register == -* Read/Write (writes ignored as of QEMU v2.4) +* Read/Write (writes ignored as of QEMU v2.4, but see the DMA interface) * Location: platform dependent (IOport [*] or MMIO) * Width: 8-bit (if IOport), 8/16/32/64-bit (if MMIO) * Endianness: string-preserving @@ -134,8 +138,8 @@ struct FWCfgFile { /* an individual file entry, 64 bytes total */ === All Other Data Items === -Please consult the QEMU source for the most up-to-date and authoritative -list of selector keys and their respective items' purpose and format. +Please consult the QEMU source for the most up-to-date and authoritative list +of selector keys and their respective items' purpose, format and writeability. === Ranges === @@ -144,9 +148,11 @@ items, and up to 0x4000 architecturally specific ones. Selector Reg. Range Usage --------------- ----------- -0x0000 - 0x3fff Generic (0x0000 - 0x3fff, RO) +0x0000 - 0x3fff Generic (0x0000 - 0x3fff, generally RO, possibly RW through + the DMA interface in QEMU v2.9+) 0x4000 - 0x7fff Generic (0x0000 - 0x3fff, RW, ignored in QEMU v2.4+) -0x8000 - 0xbfff Arch. Specific (0x0000 - 0x3fff, RO) +0x8000 - 0xbfff Arch. Specific (0x0000 - 0x3fff, generally RO, possibly RW + through the DMA interface in QEMU v2.9+) 0xc000 - 0xffff Arch. Specific (0x0000 - 0x3fff, RW, ignored in v2.4+) In practice, the number of allowed firmware configuration items is given @@ -182,6 +188,7 @@ The "control" field has the following bits: - Bit 1: Read - Bit 2: Skip - Bit 3: Select. The upper 16 bits are the selected index. + - Bit 4: Write When an operation is triggered, if the "control" field has bit 3 set, the upper 16 bits are interpreted as an index of a firmware configuration item. @@ -191,8 +198,17 @@ If the "control" field has bit 1 set, a read operation will be performed. "length" bytes for the current selector and offset will be copied into the physical RAM address specified by the "address" field. -If the "control" field has bit 2 set (and not bit 1), a skip operation will be -performed. The offset for the current selector will be advanced "length" bytes. +If the "control" field has bit 4 set (and not bit 1), a write operation will be +performed. "length" bytes will be copied from the physical RAM address +specified by the "address" field to the current selector and offset. QEMU +prevents starting or finishing the write beyond the end of the item associated +with the current selector (i.e., the item cannot be resized). Truncated writes +are dropped entirely. Writes to read-only items are also rejected. All of these +write errors set bit 0 (the error bit) in the "control" field. + +If the "control" field has bit 2 set (and neither bit 1 nor bit 4), a skip +operation will be performed. The offset for the current selector will be +advanced "length" bytes. To check the result, read the "control" field: error bit set -> something went wrong. @@ -234,3 +250,5 @@ Prefix "opt/org.qemu/" is reserved for QEMU itself. Use of names not beginning with "opt/" is potentially dangerous and entirely unsupported. QEMU will warn if you try. + +All externally provided fw_cfg items are read-only to the guest. diff --git a/hw/lm32/lm32_hwsetup.h b/hw/lm32/lm32_hwsetup.h index 23e18784dffd..a01f6bc5dfeb 100644 --- a/hw/lm32/lm32_hwsetup.h +++ b/hw/lm32/lm32_hwsetup.h @@ -75,7 +75,7 @@ static inline void hwsetup_create_rom(HWSetup *hw, hwaddr base) { rom_add_blob("hwsetup", hw->data, TARGET_PAGE_SIZE, - TARGET_PAGE_SIZE, base, NULL, NULL, NULL, NULL); + TARGET_PAGE_SIZE, base, NULL, NULL, NULL, NULL, true); } static inline void hwsetup_add_u8(HWSetup *hw, uint8_t u) diff --git a/include/hw/loader.h b/include/hw/loader.h index 0c864cfd6046..0dbd8d6bf37a 100644 --- a/include/hw/loader.h +++ b/include/hw/loader.h @@ -180,7 +180,8 @@ MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len, size_t max_len, hwaddr addr, const char *fw_file_name, FWCfgReadCallback fw_callback, - void *callback_opaque, AddressSpace *as); + void *callback_opaque, AddressSpace *as, + bool read_only); int rom_add_elf_program(const char *name, void *data, size_t datasize, size_t romsize, hwaddr addr, AddressSpace *as); int rom_check_and_register_reset(void); @@ -194,7 +195,7 @@ void hmp_info_roms(Monitor *mon, const QDict *qdict); #define rom_add_file_fixed(_f, _a, _i) \ rom_add_file(_f, NULL, _a, _i, false, NULL, NULL) #define rom_add_blob_fixed(_f, _b, _l, _a) \ - rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL, NULL) + rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL, NULL, true) #define rom_add_file_mr(_f, _mr, _i) \ rom_add_file(_f, NULL, 0, _i, false, _mr, NULL) #define rom_add_file_as(_f, _as, _i) \ @@ -202,7 +203,7 @@ void hmp_info_roms(Monitor *mon, const QDict *qdict); #define rom_add_file_fixed_as(_f, _a, _i, _as) \ rom_add_file(_f, NULL, _a, _i, false, NULL, _as) #define rom_add_blob_fixed_as(_f, _b, _l, _a, _as) \ - rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL, _as) + rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL, _as, true) #define PC_ROM_MIN_VGA 0xc0000 #define PC_ROM_MIN_OPTION 0xc8000 diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h index 5c27a1f0d50b..b980cbaebf43 100644 --- a/include/hw/nvram/fw_cfg.h +++ b/include/hw/nvram/fw_cfg.h @@ -136,6 +136,7 @@ void fw_cfg_add_file(FWCfgState *s, const char *filename, void *data, * @callback_opaque: argument to be passed into callback function * @data: pointer to start of item data * @len: size of item data + * @read_only: is file read only * * Add a new NAMED fw_cfg item as a raw "blob" of the given size. The data * referenced by the starting pointer is only linked, NOT copied, into the @@ -151,7 +152,7 @@ void fw_cfg_add_file(FWCfgState *s, const char *filename, void *data, */ void fw_cfg_add_file_callback(FWCfgState *s, const char *filename, FWCfgReadCallback callback, void *callback_opaque, - void *data, size_t len); + void *data, size_t len, bool read_only); /** * fw_cfg_modify_file: diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 085a61117378..205d6268e217 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -818,7 +818,7 @@ static MemoryRegion *acpi_add_rom_blob(AcpiBuildState *build_state, uint64_t max_size) { return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1, - name, virt_acpi_build_update, build_state, NULL); + name, virt_acpi_build_update, build_state, NULL, true); } static const VMStateDescription vmstate_virt_acpi_build = { diff --git a/hw/core/loader.c b/hw/core/loader.c index 45742494e6fd..ee5abd6eb7f4 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -853,7 +853,7 @@ static void fw_cfg_resized(const char *id, uint64_t length, void *host) } } -static void *rom_set_mr(Rom *rom, Object *owner, const char *name) +static void *rom_set_mr(Rom *rom, Object *owner, const char *name, bool ro) { void *data; @@ -862,7 +862,7 @@ static void *rom_set_mr(Rom *rom, Object *owner, const char *name) rom->datasize, rom->romsize, fw_cfg_resized, &error_fatal); - memory_region_set_readonly(rom->mr, true); + memory_region_set_readonly(rom->mr, ro); vmstate_register_ram_global(rom->mr); data = memory_region_get_ram_ptr(rom->mr); @@ -942,7 +942,7 @@ int rom_add_file(const char *file, const char *fw_dir, snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name); if ((!option_rom || mc->option_rom_has_mr) && mc->rom_file_has_mr) { - data = rom_set_mr(rom, OBJECT(fw_cfg), devpath); + data = rom_set_mr(rom, OBJECT(fw_cfg), devpath, true); } else { data = rom->data; } @@ -979,7 +979,7 @@ err: MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len, size_t max_len, hwaddr addr, const char *fw_file_name, FWCfgReadCallback fw_callback, void *callback_opaque, - AddressSpace *as) + AddressSpace *as, bool read_only) { MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); Rom *rom; @@ -998,10 +998,14 @@ MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len, char devpath[100]; void *data; - snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name); + if (read_only) { + snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name); + } else { + snprintf(devpath, sizeof(devpath), "/ram@%s", fw_file_name); + } if (mc->rom_file_has_mr) { - data = rom_set_mr(rom, OBJECT(fw_cfg), devpath); + data = rom_set_mr(rom, OBJECT(fw_cfg), devpath, read_only); mr = rom->mr; } else { data = rom->data; @@ -1009,7 +1013,7 @@ MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len, fw_cfg_add_file_callback(fw_cfg, fw_file_name, fw_callback, callback_opaque, - data, rom->datasize); + data, rom->datasize, read_only); } return mr; } diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 42ecf619d523..20d70fa114a7 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -2936,7 +2936,7 @@ static MemoryRegion *acpi_add_rom_blob(AcpiBuildState *build_state, uint64_t max_size) { return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1, - name, acpi_build_update, build_state, NULL); + name, acpi_build_update, build_state, NULL, true); } static const VMStateDescription vmstate_acpi_build = { @@ -3002,7 +3002,7 @@ void acpi_setup(void) build_state->rsdp = g_memdup(tables.rsdp->data, rsdp_size); fw_cfg_add_file_callback(pcms->fw_cfg, ACPI_BUILD_RSDP_FILE, acpi_build_update, build_state, - build_state->rsdp, rsdp_size); + build_state->rsdp, rsdp_size, true); build_state->rsdp_mr = NULL; } else { build_state->rsdp = NULL; diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index 3ebecb22606a..e0145c11a19b 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -54,11 +54,13 @@ #define FW_CFG_DMA_CTL_READ 0x02 #define FW_CFG_DMA_CTL_SKIP 0x04 #define FW_CFG_DMA_CTL_SELECT 0x08 +#define FW_CFG_DMA_CTL_WRITE 0x10 #define FW_CFG_DMA_SIGNATURE 0x51454d5520434647ULL /* "QEMU CFG" */ typedef struct FWCfgEntry { uint32_t len; + bool allow_write; uint8_t *data; void *callback_opaque; FWCfgReadCallback read_callback; @@ -326,7 +328,7 @@ static void fw_cfg_dma_transfer(FWCfgState *s) FWCfgDmaAccess dma; int arch; FWCfgEntry *e; - int read; + int read = 0, write = 0; dma_addr_t dma_addr; /* Reset the address before the next access */ @@ -353,8 +355,13 @@ static void fw_cfg_dma_transfer(FWCfgState *s) if (dma.control & FW_CFG_DMA_CTL_READ) { read = 1; + write = 0; + } else if (dma.control & FW_CFG_DMA_CTL_WRITE) { + read = 0; + write = 1; } else if (dma.control & FW_CFG_DMA_CTL_SKIP) { read = 0; + write = 0; } else { dma.length = 0; } @@ -374,7 +381,9 @@ static void fw_cfg_dma_transfer(FWCfgState *s) dma.control |= FW_CFG_DMA_CTL_ERROR; } } - + if (write) { + dma.control |= FW_CFG_DMA_CTL_ERROR; + } } else { if (dma.length <= (e->len - s->cur_offset)) { len = dma.length; @@ -391,6 +400,14 @@ static void fw_cfg_dma_transfer(FWCfgState *s) dma.control |= FW_CFG_DMA_CTL_ERROR; } } + if (write) { + if (!e->allow_write || + len != dma.length || + dma_memory_read(s->dma_as, dma.address, + &e->data[s->cur_offset], len)) { + dma.control |= FW_CFG_DMA_CTL_ERROR; + } + } s->cur_offset += len; } @@ -586,7 +603,8 @@ static const VMStateDescription vmstate_fw_cfg = { static void fw_cfg_add_bytes_read_callback(FWCfgState *s, uint16_t key, FWCfgReadCallback callback, void *callback_opaque, - void *data, size_t len) + void *data, size_t len, + bool read_only) { int arch = !!(key & FW_CFG_ARCH_LOCAL); @@ -599,6 +617,7 @@ static void fw_cfg_add_bytes_read_callback(FWCfgState *s, uint16_t key, s->entries[arch][key].len = (uint32_t)len; s->entries[arch][key].read_callback = callback; s->entries[arch][key].callback_opaque = callback_opaque; + s->entries[arch][key].allow_write = !read_only; } static void *fw_cfg_modify_bytes_read(FWCfgState *s, uint16_t key, @@ -616,13 +635,14 @@ static void *fw_cfg_modify_bytes_read(FWCfgState *s, uint16_t key, s->entries[arch][key].data = data; s->entries[arch][key].len = len; s->entries[arch][key].callback_opaque = NULL; + s->entries[arch][key].allow_write = false; return ptr; } void fw_cfg_add_bytes(FWCfgState *s, uint16_t key, void *data, size_t len) { - fw_cfg_add_bytes_read_callback(s, key, NULL, NULL, data, len); + fw_cfg_add_bytes_read_callback(s, key, NULL, NULL, data, len, true); } void fw_cfg_add_string(FWCfgState *s, uint16_t key, const char *value) @@ -749,7 +769,7 @@ static int get_fw_cfg_order(FWCfgState *s, const char *name) void fw_cfg_add_file_callback(FWCfgState *s, const char *filename, FWCfgReadCallback callback, void *callback_opaque, - void *data, size_t len) + void *data, size_t len, bool read_only) { int i, index, count; size_t dsize; @@ -811,7 +831,8 @@ void fw_cfg_add_file_callback(FWCfgState *s, const char *filename, } fw_cfg_add_bytes_read_callback(s, FW_CFG_FILE_FIRST + index, - callback, callback_opaque, data, len); + callback, callback_opaque, data, len, + read_only); s->files->f[index].size = cpu_to_be32(len); s->files->f[index].select = cpu_to_be16(FW_CFG_FILE_FIRST + index); @@ -824,7 +845,7 @@ void fw_cfg_add_file_callback(FWCfgState *s, const char *filename, void fw_cfg_add_file(FWCfgState *s, const char *filename, void *data, size_t len) { - fw_cfg_add_file_callback(s, filename, NULL, NULL, data, len); + fw_cfg_add_file_callback(s, filename, NULL, NULL, data, len, true); } void *fw_cfg_modify_file(FWCfgState *s, const char *filename, @@ -847,7 +868,7 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename, } } /* add new one */ - fw_cfg_add_file_callback(s, filename, NULL, NULL, data, len); + fw_cfg_add_file_callback(s, filename, NULL, NULL, data, len, true); return NULL; } -- 2.9.3 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v5 wave 1 2/4] fw-cfg: turn FW_CFG_FILE_SLOTS into a device property 2017-01-11 17:34 ` [Qemu-devel] " Laszlo Ersek (?) (?) @ 2017-01-11 17:34 ` Laszlo Ersek 2017-01-12 13:10 ` Eduardo Habkost 2017-01-12 14:29 ` Michael S. Tsirkin -1 siblings, 2 replies; 20+ messages in thread From: Laszlo Ersek @ 2017-01-11 17:34 UTC (permalink / raw) To: qemu devel list Cc: Gabriel L. Somlo, Michael S. Tsirkin, Gerd Hoffmann, Igor Mammedov, Paolo Bonzini 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 (and device property) FWCfgState.file_slots. 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> --- Notes: v5: - change the type of "FWCfgState.file_slots" to "uint16_t" [Igor] - same for the retval of the trivial wrapper function fw_cfg_file_slots(), and for the corresponding "file_slots" device properties - preserve the fw_cfg_file_slots() wrapper func (Igor was okay with it in the end) - rename FW_CFG_FILE_SLOTS_TRAD to FW_CFG_FILE_SLOTS_MIN - Remove explicit qdev_prop_set_uint32() calls from fw_cfg_init_io_dma() and fw_cfg_init_mem_wide(), but set the property default to FW_CFG_FILE_SLOTS_MIN (0x10). This is the main change relative to v4; the idea is that per-board opt-in shouldn't be necessary for an increased file_slots count *in addition to* selecting a 2.9 machine type. [Igor] - delay the now-unused FW_CFG_FILE_SLOTS_DFLT macro to a later patch v4: - I know that upstream doesn't care about backward migration, but some downstreams might. docs/specs/fw_cfg.txt | 2 +- include/hw/nvram/fw_cfg_keys.h | 3 +- hw/nvram/fw_cfg.c | 71 +++++++++++++++++++++++++++++++++++++----- 3 files changed, 65 insertions(+), 11 deletions(-) diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt index a19e2adbe1c6..9373bbc64743 100644 --- a/docs/specs/fw_cfg.txt +++ b/docs/specs/fw_cfg.txt @@ -156,7 +156,7 @@ Selector Reg. Range Usage 0xc000 - 0xffff Arch. Specific (0x0000 - 0x3fff, RW, ignored in v2.4+) In practice, the number of allowed firmware configuration items is given -by the value of FW_CFG_MAX_ENTRY (see fw_cfg.h). +by the value (FW_CFG_FILE_FIRST + FW_CFG_FILE_SLOTS_MIN) (see fw_cfg.h). = Guest-side DMA Interface = diff --git a/include/hw/nvram/fw_cfg_keys.h b/include/hw/nvram/fw_cfg_keys.h index 0f3e871884c0..b6919451f5bd 100644 --- a/include/hw/nvram/fw_cfg_keys.h +++ b/include/hw/nvram/fw_cfg_keys.h @@ -29,8 +29,7 @@ #define FW_CFG_FILE_DIR 0x19 #define FW_CFG_FILE_FIRST 0x20 -#define FW_CFG_FILE_SLOTS 0x10 -#define FW_CFG_MAX_ENTRY (FW_CFG_FILE_FIRST + FW_CFG_FILE_SLOTS) +#define FW_CFG_FILE_SLOTS_MIN 0x10 #define FW_CFG_WRITE_CHANNEL 0x4000 #define FW_CFG_ARCH_LOCAL 0x8000 diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index e0145c11a19b..313d943ebd27 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -33,6 +33,7 @@ #include "qemu/error-report.h" #include "qemu/config-file.h" #include "qemu/cutils.h" +#include "qapi/error.h" #define FW_CFG_NAME "fw_cfg" #define FW_CFG_PATH "/machine/" FW_CFG_NAME @@ -71,8 +72,9 @@ struct FWCfgState { SysBusDevice parent_obj; /*< public >*/ - FWCfgEntry entries[2][FW_CFG_MAX_ENTRY]; - int entry_order[FW_CFG_MAX_ENTRY]; + uint16_t file_slots; + FWCfgEntry *entries[2]; + int *entry_order; FWCfgFiles *files; uint16_t cur_entry; uint32_t cur_offset; @@ -257,13 +259,24 @@ static void fw_cfg_write(FWCfgState *s, uint8_t value) /* nothing, write support removed in QEMU v2.4+ */ } +static inline uint16_t fw_cfg_file_slots(const FWCfgState *s) +{ + return s->file_slots; +} + +/* Note: this function returns an exclusive limit. */ +static inline uint32_t fw_cfg_max_entry(const FWCfgState *s) +{ + return FW_CFG_FILE_FIRST + fw_cfg_file_slots(s); +} + static int fw_cfg_select(FWCfgState *s, uint16_t key) { int arch, ret; FWCfgEntry *e; s->cur_offset = 0; - if ((key & FW_CFG_ENTRY_MASK) >= FW_CFG_MAX_ENTRY) { + if ((key & FW_CFG_ENTRY_MASK) >= fw_cfg_max_entry(s)) { s->cur_entry = FW_CFG_INVALID; ret = 0; } else { @@ -610,7 +623,7 @@ static void fw_cfg_add_bytes_read_callback(FWCfgState *s, uint16_t key, key &= FW_CFG_ENTRY_MASK; - assert(key < FW_CFG_MAX_ENTRY && len < UINT32_MAX); + assert(key < fw_cfg_max_entry(s) && len < UINT32_MAX); assert(s->entries[arch][key].data == NULL); /* avoid key conflict */ s->entries[arch][key].data = data; @@ -628,7 +641,7 @@ static void *fw_cfg_modify_bytes_read(FWCfgState *s, uint16_t key, key &= FW_CFG_ENTRY_MASK; - assert(key < FW_CFG_MAX_ENTRY && len < UINT32_MAX); + assert(key < fw_cfg_max_entry(s) && len < UINT32_MAX); /* return the old data to the function caller, avoid memory leak */ ptr = s->entries[arch][key].data; @@ -777,13 +790,13 @@ void fw_cfg_add_file_callback(FWCfgState *s, const char *filename, int order = 0; if (!s->files) { - dsize = sizeof(uint32_t) + sizeof(FWCfgFile) * FW_CFG_FILE_SLOTS; + dsize = sizeof(uint32_t) + sizeof(FWCfgFile) * fw_cfg_file_slots(s); s->files = g_malloc0(dsize); fw_cfg_add_bytes(s, FW_CFG_FILE_DIR, s->files, dsize); } count = be32_to_cpu(s->files->count); - assert(count < FW_CFG_FILE_SLOTS); + assert(count < fw_cfg_file_slots(s)); /* Find the insertion point. */ if (mc->legacy_fw_cfg_order) { @@ -857,7 +870,7 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename, assert(s->files); index = be32_to_cpu(s->files->count); - assert(index < FW_CFG_FILE_SLOTS); + assert(index < fw_cfg_file_slots(s)); for (i = 0; i < index; i++) { if (strcmp(filename, s->files->f[i].name) == 0) { @@ -1014,12 +1027,38 @@ static const TypeInfo fw_cfg_info = { .class_init = fw_cfg_class_init, }; +static void fw_cfg_file_slots_allocate(FWCfgState *s, Error **errp) +{ + uint16_t file_slots_max; + + if (fw_cfg_file_slots(s) < FW_CFG_FILE_SLOTS_MIN) { + error_setg(errp, "\"file_slots\" must be at least 0x%x", + FW_CFG_FILE_SLOTS_MIN); + return; + } + + /* (UINT16_MAX & FW_CFG_ENTRY_MASK) is the highest inclusive selector value + * that we permit. The actual (exclusive) value coming from the + * configuration is (FW_CFG_FILE_FIRST + fw_cfg_file_slots(s)). */ + file_slots_max = (UINT16_MAX & FW_CFG_ENTRY_MASK) - FW_CFG_FILE_FIRST + 1; + if (fw_cfg_file_slots(s) > file_slots_max) { + error_setg(errp, "\"file_slots\" must not exceed 0x%" PRIx16, + file_slots_max); + return; + } + + s->entries[0] = g_new0(FWCfgEntry, fw_cfg_max_entry(s)); + s->entries[1] = g_new0(FWCfgEntry, fw_cfg_max_entry(s)); + s->entry_order = g_new0(int, fw_cfg_max_entry(s)); +} static Property fw_cfg_io_properties[] = { DEFINE_PROP_UINT32("iobase", FWCfgIoState, iobase, -1), DEFINE_PROP_UINT32("dma_iobase", FWCfgIoState, dma_iobase, -1), DEFINE_PROP_BOOL("dma_enabled", FWCfgIoState, parent_obj.dma_enabled, true), + DEFINE_PROP_UINT16("file_slots", FWCfgIoState, parent_obj.file_slots, + FW_CFG_FILE_SLOTS_MIN), DEFINE_PROP_END_OF_LIST(), }; @@ -1027,6 +1066,13 @@ static void fw_cfg_io_realize(DeviceState *dev, Error **errp) { FWCfgIoState *s = FW_CFG_IO(dev); SysBusDevice *sbd = SYS_BUS_DEVICE(dev); + Error *local_err = NULL; + + fw_cfg_file_slots_allocate(FW_CFG(s), &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } /* when using port i/o, the 8-bit data register ALWAYS overlaps * with half of the 16-bit control register. Hence, the total size @@ -1063,6 +1109,8 @@ static Property fw_cfg_mem_properties[] = { DEFINE_PROP_UINT32("data_width", FWCfgMemState, data_width, -1), DEFINE_PROP_BOOL("dma_enabled", FWCfgMemState, parent_obj.dma_enabled, true), + DEFINE_PROP_UINT16("file_slots", FWCfgMemState, parent_obj.file_slots, + FW_CFG_FILE_SLOTS_MIN), DEFINE_PROP_END_OF_LIST(), }; @@ -1071,6 +1119,13 @@ static void fw_cfg_mem_realize(DeviceState *dev, Error **errp) FWCfgMemState *s = FW_CFG_MEM(dev); SysBusDevice *sbd = SYS_BUS_DEVICE(dev); const MemoryRegionOps *data_ops = &fw_cfg_data_mem_ops; + Error *local_err = NULL; + + fw_cfg_file_slots_allocate(FW_CFG(s), &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } memory_region_init_io(&s->ctl_iomem, OBJECT(s), &fw_cfg_ctl_mem_ops, FW_CFG(s), "fwcfg.ctl", FW_CFG_CTL_SIZE); -- 2.9.3 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v5 wave 1 2/4] fw-cfg: turn FW_CFG_FILE_SLOTS into a device property 2017-01-11 17:34 ` [Qemu-devel] [PATCH v5 wave 1 2/4] fw-cfg: turn FW_CFG_FILE_SLOTS into a device property Laszlo Ersek @ 2017-01-12 13:10 ` Eduardo Habkost 2017-01-12 16:30 ` Laszlo Ersek 2017-01-12 14:29 ` Michael S. Tsirkin 1 sibling, 1 reply; 20+ messages in thread From: Eduardo Habkost @ 2017-01-12 13:10 UTC (permalink / raw) To: Laszlo Ersek Cc: qemu devel list, Igor Mammedov, Gabriel L. Somlo, Paolo Bonzini, Gerd Hoffmann, Michael S. Tsirkin On Wed, Jan 11, 2017 at 06:34:55PM +0100, Laszlo Ersek wrote: [...] > static Property fw_cfg_io_properties[] = { > DEFINE_PROP_UINT32("iobase", FWCfgIoState, iobase, -1), > DEFINE_PROP_UINT32("dma_iobase", FWCfgIoState, dma_iobase, -1), > DEFINE_PROP_BOOL("dma_enabled", FWCfgIoState, parent_obj.dma_enabled, > true), > + DEFINE_PROP_UINT16("file_slots", FWCfgIoState, parent_obj.file_slots, > + FW_CFG_FILE_SLOTS_MIN), > DEFINE_PROP_END_OF_LIST(), > }; > > @@ -1027,6 +1066,13 @@ static void fw_cfg_io_realize(DeviceState *dev, Error **errp) I'm not sure what is more important here: following the QOM property name convention using "-" instead of "_", or being consistent with the other existing properties. In either case, we could add a "x-" prefix to indicate it is not supposed to be configured directly by the user. [...] > @@ -1063,6 +1109,8 @@ static Property fw_cfg_mem_properties[] = { > DEFINE_PROP_UINT32("data_width", FWCfgMemState, data_width, -1), > DEFINE_PROP_BOOL("dma_enabled", FWCfgMemState, parent_obj.dma_enabled, > true), > + DEFINE_PROP_UINT16("file_slots", FWCfgMemState, parent_obj.file_slots, > + FW_CFG_FILE_SLOTS_MIN), It looks like you can add the property to the TYPE_FW_CFG parent class instead of duplicating it on the subclasses. The existing "dma_enabled" property could be moved there as well. -- Eduardo ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v5 wave 1 2/4] fw-cfg: turn FW_CFG_FILE_SLOTS into a device property 2017-01-12 13:10 ` Eduardo Habkost @ 2017-01-12 16:30 ` Laszlo Ersek 2017-01-12 16:45 ` Eduardo Habkost 0 siblings, 1 reply; 20+ messages in thread From: Laszlo Ersek @ 2017-01-12 16:30 UTC (permalink / raw) To: Eduardo Habkost Cc: qemu devel list, Igor Mammedov, Gabriel L. Somlo, Paolo Bonzini, Gerd Hoffmann, Michael S. Tsirkin On 01/12/17 14:10, Eduardo Habkost wrote: > On Wed, Jan 11, 2017 at 06:34:55PM +0100, Laszlo Ersek wrote: > [...] >> static Property fw_cfg_io_properties[] = { >> DEFINE_PROP_UINT32("iobase", FWCfgIoState, iobase, -1), >> DEFINE_PROP_UINT32("dma_iobase", FWCfgIoState, dma_iobase, -1), >> DEFINE_PROP_BOOL("dma_enabled", FWCfgIoState, parent_obj.dma_enabled, >> true), >> + DEFINE_PROP_UINT16("file_slots", FWCfgIoState, parent_obj.file_slots, >> + FW_CFG_FILE_SLOTS_MIN), >> DEFINE_PROP_END_OF_LIST(), >> }; >> >> @@ -1027,6 +1066,13 @@ static void fw_cfg_io_realize(DeviceState *dev, Error **errp) > > I'm not sure what is more important here: following the QOM > property name convention using "-" instead of "_", or being > consistent with the other existing properties. Right, when working with the compat settings, I saw both schemes. I couldn't decide. I figured I'd stick with the underscore seen with "dma_enabled". > > In either case, we could add a "x-" prefix to indicate it is not > supposed to be configured directly by the user. I thought "x-" meant "experimental"; i.e., the property could be changed or could go away at any moment. That's a slightly different meaning than "not meant for users". BTW I think the same (== not meant for the user) applies to a number of other properties (dma_enabled is one of them, but other devices have this kind too); do we consistently call them x-*? > > [...] >> @@ -1063,6 +1109,8 @@ static Property fw_cfg_mem_properties[] = { >> DEFINE_PROP_UINT32("data_width", FWCfgMemState, data_width, -1), >> DEFINE_PROP_BOOL("dma_enabled", FWCfgMemState, parent_obj.dma_enabled, >> true), >> + DEFINE_PROP_UINT16("file_slots", FWCfgMemState, parent_obj.file_slots, >> + FW_CFG_FILE_SLOTS_MIN), > > It looks like you can add the property to the TYPE_FW_CFG parent > class instead of duplicating it on the subclasses. The existing > "dma_enabled" property could be moved there as well. I would prefer if someone could pick up that task, separately. The idea crossed my mind when I was working on the common base class, originally, wrt. dma_enabled. I vaguely remember that there was some problem with moving up the property. Ultimately the reviewers didn't expect me, or suggest, to move it up, hence the current status. I think it's out of scope for this series. If you (or someone else) can do it, I agree it would be an improvement, but I'd rather learn the method from someone else's patch than experiment with it myself. If there's a consensus on the x- prefix and/or the underscore/hyphen question, I'm happy to send a v6 with those changes. Thanks, Laszlo ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v5 wave 1 2/4] fw-cfg: turn FW_CFG_FILE_SLOTS into a device property 2017-01-12 16:30 ` Laszlo Ersek @ 2017-01-12 16:45 ` Eduardo Habkost 0 siblings, 0 replies; 20+ messages in thread From: Eduardo Habkost @ 2017-01-12 16:45 UTC (permalink / raw) To: Laszlo Ersek Cc: qemu devel list, Igor Mammedov, Gabriel L. Somlo, Paolo Bonzini, Gerd Hoffmann, Michael S. Tsirkin On Thu, Jan 12, 2017 at 05:30:42PM +0100, Laszlo Ersek wrote: > On 01/12/17 14:10, Eduardo Habkost wrote: > > On Wed, Jan 11, 2017 at 06:34:55PM +0100, Laszlo Ersek wrote: > > [...] > >> static Property fw_cfg_io_properties[] = { > >> DEFINE_PROP_UINT32("iobase", FWCfgIoState, iobase, -1), > >> DEFINE_PROP_UINT32("dma_iobase", FWCfgIoState, dma_iobase, -1), > >> DEFINE_PROP_BOOL("dma_enabled", FWCfgIoState, parent_obj.dma_enabled, > >> true), > >> + DEFINE_PROP_UINT16("file_slots", FWCfgIoState, parent_obj.file_slots, > >> + FW_CFG_FILE_SLOTS_MIN), > >> DEFINE_PROP_END_OF_LIST(), > >> }; > >> > >> @@ -1027,6 +1066,13 @@ static void fw_cfg_io_realize(DeviceState *dev, Error **errp) > > > > I'm not sure what is more important here: following the QOM > > property name convention using "-" instead of "_", or being > > consistent with the other existing properties. > > Right, when working with the compat settings, I saw both schemes. I > couldn't decide. I figured I'd stick with the underscore seen with > "dma_enabled". > > > > > In either case, we could add a "x-" prefix to indicate it is not > > supposed to be configured directly by the user. > > I thought "x-" meant "experimental"; i.e., the property could be changed > or could go away at any moment. That's a slightly different meaning than > "not meant for users". That's true: it means the property could be changed or go away. And that's the only mechanism we have to indicate that users shouldn't rely on the property being always available on the command-line in future versions. (Yes, this should be better documented. We found out very recently that people have very different expectations about what QOM properties should be used for, so we are still figuring out what should be the best practices and didn't write everything down yet.) > > BTW I think the same (== not meant for the user) applies to a number of > other properties (dma_enabled is one of them, but other devices have > this kind too); do we consistently call them x-*? We don't, but that's because we haven't been paying attention to that until recently. Renaming existing properties is possible, but risks breaking existing software and scripts. > > > > > [...] > >> @@ -1063,6 +1109,8 @@ static Property fw_cfg_mem_properties[] = { > >> DEFINE_PROP_UINT32("data_width", FWCfgMemState, data_width, -1), > >> DEFINE_PROP_BOOL("dma_enabled", FWCfgMemState, parent_obj.dma_enabled, > >> true), > >> + DEFINE_PROP_UINT16("file_slots", FWCfgMemState, parent_obj.file_slots, > >> + FW_CFG_FILE_SLOTS_MIN), > > > > It looks like you can add the property to the TYPE_FW_CFG parent > > class instead of duplicating it on the subclasses. The existing > > "dma_enabled" property could be moved there as well. > > I would prefer if someone could pick up that task, separately. > > The idea crossed my mind when I was working on the common base class, > originally, wrt. dma_enabled. I vaguely remember that there was some > problem with moving up the property. Ultimately the reviewers didn't > expect me, or suggest, to move it up, hence the current status. > > I think it's out of scope for this series. If you (or someone else) can > do it, I agree it would be an improvement, but I'd rather learn the > method from someone else's patch than experiment with it myself. Agreed. > > > If there's a consensus on the x- prefix and/or the underscore/hyphen > question, I'm happy to send a v6 with those changes. I don't mind about the underscore, but IMO the "x-" prefix is important. -- Eduardo ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v5 wave 1 2/4] fw-cfg: turn FW_CFG_FILE_SLOTS into a device property 2017-01-11 17:34 ` [Qemu-devel] [PATCH v5 wave 1 2/4] fw-cfg: turn FW_CFG_FILE_SLOTS into a device property Laszlo Ersek 2017-01-12 13:10 ` Eduardo Habkost @ 2017-01-12 14:29 ` Michael S. Tsirkin 2017-01-12 16:34 ` Laszlo Ersek 1 sibling, 1 reply; 20+ messages in thread From: Michael S. Tsirkin @ 2017-01-12 14:29 UTC (permalink / raw) To: Laszlo Ersek Cc: qemu devel list, Gabriel L. Somlo, Gerd Hoffmann, Igor Mammedov, Paolo Bonzini On Wed, Jan 11, 2017 at 06:34:55PM +0100, Laszlo Ersek wrote: > 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 (and device property) FWCfgState.file_slots. > > 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> > --- > > Notes: > v5: > - change the type of "FWCfgState.file_slots" to "uint16_t" [Igor] > > - same for the retval of the trivial wrapper function > fw_cfg_file_slots(), and for the corresponding "file_slots" device > properties > > - preserve the fw_cfg_file_slots() wrapper func (Igor was okay with it > in the end) > > - rename FW_CFG_FILE_SLOTS_TRAD to FW_CFG_FILE_SLOTS_MIN > > - Remove explicit qdev_prop_set_uint32() calls from fw_cfg_init_io_dma() > and fw_cfg_init_mem_wide(), but set the property default to > FW_CFG_FILE_SLOTS_MIN (0x10). This is the main change relative to v4; > the idea is that per-board opt-in shouldn't be necessary for an > increased file_slots count *in addition to* selecting a 2.9 machine > type. [Igor] > > - delay the now-unused FW_CFG_FILE_SLOTS_DFLT macro to a later patch > > v4: > - I know that upstream doesn't care about backward migration, but some > downstreams might. > > docs/specs/fw_cfg.txt | 2 +- > include/hw/nvram/fw_cfg_keys.h | 3 +- > hw/nvram/fw_cfg.c | 71 +++++++++++++++++++++++++++++++++++++----- > 3 files changed, 65 insertions(+), 11 deletions(-) > > diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt > index a19e2adbe1c6..9373bbc64743 100644 > --- a/docs/specs/fw_cfg.txt > +++ b/docs/specs/fw_cfg.txt > @@ -156,7 +156,7 @@ Selector Reg. Range Usage > 0xc000 - 0xffff Arch. Specific (0x0000 - 0x3fff, RW, ignored in v2.4+) > > In practice, the number of allowed firmware configuration items is given > -by the value of FW_CFG_MAX_ENTRY (see fw_cfg.h). > +by the value (FW_CFG_FILE_FIRST + FW_CFG_FILE_SLOTS_MIN) (see fw_cfg.h). > > = Guest-side DMA Interface = > > diff --git a/include/hw/nvram/fw_cfg_keys.h b/include/hw/nvram/fw_cfg_keys.h > index 0f3e871884c0..b6919451f5bd 100644 > --- a/include/hw/nvram/fw_cfg_keys.h > +++ b/include/hw/nvram/fw_cfg_keys.h > @@ -29,8 +29,7 @@ > #define FW_CFG_FILE_DIR 0x19 > > #define FW_CFG_FILE_FIRST 0x20 > -#define FW_CFG_FILE_SLOTS 0x10 > -#define FW_CFG_MAX_ENTRY (FW_CFG_FILE_FIRST + FW_CFG_FILE_SLOTS) > +#define FW_CFG_FILE_SLOTS_MIN 0x10 > > #define FW_CFG_WRITE_CHANNEL 0x4000 > #define FW_CFG_ARCH_LOCAL 0x8000 > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > index e0145c11a19b..313d943ebd27 100644 > --- a/hw/nvram/fw_cfg.c > +++ b/hw/nvram/fw_cfg.c > @@ -33,6 +33,7 @@ > #include "qemu/error-report.h" > #include "qemu/config-file.h" > #include "qemu/cutils.h" > +#include "qapi/error.h" > > #define FW_CFG_NAME "fw_cfg" > #define FW_CFG_PATH "/machine/" FW_CFG_NAME > @@ -71,8 +72,9 @@ struct FWCfgState { > SysBusDevice parent_obj; > /*< public >*/ > > - FWCfgEntry entries[2][FW_CFG_MAX_ENTRY]; > - int entry_order[FW_CFG_MAX_ENTRY]; > + uint16_t file_slots; > + FWCfgEntry *entries[2]; > + int *entry_order; > FWCfgFiles *files; > uint16_t cur_entry; > uint32_t cur_offset; > @@ -257,13 +259,24 @@ static void fw_cfg_write(FWCfgState *s, uint8_t value) > /* nothing, write support removed in QEMU v2.4+ */ > } > > +static inline uint16_t fw_cfg_file_slots(const FWCfgState *s) > +{ > + return s->file_slots; > +} > + > +/* Note: this function returns an exclusive limit. */ > +static inline uint32_t fw_cfg_max_entry(const FWCfgState *s) > +{ > + return FW_CFG_FILE_FIRST + fw_cfg_file_slots(s); > +} > + > static int fw_cfg_select(FWCfgState *s, uint16_t key) > { > int arch, ret; > FWCfgEntry *e; > > s->cur_offset = 0; > - if ((key & FW_CFG_ENTRY_MASK) >= FW_CFG_MAX_ENTRY) { > + if ((key & FW_CFG_ENTRY_MASK) >= fw_cfg_max_entry(s)) { > s->cur_entry = FW_CFG_INVALID; > ret = 0; > } else { > @@ -610,7 +623,7 @@ static void fw_cfg_add_bytes_read_callback(FWCfgState *s, uint16_t key, > > key &= FW_CFG_ENTRY_MASK; > > - assert(key < FW_CFG_MAX_ENTRY && len < UINT32_MAX); > + assert(key < fw_cfg_max_entry(s) && len < UINT32_MAX); > assert(s->entries[arch][key].data == NULL); /* avoid key conflict */ > > s->entries[arch][key].data = data; > @@ -628,7 +641,7 @@ static void *fw_cfg_modify_bytes_read(FWCfgState *s, uint16_t key, > > key &= FW_CFG_ENTRY_MASK; > > - assert(key < FW_CFG_MAX_ENTRY && len < UINT32_MAX); > + assert(key < fw_cfg_max_entry(s) && len < UINT32_MAX); > > /* return the old data to the function caller, avoid memory leak */ > ptr = s->entries[arch][key].data; > @@ -777,13 +790,13 @@ void fw_cfg_add_file_callback(FWCfgState *s, const char *filename, > int order = 0; > > if (!s->files) { > - dsize = sizeof(uint32_t) + sizeof(FWCfgFile) * FW_CFG_FILE_SLOTS; > + dsize = sizeof(uint32_t) + sizeof(FWCfgFile) * fw_cfg_file_slots(s); > s->files = g_malloc0(dsize); > fw_cfg_add_bytes(s, FW_CFG_FILE_DIR, s->files, dsize); > } > > count = be32_to_cpu(s->files->count); > - assert(count < FW_CFG_FILE_SLOTS); > + assert(count < fw_cfg_file_slots(s)); > > /* Find the insertion point. */ > if (mc->legacy_fw_cfg_order) { > @@ -857,7 +870,7 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename, > assert(s->files); > > index = be32_to_cpu(s->files->count); > - assert(index < FW_CFG_FILE_SLOTS); > + assert(index < fw_cfg_file_slots(s)); > > for (i = 0; i < index; i++) { > if (strcmp(filename, s->files->f[i].name) == 0) { > @@ -1014,12 +1027,38 @@ static const TypeInfo fw_cfg_info = { > .class_init = fw_cfg_class_init, > }; > > +static void fw_cfg_file_slots_allocate(FWCfgState *s, Error **errp) > +{ > + uint16_t file_slots_max; > + > + if (fw_cfg_file_slots(s) < FW_CFG_FILE_SLOTS_MIN) { > + error_setg(errp, "\"file_slots\" must be at least 0x%x", > + FW_CFG_FILE_SLOTS_MIN); > + return; > + } > + > + /* (UINT16_MAX & FW_CFG_ENTRY_MASK) is the highest inclusive selector value > + * that we permit. The actual (exclusive) value coming from the > + * configuration is (FW_CFG_FILE_FIRST + fw_cfg_file_slots(s)). */ > + file_slots_max = (UINT16_MAX & FW_CFG_ENTRY_MASK) - FW_CFG_FILE_FIRST + 1; > + if (fw_cfg_file_slots(s) > file_slots_max) { > + error_setg(errp, "\"file_slots\" must not exceed 0x%" PRIx16, > + file_slots_max); > + return; > + } > + > + s->entries[0] = g_new0(FWCfgEntry, fw_cfg_max_entry(s)); > + s->entries[1] = g_new0(FWCfgEntry, fw_cfg_max_entry(s)); > + s->entry_order = g_new0(int, fw_cfg_max_entry(s)); > +} > > static Property fw_cfg_io_properties[] = { > DEFINE_PROP_UINT32("iobase", FWCfgIoState, iobase, -1), > DEFINE_PROP_UINT32("dma_iobase", FWCfgIoState, dma_iobase, -1), > DEFINE_PROP_BOOL("dma_enabled", FWCfgIoState, parent_obj.dma_enabled, > true), > + DEFINE_PROP_UINT16("file_slots", FWCfgIoState, parent_obj.file_slots, > + FW_CFG_FILE_SLOTS_MIN), > DEFINE_PROP_END_OF_LIST(), > }; > > @@ -1027,6 +1066,13 @@ static void fw_cfg_io_realize(DeviceState *dev, Error **errp) > { > FWCfgIoState *s = FW_CFG_IO(dev); > SysBusDevice *sbd = SYS_BUS_DEVICE(dev); > + Error *local_err = NULL; > + > + fw_cfg_file_slots_allocate(FW_CFG(s), &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return; > + } > > /* when using port i/o, the 8-bit data register ALWAYS overlaps > * with half of the 16-bit control register. Hence, the total size > @@ -1063,6 +1109,8 @@ static Property fw_cfg_mem_properties[] = { > DEFINE_PROP_UINT32("data_width", FWCfgMemState, data_width, -1), > DEFINE_PROP_BOOL("dma_enabled", FWCfgMemState, parent_obj.dma_enabled, > true), > + DEFINE_PROP_UINT16("file_slots", FWCfgMemState, parent_obj.file_slots, > + FW_CFG_FILE_SLOTS_MIN), I think it's an internal compatibility thing, so we want to call it x-file-slots instead. > DEFINE_PROP_END_OF_LIST(), > }; > > @@ -1071,6 +1119,13 @@ static void fw_cfg_mem_realize(DeviceState *dev, Error **errp) > FWCfgMemState *s = FW_CFG_MEM(dev); > SysBusDevice *sbd = SYS_BUS_DEVICE(dev); > const MemoryRegionOps *data_ops = &fw_cfg_data_mem_ops; > + Error *local_err = NULL; > + > + fw_cfg_file_slots_allocate(FW_CFG(s), &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return; > + } > > memory_region_init_io(&s->ctl_iomem, OBJECT(s), &fw_cfg_ctl_mem_ops, > FW_CFG(s), "fwcfg.ctl", FW_CFG_CTL_SIZE); > -- > 2.9.3 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v5 wave 1 2/4] fw-cfg: turn FW_CFG_FILE_SLOTS into a device property 2017-01-12 14:29 ` Michael S. Tsirkin @ 2017-01-12 16:34 ` Laszlo Ersek 0 siblings, 0 replies; 20+ messages in thread From: Laszlo Ersek @ 2017-01-12 16:34 UTC (permalink / raw) To: Michael S. Tsirkin Cc: qemu devel list, Gabriel L. Somlo, Gerd Hoffmann, Igor Mammedov, Paolo Bonzini On 01/12/17 15:29, Michael S. Tsirkin wrote: > On Wed, Jan 11, 2017 at 06:34:55PM +0100, Laszlo Ersek wrote: >> 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 (and device property) FWCfgState.file_slots. >> >> 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> >> --- >> >> Notes: >> v5: >> - change the type of "FWCfgState.file_slots" to "uint16_t" [Igor] >> >> - same for the retval of the trivial wrapper function >> fw_cfg_file_slots(), and for the corresponding "file_slots" device >> properties >> >> - preserve the fw_cfg_file_slots() wrapper func (Igor was okay with it >> in the end) >> >> - rename FW_CFG_FILE_SLOTS_TRAD to FW_CFG_FILE_SLOTS_MIN >> >> - Remove explicit qdev_prop_set_uint32() calls from fw_cfg_init_io_dma() >> and fw_cfg_init_mem_wide(), but set the property default to >> FW_CFG_FILE_SLOTS_MIN (0x10). This is the main change relative to v4; >> the idea is that per-board opt-in shouldn't be necessary for an >> increased file_slots count *in addition to* selecting a 2.9 machine >> type. [Igor] >> >> - delay the now-unused FW_CFG_FILE_SLOTS_DFLT macro to a later patch >> >> v4: >> - I know that upstream doesn't care about backward migration, but some >> downstreams might. >> >> docs/specs/fw_cfg.txt | 2 +- >> include/hw/nvram/fw_cfg_keys.h | 3 +- >> hw/nvram/fw_cfg.c | 71 +++++++++++++++++++++++++++++++++++++----- >> 3 files changed, 65 insertions(+), 11 deletions(-) >> >> diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt >> index a19e2adbe1c6..9373bbc64743 100644 >> --- a/docs/specs/fw_cfg.txt >> +++ b/docs/specs/fw_cfg.txt >> @@ -156,7 +156,7 @@ Selector Reg. Range Usage >> 0xc000 - 0xffff Arch. Specific (0x0000 - 0x3fff, RW, ignored in v2.4+) >> >> In practice, the number of allowed firmware configuration items is given >> -by the value of FW_CFG_MAX_ENTRY (see fw_cfg.h). >> +by the value (FW_CFG_FILE_FIRST + FW_CFG_FILE_SLOTS_MIN) (see fw_cfg.h). >> >> = Guest-side DMA Interface = >> >> diff --git a/include/hw/nvram/fw_cfg_keys.h b/include/hw/nvram/fw_cfg_keys.h >> index 0f3e871884c0..b6919451f5bd 100644 >> --- a/include/hw/nvram/fw_cfg_keys.h >> +++ b/include/hw/nvram/fw_cfg_keys.h >> @@ -29,8 +29,7 @@ >> #define FW_CFG_FILE_DIR 0x19 >> >> #define FW_CFG_FILE_FIRST 0x20 >> -#define FW_CFG_FILE_SLOTS 0x10 >> -#define FW_CFG_MAX_ENTRY (FW_CFG_FILE_FIRST + FW_CFG_FILE_SLOTS) >> +#define FW_CFG_FILE_SLOTS_MIN 0x10 >> >> #define FW_CFG_WRITE_CHANNEL 0x4000 >> #define FW_CFG_ARCH_LOCAL 0x8000 >> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c >> index e0145c11a19b..313d943ebd27 100644 >> --- a/hw/nvram/fw_cfg.c >> +++ b/hw/nvram/fw_cfg.c >> @@ -33,6 +33,7 @@ >> #include "qemu/error-report.h" >> #include "qemu/config-file.h" >> #include "qemu/cutils.h" >> +#include "qapi/error.h" >> >> #define FW_CFG_NAME "fw_cfg" >> #define FW_CFG_PATH "/machine/" FW_CFG_NAME >> @@ -71,8 +72,9 @@ struct FWCfgState { >> SysBusDevice parent_obj; >> /*< public >*/ >> >> - FWCfgEntry entries[2][FW_CFG_MAX_ENTRY]; >> - int entry_order[FW_CFG_MAX_ENTRY]; >> + uint16_t file_slots; >> + FWCfgEntry *entries[2]; >> + int *entry_order; >> FWCfgFiles *files; >> uint16_t cur_entry; >> uint32_t cur_offset; >> @@ -257,13 +259,24 @@ static void fw_cfg_write(FWCfgState *s, uint8_t value) >> /* nothing, write support removed in QEMU v2.4+ */ >> } >> >> +static inline uint16_t fw_cfg_file_slots(const FWCfgState *s) >> +{ >> + return s->file_slots; >> +} >> + >> +/* Note: this function returns an exclusive limit. */ >> +static inline uint32_t fw_cfg_max_entry(const FWCfgState *s) >> +{ >> + return FW_CFG_FILE_FIRST + fw_cfg_file_slots(s); >> +} >> + >> static int fw_cfg_select(FWCfgState *s, uint16_t key) >> { >> int arch, ret; >> FWCfgEntry *e; >> >> s->cur_offset = 0; >> - if ((key & FW_CFG_ENTRY_MASK) >= FW_CFG_MAX_ENTRY) { >> + if ((key & FW_CFG_ENTRY_MASK) >= fw_cfg_max_entry(s)) { >> s->cur_entry = FW_CFG_INVALID; >> ret = 0; >> } else { >> @@ -610,7 +623,7 @@ static void fw_cfg_add_bytes_read_callback(FWCfgState *s, uint16_t key, >> >> key &= FW_CFG_ENTRY_MASK; >> >> - assert(key < FW_CFG_MAX_ENTRY && len < UINT32_MAX); >> + assert(key < fw_cfg_max_entry(s) && len < UINT32_MAX); >> assert(s->entries[arch][key].data == NULL); /* avoid key conflict */ >> >> s->entries[arch][key].data = data; >> @@ -628,7 +641,7 @@ static void *fw_cfg_modify_bytes_read(FWCfgState *s, uint16_t key, >> >> key &= FW_CFG_ENTRY_MASK; >> >> - assert(key < FW_CFG_MAX_ENTRY && len < UINT32_MAX); >> + assert(key < fw_cfg_max_entry(s) && len < UINT32_MAX); >> >> /* return the old data to the function caller, avoid memory leak */ >> ptr = s->entries[arch][key].data; >> @@ -777,13 +790,13 @@ void fw_cfg_add_file_callback(FWCfgState *s, const char *filename, >> int order = 0; >> >> if (!s->files) { >> - dsize = sizeof(uint32_t) + sizeof(FWCfgFile) * FW_CFG_FILE_SLOTS; >> + dsize = sizeof(uint32_t) + sizeof(FWCfgFile) * fw_cfg_file_slots(s); >> s->files = g_malloc0(dsize); >> fw_cfg_add_bytes(s, FW_CFG_FILE_DIR, s->files, dsize); >> } >> >> count = be32_to_cpu(s->files->count); >> - assert(count < FW_CFG_FILE_SLOTS); >> + assert(count < fw_cfg_file_slots(s)); >> >> /* Find the insertion point. */ >> if (mc->legacy_fw_cfg_order) { >> @@ -857,7 +870,7 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename, >> assert(s->files); >> >> index = be32_to_cpu(s->files->count); >> - assert(index < FW_CFG_FILE_SLOTS); >> + assert(index < fw_cfg_file_slots(s)); >> >> for (i = 0; i < index; i++) { >> if (strcmp(filename, s->files->f[i].name) == 0) { >> @@ -1014,12 +1027,38 @@ static const TypeInfo fw_cfg_info = { >> .class_init = fw_cfg_class_init, >> }; >> >> +static void fw_cfg_file_slots_allocate(FWCfgState *s, Error **errp) >> +{ >> + uint16_t file_slots_max; >> + >> + if (fw_cfg_file_slots(s) < FW_CFG_FILE_SLOTS_MIN) { >> + error_setg(errp, "\"file_slots\" must be at least 0x%x", >> + FW_CFG_FILE_SLOTS_MIN); >> + return; >> + } >> + >> + /* (UINT16_MAX & FW_CFG_ENTRY_MASK) is the highest inclusive selector value >> + * that we permit. The actual (exclusive) value coming from the >> + * configuration is (FW_CFG_FILE_FIRST + fw_cfg_file_slots(s)). */ >> + file_slots_max = (UINT16_MAX & FW_CFG_ENTRY_MASK) - FW_CFG_FILE_FIRST + 1; >> + if (fw_cfg_file_slots(s) > file_slots_max) { >> + error_setg(errp, "\"file_slots\" must not exceed 0x%" PRIx16, >> + file_slots_max); >> + return; >> + } >> + >> + s->entries[0] = g_new0(FWCfgEntry, fw_cfg_max_entry(s)); >> + s->entries[1] = g_new0(FWCfgEntry, fw_cfg_max_entry(s)); >> + s->entry_order = g_new0(int, fw_cfg_max_entry(s)); >> +} >> >> static Property fw_cfg_io_properties[] = { >> DEFINE_PROP_UINT32("iobase", FWCfgIoState, iobase, -1), >> DEFINE_PROP_UINT32("dma_iobase", FWCfgIoState, dma_iobase, -1), >> DEFINE_PROP_BOOL("dma_enabled", FWCfgIoState, parent_obj.dma_enabled, >> true), >> + DEFINE_PROP_UINT16("file_slots", FWCfgIoState, parent_obj.file_slots, >> + FW_CFG_FILE_SLOTS_MIN), >> DEFINE_PROP_END_OF_LIST(), >> }; >> >> @@ -1027,6 +1066,13 @@ static void fw_cfg_io_realize(DeviceState *dev, Error **errp) >> { >> FWCfgIoState *s = FW_CFG_IO(dev); >> SysBusDevice *sbd = SYS_BUS_DEVICE(dev); >> + Error *local_err = NULL; >> + >> + fw_cfg_file_slots_allocate(FW_CFG(s), &local_err); >> + if (local_err) { >> + error_propagate(errp, local_err); >> + return; >> + } >> >> /* when using port i/o, the 8-bit data register ALWAYS overlaps >> * with half of the 16-bit control register. Hence, the total size >> @@ -1063,6 +1109,8 @@ static Property fw_cfg_mem_properties[] = { >> DEFINE_PROP_UINT32("data_width", FWCfgMemState, data_width, -1), >> DEFINE_PROP_BOOL("dma_enabled", FWCfgMemState, parent_obj.dma_enabled, >> true), >> + DEFINE_PROP_UINT16("file_slots", FWCfgMemState, parent_obj.file_slots, >> + FW_CFG_FILE_SLOTS_MIN), > > I think it's an internal compatibility thing, so we want to call it > x-file-slots instead. Alright, Eduardo suggested the same independently; I will send a v6 with this update. Thanks! Laszlo > > >> DEFINE_PROP_END_OF_LIST(), >> }; >> >> @@ -1071,6 +1119,13 @@ static void fw_cfg_mem_realize(DeviceState *dev, Error **errp) >> FWCfgMemState *s = FW_CFG_MEM(dev); >> SysBusDevice *sbd = SYS_BUS_DEVICE(dev); >> const MemoryRegionOps *data_ops = &fw_cfg_data_mem_ops; >> + Error *local_err = NULL; >> + >> + fw_cfg_file_slots_allocate(FW_CFG(s), &local_err); >> + if (local_err) { >> + error_propagate(errp, local_err); >> + return; >> + } >> >> memory_region_init_io(&s->ctl_iomem, OBJECT(s), &fw_cfg_ctl_mem_ops, >> FW_CFG(s), "fwcfg.ctl", FW_CFG_CTL_SIZE); >> -- >> 2.9.3 >> ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v5 wave 1 3/4] pc: Add 2.9 machine-types 2017-01-11 17:34 ` [Qemu-devel] " Laszlo Ersek ` (2 preceding siblings ...) (?) @ 2017-01-11 17:34 ` Laszlo Ersek -1 siblings, 0 replies; 20+ messages in thread From: Laszlo Ersek @ 2017-01-11 17:34 UTC (permalink / raw) To: qemu devel list; +Cc: Michael S. Tsirkin, Igor Mammedov From: Eduardo Habkost <ehabkost@redhat.com> Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> --- Notes: v5: - Include this patch from <http://lists.nongnu.org/archive/html/qemu-devel/2017-01/msg01020.html> verbatim, as a dependency, with the R-b tags already posted, replacing v4's "hw/i386/pc: introduce 2.9 machine types with 0x20 fw_cfg file slots". include/hw/i386/pc.h | 2 ++ hw/i386/pc_piix.c | 15 ++++++++++++--- hw/i386/pc_q35.c | 13 +++++++++++-- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index b22e699c46cd..230e9e70c504 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -375,6 +375,8 @@ int e820_get_num_entries(void); bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); #define PC_COMPAT_2_8 \ + HW_COMPAT_2_8 \ + #define PC_COMPAT_2_7 \ HW_COMPAT_2_7 \ diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 5e1adbe53c94..9f102aa38875 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -437,13 +437,24 @@ static void pc_i440fx_machine_options(MachineClass *m) m->default_display = "std"; } -static void pc_i440fx_2_8_machine_options(MachineClass *m) +static void pc_i440fx_2_9_machine_options(MachineClass *m) { pc_i440fx_machine_options(m); m->alias = "pc"; m->is_default = 1; } +DEFINE_I440FX_MACHINE(v2_9, "pc-i440fx-2.9", NULL, + pc_i440fx_2_9_machine_options); + +static void pc_i440fx_2_8_machine_options(MachineClass *m) +{ + pc_i440fx_2_9_machine_options(m); + m->is_default = 0; + m->alias = NULL; + SET_MACHINE_COMPAT(m, PC_COMPAT_2_8); +} + DEFINE_I440FX_MACHINE(v2_8, "pc-i440fx-2.8", NULL, pc_i440fx_2_8_machine_options); @@ -451,8 +462,6 @@ DEFINE_I440FX_MACHINE(v2_8, "pc-i440fx-2.8", NULL, static void pc_i440fx_2_7_machine_options(MachineClass *m) { pc_i440fx_2_8_machine_options(m); - m->is_default = 0; - m->alias = NULL; SET_MACHINE_COMPAT(m, PC_COMPAT_2_7); } diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index d042fe0843c6..dd792a8547b3 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -301,19 +301,28 @@ static void pc_q35_machine_options(MachineClass *m) m->max_cpus = 288; } -static void pc_q35_2_8_machine_options(MachineClass *m) +static void pc_q35_2_9_machine_options(MachineClass *m) { pc_q35_machine_options(m); m->alias = "q35"; } +DEFINE_Q35_MACHINE(v2_9, "pc-q35-2.9", NULL, + pc_q35_2_9_machine_options); + +static void pc_q35_2_8_machine_options(MachineClass *m) +{ + pc_q35_2_9_machine_options(m); + m->alias = NULL; + SET_MACHINE_COMPAT(m, PC_COMPAT_2_8); +} + DEFINE_Q35_MACHINE(v2_8, "pc-q35-2.8", NULL, pc_q35_2_8_machine_options); static void pc_q35_2_7_machine_options(MachineClass *m) { pc_q35_2_8_machine_options(m); - m->alias = NULL; m->max_cpus = 255; SET_MACHINE_COMPAT(m, PC_COMPAT_2_7); } -- 2.9.3 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v5 wave 1 4/4] fw-cfg: bump "file_slots" to 0x20 for 2.9+ machine types 2017-01-11 17:34 ` [Qemu-devel] " Laszlo Ersek ` (3 preceding siblings ...) (?) @ 2017-01-11 17:34 ` Laszlo Ersek -1 siblings, 0 replies; 20+ messages in thread From: Laszlo Ersek @ 2017-01-11 17:34 UTC (permalink / raw) To: qemu devel list Cc: Gabriel L. Somlo, Michael S. Tsirkin, Alexander Graf, Anthony Perard, Artyom Tarasenko, David Gibson, Eduardo Habkost, Gerd Hoffmann, Igor Mammedov, Mark Cave-Ayland, Paolo Bonzini, Peter Maydell, Stefano Stabellini More precisely, the "file_slots" count is bumped for all machine types that: (a) use fw_cfg, and (b) are not versioned (hence migration is not expected to work for them across QEMU releases anyway), or have version 2.9. This affects machine types implemented in the following source files: - "hw/arm/virt.c". The "virt-*" machine type is versioned, and the <= 2.8 versions already depend on HW_COMPAT_2_8 (see commit e353aac51b944). Therefore adding the "file_slots" compat values to HW_COMPAT_2_8 suffices. - "hw/i386/pc.c". The "pc-i440fx-*" (including "pc-*") and "pc-q35-*" machine types are versioned. Modifying HW_COMPAT_2_8 is sufficient here too (see commit "pc: Add 2.9 machine-types"). The "isapc" machtype is not versioned. The "xenfv" machine type, which uses fw_cfg for direct kernel booting, is also not versioned. - "hw/ppc/mac_newworld.c". The "mac99" machine type is not versioned. - "hw/ppc/mac_oldworld.c". The "g3beige" machine type is not versioned. - "hw/sparc/sun4m.c". None of the 9 machine types defined in this file appear versioned. - "hw/sparc64/sun4u.c". None of the 3 machine types defined in this file appear versioned. Cc: "Gabriel L. Somlo" <somlo@cmu.edu> Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Alexander Graf <agraf@suse.de> Cc: Anthony Perard <anthony.perard@citrix.com> Cc: Artyom Tarasenko <atar4qemu@gmail.com> Cc: David Gibson <david@gibson.dropbear.id.au> Cc: Eduardo Habkost <ehabkost@redhat.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Igor Mammedov <imammedo@redhat.com> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: Stefano Stabellini <sstabellini@kernel.org> Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: v5: - based on the previous patch from Eduardo, replace the following two patches from v4 [Igor]: * fw-cfg: expose "file_slots" parameter in fw_cfg_init_io_dma() * hw/i386/pc: introduce 2.9 machine types with 0x20 fw_cfg file slots docs/specs/fw_cfg.txt | 4 ++-- include/hw/compat.h | 10 +++++++++- hw/nvram/fw_cfg.c | 6 ++++-- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt index 9373bbc64743..08c00bdf44a2 100644 --- a/docs/specs/fw_cfg.txt +++ b/docs/specs/fw_cfg.txt @@ -155,8 +155,8 @@ Selector Reg. Range Usage through the DMA interface in QEMU v2.9+) 0xc000 - 0xffff Arch. Specific (0x0000 - 0x3fff, RW, ignored in v2.4+) -In practice, the number of allowed firmware configuration items is given -by the value (FW_CFG_FILE_FIRST + FW_CFG_FILE_SLOTS_MIN) (see fw_cfg.h). +In practice, the number of allowed firmware configuration items depends on the +machine type/version. = Guest-side DMA Interface = diff --git a/include/hw/compat.h b/include/hw/compat.h index 4fe44d1c7a6c..a8f344aa738f 100644 --- a/include/hw/compat.h +++ b/include/hw/compat.h @@ -2,7 +2,15 @@ #define HW_COMPAT_H #define HW_COMPAT_2_8 \ - /* empty */ + {\ + .driver = "fw_cfg_mem",\ + .property = "file_slots",\ + .value = stringify(0x10),\ + },{\ + .driver = "fw_cfg_io",\ + .property = "file_slots",\ + .value = stringify(0x10),\ + }, #define HW_COMPAT_2_7 \ {\ diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index 313d943ebd27..a2e5b69fb52a 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -35,6 +35,8 @@ #include "qemu/cutils.h" #include "qapi/error.h" +#define FW_CFG_FILE_SLOTS_DFLT 0x20 + #define FW_CFG_NAME "fw_cfg" #define FW_CFG_PATH "/machine/" FW_CFG_NAME @@ -1058,7 +1060,7 @@ static Property fw_cfg_io_properties[] = { DEFINE_PROP_BOOL("dma_enabled", FWCfgIoState, parent_obj.dma_enabled, true), DEFINE_PROP_UINT16("file_slots", FWCfgIoState, parent_obj.file_slots, - FW_CFG_FILE_SLOTS_MIN), + FW_CFG_FILE_SLOTS_DFLT), DEFINE_PROP_END_OF_LIST(), }; @@ -1110,7 +1112,7 @@ static Property fw_cfg_mem_properties[] = { DEFINE_PROP_BOOL("dma_enabled", FWCfgMemState, parent_obj.dma_enabled, true), DEFINE_PROP_UINT16("file_slots", FWCfgMemState, parent_obj.file_slots, - FW_CFG_FILE_SLOTS_MIN), + FW_CFG_FILE_SLOTS_DFLT), DEFINE_PROP_END_OF_LIST(), }; -- 2.9.3 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-arm] [PATCH v5 wave 1 0/4] fw-cfg: support writeable blobs and more files 2017-01-11 17:34 ` [Qemu-devel] " Laszlo Ersek @ 2017-01-12 2:25 ` Gabriel L. Somlo -1 siblings, 0 replies; 20+ messages in thread From: Gabriel L. Somlo @ 2017-01-12 2:25 UTC (permalink / raw) To: Laszlo Ersek Cc: Peter Maydell, Stefano Stabellini, Eduardo Habkost, Michael S. Tsirkin, Shannon Zhao, Mark Cave-Ayland, Alexander Graf, qemu devel list, Michael Walle, qemu-arm, Gerd Hoffmann, Paolo Bonzini, Anthony Perard, Igor Mammedov, Artyom Tarasenko, David Gibson On Wed, Jan 11, 2017 at 06:34:53PM +0100, Laszlo Ersek wrote: > This is the first (fw_cfg) half of the v5 iteration of the series posted > here: > <http://lists.nongnu.org/archive/html/qemu-devel/2016-12/msg00129.html>. > > In this version, the fw_cfg patches have been separated into a > standalone "wave", for helping review / maintenance, and also for > enabling independent features on top of writeable blobs. More > importantly, I've addressed Igor's v4 feedback. See the individual > patches for the details. > > Patch #3 is included verbatim from Eduardo's pending series (see the > patch notes for the archive URL), as a dependency for patch #4. If > Eduardo's series is merged first, patch #3 can be dropped (in fact > git-rebase should do it automatically). > > Please excuse the surprisingly long list of CC's, it's due to the fact > that fw_cfg is quite widely used (see patch #4). > > Cc: "Gabriel L. Somlo" <somlo@cmu.edu> Whole series: Acked-by: Gabriel Somlo <somlo@cmu.edu> Data passed in via the "-fw_cfg" qemu command still shows up fine in /sys/firmware/qemu-fw-cfg on the guest, so also: Tested-by: Gabriel Somlo <somlo@cmu.edu> Thanks, --Gabriel > Cc: "Michael S. Tsirkin" <mst@redhat.com> > Cc: Alexander Graf <agraf@suse.de> > Cc: Anthony Perard <anthony.perard@citrix.com> > Cc: Artyom Tarasenko <atar4qemu@gmail.com> > Cc: David Gibson <david@gibson.dropbear.id.au> > Cc: Eduardo Habkost <ehabkost@redhat.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Igor Mammedov <imammedo@redhat.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > Cc: Michael Walle <michael@walle.cc> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Peter Maydell <peter.maydell@linaro.org> > Cc: Shannon Zhao <zhaoshenglong@huawei.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: qemu-arm@nongnu.org > > Thanks > Laszlo > > > Eduardo Habkost (1): > pc: Add 2.9 machine-types > > Laszlo Ersek (2): > fw-cfg: turn FW_CFG_FILE_SLOTS into a device property > fw-cfg: bump "file_slots" to 0x20 for 2.9+ machine types > > Michael S. Tsirkin (1): > fw-cfg: support writeable blobs > > docs/specs/fw_cfg.txt | 36 ++++++++++---- > hw/lm32/lm32_hwsetup.h | 2 +- > include/hw/compat.h | 10 +++- > include/hw/i386/pc.h | 2 + > include/hw/loader.h | 7 +-- > include/hw/nvram/fw_cfg.h | 3 +- > include/hw/nvram/fw_cfg_keys.h | 3 +- > hw/arm/virt-acpi-build.c | 2 +- > hw/core/loader.c | 18 ++++--- > hw/i386/acpi-build.c | 4 +- > hw/i386/pc_piix.c | 15 ++++-- > hw/i386/pc_q35.c | 13 ++++- > hw/nvram/fw_cfg.c | 110 +++++++++++++++++++++++++++++++++++------ > 13 files changed, 177 insertions(+), 48 deletions(-) > > -- > 2.9.3 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v5 wave 1 0/4] fw-cfg: support writeable blobs and more files @ 2017-01-12 2:25 ` Gabriel L. Somlo 0 siblings, 0 replies; 20+ messages in thread From: Gabriel L. Somlo @ 2017-01-12 2:25 UTC (permalink / raw) To: Laszlo Ersek Cc: qemu devel list, Michael S. Tsirkin, Alexander Graf, Anthony Perard, Artyom Tarasenko, David Gibson, Eduardo Habkost, Gerd Hoffmann, Igor Mammedov, Mark Cave-Ayland, Michael Walle, Paolo Bonzini, Peter Maydell, Shannon Zhao, Stefano Stabellini, qemu-arm On Wed, Jan 11, 2017 at 06:34:53PM +0100, Laszlo Ersek wrote: > This is the first (fw_cfg) half of the v5 iteration of the series posted > here: > <http://lists.nongnu.org/archive/html/qemu-devel/2016-12/msg00129.html>. > > In this version, the fw_cfg patches have been separated into a > standalone "wave", for helping review / maintenance, and also for > enabling independent features on top of writeable blobs. More > importantly, I've addressed Igor's v4 feedback. See the individual > patches for the details. > > Patch #3 is included verbatim from Eduardo's pending series (see the > patch notes for the archive URL), as a dependency for patch #4. If > Eduardo's series is merged first, patch #3 can be dropped (in fact > git-rebase should do it automatically). > > Please excuse the surprisingly long list of CC's, it's due to the fact > that fw_cfg is quite widely used (see patch #4). > > Cc: "Gabriel L. Somlo" <somlo@cmu.edu> Whole series: Acked-by: Gabriel Somlo <somlo@cmu.edu> Data passed in via the "-fw_cfg" qemu command still shows up fine in /sys/firmware/qemu-fw-cfg on the guest, so also: Tested-by: Gabriel Somlo <somlo@cmu.edu> Thanks, --Gabriel > Cc: "Michael S. Tsirkin" <mst@redhat.com> > Cc: Alexander Graf <agraf@suse.de> > Cc: Anthony Perard <anthony.perard@citrix.com> > Cc: Artyom Tarasenko <atar4qemu@gmail.com> > Cc: David Gibson <david@gibson.dropbear.id.au> > Cc: Eduardo Habkost <ehabkost@redhat.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Igor Mammedov <imammedo@redhat.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > Cc: Michael Walle <michael@walle.cc> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Peter Maydell <peter.maydell@linaro.org> > Cc: Shannon Zhao <zhaoshenglong@huawei.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: qemu-arm@nongnu.org > > Thanks > Laszlo > > > Eduardo Habkost (1): > pc: Add 2.9 machine-types > > Laszlo Ersek (2): > fw-cfg: turn FW_CFG_FILE_SLOTS into a device property > fw-cfg: bump "file_slots" to 0x20 for 2.9+ machine types > > Michael S. Tsirkin (1): > fw-cfg: support writeable blobs > > docs/specs/fw_cfg.txt | 36 ++++++++++---- > hw/lm32/lm32_hwsetup.h | 2 +- > include/hw/compat.h | 10 +++- > include/hw/i386/pc.h | 2 + > include/hw/loader.h | 7 +-- > include/hw/nvram/fw_cfg.h | 3 +- > include/hw/nvram/fw_cfg_keys.h | 3 +- > hw/arm/virt-acpi-build.c | 2 +- > hw/core/loader.c | 18 ++++--- > hw/i386/acpi-build.c | 4 +- > hw/i386/pc_piix.c | 15 ++++-- > hw/i386/pc_q35.c | 13 ++++- > hw/nvram/fw_cfg.c | 110 +++++++++++++++++++++++++++++++++++------ > 13 files changed, 177 insertions(+), 48 deletions(-) > > -- > 2.9.3 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-arm] [PATCH v5 wave 1 0/4] fw-cfg: support writeable blobs and more files 2017-01-12 2:25 ` [Qemu-devel] " Gabriel L. Somlo @ 2017-01-12 10:09 ` Laszlo Ersek -1 siblings, 0 replies; 20+ messages in thread From: Laszlo Ersek @ 2017-01-12 10:09 UTC (permalink / raw) To: Gabriel L. Somlo Cc: Peter Maydell, Stefano Stabellini, Eduardo Habkost, Michael S. Tsirkin, Shannon Zhao, Mark Cave-Ayland, Alexander Graf, qemu devel list, Michael Walle, qemu-arm, Gerd Hoffmann, Paolo Bonzini, Anthony Perard, Igor Mammedov, Artyom Tarasenko, David Gibson On 01/12/17 03:25, Gabriel L. Somlo wrote: > On Wed, Jan 11, 2017 at 06:34:53PM +0100, Laszlo Ersek wrote: >> This is the first (fw_cfg) half of the v5 iteration of the series posted >> here: >> <http://lists.nongnu.org/archive/html/qemu-devel/2016-12/msg00129.html>. >> >> In this version, the fw_cfg patches have been separated into a >> standalone "wave", for helping review / maintenance, and also for >> enabling independent features on top of writeable blobs. More >> importantly, I've addressed Igor's v4 feedback. See the individual >> patches for the details. >> >> Patch #3 is included verbatim from Eduardo's pending series (see the >> patch notes for the archive URL), as a dependency for patch #4. If >> Eduardo's series is merged first, patch #3 can be dropped (in fact >> git-rebase should do it automatically). >> >> Please excuse the surprisingly long list of CC's, it's due to the fact >> that fw_cfg is quite widely used (see patch #4). >> >> Cc: "Gabriel L. Somlo" <somlo@cmu.edu> > > Whole series: > > Acked-by: Gabriel Somlo <somlo@cmu.edu> > > Data passed in via the "-fw_cfg" qemu command still shows up fine in > /sys/firmware/qemu-fw-cfg on the guest, so also: > > Tested-by: Gabriel Somlo <somlo@cmu.edu> Thank you! Laszlo > > Thanks, > --Gabriel > >> Cc: "Michael S. Tsirkin" <mst@redhat.com> >> Cc: Alexander Graf <agraf@suse.de> >> Cc: Anthony Perard <anthony.perard@citrix.com> >> Cc: Artyom Tarasenko <atar4qemu@gmail.com> >> Cc: David Gibson <david@gibson.dropbear.id.au> >> Cc: Eduardo Habkost <ehabkost@redhat.com> >> Cc: Gerd Hoffmann <kraxel@redhat.com> >> Cc: Igor Mammedov <imammedo@redhat.com> >> Cc: Laszlo Ersek <lersek@redhat.com> >> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> Cc: Michael Walle <michael@walle.cc> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: Peter Maydell <peter.maydell@linaro.org> >> Cc: Shannon Zhao <zhaoshenglong@huawei.com> >> Cc: Stefano Stabellini <sstabellini@kernel.org> >> Cc: qemu-arm@nongnu.org >> >> Thanks >> Laszlo >> >> >> Eduardo Habkost (1): >> pc: Add 2.9 machine-types >> >> Laszlo Ersek (2): >> fw-cfg: turn FW_CFG_FILE_SLOTS into a device property >> fw-cfg: bump "file_slots" to 0x20 for 2.9+ machine types >> >> Michael S. Tsirkin (1): >> fw-cfg: support writeable blobs >> >> docs/specs/fw_cfg.txt | 36 ++++++++++---- >> hw/lm32/lm32_hwsetup.h | 2 +- >> include/hw/compat.h | 10 +++- >> include/hw/i386/pc.h | 2 + >> include/hw/loader.h | 7 +-- >> include/hw/nvram/fw_cfg.h | 3 +- >> include/hw/nvram/fw_cfg_keys.h | 3 +- >> hw/arm/virt-acpi-build.c | 2 +- >> hw/core/loader.c | 18 ++++--- >> hw/i386/acpi-build.c | 4 +- >> hw/i386/pc_piix.c | 15 ++++-- >> hw/i386/pc_q35.c | 13 ++++- >> hw/nvram/fw_cfg.c | 110 +++++++++++++++++++++++++++++++++++------ >> 13 files changed, 177 insertions(+), 48 deletions(-) >> >> -- >> 2.9.3 >> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v5 wave 1 0/4] fw-cfg: support writeable blobs and more files @ 2017-01-12 10:09 ` Laszlo Ersek 0 siblings, 0 replies; 20+ messages in thread From: Laszlo Ersek @ 2017-01-12 10:09 UTC (permalink / raw) To: Gabriel L. Somlo Cc: qemu devel list, Michael S. Tsirkin, Alexander Graf, Anthony Perard, Artyom Tarasenko, David Gibson, Eduardo Habkost, Gerd Hoffmann, Igor Mammedov, Mark Cave-Ayland, Michael Walle, Paolo Bonzini, Peter Maydell, Shannon Zhao, Stefano Stabellini, qemu-arm On 01/12/17 03:25, Gabriel L. Somlo wrote: > On Wed, Jan 11, 2017 at 06:34:53PM +0100, Laszlo Ersek wrote: >> This is the first (fw_cfg) half of the v5 iteration of the series posted >> here: >> <http://lists.nongnu.org/archive/html/qemu-devel/2016-12/msg00129.html>. >> >> In this version, the fw_cfg patches have been separated into a >> standalone "wave", for helping review / maintenance, and also for >> enabling independent features on top of writeable blobs. More >> importantly, I've addressed Igor's v4 feedback. See the individual >> patches for the details. >> >> Patch #3 is included verbatim from Eduardo's pending series (see the >> patch notes for the archive URL), as a dependency for patch #4. If >> Eduardo's series is merged first, patch #3 can be dropped (in fact >> git-rebase should do it automatically). >> >> Please excuse the surprisingly long list of CC's, it's due to the fact >> that fw_cfg is quite widely used (see patch #4). >> >> Cc: "Gabriel L. Somlo" <somlo@cmu.edu> > > Whole series: > > Acked-by: Gabriel Somlo <somlo@cmu.edu> > > Data passed in via the "-fw_cfg" qemu command still shows up fine in > /sys/firmware/qemu-fw-cfg on the guest, so also: > > Tested-by: Gabriel Somlo <somlo@cmu.edu> Thank you! Laszlo > > Thanks, > --Gabriel > >> Cc: "Michael S. Tsirkin" <mst@redhat.com> >> Cc: Alexander Graf <agraf@suse.de> >> Cc: Anthony Perard <anthony.perard@citrix.com> >> Cc: Artyom Tarasenko <atar4qemu@gmail.com> >> Cc: David Gibson <david@gibson.dropbear.id.au> >> Cc: Eduardo Habkost <ehabkost@redhat.com> >> Cc: Gerd Hoffmann <kraxel@redhat.com> >> Cc: Igor Mammedov <imammedo@redhat.com> >> Cc: Laszlo Ersek <lersek@redhat.com> >> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> Cc: Michael Walle <michael@walle.cc> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: Peter Maydell <peter.maydell@linaro.org> >> Cc: Shannon Zhao <zhaoshenglong@huawei.com> >> Cc: Stefano Stabellini <sstabellini@kernel.org> >> Cc: qemu-arm@nongnu.org >> >> Thanks >> Laszlo >> >> >> Eduardo Habkost (1): >> pc: Add 2.9 machine-types >> >> Laszlo Ersek (2): >> fw-cfg: turn FW_CFG_FILE_SLOTS into a device property >> fw-cfg: bump "file_slots" to 0x20 for 2.9+ machine types >> >> Michael S. Tsirkin (1): >> fw-cfg: support writeable blobs >> >> docs/specs/fw_cfg.txt | 36 ++++++++++---- >> hw/lm32/lm32_hwsetup.h | 2 +- >> include/hw/compat.h | 10 +++- >> include/hw/i386/pc.h | 2 + >> include/hw/loader.h | 7 +-- >> include/hw/nvram/fw_cfg.h | 3 +- >> include/hw/nvram/fw_cfg_keys.h | 3 +- >> hw/arm/virt-acpi-build.c | 2 +- >> hw/core/loader.c | 18 ++++--- >> hw/i386/acpi-build.c | 4 +- >> hw/i386/pc_piix.c | 15 ++++-- >> hw/i386/pc_q35.c | 13 ++++- >> hw/nvram/fw_cfg.c | 110 +++++++++++++++++++++++++++++++++++------ >> 13 files changed, 177 insertions(+), 48 deletions(-) >> >> -- >> 2.9.3 >> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-arm] [PATCH v5 wave 1 0/4] fw-cfg: support writeable blobs and more files 2017-01-11 17:34 ` [Qemu-devel] " Laszlo Ersek @ 2017-01-12 14:30 ` Michael S. Tsirkin -1 siblings, 0 replies; 20+ messages in thread From: Michael S. Tsirkin @ 2017-01-12 14:30 UTC (permalink / raw) To: Laszlo Ersek Cc: Peter Maydell, Stefano Stabellini, Eduardo Habkost, Gabriel L. Somlo, Shannon Zhao, Mark Cave-Ayland, qemu devel list, Alexander Graf, Michael Walle, qemu-arm, Gerd Hoffmann, Paolo Bonzini, Anthony Perard, Igor Mammedov, Artyom Tarasenko, David Gibson On Wed, Jan 11, 2017 at 06:34:53PM +0100, Laszlo Ersek wrote: > This is the first (fw_cfg) half of the v5 iteration of the series posted > here: > <http://lists.nongnu.org/archive/html/qemu-devel/2016-12/msg00129.html>. > > In this version, the fw_cfg patches have been separated into a > standalone "wave", for helping review / maintenance, and also for > enabling independent features on top of writeable blobs. More > importantly, I've addressed Igor's v4 feedback. See the individual > patches for the details. > > Patch #3 is included verbatim from Eduardo's pending series (see the > patch notes for the archive URL), as a dependency for patch #4. If > Eduardo's series is merged first, patch #3 can be dropped (in fact > git-rebase should do it automatically). > > Please excuse the surprisingly long list of CC's, it's due to the fact > that fw_cfg is quite widely used (see patch #4). Looks good. So what's the plan for merging this? I think I'll merge part 1 and then Paolo merges part 2 - is that what you had in mind? > Cc: "Gabriel L. Somlo" <somlo@cmu.edu> > Cc: "Michael S. Tsirkin" <mst@redhat.com> > Cc: Alexander Graf <agraf@suse.de> > Cc: Anthony Perard <anthony.perard@citrix.com> > Cc: Artyom Tarasenko <atar4qemu@gmail.com> > Cc: David Gibson <david@gibson.dropbear.id.au> > Cc: Eduardo Habkost <ehabkost@redhat.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Igor Mammedov <imammedo@redhat.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > Cc: Michael Walle <michael@walle.cc> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Peter Maydell <peter.maydell@linaro.org> > Cc: Shannon Zhao <zhaoshenglong@huawei.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: qemu-arm@nongnu.org > > Thanks > Laszlo > > > Eduardo Habkost (1): > pc: Add 2.9 machine-types > > Laszlo Ersek (2): > fw-cfg: turn FW_CFG_FILE_SLOTS into a device property > fw-cfg: bump "file_slots" to 0x20 for 2.9+ machine types > > Michael S. Tsirkin (1): > fw-cfg: support writeable blobs > > docs/specs/fw_cfg.txt | 36 ++++++++++---- > hw/lm32/lm32_hwsetup.h | 2 +- > include/hw/compat.h | 10 +++- > include/hw/i386/pc.h | 2 + > include/hw/loader.h | 7 +-- > include/hw/nvram/fw_cfg.h | 3 +- > include/hw/nvram/fw_cfg_keys.h | 3 +- > hw/arm/virt-acpi-build.c | 2 +- > hw/core/loader.c | 18 ++++--- > hw/i386/acpi-build.c | 4 +- > hw/i386/pc_piix.c | 15 ++++-- > hw/i386/pc_q35.c | 13 ++++- > hw/nvram/fw_cfg.c | 110 +++++++++++++++++++++++++++++++++++------ > 13 files changed, 177 insertions(+), 48 deletions(-) > > -- > 2.9.3 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v5 wave 1 0/4] fw-cfg: support writeable blobs and more files @ 2017-01-12 14:30 ` Michael S. Tsirkin 0 siblings, 0 replies; 20+ messages in thread From: Michael S. Tsirkin @ 2017-01-12 14:30 UTC (permalink / raw) To: Laszlo Ersek Cc: qemu devel list, Gabriel L. Somlo, Alexander Graf, Anthony Perard, Artyom Tarasenko, David Gibson, Eduardo Habkost, Gerd Hoffmann, Igor Mammedov, Mark Cave-Ayland, Michael Walle, Paolo Bonzini, Peter Maydell, Shannon Zhao, Stefano Stabellini, qemu-arm On Wed, Jan 11, 2017 at 06:34:53PM +0100, Laszlo Ersek wrote: > This is the first (fw_cfg) half of the v5 iteration of the series posted > here: > <http://lists.nongnu.org/archive/html/qemu-devel/2016-12/msg00129.html>. > > In this version, the fw_cfg patches have been separated into a > standalone "wave", for helping review / maintenance, and also for > enabling independent features on top of writeable blobs. More > importantly, I've addressed Igor's v4 feedback. See the individual > patches for the details. > > Patch #3 is included verbatim from Eduardo's pending series (see the > patch notes for the archive URL), as a dependency for patch #4. If > Eduardo's series is merged first, patch #3 can be dropped (in fact > git-rebase should do it automatically). > > Please excuse the surprisingly long list of CC's, it's due to the fact > that fw_cfg is quite widely used (see patch #4). Looks good. So what's the plan for merging this? I think I'll merge part 1 and then Paolo merges part 2 - is that what you had in mind? > Cc: "Gabriel L. Somlo" <somlo@cmu.edu> > Cc: "Michael S. Tsirkin" <mst@redhat.com> > Cc: Alexander Graf <agraf@suse.de> > Cc: Anthony Perard <anthony.perard@citrix.com> > Cc: Artyom Tarasenko <atar4qemu@gmail.com> > Cc: David Gibson <david@gibson.dropbear.id.au> > Cc: Eduardo Habkost <ehabkost@redhat.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Igor Mammedov <imammedo@redhat.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > Cc: Michael Walle <michael@walle.cc> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Peter Maydell <peter.maydell@linaro.org> > Cc: Shannon Zhao <zhaoshenglong@huawei.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: qemu-arm@nongnu.org > > Thanks > Laszlo > > > Eduardo Habkost (1): > pc: Add 2.9 machine-types > > Laszlo Ersek (2): > fw-cfg: turn FW_CFG_FILE_SLOTS into a device property > fw-cfg: bump "file_slots" to 0x20 for 2.9+ machine types > > Michael S. Tsirkin (1): > fw-cfg: support writeable blobs > > docs/specs/fw_cfg.txt | 36 ++++++++++---- > hw/lm32/lm32_hwsetup.h | 2 +- > include/hw/compat.h | 10 +++- > include/hw/i386/pc.h | 2 + > include/hw/loader.h | 7 +-- > include/hw/nvram/fw_cfg.h | 3 +- > include/hw/nvram/fw_cfg_keys.h | 3 +- > hw/arm/virt-acpi-build.c | 2 +- > hw/core/loader.c | 18 ++++--- > hw/i386/acpi-build.c | 4 +- > hw/i386/pc_piix.c | 15 ++++-- > hw/i386/pc_q35.c | 13 ++++- > hw/nvram/fw_cfg.c | 110 +++++++++++++++++++++++++++++++++++------ > 13 files changed, 177 insertions(+), 48 deletions(-) > > -- > 2.9.3 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-arm] [PATCH v5 wave 1 0/4] fw-cfg: support writeable blobs and more files 2017-01-12 14:30 ` [Qemu-devel] " Michael S. Tsirkin @ 2017-01-12 16:37 ` Laszlo Ersek -1 siblings, 0 replies; 20+ messages in thread From: Laszlo Ersek @ 2017-01-12 16:37 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Peter Maydell, Stefano Stabellini, Eduardo Habkost, Gabriel L. Somlo, Shannon Zhao, Mark Cave-Ayland, qemu devel list, Alexander Graf, Michael Walle, qemu-arm, Gerd Hoffmann, Paolo Bonzini, Anthony Perard, Igor Mammedov, Artyom Tarasenko, David Gibson On 01/12/17 15:30, Michael S. Tsirkin wrote: > On Wed, Jan 11, 2017 at 06:34:53PM +0100, Laszlo Ersek wrote: >> This is the first (fw_cfg) half of the v5 iteration of the series posted >> here: >> <http://lists.nongnu.org/archive/html/qemu-devel/2016-12/msg00129.html>. >> >> In this version, the fw_cfg patches have been separated into a >> standalone "wave", for helping review / maintenance, and also for >> enabling independent features on top of writeable blobs. More >> importantly, I've addressed Igor's v4 feedback. See the individual >> patches for the details. >> >> Patch #3 is included verbatim from Eduardo's pending series (see the >> patch notes for the archive URL), as a dependency for patch #4. If >> Eduardo's series is merged first, patch #3 can be dropped (in fact >> git-rebase should do it automatically). >> >> Please excuse the surprisingly long list of CC's, it's due to the fact >> that fw_cfg is quite widely used (see patch #4). > > Looks good. So what's the plan for merging this? > I think I'll merge part 1 and then Paolo merges part 2 - > is that what you had in mind? It certainly works for me, thank you! Paolo hasn't commented on wave 2 yet, so I'm not sure about that half, but you did express a preference for this kind of work distribution in v4, so that's fine with me. I'll send out v6 with the property name update in a minute (picking up the feedback tags from Gabriel -- thanks agan for that!) Cheers Laszlo > >> Cc: "Gabriel L. Somlo" <somlo@cmu.edu> >> Cc: "Michael S. Tsirkin" <mst@redhat.com> >> Cc: Alexander Graf <agraf@suse.de> >> Cc: Anthony Perard <anthony.perard@citrix.com> >> Cc: Artyom Tarasenko <atar4qemu@gmail.com> >> Cc: David Gibson <david@gibson.dropbear.id.au> >> Cc: Eduardo Habkost <ehabkost@redhat.com> >> Cc: Gerd Hoffmann <kraxel@redhat.com> >> Cc: Igor Mammedov <imammedo@redhat.com> >> Cc: Laszlo Ersek <lersek@redhat.com> >> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> Cc: Michael Walle <michael@walle.cc> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: Peter Maydell <peter.maydell@linaro.org> >> Cc: Shannon Zhao <zhaoshenglong@huawei.com> >> Cc: Stefano Stabellini <sstabellini@kernel.org> >> Cc: qemu-arm@nongnu.org >> >> Thanks >> Laszlo >> >> >> Eduardo Habkost (1): >> pc: Add 2.9 machine-types >> >> Laszlo Ersek (2): >> fw-cfg: turn FW_CFG_FILE_SLOTS into a device property >> fw-cfg: bump "file_slots" to 0x20 for 2.9+ machine types >> >> Michael S. Tsirkin (1): >> fw-cfg: support writeable blobs >> >> docs/specs/fw_cfg.txt | 36 ++++++++++---- >> hw/lm32/lm32_hwsetup.h | 2 +- >> include/hw/compat.h | 10 +++- >> include/hw/i386/pc.h | 2 + >> include/hw/loader.h | 7 +-- >> include/hw/nvram/fw_cfg.h | 3 +- >> include/hw/nvram/fw_cfg_keys.h | 3 +- >> hw/arm/virt-acpi-build.c | 2 +- >> hw/core/loader.c | 18 ++++--- >> hw/i386/acpi-build.c | 4 +- >> hw/i386/pc_piix.c | 15 ++++-- >> hw/i386/pc_q35.c | 13 ++++- >> hw/nvram/fw_cfg.c | 110 +++++++++++++++++++++++++++++++++++------ >> 13 files changed, 177 insertions(+), 48 deletions(-) >> >> -- >> 2.9.3 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v5 wave 1 0/4] fw-cfg: support writeable blobs and more files @ 2017-01-12 16:37 ` Laszlo Ersek 0 siblings, 0 replies; 20+ messages in thread From: Laszlo Ersek @ 2017-01-12 16:37 UTC (permalink / raw) To: Michael S. Tsirkin Cc: qemu devel list, Gabriel L. Somlo, Alexander Graf, Anthony Perard, Artyom Tarasenko, David Gibson, Eduardo Habkost, Gerd Hoffmann, Igor Mammedov, Mark Cave-Ayland, Michael Walle, Paolo Bonzini, Peter Maydell, Shannon Zhao, Stefano Stabellini, qemu-arm On 01/12/17 15:30, Michael S. Tsirkin wrote: > On Wed, Jan 11, 2017 at 06:34:53PM +0100, Laszlo Ersek wrote: >> This is the first (fw_cfg) half of the v5 iteration of the series posted >> here: >> <http://lists.nongnu.org/archive/html/qemu-devel/2016-12/msg00129.html>. >> >> In this version, the fw_cfg patches have been separated into a >> standalone "wave", for helping review / maintenance, and also for >> enabling independent features on top of writeable blobs. More >> importantly, I've addressed Igor's v4 feedback. See the individual >> patches for the details. >> >> Patch #3 is included verbatim from Eduardo's pending series (see the >> patch notes for the archive URL), as a dependency for patch #4. If >> Eduardo's series is merged first, patch #3 can be dropped (in fact >> git-rebase should do it automatically). >> >> Please excuse the surprisingly long list of CC's, it's due to the fact >> that fw_cfg is quite widely used (see patch #4). > > Looks good. So what's the plan for merging this? > I think I'll merge part 1 and then Paolo merges part 2 - > is that what you had in mind? It certainly works for me, thank you! Paolo hasn't commented on wave 2 yet, so I'm not sure about that half, but you did express a preference for this kind of work distribution in v4, so that's fine with me. I'll send out v6 with the property name update in a minute (picking up the feedback tags from Gabriel -- thanks agan for that!) Cheers Laszlo > >> Cc: "Gabriel L. Somlo" <somlo@cmu.edu> >> Cc: "Michael S. Tsirkin" <mst@redhat.com> >> Cc: Alexander Graf <agraf@suse.de> >> Cc: Anthony Perard <anthony.perard@citrix.com> >> Cc: Artyom Tarasenko <atar4qemu@gmail.com> >> Cc: David Gibson <david@gibson.dropbear.id.au> >> Cc: Eduardo Habkost <ehabkost@redhat.com> >> Cc: Gerd Hoffmann <kraxel@redhat.com> >> Cc: Igor Mammedov <imammedo@redhat.com> >> Cc: Laszlo Ersek <lersek@redhat.com> >> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> Cc: Michael Walle <michael@walle.cc> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: Peter Maydell <peter.maydell@linaro.org> >> Cc: Shannon Zhao <zhaoshenglong@huawei.com> >> Cc: Stefano Stabellini <sstabellini@kernel.org> >> Cc: qemu-arm@nongnu.org >> >> Thanks >> Laszlo >> >> >> Eduardo Habkost (1): >> pc: Add 2.9 machine-types >> >> Laszlo Ersek (2): >> fw-cfg: turn FW_CFG_FILE_SLOTS into a device property >> fw-cfg: bump "file_slots" to 0x20 for 2.9+ machine types >> >> Michael S. Tsirkin (1): >> fw-cfg: support writeable blobs >> >> docs/specs/fw_cfg.txt | 36 ++++++++++---- >> hw/lm32/lm32_hwsetup.h | 2 +- >> include/hw/compat.h | 10 +++- >> include/hw/i386/pc.h | 2 + >> include/hw/loader.h | 7 +-- >> include/hw/nvram/fw_cfg.h | 3 +- >> include/hw/nvram/fw_cfg_keys.h | 3 +- >> hw/arm/virt-acpi-build.c | 2 +- >> hw/core/loader.c | 18 ++++--- >> hw/i386/acpi-build.c | 4 +- >> hw/i386/pc_piix.c | 15 ++++-- >> hw/i386/pc_q35.c | 13 ++++- >> hw/nvram/fw_cfg.c | 110 +++++++++++++++++++++++++++++++++++------ >> 13 files changed, 177 insertions(+), 48 deletions(-) >> >> -- >> 2.9.3 ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-01-12 16:45 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-01-11 17:34 [Qemu-arm] [PATCH v5 wave 1 0/4] fw-cfg: support writeable blobs and more files Laszlo Ersek 2017-01-11 17:34 ` [Qemu-devel] " Laszlo Ersek 2017-01-11 17:34 ` [Qemu-arm] [PATCH v5 wave 1 1/4] fw-cfg: support writeable blobs Laszlo Ersek 2017-01-11 17:34 ` [Qemu-devel] " Laszlo Ersek 2017-01-11 17:34 ` [Qemu-devel] [PATCH v5 wave 1 2/4] fw-cfg: turn FW_CFG_FILE_SLOTS into a device property Laszlo Ersek 2017-01-12 13:10 ` Eduardo Habkost 2017-01-12 16:30 ` Laszlo Ersek 2017-01-12 16:45 ` Eduardo Habkost 2017-01-12 14:29 ` Michael S. Tsirkin 2017-01-12 16:34 ` Laszlo Ersek 2017-01-11 17:34 ` [Qemu-devel] [PATCH v5 wave 1 3/4] pc: Add 2.9 machine-types Laszlo Ersek 2017-01-11 17:34 ` [Qemu-devel] [PATCH v5 wave 1 4/4] fw-cfg: bump "file_slots" to 0x20 for 2.9+ machine types Laszlo Ersek 2017-01-12 2:25 ` [Qemu-arm] [PATCH v5 wave 1 0/4] fw-cfg: support writeable blobs and more files Gabriel L. Somlo 2017-01-12 2:25 ` [Qemu-devel] " Gabriel L. Somlo 2017-01-12 10:09 ` [Qemu-arm] " Laszlo Ersek 2017-01-12 10:09 ` [Qemu-devel] " Laszlo Ersek 2017-01-12 14:30 ` [Qemu-arm] " Michael S. Tsirkin 2017-01-12 14:30 ` [Qemu-devel] " Michael S. Tsirkin 2017-01-12 16:37 ` [Qemu-arm] " Laszlo Ersek 2017-01-12 16:37 ` [Qemu-devel] " Laszlo Ersek
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.