linux-kernel - Re: [PATCH v4 22/32] KVM: s390: pci: routines for (dis)associating zPCI devices with a KVM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 14 Mar 2022 18:46:33 -0300
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Matthew Rosato <mjrosato@...ux.ibm.com>
Cc:     linux-s390@...r.kernel.org, alex.williamson@...hat.com,
        cohuck@...hat.com, schnelle@...ux.ibm.com, farman@...ux.ibm.com,
        pmorel@...ux.ibm.com, borntraeger@...ux.ibm.com, hca@...ux.ibm.com,
        gor@...ux.ibm.com, gerald.schaefer@...ux.ibm.com,
        agordeev@...ux.ibm.com, svens@...ux.ibm.com, frankja@...ux.ibm.com,
        david@...hat.com, imbrenda@...ux.ibm.com, vneethv@...ux.ibm.com,
        oberpar@...ux.ibm.com, freude@...ux.ibm.com, thuth@...hat.com,
        pasic@...ux.ibm.com, joro@...tes.org, will@...nel.org,
        pbonzini@...hat.com, corbet@....net, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, iommu@...ts.linux-foundation.org,
        linux-doc@...r.kernel.org
Subject: Re: [PATCH v4 22/32] KVM: s390: pci: routines for (dis)associating
 zPCI devices with a KVM

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?

Jason