| Message ID | 1446162045-26496-3-git-send-email-warrier@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Suresh, On Thu, 2015-29-10 at 23:40:45 UTC, "Suresh E. Warrier" wrote: > This function supports IPI message passing for real > mode callers. > > diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c > index a53a130..8c07bfad 100644 > --- a/arch/powerpc/kernel/smp.c > +++ b/arch/powerpc/kernel/smp.c > @@ -235,6 +238,33 @@ void smp_muxed_ipi_message_pass(int cpu, int msg) > smp_ops->cause_ipi(cpu, info->data); > } > > +#if defined(CONFIG_KVM_XICS) && defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) > +/* > + * Message passing code for real mode callers. It does not use the > + * smp_ops->cause_ipi function to cause an IPI, because those functions > + * access the MFFR through an ioremapped address. > + */ > +void smp_muxed_ipi_rm_message_pass(int cpu, int msg) > +{ > + struct cpu_messages *info = &per_cpu(ipi_message, cpu); > + char *message = (char *)&info->messages; > + unsigned long xics_phys; > + > + /* > + * Order previous accesses before accesses in the IPI handler. > + */ > + smp_mb(); > + message[msg] = 1; > + > + /* > + * cause_ipi functions are required to include a full barrier > + * before doing whatever causes the IPI. > + */ > + xics_phys = paca[cpu].kvm_hstate.xics_phys; > + out_rm8((u8 *)(xics_phys + XICS_MFRR), IPI_PRIORITY); > +} > +#endif I'm not all that happy with this. This function does two things, one of which belongs in this file (setting message), and the other which definitely does not (the XICs part). I think the end result would be cleaner if we did something like: void smp_muxed_ipi_set_message(int cpu, int msg) { struct cpu_messages *info = &per_cpu(ipi_message, cpu); char *message = (char *)&info->messages; unsigned long xics_phys; /* * Order previous accesses before accesses in the IPI handler. */ smp_mb(); message[msg] = 1; } Which would be exported, and could also be used by smp_muxed_ipi_message_pass(). Then in icp_rm_set_vcpu_irq(), you would do something like: if (hcore != -1) { hcpu = hcore << threads_shift; kvmppc_host_rm_ops_hv->rm_core[hcore].rm_data = vcpu; smp_muxed_ipi_set_message(hcpu, PPC_MSG_RM_HOST_ACTION); icp_native_cause_ipi_real_mode(); } Where icp_native_cause_ipi_real_mode() is a new hook you define in icp_native.c which does the real mode write to MFRR. cheers
Hi Mike, The changes you proposed look nicer than what I have here. I will get that coded and tested and re=submit. Thanks. -suresh On 11/15/2015 11:53 PM, Michael Ellerman wrote: > Hi Suresh, > > On Thu, 2015-29-10 at 23:40:45 UTC, "Suresh E. Warrier" wrote: >> This function supports IPI message passing for real >> mode callers. >> >> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c >> index a53a130..8c07bfad 100644 >> --- a/arch/powerpc/kernel/smp.c >> +++ b/arch/powerpc/kernel/smp.c >> @@ -235,6 +238,33 @@ void smp_muxed_ipi_message_pass(int cpu, int msg) >> smp_ops->cause_ipi(cpu, info->data); >> } >> >> +#if defined(CONFIG_KVM_XICS) && defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) >> +/* >> + * Message passing code for real mode callers. It does not use the >> + * smp_ops->cause_ipi function to cause an IPI, because those functions >> + * access the MFFR through an ioremapped address. >> + */ >> +void smp_muxed_ipi_rm_message_pass(int cpu, int msg) >> +{ >> + struct cpu_messages *info = &per_cpu(ipi_message, cpu); >> + char *message = (char *)&info->messages; >> + unsigned long xics_phys; >> + >> + /* >> + * Order previous accesses before accesses in the IPI handler. >> + */ >> + smp_mb(); >> + message[msg] = 1; >> + >> + /* >> + * cause_ipi functions are required to include a full barrier >> + * before doing whatever causes the IPI. >> + */ >> + xics_phys = paca[cpu].kvm_hstate.xics_phys; >> + out_rm8((u8 *)(xics_phys + XICS_MFRR), IPI_PRIORITY); >> +} >> +#endif > > > I'm not all that happy with this. This function does two things, one of which > belongs in this file (setting message), and the other which definitely does > not (the XICs part). > > I think the end result would be cleaner if we did something like: > > void smp_muxed_ipi_set_message(int cpu, int msg) > { > struct cpu_messages *info = &per_cpu(ipi_message, cpu); > char *message = (char *)&info->messages; > unsigned long xics_phys; > > /* > * Order previous accesses before accesses in the IPI handler. > */ > smp_mb(); > message[msg] = 1; > } > > Which would be exported, and could also be used by smp_muxed_ipi_message_pass(). > > Then in icp_rm_set_vcpu_irq(), you would do something like: > > if (hcore != -1) { > hcpu = hcore << threads_shift; > kvmppc_host_rm_ops_hv->rm_core[hcore].rm_data = vcpu; > smp_muxed_ipi_set_message(hcpu, PPC_MSG_RM_HOST_ACTION); > icp_native_cause_ipi_real_mode(); > } > > Where icp_native_cause_ipi_real_mode() is a new hook you define in icp_native.c > which does the real mode write to MFRR. > > cheers >
On Mon, 2015-11-16 at 15:34 -0600, Suresh E. Warrier wrote: > Hi Mike, > > The changes you proposed look nicer than what I have here. > I will get that coded and tested and re=submit. Thanks. cheers
Hi Mike, After looking at this a little more, I think it would perhaps be better to define the real-mode function that causes IPI in book3s_hv_rm_xics.c along with other real-mode functions that operate on the xics. Hope this is acceptable to you. If not, we can discuss when I re-submit the patch. Thanks. -suresh On 11/16/2015 03:34 PM, Suresh E. Warrier wrote: > Hi Mike, > > The changes you proposed look nicer than what I have here. > I will get that coded and tested and re=submit. > > Thanks. > -suresh > > On 11/15/2015 11:53 PM, Michael Ellerman wrote: >> Hi Suresh, >> >> On Thu, 2015-29-10 at 23:40:45 UTC, "Suresh E. Warrier" wrote: >>> This function supports IPI message passing for real >>> mode callers. >>> >>> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c >>> index a53a130..8c07bfad 100644 >>> --- a/arch/powerpc/kernel/smp.c >>> +++ b/arch/powerpc/kernel/smp.c >>> @@ -235,6 +238,33 @@ void smp_muxed_ipi_message_pass(int cpu, int msg) >>> smp_ops->cause_ipi(cpu, info->data); >>> } >>> >>> +#if defined(CONFIG_KVM_XICS) && defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) >>> +/* >>> + * Message passing code for real mode callers. It does not use the >>> + * smp_ops->cause_ipi function to cause an IPI, because those functions >>> + * access the MFFR through an ioremapped address. >>> + */ >>> +void smp_muxed_ipi_rm_message_pass(int cpu, int msg) >>> +{ >>> + struct cpu_messages *info = &per_cpu(ipi_message, cpu); >>> + char *message = (char *)&info->messages; >>> + unsigned long xics_phys; >>> + >>> + /* >>> + * Order previous accesses before accesses in the IPI handler. >>> + */ >>> + smp_mb(); >>> + message[msg] = 1; >>> + >>> + /* >>> + * cause_ipi functions are required to include a full barrier >>> + * before doing whatever causes the IPI. >>> + */ >>> + xics_phys = paca[cpu].kvm_hstate.xics_phys; >>> + out_rm8((u8 *)(xics_phys + XICS_MFRR), IPI_PRIORITY); >>> +} >>> +#endif >> >> >> I'm not all that happy with this. This function does two things, one of which >> belongs in this file (setting message), and the other which definitely does >> not (the XICs part). >> >> I think the end result would be cleaner if we did something like: >> >> void smp_muxed_ipi_set_message(int cpu, int msg) >> { >> struct cpu_messages *info = &per_cpu(ipi_message, cpu); >> char *message = (char *)&info->messages; >> unsigned long xics_phys; >> >> /* >> * Order previous accesses before accesses in the IPI handler. >> */ >> smp_mb(); >> message[msg] = 1; >> } >> >> Which would be exported, and could also be used by smp_muxed_ipi_message_pass(). >> >> Then in icp_rm_set_vcpu_irq(), you would do something like: >> >> if (hcore != -1) { >> hcpu = hcore << threads_shift; >> kvmppc_host_rm_ops_hv->rm_core[hcore].rm_data = vcpu; >> smp_muxed_ipi_set_message(hcpu, PPC_MSG_RM_HOST_ACTION); >> icp_native_cause_ipi_real_mode(); >> } >> >> Where icp_native_cause_ipi_real_mode() is a new hook you define in icp_native.c >> which does the real mode write to MFRR. >> >> cheers >>
diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h index 9ef9c37..851a37a 100644 --- a/arch/powerpc/include/asm/smp.h +++ b/arch/powerpc/include/asm/smp.h @@ -124,6 +124,7 @@ extern const char *smp_ipi_name[]; /* for irq controllers with only a single ipi */ extern void smp_muxed_ipi_set_data(int cpu, unsigned long data); extern void smp_muxed_ipi_message_pass(int cpu, int msg); +extern void smp_muxed_ipi_rm_message_pass(int cpu, int msg); extern irqreturn_t smp_ipi_demux(void); void smp_init_pSeries(void); diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c index a53a130..8c07bfad 100644 --- a/arch/powerpc/kernel/smp.c +++ b/arch/powerpc/kernel/smp.c @@ -53,6 +53,9 @@ #include <asm/vdso.h> #include <asm/debug.h> #include <asm/kexec.h> +#ifdef CONFIG_KVM_XICS +#include <asm/xics.h> +#endif #ifdef DEBUG #include <asm/udbg.h> @@ -235,6 +238,33 @@ void smp_muxed_ipi_message_pass(int cpu, int msg) smp_ops->cause_ipi(cpu, info->data); } +#if defined(CONFIG_KVM_XICS) && defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) +/* + * Message passing code for real mode callers. It does not use the + * smp_ops->cause_ipi function to cause an IPI, because those functions + * access the MFFR through an ioremapped address. + */ +void smp_muxed_ipi_rm_message_pass(int cpu, int msg) +{ + struct cpu_messages *info = &per_cpu(ipi_message, cpu); + char *message = (char *)&info->messages; + unsigned long xics_phys; + + /* + * Order previous accesses before accesses in the IPI handler. + */ + smp_mb(); + message[msg] = 1; + + /* + * cause_ipi functions are required to include a full barrier + * before doing whatever causes the IPI. + */ + xics_phys = paca[cpu].kvm_hstate.xics_phys; + out_rm8((u8 *)(xics_phys + XICS_MFRR), IPI_PRIORITY); +} +#endif + #ifdef __BIG_ENDIAN__ #define IPI_MESSAGE(A) (1uL << ((BITS_PER_LONG - 8) - 8 * (A))) #else
This function supports IPI message passing for real mode callers. Signed-off-by: Suresh Warrier <warrier@linux.vnet.ibm.com> --- arch/powerpc/include/asm/smp.h | 1 + arch/powerpc/kernel/smp.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+)