Re: [RFC PATCH] xen/amd-iommu: Add interrupt remapping quirk for ath11k

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 26/02/2025 9:11 pm, Jason Andryuk wrote:
> @@ -316,6 +327,19 @@ static void apply_quirks(struct pci_dev *pdev)
>               * from trying to size the BARs or add handlers to trap accesses.
>               */
>              pdev->ignore_bars = true;
> +
> +    for ( i = 0; i < ARRAY_SIZE(hide_irt); i++)
> +    {
> +        if ( vendor == hide_irt[i].vendor &&
> +             device == hide_irt[i].device )
> +        {
> +            pdev->gvec_as_irte_idx = true;
> +            printk("%pp %04x:%04x quirk gvec as intr remap index\n",
> +                   &pdev->sbdf, hide_irt[i].vendor, hide_irt[i].device);
> +            if ( !amd_iommu_perdev_intremap )
> +                printk("gvec quirk requires per-device intr remap!\n");

(Covering only what others haven't.)

amd_iommu_perdev_intremap is the subject of XSA-36.

Sadly it still exists, and only because Xen does not have a viable
IOMMU-groups model, so can only fix amd_sp5100_erratum28() by turning
disabling the XSA-36 fix and switching back into one fully-shared
interrupt remapping table.

This is of course horrible.  The proper fix is to put the IDE and SATA
controller into the same IOMMU group (force them to be handled as a
unit) at which point *they* can share a intremap table while the rest of
the system uses unique ones.   (Also, disabling the IOMMUs globally
because perdev remapping was disabled and sata combined mode is active,
is a truly unacceptable thing to do.)

Unfortunately, the Intel problems are relevant here.

amd_iommu_perdev_intremap exists because it was trying to copy how Intel
works.  Intel IOMMUs have a single interrupt remapping table shared by
all devices behind it.  Then Intel realised this was a giant security
vulnerability, and introduced the concept of Source ID verification, to
fix a problem which only exists because the remapping table was shared
to begin with.

On AMD, because we have per domain tables, we allocate in order simply
because it's easy.  And yes, we can allocate out-of-order to match other
setups.

But on Intel, you can't.  The table, and therefore the indices in it,
are shared.

In principle, if you only have a single ath11k device behind the IOMMU,
you could shuffle around existing entries to make holes where you want
them, but this can't be done atomically and you risk interfering with an
active device.

You might get somewhere with allocating top-down in the table by default
and leaving the bottom for magic like this?  But then you've still got
to fix the remappable-form problem that Roger pointed out.

~Andrew