linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] KVM: PPC: Book3S HV: Check for updated HDSISR on P9 HDSI exception
@ 2017-09-15  5:26 Michael Neuling
  2017-09-20  6:46 ` David Gibson
  2017-09-22  8:46 ` Paolo Bonzini
  0 siblings, 2 replies; 5+ messages in thread
From: Michael Neuling @ 2017-09-15  5:26 UTC (permalink / raw)
  To: kvm-ppc, kvm, linuxppc-dev; +Cc: david, paulus, benh, mikey

On POWER9 DD2.1 and below, sometimes on a Hypervisor Data Storage
Interrupt (HDSI) the HDSISR is not be updated at all.

To work around this we put a canary value into the HDSISR before
returning to a guest and then check for this canary when we take a
HDSI. If we find the canary on a HDSI, we know the hardware didn't
update the HDSISR. In this case we return to the guest to retake the
HDSI which should correctly update the HDSISR the second time HDSI
entry.

After talking to Paulus we've applied this workaround to all POWER9
CPUs. The workaround of returning to the guest shouldn't ever be
triggered on well behaving CPU. The extra instructions should have
negligible performance impact.

Signed-off-by: Michael Neuling <mikey@neuling.org>
---
 arch/powerpc/kvm/book3s_hv_rmhandlers.S | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 663a4a861e..70dca60569 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -1118,6 +1118,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 BEGIN_FTR_SECTION
 	mtspr	SPRN_PPR, r0
 END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
+
+/* Move canary into DSISR to check for later */
+BEGIN_FTR_SECTION
+	li	r0, 0x7fff
+	mtspr	SPRN_HDSISR, r0
+END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
+
 	ld	r0, VCPU_GPR(R0)(r4)
 	ld	r4, VCPU_GPR(R4)(r4)
 
@@ -1947,9 +1954,14 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_RADIX)
 kvmppc_hdsi:
 	ld	r3, VCPU_KVM(r9)
 	lbz	r0, KVM_RADIX(r3)
-	cmpwi	r0, 0
 	mfspr	r4, SPRN_HDAR
 	mfspr	r6, SPRN_HDSISR
+BEGIN_FTR_SECTION
+	/* Look for DSISR canary. If we find it, retry instruction */
+	cmpdi	r6, 0x7fff
+	beq	6f
+END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
+	cmpwi	r0, 0
 	bne	.Lradix_hdsi		/* on radix, just save DAR/DSISR/ASDR */
 	/* HPTE not found fault or protection fault? */
 	andis.	r0, r6, (DSISR_NOHPTE | DSISR_PROTFAULT)@h
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: PPC: Book3S HV: Check for updated HDSISR on P9 HDSI exception
  2017-09-15  5:26 [PATCH] KVM: PPC: Book3S HV: Check for updated HDSISR on P9 HDSI exception Michael Neuling
@ 2017-09-20  6:46 ` David Gibson
  2017-09-22  8:46 ` Paolo Bonzini
  1 sibling, 0 replies; 5+ messages in thread
From: David Gibson @ 2017-09-20  6:46 UTC (permalink / raw)
  To: Michael Neuling; +Cc: kvm-ppc, kvm, linuxppc-dev, paulus, benh, pbonzini

[-- Attachment #1: Type: text/plain, Size: 2685 bytes --]

On Fri, Sep 15, 2017 at 03:26:14PM +1000, Michael Neuling wrote:
> On POWER9 DD2.1 and below, sometimes on a Hypervisor Data Storage
> Interrupt (HDSI) the HDSISR is not be updated at all.
> 
> To work around this we put a canary value into the HDSISR before
> returning to a guest and then check for this canary when we take a
> HDSI. If we find the canary on a HDSI, we know the hardware didn't
> update the HDSISR. In this case we return to the guest to retake the
> HDSI which should correctly update the HDSISR the second time HDSI
> entry.
> 
> After talking to Paulus we've applied this workaround to all POWER9
> CPUs. The workaround of returning to the guest shouldn't ever be
> triggered on well behaving CPU. The extra instructions should have
> negligible performance impact.
> 
> Signed-off-by: Michael Neuling <mikey@neuling.org>

Paolo,

Any chance you can take this directly into your staging tree.  This is
a workaround for a potentially serious hardware bug, and we want to
get it into the downstream release ASAP.  However, Paulus is away for
a couple of weeks, so we can't count on him staging it in the kvm-ppc
tree soon.

> ---
>  arch/powerpc/kvm/book3s_hv_rmhandlers.S | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index 663a4a861e..70dca60569 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -1118,6 +1118,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>  BEGIN_FTR_SECTION
>  	mtspr	SPRN_PPR, r0
>  END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> +
> +/* Move canary into DSISR to check for later */
> +BEGIN_FTR_SECTION
> +	li	r0, 0x7fff
> +	mtspr	SPRN_HDSISR, r0
> +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
> +
>  	ld	r0, VCPU_GPR(R0)(r4)
>  	ld	r4, VCPU_GPR(R4)(r4)
>  
> @@ -1947,9 +1954,14 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_RADIX)
>  kvmppc_hdsi:
>  	ld	r3, VCPU_KVM(r9)
>  	lbz	r0, KVM_RADIX(r3)
> -	cmpwi	r0, 0
>  	mfspr	r4, SPRN_HDAR
>  	mfspr	r6, SPRN_HDSISR
> +BEGIN_FTR_SECTION
> +	/* Look for DSISR canary. If we find it, retry instruction */
> +	cmpdi	r6, 0x7fff
> +	beq	6f
> +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
> +	cmpwi	r0, 0
>  	bne	.Lradix_hdsi		/* on radix, just save DAR/DSISR/ASDR */
>  	/* HPTE not found fault or protection fault? */
>  	andis.	r0, r6, (DSISR_NOHPTE | DSISR_PROTFAULT)@h

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: PPC: Book3S HV: Check for updated HDSISR on P9 HDSI exception
  2017-09-15  5:26 [PATCH] KVM: PPC: Book3S HV: Check for updated HDSISR on P9 HDSI exception Michael Neuling
  2017-09-20  6:46 ` David Gibson
@ 2017-09-22  8:46 ` Paolo Bonzini
  2017-09-22 10:30   ` David Gibson
  1 sibling, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2017-09-22  8:46 UTC (permalink / raw)
  To: Michael Neuling, kvm-ppc, kvm, linuxppc-dev; +Cc: david, paulus, benh

On 15/09/2017 07:26, Michael Neuling wrote:
> On POWER9 DD2.1 and below, sometimes on a Hypervisor Data Storage
> Interrupt (HDSI) the HDSISR is not be updated at all.
> 
> To work around this we put a canary value into the HDSISR before
> returning to a guest and then check for this canary when we take a
> HDSI. If we find the canary on a HDSI, we know the hardware didn't
> update the HDSISR. In this case we return to the guest to retake the
> HDSI which should correctly update the HDSISR the second time HDSI
> entry.
> 
> After talking to Paulus we've applied this workaround to all POWER9
> CPUs. The workaround of returning to the guest shouldn't ever be
> triggered on well behaving CPU. The extra instructions should have
> negligible performance impact.
> 
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> ---
>  arch/powerpc/kvm/book3s_hv_rmhandlers.S | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index 663a4a861e..70dca60569 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -1118,6 +1118,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>  BEGIN_FTR_SECTION
>  	mtspr	SPRN_PPR, r0
>  END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> +
> +/* Move canary into DSISR to check for later */
> +BEGIN_FTR_SECTION
> +	li	r0, 0x7fff
> +	mtspr	SPRN_HDSISR, r0
> +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
> +
>  	ld	r0, VCPU_GPR(R0)(r4)
>  	ld	r4, VCPU_GPR(R4)(r4)
>  
> @@ -1947,9 +1954,14 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_RADIX)
>  kvmppc_hdsi:
>  	ld	r3, VCPU_KVM(r9)
>  	lbz	r0, KVM_RADIX(r3)
> -	cmpwi	r0, 0
>  	mfspr	r4, SPRN_HDAR
>  	mfspr	r6, SPRN_HDSISR
> +BEGIN_FTR_SECTION
> +	/* Look for DSISR canary. If we find it, retry instruction */
> +	cmpdi	r6, 0x7fff
> +	beq	6f
> +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
> +	cmpwi	r0, 0
>  	bne	.Lradix_hdsi		/* on radix, just save DAR/DSISR/ASDR */
>  	/* HPTE not found fault or protection fault? */
>  	andis.	r0, r6, (DSISR_NOHPTE | DSISR_PROTFAULT)@h
> 

Applied to kvm/master, thanks.

Paolo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: PPC: Book3S HV: Check for updated HDSISR on P9 HDSI exception
  2017-09-22  8:46 ` Paolo Bonzini
@ 2017-09-22 10:30   ` David Gibson
  2017-09-22 10:32     ` Paolo Bonzini
  0 siblings, 1 reply; 5+ messages in thread
From: David Gibson @ 2017-09-22 10:30 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Michael Neuling, kvm-ppc, kvm, linuxppc-dev, paulus, benh

[-- Attachment #1: Type: text/plain, Size: 2625 bytes --]

On Fri, Sep 22, 2017 at 10:46:36AM +0200, Paolo Bonzini wrote:
> On 15/09/2017 07:26, Michael Neuling wrote:
> > On POWER9 DD2.1 and below, sometimes on a Hypervisor Data Storage
> > Interrupt (HDSI) the HDSISR is not be updated at all.
> > 
> > To work around this we put a canary value into the HDSISR before
> > returning to a guest and then check for this canary when we take a
> > HDSI. If we find the canary on a HDSI, we know the hardware didn't
> > update the HDSISR. In this case we return to the guest to retake the
> > HDSI which should correctly update the HDSISR the second time HDSI
> > entry.
> > 
> > After talking to Paulus we've applied this workaround to all POWER9
> > CPUs. The workaround of returning to the guest shouldn't ever be
> > triggered on well behaving CPU. The extra instructions should have
> > negligible performance impact.
> > 
> > Signed-off-by: Michael Neuling <mikey@neuling.org>
> > ---
> >  arch/powerpc/kvm/book3s_hv_rmhandlers.S | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> > index 663a4a861e..70dca60569 100644
> > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> > @@ -1118,6 +1118,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> >  BEGIN_FTR_SECTION
> >  	mtspr	SPRN_PPR, r0
> >  END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> > +
> > +/* Move canary into DSISR to check for later */
> > +BEGIN_FTR_SECTION
> > +	li	r0, 0x7fff
> > +	mtspr	SPRN_HDSISR, r0
> > +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
> > +
> >  	ld	r0, VCPU_GPR(R0)(r4)
> >  	ld	r4, VCPU_GPR(R4)(r4)
> >  
> > @@ -1947,9 +1954,14 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_RADIX)
> >  kvmppc_hdsi:
> >  	ld	r3, VCPU_KVM(r9)
> >  	lbz	r0, KVM_RADIX(r3)
> > -	cmpwi	r0, 0
> >  	mfspr	r4, SPRN_HDAR
> >  	mfspr	r6, SPRN_HDSISR
> > +BEGIN_FTR_SECTION
> > +	/* Look for DSISR canary. If we find it, retry instruction */
> > +	cmpdi	r6, 0x7fff
> > +	beq	6f
> > +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
> > +	cmpwi	r0, 0
> >  	bne	.Lradix_hdsi		/* on radix, just save DAR/DSISR/ASDR */
> >  	/* HPTE not found fault or protection fault? */
> >  	andis.	r0, r6, (DSISR_NOHPTE | DSISR_PROTFAULT)@h
> > 
> 
> Applied to kvm/master, thanks.

I'm not seeing it; have you pushed out the tree?

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: PPC: Book3S HV: Check for updated HDSISR on P9 HDSI exception
  2017-09-22 10:30   ` David Gibson
@ 2017-09-22 10:32     ` Paolo Bonzini
  0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2017-09-22 10:32 UTC (permalink / raw)
  To: David Gibson; +Cc: Michael Neuling, kvm-ppc, kvm, linuxppc-dev, paulus, benh


[-- Attachment #1.1: Type: text/plain, Size: 2522 bytes --]

On 22/09/2017 12:30, David Gibson wrote:
> On Fri, Sep 22, 2017 at 10:46:36AM +0200, Paolo Bonzini wrote:
>> On 15/09/2017 07:26, Michael Neuling wrote:
>>> On POWER9 DD2.1 and below, sometimes on a Hypervisor Data Storage
>>> Interrupt (HDSI) the HDSISR is not be updated at all.
>>>
>>> To work around this we put a canary value into the HDSISR before
>>> returning to a guest and then check for this canary when we take a
>>> HDSI. If we find the canary on a HDSI, we know the hardware didn't
>>> update the HDSISR. In this case we return to the guest to retake the
>>> HDSI which should correctly update the HDSISR the second time HDSI
>>> entry.
>>>
>>> After talking to Paulus we've applied this workaround to all POWER9
>>> CPUs. The workaround of returning to the guest shouldn't ever be
>>> triggered on well behaving CPU. The extra instructions should have
>>> negligible performance impact.
>>>
>>> Signed-off-by: Michael Neuling <mikey@neuling.org>
>>> ---
>>>  arch/powerpc/kvm/book3s_hv_rmhandlers.S | 14 +++++++++++++-
>>>  1 file changed, 13 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
>>> index 663a4a861e..70dca60569 100644
>>> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
>>> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
>>> @@ -1118,6 +1118,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>>>  BEGIN_FTR_SECTION
>>>  	mtspr	SPRN_PPR, r0
>>>  END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>>> +
>>> +/* Move canary into DSISR to check for later */
>>> +BEGIN_FTR_SECTION
>>> +	li	r0, 0x7fff
>>> +	mtspr	SPRN_HDSISR, r0
>>> +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
>>> +
>>>  	ld	r0, VCPU_GPR(R0)(r4)
>>>  	ld	r4, VCPU_GPR(R4)(r4)
>>>  
>>> @@ -1947,9 +1954,14 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_RADIX)
>>>  kvmppc_hdsi:
>>>  	ld	r3, VCPU_KVM(r9)
>>>  	lbz	r0, KVM_RADIX(r3)
>>> -	cmpwi	r0, 0
>>>  	mfspr	r4, SPRN_HDAR
>>>  	mfspr	r6, SPRN_HDSISR
>>> +BEGIN_FTR_SECTION
>>> +	/* Look for DSISR canary. If we find it, retry instruction */
>>> +	cmpdi	r6, 0x7fff
>>> +	beq	6f
>>> +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
>>> +	cmpwi	r0, 0
>>>  	bne	.Lradix_hdsi		/* on radix, just save DAR/DSISR/ASDR */
>>>  	/* HPTE not found fault or protection fault? */
>>>  	andis.	r0, r6, (DSISR_NOHPTE | DSISR_PROTFAULT)@h
>>>
>>
>> Applied to kvm/master, thanks.
> 
> I'm not seeing it; have you pushed out the tree?

No, not yet (doing some tests).

Paolo



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-09-22 10:32 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-15  5:26 [PATCH] KVM: PPC: Book3S HV: Check for updated HDSISR on P9 HDSI exception Michael Neuling
2017-09-20  6:46 ` David Gibson
2017-09-22  8:46 ` Paolo Bonzini
2017-09-22 10:30   ` David Gibson
2017-09-22 10:32     ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).