Re: [Xen-devel] [RFC v1 12/15] vmx: Properly handle notification event when vCPU is running

["Wu, Feng" <feng.wu@xxxxxxxxx>]

> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@xxxxxxxxxx]
> Sent: Friday, April 03, 2015 9:37 PM
> To: Wu, Feng
> Cc: Tian, Kevin; Zhang, Yang Z; xen-devel@xxxxxxxxxxxxx; keir@xxxxxxx;
> JBeulich@xxxxxxxx
> Subject: Re: [Xen-devel] [RFC v1 12/15] vmx: Properly handle notification 
> event
> when vCPU is running
> 
> On Fri, Apr 03, 2015 at 02:00:24AM +0000, Wu, Feng wrote:
> >
> >
> > > -----Original Message-----
> > > From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@xxxxxxxxxx]
> > > Sent: Friday, April 03, 2015 3:15 AM
> > > To: Tian, Kevin
> > > Cc: Wu, Feng; Zhang, Yang Z; xen-devel@xxxxxxxxxxxxx; keir@xxxxxxx;
> > > JBeulich@xxxxxxxx
> > > Subject: Re: [Xen-devel] [RFC v1 12/15] vmx: Properly handle notification
> event
> > > when vCPU is running
> > >
> > > On Thu, Apr 02, 2015 at 06:08:12AM +0000, Tian, Kevin wrote:
> > > > > From: Wu, Feng
> > > > > Sent: Friday, March 27, 2015 12:58 PM
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Zhang, Yang Z
> > > > > > Sent: Friday, March 27, 2015 12:44 PM
> > > > > > To: Wu, Feng; xen-devel@xxxxxxxxxxxxx
> > > > > > Cc: JBeulich@xxxxxxxx; keir@xxxxxxx; Tian, Kevin
> > > > > > Subject: RE: [RFC v1 12/15] vmx: Properly handle notification event
> when
> > > > > vCPU
> > > > > > is running
> > > > > >
> > > > > > Wu, Feng wrote on 2015-03-27:
> > > > > > >
> > > > > > >
> > > > > > > Zhang, Yang Z wrote on 2015-03-25:
> > > > > > >> when vCPU is running
> > > > > > >>
> > > > > > >> Wu, Feng wrote on 2015-03-25:
> > > > > > >>> When a vCPU is running in Root mode and a notification event has
> > > > > > >>> been injected to it. we need to set VCPU_KICK_SOFTIRQ for the
> > > > > > >>> current cpu, so the pending interrupt in PIRR will be synced to
> > > > > > >>> vIRR before
> > >
> > > This would imply that we had VMEXIT-ed due to pending interrupt? And we
> > > end up calling 'do_IRQ'? If so then the DPCI_SOFTIRQ ends up being set
> > > and you stll end up calling the softirq code?
> >
> > No.
> >
> > Here is the scenario for the description of this patch:
> >
> > When vCPU is running in root-mode (such as via hypercall, or any other
> > reasons which can result in VM-Exit), and before vCPU is back to non-root,
> > external interrupts happen. Notice that the VM-exit is not caused by this
> > external interrupt.
> 
> Thank you for the explanation. You might want to add that in the commit
> along with the explanation of the code flow below!

Good idea! Thank you!

Thanks,
Feng

> 
> >
> > Thanks,
> > Feng
> >
> > >
> > > > > > > VM-Exit in time.
> > > > > > >>
> > > > > > >> Shouldn't the pending interrupt be synced unconditionally before
> next
> > > > > > >> vmentry? What happens if we didn't set the softirq?
> > > > > > >
> > > > > > > If we didn't set the softirq in the notification handler, the
> > > > > > > interrupts happened exactly before VM-entry cannot be delivered to
> > > > > > > guest at this time. Please see the following code fragments from
> > > > > > > xen/arch/x86/hvm/vmx/entry.S: (pls pay attention to the comments)
> > > > > > >
> > > > > > > .Lvmx_do_vmentry
> > > > > > >
> > > > > > > ......
> > > > > > >           /* If Vt-d engine issues a notification event here,
> > > > > > >          * it cannot be delivered to guest during this VM-entry
> > > > > > >          * without raising the softirq in notification handler. */
> > > > > > >         cmp  %ecx,(%rdx,%rax,1)
> > > > > > >         jnz  .Lvmx_process_softirqs
> > > > > > > ......
> > > > > > >
> > > > > > >         je   .Lvmx_launch
> > > > > > > ......
> > > > > > >
> > > > > > >
> > > > > > > .Lvmx_process_softirqs:
> > > > > > >         sti
> > > > > > >         call do_softirq
> > > > > > >         jmp  .Lvmx_do_vmentry
> > > > > >
> > > > > > You are right! This helps me to recall why raise the softirq when
> delivering
> > > > > the
> > > > > > PI.
> > > > >
> > > > > Yes, __vmx_deliver_posted_interrupt() is the software way to deliver 
> > > > > PI,
> it
> > > sets
> > > > > the
> > > > > softirq for this purpose, however, when VT-d HW delivers PI, we have 
> > > > > no
> > > > > control to
> > > > > the HW itself, hence we need to set this softirq in the Notification 
> > > > > Event
> > > > > handler.
> > > > >
> > > >
> > > > could you include this information in the comment so others can easily
> > > > understand this requirement? from code you only mentioned VCPU_KICK
> > > > _SOFTIRQ is required, but how it leads to PIRR->VIRR sync is not 
> > > > explained.
> > > >
> > > > Thanks
> > > > Kevin
> > > >
> > > > _______________________________________________
> > > > Xen-devel mailing list
> > > > Xen-devel@xxxxxxxxxxxxx
> > > > http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel