[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH for-4.13 v3 2/2] x86/vmx: always sync PIR to IRR before vmentry
On 26.11.2019 14:26, Roger Pau Monne wrote: > --- a/xen/arch/x86/hvm/irq.c > +++ b/xen/arch/x86/hvm/irq.c > @@ -515,7 +515,11 @@ void hvm_set_callback_via(struct domain *d, uint64_t via) > struct hvm_intack hvm_vcpu_has_pending_irq(struct vcpu *v) > { > struct hvm_domain *plat = &v->domain->arch.hvm; > - int vector; > + /* > + * Always call vlapic_has_pending_irq so that PIR is synced into IRR when > + * using posted interrupts. > + */ > + int vector = vlapic_has_pending_irq(v); Did you consider doing this conditionally either here ... > @@ -530,7 +534,6 @@ struct hvm_intack hvm_vcpu_has_pending_irq(struct vcpu *v) > if ( vlapic_accept_pic_intr(v) && plat->vpic[0].int_output ) > return hvm_intack_pic(0); > > - vector = vlapic_has_pending_irq(v); > if ( vector != -1 ) > return hvm_intack_lapic(vector); ... or here? I ask not only because the function isn't exactly cheap to call (as iirc you did also mention during the v2 discussion), but also because of its interaction with Viridian and nested mode. In case of problems there, avoiding the use of interrupt posting would be a workaround in such cases then. > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -1945,57 +1945,31 @@ static void vmx_process_isr(int isr, struct vcpu *v) > > static void __vmx_deliver_posted_interrupt(struct vcpu *v) > { > - bool_t running = v->is_running; > - > vcpu_unblock(v); > - /* > - * Just like vcpu_kick(), nothing is needed for the following two cases: > - * 1. The target vCPU is not running, meaning it is blocked or runnable. > - * 2. The target vCPU is the current vCPU and we're in non-interrupt > - * context. > - */ > - if ( running && (in_irq() || (v != current)) ) > - { > + if ( v->is_running && v != current ) I continue to be concerned by this evaluation of ->is_running _after_ vcpu_unblock(), when previously (just like vcpu_kick() does) it was intentionally done before. I wonder anyway whether this and the change to irq.c should really be in a single patch, the more that you start the respective part of the description with "While there also simplify ...". But in the end it is up to Kevin's or Jun's judgement anyway. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |