Re: [PATCH v6 4/8] vringh: introduce vringh_is_little_endian() helper
On Fri, 24 Apr 2015 14:24:58 +0200 Greg Kurz gk...@linux.vnet.ibm.com wrote: Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- include/linux/vringh.h | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) Acked-by: Cornelia Huck cornelia.h...@de.ibm.com -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 3/8] macvtap: introduce macvtap_is_little_endian() helper
On Fri, 24 Apr 2015 14:24:48 +0200 Greg Kurz gk...@linux.vnet.ibm.com wrote: Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/net/macvtap.c |9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) Acked-by: Cornelia Huck cornelia.h...@de.ibm.com -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v5 15/17] target-s390x: Extend arch specific QMP command query-cpu-definitions
On Mon, 27 Apr 2015 10:11:29 +0200 Christian Borntraeger borntrae...@de.ibm.com wrote: This patch implements the QMP command 'query-cpu-definitions' in the S390 context. The command returns a list of cpu definitions in the current host context. A runnable and migratable cpu model has the related attributes set to true. The order attribute is used to bring the listed cpu definitions in a release order. Can you add some explanation why we need the fallback code (e.g. something along the line, when querying no KVM guest is available and to query the capabilities we have to open a dummy VM bla bla) Maybe something like that: The returned values for attributes like runnable depend on the machine type QEMU is running on. The function kvm_s390_get_machine_props() is used to determine that. If QEMU was started for accelerator KVM, a KVMstate is established and machine properties are retrieved by cpu_model_get(). In case no KVMstate was established, e.g. during QEMU startup in daemonized mode with the default accelerator TCG, a fallback routine named get_machine_props_fallback() is used to retrieve the KVM machine properties. It first creates a temporary VM, performs the required ioctls and finally destroys the VM again. Michael -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v5 11/17] target-s390x: Add KVM VM attribute interface for S390 CPU models
On Mon, 27 Apr 2015 12:52:54 +0200 Christian Borntraeger borntrae...@de.ibm.com wrote: Am 27.04.2015 um 11:43 schrieb Michael Mueller: On Mon, 27 Apr 2015 10:15:47 +0200 Christian Borntraeger borntrae...@de.ibm.com wrote: Am 13.04.2015 um 15:56 schrieb Michael Mueller: [...] +static int cpu_model_get(KVMState *s, uint64_t attr, uint64_t addr) +{ +int rc = -ENOSYS; +struct kvm_device_attr dev_attr = { +.group = KVM_S390_VM_CPU_MODEL, +.attr = attr, +.addr = addr, Would it make sense to do the cast here cpu_model_get/set() is used to handle both attributes, KVM_S390_VM_CPU_MACHINE and KVM_S390_VM_CPU_PROCESSOR. Both require a different type in the signature, (S390ProcessorProps*) and (S390MachineProps*). Adding both as parameters seems to be odd and would require additionally logic in the function. Thus I think doing the cast outside is just the right thing to do. So what about a void pointer then as parameter? I prefer a pointer for qemu process memory over uint64_t as part of the function interface. This makes it somewhat clearer that this is an address within QEMU. Both ways will certainly work, though. The interface calls are: int kvm_s390_get_machine_props(KVMState *s, S390MachineProps *prop) int kvm_s390_get_processor_props(S390ProcessorProps *prop) cpu_model_get/set() are just static helpers. Conny, I guess you will pick up the patches. Any preference? Christian -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 7/8] vhost: cross-endian support for legacy devices
On Fri, 24 Apr 2015 14:27:24 +0200 Greg Kurz gk...@linux.vnet.ibm.com wrote: This patch brings cross-endian support to vhost when used to implement legacy virtio devices. Since it is a relatively rare situation, the feature availability is controlled by a kernel config option (not set by default). The vq-is_le boolean field is added to cache the endianness to be used for ring accesses. It defaults to native endian, as expected by legacy virtio devices. When the ring gets active, we force little endian if the device is modern. When the ring is deactivated, we revert to the native endian default. If cross-endian was compiled in, a vq-user_be boolean field is added so that userspace may request a specific endianness. This field is used to override the default when activating the ring of a legacy device. It has no effect on modern devices. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- Changes since v5: - fixed description in Kconfig - fixed error description in uapi header - dropped useless semi-colon in the vhost_vq_reset_user_be() stub drivers/vhost/Kconfig | 15 drivers/vhost/vhost.c | 85 +++- drivers/vhost/vhost.h | 11 +- include/uapi/linux/vhost.h | 14 +++ 4 files changed, 122 insertions(+), 3 deletions(-) Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v5 06/17] target-s390x: Introduce S390 CPU facilities
Am 27.04.2015 um 10:55 schrieb Michael Mueller: On Mon, 27 Apr 2015 10:11:37 +0200 Christian Borntraeger borntrae...@de.ibm.com wrote: Am 13.04.2015 um 15:56 schrieb Michael Mueller: [...] +FAC_TRANSACTIONAL_EXE = 73, +/* + * The store-hypervisor-information facility #74 is + * z/VM related and when added to be handled by QEMU + * when hosted on LPAR. (see: SC24-6179-05 page 953) + */ I find this sentence hard to read. I would not mind to fully skip it then. Maybe just add +/* z/VM-specific, see: SC24-6179-05 page 953) */ +FAC_STHYI = 74, but do not use? -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 6/8] virtio: add explicit big-endian support to memory accessors
On Fri, 24 Apr 2015 14:26:24 +0200 Greg Kurz gk...@linux.vnet.ibm.com wrote: The current memory accessors logic is: - little endian if little_endian - native endian (i.e. no byteswap) if !little_endian If we want to fully support cross-endian vhost, we also need to be able to convert to big endian. Instead of changing the little_endian argument to some 3-value enum, this patch changes the logic to: - little endian if little_endian - big endian if !little_endian The native endian case is handled by all users with a trivial helper. This patch doesn't change any functionality, nor it does add overhead. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- Changes since v5: - changed endian checking helpers as suggested by Thomas (use || and line breaker) drivers/net/macvtap.c|3 ++- drivers/net/tun.c|3 ++- drivers/vhost/vhost.h|3 ++- include/linux/virtio_byteorder.h | 24 ++-- include/linux/virtio_config.h|3 ++- include/linux/vringh.h |3 ++- 6 files changed, 24 insertions(+), 15 deletions(-) Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v5 06/17] target-s390x: Introduce S390 CPU facilities
On Mon, 27 Apr 2015 12:14:19 +0200 Christian Borntraeger borntrae...@de.ibm.com wrote: +/* z/VM-specific, see: SC24-6179-05 page 953) */ +FAC_STHYI = 74, picked -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v5 11/17] target-s390x: Add KVM VM attribute interface for S390 CPU models
Am 27.04.2015 um 11:43 schrieb Michael Mueller: On Mon, 27 Apr 2015 10:15:47 +0200 Christian Borntraeger borntrae...@de.ibm.com wrote: Am 13.04.2015 um 15:56 schrieb Michael Mueller: [...] +static int cpu_model_get(KVMState *s, uint64_t attr, uint64_t addr) +{ +int rc = -ENOSYS; +struct kvm_device_attr dev_attr = { +.group = KVM_S390_VM_CPU_MODEL, +.attr = attr, +.addr = addr, Would it make sense to do the cast here cpu_model_get/set() is used to handle both attributes, KVM_S390_VM_CPU_MACHINE and KVM_S390_VM_CPU_PROCESSOR. Both require a different type in the signature, (S390ProcessorProps*) and (S390MachineProps*). Adding both as parameters seems to be odd and would require additionally logic in the function. Thus I think doing the cast outside is just the right thing to do. So what about a void pointer then as parameter? I prefer a pointer for qemu process memory over uint64_t as part of the function interface. This makes it somewhat clearer that this is an address within QEMU. Both ways will certainly work, though. Conny, I guess you will pick up the patches. Any preference? Christian -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 5/8] vhost: introduce vhost_is_little_endian() helper
On Fri, 24 Apr 2015 14:25:12 +0200 Greg Kurz gk...@linux.vnet.ibm.com wrote: Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/vhost/vhost.h | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) Acked-by: Cornelia Huck cornelia.h...@de.ibm.com -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v5 11/17] target-s390x: Add KVM VM attribute interface for S390 CPU models
On Mon, 27 Apr 2015 10:15:47 +0200 Christian Borntraeger borntrae...@de.ibm.com wrote: Am 13.04.2015 um 15:56 schrieb Michael Mueller: [...] +static int cpu_model_get(KVMState *s, uint64_t attr, uint64_t addr) +{ +int rc = -ENOSYS; +struct kvm_device_attr dev_attr = { +.group = KVM_S390_VM_CPU_MODEL, +.attr = attr, +.addr = addr, Would it make sense to do the cast here cpu_model_get/set() is used to handle both attributes, KVM_S390_VM_CPU_MACHINE and KVM_S390_VM_CPU_PROCESSOR. Both require a different type in the signature, (S390ProcessorProps*) and (S390MachineProps*). Adding both as parameters seems to be odd and would require additionally logic in the function. Thus I think doing the cast outside is just the right thing to do. Michael +}; + +if (kvm_vm_check_attr(s, dev_attr.group, dev_attr.attr)) { +rc = kvm_vm_ioctl(s, KVM_GET_DEVICE_ATTR, dev_attr); +} + +return rc; +} + +static int cpu_model_set(KVMState *s, uint64_t attr, uint64_t addr) +{ +int rc = -ENOSYS; +struct kvm_device_attr dev_attr = { +.group = KVM_S390_VM_CPU_MODEL, +.attr = attr, +.addr = addr, ...and here... +}; + +if (kvm_vm_check_attr(s, dev_attr.group, dev_attr.attr)) { +rc = kvm_vm_ioctl(s, KVM_SET_DEVICE_ATTR, dev_attr); +} + +return rc; +} + +static int kvm_s390_get_machine_props(KVMState *s, S390MachineProps *prop) +{ +int rc = -EFAULT; + +if (s) { +rc = cpu_model_get(s, KVM_S390_VM_CPU_MACHINE, (uint64_t) prop); and pass S390MachineProps as 3rd parameter? +int kvm_s390_get_processor_props(S390ProcessorProps *prop) +{ +int rc; + +rc = cpu_model_get(kvm_state, KVM_S390_VM_CPU_PROCESSOR, (uint64_t) prop); dito +trace_kvm_get_processor_props(rc, prop-cpuid, prop-ibc); +return rc; +} + +int kvm_s390_set_processor_props(S390ProcessorProps *prop) +{ +int rc; + +rc = cpu_model_set(kvm_state, KVM_S390_VM_CPU_PROCESSOR, (uint64_t) prop); dito -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: KVM: x86: question about kvm_ioapic_destroy
On Sun, Apr 26, 2015 at 07:19:58PM +0200, Julia Lawall wrote: The function kvm_ioapic_destroy is defined as follows: void kvm_ioapic_destroy(struct kvm *kvm) { struct kvm_ioapic *ioapic = kvm-arch.vioapic; cancel_delayed_work_sync(ioapic-eoi_inject); if (ioapic) { kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS, ioapic-dev); kvm-arch.vioapic = NULL; kfree(ioapic); } } Is there any way that cancel_delayed_work_sync can work if ioapic is NULL? Should the call be moved down under the NULL test? Or is the NULL test not needed? The NULL test has been there longer than the call to cancel_delayed_work_sync, which was introduced in 184564ef. thanks, julia I think the NULL test is not needed. kvm_ioapic_destroy is only called if kvm_ioapic_init completed successfully, and that sets kvm-arch.vioapic. -- MST -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 06/10] KVM: arm64: guest debug, add SW break point support
On Thu, Apr 23, 2015 at 03:26:53PM +0100, Alex Bennée wrote: Christoffer Dall christoffer.d...@linaro.org writes: On Tue, Mar 31, 2015 at 04:08:04PM +0100, Alex Bennée wrote: This adds support for SW breakpoints inserted by userspace. We do this by trapping all BKPT exceptions in the hypervisor (MDCR_EL2_TDE). you mean trapping all exceptions in the guest to the hypervisor? The kvm_debug_exit_arch carries the address of the exception. why? can userspace not simply read out the PC using GET_ONE_REG? Yes, I have re-worded and removed PC from the debug information. snip + /* Trap breakpoints? */ + if (vcpu-guest_debug KVM_GUESTDBG_USE_SW_BP) + vcpu-arch.mdcr_el2 |= MDCR_EL2_TDE; + else + vcpu-arch.mdcr_el2 = ~MDCR_EL2_TDE; so now you're trapping all debug exceptions, right? what happens if the guest is using the hardware to debug debug stuff and generates other kinds of debug exceptions, like a hardware breakpoint, will we not see an unhandled exception and the guest being forcefully killed? Yes until the later patches which stop the guest using HW debug registers while we are using them. + } void kvm_arch_clear_debug(struct kvm_vcpu *vcpu) diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c index 524fa25..ed1bbb4 100644 --- a/arch/arm64/kvm/handle_exit.c +++ b/arch/arm64/kvm/handle_exit.c @@ -82,6 +82,37 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct kvm_run *run) return 1; } +/** + * kvm_handle_debug_exception - handle a debug exception instruction handle a software breadkpoint exception + * + * @vcpu: the vcpu pointer + * @run: access to the kvm_run structure for results + * + * We route all debug exceptions through the same handler as we all debug exceptions? software breakpoints and all? then why the above shot text? I think the issue here was debug exception instruction making me think this is just for software breakpoints... Not sure what I meant by 'shot text' - probably 'short text' + * just need to report the PC and the HSR values to userspace. + * Userspace may decide to re-inject the exception and deliver it to + * the guest if it wasn't for the host to deal with. now I'm confused - does userspace setup the guest to receive an exception or does it tell KVM to emulate an exception for the guest or do we execute the breakpoint without trapping the debug exception? I've made it all go through userspace as we may have to translate the hypervisor visible exception code to what the guest was expecting to see. ok, so I think you should re-phrase something like: Userspace may decide that this exception is caused by the guest using debugging itself, and may in that case emulate the guest debug exception in userspace before resuming KVM. But does that really work? Given that we don't support KVM-TCG migration, this sounds a little strange. Did we test it? + */ +static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu, struct kvm_run *run) +{ + u32 hsr = kvm_vcpu_get_hsr(vcpu); + + run-exit_reason = KVM_EXIT_DEBUG; + run-debug.arch.hsr = hsr; + + switch (hsr ESR_ELx_EC_SHIFT) { + case ESR_ELx_EC_BKPT32: + case ESR_ELx_EC_BRK64: + run-debug.arch.pc = *vcpu_pc(vcpu); + break; + default: + kvm_err(%s: un-handled case hsr: %#08x\n, + __func__, (unsigned int) hsr); this should never happen right? At the moment it could, at the end of the patch series we should cover all the cases so it would indicate a bug. I've made it return an error code so it fails hard as suggested by David. hmm, ok, so I'm not so worried about that kind of bisectability (although it would be nice to keep that working too), but reading patches that way is a bit annoying for reviewers, so I recommend you deal with the patch ordering in some way that makes it more obvious what happens as reviewers read the patches, one at a time. Thanks, -Christoffer -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 06/10] KVM: arm64: guest debug, add SW break point support
On 27 April 2015 at 21:04, Christoffer Dall christoffer.d...@linaro.org wrote: On Thu, Apr 23, 2015 at 03:26:53PM +0100, Alex Bennée wrote: Christoffer Dall christoffer.d...@linaro.org writes: On Tue, Mar 31, 2015 at 04:08:04PM +0100, Alex Bennée wrote: + * just need to report the PC and the HSR values to userspace. + * Userspace may decide to re-inject the exception and deliver it to + * the guest if it wasn't for the host to deal with. now I'm confused - does userspace setup the guest to receive an exception or does it tell KVM to emulate an exception for the guest or do we execute the breakpoint without trapping the debug exception? I've made it all go through userspace as we may have to translate the hypervisor visible exception code to what the guest was expecting to see. ok, so I think you should re-phrase something like: Userspace may decide that this exception is caused by the guest using debugging itself, and may in that case emulate the guest debug exception in userspace before resuming KVM. But does that really work? Given that we don't support KVM-TCG migration, this sounds a little strange. Did we test it? The QEMU patches have a TODO note at the point where you'd want to do this... Design-wise you can do the reinjection in the kernel or in userspace (ppc QEMU does this in userspace, for instance) because it's pretty much just setting registers to fake up the exception-entry into EL1. Code-wise QEMU's ARM code isn't set up to do it right now, but it shouldn't be too difficult to persuade the TCG exception-entry code to work for this case too. Does the kernel already have a conveniently implemented inject exception into guest lump of code? If so it might be less effort to do it that way round, maybe. -- PMM -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] KVM: x86: drop unneeded null test
From: Julia Lawall julia.law...@lip6.fr If the null test is needed, the call to cancel_delayed_work_sync would have already crashed. Normally, the destroy function should only be called if the init functoin has succeeded, in which case ioapic is not null. Problem found using Coccinelle. Signed-off-by: Julia Lawall julia.law...@lip6.fr --- arch/x86/kvm/ioapic.c |8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/arch/x86/kvm/ioapic.c b2/arch/x86/kvm/ioapic.c index 28146f0..09828e2 100644 --- a/arch/x86/kvm/ioapic.c +++ b2/arch/x86/kvm/ioapic.c @@ -637,11 +637,9 @@ void kvm_ioapic_destroy(struct kvm *kvm) struct kvm_ioapic *ioapic = kvm-arch.vioapic; cancel_delayed_work_sync(ioapic-eoi_inject); - if (ioapic) { - kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS, ioapic-dev); - kvm-arch.vioapic = NULL; - kfree(ioapic); - } + kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS, ioapic-dev); + kvm-arch.vioapic = NULL; + kfree(ioapic); } int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: x86: drop unneeded null test
On Mon, Apr 27, 2015 at 10:35:34PM +0200, Julia Lawall wrote: From: Julia Lawall julia.law...@lip6.fr If the null test is needed, the call to cancel_delayed_work_sync would have already crashed. Normally, the destroy function should only be called if the init functoin has succeeded, in which case ioapic is not null. Problem found using Coccinelle. Signed-off-by: Julia Lawall julia.law...@lip6.fr Acked-by: Michael S. Tsirkin m...@redhat.com --- arch/x86/kvm/ioapic.c |8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/arch/x86/kvm/ioapic.c b2/arch/x86/kvm/ioapic.c index 28146f0..09828e2 100644 --- a/arch/x86/kvm/ioapic.c +++ b2/arch/x86/kvm/ioapic.c @@ -637,11 +637,9 @@ void kvm_ioapic_destroy(struct kvm *kvm) struct kvm_ioapic *ioapic = kvm-arch.vioapic; cancel_delayed_work_sync(ioapic-eoi_inject); - if (ioapic) { - kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS, ioapic-dev); - kvm-arch.vioapic = NULL; - kfree(ioapic); - } + kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS, ioapic-dev); + kvm-arch.vioapic = NULL; + kfree(ioapic); } int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v5 09/17] target-s390x: Define S390 CPU model specific facility lists
On Mon, 27 Apr 2015 10:11:40 +0200 Christian Borntraeger borntrae...@de.ibm.com wrote: Am 13.04.2015 um 15:56 schrieb Michael Mueller: --- a/target-s390x/cpu-models.c +++ b/target-s390x/cpu-models.c @@ -76,3 +87,4 @@ S390_PROC_DEF(2827-ga1, CPU_S390_2827_GA1, IBM zEnterprise EC12 GA1) S390_PROC_DEF(2827-ga2, CPU_S390_2827_GA2, IBM zEnterprise EC12 GA2) S390_PROC_DEF(2828-ga1, CPU_S390_2828_GA1, IBM zEnterprise BC12 GA1) S390_PROC_DEF(2964-ga1, CPU_S390_2964_GA1, IBM z13 GA1) + Blank line at end of file, git am also complains: Applying: target-s390x: Define S390 CPU model specific facility lists /home/cborntra/REPOS/qemu/.git/rebase-apply/patch:54: new blank line at EOF. + warning: 1 line adds whitespace errors. I will address this here and in the next patch, checkpatch does not seem to catch that. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v5 15/17] target-s390x: Extend arch specific QMP command query-cpu-definitions
Am 13.04.2015 um 15:56 schrieb Michael Mueller: This patch implements the QMP command 'query-cpu-definitions' in the S390 context. The command returns a list of cpu definitions in the current host context. A runnable and migratable cpu model has the related attributes set to true. The order attribute is used to bring the listed cpu definitions in a release order. Can you add some explanation why we need the fallback code (e.g. something along the line, when querying no KVM guest is available and to query the capabilities we have to open a dummy VM bla bla) [...] -static int kvm_s390_get_machine_props(KVMState *s, S390MachineProps *prop) +static int get_machine_props_fallback(S390MachineProps *prop) +{ +struct kvm_device_attr dev_attr; +int rc, kvmfd = -1, vmfd = -1; + +rc = qemu_open(/dev/kvm, O_RDWR); +if (rc 0) { +goto out_err; +} +kvmfd = rc; + +rc = ioctl(kvmfd, KVM_CREATE_VM, 0); +if (rc 0) { +goto out_err; +} +vmfd = rc; + +rc = ioctl(vmfd, KVM_CHECK_EXTENSION, KVM_CAP_VM_ATTRIBUTES); +if (rc 0) { +rc = -ENOSYS; +goto out_err; +} + +dev_attr.group = KVM_S390_VM_CPU_MODEL; +dev_attr.attr = KVM_S390_VM_CPU_MACHINE; +rc = ioctl(vmfd, KVM_HAS_DEVICE_ATTR, dev_attr); +if (rc 0) { +rc = -EFAULT; +goto out_err; +} + +dev_attr.addr = (uint64_t) prop; +rc = ioctl(vmfd, KVM_GET_DEVICE_ATTR, dev_attr); + +out_err: +if (vmfd = 0) { +close(vmfd); +} +if (kvmfd = 0) { +close(kvmfd); +} + +return rc; +} + +int kvm_s390_get_machine_props(KVMState *s, S390MachineProps *prop) { int rc = -EFAULT; if (s) { rc = cpu_model_get(s, KVM_S390_VM_CPU_MACHINE, (uint64_t) prop); +} else { +rc = get_machine_props_fallback(prop); } trace_kvm_get_machine_props(rc, prop-cpuid, prop-ibc); -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v5 06/17] target-s390x: Introduce S390 CPU facilities
Am 13.04.2015 um 15:56 schrieb Michael Mueller: [...] +FAC_TRANSACTIONAL_EXE = 73, +/* + * The store-hypervisor-information facility #74 is + * z/VM related and when added to be handled by QEMU + * when hosted on LPAR. (see: SC24-6179-05 page 953) + */ I find this sentence hard to read. +FAC_ACCESS_EXCEPTION_FS_INDICATION = 75, [...] -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v5 09/17] target-s390x: Define S390 CPU model specific facility lists
Am 13.04.2015 um 15:56 schrieb Michael Mueller: --- a/target-s390x/cpu-models.c +++ b/target-s390x/cpu-models.c @@ -76,3 +87,4 @@ S390_PROC_DEF(2827-ga1, CPU_S390_2827_GA1, IBM zEnterprise EC12 GA1) S390_PROC_DEF(2827-ga2, CPU_S390_2827_GA2, IBM zEnterprise EC12 GA2) S390_PROC_DEF(2828-ga1, CPU_S390_2828_GA1, IBM zEnterprise BC12 GA1) S390_PROC_DEF(2964-ga1, CPU_S390_2964_GA1, IBM z13 GA1) + Blank line at end of file, git am also complains: Applying: target-s390x: Define S390 CPU model specific facility lists /home/cborntra/REPOS/qemu/.git/rebase-apply/patch:54: new blank line at EOF. + warning: 1 line adds whitespace errors. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v5 06/17] target-s390x: Introduce S390 CPU facilities
On Mon, 27 Apr 2015 10:11:37 +0200 Christian Borntraeger borntrae...@de.ibm.com wrote: Am 13.04.2015 um 15:56 schrieb Michael Mueller: [...] +FAC_TRANSACTIONAL_EXE = 73, +/* + * The store-hypervisor-information facility #74 is + * z/VM related and when added to be handled by QEMU + * when hosted on LPAR. (see: SC24-6179-05 page 953) + */ I find this sentence hard to read. I would not mind to fully skip it then. +FAC_ACCESS_EXCEPTION_FS_INDICATION = 75, [...] -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/8] virtio: introduce virtio_is_little_endian() helper
On Fri, 24 Apr 2015 14:24:27 +0200 Greg Kurz gk...@linux.vnet.ibm.com wrote: Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- include/linux/virtio_config.h | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h index ca3ed78..bd1a582 100644 --- a/include/linux/virtio_config.h +++ b/include/linux/virtio_config.h @@ -205,35 +205,40 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu) return 0; } +static inline bool virtio_is_little_endian(struct virtio_device *vdev) +{ + return virtio_has_feature(vdev, VIRTIO_F_VERSION_1); +} + While this reads a bit funny in this patch, it will end up correctly at the end of the series, so Acked-by: Cornelia Huck cornelia.h...@de.ibm.com -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 2/8] tun: add tun_is_little_endian() helper
On Fri, 24 Apr 2015 14:24:38 +0200 Greg Kurz gk...@linux.vnet.ibm.com wrote: Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/net/tun.c |9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) Acked-by: Cornelia Huck cornelia.h...@de.ibm.com -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v5 11/17] target-s390x: Add KVM VM attribute interface for S390 CPU models
Am 13.04.2015 um 15:56 schrieb Michael Mueller: [...] +static int cpu_model_get(KVMState *s, uint64_t attr, uint64_t addr) +{ +int rc = -ENOSYS; +struct kvm_device_attr dev_attr = { +.group = KVM_S390_VM_CPU_MODEL, +.attr = attr, +.addr = addr, Would it make sense to do the cast here +}; + +if (kvm_vm_check_attr(s, dev_attr.group, dev_attr.attr)) { +rc = kvm_vm_ioctl(s, KVM_GET_DEVICE_ATTR, dev_attr); +} + +return rc; +} + +static int cpu_model_set(KVMState *s, uint64_t attr, uint64_t addr) +{ +int rc = -ENOSYS; +struct kvm_device_attr dev_attr = { +.group = KVM_S390_VM_CPU_MODEL, +.attr = attr, +.addr = addr, ...and here... +}; + +if (kvm_vm_check_attr(s, dev_attr.group, dev_attr.attr)) { +rc = kvm_vm_ioctl(s, KVM_SET_DEVICE_ATTR, dev_attr); +} + +return rc; +} + +static int kvm_s390_get_machine_props(KVMState *s, S390MachineProps *prop) +{ +int rc = -EFAULT; + +if (s) { +rc = cpu_model_get(s, KVM_S390_VM_CPU_MACHINE, (uint64_t) prop); and pass S390MachineProps as 3rd parameter? +int kvm_s390_get_processor_props(S390ProcessorProps *prop) +{ +int rc; + +rc = cpu_model_get(kvm_state, KVM_S390_VM_CPU_PROCESSOR, (uint64_t) prop); dito +trace_kvm_get_processor_props(rc, prop-cpuid, prop-ibc); +return rc; +} + +int kvm_s390_set_processor_props(S390ProcessorProps *prop) +{ +int rc; + +rc = cpu_model_set(kvm_state, KVM_S390_VM_CPU_PROCESSOR, (uint64_t) prop); dito -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v5 10/17] target-s390x: Add S390 CPU model alias definition routines
Am 13.04.2015 um 15:56 schrieb Michael Mueller: --- a/target-s390x/cpu-models.c +++ b/target-s390x/cpu-models.c @@ -88,3 +88,85 @@ S390_PROC_DEF(2827-ga2, CPU_S390_2827_GA2, IBM zEnterprise EC12 GA2) [...] +int set_s390_cpu_alias(const char *name, const char *model) +{ +S390CPUAlias *alias; + +if (!name || !model) { +return -EINVAL; +} +if (!strcmp(name, model)) { +return -EINVAL; +} +if (!s390_cpu_class_by_name(model)) { +return -EINVAL; +} +alias = g_try_malloc0(sizeof(S390CPUAlias)); +if (!alias) { +return -ENOMEM; +} +alias-name = g_strdup(name); +alias-model = g_strdup(model); +s390_cpu_aliases = g_slist_append(s390_cpu_aliases, alias); +return 0; +} + Applying: target-s390x: Add S390 CPU model alias definition routines /home/cborntra/REPOS/qemu/.git/rebase-apply/patch:96: new blank line at EOF. + warning: 1 line adds whitespace errors. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v5 00/17] s390x cpu model implementation
Am 13.04.2015 um 15:56 schrieb Michael Mueller: This patch set in combination with its kernel kvm patch set proposes an implementation of S390 CPU models. The origin of this item is to provide a means for management interfaces like libvirt to draw decisions if life guest migration to a target hypervisor is reasonable. A migration constraint is that a target hypervisor is capable to run a guest with the same CPU model as the source hypervisor does. To verify this condition, the administration interface employes the existing QMP command query-cpu-definitions which returns a list of all currently supported CPU models of a given host system. Together with the extension of QMP command query-cpus, the current CPU model of a guest is retrievable and a conclusion can be drawn if a migration is possible. A S390 CPU model is defined as a triple of machine type, CPU facility set and IBC value. Each historic, current and future triple receives a name composed of the machine type and its general availability counter. This name forms the CPU model name (e.g.: 2817-ga2.) With means of the Instruction Blocking Control feature (IBC), the instruction set available to a given guest is limitable. From an s390 perspective this series looks like the right approach. When looking closer things are slightly more complicated than expected: The base line is the CPU type (like 2964 aka z13). This also qualifies the type of the whole guest (its not a cpu but a system property). This can be queried in the guest with the STIPD(store cpu id) instruction. Then there a the guest visible facilities. Those are the common subset of the facilities provides by the machine, LPAR and potentially z/VM. We can then add the facilities that are fully handled by QEMU. In theory this can be up to 16k facility bits and can be queried in the guest with the STFLE(store facility list extended) instruction. It is the responsibility of QEMU/KVM to follow the facility bits when doing an instruction handling. For things handled by the HW we have to setup the IBC control to be as good as possible. This series seem to handle all cases as far as I can see during test and review. Christian As long as the libvirt ans x86 folks are fine with the interfaces on the QMP level I think this is almost good to go. I have some minor style things as followup. With that fixed Acked-by: Christian Borntraeger borntrae...@de.ibm.com for the series. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v5 11/17] target-s390x: Add KVM VM attribute interface for S390 CPU models
On Mon, 27 Apr 2015 13:07:58 +0200 Michael Mueller m...@linux.vnet.ibm.com wrote: On Mon, 27 Apr 2015 12:52:54 +0200 Christian Borntraeger borntrae...@de.ibm.com wrote: Am 27.04.2015 um 11:43 schrieb Michael Mueller: On Mon, 27 Apr 2015 10:15:47 +0200 Christian Borntraeger borntrae...@de.ibm.com wrote: Am 13.04.2015 um 15:56 schrieb Michael Mueller: [...] +static int cpu_model_get(KVMState *s, uint64_t attr, uint64_t addr) +{ +int rc = -ENOSYS; +struct kvm_device_attr dev_attr = { +.group = KVM_S390_VM_CPU_MODEL, +.attr = attr, +.addr = addr, Would it make sense to do the cast here cpu_model_get/set() is used to handle both attributes, KVM_S390_VM_CPU_MACHINE and KVM_S390_VM_CPU_PROCESSOR. Both require a different type in the signature, (S390ProcessorProps*) and (S390MachineProps*). Adding both as parameters seems to be odd and would require additionally logic in the function. Thus I think doing the cast outside is just the right thing to do. So what about a void pointer then as parameter? I prefer a pointer for qemu process memory over uint64_t as part of the function interface. This makes it somewhat clearer that this is an address within QEMU. Both ways will certainly work, though. The interface calls are: int kvm_s390_get_machine_props(KVMState *s, S390MachineProps *prop) int kvm_s390_get_processor_props(S390ProcessorProps *prop) cpu_model_get/set() are just static helpers. So this makes them internal calls... Conny, I guess you will pick up the patches. Any preference? ...and I'd prefer using a void pointer for them. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 65081] Starting a second guest with -smp 2 makes the first guest very slow and the second one hang (or also very slow)
https://bugzilla.kernel.org/show_bug.cgi?id=65081 --- Comment #5 from Daniel Haid d.h...@gogi.tv --- The problem seems to be fixed with linux 3.19 and qemu 2.2.1. -- You are receiving this mail because: You are watching the assignee of the bug. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: KVM: x86: question about kvm_ioapic_destroy
On Mon, 27 Apr 2015, Michael S. Tsirkin wrote: On Sun, Apr 26, 2015 at 07:19:58PM +0200, Julia Lawall wrote: The function kvm_ioapic_destroy is defined as follows: void kvm_ioapic_destroy(struct kvm *kvm) { struct kvm_ioapic *ioapic = kvm-arch.vioapic; cancel_delayed_work_sync(ioapic-eoi_inject); if (ioapic) { kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS, ioapic-dev); kvm-arch.vioapic = NULL; kfree(ioapic); } } Is there any way that cancel_delayed_work_sync can work if ioapic is NULL? Should the call be moved down under the NULL test? Or is the NULL test not needed? The NULL test has been there longer than the call to cancel_delayed_work_sync, which was introduced in 184564ef. thanks, julia I think the NULL test is not needed. kvm_ioapic_destroy is only called if kvm_ioapic_init completed successfully, and that sets kvm-arch.vioapic. Thanks. I will send a patch. julia -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] KVM: x86: fix initial PAT value
PAT should be 0007_0406_0007_0406h on RESET and not modified on INIT. VMX used a wrong value (host's PAT) and while SVM used the right one, it never got to arch.pat. This is not an issue with QEMU as it will force the correct value. Signed-off-by: Radim Krčmář rkrc...@redhat.com --- arch/x86/kvm/svm.c | 2 +- arch/x86/kvm/vmx.c | 12 ++-- arch/x86/kvm/x86.c | 2 ++ arch/x86/kvm/x86.h | 2 ++ 4 files changed, 7 insertions(+), 11 deletions(-) diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index ce741b8650f6..9aceb4a8c147 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -1176,7 +1176,7 @@ static void init_vmcb(struct vcpu_svm *svm) clr_exception_intercept(svm, PF_VECTOR); clr_cr_intercept(svm, INTERCEPT_CR3_READ); clr_cr_intercept(svm, INTERCEPT_CR3_WRITE); - save-g_pat = 0x0007040600070406ULL; + save-g_pat = svm-vcpu.arch.pat; save-cr3 = 0; save-cr4 = 0; } diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index f7b61687bd79..62459382d305 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -4667,16 +4667,8 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx) vmcs_write32(VM_ENTRY_MSR_LOAD_COUNT, 0); vmcs_write64(VM_ENTRY_MSR_LOAD_ADDR, __pa(vmx-msr_autoload.guest)); - if (vmcs_config.vmentry_ctrl VM_ENTRY_LOAD_IA32_PAT) { - u32 msr_low, msr_high; - u64 host_pat; - rdmsr(MSR_IA32_CR_PAT, msr_low, msr_high); - host_pat = msr_low | ((u64) msr_high 32); - /* Write the default value follow host pat */ - vmcs_write64(GUEST_IA32_PAT, host_pat); - /* Keep arch.pat sync with GUEST_IA32_PAT */ - vmx-vcpu.arch.pat = host_pat; - } + if (vmcs_config.vmentry_ctrl VM_ENTRY_LOAD_IA32_PAT) + vmcs_write64(GUEST_IA32_PAT, vmx-vcpu.arch.pat); for (i = 0; i ARRAY_SIZE(vmx_msr_index); ++i) { u32 index = vmx_msr_index[i]; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index c73efcd03e29..a20898f9e43a 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -7363,6 +7363,8 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) vcpu-arch.maxphyaddr = cpuid_query_maxphyaddr(vcpu); + vcpu-arch.pat = MSR_IA32_CR_PAT_DEFAULT; + kvm_async_pf_hash_reset(vcpu); kvm_pmu_init(vcpu); diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h index f5fef1868096..01a1d011e073 100644 --- a/arch/x86/kvm/x86.h +++ b/arch/x86/kvm/x86.h @@ -4,6 +4,8 @@ #include linux/kvm_host.h #include kvm_cache_regs.h +#define MSR_IA32_CR_PAT_DEFAULT 0x0007040600070406ULL + static inline void kvm_clear_exception_queue(struct kvm_vcpu *vcpu) { vcpu-arch.exception.pending = false; -- 2.3.6 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v5 11/17] target-s390x: Add KVM VM attribute interface for S390 CPU models
On Mon, 27 Apr 2015 14:19:13 +0200 Cornelia Huck cornelia.h...@de.ibm.com wrote: Would it make sense to do the cast here cpu_model_get/set() is used to handle both attributes, KVM_S390_VM_CPU_MACHINE and KVM_S390_VM_CPU_PROCESSOR. Both require a different type in the signature, (S390ProcessorProps*) and (S390MachineProps*). Adding both as parameters seems to be odd and would require additionally logic in the function. Thus I think doing the cast outside is just the right thing to do. So what about a void pointer then as parameter? I prefer a pointer for qemu process memory over uint64_t as part of the function interface. This makes it somewhat clearer that this is an address within QEMU. Both ways will certainly work, though. The interface calls are: int kvm_s390_get_machine_props(KVMState *s, S390MachineProps *prop) int kvm_s390_get_processor_props(S390ProcessorProps *prop) cpu_model_get/set() are just static helpers. So this makes them internal calls... Conny, I guess you will pick up the patches. Any preference? ...and I'd prefer using a void pointer for them. Ok, I will make void pointers then to emphasize their address characteristics. Michael -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: KVM: x86: question about kvm_ioapic_destroy
On 27/04/2015 12:05, Michael S. Tsirkin wrote: On Sun, Apr 26, 2015 at 07:19:58PM +0200, Julia Lawall wrote: The function kvm_ioapic_destroy is defined as follows: void kvm_ioapic_destroy(struct kvm *kvm) { struct kvm_ioapic *ioapic = kvm-arch.vioapic; cancel_delayed_work_sync(ioapic-eoi_inject); if (ioapic) { kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS, ioapic-dev); kvm-arch.vioapic = NULL; kfree(ioapic); } } Is there any way that cancel_delayed_work_sync can work if ioapic is NULL? Should the call be moved down under the NULL test? Or is the NULL test not needed? The NULL test has been there longer than the call to cancel_delayed_work_sync, which was introduced in 184564ef. I think the NULL test is not needed. kvm_ioapic_destroy is only called if kvm_ioapic_init completed successfully, and that sets kvm-arch.vioapic. Agreed. By the way, in that case the cancel_delayed_work_sync is really a nop. Paolo -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: s390: remove delayed reallocation of page tables for KVM
On 04/27/2015 03:57 PM, Martin Schwidefsky wrote: On Mon, 27 Apr 2015 15:48:42 +0200 Alexander Graf ag...@suse.de wrote: On 04/23/2015 02:13 PM, Martin Schwidefsky wrote: On Thu, 23 Apr 2015 14:01:23 +0200 Alexander Graf ag...@suse.de wrote: As far as alternative approaches go, I don't have a great idea otoh. We could have an elf flag indicating that this process needs 4k page tables to limit the impact to a single process. In fact, could we maybe still limit the scope to non-global? A personality may work as well. Or ulimit? I tried the ELF flag approach, does not work. The trouble is that allocate_mm() has to create the page tables with 4K tables if you want to change the page table layout later on. We have learned the hard way that the direction 2K to 4K does not work due to races in the mm. Now there are two major cases: 1) fork + execve and 2) fork only. The ELF flag can be used to reduce from 4K to 2K for 1) but not 2). 2) is required for apps that use lots of forking, e.g. database or web servers. Same goes for the approach with a personality flag or ulimit. We would have to distinguish the two cases for allocate_mm(), if the new mm is allocated for a fork the current mm decides 2K vs. 4K. If the new mm is allocated by binfmt_elf, then start with 4K and do the downgrade after the ELF flag has been evaluated. Well, you could also make it a personality flag for example, no? Then every new process below a certain one always gets 4k page tables until they drop the personality, at which point each child would only get 2k page tables again. I'm mostly concerned that people will end up mixing VMs and other workloads on the same LPAR, so I don't think there's a one-shoe-fits-all solution. If I add an argument to mm_init() to indicate if this context is for fork() or execve() then the ELF header flag approach works. So you don't need the sysctl? Alex -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 65081] Starting a second guest with -smp 2 makes the first guest very slow and the second one hang (or also very slow)
https://bugzilla.kernel.org/show_bug.cgi?id=65081 Paolo Bonzini bonz...@gnu.org changed: What|Removed |Added Status|NEW |RESOLVED CC||bonz...@gnu.org Resolution|--- |CODE_FIX -- You are receiving this mail because: You are watching the assignee of the bug. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: s390: remove delayed reallocation of page tables for KVM
On 04/23/2015 02:13 PM, Martin Schwidefsky wrote: On Thu, 23 Apr 2015 14:01:23 +0200 Alexander Graf ag...@suse.de wrote: As far as alternative approaches go, I don't have a great idea otoh. We could have an elf flag indicating that this process needs 4k page tables to limit the impact to a single process. In fact, could we maybe still limit the scope to non-global? A personality may work as well. Or ulimit? I tried the ELF flag approach, does not work. The trouble is that allocate_mm() has to create the page tables with 4K tables if you want to change the page table layout later on. We have learned the hard way that the direction 2K to 4K does not work due to races in the mm. Now there are two major cases: 1) fork + execve and 2) fork only. The ELF flag can be used to reduce from 4K to 2K for 1) but not 2). 2) is required for apps that use lots of forking, e.g. database or web servers. Same goes for the approach with a personality flag or ulimit. We would have to distinguish the two cases for allocate_mm(), if the new mm is allocated for a fork the current mm decides 2K vs. 4K. If the new mm is allocated by binfmt_elf, then start with 4K and do the downgrade after the ELF flag has been evaluated. Well, you could also make it a personality flag for example, no? Then every new process below a certain one always gets 4k page tables until they drop the personality, at which point each child would only get 2k page tables again. I'm mostly concerned that people will end up mixing VMs and other workloads on the same LPAR, so I don't think there's a one-shoe-fits-all solution. Alex -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: s390: remove delayed reallocation of page tables for KVM
Am 27.04.2015 um 16:03 schrieb Alexander Graf: On 04/27/2015 03:57 PM, Martin Schwidefsky wrote: On Mon, 27 Apr 2015 15:48:42 +0200 Alexander Graf ag...@suse.de wrote: On 04/23/2015 02:13 PM, Martin Schwidefsky wrote: On Thu, 23 Apr 2015 14:01:23 +0200 Alexander Graf ag...@suse.de wrote: As far as alternative approaches go, I don't have a great idea otoh. We could have an elf flag indicating that this process needs 4k page tables to limit the impact to a single process. In fact, could we maybe still limit the scope to non-global? A personality may work as well. Or ulimit? I tried the ELF flag approach, does not work. The trouble is that allocate_mm() has to create the page tables with 4K tables if you want to change the page table layout later on. We have learned the hard way that the direction 2K to 4K does not work due to races in the mm. Now there are two major cases: 1) fork + execve and 2) fork only. The ELF flag can be used to reduce from 4K to 2K for 1) but not 2). 2) is required for apps that use lots of forking, e.g. database or web servers. Same goes for the approach with a personality flag or ulimit. We would have to distinguish the two cases for allocate_mm(), if the new mm is allocated for a fork the current mm decides 2K vs. 4K. If the new mm is allocated by binfmt_elf, then start with 4K and do the downgrade after the ELF flag has been evaluated. Well, you could also make it a personality flag for example, no? Then every new process below a certain one always gets 4k page tables until they drop the personality, at which point each child would only get 2k page tables again. I'm mostly concerned that people will end up mixing VMs and other workloads on the same LPAR, so I don't think there's a one-shoe-fits-all solution. If I add an argument to mm_init() to indicate if this context is for fork() or execve() then the ELF header flag approach works. So you don't need the sysctl? It would not be enough to enable old userspaces that do not have the ELF header flag. So we need both to enable old userspace - new kernel. Christian -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: s390: remove delayed reallocation of page tables for KVM
On Mon, 27 Apr 2015 15:48:42 +0200 Alexander Graf ag...@suse.de wrote: On 04/23/2015 02:13 PM, Martin Schwidefsky wrote: On Thu, 23 Apr 2015 14:01:23 +0200 Alexander Graf ag...@suse.de wrote: As far as alternative approaches go, I don't have a great idea otoh. We could have an elf flag indicating that this process needs 4k page tables to limit the impact to a single process. In fact, could we maybe still limit the scope to non-global? A personality may work as well. Or ulimit? I tried the ELF flag approach, does not work. The trouble is that allocate_mm() has to create the page tables with 4K tables if you want to change the page table layout later on. We have learned the hard way that the direction 2K to 4K does not work due to races in the mm. Now there are two major cases: 1) fork + execve and 2) fork only. The ELF flag can be used to reduce from 4K to 2K for 1) but not 2). 2) is required for apps that use lots of forking, e.g. database or web servers. Same goes for the approach with a personality flag or ulimit. We would have to distinguish the two cases for allocate_mm(), if the new mm is allocated for a fork the current mm decides 2K vs. 4K. If the new mm is allocated by binfmt_elf, then start with 4K and do the downgrade after the ELF flag has been evaluated. Well, you could also make it a personality flag for example, no? Then every new process below a certain one always gets 4k page tables until they drop the personality, at which point each child would only get 2k page tables again. I'm mostly concerned that people will end up mixing VMs and other workloads on the same LPAR, so I don't think there's a one-shoe-fits-all solution. If I add an argument to mm_init() to indicate if this context is for fork() or execve() then the ELF header flag approach works. -- blue skies, Martin. Reality continues to ruin my life. - Calvin. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: x86: Fix cmpxchg with two 32-bit registers
On 26/04/2015 17:10, Nadav Amit wrote: The emulation of CMPXCHG with two register operands in 64-bit mistakenly masks the high 32-bits as it performs assignment. Fix it. Fixes: 2fcf5c8ae244b4c298d2111a288d410a719ac626 Signed-off-by: Nadav Amit na...@cs.technion.ac.il --- arch/x86/kvm/emulate.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 630bcb0..84c132b 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -2242,6 +2242,8 @@ static int em_cmpxchg(struct x86_emulate_ctxt *ctxt) ctxt-src.val = ctxt-dst.orig_val; /* Create write-cycle to dest by writing the same value */ ctxt-dst.val = ctxt-dst.orig_val; + if (ctxt-dst.type != OP_MEM) + ctxt-dst.type = OP_NONE; } return X86EMUL_CONTINUE; } Applied, thanks. Paolo -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: x86: fix initial PAT value
On 27/04/2015 15:11, Radim Krčmář wrote: PAT should be 0007_0406_0007_0406h on RESET and not modified on INIT. VMX used a wrong value (host's PAT) and while SVM used the right one, it never got to arch.pat. This is not an issue with QEMU as it will force the correct value. Signed-off-by: Radim Krčmář rkrc...@redhat.com --- arch/x86/kvm/svm.c | 2 +- arch/x86/kvm/vmx.c | 12 ++-- arch/x86/kvm/x86.c | 2 ++ arch/x86/kvm/x86.h | 2 ++ 4 files changed, 7 insertions(+), 11 deletions(-) diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index ce741b8650f6..9aceb4a8c147 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -1176,7 +1176,7 @@ static void init_vmcb(struct vcpu_svm *svm) clr_exception_intercept(svm, PF_VECTOR); clr_cr_intercept(svm, INTERCEPT_CR3_READ); clr_cr_intercept(svm, INTERCEPT_CR3_WRITE); - save-g_pat = 0x0007040600070406ULL; + save-g_pat = svm-vcpu.arch.pat; save-cr3 = 0; save-cr4 = 0; } diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index f7b61687bd79..62459382d305 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -4667,16 +4667,8 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx) vmcs_write32(VM_ENTRY_MSR_LOAD_COUNT, 0); vmcs_write64(VM_ENTRY_MSR_LOAD_ADDR, __pa(vmx-msr_autoload.guest)); - if (vmcs_config.vmentry_ctrl VM_ENTRY_LOAD_IA32_PAT) { - u32 msr_low, msr_high; - u64 host_pat; - rdmsr(MSR_IA32_CR_PAT, msr_low, msr_high); - host_pat = msr_low | ((u64) msr_high 32); - /* Write the default value follow host pat */ - vmcs_write64(GUEST_IA32_PAT, host_pat); - /* Keep arch.pat sync with GUEST_IA32_PAT */ - vmx-vcpu.arch.pat = host_pat; - } + if (vmcs_config.vmentry_ctrl VM_ENTRY_LOAD_IA32_PAT) + vmcs_write64(GUEST_IA32_PAT, vmx-vcpu.arch.pat); for (i = 0; i ARRAY_SIZE(vmx_msr_index); ++i) { u32 index = vmx_msr_index[i]; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index c73efcd03e29..a20898f9e43a 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -7363,6 +7363,8 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) vcpu-arch.maxphyaddr = cpuid_query_maxphyaddr(vcpu); + vcpu-arch.pat = MSR_IA32_CR_PAT_DEFAULT; + kvm_async_pf_hash_reset(vcpu); kvm_pmu_init(vcpu); diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h index f5fef1868096..01a1d011e073 100644 --- a/arch/x86/kvm/x86.h +++ b/arch/x86/kvm/x86.h @@ -4,6 +4,8 @@ #include linux/kvm_host.h #include kvm_cache_regs.h +#define MSR_IA32_CR_PAT_DEFAULT 0x0007040600070406ULL + static inline void kvm_clear_exception_queue(struct kvm_vcpu *vcpu) { vcpu-arch.exception.pending = false; Applied, thanks. Paolo -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: s390: remove delayed reallocation of page tables for KVM
On 04/23/2015 02:08 PM, Christian Borntraeger wrote: Am 23.04.2015 um 14:01 schrieb Alexander Graf: Am 23.04.2015 um 13:43 schrieb Christian Borntraeger borntrae...@de.ibm.com: Am 23.04.2015 um 13:37 schrieb Alexander Graf: Am 23.04.2015 um 13:08 schrieb Christian Borntraeger borntrae...@de.ibm.com: From: Martin Schwidefsky schwidef...@de.ibm.com Replacing a 2K page table with a 4K page table while a VMA is active for the affected memory region is fundamentally broken. Rip out the page table reallocation code and replace it with a simple system control 'vm.allocate_pgste'. If the system control is set the page tables for all processes are allocated as full 4K pages, even for processes that do not need it. Signed-off-by: Martin Schwidefsky schwidef...@de.ibm.com Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com Couldn't you make this a hidden kconfig option that gets automatically selected when kvm is enabled? Or is there a non-kvm case that needs it too? For things like RHEV the default could certainly be enabled, but for normal distros like SLES/RHEL, the idea was to NOT enable that by default, as the non-KVM case is more common and might suffer from the additional memory consumption of the page tables. (big databases come to mind) We could think about having rpms like kvm to provide a sysctl file that sets it if we want to minimize the impact. Other ideas? Oh, I'm sorry, I misread the ifdef. I don't think it makes sense to have a config option for the default value then, just rely only on sysctl.conf for changed defaults. As far as mechanisms to change it go, every distribution has their own ways of dealing with this. RH has a profile thing, we don't really have anything central, but individual sysctl.d files for example that a kvm package could provide. Either way, the default choosing shouldn't happen in .config ;). So you vote for getting rid of the Kconfig? Also, please add some helpful error message in qemu to guide users to the sysctl. Yes, we will provide a qemu patch (cc stable) after this hits the kernel. As far as alternative approaches go, I don't have a great idea otoh. We could have an elf flag indicating that this process needs 4k page tables to limit the impact to a single process. This approach was actually Martins first fix. The problem is that the decision takes place on execve, but we need an answer at fork time. So we always started with 4k page tables and freed the 2nd halv on execve. Now this did not work for processes that only fork (without execve). In fact, could we maybe still limit the scope to non-global? A personality may work as well. Or ulimit? I think we will go for now with the sysctl and see if we can come up with some automatic way as additional patch later on. Sounds perfectly reasonable to me. You can for example also just set the sysctl bit in libvirtd :). Alex -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: s390: remove delayed reallocation of page tables for KVM
On 27/04/2015 15:48, Alexander Graf wrote: Well, you could also make it a personality flag for example, no? Then every new process below a certain one always gets 4k page tables until they drop the personality, at which point each child would only get 2k page tables again. But if you have to make (say) bash get the personality, this means that you need a full userspace recompile to run VMs. Paolo I'm mostly concerned that people will end up mixing VMs and other workloads on the same LPAR, so I don't think there's a one-shoe-fits-all solution. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] KVM: PPC: Book3S HV: Fix race in reading change bit when removing HPTE
Paul Mackerras pau...@samba.org writes: The reference (R) and change (C) bits in a HPT entry can be set by hardware at any time up until the HPTE is invalidated and the TLB invalidation sequence has completed. This means that when removing a HPTE, we need to read the HPTE after the invalidation sequence has completed in order to obtain reliable values of R and C. The code in kvmppc_do_h_remove() used to do this. However, commit 6f22bd3265fb (KVM: PPC: Book3S HV: Make HTAB code LE host aware) removed the read after invalidation as a side effect of other changes. This restores the read of the HPTE after invalidation. The user-visible effect of this bug would be that when migrating a guest, there is a small probability that a page modified by the guest and then unmapped by the guest might not get re-transmitted and thus the destination might end up with a stale copy of the page. Fixes: 6f22bd3265fb Cc: sta...@vger.kernel.org # v3.17+ Signed-off-by: Paul Mackerras pau...@samba.org --- arch/powerpc/kvm/book3s_hv_rm_mmu.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c index f6bf0b1..5c1737f 100644 --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c @@ -413,14 +413,12 @@ long kvmppc_do_h_remove(struct kvm *kvm, unsigned long flags, rev = real_vmalloc_addr(kvm-arch.revmap[pte_index]); v = pte ~HPTE_V_HVLOCK; if (v HPTE_V_VALID) { - u64 pte1; - - pte1 = be64_to_cpu(hpte[1]); hpte[0] = ~cpu_to_be64(HPTE_V_VALID); - rb = compute_tlbie_rb(v, pte1, pte_index); + rb = compute_tlbie_rb(v, be64_to_cpu(hpte[1]), pte_index); do_tlbies(kvm, rb, 1, global_invalidates(kvm, flags), true); /* Read PTE low word after tlbie to get final R/C values */ - remove_revmap_chain(kvm, pte_index, rev, v, pte1); + remove_revmap_chain(kvm, pte_index, rev, v, + be64_to_cpu(hpte[1])); } May be add the above commit message as a code comment ? r = rev-guest_rpte ~HPTE_GR_RESERVED; note_hpte_modification(kvm, rev); -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: KVM Forum 2015 Call for Participation
A friendly reminder that KVM Forum 2015 accepts proposal submissions until next Friday. Thanks on behalf of the KVM Forum 2015 Program Committee. Paolo On 11/03/2015 18:53, Paolo Bonzini wrote: = KVM Forum 2015: Call For Participation August 19-21, 2015 - Sheraton Seattle - Seattle, WA (All submissions must be received before midnight May 1, 2015) = KVM is an industry leading open source hypervisor that provides an ideal platform for datacenter virtualization, virtual desktop infrastructure, and cloud computing. Once again, it's time to bring together the community of developers and users that define the KVM ecosystem for our annual technical conference. We will discuss the current state of affairs and plan for the future of KVM, its surrounding infrastructure, and management tools. Mark your calendar and join us in advancing KVM. http://events.linuxfoundation.org/events/kvm-forum/ This year, the KVM Forum is moving back to North America. We will be colocated with the Linux Foundation's LinuxCon North America, CloudOpen North America, ContainerCon and Linux Plumbers Conference events. Attendees of KVM Forum will also be able to attend a shared hackathon event with Xen Project Developer Summit on August 18, 2015. We invite you to lead part of the discussion by submitting a speaking proposal for KVM Forum 2015. http://events.linuxfoundation.org/cfp Suggested topics: KVM/Kernel * Scaling and optimizations * Nested virtualization * Linux kernel performance improvements * Resource management (CPU, I/O, memory) * Hardening and security * VFIO: SR-IOV, GPU, platform device assignment * Architecture ports QEMU * Management interfaces: QOM and QMP * New devices, new boards, new architectures * Scaling and optimizations * Desktop virtualization and SPICE * Virtual GPU * virtio and vhost, including non-Linux or non-virtualized uses * Hardening and security * New storage features * Live migration and fault tolerance * High availability and continuous backup * Real-time guest support * Emulation and TCG * Firmware: ACPI, UEFI, coreboot, u-Boot, etc. * Testing Management and infrastructure * Managing KVM: Libvirt, OpenStack, oVirt, etc. * Storage: glusterfs, Ceph, etc. * Software defined networking: Open vSwitch, OpenDaylight, etc. * Network Function Virtualization * Security * Provisioning * Performance tuning === SUBMITTING YOUR PROPOSAL === Abstracts due: May 1, 2015 Please submit a short abstract (~150 words) describing your presentation proposal. Slots vary in length up to 45 minutes. Also include in your proposal the proposal type -- one of: - technical talk - end-user talk Submit your proposal here: http://events.linuxfoundation.org/cfp Please only use the categories presentation and panel discussion You will receive a notification whether or not your presentation proposal was accepted by May 29, 2015. Speakers will receive a complimentary pass for the event. In the instance that your submission has multiple presenters, only the primary speaker for a proposal will receive a complementary event pass. For panel discussions, all panelists will receive a complimentary event pass. TECHNICAL TALKS A good technical talk should not just report on what has happened over the last year; it should present a concrete problem and how it impacts the user and/or developer community. Whenever applicable, focus on work that needs to be done, difficulties that haven't yet been solved, and on decisions that other developers should be aware of. Summarizing recent developments is okay but it should not be more than a small portion of the overall talk. END-USER TALKS One of the big challenges as developers is to know what, where and how people actually use our software. We will reserve a few slots for end users talking about their deployment challenges and achievements. If you are using KVM in production you are encouraged submit a speaking proposal. Simply mark it as an end-user talk. As an end user, this is a unique opportunity to get your input to developers. HANDS-ON / BOF SESSIONS We will reserve some time for people to get together and discuss strategic decisions as well as other topics that are best solved within smaller groups. These sessions will be announced during the event. If you are interested in organizing such a session, please add it to the list at http://www.linux-kvm.org/page/KVM_Forum_2015_BOF Let people you think might be interested know about it, and encourage them to add their names to the wiki page as well. Please try to add your ideas to the list before KVM Forum starts. PANEL DISCUSSIONS If you are proposing a panel discussion, please make sure that you list all of your potential