Re: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size
On Wed, Aug 12, 2015 at 08:53:44AM +, Wu, Feng wrote: -Original Message- From: Jan Beulich [mailto:jbeul...@suse.com] Sent: Wednesday, August 12, 2015 4:43 PM To: Wu, Feng Cc: stefano.stabell...@eu.citrix.com; xen-de...@lists.xensource.com; qemu-devel@nongnu.org Subject: RE: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size On 12.08.15 at 09:10, feng...@intel.com wrote: -Original Message- From: qemu-devel-bounces+feng.wu=intel@nongnu.org [mailto:qemu-devel-bounces+feng.wu=intel@nongnu.org] On Behalf Of Jan Beulich Sent: Wednesday, August 12, 2015 2:59 PM To: Wu, Feng Cc: xen-de...@lists.xensource.com; qemu-devel@nongnu.org; stefano.stabell...@eu.citrix.com Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size On 05.08.15 at 04:02, feng...@intel.com wrote: @@ -491,8 +474,9 @@ static int xen_pt_bar_reg_write(XenPCIPassthroughState *s, XenPTReg *cfg_entry, bar_ro_mask = XEN_PT_BAR_IO_RO_MASK | (r_size - 1); break; case XEN_PT_BAR_FLAG_UPPER: +r = d-io_regions[index-1]; Perhaps worth an assert(index 0)? No problem, I will add it. BTW, do you have any other comments about this patch? If no, I am going to send out the new version with this changes. No - everything else looks to make sense (but continues to need testing). I don't have such a device in hand. Can anybody who has such a device help to test this patch? It would be highly appreciated! Um, 4GB MMIO bars? Wouldn't the Nvidia Quadro K6000 12GB GDDR do it? I am sure that Intel would be OK expensing $4K of hardware :-) Thanks, Feng Jan ___ Xen-devel mailing list xen-de...@lists.xen.org http://lists.xen.org/xen-devel
Re: [Qemu-devel] [PATCH] MAINTAINERS: list smbios maintainers
On 8/12/15 04:20, Michael S. Tsirkin wrote: Now that smbios has its own directory, list its maintainers. Same people as ACPI so just reuse that entry. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- MAINTAINERS | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 978b717..a059d5d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -645,13 +645,15 @@ S: Supported F: include/hw/pci/* F: hw/pci/* -ACPI +ACPI/SMBIOS M: Michael S. Tsirkin m...@redhat.com M: Igor Mammedov imamm...@redhat.com S: Supported F: include/hw/acpi/* +F: include/hw/smbios/* F: hw/mem/* F: hw/acpi/* +F: hw/smbios/* F: hw/i386/acpi-build.[hc] F: hw/i386/*dsl F: hw/arm/virt-acpi-build.c Acked-by: Wei Huang w...@redhat.com Note that SMBIOS patch isn't in yet. -Wei
Re: [Qemu-devel] [PATCH for-2.5 15/30] m68k: add more modes to movem
On 08/12/2015 01:07 AM, Andreas Schwab wrote: Richard Henderson r...@twiddle.net writes: On 08/09/2015 01:13 PM, Laurent Vivier wrote: +opsize = (insn 0x40) != 0 ? OS_LONG : OS_WORD; +incr = opsize_bytes(opsize); +if (!is_load (insn 070) == 040) { +for (i = 15; i = 0; i--, mask = 1) { This has got to be wrong. Just because it's pre-decrement doesn't mean you should skip all of the loads. Pre-dec only supports reg-to-mem, and is special because mask is bit reversed. Ah, I'd never noticed that. A comment to that effect would be good. r~
Re: [Qemu-devel] [RFC PATCH V7 00/19] Multithread TCG.
On 11/08/2015 15:59, Frederic Konrad wrote: On 11/08/2015 14:45, Paolo Bonzini wrote: On 10/08/2015 17:26, fred.kon...@greensocs.com wrote: From: KONRAD Frederic fred.kon...@greensocs.com This is the 7th round of the MTTCG patch series. Thanks to look at this. Here is a list of issues that I found: - tb_lock usage in tb_find_fast is complicated and introduces the need for other complicated code such as the tb_invalidate callback. Instead, the tb locking should reuse the cpu-exec.c code for user-mode emulation, with additional locking in the spots identified by Fred. The reason for this is that locking around tb_find_fast just kills the performance. - tb_lock uses a recursive lock, but this is not necessary. Did I ever say I dislike recursive mutexes? :) The wrappers tb_lock()/tb_unlock()/tb_lock_reset() can catch recursive locking for us, so it's not hard to do without it. True this is definitely not nice but seems some code eg: tb_invalidate are called from different place with or without the tb_lock. We might probably be able to lock the caller. - code_bitmap is not protected by any mutex (tb_invalidate_phys_page_fast is called with the iothread mutex taken, but other users of code_bitmap do not use it). Writes should be protected by the tb_lock, reads by either tb_lock or RCU. Yes good point I missed this one. - memory barriers are probably requested around accesses to -exit_request. -thread_kicked also needs to be accessed with atomics, because async_run_{,safe_}on_cpu can be called outside the big QEMU lock. - the whole signal-based qemu_cpu_kick can just go away. Just setting tcg_exit_req and exit_request will kick the TCG thread. The hairy Win32 SuspendThread/ResumeThread goes away too. I suggest doing it now, because proving it unnecessary is easier than proving it correct. Just setting tcg_exit_req and exit_request and signal the cpu-halt_cond I guess? BTW that affect KVM as well. Seems this mechanism is used as well with qemu_cpu_kick_self().. Which is a little strange as it seems the SIGIPI trigger a dummy signal handler? memset(sigact, 0, sizeof(sigact)); sigact.sa_handler = dummy_signal; sigaction(SIG_IPI, sigact, NULL); - user-mode emulation is broken (does not compile) - the big QEMU lock is not taken anywhere for MMIO accesses that require it (i.e. basically all of them) Isn't that handled by prepare_mmio_access? - some code wants to be called _outside_ the big QEMU lock, for example because it longjmps back to cpu_exec. For example, I suspect that the notdirty callbacks must be marked with memory_region_clear_global_locking. Fred I've started looking at them (and documenting the locking conventions for functions), and I hope to post it to some git repo later this week. Paolo It can be cloned from: g...@git.greensocs.com:fkonrad/mttcg.git branch multi_tcg_v7. This patch-set try to address the different issues in the global picture of MTTCG, presented on the wiki. == Needed patch for our work == Some preliminaries are needed for our work: * current_cpu doesn't make sense in mttcg so a tcg_executing flag is added to the CPUState. * We need to run some work safely when all VCPUs are outside their execution loop. This is done with the async_run_safe_work_on_cpu function introduced in this series. * QemuSpin lock is introduced (on posix only yet) to allow a faster handling of atomic instruction. == Code generation and cache == As Qemu stands, there is no protection at all against two threads attempting to generate code at the same time or modifying a TranslationBlock. The protect TBContext with tb_lock patch address the issue of code generation and makes all the tb_* function thread safe (except tb_flush). This raised the question of one or multiple caches. We choosed to use one unified cache because it's easier as a first step and since the structure of QEMU effectively has a ‘local’ cache per CPU in the form of the jump cache, we don't see the benefit of having two pools of tbs. == Dirty tracking == Protecting the IOs: To allows all VCPUs threads to run at the same time we need to drop the global_mutex as soon as possible. The io access need to take the mutex. This is likely to change when http://thread.gmane.org/gmane.comp.emulators.qemu/345258 will be upstreamed. Invalidation of TranslationBlocks: We can have all VCPUs running during an invalidation. Each VCPU is able to clean it's jump cache itself as it is in CPUState so that can be handled by a simple call to async_run_on_cpu. However tb_invalidate also writes to the TranslationBlock which is shared as we have only one pool. Hence this part of invalidate requires all VCPUs to exit before it can be done. Hence the async_run_safe_work_on_cpu is introduced to handle this case. == Atomic instruction == For now only ARM on x64 is supported by using an cmpxchg instruction. Specifically the limitation of this approach is
Re: [Qemu-devel] [PATCH v8 3/5] Introduce irqchip type specification for KVM
On 08/12/2015 04:24 PM, Christoffer Dall wrote: On Wed, Aug 12, 2015 at 4:14 PM, Eric Auger eric.au...@linaro.org wrote: Hi, On 08/12/2015 03:23 PM, Christoffer Dall wrote: On Wed, Aug 12, 2015 at 2:59 PM, Peter Maydell peter.mayd...@linaro.org wrote: On 12 August 2015 at 13:27, Pavel Fedin p.fe...@samsung.com wrote: Hello! I still think this is the wrong approach -- see my remarks in the previous round of patch review. You know... I thought a little bit... So far, test = true in KVM_CREATE_DEVICE means that we just want to know whether this type is supported. No actual actions is done by the kernel. Is it correct? yes If yes, we can just leave this test as it is, because if it says that GICv2 is supported by KVM_CREATE_DEVICE, then: 1. We use new API. No KVM_IRQCHIP_CREATE. 2. GICv3 may be supported. Therefore, if we do this check, and it succeeds, then we just proceed, and later actually try to create GICv3. If it fails for some reason, we will see error message anyway. So would it be OK just not to touch kvm_arch_irqchip_create() at all? No, because then if the kernel only supports GICv3 the code in kvm_arch_irqchip_create() (as it is currently written) will erroneously fall back to using the old API. Christoffer: the question was, why does kvm_arch_irqchip_create() not only check the KVM_CAP_DEVICE_CTRL but also try to see if it can KVM_CREATE_DEVICE the GICv2 in order to avoid falling back to the old pre-KVM_CREATE_DEVICE API ? Are there kernels which have the capability bit set but which can't actually use KVM_CREATE_DEVICE to create the irqchip? My thinking probably was that technically the KVM_CAP_DEVICE_CTRL is an orthogonal concept from how to create the vgic, and you could advertise this capability without also supporting the GICv2 device type. However, I don't believe such kernels exist the capability was advertised for arm/arm64 with GICv2 7330672 KVM: arm-vgic: Support KVM_CREATE_DEVICE for VGIC (1 year, 8 months ago) Christoffer Dall so effectively I don't think we have any arm kernel advertising the CAP without GICv3. and they cannot in the future as that would be because we would remove an actively supported API. My preference here would be for kvm_arch_irqchip_create() to just use 'is the KVM_CAP_DEVICE_CTRL capability set' for its can we use the new API test; that will then work whether we have a GICv2 or GICv3 in the host. (The actual GIC device creation later on might fail, of course, but that can be handled at that point. The only thing we need to do as early as kvm_arch_irqchip_create is determine whether we must use the old API.) another way was proposed in the past was consisting in calling first ret = kvm_create_device(s, KVM_DEV_TYPE_ARM_VGIC_V3, true); if this succeeds this means we have the new API (the only one used vy v3) and hence we have it as well for VGIC_V2, we can return ... if this fails we just can say VGICv3 KVM device hasn't registered, try KVM_DEV_TYPE_ARM_VGIC_V2 if we have it return else fall back to older API I think it worked as well? Besides I think Peter's suggestion is simpler. For info we also use KVM VFIO device now. Note that there's a difference between I called the create-device ioctl, and the creation of the device failed vs. I called the ioctl and I got an error because the ioctl is not supported, only in the latter case should you fall back to the older API. yes understood; practically kvm_ioctl_create_device used in test mode just looks for the specific device in the registered KVM device list and that's it. whatever the case, API not supported or device not found it reports -ENODEV. Eric Not sure if the end result is the same with the suggested approach, based on the right error values etc. -Christoffer
Re: [Qemu-devel] [PATCH v8 3/5] Introduce irqchip type specification for KVM
On Wed, Aug 12, 2015 at 4:14 PM, Eric Auger eric.au...@linaro.org wrote: Hi, On 08/12/2015 03:23 PM, Christoffer Dall wrote: On Wed, Aug 12, 2015 at 2:59 PM, Peter Maydell peter.mayd...@linaro.org wrote: On 12 August 2015 at 13:27, Pavel Fedin p.fe...@samsung.com wrote: Hello! I still think this is the wrong approach -- see my remarks in the previous round of patch review. You know... I thought a little bit... So far, test = true in KVM_CREATE_DEVICE means that we just want to know whether this type is supported. No actual actions is done by the kernel. Is it correct? yes If yes, we can just leave this test as it is, because if it says that GICv2 is supported by KVM_CREATE_DEVICE, then: 1. We use new API. No KVM_IRQCHIP_CREATE. 2. GICv3 may be supported. Therefore, if we do this check, and it succeeds, then we just proceed, and later actually try to create GICv3. If it fails for some reason, we will see error message anyway. So would it be OK just not to touch kvm_arch_irqchip_create() at all? No, because then if the kernel only supports GICv3 the code in kvm_arch_irqchip_create() (as it is currently written) will erroneously fall back to using the old API. Christoffer: the question was, why does kvm_arch_irqchip_create() not only check the KVM_CAP_DEVICE_CTRL but also try to see if it can KVM_CREATE_DEVICE the GICv2 in order to avoid falling back to the old pre-KVM_CREATE_DEVICE API ? Are there kernels which have the capability bit set but which can't actually use KVM_CREATE_DEVICE to create the irqchip? My thinking probably was that technically the KVM_CAP_DEVICE_CTRL is an orthogonal concept from how to create the vgic, and you could advertise this capability without also supporting the GICv2 device type. However, I don't believe such kernels exist the capability was advertised for arm/arm64 with GICv2 7330672 KVM: arm-vgic: Support KVM_CREATE_DEVICE for VGIC (1 year, 8 months ago) Christoffer Dall so effectively I don't think we have any arm kernel advertising the CAP without GICv3. and they cannot in the future as that would be because we would remove an actively supported API. My preference here would be for kvm_arch_irqchip_create() to just use 'is the KVM_CAP_DEVICE_CTRL capability set' for its can we use the new API test; that will then work whether we have a GICv2 or GICv3 in the host. (The actual GIC device creation later on might fail, of course, but that can be handled at that point. The only thing we need to do as early as kvm_arch_irqchip_create is determine whether we must use the old API.) another way was proposed in the past was consisting in calling first ret = kvm_create_device(s, KVM_DEV_TYPE_ARM_VGIC_V3, true); if this succeeds this means we have the new API (the only one used vy v3) and hence we have it as well for VGIC_V2, we can return ... if this fails we just can say VGICv3 KVM device hasn't registered, try KVM_DEV_TYPE_ARM_VGIC_V2 if we have it return else fall back to older API I think it worked as well? Besides I think Peter's suggestion is simpler. For info we also use KVM VFIO device now. Note that there's a difference between I called the create-device ioctl, and the creation of the device failed vs. I called the ioctl and I got an error because the ioctl is not supported, only in the latter case should you fall back to the older API. Not sure if the end result is the same with the suggested approach, based on the right error values etc. -Christoffer
Re: [Qemu-devel] [RFC v4 1/9] exec.c: Add new exclusive bitmap to ram_list
On Wed, Aug 12, 2015 at 4:10 PM, Paolo Bonzini pbonz...@redhat.com wrote: On 12/08/2015 16:04, alvise rigo wrote: clear algorithm: if bytemap[vaddr] == 254 bytemap[vaddr] = CPU_ID Isn't this also required for the clear algorithm? if bytemap[vaddr] 254 /* this can happen for the TLB_EXCL slow path effect */ bytemap[vaddr] = 255 I don't think so because clear doesn't clear TLB_EXCL. But maybe we're talking about two different things. :) Let me go through it again, I might have missed something :) alvise Paolo
Re: [Qemu-devel] [PATCH v8 3/5] Introduce irqchip type specification for KVM
Hi, On 08/12/2015 03:23 PM, Christoffer Dall wrote: On Wed, Aug 12, 2015 at 2:59 PM, Peter Maydell peter.mayd...@linaro.org wrote: On 12 August 2015 at 13:27, Pavel Fedin p.fe...@samsung.com wrote: Hello! I still think this is the wrong approach -- see my remarks in the previous round of patch review. You know... I thought a little bit... So far, test = true in KVM_CREATE_DEVICE means that we just want to know whether this type is supported. No actual actions is done by the kernel. Is it correct? yes If yes, we can just leave this test as it is, because if it says that GICv2 is supported by KVM_CREATE_DEVICE, then: 1. We use new API. No KVM_IRQCHIP_CREATE. 2. GICv3 may be supported. Therefore, if we do this check, and it succeeds, then we just proceed, and later actually try to create GICv3. If it fails for some reason, we will see error message anyway. So would it be OK just not to touch kvm_arch_irqchip_create() at all? No, because then if the kernel only supports GICv3 the code in kvm_arch_irqchip_create() (as it is currently written) will erroneously fall back to using the old API. Christoffer: the question was, why does kvm_arch_irqchip_create() not only check the KVM_CAP_DEVICE_CTRL but also try to see if it can KVM_CREATE_DEVICE the GICv2 in order to avoid falling back to the old pre-KVM_CREATE_DEVICE API ? Are there kernels which have the capability bit set but which can't actually use KVM_CREATE_DEVICE to create the irqchip? My thinking probably was that technically the KVM_CAP_DEVICE_CTRL is an orthogonal concept from how to create the vgic, and you could advertise this capability without also supporting the GICv2 device type. However, I don't believe such kernels exist the capability was advertised for arm/arm64 with GICv2 7330672 KVM: arm-vgic: Support KVM_CREATE_DEVICE for VGIC (1 year, 8 months ago) Christoffer Dall so effectively I don't think we have any arm kernel advertising the CAP without GICv3. and they cannot in the future as that would be because we would remove an actively supported API. My preference here would be for kvm_arch_irqchip_create() to just use 'is the KVM_CAP_DEVICE_CTRL capability set' for its can we use the new API test; that will then work whether we have a GICv2 or GICv3 in the host. (The actual GIC device creation later on might fail, of course, but that can be handled at that point. The only thing we need to do as early as kvm_arch_irqchip_create is determine whether we must use the old API.) another way was proposed in the past was consisting in calling first ret = kvm_create_device(s, KVM_DEV_TYPE_ARM_VGIC_V3, true); if this succeeds this means we have the new API (the only one used vy v3) and hence we have it as well for VGIC_V2, we can return ... if this fails we just can say VGICv3 KVM device hasn't registered, try KVM_DEV_TYPE_ARM_VGIC_V2 if we have it return else fall back to older API I think it worked as well? Besides I think Peter's suggestion is simpler. For info we also use KVM VFIO device now. Best Regards Eric I'm fine with this. -Christoffer
[Qemu-devel] [PATCH] sh4: Fix initramfs initialization for endiannes-mismatched targets
If host and target endianness does not match, loding an initramfs does not work. Fix by writing boot parameters with appropriate endianness conversion. Signed-off-by: Guenter Roeck li...@roeck-us.net --- hw/sh4/r2d.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c index 5e22ed7..3b0b2ec 100644 --- a/hw/sh4/r2d.c +++ b/hw/sh4/r2d.c @@ -338,9 +338,9 @@ static void r2d_init(MachineState *machine) } /* initialization which should be done by firmware */ -boot_params.loader_type = 1; -boot_params.initrd_start = INITRD_LOAD_OFFSET; -boot_params.initrd_size = initrd_size; +boot_params.loader_type = tswap32(1); +boot_params.initrd_start = tswap32(INITRD_LOAD_OFFSET); +boot_params.initrd_size = tswap32(initrd_size); } if (kernel_cmdline) { -- 2.1.4
[Qemu-devel] [PATCH] hw/misc/zynq_slcr: Change CPU clock rate
The Linux kernel only accepts 34 Khz and 67 Khz clock rates, and may crash if the actual clock rate is too low. The clock rate used to be (ps-clk-frequency * 26 / 4), which resulted in a CPU frequency of 21 Khz if ps-clk-frequency was set to Hz. Change it to (ps-clk-frequency * 20 / 2) = 33 Khz to make Linux happy. Signed-off-by: Guenter Roeck li...@roeck-us.net --- hw/misc/zynq_slcr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c index 964f253..d3e1ce0 100644 --- a/hw/misc/zynq_slcr.c +++ b/hw/misc/zynq_slcr.c @@ -189,7 +189,7 @@ static void zynq_slcr_reset(DeviceState *d) s-regs[LOCKSTA] = 1; /* 0x100 - 0x11C */ -s-regs[ARM_PLL_CTRL] = 0x0001A008; +s-regs[ARM_PLL_CTRL] = 0x00014008; s-regs[DDR_PLL_CTRL] = 0x0001A008; s-regs[IO_PLL_CTRL]= 0x0001A008; s-regs[PLL_STATUS] = 0x003F; @@ -198,7 +198,7 @@ static void zynq_slcr_reset(DeviceState *d) s-regs[IO_PLL_CFG] = 0x00014000; /* 0x120 - 0x16C */ -s-regs[ARM_CLK_CTRL] = 0x1F000400; +s-regs[ARM_CLK_CTRL] = 0x1F000200; s-regs[DDR_CLK_CTRL] = 0x1843; s-regs[DCI_CLK_CTRL] = 0x01E03201; s-regs[APER_CLK_CTRL] = 0x01FFCCCD; -- 2.1.4
Re: [Qemu-devel] [PATCH] Makefile.target: include top level build dir in vpath
On Aug 12, 2015 6:32 AM, Paolo Bonzini pbonz...@redhat.com wrote: On 09/08/2015 09:02, Michael Marineau wrote: Using ccache with CCACHE_BASEDIR set to $(SRC_PATH) or a parent will rewrite all absolute paths to relative paths. This interacts poorly with QEMU's two-level build directory scheme. For example, lets say BUILD_DIR=$(SRC_PATH)/build so build/blockdev.d will contain: blockdev.o: ../blockdev.c ../include/sysemu/block-backend.h \ Now the target build under build/x86_64-softmmu or similar will depend on ../blockdev.o which in turn will get make to source ../blockdev.d to check its dependencies. Since make always considers paths relative to the current working directory rather than the makefile the path appeared in the relative path to ../blockdev.c is useless. This change simply adds the top level build directory to vpath so paths relative to the source directory, top build directory, and target build directory all work just fine. --- Makefile.target | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.target b/Makefile.target index 3e7aafd..dc32294 100644 --- a/Makefile.target +++ b/Makefile.target @@ -7,7 +7,7 @@ include config-target.mak include config-devices.mak include $(SRC_PATH)/rules.mak -$(call set-vpath, $(SRC_PATH)) +$(call set-vpath, $(SRC_PATH):$(BUILD_DIR)) ifdef CONFIG_LINUX QEMU_CFLAGS += -I../linux-headers endif Hi, can you reply with Signed-off-by: Michael Marineau michael.marin...@coreos.com, representing that you've read and agreed to the Developer Certificate of Origin (http://developercertificate.org/)? This is necessary to include your patch in QEMU. Paolo Oops, forgot about that. Signed-off-by: Michael Marineau michael.marin...@coreos.com
Re: [Qemu-devel] [PATCH v2] target-cris: update CPU state save/load to use VMStateDescription
On Fri, Aug 07, 2015 at 05:02:14PM +0100, Peter Maydell wrote: From: Juan Quintela quint...@redhat.com Update the CRIS CPU state save/load to use a VMStateDescription struct rather than cpu_save/cpu_load functions. Have to define TLBSet struct. Multidimensional arrays in C are a mess, just unroll them. Signed-off-by: Juan Quintela quint...@redhat.com Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com [PMM: * expand commit message a little since it's no longer one patch in a 35-patch series * add header/copyright comment to machine.c; credited copyright is Red Hat and author is Juan, since this commit gives the file all-new contents; license is LGPL-2-or-later, to match other target-cris code * remove hardcoded tab * add fields for locked_irq, interrupt_vector, fault_vector, trap_vector * drop minimum_version_id_old fields * bump version_id to 2 as we are not compatible with old state format * remove unnecessary hw/boards.h include * update to register via dc-vmsd] Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- This is a patch of Juan's from way back in 2012, which I am resurrecting because we now have only two CPUs which still use old-style non-VMState save/load (CRIS and SPARC). If we can update them both we can drop the machinery in the common code which supports this. Notes: * CPUCRISState indent style is somewhat mismatched (cf load_info); I took the minimal make checkpatch happy approach, but am happy to do something else with the line changed here * I have added a copyright header to target-cris/machine.c, because it did not have one at all, and this commit effectively gives the file all-new contents. I have set it up as LGPLv2-or-later, copyright Red Hat, author Juan. Please let me know if you would prefer something else! * I added vmstate entries for four fields which did not previously get saved and restored, which is presumably a bug fix... * vmsave/vmload on the axis-dev88 board does not currently seem to work (among other obvious problems, there is no vmstate support in the interrupt controller), so we're limited to looks good on code review here. Changes v1-v2: * actually register the vmstate struct, by setting dc-vmsd (discussion on IRC with Andreas a few weeks back indicated that dc-vmsd is preferred over cc-vmsd for new CPUs) * update it to be the right format for a dc-vmsd struct Testing: I checked with a host gdb that we seem to be filling in the registers with the right values. Since none of the devices in the axis model (including the interrupt controller) support vmstate save/load, the run after load from snapshot goes wrong pretty quickly, obviously. target-cris/cpu-qom.h | 4 ++ target-cris/cpu.c | 1 + target-cris/cpu.h | 13 ++-- target-cris/machine.c | 167 +- 4 files changed, 95 insertions(+), 90 deletions(-) diff --git a/target-cris/cpu-qom.h b/target-cris/cpu-qom.h index 6fc30c2..df4c0b5 100644 --- a/target-cris/cpu-qom.h +++ b/target-cris/cpu-qom.h @@ -73,6 +73,10 @@ static inline CRISCPU *cris_env_get_cpu(CPUCRISState *env) #define ENV_OFFSET offsetof(CRISCPU, env) +#ifndef CONFIG_USER_ONLY +extern const struct VMStateDescription vmstate_cris_cpu; +#endif + void cris_cpu_do_interrupt(CPUState *cpu); void crisv10_cpu_do_interrupt(CPUState *cpu); bool cris_cpu_exec_interrupt(CPUState *cpu, int int_req); diff --git a/target-cris/cpu.c b/target-cris/cpu.c index b17e849..d461e07 100644 --- a/target-cris/cpu.c +++ b/target-cris/cpu.c @@ -302,6 +302,7 @@ static void cris_cpu_class_init(ObjectClass *oc, void *data) cc-handle_mmu_fault = cris_cpu_handle_mmu_fault; #else cc-get_phys_page_debug = cris_cpu_get_phys_page_debug; +dc-vmsd = vmstate_cris_cpu; #endif cc-gdb_num_core_regs = 49; diff --git a/target-cris/cpu.h b/target-cris/cpu.h index d422e35..6760dc6 100644 --- a/target-cris/cpu.h +++ b/target-cris/cpu.h @@ -108,6 +108,11 @@ #define NB_MMU_MODES 2 +typedef struct { +uint32_t hi; +uint32_t lo; +} TLBSet; + typedef struct CPUCRISState { uint32_t regs[16]; /* P0 - P15 are referred to as special registers in the docs. */ @@ -161,11 +166,7 @@ typedef struct CPUCRISState { * * One for I and another for D. */ - struct - { - uint32_t hi; - uint32_t lo; - } tlbsets[2][4][16]; +TLBSet tlbsets[2][4][16]; CPU_COMMON @@ -227,8 +228,6 @@ enum { #define cpu_gen_code cpu_cris_gen_code #define cpu_signal_handler cpu_cris_signal_handler -#define CPU_SAVE_VERSION 1 - /* MMU modes definitions */ #define MMU_MODE0_SUFFIX _kernel #define MMU_MODE1_SUFFIX _user diff --git a/target-cris/machine.c b/target-cris/machine.c index 8f9c0dd..983b67c 100644 --- a/target-cris/machine.c +++
Re: [Qemu-devel] [RFC PATCH V7 16/19] translate-all: introduces tb_flush_safe.
On 12/08/2015 16:11, Frederic Konrad wrote: You could also allocate a new code buffer and free the old one with call_rcu. This should simplify things a lot. Depending the size of the code buffer this might be a good idea. :). 32 megabytes. Paolo
Re: [Qemu-devel] [PATCH] linux-user: elfload: Still use TARGET_PAGE_SIZE for i386 guest
On 08/12/2015 12:59 AM, gchen gchen wrote: Nack. There's 99 problems with host page size guest page size. This solves none of them, and in the hackiest way possible. Under alpha virtual machine, if set i386 guest page size 8KB, it will cause failure directly (any dynamically linked binaries can not work). Yes, I know. The same thing happens when running i386 guests on other (non-virtual) hosts. E.g. Sparc64's 8kB page, PowerPC64's 64kB page. Do you have any other ideas for solving this issue? The only complete solution that I see is to use softmmu with linux-user, so that we properly emulate the guest pages. Yes, it will cause quite some slow-down in emulation, but I believe it's the only reliable way. r~
[Qemu-devel] [PATCH] block/raw-posix: Use raw_normalize_devicepath()
The filename given to qemu_open() in block/raw-posix.c should generally have been processed by raw_normalize_devicepath(); unless we are only probing (in which case the caller often checks whether the file is a block device or not, and this property will be changed by raw_normalize_devicepath() on NetBSD) or it is about a deprecated device (i.e. floppy). Signed-off-by: Max Reitz mre...@redhat.com --- block/raw-posix.c | 22 +- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index 855febe..30df8ad 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -670,11 +670,17 @@ static int raw_reopen_prepare(BDRVReopenState *state, /* If we cannot use fcntl, or fcntl failed, fall back to qemu_open() */ if (raw_s-fd == -1) { -assert(!(raw_s-open_flags O_CREAT)); -raw_s-fd = qemu_open(state-bs-filename, raw_s-open_flags); -if (raw_s-fd == -1) { -error_setg_errno(errp, errno, Could not reopen file); -ret = -1; +const char *normalized_filename = state-bs-filename; +ret = raw_normalize_devicepath(normalized_filename); +if (ret 0) { +error_setg_errno(errp, -ret, Could not normalize device path); +} else { +assert(!(raw_s-open_flags O_CREAT)); +raw_s-fd = qemu_open(normalized_filename, raw_s-open_flags); +if (raw_s-fd == -1) { +error_setg_errno(errp, errno, Could not reopen file); +ret = -1; +} } } @@ -2314,6 +2320,12 @@ static int hdev_create(const char *filename, QemuOpts *opts, (void)has_prefix; +ret = raw_normalize_devicepath(filename); +if (ret 0) { +error_setg_errno(errp, -ret, Could not normalize device path); +return ret; +} + /* Read out options */ total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0), BDRV_SECTOR_SIZE); -- 2.4.6
Re: [Qemu-devel] [RFC PATCH V7 00/19] Multithread TCG.
On 12/08/2015 17:19, Frederic Konrad wrote: BTW that affect KVM as well. Seems this mechanism is used as well with qemu_cpu_kick_self().. Which is a little strange as it seems the SIGIPI trigger a dummy signal handler? memset(sigact, 0, sizeof(sigact)); sigact.sa_handler = dummy_signal; sigaction(SIG_IPI, sigact, NULL); KVM is different, the signal handler is used to kick the VM out of KVM_RUN. We're going to add another path (a ioctl) but it cannot use the same code as TCG. qemu_cpu_kick_self is needed in some special cases where KVM tells you call KVM_RUN asap but you know you have more work to do in userspace. Calling qemu_cpu_kick_self lets you call KVM_RUN work and immediately do the userspace work. Paolo
[Qemu-devel] [PATCH v3 0/5] block: Drop BDS.filename
This series depends on [PATCH] block/raw-posix: Use raw_normalize_devicepath(). The BDS filename field is generally only used when opening disk images or emitting error or warning messages, the only exception to this rule is the map command of qemu-img. However, using exact_filename there instead should not be a problem. Therefore, we can drop the filename field from the BlockDriverState and use a function instead which builds the filename from scratch when called. This is slower than reading a static char array but the problem of that static array is that it may become obsolete due to changes in any BlockDriverState or in the BDS graph. Using a function which rebuilds the filename every time it is called resolves this problem. The disadvantage of worse performance is negligible, on the other hand. After patch 2 of this series, which replaces some queries of BDS.filename by reads from somewhere else (mostly BDS.exact_filename), the filename field is only used when a disk image is opened or some message should be emitted, both of which cases do not suffer from the performance hit. http://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg07025.html gives an example of how to achieve an outdated filename, so we do need this series. We can technically fix it differently (by appropriately invoking bdrv_refresh_filename()), but that is pretty difficult™. I find this series simpler and it it's something we wanted to do anyway. Regarding the fear that this might change current behavior, especially for libvirt which used filenames to identify nodes in the BDS graph: Since bdrv_open() already calls bdrv_refresh_filename() today, and apparently everything works fine, this series will most likely not break anything. The only thing that will change is if the BDS graph is dynamically reconfigured at runtime, and in that case it's most likely a bug fix. Also, I don't think libvirt supports such cases already. tl;dr: This series is a bug fix and I don't think it'll break legacy management applications relying on the filename to identify a BDS; as long as they are not trying to continue that practice with more advanced configurations (e.g. with Quorum). v2: - Patches 1 and 4: Store filename in heap rather than on the stack [Wen Congyang] - Patch 2: - Rebased on [PATCH] block/raw-posix: Use raw_normalize_devicepath() - Fixed 85-character line - Patch 3: Added wrapper function which facilitates generation of a filename into a temporary heap buffer git-backport-diff against v2: Key: [] : patches are identical [] : number of functional differences between upstream/downstream patch [down] : patch is downstream-only The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively 001/5:[0011] [FC] 'block: Change bdrv_get_encrypted_filename()' 002/5:[0007] [FC] 'block: Avoid BlockDriverState.filename' 003/5:[0009] [FC] 'block: Add bdrv_filename()' 004/5:[0129] [FC] 'block: Drop BlockDriverState.filename' 005/5:[] [--] 'iotests: Test changed Quorum filename' Max Reitz (5): block: Change bdrv_get_encrypted_filename() block: Avoid BlockDriverState.filename block: Add bdrv_filename() block: Drop BlockDriverState.filename iotests: Test changed Quorum filename block.c | 108 +- block/commit.c| 4 +- block/gluster.c | 2 +- block/mirror.c| 16 +-- block/qapi.c | 4 +- block/raw-posix.c | 20 ++--- block/raw-win32.c | 4 +- block/raw_bsd.c | 4 +- block/vhdx-log.c | 5 ++- block/vmdk.c | 22 +++--- block/vpc.c | 7 ++- blockdev.c| 22 +++--- include/block/block.h | 4 +- include/block/block_int.h | 1 - monitor.c | 6 ++- qemu-img.c| 2 +- tests/qemu-iotests/041| 17 ++-- 17 files changed, 179 insertions(+), 69 deletions(-) -- 2.4.6
[Qemu-devel] [PATCH v3 5/5] iotests: Test changed Quorum filename
After drive-mirror replacing a Quorum child, the filename of the Quorum BDS should reflect the change. This patch replaces the existing test for whether the operation did actually exchange the BDS (which simply tested whether the new BDS existed) by a test which examines the children list contained in the Quorum BDS's filename as returned by query-block. As a nice side effect of confirming that the new BDS actually belongs to the Quorum BDS, this checks whether the filename was properly updated. Signed-off-by: Max Reitz mre...@redhat.com --- tests/qemu-iotests/041 | 17 + 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 index 3d46ed7..316f42a 100755 --- a/tests/qemu-iotests/041 +++ b/tests/qemu-iotests/041 @@ -18,6 +18,7 @@ # along with this program. If not, see http://www.gnu.org/licenses/. # +import json import time import os import iotests @@ -908,10 +909,18 @@ class TestRepairQuorum(iotests.QMPTestCase): self.assert_qmp(result, 'return', {}) self.complete_and_wait(drive=quorum0) -result = self.vm.qmp('query-named-block-nodes') -self.assert_qmp(result, 'return[0]/file', quorum_repair_img) -# TODO: a better test requiring some QEMU infrastructure will be added -# to check that this file is really driven by quorum + +result = self.vm.qmp('query-block') +for blockdev in result['return']: +if blockdev['device'] == 'quorum0': +filename = blockdev['inserted']['image']['filename'] +self.assertEquals(filename[:5], 'json:') +children = json.loads(filename[5:])['children'] +self.assertEquals(children[0]['file']['filename'], quorum_img1) +self.assertEquals(children[1]['file']['filename'], + quorum_repair_img) +self.assertEquals(children[2]['file']['filename'], quorum_img3) + self.vm.shutdown() if __name__ == '__main__': -- 2.4.6
[Qemu-devel] [PATCH v3 2/5] block: Avoid BlockDriverState.filename
In places which directly pass a filename to the OS, we should not use the filename field at all but exact_filename instead (although the former currently equals the latter if that is set). In qemu-img's map command, we should be using the filename field; but since this commit prepares to remove that field, using exact_filename is fine, too (this is the only user of BlockDriverState.filename which frequently queries that field). Signed-off-by: Max Reitz mre...@redhat.com --- block.c | 5 +++-- block/gluster.c | 2 +- block/raw-posix.c | 8 block/raw-win32.c | 4 ++-- qemu-img.c| 2 +- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/block.c b/block.c index 41b0f85..1572785 100644 --- a/block.c +++ b/block.c @@ -918,8 +918,9 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, if (ret 0) { if (local_err) { error_propagate(errp, local_err); -} else if (bs-filename[0]) { -error_setg_errno(errp, -ret, Could not open '%s', bs-filename); +} else if (bs-exact_filename[0]) { +error_setg_errno(errp, -ret, Could not open '%s', + bs-exact_filename); } else { error_setg_errno(errp, -ret, Could not open image); } diff --git a/block/gluster.c b/block/gluster.c index 1eb3a8c..176682b 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -358,7 +358,7 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state, gconf = g_new0(GlusterConf, 1); -reop_s-glfs = qemu_gluster_init(gconf, state-bs-filename, errp); +reop_s-glfs = qemu_gluster_init(gconf, state-bs-exact_filename, errp); if (reop_s-glfs == NULL) { ret = -errno; goto exit; diff --git a/block/raw-posix.c b/block/raw-posix.c index 30df8ad..10f8f62 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -670,7 +670,7 @@ static int raw_reopen_prepare(BDRVReopenState *state, /* If we cannot use fcntl, or fcntl failed, fall back to qemu_open() */ if (raw_s-fd == -1) { -const char *normalized_filename = state-bs-filename; +const char *normalized_filename = state-bs-exact_filename; ret = raw_normalize_devicepath(normalized_filename); if (ret 0) { error_setg_errno(errp, -ret, Could not normalize device path); @@ -2201,7 +2201,7 @@ static int fd_open(BlockDriverState *bs) DPRINTF(No floppy (open delayed)\n); return -EIO; } -s-fd = qemu_open(bs-filename, s-open_flags ~O_NONBLOCK); +s-fd = qemu_open(bs-exact_filename, s-open_flags ~O_NONBLOCK); if (s-fd 0) { s-fd_error_time = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); s-fd_got_error = 1; @@ -2498,7 +2498,7 @@ static void floppy_eject(BlockDriverState *bs, bool eject_flag) qemu_close(s-fd); s-fd = -1; } -fd = qemu_open(bs-filename, s-open_flags | O_NONBLOCK); +fd = qemu_open(bs-exact_filename, s-open_flags | O_NONBLOCK); if (fd = 0) { if (ioctl(fd, FDEJECT, 0) 0) perror(FDEJECT); @@ -2722,7 +2722,7 @@ static int cdrom_reopen(BlockDriverState *bs) */ if (s-fd = 0) qemu_close(s-fd); -fd = qemu_open(bs-filename, s-open_flags, 0644); +fd = qemu_open(bs-exact_filename, s-open_flags, 0644); if (fd 0) { s-fd = -1; return -EIO; diff --git a/block/raw-win32.c b/block/raw-win32.c index 68f2338..5c8a894 100644 --- a/block/raw-win32.c +++ b/block/raw-win32.c @@ -417,7 +417,7 @@ static void raw_close(BlockDriverState *bs) CloseHandle(s-hfile); if (bs-open_flags BDRV_O_TEMPORARY) { -unlink(bs-filename); +unlink(bs-exact_filename); } } @@ -485,7 +485,7 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs) DWORD * high); get_compressed_t get_compressed; struct _stati64 st; -const char *filename = bs-filename; +const char *filename = bs-exact_filename; /* WinNT support GetCompressedFileSize to determine allocate size */ get_compressed = (get_compressed_t) GetProcAddress(GetModuleHandle(kernel32), diff --git a/qemu-img.c b/qemu-img.c index 75f4ee4..3d5587a 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2155,7 +2155,7 @@ static void dump_map_entry(OutputFormat output_format, MapEntry *e, } if ((e-flags (BDRV_BLOCK_DATA|BDRV_BLOCK_ZERO)) == BDRV_BLOCK_DATA) { printf(%#-16PRIx64%#-16PRIx64%#-16PRIx64%s\n, - e-start, e-length, e-offset, e-bs-filename); + e-start, e-length, e-offset, e-bs-exact_filename); } /* This format ignores the distinction between 0, ZERO and ZERO|DATA. * Modify the flags here to allow more coalescing. -- 2.4.6
[Qemu-devel] [PATCH v3 1/5] block: Change bdrv_get_encrypted_filename()
Instead of returning a pointer to the filename, copy it into a buffer specified by the caller. Signed-off-by: Max Reitz mre...@redhat.com --- block.c | 25 ++--- include/block/block.h | 2 +- monitor.c | 6 +- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/block.c b/block.c index d088ee0..41b0f85 100644 --- a/block.c +++ b/block.c @@ -2760,10 +2760,13 @@ void bdrv_add_key(BlockDriverState *bs, const char *key, Error **errp) } } else { if (bdrv_key_required(bs)) { +char *enc_filename = g_malloc(PATH_MAX); +bdrv_get_encrypted_filename(bs, enc_filename, PATH_MAX); error_set(errp, ERROR_CLASS_DEVICE_ENCRYPTED, '%s' (%s) is encrypted, bdrv_get_device_or_node_name(bs), - bdrv_get_encrypted_filename(bs)); + enc_filename); +g_free(enc_filename); } } } @@ -2980,14 +2983,22 @@ bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs) return false; } -const char *bdrv_get_encrypted_filename(BlockDriverState *bs) +char *bdrv_get_encrypted_filename(BlockDriverState *bs, char *dest, size_t sz) { -if (bs-backing_hd bs-backing_hd-encrypted) -return bs-backing_file; -else if (bs-encrypted) -return bs-filename; -else +if (sz INT_MAX) { +sz = INT_MAX; +} + +if (bs-backing_hd bs-backing_hd-encrypted) { +pstrcpy(dest, sz, bs-backing_file); +return dest; +} else if (bs-encrypted) { +pstrcpy(dest, sz, bs-filename); +return dest; +} else { +dest[0] = '\0'; return NULL; +} } void bdrv_get_backing_filename(BlockDriverState *bs, diff --git a/include/block/block.h b/include/block/block.h index 37916f7..a78e4f1 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -430,7 +430,7 @@ void bdrv_round_to_clusters(BlockDriverState *bs, int64_t *cluster_sector_num, int *cluster_nb_sectors); -const char *bdrv_get_encrypted_filename(BlockDriverState *bs); +char *bdrv_get_encrypted_filename(BlockDriverState *bs, char *dest, size_t sz); void bdrv_get_backing_filename(BlockDriverState *bs, char *filename, int filename_size); void bdrv_get_full_backing_filename(BlockDriverState *bs, diff --git a/monitor.c b/monitor.c index aeea2b5..a7d1650 100644 --- a/monitor.c +++ b/monitor.c @@ -5292,10 +5292,14 @@ int monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs, BlockCompletionFunc *completion_cb, void *opaque) { +char *enc_filename; int err; +enc_filename = g_malloc(PATH_MAX); +bdrv_get_encrypted_filename(bs, enc_filename, PATH_MAX); monitor_printf(mon, %s (%s) is encrypted.\n, bdrv_get_device_name(bs), - bdrv_get_encrypted_filename(bs)); + enc_filename); +g_free(enc_filename); mon-password_completion_cb = completion_cb; mon-password_opaque = opaque; -- 2.4.6
[Qemu-devel] [PATCH v3 4/5] block: Drop BlockDriverState.filename
That field is now only used during initialization of BlockDriverStates (opening images) and for error or warning messages. Performance is not that much of an issue here, so we can drop the field and replace its use by a call to bdrv_filename() or bdrv_filename_alloc(). By doing so we can ensure the result always to be recent, whereas the contents of BlockDriverState.filename may have been obsoleted by manipulations of single BlockDriverStates or of the BDS graph. The users of the BDS filename field were changed as follows: - copying the filename into another buffer is trivially replaced by using bdrv_filename() instead of the copy function - strdup() on the filename is replaced by a call to bdrv_filename_alloc() - bdrv_filename(bs, bs-filename, sizeof(bs-filename)) is replaced by bdrv_refresh_filename(bs) (these were introduced by the previous patch) - anywhere else bdrv_filename_alloc() is used, any access to BlockDriverState.filename is then replaced by the buffer returned, and it is freed when it is no longer needed Signed-off-by: Max Reitz mre...@redhat.com --- block.c | 47 ++- block/blkverify.c | 3 +-- block/commit.c| 4 +++- block/mirror.c| 16 block/qapi.c | 4 ++-- block/quorum.c| 2 +- block/raw-posix.c | 12 +--- block/raw_bsd.c | 4 +++- block/vhdx-log.c | 5 - block/vmdk.c | 22 -- block/vpc.c | 7 +-- blockdev.c| 22 +- include/block/block_int.h | 1 - 13 files changed, 103 insertions(+), 46 deletions(-) diff --git a/block.c b/block.c index d2a3e8d..c53d1d6 100644 --- a/block.c +++ b/block.c @@ -227,10 +227,10 @@ void bdrv_get_full_backing_filename_from_filename(const char *backed, void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t sz, Error **errp) { -char *backed = bs-exact_filename[0] ? bs-exact_filename : bs-filename; - +char *backed = bdrv_filename_alloc(bs); bdrv_get_full_backing_filename_from_filename(backed, bs-backing_file, dest, sz, errp); +g_free(backed); } void bdrv_register(BlockDriver *bdrv) @@ -821,6 +821,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, QDict *options, int flags, BlockDriver *drv, Error **errp) { int ret, open_flags; +char *filename_buffer = NULL; const char *filename; const char *node_name = NULL; QemuOpts *opts; @@ -831,7 +832,8 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, assert(options != NULL bs-options != options); if (file != NULL) { -filename = file-filename; +filename_buffer = bdrv_filename_alloc(file); +filename = filename_buffer; } else { filename = qdict_get_try_str(options, filename); } @@ -839,7 +841,8 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, if (drv-bdrv_needs_filename !filename) { error_setg(errp, The '%s' block driver requires a file name, drv-format_name); -return -EINVAL; +ret = -EINVAL; +goto fail_filename; } trace_bdrv_open_common(bs, filename ?: , flags, drv-format_name); @@ -888,11 +891,10 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, } if (filename != NULL) { -pstrcpy(bs-filename, sizeof(bs-filename), filename); +pstrcpy(bs-exact_filename, sizeof(bs-exact_filename), filename); } else { -bs-filename[0] = '\0'; +bs-exact_filename[0] = '\0'; } -pstrcpy(bs-exact_filename, sizeof(bs-exact_filename), bs-filename); bs-drv = drv; bs-opaque = g_malloc0(drv-instance_size); @@ -952,6 +954,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, assert((bs-request_alignment != 0) || bdrv_is_sg(bs)); qemu_opts_del(opts); +g_free(filename_buffer); return 0; free_and_fail: @@ -961,6 +964,8 @@ free_and_fail: bs-drv = NULL; fail_opts: qemu_opts_del(opts); +fail_filename: +g_free(filename_buffer); return ret; } @@ -1158,7 +1163,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd) } bs-backing_child = bdrv_attach_child(bs, backing_hd, child_backing); bs-open_flags = ~BDRV_O_NO_BACKING; -pstrcpy(bs-backing_file, sizeof(bs-backing_file), backing_hd-filename); +bdrv_filename(backing_hd, bs-backing_file, sizeof(bs-backing_file)); pstrcpy(bs-backing_format, sizeof(bs-backing_format), backing_hd-drv ? backing_hd-drv-format_name : ); @@ -1559,7 +1564,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, } } -
[Qemu-devel] [PATCH v3 3/5] block: Add bdrv_filename()
Split the part which actually refreshes the BlockDriverState.filename field off of bdrv_refresh_filename() into a more generic function bdrv_filename(), which first calls bdrv_refresh_filename() and then stores a qemu-usable filename into the given buffer instead of BlockDriverState.filename. Since bdrv_refresh_filename() therefore no longer refreshes that field, some calls to that function have to be replaced by calls to bdrv_filename() manually refreshing the BDS filename field (this is only temporary). Additionally, a wrapper function bdrv_filename_alloc() is added which allocates a buffer of size PATH_MAX, call bdrv_filename() on that buffer and returns it, since needing a temporary buffer for the filename is a rather common pattern. Signed-off-by: Max Reitz mre...@redhat.com --- block.c | 39 --- block/blkverify.c | 3 ++- block/quorum.c| 2 +- include/block/block.h | 2 ++ 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/block.c b/block.c index 1572785..d2a3e8d 100644 --- a/block.c +++ b/block.c @@ -1559,7 +1559,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, } } -bdrv_refresh_filename(bs); +bdrv_filename(bs, bs-filename, sizeof(bs-filename)); /* For snapshot=on, create a temporary qcow2 overlay. bs points to the * temporary snapshot afterwards. */ @@ -4153,9 +4153,6 @@ static bool append_open_options(QDict *d, BlockDriverState *bs) * - full_open_options: Options which, when given when opening a block device * (without a filename), result in a BDS (mostly) * equalling the given one - * - filename: If exact_filename is set, it is copied here. Otherwise, - * full_open_options is converted to a JSON object, prefixed with - * json: (for use through the JSON pseudo protocol) and put here. */ void bdrv_refresh_filename(BlockDriverState *bs) { @@ -4242,15 +4239,43 @@ void bdrv_refresh_filename(BlockDriverState *bs) bs-full_open_options = opts; } +} + +/* First refreshes exact_filename and full_open_options by calling + * bdrv_refresh_filename(). Then, if exact_filename is set, it is copied into + * the target buffer. Otherwise, full_open_options is converted to a JSON + * object, prefixed with json: (for use through the JSON pseudo protocol) and + * put there. + * + * If sz 0, the string put into the buffer will always be null-terminated. + * + * Returns @dest. + */ +char *bdrv_filename(BlockDriverState *bs, char *dest, size_t sz) +{ +bdrv_refresh_filename(bs); + +if (sz INT_MAX) { +sz = INT_MAX; +} if (bs-exact_filename[0]) { -pstrcpy(bs-filename, sizeof(bs-filename), bs-exact_filename); +pstrcpy(dest, sz, bs-exact_filename); } else if (bs-full_open_options) { QString *json = qobject_to_json(QOBJECT(bs-full_open_options)); -snprintf(bs-filename, sizeof(bs-filename), json:%s, - qstring_get_str(json)); +snprintf(dest, sz, json:%s, qstring_get_str(json)); QDECREF(json); } + +return dest; +} + +/* Wrapper around bdrv_filename() using g_malloc(PATH_MAX) for the destination + * buffer. + */ +char *bdrv_filename_alloc(BlockDriverState *bs) +{ +return bdrv_filename(bs, g_malloc(PATH_MAX), PATH_MAX); } /* This accessor function purpose is to allow the device models to access the diff --git a/block/blkverify.c b/block/blkverify.c index d277e63..cb9ce02 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -309,7 +309,8 @@ static void blkverify_refresh_filename(BlockDriverState *bs) BDRVBlkverifyState *s = bs-opaque; /* bs-file has already been refreshed */ -bdrv_refresh_filename(s-test_file); +bdrv_filename(s-test_file, s-test_file-filename, + sizeof(s-test_file-filename)); if (bs-file-full_open_options s-test_file-full_open_options) { QDict *opts = qdict_new(); diff --git a/block/quorum.c b/block/quorum.c index 2f6c45f..00d1fb0 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -1003,7 +1003,7 @@ static void quorum_refresh_filename(BlockDriverState *bs) int i; for (i = 0; i s-num_children; i++) { -bdrv_refresh_filename(s-bs[i]); +bdrv_filename(s-bs[i], s-bs[i]-filename, sizeof(s-bs[i]-filename)); if (!s-bs[i]-full_open_options) { return; } diff --git a/include/block/block.h b/include/block/block.h index a78e4f1..4be6c18 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -267,6 +267,8 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, const char *backing_file); int bdrv_get_backing_file_depth(BlockDriverState *bs); void bdrv_refresh_filename(BlockDriverState *bs); +char *bdrv_filename(BlockDriverState *bs, char *dest, size_t sz); +char *bdrv_filename_alloc(BlockDriverState *bs); int
Re: [Qemu-devel] [PATCH 08/10] tcg: add memory barriers in page_find_alloc accesses
On Wed, Aug 12, 2015 at 18:41:00 +0200, Paolo Bonzini wrote: page_find is reading the radix tree outside all locks, so it has to use the RCU primitives. It does not need RCU critical sections because the PageDescs are never removed, so there is never a need to wait for the end of code sections that use a PageDesc. Note that rcu_find_alloc might end up writing to the tree, see below. BTW the fact that there are no removals makes the use of RCU unnecessary. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- translate-all.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/translate-all.c b/translate-all.c index 7727091..78a787d 100644 --- a/translate-all.c +++ b/translate-all.c @@ -437,14 +437,14 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, int alloc) /* Level 2..N-1. */ for (i = V_L1_SHIFT / V_L2_BITS - 1; i 0; i--) { -void **p = *lp; +void **p = atomic_rcu_read(lp); if (p == NULL) { if (!alloc) { return NULL; } p = g_new0(void *, V_L2_SIZE); -*lp = p; +atomic_rcu_set(lp, p); } lp = p + ((index (i * V_L2_BITS)) (V_L2_SIZE - 1)); @@ -456,7 +456,7 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, int alloc) return NULL; } pd = g_new0(PageDesc, V_L2_SIZE); -*lp = pd; +atomic_rcu_set(lp, pd); rcu_set is not enough; a cmpxchg with a fail path would be needed here, since if the find_alloc is called without any locks held (as described in the commit message) several threads could concurrently write to the same node, corrupting the tree. I argue however that it is better to call page_find/_alloc with a mutex held, since otherwise we'd have to add per-PageDesc locks (it's very common to call page_find and then update the PageDesc). I have an RFC patchset for multithreaded tcg that deals with this, will submit once I bring it up to date with upstream. Emilio
Re: [Qemu-devel] [RFC 1/3] monitor: split MonitorQAPIEventState
On 08/12/2015 02:00 PM, Laszlo Ersek wrote: Assume there has been a long period of silence (no attempts to emit an event). Now some client code makes a call to emit the event. Will that event be emitted immediately, or will it be delayed to see if more are coming? I'd like to understand this aspect first. I think the first instance of the event, after the grace period, should be emitted immediately, and further instances that quickly follow should be suppressed. That has always been the goal of event throttling: when a new event arrives and the timer is not running, emit it right away and start the timer. If another event arrives while the timer has not expired, save it. If multiple events arrive during the timer, the last one received overwrites any others. Then, when the timer expires, either there is a saved event (one or more events arrived during the throttling window), so we send it and restart another throttling window, or there was no event pending so the line is quiet and the next event can send right away. Thus, management will get notification as soon as possible that events are starting to happen after a period of quiet, but will then not be spammed any faster than once per throttle period (once per second) with additional events. When additional events are sent, they are always the most accurate state of the system at the time the event was queued, but intermediate events may have been lost. Furthermore, unless the events have not been happening, the throttling of the event means the management might not see the event until nearly a second late, although the timestamp associated with the event should still be accurate to the time it was queued up, not finally sent. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH for-2.5 08/30] m68k: update CPU flags management
Le 12/08/2015 07:12, Richard Henderson a écrit : On 08/09/2015 01:13 PM, Laurent Vivier wrote: @@ -798,9 +796,9 @@ void HELPER(mac_set_flags)(CPUM68KState *env, uint32_t acc) } } -void HELPER(flush_flags)(CPUM68KState *env, uint32_t cc_op) +uint32_t HELPER(flush_flags)(CPUM68KState *env, uint32_t op) { -cpu_m68k_flush_flags(env, cc_op); +return cpu_m68k_flush_flags(env, op); } Since this function no longer modifies ENV, it probably deserves a better name than flush_flags. FWIW cc_compute_all isn't a bad name, if you're copying i386 anyway... -DEF_HELPER_2(flush_flags, void, env, i32) +DEF_HELPER_2(flush_flags, i32, env, i32) Modify to use DEF_HELPER_FLAGS while you're at it. At the moment it reads some globals, but doesn't write any, or have any other side effects. It writes env-cc_x, so I guess I can't use DEF_HELPER_FLAGS ? static inline void gen_flush_flags(DisasContext *s) { if (s-cc_op == CC_OP_FLAGS) return; -gen_flush_cc_op(s); -gen_helper_flush_flags(cpu_env, QREG_CC_OP); -s-cc_op = CC_OP_FLAGS; +if (s-cc_op == CC_OP_DYNAMIC) { +gen_helper_flush_flags(QREG_CC_DEST, cpu_env, QREG_CC_OP); +} else { +gen_helper_flush_flags(QREG_CC_DEST, cpu_env, tcg_const_i32(s-cc_op)); +} That const needs to be freed. perhaps I'm wrong, what I had understood is: tcg_const_i32() creates a tcg_temp_new_i32(), and tcg_temp_new_i32() are automatically freed at end of tcg block (whereas tcg_const_local adn tcg_temp_local are not). @@ -1248,7 +1294,6 @@ DISAS_INSN(bitop_im) DEST_EA(env, insn, opsize, tmp, addr); } } - DISAS_INSN(arith_im) { int op; Careful with the errant whitespace changes. @@ -1706,16 +1745,18 @@ DISAS_INSN(branch) /* bsr */ gen_push(s, tcg_const_i32(s-pc)); } -gen_flush_cc_op(s); if (op 1) { /* Bcc */ l1 = gen_new_label(); gen_jmpcc(s, ((insn 8) 0xf) ^ 1, l1); +update_cc_op(s); gen_jmp_tb(s, 1, base + offset); gen_set_label(l1); +update_cc_op(s); gen_jmp_tb(s, 0, s-pc); Ideally you'd do this only once, before the jmpcc. r~
Re: [Qemu-devel] [PATCH for-2.5 30/30] m68k: add bitfield instructions
On 08/09/2015 01:13 PM, Laurent Vivier wrote: uint32_t HELPER(rol32)(uint32_t val, uint32_t shift) { uint32_t result; @@ -1227,6 +1241,53 @@ void HELPER(set_mac_extu)(CPUM68KState *env, uint32_t val, uint32_t acc) env-macc[acc + 1] = res; } +/* load from a bitfield */ + +uint64_t HELPER(bitfield_load)(uint32_t addr, uint32_t offset, uint32_t width) +{ +uint8_t data[8]; +uint64_t bitfield; +int size; +int i; + +size = (offset + width + 7) 3; +#if defined(CONFIG_USER_ONLY) +cpu_memory_rw_debug(NULL, (target_ulong)addr, data, size, 0); +#else +cpu_physical_memory_rw(addr, data, size, 0); +#endif Err, this bypasses virtual memory. Definitely not correct. You'll need to use cpu_ld*_data instead. +DISAS_INSN(bitfield_reg) +{ +uint16_t ext; +TCGv tmp; +TCGv tmp1; +TCGv reg; +TCGv offset; +TCGv width; +int op; +TCGv reg2; +TCGv mask; + +reg = DREG(insn, 0); +op = (insn 8) 7; +ext = read_im16(env, s); + +bitfield_param(ext, offset, width, mask); + +if (ext 0x0800) { +tcg_gen_andi_i32(offset, offset, 31); +} +gen_helper_ror32(mask, mask, offset); Ah, the curious unused helpers. Anyway, tcg_gen_rotr_i32. Anyway, there's so much redundant computation in here let's name some sub-expressions, and give them their own temporaries. Let the tcg optimizer do its job if they turn out to be unused for a specific case. mask_msb = mask before the rotate (at msb). mask_inp = mask after the rotate (in place). +tmp = tcg_temp_new_i32(); +tcg_gen_and_i32(tmp, reg, mask); oldf_inp = tmp (old field in place). + +tmp1 = tcg_temp_new_i32(); +gen_helper_rol32(tmp1, tmp, offset); oldf_msb = tmp1 (old field at msb). +tcg_gen_sub_i32(tmp2, tcg_const_i32(32), width); comp_width = tmp2 (compliment of width). Use tcg_gen_subfi so you won't leak the const. Compute this once for all of the sub-cases. +switch (op) { +case 0: /* bftst */ +break; +case 1: /* bfextu */ +tcg_gen_add_i32(tmp1, offset, width); +tcg_gen_andi_i32(tmp1, tmp1, 31); +gen_helper_rol32(reg2, tmp, tmp1); tcg_gen_shr_i32(reg2, oldf_msb, comp_width); +break; +case 2: /* bfchg */ +tcg_gen_xor_i32(reg, reg, mask); +break; +case 3: /* bfexts */ +gen_helper_rol32(reg2, tmp, offset); +tcg_gen_sub_i32(width, tcg_const_i32(32), width); +tcg_gen_sar_i32(reg2, reg2, width); tcg_gen_sar_i32(reg2, oldf_msb, comp_width); +case 4: /* bfclr */ +tcg_gen_not_i32(mask, mask); +tcg_gen_and_i32(reg, reg, mask); tcg_gen_andc_i32(reg, reg, mask_inp); +break; +case 5: /* bfffo */ +gen_helper_rol32(reg2, tmp, offset); +gen_helper_bfffo(tmp, tmp, width); +tcg_gen_add_i32(reg2, tmp, offset); There's a typo here if you look close. That said, tcg_gen_orc_i32(tmp, oldf_msb, mask_msb); gen_helper_clo32(tmp, tmp); tcg_gen_add_i32(reg2, tmp, offset); with uint32_t helper_clo32(uint32_t x) { return clo32(x); } The first opcode sets all bits outside the field, so that (if the field is smaller than 32 bits) we're guaranteed to find a one. At which point there's really very little that needs doing in the helper. +case 6: /* bfset */ +tcg_gen_or_i32(reg, reg, mask); +break; tcg_gen_or_i32(reg, reg, mask_inp); +case 7: /* bfins */ +tcg_gen_shl_i32(tmp1, tcg_const_i32(1), width); Undefined if width == 32. That said... +tcg_gen_subi_i32(tmp1, tmp1, 1); +tcg_gen_and_i32(tmp, reg2, tmp1); +tcg_gen_add_i32(tmp1, offset, width); +tcg_gen_andi_i32(tmp1, tmp1, 31); +gen_helper_ror32(tmp, tmp, tmp1); +tcg_gen_not_i32(mask, mask); +tcg_gen_and_i32(reg, reg, mask); +tcg_gen_or_i32(reg, reg, tmp); +break; /* Rotate the source so that the field is at msb. */ tcg_gen_rotr_i32(tmp, reg2, width); /* Isolate the field and set flags. */ tcg_gen_and_i32(tmp, tmp, mask_msb); gen_logic_cc(s, tmp, OS_LONG); /* Rotate the field into position. */ tcg_gen_rotr_i32(tmp, tmp, offset); /* Merge field into destination. */ tcg_gen_andc_i32(reg, reg, mask_inp); tcg_gen_or_i32(reg, reg, tmp); return; Handle the non-bfins after the switch with gen_logic_cc(s, oldf_msb, OS_LONG); +DISAS_INSN(bitfield_mem) +{ +uint16_t ext; +int op; +TCGv_i64 bitfield; +TCGv_i64 mask_bitfield; +TCGv mask_cc; +TCGv shift; +TCGv val; +TCGv src; +TCGv offset; +TCGv width; +TCGv reg; +TCGv tmp; + +op = (insn 8) 7; +ext = read_im16(env, s); +src = gen_lea(env, s, insn, OS_LONG); +if (IS_NULL_QREG(src)) { +gen_addr_fault(s); +return; +} + +bitfield_param(ext, offset, width,
Re: [Qemu-devel] [RFC 1/3] monitor: split MonitorQAPIEventState
Hi - Original Message - On 08/12/15 21:46, marcandre.lur...@redhat.com wrote: From: Marc-André Lureau marcandre.lur...@redhat.com Create a seperate pending event structure MonitorQAPIEventPending. Use a MonitorQAPIEventDelay callback to handle the delaying. This allows other implementations of throttling. Signed-off-by: Marc-André Lureau marcandre.lur...@redhat.com --- monitor.c| 124 +-- trace-events | 2 +- 2 files changed, 79 insertions(+), 47 deletions(-) Assume there has been a long period of silence (no attempts to emit an event). Now some client code makes a call to emit the event. Will that event be emitted immediately, or will it be delayed to see if more are coming? I'd like to understand this aspect first. I think the first instance of the event, after the grace period, should be emitted immediately, and further instances that quickly follow should be suppressed. This is what qemu already does. The first event is sent immediately, the later ones may be delayed (but there will be at least one event every period, if the event is flooded). This patch 1/3 doesn't change the logic, only it split things to make them a bit more modular. So the rest of the patches do not change the qemu delay logic, it adds a way to delay based on (event + id) instead of just (event). It does that by adding an additional id hashtable for the event type. My hope is that this apporach could be reused if other field or combinations of fields are necessary for other events, but for now, it's hardcoded for id. cheers
Re: [Qemu-devel] [PATCH for-2.5 08/30] m68k: update CPU flags management
Le 12/08/2015 23:19, Richard Henderson a écrit : On 08/12/2015 01:56 PM, Laurent Vivier wrote: -DEF_HELPER_2(flush_flags, void, env, i32) +DEF_HELPER_2(flush_flags, i32, env, i32) Modify to use DEF_HELPER_FLAGS while you're at it. At the moment it reads some globals, but doesn't write any, or have any other side effects. It writes env-cc_x, so I guess I can't use DEF_HELPER_FLAGS ? Ah, missed that. So, no, not usefully. That const needs to be freed. perhaps I'm wrong, what I had understood is: tcg_const_i32() creates a tcg_temp_new_i32(), and tcg_temp_new_i32() are automatically freed at end of tcg block (whereas tcg_const_local adn tcg_temp_local are not). They are freed at the end of a basic block. But the total number of temps affects the speed of the tcg code generator. So you can improve the speed of qemu by freeing temporaries that are no longer needed. OK, thank you. I was wondering if it is useful to free temp or not... Laurent
[Qemu-devel] [PATCH] hw/misc: Add support for ADC controller in Xilinx Zynq 7000
Add support for the Xilinx XADC core used in Zynq 7000. References: - Zynq-7000 All Programmable SoC Technical Reference Manual - 7 Series FPGAs and Zynq-7000 All Programmable SoC XADC Dual 12-Bit 1 MSPS Analog-to-Digital Converter Tested with Linux using qemu machine xilinx-zynq-a9 with devicetree files zynq-zc702.dtb and zynq-zc706.dtb, and kernel configuration multi_v7_defconfig. Signed-off-by: Guenter Roeck li...@roeck-us.net --- hw/arm/xilinx_zynq.c | 5 + hw/misc/Makefile.objs | 1 + hw/misc/zynq_xadc.c | 305 ++ 3 files changed, 311 insertions(+) create mode 100644 hw/misc/zynq_xadc.c diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c index a4e7b5c..195e9f1 100644 --- a/hw/arm/xilinx_zynq.c +++ b/hw/arm/xilinx_zynq.c @@ -225,6 +225,11 @@ static void zynq_init(MachineState *machine) sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0101000); sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[79-IRQ_OFFSET]); +dev = qdev_create(NULL, xilinx,zynq_xadc); +qdev_init_nofail(dev); +sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8007100); +sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[39-IRQ_OFFSET]); + dev = qdev_create(NULL, pl330); qdev_prop_set_uint8(dev, num_chnls, 8); qdev_prop_set_uint8(dev, num_periph_req, 4); diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs index 4aa76ff..5f76f05 100644 --- a/hw/misc/Makefile.objs +++ b/hw/misc/Makefile.objs @@ -36,6 +36,7 @@ obj-$(CONFIG_OMAP) += omap_sdrc.o obj-$(CONFIG_OMAP) += omap_tap.o obj-$(CONFIG_SLAVIO) += slavio_misc.o obj-$(CONFIG_ZYNQ) += zynq_slcr.o +obj-$(CONFIG_ZYNQ) += zynq_xadc.o obj-$(CONFIG_STM32F2XX_SYSCFG) += stm32f2xx_syscfg.o obj-$(CONFIG_PVPANIC) += pvpanic.o diff --git a/hw/misc/zynq_xadc.c b/hw/misc/zynq_xadc.c new file mode 100644 index 000..480e2bf --- /dev/null +++ b/hw/misc/zynq_xadc.c @@ -0,0 +1,305 @@ +/* + * ADC registers for Xilinx Zynq Platform + * + * Copyright (c) 2015 Guenter Roeck + * Based on hw/misc/zynq_slcr.c, written by Michal Simek + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version + * 2 of the License, or (at your option) any later version. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, see http://www.gnu.org/licenses/. + */ + +#include hw/hw.h +#include qemu/timer.h +#include hw/sysbus.h +#include sysemu/sysemu.h + +enum { +CFG= 0x000 / 4, +INTSTS, +INTMSK, +STATUS, +CFIFO, +DFIFO, +CTL, +}; + +#define XADC_ZYNQ_CFG_ENABLEBIT(31) +#define XADC_ZYNQ_CFG_CFIFOTH_RD(x) (((x) 20) 0x0f) +#define XADC_ZYNQ_CFG_DFIFOTH_RD(x) (((x) 16) 0x0f) +#define XADC_ZYNQ_CFG_WEDGE BIT(13) +#define XADC_ZYNQ_CFG_REDGE BIT(12) +#define XADC_ZYNQ_CFG_TCKRATE_DIV2 (0x0 8) +#define XADC_ZYNQ_CFG_TCKRATE_DIV4 (0x1 8) +#define XADC_ZYNQ_CFG_TCKRATE_DIV8 (0x2 8) +#define XADC_ZYNQ_CFG_TCKRATE_DIV16 (0x3 8) +#define XADC_ZYNQ_CFG_IGAP_MASK 0x1f +#define XADC_ZYNQ_CFG_IGAP(x) ((x) XADC_ZYNQ_CFG_IGAP_MASK) + +#define XADC_ZYNQ_INT_CFIFO_LTH BIT(9) +#define XADC_ZYNQ_INT_DFIFO_GTH BIT(8) +#define XADC_ZYNQ_INT_ALARM_MASK0xff +#define XADC_ZYNQ_INT_ALARM_OFFSET 0 + +#define XADC_ZYNQ_STATUS_DFIFO_LVL(x) (((x) 0x0f) 12) +#define XADC_ZYNQ_STATUS_CFIFOF BIT(11) +#define XADC_ZYNQ_STATUS_CFIFOE BIT(10) +#define XADC_ZYNQ_STATUS_DFIFOF BIT(9) +#define XADC_ZYNQ_STATUS_DFIFOE BIT(8) +#define XADC_ZYNQ_STATUS_OT BIT(7) +#define XADC_ZYNQ_STATUS_ALM(x) BIT(x) + +#define XADC_ZYNQ_CTL_RESET BIT(4) + +#define XADC_ZYNQ_CMD_NOP 0x00 +#define XADC_ZYNQ_CMD_READ 0x01 +#define XADC_ZYNQ_CMD_WRITE 0x02 + +#define ZYNQ_XADC_MMIO_SIZE 0x0020 +#define ZYNQ_XADC_NUM_IO_REGS (ZYNQ_XADC_MMIO_SIZE / 4) +#define ZYNQ_XADC_NUM_ADC_REGS 128 +#define ZYNQ_XADC_FIFO_DEPTH15 + +#define TYPE_ZYNQ_XADC xilinx,zynq_xadc +#define ZYNQ_XADC(obj) \ +OBJECT_CHECK(ZynqXADCState, (obj), TYPE_ZYNQ_XADC) + +typedef struct ZynqXADCState { +SysBusDevice parent_obj; + +MemoryRegion iomem; + +uint32_t regs[ZYNQ_XADC_NUM_IO_REGS]; /* I/O registers */ +uint16_t xadc_regs[ZYNQ_XADC_NUM_ADC_REGS]; /* XADC registers */ +uint16_t xadc_read_reg_previous;/* result of most recent read command */ +uint16_t xadc_dfifo[ZYNQ_XADC_FIFO_DEPTH]; /* data fifo */ +uint16_t xadc_dfifo_depth; /* entries in data fifo */ + +struct IRQState *irq; + +} ZynqXADCState; + +static void _zynq_xadc_reset(ZynqXADCState *s) +{ +int i; + +s-regs[CFG] = XADC_ZYNQ_CFG_IGAP(0x14) | XADC_ZYNQ_CFG_TCKRATE_DIV4 | +XADC_ZYNQ_CFG_REDGE; +
Re: [Qemu-devel] [PATCH for-2.5 20/30] m68k: add exg
Le 12/08/2015 19:05, Richard Henderson a écrit : On 08/09/2015 01:13 PM, Laurent Vivier wrote: Signed-off-by: Laurent Vivier laur...@vivier.eu --- target-m68k/translate.c | 34 +- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/target-m68k/translate.c b/target-m68k/translate.c index adf4521..b7d15e9 100644 --- a/target-m68k/translate.c +++ b/target-m68k/translate.c @@ -2035,10 +2035,42 @@ DISAS_INSN(and) TCGv dest; TCGv addr; int opsize; +int exg_mode; +dest = tcg_temp_new(); + +/* exg */ + +exg_mode = insn 0x1f8; Likewise, surely we can decode EXG separately from AND, and avoid doing so for coldfire. I agree for CMPM, not for EXG. Let's have a look to instructions encoding :) AND 1100dddooommmrrr ddd data register number ooo opmode, invalid: 011, 111 mmmrrrea mode, if ooo = { 000, 001, 010} invalid: 001000 .. 00 if ooo = { 100, 101, 110} invalid; 00 .. 00 EXG 1100xxx1oyyy xxx register o valid: 01000, 01001, 10001 yyy register So, EXG is an AND with ooo 101, 110 mmm 000, 001 which are invalid combinations for AND. IMHO, EXG looks like a wart on the AND and should be decoded like that... I don't know how to add this easily in the table... except by adding 3 entries to decode 1 instruction. Is it acceptable ? Laurent
Re: [Qemu-devel] [PATCH for-2.5 20/30] m68k: add exg
On 08/12/2015 03:43 PM, Laurent Vivier wrote: Le 12/08/2015 19:05, Richard Henderson a écrit : On 08/09/2015 01:13 PM, Laurent Vivier wrote: Signed-off-by: Laurent Vivier laur...@vivier.eu --- target-m68k/translate.c | 34 +- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/target-m68k/translate.c b/target-m68k/translate.c index adf4521..b7d15e9 100644 --- a/target-m68k/translate.c +++ b/target-m68k/translate.c @@ -2035,10 +2035,42 @@ DISAS_INSN(and) TCGv dest; TCGv addr; int opsize; +int exg_mode; +dest = tcg_temp_new(); + +/* exg */ + +exg_mode = insn 0x1f8; Likewise, surely we can decode EXG separately from AND, and avoid doing so for coldfire. I agree for CMPM, not for EXG. Let's have a look to instructions encoding :) AND 1100dddooommmrrr ddd data register number ooo opmode, invalid: 011, 111 mmmrrrea mode, if ooo = { 000, 001, 010} invalid: 001000 .. 00 if ooo = { 100, 101, 110} invalid; 00 .. 00 EXG 1100xxx1oyyy xxx register o valid: 01000, 01001, 10001 yyy register So, EXG is an AND with ooo 101, 110 mmm 000, 001 which are invalid combinations for AND. IMHO, EXG looks like a wart on the AND and should be decoded like that... Hmm, perhaps you're right. On the other hand, maybe we should rename the function and_exg, and also properly check for M68000 before accepting exg? r~
Re: [Qemu-devel] [PATCH for-2.5 20/30] m68k: add exg
Le 13/08/2015 01:09, Richard Henderson a écrit : On 08/12/2015 03:43 PM, Laurent Vivier wrote: Le 12/08/2015 19:05, Richard Henderson a écrit : On 08/09/2015 01:13 PM, Laurent Vivier wrote: Signed-off-by: Laurent Vivier laur...@vivier.eu --- target-m68k/translate.c | 34 +- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/target-m68k/translate.c b/target-m68k/translate.c index adf4521..b7d15e9 100644 --- a/target-m68k/translate.c +++ b/target-m68k/translate.c @@ -2035,10 +2035,42 @@ DISAS_INSN(and) TCGv dest; TCGv addr; int opsize; +int exg_mode; +dest = tcg_temp_new(); + +/* exg */ + +exg_mode = insn 0x1f8; Likewise, surely we can decode EXG separately from AND, and avoid doing so for coldfire. I agree for CMPM, not for EXG. Let's have a look to instructions encoding :) AND 1100dddooommmrrr ddd data register number ooo opmode, invalid: 011, 111 mmmrrrea mode, if ooo = { 000, 001, 010} invalid: 001000 .. 00 if ooo = { 100, 101, 110} invalid; 00 .. 00 EXG 1100xxx1oyyy xxx register o valid: 01000, 01001, 10001 yyy register So, EXG is an AND with ooo 101, 110 mmm 000, 001 which are invalid combinations for AND. IMHO, EXG looks like a wart on the AND and should be decoded like that... Hmm, perhaps you're right. On the other hand, maybe we should rename the function and_exg, and also properly check for M68000 before accepting exg? I agree. Thank you for all your comments. Laurent
Re: [Qemu-devel] [PATCH for-2.5 27/30] m68k: add addx/subx/negx
Le 12/08/2015 20:46, Richard Henderson a écrit : On 08/09/2015 01:13 PM, Laurent Vivier wrote: +return (op1 ~((1UL bits) - 1)) | res; \ deposit32(op1, res, bits, 0) You mean: deposit32(op1, 0, bits, res) ? Laurent
Re: [Qemu-devel] [PATCH for-2.5 08/30] m68k: update CPU flags management
On 08/12/2015 01:56 PM, Laurent Vivier wrote: -DEF_HELPER_2(flush_flags, void, env, i32) +DEF_HELPER_2(flush_flags, i32, env, i32) Modify to use DEF_HELPER_FLAGS while you're at it. At the moment it reads some globals, but doesn't write any, or have any other side effects. It writes env-cc_x, so I guess I can't use DEF_HELPER_FLAGS ? Ah, missed that. So, no, not usefully. That const needs to be freed. perhaps I'm wrong, what I had understood is: tcg_const_i32() creates a tcg_temp_new_i32(), and tcg_temp_new_i32() are automatically freed at end of tcg block (whereas tcg_const_local adn tcg_temp_local are not). They are freed at the end of a basic block. But the total number of temps affects the speed of the tcg code generator. So you can improve the speed of qemu by freeing temporaries that are no longer needed. r~
Re: [Qemu-devel] [PATCH v4 1/3] linux-headers: Add eeh.h
On Wed, Aug 12, 2015 at 12:00:35PM +1000, Alexey Kardashevskiy wrote: On 08/11/2015 07:11 PM, Peter Maydell wrote: On 10 August 2015 at 08:13, Gavin Shan gws...@linux.vnet.ibm.com wrote: The header file was introduced by following Linux upstream commits: commit ed3e81f (powerpc/eeh: Move PE state constants around) commit ec33d36 (powerpc/eeh: Introduce eeh_pe_inject_err()) Signed-off-by: Gavin Shan gws...@linux.vnet.ibm.com --- linux-headers/asm-powerpc/eeh.h | 56 + 1 file changed, 56 insertions(+) create mode 100644 linux-headers/asm-powerpc/eeh.h Shouldn't this be added by updating scripts/update-linux-headers.sh and then doing a plain synchronize headers against kernel version $X ? I also thought this is the protocol but then I looked into the git history and discovered this is not always the case :) So should I use scripts/update-linux-headers.sh or what I had is fine? Otherwise you won't get any future changes to this file. Thanks, Gavin
Re: [Qemu-devel] [PATCH for-2.5 00/30] 680x0 instructions emulation
On 08/09/2015 01:13 PM, Laurent Vivier wrote: m68k: allow to update flags with operation on words and bytes m68k: update CPU flags management m68k: add X flag helpers I wonder if we can talk about a different mechanism for tracking flags. The x86 scheme tracks flags with 3 words: { op, dest, src }. This is easy to handle arithmetic, logicals, and comparisons. But shifts and other weird things require complex helpers. The m68k scheme, which you're enhancing, is the same, except with an additional word for X. It's this extra X flag that complicates things. The arm scheme tracks 4 flags with 4 individual words. This is more complicated for arithmetic (especially computing overflow), but it rarely requires complicated helpers. I propose using a hybrid between the two, using 6 words: { op, c, v, z, n, x }. op = OP_FLAGS (i.e. fully computed state) X = cc_x is 0/1 C = cc_c is 0/1 V = cc_v 0 N = cc_n 0 Z = cc_z == 0 op = OP_ADD X = C = cc_x is 0/1 cc_n = result cc_v = src1 cc_c = unused cc_z = unused N = cc_n 0 Z = cc_n == 0 V = ((result ^ src1) ~((result - src1) ^ src1)) 0 op = OP_SUB like OP_ADD, except V = ((result ^ src1) (src1 ^ (src1 - result))) 0 op = OP_LOGIC X = cc_x is 0/1 C = 0 V = 0 cc_n = result N = cc_n 0 Z = cc_n == 0 All of the byte and word variants are handled by sign-extending the quantities. This keeps X up-to-date at all times. It minimizes the amount to data that needs to be synced back to env before memory operations. It leaves enough data to easily compute signed comparisons. The more complicated cases of addx and shifts, now simply operate in OP_FLAGS mode and don't require helpers. I'll follow up with some patches... r~
Re: [Qemu-devel] [PATCH] linux-user: elfload: Still use TARGET_PAGE_SIZE for i386 guest
Am 13.08.2015 um 04:45 schrieb gchen gchen xili_gchen_5...@hotmail.com: On 2015年08月12日 23:06, Richard Henderson wrote: On 08/12/2015 12:59 AM, gchen gchen wrote: Nack. There's 99 problems with host page size guest page size. This solves none of them, and in the hackiest way possible. Under alpha virtual machine, if set i386 guest page size 8KB, it will cause failure directly (any dynamically linked binaries can not work). Yes, I know. The same thing happens when running i386 guests on other (non-virtual) hosts. E.g. Sparc64's 8kB page, PowerPC64's 64kB page. Yes. The reason why I am only focus on Alpha is the machine which I am working for is almost the same as Alpha. But this machine is very slow, its performance maybe like 10 years ago's x86_64 laptop. What does Almost the same as Alpha mean? Does it use 8k or 4k page size? Do you have any other ideas for solving this issue? The only complete solution that I see is to use softmmu with linux-user, so that we properly emulate the guest pages. Yes, it will cause quite some slow-down in emulation, but I believe it's the only reliable way. I have tried softmmu, for me, the performance is not acceptable, we can not use this way. Softmmu but without system emulation. Performance should be about half of today's linux-user. Alex
Re: [Qemu-devel] [PATCH v6 0/2] vhost user: Add live migration
-Original Message- From: Marc-André Lureau [mailto:marcandre.lur...@gmail.com] Sent: Wednesday, August 12, 2015 6:07 PM To: Michael S. Tsirkin Cc: Thibaut Collet; QEMU; stefa...@redhat.com; Jason Wang; Paolo Bonzini; Linhaifeng; Ouyang, Changchun Subject: Re: [PATCH v6 0/2] vhost user: Add live migration Hi On Wed, Aug 12, 2015 at 9:25 AM, Michael S. Tsirkin m...@redhat.com wrote: I think these patches need to be rebased on top of Marc Andre's ones, and use protocol flags to negotiate capabilities. Right? Correct. His patches should be applied before my migration tests, though. Good point, do we have plan how to make these patches applied serially? M.s.t's protocol feature patch, marc andre's patch, Thibaut's patch for adding live migration, and my patch for vhost-user multi queue. -- Marc-André Lureau
Re: [Qemu-devel] Qemu-devel Digest, Vol 149, Issue 266
-Original Message- Date: Wed, 12 Aug 2015 14:15:54 +0300 From: Michael S. Tsirkin m...@redhat.com To: Marcel Apfelbaum mar...@redhat.com Cc: qemu-devel@nongnu.org Subject: Re: [Qemu-devel] [PATCH] virtio/vhost: drop unnecessary VHOST_SET_VRING call Message-ID: 20150812141448-mutt-send-email-...@redhat.com Content-Type: text/plain; charset=us-ascii On Wed, Aug 12, 2015 at 02:10:56PM +0300, Marcel Apfelbaum wrote: On 08/12/2015 01:34 PM, Michael S. Tsirkin wrote: On Wed, Aug 12, 2015 at 01:19:51PM +0300, Marcel Apfelbaum wrote: No need to send VHOST_SET_VRING_CALL to backend before the negotiation with the guest is finished. Signed-off-by: Marcel Apfelbaum mar...@redhat.com Well - we do need to set it to the masked notifier initially to avoid losing events. You can't just drop it - need to move this call somewhere else. Agree with m.s.t. We could not drop it. Vhost-user multi queue also need this. What do we need to set? I just dropped the call to VHOST_SET_VRING_CALL. Thanks, Marcel We use two eventfds: masked and unmasked one. We switch dynamically dependent on msi mask value. Code assumes we start out masked, so we need to match that. --- hw/virtio/vhost.c | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 2712c6f..b448542 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -875,24 +875,13 @@ static void vhost_eventfd_del(MemoryListener *listener, static int vhost_virtqueue_init(struct vhost_dev *dev, struct vhost_virtqueue *vq, int n) { -struct vhost_vring_file file = { -.index = n, -}; int r = event_notifier_init(vq-masked_notifier, 0); + if (r 0) { return r; } -file.fd = event_notifier_get_fd(vq-masked_notifier); -r = dev-vhost_ops-vhost_call(dev, VHOST_SET_VRING_CALL, file); -if (r) { -r = -errno; -goto fail_call; -} return 0; -fail_call: -event_notifier_cleanup(vq-masked_notifier); -return r; } static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq) -- 2.1.0
[Qemu-devel] [v2 1/4] migration: do cleanup operation after completion
Because of the patch 3ea3b7fa9af067982f34b of kvm, now the migration_end() is a time consuming operation, which takes about dozens of milliseconds, and will prolong VM downtime. Such an operation should be done after migration completion. For a VM with 8G RAM, this patch can reduce the VM downtime about 32 ms during live migration. Signed-off-by: Liang Li liang.z...@intel.com Reviewed-by: Paolo Bonzini pbonz...@redhat.com --- migration/block.c | 1 - migration/migration.c | 14 +++--- migration/ram.c | 1 - 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/migration/block.c b/migration/block.c index ed865ed..85496fd 100644 --- a/migration/block.c +++ b/migration/block.c @@ -750,7 +750,6 @@ static int block_save_complete(QEMUFile *f, void *opaque) qemu_put_be64(f, BLK_MIG_FLAG_EOS); -blk_mig_cleanup(); return 0; } diff --git a/migration/migration.c b/migration/migration.c index 662e77e..4ddb9ad 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -560,12 +560,9 @@ static void migrate_fd_cleanup(void *opaque) assert(s-state != MIGRATION_STATUS_ACTIVE); -if (s-state != MIGRATION_STATUS_COMPLETED) { -qemu_savevm_state_cancel(); -if (s-state == MIGRATION_STATUS_CANCELLING) { -migrate_set_state(s, MIGRATION_STATUS_CANCELLING, - MIGRATION_STATUS_CANCELLED); -} +if (s-state == MIGRATION_STATUS_CANCELLING) { +migrate_set_state(s, MIGRATION_STATUS_CANCELLING, + MIGRATION_STATUS_CANCELLED); } notifier_list_notify(migration_state_notifiers, s); @@ -923,6 +920,7 @@ static void *migration_thread(void *opaque) int64_t initial_bytes = 0; int64_t max_size = 0; int64_t start_time = initial_time; +int64_t end_time; bool old_vm_running = false; rcu_register_thread(); @@ -1007,9 +1005,11 @@ static void *migration_thread(void *opaque) } } +end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); + qemu_mutex_lock_iothread(); +qemu_savevm_state_cancel(); if (s-state == MIGRATION_STATUS_COMPLETED) { -int64_t end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); uint64_t transferred_bytes = qemu_ftell(s-file); s-total_time = end_time - s-total_time; s-downtime = end_time - start_time; diff --git a/migration/ram.c b/migration/ram.c index 7f007e6..6249f6e 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -1269,7 +1269,6 @@ static int ram_save_complete(QEMUFile *f, void *opaque) rcu_read_unlock(); -migration_end(); qemu_put_be64(f, RAM_SAVE_FLAG_EOS); return 0; -- 1.9.1
[Qemu-devel] [v2 3/4] migration: rename cancel to cleanup in SaveVMHandles
'cleanup' seems more appropriate than 'cancel'. Signed-off-by: Liang Li liang.z...@intel.com --- include/migration/vmstate.h | 2 +- migration/block.c | 2 +- migration/ram.c | 2 +- migration/savevm.c | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 2e5a97d..24b939c 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -39,7 +39,7 @@ typedef struct SaveVMHandlers { void (*set_params)(const MigrationParams *params, void * opaque); SaveStateHandler *save_state; -void (*cancel)(void *opaque); +void (*cleanup)(void *opaque); int (*save_live_complete)(QEMUFile *f, void *opaque); /* This runs both outside and inside the iothread lock. */ diff --git a/migration/block.c b/migration/block.c index 85496fd..55442bd 100644 --- a/migration/block.c +++ b/migration/block.c @@ -879,7 +879,7 @@ static SaveVMHandlers savevm_block_handlers = { .save_live_complete = block_save_complete, .save_live_pending = block_save_pending, .load_state = block_load, -.cancel = block_migration_cancel, +.cleanup = block_migration_cancel, .is_active = block_is_active, }; diff --git a/migration/ram.c b/migration/ram.c index 6249f6e..e7f711e 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -1610,7 +1610,7 @@ static SaveVMHandlers savevm_ram_handlers = { .save_live_complete = ram_save_complete, .save_live_pending = ram_save_pending, .load_state = ram_load, -.cancel = ram_migration_cancel, +.cleanup = ram_migration_cancel, }; void ram_mig_init(void) diff --git a/migration/savevm.c b/migration/savevm.c index b141b17..96d0aab 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -911,8 +911,8 @@ void qemu_savevm_state_cleanup(void) trace_savevm_state_cleanup(); QTAILQ_FOREACH(se, savevm_state.handlers, entry) { -if (se-ops se-ops-cancel) { -se-ops-cancel(se-opaque); +if (se-ops se-ops-cleanup) { +se-ops-cleanup(se-opaque); } } } -- 1.9.1
[Qemu-devel] [v2 2/4] migration: rename qemu_savevm_state_cancel
The function qemu_savevm_state_cancel is called after the migration in migration_thread, it seems strange to 'cancel' it after completion, rename it to qemu_savevm_state_cleanup looks better. Signed-off-by: Liang Li liang.z...@intel.com --- include/sysemu/sysemu.h | 2 +- migration/migration.c | 2 +- migration/savevm.c | 6 +++--- trace-events| 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index 44570d1..9cc0240 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -88,7 +88,7 @@ void qemu_savevm_state_begin(QEMUFile *f, void qemu_savevm_state_header(QEMUFile *f); int qemu_savevm_state_iterate(QEMUFile *f); void qemu_savevm_state_complete(QEMUFile *f); -void qemu_savevm_state_cancel(void); +void qemu_savevm_state_cleanup(void); uint64_t qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size); int qemu_loadvm_state(QEMUFile *f); diff --git a/migration/migration.c b/migration/migration.c index 4ddb9ad..1ff30fe 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1008,7 +1008,7 @@ static void *migration_thread(void *opaque) end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); qemu_mutex_lock_iothread(); -qemu_savevm_state_cancel(); +qemu_savevm_state_cleanup(); if (s-state == MIGRATION_STATUS_COMPLETED) { uint64_t transferred_bytes = qemu_ftell(s-file); s-total_time = end_time - s-total_time; diff --git a/migration/savevm.c b/migration/savevm.c index 6071215..b141b17 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -905,11 +905,11 @@ uint64_t qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size) return ret; } -void qemu_savevm_state_cancel(void) +void qemu_savevm_state_cleanup(void) { SaveStateEntry *se; -trace_savevm_state_cancel(); +trace_savevm_state_cleanup(); QTAILQ_FOREACH(se, savevm_state.handlers, entry) { if (se-ops se-ops-cancel) { se-ops-cancel(se-opaque); @@ -946,7 +946,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp) ret = qemu_file_get_error(f); } if (ret != 0) { -qemu_savevm_state_cancel(); +qemu_savevm_state_cleanup(); error_setg_errno(errp, -ret, Error while writing VM state); } return ret; diff --git a/trace-events b/trace-events index 94bf3bb..102373c 100644 --- a/trace-events +++ b/trace-events @@ -1195,7 +1195,7 @@ savevm_state_begin(void) savevm_state_header(void) savevm_state_iterate(void) savevm_state_complete(void) -savevm_state_cancel(void) +savevm_state_cleanup(void) vmstate_save(const char *idstr, const char *vmsd_name) %s, %s vmstate_load(const char *idstr, const char *vmsd_name) %s, %s qemu_announce_self_iter(const char *mac) %s -- 1.9.1
[Qemu-devel] [v2 4/4] migration: code clean up
Just clean up code, no behavior change. Signed-off-by: Liang Li liang.z...@intel.com --- migration/block.c | 9 ++--- migration/ram.c | 9 ++--- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/migration/block.c b/migration/block.c index 55442bd..869798c 100644 --- a/migration/block.c +++ b/migration/block.c @@ -591,7 +591,7 @@ static int64_t get_remaining_dirty(void) /* Called with iothread lock taken. */ -static void blk_mig_cleanup(void) +static void block_migration_cleanup(void *opaque) { BlkMigDevState *bmds; BlkMigBlock *blk; @@ -618,11 +618,6 @@ static void blk_mig_cleanup(void) blk_mig_unlock(); } -static void block_migration_cancel(void *opaque) -{ -blk_mig_cleanup(); -} - static int block_save_setup(QEMUFile *f, void *opaque) { int ret; @@ -879,7 +874,7 @@ static SaveVMHandlers savevm_block_handlers = { .save_live_complete = block_save_complete, .save_live_pending = block_save_pending, .load_state = block_load, -.cleanup = block_migration_cancel, +.cleanup = block_migration_cleanup, .is_active = block_is_active, }; diff --git a/migration/ram.c b/migration/ram.c index e7f711e..8b045fe 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -1024,7 +1024,7 @@ void free_xbzrle_decoded_buf(void) xbzrle_decoded_buf = NULL; } -static void migration_end(void) +static void ram_migration_cleanup(void *opaque) { /* caller have hold iothread lock or is in a bh, so there is * no writing race against this migration_bitmap @@ -1049,11 +1049,6 @@ static void migration_end(void) XBZRLE_cache_unlock(); } -static void ram_migration_cancel(void *opaque) -{ -migration_end(); -} - static void reset_ram_globals(void) { last_seen_block = NULL; @@ -1610,7 +1605,7 @@ static SaveVMHandlers savevm_ram_handlers = { .save_live_complete = ram_save_complete, .save_live_pending = ram_save_pending, .load_state = ram_load, -.cleanup = ram_migration_cancel, +.cleanup = ram_migration_cleanup, }; void ram_mig_init(void) -- 1.9.1
[Qemu-devel] [v2 0/4] Fix long vm downtime during live migration
Some cleanup operations take long time during the pause and copy stage, especially with the KVM patch 3ea3b7fa9af067, do these operations after the completion of live migration can help to reduce VM downtime. Ony the first patch changes the behavior, the rest 3 patches are for code cleanup. Changes: * Remove qemu_savevm_sate_cancel() in migrate_fd_cleanup() * Add 2 more patches for code clean up Liang Li (4): migration: do cleanup operation after completion migration: rename qemu_savevm_state_cancel migration: rename cancel to cleanup in SaveVMHandles migration: code clean up include/migration/vmstate.h | 2 +- include/sysemu/sysemu.h | 2 +- migration/block.c | 10 ++ migration/migration.c | 14 +++--- migration/ram.c | 10 ++ migration/savevm.c | 10 +- trace-events| 2 +- 7 files changed, 19 insertions(+), 31 deletions(-) -- 1.9.1
[Qemu-devel] about the patch kvmclock Ensure proper env-tsc value for kvmclock_current_nsec calculation
Hi Paolo Marcelo, Could please point out what issue the patch 317b0a6d8ba44e try to fix? I found in live migration the cpu_synchronize_all_states will be called twice, and it will take more than 1 ms sometimes. I try to do some optimization but lack the knowledge about the background. Thanks Liang
Re: [Qemu-devel] [PATCH for-2.5 27/30] m68k: add addx/subx/negx
On 08/12/2015 05:11 PM, Laurent Vivier wrote: Le 12/08/2015 20:46, Richard Henderson a écrit : On 08/09/2015 01:13 PM, Laurent Vivier wrote: +return (op1 ~((1UL bits) - 1)) | res; \ deposit32(op1, res, bits, 0) You mean: deposit32(op1, 0, bits, res) ? Yes, sorry. The operand order is different for tcg_gen_deposit_i32. ;-) r~
Re: [Qemu-devel] [PATCH] monitor: remove QAPI_EVENT_VSERPORT_CHANGE throttle
On (Tue) 11 Aug 2015 [20:21:18], Laszlo Ersek wrote: On 08/11/15 19:04, marcandre.lur...@redhat.com wrote: From: Marc-André Lureau marcandre.lur...@redhat.com QAPI_EVENT_VSERPORT_CHANGE reports changes of a virtio serial port state. However, the events may be for different ports, but the throttle mechanism may replace the event for a different port, since it only checks the event type. libvirt relies on a correct state to be reported for all channels: the qemu-ga commands may no longer work if the state is reported disconnected. This can be triggered easily by having more than 1 virtio-serial (qemu-ga + spice agent for example), and restarting quickly daemons or more realistically going quickly in and out of suspend. In a future patch, we may want to throttle events based on their arguments, but this will likely require dynamic allocations and more complicated code to insert/lookup pending events based on various arguments (id in QAPI_EVENT_VSERPORT_CHANGE case). Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1244064 Signed-off-by: Marc-André Lureau marcandre.lur...@redhat.com --- monitor.c | 1 - 1 file changed, 1 deletion(-) diff --git a/monitor.c b/monitor.c index aeea2b5..e4d56f7 100644 --- a/monitor.c +++ b/monitor.c @@ -558,7 +558,6 @@ static void monitor_qapi_event_init(void) monitor_qapi_event_throttle(QAPI_EVENT_BALLOON_CHANGE, 1000); monitor_qapi_event_throttle(QAPI_EVENT_QUORUM_REPORT_BAD, 1000); monitor_qapi_event_throttle(QAPI_EVENT_QUORUM_FAILURE, 1000); -monitor_qapi_event_throttle(QAPI_EVENT_VSERPORT_CHANGE, 1000); qmp_event_set_func_emit(monitor_qapi_event_queue); } I don't mind the change (and the point of argument sensitivity is not lost on me), but note that this undoes the protection that is spelled out in the leading comment of the function, not visible in the context here: /* Limit guest-triggerable events to 1 per second */ That was probably put in place in order to prevent a malicious guest from spamming the log files (and the CPU usage) of the management apps. One solution to that would be arg sensitivity. Another would be a burst-capable (ie. a slowly re-filling, limited size token bucket) throttle, maintained per event type. Until one of those gets written, I guess this patch is acceptable -- as long as mgmt people are okay with it. Daniel seems to be, so I don't mind. OK - so I'll queue it. Thanks, Amit
Re: [Qemu-devel] [PATCH v3 2/3] sPAPR: Support RTAS call ibm, {open, close}-errinjct
On Tue, Aug 11, 2015 at 11:54:08AM +1000, Alexey Kardashevskiy wrote: On 08/11/2015 10:57 AM, Gavin Shan wrote: On Mon, Aug 10, 2015 at 10:24:56PM +1000, David Gibson wrote: On Fri, Aug 07, 2015 at 01:33:32PM +1000, Gavin Shan wrote: The patch supports RTAS calls ibm,{open,close}-errinjct to manupliate the token, which is passed to RTAS call ibm,errinjct to indicate the valid context for error injection. Each VM is permitted to have only one token at once and we simply have one random number for that. Signed-off-by: Gavin Shan gws...@linux.vnet.ibm.com --- hw/ppc/spapr.c | 5 hw/ppc/spapr_rtas.c| 66 ++ include/hw/ppc/spapr.h | 10 +++- 3 files changed, 80 insertions(+), 1 deletion(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index dfd808f..1ebd0b2 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1225,6 +1225,11 @@ static const VMStateDescription vmstate_spapr = { VMSTATE_UINT64_TEST(rtc_offset, sPAPRMachineState, version_before_3), VMSTATE_PPC_TIMEBASE_V(tb, sPAPRMachineState, 2), + +/* Error injection token */ +VMSTATE_BOOL(is_errinjct_opened, sPAPRMachineState), +VMSTATE_UINT32(errinjct_next_token, sPAPRMachineState), Because you're adding fields to the vmstate you'll need to define a new version number and make these fields only considered in the new version. Agree, do we have an example for me to refer to? Technically it is enough to send a token which is not opened when it is zero. The chunk below demonstrates versions use. Agree. we don't have to track the token is opened or not. It's fine to have not opened state in the target VM. I'll integrate this piece of code into next revision. Thanks! diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index e046265..e568d41 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1213,17 +1213,21 @@ static bool version_before_3(void *opaque, int version_id) static const VMStateDescription vmstate_spapr = { .name = spapr, -.version_id = 3, +.version_id = 4, .minimum_version_id = 1, .post_load = spapr_post_load, .fields = (VMStateField[]) { /* used to be @next_irq */ VMSTATE_UNUSED_BUFFER(version_before_3, 0, 4), + /* RTC offset */ VMSTATE_UINT64_TEST(rtc_offset, sPAPRMachineState, version_before_3), VMSTATE_PPC_TIMEBASE_V(tb, sPAPRMachineState, 2), + +/* Error injection token */ +VMSTATE_UINT32_V(errinjct_next_token, sPAPRMachineState, 4), VMSTATE_END_OF_LIST() }, }; Technically you only need to transfer whether the token is open, and *if* it is open the current token value. Not sure if it's worth restricting to that though. It would be fine to transfer the token even it's closed, I think. Thanks, Gavin
Re: [Qemu-devel] [PATCH v4 1/3] linux-headers: Add eeh.h
On Thu, Aug 13, 2015 at 11:30:37AM +1000, Gavin Shan wrote: On Wed, Aug 12, 2015 at 12:00:35PM +1000, Alexey Kardashevskiy wrote: On 08/11/2015 07:11 PM, Peter Maydell wrote: On 10 August 2015 at 08:13, Gavin Shan gws...@linux.vnet.ibm.com wrote: The header file was introduced by following Linux upstream commits: commit ed3e81f (powerpc/eeh: Move PE state constants around) commit ec33d36 (powerpc/eeh: Introduce eeh_pe_inject_err()) Signed-off-by: Gavin Shan gws...@linux.vnet.ibm.com --- linux-headers/asm-powerpc/eeh.h | 56 + 1 file changed, 56 insertions(+) create mode 100644 linux-headers/asm-powerpc/eeh.h Shouldn't this be added by updating scripts/update-linux-headers.sh and then doing a plain synchronize headers against kernel version $X ? I also thought this is the protocol but then I looked into the git history and discovered this is not always the case :) So should I use scripts/update-linux-headers.sh or what I had is fine? Please use update-linux-headers.sh - and make sure you also update the script itself (if necessary) so it will pull in future updates to eeh.h -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson pgplWSfQxnsgz.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH] linux-user: elfload: Still use TARGET_PAGE_SIZE for i386 guest
On 2015年08月12日 23:06, Richard Henderson wrote:brgt; On 08/12/2015 12:59 AM, gchen gchen wrote:brgt;gt;gt; Nack. There's 99 problems with host page sizegt; guest page size. Thisbrgt;gt;gt; solves none of them, and in the hackiest way possible.brgt;gt;gt;brgt;gt;brgt;gt; Under alpha virtual machine, if set i386 guest page size 8KB, it willbrgt;gt; cause failure directly (any dynamically linked binaries can not work).brgt;brgt; Yes, I know. The same thing happens when running i386 guests on otherbrgt; (non-virtual) hosts. E.g. Sparc64's 8kB page, PowerPC64's 64kB page.brgt;brbrYes. The reason why I am only focus on Alpha is the machine which I ambrworking for is almost the same as Alpha. But this machine is very slow,brits performance maybe like 10 years ago's x86_64 laptop.brbrgt;gt; Do you have any other ideas for solving this issue?brgt;brgt; The only complete solution that I see is to use softmmu with linux-user, sobrgt; that we properly emulate the guest pages. Yes, it will cause quite somebrgt; slow-down in emulation, but I believe it's the only reliable way.brgt;brbrI have tried softmmu, for me, the performance is not acceptable, we canbrnot use this way.brbrOur main goal is let most of Windows XP graphic free programs (but notbropen source), can be used by user under Alpha like machine, also itsbrperformance and stability are acceptable.brbrbrThanks.br--brChen GangbrbrOpen, share, and attitude like air, water, and life which God blessedbr
[Qemu-devel] [Consult] linux-user: Let 8KB host support 4KB guest
Hello All: For my company, it is useful to let qemu linux-user 8KB host support 4KB guest, it may have many issues, but I want to try to fix them (with the aid from qemu members). So I want to consult: - Is it valuable and possible to fix these issues? (I guess, it is valuable and possible, but need much work). - How many resources do we need? (e.g. I can spend at least 1 months working days on it, with the aid from qemu members, is it enough?). - Can we describe all related issues ? (also describing each root cause will be better). Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] linux-user: elfload: Still use TARGET_PAGE_SIZE for i386 guest
On 2015年08月12日 23:06, Richard Henderson wrote: On 08/12/2015 12:59 AM, gchen gchen wrote: Nack. There's 99 problems with host page size guest page size. This solves none of them, and in the hackiest way possible. Under alpha virtual machine, if set i386 guest page size 8KB, it will cause failure directly (any dynamically linked binaries can not work). Yes, I know. The same thing happens when running i386 guests on other (non-virtual) hosts. E.g. Sparc64's 8kB page, PowerPC64's 64kB page. Yes. The reason why I am only focus on Alpha is the machine which I am working for is almost the same as Alpha. But this machine is very slow, its performance maybe like 10 years ago's x86_64 laptop. Do you have any other ideas for solving this issue? The only complete solution that I see is to use softmmu with linux-user, so that we properly emulate the guest pages. Yes, it will cause quite some slow-down in emulation, but I believe it's the only reliable way. I have tried softmmu, for me, the performance is not acceptable, we can not use this way. Our main goal is let most of Windows XP graphic free programs (but not open source), can be used by user under Alpha like machine, also its performance and stability are acceptable. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
[Qemu-devel] [PATCH] linux-user: elfload: Still use TARGET_PAGE_SIZE for i386 guest
Under Alpha host, for ubuntu12.04.5 i386 guest, it will cause failure: Invalid ELF image for this architecture. The related issue commit is a70daba linux-user: Tell guest about big host page sizes. Signed-off-by: Chen Gang gang.chen.5...@gmail.com --- linux-user/elfload.c | 4 1 file changed, 4 insertions(+) diff --git a/linux-user/elfload.c b/linux-user/elfload.c index 1788368..f4cf9b6 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -1614,7 +1614,11 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, NEW_AUX_ENT(AT_PHDR, (abi_ulong)(info-load_addr + exec-e_phoff)); NEW_AUX_ENT(AT_PHENT, (abi_ulong)(sizeof (struct elf_phdr))); NEW_AUX_ENT(AT_PHNUM, (abi_ulong)(exec-e_phnum)); +#ifdef TARGET_I386 +NEW_AUX_ENT(AT_PAGESZ, (abi_ulong)TARGET_PAGE_SIZE); +#else NEW_AUX_ENT(AT_PAGESZ, (abi_ulong)(MAX(TARGET_PAGE_SIZE, getpagesize(; +#endif NEW_AUX_ENT(AT_BASE, (abi_ulong)(interp_info ? interp_info-load_addr : 0)); NEW_AUX_ENT(AT_FLAGS, (abi_ulong)0); NEW_AUX_ENT(AT_ENTRY, info-entry); -- 1.9.1
Re: [Qemu-devel] [PATCH v6 0/2] vhost user: Add live migration
On Thu, Aug 06, 2015 at 10:45:07AM +0200, Thibaut Collet wrote: v5-v6 1. First patch: remove a warning log 2. Second patch: rename some functions to be more explicit on the purpose of these functions. The first patch provides limited live migration: - guest without GUEST_ANNOUNCE capabilities does not announce migration ending and peers talking to the migrated guest can suffer important network outage. - Some packets sent by remote peers to the guest can be lost during migration. The second patch fixes limitation for guest without GUEST_ANNOUNCE capabilities and patches from Marc Andre Lureau fix potential packet's lost during migration. Thibaut Collet (2): vhost user: add support of live migration vhost user: add rarp sending after live migration for legacy guest I think these patches need to be rebased on top of Marc Andre's ones, and use protocol flags to negotiate capabilities. Right? docs/specs/vhost-user.txt | 15 +++ hw/net/vhost_net.c| 18 ++ hw/virtio/vhost-backend.c |3 ++- hw/virtio/vhost-user.c| 32 ++-- include/hw/virtio/vhost-backend.h |2 ++ include/net/vhost_net.h |1 + net/vhost-user.c | 31 +-- 7 files changed, 97 insertions(+), 5 deletions(-) -- 1.7.10.4
Re: [Qemu-devel] [PATCH for-2.5 0/4] vhost: cleanups and switching to sorted memory map
On Tue, Jul 28, 2015 at 04:52:49PM +0200, Igor Mammedov wrote: making memory map a sorted array helps to simplify and speed up lookup/insertion and deletion ops on it. It also makes insertion/deteletion code easier to read. I'm a bit confused by all the vhost patches you sent. Is this series still something you want me to merge? Or was it the one that caused memory corruptions with mem hotplug? Igor Mammedov (4): vhost: codding style fix tab indents vhost: simplify/speedify vhost_dev_assign_memory() vhost: switch region lookup from linear to bsearch vhost: simplify/speedify vhost_dev_unassign_memory() hw/virtio/vhost.c | 252 ++ 1 file changed, 123 insertions(+), 129 deletions(-) -- 1.8.3.1
Re: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size
-Original Message- From: Jan Beulich [mailto:jbeul...@suse.com] Sent: Wednesday, August 12, 2015 4:43 PM To: Wu, Feng Cc: stefano.stabell...@eu.citrix.com; xen-de...@lists.xensource.com; qemu-devel@nongnu.org Subject: RE: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size On 12.08.15 at 09:10, feng...@intel.com wrote: -Original Message- From: qemu-devel-bounces+feng.wu=intel@nongnu.org [mailto:qemu-devel-bounces+feng.wu=intel@nongnu.org] On Behalf Of Jan Beulich Sent: Wednesday, August 12, 2015 2:59 PM To: Wu, Feng Cc: xen-de...@lists.xensource.com; qemu-devel@nongnu.org; stefano.stabell...@eu.citrix.com Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size On 05.08.15 at 04:02, feng...@intel.com wrote: @@ -491,8 +474,9 @@ static int xen_pt_bar_reg_write(XenPCIPassthroughState *s, XenPTReg *cfg_entry, bar_ro_mask = XEN_PT_BAR_IO_RO_MASK | (r_size - 1); break; case XEN_PT_BAR_FLAG_UPPER: +r = d-io_regions[index-1]; Perhaps worth an assert(index 0)? No problem, I will add it. BTW, do you have any other comments about this patch? If no, I am going to send out the new version with this changes. No - everything else looks to make sense (but continues to need testing). I don't have such a device in hand. Can anybody who has such a device help to test this patch? It would be highly appreciated! Thanks, Feng Jan
Re: [Qemu-devel] [PATCH] linux-user: elfload: Still use TARGET_PAGE_SIZE for i386 guest
On 08/11/2015 11:03 PM, gchen gchen wrote: Under Alpha host, for ubuntu12.04.5 i386 guest, it will cause failure: Invalid ELF image for this architecture. The related issue commit is a70daba linux-user: Tell guest about big host page sizes. Signed-off-by: Chen Ganggang.chen.5...@gmail.com --- linux-user/elfload.c | 4 1 file changed, 4 insertions(+) Nack. There's 99 problems with host page size guest page size. This solves none of them, and in the hackiest way possible. r~
Re: [Qemu-devel] [PATCH v2 2/6] hw/virtio/virtio-pci: Use pow2ceil() rather than hand-calculation
On Fri, Jul 24, 2015 at 01:33:08PM +0100, Peter Maydell wrote: Use the utility function pow2ceil() for rounding up to the next largest power of 2, rather than inline calculation. Signed-off-by: Peter Maydell peter.mayd...@linaro.org Reviewed-by: Michael S. Tsirkin m...@redhat.com --- hw/virtio/virtio-pci.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 283401a..845f52f 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1497,9 +1497,7 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) if (legacy) { size = VIRTIO_PCI_REGION_SIZE(proxy-pci_dev) + virtio_bus_get_vdev_config_len(bus); -if (size (size - 1)) { -size = 1 qemu_fls(size); -} +size = pow2ceil(size); memory_region_init_io(proxy-bar, OBJECT(proxy), virtio_pci_config_ops, -- 1.9.1
Re: [Qemu-devel] [PATCH for-2.5 12/30] m68k: Manage divw overflow
On 08/09/2015 01:13 PM, Laurent Vivier wrote: Overflow may be detected and set before the instruction completes. If the instruction detects an overflow, it sets the overflow condition code, and the operands are unaffected. May also implies may not. I presume this is important for matching hardware? Is there some program you know of that depends on this? +/* dest.l / src.w */ + +dest = DREG(insn, 9); +tcg_gen_mov_i32(QREG_DIV1, dest); + SRC_EA(env, src, OS_WORD, sign, NULL); tcg_gen_mov_i32(QREG_DIV2, src); + +/* div1 / div2 */ + if (sign) { gen_helper_divs(cpu_env, tcg_const_i32(1)); } else { gen_helper_divu(cpu_env, tcg_const_i32(1)); } +set_cc_op(s, CC_OP_FLAGS); + +l1 = gen_new_label(); +gen_jmpcc(s, 9 /* V */, l1); tmp = tcg_temp_new(); src = tcg_temp_new(); tcg_gen_ext16u_i32(tmp, QREG_DIV1); tcg_gen_shli_i32(src, QREG_DIV2, 16); -tcg_gen_or_i32(reg, tmp, src); -set_cc_op(s, CC_OP_FLAGS); +tcg_gen_or_i32(dest, tmp, src); +gen_set_label(l1); All that said, it's possible to implement this branch inside the helper via exception. Or simply return the inputs to effect no change. r~
[Qemu-devel] [PATCH v6 1/2] vhost-user: add multi queue support
Based on patch by Nikolay Nikolaev: Vhost-user will implement the multi queue support in a similar way to what vhost already has - a separate thread for each queue. To enable the multi queue functionality - a new command line parameter queues is introduced for the vhost-user netdev. The RESET_OWNER change is based on commit: 294ce717e0f212ed0763307f3eab72b4a1bdf4d0 If it is reverted, the patch need update for it accordingly. Signed-off-by: Nikolay Nikolaev n.nikol...@virtualopensystems.com Signed-off-by: Changchun Ouyang changchun.ouy...@intel.com --- Changes since v5: - fix the message descption for VHOST_RESET_OWNER in vhost-user txt Changes since v4: - remove the unnecessary trailing '\n' Changes since v3: - fix one typo and wrap one long line Changes since v2: - fix vq index issue for set_vring_call When it is the case of VHOST_SET_VRING_CALL, The vq_index is not initialized before it is used, thus it could be a random value. The random value leads to crash in vhost after passing down to vhost, as vhost use this random value to index an array index. - fix the typo in the doc and description - address vq index for reset_owner Changes since v1: - use s-nc.info_str when bringing up/down the backend docs/specs/vhost-user.txt | 7 ++- hw/net/vhost_net.c| 3 ++- hw/virtio/vhost-user.c| 11 ++- net/vhost-user.c | 37 - qapi-schema.json | 6 +- qemu-options.hx | 5 +++-- 6 files changed, 50 insertions(+), 19 deletions(-) diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt index 70da3b1..9390f89 100644 --- a/docs/specs/vhost-user.txt +++ b/docs/specs/vhost-user.txt @@ -135,6 +135,11 @@ As older slaves don't support negotiating protocol features, a feature bit was dedicated for this purpose: #define VHOST_USER_F_PROTOCOL_FEATURES 30 +Multi queue support +--- +The protocol supports multiple queues by setting all index fields in the sent +messages to a properly calculated value. + Message types - @@ -198,7 +203,7 @@ Message types Id: 4 Equivalent ioctl: VHOST_RESET_OWNER - Master payload: N/A + Master payload: vring state description Issued when a new connection is about to be closed. The Master will no longer own this connection (and will usually close it). diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index 1f25cb3..9cd6c05 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -159,6 +159,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options) net-dev.nvqs = 2; net-dev.vqs = net-vqs; +net-dev.vq_index = net-nc-queue_index; r = vhost_dev_init(net-dev, options-opaque, options-backend_type, options-force); @@ -269,7 +270,7 @@ static void vhost_net_stop_one(struct vhost_net *net, for (file.index = 0; file.index net-dev.nvqs; ++file.index) { const VhostOps *vhost_ops = net-dev.vhost_ops; int r = vhost_ops-vhost_call(net-dev, VHOST_RESET_OWNER, - NULL); + file); assert(r = 0); } } diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 27ba035..fb11d4c 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -219,7 +219,12 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request, break; case VHOST_USER_SET_OWNER: +break; + case VHOST_USER_RESET_OWNER: +memcpy(msg.state, arg, sizeof(struct vhost_vring_state)); +msg.state.index += dev-vq_index; +msg.size = sizeof(m.state); break; case VHOST_USER_SET_MEM_TABLE: @@ -262,17 +267,20 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request, case VHOST_USER_SET_VRING_NUM: case VHOST_USER_SET_VRING_BASE: memcpy(msg.state, arg, sizeof(struct vhost_vring_state)); +msg.state.index += dev-vq_index; msg.size = sizeof(m.state); break; case VHOST_USER_GET_VRING_BASE: memcpy(msg.state, arg, sizeof(struct vhost_vring_state)); +msg.state.index += dev-vq_index; msg.size = sizeof(m.state); need_reply = 1; break; case VHOST_USER_SET_VRING_ADDR: memcpy(msg.addr, arg, sizeof(struct vhost_vring_addr)); +msg.addr.index += dev-vq_index; msg.size = sizeof(m.addr); break; @@ -280,7 +288,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request, case VHOST_USER_SET_VRING_CALL: case VHOST_USER_SET_VRING_ERR: file = arg; -msg.u64 = file-index VHOST_USER_VRING_IDX_MASK; +msg.u64 = (file-index + dev-vq_index) VHOST_USER_VRING_IDX_MASK; msg.size = sizeof(m.u64); if (ioeventfd_enabled() file-fd 0) { fds[fd_num++]
[Qemu-devel] [PATCH v6 0/2] vhost-user multi queue support
Vhost-user will implement the multi queue support in a similar way to what vhost already has - a separate thread for each queue. To enable the multi queue functionality - a new command line parameter queues is introduced for the vhost-user netdev. Use new message VHOST_USER_SET_VRING_FLAG to enable and disable an actual virt queue, which is similar to attach/detach queue for tap device. virtio driver on guest doesn't have to use max virt queue pair, it could enable any number of virt queue ranging from 1 to max virt queue pair. It requires that VHOST_USER_F_PROTOCOL_FEATURES is present. Any comment is welcome for the patch set. It has dependency on a few other patch(set): vhost-user: protocol updates series proposed earlier by Michael S. Tsirkin RESET_OWNER change is based on commit: 294ce717e0f212ed0763307f3eab72b4a1bdf4d0 Changchun Ouyang (2): vhost-user: add multi queue support vhost-user: new protocol feature for multi queue docs/specs/vhost-user.txt | 24 +++- hw/net/vhost_net.c| 21 +- hw/net/virtio-net.c | 2 ++ hw/virtio/vhost-user.c| 46 --- include/hw/virtio/vhost-backend.h | 2 ++ include/net/vhost_net.h | 1 + net/vhost-user.c | 37 --- qapi-schema.json | 6 - qemu-options.hx | 5 +++-- 9 files changed, 123 insertions(+), 21 deletions(-) -- 1.8.4.2
Re: [Qemu-devel] [PATCH] linux-user: elfload: Still use TARGET_PAGE_SIZE for i386 guest
On 2015年08月12日 14:45, Richard Henderson wrote: On 08/11/2015 11:03 PM, gchen gchen wrote: Under Alpha host, for ubuntu12.04.5 i386 guest, it will cause failure: Invalid ELF image for this architecture. The related issue commit is a70daba linux-user: Tell guest about big host page sizes. Signed-off-by: Chen Ganggang.chen.5...@gmail.com --- linux-user/elfload.c | 4 1 file changed, 4 insertions(+) Nack. There's 99 problems with host page size guest page size. This solves none of them, and in the hackiest way possible. Under alpha virtual machine, if set i386 guest page size 8KB, it will cause failure directly (any dynamically linked binaries can not work). This fix can let i386 bash/cp/vi/ls run under alpha virtual machine, although there maybe be many other issues (which I did not meet, now). Do you have any other ideas for solving this issue? e.g. can we rebuild all i386 programs with segment alignment to 8KB? I am not quite sure, but some i386 programs (which are interested in) have no source code! :-(. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH for-2.5 15/30] m68k: add more modes to movem
Richard Henderson r...@twiddle.net writes: On 08/09/2015 01:13 PM, Laurent Vivier wrote: +opsize = (insn 0x40) != 0 ? OS_LONG : OS_WORD; +incr = opsize_bytes(opsize); +if (!is_load (insn 070) == 040) { +for (i = 15; i = 0; i--, mask = 1) { This has got to be wrong. Just because it's pre-decrement doesn't mean you should skip all of the loads. Pre-dec only supports reg-to-mem, and is special because mask is bit reversed. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different.
Re: [Qemu-devel] [PATCH for-2.5 05/30] m68k: define operand sizes
Le 12/08/2015 06:07, Richard Henderson a écrit : On 08/09/2015 01:13 PM, Laurent Vivier wrote: -#define OS_BYTE 0 -#define OS_WORD 1 -#define OS_LONG 2 -#define OS_SINGLE 4 -#define OS_DOUBLE 5 +#define OS_BYTE 1 +#define OS_WORD 2 +#define OS_LONG 3 +#define OS_SINGLE 4 +#define OS_DOUBLE 5 +#define OS_EXTENDED 6 +#define OS_PACKED 7 Is there a reason you've skipped the 0 value when adding the new values? I think there is no reason, but if I change the value I have to update abdc_mem, sbcd_mem instructions as they use it as an incrementer/decrementer. I agree, it's a strange idea. +static inline int insn_opsize(int insn, int pos) +{ +switch ((insn pos) 3) { In particular, that change means that insn_opsize is more complicated than needed. Further, is there any reason for POS to be a varable? Isn't it at the same place for all insns? +static inline int ext_opsize(int ext, int pos) This should probably wait until the fp insns get added. Yes. Laurent
[Qemu-devel] [PATCH v6 2/2] vhost-user: new protocol feature for multi queue
This patch is based on top of vhost-user: protocol updates series proposed earlier by Michael S. Tsirkin. Use new message VHOST_USER_SET_VRING_FLAG to enable and disable an actual virt queue, which is similar to attach/detach queue for tap device. virtio driver on guest doesn't have to use max virt queue pair, it could enable any number of virt queue ranging from 1 to max virt queue pair. It requires that VHOST_USER_F_PROTOCOL_FEATURES is present. Signed-off-by: Changchun Ouyang changchun.ouy...@intel.com --- This is added since v5 docs/specs/vhost-user.txt | 17 + hw/net/vhost_net.c| 18 ++ hw/net/virtio-net.c | 2 ++ hw/virtio/vhost-user.c| 35 +-- include/hw/virtio/vhost-backend.h | 2 ++ include/net/vhost_net.h | 1 + 6 files changed, 73 insertions(+), 2 deletions(-) diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt index 9390f89..cca3e5b 100644 --- a/docs/specs/vhost-user.txt +++ b/docs/specs/vhost-user.txt @@ -135,6 +135,10 @@ As older slaves don't support negotiating protocol features, a feature bit was dedicated for this purpose: #define VHOST_USER_F_PROTOCOL_FEATURES 30 +The Slave uses vring flag to notify the vhost-user whether one virtq is enabled +or not. This request doesn't require replies: +#define VHOST_USER_PROTOCOL_F_VRING_FLAG 2 + Multi queue support --- The protocol supports multiple queues by setting all index fields in the sent @@ -306,3 +310,16 @@ Message types Bits (0-7) of the payload contain the vring index. Bit 8 is the invalid FD flag. This flag is set when there is no file descriptor in the ancillary data. + + * VHOST_USER_SET_VRING_FLAG + + Id: 18 + Equivalent ioctl: N/A + Master payload: vring state description + + Set the flag(enable or disable) in the vring, the vhost user backend + enable or disable the vring according to state.num. Olny legal if feature + bit VHOST_USER_F_PROTOCOL_FEATURES is present in VHOST_USER_GET_FEATURE + and feature bit VHOST_USER_PROTOCOL_F_VRING_FLAG is present in + VHOST_USER_GET_PROTOCOL_FEATURES. The vring is enabled when state.num is + 1, otherwise, the vring is disabled. diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index 9cd6c05..5fa341c 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -405,6 +405,19 @@ VHostNetState *get_vhost_net(NetClientState *nc) return vhost_net; } + +int vhost_set_vring_flag(NetClientState *nc, unsigned int enable) +{ +if (nc-info-type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) { +struct vhost_net *net = get_vhost_net(nc); +const VhostOps *vhost_ops = net-dev.vhost_ops; +if (vhost_ops-vhost_backend_mq_set_vring_flag) +return vhost_ops-vhost_backend_mq_set_vring_flag(net-dev, enable); +} + +return 0; +} + #else struct vhost_net *vhost_net_init(VhostNetOptions *options) { @@ -455,4 +468,9 @@ VHostNetState *get_vhost_net(NetClientState *nc) { return 0; } + +int vhost_set_vring_flag(NetClientState *nc, unsigned int enable) +{ +return 0; +} #endif diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 3af6faf..272b77d 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -396,6 +396,7 @@ static int peer_attach(VirtIONet *n, int index) } if (nc-peer-info-type != NET_CLIENT_OPTIONS_KIND_TAP) { +vhost_set_vring_flag(nc-peer, 1); return 0; } @@ -411,6 +412,7 @@ static int peer_detach(VirtIONet *n, int index) } if (nc-peer-info-type != NET_CLIENT_OPTIONS_KIND_TAP) { +vhost_set_vring_flag(nc-peer, 0); return 0; } diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index fb11d4c..d806ce2 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -25,7 +25,8 @@ #define VHOST_MEMORY_MAX_NREGIONS8 #define VHOST_USER_F_PROTOCOL_FEATURES 30 -#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x0ULL +#define VHOST_USER_PROTOCOL_F_VRING_FLAG 2 +#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x7ULL typedef enum VhostUserRequest { VHOST_USER_NONE = 0, @@ -45,6 +46,7 @@ typedef enum VhostUserRequest { VHOST_USER_SET_VRING_ERR = 14, VHOST_USER_GET_PROTOCOL_FEATURES = 15, VHOST_USER_SET_PROTOCOL_FEATURES = 16, +VHOST_USER_SET_VRING_FLAG = 18, VHOST_USER_MAX } VhostUserRequest; @@ -399,6 +401,34 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque) return 0; } +static int vhost_user_set_vring_flag(struct vhost_dev *dev, unsigned int enable) +{ +VhostUserMsg msg = { 0 }; +int err; + +assert(dev-vhost_ops-backend_type == VHOST_BACKEND_TYPE_USER); + +if ((dev-backend_features ( 1ULL VHOST_USER_F_PROTOCOL_FEATURES)) == 0) +return -1; + +if ((dev-protocol_features (1ULL VHOST_USER_PROTOCOL_F_VRING_FLAG)) == 0) +return -1; + +
Re: [Qemu-devel] [PATCH v2 1/6] hw/pci: Use pow2ceil() rather than hand-calculation
On Fri, Jul 24, 2015 at 01:33:07PM +0100, Peter Maydell wrote: A couple of places in hw/pci use an inline calculation to round a size up to the next largest power of 2. We have a utility routine for this, so use it. (The behaviour of the old code is different if the size value is 0 -- it would leave it as 0 rather than rounding up to 1, but in both cases we know the size can't be 0. In the case where the size value had bit 31 set, the old code would invoke undefined behaviour; the new code will give a result of 0. Presumably that could never happen either.) Signed-off-by: Peter Maydell peter.mayd...@linaro.org Reviewed-by: Michael S. Tsirkin m...@redhat.com --- hw/pci/msix.c | 4 +--- hw/pci/pci.c | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/hw/pci/msix.c b/hw/pci/msix.c index 7716bf3..2fdada4 100644 --- a/hw/pci/msix.c +++ b/hw/pci/msix.c @@ -314,9 +314,7 @@ int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries, bar_size = bar_pba_offset + bar_pba_size; } -if (bar_size (bar_size - 1)) { -bar_size = 1 qemu_fls(bar_size); -} +bar_size = pow2ceil(bar_size); name = g_strdup_printf(%s-msix, dev-name); memory_region_init(dev-msix_exclusive_bar, OBJECT(dev), name, bar_size); diff --git a/hw/pci/pci.c b/hw/pci/pci.c index a017614..502da8d 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -2065,9 +2065,7 @@ static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom, g_free(path); return; } -if (size (size - 1)) { -size = 1 qemu_fls(size); -} +size = pow2ceil(size); vmsd = qdev_get_vmsd(DEVICE(pdev)); -- 1.9.1
Re: [Qemu-devel] [PATCH for-2.5 13/30] m68k: set Z and N on divu/muls overflow as a real 68040
On 08/09/2015 01:13 PM, Laurent Vivier wrote: This allows to compare simulation results with a real 68040. Signed-off-by: Laurent Vivierlaur...@vivier.eu --- target-m68k/op_helper.c | 38 ++ 1 file changed, 26 insertions(+), 12 deletions(-) A good incentive, yes, but... hmm. Certainly another incentive to split helpers, rather than pass in word. I don't have a way to review this though, so the best I can do is Acked-by: Richard Henderson r...@twiddle.net r~
Re: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size
-Original Message- From: qemu-devel-bounces+feng.wu=intel@nongnu.org [mailto:qemu-devel-bounces+feng.wu=intel@nongnu.org] On Behalf Of Jan Beulich Sent: Wednesday, August 12, 2015 2:59 PM To: Wu, Feng Cc: xen-de...@lists.xensource.com; qemu-devel@nongnu.org; stefano.stabell...@eu.citrix.com Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size On 05.08.15 at 04:02, feng...@intel.com wrote: @@ -491,8 +474,9 @@ static int xen_pt_bar_reg_write(XenPCIPassthroughState *s, XenPTReg *cfg_entry, bar_ro_mask = XEN_PT_BAR_IO_RO_MASK | (r_size - 1); break; case XEN_PT_BAR_FLAG_UPPER: +r = d-io_regions[index-1]; Perhaps worth an assert(index 0)? No problem, I will add it. BTW, do you have any other comments about this patch? If no, I am going to send out the new version with this changes. Thanks, Feng Jan
Re: [Qemu-devel] [RFC v4 1/9] exec.c: Add new exclusive bitmap to ram_list
I think that tlb_flush_entry is not enough, since in theory another vCPU could have a different TLB address referring the same phys address. alvise On Tue, Aug 11, 2015 at 6:32 PM, Paolo Bonzini pbonz...@redhat.com wrote: On 11/08/2015 18:11, alvise rigo wrote: Why flush the entire cache (I understand you mean TLB)? Sorry, I meant the TLB. If for each removal of an exclusive entry we set also the bit to 1, we force the following LL to make a tlb_flush() on every vCPU. What if you only flush one entry with tlb_flush_entry (on every vCPU)? Paolo
Re: [Qemu-devel] [PATCH for-2.5 01/30] m68k: define m680x0 CPUs and features
Le 12/08/2015 01:13, Richard Henderson a écrit : On 08/09/2015 01:13 PM, Laurent Vivier wrote: INSN(undef, , , CF_ISA_A); +INSN(undef, , , M68000); INSN(arith_im, 0080, fff8, CF_ISA_A); +INSN(arith_im, , ff00, M68000); +INSN(undef, 00c0, ffc0, M68000); INSN(bitrev,00c0, fff8, CF_ISA_APLUSC); INSN(bitop_reg, 0100, f1c0, CF_ISA_A); +INSN(bitop_reg, 0100, f1c0, M68000); INSN(bitop_reg, 0140, f1c0, CF_ISA_A); +INSN(bitop_reg, 0140, f1c0, M68000); There's a *lot* of repetition in here. Can we also introduce a BASE() macro that's like INSN() except that it doesn't bother checking m68k_feature? That way if both CF_ISA_A and M68000 are set, we don't have to duplicate the entry. Thank you, good idea. Laurent
Re: [Qemu-devel] [PATCH COLO-Frame v8 00/34] COarse-grain LOck-stepping(COLO) Virtual Machines for Non-stop Service (FT)
On 2015/8/5 19:24, Dr. David Alan Gilbert wrote: * zhanghailiang (zhang.zhanghaili...@huawei.com) wrote: This is the 8th version of COLO. Here is only COLO frame part, include: VM checkpoint, failover, proxy API, block replication API, not include block replication. The block part is treated as a separate series. As usual, we provide 'basic' and 'developing' branches in github: https://github.com/coloft/qemu/commits/colo-v1.5-basic https://github.com/coloft/qemu/commits/colo-v1.5-developing (more features) The 'basic' branch is exactly the same with this patch series, We will keep this series simple as possible, just for easy review. The extra features in colo-v1.5-developing branch: 1) Separate ram and device save/load process to reduce size of extra memory used during checkpoint 2) Live migrate part of dirty pages to slave during sleep time. 3) You get the statistic info about checkpoint by command 'info migrate' I'm hitting a problem that I think is due to the new global_state section that Juan recently added; if I cause a failover I hit: ERROR: invalid runstate transition: 'colo' - 'prelaunch' (on the secondary). I think the problem is that, the global_state is only sent for any 'unusual' states, so in the first migration that gets done at startup, 'prelaunch' is included in the stream in the global state, but then for later checkpoints the global_state probably isn't sent. I hacked around it by making global_state_needed return false; I guess we need to find a better fix! Hi Dave, This problem can be fixed by the following modification: diff --git a/migration/colo.c b/migration/colo.c index 89f4a3f..fccb384 100644 --- a/migration/colo.c +++ b/migration/colo.c @@ -424,6 +424,10 @@ static void *colo_thread(void *opaque) qemu_mutex_unlock_iothread(); trace_colo_vm_state_change(stop, run); +if (global_state_store() 0) { +goto out; +} + Actually, the global_state is sent in every cycle of COLO checkpoint. But the value is always the old one (prelaunch). It is only stored in the first migration's last stage, but not been updated after going into colo mode, where the new state is 'running'. Thanks, zhanghailiang Please reference to the follow link to test COLO. http://wiki.qemu.org/Features/COLO. COLO is a totally new feature which is still in early stage, your comments and feedback are warmly welcomed. NOTE: We have decided to re-implement the colo proxy in userspace (In qemu exactly). you can find the discussion about why how to realize the colo proxy in qemu from the follow link: http://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg04069.html TODO: 1. COLO function switch on/off 2. The capability of continuous FT 3. Optimize the performance. v8: - Move some global variables into MigrationIncomingState and MigrationState - Move some cleanup work form colo thread and colo incoming thread into failover BH function and also fix the code logic for the cleanup work. - fix the bug that colo thread and colo incoming thread possibly block in the socket 'recv' call when do failover work. - Optimize colo_flush_ram_cache() - Add migration state for incoming side, we use the state to verify if migration incoming side is in COLO state or not (Patch 5). - Drop the patch 'COLO: Disable qdev hotplug when VM is in COLO mode', since it is not correct. zhanghailiang (34): configure: Add parameter for configure to enable/disable COLO support migration: Introduce capability 'colo' to migration COLO: migrate colo related info to slave colo-comm/migration: skip colo info section for special cases migration: Add state records for migration incoming migration: Integrate COLO checkpoint process into migration migration: Integrate COLO checkpoint process into loadvm COLO: Implement colo checkpoint protocol COLO: Add a new RunState RUN_STATE_COLO QEMUSizedBuffer: Introduce two help functions for qsb COLO: Save VM state to slave when do checkpoint COLO RAM: Load PVM's dirty page into SVM's RAM cache temporarily COLO VMstate: Load VM state into qsb before restore it arch_init: Start to trace dirty pages of SVM COLO RAM: Flush cached RAM into SVM's memory COLO failover: Introduce a new command to trigger a failover COLO failover: Introduce state to record failover process COLO failover: Implement COLO primary/secondary vm failover work qmp event: Add event notification for COLO error COLO failover: Don't do failover during loading VM's state COLO: Add new command parameter 'forward_nic' 'colo_script' for net COLO NIC: Init/remove colo nic devices when add/cleanup tap devices tap: Make launch_script() public COLO NIC: Implement colo nic device interface configure() colo-nic: Handle secondary VM's original net device configure COLO NIC: Implement colo nic init/destroy function COLO NIC: Some init work related with proxy module COLO: Handle nfnetlink message from proxy module
Re: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size
On 12.08.15 at 09:10, feng...@intel.com wrote: -Original Message- From: qemu-devel-bounces+feng.wu=intel@nongnu.org [mailto:qemu-devel-bounces+feng.wu=intel@nongnu.org] On Behalf Of Jan Beulich Sent: Wednesday, August 12, 2015 2:59 PM To: Wu, Feng Cc: xen-de...@lists.xensource.com; qemu-devel@nongnu.org; stefano.stabell...@eu.citrix.com Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size On 05.08.15 at 04:02, feng...@intel.com wrote: @@ -491,8 +474,9 @@ static int xen_pt_bar_reg_write(XenPCIPassthroughState *s, XenPTReg *cfg_entry, bar_ro_mask = XEN_PT_BAR_IO_RO_MASK | (r_size - 1); break; case XEN_PT_BAR_FLAG_UPPER: +r = d-io_regions[index-1]; Perhaps worth an assert(index 0)? No problem, I will add it. BTW, do you have any other comments about this patch? If no, I am going to send out the new version with this changes. No - everything else looks to make sense (but continues to need testing). Jan
Re: [Qemu-devel] [PATCH for-2.5 05/30] m68k: define operand sizes
Laurent Vivier laur...@vivier.eu writes: Le 12/08/2015 06:07, Richard Henderson a écrit : Is there a reason you've skipped the 0 value when adding the new values? I think there is no reason, but if I change the value I have to update abdc_mem, sbcd_mem instructions as they use it as an incrementer/decrementer. I agree, it's a strange idea. Those uses are really opsize_bytes(OS_BYTE), technically. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different.
[Qemu-devel] [PATCH] user-exec: alpha-host: Add type cast to avoid compiling warning
The related building warnings in alpha virtual machine: CCi386-linux-user/user-exec.o user-exec.c: In function 'cpu_x86_signal_handler': user-exec.c:363:20: error: initialization makes pointer from integer without a cast [-Werror=int-conversion] uint32_t *pc = uc-uc_mcontext.sc_pc; ^ user-exec.c:383:30: error: passing argument 1 of 'handle_cpu_signal' makes integer from pointer without a cast [-Werror=int-conversion] return handle_cpu_signal(pc, (unsigned long)info-si_addr, ^ user-exec.c:86:19: note: expected 'uintptr_t {aka long unsigned int}' but argument is of type 'uint32_t * {aka unsigned int *}' static inline int handle_cpu_signal(uintptr_t pc, unsigned long address, ^ Signed-off-by: Chen Gang gang.chen.5...@gmail.com --- user-exec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/user-exec.c b/user-exec.c index ed9a07f..baaeb09 100644 --- a/user-exec.c +++ b/user-exec.c @@ -360,7 +360,7 @@ int cpu_signal_handler(int host_signum, void *pinfo, { siginfo_t *info = pinfo; struct ucontext *uc = puc; -uint32_t *pc = uc-uc_mcontext.sc_pc; +uint32_t *pc = (uint32_t *)uc-uc_mcontext.sc_pc; uint32_t insn = *pc; int is_write = 0; @@ -380,7 +380,7 @@ int cpu_signal_handler(int host_signum, void *pinfo, is_write = 1; } -return handle_cpu_signal(pc, (unsigned long)info-si_addr, +return handle_cpu_signal((unsigned long)pc, (unsigned long)info-si_addr, is_write, uc-uc_sigmask, puc); } #elif defined(__sparc__) -- 1.9.1
Re: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size
On 05.08.15 at 04:02, feng...@intel.com wrote: @@ -491,8 +474,9 @@ static int xen_pt_bar_reg_write(XenPCIPassthroughState *s, XenPTReg *cfg_entry, bar_ro_mask = XEN_PT_BAR_IO_RO_MASK | (r_size - 1); break; case XEN_PT_BAR_FLAG_UPPER: +r = d-io_regions[index-1]; Perhaps worth an assert(index 0)? Jan
Re: [Qemu-devel] [PATCH for-2.5 14/30] m68k: allow adda/suba to add/sub word
On 08/09/2015 01:13 PM, Laurent Vivier wrote: Signed-off-by: Laurent Vivierlaur...@vivier.eu --- target-m68k/translate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson r...@twiddle.net r~
Re: [Qemu-devel] [PATCH for-2.5 15/30] m68k: add more modes to movem
On 08/09/2015 01:13 PM, Laurent Vivier wrote: +opsize = (insn 0x40) != 0 ? OS_LONG : OS_WORD; +incr = opsize_bytes(opsize); +if (!is_load (insn 070) == 040) { +for (i = 15; i = 0; i--, mask = 1) { This has got to be wrong. Just because it's pre-decrement doesn't mean you should skip all of the loads. r~
Re: [Qemu-devel] [PATCH] linux-user: elfload: Still use TARGET_PAGE_SIZE for i386 guest
On 2015年08月12日 15:59, gchen gchen wrote: On 2015年08月12日 14:45, Richard Henderson wrote: On 08/11/2015 11:03 PM, gchen gchen wrote: Under Alpha host, for ubuntu12.04.5 i386 guest, it will cause failure: Invalid ELF image for this architecture. The related issue commit is a70daba linux-user: Tell guest about big host page sizes. Signed-off-by: Chen Ganggang.chen.5...@gmail.com --- linux-user/elfload.c | 4 1 file changed, 4 insertions(+) Nack. There's 99 problems with host page size guest page size. This solves none of them, and in the hackiest way possible. Under alpha virtual machine, if set i386 guest page size 8KB, it will cause failure directly (any dynamically linked binaries can not work). This fix can let i386 bash/cp/vi/ls run under alpha virtual machine, although there maybe be many other issues (which I did not meet, now). Do you have any other ideas for solving this issue? e.g. can we rebuild all i386 programs with segment alignment to 8KB? I am not quite sure, but some i386 programs (which are interested in) have no source code! :-(. By the way, the patches about the alpha are my current work, I can do it during my work time, it will have no any negative effect with my tilegx linux-user development in my free time. :-) One of my current work contents are: run ubuntu12.04.5 (no graphic) under alpha virtual machine, and run wine under ubuntu12.04.5 i386 guest under ubuntu alpha vm, and wine will run Win32 graphic programs. - I want to x86_64 laptop run Alpha linux-user run i386 linux-user run wine run Win32 graphic programs. I guess, I need build X libs under ubuntu Alpha vm (which nographic) -- use them for alpha linux-user. - At present, x86_64 laptop run arm linux-user run i386 linux-user run wine run Windows i386 graphic programs are OK, the performance is acceptable! the qemu code keeps no touch -- our qemu is very good!!!. Welcome any ideas, suggestions and completions. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH 1/2] migration: do cleanup operation after completion
On 12/08/2015 23:04, Liang Li wrote: @@ -1008,8 +1009,10 @@ static void *migration_thread(void *opaque) } qemu_mutex_lock_iothread(); +end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); +qemu_savevm_state_cancel(); + You can remove the qemu_savevm_state_cancel() call from migrate_fd_cleanup, too. Probably best to post a v2 with that change as well. Paolo You are right. Done. Liang
[Qemu-devel] [PATCH 1/2] migration: do cleanup operation after completion
Because of the patch 3ea3b7fa9af067982f34b of kvm, now the migration_end() is a time consuming operation, which takes about dozens of milliseconds, and will prolong VM downtime. Such an operation should be done after migration completion. For a VM with 8G RAM, this patch can reduce the VM downtime about 32 ms during live migration. Signed-off-by: Liang Li liang.z...@intel.com --- migration/block.c | 1 - migration/migration.c | 5 - migration/ram.c | 1 - 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/migration/block.c b/migration/block.c index ed865ed..85496fd 100644 --- a/migration/block.c +++ b/migration/block.c @@ -750,7 +750,6 @@ static int block_save_complete(QEMUFile *f, void *opaque) qemu_put_be64(f, BLK_MIG_FLAG_EOS); -blk_mig_cleanup(); return 0; } diff --git a/migration/migration.c b/migration/migration.c index 662e77e..c22095e 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -923,6 +923,7 @@ static void *migration_thread(void *opaque) int64_t initial_bytes = 0; int64_t max_size = 0; int64_t start_time = initial_time; +int64_t end_time; bool old_vm_running = false; rcu_register_thread(); @@ -1008,8 +1009,10 @@ static void *migration_thread(void *opaque) } qemu_mutex_lock_iothread(); +end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); +qemu_savevm_state_cancel(); + if (s-state == MIGRATION_STATUS_COMPLETED) { -int64_t end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); uint64_t transferred_bytes = qemu_ftell(s-file); s-total_time = end_time - s-total_time; s-downtime = end_time - start_time; diff --git a/migration/ram.c b/migration/ram.c index 7f007e6..6249f6e 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -1269,7 +1269,6 @@ static int ram_save_complete(QEMUFile *f, void *opaque) rcu_read_unlock(); -migration_end(); qemu_put_be64(f, RAM_SAVE_FLAG_EOS); return 0; -- 1.9.1
[Qemu-devel] [PATCH 2/2] migration: rename qemu_savevm_state_cancel
The function qemu_savevm_state_cancel is called after the migration in migration_thread, it seems strange to 'cancel' it after completion, rename it to qemu_savevm_state_cleanup looks better. Signed-off-by: Liang Li liang.z...@intel.com --- include/sysemu/sysemu.h | 2 +- migration/migration.c | 4 ++-- migration/savevm.c | 6 +++--- trace-events| 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index 44570d1..9cc0240 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -88,7 +88,7 @@ void qemu_savevm_state_begin(QEMUFile *f, void qemu_savevm_state_header(QEMUFile *f); int qemu_savevm_state_iterate(QEMUFile *f); void qemu_savevm_state_complete(QEMUFile *f); -void qemu_savevm_state_cancel(void); +void qemu_savevm_state_cleanup(void); uint64_t qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size); int qemu_loadvm_state(QEMUFile *f); diff --git a/migration/migration.c b/migration/migration.c index c22095e..e7fe50f 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -561,7 +561,7 @@ static void migrate_fd_cleanup(void *opaque) assert(s-state != MIGRATION_STATUS_ACTIVE); if (s-state != MIGRATION_STATUS_COMPLETED) { -qemu_savevm_state_cancel(); +qemu_savevm_state_cleanup(); if (s-state == MIGRATION_STATUS_CANCELLING) { migrate_set_state(s, MIGRATION_STATUS_CANCELLING, MIGRATION_STATUS_CANCELLED); @@ -1010,7 +1010,7 @@ static void *migration_thread(void *opaque) qemu_mutex_lock_iothread(); end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); -qemu_savevm_state_cancel(); +qemu_savevm_state_cleanup(); if (s-state == MIGRATION_STATUS_COMPLETED) { uint64_t transferred_bytes = qemu_ftell(s-file); diff --git a/migration/savevm.c b/migration/savevm.c index 6071215..b141b17 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -905,11 +905,11 @@ uint64_t qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size) return ret; } -void qemu_savevm_state_cancel(void) +void qemu_savevm_state_cleanup(void) { SaveStateEntry *se; -trace_savevm_state_cancel(); +trace_savevm_state_cleanup(); QTAILQ_FOREACH(se, savevm_state.handlers, entry) { if (se-ops se-ops-cancel) { se-ops-cancel(se-opaque); @@ -946,7 +946,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp) ret = qemu_file_get_error(f); } if (ret != 0) { -qemu_savevm_state_cancel(); +qemu_savevm_state_cleanup(); error_setg_errno(errp, -ret, Error while writing VM state); } return ret; diff --git a/trace-events b/trace-events index 94bf3bb..102373c 100644 --- a/trace-events +++ b/trace-events @@ -1195,7 +1195,7 @@ savevm_state_begin(void) savevm_state_header(void) savevm_state_iterate(void) savevm_state_complete(void) -savevm_state_cancel(void) +savevm_state_cleanup(void) vmstate_save(const char *idstr, const char *vmsd_name) %s, %s vmstate_load(const char *idstr, const char *vmsd_name) %s, %s qemu_announce_self_iter(const char *mac) %s -- 1.9.1
[Qemu-devel] [PATCH] MAINTAINERS: list smbios maintainers
Now that smbios has its own directory, list its maintainers. Same people as ACPI so just reuse that entry. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- MAINTAINERS | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 978b717..a059d5d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -645,13 +645,15 @@ S: Supported F: include/hw/pci/* F: hw/pci/* -ACPI +ACPI/SMBIOS M: Michael S. Tsirkin m...@redhat.com M: Igor Mammedov imamm...@redhat.com S: Supported F: include/hw/acpi/* +F: include/hw/smbios/* F: hw/mem/* F: hw/acpi/* +F: hw/smbios/* F: hw/i386/acpi-build.[hc] F: hw/i386/*dsl F: hw/arm/virt-acpi-build.c -- MST
Re: [Qemu-devel] [PATCH v6 0/2] vhost user: Add live migration
Hi On Wed, Aug 12, 2015 at 9:25 AM, Michael S. Tsirkin m...@redhat.com wrote: I think these patches need to be rebased on top of Marc Andre's ones, and use protocol flags to negotiate capabilities. Right? Correct. His patches should be applied before my migration tests, though. -- Marc-André Lureau
Re: [Qemu-devel] [PATCH 0/2] Fix long vm downtime during live migration
On 12/08/2015 23:04, Liang Li wrote: Some cleanup operations take long time during the pause and copy stage, especially with the KVM patch 3ea3b7fa9af067, do these operation after the completion of live migration can help to reduce VM downtime. Liang Li (2): migration: do cleanup operation after completion migration: rename qemu_savevm_state_cancel include/sysemu/sysemu.h | 2 +- migration/block.c | 1 - migration/migration.c | 7 +-- migration/ram.c | 1 - migration/savevm.c | 6 +++--- trace-events| 2 +- 6 files changed, 10 insertions(+), 9 deletions(-) That's really a clever solution! Reviewed-by: Paolo Bonzini pbonz...@redhat.com Cc: qemu-sta...@nongnu.org
[Qemu-devel] [PATCH] virtio/vhost: drop unnecessary VHOST_SET_VRING call
No need to send VHOST_SET_VRING_CALL to backend before the negotiation with the guest is finished. Signed-off-by: Marcel Apfelbaum mar...@redhat.com --- hw/virtio/vhost.c | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 2712c6f..b448542 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -875,24 +875,13 @@ static void vhost_eventfd_del(MemoryListener *listener, static int vhost_virtqueue_init(struct vhost_dev *dev, struct vhost_virtqueue *vq, int n) { -struct vhost_vring_file file = { -.index = n, -}; int r = event_notifier_init(vq-masked_notifier, 0); + if (r 0) { return r; } -file.fd = event_notifier_get_fd(vq-masked_notifier); -r = dev-vhost_ops-vhost_call(dev, VHOST_SET_VRING_CALL, file); -if (r) { -r = -errno; -goto fail_call; -} return 0; -fail_call: -event_notifier_cleanup(vq-masked_notifier); -return r; } static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq) -- 2.1.0
Re: [Qemu-devel] [PATCH] virtio/vhost: drop unnecessary VHOST_SET_VRING call
On Wed, Aug 12, 2015 at 01:19:51PM +0300, Marcel Apfelbaum wrote: No need to send VHOST_SET_VRING_CALL to backend before the negotiation with the guest is finished. Signed-off-by: Marcel Apfelbaum mar...@redhat.com Well - we do need to set it to the masked notifier initially to avoid losing events. You can't just drop it - need to move this call somewhere else. --- hw/virtio/vhost.c | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 2712c6f..b448542 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -875,24 +875,13 @@ static void vhost_eventfd_del(MemoryListener *listener, static int vhost_virtqueue_init(struct vhost_dev *dev, struct vhost_virtqueue *vq, int n) { -struct vhost_vring_file file = { -.index = n, -}; int r = event_notifier_init(vq-masked_notifier, 0); + if (r 0) { return r; } -file.fd = event_notifier_get_fd(vq-masked_notifier); -r = dev-vhost_ops-vhost_call(dev, VHOST_SET_VRING_CALL, file); -if (r) { -r = -errno; -goto fail_call; -} return 0; -fail_call: -event_notifier_cleanup(vq-masked_notifier); -return r; } static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq) -- 2.1.0
Re: [Qemu-devel] [ARM SMBIOS V3 PATCH 4/5] smbios: add smbios 3.0 support
On Tue, Aug 11, 2015 at 10:08:21PM -0400, Wei Huang wrote: This patch adds support for SMBIOS 3.0 entry point. When caller invokes smbios_set_defaults(), it can specify entry point as 2.1 or 3.0. Then smbios_get_tables() will return the entry point table in right format. Acked-by: Gabriel Somlo so...@cmu.edu Tested-by: Gabriel Somlo so...@cmu.edu Tested-by: Leif Lindholm leif.lindh...@linaro.org Signed-off-by: Wei Huang w...@redhat.com --- hw/i386/pc_piix.c | 3 +- hw/i386/pc_q35.c | 3 +- hw/smbios/smbios.c | 83 +- include/hw/smbios/smbios.h | 51 4 files changed, 101 insertions(+), 39 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 653c710..04636b1 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -173,7 +173,8 @@ static void pc_init1(MachineState *machine) MachineClass *mc = MACHINE_GET_CLASS(machine); /* These values are guest ABI, do not change */ smbios_set_defaults(QEMU, Standard PC (i440FX + PIIX, 1996), -mc-name, smbios_legacy_mode, smbios_uuid_encoded); +mc-name, smbios_legacy_mode, smbios_uuid_encoded, +SMBIOS_ENTRY_POINT_21); } /* allocate ram and load rom/bios */ diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index d83df14..061507d 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -165,7 +165,8 @@ static void pc_q35_init(MachineState *machine) if (smbios_defaults) { /* These values are guest ABI, do not change */ smbios_set_defaults(QEMU, Standard PC (Q35 + ICH9, 2009), -mc-name, smbios_legacy_mode, smbios_uuid_encoded); +mc-name, smbios_legacy_mode, smbios_uuid_encoded, +SMBIOS_ENTRY_POINT_21); } /* allocate ram and load rom/bios */ diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c index efdbb5d..de3cab5 100644 --- a/hw/smbios/smbios.c +++ b/hw/smbios/smbios.c @@ -55,7 +55,10 @@ static uint8_t *smbios_tables; static size_t smbios_tables_len; static unsigned smbios_table_max; static unsigned smbios_table_cnt; -static struct smbios_entry_point ep; +static SmbiosEntryPointType smbios_ep_type = SMBIOS_ENTRY_POINT_21; + +static SmbiosEntryPoint ep; +static size_t ep_length; I think it's better to drop ep_length and write a function that retrieves length from ep, based on anchor string. Don't duplicate information. static int smbios_type4_count = 0; static bool smbios_immutable; @@ -771,11 +774,12 @@ void smbios_set_cpuid(uint32_t version, uint32_t features) void smbios_set_defaults(const char *manufacturer, const char *product, const char *version, bool legacy_mode, - bool uuid_encoded) + bool uuid_encoded, SmbiosEntryPointType ep_type) { smbios_have_defaults = true; smbios_legacy = legacy_mode; smbios_uuid_encoded = uuid_encoded; +smbios_ep_type = ep_type; /* drop unwanted version of command-line file blob(s) */ if (smbios_legacy) { @@ -808,26 +812,59 @@ void smbios_set_defaults(const char *manufacturer, const char *product, static void smbios_entry_point_setup(void) { -memcpy(ep.anchor_string, _SM_, 4); -memcpy(ep.intermediate_anchor_string, _DMI_, 5); -ep.length = sizeof(struct smbios_entry_point); -ep.entry_point_revision = 0; /* formatted_area reserved, per spec v2.1+ */ -memset(ep.formatted_area, 0, 5); - -/* compliant with smbios spec v2.8 */ -ep.smbios_major_version = 2; -ep.smbios_minor_version = 8; -ep.smbios_bcd_revision = 0x28; - -/* set during table construction, but BIOS may override: */ -ep.structure_table_length = cpu_to_le16(smbios_tables_len); -ep.max_structure_size = cpu_to_le16(smbios_table_max); -ep.number_of_structures = cpu_to_le16(smbios_table_cnt); - -/* BIOS must recalculate: */ -ep.checksum = 0; -ep.intermediate_checksum = 0; -ep.structure_table_address = cpu_to_le32(0); +switch (smbios_ep_type) { +case SMBIOS_ENTRY_POINT_21: +memcpy(ep.ep21.anchor_string, _SM_, 4); +memcpy(ep.ep21.intermediate_anchor_string, _DMI_, 5); +ep.ep21.length = sizeof(struct smbios_21_entry_point); +ep.ep21.entry_point_revision = 0; /* formatted_area reserved */ +memset(ep.ep21.formatted_area, 0, 5); + +/* compliant with smbios spec v2.8 */ +ep.ep21.smbios_major_version = 2; +ep.ep21.smbios_minor_version = 8; +ep.ep21.smbios_bcd_revision = 0x28; + +/* set during table construction, but BIOS may override: */ +ep.ep21.structure_table_length = cpu_to_le16(smbios_tables_len); +
[Qemu-devel] [PATCH 0/2] Fix long vm downtime during live migration
Some cleanup operations take long time during the pause and copy stage, especially with the KVM patch 3ea3b7fa9af067, do these operation after the completion of live migration can help to reduce VM downtime. Liang Li (2): migration: do cleanup operation after completion migration: rename qemu_savevm_state_cancel include/sysemu/sysemu.h | 2 +- migration/block.c | 1 - migration/migration.c | 7 +-- migration/ram.c | 1 - migration/savevm.c | 6 +++--- trace-events| 2 +- 6 files changed, 10 insertions(+), 9 deletions(-) -- 1.9.1
Re: [Qemu-devel] [RFC PATCH V7 09/19] Drop global lock during TCG code execution
On 11/08/2015 23:34, Frederic Konrad wrote: Also if qemu_cond_broadcast(qemu_io_proceeded_cond) is being dropped there is no point keeping the guff around in qemu_tcg_wait_io_event. Yes good point. BTW this leads to high consumption of host CPU eg: 100% per VCPU thread as the VCPUs thread are no longer waiting for qemu_io_proceeded_cond. If the guest CPU is busy waiting, that's expected. But if the guest CPU is halted, it should not have 100% host CPU consumption. Paolo
Re: [Qemu-devel] [PATCH] monitor: remove QAPI_EVENT_VSERPORT_CHANGE throttle
On Tue, Aug 11, 2015 at 08:21:18PM +0200, Laszlo Ersek wrote: On 08/11/15 19:04, marcandre.lur...@redhat.com wrote: From: Marc-André Lureau marcandre.lur...@redhat.com QAPI_EVENT_VSERPORT_CHANGE reports changes of a virtio serial port state. However, the events may be for different ports, but the throttle mechanism may replace the event for a different port, since it only checks the event type. libvirt relies on a correct state to be reported for all channels: the qemu-ga commands may no longer work if the state is reported disconnected. This can be triggered easily by having more than 1 virtio-serial (qemu-ga + spice agent for example), and restarting quickly daemons or more realistically going quickly in and out of suspend. In a future patch, we may want to throttle events based on their arguments, but this will likely require dynamic allocations and more complicated code to insert/lookup pending events based on various arguments (id in QAPI_EVENT_VSERPORT_CHANGE case). Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1244064 Signed-off-by: Marc-André Lureau marcandre.lur...@redhat.com --- monitor.c | 1 - 1 file changed, 1 deletion(-) diff --git a/monitor.c b/monitor.c index aeea2b5..e4d56f7 100644 --- a/monitor.c +++ b/monitor.c @@ -558,7 +558,6 @@ static void monitor_qapi_event_init(void) monitor_qapi_event_throttle(QAPI_EVENT_BALLOON_CHANGE, 1000); monitor_qapi_event_throttle(QAPI_EVENT_QUORUM_REPORT_BAD, 1000); monitor_qapi_event_throttle(QAPI_EVENT_QUORUM_FAILURE, 1000); -monitor_qapi_event_throttle(QAPI_EVENT_VSERPORT_CHANGE, 1000); qmp_event_set_func_emit(monitor_qapi_event_queue); } I don't mind the change (and the point of argument sensitivity is not lost on me), but note that this undoes the protection that is spelled out in the leading comment of the function, not visible in the context here: /* Limit guest-triggerable events to 1 per second */ That was probably put in place in order to prevent a malicious guest from spamming the log files (and the CPU usage) of the management apps. One solution to that would be arg sensitivity. Another would be a burst-capable (ie. a slowly re-filling, limited size token bucket) throttle, maintained per event type. Until one of those gets written, I guess this patch is acceptable -- as long as mgmt people are okay with it. Daniel seems to be, so I don't mind. Not having rate limiting is certainly an undesirable situation, but rate limiting which discards unrecoverable data is even worse as it makes the event useless for the app. We've got a number of events which are not rate limited for this reason. Ultimately we do need to figure out a better way to rate limit such events, but until then we have no option but to skip rate limiting for events which have this issue. NB, some events which have args are acceptable to rate limit. eg the balloon change event is fine, because apps generally only care about the /current/ balloon level, so if 3 balloon events are emitted and the first 2 are discarded that is fine, because the most recent balloon level is the only one that matters. 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] [PATCH] virtio/vhost: drop unnecessary VHOST_SET_VRING call
On Wed, Aug 12, 2015 at 02:10:56PM +0300, Marcel Apfelbaum wrote: On 08/12/2015 01:34 PM, Michael S. Tsirkin wrote: On Wed, Aug 12, 2015 at 01:19:51PM +0300, Marcel Apfelbaum wrote: No need to send VHOST_SET_VRING_CALL to backend before the negotiation with the guest is finished. Signed-off-by: Marcel Apfelbaum mar...@redhat.com Well - we do need to set it to the masked notifier initially to avoid losing events. You can't just drop it - need to move this call somewhere else. What do we need to set? I just dropped the call to VHOST_SET_VRING_CALL. Thanks, Marcel We use two eventfds: masked and unmasked one. We switch dynamically dependent on msi mask value. Code assumes we start out masked, so we need to match that. --- hw/virtio/vhost.c | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 2712c6f..b448542 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -875,24 +875,13 @@ static void vhost_eventfd_del(MemoryListener *listener, static int vhost_virtqueue_init(struct vhost_dev *dev, struct vhost_virtqueue *vq, int n) { -struct vhost_vring_file file = { -.index = n, -}; int r = event_notifier_init(vq-masked_notifier, 0); + if (r 0) { return r; } -file.fd = event_notifier_get_fd(vq-masked_notifier); -r = dev-vhost_ops-vhost_call(dev, VHOST_SET_VRING_CALL, file); -if (r) { -r = -errno; -goto fail_call; -} return 0; -fail_call: -event_notifier_cleanup(vq-masked_notifier); -return r; } static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq) -- 2.1.0
Re: [Qemu-devel] [PATCH 1/2] migration: do cleanup operation after completion
On 12/08/2015 23:04, Liang Li wrote: @@ -1008,8 +1009,10 @@ static void *migration_thread(void *opaque) } qemu_mutex_lock_iothread(); +end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); +qemu_savevm_state_cancel(); + You can remove the qemu_savevm_state_cancel() call from migrate_fd_cleanup, too. Probably best to post a v2 with that change as well. Paolo
Re: [Qemu-devel] [PATCH] virtio/vhost: drop unnecessary VHOST_SET_VRING call
On 08/12/2015 01:34 PM, Michael S. Tsirkin wrote: On Wed, Aug 12, 2015 at 01:19:51PM +0300, Marcel Apfelbaum wrote: No need to send VHOST_SET_VRING_CALL to backend before the negotiation with the guest is finished. Signed-off-by: Marcel Apfelbaum mar...@redhat.com Well - we do need to set it to the masked notifier initially to avoid losing events. You can't just drop it - need to move this call somewhere else. What do we need to set? I just dropped the call to VHOST_SET_VRING_CALL. Thanks, Marcel --- hw/virtio/vhost.c | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 2712c6f..b448542 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -875,24 +875,13 @@ static void vhost_eventfd_del(MemoryListener *listener, static int vhost_virtqueue_init(struct vhost_dev *dev, struct vhost_virtqueue *vq, int n) { -struct vhost_vring_file file = { -.index = n, -}; int r = event_notifier_init(vq-masked_notifier, 0); + if (r 0) { return r; } -file.fd = event_notifier_get_fd(vq-masked_notifier); -r = dev-vhost_ops-vhost_call(dev, VHOST_SET_VRING_CALL, file); -if (r) { -r = -errno; -goto fail_call; -} return 0; -fail_call: -event_notifier_cleanup(vq-masked_notifier); -return r; } static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq) -- 2.1.0
Re: [Qemu-devel] [PATCH for-2.5 16/30] m68k: Add all access modes and data sizes to some 680x0 instructions
On 08/09/2015 01:13 PM, Laurent Vivier wrote: case 6: /* cmpi */ tcg_gen_mov_i32(dest, src1); tcg_gen_subi_i32(dest, dest, im); gen_update_cc_add(dest, tcg_const_i32(im)); -set_cc_op(s, CC_OP_SUB); +SET_CC_OP(opsize, SUB); break; default: abort(); } -if (op != 6) { -DEST_EA(env, insn, OS_LONG, dest, addr); -} +DEST_EA(env, insn, opsize, dest, addr); It appears you've lost the if that doesn't write back the result of compare. Did you intend to change the case 6 break to a return? r~
[Qemu-devel] [PATCH 02/10] cpus: remove tcg_halt_cond global variable.
From: KONRAD Frederic fred.kon...@greensocs.com This removes tcg_halt_cond global variable. We need one QemuCond per virtual cpu for multithread TCG. Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com Message-Id: 1439220437-23957-9-git-send-email-fred.kon...@greensocs.com [Keep tcg_halt_cond for bisectability, while making it static. - Paolo] Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- cpus.c | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/cpus.c b/cpus.c index 9224488..8884278 100644 --- a/cpus.c +++ b/cpus.c @@ -813,7 +813,6 @@ static unsigned iothread_requesting_mutex; static QemuThread io_thread; static QemuThread *tcg_cpu_thread; -static QemuCond *tcg_halt_cond; /* cpu creation */ static QemuCond qemu_cpu_cond; @@ -933,15 +932,13 @@ static void qemu_wait_io_event_common(CPUState *cpu) cpu-thread_kicked = false; } -static void qemu_tcg_wait_io_event(void) +static void qemu_tcg_wait_io_event(CPUState *cpu) { -CPUState *cpu; - while (all_cpu_threads_idle()) { /* Start accounting real time to the virtual clock if the CPUs are idle. */ qemu_clock_warp(QEMU_CLOCK_VIRTUAL); -qemu_cond_wait(tcg_halt_cond, qemu_global_mutex); +qemu_cond_wait(cpu-halt_cond, qemu_global_mutex); } while (iothread_requesting_mutex) { @@ -1067,7 +1064,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg) /* wait for initial kick-off after machine start */ while (first_cpu-stopped) { -qemu_cond_wait(tcg_halt_cond, qemu_global_mutex); +qemu_cond_wait(first_cpu-halt_cond, qemu_global_mutex); /* process any pending work */ CPU_FOREACH(cpu) { @@ -1088,7 +1085,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg) qemu_clock_notify(QEMU_CLOCK_VIRTUAL); } } -qemu_tcg_wait_io_event(); +qemu_tcg_wait_io_event(QTAILQ_FIRST(cpus)); } return NULL; @@ -1265,6 +1262,7 @@ void resume_all_vcpus(void) static void qemu_tcg_init_vcpu(CPUState *cpu) { char thread_name[VCPU_THREAD_NAME_SIZE]; +static QemuCond *tcg_halt_cond; tcg_cpu_address_space_init(cpu, cpu-as); -- 1.8.3.1
[Qemu-devel] [PATCH 01/10] cpus: protect work list with work_mutex
From: KONRAD Frederic fred.kon...@greensocs.com Protect the list of queued work items with something other than the BQL, as a preparation for running the work items outside it. Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- cpus.c| 22 ++ include/qom/cpu.h | 6 +- qom/cpu.c | 1 + 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/cpus.c b/cpus.c index c1e74d9..9224488 100644 --- a/cpus.c +++ b/cpus.c @@ -845,6 +845,8 @@ void run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data) wi.func = func; wi.data = data; wi.free = false; + +qemu_mutex_lock(cpu-work_mutex); if (cpu-queued_work_first == NULL) { cpu-queued_work_first = wi; } else { @@ -853,9 +855,10 @@ void run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data) cpu-queued_work_last = wi; wi.next = NULL; wi.done = false; +qemu_mutex_unlock(cpu-work_mutex); qemu_cpu_kick(cpu); -while (!wi.done) { +while (!atomic_mb_read(wi.done)) { CPUState *self_cpu = current_cpu; qemu_cond_wait(qemu_work_cond, qemu_global_mutex); @@ -876,6 +879,8 @@ void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data) wi-func = func; wi-data = data; wi-free = true; + +qemu_mutex_lock(cpu-work_mutex); if (cpu-queued_work_first == NULL) { cpu-queued_work_first = wi; } else { @@ -884,6 +889,7 @@ void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data) cpu-queued_work_last = wi; wi-next = NULL; wi-done = false; +qemu_mutex_unlock(cpu-work_mutex); qemu_cpu_kick(cpu); } @@ -896,15 +902,23 @@ static void flush_queued_work(CPUState *cpu) return; } -while ((wi = cpu-queued_work_first)) { +qemu_mutex_lock(cpu-work_mutex); +while (cpu-queued_work_first != NULL) { +wi = cpu-queued_work_first; cpu-queued_work_first = wi-next; +if (!cpu-queued_work_first) { +cpu-queued_work_last = NULL; +} +qemu_mutex_unlock(cpu-work_mutex); wi-func(wi-data); -wi-done = true; +qemu_mutex_lock(cpu-work_mutex); if (wi-free) { g_free(wi); +} else { +atomic_mb_set(wi-done, true); } } -cpu-queued_work_last = NULL; +qemu_mutex_unlock(cpu-work_mutex); qemu_cond_broadcast(qemu_work_cond); } diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 39712ab..77bbff2 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -244,6 +244,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. + * @work_mutex: Lock to prevent multiple access to queued_work_*. + * @queued_work_first: First asynchronous work pending. * * State of one CPU core or thread. */ @@ -264,7 +266,6 @@ struct CPUState { uint32_t host_tid; bool running; struct QemuCond *halt_cond; -struct qemu_work_item *queued_work_first, *queued_work_last; bool thread_kicked; bool created; bool stop; @@ -275,6 +276,9 @@ struct CPUState { int64_t icount_extra; sigjmp_buf jmp_env; +QemuMutex work_mutex; +struct qemu_work_item *queued_work_first, *queued_work_last; + AddressSpace *as; struct AddressSpaceDispatch *memory_dispatch; MemoryListener *tcg_as_listener; diff --git a/qom/cpu.c b/qom/cpu.c index 62f4b5d..3e93223 100644 --- a/qom/cpu.c +++ b/qom/cpu.c @@ -314,6 +314,7 @@ static void cpu_common_initfn(Object *obj) cpu-cpu_index = -1; cpu-gdb_num_regs = cpu-gdb_num_g_regs = cc-gdb_num_core_regs; +qemu_mutex_init(cpu-work_mutex); QTAILQ_INIT(cpu-breakpoints); QTAILQ_INIT(cpu-watchpoints); } -- 1.8.3.1
[Qemu-devel] [Bug 1297218] Re: guest hangs after live migration due to tsc jump
Could someone confirm whether this is fixed in 15.04 and/or 15.10? ** Changed in: qemu (Ubuntu) Status: Triaged = 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/1297218 Title: guest hangs after live migration due to tsc jump Status in QEMU: New Status in glusterfs package in Ubuntu: Invalid Status in qemu package in Ubuntu: Incomplete Status in glusterfs source package in Trusty: Confirmed Status in qemu source package in Trusty: Confirmed Bug description: We have two identical Ubuntu servers running libvirt/kvm/qemu, sharing a Gluster filesystem. Guests can be live migrated between them. However, live migration often leads to the guest being stuck at 100% for a while. In that case, the dmesg output for such a guest will show (once it recovers): Clocksource tsc unstable (delta = 662463064082 ns). In this particular example, a guest was migrated and only after 11 minutes (662 seconds) did it become responsive again. It seems that newly booted guests doe not suffer from this problem, these can be migrated back and forth at will. After a day or so, the problem becomes apparent. It also seems that migrating from server A to server B causes much more problems than going from B back to A. If necessary, I can do more measurements to qualify these observations. The VM servers run Ubuntu 13.04 with these packages: Kernel: 3.8.0-35-generic x86_64 Libvirt: 1.0.2 Qemu: 1.4.0 Gluster-fs: 3.4.2 (libvirt access the images via the filesystem, not using libgfapi yet as the Ubuntu libvirt is not linked against libgfapi). The interconnect between both machines (both for migration and gluster) is 10GbE. Both servers are synced to NTP and well within 1ms form one another. Guests are either Ubuntu 13.04 or 13.10. On the guests, the current_clocksource is kvm-clock. The XML definition of the guests only contains: clock offset='utc'/ Now as far as I've read in the documentation of kvm-clock, it specifically supports live migrations, so I'm a bit surprised at these problems. There isn't all that much information to find on these issue, although I have found postings by others that seem to have run into the same issues, but without a solution. --- ApportVersion: 2.14.1-0ubuntu3 Architecture: amd64 DistroRelease: Ubuntu 14.04 Package: libvirt (not installed) ProcCmdline: BOOT_IMAGE=/boot/vmlinuz-3.13.0-24-generic root=UUID=1b0c3c6d-a9b8-4e84-b076-117ae267d178 ro console=ttyS1,115200n8 BOOTIF=01-00-25-90-75-b5-c8 ProcVersionSignature: Ubuntu 3.13.0-24.47-generic 3.13.9 Tags: trusty apparmor apparmor apparmor apparmor apparmor Uname: Linux 3.13.0-24-generic x86_64 UpgradeStatus: No upgrade log present (probably fresh install) UserGroups: _MarkForUpload: True modified.conffile..etc.default.libvirt.bin: [modified] modified.conffile..etc.libvirt.libvirtd.conf: [modified] modified.conffile..etc.libvirt.qemu.conf: [modified] modified.conffile..etc.libvirt.qemu.networks.default.xml: [deleted] mtime.conffile..etc.default.libvirt.bin: 2014-05-12T19:07:40.020662 mtime.conffile..etc.libvirt.libvirtd.conf: 2014-05-13T14:40:25.894837 mtime.conffile..etc.libvirt.qemu.conf: 2014-05-12T18:58:27.885506 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1297218/+subscriptions
Re: [Qemu-devel] [PATCH for-2.5 17/30] m68k: ori/andi/subi/addi/eori/cmpi can modify SR/CCR
On 08/09/2015 01:13 PM, Laurent Vivier wrote: Signed-off-by: Laurent Vivier laur...@vivier.eu --- target-m68k/translate.c | 95 ++--- 1 file changed, 58 insertions(+), 37 deletions(-) diff --git a/target-m68k/translate.c b/target-m68k/translate.c index 6a426e1..9e379b3 100644 --- a/target-m68k/translate.c +++ b/target-m68k/translate.c @@ -1343,6 +1343,42 @@ DISAS_INSN(bitop_im) DEST_EA(env, insn, opsize, tmp, addr); } } + +static TCGv gen_get_ccr(DisasContext *s) +{ +TCGv dest; + +gen_flush_flags(s); +dest = tcg_temp_new(); +tcg_gen_shli_i32(dest, QREG_CC_X, 4); +tcg_gen_or_i32(dest, dest, QREG_CC_DEST); +return dest; +} + +static TCGv gen_get_sr(DisasContext *s) +{ +TCGv ccr; +TCGv sr; + +ccr = gen_get_ccr(s); +sr = tcg_temp_new(); +tcg_gen_andi_i32(sr, QREG_SR, 0xffe0); +tcg_gen_or_i32(sr, sr, ccr); +return sr; +} Leaking the temporary produced by gen_get_ccr. + +static void gen_set_sr(DisasContext *s, TCGv val, int ccr_only) +{ +TCGv tmp; +tmp = tcg_temp_new(); +tcg_gen_andi_i32(QREG_CC_DEST, val, 0xf); +tcg_gen_shri_i32(tmp, val, 4); +tcg_gen_andi_i32(QREG_CC_X, tmp, 1); +if (!ccr_only) { +gen_helper_set_sr(cpu_env, val); +} +} Leaking tmp. And you don't even need to allocate it -- perform the shift into QREG_CC_X. + DISAS_INSN(arith_im) { int op; @@ -1367,7 +1403,20 @@ DISAS_INSN(arith_im) default: abort(); } -SRC_EA(env, src1, opsize, -1, (op == 6) ? NULL : addr); +if ((op == 0 || op == 1) The subject line is surely misleading, as this is only ANDI/ORI, right? Again, some more commentary would be helpful. +(insn 0x3f) == 0x3c) { +if (opsize == OS_BYTE) { +src1 = gen_get_ccr(s); +} else { +if (IS_USER(s)) { +gen_exception(s, s-pc - 2, EXCP_PRIVILEGE); +return; +} +src1 = gen_get_sr(s); +} +} else { +SRC_EA(env, src1, opsize, -1, (op == 6) ? NULL : addr); +} dest = tcg_temp_new(); switch (op) { case 0: /* ori */ @@ -1405,7 +1454,14 @@ DISAS_INSN(arith_im) default: abort(); } -DEST_EA(env, insn, opsize, dest, addr); +if (op != 6) { +if ((op == 0 || op == 1) +(insn 0x3f) == 0x3c) { +gen_set_sr(s, dest, opsize == OS_BYTE); +} else { +DEST_EA(env, insn, opsize, dest, addr); +} +} That said, I think this code should be rearranged so that you don't have to replicate so many conditionals. In particular, the only thing of use in the middle of import for the ccr insns are two lines: tcg_gen_andi/ori_tl. I think it would be better to structure as if ((insn 0x3f) == 0x3c (op == 0 || op == 1)) { if (opsize == OS_BYTE) { src1 = gen_get_ccr (); } else { ... } if (op == 0) { tcg_gen_ori_i32 ... } else { tcg_gen_andi_i32 ... } gen_set_sr(s, dest, opsize == OS_BYTE); return; } // existing code r~
[Qemu-devel] [Bug 1163034] Re: linux-user mode can't handle guest setting RLIMIT_AS (hangs running gnutls28 configure check code)
This will come in when implemented upstream. ** Changed in: qemu (Ubuntu) Importance: High = Wishlist -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1163034 Title: linux-user mode can't handle guest setting RLIMIT_AS (hangs running gnutls28 configure check code) Status in QEMU: Confirmed Status in qemu package in Ubuntu: Confirmed Bug description: Please look at https://code.launchpad.net/~costamagnagianfranco/+archive/costamagnagianfranco-ppa/+packages and https://code.launchpad.net/~costamagnagianfranco/+archive/costamagnagianfranco-ppa/+build/4457434 I cannot make gnutls28 build on armhf, I suspect a builder problem To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1163034/+subscriptions
Re: [Qemu-devel] [PATCH for-2.5 19/30] m68k: add cmpm
On 08/09/2015 01:13 PM, Laurent Vivier wrote: Signed-off-by: Laurent Vivier laur...@vivier.eu --- target-m68k/translate.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/target-m68k/translate.c b/target-m68k/translate.c index ae57792..adf4521 100644 --- a/target-m68k/translate.c +++ b/target-m68k/translate.c @@ -2002,6 +2002,24 @@ DISAS_INSN(eor) opsize = insn_opsize(insn, 6); +if (((insn 3) 7) == 1) { +/* cmpm */ Surely this can be separated out from EOR via masks at register_opcode time. And since this isn't a coldfire instruction... +reg = AREG(insn, 0); +src = gen_load(s, opsize, reg, 1); +tcg_gen_addi_i32(reg, reg, opsize_bytes(opsize)); + +reg = AREG(insn, 9); +dest = gen_load(s, opsize, reg, 1); +tcg_gen_addi_i32(reg, reg, opsize_bytes(opsize)); + +reg = tcg_temp_new(); +tcg_gen_sub_i32(reg, dest, src); +gen_update_cc_add(reg, src); No need for the extra temp, reg. Simply modify dest. +SET_CC_OP(opsize, SUB); + +return; +} + SRC_EA(env, src, opsize, -1, addr); reg = DREG(insn, 9); dest = tcg_temp_new();