RE: [PATCH 7/9] KVM: PPC: Emulate trap SRR1 flags properly

2010-01-21 Thread Liu Yu-B13201
 

 -Original Message-
 From: kvm-ppc-ow...@vger.kernel.org 
 [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Hollis Blanchard
 Sent: Saturday, January 09, 2010 3:30 AM
 To: Alexander Graf
 Cc: k...@vger.kernel.org; kvm-ppc; Benjamin Herrenschmidt; Liu Yu
 Subject: Re: [PATCH 7/9] KVM: PPC: Emulate trap SRR1 flags properly
 
  diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
  index 338baf9..e283e44 100644
  --- a/arch/powerpc/kvm/booke.c
  +++ b/arch/powerpc/kvm/booke.c
  @@ -82,8 +82,9 @@ static void 
 kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu,
         set_bit(priority, vcpu-arch.pending_exceptions);
   }
 
  -void kvmppc_core_queue_program(struct kvm_vcpu *vcpu)
  +void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags)
   {
  +       /* BookE does flags in ESR, so ignore those we get here */
         kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_PROGRAM);
   }
 
 Actually, I think Book E prematurely sets ESR, since it's done before
 the program interrupt is actually delivered. Architecturally, I'm not
 sure if it's a problem, but philosophically I've always wanted it to
 work the way you've just implemented for Book S.
 

ESR is updated not only by program but by data_tlb, data_storage, etc.
Should we rearrange them all? 
Also DEAR has the same situation as ESR.
Should it be updated when we decide to inject interrupt to guest?


--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 7/9] KVM: PPC: Emulate trap SRR1 flags properly

2010-01-21 Thread Alexander Graf

On 21.01.2010, at 09:09, Liu Yu-B13201 wrote:

 
 
 -Original Message-
 From: kvm-ppc-ow...@vger.kernel.org 
 [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Hollis Blanchard
 Sent: Saturday, January 09, 2010 3:30 AM
 To: Alexander Graf
 Cc: k...@vger.kernel.org; kvm-ppc; Benjamin Herrenschmidt; Liu Yu
 Subject: Re: [PATCH 7/9] KVM: PPC: Emulate trap SRR1 flags properly
 
 diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
 index 338baf9..e283e44 100644
 --- a/arch/powerpc/kvm/booke.c
 +++ b/arch/powerpc/kvm/booke.c
 @@ -82,8 +82,9 @@ static void 
 kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu,
   set_bit(priority, vcpu-arch.pending_exceptions);
 }
 
 -void kvmppc_core_queue_program(struct kvm_vcpu *vcpu)
 +void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags)
 {
 +   /* BookE does flags in ESR, so ignore those we get here */
   kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_PROGRAM);
 }
 
 Actually, I think Book E prematurely sets ESR, since it's done before
 the program interrupt is actually delivered. Architecturally, I'm not
 sure if it's a problem, but philosophically I've always wanted it to
 work the way you've just implemented for Book S.
 
 
 ESR is updated not only by program but by data_tlb, data_storage, etc.
 Should we rearrange them all? 
 Also DEAR has the same situation as ESR.
 Should it be updated when we decide to inject interrupt to guest?

If that's what the hardware does, then yes. I'm good with taking small steps 
though. So if you don't have the time to convert all of the handlers, you can 
easily start off with program interrupts.

Alex--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 7/9] KVM: PPC: Emulate trap SRR1 flags properly

2010-01-12 Thread Liu Yu-B13201
 

 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de] 
 Sent: Tuesday, January 12, 2010 8:08 AM
 To: Liu Yu-B13201
 Cc: Hollis Blanchard; k...@vger.kernel.org; kvm-ppc; Benjamin 
 Herrenschmidt; Liu Yu
 Subject: Re: [PATCH 7/9] KVM: PPC: Emulate trap SRR1 flags properly
 
  
  Anyways, since we can't test changes at the moment (Yu, 
 can you?), I'd
  settle for a comment to the effect that Book E code 
 *should* do this,
  but doesn't (rather than the comment above that says it's ok).
  
  
  Sure.
  You mean set the ESR at the moment we inject trap to guest?
  If it is not urgent, I'll do it later.
 
 It's definitely not urgent. In fact, If you could just test 
 patches I send to you I can write it and have you test it :-).
 

That would be nice.:-)
But last time I try the top tree, it seemed not work.
Maybe I need to find out the reason and fix it first...
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 7/9] KVM: PPC: Emulate trap SRR1 flags properly

2010-01-11 Thread Alexander Graf

On 11.01.2010, at 07:58, Liu Yu-B13201 wrote:

 
 
 -Original Message-
 From: kvm-ppc-ow...@vger.kernel.org 
 [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Hollis Blanchard
 Sent: Saturday, January 09, 2010 3:30 AM
 To: Alexander Graf
 Cc: k...@vger.kernel.org; kvm-ppc; Benjamin Herrenschmidt; Liu Yu
 Subject: Re: [PATCH 7/9] KVM: PPC: Emulate trap SRR1 flags properly
 
 On Thu, Jan 7, 2010 at 5:58 PM, Alexander Graf ag...@suse.de wrote:
 Book3S needs some flags in SRR1 to get to know details 
 about an interrupt.
 
 One such example is the trap instruction. It tells the 
 guest kernel that
 a program interrupt is due to a trap using a bit in SRR1.
 
 This patch implements above behavior, making WARN_ON behave 
 like WARN_ON.
 
 ... for Book S. It already works properly for Book E, 
 thankyouverymuch. ;)
 
 diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
 index 338baf9..e283e44 100644
 --- a/arch/powerpc/kvm/booke.c
 +++ b/arch/powerpc/kvm/booke.c
 @@ -82,8 +82,9 @@ static void 
 kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu,
set_bit(priority, vcpu-arch.pending_exceptions);
  }
 
 -void kvmppc_core_queue_program(struct kvm_vcpu *vcpu)
 +void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags)
  {
 +   /* BookE does flags in ESR, so ignore those we get here */
kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_PROGRAM);
  }
 
 Actually, I think Book E prematurely sets ESR, since it's done before
 the program interrupt is actually delivered. Architecturally, I'm not
 sure if it's a problem, but philosophically I've always wanted it to
 work the way you've just implemented for Book S.
 
 Anyways, since we can't test changes at the moment (Yu, can you?), I'd
 settle for a comment to the effect that Book E code *should* do this,
 but doesn't (rather than the comment above that says it's ok).
 
 
 Sure.
 You mean set the ESR at the moment we inject trap to guest?
 If it is not urgent, I'll do it later.

It's definitely not urgent. In fact, If you could just test patches I send to 
you I can write it and have you test it :-).

Alex

--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 7/9] KVM: PPC: Emulate trap SRR1 flags properly

2010-01-10 Thread Liu Yu-B13201
 

 -Original Message-
 From: kvm-ppc-ow...@vger.kernel.org 
 [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Hollis Blanchard
 Sent: Saturday, January 09, 2010 3:30 AM
 To: Alexander Graf
 Cc: k...@vger.kernel.org; kvm-ppc; Benjamin Herrenschmidt; Liu Yu
 Subject: Re: [PATCH 7/9] KVM: PPC: Emulate trap SRR1 flags properly
 
 On Thu, Jan 7, 2010 at 5:58 PM, Alexander Graf ag...@suse.de wrote:
  Book3S needs some flags in SRR1 to get to know details 
 about an interrupt.
 
  One such example is the trap instruction. It tells the 
 guest kernel that
  a program interrupt is due to a trap using a bit in SRR1.
 
  This patch implements above behavior, making WARN_ON behave 
 like WARN_ON.
 
 ... for Book S. It already works properly for Book E, 
 thankyouverymuch. ;)
 
  diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
  index 338baf9..e283e44 100644
  --- a/arch/powerpc/kvm/booke.c
  +++ b/arch/powerpc/kvm/booke.c
  @@ -82,8 +82,9 @@ static void 
 kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu,
         set_bit(priority, vcpu-arch.pending_exceptions);
   }
 
  -void kvmppc_core_queue_program(struct kvm_vcpu *vcpu)
  +void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags)
   {
  +       /* BookE does flags in ESR, so ignore those we get here */
         kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_PROGRAM);
   }
 
 Actually, I think Book E prematurely sets ESR, since it's done before
 the program interrupt is actually delivered. Architecturally, I'm not
 sure if it's a problem, but philosophically I've always wanted it to
 work the way you've just implemented for Book S.
 
 Anyways, since we can't test changes at the moment (Yu, can you?), I'd
 settle for a comment to the effect that Book E code *should* do this,
 but doesn't (rather than the comment above that says it's ok).
 

Sure.
You mean set the ESR at the moment we inject trap to guest?
If it is not urgent, I'll do it later.
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 7/9] KVM: PPC: Emulate trap SRR1 flags properly

2010-01-08 Thread Alexander Graf

On 08.01.2010, at 20:29, Hollis Blanchard wrote:

 On Thu, Jan 7, 2010 at 5:58 PM, Alexander Graf ag...@suse.de wrote:
 Book3S needs some flags in SRR1 to get to know details about an interrupt.
 
 One such example is the trap instruction. It tells the guest kernel that
 a program interrupt is due to a trap using a bit in SRR1.
 
 This patch implements above behavior, making WARN_ON behave like WARN_ON.
 
 ... for Book S. It already works properly for Book E, thankyouverymuch. ;)
 
 diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
 index 338baf9..e283e44 100644
 --- a/arch/powerpc/kvm/booke.c
 +++ b/arch/powerpc/kvm/booke.c
 @@ -82,8 +82,9 @@ static void kvmppc_booke_queue_irqprio(struct kvm_vcpu 
 *vcpu,
set_bit(priority, vcpu-arch.pending_exceptions);
  }
 
 -void kvmppc_core_queue_program(struct kvm_vcpu *vcpu)
 +void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags)
  {
 +   /* BookE does flags in ESR, so ignore those we get here */
kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_PROGRAM);
  }
 
 Actually, I think Book E prematurely sets ESR, since it's done before
 the program interrupt is actually delivered. Architecturally, I'm not
 sure if it's a problem, but philosophically I've always wanted it to
 work the way you've just implemented for Book S.
 
 Anyways, since we can't test changes at the moment (Yu, can you?), I'd
 settle for a comment to the effect that Book E code *should* do this,
 but doesn't (rather than the comment above that says it's ok).

Hm, can't you just write up a patch that removes the comment I put in, does the 
ESR setting in the function and do an #ifdef in emulate.c?

That way we can incrementally improve things. This series is really about 
Book3S. Improving BookE should go in a different patch.

Alex--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 7/9] KVM: PPC: Emulate trap SRR1 flags properly

2010-01-08 Thread Hollis Blanchard
On Fri, Jan 8, 2010 at 11:32 AM, Alexander Graf ag...@suse.de wrote:

 On 08.01.2010, at 20:29, Hollis Blanchard wrote:

 On Thu, Jan 7, 2010 at 5:58 PM, Alexander Graf ag...@suse.de wrote:
 Book3S needs some flags in SRR1 to get to know details about an interrupt.

 One such example is the trap instruction. It tells the guest kernel that
 a program interrupt is due to a trap using a bit in SRR1.

 This patch implements above behavior, making WARN_ON behave like WARN_ON.

 ... for Book S. It already works properly for Book E, thankyouverymuch. ;)

 diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
 index 338baf9..e283e44 100644
 --- a/arch/powerpc/kvm/booke.c
 +++ b/arch/powerpc/kvm/booke.c
 @@ -82,8 +82,9 @@ static void kvmppc_booke_queue_irqprio(struct kvm_vcpu 
 *vcpu,
        set_bit(priority, vcpu-arch.pending_exceptions);
  }

 -void kvmppc_core_queue_program(struct kvm_vcpu *vcpu)
 +void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags)
  {
 +       /* BookE does flags in ESR, so ignore those we get here */
        kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_PROGRAM);
  }

 Actually, I think Book E prematurely sets ESR, since it's done before
 the program interrupt is actually delivered. Architecturally, I'm not
 sure if it's a problem, but philosophically I've always wanted it to
 work the way you've just implemented for Book S.

 Anyways, since we can't test changes at the moment (Yu, can you?), I'd
 settle for a comment to the effect that Book E code *should* do this,
 but doesn't (rather than the comment above that says it's ok).

 Hm, can't you just write up a patch that removes the comment I put in, does 
 the ESR setting in the function and do an #ifdef in emulate.c?

 That way we can incrementally improve things. This series is really about 
 Book3S. Improving BookE should go in a different patch.

Yes, but I'd rather minimize the behavioral changes as long as we can't test it.

-Hollis
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 7/9] KVM: PPC: Emulate trap SRR1 flags properly

2010-01-08 Thread Alexander Graf

On 08.01.2010, at 20:35, Hollis Blanchard wrote:

 On Fri, Jan 8, 2010 at 11:32 AM, Alexander Graf ag...@suse.de wrote:
 
 On 08.01.2010, at 20:29, Hollis Blanchard wrote:
 
 On Thu, Jan 7, 2010 at 5:58 PM, Alexander Graf ag...@suse.de wrote:
 Book3S needs some flags in SRR1 to get to know details about an interrupt.
 
 One such example is the trap instruction. It tells the guest kernel that
 a program interrupt is due to a trap using a bit in SRR1.
 
 This patch implements above behavior, making WARN_ON behave like WARN_ON.
 
 ... for Book S. It already works properly for Book E, thankyouverymuch. ;)
 
 diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
 index 338baf9..e283e44 100644
 --- a/arch/powerpc/kvm/booke.c
 +++ b/arch/powerpc/kvm/booke.c
 @@ -82,8 +82,9 @@ static void kvmppc_booke_queue_irqprio(struct kvm_vcpu 
 *vcpu,
set_bit(priority, vcpu-arch.pending_exceptions);
  }
 
 -void kvmppc_core_queue_program(struct kvm_vcpu *vcpu)
 +void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags)
  {
 +   /* BookE does flags in ESR, so ignore those we get here */
kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_PROGRAM);
  }
 
 Actually, I think Book E prematurely sets ESR, since it's done before
 the program interrupt is actually delivered. Architecturally, I'm not
 sure if it's a problem, but philosophically I've always wanted it to
 work the way you've just implemented for Book S.
 
 Anyways, since we can't test changes at the moment (Yu, can you?), I'd
 settle for a comment to the effect that Book E code *should* do this,
 but doesn't (rather than the comment above that says it's ok).
 
 Hm, can't you just write up a patch that removes the comment I put in, does 
 the ESR setting in the function and do an #ifdef in emulate.c?
 
 That way we can incrementally improve things. This series is really about 
 Book3S. Improving BookE should go in a different patch.
 
 Yes, but I'd rather minimize the behavioral changes as long as we can't test 
 it.

Right. That's why the semantics of flags are Book3S specific right now and as 
such the comment is correct.

The behavioral change we need to do later is to take the ESR setting into 
queue_program, but that should wait until after someone has a board to test 
things on again ;-). The reminder that we need to do this is hereby archived in 
the mails :).

Alex--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html