Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <3cc6dff9-0346-c449-249f-5812b3df379c@linux.ibm.com>
Date:   Tue, 15 Mar 2022 12:39:13 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.6.0
Subject: Re: [PATCH v4 22/32] KVM: s390: pci: routines for (dis)associating
 zPCI devices with a KVM
Content-Language: en-US
To:     Jason Gunthorpe <jgg@nvidia.com>
Cc:     linux-s390@vger.kernel.org, alex.williamson@redhat.com,
        cohuck@redhat.com, schnelle@linux.ibm.com, farman@linux.ibm.com,
        pmorel@linux.ibm.com, borntraeger@linux.ibm.com, hca@linux.ibm.com,
        gor@linux.ibm.com, gerald.schaefer@linux.ibm.com,
        agordeev@linux.ibm.com, svens@linux.ibm.com, frankja@linux.ibm.com,
        david@redhat.com, imbrenda@linux.ibm.com, vneethv@linux.ibm.com,
        oberpar@linux.ibm.com, freude@linux.ibm.com, thuth@redhat.com,
        pasic@linux.ibm.com, joro@8bytes.org, will@kernel.org,
        pbonzini@redhat.com, corbet@lwn.net, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org,
        linux-doc@vger.kernel.org
References: <20220314194451.58266-1-mjrosato@linux.ibm.com>
 <20220314194451.58266-23-mjrosato@linux.ibm.com>
 <20220314214633.GJ11336@nvidia.com>
From:   Matthew Rosato <mjrosato@linux.ibm.com>
In-Reply-To: <20220314214633.GJ11336@nvidia.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 3/14/22 5:46 PM, Jason Gunthorpe wrote:
> On Mon, Mar 14, 2022 at 03:44:41PM -0400, Matthew Rosato wrote:
>> +int kvm_s390_pci_zpci_start(struct kvm *kvm, struct zpci_dev *zdev)
>> +{
>> +	struct vfio_device *vdev;
>> +	struct pci_dev *pdev;
>> +	int rc;
>> +
>> +	rc = kvm_s390_pci_dev_open(zdev);
>> +	if (rc)
>> +		return rc;
>> +
>> +	pdev = pci_get_slot(zdev->zbus->bus, zdev->devfn);
>> +	if (!pdev) {
>> +		rc = -ENODEV;
>> +		goto exit_err;
>> +	}
>> +
>> +	vdev = get_vdev(&pdev->dev);
>> +	if (!vdev) {
>> +		pci_dev_put(pdev);
>> +		rc = -ENODEV;
>> +		goto exit_err;
>> +	}
>> +
>> +	zdev->kzdev->nb.notifier_call = kvm_s390_pci_group_notifier;
>> +
>> +	/*
>> +	 * At this point, a KVM should already be associated with this device,
>> +	 * so registering the notifier now should immediately trigger the
>> +	 * event.  We also want to know if the KVM association is later removed
>> +	 * to ensure proper cleanup happens.
>> +	 */
>> +	rc = register_notifier(vdev->dev, &zdev->kzdev->nb);
>> +
>> +	put_vdev(vdev);
>> +	pci_dev_put(pdev);
>> +
>> +	/* Make sure the registered KVM matches the KVM issuing the ioctl */
>> +	if (rc || zdev->kzdev->kvm != kvm) {
>> +		rc = -ENODEV;
>> +		goto exit_err;
>> +	}
>> +
>> +	/* Must support KVM-managed IOMMU to proceed */
>> +	if (IS_ENABLED(CONFIG_S390_KVM_IOMMU))
>> +		rc = zpci_iommu_attach_kvm(zdev, kvm);
>> +	else
>> +		rc = -EINVAL;
> 
> This seems like kind of a strange API, shouldn't kvm be getting a
> reference on the underlying iommu_domain and then calling into it to
> get the mapping table instead of pushing KVM specific logic into the
> iommu driver?
> 
> I would be nice if all the special kvm stuff could more isolated in
> kvm code.
> 
> I'm still a little unclear about why this is so complicated - can't
> you get the iommu_domain from the group FD directly in KVM code as
> power does?

Yeah, I think I could do something like that using the vfio group fd 
like power does.

Providing a reference to the kvm itself inside iommu was being used for 
the pin/unpin operations, which would not be necessary if we switched to 
the 1st layer iommu pinning all of guest memory.