[SRU][Kinetic][PATCH 1/1] UBUNTU: SAUCE: (no-up) Add mdev_set_iommu_device() kABI.

[Andrea Righi]

Hello Tarun,

some comments below.

On Tue, Sep 06, 2022 at 11:38:01PM +0530, Tarun Gupta wrote:
> With below commit present from 5.16+ upstream kernel onwards, support
> mdev_set_iommu_device() kABI has been removed from kernel due to lack of
> in-tree vendor drivers using the kABI.
> 
> fda49d97f2c4 ("vfio: remove the unused mdev iommu hook")
> 
> This patch partially reverts the above commit so that
> mdev_set_iommu_device() kABI is still supported with HWE kernels
> for Ubuntu 22.04.
> 
> This kABI is used by SRIOV based Nvidia vGPU to pi all guest sysmem
> during vGPU VM boot. With this patch, SRIOV based Nvidia vGPU will
> continue to work with upstream kernels.
> 
> BugLink : https://bugs.launchpad.net/bugs/1988806
> 
> Signed-off-by: Tarun Gupta <targupta at nvidia.com>
> ---

First of all this patch seems to contain two different logical changes:
 1) the revert of fda49d97f2c4 ("vfio: remove the unused mdev iommu hook")
 2) some additional changes that are not very clear why they are needed

I think it would be more clear to split the patch in 2: the revert + the
patch with the additional changes, explaining what the additional
changes are doing and why they're required.

Otherwise, if the additional changes are required because we can't
cleanly revert the commit, please mention that in the patch description,
explaining a bit the required "adjustments".

> @@ -2180,6 +2257,25 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
>  	group->iommu_group = iommu_group;
>  
>  	if (type == VFIO_EMULATED_IOMMU) {
> +		/* Ubuntu-only - BEGIN */
> +		ret = iommu_group_for_each_dev(iommu_group, &bus, vfio_bus_type);
> +
> +		if (!ret && vfio_bus_is_mdev(bus)) {
> +			struct device *iommu_device = NULL;
> +
> +			group->mdev_group = true;
> +
> +			/* Determine the isolation type */
> +			ret = iommu_group_for_each_dev(iommu_group,
> +						       &iommu_device,
> +						       vfio_mdev_iommu_device);
> +			if (!ret && iommu_device) {
> +				bus = iommu_device->bus;
> +				goto mdev_iommu_device;
> +			}
> +		}
> +		/* Ubuntu-only - END */
> +

These 'Ubuntu-only' markers are not really adding any useful
information, we can already determine that this code is Ubuntu only,
because it's part of a SAUCE patch. I don't think there's any benefit
duplicating this information in the code, so I would drop these
comments.

Moreover (minor nitpick), we're not using "(no-up)" anymore, so we can
remove that from the subject.

Thanks,
-Andrea