Re: [Xen-ia64-devel] [RFC][PATCH] hypercall_preempt_check() andpsr.i

[Isaku Yamahata <yamahata@xxxxxxxxxxxxx>]

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