Applied, thanks.
On Fri, Oct 24, 2008 at 11:02:21AM +0800, Xu, Anthony wrote:
> Isaku Yamahata wrote:
> > On Thu, Oct 23, 2008 at 03:10:55PM +0800, Xu, Anthony wrote:
> >> The new one,
> >
> > It looks almost okay. The last one.
> >
> >
> >> diff -r 02c8733e2d91 xen/arch/ia64/vmx/viosapic.c
> >> --- a/xen/arch/ia64/vmx/viosapic.c Wed Oct 22 17:20:15 2008 +0900
> >> +++ b/xen/arch/ia64/vmx/viosapic.c Thu Oct 23 14:48:09 2008 +0800 @@
> >> -121,6 +121,13 @@ redir_num, vector);
> >> return;
> >> }
> >> + if ( iommu_enabled )
> >> + {
> >> + spin_unlock(&viosapic->lock);
> >> + hvm_dpci_eoi(current->domain, redir_num,
> >> &viosapic->redirtbl[redir_num]); + spin_lock(&viosapic->lock);
> >> + }
> >> +
> >> service_iosapic(viosapic);
> >> spin_unlock(&viosapic->lock);
> >> }
> >
> > Is this unlock/lock sequence okay?
> > I'm asking simply because I'm not sure.
> >
> > viosapic->irr and isr are protected by the lock.
> > And viosapic_update_EOI() updates them atomically.
> > The above unlock/lock seems to break its atomicity.
>
> I think it is Okay,
> One atomical operation in viosapic_update_EOI is divided into two atomical
> operations.
> If you get spin_lock again, when returning from hvm_dpci_eoi.
> There are many code segments in linux kernel.
> And viosapic->irr and isr is still protected by lock.
>
> Anthony
>
> >
> > I'm not sure it's okay or not. To make sure, it is required
> > to take closer look at viosapic.c.
> >
> > thanks,
> _______________________________________________
> Xen-ia64-devel mailing list
> Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-ia64-devel
>
--
yamahata
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|