From 98bc3ab0f256cb983700089770db0823e59c7ceb Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Sun, 9 Mar 2014 18:42:06 +0200 Subject: [PATCH 1/6] 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 +- include/hw/loader.h | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) 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; } diff --git a/include/hw/loader.h b/include/hw/loader.h index aaf08c377e..3dc5b948cf 100644 --- a/include/hw/loader.h +++ b/include/hw/loader.h @@ -49,7 +49,7 @@ void pstrcpy_targphys(const char *name, hwaddr dest, int buf_size, const char *source); -extern bool rom_file_in_ram; +extern bool rom_file_has_mr; int rom_add_file(const char *file, const char *fw_dir, hwaddr addr, int32_t bootindex); From ac41881b48858b279960a17c07f0b159af91e75a Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Thu, 6 Mar 2014 14:57:09 +0200 Subject: [PATCH 2/6] 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 + include/hw/loader.h | 6 ++++-- 4 files changed, 12 insertions(+), 6 deletions(-) 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) diff --git a/include/hw/loader.h b/include/hw/loader.h index 3dc5b948cf..796cbf9b39 100644 --- a/include/hw/loader.h +++ b/include/hw/loader.h @@ -49,10 +49,12 @@ void pstrcpy_targphys(const char *name, hwaddr dest, int buf_size, const char *source); +extern bool option_rom_has_mr; extern bool rom_file_has_mr; int rom_add_file(const char *file, const char *fw_dir, - hwaddr addr, int32_t bootindex); + hwaddr addr, int32_t bootindex, + bool option_rom); void *rom_add_blob(const char *name, const void *blob, size_t len, hwaddr addr, const char *fw_file_name, FWCfgReadCallback fw_callback, void *callback_opaque); @@ -66,7 +68,7 @@ void *rom_ptr(hwaddr addr); void do_info_roms(Monitor *mon, const QDict *qdict); #define rom_add_file_fixed(_f, _a, _i) \ - rom_add_file(_f, NULL, _a, _i) + rom_add_file(_f, NULL, _a, _i, false) #define rom_add_blob_fixed(_f, _b, _l, _a) \ rom_add_blob(_f, _b, _l, _a, NULL, NULL, NULL) From dc655404659def26fbcd66583ca61575af9da8b9 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Sun, 9 Mar 2014 17:37:49 +0200 Subject: [PATCH 3/6] configure: don't modify .status on error ./configure --help make will try to re-run configure with --help which isn't what was intended. The reason is that config.status was written even on configure error. Defer writing config.status until configure has completed successfully. Signed-off-by: Michael S. Tsirkin Reviewed-by: Peter Maydell --- configure | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/configure b/configure index 8689435ccf..3ae57d7480 100755 --- a/configure +++ b/configure @@ -31,19 +31,6 @@ printf " '%s'" "$0" "$@" >> config.log echo >> config.log echo "#" >> config.log -# Save the configure command line for later reuse. -cat <config.status -#!/bin/sh -# Generated by configure. -# Run this file to recreate the current configuration. -# Compiler output produced by configure, useful for debugging -# configure, is in config.log if it exists. -EOD -printf "exec" >>config.status -printf " '%s'" "$0" "$@" >>config.status -echo >>config.status -chmod +x config.status - error_exit() { echo echo "ERROR: $1" @@ -5136,3 +5123,17 @@ done if test "$docs" = "yes" ; then mkdir -p QMP fi + +# Save the configure command line for later reuse. +cat <config.status +#!/bin/sh +# Generated by configure. +# Run this file to recreate the current configuration. +# Compiler output produced by configure, useful for debugging +# configure, is in config.log if it exists. +EOD +printf "exec" >>config.status +printf " '%s'" "$0" "$@" >>config.status +echo >>config.status +chmod +x config.status + From 263cf4367fd86dc0e15beebe65919cd50501844d Mon Sep 17 00:00:00 2001 From: BALATON Zoltan Date: Fri, 28 Feb 2014 11:28:03 +0100 Subject: [PATCH 4/6] 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 +++++----- include/hw/i386/ich9.h | 2 +- include/hw/pci-host/q35.h | 24 ++++++++++++------------ 3 files changed, 18 insertions(+), 18 deletions(-) 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); } diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h index 9e4a0e4b8d..e19143555e 100644 --- a/include/hw/i386/ich9.h +++ b/include/hw/i386/ich9.h @@ -102,7 +102,7 @@ Object *ich9_lpc_find(void); #define ICH9_USB_UHCI1_DEV 29 #define ICH9_USB_UHCI1_FUNC 0 -/* D30:F0 DMI-to-PCI brdige */ +/* D30:F0 DMI-to-PCI bridge */ #define ICH9_D2P_BRIDGE "ICH9 D2P BRIDGE" #define ICH9_D2P_BRIDGE_SAVEVM_VERSION 0 diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h index d0355b712b..d9ee97845b 100644 --- a/include/hw/pci-host/q35.h +++ b/include/hw/pci-host/q35.h @@ -125,8 +125,8 @@ typedef struct Q35PCIHost { #define MCH_HOST_BRIDGE_PAM_RE ((uint8_t)0x1) #define MCH_HOST_BRIDGE_PAM_MASK ((uint8_t)0x3) -#define MCH_HOST_BRDIGE_SMRAM 0x9d -#define MCH_HOST_BRDIGE_SMRAM_SIZE 1 +#define MCH_HOST_BRIDGE_SMRAM 0x9d +#define MCH_HOST_BRIDGE_SMRAM_SIZE 1 #define MCH_HOST_BRIDGE_SMRAM_DEFAULT ((uint8_t)0x2) #define MCH_HOST_BRIDGE_SMRAM_D_OPEN ((uint8_t)(1 << 6)) #define MCH_HOST_BRIDGE_SMRAM_D_CLS ((uint8_t)(1 << 5)) @@ -140,16 +140,16 @@ typedef struct Q35PCIHost { #define MCH_HOST_BRIDGE_UPPER_SYSTEM_BIOS_END 0x100000 #define MCH_HOST_BRIDGE_ESMRAMC 0x9e -#define MCH_HOST_BRDIGE_ESMRAMC_H_SMRAME ((uint8_t)(1 << 6)) -#define MCH_HOST_BRDIGE_ESMRAMC_E_SMERR ((uint8_t)(1 << 5)) -#define MCH_HOST_BRDIGE_ESMRAMC_SM_CACHE ((uint8_t)(1 << 4)) -#define MCH_HOST_BRDIGE_ESMRAMC_SM_L1 ((uint8_t)(1 << 3)) -#define MCH_HOST_BRDIGE_ESMRAMC_SM_L2 ((uint8_t)(1 << 2)) -#define MCH_HOST_BRDIGE_ESMRAMC_TSEG_SZ_MASK ((uint8_t)(0x3 << 1)) -#define MCH_HOST_BRDIGE_ESMRAMC_TSEG_SZ_1MB ((uint8_t)(0x0 << 1)) -#define MCH_HOST_BRDIGE_ESMRAMC_TSEG_SZ_2MB ((uint8_t)(0x1 << 1)) -#define MCH_HOST_BRDIGE_ESMRAMC_TSEG_SZ_8MB ((uint8_t)(0x2 << 1)) -#define MCH_HOST_BRDIGE_ESMRAMC_T_EN ((uint8_t)1) +#define MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME ((uint8_t)(1 << 6)) +#define MCH_HOST_BRIDGE_ESMRAMC_E_SMERR ((uint8_t)(1 << 5)) +#define MCH_HOST_BRIDGE_ESMRAMC_SM_CACHE ((uint8_t)(1 << 4)) +#define MCH_HOST_BRIDGE_ESMRAMC_SM_L1 ((uint8_t)(1 << 3)) +#define MCH_HOST_BRIDGE_ESMRAMC_SM_L2 ((uint8_t)(1 << 2)) +#define MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_MASK ((uint8_t)(0x3 << 1)) +#define MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_1MB ((uint8_t)(0x0 << 1)) +#define MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_2MB ((uint8_t)(0x1 << 1)) +#define MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_8MB ((uint8_t)(0x2 << 1)) +#define MCH_HOST_BRIDGE_ESMRAMC_T_EN ((uint8_t)1) /* D1:F0 PCIE* port*/ #define MCH_PCIE_DEV 1 From b4e5a4bffda0d5dd79c87c66f28a5fac87182e30 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Mon, 10 Mar 2014 21:30:16 +0200 Subject: [PATCH 5/6] 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(-) 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); } } From 13f65b2e1073cf7e2c8fb3880c77d8a53fa2f95e Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Mon, 10 Mar 2014 21:13:59 +0200 Subject: [PATCH 6/6] acpi-test: update expected SSDT files SSDT doesn't have _SUN for non hotpluggable slots anymore. Signed-off-by: Michael S. Tsirkin --- tests/acpi-test-data/pc/SSDT | Bin 2275 -> 2363 bytes tests/acpi-test-data/q35/SSDT | Bin 564 -> 652 bytes 2 files changed, 0 insertions(+), 0 deletions(-) diff --git a/tests/acpi-test-data/pc/SSDT b/tests/acpi-test-data/pc/SSDT index c1a4589db0db12289a4f367d2e1f377c9cb8aa2f..ae5a9a57d63c5bdc72b1a16c3ce36a9516143fbb 100644 GIT binary patch delta 184 zcmaDXxLb%TIM^k`nv;Qnk#!?iBqMi6Ekk^;Q+#x@2lwPW#(*@Ac<0~{AqO@VE@mM) z7C{j%!6qJtdIkijWMJfe!3;z~vM^~zMg|54s0yGo7ZdjvMuaS?W-cZ!4X%{Q`xxa^ sJo5vbJqBAFI&PtM?C zXmbuQFv#R^4)FIAa$#U(6mTyHaP~Cda0+(yVTf)L3pOx_=ZFsu_2XjTh<9`e;$Z-S R=qAHpLzp zZO#D(2ALer0sekME)0x}0v@aZ&YlL*O=7_Y2Jsy6!J&R!3>@)}E