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 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;
> > +
> > +static u32 kvmppc_44x_tlb_shadow_attrib(u32 attrib, int usermode)
> > +{
> > +   /* XXX remove mask when Linux is fixed */
> > +   attrib &= 0xf03f;
>
> What does that mean?  Is this the "44x TLB handler writes to reserved
> fields" issue?  If so, could you comment that a little more verbosely?

Yup, you're right. Actually, what I should really do is this:
attrib &= PPC44x_TLB_ATTR_MASK | PPC44x_TLB_PERM_MASK;

> Or you could just send a patch to fix Linux... ;).

I had a look at it once, but didn't have time to grok the assembly bit 
manipulations. I guess nobody else has either. :)

> > +
> > +   if 

Re: [kvm-devel] [PATCH 3 of 3] [KVM POWERPC] PowerPC 440 KVM implementation

2008-04-07 Thread Arnd Bergmann
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.

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?

Arnd <><

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

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

> 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;
> +
> +static u32 kvmppc_44x_tlb_shadow_attrib(u32 attrib, int usermode)
> +{
> + /* XXX remove mask when Linux is fixed */
> + attrib &= 0xf03f;

What does that mean?  Is this the "44x TLB handler writes to reserved
fields" issue?  If so, could you comment that a little more verbosely?

Or you could just send a patch to fix Linux... ;).

> +
> + if (!usermode) {
> + /* Guest is in supervisor mode, so we need to translate guest
> +  * supervisor permissions into user permissions. */
> + attrib &= ~PPC44x_TLB_USER_PERM_MASK;
> + attrib |= (attrib & PPC44x_TLB_SUPER_PERM_MASK) << 3;
> + }
> +
> + /* Make sure host can always access this memory. */
> + attrib |= PPC44x_TLB_SX|PPC44x_TLB_SR|PPC44x_TLB_SW;
> +
> + return attrib;
> +}
> +



> diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
> new file mode 100644
> --- /dev/null
> +++ b/arch/powerpc/kvm/emulate.c
> @@ -0,0 +1,75