Re: [PATCH 2/6] KVM: PPC: Add support for multiple-TCE hcalls
On Mon, May 06, 2013 at 05:25:53PM +1000, Alexey Kardashevskiy wrote: This adds real mode handlers for the H_PUT_TCE_INDIRECT and H_STUFF_TCE hypercalls for QEMU emulated devices such as virtio devices or emulated PCI. These calls allow adding multiple entries (up to 512) into the TCE table in one call which saves time on transition to/from real mode. This adds a guest physical to host real address converter and calls the existing H_PUT_TCE handler. The converting function is going to be fully utilized by upcoming VFIO supporting patches. This also implements the KVM_CAP_PPC_MULTITCE capability, so in order to support the functionality of this patch, QEMU needs to query for this capability and set the hcall-multi-tce hypertas property only if the capability is present, otherwise there will be serious performance degradation. Hrm. Clearly I didn't read this carefully enough before. There are some problems here. [snip] diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c index 72ffc89..643ac1e 100644 --- a/arch/powerpc/kvm/book3s_64_vio.c +++ b/arch/powerpc/kvm/book3s_64_vio.c @@ -14,6 +14,7 @@ * * Copyright 2010 Paul Mackerras, IBM Corp. pau...@au1.ibm.com * Copyright 2011 David Gibson, IBM Corporation d...@au1.ibm.com + * Copyright 2013 Alexey Kardashevskiy, IBM Corporation a...@au1.ibm.com */ #include linux/types.h @@ -36,9 +37,14 @@ #include asm/ppc-opcode.h #include asm/kvm_host.h #include asm/udbg.h +#include asm/iommu.h #define TCES_PER_PAGE(PAGE_SIZE / sizeof(u64)) +#define ERROR_ADDR (~(unsigned long)0x0) +/* + * TCE tables handlers. + */ static long kvmppc_stt_npages(unsigned long window_size) { return ALIGN((window_size SPAPR_TCE_SHIFT) @@ -148,3 +154,111 @@ fail: } return ret; } + +/* + * Virtual mode handling of IOMMU map/unmap. + */ +/* Converts guest physical address into host virtual */ +static unsigned long get_virt_address(struct kvm_vcpu *vcpu, + unsigned long gpa) This should probably return a void * rather than an unsigned long. Well, actually a void __user *. +{ + unsigned long hva, gfn = gpa PAGE_SHIFT; + struct kvm_memory_slot *memslot; + + memslot = search_memslots(kvm_memslots(vcpu-kvm), gfn); + if (!memslot) + return ERROR_ADDR; + + /* + * Convert gfn to hva preserving flags and an offset + * within a system page + */ + hva = __gfn_to_hva_memslot(memslot, gfn) + (gpa ~PAGE_MASK); + return hva; +} + +long kvmppc_virtmode_h_put_tce(struct kvm_vcpu *vcpu, + unsigned long liobn, unsigned long ioba, + unsigned long tce) +{ + struct kvmppc_spapr_tce_table *tt; + + tt = kvmppc_find_tce_table(vcpu, liobn); + /* Didn't find the liobn, put it to userspace */ + if (!tt) + return H_TOO_HARD; + + /* Emulated IO */ + return kvmppc_emulated_h_put_tce(tt, ioba, tce); +} + +long kvmppc_virtmode_h_put_tce_indirect(struct kvm_vcpu *vcpu, + unsigned long liobn, unsigned long ioba, + unsigned long tce_list, unsigned long npages) +{ + struct kvmppc_spapr_tce_table *tt; + long i; + unsigned long tces; + + /* The whole table addressed by tce_list resides in 4K page */ + if (npages 512) + return H_PARAMETER; So, that doesn't actually verify what the comment says it does - only that the list is 4K in total. You need to check the alignment of tce_list as well. + + tt = kvmppc_find_tce_table(vcpu, liobn); + /* Didn't find the liobn, put it to userspace */ + if (!tt) + return H_TOO_HARD; + + tces = get_virt_address(vcpu, tce_list); + if (tces == ERROR_ADDR) + return H_TOO_HARD; + + /* Emulated IO */ This comment doesn't seem to have any bearing on the test which follows it. + if ((ioba + (npages IOMMU_PAGE_SHIFT)) tt-window_size) + return H_PARAMETER; + + for (i = 0; i npages; ++i) { + unsigned long tce; + unsigned long ptce = tces + i * sizeof(unsigned long); + + if (get_user(tce, (unsigned long __user *)ptce)) + break; + + if (kvmppc_emulated_h_put_tce(tt, + ioba + (i IOMMU_PAGE_SHIFT), tce)) + break; + } + if (i == npages) + return H_SUCCESS; + + /* Failed, do cleanup */ + do { + --i; + kvmppc_emulated_h_put_tce(tt, ioba + (i IOMMU_PAGE_SHIFT), + 0); + } while (i); Hrm, so, actually PAPR specifies that this hcall is supposed to first copy the given tces to hypervisor memory, then translate (and validate) them all, and only then touch the actual TCE table. Rather more complicated to do, but I guess we should - that would get rid
Re: [PATCH v2 1/4] powerpc: hard_irq_disable(): Call trace_hardirqs_off after disabling
On Thu, 2013-05-09 at 22:09 -0500, Scott Wood wrote: lockdep.c has this: /* * So we're supposed to get called after you mask local IRQs, * but for some reason the hardware doesn't quite think you did * a proper job. */ if (DEBUG_LOCKS_WARN_ON(!irqs_disabled())) return; Since irqs_disabled() is based on soft_enabled(), that (not just the hard EE bit) needs to be 0 before we call trace_hardirqs_off. Signed-off-by: Scott Wood scottw...@freescale.com Oops ;-) I'll apply that to my tree and will send it to Linus right after -rc1, the rest will go the normal way for KVM patches. Cheers, Ben. --- arch/powerpc/include/asm/hw_irq.h |5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h index d615b28..ba713f1 100644 --- a/arch/powerpc/include/asm/hw_irq.h +++ b/arch/powerpc/include/asm/hw_irq.h @@ -96,11 +96,12 @@ static inline bool arch_irqs_disabled(void) #endif #define hard_irq_disable() do {\ + u8 _was_enabled = get_paca()-soft_enabled; \ __hard_irq_disable(); \ - if (local_paca-soft_enabled) \ - trace_hardirqs_off(); \ get_paca()-soft_enabled = 0; \ get_paca()-irq_happened |= PACA_IRQ_HARD_DIS; \ + if (_was_enabled) \ + trace_hardirqs_off(); \ } while(0) static inline bool lazy_irq_pending(void) -- 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: [RFC PATCH 1/2] ARM: KVM: move GIC/timer code to a common location
On Thu, 9 May 2013 11:11:01 -0700, Christoffer Dall cd...@cs.columbia.edu wrote: On Fri, May 03, 2013 at 03:02:52PM +0100, Marc Zyngier wrote: As KVM/arm64 is looming on the horizon, it makes sense to move some of the common code to a single location in order to reduce duplication. The code could live anywhere. Actually, most of KVM is already built with a bunch of ugly ../../.. hacks in the various Makefiles, so we're not exactly talking about style here. But maybe it is time to start moving into a less ugly direction. The include files must be in a public location, as they are accessed from non-KVM files (arch/arm/kernel/asm-offsets.c). For this purpose, introduce two new locations: - virt/kvm/arm/ : x86 and ia64 already share the ioapic code in virt/kvm, so this could be seen as a (very ugly) precedent. - include/kvm/ : there is already an include/xen, and while the intent is slightly different, this seems as good a location as any This overall looks ok, just a few points: 1. Should we have a namespace per arch in the include directory, as in include/kvm/arm? So I thought of that at one point, but discarded the idea because it seems to convey the wrong message: We're moving the include files because they are architecture independent, and referring to an architecture name in the path feels a bit odd. Or maybe arm-common? I don't have strong feelings about it though... 2. We could drop the kvm_ prefix from the include files now Agreed. It would be interesting to see what the KVM maintainers think of all this. Gleb? Paolo? M. -- Fast, cheap, reliable. Pick two. -- 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: how emulated disk IO translated to physical disk IO on host side
On Tue, May 07, 2013 at 09:58:52AM -0500, sheng qiu wrote: i am trying to figure out the code path which translate the emulated disk IO issued by VM to actual physical disk IO on host side. Can anyone give me a clear view about this? For an overview of the stack: http://events.linuxfoundation.org/slides/2011/linuxcon-japan/lcj2011_hajnoczi.pdf i read the kvm side code about the VMexit handling, the handle_io() will be called for IO exits. for IO job that cannot be handled inside the hyperviser, it will switch to qemu-kvm process and the handle_io() at qemu side will be called. finally it seems invoke the ioport_read()/ioport_write() which invoke the actual registered read/write operator. Then i get lost here, i do not know for emulated disk io which function will be responsible for the remaining job. i.e. catch the cmd for accessing the virtual disk and translate to the read/write to offset of the disk img file (assume we use file for virtual disk), and then issue the system call to the host to issue the real io cmd to physical disk. If you are running with a virtio-blk PCI adapter, then QEMU's virtio_queue_host_notifier_read() or virtio_ioport_write() is invoked (it depends whether ioeventfd is being used or regular I/O dispatch). Then QEMU's hw/block/virtio-blk.c will call bdrv_aio_readv()/bdrv_aio_writev()/bdrv_aio_flush(). This enters the QEMU block layer (see block.c and block/) where image file formats are handled. Eventually you get to block/raw-posix.c which issues either a preadv()/pwritev()/fdatasync() in a worker thread or a Linux AIO io_submit() (if -drive aio=native was used). Stefan -- 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 2/6] KVM: PPC: Add support for multiple-TCE hcalls
On 05/10/2013 04:51 PM, David Gibson wrote: On Mon, May 06, 2013 at 05:25:53PM +1000, Alexey Kardashevskiy wrote: This adds real mode handlers for the H_PUT_TCE_INDIRECT and H_STUFF_TCE hypercalls for QEMU emulated devices such as virtio devices or emulated PCI. These calls allow adding multiple entries (up to 512) into the TCE table in one call which saves time on transition to/from real mode. This adds a guest physical to host real address converter and calls the existing H_PUT_TCE handler. The converting function is going to be fully utilized by upcoming VFIO supporting patches. This also implements the KVM_CAP_PPC_MULTITCE capability, so in order to support the functionality of this patch, QEMU needs to query for this capability and set the hcall-multi-tce hypertas property only if the capability is present, otherwise there will be serious performance degradation. Hrm. Clearly I didn't read this carefully enough before. There are some problems here. ? [snip] diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c index 72ffc89..643ac1e 100644 --- a/arch/powerpc/kvm/book3s_64_vio.c +++ b/arch/powerpc/kvm/book3s_64_vio.c @@ -14,6 +14,7 @@ * * Copyright 2010 Paul Mackerras, IBM Corp. pau...@au1.ibm.com * Copyright 2011 David Gibson, IBM Corporation d...@au1.ibm.com + * Copyright 2013 Alexey Kardashevskiy, IBM Corporation a...@au1.ibm.com */ #include linux/types.h @@ -36,9 +37,14 @@ #include asm/ppc-opcode.h #include asm/kvm_host.h #include asm/udbg.h +#include asm/iommu.h #define TCES_PER_PAGE (PAGE_SIZE / sizeof(u64)) +#define ERROR_ADDR (~(unsigned long)0x0) +/* + * TCE tables handlers. + */ static long kvmppc_stt_npages(unsigned long window_size) { return ALIGN((window_size SPAPR_TCE_SHIFT) @@ -148,3 +154,111 @@ fail: } return ret; } + +/* + * Virtual mode handling of IOMMU map/unmap. + */ +/* Converts guest physical address into host virtual */ +static unsigned long get_virt_address(struct kvm_vcpu *vcpu, +unsigned long gpa) This should probably return a void * rather than an unsigned long. Well, actually a void __user *. +{ +unsigned long hva, gfn = gpa PAGE_SHIFT; +struct kvm_memory_slot *memslot; + +memslot = search_memslots(kvm_memslots(vcpu-kvm), gfn); +if (!memslot) +return ERROR_ADDR; + +/* + * Convert gfn to hva preserving flags and an offset + * within a system page + */ +hva = __gfn_to_hva_memslot(memslot, gfn) + (gpa ~PAGE_MASK); +return hva; +} + +long kvmppc_virtmode_h_put_tce(struct kvm_vcpu *vcpu, +unsigned long liobn, unsigned long ioba, +unsigned long tce) +{ +struct kvmppc_spapr_tce_table *tt; + +tt = kvmppc_find_tce_table(vcpu, liobn); +/* Didn't find the liobn, put it to userspace */ +if (!tt) +return H_TOO_HARD; + +/* Emulated IO */ +return kvmppc_emulated_h_put_tce(tt, ioba, tce); +} + +long kvmppc_virtmode_h_put_tce_indirect(struct kvm_vcpu *vcpu, +unsigned long liobn, unsigned long ioba, +unsigned long tce_list, unsigned long npages) +{ +struct kvmppc_spapr_tce_table *tt; +long i; +unsigned long tces; + +/* The whole table addressed by tce_list resides in 4K page */ +if (npages 512) +return H_PARAMETER; So, that doesn't actually verify what the comment says it does - only that the list is 4K in total. You need to check the alignment of tce_list as well. The spec says to return H_PARAMETER if 512. I.e. it takes just 1 page and I do not need to bother if pages may not lay continuously in RAM (matters for real mode). /* * As the spec is saying that maximum possible number of TCEs is 512, * the whole TCE page is no more than 4K. Therefore we do not have to * worry if pages do not lie continuously in the RAM */ Any better?... + +tt = kvmppc_find_tce_table(vcpu, liobn); +/* Didn't find the liobn, put it to userspace */ +if (!tt) +return H_TOO_HARD; + +tces = get_virt_address(vcpu, tce_list); +if (tces == ERROR_ADDR) +return H_TOO_HARD; + +/* Emulated IO */ This comment doesn't seem to have any bearing on the test which follows it. +if ((ioba + (npages IOMMU_PAGE_SHIFT)) tt-window_size) +return H_PARAMETER; + +for (i = 0; i npages; ++i) { +unsigned long tce; +unsigned long ptce = tces + i * sizeof(unsigned long); + +if (get_user(tce, (unsigned long __user *)ptce)) +break; + +if (kvmppc_emulated_h_put_tce(tt, +ioba + (i IOMMU_PAGE_SHIFT), tce)) +break; +} +if (i == npages) +return H_SUCCESS; + +/* Failed, do cleanup */ +do { +--i; +
Re: [RFC PATCH 1/2] ARM: KVM: move GIC/timer code to a common location
Il 10/05/2013 09:23, Marc Zyngier ha scritto: On Thu, 9 May 2013 11:11:01 -0700, Christoffer Dall cd...@cs.columbia.edu wrote: On Fri, May 03, 2013 at 03:02:52PM +0100, Marc Zyngier wrote: As KVM/arm64 is looming on the horizon, it makes sense to move some of the common code to a single location in order to reduce duplication. The code could live anywhere. Actually, most of KVM is already built with a bunch of ugly ../../.. hacks in the various Makefiles, so we're not exactly talking about style here. But maybe it is time to start moving into a less ugly direction. The include files must be in a public location, as they are accessed from non-KVM files (arch/arm/kernel/asm-offsets.c). For this purpose, introduce two new locations: - virt/kvm/arm/ : x86 and ia64 already share the ioapic code in virt/kvm, so this could be seen as a (very ugly) precedent. - include/kvm/ : there is already an include/xen, and while the intent is slightly different, this seems as good a location as any This overall looks ok, just a few points: 1. Should we have a namespace per arch in the include directory, as in include/kvm/arm? So I thought of that at one point, but discarded the idea because it seems to convey the wrong message: We're moving the include files because they are architecture independent, and referring to an architecture name in the path feels a bit odd. Or maybe arm-common? I don't have strong feelings about it though... 2. We could drop the kvm_ prefix from the include files now Agreed. It would be interesting to see what the KVM maintainers think of all this. Gleb? Paolo? include/kvm is good, there is no user-level API to care about. Perhaps you can name the includes kvm/arm_vgic.h and kvm/arm_arch_timer.h. It keeps the tree shallow but at the same time it suggests some parallel between the source tree and the include tree. virt/kvm/arm is certainly better than anything else that comes to mind :) but I'm not a big fan of $(addprefix); this looks tidier to me: KVM := ../../../virt/kvm kvm-arm-y = $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o ... +obj-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic.o +obj-$(CONFIG_KVM_ARM_TIMER) += $(KVM)/arm/arch_timer.o 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: [RFC PATCH 1/2] ARM: KVM: move GIC/timer code to a common location
Il 10/05/2013 09:23, Marc Zyngier ha scritto: 1. Should we have a namespace per arch in the include directory, as in include/kvm/arm? So I thought of that at one point, but discarded the idea because it seems to convey the wrong message: We're moving the include files because they are architecture independent, and referring to an architecture name in the path feels a bit odd. Or maybe arm-common? As I wrote in the other message, Linux in general has a shallow include/ tree, so I think putting them in include/kvm/ is good. Is there any precedent for naming stuff that is common to arm and aarch64? I think to 99% of the world they will both be arm, but of course the remaining 1% is likely over-represented among KVM-ARM maintainers. :) 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: [RFC PATCH 1/2] ARM: KVM: move GIC/timer code to a common location
On 10/05/13 09:11, Paolo Bonzini wrote: Il 10/05/2013 09:23, Marc Zyngier ha scritto: 1. Should we have a namespace per arch in the include directory, as in include/kvm/arm? So I thought of that at one point, but discarded the idea because it seems to convey the wrong message: We're moving the include files because they are architecture independent, and referring to an architecture name in the path feels a bit odd. Or maybe arm-common? As I wrote in the other message, Linux in general has a shallow include/ tree, so I think putting them in include/kvm/ is good. Is there any precedent for naming stuff that is common to arm and aarch64? So far, we have: - include/linux/irqchip/arm-gic.h - include/clocksource/arm_arch_timer.h So the trend seems to use arm as a prefix, and I will rename the files to match this convention (which you actually suggested in your other email). I think to 99% of the world they will both be arm, but of course the remaining 1% is likely over-represented among KVM-ARM maintainers. :) Who? What? ;-) Do you have any comment about patch 2/2? It is a bit more invasive, but it is a cleanup in my opinion. Thanks for the feedback, M. -- Jazz is not dead. It just smells funny... -- 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] powerpc: export debug register save function for KVM
On 07.05.2013, at 11:40, Bharat Bhushan wrote: KVM need this function when switching from vcpu to user-space thread. My subsequent patch will use this function. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/include/asm/switch_to.h |4 arch/powerpc/kernel/process.c|3 ++- 2 files changed, 6 insertions(+), 1 deletions(-) diff --git a/arch/powerpc/include/asm/switch_to.h b/arch/powerpc/include/asm/switch_to.h index 200d763..50b357f 100644 --- a/arch/powerpc/include/asm/switch_to.h +++ b/arch/powerpc/include/asm/switch_to.h @@ -30,6 +30,10 @@ extern void enable_kernel_spe(void); extern void giveup_spe(struct task_struct *); extern void load_up_spe(struct task_struct *); +#ifdef CONFIG_PPC_ADV_DEBUG_REGS +extern void switch_booke_debug_regs(struct thread_struct *new_thread); +#endif + #ifndef CONFIG_SMP extern void discard_lazy_cpu_state(void); #else diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c index ca89375..a938138 100644 --- a/arch/powerpc/kernel/process.c +++ b/arch/powerpc/kernel/process.c @@ -362,12 +362,13 @@ static void prime_debug_regs(struct thread_struct *thread) * debug registers, set the debug registers from the values * stored in the new thread. */ -static void switch_booke_debug_regs(struct thread_struct *new_thread) +void switch_booke_debug_regs(struct thread_struct *new_thread) { if ((current-thread.debug.dbcr0 DBCR0_IDM) || (new_thread-debug.dbcr0 DBCR0_IDM)) prime_debug_regs(new_thread); } +EXPORT_SYMBOL(switch_booke_debug_regs); EXPORT_SYMBOL_GPL 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/ppc/booke64: fix build breakage from Altivec, and disable e6500
-Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. Add the new vectors, but there's more than this required to make Altivec work in the guest, and we can't prevent the guest from turning on Altivec (which can corrupt host state until state save/restore is implemented). Disable e6500 on KVM until this is fixed. To be more precise it can corrupt Altivec host state. Signed-off-by: Scott Wood scottw...@freescale.com Cc: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/bookehv_interrupts.S |4 arch/powerpc/kvm/e500mc.c |2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index e8ed7d6..6397613 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -309,6 +309,10 @@ kvm_handler BOOKE_INTERRUPT_DOORBELL_CRITICAL, EX_PARAMS(CRIT), \ SPRN_CSRR0, SPRN_CSRR1, 0 kvm_handler BOOKE_INTERRUPT_HV_PRIV, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, NEED_EMU +kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 +kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 Why not NEED_ESR as we did in our SDK? kvm_handler BOOKE_INTERRUPT_HV_SYSCALL, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, 0 kvm_handler BOOKE_INTERRUPT_GUEST_DBELL, EX_PARAMS(GDBELL), \ diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index 753cc99..19c8379 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -177,8 +177,6 @@ int kvmppc_core_check_processor_compat(void) r = 0; else if (strcmp(cur_cpu_spec-cpu_name, e5500) == 0) r = 0; - else if (strcmp(cur_cpu_spec-cpu_name, e6500) == 0) - r = 0; Hmm ... I made it clear in the commit message that Altivec is not supported, why couldn't we be more gentle: diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index c3bdc0a..4f43a36 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -172,8 +172,11 @@ int kvmppc_core_check_processor_compat(void) r = 0; else if (strcmp(cur_cpu_spec-cpu_name, e5500) == 0) r = 0; +#ifndef CONFIG_ALTIVEC +/* ALTIVEC is not supported now by KVM so only one of them can work */ else if (strcmp(cur_cpu_spec-cpu_name, e6500) == 0) r = 0; +#endif else r = -ENOTSUPP; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 82f155e..bb1a9e0 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -973,6 +973,16 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, break; #endif + case BOOKE_INTERRUPT_ALTIVEC_UNAVAIL: + case BOOKE_INTERRUPT_ALTIVEC_ASSIST: + /* +* Guest wants ALTIVEC, but host kernel doesn't support it. +* send an unimplemented operation program check to the guest. +*/ + kvmppc_core_queue_program(vcpu, ESR_PUO | ESR_SPV); + r = RESUME_GUEST; + break; + case BOOKE_INTERRUPT_DATA_STORAGE: kvmppc_core_queue_data_storage(vcpu, vcpu-arch.fault_dear, vcpu-arch.fault_esr); Kconfig should also be updated (in both proposals). -Mike -- 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: [RFC PATCH 2/2] ARM: KVM: standalone Makefile for vgic and timers
Il 03/05/2013 16:02, Marc Zyngier ha scritto: On the road to turn KVM/arm64 into something more sensible, change the way we compile the VGIC and arch_timer code by using local Makefiles instead of the very backward method we used before. Signed-off-by: Marc Zyngier marc.zyng...@arm.com --- Makefile | 2 +- arch/arm/kvm/Makefile | 2 -- virt/Makefile | 1 + virt/kvm/Makefile | 1 + virt/kvm/arm/Makefile | 2 ++ 5 files changed, 5 insertions(+), 3 deletions(-) create mode 100644 virt/Makefile create mode 100644 virt/kvm/Makefile create mode 100644 virt/kvm/arm/Makefile diff --git a/Makefile b/Makefile index 6db672b..19d51df 100644 --- a/Makefile +++ b/Makefile @@ -732,7 +732,7 @@ export mod_sign_cmd ifeq ($(KBUILD_EXTMOD),) -core-y += kernel/ mm/ fs/ ipc/ security/ crypto/ block/ +core-y += kernel/ mm/ fs/ ipc/ security/ crypto/ block/ virt/ vmlinux-dirs := $(patsubst %/,%,$(filter %/, $(init-y) $(init-m) \ $(core-y) $(core-m) $(drivers-y) $(drivers-m) \ diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile index 110d6da..e0540fd 100644 --- a/arch/arm/kvm/Makefile +++ b/arch/arm/kvm/Makefile @@ -20,5 +20,3 @@ kvm-arm-y = $(addprefix $(KVM)/, kvm_main.o coalesced_mmio.o) obj-y += kvm-arm.o init.o interrupts.o obj-y += arm.o handle_exit.o guest.o mmu.o emulate.o reset.o obj-y += coproc.o coproc_a15.o mmio.o psci.o perf.o -obj-$(CONFIG_KVM_ARM_VGIC) += $(addprefix $(KVM)/arm/, vgic.o) -obj-$(CONFIG_KVM_ARM_TIMER) += $(addprefix $(KVM)/arm/, arch_timer.o) diff --git a/virt/Makefile b/virt/Makefile new file mode 100644 index 000..c3b9ad0 --- /dev/null +++ b/virt/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_KVM)+= kvm/ diff --git a/virt/kvm/Makefile b/virt/kvm/Makefile new file mode 100644 index 000..dc87943 --- /dev/null +++ b/virt/kvm/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_KVM_ARM_HOST) += arm/ diff --git a/virt/kvm/arm/Makefile b/virt/kvm/arm/Makefile new file mode 100644 index 000..4df943b --- /dev/null +++ b/virt/kvm/arm/Makefile @@ -0,0 +1,2 @@ +obj-$(CONFIG_KVM_ARM_VGIC) += vgic.o +obj-$(CONFIG_KVM_ARM_TIMER) += arch_timer.o I'm not sure it is a good idea to do this just for ARM, and I think the patch would look quite different if you were to support x86 (because x86 builds KVM as a module). So I would hold off this for now. OTOH, it is a good idea to use the same idiom to access ../../../virt/kvm from the arch directories. Can you change x86/PPC/s390/ia64 as well to use $(KVM) instead of $(addprefix)? 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: [RFC PATCH 2/2] ARM: KVM: standalone Makefile for vgic and timers
On 10/05/13 10:39, Paolo Bonzini wrote: Il 03/05/2013 16:02, Marc Zyngier ha scritto: On the road to turn KVM/arm64 into something more sensible, change the way we compile the VGIC and arch_timer code by using local Makefiles instead of the very backward method we used before. Signed-off-by: Marc Zyngier marc.zyng...@arm.com --- Makefile | 2 +- arch/arm/kvm/Makefile | 2 -- virt/Makefile | 1 + virt/kvm/Makefile | 1 + virt/kvm/arm/Makefile | 2 ++ 5 files changed, 5 insertions(+), 3 deletions(-) create mode 100644 virt/Makefile create mode 100644 virt/kvm/Makefile create mode 100644 virt/kvm/arm/Makefile diff --git a/Makefile b/Makefile index 6db672b..19d51df 100644 --- a/Makefile +++ b/Makefile @@ -732,7 +732,7 @@ export mod_sign_cmd ifeq ($(KBUILD_EXTMOD),) -core-y += kernel/ mm/ fs/ ipc/ security/ crypto/ block/ +core-y += kernel/ mm/ fs/ ipc/ security/ crypto/ block/ virt/ vmlinux-dirs:= $(patsubst %/,%,$(filter %/, $(init-y) $(init-m) \ $(core-y) $(core-m) $(drivers-y) $(drivers-m) \ diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile index 110d6da..e0540fd 100644 --- a/arch/arm/kvm/Makefile +++ b/arch/arm/kvm/Makefile @@ -20,5 +20,3 @@ kvm-arm-y = $(addprefix $(KVM)/, kvm_main.o coalesced_mmio.o) obj-y += kvm-arm.o init.o interrupts.o obj-y += arm.o handle_exit.o guest.o mmu.o emulate.o reset.o obj-y += coproc.o coproc_a15.o mmio.o psci.o perf.o -obj-$(CONFIG_KVM_ARM_VGIC) += $(addprefix $(KVM)/arm/, vgic.o) -obj-$(CONFIG_KVM_ARM_TIMER) += $(addprefix $(KVM)/arm/, arch_timer.o) diff --git a/virt/Makefile b/virt/Makefile new file mode 100644 index 000..c3b9ad0 --- /dev/null +++ b/virt/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_KVM) += kvm/ diff --git a/virt/kvm/Makefile b/virt/kvm/Makefile new file mode 100644 index 000..dc87943 --- /dev/null +++ b/virt/kvm/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_KVM_ARM_HOST) += arm/ diff --git a/virt/kvm/arm/Makefile b/virt/kvm/arm/Makefile new file mode 100644 index 000..4df943b --- /dev/null +++ b/virt/kvm/arm/Makefile @@ -0,0 +1,2 @@ +obj-$(CONFIG_KVM_ARM_VGIC) += vgic.o +obj-$(CONFIG_KVM_ARM_TIMER) += arch_timer.o I'm not sure it is a good idea to do this just for ARM, and I think the patch would look quite different if you were to support x86 (because x86 builds KVM as a module). So I would hold off this for now. Fair enough. OTOH, it is a good idea to use the same idiom to access ../../../virt/kvm from the arch directories. Can you change x86/PPC/s390/ia64 as well to use $(KVM) instead of $(addprefix)? Sure. I'll rework the series to include the other arches. Thanks, M. -- Jazz is not dead. It just smells funny... -- 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: PPC: Add userspace debug stub support
On 07.05.2013, at 11:40, Bharat Bhushan wrote: This patch adds the debug stub support on booke/bookehv. Now QEMU debug stub can use hw breakpoint, watchpoint and software breakpoint to debug guest. This is how we save/restore debug register context when switching between guest, userspace and kernel user-process: When QEMU is running - thread-debug_reg == QEMU debug register context. - Kernel will handle switching the debug register on context switch. - no vcpu_load() called QEMU makes ioctls (except RUN) - This will call vcpu_load() - should not change context. - Some ioctls can change vcpu debug register, context saved in vcpu-debug_regs QEMU Makes RUN ioctl - Save thread-debug_reg on STACK - Store thread-debug_reg == vcpu-debug_reg - load thread-debug_reg - RUN VCPU ( So thread points to vcpu context ) Context switch happens When VCPU running - makes vcpu_load() should not load any context - kernel loads the vcpu context as thread-debug_regs points to vcpu context. On heavyweight_exit - Load the context saved on stack in thread-debug_reg Currently we do not support debug resource emulation to guest, On debug exception, always exit to user space irrespective of user space is expecting the debug exception or not. If this is unexpected exception (breakpoint/watchpoint event not set by userspace) then let us leave the action on user space. This is similar to what it was before, only thing is that now we have proper exit state available to user space. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/include/asm/kvm_host.h |3 + arch/powerpc/include/uapi/asm/kvm.h |1 + arch/powerpc/kvm/booke.c| 242 --- arch/powerpc/kvm/booke.h|5 + 4 files changed, 233 insertions(+), 18 deletions(-) diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index 838a577..1b29945 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -524,7 +524,10 @@ struct kvm_vcpu_arch { u32 eptcfg; u32 epr; u32 crit_save; + /* guest debug registers*/ struct debug_reg dbg_reg; + /* shadow debug registers */ Please be more verbose here. What exactly does this contain? Why do we need shadow and non-shadow registers? The comment as it is reads like /* Add one plus one */ x = 1 + 1; + struct debug_reg shadow_dbg_reg; #endif gpa_t paddr_accessed; gva_t vaddr_accessed; diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h index ded0607..f5077c2 100644 --- a/arch/powerpc/include/uapi/asm/kvm.h +++ b/arch/powerpc/include/uapi/asm/kvm.h @@ -27,6 +27,7 @@ #define __KVM_HAVE_PPC_SMT #define __KVM_HAVE_IRQCHIP #define __KVM_HAVE_IRQ_LINE +#define __KVM_HAVE_GUEST_DEBUG struct kvm_regs { __u64 pc; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index ef99536..6a44ad4 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -133,6 +133,29 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu) #endif } +static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) +{ + /* Synchronize guest's desire to get debug interrupts into shadow MSR */ +#ifndef CONFIG_KVM_BOOKE_HV + vcpu-arch.shadow_msr = ~MSR_DE; + vcpu-arch.shadow_msr |= vcpu-arch.shared-msr MSR_DE; +#endif + + /* Force enable debug interrupts when user space wants to debug */ + if (vcpu-guest_debug) { +#ifdef CONFIG_KVM_BOOKE_HV + /* + * Since there is no shadow MSR, sync MSR_DE into the guest + * visible MSR. + */ + vcpu-arch.shared-msr |= MSR_DE; +#else + vcpu-arch.shadow_msr |= MSR_DE; + vcpu-arch.shared-msr = ~MSR_DE; +#endif + } +} + /* * Helper function for full MSR writes. No need to call this if only * EE/CE/ME/DE/RI are changing. @@ -150,6 +173,7 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr) kvmppc_mmu_msr_notify(vcpu, old_msr); kvmppc_vcpu_sync_spe(vcpu); kvmppc_vcpu_sync_fpu(vcpu); + kvmppc_vcpu_sync_debug(vcpu); } static void kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu, @@ -655,6 +679,7 @@ int kvmppc_core_check_requests(struct kvm_vcpu *vcpu) int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu) { int ret, s; + struct debug_reg debug; #ifdef CONFIG_PPC_FPU unsigned int fpscr; int fpexc_mode; @@ -699,11 +724,27 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu) kvmppc_load_guest_fp(vcpu); #endif + /* + * Clear current-thread.dbcr0 so that kernel does not + * restore h/w registers on context switch in vcpu running state. + */ Incorrect comment? + /* Save thread-debug context on stack */ /* Switch to guest debug
Re: [RFC 1/2] virtio_balloon: move balloon_lock mutex to callers
On Thu, 9 May 2013 18:03:09 -0300 Rafael Aquini aqu...@redhat.com wrote: On Thu, May 09, 2013 at 10:53:48AM -0400, Luiz Capitulino wrote: This commit moves the balloon_lock mutex out of the fill_balloon() and leak_balloon() functions to their callers. The reason for this change is that the next commit will introduce a shrinker callback for the balloon driver, which will also call leak_balloon() but will require different locking semantics. Signed-off-by: Luiz Capitulino lcapitul...@redhat.com --- drivers/virtio/virtio_balloon.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index bd3ae32..9d5fe2b 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -133,7 +133,6 @@ static void fill_balloon(struct virtio_balloon *vb, size_t num) /* We can only do one array worth at a time. */ num = min(num, ARRAY_SIZE(vb-pfns)); - mutex_lock(vb-balloon_lock); for (vb-num_pfns = 0; vb-num_pfns num; vb-num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) { struct page *page = balloon_page_enqueue(vb_dev_info); @@ -154,7 +153,6 @@ static void fill_balloon(struct virtio_balloon *vb, size_t num) /* Did we get any? */ if (vb-num_pfns != 0) tell_host(vb, vb-inflate_vq); - mutex_unlock(vb-balloon_lock); } static void release_pages_by_pfn(const u32 pfns[], unsigned int num) @@ -176,7 +174,6 @@ static void leak_balloon(struct virtio_balloon *vb, size_t num) /* We can only do one array worth at a time. */ num = min(num, ARRAY_SIZE(vb-pfns)); - mutex_lock(vb-balloon_lock); for (vb-num_pfns = 0; vb-num_pfns num; vb-num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) { page = balloon_page_dequeue(vb_dev_info); @@ -192,7 +189,6 @@ static void leak_balloon(struct virtio_balloon *vb, size_t num) * is true, we *have* to do it in this order */ tell_host(vb, vb-deflate_vq); - mutex_unlock(vb-balloon_lock); release_pages_by_pfn(vb-pfns, vb-num_pfns); } @@ -305,11 +301,13 @@ static int balloon(void *_vballoon) || freezing(current)); if (vb-need_stats_update) stats_handle_request(vb); + mutex_lock(vb-balloon_lock); if (diff 0) fill_balloon(vb, diff); else if (diff 0) leak_balloon(vb, -diff); update_balloon_size(vb); + mutex_unlock(vb-balloon_lock); } return 0; } @@ -490,9 +488,11 @@ out: static void remove_common(struct virtio_balloon *vb) { /* There might be pages left in the balloon: free them. */ + mutex_lock(vb-balloon_lock); while (vb-num_pages) leak_balloon(vb, vb-num_pages); update_balloon_size(vb); + mutex_unlock(vb-balloon_lock); I think you will need to introduce the same change as above to virtballoon_restore() Thanks Rafael, I've fixed it in my tree. -- 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
[nVMX with: v3.9-11789-ge0fd9af] Stack trace when L2 guest is rebooted.
Heya, This is on Intel Haswell. First, some version info: L0, L1 -- both of them have same versions of kernel, qemu: = $ rpm -q kernel --changelog | head -2 * Thu May 09 2013 Josh Boyer - 3.10.0-0.rc0.git23.1 - Linux v3.9-11789-ge0fd9af = = $ uname -r ; rpm -q qemu-kvm libvirt-daemon-kvm libguestfs 3.10.0-0.rc0.git23.1.fc20.x86_64 qemu-kvm-1.4.1-1.fc19.x86_64 libvirt-daemon-kvm-1.0.5-2.fc19.x86_64 libguestfs-1.21.35-1.fc19.x86_64 = Additionally, neither nmi_watchdog, nor hpet enabled on L0 L1 kernels: = $ egrep -i 'nmi|hpet' /etc/grub2.cfg $ = KVM parameters on L0 : = $ cat /sys/module/kvm_intel/parameters/nested Y $ cat /sys/module/kvm_intel/parameters/enable_shadow_vmcs Y $ cat /sys/module/kvm_intel/parameters/enable_apicv N $ cat /sys/module/kvm_intel/parameters/ept Y = - That's the stack trace I'm seeing, when I start the L2 guest: ... [2.162235] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0) [2.163080] Pid: 1, comm: swapper/0 Not tainted 3.8.11-200.fc18.x86_64 #1 [2.163080] Call Trace: [2.163080] [81649c19] panic+0xc1/0x1d0 [2.163080] [81d010e0] mount_block_root+0x1fa/0x2ac [2.163080] [81d011e9] mount_root+0x57/0x5b [2.163080] [81d0132a] prepare_namespace+0x13d/0x176 [2.163080] [81d00e1c] kernel_init_freeable+0x1cf/0x1da [2.163080] [81d00610] ? do_early_param+0x8c/0x8c [2.163080] [81637ca0] ? rest_init+0x80/0x80 [2.163080] [81637cae] kernel_init+0xe/0xf0 [2.163080] [8165bd6c] ret_from_fork+0x7c/0xb0 [2.163080] [81637ca0] ? rest_init+0x80/0x80 [2.163080] Uhhuh. NMI received for unknown reason 30 on CPU 0. [2.163080] Do you have a strange power saving mode enabled? [2.163080] Dazed and confused, but trying to continue [2.163080] Uhhuh. NMI received for unknown reason 20 on CPU 0. [2.163080] Do you have a strange power saving mode enabled? [2.163080] Dazed and confused, but trying to continue [2.163080] Uhhuh. NMI received for unknown reason 30 on CPU 0. I'm able to reproduce to reproduce this consistently. L1 QEMU command-line: $ ps -ef | grep -i qemu qemu 4962 1 21 15:41 ?00:00:41 /usr/bin/qemu-system-x86_64 -machine accel=kvm -name regular-guest -S -machine pc-i440fx-1.4,accel=kvm,usb=off -cpu Haswell,+vmx -m 6144 -smp 4,sockets=4,cores=1,threads=1 -uuid 4ed9ac0b-7f72-dfcf-68b3-e6fe2ac588b2 -nographic -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/regular-guest.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -drive file=/home/test/vmimages/regular-guest.qcow2,if=none,id=drive-virtio-disk0,format=qcow2,cache=none -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -netdev tap,fd=23,id=hostnet0,vhost=on,vhostfd=24 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:80:c1:34,bus=pci.0,addr=0x3 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -device usb-tablet,id=input0 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 L2 QEMU command-line: $ qemu 2042 1 0 May09 ?00:05:03 /usr/bin/qemu-system-x86_64 -machine accel=kvm -name nested-guest -S -machine pc-i440fx-1.4,accel=kvm,usb=off -m 2048 -smp 2,sockets=2,cores=1,threads=1 -uuid 02ea8988-1054-b08b-bafe-cfbe9659976c -nographic -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/nested-guest.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -drive file=/home/test/vmimages/nested-guest.qcow2,if=none,id=drive-virtio-disk0,format=qcow2,cache=none -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -netdev tap,fd=23,id=hostnet0,vhost=on,vhostfd=24 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:65:c4:e6,bus=pci.0,addr=0x3 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -device usb-tablet,id=input0 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 I attached the vmxcap script output. Before I debug further, anyone has hints here? Many thanks in advance. [1] Notes -- https://github.com/kashyapc/nested-virt-notes-intel-f18 /kashyap Basic VMX Information Revision 18 VMCS size1024 VMCS restricted to 32 bit addresses no Dual-monitor support yes VMCS memory type 6 INS/OUTS instruction information yes IA32_VMX_TRUE_*_CTLS
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. How hard would it be to add? I suppose it's broken in 3.10, so we need something quick before it gets released? Add the new vectors, but there's more than this required to make Altivec work in the guest, and we can't prevent the guest from turning on Altivec (which can corrupt host state until state save/restore is implemented). Disable e6500 on KVM until this is fixed. To be more precise it can corrupt Altivec host state. Signed-off-by: Scott Wood scottw...@freescale.com Cc: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/bookehv_interrupts.S |4 arch/powerpc/kvm/e500mc.c |2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index e8ed7d6..6397613 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -309,6 +309,10 @@ kvm_handler BOOKE_INTERRUPT_DOORBELL_CRITICAL, EX_PARAMS(CRIT), \ SPRN_CSRR0, SPRN_CSRR1, 0 kvm_handler BOOKE_INTERRUPT_HV_PRIV, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, NEED_EMU +kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \ +SPRN_SRR0, SPRN_SRR1, 0 +kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \ +SPRN_SRR0, SPRN_SRR1, 0 Why not NEED_ESR as we did in our SDK? kvm_handler BOOKE_INTERRUPT_HV_SYSCALL, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, 0 kvm_handler BOOKE_INTERRUPT_GUEST_DBELL, EX_PARAMS(GDBELL), \ diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index 753cc99..19c8379 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -177,8 +177,6 @@ int kvmppc_core_check_processor_compat(void) r = 0; else if (strcmp(cur_cpu_spec-cpu_name, e5500) == 0) r = 0; -else if (strcmp(cur_cpu_spec-cpu_name, e6500) == 0) -r = 0; Hmm ... I made it clear in the commit message that Altivec is not supported, why couldn't we be more gentle: diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index c3bdc0a..4f43a36 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -172,8 +172,11 @@ int kvmppc_core_check_processor_compat(void) r = 0; else if (strcmp(cur_cpu_spec-cpu_name, e5500) == 0) r = 0; +#ifndef CONFIG_ALTIVEC This would still be wrong, since running 2 guests with altivec would break each other. Alex +/* ALTIVEC is not supported now by KVM so only one of them can work */ else if (strcmp(cur_cpu_spec-cpu_name, e6500) == 0) r = 0; +#endif else r = -ENOTSUPP; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 82f155e..bb1a9e0 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -973,6 +973,16 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, break; #endif + case BOOKE_INTERRUPT_ALTIVEC_UNAVAIL: + case BOOKE_INTERRUPT_ALTIVEC_ASSIST: + /* +* Guest wants ALTIVEC, but host kernel doesn't support it. +* send an unimplemented operation program check to the guest. +*/ + kvmppc_core_queue_program(vcpu, ESR_PUO | ESR_SPV); + r = RESUME_GUEST; + break; + case BOOKE_INTERRUPT_DATA_STORAGE: kvmppc_core_queue_data_storage(vcpu, vcpu-arch.fault_dear, vcpu-arch.fault_esr); Kconfig should also be updated (in both proposals). -Mike -- 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: [RFC 2/2] virtio_balloon: auto-ballooning support
On Thu, 9 May 2013 18:15:19 -0300 Rafael Aquini aqu...@redhat.com wrote: On Thu, May 09, 2013 at 10:53:49AM -0400, Luiz Capitulino wrote: Automatic ballooning consists of dynamically adjusting the guest's balloon according to memory pressure in the host and in the guest. This commit implements the guest side of automatic balloning, which basically consists of registering a shrinker callback with the kernel, which will try to deflate the guest's balloon by the amount of pages being requested. The shrinker callback is only registered if the host supports the VIRTIO_BALLOON_F_AUTO_BALLOON feature bit. Automatic inflate is performed by the host. Here are some numbers. The test-case is to run 35 VMs (1G of RAM each) in parallel doing a kernel build. Host has 32GB of RAM and 16GB of swap. SWAP IN and SWAP OUT correspond to the number of pages swapped in and swapped out, respectively. Auto-ballooning disabled: RUN TIME(s) SWAP IN SWAP OUT 1634 930980 1588522 2610 627422 1362174 3649 1079847 1616367 4543 953289 1635379 5642 913237 1514000 Auto-ballooning enabled: RUN TIME(s) SWAP IN SWAP OUT 1629 901 12537 2624 981 18506 3626 573 9085 4631 2250 42534 5627 1610 20808 Signed-off-by: Luiz Capitulino lcapitul...@redhat.com --- Nice work Luiz! Just allow me a silly question, though. I have 100% more chances of committing sillynesses than you, so please go ahead. Since your shrinker doesn't change the balloon target size, Which target size are you referring to? The one in the host (member num_pages of VirtIOBalloon in QEMU)? If it the one in the host, then my understanding is that that member is only used to communicate the new balloon target to the guest. The guest driver will only read it when told (by the host) to do so, and when it does the target value will be correct. Am I right? as soon as the shrink round finishes the balloon will re-inflate again, won't it? Doesn't this cause a sort of balloon thrashing scenario, if both guest and host are suffering from memory pressure? The rest I have for the moment, are only nitpicks :) drivers/virtio/virtio_balloon.c | 55 + include/uapi/linux/virtio_balloon.h | 1 + 2 files changed, 56 insertions(+) diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index 9d5fe2b..f9dcae8 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -71,6 +71,9 @@ struct virtio_balloon /* Memory statistics */ int need_stats_update; struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR]; + + /* Memory shrinker */ + struct shrinker shrinker; }; static struct virtio_device_id id_table[] = { @@ -126,6 +129,7 @@ static void set_page_pfns(u32 pfns[], struct page *page) pfns[i] = page_to_balloon_pfn(page) + i; } +/* This function should be called with vb-balloon_mutex held */ static void fill_balloon(struct virtio_balloon *vb, size_t num) { struct balloon_dev_info *vb_dev_info = vb-vb_dev_info; @@ -166,6 +170,7 @@ static void release_pages_by_pfn(const u32 pfns[], unsigned int num) } } +/* This function should be called with vb-balloon_mutex held */ static void leak_balloon(struct virtio_balloon *vb, size_t num) { struct page *page; @@ -285,6 +290,45 @@ static void update_balloon_size(struct virtio_balloon *vb) actual, sizeof(actual)); } +static unsigned long balloon_get_nr_pages(const struct virtio_balloon *vb) +{ + return vb-num_pages / VIRTIO_BALLOON_PAGES_PER_PAGE; +} + +static int balloon_shrinker(struct shrinker *shrinker,struct shrink_control *sc) +{ + unsigned int nr_pages, new_target; + struct virtio_balloon *vb; + + vb = container_of(shrinker, struct virtio_balloon, shrinker); + if (!mutex_trylock(vb-balloon_lock)) { + return -1; + } + + nr_pages = balloon_get_nr_pages(vb); + if (!sc-nr_to_scan || !nr_pages) { + goto out; + } + + /* +* If the current balloon size is greater than the number of +* pages being reclaimed by the kernel, deflate only the needed +* amount. Otherwise deflate everything we have. +*/ + new_target = 0; + if (nr_pages sc-nr_to_scan) { + new_target = nr_pages - sc-nr_to_scan; + } + CodingStyle: you don't need the curly-braces for all these single staments above Oh, this comes from QEMU coding style. Fixed. + leak_balloon(vb, new_target); + update_balloon_size(vb); + nr_pages = balloon_get_nr_pages(vb); + +out: + mutex_unlock(vb-balloon_lock); + return nr_pages; +} + static int balloon(void
Re: [RFC 2/2] virtio_balloon: auto-ballooning support
On Fri, 10 May 2013 09:20:46 -0400 Luiz Capitulino lcapitul...@redhat.com wrote: On Thu, 9 May 2013 18:15:19 -0300 Rafael Aquini aqu...@redhat.com wrote: On Thu, May 09, 2013 at 10:53:49AM -0400, Luiz Capitulino wrote: Automatic ballooning consists of dynamically adjusting the guest's balloon according to memory pressure in the host and in the guest. This commit implements the guest side of automatic balloning, which basically consists of registering a shrinker callback with the kernel, which will try to deflate the guest's balloon by the amount of pages being requested. The shrinker callback is only registered if the host supports the VIRTIO_BALLOON_F_AUTO_BALLOON feature bit. Automatic inflate is performed by the host. Here are some numbers. The test-case is to run 35 VMs (1G of RAM each) in parallel doing a kernel build. Host has 32GB of RAM and 16GB of swap. SWAP IN and SWAP OUT correspond to the number of pages swapped in and swapped out, respectively. Auto-ballooning disabled: RUN TIME(s) SWAP IN SWAP OUT 1634 930980 1588522 2610 627422 1362174 3649 1079847 1616367 4543 953289 1635379 5642 913237 1514000 Auto-ballooning enabled: RUN TIME(s) SWAP IN SWAP OUT 1629 901 12537 2624 981 18506 3626 573 9085 4631 2250 42534 5627 1610 20808 Signed-off-by: Luiz Capitulino lcapitul...@redhat.com --- Nice work Luiz! Just allow me a silly question, though. I have 100% more chances of committing sillynesses than you, so please go ahead. Since your shrinker doesn't change the balloon target size, Which target size are you referring to? The one in the host (member num_pages of VirtIOBalloon in QEMU)? If it the one in the host, then my understanding is that that member is only used to communicate the new balloon target to the guest. The guest driver will only read it when told (by the host) to do so, and when it does the target value will be correct. Am I right? as soon as the shrink round finishes the balloon will re-inflate again, won't it? Doesn't this cause a sort of balloon thrashing scenario, if both guest and host are suffering from memory pressure? Forgot to say that I didn't observe this in my testing. But I'll try harder as soon as we clarify which target size we're talking about. -- 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/ppc/booke64: fix build breakage from Altivec, and disable e6500
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 4:16 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. How hard would it be to add? I suppose it's broken in 3.10, so we need something quick before it gets released? Not so hard. Yes. I was surprised by this patch given the fact that we have planned to send altivec support upstream this days and that we already have a similar patch from Tiejun on our list. Add the new vectors, but there's more than this required to make Altivec work in the guest, and we can't prevent the guest from turning on Altivec (which can corrupt host state until state save/restore is implemented). Disable e6500 on KVM until this is fixed. To be more precise it can corrupt Altivec host state. Signed-off-by: Scott Wood scottw...@freescale.com Cc: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/bookehv_interrupts.S |4 arch/powerpc/kvm/e500mc.c |2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index e8ed7d6..6397613 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -309,6 +309,10 @@ kvm_handler BOOKE_INTERRUPT_DOORBELL_CRITICAL, EX_PARAMS(CRIT), \ SPRN_CSRR0, SPRN_CSRR1, 0 kvm_handler BOOKE_INTERRUPT_HV_PRIV, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, NEED_EMU +kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 +kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 Why not NEED_ESR as we did in our SDK? kvm_handler BOOKE_INTERRUPT_HV_SYSCALL, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, 0 kvm_handler BOOKE_INTERRUPT_GUEST_DBELL, EX_PARAMS(GDBELL), \ diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index 753cc99..19c8379 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -177,8 +177,6 @@ int kvmppc_core_check_processor_compat(void) r = 0; else if (strcmp(cur_cpu_spec-cpu_name, e5500) == 0) r = 0; - else if (strcmp(cur_cpu_spec-cpu_name, e6500) == 0) - r = 0; Hmm ... I made it clear in the commit message that Altivec is not supported, why couldn't we be more gentle: diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index c3bdc0a..4f43a36 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -172,8 +172,11 @@ int kvmppc_core_check_processor_compat(void) r = 0; else if (strcmp(cur_cpu_spec-cpu_name, e5500) == 0) r = 0; +#ifndef CONFIG_ALTIVEC This would still be wrong, since running 2 guests with altivec would break each other. It's wrong if you expect to have altivec supported in the VM which is not the case. Guests that executes altivec instructions will get a program (unimplemented op) exception see below. Do we really need to remove e6500 all together for this? Alex +/* ALTIVEC is not supported now by KVM so only one of them can work */ else if (strcmp(cur_cpu_spec-cpu_name, e6500) == 0) r = 0; +#endif else r = -ENOTSUPP; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 82f155e..bb1a9e0 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -973,6 +973,16 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, break; #endif + case BOOKE_INTERRUPT_ALTIVEC_UNAVAIL: + case BOOKE_INTERRUPT_ALTIVEC_ASSIST: + /* +* Guest wants ALTIVEC, but host kernel doesn't support it. +* send an unimplemented operation program check to the guest. +*/ + kvmppc_core_queue_program(vcpu, ESR_PUO | ESR_SPV); + r = RESUME_GUEST; +
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On Fri, May 10, 2013 at 08:07:00AM +1000, Benjamin Herrenschmidt wrote: On Thu, 2013-05-09 at 16:27 -0500, Scott Wood wrote: On 05/09/2013 07:37:42 AM, Benjamin Herrenschmidt wrote: On Thu, 2013-05-09 at 17:44 +0800, tiejun.chen wrote: Actually in the case GS=1 even if EE=0, EXT/DEC/DBELL still occur as I recall. Only if directed to the hypervisor. This is always the case with KVM, right? At least on booke... Hrm, on A2 we could choose iirc. Well not DEC but EXT at least, I don't remember about DBELL. Case 1) - Local_irq_disable() will set soft_enabled = 0 - Now Externel interrupt happens, there we set PACA_IRQ_EE in irq_happened, Also clears EE in SRR1 and rfi. So interrupts are hard disabled. No more other interrupt gated by MSR.EE can happen. Looks like the idea here is to not let a device keep on inserting interrupt till the interrupt condition on device is cleared, right? The external interrupt line is level sensitive normally, so we have to mask MSR:EE in that case or the interrupt would keep re-occurring (note that FSL has this concept of auto-acked interrupts via the on die MPIC for which you can potentially use PACA_IRQ_EE_EDGE instead and avoid having to mask MSR:EE). Note that if we do this, we can no longer leave the interrupt vector in EPR, and would have to track (potentially multiple different) pending external interrupts in software. Right, you can have a little queue in the PACA and leave EE disabled if it's full. You can probably get away with a queue of 1 though :-) So I would assume you will not pick up these two patches, right? http://patchwork.ozlabs.org/patch/235530/ http://patchwork.ozlabs.org/patch/235532/ Anyway it is more easier to enable the external proxy by using this method. But if you insist, I can respin a patch to use the method you suggested since it will definitely reduce the window where the irq is hard disabled. Thanks, Kevin Cheers, Ben. -- 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 pgprrSZaX_six.pgp Description: PGP signature
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 10.05.2013, at 16:11, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 4:16 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. How hard would it be to add? I suppose it's broken in 3.10, so we need something quick before it gets released? Not so hard. Yes. I was surprised by this patch given the fact that we have planned to send altivec support upstream this days and that we already have a similar patch from Tiejun on our list. Add the new vectors, but there's more than this required to make Altivec work in the guest, and we can't prevent the guest from turning on Altivec (which can corrupt host state until state save/restore is implemented). Disable e6500 on KVM until this is fixed. To be more precise it can corrupt Altivec host state. Signed-off-by: Scott Wood scottw...@freescale.com Cc: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/bookehv_interrupts.S |4 arch/powerpc/kvm/e500mc.c |2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index e8ed7d6..6397613 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -309,6 +309,10 @@ kvm_handler BOOKE_INTERRUPT_DOORBELL_CRITICAL, EX_PARAMS(CRIT), \ SPRN_CSRR0, SPRN_CSRR1, 0 kvm_handler BOOKE_INTERRUPT_HV_PRIV, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, NEED_EMU +kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 +kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 Why not NEED_ESR as we did in our SDK? kvm_handler BOOKE_INTERRUPT_HV_SYSCALL, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, 0 kvm_handler BOOKE_INTERRUPT_GUEST_DBELL, EX_PARAMS(GDBELL), \ diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index 753cc99..19c8379 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -177,8 +177,6 @@ int kvmppc_core_check_processor_compat(void) r = 0; else if (strcmp(cur_cpu_spec-cpu_name, e5500) == 0) r = 0; - else if (strcmp(cur_cpu_spec-cpu_name, e6500) == 0) - r = 0; Hmm ... I made it clear in the commit message that Altivec is not supported, why couldn't we be more gentle: diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index c3bdc0a..4f43a36 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -172,8 +172,11 @@ int kvmppc_core_check_processor_compat(void) r = 0; else if (strcmp(cur_cpu_spec-cpu_name, e5500) == 0) r = 0; +#ifndef CONFIG_ALTIVEC This would still be wrong, since running 2 guests with altivec would break each other. It's wrong if you expect to have altivec supported in the VM which is not the case. Guests that executes altivec instructions will get a program (unimplemented op) exception see below. Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. 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: [nVMX with: v3.9-11789-ge0fd9af] Stack trace when L2 guest is rebooted.
On Fri, May 10, 2013 at 8:03 PM, Kashyap Chamarthy kashyap...@gmail.com wrote: Also, I'm able to reproduce this consistently: When I create an L2 guest: -- [.] [ 463.655031] Dazed and confused, but trying to continue [ 463.975563] Uhhuh. NMI received for unknown reason 20 on CPU 1. [ 463.976040] Do you have a strange power saving mode enabled? [ 463.976040] Dazed and confused, but trying to continue 29 199M 29 58.7M0 0 136k 0 0:25:02 0:07:20 0:17:42 153k [ 465.136405] Uhhuh. NMI received for unknown reason 30 on CPU 1. [ 465.137042] Do you have a strange power saving mode enabled? [ 465.137042] Dazed and confused, but trying to continue [ 466.645818] Uhhuh. NMI received for unknown reason 20 on CPU 1. [ 466.646044] Do you have a strange power saving mode enabled? [ 466.646044] Dazed and confused, but trying to continue [ 466.907999] Uhhuh. NMI received for unknown reason 30 on CPU 1. [ 466.908033] Do you have a strange power saving mode enabled? -- On Fri, May 10, 2013 at 6:30 PM, Kashyap Chamarthy kashyap...@gmail.com wrote: Heya, This is on Intel Haswell. First, some version info: L0, L1 -- both of them have same versions of kernel, qemu: = $ rpm -q kernel --changelog | head -2 * Thu May 09 2013 Josh Boyer - 3.10.0-0.rc0.git23.1 - Linux v3.9-11789-ge0fd9af = = $ uname -r ; rpm -q qemu-kvm libvirt-daemon-kvm libguestfs 3.10.0-0.rc0.git23.1.fc20.x86_64 qemu-kvm-1.4.1-1.fc19.x86_64 libvirt-daemon-kvm-1.0.5-2.fc19.x86_64 libguestfs-1.21.35-1.fc19.x86_64 = Additionally, neither nmi_watchdog, nor hpet enabled on L0 L1 kernels: = $ egrep -i 'nmi|hpet' /etc/grub2.cfg $ = KVM parameters on L0 : = $ cat /sys/module/kvm_intel/parameters/nested Y $ cat /sys/module/kvm_intel/parameters/enable_shadow_vmcs Y $ cat /sys/module/kvm_intel/parameters/enable_apicv N $ cat /sys/module/kvm_intel/parameters/ept Y = - That's the stack trace I'm seeing, when I start the L2 guest: ... [2.162235] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0) [2.163080] Pid: 1, comm: swapper/0 Not tainted 3.8.11-200.fc18.x86_64 #1 [2.163080] Call Trace: [2.163080] [81649c19] panic+0xc1/0x1d0 [2.163080] [81d010e0] mount_block_root+0x1fa/0x2ac [2.163080] [81d011e9] mount_root+0x57/0x5b [2.163080] [81d0132a] prepare_namespace+0x13d/0x176 [2.163080] [81d00e1c] kernel_init_freeable+0x1cf/0x1da [2.163080] [81d00610] ? do_early_param+0x8c/0x8c [2.163080] [81637ca0] ? rest_init+0x80/0x80 [2.163080] [81637cae] kernel_init+0xe/0xf0 [2.163080] [8165bd6c] ret_from_fork+0x7c/0xb0 [2.163080] [81637ca0] ? rest_init+0x80/0x80 [2.163080] Uhhuh. NMI received for unknown reason 30 on CPU 0. [2.163080] Do you have a strange power saving mode enabled? [2.163080] Dazed and confused, but trying to continue [2.163080] Uhhuh. NMI received for unknown reason 20 on CPU 0. [2.163080] Do you have a strange power saving mode enabled? [2.163080] Dazed and confused, but trying to continue [2.163080] Uhhuh. NMI received for unknown reason 30 on CPU 0. I'm able to reproduce to reproduce this consistently. L1 QEMU command-line: $ ps -ef | grep -i qemu qemu 4962 1 21 15:41 ?00:00:41 /usr/bin/qemu-system-x86_64 -machine accel=kvm -name regular-guest -S -machine pc-i440fx-1.4,accel=kvm,usb=off -cpu Haswell,+vmx -m 6144 -smp 4,sockets=4,cores=1,threads=1 -uuid 4ed9ac0b-7f72-dfcf-68b3-e6fe2ac588b2 -nographic -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/regular-guest.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -drive file=/home/test/vmimages/regular-guest.qcow2,if=none,id=drive-virtio-disk0,format=qcow2,cache=none -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -netdev tap,fd=23,id=hostnet0,vhost=on,vhostfd=24 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:80:c1:34,bus=pci.0,addr=0x3 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -device usb-tablet,id=input0 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 L2 QEMU command-line: $ qemu 2042 1 0 May09 ?00:05:03 /usr/bin/qemu-system-x86_64 -machine accel=kvm -name nested-guest -S -machine pc-i440fx-1.4,accel=kvm,usb=off -m 2048 -smp 2,sockets=2,cores=1,threads=1 -uuid 02ea8988-1054-b08b-bafe-cfbe9659976c -nographic -no-user-config -nodefaults -chardev
Re: [nVMX with: v3.9-11789-ge0fd9af] Stack trace when L2 guest is rebooted.
On 2013-05-10 15:00, Kashyap Chamarthy wrote: Heya, This is on Intel Haswell. First, some version info: L0, L1 -- both of them have same versions of kernel, qemu: = $ rpm -q kernel --changelog | head -2 * Thu May 09 2013 Josh Boyer - 3.10.0-0.rc0.git23.1 - Linux v3.9-11789-ge0fd9af Please recheck with kvm.git, next branch. Thanks, Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux -- 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: [nVMX with: v3.9-11789-ge0fd9af] Stack trace when L2 guest is rebooted.
On 2013-05-10 17:12, Jan Kiszka wrote: On 2013-05-10 15:00, Kashyap Chamarthy wrote: Heya, This is on Intel Haswell. First, some version info: L0, L1 -- both of them have same versions of kernel, qemu: = $ rpm -q kernel --changelog | head -2 * Thu May 09 2013 Josh Boyer - 3.10.0-0.rc0.git23.1 - Linux v3.9-11789-ge0fd9af Please recheck with kvm.git, next branch. Hmm, looks like your branch already contains the patch I was thinking of (03b28f8). You could try if leaving shadow VMCS off makes a difference, but I bet that is unrelated. You get that backtrace in L1, correct? I'll have to see if I can reproduce it. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux -- 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: [nVMX with: v3.9-11789-ge0fd9af] Stack trace when L2 guest is rebooted.
On Fri, May 10, 2013 at 8:54 PM, Jan Kiszka jan.kis...@siemens.com wrote: On 2013-05-10 17:12, Jan Kiszka wrote: On 2013-05-10 15:00, Kashyap Chamarthy wrote: Heya, This is on Intel Haswell. First, some version info: L0, L1 -- both of them have same versions of kernel, qemu: = $ rpm -q kernel --changelog | head -2 * Thu May 09 2013 Josh Boyer - 3.10.0-0.rc0.git23.1 - Linux v3.9-11789-ge0fd9af Please recheck with kvm.git, next branch. Hmm, looks like your branch already contains the patch I was thinking of (03b28f8). Yes. You could try if leaving shadow VMCS off makes a difference, but I bet that is unrelated. Right. I could try. But, like you said, does it *really* make a difference. You get that backtrace in L1, correct? Yes. If you have any further tracing pointers, I could do some debugging. I'll have to see if I can reproduce it. Thanks. If you're looking for a clear reproducer, this is how I conducted my tests, and here's where I'm capturing all of the related work: [1] Setup -- https://raw.github.com/kashyapc/nvmx-haswell/master/SETUP-nVMX.rst [2] Simple scripts used to create L1 and L2 -- https://github.com/kashyapc/nvmx-haswell/tree/master/tests/scripts [3] Libvirt XMLs I used (for reference) -- https://github.com/kashyapc/nvmx-haswell/tree/master/tests/libvirt-xmls-for-l1-l2 Thanks in advance. /kashyap -- 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: [nVMX with: v3.9-11789-ge0fd9af] Stack trace when L2 guest is rebooted.
[3] Libvirt XMLs I used (for reference) -- https://github.com/kashyapc/nvmx-haswell/tree/master/tests/libvirt-xmls-for-l1-l2 Oops, forgot to add, here we go -- https://github.com/kashyapc/nvmx-haswell/tree/master/tests/libvirt-xmls-for-l1-l2 /kashyap -- 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: [nVMX with: v3.9-11789-ge0fd9af] Stack trace when L2 guest is rebooted.
On 2013-05-10 17:39, Kashyap Chamarthy wrote: On Fri, May 10, 2013 at 8:54 PM, Jan Kiszka jan.kis...@siemens.com wrote: On 2013-05-10 17:12, Jan Kiszka wrote: On 2013-05-10 15:00, Kashyap Chamarthy wrote: Heya, This is on Intel Haswell. First, some version info: L0, L1 -- both of them have same versions of kernel, qemu: = $ rpm -q kernel --changelog | head -2 * Thu May 09 2013 Josh Boyer - 3.10.0-0.rc0.git23.1 - Linux v3.9-11789-ge0fd9af Please recheck with kvm.git, next branch. Hmm, looks like your branch already contains the patch I was thinking of (03b28f8). Yes. You could try if leaving shadow VMCS off makes a difference, but I bet that is unrelated. Right. I could try. But, like you said, does it *really* make a difference. We know after you tried. I don't have access to a Haswell box, so we better exclude this beforehand. You get that backtrace in L1, correct? Yes. If you have any further tracing pointers, I could do some debugging. Thanks, I may come back to you if reproduction fails here. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux -- 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
Has anyone used tc to police traffic for VMs?
Hello: I posted this question to the LARTC mailing list and apparently nobody on that list has used tc to control traffic to VMs. I am hoping someone on this list has used it before and can help me. I have a CentOS 6 server running two virtual machines using KVM. Each machine has its own virtual device since I am using bridged networking. I am trying to allocate bandwidth to each virtual machine. I have these rules set up: /sbin/tc qdisc add dev v1262 ingress handle : /sbin/tc filter add dev v1262 parent : protocol ip prio 50 u32 match ip src 0.0.0.0/0 police rate 5mbit burst 500k mtu 3100 drop flowid :1 /sbin/tc qdisc add dev v1263 ingress handle : /sbin/tc filter add dev v1263 parent : protocol ip prio 50 u32 match ip src 0.0.0.0/0 police rate 5mbit burst 500k mtu 3100 drop flowid :1 You can see them when I do a qdisc ls: qdisc ingress : dev v1262 parent :fff1 qdisc ingress : dev v1263 parent :fff1 The problem is when v1262 is doing a large upload, v1263 gets starved. If I remove the ingress qdisc, v1263 runs at full speed regardless of what v1262 is doing. It's as if the two devices are using the same queue. I did not think that would happen since they are totally separate devices. Has anyone been able to limit traffic separately to individual VMs? Thanks, Neil -- Neil Aggarwal, (972)834-1565, http://UnmeteredVPS.net/centos Virtual private server with CentOS 6 preinstalled Unmetered bandwidth = no overage charges -- 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/ppc/booke64: fix build breakage from Altivec, and disable e6500
Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. -Mike -- 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/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) 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: PPC: Add userspace debug stub support
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 3:48 PM To: Bhushan Bharat-R65777 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; tiejun.c...@windriver.com; Bhushan Bharat-R65777 Subject: Re: [PATCH] KVM: PPC: Add userspace debug stub support On 07.05.2013, at 11:40, Bharat Bhushan wrote: This patch adds the debug stub support on booke/bookehv. Now QEMU debug stub can use hw breakpoint, watchpoint and software breakpoint to debug guest. This is how we save/restore debug register context when switching between guest, userspace and kernel user-process: When QEMU is running - thread-debug_reg == QEMU debug register context. - Kernel will handle switching the debug register on context switch. - no vcpu_load() called QEMU makes ioctls (except RUN) - This will call vcpu_load() - should not change context. - Some ioctls can change vcpu debug register, context saved in - vcpu-debug_regs QEMU Makes RUN ioctl - Save thread-debug_reg on STACK - Store thread-debug_reg == vcpu-debug_reg load thread-debug_reg - RUN VCPU ( So thread points to vcpu context ) Context switch happens When VCPU running - makes vcpu_load() should not load any context kernel loads the vcpu - context as thread-debug_regs points to vcpu context. On heavyweight_exit - Load the context saved on stack in thread-debug_reg Currently we do not support debug resource emulation to guest, On debug exception, always exit to user space irrespective of user space is expecting the debug exception or not. If this is unexpected exception (breakpoint/watchpoint event not set by userspace) then let us leave the action on user space. This is similar to what it was before, only thing is that now we have proper exit state available to user space. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/include/asm/kvm_host.h |3 + arch/powerpc/include/uapi/asm/kvm.h |1 + arch/powerpc/kvm/booke.c| 242 --- arch/powerpc/kvm/booke.h|5 + 4 files changed, 233 insertions(+), 18 deletions(-) diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index 838a577..1b29945 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -524,7 +524,10 @@ struct kvm_vcpu_arch { u32 eptcfg; u32 epr; u32 crit_save; + /* guest debug registers*/ struct debug_reg dbg_reg; + /* shadow debug registers */ Please be more verbose here. What exactly does this contain? Why do we need shadow and non-shadow registers? The comment as it is reads like /* Add one plus one */ x = 1 + 1; /* * Shadow debug registers hold the debug register content * to be written in h/w debug register on behalf of guest * written value or user space written value. */ + struct debug_reg shadow_dbg_reg; #endif gpa_t paddr_accessed; gva_t vaddr_accessed; diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h index ded0607..f5077c2 100644 --- a/arch/powerpc/include/uapi/asm/kvm.h +++ b/arch/powerpc/include/uapi/asm/kvm.h @@ -27,6 +27,7 @@ #define __KVM_HAVE_PPC_SMT #define __KVM_HAVE_IRQCHIP #define __KVM_HAVE_IRQ_LINE +#define __KVM_HAVE_GUEST_DEBUG struct kvm_regs { __u64 pc; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index ef99536..6a44ad4 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -133,6 +133,29 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu) #endif } +static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) { + /* Synchronize guest's desire to get debug interrupts into shadow +MSR */ #ifndef CONFIG_KVM_BOOKE_HV + vcpu-arch.shadow_msr = ~MSR_DE; + vcpu-arch.shadow_msr |= vcpu-arch.shared-msr MSR_DE; #endif + + /* Force enable debug interrupts when user space wants to debug */ + if (vcpu-guest_debug) { +#ifdef CONFIG_KVM_BOOKE_HV + /* +* Since there is no shadow MSR, sync MSR_DE into the guest +* visible MSR. +*/ + vcpu-arch.shared-msr |= MSR_DE; +#else + vcpu-arch.shadow_msr |= MSR_DE; + vcpu-arch.shared-msr = ~MSR_DE; +#endif + } +} + /* * Helper function for full MSR writes. No need to call this if only * EE/CE/ME/DE/RI are changing. @@ -150,6 +173,7 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr) kvmppc_mmu_msr_notify(vcpu, old_msr); kvmppc_vcpu_sync_spe(vcpu); kvmppc_vcpu_sync_fpu(vcpu); + kvmppc_vcpu_sync_debug(vcpu); } static void kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu, @@ -655,6 +679,7 @@ int kvmppc_core_check_requests(struct kvm_vcpu *vcpu) int kvmppc_vcpu_run(struct
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 05/10/2013 04:40:01 AM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. Exceptions don't get handled all that often, and ideally we catch it when it's added rather than after-the-fact. diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index e8ed7d6..6397613 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -309,6 +309,10 @@ kvm_handler BOOKE_INTERRUPT_DOORBELL_CRITICAL, EX_PARAMS(CRIT), \ SPRN_CSRR0, SPRN_CSRR1, 0 kvm_handler BOOKE_INTERRUPT_HV_PRIV, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, NEED_EMU +kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 +kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 Why not NEED_ESR as we did in our SDK? This is just to fix the build break -- we don't need ESR yet. Though I did look at the ESR documentation and didn't see where Altivec exceptions used it (I do see a couple places now). -Scott -- 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: [nVMX with: v3.9-11789-ge0fd9af] Stack trace when L2 guest is rebooted.
On Fri, May 10, 2013 at 9:33 AM, Jan Kiszka jan.kis...@siemens.com wrote: On 2013-05-10 17:39, Kashyap Chamarthy wrote: On Fri, May 10, 2013 at 8:54 PM, Jan Kiszka jan.kis...@siemens.com wrote: On 2013-05-10 17:12, Jan Kiszka wrote: On 2013-05-10 15:00, Kashyap Chamarthy wrote: Heya, This is on Intel Haswell. First, some version info: L0, L1 -- both of them have same versions of kernel, qemu: I tried to reproduce such a problem, and I found L2 (Linux) hangs in SeaBIOS, after line iPXE (http://ipxe.org) It happens with or w/o VMCS shadowing (and even without my virtual EPT patches). I didn't realize this problem until I updated the L1 kernel to the latest (e.g. 3.9.0) from 3.7.0. L0 uses the kvm.git, next branch. It's possible that the L1 kernel exposed a bug with the nested virtualization, as we saw such cases before. -- Jun Intel Open Source Technology Center -- 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/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 05/10/2013 09:11:24 AM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 4:16 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. How hard would it be to add? I suppose it's broken in 3.10, so we need something quick before it gets released? Not so hard. Yes. I was surprised by this patch given the fact that we have planned to send altivec support upstream this days and that we already have a similar patch from Tiejun on our list. I didn't see Tiejun's patch... My goal was just to fix the build break without exposing problems, and to encourage a patch to fix it properly to happen sooner rather than later. With Tiejun's patch, which is similar to mine except that it doesn't disable e6500 support, a user could BUG() the kernel by forcing an Altivec exception in a guest. I didn't want to go further down the road of adding reflectors for those exceptions, which could make it look like the problem was dealt with even though it's still not done. -Scott -- 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: PPC: Add userspace debug stub support
On 10.05.2013, at 19:31, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 3:48 PM To: Bhushan Bharat-R65777 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; tiejun.c...@windriver.com; Bhushan Bharat-R65777 Subject: Re: [PATCH] KVM: PPC: Add userspace debug stub support On 07.05.2013, at 11:40, Bharat Bhushan wrote: This patch adds the debug stub support on booke/bookehv. Now QEMU debug stub can use hw breakpoint, watchpoint and software breakpoint to debug guest. This is how we save/restore debug register context when switching between guest, userspace and kernel user-process: When QEMU is running - thread-debug_reg == QEMU debug register context. - Kernel will handle switching the debug register on context switch. - no vcpu_load() called QEMU makes ioctls (except RUN) - This will call vcpu_load() - should not change context. - Some ioctls can change vcpu debug register, context saved in - vcpu-debug_regs QEMU Makes RUN ioctl - Save thread-debug_reg on STACK - Store thread-debug_reg == vcpu-debug_reg load thread-debug_reg - RUN VCPU ( So thread points to vcpu context ) Context switch happens When VCPU running - makes vcpu_load() should not load any context kernel loads the vcpu - context as thread-debug_regs points to vcpu context. On heavyweight_exit - Load the context saved on stack in thread-debug_reg Currently we do not support debug resource emulation to guest, On debug exception, always exit to user space irrespective of user space is expecting the debug exception or not. If this is unexpected exception (breakpoint/watchpoint event not set by userspace) then let us leave the action on user space. This is similar to what it was before, only thing is that now we have proper exit state available to user space. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/include/asm/kvm_host.h |3 + arch/powerpc/include/uapi/asm/kvm.h |1 + arch/powerpc/kvm/booke.c| 242 --- arch/powerpc/kvm/booke.h|5 + 4 files changed, 233 insertions(+), 18 deletions(-) diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index 838a577..1b29945 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -524,7 +524,10 @@ struct kvm_vcpu_arch { u32 eptcfg; u32 epr; u32 crit_save; + /* guest debug registers*/ struct debug_reg dbg_reg; + /* shadow debug registers */ Please be more verbose here. What exactly does this contain? Why do we need shadow and non-shadow registers? The comment as it is reads like /* Add one plus one */ x = 1 + 1; /* * Shadow debug registers hold the debug register content * to be written in h/w debug register on behalf of guest * written value or user space written value. */ /* hardware visible debug registers when in guest state */ + struct debug_reg shadow_dbg_reg; #endif gpa_t paddr_accessed; gva_t vaddr_accessed; diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h index ded0607..f5077c2 100644 --- a/arch/powerpc/include/uapi/asm/kvm.h +++ b/arch/powerpc/include/uapi/asm/kvm.h @@ -27,6 +27,7 @@ #define __KVM_HAVE_PPC_SMT #define __KVM_HAVE_IRQCHIP #define __KVM_HAVE_IRQ_LINE +#define __KVM_HAVE_GUEST_DEBUG struct kvm_regs { __u64 pc; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index ef99536..6a44ad4 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -133,6 +133,29 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu) #endif } +static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) { + /* Synchronize guest's desire to get debug interrupts into shadow +MSR */ #ifndef CONFIG_KVM_BOOKE_HV + vcpu-arch.shadow_msr = ~MSR_DE; + vcpu-arch.shadow_msr |= vcpu-arch.shared-msr MSR_DE; #endif + + /* Force enable debug interrupts when user space wants to debug */ + if (vcpu-guest_debug) { +#ifdef CONFIG_KVM_BOOKE_HV + /* +* Since there is no shadow MSR, sync MSR_DE into the guest +* visible MSR. +*/ + vcpu-arch.shared-msr |= MSR_DE; +#else + vcpu-arch.shadow_msr |= MSR_DE; + vcpu-arch.shared-msr = ~MSR_DE; +#endif + } +} + /* * Helper function for full MSR writes. No need to call this if only * EE/CE/ME/DE/RI are changing. @@ -150,6 +173,7 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr) kvmppc_mmu_msr_notify(vcpu, old_msr); kvmppc_vcpu_sync_spe(vcpu); kvmppc_vcpu_sync_fpu(vcpu); + kvmppc_vcpu_sync_debug(vcpu); } static void kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu, @@ -655,6 +679,7 @@ int kvmppc_core_check_requests(struct kvm_vcpu *vcpu) int
RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
-Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Friday, May 10, 2013 7:51 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) I can make sure that it doesn't break 32-bit. Altivec is not present in e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host. Are you looking for G4 support? -Mike -- 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/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 05/10/2013 01:03:27 PM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Friday, May 10, 2013 7:51 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) I can make sure that it doesn't break 32-bit. Altivec is not present in e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host. Are you looking for G4 support? Just because Altivec isn't present on 32-bit doesn't mean that a particular patch can't break things there. Just as a patch to deal with lazy ee issues can break 32-bit, even though 32-bit doesn't have lazy ee. :-) -Scott -- 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: [nVMX with: v3.9-11789-ge0fd9af] Stack trace when L2 guest is rebooted.
On 2013-05-10 19:40, Nakajima, Jun wrote: On Fri, May 10, 2013 at 9:33 AM, Jan Kiszka jan.kis...@siemens.com wrote: On 2013-05-10 17:39, Kashyap Chamarthy wrote: On Fri, May 10, 2013 at 8:54 PM, Jan Kiszka jan.kis...@siemens.com wrote: On 2013-05-10 17:12, Jan Kiszka wrote: On 2013-05-10 15:00, Kashyap Chamarthy wrote: Heya, This is on Intel Haswell. First, some version info: L0, L1 -- both of them have same versions of kernel, qemu: I tried to reproduce such a problem, and I found L2 (Linux) hangs in SeaBIOS, after line iPXE (http://ipxe.org) It happens with or w/o VMCS shadowing (and even without my virtual EPT patches). I didn't realize this problem until I updated the L1 kernel to the latest (e.g. 3.9.0) from 3.7.0. L0 uses the kvm.git, next branch. It's possible that the L1 kernel exposed a bug with the nested virtualization, as we saw such cases before. Hmm, no such issues here ATM although I'm on 3.9 for L1 as well. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux -- 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: [v2][KVM][PATCH 1/1] kvm:ppc: enable doorbell exception with CONFIG_PPC_DOORBELL
On 07.05.2013, at 12:23, Tiejun Chen wrote: CONFIG_PPC_DOORBELL is enough to cover all variants. Signed-off-by: Tiejun Chen tiejun.c...@windriver.com --- arch/powerpc/kvm/booke.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 1020119..62d4ece 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -795,7 +795,7 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu, kvmppc_fill_pt_regs(regs); timer_interrupt(regs); break; -#if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_BOOK3E_64) +#if defined(CONFIG_PPC_DOORBELL) The same question still holds. How is this an improvement over the previous code? Does this fix any issues for you? Is this just a coding style cleanup? Alex case BOOKE_INTERRUPT_DOORBELL: kvmppc_fill_pt_regs(regs); doorbell_exception(regs); -- 1.7.9.5 -- 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: [v2][KVM][PATCH 1/1] kvm:ppc: enable doorbell exception with CONFIG_PPC_DOORBELL
On 05/10/2013 01:14:27 PM, Alexander Graf wrote: On 07.05.2013, at 12:23, Tiejun Chen wrote: CONFIG_PPC_DOORBELL is enough to cover all variants. Signed-off-by: Tiejun Chen tiejun.c...@windriver.com --- arch/powerpc/kvm/booke.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 1020119..62d4ece 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -795,7 +795,7 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu, kvmppc_fill_pt_regs(regs); timer_interrupt(regs); break; -#if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_BOOK3E_64) +#if defined(CONFIG_PPC_DOORBELL) The same question still holds. How is this an improvement over the previous code? Does this fix any issues for you? Is this just a coding style cleanup? This is an improvement because CONFIG_PPC_DOORBELL is what controls whether the function that is called inside the ifdef exists. -Scott -- 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: [v2][KVM][PATCH 1/1] kvm:ppc: enable doorbell exception with CONFIG_PPC_DOORBELL
On 10.05.2013, at 20:17, Scott Wood wrote: On 05/10/2013 01:14:27 PM, Alexander Graf wrote: On 07.05.2013, at 12:23, Tiejun Chen wrote: CONFIG_PPC_DOORBELL is enough to cover all variants. Signed-off-by: Tiejun Chen tiejun.c...@windriver.com --- arch/powerpc/kvm/booke.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 1020119..62d4ece 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -795,7 +795,7 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu, kvmppc_fill_pt_regs(regs); timer_interrupt(regs); break; -#if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_BOOK3E_64) +#if defined(CONFIG_PPC_DOORBELL) The same question still holds. How is this an improvement over the previous code? Does this fix any issues for you? Is this just a coding style cleanup? This is an improvement because CONFIG_PPC_DOORBELL is what controls whether the function that is called inside the ifdef exists. Aha! Now that's a good reason. Tiejun, please adjust your patch description accordingly. 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/ppc/booke64: fix build breakage from Altivec, and disable e6500
-Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 8:42 PM To: Caraman Mihai Claudiu-B02008 Cc: Alexander Graf; Wood Scott-B07421; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 05/10/2013 09:11:24 AM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 4:16 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. How hard would it be to add? I suppose it's broken in 3.10, so we need something quick before it gets released? Not so hard. Yes. I was surprised by this patch given the fact that we have planned to send altivec support upstream this days and that we already have a similar patch from Tiejun on our list. I didn't see Tiejun's patch... My goal was just to fix the build break without exposing problems, and to encourage a patch to fix it properly to happen sooner rather than later. With Tiejun's patch, which is similar to mine except that it doesn't disable e6500 support, a user could BUG() the kernel by forcing an Altivec exception in a guest. I didn't want to go further down the road of adding reflectors for those exceptions, which could make it look like the problem was dealt with even though it's still not done. I agree it's quite annoying to hit a build breakage. Reflection is not a proper solution for this problem (though we will require it later) but program exception injection looks feasible as a simple fix. I wouldn't want to see e6500 removed for this reason. Thanks, Mike -- 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/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 10.05.2013, at 20:06, Scott Wood wrote: On 05/10/2013 01:03:27 PM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Friday, May 10, 2013 7:51 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) I can make sure that it doesn't break 32-bit. Altivec is not present in e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host. Are you looking for G4 support? Just because Altivec isn't present on 32-bit doesn't mean that a particular patch can't break things there. Just as a patch to deal with lazy ee issues can break 32-bit, even though 32-bit doesn't have lazy ee. :-) Yeah, that's what I was referring to :). G4s use a different code path altogether and already work with Altivec for years. I doubt you'll manage to break anything there :). So what you should ensure is 1) 32bit e500 still compiles 2) 32bit e500 still executes fine, albeit without altivec 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/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 05/10/2013 01:20:33 PM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 8:42 PM To: Caraman Mihai Claudiu-B02008 Cc: Alexander Graf; Wood Scott-B07421; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 05/10/2013 09:11:24 AM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 4:16 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. How hard would it be to add? I suppose it's broken in 3.10, so we need something quick before it gets released? Not so hard. Yes. I was surprised by this patch given the fact that we have planned to send altivec support upstream this days and that we already have a similar patch from Tiejun on our list. I didn't see Tiejun's patch... My goal was just to fix the build break without exposing problems, and to encourage a patch to fix it properly to happen sooner rather than later. With Tiejun's patch, which is similar to mine except that it doesn't disable e6500 support, a user could BUG() the kernel by forcing an Altivec exception in a guest. I didn't want to go further down the road of adding reflectors for those exceptions, which could make it look like the problem was dealt with even though it's still not done. I agree it's quite annoying to hit a build breakage. Reflection is not a proper solution for this problem (though we will require it later) but program exception injection looks feasible as a simple fix. Program exception injection still doesn't deal with state corruption. I wouldn't want to see e6500 removed for this reason. It's not being removed, just closed while under construction to prevent damage to people passing through. :-) -Scott -- 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/ppc/booke64: fix build breakage from Altivec, and disable e6500
-Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 9:06 PM To: Caraman Mihai Claudiu-B02008 Cc: Alexander Graf; Wood Scott-B07421; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 05/10/2013 01:03:27 PM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Friday, May 10, 2013 7:51 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) I can make sure that it doesn't break 32-bit. Altivec is not present in e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host. Are you looking for G4 support? Just because Altivec isn't present on 32-bit doesn't mean that a particular patch can't break things there. Just as a patch to deal with lazy ee issues can break 32-bit, even though 32-bit doesn't have lazy ee. :-) That's exactly what I have said. That patch was intended as a RFC I forgot to mark it as such under time constraints, however it reached its goal. -Mike -- 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/ppc/booke64: fix build breakage from Altivec, and disable e6500
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 9:23 PM To: Wood Scott-B07421 Cc: Caraman Mihai Claudiu-B02008; Wood Scott-B07421; kvm- p...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 20:06, Scott Wood wrote: On 05/10/2013 01:03:27 PM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Friday, May 10, 2013 7:51 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) I can make sure that it doesn't break 32-bit. Altivec is not present in e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host. Are you looking for G4 support? Just because Altivec isn't present on 32-bit doesn't mean that a particular patch can't break things there. Just as a patch to deal with lazy ee issues can break 32-bit, even though 32-bit doesn't have lazy ee. :-) Yeah, that's what I was referring to :). G4s use a different code path altogether and already work with Altivec for years. Huh ... I forgot that G4 in handled under Book3S, not so obvious. I doubt you'll manage to break anything there :). If you have stated breaks instead of works would have been more clear where you were heading :-J -Mike -- 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/ppc/booke64: fix build breakage from Altivec, and disable e6500
Am 10.05.2013 um 20:51 schrieb Caraman Mihai Claudiu-B02008 b02...@freescale.com: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 9:23 PM To: Wood Scott-B07421 Cc: Caraman Mihai Claudiu-B02008; Wood Scott-B07421; kvm- p...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 20:06, Scott Wood wrote: On 05/10/2013 01:03:27 PM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Friday, May 10, 2013 7:51 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-...@vger.kernel.org; kvm@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) I can make sure that it doesn't break 32-bit. Altivec is not present in e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host. Are you looking for G4 support? Just because Altivec isn't present on 32-bit doesn't mean that a particular patch can't break things there. Just as a patch to deal with lazy ee issues can break 32-bit, even though 32-bit doesn't have lazy ee. :-) Yeah, that's what I was referring to :). G4s use a different code path altogether and already work with Altivec for years. Huh ... I forgot that G4 in handled under Book3S, not so obvious. I doubt you'll manage to break anything there :). If you have stated breaks instead of works would have been more clear where you were heading :-J Ok, next time I'll make it more obvious :). Alex -Mike -- 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 v19] pvpanic: pvpanic device driver
On Wed, May 08, 2013 at 11:15:32AM +0800, Hu Tao wrote: pvpanic device is a qemu simulated device through which guest panic event is sent to host. Signed-off-by: Hu Tao hu...@cn.fujitsu.com --- v18 - v19: 1. return -ENODEV early if port is 0 v17 - v18: 1. call acpi_walk_resources to get the port, and usb outb instead of acpi_evaluate_oject in panic notifier callback 2. reword help message drivers/platform/x86/Kconfig | 8 +++ drivers/platform/x86/Makefile | 2 + drivers/platform/x86/pvpanic.c | 124 + 3 files changed, 134 insertions(+) create mode 100644 drivers/platform/x86/pvpanic.c diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 3338437..8577261 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -781,4 +781,12 @@ config APPLE_GMUX graphics as well as the backlight. Currently only backlight control is supported by the driver. +config PVPANIC + tristate pvpanic device support + depends on ACPI + ---help--- + This driver provides support for the pvpanic device. pvpanic is + a paravirtualized device provided by QEMU; it lets a virtual machine + (guest) communicate panic events to the host. + endif # X86_PLATFORM_DEVICES diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index ace2b38..ef0ec74 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -51,3 +51,5 @@ obj-$(CONFIG_INTEL_OAKTRAIL)+= intel_oaktrail.o obj-$(CONFIG_SAMSUNG_Q10)+= samsung-q10.o obj-$(CONFIG_APPLE_GMUX) += apple-gmux.o obj-$(CONFIG_CHROMEOS_LAPTOP)+= chromeos_laptop.o + +obj-$(CONFIG_PVPANIC) += pvpanic.o diff --git a/drivers/platform/x86/pvpanic.c b/drivers/platform/x86/pvpanic.c new file mode 100644 index 000..47ae0c4 --- /dev/null +++ b/drivers/platform/x86/pvpanic.c @@ -0,0 +1,124 @@ +/* + * pvpanic.c - pvpanic Device Support + * + * Copyright (C) 2013 Fujitsu. + * + * 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. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#define pr_fmt(fmt) KBUILD_MODNAME : fmt + +#include linux/kernel.h +#include linux/module.h +#include linux/init.h +#include linux/types.h +#include acpi/acpi_bus.h +#include acpi/acpi_drivers.h + +MODULE_AUTHOR(Hu Tao hu...@cn.fujitsu.com); +MODULE_DESCRIPTION(pvpanic device driver); +MODULE_LICENSE(GPL); + +static int pvpanic_add(struct acpi_device *device); +static int pvpanic_remove(struct acpi_device *device); + +static const struct acpi_device_id pvpanic_device_ids[] = { + { QEMU0001, 0 }, + { , 0 }, +}; +MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids); + +#define PVPANIC_PANICKED (1 0) + +static u16 port; + +static struct acpi_driver pvpanic_driver = { + .name = pvpanic, + .class =QEMU, + .ids = pvpanic_device_ids, + .ops = { + .add = pvpanic_add, + .remove = pvpanic_remove, + }, + .owner =THIS_MODULE, +}; + +static void +pvpanic_send_event(unsigned int event) +{ + outb(event, port); +} + +static int +pvpanic_panic_notify(struct notifier_block *nb, unsigned long code, + void *unused) +{ + pvpanic_send_event(PVPANIC_PANICKED); + return NOTIFY_DONE; +} + +static struct notifier_block pvpanic_panic_nb = { + .notifier_call = pvpanic_panic_notify, +}; + + +static acpi_status +pvpanic_walk_resources(struct acpi_resource *res, void *context) +{ + switch (res-type) { + case ACPI_RESOURCE_TYPE_END_TAG: + return AE_OK; + + case ACPI_RESOURCE_TYPE_IO: + port = res-data.io.minimum; + return AE_OK; + + default: + return AE_ERROR; + } +} + +static int pvpanic_add(struct acpi_device *device) +{ + acpi_status status; + u64 ret; + + status = acpi_evaluate_integer(device-handle, _STA, NULL, +ret); + + if (ACPI_FAILURE(status) || (ret 0x0B) != 0x0B) + return -ENODEV; + +
RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
I didn't see Tiejun's patch... My goal was just to fix the build break without exposing problems, and to encourage a patch to fix it properly to happen sooner rather than later. With Tiejun's patch, which is similar to mine except that it doesn't disable e6500 support, a user could BUG() the kernel by forcing an Altivec exception in a guest. I didn't want to go further down the road of adding reflectors for those exceptions, which could make it look like the problem was dealt with even though it's still not done. I agree it's quite annoying to hit a build breakage. Reflection is not a proper solution for this problem (though we will require it later) but program exception injection looks feasible as a simple fix. Program exception injection still doesn't deal with state corruption. Yes but it's not critical for this particular case since nobody is able to effectively use that state via altivec instructions. Leaking state however can be a real issue. -Mike -- 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/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 05/10/2013 02:06:53 PM, Caraman Mihai Claudiu-B02008 wrote: I didn't see Tiejun's patch... My goal was just to fix the build break without exposing problems, and to encourage a patch to fix it properly to happen sooner rather than later. With Tiejun's patch, which is similar to mine except that it doesn't disable e6500 support, a user could BUG() the kernel by forcing an Altivec exception in a guest. I didn't want to go further down the road of adding reflectors for those exceptions, which could make it look like the problem was dealt with even though it's still not done. I agree it's quite annoying to hit a build breakage. Reflection is not a proper solution for this problem (though we will require it later) but program exception injection looks feasible as a simple fix. Program exception injection still doesn't deal with state corruption. Yes but it's not critical for this particular case since nobody is able to effectively use that state via altivec instructions. Leaking state however can be a real issue. Depending on guest behavior it could look like things are working even though they aren't (e.g. a guest just enables MSR[VEC] and uses altivec instructions, not relying on exceptions). This really isn't worth spending a lot of time debating... Once Altivec is fixed properly (you said that'd be soon, right?), we can add e6500 back to the list. -Scott -- 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: [v1][KVM][PATCH 1/1] kvm:ppc:booehv: direct ISI exception to Guest
Am 10.05.2013 um 21:22 schrieb Scott Wood scottw...@freescale.com: On 05/10/2013 12:57:33 PM, Alexander Graf wrote: Could you guys please collect performance data during the next weeks on both guest-directed ISIs as well as VF MMIOs (preferably with in-kernel MMIO), so that we can decide on the direction that's worth going towards? Collecting data on VF MMIO would require implementing it (or at least salvaging and fixing some old code), which is not a high priority at the moment. If we do implement VF in the future we could always undo the direct ISI change, but it would still be nice to know if there's any real benefit in the first place. Mike sounded like he had an almost working poc, which is good enough to collect rough numbers. And yes, changes like these should always get at least basic performance numbers along with them, regardless of drawbacks. Alex FWIW, I doubt that the more stress on HW TLB will be significant. -Scott -- 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: [nVMX with: v3.9-11789-ge0fd9af] Stack trace when L2 guest is rebooted.
We know after you tried. I don't have access to a Haswell box, so we better exclude this beforehand. Fair enough. I'll try that too, and let you know. You get that backtrace in L1, correct? Yes. If you have any further tracing pointers, I could do some debugging. Thanks, I may come back to you if reproduction fails here. -- 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: [nVMX with: v3.9-11789-ge0fd9af] Stack trace when L2 guest is rebooted.
On Fri, May 10, 2013 at 11:39 PM, Jan Kiszka jan.kis...@siemens.com wrote: On 2013-05-10 19:40, Nakajima, Jun wrote: On Fri, May 10, 2013 at 9:33 AM, Jan Kiszka jan.kis...@siemens.com wrote: On 2013-05-10 17:39, Kashyap Chamarthy wrote: On Fri, May 10, 2013 at 8:54 PM, Jan Kiszka jan.kis...@siemens.com wrote: On 2013-05-10 17:12, Jan Kiszka wrote: On 2013-05-10 15:00, Kashyap Chamarthy wrote: Heya, This is on Intel Haswell. First, some version info: L0, L1 -- both of them have same versions of kernel, qemu: I tried to reproduce such a problem, and I found L2 (Linux) hangs in SeaBIOS, after line iPXE (http://ipxe.org) It happens with or w/o VMCS shadowing (and even without my virtual EPT patches). I didn't realize this problem until I updated the L1 kernel to the latest (e.g. 3.9.0) from 3.7.0. L0 uses the kvm.git, next branch. It's possible that the L1 kernel exposed a bug with the nested virtualization, as we saw such cases before. Hmm, no such issues here ATM although I'm on 3.9 for L1 as well. Interesting. But Jan, you're not using an Haswell machine, right ? Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux -- 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: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On Fri, 2013-05-10 at 22:12 +0800, Kevin Hao wrote: So I would assume you will not pick up these two patches, right? http://patchwork.ozlabs.org/patch/235530/ http://patchwork.ozlabs.org/patch/235532/ Anyway it is more easier to enable the external proxy by using this method. But if you insist, I can respin a patch to use the method you suggested since it will definitely reduce the window where the irq is hard disabled. I would keep the EE_EDGE bit definition. I have no objection to a gradual approach however for the other one where we apply it as is now to enable coreint while you do a rework to make it better :-) Cheers, Ben. -- 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: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On Sat, 2013-05-11 at 07:49 +1000, Benjamin Herrenschmidt wrote: I would keep the EE_EDGE bit definition. I have no objection to a gradual approach however for the other one where we apply it as is now to enable coreint while you do a rework to make it better :-) Note also that I generally don't apply FSL related patches directly, I rely on Kumar Gala picking them up so he's the one ultimately making that choice. Cheers, Ben. -- 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 v3 24/32] arm64: KVM: 32bit GP register access
On Tue, May 07, 2013 at 05:33:03PM +0100, Catalin Marinas wrote: On Tue, May 07, 2013 at 05:28:00PM +0100, Marc Zyngier wrote: On 02/05/13 17:09, Catalin Marinas wrote: BTW, on arch/arm it looks like this is used when you get a data abort with PC as the destination register and you inject a prefetch abort in this case. Why isn't this a normal data abort? Once you get the information, you load it into PC but first you need to sort out the data abort (unless I don't understand how the kvm_inject_pabt works). Indeed, it should be a data abort, as we correctly fetched the instruction. Now, I wonder why we even bother trying to catch this case. Fetching PC from MMIO looks quite silly, but I don't think anything really forbids it in the architecture. It's not forbidden and you should just treat it as any other data abort, no need to check whether the register is PC. If you do the PC adjustment further down in that function it will be overridden by the instruction emulation anyway. There is no optimisation in checking for PC since such fault is very unlikely in sane code anyway. The reason is simply that it is most likely because of a bug that this happens, and we would rather catch such an error in a meaningful way than let this go crazy and have users track it down for a long time. Especially, this was relevant when we did io instruction decoding and we wanted to catch potential bugs in that when it was development code. That all being said, we can remove the check. I don't think, however, that it being an unlikely thing is a good argument: if we remove the check we need to make sure that the VM does whatever the architecture dictates, which I assume is to not skip the MMIO instruction and support setting the PC to the value returned from an I/O emulation device in QEMU. I think the scenario sounds crazy and is more than anything a sign of a bug somewhere, and whether it should be a PABT or a DABT is a judgement call, but I chose a PABT because the thing that's weird is jumping to an I/O address, it's not that the memory address for the load is problematic. -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 v3 30/32] arm64: KVM: enable initialization of a 32bit vcpu
On Tue, May 07, 2013 at 05:36:52PM +0100, Marc Zyngier wrote: On 24/04/13 18:17, Christoffer Dall wrote: On Wed, Apr 24, 2013 at 6:49 AM, Marc Zyngier marc.zyng...@arm.com wrote: On 24/04/13 00:02, Christoffer Dall wrote: On Mon, Apr 08, 2013 at 05:17:32PM +0100, Marc Zyngier wrote: Wire the init of a 32bit vcpu by allowing 32bit modes in pstate, and providing sensible defaults out of reset state. This feature is of course conditioned by the presence of 32bit capability on the physical CPU, and is checked by the KVM_CAP_ARM_EL1_32BIT capability. Signed-off-by: Marc Zyngier marc.zyng...@arm.com --- arch/arm64/include/asm/kvm_host.h | 2 +- arch/arm64/include/uapi/asm/kvm.h | 1 + arch/arm64/kvm/guest.c| 6 ++ arch/arm64/kvm/reset.c| 25 - include/uapi/linux/kvm.h | 1 + 5 files changed, 33 insertions(+), 2 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index d44064d..c3ec107 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -34,7 +34,7 @@ #include asm/kvm_vgic.h #include asm/kvm_arch_timer.h -#define KVM_VCPU_MAX_FEATURES 1 +#define KVM_VCPU_MAX_FEATURES 2 /* We don't currently support large pages. */ #define KVM_HPAGE_GFN_SHIFT(x) 0 diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h index 5b1110c..5031f42 100644 --- a/arch/arm64/include/uapi/asm/kvm.h +++ b/arch/arm64/include/uapi/asm/kvm.h @@ -75,6 +75,7 @@ struct kvm_regs { #define KVM_VGIC_V2_CPU_SIZE0x2000 #define KVM_ARM_VCPU_POWER_OFF 0 /* CPU is started in OFF state */ +#define KVM_ARM_VCPU_EL1_32BIT 1 /* CPU running a 32bit VM */ struct kvm_vcpu_init { __u32 target; diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c index 47d3729..74ef7d5 100644 --- a/arch/arm64/kvm/guest.c +++ b/arch/arm64/kvm/guest.c @@ -93,6 +93,12 @@ static int set_core_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) if (off == KVM_REG_ARM_CORE_REG(regs.pstate)) { unsigned long mode = (*(unsigned long *)valp) COMPAT_PSR_MODE_MASK; switch (mode) { +case COMPAT_PSR_MODE_USR: +case COMPAT_PSR_MODE_FIQ: +case COMPAT_PSR_MODE_IRQ: +case COMPAT_PSR_MODE_SVC: +case COMPAT_PSR_MODE_ABT: +case COMPAT_PSR_MODE_UND: case PSR_MODE_EL0t: case PSR_MODE_EL1t: case PSR_MODE_EL1h: diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c index bc33e76..a282d35 100644 --- a/arch/arm64/kvm/reset.c +++ b/arch/arm64/kvm/reset.c @@ -35,11 +35,27 @@ static struct kvm_regs default_regs_reset = { .regs.pstate = PSR_MODE_EL1h | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT, }; +static struct kvm_regs default_regs_reset32 = { +.regs.pstate = (COMPAT_PSR_MODE_SVC | COMPAT_PSR_A_BIT | +COMPAT_PSR_I_BIT | COMPAT_PSR_F_BIT), +}; + +static bool cpu_has_32bit_el1(void) +{ +u64 pfr0; + +pfr0 = read_cpuid(ID_AA64PFR0_EL1); +return !!(pfr0 0x20); again we don't need the double negation I still hold that it makes things more readable. +} + int kvm_arch_dev_ioctl_check_extention(long ext) { int r; switch (ext) { +case KVM_CAP_ARM_EL1_32BIT: +r = cpu_has_32bit_el1(); +break; default: r = 0; } @@ -62,7 +78,14 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu) switch (vcpu-arch.target) { default: -cpu_reset = default_regs_reset; +if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu-arch.features)) { +if (!cpu_has_32bit_el1()) +return -EINVAL; I'm not sure EINVAL is appropriate here, the value specified was not incorrect, it's that the hardware doesn't support it. ENXIO, ENODEV, and add that in Documentation/virtual/kvm/api.txt ? Not sure. If you ended up here, it means you tried to start a 32bit guest on a 64bit-only CPU, despite KVM_CAP_ARM_EL1_32BIT telling you that your CPU is not 32bit capable. This is clearly an invalid input, isn't it? check the API documentation for this ioctl, I don't think that's the type of invalid input meant when describing the meaning of EINVAL. If you feel strongly about it of course it's no big deal, but I think EINVAL is so overloaded anyway that telling the user something more specific would be great, but I'll leave it up to you. [bit late on this one...] Here's what the documentation says: quote 4.77 KVM_ARM_VCPU_INIT Capability: basic Architectures: arm, arm64 Type: vcpu ioctl Parameters: struct struct kvm_vcpu_init (in) Returns: 0 on success; -1
Re: [PATCH 2/6] KVM: PPC: Add support for multiple-TCE hcalls
On Mon, May 06, 2013 at 05:25:53PM +1000, Alexey Kardashevskiy wrote: This adds real mode handlers for the H_PUT_TCE_INDIRECT and H_STUFF_TCE hypercalls for QEMU emulated devices such as virtio devices or emulated PCI. These calls allow adding multiple entries (up to 512) into the TCE table in one call which saves time on transition to/from real mode. This adds a guest physical to host real address converter and calls the existing H_PUT_TCE handler. The converting function is going to be fully utilized by upcoming VFIO supporting patches. This also implements the KVM_CAP_PPC_MULTITCE capability, so in order to support the functionality of this patch, QEMU needs to query for this capability and set the hcall-multi-tce hypertas property only if the capability is present, otherwise there will be serious performance degradation. Hrm. Clearly I didn't read this carefully enough before. There are some problems here. [snip] diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c index 72ffc89..643ac1e 100644 --- a/arch/powerpc/kvm/book3s_64_vio.c +++ b/arch/powerpc/kvm/book3s_64_vio.c @@ -14,6 +14,7 @@ * * Copyright 2010 Paul Mackerras, IBM Corp. pau...@au1.ibm.com * Copyright 2011 David Gibson, IBM Corporation d...@au1.ibm.com + * Copyright 2013 Alexey Kardashevskiy, IBM Corporation a...@au1.ibm.com */ #include linux/types.h @@ -36,9 +37,14 @@ #include asm/ppc-opcode.h #include asm/kvm_host.h #include asm/udbg.h +#include asm/iommu.h #define TCES_PER_PAGE(PAGE_SIZE / sizeof(u64)) +#define ERROR_ADDR (~(unsigned long)0x0) +/* + * TCE tables handlers. + */ static long kvmppc_stt_npages(unsigned long window_size) { return ALIGN((window_size SPAPR_TCE_SHIFT) @@ -148,3 +154,111 @@ fail: } return ret; } + +/* + * Virtual mode handling of IOMMU map/unmap. + */ +/* Converts guest physical address into host virtual */ +static unsigned long get_virt_address(struct kvm_vcpu *vcpu, + unsigned long gpa) This should probably return a void * rather than an unsigned long. Well, actually a void __user *. +{ + unsigned long hva, gfn = gpa PAGE_SHIFT; + struct kvm_memory_slot *memslot; + + memslot = search_memslots(kvm_memslots(vcpu-kvm), gfn); + if (!memslot) + return ERROR_ADDR; + + /* + * Convert gfn to hva preserving flags and an offset + * within a system page + */ + hva = __gfn_to_hva_memslot(memslot, gfn) + (gpa ~PAGE_MASK); + return hva; +} + +long kvmppc_virtmode_h_put_tce(struct kvm_vcpu *vcpu, + unsigned long liobn, unsigned long ioba, + unsigned long tce) +{ + struct kvmppc_spapr_tce_table *tt; + + tt = kvmppc_find_tce_table(vcpu, liobn); + /* Didn't find the liobn, put it to userspace */ + if (!tt) + return H_TOO_HARD; + + /* Emulated IO */ + return kvmppc_emulated_h_put_tce(tt, ioba, tce); +} + +long kvmppc_virtmode_h_put_tce_indirect(struct kvm_vcpu *vcpu, + unsigned long liobn, unsigned long ioba, + unsigned long tce_list, unsigned long npages) +{ + struct kvmppc_spapr_tce_table *tt; + long i; + unsigned long tces; + + /* The whole table addressed by tce_list resides in 4K page */ + if (npages 512) + return H_PARAMETER; So, that doesn't actually verify what the comment says it does - only that the list is 4K in total. You need to check the alignment of tce_list as well. + + tt = kvmppc_find_tce_table(vcpu, liobn); + /* Didn't find the liobn, put it to userspace */ + if (!tt) + return H_TOO_HARD; + + tces = get_virt_address(vcpu, tce_list); + if (tces == ERROR_ADDR) + return H_TOO_HARD; + + /* Emulated IO */ This comment doesn't seem to have any bearing on the test which follows it. + if ((ioba + (npages IOMMU_PAGE_SHIFT)) tt-window_size) + return H_PARAMETER; + + for (i = 0; i npages; ++i) { + unsigned long tce; + unsigned long ptce = tces + i * sizeof(unsigned long); + + if (get_user(tce, (unsigned long __user *)ptce)) + break; + + if (kvmppc_emulated_h_put_tce(tt, + ioba + (i IOMMU_PAGE_SHIFT), tce)) + break; + } + if (i == npages) + return H_SUCCESS; + + /* Failed, do cleanup */ + do { + --i; + kvmppc_emulated_h_put_tce(tt, ioba + (i IOMMU_PAGE_SHIFT), + 0); + } while (i); Hrm, so, actually PAPR specifies that this hcall is supposed to first copy the given tces to hypervisor memory, then translate (and validate) them all, and only then touch the actual TCE table. Rather more complicated to do, but I guess we should - that would get rid
Re: [PATCH v2 1/4] powerpc: hard_irq_disable(): Call trace_hardirqs_off after disabling
On Thu, 2013-05-09 at 22:09 -0500, Scott Wood wrote: lockdep.c has this: /* * So we're supposed to get called after you mask local IRQs, * but for some reason the hardware doesn't quite think you did * a proper job. */ if (DEBUG_LOCKS_WARN_ON(!irqs_disabled())) return; Since irqs_disabled() is based on soft_enabled(), that (not just the hard EE bit) needs to be 0 before we call trace_hardirqs_off. Signed-off-by: Scott Wood scottw...@freescale.com Oops ;-) I'll apply that to my tree and will send it to Linus right after -rc1, the rest will go the normal way for KVM patches. Cheers, Ben. --- arch/powerpc/include/asm/hw_irq.h |5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h index d615b28..ba713f1 100644 --- a/arch/powerpc/include/asm/hw_irq.h +++ b/arch/powerpc/include/asm/hw_irq.h @@ -96,11 +96,12 @@ static inline bool arch_irqs_disabled(void) #endif #define hard_irq_disable() do {\ + u8 _was_enabled = get_paca()-soft_enabled; \ __hard_irq_disable(); \ - if (local_paca-soft_enabled) \ - trace_hardirqs_off(); \ get_paca()-soft_enabled = 0; \ get_paca()-irq_happened |= PACA_IRQ_HARD_DIS; \ + if (_was_enabled) \ + trace_hardirqs_off(); \ } while(0) static inline bool lazy_irq_pending(void) -- 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
Re: [PATCH 2/6] KVM: PPC: Add support for multiple-TCE hcalls
On 05/10/2013 04:51 PM, David Gibson wrote: On Mon, May 06, 2013 at 05:25:53PM +1000, Alexey Kardashevskiy wrote: This adds real mode handlers for the H_PUT_TCE_INDIRECT and H_STUFF_TCE hypercalls for QEMU emulated devices such as virtio devices or emulated PCI. These calls allow adding multiple entries (up to 512) into the TCE table in one call which saves time on transition to/from real mode. This adds a guest physical to host real address converter and calls the existing H_PUT_TCE handler. The converting function is going to be fully utilized by upcoming VFIO supporting patches. This also implements the KVM_CAP_PPC_MULTITCE capability, so in order to support the functionality of this patch, QEMU needs to query for this capability and set the hcall-multi-tce hypertas property only if the capability is present, otherwise there will be serious performance degradation. Hrm. Clearly I didn't read this carefully enough before. There are some problems here. ? [snip] diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c index 72ffc89..643ac1e 100644 --- a/arch/powerpc/kvm/book3s_64_vio.c +++ b/arch/powerpc/kvm/book3s_64_vio.c @@ -14,6 +14,7 @@ * * Copyright 2010 Paul Mackerras, IBM Corp. pau...@au1.ibm.com * Copyright 2011 David Gibson, IBM Corporation d...@au1.ibm.com + * Copyright 2013 Alexey Kardashevskiy, IBM Corporation a...@au1.ibm.com */ #include linux/types.h @@ -36,9 +37,14 @@ #include asm/ppc-opcode.h #include asm/kvm_host.h #include asm/udbg.h +#include asm/iommu.h #define TCES_PER_PAGE (PAGE_SIZE / sizeof(u64)) +#define ERROR_ADDR (~(unsigned long)0x0) +/* + * TCE tables handlers. + */ static long kvmppc_stt_npages(unsigned long window_size) { return ALIGN((window_size SPAPR_TCE_SHIFT) @@ -148,3 +154,111 @@ fail: } return ret; } + +/* + * Virtual mode handling of IOMMU map/unmap. + */ +/* Converts guest physical address into host virtual */ +static unsigned long get_virt_address(struct kvm_vcpu *vcpu, +unsigned long gpa) This should probably return a void * rather than an unsigned long. Well, actually a void __user *. +{ +unsigned long hva, gfn = gpa PAGE_SHIFT; +struct kvm_memory_slot *memslot; + +memslot = search_memslots(kvm_memslots(vcpu-kvm), gfn); +if (!memslot) +return ERROR_ADDR; + +/* + * Convert gfn to hva preserving flags and an offset + * within a system page + */ +hva = __gfn_to_hva_memslot(memslot, gfn) + (gpa ~PAGE_MASK); +return hva; +} + +long kvmppc_virtmode_h_put_tce(struct kvm_vcpu *vcpu, +unsigned long liobn, unsigned long ioba, +unsigned long tce) +{ +struct kvmppc_spapr_tce_table *tt; + +tt = kvmppc_find_tce_table(vcpu, liobn); +/* Didn't find the liobn, put it to userspace */ +if (!tt) +return H_TOO_HARD; + +/* Emulated IO */ +return kvmppc_emulated_h_put_tce(tt, ioba, tce); +} + +long kvmppc_virtmode_h_put_tce_indirect(struct kvm_vcpu *vcpu, +unsigned long liobn, unsigned long ioba, +unsigned long tce_list, unsigned long npages) +{ +struct kvmppc_spapr_tce_table *tt; +long i; +unsigned long tces; + +/* The whole table addressed by tce_list resides in 4K page */ +if (npages 512) +return H_PARAMETER; So, that doesn't actually verify what the comment says it does - only that the list is 4K in total. You need to check the alignment of tce_list as well. The spec says to return H_PARAMETER if 512. I.e. it takes just 1 page and I do not need to bother if pages may not lay continuously in RAM (matters for real mode). /* * As the spec is saying that maximum possible number of TCEs is 512, * the whole TCE page is no more than 4K. Therefore we do not have to * worry if pages do not lie continuously in the RAM */ Any better?... + +tt = kvmppc_find_tce_table(vcpu, liobn); +/* Didn't find the liobn, put it to userspace */ +if (!tt) +return H_TOO_HARD; + +tces = get_virt_address(vcpu, tce_list); +if (tces == ERROR_ADDR) +return H_TOO_HARD; + +/* Emulated IO */ This comment doesn't seem to have any bearing on the test which follows it. +if ((ioba + (npages IOMMU_PAGE_SHIFT)) tt-window_size) +return H_PARAMETER; + +for (i = 0; i npages; ++i) { +unsigned long tce; +unsigned long ptce = tces + i * sizeof(unsigned long); + +if (get_user(tce, (unsigned long __user *)ptce)) +break; + +if (kvmppc_emulated_h_put_tce(tt, +ioba + (i IOMMU_PAGE_SHIFT), tce)) +break; +} +if (i == npages) +return H_SUCCESS; + +/* Failed, do cleanup */ +do { +--i; +
Re: [PATCH] powerpc: export debug register save function for KVM
On 07.05.2013, at 11:40, Bharat Bhushan wrote: KVM need this function when switching from vcpu to user-space thread. My subsequent patch will use this function. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/include/asm/switch_to.h |4 arch/powerpc/kernel/process.c|3 ++- 2 files changed, 6 insertions(+), 1 deletions(-) diff --git a/arch/powerpc/include/asm/switch_to.h b/arch/powerpc/include/asm/switch_to.h index 200d763..50b357f 100644 --- a/arch/powerpc/include/asm/switch_to.h +++ b/arch/powerpc/include/asm/switch_to.h @@ -30,6 +30,10 @@ extern void enable_kernel_spe(void); extern void giveup_spe(struct task_struct *); extern void load_up_spe(struct task_struct *); +#ifdef CONFIG_PPC_ADV_DEBUG_REGS +extern void switch_booke_debug_regs(struct thread_struct *new_thread); +#endif + #ifndef CONFIG_SMP extern void discard_lazy_cpu_state(void); #else diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c index ca89375..a938138 100644 --- a/arch/powerpc/kernel/process.c +++ b/arch/powerpc/kernel/process.c @@ -362,12 +362,13 @@ static void prime_debug_regs(struct thread_struct *thread) * debug registers, set the debug registers from the values * stored in the new thread. */ -static void switch_booke_debug_regs(struct thread_struct *new_thread) +void switch_booke_debug_regs(struct thread_struct *new_thread) { if ((current-thread.debug.dbcr0 DBCR0_IDM) || (new_thread-debug.dbcr0 DBCR0_IDM)) prime_debug_regs(new_thread); } +EXPORT_SYMBOL(switch_booke_debug_regs); EXPORT_SYMBOL_GPL Alex -- 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
RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
-Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. Add the new vectors, but there's more than this required to make Altivec work in the guest, and we can't prevent the guest from turning on Altivec (which can corrupt host state until state save/restore is implemented). Disable e6500 on KVM until this is fixed. To be more precise it can corrupt Altivec host state. Signed-off-by: Scott Wood scottw...@freescale.com Cc: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/bookehv_interrupts.S |4 arch/powerpc/kvm/e500mc.c |2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index e8ed7d6..6397613 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -309,6 +309,10 @@ kvm_handler BOOKE_INTERRUPT_DOORBELL_CRITICAL, EX_PARAMS(CRIT), \ SPRN_CSRR0, SPRN_CSRR1, 0 kvm_handler BOOKE_INTERRUPT_HV_PRIV, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, NEED_EMU +kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 +kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 Why not NEED_ESR as we did in our SDK? kvm_handler BOOKE_INTERRUPT_HV_SYSCALL, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, 0 kvm_handler BOOKE_INTERRUPT_GUEST_DBELL, EX_PARAMS(GDBELL), \ diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index 753cc99..19c8379 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -177,8 +177,6 @@ int kvmppc_core_check_processor_compat(void) r = 0; else if (strcmp(cur_cpu_spec-cpu_name, e5500) == 0) r = 0; - else if (strcmp(cur_cpu_spec-cpu_name, e6500) == 0) - r = 0; Hmm ... I made it clear in the commit message that Altivec is not supported, why couldn't we be more gentle: diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index c3bdc0a..4f43a36 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -172,8 +172,11 @@ int kvmppc_core_check_processor_compat(void) r = 0; else if (strcmp(cur_cpu_spec-cpu_name, e5500) == 0) r = 0; +#ifndef CONFIG_ALTIVEC +/* ALTIVEC is not supported now by KVM so only one of them can work */ else if (strcmp(cur_cpu_spec-cpu_name, e6500) == 0) r = 0; +#endif else r = -ENOTSUPP; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 82f155e..bb1a9e0 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -973,6 +973,16 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, break; #endif + case BOOKE_INTERRUPT_ALTIVEC_UNAVAIL: + case BOOKE_INTERRUPT_ALTIVEC_ASSIST: + /* +* Guest wants ALTIVEC, but host kernel doesn't support it. +* send an unimplemented operation program check to the guest. +*/ + kvmppc_core_queue_program(vcpu, ESR_PUO | ESR_SPV); + r = RESUME_GUEST; + break; + case BOOKE_INTERRUPT_DATA_STORAGE: kvmppc_core_queue_data_storage(vcpu, vcpu-arch.fault_dear, vcpu-arch.fault_esr); Kconfig should also be updated (in both proposals). -Mike -- 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
Re: [PATCH] KVM: PPC: Add userspace debug stub support
On 07.05.2013, at 11:40, Bharat Bhushan wrote: This patch adds the debug stub support on booke/bookehv. Now QEMU debug stub can use hw breakpoint, watchpoint and software breakpoint to debug guest. This is how we save/restore debug register context when switching between guest, userspace and kernel user-process: When QEMU is running - thread-debug_reg == QEMU debug register context. - Kernel will handle switching the debug register on context switch. - no vcpu_load() called QEMU makes ioctls (except RUN) - This will call vcpu_load() - should not change context. - Some ioctls can change vcpu debug register, context saved in vcpu-debug_regs QEMU Makes RUN ioctl - Save thread-debug_reg on STACK - Store thread-debug_reg == vcpu-debug_reg - load thread-debug_reg - RUN VCPU ( So thread points to vcpu context ) Context switch happens When VCPU running - makes vcpu_load() should not load any context - kernel loads the vcpu context as thread-debug_regs points to vcpu context. On heavyweight_exit - Load the context saved on stack in thread-debug_reg Currently we do not support debug resource emulation to guest, On debug exception, always exit to user space irrespective of user space is expecting the debug exception or not. If this is unexpected exception (breakpoint/watchpoint event not set by userspace) then let us leave the action on user space. This is similar to what it was before, only thing is that now we have proper exit state available to user space. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/include/asm/kvm_host.h |3 + arch/powerpc/include/uapi/asm/kvm.h |1 + arch/powerpc/kvm/booke.c| 242 --- arch/powerpc/kvm/booke.h|5 + 4 files changed, 233 insertions(+), 18 deletions(-) diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index 838a577..1b29945 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -524,7 +524,10 @@ struct kvm_vcpu_arch { u32 eptcfg; u32 epr; u32 crit_save; + /* guest debug registers*/ struct debug_reg dbg_reg; + /* shadow debug registers */ Please be more verbose here. What exactly does this contain? Why do we need shadow and non-shadow registers? The comment as it is reads like /* Add one plus one */ x = 1 + 1; + struct debug_reg shadow_dbg_reg; #endif gpa_t paddr_accessed; gva_t vaddr_accessed; diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h index ded0607..f5077c2 100644 --- a/arch/powerpc/include/uapi/asm/kvm.h +++ b/arch/powerpc/include/uapi/asm/kvm.h @@ -27,6 +27,7 @@ #define __KVM_HAVE_PPC_SMT #define __KVM_HAVE_IRQCHIP #define __KVM_HAVE_IRQ_LINE +#define __KVM_HAVE_GUEST_DEBUG struct kvm_regs { __u64 pc; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index ef99536..6a44ad4 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -133,6 +133,29 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu) #endif } +static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) +{ + /* Synchronize guest's desire to get debug interrupts into shadow MSR */ +#ifndef CONFIG_KVM_BOOKE_HV + vcpu-arch.shadow_msr = ~MSR_DE; + vcpu-arch.shadow_msr |= vcpu-arch.shared-msr MSR_DE; +#endif + + /* Force enable debug interrupts when user space wants to debug */ + if (vcpu-guest_debug) { +#ifdef CONFIG_KVM_BOOKE_HV + /* + * Since there is no shadow MSR, sync MSR_DE into the guest + * visible MSR. + */ + vcpu-arch.shared-msr |= MSR_DE; +#else + vcpu-arch.shadow_msr |= MSR_DE; + vcpu-arch.shared-msr = ~MSR_DE; +#endif + } +} + /* * Helper function for full MSR writes. No need to call this if only * EE/CE/ME/DE/RI are changing. @@ -150,6 +173,7 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr) kvmppc_mmu_msr_notify(vcpu, old_msr); kvmppc_vcpu_sync_spe(vcpu); kvmppc_vcpu_sync_fpu(vcpu); + kvmppc_vcpu_sync_debug(vcpu); } static void kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu, @@ -655,6 +679,7 @@ int kvmppc_core_check_requests(struct kvm_vcpu *vcpu) int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu) { int ret, s; + struct debug_reg debug; #ifdef CONFIG_PPC_FPU unsigned int fpscr; int fpexc_mode; @@ -699,11 +724,27 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu) kvmppc_load_guest_fp(vcpu); #endif + /* + * Clear current-thread.dbcr0 so that kernel does not + * restore h/w registers on context switch in vcpu running state. + */ Incorrect comment? + /* Save thread-debug context on stack */ /* Switch to guest debug
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. How hard would it be to add? I suppose it's broken in 3.10, so we need something quick before it gets released? Add the new vectors, but there's more than this required to make Altivec work in the guest, and we can't prevent the guest from turning on Altivec (which can corrupt host state until state save/restore is implemented). Disable e6500 on KVM until this is fixed. To be more precise it can corrupt Altivec host state. Signed-off-by: Scott Wood scottw...@freescale.com Cc: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/bookehv_interrupts.S |4 arch/powerpc/kvm/e500mc.c |2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index e8ed7d6..6397613 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -309,6 +309,10 @@ kvm_handler BOOKE_INTERRUPT_DOORBELL_CRITICAL, EX_PARAMS(CRIT), \ SPRN_CSRR0, SPRN_CSRR1, 0 kvm_handler BOOKE_INTERRUPT_HV_PRIV, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, NEED_EMU +kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \ +SPRN_SRR0, SPRN_SRR1, 0 +kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \ +SPRN_SRR0, SPRN_SRR1, 0 Why not NEED_ESR as we did in our SDK? kvm_handler BOOKE_INTERRUPT_HV_SYSCALL, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, 0 kvm_handler BOOKE_INTERRUPT_GUEST_DBELL, EX_PARAMS(GDBELL), \ diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index 753cc99..19c8379 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -177,8 +177,6 @@ int kvmppc_core_check_processor_compat(void) r = 0; else if (strcmp(cur_cpu_spec-cpu_name, e5500) == 0) r = 0; -else if (strcmp(cur_cpu_spec-cpu_name, e6500) == 0) -r = 0; Hmm ... I made it clear in the commit message that Altivec is not supported, why couldn't we be more gentle: diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index c3bdc0a..4f43a36 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -172,8 +172,11 @@ int kvmppc_core_check_processor_compat(void) r = 0; else if (strcmp(cur_cpu_spec-cpu_name, e5500) == 0) r = 0; +#ifndef CONFIG_ALTIVEC This would still be wrong, since running 2 guests with altivec would break each other. Alex +/* ALTIVEC is not supported now by KVM so only one of them can work */ else if (strcmp(cur_cpu_spec-cpu_name, e6500) == 0) r = 0; +#endif else r = -ENOTSUPP; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 82f155e..bb1a9e0 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -973,6 +973,16 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, break; #endif + case BOOKE_INTERRUPT_ALTIVEC_UNAVAIL: + case BOOKE_INTERRUPT_ALTIVEC_ASSIST: + /* +* Guest wants ALTIVEC, but host kernel doesn't support it. +* send an unimplemented operation program check to the guest. +*/ + kvmppc_core_queue_program(vcpu, ESR_PUO | ESR_SPV); + r = RESUME_GUEST; + break; + case BOOKE_INTERRUPT_DATA_STORAGE: kvmppc_core_queue_data_storage(vcpu, vcpu-arch.fault_dear, vcpu-arch.fault_esr); Kconfig should also be updated (in both proposals). -Mike -- 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-ppc 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/ppc/booke64: fix build breakage from Altivec, and disable e6500
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 4:16 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. How hard would it be to add? I suppose it's broken in 3.10, so we need something quick before it gets released? Not so hard. Yes. I was surprised by this patch given the fact that we have planned to send altivec support upstream this days and that we already have a similar patch from Tiejun on our list. Add the new vectors, but there's more than this required to make Altivec work in the guest, and we can't prevent the guest from turning on Altivec (which can corrupt host state until state save/restore is implemented). Disable e6500 on KVM until this is fixed. To be more precise it can corrupt Altivec host state. Signed-off-by: Scott Wood scottw...@freescale.com Cc: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/bookehv_interrupts.S |4 arch/powerpc/kvm/e500mc.c |2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index e8ed7d6..6397613 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -309,6 +309,10 @@ kvm_handler BOOKE_INTERRUPT_DOORBELL_CRITICAL, EX_PARAMS(CRIT), \ SPRN_CSRR0, SPRN_CSRR1, 0 kvm_handler BOOKE_INTERRUPT_HV_PRIV, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, NEED_EMU +kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 +kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 Why not NEED_ESR as we did in our SDK? kvm_handler BOOKE_INTERRUPT_HV_SYSCALL, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, 0 kvm_handler BOOKE_INTERRUPT_GUEST_DBELL, EX_PARAMS(GDBELL), \ diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index 753cc99..19c8379 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -177,8 +177,6 @@ int kvmppc_core_check_processor_compat(void) r = 0; else if (strcmp(cur_cpu_spec-cpu_name, e5500) == 0) r = 0; - else if (strcmp(cur_cpu_spec-cpu_name, e6500) == 0) - r = 0; Hmm ... I made it clear in the commit message that Altivec is not supported, why couldn't we be more gentle: diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index c3bdc0a..4f43a36 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -172,8 +172,11 @@ int kvmppc_core_check_processor_compat(void) r = 0; else if (strcmp(cur_cpu_spec-cpu_name, e5500) == 0) r = 0; +#ifndef CONFIG_ALTIVEC This would still be wrong, since running 2 guests with altivec would break each other. It's wrong if you expect to have altivec supported in the VM which is not the case. Guests that executes altivec instructions will get a program (unimplemented op) exception see below. Do we really need to remove e6500 all together for this? Alex +/* ALTIVEC is not supported now by KVM so only one of them can work */ else if (strcmp(cur_cpu_spec-cpu_name, e6500) == 0) r = 0; +#endif else r = -ENOTSUPP; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 82f155e..bb1a9e0 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -973,6 +973,16 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, break; #endif + case BOOKE_INTERRUPT_ALTIVEC_UNAVAIL: + case BOOKE_INTERRUPT_ALTIVEC_ASSIST: + /* +* Guest wants ALTIVEC, but host kernel doesn't support it. +* send an unimplemented operation program check to the guest. +*/ + kvmppc_core_queue_program(vcpu, ESR_PUO | ESR_SPV); + r = RESUME_GUEST; +
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On Fri, May 10, 2013 at 08:07:00AM +1000, Benjamin Herrenschmidt wrote: On Thu, 2013-05-09 at 16:27 -0500, Scott Wood wrote: On 05/09/2013 07:37:42 AM, Benjamin Herrenschmidt wrote: On Thu, 2013-05-09 at 17:44 +0800, tiejun.chen wrote: Actually in the case GS=1 even if EE=0, EXT/DEC/DBELL still occur as I recall. Only if directed to the hypervisor. This is always the case with KVM, right? At least on booke... Hrm, on A2 we could choose iirc. Well not DEC but EXT at least, I don't remember about DBELL. Case 1) - Local_irq_disable() will set soft_enabled = 0 - Now Externel interrupt happens, there we set PACA_IRQ_EE in irq_happened, Also clears EE in SRR1 and rfi. So interrupts are hard disabled. No more other interrupt gated by MSR.EE can happen. Looks like the idea here is to not let a device keep on inserting interrupt till the interrupt condition on device is cleared, right? The external interrupt line is level sensitive normally, so we have to mask MSR:EE in that case or the interrupt would keep re-occurring (note that FSL has this concept of auto-acked interrupts via the on die MPIC for which you can potentially use PACA_IRQ_EE_EDGE instead and avoid having to mask MSR:EE). Note that if we do this, we can no longer leave the interrupt vector in EPR, and would have to track (potentially multiple different) pending external interrupts in software. Right, you can have a little queue in the PACA and leave EE disabled if it's full. You can probably get away with a queue of 1 though :-) So I would assume you will not pick up these two patches, right? http://patchwork.ozlabs.org/patch/235530/ http://patchwork.ozlabs.org/patch/235532/ Anyway it is more easier to enable the external proxy by using this method. But if you insist, I can respin a patch to use the method you suggested since it will definitely reduce the window where the irq is hard disabled. Thanks, Kevin Cheers, Ben. -- 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 pgpVwG6hJXYWj.pgp Description: PGP signature
RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. -Mike -- 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
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) Alex -- 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
RE: [PATCH] KVM: PPC: Add userspace debug stub support
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 3:48 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; tiejun.c...@windriver.com; Bhushan Bharat-R65777 Subject: Re: [PATCH] KVM: PPC: Add userspace debug stub support On 07.05.2013, at 11:40, Bharat Bhushan wrote: This patch adds the debug stub support on booke/bookehv. Now QEMU debug stub can use hw breakpoint, watchpoint and software breakpoint to debug guest. This is how we save/restore debug register context when switching between guest, userspace and kernel user-process: When QEMU is running - thread-debug_reg == QEMU debug register context. - Kernel will handle switching the debug register on context switch. - no vcpu_load() called QEMU makes ioctls (except RUN) - This will call vcpu_load() - should not change context. - Some ioctls can change vcpu debug register, context saved in - vcpu-debug_regs QEMU Makes RUN ioctl - Save thread-debug_reg on STACK - Store thread-debug_reg == vcpu-debug_reg load thread-debug_reg - RUN VCPU ( So thread points to vcpu context ) Context switch happens When VCPU running - makes vcpu_load() should not load any context kernel loads the vcpu - context as thread-debug_regs points to vcpu context. On heavyweight_exit - Load the context saved on stack in thread-debug_reg Currently we do not support debug resource emulation to guest, On debug exception, always exit to user space irrespective of user space is expecting the debug exception or not. If this is unexpected exception (breakpoint/watchpoint event not set by userspace) then let us leave the action on user space. This is similar to what it was before, only thing is that now we have proper exit state available to user space. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/include/asm/kvm_host.h |3 + arch/powerpc/include/uapi/asm/kvm.h |1 + arch/powerpc/kvm/booke.c| 242 --- arch/powerpc/kvm/booke.h|5 + 4 files changed, 233 insertions(+), 18 deletions(-) diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index 838a577..1b29945 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -524,7 +524,10 @@ struct kvm_vcpu_arch { u32 eptcfg; u32 epr; u32 crit_save; + /* guest debug registers*/ struct debug_reg dbg_reg; + /* shadow debug registers */ Please be more verbose here. What exactly does this contain? Why do we need shadow and non-shadow registers? The comment as it is reads like /* Add one plus one */ x = 1 + 1; /* * Shadow debug registers hold the debug register content * to be written in h/w debug register on behalf of guest * written value or user space written value. */ + struct debug_reg shadow_dbg_reg; #endif gpa_t paddr_accessed; gva_t vaddr_accessed; diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h index ded0607..f5077c2 100644 --- a/arch/powerpc/include/uapi/asm/kvm.h +++ b/arch/powerpc/include/uapi/asm/kvm.h @@ -27,6 +27,7 @@ #define __KVM_HAVE_PPC_SMT #define __KVM_HAVE_IRQCHIP #define __KVM_HAVE_IRQ_LINE +#define __KVM_HAVE_GUEST_DEBUG struct kvm_regs { __u64 pc; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index ef99536..6a44ad4 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -133,6 +133,29 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu) #endif } +static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) { + /* Synchronize guest's desire to get debug interrupts into shadow +MSR */ #ifndef CONFIG_KVM_BOOKE_HV + vcpu-arch.shadow_msr = ~MSR_DE; + vcpu-arch.shadow_msr |= vcpu-arch.shared-msr MSR_DE; #endif + + /* Force enable debug interrupts when user space wants to debug */ + if (vcpu-guest_debug) { +#ifdef CONFIG_KVM_BOOKE_HV + /* +* Since there is no shadow MSR, sync MSR_DE into the guest +* visible MSR. +*/ + vcpu-arch.shared-msr |= MSR_DE; +#else + vcpu-arch.shadow_msr |= MSR_DE; + vcpu-arch.shared-msr = ~MSR_DE; +#endif + } +} + /* * Helper function for full MSR writes. No need to call this if only * EE/CE/ME/DE/RI are changing. @@ -150,6 +173,7 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr) kvmppc_mmu_msr_notify(vcpu, old_msr); kvmppc_vcpu_sync_spe(vcpu); kvmppc_vcpu_sync_fpu(vcpu); + kvmppc_vcpu_sync_debug(vcpu); } static void kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu, @@ -655,6 +679,7 @@ int kvmppc_core_check_requests(struct kvm_vcpu *vcpu) int kvmppc_vcpu_run(struct
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 05/10/2013 04:40:01 AM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. Exceptions don't get handled all that often, and ideally we catch it when it's added rather than after-the-fact. diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index e8ed7d6..6397613 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -309,6 +309,10 @@ kvm_handler BOOKE_INTERRUPT_DOORBELL_CRITICAL, EX_PARAMS(CRIT), \ SPRN_CSRR0, SPRN_CSRR1, 0 kvm_handler BOOKE_INTERRUPT_HV_PRIV, EX_PARAMS(GEN), \ SPRN_SRR0, SPRN_SRR1, NEED_EMU +kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 +kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 Why not NEED_ESR as we did in our SDK? This is just to fix the build break -- we don't need ESR yet. Though I did look at the ESR documentation and didn't see where Altivec exceptions used it (I do see a couple places now). -Scott -- 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
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 05/10/2013 09:11:24 AM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 4:16 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. How hard would it be to add? I suppose it's broken in 3.10, so we need something quick before it gets released? Not so hard. Yes. I was surprised by this patch given the fact that we have planned to send altivec support upstream this days and that we already have a similar patch from Tiejun on our list. I didn't see Tiejun's patch... My goal was just to fix the build break without exposing problems, and to encourage a patch to fix it properly to happen sooner rather than later. With Tiejun's patch, which is similar to mine except that it doesn't disable e6500 support, a user could BUG() the kernel by forcing an Altivec exception in a guest. I didn't want to go further down the road of adding reflectors for those exceptions, which could make it look like the problem was dealt with even though it's still not done. -Scott -- 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
Re: [PATCH] KVM: PPC: Add userspace debug stub support
On 10.05.2013, at 19:31, Bhushan Bharat-R65777 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 3:48 PM To: Bhushan Bharat-R65777 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; tiejun.c...@windriver.com; Bhushan Bharat-R65777 Subject: Re: [PATCH] KVM: PPC: Add userspace debug stub support On 07.05.2013, at 11:40, Bharat Bhushan wrote: This patch adds the debug stub support on booke/bookehv. Now QEMU debug stub can use hw breakpoint, watchpoint and software breakpoint to debug guest. This is how we save/restore debug register context when switching between guest, userspace and kernel user-process: When QEMU is running - thread-debug_reg == QEMU debug register context. - Kernel will handle switching the debug register on context switch. - no vcpu_load() called QEMU makes ioctls (except RUN) - This will call vcpu_load() - should not change context. - Some ioctls can change vcpu debug register, context saved in - vcpu-debug_regs QEMU Makes RUN ioctl - Save thread-debug_reg on STACK - Store thread-debug_reg == vcpu-debug_reg load thread-debug_reg - RUN VCPU ( So thread points to vcpu context ) Context switch happens When VCPU running - makes vcpu_load() should not load any context kernel loads the vcpu - context as thread-debug_regs points to vcpu context. On heavyweight_exit - Load the context saved on stack in thread-debug_reg Currently we do not support debug resource emulation to guest, On debug exception, always exit to user space irrespective of user space is expecting the debug exception or not. If this is unexpected exception (breakpoint/watchpoint event not set by userspace) then let us leave the action on user space. This is similar to what it was before, only thing is that now we have proper exit state available to user space. Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- arch/powerpc/include/asm/kvm_host.h |3 + arch/powerpc/include/uapi/asm/kvm.h |1 + arch/powerpc/kvm/booke.c| 242 --- arch/powerpc/kvm/booke.h|5 + 4 files changed, 233 insertions(+), 18 deletions(-) diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index 838a577..1b29945 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -524,7 +524,10 @@ struct kvm_vcpu_arch { u32 eptcfg; u32 epr; u32 crit_save; + /* guest debug registers*/ struct debug_reg dbg_reg; + /* shadow debug registers */ Please be more verbose here. What exactly does this contain? Why do we need shadow and non-shadow registers? The comment as it is reads like /* Add one plus one */ x = 1 + 1; /* * Shadow debug registers hold the debug register content * to be written in h/w debug register on behalf of guest * written value or user space written value. */ /* hardware visible debug registers when in guest state */ + struct debug_reg shadow_dbg_reg; #endif gpa_t paddr_accessed; gva_t vaddr_accessed; diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h index ded0607..f5077c2 100644 --- a/arch/powerpc/include/uapi/asm/kvm.h +++ b/arch/powerpc/include/uapi/asm/kvm.h @@ -27,6 +27,7 @@ #define __KVM_HAVE_PPC_SMT #define __KVM_HAVE_IRQCHIP #define __KVM_HAVE_IRQ_LINE +#define __KVM_HAVE_GUEST_DEBUG struct kvm_regs { __u64 pc; diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index ef99536..6a44ad4 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -133,6 +133,29 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu) #endif } +static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) { + /* Synchronize guest's desire to get debug interrupts into shadow +MSR */ #ifndef CONFIG_KVM_BOOKE_HV + vcpu-arch.shadow_msr = ~MSR_DE; + vcpu-arch.shadow_msr |= vcpu-arch.shared-msr MSR_DE; #endif + + /* Force enable debug interrupts when user space wants to debug */ + if (vcpu-guest_debug) { +#ifdef CONFIG_KVM_BOOKE_HV + /* +* Since there is no shadow MSR, sync MSR_DE into the guest +* visible MSR. +*/ + vcpu-arch.shared-msr |= MSR_DE; +#else + vcpu-arch.shadow_msr |= MSR_DE; + vcpu-arch.shared-msr = ~MSR_DE; +#endif + } +} + /* * Helper function for full MSR writes. No need to call this if only * EE/CE/ME/DE/RI are changing. @@ -150,6 +173,7 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr) kvmppc_mmu_msr_notify(vcpu, old_msr); kvmppc_vcpu_sync_spe(vcpu); kvmppc_vcpu_sync_fpu(vcpu); + kvmppc_vcpu_sync_debug(vcpu); } static void kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu, @@ -655,6 +679,7 @@ int kvmppc_core_check_requests(struct kvm_vcpu *vcpu) int
RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
-Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Friday, May 10, 2013 7:51 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) I can make sure that it doesn't break 32-bit. Altivec is not present in e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host. Are you looking for G4 support? -Mike -- 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
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 05/10/2013 01:03:27 PM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Friday, May 10, 2013 7:51 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) I can make sure that it doesn't break 32-bit. Altivec is not present in e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host. Are you looking for G4 support? Just because Altivec isn't present on 32-bit doesn't mean that a particular patch can't break things there. Just as a patch to deal with lazy ee issues can break 32-bit, even though 32-bit doesn't have lazy ee. :-) -Scott -- 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
Re: [v2][KVM][PATCH 1/1] kvm:ppc: enable doorbell exception with CONFIG_PPC_DOORBELL
On 07.05.2013, at 12:23, Tiejun Chen wrote: CONFIG_PPC_DOORBELL is enough to cover all variants. Signed-off-by: Tiejun Chen tiejun.c...@windriver.com --- arch/powerpc/kvm/booke.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 1020119..62d4ece 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -795,7 +795,7 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu, kvmppc_fill_pt_regs(regs); timer_interrupt(regs); break; -#if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_BOOK3E_64) +#if defined(CONFIG_PPC_DOORBELL) The same question still holds. How is this an improvement over the previous code? Does this fix any issues for you? Is this just a coding style cleanup? Alex case BOOKE_INTERRUPT_DOORBELL: kvmppc_fill_pt_regs(regs); doorbell_exception(regs); -- 1.7.9.5 -- 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
Re: [v2][KVM][PATCH 1/1] kvm:ppc: enable doorbell exception with CONFIG_PPC_DOORBELL
On 05/10/2013 01:14:27 PM, Alexander Graf wrote: On 07.05.2013, at 12:23, Tiejun Chen wrote: CONFIG_PPC_DOORBELL is enough to cover all variants. Signed-off-by: Tiejun Chen tiejun.c...@windriver.com --- arch/powerpc/kvm/booke.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 1020119..62d4ece 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -795,7 +795,7 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu, kvmppc_fill_pt_regs(regs); timer_interrupt(regs); break; -#if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_BOOK3E_64) +#if defined(CONFIG_PPC_DOORBELL) The same question still holds. How is this an improvement over the previous code? Does this fix any issues for you? Is this just a coding style cleanup? This is an improvement because CONFIG_PPC_DOORBELL is what controls whether the function that is called inside the ifdef exists. -Scott -- 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
Re: [v2][KVM][PATCH 1/1] kvm:ppc: enable doorbell exception with CONFIG_PPC_DOORBELL
On 10.05.2013, at 20:17, Scott Wood wrote: On 05/10/2013 01:14:27 PM, Alexander Graf wrote: On 07.05.2013, at 12:23, Tiejun Chen wrote: CONFIG_PPC_DOORBELL is enough to cover all variants. Signed-off-by: Tiejun Chen tiejun.c...@windriver.com --- arch/powerpc/kvm/booke.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 1020119..62d4ece 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -795,7 +795,7 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu, kvmppc_fill_pt_regs(regs); timer_interrupt(regs); break; -#if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_BOOK3E_64) +#if defined(CONFIG_PPC_DOORBELL) The same question still holds. How is this an improvement over the previous code? Does this fix any issues for you? Is this just a coding style cleanup? This is an improvement because CONFIG_PPC_DOORBELL is what controls whether the function that is called inside the ifdef exists. Aha! Now that's a good reason. Tiejun, please adjust your patch description accordingly. Alex -- 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
RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
-Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 8:42 PM To: Caraman Mihai Claudiu-B02008 Cc: Alexander Graf; Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 05/10/2013 09:11:24 AM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 4:16 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. How hard would it be to add? I suppose it's broken in 3.10, so we need something quick before it gets released? Not so hard. Yes. I was surprised by this patch given the fact that we have planned to send altivec support upstream this days and that we already have a similar patch from Tiejun on our list. I didn't see Tiejun's patch... My goal was just to fix the build break without exposing problems, and to encourage a patch to fix it properly to happen sooner rather than later. With Tiejun's patch, which is similar to mine except that it doesn't disable e6500 support, a user could BUG() the kernel by forcing an Altivec exception in a guest. I didn't want to go further down the road of adding reflectors for those exceptions, which could make it look like the problem was dealt with even though it's still not done. I agree it's quite annoying to hit a build breakage. Reflection is not a proper solution for this problem (though we will require it later) but program exception injection looks feasible as a simple fix. I wouldn't want to see e6500 removed for this reason. Thanks, Mike -- 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
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 10.05.2013, at 20:06, Scott Wood wrote: On 05/10/2013 01:03:27 PM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Friday, May 10, 2013 7:51 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) I can make sure that it doesn't break 32-bit. Altivec is not present in e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host. Are you looking for G4 support? Just because Altivec isn't present on 32-bit doesn't mean that a particular patch can't break things there. Just as a patch to deal with lazy ee issues can break 32-bit, even though 32-bit doesn't have lazy ee. :-) Yeah, that's what I was referring to :). G4s use a different code path altogether and already work with Altivec for years. I doubt you'll manage to break anything there :). So what you should ensure is 1) 32bit e500 still compiles 2) 32bit e500 still executes fine, albeit without altivec Alex -- 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
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 05/10/2013 01:20:33 PM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 8:42 PM To: Caraman Mihai Claudiu-B02008 Cc: Alexander Graf; Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 05/10/2013 09:11:24 AM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 4:16 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 6:15 AM To: Alexander Graf Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 BookE altivec support brought two new exceptions, but KVM was not updated, so the build broke for all 64-bit booke with KVM enabled. We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/ BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have done this in Kumar's tree but we missed that chance. We will face similar issues every time an exception handler will be added. How hard would it be to add? I suppose it's broken in 3.10, so we need something quick before it gets released? Not so hard. Yes. I was surprised by this patch given the fact that we have planned to send altivec support upstream this days and that we already have a similar patch from Tiejun on our list. I didn't see Tiejun's patch... My goal was just to fix the build break without exposing problems, and to encourage a patch to fix it properly to happen sooner rather than later. With Tiejun's patch, which is similar to mine except that it doesn't disable e6500 support, a user could BUG() the kernel by forcing an Altivec exception in a guest. I didn't want to go further down the road of adding reflectors for those exceptions, which could make it look like the problem was dealt with even though it's still not done. I agree it's quite annoying to hit a build breakage. Reflection is not a proper solution for this problem (though we will require it later) but program exception injection looks feasible as a simple fix. Program exception injection still doesn't deal with state corruption. I wouldn't want to see e6500 removed for this reason. It's not being removed, just closed while under construction to prevent damage to people passing through. :-) -Scott -- 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
RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
-Original Message- From: Wood Scott-B07421 Sent: Friday, May 10, 2013 9:06 PM To: Caraman Mihai Claudiu-B02008 Cc: Alexander Graf; Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 05/10/2013 01:03:27 PM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Friday, May 10, 2013 7:51 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) I can make sure that it doesn't break 32-bit. Altivec is not present in e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host. Are you looking for G4 support? Just because Altivec isn't present on 32-bit doesn't mean that a particular patch can't break things there. Just as a patch to deal with lazy ee issues can break 32-bit, even though 32-bit doesn't have lazy ee. :-) That's exactly what I have said. That patch was intended as a RFC I forgot to mark it as such under time constraints, however it reached its goal. -Mike -- 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
RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 9:23 PM To: Wood Scott-B07421 Cc: Caraman Mihai Claudiu-B02008; Wood Scott-B07421; kvm- p...@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 20:06, Scott Wood wrote: On 05/10/2013 01:03:27 PM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Friday, May 10, 2013 7:51 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) I can make sure that it doesn't break 32-bit. Altivec is not present in e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host. Are you looking for G4 support? Just because Altivec isn't present on 32-bit doesn't mean that a particular patch can't break things there. Just as a patch to deal with lazy ee issues can break 32-bit, even though 32-bit doesn't have lazy ee. :-) Yeah, that's what I was referring to :). G4s use a different code path altogether and already work with Altivec for years. Huh ... I forgot that G4 in handled under Book3S, not so obvious. I doubt you'll manage to break anything there :). If you have stated breaks instead of works would have been more clear where you were heading :-J -Mike -- 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
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
Am 10.05.2013 um 20:51 schrieb Caraman Mihai Claudiu-B02008 b02...@freescale.com: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Friday, May 10, 2013 9:23 PM To: Wood Scott-B07421 Cc: Caraman Mihai Claudiu-B02008; Wood Scott-B07421; kvm- p...@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 20:06, Scott Wood wrote: On 05/10/2013 01:03:27 PM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Friday, May 10, 2013 7:51 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500 On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote: Do we really need to remove e6500 all together for this? No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase. I should have it by Monday, the smoke tests passed. Please make sure it also works on 32bit :) I can make sure that it doesn't break 32-bit. Altivec is not present in e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host. Are you looking for G4 support? Just because Altivec isn't present on 32-bit doesn't mean that a particular patch can't break things there. Just as a patch to deal with lazy ee issues can break 32-bit, even though 32-bit doesn't have lazy ee. :-) Yeah, that's what I was referring to :). G4s use a different code path altogether and already work with Altivec for years. Huh ... I forgot that G4 in handled under Book3S, not so obvious. I doubt you'll manage to break anything there :). If you have stated breaks instead of works would have been more clear where you were heading :-J Ok, next time I'll make it more obvious :). Alex -Mike -- 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
Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500
On 05/10/2013 02:06:53 PM, Caraman Mihai Claudiu-B02008 wrote: I didn't see Tiejun's patch... My goal was just to fix the build break without exposing problems, and to encourage a patch to fix it properly to happen sooner rather than later. With Tiejun's patch, which is similar to mine except that it doesn't disable e6500 support, a user could BUG() the kernel by forcing an Altivec exception in a guest. I didn't want to go further down the road of adding reflectors for those exceptions, which could make it look like the problem was dealt with even though it's still not done. I agree it's quite annoying to hit a build breakage. Reflection is not a proper solution for this problem (though we will require it later) but program exception injection looks feasible as a simple fix. Program exception injection still doesn't deal with state corruption. Yes but it's not critical for this particular case since nobody is able to effectively use that state via altivec instructions. Leaking state however can be a real issue. Depending on guest behavior it could look like things are working even though they aren't (e.g. a guest just enables MSR[VEC] and uses altivec instructions, not relying on exceptions). This really isn't worth spending a lot of time debating... Once Altivec is fixed properly (you said that'd be soon, right?), we can add e6500 back to the list. -Scott -- 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
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On Fri, 2013-05-10 at 22:12 +0800, Kevin Hao wrote: So I would assume you will not pick up these two patches, right? http://patchwork.ozlabs.org/patch/235530/ http://patchwork.ozlabs.org/patch/235532/ Anyway it is more easier to enable the external proxy by using this method. But if you insist, I can respin a patch to use the method you suggested since it will definitely reduce the window where the irq is hard disabled. I would keep the EE_EDGE bit definition. I have no objection to a gradual approach however for the other one where we apply it as is now to enable coreint while you do a rework to make it better :-) Cheers, Ben. -- 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
Re: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
On Sat, 2013-05-11 at 07:49 +1000, Benjamin Herrenschmidt wrote: I would keep the EE_EDGE bit definition. I have no objection to a gradual approach however for the other one where we apply it as is now to enable coreint while you do a rework to make it better :-) Note also that I generally don't apply FSL related patches directly, I rely on Kumar Gala picking them up so he's the one ultimately making that choice. Cheers, Ben. -- 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
Re: [PATCH v2 2/4] kvm/ppc/booke64: Fix lazy ee handling in kvmppc_handle_exit()
On 05/10/2013 12:01:19 AM, Bhushan Bharat-R65777 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Scott Wood Sent: Friday, May 10, 2013 8:40 AM To: Alexander Graf; Benjamin Herrenschmidt Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc-...@lists.ozlabs.org; Wood Scott-B07421 Subject: [PATCH v2 2/4] kvm/ppc/booke64: Fix lazy ee handling in kvmppc_handle_exit() EE is hard-disabled on entry to kvmppc_handle_exit(), so call hard_irq_disable() so that PACA_IRQ_HARD_DIS is set, and soft_enabled is unset. Without this, we get warnings such as arch/powerpc/kernel/time.c:300, and sometimes host kernel hangs. Signed-off-by: Scott Wood scottw...@freescale.com --- arch/powerpc/kvm/booke.c |5 + 1 file changed, 5 insertions(+) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 1020119..705fc5c 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -833,6 +833,11 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, int r = RESUME_HOST; int s; +#ifdef CONFIG_PPC64 + WARN_ON(local_paca-irq_happened != 0); +#endif + hard_irq_disable(); It is not actually to hard disable as EE is already clear but to make it looks like hard_disable to host. Right? If so, should we write a comment here on why we are doing this? Yes, I can add a comment. -Scott -- 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
Re: [PATCH v2 4/4] kvm/ppc: IRQ disabling cleanup
On 05/10/2013 12:01:38 AM, Bhushan Bharat-R65777 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Scott Wood Sent: Friday, May 10, 2013 8:40 AM To: Alexander Graf; Benjamin Herrenschmidt Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc-...@lists.ozlabs.org; Wood Scott-B07421 Subject: [PATCH v2 4/4] kvm/ppc: IRQ disabling cleanup - WARN_ON_ONCE(!irqs_disabled()); + WARN_ON(irqs_disabled()); + hard_irq_disable(); Here we hard disable in kvmppc_prepare_to_enter(), so my comment in other patch about interrupt loss is no more valid. So here MSR.EE = 0 local_paca-soft_enabled = 0 local_paca-irq_happened |= PACA_IRQ_HARD_DIS; + while (true) { if (need_resched()) { local_irq_enable(); This will make the state: MSR.EE = 1 local_paca-soft_enabled = 1 local_paca-irq_happened = PACA_IRQ_HARD_DIS; //same as before Is that a valid state where interrupts are fully enabled and irq_happend in not 0? PACA_IRQ_HARD_DIS will have been cleared by local_irq_enable(), as Tiejun pointed out. int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu) { int r = 0; WARN_ON_ONCE(!irqs_disabled()); kvmppc_core_check_exceptions(vcpu); if (vcpu-requests) { /* Exception delivery raised request; start over */ return 1; } if (vcpu-arch.shared-msr MSR_WE) { local_irq_enable(); kvm_vcpu_block(vcpu); clear_bit(KVM_REQ_UNHALT, vcpu-requests); local_irq_disable(); ^^^ We do not require hard_irq_disable() here? Yes, that should be changed to hard_irq_disable(), and I'll add a WARN_ON to double check that interrupts are hard-disabled (eventually we'll probably want to make these critical-path assertions dependent on a debug option...). It doesn't really matter all that much, though, since we don't have MSR_WE on any 64-bit booke chips. :-) -Scott -- 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