| Message ID | 20170803041551.7934-1-sjitindarsingh@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 7cd2a8695ef9c31e8f51773f0de9e6007d6dc083 |
| Headers | show |
Suraj Jitindar Singh <sjitindarsingh@gmail.com> 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 <sjitindarsingh@gmail.com> 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 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: > > Suraj Jitindar Singh <sjitindarsingh@gmail.com> 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 > > 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: > > > > 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 <sjitindarsingh@gmail.com> writes: > On Thu, 2017-08-03 at 17:35 +1000, Benjamin Herrenschmidt wrote: >> On Thu, 2017-08-03 at 16:30 +1000, Michael Ellerman wrote: >> > Suraj Jitindar Singh <sjitindarsingh@gmail.com> 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 >> >> 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 <sjitindarsingh@gmail.com> 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 <aneesh.kumar@linux.vnet.ibm.com> > 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
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 <aneesh.kumar@linux.vnet.ibm.com> Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/7cd2a8695ef9c31e8f51773f0de9e6 cheers
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; }
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(-)