From 98bc3ab0f256cb983700089770db0823e59c7ceb Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Sun, 9 Mar 2014 18:42:06 +0200 Subject: loader: rename in_ram/has_mr we put copy of ROMs in MR for migration. but the name rom_in_ram makes one think we load it in guest RAM. Rename has_mr to make intent clearer. Signed-off-by: Michael S. Tsirkin --- hw/core/loader.c | 6 +++--- hw/i386/pc_piix.c | 2 +- hw/i386/pc_q35.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) (limited to 'hw') diff --git a/hw/core/loader.c b/hw/core/loader.c index b323c0c7b8..13e98d8dbb 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -54,7 +54,7 @@ #include -bool rom_file_in_ram = true; +bool rom_file_has_mr = true; static int roms_loaded; @@ -694,7 +694,7 @@ int rom_add_file(const char *file, const char *fw_dir, basename); snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name); - if (rom_file_in_ram) { + if (rom_file_has_mr) { data = rom_set_mr(rom, OBJECT(fw_cfg), devpath); } else { data = rom->data; @@ -738,7 +738,7 @@ void *rom_add_blob(const char *name, const void *blob, size_t len, snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name); - if (rom_file_in_ram) { + if (rom_file_has_mr) { data = rom_set_mr(rom, OBJECT(fw_cfg), devpath); } else { data = rom->data; diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index ae1699d6db..fb2d636a78 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -272,7 +272,7 @@ static void pc_compat_1_6(QEMUMachineInitArgs *args) { pc_compat_1_7(args); has_pci_info = false; - rom_file_in_ram = false; + rom_file_has_mr = false; has_acpi_build = false; } diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index a7f626096a..eb55ae4823 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -250,7 +250,7 @@ static void pc_compat_1_6(QEMUMachineInitArgs *args) { pc_compat_1_7(args); has_pci_info = false; - rom_file_in_ram = false; + rom_file_has_mr = false; has_acpi_build = false; } -- cgit v1.2.1 From ac41881b48858b279960a17c07f0b159af91e75a Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Thu, 6 Mar 2014 14:57:09 +0200 Subject: pc: avoid duplicate names for ROM MRs Since commit 04920fc0faa4760f9c4fc0e73b992b768099be70 loader: store FW CFG ROM files in RAM RAM MRs including ROM files in FW CFGs are created and named using the file basename. This becomes problematic if these names are supplied by user, since the basename might not be unique. There are two cases we care about: - option-rom flag. - option ROM for devices. This triggers e.g. when using rombar=0. At the moment we get an assert. E.g qemu -option-rom /usr/share/ipxe/8086100e.rom -option-rom /usr/share/ipxe.efi/8086100e.rom RAMBlock "/rom@genroms/8086100e.rom" already registered, abort! This is a regression from 1.6. For now let's keep it simple and just avoid creating the MRs in case of option ROMs. when using 1.7 machine types, enable option ROMs in RAM to match that version. Signed-off-by: Michael S. Tsirkin --- hw/core/loader.c | 10 ++++++---- hw/i386/pc_piix.c | 1 + hw/i386/pc_q35.c | 1 + 3 files changed, 8 insertions(+), 4 deletions(-) (limited to 'hw') diff --git a/hw/core/loader.c b/hw/core/loader.c index 13e98d8dbb..2bf6b8ff85 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -54,6 +54,7 @@ #include +bool option_rom_has_mr = false; bool rom_file_has_mr = true; static int roms_loaded; @@ -642,7 +643,8 @@ static void *rom_set_mr(Rom *rom, Object *owner, const char *name) } int rom_add_file(const char *file, const char *fw_dir, - hwaddr addr, int32_t bootindex) + hwaddr addr, int32_t bootindex, + bool option_rom) { Rom *rom; int rc, fd = -1; @@ -694,7 +696,7 @@ int rom_add_file(const char *file, const char *fw_dir, basename); snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name); - if (rom_file_has_mr) { + if ((!option_rom || option_rom_has_mr) && rom_file_has_mr) { data = rom_set_mr(rom, OBJECT(fw_cfg), devpath); } else { data = rom->data; @@ -773,12 +775,12 @@ int rom_add_elf_program(const char *name, void *data, size_t datasize, int rom_add_vga(const char *file) { - return rom_add_file(file, "vgaroms", 0, -1); + return rom_add_file(file, "vgaroms", 0, -1, true); } int rom_add_option(const char *file, int32_t bootindex) { - return rom_add_file(file, "genroms", 0, bootindex); + return rom_add_file(file, "genroms", 0, bootindex, true); } static void rom_reset(void *unused) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index fb2d636a78..5e1d2d3de3 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -266,6 +266,7 @@ static void pc_compat_1_7(QEMUMachineInitArgs *args) { smbios_type1_defaults = false; gigabyte_align = false; + option_rom_has_mr = true; } static void pc_compat_1_6(QEMUMachineInitArgs *args) diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index eb55ae4823..4b0456a95b 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -244,6 +244,7 @@ static void pc_compat_1_7(QEMUMachineInitArgs *args) { smbios_type1_defaults = false; gigabyte_align = false; + option_rom_has_mr = true; } static void pc_compat_1_6(QEMUMachineInitArgs *args) -- cgit v1.2.1 From 263cf4367fd86dc0e15beebe65919cd50501844d Mon Sep 17 00:00:00 2001 From: BALATON Zoltan Date: Fri, 28 Feb 2014 11:28:03 +0100 Subject: q35: Correct typo BRDIGE -> BRIDGE Signed-off-by: BALATON Zoltan Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/pci-host/q35.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'hw') diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index 4bc2e0118e..8b8cc4e294 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -272,7 +272,7 @@ static void mch_update_smram(MCHPCIState *mch) PCIDevice *pd = PCI_DEVICE(mch); memory_region_transaction_begin(); - smram_update(&mch->smram_region, pd->config[MCH_HOST_BRDIGE_SMRAM], + smram_update(&mch->smram_region, pd->config[MCH_HOST_BRIDGE_SMRAM], mch->smm_enabled); memory_region_transaction_commit(); } @@ -283,7 +283,7 @@ static void mch_set_smm(int smm, void *arg) PCIDevice *pd = PCI_DEVICE(mch); memory_region_transaction_begin(); - smram_set_smm(&mch->smm_enabled, smm, pd->config[MCH_HOST_BRDIGE_SMRAM], + smram_set_smm(&mch->smm_enabled, smm, pd->config[MCH_HOST_BRIDGE_SMRAM], &mch->smram_region); memory_region_transaction_commit(); } @@ -306,8 +306,8 @@ static void mch_write_config(PCIDevice *d, mch_update_pciexbar(mch); } - if (ranges_overlap(address, len, MCH_HOST_BRDIGE_SMRAM, - MCH_HOST_BRDIGE_SMRAM_SIZE)) { + if (ranges_overlap(address, len, MCH_HOST_BRIDGE_SMRAM, + MCH_HOST_BRIDGE_SMRAM_SIZE)) { mch_update_smram(mch); } } @@ -347,7 +347,7 @@ static void mch_reset(DeviceState *qdev) pci_set_quad(d->config + MCH_HOST_BRIDGE_PCIEXBAR, MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT); - d->config[MCH_HOST_BRDIGE_SMRAM] = MCH_HOST_BRIDGE_SMRAM_DEFAULT; + d->config[MCH_HOST_BRIDGE_SMRAM] = MCH_HOST_BRIDGE_SMRAM_DEFAULT; mch_update(mch); } -- cgit v1.2.1 From b4e5a4bffda0d5dd79c87c66f28a5fac87182e30 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Mon, 10 Mar 2014 21:30:16 +0200 Subject: acpi-build: don't access unaligned addresses casting an unaligned address to e.g. uint32_t can trigger undefined behaviour in C. Replace cast + assignment with memcpy. Reported-by: Peter Maydell Signed-off-by: Michael S. Tsirkin --- hw/i386/acpi-build.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) (limited to 'hw') diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index b667d31de5..7ecfd7004b 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -466,9 +466,15 @@ static void acpi_align_size(GArray *blob, unsigned align) g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align)); } -/* Get pointer within table in a safe manner */ -#define ACPI_BUILD_PTR(table, size, off, type) \ - ((type *)(acpi_data_get_ptr(table, size, off, sizeof(type)))) +/* Set a value within table in a safe manner */ +#define ACPI_BUILD_SET_LE(table, size, off, bits, val) \ + do { \ + uint64_t ACPI_BUILD_SET_LE_val = cpu_to_le64(val); \ + memcpy(acpi_data_get_ptr(table, size, off, \ + (bits) / BITS_PER_BYTE), \ + &ACPI_BUILD_SET_LE_val, \ + (bits) / BITS_PER_BYTE); \ + } while (0) static inline void *acpi_data_get_ptr(uint8_t *table_data, unsigned table_size, unsigned off, unsigned size) @@ -974,22 +980,17 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) static void patch_pci_windows(PcPciInfo *pci, uint8_t *start, unsigned size) { - *ACPI_BUILD_PTR(start, size, acpi_pci32_start[0], uint32_t) = - cpu_to_le32(pci->w32.begin); + ACPI_BUILD_SET_LE(start, size, acpi_pci32_start[0], 32, pci->w32.begin); - *ACPI_BUILD_PTR(start, size, acpi_pci32_end[0], uint32_t) = - cpu_to_le32(pci->w32.end - 1); + ACPI_BUILD_SET_LE(start, size, acpi_pci32_end[0], 32, pci->w32.end - 1); if (pci->w64.end || pci->w64.begin) { - *ACPI_BUILD_PTR(start, size, acpi_pci64_valid[0], uint8_t) = 1; - *ACPI_BUILD_PTR(start, size, acpi_pci64_start[0], uint64_t) = - cpu_to_le64(pci->w64.begin); - *ACPI_BUILD_PTR(start, size, acpi_pci64_end[0], uint64_t) = - cpu_to_le64(pci->w64.end - 1); - *ACPI_BUILD_PTR(start, size, acpi_pci64_length[0], uint64_t) = - cpu_to_le64(pci->w64.end - pci->w64.begin); + ACPI_BUILD_SET_LE(start, size, acpi_pci64_valid[0], 8, 1); + ACPI_BUILD_SET_LE(start, size, acpi_pci64_start[0], 64, pci->w64.begin); + ACPI_BUILD_SET_LE(start, size, acpi_pci64_end[0], 64, pci->w64.end - 1); + ACPI_BUILD_SET_LE(start, size, acpi_pci64_length[0], 64, pci->w64.end - pci->w64.begin); } else { - *ACPI_BUILD_PTR(start, size, acpi_pci64_valid[0], uint8_t) = 0; + ACPI_BUILD_SET_LE(start, size, acpi_pci64_valid[0], 8, 0); } } -- cgit v1.2.1