[PATCH] powerpc/mm: Invalidate partition table cache on host proc tbl base update

The host process table base is stored in the partition table by calling
the function native_register_process_table(). Currently this just sets
the entry in memory and is missing a proceeding cache invalidation
instruction. Any update to the partition table should be followed by a
cache invalidation instruction specifying invalidation of the caching of
any partition table entries (RIC = 2, PRS = 0).

We already have a function to update the partition table with the
required cache invalidation instructions - mmu_partition_table_set_entry().
Update the native_register_process_table() function to call
mmu_partition_table_set_entry(), this ensures all appropriate
invalidation will be performed.

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
 arch/powerpc/mm/pgtable-radix.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/mm/pgtable-radix.c b/arch/powerpc/mm/pgtable-radix.c
index 671a45d..1d5178f 100644
--- a/arch/powerpc/mm/pgtable-radix.c
+++ b/arch/powerpc/mm/pgtable-radix.c

@@ -33,7 +33,8 @@ static int native_register_process_table(unsigned long base, unsigned long pg_sz
 {
 	unsigned long patb1 = base | table_size | PATB_GR;
 
-	partition_tb->patb1 = cpu_to_be64(patb1);
+	mmu_partition_table_set_entry(0, be64_to_cpu(partition_tb->patb0),
+				      patb1);
 	return 0;
 }
 

-- 
2.9.4

Suraj Jitindar Singh [off-list ref] writes:

The host process table base is stored in the partition table by calling
the function native_register_process_table(). Currently this just sets
the entry in memory and is missing a proceeding cache invalidation
instruction. Any update to the partition table should be followed by a
cache invalidation instruction specifying invalidation of the caching of
any partition table entries (RIC = 2, PRS = 0).

We already have a function to update the partition table with the
required cache invalidation instructions - mmu_partition_table_set_entry().
Update the native_register_process_table() function to call
mmu_partition_table_set_entry(), this ensures all appropriate
invalidation will be performed.

Without this patch the kernel will:
 [ ] work normally
 [ ] randomly crash
 [ ] catch fire

?

cheers

On Thu, 2017-08-03 at 16:30 +1000, Michael Ellerman wrote:

Suraj Jitindar Singh [off-list ref] writes:

quoted

The host process table base is stored in the partition table by calling
the function native_register_process_table(). Currently this just sets
the entry in memory and is missing a proceeding cache invalidation
instruction. Any update to the partition table should be followed by a
cache invalidation instruction specifying invalidation of the caching of
any partition table entries (RIC = 2, PRS = 0).

We already have a function to update the partition table with the
required cache invalidation instructions - mmu_partition_table_set_entry().
Update the native_register_process_table() function to call
mmu_partition_table_set_entry(), this ensures all appropriate
invalidation will be performed.

Without this patch the kernel will:
 [ ] work normally
 [ ] randomly crash
 [ ] catch fire

I think we get lucky because OPAL added a "flush the whole world" to
opal_reinit_cpus() but this patch seems to improve general code
"correctness".

Cheers,
Ben.

On Thu, 2017-08-03 at 17:35 +1000, Benjamin Herrenschmidt wrote:

On Thu, 2017-08-03 at 16:30 +1000, Michael Ellerman wrote:

quoted

Suraj Jitindar Singh [off-list ref] writes:

quoted

The host process table base is stored in the partition table by
calling
the function native_register_process_table(). Currently this just
sets
the entry in memory and is missing a proceeding cache
invalidation
instruction. Any update to the partition table should be followed
by a
cache invalidation instruction specifying invalidation of the
caching of
any partition table entries (RIC = 2, PRS = 0).

We already have a function to update the partition table with the
required cache invalidation instructions -
mmu_partition_table_set_entry().
Update the native_register_process_table() function to call
mmu_partition_table_set_entry(), this ensures all appropriate
invalidation will be performed.

Without this patch the kernel will:
 [ ] work normally
 [ ] randomly crash
 [ ] catch fire

I think we get lucky because OPAL added a "flush the whole world" to
opal_reinit_cpus() but this patch seems to improve general code
"correctness".

Cheers,
Ben.

I guess there's the possibility of:
[x] randomly crash

This is required to run a powernv kernel as a guest because we need to
know when it's updated its process table location.

On Fri, 2017-08-04 at 11:02 +1000, Suraj Jitindar Singh wrote:

I guess there's the possibility of:
[x] randomly crash

This is required to run a powernv kernel as a guest because we need to
know when it's updated its process table location.

You mean in qemu full emu ? powernv kernels don't run as guest do they
?

Cheers,
Ben.

On Fri, 2017-08-04 at 11:31 +1000, Benjamin Herrenschmidt wrote:

On Fri, 2017-08-04 at 11:02 +1000, Suraj Jitindar Singh wrote:

quoted

I guess there's the possibility of:
[x] randomly crash

This is required to run a powernv kernel as a guest because we need
to
know when it's updated its process table location.

You mean in qemu full emu ? powernv kernels don't run as guest do
they
?

For now they don't... :p

Cheers,
Ben.

Suraj Jitindar Singh [off-list ref] writes:

On Thu, 2017-08-03 at 17:35 +1000, Benjamin Herrenschmidt wrote:

quoted

On Thu, 2017-08-03 at 16:30 +1000, Michael Ellerman wrote:

quoted

Suraj Jitindar Singh [off-list ref] writes:
=20

quoted

The host process table base is stored in the partition table by
calling
the function native_register_process_table(). Currently this just
sets
the entry in memory and is missing a proceeding cache
invalidation
instruction. Any update to the partition table should be followed
by a
cache invalidation instruction specifying invalidation of the
caching of
any partition table entries (RIC =3D 2, PRS =3D 0).
=20
We already have a function to update the partition table with the
required cache invalidation instructions -
mmu_partition_table_set_entry().
Update the native_register_process_table() function to call
mmu_partition_table_set_entry(), this ensures all appropriate
invalidation will be performed.

=20
Without this patch the kernel will:
=C2=A0[ ] work normally
=C2=A0[ ] randomly crash
=C2=A0[ ] catch fire

=20
I think we get lucky because OPAL added a "flush the whole world" to
opal_reinit_cpus() but this patch seems to improve general code
"correctness".

I guess there's the possibility of:
[x] randomly crash

But you haven't actually see any right?

This is required to run a powernv kernel as a guest because we need to
know when it's updated its process table location.

Which doesn't currently work for other reasons :)

cheers

Suraj Jitindar Singh [off-list ref] writes:

The host process table base is stored in the partition table by calling
the function native_register_process_table(). Currently this just sets
the entry in memory and is missing a proceeding cache invalidation
instruction. Any update to the partition table should be followed by a
cache invalidation instruction specifying invalidation of the caching of
any partition table entries (RIC = 2, PRS = 0).

We already have a function to update the partition table with the
required cache invalidation instructions - mmu_partition_table_set_entry().
Update the native_register_process_table() function to call
mmu_partition_table_set_entry(), this ensures all appropriate
invalidation will be performed.

Reviewed-by: Aneesh Kumar K.V <redacted>

Suraj Jitindar Singh [off-list ref] writes:

This is really a bit gross.

Can we agree on whether partition_tb is an array or not?

How about ...

cheers

diff --git a/arch/powerpc/mm/pgtable-radix.c b/arch/powerpc/mm/pgtable-radix.c
index c1185c8ecb22..5d8be076f8e5 100644
--- a/arch/powerpc/mm/pgtable-radix.c
+++ b/arch/powerpc/mm/pgtable-radix.c

@@ -28,9 +28,13 @@
 static int native_register_process_table(unsigned long base, unsigned long pg_sz,
                                         unsigned long table_size)
 {
-       unsigned long patb1 = base | table_size | PATB_GR;
+       unsigned long patb0, patb1;
+
+       patb0 = be64_to_cpu(partition_tb[0].patb0);
+       patb1 = base | table_size | PATB_GR;
+
+       mmu_partition_table_set_entry(0, patb0, patb1);
 
-       partition_tb->patb1 = cpu_to_be64(patb1);
        return 0;
 }

On Thu, 2017-08-03 at 04:15:51 UTC, Suraj Jitindar Singh wrote:

The host process table base is stored in the partition table by calling
the function native_register_process_table(). Currently this just sets
the entry in memory and is missing a proceeding cache invalidation
instruction. Any update to the partition table should be followed by a
cache invalidation instruction specifying invalidation of the caching of
any partition table entries (RIC = 2, PRS = 0).

We already have a function to update the partition table with the
required cache invalidation instructions - mmu_partition_table_set_entry().
Update the native_register_process_table() function to call
mmu_partition_table_set_entry(), this ensures all appropriate
invalidation will be performed.

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Reviewed-by: Aneesh Kumar K.V <redacted>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/7cd2a8695ef9c31e8f51773f0de9e6

cheers

On Wed, 2017-08-09 at 20:30 +1000, Michael Ellerman wrote:

Suraj Jitindar Singh [off-list ref] writes:

quoted

The host process table base is stored in the partition table by
calling
the function native_register_process_table(). Currently this just
sets
the entry in memory and is missing a proceeding cache invalidation
instruction. Any update to the partition table should be followed
by a
cache invalidation instruction specifying invalidation of the
caching of
any partition table entries (RIC = 2, PRS = 0).

We already have a function to update the partition table with the
required cache invalidation instructions -
mmu_partition_table_set_entry().
Update the native_register_process_table() function to call
mmu_partition_table_set_entry(), this ensures all appropriate
invalidation will be performed.

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
 arch/powerpc/mm/pgtable-radix.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/mm/pgtable-radix.c

b/arch/powerpc/mm/pgtable-radix.c
index 671a45d..1d5178f 100644

--- a/arch/powerpc/mm/pgtable-radix.c
+++ b/arch/powerpc/mm/pgtable-radix.c

@@ -33,7 +33,8 @@ static int native_register_process_table(unsigned

long base, unsigned long pg_sz
 {
 	unsigned long patb1 = base | table_size | PATB_GR;
 
-	partition_tb->patb1 = cpu_to_be64(patb1);
+	mmu_partition_table_set_entry(0, be64_to_cpu(partition_tb-

quoted

patb0),

+				      patb1);

This is really a bit gross.

Can we agree on whether partition_tb is an array or not?

Well it is an array, it's just we only ever want the first element in
this function. That being said we might as well access it as an array
to make that clear.

Looks good :)