Patchwork powerpc/kvmbook3s_hv: propagate H_SET_MODE to the host

login
register
mail settings
Submitter Laurent Dufour
Date Sept. 25, 2013, 12:10 p.m.
Message ID <20130925121027.29504.19269.stgit@nimbus>
Download mbox | patch
Permalink /patch/277822/
State Not Applicable
Headers show

Comments

Laurent Dufour - Sept. 25, 2013, 12:10 p.m.
Follow-up to Anton's H_SET_MODE patch, the host should be taken aware of
guest endianess change.

The hcall H_SET_MODE is processed in kvm then in the host.

Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
 arch/powerpc/kvm/book3s_hv.c |    8 ++++++++
 1 file changed, 8 insertions(+)
Greg Kurz - Sept. 25, 2013, 12:27 p.m.
On Wed, 25 Sep 2013 14:10:27 +0200
Laurent Dufour <ldufour@linux.vnet.ibm.com> wrote:
> Follow-up to Anton's H_SET_MODE patch, the host should be taken aware of
> guest endianess change.
> 
> The hcall H_SET_MODE is processed in kvm then in the host.
> 
> Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>

Tested-by: Greg Kurz <gkurz@linux.vnet.ibm.com>

> ---
>  arch/powerpc/kvm/book3s_hv.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 998cad3..4a47c74 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -599,6 +599,14 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
>  					kvmppc_get_gpr(vcpu, 5),
>  					kvmppc_get_gpr(vcpu, 6),
>  					kvmppc_get_gpr(vcpu, 7));
> +		/*
> +		 * If the hcall succeeded, we propagate it to the host.
> +		 * This way, it will be aware of the endianess's change too.
> +		 * The assumption is made that the hcall will succeed in the
> +		 * host.

Hmmm... Not sure the last sentence is appropriate from a kernel
perspective: it is up to the userland code to remain consistent
with this endianess change.

> +		 */
> +		if (ret == H_SUCCESS)
> +			return RESUME_HOST;
>  		break;
> 
>  	case H_XIRR:
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
Paul Mackerras - Sept. 25, 2013, 10:31 p.m.
On Wed, Sep 25, 2013 at 02:10:27PM +0200, Laurent Dufour wrote:
> Follow-up to Anton's H_SET_MODE patch, the host should be taken aware of
> guest endianess change.
> 
> The hcall H_SET_MODE is processed in kvm then in the host.
> 
> Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
> ---
>  arch/powerpc/kvm/book3s_hv.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 998cad3..4a47c74 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -599,6 +599,14 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
>  					kvmppc_get_gpr(vcpu, 5),
>  					kvmppc_get_gpr(vcpu, 6),
>  					kvmppc_get_gpr(vcpu, 7));
> +		/*
> +		 * If the hcall succeeded, we propagate it to the host.
> +		 * This way, it will be aware of the endianess's change too.
> +		 * The assumption is made that the hcall will succeed in the
> +		 * host.
> +		 */
> +		if (ret == H_SUCCESS)
> +			return RESUME_HOST;
>  		break;

The problem with this is that H_SET_MODE isn't just used for setting
endianness; it also does breakpoint setting (DAWR/X and CIABR), which
might happen very frequently, so we don't want them being punted up to
userspace.

Paul.
Laurent Dufour - Sept. 27, 2013, 8:14 a.m.
On 26/09/2013 00:31, Paul Mackerras wrote:
> On Wed, Sep 25, 2013 at 02:10:27PM +0200, Laurent Dufour wrote:
>> Follow-up to Anton's H_SET_MODE patch, the host should be taken aware of
>> guest endianess change.
>>
>> The hcall H_SET_MODE is processed in kvm then in the host.
>>
>> Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/kvm/book3s_hv.c |    8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index 998cad3..4a47c74 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -599,6 +599,14 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
>>  					kvmppc_get_gpr(vcpu, 5),
>>  					kvmppc_get_gpr(vcpu, 6),
>>  					kvmppc_get_gpr(vcpu, 7));
>> +		/*
>> +		 * If the hcall succeeded, we propagate it to the host.
>> +		 * This way, it will be aware of the endianess's change too.
>> +		 * The assumption is made that the hcall will succeed in the
>> +		 * host.
>> +		 */
>> +		if (ret == H_SUCCESS)
>> +			return RESUME_HOST;
>>  		break;
> 
> The problem with this is that H_SET_MODE isn't just used for setting
> endianness; it also does breakpoint setting (DAWR/X and CIABR), which
> might happen very frequently, so we don't want them being punted up to
> userspace.
> 
> Paul.

Hi Paul,

My mistake, the patch was based on a too old kernel missing yours and
Michael's latest patches on H_SET_MODE handling.

I'll propose a new one asap.

Thanks,
Laurent.

Patch

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 998cad3..4a47c74 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -599,6 +599,14 @@  int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
 					kvmppc_get_gpr(vcpu, 5),
 					kvmppc_get_gpr(vcpu, 6),
 					kvmppc_get_gpr(vcpu, 7));
+		/*
+		 * If the hcall succeeded, we propagate it to the host.
+		 * This way, it will be aware of the endianess's change too.
+		 * The assumption is made that the hcall will succeed in the
+		 * host.
+		 */
+		if (ret == H_SUCCESS)
+			return RESUME_HOST;
 		break;
 
 	case H_XIRR: