On Thu, Apr 28, 2016 at 02:06:17PM +0800, Peter Xu wrote:
> On Wed, Apr 27, 2016 at 04:31:13PM +0200, Radim Krčmář wrote:
> > >> > + */
> > >> > +static inline void
> > >> > +ioapic_fix_edge_remote_irr(uint64_t *entry)
> > >> > +{
> > >> > + if (*entry & IOAPIC_LVT_TRIGGER_MODE) {
> > >> > + /* Level triggered interrupts, make sure remote IRR is zero */
> > >> > + *entry &= ~((uint64_t)IOAPIC_LVT_REMOTE_IRR);
> > >>
> > >> (You can just unconditionally zero it, edge doesn't care.)
> > >
> > > Ah! I made a mistake. I suppose what I really want is:
> > >
> > > + if (!(*entry & IOAPIC_LVT_TRIGGER_MODE)) {
> > > + /* Edge-triggered interrupts, make sure remote IRR is zero */
> > > + *entry &= ~((uint64_t)IOAPIC_LVT_REMOTE_IRR);
> > > + }
> > >
> > > Though both should help do the trick, I should be using this new
> > > one in v5.
> >
> > (You'd need to look at the old value for this to work.)
>
> Yes, you are right. The problem is that, we actually has RW
> permission for remote IRR bit in emulated IOAPIC. If so, I'd rather
> take the original version, and unconditionally zero it, as you have
> adviced (also, will fix up the comments to get them aligned).
After a second thought, a better idea (though may need several more
lines of codes) is to make sure the RO bits in IOAPIC entry are
read-only (I mean, "real" read-only) before the above hack. I
suppose this further matches with real hardware behavior.
Let me send v5 directly to see the codes.
Thanks,
-- peterx
