| Message ID | 1500470851-6262-1-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> -----Original Message----- > From: Andrew Cooper [mailto:andrew.cooper3@citrix.com] > Sent: 19 July 2017 14:28 > To: Xen-devel <xen-devel@lists.xen.org> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; George Dunlap > <George.Dunlap@citrix.com>; Jan Beulich <JBeulich@suse.com>; Wei Liu > <wei.liu2@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com>; Roger Pau > Monne <roger.pau@citrix.com> > Subject: [PATCH] x86/hvm: Drop more remains of the PVHv1 implementation > > These functions don't need is_hvm_{vcpu,domain}() predicates. > > hvmop_set_evtchn_upcall_vector() does need the predicate to prevent a > PV > caller accessing the hvm union, but swap the copy_from_guest() and > is_hvm_domain() predicate to avoid reading the hypercall parameter if we > not > going to use it. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Paul Durrant <paul.durrant@citrix.com> > --- > CC: George Dunlap <George.Dunlap@eu.citrix.com> > CC: Jan Beulich <JBeulich@suse.com> > CC: Wei Liu <wei.liu2@citrix.com> > CC: Paul Durrant <paul.durrant@citrix.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > --- > xen/arch/x86/hvm/hvm.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index 8145385..4fef616 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -506,8 +506,7 @@ void hvm_do_resume(struct vcpu *v) > { > check_wakeup_from_wait(); > > - if ( is_hvm_domain(v->domain) ) > - pt_restore_timer(v); > + pt_restore_timer(v); > > if ( !handle_hvm_io_completion(v) ) > return; > @@ -1544,8 +1543,7 @@ void hvm_vcpu_destroy(struct vcpu *v) > tasklet_kill(&v->arch.hvm_vcpu.assert_evtchn_irq_tasklet); > hvm_funcs.vcpu_destroy(v); > > - if ( is_hvm_vcpu(v) ) > - vlapic_destroy(v); > + vlapic_destroy(v); > > hvm_vcpu_cacheattr_destroy(v); > } > @@ -1711,7 +1709,6 @@ int hvm_hap_nested_page_fault(paddr_t gpa, > unsigned long gla, > * - newer Windows (like Server 2012) for HPET accesses. > */ > if ( !nestedhvm_vcpu_in_guestmode(curr) > - && is_hvm_domain(currd) > && hvm_mmio_internal(gpa) ) > { > if ( !handle_mmio_with_translation(gla, gpa >> PAGE_SHIFT, npfec) ) > @@ -3139,7 +3136,7 @@ static enum hvm_copy_result __hvm_copy( > * - 32-bit WinXP (& older Windows) on AMD CPUs for LAPIC accesses, > * - newer Windows (like Server 2012) for HPET accesses. > */ > - if ( v == current && is_hvm_vcpu(v) > + if ( v == current > && !nestedhvm_vcpu_in_guestmode(v) > && hvm_mmio_internal(gpa) ) > return HVMCOPY_bad_gfn_to_mfn; > @@ -3971,12 +3968,12 @@ static int hvmop_set_evtchn_upcall_vector( > struct domain *d = current->domain; > struct vcpu *v; > > - if ( copy_from_guest(&op, uop, 1) ) > - return -EFAULT; > - > if ( !is_hvm_domain(d) ) > return -EINVAL; > > + if ( copy_from_guest(&op, uop, 1) ) > + return -EFAULT; > + > if ( op.vector < 0x10 ) > return -EINVAL; > > -- > 2.1.4
On Wed, Jul 19, 2017 at 02:27:31PM +0100, Andrew Cooper wrote: > These functions don't need is_hvm_{vcpu,domain}() predicates. > > hvmop_set_evtchn_upcall_vector() does need the predicate to prevent a PV > caller accessing the hvm union, but swap the copy_from_guest() and > is_hvm_domain() predicate to avoid reading the hypercall parameter if we not > going to use it. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
On Wed, Jul 19, 2017 at 02:27:31PM +0100, Andrew Cooper wrote: > These functions don't need is_hvm_{vcpu,domain}() predicates. > > hvmop_set_evtchn_upcall_vector() does need the predicate to prevent a PV > caller accessing the hvm union, but swap the copy_from_guest() and > is_hvm_domain() predicate to avoid reading the hypercall parameter if we not > going to use it. IC, certain HVMOPs are available to PV guests (ie: the control domain). > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Thanks. Just one style nit. > CC: George Dunlap <George.Dunlap@eu.citrix.com> > CC: Jan Beulich <JBeulich@suse.com> > CC: Wei Liu <wei.liu2@citrix.com> > CC: Paul Durrant <paul.durrant@citrix.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > --- > xen/arch/x86/hvm/hvm.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index 8145385..4fef616 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -506,8 +506,7 @@ void hvm_do_resume(struct vcpu *v) > { > check_wakeup_from_wait(); > > - if ( is_hvm_domain(v->domain) ) > - pt_restore_timer(v); > + pt_restore_timer(v); > > if ( !handle_hvm_io_completion(v) ) > return; > @@ -1544,8 +1543,7 @@ void hvm_vcpu_destroy(struct vcpu *v) > tasklet_kill(&v->arch.hvm_vcpu.assert_evtchn_irq_tasklet); > hvm_funcs.vcpu_destroy(v); > > - if ( is_hvm_vcpu(v) ) > - vlapic_destroy(v); > + vlapic_destroy(v); > > hvm_vcpu_cacheattr_destroy(v); > } > @@ -1711,7 +1709,6 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla, > * - newer Windows (like Server 2012) for HPET accesses. > */ > if ( !nestedhvm_vcpu_in_guestmode(curr) > - && is_hvm_domain(currd) > && hvm_mmio_internal(gpa) ) Can this be moved to the previous line? Roger.
On 19/07/17 15:12, Roger Pau Monné wrote: > On Wed, Jul 19, 2017 at 02:27:31PM +0100, Andrew Cooper wrote: >> These functions don't need is_hvm_{vcpu,domain}() predicates. >> >> hvmop_set_evtchn_upcall_vector() does need the predicate to prevent a PV >> caller accessing the hvm union, but swap the copy_from_guest() and >> is_hvm_domain() predicate to avoid reading the hypercall parameter if we not >> going to use it. > IC, certain HVMOPs are available to PV guests (ie: the control domain). At the very least, the control domain needs to use HVMOP_getparam for construction and migration purposes. As a result, PV guests have always had blanket reign on HVMOPs. > >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > > Thanks. Just one style nit. > >> CC: George Dunlap <George.Dunlap@eu.citrix.com> >> CC: Jan Beulich <JBeulich@suse.com> >> CC: Wei Liu <wei.liu2@citrix.com> >> CC: Paul Durrant <paul.durrant@citrix.com> >> CC: Roger Pau Monné <roger.pau@citrix.com> >> --- >> xen/arch/x86/hvm/hvm.c | 15 ++++++--------- >> 1 file changed, 6 insertions(+), 9 deletions(-) >> >> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c >> index 8145385..4fef616 100644 >> --- a/xen/arch/x86/hvm/hvm.c >> +++ b/xen/arch/x86/hvm/hvm.c >> @@ -506,8 +506,7 @@ void hvm_do_resume(struct vcpu *v) >> { >> check_wakeup_from_wait(); >> >> - if ( is_hvm_domain(v->domain) ) >> - pt_restore_timer(v); >> + pt_restore_timer(v); >> >> if ( !handle_hvm_io_completion(v) ) >> return; >> @@ -1544,8 +1543,7 @@ void hvm_vcpu_destroy(struct vcpu *v) >> tasklet_kill(&v->arch.hvm_vcpu.assert_evtchn_irq_tasklet); >> hvm_funcs.vcpu_destroy(v); >> >> - if ( is_hvm_vcpu(v) ) >> - vlapic_destroy(v); >> + vlapic_destroy(v); >> >> hvm_vcpu_cacheattr_destroy(v); >> } >> @@ -1711,7 +1709,6 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla, >> * - newer Windows (like Server 2012) for HPET accesses. >> */ >> if ( !nestedhvm_vcpu_in_guestmode(curr) >> - && is_hvm_domain(currd) >> && hvm_mmio_internal(gpa) ) > Can this be moved to the previous line? Will fix on commit. ~Andrew
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 8145385..4fef616 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -506,8 +506,7 @@ void hvm_do_resume(struct vcpu *v) { check_wakeup_from_wait(); - if ( is_hvm_domain(v->domain) ) - pt_restore_timer(v); + pt_restore_timer(v); if ( !handle_hvm_io_completion(v) ) return; @@ -1544,8 +1543,7 @@ void hvm_vcpu_destroy(struct vcpu *v) tasklet_kill(&v->arch.hvm_vcpu.assert_evtchn_irq_tasklet); hvm_funcs.vcpu_destroy(v); - if ( is_hvm_vcpu(v) ) - vlapic_destroy(v); + vlapic_destroy(v); hvm_vcpu_cacheattr_destroy(v); } @@ -1711,7 +1709,6 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla, * - newer Windows (like Server 2012) for HPET accesses. */ if ( !nestedhvm_vcpu_in_guestmode(curr) - && is_hvm_domain(currd) && hvm_mmio_internal(gpa) ) { if ( !handle_mmio_with_translation(gla, gpa >> PAGE_SHIFT, npfec) ) @@ -3139,7 +3136,7 @@ static enum hvm_copy_result __hvm_copy( * - 32-bit WinXP (& older Windows) on AMD CPUs for LAPIC accesses, * - newer Windows (like Server 2012) for HPET accesses. */ - if ( v == current && is_hvm_vcpu(v) + if ( v == current && !nestedhvm_vcpu_in_guestmode(v) && hvm_mmio_internal(gpa) ) return HVMCOPY_bad_gfn_to_mfn; @@ -3971,12 +3968,12 @@ static int hvmop_set_evtchn_upcall_vector( struct domain *d = current->domain; struct vcpu *v; - if ( copy_from_guest(&op, uop, 1) ) - return -EFAULT; - if ( !is_hvm_domain(d) ) return -EINVAL; + if ( copy_from_guest(&op, uop, 1) ) + return -EFAULT; + if ( op.vector < 0x10 ) return -EINVAL;
These functions don't need is_hvm_{vcpu,domain}() predicates. hvmop_set_evtchn_upcall_vector() does need the predicate to prevent a PV caller accessing the hvm union, but swap the copy_from_guest() and is_hvm_domain() predicate to avoid reading the hypercall parameter if we not going to use it. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: George Dunlap <George.Dunlap@eu.citrix.com> CC: Jan Beulich <JBeulich@suse.com> CC: Wei Liu <wei.liu2@citrix.com> CC: Paul Durrant <paul.durrant@citrix.com> CC: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/hvm/hvm.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)