(no subject)
Date: Thu, 20 Oct 2016 17:38:24 +1100
Subject: [PATCH 0/3] ppc: system reset interrupt fixes and new hcall IPI
Hi,
We are implementing this new unmaskable IPI hcall for crash dumping and debugging. I had some issues with QEMU delivering system reset interrupt to guests, which was caused by the HV bit being set. After changing that, the interrupt was being handled okay. Should there be a more general check to ensure the HV bit is not set in the guest?
I implemented Linux support for the new hcall in crashdump code, and it works.
Thanks,
Nick
Nicholas Piggin (3):
ppc: fix MSR_ME handling for system reset interrupt
ppc: allow system reset interrupt to be delivered to guests ppc/spapr: implement H_SIGNAL_SYS_RESET
hw/ppc/spapr_hcall.c | 42 ++++++++++++++++++++++++++++++++++++++++++ include/hw/ppc/spapr.h | 8 +++++++- target-ppc/excp_helper.c | 10 +++++++--- 3 files changed, 56 insertions(+), 4 deletions(-)
Signature · 1 line
2.9.3
PATCH 1/3 ppc: fix MSR_ME handling for system reset interrupt
Power ISA specifies ME bit handling for system reset interrupt:
if the interrupt occurred while the thread was in power-saving mode, set to 1; otherwise not altered
--- target-ppc/excp_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
target-ppc/excp_helper.c +2 -2
@@ -385,11 +385,11 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) srr1 =SPR_BOOKE_CSRR1 ;break ;case POWERPC_EXCP_RESET :/* System reset exception */ +/* A power-saving exception sets ME, otherwise it is unchanged */ if (msr_pow) {/* indicate that we resumed from power save mode */ msr |=0x10000 ; - }else { - new_msr &= ~((target_ulong )1 <<MSR_ME ); + new_msr |= ((target_ulong )1 <<MSR_ME ); } new_msr |= (target_ulong )MSR_HVB ;
Signature · 1 line
2.9.3
Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/3] ppc: fix MSR_ME handling for system reset interrupt
Quoted 7 lines · On Thu, 20 Oct 2016 17:59:10 +1100 Nicholas Piggin <[email protected]> wrote:
Power ISA specifies ME bit handling for system reset interrupt:
if the interrupt occurred while the thread was in power-saving mode, set to 1; otherwise not altered
Signed-off-by Nicholas Piggin <[email protected]>---
As described in "6.5 Interrupt Definitions, Figure 64" of Power ISA 3.0.
Quoted 21 lines
target-ppc/excp_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)target-ppc/excp_helper.c +2
-2@@ -385,11 +385,11 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) srr1 =SPR_BOOKE_CSRR1 ;break ;case POWERPC_EXCP_RESET :/* System reset exception */ +/* A power-saving exception sets ME, otherwise it is unchanged */ if (msr_pow) {/* indicate that we resumed from power save mode */ msr |=0x10000 ; - }else { - new_msr &= ~((target_ulong )1 <<MSR_ME ); + new_msr |= ((target_ulong )1 <<MSR_ME ); } new_msr |= (target_ulong )MSR_HVB ;
Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/3] ppc: fix MSR_ME handling for system reset interrupt
In reply to Nicholas PigginQuoted 6 lines · Nicholas Piggin
Power ISA specifies ME bit handling for system reset interrupt:
if the interrupt occurred while the thread was in power-saving mode, set to 1; otherwise not altered
Signed-off-by Nicholas Piggin <[email protected]>
Quoted 23 lines
--- target-ppc/excp_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)target-ppc/excp_helper.c +2
-2@@ -385,11 +385,11 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) srr1 =SPR_BOOKE_CSRR1 ;break ;case POWERPC_EXCP_RESET :/* System reset exception */ +/* A power-saving exception sets ME, otherwise it is unchanged */ if (msr_pow) {/* indicate that we resumed from power save mode */ msr |=0x10000 ; - }else { - new_msr &= ~((target_ulong )1 <<MSR_ME ); + new_msr |= ((target_ulong )1 <<MSR_ME ); } new_msr |= (target_ulong )MSR_HVB ;
Re: [Qemu-devel] [PATCH 1/3] ppc: fix MSR_ME handling for system reset interrupt
In reply to Nicholas PigginQuoted 6 lines · Nicholas Piggin
Power ISA specifies ME bit handling for system reset interrupt:
if the interrupt occurred while the thread was in power-saving mode, set to 1; otherwise not altered
Signed-off-by Nicholas Piggin <[email protected]>
I've applied this one to ppc-for-2.8. Looks like the remaining patches in the series need a little more polish.
Quoted 22 lines
--- target-ppc/excp_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)target-ppc/excp_helper.c +2
-2@@ -385,11 +385,11 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) srr1 =SPR_BOOKE_CSRR1 ;break ;case POWERPC_EXCP_RESET :/* System reset exception */ +/* A power-saving exception sets ME, otherwise it is unchanged */ if (msr_pow) {/* indicate that we resumed from power save mode */ msr |=0x10000 ; - }else { - new_msr &= ~((target_ulong )1 <<MSR_ME ); + new_msr |= ((target_ulong )1 <<MSR_ME ); } new_msr |= (target_ulong )MSR_HVB ;
Signature · 4 lines
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
Re: [Qemu-devel] [PATCH 1/3] ppc: fix MSR_ME handling for system reset interrupt
Quoted 10 lines · On Fri, 21 Oct 2016 12:09:15 +1100 David Gibson <[email protected]>…
Quoted 6 lines · Nicholas Piggin
Power ISA specifies ME bit handling for system reset interrupt:
if the interrupt occurred while the thread was in power-saving mode, set to 1; otherwise not altered
Signed-off-by Nicholas Piggin <[email protected]>I've applied this one to ppc-for-2.8. Looks like the remaining patches in the series need a little more polish.
Yes I'll respin the other two based on feedback.
Thanks,
Nick
PATCH 2/3 ppc: allow system reset interrupt to be delivered to guests
In reply to Nicholas Piggin--- target-ppc/excp_helper.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
target-ppc/excp_helper.c +6 -2
@@ -390,9 +390,13 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)/* indicate that we resumed from power save mode */ msr |=0x10000 ; new_msr |= ((target_ulong )1 <<MSR_ME ); + new_msr |= (target_ulong )MSR_HVB ; + }else { +/* The ISA specifies the HV bit is set when the hardware interrupt +* is raised, however when hypervisors deliver the exception to +* guests, it should not be set. +*/ } - - new_msr |= (target_ulong )MSR_HVB ; ail =0 ;break ;case POWERPC_EXCP_DSEG :/* Data segment exception */
Signature · 1 line
2.9.3
Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/3] ppc: allow system reset interrupt to be delivered to guests
Quoted 16 lines · Nicholas Piggin
Signed-off-by Nicholas Piggin <[email protected]>--- target-ppc/excp_helper.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)target-ppc/excp_helper.c +2
-0@@ -390,9 +390,13 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)/* indicate that we resumed from power save mode */ msr |=0x10000 ; new_msr |= ((target_ulong )1 <<MSR_ME ); + new_msr |= (target_ulong )MSR_HVB ; + }else {
target-ppc/excp_helper.c static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) · line 402
Quoted 10 lines
target-ppc/excp_helper.c +4
-2+/* The ISA specifies the HV bit is set when the hardware interrupt +* is raised, however when hypervisors deliver the exception to +* guests, it should not be set. +*/ } - - new_msr |= (target_ulong )MSR_HVB ; ail =0 ;break ;case POWERPC_EXCP_DSEG :/* Data segment exception */
should not that be cleared later on in powerpc_excp() by :
env->msr = new_msr & env->msr_mask;
? but the routine is rather long so I might be missing a branch.
C.
Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/3] ppc: allow system reset interrupt to be delivered to guests
Quoted 30 lines · On Thu, 20 Oct 2016 15:08:07 +0200 Cédric Le Goater <[email protected]> wrote:
Quoted 16 lines · Nicholas Piggin
Signed-off-by Nicholas Piggin <[email protected]>--- target-ppc/excp_helper.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)target-ppc/excp_helper.c +2
-0@@ -390,9 +390,13 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)/* indicate that we resumed from power save mode */ msr |=0x10000 ; new_msr |= ((target_ulong )1 <<MSR_ME ); + new_msr |= (target_ulong )MSR_HVB ; + }else {target-ppc/excp_helper.c static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) · line 402
Quoted 10 lines
target-ppc/excp_helper.c +4
-2+/* The ISA specifies the HV bit is set when the hardware interrupt +* is raised, however when hypervisors deliver the exception to +* guests, it should not be set. +*/ } - - new_msr |= (target_ulong )MSR_HVB ; ail =0 ;break ;case POWERPC_EXCP_DSEG :/* Data segment exception */ should not that be cleared later on in
powerpc_excp()by :
Quoted 3 lines
env->msr = new_msr & env->msr_mask;? but the routine is rather long so I might be missing a branch.
No you're right, so it can't leak into the guest, phew!
The problem I get is the interrupt code doing some things differently depending on on the HV bit. For example what I noticed is the guest losing its LE bit upon entry.
Perhaps a cleaner way is for the system reset case to set new_msr according to the ISA, and then apply the msr_mask (or at least mask out HV) before calculating the exception model? Any preference?
Thanks,
Nick
Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/3] ppc: allow system reset interrupt to be delivered to guests
Quoted 39 lines · Nicholas Piggin
Quoted 30 lines · On Thu, 20 Oct 2016 15:08:07 +0200 Cédric Le Goater <[email protected]> wrote:
Quoted 16 lines · Nicholas Piggin
Signed-off-by Nicholas Piggin <[email protected]>--- target-ppc/excp_helper.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)target-ppc/excp_helper.c +2
-0@@ -390,9 +390,13 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)/* indicate that we resumed from power save mode */ msr |=0x10000 ; new_msr |= ((target_ulong )1 <<MSR_ME ); + new_msr |= (target_ulong )MSR_HVB ; + }else {target-ppc/excp_helper.c static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) · line 402
Quoted 10 lines
target-ppc/excp_helper.c +4
-2+/* The ISA specifies the HV bit is set when the hardware interrupt +* is raised, however when hypervisors deliver the exception to +* guests, it should not be set. +*/ } - - new_msr |= (target_ulong )MSR_HVB ; ail =0 ;break ;case POWERPC_EXCP_DSEG :/* Data segment exception */ should not that be cleared later on in
powerpc_excp()by :Quoted 3 lines
env->msr = new_msr & env->msr_mask;? but the routine is rather long so I might be missing a branch.
No you're right, so it can't leak into the guest, phew!
Quoted 7 lines
The problem I get is the interrupt code doing some things differently depending on on the HV bit. For example what I noticed is the guest losing its LE bit upon entry.
Perhaps a cleaner way is for the system reset case to set
new_msraccording to the ISA, and then apply themsr_mask(or at least mask out HV) before calculating the exception model? Any preference?
I think the proposed revision makes sense.
Signature · 4 lines
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
PATCH v2 1/1 ppc: allow certain HV interrupts to be delivered to guests
Quoted 44 lines · On Fri, 21 Oct 2016 12:09:54 +1100 David Gibson <[email protected]>…
Quoted 43 lines · Nicholas Piggin
Quoted 30 lines · On Thu, 20 Oct 2016 15:08:07 +0200 Cédric Le Goater <[email protected]> wrote:
Quoted 16 lines · Nicholas Piggin
Signed-off-by Nicholas Piggin <[email protected]>--- target-ppc/excp_helper.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)target-ppc/excp_helper.c +2
-0@@ -390,9 +390,13 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)/* indicate that we resumed from power save mode */ msr |=0x10000 ; new_msr |= ((target_ulong )1 <<MSR_ME ); + new_msr |= (target_ulong )MSR_HVB ; + }else {target-ppc/excp_helper.c static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) · line 402
Quoted 10 lines
target-ppc/excp_helper.c +4
-2+/* The ISA specifies the HV bit is set when the hardware interrupt +* is raised, however when hypervisors deliver the exception to +* guests, it should not be set. +*/ } - - new_msr |= (target_ulong )MSR_HVB ; ail =0 ;break ;case POWERPC_EXCP_DSEG :/* Data segment exception */ should not that be cleared later on in
powerpc_excp()by :Quoted 3 lines
env->msr = new_msr & env->msr_mask;? but the routine is rather long so I might be missing a branch.
No you're right, so it can't leak into the guest, phew!
The problem I get is the interrupt code doing some things differently depending on on the HV bit. For example what I noticed is the guest losing its LE bit upon entry.
Quoted 6 lines
Quoted 3 lines
Perhaps a cleaner way is for the system reset case to set
new_msraccording to the ISA, and then apply themsr_mask(or at least mask out HV) before calculating the exception model? Any preference?I think the proposed revision makes sense.
What do you think of this version? This fixes up machine check guest delivery as well. I'm sending this ahead of the new hcall patch, because it's a bugfix for existing code. I'll get around to the hcall again next week.
Thanks,
Nick
ppc hypervisors have delivered system reset and machine check exception interrupts to guests in some situations (e.g., see FWNMI feature of LoPAPR, or NMI injection in QEMU).
These exceptions are architected to set the HV bit in hardware, however when injected into a guest, the HV bit should be cleared. Current code masks off the HV bit before setting the new MSR, however this happens after the interrupt delivery model has calculated delivery mode for the exception. This can result in the guest's MSR LE bit being lost.
Provide a new flag for HV exceptions to allow delivery to guests. The exception model masks out the HV bit.
Also add another sanity check to ensure other such exceptions don't try to set HV in guest without setting guest_hv_excp
--- target-ppc/excp_helper.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-)
target-ppc/excp_helper.c +22 -3
@@ -77,7 +77,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)CPUState *cs =CPU (cpu);CPUPPCState *env = &cpu->env;target_ulong msr, new_msr, vector; -int srr0, srr1, asrr0, asrr1, lev, ail; +int srr0, srr1, asrr0, asrr1, lev, ail, guest_hv_excp;bool lpes0; qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx
@@ -149,6 +149,10 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)* * AIL is initialized here but can be cleared by * selected exceptions +* +* guest_hv_excp is a provision to raise HV architected +* exceptions in guests by delivering them with HV bit +* clear. System reset and machine check use this. */ #if defined(TARGET_PPC64 ) if (excp_model == POWERPC_EXCP_POWER7 ||
@@ -165,6 +169,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) lpes0 = true; ail =0 ; } + guest_hv_excp =0 ;/* Hypervisor emulation assistance interrupt only exists on server * arch 2.05 server or later. We also don't want to generate it if
@@ -214,6 +219,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) cs->interrupt_request |=CPU_INTERRUPT_EXITTB ; } new_msr |= (target_ulong )MSR_HVB ; + guest_hv_excp =1 ; ail =0 ;/* machine check exceptions don't have ME set */
@@ -391,8 +397,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) msr |=0x10000 ; new_msr |= ((target_ulong )1 <<MSR_ME ); } - new_msr |= (target_ulong )MSR_HVB ; + guest_hv_excp =1 ; ail =0 ;break ;case POWERPC_EXCP_DSEG :/* Data segment exception */
@@ -610,10 +616,23 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)/* Sanity check */ if (!(env->msr_mask &MSR_HVB ) && (srr0 ==SPR_HSRR0 )) { -cpu_abort (cs,"Trying to deliver HV exception %d with " +cpu_abort (cs,"Trying to deliver HV exception (HSRR) %d with " "no HV support\n" , excp); } +/* The ISA specifies the HV bit is set when the hardware interrupt +* is raised, however in some cases hypervisors deliver the exception +* to guests. HV should be cleared in that case. +*/ +if (!(env->msr_mask &MSR_HVB ) && (new_msr &MSR_HVB )) { +if (guest_hv_excp) { + new_msr &= ~MSR_HVB ; + }else { +cpu_abort (cs,"Trying to deliver HV exception (MSR) %d with " +"no HV support\n" , excp); + } + } +/* If any alternate SRR register are defined, duplicate saved values */ if (asrr0 !=-1 ) { env->spr[asrr0] = env->spr[srr0];
Signature · 1 line
2.9.3
Re: [Qemu-devel] [PATCH v2] ppc: allow certain HV interrupts to be delivered to guests
Quoted 73 lines · Nicholas Piggin
Quoted 44 lines · On Fri, 21 Oct 2016 12:09:54 +1100 David Gibson <[email protected]>…
Quoted 43 lines · Nicholas Piggin
Quoted 30 lines · On Thu, 20 Oct 2016 15:08:07 +0200 Cédric Le Goater <[email protected]> wrote:
Quoted 16 lines · Nicholas Piggin
Signed-off-by Nicholas Piggin <[email protected]>--- target-ppc/excp_helper.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)target-ppc/excp_helper.c +2
-0@@ -390,9 +390,13 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)/* indicate that we resumed from power save mode */ msr |=0x10000 ; new_msr |= ((target_ulong )1 <<MSR_ME ); + new_msr |= (target_ulong )MSR_HVB ; + }else {target-ppc/excp_helper.c static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) · line 402
Quoted 10 lines
target-ppc/excp_helper.c +4
-2+/* The ISA specifies the HV bit is set when the hardware interrupt +* is raised, however when hypervisors deliver the exception to +* guests, it should not be set. +*/ } - - new_msr |= (target_ulong )MSR_HVB ; ail =0 ;break ;case POWERPC_EXCP_DSEG :/* Data segment exception */ should not that be cleared later on in
powerpc_excp()by :Quoted 3 lines
env->msr = new_msr & env->msr_mask;? but the routine is rather long so I might be missing a branch.
No you're right, so it can't leak into the guest, phew!
The problem I get is the interrupt code doing some things differently depending on on the HV bit. For example what I noticed is the guest losing its LE bit upon entry.
Quoted 6 lines
Quoted 3 lines
Perhaps a cleaner way is for the system reset case to set
new_msraccording to the ISA, and then apply themsr_mask(or at least mask out HV) before calculating the exception model? Any preference?I think the proposed revision makes sense.
What do you think of this version? This fixes up machine check guest delivery as well. I'm sending this ahead of the new hcall patch, because it's a bugfix for existing code. I'll get around to the hcall again next week.
Thanks,
Nickppc hypervisors have delivered system reset and machine check exception interrupts to guests in some situations (e.g., see FWNMI feature of LoPAPR, or NMI injection in QEMU).
These exceptions are architected to set the HV bit in hardware, however when injected into a guest, the HV bit should be cleared. Current code masks off the HV bit before setting the new MSR, however this happens after the interrupt delivery model has calculated delivery mode for the exception. This can result in the guest's MSR LE bit being lost.
Quoted 7 lines
Provide a new flag for HV exceptions to allow delivery to guests. The exception model masks out the HV bit.
Also add another sanity check to ensure other such exceptions don't try to set HV in guest without setting
guest_hv_excpSigned-off-by Nicholas Piggin <[email protected]>
Looks good to me and I gave it a try on the pnv platform which runs in HV mode.
Thanks,
C.
Quoted 80 lines
--- target-ppc/excp_helper.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-)target-ppc/excp_helper.c +22
-3@@ -77,7 +77,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)CPUState *cs =CPU (cpu);CPUPPCState *env = &cpu->env;target_ulong msr, new_msr, vector; -int srr0, srr1, asrr0, asrr1, lev, ail; +int srr0, srr1, asrr0, asrr1, lev, ail, guest_hv_excp;bool lpes0; qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx@@ -149,6 +149,10 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)* * AIL is initialized here but can be cleared by * selected exceptions +* +* guest_hv_excp is a provision to raise HV architected +* exceptions in guests by delivering them with HV bit +* clear. System reset and machine check use this. */ #if defined(TARGET_PPC64 ) if (excp_model == POWERPC_EXCP_POWER7 ||@@ -165,6 +169,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) lpes0 = true; ail =0 ; } + guest_hv_excp =0 ;/* Hypervisor emulation assistance interrupt only exists on server * arch 2.05 server or later. We also don't want to generate it if @@ -214,6 +219,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) cs->interrupt_request |=CPU_INTERRUPT_EXITTB ; } new_msr |= (target_ulong )MSR_HVB ; + guest_hv_excp =1 ; ail =0 ;/* machine check exceptions don't have ME set */ @@ -391,8 +397,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) msr |=0x10000 ; new_msr |= ((target_ulong )1 <<MSR_ME ); } - new_msr |= (target_ulong )MSR_HVB ; + guest_hv_excp =1 ; ail =0 ;break ;case POWERPC_EXCP_DSEG :/* Data segment exception */ @@ -610,10 +616,23 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)/* Sanity check */ if (!(env->msr_mask &MSR_HVB ) && (srr0 ==SPR_HSRR0 )) { -cpu_abort (cs,"Trying to deliver HV exception %d with " +cpu_abort (cs,"Trying to deliver HV exception (HSRR) %d with " "no HV support\n" , excp); } +/* The ISA specifies the HV bit is set when the hardware interrupt +* is raised, however in some cases hypervisors deliver the exception +* to guests. HV should be cleared in that case. +*/ +if (!(env->msr_mask &MSR_HVB ) && (new_msr &MSR_HVB )) { +if (guest_hv_excp) { + new_msr &= ~MSR_HVB ; + }else { +cpu_abort (cs,"Trying to deliver HV exception (MSR) %d with " +"no HV support\n" , excp); + } + } +/* If any alternate SRR register are defined, duplicate saved values */ if (asrr0 !=-1 ) { env->spr[asrr0] = env->spr[srr0];
Re: [Qemu-devel] [PATCH v2] ppc: allow certain HV interrupts to be delivered to guests
In reply to Nicholas PigginQuoted 89 lines · Nicholas Piggin
Quoted 44 lines · On Fri, 21 Oct 2016 12:09:54 +1100 David Gibson <[email protected]>…
Quoted 43 lines · Nicholas Piggin
Quoted 30 lines · On Thu, 20 Oct 2016 15:08:07 +0200 Cédric Le Goater <[email protected]> wrote:
Quoted 16 lines · Nicholas Piggin
Signed-off-by Nicholas Piggin <[email protected]>--- target-ppc/excp_helper.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)target-ppc/excp_helper.c +2
-0@@ -390,9 +390,13 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)/* indicate that we resumed from power save mode */ msr |=0x10000 ; new_msr |= ((target_ulong )1 <<MSR_ME ); + new_msr |= (target_ulong )MSR_HVB ; + }else {target-ppc/excp_helper.c static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) · line 402
Quoted 10 lines
target-ppc/excp_helper.c +4
-2+/* The ISA specifies the HV bit is set when the hardware interrupt +* is raised, however when hypervisors deliver the exception to +* guests, it should not be set. +*/ } - - new_msr |= (target_ulong )MSR_HVB ; ail =0 ;break ;case POWERPC_EXCP_DSEG :/* Data segment exception */ should not that be cleared later on in
powerpc_excp()by :Quoted 3 lines
env->msr = new_msr & env->msr_mask;? but the routine is rather long so I might be missing a branch.
No you're right, so it can't leak into the guest, phew!
The problem I get is the interrupt code doing some things differently depending on on the HV bit. For example what I noticed is the guest losing its LE bit upon entry.
Quoted 6 lines
Quoted 3 lines
Perhaps a cleaner way is for the system reset case to set
new_msraccording to the ISA, and then apply themsr_mask(or at least mask out HV) before calculating the exception model? Any preference?I think the proposed revision makes sense.
What do you think of this version? This fixes up machine check guest delivery as well. I'm sending this ahead of the new hcall patch, because it's a bugfix for existing code. I'll get around to the hcall again next week.
Thanks,
Nickppc hypervisors have delivered system reset and machine check exception interrupts to guests in some situations (e.g., see FWNMI feature of LoPAPR, or NMI injection in QEMU).
These exceptions are architected to set the HV bit in hardware, however when injected into a guest, the HV bit should be cleared. Current code masks off the HV bit before setting the new MSR, however this happens after the interrupt delivery model has calculated delivery mode for the exception. This can result in the guest's MSR LE bit being lost.
Provide a new flag for HV exceptions to allow delivery to guests. The exception model masks out the HV bit.
Also add another sanity check to ensure other such exceptions don't try to set HV in guest without setting
guest_hv_excpSigned-off-by Nicholas Piggin <[email protected]>--- target-ppc/excp_helper.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-)target-ppc/excp_helper.c +0
-0
target-ppc/excp_helper.c static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) · line 80
Quoted 5 lines
target-ppc/excp_helper.c +1
-1@@ -77,7 +77,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)CPUState *cs =CPU (cpu);CPUPPCState *env = &cpu->env;target_ulong msr, new_msr, vector; -int srr0, srr1, asrr0, asrr1, lev, ail; +int srr0, srr1, asrr0, asrr1, lev, ail, guest_hv_excp;
So, to clarify my understanding of this.
The guest_hv_excp flag indicates that this is a normally-HV exception which *could* be delivered to a guest with HV clear, *not* that we're actually doing so in this instance. Yes?
Quoted 65 lines
target-ppc/excp_helper.c +21
-2bool lpes0; qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx@@ -149,6 +149,10 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)* * AIL is initialized here but can be cleared by * selected exceptions +* +* guest_hv_excp is a provision to raise HV architected +* exceptions in guests by delivering them with HV bit +* clear. System reset and machine check use this. */ #if defined(TARGET_PPC64 ) if (excp_model == POWERPC_EXCP_POWER7 ||@@ -165,6 +169,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) lpes0 = true; ail =0 ; } + guest_hv_excp =0 ;/* Hypervisor emulation assistance interrupt only exists on server * arch 2.05 server or later. We also don't want to generate it if @@ -214,6 +219,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) cs->interrupt_request |=CPU_INTERRUPT_EXITTB ; } new_msr |= (target_ulong )MSR_HVB ; + guest_hv_excp =1 ; ail =0 ;/* machine check exceptions don't have ME set */ @@ -391,8 +397,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) msr |=0x10000 ; new_msr |= ((target_ulong )1 <<MSR_ME ); } - new_msr |= (target_ulong )MSR_HVB ; + guest_hv_excp =1 ; ail =0 ;break ;case POWERPC_EXCP_DSEG :/* Data segment exception */ @@ -610,10 +616,23 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)/* Sanity check */ if (!(env->msr_mask &MSR_HVB ) && (srr0 ==SPR_HSRR0 )) { -cpu_abort (cs,"Trying to deliver HV exception %d with " +cpu_abort (cs,"Trying to deliver HV exception (HSRR) %d with " "no HV support\n" , excp); } +/* The ISA specifies the HV bit is set when the hardware interrupt +* is raised, however in some cases hypervisors deliver the exception +* to guests. HV should be cleared in that case. +*/ +if (!(env->msr_mask &MSR_HVB ) && (new_msr &MSR_HVB )) { +if (guest_hv_excp) { + new_msr &= ~MSR_HVB ; + }else { +cpu_abort (cs,"Trying to deliver HV exception (MSR) %d with " +"no HV support\n" , excp); + } + } +/* If any alternate SRR register are defined, duplicate saved values */ if (asrr0 !=-1 ) { env->spr[asrr0] = env->spr[srr0];
Signature · 4 lines
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
Re: [Qemu-devel] [PATCH v2] ppc: allow certain HV interrupts to be delivered to guests
Quoted 98 lines · On Mon, 24 Oct 2016 12:16:19 +1100 David Gibson <[email protected]>…
Quoted 89 lines · Nicholas Piggin
Quoted 44 lines · On Fri, 21 Oct 2016 12:09:54 +1100 David Gibson <[email protected]>…
Quoted 43 lines · Nicholas Piggin
Quoted 30 lines · On Thu, 20 Oct 2016 15:08:07 +0200 Cédric Le Goater <[email protected]> wrote:
Quoted 16 lines · Nicholas Piggin
Signed-off-by Nicholas Piggin <[email protected]>--- target-ppc/excp_helper.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)target-ppc/excp_helper.c +2
-0@@ -390,9 +390,13 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)/* indicate that we resumed from power save mode */ msr |=0x10000 ; new_msr |= ((target_ulong )1 <<MSR_ME ); + new_msr |= (target_ulong )MSR_HVB ; + }else {target-ppc/excp_helper.c static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) · line 402
Quoted 10 lines
target-ppc/excp_helper.c +4
-2+/* The ISA specifies the HV bit is set when the hardware interrupt +* is raised, however when hypervisors deliver the exception to +* guests, it should not be set. +*/ } - - new_msr |= (target_ulong )MSR_HVB ; ail =0 ;break ;case POWERPC_EXCP_DSEG :/* Data segment exception */ should not that be cleared later on in
powerpc_excp()by :Quoted 3 lines
env->msr = new_msr & env->msr_mask;? but the routine is rather long so I might be missing a branch.
No you're right, so it can't leak into the guest, phew!
The problem I get is the interrupt code doing some things differently depending on on the HV bit. For example what I noticed is the guest losing its LE bit upon entry.
Quoted 6 lines
Quoted 3 lines
Perhaps a cleaner way is for the system reset case to set
new_msraccording to the ISA, and then apply themsr_mask(or at least mask out HV) before calculating the exception model? Any preference?I think the proposed revision makes sense.
What do you think of this version? This fixes up machine check guest delivery as well. I'm sending this ahead of the new hcall patch, because it's a bugfix for existing code. I'll get around to the hcall again next week.
Thanks,
Nickppc hypervisors have delivered system reset and machine check exception interrupts to guests in some situations (e.g., see FWNMI feature of LoPAPR, or NMI injection in QEMU).
These exceptions are architected to set the HV bit in hardware, however when injected into a guest, the HV bit should be cleared. Current code masks off the HV bit before setting the new MSR, however this happens after the interrupt delivery model has calculated delivery mode for the exception. This can result in the guest's MSR LE bit being lost.
Provide a new flag for HV exceptions to allow delivery to guests. The exception model masks out the HV bit.
Also add another sanity check to ensure other such exceptions don't try to set HV in guest without setting
guest_hv_excpSigned-off-by Nicholas Piggin <[email protected]>--- target-ppc/excp_helper.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-)target-ppc/excp_helper.c +0
-0target-ppc/excp_helper.c static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) · line 80
Quoted 5 lines
target-ppc/excp_helper.c +1
-1@@ -77,7 +77,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)CPUState *cs =CPU (cpu);CPUPPCState *env = &cpu->env;target_ulong msr, new_msr, vector; -int srr0, srr1, asrr0, asrr1, lev, ail; +int srr0, srr1, asrr0, asrr1, lev, ail, guest_hv_excp;So, to clarify my understanding of this.
Quoted 3 lines
The
guest_hv_excpflag indicates that this is a normally-HV exception which *could* be delivered to a guest with HV clear, *not* that we're actually doing so in this instance. Yes?
Correct.
Thanks,
Nick
Re: [Qemu-devel] [PATCH v2] ppc: allow certain HV interrupts to be delivered to guests
Quoted 101 lines · Nicholas Piggin
Quoted 98 lines · On Mon, 24 Oct 2016 12:16:19 +1100 David Gibson <[email protected]>…
Quoted 89 lines · Nicholas Piggin
Quoted 44 lines · On Fri, 21 Oct 2016 12:09:54 +1100 David Gibson <[email protected]>…
Quoted 43 lines · Nicholas Piggin
Quoted 30 lines · On Thu, 20 Oct 2016 15:08:07 +0200 Cédric Le Goater <[email protected]> wrote:
Quoted 16 lines · Nicholas Piggin
Signed-off-by Nicholas Piggin <[email protected]>--- target-ppc/excp_helper.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)target-ppc/excp_helper.c +2
-0@@ -390,9 +390,13 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)/* indicate that we resumed from power save mode */ msr |=0x10000 ; new_msr |= ((target_ulong )1 <<MSR_ME ); + new_msr |= (target_ulong )MSR_HVB ; + }else {target-ppc/excp_helper.c static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) · line 402
Quoted 10 lines
target-ppc/excp_helper.c +4
-2+/* The ISA specifies the HV bit is set when the hardware interrupt +* is raised, however when hypervisors deliver the exception to +* guests, it should not be set. +*/ } - - new_msr |= (target_ulong )MSR_HVB ; ail =0 ;break ;case POWERPC_EXCP_DSEG :/* Data segment exception */ should not that be cleared later on in
powerpc_excp()by :Quoted 3 lines
env->msr = new_msr & env->msr_mask;? but the routine is rather long so I might be missing a branch.
No you're right, so it can't leak into the guest, phew!
The problem I get is the interrupt code doing some things differently depending on on the HV bit. For example what I noticed is the guest losing its LE bit upon entry.
Quoted 6 lines
Quoted 3 lines
Perhaps a cleaner way is for the system reset case to set
new_msraccording to the ISA, and then apply themsr_mask(or at least mask out HV) before calculating the exception model? Any preference?I think the proposed revision makes sense.
What do you think of this version? This fixes up machine check guest delivery as well. I'm sending this ahead of the new hcall patch, because it's a bugfix for existing code. I'll get around to the hcall again next week.
Thanks,
Nickppc hypervisors have delivered system reset and machine check exception interrupts to guests in some situations (e.g., see FWNMI feature of LoPAPR, or NMI injection in QEMU).
These exceptions are architected to set the HV bit in hardware, however when injected into a guest, the HV bit should be cleared. Current code masks off the HV bit before setting the new MSR, however this happens after the interrupt delivery model has calculated delivery mode for the exception. This can result in the guest's MSR LE bit being lost.
Provide a new flag for HV exceptions to allow delivery to guests. The exception model masks out the HV bit.
Also add another sanity check to ensure other such exceptions don't try to set HV in guest without setting
guest_hv_excpSigned-off-by Nicholas Piggin <[email protected]>--- target-ppc/excp_helper.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-)target-ppc/excp_helper.c +0
-0target-ppc/excp_helper.c static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) · line 80
Quoted 5 lines
target-ppc/excp_helper.c +1
-1@@ -77,7 +77,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)CPUState *cs =CPU (cpu);CPUPPCState *env = &cpu->env;target_ulong msr, new_msr, vector; -int srr0, srr1, asrr0, asrr1, lev, ail; +int srr0, srr1, asrr0, asrr1, lev, ail, guest_hv_excp;So, to clarify my understanding of this.
Quoted 5 lines
Quoted 3 lines
The
guest_hv_excpflag indicates that this is a normally-HV exception which *could* be delivered to a guest with HV clear, *not* that we're actually doing so in this instance. Yes?Correct.
Ok. Hmm. Could you please do a minor respin and:
- change the flag to a bool
- add a comment explaining this meaning
- if you can think of a name which makes the meaning more obvious (I can't, quickly), change that too
Signature · 4 lines
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
PATCH 3/3 ppc/spapr: implement H_SIGNAL_SYS_RESET
In reply to Nicholas PigginThe H_SIGNAL_SYS_RESET hcall allows a guest CPU to raise a system reset exception on other CPUs in the same guest.
--- hw/ppc/spapr_hcall.c | 42 ++++++++++++++++++++++++++++++++++++++++++ include/hw/ppc/spapr.h | 8 +++++++- 2 files changed, 49 insertions(+), 1 deletion(-)
hw/ppc/spapr_hcall.c +42 -0
@@ -880,6 +880,47 @@ static target_ulong h_set_mode(PowerPCCPU *cpu, sPAPRMachineState *spapr,return ret; } +static void do_sys_reset (CPUState *cs,void *arg) +{ +cpu_synchronize_state (cs); +ppc_cpu_do_system_reset (cs); +} + +static target_ulong h_signal_sys_reset (PowerPCCPU *cpu, +sPAPRMachineState *spapr, +target_ulong opcode,target_ulong *args) +{ +target_long target = args[0 ]; +CPUState *cs; + +if (target <H_SIGNAL_SYS_RESET_ALLBUTSELF ) { +return H_PARAMETER ; + } + +CPU_FOREACH (cs) { +PowerPCCPU *c =POWERPC_CPU (cs); + +if (cpu->cpu_dt_id == target) { +run_on_cpu (cs, do_sys_reset,NULL ); +return H_SUCCESS ; + } + +if (target ==H_SIGNAL_SYS_RESET_ALLBUTSELF ) { +if (c == cpu) { +continue ; + } + } + +run_on_cpu (cs, do_sys_reset,NULL ); + } + +if (target >=0 ) { +return H_PARAMETER ; + } + +return H_SUCCESS ; +} +/* * Return the offset to the requested option vector @vector in the * option vector table @table.
@@ -1113,6 +1154,7 @@ static void hypercall_register_types(void)/* hcall-splpar */ spapr_register_hypercall (H_REGISTER_VPA , h_register_vpa);spapr_register_hypercall (H_CEDE , h_cede); +spapr_register_hypercall (H_SIGNAL_SYS_RESET , h_signal_sys_reset);/* processor register resource access h-calls */ spapr_register_hypercall (H_SET_SPRG0 , h_set_sprg0);
include/hw/ppc/spapr.h +7 -1
@@ -339,7 +339,13 @@ struct sPAPRMachineState {#define H_XIRR_X 0x2FC#define H_RANDOM 0x300#define H_SET_MODE 0x31C -#define MAX_HCALL_OPCODE H_SET_MODE +#define H_SIGNAL_SYS_RESET 0x380 +#define MAX_HCALL_OPCODE H_SIGNAL_SYS_RESET + +/* Parameters to H_SIGNAL_SYS_RESET */ +#define H_SIGNAL_SYS_RESET_ALL -1 +#define H_SIGNAL_SYS_RESET_ALLBUTSELF -2 +/* The hcalls above are standardized in PAPR and implemented by pHyp * as well.
Signature · 1 line
2.9.3
Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/3] ppc/spapr: implement H_SIGNAL_SYS_RESET
Quoted 8 lines · Nicholas Piggin
The
H_SIGNAL_SYS_RESEThcall allows a guest CPU to raise a system reset exception on other CPUs in the same guest.Signed-off-by Nicholas Piggin <[email protected]>--- hw/ppc/spapr_hcall.c | 42 ++++++++++++++++++++++++++++++++++++++++++ include/hw/ppc/spapr.h | 8 +++++++- 2 files changed, 49 insertions(+), 1 deletion(-)
...
Quoted 9 lines
include/hw/ppc/spapr.h +0
-1@@ -339,7 +339,13 @@ struct sPAPRMachineState {#define H_XIRR_X 0x2FC#define H_RANDOM 0x300#define H_SET_MODE 0x31C -#define MAX_HCALL_OPCODE H_SET_MODE
include/hw/ppc/spapr.h struct sPAPRMachineState { · line 351
Quoted 10 lines
include/hw/ppc/spapr.h +7
-0+#define H_SIGNAL_SYS_RESET 0x380 +#define MAX_HCALL_OPCODE H_SIGNAL_SYS_RESET + +/* Parameters to H_SIGNAL_SYS_RESET */ +#define H_SIGNAL_SYS_RESET_ALL -1 +#define H_SIGNAL_SYS_RESET_ALLBUTSELF -2 +/* The hcalls above are standardized in PAPR and implemented by pHyp * as well.
Is there a spec for this hypercall? I can't find it in LoPAPR v1.1 ?
Thomas
Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/3] ppc/spapr: implement H_SIGNAL_SYS_RESET
Quoted 27 lines · On Thu, 20 Oct 2016 11:21:22 +0200 Thomas Huth <[email protected]> wrote:
Quoted 8 lines · Nicholas Piggin
The
H_SIGNAL_SYS_RESEThcall allows a guest CPU to raise a system reset exception on other CPUs in the same guest.Signed-off-by Nicholas Piggin <[email protected]>--- hw/ppc/spapr_hcall.c | 42 ++++++++++++++++++++++++++++++++++++++++++ include/hw/ppc/spapr.h | 8 +++++++- 2 files changed, 49 insertions(+), 1 deletion(-)...
Quoted 17 lines
include/hw/ppc/spapr.h +7
-1@@ -339,7 +339,13 @@ struct sPAPRMachineState {#define H_XIRR_X 0x2FC#define H_RANDOM 0x300#define H_SET_MODE 0x31C -#define MAX_HCALL_OPCODE H_SET_MODE +#define H_SIGNAL_SYS_RESET 0x380 +#define MAX_HCALL_OPCODE H_SIGNAL_SYS_RESET + +/* Parameters to H_SIGNAL_SYS_RESET */ +#define H_SIGNAL_SYS_RESET_ALL -1 +#define H_SIGNAL_SYS_RESET_ALLBUTSELF -2 +
Quoted 4 lines
include/hw/ppc/spapr.h struct sPAPRMachineState { · line 351
Quoted 2 lines
include/hw/ppc/spapr.h +0
-0/* The hcalls above are standardized in PAPR and implemented by pHyp * as well. Is there a spec for this hypercall? I can't find it in LoPAPR v1.1 ?
Oh sorry, I should have said that this is going through an internal process at moment and not in a released document yet. I'll try to get a more satisfying answer on that one.
Thanks,
Nick
Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/3] ppc/spapr: implement H_SIGNAL_SYS_RESET
In reply to Nicholas PigginQuoted 3 lines · On Thu, 20 Oct 2016 17:59:12 +1100 Nicholas Piggin <[email protected]> wrote:
The
H_SIGNAL_SYS_RESEThcall allows a guest CPU to raise a system reset exception on other CPUs in the same guest.
Actually on all CPUs or all-but-self CPUs or a specific CPU (including self).
Quoted 11 lines
Signed-off-by Nicholas Piggin <[email protected]>--- hw/ppc/spapr_hcall.c | 42 ++++++++++++++++++++++++++++++++++++++++++ include/hw/ppc/spapr.h | 8 +++++++- 2 files changed, 49 insertions(+), 1 deletion(-)hw/ppc/spapr_hcall.c +0
-0
hw/ppc/spapr_hcall.c static target_ulong h_set_mode(PowerPCCPU *cpu, sPAPRMachineState *spapr, · line 888
Quoted 9 lines
hw/ppc/spapr_hcall.c +6
-0@@ -880,6 +880,47 @@ static target_ulong h_set_mode(PowerPCCPU *cpu, sPAPRMachineState *spapr,return ret; } +static void do_sys_reset (CPUState *cs,void *arg) +{ +cpu_synchronize_state (cs); +ppc_cpu_do_system_reset (cs); +} +
We already have the following function in hw/ppc/spapr.c, which serves the same purpose:
static void ppc_cpu_do_nmi_on_cpu(CPUState *cs, void *arg)
{
cpu_synchronize_state(cs);
ppc_cpu_do_system_reset(cs);
}What about using it ?
Quoted 52 lines
hw/ppc/spapr_hcall.c +36
-0+static target_ulong h_signal_sys_reset (PowerPCCPU *cpu, +sPAPRMachineState *spapr, +target_ulong opcode,target_ulong *args) +{ +target_long target = args[0 ]; +CPUState *cs; + +if (target <H_SIGNAL_SYS_RESET_ALLBUTSELF ) { +return H_PARAMETER ; + } + +CPU_FOREACH (cs) { +PowerPCCPU *c =POWERPC_CPU (cs); + +if (cpu->cpu_dt_id == target) { +run_on_cpu (cs, do_sys_reset,NULL ); +return H_SUCCESS ; + } + +if (target ==H_SIGNAL_SYS_RESET_ALLBUTSELF ) { +if (c == cpu) { +continue ; + } + } + +run_on_cpu (cs, do_sys_reset,NULL ); + } + +if (target >=0 ) { +return H_PARAMETER ; + } + +return H_SUCCESS ; +} +/* * Return the offset to the requested option vector @vector in the * option vector table @table. @@ -1113,6 +1154,7 @@ static void hypercall_register_types(void)/* hcall-splpar */ spapr_register_hypercall (H_REGISTER_VPA , h_register_vpa);spapr_register_hypercall (H_CEDE , h_cede); +spapr_register_hypercall (H_SIGNAL_SYS_RESET , h_signal_sys_reset);/* processor register resource access h-calls */ spapr_register_hypercall (H_SET_SPRG0 , h_set_sprg0);include/hw/ppc/spapr.h +0
-0@@ -339,7 +339,13 @@ struct sPAPRMachineState {#define H_XIRR_X 0x2FC
include/hw/ppc/spapr.h struct sPAPRMachineState { · line 348
Quoted 10 lines
include/hw/ppc/spapr.h +7
-1#define H_RANDOM 0x300#define H_SET_MODE 0x31C -#define MAX_HCALL_OPCODE H_SET_MODE +#define H_SIGNAL_SYS_RESET 0x380 +#define MAX_HCALL_OPCODE H_SIGNAL_SYS_RESET + +/* Parameters to H_SIGNAL_SYS_RESET */ +#define H_SIGNAL_SYS_RESET_ALL -1 +#define H_SIGNAL_SYS_RESET_ALLBUTSELF -2 +
I'd rather move these to hw/ppc/spapr_hcall.c just above h_signal_sys_reset(), the same way it is done for h_bulk_remove().
Quoted 3 lines
include/hw/ppc/spapr.h +0
-0/* The hcalls above are standardized in PAPR and implemented by pHyp * as well.
Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/3] ppc/spapr: implement H_SIGNAL_SYS_RESET
Quoted 8 lines · On Thu, 20 Oct 2016 18:49:22 +0200 Greg Kurz <[email protected]> wrote:
Quoted 3 lines · On Thu, 20 Oct 2016 17:59:12 +1100 Nicholas Piggin <[email protected]> wrote:
The
H_SIGNAL_SYS_RESEThcall allows a guest CPU to raise a system reset exception on other CPUs in the same guest.Actually on all CPUs or all-but-self CPUs or a specific CPU (including self).
Exactly right. I'll improve the changelog and try to add something more official.
Quoted 24 lines
Quoted 11 lines
Signed-off-by Nicholas Piggin <[email protected]>--- hw/ppc/spapr_hcall.c | 42 ++++++++++++++++++++++++++++++++++++++++++ include/hw/ppc/spapr.h | 8 +++++++- 2 files changed, 49 insertions(+), 1 deletion(-)hw/ppc/spapr_hcall.c +0
-0hw/ppc/spapr_hcall.c static target_ulong h_set_mode(PowerPCCPU *cpu, sPAPRMachineState *spapr, · line 888
Quoted 9 lines
hw/ppc/spapr_hcall.c +6
-0@@ -880,6 +880,47 @@ static target_ulong h_set_mode(PowerPCCPU *cpu, sPAPRMachineState *spapr,return ret; } +static void do_sys_reset (CPUState *cs,void *arg) +{ +cpu_synchronize_state (cs); +ppc_cpu_do_system_reset (cs); +} +We already have the following function in
hw/ppc/spapr.c, which serves the same purpose:
Quoted 7 lines
static void
ppc_cpu_do_nmi_on_cpu(CPUState *cs, void *arg){ cpu_synchronize_state(cs); ppc_cpu_do_system_reset(cs); }What about using it ?
I suppose. "nmi" isn't really architected, whereas system reset is. I was considering ppc_cpu_do_nmi_on_cpu call do_sys_reset, but it didn't seem like much of an improvement. I'll use a single function for both if you prefer.
Quoted 9 lines
Quoted 9 lines
include/hw/ppc/spapr.h +2
-1@@ -339,7 +339,13 @@ struct sPAPRMachineState {#define H_XIRR_X 0x2FC#define H_RANDOM 0x300#define H_SET_MODE 0x31C -#define MAX_HCALL_OPCODE H_SET_MODE +#define H_SIGNAL_SYS_RESET 0x380 +#define MAX_HCALL_OPCODE H_SIGNAL_SYS_RESET
Quoted 8 lines
include/hw/ppc/spapr.h struct sPAPRMachineState { · line 348
Quoted 5 lines
include/hw/ppc/spapr.h +5
-0+ +/* Parameters to H_SIGNAL_SYS_RESET */ +#define H_SIGNAL_SYS_RESET_ALL -1 +#define H_SIGNAL_SYS_RESET_ALLBUTSELF -2 +I'd rather move these to
hw/ppc/spapr_hcall.cjust aboveh_signal_sys_reset(), the same way it is done forh_bulk_remove().
Will do.
Thanks,
Nick
Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/3] ppc/spapr: implement H_SIGNAL_SYS_RESET
Quoted 46 lines · Nicholas Piggin
Quoted 8 lines · On Thu, 20 Oct 2016 18:49:22 +0200 Greg Kurz <[email protected]> wrote:
Quoted 3 lines · On Thu, 20 Oct 2016 17:59:12 +1100 Nicholas Piggin <[email protected]> wrote:
The
H_SIGNAL_SYS_RESEThcall allows a guest CPU to raise a system reset exception on other CPUs in the same guest.Actually on all CPUs or all-but-self CPUs or a specific CPU (including self).
Exactly right. I'll improve the changelog and try to add something more official.
Quoted 30 lines
Quoted 11 lines
Signed-off-by Nicholas Piggin <[email protected]>--- hw/ppc/spapr_hcall.c | 42 ++++++++++++++++++++++++++++++++++++++++++ include/hw/ppc/spapr.h | 8 +++++++- 2 files changed, 49 insertions(+), 1 deletion(-)hw/ppc/spapr_hcall.c +0
-0hw/ppc/spapr_hcall.c static target_ulong h_set_mode(PowerPCCPU *cpu, sPAPRMachineState *spapr, · line 888
Quoted 9 lines
hw/ppc/spapr_hcall.c +6
-0@@ -880,6 +880,47 @@ static target_ulong h_set_mode(PowerPCCPU *cpu, sPAPRMachineState *spapr,return ret; } +static void do_sys_reset (CPUState *cs,void *arg) +{ +cpu_synchronize_state (cs); +ppc_cpu_do_system_reset (cs); +} +We already have the following function in
hw/ppc/spapr.c, which serves the same purpose:static void
ppc_cpu_do_nmi_on_cpu(CPUState *cs, void *arg){ cpu_synchronize_state(cs); ppc_cpu_do_system_reset(cs); }
Quoted 6 lines
Quoted 1 line
What about using it ?
I suppose. "nmi" isn't really architected, whereas system reset is. I was considering
ppc_cpu_do_nmi_on_cpucalldo_sys_reset, but it didn't seem like much of an improvement. I'll use a single function for both if you prefer.
I think system_reset is a better name, so it's probably best to rename the existing function.
Quoted 24 lines
Quoted 10 lines
Quoted 10 lines
include/hw/ppc/spapr.h +2
-1@@ -339,7 +339,13 @@ struct sPAPRMachineState {#define H_XIRR_X 0x2FC#define H_RANDOM 0x300#define H_SET_MODE 0x31C -#define MAX_HCALL_OPCODE H_SET_MODE +#define H_SIGNAL_SYS_RESET 0x380 +#define MAX_HCALL_OPCODE H_SIGNAL_SYS_RESETQuoted 8 lines
include/hw/ppc/spapr.h struct sPAPRMachineState { · line 348
Quoted 5 lines
include/hw/ppc/spapr.h +5
-0+ +/* Parameters to H_SIGNAL_SYS_RESET */ +#define H_SIGNAL_SYS_RESET_ALL -1 +#define H_SIGNAL_SYS_RESET_ALLBUTSELF -2 +I'd rather move these to
hw/ppc/spapr_hcall.cjust aboveh_signal_sys_reset(), the same way it is done forh_bulk_remove().Will do.
Thanks,
Nick
Signature · 4 lines
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
Quoted 22 lines · On Thu, 20 Oct 2016 17:59:09 +1100 Nicholas Piggin <[email protected]> wrote:
Date: Thu, 20 Oct 2016 17:38:24 +1100
Subject: [PATCH 0/3] ppc: system reset interrupt fixes and new hcall IPIHi,
We are implementing this new unmaskable IPI hcall for crash dumping and debugging. I had some issues with QEMU delivering system reset interrupt to guests, which was caused by the HV bit being set. After changing that, the interrupt was being handled okay. Should there be a more general check to ensure the HV bit is not set in the guest?
I implemented Linux support for the new hcall in crashdump code, and it works.
Thanks,
NickNicholas Piggin (3):
ppc: fixMSR_MEhandling for system reset interrupt
ppc: allow system reset interrupt to be delivered to guests ppc/spapr: implementH_SIGNAL_SYS_RESET
Quoted 5 lines
hw/ppc/spapr_hcall.c | 42 ++++++++++++++++++++++++++++++++++++++++++ include/hw/ppc/spapr.h | 8 +++++++- target-ppc/excp_helper.c | 10 +++++++--- 3 files changed, 56 insertions(+), 4 deletions(-)
Weird... these patches didn't make it to qemu-devel. As a result, they don't appear in patchwork and http://wiki.qemu.org/patches/patches.json
Quoted 25 lines · On Thu, 20 Oct 2016 13:49:25 +0200 Greg Kurz <[email protected]> wrote:
Quoted 22 lines · On Thu, 20 Oct 2016 17:59:09 +1100 Nicholas Piggin <[email protected]> wrote:
Date: Thu, 20 Oct 2016 17:38:24 +1100
Subject: [PATCH 0/3] ppc: system reset interrupt fixes and new hcall IPIHi,
We are implementing this new unmaskable IPI hcall for crash dumping and debugging. I had some issues with QEMU delivering system reset interrupt to guests, which was caused by the HV bit being set. After changing that, the interrupt was being handled okay. Should there be a more general check to ensure the HV bit is not set in the guest?
I implemented Linux support for the new hcall in crashdump code, and it works.
Thanks,
NickNicholas Piggin (3):
ppc: fixMSR_MEhandling for system reset interrupt
ppc: allow system reset interrupt to be delivered to guests ppc/spapr: implementH_SIGNAL_SYS_RESET
Quoted 8 lines
Quoted 5 lines
hw/ppc/spapr_hcall.c | 42 ++++++++++++++++++++++++++++++++++++++++++ include/hw/ppc/spapr.h | 8 +++++++- target-ppc/excp_helper.c | 10 +++++++--- 3 files changed, 56 insertions(+), 4 deletions(-)Weird... these patches didn't make it to qemu-devel. As a result, they don't appear in patchwork and http://wiki.qemu.org/patches/patches.json
Maybe because I messed up the first mail. I'll resend the series after feedback.
Thanks,
Nick
Quoted 10 lines · Nicholas Piggin
Date: Thu, 20 Oct 2016 17:38:24 +1100
Subject: [PATCH 0/3] ppc: system reset interrupt fixes and new hcall IPIHi,
We are implementing this new unmaskable IPI hcall for crash dumping and debugging. I had some issues with QEMU delivering system reset interrupt to guests, which was caused by the HV bit being set. After changing that, the interrupt was being handled okay. Should there be a more general check to ensure the HV bit is not set in the guest?
cpu_ppc_set_papr() called by cpu_init() does :
env->msr_mask &= ~((1ull << MSR_EP) | MSR_HVB);
and then for the reset :
ppc_cpu_reset()
hreg_store_msr() value &= env->msr_mask;
at first sight, we should be fine ?
C.
Quoted 17 lines
I implemented Linux support for the new hcall in crashdump code, and it works.
Thanks,
NickNicholas Piggin (3):
ppc: fixMSR_MEhandling for system reset interrupt
ppc: allow system reset interrupt to be delivered to guests ppc/spapr: implementH_SIGNAL_SYS_RESEThw/ppc/spapr_hcall.c | 42 ++++++++++++++++++++++++++++++++++++++++++ include/hw/ppc/spapr.h | 8 +++++++- target-ppc/excp_helper.c | 10 +++++++--- 3 files changed, 56 insertions(+), 4 deletions(-)