Re: [PATCH v5 5/7] powerpc/mm: Write to PTCR only if ultravisor disabled

[Michael Ellerman]

Claudio Carvalho  writes:
> In ultravisor enabled systems, PTCR becomes ultravisor privileged only
> for writing and an attempt to write to it will cause a Hypervisor
> Emulation Assitance interrupt.
>
> This patch adds the try_set_ptcr(val) macro as an accessor to
> mtspr(SPRN_PTCR, val), which will be executed only if ultravisor
> disabled.
>
> Signed-off-by: Claudio Carvalho 
> ---
>  arch/powerpc/include/asm/reg.h   | 13 +
>  arch/powerpc/mm/book3s64/hash_utils.c|  4 ++--
>  arch/powerpc/mm/book3s64/pgtable.c   |  2 +-
>  arch/powerpc/mm/book3s64/radix_pgtable.c |  6 +++---
>  4 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 10caa145f98b..14139b1ebdb8 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -15,6 +15,7 @@
>  #include 
>  #include 
>  #include 
> +#include 

reg.h is already too big and unwieldy.

Can you put this in ultravisor.h and include that in the appropriate places.

> @@ -1452,6 +1453,18 @@ static inline void update_power8_hid0(unsigned long 
> hid0)
>*/
>   asm volatile("sync; mtspr %0,%1; isync":: "i"(SPRN_HID0), "r"(hid0));
>  }
> +
> +/*
> + * In ultravisor enabled systems, PTCR becomes ultravisor privileged only for
> + * writing and an attempt to write to it will cause a Hypervisor Emulation
> + * Assistance interrupt.
> + */
> +#define try_set_ptcr(val)\
> + do {\
> + if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))   \
> + mtspr(SPRN_PTCR, val);  \
> + } while (0)

This should be a static inline please, not a macro.

Sorry, I don't like the name, we're not trying to set it, we know when
to set it and when not to.

It is awkward to come up with a good name because we don't have a term
for "hypervisor that's not running under an ultravisor".

Maybe set_ptcr_when_no_uv()

Which is kinda messy, someone feel free to come up with something
better.

I also see some more accesses to the PTCR in
arch/powerpc/platforms/powernv/idle.c which you haven't patched?

cheers

[PATCH v5 5/7] powerpc/mm: Write to PTCR only if ultravisor disabled

[Claudio Carvalho]

In ultravisor enabled systems, PTCR becomes ultravisor privileged only
for writing and an attempt to write to it will cause a Hypervisor
Emulation Assitance interrupt.

This patch adds the try_set_ptcr(val) macro as an accessor to
mtspr(SPRN_PTCR, val), which will be executed only if ultravisor
disabled.

Signed-off-by: Claudio Carvalho 
---
 arch/powerpc/include/asm/reg.h   | 13 +
 arch/powerpc/mm/book3s64/hash_utils.c|  4 ++--
 arch/powerpc/mm/book3s64/pgtable.c   |  2 +-
 arch/powerpc/mm/book3s64/radix_pgtable.c |  6 +++---
 4 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 10caa145f98b..14139b1ebdb8 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* Pickup Book E specific registers. */
 #if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
@@ -1452,6 +1453,18 @@ static inline void update_power8_hid0(unsigned long hid0)
 */
asm volatile("sync; mtspr %0,%1; isync":: "i"(SPRN_HID0), "r"(hid0));
 }
+
+/*
+ * In ultravisor enabled systems, PTCR becomes ultravisor privileged only for
+ * writing and an attempt to write to it will cause a Hypervisor Emulation
+ * Assistance interrupt.
+ */
+#define try_set_ptcr(val)  \
+   do {\
+   if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))   \
+   mtspr(SPRN_PTCR, val);  \
+   } while (0)
+
 #endif /* __ASSEMBLY__ */
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_REG_H */
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c 
b/arch/powerpc/mm/book3s64/hash_utils.c
index 25a2cf32d544..048b7f58deae 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -1079,8 +1079,8 @@ void hash__early_init_mmu_secondary(void)
if (!cpu_has_feature(CPU_FTR_ARCH_300))
mtspr(SPRN_SDR1, _SDR1);
else
-   mtspr(SPRN_PTCR,
- __pa(partition_tb) | (PATB_SIZE_SHIFT - 12));
+   try_set_ptcr(__pa(partition_tb) |
+(PATB_SIZE_SHIFT - 12));
}
/* Initialize SLB */
slb_initialize();
diff --git a/arch/powerpc/mm/book3s64/pgtable.c 
b/arch/powerpc/mm/book3s64/pgtable.c
index 033731f5dbaa..016c6ccb5b81 100644
--- a/arch/powerpc/mm/book3s64/pgtable.c
+++ b/arch/powerpc/mm/book3s64/pgtable.c
@@ -219,7 +219,7 @@ void __init mmu_partition_table_init(void)
 * 64 K size.
 */
ptcr = __pa(partition_tb) | (PATB_SIZE_SHIFT - 12);
-   mtspr(SPRN_PTCR, ptcr);
+   try_set_ptcr(ptcr);
powernv_set_nmmu_ptcr(ptcr);
 }
 
diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c 
b/arch/powerpc/mm/book3s64/radix_pgtable.c
index e92c6472a20c..246b32550eab 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -654,8 +654,8 @@ void radix__early_init_mmu_secondary(void)
lpcr = mfspr(SPRN_LPCR);
mtspr(SPRN_LPCR, lpcr | LPCR_UPRT | LPCR_HR);
 
-   mtspr(SPRN_PTCR,
- __pa(partition_tb) | (PATB_SIZE_SHIFT - 12));
+   try_set_ptcr(__pa(partition_tb) | (PATB_SIZE_SHIFT - 12));
+
radix_init_amor();
}
 
@@ -671,7 +671,7 @@ void radix__mmu_cleanup_all(void)
if (!firmware_has_feature(FW_FEATURE_LPAR)) {
lpcr = mfspr(SPRN_LPCR);
mtspr(SPRN_LPCR, lpcr & ~LPCR_UPRT);
-   mtspr(SPRN_PTCR, 0);
+   try_set_ptcr(0);
powernv_set_nmmu_ptcr(0);
radix__flush_tlb_all();
}
-- 
2.20.1