[PATCH v4 1/8] KVM: PPC: Ultravisor: Introduce the MSR_S bit
cclaudio at linux.ibm.com
Sat Jul 13 07:07:12 AEST 2019
On 7/11/19 9:57 PM, Nicholas Piggin wrote:
> Claudio Carvalho's on June 29, 2019 6:08 am:
>> From: Sukadev Bhattiprolu <sukadev at 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
Yes, it means either. The table was arranged that way to say that:
- hypervisor state is also a privileged state,
- ultravisor state is also a hypervisor state.
> , 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?
Yes, it is. I also like this format. I will consider it.
>> 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 <sukadev at linux.vnet.ibm.com>
>> Signed-off-by: Ram Pai <linuxram at us.ibm.com>
>> [ Update the commit message ]
>> Signed-off-by: Claudio Carvalho <cclaudio at 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?
Fixed for the next version. Both comments will be /* Secure state */
More information about the Linuxppc-dev