On Wed, 22 Jun 2022 08:48:27 +0200 Christian Borntraeger <borntraeger@xxxxxxxxxx> wrote: > Am 21.06.22 um 17:51 schrieb Tony Krowiak: > > The current design for AP pass-through does not support making dynamic > > changes to the AP matrix of a running guest resulting in a few > > deficiencies this patch series is intended to mitigate: > > > > 1. Adapters, domains and control domains can not be added to or removed > > from a running guest. In order to modify a guest's AP configuration, > > the guest must be terminated; only then can AP resources be assigned > > to or unassigned from the guest's matrix mdev. The new AP > > configuration becomes available to the guest when it is subsequently > > restarted. > > > > 2. The AP bus's /sys/bus/ap/apmask and /sys/bus/ap/aqmask interfaces can > > be modified by a root user without any restrictions. A change to > > either mask can result in AP queue devices being unbound from the > > vfio_ap device driver and bound to a zcrypt device driver even if a > > guest is using the queues, thus giving the host access to the guest's > > private crypto data and vice versa. > > > > 3. The APQNs derived from the Cartesian product of the APIDs of the > > adapters and APQIs of the domains assigned to a matrix mdev must > > reference an AP queue device bound to the vfio_ap device driver. The > > AP architecture allows assignment of AP resources that are not > > available to the system, so this artificial restriction is not > > compliant with the architecture. > > > > 4. The AP configuration profile can be dynamically changed for the linux > > host after a KVM guest is started. For example, a new domain can be > > dynamically added to the configuration profile via the SE or an HMC > > connected to a DPM enabled lpar. Likewise, AP adapters can be > > dynamically configured (online state) and deconfigured (standby state) > > using the SE, an SCLP command or an HMC connected to a DPM enabled > > lpar. This can result in inadvertent sharing of AP queues between the > > guest and host. > > > > 5. A root user can manually unbind an AP queue device representing a > > queue in use by a KVM guest via the vfio_ap device driver's sysfs > > unbind attribute. In this case, the guest will be using a queue that > > is not bound to the driver which violates the device model. > > > > This patch series introduces the following changes to the current design > > to alleviate the shortcomings described above as well as to implement > > more of the AP architecture: > > > > 1. A root user will be prevented from making edits to the AP bus's > > /sys/bus/ap/apmask or /sys/bus/ap/aqmask if the change would transfer > > ownership of an APQN from the vfio_ap device driver to a zcrypt driver > > while the APQN is assigned to a matrix mdev. > > > > 2. Allow a root user to hot plug/unplug AP adapters, domains and control > > domains for a KVM guest using the matrix mdev via its sysfs > > assign/unassign attributes. > > > > 4. Allow assignment of an AP adapter or domain to a matrix mdev even if > > it results in assignment of an APQN that does not reference an AP > > queue device bound to the vfio_ap device driver, as long as the APQN > > is not reserved for use by the default zcrypt drivers (also known as > > over-provisioning of AP resources). Allowing over-provisioning of AP > > resources better models the architecture which does not preclude > > assigning AP resources that are not yet available in the system. Such > > APQNs, however, will not be assigned to the guest using the matrix > > mdev; only APQNs referencing AP queue devices bound to the vfio_ap > > device driver will actually get assigned to the guest. > > > > 5. Handle dynamic changes to the AP device model. > > > > 1. Rationale for changes to AP bus's apmask/aqmask interfaces: > > ---------------------------------------------------------- > > Due to the extremely sensitive nature of cryptographic data, it is > > imperative that great care be taken to ensure that such data is secured. > > Allowing a root user, either inadvertently or maliciously, to configure > > these masks such that a queue is shared between the host and a guest is > > not only avoidable, it is advisable. It was suggested that this scenario > > is better handled in user space with management software, but that does > > not preclude a malicious administrator from using the sysfs interfaces > > to gain access to a guest's crypto data. It was also suggested that this > > scenario could be avoided by taking access to the adapter away from the > > guest and zeroing out the queues prior to the vfio_ap driver releasing the > > device; however, stealing an adapter in use from a guest as a by-product > > of an operation is bad and will likely cause problems for the guest > > unnecessarily. It was decided that the most effective solution with the > > least number of negative side effects is to prevent the situation at the > > source. > > > > 2. Rationale for hot plug/unplug using matrix mdev sysfs interfaces: > > ---------------------------------------------------------------- > > Allowing a user to hot plug/unplug AP resources using the matrix mdev > > sysfs interfaces circumvents the need to terminate the guest in order to > > modify its AP configuration. Allowing dynamic configuration makes > > reconfiguring a guest's AP matrix much less disruptive. > > > > 3. Rationale for allowing over-provisioning of AP resources: > > ----------------------------------------------------------- > > Allowing assignment of AP resources to a matrix mdev and ultimately to a > > guest better models the AP architecture. The architecture does not > > preclude assignment of unavailable AP resources. If a queue subsequently > > becomes available while a guest using the matrix mdev to which its APQN > > is assigned, the guest will be given access to it. If an APQN > > is dynamically unassigned from the underlying host system, it will > > automatically become unavailable to the guest. > > > > Change log v19-v20: > > ------------------ > > * Fixed patch 02/20: failed to move creation of status attribute > > for a queue device to the vfio_ap_mdev_probe_queue function in > > drivers/s390/crypto/vfio_ap_ops.c. (Jason) > > > > * Fixed signature of get_update_locks_for_queue macro > > > > * Take lock in get_update_locks_for_queue macro before > > accessing q->matrix_mdev > > > > * Renamed vfio_ap_mdev_get_update_locks_for_apqn function to > > get_update_locks_for_apqn (Jason) > > > > * Fix comments in function implementing the AP bus's in_use callback (Jason) > > > > * Fix function name in prologue for ap_owned_by_def_drv function > > > > Tony Krowiak (20): > > s390/vfio-ap: use new AP bus interface to search for queue devices > > s390/vfio-ap: move probe and remove callbacks to vfio_ap_ops.c > > s390/vfio-ap: manage link between queue struct and matrix mdev > > s390/vfio-ap: introduce shadow APCB > > s390/vfio-ap: refresh guest's APCB by filtering AP resources assigned > > to mdev > > s390/vfio-ap: allow assignment of unavailable AP queues to mdev device > > s390/vfio-ap: rename matrix_dev->lock mutex to matrix_dev->mdevs_lock > > s390/vfio-ap: introduce new mutex to control access to the KVM pointer > > s390/vfio-ap: use proper locking order when setting/clearing KVM > > pointer > > s390/vfio-ap: prepare for dynamic update of guest's APCB on > > assign/unassign > > s390/vfio-ap: prepare for dynamic update of guest's APCB on queue > > probe/remove > > s390/vfio-ap: allow hot plug/unplug of AP devices when > > assigned/unassigned > > s390/vfio-ap: hot plug/unplug of AP devices when probed/removed > > s390/vfio-ap: reset queues after adapter/domain unassignment > > s390/vfio-ap: implement in-use callback for vfio_ap driver > > s390/vfio-ap: sysfs attribute to display the guest's matrix > > s390/vfio-ap: handle config changed and scan complete notification > > s390/vfio-ap: update docs to include dynamic config support > > s390/Docs: new doc describing lock usage by the vfio_ap device driver > > MAINTAINERS: pick up all vfio_ap docs for VFIO AP maintainers > > > > Documentation/s390/vfio-ap-locking.rst | 105 ++ > > Documentation/s390/vfio-ap.rst | 492 +++++--- > > MAINTAINERS | 2 +- > > drivers/s390/crypto/ap_bus.c | 35 +- > > drivers/s390/crypto/vfio_ap_drv.c | 124 +- > > drivers/s390/crypto/vfio_ap_ops.c | 1436 ++++++++++++++++++------ > > drivers/s390/crypto/vfio_ap_private.h | 47 +- > > 7 files changed, 1648 insertions(+), 593 deletions(-) > > create mode 100644 Documentation/s390/vfio-ap-locking.rst > > Unless somebody disagrees, I think we will carry these patches via the s390 tree. No objections. I failed to verify that everything is in great shape (no deadlocks, no races, and the logic is implemented like designed) because I didn't have the bandwidth. I did spend some time looking at the locking in v19 and I didn't find any problems. The locking design is IMHO viable. I think Jason did a full review and r-b-ed all the patches or is at least very close to it. I don't think it makes any sense to wait for me. Sorry I couldn't do as much as I would have liked to. Regards, Halil
