diff mbox series

[v3,07/16] KVM: Move vcpu_load to arch-specific kvm_arch_vcpu_ioctl_set_sregs

Message ID 20171204203538.8370-8-cdall@kernel.org
State Accepted
Headers show
Series Move vcpu_load and vcpu_put calls to arch code | expand

Commit Message

Christoffer Dall Dec. 4, 2017, 8:35 p.m. UTC 
From: Christoffer Dall <christoffer.dall@linaro.org>

Move vcpu_load() and vcpu_put() into the architecture specific
implementations of kvm_arch_vcpu_ioctl_set_sregs().

Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 arch/powerpc/kvm/book3s.c |  8 +++++++-
 arch/powerpc/kvm/booke.c  | 15 +++++++++++----
 arch/s390/kvm/kvm-s390.c  |  4 ++++
 arch/x86/kvm/x86.c        | 13 ++++++++++---
 virt/kvm/kvm_main.c       |  2 --
 5 files changed, 32 insertions(+), 10 deletions(-)

Comments

David Hildenbrand Dec. 8, 2017, 4:26 p.m. UTC | #1 
>  
>  int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index f647e121070e..cdf0be02c95a 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -1632,18 +1632,25 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
>  {
>  	int ret;
>  
> +	vcpu_load(vcpu);
> +
> +	ret = -EINVAL;

you can initialize this directly.

>  	if (vcpu->arch.pvr != sregs->pvr)
> -		return -EINVAL;
> +		goto out;
>  
>  	ret = set_sregs_base(vcpu, sregs);
>  	if (ret < 0)
> -		return ret;
> +		goto out;
>  
>  	ret = set_sregs_arch206(vcpu, sregs);
>  	if (ret < 0)
> -		return ret;
> +		goto out;
> +
> +	ret = vcpu->kvm->arch.kvm_ops->set_sregs(vcpu, sregs);
>  
> -	return vcpu->kvm->arch.kvm_ops->set_sregs(vcpu, sregs);
> +out:
> +	vcpu_put(vcpu);
> +	return ret;
>  }
>  
>  int kvmppc_get_one_reg(struct kvm_vcpu *vcpu, u64 id,
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 18011fc4ac49..d95b4f15e52b 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2729,8 +2729,12 @@ int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
>  int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
>  				  struct kvm_sregs *sregs)
>  {
> +	vcpu_load(vcpu);
> +
>  	memcpy(&vcpu->run->s.regs.acrs, &sregs->acrs, sizeof(sregs->acrs));
>  	memcpy(&vcpu->arch.sie_block->gcr, &sregs->crs, sizeof(sregs->crs));
> +
> +	vcpu_put(vcpu);
>  	return 0;
>  }
>  
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 20a5f6776eea..a31a80aee0b9 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -7500,15 +7500,19 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
>  	int mmu_reset_needed = 0;
>  	int pending_vec, max_bits, idx;
>  	struct desc_ptr dt;
> +	int ret;
> +
> +	vcpu_load(vcpu);
>  
> +	ret = -EINVAL;

dito


Reviewed-by: David Hildenbrand <david@redhat.com>
Christoffer Dall Dec. 11, 2017, 9:19 a.m. UTC | #2 
On Fri, Dec 08, 2017 at 05:26:02PM +0100, David Hildenbrand wrote:
> 
> >  
> >  int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
> > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> > index f647e121070e..cdf0be02c95a 100644
> > --- a/arch/powerpc/kvm/booke.c
> > +++ b/arch/powerpc/kvm/booke.c
> > @@ -1632,18 +1632,25 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
> >  {
> >  	int ret;
> >  
> > +	vcpu_load(vcpu);
> > +
> > +	ret = -EINVAL;
> 
> you can initialize this directly.
> 
> >  	if (vcpu->arch.pvr != sregs->pvr)
> > -		return -EINVAL;
> > +		goto out;
> >  
> >  	ret = set_sregs_base(vcpu, sregs);
> >  	if (ret < 0)
> > -		return ret;
> > +		goto out;
> >  
> >  	ret = set_sregs_arch206(vcpu, sregs);
> >  	if (ret < 0)
> > -		return ret;
> > +		goto out;
> > +
> > +	ret = vcpu->kvm->arch.kvm_ops->set_sregs(vcpu, sregs);
> >  
> > -	return vcpu->kvm->arch.kvm_ops->set_sregs(vcpu, sregs);
> > +out:
> > +	vcpu_put(vcpu);
> > +	return ret;
> >  }
> >  
> >  int kvmppc_get_one_reg(struct kvm_vcpu *vcpu, u64 id,
> > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> > index 18011fc4ac49..d95b4f15e52b 100644
> > --- a/arch/s390/kvm/kvm-s390.c
> > +++ b/arch/s390/kvm/kvm-s390.c
> > @@ -2729,8 +2729,12 @@ int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
> >  int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
> >  				  struct kvm_sregs *sregs)
> >  {
> > +	vcpu_load(vcpu);
> > +
> >  	memcpy(&vcpu->run->s.regs.acrs, &sregs->acrs, sizeof(sregs->acrs));
> >  	memcpy(&vcpu->arch.sie_block->gcr, &sregs->crs, sizeof(sregs->crs));
> > +
> > +	vcpu_put(vcpu);
> >  	return 0;
> >  }
> >  
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 20a5f6776eea..a31a80aee0b9 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -7500,15 +7500,19 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
> >  	int mmu_reset_needed = 0;
> >  	int pending_vec, max_bits, idx;
> >  	struct desc_ptr dt;
> > +	int ret;
> > +
> > +	vcpu_load(vcpu);
> >  
> > +	ret = -EINVAL;
> 
> dito

Sure.

> 
> 
> Reviewed-by: David Hildenbrand <david@redhat.com>
> 

Thanks for the review!
-Christoffer
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Cornelia Huck Dec. 11, 2017, 12:15 p.m. UTC | #3 
On Mon,  4 Dec 2017 21:35:29 +0100
Christoffer Dall <cdall@kernel.org> wrote:

> From: Christoffer Dall <christoffer.dall@linaro.org>
> 
> Move vcpu_load() and vcpu_put() into the architecture specific
> implementations of kvm_arch_vcpu_ioctl_set_sregs().
> 
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
>  arch/powerpc/kvm/book3s.c |  8 +++++++-
>  arch/powerpc/kvm/booke.c  | 15 +++++++++++----
>  arch/s390/kvm/kvm-s390.c  |  4 ++++
>  arch/x86/kvm/x86.c        | 13 ++++++++++---
>  virt/kvm/kvm_main.c       |  2 --
>  5 files changed, 32 insertions(+), 10 deletions(-)

With David's suggestions included:

Reviewed-by: Cornelia Huck <cohuck@redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paolo Bonzini Dec. 12, 2017, 4:33 p.m. UTC | #4 
On 11/12/2017 10:19, Christoffer Dall wrote:
> On Fri, Dec 08, 2017 at 05:26:02PM +0100, David Hildenbrand wrote:
>>
>>>  
>>>  int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
>>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
>>> index f647e121070e..cdf0be02c95a 100644
>>> --- a/arch/powerpc/kvm/booke.c
>>> +++ b/arch/powerpc/kvm/booke.c
>>> @@ -1632,18 +1632,25 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
>>>  {
>>>  	int ret;
>>>  
>>> +	vcpu_load(vcpu);
>>> +
>>> +	ret = -EINVAL;
>>
>> you can initialize this directly.
>>
>>>  	if (vcpu->arch.pvr != sregs->pvr)
>>> -		return -EINVAL;
>>> +		goto out;
>>>  
>>>  	ret = set_sregs_base(vcpu, sregs);
>>>  	if (ret < 0)
>>> -		return ret;
>>> +		goto out;
>>>  
>>>  	ret = set_sregs_arch206(vcpu, sregs);
>>>  	if (ret < 0)
>>> -		return ret;
>>> +		goto out;
>>> +
>>> +	ret = vcpu->kvm->arch.kvm_ops->set_sregs(vcpu, sregs);
>>>  
>>> -	return vcpu->kvm->arch.kvm_ops->set_sregs(vcpu, sregs);
>>> +out:
>>> +	vcpu_put(vcpu);
>>> +	return ret;
>>>  }
>>>  
>>>  int kvmppc_get_one_reg(struct kvm_vcpu *vcpu, u64 id,
>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>> index 18011fc4ac49..d95b4f15e52b 100644
>>> --- a/arch/s390/kvm/kvm-s390.c
>>> +++ b/arch/s390/kvm/kvm-s390.c
>>> @@ -2729,8 +2729,12 @@ int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
>>>  int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
>>>  				  struct kvm_sregs *sregs)
>>>  {
>>> +	vcpu_load(vcpu);
>>> +
>>>  	memcpy(&vcpu->run->s.regs.acrs, &sregs->acrs, sizeof(sregs->acrs));
>>>  	memcpy(&vcpu->arch.sie_block->gcr, &sregs->crs, sizeof(sregs->crs));
>>> +
>>> +	vcpu_put(vcpu);
>>>  	return 0;
>>>  }
>>>  
>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>>> index 20a5f6776eea..a31a80aee0b9 100644
>>> --- a/arch/x86/kvm/x86.c
>>> +++ b/arch/x86/kvm/x86.c
>>> @@ -7500,15 +7500,19 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
>>>  	int mmu_reset_needed = 0;
>>>  	int pending_vec, max_bits, idx;
>>>  	struct desc_ptr dt;
>>> +	int ret;
>>> +
>>> +	vcpu_load(vcpu);
>>>  
>>> +	ret = -EINVAL;
>>
>> dito
> 
> Sure.

I'm doing it when applying.

Paolo

>> Reviewed-by: David Hildenbrand <david@redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 6cc2377549f7..047651622cb8 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -496,7 +496,13 @@  int kvm_arch_vcpu_ioctl_get_sregs(struct kvm_vcpu *vcpu,
 int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
 				  struct kvm_sregs *sregs)
 {
-	return vcpu->kvm->arch.kvm_ops->set_sregs(vcpu, sregs);
+	int ret;
+
+	vcpu_load(vcpu);
+	ret = vcpu->kvm->arch.kvm_ops->set_sregs(vcpu, sregs);
+	vcpu_put(vcpu);
+
+	return ret;
 }
 
 int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index f647e121070e..cdf0be02c95a 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -1632,18 +1632,25 @@  int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
 {
 	int ret;
 
+	vcpu_load(vcpu);
+
+	ret = -EINVAL;
 	if (vcpu->arch.pvr != sregs->pvr)
-		return -EINVAL;
+		goto out;
 
 	ret = set_sregs_base(vcpu, sregs);
 	if (ret < 0)
-		return ret;
+		goto out;
 
 	ret = set_sregs_arch206(vcpu, sregs);
 	if (ret < 0)
-		return ret;
+		goto out;
+
+	ret = vcpu->kvm->arch.kvm_ops->set_sregs(vcpu, sregs);
 
-	return vcpu->kvm->arch.kvm_ops->set_sregs(vcpu, sregs);
+out:
+	vcpu_put(vcpu);
+	return ret;
 }
 
 int kvmppc_get_one_reg(struct kvm_vcpu *vcpu, u64 id,
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 18011fc4ac49..d95b4f15e52b 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2729,8 +2729,12 @@  int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
 int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
 				  struct kvm_sregs *sregs)
 {
+	vcpu_load(vcpu);
+
 	memcpy(&vcpu->run->s.regs.acrs, &sregs->acrs, sizeof(sregs->acrs));
 	memcpy(&vcpu->arch.sie_block->gcr, &sregs->crs, sizeof(sregs->crs));
+
+	vcpu_put(vcpu);
 	return 0;
 }
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 20a5f6776eea..a31a80aee0b9 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7500,15 +7500,19 @@  int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
 	int mmu_reset_needed = 0;
 	int pending_vec, max_bits, idx;
 	struct desc_ptr dt;
+	int ret;
+
+	vcpu_load(vcpu);
 
+	ret = -EINVAL;
 	if (!guest_cpuid_has(vcpu, X86_FEATURE_XSAVE) &&
 			(sregs->cr4 & X86_CR4_OSXSAVE))
-		return -EINVAL;
+		goto out;
 
 	apic_base_msr.data = sregs->apic_base;
 	apic_base_msr.host_initiated = true;
 	if (kvm_set_apic_base(vcpu, &apic_base_msr))
-		return -EINVAL;
+		goto out;
 
 	dt.size = sregs->idt.limit;
 	dt.address = sregs->idt.base;
@@ -7574,7 +7578,10 @@  int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
 
 	kvm_make_request(KVM_REQ_EVENT, vcpu);
 
-	return 0;
+	ret = 0;
+out:
+	vcpu_put(vcpu);
+	return ret;
 }
 
 int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 779c03e39fa4..19cf2d11f80f 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2597,9 +2597,7 @@  static long kvm_vcpu_ioctl(struct file *filp,
 			kvm_sregs = NULL;
 			goto out;
 		}
-		vcpu_load(vcpu);
 		r = kvm_arch_vcpu_ioctl_set_sregs(vcpu, kvm_sregs);
-		vcpu_put(vcpu);
 		break;
 	}
 	case KVM_GET_MP_STATE: {