[PATCH 01/23] Pass PVR in sregs

[PATCH 01/23] Pass PVR in sregs

[Alexander Graf]

Right now sregs is unused on PPC, so we can use it for initialization
of the CPU.

KVM on BookE always virtualizes the host CPU. On PPC64 we go a step further
and take the PVR from userspace that tells us what kind of CPU we are supposed
to virtualize, because we support PPC32 and PPC64 guests.

In order to get that information, we use the sregs ioctl, because we don't
want to reset the guest CPU on every normal register set.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/powerpc/include/asm/kvm.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/asm/kvm.h
index bb2de6a..96b02cd 100644
--- a/arch/powerpc/include/asm/kvm.h
+++ b/arch/powerpc/include/asm/kvm.h
@@ -46,6 +46,7 @@ struct kvm_regs {
 };
 
 struct kvm_sregs {
+	__u64 pvr;
 };
 
 struct kvm_fpu {
-- 
1.6.0.2

Re: [PATCH 01/23] Pass PVR in sregs

[Avi Kivity]

On 07/07/2009 05:17 PM, Alexander Graf wrote:
> Right now sregs is unused on PPC, so we can use it for initialization
> of the CPU.
>
> KVM on BookE always virtualizes the host CPU. On PPC64 we go a step further
> and take the PVR from userspace that tells us what kind of CPU we are supposed
> to virtualize, because we support PPC32 and PPC64 guests.
>
> In order to get that information, we use the sregs ioctl, because we don't
> want to reset the guest CPU on every normal register set.
>
> Signed-off-by: Alexander Graf<agraf@suse.de>
> ---
>   arch/powerpc/include/asm/kvm.h |    1 +
>   1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/asm/kvm.h
> index bb2de6a..96b02cd 100644
> --- a/arch/powerpc/include/asm/kvm.h
> +++ b/arch/powerpc/include/asm/kvm.h
> @@ -46,6 +46,7 @@ struct kvm_regs {
>   };
>
>   struct kvm_sregs {
> +	__u64 pvr;
>   };
>    

You can only do that if existing userspace never calls KVM_SET_SREGS.  
Hollis?

Also, make sure to reserve a bunch of space in there so you if you 
forget something you can add it later.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 01/23] Pass PVR in sregs

[Hollis Blanchard]

On Tue, 2009-07-07 at 18:40 +0300, Avi Kivity wrote:
> On 07/07/2009 05:17 PM, Alexander Graf wrote:
> > Right now sregs is unused on PPC, so we can use it for initialization
> > of the CPU.
> >
> > KVM on BookE always virtualizes the host CPU. On PPC64 we go a step further
> > and take the PVR from userspace that tells us what kind of CPU we are supposed
> > to virtualize, because we support PPC32 and PPC64 guests.
> >
> > In order to get that information, we use the sregs ioctl, because we don't
> > want to reset the guest CPU on every normal register set.
> >
> > Signed-off-by: Alexander Graf<agraf@suse.de>
> > ---
> >   arch/powerpc/include/asm/kvm.h |    1 +
> >   1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/asm/kvm.h
> > index bb2de6a..96b02cd 100644
> > --- a/arch/powerpc/include/asm/kvm.h
> > +++ b/arch/powerpc/include/asm/kvm.h
> > @@ -46,6 +46,7 @@ struct kvm_regs {
> >   };
> >
> >   struct kvm_sregs {
> > +	__u64 pvr;
> >   };
> >    
> 
> You can only do that if existing userspace never calls KVM_SET_SREGS.  
> Hollis?

It doesn't.

> Also, make sure to reserve a bunch of space in there so you if you 
> forget something you can add it later.

Agreed.

The PVR register is basically the equivalent of cpuid. It might make
more sense to exit to qemu to handle those accesses. Today, PVR reads
are emulated in-kernel.

Hmm, I don't remember where arch->vcpu.pvr is being set at all for 440
and e500...

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: [PATCH 01/23] Pass PVR in sregs

[Alexander Graf]

On 08.07.2009, at 00:50, Hollis Blanchard wrote:

> On Tue, 2009-07-07 at 18:40 +0300, Avi Kivity wrote:
>> On 07/07/2009 05:17 PM, Alexander Graf wrote:
>>> Right now sregs is unused on PPC, so we can use it for  
>>> initialization
>>> of the CPU.
>>>
>>> KVM on BookE always virtualizes the host CPU. On PPC64 we go a  
>>> step further
>>> and take the PVR from userspace that tells us what kind of CPU we  
>>> are supposed
>>> to virtualize, because we support PPC32 and PPC64 guests.
>>>
>>> In order to get that information, we use the sregs ioctl, because  
>>> we don't
>>> want to reset the guest CPU on every normal register set.
>>>
>>> Signed-off-by: Alexander Graf<agraf@suse.de>
>>> ---
>>>  arch/powerpc/include/asm/kvm.h |    1 +
>>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/ 
>>> asm/kvm.h
>>> index bb2de6a..96b02cd 100644
>>> --- a/arch/powerpc/include/asm/kvm.h
>>> +++ b/arch/powerpc/include/asm/kvm.h
>>> @@ -46,6 +46,7 @@ struct kvm_regs {
>>>  };
>>>
>>>  struct kvm_sregs {
>>> +	__u64 pvr;
>>>  };
>>>
>>
>> You can only do that if existing userspace never calls KVM_SET_SREGS.
>> Hollis?
>
> It doesn't.
>
>> Also, make sure to reserve a bunch of space in there so you if you
>> forget something you can add it later.
>
> Agreed.
>
> The PVR register is basically the equivalent of cpuid. It might make
> more sense to exit to qemu to handle those accesses. Today, PVR reads
> are emulated in-kernel.

I actually use it in 970.c to find out which guest MMU to choose from.
So even if we were to do it in userspace, we'd still need the  
information what CPU to create in the guest on the kernel side. Which  
in turn means we don't win anything from leaving the PVR emulation to  
userspace.

> Hmm, I don't remember where arch->vcpu.pvr is being set at all for 440
> and e500...

It used to be in some creation code - either general kvm or vcpu. But  
some recent patch removed vcpu->arch.pvr and made emulate.c do an  
mfspr(SPRN_PVR).

Alex

RE: [PATCH 01/23] Pass PVR in sregs

[Liu Yu-B13201]

 

> -----Original Message-----
> From: kvm-ppc-owner@vger.kernel.org 
> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Alexander Graf
> Sent: Wednesday, July 08, 2009 7:23 AM
> To: Hollis Blanchard
> Cc: Avi Kivity; kvm-ppc@vger.kernel.org; arnd@arndb.de; 
> kwolf@redhat.com
> Subject: Re: [PATCH 01/23] Pass PVR in sregs
> 
> 
> On 08.07.2009, at 00:50, Hollis Blanchard wrote:
> >
> > The PVR register is basically the equivalent of cpuid. It might make
> > more sense to exit to qemu to handle those accesses. Today, 
> PVR reads
> > are emulated in-kernel.
> 
> I actually use it in 970.c to find out which guest MMU to choose from.
> So even if we were to do it in userspace, we'd still need the  
> information what CPU to create in the guest on the kernel 
> side. Which  
> in turn means we don't win anything from leaving the PVR 
> emulation to  
> userspace.
> 
> > Hmm, I don't remember where arch->vcpu.pvr is being set at 
> all for 440
> > and e500...
> 
> It used to be in some creation code - either general kvm or 
> vcpu. But  
> some recent patch removed vcpu->arch.pvr and made emulate.c do an  
> mfspr(SPRN_PVR).
> 

Yes. Since the demand to emulate PVR for 440 and e500 is still vague.
Directly return the real value can simplify the code, and latter patches
can easily change it.
The same thing goes for PIR.

RE: [PATCH 01/23] Pass PVR in sregs

[Hollis Blanchard]

On Wed, 2009-07-08 at 10:28 +0800, Liu Yu-B13201 wrote:
> 
> > -----Original Message-----
> > From: kvm-ppc-owner@vger.kernel.org 
> > [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Alexander Graf
> > Sent: Wednesday, July 08, 2009 7:23 AM
> > To: Hollis Blanchard
> > Cc: Avi Kivity; kvm-ppc@vger.kernel.org; arnd@arndb.de; 
> > kwolf@redhat.com
> > Subject: Re: [PATCH 01/23] Pass PVR in sregs
> > 
> > 
> > On 08.07.2009, at 00:50, Hollis Blanchard wrote:
> > >
> > > The PVR register is basically the equivalent of cpuid. It might make
> > > more sense to exit to qemu to handle those accesses. Today, 
> > PVR reads
> > > are emulated in-kernel.
> > 
> > I actually use it in 970.c to find out which guest MMU to choose from.
> > So even if we were to do it in userspace, we'd still need the  
> > information what CPU to create in the guest on the kernel 
> > side. Which  
> > in turn means we don't win anything from leaving the PVR 
> > emulation to  
> > userspace.
> > 
> > > Hmm, I don't remember where arch->vcpu.pvr is being set at 
> > all for 440
> > > and e500...
> > 
> > It used to be in some creation code - either general kvm or 
> > vcpu. But  
> > some recent patch removed vcpu->arch.pvr and made emulate.c do an  
> > mfspr(SPRN_PVR).
> > 
> 
> Yes. Since the demand to emulate PVR for 440 and e500 is still vague.
> Directly return the real value can simplify the code, and latter patches
> can easily change it.
> The same thing goes for PIR.

By the way, I don't like that PVR patch you sent to Avi (and he
committed). It's my own fault for not reading more closely, but in the
future could you send me patches that touch code which isn't
e500-specific?

-- 
Hollis Blanchard
IBM Linux Technology Center

RE: [PATCH 01/23] Pass PVR in sregs

[Liu Yu-B13201]

 

> -----Original Message-----
> From: kvm-ppc-owner@vger.kernel.org 
> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Hollis Blanchard
> Sent: Friday, July 10, 2009 8:52 AM
> To: Liu Yu-B13201
> Cc: Alexander Graf; Avi Kivity; kvm-ppc@vger.kernel.org; 
> arnd@arndb.de; kwolf@redhat.com
> Subject: RE: [PATCH 01/23] Pass PVR in sregs
> 
> On Wed, 2009-07-08 at 10:28 +0800, Liu Yu-B13201 wrote:
> > 
> > > -----Original Message-----
> > > From: kvm-ppc-owner@vger.kernel.org 
> > > [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Alexander Graf
> > > Sent: Wednesday, July 08, 2009 7:23 AM
> > > To: Hollis Blanchard
> > > Cc: Avi Kivity; kvm-ppc@vger.kernel.org; arnd@arndb.de; 
> > > kwolf@redhat.com
> > > Subject: Re: [PATCH 01/23] Pass PVR in sregs
> > > 
> > > 
> > > On 08.07.2009, at 00:50, Hollis Blanchard wrote:
> > > >
> > > > The PVR register is basically the equivalent of cpuid. 
> It might make
> > > > more sense to exit to qemu to handle those accesses. Today, 
> > > PVR reads
> > > > are emulated in-kernel.
> > > 
> > > I actually use it in 970.c to find out which guest MMU to 
> choose from.
> > > So even if we were to do it in userspace, we'd still need the  
> > > information what CPU to create in the guest on the kernel 
> > > side. Which  
> > > in turn means we don't win anything from leaving the PVR 
> > > emulation to  
> > > userspace.
> > > 
> > > > Hmm, I don't remember where arch->vcpu.pvr is being set at 
> > > all for 440
> > > > and e500...
> > > 
> > > It used to be in some creation code - either general kvm or 
> > > vcpu. But  
> > > some recent patch removed vcpu->arch.pvr and made 
> emulate.c do an  
> > > mfspr(SPRN_PVR).
> > > 
> > 
> > Yes. Since the demand to emulate PVR for 440 and e500 is 
> still vague.
> > Directly return the real value can simplify the code, and 
> latter patches
> > can easily change it.
> > The same thing goes for PIR.
> 
> By the way, I don't like that PVR patch you sent to Avi (and he
> committed). It's my own fault for not reading more closely, but in the
> future could you send me patches that touch code which isn't
> e500-specific?
> 

OK.

[PATCH 01/23] Pass PVR in sregs

[Alexander Graf]

Right now sregs is unused on PPC, so we can use it for initialization
of the CPU.

KVM on BookE always virtualizes the host CPU. On Book3s we go a step further
and take the PVR from userspace that tells us what kind of CPU we are supposed
to virtualize, because we support Book3s_32 and Book3s_64 guests.

In order to get that information, we use the sregs ioctl, because we don't
want to reset the guest CPU on every normal register set.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/powerpc/include/asm/kvm.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/asm/kvm.h
index bb2de6a..b82bd68 100644
--- a/arch/powerpc/include/asm/kvm.h
+++ b/arch/powerpc/include/asm/kvm.h
@@ -46,6 +46,8 @@ struct kvm_regs {
 };
 
 struct kvm_sregs {
+	__u64 pvr;
+	char pad[1016];
 };
 
 struct kvm_fpu {
-- 
1.6.0.2

Re: [PATCH 01/23] Pass PVR in sregs

[Hollis Blanchard]

Hi Avi, would you apply this patch? Looks like the corresponding qemu
patch went in a while ago, so the qemu build has been broken for some
time.

Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>

-- 
Hollis Blanchard
IBM Linux Technology Center

On Thu, 2009-07-16 at 15:29 +0200, Alexander Graf wrote:
> Right now sregs is unused on PPC, so we can use it for initialization
> of the CPU.
> 
> KVM on BookE always virtualizes the host CPU. On Book3s we go a step further
> and take the PVR from userspace that tells us what kind of CPU we are supposed
> to virtualize, because we support Book3s_32 and Book3s_64 guests.
> 
> In order to get that information, we use the sregs ioctl, because we don't
> want to reset the guest CPU on every normal register set.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/powerpc/include/asm/kvm.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/asm/kvm.h
> index bb2de6a..b82bd68 100644
> --- a/arch/powerpc/include/asm/kvm.h
> +++ b/arch/powerpc/include/asm/kvm.h
> @@ -46,6 +46,8 @@ struct kvm_regs {
>  };
> 
>  struct kvm_sregs {
> +	__u64 pvr;
> +	char pad[1016];
>  };
> 
>  struct kvm_fpu {