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).

-- peterx

Reply via email to