| Message ID | 20171204203538.8370-8-cdall@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> > 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>
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
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>
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>
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: {