Eric,
On Mon, 25 Jul 2016, Auger Eric wrote:
> On 20/07/2016 11:09, Thomas Gleixner wrote:
> > On Tue, 19 Jul 2016, Eric Auger wrote:
> >> @@ -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
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
