[PATCH 0/3] Setup RFI flush after PowerVM LPM migration

[PATCH 0/3] Setup RFI flush after PowerVM LPM migration

[Mauricio Faria de Oliveira]

This patchset allows for setup_rfi_flush() to be called again
after PowerVM LPM (live partition mobility) aka LPAR migration.

It's originally written by Michael Ellerman; I just rebased it
on top of powerpc/merge as of 2018-02-14 BRST (commit 3f6f556).

I tested it with a debug patch (sending shortly) to shortcut
the LPM functions migration_store() and post_mobility_fixup()
to just reach pseries_setup_rfi_flush(), and abuse the boot
option 'no_rfi_flush' not to allocate the fallback flush area
at boot (to simulate the migration from patched to unpatched
system).

Testing:
-------

Fallback flush area allocated at boot time:

    # dmesg | grep rfi-flush
    [    0.000000] rfi-flush: Using fallback displacement flush
    [    0.000000] rfi-flush: patched 8 locations

    # echo > /sys/kernel/mobility/migration 

    # dmesg | grep rfi-flush
    [    0.000000] rfi-flush: Using fallback displacement flush
    [    0.000000] rfi-flush: patched 8 locations
    [   51.793238] rfi-flush: Using fallback displacement flush
    [   51.793258] rfi-flush: patched 8 locations

Fallback flush area NOT allocated at boot time:

    # grep -o no_rfi_flush /proc/cmdline
    no_rfi_flush
    
    # dmesg | grep rfi-flush
    [    0.000000] rfi-flush: disabled on command line.
    [    0.000000] rfi-flush: re-enabled

    # echo > /sys/kernel/mobility/migration 

    # dmesg | grep rfi-flush
    [    0.000000] rfi-flush: disabled on command line.
    [    0.000000] rfi-flush: re-enabled
    [   31.817921] rfi-flush: Error unable to use fallback displacement flush!
    [   31.817927] rfi-flush: patched 8 locations

Michael Ellerman (3):
  rfi-flush: Move the logic to avoid a redo into the debugfs code
  rfi-flush: Make it possible to call setup_rfi_flush() again
  rfi-flush: Call setup_rfi_flush() after LPM migration

 arch/powerpc/include/asm/setup.h          |  2 +-
 arch/powerpc/kernel/setup_64.c            | 38 +++++++++++++++++++++++--------
 arch/powerpc/platforms/pseries/mobility.c |  3 +++
 arch/powerpc/platforms/pseries/pseries.h  |  2 ++
 arch/powerpc/platforms/pseries/setup.c    |  2 +-
 5 files changed, 36 insertions(+), 11 deletions(-)

-- 
2.7.4

[PATCH 1/3] rfi-flush: Move the logic to avoid a redo into the debugfs code

[Mauricio Faria de Oliveira]

From: Michael Ellerman <mpe@ellerman.id.au>

rfi_flush_enable() includes a check to see if we're already
enabled (or disabled), and in that case does nothing.

But that means calling setup_rfi_flush() a 2nd time doesn't actually
work, which is a bit confusing.

Move that check into the debugfs code, where it really belongs.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/setup_64.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index c388cc3..3efc01a 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -846,9 +846,6 @@ static void do_nothing(void *unused)
 
 void rfi_flush_enable(bool enable)
 {
-	if (rfi_flush == enable)
-		return;
-
 	if (enable) {
 		do_rfi_flush_fixups(enabled_flush_types);
 		on_each_cpu(do_nothing, NULL, 1);
@@ -902,13 +899,19 @@ void __init setup_rfi_flush(enum l1d_flush_type types, bool enable)
 #ifdef CONFIG_DEBUG_FS
 static int rfi_flush_set(void *data, u64 val)
 {
+	bool enable;
+
 	if (val == 1)
-		rfi_flush_enable(true);
+		enable = true;
 	else if (val == 0)
-		rfi_flush_enable(false);
+		enable = false;
 	else
 		return -EINVAL;
 
+	/* Only do anything if we're changing state */
+	if (enable != rfi_flush)
+		rfi_flush_enable(enable);
+
 	return 0;
 }
 
-- 
2.7.4

[PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again

[Mauricio Faria de Oliveira]

From: Michael Ellerman <mpe@ellerman.id.au>

For PowerVM migration we want to be able to call setup_rfi_flush()
again after we've migrated the partition.

To support that we need to check that we're not trying to allocate the
fallback flush area after memblock has gone away. If so we just fail,
we don't support migrating from a patched to an unpatched machine. Or
we do support it, but there will be no RFI flush enabled on the
destination.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/setup.h |  2 +-
 arch/powerpc/kernel/setup_64.c   | 25 +++++++++++++++++++++----
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/setup.h b/arch/powerpc/include/asm/setup.h
index 469b7fd..bbcdf929 100644
--- a/arch/powerpc/include/asm/setup.h
+++ b/arch/powerpc/include/asm/setup.h
@@ -49,7 +49,7 @@ enum l1d_flush_type {
 	L1D_FLUSH_MTTRIG	= 0x8,
 };
 
-void __init setup_rfi_flush(enum l1d_flush_type, bool enable);
+void setup_rfi_flush(enum l1d_flush_type, bool enable);
 void do_rfi_flush_fixups(enum l1d_flush_type types);
 
 #endif /* !__ASSEMBLY__ */
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 3efc01a..d692f71 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -855,11 +855,22 @@ void rfi_flush_enable(bool enable)
 	rfi_flush = enable;
 }
 
-static void init_fallback_flush(void)
+static bool init_fallback_flush(void)
 {
 	u64 l1d_size, limit;
 	int cpu;
 
+	if (l1d_flush_fallback_area)
+		return true;
+
+	/*
+	 * Once the slab allocator is up it's too late to allocate the fallback
+	 * flush area, so return an error. This could happen if we migrated from
+	 * a patched machine to an unpatched machine.
+	 */
+	if (slab_is_available())
+		return false;
+
 	l1d_size = ppc64_caches.l1d.size;
 	limit = min(ppc64_bolted_size(), ppc64_rma_size);
 
@@ -875,13 +886,19 @@ static void init_fallback_flush(void)
 		paca[cpu].rfi_flush_fallback_area = l1d_flush_fallback_area;
 		paca[cpu].l1d_flush_size = l1d_size;
 	}
+
+	return true;
 }
 
-void __init setup_rfi_flush(enum l1d_flush_type types, bool enable)
+void setup_rfi_flush(enum l1d_flush_type types, bool enable)
 {
 	if (types & L1D_FLUSH_FALLBACK) {
-		pr_info("rfi-flush: Using fallback displacement flush\n");
-		init_fallback_flush();
+		if (init_fallback_flush())
+			pr_info("rfi-flush: Using fallback displacement flush\n");
+		else {
+			pr_warn("rfi-flush: Error unable to use fallback displacement flush!\n");
+			types &= ~L1D_FLUSH_FALLBACK;
+		}
 	}
 
 	if (types & L1D_FLUSH_ORI)
-- 
2.7.4

Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again

[Michal Suchánek]

Hello,

On Wed, 14 Feb 2018 19:33:40 -0200
Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> wrote:

> From: Michael Ellerman <mpe@ellerman.id.au>
> 
> For PowerVM migration we want to be able to call setup_rfi_flush()
> again after we've migrated the partition.
> 
> To support that we need to check that we're not trying to allocate the
> fallback flush area after memblock has gone away. If so we just fail,
> we don't support migrating from a patched to an unpatched machine. Or
> we do support it, but there will be no RFI flush enabled on the
> destination.
> 

This sounds bad to me. Either we support RFI flush or we don't.

If we do the fallback area should be allocated at boot so it is always
available. The user can use nopti to disable all RFI flush and then it
is OK to not allocate it.

Thanks

Michal

Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again

[Mauricio Faria de Oliveira]

Hi Michal and Michael,

On 02/15/2018 05:13 AM, Michal Suchánek wrote:
>> From: Michael Ellerman<mpe@ellerman.id.au>
>>
>> For PowerVM migration we want to be able to call setup_rfi_flush()
>> again after we've migrated the partition.
>>
>> To support that we need to check that we're not trying to allocate the
>> fallback flush area after memblock has gone away. If so we just fail,
>> we don't support migrating from a patched to an unpatched machine. Or
>> we do support it, but there will be no RFI flush enabled on the
>> destination.
>>
> This sounds bad to me. Either we support RFI flush or we don't.
> 
> If we do the fallback area should be allocated at boot so it is always
> available. [snip]

I think the problem with this is the size of the fallback area might
have to be different between the origin and destination systems, say,
a larger L1 data cache at the destination.

In that case, the original size might not be enough to fully flush
the L1 data cache.

Michael, is that the reason it is done that way?  I thought of that,
but don't know for sure.

Thanks!
Mauricio

Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again

[Michael Ellerman]

Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> writes:

> Hi Michal and Michael,
>
> On 02/15/2018 05:13 AM, Michal Such=C3=A1nek wrote:
>>> From: Michael Ellerman<mpe@ellerman.id.au>
>>>
>>> For PowerVM migration we want to be able to call setup_rfi_flush()
>>> again after we've migrated the partition.
>>>
>>> To support that we need to check that we're not trying to allocate the
>>> fallback flush area after memblock has gone away. If so we just fail,
>>> we don't support migrating from a patched to an unpatched machine. Or
>>> we do support it, but there will be no RFI flush enabled on the
>>> destination.
>>>
>> This sounds bad to me. Either we support RFI flush or we don't.
>>=20
>> If we do the fallback area should be allocated at boot so it is always
>> available. [snip]
>
> I think the problem with this is the size of the fallback area might
> have to be different between the origin and destination systems, say,
> a larger L1 data cache at the destination.
>
> In that case, the original size might not be enough to fully flush
> the L1 data cache.
>
> Michael, is that the reason it is done that way?  I thought of that,
> but don't know for sure.

No, supporting different cache sizes is a good idea though :)

I did it the way I did because otherwise we waste memory on every system
on earth just to support a use case that we don't actually intend for
anyone to ever use - ie. migrating from a patched machine to an
unpatched machine.

In fact without further checks we'd be allocating the fallback area on
powernv machines which don't even support LPM.

So that just seemed a bit gross.

I think I'm inclined to leave it the way it is, unless you feel strongly
about it Michal?

cheers

Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again

[Michal Suchánek]

On Tue, 20 Feb 2018 20:59:18 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:

> Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> writes:
>=20
> > Hi Michal and Michael,
> >
> > On 02/15/2018 05:13 AM, Michal Such=C3=A1nek wrote: =20
> >>> From: Michael Ellerman<mpe@ellerman.id.au>
> >>>
> >>> For PowerVM migration we want to be able to call setup_rfi_flush()
> >>> again after we've migrated the partition.
> >>>
> >>> To support that we need to check that we're not trying to
> >>> allocate the fallback flush area after memblock has gone away. If
> >>> so we just fail, we don't support migrating from a patched to an
> >>> unpatched machine. Or we do support it, but there will be no RFI
> >>> flush enabled on the destination.
> >>> =20
> >> This sounds bad to me. Either we support RFI flush or we don't.
> >>=20
> >> If we do the fallback area should be allocated at boot so it is
> >> always available. [snip] =20
> >
> > I think the problem with this is the size of the fallback area might
> > have to be different between the origin and destination systems,
> > say, a larger L1 data cache at the destination.
> >
> > In that case, the original size might not be enough to fully flush
> > the L1 data cache.
> >
> > Michael, is that the reason it is done that way?  I thought of that,
> > but don't know for sure. =20
>=20
> No, supporting different cache sizes is a good idea though :)
>=20
> I did it the way I did because otherwise we waste memory on every
> system on earth just to support a use case that we don't actually
> intend for anyone to ever use - ie. migrating from a patched machine
> to an unpatched machine.

If you have multiple hosts running some LPMs and want to update them
without shutting down the whole thing I suppose it might easily happen
that a machine (re)started on a patched host is migrated to unpatched
host.

>=20
> In fact without further checks we'd be allocating the fallback area on
> powernv machines which don't even support LPM.

They support suspend to disk which basically amounts to the same thing.
Downgrading the firmware while in sleep does not sound like a good
idea, though.

>=20
> So that just seemed a bit gross.
>=20
> I think I'm inclined to leave it the way it is, unless you feel
> strongly about it Michal?

I think it would be more user friendly to either support the fallback
method 100% or remove it and require patched firmware.=20

Thanks

Michal

Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again

[Mauricio Faria de Oliveira]

Hi Michael, Michal,

I got back from vacation. Checking this one.

On 02/20/2018 02:06 PM, Michal Suchánek wrote:
>> I did it the way I did because otherwise we waste memory on every
>> system on earth just to support a use case that we don't actually
>> intend for anyone to ever use - ie. migrating from a patched machine
>> to an unpatched machine.

If this thread eventually closes in 'ok, so that memory has to be
reserved/wasted anyway', that can be done only in pseries, right?

It seems not so much memory for this particular platform/hardware.

> If you have multiple hosts running some LPMs and want to update them
> without shutting down the whole thing I suppose it might easily happen
> that a machine (re)started on a patched host is migrated to unpatched
> host.

Right, but that should be temporary, I think -- after updating some of
the hosts, the LPAR(s) can be migrated back to one of them, where the
fallback flush is not required anymore.

>> I think I'm inclined to leave it the way it is, unless you feel
>> strongly about it Michal?

> I think it would be more user friendly to either support the fallback
> method 100% or remove it and require patched firmware.

I beg to disagree.  Since the matter is a security issue, the option
of still have some sort of fix that works on unpatched firmware does
look good and friendly to users (rather than require 'you _must_ get
the firmware update') IMHO.

cheers,
Mauricio

Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again

[Michal Suchánek]

On Mon, 5 Mar 2018 19:46:12 -0300
Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> wrote:

> Hi Michael, Michal,
>=20
> I got back from vacation. Checking this one.
>=20
> On 02/20/2018 02:06 PM, Michal Such=C3=A1nek wrote:
> >> I did it the way I did because otherwise we waste memory on every
> >> system on earth just to support a use case that we don't actually
> >> intend for anyone to ever use - ie. migrating from a patched
> >> machine to an unpatched machine. =20
>=20
> If this thread eventually closes in 'ok, so that memory has to be
> reserved/wasted anyway', that can be done only in pseries, right?
>=20
> It seems not so much memory for this particular platform/hardware.

Yes, that is what I think too.

>=20
> > If you have multiple hosts running some LPMs and want to update them
> > without shutting down the whole thing I suppose it might easily
> > happen that a machine (re)started on a patched host is migrated to
> > unpatched host. =20
>=20
> Right, but that should be temporary, I think -- after updating some of
> the hosts, the LPAR(s) can be migrated back to one of them, where the
> fallback flush is not required anymore.

Temporary, yes. But for how long?

Also is it possible to migrate between P8 and P9 in P8 compat mode?

AFAIK on P9 fallback mode is used exclusively so far.

>=20
> >> I think I'm inclined to leave it the way it is, unless you feel
> >> strongly about it Michal? =20
>=20
> > I think it would be more user friendly to either support the
> > fallback method 100% or remove it and require patched firmware. =20
>=20
> I beg to disagree.  Since the matter is a security issue, the option
> of still have some sort of fix that works on unpatched firmware does
> look good and friendly to users (rather than require 'you _must_ get
> the firmware update') IMHO.

Specifically because this is a security issue user friendly is that
user can tell clearly if the fix is applied or not. If the kernel tells
the user the fix is applied and later it tells "sorry, could not
allocate more memory. disabling fix" this is really unfriendly.=20

Thanks

Michal

Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again

[Michael Ellerman]

Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> writes:

> Hi Michael, Michal,
>
> I got back from vacation. Checking this one.

Yeah it got stuck in a rut.

> On 02/20/2018 02:06 PM, Michal Such=C3=A1nek wrote:
>>> I did it the way I did because otherwise we waste memory on every
>>> system on earth just to support a use case that we don't actually
>>> intend for anyone to ever use - ie. migrating from a patched machine
>>> to an unpatched machine.
>
> If this thread eventually closes in 'ok, so that memory has to be
> reserved/wasted anyway', that can be done only in pseries, right?
>
> It seems not so much memory for this particular platform/hardware.

Yes and no. We certainly have pseries machines with 10s of TBs of
memory, but pseries is also used when running on KVM/Openstack where
some people are interested in running small VMs - so as usual it's not
so simple :)

But it's not *that* much memory so we should probably stop worrying
about it and just always allocate the fallback on pseries.

I *think* the patch below is all we need, as well as some tweaking of
patch 2, are you able to test and repost?

cheers

diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platform=
s/pseries/setup.c
index 1a527625acf7..9116824bd7c5 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -468,26 +468,18 @@ static void pseries_setup_rfi_flush(void)
=20
 	/* Enable by default */
 	enable =3D true;
+	types =3D L1D_FLUSH_FALLBACK;
=20
 	rc =3D plpar_get_cpu_characteristics(&result);
 	if (rc =3D=3D H_SUCCESS) {
-		types =3D L1D_FLUSH_NONE;
-
 		if (result.character & H_CPU_CHAR_L1D_FLUSH_TRIG2)
 			types |=3D L1D_FLUSH_MTTRIG;
 		if (result.character & H_CPU_CHAR_L1D_FLUSH_ORI30)
 			types |=3D L1D_FLUSH_ORI;
=20
-		/* Use fallback if nothing set in hcall */
-		if (types =3D=3D L1D_FLUSH_NONE)
-			types =3D L1D_FLUSH_FALLBACK;
-
 		if ((!(result.behaviour & H_CPU_BEHAV_L1D_FLUSH_PR)) ||
 		    (!(result.behaviour & H_CPU_BEHAV_FAVOUR_SECURITY)))
 			enable =3D false;
-	} else {
-		/* Default to fallback if case hcall is not available */
-		types =3D L1D_FLUSH_FALLBACK;
 	}
=20
 	setup_rfi_flush(types, enable);

Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again

[Michal Suchánek]

On Tue, 06 Mar 2018 23:15:45 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:

> Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> writes:
>=20
> > Hi Michael, Michal,
> >
> > I got back from vacation. Checking this one. =20
>=20
> Yeah it got stuck in a rut.
>=20
> > On 02/20/2018 02:06 PM, Michal Such=C3=A1nek wrote: =20
> >>> I did it the way I did because otherwise we waste memory on every
> >>> system on earth just to support a use case that we don't actually
> >>> intend for anyone to ever use - ie. migrating from a patched
> >>> machine to an unpatched machine. =20
> >
> > If this thread eventually closes in 'ok, so that memory has to be
> > reserved/wasted anyway', that can be done only in pseries, right?
> >
> > It seems not so much memory for this particular platform/hardware. =20
>=20
> Yes and no. We certainly have pseries machines with 10s of TBs of
> memory, but pseries is also used when running on KVM/Openstack where
> some people are interested in running small VMs - so as usual it's not
> so simple :)
>=20
> But it's not *that* much memory so we should probably stop worrying
> about it and just always allocate the fallback on pseries.
>=20
> I *think* the patch below is all we need, as well as some tweaking of
> patch 2, are you able to test and repost?
>=20
> cheers
>=20
> diff --git a/arch/powerpc/platforms/pseries/setup.c
> b/arch/powerpc/platforms/pseries/setup.c index
> 1a527625acf7..9116824bd7c5 100644 ---
> a/arch/powerpc/platforms/pseries/setup.c +++
> b/arch/powerpc/platforms/pseries/setup.c @@ -468,26 +468,18 @@ static
> void pseries_setup_rfi_flush(void)=20
>  	/* Enable by default */
>  	enable =3D true;
> +	types =3D L1D_FLUSH_FALLBACK;
> =20
>  	rc =3D plpar_get_cpu_characteristics(&result);
>  	if (rc =3D=3D H_SUCCESS) {
> -		types =3D L1D_FLUSH_NONE;
> -
>  		if (result.character & H_CPU_CHAR_L1D_FLUSH_TRIG2)
>  			types |=3D L1D_FLUSH_MTTRIG;
>  		if (result.character & H_CPU_CHAR_L1D_FLUSH_ORI30)
>  			types |=3D L1D_FLUSH_ORI;
> =20
> -		/* Use fallback if nothing set in hcall */
> -		if (types =3D=3D L1D_FLUSH_NONE)
> -			types =3D L1D_FLUSH_FALLBACK;
> -
>  		if ((!(result.behaviour & H_CPU_BEHAV_L1D_FLUSH_PR))
> || (!(result.behaviour & H_CPU_BEHAV_FAVOUR_SECURITY)))
>  			enable =3D false;
> -	} else {
> -		/* Default to fallback if case hcall is not
> available */
> -		types =3D L1D_FLUSH_FALLBACK;
>  	}
> =20
>  	setup_rfi_flush(types, enable);
>=20

Enabling the fallback flush always looks a bit dodgy but
do_rfi_flush_fixups will overwrite the jump so long any other fixup is
enabled.

Reviewed-by: Michal Such=C3=A1nek <msuchanek@suse.de>

Thanks

Michal

Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again

[Mauricio Faria de Oliveira]

Hi Michael and Michal,

Got back to this; sorry for the delay.

On 03/06/2018 09:55 AM, Michal Suchánek wrote:
> Michael Ellerman<mpe@ellerman.id.au>  wrote:

>> I*think*  the patch below is all we need, as well as some tweaking of
>> patch 2, are you able to test and repost?

> Enabling the fallback flush always looks a bit dodgy but
> do_rfi_flush_fixups will overwrite the jump so long any other fixup is
> enabled.

I agree; the 'Using fallback displacement flush' message is misleading
(is the system slower/fallback or not? Ô_o)

The suggested patch is definitely more contained within pseries, but
the informational part seemed dodgy IMHO too.

So I wrote something with a new function parameter to force the init of
the fallback flush area (true in pseries, false in powernv).  Not that
contained, but it seemed to convey the intent here in a clear way.

That's v2, just sent.

cheers,
Mauricio

Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again

[Michael Ellerman]

Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> writes:

> Hi Michael and Michal,
>
> Got back to this; sorry for the delay.
>
> On 03/06/2018 09:55 AM, Michal Such=C3=A1nek wrote:
>> Michael Ellerman<mpe@ellerman.id.au>  wrote:
>
>>> I*think*  the patch below is all we need, as well as some tweaking of
>>> patch 2, are you able to test and repost?
>
>> Enabling the fallback flush always looks a bit dodgy but
>> do_rfi_flush_fixups will overwrite the jump so long any other fixup is
>> enabled.
>
> I agree; the 'Using fallback displacement flush' message is misleading
> (is the system slower/fallback or not? =C3=94_o)

That message is actually just wrong.

It still prints that even if enable=3Dfalse.

So we should change all those messages, perhaps:

	pr_info("rfi-flush: fallback displacement flush available\n");
	pr_info("rfi-flush: ori type flush available\n");
	pr_info("rfi-flush: mttrig type flush available\n");


> So I wrote something with a new function parameter to force the init of
> the fallback flush area (true in pseries, false in powernv).  Not that
> contained, but it seemed to convey the intent here in a clear way.
>
> That's v2, just sent.

OK thanks. I don't really like it :D - sorry!

It's a lot of plumbing of that bool just to avoid the message, whereas I
think we could just change the message like above.

cheers

Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again

[Mauricio Faria de Oliveira]

Hi Michael,

On 03/13/2018 08:39 AM, Michael Ellerman wrote:
>> I agree; the 'Using fallback displacement flush' message is misleading
>> (is the system slower/fallback or not? Ô_o)

> That message is actually just wrong.
> 
> It still prints that even if enable=false.
> 
> So we should change all those messages, perhaps:
> 
> 	pr_info("rfi-flush: fallback displacement flush available\n");
> 	pr_info("rfi-flush: ori type flush available\n");
> 	pr_info("rfi-flush: mttrig type flush available\n");

Indeed.

>> So I wrote something with a new function parameter to force the init of
>> the fallback flush area (true in pseries, false in powernv).  Not that
>> contained, but it seemed to convey the intent here in a clear way.
>>
>> That's v2, just sent.

> OK thanks. I don't really like it :D - sorry!

No worries :) fair enough. Well, I didn't like it much, either, TBH.

> It's a lot of plumbing of that bool just to avoid the message, whereas I
> think we could just change the message like above.

Yup.

And what you think about a more descriptive confirmation of what flush
instructions/methods are _actually_ being used?

Currently and w/ your suggestion aobve, all that is known is what is
_available_, not what has gone in (or out, in the disable case) the
nop slots.

cheers,
mauricio

Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again

[Michal Suchánek]

On Tue, 13 Mar 2018 09:14:39 -0300
Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> wrote:

> Hi Michael,
>=20
> On 03/13/2018 08:39 AM, Michael Ellerman wrote:
> >> I agree; the 'Using fallback displacement flush' message is
> >> misleading (is the system slower/fallback or not? =C3=94_o) =20
>=20
> > That message is actually just wrong.
> >=20
> > It still prints that even if enable=3Dfalse.
> >=20
> > So we should change all those messages, perhaps:
> >=20
> > 	pr_info("rfi-flush: fallback displacement flush
> > available\n"); pr_info("rfi-flush: ori type flush available\n");
> > 	pr_info("rfi-flush: mttrig type flush available\n"); =20
>=20
> Indeed.

Maybe it would make more sense to move the messages to the function
that actually patches in the instructions?

Thanks

Michal

Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again

[Mauricio Faria de Oliveira]

On 03/13/2018 02:59 PM, Michal Suchánek wrote:
> Maybe it would make more sense to move the messages to the function
> that actually patches in the instructions?

That helps, but if the instructions are not patched (e.g., no_rfi_flush)
then there is no information about what the system actually supports,
which is useful for diagnostics/debugging (and patch verification! :-) )

cheers,
mauricio

Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again

[Michal Suchánek]

On Tue, 13 Mar 2018 15:13:11 -0300
Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> wrote:

> On 03/13/2018 02:59 PM, Michal Such=C3=A1nek wrote:
> > Maybe it would make more sense to move the messages to the function
> > that actually patches in the instructions? =20
>=20
> That helps, but if the instructions are not patched (e.g.,
> no_rfi_flush) then there is no information about what the system
> actually supports, which is useful for diagnostics/debugging (and
> patch verification! :-) )

Can't you patch with debugfs in that case?

Thanks

Michal

Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again

[Mauricio Faria de Oliveira]

On 03/13/2018 03:36 PM, Michal Suchánek wrote:
> On Tue, 13 Mar 2018 15:13:11 -0300
> Mauricio Faria de Oliveira<mauricfo@linux.vnet.ibm.com>  wrote:
> 
>> On 03/13/2018 02:59 PM, Michal Suchánek wrote:
>>> Maybe it would make more sense to move the messages to the function
>>> that actually patches in the instructions?

>> That helps, but if the instructions are not patched (e.g.,
>> no_rfi_flush) then there is no information about what the system
>> actually supports, which is useful for diagnostics/debugging (and
>> patch verification!:-)  )

> Can't you patch with debugfs in that case?

For development purposes, yes, sure; but unfortunately sometimes only a
dmesg output or other offline/postmortem data is available.

And there's the user case where he is not aware/willing/allowed to use
the debugfs switch.

I still think the correct, informative messages are a good way to go :)

cheers,
mauricio

Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again

[Michael Ellerman]

Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> writes:

> On 03/13/2018 03:36 PM, Michal Such=C3=A1nek wrote:
>> On Tue, 13 Mar 2018 15:13:11 -0300
>> Mauricio Faria de Oliveira<mauricfo@linux.vnet.ibm.com>  wrote:
>>=20
>>> On 03/13/2018 02:59 PM, Michal Such=C3=A1nek wrote:
>>>> Maybe it would make more sense to move the messages to the function
>>>> that actually patches in the instructions?
>
>>> That helps, but if the instructions are not patched (e.g.,
>>> no_rfi_flush) then there is no information about what the system
>>> actually supports, which is useful for diagnostics/debugging (and
>>> patch verification!:-)  )
>
>> Can't you patch with debugfs in that case?
>
> For development purposes, yes, sure; but unfortunately sometimes only a
> dmesg output or other offline/postmortem data is available.
>
> And there's the user case where he is not aware/willing/allowed to use
> the debugfs switch.
>
> I still think the correct, informative messages are a good way to go :)

Yeah I agree.

We probably want to do both, print what's available at boot, and print
what's actually patched when the patching happens.

cheers

Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again

[Mauricio Faria de Oliveira]

Michael,

On 03/16/2018 11:18 AM, Michael Ellerman wrote:
>> I still think the correct, informative messages are a good way to go:)
> Yeah I agree.
> 
> We probably want to do both, print what's available at boot, and print
> what's actually patched when the patching happens.

Nice.  Not sure you had a chance to review yet, but 'PATCH v3 4/5' does
exactly that :- )

I think its implementation of the latter part looks a bit strange, but I
couldn't figure an elegant way to fit that in (either that one or string
array indexed by flush-type possible values or a long if-else chain).

I'd be happy with suggestions if it's preferred in some other way.

cheers,
Mauricio

[PATCH 3/3] rfi-flush: Call setup_rfi_flush() after LPM migration

[Mauricio Faria de Oliveira]

From: Michael Ellerman <mpe@ellerman.id.au>

We might have migrated to a machine that uses a different flush type,
or doesn't need flushing at all.

If we migrate to a machine with no flush support, ie. that would use
fallback, we just print an error and switch flushing off. We could
support that, but it would complicate the implementation of the
fallback flush, and we don't expect anyone will ever do it.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/mobility.c | 3 +++
 arch/powerpc/platforms/pseries/pseries.h  | 2 ++
 arch/powerpc/platforms/pseries/setup.c    | 2 +-
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index 0f7fb71..8a8033a 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -348,6 +348,9 @@ void post_mobility_fixup(void)
 		printk(KERN_ERR "Post-mobility device tree update "
 			"failed: %d\n", rc);
 
+	/* Possibly switch to a new RFI flush type */
+	pseries_setup_rfi_flush();
+
 	return;
 }
 
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 1ae1d9f..27cdcb6 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -100,4 +100,6 @@ static inline unsigned long cmo_get_page_size(void)
 
 int dlpar_workqueue_init(void);
 
+void pseries_setup_rfi_flush(void);
+
 #endif /* _PSERIES_PSERIES_H */
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 372d7ad..dad8197 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -459,7 +459,7 @@ static void __init find_and_init_phbs(void)
 	of_pci_check_probe_only();
 }
 
-static void pseries_setup_rfi_flush(void)
+void pseries_setup_rfi_flush(void)
 {
 	struct h_cpu_char_result result;
 	enum l1d_flush_type types;
-- 
2.7.4

[PATCH] DEBUG: shortcut mobility fixup/migration store, and abuse no_rfi_flush

[Mauricio Faria de Oliveira]

Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/setup_64.c            | 8 ++++++++
 arch/powerpc/platforms/pseries/mobility.c | 4 ++++
 2 files changed, 12 insertions(+)

diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index d692f71..a05b9f4 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -892,6 +892,9 @@ static bool init_fallback_flush(void)
 
 void setup_rfi_flush(enum l1d_flush_type types, bool enable)
 {
+	if (no_rfi_flush)
+		types = L1D_FLUSH_NONE;
+
 	if (types & L1D_FLUSH_FALLBACK) {
 		if (init_fallback_flush())
 			pr_info("rfi-flush: Using fallback displacement flush\n");
@@ -911,6 +914,11 @@ void setup_rfi_flush(enum l1d_flush_type types, bool enable)
 
 	if (!no_rfi_flush)
 		rfi_flush_enable(enable);
+
+	if (no_rfi_flush) {
+		pr_info("rfi-flush: re-enabled\n");
+		no_rfi_flush = 0;
+	}
 }
 
 #ifdef CONFIG_DEBUG_FS
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index 8a8033a..201710e 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -326,6 +326,7 @@ int pseries_devicetree_update(s32 scope)
 
 void post_mobility_fixup(void)
 {
+#if 0
 	int rc;
 	int activate_fw_token;
 
@@ -347,6 +348,7 @@ void post_mobility_fixup(void)
 	if (rc)
 		printk(KERN_ERR "Post-mobility device tree update "
 			"failed: %d\n", rc);
+#endif
 
 	/* Possibly switch to a new RFI flush type */
 	pseries_setup_rfi_flush();
@@ -358,6 +360,7 @@ static ssize_t migration_store(struct class *class,
 			       struct class_attribute *attr, const char *buf,
 			       size_t count)
 {
+#if 0
 	u64 streamid;
 	int rc;
 
@@ -373,6 +376,7 @@ static ssize_t migration_store(struct class *class,
 
 	if (rc)
 		return rc;
+#endif
 
 	post_mobility_fixup();
 	return count;
-- 
2.7.4