Re: [kvm-devel] [RFC] Reworking KVM_DEBUG_GUEST
On Wednesday 14 May 2008 10:28:51 Jan Kiszka wrote: So gdb on power relies only on those few hw-breakpoints? With x86 you can perfectly run gdb (with soft BPs) in parallel with the gdbstub (currently based on hw-BPs, but the same would be true for soft-BPs inserted by the gdbstub). GDB on Power inserts trap instructions, i.e. standard soft breakpoints. It does not rely on the hardware breakpoints. It gets a little more complicated when you involve a GDB stub. IIRC, GDB will ask the stub to set the breakpoint, and if the stub doesn't support it, GDB will fall back to overwriting the instructions in memory. Does the Qemu GDB stub advertise breakpoint support? If not, the only support needed in KVM would be to send all debug interrupts to qemu, and allow qemu to send them back down for in-guest breakpoints. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [RFC] Reworking KVM_DEBUG_GUEST
On Wednesday 14 May 2008 14:10:06 Jan Kiszka wrote: Hollis Blanchard wrote: On Wednesday 14 May 2008 10:28:51 Jan Kiszka wrote: So gdb on power relies only on those few hw-breakpoints? With x86 you can perfectly run gdb (with soft BPs) in parallel with the gdbstub (currently based on hw-BPs, but the same would be true for soft-BPs inserted by the gdbstub). GDB on Power inserts trap instructions, i.e. standard soft breakpoints. It does not rely on the hardware breakpoints. It gets a little more complicated when you involve a GDB stub. IIRC, GDB will ask the stub to set the breakpoint, and if the stub doesn't support it, GDB will fall back to overwriting the instructions in memory. Does the Qemu GDB stub advertise breakpoint support? Yes, QEMU reacts on both Z0 (soft-BP) and Z1 (hard-BP). That's something even gdbserver does not do! It just handles watchpoints (Z2..4). If not, the only support needed in KVM would be to send all debug interrupts to qemu, and allow qemu to send them back down for in-guest breakpoints. Simply returning unsupported on Z0 is not yet enough for x86, KVM's kernel side should not yet inform QEMU about soft-BP exceptions. But in theory, this should be easily fixable (or is already the case for other archs). And it would safe us from keeping track of N software breakpoints, where N could even become larger than 32, the current hardcoded limit for plain QEMU. :) Meanwhile I realized that the proposed KVM_DEBUG_GUEST API is insufficient: We need a return channel for the debug register state (specifically to figure out details about hit watchpoints). I'm now favoring KVM_SET_DEBUG and KVM_GET_DEBUG as two new IOCTLs, enabling us to write _and_ read-back the suggested data structure. How about simply extending kvm_exit.debug to contain the virtual address of the breakpoint hit? In Qemu, when exit_reason == KVM_EXIT_DEBUG, it would just need to see if that address is for a breakpoint Qemu set or not. If so, it's happy. If not, (commence handwaving) tell KVM to forward the debug interrupt to the guest. This way, the list of breakpoints is maintained in userspace (in the qemu gdb stub), which is nice because it could be arbitrarily large. Also, this is not specific to hardware debug registers: soft and hard breakpoint interrupts would follow the same path. There's still a question of whether the GDB stub should set the breakpoint itself (Z0/Z1) or force GDB to modify memory, but either way the KVM code is simple. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [RFC] Reworking KVM_DEBUG_GUEST
On Wednesday 14 May 2008 14:49:02 Jan Kiszka wrote: In Qemu, when exit_reason == KVM_EXIT_DEBUG, it would just need to see if that address is for a breakpoint Qemu set or not. If so, it's happy. If not, (commence handwaving) tell KVM to forward the debug interrupt to the guest. This way, the list of breakpoints is maintained in userspace (in the qemu gdb stub), which is nice because it could be arbitrarily large. Yes, but I would rather pass the debug registers (more general: some arch dependent state set) back in this slot. Those contain everything the gdbstub needs to know to catch relevant hardware-BP/watchpoint events (and report them to the gdb frontend). But what would the stub *do* with the contents of the debug registers? The only reason they were set is on behalf of the stub in the first place. In fact, in the case of soft breakpoints, KVM doesn't even know where all the set breakpoints are. The only thing KVM needs to report is the address of the breakpoint that was just hit. Sorry if this gets formatted badly: gdb qemu stub KVM break *0xf00 sends Z0 packet 0xf00 0xf00 - BP list ioctl(KVM_DEBUG, 0xf00) continue ioctl(KVM_RUN) running... breakpoint hit exit_reason = KVM_EXIT_DEBUG kvm_run.debug.address = current PC value search BP list for address bp hit ---presentnot present --- send debug interrupt to guest Notes: - KVM_DEBUG in this case will set a hardware breakpoint. The alternative is to write an int3 into guest memory. - The stub doesn't care how the hardware registers were configured. All it needs to know is a) that a breakpoint was hit, and b) at what address. Does this make sense? -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [RFC] Reworking KVM_DEBUG_GUEST
On Wednesday 14 May 2008 16:06:00 Hollis Blanchard wrote: In fact, in the case of soft breakpoints, KVM doesn't even know where all the set breakpoints are. Side note: I'm retract this sentence: I wrote it before I sketched out the pseudocode, and forgot to remove it. :) -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [RFC] Reworking KVM_DEBUG_GUEST
On Wednesday 14 May 2008 16:11:39 Hollis Blanchard wrote: On Wednesday 14 May 2008 16:06:00 Hollis Blanchard wrote: In fact, in the case of soft breakpoints, KVM doesn't even know where all the set breakpoints are. Side note: I'm retract this sentence: I wrote it before I sketched out the pseudocode, and forgot to remove it. :) Er no, let me try that again: In my proposed design, the stub needs to know where all breakpoints are, HW or SW. (That means it must implement Z0/Z1.) However, KVM itself doesn't need to know any of that: all breakpoints are referred to the stub for handling, and the stub will notify KVM if further action is needed in-kernel. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] kvm trace support for ppc
On Tuesday 13 May 2008 03:06:19 Tan, Li wrote: Hollisb, I have 2 more questions: 1. seems record won't be overwritten because current code is as following: /* * The relay channel is used in no-overwrite mode, it keeps trace of how * many times we encountered a full subbuffer, to tell user space app the * lost records there were. */ static int kvm_subbuf_start_callback(struct rchan_buf *buf, void *subbuf, void *prev_subbuf, size_t prev_padding) { struct kvm_trace *kt; if (!relay_buf_full(buf)) return 1; kt = buf-chan-private_data; atomic_inc(kt-lost_records); return 0; } 2. Then needn't expose debugfs entry kvmtrace-metadata, we can use existing relayfs to output struct metadata with kmagic, if we update code as following? static int kvm_subbuf_start_callback(struct rchan_buf *buf, void *subbuf, void *prev_subbuf, size_t prev_padding) { struct kvm_trace *kt; if (!relay_buf_full(buf)) { if (!prev_subbuf) { //here is executed only once (when the channel is opened) subbuf_start_reserve(buf, sizeof(struct metadata)); ((struct metadata *)subbuf)-kmagic = 0x1234; } return 1; } kt = buf-chan-private_data; atomic_inc(kt-lost_records); return 0; } Ah, I didn't understand what the lost records handling was about. Given that it won't be lost, it would be OK for the kernel to export the header, and in that case I guess you would want it to be the same size as the other records. I'm not sure how I feel about that from a layering point of view, but at least it would be functional. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 0 of 2] [RESEND] [PowerPC] Fix setting memory for bamboo board model
On Monday 05 May 2008 11:04:52 Jerone Young wrote: These patches fell through the cracks. This set of patches fixes setting memory for PowerPC bamboo board model. Besides just setting memory in qemu, you must also set it in the device tree. This sets the memory in the device tree so that it can be something other then the hard coded memory size of 144MB. Signed-off-by: Jerone Young [EMAIL PROTECTED] Acked-by: Hollis Blanchard [EMAIL PROTECTED] Avi, please apply to kvm-userspace; thanks. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Handle vma regions with no backing page (v2)
On Tuesday 29 April 2008 18:12:51 Anthony Liguori wrote: IIUC PPC correctly, all IO pages have corresponding struct pages. This means that get_user_pages() would succeed and you can reference count them? In this case, we would never take the VM_PFNMAP path. Is that correct? I think Andrea's answered this, but for the record: I believe ioremap() will get you struct pages on PPC, but they don't automatically exist. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH] Fix kvm-userspace configure script so that cc=gcc
On Wednesday 30 April 2008 15:53:47 Jerone Young wrote: 1 file changed, 1 insertion(+), 1 deletion(-) configure |2 +- This fixes compilation for cross compilers as many do not create a ${cross_prefix}cc link. But the do a ${cross_prefix}gcc. This is what the kernel does so this will work for everyone. This breaks some who do not have a cc link (cross tools does not create), when I put a patch to remove libkvm dependence on test config.mak. Signed-off-by: Jerone Young [EMAIL PROTECTED] diff --git a/configure b/configure --- a/configure +++ b/configure @@ -2,7 +2,7 @@ prefix=/usr/local kerneldir=/lib/modules/$(uname -r)/build -cc=cc +cc=gcc ld=ld objcopy=objcopy want_module=1 To clarify: there is no such thing as ${cross_prefix}cc, so the configure script is currently broken for cross-compiling. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH] Update kernel/Makefile and remove x86 only entries
On Wednesday 30 April 2008 15:16:09 Avi Kivity wrote: hack = $(call _hack,$T/$(strip $1)) + +ifneq '$(filter $(ARCH_DIR), x86)' '' +HACK_FILES = kvm_main.c \ + mmu.c \ + vmx.c \ + svm.c \ + x86.c \ + irq.h +endif hack-files-x86 = ... hack-files-ppc = ... hack-files = $(hack-files-$(ARCH_DIR)) Agreed; this is exactly what I had suggested previously. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 2 of 3] Add function dt_cell_multi to hw/device_tree.c
On Monday 28 April 2008 16:23:04 Jerone Young wrote: +/* This function is to manipulate a cell with multiple values */ +void dt_cell_multi(void *fdt, char *node_path, char *property, + uint32_t *val_array, int size) +{ + + int offset; + int ret; Could you please be more careful with your whitespace? -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
[kvm-devel] Fwd: [kvm-ppc-devel] [PATCH] kvmppc: deliver INTERRUPT_FP_UNAVAIL to the guest
Acked-by: Hollis Blanchard [EMAIL PROTECTED] Avi, please apply for 2.6.26. -- Hollis Blanchard IBM Linux Technology Center ---BeginMessage--- From: Christian Ehrhardt [EMAIL PROTECTED] This patch adds the delivery of INTERRUPT_FP_UNAVAIL exceptions to the guest. It's needed if a guest uses ppc binaries using the Floating point instructions. Signed-off-by: Christian Ehrhardt [EMAIL PROTECTED] --- [diffstat] [diff] booke_guest.c |5 + 1 files changed, 5 insertions(+) diff --git a/arch/powerpc/kvm/booke_guest.c b/arch/powerpc/kvm/booke_guest.c --- a/arch/powerpc/kvm/booke_guest.c +++ b/arch/powerpc/kvm/booke_guest.c @@ -340,6 +340,11 @@ int kvmppc_handle_exit(struct kvm_run *r default: BUG(); } + break; + + case BOOKE_INTERRUPT_FP_UNAVAIL: + kvmppc_queue_exception(vcpu, exit_nr); + r = RESUME_GUEST; break; case BOOKE_INTERRUPT_DATA_STORAGE: - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-ppc-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel ---End Message--- - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 2 of 3] Add PowerPC KVM guest wait handling support
On Friday 25 April 2008 00:56:03 Jerone Young wrote: This patch handles a guest that is in a wait state. This ensures that the guest is not allways eating up 100% cpu when it is idle. Signed-off-by: Jerone Young [EMAIL PROTECTED] diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c --- a/arch/powerpc/kvm/emulate.c +++ b/arch/powerpc/kvm/emulate.c @@ -265,6 +265,11 @@ int kvmppc_emulate_instruction(struct kv case 146: /* mtmsr */ rs = get_rs(inst); kvmppc_set_msr(vcpu, vcpu-arch.gpr[rs]); + + /* handle guest vcpu that is in wait state */ + if (vcpu-arch.msr MSR_WE) { + kvm_vcpu_block(vcpu); + } break; case 163: /* wrteei */ So if I apply this patch and not #3, the guest will put itself to sleep and never wake up? You need to combine patches 2 and 3. Also, for completeness, you should add the same test to the rfi emulation, which could (theoretically) also set MSR[WE]. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 3 of 3] Add premption handlers properly wake sleeping guest
On Friday 25 April 2008 00:56:04 Jerone Young wrote: diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -212,6 +212,9 @@ static void kvmppc_decrementer_func(unsi { struct kvm_vcpu *vcpu = (struct kvm_vcpu *)data; + if (waitqueue_active(vcpu-wq)) + wake_up_interruptible(vcpu-wq); + kvmppc_queue_exception(vcpu, BOOKE_INTERRUPT_DECREMENTER); } Hooray! int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu, struct kvm_interrupt *irq) { + vcpu_load(vcpu); kvmppc_queue_exception(vcpu, BOOKE_INTERRUPT_EXTERNAL); + vcpu_put(vcpu); + return 0; } load/put here is definitely unnecessary. That makes me question how necessary it is in other parts of this patch too. I think the (hardware) TLB is the only state we really need to worry about, because there is no other state that our guest can load into the hardware that is not handled by a regular context switch. If that's true, we should only need vcpu_load/put() on paths where we muck with the TLB behind the host's back, and that is only in the run path. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 0 of 3] Fix guest eating 100% cpu when guest is idle on PowerPC
On Friday 25 April 2008 00:56:01 Jerone Young wrote: This set of patches fixes 100% CPU usage when a guest is idle on PowerPC. This time it uses common kvm functions to sleep the guest. Looking much better now, with just a few minor issues to correct. With these patches applied, about how much CPU *does* an idling guest consume? By the way, you don't explicitly *unset* MSR[WE]. I think this works implicitly because of the way we deliver interrupts; could you add a comment explaining that? -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] Moving kvm lists to kernel.org?
On Thursday 24 April 2008 07:54:10 Avi Kivity wrote: I propose moving the kvm lists to vger.kernel.org, for the following benefits: - better spam control - faster service (I see significant lag with the sourceforge lists) - no ads appended to the end of each email If no objections are raised, and if the vger postmasters agree, I will mass subscribe the current subscribers so that there will be no service interruption. Sounds good to me. Will we continue to have manual moderation for non-subscribers? That seems to be a good way to handle the last 1% of spam that sneaks through the automated filters. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [RFC PATCH] virtio: change config to guest endian.
On Tuesday 22 April 2008 06:22:48 Avi Kivity wrote: Rusty Russell wrote: [Christian, Hollis, how much is this ABI breakage going to hurt you?] A recent proposed feature addition to the virtio block driver revealed some flaws in the API, in particular how easy it is to break big endian machines. The virtio config space was originally chosen to be little-endian, because we thought the config might be part of the PCI config space for virtio_pci. It's actually a separate mmio region, so that argument holds little water; as only x86 is currently using the virtio mechanism, we can change this (but must do so now, before the impending s390 and ppc merges). This will probably annoy Hollis which has guests that can go both ways. Rusty and I have discussed it. Ultimately, this just takes us from a cross-architecture endianness definition to a per-architecture definition. Anyways, we've already fallen into this situation with the virtio ring data itself, so we're really saying same endianness as the ring. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [RFC PATCH] virtio: change config to guest endian.
On Tuesday 22 April 2008 09:31:35 Rusty Russell wrote: On Tuesday 22 April 2008 21:22:48 Avi Kivity wrote: The virtio config space was originally chosen to be little-endian, because we thought the config might be part of the PCI config space for virtio_pci. It's actually a separate mmio region, so that argument holds little water; as only x86 is currently using the virtio mechanism, we can change this (but must do so now, before the impending s390 and ppc merges). This will probably annoy Hollis which has guests that can go both ways. Yes, I discussed this with Hollis. But the virtio rings themselves already have this issue: we don't do any endian conversion on them and assume they're our endian in the guest. We may still regret not doing *everything* little-endian, but this doesn't make it worse. Hmm, why *don't* we just do everything LE, including the ring? -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [RFC PATCH] virtio: change config to guest endian.
On Tuesday 22 April 2008 16:05:38 Rusty Russell wrote: On Wednesday 23 April 2008 06:29:14 Hollis Blanchard wrote: On Tuesday 22 April 2008 09:31:35 Rusty Russell wrote: We may still regret not doing *everything* little-endian, but this doesn't make it worse. Hmm, why *don't* we just do everything LE, including the ring? Mainly because when requirements are in doubt, simplicity wins, I think. Well, I think the definition of simplicity is up for debate in this case... LE everywhere is much simpler than it depends, IMHO. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [RFC PATCH] virtio: change config to guest endian.
On Tuesday 22 April 2008 17:13:01 Anthony Liguori wrote: Hollis Blanchard wrote: On Tuesday 22 April 2008 16:05:38 Rusty Russell wrote: On Wednesday 23 April 2008 06:29:14 Hollis Blanchard wrote: On Tuesday 22 April 2008 09:31:35 Rusty Russell wrote: We may still regret not doing *everything* little-endian, but this doesn't make it worse. Hmm, why *don't* we just do everything LE, including the ring? Mainly because when requirements are in doubt, simplicity wins, I think. Well, I think the definition of simplicity is up for debate in this case... LE everywhere is much simpler than it depends, IMHO. You couldn't use the vringfd direct ring mapping optimization in KVM for PPC without teaching the kernel to access a vring in LE format. I'm pretty sure the later would get rejected on LKML anyway for vringfd as a generic mechanism. You mean vringfd for use cases other than virtual IO drivers? I have a poor imagination; can you give some examples? Even then, it should be possible to have VIO drivers use a different set of accessors, just like there are swapping and non-swapping accessors for real IO, so I still don't see the problem. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [RFC PATCH] virtio: change config to guest endian.
On Tuesday 22 April 2008 17:13:01 Anthony Liguori wrote: Hollis Blanchard wrote: On Tuesday 22 April 2008 16:05:38 Rusty Russell wrote: On Wednesday 23 April 2008 06:29:14 Hollis Blanchard wrote: On Tuesday 22 April 2008 09:31:35 Rusty Russell wrote: We may still regret not doing *everything* little-endian, but this doesn't make it worse. Hmm, why *don't* we just do everything LE, including the ring? Mainly because when requirements are in doubt, simplicity wins, I think. Well, I think the definition of simplicity is up for debate in this case... LE everywhere is much simpler than it depends, IMHO. You couldn't use the vringfd direct ring mapping optimization in KVM for PPC without teaching the kernel to access a vring in LE format. I'm pretty sure the later would get rejected on LKML anyway for vringfd as a generic mechanism. (Since the IA64 guys have already implemented BE guests on LE hosts, they should be aware of this discussion too, which is why I've CCed them.) After a short but torturous whiteboard session, followed by a much longer but less painful discussion, I'm fine with the virtio device config space being BE for PowerPC and LE for x86. In the future, we can use a feature bit to indicate that PCI config space contains an explicit endianness flag. (This will be set to BE or LE, *not* to opposite of normal, because normal is also too vague.) -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
[kvm-devel] [PATCH] [QEMU POWERPC] FPRs no longer live in kvm_vcpu
Signed-off-by: Hollis Blanchard [EMAIL PROTECTED] diff --git a/qemu/qemu-kvm-powerpc.c b/qemu/qemu-kvm-powerpc.c --- a/qemu/qemu-kvm-powerpc.c +++ b/qemu/qemu-kvm-powerpc.c @@ -72,7 +72,6 @@ for (i = 0;i 32; i++){ regs.gpr[i] = env-gpr[i]; -regs.fpr[i] = env-fpr[i]; } rc = kvm_set_regs(kvm_context, env-cpu_index, regs); @@ -113,7 +112,6 @@ for (i = 0;i 32; i++){ env-gpr[i] = regs.gpr[i]; -env-fpr[i] = regs.fpr[i]; } } - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
[kvm-devel] [PATCH] [POWERPC KVM] Kconfig fixes
1 file changed, 5 insertions(+), 6 deletions(-) arch/powerpc/kvm/Kconfig | 11 +-- Don't allow building as a module (asm-offsets dependencies). Also, automatically select KVM_BOOKE_HOST until we better separate the guest and host layers. Signed-off-by: Hollis Blanchard [EMAIL PROTECTED] diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig --- a/arch/powerpc/kvm/Kconfig +++ b/arch/powerpc/kvm/Kconfig @@ -15,10 +15,12 @@ if VIRTUALIZATION config KVM - tristate Kernel-based Virtual Machine (KVM) support - depends on EXPERIMENTAL + bool Kernel-based Virtual Machine (KVM) support + depends on 44x EXPERIMENTAL select PREEMPT_NOTIFIERS select ANON_INODES + # We can only run on Book E hosts so far + select KVM_BOOKE_HOST ---help--- Support hosting virtualized guest machines. You will also need to select one or more of the processor modules below. @@ -26,13 +28,10 @@ This module provides access to the hardware capabilities through a character device node named /dev/kvm. - To compile this as a module, choose M here: the module - will be called kvm. - If unsure, say N. config KVM_BOOKE_HOST - tristate KVM host support for Book E PowerPC processors + bool KVM host support for Book E PowerPC processors depends on KVM 44x ---help--- Provides host support for KVM on Book E PowerPC processors. Currently - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 1/5]Add some trace markers and exposeinterfaces in kernel for tracing
On Wednesday 16 April 2008 01:45:34 Liu, Eric E wrote: Hmm, while we're on the subject, I'm not sure what the best way to automatically byteswap will be. It probably isn't worth it to convert all trace data to a standard ordering (which would add overhead to tracing), but I suppose there is no metadata in the trace log? A command line switch might be inconvenient but inevitable. A tricky approach is that we insert medadata to the trace file before reading the trace log, so that the analysis tool can look at the medadata to check whether we need to convert byte order? Actually, can't we lose trace records? It would be unfortunate if the magic metadata record was overwritten by the trace data. Perhaps a 1-byte flags variable at the beginning of each record could indicate the data's endianness? Another option would be to have the kvmtrace tool transcribe the data itself. I think that would be fine, since kvmtrace must run on the target anyways, but your kvmtrace implementation doesn't actually understand the format of the data. Actually... we could have kvmtrace itself insert the metadata, so there would be no chance of it being overwritten in the kernel buffers. The header could be written in tip_open_output(), and update fs_size accordingly. Do you have any suggestions for the format of the metadata? I'm not sure how it should fit into the record format expected by kvmtrace_format. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 1/2] kvm-s390: provide ge t/set_mp_state stubs to fix compile error
By the way Marcelo, it would be polite to provide these stubs yourself to avoid breaking the build on other architectures. It looks like IA64 is still broken because of this. -- Hollis Blanchard IBM Linux Technology Center On Wednesday 16 April 2008 09:06:34 Carsten Otte wrote: From: Christian Borntraeger [EMAIL PROTECTED] Since commit ded6fb24fb694bcc5f308a02ec504d45fbc8aaa6 Author: Marcelo Tosatti [EMAIL PROTECTED] Date: Fri Apr 11 13:24:45 2008 -0300 KVM: add ioctls to save/store mpstate kvm does not compile on s390. This patch provides ioctl stubs for s390 to make kvm.git compile again. As migration is not yet supported, the ioctl definitions are empty. Signed-off-by: Christian Borntraeger [EMAIL PROTECTED] Signed-off-by: Carsten Otte [EMAIL PROTECTED] --- arch/s390/kvm/kvm-s390.c | 12 1 file changed, 12 insertions(+) Index: kvm/arch/s390/kvm/kvm-s390.c === --- kvm.orig/arch/s390/kvm/kvm-s390.c +++ kvm/arch/s390/kvm/kvm-s390.c @@ -414,6 +414,18 @@ int kvm_arch_vcpu_ioctl_debug_guest(stru return -EINVAL; /* not implemented yet */ } +int kvm_arch_vcpu_ioctl_get_mpstate(struct kvm_vcpu *vcpu, + struct kvm_mp_state *mp_state) +{ + return -EINVAL; /* not implemented yet */ +} + +int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu, + struct kvm_mp_state *mp_state) +{ + return -EINVAL; /* not implemented yet */ +} + static void __vcpu_run(struct kvm_vcpu *vcpu) { memcpy(vcpu-arch.sie_block-gg14, vcpu-arch.guest_gprs[14], 16); - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] add virtio disk geometry feature
On Wednesday 16 April 2008 16:32:30 Anthony Liguori wrote: diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -157,10 +157,25 @@ static int virtblk_ioctl(struct inode *i /* We provide getgeo only to please some old bootloader/partitioning tools */ static int virtblk_getgeo(struct block_device *bd, struct hd_geometry *geo) { - /* some standard values, similar to sd */ - geo-heads = 1 6; - geo-sectors = 1 5; - geo-cylinders = get_capacity(bd-bd_disk) 11; + struct virtio_blk *vblk = bd-bd_disk-private_data; + struct virtio_blk_geometry vgeo; + int err; + + /* see if the host passed in geometry config */ + err = virtio_config_val(vblk-vdev, VIRTIO_BLK_F_GEOMETRY, + offsetof(struct virtio_blk_config, geometry), + vgeo); + + if (!err) { + geo-heads = vgeo.heads; + geo-sectors = vgeo.sectors; + geo-cylinders = vgeo.cylinders; + } else { + /* some standard values, similar to sd */ + geo-heads = 1 6; + geo-sectors = 1 5; + geo-cylinders = get_capacity(bd-bd_disk) 11; + } return 0; } You're probably breaking PPC since the values in the config space are in little endian format. virtio_config_val does automagic endianness conversion if the size is 2, 4, or 8. In this case, the structure size is 4 so the endianness conversion will do the wrong thing. Good catch; byte-swapping an entire structure is a terrible terrible idea. Magic endianness conversion based on read size is looking pretty evil to me... Perhaps we need explicit *_val[8,16,32,64]? Implicit byteswapping based on access size is the standard way of implementing accessors. In this case, reading each structure member individually will do the right implicit swapping, rather than trying to load the whole thing as a single access. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] [kvm-userspace] Make make sync in kernel dir work for multiple archs
On Monday 14 April 2008 21:46:43 Jerone Young wrote: 1 file changed, 13 insertions(+), 5 deletions(-) kernel/Makefile | 18 +- This patch add the ability for make sync in the kernel directory to work for mulitiple architectures and not just x86. Signed-off-by: Jerone Young [EMAIL PROTECTED] diff --git a/kernel/Makefile b/kernel/Makefile --- a/kernel/Makefile +++ b/kernel/Makefile @@ -1,5 +1,10 @@ include ../config.mak include ../config.mak +ASM_DIR=$(ARCH) +ifneq '$(filter $(ASM_DIR), x86_64 i386 ia64)' '' + ASM_DIR=x86 +endif Minor complaint: ASM_DIR really isn't. You use it as arch/$(ASM_DIR) and also as include/asm-$(ASM_DIR). I think what you really meant is ARCH_DIR (or similar). +ifneq '$(filter $(ASM_DIR), x86_64 i386 ia64)' '' $(call unifdef, include/linux/kvm.h) $(call unifdef, include/linux/kvm_para.h) $(call unifdef, include/asm-x86/kvm.h) @@ -54,6 +60,8 @@ sync: $(call hack, svm.c) $(call hack, x86.c) $(call hack, irq.h) +endif + Why are you keeping IA64 touching asm-x86? What happened to my suggestion of creating a per-arch HACK_FILES and UNIFDEF_FILES variables, and looping over those? -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] [kvm-userspace] Make make sync in kernel dir work for multiple archs
On Tuesday 15 April 2008 11:20:58 Jerone Young wrote: What happened to my suggestion of creating a per-arch HACK_FILES and UNIFDEF_FILES variables, and looping over those? These macros are only for x86. We don't want them or need them. So I just left them be as not to accidentally miss or break anything. Right, they are only used for x86. So as I said before, create arch-specific HACK_FILES and UNIFDEF_FILES variables, and use those instead. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH] [v2 ][kvm-userspace] Make make sync in kern el dir work for multiple archs
On Tuesday 15 April 2008 14:43:12 Jerone Young wrote: 1 file changed, 31 insertions(+), 17 deletions(-) kernel/Makefile | 48 +++- This patch add the ability for make sync in the kernel directory to work for mulitiple architectures and not just x86. Signed-off-by: Jerone Young [EMAIL PROTECTED] diff --git a/kernel/Makefile b/kernel/Makefile --- a/kernel/Makefile +++ b/kernel/Makefile @@ -1,5 +1,10 @@ include ../config.mak include ../config.mak +ARCH_DIR=$(ARCH) +ifneq '$(filter $(ARCH_DIR), x86_64 i386)' '' + ARCH_DIR=x86 +endif + KVERREL = $(patsubst /lib/modules/%/build,%,$(KERNELDIR)) DESTDIR= @@ -18,11 +23,25 @@ _hack = mv $1 $1.orig \ | sed '/\#include/! s/\blapic\b/l_apic/g' $1 rm $1.orig _unifdef = mv $1 $1.orig \ - unifdef -DCONFIG_X86 $1.orig $1; \ + unifdef -DCONFIG_$(ARCH_DIR) $1.orig $1; \ [ $$? -le 1 ] rm $1.orig This isn't going to work because you've changed -DCONFIG_X86 to -DCONFIG_x86 . -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 1/5]Add some trace markers and expose interfaces in kernel for tracing
On Wednesday 09 April 2008 05:01:36 Liu, Eric E wrote: +/* This structure represents a single trace buffer record. */ +struct kvm_trace_rec { + __u32 event:28; + __u32 extra_u32:3; + __u32 cycle_in:1; + __u32 pid; + __u32 vcpu_id; + union { + struct { + __u32 cycle_lo, cycle_hi; + __u32 extra_u32[KVM_TRC_EXTRA_MAX]; + } cycle; + struct { + __u32 extra_u32[KVM_TRC_EXTRA_MAX]; + } nocycle; + } u; +}; Do we really need bitfields here? They are notoriously non-portable. Practically speaking, this will prevent me from copying a trace file from my big-endian target to my little-endian workstation for analysis, at least without some ugly hacking in the userland tool. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
[kvm-devel] [PATCH] [KVM] Rename debugfs_dir to kvm_debugfs_dir
3 files changed, 7 insertions(+), 7 deletions(-) include/linux/kvm_host.h |2 +- virt/kvm/kvm_main.c |8 virt/kvm/kvm_trace.c |4 ++-- # HG changeset patch # User Hollis Blanchard [EMAIL PROTECTED] # Date 1208293411 18000 # Node ID 524092a595b246f17ab56199e3afebded1e987a6 # Parent 8ad2f90233993539c3c919c2c303041611ecdcb4 [KVM] Rename debugfs_dir to kvm_debugfs_dir. It's a globally exported symbol now. Signed-off-by: Hollis Blanchard [EMAIL PROTECTED] diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -315,7 +315,7 @@ struct dentry *dentry; }; extern struct kvm_stats_debugfs_item debugfs_entries[]; -extern struct dentry *debugfs_dir; +extern struct dentry *kvm_debugfs_dir; #ifdef CONFIG_KVM_TRACE int kvm_trace_ioctl(unsigned int ioctl, unsigned long arg); diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -60,7 +60,7 @@ static __read_mostly struct preempt_ops kvm_preempt_ops; -struct dentry *debugfs_dir; +struct dentry *kvm_debugfs_dir; static long kvm_vcpu_ioctl(struct file *file, unsigned int ioctl, unsigned long arg); @@ -1392,9 +1392,9 @@ { struct kvm_stats_debugfs_item *p; - debugfs_dir = debugfs_create_dir(kvm, NULL); + kvm_debugfs_dir = debugfs_create_dir(kvm, NULL); for (p = debugfs_entries; p-name; ++p) - p-dentry = debugfs_create_file(p-name, 0444, debugfs_dir, + p-dentry = debugfs_create_file(p-name, 0444, kvm_debugfs_dir, (void *)(long)p-offset, stat_fops[p-kind]); } @@ -1405,7 +1405,7 @@ for (p = debugfs_entries; p-name; ++p) debugfs_remove(p-dentry); - debugfs_remove(debugfs_dir); + debugfs_remove(kvm_debugfs_dir); } static int kvm_suspend(struct sys_device *dev, pm_message_t state) diff --git a/virt/kvm/kvm_trace.c b/virt/kvm/kvm_trace.c --- a/virt/kvm/kvm_trace.c +++ b/virt/kvm/kvm_trace.c @@ -159,12 +159,12 @@ r = -EIO; atomic_set(kt-lost_records, 0); - kt-lost_file = debugfs_create_file(lost_records, 0444, debugfs_dir, + kt-lost_file = debugfs_create_file(lost_records, 0444, kvm_debugfs_dir, kt, kvm_trace_lost_ops); if (!kt-lost_file) goto err; - kt-rchan = relay_open(trace, debugfs_dir, kuts-buf_size, + kt-rchan = relay_open(trace, kvm_debugfs_dir, kuts-buf_size, kuts-buf_nr, kvm_relay_callbacks, kt); if (!kt-rchan) goto err; - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 1/5]Add some trace markers and exposeinterfaces in kernel for tracing
On Tuesday 15 April 2008 22:13:28 Liu, Eric E wrote: Hollis Blanchard wrote: On Wednesday 09 April 2008 05:01:36 Liu, Eric E wrote: +/* This structure represents a single trace buffer record. */ +struct kvm_trace_rec { + __u32 event:28; + __u32 extra_u32:3; + __u32 cycle_in:1; + __u32 pid; + __u32 vcpu_id; + union { + struct { + __u32 cycle_lo, cycle_hi; + __u32 extra_u32[KVM_TRC_EXTRA_MAX]; + } cycle; + struct { + __u32 extra_u32[KVM_TRC_EXTRA_MAX]; + } nocycle; + } u; +}; Do we really need bitfields here? They are notoriously non-portable. Practically speaking, this will prevent me from copying a trace file from my big-endian target to my little-endian workstation for analysis, at least without some ugly hacking in the userland tool. Here the main consideration using bitfields is to save storage space for each record, but as you said it is non-portable for your mentioned case, so should we need to adjust the struct like this? __u32 event; __16 extra_u32; __16 cycle_in; If space really is a worry, you could still combine the fields, and just use masks to extract the data later. No matter what, byteswapping is required in the userland tool. I suspect this isn't there already, but it will be easier to add without the bitfields. Hmm, while we're on the subject, I'm not sure what the best way to automatically byteswap will be. It probably isn't worth it to convert all trace data to a standard ordering (which would add overhead to tracing), but I suppose there is no metadata in the trace log? A command line switch might be inconvenient but inevitable. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 2 of 3] [KVM] Add DCR access information to struct kvm_run
On Monday 07 April 2008 20:11:28 David Gibson wrote: On Mon, Apr 07, 2008 at 03:53:33PM -0500, Hollis Blanchard wrote: 1 file changed, 7 insertions(+) include/linux/kvm.h |7 +++ Device Control Registers are essentially another address space found on PowerPC 4xx processors, analogous to PIO on x86. DCRs are always 32 bits, and are identified by a 32-bit number. Well... 10-bit, actually. The mtdcrux description in the ppc440x6 user manual says the following: Let the contents of register RA denote a Device Control Register. The contents of GPR[RS] are placed into the designated Device Control Register. I take that to mean that we must worry about 32 bits worth of DCR numbers. Perhaps I should say no more than rather than always. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 3 of 3] [KVM POWERPC] PowerPC 440 KVM implementation
On Monday 07 April 2008 21:12:40 Josh Boyer wrote: On Mon, 07 Apr 2008 15:53:34 -0500 Hollis Blanchard [EMAIL PROTECTED] wrote: Currently supports only PowerPC 440 Linux guests on 440 hosts. (Only tested with 440EP Bamboo guests so far, but with appropriate userspace support other SoC/board combinations should work.) I haven't reviewed the whole patch yet, but lots of it looks pretty clean. A couple comments on some things that made me scratch my head below. Interrupt handling: We use IVPR to hijack the host interrupt vectors while running the guest, but hand off decrementer and external interrupts for normal guest processing. Address spaces: We take advantage of the fact that Linux doesn't use the AS=1 address space (in host or guest), which gives us virtual address space to use for guest mappings. While the guest is running, the host kernel remains mapped in AS=0, but the guest can only use AS=1 mappings. TLB entries: The TLB entries covering the host linear address space remain present while running the guest (which reduces the overhead of lightweight exits). We keep three copies of the TLB: - guest TLB: contents of the TLB as the guest sees it - shadow TLB: the TLB that is actually in hardware while guest is running - host TLB: to restore TLB state when context switching guest - host When a TLB miss occurs because a mapping was not present in the shadow TLB, but was present in the guest TLB, KVM handles the fault without invoking the guest. Large guest pages are backed by multiple 4KB shadow pages through this mechanism. Instruction emulation: The guest kernel executes at user level, so executing privileged instructions trap into KVM, where we decode and emulate them. Future performance work will focus on reducing the overhead and frequency of these traps. IO: MMIO and DCR accesses are emulated by userspace. We use virtio for network and block IO, so those drivers must be enabled in the guest. It's possible that some qemu device emulation (e.g. e1000 or rtl8139) may also work with little effort. Signed-off-by: Hollis Blanchard [EMAIL PROTECTED] As Olof pointed out, having this in Documentation might be nice. Yeah, the trouble is that a book could be written on the subject. (OK, a short book. At least a paper.) Anyways, I will provide something... diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug --- a/arch/powerpc/Kconfig.debug +++ b/arch/powerpc/Kconfig.debug @@ -151,6 +151,7 @@ config PPC_EARLY_DEBUG bool Early debugging (dangerous) + depends on !KVM help Say Y to enable some early debugging facilities that may be available for your processor/board combination. Those facilities are hacks Might want to add a brief explanation as to why this doesn't work with KVM due to the AS conflict. Will do. diff --git a/arch/powerpc/kvm/44x_tlb.c b/arch/powerpc/kvm/44x_tlb.c new file mode 100644 --- /dev/null +++ b/arch/powerpc/kvm/44x_tlb.c @@ -0,0 +1,224 @@ +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License, version 2, as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * + * Copyright IBM Corp. 2007 + * + * Authors: Hollis Blanchard [EMAIL PROTECTED] + */ + +#include linux/types.h +#include linux/string.h +#include linux/kvm_host.h +#include linux/highmem.h +#include asm/mmu-44x.h +#include asm/kvm_ppc.h + +#include 44x_tlb.h + +#define PPC44x_TLB_USER_PERM_MASK (PPC44x_TLB_UX|PPC44x_TLB_UR|PPC44x_TLB_UW) +#define PPC44x_TLB_SUPER_PERM_MASK (PPC44x_TLB_SX|PPC44x_TLB_SR|PPC44x_TLB_SW) + +static unsigned int kvmppc_tlb_44x_pos; + +static u32 kvmppc_44x_tlb_shadow_attrib(u32 attrib, int usermode) +{ + /* XXX remove mask when Linux is fixed */ + attrib = 0xf03f; What does that mean? Is this the 44x TLB handler writes to reserved fields issue? If so, could you comment that a little more verbosely? Yup, you're right. Actually, what I should really do is this: attrib = PPC44x_TLB_ATTR_MASK | PPC44x_TLB_PERM_MASK; Or you could just send a patch to fix Linux... ;). I had a look at it once, but didn't have time to grok the assembly bit manipulations. I guess nobody else has either. :) + + if (!usermode) { + /* Guest is in supervisor mode, so we need to translate guest +* supervisor
Re: [kvm-devel] [PATCH 2 of 3] [KVM] Add DCR access information to struct kvm_run
On Monday 07 April 2008 22:54:41 David Gibson wrote: On Mon, Apr 07, 2008 at 10:25:32PM -0500, Hollis Blanchard wrote: On Monday 07 April 2008 20:11:28 David Gibson wrote: On Mon, Apr 07, 2008 at 03:53:33PM -0500, Hollis Blanchard wrote: 1 file changed, 7 insertions(+) include/linux/kvm.h |7 +++ Device Control Registers are essentially another address space found on PowerPC 4xx processors, analogous to PIO on x86. DCRs are always 32 bits, and are identified by a 32-bit number. Well... 10-bit, actually. The mtdcrux description in the ppc440x6 user manual says the following: Let the contents of register RA denote a Device Control Register. The contents of GPR[RS] are placed into the designated Device Control Register. I take that to mean that we must worry about 32 bits worth of DCR numbers. Perhaps I should say no more than rather than always. I think that's less misleading. mtdcrux is very new, anything which only has the mtdcr instruction certainly can't take DCR numbers above 10 bits, and I would expect that even on chips with mtdcrux the DCR bus is probably still only 10-bits, although it could be extended. We're defining a kernel/userspace interface here, and since the hardware is capable of 32-bit DCR numbers, I don't think it makes any sense to not support that. Also, we would just end up placing that number into a u32 anyways, so... :) -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 3 of 3] [KVM POWERPC] PowerPC 440 KVM implementation
On Monday 07 April 2008 21:58:17 Arnd Bergmann wrote: On Monday 07 April 2008, Hollis Blanchard wrote: --- a/include/asm-powerpc/kvm.h +++ b/include/asm-powerpc/kvm.h @@ -1,6 +1,55 @@ +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License, version 2, as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * + * Copyright IBM Corp. 2007 + * + * Authors: Hollis Blanchard [EMAIL PROTECTED] + */ + #ifndef __LINUX_KVM_POWERPC_H #define __LINUX_KVM_POWERPC_H -/* powerpc does not support KVM */ +#include asm/types.h -#endif +struct kvm_regs { + __u32 pc; + __u32 cr; + __u32 ctr; + __u32 lr; + __u32 xer; + __u32 msr; + __u32 srr0; + __u32 srr1; + __u32 pid; + + __u32 sprg0; + __u32 sprg1; + __u32 sprg2; + __u32 sprg3; + __u32 sprg4; + __u32 sprg5; + __u32 sprg6; + __u32 sprg7; + + __u64 fpr[32]; + __u32 gpr[32]; +}; + +struct kvm_sregs { +}; + +struct kvm_fpu { +}; + +#endif /* __LINUX_KVM_POWERPC_H */ Since this defines part of the ABI, it would be nice if it's possible to have it in a platform independent way. Most of the registers here should probably become unsigned long instead of __u32 so that the definition can be used for a potential 64 bit port. If there is one thing I have learned in my various porting efforts, it's that using a variable-sized type in an interface is just begging for trouble. x86 uses fixed 64-bit variables here (even with x86-32), so that might be the right solution here. Also, I noticed that you lump everything into kvm_regs, instead of using sregs for stuff like srr0 and kvm_fpu for the fprs. What is the reason for that? The FPRs and SPRs are only really useful for two things here: debugger support and migration. We don't really support either at the moment, so this part of the user/kernel ABI will need change as we implement those. I will move the FPR stuff into kvm_fpu though. (I think when I originally wrote this, kvm_fpu was defined to be x86 stuff, but it obviously isn't now...) -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] booting from virtio-blk
On Tue, 2008-04-01 at 09:46 -0500, Anthony Liguori wrote: Hollis Blanchard wrote: On Tue, 2008-04-01 at 14:08 +0200, Christian Ehrhardt wrote: bash-3.00# cat /proc/partitions major minor #blocks name [...] 254 0 22517998136852480 vda - ?broken? My guess is this is run-of-the-mill endianness mismatch. 22517998136852480 = 0x0050_, which 64-bit byteswapped would be 0x5000, and that's probably a reasonable number of 512-byte blocks. Is your disk image 10MB? Why would we have a problem, since both guest and host are big-endian? Because virtio is a PCI device, and PCI MMIO are LE, so __virtio_config_val() in the guest is (correctly) using le64_to_cpu(). Why didn't we have problems with virtio-net? Because virtio-net doesn't seem to have anything interesting in PCI config space. virtio-blk's config space contains the capacity and a few other pieces of information. The fix needs to be in qemu, and given the lack of qemu endianness infrastructure, I'm afraid it will be a hack. See http://svn.savannah.nongnu.org/viewvc/trunk/hw/e1000.c?root=qemur1=4046r2=4045pathrev=4046 for reference. We all know that TARGET_WORDS_BIGENDIAN is totally wrong, but unfortunately it also seems to be the only (accidentally) working solution in qemu without major IO system rework. :( It's actually not so bad since the virtio config space is already read one byte at a time. The following should help. diff --git a/qemu/hw/virtio-blk.c b/qemu/hw/virtio-blk.c index 0f55d2a..492bd7f 100644 --- a/qemu/hw/virtio-blk.c +++ b/qemu/hw/virtio-blk.c @@ -134,8 +134,8 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uin int64_t capacity; bdrv_get_geometry(s-bs, capacity); -blkcfg.capacity = capacity; -blkcfg.seg_max = 128 - 2; +blkcfg.capacity = cpu_to_le64(capacity); +blkcfg.seg_max = cpu_to_le32(128 - 2); memcpy(config, blkcfg, sizeof(blkcfg)); } Thanks Anthony, you've saved me a lot of debug time! Rusty, doing 64-bit PCI config space accesses with ioread8() definitely violates the principle of least surprises, and would have taken me a long time to track down. :( Attached is a boot log of a PowerPC guest booting from virtio-blk root. ramdisk_image is the standard ~4MB image provided with DENX Embedded Linux Development Kit. Booting is also *way* faster than NFS root (a few seconds to get to a shell :) . -- Hollis Blanchard IBM Linux Technology Center bash-3.00# ./qemu-system-ppcemb -M bamboo -nographic -kernel ../../uImage.bamboo -L ../pc-bios/ -append root=/dev/vda rw debug -net nic,model=virtio -net tap -drive file=/images/ramdisk_image,if=virtio,boot=on bamboo_init: START Ram size passed is: 144 MB Calling function ppc440_init setup mmio setup universal controller trying to setup sdram controller sdram_unmap_bcr: Unmap RAM area 0040 sdram_unmap_bcr: Unmap RAM area 0040 sdram_set_bcr: Map RAM area 0800 sdram_set_bcr: Map RAM area 0100 Initializing first serial port ppc405_serial_init: offset 0300 Done calling ppc440_init bamboo_init: load kernel kernel is at guest address: 0x0 bamboo_init: load device tree file device tree address is at guest address: 0x2b2100 bamboo_init: loading kvm registers bamboo_init: DONE Using Bamboo machine description Linux version 2.6.25-rc3-hg1858cec8eb87-dirty ([EMAIL PROTECTED]) (gcc version 3.4.2) #152 Tue Apr 1 10:52:01 CDT 2008 Found legacy serial port 0 for /plb/opb/[EMAIL PROTECTED] mem=ef600300, taddr=ef600300, irq=0, clk=11059200, speed=115200 Found legacy serial port 1 for /plb/opb/[EMAIL PROTECTED] mem=ef600400, taddr=ef600400, irq=0, clk=11059200, speed=0 console [udbg0] enabled Entering add_active_range(0, 0, 36864) 0 entries of 256 used setup_arch: bootmem arch: exit Top of RAM: 0x900, Total RAM: 0x900 Memory hole size: 0MB Zone PFN ranges: DMA 0 -36864 Normal 36864 -36864 Movable zone start PFN for each node early_node_map[1] active PFN ranges 0:0 -36864 On node 0 totalpages: 36864 DMA zone: 288 pages used for memmap DMA zone: 0 pages reserved DMA zone: 36576 pages, LIFO batch:7 Normal zone: 0 pages used for memmap Movable zone: 0 pages used for memmap Built 1 zonelists in Zone order, mobility grouping on. Total pages: 36576 Kernel command line: root=/dev/vda rw debug irq: Allocated host of type 2 @0xc03f3880 UIC0 (32 IRQ sources) at DCR 0xc0 irq: Default host set to @0xc03f3880 PID hash table entries: 1024 (order: 10, 4096 bytes) time_init: decrementer frequency = 666.60 MHz time_init: processor frequency = 666.60 MHz clocksource: timebase mult[60] shift[22] registered clockevent: decrementer mult[] shift[16] cpu[0] Dentry cache hash table entries: 32768 (order: 5, 131072 bytes) Inode-cache hash table entries: 16384 (order: 4, 65536 bytes) Memory: 143060k
Re: [kvm-devel] [PATCH] Fix endianness for virtio-blk config space
On Tue, 2008-04-01 at 11:04 -0500, Anthony Liguori wrote: The virtio config space is little endian. Make sure that in virtio-blk we store the values in little endian format. Signed-off-by: Anthony Liguori [EMAIL PROTECTED] diff --git a/qemu/hw/virtio-blk.c b/qemu/hw/virtio-blk.c index 0f55d2a..492bd7f 100644 --- a/qemu/hw/virtio-blk.c +++ b/qemu/hw/virtio-blk.c @@ -134,8 +134,8 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) int64_t capacity; bdrv_get_geometry(s-bs, capacity); -blkcfg.capacity = capacity; -blkcfg.seg_max = 128 - 2; +blkcfg.capacity = cpu_to_le64(capacity); +blkcfg.seg_max = cpu_to_le32(128 - 2); memcpy(config, blkcfg, sizeof(blkcfg)); } Fixes virtio-blk for PowerPC KVM. Acked-by: Hollis Blanchard [EMAIL PROTECTED] -- Hollis Blanchard IBM Linux Technology Center - Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] booting from virtio-blk
On Tue, 2008-04-01 at 16:03 -0500, Anthony Liguori wrote: Benjamin Herrenschmidt wrote: On Tue, 2008-04-01 at 12:09 -0500, Anthony Liguori wrote: It's the unfortunate side-effect of using PCI config space without passing it's semantics through to the virtio devices. Right now, you do a config_get which is basically a memcpy. If we didn't do accesses with ioread8(), you could potentially have a caller than did a config_get() of size 4 that didn't intend on having endian conversion applied. The other option would have been to provide config_get() and config_get8/16/32/64() the later performing endian conversion. Config space should be 8/16/32. Is that ever bridged to real PCI config space anyway ? Or only virtio ? And it should be endian swapped at the low level, either by your HV calls or by the low level kernel. Always. That's how PCI config space is supposed to work. Virtio accesses will not be bridged to real PCI space. I guess the point is, is that virtio config space is an abstraction with the implementation that is based on PCI converting all accesses to a series of 8-bit accesses. The virtio config space happens to be little endian just like the PCI config space. The point is that a virtio device appears as a PCI device. Like all other PCI devices, it has config space. Unlike all other PCI devices, its config space is accessed with 1-byte reads. -- Hollis Blanchard IBM Linux Technology Center - Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] buliding and testing PowerPC KVM
On Tue, 2008-03-25 at 18:56 +0200, Avi Kivity wrote: Hollis Blanchard wrote: On Fri, 2008-03-21 at 13:02 +0200, Avi Kivity wrote: Other than that, and the few minor comments that popped up, this (very nice) patchset will be very easy to merge. IIRC you mentioned it is possible for me to get an s390 account; this will be very useful in avoiding breaking this port, as happens quite often with ppc and ia64. I'd like to be able to do both build and run testing. As for building the PowerPC code, cross-compiling is easy with http://kegel.com/crosstool . There are also a number of servers offering remote PowerPC ssh access: see http://penguinppc.org/dev/#remote . I now have a ppc account. Once you point me at the ppc kernel repo I can start build testing. % cd kvm.git % curl http://penguinppc.org/~hollisb/kvm/kvmppc.mbox | git-am That directory also contains tip, which tells you what upstream changeset the patchqueue is based on. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] buliding and testing PowerPC KVM
On Fri, 2008-03-21 at 13:02 +0200, Avi Kivity wrote: Other than that, and the few minor comments that popped up, this (very nice) patchset will be very easy to merge. IIRC you mentioned it is possible for me to get an s390 account; this will be very useful in avoiding breaking this port, as happens quite often with ppc and ia64. I'd like to be able to do both build and run testing. As for building the PowerPC code, cross-compiling is easy with http://kegel.com/crosstool . There are also a number of servers offering remote PowerPC ssh access: see http://penguinppc.org/dev/#remote . For run testing, we are only working on 440 host support right now, so you would need a board like the Sequoia (https://www.em.avnet.com/evk/home/0,1719,RID%253D0%2526CID%253D37101% 2526CCD%253DUSA%2526SID%253D32214%2526DID%253DDF2%2526LID%253D32232% 2526BID%253DDF2%2526CTP%253DEVK,00.html). In the future we should be able to run 440 guests on e.g. POWER5 hosts, but we've already got our hands full without that. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH][QEMU] Use a separate device for in-kernel PIT
On Wed, 2008-03-12 at 12:38 -0500, Anthony Liguori wrote: Part of the feedback we received from Fabrice about the KVM patches for QEMU is that we should create a separate device for the in-kernel APIC to avoid having lots of if (kvm_enabled()) within the APIC code that were difficult to understand why there were needed. This patch separates the in-kernel PIT into a separate device. It also introduces some configure logic to only compile in support for the in-kernel PIT if it's available. The result of this is that we now only need a single if (kvm_enabled()) to determine which device to use. Besides making it more upstream friendly, I think this makes the code much easier to understand. Signed-off-by: Anthony Liguori [EMAIL PROTECTED] This patch solves annoying qemu build breakage hitting PowerPC around struct kvm_pit_state, so that's another vote in favor... -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Move kvm_get_pit to libkvm.c common code
Avi, please apply the patch at the end of this mail. On Tue, 2008-03-11 at 15:17 -0500, Jerone Young wrote: # HG changeset patch # User Jerone Young [EMAIL PROTECTED] # Date 1205266548 18000 # Branch merge # Node ID b136c0450c0f7c6ff2262437b1beb9896b1585e3 # Parent c14fbbaee36241aa0fab0d6391e47cf9f4ac8012 Move kvm_get_pit to libkvm.c common code This fixes compilation issues for PowerPC and other non x86 archs that do not have in kernel pit. The pit code is added into the kvm_context in kvm-common.h the error causing the issue is coming from a definition in qemu. This seems to be the proper fix as there is also a common function: kvm_irqchip_in_kernel for in kernel irq that handles this the same way. Signed-off-by: Jerone Young [EMAIL PROTECTED] diff --git a/libkvm/libkvm-x86.c b/libkvm/libkvm-x86.c --- a/libkvm/libkvm-x86.c +++ b/libkvm/libkvm-x86.c @@ -660,12 +660,3 @@ int kvm_disable_tpr_access_reporting(kvm } #endif - -int kvm_pit_in_kernel(kvm_context_t kvm) -{ -#ifdef KVM_CAP_PIT - return kvm-pit_in_kernel; -#else - return 0; -#endif -} diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c --- a/libkvm/libkvm.c +++ b/libkvm/libkvm.c @@ -962,3 +962,8 @@ int kvm_irqchip_in_kernel(kvm_context_t { return kvm-irqchip_in_kernel; } + +int kvm_pit_in_kernel(kvm_context_t kvm) +{ + return kvm-pit_in_kernel; +} diff --git a/libkvm/libkvm.h b/libkvm/libkvm.h --- a/libkvm/libkvm.h +++ b/libkvm/libkvm.h @@ -530,6 +530,13 @@ int kvm_set_lapic(kvm_context_t kvm, int #endif +/*! + * \brief Query wheather in kernel pit is used + * + * \param kvm Pointer to the current kvm_context + */ +int kvm_pit_in_kernel(kvm_context_t kvm); + #ifdef KVM_CAP_PIT /*! This doesn't fix libkvm, and qemu is even worse off: In file included from ../qemu-kvm.h:80, from ../hw/i8254.c:29: /home/hollisb/source/kvm-userspace-ppc.hg/qemu/../libkvm/libkvm.h:550: warning: struct kvm_pit_state declared inside parameter list /home/hollisb/source/kvm-userspace-ppc.hg/qemu/../libkvm/libkvm.h:550: warning: its scope is only this definition or declaration, which is probably not what you want /home/hollisb/source/kvm-userspace-ppc.hg/qemu/../libkvm/libkvm.h:561: warning: struct kvm_pit_state declared inside parameter list ../hw/i8254.c: In function `kvm_kernel_pit_save_to_user': ../hw/i8254.c:421: error: storage size of 'pit' isn't known ../hw/i8254.c:431: error: dereferencing pointer to incomplete type [repeated a lot] The below patch fixes the libkvm.h issue, taking the same approach as kvm_get/set_lapic() just above it. (I can't say I'm a fan of this approach, but kvm-userspace is eroding my idealism.) The qemu breakage is fixed by Anthony's PIT patch that creates i8254-kvm.c. Don't compile kvm_*_pit() on architectures whose currently supported platforms do not contain a PIT. Signed-off-by: Hollis Blanchard [EMAIL PROTECTED] diff --git a/libkvm/libkvm.h b/libkvm/libkvm.h --- a/libkvm/libkvm.h +++ b/libkvm/libkvm.h @@ -539,6 +539,7 @@ int kvm_pit_in_kernel(kvm_context_t kvm) #ifdef KVM_CAP_PIT +#if defined(__i386__) || defined(__x86_64__) || defined(__ia64__) /*! * \brief Get in kernel PIT of the virtual domain * @@ -562,6 +563,8 @@ int kvm_set_pit(kvm_context_t kvm, struc #endif +#endif + #ifdef KVM_CAP_VAPIC /*! -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH] Fix undefined refrence of qemu_system_device_hot_add for non x86 archs
On Wed, 2008-03-19 at 11:06 -0500, Jerone Young wrote: # HG changeset patch # User Jerone Young [EMAIL PROTECTED] # Date 1205942671 18000 # Branch merge # Node ID 782ef2276af9ca360e25e07ec5ac0ec387428397 # Parent 972f62b6acae693c388d7b05d3a9ba7ef26ab4a0 Fix undefined refrence of qemu_system_device_hot_add for non x86 archs This patch fixes it so that functions that depend on qemu_system_device_hot_add() are only compiled for x86 archs. Signed-off-by: Jerone Young [EMAIL PROTECTED] diff --git a/qemu/hw/device-hotplug.c b/qemu/hw/device-hotplug.c --- a/qemu/hw/device-hotplug.c +++ b/qemu/hw/device-hotplug.c @@ -140,6 +140,7 @@ static PCIDevice *qemu_system_hot_add_st return opaque; } +#if defined(TARGET_I386) || defined(TARGET_X86_64) void device_hot_add(int pcibus, const char *type, const char *opts) { PCIDevice *dev = NULL; @@ -171,6 +172,7 @@ void device_hot_remove(int pcibus, int s qemu_system_device_hot_add(pcibus, slot, 0); } +#endif static void destroy_nic(int slot) { diff --git a/qemu/monitor.c b/qemu/monitor.c --- a/qemu/monitor.c +++ b/qemu/monitor.c @@ -1359,6 +1359,7 @@ static term_cmd_t term_cmds[] = { { migrate_set_speed, s, do_migrate_set_speed, value, set maximum speed (in bytes) for migrations }, { cpu_set, is, do_cpu_set_nr, cpu [online|offline], change cpu state }, +#if defined(TARGET_I386) || defined(TARGET_X86_64) { drive_add, iss, drive_hot_add, pcibus pcidevfn [file=file][,if=type][,bus=n]\n [,unit=m][,media=d][index=i]\n [,cyls=c,heads=h,secs=s[,trans=t]]\n @@ -1366,6 +1367,7 @@ static term_cmd_t term_cmds[] = { add drive to PCI storage controller }, { pci_add, iss, device_hot_add, bus nic|storage [[vlan=n][,macaddr=addr][,model=type]] [file=file][,if=type][,bus=nr]..., hot-add PCI device }, { pci_del, ii, device_hot_remove, bus slot-number, hot remove PCI device }, +#endif { NULL, NULL, }, }; Why would we build any of this code? This whole file should be disabled at the Makefile level (with a configure patch to ifdef in monitor.c). -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 7 of 7] Add ability to specify ram on command line for bamboo board model
On Wed, 2008-03-19 at 09:45 -0500, Jerone Young wrote: Add ability to specify ram on command line for bamboo board model I get the following output with this patch: ... Ram size passed is: 144 MB WARNING: -368 MB left over memory is ram ... -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 7 of 7] Add ability to specify ram on command line for bamboo board model
On Tue, 2008-03-18 at 15:06 -0500, Jerone Young wrote: # HG changeset patch # User Jerone Young [EMAIL PROTECTED] # Date 1205870472 18000 # Branch merge # Node ID 4f90f7d25186f55bfb1503764af5264201df067f # Parent ac0fc9dfd78d2eddd083326e9b635a9286fc3b19 Add ability to specify ram on command line for bamboo board model This patch adds the ability to now specify ram sizes on the command line. Due to the nature of the code there are restictions on exactly how much ram and the multiple that the size must match. Signed-off-by: Jerone Young [EMAIL PROTECTED] diff --git a/qemu/hw/ppc440_bamboo.c b/qemu/hw/ppc440_bamboo.c --- a/qemu/hw/ppc440_bamboo.c +++ b/qemu/hw/ppc440_bamboo.c @@ -15,6 +15,9 @@ #define BINARY_DEVICE_TREE_FILE bamboo.dtb +#define bytes_to_mb(a) (a20) +#define mb_to_bytes(a) (a20) + /* PPC 440 refrence demo board * * 440 PowerPC CPU @@ -28,7 +31,7 @@ void bamboo_init(ram_addr_t ram_size, in const char *cpu_model) { char buf[1024]; - target_phys_addr_t ram_bases[2], ram_sizes[2]; + target_phys_addr_t ram_bases[2], ram_sizes[2]; qemu_irq *pic; CPUState *env; target_ulong ep=0; Here you have added a trailing space. @@ -40,32 +43,39 @@ void bamboo_init(ram_addr_t ram_size, in target_ulong dt_base=0; void *fdt; int ret; + int ram_stick_sizes[] = {256, 128, 64, 32, 16, 8 }; /* in Mega bytes */ + ram_addr_t tmp_ram_size; + int i=0, k=0; Define ram_stick_sizes[] in MB and then remove all the shifting inside the loops. uint32_t cpu_freq; uint32_t timebase_freq; printf(%s: START\n, __func__); - /* Setup Memory */ - if (ram_size) { - printf(Ram size specified on command line is %i bytes\n, - (int)ram_size); - printf(WARNING: RAM is hard coded to 144MB\n); - } - else { - printf(Using defualt ram size of %iMB\n, - ((int)ram_size/1024)/1024); + /* Setup Memory */ + printf(Ram size passed is: %i MB\n, + bytes_to_mb((int)ram_size)); + + tmp_ram_size = ram_size; + + for (i=0; i (sizeof(ram_sizes)/sizeof(ram_sizes[0])); i++) + { + for (k=0; k (sizeof(ram_stick_sizes)/sizeof(ram_stick_sizes[0])); k++) + { + if ((bytes_to_mb(ram_size)/ram_stick_sizes[k]) 0) + { Don't divide, just use a logical comparison. Also, put all the open-braces on the previous lines. + ram_sizes[i] = mb_to_bytes(ram_stick_sizes[k]); + tmp_ram_size -= mb_to_bytes(ram_stick_sizes[k]); + break; + } + } } - /* Each bank can only have memory in configurations of - * 16MB, 32MB, 64MB, 128MB, or 256MB - */ - ram_bases[0] = 0x0; - ram_sizes[0] = 0x0800; - ram_bases[1] = 0x0; - ram_sizes[1] = 0x0100; - - printf(Ram size of domain is %d bytes\n, (int)ram_size); + if (tmp_ram_size) { + printf(WARNING: %i MB left over memory is ram\n, + bytes_to_mb((int)tmp_ram_size)); + ram_size -= tmp_ram_size; + } /* Setup CPU */ env = cpu_ppc_init(440); Remove tmp_ram_size completely. Just decrement ram_size in the loop and check if it's non-zero at the end. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 3 of 7] Create new load_uimage() gunzip support to uboot loader in Qemu
On Tue, 2008-03-18 at 15:06 -0500, Jerone Young wrote: +tmp_loaded_image_size = hdr-ih_size; + +if (hdr-ih_comp == IH_COMP_GZIP) { + uncompressed_data = qemu_malloc(MAX_KERNEL_SIZE); + ret = gunzip(uncompressed_data, MAX_KERNEL_SIZE, +(unsigned char *) data, +tmp_loaded_image_size); + + if (ret 0) { +fprintf(stderr, Unable to decompress gziped image!\n); +goto fail; +} + +qemu_free(data); +cpu_physical_memory_write_rom(hdr-ih_load, uncompressed_data, + tmp_loaded_image_size); + +} +else { +cpu_physical_memory_write_rom(hdr-ih_load, data, + tmp_loaded_image_size); +} + +if (loaded_image_size != NULL) +*loaded_image_size = tmp_loaded_image_size; + +if ( load_address != NULL) + *load_address = hdr-ih_load; Your whitespace in here is all over the place. Please fix. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 2 of 7] Add libfdt support to qemu
On Tue, 2008-03-18 at 15:06 -0500, Jerone Young wrote: +## +# libfdt probe +# +if test -z $device_tree_support -a \ + $cpu = powerpc; then + device_tree_support=no + cat $TMPC EOF +#include libfdt.h +/* XXX uncomment later when libfdt is built before this test */ +//int main(void) { void *fdt; return fdt_create(fdt, 1024); } +int main (void) {return 0;} +EOF +# XXX for now do not try to link to libfdt and just check for header */ +# if $cc $ARCH_CFLAGS $CFLAGS $LDFLAGS -o $TMPE $TMPC -lfdt 2 /dev/null ; then + if $cc $ARCH_CFLAGS $CFLAGS $LDFLAGS -o $TMPE $TMPC 2 /dev/null; then + device_tree_support=yes + else +echo +echo Error: Could not find libfdt +echo Make sure to have the libfdt libs and headers installed. +echo +exit 1 + fi +fi What is the problem here? Should you describe it in the patch description? Did you mean to fix this before committing? -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 5 of 7] Add dynamic device tree manipulation change uboot loader for PPC bamboo board model
allways is Linux for now */ - long kernel_size=0; + target_long kernel_size=0; target_ulong initrd_base=0; - target_ulong initrd_size=0; + target_long initrd_size=0; + target_ulong dt_base=0; + void *fdt; + int ret; + + uint32_t cpu_freq; + uint32_t timebase_freq; Why is there an extra blank line here? -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 2 of 7] Add libfdt support to qemu
OK, if that's acceptable to the qemu folks, could you put that in the patch description? -- Hollis Blanchard IBM Linux Technology Center On Tue, 2008-03-18 at 16:22 -0500, Jerone Young wrote: So this is the code Anthony asked for for probing libfdt. The problem is that if you do ./configure params at kvm-userpace top directory for the first time or after a clean you do not have libfdt.a. When qemu configure is run this probe would allways fail. So to check we just check for the header. So we compile a very simple program that include libfdt.h as the test. All the XXX are if we ever do break libfdt out then we can do the proper probing for it (and the code for it is ready to be uncommented). On Tue, 2008-03-18 at 16:16 -0500, Hollis Blanchard wrote: On Tue, 2008-03-18 at 15:06 -0500, Jerone Young wrote: +## +# libfdt probe +# +if test -z $device_tree_support -a \ + $cpu = powerpc; then + device_tree_support=no + cat $TMPC EOF +#include libfdt.h +/* XXX uncomment later when libfdt is built before this test */ +//int main(void) { void *fdt; return fdt_create(fdt, 1024); } +int main (void) {return 0;} +EOF +# XXX for now do not try to link to libfdt and just check for header */ +# if $cc $ARCH_CFLAGS $CFLAGS $LDFLAGS -o $TMPE $TMPC -lfdt 2 /dev/null ; then + if $cc $ARCH_CFLAGS $CFLAGS $LDFLAGS -o $TMPE $TMPC 2 /dev/null; then + device_tree_support=yes + else +echo +echo Error: Could not find libfdt +echo Make sure to have the libfdt libs and headers installed. +echo +exit 1 + fi +fi What is the problem here? Should you describe it in the patch description? Did you mean to fix this before committing? - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-ppc-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 5 of 7] Add dynamic device tree manipulation change uboot loader for PPC bamboo board model
I can only assume that you will actually make the corrections that you didn't respond to in this mail. On Tue, 2008-03-18 at 16:35 -0500, Jerone Young wrote: On Tue, 2008-03-18 at 16:25 -0500, Hollis Blanchard wrote: +#define DT_PROC_INTERFACE_PATH /proc/device-tree + +/* FUNCTIONS FOR READING FROM DEVICE TREE OF HOST IN /PROC */ + +/* This function reads device-tree property files that are of + * a single cell size + */ +uint32_t read_proc_dt_prop_cell(char *path_in_device_tree) +{ + char *buf = NULL; + int i; + uint32_t num; + FILE *stream; + + i = snprintf(buf, 0, %s/%s, DT_PROC_INTERFACE_PATH, + path_in_device_tree); + + buf = (char *)malloc(i); + if (buf == NULL) + { + printf(%s: Unable to malloc string buffer buf\n, + __func__); + exit(1); + } Braces. What is the deal. They are braces. They are done diffrenent through outt the qemu code. This I don't enjoy correcting whitespace, and in fact I hate it when that's all comments are about. If it weren't so egregious in this patch series, I probably would have let it slide. In general I don't really care as long as it's *consistent*. The tabs you use in this code clashes with the rest of qemu, and that makes it difficult to use the right editor settings. Despite that, I still didn't say anything, because at least it's consistent. In general, don't just make your code work; make it pretty too. @@ -26,14 +27,22 @@ void bamboo_init(ram_addr_t ram_size, in const char *initrd_filename, const char *cpu_model) { + char buf[1024]; You previously said you had removed 'buf' and replaced it with dynamic allocation, but I don't see that here. Removing of buf discussed was from hw/device_tree.c not this file. So you can fix it here too, right? -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 3 of 7] Create new load_uimage() gunzip support to uboot loader in Qemu
On Tue, 2008-03-18 at 16:46 -0500, Jerone Young wrote: On Tue, 2008-03-18 at 16:14 -0500, Hollis Blanchard wrote: On Tue, 2008-03-18 at 15:06 -0500, Jerone Young wrote: +tmp_loaded_image_size = hdr-ih_size; + +if (hdr-ih_comp == IH_COMP_GZIP) { + uncompressed_data = qemu_malloc(MAX_KERNEL_SIZE); + ret = gunzip(uncompressed_data, MAX_KERNEL_SIZE, +(unsigned char *) data, +tmp_loaded_image_size); + + if (ret 0) { +fprintf(stderr, Unable to decompress gziped image! \n); +goto fail; +} + +qemu_free(data); +cpu_physical_memory_write_rom(hdr-ih_load, uncompressed_data, + tmp_loaded_image_size); + +} +else { +cpu_physical_memory_write_rom(hdr-ih_load, data, + tmp_loaded_image_size); +} + +if (loaded_image_size != NULL) +*loaded_image_size = tmp_loaded_image_size; + +if ( load_address != NULL) + *load_address = hdr-ih_load; Your whitespace in here is all over the place. Please fix. Actually this matches the style of this entire file. I see the one white space. I see also I used a tab one place. You switch from 8-space indentation in gunzip() to 4-space in load_uimage(), and then things go *really* screwy around Unable to decompress gziped image (which you spelled wrong btw). It looks like you alternate between 4- and 3-space indentation after that. That does NOT match the style of the rest of the file. As for the if ( syntax, there are only a handful of people doing it that way in the whole qemu tree, and more importantly, none of them are in our code. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 4 of 7] Add PPC 440EP bamboo board device tree source binary into qemu
There is no zImage, so those comments do not make sense. Filled in by loader would be more accurate. You left MAL0 and EMAC0 commented out; please remove them. You left PCI0 uncommented; please comment it out until qemu actually emulates the PCI controller. -- Hollis Blanchard IBM Linux Technology Center On Fri, 2008-03-14 at 12:09 -0500, Jerone Young wrote: # HG changeset patch # User Jerone Young [EMAIL PROTECTED] # Date 1205514170 18000 # Branch merge # Node ID 60d8930ecedd292053f9c5340c95704b20e10c65 # Parent 8b68dc88abc897e7502e2c73ca1e40eb2084104f Add PPC 440EP bamboo board device tree source binary into qemu This patch places the bamboo device tree for the PPC 440EP bamboo board into the pc-bios directory of the qemu source. This also adds a rule into the pc-bios/Makefile to build device tree files. Signed-off-by: Jerone Young [EMAIL PROTECTED] diff --git a/qemu/Makefile b/qemu/Makefile --- a/qemu/Makefile +++ b/qemu/Makefile @@ -195,7 +195,8 @@ endif mkdir -p $(DESTDIR)$(datadir) for x in bios.bin vgabios.bin vgabios-cirrus.bin ppc_rom.bin \ video.x openbios-sparc32 pxe-ne2k_pci.bin \ - pxe-rtl8139.bin pxe-pcnet.bin pxe-e1000.bin extboot.bin; \ + pxe-rtl8139.bin pxe-pcnet.bin pxe-e1000.bin extboot.bin \ + bamboo.dtb; \ do \ $(INSTALL) -m 644 $(SRC_PATH)/pc-bios/$$x $(DESTDIR)$(datadir); \ done diff --git a/qemu/pc-bios/Makefile b/qemu/pc-bios/Makefile --- a/qemu/pc-bios/Makefile +++ b/qemu/pc-bios/Makefile @@ -12,6 +12,9 @@ all: $(TARGETS) %.o: %.S $(CC) $(DEFINES) -c -o $@ $ +%.dtb: %.dts + dtc -O dtb -I dts -o $@ $ + clean: - rm -f $(TARGETS) *.o *~ + rm -f $(TARGETS) *.o *~ *.dtb diff --git a/qemu/pc-bios/bamboo.dtb b/qemu/pc-bios/bamboo.dtb new file mode 100644 index ..c7b964a26657d9cc5f7d3c6d0d9873bdfa44b9e0 GIT binary patch literal 3175 zc$~FXPm3Kz5U+XJup8Ozt|DSg#J8*x4{z8e4Fe^AByb7!$O3dMEmvhKolS?3uUj zE(j}LbMXs^;6?BYc#H3;3?O90=*;)o;[EMAIL PROTECTED]n~S5;m0tLo`L4=+D` z46qsjz%IaZKjZgJ?9XH0c4IqGXUfl;4NN=9%vW`P`mAb8!_D7=dODoZZO6k4 zb0G4~9=V!7GV?u_#Hvh)m_1Ud%H-c+j%kFE`^L)G7$[EMAIL PROTECTED]| zcrh~a`B*3mDr;(6!w|nZ;`=;9%}A@|=KjmN?J`(4{RPMo{1{d#eq;[EMAIL PROTECTED] z_94Od9sD)GDfRa~!K(dW#?1$$ygO1icb7TCeMMbMJm!9yc~-ku{{C3(OlQpY%} [EMAIL PROTECTED]SOz8O!`(LClp)zUyP-~;wZ-n1pq+Tl+Lh*TE!qT;(IjwiG zw+`L^@)6Cq45hVQv;0tI_$|aqjPzI2UEln!8Hp;%{m7Z`ipgBXuz-YS7rp)7HIj zzp3Gn=[EMAIL PROTECTED]|e|)Q)czotS^)iPX;ac(Z|Pi[EMAIL PROTECTED] z)xU22pp5u2C~A;f3wOU-JT$dv+NtTz*m6^xmW`jLel~`f@%qKBRma?8sygR1!vvP zSw|h4a=Qrap7nZ;zhXoI+`17X621rJ39fD-8uV2hgxYlcsGr#HuXx1bc6T_Y=h* zkQ^*eFmo4pj{h^yr5J3|ID*p+h_6gD9`vNuSwqS+$H*)N8Q5O$DOmxpr}C(f0Zu0 zcn+UIFQ482gU`yp;jKnh?H^vB#q;mRcKf#d-dP$DV47;f3eyXLy([EMAIL PROTECTED] z!|}4h-LsQpbROvD)zBQz3E=Ed}K[EMAIL PROTECTED] z)[EMAIL PROTECTED]|w72TDVq$KNLA+(Py+kUqLuih{Ez7m-%eZ38(~wf4U}0 zhjQ;rL!Guk?O!Q2`gB)o-OGKtS8Cm`Pj{u(k-K!3mm+KUpx*oHoHw4GdufYp%i85 zbCDz^Y?bkeFyk~2MFKoe3w--b69FNYe!-;3DyY2%=6eG|aTs)adlhkRk$}ouq0 zAjPM1k?~`w;#5rWAxcECl#TyKZ!HptFRC*NUTjqT?6FOzLYd%oU24qQO)uY(8I0 zRLocwBK5xK6{s|EzlGvRsV[EMAIL PROTECTED]@VJxdTOSCzkOH~fPEQAP25L2Z f#wo+spSs3fM}Eo*?B%_#$nY+!FrOw)eQay0LsJ) diff --git a/qemu/pc-bios/bamboo.dts b/qemu/pc-bios/bamboo.dts new file mode 100644 --- /dev/null +++ b/qemu/pc-bios/bamboo.dts @@ -0,0 +1,301 @@ +/* + * Device Tree Source for AMCC Bamboo + * + * Copyright (c) 2006, 2007 IBM Corp. + * Josh Boyer [EMAIL PROTECTED] + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed as is without + * any warranty of any kind, whether express or implied. + */ + +/ { + #address-cells = 2; + #size-cells = 1; + model = amcc,bamboo; + compatible = amcc,bamboo; + dcr-parent = /cpus/[EMAIL PROTECTED]; + + aliases { + serial0 = UART0; + serial1 = UART1; + }; + + cpus { + #address-cells = 1; + #size-cells = 0; + + [EMAIL PROTECTED] { + device_type = cpu; + model = PowerPC,440EP; + reg = 0; + clock-frequency = 1fca0550; + timebase-frequency = 017d7840; + i-cache-line-size = 20; + d-cache-line-size = 20; + i-cache-size = 8000; + d-cache-size = 8000; + dcr-controller; + dcr-access-method = native; + }; + }; + + memory { + device_type = memory; + reg = 0 0 900; + }; + + UIC0: interrupt-controller0 { + compatible = ibm,uic-440ep,ibm,uic; + interrupt-controller; + cell-index = 0; + dcr-reg = 0c0 009; + #address-cells
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 7 of 7] Add ability to specify ram on command line for bamboo board model
On Fri, 2008-03-14 at 12:09 -0500, Jerone Young wrote: # HG changeset patch # User Jerone Young [EMAIL PROTECTED] # Date 1205514174 18000 # Branch merge # Node ID 3060b75a9597d4ab67c23871df41fc5e5476df2b # Parent 63237bde74818a5dc3cdb1baee781dab101290ce Add ability to specify ram on command line for bamboo board model This patch adds the ability to now specify ram sizes on the command line. Due to the nature of the code there are restictions on exactly how much ram and the multiple that the size must match. Signed-off-by: Jerone Young [EMAIL PROTECTED] diff --git a/qemu/hw/ppc440_bamboo.c b/qemu/hw/ppc440_bamboo.c --- a/qemu/hw/ppc440_bamboo.c +++ b/qemu/hw/ppc440_bamboo.c @@ -40,32 +40,50 @@ void bamboo_init(ram_addr_t ram_size, in target_ulong dt_base=0; void *fdt; int ret; + unsigned long ram_sticks[] = {0, 0}; /* Value will be in bytes */ + ram_addr_t tmp_ram_size; + int ram_stick_sizes[] = {256, 128, 64, 32, 16, 8 }; /* in Mega bytes */ + int i=0, k=0; uint32_t cpu_freq; uint32_t timebase_freq; printf(%s: START\n, __func__); - /* Setup Memory */ - if (ram_size) { - printf(Ram size specified on command line is %i bytes\n, - (int)ram_size); - printf(WARNING: RAM is hard coded to 144MB\n); - } - else { - printf(Using defualt ram size of %iMB\n, - ((int)ram_size/1024)/1024); + /* Setup Memory */ + printf(Ram size passed is: %i MB\n, + ((int)ram_size/1024)/1024); + + tmp_ram_size = ram_size; + + for (i=0; i (sizeof(ram_sticks)/sizeof(unsigned long)); i++) + { + for (k=0; k (sizeof(ram_stick_sizes)/sizeof(int)); k++) + { + if tmp_ram_size/1024)/1024)/ram_stick_sizes[k]) 0) + { + ram_sticks[i] = ram_stick_sizes[k]*1024*1024; + tmp_ram_size -= ram_stick_sizes[k]*1024*1024; + break; + } + } + } Please match the curly brace syntax used everywhere else. Also, I don't think it makes any sense to multiply and divide by 1024*1024 everywhere; just use the proper units to begin with. Try N20 to represent N MB. tmp_ram_size is also unnecessary; just subtract from ram_size and see if it's non-zero at the end. - /* Each bank can only have memory in configurations of - * 16MB, 32MB, 64MB, 128MB, or 256MB - */ - ram_bases[0] = 0x0; - ram_sizes[0] = 0x0800; - ram_bases[1] = 0x0; - ram_sizes[1] = 0x0100; + if (tmp_ram_size) + printf(WARNING: %i left over memory is ram\n, + (tmp_ram_size/1024)/1024); - printf(Ram size of domain is %d bytes\n, (int)ram_size); + /* Each bank can only have memory in configurations of + * 4MB, 8MB, 16MB, 32MB, 64MB, 128MB, or 256MB + * why? see sdram_bcr() + * + * Max of 512MB + */ + ram_bases[0] = 0x0; + ram_sizes[0] = ram_sticks[0]; + ram_bases[1] = 0x0; + ram_sizes[1] = ram_sticks[1]; Why keep a separate ram_sticks[] array? Just operate directly on ram_sizes[], and while you're at it stop hardcoding 2 entries: the SDRAM controller can emulate all 4. Also, ram_bases[1] here is very wrong; that definitely needs to be fixed. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 0 of 7] [v2] PowerPC kvm-userspace patches
On Fri, 2008-03-14 at 12:09 -0500, Jerone Young wrote: This set address issues disscussed by Hollis on the first go around. As well as some minor fixes. Btw, please also update the description for patches 3 and 5 to rename load_uboot_l. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 7 of 7] Add ability to specify ram on command line for bamboo board model
On Tue, 2008-03-11 at 23:50 -0500, Jerone Young wrote: # HG changeset patch # User Jerone Young [EMAIL PROTECTED] # Date 1205296680 18000 # Branch merge # Node ID 8b1dd3609551efefbd6633ac6fe4caa3a6cbe5e9 # Parent 3a891d8fada96166089b5796f3241087d4aae50f Add ability to specify ram on command line for bamboo board model This patch adds the ability to now specify ram sizes on the command line. Due to the nature of the code there are restictions on exactly how much ram and the multiple that the size must match. Signed-off-by: Jerone Young [EMAIL PROTECTED] NAK, this is both brittle and overcomplicated. Try translating the following Python to C: ram = 144 reg = [0, 0, 0, 0] sizes = (256, 128, 64, 32, 16, 8) for i in range(len(reg)): for size in sizes: if ram / size: reg[i] = size ram -= size break if ram: print warning: %d left over % ram print reg -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 1 of 7] Add libfdt to KVM userspace
On Tue, 2008-03-11 at 23:50 -0500, Jerone Young wrote: diff --git a/libfdt/Makefile b/libfdt/Makefile new file mode 100644 --- /dev/null +++ b/libfdt/Makefile @@ -0,0 +1,19 @@ +include ../config.mak +include ../user/config.mak + +LIBFDT_OBJS = fdt.o fdt_ro.o fdt_wip.o fdt_sw.o fdt_rw.o fdt_strerror.o + +LIBFDT_INCLUDES = fdt.h libfdt.h +LIBFDT_EXTRA = libfdt_internal.h + +LIBFDT_LIB = libfdt.a + +CFLAGS += -I . + +all: libfdt.a + +libfdt.a: $(LIBFDT_OBJS) + $(AR) rcs $@ $^ + +clean: + rm -rf *.o *.a diff --git a/libfdt/Makefile.libfdt b/libfdt/Makefile.libfdt new file mode 100644 --- /dev/null +++ b/libfdt/Makefile.libfdt @@ -0,0 +1,14 @@ +# Makefile.libfdt +# +# This is not a complete Makefile of itself. Instead, it is designed to +# be easily embeddable into other systems of Makefiles. +# +LIBFDT_SRCS = fdt.c fdt_ro.c fdt_wip.c fdt_sw.c fdt_rw.c fdt_strerror.c +LIBFDT_INCLUDES = fdt.h libfdt.h +LIBFDT_EXTRA = libfdt_internal.h +LIBFDT_LIB = libfdt/libfdt.a + +LIBFDT_OBJS = $(LIBFDT_SRCS:%.c=%.o) + +$(LIBFDT_objdir)/$(LIBFDT_LIB): $(addprefix $(LIBFDT_objdir)/,$(LIBFDT_OBJS)) + Why are you duplicating Makefile.libfdt instead of including it? -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 3 of 7] Create new load_uboot() gunzip support to uboot loader in Qemu
On Tue, 2008-03-11 at 23:50 -0500, Jerone Young wrote: diff --git a/qemu/sysemu.h b/qemu/sysemu.h --- a/qemu/sysemu.h +++ b/qemu/sysemu.h @@ -182,6 +182,9 @@ int load_elf(const char *filename, int64 uint64_t *pentry, uint64_t *lowaddr, uint64_t *highaddr); int load_aout(const char *filename, uint8_t *addr); int load_uboot(const char *filename, target_ulong *ep, int *is_linux); +int load_uboot_l(const char *filename, target_ulong *ep, + target_ulong *la, target_ulong *loaded_image_size, + int *is_linux); #endif #ifdef HAS_AUDIO I don't like the _l name, nor la. Without reading the code I have no idea what those mean. Can't you just update the other load_uboot() callers? There are only 4 of them... and while you're at it, you should rename the function to load_uimage(). Pass NULL for whatever you rename la to, just like is_linux is handled already. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 4 of 7] Add PPC 440EP bamboo board device tree source binary into qemu
*/ + 0800 0 0 0 UIC0 1c 8 + + /* IDSEL 2 */ + 1000 0 0 0 UIC0 1b 8 + + /* IDSEL 3 */ + 1800 0 0 0 UIC0 1a 8 + + /* IDSEL 4 */ + 2000 0 0 0 UIC0 19 8 + ; + }; + }; PCI0 is the only node you might want to leave commented out, since we will have a patch for that in the near future. However, since we haven't posted that patch yet, the PCI0 node shouldn't be visible to the guest yet. + chosen { + linux,stdout-path = /plb/opb/[EMAIL PROTECTED]; + linux,initrd-start = 0; + linux,initrd-end = 0; + bootargs = ; + }; +}; Why is bootargs filled with spaces? Also, do the initrd properties need to be present? I thought you figured out how to add them at runtime. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 5 of 7] Add dynamic device tree manipulation change uboot loader for PPC bamboo board model
()) { - /* XXX insert TLB entries */ - env-gpr[1] = (1620) - 8; - env-gpr[4] = initrd_base; - env-gpr[5] = initrd_size; - env-nip = ep; + /* XXX insert TLB entries */ + env-gpr[1] = (1620) - 8; Please fix your whitespace. - env-cpu_index = 0; - printf(%s: loading kvm registers\n, __func__); - kvm_load_registers(env); +#ifdef CONFIG_LIBFDT + /* location of device tree in register */ + env-gpr[3] = dt_base; +#else + env-gpr[4] = initrd_base; + env-gpr[5] = initrd_size; +#endif Linux ignores R4 and R5 regardless of whether qemu has libfdt support or not. Those should be removed. + env-nip = ep; + + printf(%s: loading kvm registers\n, __func__); + kvm_load_registers(env); } printf(%s: DONE\n, __func__); diff --git a/qemu/hw/ppc_device_tree_support.c b/qemu/hw/ppc_device_tree_support.c new file mode 100644 --- /dev/null +++ b/qemu/hw/ppc_device_tree_support.c This is a really long file name. @@ -0,0 +1,223 @@ +/* + * Functions to help device tree manipulation using libfdt. + * It also provides functions to read entries from device tree proc + * interface. + * + * Copyright 2008 IBM Corporation. + * Authors: Jerone Young [EMAIL PROTECTED] + * + * This work is licensed under the GNU GPL licence version 2 or later. + * + */ + +#include stdio.h +#include sys/types.h +#include sys/stat.h +#include fcntl.h +#include unistd.h +#include stdlib.h + +#include config.h +#include ppc440.h + +#ifdef CONFIG_LIBFDT + #include libfdt.h +#endif Don't indent this. +#define DT_PROC_INTERFACE_PATH /proc/device-tree + +/* FUNCTIONS FOR READING FROM DEVICE TREE OF HOST IN /PROC */ + +/* This function reads device-tree property files that are of + * a single cell size + */ +static uint32_t read_proc_device_tree_prop_cell(char *path_in_device_tree) +{ + char buf[1024]; + uint32_t num; + FILE *stream; + + snprintf(buf, sizeof(buf), %s/%s, DT_PROC_INTERFACE_PATH, + path_in_device_tree); + + stream = fopen(buf, rb); + + if (stream == NULL) + { + printf(%s: Unable to open '%s'\n, __func__, buf); + exit(1); + } + + fread(num, sizeof(num), 1, stream); + fclose(stream); + + return num; +} I hate that buf variable. You can use snprintf() with length 0 to find out how much memory to malloc. ... +void set_dt_cpu_0_timebase_prop(void *fdt, uint32_t timebase) +{ + int offset; + offset = get_offset_of_node(fdt, /cpus/[EMAIL PROTECTED]); + set_dt_prop_cell(fdt, offset, timebase-frequency, + timebase); + +} + +void set_dt_initrd_start_prop(void *fdt, uint32_t start_addr) +{ + int offset; + offset = get_offset_of_node(fdt, /chosen); + set_dt_prop_cell(fdt, offset, linux,initrd-start, + start_addr); +} + +void set_dt_initrd_end_prop(void *fdt, uint32_t end_addr) +{ + int offset; + offset = get_offset_of_node(fdt, /chosen); + set_dt_prop_cell(fdt, offset, linux,initrd-end, + end_addr); +} + +void set_dt_bootargs_prop(void *fdt, char *cmdline) +{ + int offset; + offset = get_offset_of_node(fdt, /chosen); + set_dt_prop_string(fdt,offset, bootargs, cmdline); +} + +#endif These are also really long function names. If you're basically going to encode the full device tree path into the function name, I don't think they're very convenient and they're not worth having. You could just pass the path in from the caller. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 0 of 7] PowerPC kvm-userspace patches
On Tue, 2008-03-11 at 23:50 -0500, Jerone Young wrote: This set of patches enables the following: -Device tree Support - Add libfdt to kvm-userspace - Add bamboo device tree to qemu source - Detection of host Device Tree attributes - Device tree loading - Ability to specify initrd on the command line - Ability to add kernel arguments on the command line - Ability to load compressed uImages - Ability to specify memory on the command line This is good work, but I have some comments. :) I'll be happy to ack this stuff once those are addressed. So now when running powerpc code on a 440board. Known Issues: There is an issue currently where guest kernel is not mounting the initrd. Working it now! But these changes should go in anyway. I don't believe our initrd troubles have anything to do with qemu, so I wouldn't worry about that here. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 3/6] kvm: qemu: Add option for enable/disable in kernel PIT
On Fri, 2008-03-07 at 20:52 +0800, Yang, Sheng wrote: From 98543bb3c3821e5bc9003bb91d7d0c755394ffac Mon Sep 17 00:00:00 2001 From: Sheng Yang [EMAIL PROTECTED] Date: Fri, 7 Mar 2008 14:24:32 +0800 Subject: [PATCH] kvm: qemu: Add option for enable/disable in kernel PIT This patch breaks all non-x86 architectures, since common code now calls functions defined only in libkvm-x86.c . -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] Top level kvm-userspace directory getting crowded ... need new dir for qemu dependencies
On Tue, 2008-02-26 at 11:24 -0600, Jerone Young wrote: However, why do we need libfdt? Is it not carried by distros, or do you need to make changes? Well it actually isn't distributed with each distro .. sigh .. actually this comes from a tool called dtc, compiles/decompiles a device tree. Even the linux kernel has it's own version of libfdt ... so it's not exactly a central coordinated effort. It's something that kind of gets passed from project to project but never stand alone. So we kind of have to do the same. It is a centrally co-ordinated effort, but it is not a package a distro would carry. It is code shared by anything that needs to load a PowerPC Linux kernel, for example: the kernel bootwrapper (part of the Linux source tree), u-boot firmware, Xend, and now qemu. Accordingly, a libfdt.rpm simply doesn't make sense, and the code is intended to be copied into any codebase that needs it. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] Top level kvm-userspace directory getting crowded ... need new dir for qemu dependencies
On Wed, 2008-02-27 at 17:48 +0100, Alexander Graf wrote: On Feb 27, 2008, at 5:34 PM, Avi Kivity wrote: Hollis Blanchard wrote: It is a centrally co-ordinated effort, but it is not a package a distro would carry. It is code shared by anything that needs to load a PowerPC Linux kernel, for example: the kernel bootwrapper (part of the Linux source tree), u-boot firmware, Xend, and now qemu. Accordingly, a libfdt.rpm simply doesn't make sense, and the code is intended to be copied into any codebase that needs it. A static library + headers (i.e. libfdt-devel.rpm) could have been used, though Linux avoids external dependencies. Why don't you try to talk to the other possible users and create a version of the library, that at least can be packaged, even though for now KVM would be the only user? Maybe others (unlikely Linux, maybe Xen, probably dtc) would like to have a central library for device trees too. I think it's obvious that Linux and uboot will never use this. Unless someone steps up to continue PowerPC Xen development, neither will Xen. So you've now narrowed down the use case to dtc (which is libfdt upstream) and qemu. Whose problem are you trying to solve? It doesn't seem to be one that any existing users have. If you want to push it, you should probably propose it on [EMAIL PROTECTED] , which is where libfdt is discussed. I'm sure as hell not going to advocate creating a standalone library, push it into every package that supports PowerPC, and then telling users they must build on a supported version of a supported distribution. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] Top level kvm-userspace directory getting crowded ... need new dir for qemu dependencies
On Wed, 2008-02-27 at 20:18 +0100, Alexander Graf wrote: On Feb 27, 2008, at 7:56 PM, Hollis Blanchard wrote: On Wed, 2008-02-27 at 17:48 +0100, Alexander Graf wrote: On Feb 27, 2008, at 5:34 PM, Avi Kivity wrote: Hollis Blanchard wrote: It is a centrally co-ordinated effort, but it is not a package a distro would carry. It is code shared by anything that needs to load a PowerPC Linux kernel, for example: the kernel bootwrapper (part of the Linux source tree), u-boot firmware, Xend, and now qemu. Accordingly, a libfdt.rpm simply doesn't make sense, and the code is intended to be copied into any codebase that needs it. A static library + headers (i.e. libfdt-devel.rpm) could have been used, though Linux avoids external dependencies. Why don't you try to talk to the other possible users and create a version of the library, that at least can be packaged, even though for now KVM would be the only user? Maybe others (unlikely Linux, maybe Xen, probably dtc) would like to have a central library for device trees too. I think it's obvious that Linux and uboot will never use this. Unless someone steps up to continue PowerPC Xen development, neither will Xen. So you've now narrowed down the use case to dtc (which is libfdt upstream) and qemu. and kvm. == qemu Maybe OpenHackware as well. I don't know if there are more projects that want to build/read device trees, but these are absolute candidates. Nope, OpenHackware is a real (albeit crappy) Open Firmware implementation, so it has no need for libfdt. (Open Firmware uses client-firmware callbacks to transfer data. The flat device tree avoids the need for callbacks by packaging up all the data into an standardized format. libfdt is a set of convenience functions to work with that format.) So again, we the potential users are qemu and dtc. Whose problem are you trying to solve? It doesn't seem to be one that any existing users have. If you want to push it, you should probably I am seeing the problems KVM has with qemu migrations and the problems I have maintaining patches for both (KVM and qemu). I would greatly appreciate if those two would not be forking that much. Xen is even worse in that respect. Just read the qemu ML and search for patches from Ian, who desperately tries to get Xen patches upstream to reduce the forking. So basically what I am concerned about is that forking is bad for most people. There are cases where forking is the only chance to continue development, but I don't see this is the case here. Currently there is nobody who has a problem. There is no need to equate copy with fork. We will not be modifying this code, so there is no fork. But there is no problem in providing a library either, right? What exactly would improve if you provide a library in the very same source tree you build your program or a different one? Either you build both from source or you get packages for both. In the best case you can even get a package for the library and only have to recompile KVM. Nobody would want to maintain SDL in KVM, just because it uses it. There is a problem. Who is going to maintain it and integrate it with every distribution? It's not going to be me, it's apparently not going to be you, and I imagine it's not going to be Avi. propose it on [EMAIL PROTECTED] , which is where libfdt is discussed. I guess I'm the wrong person to do that. I merely suggested that it's not that bad of an idea. I'm sure as hell not going to advocate creating a standalone library, push it into every package that supports PowerPC, and then telling users they must build on a supported version of a supported distribution. Again, nothing changes between an external library and an internal one, except for improved maintainability. Nobody was talking about anything distribution specific. Currently no distribution I know of bundles KVM for PPC anyway. And as soon as they do they will include the library. The internal library has better maintainability because you maintain complete control. This is a question of taste though and I don't want to have this ending as a flame war. So please just ask the other users if they like the idea. As I lack real knowledge of device trees and PPC specifics, I wouldn't make a good moderator. The one piece of feedback I've gotten is (verbatim): Unless they have a really good reason why, I think it's pointless. I agree, this is a ridiculous thing to be arguing over, and I expected to spend my day actually being productive. Maybe the problem here is really the abbreviation lib in the name. How about I just call it fdt? -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http
Re: [kvm-devel] [kvm-ppc-devel] Top level kvm-userspace directory getting crowded ... need new dir for qemu dependencies
On Wed, 2008-02-27 at 22:20 +0100, Alexander Graf wrote: On Feb 27, 2008, at 9:22 PM, Hollis Blanchard wrote: So again, we the potential users are qemu and dtc. Just while reading this I thought Hey cool, dtc is packaged in most distributions anyway. So why not modify dtc to provide the library, so we have a common code base and make it a build dependency? That's a strange assertion, considering that Debian (and thus Ubuntu) doesn't have it. There is no need to equate copy with fork. We will not be modifying this code, so there is no fork. Cool! No need to provide a copy of it then, as we can use the 'upstream' one. I'm aware that we *could* use an upstream version of libfdt, if everybody packaged and distributed it. However, they don't, I'm not going to create and maintain those packages, and apparently you're not volunteering either. So what upsteam could we use if we wanted to? This is a question of taste though and I don't want to have this ending as a flame war. So please just ask the other users if they like the idea. As I lack real knowledge of device trees and PPC specifics, I wouldn't make a good moderator. The one piece of feedback I've gotten is (verbatim): Unless they have a really good reason why, I think it's pointless. I agree, this is a ridiculous thing to be arguing over, and I expected to spend my day actually being productive. Maybe the problem here is really the abbreviation lib in the name. How about I just call it fdt? I'm sorry. In the end it's more or less your decision anyway. Is it? If so, I think I've made my decision clear... If you plan to make frequent changes to the code (aka fork), include it in kvm. If you are only planning on using code already available without changes (aka copy), please change dtc to make the functionality that exists available to kvm (e.g. a dot a file). This mostly seems to be Avi's opinion as well as far as I understood it. Have you actually looked at the code in question, or just saw that it has lib in the name? It's 1600 lines of C. In contrast, zlib, which is used in a large number of projects, and despite that is often statically linked, is 8500. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH] Fix qemu PPC breakage in monitor.c
On Wed, 2008-02-27 at 16:14 -0600, Jerone Young wrote: # HG changeset patch # User Jerone Young [EMAIL PROTECTED] # Date 1204150440 21600 # Branch merge # Node ID f255b23b6ef9461be4ee18fa0745f30c4fb66e6a # Parent 64a281615f436e65ca7fb2f3c2721c374fbfc8be Fix qemu PPC breakage in monitor.c Recent pull of qemu_cvs has added function qemu_system_cpu_hot_add to the function do_cput_set_nr in monitor.c . Issue is qemu_system_cpu_hot_add is defined in acpi.c which is only compiled for arch with target base i386 (which are i386 x86-64). Signed-off-by: Jerone Young [EMAIL PROTECTED] diff --git a/qemu/monitor.c b/qemu/monitor.c --- a/qemu/monitor.c +++ b/qemu/monitor.c @@ -357,7 +357,9 @@ static void do_cpu_set_nr(int value, con term_printf(invalid status: %s\n, status); return; } +#if defined(TARGET_I386) || defined(TARGET_X86_64) qemu_system_cpu_hot_add(value, state); +#endif } static void do_info_jit(void) This should be submitted to qemu-devel too, no? -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH] Fix PowerPC Qemu CPU initilization when using target-ppc/fake-exec.c
Acked-by: Hollis Blanchard [EMAIL PROTECTED] Avi, please apply. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Using kzalloc to avoid allocating kvm_regs from kernel stack
On Mon, 2008-02-25 at 10:38 -0600, Hollis Blanchard wrote: On Mon, 2008-02-25 at 17:34 +0800, Zhang, Xiantao wrote: From: Xiantao Zhang [EMAIL PROTECTED] Date: Mon, 25 Feb 2008 17:11:43 +0800 Subject: [PATCH] kvm: Using kzalloc to avoid allocating kvm_regs from kernel stack. Since the size of struct kvm_regs maybe too big to allocate from kernel stack, here use kzalloc to allocate it. Where is this freed? Never mind; I see it now in rev #3. :) -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Using kzalloc to avoid allocating kvm_regs from kernel stack
On Mon, 2008-02-25 at 17:34 +0800, Zhang, Xiantao wrote: From: Xiantao Zhang [EMAIL PROTECTED] Date: Mon, 25 Feb 2008 17:11:43 +0800 Subject: [PATCH] kvm: Using kzalloc to avoid allocating kvm_regs from kernel stack. Since the size of struct kvm_regs maybe too big to allocate from kernel stack, here use kzalloc to allocate it. Signed-off-by: Xiantao Zhang [EMAIL PROTECTED] --- virt/kvm/kvm_main.c | 15 --- 1 files changed, 8 insertions(+), 7 deletions(-) diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index cf6df51..5348538 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -806,25 +806,26 @@ static long kvm_vcpu_ioctl(struct file *filp, r = kvm_arch_vcpu_ioctl_run(vcpu, vcpu-run); break; case KVM_GET_REGS: { - struct kvm_regs kvm_regs; + struct kvm_regs *kvm_regs; - memset(kvm_regs, 0, sizeof kvm_regs); - r = kvm_arch_vcpu_ioctl_get_regs(vcpu, kvm_regs); + kvm_regs = kzalloc(sizeof(struct kvm_regs), GFP_KERNEL); + r = kvm_arch_vcpu_ioctl_get_regs(vcpu, kvm_regs); if (r) goto out; r = -EFAULT; - if (copy_to_user(argp, kvm_regs, sizeof kvm_regs)) + if (copy_to_user(argp, kvm_regs, sizeof(struct kvm_regs))) goto out; r = 0; break; } case KVM_SET_REGS: { - struct kvm_regs kvm_regs; + struct kvm_regs *kvm_regs; + kvm_regs = kzalloc(sizeof(struct kvm_regs), GFP_KERNEL); r = -EFAULT; - if (copy_from_user(kvm_regs, argp, sizeof kvm_regs)) + if (copy_from_user(kvm_regs, argp, sizeof(struct kvm_regs))) goto out; - r = kvm_arch_vcpu_ioctl_set_regs(vcpu, kvm_regs); + r = kvm_arch_vcpu_ioctl_set_regs(vcpu, kvm_regs); if (r) goto out; r = 0; Where is this freed? -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] upstream PowerPC qemu breakage
On Sat, 2008-02-16 at 10:47 +0200, Avi Kivity wrote: Hollis Blanchard wrote: On Wed, 2008-02-13 at 08:58 +0200, Avi Kivity wrote: It'll need to be built against your kernel tree; please provide a URL. curl http://penguinppc.org/~hollisb/kvm/kvm-powerpc.mbox | git-am Unfortunately I wasn't able to get an F8 ppc rescue cd ISO to boot with qemu 0.9.0. Can you point me to a working combination? It's difficult to get anything booting with upstream PowerPC qemu, mostly because of the unmaintained firmware they use (called Open Hackware). That said, upstream qemu does not support 440 cores, which is what our KVM work is targeting. Also, Fedora 8 doesn't either, though it may be possible to get F8 working by providing your own kernel and some small configuration tweaks (inittab, securetty). There are other distributions that will work with little to no tweaking for 440, but until we can get IO (other than console) working we have been using very simple root filesystems. However, none of these will be useful to you in KVM unless you have a 440 host to run them on. Originally you said you just need a kernel tree to build against. Can you elaborate on what you're trying to do now? -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] upstream PowerPC qemu breakage
On Mon, 2008-02-18 at 22:22 +0200, Avi Kivity wrote: Hollis Blanchard wrote: Unfortunately I wasn't able to get an F8 ppc rescue cd ISO to boot with qemu 0.9.0. Can you point me to a working combination? It's difficult to get anything booting with upstream PowerPC qemu, mostly because of the unmaintained firmware they use (called Open Hackware). That said, upstream qemu does not support 440 cores, which is what our KVM work is targeting. Also, Fedora 8 doesn't either, though it may be possible to get F8 working by providing your own kernel and some small configuration tweaks (inittab, securetty). There are other distributions that will work with little to no tweaking for 440, but until we can get IO (other than console) working we have been using very simple root filesystems. However, none of these will be useful to you in KVM unless you have a 440 host to run them on. Originally you said you just need a kernel tree to build against. Can you elaborate on what you're trying to do now? I'd like to have a full setup so I can do run testing as well. I'd love to help make this happen. I was thinking of running a ppc distro on qemu, and building and running kvm-ppc on that to test. This isn't going to work in the near future, because KVM today requires a 440 host, and qemu today doesn't emulate 440 instructions. Even if I don't run it, it's a much better build-time environment. Of course running the unit tests and an image or two will be even better. So, at best, I'd like an emulation environment that allows me to build and run; at worst, build only. Building would be better done with a cross-compiler than inside qemu. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [patch 3/5] KVM: hypercall batching
On Sat, 2008-02-16 at 17:09 -0500, Marcelo Tosatti wrote: plain text document attachment (kvm-multicall) Batch pte updates and tlb flushes in lazy MMU mode. Signed-off-by: Marcelo Tosatti [EMAIL PROTECTED] Cc: Anthony Liguori [EMAIL PROTECTED] Index: kvm.paravirt/arch/x86/kernel/kvm.c === --- kvm.paravirt.orig/arch/x86/kernel/kvm.c +++ kvm.paravirt/arch/x86/kernel/kvm.c @@ -25,6 +25,74 @@ #include linux/kvm_para.h #include linux/cpu.h #include linux/mm.h +#include linux/hardirq.h + +#define MAX_MULTICALL_NR (PAGE_SIZE / sizeof(struct kvm_multicall_entry)) + +struct kvm_para_state { + struct kvm_multicall_entry queue[MAX_MULTICALL_NR]; + int queue_index; + enum paravirt_lazy_mode mode; +}; + +static DEFINE_PER_CPU(struct kvm_para_state, para_state); AFAICS there is no guarantee about page-alignment here... +static int kvm_hypercall_multicall(struct kvm_vcpu *vcpu, gpa_t addr, u32 nents) +{ + int i, result = 0; + + ++vcpu-stat.multicall; + vcpu-stat.multicall_nr += nents; + + for (i = 0; i nents; i++) { + struct kvm_multicall_entry mc; + int ret; + + down_read(vcpu-kvm-slots_lock); + ret = kvm_read_guest(vcpu-kvm, addr, mc, sizeof(mc)); + up_read(vcpu-kvm-slots_lock); + if (ret) + return -KVM_EFAULT; + + ret = dispatch_hypercall(vcpu, mc.nr, mc.a0, mc.a1, mc.a2, + mc.a3); + if (ret) + result = ret; + addr += sizeof(mc); + } + if (result 0) + return -KVM_EINVAL; + return result; +} ... but here you're assuming that 'queue' is physically contiguous, which is not necessarily true one you cross a page boundary. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] upstream PowerPC qemu breakage
On Wed, 2008-02-13 at 08:58 +0200, Avi Kivity wrote: It'll need to be built against your kernel tree; please provide a URL. curl http://penguinppc.org/~hollisb/kvm/kvm-powerpc.mbox | git-am -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
[kvm-devel] KVM's signal masking
We're having a hard time tracking down a PowerPC bug that seems to be related to KVM's signal handling (SIGALRM in particular), so we're trying to understand the overall signal handling design. It looks like the run sequence goes something like this: 1. qemu: block SIGALRM (and a couple others) 2. qemu: call kvm_run 3. kvm: unblocks SIGALRM 4. kvm: executes guest 5. kvm: exit handler checks signal_pending(); if true returns to qemu 6. kvm: re-blocks SIGALRM and returns to qemu 7. qemu: kvm_eat_signals() synchronously calls the normal handlers for blocked signals I'm confused about a few things. First, why must qemu unblock these signals? AFAICS signal_pending() still returns true regardless of the process's signal mask. Second, why are we synchronously calling the signal handlers in the first place? Why not allow the signals simply to be delivered? -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] upstream PowerPC qemu breakage
On Tue, 2008-02-12 at 12:45 +0200, Avi Kivity wrote: Hollis Blanchard wrote: Long term, one option is to try to define a new qemu target that completely bypasses the code generation parts of qemu. Anthony did that for x86 once, but there are at least a couple sticking points; not sure how long it will take. This is probably the best long-term way to avoid this situation in the future. It kills -no-kvm, which is a powerful debugging aid. Build failures kill a lot more functionality than -no-kvm. Beyond the immediate issue, there is also the question of carrying the memory footprint for a bunch of functionality that we aren't using. I guess it could increase exposure security issues too. Generally, I don't see that it makes sense to build a bunch of code we don't use, especially if your only merge criterion is x86 works... (By the way, upstream qemu doesn't even support 440 or IA64 instruction emulation right now, so -no-kvm is worthless to us anyways.) Another long-term option is to fix TCG for PowerPC upstream, and I'm afraid that isn't feasible. I saw some talk that dyngen and tcg can coexist; but apparently that's not the case. I have no reason to believe that's not true, in theory. In practice, we're broken right now... Hopefully qemu upstream will unbreak the damage. What do you suggest, waiting until they fix it? -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
[kvm-devel] upstream PowerPC qemu breakage
Hi Avi, we're having a problem with the qemu merge you just did in kvm-userspace. Upstream qemu recently added the TCG code generator to phase out dyngen. When he did that, Fabrice explicitly broke the build every non-x86 architecture, and since you've now pulled that breakage into KVM, we're stuck in an awkward situation. In the short term we'll have to fork a working userspace, since we're in the middle of some other stuff (such as real guest IO, which I think is pretty important :) . Long term, one option is to try to define a new qemu target that completely bypasses the code generation parts of qemu. Anthony did that for x86 once, but there are at least a couple sticking points; not sure how long it will take. This is probably the best long-term way to avoid this situation in the future. Another long-term option is to fix TCG for PowerPC upstream, and I'm afraid that isn't feasible. I guess merging with qemu while it's in a period of massive change wasn't the most opportune moment. Were there some device model changes you were eager to pick up? -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH] Add print for PowerPC qemu for failed DCR read/writes
Yeah, rate-limiting makes sense. Maybe we could take it one step further and only print the warning the first time a particular unemulated DCR is accessed. I also agree about the captalization. :) -- Hollis Blanchard IBM Linux Technology Center On Wed, 2008-02-06 at 12:25 +0100, Christian Ehrhardt wrote: hi Jerone, I think this is good for debugging to find unsupported hardware, but it should not be enabled by default (you could get a printf storm if a guest workload does stupid things). Maybe qemu has some debug/verbose options you can use. And additionally it would be useful for the dcr_write patch to print the value it tried to write. I also don't know if we really need that caps-locked in the output. Jerone Young wrote: # HG changeset patch # User Jerone Young [EMAIL PROTECTED] # Date 1202249136 21600 # Node ID 4bbbf98ebf05ef77dbb68e2131b3bc0764767c99 # Parent f8cab6a29bf3f34f1cbf4d1e6d7bd21809fd4184 Add print for PowerPC qemu for failed DCR read/writes This patch adds a print to notify of failed reads and rights. Currently we will still ignore them (until development is fully done). But this makes them easier to spot. Signed-off-by: Jerone Young [EMAIL PROTECTED] diff --git a/qemu/qemu-kvm-powerpc.c b/qemu/qemu-kvm-powerpc.c --- a/qemu/qemu-kvm-powerpc.c +++ b/qemu/qemu-kvm-powerpc.c @@ -178,13 +178,17 @@ int handle_powerpc_dcr_read(int vcpu, ui int handle_powerpc_dcr_read(int vcpu, uint32_t dcrn, uint32_t *data) { CPUState *env = cpu_single_env; -ppc_dcr_read(env-dcr_env, dcrn, data); +if (ppc_dcr_read(env-dcr_env, dcrn, data) 0) +printf(DCR FAILED on READ at 0x%x\n, dcrn); + return 0; /* XXX ignore failed DCR ops */ } int handle_powerpc_dcr_write(int vcpu, uint32_t dcrn, uint32_t data) { CPUState *env = cpu_single_env; -ppc_dcr_write(env-dcr_env, dcrn, data); +if (ppc_dcr_write(env-dcr_env, dcrn, data) 0) +printf(DCR FAILED on WRITE at 0x%x\n, dcrn); just a suggestion printf(%s - failed writing 0x%x @ 0x%x\n, dcrn, data); + return 0; /* XXX ignore failed DCR ops */ } - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-ppc-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 0 of 3] RFC: creating a particular vcpu type
If it's the ioctl in the function name you object to, that's easily changed. I think it's reasonable to say that single-system-image software requires identical cores, but that's not what we're talking about here. Heterogeneous core designs are not common, but a VM needs to reflect hardware layout, and people do it in hardware (again, not running a single system image). VCPU type is a VCPU property, and I think the design should reflect that, and as you can see from the patch it's not at all difficult to do. -- Hollis Blanchard IBM Linux Technology Center On Tue, 2008-02-05 at 10:44 -0600, Anthony Liguori wrote: Why do this at the VCPU level? Would you ever want a VM with two VCPUs with different cores? You could just add a per-VM arch ioctl to set the core type that has to be issued before any VCPU creates. Then you don't have to do ugly stuff like calling ioctls from modules. Regards, Anthony Liguori Hollis Blanchard wrote: These patches allow PowerPC to create vcpus of a particular type. Since we are actually emulating the core's supervisor mode, we can choose to emulate any type of core. However, since the core chosen will change the size of the vcpu structure (among other things), we need to know it at vcpu creation time, rather than after the fact (which is how x86's cpuid is handled). I've included the first example of how PowerPC will be using the new capability, and this will be significantly extended in the future. I think you get the idea... I still need to update my tree and patch IA64 to match, but is this approach acceptable? 6 files changed, 99 insertions(+), 11 deletions(-) arch/powerpc/kvm/powerpc.c | 80 ++-- arch/x86/kvm/x86.c |4 +- include/asm-powerpc/kvm_host.h |5 ++ include/linux/kvm.h|8 include/linux/kvm_host.h |4 +- virt/kvm/kvm_main.c|9 ++-- - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 0 of 3] RFC: creating a particular vcpu type
On Tue, 2008-02-05 at 12:05 -0600, Anthony Liguori wrote: Hollis Blanchard wrote: If it's the ioctl in the function name you object to, that's easily changed. It's not the name, it's what you're doing. You're introducing an architecture specific ioctl that essentially overrides an common-ioctl(). No, I am introducing an architecture-specific ioctl that shares common code, which after all is the goal. If anything, I would think it would be better to expand the existing common-ioctl with the notion and then have a per-architecture hook within that ioctl. I *am* expanding the common ioctl. I am also preserving the existing ABI: CREATE_VCPU still works, and CREATE_VCPU_TYPE is the new ioctl. And then, voila, we have an architecture-specific hook: kvm_arch_vcpu_create(). I will happily move the KVM_CREATE_VCPU_TYPE case from kvm_arch_vm_ioctl() to kvm_vm_ioctl(), and since the additional parameter is necessarily architecture-specific, it will simply call kvm_arch_vcpu_create_type(). -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Make non-x86 arch partially support make sync.
On Fri, 2008-02-01 at 17:34 +0800, Zhang, Xiantao wrote: From: Xiantao Zhang [EMAIL PROTECTED] Date: Fri, 1 Feb 2008 17:18:03 +0800 Subject: [PATCH] Make non-x86 arch partially support make sync. Make non-x86 arch partially support make sync, and other archs can get right header files for userspace. Signed-off-by: Xiantao Zhang [EMAIL PROTECTED] --- kernel/Makefile | 19 --- 1 files changed, 16 insertions(+), 3 deletions(-) diff --git a/kernel/Makefile b/kernel/Makefile index 7a435b5..2f0d7d5 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -13,6 +13,17 @@ LINUX = ../linux-2.6 version = $(shell cd $(LINUX); git describe) +ARCH := $(shell uname -m | sed -e s/i.86/i386/) +SRCARCH := $(ARCH) + +# Additional ARCH settings for x86 +ifeq ($(ARCH),i386) +SRCARCH := x86 +endif +ifeq ($(ARCH),x86_64) +SRCARCH := x86 +endif + _hack = mv $1 $1.orig \ gawk -v version=$(version) -f hack-module.awk $1.orig \ | sed '/\#include/! s/\blapic\b/l_apic/g' $1 rm $1.orig ARCH is already set in ../config.mak. case would be a better shell construct than ifeq for this translation. @@ -30,14 +41,15 @@ all:: sync: rm -rf tmp rsync --exclude='*.mod.c' -R \ - $(LINUX)/arch/x86/kvm/./*.[ch] \ + $(LINUX)/arch/$(SRCARCH)/kvm/./*.[cSh] \ $(LINUX)/virt/kvm/./*.[ch] \ $(LINUX)/./include/linux/kvm*.h \ - $(LINUX)/./include/asm-x86/kvm*.h \ + $(LINUX)/./include/asm-$(SRCARCH)/kvm*.h \ tmp/ rm -rf include/asm - ln -s asm-x86 include/asm + ln -s asm-$(SRCARCH) include/asm +ifeq ($(SRCARCH),x86) Rather than adding an ifdef for every architecture here, can we define a HEADERS variable and do something like for hdr in $HEADERS ; do $(call hack, $hdr) done HEADERS could be defined in the case statements you're going to add. ;) $(call unifdef, include/linux/kvm.h) $(call unifdef, include/linux/kvm_para.h) Why are these headers not common? $(call unifdef, include/asm-x86/kvm.h) @@ -48,6 +60,7 @@ sync: $(call hack, svm.c) $(call hack, x86.c) $(call hack, irq.h) +endif for i in $$(find tmp -type f -printf '%P '); \ do cmp -s $$i tmp/$$i || cp tmp/$$i $$i; done rm -rf tmp -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
[kvm-devel] [PATCH 0 of 3] RFC: creating a particular vcpu type
These patches allow PowerPC to create vcpus of a particular type. Since we are actually emulating the core's supervisor mode, we can choose to emulate any type of core. However, since the core chosen will change the size of the vcpu structure (among other things), we need to know it at vcpu creation time, rather than after the fact (which is how x86's cpuid is handled). I've included the first example of how PowerPC will be using the new capability, and this will be significantly extended in the future. I think you get the idea... I still need to update my tree and patch IA64 to match, but is this approach acceptable? 6 files changed, 99 insertions(+), 11 deletions(-) arch/powerpc/kvm/powerpc.c | 80 ++-- arch/x86/kvm/x86.c |4 +- include/asm-powerpc/kvm_host.h |5 ++ include/linux/kvm.h|8 include/linux/kvm_host.h |4 +- virt/kvm/kvm_main.c|9 ++-- - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
[kvm-devel] [PATCH 1 of 3] Pass an opaque parameter through kvm_vm_ioctl_vcpu_create() to kvm_arch_vcpu_create()
# HG changeset patch # User Hollis Blanchard [EMAIL PROTECTED] # Date 1202189664 21600 # Node ID bede9476e203f5bf59d21cc3cd71a30de2ce2c44 # Parent dfb0e1d58b57dfdf76b3111565815599bd38b92d Signed-off-by: Hollis Blanchard [EMAIL PROTECTED] --- 4 files changed, 9 insertions(+), 7 deletions(-) arch/powerpc/kvm/powerpc.c |3 ++- arch/x86/kvm/x86.c |4 ++-- include/linux/kvm_host.h |3 ++- virt/kvm/kvm_main.c|6 +++--- diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -460,7 +460,8 @@ int kvm_arch_set_memory_region(struct kv return 0; } -struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id) +struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id, + void *opaque) { struct kvm_vcpu *vcpu; int err; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3052,8 +3052,8 @@ void kvm_arch_vcpu_free(struct kvm_vcpu kvm_x86_ops-vcpu_free(vcpu); } -struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, - unsigned int id) +struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id, + void *opaque) { return kvm_x86_ops-vcpu_create(kvm, id); } diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -237,7 +237,8 @@ void kvm_arch_vcpu_free(struct kvm_vcpu void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu); void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu); void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu); -struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id); +struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id, + void *opaque); int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu); void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu); diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -733,7 +733,7 @@ static int create_vcpu_fd(struct kvm_vcp /* * 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, int n, void *opaque) { int r; struct kvm_vcpu *vcpu; @@ -741,7 +741,7 @@ static int kvm_vm_ioctl_create_vcpu(stru if (!valid_vcpu(n)) return -EINVAL; - vcpu = kvm_arch_vcpu_create(kvm, n); + vcpu = kvm_arch_vcpu_create(kvm, n, opaque); if (IS_ERR(vcpu)) return PTR_ERR(vcpu); @@ -945,7 +945,7 @@ static long kvm_vm_ioctl(struct file *fi return -EIO; switch (ioctl) { case KVM_CREATE_VCPU: - r = kvm_vm_ioctl_create_vcpu(kvm, arg); + r = kvm_vm_ioctl_create_vcpu(kvm, arg, NULL); if (r 0) goto out; break; - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
[kvm-devel] [PATCH 3 of 3] [POWERPC] Implement an ioctl that creates a vcpu of a particular type
# HG changeset patch # User Hollis Blanchard [EMAIL PROTECTED] # Date 1202189668 21600 # Node ID e6e0239e8df55c6af4e0b2959350215aaa119254 # Parent 7dd50dab9096c8e0125792e3f48083c3f47fceab The ioctl accepts a core name as input and calls kvm_vm_ioctl_create_vcpu() with the corresponding guest operations structure. That structure, which will be extended with additional core-specific function pointers, is saved into the new vcpu by kvm_arch_vcpu_create(). Signed-off-by: Hollis Blanchard [EMAIL PROTECTED] --- 3 files changed, 87 insertions(+), 3 deletions(-) arch/powerpc/kvm/powerpc.c | 77 ++-- include/asm-powerpc/kvm_host.h |5 ++ include/linux/kvm.h|8 diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -460,13 +460,22 @@ int kvm_arch_set_memory_region(struct kv return 0; } +/* XXX Make modules register kvmppc_core_spec pointers at init. */ +static struct kvmppc_core_spec kvmppc_supported_guests[] = { + { + .name = ppc440, + .vcpu_size = sizeof(struct kvm_vcpu), + }, +}; + struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id, void *opaque) { + struct kvmppc_core_spec *s = opaque; struct kvm_vcpu *vcpu; int err; - vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL); + vcpu = vmalloc(s-vcpu_size); if (!vcpu) { err = -ENOMEM; goto out; @@ -479,7 +488,7 @@ struct kvm_vcpu *kvm_arch_vcpu_create(st return vcpu; free_vcpu: - kmem_cache_free(kvm_vcpu_cache, vcpu); + vfree(vcpu); out: return ERR_PTR(err); } @@ -487,7 +496,7 @@ void kvm_arch_vcpu_free(struct kvm_vcpu void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu) { kvm_vcpu_uninit(vcpu); - kmem_cache_free(kvm_vcpu_cache, vcpu); + vfree(vcpu); } void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu) @@ -800,12 +809,74 @@ int kvm_vm_ioctl_get_dirty_log(struct kv return -ENOTSUPP; } +static struct kvmppc_core_spec *kvmppc_guest_type(char *coretype) +{ + struct kvmppc_core_spec *c = kvmppc_supported_guests; + int i; + + for (i = 0; i ARRAY_SIZE(kvmppc_supported_guests); i++,c++) + if (strcmp(c-name, coretype)) + return c; + + return 0; +} + +static long kvmppc_arch_vcpu_create_type(struct kvm *kvm, + struct kvm_vcpu_create_type __user *user_type) +{ + struct kvm_vcpu_create_type type; + struct kvmppc_core_spec *s; + char *coretype; + long r; + + if (copy_from_user(type, user_type, sizeof(type))) { + r = -EFAULT; + goto out; + } + + if (type.typelen 32) { + r = -E2BIG; + goto out; + } + + coretype = vmalloc(type.typelen); + if (!coretype) { + r = -ENOMEM; + goto out; + } + + if (copy_from_user(coretype, type.type, type.typelen)) { + r = -EFAULT; + goto out_free; + } + coretype[type.typelen-1] = '\0'; + + s = kvmppc_guest_type(coretype); + if (!s) { + r = -ENOTSUPP; + goto out_free; + } + + r = kvm_vm_ioctl_create_vcpu(kvm, type.id, s); + +out_free: + vfree(coretype); +out: + return r; +} + long kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg) { + struct kvm *kvm = filp-private_data; + void __user *argp = (void __user *)arg; long r; switch (ioctl) { + case KVM_CREATE_VCPU_TYPE: { + r = kvmppc_arch_vcpu_create_type(kvm, argp); + break; + } default: r = -EINVAL; } diff --git a/include/asm-powerpc/kvm_host.h b/include/asm-powerpc/kvm_host.h --- a/include/asm-powerpc/kvm_host.h +++ b/include/asm-powerpc/kvm_host.h @@ -49,6 +49,11 @@ struct tlbe { }; struct kvm_arch { +}; + +struct kvmppc_core_spec { + const char *name; + unsigned int vcpu_size; }; struct kvm_vcpu_arch { diff --git a/include/linux/kvm.h b/include/linux/kvm.h --- a/include/linux/kvm.h +++ b/include/linux/kvm.h @@ -211,6 +211,12 @@ struct kvm_vapic_addr { __u64 vapic_addr; }; +struct kvm_vcpu_create_type { + __u32 id; + __u32 typelen; + char type[0]; +}; + #define KVMIO 0xAE /* @@ -257,6 +263,8 @@ struct kvm_vapic_addr { #define KVM_GET_DIRTY_LOG _IOW(KVMIO, 0x42, struct kvm_dirty_log) #define KVM_SET_MEMORY_ALIAS _IOW(KVMIO, 0x43, struct kvm_memory_alias) #define KVM_GET_SUPPORTED_CPUID _IOWR(KVMIO, 0x48, struct kvm_cpuid2) +#define KVM_CREATE_VCPU_TYPE _IOW(KVMIO, 0x49, struct kvm_vcpu_create_type) + /* Device model IOC */ #define
Re: [kvm-devel] [kvm-ppc-devel] [RFC PATCH] KVM for PowerPC: simpler TLB handling, better page management
On Fri, 2008-02-01 at 13:07 -0600, Nathan Lynch wrote: Hollis Blanchard wrote: --- a/arch/powerpc/kernel/asm-offsets.c +++ b/arch/powerpc/kernel/asm-offsets.c @@ -22,6 +22,7 @@ #include linux/mman.h #include linux/mm.h #include linux/suspend.h +#include linux/kvm_host.h #ifdef CONFIG_PPC64 #include linux/time.h #include linux/hardirq.h @@ -328,5 +329,31 @@ int main(void) DEFINE(PGD_TABLE_SIZE, PGD_TABLE_SIZE); +#ifdef CONFIG_KVM + DEFINE(TLBE_BYTES, sizeof(struct tlbe)); + + DEFINE(VCPU_HOST_STACK, offsetof(struct kvm_vcpu, arch.host_stack)); + DEFINE(VCPU_HOST_PID, offsetof(struct kvm_vcpu, arch.host_pid)); + DEFINE(VCPU_HOST_TLB, offsetof(struct kvm_vcpu, arch.host_tlb)); + DEFINE(VCPU_SHADOW_TLB, offsetof(struct kvm_vcpu, arch.shadow_tlb)); I found that if CONFIG_KVM=m these definitions don't get picked up when generating asm-offsets.h, which causes the build to break: AS arch/powerpc/kvm/booke_interrupts.o arch/powerpc/kvm/booke_interrupts.S: Assembler messages: arch/powerpc/kvm/booke_interrupts.S:347: Error: unsupported relocation against VCPU_HOST_TLB arch/powerpc/kvm/booke_interrupts.S:348: Error: unsupported relocation against VCPU_SHADOW_TLB make[1]: *** [arch/powerpc/kvm/booke_interrupts.o] Error 1 Changing it to ifdef CONFIG_KVM_POWERPC (which is a bool) seems to do the right thing. Hmm... building as a module is something we've struggled with in the past, so I just left KVM_POWERPC as a bool because I didn't want to mess with it. With PowerPC's asm-offsets dependency, is it really possible to build a standalone module if the kernel didn't have KVM=y? -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Remove unoptimal code from qemu dcr handles for powerpc
On Mon, 2008-01-28 at 23:38 -0600, Jerone Young wrote: A patch I submitted yesterday to use the call qemu_kvm_cpu_env() in the dcr handles is not needed since in kvm_arch_post_kvm_run variable cpu_single_env is set to env. So just use cpu_single_env to get env. Signed-off-by: Jerone Young [EMAIL PROTECTED] Signed-off-by: Hollis Blanchard [EMAIL PROTECTED] -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Cleanup extern declerations for now removed vcpu_env in Qemu
On Mon, 2008-01-28 at 19:03 -0600, Jerone Young wrote: # HG changeset patch # User Jerone Young [EMAIL PROTECTED] # Date 1201568508 21600 # Node ID a568d031723942e1baf77077031d2b77795cbd8a # Parent 5ce532cf9a1f711d1fecb42814d301abd37aa378 Cleanup extern declerations for now removed vcpu_env in Qemu This patch removes extern decleration for vcpu_env that was recently removed for PowerPC IA64 in KVM. Signed-off-by: Jerone Young [EMAIL PROTECTED] Signed-off-by: Hollis Blanchard [EMAIL PROTECTED] -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH] Clean up KVM/QEMU interaction
On Tue, 2008-01-29 at 16:46 -0600, Anthony Liguori wrote: The following patch eliminates almost all uses of #ifdef USE_KVM by introducing a kvm_enabled() macro. If USE_KVM is set, this macro evaluates to kvm_allowed. If USE_KVM isn't set, the macro evaluates to 0. This is badly needed IMHO. Qemu seems to conform to the broken window theory... -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH] Use CONFIG_PREEMPT_NOTIFIERS around struct preempt_notifier
On Tue, 2008-01-29 at 18:22 -0500, Chris Lalancette wrote: Hollis Blanchard wrote: diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -67,7 +67,9 @@ void kvm_io_bus_register_dev(struct kvm_ struct kvm_vcpu { struct kvm *kvm; +#ifdef CONFIG_PREEMPT_NOTIFIERS struct preempt_notifier preempt_notifier; +#endif int vcpu_id; struct mutex mutex; int cpu; Hm, this causes my build to fail on x86_64: make -C /lib/modules/2.6.23.8-63.fc8/build M=`pwd` $@ make[2]: Entering directory `/usr/src/kernels/2.6.23.8-63.fc8-x86_64' LD /tmp/kvm-userspace/kernel/built-in.o CC [M] /tmp/kvm-userspace/kernel/svm.o CC [M] /tmp/kvm-userspace/kernel/vmx.o CC [M] /tmp/kvm-userspace/kernel/vmx-debug.o CC [M] /tmp/kvm-userspace/kernel/kvm_main.o /tmp/kvm-userspace/kernel/kvm_main.c: In function ‘vcpu_load’: /tmp/kvm-userspace/kernel/kvm_main.c:82: error: ‘struct kvm_vcpu’ has no member named ‘preempt_notifier’ /tmp/kvm-userspace/kernel/kvm_main.c: In function ‘vcpu_put’: /tmp/kvm-userspace/kernel/kvm_main.c:91: error: ‘struct kvm_vcpu’ has no member named ‘preempt_notifier’ /tmp/kvm-userspace/kernel/kvm_main.c: In function ‘kvm_vm_ioctl_create_vcpu’: /tmp/kvm-userspace/kernel/kvm_main.c:749: error: ‘struct kvm_vcpu’ has no member named ‘preempt_notifier’ /tmp/kvm-userspace/kernel/kvm_main.c: In function ‘preempt_notifier_to_vcpu’: /tmp/kvm-userspace/kernel/kvm_main.c:1284: error: ‘struct kvm_vcpu’ has no member named ‘preempt_notifier’ /tmp/kvm-userspace/kernel/kvm_main.c:1284: warning: type defaults to ‘int’ in declaration of ‘__mptr’ /tmp/kvm-userspace/kernel/kvm_main.c:1284: warning: initialization from incompatible pointer type /tmp/kvm-userspace/kernel/kvm_main.c:1284: error: ‘struct kvm_vcpu’ has no member named ‘preempt_notifier’ make[3]: *** [/tmp/kvm-userspace/kernel/kvm_main.o] Error 1 make[2]: *** [_module_/tmp/kvm-userspace/kernel] Error 2 make[2]: Leaving directory `/usr/src/kernels/2.6.23.8-63.fc8-x86_64' make[1]: *** [all] Error 2 make[1]: Leaving directory `/tmp/kvm-userspace/kernel' make: *** [kernel] Error 2 This seems to be an artifact of the hackage in external-module-compat.h, since you're building with a pre-PREEMPT_NOTIFIERS kernel. Maybe adding #define CONFIG_PREEMPT_NOTIFIERS after #ifndef CONFIG_PREEMPT_NOTIFIERS in external-module-compat.h would fix it, since kvm_host.h would pick up the define when it's included later. The other hackful alternative would be this in kvm_host.h: #ifdef CONFIG_PREEMPT_NOTIFIERS struct preempt_notifier preempt_notifier; #else long preempt_notifier; #endif -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH] Use CONFIG_PREEMPT_NOTIFIERS around struct preempt_notifier
On Tue, 2008-01-29 at 18:22 -0500, Chris Lalancette wrote: Hollis Blanchard wrote: diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -67,7 +67,9 @@ void kvm_io_bus_register_dev(struct kvm_ struct kvm_vcpu { struct kvm *kvm; +#ifdef CONFIG_PREEMPT_NOTIFIERS struct preempt_notifier preempt_notifier; +#endif int vcpu_id; struct mutex mutex; int cpu; Hm, this causes my build to fail on x86_64: make -C /lib/modules/2.6.23.8-63.fc8/build M=`pwd` $@ make[2]: Entering directory `/usr/src/kernels/2.6.23.8-63.fc8-x86_64' LD /tmp/kvm-userspace/kernel/built-in.o CC [M] /tmp/kvm-userspace/kernel/svm.o CC [M] /tmp/kvm-userspace/kernel/vmx.o CC [M] /tmp/kvm-userspace/kernel/vmx-debug.o CC [M] /tmp/kvm-userspace/kernel/kvm_main.o /tmp/kvm-userspace/kernel/kvm_main.c: In function ‘vcpu_load’: /tmp/kvm-userspace/kernel/kvm_main.c:82: error: ‘struct kvm_vcpu’ has no member named ‘preempt_notifier’ /tmp/kvm-userspace/kernel/kvm_main.c: In function ‘vcpu_put’: /tmp/kvm-userspace/kernel/kvm_main.c:91: error: ‘struct kvm_vcpu’ has no member named ‘preempt_notifier’ /tmp/kvm-userspace/kernel/kvm_main.c: In function ‘kvm_vm_ioctl_create_vcpu’: /tmp/kvm-userspace/kernel/kvm_main.c:749: error: ‘struct kvm_vcpu’ has no member named ‘preempt_notifier’ /tmp/kvm-userspace/kernel/kvm_main.c: In function ‘preempt_notifier_to_vcpu’: /tmp/kvm-userspace/kernel/kvm_main.c:1284: error: ‘struct kvm_vcpu’ has no member named ‘preempt_notifier’ /tmp/kvm-userspace/kernel/kvm_main.c:1284: warning: type defaults to ‘int’ in declaration of ‘__mptr’ /tmp/kvm-userspace/kernel/kvm_main.c:1284: warning: initialization from incompatible pointer type /tmp/kvm-userspace/kernel/kvm_main.c:1284: error: ‘struct kvm_vcpu’ has no member named ‘preempt_notifier’ make[3]: *** [/tmp/kvm-userspace/kernel/kvm_main.o] Error 1 make[2]: *** [_module_/tmp/kvm-userspace/kernel] Error 2 make[2]: Leaving directory `/usr/src/kernels/2.6.23.8-63.fc8-x86_64' make[1]: *** [all] Error 2 make[1]: Leaving directory `/tmp/kvm-userspace/kernel' make: *** [kernel] Error 2 Reverting this patch makes the build succeed again. Chris Lalancette Actually, I think this should do the trick: Always use CONFIG_PREEMPT_NOTIFIERS in the external module hack. Signed-off-by: Hollis Blanchard [EMAIL PROTECTED] diff --git a/kernel/hack-module.awk b/kernel/hack-module.awk --- a/kernel/hack-module.awk +++ b/kernel/hack-module.awk @@ -42,6 +42,8 @@ { sub(/linux\/mm_types\.h/, linux/mm.h) } +/#ifdef CONFIG_PREEMPT_NOTIFIERS/ { $0 = #if 1 } + { print } /kvm_x86_ops-run/ { -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] [PATCH 0 of 7] PowerPC Embedded KVM qemu enablement patches
On Mon, 2008-01-28 at 10:17 -0600, Anthony Liguori wrote: Jerone Young wrote: The issue here is that qemu does not support u-boot firmware. What we are doing is actually loading a u-boot image and booting Linux directly with a pre made device tree (which is something normally u-boot firmware would provide Linux). So this code wouldn't fit into plain qemu just yet. In theory, it would still work with QEMU though right? It just requires a custom guest kernel. Qemu does not emulate the 440 core, but that's what our KVM code supports. Bamboo is a 440EP reference board, so the Bamboo setup will be useless to qemu until they get 440 support. That said, there are only 2 places that we make KVM-specific calls in this code, and those can easily be protected with kvm_allowed checks in the future. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
[kvm-devel] [PATCH] Use CONFIG_PREEMPT_NOTIFIERS around struct preempt_notifier
# HG changeset patch # User Hollis Blanchard [EMAIL PROTECTED] # Date 1201563731 21600 # Node ID 6a2f4869cf5da00fa0ccacc9188ea493459ce4cb # Parent a6dd6b5a597903a069cb9711f73ea1b5c4f0b764 This allows kvm_host.h to be #included even when struct preempt_notifier is undefined. This is needed to build asm-offsets.h. Signed-off-by: Hollis Blanchard [EMAIL PROTECTED] --- 1 file changed, 2 insertions(+) include/linux/kvm_host.h |2 ++ diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -67,7 +67,9 @@ void kvm_io_bus_register_dev(struct kvm_ struct kvm_vcpu { struct kvm *kvm; +#ifdef CONFIG_PREEMPT_NOTIFIERS struct preempt_notifier preempt_notifier; +#endif int vcpu_id; struct mutex mutex; int cpu; - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
[kvm-devel] [PATCH 0 of 6] Enhance PowerPC unit tests
These patches create libcflat for PowerPC and allow testcases to communicate with kvmctl via MMIO. They culminate in a C testcase that returns an error code through kvmctl to the shell. The x86 Makefiles looked hairy enough that I didn't want to mess with them, but it should be fairly easy to convert x86 to use the new test/lib/ files. 16 files changed, 436 insertions(+), 130 deletions(-) user/Makefile | 15 +++- user/config-powerpc.mak | 65 + user/config-x86-common.mak|6 ++- user/iotable.c| 53 ++ user/iotable.h| 40 ++ user/main-ppc.c | 65 + user/main.c | 52 + user/test/lib/libcflat.h | 37 + user/test/lib/panic.c | 13 +++ user/test/lib/powerpc/44x/map.c | 51 + user/test/lib/powerpc/44x/tlbwe.S | 50 ++-- user/test/lib/powerpc/io.c| 35 +++ user/test/lib/printf.c| 21 +-- user/test/lib/string.c|2 - user/test/powerpc/cstart.S| 38 + user/test/powerpc/exit.c | 23 + - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
[kvm-devel] [PATCH 1 of 2] Define and use CONFIG_KVM_HAS_PIO so that we don't need pio_data in kvm_arch_vcpu
# HG changeset patch # User Hollis Blanchard [EMAIL PROTECTED] # Date 1200434310 21600 # Node ID 7fa5947a2da8c0c7424ebdcfaebcae624d6cf015 # Parent ee0c227fe3f6632f4b1b5fde3f7e05c8ea0a4378 Signed-off-by: Hollis Blanchard [EMAIL PROTECTED] Signed-off-by: Christian Ehrhardt [EMAIL PROTECTED] --- 2 files changed, 7 insertions(+) arch/x86/kvm/Kconfig |5 + virt/kvm/kvm_main.c |2 ++ diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig --- a/arch/x86/kvm/Kconfig +++ b/arch/x86/kvm/Kconfig @@ -33,9 +33,13 @@ config KVM If unsure, say N. +config KVM_HAS_PIO + bool + config KVM_INTEL tristate KVM for Intel processors support depends on KVM + select KVM_HAS_PIO ---help--- Provides support for KVM on Intel processors equipped with the VT extensions. @@ -43,6 +47,7 @@ config KVM_AMD config KVM_AMD tristate KVM for AMD processors support depends on KVM + select KVM_HAS_PIO ---help--- Provides support for KVM on AMD processors equipped with the AMD-V (SVM) extensions. diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -677,8 +677,10 @@ static int kvm_vcpu_fault(struct vm_area if (vmf-pgoff == 0) page = virt_to_page(vcpu-run); +#ifdef CONFIG_KVM_HAS_PIO else if (vmf-pgoff == KVM_PIO_PAGE_OFFSET) page = virt_to_page(vcpu-arch.pio_data); +#endif /* CONFIG_KVM_HAS_PIO */ else return VM_FAULT_SIGBUS; get_page(page); - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 2 of 2] Use CONFIG_PREEMPT_NOTIFIERS around struct preempt_notifier
On Wed, 2008-01-16 at 10:08 +0200, Avi Kivity wrote: Hollis Blanchard wrote: # HG changeset patch # User Hollis Blanchard [EMAIL PROTECTED] # Date 1200434370 21600 # Node ID 9878c9cec5f831ff5e9b97539aabc5fa3d934501 # Parent 931a81e1002110be0e8bf5b335bf199d43534c2c This allows kvm_host.h to be #included even when struct preempt_notifier is undefined. Don't you actually need preempt notifiers? They are useful if you have state that is only needed from userspace, but is expensive to switch. For x86, this is the syscall msrs (which define the syscall entry point), the fpu (which is not used in the kernel), and a few other bits (which I'm too lazy too look up and are esoteric anyway). Yes, I do. However, if you #include kvm_host.h *without* CONFIG_VIRTUALIZATION=y, CONFIG_PREEMPT_NOTIFIERS is not set and the structure is undefined. It is Linux policy to be able to unconditionally include headers, and indeed I already hit this problem when I added that #include to arch/powerpc/kernel/asm-offsets.c. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] RFC: MMIO endianness flag
On Tue, 2008-01-15 at 16:57 +0200, Avi Kivity wrote: Hollis Blanchard wrote: btw, isn't passthrough better handled through the tlb? i.e. actually let the guest access the specially-configured memory? You can have qemu mmap /dev/mem and install it as a memslot, and things should work, no? (well, you might need to set some cachablility flag or other). Hmm, yes you're right. Of course, qemu offers greater flexibility than MMUs (which are limited to page-sized granularity, for example), so it might still be useful to have qemu intercede. With the endian-aware instructions that doesn't matter, since you set the endianness on a per-instruction granularity. And with guest tlb controlled endianness, surely you get page granularity as well? Since we're defining a stable ABI, I'd rather have the information present than miss it in the future... So now the question is, do we see the need for qemu to intercept writes to pass-through devices? IMO the answer is no. If it doesn't understand anything about the device, it would be better off doing a real pass through. If it does understand the device, it should know which endianness it likes. OK, I'm willing to go along with this, and hope that we don't run into another use case for an endianness flag in the future. -- Hollis Blanchard IBM Linux Technology Center - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] RFC: MMIO endianness flag
I hope I've explained in the other mail I just sent why Qemu assuming little-endian for everything is not OK. One other important clarification: kvm_run-mmio.is_bigendian is about *one* *particular* *MMIO* *access*. It has only coincidental relationship to the endianness mode of the guest. -- Hollis Blanchard IBM Linux Technology Center On Mon, 2008-01-14 at 13:42 +0800, Xu, Anthony wrote: kvm_run-mmio.is_bigendian = vcpu-arch.some_register From your example code, I can know is_bigendian indicate whether guest is in bigendian mode when accessing MMIO. Qemu is responsible for handling MMIO emulation, Qemus always assume it is running on littleendian mode. So if guest accesses MMIO in bigendian mode, kvm need to transform it to littleendian before delivering this MMIO request to Qemu. This works for IA64 side. Thanks, - Anthony Hollis Blanchard wrote: We're just talking about a flag in the kvm_run.mmio structure, so it does not represent the state of any software, guest or host, other than that single MMIO access. This flag is only used for communication between host kernel and host userland, so the host kernel is always responsible for setting it. is_bigendian is just one more necessary piece of information for MMIO emulation. kvm_run already tells you that you are loading 4 bytes from address 0. What you don't know today is if byte 0 is the least significant or most significant byte. If is_bigendian is set, you know that byte 0 is the MSB and byte 3 is the LSB. If not, the opposite is true. In the simplest case, IA64's in-kernel MMIO emulation code could look something like: kvm_run-mmio.phys_addr = addr; kvm_run-mmio.len = len; ... kvm_run-mmio.is_bigendian = vcpu-arch.some_register BIGENDIAN; If IA64 has reverse-endian load/store instructions like PowerPC, then you would also need to consider the particular instruction used as well as the guest state. On Fri, 2008-01-11 at 10:02 +0800, Xu, Anthony wrote: Hi all, That's a good start to consider BE. Yes, IA64 support BE and LE. I have below comments. What does is_bigendian mean? Host is runing with BE or guest is running with BE. Who will set is_bigendian? For supporing BE, We need to consider host BE and guest BE. For IA64, most OS is running with LE, and Application can run with BE or LE, For example, Qemu can run with BE or LE. IMHO, we need two flags, host_is_bigendian indicates Qemu is running with BE Guest_is_bigendian indecates the guest application who is accessing MMIO Is running with LE. - Anthony Hollis Blanchard wrote: On Thu, 2008-01-10 at 17:28 +0200, Avi Kivity wrote: I'll apply that patch (with a #ifdef CONFIG_PPC so other archs don't use it by mistake). I don't think that's the right ifdef. For example, I believe IA64 can run in BE mode and so will have the same issue, and there are certainly other architectures (less relevant to the current code) that definitely are in the same situation. We need to plumb this through to the libkvm users anyways. Take a look at the patch below and tell me if you think it's not the right approach. x86 simply won't consider 'is_bigendian'. I spent a lot of time on this, and it's by far the cleanest solution I could come up with. diff --git a/libkvm/config-i386.mak b/libkvm/config-i386.mak --- a/libkvm/config-i386.mak +++ b/libkvm/config-i386.mak @@ -2,5 +2,6 @@ LIBDIR := /lib LIBDIR := /lib CFLAGS += -m32 CFLAGS += -D__i386__ +CFLAGS += -DARCH_MMIO_ENDIAN_LITTLE libkvm-$(ARCH)-objs := libkvm-x86.o diff --git a/libkvm/config-ia64.mak b/libkvm/config-ia64.mak --- a/libkvm/config-ia64.mak +++ b/libkvm/config-ia64.mak @@ -1,5 +1,6 @@ LIBDIR := /lib CFLAGS += -D__ia64__ +CFLAGS += -DARCH_MMIO_ENDIAN_LITTLE -DARCH_MMIO_ENDIAN_BIG libkvm-$(ARCH)-objs := libkvm-ia64.o diff --git a/libkvm/config-powerpc.mak b/libkvm/config-powerpc.mak --- a/libkvm/config-powerpc.mak +++ b/libkvm/config-powerpc.mak @@ -2,5 +2,6 @@ LIBDIR := /lib LIBDIR := /lib CFLAGS += -m32 CFLAGS += -D__powerpc__ +CFLAGS += -DARCH_MMIO_ENDIAN_BIG -DARCH_MMIO_ENDIAN_LITTLE libkvm-$(ARCH)-objs := libkvm-powerpc.o diff --git a/libkvm/config-x86_64.mak b/libkvm/config-x86_64.mak --- a/libkvm/config-x86_64.mak +++ b/libkvm/config-x86_64.mak @@ -2,5 +2,6 @@ LIBDIR := /lib64 LIBDIR := /lib64 CFLAGS += -m64 CFLAGS += -D__x86_64__ +CFLAGS += -DARCH_MMIO_ENDIAN_LITTLE libkvm-$(ARCH)-objs := libkvm-x86.o diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c --- a/libkvm/libkvm.c +++ b/libkvm/libkvm.c @@ -774,21 +774,56 @@ int kvm_set_sregs(kvm_context_t kvm, int return ioctl(kvm-vcpu_fd[vcpu], KVM_SET_SREGS, sregs); } +#ifdef ARCH_MMIO_ENDIAN_BIG +static int handle_mmio_bigendian
Re: [kvm-devel] [kvm-ppc-devel] RFC: MMIO endianness flag
On Sun, 2008-01-13 at 11:42 +0200, Avi Kivity wrote: Do we really need to propagate endianness all the way to the user? Perhaps libkvm could call the regular mmio functions and do the transformation itself. Or maybe even the kernel can do this by itself? The kernel *already* does this by itself, and I'm attempting to explain why that is not sufficient. My point is precisely that the endianness information must be propagated to the user, otherwise the user may not have all the information it needs to emulate it. Here is the concrete example: * guest writes to MMIO * KVM passes MMIO information (physical address, number of bytes, value) to qemu * Qemu knows from the address that this access is for a passthough device, a special case the administrator has pre-configured * Qemu does mmap(/dev/mem), and writes length bytes of value at offset address. Now here's the catch: what endianness does qemu use when doing the write? If qemu only does BE, then a LE access from the guest will be byte-reversed when presented to the real hardware. -- Hollis Blanchard IBM Linux Technology Center - Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [kvm-ppc-devel] RFC: MMIO endianness flag
We're just talking about a flag in the kvm_run.mmio structure, so it does not represent the state of any software, guest or host, other than that single MMIO access. This flag is only used for communication between host kernel and host userland, so the host kernel is always responsible for setting it. is_bigendian is just one more necessary piece of information for MMIO emulation. kvm_run already tells you that you are loading 4 bytes from address 0. What you don't know today is if byte 0 is the least significant or most significant byte. If is_bigendian is set, you know that byte 0 is the MSB and byte 3 is the LSB. If not, the opposite is true. In the simplest case, IA64's in-kernel MMIO emulation code could look something like: kvm_run-mmio.phys_addr = addr; kvm_run-mmio.len = len; ... kvm_run-mmio.is_bigendian = vcpu-arch.some_register BIGENDIAN; If IA64 has reverse-endian load/store instructions like PowerPC, then you would also need to consider the particular instruction used as well as the guest state. -- Hollis Blanchard IBM Linux Technology Center On Fri, 2008-01-11 at 10:02 +0800, Xu, Anthony wrote: Hi all, That's a good start to consider BE. Yes, IA64 support BE and LE. I have below comments. What does is_bigendian mean? Host is runing with BE or guest is running with BE. Who will set is_bigendian? For supporing BE, We need to consider host BE and guest BE. For IA64, most OS is running with LE, and Application can run with BE or LE, For example, Qemu can run with BE or LE. IMHO, we need two flags, host_is_bigendian indicates Qemu is running with BE Guest_is_bigendian indecates the guest application who is accessing MMIO Is running with LE. - Anthony Hollis Blanchard wrote: On Thu, 2008-01-10 at 17:28 +0200, Avi Kivity wrote: I'll apply that patch (with a #ifdef CONFIG_PPC so other archs don't use it by mistake). I don't think that's the right ifdef. For example, I believe IA64 can run in BE mode and so will have the same issue, and there are certainly other architectures (less relevant to the current code) that definitely are in the same situation. We need to plumb this through to the libkvm users anyways. Take a look at the patch below and tell me if you think it's not the right approach. x86 simply won't consider 'is_bigendian'. I spent a lot of time on this, and it's by far the cleanest solution I could come up with. diff --git a/libkvm/config-i386.mak b/libkvm/config-i386.mak --- a/libkvm/config-i386.mak +++ b/libkvm/config-i386.mak @@ -2,5 +2,6 @@ LIBDIR := /lib LIBDIR := /lib CFLAGS += -m32 CFLAGS += -D__i386__ +CFLAGS += -DARCH_MMIO_ENDIAN_LITTLE libkvm-$(ARCH)-objs := libkvm-x86.o diff --git a/libkvm/config-ia64.mak b/libkvm/config-ia64.mak --- a/libkvm/config-ia64.mak +++ b/libkvm/config-ia64.mak @@ -1,5 +1,6 @@ LIBDIR := /lib CFLAGS += -D__ia64__ +CFLAGS += -DARCH_MMIO_ENDIAN_LITTLE -DARCH_MMIO_ENDIAN_BIG libkvm-$(ARCH)-objs := libkvm-ia64.o diff --git a/libkvm/config-powerpc.mak b/libkvm/config-powerpc.mak --- a/libkvm/config-powerpc.mak +++ b/libkvm/config-powerpc.mak @@ -2,5 +2,6 @@ LIBDIR := /lib LIBDIR := /lib CFLAGS += -m32 CFLAGS += -D__powerpc__ +CFLAGS += -DARCH_MMIO_ENDIAN_BIG -DARCH_MMIO_ENDIAN_LITTLE libkvm-$(ARCH)-objs := libkvm-powerpc.o diff --git a/libkvm/config-x86_64.mak b/libkvm/config-x86_64.mak --- a/libkvm/config-x86_64.mak +++ b/libkvm/config-x86_64.mak @@ -2,5 +2,6 @@ LIBDIR := /lib64 LIBDIR := /lib64 CFLAGS += -m64 CFLAGS += -D__x86_64__ +CFLAGS += -DARCH_MMIO_ENDIAN_LITTLE libkvm-$(ARCH)-objs := libkvm-x86.o diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c --- a/libkvm/libkvm.c +++ b/libkvm/libkvm.c @@ -774,21 +774,56 @@ int kvm_set_sregs(kvm_context_t kvm, int return ioctl(kvm-vcpu_fd[vcpu], KVM_SET_SREGS, sregs); } +#ifdef ARCH_MMIO_ENDIAN_BIG +static int handle_mmio_bigendian(kvm_context_t kvm, struct kvm_run *kvm_run) +{ + if (kvm_run-mmio.is_write) + return kvm-callbacks-mmio_write_be(kvm-opaque, + kvm_run-mmio.phys_addr, +kvm_run-mmio.data, +kvm_run-mmio.len); + else + return kvm-callbacks-mmio_read_be(kvm-opaque, + kvm_run-mmio.phys_addr, + kvm_run-mmio.data, + kvm_run-mmio.len); +} +#endif + +#ifdef ARCH_MMIO_ENDIAN_LITTLE +static int handle_mmio_littleendian(kvm_context_t kvm, struct kvm_run *kvm_run) +{ + if (kvm_run-mmio.is_write) + return kvm-callbacks-mmio_write_le(kvm-opaque, + kvm_run-mmio.phys_addr, +kvm_run-mmio.data
Re: [kvm-devel] RFC: MMIO endianness flag
On Thu, 2008-01-10 at 08:56 +0200, Avi Kivity wrote: IIRC endianness is a per-page attribute on ppc, no? Otherwise you'd have a global attribute instead of per-access. The MMU in some PowerPC can have per-page endianness, but not all. On a processor that supports this attribute, I expect that when an MMIO trap occurs we'll need to inspect the guest MMU state in order to set the is_bigendian flag correctly. The real issue I'm looking at right now is byte-reversed loads and stores. For example, lwzx (Load Word and Zero Indexed) does a big-endian 4-byte load, while lwbrx (Load Word Byte-Reverse Indexed) does a little-endian 4-byte load. These instructions exist on all PowerPC, and they can be issued at any time and do not depend on MMU mappings. -- Hollis Blanchard IBM Linux Technology Center - Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel