[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: xen spl rework
Taking off some of the lists on Cc: to reduce noise.....
Thanks for taking the time to review this.
On 11 November 2012 18:21, Jean-Yves Migeon <jeanyves.migeon%free.fr@localhost>
> Le 10/11/12 19:15, Cherry G. Mathew a écrit :
>>> Some comments:
>>> - remove hypervisor_enable_ipl from headers, it does not seem to be used
>>> - if you are in a cleanup process, have a consistent clear/set interrupt
>>> flag function for x86 and xen. Use x86_enable/disable_intr() stubs, and
>>> not spread cli()/sti() directly. It's easier to grep/grok for x86_
>>> - regarding XXX comments:
>> I've reworked this in stages, here's the first bit (C only) that
>> addresses some of your suggestions:
> I took a look at it this afternoon and code looks fine to me. But this goes
> a bit further than s/sti/x86_enable_intr.
> - typo line 14, you have an extra space to the function prototype
> - line 206, shouldn't it be better to set the pending IPL when interrupts
> are disabled (hence, after cli) rather than before? It can silently mask the
> interrupt if one is raised before returning from hypervisor_set_ipending,
> (I guess that the mutex for the affected evtchn will protect the call
> anyway, but I prefer asking first).
Thanks for catching this - I've committed a fix for this:
> - line 218, the KASSERT() is not needed given the instruction just above.
I've updated the patch with your suggestions.
Thanks again for the review!
Main Index |
Thread Index |