Patchwork [RFC,04/17] KVM: PPC64: booke: Add guest computation mode for irq delivery

login
register
mail settings
Submitter Mihai Caraman
Date June 25, 2012, 12:26 p.m.
Message ID <1340627195-11544-5-git-send-email-mihai.caraman@freescale.com>
Download mbox | patch
Permalink /patch/167074/
State New
Headers show

Comments

Mihai Caraman - June 25, 2012, 12:26 p.m.
When delivering guest IRQs, update MSR computaion mode according to guest
interrupt computation mode found in EPCR.

Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
 arch/powerpc/kvm/booke.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)
Alexander Graf - July 4, 2012, 1:40 p.m.
On 25.06.2012, at 14:26, Mihai Caraman wrote:

> When delivering guest IRQs, update MSR computaion

computation

> mode according to guest
> interrupt computation mode found in EPCR.
> 
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> arch/powerpc/kvm/booke.c |    8 +++++++-
> 1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index d15c4b5..93b48e0 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -287,6 +287,7 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu,
> 	bool crit;
> 	bool keep_irq = false;
> 	enum int_class int_class;
> +	ulong msr_cm = 0;
> 
> 	/* Truncate crit indicators in 32 bit mode */
> 	if (!(vcpu->arch.shared->msr & MSR_SF)) {
> @@ -299,6 +300,10 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu,
> 	/* ... and we're in supervisor mode */
> 	crit = crit && !(vcpu->arch.shared->msr & MSR_PR);
> 
> +#ifdef CONFIG_64BIT
> +	msr_cm = vcpu->arch.epcr & SPRN_EPCR_ICM ? MSR_CM : 0;
> +#endif

No need for the ifdef, no?. Just mask EPCR_ICM out in the 32-bit host case, then this check is always false on 32-bit hosts.

> +
> 	if (priority == BOOKE_IRQPRIO_EXTERNAL_LEVEL) {
> 		priority = BOOKE_IRQPRIO_EXTERNAL;
> 		keep_irq = true;
> @@ -381,7 +386,8 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu,
> 			set_guest_esr(vcpu, vcpu->arch.queued_esr);
> 		if (update_dear == true)
> 			set_guest_dear(vcpu, vcpu->arch.queued_dear);
> -		kvmppc_set_msr(vcpu, vcpu->arch.shared->msr & msr_mask);
> +		kvmppc_set_msr(vcpu, (vcpu->arch.shared->msr & msr_mask)
> +				| msr_cm);

Please split this computation out into its own variable and apply the masking regardless. Something like

ulong new_msr = vcpu->arch.shared->msr;
if (vcpu->arch.epcr & SPRN_EPCR_ICM)
    new_msr |= MSR_CM;
new_msr &= msr_mask;
kvmppc_set_msr(vcpu, new_msr);

Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Caraman Mihai Claudiu-B02008 - July 5, 2012, 9:28 a.m.
> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Wednesday, July 04, 2012 4:41 PM
> To: Caraman Mihai Claudiu-B02008
> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; linuxppc-
> dev@lists.ozlabs.org; qemu-ppc@nongnu.org
> Subject: Re: [Qemu-ppc] [RFC PATCH 04/17] KVM: PPC64: booke: Add guest
> computation mode for irq delivery
> 
> 
> On 25.06.2012, at 14:26, Mihai Caraman wrote:
>
> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> > ---
> > arch/powerpc/kvm/booke.c |    8 +++++++-
> > 1 files changed, 7 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> > index d15c4b5..93b48e0 100644
> > --- a/arch/powerpc/kvm/booke.c
> > +++ b/arch/powerpc/kvm/booke.c
> > @@ -287,6 +287,7 @@ static int kvmppc_booke_irqprio_deliver(struct
> kvm_vcpu *vcpu,
> > 	bool crit;
> > 	bool keep_irq = false;
> > 	enum int_class int_class;
> > +	ulong msr_cm = 0;
> >
> > 	/* Truncate crit indicators in 32 bit mode */
> > 	if (!(vcpu->arch.shared->msr & MSR_SF)) {
> > @@ -299,6 +300,10 @@ static int kvmppc_booke_irqprio_deliver(struct
> kvm_vcpu *vcpu,
> > 	/* ... and we're in supervisor mode */
> > 	crit = crit && !(vcpu->arch.shared->msr & MSR_PR);
> >
> > +#ifdef CONFIG_64BIT
> > +	msr_cm = vcpu->arch.epcr & SPRN_EPCR_ICM ? MSR_CM : 0;
> > +#endif
> 
> No need for the ifdef, no?. Just mask EPCR_ICM out in the 32-bit host
> case, then this check is always false on 32-bit hosts.

It will break e500v2. epcr field is declared only for CONFIG_KVM_BOOKE_HV,
we can limit to this instead of CONFIG_64BIT.

-Mike

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood - July 5, 2012, 11:51 p.m.
On 07/04/2012 08:40 AM, Alexander Graf wrote:
> On 25.06.2012, at 14:26, Mihai Caraman wrote:
>> @@ -381,7 +386,8 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu,
>> 			set_guest_esr(vcpu, vcpu->arch.queued_esr);
>> 		if (update_dear == true)
>> 			set_guest_dear(vcpu, vcpu->arch.queued_dear);
>> -		kvmppc_set_msr(vcpu, vcpu->arch.shared->msr & msr_mask);
>> +		kvmppc_set_msr(vcpu, (vcpu->arch.shared->msr & msr_mask)
>> +				| msr_cm);
> 
> Please split this computation out into its own variable and apply the masking regardless. Something like
> 
> ulong new_msr = vcpu->arch.shared->msr;
> if (vcpu->arch.epcr & SPRN_EPCR_ICM)
>     new_msr |= MSR_CM;
> new_msr &= msr_mask;
> kvmppc_set_msr(vcpu, new_msr);

This will fail to clear MSR[CM] in the odd but legal situation where you
have MSR[CM] set but EPCR[ICM] unset.

-Scott

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf - July 6, 2012, 7:03 a.m.
On 06.07.2012, at 01:51, Scott Wood <scottwood@freescale.com> wrote:

> On 07/04/2012 08:40 AM, Alexander Graf wrote:
>> On 25.06.2012, at 14:26, Mihai Caraman wrote:
>>> @@ -381,7 +386,8 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu,
>>>            set_guest_esr(vcpu, vcpu->arch.queued_esr);
>>>        if (update_dear == true)
>>>            set_guest_dear(vcpu, vcpu->arch.queued_dear);
>>> -        kvmppc_set_msr(vcpu, vcpu->arch.shared->msr & msr_mask);
>>> +        kvmppc_set_msr(vcpu, (vcpu->arch.shared->msr & msr_mask)
>>> +                | msr_cm);
>> 
>> Please split this computation out into its own variable and apply the masking regardless. Something like
>> 
>> ulong new_msr = vcpu->arch.shared->msr;
>> if (vcpu->arch.epcr & SPRN_EPCR_ICM)
>>    new_msr |= MSR_CM;
>> new_msr &= msr_mask;
>> kvmppc_set_msr(vcpu, new_msr);
> 
> This will fail to clear MSR[CM] in the odd but legal situation where you
> have MSR[CM] set but EPCR[ICM] unset.

Ah. Good point. Then leave the msr_mask logic as before and only stretch it out into its own variable.

Alex

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

Patch

diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index d15c4b5..93b48e0 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -287,6 +287,7 @@  static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu,
 	bool crit;
 	bool keep_irq = false;
 	enum int_class int_class;
+	ulong msr_cm = 0;
 
 	/* Truncate crit indicators in 32 bit mode */
 	if (!(vcpu->arch.shared->msr & MSR_SF)) {
@@ -299,6 +300,10 @@  static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu,
 	/* ... and we're in supervisor mode */
 	crit = crit && !(vcpu->arch.shared->msr & MSR_PR);
 
+#ifdef CONFIG_64BIT
+	msr_cm = vcpu->arch.epcr & SPRN_EPCR_ICM ? MSR_CM : 0;
+#endif
+
 	if (priority == BOOKE_IRQPRIO_EXTERNAL_LEVEL) {
 		priority = BOOKE_IRQPRIO_EXTERNAL;
 		keep_irq = true;
@@ -381,7 +386,8 @@  static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu,
 			set_guest_esr(vcpu, vcpu->arch.queued_esr);
 		if (update_dear == true)
 			set_guest_dear(vcpu, vcpu->arch.queued_dear);
-		kvmppc_set_msr(vcpu, vcpu->arch.shared->msr & msr_mask);
+		kvmppc_set_msr(vcpu, (vcpu->arch.shared->msr & msr_mask)
+				| msr_cm);
 
 		if (!keep_irq)
 			clear_bit(priority, &vcpu->arch.pending_exceptions);