Claudio Carvalho's on June 29, 2019 6:08 am:
> From: Sukadev Bhattiprolu <suka...@linux.vnet.ibm.com>
> 
> The ultravisor processor mode is introduced in POWER platforms that
> supports the Protected Execution Facility (PEF). Ultravisor is higher
> privileged than hypervisor mode.
> 
> In PEF enabled platforms, the MSR_S bit is used to indicate if the
> thread is in secure state. With the MSR_S bit, the privilege state of
> the thread is now determined by MSR_S, MSR_HV and MSR_PR, as follows:
> 
> S   HV  PR
> -----------------------
> 0   x   1   problem
> 1   0   1   problem
> x   x   0   privileged
> x   1   0   hypervisor
> 1   1   0   ultravisor
> 1   1   1   reserved

What does this table mean? I thought 'x' meant either, but in that
case there are several states that can apply to the same
combination of bits.

Would it be clearer to rearrange the table so the columns are the HV
and PR bits we know and love, plus the effect of S=1 on each of them?

      HV  PR  S=0         S=1
      ---------------------------------------------
      0   0   privileged  privileged (secure guest kernel)
      0   1   problem     problem (secure guest userspace)
      1   0   hypervisor  ultravisor
      1   1   problem     reserved

Is that accurate?


> 
> The hypervisor doesn't (and can't) run with the MSR_S bit set, but a
> secure guest and the ultravisor firmware do.
> 
> Signed-off-by: Sukadev Bhattiprolu <suka...@linux.vnet.ibm.com>
> Signed-off-by: Ram Pai <linux...@us.ibm.com>
> [ Update the commit message ]
> Signed-off-by: Claudio Carvalho <cclau...@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/reg.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 10caa145f98b..39b4c0a519f5 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -38,6 +38,7 @@
>  #define MSR_TM_LG    32              /* Trans Mem Available */
>  #define MSR_VEC_LG   25              /* Enable AltiVec */
>  #define MSR_VSX_LG   23              /* Enable VSX */
> +#define MSR_S_LG     22              /* Secure VM bit */
>  #define MSR_POW_LG   18              /* Enable Power Management */
>  #define MSR_WE_LG    18              /* Wait State Enable */
>  #define MSR_TGPR_LG  17              /* TLB Update registers in use */
> @@ -71,11 +72,13 @@
>  #define MSR_SF               __MASK(MSR_SF_LG)       /* Enable 64 bit mode */
>  #define MSR_ISF              __MASK(MSR_ISF_LG)      /* Interrupt 64b mode 
> valid on 630 */
>  #define MSR_HV               __MASK(MSR_HV_LG)       /* Hypervisor state */
> +#define MSR_S                __MASK(MSR_S_LG)        /* Secure state */

This is a real nitpick, but why two different comments for the bit 
number and the mask?

Reply via email to