Message ID | 20220621155134.1932383-1-akrowiak@linux.ibm.com (mailing list archive) |
---|---|
Headers | show |
Series | s390/vfio-ap: dynamic configuration support | expand |
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.
On Wed, 22 Jun 2022 08:48:27 +0200 Christian Borntraeger <borntraeger@de.ibm.com> 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
On 6/22/22 2:48 AM, Christian Borntraeger 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. Fine by me.