Re: [XEN PATCH v3] amd/iommu: clean up unused guest iommu related functions

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.03.2024 16:55, Nicola Vetrini wrote:
> Delete unused functions from 'iommu_guest.c'.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> ---
> guest_iommu_add_ptr_log has still one caller, but even that seems
> suspicious.

As said, it should be dropped. You remove ...

> -/* Domain specific initialization */
> -int guest_iommu_init(struct domain* d)
> -{
> -    struct guest_iommu *iommu;
> -    struct domain_iommu *hd = dom_iommu(d);
> -
> -    if ( !is_hvm_domain(d) || !is_iommu_enabled(d) || !iommuv2_enabled ||
> -         !has_viommu(d) )
> -        return 0;
> -
> -    iommu = xzalloc(struct guest_iommu);
> -    if ( !iommu )
> -    {
> -        AMD_IOMMU_DEBUG("Error allocating guest iommu structure.\n");
> -        return 1;
> -    }
> -
> -    guest_iommu_reg_init(iommu);
> -    iommu->mmio_base = ~0ULL;
> -    iommu->domain = d;
> -    hd->arch.amd.g_iommu = iommu;

... the only place setting  to non-NULL. With that the 2nd if() in
guest_iommu_add_ppr_log() is guaranteed to cause an early return, at which
point the function is meaningless to call.

But yeah, we can of course clean that up in a 2nd step. It's not like in
order to make a new attempt we would be very likely to simply revert the
changes to drop all of this code (in which case it being a single commit
would make things easier). It very likely wants re-writing from scratch.
Hence on top of Stefano's R-b:
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan