re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

[gengdongjiu]

The negative effect is that kernel can not access kernel space adress through 
copy_to/from_user even KERNEL_DS is set
发件人：vladimir.murzin
收件人：耿东久,marc.zyngier,christoffer.dall,pbonzini,rkrcmar,linux-arm-kernel,kvmarm,kvm,linux-kernel,suzuki.poulose,mark.rutland,Catalin
 Marinas
抄送：James.Morse,张海斌,黄韶宇
时间：2017-09-06 23:19:38
主题:Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

On 06/09/17 16:08, gengdongjiu wrote:
> It is similar with the PAN，when the guest traps to el2，it will reset the 
> pstate.PAN <http://pstate.PAN> to 0, and continue run。In fact the host 
> pstate.UAO <http://pstate.UAO> can be 1, but guest change it to 0 when trap 
> to EL2。so after swich to host，need to check whether set pstate.UAO 
> <http://pstate.UAO> again。

What negative effect do you see with change UAO to 0 (i.e. do not override)?

P. S.
Please, avoid HTML and top posting

Vladimir

> *发件人：*Vladimir Murzin
> *收件人：*耿东久,marc.zyngier,christoffer.d...@linaro.org,pbonz...@redhat.com,rkrc...@redhat.com,linux-arm-ker...@lists.infradead.org,kvmarm@lists.cs.columbia.edu,k...@vger.kernel.org,linux-kernel,suzuki.poul...@arm.com,mark.rutl...@arm.com,Catalin
>  Marinas
> *抄送：*James Morse,张海斌,黄韶宇
> *时间：*2017-09-06 22:41:09
> *主题:*Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
>
> On 06/09/17 15:10, gengdongjiu wrote:
>> Hi, Vladimir
>>
>>>>> Do you see effect of "PAN is unexpectedly enabled"?
>>>> In fact I did not encounter this case, but I think it can exist.
>>>> I think if host OS dynamically disable PAN, it wants the host kernel 
>>>> access the user space address space not through copy_to/from_user
>>> API.
>>>> Now if it is unexpectedly enabled, when host kernel still accesses the 
>>>> user space address,  it will happen MMU fault exception.
>>>
>>> And this is expected! The only allowed channel for kernel <-> user is 
>>> uaccess API.
>>>
>>> I guess that you have test (and that great!) which violates that rule (for 
>>> testing purpose, why not?) and now you are trying to fit kernel into
>>> it...
>>
>>
>> If you think that makes sense for it, we do not consider the paste.PAN in 
>> the world-switch.
>> For the pstate.UAO issue, do you agree my fixing or you have other 
>> suggestion?  Also to other reviewer. Thanks.
>
> It would help if you give precise description on "pstate.UAO issue".
>
> Thanks
> Vladimir
>
>>
>> diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
>> index 9341376..c3dd761 100644
>> --- a/arch/arm64/kvm/hyp/sysreg-sr.c
>> +++ b/arch/arm64/kvm/hyp/sysreg-sr.c
>> @@ -21,6 +21,8 @@
>>  #include 
>>  #include 
>>
>> +#include 
>>
>>  /* Yes, this does nothing, on purpose */
>>  static void __hyp_text __sysreg_do_nothing(struct kvm_cpu_context *ctxt) { }
>>
>> @@ -121,8 +123,13 @@ static void __hyp_text __sysreg_restore_state(struct 
>> kvm_cpu_context *ctxt)
>> write_sysreg_el1(ctxt->gp_regs.spsr[KVM_SPSR_EL1],spsr);
>>  }
>>
>> +static void __hyp_text __sysreg_restore_state_vhe(struct kvm_cpu_context 
>> *ctxt)
>> +{
>> +uao_thread_switch(current);
>> +}
>> +
>>  static hyp_alternate_select(__sysreg_call_restore_host_state,
>> -   __sysreg_restore_state, __sysreg_do_nothing,
>> +   __sysreg_restore_state, 
>> __sysreg_restore_state_vhe,
>> ARM64_HAS_VIRT_HOST_EXTN);
>>
>>  void __hyp_text __sysreg_restore_host_state(struct kvm_cpu_context *ctxt)
>>
>>
>>>
>>> Cheers
>>> Vladimir
>>>
>>>>
>>>>
>>>>>
>>>>> Cheers
>>>>> Vladimir
>>>>>
>>>>>>
>>>>>>>
>>>>>>> Cheers
>>>>>>> Vladimir
>>>>>>>
>>>>>>> .
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> .
>>>>>
>>>>
>>>>
>>
>

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

[kbuild test robot]

Hi gengdongjiu,

[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on v4.13 next-20170906]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/gengdongjiu/arm64-KVM-VHE-save-and-restore-some-PSTATE-bits/20170907-013418
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git 
for-next/core
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   /tmp/ccT6UpbP.s: Assembler messages:
>> /tmp/ccT6UpbP.s:1415: Error: constant expression required
   /tmp/ccT6UpbP.s:1439: Error: constant expression required
>> /tmp/ccT6UpbP.s:1420: Error: attempt to move .org backwards
   /tmp/ccT6UpbP.s:1444: Error: attempt to move .org backwards

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

[Vladimir Murzin]

On 06/09/17 16:08, gengdongjiu wrote:
> It is similar with the PAN，when the guest traps to el2，it will reset the 
> pstate.PAN <http://pstate.PAN> to 0, and continue run。In fact the host 
> pstate.UAO <http://pstate.UAO> can be 1, but guest change it to 0 when trap 
> to EL2。so after swich to host，need to check whether set pstate.UAO 
> <http://pstate.UAO> again。

What negative effect do you see with change UAO to 0 (i.e. do not override)?

P. S.
Please, avoid HTML and top posting

Vladimir

> *发件人：*Vladimir Murzin
> *收件人：*耿东久,marc.zyngier,christoffer.d...@linaro.org,pbonz...@redhat.com,rkrc...@redhat.com,linux-arm-ker...@lists.infradead.org,kvmarm@lists.cs.columbia.edu,k...@vger.kernel.org,linux-kernel,suzuki.poul...@arm.com,mark.rutl...@arm.com,Catalin
>  Marinas
> *抄送：*James Morse,张海斌,黄韶宇
> *时间：*2017-09-06 22:41:09
> *主题:*Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
> 
> On 06/09/17 15:10, gengdongjiu wrote:
>> Hi, Vladimir
>> 
>>>>> Do you see effect of "PAN is unexpectedly enabled"?
>>>> In fact I did not encounter this case, but I think it can exist.
>>>> I think if host OS dynamically disable PAN, it wants the host kernel 
>>>> access the user space address space not through copy_to/from_user
>>> API.
>>>> Now if it is unexpectedly enabled, when host kernel still accesses the 
>>>> user space address,  it will happen MMU fault exception.
>>>
>>> And this is expected! The only allowed channel for kernel <-> user is 
>>> uaccess API.
>>>
>>> I guess that you have test (and that great!) which violates that rule (for 
>>> testing purpose, why not?) and now you are trying to fit kernel into
>>> it...
>> 
>> 
>> If you think that makes sense for it, we do not consider the paste.PAN in 
>> the world-switch.
>> For the pstate.UAO issue, do you agree my fixing or you have other 
>> suggestion?  Also to other reviewer. Thanks.
> 
> It would help if you give precise description on "pstate.UAO issue".
> 
> Thanks
> Vladimir
> 
>> 
>> diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
>> index 9341376..c3dd761 100644
>> --- a/arch/arm64/kvm/hyp/sysreg-sr.c
>> +++ b/arch/arm64/kvm/hyp/sysreg-sr.c
>> @@ -21,6 +21,8 @@
>>  #include 
>>  #include 
>> 
>> +#include 
>> 
>>  /* Yes, this does nothing, on purpose */
>>  static void __hyp_text __sysreg_do_nothing(struct kvm_cpu_context *ctxt) { }
>> 
>> @@ -121,8 +123,13 @@ static void __hyp_text __sysreg_restore_state(struct 
>> kvm_cpu_context *ctxt)
>> write_sysreg_el1(ctxt->gp_regs.spsr[KVM_SPSR_EL1],spsr);
>>  }
>> 
>> +static void __hyp_text __sysreg_restore_state_vhe(struct kvm_cpu_context 
>> *ctxt)
>> +{
>> +uao_thread_switch(current);
>> +}
>> +
>>  static hyp_alternate_select(__sysreg_call_restore_host_state,
>> -   __sysreg_restore_state, __sysreg_do_nothing,
>> +   __sysreg_restore_state, 
>> __sysreg_restore_state_vhe,
>> ARM64_HAS_VIRT_HOST_EXTN);
>> 
>>  void __hyp_text __sysreg_restore_host_state(struct kvm_cpu_context *ctxt)
>> 
>> 
>>>
>>> Cheers
>>> Vladimir
>>>
>>>>
>>>>
>>>>>
>>>>> Cheers
>>>>> Vladimir
>>>>>
>>>>>>
>>>>>>>
>>>>>>> Cheers
>>>>>>> Vladimir
>>>>>>>
>>>>>>> .
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> .
>>>>>
>>>>
>>>>
>> 
> 

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

答复:re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

[gengdongjiu]

Fix a typo.

It is similar with the PAN，when the guest traps to el2，it will reset the 
pstate.<http://pstate.PAN>UAO to 0, and continue run。In fact the host 
pstate.UAO<http://pstate.UAO> can be 1, but guest change it to 0 when trap to 
EL2, so after swich to host，need to check whether set 
pstate.UAO<http://pstate.UAO> again。

发件人：Vladimir Murzin
收件人：耿东久,marc.zyngier,christoffer.d...@linaro.org,pbonz...@redhat.com,rkrc...@redhat.com,linux-arm-ker...@lists.infradead.org,kvmarm@lists.cs.columbia.edu,k...@vger.kernel.org,linux-kernel,suzuki.poul...@arm.com,mark.rutl...@arm.com,Catalin
 Marinas
抄送：James Morse,张海斌,黄韶宇
时间：2017-09-06 22:41:09
主题:Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

On 06/09/17 15:10, gengdongjiu wrote:
> Hi, Vladimir
>
>>>> Do you see effect of "PAN is unexpectedly enabled"?
>>> In fact I did not encounter this case, but I think it can exist.
>>> I think if host OS dynamically disable PAN, it wants the host kernel access 
>>> the user space address space not through copy_to/from_user
>> API.
>>> Now if it is unexpectedly enabled, when host kernel still accesses the user 
>>> space address,  it will happen MMU fault exception.
>>
>> And this is expected! The only allowed channel for kernel <-> user is 
>> uaccess API.
>>
>> I guess that you have test (and that great!) which violates that rule (for 
>> testing purpose, why not?) and now you are trying to fit kernel into
>> it...
>
>
> If you think that makes sense for it, we do not consider the paste.PAN in the 
> world-switch.
> For the pstate.UAO issue, do you agree my fixing or you have other 
> suggestion?  Also to other reviewer. Thanks.

It would help if you give precise description on "pstate.UAO issue".

Thanks
Vladimir

>
> diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
> index 9341376..c3dd761 100644
> --- a/arch/arm64/kvm/hyp/sysreg-sr.c
> +++ b/arch/arm64/kvm/hyp/sysreg-sr.c
> @@ -21,6 +21,8 @@
>  #include 
>  #include 
>
> +#include 
>
>  /* Yes, this does nothing, on purpose */
>  static void __hyp_text __sysreg_do_nothing(struct kvm_cpu_context *ctxt) { }
>
> @@ -121,8 +123,13 @@ static void __hyp_text __sysreg_restore_state(struct 
> kvm_cpu_context *ctxt)
> write_sysreg_el1(ctxt->gp_regs.spsr[KVM_SPSR_EL1],spsr);
>  }
>
> +static void __hyp_text __sysreg_restore_state_vhe(struct kvm_cpu_context 
> *ctxt)
> +{
> +uao_thread_switch(current);
> +}
> +
>  static hyp_alternate_select(__sysreg_call_restore_host_state,
> -   __sysreg_restore_state, __sysreg_do_nothing,
> +   __sysreg_restore_state, 
> __sysreg_restore_state_vhe,
> ARM64_HAS_VIRT_HOST_EXTN);
>
>  void __hyp_text __sysreg_restore_host_state(struct kvm_cpu_context *ctxt)
>
>
>>
>> Cheers
>> Vladimir
>>
>>>
>>>
>>>>
>>>> Cheers
>>>> Vladimir
>>>>
>>>>>
>>>>>>
>>>>>> Cheers
>>>>>> Vladimir
>>>>>>
>>>>>> .
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> .
>>>>
>>>
>>>
>

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

[gengdongjiu]

It is similar with the PAN，when the guest traps to el2，it will reset the 
pstate.PAN<http://pstate.PAN> to 0, and continue run。In fact the host 
pstate.UAO<http://pstate.UAO> can be 1, but guest change it to 0 when trap to 
EL2。so after swich to host，need to check whether set 
pstate.UAO<http://pstate.UAO> again。
发件人：Vladimir Murzin
收件人：耿东久,marc.zyngier,christoffer.d...@linaro.org,pbonz...@redhat.com,rkrc...@redhat.com,linux-arm-ker...@lists.infradead.org,kvmarm@lists.cs.columbia.edu,k...@vger.kernel.org,linux-kernel,suzuki.poul...@arm.com,mark.rutl...@arm.com,Catalin
 Marinas
抄送：James Morse,张海斌,黄韶宇
时间：2017-09-06 22:41:09
主题:Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

On 06/09/17 15:10, gengdongjiu wrote:
> Hi, Vladimir
>
>>>> Do you see effect of "PAN is unexpectedly enabled"?
>>> In fact I did not encounter this case, but I think it can exist.
>>> I think if host OS dynamically disable PAN, it wants the host kernel access 
>>> the user space address space not through copy_to/from_user
>> API.
>>> Now if it is unexpectedly enabled, when host kernel still accesses the user 
>>> space address,  it will happen MMU fault exception.
>>
>> And this is expected! The only allowed channel for kernel <-> user is 
>> uaccess API.
>>
>> I guess that you have test (and that great!) which violates that rule (for 
>> testing purpose, why not?) and now you are trying to fit kernel into
>> it...
>
>
> If you think that makes sense for it, we do not consider the paste.PAN in the 
> world-switch.
> For the pstate.UAO issue, do you agree my fixing or you have other 
> suggestion?  Also to other reviewer. Thanks.

It would help if you give precise description on "pstate.UAO issue".

Thanks
Vladimir

>
> diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
> index 9341376..c3dd761 100644
> --- a/arch/arm64/kvm/hyp/sysreg-sr.c
> +++ b/arch/arm64/kvm/hyp/sysreg-sr.c
> @@ -21,6 +21,8 @@
>  #include 
>  #include 
>
> +#include 
>
>  /* Yes, this does nothing, on purpose */
>  static void __hyp_text __sysreg_do_nothing(struct kvm_cpu_context *ctxt) { }
>
> @@ -121,8 +123,13 @@ static void __hyp_text __sysreg_restore_state(struct 
> kvm_cpu_context *ctxt)
> write_sysreg_el1(ctxt->gp_regs.spsr[KVM_SPSR_EL1],spsr);
>  }
>
> +static void __hyp_text __sysreg_restore_state_vhe(struct kvm_cpu_context 
> *ctxt)
> +{
> +uao_thread_switch(current);
> +}
> +
>  static hyp_alternate_select(__sysreg_call_restore_host_state,
> -   __sysreg_restore_state, __sysreg_do_nothing,
> +   __sysreg_restore_state, 
> __sysreg_restore_state_vhe,
> ARM64_HAS_VIRT_HOST_EXTN);
>
>  void __hyp_text __sysreg_restore_host_state(struct kvm_cpu_context *ctxt)
>
>
>>
>> Cheers
>> Vladimir
>>
>>>
>>>
>>>>
>>>> Cheers
>>>> Vladimir
>>>>
>>>>>
>>>>>>
>>>>>> Cheers
>>>>>> Vladimir
>>>>>>
>>>>>> .
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> .
>>>>
>>>
>>>
>

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

[Vladimir Murzin]

On 06/09/17 15:10, gengdongjiu wrote:
> Hi, Vladimir
> 
 Do you see effect of "PAN is unexpectedly enabled"?
>>> In fact I did not encounter this case, but I think it can exist.
>>> I think if host OS dynamically disable PAN, it wants the host kernel access 
>>> the user space address space not through copy_to/from_user
>> API.
>>> Now if it is unexpectedly enabled, when host kernel still accesses the user 
>>> space address,  it will happen MMU fault exception.
>>
>> And this is expected! The only allowed channel for kernel <-> user is 
>> uaccess API.
>>
>> I guess that you have test (and that great!) which violates that rule (for 
>> testing purpose, why not?) and now you are trying to fit kernel into
>> it...
> 
> 
> If you think that makes sense for it, we do not consider the paste.PAN in the 
> world-switch.
> For the pstate.UAO issue, do you agree my fixing or you have other 
> suggestion?  Also to other reviewer. Thanks.

It would help if you give precise description on "pstate.UAO issue".

Thanks
Vladimir

> 
> diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
> index 9341376..c3dd761 100644
> --- a/arch/arm64/kvm/hyp/sysreg-sr.c
> +++ b/arch/arm64/kvm/hyp/sysreg-sr.c
> @@ -21,6 +21,8 @@
>  #include 
>  #include 
> 
> +#include 
> 
>  /* Yes, this does nothing, on purpose */
>  static void __hyp_text __sysreg_do_nothing(struct kvm_cpu_context *ctxt) { }
> 
> @@ -121,8 +123,13 @@ static void __hyp_text __sysreg_restore_state(struct 
> kvm_cpu_context *ctxt)
> write_sysreg_el1(ctxt->gp_regs.spsr[KVM_SPSR_EL1],spsr);
>  }
> 
> +static void __hyp_text __sysreg_restore_state_vhe(struct kvm_cpu_context 
> *ctxt)
> +{
> +uao_thread_switch(current);
> +}
> +
>  static hyp_alternate_select(__sysreg_call_restore_host_state,
> -   __sysreg_restore_state, __sysreg_do_nothing,
> +   __sysreg_restore_state, 
> __sysreg_restore_state_vhe,
> ARM64_HAS_VIRT_HOST_EXTN);
> 
>  void __hyp_text __sysreg_restore_host_state(struct kvm_cpu_context *ctxt)
> 
> 
>>
>> Cheers
>> Vladimir
>>
>>>
>>>

 Cheers
 Vladimir

>
>>
>> Cheers
>> Vladimir
>>
>> .
>>
>
>


 .

>>>
>>>
> 

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

[gengdongjiu]

Hi, Vladimir

> >> Do you see effect of "PAN is unexpectedly enabled"?
> > In fact I did not encounter this case, but I think it can exist.
> > I think if host OS dynamically disable PAN, it wants the host kernel access 
> > the user space address space not through copy_to/from_user
> API.
> > Now if it is unexpectedly enabled, when host kernel still accesses the user 
> > space address,  it will happen MMU fault exception.
> 
> And this is expected! The only allowed channel for kernel <-> user is uaccess 
> API.
> 
> I guess that you have test (and that great!) which violates that rule (for 
> testing purpose, why not?) and now you are trying to fit kernel into
> it...


If you think that makes sense for it, we do not consider the paste.PAN in the 
world-switch.
For the pstate.UAO issue, do you agree my fixing or you have other suggestion?  
Also to other reviewer. Thanks.

diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
index 9341376..c3dd761 100644
--- a/arch/arm64/kvm/hyp/sysreg-sr.c
+++ b/arch/arm64/kvm/hyp/sysreg-sr.c
@@ -21,6 +21,8 @@
 #include 
 #include 

+#include 

 /* Yes, this does nothing, on purpose */
 static void __hyp_text __sysreg_do_nothing(struct kvm_cpu_context *ctxt) { }

@@ -121,8 +123,13 @@ static void __hyp_text __sysreg_restore_state(struct 
kvm_cpu_context *ctxt)
write_sysreg_el1(ctxt->gp_regs.spsr[KVM_SPSR_EL1],spsr);
 }

+static void __hyp_text __sysreg_restore_state_vhe(struct kvm_cpu_context *ctxt)
+{
+uao_thread_switch(current);
+}
+
 static hyp_alternate_select(__sysreg_call_restore_host_state,
-   __sysreg_restore_state, __sysreg_do_nothing,
+   __sysreg_restore_state, __sysreg_restore_state_vhe,
ARM64_HAS_VIRT_HOST_EXTN);

 void __hyp_text __sysreg_restore_host_state(struct kvm_cpu_context *ctxt)


> 
> Cheers
> Vladimir
> 
> >
> >
> >>
> >> Cheers
> >> Vladimir
> >>
> >>>
> 
>  Cheers
>  Vladimir
> 
>  .
> 
> >>>
> >>>
> >>
> >>
> >> .
> >>
> >
> >

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

[Vladimir Murzin]

On 06/09/17 13:44, gengdongjiu wrote:
> 
> 
> On 2017/9/6 20:30, Vladimir Murzin wrote:
>> On 06/09/17 13:14, gengdongjiu wrote:
>>>
>>>
>>> On 2017/9/6 20:00, Vladimir Murzin wrote:
 On 06/09/17 11:35, gengdongjiu wrote:
> Vladimir,
>
> On 2017/9/6 17:41, Vladimir Murzin wrote:
>> Can you please elaborate on cases where PAN is not enabled?
>
> I mean the informal private usage, For example, he disabled the PAN 
> dynamically to let kernel space to access the user space.
> After he dynamic disabled the PAN, then switched to guest OS. after 
> return to host. he found the PAN stage is modified.
> Of cause this is not a formal usage, in our host kernel, it is always 
> enabled, no dynamic change, but I means it may exist such cases.
>
>

 So, in short, there is no real issue with PAN, right? What about UAO?
>>> For the PAN, if host OS dynamically enable/disable PAN should have issue.
>>> Do you think that is not a issue as above description?
>>>
>>> "host OS dynamically disable the PAN, but after go back from the guest OS, 
>>> The PAN is unexpectedly enabled"
>>
>> Do you see effect of "PAN is unexpectedly enabled"?
> In fact I did not encounter this case, but I think it can exist.
> I think if host OS dynamically disable PAN, it wants the host kernel access 
> the user space address space not through copy_to/from_user API.
> Now if it is unexpectedly enabled, when host kernel still accesses the user 
> space address,  it will happen MMU fault exception.

And this is expected! The only allowed channel for kernel <-> user is uaccess
API.

I guess that you have test (and that great!) which violates that rule (for
testing purpose, why not?) and now you are trying to fit kernel into it...

Cheers
Vladimir

> 
> 
>>
>> Cheers
>> Vladimir
>>
>>>

 Cheers
 Vladimir

 .

>>>
>>>
>>
>>
>> .
>>
> 
> 

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

[gengdongjiu]

On 2017/9/6 20:30, Vladimir Murzin wrote:
> On 06/09/17 13:14, gengdongjiu wrote:
>>
>>
>> On 2017/9/6 20:00, Vladimir Murzin wrote:
>>> On 06/09/17 11:35, gengdongjiu wrote:
 Vladimir,

 On 2017/9/6 17:41, Vladimir Murzin wrote:
> Can you please elaborate on cases where PAN is not enabled?

 I mean the informal private usage, For example, he disabled the PAN 
 dynamically to let kernel space to access the user space.
 After he dynamic disabled the PAN, then switched to guest OS. after return 
 to host. he found the PAN stage is modified.
 Of cause this is not a formal usage, in our host kernel, it is always 
 enabled, no dynamic change, but I means it may exist such cases.


>>>
>>> So, in short, there is no real issue with PAN, right? What about UAO?
>> For the PAN, if host OS dynamically enable/disable PAN should have issue.
>> Do you think that is not a issue as above description?
>>
>> "host OS dynamically disable the PAN, but after go back from the guest OS, 
>> The PAN is unexpectedly enabled"
> 
> Do you see effect of "PAN is unexpectedly enabled"?
In fact I did not encounter this case, but I think it can exist.
I think if host OS dynamically disable PAN, it wants the host kernel access the 
user space address space not through copy_to/from_user API.
Now if it is unexpectedly enabled, when host kernel still accesses the user 
space address,  it will happen MMU fault exception.


> 
> Cheers
> Vladimir
> 
>>
>>>
>>> Cheers
>>> Vladimir
>>>
>>> .
>>>
>>
>>
> 
> 
> .
> 

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

[gengdongjiu]

On 2017/9/6 20:00, Vladimir Murzin wrote:
> On 06/09/17 11:35, gengdongjiu wrote:
>> Vladimir,
>>
>> On 2017/9/6 17:41, Vladimir Murzin wrote:
>>> Can you please elaborate on cases where PAN is not enabled?
>>
>> I mean the informal private usage, For example, he disabled the PAN 
>> dynamically to let kernel space to access the user space.
>> After he dynamic disabled the PAN, then switched to guest OS. after return 
>> to host. he found the PAN stage is modified.
>> Of cause this is not a formal usage, in our host kernel, it is always 
>> enabled, no dynamic change, but I means it may exist such cases.
>>
>>
> 
> So, in short, there is no real issue with PAN, right? What about UAO?
For the pstate.UAO, current code has issue from my test. Because after 
switching from guest os, it does not set pstate.UAO again.
PAN is set again in your previous patch when switch to host, but UAO is not.
If you have concern about the save/restore PSTATE bits, may be we can use below 
modification to fix UAO issue.

diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
index 9341376..c3dd761 100644
--- a/arch/arm64/kvm/hyp/sysreg-sr.c
+++ b/arch/arm64/kvm/hyp/sysreg-sr.c
@@ -21,6 +21,8 @@
 #include 
 #include 

+#include 

 /* Yes, this does nothing, on purpose */
 static void __hyp_text __sysreg_do_nothing(struct kvm_cpu_context *ctxt) { }

@@ -121,8 +123,13 @@ static void __hyp_text __sysreg_restore_state(struct 
kvm_cpu_context *ctxt)
write_sysreg_el1(ctxt->gp_regs.spsr[KVM_SPSR_EL1],spsr);
 }

+static void __hyp_text __sysreg_restore_state_vhe(struct kvm_cpu_context *ctxt)
+{
+uao_thread_switch(current);
+}
+
 static hyp_alternate_select(__sysreg_call_restore_host_state,
-   __sysreg_restore_state, __sysreg_do_nothing,
+   __sysreg_restore_state, __sysreg_restore_state_vhe,
ARM64_HAS_VIRT_HOST_EXTN);

 void __hyp_text __sysreg_restore_host_state(struct kvm_cpu_context *ctxt)


> 
> Cheers
> Vladimir
> 
> .
> 

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

[Vladimir Murzin]

On 06/09/17 13:14, gengdongjiu wrote:
> 
> 
> On 2017/9/6 20:00, Vladimir Murzin wrote:
>> On 06/09/17 11:35, gengdongjiu wrote:
>>> Vladimir,
>>>
>>> On 2017/9/6 17:41, Vladimir Murzin wrote:
 Can you please elaborate on cases where PAN is not enabled?
>>>
>>> I mean the informal private usage, For example, he disabled the PAN 
>>> dynamically to let kernel space to access the user space.
>>> After he dynamic disabled the PAN, then switched to guest OS. after return 
>>> to host. he found the PAN stage is modified.
>>> Of cause this is not a formal usage, in our host kernel, it is always 
>>> enabled, no dynamic change, but I means it may exist such cases.
>>>
>>>
>>
>> So, in short, there is no real issue with PAN, right? What about UAO?
> For the PAN, if host OS dynamically enable/disable PAN should have issue.
> Do you think that is not a issue as above description?
> 
> "host OS dynamically disable the PAN, but after go back from the guest OS, 
> The PAN is unexpectedly enabled"

Do you see effect of "PAN is unexpectedly enabled"?

Cheers
Vladimir

> 
>>
>> Cheers
>> Vladimir
>>
>> .
>>
> 
> 

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

[gengdongjiu]

On 2017/9/6 20:00, Vladimir Murzin wrote:
> On 06/09/17 11:35, gengdongjiu wrote:
>> Vladimir,
>>
>> On 2017/9/6 17:41, Vladimir Murzin wrote:
>>> Can you please elaborate on cases where PAN is not enabled?
>>
>> I mean the informal private usage, For example, he disabled the PAN 
>> dynamically to let kernel space to access the user space.
>> After he dynamic disabled the PAN, then switched to guest OS. after return 
>> to host. he found the PAN stage is modified.
>> Of cause this is not a formal usage, in our host kernel, it is always 
>> enabled, no dynamic change, but I means it may exist such cases.
>>
>>
> 
> So, in short, there is no real issue with PAN, right? What about UAO?
For the PAN, if host OS dynamically enable/disable PAN should have issue.
Do you think that is not a issue as above description?

"host OS dynamically disable the PAN, but after go back from the guest OS, The 
PAN is unexpectedly enabled"

> 
> Cheers
> Vladimir
> 
> .
> 

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

[Vladimir Murzin]

On 06/09/17 11:35, gengdongjiu wrote:
> Vladimir,
> 
> On 2017/9/6 17:41, Vladimir Murzin wrote:
>> Can you please elaborate on cases where PAN is not enabled?
> 
> I mean the informal private usage, For example, he disabled the PAN 
> dynamically to let kernel space to access the user space.
> After he dynamic disabled the PAN, then switched to guest OS. after return to 
> host. he found the PAN stage is modified.
> Of cause this is not a formal usage, in our host kernel, it is always 
> enabled, no dynamic change, but I means it may exist such cases.
> 
> 

So, in short, there is no real issue with PAN, right? What about UAO?

Cheers
Vladimir
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

[gengdongjiu]

Vladimir,

On 2017/9/6 17:41, Vladimir Murzin wrote:
> Can you please elaborate on cases where PAN is not enabled?

I mean the informal private usage, For example, he disabled the PAN dynamically 
to let kernel space to access the user space.
After he dynamic disabled the PAN, then switched to guest OS. after return to 
host. he found the PAN stage is modified.
Of cause this is not a formal usage, in our host kernel, it is always enabled, 
no dynamic change, but I means it may exist such cases.

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

[gengdongjiu]

For UAO, if not save/restore PSTATE.UAO, we can use below fixing.

diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
index 9341376..c3dd761 100644
--- a/arch/arm64/kvm/hyp/sysreg-sr.c
+++ b/arch/arm64/kvm/hyp/sysreg-sr.c
@@ -21,6 +21,8 @@
 #include 
 #include 

+#include 
+
 /* Yes, this does nothing, on purpose */
 static void __hyp_text __sysreg_do_nothing(struct kvm_cpu_context *ctxt) { }

@@ -121,8 +123,13 @@ static void __hyp_text __sysreg_restore_state(struct 
kvm_cpu_context *ctxt)
write_sysreg_el1(ctxt->gp_regs.spsr[KVM_SPSR_EL1],spsr);
 }

+static void __hyp_text __sysreg_restore_state_vhe(struct kvm_cpu_context *ctxt)
+{
+uao_thread_switch(current);
+}
+
 static hyp_alternate_select(__sysreg_call_restore_host_state,
-   __sysreg_restore_state, __sysreg_do_nothing,
+   __sysreg_restore_state, __sysreg_restore_state_vhe,
ARM64_HAS_VIRT_HOST_EXTN);

 void __hyp_text __sysreg_restore_host_state(struct kvm_cpu_context *ctxt)



On 2017/9/6 17:32, gengdongjiu wrote:
> Hi Marc,
> 
> On 2017/9/6 16:17, Marc Zyngier wrote:
>> On 05/09/17 19:58, gengdongjiu wrote:
>>> when exit from guest, some host PSTATE bits may be lost, such as
>>> PSTATE.PAN or PSTATE.UAO. It is because host and hypervisor all run
>>> in the EL2, host PSTATE value cannot be saved and restored via
>>> SPSR_EL2. So if guest has changed the PSTATE, host continues with
>>> a wrong value guest has set.
>>>
>>> Signed-off-by: Dongjiu Geng 
>>> Signed-off-by: Haibin Zhang 
>>> ---
>>>  arch/arm64/include/asm/kvm_host.h |  8 +++
>>>  arch/arm64/include/asm/kvm_hyp.h  |  2 ++
>>>  arch/arm64/include/asm/sysreg.h   | 23 +++
>>>  arch/arm64/kvm/hyp/entry.S|  2 --
>>>  arch/arm64/kvm/hyp/switch.c   | 24 ++--
>>>  arch/arm64/kvm/hyp/sysreg-sr.c| 48 
>>> ---
>>>  6 files changed, 100 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/arch/arm64/include/asm/kvm_host.h 
>>> b/arch/arm64/include/asm/kvm_host.h
>>> index e923b58..cba7d3e 100644
>>> --- a/arch/arm64/include/asm/kvm_host.h
>>> +++ b/arch/arm64/include/asm/kvm_host.h
>>> @@ -193,6 +193,12 @@ struct kvm_cpu_context {
>>> };
>>>  };
>>>  
>>> +struct kvm_cpu_host_pstate {
>>> +   u64 daif;
>>> +   u64 uao;
>>> +   u64 pan;
>>> +};
>>
>> I love it. This is the most expensive way of saving/restoring a single
>> 32bit value.
>>
>> More seriously, please see the discussion between James and Christoffer
>> there[1]. I expect James to address the PAN/UAO states together with the
>> debug state in the next iteration of his patch.
> 
> I roughly see the discussion between James and Christoffer, Seems Christoffer 
> does not suggest save and
> restore it, and suggest to do below, and UAO/PAN may not use the same ways.
> 
>   __kvm_vcpu_run(struct kvm_vcpu *vcpu)
>   {
>   if (has_vhe())
>   asm("msr daifset, #0xf");
> 
>   ...
>  exit_code = __guest_enter(vcpu, host_ctxt);
>   ...
> 
>   if (has_vhe())
>   asm("msr daifclr, #0xd");
>   }
> 
> 
> If not save/restore them, the KVM will set them according to the CPU 
> capability. For example below fixing, it will check CPU capability, If CPU 
> supports PAN,
> the kvm will always enable the PAN for the host. But in fact, the host may be 
> not enable the PAN.
> Of course for the UAO, we can use the similar fixing if Marc or Christoffer 
> is agreed. but seems not make sense.
> 
> commit cb96408da4e11698674abd04aeac941c1bed2038
> Author: Vladimir Murzin 
> Date:   Thu Sep 1 15:29:03 2016 +0100
> 
> arm64: KVM: VHE: reset PSTATE.PAN on entry to EL2
> 
> SCTLR_EL2.SPAN bit controls what happens with the PSTATE.PAN bit on an
> exception. However, this bit has no effect on the PSTATE.PAN when
> HCR_EL2.E2H or HCR_EL2.TGE is unset. Thus when VHE is used and
> exception taken from a guest PSTATE.PAN bit left unchanged and we
> continue with a value guest has set.
> 
> To address that always reset PSTATE.PAN on entry from EL1.
> 
> Fixes: 1f364c8c48a0 ("arm64: VHE: Add support for running Linux in EL2 
> mode")
> 
> Signed-off-by: Vladimir Murzin 
> Reviewed-by: James Morse 
> Acked-by: Marc Zyngier 
> Cc:  # v4.6+
> Signed-off-by: Christoffer Dall 
> 
> diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
> index 3967c231..b5926ee 100644
> --- a/arch/arm64/kvm/hyp/entry.S
> +++ b/arch/arm64/kvm/hyp/entry.S
> @@ -96,6 +96,8 @@ ENTRY(__guest_exit)
> 
> add x1, x1, #VCPU_CONTEXT
> 
> +   ALTERNATIVE(nop, SET_PSTATE_PAN(1), ARM64_HAS_PAN, CONFIG_ARM64_PAN)
> +
> // Store the guest regs x2 and x3
> 

Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

[Vladimir Murzin]

On 06/09/17 10:32, gengdongjiu wrote:
> Hi Marc,
> 
> On 2017/9/6 16:17, Marc Zyngier wrote:
>> On 05/09/17 19:58, gengdongjiu wrote:
>>> when exit from guest, some host PSTATE bits may be lost, such as
>>> PSTATE.PAN or PSTATE.UAO. It is because host and hypervisor all run
>>> in the EL2, host PSTATE value cannot be saved and restored via
>>> SPSR_EL2. So if guest has changed the PSTATE, host continues with
>>> a wrong value guest has set.
>>>
>>> Signed-off-by: Dongjiu Geng 
>>> Signed-off-by: Haibin Zhang 
>>> ---
>>>  arch/arm64/include/asm/kvm_host.h |  8 +++
>>>  arch/arm64/include/asm/kvm_hyp.h  |  2 ++
>>>  arch/arm64/include/asm/sysreg.h   | 23 +++
>>>  arch/arm64/kvm/hyp/entry.S|  2 --
>>>  arch/arm64/kvm/hyp/switch.c   | 24 ++--
>>>  arch/arm64/kvm/hyp/sysreg-sr.c| 48 
>>> ---
>>>  6 files changed, 100 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/arch/arm64/include/asm/kvm_host.h 
>>> b/arch/arm64/include/asm/kvm_host.h
>>> index e923b58..cba7d3e 100644
>>> --- a/arch/arm64/include/asm/kvm_host.h
>>> +++ b/arch/arm64/include/asm/kvm_host.h
>>> @@ -193,6 +193,12 @@ struct kvm_cpu_context {
>>> };
>>>  };
>>>  
>>> +struct kvm_cpu_host_pstate {
>>> +   u64 daif;
>>> +   u64 uao;
>>> +   u64 pan;
>>> +};
>>
>> I love it. This is the most expensive way of saving/restoring a single
>> 32bit value.
>>
>> More seriously, please see the discussion between James and Christoffer
>> there[1]. I expect James to address the PAN/UAO states together with the
>> debug state in the next iteration of his patch.
> 
> I roughly see the discussion between James and Christoffer, Seems Christoffer 
> does not suggest save and
> restore it, and suggest to do below, and UAO/PAN may not use the same ways.
> 
>   __kvm_vcpu_run(struct kvm_vcpu *vcpu)
>   {
>   if (has_vhe())
>   asm("msr daifset, #0xf");
> 
>   ...
>  exit_code = __guest_enter(vcpu, host_ctxt);
>   ...
> 
>   if (has_vhe())
>   asm("msr daifclr, #0xd");
>   }
> 
> 
> If not save/restore them, the KVM will set them according to the CPU 
> capability. For example below fixing, it will check CPU capability, If CPU 
> supports PAN,
> the kvm will always enable the PAN for the host. But in fact, the host may be 
> not enable the PAN.

Can you please elaborate on cases where PAN is not enabled?

Thanks
Vladimir

> Of course for the UAO, we can use the similar fixing if Marc or Christoffer 
> is agreed. but seems not make sense.
> 
> commit cb96408da4e11698674abd04aeac941c1bed2038
> Author: Vladimir Murzin 
> Date:   Thu Sep 1 15:29:03 2016 +0100
> 
> arm64: KVM: VHE: reset PSTATE.PAN on entry to EL2
> 
> SCTLR_EL2.SPAN bit controls what happens with the PSTATE.PAN bit on an
> exception. However, this bit has no effect on the PSTATE.PAN when
> HCR_EL2.E2H or HCR_EL2.TGE is unset. Thus when VHE is used and
> exception taken from a guest PSTATE.PAN bit left unchanged and we
> continue with a value guest has set.
> 
> To address that always reset PSTATE.PAN on entry from EL1.
> 
> Fixes: 1f364c8c48a0 ("arm64: VHE: Add support for running Linux in EL2 
> mode")
> 
> Signed-off-by: Vladimir Murzin 
> Reviewed-by: James Morse 
> Acked-by: Marc Zyngier 
> Cc:  # v4.6+
> Signed-off-by: Christoffer Dall 
> 
> diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
> index 3967c231..b5926ee 100644
> --- a/arch/arm64/kvm/hyp/entry.S
> +++ b/arch/arm64/kvm/hyp/entry.S
> @@ -96,6 +96,8 @@ ENTRY(__guest_exit)
> 
> add x1, x1, #VCPU_CONTEXT
> 
> +   ALTERNATIVE(nop, SET_PSTATE_PAN(1), ARM64_HAS_PAN, CONFIG_ARM64_PAN)
> +
> // Store the guest regs x2 and x3
> stp x2, x3,   [x1, #CPU_XREG_OFFSET(2)]
> 
> 
>>
>> Thanks,
>>
>>  M.
>>
>> [1] https://www.spinics.net/lists/arm-kernel/msg599798.html
>>
> 
> 

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

[gengdongjiu]

Hi Marc,

On 2017/9/6 16:17, Marc Zyngier wrote:
> On 05/09/17 19:58, gengdongjiu wrote:
>> when exit from guest, some host PSTATE bits may be lost, such as
>> PSTATE.PAN or PSTATE.UAO. It is because host and hypervisor all run
>> in the EL2, host PSTATE value cannot be saved and restored via
>> SPSR_EL2. So if guest has changed the PSTATE, host continues with
>> a wrong value guest has set.
>>
>> Signed-off-by: Dongjiu Geng 
>> Signed-off-by: Haibin Zhang 
>> ---
>>  arch/arm64/include/asm/kvm_host.h |  8 +++
>>  arch/arm64/include/asm/kvm_hyp.h  |  2 ++
>>  arch/arm64/include/asm/sysreg.h   | 23 +++
>>  arch/arm64/kvm/hyp/entry.S|  2 --
>>  arch/arm64/kvm/hyp/switch.c   | 24 ++--
>>  arch/arm64/kvm/hyp/sysreg-sr.c| 48 
>> ---
>>  6 files changed, 100 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/kvm_host.h 
>> b/arch/arm64/include/asm/kvm_host.h
>> index e923b58..cba7d3e 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -193,6 +193,12 @@ struct kvm_cpu_context {
>>  };
>>  };
>>  
>> +struct kvm_cpu_host_pstate {
>> +u64 daif;
>> +u64 uao;
>> +u64 pan;
>> +};
> 
> I love it. This is the most expensive way of saving/restoring a single
> 32bit value.
> 
> More seriously, please see the discussion between James and Christoffer
> there[1]. I expect James to address the PAN/UAO states together with the
> debug state in the next iteration of his patch.

I roughly see the discussion between James and Christoffer, Seems Christoffer 
does not suggest save and
restore it, and suggest to do below, and UAO/PAN may not use the same ways.

  __kvm_vcpu_run(struct kvm_vcpu *vcpu)
  {
  if (has_vhe())
  asm("msr daifset, #0xf");

...
 exit_code = __guest_enter(vcpu, host_ctxt);
...

if (has_vhe())
  asm("msr daifclr, #0xd");
  }


If not save/restore them, the KVM will set them according to the CPU 
capability. For example below fixing, it will check CPU capability, If CPU 
supports PAN,
the kvm will always enable the PAN for the host. But in fact, the host may be 
not enable the PAN.
Of course for the UAO, we can use the similar fixing if Marc or Christoffer is 
agreed. but seems not make sense.

commit cb96408da4e11698674abd04aeac941c1bed2038
Author: Vladimir Murzin 
Date:   Thu Sep 1 15:29:03 2016 +0100

arm64: KVM: VHE: reset PSTATE.PAN on entry to EL2

SCTLR_EL2.SPAN bit controls what happens with the PSTATE.PAN bit on an
exception. However, this bit has no effect on the PSTATE.PAN when
HCR_EL2.E2H or HCR_EL2.TGE is unset. Thus when VHE is used and
exception taken from a guest PSTATE.PAN bit left unchanged and we
continue with a value guest has set.

To address that always reset PSTATE.PAN on entry from EL1.

Fixes: 1f364c8c48a0 ("arm64: VHE: Add support for running Linux in EL2 
mode")

Signed-off-by: Vladimir Murzin 
Reviewed-by: James Morse 
Acked-by: Marc Zyngier 
Cc:  # v4.6+
Signed-off-by: Christoffer Dall 

diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
index 3967c231..b5926ee 100644
--- a/arch/arm64/kvm/hyp/entry.S
+++ b/arch/arm64/kvm/hyp/entry.S
@@ -96,6 +96,8 @@ ENTRY(__guest_exit)

add x1, x1, #VCPU_CONTEXT

+   ALTERNATIVE(nop, SET_PSTATE_PAN(1), ARM64_HAS_PAN, CONFIG_ARM64_PAN)
+
// Store the guest regs x2 and x3
stp x2, x3,   [x1, #CPU_XREG_OFFSET(2)]


> 
> Thanks,
> 
>   M.
> 
> [1] https://www.spinics.net/lists/arm-kernel/msg599798.html
> 

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

[Marc Zyngier]

On 05/09/17 19:58, gengdongjiu wrote:
> when exit from guest, some host PSTATE bits may be lost, such as
> PSTATE.PAN or PSTATE.UAO. It is because host and hypervisor all run
> in the EL2, host PSTATE value cannot be saved and restored via
> SPSR_EL2. So if guest has changed the PSTATE, host continues with
> a wrong value guest has set.
> 
> Signed-off-by: Dongjiu Geng 
> Signed-off-by: Haibin Zhang 
> ---
>  arch/arm64/include/asm/kvm_host.h |  8 +++
>  arch/arm64/include/asm/kvm_hyp.h  |  2 ++
>  arch/arm64/include/asm/sysreg.h   | 23 +++
>  arch/arm64/kvm/hyp/entry.S|  2 --
>  arch/arm64/kvm/hyp/switch.c   | 24 ++--
>  arch/arm64/kvm/hyp/sysreg-sr.c| 48 
> ---
>  6 files changed, 100 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h 
> b/arch/arm64/include/asm/kvm_host.h
> index e923b58..cba7d3e 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -193,6 +193,12 @@ struct kvm_cpu_context {
>   };
>  };
>  
> +struct kvm_cpu_host_pstate {
> + u64 daif;
> + u64 uao;
> + u64 pan;
> +};

I love it. This is the most expensive way of saving/restoring a single
32bit value.

More seriously, please see the discussion between James and Christoffer
there[1]. I expect James to address the PAN/UAO states together with the
debug state in the next iteration of his patch.

Thanks,

M.

[1] https://www.spinics.net/lists/arm-kernel/msg599798.html
-- 
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

[gengdongjiu]

CC Catalin


On 2017/9/6 2:58, gengdongjiu wrote:
> when exit from guest, some host PSTATE bits may be lost, such as
> PSTATE.PAN or PSTATE.UAO. It is because host and hypervisor all run
> in the EL2, host PSTATE value cannot be saved and restored via
> SPSR_EL2. So if guest has changed the PSTATE, host continues with
> a wrong value guest has set.
> 
> Signed-off-by: Dongjiu Geng 
> Signed-off-by: Haibin Zhang 
> ---
>  arch/arm64/include/asm/kvm_host.h |  8 +++
>  arch/arm64/include/asm/kvm_hyp.h  |  2 ++
>  arch/arm64/include/asm/sysreg.h   | 23 +++
>  arch/arm64/kvm/hyp/entry.S|  2 --
>  arch/arm64/kvm/hyp/switch.c   | 24 ++--
>  arch/arm64/kvm/hyp/sysreg-sr.c| 48 
> ---
>  6 files changed, 100 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h 
> b/arch/arm64/include/asm/kvm_host.h
> index e923b58..cba7d3e 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -193,6 +193,12 @@ struct kvm_cpu_context {
>   };
>  };
>  
> +struct kvm_cpu_host_pstate {
> + u64 daif;
> + u64 uao;
> + u64 pan;
> +};
> +
>  typedef struct kvm_cpu_context kvm_cpu_context_t;
>  
>  struct kvm_vcpu_arch {
> @@ -227,6 +233,8 @@ struct kvm_vcpu_arch {
>  
>   /* Pointer to host CPU context */
>   kvm_cpu_context_t *host_cpu_context;
> + /* Host PSTATE value */
> + struct kvm_cpu_host_pstate host_pstate;
>   struct {
>   /* {Break,watch}point registers */
>   struct kvm_guest_debug_arch regs;
> diff --git a/arch/arm64/include/asm/kvm_hyp.h 
> b/arch/arm64/include/asm/kvm_hyp.h
> index 4572a9b..a75587a 100644
> --- a/arch/arm64/include/asm/kvm_hyp.h
> +++ b/arch/arm64/include/asm/kvm_hyp.h
> @@ -134,6 +134,8 @@
>  
>  void __sysreg_save_host_state(struct kvm_cpu_context *ctxt);
>  void __sysreg_restore_host_state(struct kvm_cpu_context *ctxt);
> +void __sysreg_save_host_pstate(struct kvm_vcpu *vcpu);
> +void __sysreg_restore_host_pstate(struct kvm_vcpu *vcpu);
>  void __sysreg_save_guest_state(struct kvm_cpu_context *ctxt);
>  void __sysreg_restore_guest_state(struct kvm_cpu_context *ctxt);
>  void __sysreg32_save_state(struct kvm_vcpu *vcpu);
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 248339e..efdcf40 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -295,6 +295,29 @@
>  #define SYS_ICH_LR14_EL2 __SYS__LR8_EL2(6)
>  #define SYS_ICH_LR15_EL2 __SYS__LR8_EL2(7)
>  
> +#define REG_PSTATE_PAN   sys_reg(3, 0, 4, 2, 3)
> +#define REG_PSTATE_UAO   sys_reg(3, 0, 4, 2, 4)
> +
> +#define GET_PSTATE_PAN   \
> + ({  \
> + u64 reg;\
> + asm volatile(ALTERNATIVE("mov %0, #0",  \
> + "mrs_s %0, " __stringify(REG_PSTATE_PAN),\
> + ARM64_HAS_PAN)\
> + : "=r" (reg));\
> + reg;\
> + })
> +
> +#define GET_PSTATE_UAO   \
> + ({  \
> + u64 reg;\
> + asm volatile(ALTERNATIVE("mov %0, #0",\
> + "mrs_s %0, " __stringify(REG_PSTATE_UAO),\
> + ARM64_HAS_UAO)\
> + : "=r" (reg));\
> + reg;\
> + })
> +
>  /* Common SCTLR_ELx flags. */
>  #define SCTLR_ELx_EE(1 << 25)
>  #define SCTLR_ELx_I  (1 << 12)
> diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
> index 12ee62d..7662ef5 100644
> --- a/arch/arm64/kvm/hyp/entry.S
> +++ b/arch/arm64/kvm/hyp/entry.S
> @@ -96,8 +96,6 @@ ENTRY(__guest_exit)
>  
>   add x1, x1, #VCPU_CONTEXT
>  
> - ALTERNATIVE(nop, SET_PSTATE_PAN(1), ARM64_HAS_PAN, CONFIG_ARM64_PAN)
> -
>   // Store the guest regs x2 and x3
>   stp x2, x3,   [x1, #CPU_XREG_OFFSET(2)]
>  
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 945e79c..9b380a1 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -278,6 +278,26 @@ static void __hyp_text __skip_instr(struct kvm_vcpu 
> *vcpu)
>   write_sysreg_el2(*vcpu_pc(vcpu), elr);
>  }
>  
> +static void __hyp_text __save_host_state(struct kvm_vcpu *vcpu)
> +{
> + struct kvm_cpu_context *host_ctxt;
> +
> + host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
> +
> + __sysreg_save_host_state(host_ctxt);
> + __sysreg_save_host_pstate(vcpu);
> +}
> +
> +static void __hyp_text __restore_host_state(struct kvm_vcpu *vcpu)
> +{
> + struct