Re: [kvm-devel] kvm trace support for ppc

2008-05-14 Thread Hollis Blanchard
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] [RFC] Reworking KVM_DEBUG_GUEST

2008-05-14 Thread Hollis Blanchard
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-ppc-devel] [RFC] Reworking KVM_DEBUG_GUEST

2008-05-14 Thread Hollis Blanchard
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] [RFC] Reworking KVM_DEBUG_GUEST

2008-05-14 Thread Hollis Blanchard
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] [RFC] Reworking KVM_DEBUG_GUEST

2008-05-14 Thread Hollis Blanchard
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

2008-05-14 Thread Hollis Blanchard
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] [kvm-ppc-devel] [PATCH 0 of 2] [RESEND] [PowerPC] Fix setting memory for bamboo board model

2008-05-05 Thread Hollis Blanchard
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] [kvm-ppc-devel] [PATCH] Update kernel/Makefile and remove x86 only entries

2008-04-30 Thread Hollis Blanchard
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] Fix kvm-userspace configure script so that cc=gcc

2008-04-30 Thread Hollis Blanchard
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] [PATCH] Handle vma regions with no backing page (v2)

2008-04-30 Thread Hollis Blanchard
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] [PATCH] Handle vma regions with no backing page (v2)

2008-04-29 Thread Hollis Blanchard
On Tuesday 29 April 2008 17:17:49 Avi Kivity wrote:
> Anthony Liguori wrote:
> > This patch allows VMA's that contain no backing page to be used for guest
> > memory.  This is a drop-in replacement for Ben-Ami's first page in his 
direct
> > mmio series.  Here, we continue to allow mmio pages to be represented in 
the
> > rmap.
> >
> >   
> 
> I like this very much, as it only affects accessors and not the mmu core 
> itself.
> 
> Hollis/Xiantao/Carsten, can you confirm that this approach works for 
> you?  Carsten, I believe you don't have mmio, but at least this 
> shouldn't interfere.

OK, so the idea is to mmap /sys/bus/pci/.../region within the guest RAM area, 
and just include it within the normal guest RAM memslot?

How will the IOMMU be programmed? Wouldn't you still need to register a 
special type of memslot for 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


[kvm-devel] Fwd: [kvm-ppc-devel] [PATCH] kvmppc: deliver INTERRUPT_FP_UNAVAIL to the guest

2008-04-29 Thread Hollis Blanchard
Acked-by: Hollis Blanchard <[EMAIL PROTECTED]>

Avi, please apply for 2.6.26.

-- 
Hollis Blanchard
IBM Linux Technology Center
--- Begin Message ---
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 function dt_cell_multi to hw/device_tree.c

2008-04-29 Thread Hollis Blanchard
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


Re: [kvm-devel] [kvm-ppc-devel] [PATCH 0 of 3] Fix guest eating 100% cpu when guest is idle on PowerPC

2008-04-25 Thread Hollis Blanchard
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] [PATCH 3 of 3] Add premption handlers & properly wake sleeping guest

2008-04-25 Thread Hollis Blanchard
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 2 of 3] Add PowerPC KVM guest wait handling support

2008-04-25 Thread Hollis Blanchard
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] Moving kvm lists to kernel.org?

2008-04-24 Thread Hollis Blanchard
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.

2008-04-22 Thread Hollis Blanchard
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


Re: [kvm-devel] [RFC PATCH] virtio: change config to guest endian.

2008-04-22 Thread Hollis Blanchard
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.

2008-04-22 Thread Hollis Blanchard
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.

2008-04-22 Thread Hollis Blanchard
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.

2008-04-22 Thread Hollis Blanchard
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] [kvm-ppc-devel] [PATCH 1/5]Add some trace markers and exposeinterfaces in kernel for tracing

2008-04-21 Thread Hollis Blanchard
On Sunday 20 April 2008 00:38:32 Liu, Eric E wrote:
> Christian Ehrhardt wrote:
> > Liu, Eric E wrote:
> >> Hollis Blanchard wrote:
> >>> On Wednesday 16 April 2008 01:45:34 Liu, Eric E wrote: [...]
> >>> 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. 
> >>> 
> >> Yes, let kvmtrace insert the metadata is more reasonable.
> >> 
> > 
> > I wanted to note that the kvmtrace tool should, but not need to know
> > everything about the data format. 
> > I think of e.g. changing kernel implementations that change endianess
> > or even flags we don't yet know, but we might need in the future. 
> > 
> > What about adding another debugfs entry the kernel can use to expose
> > the "kvmtrace-metadata" defined by the kernel implementation. 
> > The kvmtrace tool could then use that to build up the record by using
> > one entry for kernel defined metadata and another to add any metadata
> > that would be defined by kvmtrace tool itself.  
> > 
> > what about that one:
> > struct metadata {
> > u32 kmagic; /* stores kernel defined metadata read from
> debugfs
> > entry */ u32 umagic; /* stores userspace tool defined
> metadata */
> > u32 extra;  /* it is redundant, only use to fit into
> record. */
> > }
> > 
> > That should give us the flexibility to keep the format if we get more
> > metadata requirements in the future. 
> 
> Yes, maybe we need metadata to indicate the changing kernel
> implementations in the future, but adding debugfs entry seems not a good
> approach. What about defining a similar metadat in kernel rather than in
> userland and write it in rchan at the first time we add trace data. Then
> we don't need kvmtrace tool to insert the medadata again.
> like this: 
>   struct kvm_trace_metadata {
>   u32 kmagic; /* stores kernel defined metadata */
>   u64 extra;  /* use to fit into record. */
>   }

So you've gone back to the idea about the kernel inserting a special trace 
record? How do you handle the case where this record is overwritten before 
the logging app gets a chance to extract it? This issue is why I would prefer 
Christian's idea of a separate debugfs file (*not* relay channel) to export 
kernel flags.

At that point, kvmtrace can insert the flags in any way it wants. It doesn't 
need to appear as a trace record at all; it should simply be a header at the 
beginning of the trace file.

-- 
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

2008-04-18 Thread Hollis Blanchard
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, ®s);
@@ -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


Re: [kvm-devel] [kvm-ppc-devel] [PATCH 1/5]Add some trace markers and exposeinterfaces in kernel for tracing

2008-04-17 Thread Hollis Blanchard
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


[kvm-devel] [PATCH] [POWERPC KVM] Kconfig fixes

2008-04-17 Thread Hollis Blanchard
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] [PATCH] add virtio disk geometry feature

2008-04-16 Thread Hollis Blanchard
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 1/2] kvm-s390: provide ge t/set_mp_state stubs to fix compile error

2008-04-16 Thread Hollis Blanchard
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 1/5]Add some trace markers and exposeinterfaces in kernel for tracing

2008-04-15 Thread Hollis Blanchard
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


[kvm-devel] [PATCH] [KVM] Rename debugfs_dir to kvm_debugfs_dir

2008-04-15 Thread Hollis Blanchard
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 expose interfaces in kernel for tracing

2008-04-15 Thread Hollis Blanchard
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


Re: [kvm-devel] [kvm-ppc-devel] [PATCH] [v2 ][kvm-userspace] Make "make sync" in kern el dir work for multiple archs

2008-04-15 Thread Hollis Blanchard
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] [kvm-userspace] Make "make sync" in kernel dir work for multiple archs

2008-04-15 Thread Hollis Blanchard
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] [PATCH] [kvm-userspace] Make "make sync" in kernel dir work for multiple archs

2008-04-15 Thread Hollis Blanchard
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 0 of 3] KVM for PowerPC 440

2008-04-10 Thread Hollis Blanchard
On Thursday 10 April 2008 06:55:18 Josh Boyer wrote:
> On Mon, 07 Apr 2008 15:53:31 -0500
>
> Hollis Blanchard <[EMAIL PROTECTED]> wrote:
> > Implement initial support for KVM for PowerPC 440. There are just two
> > small prerequisite patches, and then the bulk of the code can't be split
> > easily.
> >
> > Please review; I would like to submit these for 2.6.26. There is plenty
> > of work to do, both functional and optimization, but this code is
> > sufficient to run unmodified 440 Linux guests on a 440 Linux host. What's
> > your favorite bike shed color?
>
> Who's tree are you looking to get this patch set into?

Since the only PPC-specific change is patch 1, if I can get your Acked-by for 
it I will send it via Avi.

-- 
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 3 of 3] [KVM POWERPC] PowerPC 440 KVM implementation

2008-04-07 Thread Hollis Blanchard
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 
> >  
> > -#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] [PATCH 2 of 3] [KVM] Add DCR access information to struct kvm_run

2008-04-07 Thread Hollis Blanchard
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

2008-04-07 Thread Hollis Blanchard
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 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#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;
> > +
> > +st

Re: [kvm-devel] [PATCH 2 of 3] [KVM] Add DCR access information to struct kvm_run

2008-04-07 Thread Hollis Blanchard
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] [kvm-ppc-devel] booting from virtio-blk

2008-04-01 Thread Hollis Blanchard
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] [PATCH] Fix endianness for virtio-blk config space

2008-04-01 Thread Hollis Blanchard
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] booting from virtio-blk

2008-04-01 Thread Hollis Blanchard
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=qemu&r1=4046&r2=4045&pathrev=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 s

[kvm-devel] virtio-net working on PowerPC KVM

2008-03-28 Thread Hollis Blanchard
I'm pleased to report that we now have working network support in the
guest, via the virtio-net driver. In fact, we can use NFS for the
guest's root filesystem. :) Boot log attached.

The bad news is that it's very slow, but the good news is that it's nice
to be improving performance rather than debugging mysterious
crashes... ;)

With this milestone reached, in the near future I intend to start
sending patches to Avi and linuxppc-dev for review, hopefully for
inclusion in 2.6.26. However, I do want to see if we can improve the
performance a little bit first...

-- 
Hollis Blanchard
IBM Linux Technology Center
bash-3.00# ./qemu-system-ppcemb -M bamboo -nographic -kernel 
../../uImage.bamboo -L ../pc-bios/ -append "ip=dhcp 
nfsroot=9.XX.XX.XX:/netroot/ppc_4xx root=/dev/nfs rw debug" -net 
nic,model=virtio -net tap 
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) #150 Wed Mar 19 12:51:20 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: ip=dhcp nfsroot=9.XX.XX.XX:/netroot/ppc_4xx root=/dev/nfs 
rw debug
irq: Allocated host of type 2 @0xc03f3900
UIC0 (32 IRQ sources) at DCR 0xc0
irq: Default host set to @0xc03f3900
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/147456k available (2632k kernel code, 4252k reserved, 100k 
data, 125k bss, 132k init)
SLUB: Genslabs=10, HWalign=32, Order=0-1, MinObjects=4, CPUs=1, Nodes=1
Calibrating delay loop... 2490.36 BogoMIPS (lpj=4980736)
Mount-cache hash table entries: 512
net_namespace: 156 bytes
NET: Registered protocol family 16
 
PCI host bridge /plb/[EMAIL PROTECTED] (primary) ranges:
 MEM 0xa000..0xbfff -> 0xa000 
  IO 0xe800..0xe800 -> 0x
4xx PCI DMA offset set to 0x
PCI: Probing PCI hardware
PCI: Hiding 4xx host bridge resources :00:00.0
irq: irq_create_mapping(0xc03f3900, 0x1c)
irq: -> using host @c03f3900
irq: -> obtained virq 28
Time: timebase clocksource has been installed.
NET: Registered protocol family 2
IP route cache hash table entries: 2048 (order: 1, 8192 bytes)
TCP established hash table entries: 8192 (order: 4, 65536 bytes)
TCP bind hash table entries: 8192 (order: 3, 32768 bytes)
TCP: Hash tables configured (established 8192 bind 8192)
TCP reno registered
irq: irq_create_mapping(0xc03f3900, 0x0)
irq: -> using host @c03f3900
irq: -> obtained virq 16
irq: irq_create_mapping(0xc03f3900, 0x1)
irq: -> using host @c03f3900
irq: -> obtained virq 17
io scheduler noop registered
io scheduler anticipatory registered (default)
io scheduler deadline registered
io scheduler cfq registered
Serial: 8250/16550 driver $Revision: 1.90 $ 4 ports, IRQ sharing enabled
serial8250.0: ttyS0 at MMIO 0xef600300 (irq = 16) is a 16450
console handover: boot [udbg0] -> real [ttyS0]
irq: irq_create_mapping(0xc03f3900, 0x0)
irq: -> using host @c03f3900
irq: -> existing mapping on virq 16
ef600300.serial: ttyS0 at MMIO 0xef600300 (irq = 16) is a 16450
irq: irq_create_mapping(0xc03f390

Re: [kvm-devel] buliding and testing PowerPC KVM

2008-03-25 Thread Hollis Blanchard
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] [PATCH] Move kvm_get_pit to libkvm.c common code

2008-03-21 Thread Hollis Blanchard
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] [PATCH][QEMU] Use a separate device for in-kernel PIT

2008-03-21 Thread Hollis Blanchard
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] buliding and testing PowerPC KVM

2008-03-21 Thread Hollis Blanchard
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] [RFC/PATCH 05/15] KVM_MAX_VCPUS

2008-03-20 Thread Hollis Blanchard
On Thu, 2008-03-20 at 17:24 +0100, Carsten Otte wrote:
> Index: kvm/include/linux/kvm_host.h
> ===
> --- kvm.orig/include/linux/kvm_host.h
> +++ kvm/include/linux/kvm_host.h
> @@ -24,7 +24,11 @@
> 
>  #include 
> 
> +#ifdef CONFIG_S390
> +#define KVM_MAX_VCPUS 64
> +#else
>  #define KVM_MAX_VCPUS 16
> +#endif
>  #define KVM_MEMORY_SLOTS 32
>  /* memory slots that does not exposed to userspace */
>  #define KVM_PRIVATE_MEM_SLOTS 4
> 
Why don't we just define this in  ?

-- 
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

2008-03-19 Thread Hollis Blanchard
On Wed, 2008-03-19 at 09:45 -0500, Jerone Young wrote:
> Add dynamic device tree manipulation & change uboot loader for PPC
> bamboo board model
> 
> This patch adds code to dynamically manipulate the device tree when
> loaded into memory. This allows us to finally have the ability to
> manipulate the kernel command line & initrd from the qemu command
> line. This will also let us setup different settings for the board.
> 
> This patch also now uses new uboot loader load_uimage() to load kernel
> image.
> 
> Signed-off-by: Jerone Young <[EMAIL PROTECTED]>

You've introduced the following warnings:

../hw/ppc440_bamboo.c: In function `bamboo_init':
../hw/ppc440_bamboo.c:151: warning: passing arg 2 of `load_device_tree'
makes integer from pointer without a cast
../hw/ppc440_bamboo.c:166: warning: passing arg 4 of `dt_string'
discards qualifiers from pointer target type
../hw/ppc440_bamboo.c:31: warning: 'buf' might be used uninitialized in
this function

-- 
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

2008-03-19 Thread Hollis Blanchard
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] Fix undefined refrence of qemu_system_device_hot_add for non x86 archs

2008-03-19 Thread Hollis Blanchard
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 3 of 7] Create new load_uimage() & gunzip support to uboot loader in Qemu

2008-03-18 Thread Hollis Blanchard
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 5 of 7] Add dynamic device tree manipulation & change uboot loader for PPC bamboo board model

2008-03-18 Thread Hollis Blanchard
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 2 of 7] Add libfdt support to qemu

2008-03-18 Thread Hollis Blanchard
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  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 
> > > +/* 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

2008-03-18 Thread Hollis Blanchard
nclude "device_tree.h"
> 
> -#define KERNEL_LOAD_ADDR 0x40 /* uboot loader puts kernel at 4MB */
> -
> -#include "qemu-kvm.h"
> +#define BINARY_DEVICE_TREE_FILE "bamboo.dtb"
> 
>  /* PPC 440 refrence demo board

Could you fix this typo while you're at it?

> @@ -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.

>   target_phys_addr_t ram_bases[2], ram_sizes[2];
>   qemu_irq *pic;
>   CPUState *env;
> - target_ulong ep;
> + target_ulong ep=0;
> + target_ulong la=0;
>   int is_linux=1; /* Will assume 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

2008-03-18 Thread Hollis Blanchard
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 
> +/* 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 3 of 7] Create new load_uimage() & gunzip support to uboot loader in Qemu

2008-03-18 Thread Hollis Blanchard
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 7 of 7] Add ability to specify ram on command line for bamboo board model

2008-03-18 Thread Hollis Blanchard
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) (a>>20)
> +#define mb_to_bytes(a) (a<<20)
> +
>  /* 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 0 of 7] [v2] PowerPC kvm-userspace patches

2008-03-14 Thread Hollis Blanchard
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

2008-03-14 Thread Hollis Blanchard
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 N<<20
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 4 of 7] Add PPC 440EP bamboo board device tree source & binary into qemu

2008-03-14 Thread Hollis Blanchard
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{z8>e4Fe^AByb7!$O3dMEmvh&KolS?3uUj
> zE(j}LbMXs^;6?BYc#n~S5;m0tLo`L4=+D`
> z46qsjz%IaZKjZgJ?9XH0c>4IqGXU>fl;4NN=9%vW>`P`mAb8!_D7=dODoZ&ZO<6k4
> zb0G4~9=V!7GV?u_#H 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) zw+`L^@)6Cq45hVQv;0tI_$|a 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;zh>XoI+`17X621rJ39fD-8uV2hgxYlcsGr#&HuXx1bc6T_Y=h*
> zkQ^*eFmo4pj{h^yr5>J3|ID*p+h_6gD9`vNuSwqS+$H*)N8Q5O$DOmxpr}C(f0Zu0
> zcn+UIFQ482gU`yp;j>Knh?H^vB#q;mRcKf#d-dP$DV47;f&<3eyXLy([EMAIL PROTECTED]
> z!|}4h-LsQpbROvD)&zBQz3E=Ed&}K>[EMAIL PROTECTED]
> z)[EMAIL PROTECTED]|w72>TDVq$KN>LA+(Py+kUqLui(~wf4U}0
> zhjQ;rL!Guk?O!Q2`gB)o-OGKtS8Cm`Pj{u(k-K!3mm+KUpx*oHoHw4GdufY>p%i85
> zbCDz^Y?bkeFyk~2MFKoe3w--b69FNYe!-;3DyY2%=6eG|aTs&)adlh>kRk$} zAjPM1k?~`w;#5rWAxcEC&l#TyKZ!HptFRC*NUTjqT?6FOzLYd%oU24qQO)uY(8>I0
> zRLocwBK5xK6{s|EzlGvR&sV&[EMAIL PROTECTED]@VJxdTOSCzkOH~fPEQAP25L2Z>
> f#wo+spSs3fM}Eo*?B%_#$nY+!FrO 
> 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>;
> +   

Re: [kvm-devel] [kvm-ppc-devel] [PATCH 0 of 7] PowerPC kvm-userspace patches

2008-03-12 Thread Hollis Blanchard
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] [kvm-ppc-devel] [PATCH 5 of 7] Add dynamic device tree manipulation & change uboot loader for PPC bamboo board model

2008-03-12 Thread Hollis Blanchard
 initrd_size;
> + else
> + dt_base = kernel_size + la;
> +
> + fdt = load_device_tree(buf, (phys_ram_base + dt_base));
> + if (fdt == NULL) {
> + printf("Loading device tree failed!\n");
> + exit(1);
> + }
> +
> + printf("device tree address is at guest address: 0x%lx\n", 
> + (unsigned long) dt_base);
> +
> + /* manipulate device tree in memory */
> + set_dt_cpu_0_clock_freq_prop(fdt, cpu_freq);
> + set_dt_cpu_0_timebase_prop(fdt, timebase_freq);
> +
> + set_dt_initrd_start_prop(fdt, initrd_base);
> + set_dt_initrd_end_prop(fdt, (initrd_base + initrd_size));
> +
> + set_dt_bootargs_prop(fdt, kernel_cmdline);
> +#endif
> +
>   if (kvm_enabled()) {
> - /* XXX insert TLB entries */
> - env->gpr[1] = (16<<20) - 8;
> - env->gpr[4] = initrd_base;
> - env->gpr[5] = initrd_size;
> 
> - env->nip = ep;
> + /* XXX insert TLB entries */
> + env->gpr[1] = (16<<20) - 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 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#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 4 of 7] Add PPC 440EP bamboo board device tree source & binary into qemu

2008-03-12 Thread Hollis Blanchard
s */
> +0 eed0 4 /* IACK */
> +0 eed0 4 /* Special cycle */
> +0 ef40 40>;  /* Internal registers */
> +
> + /* Outbound ranges, one memory and one IO,
> +  * later cannot be changed. Chip supports a second
> +  * IO range but we don't use it for now
> +  */
> + ranges = <0200 0 a000 0 a000 0 2000
> +   0100 0  0 e800 0 0001>;
> +
> + /* Inbound 2GB range starting at 0 */
> + dma-ranges = <4200 0 0 0 0 0 8000>;
> +
> + /* Bamboo has all 4 IRQ pins tied together per slot */
> + interrupt-map-mask = ;
> + interrupt-map = <
> + /* IDSEL 1 */
> + 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 3 of 7] Create new load_uboot() & gunzip support to uboot loader in Qemu

2008-03-12 Thread Hollis Blanchard
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 1 of 7] Add libfdt to KVM userspace

2008-03-12 Thread Hollis Blanchard
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 7 of 7] Add ability to specify ram on command line for bamboo board model

2008-03-12 Thread Hollis Blanchard
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] [PATCH 3/6] kvm: qemu: Add option for enable/disable in kernel PIT

2008-03-11 Thread Hollis Blanchard
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] [kvm-ppc-devel] [PATCH] Add --disable-cpu-emulation to qemu help screen

2008-02-27 Thread Hollis Blanchard
On Wed, 2008-02-27 at 16:34 -0600, Jerone Young wrote:
> # HG changeset patch
> # User Jerone Young <[EMAIL PROTECTED]>
> # Date 1204151598 21600
> # Branch merge
> # Node ID cd9eab52ef2d78809540518c4e18f4730d5d8400
> # Parent  f255b23b6ef9461be4ee18fa0745f30c4fb66e6a
> Add --disable-cpu-emulation to qemu help screen.
> 
> This patch adds a line to the help screen of configure for qemu to show the 
> option --disable-cpu-emulation
> 
> Signed-off-by: Jerone Young <[EMAIL PROTECTED]>
> 
> diff --git a/qemu/configure b/qemu/configure
> --- a/qemu/configure
> +++ b/qemu/configure
> @@ -445,6 +445,7 @@ echo "  --fmod-inc   path to
>  echo "  --fmod-inc   path to FMOD includes"
>  echo "  --enable-uname-release=R Return R for uname -r in usermode emulation"
>  echo "  --sparc_cpu=VBuild qemu for Sparc architecture v7, v8, 
> v8plus, v8plusa, v9"
> +echo "  --no-cpu-emulation   disables use of qemu cpu emulation code"
>  echo ""
>  echo "NOTE: The object files are built at the place where configure is 
> launched"
>  exit 1

The patch summary is correct, but the help text is not.

-- 
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

2008-02-27 Thread Hollis Blanchard
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] Top level kvm-userspace directory getting crowded ... need new dir for qemu dependencies

2008-02-27 Thread Hollis Blanchard
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] Top level kvm-userspace directory getting crowded ... need new dir for qemu dependencies

2008-02-27 Thread Hollis Blanchard
On Wed, 2008-02-27 at 21:25 +0200, Avi Kivity wrote:
> Hollis Blanchard wrote:
> > 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.
> >   
> 
> Is Xen ppc discontinued?

I have no plans to continue development; I can't speak for anybody else.

> > 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.
> 
> It doesn't have to be a package; it can be as simple as a tarball that 
> people have to make; && sudo make install before compiling kvm, the same 
> as other prerequisite libraries.

Sure. Let's put that tarball inside the qemu directory, and then have it
extracted and built automatically when the user types "make".

I'm really not clear on what advantage you think will be gained here.

> The barrier should be whether we need to carry local changes or not.  If 
> we can use upstream as is, then it should be installed independently.

So let me get this straight... you think it's cool to awk kernel source,
but not to copy library code that was designed to be copied in the first
place? Seriously? Would it be more palatable to you if I ran awk over
arch/powerpc/boot/libdft?

-- 
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

2008-02-27 Thread Hollis Blanchard
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 thou

Re: [kvm-devel] Top level kvm-userspace directory getting crowded ... need new dir for qemu dependencies

2008-02-27 Thread Hollis Blanchard
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] Top level kvm-userspace directory getting crowded ... need new dir for qemu dependencies

2008-02-27 Thread Hollis Blanchard
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] [kvm-ppc-devel] [PATCH] Fix PowerPC Qemu CPU initilization when using target-ppc/fake-exec.c

2008-02-26 Thread Hollis Blanchard
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

2008-02-25 Thread Hollis Blanchard
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] [PATCH] Using kzalloc to avoid allocating kvm_regs from kernel stack

2008-02-25 Thread Hollis Blanchard
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] [kvm-ppc-devel] upstream PowerPC qemu breakage

2008-02-18 Thread Hollis Blanchard

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] [kvm-ppc-devel] upstream PowerPC qemu breakage

2008-02-18 Thread Hollis Blanchard
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] [patch 3/5] KVM: hypercall batching

2008-02-17 Thread Hollis Blanchard
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 
>  #include 
>  #include 
> +#include 
> +
> +#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

2008-02-15 Thread Hollis Blanchard
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

2008-02-14 Thread Hollis Blanchard
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

2008-02-12 Thread Hollis Blanchard
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


Re: [kvm-devel] [PATCH] Make non-x86 arch partially support makesync.

2008-02-12 Thread Hollis Blanchard
On Tue, 2008-02-12 at 22:06 +0800, Zhang, Xiantao wrote:
> Hollis Blanchard wrote:
> > 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(-)
> 
> Hi, Hollis
>   This is a intial version to support more arch. Yes, as you
> pointed, we also need more work to support it fully. Could you make a
> patch for that ?

It's not just that the original doesn't support all architectures; I
think the comments and suggestions I made need to be addressed before it
can be committed even for just ia64.

-- 
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

2008-02-11 Thread Hollis Blanchard
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

2008-02-06 Thread Hollis Blanchard
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

2008-02-05 Thread Hollis Blanchard
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 0 of 3] RFC: creating a particular vcpu type

2008-02-05 Thread Hollis Blanchard
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 3 of 3] [POWERPC] Implement an ioctl that creates a vcpu of a particular type

2008-02-05 Thread Hollis Blanchard

On Tue, 2008-02-05 at 13:52 +0100, Christian Ehrhardt wrote:
> Hollis Blanchard wrote:
> > 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]>
> > 
> [..]
> > +static struct kvmppc_core_spec kvmppc_supported_guests[] = {
> > +   {
> > +   .name = "ppc440",
> > +   .vcpu_size = sizeof(struct kvm_vcpu),
> 
> Just for clarification, here you want to add other configuration
> settings you want to imply with the set of the cpu core right ?
> E.g. we have discussed the pvr initialization on kvm-powerpc-devel and
> #kvm, do you intend to define the ppv440-pvr we want to set here in
> this struct?

Yes, I tried to hint at this in the first mail. Note however, that this
approach is an alternate design: rather than embedded a "cpu type"
integer in the vcpu and adding switch tables at all decision points, we
will add callbacks to this structure.

I have a followup patch to handle our TLB type disambiguation, but it
was late and I didn't want to clean it up.

> > +
> > +   if (type.typelen > 32) {
> > +   r = -E2BIG;
> > +   goto out;
> > +   }
> > +
> > +   coretype = vmalloc(type.typelen);
> > +   if (!coretype) {
> > +   r = -ENOMEM;
> > +   goto out;
> > +   }
> 
> If I don't overlook something we could use strnlen here instead of
> type.typelen and not trust userspace in any way (which is always good)
> that it passes us a good value.

I wasn't liking that part of the interface anyways, so strnlen() sounds
good to me. :)

> > +struct kvm_vcpu_create_type {
> > +   __u32 id;
> > +   __u32 typelen;
> > +   char type[0];
> > +};
> 
> This should work for other architectures too. While we need to specify
> cpu cores here others might specify something completely different
> with the same interface.
> So kvm_vcpu_create_type might be misleading while something like
> kvm_vcpu_create_info might be more generic.

Yup, I think it makes sense to abstract this from a string to an
arch-defined structure.

Another arch-specific interface might be needed to query capabilities in
a similar way. Right now the extension stuff is based on encoded
constants, but I'd explicitly like to avoid that because then you must
also know the meaning of each constant. In other words, if you have
#define POWERPC440 1
both the kernel and userspace must understand what "1" means as a
parameter. That's why I went with strings...

On the other hand, encoded constants might be a better approach. For
example, if we change what information we want to pass down for a
particular vcpu type, we could do it like this:
struct vcpu_type {
__u32 type;
union {
struct { foo; } type1;
struct { foo; bar; } type2;
}
}
and we could then use extension queries to find out if type=1 and type=2
are supported.

> Just to get some background - do anyone else see the need to specify a
> detail on vcpu_create for their implementation in future ?

After talking to Carsten, it sounds like S390 could use this approach as
well.

> Finally I wanted to add something more to think about while we discuss
> this interface. The approach itself looks good to me, but it might
> somewhen need an extension we should discuss and decide by now.
> Think of the following situation:
> a) In two years we might have some generic features which are part of
> the shared vcpu struct and we would need to specify sometihng for
> these features on vcpu_create
> b) At the same time we would still have our need to specify arch
> dependent information on vcpu create.
> Because the KVM_CREATE_VCPU_TYPE ioctl implies a vcpu create, we would
> need to set up these new stuff prior to that create in another new
> call. I think it might be much better if we would think now of how to
> specify multiple things in this extended vcpu_create.

Agreed, I think we can handle that with my thoughts above.

> This might either be done by removing the implicit vcpu creation for a
> workflow like that:
>   1. set generic feature a enabled
>   2. set generic feature b disabled
>   3. set arch specific core type to foo
>   4. create_vcpu (using all that)
> Or on the other Hand we might think of a encapsulated or chained
> information that allows us to pass a variable number of settings with
> KVM_CREA

[kvm-devel] [PATCH 1 of 3] Pass an opaque parameter through kvm_vm_ioctl_vcpu_create() to kvm_arch_vcpu_create()

2008-02-04 Thread Hollis Blanchard
# 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

2008-02-04 Thread Hollis Blanchard
# 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(K

[kvm-devel] [PATCH 0 of 3] RFC: creating a particular vcpu type

2008-02-04 Thread Hollis Blanchard
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 2 of 3] Export kvm_vm_ioctl_create_vcpu() to be called from architecture modules

2008-02-04 Thread Hollis Blanchard
# HG changeset patch
# User Hollis Blanchard <[EMAIL PROTECTED]>
# Date 1202189666 21600
# Node ID 7dd50dab9096c8e0125792e3f48083c3f47fceab
# Parent  bede9476e203f5bf59d21cc3cd71a30de2ce2c44

Signed-off-by: Hollis Blanchard <[EMAIL PROTECTED]>

---
2 files changed, 3 insertions(+), 1 deletion(-)
include/linux/kvm_host.h |1 +
virt/kvm/kvm_main.c  |3 ++-


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
@@ -208,6 +208,7 @@ int kvm_vm_ioctl_set_memory_region(struc
   struct
   kvm_userspace_memory_region *mem,
   int user_alloc);
+int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, int n, void *opaque);
 long kvm_arch_vm_ioctl(struct file *filp,
   unsigned int ioctl, unsigned long arg);
 void kvm_arch_destroy_vm(struct kvm *kvm);
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, void *opaque)
+int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, int n, void *opaque)
 {
int r;
struct kvm_vcpu *vcpu;
@@ -774,6 +774,7 @@ vcpu_destroy:
kvm_arch_vcpu_destroy(vcpu);
return r;
 }
+EXPORT_SYMBOL_GPL(kvm_vm_ioctl_create_vcpu);
 
 static int kvm_vcpu_ioctl_set_sigmask(struct kvm_vcpu *vcpu, sigset_t *sigset)
 {

-
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.

2008-02-04 Thread Hollis Blanchard
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


Re: [kvm-devel] [kvm-ppc-devel] [RFC PATCH] KVM for PowerPC: simpler TLB handling, better page management

2008-02-01 Thread Hollis Blanchard
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 
> >  #include 
> >  #include 
> > +#include 
> >  #ifdef CONFIG_PPC64
> >  #include 
> >  #include 
> > @@ -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] Use CONFIG_PREEMPT_NOTIFIERS around struct preempt_notifier

2008-01-29 Thread Hollis Blanchard

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] Use CONFIG_PREEMPT_NOTIFIERS around struct preempt_notifier

2008-01-29 Thread Hollis Blanchard
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] [kvm-ppc-devel] [PATCH] Clean up KVM/QEMU interaction

2008-01-29 Thread Hollis Blanchard
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] [PATCH] Cleanup extern declerations for now removed vcpu_env in Qemu

2008-01-29 Thread Hollis Blanchard
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


  1   2   3   >