Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU

[Peter Xu]

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