[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 1/2] iommu/amd: Fix logic for clearing the IOMMU interrupt bits

On 4/19/2013 3:18 AM, Jan Beulich wrote:
On 18.04.13 at 20:41, <suravee.suthikulpanit@xxxxxxx> wrote:
The IOMMU interrupt bits in the IOMMU status registers are
cleared when writing 1.  Therefore, the existing logic which reads
the register, set the bit, and then writing back the values
could accidentally clear certain bits if it has been set.

The correct logic would just be writing only the value which only
set the interrupt bits, and leave the rest to zeros.
So looking through the status register accesses, I would assume
that guest_iommu_mmio_write64() is broken too? In that it
clearly doesn't implement the RW1C behavior for the interrupt
and overflow bits?
Actually, I'm not quite sure why hvm guest would be writing to IOMMU mmio. I don't actually see AMD IOMMU is exposed to the hvm guest. However, isn't this function is supposed to be passing on the value intended to be written from guest? In the sense, if the guest is trying to set the bit to 1, the function will just do what it's been told to write to the register?

Further (as indicated above), the overflow bits being RW1C too,
doesn't iommu_reset_log() need a similar fix
(iommu_set_bit(&entry, of_bit) instead of
iommu_clear_bit(&entry, of_bit))?

And, perhaps even more importantly, iommu_interrupt_handler()
also ought to use iommu_set_bit()? Or wait - isn't this touching
the wrong bits altogether? Logically I would expect interrupt
enable bits to be cleared here, i.e. IOMMU_CONTROL_* to be
used throughout this function instead of IOMMU_STATUS_*.


Thanks for catching all these several places where RW1C is not properly implemented. I'll submit a new patch.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.