Re: [Qemu-devel] [RFC PATCH v3 12/24] spapr: CPU hotplug support
On Tue, May 05, 2015 at 04:59:51PM +1000, David Gibson wrote: On Fri, Apr 24, 2015 at 12:17:34PM +0530, Bharata B Rao wrote: Support CPU hotplug via device-add command. Set up device tree entries for the hotplugged CPU core and use the exising EPOW event infrastructure to send CPU hotplug notification to the guest. Also support cold plugged CPUs that are specified by -device option on cmdline. Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com --- hw/ppc/spapr.c| 129 ++ hw/ppc/spapr_events.c | 8 ++-- hw/ppc/spapr_rtas.c | 11 + 3 files changed, 145 insertions(+), 3 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index b526b7d..9b0701c 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -33,6 +33,7 @@ #include sysemu/block-backend.h #include sysemu/cpus.h #include sysemu/kvm.h +#include sysemu/device_tree.h #include kvm_ppc.h #include mmu-hash64.h #include qom/cpu.h @@ -662,6 +663,17 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset) unsigned sockets = opts ? qemu_opt_get_number(opts, sockets, 0) : 0; uint32_t cpus_per_socket = sockets ? (smp_cpus / sockets) : 1; uint32_t pft_size_prop[] = {0, cpu_to_be32(spapr-htab_shift)}; +sPAPRDRConnector *drc; +sPAPRDRConnectorClass *drck; +int drc_index; + +if (spapr-dr_cpu_enabled) { +drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index); +g_assert(drc); +drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); +drc_index = drck-get_index(drc); +_FDT((fdt_setprop_cell(fdt, offset, ibm,my-drc-index, drc_index))); +} _FDT((fdt_setprop_cell(fdt, offset, reg, index))); _FDT((fdt_setprop_string(fdt, offset, device_type, cpu))); @@ -1850,6 +1862,114 @@ static void spapr_nmi(NMIState *n, int cpu_index, Error **errp) } } +static void *spapr_populate_hotplug_cpu_dt(DeviceState *dev, CPUState *cs, +int *fdt_offset) +{ +PowerPCCPU *cpu = POWERPC_CPU(cs); +DeviceClass *dc = DEVICE_GET_CLASS(cs); +int id = ppc_get_vcpu_dt_id(cpu); +void *fdt; +int offset, fdt_size; +char *nodename; + +fdt = create_device_tree(fdt_size); +nodename = g_strdup_printf(%s@%x, dc-fw_name, id); +offset = fdt_add_subnode(fdt, 0, nodename); + +spapr_populate_cpu_dt(cs, fdt, offset); +g_free(nodename); + +*fdt_offset = offset; +return fdt; +} + +static void spapr_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev, +Error **errp) +{ +CPUState *cs = CPU(dev); +PowerPCCPU *cpu = POWERPC_CPU(cs); +int id = ppc_get_vcpu_dt_id(cpu); +sPAPRDRConnector *drc = +spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, id); +sPAPRDRConnectorClass *drck; +int smt = kvmppc_smt_threads(); +Error *local_err = NULL; +void *fdt = NULL; +int i, fdt_offset = 0; + +/* Set NUMA node for the added CPUs */ +for (i = 0; i nb_numa_nodes; i++) { +if (test_bit(cs-cpu_index, numa_info[i].node_cpu)) { +cs-numa_node = i; +break; +} +} + +/* + * SMT threads return from here, only main thread (core) will + * continue and signal hotplug event to the guest. + */ +if ((id % smt) != 0) { +return; +} Couldn't you avoid this by attaching this call to the core device, rather than the individual vcpu thread objects? Adding a socket device will result in realize call for that device. Socket realizefn will call core realizefn and core realizefn will call thread (or CPU) realizefn. device_set_realized() will call -plug handler for all these devices (socket, cores and threads) and that's how we end up here even for threads. This will be same when I get rid of socket abstraction and hot plug cores for the same reason as above. And calling -plug handler in the context of threads is required to initialize board specific CPU bits for the CPU thread that is being realized. +if (!spapr-dr_cpu_enabled) { +/* + * This is a cold plugged CPU but the machine doesn't support + * DR. So skip the hotplug path ensuring that the CPU is brought + * up online with out an associated DR connector. + */ +return; +} + +g_assert(drc); + +/* + * Setup CPU DT entries only for hotplugged CPUs. For boot time or + * coldplugged CPUs DT entries are setup in spapr_finalize_fdt(). + */ +if (dev-hotplugged) { +fdt = spapr_populate_hotplug_cpu_dt(dev, cs, fdt_offset); +} + +drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); +drck-attach(drc, dev, fdt, fdt_offset, !dev-hotplugged,
Re: [Qemu-devel] [RFC PATCH v3 06/24] spapr: Consolidate cpu init code into a routine
On Wed, 6 May 2015 09:58:09 +0530 Bharata B Rao bhar...@linux.vnet.ibm.com wrote: On Mon, May 04, 2015 at 06:10:59PM +0200, Thomas Huth wrote: On Fri, 24 Apr 2015 12:17:28 +0530 Bharata B Rao bhar...@linux.vnet.ibm.com wrote: Factor out bits of sPAPR specific CPU initialization code into a separate routine so that it can be called from CPU hotplug path too. Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com Reviewed-by: David Gibson da...@gibson.dropbear.id.au --- hw/ppc/spapr.c | 54 +- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index a56f9a1..5c8f2ff 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1440,6 +1440,34 @@ static void spapr_drc_reset(void *opaque) } } +static void spapr_cpu_init(PowerPCCPU *cpu) +{ +CPUPPCState *env = cpu-env; + +/* Set time-base frequency to 512 MHz */ +cpu_ppc_tb_init(env, TIMEBASE_FREQ); + +/* PAPR always has exception vectors in RAM not ROM. To ensure this, + * MSR[IP] should never be set. + */ +env-msr_mask = ~(1 6); While you're at it ... could we maybe get a proper #define for that MSR bit? (just like the other ones in target-ppc/cpu.h) Sure will use MSR_EP here next time. According to the comment in cpu.h, the EP bit was for the 601 CPU only, so I think it would be better to introduce a new #define MSR_IP 6 (or so) here instead. Thomas
Re: [Qemu-devel] [PATCH v3 2/6] spapr_pci: encode missing 64-bit memory address space
On Wed, 06 May 2015 11:14:32 +0530 Nikunj A Dadhania nik...@linux.vnet.ibm.com wrote: Thomas Huth th...@redhat.com writes: [...] BTW, does this also require the new version of SLOF already? Not yet, only after patch 4/6 newer SLOF would be needed. Ok, ... but it will also still work with old SLOF version? If not, I think you should eventually also include a patch in this series to update the slof.bin of QEMU. Thomas
Re: [Qemu-devel] [PATCH 1/4] block: Fix dirty bitmap in bdrv_co_discard
On 06/05/2015 03:45, Fam Zheng wrote: This is not enough, you also have to do the discard in block/mirror.c, otherwise the destination image could even become fully provisioned! I wasn't sure what if src and dest have different can_write_zeroes_with_unmap value, but your argument is stronger. I will add discard in mirror. Besides that, do we need to compare the can_write_zeroes_with_unmap? Hmm, if can_write_zeroes_with_unmap is set, it's probably better to write zeroes instead of discarding, in case the guest is relying on it. Paolo
Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
On Tue, 5 May 2015 10:26:02 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Mon, Apr 27, 2015 at 04:53:16PM +0200, Michael Mueller wrote: The patch defines ids per accelerator and adds the accel_id and the model_name to the CPUState. The accel_id is initialized by common code, the model name needs to be initialized by target specific code. Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com Acked-by: Christian Borntraeger borntrae...@de.ibm.com --- include/qom/cpu.h | 5 + qapi-schema.json | 9 + qom/cpu.c | 14 ++ 3 files changed, 28 insertions(+) diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 9dafb48..4ffc050 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -236,6 +236,8 @@ struct kvm_run; * @mem_io_pc: Host Program Counter at which the memory was accessed. * @mem_io_vaddr: Target virtual address at which the memory was accessed. * @kvm_fd: vCPU file descriptor for KVM. + * @accel_id: accelerator id of this CPU. + * @model_name: model name of this CPU * * State of one CPU core or thread. */ @@ -313,6 +315,9 @@ struct CPUState { (absolute value) offset as small as possible. This reduces code size, especially for hosts without large memory offsets. */ volatile sig_atomic_t tcg_exit_req; + +AccelId accel_id; This can be a AccelState pointer, set on initialization, because we have another user case for having a AccelState pointer: query-cpu-definition implementations may create temporary CPU objects with a different accel object to be able to probe for accel-specific data. (The pointer may become a link QOM property later.) +char *model_name; }; QTAILQ_HEAD(CPUTailQ, CPUState); diff --git a/qapi-schema.json b/qapi-schema.json index ac9594d..540e520 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2515,6 +2515,15 @@ ## { 'command': 'query-machines', 'returns': ['MachineInfo'] } +# @AccelId +# +# Defines accelerator ids +# +# Since: 2.4 +## +{ 'enum': 'AccelId', + 'data': ['qtest', 'tcg', 'kvm', 'xen'] } + Not sure if it is better to have an enum or simply a string here. ## # @CpuDefinitionInfo: # diff --git a/qom/cpu.c b/qom/cpu.c index 108bfa2..457afc7 100644 --- a/qom/cpu.c +++ b/qom/cpu.c @@ -67,6 +67,20 @@ CPUState *cpu_generic_init(const char *typename, const char *cpu_model) goto out; } +if (tcg_enabled()) { +cpu-accel_id = ACCEL_ID_TCG; +} else if (kvm_enabled()) { +cpu-accel_id = ACCEL_ID_KVM; +} +#ifdef CONFIG_XEN +else if (xen_enabled()) { +cpu-accel_id = ACCEL_ID_XEN; +} +#endif +else { +cpu-accel_id = ACCEL_ID_QTEST; +} You can simply use ACCEL_GET_CLASS(current_machine-accelerator)-name here. If we really want an enum, we can add an AccelId field to AccelClass, and initialize it properly on the accel class_init functions. The AccelClass (ac = ACCEL_GET_CLASS(current_machine-accelerator) would be ok though. That will allow to access the ac-accel_id (not yet there) at places where required. I'm just not sure how to access current_machine here. CONFIG_USER may require some special code when returning the accelerator ID as tcg because IIRC it doesn't use the QOM accel classes (yet). + object_property_set_bool(OBJECT(cpu), true, realized, err); out: -- 1.8.3.1
Re: [Qemu-devel] [RFC PATCH v3 23/24] spapr: Support ibm, dynamic-reconfiguration-memory
On Tue, May 05, 2015 at 05:40:32PM +1000, David Gibson wrote: On Fri, Apr 24, 2015 at 12:17:45PM +0530, Bharata B Rao wrote: Parse ibm,architecture.vec table obtained from the guest and enable memory node configuration via ibm,dynamic-reconfiguration-memory if guest supports it. This is in preparation to support memory hotplug for sPAPR guests. This changes the way memory node configuration is done. Currently all memory nodes are built upfront. But after this patch, only memory@0 node for RMA is built upfront. Guest kernel boots with just that and rest of the memory nodes (via memory@XXX or ibm,dynamic-reconfiguration-memory) are built when guest does ibm,client-architecture-support call. Note: This patch needs a SLOF enhancement which is already part of upstream SLOF. Is it in the SLOF included in the qemu submodule though? If not you should have a patch to update the submodule first. Nikunj confirms that SLOF change needed to support ibm,dynamic-reconfiguration-memory is already part of QEMU. Regards, Bharata.
Re: [Qemu-devel] [PATCH v6 01/17] Introduce stub routine cpu_desc_avail
On Tue, 5 May 2015 14:41:01 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Tue, May 05, 2015 at 06:12:16PM +0200, Michael Mueller wrote: On Tue, 5 May 2015 10:55:47 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Mon, Apr 27, 2015 at 04:53:15PM +0200, Michael Mueller wrote: This patch introduces the function cpu_desc_avail() which returns by default true if not architecture specific implemented. Its intention is to indicate if the cpu model description is available for display by list_cpus(). This change allows cpu model descriptions to become dynamically created by evaluating the runtime context instead of putting static cpu model information at display. Why are you deliberately breaking -cpu ? when cpu_desc_avail() is false? In the s390x case cpu_desc_avail() is per se false in this code section of vl.c: /* Init CPU def lists, based on config * - Must be called after all the qemu_read_config_file() calls * - Must be called before list_cpu() * - Must be called before machine-init() */ (Side note: I believe the above outdated, I will send a patch to update it.) Will be interesting to see what the change is, master is currently showing this code. cpudef_init(); if (cpu_model cpu_desc_avail() is_help_option(cpu_model)) { list_cpus(stdout, fprintf, cpu_model); exit(0); } That is because the output does not solely depend on static definitions but also on runtime context. Here the host machine type this instance of QEMU is running on, at least for the KVM case. Is this a required feature? I would prefer to have the main() code simple even if it means not having runnable information in -cpu ? by now (about possible ways to implement this without cpu_desc_avail(), see below). I think it is more than a desired feature because one might end up with a failed CPU object instantiation although the help screen claims to CPU model to be valid. Once the accelerator has been initialized AND the S390 cpu classes have been setup by means of the following code: static void kvm_setup_cpu_classes(KVMState *s) { S390MachineProps mach; if (!kvm_s390_get_machine_props(s, mach)) { s390_setup_cpu_classes(ACCEL_CURRENT, mach, s390_current_fac_list_mask()); s390_setup_cpu_aliases(); cpu_classes_initialized = true; } } cpu_desc_avail() becomes true. In case the selceted mode was ? the list_cpu() is now done right before the cpu model is used as part of the cpu initialization (hw/s390-virtio.c): void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys) { int i; if (cpu_model == NULL) { cpu_model = none; } if (is_help_option(cpu_model)) { list_cpus(stdout, fprintf, cpu_model); exit(0); } ... for (i = 0; i smp_cpus; i++) { ... cpu = cpu_s390x_init(cpu_model); ... } } In other words, you just need to ensure that s390_cpu_list() run after kvm_setup_cpu_classes(). ... which is part of the KVM/accel init process but it could of course make use of the ACCEL_TMP use case as query-cpu-definitions does. Can't you simply call s390_setup_cpu_classes(ACCEL_TEMP) inside s390_init_cpus(), just like arch_query_cpu_definitions()? You could even share code between both functions. That would not help with the current placement of list_cpus() in main() as it happens way to early. Not s390_init_cpus() is the issue, the context information has been processed already at that time. Currently I just kind of delay the list_cpus() until all required information is available. (In the future, we should be able to implement -cpu ? by simply calling the query-cpu-definitions implementation.) Right but the -machine name,accel=accel options have to be processed already. What exactly could cause cpu_desc_avail() to be false? If CPU model information is not yet available when cpu_list() is called, it is a bug. Here an example output that shows only runnable cpu models: $ ./s390x-softmmu/qemu-system-s390x -machine s390,accel=kvm -cpu ? s390 none s390 2064-ga1 IBM zSeries 900 GA1 s390 2064-ga2 IBM zSeries 900 GA2 s390 2064-ga3 IBM zSeries 900 GA3 s390 2064 (alias for 2064-ga3) s390 z900 (alias for 2064-ga3) s390 2066-ga1 IBM zSeries 800 GA1 s390 2066 (alias for 2066-ga1) s390 z800 (alias for 2066-ga1) s390 2084-ga1 IBM zSeries 990 GA1 s390 2084-ga2 IBM zSeries 990 GA2 s390 2084-ga3 IBM zSeries 990 GA3 s390 2084-ga4 IBM zSeries 990 GA4 s390 2084-ga5 IBM zSeries 990 GA5 s390 2084 (alias for 2084-ga5) s390 z990 (alias for 2084-ga5) s390 2086-ga1 IBM zSeries 890 GA1 s390 2086-ga2 IBM zSeries 890 GA2 s390
Re: [Qemu-devel] [Qemu-block] [RFC] Differential Backups
On Tue, May 05, 2015 at 11:55:49AM -0400, John Snow wrote: On 05/05/2015 06:25 AM, Stefan Hajnoczi wrote: On Wed, Apr 29, 2015 at 06:51:08PM -0400, John Snow wrote: This is a feature that should be very easy to add on top of the existing incremental feature, since it's just a difference in how the bitmap is treated: Incremental - Links to the last incremental (managed by libvirt) - Clears the bitmap after creation Differential: - Links to the last full backup always (managed by libvirt) - Does not clear the bitmap after creation No biggie. Differential backups can be done using incremental backup functionality in QEMU: The client application points QEMU to the same target repeatedly instead of keeping separate incremental backups. Stefan Oh, so you're saying: [anchor]--[diff1] And then when making a new incremental, we re-use diff1 as a target and overwrite it so that it becomes: [anchor]--[diff2] In effect giving us a differential. OK, so it's possible, but we still lose out on some flexibility that a slightly different mode would provide us, like the ability to keep multiple differentials if desired. (Well, I suppose we *can* create those by manually copying differentials after we create them, but that seems hackier than necessary.) Still, it would be such a paltry few lines of code and introduce no real complexity to the subsystem, and it might make libvirt's time a little easier for managing such things. I have CCed Eric Blake and the libvirt mailing list. This is a good time to discuss libvirt requirements for backup APIs. Current work for QEMU 2.4: * QMP command to take incremental backup of guest disks in a single atomic operation * Dirty bitmap persistence across live migration and QEMU restart Eric: Do you have any opinion on a differential backup feature in addition to incremental backup. My thoughts are that QEMU should only copy out changed blocks and let the backup application worry about concepts like incremental vs differential. From a host performance perspective, copying out the same blocks that have already been sent in a previous backup is a waste of I/O bandwidth. Even backup applications that want differential backup may not actually use the QEMU feature for this reason. Regarding the size of the patch, there's a cost to adding the tests, documentation, and commiting to QMP APIs. These costs dwarf the small code change that preserves the dirty bitmap. If there's a concrete requirement for this feature then I'm happy with it, but let's not add bells and whistles just because we can. Stefan pgpY7iRnBTj5L.pgp Description: PGP signature
[Qemu-devel] [Bug 1452062] Re: qemu-img will fail to convert images in 2.3.0
The only possibly relevant change I can see in 2.3 is that the qcow2 driver added an additional error check to a truncate operation. Can you please run qemu-img under strace -f and either provide the output as an attachment or paste the relevant failing call(s) if you can recognise them? -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1452062 Title: qemu-img will fail to convert images in 2.3.0 Status in QEMU: Incomplete Bug description: Hello guys, There seems to be a bug in qemu-img with 2.3.0 that wasn't there in 2.2.1 qemu convert will always fail converting. See the output below: Started by upstream project Create windows image build number 73 originally caused by: Started by user anonymous Building remotely on builder (builder lab) in workspace /var/lib/jenkins/remote/workspace/Prepare windows Image [Prepare WS2008R2 Image] $ /bin/sh -xe /tmp/hudson4138890034689910617.sh + sudo bash -x /var/images/prepare_windows.sh WS2008R2 + set +x Preparing: windows Virtio CD: virtio-win-0.1.96.iso Sparsifying... qemu-img: error while compressing sector 11131648: Input/output error virt-sparsify: error: external command failed: qemu-img convert -f qcow2 -O 'qcow2' -c -o 'compat=0.10' '/tmp/sparsifyb459a0.qcow2' '/var/images/generated/WS2008R2.qcow2' virt-sparsify: If reporting bugs, run virt-sparsify with debugging enabled (-v) and include the complete output. Build step 'Execute shell' marked build as failure Warning: you have no plugins providing access control for builds, so falling back to legacy behavior of permitting any downstream builds to be triggered Finished: FAILURE Thanks, Dave To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1452062/+subscriptions
Re: [Qemu-devel] [PATCH 1/4] block: Fix dirty bitmap in bdrv_co_discard
On 06/05/2015 11:50, Fam Zheng wrote: # src can_write_zeroes_with_unmap target can_write_zeroes_with_unmap 1 true true 2 true false 3 false true 4 false false 1 should replicate WRITE SAME, in case the unmap granularity of the target is different from that of the source. In that case, a discard on the target might leave some sectors untouched. Writing zeroes would ensure matching data between the source and the target. 2 should _not_ discard: it should write zeroes even at the cost of making the target fully provisioned. Perhaps you can optimize it by looking at bdrv_get_block_status for the target, and checking the answer for BDRV_ZERO. 3 and 4 can use discard on the target. So it looks like only the source setting matters. We need to check the cost of bdrv_co_get_block_status for raw, too. If it's too expensive, that can be a problem. Paolo For case 2 3 it's probably better to mirror the actual reading of source. I'm not sure about 4. Even in case 1, discard could be UNMAP and write zeroes could be WRITE SAME. If the unmap granularity of the target is For unaligned sectors, UNMAP might leave some sectors aside while WRITE SAME will write with zeroes.
Re: [Qemu-devel] [RFC PATCH v3 21/24] spapr: Initialize hotplug memory address space
On Tue, May 05, 2015 at 10:48:50AM +0200, Igor Mammedov wrote: On Fri, 24 Apr 2015 12:17:43 +0530 Bharata B Rao bhar...@linux.vnet.ibm.com wrote: Initialize a hotplug memory region under which all the hotplugged memory is accommodated. Also enable memory hotplug by setting CONFIG_MEM_HOTPLUG. Modelled on i386 memory hotplug. Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com --- default-configs/ppc64-softmmu.mak | 1 + hw/ppc/spapr.c| 38 ++ include/hw/ppc/spapr.h| 12 3 files changed, 51 insertions(+) diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak index 22ef132..16b3011 100644 --- a/default-configs/ppc64-softmmu.mak +++ b/default-configs/ppc64-softmmu.mak @@ -51,3 +51,4 @@ CONFIG_XICS_KVM=$(and $(CONFIG_PSERIES),$(CONFIG_KVM)) # For PReP CONFIG_MC146818RTC=y CONFIG_ISA_TESTDEV=y +CONFIG_MEM_HOTPLUG=y diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 910a50f..9dc4c36 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -125,6 +125,9 @@ struct sPAPRMachineState { /* public */ char *kvm_type; +ram_addr_t hotplug_memory_base; +MemoryRegion hotplug_memory; +bool enforce_aligned_dimm; }; sPAPREnvironment *spapr; @@ -1514,6 +1517,7 @@ static void ppc_spapr_init(MachineState *machine) QemuOpts *opts = qemu_opts_find(qemu_find_opts(smp-opts), NULL); int sockets = opts ? qemu_opt_get_number(opts, sockets, 0) : 0; int cores = (smp_cpus/smp_threads) ? smp_cpus/smp_threads : 1; +sPAPRMachineState *ms = SPAPR_MACHINE(machine); sockets = sockets ? sockets : cores; msi_supported = true; @@ -1613,6 +1617,36 @@ static void ppc_spapr_init(MachineState *machine) memory_region_add_subregion(sysmem, 0, rma_region); } +/* initialize hotplug memory address space */ +if (machine-ram_size machine-maxram_size) { +ram_addr_t hotplug_mem_size = +machine-maxram_size - machine-ram_size; + +if (machine-ram_slots SPAPR_MAX_RAM_SLOTS) { +error_report(unsupported amount of memory slots: %PRIu64, + machine-ram_slots); +exit(EXIT_FAILURE); +} + +ms-hotplug_memory_base = ROUND_UP(machine-ram_size, +SPAPR_HOTPLUG_MEM_ALIGN); + +if (ms-enforce_aligned_dimm) { +hotplug_mem_size += SPAPR_HOTPLUG_MEM_ALIGN * machine-ram_slots; +} + +if ((ms-hotplug_memory_base + hotplug_mem_size) hotplug_mem_size) { +error_report(unsupported amount of maximum memory: RAM_ADDR_FMT, + machine-maxram_size); +exit(EXIT_FAILURE); +} + +memory_region_init(ms-hotplug_memory, OBJECT(ms), + hotplug-memory, hotplug_mem_size); +memory_region_add_subregion(sysmem, ms-hotplug_memory_base, +ms-hotplug_memory); +} + filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, spapr-rtas.bin); spapr-rtas_size = get_image_size(filename); spapr-rtas_blob = g_malloc(spapr-rtas_size); @@ -1844,11 +1878,15 @@ static void spapr_set_kvm_type(Object *obj, const char *value, Error **errp) static void spapr_machine_initfn(Object *obj) { +sPAPRMachineState *ms = SPAPR_MACHINE(obj); + object_property_add_str(obj, kvm-type, spapr_get_kvm_type, spapr_set_kvm_type, NULL); object_property_set_description(obj, kvm-type, Specifies the KVM virtualization mode (HV, PR), NULL); + +ms-enforce_aligned_dimm = true; } static void ppc_cpu_do_nmi_on_cpu(void *arg) diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index ecac6e3..53560e9 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -542,6 +542,18 @@ struct sPAPREventLogEntry { #define SPAPR_MEMORY_BLOCK_SIZE (1 28) /* 256MB */ +/* + * This defines the maximum number of DIMM slots we can have for sPAPR + * guest. This is not defined by sPAPR but we are defining it to 4096 slots + * here. With the worst case addition of SPAPR_MEMORY_BLOCK_SIZE + * (256MB) memory per slot, we should be able to support 1TB of guest + * hotpluggable memory. + */ +#define SPAPR_MAX_RAM_SLOTS (1ULL 12) why not write 4096 instead of (1ULL 12), much easier to read. Sure. BTW: KVM supports upto 509 memory slots including slots consumed by initial memory. I see that PowerPC defaults to 32 slots. So having 4096 slots is really pointless then ? So to ensure more hot-pluggable memory space is available should I be increasing the size of the minimum
Re: [Qemu-devel] [PATCH v3 2/6] spapr_pci: encode missing 64-bit memory address space
Thomas Huth th...@redhat.com writes: On Wed, 06 May 2015 11:14:32 +0530 Nikunj A Dadhania nik...@linux.vnet.ibm.com wrote: Thomas Huth th...@redhat.com writes: [...] BTW, does this also require the new version of SLOF already? Not yet, only after patch 4/6 newer SLOF would be needed. Ok, ... but it will also still work with old SLOF version? If not, I think you should eventually also include a patch in this series to update the slof.bin of QEMU. Alexey, can you pull the SLOF changes to your tree. I have it ready here: The following changes since commit bf641aa923237b5c71b472f536246b3f97d7dffb: scsi: handle report-luns failure (2015-04-29 15:20:31 +0530) are available in the git repository at: https://github.com/nikunjad/SLOF master for you to fetch changes up to ad8d7045375ac47cf69080fcbd24788ed32000f8: version: update to 20150429 (2015-04-29 15:27:04 +0530) Nikunj A Dadhania (5): pci: program correct bridge limit registers during probe pci: Support 64-bit address translation usb: support 64-bit pci bars pci: Use QEMU created PCI device nodes version: update to 20150429 VERSION | 2 +- board-qemu/slof/pci-phb.fs | 44 +++- slof/fs/devices/pci-class_0c.fs | 10 -- slof/fs/pci-properties.fs | 6 +- slof/fs/pci-scan.fs | 6 +++--- slof/fs/translate.fs| 6 ++ 6 files changed, 62 insertions(+), 12 deletions(-)
Re: [Qemu-devel] [PATCH v1 RFC 34/34] char: introduce support for TLS encrypted TCP chardev backend
On Tue, May 05, 2015 at 04:54:44PM +0200, Kashyap Chamarthy wrote: [. . .] While running QEMU as TLS server, the TLS handshake completes successfully when connected via `gnutls-cli`. However, when using QEMU as client to connect to an existing GnuTLS server, I notice a segmentation fault: $ /home/kashyapc/build/tls-qemu/x86_64-softmmu/qemu-system-x86_64 \ -nodefconfig -nodefaults -device sga -display none \ -chardev socket,id=s0,host=localhost,port=9000,tls-cred=tls0 \ -device isa-serial,chardev=s0 \ -object qcrypto-tls-creds,id=tls0,credtype=x509,endpoint=client,dir=/export/security/gnutls Segmentation fault (core dumped) Some debugging with `gdb` below. QEMU was built with: ./configure --target-list=x86_64-softmmu --enable-debug make -j4 Stack traces: $ gdb /home/kashyapc/build/tls-qemu/x86_64-softmmu/qemu-system-x86_64 [. . .] (gdb) run -nodefconfig -nodefaults -device sga -display none -chardev socket,id=s0,host=localhost,port=9000,tls-cred=tls0 -device isa-serial,chardev=s0 -object qcrypto-tls-creds,id=tls0,credtype=x509,endpoint=client,dir=/export/security/gnutls Starting program: /home/kashyapc/build/tls-qemu/x86_64-softmmu/qemu-system-x86_64 -nodefconfig -nodefaults -device sga -display none -chardev socket,id=s0,host=localhost,port=9000,tls-cred=tls0 -device isa-serial,chardev=s0 -object qcrypto-tls-creds,id=tls0,credtype=x509,endpoint=client,dir=/export/security/gnutls [. . .] Program received signal SIGSEGV, Segmentation fault. __strstr_sse2_unaligned () at ../sysdeps/x86_64/multiarch/strstr-sse2-unaligned.S:40 40 movdqu (%rdi), %xmm3 (gdb) thread apply all bt full Thread 2 (Thread 0x7fffe4fcc700 (LWP 5393)): #0 0x76bce8fd in nanosleep () at ../sysdeps/unix/syscall-template.S:81 #1 0x764f1de8 in g_usleep () at /lib64/libglib-2.0.so.0 #2 0x559d32d7 in call_rcu_thread (opaque=0x0) at /home/kashyapc/tinker-space/qemu/util/rcu.c:228 tries = 0 n = 0 node = 0x77fd19a0 #3 0x76bc652a in start_thread (arg=0x7fffe4fcc700) at pthread_create.c:310 __res = optimized out pd = 0x7fffe4fcc700 now = optimized out unwind_buf = {cancel_jmp_buf = {{jmp_buf = {140737035159296, 3180389637749088242, 140737488345857, 4096, 140737035159296, 14073703516, -3180444589616128014, -3180404459381186574}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}} not_first_call = optimized out pagesize_m1 = optimized out sp = optimized out freesize = optimized out #4 0x7fffeea0979d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109 ---Type return to continue, or q return to quit--- Thread 1 (Thread 0x77f89bc0 (LWP 5389)): #0 0x7fffee9ae6dd in __strstr_sse2_unaligned () at ../sysdeps/x86_64/multiarch/strstr-sse2-unaligned.S:40 #1 0x71c6b370 in _gnutls_url_is_known () at /lib64/libgnutls.so.28 #2 0x71c6b3d9 in gnutls_certificate_set_x509_key_file2 () at /lib64/libgnutls.so.28 #3 0x559aba85 in qcrypto_tls_creds_load_x509 (creds=0x5639ac60, errp=0x7fffd8d8) at /home/kashyapc/tinker-space/qemu/crypto/tlscreds.c:728 cacert = 0x5639a8c0 /export/security/gnutls/ca-cert.pem cacrl = 0x0 cert = 0x0 key = 0x0 dhparams = 0x0 ret = 1 rv = -1 #4 0x559abdb2 in qcrypto_tls_creds_load (creds=0x5639ac60, errp=0x7fffd8d8) at /home/kashyapc/tinker-space/qemu/crypto/tlscreds.c:820 #5 0x559abf30 in qcrypto_tls_creds_prop_set_loaded (obj=0x5639ac60, value=true, errp=0x7fffd8d8) at /home/kashyapc/tinker-space/qemu/crypto/tlscreds.c:888 creds = 0x5639ac60 __func__ = qcrypto_tls_creds_prop_set_loaded #6 0x558cec1c in property_set_bool (obj=0x5639ac60, v=0x5639b4d0, opaque=0x5639ad40, name=0x55a59695 loaded, errp=0x7fffd8d8) at /home/kashyapc/tinker-space/qemu/qom/object.c:1600 prop = 0x5639ad40 value = true local_err = 0x0 ---Type return to continue, or q return to quit--- #7 0x558cd485 in object_property_set (obj=0x5639ac60, v=0x5639b4d0, name=0x55a59695 loaded, errp=0x7fffd8d8) at /home/kashyapc/tinker-space/qemu/qom/object.c:901 prop = 0x5639ad60 #8 0x558cfa47 in object_property_set_qobject (obj=0x5639ac60, value=0x5639b200, name=0x55a59695 loaded, errp=0x7fffd8d8) at /home/kashyapc/tinker-space/qemu/qom/qom-qobject.c:24 mi = 0x5639b4d0 #9 0x558cd6f4 in object_property_set_bool (obj=0x5639ac60, value=true, name=0x55a59695 loaded, errp=0x7fffd8d8) at /home/kashyapc/tinker-space/qemu/qom/object.c:969 qbool = 0x5639b200 #10 0x559ac2e5 in qcrypto_tls_creds_complete (uc=0x5639ac60,
Re: [Qemu-devel] [RFC PATCH v3 06/24] spapr: Consolidate cpu init code into a routine
On Wed, May 06, 2015 at 08:32:03AM +0200, Thomas Huth wrote: On Wed, 6 May 2015 09:58:09 +0530 Bharata B Rao bhar...@linux.vnet.ibm.com wrote: On Mon, May 04, 2015 at 06:10:59PM +0200, Thomas Huth wrote: On Fri, 24 Apr 2015 12:17:28 +0530 Bharata B Rao bhar...@linux.vnet.ibm.com wrote: Factor out bits of sPAPR specific CPU initialization code into a separate routine so that it can be called from CPU hotplug path too. Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com Reviewed-by: David Gibson da...@gibson.dropbear.id.au --- hw/ppc/spapr.c | 54 +- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index a56f9a1..5c8f2ff 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1440,6 +1440,34 @@ static void spapr_drc_reset(void *opaque) } } +static void spapr_cpu_init(PowerPCCPU *cpu) +{ +CPUPPCState *env = cpu-env; + +/* Set time-base frequency to 512 MHz */ +cpu_ppc_tb_init(env, TIMEBASE_FREQ); + +/* PAPR always has exception vectors in RAM not ROM. To ensure this, + * MSR[IP] should never be set. + */ +env-msr_mask = ~(1 6); While you're at it ... could we maybe get a proper #define for that MSR bit? (just like the other ones in target-ppc/cpu.h) Sure will use MSR_EP here next time. According to the comment in cpu.h, the EP bit was for the 601 CPU only, so I think it would be better to introduce a new #define MSR_IP 6 (or so) here instead. Kernel defines bit 6 as #define MSR_IP_LG 6 /* Exception prefix 0x000/0xFFF */ (arch/powerpc/include/asm/reg.h) QEMU defines it as #define MSR_EP 6 /* Exception prefix on 601 */ I can add MSR_IP in QEMU, but that will mean two defines for same bit position, but I think MSR_IP_LG in kernel or MSR_EP in QEMU both mean the same, but called differently. Regards, Bharata.
Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
On Tue, 5 May 2015 11:46:04 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Tue, May 05, 2015 at 08:36:45AM -0600, Eric Blake wrote: On 05/05/2015 07:26 AM, Eduardo Habkost wrote: +{ 'enum': 'AccelId', + 'data': ['qtest', 'tcg', 'kvm', 'xen'] } + Not sure if it is better to have an enum or simply a string here. enum is almost always better when there is a finite set of possible strings - it's better documented, and when introspection is in place, will make it easier to determine when the set has grown. True, and there are other cases where we could use an enum internally in QEMU (e.g. arrays for accelerator-specific data inside CPU classes). Actually the accelerator name currently represented as string (ac-name = KVM;) can be initialized from the respective lookup value (AccelId_lookup[]) instead.
Re: [Qemu-devel] [RFC PATCH v3 06/24] spapr: Consolidate cpu init code into a routine
On Wed, 6 May 2015 14:15:37 +0530 Bharata B Rao bhar...@linux.vnet.ibm.com wrote: On Wed, May 06, 2015 at 08:32:03AM +0200, Thomas Huth wrote: On Wed, 6 May 2015 09:58:09 +0530 Bharata B Rao bhar...@linux.vnet.ibm.com wrote: On Mon, May 04, 2015 at 06:10:59PM +0200, Thomas Huth wrote: On Fri, 24 Apr 2015 12:17:28 +0530 Bharata B Rao bhar...@linux.vnet.ibm.com wrote: Factor out bits of sPAPR specific CPU initialization code into a separate routine so that it can be called from CPU hotplug path too. Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com Reviewed-by: David Gibson da...@gibson.dropbear.id.au --- hw/ppc/spapr.c | 54 +- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index a56f9a1..5c8f2ff 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1440,6 +1440,34 @@ static void spapr_drc_reset(void *opaque) } } +static void spapr_cpu_init(PowerPCCPU *cpu) +{ +CPUPPCState *env = cpu-env; + +/* Set time-base frequency to 512 MHz */ +cpu_ppc_tb_init(env, TIMEBASE_FREQ); + +/* PAPR always has exception vectors in RAM not ROM. To ensure this, + * MSR[IP] should never be set. + */ +env-msr_mask = ~(1 6); While you're at it ... could we maybe get a proper #define for that MSR bit? (just like the other ones in target-ppc/cpu.h) Sure will use MSR_EP here next time. According to the comment in cpu.h, the EP bit was for the 601 CPU only, so I think it would be better to introduce a new #define MSR_IP 6 (or so) here instead. Kernel defines bit 6 as #define MSR_IP_LG 6 /* Exception prefix 0x000/0xFFF */ (arch/powerpc/include/asm/reg.h) QEMU defines it as #define MSR_EP 6 /* Exception prefix on 601 */ I can add MSR_IP in QEMU, but that will mean two defines for same bit position, but I think MSR_IP_LG in kernel or MSR_EP in QEMU both mean the same, but called differently. Ok, so EP = IP ... then I also think it's fine to use the MSR_EP define here. I first thought that EP and IP would mean something different, since a lot of MSR bits are defined differently on the various POWER/PowerPC chips (see the MSR bit 10 for example, it has three defines in cpu.h), but in this case they really seem to be the same. By the way, is this bit still used at all on recent chips (which are used for the spapr machine)? It's apparently not defined in the PowerISA spec anymore... Thomas
Re: [Qemu-devel] qemu-img convert (vmdk)
Am 05.05.2015 um 19:01 hat Antoni Villalonga geschrieben: Hi, Is my first email to that list ;) I can reproduce this bug with v2.2 and v2.3. I'm not sure about the results after testing with v2.1 (doesn't show errors but seems to be still broken). % qemu-img convert -f raw -O vmdk -o subformat=streamOptimized 100GB_inputfile.img outputfile0.vmdk (no shown errors/warnings/info) % qemu-img convert -f vmdk -O qcow2 outputfile0.vmdk outputfile11.qcow2 qemu-img: error while reading sector 278528: Invalid argument Let me know if I can provide you more info. Fam, any idea? Kevin
Re: [Qemu-devel] qemu-img convert (vmdk)
Hi again, it seems to be related to the subformat option. With the default option seems to work fine. I think StreamOptimized is mantatory for 'ova' files. Thanks On Tue, May 05, 2015 at 05:01:34PM +, Antoni Villalonga wrote: Hi, Is my first email to that list ;) I can reproduce this bug with v2.2 and v2.3. I'm not sure about the results after testing with v2.1 (doesn't show errors but seems to be still broken). % qemu-img convert -f raw -O vmdk -o subformat=streamOptimized 100GB_inputfile.img outputfile0.vmdk (no shown errors/warnings/info) % qemu-img convert -f vmdk -O qcow2 outputfile0.vmdk outputfile11.qcow2 qemu-img: error while reading sector 278528: Invalid argument Let me know if I can provide you more info. Regards -- Antoni Villalonga http://friki.cat/ -- Antoni Villalonga http://friki.cat/
[Qemu-devel] [PATCH] ich9: implement SMI_LOCK
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/acpi/ich9.c | 4 +++- hw/isa/lpc_ich9.c | 19 +++ include/hw/acpi/ich9.h | 1 + include/hw/i386/ich9.h | 6 ++ 4 files changed, 29 insertions(+), 1 deletion(-) diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c index eefd8b0..86deb2b 100644 --- a/hw/acpi/ich9.c +++ b/hw/acpi/ich9.c @@ -94,7 +94,8 @@ static void ich9_smi_writel(void *opaque, hwaddr addr, uint64_t val, ICH9LPCPMRegs *pm = opaque; switch (addr) { case 0: -pm-smi_en = val; +pm-smi_en = ~pm-smi_en_wmask; +pm-smi_en |= (val pm-smi_en_wmask); break; } } @@ -198,6 +199,7 @@ static void pm_reset(void *opaque) * support SMM mode. */ pm-smi_en |= ICH9_PMIO_SMI_EN_APMC_EN; } +pm-smi_en_wmask = ~0; acpi_update_sci(pm-acpi_regs, pm-irq); } diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c index 3e30273..ac0e63e 100644 --- a/hw/isa/lpc_ich9.c +++ b/hw/isa/lpc_ich9.c @@ -410,12 +410,28 @@ static void ich9_lpc_rcba_update(ICH9LPCState *lpc, uint32_t rbca_old) } } +/* config:GEN_PMCON* */ +static void +ich9_lpc_pmcon_update(ICH9LPCState *lpc) +{ +uint16_t gen_pmcon_1 = pci_get_word(lpc-d.config + ICH9_LPC_GEN_PMCON_1); +uint16_t wmask; + +if (gen_pmcon_1 ICH9_LPC_GEN_PMCON_1_SMI_LOCK) { +wmask = pci_get_word(lpc-d.wmask + ICH9_LPC_GEN_PMCON_1); +wmask = ~ICH9_LPC_GEN_PMCON_1_SMI_LOCK; +pci_set_word(lpc-d.wmask + ICH9_LPC_GEN_PMCON_1, wmask); +lpc-pm.smi_en_wmask = ~1; +} +} + static int ich9_lpc_post_load(void *opaque, int version_id) { ICH9LPCState *lpc = opaque; ich9_lpc_pmbase_update(lpc); ich9_lpc_rcba_update(lpc, 0 /* disabled ICH9_LPC_RBCA_EN */); +ich9_lpc_pmcon_update(lpc); return 0; } @@ -438,6 +454,9 @@ static void ich9_lpc_config_write(PCIDevice *d, if (ranges_overlap(addr, len, ICH9_LPC_PIRQE_ROUT, 4)) { pci_bus_fire_intx_routing_notifier(lpc-d.bus); } +if (ranges_overlap(addr, len, ICH9_LPC_GEN_PMCON_1, 8)) { +ich9_lpc_pmcon_update(lpc); +} } static void ich9_lpc_reset(DeviceState *qdev) diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h index f03f7c2..ac24bbe 100644 --- a/include/hw/acpi/ich9.h +++ b/include/hw/acpi/ich9.h @@ -39,6 +39,7 @@ typedef struct ICH9LPCPMRegs { MemoryRegion io_smi; uint32_t smi_en; +uint32_t smi_en_wmask; uint32_t smi_sts; qemu_irq irq; /* SCI */ diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h index 874c131..0c973bb 100644 --- a/include/hw/i386/ich9.h +++ b/include/hw/i386/ich9.h @@ -154,6 +154,12 @@ Object *ich9_lpc_find(void); #define ICH9_LPC_PIRQ_ROUT_MASK Q35_MASK(8, 3, 0) #define ICH9_LPC_PIRQ_ROUT_DEFAULT 0x80 +#define ICH9_LPC_GEN_PMCON_10xa0 +#define ICH9_LPC_GEN_PMCON_1_SMI_LOCK (1 4) +#define ICH9_LPC_GEN_PMCON_20xa2 +#define ICH9_LPC_GEN_PMCON_30xa4 +#define ICH9_LPC_GEN_PMCON_LOCK 0xa6 + #define ICH9_LPC_RCBA 0xf0 #define ICH9_LPC_RCBA_BA_MASK Q35_MASK(32, 31, 14) #define ICH9_LPC_RCBA_EN0x1 -- 1.8.3.1
Re: [Qemu-devel] [PATCH v6 03/17] Extend QMP command query-cpus to return accelerator id and model name
On Tue, 5 May 2015 10:11:15 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Mon, Apr 27, 2015 at 04:53:17PM +0200, Michael Mueller wrote: The QMP command query-cpus now additionally displays a model name and the backing accelerator. Both are omitted if the model name is not initialized. request: { execute : query-cpus } answer: { { current: true, CPU: 0, model: 2827-ga2, halted: false, accel: kvm, thread_id: 31917 }, ... } Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com Acked-by: Christian Borntraeger borntrae...@de.ibm.com With the new qom-path field I submitted yesterday, this can be provided as QOM properties through qom-get. Is that really a good idea to make the object representation part of the ABI. I guess there is a related discussion already somewhere. I mean not just adding the qom-path field, I saw that suggested patch, I mean the approach to expose the objects themselves... I will try your patch of course as well... Michael
Re: [Qemu-devel] [PATCH v1 RFC 34/34] char: introduce support for TLS encrypted TCP chardev backend
On Wed, May 06, 2015 at 10:34:06AM +0200, Kashyap Chamarthy wrote: On Tue, May 05, 2015 at 04:54:44PM +0200, Kashyap Chamarthy wrote: [. . .] While running QEMU as TLS server, the TLS handshake completes successfully when connected via `gnutls-cli`. However, when using QEMU as client to connect to an existing GnuTLS server, I notice a segmentation fault: $ /home/kashyapc/build/tls-qemu/x86_64-softmmu/qemu-system-x86_64 \ -nodefconfig -nodefaults -device sga -display none \ -chardev socket,id=s0,host=localhost,port=9000,tls-cred=tls0 \ -device isa-serial,chardev=s0 \ -object qcrypto-tls-creds,id=tls0,credtype=x509,endpoint=client,dir=/export/security/gnutls Segmentation fault (core dumped) Some debugging with `gdb` below. QEMU was built with: ./configure --target-list=x86_64-softmmu --enable-debug make -j4 Stack traces: $ gdb /home/kashyapc/build/tls-qemu/x86_64-softmmu/qemu-system-x86_64 #2 0x71c6b3d9 in gnutls_certificate_set_x509_key_file2 () at /lib64/libgnutls.so.28 #3 0x559aba85 in qcrypto_tls_creds_load_x509 (creds=0x5639ac60, errp=0x7fffd8d8) at /home/kashyapc/tinker-space/qemu/crypto/tlscreds.c:728 cacert = 0x5639a8c0 /export/security/gnutls/ca-cert.pem cacrl = 0x0 cert = 0x0 key = 0x0 dhparams = 0x0 ret = 1 rv = -1 Ah, with QEMU running in client mode, the client cert + key are optional. In this case you've not provided them (cert key are 0x0 ie NULL). We are then mistakenly calling gnutls_certificate_set_x509_key_file2 - if I simply skip that I'll avoid the crash. Thanks for testing this - I'll add a test case to validate this scenario too Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [RFC PATCH v3 24/24] spapr: Memory hotplug support
On Tue, May 05, 2015 at 05:45:01PM +1000, David Gibson wrote: On Fri, Apr 24, 2015 at 12:17:46PM +0530, Bharata B Rao wrote: Make use of pc-dimm infrastructure to support memory hotplug for PowerPC. Modelled on i386 memory hotplug. Can the previous patch actually do anything without this one? Yes, the previous patch adds ibm,dynamic-reconfiguration-memory node and is contained in itself. Regards, Bharata.
Re: [Qemu-devel] [PATCH v2] libcacard: stop including qemu-common.h
06.05.2015 12:23, Laurent Desnogues wrote: Hello, On Mon, Apr 27, 2015 at 3:27 PM, Michael Tokarev m...@tls.msk.ru wrote: From: Paolo Bonzini pbonz...@redhat.com This is a small step towards making libcacard standalone. on my system the removal of qemu-common.h inclusion broke compilation due to assert being used in glib-compat.h. Interesting. What kind of build environment is that? I compile-tested on several platforms, all went fine.. ;) A fix might be to include assert.h in glib-compat.h. I prefer s/assert/g_assert/ in glib-compat.h. Thanks, /mjt
Re: [Qemu-devel] [PATCH v2] libcacard: stop including qemu-common.h
Hello, On Mon, Apr 27, 2015 at 3:27 PM, Michael Tokarev m...@tls.msk.ru wrote: From: Paolo Bonzini pbonz...@redhat.com This is a small step towards making libcacard standalone. on my system the removal of qemu-common.h inclusion broke compilation due to assert being used in glib-compat.h. A fix might be to include assert.h in glib-compat.h. Thanks, Laurent Signed-off-by: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Michael Tokarev m...@tls.msk.ru --- v2: update vscclient.c too, use unistd.h on !WIN32 and getopt.h on *nix libcacard/cac.c| 5 - libcacard/card_7816.c | 4 +++- libcacard/event.c | 2 +- libcacard/vcard.c | 4 +++- libcacard/vcard_emul_nss.c | 2 +- libcacard/vreader.c| 4 +++- libcacard/vscclient.c | 8 +++- 7 files changed, 22 insertions(+), 7 deletions(-) diff --git a/libcacard/cac.c b/libcacard/cac.c index f38fdce..bc84534 100644 --- a/libcacard/cac.c +++ b/libcacard/cac.c @@ -5,7 +5,10 @@ * See the COPYING.LIB file in the top-level directory. */ -#include qemu-common.h +#include glib-compat.h + +#include string.h +#include stdbool.h #include cac.h #include vcard.h diff --git a/libcacard/card_7816.c b/libcacard/card_7816.c index 814fa16..22fd334 100644 --- a/libcacard/card_7816.c +++ b/libcacard/card_7816.c @@ -5,7 +5,9 @@ * See the COPYING.LIB file in the top-level directory. */ -#include qemu-common.h +#include glib-compat.h + +#include string.h #include vcard.h #include vcard_emul.h diff --git a/libcacard/event.c b/libcacard/event.c index 4c551e4..63f4057 100644 --- a/libcacard/event.c +++ b/libcacard/event.c @@ -5,7 +5,7 @@ * See the COPYING.LIB file in the top-level directory. */ -#include qemu-common.h +#include glib-compat.h #include vcard.h #include vreader.h diff --git a/libcacard/vcard.c b/libcacard/vcard.c index d140a8e..1a87208 100644 --- a/libcacard/vcard.c +++ b/libcacard/vcard.c @@ -5,7 +5,9 @@ * See the COPYING.LIB file in the top-level directory. */ -#include qemu-common.h +#include glib-compat.h + +#include string.h #include vcard.h #include vcard_emul.h diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c index 950edee..6955f69 100644 --- a/libcacard/vcard_emul_nss.c +++ b/libcacard/vcard_emul_nss.c @@ -25,7 +25,7 @@ #include prthread.h #include secerr.h -#include qemu-common.h +#include glib-compat.h #include vcard.h #include card_7816t.h diff --git a/libcacard/vreader.c b/libcacard/vreader.c index 0315dd8..9725f46 100644 --- a/libcacard/vreader.c +++ b/libcacard/vreader.c @@ -10,7 +10,9 @@ #endif #define G_LOG_DOMAIN libcacard -#include qemu-common.h +#include glib-compat.h + +#include string.h #include vcard.h #include vcard_emul.h diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c index fa6041d..0652684 100644 --- a/libcacard/vscclient.c +++ b/libcacard/vscclient.c @@ -10,14 +10,20 @@ * See the COPYING.LIB file in the top-level directory. */ +#include stdio.h +#include stdlib.h +#include string.h #ifndef _WIN32 #include sys/socket.h #include netinet/in.h #include netdb.h +#include unistd.h #define closesocket(x) close(x) +#else +#include getopt.h #endif -#include qemu-common.h +#include glib-compat.h #include vscard_common.h -- 2.1.4
Re: [Qemu-devel] [PATCH 1/4] block: Fix dirty bitmap in bdrv_co_discard
On Wed, 05/06 10:59, Paolo Bonzini wrote: On 06/05/2015 03:45, Fam Zheng wrote: This is not enough, you also have to do the discard in block/mirror.c, otherwise the destination image could even become fully provisioned! I wasn't sure what if src and dest have different can_write_zeroes_with_unmap value, but your argument is stronger. I will add discard in mirror. Besides that, do we need to compare the can_write_zeroes_with_unmap? Hmm, if can_write_zeroes_with_unmap is set, it's probably better to write zeroes instead of discarding, in case the guest is relying on it. I think there are four cases: # src can_write_zeroes_with_unmap target can_write_zeroes_with_unmap 1 true true 2 true false 3 false true 4 false false I think replicating discard only works for 1, because both side would then read 0. For case 2 3 it's probably better to mirror the actual reading of source. I'm not sure about 4. What do you think? Fam
[Qemu-devel] [vhost] vhost_get_vq_desc, vq-iov mapping
From what I know, qemu and vhost backends use some functions in order to access the memory between the guest and the backend. From what I see, vhost does this with vhost_get_vq_desc(). How much do vq-iov mappings remain valid? Does it get unmapped at some time (you could answer me within vhost-net or vhost-scsi as examples)?
Re: [Qemu-devel] [PULL 00/40] drop qapi nested structs
On 5 May 2015 at 17:46, Markus Armbruster arm...@redhat.com wrote: We want to eventually allow qapi defaults, by making: 'data':{'*flag':'bool'} as shorthand for something like: 'data':{'flag':{'type':'bool', 'optional':true}} so that the default can be specified: 'data':{'flag':{'type':'bool', 'optional':true, 'default':true}} This series does not quite get us there, but it DOES do a number of other things. It gets rid of the three uses of nested inline structs, changes anonymous unions to use a specific 'alternate' metatype rather than abusing 'union', changes the ambiguous 'type' to the obvious 'struct', and fixes lots of other parser bugs found while designing the testsuite. The testsuite changes make the bulk of this series, with a repeating pattern of writing tests that expose weaknesses in the old parser, then beefing up the generator to catch the problem during the initial parse rather than choking with an obscure python message or even causing a C compilation failure. The following changes since commit 874e9aeeeb74c5459639a93439a502d262847e68: Merge remote-tracking branch 'remotes/kraxel/tags/pull-sdl-20150505-1' into staging (2015-05-05 14:06:12 +0100) are available in the git repository at: git://repo.or.cz/qemu/armbru.git tags/pull-qmp-2015-05-05 for you to fetch changes up to ff55d72eaf9628e7d58e7b067b361cdbf789c9f4: qapi: Check for member name conflicts with a base class (2015-05-05 18:39:02 +0200) drop qapi nested structs Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH COLO v3 01/14] docs: block replication's description
-Original Message- From: Dr. David Alan Gilbert [mailto:dgilb...@redhat.com] Sent: Tuesday, May 05, 2015 11:24 PM To: Stefan Hajnoczi Cc: Paolo Bonzini; Wen Congyang; Fam Zheng; Kevin Wolf; Lai Jiangshan; qemu block; Jiang, Yunhong; Dong, Eddie; qemu devel; Max Reitz; Gonglei; Yang Hongyang; zhanghailiang; arm...@redhat.com; jc...@redhat.com Subject: Re: [PATCH COLO v3 01/14] docs: block replication's description * Stefan Hajnoczi (stefa...@redhat.com) wrote: On Fri, Apr 24, 2015 at 11:36:35AM +0200, Paolo Bonzini wrote: On 24/04/2015 11:38, Wen Congyang wrote: That can be done with drive-mirror. But I think it's too early for that. Do you mean use drive-mirror instead of quorum? Only before starting up a new secondary. Basically you do a migration with non-shared storage, and then start the secondary in colo mode. But it's only for the failover case. Quorum (or a new block/colo.c driver or filter) is fine for normal colo operation. Perhaps this patch series should mirror the Secondary's disk to a Backup Secondary so that the system can be protected very quickly after failover. I think anyone serious about fault tolerance would deploy a Backup Secondary, otherwise the system cannot survive two failures unless a human administrator is lucky/fast enough to set up a new Secondary. I'd assumed that a higher level management layer would do the allocation of a new secondary after the first failover, so no human need be involved. I agree. The cloud OS, such as open stack, will have the capability to handle the case, together with certain API in VMM side for this (libvirt?). Thx Eddie
Re: [Qemu-devel] [RFC PATCH v3 08/24] ppc: Prepare CPU socket/core abstraction
On Wed, 6 May 2015 10:10:15 +0530 Bharata B Rao bhar...@linux.vnet.ibm.com wrote: On Mon, May 04, 2015 at 05:15:04PM +0200, Thomas Huth wrote: On Fri, 24 Apr 2015 12:17:30 +0530 Bharata B Rao bhar...@linux.vnet.ibm.com wrote: Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com Signed-off-by: Andreas Färber afaer...@suse.de Not sure if QEMU is fully following the kernel Sob-process, but if that's the case, I think your Sob should be below the one of Andreas in case you're the last person who touched the patch (and I think that's the case here since you've sent it out)? The patch is from me, but I included Andreas' sob and copyrights since the code was derived from his equivalent x86 implementation. I am neither a lawyer nor an expert here, but I think in case the patch itself never really passed through the hands of Andreas, it should not include an Sob of him. An Sob is like a signature - you can/should not put an Sob from somebody else into a file, everybody has to do that personally. So in case you copied code from another file, simply state that clearly in the patch description and make sure that the original author is on CC:. You can also explicitly put a Cc: Original Author e@mail line after your Sob in the patch description to make sure that the Cc: information is retained in the patch description. Thomas
Re: [Qemu-devel] [PATCH] parallel: Allow to disable CONFIG_PARALLEL
On Tue, May 05, 2015 at 12:32:37PM +0200, Thomas Huth wrote: On Tue, 5 May 2015 12:18:05 +0200 Miroslav Rezanina mreza...@redhat.com wrote: On Tue, May 05, 2015 at 11:59:50AM +0200, Thomas Huth wrote: On Tue, 5 May 2015 11:30:49 +0200 mreza...@redhat.com wrote: From: Miroslav Rezanina mreza...@redhat.com Disabling CONFIG_PARALLEL cause build failure as commit 07dc788 factored out initialization to parallel_hds_isa_init that is not build. Make calling parallel_hds_isa_init depending on CONFIG_PARALLEL so it can be correctly disabled. Signed-off-by: Miroslav Rezanina mreza...@redhat.com --- hw/i386/pc.c| 2 ++ hw/mips/mips_fulong2e.c | 2 ++ hw/mips/mips_malta.c| 2 ++ hw/sparc64/sun4u.c | 2 ++ 4 files changed, 8 insertions(+) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index a8e6be1..560464e 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1465,7 +1465,9 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, } serial_hds_isa_init(isa_bus, MAX_SERIAL_PORTS); +#ifdef CONFIG_PARALLEL parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS); +#endif Not sure, but is this pre-processor macro really defined if CONFIG_PARALLEL has been set in the makefile? I've hit some similar problem in the past and I had to discover that only the CONFIG_* options from config-host.mak get translated into #defines, all the others don't get translated. I might be wrong, but just to be sure, could you please double-check that CONFIG_PARALLEL is #defined if it's enabled in the .mak file? Yeah, you're right..I switch the behavior so thought of CONFIG_* from config-host.mak to be the one to not translated. And ifdef masked the issue. This patch is not working and has to be redone. If not: Where does the build break exactly? Does it fail for all three types, i386, mips and sun? I can't test mips/sun build but for i386 it fails on linking where parallel_hds_isa_init does not exists. That's strange, there is a CONFIG_PARALLEL=y in both, default-configs/x86_64-softmmu.mak and i386-softmmu.mak so I wonder why the parallel.o object file is not linked in your case? Thomas It is enabled by default but build fails in case you disable the option for these architectures. Mirek
Re: [Qemu-devel] [PATCH v6 04/17] Extend HMP command info cpus to display accelerator id and model name
On Tue, 5 May 2015 10:14:32 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Mon, Apr 27, 2015 at 04:53:18PM +0200, Michael Mueller wrote: The HMP command info cpus now displays the CPU model name and the backing accelerator if part of the CPUState. (qemu) info cpus * CPU #0: (halted) model=2827-ga2 accel=kvm thread_id=1679 Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com Acked-by: Christian Borntraeger borntrae...@de.ibm.com Do we really need this? I mean: I expect the amount of CPU data we provide to QMP clients to grow a lot in the near future, but that doesn't mean HMP users need all that data to be printed by info cpus. Where do you see the limit of what is worth to be shown an what not. I personally use info cpus less then sporadic but already got a comment internally on that information being worthwhile to be shown. --- hmp.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/hmp.c b/hmp.c index f142d36..676d821 100644 --- a/hmp.c +++ b/hmp.c @@ -290,6 +290,13 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict) monitor_printf(mon, (halted)); } +if (cpu-value-has_model) { +monitor_printf(mon, model=%s, cpu-value-model); +} +if (cpu-value-has_accel) { +monitor_printf(mon, accel=%s, AccelId_lookup[cpu-value-accel]); +} + monitor_printf(mon, thread_id=% PRId64 \n, cpu-value-thread_id); } -- 1.8.3.1
[Qemu-devel] [PATCH 4/4] Do not fail if eventfds are not supported
Signed-off-by: Pavel Fedin p.fe...@samsung.com --- hw/virtio/virtio-mmio.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c index 97a1fb0..a86c816 100644 --- a/hw/virtio/virtio-mmio.c +++ b/hw/virtio/virtio-mmio.c @@ -22,6 +22,7 @@ #include hw/sysbus.h #include hw/virtio/virtio.h #include qemu/host-utils.h +#include sysemu/kvm.h #include hw/virtio/virtio-bus.h #include qemu/error-report.h @@ -124,7 +125,8 @@ static void virtio_mmio_start_ioeventfd(VirtIOMMIOProxy *proxy) VirtIODevice *vdev = virtio_bus_get_device(proxy-bus); int n, r; -if (proxy-ioeventfd_disabled || +if (!kvm_eventfds_enabled() || +proxy-ioeventfd_disabled || proxy-ioeventfd_started) { return; } -- 1.9.5.msysgit.0
Re: [Qemu-devel] [RFC PATCH v3 20/24] spapr: CPU hot unplug support
On Tue, May 05, 2015 at 05:28:38PM +1000, David Gibson wrote: On Fri, Apr 24, 2015 at 12:17:42PM +0530, Bharata B Rao wrote: Support hot removal of CPU for sPAPR guests by sending the hot unplug notification to the guest via EPOW interrupt. Release the vCPU object after CPU hot unplug so that vCPU fd can be parked and reused. Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com --- hw/ppc/spapr.c | 101 +++- target-ppc/translate_init.c | 10 + 2 files changed, 110 insertions(+), 1 deletion(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 9b0701c..910a50f 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1481,6 +1481,12 @@ static void spapr_cpu_init(PowerPCCPU *cpu) qemu_register_reset(spapr_cpu_reset, cpu); } +static void spapr_cpu_destroy(PowerPCCPU *cpu) +{ +xics_cpu_destroy(spapr-icp, cpu); +qemu_unregister_reset(spapr_cpu_reset, cpu); +} + /* pSeries LPAR / sPAPR hardware init */ static void ppc_spapr_init(MachineState *machine) { @@ -1883,6 +1889,24 @@ static void *spapr_populate_hotplug_cpu_dt(DeviceState *dev, CPUState *cs, return fdt; } +static void spapr_cpu_release(DeviceState *dev, void *opaque) +{ +CPUState *cs; +int i; +int id = ppc_get_vcpu_dt_id(POWERPC_CPU(CPU(dev))); + +for (i = id; i id + smp_threads; i++) { +CPU_FOREACH(cs) { +PowerPCCPU *cpu = POWERPC_CPU(cs); + +if (i == ppc_get_vcpu_dt_id(cpu)) { +spapr_cpu_destroy(cpu); +cpu_remove(cs); +} +} +} +} + static void spapr_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { @@ -1970,6 +1994,59 @@ static void spapr_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev, return; } +static int spapr_cpu_unplug(Object *obj, void *opaque) +{ +Error **errp = opaque; +DeviceState *dev = DEVICE(obj); +CPUState *cs = CPU(dev); +PowerPCCPU *cpu = POWERPC_CPU(cs); +int id = ppc_get_vcpu_dt_id(cpu); +int smt = kvmppc_smt_threads(); +sPAPRDRConnector *drc = +spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, id); +sPAPRDRConnectorClass *drck; +Error *local_err = NULL; + +/* + * SMT threads return from here, only main thread (core) will + * continue and signal hot unplug event to the guest. + */ +if ((id % smt) != 0) { +return 0; +} As with the in-plug side couldn't this be done more naturally by attaching this function to the cpu core object rather than the thread? When I switch to core level addition in the next version, I will see how best this can be handled. +g_assert(drc); + +drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); +drck-detach(drc, dev, spapr_cpu_release, NULL, local_err); +if (local_err) { +error_propagate(errp, local_err); +return -1; +} + +/* + * In addition to hotplugged CPUs, send the hot-unplug notification + * interrupt to the guest for coldplugged CPUs started via -device + * option too. + */ +spapr_hotplug_req_remove_event(drc); Um.. doesn't the remove notification need to go *before* the physical unplug? Along with a wait for acknowledgement from the guest? If I am reading it right, PCI hotplug is also following the same order (detach followed by guest notification). Let me experiment with this in the next version. Regards, Bharata.
Re: [Qemu-devel] [PATCH v3 6/6] spapr_pci: populate ibm,loc-code
Thomas Huth th...@redhat.com writes: On Tue, 5 May 2015 14:23:56 +0530 Nikunj A Dadhania nik...@linux.vnet.ibm.com wrote: Each hardware instance has a platform unique location code. The OF device tree that describes a part of a hardware entity must include the “ibm,loc-code” property with a value that represents the location code for that hardware entity. Populate ibm,loc-code. 1) PCI passthru devices need to identify with its own ibm,loc-code available on the host. 2) Emulated devices encode as following: qemu_name:phb-index:slot.fn Signed-off-by: Nikunj A Dadhania nik...@linux.vnet.ibm.com --- hw/ppc/spapr_pci.c | 86 +++--- 1 file changed, 75 insertions(+), 11 deletions(-) diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index cbd5661..eacf0bd 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -744,6 +744,70 @@ static AddressSpace *spapr_pci_dma_iommu(PCIBus *bus, void *opaque, int devfn) return phb-iommu_as; } +static bool spapr_phb_vfio_get_devspec_value(PCIDevice *pdev, char **value) +{ +char *host; +char path[PATH_MAX]; + +host = object_property_get_str(OBJECT(pdev), host, NULL); +if (!host) { +return false; +} + +snprintf(path, sizeof(path), /sys/bus/pci/devices/%s/devspec, host); +g_free(host); + +return g_file_get_contents(path, value, NULL, NULL); +} + +static char *spapr_phb_vfio_get_loc_code(sPAPRPHBState *sphb, PCIDevice *pdev) +{ +char path[PATH_MAX], *buf = NULL; + +/* We have a vfio host bridge lets get the path. */ +if (!spapr_phb_vfio_get_devspec_value(pdev, buf)) { +return NULL; +} + +snprintf(path, sizeof(path), /proc/device-tree%s/ibm,loc-code, buf); +g_free(buf); + +if (g_file_get_contents(path, buf, NULL, NULL)) { +return buf; +} else { +return NULL; +} Minor idea for an optimization: g_file_get_contents() should set buf to NULL in case of errors anyway, so you could omit the if and return NULLL and simply always return buf here. Sure +} + +static char *spapr_phb_get_loc_code(sPAPRPHBState *sphb, PCIDevice *pdev) +{ +char *path = g_malloc(PATH_MAX); + +if (!path) { +return NULL; +} + +/* + * For non-vfio devices and failures make up the location code out This comment (and failures) ... Oh right, one option is to drop that... + * of the name, slot and function. + * + * qemu_name:phb-index:slot.fn + */ +snprintf(path, PATH_MAX, qemu_%s:%02d:%02d.%1d, pdev-name, + sphb-index, PCI_SLOT(pdev-devfn), PCI_FUNC(pdev-devfn)); +return path; +} + + +static char *spapr_ibm_get_loc_code(sPAPRPHBState *sphb, PCIDevice *pdev) +{ +if (object_dynamic_cast(OBJECT(pdev), vfio-pci) != NULL) { +return spapr_phb_vfio_get_loc_code(sphb, pdev); +} else { +return spapr_phb_get_loc_code(sphb, pdev); +} ... does not match quite the behavior here, as far as I can see. I guess you also wanted to fall back to spapr_phb_get_loc_code() in case spapr_phb_vfio_get_loc_code() did not work as expected (eg. when the access to the /sys or /proc filesystem failed)? ... In current case, if getting vfio device loc_code fails, we would not add any ibm,loc_code info. I thought that would be the proper behaviour. As qemu_ would indicate it as an emulated device, which is not true. Or encode it as follows: vfio_name:phb-index:slot.fn So more checks would be needed here in that case: if (vfio) { buf = vfio_get_loc_code() if (!buf) buf = cook_vfio_get_loc_code() return buf; } else { ... } +} + /* Macros to operate with address in OF binding to PCI */ #define b_x(x, p, l)(((x) ((1(l))-1)) (p)) #define b_n(x) b_x((x), 31, 1) /* 0 if relocatable */ @@ -881,12 +945,12 @@ static void populate_resource_props(PCIDevice *d, ResourceProps *rp) } static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, - int phb_index, int drc_index, - const char *drc_name) + sPAPRPHBState *phb, int drc_index) As David already noted, the sPAPRPHBState related hunks likely rather belong to an earlier patch already? Yes, I will take care, this resuled while juggling with the patches :-) Regards Nikunj
Re: [Qemu-devel] [PATCH v15 00/10] KVM platform device passthrough
Re: [Qemu-devel] [PATCH 6/7] monitor: i: Add ARM specifics
On Tue, May 5, 2015 at 10:43 AM, Richard Henderson r...@twiddle.net wrote: On 05/05/2015 10:19 AM, Peter Maydell wrote: On 5 May 2015 at 05:45, Peter Crosthwaite crosthwaitepe...@gmail.com wrote: Add the ARM specific disassembly flags setup, so ARM can be correctly disassembled from the monitor. Signed-off-by: Peter Crosthwaite crosthwaite.pe...@gmail.com --- monitor.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/monitor.c b/monitor.c index d831d98..9d9f1e2 100644 --- a/monitor.c +++ b/monitor.c @@ -1217,6 +1217,17 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize, int flags; flags = 0; env = mon_get_cpu(); +#ifdef TARGET_ARM +if (env-thumb) { +flags |= 1; +} +if (env-bswap_code) { +flags |= 2; +} +if (env-aarch64) { +flags |= 4; +} +#endif monitor.c has no business poking around in the CPU state internals like this... You probably want a CPU method get_disas_flags() or something. -- PMM While this patch set does improve the current dismal state of affairs, I think the ideal solution is a cpu method that takes care of all the disassembly info setup. So I have made a start on this. The ARM, MB and CRIS in this patch series is rather easy. Its X86 im having trouble with but your example here looks like most of the work ... Indeed, the flags setup becomes less obscure when, instead of #ifdef TARGET_I386 if (wsize == 2) { flags = 1; } else if (wsize == 4) { flags = 0; } else { So here the monitor is actually using the argument memory-dump size to dictate the flags. Is this flawed and should we delete this wsize if-else and rely on the CPU-state driven logic for correct disas info selection? This wsize reliance seems unique to x86. I think we would have to give this up in a QOMified approach. /* as default we use the current CS size */ flags = 0; if (env) { #ifdef TARGET_X86_64 if ((env-efer MSR_EFER_LMA) (env-segs[R_CS].flags DESC_L_MASK)) This uses env-efer and segs to drive the flags... flags = 2; else #endif if (!(env-segs[R_CS].flags DESC_B_MASK)) flags = 1; } } in one place and #if defined(TARGET_I386) if (flags == 2) { s.info.mach = bfd_mach_x86_64; } else if (flags == 1) { s.info.mach = bfd_mach_i386_i8086; } else { s.info.mach = bfd_mach_i386_i386; } print_insn = print_insn_i386; in another, we merge the two so that we get s.info.mach = bfd_mach_i386_i8086; if (env-hflags (1U HF_CS32_SHIFT)) { But your new implementation uses hflags. Are they the same state? I couldnt find easy correltation between MSR_EFER_LMA and HF_CSXX_SHIFT (although I do see that map to hflags HF_LMA?). Is your code a functional change? s.info.mach = bfd_mach_i386_i386; } #ifdef TARGET_X86_64 if (env-hflags (1U HF_CS64_SHIFT)) { s.info.mach = bfd_mach_x86_64; } #endif I'm not sure what the right interface for this is, exactly. But I'd think that the CPUDebug structure would be initialized in the caller -- target or monitor -- while the mach and whatnot get filled in by the cpu hook. Maybe even have the cpu hook return the print_insn function to use. I went for adding print_insn to disassembly_info and passing just that to the hook. Patches soon! I might leave X86 out for the first spin. Regards, Peter r~
[Qemu-devel] [PATCH 0/4] Introduce eventfd support for virtio-mmio
Hello! I have updated and successfully tested an old patch set introducing eventfd support for virtio-mmio, enabling to use vhost-net with it: https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg00715.html I would like to upstream it, since virtio-mmio is still there. I know that some of you consider it deprecated, however i believe this is not entirely true. Because you can add it to machine models which are not supposed to have PCI (like vexpress). An old patch set relied on additional eventfd option in order to disable the support if not implemented in kernel. My version simply checks kvm_eventfds_enabled() for this purpose, so backwards compatibility is much better. I confirm that this solution significantly improves the network performance even without using irqfd. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia
[Qemu-devel] [Bug 1452062] Re: qemu-img will fail to convert images in 2.3.0
I can't reproduce this. qemu-img convert works just fine here. qemu-img convert -f qcow2 -O qcow2 -c -o compat=0.10 x.img y.img (from a random winXP guest image). ** Changed in: qemu Status: New = Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1452062 Title: qemu-img will fail to convert images in 2.3.0 Status in QEMU: Incomplete Bug description: Hello guys, There seems to be a bug in qemu-img with 2.3.0 that wasn't there in 2.2.1 qemu convert will always fail converting. See the output below: Started by upstream project Create windows image build number 73 originally caused by: Started by user anonymous Building remotely on builder (builder lab) in workspace /var/lib/jenkins/remote/workspace/Prepare windows Image [Prepare WS2008R2 Image] $ /bin/sh -xe /tmp/hudson4138890034689910617.sh + sudo bash -x /var/images/prepare_windows.sh WS2008R2 + set +x Preparing: windows Virtio CD: virtio-win-0.1.96.iso Sparsifying... qemu-img: error while compressing sector 11131648: Input/output error virt-sparsify: error: external command failed: qemu-img convert -f qcow2 -O 'qcow2' -c -o 'compat=0.10' '/tmp/sparsifyb459a0.qcow2' '/var/images/generated/WS2008R2.qcow2' virt-sparsify: If reporting bugs, run virt-sparsify with debugging enabled (-v) and include the complete output. Build step 'Execute shell' marked build as failure Warning: you have no plugins providing access control for builds, so falling back to legacy behavior of permitting any downstream builds to be triggered Finished: FAILURE Thanks, Dave To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1452062/+subscriptions
Re: [Qemu-devel] [PATCH v15 00/10] KVM platform device passthrough
Dear All, Please ignore the previous void message. For unknown reason the reply systematically ignores the content of the message? Retrying breaking the history... Content was: I am looking for Tested-by for this series and related Machvirt dynamic sysbus device instantiation. Did anyone try the last versions? I know those series are not plug play since additions are needed for your device: - device tree node addition in sysbus device - creation of a specialized QEMU VFIO platform device since base class is abstract For those willing to try and assess that code, please do not hesitate to contact me. I can and I am willing to help. If you already did, with success or facing issues, please send your Tested-by or report any issue for quick fix. This will definitively help in the upstream of this code. Thanks Best Regards Eric This series aims at enabling KVM platform device passthrough. On kernel side, the vfio platform driver is needed, available from 4.1-rc1 onwards. This series now only relies on the following QEMU series, for dynamic instantiation of the VFIO platform device from qemu command line: [1] [PATCH v12 0/4] machvirt dynamic sysbus device instantiation http://comments.gmane.org/gmane.comp.emulators.kvm.arm.devel/886 Both series are candidate for QEMU 2.4 and available at http://git.linaro.org/people/eric.auger/qemu.git (branch vfio_integ_v15) The series was tested on Calxeda Midway (ARMv7) where one xgmac is assigned to KVM host while the second one is assigned to the guest. Wiki for Calxeda Midway setup: https://wiki.linaro.org/LEG/Engineering/Virtualization/Platform_Device_Passthrough_on_Midway History: v14 - v15: - add Peter R-b on sysbus: add irq_routing_notifier - correct g_malloc0_n usage in skeleton - correct return values of reset related functions - include Cornelia's patch for header update v13 - v14: - remove v13 9, 10, 11 patch files and replace them by a single patch file sysbus: add irq_routing_notifier. - in skeleton, fix ENAMETOOLONG sign - remove VFIOINTp virtualID in add irq assignment patch file - removed trace_vfio_platform_start_eventfd v12 - v13: - header update but same update was already sent by Cornelia - Rework VFIO signaling irqfd setup: restored 2-step setup featuring eventfd setup on realize and then irqfd setup on irq binding. - irqfd setup now uses kvm_irqchip_add_irqfd_notifier and sysbus irq_set_hook override. This leads to the introduction of 6 patch files enabling those 2 features. Paolo advised to introduce kvm_irqchip_add_irqfd_notifier series in the VFIO one. I did the same for irq_set_hook series but if it is better I can submit it aside. - above changes made possible to remove x hw/vfio/platform: add capability to start IRQ propagation x hw/arm/virt: start VFIO IRQ propagation - in sysbus-fdt.c, use platform_bus_get_mmio_addr instead of deprecated mmio[0] property. Thanks to Bharat who pointed this issue out. also cpu_to_be32 was used for size and base (Vikram input) . - in skeleton misc corrections following Alex review. v11-v12: - add x-mmap property definition, without which the default value of vbasedev.allow_mmap is false, hence preventing the reg space from being mmapped. v10-v11: - rebase onto v2.3.0-rc0 (mainly related to PCIe support in virt) - add dma-coherent property for calxeda midway (fix revealed by removal of kernel-side vfio: type1: support for ARM SMMUS with VFIO_IOMMU_TYPE1) - virt modifications to start VFIO IRQ forwarding are now in a separate patch - rearrange linux header exports (those are still partial exports waiting for definitive 4.1-rc0) - take into account Alex Bennée comments: - use g_malloc0_n instead of g_malloc0 - use block declarations when possible - rework readlink returned value treatment - use g_strlcat in place strncat - re-arrange mutex locking for multiple IRQ support (user-side handled eventfds) - use g_snprintf instead of snprintf - change the order of functions to avoid pre-declaration in platform.c - add flags in VFIOINTp struct to detect whether the IRQ is automasked - some comment rewriting v9-v10: - rebase on vfio: cleanup vfio_get_device error path, remove vfio_populate_device: vfio_populate_device no more called in vfio_get_device but in vfio_base_device_init - update VFIO header according to vfio platform driver v13 (no AMBA) v8-v9: - rebase on 2.2.0 and machvirt dynamic sysbus instantiation v10 - v8 1-11 were pulled - patch files related to forwarding are moved in a seperate series since it depends on kernel series still in RFC. - introduction of basic VFIO platform device split into 3 patch files to ease the review (hope it will help). - add an author in platform.c - add deallocation in vfio_populate_device error case - add patch file doing the VFIO header sync - use VFIO_DEVICE_FLAGS_PLATFORM in vfio_populate_device - rename calxeda_xgmac.c into calxeda-xgmac.c - sysbus-fdt: add_calxeda_midway_xgmac_fdt_node g_free in case
Re: [Qemu-devel] [PATCH v3 4/5] qtest: precompute hex nibs
John Snow js...@redhat.com writes: Instead of letting printf and friends do this for us one byte at a time, fill a buffer ourselves and then send the entire buffer in one go. This gives a moderate speed improvement over the old method. Out of curiosity: how much of the improvement is due to doing our own buffering instead of printf()'s (assuming the stream is buffered), and how much is due to doing our own hex formatting instead of printf()'s?
Re: [Qemu-devel] [RFC PATCH v3 17/24] cpus: Reclaim vCPU objects
On Tue, May 05, 2015 at 05:20:04PM +1000, David Gibson wrote: On Fri, Apr 24, 2015 at 12:17:39PM +0530, Bharata B Rao wrote: From: Gu Zheng guz.f...@cn.fujitsu.com In order to deal well with the kvm vcpus (which can not be removed without any protection), we do not close KVM vcpu fd, just record and mark it as stopped into a list, so that we can reuse it for the appending cpu hot-add request if possible. It is also the approach that kvm guys suggested: https://www.mail-archive.com/kvm@vger.kernel.org/msg102839.html This patch also adds a QOM API object_has_no_children(Object *obj) that checks whether a given object has any child objects. This API is needed to release CPU core and socket objects when a vCPU is destroyed. I'm guessing this commit message needs updating, since you seem to have split this out into the previous patch. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com Signed-off-by: Gu Zheng guz.f...@cn.fujitsu.com Signed-off-by: Zhu Guihua zhugh.f...@cn.fujitsu.com Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com [Added core and socket removal bits] --- cpus.c | 67 include/qom/cpu.h| 11 + include/sysemu/kvm.h | 1 + kvm-all.c| 57 +++- kvm-stub.c | 5 5 files changed, 140 insertions(+), 1 deletion(-) diff --git a/cpus.c b/cpus.c index 0fac143..325f8a6 100644 --- a/cpus.c +++ b/cpus.c @@ -858,6 +858,47 @@ void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data) qemu_cpu_kick(cpu); } +static void qemu_destroy_cpu_core(Object *core) +{ +Object *socket = core-parent; + +object_unparent(core); +if (socket object_has_no_children(socket)) { +object_unparent(socket); +} This seems a bit odd to me. I thought the general idea of the new approaches to cpu hotplug meant that the hotplug sequence started from the top (the socket or core) and worked down to the threads. Rather than starting at the thread, and working up to the core and socket level. Yes that's true for hotplug as well as hot unplug curently. Plug or unplug starts at socket, moves down to cores and threads. However when the unplug request comes down to the thread, we have to destroy the vCPU and that's when we end up in this part of the code. Here the thread (vCPU) unparents itself from the core. The core can't unparent untill all its threads have unparented themselves. When all threads of a core are done unparenting, core goes ahead and unparents itself from its parent socket. Similarly socket can unparent when all cores under it have unparented themselves from the socket. This is the code that ensures that the socket device object finally gets cleared and the id associated with the hot removed socket device is available for reuse with next hotplug. +} + +static void qemu_kvm_destroy_vcpu(CPUState *cpu) +{ +Object *thread = OBJECT(cpu); +Object *core = thread-parent; + +CPU_REMOVE(cpu); + +if (kvm_destroy_vcpu(cpu) 0) { +error_report(kvm_destroy_vcpu failed.\n); +exit(EXIT_FAILURE); +} + +object_unparent(thread); +if (core object_has_no_children(core)) { +qemu_destroy_cpu_core(core); +} +} + Regards, Bharata.
Re: [Qemu-devel] [PATCH] parallel: Allow to disable CONFIG_PARALLEL
On Tue, May 05, 2015 at 11:59:50AM +0200, Thomas Huth wrote: On Tue, 5 May 2015 11:30:49 +0200 mreza...@redhat.com wrote: From: Miroslav Rezanina mreza...@redhat.com Disabling CONFIG_PARALLEL cause build failure as commit 07dc788 factored out initialization to parallel_hds_isa_init that is not build. Make calling parallel_hds_isa_init depending on CONFIG_PARALLEL so it can be correctly disabled. Signed-off-by: Miroslav Rezanina mreza...@redhat.com --- hw/i386/pc.c| 2 ++ hw/mips/mips_fulong2e.c | 2 ++ hw/mips/mips_malta.c| 2 ++ hw/sparc64/sun4u.c | 2 ++ 4 files changed, 8 insertions(+) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index a8e6be1..560464e 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1465,7 +1465,9 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, } serial_hds_isa_init(isa_bus, MAX_SERIAL_PORTS); +#ifdef CONFIG_PARALLEL parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS); +#endif Not sure, but is this pre-processor macro really defined if CONFIG_PARALLEL has been set in the makefile? I've hit some similar problem in the past and I had to discover that only the CONFIG_* options from config-host.mak get translated into #defines, all the others don't get translated. I might be wrong, but just to be sure, could you please double-check that CONFIG_PARALLEL is #defined if it's enabled in the .mak file? Yeah, you're right..I switch the behavior so thought of CONFIG_* from config-host.mak to be the one to not translated. And ifdef masked the issue. This patch is not working and has to be redone. If not: Where does the build break exactly? Does it fail for all three types, i386, mips and sun? I can't test mips/sun build but for i386 it fails on linking where parallel_hds_isa_init does not exists. Mirek Thomas
Re: [Qemu-devel] [PATCH v3 0/3] vnc: Fixes for unix socket error handling
On Di, 2015-05-05 at 11:07 -0400, Cole Robinson wrote: Minor fixes for unix socket error handling, see patches for details added to vnc queue. thanks, Gerd
Re: [Qemu-devel] Bug report - Windows XP guest failure
06.05.2015 08:41, Programmingkid wrote Just wanted to note that for the i386 target, Windows XP as a guest fails to boot. When it safe mode, loading always stops at Windows\System32\Drivers\Mup.sys. The guest boots in QEMU 2.2.0, so this seems to indicate a bug with the May 5th or earlier patch set. Please be a bit more specific, what do you run, with which options etc. Current git master works just fine with winXP guest, be it i386 or x86_64. Thanks, /mjt
[Qemu-devel] [PATCH 2/4] virtio-mmio: introduce set_guest_notifiers
Same as host notifier of virtio-mmio, most of codes came from virtio-pci. The kvm-arm does not yet support irqfd, need to fix the hard-coded part after kvm-arm gets irqfd support. Signed-off-by: Ying-Shiuan Pan address@hidden Signed-off-by: Pavel Fedin p.fe...@samsung.com --- hw/virtio/virtio-mmio.c | 60 + 1 file changed, 60 insertions(+) diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c index 32bf240..eab74ce 100644 --- a/hw/virtio/virtio-mmio.c +++ b/hw/virtio/virtio-mmio.c @@ -393,6 +393,65 @@ static void virtio_mmio_reset(DeviceState *d) proxy-guest_page_shift = 0; } +static int virtio_mmio_set_guest_notifier(DeviceState *d, int n, bool assign, + bool with_irqfd) +{ +VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d); +VirtIODevice *vdev = virtio_bus_get_device(proxy-bus); +VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev); +VirtQueue *vq = virtio_get_queue(vdev, n); +EventNotifier *notifier = virtio_queue_get_guest_notifier(vq); + +if (assign) { +int r = event_notifier_init(notifier, 0); +if (r 0) { +return r; +} +virtio_queue_set_guest_notifier_fd_handler(vq, true, with_irqfd); +} else { +virtio_queue_set_guest_notifier_fd_handler(vq, false, with_irqfd); +event_notifier_cleanup(notifier); +} + +if (vdc-guest_notifier_mask) { +vdc-guest_notifier_mask(vdev, n, !assign); +} + +return 0; +} + +static int virtio_mmio_set_guest_notifiers(DeviceState *d, int nvqs, bool assign) +{ +VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d); +VirtIODevice *vdev = virtio_bus_get_device(proxy-bus); +/* TODO: need to check if kvm-arm supports irqfd */ +bool with_irqfd = false; +int r, n; + +nvqs = MIN(nvqs, VIRTIO_PCI_QUEUE_MAX); + +for (n = 0; n nvqs; n++) { +if (!virtio_queue_get_num(vdev, n)) { +break; +} + +r = virtio_mmio_set_guest_notifier(d, n, assign, with_irqfd); +if (r 0) { +goto assign_error; +} +} + +return 0; + +assign_error: +/* We get here on assignment failure. Recover by undoing for VQs 0 .. n. */ +assert(assign); +while (--n = 0) { +virtio_mmio_set_guest_notifier(d, n, !assign, false); +} +return r; +} + static int virtio_mmio_set_host_notifier(DeviceState *opaque, int n, bool assign) { VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque); @@ -469,6 +528,7 @@ static void virtio_mmio_bus_class_init(ObjectClass *klass, void *data) k-save_config = virtio_mmio_save_config; k-load_config = virtio_mmio_load_config; k-set_host_notifier = virtio_mmio_set_host_notifier; +k-set_guest_notifiers = virtio_mmio_set_guest_notifiers; k-get_features = virtio_mmio_get_features; k-device_plugged = virtio_mmio_device_plugged; k-has_variable_vring_alignment = true; -- 1.9.5.msysgit.0
[Qemu-devel] [PATCH 3/4] virtio-mmio: start ioeventfd when status gets DRIVER_OK
Signed-off-by: Ying-Shiuan Pan address@hidden Signed-off-by: Pavel Fedin p.fe...@samsung.com --- hw/virtio/virtio-mmio.c | 47 ++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c index eab74ce..97a1fb0 100644 --- a/hw/virtio/virtio-mmio.c +++ b/hw/virtio/virtio-mmio.c @@ -118,7 +118,43 @@ static int virtio_mmio_set_host_notifier_internal(VirtIOMMIOProxy *proxy, } return r; } - + +static void virtio_mmio_start_ioeventfd(VirtIOMMIOProxy *proxy) +{ +VirtIODevice *vdev = virtio_bus_get_device(proxy-bus); +int n, r; + +if (proxy-ioeventfd_disabled || +proxy-ioeventfd_started) { +return; +} + +for (n = 0; n VIRTIO_PCI_QUEUE_MAX; n++) { +if (!virtio_queue_get_num(vdev, n)) { +continue; +} + +r = virtio_mmio_set_host_notifier_internal(proxy, n, true, true); +if (r 0) { +goto assign_error; +} +} +proxy-ioeventfd_started = true; +return; + +assign_error: +while (--n = 0) { +if (!virtio_queue_get_num(vdev, n)) { +continue; +} + +r = virtio_mmio_set_host_notifier_internal(proxy, n, false, false); +assert(r = 0); +} +proxy-ioeventfd_started = false; +error_report(%s: failed. Fallback to a userspace (slower)., __func__); +} + static void virtio_mmio_stop_ioeventfd(VirtIOMMIOProxy *proxy) { int r; @@ -317,7 +353,16 @@ static void virtio_mmio_write(void *opaque, hwaddr offset, uint64_t value, virtio_update_irq(vdev); break; case VIRTIO_MMIO_STATUS: +if (!(value VIRTIO_CONFIG_S_DRIVER_OK)) { +virtio_mmio_stop_ioeventfd(proxy); +} + virtio_set_status(vdev, value 0xff); + +if (value VIRTIO_CONFIG_S_DRIVER_OK) { +virtio_mmio_start_ioeventfd(proxy); +} + if (vdev-status == 0) { virtio_reset(vdev); } -- 1.9.5.msysgit.0
Re: [Qemu-devel] [PATCH 6/7] monitor: i: Add ARM specifics
On Tue, May 5, 2015 at 10:43 AM, Richard Henderson r...@twiddle.net wrote: On 05/05/2015 10:19 AM, Peter Maydell wrote: On 5 May 2015 at 05:45, Peter Crosthwaite crosthwaitepe...@gmail.com wrote: Add the ARM specific disassembly flags setup, so ARM can be correctly disassembled from the monitor. Signed-off-by: Peter Crosthwaite crosthwaite.pe...@gmail.com --- monitor.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/monitor.c b/monitor.c index d831d98..9d9f1e2 100644 --- a/monitor.c +++ b/monitor.c @@ -1217,6 +1217,17 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize, int flags; flags = 0; env = mon_get_cpu(); +#ifdef TARGET_ARM +if (env-thumb) { +flags |= 1; +} +if (env-bswap_code) { +flags |= 2; +} +if (env-aarch64) { +flags |= 4; +} +#endif monitor.c has no business poking around in the CPU state internals like this... You probably want a CPU method get_disas_flags() or something. -- PMM While this patch set does improve the current dismal state of affairs, I think the ideal solution is a cpu method that takes care of all the disassembly info setup. Indeed, the flags setup becomes less obscure when, instead of #ifdef TARGET_I386 if (wsize == 2) { flags = 1; } else if (wsize == 4) { flags = 0; } else { /* as default we use the current CS size */ flags = 0; if (env) { #ifdef TARGET_X86_64 if ((env-efer MSR_EFER_LMA) (env-segs[R_CS].flags DESC_L_MASK)) flags = 2; else #endif if (!(env-segs[R_CS].flags DESC_B_MASK)) flags = 1; } } in one place and #if defined(TARGET_I386) if (flags == 2) { s.info.mach = bfd_mach_x86_64; } else if (flags == 1) { s.info.mach = bfd_mach_i386_i8086; } else { s.info.mach = bfd_mach_i386_i386; } print_insn = print_insn_i386; in another, we merge the two so that we get s.info.mach = bfd_mach_i386_i8086; if (env-hflags (1U HF_CS32_SHIFT)) { s.info.mach = bfd_mach_i386_i386; } #ifdef TARGET_X86_64 if (env-hflags (1U HF_CS64_SHIFT)) { s.info.mach = bfd_mach_x86_64; } #endif I'm not sure what the right interface for this is, exactly. But I'd think that the CPUDebug structure would be initialized in the caller -- target or monitor -- while the mach and whatnot get filled in by the cpu hook. Maybe even have the cpu hook return the print_insn function to use. So something else I just thought of and started worrying about, is the the target_disas path is driving some of the flags from the disas context, whereas a hook will only have access to the CPUState/env. Can we rely on the env/CPUState always being up to date during target_disas (which happens at translate time?) or will we need to go field by field to make sure any env updates explicitly occur before target_disas? Regards, Peter r~
Re: [Qemu-devel] [RFC PATCH v3 21/24] spapr: Initialize hotplug memory address space
On Tue, May 05, 2015 at 05:33:33PM +1000, David Gibson wrote: On Fri, Apr 24, 2015 at 12:17:43PM +0530, Bharata B Rao wrote: Initialize a hotplug memory region under which all the hotplugged memory is accommodated. Also enable memory hotplug by setting CONFIG_MEM_HOTPLUG. Modelled on i386 memory hotplug. Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com So, the cpu hotplug stuff has been caught up in these generic level design discussions. Could you split out the memory hotplug part of this series so that we might be able to move forwards with that while the cpu hotplug discussion continues? Yes, in fact I too was planning to split memory and push it separately. --- default-configs/ppc64-softmmu.mak | 1 + hw/ppc/spapr.c| 38 ++ include/hw/ppc/spapr.h| 12 3 files changed, 51 insertions(+) diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak index 22ef132..16b3011 100644 --- a/default-configs/ppc64-softmmu.mak +++ b/default-configs/ppc64-softmmu.mak @@ -51,3 +51,4 @@ CONFIG_XICS_KVM=$(and $(CONFIG_PSERIES),$(CONFIG_KVM)) # For PReP CONFIG_MC146818RTC=y CONFIG_ISA_TESTDEV=y +CONFIG_MEM_HOTPLUG=y diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 910a50f..9dc4c36 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -125,6 +125,9 @@ struct sPAPRMachineState { /* public */ char *kvm_type; +ram_addr_t hotplug_memory_base; +MemoryRegion hotplug_memory; +bool enforce_aligned_dimm; As mentioned on the earlier version, on ppc we don't have compatibility reasons we need to have a mode for unaligned dimms. Get rid of this bool and treat it as always on instead. ok. Regards, Bharata.
Re: [Qemu-devel] [PATCH v6 12/17] target-s390x: Add S390 CPU class initialization routines
On Tue, 5 May 2015 11:34:06 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Mon, Apr 27, 2015 at 04:53:26PM +0200, Michael Mueller wrote: This patch provides routines to dynamically update the previously defined S390 CPU classes in the current host context. The main function performing this process is s390_setup_cpu_classes(). It takes the current host context and a facility list mask as parameter to setup the classes accordingly. It basically performs the following sub-tasks: - Update of CPU classes with accelerator specific host and QEMU properties - Mark adequate CPU class as default CPU class to be used for CPU model 'host' - Invalidate CPU classes not supported by this hosting machine - Define machine type aliases to latest GA number of a processor model - Define aliases for common CPU model names - Set CPU model alias 'host' to default CPU class Forthermore the patch provides the following routines: - cpu_desc_avail(), s390 specific stub indicating that list_cpus() can run - s390_setup_cpu_aliases(), adds cu model aliases - s390_cpu_classes_initialized(), test if CPU classes have been initialized - s390_fac_list_mask_by_machine(), returns facility list mask by machine - s390_current_fac_list_mask(), returns facility list mask of current machine Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com Acked-by: Christian Borntraeger borntrae...@de.ibm.com --- [...] +/** + * s390_setup_cpu_classes: + * @mode: the accelerator mode + * @prop: the machine property structure's address + * + * This function validates the defined cpu classes against the given + * machine properties @prop. Only cpu classes that are runnable on the + * current host will be set active. In addition the corresponding + * cpuid, ibc value and the active set of facilities will be + * initialized. Depending on @mode, the function porforms operations + * on the current or the temporary accelerator properies. + * + * Since: 2.4 + */ +void s390_setup_cpu_classes(S390AccelMode mode, S390MachineProps *prop, +uint64_t *fac_list_mask) +{ Can't you replace the S390AccelMode arguments everywhere with simply an AccelState pointer? That's the kind of thing that should have been easier to implement using the accel QOM stuff. Would just make sense in conjunction with an AccelId indexed array in the CPU class but see my concerns below. If you still need to save accel-specific data somewhere (like the is_active, is_host and fac_list arrays), maybe it can be indexed using the AccelId enum you have introduced, instead of S390AccelMode? I had an AccelId indexed array in a previous version of the patch but dismissed it in favor to this AccelMode index approach for the following reasons: a) There is just one accelerator active and and a second set of values is used for the query-cpu-definitions case. Using the AcceldId index would instantly double the required memory being used for no reason. The size of the second dimension in uint64_t fac_list[ACCEL_MODE_MAX][FAC_LIST_CPU_S390_SIZE_UINT64]; is architecturally allowed to grow up to 2KB. b) The information stored has more dimensions than just the accelerator, it also contains the selected machine (s390-virtio) which is represented by means of qemu_s390_fac_list_mask[] which currently is identical for all machines but that will change as the implementation progresses. So AccelMode (current, tmp) might also not fully express the semantics. Michael +GSList *list; +ParmAddrAddrModeMask parm = { +.mode = mode, +.prop = prop, +.mask = fac_list_mask, +.host_cc = NULL, +}; + +list = object_class_get_list(TYPE_S390_CPU, false); +list = g_slist_sort(list, s390_cpu_class_asc_order_compare); + +g_slist_foreach(list, (GFunc) s390_update_cpu_class, (gpointer) parm); +g_slist_foreach(list, (GFunc) s390_mark_host_cpu_class, (gpointer) parm); +g_slist_foreach(list, (GFunc) s390_disable_not_supported_cpu_class, parm); + +g_slist_free(list); +} + [...]
Re: [Qemu-devel] [PATCH v2 1/6] mirror: Discard target sectors if not allocated at source side
I just wander ifbdrv_is_allocated_above works as it is considered,migrate image in format of qcow2 run into such backtrace:#0 0x7f9e73822c6d in lseek64 () at ../sysdeps/unix/syscall-template.S:82#1 0x7f9e765f08e4 in find_allocation (bs=value optimized out, sector_num=value optimized out, nb_sectors=20480, pnum=0x7f9e7bab00fc) at block/raw-posix.c:1285#2 raw_co_get_block_status (bs=value optimized out, sector_num=value optimized out, nb_sectors=20480, pnum=0x7f9e7bab00fc) at block/raw-posix.c:1381#3 0x7f9e765cfe61 in bdrv_co_get_block_status (bs=0x7f9e790e4300, sector_num= 22470528, nb_sectors=20480, pnum=0x7f9e7bab00fc) at block.c:3593#4 0x7f9e765cffa8 in bdrv_co_get_block_status (bs=0x7f9e790e00a0, sector_num= 22466560, nb_sectors=value optimized out, pnum=0x7f9e7bab00fc) at block.c:3620#5 0x7f9e765cfff7 in bdrv_get_block_status_co_entry (opaque=0x7f9e7bab0080) at block.c:3639#6 0x7f9e765d0088 in bdrv_get_block_status (bs=value optimized out, sector_num=value optimized out, nb_sectors=value optimized out, pnum=value optimized out) at block.c:3663#7 0x7f9e765d00a9 in bdrv_is_allocated (bs=0x7f9e790e00a0, sector_num=value optimized out, nb_sectors=value optimized out, pnum=value optimized out) at block.c:3677#8 0x7f9e765d0166 in bdrv_is_allocated_above (top=0x7f9e790e00a0, base=0x0, sector_num=22466560, nb_sectors=20480, pnum=0x7f9e7bab020c) at block.c:3709#9 0x7f9e765dcf9e in mirror_iteration (opaque=0x7f9e79486110) at block/mirror.c:305#10 mirror_run (opaque=0x7f9e79486110) at block/mirror.c:430#11 0x7f9e766120eb in coroutine_trampoline (i0=value optimized out, i1=value optimized out) at coroutine-ucontext.c:118#12 0x7f9e734c4b70 in ?? () from /lib64/libc.so.6#13 0x7a897850 in ?? ()#14 0x in ?? ()it is too slow to tolerate…...Original MessageSender:Fam Zhengf...@redhat.comRecipient:qemu-develqemu-devel@nongnu.orgCc:Kevin Wolfkw...@redhat.com; Stefan Hajnoczistefa...@redhat.com; qemu-blockqemu-bl...@nongnu.org; pbonzinipbonz...@redhat.com; jsnowjs...@redhat.com; wangxiaolongwangxiaol...@ucloud.cnDate:Wednesday, May 6, 2015 12:52Subject:[PATCH v2 1/6] mirror: Discard target sectors if not allocated at source sideIf guest discards a source cluster during mirror, we would want to discard target side as well. Signed-off-by: Fam Zheng f...@redhat.com --- block/mirror.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 58f391a..37a5b61 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -163,6 +163,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) int64_t end, sector_num, next_chunk, next_sector, hbitmap_next_sector; uint64_t delay_ns = 0; MirrorOp *op; +int pnum; s-sector_num = hbitmap_iter_next(s-hbi); if (s-sector_num 0) { @@ -289,8 +290,15 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) s-in_flight++; s-sectors_in_flight += nb_sectors; trace_mirror_one_iteration(s, sector_num, nb_sectors); -bdrv_aio_readv(source, sector_num, op-qiov, nb_sectors, - mirror_read_complete, op); + +if (!bdrv_is_allocated_above(source, NULL, sector_num, + nb_sectors, pnum)) { +bdrv_aio_discard(s-target, sector_num, nb_sectors, + mirror_write_complete, op); +} else { +bdrv_aio_readv(source, sector_num, op-qiov, nb_sectors, + mirror_read_complete, op); +} return delay_ns; } -- 1.9.3
Re: [Qemu-devel] [RFC PATCH v12 16/21]
From: Paolo Bonzini [mailto:pbonz...@redhat.com] On 05/05/2015 12:22, Pavel Dovgaluk wrote: This patch is the reduced version of prior bottom halves patch. dma-helpers.c is also related to block devices, so it's better not to change it now. Ok. Perhaps you can add a replay event for ptimers instead of touching bottom halves? It seems reasonable. Then I'll have to use BH as a simple callback by implementing run bh function. This BH will be added to replay queue instead of adding it to BH queue. Pavel Dovgalyuk -Original Message- From: Pavel Dovgalyuk [mailto:pavel.dovga...@ispras.ru] Sent: Tuesday, May 05, 2015 1:19 PM To: qemu-devel@nongnu.org Cc: peter.mayd...@linaro.org; peter.crosthwa...@xilinx.com; ebl...@redhat.com; mark.bur...@greensocs.com; r...@ispras.ru; batuz...@ispras.ru; maria.klimushenk...@ispras.ru; pavel.dovga...@ispras.ru; pbonz...@redhat.com; alex.ben...@linaro.org; fred.kon...@greensocs.com Subject: [RFC PATCH v12 16/21] Signed-off-by: Pavel Dovgalyuk pavel.dovga...@ispras.ru --- async.c | 24 +++- dma-helpers.c|4 +++- hw/timer/arm_timer.c |2 +- include/block/aio.h | 18 ++ include/qemu/main-loop.h |1 + main-loop.c |5 + replay/replay-events.c | 16 replay/replay-internal.h |1 + replay/replay.h |2 ++ stubs/replay.c |4 10 files changed, 74 insertions(+), 3 deletions(-) diff --git a/async.c b/async.c index bd975c9..d092aa2 100644 --- a/async.c +++ b/async.c @@ -27,6 +27,7 @@ #include block/thread-pool.h #include qemu/main-loop.h #include qemu/atomic.h +#include replay/replay.h /***/ /* bottom halves (can be seen as timers which expire ASAP) */ @@ -39,6 +40,8 @@ struct QEMUBH { bool scheduled; bool idle; bool deleted; +bool replay; +uint64_t id; }; QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque) @@ -56,6 +59,21 @@ QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque) return bh; } +QEMUBH *aio_bh_new_replay(AioContext *ctx, QEMUBHFunc *cb, void *opaque, + uint64_t id) +{ +QEMUBH *bh = aio_bh_new(ctx, cb, opaque); +bh-replay = true; +bh-id = id; +return bh; +} + +void aio_bh_call(void *opaque) +{ +QEMUBH *bh = (QEMUBH *)opaque; +bh-cb(bh-opaque); +} + /* Multiple occurrences of aio_bh_poll cannot be called concurrently */ int aio_bh_poll(AioContext *ctx) { @@ -78,7 +96,11 @@ int aio_bh_poll(AioContext *ctx) if (!bh-idle) ret = 1; bh-idle = 0; -bh-cb(bh-opaque); +if (!bh-replay) { +aio_bh_call(bh); +} else { +replay_add_bh_event(bh, bh-id); +} } } diff --git a/dma-helpers.c b/dma-helpers.c index 6918572..357d7e9 100644 --- a/dma-helpers.c +++ b/dma-helpers.c @@ -13,6 +13,7 @@ #include qemu/range.h #include qemu/thread.h #include qemu/main-loop.h +#include replay/replay.h /* #define DEBUG_IOMMU */ @@ -96,7 +97,8 @@ static void continue_after_map_failure(void *opaque) { DMAAIOCB *dbs = (DMAAIOCB *)opaque; -dbs-bh = qemu_bh_new(reschedule_dma, dbs); +dbs-bh = qemu_bh_new_replay(reschedule_dma, dbs, + replay_get_current_step()); qemu_bh_schedule(dbs-bh); } diff --git a/hw/timer/arm_timer.c b/hw/timer/arm_timer.c index 1452910..97784a0 100644 --- a/hw/timer/arm_timer.c +++ b/hw/timer/arm_timer.c @@ -168,7 +168,7 @@ static arm_timer_state *arm_timer_init(uint32_t freq) s-freq = freq; s-control = TIMER_CTRL_IE; -bh = qemu_bh_new(arm_timer_tick, s); +bh = qemu_bh_new_replay(arm_timer_tick, s, 0); s-timer = ptimer_init(bh); vmstate_register(NULL, -1, vmstate_arm_timer, s); return s; diff --git a/include/block/aio.h b/include/block/aio.h index 82cdf78..ed76b43 100644 --- a/include/block/aio.h +++ b/include/block/aio.h @@ -35,6 +35,8 @@ struct BlockAIOCB { const AIOCBInfo *aiocb_info; BlockDriverState *bs; BlockCompletionFunc *cb; +bool replay; +uint64_t replay_step; void *opaque; int refcnt; }; @@ -144,6 +146,17 @@ void aio_context_release(AioContext *ctx); QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque); /** + * aio_bh_new_replay: Allocate a new bottom half structure for replay. + * + * This function calls aio_bh_new function and also fills replay parameters + * of the BH structure. BH created with this function in record/replay mode + * are executed through the replay queue
Re: [Qemu-devel] [PATCH 1/2] virtio-console: notify chardev when writable
On Di, 2015-05-05 at 16:58 +0200, Marc-André Lureau wrote: When the virtio serial is writable, notify the chardev backend with qemu_chr_accept_input(). added both patches to spice patch queue. thanks, Gerd
[Qemu-devel] [PATCH 1/4] virtio-mmio: introduce set_host_notifier()
set_host_notifier() is introduced into virtio-mmio now. Most of codes came from virtio-pci. Signed-off-by: Ying-Shiuan Pan address@hidden Signed-off-by: Pavel Fedin p.fe...@samsung.com --- hw/virtio/virtio-mmio.c | 70 + 1 file changed, 70 insertions(+) diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c index 10123f3..32bf240 100644 --- a/hw/virtio/virtio-mmio.c +++ b/hw/virtio/virtio-mmio.c @@ -23,6 +23,7 @@ #include hw/virtio/virtio.h #include qemu/host-utils.h #include hw/virtio/virtio-bus.h +#include qemu/error-report.h /* #define DEBUG_VIRTIO_MMIO */ @@ -87,8 +88,58 @@ typedef struct { uint32_t guest_page_shift; /* virtio-bus */ VirtioBusState bus; +bool ioeventfd_disabled; +bool ioeventfd_started; } VirtIOMMIOProxy; +static int virtio_mmio_set_host_notifier_internal(VirtIOMMIOProxy *proxy, + int n, bool assign, bool set_handler) +{ +VirtIODevice *vdev = virtio_bus_get_device(proxy-bus); +VirtQueue *vq = virtio_get_queue(vdev, n); +EventNotifier *notifier = virtio_queue_get_host_notifier(vq); +int r = 0; + +if (assign) { +r = event_notifier_init(notifier, 1); +if (r 0) { +error_report(%s: unable to init event notifier: %d, + __func__, r); +return r; +} +virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler); +memory_region_add_eventfd(proxy-iomem, VIRTIO_MMIO_QUEUENOTIFY, 4, + true, n, notifier); +} else { +memory_region_del_eventfd(proxy-iomem, VIRTIO_MMIO_QUEUENOTIFY, 4, + true, n, notifier); +virtio_queue_set_host_notifier_fd_handler(vq, false, false); +event_notifier_cleanup(notifier); +} +return r; +} + +static void virtio_mmio_stop_ioeventfd(VirtIOMMIOProxy *proxy) +{ +int r; +int n; +VirtIODevice *vdev = virtio_bus_get_device(proxy-bus); + +if (!proxy-ioeventfd_started) { +return; +} + +for (n = 0; n VIRTIO_PCI_QUEUE_MAX; n++) { +if (!virtio_queue_get_num(vdev, n)) { +continue; +} + +r = virtio_mmio_set_host_notifier_internal(proxy, n, false, false); +assert(r = 0); +} +proxy-ioeventfd_started = false; +} + static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size) { VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque; @@ -342,6 +393,24 @@ static void virtio_mmio_reset(DeviceState *d) proxy-guest_page_shift = 0; } +static int virtio_mmio_set_host_notifier(DeviceState *opaque, int n, bool assign) +{ +VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque); + +/* Stop using ioeventfd for virtqueue kick if the device starts using host + * notifiers. This makes it easy to avoid stepping on each others' toes. + */ +proxy-ioeventfd_disabled = assign; +if (assign) { +virtio_mmio_stop_ioeventfd(proxy); +} +/* We don't need to start here: it's not needed because backend + * currently only stops on status change away from ok, + * reset, vmstop and such. If we do add code to start here, + * need to check vmstate, device state etc. */ +return virtio_mmio_set_host_notifier_internal(proxy, n, assign, false); +} + /* virtio-mmio device */ /* This is called by virtio-bus just after the device is plugged. */ @@ -399,6 +468,7 @@ static void virtio_mmio_bus_class_init(ObjectClass *klass, void *data) k-notify = virtio_mmio_update_irq; k-save_config = virtio_mmio_save_config; k-load_config = virtio_mmio_load_config; +k-set_host_notifier = virtio_mmio_set_host_notifier; k-get_features = virtio_mmio_get_features; k-device_plugged = virtio_mmio_device_plugged; k-has_variable_vring_alignment = true; -- 1.9.5.msysgit.0
Re: [Qemu-devel] qemu-img convert (vmdk)
On Wed, 05/06 12:01, Kevin Wolf wrote: Am 05.05.2015 um 19:01 hat Antoni Villalonga geschrieben: Hi, Is my first email to that list ;) I can reproduce this bug with v2.2 and v2.3. I'm not sure about the results after testing with v2.1 (doesn't show errors but seems to be still broken). % qemu-img convert -f raw -O vmdk -o subformat=streamOptimized 100GB_inputfile.img outputfile0.vmdk (no shown errors/warnings/info) % qemu-img convert -f vmdk -O qcow2 outputfile0.vmdk outputfile11.qcow2 qemu-img: error while reading sector 278528: Invalid argument Let me know if I can provide you more info. Fam, any idea? It's a bug. I can reproduce it with my 21G guest image. I'll take a look. Fam
[Qemu-devel] [Patch V2 0/4] [Patch V2 0/4] Windows MSI installation package
The second version of commits's set take into account Paolo Bonzini remarks. Typo in WXS file fixed, QEMU GA-related CLI options renamed, '--enable-guest-agent-msi'/ '--disable-guest-agent-msi' processing logic changed so that MSI build is configured by default, unless some prerequisite is missing and MinGW DLL path variable is computed together with all QEMU GA MSI variables. Also, I've slightly changed Makefile structure so that if MSI build was not configured, Makefile prints suitable message. Yossi Hindin (4): qemu-ga: adding vss-[un]install options qemu-ga: debug printouts to help troubleshoot installation qemu-ga: Introduce Windows MSI script qemu-ga: Building Windows MSI installation with configure/Makefile Makefile | 24 +++- configure | 66 + qga/channel-win32.c | 2 +- qga/commands-win32.c | 1 + qga/installer/qemu-ga.wxs | 145 ++ qga/main.c| 10 +++- 6 files changed, 245 insertions(+), 3 deletions(-) create mode 100644 qga/installer/qemu-ga.wxs -- 2.1.0
[Qemu-devel] [Patch V2 1/4] qemu-ga: adding vss-[un]install options
Existing command line options include '-s install' and '-s uninstall'. These options install/uninstall both Windows QEMU GA service and optional VSS COM server. The QEMU GA Windows service allows always-on serving guest agent's QMP commands and VSS COM server enables guest agent integration with Volume Shadow Service. This commit introdices new options '-s vss-install' and '-s vss-uninstall', affecting only GA VSS COM server registration. The new options are useful for registering and unregistering the COM server during MSI installation, upgrade and uninstallation. Signed-off-by: Yossi Hindin yhin...@redhat.com --- qga/main.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/qga/main.c b/qga/main.c index 9939a2b..7e1e438 100644 --- a/qga/main.c +++ b/qga/main.c @@ -211,7 +211,7 @@ static void usage(const char *cmd) -V, --version print version information and exit\n -d, --daemonize become a daemon\n #ifdef _WIN32 - -s, --service service commands: install, uninstall\n + -s, --service service commands: install, uninstall, vss-install, vss-uninstall\n #endif -b, --blacklist comma-separated list of RPCs to disable (no spaces, \?\\n to list available RPCs)\n @@ -1036,6 +1036,14 @@ int main(int argc, char **argv) } else if (strcmp(service, uninstall) == 0) { ga_uninstall_vss_provider(); return ga_uninstall_service(); +} else if (strcmp(service, vss-install) == 0) { +if (ga_install_vss_provider()) { +return EXIT_FAILURE; +} +return EXIT_SUCCESS; +} else if (strcmp(service, vss-uninstall) == 0) { +ga_uninstall_vss_provider(); +return EXIT_SUCCESS; } else { printf(Unknown service command.\n); return EXIT_FAILURE; -- 2.1.0
[Qemu-devel] [Patch V2 2/4] qemu-ga: debug printouts to help troubleshoot installation
Debug printouts extended, helps installation troubleshooting Signed-off-by: Yossi Hindin yhin...@redhat.com --- qga/channel-win32.c | 2 +- qga/commands-win32.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/qga/channel-win32.c b/qga/channel-win32.c index 0d5e5f5..04fa5e4 100644 --- a/qga/channel-win32.c +++ b/qga/channel-win32.c @@ -306,7 +306,7 @@ static gboolean ga_channel_open(GAChannel *c, GAChannelMethod method, OPEN_EXISTING, FILE_FLAG_NO_BUFFERING | FILE_FLAG_OVERLAPPED, NULL); if (c-handle == INVALID_HANDLE_VALUE) { -g_critical(error opening path); +g_critical(error opening path %s, newpath); return false; } diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 3ef0549..d0aaec7 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -721,6 +721,7 @@ GList *ga_command_blacklist_init(GList *blacklist) } if (!vss_init(true)) { +g_debug(vss_init failed, vss commands are going to be disabled); const char *list[] = { guest-get-fsinfo, guest-fsfreeze-status, guest-fsfreeze-freeze, guest-fsfreeze-thaw, NULL}; -- 2.1.0
[Qemu-devel] [Patch V2 4/4] qemu-ga: Building Windows MSI installation with configure/Makefile
New options were added to enable Windows MSI installation package creation: Option --enable-guest-agent-msi, like the name suggests, enables building Windows MSI package for QEMU guest agent; option --disable-guest-agent-msi disables MSI package creation; by default, no MSI package is created Signed-off-by: Yossi Hindin yhin...@redhat.com --- Makefile | 24 ++- configure | 66 +++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 93af871..ef14cb8 100644 --- a/Makefile +++ b/Makefile @@ -74,7 +74,7 @@ Makefile: ; configure: ; .PHONY: all clean cscope distclean dvi html info install install-doc \ - pdf recurse-all speed test dist + pdf recurse-all speed test dist msi $(call set-vpath, $(SRC_PATH)) @@ -287,10 +287,32 @@ $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN) qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a $(call LINK, $^) +ifdef QEMU_GA_MSI_ENABLED +QEMU_GA_MSI=qemu-ga-$(ARCH).msi + +msi: ${QEMU_GA_MSI} + +$(QEMU_GA_MSI): qemu-ga.exe + +ifdef QEMU_GA_MSI_WITH_VSS +$(QEMU_GA_MSI): qga/vss-win32/qga-vss.dll +endif + +$(QEMU_GA_MSI): config-host.mak + +$(QEMU_GA_MSI): qga/installer/qemu-ga.wxs + $(call quiet-command,QEMU_GA_VERSION=$(QEMU_GA_VERSION) QEMU_GA_MANUFACTURER=$(QEMU_GA_MANUFACTURER) QEMU_GA_DISTRO=$(QEMU_GA_DISTRO) \ + wixl -o $@ $(QEMU_GA_MSI_ARCH) $(QEMU_GA_MSI_WITH_VSS) $(QEMU_GA_MSI_MINGW_DLL_PATH) $, WIXL $@) +else +msi: + @echo MSI build not configured or dependency resolution failed (reconfigure with --enable-guest-agent-msi option) +endif + clean: # avoid old build problems by removing potentially incorrect old files rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h rm -f qemu-options.def + rm -f *.msi find . \( -name '*.l[oa]' -o -name '*.so' -o -name '*.dll' -o -name '*.mo' -o -name '*.[oda]' \) -type f -exec rm {} + rm -f $(filter-out %.tlb,$(TOOLS)) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~ rm -f fsdev/*.pod diff --git a/configure b/configure index 6969f6f..76b5560 100755 --- a/configure +++ b/configure @@ -316,6 +316,7 @@ snappy= bzip2= guest_agent= guest_agent_with_vss=no +guest_agent_msi= vss_win32_sdk= win_sdk=no want_tools=yes @@ -1069,6 +1070,10 @@ for opt do ;; --disable-guest-agent) guest_agent=no ;; + --enable-guest-agent-msi) guest_agent_msi=yes + ;; + --disable-guest-agent-msi) guest_agent_msi=no + ;; --with-vss-sdk) vss_win32_sdk= ;; --with-vss-sdk=*) vss_win32_sdk=$optarg @@ -1383,6 +1388,8 @@ Advanced options (experts only): reading bzip2-compressed dmg images) --disable-guest-agentdisable building of the QEMU Guest Agent --enable-guest-agent enable building of the QEMU Guest Agent + --enable-guest-agent-msi enable building guest agent Windows MSI installation package + --disable-guest-agent-msi disable building guest agent Windows MSI installation --with-vss-sdk=SDK-path enable Windows VSS support in QEMU Guest Agent --with-win-sdk=SDK-path path to Windows Platform SDK (to build VSS .tlb) --disable-seccompdisable seccomp support @@ -3832,6 +3839,56 @@ if test $mingw32 = yes -a $guest_agent != no -a $guest_agent_with_vss fi ## +# Guest agent Window MSI package + +if test $guest_agent != yes; then + if test $guest_agent_msi = yes; then +error_exit MSI guest agent package requires guest agent enabled + fi + guest_agent_msi=no +elif test $mingw32 != yes; then + if test $guest_agent_msi = yes; then +error_exit MSI guest agent package is available only for MinGW Windows cross-compilation + fi + guest_agent_msi=no +elif ! has wixl; then + if test $guest_agent_msi = yes; then +error_exit MSI guest agent package requires wixl tool installed ( usually from msitools package ) + fi + guest_agent_msi=no +fi + +if test $guest_agent_msi != no; then + if test $guest_agent_with_vss = yes; then +QEMU_GA_MSI_WITH_VSS=-D InstallVss + fi + + if test $QEMU_GA_MANUFACTURER = ; then +QEMU_GA_MANUFACTURER=QEMU + fi + + if test $QEMU_GA_DISTRO = ; then +QEMU_GA_DISTRO=Linux + fi + + if test $QEMU_GA_VERSION = ; then + QEMU_GA_VERSION=`cat $source_path/VERSION` + fi + + QEMU_GA_MSI_MINGW_DLL_PATH=-D Mingw_dlls=`$pkg_config --variable=prefix glib-2.0`/bin + + case $cpu in + x86_64) +QEMU_GA_MSI_ARCH=-a x64 -D Arch=64 +;; + i386) +QEMU_GA_MSI_ARCH=-D Arch=32 +;; + *) +error_exit CPU $cpu not supported for building installation package +;; + esac +fi ## # check if we have fdatasync @@ -4500,6 +4557,15 @@ if test $mingw32 = yes ; then echo CONFIG_QGA_VSS=y $config_host_mak echo WIN_SDK=\$win_sdk\ $config_host_mak fi + if test $guest_agent_msi != no;
[Qemu-devel] [Patch V2 3/4] qemu-ga: Introduce Windows MSI script
The script enables building Windows MSI installation package on Linux with wixl tool. Signed-off-by: Yossi Hindin yhin...@redhat.com --- qga/installer/qemu-ga.wxs | 145 ++ 1 file changed, 145 insertions(+) create mode 100644 qga/installer/qemu-ga.wxs diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs new file mode 100644 index 000..2c43f1b --- /dev/null +++ b/qga/installer/qemu-ga.wxs @@ -0,0 +1,145 @@ +?xml version=1.0 encoding=UTF-8? +Wix xmlns=http://schemas.microsoft.com/wix/2006/wi; + ?ifndef env.QEMU_GA_VERSION ? +?error Environment variable QEMU_GA_VERSION undefined? + ?endif? + + ?ifndef env.QEMU_GA_DISTRO ? +?error Environment variable QEMU_GA_DISTRO undefined? + ?endif? + + ?ifndef env.QEMU_GA_MANUFACTURER ? +?error Environment variable QEMU_GA_MANUFACTURER undefined? + ?endif? + + ?ifndef var.Arch? +?error Define Arch to 32 or 64? + ?endif? + + ?ifndef var.Mingw_bin? +?if $(var.Arch) = 64? + ?define Mingw_bin=/usr/x86_64-w64-mingw32/sys-root/mingw/bin ? +?endif? +?if $(var.Arch) = 32? + ?define Mingw_bin=/usr/i686-w64-mingw32/sys-root/mingw/bin ? +?endif? + ?endif? + + ?if $(var.Arch) = 64? +?define ArchLib=libgcc_s_seh-1.dll? +?define GaProgramFilesFolder=ProgramFiles64Folder ? + ?endif? + + ?if $(var.Arch) = 32? +?define ArchLib=libgcc_s_sjlj-1.dll? +?define GaProgramFilesFolder=ProgramFilesFolder ? + ?endif? + + ?ifndef var.ArchLib ? +?error Unexpected Arch value $(var.Arch)? + ?endif? + + Product +Name=QEMU guest agent +Id=* +UpgradeCode={EB6B8302-C06E-4bec-ADAC-932C68A3A98D} +Manufacturer=$(env.QEMU_GA_MANUFACTURER) +Version=$(env.QEMU_GA_VERSION) +Language=1033 +?if $(var.Arch) = 32 ? +Condition Message=Error: 32-bit version of Qemu GA can not be installed on 64-bit Windows.NOT VersionNT64/Condition +?endif? +Package + Manufacturer=$(env.QEMU_GA_MANUFACTURER) + InstallerVersion=200 + Languages=1033 + Compressed=yes + InstallScope=perMachine + / +Media Id=1 Cabinet=qemu_ga.$(env.QEMU_GA_VERSION).cab EmbedCab=yes / +Property Id=WHSLogo1/Property +Property Id=PREVIOUSVERSIONSINSTALLED / +Upgrade Id={EB6B8302-C06E-4bec-ADAC-932C68A3A98D} + UpgradeVersion +Minimum=1.0.0.0 Maximum=$(env.QEMU_GA_VERSION) +Property=PREVIOUSVERSIONSINSTALLED +IncludeMinimum=yes IncludeMaximum=no / +/Upgrade + +Directory Id=TARGETDIR Name=SourceDir + Directory Id=$(var.GaProgramFilesFolder) Name=QEMU Guest Agent +Directory Id=qemu_ga_directory Name=Qemu-ga + Component Id=qemu_ga Guid={908B7199-DE2A-4dc6-A8D0-27A5AE444FEA} +File Id=qemu_ga.exe Name=qemu-ga.exe Source=../../qemu-ga.exe KeyPath=yes DiskId=1/ +?ifdef var.InstallVss ? +File Id=qga_vss.dll Name=qga-vss.dll Source=../vss-win32/qga-vss.dll KeyPath=no DiskId=1/ +File Id=qga_vss.tlb Name=qga-vss.tlb Source=../vss-win32/qga-vss.tlb KeyPath=no DiskId=1/ +?endif? +File Id=iconv.dll Name=iconv.dll Source=$(var.Mingw_bin)/iconv.dll KeyPath=no DiskId=1/ +File Id=libgcc_arch_lib Name=$(var.ArchLib) Source=$(var.Mingw_bin)/$(var.ArchLib) KeyPath=no DiskId=1/ +File Id=libglib_2.0_0.dll Name=libglib-2.0-0.dll Source=$(var.Mingw_bin)/libglib-2.0-0.dll KeyPath=no DiskId=1/ +File Id=libintl_8.dll Name=libintl-8.dll Source=$(var.Mingw_bin)/libintl-8.dll KeyPath=no DiskId=1/ +File Id=libssp_0.dll Name=libssp-0.dll Source=$(var.Mingw_bin)/libssp-0.dll KeyPath=no DiskId=1/ +File Id=libwinpthread_1.dll Name=libwinpthread-1.dll Source=$(var.Mingw_bin)/libwinpthread-1.dll KeyPath=no DiskId=1/ +ServiceInstall + Id=ServiceInstaller + Type=ownProcess + Vital=yes + Name=QEMU-GA + DisplayName=QEMU Guest Agent + Description=QEMU Guest Agent + Start=auto + Account=LocalSystem + ErrorControl=ignore + Interactive=no + Arguments=-d + +/ServiceInstall +ServiceControl Id=StartService Start=install Stop=both Remove=uninstall Name=QEMU-GA Wait=no / + /Component + + Component Id=registry_entries Guid=d075d109-51ca-11e3-9f8b-000c29858960 +RegistryKey Root=HKLM + Key=Software\$(env.QEMU_GA_MANUFACTURER)\$(env.QEMU_GA_DISTRO)\Tools\QemuGA + RegistryValue Type=string Name=ProductID Value=fb0a0d66-c7fb-4e2e-a16b-c4a3bfe8d13b / + RegistryValue Type=string Name=Version Value=$(env.QEMU_GA_VERSION) / +/RegistryKey + /Component +/Directory + /Directory +/Directory + +Property Id=cmd Value=cmd.exe/ + +?ifdef var.InstallVss ? +CustomAction
[Qemu-devel] [PATCH] vmdk: Fix next_cluster_sector for compressed write
This fixes the bug introduced by commit c6ac36e (vmdk: Optimize cluster allocation). Sometimes, write_len could be larger than cluster size, because it contains both data and marker. We must advance next_cluster_sector in this case, otherwise the image gets corrupted. Reported-by: Antoni Villalonga qemu-l...@friki.cat Signed-off-by: Fam Zheng f...@redhat.com --- block/vmdk.c | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 1c5e2ef..4b4a862 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1302,6 +1302,8 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset, uLongf buf_len; const uint8_t *write_buf = buf; int write_len = nb_sectors * 512; +int64_t write_offset; +int64_t write_end_sector; if (extent-compressed) { if (!extent-has_marker) { @@ -1320,10 +1322,14 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset, write_buf = (uint8_t *)data; write_len = buf_len + sizeof(VmdkGrainMarker); } -ret = bdrv_pwrite(extent-file, -cluster_offset + offset_in_cluster, -write_buf, -write_len); +write_offset = cluster_offset + offset_in_cluster, +ret = bdrv_pwrite(extent-file, write_offset, write_buf, write_len); + +write_end_sector = DIV_ROUND_UP(write_offset + write_len, BDRV_SECTOR_SIZE); + +extent-next_cluster_sector = MAX(extent-next_cluster_sector, + write_end_sector); + if (ret != write_len) { ret = ret 0 ? ret : -EIO; goto out; -- 1.9.3
Re: [Qemu-devel] [PATCH v6 15/17] target-s390x: Extend arch specific QMP command query-cpu-definitions
On Mon, Apr 27, 2015 at 04:53:29PM +0200, Michael Mueller wrote: [...] #ifndef CONFIG_USER_ONLY +static CpuDefinitionInfoList *qmp_query_cpu_definition_host(void) +{ +CpuDefinitionInfoList *host = NULL; +CpuDefinitionInfo *info; + +info = g_try_new0(CpuDefinitionInfo, 1); +if (!info) { +goto out; +} +info-name = g_strdup(host); + +host = g_try_new0(CpuDefinitionInfoList, 1); +if (!host) { +g_free(info-name); +g_free(info); +goto out; +} +host-value = info; +out: +return host; +} [...] CpuDefinitionInfoList *arch_query_cpu_definitions(bool has_machine, const char *machine, bool has_accel, AccelId accel, Error **errp) { -CpuDefinitionInfoList *entry; -CpuDefinitionInfo *info; +S390MachineProps mach; +GSList *classes; +uint64_t *mask = NULL; +CpuDefinitionInfoList *list = NULL; + +if (has_machine) { +mask = s390_fac_list_mask_by_machine(machine); +if (!mask) { +error_set(errp, QERR_INVALID_PARAMETER_VALUE, machine, + a valid machine type); +return NULL; +} +} I would like to understand better the meaning of runnable when machine is omitted. Is it really possible to tell if a CPU model is runnable if no machine/mask info is provided as input? If machine is omitted and the command returns runnable=true, does that mean the CPU model is runnable using any machine? Does it mean it is runnable using some of the available machines? If so, which ones? Does it mean something else? -info = g_malloc0(sizeof(*info)); -info-name = g_strdup(host); +memset(mach, 0, sizeof(mach)); +if (has_accel) { +switch (accel) { +case ACCEL_ID_KVM: +kvm_s390_get_machine_props(NULL, mach); +break; +default: +return qmp_query_cpu_definition_host(); This will return only a single element. I don't think that's correct. If machine or accel is omitted, I believe we should just omit the runnable field, but always return the full list of CPU models. +} +} -entry = g_malloc0(sizeof(*entry)); -entry-value = info; +s390_setup_cpu_classes(ACCEL_TEMP, mach, mask); + +classes = object_class_get_list(TYPE_S390_CPU, false); +classes = g_slist_sort(classes, s390_cpu_class_asc_order_compare); +g_slist_foreach(classes, qmp_query_cpu_definition_entry, list); +g_slist_free(classes); -return entry; +return list; } #endif [...] -- Eduardo
[Qemu-devel] [RFC PATCH 3/7] block: Add op blocker notifier list
BDS users can register a notifier and get notified about op blocker changes. Signed-off-by: Fam Zheng f...@redhat.com --- block.c | 20 include/block/block.h | 8 include/block/block_int.h | 3 +++ 3 files changed, 31 insertions(+) diff --git a/block.c b/block.c index 7904098..054ddb4 100644 --- a/block.c +++ b/block.c @@ -3375,6 +3375,12 @@ struct BdrvOpBlocker { QLIST_ENTRY(BdrvOpBlocker) list; }; +void bdrv_op_blocker_add_notifier(BlockDriverState *bs, + Notifier *notifier) +{ +notifier_list_add(bs-op_blocker_notifiers, notifier); +} + bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp) { BdrvOpBlocker *blocker; @@ -3391,11 +3397,24 @@ bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp) return false; } +static void bdrv_op_blocker_notify(BlockDriverState *bs, BlockOpType op, + Error *reason, bool blocking) +{ +BlockOpEvent event = (BlockOpEvent) { +op = op, +reason = reason, +blocking = true, +}; + +notifier_list_notify(bs-op_blocker_notifiers, event); +} + void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Error *reason) { BdrvOpBlocker *blocker; assert((int) op = 0 op BLOCK_OP_TYPE_MAX); +bdrv_op_blocker_notify(bs, op, reason, true); blocker = g_new0(BdrvOpBlocker, 1); blocker-reason = reason; QLIST_INSERT_HEAD(bs-op_blockers[op], blocker, list); @@ -3405,6 +3424,7 @@ void bdrv_op_unblock(BlockDriverState *bs, BlockOpType op, Error *reason) { BdrvOpBlocker *blocker, *next; assert((int) op = 0 op BLOCK_OP_TYPE_MAX); +bdrv_op_blocker_notify(bs, op, reason, false); QLIST_FOREACH_SAFE(blocker, bs-op_blockers[op], list, next) { if (blocker-reason == reason) { QLIST_REMOVE(blocker, list); diff --git a/include/block/block.h b/include/block/block.h index 906fb31..3420b2c 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -163,6 +163,12 @@ typedef enum BlockOpType { BLOCK_OP_TYPE_MAX, } BlockOpType; +typedef struct { +BlockOpType type; +Error *reason; +bool blocking; +} BlockOpEvent; + void bdrv_iostatus_enable(BlockDriverState *bs); void bdrv_iostatus_reset(BlockDriverState *bs); void bdrv_iostatus_disable(BlockDriverState *bs); @@ -491,6 +497,8 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs); void bdrv_ref(BlockDriverState *bs); void bdrv_unref(BlockDriverState *bs); +void bdrv_op_blocker_add_notifier(BlockDriverState *bs, + Notifier *notifier); bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp); void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Error *reason); void bdrv_op_unblock(BlockDriverState *bs, BlockOpType op, Error *reason); diff --git a/include/block/block_int.h b/include/block/block_int.h index db29b74..195ae30 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -418,6 +418,9 @@ struct BlockDriverState { /* operation blockers */ QLIST_HEAD(, BdrvOpBlocker) op_blockers[BLOCK_OP_TYPE_MAX]; +/* Callback before any op blocker change */ +NotifierList op_blocker_notifiers; + /* long-running background operation */ BlockJob *job; -- 1.9.3
Re: [Qemu-devel] [PATCH v1 RFC 34/34] char: introduce support for TLS encrypted TCP chardev backend
On Wed, May 06, 2015 at 11:18:23AM +0100, Daniel P. Berrange wrote: On Wed, May 06, 2015 at 10:34:06AM +0200, Kashyap Chamarthy wrote: On Tue, May 05, 2015 at 04:54:44PM +0200, Kashyap Chamarthy wrote: [. . .] While running QEMU as TLS server, the TLS handshake completes successfully when connected via `gnutls-cli`. However, when using QEMU as client to connect to an existing GnuTLS server, I notice a segmentation fault: $ /home/kashyapc/build/tls-qemu/x86_64-softmmu/qemu-system-x86_64 \ -nodefconfig -nodefaults -device sga -display none \ -chardev socket,id=s0,host=localhost,port=9000,tls-cred=tls0 \ -device isa-serial,chardev=s0 \ -object qcrypto-tls-creds,id=tls0,credtype=x509,endpoint=client,dir=/export/security/gnutls Segmentation fault (core dumped) Some debugging with `gdb` below. QEMU was built with: ./configure --target-list=x86_64-softmmu --enable-debug make -j4 Stack traces: $ gdb /home/kashyapc/build/tls-qemu/x86_64-softmmu/qemu-system-x86_64 #2 0x71c6b3d9 in gnutls_certificate_set_x509_key_file2 () at /lib64/libgnutls.so.28 #3 0x559aba85 in qcrypto_tls_creds_load_x509 (creds=0x5639ac60, errp=0x7fffd8d8) at /home/kashyapc/tinker-space/qemu/crypto/tlscreds.c:728 cacert = 0x5639a8c0 /export/security/gnutls/ca-cert.pem cacrl = 0x0 cert = 0x0 key = 0x0 dhparams = 0x0 ret = 1 rv = -1 Ah, with QEMU running in client mode, the client cert + key are optional. In this case you've not provided them (cert key are 0x0 ie NULL). Yep, I generated a client-key.pem, client-cert.pem and placed them in the same gnutls directory where the ca* and server* files are located. Indeed the TLS handshake completes succesfully (tested with the same QEMU CLI as above placed in a script): $ ./chardev-backend-qemu-as-tls-client.sh Handshake still running 2 Handshake still running 2 Handshake compelte session=0x7f8bdda6f4c0 On the GnuTLS server: [. . .] * Accepted connection from IPv4 127.0.0.1 port 51353 on Wed May 6 13:19:10 2015 - Description: (TLS1.2)-(ECDHE-RSA-SECP256R1)-(AES-128-CBC)-(SHA1) [. . .] - Server has requested a certificate. - Certificate type: X.509 - Got a certificate list of 1 certificates. [. . .] - Version: TLS1.2 - Key Exchange: ECDHE-RSA - Server Signature: RSA-SHA256 - Client Signature: RSA-SHA256 - Compression: NULL - Options: safe renegotiation, - Channel binding 'tls-unique': 7f4ae1e0dc02dbad602a9c27 We are then mistakenly calling gnutls_certificate_set_x509_key_file2 - if I simply skip that I'll avoid the crash. Thanks for testing this - I'll add a test case to validate this scenario too -- /kashyap
Re: [Qemu-devel] [PULL] Queued tcg patch
On 6 May 2015 at 00:24, Richard Henderson r...@twiddle.net wrote: Only one tcg related patch since the 2.3 freeze. r~ The following changes since commit 874e9aeeeb74c5459639a93439a502d262847e68: Merge remote-tracking branch 'remotes/kraxel/tags/pull-sdl-20150505-1' into staging (2015-05-05 14:06:12 +0100) are available in the git repository at: git://github.com/rth7680/qemu.git tags/tcg-next-20150505 for you to fetch changes up to 00c8fa9ffeee7458e5ed62c962faf638156c18da: tcg: optimise memory layout of TCGTemp (2015-05-05 08:44:46 -0700) size reduction merge Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH target-arm v6 00/14] Next Generation Xilinx Zynq SoC
On 1 May 2015 at 18:25, Peter Crosthwaite peter.crosthwa...@xilinx.com wrote: Ping! On Fri, Apr 24, 2015 at 1:28 PM, Peter Crosthwaite peter.crosthwa...@xilinx.com wrote: Hi Peter and all, Xilinx's next gen SoC has been announced. This series adds a SoC and board. Neither patchwork nor patches seem to have the complete set of these patch emails -- can you try resending, please? thanks -- PMM
Re: [Qemu-devel] [PATCH v6 04/17] Extend HMP command info cpus to display accelerator id and model name
On Wed, May 06, 2015 at 09:32:58AM +0200, Michael Mueller wrote: On Tue, 5 May 2015 10:14:32 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Mon, Apr 27, 2015 at 04:53:18PM +0200, Michael Mueller wrote: The HMP command info cpus now displays the CPU model name and the backing accelerator if part of the CPUState. (qemu) info cpus * CPU #0: (halted) model=2827-ga2 accel=kvm thread_id=1679 Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com Acked-by: Christian Borntraeger borntrae...@de.ibm.com Do we really need this? I mean: I expect the amount of CPU data we provide to QMP clients to grow a lot in the near future, but that doesn't mean HMP users need all that data to be printed by info cpus. Where do you see the limit of what is worth to be shown an what not. I personally use info cpus less then sporadic but already got a comment internally on that information being worthwhile to be shown. I really don't know, but I think we shouldn't add stuff to HMP unless we have a good reason. For each new piece of data in HMP I would like to at least see the description of a real use case that justifies adding it to HMP and not just implementing a simple script on top of QMP. For accel info we already have info kvm that is not ideal but is enough for current use cases, isn't it? CPU model name information seems to be more useful, but if it is just for debugging, people can just run QMP query-cpus command. Luiz, what do you think? --- hmp.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/hmp.c b/hmp.c index f142d36..676d821 100644 --- a/hmp.c +++ b/hmp.c @@ -290,6 +290,13 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict) monitor_printf(mon, (halted)); } +if (cpu-value-has_model) { +monitor_printf(mon, model=%s, cpu-value-model); +} +if (cpu-value-has_accel) { +monitor_printf(mon, accel=%s, AccelId_lookup[cpu-value-accel]); +} + monitor_printf(mon, thread_id=% PRId64 \n, cpu-value-thread_id); } -- 1.8.3.1 -- Eduardo
Re: [Qemu-devel] [PATCH v6 01/17] Introduce stub routine cpu_desc_avail
On Wed, May 06, 2015 at 11:17:20AM +0200, Michael Mueller wrote: On Tue, 5 May 2015 14:41:01 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Tue, May 05, 2015 at 06:12:16PM +0200, Michael Mueller wrote: On Tue, 5 May 2015 10:55:47 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Mon, Apr 27, 2015 at 04:53:15PM +0200, Michael Mueller wrote: This patch introduces the function cpu_desc_avail() which returns by default true if not architecture specific implemented. Its intention is to indicate if the cpu model description is available for display by list_cpus(). This change allows cpu model descriptions to become dynamically created by evaluating the runtime context instead of putting static cpu model information at display. Why are you deliberately breaking -cpu ? when cpu_desc_avail() is false? In the s390x case cpu_desc_avail() is per se false in this code section of vl.c: /* Init CPU def lists, based on config * - Must be called after all the qemu_read_config_file() calls * - Must be called before list_cpu() * - Must be called before machine-init() */ (Side note: I believe the above outdated, I will send a patch to update it.) Will be interesting to see what the change is, master is currently showing this code. We don't (and shouldn't) have the qemu_read_config_file() requirement, as CPU models are not loaded from config files anymore. And we should be able to eliminate cpudef_init() completely, soon. I think the only user of cpudef_init() is x86. cpudef_init(); if (cpu_model cpu_desc_avail() is_help_option(cpu_model)) { list_cpus(stdout, fprintf, cpu_model); exit(0); } That is because the output does not solely depend on static definitions but also on runtime context. Here the host machine type this instance of QEMU is running on, at least for the KVM case. Is this a required feature? I would prefer to have the main() code simple even if it means not having runnable information in -cpu ? by now (about possible ways to implement this without cpu_desc_avail(), see below). I think it is more than a desired feature because one might end up with a failed CPU object instantiation although the help screen claims to CPU model to be valid. I think you are more likely to confuse users by not showing information on -cpu ? when -machine is not present. I believe most people use -cpu ? with no other arguments, to see what the QEMU binary is capable of. Anyway, whatever we decide to do, I believe we should start with something simple to get things working, and after that we can look for ways improve the help output with runnable info. Once the accelerator has been initialized AND the S390 cpu classes have been setup by means of the following code: static void kvm_setup_cpu_classes(KVMState *s) { S390MachineProps mach; if (!kvm_s390_get_machine_props(s, mach)) { s390_setup_cpu_classes(ACCEL_CURRENT, mach, s390_current_fac_list_mask()); s390_setup_cpu_aliases(); cpu_classes_initialized = true; } } cpu_desc_avail() becomes true. In case the selceted mode was ? the list_cpu() is now done right before the cpu model is used as part of the cpu initialization (hw/s390-virtio.c): void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys) { int i; if (cpu_model == NULL) { cpu_model = none; } if (is_help_option(cpu_model)) { list_cpus(stdout, fprintf, cpu_model); exit(0); } ... for (i = 0; i smp_cpus; i++) { ... cpu = cpu_s390x_init(cpu_model); ... } } In other words, you just need to ensure that s390_cpu_list() run after kvm_setup_cpu_classes(). ... which is part of the KVM/accel init process but it could of course make use of the ACCEL_TMP use case as query-cpu-definitions does. Exactly. I don't see a reason to not share code between query-cpu-definitions and -cpu ?. Can't you simply call s390_setup_cpu_classes(ACCEL_TEMP) inside s390_init_cpus(), just like arch_query_cpu_definitions()? You could even share code between both functions. That would not help with the current placement of list_cpus() in main() as it happens way to early. Not s390_init_cpus() is the issue, the context information has been processed already at that time. Currently I just kind of delay the list_cpus() until all required information is available. I understand you are just delaying it. But it is requiring a hack inside generic main() code that could be avoided. About the actual reasons to delay it, see below. (In the future, we should be able to implement -cpu ? by simply calling the
[Qemu-devel] [RFC PATCH 1/7] block: Add op blocker type device IO
Preventing device from submitting IO is useful around various nested poll. Op blocker is a good place to put this flag. Devices would submit IO requests through blk_* block backend interface, which calls blk_check_request to check the validity. Return -EBUSY if the operation is blocked, in which case device IO is temporarily disabled by another BDS user. Signed-off-by: Fam Zheng f...@redhat.com --- block/block-backend.c | 4 include/block/block.h | 1 + 2 files changed, 5 insertions(+) diff --git a/block/block-backend.c b/block/block-backend.c index 93e46f3..71fc695 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -478,6 +478,10 @@ static int blk_check_request(BlockBackend *blk, int64_t sector_num, return -EIO; } +if (bdrv_op_is_blocked(blk-bs, BLOCK_OP_TYPE_DEVICE_IO, NULL)) { +return -EBUSY; +} + return blk_check_byte_request(blk, sector_num * BDRV_SECTOR_SIZE, nb_sectors * BDRV_SECTOR_SIZE); } diff --git a/include/block/block.h b/include/block/block.h index 7d1a717..906fb31 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -159,6 +159,7 @@ typedef enum BlockOpType { BLOCK_OP_TYPE_RESIZE, BLOCK_OP_TYPE_STREAM, BLOCK_OP_TYPE_REPLACE, +BLOCK_OP_TYPE_DEVICE_IO, BLOCK_OP_TYPE_MAX, } BlockOpType; -- 1.9.3
Re: [Qemu-devel] [PATCH 4/4] qemu-ga: Building Windows MSI installation with configure/Makefile
On 06/05/2015 13:53, Yossi Hindin wrote: +${QEMU_GA_MSI}: config-host.mak + +${QEMU_GA_MSI}: qga/installer/qemu-ga.wxs + $(call quiet-command,QEMU_GA_VERSION=$(QEMU_GA_VERSION) QEMU_GA_MANUFACTURER=$(QEMU_GA_MANUFACTURER) QEMU_GA_DISTRO=$(QEMU_GA_DISTRO) \ + wixl -o $@ ${QEMU_GA_MSI_ARCH} ${QEMU_GA_MSI_WITH_VSS} ${QEMU_GA_MSI_MINGW_DLL_PATH} $, WIXL $@) Please use $(...) instead of ${...} for consistency. Fixed. BTW. there are other places in the 'configure' script where ${} is used. Do you mean in the Makefile (or the generated .mak files)? The configure script is a bash script, so it uses ${...} to access variables. The Makefile can use either ${...} or $(...), but the parentheses are preferred over the braces. Thanks, Paolo
Re: [Qemu-devel] [RFC PATCH 6/7] virtio-blk: Don't handle output when there is device IO op blocker
On 06/05/2015 13:23, Fam Zheng wrote: virtio-blk now listens to op blocker change of the associated block backend. Up on setting op blocker on BLOCK_OP_TYPE_DEVICE_IO: non-dataplane: 1) Set VirtIOBlock.paused 2) In virtio_blk_handle_output, do nothing if VirtIOBlock.paused dataplane: 1) Clear the host event notifier 2) In handle_notify, do nothing if VirtIOBlock.paused Up on removing the op blocker: non-dataplane: 1) Clear VirtIOBlock.paused 2) Schedule a BH on the AioContext of the backend, which calls virtio_blk_handle_output, so that the previous unhandled kicks can make progress dataplane: 1) Set the host event notifier 2) Notify the host event notifier so that unhandled events are processed Signed-off-by: Fam Zheng f...@redhat.com Does non-dataplane need to do anything, since it uses iohandlers rather than aio_set_event_notifier_handler? Paolo
[Qemu-devel] [PATCH RFC 0/7] vhost: cross-endian support (vhost-net only)
Hi, This series allows QEMU to use vhost with legacy virtio devices when host and target don't have the same endianness. Only network devices are covered for the moment. I had already posted a series some monthes ago but it never got reviewed. Moreover, the underlying kernel support was entirely re-written and is still waiting to be applied by Michael. I hence post as RFC. The corresponding kernel patches are available here: http://lists.linuxfoundation.org/pipermail/virtualization/2015-April/029885.html Please comment. --- Cédric Le Goater (1): vhost_net: re-enable when cross endian Greg Kurz (6): virtio: relax feature check linux-headers: sync vhost.h virtio: introduce virtio_legacy_is_cross_endian() vhost: set vring endianness for legacy virtio tap: add VNET_LE/VNET_BE operations vhost-net: tell tap backend about the vnet endianness hw/net/vhost_net.c| 50 +++-- hw/virtio/vhost.c | 50 - include/hw/virtio/virtio-access.h | 13 ++ include/hw/virtio/virtio.h|1 - include/net/net.h |6 linux-headers/linux/vhost.h | 14 ++ net/net.c | 18 + net/tap-linux.c | 34 + net/tap-linux.h |2 + net/tap.c | 16 net/tap_int.h |2 + 11 files changed, 185 insertions(+), 21 deletions(-) -- Greg
[Qemu-devel] [PATCH RFC 5/7] tap: add VNET_LE/VNET_BE operations
The linux tap and macvtap backends can be told to parse vnet headers according to little or big endian. This is done through the TUNSETVNETLE and TUNSETVNETBE ioctls. This patch brings all the plumbing for QEMU to use these APIs. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- include/net/net.h |6 ++ net/net.c | 18 ++ net/tap-linux.c | 34 ++ net/tap-linux.h |2 ++ net/tap.c | 16 net/tap_int.h |2 ++ 6 files changed, 78 insertions(+) diff --git a/include/net/net.h b/include/net/net.h index 50ffcb9..86f57f7 100644 --- a/include/net/net.h +++ b/include/net/net.h @@ -55,6 +55,8 @@ typedef bool (HasVnetHdrLen)(NetClientState *, int); typedef void (UsingVnetHdr)(NetClientState *, bool); typedef void (SetOffload)(NetClientState *, int, int, int, int, int); typedef void (SetVnetHdrLen)(NetClientState *, int); +typedef int (SetVnetLE)(NetClientState *, bool); +typedef int (SetVnetBE)(NetClientState *, bool); typedef struct NetClientInfo { NetClientOptionsKind type; @@ -73,6 +75,8 @@ typedef struct NetClientInfo { UsingVnetHdr *using_vnet_hdr; SetOffload *set_offload; SetVnetHdrLen *set_vnet_hdr_len; +SetVnetLE *set_vnet_le; +SetVnetBE *set_vnet_be; } NetClientInfo; struct NetClientState { @@ -138,6 +142,8 @@ void qemu_using_vnet_hdr(NetClientState *nc, bool enable); void qemu_set_offload(NetClientState *nc, int csum, int tso4, int tso6, int ecn, int ufo); void qemu_set_vnet_hdr_len(NetClientState *nc, int len); +int qemu_set_vnet_le(NetClientState *nc, bool is_le); +int qemu_set_vnet_be(NetClientState *nc, bool is_be); void qemu_macaddr_default_if_unset(MACAddr *macaddr); int qemu_show_nic_models(const char *arg, const char *const *models); void qemu_check_nic_model(NICInfo *nd, const char *model); diff --git a/net/net.c b/net/net.c index 0be084d..eb8ef3e 100644 --- a/net/net.c +++ b/net/net.c @@ -452,6 +452,24 @@ void qemu_set_vnet_hdr_len(NetClientState *nc, int len) nc-info-set_vnet_hdr_len(nc, len); } +int qemu_set_vnet_le(NetClientState *nc, bool is_le) +{ +if (!nc || !nc-info-set_vnet_le) { +return -ENOSYS; +} + +return nc-info-set_vnet_le(nc, is_le); +} + +int qemu_set_vnet_be(NetClientState *nc, bool is_be) +{ +if (!nc || !nc-info-set_vnet_be) { +return -ENOSYS; +} + +return nc-info-set_vnet_be(nc, is_be); +} + int qemu_can_send_packet(NetClientState *sender) { int vm_running = runstate_is_running(); diff --git a/net/tap-linux.c b/net/tap-linux.c index 812bf2d..15b57a7 100644 --- a/net/tap-linux.c +++ b/net/tap-linux.c @@ -198,6 +198,40 @@ void tap_fd_set_vnet_hdr_len(int fd, int len) } } +int tap_fd_set_vnet_le(int fd, int is_le) +{ +int arg = is_le ? 1 : 0; + +if (!ioctl(fd, TUNSETVNETLE, arg)) { +return 0; +} + +/* Check if our kernel supports TUNSETVNETLE */ +if (errno == EINVAL) { +return -errno; +} + +error_report(TUNSETVNETLE ioctl() failed: %s.\n, strerror(errno)); +abort(); +} + +int tap_fd_set_vnet_be(int fd, int is_be) +{ +int arg = is_be ? 1 : 0; + +if (!ioctl(fd, TUNSETVNETBE, arg)) { +return 0; +} + +/* Check if our kernel supports TUNSETVNETBE */ +if (errno == EINVAL) { +return -errno; +} + +error_report(TUNSETVNETBE ioctl() failed: %s.\n, strerror(errno)); +abort(); +} + void tap_fd_set_offload(int fd, int csum, int tso4, int tso6, int ecn, int ufo) { diff --git a/net/tap-linux.h b/net/tap-linux.h index 1cf35d4..01dc6f8 100644 --- a/net/tap-linux.h +++ b/net/tap-linux.h @@ -30,6 +30,8 @@ #define TUNGETVNETHDRSZ _IOR('T', 215, int) #define TUNSETVNETHDRSZ _IOW('T', 216, int) #define TUNSETQUEUE _IOW('T', 217, int) +#define TUNSETVNETLE _IOW('T', 220, int) +#define TUNSETVNETBE _IOW('T', 222, int) #endif diff --git a/net/tap.c b/net/tap.c index 968df46..c6f9a7d 100644 --- a/net/tap.c +++ b/net/tap.c @@ -274,6 +274,20 @@ static void tap_using_vnet_hdr(NetClientState *nc, bool using_vnet_hdr) s-using_vnet_hdr = using_vnet_hdr; } +static int tap_set_vnet_le(NetClientState *nc, bool is_le) +{ +TAPState *s = DO_UPCAST(TAPState, nc, nc); + +return tap_fd_set_vnet_le(s-fd, is_le); +} + +static int tap_set_vnet_be(NetClientState *nc, bool is_be) +{ +TAPState *s = DO_UPCAST(TAPState, nc, nc); + +return tap_fd_set_vnet_be(s-fd, is_be); +} + static void tap_set_offload(NetClientState *nc, int csum, int tso4, int tso6, int ecn, int ufo) { @@ -335,6 +349,8 @@ static NetClientInfo net_tap_info = { .using_vnet_hdr = tap_using_vnet_hdr, .set_offload = tap_set_offload, .set_vnet_hdr_len = tap_set_vnet_hdr_len, +.set_vnet_le = tap_set_vnet_le, +.set_vnet_be = tap_set_vnet_be, }; static TAPState *net_tap_fd_init(NetClientState *peer, diff --git
[Qemu-devel] [PATCH RFC 1/7] virtio: relax feature check
Unlike with add and clear, there is no valid reason to abort when checking for a feature. It makes more sense to return false (i.e. the feature bit isn't set). This is exactly what __virtio_has_feature() does if fbit = 32. This allows to introduce code that is aware about new 64-bit features like VIRTIO_F_VERSION_1, even if they are still not implemented. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- include/hw/virtio/virtio.h |1 - 1 file changed, 1 deletion(-) diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index d95f8b6..6ef70f1 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -233,7 +233,6 @@ static inline void virtio_clear_feature(uint32_t *features, unsigned int fbit) static inline bool __virtio_has_feature(uint32_t features, unsigned int fbit) { -assert(fbit 32); return !!(features (1 fbit)); }
Re: [Qemu-devel] [RFC PATCH 6/7] virtio-blk: Don't handle output when there is device IO op blocker
On Wed, 05/06 14:07, Paolo Bonzini wrote: On 06/05/2015 13:23, Fam Zheng wrote: virtio-blk now listens to op blocker change of the associated block backend. Up on setting op blocker on BLOCK_OP_TYPE_DEVICE_IO: non-dataplane: 1) Set VirtIOBlock.paused 2) In virtio_blk_handle_output, do nothing if VirtIOBlock.paused dataplane: 1) Clear the host event notifier 2) In handle_notify, do nothing if VirtIOBlock.paused Up on removing the op blocker: non-dataplane: 1) Clear VirtIOBlock.paused 2) Schedule a BH on the AioContext of the backend, which calls virtio_blk_handle_output, so that the previous unhandled kicks can make progress dataplane: 1) Set the host event notifier 2) Notify the host event notifier so that unhandled events are processed Signed-off-by: Fam Zheng f...@redhat.com Does non-dataplane need to do anything, since it uses iohandlers rather than aio_set_event_notifier_handler? I guess it's not for this specific bug. See this as an attempt on a general purpose pause mechanism to the device in investment for the future, for example, bdrv_aio_drain. ;-) I can drop it in v2 if you think the idea is not very helpful. Fam
Re: [Qemu-devel] [PATCH v6 12/17] target-s390x: Add S390 CPU class initialization routines
On Wed, May 06, 2015 at 10:02:22AM +0200, Michael Mueller wrote: On Tue, 5 May 2015 11:34:06 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Mon, Apr 27, 2015 at 04:53:26PM +0200, Michael Mueller wrote: This patch provides routines to dynamically update the previously defined S390 CPU classes in the current host context. The main function performing this process is s390_setup_cpu_classes(). It takes the current host context and a facility list mask as parameter to setup the classes accordingly. It basically performs the following sub-tasks: - Update of CPU classes with accelerator specific host and QEMU properties - Mark adequate CPU class as default CPU class to be used for CPU model 'host' - Invalidate CPU classes not supported by this hosting machine - Define machine type aliases to latest GA number of a processor model - Define aliases for common CPU model names - Set CPU model alias 'host' to default CPU class Forthermore the patch provides the following routines: - cpu_desc_avail(), s390 specific stub indicating that list_cpus() can run - s390_setup_cpu_aliases(), adds cu model aliases - s390_cpu_classes_initialized(), test if CPU classes have been initialized - s390_fac_list_mask_by_machine(), returns facility list mask by machine - s390_current_fac_list_mask(), returns facility list mask of current machine Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com Acked-by: Christian Borntraeger borntrae...@de.ibm.com --- [...] +/** + * s390_setup_cpu_classes: + * @mode: the accelerator mode + * @prop: the machine property structure's address + * + * This function validates the defined cpu classes against the given + * machine properties @prop. Only cpu classes that are runnable on the + * current host will be set active. In addition the corresponding + * cpuid, ibc value and the active set of facilities will be + * initialized. Depending on @mode, the function porforms operations + * on the current or the temporary accelerator properies. + * + * Since: 2.4 + */ +void s390_setup_cpu_classes(S390AccelMode mode, S390MachineProps *prop, +uint64_t *fac_list_mask) +{ Can't you replace the S390AccelMode arguments everywhere with simply an AccelState pointer? That's the kind of thing that should have been easier to implement using the accel QOM stuff. Would just make sense in conjunction with an AccelId indexed array in the CPU class but see my concerns below. If you still need to save accel-specific data somewhere (like the is_active, is_host and fac_list arrays), maybe it can be indexed using the AccelId enum you have introduced, instead of S390AccelMode? I had an AccelId indexed array in a previous version of the patch but dismissed it in favor to this AccelMode index approach for the following reasons: a) There is just one accelerator active and and a second set of values is used for the query-cpu-definitions case. Using the AcceldId index would instantly double the required memory being used for no reason. The size of the second dimension in uint64_t fac_list[ACCEL_MODE_MAX][FAC_LIST_CPU_S390_SIZE_UINT64]; is architecturally allowed to grow up to 2KB. b) The information stored has more dimensions than just the accelerator, it also contains the selected machine (s390-virtio) which is represented by means of qemu_s390_fac_list_mask[] which currently is identical for all machines but that will change as the implementation progresses. So AccelMode (current, tmp) might also not fully express the semantics. Right. So the data depends on (cpu_model, accel, machine), but we need to cache the results for very few (just 2) accel+machine combinations at any given moment. AccelMode makes more sense to me, now. -- Eduardo
Re: [Qemu-devel] [PATCH v6 05/17] Add optional parameters to QMP command query-cpu-definitions
On Mon, Apr 27, 2015 at 04:53:19PM +0200, Michael Mueller wrote: [...] diff --git a/qapi-schema.json b/qapi-schema.json index 215a7bc..285b2d3 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2536,21 +2536,43 @@ # # @name: the name of the CPU definition # +# @default: #optional true if cpu model is the default, +# omitted if false (since 2.4) Maybe we should clarify that it is the default in the machine provided as argument to query-cpu-definitions? +# +# @runnable: #optional true if cpu model is runnable, +#omitted if false (since 2.4) Maybe we should clarify that it means the CPU model is runnable using the machine+accel combination provided as arguments to query-cpu-definitions? (See also my question about the meaning of runnable when machine is omitted, in my reply to patch 15/17). +# +# @live-migration-safe: #optional true if cpu model represents a +# cpu model that is safely migratable +# omitted if false (since 2.4) +# +# @order: #optional order criterion +# # Since: 1.2.0 ## { 'type': 'CpuDefinitionInfo', - 'data': { 'name': 'str' } } + 'data': { 'name': 'str', '*is-default': 'bool', '*runnable': 'bool', +'*live-migration-safe': 'bool', '*order': 'int' } } ## # @query-cpu-definitions: # -# Return a list of supported virtual CPU definitions +# Return a list of supported virtual CPU definitions. In context with the +# optional parameters @machine and @accel the returned list contains +# also information if the respective cpu definition is runnable or the +# default to be used. +# +# @machine: #optional machine type (since 2.4) +# +# @accel: #optional accelerator id (since 2.4) # # Returns: a list of CpuDefInfo # # Since: 1.2.0 ## -{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] } +{ 'command': 'query-cpu-definitions', + 'data': { '*machine': 'str', '*accel': 'AccelId' }, + 'returns': ['CpuDefinitionInfo'] } -- Eduardo
[Qemu-devel] [RFC PATCH 0/7] Fix transactional snapshot with virtio-blk dataplane
Reported by Paolo. Unlike the iohandler in main loop, iothreads currently process the event notifier used as virtio-blk ioeventfd in all nested aio_poll. This is dangerous without proper protection, because guest requests could sneak to block layer where they mustn't. For example, a QMP transaction may involve multiple bdrv_drain_all() in handling the list of AioContext it works on. If an aio_poll in one of the bdrv_drain_all() happens to process a guest VQ kick by dispatching the ioeventfd event, a new guest write is then submitted, and voila, the transaction semantics is violated. This series avoids this problem by disabling virtio-blk handlers during bdrv_drain_all() and transactions. Notes: If the general approach is right, other transaction types could get the blockers similarly, in next revision. And some related bdrv_drain_all() could also be changed to bdrv_drain(). virtio-scsi-dataplane will be a bit more complicated, but still doable. It would probably need one more interface abstraction between scsi-disk, scsi-bus and virtio-scsi. Although other devices don't have a pause/resume callback yet, the blk_check_request, which returns -EBUSY if device io op blocker is set, could hopefully cover most cases already. Timers and block jobs also generate IO, but it should be fine as long as they don't change guest visible data, which is true AFAICT. Fam Zheng (7): block: Add op blocker type device IO block: Block device IO during bdrv_drain and bdrv_drain_all block: Add op blocker notifier list block-backend: Add blk_op_blocker_add_notifier virtio-blk: Move complete_request to 'ops' structure virtio-blk: Don't handle output when there is device IO op blocker blockdev: Add device IO op blocker during snapshot transaction block.c | 20 block/block-backend.c | 10 ++ block/io.c | 12 +++ blockdev.c | 7 + hw/block/dataplane/virtio-blk.c | 36 ++--- hw/block/virtio-blk.c | 69 +++-- include/block/block.h | 9 ++ include/block/block_int.h | 3 ++ include/hw/virtio/virtio-blk.h | 17 -- include/sysemu/block-backend.h | 2 ++ 10 files changed, 174 insertions(+), 11 deletions(-) -- 1.9.3
[Qemu-devel] [RFC PATCH 2/7] block: Block device IO during bdrv_drain and bdrv_drain_all
We don't want new requests from guest, so block the operation around the nested poll. Signed-off-by: Fam Zheng f...@redhat.com --- block/io.c | 12 1 file changed, 12 insertions(+) diff --git a/block/io.c b/block/io.c index 1ce62c4..d369de3 100644 --- a/block/io.c +++ b/block/io.c @@ -289,9 +289,15 @@ static bool bdrv_drain_one(BlockDriverState *bs) */ void bdrv_drain(BlockDriverState *bs) { +Error *blocker = NULL; + +error_setg(blocker, bdrv_drain in progress); +bdrv_op_block(bs, BLOCK_OP_TYPE_DEVICE_IO, blocker); while (bdrv_drain_one(bs)) { /* Keep iterating */ } +bdrv_op_unblock(bs, BLOCK_OP_TYPE_DEVICE_IO, blocker); +error_free(blocker); } /* @@ -311,6 +317,9 @@ void bdrv_drain_all(void) /* Always run first iteration so any pending completion BHs run */ bool busy = true; BlockDriverState *bs = NULL; +Error *blocker = NULL; + +error_setg(blocker, bdrv_drain_all in progress); while ((bs = bdrv_next(bs))) { AioContext *aio_context = bdrv_get_aio_context(bs); @@ -319,6 +328,7 @@ void bdrv_drain_all(void) if (bs-job) { block_job_pause(bs-job); } +bdrv_op_block(bs, BLOCK_OP_TYPE_DEVICE_IO, blocker); aio_context_release(aio_context); } @@ -343,8 +353,10 @@ void bdrv_drain_all(void) if (bs-job) { block_job_resume(bs-job); } +bdrv_op_unblock(bs, BLOCK_OP_TYPE_DEVICE_IO, blocker); aio_context_release(aio_context); } +error_free(blocker); } /** -- 1.9.3
[Qemu-devel] [RFC PATCH 4/7] block-backend: Add blk_op_blocker_add_notifier
Forward the call to bdrv_op_blocker_add_notifier. Signed-off-by: Fam Zheng f...@redhat.com --- block.c| 4 ++-- block/block-backend.c | 6 ++ include/sysemu/block-backend.h | 2 ++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c index 054ddb4..d98a304 100644 --- a/block.c +++ b/block.c @@ -3414,23 +3414,23 @@ void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Error *reason) BdrvOpBlocker *blocker; assert((int) op = 0 op BLOCK_OP_TYPE_MAX); -bdrv_op_blocker_notify(bs, op, reason, true); blocker = g_new0(BdrvOpBlocker, 1); blocker-reason = reason; QLIST_INSERT_HEAD(bs-op_blockers[op], blocker, list); +bdrv_op_blocker_notify(bs, op, reason, true); } void bdrv_op_unblock(BlockDriverState *bs, BlockOpType op, Error *reason) { BdrvOpBlocker *blocker, *next; assert((int) op = 0 op BLOCK_OP_TYPE_MAX); -bdrv_op_blocker_notify(bs, op, reason, false); QLIST_FOREACH_SAFE(blocker, bs-op_blockers[op], list, next) { if (blocker-reason == reason) { QLIST_REMOVE(blocker, list); g_free(blocker); } } +bdrv_op_blocker_notify(bs, op, reason, false); } void bdrv_op_block_all(BlockDriverState *bs, Error *reason) diff --git a/block/block-backend.c b/block/block-backend.c index 71fc695..90d7476 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -806,6 +806,12 @@ void blk_op_unblock_all(BlockBackend *blk, Error *reason) bdrv_op_unblock_all(blk-bs, reason); } +void blk_op_blocker_add_notifier(BlockBackend *blk, + Notifier *notifier) +{ +bdrv_op_blocker_add_notifier(blk-bs, notifier); +} + AioContext *blk_get_aio_context(BlockBackend *blk) { return bdrv_get_aio_context(blk-bs); diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index b4a4d5e..cde9651 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -136,6 +136,8 @@ int blk_get_flags(BlockBackend *blk); int blk_get_max_transfer_length(BlockBackend *blk); void blk_set_guest_block_size(BlockBackend *blk, int align); void *blk_blockalign(BlockBackend *blk, size_t size); +void blk_op_blocker_add_notifier(BlockBackend *blk, + Notifier *notifier); bool blk_op_is_blocked(BlockBackend *blk, BlockOpType op, Error **errp); void blk_op_unblock(BlockBackend *blk, BlockOpType op, Error *reason); void blk_op_block_all(BlockBackend *blk, Error *reason); -- 1.9.3
[Qemu-devel] [RFC PATCH 6/7] virtio-blk: Don't handle output when there is device IO op blocker
virtio-blk now listens to op blocker change of the associated block backend. Up on setting op blocker on BLOCK_OP_TYPE_DEVICE_IO: non-dataplane: 1) Set VirtIOBlock.paused 2) In virtio_blk_handle_output, do nothing if VirtIOBlock.paused dataplane: 1) Clear the host event notifier 2) In handle_notify, do nothing if VirtIOBlock.paused Up on removing the op blocker: non-dataplane: 1) Clear VirtIOBlock.paused 2) Schedule a BH on the AioContext of the backend, which calls virtio_blk_handle_output, so that the previous unhandled kicks can make progress dataplane: 1) Set the host event notifier 2) Notify the host event notifier so that unhandled events are processed Signed-off-by: Fam Zheng f...@redhat.com --- hw/block/dataplane/virtio-blk.c | 25 +++- hw/block/virtio-blk.c | 63 +++-- include/hw/virtio/virtio-blk.h | 8 +- 3 files changed, 92 insertions(+), 4 deletions(-) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index ec0c8f4..d6c943c 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -87,8 +87,28 @@ static void complete_request_vring(VirtIOBlockReq *req, unsigned char status) qemu_bh_schedule(s-bh); } +static void virtio_blk_data_plane_pause(VirtIOBlock *vblk) +{ +VirtIOBlockDataPlane *s = vblk-dataplane; + +event_notifier_test_and_clear(s-host_notifier); +aio_set_event_notifier(s-ctx, s-host_notifier, NULL); +} + +static void handle_notify(EventNotifier *e); +static void virtio_blk_data_plane_resume(VirtIOBlock *vblk) +{ +VirtIOBlockDataPlane *s = vblk-dataplane; + +aio_set_event_notifier(s-ctx, s-host_notifier, handle_notify); + +event_notifier_set(s-host_notifier); +} + static const VirtIOBlockOps virtio_blk_data_plane_ops = { -.complete_request = complete_request_vring, +.complete_request = complete_request_vring, +.pause = virtio_blk_data_plane_pause, +.resume = virtio_blk_data_plane_resume, }; static void handle_notify(EventNotifier *e) @@ -98,6 +118,9 @@ static void handle_notify(EventNotifier *e) VirtIOBlock *vblk = VIRTIO_BLK(s-vdev); event_notifier_test_and_clear(s-host_notifier); +if (vblk-paused) { +return; +} blk_io_plug(s-conf-conf.blk); for (;;) { MultiReqBuffer mrb = {}; diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index f4a9d19..4bc1b2a 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -59,8 +59,38 @@ static void virtio_blk_complete_request(VirtIOBlockReq *req, virtio_notify(vdev, s-vq); } +typedef struct { +QEMUBH *bh; +VirtIOBlock *s; +} VirtIOBlockResumeData; + +static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq); +static void virtio_blk_resume_bh_cb(void *opaque) +{ +VirtIOBlockResumeData *data = opaque; +qemu_bh_delete(data-bh); +virtio_blk_handle_output(VIRTIO_DEVICE(data-s), data-s-vq); +} + +static void virtio_blk_pause(VirtIOBlock *vblk) +{ +/* TODO: stop ioeventfd */ +} + +static void virtio_blk_resume(VirtIOBlock *vblk) +{ +VirtIOBlockResumeData *data = g_new(VirtIOBlockResumeData, 1); +data-bh = aio_bh_new(blk_get_aio_context(vblk-blk), +virtio_blk_resume_bh_cb, data); +data-s = vblk; +data-s-paused = false; +qemu_bh_schedule(data-bh); +} + static const VirtIOBlockOps virtio_blk_ops = (VirtIOBlockOps) { -.complete_request = virtio_blk_complete_request, +.complete_request = virtio_blk_complete_request, +.pause = virtio_blk_pause, +.resume = virtio_blk_resume, }; static void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status) @@ -597,6 +627,9 @@ static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq) VirtIOBlockReq *req; MultiReqBuffer mrb = {}; +if (s-paused) { +return; +} /* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start * dataplane here instead of waiting for .set_status(). */ @@ -787,7 +820,7 @@ static void virtio_blk_save(QEMUFile *f, void *opaque) virtio_save(vdev, f); } - + static void virtio_blk_save_device(VirtIODevice *vdev, QEMUFile *f) { VirtIOBlock *s = VIRTIO_BLK(vdev); @@ -875,6 +908,29 @@ static void virtio_blk_migration_state_changed(Notifier *notifier, void *data) } } +static void virtio_blk_op_blocker_changed(Notifier *notifier, void *opaque) +{ +BlockOpEvent *event = opaque; +VirtIOBlock *s = container_of(notifier, VirtIOBlock, + op_blocker_notifier); +bool pause; + +if (event-type != BLOCK_OP_TYPE_DEVICE_IO) { +return; +} +pause = event-blocking || blk_op_is_blocked(s-blk, +BLOCK_OP_TYPE_DEVICE_IO, +
[Qemu-devel] [RFC PATCH 5/7] virtio-blk: Move complete_request to 'ops' structure
Should more ops be added to differentiate code between dataplane and non-dataplane, the new saved_ops approach will be cleaner than messing with N pointers. Signed-off-by: Fam Zheng f...@redhat.com --- hw/block/dataplane/virtio-blk.c | 13 - hw/block/virtio-blk.c | 8 ++-- include/hw/virtio/virtio-blk.h | 9 +++-- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 3db139b..ec0c8f4 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -51,8 +51,7 @@ struct VirtIOBlockDataPlane { /* Operation blocker on BDS */ Error *blocker; -void (*saved_complete_request)(struct VirtIOBlockReq *req, - unsigned char status); +const VirtIOBlockOps *saved_ops; }; /* Raise an interrupt to signal guest, if necessary */ @@ -88,6 +87,10 @@ static void complete_request_vring(VirtIOBlockReq *req, unsigned char status) qemu_bh_schedule(s-bh); } +static const VirtIOBlockOps virtio_blk_data_plane_ops = { +.complete_request = complete_request_vring, +}; + static void handle_notify(EventNotifier *e) { VirtIOBlockDataPlane *s = container_of(e, VirtIOBlockDataPlane, @@ -269,8 +272,8 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s) } s-host_notifier = *virtio_queue_get_host_notifier(vq); -s-saved_complete_request = vblk-complete_request; -vblk-complete_request = complete_request_vring; +s-saved_ops = vblk-ops; +vblk-ops = virtio_blk_data_plane_ops; s-starting = false; s-started = true; @@ -313,7 +316,7 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s) return; } s-stopping = true; -vblk-complete_request = s-saved_complete_request; +vblk-ops = s-saved_ops; trace_virtio_blk_data_plane_stop(s); aio_context_acquire(s-ctx); diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index e6afe97..f4a9d19 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -59,9 +59,13 @@ static void virtio_blk_complete_request(VirtIOBlockReq *req, virtio_notify(vdev, s-vq); } +static const VirtIOBlockOps virtio_blk_ops = (VirtIOBlockOps) { +.complete_request = virtio_blk_complete_request, +}; + static void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status) { -req-dev-complete_request(req, status); +req-dev-ops-complete_request(req, status); } static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, @@ -905,7 +909,7 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) s-sector_mask = (s-conf.conf.logical_block_size / BDRV_SECTOR_SIZE) - 1; s-vq = virtio_add_queue(vdev, 128, virtio_blk_handle_output); -s-complete_request = virtio_blk_complete_request; +s-ops = virtio_blk_ops; virtio_blk_data_plane_create(vdev, conf, s-dataplane, err); if (err != NULL) { error_propagate(errp, err); diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index 6bf5905..28b3436 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -44,6 +44,12 @@ struct VirtIOBlkConf struct VirtIOBlockDataPlane; struct VirtIOBlockReq; + +typedef struct { +/* Function to push to vq and notify guest */ +void (*complete_request)(struct VirtIOBlockReq *req, unsigned char status); +} VirtIOBlockOps; + typedef struct VirtIOBlock { VirtIODevice parent_obj; BlockBackend *blk; @@ -54,8 +60,7 @@ typedef struct VirtIOBlock { unsigned short sector_mask; bool original_wce; VMChangeStateEntry *change; -/* Function to push to vq and notify guest */ -void (*complete_request)(struct VirtIOBlockReq *req, unsigned char status); +const VirtIOBlockOps *ops; Notifier migration_state_notifier; struct VirtIOBlockDataPlane *dataplane; } VirtIOBlock; -- 1.9.3
[Qemu-devel] [PATCH] serial: fix multi-pci card error cleanup.
Put the number of serial ports into a local variable in multi_serial_pci_realize, then increment the port count (pci-ports) as we initialize the serial port cores. Now pci-ports always holds the number of successfully initialized ports and we can use multi_serial_pci_exit to properly cleanup the already initialized bits in case of a init failure. https://bugzilla.redhat.com/show_bug.cgi?id=970551 Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/char/serial-pci.c | 20 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c index 467c3b4..653064f 100644 --- a/hw/char/serial-pci.c +++ b/hw/char/serial-pci.c @@ -48,6 +48,8 @@ typedef struct PCIMultiSerialState { uint8_t prog_if; } PCIMultiSerialState; +static void multi_serial_pci_exit(PCIDevice *dev); + static void serial_pci_realize(PCIDevice *dev, Error **errp) { PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev); @@ -89,32 +91,33 @@ static void multi_serial_pci_realize(PCIDevice *dev, Error **errp) PCIMultiSerialState *pci = DO_UPCAST(PCIMultiSerialState, dev, dev); SerialState *s; Error *err = NULL; -int i; +int i, nr_ports = 0; switch (pc-device_id) { case 0x0003: -pci-ports = 2; +nr_ports = 2; break; case 0x0004: -pci-ports = 4; +nr_ports = 4; break; } -assert(pci-ports 0); -assert(pci-ports = PCI_SERIAL_MAX_PORTS); +assert(nr_ports 0); +assert(nr_ports = PCI_SERIAL_MAX_PORTS); pci-dev.config[PCI_CLASS_PROG] = pci-prog_if; pci-dev.config[PCI_INTERRUPT_PIN] = 0x01; -memory_region_init(pci-iobar, OBJECT(pci), multiserial, 8 * pci-ports); +memory_region_init(pci-iobar, OBJECT(pci), multiserial, 8 * nr_ports); pci_register_bar(pci-dev, 0, PCI_BASE_ADDRESS_SPACE_IO, pci-iobar); pci-irqs = qemu_allocate_irqs(multi_serial_irq_mux, pci, - pci-ports); + nr_ports); -for (i = 0; i pci-ports; i++) { +for (i = 0; i nr_ports; i++) { s = pci-state + i; s-baudbase = 115200; serial_realize_core(s, err); if (err != NULL) { error_propagate(errp, err); +multi_serial_pci_exit(dev); return; } s-irq = pci-irqs[i]; @@ -122,6 +125,7 @@ static void multi_serial_pci_realize(PCIDevice *dev, Error **errp) memory_region_init_io(s-io, OBJECT(pci), serial_io_ops, s, pci-name[i], 8); memory_region_add_subregion(pci-iobar, 8 * i, s-io); +pci-ports++; } } -- 1.8.3.1
Re: [Qemu-devel] [PATCH v6 03/17] Extend QMP command query-cpus to return accelerator id and model name
On Wed, May 06, 2015 at 11:49:50AM +0200, Michael Mueller wrote: On Tue, 5 May 2015 10:11:15 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Mon, Apr 27, 2015 at 04:53:17PM +0200, Michael Mueller wrote: The QMP command query-cpus now additionally displays a model name and the backing accelerator. Both are omitted if the model name is not initialized. request: { execute : query-cpus } answer: { { current: true, CPU: 0, model: 2827-ga2, halted: false, accel: kvm, thread_id: 31917 }, ... } Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com Acked-by: Christian Borntraeger borntrae...@de.ibm.com With the new qom-path field I submitted yesterday, this can be provided as QOM properties through qom-get. Is that really a good idea to make the object representation part of the ABI. I believe that's the whole point of QOM properties. I guess there is a related discussion already somewhere. I mean not just adding the qom-path field, I saw that suggested patch, I mean the approach to expose the objects themselves... I will try your patch of course as well... Yes, there are two approaches we are considering to allow clients to find the CPU QOM objects (qom-path in query-cpus, and links/aliases in /machine/cpus). But whatever approach we use, if clients can find the CPU objects in the QOM tree, you won't need the new fields in query-cpus and the info can be provided using qom-get. -- Eduardo
Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
On Wed, May 06, 2015 at 11:59:38AM +0200, Michael Mueller wrote: On Tue, 5 May 2015 10:26:02 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Mon, Apr 27, 2015 at 04:53:16PM +0200, Michael Mueller wrote: The patch defines ids per accelerator and adds the accel_id and the model_name to the CPUState. The accel_id is initialized by common code, the model name needs to be initialized by target specific code. Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com Acked-by: Christian Borntraeger borntrae...@de.ibm.com --- include/qom/cpu.h | 5 + qapi-schema.json | 9 + qom/cpu.c | 14 ++ 3 files changed, 28 insertions(+) diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 9dafb48..4ffc050 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -236,6 +236,8 @@ struct kvm_run; * @mem_io_pc: Host Program Counter at which the memory was accessed. * @mem_io_vaddr: Target virtual address at which the memory was accessed. * @kvm_fd: vCPU file descriptor for KVM. + * @accel_id: accelerator id of this CPU. + * @model_name: model name of this CPU * * State of one CPU core or thread. */ @@ -313,6 +315,9 @@ struct CPUState { (absolute value) offset as small as possible. This reduces code size, especially for hosts without large memory offsets. */ volatile sig_atomic_t tcg_exit_req; + +AccelId accel_id; This can be a AccelState pointer, set on initialization, because we have another user case for having a AccelState pointer: query-cpu-definition implementations may create temporary CPU objects with a different accel object to be able to probe for accel-specific data. (The pointer may become a link QOM property later.) +char *model_name; }; QTAILQ_HEAD(CPUTailQ, CPUState); diff --git a/qapi-schema.json b/qapi-schema.json index ac9594d..540e520 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2515,6 +2515,15 @@ ## { 'command': 'query-machines', 'returns': ['MachineInfo'] } +# @AccelId +# +# Defines accelerator ids +# +# Since: 2.4 +## +{ 'enum': 'AccelId', + 'data': ['qtest', 'tcg', 'kvm', 'xen'] } + Not sure if it is better to have an enum or simply a string here. ## # @CpuDefinitionInfo: # diff --git a/qom/cpu.c b/qom/cpu.c index 108bfa2..457afc7 100644 --- a/qom/cpu.c +++ b/qom/cpu.c @@ -67,6 +67,20 @@ CPUState *cpu_generic_init(const char *typename, const char *cpu_model) goto out; } +if (tcg_enabled()) { +cpu-accel_id = ACCEL_ID_TCG; +} else if (kvm_enabled()) { +cpu-accel_id = ACCEL_ID_KVM; +} +#ifdef CONFIG_XEN +else if (xen_enabled()) { +cpu-accel_id = ACCEL_ID_XEN; +} +#endif +else { +cpu-accel_id = ACCEL_ID_QTEST; +} You can simply use ACCEL_GET_CLASS(current_machine-accelerator)-name here. If we really want an enum, we can add an AccelId field to AccelClass, and initialize it properly on the accel class_init functions. The AccelClass (ac = ACCEL_GET_CLASS(current_machine-accelerator) would be ok though. That will allow to access the ac-accel_id (not yet there) at places where required. I'm just not sure how to access current_machine here. It is a global variable declared in hw/boards.h. But it makes sense only if !CONFIG_USER. But I am starting to wonder if we shouldn't simply expose accel info as a /mmachine property, instead of per-CPU information. It may become per-CPU in the future, but right now we really don't support having multiple accelerators so why are we trying to pretend we do? CONFIG_USER may require some special code when returning the accelerator ID as tcg because IIRC it doesn't use the QOM accel classes (yet). + object_property_set_bool(OBJECT(cpu), true, realized, err); out: -- 1.8.3.1 -- Eduardo
Re: [Qemu-devel] [PATCH v2] libcacard: stop including qemu-common.h
On Wed, May 6, 2015 at 12:05 PM, Michael Tokarev m...@tls.msk.ru wrote: 06.05.2015 12:23, Laurent Desnogues wrote: Hello, On Mon, Apr 27, 2015 at 3:27 PM, Michael Tokarev m...@tls.msk.ru wrote: From: Paolo Bonzini pbonz...@redhat.com This is a small step towards making libcacard standalone. on my system the removal of qemu-common.h inclusion broke compilation due to assert being used in glib-compat.h. Interesting. What kind of build environment is that? I compile-tested on several platforms, all went fine.. ;) That's a CentOS 6.6 machine with glib2 2.28.8. A fix might be to include assert.h in glib-compat.h. I prefer s/assert/g_assert/ in glib-compat.h. That indeed looks better :-) Thanks, Laurent Thanks, /mjt
Re: [Qemu-devel] qemu-img convert (vmdk)
On Wed, 05/06 10:15, Antoni Villalonga wrote: Hi again, it seems to be related to the subformat option. With the default option seems to work fine. I think StreamOptimized is mantatory for 'ova' files. I guess I see the problem now - the data is corrupted due to a wrong cluster allocation during converting to streamOptimized. I'm testing a patch locally. However there is another problem with streamOptimized which makes it refused in exporting to VMware application. See also: http://lists.gnu.org/archive/html/qemu-devel/2014-08/msg00467.html https://lists.gnu.org/archive/html/qemu-devel/2015-03/msg00024.html Fam Thanks On Tue, May 05, 2015 at 05:01:34PM +, Antoni Villalonga wrote: Hi, Is my first email to that list ;) I can reproduce this bug with v2.2 and v2.3. I'm not sure about the results after testing with v2.1 (doesn't show errors but seems to be still broken). % qemu-img convert -f raw -O vmdk -o subformat=streamOptimized 100GB_inputfile.img outputfile0.vmdk (no shown errors/warnings/info) % qemu-img convert -f vmdk -O qcow2 outputfile0.vmdk outputfile11.qcow2 qemu-img: error while reading sector 278528: Invalid argument Let me know if I can provide you more info. Regards -- Antoni Villalonga http://friki.cat/ -- Antoni Villalonga http://friki.cat/
[Qemu-devel] [PATCH RFC 6/7] vhost-net: tell tap backend about the vnet endianness
The default behaviour for TAP/MACVTAP is to consider vnet as native endian. This patch handles the cases when this is not true: - virtio 1.0: always little-endian - legacy cross-endian Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- hw/net/vhost_net.c | 33 - 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index cf23335..1884e59 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -38,6 +38,7 @@ #include standard-headers/linux/virtio_ring.h #include hw/virtio/vhost.h #include hw/virtio/virtio-bus.h +#include hw/virtio/virtio-access.h struct vhost_net { struct vhost_dev dev; @@ -194,6 +195,27 @@ static void vhost_net_set_vq_index(struct vhost_net *net, int vq_index) net-dev.vq_index = vq_index; } +static int vhost_net_set_vnet_endian(VirtIODevice *dev, NetClientState *peer, + bool set) +{ +int r = 0; + +if (virtio_has_feature(dev, VIRTIO_F_VERSION_1) || +(virtio_legacy_is_cross_endian(dev) !virtio_is_big_endian(dev))) { +r = qemu_set_vnet_le(peer, set); +if (r) { +error_report(backend does not support LE vnet headers); +} +} else if (virtio_legacy_is_cross_endian(dev)) { +r = qemu_set_vnet_be(peer, set); +if (r) { +error_report(backend does not support BE vnet headers); +} +} + +return r; +} + static int vhost_net_start_one(struct vhost_net *net, VirtIODevice *dev) { @@ -304,6 +326,11 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, goto err; } +r = vhost_net_set_vnet_endian(dev, ncs[0].peer, true); +if (r 0) { +goto err; +} + for (i = 0; i total_queues; i++) { vhost_net_set_vq_index(get_vhost_net(ncs[i].peer), i * 2); } @@ -311,7 +338,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, r = k-set_guest_notifiers(qbus-parent, total_queues * 2, true); if (r 0) { error_report(Error binding guest notifier: %d, -r); -goto err; +goto err_endian; } for (i = 0; i total_queues; i++) { @@ -333,6 +360,8 @@ err_start: fprintf(stderr, vhost guest notifier cleanup failed: %d\n, e); fflush(stderr); } +err_endian: +vhost_net_set_vnet_endian(dev, ncs[0].peer, false); err: return r; } @@ -355,6 +384,8 @@ void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs, fflush(stderr); } assert(r = 0); + +assert(vhost_net_set_vnet_endian(dev, ncs[0].peer, false) = 0); } void vhost_net_cleanup(struct vhost_net *net)
[Qemu-devel] [PATCH RFC 2/7] linux-headers: sync vhost.h
This patch brings the cross-endian vhost API to QEMU. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- linux-headers/linux/vhost.h | 14 ++ 1 file changed, 14 insertions(+) diff --git a/linux-headers/linux/vhost.h b/linux-headers/linux/vhost.h index c656f61..ead86db 100644 --- a/linux-headers/linux/vhost.h +++ b/linux-headers/linux/vhost.h @@ -103,6 +103,20 @@ struct vhost_memory { /* Get accessor: reads index, writes value in num */ #define VHOST_GET_VRING_BASE _IOWR(VHOST_VIRTIO, 0x12, struct vhost_vring_state) +/* Set the vring byte order in num. Valid values are VHOST_VRING_LITTLE_ENDIAN + * or VHOST_VRING_BIG_ENDIAN (other values return -EINVAL). + * The byte order cannot be changed while the device is active: trying to do so + * returns -EBUSY. + * This is a legacy only API that is simply ignored when VIRTIO_F_VERSION_1 is + * set. + * Not all kernel configurations support this ioctl, but all configurations that + * support SET also support GET. + */ +#define VHOST_VRING_LITTLE_ENDIAN 0 +#define VHOST_VRING_BIG_ENDIAN 1 +#define VHOST_SET_VRING_ENDIAN _IOW(VHOST_VIRTIO, 0x13, struct vhost_vring_state) +#define VHOST_GET_VRING_ENDIAN _IOW(VHOST_VIRTIO, 0x14, struct vhost_vring_state) + /* The following ioctls use eventfd file descriptors to signal and poll * for events. */
[Qemu-devel] [PATCH RFC 7/7] vhost_net: re-enable when cross endian
From: Cédric Le Goater c...@fr.ibm.com Cross-endianness is now checked by the core vhost code. revert 371df9f5e0f1 vhost-net: disable when cross-endian Signed-off-by: Cédric Le Goater c...@fr.ibm.com [ added commit message, Greg Kurz gk...@linux.vnet.ibm.com ] Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- hw/net/vhost_net.c | 19 --- 1 file changed, 19 deletions(-) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index 1884e59..3e4b0f2 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -293,19 +293,6 @@ static void vhost_net_stop_one(struct vhost_net *net, vhost_dev_disable_notifiers(net-dev, dev); } -static bool vhost_net_device_endian_ok(VirtIODevice *vdev) -{ -#ifdef TARGET_IS_BIENDIAN -#ifdef HOST_WORDS_BIGENDIAN -return virtio_is_big_endian(vdev); -#else -return !virtio_is_big_endian(vdev); -#endif -#else -return true; -#endif -} - int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, int total_queues) { @@ -314,12 +301,6 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus); int r, e, i; -if (!vhost_net_device_endian_ok(dev)) { -error_report(vhost-net does not support cross-endian); -r = -ENOSYS; -goto err; -} - if (!k-set_guest_notifiers) { error_report(binding does not support guest notifiers); r = -ENOSYS;
[Qemu-devel] [PATCH RFC 4/7] vhost: set vring endianness for legacy virtio
Legacy virtio is native endian: if the guest and host endianness differ, we have to tell vhost so it can swap bytes where appropriate. This is done through a vhost ring ioctl. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- hw/virtio/vhost.c | 50 +- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 54851b7..1d7b939 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -17,9 +17,11 @@ #include hw/hw.h #include qemu/atomic.h #include qemu/range.h +#include qemu/error-report.h #include linux/vhost.h #include exec/address-spaces.h #include hw/virtio/virtio-bus.h +#include hw/virtio/virtio-access.h #include migration/migration.h static void vhost_dev_sync_region(struct vhost_dev *dev, @@ -647,6 +649,27 @@ static void vhost_log_stop(MemoryListener *listener, /* FIXME: implement */ } +static int vhost_virtqueue_set_vring_endian_legacy(struct vhost_dev *dev, + bool is_big_endian, + int vhost_vq_index) +{ +struct vhost_vring_state s = { +.index = vhost_vq_index, +.num = is_big_endian +}; + +if (!dev-vhost_ops-vhost_call(dev, VHOST_SET_VRING_ENDIAN, s)) { +return 0; +} + +if (errno == ENOTTY) { +error_report(vhost does not support cross-endian); +return -ENOSYS; +} + +return -errno; +} + static int vhost_virtqueue_start(struct vhost_dev *dev, struct VirtIODevice *vdev, struct vhost_virtqueue *vq, @@ -677,6 +700,16 @@ static int vhost_virtqueue_start(struct vhost_dev *dev, return -errno; } +if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1) +virtio_legacy_is_cross_endian(vdev)) { +r = vhost_virtqueue_set_vring_endian_legacy(dev, +virtio_is_big_endian(vdev), +vhost_vq_index); +if (r) { +return -errno; +} +} + s = l = virtio_queue_get_desc_size(vdev, idx); a = virtio_queue_get_desc_addr(vdev, idx); vq-desc = cpu_physical_memory_map(a, l, 0); @@ -747,8 +780,9 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev, struct vhost_virtqueue *vq, unsigned idx) { +int vhost_vq_index = idx - dev-vq_index; struct vhost_vring_state state = { -.index = idx - dev-vq_index +.index = vhost_vq_index, }; int r; assert(idx = dev-vq_index idx dev-vq_index + dev-nvqs); @@ -759,6 +793,20 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev, } virtio_queue_set_last_avail_idx(vdev, idx, state.num); virtio_queue_invalidate_signalled_used(vdev, idx); + +/* In the cross-endian case, we need to reset the vring endianness to + * native as legacy devices expect so by default. + */ +if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1) +virtio_legacy_is_cross_endian(vdev)) { +r = vhost_virtqueue_set_vring_endian_legacy(dev, + !virtio_is_big_endian(vdev), +vhost_vq_index); +if (r 0) { +error_report(failed to reset vring endianness); +} +} + assert (r = 0); cpu_physical_memory_unmap(vq-ring, virtio_queue_get_ring_size(vdev, idx), 0, virtio_queue_get_ring_size(vdev, idx));
Re: [Qemu-devel] [PATCH target-arm v6 00/14] Next Generation Xilinx Zynq SoC
On Wed, May 6, 2015 at 5:14 AM, Peter Maydell peter.mayd...@linaro.org wrote: On 1 May 2015 at 18:25, Peter Crosthwaite peter.crosthwa...@xilinx.com wrote: Ping! On Fri, Apr 24, 2015 at 1:28 PM, Peter Crosthwaite peter.crosthwa...@xilinx.com wrote: Hi Peter and all, Xilinx's next gen SoC has been announced. This series adds a SoC and board. Neither patchwork nor patches seem to have the complete set of these patch emails -- can you try resending, please? Hi Peter, Where does patches live these days? Has it moved last couple of months? I was having trouble with patches update recently. Regards, Peter thanks -- PMM
[Qemu-devel] [PATCH 00/14] usb: QOMify
From: Gonglei arei.gong...@huawei.com Because DO_UPCAST() is long deprecated, let me do some cleanup work for usb sub-system, which I have said in previous conversation of a bugfix. Patch 7 is a bugfix. Please review, thanks :) Gonglei (14): uhci: QOMify usb: usb-audio QOMify usb: usb-bt QOMify usb: usb-hid QOMify usb: usb-hub QOMify usb: usb-mtp QOMify usb-mtp: fix segmentation fault usb: usb-net QOMify usb: usb-ccid QOMify usb: usb-storage QOMify usb: usb-uas QOMify usb: usb-wacom-tablet QOMify usb: usb-redir QOMify usb: usb-serial QOMify hw/usb/dev-audio.c| 23 +++- hw/usb/dev-bluetooth.c| 13 ++- hw/usb/dev-hid.c | 34 + hw/usb/dev-hub.c | 11 ++ hw/usb/dev-mtp.c | 15 + hw/usb/dev-network.c | 11 ++ hw/usb/dev-serial.c | 43 +++-- hw/usb/dev-smartcard-reader.c | 50 --- hw/usb/dev-storage.c | 32 --- hw/usb/dev-uas.c | 15 +++-- hw/usb/dev-wacom.c| 9 +--- hw/usb/hcd-uhci.c | 43 ++--- hw/usb/redirect.c | 25 -- 13 files changed, 201 insertions(+), 123 deletions(-) -- 1.7.12.4
[Qemu-devel] [PATCH 14/14] usb: usb-serial QOMify
From: Gonglei arei.gong...@huawei.com Signed-off-by: Gonglei arei.gong...@huawei.com --- hw/usb/dev-serial.c | 43 +++ 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c index 67c2072..6ca3da9 100644 --- a/hw/usb/dev-serial.c +++ b/hw/usb/dev-serial.c @@ -103,6 +103,9 @@ typedef struct { CharDriverState *cs; } USBSerialState; +#define TYPE_USB_SERIAL usb-serial-dev +#define USB_SERIAL_DEV(obj) OBJECT_CHECK(USBSerialState, (obj), TYPE_USB_SERIAL) + enum { STR_MANUFACTURER = 1, STR_PRODUCT_SERIAL, @@ -473,7 +476,7 @@ static void usb_serial_event(void *opaque, int event) static void usb_serial_realize(USBDevice *dev, Error **errp) { -USBSerialState *s = DO_UPCAST(USBSerialState, dev, dev); +USBSerialState *s = USB_SERIAL_DEV(dev); Error *local_err = NULL; usb_desc_create_serial(dev); @@ -576,26 +579,40 @@ static Property serial_properties[] = { DEFINE_PROP_END_OF_LIST(), }; -static void usb_serial_class_initfn(ObjectClass *klass, void *data) +static void usb_serial_dev_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); USBDeviceClass *uc = USB_DEVICE_CLASS(klass); -uc-realize = usb_serial_realize; -uc-product_desc = QEMU USB Serial; -uc-usb_desc = desc_serial; +uc-realize= usb_serial_realize; uc-handle_reset = usb_serial_handle_reset; uc-handle_control = usb_serial_handle_control; uc-handle_data= usb_serial_handle_data; dc-vmsd = vmstate_usb_serial; -dc-props = serial_properties; set_bit(DEVICE_CATEGORY_INPUT, dc-categories); } +static const TypeInfo usb_serial_dev_type_info = { +.name = TYPE_USB_SERIAL, +.parent = TYPE_USB_DEVICE, +.instance_size = sizeof(USBSerialState), +.abstract = true, +.class_init = usb_serial_dev_class_init, +}; + +static void usb_serial_class_initfn(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); +USBDeviceClass *uc = USB_DEVICE_CLASS(klass); + +uc-product_desc = QEMU USB Serial; +uc-usb_desc = desc_serial; +dc-props = serial_properties; +} + static const TypeInfo serial_info = { .name = usb-serial, -.parent= TYPE_USB_DEVICE, -.instance_size = sizeof(USBSerialState), +.parent= TYPE_USB_SERIAL, .class_init= usb_serial_class_initfn, }; @@ -609,26 +626,20 @@ static void usb_braille_class_initfn(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); USBDeviceClass *uc = USB_DEVICE_CLASS(klass); -uc-realize= usb_serial_realize; uc-product_desc = QEMU USB Braille; uc-usb_desc = desc_braille; -uc-handle_reset = usb_serial_handle_reset; -uc-handle_control = usb_serial_handle_control; -uc-handle_data= usb_serial_handle_data; -dc-vmsd = vmstate_usb_serial; dc-props = braille_properties; -set_bit(DEVICE_CATEGORY_INPUT, dc-categories); } static const TypeInfo braille_info = { .name = usb-braille, -.parent= TYPE_USB_DEVICE, -.instance_size = sizeof(USBSerialState), +.parent= TYPE_USB_SERIAL, .class_init= usb_braille_class_initfn, }; static void usb_serial_register_types(void) { +type_register_static(usb_serial_dev_type_info); type_register_static(serial_info); usb_legacy_register(usb-serial, serial, usb_serial_init); type_register_static(braille_info); -- 1.7.12.4
Re: [Qemu-devel] [PATCH target-arm v6 00/14] Next Generation Xilinx Zynq SoC
On 6 May 2015 at 14:08, Peter Crosthwaite peter.crosthwa...@xilinx.com wrote: Ergh, I have to backpeddle again (sry for the noise), patches still doesnt work for me (serves me right for not looking at the year of submission dates). Latest patches I have from http://wiki.qemu.org/patches/patches.json is: Message-id: 1424264377-5992-1-git-send-email-r...@ispras.ru From: Vasily Efimov r...@ispras.ru Date: 2015-02-18 [1/1] Makefile: fix up parallel building under MSYS+MinGW Message-id: 1424237320-12189-1-git-send-email-da...@gibson.dropbear.id. au From: David Gibson da...@gibson.dropbear.id.au Date: 2015-02-17 [1/1] Make platform-bus enabled only on E500 by default Has patches moved? Yes; please use url=http://vmsplice.net/~patches/patches.json (run by Stefan). thanks -- PMM
Re: [Qemu-devel] [PATCH target-arm v6 00/14] Next Generation Xilinx Zynq SoC
On 6 May 2015 at 14:02, Peter Crosthwaite peter.crosthwa...@xilinx.com wrote: Actually NVM. I got it working on a diff machine. Did you have any thoughts on the GICC mirror issue while I respin? You mean the thing where the GICC might not be at the bottom of a 64K page? Just map it wherever it lives in the hardware you're modelling... If your h/w is one of the few that's taken the map the pages mirrored over the page then a suitable container and aliases should handle that I think. -- PMM
[Qemu-devel] [PATCH 2/7] qcow2: simplify qcow2_cache_put() and qcow2_cache_entry_mark_dirty()
Since all tables are now stored together, it is possible to obtain the position of a particular table directly from its address, so the operation becomes O(1). Signed-off-by: Alberto Garcia be...@igalia.com --- block/qcow2-cache.c | 32 +++- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c index f0dfb69..6e78c8f 100644 --- a/block/qcow2-cache.c +++ b/block/qcow2-cache.c @@ -49,6 +49,16 @@ static inline void *qcow2_cache_get_table_addr(BlockDriverState *bs, return (uint8_t *) c-table_array + (size_t) table * s-cluster_size; } +static inline int qcow2_cache_get_table_idx(BlockDriverState *bs, + Qcow2Cache *c, void *table) +{ +BDRVQcowState *s = bs-opaque; +ptrdiff_t table_offset = (uint8_t *) table - (uint8_t *) c-table_array; +int idx = table_offset / s-cluster_size; +assert(idx = 0 idx c-size table_offset % s-cluster_size == 0); +return idx; +} + Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables) { BDRVQcowState *s = bs-opaque; @@ -337,16 +347,12 @@ int qcow2_cache_get_empty(BlockDriverState *bs, Qcow2Cache *c, uint64_t offset, int qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table) { -int i; +int i = qcow2_cache_get_table_idx(bs, c, *table); -for (i = 0; i c-size; i++) { -if (qcow2_cache_get_table_addr(bs, c, i) == *table) { -goto found; -} +if (c-entries[i].offset == 0) { +return -ENOENT; } -return -ENOENT; -found: c-entries[i].ref--; *table = NULL; @@ -357,15 +363,7 @@ found: void qcow2_cache_entry_mark_dirty(BlockDriverState *bs, Qcow2Cache *c, void *table) { -int i; - -for (i = 0; i c-size; i++) { -if (qcow2_cache_get_table_addr(bs, c, i) == table) { -goto found; -} -} -abort(); - -found: +int i = qcow2_cache_get_table_idx(bs, c, table); +assert(c-entries[i].offset != 0); c-entries[i].dirty = true; } -- 2.1.4
[Qemu-devel] [PATCH 3/7] qcow2: use an LRU algorithm to replace entries from the L2 cache
The current algorithm to evict entries from the cache gives always preference to those in the lowest positions. As the size of the cache increases, the chances of the later elements of being removed decrease exponentially. In a scenario with random I/O and lots of cache misses, entries in positions 8 and higher are rarely (if ever) evicted. This can be seen even with the default cache size, but with larger caches the problem becomes more obvious. Using an LRU algorithm makes the chances of being removed from the cache independent from the position. Signed-off-by: Alberto Garcia be...@igalia.com --- block/qcow2-cache.c | 31 +++ 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c index 6e78c8f..786c10a 100644 --- a/block/qcow2-cache.c +++ b/block/qcow2-cache.c @@ -28,10 +28,10 @@ #include trace.h typedef struct Qcow2CachedTable { -int64_t offset; -booldirty; -int cache_hits; -int ref; +int64_t offset; +bool dirty; +uint64_t lru_counter; +int ref; } Qcow2CachedTable; struct Qcow2Cache { @@ -40,6 +40,7 @@ struct Qcow2Cache { int size; booldepends_on_flush; void *table_array; +uint64_tlru_counter; }; static inline void *qcow2_cache_get_table_addr(BlockDriverState *bs, @@ -233,16 +234,18 @@ int qcow2_cache_empty(BlockDriverState *bs, Qcow2Cache *c) for (i = 0; i c-size; i++) { assert(c-entries[i].ref == 0); c-entries[i].offset = 0; -c-entries[i].cache_hits = 0; +c-entries[i].lru_counter = 0; } +c-lru_counter = 0; + return 0; } static int qcow2_cache_find_entry_to_replace(Qcow2Cache *c) { int i; -int min_count = INT_MAX; +uint64_t min_lru_counter = UINT64_MAX; int min_index = -1; @@ -251,15 +254,9 @@ static int qcow2_cache_find_entry_to_replace(Qcow2Cache *c) continue; } -if (c-entries[i].cache_hits min_count) { +if (c-entries[i].lru_counter min_lru_counter) { min_index = i; -min_count = c-entries[i].cache_hits; -} - -/* Give newer hits priority */ -/* TODO Check how to optimize the replacement strategy */ -if (c-entries[i].cache_hits 1) { -c-entries[i].cache_hits /= 2; +min_lru_counter = c-entries[i].lru_counter; } } @@ -318,12 +315,10 @@ static int qcow2_cache_do_get(BlockDriverState *bs, Qcow2Cache *c, /* Give the table some hits for the start so that it won't be replaced * immediately. The number 32 is completely arbitrary. */ -c-entries[i].cache_hits = 32; c-entries[i].offset = offset; /* And return the right table */ found: -c-entries[i].cache_hits++; c-entries[i].ref++; *table = qcow2_cache_get_table_addr(bs, c, i); @@ -356,6 +351,10 @@ int qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table) c-entries[i].ref--; *table = NULL; +if (c-entries[i].ref == 0) { +c-entries[i].lru_counter = ++c-lru_counter; +} + assert(c-entries[i].ref = 0); return 0; } -- 2.1.4
[Qemu-devel] [PATCH] rules.mak: Force CFLAGS for all objects in DSO
Because of the trick of process-archive-undefs, all .mo objects, even with --enable-modules, are dependencies of executables. This breaks CFLAGS propogation because the compiling of module object will happen too early before building for DSO. With GCC 5, the linking would fail because .o doesn't have -fPIC. Also, BUILD_DSO will be missed. (module-common.o will have it, so the stamp symbol was still liked in .so). Fix the problem by forcing the CFLAGS during unnest-vars. Reported-by: Alexander Graf ag...@suse.de Signed-off-by: Fam Zheng f...@redhat.com --- rules.mak | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules.mak b/rules.mak index 3a05627..6c0caf3 100644 --- a/rules.mak +++ b/rules.mak @@ -102,7 +102,6 @@ endif %.o: %.dtrace $(call quiet-command,dtrace -o $@ -G -s $, GEN $(TARGET_DIR)$@) -%$(DSOSUF): CFLAGS += -fPIC -DBUILD_DSO %$(DSOSUF): LDFLAGS += $(LDFLAGS_SHARED) %$(DSOSUF): %.mo $(call LINK,$^) @@ -353,6 +352,7 @@ define unnest-vars $(foreach o,$($v), $(eval $o: $($o-objs))) $(eval $(patsubst %-m,%-y,$v) += $($v)) + $(eval $($v:%.mo=%$(DSOSUF)) $($v) $(foreach o,$($v),$($o-objs)) .PHONY: CFLAGS += -fPIC -DBUILD_DSO) $(eval modules: $($v:%.mo=%$(DSOSUF))), $(eval $(patsubst %-m,%-y,$v) += $(call expand-objs, $($v) -- 1.9.3
Re: [Qemu-devel] blkdebug and VMDK (iotests 033 failure on monolithicFlat)
On 05.05.2015 12:44, Fam Zheng wrote: Max, Since you once fixed VMDK with the json descriptor filename, could you take a look at the error: $ ./check -vmdk -o subformat=monolithicFlat 033 033 - output mismatch (see 033.out.bad) --- /home/fam/qemu/tests/qemu-iotests/033.out 2015-05-05 10:52:50.524378312 +0800 +++ 033.out.bad 2015-05-05 17:38:36.147369924 +0800 @@ -2,54 +2,36 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 == preparing image == -wrote 1024/1024 bytes at offset 512 -1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -wrote 1536/1536 bytes at offset 131072 -1.500 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -wrote 131072/131072 bytes at offset 1024 -128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +qemu-io: can't open device blkdebug::/tmp/qemu-iotests/t.vmdk: Cannot use relative extent paths with VMDK descriptor file 'json:{image: {driver: file, filename: /tmp/qemu-iotests/t.vmdk}, driver: blkdebug, align: 512}' +qemu-io: can't open device blkdebug::/tmp/qemu-iotests/t.vmdk: Cannot use relative extent paths with VMDK descriptor file 'json:{image: {driver: file, filename: /tmp/qemu-iotests/t.vmdk}, driver: blkdebug, align: 512}' +qemu-io: can't open device blkdebug::/tmp/qemu-iotests/t.vmdk: Cannot use relative extent paths with VMDK descriptor file 'json:{image: {driver: file, filename: /tmp/qemu-iotests/t.vmdk}, driver: blkdebug, align: 512}' ... I'm not sure what's the best fix here. It's possible to dig out bs-file-file-filename in this case, but I'm not sure what's the rule of this filename mystery. Fam Hi Fam, The problem is that the align option is (consciously) set for blkdebug, so the blkdebug driver cannot just convert it to a blkdebug::$foo filename, but must create a JSON filename so that the align option is included. I seem to recall that someone (me? I don't know) once proposed a way for block drivers not only to present a file name (which is json:{${baz}} in this case), but also the name of the directory they are in, or better, a template to which a relative path can be appended (in Unix syntax, that is, with / as separators and .. and so on). The difference to the file name would be that we can ignore all the options given to the block driver for controlling its behavior, we just need a path name. I don't know why we didn't pursue that proposal, but it does seem to be necessary here. I think we can integrate it into the existing bdrv_refresh_filename() infrastructure, so it shouldn't be too difficult. I can take a shot at it, if you agree. Max
Re: [Qemu-devel] [PATCH 6/7] monitor: i: Add ARM specifics
On 05/05/2015 11:57 PM, Peter Crosthwaite wrote: So I have made a start on this. The ARM, MB and CRIS in this patch series is rather easy. Its X86 im having trouble with but your example here looks like most of the work ... Indeed, the flags setup becomes less obscure when, instead of #ifdef TARGET_I386 if (wsize == 2) { flags = 1; } else if (wsize == 4) { flags = 0; } else { So here the monitor is actually using the argument memory-dump size to dictate the flags. Is this flawed and should we delete this wsize if-else and rely on the CPU-state driven logic for correct disas info selection? This wsize reliance seems unique to x86. I think we would have to give this up in a QOMified approach. Hmm. I don't think that I've ever noticed the monitor disassembly could do that. If I were going to do that kind of debugging I certainly wouldn't use the monitor -- I'd use gdb. ;-) If someone thinks we ought to keep that feature, speak now... /* as default we use the current CS size */ flags = 0; if (env) { #ifdef TARGET_X86_64 if ((env-efer MSR_EFER_LMA) (env-segs[R_CS].flags DESC_L_MASK)) This uses env-efer and segs to drive the flags... flags = 2; else #endif if (!(env-segs[R_CS].flags DESC_B_MASK)) flags = 1; } } in one place and #if defined(TARGET_I386) if (flags == 2) { s.info.mach = bfd_mach_x86_64; } else if (flags == 1) { s.info.mach = bfd_mach_i386_i8086; } else { s.info.mach = bfd_mach_i386_i386; } print_insn = print_insn_i386; in another, we merge the two so that we get s.info.mach = bfd_mach_i386_i8086; if (env-hflags (1U HF_CS32_SHIFT)) { But your new implementation uses hflags. Are they the same state? I couldnt find easy correltation between MSR_EFER_LMA and HF_CSXX_SHIFT (although I do see that map to hflags HF_LMA?). Is your code a functional change? It's not intended to be. Since I couldn't find where wsize was initialized, I pulled the tests used by target-i386/translator.c, for dc-code32 and dc-code64, since I knew where to find them right away. ;-) Without going back to the manuals, I don't know the difference between CS64 and LMA; from the code it appears only the behaviour of sysret, which seems surprising. I went for adding print_insn to disassembly_info and passing just that to the hook. Patches soon! I might leave X86 out for the first spin. Sounds good. r~