* [PATCH] KVM: PPC: Book3S HV: Synthesize segment fault if SLB lookup fails
@ 2015-10-27 5:13 ` Paul Mackerras
0 siblings, 0 replies; 6+ messages in thread
From: Paul Mackerras @ 2015-10-27 5:13 UTC (permalink / raw)
To: kvm-ppc, kvm; +Cc: David Gibson
When handling a hypervisor data or instruction storage interrupt (HDSI
or HISI), we look up the SLB entry for the address being accessed in
order to translate the effective address to a virtual address which can
be looked up in the guest HPT. This lookup can occasionally fail due
to the guest replacing an SLB entry without invalidating the evicted
SLB entry. In this situation an ERAT (effective to real address
translation cache) entry can persist and be used by the hardware even
though there is no longer a corresponding SLB entry.
Previously we would just deliver a data or instruction storage interrupt
(DSI or ISI) to the guest in this case. However, this is not correct
and has been observed to cause guests to crash, typically with a
data storage protection interrupt on a store to the vmemmap area.
Instead, what we do now is to synthesize a data or instruction segment
interrupt. That should cause the guest to reload an appropriate entry
into the SLB and retry the faulting instruction. If it still faults,
we should find an appropriate SLB entry next time and be able to handle
the fault.
Signed-off-by: Paul Mackerras <paulus@samba.org>
---
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index b1dab8d..3c6badc 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -1749,7 +1749,8 @@ kvmppc_hdsi:
beq 3f
clrrdi r0, r4, 28
PPC_SLBFEE_DOT(R5, R0) /* if so, look up SLB */
- bne 1f /* if no SLB entry found */
+ li r0, BOOK3S_INTERRUPT_DATA_SEGMENT
+ bne 7f /* if no SLB entry found */
4: std r4, VCPU_FAULT_DAR(r9)
stw r6, VCPU_FAULT_DSISR(r9)
@@ -1768,14 +1769,15 @@ kvmppc_hdsi:
cmpdi r3, -2 /* MMIO emulation; need instr word */
beq 2f
- /* Synthesize a DSI for the guest */
+ /* Synthesize a DSI (or DSegI) for the guest */
ld r4, VCPU_FAULT_DAR(r9)
mr r6, r3
-1: mtspr SPRN_DAR, r4
+1: li r0, BOOK3S_INTERRUPT_DATA_STORAGE
mtspr SPRN_DSISR, r6
+7: mtspr SPRN_DAR, r4
mtspr SPRN_SRR0, r10
mtspr SPRN_SRR1, r11
- li r10, BOOK3S_INTERRUPT_DATA_STORAGE
+ mr r10, r0
bl kvmppc_msr_interrupt
fast_interrupt_c_return:
6: ld r7, VCPU_CTR(r9)
@@ -1823,7 +1825,8 @@ kvmppc_hisi:
beq 3f
clrrdi r0, r10, 28
PPC_SLBFEE_DOT(R5, R0) /* if so, look up SLB */
- bne 1f /* if no SLB entry found */
+ li r0, BOOK3S_INTERRUPT_INST_SEGMENT
+ bne 7f /* if no SLB entry found */
4:
/* Search the hash table. */
mr r3, r9 /* vcpu pointer */
@@ -1840,11 +1843,12 @@ kvmppc_hisi:
cmpdi r3, -1 /* handle in kernel mode */
beq guest_exit_cont
- /* Synthesize an ISI for the guest */
+ /* Synthesize an ISI (or ISegI) for the guest */
mr r11, r3
-1: mtspr SPRN_SRR0, r10
+1: li r0, BOOK3S_INTERRUPT_INST_STORAGE
+7: mtspr SPRN_SRR0, r10
mtspr SPRN_SRR1, r11
- li r10, BOOK3S_INTERRUPT_INST_STORAGE
+ mr r10, r0
bl kvmppc_msr_interrupt
b fast_interrupt_c_return
--
2.6.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] KVM: PPC: Book3S HV: Synthesize segment fault if SLB lookup fails
@ 2015-10-27 5:13 ` Paul Mackerras
0 siblings, 0 replies; 6+ messages in thread
From: Paul Mackerras @ 2015-10-27 5:13 UTC (permalink / raw)
To: kvm-ppc, kvm; +Cc: David Gibson
When handling a hypervisor data or instruction storage interrupt (HDSI
or HISI), we look up the SLB entry for the address being accessed in
order to translate the effective address to a virtual address which can
be looked up in the guest HPT. This lookup can occasionally fail due
to the guest replacing an SLB entry without invalidating the evicted
SLB entry. In this situation an ERAT (effective to real address
translation cache) entry can persist and be used by the hardware even
though there is no longer a corresponding SLB entry.
Previously we would just deliver a data or instruction storage interrupt
(DSI or ISI) to the guest in this case. However, this is not correct
and has been observed to cause guests to crash, typically with a
data storage protection interrupt on a store to the vmemmap area.
Instead, what we do now is to synthesize a data or instruction segment
interrupt. That should cause the guest to reload an appropriate entry
into the SLB and retry the faulting instruction. If it still faults,
we should find an appropriate SLB entry next time and be able to handle
the fault.
Signed-off-by: Paul Mackerras <paulus@samba.org>
---
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index b1dab8d..3c6badc 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -1749,7 +1749,8 @@ kvmppc_hdsi:
beq 3f
clrrdi r0, r4, 28
PPC_SLBFEE_DOT(R5, R0) /* if so, look up SLB */
- bne 1f /* if no SLB entry found */
+ li r0, BOOK3S_INTERRUPT_DATA_SEGMENT
+ bne 7f /* if no SLB entry found */
4: std r4, VCPU_FAULT_DAR(r9)
stw r6, VCPU_FAULT_DSISR(r9)
@@ -1768,14 +1769,15 @@ kvmppc_hdsi:
cmpdi r3, -2 /* MMIO emulation; need instr word */
beq 2f
- /* Synthesize a DSI for the guest */
+ /* Synthesize a DSI (or DSegI) for the guest */
ld r4, VCPU_FAULT_DAR(r9)
mr r6, r3
-1: mtspr SPRN_DAR, r4
+1: li r0, BOOK3S_INTERRUPT_DATA_STORAGE
mtspr SPRN_DSISR, r6
+7: mtspr SPRN_DAR, r4
mtspr SPRN_SRR0, r10
mtspr SPRN_SRR1, r11
- li r10, BOOK3S_INTERRUPT_DATA_STORAGE
+ mr r10, r0
bl kvmppc_msr_interrupt
fast_interrupt_c_return:
6: ld r7, VCPU_CTR(r9)
@@ -1823,7 +1825,8 @@ kvmppc_hisi:
beq 3f
clrrdi r0, r10, 28
PPC_SLBFEE_DOT(R5, R0) /* if so, look up SLB */
- bne 1f /* if no SLB entry found */
+ li r0, BOOK3S_INTERRUPT_INST_SEGMENT
+ bne 7f /* if no SLB entry found */
4:
/* Search the hash table. */
mr r3, r9 /* vcpu pointer */
@@ -1840,11 +1843,12 @@ kvmppc_hisi:
cmpdi r3, -1 /* handle in kernel mode */
beq guest_exit_cont
- /* Synthesize an ISI for the guest */
+ /* Synthesize an ISI (or ISegI) for the guest */
mr r11, r3
-1: mtspr SPRN_SRR0, r10
+1: li r0, BOOK3S_INTERRUPT_INST_STORAGE
+7: mtspr SPRN_SRR0, r10
mtspr SPRN_SRR1, r11
- li r10, BOOK3S_INTERRUPT_INST_STORAGE
+ mr r10, r0
bl kvmppc_msr_interrupt
b fast_interrupt_c_return
--
2.6.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: PPC: Book3S HV: Synthesize segment fault if SLB lookup fails
2015-10-27 5:13 ` Paul Mackerras
@ 2015-11-02 10:06 ` Thomas Huth
-1 siblings, 0 replies; 6+ messages in thread
From: Thomas Huth @ 2015-11-02 10:06 UTC (permalink / raw)
To: Paul Mackerras, kvm-ppc, kvm; +Cc: David Gibson
On 27/10/15 06:13, Paul Mackerras wrote:
> When handling a hypervisor data or instruction storage interrupt (HDSI
> or HISI), we look up the SLB entry for the address being accessed in
> order to translate the effective address to a virtual address which can
> be looked up in the guest HPT. This lookup can occasionally fail due
> to the guest replacing an SLB entry without invalidating the evicted
> SLB entry. In this situation an ERAT (effective to real address
> translation cache) entry can persist and be used by the hardware even
> though there is no longer a corresponding SLB entry.
>
> Previously we would just deliver a data or instruction storage interrupt
> (DSI or ISI) to the guest in this case. However, this is not correct
> and has been observed to cause guests to crash, typically with a
> data storage protection interrupt on a store to the vmemmap area.
>
> Instead, what we do now is to synthesize a data or instruction segment
> interrupt. That should cause the guest to reload an appropriate entry
> into the SLB and retry the faulting instruction. If it still faults,
> we should find an appropriate SLB entry next time and be able to handle
> the fault.
>
> Signed-off-by: Paul Mackerras <paulus@samba.org>
> ---
> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index b1dab8d..3c6badc 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -1749,7 +1749,8 @@ kvmppc_hdsi:
> beq 3f
> clrrdi r0, r4, 28
> PPC_SLBFEE_DOT(R5, R0) /* if so, look up SLB */
> - bne 1f /* if no SLB entry found */
> + li r0, BOOK3S_INTERRUPT_DATA_SEGMENT
> + bne 7f /* if no SLB entry found */
> 4: std r4, VCPU_FAULT_DAR(r9)
> stw r6, VCPU_FAULT_DSISR(r9)
>
> @@ -1768,14 +1769,15 @@ kvmppc_hdsi:
> cmpdi r3, -2 /* MMIO emulation; need instr word */
> beq 2f
>
> - /* Synthesize a DSI for the guest */
> + /* Synthesize a DSI (or DSegI) for the guest */
> ld r4, VCPU_FAULT_DAR(r9)
> mr r6, r3
> -1: mtspr SPRN_DAR, r4
> +1: li r0, BOOK3S_INTERRUPT_DATA_STORAGE
> mtspr SPRN_DSISR, r6
> +7: mtspr SPRN_DAR, r4
I first though "hey, you're not setting DSISR for the DsegI now" ... but
then I saw in the PowerISA that DSISR is "undefined" for the DSegI, so
this should be fine, I think.
> mtspr SPRN_SRR0, r10
> mtspr SPRN_SRR1, r11
> - li r10, BOOK3S_INTERRUPT_DATA_STORAGE
> + mr r10, r0
> bl kvmppc_msr_interrupt
> fast_interrupt_c_return:
> 6: ld r7, VCPU_CTR(r9)
> @@ -1823,7 +1825,8 @@ kvmppc_hisi:
> beq 3f
> clrrdi r0, r10, 28
> PPC_SLBFEE_DOT(R5, R0) /* if so, look up SLB */
> - bne 1f /* if no SLB entry found */
> + li r0, BOOK3S_INTERRUPT_INST_SEGMENT
> + bne 7f /* if no SLB entry found */
> 4:
> /* Search the hash table. */
> mr r3, r9 /* vcpu pointer */
> @@ -1840,11 +1843,12 @@ kvmppc_hisi:
> cmpdi r3, -1 /* handle in kernel mode */
> beq guest_exit_cont
>
> - /* Synthesize an ISI for the guest */
> + /* Synthesize an ISI (or ISegI) for the guest */
> mr r11, r3
> -1: mtspr SPRN_SRR0, r10
> +1: li r0, BOOK3S_INTERRUPT_INST_STORAGE
> +7: mtspr SPRN_SRR0, r10
> mtspr SPRN_SRR1, r11
> - li r10, BOOK3S_INTERRUPT_INST_STORAGE
> + mr r10, r0
> bl kvmppc_msr_interrupt
> b fast_interrupt_c_return
As far as I can see (but I'm really not an expert here), the patch seems
to be fine. And we have a test running with it for almost 6 days now and
did not see any further guest crashes, so it seems to me like this is
the right fix for this issue! So if you like, you can add my
"Reviewed-by" or "Tested-by" to this patch.
Thanks a lot for fixing this!
Thomas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: PPC: Book3S HV: Synthesize segment fault if SLB lookup fails
@ 2015-11-02 10:06 ` Thomas Huth
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Huth @ 2015-11-02 10:06 UTC (permalink / raw)
To: Paul Mackerras, kvm-ppc, kvm; +Cc: David Gibson
On 27/10/15 06:13, Paul Mackerras wrote:
> When handling a hypervisor data or instruction storage interrupt (HDSI
> or HISI), we look up the SLB entry for the address being accessed in
> order to translate the effective address to a virtual address which can
> be looked up in the guest HPT. This lookup can occasionally fail due
> to the guest replacing an SLB entry without invalidating the evicted
> SLB entry. In this situation an ERAT (effective to real address
> translation cache) entry can persist and be used by the hardware even
> though there is no longer a corresponding SLB entry.
>
> Previously we would just deliver a data or instruction storage interrupt
> (DSI or ISI) to the guest in this case. However, this is not correct
> and has been observed to cause guests to crash, typically with a
> data storage protection interrupt on a store to the vmemmap area.
>
> Instead, what we do now is to synthesize a data or instruction segment
> interrupt. That should cause the guest to reload an appropriate entry
> into the SLB and retry the faulting instruction. If it still faults,
> we should find an appropriate SLB entry next time and be able to handle
> the fault.
>
> Signed-off-by: Paul Mackerras <paulus@samba.org>
> ---
> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index b1dab8d..3c6badc 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -1749,7 +1749,8 @@ kvmppc_hdsi:
> beq 3f
> clrrdi r0, r4, 28
> PPC_SLBFEE_DOT(R5, R0) /* if so, look up SLB */
> - bne 1f /* if no SLB entry found */
> + li r0, BOOK3S_INTERRUPT_DATA_SEGMENT
> + bne 7f /* if no SLB entry found */
> 4: std r4, VCPU_FAULT_DAR(r9)
> stw r6, VCPU_FAULT_DSISR(r9)
>
> @@ -1768,14 +1769,15 @@ kvmppc_hdsi:
> cmpdi r3, -2 /* MMIO emulation; need instr word */
> beq 2f
>
> - /* Synthesize a DSI for the guest */
> + /* Synthesize a DSI (or DSegI) for the guest */
> ld r4, VCPU_FAULT_DAR(r9)
> mr r6, r3
> -1: mtspr SPRN_DAR, r4
> +1: li r0, BOOK3S_INTERRUPT_DATA_STORAGE
> mtspr SPRN_DSISR, r6
> +7: mtspr SPRN_DAR, r4
I first though "hey, you're not setting DSISR for the DsegI now" ... but
then I saw in the PowerISA that DSISR is "undefined" for the DSegI, so
this should be fine, I think.
> mtspr SPRN_SRR0, r10
> mtspr SPRN_SRR1, r11
> - li r10, BOOK3S_INTERRUPT_DATA_STORAGE
> + mr r10, r0
> bl kvmppc_msr_interrupt
> fast_interrupt_c_return:
> 6: ld r7, VCPU_CTR(r9)
> @@ -1823,7 +1825,8 @@ kvmppc_hisi:
> beq 3f
> clrrdi r0, r10, 28
> PPC_SLBFEE_DOT(R5, R0) /* if so, look up SLB */
> - bne 1f /* if no SLB entry found */
> + li r0, BOOK3S_INTERRUPT_INST_SEGMENT
> + bne 7f /* if no SLB entry found */
> 4:
> /* Search the hash table. */
> mr r3, r9 /* vcpu pointer */
> @@ -1840,11 +1843,12 @@ kvmppc_hisi:
> cmpdi r3, -1 /* handle in kernel mode */
> beq guest_exit_cont
>
> - /* Synthesize an ISI for the guest */
> + /* Synthesize an ISI (or ISegI) for the guest */
> mr r11, r3
> -1: mtspr SPRN_SRR0, r10
> +1: li r0, BOOK3S_INTERRUPT_INST_STORAGE
> +7: mtspr SPRN_SRR0, r10
> mtspr SPRN_SRR1, r11
> - li r10, BOOK3S_INTERRUPT_INST_STORAGE
> + mr r10, r0
> bl kvmppc_msr_interrupt
> b fast_interrupt_c_return
As far as I can see (but I'm really not an expert here), the patch seems
to be fine. And we have a test running with it for almost 6 days now and
did not see any further guest crashes, so it seems to me like this is
the right fix for this issue! So if you like, you can add my
"Reviewed-by" or "Tested-by" to this patch.
Thanks a lot for fixing this!
Thomas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: PPC: Book3S HV: Synthesize segment fault if SLB lookup fails
2015-10-27 5:13 ` Paul Mackerras
@ 2015-11-04 1:42 ` David Gibson
-1 siblings, 0 replies; 6+ messages in thread
From: David Gibson @ 2015-11-04 1:42 UTC (permalink / raw)
To: Paul Mackerras; +Cc: kvm-ppc, kvm
[-- Attachment #1: Type: text/plain, Size: 1543 bytes --]
On Tue, Oct 27, 2015 at 04:13:56PM +1100, Paul Mackerras wrote:
> When handling a hypervisor data or instruction storage interrupt (HDSI
> or HISI), we look up the SLB entry for the address being accessed in
> order to translate the effective address to a virtual address which can
> be looked up in the guest HPT. This lookup can occasionally fail due
> to the guest replacing an SLB entry without invalidating the evicted
> SLB entry. In this situation an ERAT (effective to real address
> translation cache) entry can persist and be used by the hardware even
> though there is no longer a corresponding SLB entry.
>
> Previously we would just deliver a data or instruction storage interrupt
> (DSI or ISI) to the guest in this case. However, this is not correct
> and has been observed to cause guests to crash, typically with a
> data storage protection interrupt on a store to the vmemmap area.
>
> Instead, what we do now is to synthesize a data or instruction segment
> interrupt. That should cause the guest to reload an appropriate entry
> into the SLB and retry the faulting instruction. If it still faults,
> we should find an appropriate SLB entry next time and be able to handle
> the fault.
>
> Signed-off-by: Paul Mackerras <paulus@samba.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--
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: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: PPC: Book3S HV: Synthesize segment fault if SLB lookup fails
@ 2015-11-04 1:42 ` David Gibson
0 siblings, 0 replies; 6+ messages in thread
From: David Gibson @ 2015-11-04 1:42 UTC (permalink / raw)
To: Paul Mackerras; +Cc: kvm-ppc, kvm
[-- Attachment #1: Type: text/plain, Size: 1543 bytes --]
On Tue, Oct 27, 2015 at 04:13:56PM +1100, Paul Mackerras wrote:
> When handling a hypervisor data or instruction storage interrupt (HDSI
> or HISI), we look up the SLB entry for the address being accessed in
> order to translate the effective address to a virtual address which can
> be looked up in the guest HPT. This lookup can occasionally fail due
> to the guest replacing an SLB entry without invalidating the evicted
> SLB entry. In this situation an ERAT (effective to real address
> translation cache) entry can persist and be used by the hardware even
> though there is no longer a corresponding SLB entry.
>
> Previously we would just deliver a data or instruction storage interrupt
> (DSI or ISI) to the guest in this case. However, this is not correct
> and has been observed to cause guests to crash, typically with a
> data storage protection interrupt on a store to the vmemmap area.
>
> Instead, what we do now is to synthesize a data or instruction segment
> interrupt. That should cause the guest to reload an appropriate entry
> into the SLB and retry the faulting instruction. If it still faults,
> we should find an appropriate SLB entry next time and be able to handle
> the fault.
>
> Signed-off-by: Paul Mackerras <paulus@samba.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--
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: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-11-04 2:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-27 5:13 [PATCH] KVM: PPC: Book3S HV: Synthesize segment fault if SLB lookup fails Paul Mackerras
2015-10-27 5:13 ` Paul Mackerras
2015-11-02 10:06 ` Thomas Huth
2015-11-02 10:06 ` Thomas Huth
2015-11-04 1:42 ` David Gibson
2015-11-04 1:42 ` David Gibson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.