The two are really meant to be independent settings; iov_supports_xt()
using || instead of && was simply wrong. The corrected check is,
however, redundant, just like the (correct) one in iov_detect(): These
hook functions are unreachable without acpi_ivrs_init() installing the
iommu_init_ops pointer, which it does only upon success. (Unlike for
VT-d there is no late clearing of iommu_enable due to quirks, and any
possible clearing of iommu_intremap happens only after iov_supports_xt()
has run.)
Signed-off-by: Jan Beulich <jbeul...@suse.com>
---
In fact in iov_detect() it could be iommu_enable alone which gets
checked, but this felt overly aggressive to me. Instead I'm getting the
impression that the function may wrongly not get called when "iommu=off"
but interrupt remapping is in use: We'd not get the interrupt handler
installed, and hence interrupt remapping related events would never get
reported. (Same on VT-d, FTAOD.)
For iov_supports_xt() the question is whether, like VT-d's
intel_iommu_supports_eim(), it shouldn't rather check iommu_intremap
alone (in which case it would need to remain a check rather than getting
converted to ASSERT()).
---
v2: New.
--- a/xen/drivers/passthrough/amd/iommu_intr.c
+++ b/xen/drivers/passthrough/amd/iommu_intr.c
@@ -731,8 +731,7 @@ bool __init iov_supports_xt(void)
{
unsigned int apic;
- if ( !iommu_enable || !iommu_intremap )
- return false;
+ ASSERT(iommu_enable || iommu_intremap);
if ( amd_iommu_prepare(true) )
return false;
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -199,8 +199,7 @@ int __init acpi_ivrs_init(void)
static int __init iov_detect(void)
{
- if ( !iommu_enable && !iommu_intremap )
- return 0;
+ ASSERT(iommu_enable || iommu_intremap);
if ( (init_done ? amd_iommu_init_late()
: amd_iommu_init(false)) != 0 )
