* [PATCH 0/2] ppc: add support for Directed Privileged Doorbell (non-hypervisor) @ 2020-01-09 16:33 Cédric Le Goater 2020-01-09 16:33 ` [PATCH 1/2] target/ppc: Add privileged message send facilities Cédric Le Goater 2020-01-09 16:33 ` [PATCH 2/2] target/ppc: add support for Hypervisor Facility Unavailable Exception Cédric Le Goater 0 siblings, 2 replies; 6+ messages in thread From: Cédric Le Goater @ 2020-01-09 16:33 UTC (permalink / raw) To: David Gibson; +Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, qemu-devel Hello, The Processor Control facility POWER8 processors and later provides a mechanism for the hypervisor to send messages to other threads in the system (msgsnd instruction) and cause hypervisor-level exceptions. Privileged non-hypervisor programs can also send messages (msgsndp instruction) but are restricted to the threads of the same subprocessor and cause privileged-level exceptions. The Directed Privileged Doorbell Exception State (DPDES) register reflects the state of pending privileged-level doorbell exceptions for all threads and can be used to modify that state. If the MSGP facility is not in the HFSCR, a hypervisor facility unavailable exception is generated when these instructions are used or when the DPDES register is accessed by the supervisor. Based on previous work from Suraj Jitindar Singh. I took ownership due to the amount of changes. Thanks, C. Cédric Le Goater (2): target/ppc: Add privileged message send facilities target/ppc: add support for Hypervisor Facility Unavailable Exception target/ppc/cpu.h | 7 +++ target/ppc/helper.h | 4 ++ target/ppc/excp_helper.c | 81 ++++++++++++++++++++++++++++----- target/ppc/misc_helper.c | 63 +++++++++++++++++++++++++ target/ppc/translate.c | 26 +++++++++++ target/ppc/translate_init.inc.c | 20 ++++++-- 6 files changed, 186 insertions(+), 15 deletions(-) -- 2.21.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] target/ppc: Add privileged message send facilities 2020-01-09 16:33 [PATCH 0/2] ppc: add support for Directed Privileged Doorbell (non-hypervisor) Cédric Le Goater @ 2020-01-09 16:33 ` Cédric Le Goater 2020-01-17 9:46 ` David Gibson 2020-01-09 16:33 ` [PATCH 2/2] target/ppc: add support for Hypervisor Facility Unavailable Exception Cédric Le Goater 1 sibling, 1 reply; 6+ messages in thread From: Cédric Le Goater @ 2020-01-09 16:33 UTC (permalink / raw) To: David Gibson Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, Suraj Jitindar Singh, qemu-devel The Processor Control facility POWER8 processors and later provides a mechanism for the hypervisor to send messages to other threads in the system (msgsnd instruction) and cause hypervisor-level exceptions. Privileged non-hypervisor programs are also able to send messages (msgsndp instruction) but are restricted to the threads of the same core and cause privileged-level exceptions. The Directed Privileged Doorbell Exception State (DPDES) register reflects the state of pending privileged doorbell exceptions and can also be used to modify that state. The register can be used to read and modify the state of privileged doorbell exceptions for all threads of a subprocessor and thus is a shared facility for that subprocessor. The register can be read/written by the hypervisor and read by the supervisor if enabled in the HFSCR, otherwise a hypervisor facility unavailable exception is generated. The privileged message send and clear instructions (msgsndp & msgclrp) are used to generate and clear the presence of a directed privileged doorbell exception, respectively. The msgsndp instruction can be used to target any thread of the current subprocessor, msgclrp acts on the thread issuing the instruction. These instructions are privileged, but will generate a hypervisor facility unavailable exception if not enabled in the HFSCR and executed in privileged non-hypervisor state. The HV facility unavailable exception will be addressed in other patch. Add and implement this register and instructions by reading or modifying the pending interrupt state of the cpu. Note that TCG only supports one thread per core and so we only need to worry about the cpu making the access. Based on previous work from Suraj Jitindar Singh. Cc: Suraj Jitindar Singh <sjitindarsingh@gmail.com> [clg: took ownership due to the amount of changes ] Signed-off-by: Cédric Le Goater <clg@kaod.org> --- target/ppc/cpu.h | 1 + target/ppc/helper.h | 4 ++ target/ppc/excp_helper.c | 68 +++++++++++++++++++++++++++------ target/ppc/misc_helper.c | 36 +++++++++++++++++ target/ppc/translate.c | 26 +++++++++++++ target/ppc/translate_init.inc.c | 20 ++++++++-- 6 files changed, 140 insertions(+), 15 deletions(-) diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index 103bfe9dc274..d175ec9a641d 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -935,6 +935,7 @@ enum { #define DBELL_PIRTAG_MASK 0x3fff #define DBELL_PROCIDTAG_MASK PPC_BITMASK(44, 63) +#define DBELL_TIRTAG_MASK PPC_BITMASK(57, 63) #define PPC_PAGE_SIZES_MAX_SZ 8 diff --git a/target/ppc/helper.h b/target/ppc/helper.h index cd0dfe383a2a..cfb4c07085ca 100644 --- a/target/ppc/helper.h +++ b/target/ppc/helper.h @@ -657,6 +657,10 @@ DEF_HELPER_FLAGS_1(load_601_rtcu, TCG_CALL_NO_RWG, tl, env) DEF_HELPER_FLAGS_1(load_purr, TCG_CALL_NO_RWG, tl, env) DEF_HELPER_FLAGS_2(store_purr, TCG_CALL_NO_RWG, void, env, tl) DEF_HELPER_2(store_ptcr, void, env, tl) +DEF_HELPER_FLAGS_1(load_dpdes, TCG_CALL_NO_RWG, tl, env) +DEF_HELPER_FLAGS_2(store_dpdes, TCG_CALL_NO_RWG, void, env, tl) +DEF_HELPER_2(book3s_msgsndp, void, env, tl) +DEF_HELPER_2(book3s_msgclrp, void, env, tl) #endif DEF_HELPER_2(store_sdr1, void, env, tl) DEF_HELPER_2(store_pidr, void, env, tl) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 5752ed4a4d83..343f3a6b30c4 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -900,7 +900,11 @@ static void ppc_hw_interrupt(CPUPPCState *env) } if (env->pending_interrupts & (1 << PPC_INTERRUPT_DOORBELL)) { env->pending_interrupts &= ~(1 << PPC_INTERRUPT_DOORBELL); - powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_DOORI); + if (env->insns_flags & PPC_SEGMENT_64B) { + powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_SDOOR); + } else { + powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_DOORI); + } return; } if (env->pending_interrupts & (1 << PPC_INTERRUPT_HDOORBELL)) { @@ -1221,7 +1225,7 @@ void helper_msgsnd(target_ulong rb) } /* Server Processor Control */ -static int book3s_dbell2irq(target_ulong rb) +static int book3s_dbell2irq(target_ulong rb, bool hv_dbell) { int msg = rb & DBELL_TYPE_MASK; @@ -1230,12 +1234,16 @@ static int book3s_dbell2irq(target_ulong rb) * message type is 5. All other types are reserved and the * instruction is a no-op */ - return msg == DBELL_TYPE_DBELL_SERVER ? PPC_INTERRUPT_HDOORBELL : -1; + if (msg == DBELL_TYPE_DBELL_SERVER) { + return hv_dbell ? PPC_INTERRUPT_HDOORBELL : PPC_INTERRUPT_DOORBELL; + } + + return -1; } void helper_book3s_msgclr(CPUPPCState *env, target_ulong rb) { - int irq = book3s_dbell2irq(rb); + int irq = book3s_dbell2irq(rb, true); if (irq < 0) { return; @@ -1244,16 +1252,10 @@ void helper_book3s_msgclr(CPUPPCState *env, target_ulong rb) env->pending_interrupts &= ~(1 << irq); } -void helper_book3s_msgsnd(target_ulong rb) +static void book3s_msgsnd_common(int pir, int irq) { - int irq = book3s_dbell2irq(rb); - int pir = rb & DBELL_PROCIDTAG_MASK; CPUState *cs; - if (irq < 0) { - return; - } - qemu_mutex_lock_iothread(); CPU_FOREACH(cs) { PowerPCCPU *cpu = POWERPC_CPU(cs); @@ -1267,6 +1269,50 @@ void helper_book3s_msgsnd(target_ulong rb) } qemu_mutex_unlock_iothread(); } + +void helper_book3s_msgsnd(target_ulong rb) +{ + int pir = rb & DBELL_PROCIDTAG_MASK; + int irq = book3s_dbell2irq(rb, true); + + if (irq < 0) { + return; + } + + book3s_msgsnd_common(pir, irq); +} + +#if defined(TARGET_PPC64) +void helper_book3s_msgclrp(CPUPPCState *env, target_ulong rb) +{ + int irq = book3s_dbell2irq(rb, false); + + if (irq < 0) { + return; + } + + env->pending_interrupts &= ~(1 << irq); +} + +/* + * sends a message to other threads that are on the same + * multi-threaded processor + */ +void helper_book3s_msgsndp(CPUPPCState *env, target_ulong rb) +{ + int irq = book3s_dbell2irq(rb, false); + int pir = env->spr_cb[SPR_PIR].default_value; + + if (irq < 0) { + return; + } + + pir &= ~DBELL_TIRTAG_MASK; + pir |= rb & DBELL_TIRTAG_MASK; + + book3s_msgsnd_common(pir, irq); +} +#endif #endif void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c index 2318f3ab45b2..66b5b0824208 100644 --- a/target/ppc/misc_helper.c +++ b/target/ppc/misc_helper.c @@ -105,6 +105,42 @@ void helper_store_pcr(CPUPPCState *env, target_ulong value) env->spr[SPR_PCR] = value & pcc->pcr_mask; } + +/* + * DPDES register is shared. Each bit reflects the state of the + * doorbell interrupt of a thread of the same core. + */ +target_ulong helper_load_dpdes(CPUPPCState *env) +{ + target_ulong dpdes = 0; + + /* TODO: TCG supports only one thread */ + if (env->pending_interrupts & (1 << PPC_INTERRUPT_DOORBELL)) { + dpdes |= (env->spr_cb[SPR_PIR].default_value & DBELL_TIRTAG_MASK); + } + + return dpdes; +} + +void helper_store_dpdes(CPUPPCState *env, target_ulong val) +{ + PowerPCCPU *cpu = env_archcpu(env); + CPUState *cs = CPU(cpu); + + /* TODO: TCG supports only one thread */ + if (val & ~0x1) { + qemu_log_mask(LOG_GUEST_ERROR, "Invalid DPDES register value " + TARGET_FMT_lx"\n", val); + return; + } + + if (val & 0x1) { + env->pending_interrupts |= 1 << PPC_INTERRUPT_DOORBELL; + cpu_interrupt(cs, CPU_INTERRUPT_HARD); + } else { + env->pending_interrupts &= ~(1 << PPC_INTERRUPT_DOORBELL); + } +} #endif /* defined(TARGET_PPC64) */ void helper_store_pidr(CPUPPCState *env, target_ulong val) diff --git a/target/ppc/translate.c b/target/ppc/translate.c index f5fe5d06118a..a3a4a95cdf53 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -6645,6 +6645,28 @@ static void gen_msgsnd(DisasContext *ctx) #endif /* defined(CONFIG_USER_ONLY) */ } +#if defined(TARGET_PPC64) +static void gen_msgclrp(DisasContext *ctx) +{ +#if defined(CONFIG_USER_ONLY) + GEN_PRIV; +#else + CHK_SV; + gen_helper_book3s_msgclrp(cpu_env, cpu_gpr[rB(ctx->opcode)]); +#endif /* defined(CONFIG_USER_ONLY) */ +} + +static void gen_msgsndp(DisasContext *ctx) +{ +#if defined(CONFIG_USER_ONLY) + GEN_PRIV; +#else + CHK_SV; + gen_helper_book3s_msgsndp(cpu_env, cpu_gpr[rB(ctx->opcode)]); +#endif /* defined(CONFIG_USER_ONLY) */ +} +#endif + static void gen_msgsync(DisasContext *ctx) { #if defined(CONFIG_USER_ONLY) @@ -7187,6 +7209,10 @@ GEN_HANDLER(vmladduhm, 0x04, 0x11, 0xFF, 0x00000000, PPC_ALTIVEC), GEN_HANDLER_E(maddhd_maddhdu, 0x04, 0x18, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA300), GEN_HANDLER_E(maddld, 0x04, 0x19, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA300), +GEN_HANDLER2_E(msgsndp, "msgsndp", 0x1F, 0x0E, 0x04, 0x03ff0001, + PPC_NONE, PPC2_ISA207S), +GEN_HANDLER2_E(msgclrp, "msgclrp", 0x1F, 0x0E, 0x05, 0x03ff0001, + PPC_NONE, PPC2_ISA207S), #endif #undef GEN_INT_ARITH_ADD diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c index d33d65dff702..9e2396a7b5a1 100644 --- a/target/ppc/translate_init.inc.c +++ b/target/ppc/translate_init.inc.c @@ -464,6 +464,17 @@ static void spr_write_pcr(DisasContext *ctx, int sprn, int gprn) { gen_helper_store_pcr(cpu_env, cpu_gpr[gprn]); } + +/* DPDES */ +static void spr_read_dpdes(DisasContext *ctx, int gprn, int sprn) +{ + gen_helper_load_dpdes(cpu_gpr[gprn], cpu_env); +} + +static void spr_write_dpdes(DisasContext *ctx, int sprn, int gprn) +{ + gen_helper_store_dpdes(cpu_env, cpu_gpr[gprn]); +} #endif #endif @@ -8238,10 +8249,11 @@ static void gen_spr_power8_dpdes(CPUPPCState *env) { #if !defined(CONFIG_USER_ONLY) /* Directed Privileged Door-bell Exception State, used for IPI */ - spr_register(env, SPR_DPDES, "DPDES", - SPR_NOACCESS, SPR_NOACCESS, - &spr_read_generic, SPR_NOACCESS, - 0x00000000); + spr_register_kvm_hv(env, SPR_DPDES, "DPDES", + SPR_NOACCESS, SPR_NOACCESS, + &spr_read_dpdes, SPR_NOACCESS, + &spr_read_dpdes, &spr_write_dpdes, + KVM_REG_PPC_DPDES, 0x00000000); #endif } -- 2.21.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] target/ppc: Add privileged message send facilities 2020-01-09 16:33 ` [PATCH 1/2] target/ppc: Add privileged message send facilities Cédric Le Goater @ 2020-01-17 9:46 ` David Gibson 2020-01-17 12:42 ` Cédric Le Goater 0 siblings, 1 reply; 6+ messages in thread From: David Gibson @ 2020-01-17 9:46 UTC (permalink / raw) To: Cédric Le Goater Cc: qemu-ppc, Greg Kurz, Suraj Jitindar Singh, qemu-devel [-- Attachment #1: Type: text/plain, Size: 12830 bytes --] On Thu, Jan 09, 2020 at 05:33:45PM +0100, Cédric Le Goater wrote: > The Processor Control facility POWER8 processors and later provides > a mechanism for the hypervisor to send messages to other threads > in the system (msgsnd instruction) and cause hypervisor-level > exceptions. Privileged non-hypervisor programs are also able to > send messages (msgsndp instruction) but are restricted to the > threads of the same core and cause privileged-level exceptions. > > The Directed Privileged Doorbell Exception State (DPDES) register > reflects the state of pending privileged doorbell exceptions and can > also be used to modify that state. The register can be used to read > and modify the state of privileged doorbell exceptions for all threads > of a subprocessor and thus is a shared facility for that subprocessor. > The register can be read/written by the hypervisor and read by the > supervisor if enabled in the HFSCR, otherwise a hypervisor facility > unavailable exception is generated. > > The privileged message send and clear instructions (msgsndp & msgclrp) > are used to generate and clear the presence of a directed privileged > doorbell exception, respectively. The msgsndp instruction can be used > to target any thread of the current subprocessor, msgclrp acts on the > thread issuing the instruction. These instructions are privileged, but > will generate a hypervisor facility unavailable exception if not > enabled in the HFSCR and executed in privileged non-hypervisor > state. The HV facility unavailable exception will be addressed in > other patch. > > Add and implement this register and instructions by reading or > modifying the pending interrupt state of the cpu. > > Note that TCG only supports one thread per core and so we only need to > worry about the cpu making the access. > > Based on previous work from Suraj Jitindar Singh. > > Cc: Suraj Jitindar Singh <sjitindarsingh@gmail.com> > [clg: took ownership due to the amount of changes ] Hrm, I think this need's Suraj's S-o-b as well. AIUI the primary purpose of S-o-b lines isn't about credit or ownership, but about tracking where code came from in case of questions of providence. So even if you've taken ownership and reworked substantially, it would retain the original author's S-o-b. > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > target/ppc/cpu.h | 1 + > target/ppc/helper.h | 4 ++ > target/ppc/excp_helper.c | 68 +++++++++++++++++++++++++++------ > target/ppc/misc_helper.c | 36 +++++++++++++++++ > target/ppc/translate.c | 26 +++++++++++++ > target/ppc/translate_init.inc.c | 20 ++++++++-- > 6 files changed, 140 insertions(+), 15 deletions(-) > > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > index 103bfe9dc274..d175ec9a641d 100644 > --- a/target/ppc/cpu.h > +++ b/target/ppc/cpu.h > @@ -935,6 +935,7 @@ enum { > #define DBELL_PIRTAG_MASK 0x3fff > > #define DBELL_PROCIDTAG_MASK PPC_BITMASK(44, 63) > +#define DBELL_TIRTAG_MASK PPC_BITMASK(57, 63) > > #define PPC_PAGE_SIZES_MAX_SZ 8 > > diff --git a/target/ppc/helper.h b/target/ppc/helper.h > index cd0dfe383a2a..cfb4c07085ca 100644 > --- a/target/ppc/helper.h > +++ b/target/ppc/helper.h > @@ -657,6 +657,10 @@ DEF_HELPER_FLAGS_1(load_601_rtcu, TCG_CALL_NO_RWG, tl, env) > DEF_HELPER_FLAGS_1(load_purr, TCG_CALL_NO_RWG, tl, env) > DEF_HELPER_FLAGS_2(store_purr, TCG_CALL_NO_RWG, void, env, tl) > DEF_HELPER_2(store_ptcr, void, env, tl) > +DEF_HELPER_FLAGS_1(load_dpdes, TCG_CALL_NO_RWG, tl, env) > +DEF_HELPER_FLAGS_2(store_dpdes, TCG_CALL_NO_RWG, void, env, tl) > +DEF_HELPER_2(book3s_msgsndp, void, env, tl) > +DEF_HELPER_2(book3s_msgclrp, void, env, tl) > #endif > DEF_HELPER_2(store_sdr1, void, env, tl) > DEF_HELPER_2(store_pidr, void, env, tl) > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > index 5752ed4a4d83..343f3a6b30c4 100644 > --- a/target/ppc/excp_helper.c > +++ b/target/ppc/excp_helper.c > @@ -900,7 +900,11 @@ static void ppc_hw_interrupt(CPUPPCState *env) > } > if (env->pending_interrupts & (1 << PPC_INTERRUPT_DOORBELL)) { > env->pending_interrupts &= ~(1 << PPC_INTERRUPT_DOORBELL); > - powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_DOORI); > + if (env->insns_flags & PPC_SEGMENT_64B) { I don't love detecting this based on insns_flags this way, but there are lots of places we do similarly ugly things, so it's probably the most expedient way forward in the short term. > + powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_SDOOR); > + } else { > + powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_DOORI); > + } > return; > } > if (env->pending_interrupts & (1 << PPC_INTERRUPT_HDOORBELL)) { > @@ -1221,7 +1225,7 @@ void helper_msgsnd(target_ulong rb) > } > > /* Server Processor Control */ > -static int book3s_dbell2irq(target_ulong rb) > +static int book3s_dbell2irq(target_ulong rb, bool hv_dbell) > { > int msg = rb & DBELL_TYPE_MASK; > > @@ -1230,12 +1234,16 @@ static int book3s_dbell2irq(target_ulong rb) > * message type is 5. All other types are reserved and the > * instruction is a no-op > */ > - return msg == DBELL_TYPE_DBELL_SERVER ? PPC_INTERRUPT_HDOORBELL : -1; > + if (msg == DBELL_TYPE_DBELL_SERVER) { > + return hv_dbell ? PPC_INTERRUPT_HDOORBELL : PPC_INTERRUPT_DOORBELL; > + } > + > + return -1; > } This function kind of seems like overkill, and also doesn't have a great name. Mostly it just tests if we're dealing with a doorbell at all. Selecting the right irq number here is a bit weird, since its based only on hv_dbell, which is a literal parameter for all callers, so they might as well just use the right doorbell irq inline. > > void helper_book3s_msgclr(CPUPPCState *env, target_ulong rb) > { > - int irq = book3s_dbell2irq(rb); > + int irq = book3s_dbell2irq(rb, true); > > if (irq < 0) { > return; > @@ -1244,16 +1252,10 @@ void helper_book3s_msgclr(CPUPPCState *env, target_ulong rb) > env->pending_interrupts &= ~(1 << irq); > } > > -void helper_book3s_msgsnd(target_ulong rb) > +static void book3s_msgsnd_common(int pir, int irq) > { > - int irq = book3s_dbell2irq(rb); > - int pir = rb & DBELL_PROCIDTAG_MASK; > CPUState *cs; > > - if (irq < 0) { > - return; > - } > - > qemu_mutex_lock_iothread(); > CPU_FOREACH(cs) { > PowerPCCPU *cpu = POWERPC_CPU(cs); > @@ -1267,6 +1269,50 @@ void helper_book3s_msgsnd(target_ulong rb) > } > qemu_mutex_unlock_iothread(); > } > + > +void helper_book3s_msgsnd(target_ulong rb) > +{ > + int pir = rb & DBELL_PROCIDTAG_MASK; > + int irq = book3s_dbell2irq(rb, true); > + > + if (irq < 0) { > + return; > + } > + > + book3s_msgsnd_common(pir, irq); > +} > + > +#if defined(TARGET_PPC64) > +void helper_book3s_msgclrp(CPUPPCState *env, target_ulong rb) > +{ > + int irq = book3s_dbell2irq(rb, false); > + > + if (irq < 0) { > + return; > + } > + > + env->pending_interrupts &= ~(1 << irq); > +} > + > +/* > + * sends a message to other threads that are on the same > + * multi-threaded processor > + */ > +void helper_book3s_msgsndp(CPUPPCState *env, target_ulong rb) > +{ > + int irq = book3s_dbell2irq(rb, false); > + int pir = env->spr_cb[SPR_PIR].default_value; > + > + if (irq < 0) { > + return; > + } > + > + pir &= ~DBELL_TIRTAG_MASK; > + pir |= rb & DBELL_TIRTAG_MASK; This seems overkill since we don't actually support > 1 thread/core. Won't the answer always be equal to pir? > + > + book3s_msgsnd_common(pir, irq); > +} > +#endif > #endif > > void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, > diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c > index 2318f3ab45b2..66b5b0824208 100644 > --- a/target/ppc/misc_helper.c > +++ b/target/ppc/misc_helper.c > @@ -105,6 +105,42 @@ void helper_store_pcr(CPUPPCState *env, target_ulong value) > > env->spr[SPR_PCR] = value & pcc->pcr_mask; > } > + > +/* > + * DPDES register is shared. Each bit reflects the state of the > + * doorbell interrupt of a thread of the same core. > + */ > +target_ulong helper_load_dpdes(CPUPPCState *env) > +{ > + target_ulong dpdes = 0; > + > + /* TODO: TCG supports only one thread */ > + if (env->pending_interrupts & (1 << PPC_INTERRUPT_DOORBELL)) { > + dpdes |= (env->spr_cb[SPR_PIR].default_value & DBELL_TIRTAG_MASK); Likewise, won't this just be 0 or 1? I dislike half-measures to achieve something we don't actually support at this stage. > + } > + > + return dpdes; > +} > + > +void helper_store_dpdes(CPUPPCState *env, target_ulong val) > +{ > + PowerPCCPU *cpu = env_archcpu(env); > + CPUState *cs = CPU(cpu); > + > + /* TODO: TCG supports only one thread */ > + if (val & ~0x1) { > + qemu_log_mask(LOG_GUEST_ERROR, "Invalid DPDES register value " > + TARGET_FMT_lx"\n", val); > + return; > + } > + > + if (val & 0x1) { > + env->pending_interrupts |= 1 << PPC_INTERRUPT_DOORBELL; > + cpu_interrupt(cs, CPU_INTERRUPT_HARD); > + } else { > + env->pending_interrupts &= ~(1 << PPC_INTERRUPT_DOORBELL); > + } > +} > #endif /* defined(TARGET_PPC64) */ > > void helper_store_pidr(CPUPPCState *env, target_ulong val) > diff --git a/target/ppc/translate.c b/target/ppc/translate.c > index f5fe5d06118a..a3a4a95cdf53 100644 > --- a/target/ppc/translate.c > +++ b/target/ppc/translate.c > @@ -6645,6 +6645,28 @@ static void gen_msgsnd(DisasContext *ctx) > #endif /* defined(CONFIG_USER_ONLY) */ > } > > +#if defined(TARGET_PPC64) > +static void gen_msgclrp(DisasContext *ctx) > +{ > +#if defined(CONFIG_USER_ONLY) > + GEN_PRIV; > +#else > + CHK_SV; > + gen_helper_book3s_msgclrp(cpu_env, cpu_gpr[rB(ctx->opcode)]); > +#endif /* defined(CONFIG_USER_ONLY) */ > +} > + > +static void gen_msgsndp(DisasContext *ctx) > +{ > +#if defined(CONFIG_USER_ONLY) > + GEN_PRIV; > +#else > + CHK_SV; > + gen_helper_book3s_msgsndp(cpu_env, cpu_gpr[rB(ctx->opcode)]); > +#endif /* defined(CONFIG_USER_ONLY) */ > +} > +#endif > + > static void gen_msgsync(DisasContext *ctx) > { > #if defined(CONFIG_USER_ONLY) > @@ -7187,6 +7209,10 @@ GEN_HANDLER(vmladduhm, 0x04, 0x11, 0xFF, 0x00000000, PPC_ALTIVEC), > GEN_HANDLER_E(maddhd_maddhdu, 0x04, 0x18, 0xFF, 0x00000000, PPC_NONE, > PPC2_ISA300), > GEN_HANDLER_E(maddld, 0x04, 0x19, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA300), > +GEN_HANDLER2_E(msgsndp, "msgsndp", 0x1F, 0x0E, 0x04, 0x03ff0001, > + PPC_NONE, PPC2_ISA207S), > +GEN_HANDLER2_E(msgclrp, "msgclrp", 0x1F, 0x0E, 0x05, 0x03ff0001, > + PPC_NONE, PPC2_ISA207S), > #endif > > #undef GEN_INT_ARITH_ADD > diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c > index d33d65dff702..9e2396a7b5a1 100644 > --- a/target/ppc/translate_init.inc.c > +++ b/target/ppc/translate_init.inc.c > @@ -464,6 +464,17 @@ static void spr_write_pcr(DisasContext *ctx, int sprn, int gprn) > { > gen_helper_store_pcr(cpu_env, cpu_gpr[gprn]); > } > + > +/* DPDES */ > +static void spr_read_dpdes(DisasContext *ctx, int gprn, int sprn) > +{ > + gen_helper_load_dpdes(cpu_gpr[gprn], cpu_env); > +} > + > +static void spr_write_dpdes(DisasContext *ctx, int sprn, int gprn) > +{ > + gen_helper_store_dpdes(cpu_env, cpu_gpr[gprn]); > +} > #endif > #endif > > @@ -8238,10 +8249,11 @@ static void gen_spr_power8_dpdes(CPUPPCState *env) > { > #if !defined(CONFIG_USER_ONLY) > /* Directed Privileged Door-bell Exception State, used for IPI */ > - spr_register(env, SPR_DPDES, "DPDES", > - SPR_NOACCESS, SPR_NOACCESS, > - &spr_read_generic, SPR_NOACCESS, > - 0x00000000); > + spr_register_kvm_hv(env, SPR_DPDES, "DPDES", > + SPR_NOACCESS, SPR_NOACCESS, > + &spr_read_dpdes, SPR_NOACCESS, > + &spr_read_dpdes, &spr_write_dpdes, > + KVM_REG_PPC_DPDES, 0x00000000); > #endif > } > -- 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] 6+ messages in thread
* Re: [PATCH 1/2] target/ppc: Add privileged message send facilities 2020-01-17 9:46 ` David Gibson @ 2020-01-17 12:42 ` Cédric Le Goater 0 siblings, 0 replies; 6+ messages in thread From: Cédric Le Goater @ 2020-01-17 12:42 UTC (permalink / raw) To: David Gibson; +Cc: qemu-ppc, Greg Kurz, Suraj Jitindar Singh, qemu-devel On 1/17/20 10:46 AM, David Gibson wrote: > On Thu, Jan 09, 2020 at 05:33:45PM +0100, Cédric Le Goater wrote: >> The Processor Control facility POWER8 processors and later provides >> a mechanism for the hypervisor to send messages to other threads >> in the system (msgsnd instruction) and cause hypervisor-level >> exceptions. Privileged non-hypervisor programs are also able to >> send messages (msgsndp instruction) but are restricted to the >> threads of the same core and cause privileged-level exceptions. >> >> The Directed Privileged Doorbell Exception State (DPDES) register >> reflects the state of pending privileged doorbell exceptions and can >> also be used to modify that state. The register can be used to read >> and modify the state of privileged doorbell exceptions for all threads >> of a subprocessor and thus is a shared facility for that subprocessor. >> The register can be read/written by the hypervisor and read by the >> supervisor if enabled in the HFSCR, otherwise a hypervisor facility >> unavailable exception is generated. >> >> The privileged message send and clear instructions (msgsndp & msgclrp) >> are used to generate and clear the presence of a directed privileged >> doorbell exception, respectively. The msgsndp instruction can be used >> to target any thread of the current subprocessor, msgclrp acts on the >> thread issuing the instruction. These instructions are privileged, but >> will generate a hypervisor facility unavailable exception if not >> enabled in the HFSCR and executed in privileged non-hypervisor >> state. The HV facility unavailable exception will be addressed in >> other patch. >> >> Add and implement this register and instructions by reading or >> modifying the pending interrupt state of the cpu. >> >> Note that TCG only supports one thread per core and so we only need to >> worry about the cpu making the access. >> >> Based on previous work from Suraj Jitindar Singh. >> >> Cc: Suraj Jitindar Singh <sjitindarsingh@gmail.com> >> [clg: took ownership due to the amount of changes ] > > Hrm, I think this need's Suraj's S-o-b as well. AIUI the primary > purpose of S-o-b lines isn't about credit or ownership, but about > tracking where code came from in case of questions of providence. So > even if you've taken ownership and reworked substantially, it would > retain the original author's S-o-b. ok. I was just tired of adding lines of changes. I can keep the S-o-b. np. > >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> target/ppc/cpu.h | 1 + >> target/ppc/helper.h | 4 ++ >> target/ppc/excp_helper.c | 68 +++++++++++++++++++++++++++------ >> target/ppc/misc_helper.c | 36 +++++++++++++++++ >> target/ppc/translate.c | 26 +++++++++++++ >> target/ppc/translate_init.inc.c | 20 ++++++++-- >> 6 files changed, 140 insertions(+), 15 deletions(-) >> >> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h >> index 103bfe9dc274..d175ec9a641d 100644 >> --- a/target/ppc/cpu.h >> +++ b/target/ppc/cpu.h >> @@ -935,6 +935,7 @@ enum { >> #define DBELL_PIRTAG_MASK 0x3fff >> >> #define DBELL_PROCIDTAG_MASK PPC_BITMASK(44, 63) >> +#define DBELL_TIRTAG_MASK PPC_BITMASK(57, 63) >> >> #define PPC_PAGE_SIZES_MAX_SZ 8 >> >> diff --git a/target/ppc/helper.h b/target/ppc/helper.h >> index cd0dfe383a2a..cfb4c07085ca 100644 >> --- a/target/ppc/helper.h >> +++ b/target/ppc/helper.h >> @@ -657,6 +657,10 @@ DEF_HELPER_FLAGS_1(load_601_rtcu, TCG_CALL_NO_RWG, tl, env) >> DEF_HELPER_FLAGS_1(load_purr, TCG_CALL_NO_RWG, tl, env) >> DEF_HELPER_FLAGS_2(store_purr, TCG_CALL_NO_RWG, void, env, tl) >> DEF_HELPER_2(store_ptcr, void, env, tl) >> +DEF_HELPER_FLAGS_1(load_dpdes, TCG_CALL_NO_RWG, tl, env) >> +DEF_HELPER_FLAGS_2(store_dpdes, TCG_CALL_NO_RWG, void, env, tl) >> +DEF_HELPER_2(book3s_msgsndp, void, env, tl) >> +DEF_HELPER_2(book3s_msgclrp, void, env, tl) >> #endif >> DEF_HELPER_2(store_sdr1, void, env, tl) >> DEF_HELPER_2(store_pidr, void, env, tl) >> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c >> index 5752ed4a4d83..343f3a6b30c4 100644 >> --- a/target/ppc/excp_helper.c >> +++ b/target/ppc/excp_helper.c >> @@ -900,7 +900,11 @@ static void ppc_hw_interrupt(CPUPPCState *env) >> } >> if (env->pending_interrupts & (1 << PPC_INTERRUPT_DOORBELL)) { >> env->pending_interrupts &= ~(1 << PPC_INTERRUPT_DOORBELL); >> - powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_DOORI); >> + if (env->insns_flags & PPC_SEGMENT_64B) { > > I don't love detecting this based on insns_flags this way, but there > are lots of places we do similarly ugly things, so it's probably the > most expedient way forward in the short term. we should be using the is_book3s_arch2x() introduced by Greg in : d0db7caddb19 ("target/ppc: Consolidate 64-bit server processor detection in a helper") > >> + powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_SDOOR); >> + } else { >> + powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_DOORI); >> + } >> return; >> } >> if (env->pending_interrupts & (1 << PPC_INTERRUPT_HDOORBELL)) { >> @@ -1221,7 +1225,7 @@ void helper_msgsnd(target_ulong rb) >> } >> >> /* Server Processor Control */ >> -static int book3s_dbell2irq(target_ulong rb) >> +static int book3s_dbell2irq(target_ulong rb, bool hv_dbell) >> { >> int msg = rb & DBELL_TYPE_MASK; >> >> @@ -1230,12 +1234,16 @@ static int book3s_dbell2irq(target_ulong rb) >> * message type is 5. All other types are reserved and the >> * instruction is a no-op >> */ >> - return msg == DBELL_TYPE_DBELL_SERVER ? PPC_INTERRUPT_HDOORBELL : -1; >> + if (msg == DBELL_TYPE_DBELL_SERVER) { >> + return hv_dbell ? PPC_INTERRUPT_HDOORBELL : PPC_INTERRUPT_DOORBELL; >> + } >> + >> + return -1; >> } > > This function kind of seems like overkill, and also doesn't have a > great name. Mostly it just tests if we're dealing with a doorbell at > all. Selecting the right irq number here is a bit weird, since its > based only on hv_dbell, which is a literal parameter for all callers, > so they might as well just use the right doorbell irq inline. I Agree. >> >> void helper_book3s_msgclr(CPUPPCState *env, target_ulong rb) >> { >> - int irq = book3s_dbell2irq(rb); >> + int irq = book3s_dbell2irq(rb, true); >> >> if (irq < 0) { >> return; >> @@ -1244,16 +1252,10 @@ void helper_book3s_msgclr(CPUPPCState *env, target_ulong rb) >> env->pending_interrupts &= ~(1 << irq); >> } >> >> -void helper_book3s_msgsnd(target_ulong rb) >> +static void book3s_msgsnd_common(int pir, int irq) >> { >> - int irq = book3s_dbell2irq(rb); >> - int pir = rb & DBELL_PROCIDTAG_MASK; >> CPUState *cs; >> >> - if (irq < 0) { >> - return; >> - } >> - >> qemu_mutex_lock_iothread(); >> CPU_FOREACH(cs) { >> PowerPCCPU *cpu = POWERPC_CPU(cs); >> @@ -1267,6 +1269,50 @@ void helper_book3s_msgsnd(target_ulong rb) >> } >> qemu_mutex_unlock_iothread(); >> } >> + >> +void helper_book3s_msgsnd(target_ulong rb) >> +{ >> + int pir = rb & DBELL_PROCIDTAG_MASK; >> + int irq = book3s_dbell2irq(rb, true); >> + >> + if (irq < 0) { >> + return; >> + } >> + >> + book3s_msgsnd_common(pir, irq); >> +} >> + >> +#if defined(TARGET_PPC64) >> +void helper_book3s_msgclrp(CPUPPCState *env, target_ulong rb) >> +{ >> + int irq = book3s_dbell2irq(rb, false); >> + >> + if (irq < 0) { >> + return; >> + } >> + >> + env->pending_interrupts &= ~(1 << irq); >> +} >> + >> +/* >> + * sends a message to other threads that are on the same >> + * multi-threaded processor >> + */ >> +void helper_book3s_msgsndp(CPUPPCState *env, target_ulong rb) >> +{ >> + int irq = book3s_dbell2irq(rb, false); >> + int pir = env->spr_cb[SPR_PIR].default_value; >> + >> + if (irq < 0) { >> + return; >> + } >> + >> + pir &= ~DBELL_TIRTAG_MASK; >> + pir |= rb & DBELL_TIRTAG_MASK; > > This seems overkill since we don't actually support > 1 thread/core. > Won't the answer always be equal to pir? yes but the mask is different from instruction msgsnd() and if SMT > 1 is support one day, it will be correct. > >> + >> + book3s_msgsnd_common(pir, irq); >> +} >> +#endif >> #endif >> >> void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, >> diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c >> index 2318f3ab45b2..66b5b0824208 100644 >> --- a/target/ppc/misc_helper.c >> +++ b/target/ppc/misc_helper.c >> @@ -105,6 +105,42 @@ void helper_store_pcr(CPUPPCState *env, target_ulong value) >> >> env->spr[SPR_PCR] = value & pcc->pcr_mask; >> } >> + >> +/* >> + * DPDES register is shared. Each bit reflects the state of the >> + * doorbell interrupt of a thread of the same core. >> + */ >> +target_ulong helper_load_dpdes(CPUPPCState *env) >> +{ >> + target_ulong dpdes = 0; >> + >> + /* TODO: TCG supports only one thread */ >> + if (env->pending_interrupts & (1 << PPC_INTERRUPT_DOORBELL)) { >> + dpdes |= (env->spr_cb[SPR_PIR].default_value & DBELL_TIRTAG_MASK); > > Likewise, won't this just be 0 or 1? I dislike half-measures to > achieve something we don't actually support at this stage. OK then. I will simplify and keep the TODOs. Thanks, C. > >> + } >> + >> + return dpdes; >> +} >> + >> +void helper_store_dpdes(CPUPPCState *env, target_ulong val) >> +{ >> + PowerPCCPU *cpu = env_archcpu(env); >> + CPUState *cs = CPU(cpu); >> + >> + /* TODO: TCG supports only one thread */ >> + if (val & ~0x1) { >> + qemu_log_mask(LOG_GUEST_ERROR, "Invalid DPDES register value " >> + TARGET_FMT_lx"\n", val); >> + return; >> + } >> + >> + if (val & 0x1) { >> + env->pending_interrupts |= 1 << PPC_INTERRUPT_DOORBELL; >> + cpu_interrupt(cs, CPU_INTERRUPT_HARD); >> + } else { >> + env->pending_interrupts &= ~(1 << PPC_INTERRUPT_DOORBELL); >> + } >> +} >> #endif /* defined(TARGET_PPC64) */ >> >> void helper_store_pidr(CPUPPCState *env, target_ulong val) >> diff --git a/target/ppc/translate.c b/target/ppc/translate.c >> index f5fe5d06118a..a3a4a95cdf53 100644 >> --- a/target/ppc/translate.c >> +++ b/target/ppc/translate.c >> @@ -6645,6 +6645,28 @@ static void gen_msgsnd(DisasContext *ctx) >> #endif /* defined(CONFIG_USER_ONLY) */ >> } >> >> +#if defined(TARGET_PPC64) >> +static void gen_msgclrp(DisasContext *ctx) >> +{ >> +#if defined(CONFIG_USER_ONLY) >> + GEN_PRIV; >> +#else >> + CHK_SV; >> + gen_helper_book3s_msgclrp(cpu_env, cpu_gpr[rB(ctx->opcode)]); >> +#endif /* defined(CONFIG_USER_ONLY) */ >> +} >> + >> +static void gen_msgsndp(DisasContext *ctx) >> +{ >> +#if defined(CONFIG_USER_ONLY) >> + GEN_PRIV; >> +#else >> + CHK_SV; >> + gen_helper_book3s_msgsndp(cpu_env, cpu_gpr[rB(ctx->opcode)]); >> +#endif /* defined(CONFIG_USER_ONLY) */ >> +} >> +#endif >> + >> static void gen_msgsync(DisasContext *ctx) >> { >> #if defined(CONFIG_USER_ONLY) >> @@ -7187,6 +7209,10 @@ GEN_HANDLER(vmladduhm, 0x04, 0x11, 0xFF, 0x00000000, PPC_ALTIVEC), >> GEN_HANDLER_E(maddhd_maddhdu, 0x04, 0x18, 0xFF, 0x00000000, PPC_NONE, >> PPC2_ISA300), >> GEN_HANDLER_E(maddld, 0x04, 0x19, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA300), >> +GEN_HANDLER2_E(msgsndp, "msgsndp", 0x1F, 0x0E, 0x04, 0x03ff0001, >> + PPC_NONE, PPC2_ISA207S), >> +GEN_HANDLER2_E(msgclrp, "msgclrp", 0x1F, 0x0E, 0x05, 0x03ff0001, >> + PPC_NONE, PPC2_ISA207S), >> #endif >> >> #undef GEN_INT_ARITH_ADD >> diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c >> index d33d65dff702..9e2396a7b5a1 100644 >> --- a/target/ppc/translate_init.inc.c >> +++ b/target/ppc/translate_init.inc.c >> @@ -464,6 +464,17 @@ static void spr_write_pcr(DisasContext *ctx, int sprn, int gprn) >> { >> gen_helper_store_pcr(cpu_env, cpu_gpr[gprn]); >> } >> + >> +/* DPDES */ >> +static void spr_read_dpdes(DisasContext *ctx, int gprn, int sprn) >> +{ >> + gen_helper_load_dpdes(cpu_gpr[gprn], cpu_env); >> +} >> + >> +static void spr_write_dpdes(DisasContext *ctx, int sprn, int gprn) >> +{ >> + gen_helper_store_dpdes(cpu_env, cpu_gpr[gprn]); >> +} >> #endif >> #endif >> >> @@ -8238,10 +8249,11 @@ static void gen_spr_power8_dpdes(CPUPPCState *env) >> { >> #if !defined(CONFIG_USER_ONLY) >> /* Directed Privileged Door-bell Exception State, used for IPI */ >> - spr_register(env, SPR_DPDES, "DPDES", >> - SPR_NOACCESS, SPR_NOACCESS, >> - &spr_read_generic, SPR_NOACCESS, >> - 0x00000000); >> + spr_register_kvm_hv(env, SPR_DPDES, "DPDES", >> + SPR_NOACCESS, SPR_NOACCESS, >> + &spr_read_dpdes, SPR_NOACCESS, >> + &spr_read_dpdes, &spr_write_dpdes, >> + KVM_REG_PPC_DPDES, 0x00000000); >> #endif >> } >> > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] target/ppc: add support for Hypervisor Facility Unavailable Exception 2020-01-09 16:33 [PATCH 0/2] ppc: add support for Directed Privileged Doorbell (non-hypervisor) Cédric Le Goater 2020-01-09 16:33 ` [PATCH 1/2] target/ppc: Add privileged message send facilities Cédric Le Goater @ 2020-01-09 16:33 ` Cédric Le Goater 2020-01-17 9:49 ` David Gibson 1 sibling, 1 reply; 6+ messages in thread From: Cédric Le Goater @ 2020-01-09 16:33 UTC (permalink / raw) To: David Gibson Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, Suraj Jitindar Singh, qemu-devel The privileged message send and clear instructions (msgsndp & msgclrp) are privileged, but will generate a hypervisor facility unavailable exception if not enabled in the HFSCR and executed in privileged non-hypervisor state. Add checks when accessing the DPDES register and when using the msgsndp and msgclrp isntructions. Based on previous work from Suraj Jitindar Singh. Cc: Suraj Jitindar Singh <sjitindarsingh@gmail.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> --- target/ppc/cpu.h | 6 ++++++ target/ppc/excp_helper.c | 13 +++++++++++++ target/ppc/misc_helper.c | 27 +++++++++++++++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index d175ec9a641d..1ff6afbccdb2 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -397,6 +397,10 @@ typedef struct ppc_v3_pate_t { #define PSSCR_ESL PPC_BIT(42) /* Enable State Loss */ #define PSSCR_EC PPC_BIT(43) /* Exit Criterion */ +/* HFSCR bits */ +#define HFSCR_MSGP PPC_BIT(53) /* Privileged Message Send Facilities */ +#define HFSCR_IC_MSGP 0xA + #define msr_sf ((env->msr >> MSR_SF) & 1) #define msr_isf ((env->msr >> MSR_ISF) & 1) #define msr_shv ((env->msr >> MSR_SHV) & 1) @@ -1332,6 +1336,8 @@ void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp); #endif void store_fpscr(CPUPPCState *env, uint64_t arg, uint32_t mask); +void helper_hfscr_facility_check(CPUPPCState *env, uint32_t bit, + const char *caller, uint32_t cause); static inline uint64_t ppc_dump_gpr(CPUPPCState *env, int gprn) { diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 343f3a6b30c4..3887f8888c6c 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -471,6 +471,15 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) case POWERPC_EXCP_FU: /* Facility unavailable exception */ #ifdef TARGET_PPC64 env->spr[SPR_FSCR] |= ((target_ulong)env->error_code << 56); +#endif + break; + case POWERPC_EXCP_HV_FU: /* Hypervisor Facility Unavailable Exception */ +#ifdef TARGET_PPC64 + env->spr[SPR_HFSCR] |= ((target_ulong)env->error_code << FSCR_IC_POS); + srr0 = SPR_HSRR0; + srr1 = SPR_HSRR1; + new_msr |= (target_ulong)MSR_HVB; + new_msr |= env->msr & ((target_ulong)1 << MSR_RI); #endif break; case POWERPC_EXCP_PIT: /* Programmable interval timer interrupt */ @@ -1287,6 +1296,8 @@ void helper_book3s_msgclrp(CPUPPCState *env, target_ulong rb) { int irq = book3s_dbell2irq(rb, false); + helper_hfscr_facility_check(env, HFSCR_MSGP, "msgclrp", HFSCR_IC_MSGP); + if (irq < 0) { return; } @@ -1303,6 +1314,8 @@ void helper_book3s_msgsndp(CPUPPCState *env, target_ulong rb) int irq = book3s_dbell2irq(rb, false); int pir = env->spr_cb[SPR_PIR].default_value; + helper_hfscr_facility_check(env, HFSCR_MSGP, "msgsndp", HFSCR_IC_MSGP); + if (irq < 0) { return; } diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c index 66b5b0824208..2ff6bed10228 100644 --- a/target/ppc/misc_helper.c +++ b/target/ppc/misc_helper.c @@ -41,6 +41,18 @@ void helper_store_dump_spr(CPUPPCState *env, uint32_t sprn) } #ifdef TARGET_PPC64 +static void raise_hv_fu_exception(CPUPPCState *env, uint32_t bit, + const char *caller, uint32_t cause, + uintptr_t raddr) +{ + qemu_log_mask(LOG_GUEST_ERROR, "HV Facility %d is unavailable (%s)\n", + bit, caller); + + env->spr[SPR_HFSCR] &= ~((target_ulong)FSCR_IC_MASK << FSCR_IC_POS); + + raise_exception_err_ra(env, POWERPC_EXCP_HV_FU, cause, raddr); +} + static void raise_fu_exception(CPUPPCState *env, uint32_t bit, uint32_t sprn, uint32_t cause, uintptr_t raddr) @@ -55,6 +67,17 @@ static void raise_fu_exception(CPUPPCState *env, uint32_t bit, } #endif +void helper_hfscr_facility_check(CPUPPCState *env, uint32_t bit, + const char *caller, uint32_t cause) +{ +#ifdef TARGET_PPC64 + if ((env->msr_mask & MSR_HVB) && !msr_hv && + !(env->spr[SPR_HFSCR] & (1UL << bit))) { + raise_hv_fu_exception(env, bit, caller, cause, GETPC()); + } +#endif +} + void helper_fscr_facility_check(CPUPPCState *env, uint32_t bit, uint32_t sprn, uint32_t cause) { @@ -114,6 +137,8 @@ target_ulong helper_load_dpdes(CPUPPCState *env) { target_ulong dpdes = 0; + helper_hfscr_facility_check(env, HFSCR_MSGP, "load DPDES", HFSCR_IC_MSGP); + /* TODO: TCG supports only one thread */ if (env->pending_interrupts & (1 << PPC_INTERRUPT_DOORBELL)) { dpdes |= (env->spr_cb[SPR_PIR].default_value & DBELL_TIRTAG_MASK); @@ -127,6 +152,8 @@ void helper_store_dpdes(CPUPPCState *env, target_ulong val) PowerPCCPU *cpu = env_archcpu(env); CPUState *cs = CPU(cpu); + helper_hfscr_facility_check(env, HFSCR_MSGP, "store DPDES", HFSCR_IC_MSGP); + /* TODO: TCG supports only one thread */ if (val & ~0x1) { qemu_log_mask(LOG_GUEST_ERROR, "Invalid DPDES register value " -- 2.21.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] target/ppc: add support for Hypervisor Facility Unavailable Exception 2020-01-09 16:33 ` [PATCH 2/2] target/ppc: add support for Hypervisor Facility Unavailable Exception Cédric Le Goater @ 2020-01-17 9:49 ` David Gibson 0 siblings, 0 replies; 6+ messages in thread From: David Gibson @ 2020-01-17 9:49 UTC (permalink / raw) To: Cédric Le Goater Cc: qemu-ppc, Greg Kurz, Suraj Jitindar Singh, qemu-devel [-- Attachment #1: Type: text/plain, Size: 6269 bytes --] On Thu, Jan 09, 2020 at 05:33:46PM +0100, Cédric Le Goater wrote: > The privileged message send and clear instructions (msgsndp & msgclrp) > are privileged, but will generate a hypervisor facility unavailable > exception if not enabled in the HFSCR and executed in privileged > non-hypervisor state. > > Add checks when accessing the DPDES register and when using the > msgsndp and msgclrp isntructions. > > Based on previous work from Suraj Jitindar Singh. > > Cc: Suraj Jitindar Singh <sjitindarsingh@gmail.com> > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > target/ppc/cpu.h | 6 ++++++ > target/ppc/excp_helper.c | 13 +++++++++++++ > target/ppc/misc_helper.c | 27 +++++++++++++++++++++++++++ > 3 files changed, 46 insertions(+) > > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > index d175ec9a641d..1ff6afbccdb2 100644 > --- a/target/ppc/cpu.h > +++ b/target/ppc/cpu.h > @@ -397,6 +397,10 @@ typedef struct ppc_v3_pate_t { > #define PSSCR_ESL PPC_BIT(42) /* Enable State Loss */ > #define PSSCR_EC PPC_BIT(43) /* Exit Criterion */ > > +/* HFSCR bits */ > +#define HFSCR_MSGP PPC_BIT(53) /* Privileged Message Send Facilities */ > +#define HFSCR_IC_MSGP 0xA > + > #define msr_sf ((env->msr >> MSR_SF) & 1) > #define msr_isf ((env->msr >> MSR_ISF) & 1) > #define msr_shv ((env->msr >> MSR_SHV) & 1) > @@ -1332,6 +1336,8 @@ void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp); > #endif > > void store_fpscr(CPUPPCState *env, uint64_t arg, uint32_t mask); > +void helper_hfscr_facility_check(CPUPPCState *env, uint32_t bit, > + const char *caller, uint32_t cause); > > static inline uint64_t ppc_dump_gpr(CPUPPCState *env, int gprn) > { > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > index 343f3a6b30c4..3887f8888c6c 100644 > --- a/target/ppc/excp_helper.c > +++ b/target/ppc/excp_helper.c > @@ -471,6 +471,15 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) > case POWERPC_EXCP_FU: /* Facility unavailable exception */ > #ifdef TARGET_PPC64 > env->spr[SPR_FSCR] |= ((target_ulong)env->error_code << 56); > +#endif > + break; > + case POWERPC_EXCP_HV_FU: /* Hypervisor Facility Unavailable Exception */ > +#ifdef TARGET_PPC64 > + env->spr[SPR_HFSCR] |= ((target_ulong)env->error_code << FSCR_IC_POS); > + srr0 = SPR_HSRR0; > + srr1 = SPR_HSRR1; > + new_msr |= (target_ulong)MSR_HVB; > + new_msr |= env->msr & ((target_ulong)1 << MSR_RI); > #endif > break; > case POWERPC_EXCP_PIT: /* Programmable interval timer interrupt */ > @@ -1287,6 +1296,8 @@ void helper_book3s_msgclrp(CPUPPCState *env, target_ulong rb) > { > int irq = book3s_dbell2irq(rb, false); > > + helper_hfscr_facility_check(env, HFSCR_MSGP, "msgclrp", HFSCR_IC_MSGP); > + > if (irq < 0) { > return; > } > @@ -1303,6 +1314,8 @@ void helper_book3s_msgsndp(CPUPPCState *env, target_ulong rb) > int irq = book3s_dbell2irq(rb, false); > int pir = env->spr_cb[SPR_PIR].default_value; > > + helper_hfscr_facility_check(env, HFSCR_MSGP, "msgsndp", HFSCR_IC_MSGP); > + > if (irq < 0) { > return; > } > diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c > index 66b5b0824208..2ff6bed10228 100644 > --- a/target/ppc/misc_helper.c > +++ b/target/ppc/misc_helper.c > @@ -41,6 +41,18 @@ void helper_store_dump_spr(CPUPPCState *env, uint32_t sprn) > } > > #ifdef TARGET_PPC64 > +static void raise_hv_fu_exception(CPUPPCState *env, uint32_t bit, > + const char *caller, uint32_t cause, > + uintptr_t raddr) > +{ > + qemu_log_mask(LOG_GUEST_ERROR, "HV Facility %d is unavailable (%s)\n", > + bit, caller); If we're using pnv and the emulated hypervisor is using HFSCR for trap-and-emulate or lazy loading or something, then an HV_FU trap doesn't necessarily indicate an error in the guest, so I'm not sure that's the right log mask. Maybe just a trace event here? > + env->spr[SPR_HFSCR] &= ~((target_ulong)FSCR_IC_MASK << FSCR_IC_POS); > + > + raise_exception_err_ra(env, POWERPC_EXCP_HV_FU, cause, raddr); > +} > + > static void raise_fu_exception(CPUPPCState *env, uint32_t bit, > uint32_t sprn, uint32_t cause, > uintptr_t raddr) > @@ -55,6 +67,17 @@ static void raise_fu_exception(CPUPPCState *env, uint32_t bit, > } > #endif > > +void helper_hfscr_facility_check(CPUPPCState *env, uint32_t bit, > + const char *caller, uint32_t cause) > +{ > +#ifdef TARGET_PPC64 > + if ((env->msr_mask & MSR_HVB) && !msr_hv && > + !(env->spr[SPR_HFSCR] & (1UL << bit))) { > + raise_hv_fu_exception(env, bit, caller, cause, GETPC()); > + } > +#endif > +} > + > void helper_fscr_facility_check(CPUPPCState *env, uint32_t bit, > uint32_t sprn, uint32_t cause) > { > @@ -114,6 +137,8 @@ target_ulong helper_load_dpdes(CPUPPCState *env) > { > target_ulong dpdes = 0; > > + helper_hfscr_facility_check(env, HFSCR_MSGP, "load DPDES", HFSCR_IC_MSGP); > + > /* TODO: TCG supports only one thread */ > if (env->pending_interrupts & (1 << PPC_INTERRUPT_DOORBELL)) { > dpdes |= (env->spr_cb[SPR_PIR].default_value & DBELL_TIRTAG_MASK); > @@ -127,6 +152,8 @@ void helper_store_dpdes(CPUPPCState *env, target_ulong val) > PowerPCCPU *cpu = env_archcpu(env); > CPUState *cs = CPU(cpu); > > + helper_hfscr_facility_check(env, HFSCR_MSGP, "store DPDES", HFSCR_IC_MSGP); > + > /* TODO: TCG supports only one thread */ > if (val & ~0x1) { > qemu_log_mask(LOG_GUEST_ERROR, "Invalid DPDES register value " -- 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] 6+ messages in thread
end of thread, other threads:[~2020-01-17 12:44 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-01-09 16:33 [PATCH 0/2] ppc: add support for Directed Privileged Doorbell (non-hypervisor) Cédric Le Goater 2020-01-09 16:33 ` [PATCH 1/2] target/ppc: Add privileged message send facilities Cédric Le Goater 2020-01-17 9:46 ` David Gibson 2020-01-17 12:42 ` Cédric Le Goater 2020-01-09 16:33 ` [PATCH 2/2] target/ppc: add support for Hypervisor Facility Unavailable Exception Cédric Le Goater 2020-01-17 9:49 ` David Gibson
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).