[1/5] powerpc/pseries: do not use msgsndp doorbells on POWER9 guests

POWER9 hypervisors will not necessarily run guest threads together on
the same core at the same time, so msgsndp should not be used.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/platforms/pseries/smp.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

On Fri, 2017-04-07 at 22:55 +1000, Nicholas Piggin wrote:
> POWER9 hypervisors will not necessarily run guest threads together on
> the same core at the same time, so msgsndp should not be used.

Maybe we shouldn't advertise doorbells at all ?

> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/platforms/pseries/smp.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/smp.c
> b/arch/powerpc/platforms/pseries/smp.c
> index f6f83aeccaaa..1fa08155206b 100644
> --- a/arch/powerpc/platforms/pseries/smp.c
> +++ b/arch/powerpc/platforms/pseries/smp.c
> @@ -200,7 +200,12 @@ static __init void pSeries_smp_probe(void)
>  {
>  	xics_smp_probe();
>  
> -	if (cpu_has_feature(CPU_FTR_DBELL)) {
> +	/*
> +	 * POWER9 can not use msgsndp doorbells for IPI because
> thread
> +	 * siblings do not necessarily run on physical cores at the
> same
> +	 * time. This could be enabled for pHyp.
> +	 */
> +	if (cpu_has_feature(CPU_FTR_DBELL) &&
> !cpu_has_feature(CPU_FTR_ARCH_300)) {
>  		xics_cause_ipi = smp_ops->cause_ipi;
>  		smp_ops->cause_ipi = pSeries_cause_ipi_mux;
>  	}

On Sun, 09 Apr 2017 18:03:35 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Fri, 2017-04-07 at 22:55 +1000, Nicholas Piggin wrote:
> > POWER9 hypervisors will not necessarily run guest threads together on
> > the same core at the same time, so msgsndp should not be used.  
> 
> Maybe we shouldn't advertise doorbells at all ?

Possibly not. I don't know if there is a good way to switch it off
in the guest from the host with device tree currently. Might be better
to clear it early in the guest rather than test it here though?

Thanks,
Nick

On Mon, 2017-04-10 at 13:22 +1000, Nicholas Piggin wrote:
> On Sun, 09 Apr 2017 18:03:35 +1000
> > Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> 
> > On Fri, 2017-04-07 at 22:55 +1000, Nicholas Piggin wrote:
> > > POWER9 hypervisors will not necessarily run guest threads together on
> > > the same core at the same time, so msgsndp should not be used.  
> > 
> > Maybe we shouldn't advertise doorbells at all ?
> 
> Possibly not. I don't know if there is a good way to switch it off
> in the guest from the host with device tree currently. Might be better
> to clear it early in the guest rather than test it here though?

Sure for now. Long term a separate DT entry would be good. Might be
worth re-enabling one day if we have HW virtualization of geust
doorbells.

Cheers,
Ben

On Mon, 10 Apr 2017 14:07:38 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Mon, 2017-04-10 at 13:22 +1000, Nicholas Piggin wrote:
> > On Sun, 09 Apr 2017 18:03:35 +1000  
> > > Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:  
> >   
> > > On Fri, 2017-04-07 at 22:55 +1000, Nicholas Piggin wrote:  
> > > > POWER9 hypervisors will not necessarily run guest threads together on
> > > > the same core at the same time, so msgsndp should not be used.    
> > > 
> > > Maybe we shouldn't advertise doorbells at all ?  
> > 
> > Possibly not. I don't know if there is a good way to switch it off
> > in the guest from the host with device tree currently. Might be better
> > to clear it early in the guest rather than test it here though?  
> 
> Sure for now. Long term a separate DT entry would be good.

cpufeatures will do that, but I wonder if some hypervisors won't implement
that binding then might be worth getting into an existing one in PAPR.

Anyway for now I'll make the change and have P9 guest clear the feature
themselves if you like.

> Might be
> worth re-enabling one day if we have HW virtualization of geust
> doorbells.

Yes. Also some hypervisor might run in a virtual sibling = physical
sibling mode on P9 too where it could still work.

Thanks,
Nick

Nicholas Piggin <npiggin@gmail.com> writes:

> POWER9 hypervisors will not necessarily run guest threads together on
> the same core at the same time, so msgsndp should not be used.

I'm worried this is encoding the behaviour of a particular hypervisor in
the guest kernel.

If we *can't* use msgsndp then the hypervisor better do something to
stop us from using it.

If it would be preferable for us not to use msgsndp, then the hypervisor
can tell us that somehow, eg. in the device tree.

?

cheers

cc'ing Paul

On Tue, 11 Apr 2017 20:10:17 +1000
Michael Ellerman <mpe@ellerman.id.au> wrote:

> Nicholas Piggin <npiggin@gmail.com> writes:
> 
> > POWER9 hypervisors will not necessarily run guest threads together on
> > the same core at the same time, so msgsndp should not be used.  
> 
> I'm worried this is encoding the behaviour of a particular hypervisor in
> the guest kernel.

Yeah, it's not ideal.

> If we *can't* use msgsndp then the hypervisor better do something to
> stop us from using it.

POWER9 hypervisor has an hfscr and should clear that if it does not gang
threads like POWER8. The guest still needs to know not to use it though...

> If it would be preferable for us not to use msgsndp, then the hypervisor
> can tell us that somehow, eg. in the device tree.

I don't know that we have a really good way to do that other than guests
to clear the doorbell feature for POWER9.

Does the hypervisor set any relevant DT we can use today that says virtual
sibling != physical sibling? If not, then we'll just have to clear it from
all POWER9 guests until we get a DT proprety from phyp.

Thanks,
Nick