Eric,

On Mon, 25 Jul 2016, Auger Eric wrote:

On 20/07/2016 11:09, Thomas Gleixner wrote:

quoted

On Tue, 19 Jul 2016, Eric Auger wrote:

quoted

@@ -63,10 +63,18 @@ static int msi_compose(struct irq_data *irq_data,
 {
 	int ret = 0;
 
-	if (erase)
+	if (erase) {
 		memset(msg, 0, sizeof(*msg));
-	else
+	} else {
+		struct device *dev;
+
 		ret = irq_chip_compose_msi_msg(irq_data, msg);
+		if (ret)
+			return ret;
+
+		dev = msi_desc_to_dev(irq_data_get_msi_desc(irq_data));
+		WARN_ON(iommu_msi_msg_pa_to_va(dev, msg));

What the heck is this call doing? And why is there only a WARN_ON and not a
proper error return code handling?

iommu_msi_msg_pa_to_va is part of the new iommu-msi API introduced in PART I
of this series. This helper function detects the physical address found in
the MSI message has a corresponding allocated IOVA. This happens if the MSI
doorbell is accessed through an IOMMU and this IOMMU do not bypass the MSI
addresses (ARM case). Allocation of this IOVA was performed in the previous
patch.

So, if this is the case, the physical address is swapped with the IOVA
address. That way the PCIe device will send the MSI with this IOVA and
the address will be translated by the IOMMU into the target MSI doorbell PA.

Hope this clarifies

No, it does not. You are explaining in great length what that function is
doing, but you are not explaining WHY your don't do a proper return code
handling and just do a WARN_ON() and happily proceed. If that function fails
then the interrupt will not be functional, so WHY on earth are you continuing?

Thanks,

	tglx