Re: [PATCH for-4.20? 1/3] AMD/IOMMU: drop stray MSI enabling

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 30/01/2025 11:11 am, Jan Beulich wrote:
> While the 2nd of the commits referenced below should have moved the call
> to amd_iommu_msi_enable() instead of adding another one, the situation
> wasn't quite right even before: It can't have done any good to enable
> MSI when no IRQ was allocated for it, yet.
>
> Fixes: 5f569f1ac50e ("AMD/IOMMU: allow enabling with IRQ not yet set up")
> Fixes: d9e49d1afe2e ("AMD/IOMMU: adjust setup of internal interrupt for x2APIC mode")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -902,8 +902,6 @@ static void enable_iommu(struct amd_iomm

There's a call to amd_iommu_msi_enable() just out of context here which
was added by the 2nd referenced commit.

Given that it's asymmetric in an if() condition regarding xt_en, and the
calls are only set_affinity() calls, why is this retained?

(I think I know, and if it is the reason I suspect, then you're missing
a very critical detail from the commit message.)

~Andrew


>          }
>      }
>  
> -    amd_iommu_msi_enable(iommu, IOMMU_CONTROL_ENABLED);
> -
>      set_iommu_ht_flags(iommu);
>      set_iommu_command_buffer_control(iommu, IOMMU_CONTROL_ENABLED);
>  
>