From d0652b5765859049c96a13372bbe075be44e756b Mon Sep 17 00:00:00 2001 From: Shannon Zhao Date: Fri, 26 Jun 2015 14:22:36 +0100 Subject: [PATCH 1/7] hw/arm/virt-acpi-build: Fix table revision and some comments The table revision is not the ACPI spec version. Fix the wrong revision and also some comments. Signed-off-by: Shannon Zhao Reviewed-by: Michael S. Tsirkin Message-id: 1433820378-8336-1-git-send-email-zhaoshenglong@huawei.com Signed-off-by: Peter Maydell --- hw/arm/virt-acpi-build.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index d5a8b9c017..40029ddfda 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -387,7 +387,7 @@ build_mcfg(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info) mcfg->allocation[0].end_bus_number = (memmap[VIRT_PCIE_ECAM].size / PCIE_MMCFG_SIZE_MIN) - 1; - build_header(linker, table_data, (void *)mcfg, "MCFG", len, 5); + build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1); } /* GTDT */ @@ -413,7 +413,7 @@ build_gtdt(GArray *table_data, GArray *linker) build_header(linker, table_data, (void *)(table_data->data + gtdt_start), "GTDT", - table_data->len - gtdt_start, 5); + table_data->len - gtdt_start, 2); } /* MADT */ @@ -450,7 +450,7 @@ build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info, build_header(linker, table_data, (void *)(table_data->data + madt_start), "APIC", - table_data->len - madt_start, 5); + table_data->len - madt_start, 3); } /* FADT */ @@ -507,7 +507,7 @@ build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info) g_array_append_vals(table_data, dsdt->buf->data, dsdt->buf->len); build_header(linker, table_data, (void *)(table_data->data + table_data->len - dsdt->buf->len), - "DSDT", dsdt->buf->len, 5); + "DSDT", dsdt->buf->len, 2); free_aml_allocator(); } @@ -545,6 +545,7 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables) * FADT * GTDT * MADT + * MCFG * DSDT */ @@ -552,7 +553,7 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables) dsdt = tables_blob->len; build_dsdt(tables_blob, tables->linker, guest_info); - /* FADT MADT GTDT SPCR pointed to by RSDT */ + /* FADT MADT GTDT MCFG SPCR pointed to by RSDT */ acpi_add_table(table_offsets, tables_blob); build_fadt(tables_blob, tables->linker, dsdt); From ca7937365305d144cf0c97b907dac6f70ea152ef Mon Sep 17 00:00:00 2001 From: Shannon Zhao Date: Fri, 26 Jun 2015 14:22:36 +0100 Subject: [PATCH 2/7] hw/arm/virt-acpi-build: Add GICv2m description in ACPI MADT table Add GICv2m description in ACPI MADT table, so guest can use MSI when booting with ACPI. Signed-off-by: Shannon Zhao Signed-off-by: Shannon Zhao Reviewed-by: Andrew Jones Tested-by: Andrew Jones Message-id: 1434676210-2276-1-git-send-email-shannon.zhao@linaro.org Signed-off-by: Peter Maydell --- hw/arm/virt-acpi-build.c | 11 +++++++++++ include/hw/acpi/acpi-defs.h | 12 ++++++++++++ 2 files changed, 23 insertions(+) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 40029ddfda..f365140319 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -423,8 +423,10 @@ build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info, { int madt_start = table_data->len; const MemMapEntry *memmap = guest_info->memmap; + const int *irqmap = guest_info->irqmap; AcpiMultipleApicTable *madt; AcpiMadtGenericDistributor *gicd; + AcpiMadtGenericMsiFrame *gic_msi; int i; madt = acpi_data_push(table_data, sizeof *madt); @@ -448,6 +450,15 @@ build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info, gicd->length = sizeof(*gicd); gicd->base_address = memmap[VIRT_GIC_DIST].base; + gic_msi = acpi_data_push(table_data, sizeof *gic_msi); + gic_msi->type = ACPI_APIC_GENERIC_MSI_FRAME; + gic_msi->length = sizeof(*gic_msi); + gic_msi->gic_msi_frame_id = 0; + gic_msi->base_address = cpu_to_le64(memmap[VIRT_GIC_V2M].base); + gic_msi->flags = cpu_to_le32(1); + gic_msi->spi_count = cpu_to_le16(NUM_GICV2M_SPIS); + gic_msi->spi_base = cpu_to_le16(irqmap[VIRT_GIC_V2M] + ARM_SPI_BASE); + build_header(linker, table_data, (void *)(table_data->data + madt_start), "APIC", table_data->len - madt_start, 3); diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h index 7b4bfb7494..2b431e6242 100644 --- a/include/hw/acpi/acpi-defs.h +++ b/include/hw/acpi/acpi-defs.h @@ -372,6 +372,18 @@ struct AcpiMadtGenericDistributor { typedef struct AcpiMadtGenericDistributor AcpiMadtGenericDistributor; +struct AcpiMadtGenericMsiFrame { + ACPI_SUB_HEADER_DEF + uint16_t reserved; + uint32_t gic_msi_frame_id; + uint64_t base_address; + uint32_t flags; + uint16_t spi_count; + uint16_t spi_base; +} QEMU_PACKED; + +typedef struct AcpiMadtGenericMsiFrame AcpiMadtGenericMsiFrame; + /* * Generic Timer Description Table (GTDT) */ From f3c2bda216a00676e40301b5843ac3d6c3b2537a Mon Sep 17 00:00:00 2001 From: Liviu Ionescu Date: Fri, 26 Jun 2015 14:22:36 +0100 Subject: [PATCH 3/7] target-arm: default empty semihosting cmdline If neither explicit semihosting args nor -kernel are used, make SYS_GET_CMDLINE return an empty string. Signed-off-by: Liviu Ionescu Message-id: AC7B5AFC-06AE-4FAD-9852-B65708E80E09@livius.net Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- target-arm/arm-semi.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/target-arm/arm-semi.c b/target-arm/arm-semi.c index 74a67e9fdd..a2a7369567 100644 --- a/target-arm/arm-semi.c +++ b/target-arm/arm-semi.c @@ -436,12 +436,19 @@ uint32_t do_arm_semihosting(CPUARMState *env) size_t input_size; size_t output_size; int status = 0; +#if !defined(CONFIG_USER_ONLY) + const char *cmdline; +#endif GET_ARG(0); GET_ARG(1); input_size = arg1; /* Compute the size of the output string. */ #if !defined(CONFIG_USER_ONLY) - output_size = strlen(semihosting_get_cmdline()) + 1; + cmdline = semihosting_get_cmdline(); + if (cmdline == NULL) { + cmdline = ""; /* Default to an empty line. */ + } + output_size = strlen(cmdline) + 1; /* Count terminating 0. */ #else unsigned int i; @@ -472,7 +479,7 @@ uint32_t do_arm_semihosting(CPUARMState *env) /* Copy the command-line arguments. */ #if !defined(CONFIG_USER_ONLY) - pstrcpy(output_buffer, output_size, semihosting_get_cmdline()); + pstrcpy(output_buffer, output_size, cmdline); #else if (output_size == 1) { /* Empty command-line. */ From b21ab1fc217b4a2b8f2f85d16bdd8510a7817a34 Mon Sep 17 00:00:00 2001 From: Soren Brinkmann Date: Fri, 26 Jun 2015 14:22:36 +0100 Subject: [PATCH 4/7] target-arm: A64: Print ELR when taking exceptions When taking an exception print the content of the exception link register. This is useful especially for synchronous exceptions because in that case this registers holds the address of the instruction that generated the exception. Signed-off-by: Soren Brinkmann Message-id: 1435036655-16132-1-git-send-email-soren.brinkmann@xilinx.com Signed-off-by: Peter Maydell --- target-arm/helper-a64.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c index e30af0659e..08c95a3f52 100644 --- a/target-arm/helper-a64.c +++ b/target-arm/helper-a64.c @@ -533,6 +533,8 @@ void aarch64_cpu_do_interrupt(CPUState *cs) env->condexec_bits = 0; } + qemu_log_mask(CPU_LOG_INT, "...with ELR 0x%" PRIx64 "\n", + env->elr_el[new_el]); pstate_write(env, PSTATE_DAIF | new_mode); env->aarch64 = 1; From f1fb9f0dc087c02b230be4cc96c5c76521f188fa Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 26 Jun 2015 14:22:36 +0100 Subject: [PATCH 5/7] qdev-properties-system: Change set_pointer's parse callback to use Error Instead of having set_pointer() call a parse callback which returns an error number that we then convert to an Error string with error_set_from_qdev_prop_error(), make the parse callback take an Error** and set the error itself. This will allow parse routines to provide more helpful error messages than the generic ones. Signed-off-by: Peter Maydell Reviewed-by: Markus Armbruster Reviewed-by: Stefan Hajnoczi Message-id: 1435068107-12594-2-git-send-email-peter.maydell@linaro.org --- hw/core/qdev-properties-system.c | 33 +++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c index aa794ca445..c731bc657f 100644 --- a/hw/core/qdev-properties-system.c +++ b/hw/core/qdev-properties-system.c @@ -35,15 +35,15 @@ static void get_pointer(Object *obj, Visitor *v, Property *prop, } static void set_pointer(Object *obj, Visitor *v, Property *prop, - int (*parse)(DeviceState *dev, const char *str, - void **ptr), + void (*parse)(DeviceState *dev, const char *str, + void **ptr, const char *propname, + Error **errp), const char *name, Error **errp) { DeviceState *dev = DEVICE(obj); Error *local_err = NULL; void **ptr = qdev_get_prop_ptr(dev, prop); char *str; - int ret; if (dev->realized) { qdev_prop_set_after_realize(dev, name, errp); @@ -60,26 +60,29 @@ static void set_pointer(Object *obj, Visitor *v, Property *prop, *ptr = NULL; return; } - ret = parse(dev, str, ptr); - error_set_from_qdev_prop_error(errp, ret, dev, prop, str); + parse(dev, str, ptr, prop->name, errp); g_free(str); } /* --- drive --- */ -static int parse_drive(DeviceState *dev, const char *str, void **ptr) +static void parse_drive(DeviceState *dev, const char *str, void **ptr, + const char *propname, Error **errp) { BlockBackend *blk; blk = blk_by_name(str); if (!blk) { - return -ENOENT; + error_setg(errp, "Property '%s.%s' can't find value '%s'", + object_get_typename(OBJECT(dev)), propname, str); + return; } if (blk_attach_dev(blk, dev) < 0) { - return -EEXIST; + error_setg(errp, "Property '%s.%s' can't take value '%s', it's in use", + object_get_typename(OBJECT(dev)), propname, str); + return; } *ptr = blk; - return 0; } static void release_drive(Object *obj, const char *name, void *opaque) @@ -121,17 +124,21 @@ PropertyInfo qdev_prop_drive = { /* --- character device --- */ -static int parse_chr(DeviceState *dev, const char *str, void **ptr) +static void parse_chr(DeviceState *dev, const char *str, void **ptr, + const char *propname, Error **errp) { CharDriverState *chr = qemu_chr_find(str); if (chr == NULL) { - return -ENOENT; + error_setg(errp, "Property '%s.%s' can't find value '%s'", + object_get_typename(OBJECT(dev)), propname, str); + return; } if (qemu_chr_fe_claim(chr) != 0) { - return -EEXIST; + error_setg(errp, "Property '%s.%s' can't take value '%s', it's in use", + object_get_typename(OBJECT(dev)), propname, str); + return; } *ptr = chr; - return 0; } static void release_chr(Object *obj, const char *name, void *opaque) From 62f7dbde4c75e48921fd1b773865250130c57bd8 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 26 Jun 2015 14:22:36 +0100 Subject: [PATCH 6/7] qdev-properties-system: Improve error message for drive assignment conflict If the user forgot if=none on their drive specification they're likely to get an error message because the drive is assigned once automatically by QEMU and once by the manual id=/drive= user command line specification. Improve the error message produced in this case to explicitly guide the user towards if=none. We rephrase the "drive conflict but not for an if=something" error as well to keep the wording in line. The two cases that change are: (1) Drive specified as to be auto-connected and also manually connected (and the board does handle this if= type): qemu-system-x86_64 -nodefaults -display none \ -drive if=scsi,file=tmp.qcow2,id=foo -device ide-hd,drive=foo Previously: qemu-system-x86_64: -device ide-hd,drive=foo: Property 'ide-hd.drive' can't take value 'foo', it's in use Now: qemu-system-x86_64: -device ide-hd,drive=foo: Drive 'foo' is already in use because it has been automatically connected to another device (did you need 'if=none' in the drive options?) (2) Drive specified to be manually connected in two different ways: qemu-system-x86_64 -nodefaults -display none \ -drive if=none,file=tmp.qcow2,id=foo -device ide-hd,drive=foo \ -device ide-hd,drive=foo Previously: qemu-system-x86_64: -device ide-hd,drive=foo: Property 'ide-hd.drive' can't take value 'foo', it's in use Now: qemu-system-x86_64: -device ide-hd,drive=foo: Drive 'foo' is already in use by another device Signed-off-by: Peter Maydell Reviewed-by: Markus Armbruster Reviewed-by: Stefan Hajnoczi Message-id: 1435068107-12594-3-git-send-email-peter.maydell@linaro.org --- hw/core/qdev-properties-system.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c index c731bc657f..921e799dbb 100644 --- a/hw/core/qdev-properties-system.c +++ b/hw/core/qdev-properties-system.c @@ -78,8 +78,17 @@ static void parse_drive(DeviceState *dev, const char *str, void **ptr, return; } if (blk_attach_dev(blk, dev) < 0) { - error_setg(errp, "Property '%s.%s' can't take value '%s', it's in use", - object_get_typename(OBJECT(dev)), propname, str); + DriveInfo *dinfo = blk_legacy_dinfo(blk); + + if (dinfo->type != IF_NONE) { + error_setg(errp, "Drive '%s' is already in use because " + "it has been automatically connected to another " + "device (did you need 'if=none' in the drive options?)", + str); + } else { + error_setg(errp, "Drive '%s' is already in use by another device", + str); + } return; } *ptr = blk; From 4e2c0b2a4ab810c8989e181a010e75aeaa1c55f3 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 26 Jun 2015 14:22:37 +0100 Subject: [PATCH 7/7] hw/arm/virt: Make block devices default to virtio Now we have virtio-pci, we can make the virt board's default block device type be IF_VIRTIO. This allows users to use simplified command lines that don't have to explicitly create virtio-pci-blk devices; the -hda &c very short options now also work. This means we also need to set no_cdrom to avoid getting a default cdrom device -- this is needed because the virtio-blk device will fail if it is connected to a block backend with no media, which is what the default cdrom device typically is. Providing a cdrom with media via -cdrom will succeed, but silently create a device with non-removable medium. this is probably not really what the user wants, but is the best we can do now. Note that this change means that some command lines which used to work (by accident) will stop working. Where a drive was connected manually to a device but without 'if=none' being specified, we used to treat this as an IDE drive, which we would then not autoplug because the board doesn't support IDE. Now we will treat it as a virtio disk and autoplug it, which means the attempt to use the drive manually will fail: qemu-system-arm: -drive file=img.qcow2,id=foo: Drive 'foo' is already in use because it has been automatically connected to another device (did you need 'if=none' in the drive options?) The command line will have to be changed to include 'if=none', as the error message suggests. Signed-off-by: Peter Maydell Reviewed-by: Markus Armbruster Reviewed-by: Stefan Hajnoczi Message-id: 1435068107-12594-4-git-send-email-peter.maydell@linaro.org --- hw/arm/virt.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 4e78083a9d..484689264c 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -956,6 +956,8 @@ static void virt_class_init(ObjectClass *oc, void *data) mc->init = machvirt_init; mc->max_cpus = 8; mc->has_dynamic_sysbus = true; + mc->block_default_type = IF_VIRTIO; + mc->no_cdrom = 1; } static const TypeInfo machvirt_info = {