Hi.
I think Kevin's patch is better.
In fact my patch was a hack. So I called it work around and used RFC.
- I wanted to work on balloon driver instead of this issue.
- Event channel would be changed.
I hoped the change will solve the issue when I posted my patch.
But Kevin solved it.
thnaks.
On Thu, Mar 30, 2006 at 09:04:05AM +0800, Tian, Kevin wrote:
> >From: Alex Williamson [mailto:alex.williamson@xxxxxx]
> >Sent: 2006年3月30日 2:55
> >On Wed, 2006-03-29 at 22:14 +0800, Tian, Kevin wrote:
> >> Hi, Isaku,
> >> Seems we're shooting same issue by different way. In the
> >beginning,
> >> I also came up same simple approach as yours. However after more
> >> thinking, I think it's better to tune xen/ia64 to adapt to common concept
> >> where evtchn_upcall_mask is the flag whether events/interrupts can be
> >injected into guest. Or else we have to add similar #ifdef as yours time to
> >> time, which especially only be observed when important bugs are
> >tracked
> >> down after many debugs.
> >
> > The simplicity and compact-ness of Isaku's patch is certainly
> >appealing. What if instead of fixing this one case with a #ifdef we
> >were to something like this:
> >
> >#define event_pending(v) \
> > ((!!(v)->vcpu_info->evtchn_upcall_pending & \
> > !(v)->vcpu_info->evtchn_upcall_mask)) && \
> > arch_event_pending(v))
> >
> >x86:
> >
> >#define arch_event_pending(v) (1)
> >
> >ia64:
> >
> >#define arch_event_pending(v) (PSCB(v, interrupt_delivery_enabled))
> >
> >Thanks,
> >
> > Alex
> >
>
> Yes, I agree Isaku's patch is simpler, which however still leaves window
> for similar bug to jump out in the future. If we still keep two flags:
> - There're other common places to operate evtchn_upcall_mask
> directly, which should be changed like this too
> - This breaks VTI domain, since VTI domain has no separated
> interrupt enable bit (instead a full PSR)
> - Most important, as I said in another paragraph, soon we'll change
> the model to event channel mechanism where external interrupt is
> eliminated and all pirq/virq/ipi are bound to events. At that time, two
> similar flags can't be updated atomically and dangerous window may be
> left there.
>
> Due to above reasons, there's no need to change common code since
> finally there'll be no difference from xen/ia64 side.
>
> Maybe I should put it in this way, my patch is bigger as a necessary step
> to merge two flags for correctness issue which is required by coming
> event channel changes. As a side effect, that bug is also solved. :-)
>
> Thanks,
> Kevin
>
> _______________________________________________
> 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
|