Re: [PATCH] kvm-kmod: Various compat fixes for older kernels
Jan Kiszka wrote: Various fixes that were required to build against a 2.6.18 kernel, but some affect newer kernels, too: - replacements for uaccess.h and relay.h - flush_work compat wrapper - fix msi_enabled hack - hack eventfd.c for INIT_WORK Please split into separate patches. - move phys_addr_t and true/false definitions as headers require it earlier - add MSR_K7_HWCR definition I had these two already, but not pushed. Maybe I need to have a script push my master branch somewhere, so you don't duplicate my work needlessly. -- error compiling committee.c: too many arguments to function -- 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 3/3] kvm-s390: streamline memslot handling
Christian Ehrhardt wrote: There already exists a loop which does this, see make_all_cpus_request(). It uses an IPI (Marcelo, can't it use the reschedule interrupt?). It has a couple of optimizations -- if the request is already set, it skips the IPI, and it avoids the IPI for vcpus out of guest mode. Maybe it could fit s390 too. I assume that the IPI on x86 is a implicit consequence of the smp_call_function_many function, Yes. It's only used to exit the guest, the IPI itself does nothing. but I think this doesn't work that way for us. The kick implied by that call would be recieved, but not reach the code the checke vcpu->request. vcpu->requests is not checked by the IPI. Instead, it is checked just before entering guest mode, with interrupts disabled. If the request is made before the check, no IPI is made, and the check finds the bit set. If the request is made after the check, an IPI is made, and the guest exits immediately after entry. I could add that behaviour, but that could make our normal interrupt handling much slower. Therefore I don't want to call that function, but on the other hand I like the "skip if the request is already set" functionality and think about adding that in my loop. I don't understand why it would affect your interrupt handling. We need someone that talks both x86 and s390 to break the language barrier... I'll apply the patches, but please do look further into increasing commonality. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] kvm-s390: infrastructure to kick vcpus out of guest state
Marcelo Tosatti wrote: On Mon, May 25, 2009 at 01:40:49PM +0200, ehrha...@linux.vnet.ibm.com wrote: From: Christian Ehrhardt To ensure vcpu's come out of guest context in certain cases this patch adds a s390 specific way to kick them out of guest context. Currently it kicks them out to rerun the vcpu_run path in the s390 code, but the mechanism itself is expandable and with a new flag we could also add e.g. kicks to userspace etc. Signed-off-by: Christian Ehrhardt "For now I added the optimization to skip kicking vcpus out of guest that had the request bit already set to the s390 specific loop (sent as v2 in a few minutes). We might one day consider standardizing some generic kickout levels e.g. kick to "inner loop", "arch vcpu run", "generic vcpu run", "userspace", ... whatever levels fit *all* our use cases. And then let that kicks be implemented in an kvm_arch_* backend as it might be very different how they behave on different architectures." That would be ideal, yes. Two things make_all_requests handles: 1) It disables preemption with get_cpu(), so it can reliably check for cpu id. Somehow you don't need that for s390 when kicking multiple vcpus? I don't even need the cpuid as make_all_requests does, I just insert a special bit in the vcpu arch part and the vcpu will "come out to me (host)". Fortunateley the kick is rare and fast so I can just insert it unconditionally (it's even ok to insert it if the vcpu is not in guest state). That prevents us from needing vcpu lock or detailed checks which would end up where we started (no guarantee that vcpu's come out of guest context while trying to aquire all vcpu locks) 2) It uses smp_call_function_many(wait=1), which guarantees that by the time make_all_requests returns no vcpus will be using stale data (the remote vcpus will have executed ack_flush). yes this is really a part my s390 implementation doesn't fulfill yet. Currently on return vcpus might still use the old memslot information. As mentioned before letting all interrupts come "too far" out of the hot loop would be a performance issue, therefore I think I will need some request&confirm mechanism. I'm not sure yet but maybe it could be as easy as this pseudo code example: # in make_all_requests # remember we have slots_lock write here and the reentry that updates the vcpu specific data aquires slots_lock for read. loop vcpus set_bit in vcpu requests kick vcpu #arch function endloop loop vcpus until the requested bit is disappeared #as the reentry path uses test_and_clear it will disappear endloop That would be a implicit synchronization and should work, as I wrote before setting memslots while the guest is running is rare if ever existant for s390. On x86 smp_call_many could then work without the wait flag being set. But I assume that this synchronization approach is slower as it serializes all vcpus on reentry (they wait for the slots_lock to get dropped), therefore I wanted to ask how often setting memslots on runtime will occur on x86 ? Would this approach be acceptable ? If it is too adventurous for now I can implement it that way in the s390 code and we split the long term discussion (synchronization + generic kickout levels + who knows what comes up). If smp_call_function_many is hidden behind kvm_arch_kick_vcpus, can you make use of make_all_requests for S390 (without the smp_call_function performance impact you mentioned) ? In combination with the request&confirm mechanism desribed above it should work if smp_call function and all the cpuid gathering which belongs to it is hidden behind kvm_arch_kick_vcpus. For x86 we can further optimize make_all_requests by checking REQ_KICK, and kvm_arch_kick_vcpus would be a good place for that. And the kickout levels idea you mentioned can come later, as an optimization? yes I agree splitting that to a later optimization is a good idea. -- 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 -- Grüsse / regards, Christian Ehrhardt IBM Linux Technology Center, Open Virtualization -- 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: 64bit guest thinks it is 32bit in kvm-86 - Intel only
Josh Wilsdon wrote: the kernel refuses to boot with the error message: "This kernel requires an x86-64 CPU, but only detected an i686 CPU. Unable to boot - please use a kernel appropriate for your CPU." This should fix it: http://git.kernel.org/?p=virt/kvm/qemu-kvm.git;a=commitdiff;h=8fa3b3ce6e Yes, that fixed it! Thanks! Out of curiosity, is there a reason that this was not included in the kvm-86 release, even though the date on the commit is May 12? I think it's the authorship date, not the commit date. -- error compiling committee.c: too many arguments to function -- 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] kvmtrace_format: read /usr/share/kvm/formats by default
Eduardo Habkost wrote: This will help distributions to package kvmtrace and kvmtrace_format, making kvmtrace_format work out-of-the-box. The directory is defined at the beginning of the script to make it easier for distributions to customize the path at install-time, depending on packaging parameters. There are no install rules to install kvmtrace and kvmtrace_format, but it would be interesting to add such rules later. They could replace the PREFIX definition with the prefix set by ./configure. Applied, thanks. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 04/11] qemu: helper routines for pci access.
Michael S. Tsirkin wrote: On Tue, May 26, 2009 at 11:33:37AM +0900, Isaku Yamahata wrote: On Mon, May 25, 2009 at 03:25:33PM +0300, Michael S. Tsirkin wrote: Add inline routines for convenient access to pci devices with correct (little) endianness. Will be used by MSI-X support. Just a minor comment. How about to add pci_[sg]et_byte() for consistency? I don't see that it makes sense - pci_set_long(config, value) is shorter than *((uint32_t *)config) = cpu_to_le32(value), but single bytes don't have endianness, and *config = value is shorter. It's nice to have consistent APIs though. -- error compiling committee.c: too many arguments to function -- 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] KVM: MCE: Add MCE support to KVM
Jan Kiszka wrote: Well, if I look at the definition of MCI_STATUS_VAL as (1UL<<63), something tells me: "Hey, only use me on 64-bit hosts!" But I have no clue about details of this stuff, and from a second glance at it is seems to include at least some parts that are valid on 32-bit as well. Ah, interesting: /someone/ removed the #ifdef __x86_64__ from arch/x86/include/asm/mce.h, but that's not mainline yet... Well that someone missed the MCI_STATUS_VAL bit. Do you see anything else fishy in there? -- error compiling committee.c: too many arguments to function -- 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 RFC v2 4/4] Use macro to iterate over vcpus.
Gleb Natapov wrote: void kvm_arch_sync_events(struct kvm *kvm) diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 0934df3..34634e0 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -170,6 +170,17 @@ struct kvm { #define kvm_printf(kvm, fmt ...) printk(KERN_DEBUG fmt) #define vcpu_printf(vcpu, fmt...) kvm_printf(vcpu->kvm, fmt) +static inline struct kvm_vcpu *kvm_get_vcpu(struct kvm *kvm, int i) +{ + smp_rmb(); +return kvm->vcpus[i]; +} Whitespace. + +#define kvm_for_each_vcpu(idx, vcpup, kvm) \ + for (idx = 0, vcpup = kvm_get_vcpu(kvm, idx); \ +idx < atomic_read(&kvm->online_vcpus) && vcpup; \ +vcpup = kvm_get_vcpu(kvm, ++idx)) + This will stop when reaching the first empty vcpu slot, while current code skips over empty slots. Please preserve existing behaviour. -- error compiling committee.c: too many arguments to function -- 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-kmod: Various compat fixes for older kernels
On (Tue) May 26 2009 [10:49:35], Avi Kivity wrote: > I had these two already, but not pushed. Maybe I need to have a script > push my master branch somewhere, so you don't duplicate my work > needlessly. A 'next' branch that's the real bleeding-edge, while the patches get tested and master gets pushed out? Amit -- 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 RFC v2 2/4] Use pointer to vcpu instead of vcpu_id in timer code.
Gleb Natapov wrote: Signed-off-by: Gleb Natapov --- arch/x86/kvm/i8254.c |2 +- arch/x86/kvm/kvm_timer.h |2 +- arch/x86/kvm/lapic.c |2 +- arch/x86/kvm/timer.c |2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c index bcf755f..85d95ff 100644 --- a/arch/x86/kvm/i8254.c +++ b/arch/x86/kvm/i8254.c @@ -291,7 +291,7 @@ static void create_pit_timer(struct kvm_kpit_state *ps, u32 val, int is_period) pt->timer.function = kvm_timer_fn; pt->t_ops = &kpit_ops; pt->kvm = ps->pit->kvm; - pt->vcpu_id = 0; + pt->vcpu = pt->kvm->bsp_vcpu; bsp_vcpu might not have been initialized at this time? -- error compiling committee.c: too many arguments to function -- 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: Need a new plan on adding kvm support to qemu
Avi Kivity wrote: While I appreciate the efforts to add clean qemu/kvm integration in upstream, it's driving me mad. Every merge I'm faced with regressions (mostly due to changing code, not to upstream breakage) and need to fix things. Work done during merges is very likely to contain errors since there's no incremental change to review and test. We now have two very different kvm implementations: The upstream implementation: - mostly clean - crippled - no smp - no kernel irqchip - no kernel pit - no device assignment - reduced support for older host kernels - no ia64 support - no nmi support - no tpr patching - almost totally untested (small userbase) The downstream implementation: - a tangled mess - featureful and performant #include - heavily tested (kvm-autotest, large kvm userbase, inclusion in distros) What we have here is the classic rewrite fallacy (rewriting from scratch is easier than fixing, now six months into the voyage with no end in sight) coupled with inflicting pain to the largest contributor to qemu (I mean the kvm community, not me personally). Meanwhile, new kvm kernel features need to be implemented twice in userspace. I propose that we stop this. It is fragmenting the development effort, causing regressions (again, mostly through merge issues, but also through new code duplicating old code incorrectly), and not really helping upstream qemu. It's also the first case I've heard of where a project competes with itself (well not really but it's a nice soundbite). As a first step I've merged a copy of libkvm into qemu linkage (i.e. not as a library), so we can start to use the upstream code (say, use kvm_ioctl() from libkvm.c). It's not going to be easy, but what we have now isn't working. I'm with you. I think at this point we should try to make the qemu-kvm code use the upstream QEMU code where ever possible and then refactor the qemu-kvm into a mergable state. A good second step would be getting rid of libkvm entirely. Regards, Anthony Liguori -- 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 3/3] kvm-s390: streamline memslot handling
Am Dienstag 26 Mai 2009 09:57:58 schrieb Avi Kivity: > > I could add that behaviour, but that could make our normal interrupt > > handling much slower. Therefore I don't want to call that function, > > but on the other hand I like the "skip if the request is already set" > > functionality and think about adding that in my loop. > > I don't understand why it would affect your interrupt handling. We need As far as I understand x86, every host interrupt causes a guest exit. On s390 the SIE instruction is interruptible. On a host interrupt (like an IPI) the host interrupt handler runs and finally jumps back into the SIE instruction. The hardware will continue with guest execution. This has the advantage, that we dont have to load/save guest and host registers on host interrupts. (the low level interrupt handler saves the registers of the interrupted context) In our low-level interrupt handler we do check for signal_pending, machine_check_pending and need_resched to leave the sie instruction. For anything else a the host sees a cpu bound guest always in the SIE instruction. Christian -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
context switches and responsiveness
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 Hi, I've recently increased the number of guests (+20%) running on one of my hosts and found that the responsiveness suffered. Before that, the context switches were hovering around 10K, now they're close to 30K. Or this could just be because I upgraded the host kernel and kvm: host and guests are running 2.6.29.4 + kvm-86 Is there anything that can be done to reduce this? I thought dynticks would prevent unecessary context switching? As you can see from the dstat sample below, the host can be very quiet at times, and yet the guests are a bit sluggish. (there is no swapping going on in the guests either) Is 30K context switches normal for a host that's mostly idle?? Here is a sample guest command line: - -clock dynticks -usbdevice tablet -m 1024 -monitor telnet:127.0.0.1:10005,server,nowait -L ./ -kernel ./bzImage-2.6.29.4 - -append earlyprintk=serial,ttyS0,115200,keep console=ttyS0,115200 root=/dev/vda elevator=noop -nographic -drive file=./vm/root_fs,if=virtio,boot=on -net nic,vlan=14,macaddr=fe:f0:00:00:00:0e,model=virtio -net tap,vlan=14,ifname=tap14,script=no Cheers Antoine - total-cpu-usage -dsk/total- -net/total- ---paging-- ---system-- usr sys idl wai hiq siq| read writ| recv send| in out | int csw 14 15 71 0 0 0| 0 0 | 942B 6912B| 0 0 | 20k 29k 11 17 71 0 0 0| 0 160k| 460B 818B| 0 0 | 19k 30k 9 20 71 0 0 0| 0 0 | 270B 567B| 0 0 | 20k 29k 17 16 66 1 0 0|8192B 1776k| 548B 876B| 0 0 | 20k 31k 8 17 75 0 0 0| 0 336k| 927B 1291B| 0 0 | 20k 29k 13 16 71 0 0 0| 0 1088k|1422B 8505B| 0 296k| 20k 31k 23 14 63 0 0 0| 0 0 |3918B 3058B| 0 0 | 19k 33k 11 16 71 0 0 0|8192B 1120k| 132B 534B| 0 0 | 19k 30k 16 18 66 0 0 0| 032k| 330B 7662B| 0 0 | 19k 30k 13 18 69 0 0 0| 0 128k| 264B 420B| 0 0 | 19k 31k 16 17 66 0 0 0| 0 240k| 474B 420B| 0 0 | 19k 30k 11 21 68 0 0 0| 0 1648k| 372B 4196B| 0 0 | 20k 32k 13 18 69 0 0 0| 096k| 708B 500B| 0 0 | 20k 30k 15 19 66 0 0 0| 0 256k| 66B 322B| 0 0 | 20k 30k 15 22 63 0 0 0| 032k| 804B 8906B| 0 0 | 20k 31k 14 19 67 0 0 0|8192B 256k| 838B 590B|4096B0 | 19k 31k 13 16 71 0 0 0| 0 0 | 132B 534B| 0 0 | 20k 31k 16 18 62 4 0 0| 312k 976k| 396B 7872B| 156k0 | 20k 32k 13 22 65 0 0 0| 0 352k| 573B 534B| 0 0 | 19k 31k 14 20 67 0 0 0| 0 160k| 304B 696B| 0 0 | 19k 31k 12 12 74 0 1 0| 0 512k| 514B 4268B| 0 0 | 19k 31k 10 19 72 0 0 0| 0 152k| 290B 420B| 0 0 | 19k 31k 14 18 67 2 0 0| 16k 232k| 224B 306B| 0 0 | 19k 31k 20 23 57 0 0 0| 080k|1898B 5544B| 0 0 | 20k 32k 14 14 72 0 0 0| 0 112k|2286B 2482B| 0 0 | 20k 32k 18 17 65 0 0 0| 0 176k|1057B 1169B| 0 0 | 20k 31k 15 13 72 0 0 0| 0 144k| 422B 7614B| 0 0 | 19k 31k 11 15 72 0 0 0| 0 944k| 448B 436B| 0 0 | 19k 31k 9 18 73 0 0 0| 0 1584k| 266B 348B| 0 0 | 19k 33k 15 17 68 0 0 0| 0 0 | 721B 7646B| 0 0 | 20k 31k 14 20 66 0 0 0| 0 0 | 356B 420B| 0 0 | 20k 31k 16 15 69 0 0 0| 0 432k| 532B 420B| 0 0 | 19k 31k 14 18 68 0 0 0| 0 144k| 628B 4058B| 0 0 | 19k 30k 10 20 71 0 0 0| 0 160k| 316B 486B| 0 0 | 20k 31k 11 17 72 0 0 0| 080k| 266B 462B| 0 0 | 20k 31k -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEAREKAAYFAkobqSwACgkQGK2zHPGK1rumkgCfVkBuS45ouLCCPXV2xSUjeTUd 9FkAnjxl5sufeL+aYUZsuw4sHLEN7UMt =wAwM -END PGP SIGNATURE- -- 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 RFC v2 3/4] Break dependency between vcpu index in vcpus array and vcpu_id.
Gleb Natapov wrote: Archs are free to use vcpu_id as they see fit. For x86 it is used as vcpu's apic id. You need a KVM_CAP to inform userspace that the vcpu id has changed meaning. inline int kvm_is_mmio_pfn(pfn_t pfn) { if (pfn_valid(pfn)) { @@ -1713,15 +1708,12 @@ static int create_vcpu_fd(struct kvm_vcpu *vcpu) /* * Creates some virtual cpus. Good luck creating more than one. */ -static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, int n) +static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id) { int r; struct kvm_vcpu *vcpu; - if (!valid_vcpu(n)) - return -EINVAL; - - vcpu = kvm_arch_vcpu_create(kvm, n); + vcpu = kvm_arch_vcpu_create(kvm, id); if (IS_ERR(vcpu)) return PTR_ERR(vcpu); @@ -1732,25 +1724,36 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, int n) return r; mutex_lock(&kvm->lock); - if (kvm->vcpus[n]) { - r = -EEXIST; + if (atomic_read(&kvm->online_vcpus) == KVM_MAX_VCPUS) { + r = -EINVAL; goto vcpu_destroy; } - kvm->vcpus[n] = vcpu; - if (n == 0) - kvm->bsp_vcpu = vcpu; - mutex_unlock(&kvm->lock); + + for (r = 0; r < atomic_read(&kvm->online_vcpus); r++) + if (kvm->vcpus[r]->vcpu_id == id) { + r = -EEXIST; + goto vcpu_destroy; + } + + BUG_ON(kvm->vcpus[atomic_read(&kvm->online_vcpus)]); /* Now it's all set up, let userspace reach it */ kvm_get_kvm(kvm); r = create_vcpu_fd(vcpu); - if (r < 0) - goto unlink; + if (r < 0) { + kvm_put_kvm(kvm); + goto vcpu_destroy; + } + + kvm->vcpus[atomic_read(&kvm->online_vcpus)] = vcpu; + smp_wmb(); + atomic_inc(&kvm->online_vcpus); + + if (id == 0) + kvm->bsp_vcpu = vcpu; + mutex_unlock(&kvm->lock); return r; -unlink: - mutex_lock(&kvm->lock); - kvm->vcpus[n] = NULL; vcpu_destroy: mutex_unlock(&kvm->lock); kvm_arch_vcpu_destroy(vcpu); Don't the vcpu ioctls need to be updated? They get the vcpu id as a parameter. -- error compiling committee.c: too many arguments to function -- 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] qemu: virtio save/load bindings
Michael S. Tsirkin wrote: Implement bindings for virtio save/load. Use them in virtio pci. Signed-off-by: Michael S. Tsirkin --- Is anyone working to fill in load/save bindings so that saving virtio devices works? Here's a trivial patch to do this (this one is on top of my MSI-X patchset). Comments? hw/virtio-pci.c | 49 - hw/virtio.c | 31 ++- hw/virtio.h |4 3 files changed, 66 insertions(+), 18 deletions(-) static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, diff --git a/hw/virtio.c b/hw/virtio.c index 63ffcff..b773dff 100644 --- a/hw/virtio.c +++ b/hw/virtio.c @@ -568,9 +568,8 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) { int i; -/* FIXME: load/save binding. */ -//pci_device_save(&vdev->pci_dev, f); -//msix_save(&vdev->pci_dev, f); qdev regressed save/restore? What else is broken right now from the qdev commit? I'm beginning to think committing in the state it was in was a mistake. Paul, can you put together a TODO so that we know all of the things that have regressed so we can get things back into shape? Regards, Anthony Liguori -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [KVM-AUTOTEST PATCH] kvm_utils.py: remote_login(): improve regular expression matching
On Mon, May 25, 2009 at 2:15 PM, Michael Goldish wrote: > > - "sudhir kumar" wrote: > >> The patch looks sane to me. A very good thing that can be done for >> remote_login() is to tune the tmeouts. I have seen especialy with >> windows guests or sometimes when the machine is heavily loaded the >> timeouts elapse and the test fails. When I increased the timeouts the >> test did not fail. internal_timeout=0.5 is too less in my views and >> even timeouts of 10 seconds prove insufficient sometimes. Do you too >> have any such experience? > > Yes, and I have a patch to fix the problem, which depends on another > patch that isn't ready yet... > > Comments: > > 1. internal_timeout has nothing to do with this -- it controls the time > duration read_nonblocking() waits until it decides there's no more output > to read and returns. 0.5 is high enough in my opinion. Increasing > internal_timeout leads to more robust prompt recognition. Decreasing it > makes all related functions return sooner and thus increases overall > performance (slightly). I think even 0.1 is a reasonable value. I noticed changing internal_timeout from 0.5 to 1.0 caused my test to pass for windows guest. > > 2. My solution to the prompt timeout problem (which isn't a very common > problem AFAIK) is not to make the timeouts configurable -- instead I use > 2 timeouts everywhere: an "initial output" timeout, and a "further output" > timeout. The first timeout (typically 10 sec) expires if the guest hasn't > responded to the SSH login request. Then the second timeout (typically 30 > sec) expires if there's no additional output. I think this makes sense > because it usually doesn't take very long to get a password prompt or an > "Are you sure" prompt. It can take a while to get the things that follow > (a shell prompt). If we got some initial output it's likely that the guest > will provide more, so we can afford to wait 30 seconds. We can make the 2 > timeouts configurable, but even fixing them at 10 and 30 will probably work > well enough. That looks ok. Though if we are going to use the two timeout values everywhere then i do not think there is any harm in making it configurable. We can keep two parameters in the config file(provided we are going to use only 2 timeouts with fixed values), give them some default value(in function or even in config file), and let the user have little bit of control. Think of the scenario when one wants to stress the system and hence a test failure because of a timeout is never ever expected. In case there are any complications in providing the config variables please let me too know. Please post the patch. i would like to test it and try to make the timeouts as config variables. > >> On Sun, May 24, 2009 at 9:16 PM, Michael Goldish >> wrote: >> > 1. Make the 'login:' regular expression stricter so it doesn't >> match >> > 'Last login: ...' messages. >> > 2. Make the 'password:' regular expression stricter. >> > 3. Handle 'Connection refused' messages. >> > >> > Signed-off-by: Michael Goldish >> > --- >> > client/tests/kvm_runtest_2/kvm_utils.py | 13 + >> > 1 files changed, 9 insertions(+), 4 deletions(-) >> > >> > diff --git a/client/tests/kvm_runtest_2/kvm_utils.py >> b/client/tests/kvm_runtest_2/kvm_utils.py >> > index be8ad95..5736cf6 100644 >> > --- a/client/tests/kvm_runtest_2/kvm_utils.py >> > +++ b/client/tests/kvm_runtest_2/kvm_utils.py >> > @@ -413,7 +413,8 @@ def remote_login(command, password, prompt, >> linesep="\n", timeout=10): >> > >> > while True: >> > (match, text) = sub.read_until_last_line_matches( >> > - ["[Aa]re you sure", "[Pp]assword:", "[Ll]ogin:", >> "[Cc]onnection.*closed", prompt], >> > + [r"[Aa]re you sure", r"[Pp]assword:\s*$", >> r"^\s*[Ll]ogin:\s*$", >> > + r"[Cc]onnection.*closed", >> r"[Cc]onnection.*refused", prompt], >> > timeout=timeout, internal_timeout=0.5) >> > if match == 0: # "Are you sure you want to continue >> connecting" >> > kvm_log.debug("Got 'Are you sure...'; sending 'yes'") >> > @@ -437,11 +438,15 @@ def remote_login(command, password, prompt, >> linesep="\n", timeout=10): >> > kvm_log.debug("Got 'Connection closed'") >> > sub.close() >> > return None >> > - elif match == 4: # prompt >> > + elif match == 4: # "Connection refused" >> > + kvm_log.debug("Got 'Connection refused'") >> > + sub.close() >> > + return None >> > + elif match == 5: # prompt >> > kvm_log.debug("Got shell prompt -- logged in") >> > return sub >> > else: # match == None >> > - kvm_log.debug("Timeout or process terminated") >> > + kvm_log.debug("Timeout elapsed or process terminated") >> > sub.close() >> > return None >> > >> > @@ -470,7 +475,7 @@ def remote_scp(command, password, timeout=300, >> login_timeou
Re: [PATCH RFC v2 4/4] Use macro to iterate over vcpus.
On Tue, May 26, 2009 at 11:18:11AM +0300, Avi Kivity wrote: > Gleb Natapov wrote: >> void kvm_arch_sync_events(struct kvm *kvm) >> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h >> index 0934df3..34634e0 100644 >> --- a/include/linux/kvm_host.h >> +++ b/include/linux/kvm_host.h >> @@ -170,6 +170,17 @@ struct kvm { >> #define kvm_printf(kvm, fmt ...) printk(KERN_DEBUG fmt) >> #define vcpu_printf(vcpu, fmt...) kvm_printf(vcpu->kvm, fmt) >> +static inline struct kvm_vcpu *kvm_get_vcpu(struct kvm *kvm, int i) >> +{ >> +smp_rmb(); >> +return kvm->vcpus[i]; >> +} >> > > Whitespace. > Sorry. Forgot to run checkpatch.pl >> + >> +#define kvm_for_each_vcpu(idx, vcpup, kvm) \ >> +for (idx = 0, vcpup = kvm_get_vcpu(kvm, idx); \ >> + idx < atomic_read(&kvm->online_vcpus) && vcpup; \ >> + vcpup = kvm_get_vcpu(kvm, ++idx)) >> + >> > This will stop when reaching the first empty vcpu slot, while current > code skips over empty slots. Please preserve existing behaviour. > That's the idea, there is no more empty slots in vcpus array. Otherwise we always need to iterate over MAX_CPUS and not online_vcpus. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 03/11] qemu: add routines to manage PCI capabilities
On Mon, May 25, 2009 at 03:25:20PM +0300, Michael S. Tsirkin wrote: > Add routines to manage PCI capability list. First user will be MSI-X. > > Signed-off-by: Michael S. Tsirkin > --- > hw/pci.c | 98 > -- > hw/pci.h | 18 +++- > 2 files changed, 106 insertions(+), 10 deletions(-) > > diff --git a/hw/pci.c b/hw/pci.c > index 5dcfb4e..6bc3819 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -130,12 +130,13 @@ void pci_device_save(PCIDevice *s, QEMUFile *f) > int version = s->cap_present ? 3 : 2; > int i; > > -qemu_put_be32(f, version); /* PCI device version */ > +/* PCI device version and capabilities */ > +qemu_put_be32(f, version); > +if (version >= 3) > +qemu_put_be32(f, s->cap_present); > qemu_put_buffer(f, s->config, 256); > for (i = 0; i < 4; i++) > qemu_put_be32(f, s->irq_state[i]); > -if (version >= 3) > -qemu_put_be32(f, s->cap_present); > } > > int pci_device_load(PCIDevice *s, QEMUFile *f) > @@ -146,12 +147,6 @@ int pci_device_load(PCIDevice *s, QEMUFile *f) > version_id = qemu_get_be32(f); > if (version_id > 3) > return -EINVAL; > -qemu_get_buffer(f, s->config, 256); > -pci_update_mappings(s); > - > -if (version_id >= 2) > -for (i = 0; i < 4; i ++) > -s->irq_state[i] = qemu_get_be32(f); > if (version_id >= 3) > s->cap_present = qemu_get_be32(f); > else > @@ -160,6 +155,18 @@ int pci_device_load(PCIDevice *s, QEMUFile *f) > if (s->cap_present & ~s->cap_supported) > return -EINVAL; > > +qemu_get_buffer(f, s->config, 256); > +pci_update_mappings(s); > + > +if (version_id >= 2) > +for (i = 0; i < 4; i ++) > +s->irq_state[i] = qemu_get_be32(f); > +/* Clear mask and used bits for capabilities. > + Must be restored separately, since capabilities can > + be placed anywhere in config space. */ > +memset(s->used, 0, PCI_CONFIG_SPACE_SIZE); > +for (i = PCI_CONFIG_HEADER_SIZE; i < PCI_CONFIG_SPACE_SIZE; ++i) > +s->mask[i] = 0xff; > return 0; > } > > @@ -870,3 +877,76 @@ PCIDevice *pci_create_simple(PCIBus *bus, int devfn, > const char *name) > > return (PCIDevice *)dev; > } > + > +static int pci_find_space(PCIDevice *pdev, uint8_t size) > +{ > +int offset = PCI_CONFIG_HEADER_SIZE; > +int i; > +for (i = PCI_CONFIG_HEADER_SIZE; i < PCI_CONFIG_SPACE_SIZE; ++i) > +if (pdev->used[i]) > +offset = i + 1; > +else if (i - offset + 1 == size) > +return offset; > +return 0; > +} > + > +static uint8_t pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id, > +uint8_t *prev_p) > +{ > +uint8_t next, prev; > + > +if (!(pdev->config[PCI_STATUS] & PCI_STATUS_CAP_LIST)) > +return 0; > + > +for (prev = PCI_CAPABILITY_LIST; (next = pdev->config[prev]); > + prev = next + PCI_CAP_LIST_NEXT) > +if (pdev->config[next + PCI_CAP_LIST_ID] != cap_id) typo? == > +break; > + > +*prev_p = prev; > +return next; > +} > + > +/* Reserve space and add capability to the linked list in pci config space */ > +int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t size) > +{ > +uint8_t offset = pci_find_space(pdev, size); > +uint8_t *config = pdev->config + offset; > +if (!offset) > +return -ENOSPC; > +config[PCI_CAP_LIST_ID] = cap_id; > +config[PCI_CAP_LIST_NEXT] = pdev->config[PCI_CAPABILITY_LIST]; > +pdev->config[PCI_CAPABILITY_LIST] = offset; > +pdev->config[PCI_STATUS] |= PCI_STATUS_CAP_LIST; > +memset(pdev->used + offset, 0xFF, size); > +/* Make capability read-only by default */ > +memset(pdev->mask + offset, 0, size); > +return offset; > +} > + > +/* Unlink capability from the pci config space. */ > +void pci_del_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t size) > +{ > +uint8_t prev, offset = pci_find_capability_list(pdev, cap_id, &prev); > +if (!offset) > +return; > +pdev->config[prev] = pdev->config[offset + PCI_CAP_LIST_NEXT]; > +/* Make capability writeable again */ > +memset(pdev->mask + offset, 0xff, size); > +memset(pdev->used + offset, 0, size); > + > +if (!pdev->config[PCI_CAPABILITY_LIST]) > +pdev->config[PCI_STATUS] &= ~PCI_STATUS_CAP_LIST; > +} > + > +/* Reserve space for capability at a known offset (to call after load). */ > +void pci_reserve_capability(PCIDevice *pdev, uint8_t offset, uint8_t size) > +{ > +memset(pdev->used + offset, 0xff, size); > +} > + > +uint8_t pci_find_capability(PCIDevice *pdev, uint8_t cap_id) > +{ > +uint8_t prev; > +return pci_find_capability_list(pdev, cap_id, &prev); > +} > diff --git a/hw/pci.h b/hw/pci.h > index 9139504..40137c6 100644 > --- a/hw/pci.h > +++ b/hw/pci.h > @@ -123,6 +123,10 @@ typedef struct PCII
Re: [PATCH RFC v2 2/4] Use pointer to vcpu instead of vcpu_id in timer code.
On Tue, May 26, 2009 at 11:30:21AM +0300, Avi Kivity wrote: > Gleb Natapov wrote: >> Signed-off-by: Gleb Natapov >> --- >> arch/x86/kvm/i8254.c |2 +- >> arch/x86/kvm/kvm_timer.h |2 +- >> arch/x86/kvm/lapic.c |2 +- >> arch/x86/kvm/timer.c |2 +- >> 4 files changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c >> index bcf755f..85d95ff 100644 >> --- a/arch/x86/kvm/i8254.c >> +++ b/arch/x86/kvm/i8254.c >> @@ -291,7 +291,7 @@ static void create_pit_timer(struct kvm_kpit_state *ps, >> u32 val, int is_period) >> pt->timer.function = kvm_timer_fn; >> pt->t_ops = &kpit_ops; >> pt->kvm = ps->pit->kvm; >> -pt->vcpu_id = 0; >> +pt->vcpu = pt->kvm->bsp_vcpu; >> >> > > bsp_vcpu might not have been initialized at this time? > There is only two code path that may lead to this code. First one is from pit_ioport_write() and at this stage BSP is already executing code and thus exists. The second is during migration and vcpus are created before incoming migration is accepted. May be BUG_ON() is appropriate here. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 04/11] qemu: helper routines for pci access.
On Tue, May 26, 2009 at 11:07:33AM +0300, Avi Kivity wrote: > Michael S. Tsirkin wrote: >> On Tue, May 26, 2009 at 11:33:37AM +0900, Isaku Yamahata wrote: >> >>> On Mon, May 25, 2009 at 03:25:33PM +0300, Michael S. Tsirkin wrote: >>> Add inline routines for convenient access to pci devices with correct (little) endianness. Will be used by MSI-X support. >>> Just a minor comment. >>> How about to add pci_[sg]et_byte() for consistency? >>> >> >> I don't see that it makes sense - pci_set_long(config, value) >> is shorter than *((uint32_t *)config) = cpu_to_le32(value), >> but single bytes don't have endianness, and *config = value >> is shorter. >> > > It's nice to have consistent APIs though. Well, if enough people feel so ... qemu: add pci_get/set_byte Add pci_get/set_byte to keep *_word and *_long access functions company. They are unused for now. Signed-off-by: Michael S. Tsirkin --- diff --git a/hw/pci.h b/hw/pci.h index 4072f16..e1e4fb4 100644 --- a/hw/pci.h +++ b/hw/pci.h @@ -263,6 +263,18 @@ PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did, pci_map_irq_fn map_irq, const char *name); static inline void +pci_set_byte(uint8_t *config, uint8_t val) +{ +*config = val; +} + +static inline uint8_t +pci_get_byte(uint8_t *config) +{ +return *config; +} + +static inline void pci_set_word(uint8_t *config, uint16_t val) { cpu_to_le16wu((uint16_t *)config, val); -- MST -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v2 3/4] Break dependency between vcpu index in vcpus array and vcpu_id.
On Tue, May 26, 2009 at 11:34:01AM +0300, Avi Kivity wrote: > Gleb Natapov wrote: >> Archs are free to use vcpu_id as they see fit. For x86 it is used as >> vcpu's apic id. >> >> > > You need a KVM_CAP to inform userspace that the vcpu id has changed meaning. > Will add. >> inline int kvm_is_mmio_pfn(pfn_t pfn) >> { >> if (pfn_valid(pfn)) { >> @@ -1713,15 +1708,12 @@ static int create_vcpu_fd(struct kvm_vcpu *vcpu) >> /* >> * Creates some virtual cpus. Good luck creating more than one. >> */ >> -static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, int n) >> +static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id) >> { >> int r; >> struct kvm_vcpu *vcpu; >> - if (!valid_vcpu(n)) >> -return -EINVAL; >> - >> -vcpu = kvm_arch_vcpu_create(kvm, n); >> +vcpu = kvm_arch_vcpu_create(kvm, id); >> if (IS_ERR(vcpu)) >> return PTR_ERR(vcpu); >> @@ -1732,25 +1724,36 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm >> *kvm, int n) >> return r; >> mutex_lock(&kvm->lock); >> -if (kvm->vcpus[n]) { >> -r = -EEXIST; >> +if (atomic_read(&kvm->online_vcpus) == KVM_MAX_VCPUS) { >> +r = -EINVAL; >> goto vcpu_destroy; >> } >> -kvm->vcpus[n] = vcpu; >> -if (n == 0) >> -kvm->bsp_vcpu = vcpu; >> -mutex_unlock(&kvm->lock); >> + >> +for (r = 0; r < atomic_read(&kvm->online_vcpus); r++) >> +if (kvm->vcpus[r]->vcpu_id == id) { >> +r = -EEXIST; >> +goto vcpu_destroy; >> +} >> + >> +BUG_ON(kvm->vcpus[atomic_read(&kvm->online_vcpus)]); >> /* Now it's all set up, let userspace reach it */ >> kvm_get_kvm(kvm); >> r = create_vcpu_fd(vcpu); >> -if (r < 0) >> -goto unlink; >> +if (r < 0) { >> +kvm_put_kvm(kvm); >> +goto vcpu_destroy; >> +} >> + >> +kvm->vcpus[atomic_read(&kvm->online_vcpus)] = vcpu; >> +smp_wmb(); >> +atomic_inc(&kvm->online_vcpus); >> + >> +if (id == 0) >> +kvm->bsp_vcpu = vcpu; >> +mutex_unlock(&kvm->lock); >> return r; >> -unlink: >> -mutex_lock(&kvm->lock); >> -kvm->vcpus[n] = NULL; >> vcpu_destroy: >> mutex_unlock(&kvm->lock); >> kvm_arch_vcpu_destroy(vcpu); >> > > Don't the vcpu ioctls need to be updated? They get the vcpu id as a > parameter. > Are you sure they do? vcpu ioctls are issued on vcpu fd, no need to pass vcpu id as a parameter. -- Gleb. -- 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 RFC v2 4/4] Use macro to iterate over vcpus.
Gleb Natapov wrote: + +#define kvm_for_each_vcpu(idx, vcpup, kvm) \ + for (idx = 0, vcpup = kvm_get_vcpu(kvm, idx); \ +idx < atomic_read(&kvm->online_vcpus) && vcpup; \ +vcpup = kvm_get_vcpu(kvm, ++idx)) + This will stop when reaching the first empty vcpu slot, while current code skips over empty slots. Please preserve existing behaviour. That's the idea, there is no more empty slots in vcpus array. Otherwise we always need to iterate over MAX_CPUS and not online_vcpus. Yes, of course. -- error compiling committee.c: too many arguments to function -- 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 RFC v2 3/4] Break dependency between vcpu index in vcpus array and vcpu_id.
Gleb Natapov wrote: Don't the vcpu ioctls need to be updated? They get the vcpu id as a parameter. Are you sure they do? vcpu ioctls are issued on vcpu fd, no need to pass vcpu id as a parameter. You're right, I was confused with an earlier version of the interface, and with libkvm. Moral: handle-based design = good. -- error compiling committee.c: too many arguments to function -- 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-kmod: Various compat fixes for older kernels
Avi Kivity wrote: > Jan Kiszka wrote: >> Various fixes that were required to build against a 2.6.18 kernel, but >> some affect newer kernels, too: >> >>- replacements for uaccess.h and relay.h >> - flush_work compat wrapper >> - fix msi_enabled hack >> - hack eventfd.c for INIT_WORK >> > > > Please split into separate patches. OK, will do. > > >> - move phys_addr_t and true/false definitions as headers require it >>earlier >> - add MSR_K7_HWCR definition >> >> > > I had these two already, but not pushed. Maybe I need to have a script > push my master branch somewhere, so you don't duplicate my work needlessly. > That would be highly appreciated. There is another MSR-related one pending here (MSR_IA32_TSC). Jan signature.asc Description: OpenPGP digital signature
Re: context switches and responsiveness
Antoine Martin wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA512 Hi, I've recently increased the number of guests (+20%) running on one of my hosts and found that the responsiveness suffered. How many guests? How many vcpus? Before that, the context switches were hovering around 10K, now they're close to 30K. Or this could just be because I upgraded the host kernel and kvm: host and guests are running 2.6.29.4 + kvm-86 Is there anything that can be done to reduce this? I thought dynticks would prevent unecessary context switching? dynticks is the default IIRC. What type guests are you running? If Linux, make sure the guests are dynticks as well. As you can see from the dstat sample below, the host can be very quiet at times, and yet the guests are a bit sluggish. (there is no swapping going on in the guests either) Is 30K context switches normal for a host that's mostly idle?? Depends on guest count! Please provide kvm_stat output. Here is a sample guest command line: - -clock dynticks -usbdevice tablet -m 1024 -monitor telnet:127.0.0.1:10005,server,nowait -L ./ -kernel ./bzImage-2.6.29.4 - -append earlyprintk=serial,ttyS0,115200,keep console=ttyS0,115200 Ah, -usbdevice tablet, interrupt generator from hell. Let it go and you'll see your context switch rate drop. - total-cpu-usage -dsk/total- -net/total- ---paging-- ---system-- usr sys idl wai hiq siq| read writ| recv send| in out | int csw 14 15 71 0 0 0| 0 0 | 942B 6912B| 0 0 | 20k 29k 11 17 71 0 0 0| 0 160k| 460B 818B| 0 0 | 19k 30k Interrupt rate is close to context switch rate, which is good. -- error compiling committee.c: too many arguments to function -- 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-kmod: Various compat fixes for older kernels
Jan Kiszka wrote: Maybe I need to have a script push my master branch somewhere, so you don't duplicate my work needlessly. That would be highly appreciated. I'm thinking how to do it. pushing requires an ssh key, and I don't want to leave one around without a passphrase, which would be required by a cron job. Any ideas? -- error compiling committee.c: too many arguments to function -- 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-kmod: Various compat fixes for older kernels
Avi Kivity wrote: > Jan Kiszka wrote: > > > >>> Maybe I need to have a script >>> push my master branch somewhere, so you don't duplicate my work >>> needlessly. >>> >>> >> >> That would be highly appreciated. >> > > I'm thinking how to do it. pushing requires an ssh key, and I don't > want to leave one around without a passphrase, which would be required > by a cron job. > > Any ideas? > Local commit hook that triggers the push (and ask you for the phrase interactively)? But that also depends on how frequent you would like to publish the tree. Jan -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 03/11] qemu: add routines to manage PCI capabilities
On Tue, May 26, 2009 at 05:49:26PM +0900, Isaku Yamahata wrote: > On Mon, May 25, 2009 at 03:25:20PM +0300, Michael S. Tsirkin wrote: > > Add routines to manage PCI capability list. First user will be MSI-X. > > > > Signed-off-by: Michael S. Tsirkin > > --- > > hw/pci.c | 98 > > -- > > hw/pci.h | 18 +++- > > 2 files changed, 106 insertions(+), 10 deletions(-) > > > > diff --git a/hw/pci.c b/hw/pci.c > > index 5dcfb4e..6bc3819 100644 > > --- a/hw/pci.c > > +++ b/hw/pci.c > > @@ -130,12 +130,13 @@ void pci_device_save(PCIDevice *s, QEMUFile *f) > > int version = s->cap_present ? 3 : 2; > > int i; > > > > -qemu_put_be32(f, version); /* PCI device version */ > > +/* PCI device version and capabilities */ > > +qemu_put_be32(f, version); > > +if (version >= 3) > > +qemu_put_be32(f, s->cap_present); > > qemu_put_buffer(f, s->config, 256); > > for (i = 0; i < 4; i++) > > qemu_put_be32(f, s->irq_state[i]); > > -if (version >= 3) > > -qemu_put_be32(f, s->cap_present); > > } > > > > int pci_device_load(PCIDevice *s, QEMUFile *f) > > @@ -146,12 +147,6 @@ int pci_device_load(PCIDevice *s, QEMUFile *f) > > version_id = qemu_get_be32(f); > > if (version_id > 3) > > return -EINVAL; > > -qemu_get_buffer(f, s->config, 256); > > -pci_update_mappings(s); > > - > > -if (version_id >= 2) > > -for (i = 0; i < 4; i ++) > > -s->irq_state[i] = qemu_get_be32(f); > > if (version_id >= 3) > > s->cap_present = qemu_get_be32(f); > > else > > @@ -160,6 +155,18 @@ int pci_device_load(PCIDevice *s, QEMUFile *f) > > if (s->cap_present & ~s->cap_supported) > > return -EINVAL; > > > > +qemu_get_buffer(f, s->config, 256); > > +pci_update_mappings(s); > > + > > +if (version_id >= 2) > > +for (i = 0; i < 4; i ++) > > +s->irq_state[i] = qemu_get_be32(f); > > +/* Clear mask and used bits for capabilities. > > + Must be restored separately, since capabilities can > > + be placed anywhere in config space. */ > > +memset(s->used, 0, PCI_CONFIG_SPACE_SIZE); > > +for (i = PCI_CONFIG_HEADER_SIZE; i < PCI_CONFIG_SPACE_SIZE; ++i) > > +s->mask[i] = 0xff; > > return 0; > > } > > > > @@ -870,3 +877,76 @@ PCIDevice *pci_create_simple(PCIBus *bus, int devfn, > > const char *name) > > > > return (PCIDevice *)dev; > > } > > + > > +static int pci_find_space(PCIDevice *pdev, uint8_t size) > > +{ > > +int offset = PCI_CONFIG_HEADER_SIZE; > > +int i; > > +for (i = PCI_CONFIG_HEADER_SIZE; i < PCI_CONFIG_SPACE_SIZE; ++i) > > +if (pdev->used[i]) > > +offset = i + 1; > > +else if (i - offset + 1 == size) > > +return offset; > > +return 0; > > +} > > + > > +static uint8_t pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id, > > +uint8_t *prev_p) > > +{ > > +uint8_t next, prev; > > + > > +if (!(pdev->config[PCI_STATUS] & PCI_STATUS_CAP_LIST)) > > +return 0; > > + > > +for (prev = PCI_CAPABILITY_LIST; (next = pdev->config[prev]); > > + prev = next + PCI_CAP_LIST_NEXT) > > +if (pdev->config[next + PCI_CAP_LIST_ID] != cap_id) > > typo? == Fixed. Thanks. > > +break; > > + > > +*prev_p = prev; > > +return next; > > +} > > + > > +/* Reserve space and add capability to the linked list in pci config space > > */ > > +int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t size) > > +{ > > +uint8_t offset = pci_find_space(pdev, size); > > +uint8_t *config = pdev->config + offset; > > +if (!offset) > > +return -ENOSPC; > > +config[PCI_CAP_LIST_ID] = cap_id; > > +config[PCI_CAP_LIST_NEXT] = pdev->config[PCI_CAPABILITY_LIST]; > > +pdev->config[PCI_CAPABILITY_LIST] = offset; > > +pdev->config[PCI_STATUS] |= PCI_STATUS_CAP_LIST; > > +memset(pdev->used + offset, 0xFF, size); > > +/* Make capability read-only by default */ > > +memset(pdev->mask + offset, 0, size); > > +return offset; > > +} > > + > > +/* Unlink capability from the pci config space. */ > > +void pci_del_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t size) > > +{ > > +uint8_t prev, offset = pci_find_capability_list(pdev, cap_id, &prev); > > +if (!offset) > > +return; > > +pdev->config[prev] = pdev->config[offset + PCI_CAP_LIST_NEXT]; > > +/* Make capability writeable again */ > > +memset(pdev->mask + offset, 0xff, size); > > +memset(pdev->used + offset, 0, size); > > + > > +if (!pdev->config[PCI_CAPABILITY_LIST]) > > +pdev->config[PCI_STATUS] &= ~PCI_STATUS_CAP_LIST; > > +} > > + > > +/* Reserve space for capability at a known offset (to call after load). */ > > +void pci_reserve_capability(PCIDevice *pdev, uint8_t offset, uint8_t size) > > +{ >
Re: [PATCH 3/3] kvm-s390: streamline memslot handling
Christian Bornträger wrote: Am Dienstag 26 Mai 2009 09:57:58 schrieb Avi Kivity: I could add that behaviour, but that could make our normal interrupt handling much slower. Therefore I don't want to call that function, but on the other hand I like the "skip if the request is already set" functionality and think about adding that in my loop. I don't understand why it would affect your interrupt handling. We need As far as I understand x86, every host interrupt causes a guest exit. Yes. On s390 the SIE instruction is interruptible. On a host interrupt (like an IPI) the host interrupt handler runs and finally jumps back into the SIE instruction. The hardware will continue with guest execution. This has the advantage, that we dont have to load/save guest and host registers on host interrupts. (the low level interrupt handler saves the registers of the interrupted context) Neat stuff. Wish I had something like that. In our low-level interrupt handler we do check for signal_pending, machine_check_pending and need_resched to leave the sie instruction. For anything else a the host sees a cpu bound guest always in the SIE instruction. Okay, now I understand (and agree with) you multi-level kick thing. Maybe we could do it like so: Interrupt handler (on s390 only) checks vcpu->requests, handles the ones it cans. If bits are still set, it exits to arch loop, which handles the bits it cans. If bits are still set, it exits to the generic code loop, which can finally exit to userspace. Does this fit with s390 hardware? -- error compiling committee.c: too many arguments to function -- 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-kmod: Various compat fixes for older kernels
Jan Kiszka wrote: I'm thinking how to do it. pushing requires an ssh key, and I don't want to leave one around without a passphrase, which would be required by a cron job. Any ideas? Local commit hook that triggers the push (and ask you for the phrase interactively)? But that also depends on how frequent you would like to publish the tree. When I'm working interactively the ssh key is in my keyring, so it would be transparent for me. But the problem is that I sometimes work from home and push into the repository, so there'd be no commit to trigger the hook. (hmm, when I push I also have the kernel.org ssh key in my keyring, and with ssh-agent, maybe it could work remotely) -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] qemu: fix pci_find_capability for multiple caps
pci_find_capability_list has a bug so it'd stop at the first capability. This only happens to work as we only support a single capability (MSI-X). Here's a fix. Found-by: Isaku Yamahata Signed-off-by: Michael S. Tsirkin --- This is a fixup for my patch "qemu: add routines to manage PCI capabilities". hw/pci.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index 19905b9..a63d988 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -918,7 +918,7 @@ static uint8_t pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id, for (prev = PCI_CAPABILITY_LIST; (next = pdev->config[prev]); prev = next + PCI_CAP_LIST_NEXT) -if (pdev->config[next + PCI_CAP_LIST_ID] != cap_id) +if (pdev->config[next + PCI_CAP_LIST_ID] == cap_id) break; *prev_p = prev; -- MST -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: context switches and responsiveness [SOLVED]
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 > Ah, -usbdevice tablet, interrupt generator from hell. Let it go and > you'll see your context switch rate drop. Indeed that solved it! (only kept it for the one windows guest that really needs it) Many many thanks. >> - total-cpu-usage -dsk/total- -net/total- ---paging-- ---system-- >> usr sys idl wai hiq siq| read writ| recv send| in out | int csw >> 14 15 71 0 0 0| 0 0 | 942B 6912B| 0 0 | 20k 29k >> 11 17 71 0 0 0| 0 160k| 460B 818B| 0 0 | 19k 30k New (healthy) dstat with 10 guests running: - total-cpu-usage -dsk/total- -net/total- ---paging-- ---system-- usr sys idl wai hiq siq| read writ| recv send| in out | int csw 2 1 92 5 0 0| 080k| 87k 88k| 0 0 |1222 2567 1 1 98 0 0 0| 0 0 | 89k 89k| 0 0 |1230 2618 What a difference this tablet makes!! Antoine -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEAREKAAYFAkobuFcACgkQGK2zHPGK1rtkKQCeIHwkjl+oue/ijeLKCZElguVd IRUAn3telEboBCeT75zOLZo3yDqsRp0Z =Veq+ -END PGP SIGNATURE- -- 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: context switches and responsiveness [SOLVED]
Antoine Martin wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA512 Ah, -usbdevice tablet, interrupt generator from hell. Let it go and you'll see your context switch rate drop. Indeed that solved it! (only kept it for the one windows guest that really needs it) Many many thanks. Why do you need it? vnc? Switching to a better vnc client (I use vinagre) may allow you to drop it. -- error compiling committee.c: too many arguments to function -- 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: context switches and responsiveness [SOLVED]
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 Avi Kivity wrote: > Antoine Martin wrote: >> -BEGIN PGP SIGNED MESSAGE- >> Hash: SHA512 >> >> >>> Ah, -usbdevice tablet, interrupt generator from hell. Let it go and >>> you'll see your context switch rate drop. >>> >> Indeed that solved it! >> (only kept it for the one windows guest that really needs it) >> Many many thanks. >> > > Why do you need it? vnc? Yes. > Switching to a better vnc client (I use > vinagre) may allow you to drop it. I thought I had tried them all in desperation (mouse going out of sync), I'll try again. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEAREKAAYFAkobuTcACgkQGK2zHPGK1rvI7wCfd0d7DE5ZYnOzu76LjLjc5Kse YlMAn0Z5KPe9FsRKUrk15aLVW++JVOcz =ysC7 -END PGP SIGNATURE- -- 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: Add qemu_send_raw() to vlan.
Gleb Natapov wrote: It gets packet without virtio header and adds it if needed. Allows to inject packets to vlan from outside. To send gracious arp for instance. This is for announce_self(), no? If so, upstream has the same problems? -- error compiling committee.c: too many arguments to function -- 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
Add qemu_send_raw() to vlan.
It gets packet without virtio header and adds it if needed. Allows to inject packets to vlan from outside. To send gracious arp for instance. diff --git a/hw/virtio-net.c b/hw/virtio-net.c index 8e9178d..3c77b99 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -389,7 +389,7 @@ static int iov_fill(struct iovec *iov, int iovcnt, const void *buf, int count) } static int receive_header(VirtIONet *n, struct iovec *iov, int iovcnt, - const void *buf, size_t size, size_t hdr_len) + const void *buf, size_t size, size_t hdr_len, int raw) { struct virtio_net_hdr *hdr = (struct virtio_net_hdr *)iov[0].iov_base; int offset = 0; @@ -399,7 +399,11 @@ static int receive_header(VirtIONet *n, struct iovec *iov, int iovcnt, #ifdef TAP_VNET_HDR if (tap_has_vnet_hdr(n->vc->vlan->first_client)) { -memcpy(hdr, buf, sizeof(*hdr)); +if (!raw) { +memcpy(hdr, buf, sizeof(*hdr)); +} else { +memset(hdr, 0, sizeof(*hdr)); +} offset = sizeof(*hdr); work_around_broken_dhclient(hdr, buf + offset, size - offset); } @@ -452,7 +456,7 @@ static int receive_filter(VirtIONet *n, const uint8_t *buf, int size) return 0; } -static void virtio_net_receive(void *opaque, const uint8_t *buf, int size) +static void virtio_net_receive2(void *opaque, const uint8_t *buf, int size, int raw) { VirtIONet *n = opaque; struct virtio_net_hdr_mrg_rxbuf *mhdr = NULL; @@ -502,7 +506,7 @@ static void virtio_net_receive(void *opaque, const uint8_t *buf, int size) mhdr = (struct virtio_net_hdr_mrg_rxbuf *)sg[0].iov_base; offset += receive_header(n, sg, elem.in_num, - buf + offset, size - offset, hdr_len); + buf + offset, size - offset, hdr_len, raw); total += hdr_len; } @@ -524,6 +528,16 @@ static void virtio_net_receive(void *opaque, const uint8_t *buf, int size) virtio_notify(&n->vdev, n->rx_vq); } +static void virtio_net_receive(void *opaque, const uint8_t *buf, int size) +{ +virtio_net_receive2(opaque, buf, size, 0); +} + +static void virtio_net_receive_raw(void *opaque, const uint8_t *buf, int size) +{ +virtio_net_receive2(opaque, buf, size, 1); +} + /* TX */ static void virtio_net_flush_tx(VirtIONet *n, VirtQueue *vq) { @@ -721,6 +735,7 @@ VirtIODevice *virtio_net_init(DeviceState *dev) virtio_net_can_receive, virtio_net_cleanup, n); n->vc->link_status_changed = virtio_net_set_link_status; +n->vc->fd_read_raw = virtio_net_receive_raw; qemu_format_nic_info_str(n->vc, n->mac); diff --git a/net.c b/net.c index 3dfc728..01e31db 100644 --- a/net.c +++ b/net.c @@ -456,6 +456,31 @@ int qemu_send_packet(VLANClientState *vc, const uint8_t *buf, int size) return ret; } +void qemu_send_packet_raw(VLANClientState *sender, const uint8_t *buf, int size) +{ +VLANState *vlan = sender->vlan; +VLANClientState *vc; + +if (sender->link_down) +return; + +#ifdef DEBUG_NET +printf("vlan %d send raw:\n", vlan->id); +hex_dump(stdout, buf, size); +#endif +for(vc = vlan->first_client; vc != NULL; vc = vc->next) { +if (vc == sender || vc->link_down) { +continue; +} +if (vc->fd_read_raw) { +vc->fd_read_raw(vc->opaque, buf, size); +} else { +vc->fd_read(vc->opaque, buf, size); +} +} +return; +} + static ssize_t vc_sendv_compat(VLANClientState *vc, const struct iovec *iov, int iovcnt) { @@ -823,6 +848,29 @@ static void tap_receive(void *opaque, const uint8_t *buf, int size) tap_receive_iov(opaque, iov, i); } +static void tap_receive_raw(void *opaque, const uint8_t *buf, int size) +{ +struct iovec iov[2]; +int i = 0; + +#ifdef IFF_VNET_HDR +TAPState *s = opaque; +struct virtio_net_hdr hdr = { 0, }; + +if (s->has_vnet_hdr && s->using_vnet_hdr) { +iov[i].iov_base = &hdr; +iov[i].iov_len = sizeof(hdr); +i++; +} +#endif + +iov[i].iov_base = (char *) buf; +iov[i].iov_len = size; +i++; + +tap_receive_iov(opaque, iov, i); +} + static int tap_can_send(void *opaque) { TAPState *s = opaque; @@ -992,6 +1040,7 @@ static TAPState *net_tap_fd_init(VLANState *vlan, s->vc = qemu_new_vlan_client(vlan, model, name, tap_receive, NULL, tap_cleanup, s); s->vc->fd_readv = tap_receive_iov; +s->vc->fd_read_raw = tap_receive_raw; #ifdef TUNSETOFFLOAD s->vc->set_offload = tap_set_offload; tap_set_offload(s->vc, 0, 0, 0, 0); diff --git a/net.h b/net.h index 931133b..3d0b6f2 100644 --- a/net.h +++ b/net.h @@ -15,6 +15,7 @@ typedef void (SetOffload)(VLANClientState *, int, int, int, int); struct VLANClientStat
Re: context switches and responsiveness [SOLVED]
Antoine Martin wrote: Switching to a better vnc client (I use vinagre) may allow you to drop it. I thought I had tried them all in desperation (mouse going out of sync), I'll try again. Vinagre does mouse capture like SDL. It isn't perfect, but much better than the standard vnc client. -- error compiling committee.c: too many arguments to function -- 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: Add qemu_send_raw() to vlan.
On Tue, May 26, 2009 at 01:05:35PM +0300, Avi Kivity wrote: > Gleb Natapov wrote: >> It gets packet without virtio header and adds it if needed. Allows to >> inject packets to vlan from outside. To send gracious arp for instance. >> > > This is for announce_self(), no? If so, upstream has the same problems? > VNET_HDR is not upstream. -- Gleb. -- 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: Add qemu_send_raw() to vlan.
Gleb Natapov wrote: On Tue, May 26, 2009 at 01:05:35PM +0300, Avi Kivity wrote: Gleb Natapov wrote: It gets packet without virtio header and adds it if needed. Allows to inject packets to vlan from outside. To send gracious arp for instance. This is for announce_self(), no? If so, upstream has the same problems? VNET_HDR is not upstream. Mark, how far are we from that? -- error compiling committee.c: too many arguments to function -- 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] qemu: virtio save/load bindings
> > -/* FIXME: load/save binding. */ > > -//pci_device_save(&vdev->pci_dev, f); > > -//msix_save(&vdev->pci_dev, f); > > qdev regressed save/restore? What else is broken right now from the > qdev commit? > > I'm beginning to think committing in the state it was in was a mistake. > Paul, can you put together a TODO so that we know all of the things that > have regressed so we can get things back into shape? Sorry, this one apparently slipped past. I#'d intended to fix it, but apparently never ported that bit of the patch. Paul -- 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: Add qemu_send_raw() to vlan.
On Tue, 2009-05-26 at 13:11 +0300, Avi Kivity wrote: > Gleb Natapov wrote: > > On Tue, May 26, 2009 at 01:05:35PM +0300, Avi Kivity wrote: > > > >> Gleb Natapov wrote: > >> > >>> It gets packet without virtio header and adds it if needed. Allows to > >>> inject packets to vlan from outside. To send gracious arp for instance. > >>> > >>> > >> This is for announce_self(), no? If so, upstream has the same problems? > >> > >> > > VNET_HDR is not upstream. > > > > > > Mark, how far are we from that? We first need to add support for paired devices (i.e. a tap/virtio-net pair) rather than trying to munge this stuff into the VLAN code. Cheers, Mark. -- 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 3/3] kvm-s390: streamline memslot handling
Avi Kivity wrote: Christian Bornträger wrote: Am Dienstag 26 Mai 2009 09:57:58 schrieb Avi Kivity: [...] In our low-level interrupt handler we do check for signal_pending, machine_check_pending and need_resched to leave the sie instruction. For anything else a the host sees a cpu bound guest always in the SIE instruction. Okay, now I understand (and agree with) you multi-level kick thing. Maybe we could do it like so: Interrupt handler (on s390 only) checks vcpu->requests, handles the ones it cans. If bits are still set, it exits to arch loop, which handles the bits it cans. If bits are still set, it exits to the generic code loop, which can finally exit to userspace. Does this fit with s390 hardware? I like this idea instead of explicitly kicking to an (upper) level to use the lowest kick and exit if not able to handle. I think it should work (no guarantee) and I try to come up with something in the next few days - either a updated patch series or additional discussion input :-). -- Grüsse / regards, Christian Ehrhardt IBM Linux Technology Center, Open Virtualization -- 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: Add qemu_send_raw() to vlan.
On Tue, 2009-05-26 at 13:03 +0300, Gleb Natapov wrote: > It gets packet without virtio header and adds it if needed. Allows to > inject packets to vlan from outside. To send gracious arp for instance. Acked-by: Mark McLoughlin This isn't ideal, but neither is the current VNET_HDR stuff and it does fix a genuine bug. At least it's not changing existing APIs and makes it clear this is a problem we also need to solve when merging VNET_HDR support upstream. (Gleb - it might be worth patching qemu.git to use qemu_send_packet() in announce_self(), that will reduce the delta between the trees) Cheers, Mark. -- 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: Add qemu_send_raw() to vlan.
Mark McLoughlin wrote: On Tue, 2009-05-26 at 13:03 +0300, Gleb Natapov wrote: It gets packet without virtio header and adds it if needed. Allows to inject packets to vlan from outside. To send gracious arp for instance. Acked-by: Mark McLoughlin This isn't ideal, but neither is the current VNET_HDR stuff and it does fix a genuine bug. At least it's not changing existing APIs and makes it clear this is a problem we also need to solve when merging VNET_HDR support upstream. (Gleb - it might be worth patching qemu.git to use qemu_send_packet() in announce_self(), that will reduce the delta between the trees) Okay. Bug Gleb, please add a signoff. -- error compiling committee.c: too many arguments to function -- 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/4] KVM: move coalesced_mmio locking to its own device
Marcelo Tosatti wrote: === --- kvm-irqlock.orig/virt/kvm/coalesced_mmio.c +++ kvm-irqlock/virt/kvm/coalesced_mmio.c @@ -26,9 +26,12 @@ static int coalesced_mmio_in_range(struc if (!is_write) return 0; - /* kvm->lock is taken by the caller and must be not released before - * dev.read/write - */ + /* +* Some other vcpu might be batching data into the ring, +* fallback to userspace. Ordering not our problem. +*/ + if (!atomic_add_unless(&dev->in_use, 1, 1)) + return 0; Ordering with simultaneous writes is indeed not our problem, but the ring may contain ordered writes (even by the same vcpu!) You mean writes by a vcpu should maintain ordering even when that vcpu migrates to a different pcpu? No. Writes by a single vcpu need to preserve their ordering relative to each other. If a write by vcpu A completes before a write by vcpu B starts, then they need to be ordered, since the vcpus may have synchronized in between. Hm, I don't thimk you broke these rules. The smp_wmb in coalesced_write guarantees that. Suggest our own lock here. in_use is basically a homemade lock, better to use the factory made ones which come with a warranty. The first submission had a mutex but was considered unorthodox. Although i agree with your argument made then, i don't see a way around that. Some pseudocode please? Why not use slots_lock to protect the entire iodevice list (rcu one day), and an internal spinlock for coalesced mmio? -- error compiling committee.c: too many arguments to function -- 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: Userspace MSR handling
Alexander Graf wrote: Does it make sense to implement a generic mechanism for handling MSRs in userspace? I imagine a mechanism analogous to PIO, adding a KVM_EXIT_MSR code and a msr type in the kvm_run struct. I'm happy to take a stab at implementing this if no one else is already working on it. I think it's a great idea. I was thinking of doing something similar for ppc's HIDs/SPRs too, so a userspace app can complement the kernel's vcpu support. Also by falling back to userspace all those MSR read/write patches I send wouldn't have to go in-kernel anymore :) I'm wary of this. It spreads the burden of implementing the cpu emulation across the kernel/user boundary. We don't really notice with qemu as userspace, because we have a cpu emulator on both sides, but consider an alternative userspace that only emulates devices and has no cpu emulation support. We want to support that scenario well. Moreover, your patches only stub out those MSRs. As soon as you implement the more interesting bits, you'll find yourself back in the kernel. Agreed. The one thing that always makes my life hard is the default policy on what to do for unknown MSRs. So if I could (by having a userspace fallback) either #GP or do nothing, I'd be able to mimic qemu's behavior more closely depending on what I need. I'm not interested in mimicing qemu, I'm interested in mimicing a real cpu. kvm is not part of qemu. Many (most?) msrs cannot be emulated in userspace. I definitely wouldn't see those approaches conflicting, but rather complementing each other. If your kvm using userspace app needs to act on a user-defined msr, you wouldn't want him to contact reshat to implement an ioctl for rhel5 just for this msr, do you? An msr is a cpu resource. I don't see how you can define a new cpu resource without changing the cpu implementation, which is in the kernel. If they want to communicate with userspace, let them use mmio or pio. Look at the Xen interface. You write to one cpu's MSR, and the cpu writes a page in guest memory. It doesn't fit; a much better interface would have been mmio. I don't want to break layering just for Xen, so I'm trying to find an alternative. -- error compiling committee.c: too many arguments to function -- 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 3/4] KVM: introduce irq_lock, use it to protect ioapic
Marcelo Tosatti wrote: On Sun, May 24, 2009 at 05:10:07PM +0300, Avi Kivity wrote: Marcelo Tosatti wrote: Subject says it all. I hate those changelogs. I guess Subject never reviews code. You might put some evidence that we're suffering from contention here (I'm very willing to believe it, but hard evidence is better). Don't have any evidence yet, the main purpose of the split is to fix the deadlock. You might have said that. But, with the data we have so far, slots_lock and kvm->lock will cause significant cache bouncing on EPT hardware (with measurements on shadow mmu_lock is #1 offender). Well, mmu_lock is probably contended, which is much worse than just cache bouncing. -- error compiling committee.c: too many arguments to function -- 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: Downsize max support MSI-X entry to 256
Sheng Yang wrote: We only trap one page for MSI-X entry now, so it's 4k/(128/8) = 256 entries at most. Applied, thanks. -#define KVM_MAX_MSIX_PER_DEV 512 +#define KVM_MAX_MSIX_PER_DEV 256 struct kvm_assigned_msix_entry { __u32 assigned_dev_id; __u32 gsi; It's really unfortunate that this is a compile-time constant instead of run-time discoverable. -- error compiling committee.c: too many arguments to function -- 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: strange guest slowness after some time
Rusty Russell wrote: On Tuesday 07 April 2009 00:49:17 Tomasz Chmielewski wrote: Tomasz Chmielewski schrieb: As I mentioned, it was using virtio net. Guests running with e1000 (and virtio_blk) don't have this problem. Also, virtio_console seem to be affected by this "slowness" issue. I'm pretty sure this is different. Older virtio_console code ignored interrupts and polled, and use a heuristic to back off on polling (this was because we used the generic "hvc" infrastructure which hacked support). You'll find a delay on the first keystroke after idle, but none on the second. I still observe this "slowness" with kvm-86 after the guest is running for some time (virtio_net and virtio_console seem to be affected; guest restart doesn't fix it). -- Tomasz Chmielewski http://wpkg.org -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/8] kvm-kmod: Include relayfs_fs.h on pre-2.6.17 kernels
Signed-off-by: Jan Kiszka --- external-module-compat-comm.h |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/external-module-compat-comm.h b/external-module-compat-comm.h index e2342db..dad43ef 100644 --- a/external-module-compat-comm.h +++ b/external-module-compat-comm.h @@ -572,7 +572,11 @@ struct pci_dev *pci_get_bus_and_slot(unsigned int bus, unsigned int devfn); #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,21) +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,17) +#include +#else #include +#endif /* relay_open() interface has changed on 2.6.21 */ -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/8] kvm-kmod: Allow to override sync source
In order to allow sync'ing the kmod dir against arbitrary kernels trees, extend the sync script to accept alternative paths and adjust the Makefile accordingly. Changes in v2: - drop KVM_VERSION override - make use of set_defaults - option help texts Signed-off-by: Jan Kiszka --- Makefile |2 +- sync | 16 +++- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 038f512..db44772 100644 --- a/Makefile +++ b/Makefile @@ -38,7 +38,7 @@ include $(MAKEFILE_PRE) .PHONY: sync sync: - ./sync $(KVM_VERSION) + ./sync -v $(KVM_VERSION) -l $(LINUX) install: mkdir -p $(DESTDIR)/$(INSTALLDIR) diff --git a/sync b/sync index 4a89296..f3f4d6a 100755 --- a/sync +++ b/sync @@ -1,6 +1,7 @@ #!/usr/bin/python import sys, os, glob, os.path, shutil, re +from optparse import OptionParser glob = glob.glob @@ -8,11 +9,16 @@ def cmd(c): if os.system(c) != 0: raise Exception('command execution failed: ' + c) -version = 'kvm-devel' -if len(sys.argv) >= 2: -version = sys.argv[1] - -linux = 'linux-2.6' +parser = OptionParser(usage = 'usage: %prog [-v VERSION][-l LINUX]') +parser.add_option('-v', action = 'store', type = 'string', dest = 'version', \ + help = 'kvm-kmod release version', default = 'kvm-devel') +parser.add_option('-l', action = 'store', type = 'string', dest = 'linux', \ + help = 'Linux kernel tree to sync from', \ + default = 'linux-2.6') +parser.set_defaults() +(options, args) = parser.parse_args() +version = options.version +linux = options.linux _re_cache = {} -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/8] kvm-kmod: Include asm/uaccess.h on pre-2.6.18 kernels
Signed-off-by: Jan Kiszka --- external-module-compat-comm.h |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/external-module-compat-comm.h b/external-module-compat-comm.h index d473ae7..e2342db 100644 --- a/external-module-compat-comm.h +++ b/external-module-compat-comm.h @@ -302,7 +302,11 @@ static inline void pagefault_enable(void) #endif +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,18) +#include +#else #include +#endif /* vm ops ->fault() was introduced in 2.6.23. */ #include -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/8] kvm-kmod: Add eventfd.c to patch list
eventfd.c has to be processed in order to wrap INIT_WORK. Signed-off-by: Jan Kiszka --- sync |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/sync b/sync index fc4311e..18f0200 100755 --- a/sync +++ b/sync @@ -124,7 +124,7 @@ def hack(T, arch, file): hack_files = { 'x86': str.split('kvm_main.c mmu.c vmx.c svm.c x86.c irq.h lapic.c' - ' i8254.c kvm_trace.c timer.c'), + ' i8254.c kvm_trace.c timer.c eventfd.c'), 'ia64': str.split('kvm_main.c kvm_fw.c kvm_lib.c kvm-ia64.c'), } -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/8] kvm-kmod: Provide flush_work compat wrapper
Signed-off-by: Jan Kiszka --- external-module-compat-comm.h |9 + 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/external-module-compat-comm.h b/external-module-compat-comm.h index dad43ef..131210a 100644 --- a/external-module-compat-comm.h +++ b/external-module-compat-comm.h @@ -562,6 +562,15 @@ static inline int cancel_work_sync(struct work_struct *work) #endif +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,27) + +static inline void flush_work(struct work_struct *work) +{ + cancel_work_sync(work); +} + +#endif + #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20) struct pci_dev; -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/8] kvm-kmod: Compat fixes & enhancements
The is a flush of my current queue against kvm-kmod. Some are reposts, some are split-ups, and a few are new (specifically the MCE fixes). I did not include patches that you reported to have already in your queue. Find the patches also at git://git.kiszka.org/kvm-kmod.git queue Jan Kiszka (8): kvm-kmod: Allow to override sync source kvm-kmod: Include asm/uaccess.h on pre-2.6.18 kernels kvm-kmod: Include relayfs_fs.h on pre-2.6.17 kernels kvm-kmod: Provide flush_work compat wrapper kvm-kmod: Fix msi_enabled patching kvm-kmod: Add eventfd.c to patch list kvm-kmod: x86: Add MSR_IA32_TSC compat define kvm-kmod: x86: Add MCE compat defines Makefile |2 +- external-module-compat-comm.h | 17 + sync | 22 ++ x86/external-module-compat.h | 12 4 files changed, 44 insertions(+), 9 deletions(-) -- Siemens AG, Corporate Technology, CT SE 2 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
[PATCH 8/8] kvm-kmod: x86: Add MCE compat defines
Signed-off-by: Jan Kiszka --- x86/external-module-compat.h |8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/x86/external-module-compat.h b/x86/external-module-compat.h index c33eb2e..cc984ab 100644 --- a/x86/external-module-compat.h +++ b/x86/external-module-compat.h @@ -540,3 +540,11 @@ struct mtrr_state_type { #ifndef CONFIG_HAVE_KVM_IRQCHIP #define CONFIG_HAVE_KVM_IRQCHIP 1 #endif + +#ifndef MCG_CTL_P +#define MCG_CTL_P(1ULL<<8) +#define MCG_STATUS_MCIP (1ULL<<2) +#define MCI_STATUS_VAL (1ULL<<63) +#define MCI_STATUS_OVER (1ULL<<62) +#define MCI_STATUS_UC(1ULL<<61) +#endif -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/8] kvm-kmod: Fix msi_enabled patching
Signed-off-by: Jan Kiszka --- sync |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sync b/sync index f3f4d6a..fc4311e 100755 --- a/sync +++ b/sync @@ -67,8 +67,8 @@ def __hack(data): if match(r'^}'): kvm_arch_init = False if match(r'MODULE_AUTHOR'): w('MODULE_INFO(version, "%s");' % (version,)) -line = sub(r'match->dev->msi_enabled', - 'kvm_pcidev_msi_enabled(match->dev)', line) +line = sub(r'(\w+)->dev->msi_enabled', + r'kvm_pcidev_msi_enabled(\1->dev)', line) if match(r'atomic_inc\(&kvm->mm->mm_count\);'): line = 'mmget(&kvm->mm->mm_count);' if match(r'^\t\.fault = '): -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/8] kvm-kmod: x86: Add MSR_IA32_TSC compat define
Signed-off-by: Jan Kiszka --- x86/external-module-compat.h |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/x86/external-module-compat.h b/x86/external-module-compat.h index 3e6fc11..c33eb2e 100644 --- a/x86/external-module-compat.h +++ b/x86/external-module-compat.h @@ -497,6 +497,10 @@ struct kvm_desc_ptr { #define FEATURE_CONTROL_VMXON_ENABLED (1<<2) #endif +#ifndef MSR_IA32_TSC +#define MSR_IA32_TSC0x0010 +#endif + #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,25) && defined(__x86_64__) #undef set_debugreg -- 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: strange guest slowness after some time
Tomasz Chmielewski wrote: I still observe this "slowness" with kvm-86 after the guest is running for some time (virtio_net and virtio_console seem to be affected; guest restart doesn't fix it). Anything in guest dmesg? -- error compiling committee.c: too many arguments to function -- 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 0/8] kvm-kmod: Compat fixes & enhancements
Jan Kiszka wrote: The is a flush of my current queue against kvm-kmod. Some are reposts, some are split-ups, and a few are new (specifically the MCE fixes). I did not include patches that you reported to have already in your queue. Find the patches also at git://git.kiszka.org/kvm-kmod.git queue All applied, thanks. -- error compiling committee.c: too many arguments to function -- 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: Add qemu_send_raw() to vlan.
On Tue, May 26, 2009 at 02:15:37PM +0300, Avi Kivity wrote: > Mark McLoughlin wrote: >> On Tue, 2009-05-26 at 13:03 +0300, Gleb Natapov wrote: >> >>> It gets packet without virtio header and adds it if needed. Allows to >>> inject packets to vlan from outside. To send gracious arp for instance. >>> >> >> Acked-by: Mark McLoughlin >> >> This isn't ideal, but neither is the current VNET_HDR stuff and it does >> fix a genuine bug. At least it's not changing existing APIs and makes it >> clear this is a problem we also need to solve when merging VNET_HDR >> support upstream. >> >> (Gleb - it might be worth patching qemu.git to use qemu_send_packet() in >> announce_self(), that will reduce the delta between the trees) >> >> > > Okay. Bug Gleb, please add a signoff. > Signed-off-by: Gleb Natapov -- Gleb. -- 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: strange guest slowness after some time
Avi Kivity wrote: Tomasz Chmielewski wrote: I still observe this "slowness" with kvm-86 after the guest is running for some time (virtio_net and virtio_console seem to be affected; guest restart doesn't fix it). Anything in guest dmesg? No. No hints in syslog, dmesg... Can it be that this is more likely to happens on "busy" hosts? It happens for me on a host where I have 16 guests running. Also, as I booted the host almost 2 days ago, 2 or 3 guests didn't start properly (16 guests were starting at the same time), with their kernel saying: Kernel panic - not syncing: IO-APIC + timer doesn't work! Can it be related? After I restarted these failed guests, they started properly. -- Tomasz Chmielewski http://wpkg.org -- 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 8/8] kvm-kmod: x86: Add MCE compat defines
Jan Kiszka wrote: Signed-off-by: Jan Kiszka --- x86/external-module-compat.h |8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/x86/external-module-compat.h b/x86/external-module-compat.h index c33eb2e..cc984ab 100644 --- a/x86/external-module-compat.h +++ b/x86/external-module-compat.h @@ -540,3 +540,11 @@ struct mtrr_state_type { #ifndef CONFIG_HAVE_KVM_IRQCHIP #define CONFIG_HAVE_KVM_IRQCHIP 1 #endif + +#ifndef MCG_CTL_P +#define MCG_CTL_P(1ULL<<8) +#define MCG_STATUS_MCIP (1ULL<<2) +#define MCI_STATUS_VAL (1ULL<<63) +#define MCI_STATUS_OVER (1ULL<<62) +#define MCI_STATUS_UC(1ULL<<61) +#endif This breaks on recent kernels (redefinition), so I removed it. Suggest adding an include-compat/asm-x86/asm/mce.h and including that. -- error compiling committee.c: too many arguments to function -- 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: strange guest slowness after some time
Tomasz Chmielewski wrote: Avi Kivity wrote: Tomasz Chmielewski wrote: I still observe this "slowness" with kvm-86 after the guest is running for some time (virtio_net and virtio_console seem to be affected; guest restart doesn't fix it). Anything in guest dmesg? No. No hints in syslog, dmesg... Can it be that this is more likely to happens on "busy" hosts? We'll only know once we fix it... It happens for me on a host where I have 16 guests running. Also, as I booted the host almost 2 days ago, 2 or 3 guests didn't start properly (16 guests were starting at the same time), with their kernel saying: Kernel panic - not syncing: IO-APIC + timer doesn't work! Can it be related? After I restarted these failed guests, they started properly. This is timing related. On a busy host you can get timeouts and thus the panics. It's unrelated. Maybe virtio is racy and a loaded host exposes the race. -- error compiling committee.c: too many arguments to function -- 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: Add qemu_send_raw() to vlan.
Gleb Natapov wrote: On Tue, May 26, 2009 at 02:15:37PM +0300, Avi Kivity wrote: Mark McLoughlin wrote: On Tue, 2009-05-26 at 13:03 +0300, Gleb Natapov wrote: It gets packet without virtio header and adds it if needed. Allows to inject packets to vlan from outside. To send gracious arp for instance. Acked-by: Mark McLoughlin This isn't ideal, but neither is the current VNET_HDR stuff and it does fix a genuine bug. At least it's not changing existing APIs and makes it clear this is a problem we also need to solve when merging VNET_HDR support upstream. (Gleb - it might be worth patching qemu.git to use qemu_send_packet() in announce_self(), that will reduce the delta between the trees) Okay. Bug Gleb, please add a signoff. Signed-off-by: Gleb Natapov Thanks, added the signoff and the patch. -- error compiling committee.c: too many arguments to function -- 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 7/8] kvm-kmod: x86: Add MSR_IA32_TSC compat define
Hello Jan, On Tue, 2009-05-26 at 13:52 +0200, Jan Kiszka wrote: > Signed-off-by: Jan Kiszka > --- > > x86/external-module-compat.h |4 > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/x86/external-module-compat.h b/x86/external-module-compat.h > index 3e6fc11..c33eb2e 100644 > --- a/x86/external-module-compat.h > +++ b/x86/external-module-compat.h > @@ -497,6 +497,10 @@ struct kvm_desc_ptr { > #define FEATURE_CONTROL_VMXON_ENABLED(1<<2) > #endif > > +#ifndef MSR_IA32_TSC > +#define MSR_IA32_TSC0x0010 > +#endif > + AFAIK, MSR_IA32_TSC is defined in include/asm/msr-index.h which is available for kernel as well as user space and it is available from long time. Can you please point me in which case MSR_IA32_TSC is not visible. Thanks, -- JSR -- 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] qemu-kvm: Flush icache after dma operations for ia64
Jes Sorensen wrote: Ok, Trying once more. After spending a couple of hours trying to follow the QEMU dma codeflow, I have convinced myself Avi is right and those two functions don't need to do the flushing as they all end up calling dma_bdrv_cb() which calls dma_brdv_unmap(). I have added a couple comments to the code, which will hopefully save the next person the 'pleasure' of trying to figure out this too. It looks right to me. Xiantao? access_len -= l; -} + } + dma_flush_range((unsigned long)buffer, + (unsigned long)buffer + flush_len); } Detab your code, please. -- error compiling committee.c: too many arguments to function -- 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 7/8] kvm-kmod: x86: Add MSR_IA32_TSC compat define
Jaswinder Singh Rajput wrote: > Hello Jan, > > On Tue, 2009-05-26 at 13:52 +0200, Jan Kiszka wrote: >> Signed-off-by: Jan Kiszka >> --- >> >> x86/external-module-compat.h |4 >> 1 files changed, 4 insertions(+), 0 deletions(-) >> >> diff --git a/x86/external-module-compat.h b/x86/external-module-compat.h >> index 3e6fc11..c33eb2e 100644 >> --- a/x86/external-module-compat.h >> +++ b/x86/external-module-compat.h >> @@ -497,6 +497,10 @@ struct kvm_desc_ptr { >> #define FEATURE_CONTROL_VMXON_ENABLED (1<<2) >> #endif >> >> +#ifndef MSR_IA32_TSC >> +#define MSR_IA32_TSC0x0010 >> +#endif >> + > > AFAIK, MSR_IA32_TSC is defined in include/asm/msr-index.h which is > available for kernel as well as user space and it is available from long > time. msr-index.h is "fairly" new (keep in mind all the x86 unifications and renaming of msr.h to msr-index.h). > > Can you please point me in which case MSR_IA32_TSC is not visible. I'm about to loose overview of all those issues I ran into with older kernels (still more to fix, sigh), but I think to remember this was a 2.6.18 x86_32 problem. Wait... yes, 2.6.18 asm-i386 does not include this. Jan -- Siemens AG, Corporate Technology, CT SE 2 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: [PATCH 7/8] kvm-kmod: x86: Add MSR_IA32_TSC compat define
Jan Kiszka wrote: I'm about to loose overview of all those issues I ran into with older kernels (still more to fix, sigh), but I think to remember this was a 2.6.18 x86_32 problem. Wait... yes, 2.6.18 asm-i386 does not include this. I have the same problem. I'll look at setting up automated build-testing of kvm-kmod.git+kvm.git for versions in range(16, 30) for arch in ['i386', 'x86_64']. -- error compiling committee.c: too many arguments to function -- 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 8/8] kvm-kmod: x86: Add MCE compat defines
Avi Kivity wrote: > Jan Kiszka wrote: >> Signed-off-by: Jan Kiszka >> --- >> >> x86/external-module-compat.h |8 >> 1 files changed, 8 insertions(+), 0 deletions(-) >> >> diff --git a/x86/external-module-compat.h b/x86/external-module-compat.h >> index c33eb2e..cc984ab 100644 >> --- a/x86/external-module-compat.h >> +++ b/x86/external-module-compat.h >> @@ -540,3 +540,11 @@ struct mtrr_state_type { >> #ifndef CONFIG_HAVE_KVM_IRQCHIP >> #define CONFIG_HAVE_KVM_IRQCHIP 1 >> #endif >> + >> +#ifndef MCG_CTL_P >> +#define MCG_CTL_P(1ULL<<8) >> +#define MCG_STATUS_MCIP (1ULL<<2) >> +#define MCI_STATUS_VAL (1ULL<<63) >> +#define MCI_STATUS_OVER (1ULL<<62) >> +#define MCI_STATUS_UC(1ULL<<61) >> +#endif >> >> > > This breaks on recent kernels (redefinition), so I removed it. Suggest > adding an include-compat/asm-x86/asm/mce.h and including that. Non-empty mce.h will not work (but a patch to add an empty one was missing from my queue) - new kernel have that file, but with improper definitions for 32-bit. But what was that recent kernel and what was the precise problem? ATM I could only imagine that the block above is included before original mce.h, right? Then adding an explicit include here should cure the problem. Jan -- Siemens AG, Corporate Technology, CT SE 2 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: [PATCH 7/8] kvm-kmod: x86: Add MSR_IA32_TSC compat define
On Tue, 2009-05-26 at 14:36 +0200, Jan Kiszka wrote: > Jaswinder Singh Rajput wrote: > > Hello Jan, > > > > On Tue, 2009-05-26 at 13:52 +0200, Jan Kiszka wrote: > >> Signed-off-by: Jan Kiszka > >> --- > >> > >> x86/external-module-compat.h |4 > >> 1 files changed, 4 insertions(+), 0 deletions(-) > >> > >> diff --git a/x86/external-module-compat.h b/x86/external-module-compat.h > >> index 3e6fc11..c33eb2e 100644 > >> --- a/x86/external-module-compat.h > >> +++ b/x86/external-module-compat.h > >> @@ -497,6 +497,10 @@ struct kvm_desc_ptr { > >> #define FEATURE_CONTROL_VMXON_ENABLED (1<<2) > >> #endif > >> > >> +#ifndef MSR_IA32_TSC > >> +#define MSR_IA32_TSC0x0010 > >> +#endif > >> + > > > > AFAIK, MSR_IA32_TSC is defined in include/asm/msr-index.h which is > > available for kernel as well as user space and it is available from long > > time. > > msr-index.h is "fairly" new (keep in mind all the x86 unifications and > renaming of msr.h to msr-index.h). > > > > > Can you please point me in which case MSR_IA32_TSC is not visible. > > I'm about to loose overview of all those issues I ran into with older > kernels (still more to fix, sigh), but I think to remember this was a > 2.6.18 x86_32 problem. Wait... yes, 2.6.18 asm-i386 does not include this. > Ahh, for ancient kernels. Ok :-) -- JSR -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] kvm-kmod: x86: Add MSR_K8_SYSCFG compat define
Jan Kiszka wrote: > The is a flush of my current queue against kvm-kmod. Some are reposts, > some are split-ups, and a few are new (specifically the MCE fixes). I > did not include patches that you reported to have already in your queue. > > Find the patches also at git://git.kiszka.org/kvm-kmod.git queue > > Jan Kiszka (8): > kvm-kmod: Allow to override sync source > kvm-kmod: Include asm/uaccess.h on pre-2.6.18 kernels > kvm-kmod: Include relayfs_fs.h on pre-2.6.17 kernels > kvm-kmod: Provide flush_work compat wrapper > kvm-kmod: Fix msi_enabled patching > kvm-kmod: Add eventfd.c to patch list > kvm-kmod: x86: Add MSR_IA32_TSC compat define > kvm-kmod: x86: Add MCE compat defines > And one more (probably not the last one, build round-trip on for my legacy kernel environment is currently longer than posting round-trip). Or do you have this in your queue already, too? > Signed-off-by: Jan Kiszka --- x86/external-module-compat.h |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/x86/external-module-compat.h b/x86/external-module-compat.h index 66f3aea..3fc91a1 100644 --- a/x86/external-module-compat.h +++ b/x86/external-module-compat.h @@ -512,6 +512,10 @@ struct kvm_desc_ptr { #define MSR_K7_HWCR 0xc0010015 #endif +#ifndef MSR_K8_SYSCFG +#define MSR_K8_SYSCFG 0xc0010010 +#endif + #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,25) && defined(__x86_64__) #undef set_debugreg -- 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 7/8] kvm-kmod: x86: Add MSR_IA32_TSC compat define
Avi Kivity wrote: > Jan Kiszka wrote: >> I'm about to loose overview of all those issues I ran into with older >> kernels (still more to fix, sigh), but I think to remember this was a >> 2.6.18 x86_32 problem. Wait... yes, 2.6.18 asm-i386 does not include >> this. >> > > I have the same problem. I'll look at setting up automated > build-testing of kvm-kmod.git+kvm.git for versions in range(16, 30) for > arch in ['i386', 'x86_64']. Applause in advance! Jan -- Siemens AG, Corporate Technology, CT SE 2 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
[PATCH] x86: Make MCE defines 32-bit-safe
These defines are used by KVM now also on 32-bit hosts. Signed-off-by: Jan Kiszka --- arch/x86/include/asm/mce.h | 30 +++--- 1 files changed, 15 insertions(+), 15 deletions(-) diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h index f6bb3ff..3b532bb 100644 --- a/arch/x86/include/asm/mce.h +++ b/arch/x86/include/asm/mce.h @@ -8,21 +8,21 @@ * Machine Check support for x86 */ -#define MCG_CTL_P (1UL<<8) /* MCG_CAP register available */ -#define MCG_EXT_P (1ULL<<9) /* Extended registers available */ -#define MCG_CMCI_P (1ULL<<10) /* CMCI supported */ - -#define MCG_STATUS_RIPV (1UL<<0) /* restart ip valid */ -#define MCG_STATUS_EIPV (1UL<<1) /* ip points to correct instruction */ -#define MCG_STATUS_MCIP (1UL<<2) /* machine check in progress */ - -#define MCI_STATUS_VAL (1UL<<63) /* valid error */ -#define MCI_STATUS_OVER (1UL<<62) /* previous errors lost */ -#define MCI_STATUS_UC(1UL<<61) /* uncorrected error */ -#define MCI_STATUS_EN(1UL<<60) /* error enabled */ -#define MCI_STATUS_MISCV (1UL<<59) /* misc error reg. valid */ -#define MCI_STATUS_ADDRV (1UL<<58) /* addr reg. valid */ -#define MCI_STATUS_PCC (1UL<<57) /* processor context corrupt */ +#define MCG_CTL_P(1ULL<<8) /* MCG_CAP register available */ +#define MCG_EXT_P(1ULL<<9) /* Extended registers available */ +#define MCG_CMCI_P (1ULL<<10) /* CMCI supported */ + +#define MCG_STATUS_RIPV (1ULL<<0) /* restart ip valid */ +#define MCG_STATUS_EIPV (1ULL<<1) /* ip points to correct instruction */ +#define MCG_STATUS_MCIP (1ULL<<2) /* machine check in progress */ + +#define MCI_STATUS_VAL (1ULL<<63) /* valid error */ +#define MCI_STATUS_OVER (1ULL<<62) /* previous errors lost */ +#define MCI_STATUS_UC(1ULL<<61) /* uncorrected error */ +#define MCI_STATUS_EN(1ULL<<60) /* error enabled */ +#define MCI_STATUS_MISCV (1ULl<<59) /* misc error reg. valid */ +#define MCI_STATUS_ADDRV (1ULL<<58) /* addr reg. valid */ +#define MCI_STATUS_PCC (1ULL<<57) /* processor context corrupt */ /* Fields are zero when not available */ struct mce { -- 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 7/8] kvm-kmod: x86: Add MSR_IA32_TSC compat define
Jan Kiszka wrote: Avi Kivity wrote: Jan Kiszka wrote: I'm about to loose overview of all those issues I ran into with older kernels (still more to fix, sigh), but I think to remember this was a 2.6.18 x86_32 problem. Wait... yes, 2.6.18 asm-i386 does not include this. I have the same problem. I'll look at setting up automated build-testing of kvm-kmod.git+kvm.git for versions in range(16, 30) for arch in ['i386', 'x86_64']. Applause in advance! Be careful, I might auto-mail you the results. -- error compiling committee.c: too many arguments to function -- 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 8/8] kvm-kmod: x86: Add MCE compat defines
Jan Kiszka wrote: + +#ifndef MCG_CTL_P +#define MCG_CTL_P(1ULL<<8) +#define MCG_STATUS_MCIP (1ULL<<2) +#define MCI_STATUS_VAL (1ULL<<63) +#define MCI_STATUS_OVER (1ULL<<62) +#define MCI_STATUS_UC(1ULL<<61) +#endif This breaks on recent kernels (redefinition), so I removed it. Suggest adding an include-compat/asm-x86/asm/mce.h and including that. Non-empty mce.h will not work (but a patch to add an empty one was missing from my queue) - new kernel have that file, but with improper definitions for 32-bit. But what was that recent kernel and what was the precise problem? 2.6.27, those defines were redefined (by the subsequent include of mce.h) ATM I could only imagine that the block above is included before original mce.h, right? Then adding an explicit include here should cure the problem. Yes. -- error compiling committee.c: too many arguments to function -- 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] x86: Make MCE defines 32-bit-safe
Jan Kiszka wrote: These defines are used by KVM now also on 32-bit hosts. Applied, thanks. -- error compiling committee.c: too many arguments to function -- 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/4] KVM: move coalesced_mmio locking to its own device
On Tue, May 26, 2009 at 02:24:33PM +0300, Avi Kivity wrote: > Marcelo Tosatti wrote: === --- kvm-irqlock.orig/virt/kvm/coalesced_mmio.c +++ kvm-irqlock/virt/kvm/coalesced_mmio.c @@ -26,9 +26,12 @@ static int coalesced_mmio_in_range(struc if (!is_write) return 0; - /* kvm->lock is taken by the caller and must be not released before - * dev.read/write - */ + /* + * Some other vcpu might be batching data into the ring, + * fallback to userspace. Ordering not our problem. + */ + if (!atomic_add_unless(&dev->in_use, 1, 1)) + return 0; >>> Ordering with simultaneous writes is indeed not our problem, but the >>> ring may contain ordered writes (even by the same vcpu!) >>> >> >> You mean writes by a vcpu should maintain ordering even when that vcpu >> migrates to a different pcpu? > > No. Writes by a single vcpu need to preserve their ordering relative to > each other. If a write by vcpu A completes before a write by vcpu B > starts, then they need to be ordered, since the vcpus may have > synchronized in between. > > Hm, I don't thimk you broke these rules. > >> The smp_wmb in coalesced_write guarantees that. >> >> >>> Suggest our own lock here. in_use is basically a homemade lock, >>> better to use the factory made ones which come with a warranty. >>> >> >> The first submission had a mutex but was considered unorthodox. Although >> i agree with your argument made then, i don't see a way around that. >> >> Some pseudocode please? > > Why not use slots_lock to protect the entire iodevice list (rcu one > day), and an internal spinlock for coalesced mmio? Don't like using slots_lock to protect the entire iodevice list, its reverse progress in my opinion. The PIO/MMIO device lists are data structures used in hot path, they are not directly related to the memslots, and therefore deserve a separate lock. Whenever slots_lock becomes an issue, you'll have to entangle the mess, so better start doing it now. Sure can switch to a internal spinlock for coalesced_mmio, but breaking out if spin_trylock fails. You're OK with that? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] allow CPUID vendor override
KVM will always report the vendor ID of the physical CPU it is running on. Allow to override this if explicitly requested on the command line. It will not suffice to name a CPU type (like -cpu phenom), but you have to explicitly set the vendor: -cpu phenom,vendor=AuthenticAMD Signed-off-by: Andre Przywara --- target-i386/cpu.h|1 + target-i386/helper.c |5 - 2 files changed, 5 insertions(+), 1 deletions(-) diff --git a/target-i386/cpu.h b/target-i386/cpu.h index d7b32d4..133a69d 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -655,6 +655,7 @@ typedef struct CPUX86State { uint32_t cpuid_ext2_features; uint32_t cpuid_ext3_features; uint32_t cpuid_apic_id; +uint32_t cpuid_vendor_override; /* MTRRs */ uint64_t mtrr_fixed[11]; diff --git a/target-i386/helper.c b/target-i386/helper.c index ed2dc41..0fb31c6 100644 --- a/target-i386/helper.c +++ b/target-i386/helper.c @@ -118,6 +118,7 @@ typedef struct x86_def_t { uint32_t features, ext_features, ext2_features, ext3_features; uint32_t xlevel; char model_id[48]; +int vendor_override; } x86_def_t; #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE) @@ -378,6 +379,7 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model) x86_cpu_def->vendor2 |= ((uint8_t)val[i + 4]) << (8 * i); x86_cpu_def->vendor3 |= ((uint8_t)val[i + 8]) << (8 * i); } +x86_cpu_def->vendor_override = 1; } else if (!strcmp(featurestr, "model_id")) { pstrcpy(x86_cpu_def->model_id, sizeof(x86_cpu_def->model_id), val); @@ -430,6 +432,7 @@ static int cpu_x86_register (CPUX86State *env, const char *cpu_model) env->cpuid_vendor2 = CPUID_VENDOR_INTEL_2; env->cpuid_vendor3 = CPUID_VENDOR_INTEL_3; } +env->cpuid_vendor_override = def->vendor_override; env->cpuid_level = def->level; if (def->family > 0x0f) env->cpuid_version = 0xf00 | ((def->family - 0x0f) << 20); @@ -1508,7 +1511,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, * isn't supported in compatibility mode on Intel. so advertise the * actuall cpu, and say goodbye to migration between different vendors * is you use compatibility mode. */ -if (kvm_enabled()) +if (kvm_enabled() && !env->cpuid_vendor_override) host_cpuid(0, 0, NULL, ebx, ecx, edx); break; case 1: -- 1.6.1.3 -- 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/4] KVM: move coalesced_mmio locking to its own device
Marcelo Tosatti wrote: Why not use slots_lock to protect the entire iodevice list (rcu one day), and an internal spinlock for coalesced mmio? Don't like using slots_lock to protect the entire iodevice list, its reverse progress in my opinion. The PIO/MMIO device lists are data structures used in hot path, they are not directly related to the memslots, and therefore deserve a separate lock. Whenever slots_lock becomes an issue, you'll have to entangle the mess, so better start doing it now. It's a reader/writer lock, so you'll never have contention. I'm okay with a new lock though. Sure can switch to a internal spinlock for coalesced_mmio, but breaking out if spin_trylock fails. You're OK with that? Why not do a normal spin_lock()? It's not like you'll wait years for the store to complete. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] kvm-kmod: x86: Add MCE compat defines
Changes in v2: - added empty mce.h for 32-bit kernels - fixed redefinition issue by pulling in mce.h early Signed-off-by: Jan Kiszka --- include-compat/asm-x86/mce.h |1 + x86/external-module-compat.h | 10 ++ 2 files changed, 11 insertions(+), 0 deletions(-) create mode 100644 include-compat/asm-x86/mce.h diff --git a/include-compat/asm-x86/mce.h b/include-compat/asm-x86/mce.h new file mode 100644 index 000..1eb03c6 --- /dev/null +++ b/include-compat/asm-x86/mce.h @@ -0,0 +1 @@ +/* empty file to keep #include happy */ diff --git a/x86/external-module-compat.h b/x86/external-module-compat.h index c33eb2e..5add5c9 100644 --- a/x86/external-module-compat.h +++ b/x86/external-module-compat.h @@ -540,3 +540,13 @@ struct mtrr_state_type { #ifndef CONFIG_HAVE_KVM_IRQCHIP #define CONFIG_HAVE_KVM_IRQCHIP 1 #endif + +#include + +#ifndef MCG_CTL_P +#define MCG_CTL_P(1ULL<<8) +#define MCG_STATUS_MCIP (1ULL<<2) +#define MCI_STATUS_VAL (1ULL<<63) +#define MCI_STATUS_OVER (1ULL<<62) +#define MCI_STATUS_UC(1ULL<<61) +#endif -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] kvm-kmod: x86: Add MCE compat defines
Jan Kiszka wrote: Changes in v2: - added empty mce.h for 32-bit kernels - fixed redefinition issue by pulling in mce.h early Applied, thanks. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/2] virtio: fix id_matching for virtio drivers
From: Christian Borntraeger This bug never appeared, since all current virtio drivers use VIRTIO_DEV_ANY_ID for the vendor field. If a real vendor would be used, the check in virtio_id_match is wrong - it returns 0 if id->vendor == dev->id.vendor. Signed-off-by: Christian Borntraeger --- drivers/virtio/virtio.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: linux-next/drivers/virtio/virtio.c === --- linux-next.orig/drivers/virtio/virtio.c +++ linux-next/drivers/virtio/virtio.c @@ -61,7 +61,7 @@ static inline int virtio_id_match(const if (id->device != dev->id.device) return 0; - return id->vendor == VIRTIO_DEV_ANY_ID || id->vendor != dev->id.vendor; + return id->vendor == VIRTIO_DEV_ANY_ID || id->vendor == dev->id.vendor; } /* This looks through all the IDs a driver claims to support. If any of them -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 0/2] Fix and enhance virtio device id matching
Hello Rusty, here are two patches that change the device matching of the virtio bus. The patches have been refreshed agains linux-next and should apply to your private virtio tree. OK for linux-next? Christian -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 2/2] virtio: enhance id_matching for virtio drivers
From: Christian Borntraeger This patch allows a virtio driver to use VIRTIO_DEV_ANY_ID for the device id. This will be used by a test module that can be bound to any virtio device. Signed-off-by: Christian Borntraeger --- drivers/virtio/virtio.c |2 +- scripts/mod/file2alias.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) Index: linux-next/drivers/virtio/virtio.c === --- linux-next.orig/drivers/virtio/virtio.c +++ linux-next/drivers/virtio/virtio.c @@ -58,7 +58,7 @@ static struct device_attribute virtio_de static inline int virtio_id_match(const struct virtio_device *dev, const struct virtio_device_id *id) { - if (id->device != dev->id.device) + if (id->device != dev->id.device && id->device != VIRTIO_DEV_ANY_ID) return 0; return id->vendor == VIRTIO_DEV_ANY_ID || id->vendor == dev->id.vendor; Index: linux-next/scripts/mod/file2alias.c === --- linux-next.orig/scripts/mod/file2alias.c +++ linux-next/scripts/mod/file2alias.c @@ -641,7 +641,7 @@ static int do_virtio_entry(const char *f id->vendor = TO_NATIVE(id->vendor); strcpy(alias, "virtio:"); - ADD(alias, "d", 1, id->device); + ADD(alias, "d", id->device != VIRTIO_DEV_ANY_ID, id->device); ADD(alias, "v", id->vendor != VIRTIO_DEV_ANY_ID, id->vendor); add_wildcard(alias); -- 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: [Lguest] [PATCH] lguest: fix on Intel when KVM loaded (unhandled trap 13)
Alan Cox wrote: On Tue, 26 May 2009 20:54:41 +0930 Rusty Russell wrote: When KVM is loaded, and hence VT set up, the vmcall instruction in an lguest guest causes a #GP, not #UD. Signed-off-by: Rusty Russell Shouldn't that be fixed in KVM ? KVM never saw that VMCALL or #GP. -- error compiling committee.c: too many arguments to function -- 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: strange guest slowness after some time
Avi Kivity wrote: Tomasz Chmielewski wrote: Avi Kivity wrote: Tomasz Chmielewski wrote: I still observe this "slowness" with kvm-86 after the guest is running for some time (virtio_net and virtio_console seem to be affected; guest restart doesn't fix it). Anything in guest dmesg? No. No hints in syslog, dmesg... Can it be that this is more likely to happens on "busy" hosts? We'll only know once we fix it... (...) Maybe virtio is racy and a loaded host exposes the race. I see it happening with virtio on 2.6.29.x guests as well. So, what would you do if you saw it on your systems as well? ;) Add some debug routines into virtio_* modules? -- Tomasz Chmielewski http://wpkg.org -- 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: strange guest slowness after some time
Tomasz Chmielewski wrote: Maybe virtio is racy and a loaded host exposes the race. I see it happening with virtio on 2.6.29.x guests as well. So, what would you do if you saw it on your systems as well? ;) Add some debug routines into virtio_* modules? I'm no virtio expert. Maybe I'd insert tracepoints to record interrupts and kicks. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [KVM PATCH v3 2/4] kvm: add return value to kvm_io_bus_register_dev
Gregory Haskins wrote: > Today this function returns void and will internally BUG_ON if it fails. > We want to create dynamic MMIO/PIO entries driven from userspace later in > the series, so enhance this API to return an error code on failure. > > We also fix up all the callsites to check the return code, handle any > failures, and percolate the error up to the caller. > > Signed-off-by: Gregory Haskins > --- > > arch/x86/kvm/i8254.c | 27 +-- > arch/x86/kvm/i8259.c |9 - > include/linux/kvm_host.h |4 ++-- > virt/kvm/coalesced_mmio.c |8 ++-- > virt/kvm/ioapic.c |9 +++-- > virt/kvm/kvm_main.c |7 +-- > 6 files changed, 45 insertions(+), 19 deletions(-) > > diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c > index 4d6f0d2..3ef8b1b 100644 > --- a/arch/x86/kvm/i8254.c > +++ b/arch/x86/kvm/i8254.c > @@ -564,33 +564,36 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm) > { > struct kvm_pit *pit; > struct kvm_kpit_state *pit_state; > + int ret; > > pit = kzalloc(sizeof(struct kvm_pit), GFP_KERNEL); > if (!pit) > return NULL; > > pit->irq_source_id = kvm_request_irq_source_id(kvm); > - if (pit->irq_source_id < 0) { > - kfree(pit); > - return NULL; > - } > - > - mutex_init(&pit->pit_state.lock); > - mutex_lock(&pit->pit_state.lock); > - spin_lock_init(&pit->pit_state.inject_lock); > + if (pit->irq_source_id < 0) > + goto fail; > > /* Initialize PIO device */ > pit->dev.read = pit_ioport_read; > pit->dev.write = pit_ioport_write; > pit->dev.in_range = pit_in_range; > pit->dev.private = pit; > - kvm_io_bus_register_dev(&kvm->pio_bus, &pit->dev); > + ret = kvm_io_bus_register_dev(&kvm->pio_bus, &pit->dev); > + if (ret < 0) > + goto fail; > > pit->speaker_dev.read = speaker_ioport_read; > pit->speaker_dev.write = speaker_ioport_write; > pit->speaker_dev.in_range = speaker_in_range; > pit->speaker_dev.private = pit; > - kvm_io_bus_register_dev(&kvm->pio_bus, &pit->speaker_dev); > + ret = kvm_io_bus_register_dev(&kvm->pio_bus, &pit->speaker_dev); > + if (ret < 0) > + goto fail; > + > + mutex_init(&pit->pit_state.lock); > + mutex_lock(&pit->pit_state.lock); > + spin_lock_init(&pit->pit_state.inject_lock); > > kvm->arch.vpit = pit; > pit->kvm = kvm; > @@ -611,6 +614,10 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm) > kvm_register_irq_mask_notifier(kvm, 0, &pit->mask_notifier); > > return pit; > + > +fail: > + kfree(pit); > Hmm, this is broken. I need to also potentially free any successfully registered devices. Looks like we need a v4 afterall. > + return NULL; > } > > void kvm_free_pit(struct kvm *kvm) > diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c > index 1ccb50c..0caf7d4 100644 > --- a/arch/x86/kvm/i8259.c > +++ b/arch/x86/kvm/i8259.c > @@ -519,6 +519,8 @@ static void pic_irq_request(void *opaque, int level) > struct kvm_pic *kvm_create_pic(struct kvm *kvm) > { > struct kvm_pic *s; > + int ret; > + > s = kzalloc(sizeof(struct kvm_pic), GFP_KERNEL); > if (!s) > return NULL; > @@ -538,6 +540,11 @@ struct kvm_pic *kvm_create_pic(struct kvm *kvm) > s->dev.write = picdev_write; > s->dev.in_range = picdev_in_range; > s->dev.private = s; > - kvm_io_bus_register_dev(&kvm->pio_bus, &s->dev); > + ret = kvm_io_bus_register_dev(&kvm->pio_bus, &s->dev); > + if (ret < 0) { > + kfree(s); > + return NULL; > + } > + > return s; > } > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index 28bd112..5289552 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -61,8 +61,8 @@ void kvm_io_bus_init(struct kvm_io_bus *bus); > void kvm_io_bus_destroy(struct kvm_io_bus *bus); > struct kvm_io_device *kvm_io_bus_find_dev(struct kvm_io_bus *bus, > gpa_t addr, int len, int is_write); > -void kvm_io_bus_register_dev(struct kvm_io_bus *bus, > - struct kvm_io_device *dev); > +int kvm_io_bus_register_dev(struct kvm_io_bus *bus, > + struct kvm_io_device *dev); > > struct kvm_vcpu { > struct kvm *kvm; > diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c > index 5ae620d..ede9087 100644 > --- a/virt/kvm/coalesced_mmio.c > +++ b/virt/kvm/coalesced_mmio.c > @@ -86,6 +86,7 @@ static void coalesced_mmio_destructor(struct kvm_io_device > *this) > int kvm_coalesced_mmio_init(struct kvm *kvm) > { > struct kvm_coalesced_mmio_dev *dev; > + int ret; > > dev = kzalloc(sizeof(struct kvm_coalesced_mmio_dev), GFP_KERNEL); > if (!dev) > @@ -96,9 +97,12 @@ int kvm_coalesced_mmio_init(struct kvm *kvm)
[PATCH][KVM_AUTOTEST] Fix to allow for "=" in the "value" of a config parameter.
example: kernel_args = "ks=floppy console=ttyS0 noacpi" fix modifies kvm_config.split_and_strip so it will only split once per line. --- client/tests/kvm_runtest_2/kvm_config.py |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/client/tests/kvm_runtest_2/kvm_config.py b/client/tests/kvm_runtest_2/kvm_config.py index 4a1e7b4..c4fe6ea 100755 --- a/client/tests/kvm_runtest_2/kvm_config.py +++ b/client/tests/kvm_runtest_2/kvm_config.py @@ -94,7 +94,7 @@ class config: def split_and_strip(self, str, sep="="): """Split str and strip quotes from the resulting parts.""" -temp = str.split(sep) +temp = str.split(sep, 1) for i in range(len(temp)): temp[i] = temp[i].strip() temp[i] = temp[i].strip("\"\'") -- 1.6.0.6 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: kvm-86 release - USB thumbdrive
I've tried adding it as 'pcidevice host=' when launch the guest and via hotplug w/ pci_add pci_addr=auto host host= after the guest is launched. -Original Message- From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org] On Behalf Of Avi Kivity Sent: Sunday, May 24, 2009 2:44 AM To: Denherder, Cindy Cc: KVM list Subject: Re: kvm-86 release - USB thumbdrive Denherder, Cindy wrote: > Hi, > > I'm not having any luck getting my usb thumb drive recognized in my Windows > guests via the hot plug method. I checked the wiki site and it looks like > this is an issue being worked on - does anyone know the status of this? (I > can get it to work by creating an img file for it.) > Are you using usb hot plug or scsi hot plug? What exactly are you doing? -- error compiling committee.c: too many arguments to function -- 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 -- 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_Autotest] Added functionality to the preprocessor to run scripts
Michael Goldish wrote: > Looks good to me. See some comments below. > > - "David Huff" wrote: > >> This patch will run pre and post scripts >> defined in config file with the parameter pre_command >> and post_command post_command. >> >> Also exports all the prameters in preprocess for passing >> arguments to the script. > > Why not do this for post_command as well? I didn't do post_command b/c I figured that they would already be exported in the pre_command, however I guess that there can be the case where there is no pre and only post and I guess exporting twice will not hurt anything I will add this to the patch. > >> --- >> client/tests/kvm_runtest_2/kvm_preprocessing.py | 31 >> +- >> 1 files changed, 29 insertions(+), 2 deletions(-) >> >> diff --git a/client/tests/kvm_runtest_2/kvm_preprocessing.py >> b/client/tests/kvm_runtest_2/kvm_preprocessing.py >> index c9eb35d..02df615 100644 >> --- a/client/tests/kvm_runtest_2/kvm_preprocessing.py >> +++ b/client/tests/kvm_runtest_2/kvm_preprocessing.py >> @@ -135,8 +135,7 @@ def postprocess_vm(test, params, env, name): >> "Waiting for VM to kill itself..."): >> kvm_log.debug("'kill_vm' specified; killing VM...") >> vm.destroy(gracefully = params.get("kill_vm_gracefully") == >> "yes") >> - >> - >> + > > I hate to be petty, but we usually keep two blank lines between top > level functions. > > Also, you have some trailing whitespace there... good catch, I will take care of this > It wouldn't hurt to make this timeout user-configurable with a default > value of 600 or so: > > timeout = int(params.get("pre_commmand_timeout", "600")) > (status, pid, output) = kvm_utils.run_bg(..., timeout=timeout) > > We can also do that in a separate patch. > I'll go ahead and add this while I rework the patch... -D -- 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_Autotest] Fix to allow for "=" in the "value" of a config parameter.
Avi Kivity wrote: > Michael Goldish wrote: >> This makes sense, thanks. >> >> One thing though -- I don't know if it's even worth mentioning -- we >> always try to put a single space after a comma (I think the Python >> style guide recommends that but I'm not sure). Generally we try to >> follow the guide (http://www.python.org/dev/peps/pep-0008/) and as far >> as I know Autotest follows it too. >> > > Strongly agree, Python's syntax allows us to write very pretty code, it > makes sense to preserve this. > Thanks for the advice new patch to follow shortly -D -- 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
[KVM_AUTOTEST] patch set to dynamically load tests
resending patch set... first pass at spiting up tests into separate files... These relatively simple changes to kvm_runtest_2 will allow for the addition of new tests without having modifying kvm_runtest "code." One would just add a newtest.py file and update the config file. This will simplify test development and make it easier to deploying new tests in an existing environment. The patch looks for tests that are not statically defined in kvm_runtest_2.py in ./kvm-autotest/client/tests/kvm_runtest_2/kvm_tests/'testname'.py and dynamically loads a module with a corresponding method run_'testname' where testname is defined by the "type" parameter in the config file. looking for comments/feedback -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Modified kvm_runtest_2.py to look for tests in kvm_tests/
This will allow for adding of additional tests with out modifying the "code." One would just add the testname.py file to the test_dir and edit the comfig file. fixed typo --- client/tests/kvm_runtest_2/kvm_runtest_2.py | 32 +++--- 1 files changed, 23 insertions(+), 9 deletions(-) diff --git a/client/tests/kvm_runtest_2/kvm_runtest_2.py b/client/tests/kvm_runtest_2/kvm_runtest_2.py index fda7282..6420191 100644 --- a/client/tests/kvm_runtest_2/kvm_runtest_2.py +++ b/client/tests/kvm_runtest_2/kvm_runtest_2.py @@ -2,6 +2,7 @@ import sys import os +import inspect import time import shelve import random @@ -21,26 +22,26 @@ class test_routine: class kvm_runtest_2(test.test): version = 1 + def setup(self): pass def initialize(self): -# Define the test routines corresponding to different values of the 'type' field +# directory where to look for tests +self.test_dir = os.path.join(self.bindir, "kvm_tests") + +# pre-defined the test routines corresponding to different values of the 'type' field self.test_routines = { -# type module nameroutine +# type module nameroutine name "steps":test_routine("kvm_guest_wizard", "run_steps"), "stepmaker":test_routine("stepmaker", "run_stepmaker"), -"boot": test_routine("kvm_tests", "run_boot"), -"migration":test_routine("kvm_tests", "run_migration"), -"yum_update": test_routine("kvm_tests", "run_yum_update"), -"autotest": test_routine("kvm_tests", "run_autotest"), "kvm_install": test_routine("kvm_install", "run_kvm_install"), -"linux_s3": test_routine("kvm_tests", "run_linux_s3"), } - + # Make it possible to import modules from the test's bindir sys.path.append(self.bindir) - +sys.path.append(self.test_dir) + def run_once(self, params): import kvm_log import kvm_utils @@ -74,6 +75,12 @@ class kvm_runtest_2(test.test): type = params.get("type") routine_obj = self.test_routines.get(type) # If type could not be found in self.test_routines... +# look for test in kvm_tests directory, where type = 'testname' +# defined in file 'testname'.py and test routine method run_'testname' +if os.path.isfile(os.path.join(self.test_dir,type+".py")): +module_name = type +routine_name = "run_"+module_name +routine_obj = test_routine(module_name,routine_name) if not routine_obj: message = "Unsupported test type: %s" % type kvm_log.error(message) @@ -83,6 +90,13 @@ class kvm_runtest_2(test.test): # Dynamically import the module module = __import__(routine_obj.module_name) # Get the needed routine +try: + inspect.isfunction(eval("module."+routine_obj.routine_name)) +except Exception, e: +kvm_log.error("Test failed: %s" % e) +kvm_log.error("could not load:%s" % eval("module."+routine_obj.routine_name)) +raise + routine_obj.routine = eval("module." + routine_obj.routine_name) # Preprocess -- 1.6.0.6 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] removed old kvm_test.py, tests now in seprate subdir
--- client/tests/kvm_runtest_2/kvm_tests.py | 396 --- 1 files changed, 0 insertions(+), 396 deletions(-) delete mode 100644 client/tests/kvm_runtest_2/kvm_tests.py diff --git a/client/tests/kvm_runtest_2/kvm_tests.py b/client/tests/kvm_runtest_2/kvm_tests.py deleted file mode 100644 index 950115d..000 --- a/client/tests/kvm_runtest_2/kvm_tests.py +++ /dev/null @@ -1,396 +0,0 @@ -import time -import os - -from autotest_lib.client.common_lib import utils, error - -import kvm_log -import kvm_utils -import ppm_utils -import scan_results - - -def run_boot(test, params, env): -vm = kvm_utils.env_get_vm(env, params.get("main_vm")) -if not vm: -raise error.TestError, "VM object not found in environment" -if not vm.is_alive(): -raise error.TestError, "VM seems to be dead; Test requires a living VM" - -kvm_log.info("Waiting for guest to be up...") - -session = kvm_utils.wait_for(vm.ssh_login, 240, 0, 2) -if not session: -raise error.TestFail, "Could not log into guest" - -kvm_log.info("Logged in") - -if params.get("reboot") == "yes": -# Send the VM's reboot command -session.sendline(vm.get_params().get("cmd_reboot")) -kvm_log.info("Reboot command sent; waiting for guest to go down...") - -if not kvm_utils.wait_for(lambda: not session.is_responsive(), 120, 0, 1): -raise error.TestFail, "Guest refuses to go down" - -session.close() - -kvm_log.info("Guest is down; waiting for it to go up again...") - -session = kvm_utils.wait_for(vm.ssh_login, 120, 0, 2) -if not session: -raise error.TestFail, "Could not log into guest after reboot" - -kvm_log.info("Guest is up again") - -session.close() - - -def run_migration(test, params, env): -src_vm_name = params.get("migration_src") -vm = kvm_utils.env_get_vm(env, src_vm_name) -if not vm: -raise error.TestError, "VM '%s' not found in environment" % src_vm_name -if not vm.is_alive(): -raise error.TestError, "VM '%s' seems to be dead; Test requires a living VM" % src_vm_name - -dest_vm_name = params.get("migration_dst") -dest_vm = kvm_utils.env_get_vm(env, dest_vm_name) -if not dest_vm: -raise error.TestError, "VM '%s' not found in environment" % dest_vm_name -if not dest_vm.is_alive(): -raise error.TestError, "VM '%s' seems to be dead; Test requires a living VM" % dest_vm_name - -pre_scrdump_filename = os.path.join(test.debugdir, "migration_pre.ppm") -post_scrdump_filename = os.path.join(test.debugdir, "migration_post.ppm") - -# See if migration is supported -s, o = vm.send_monitor_cmd("help info") -if not "info migrate" in o: -raise error.TestError, "Migration is not supported" - -# Log into guest and get the output of migration_test_command -kvm_log.info("Waiting for guest to be up...") - -session = kvm_utils.wait_for(vm.ssh_login, 240, 0, 2) -if not session: -raise error.TestFail, "Could not log into guest" - -kvm_log.info("Logged in") - -reference_output = session.get_command_output(params.get("migration_test_command")) -session.close() - -# Define the migration command -cmd = "migrate -d tcp:localhost:%d" % dest_vm.migration_port -kvm_log.debug("Migration command: %s" % cmd) - -# Migrate -s, o = vm.send_monitor_cmd(cmd) -if s: -kvm_log.error("Migration command failed (command: %r, output: %r)" % (cmd, o)) -raise error.TestFail, "Migration command failed" - -# Define some helper functions -def mig_finished(): -s, o = vm.send_monitor_cmd("info migrate") -if s: -return False -if "Migration status: active" in o: -return False -return True - -def mig_succeeded(): -s, o = vm.send_monitor_cmd("info migrate") -if s == 0 and "Migration status: completed" in o: -return True -return False - -def mig_failed(): -s, o = vm.send_monitor_cmd("info migrate") -if s == 0 and "Migration status: failed" in o: -return True -return False - -# Wait for migration to finish -if not kvm_utils.wait_for(mig_finished, 90, 2, 2, "Waiting for migration to finish..."): -raise error.TestFail, "Timeout elapsed while waiting for migration to finish" - -# Report migration status -if mig_succeeded(): -kvm_log.info("Migration finished successfully") -else: -if mig_failed(): -message = "Migration failed" -else: -message = "Migration ended with unknown status" -raise error.TestFail, message - -# Get 'post' screendump -dest_vm.send_monitor_cmd("screendump %s" % post_scrdump_filename) - -# Get 'pre' screendump -vm.send_monitor_cmd("screendump %s" % pre_scrdump_filename) - -# Kill the source VM -vm.send_monit
[PATCH] Added "stock" or existing test to ./kvm_tests/
autotest.py, boot.py, linux_s3.py, migration.py, yum_update.py --- client/tests/kvm_runtest_2/kvm_tests/autotest.py | 145 client/tests/kvm_runtest_2/kvm_tests/boot.py | 45 ++ client/tests/kvm_runtest_2/kvm_tests/linux_s3.py | 53 +++ client/tests/kvm_runtest_2/kvm_tests/migration.py | 132 ++ client/tests/kvm_runtest_2/kvm_tests/yum_update.py | 53 +++ 5 files changed, 428 insertions(+), 0 deletions(-) create mode 100644 client/tests/kvm_runtest_2/kvm_tests/autotest.py create mode 100644 client/tests/kvm_runtest_2/kvm_tests/boot.py create mode 100644 client/tests/kvm_runtest_2/kvm_tests/linux_s3.py create mode 100644 client/tests/kvm_runtest_2/kvm_tests/migration.py create mode 100644 client/tests/kvm_runtest_2/kvm_tests/yum_update.py diff --git a/client/tests/kvm_runtest_2/kvm_tests/autotest.py b/client/tests/kvm_runtest_2/kvm_tests/autotest.py new file mode 100644 index 000..7497596 --- /dev/null +++ b/client/tests/kvm_runtest_2/kvm_tests/autotest.py @@ -0,0 +1,145 @@ +import time +import os + +#from autotest_lib.client.common_lib import utils, error + +#import kvm_log +#import kvm_utils +#import ppm_utils +#import scan_results + + +def run_autotest(test, params, env): +vm = kvm_utils.env_get_vm(env, params.get("main_vm")) +if not vm: +raise error.TestError, "VM object not found in environment" +if not vm.is_alive(): +raise error.TestError, "VM seems to be dead; Test requires a living VM" + +kvm_log.info("Logging into guest...") + +session = kvm_utils.wait_for(vm.ssh_login, 240, 0, 2) +if not session: +raise error.TestFail, "Could not log into guest" + +kvm_log.info("Logged in") + +# Collect some info +test_name = params.get("test_name") +test_timeout = int(params.get("test_timeout", 300)) +test_control_file = params.get("test_control_file", "control") +tarred_autotest_path = "/tmp/autotest.tar.bz2" +tarred_test_path = "/tmp/%s.tar.bz2" % test_name + +# tar the contents of bindir/autotest +cmd = "cd %s; tar cvjf %s autotest/*" +cmd += " --exclude=autotest/tests" +cmd += " --exclude=autotest/results" +cmd += " --exclude=autotest/tmp" +cmd += " --exclude=autotest/control" +cmd += " --exclude=*.pyc" +cmd += " --exclude=*.svn" +cmd += " --exclude=*.git" +kvm_utils.run_bg(cmd % (test.bindir, tarred_autotest_path), timeout=30) + +# tar the contents of bindir/autotest/tests/ +cmd = "cd %s; tar cvjf %s %s/*" +cmd += " --exclude=*.pyc" +cmd += " --exclude=*.svn" +cmd += " --exclude=*.git" +kvm_utils.run_bg(cmd % (os.path.join(test.bindir, "autotest", "tests"), tarred_test_path, test_name), timeout=30) + +# Check if we need to copy autotest.tar.bz2 +copy = False +output = session.get_command_output("ls -l autotest.tar.bz2") +if "such file" in output: +copy = True +else: +size = int(output.split()[4]) +if size != os.path.getsize(tarred_autotest_path): +copy = True +# Perform the copy +if copy: +kvm_log.info("Copying autotest.tar.bz2 to guest (file is missing or has a different size)...") +if not vm.scp_to_remote(tarred_autotest_path, ""): +raise error.TestFail, "Could not copy autotest.tar.bz2 to guest" + +# Check if we need to copy .tar.bz2 +copy = False +output = session.get_command_output("ls -l %s.tar.bz2" % test_name) +if "such file" in output: +copy = True +else: +size = int(output.split()[4]) +if size != os.path.getsize(tarred_test_path): +copy = True +# Perform the copy +if copy: +kvm_log.info("Copying %s.tar.bz2 to guest (file is missing or has a different size)..." % test_name) +if not vm.scp_to_remote(tarred_test_path, ""): +raise error.TestFail, "Could not copy %s.tar.bz2 to guest" % test_name + +# Extract autotest.tar.bz2 +kvm_log.info("Extracting autotest.tar.bz2...") +status = session.get_command_status("tar xvfj autotest.tar.bz2") +if status != 0: +raise error.TestFail, "Could not extract autotest.tar.bz2" + +# mkdir autotest/tests +session.sendline("mkdir autotest/tests") + +# Extract .tar.bz2 into autotest/tests +kvm_log.info("Extracting %s.tar.bz2..." % test_name) +status = session.get_command_status("tar xvfj %s.tar.bz2 -C autotest/tests" % test_name) +if status != 0: +raise error.TestFail, "Could not extract %s.tar.bz2" % test_name + +# Run the test +kvm_log.info("Running test '%s'..." % test_name) +session.sendline("cd autotest/tests/%s" % test_name) +session.sendline("rm -f ./%s.state" % test_control_file) +session.read_up_to_prompt() +session.sendline("../../bin/autotest ./%s" % test_control_file) +kvm_log.info(" Test output ") +match, output = session.read_up_to_prompt(
Re: [KVM PATCH v10] kvm: add support for irqfd
On Wed, May 20, 2009 at 10:30:49AM -0400, Gregory Haskins wrote: > +static int > +irqfd_wakeup(wait_queue_t *wait, unsigned mode, int sync, void *key) > +{ > + struct _irqfd *irqfd = container_of(wait, struct _irqfd, wait); > + > + /* > + * The wake_up is called with interrupts disabled. Therefore we need > + * to defer the IRQ injection until later since we need to acquire the > + * kvm->lock to do so. > + */ > + schedule_work(&irqfd->work); > + > + return 0; > +} This schedule_work is there just to work around the spinlock in eventfd_signal, which we don't really need. Isn't this right? And this is on each interrupt. Seems like a pity. How about a flag in eventfd that would convert it from waking up someone to a plain function call? Davide, could we add something like diff --git a/fs/eventfd.c b/fs/eventfd.c index 2a701d5..8bfa308 100644 --- a/fs/eventfd.c +++ b/fs/eventfd.c @@ -29,6 +29,7 @@ struct eventfd_ctx { */ __u64 count; unsigned int flags; + int nolock; }; /* @@ -46,6 +47,12 @@ int eventfd_signal(struct file *file, int n) if (n < 0) return -EINVAL; + if (ctx->nolock) { + /* Whoever set nolock + better set wqh.func as well. */ + ctx->wqh.func(&ctx->wqh, 0, 0, NULL); + return 0; + } spin_lock_irqsave(&ctx->wqh.lock, flags); if (ULLONG_MAX - ctx->count < n) n = (int) (ULLONG_MAX - ctx->count); -- MST -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [KVM PATCH v10] kvm: add support for irqfd
Michael S. Tsirkin wrote: > On Wed, May 20, 2009 at 10:30:49AM -0400, Gregory Haskins wrote: > >> +static int >> +irqfd_wakeup(wait_queue_t *wait, unsigned mode, int sync, void *key) >> +{ >> +struct _irqfd *irqfd = container_of(wait, struct _irqfd, wait); >> + >> +/* >> + * The wake_up is called with interrupts disabled. Therefore we need >> + * to defer the IRQ injection until later since we need to acquire the >> + * kvm->lock to do so. >> + */ >> +schedule_work(&irqfd->work); >> + >> +return 0; >> +} >> > > This schedule_work is there just to work around the spinlock > in eventfd_signal, which we don't really need. Isn't this right? > Yep. > And this is on each interrupt. Seems like a pity. > I agree. Moving towards a way to be able to inject without deferring to a workqueue will be a good thing. Note, however, that addressing it at the eventfd/wqh-lock layer is only part of the picture since ideally we can inject (i.e. eventfd_signal()) from any atomic context (e.g. hard-irq), not just the artificial one created by the wqh based implementation. I think Marcelo's irq_lock split-up code is taking us in that direction by (eventually) allowing the kvm_set_irq() path to be atomic-context friendly. > How about a flag in eventfd that would > convert it from waking up someone to a plain function call? > > Davide, could we add something like > > > diff --git a/fs/eventfd.c b/fs/eventfd.c > index 2a701d5..8bfa308 100644 > --- a/fs/eventfd.c > +++ b/fs/eventfd.c > @@ -29,6 +29,7 @@ struct eventfd_ctx { >*/ > __u64 count; > unsigned int flags; > + int nolock; > }; > > /* > @@ -46,6 +47,12 @@ int eventfd_signal(struct file *file, int n) > > if (n < 0) > return -EINVAL; > + if (ctx->nolock) { > + /* Whoever set nolock > + better set wqh.func as well. */ > + ctx->wqh.func(&ctx->wqh, 0, 0, NULL); > + return 0; > + } > spin_lock_irqsave(&ctx->wqh.lock, flags); > if (ULLONG_MAX - ctx->count < n) > n = (int) (ULLONG_MAX - ctx->count); > > If we think we still need to address it at the eventfd layer (which I am not 100% convinced we do), I think we should probably generalize it a little more and make it so it doesn't completely re-route the notification (there may be other end-points interrested in the event, I suppose). I am thinking something along the lines that the internal eventfd uses an srcu_notifier, and we register a default notifier which points to a wqh path very much like what we have today. Then something like kvm could register an additional srcu_notifier which should allow it to be invoked lockless (*). This would theoretically allow the eventfd to remain free to support an arbitrary number of end-points which support both locked and lockless operation. -Greg (*) disclaimer: I've never looked at the srcu_notifier implementation, so perhaps this is not what they really offer. I base this only on basic RCU understanding. signature.asc Description: OpenPGP digital signature
[KVM PATCH v4 0/3] iosignalfd
(Applies to kvm.git/master:74dfca0a) This is v4 of the series. For more details, please see the header to patch 3/3. This series has been tested against the kvm-eventfd unit test, and appears to be functioning properly. You can download this test here: ftp://ftp.novell.com/dev/ghaskins/kvm-eventfd.tar.bz2 This series is ready to be considered for inclusion, pending any further review comments. [ Changelog: v4: *) Fixed a bug in the original 2/4 where the PIT failure case would potentially leave the io_bus components registered. *) Condensed the v3 2/4 and 3/4 into one patch (2/2) since the patches became interdependent with the fix described above *) Rebased to kvm.git/master:74dfca0a v3: *) fixed patch 2/4 to handle error cases instead of BUG_ON *) implemented same HAVE_EVENTFD protection mechanism as irqfd to prevent compilation errors on unsupported arches *) completed testing *) rebased to kvm.git/master:7391a6d5 v2: *) added optional data-matching capability (via cookie field) *) changed name from iofd to iosignalfd *) added io_bus unregister function *) implemented deassign feature v1: *) original release (integrated into irqfd v7 series as "iofd") ] --- Gregory Haskins (3): kvm: add iosignalfd support kvm: make io_bus interface more robust eventfd: export eventfd interfaces for module use arch/x86/kvm/i8254.c | 34 +++-- arch/x86/kvm/i8259.c |9 ++- arch/x86/kvm/x86.c|1 fs/eventfd.c |3 + include/linux/kvm.h | 15 include/linux/kvm_host.h | 18 - virt/kvm/coalesced_mmio.c |8 ++ virt/kvm/eventfd.c| 162 + virt/kvm/ioapic.c |9 ++- virt/kvm/kvm_main.c | 60 ++--- 10 files changed, 290 insertions(+), 29 deletions(-) -- Signature -- 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