Re: [PATCH] KVM: arm64: fix misleading comments in save/restore

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, May 28, 2015 at 10:43:06AM +0100, Alex Bennée wrote:
> The elr_el2 and spsr_el2 registers in fact contain the processor state
> before entry into the hypervisor code.

be careful with your use of the hypervisor, in the KVM design the
hypervisor is split across EL1 and EL2.

> In the case of guest state it
> could be in either el0 or el1.

true

> 
> Signed-off-by: Alex Bennée <alex.bennee@xxxxxxxxxx>
> ---
>  arch/arm64/kvm/hyp.S | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
> index d755922..1940a4c 100644
> --- a/arch/arm64/kvm/hyp.S
> +++ b/arch/arm64/kvm/hyp.S
> @@ -50,8 +50,8 @@
>  	stp	x29, lr, [x3, #80]
>  
>  	mrs	x19, sp_el0
> -	mrs	x20, elr_el2		// EL1 PC
> -	mrs	x21, spsr_el2		// EL1 pstate
> +	mrs	x20, elr_el2		// PC before hyp entry
> +	mrs	x21, spsr_el2		// pstate before hyp entry
>  
>  	stp	x19, x20, [x3, #96]
>  	str	x21, [x3, #112]
> @@ -82,8 +82,8 @@
>  	ldr	x21, [x3, #16]
>  
>  	msr	sp_el0, x19
> -	msr	elr_el2, x20 				// EL1 PC
> -	msr	spsr_el2, x21 				// EL1 pstate
> +	msr	elr_el2, x20 		// PC to restore
> +	msr	spsr_el2, x21 		// pstate to restore

I don't feel like 'to restore' is much more meaningful here.

I would actually vote for removin the comments all together, since one
should really understand the code as opposed to the comments when
reading this kind of stuff.

Meh, I'm not sure.  Your patch is definitely better than doing nothing.

Marc?

-Christoffer
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm





[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux