From 19032628bd7ce8a39cdf0521b6418bf88c25ec80 Mon Sep 17 00:00:00 2001 From: Kunwu Chan Date: Mon, 15 Jan 2024 14:34:34 +0800 Subject: vfio/pci: WARN_ON driver_override kasprintf failure kasprintf() returns a pointer to dynamically allocated memory which can be NULL upon failure. This is a blocking notifier callback, so errno isn't a proper return value. Use WARN_ON to small allocation failures. Signed-off-by: Kunwu Chan Link: https://lore.kernel.org/r/20240115063434.20278-1-chentao@kylinos.cn Signed-off-by: Alex Williamson --- drivers/vfio/pci/vfio_pci_core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index 1cbc990d42e0..61aa19666050 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -2047,6 +2047,7 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb, pci_name(pdev)); pdev->driver_override = kasprintf(GFP_KERNEL, "%s", vdev->vdev.ops->name); + WARN_ON(!pdev->driver_override); } else if (action == BUS_NOTIFY_BOUND_DRIVER && pdev->is_virtfn && physfn == vdev->pdev) { struct pci_driver *drv = pci_dev_driver(pdev); -- cgit v1.2.3 From 1cbcb564f5b67cee2fc2f78132b9733118a79c6d Mon Sep 17 00:00:00 2001 From: Yishai Hadas Date: Mon, 5 Feb 2024 14:48:24 +0200 Subject: net/mlx5: Add the IFC related bits for query tracker Add the IFC related bits for query tracker. Signed-off-by: Yishai Hadas Reviewed-by: Kevin Tian Acked-by: Leon Romanovsky Link: https://lore.kernel.org/r/20240205124828.232701-2-yishaih@nvidia.com Signed-off-by: Alex Williamson --- include/linux/mlx5/mlx5_ifc.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h index c726f90ab752..0e513e372bf0 100644 --- a/include/linux/mlx5/mlx5_ifc.h +++ b/include/linux/mlx5/mlx5_ifc.h @@ -12672,6 +12672,11 @@ struct mlx5_ifc_modify_page_track_obj_in_bits { struct mlx5_ifc_page_track_bits obj_context; }; +struct mlx5_ifc_query_page_track_obj_out_bits { + struct mlx5_ifc_general_obj_out_cmd_hdr_bits general_obj_out_cmd_hdr; + struct mlx5_ifc_page_track_bits obj_context; +}; + struct mlx5_ifc_msecq_reg_bits { u8 reserved_at_0[0x20]; -- cgit v1.2.3 From f886473071d6c0c98857eb5d7871a5e5ac26e950 Mon Sep 17 00:00:00 2001 From: Yishai Hadas Date: Mon, 5 Feb 2024 14:48:25 +0200 Subject: vfio/mlx5: Add support for tracker object change event Add support for tracker object change event by referring to its MLX5_EVENT_TYPE_OBJECT_CHANGE event when occurs. This lets the driver recognize whether the firmware moved the tracker object to an error state. In that case, the driver will skip/block any usage of that object including an early exit in case the object was previously marked with an error. This functionality also covers the case when no CQE is delivered as of the error state. The driver was adapted to the device specification to handle the above. Signed-off-by: Yishai Hadas Reviewed-by: Kevin Tian Acked-by: Leon Romanovsky Link: https://lore.kernel.org/r/20240205124828.232701-3-yishaih@nvidia.com Signed-off-by: Alex Williamson --- drivers/vfio/pci/mlx5/cmd.c | 48 +++++++++++++++++++++++++++++++++++++++++++++ drivers/vfio/pci/mlx5/cmd.h | 1 + 2 files changed, 49 insertions(+) diff --git a/drivers/vfio/pci/mlx5/cmd.c b/drivers/vfio/pci/mlx5/cmd.c index efd1d252cdc9..8a39ff19da28 100644 --- a/drivers/vfio/pci/mlx5/cmd.c +++ b/drivers/vfio/pci/mlx5/cmd.c @@ -149,6 +149,12 @@ int mlx5vf_cmd_query_vhca_migration_state(struct mlx5vf_pci_core_device *mvdev, return 0; } +static void set_tracker_change_event(struct mlx5vf_pci_core_device *mvdev) +{ + mvdev->tracker.object_changed = true; + complete(&mvdev->tracker_comp); +} + static void set_tracker_error(struct mlx5vf_pci_core_device *mvdev) { /* Mark the tracker under an error and wake it up if it's running */ @@ -900,6 +906,29 @@ static int mlx5vf_cmd_modify_tracker(struct mlx5_core_dev *mdev, return mlx5_cmd_exec(mdev, in, sizeof(in), out, sizeof(out)); } +static int mlx5vf_cmd_query_tracker(struct mlx5_core_dev *mdev, + struct mlx5_vhca_page_tracker *tracker) +{ + u32 out[MLX5_ST_SZ_DW(query_page_track_obj_out)] = {}; + u32 in[MLX5_ST_SZ_DW(general_obj_in_cmd_hdr)] = {}; + void *obj_context; + void *cmd_hdr; + int err; + + cmd_hdr = MLX5_ADDR_OF(modify_page_track_obj_in, in, general_obj_in_cmd_hdr); + MLX5_SET(general_obj_in_cmd_hdr, cmd_hdr, opcode, MLX5_CMD_OP_QUERY_GENERAL_OBJECT); + MLX5_SET(general_obj_in_cmd_hdr, cmd_hdr, obj_type, MLX5_OBJ_TYPE_PAGE_TRACK); + MLX5_SET(general_obj_in_cmd_hdr, cmd_hdr, obj_id, tracker->id); + + err = mlx5_cmd_exec(mdev, in, sizeof(in), out, sizeof(out)); + if (err) + return err; + + obj_context = MLX5_ADDR_OF(query_page_track_obj_out, out, obj_context); + tracker->status = MLX5_GET(page_track, obj_context, state); + return 0; +} + static int alloc_cq_frag_buf(struct mlx5_core_dev *mdev, struct mlx5_vhca_cq_buf *buf, int nent, int cqe_size) @@ -957,9 +986,11 @@ static int mlx5vf_event_notifier(struct notifier_block *nb, unsigned long type, mlx5_nb_cof(nb, struct mlx5_vhca_page_tracker, nb); struct mlx5vf_pci_core_device *mvdev = container_of( tracker, struct mlx5vf_pci_core_device, tracker); + struct mlx5_eqe_obj_change *object; struct mlx5_eqe *eqe = data; u8 event_type = (u8)type; u8 queue_type; + u32 obj_id; int qp_num; switch (event_type) { @@ -975,6 +1006,12 @@ static int mlx5vf_event_notifier(struct notifier_block *nb, unsigned long type, break; set_tracker_error(mvdev); break; + case MLX5_EVENT_TYPE_OBJECT_CHANGE: + object = &eqe->data.obj_change; + obj_id = be32_to_cpu(object->obj_id); + if (obj_id == tracker->id) + set_tracker_change_event(mvdev); + break; default: break; } @@ -1634,6 +1671,11 @@ int mlx5vf_tracker_read_and_clear(struct vfio_device *vdev, unsigned long iova, goto end; } + if (tracker->is_err) { + err = -EIO; + goto end; + } + mdev = mvdev->mdev; err = mlx5vf_cmd_modify_tracker(mdev, tracker->id, iova, length, MLX5_PAGE_TRACK_STATE_REPORTING); @@ -1652,6 +1694,12 @@ int mlx5vf_tracker_read_and_clear(struct vfio_device *vdev, unsigned long iova, dirty, &tracker->status); if (poll_err == CQ_EMPTY) { wait_for_completion(&mvdev->tracker_comp); + if (tracker->object_changed) { + tracker->object_changed = false; + err = mlx5vf_cmd_query_tracker(mdev, tracker); + if (err) + goto end; + } continue; } } diff --git a/drivers/vfio/pci/mlx5/cmd.h b/drivers/vfio/pci/mlx5/cmd.h index f2c7227fa683..0d6a2db3d801 100644 --- a/drivers/vfio/pci/mlx5/cmd.h +++ b/drivers/vfio/pci/mlx5/cmd.h @@ -162,6 +162,7 @@ struct mlx5_vhca_page_tracker { u32 id; u32 pdn; u8 is_err:1; + u8 object_changed:1; struct mlx5_uars_page *uar; struct mlx5_vhca_cq cq; struct mlx5_vhca_qp *host_qp; -- cgit v1.2.3 From 793d4bfa3103a238c6e48eb13c38fa31acd18b94 Mon Sep 17 00:00:00 2001 From: Yishai Hadas Date: Mon, 5 Feb 2024 14:48:26 +0200 Subject: vfio/mlx5: Handle the EREMOTEIO error upon the SAVE command The SAVE command uses the async command interface over the PF. Upon a failure in the firmware -EREMOTEIO is returned. In that case call mlx5_cmd_out_err() to let it print the command failure details including the firmware syndrome. Note: The other commands in the driver use the sync command interface in a way that a firmware syndrome is printed upon an error inside mlx5_core. Signed-off-by: Yishai Hadas Reviewed-by: Kevin Tian Acked-by: Leon Romanovsky Link: https://lore.kernel.org/r/20240205124828.232701-4-yishaih@nvidia.com Signed-off-by: Alex Williamson --- drivers/vfio/pci/mlx5/cmd.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/vfio/pci/mlx5/cmd.c b/drivers/vfio/pci/mlx5/cmd.c index 8a39ff19da28..6b45bd7d89ad 100644 --- a/drivers/vfio/pci/mlx5/cmd.c +++ b/drivers/vfio/pci/mlx5/cmd.c @@ -614,8 +614,13 @@ static void mlx5vf_save_callback(int status, struct mlx5_async_work *context) err: /* The error flow can't run from an interrupt context */ - if (status == -EREMOTEIO) + if (status == -EREMOTEIO) { status = MLX5_GET(save_vhca_state_out, async_data->out, status); + /* Failed in FW, print cmd out failure details */ + mlx5_cmd_out_err(migf->mvdev->mdev, MLX5_CMD_OP_SAVE_VHCA_STATE, 0, + async_data->out); + } + async_data->status = status; queue_work(migf->mvdev->cb_wq, &async_data->work); } -- cgit v1.2.3 From d8d577b5fa2a8832f5730ac26e905d8ac131882d Mon Sep 17 00:00:00 2001 From: Yishai Hadas Date: Mon, 5 Feb 2024 14:48:27 +0200 Subject: vfio/mlx5: Block incremental query upon migf state error Block incremental query which is state-dependent once the migration file was previously marked with state error. This may prevent redundant calls to firmware upon PRE_COPY which will end-up with a failure and a syndrome printed in dmesg. Signed-off-by: Yishai Hadas Reviewed-by: Kevin Tian Acked-by: Leon Romanovsky Link: https://lore.kernel.org/r/20240205124828.232701-5-yishaih@nvidia.com Signed-off-by: Alex Williamson --- drivers/vfio/pci/mlx5/cmd.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/vfio/pci/mlx5/cmd.c b/drivers/vfio/pci/mlx5/cmd.c index 6b45bd7d89ad..6800e4ffe9ee 100644 --- a/drivers/vfio/pci/mlx5/cmd.c +++ b/drivers/vfio/pci/mlx5/cmd.c @@ -121,6 +121,11 @@ int mlx5vf_cmd_query_vhca_migration_state(struct mlx5vf_pci_core_device *mvdev, } query_flags &= ~MLX5VF_QUERY_INC; } + /* Block incremental query which is state-dependent */ + if (mvdev->saving_migf->state == MLX5_MIGF_STATE_ERROR) { + complete(&mvdev->saving_migf->save_comp); + return -ENODEV; + } } MLX5_SET(query_vhca_migration_state_in, in, opcode, -- cgit v1.2.3 From 6de042240b0f654940c196a491c90e81257e49b9 Mon Sep 17 00:00:00 2001 From: Yishai Hadas Date: Mon, 5 Feb 2024 14:48:28 +0200 Subject: vfio/mlx5: Let firmware knows upon leaving PRE_COPY back to RUNNING Let firmware knows upon leaving PRE_COPY back to RUNNING as of some error in the target/migration cancellation. This will let firmware cleaning its internal resources that were turned on upon PRE_COPY. The flow is based on the device specification in this area. Signed-off-by: Yishai Hadas Reviewed-by: Kevin Tian Acked-by: Leon Romanovsky Link: https://lore.kernel.org/r/20240205124828.232701-6-yishaih@nvidia.com Signed-off-by: Alex Williamson --- drivers/vfio/pci/mlx5/cmd.c | 14 ++++++++++---- drivers/vfio/pci/mlx5/cmd.h | 4 +++- drivers/vfio/pci/mlx5/main.c | 39 ++++++++++++++++++++++++++++++++------- 3 files changed, 45 insertions(+), 12 deletions(-) diff --git a/drivers/vfio/pci/mlx5/cmd.c b/drivers/vfio/pci/mlx5/cmd.c index 6800e4ffe9ee..c54bcd5d0917 100644 --- a/drivers/vfio/pci/mlx5/cmd.c +++ b/drivers/vfio/pci/mlx5/cmd.c @@ -108,8 +108,9 @@ int mlx5vf_cmd_query_vhca_migration_state(struct mlx5vf_pci_core_device *mvdev, ret = wait_for_completion_interruptible(&mvdev->saving_migf->save_comp); if (ret) return ret; - if (mvdev->saving_migf->state == - MLX5_MIGF_STATE_PRE_COPY_ERROR) { + /* Upon cleanup, ignore previous pre_copy error state */ + if (mvdev->saving_migf->state == MLX5_MIGF_STATE_PRE_COPY_ERROR && + !(query_flags & MLX5VF_QUERY_CLEANUP)) { /* * In case we had a PRE_COPY error, only query full * image for final image @@ -200,7 +201,7 @@ void mlx5vf_cmd_close_migratable(struct mlx5vf_pci_core_device *mvdev) /* Must be done outside the lock to let it progress */ set_tracker_error(mvdev); mutex_lock(&mvdev->state_mutex); - mlx5vf_disable_fds(mvdev); + mlx5vf_disable_fds(mvdev, NULL); _mlx5vf_free_page_tracker_resources(mvdev); mlx5vf_state_mutex_unlock(mvdev); } @@ -639,6 +640,7 @@ int mlx5vf_cmd_save_vhca_state(struct mlx5vf_pci_core_device *mvdev, u32 in[MLX5_ST_SZ_DW(save_vhca_state_in)] = {}; struct mlx5_vhca_data_buffer *header_buf = NULL; struct mlx5vf_async_data *async_data; + bool pre_copy_cleanup = false; int err; lockdep_assert_held(&mvdev->state_mutex); @@ -649,6 +651,10 @@ int mlx5vf_cmd_save_vhca_state(struct mlx5vf_pci_core_device *mvdev, if (err) return err; + if ((migf->state == MLX5_MIGF_STATE_PRE_COPY || + migf->state == MLX5_MIGF_STATE_PRE_COPY_ERROR) && !track && !inc) + pre_copy_cleanup = true; + if (migf->state == MLX5_MIGF_STATE_PRE_COPY_ERROR) /* * In case we had a PRE_COPY error, SAVE is triggered only for @@ -667,7 +673,7 @@ int mlx5vf_cmd_save_vhca_state(struct mlx5vf_pci_core_device *mvdev, async_data = &migf->async_data; async_data->buf = buf; - async_data->stop_copy_chunk = !track; + async_data->stop_copy_chunk = (!track && !pre_copy_cleanup); async_data->out = kvzalloc(out_size, GFP_KERNEL); if (!async_data->out) { err = -ENOMEM; diff --git a/drivers/vfio/pci/mlx5/cmd.h b/drivers/vfio/pci/mlx5/cmd.h index 0d6a2db3d801..707393df36c4 100644 --- a/drivers/vfio/pci/mlx5/cmd.h +++ b/drivers/vfio/pci/mlx5/cmd.h @@ -197,6 +197,7 @@ struct mlx5vf_pci_core_device { enum { MLX5VF_QUERY_INC = (1UL << 0), MLX5VF_QUERY_FINAL = (1UL << 1), + MLX5VF_QUERY_CLEANUP = (1UL << 2), }; int mlx5vf_cmd_suspend_vhca(struct mlx5vf_pci_core_device *mvdev, u16 op_mod); @@ -232,7 +233,8 @@ int mlx5vf_add_migration_pages(struct mlx5_vhca_data_buffer *buf, struct page *mlx5vf_get_migration_page(struct mlx5_vhca_data_buffer *buf, unsigned long offset); void mlx5vf_state_mutex_unlock(struct mlx5vf_pci_core_device *mvdev); -void mlx5vf_disable_fds(struct mlx5vf_pci_core_device *mvdev); +void mlx5vf_disable_fds(struct mlx5vf_pci_core_device *mvdev, + enum mlx5_vf_migf_state *last_save_state); void mlx5vf_mig_file_cleanup_cb(struct work_struct *_work); void mlx5vf_mig_file_set_save_work(struct mlx5_vf_migration_file *migf, u8 chunk_num, size_t next_required_umem_size); diff --git a/drivers/vfio/pci/mlx5/main.c b/drivers/vfio/pci/mlx5/main.c index fe09a8c8af95..3982fcf60cf2 100644 --- a/drivers/vfio/pci/mlx5/main.c +++ b/drivers/vfio/pci/mlx5/main.c @@ -1146,7 +1146,8 @@ end: return ERR_PTR(ret); } -void mlx5vf_disable_fds(struct mlx5vf_pci_core_device *mvdev) +void mlx5vf_disable_fds(struct mlx5vf_pci_core_device *mvdev, + enum mlx5_vf_migf_state *last_save_state) { if (mvdev->resuming_migf) { mlx5vf_disable_fd(mvdev->resuming_migf); @@ -1157,6 +1158,8 @@ void mlx5vf_disable_fds(struct mlx5vf_pci_core_device *mvdev) if (mvdev->saving_migf) { mlx5_cmd_cleanup_async_ctx(&mvdev->saving_migf->async_ctx); cancel_work_sync(&mvdev->saving_migf->async_data.work); + if (last_save_state) + *last_save_state = mvdev->saving_migf->state; mlx5vf_disable_fd(mvdev->saving_migf); wake_up_interruptible(&mvdev->saving_migf->poll_wait); mlx5fv_cmd_clean_migf_resources(mvdev->saving_migf); @@ -1217,12 +1220,34 @@ mlx5vf_pci_step_device_state_locked(struct mlx5vf_pci_core_device *mvdev, return migf->filp; } - if ((cur == VFIO_DEVICE_STATE_STOP_COPY && new == VFIO_DEVICE_STATE_STOP) || - (cur == VFIO_DEVICE_STATE_PRE_COPY && new == VFIO_DEVICE_STATE_RUNNING) || + if (cur == VFIO_DEVICE_STATE_STOP_COPY && new == VFIO_DEVICE_STATE_STOP) { + mlx5vf_disable_fds(mvdev, NULL); + return NULL; + } + + if ((cur == VFIO_DEVICE_STATE_PRE_COPY && new == VFIO_DEVICE_STATE_RUNNING) || (cur == VFIO_DEVICE_STATE_PRE_COPY_P2P && new == VFIO_DEVICE_STATE_RUNNING_P2P)) { - mlx5vf_disable_fds(mvdev); - return NULL; + struct mlx5_vf_migration_file *migf = mvdev->saving_migf; + struct mlx5_vhca_data_buffer *buf; + enum mlx5_vf_migf_state state; + size_t size; + + ret = mlx5vf_cmd_query_vhca_migration_state(mvdev, &size, NULL, + MLX5VF_QUERY_INC | MLX5VF_QUERY_CLEANUP); + if (ret) + return ERR_PTR(ret); + buf = mlx5vf_get_data_buffer(migf, size, DMA_FROM_DEVICE); + if (IS_ERR(buf)) + return ERR_CAST(buf); + /* pre_copy cleanup */ + ret = mlx5vf_cmd_save_vhca_state(mvdev, migf, buf, false, false); + if (ret) { + mlx5vf_put_data_buffer(buf); + return ERR_PTR(ret); + } + mlx5vf_disable_fds(mvdev, &state); + return (state != MLX5_MIGF_STATE_ERROR) ? NULL : ERR_PTR(-EIO); } if (cur == VFIO_DEVICE_STATE_STOP && new == VFIO_DEVICE_STATE_RESUMING) { @@ -1244,7 +1269,7 @@ mlx5vf_pci_step_device_state_locked(struct mlx5vf_pci_core_device *mvdev, if (ret) return ERR_PTR(ret); } - mlx5vf_disable_fds(mvdev); + mlx5vf_disable_fds(mvdev, NULL); return NULL; } @@ -1289,7 +1314,7 @@ again: mvdev->deferred_reset = false; spin_unlock(&mvdev->reset_lock); mvdev->mig_state = VFIO_DEVICE_STATE_RUNNING; - mlx5vf_disable_fds(mvdev); + mlx5vf_disable_fds(mvdev, NULL); goto again; } mutex_unlock(&mvdev->state_mutex); -- cgit v1.2.3 From 05f3a0bd094c08dce69938306c31b0e39b5f465a Mon Sep 17 00:00:00 2001 From: Alex Williamson Date: Mon, 5 Feb 2024 16:54:18 -0700 Subject: MAINTAINERS: Re-alphabetize VFIO The vfio-pci virtio variant entry slipped in out of order. Link: https://lore.kernel.org/r/20240205235427.2103714-1-alex.williamson@redhat.com Signed-off-by: Alex Williamson --- MAINTAINERS | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 9ed4d3868539..9d0ca59f1ddc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -23089,13 +23089,6 @@ L: kvm@vger.kernel.org S: Maintained F: drivers/vfio/pci/mlx5/ -VFIO VIRTIO PCI DRIVER -M: Yishai Hadas -L: kvm@vger.kernel.org -L: virtualization@lists.linux-foundation.org -S: Maintained -F: drivers/vfio/pci/virtio - VFIO PCI DEVICE SPECIFIC DRIVERS R: Jason Gunthorpe R: Yishai Hadas @@ -23119,6 +23112,13 @@ L: kvm@vger.kernel.org S: Maintained F: drivers/vfio/platform/ +VFIO VIRTIO PCI DRIVER +M: Yishai Hadas +L: kvm@vger.kernel.org +L: virtualization@lists.linux-foundation.org +S: Maintained +F: drivers/vfio/pci/virtio + VGA_SWITCHEROO R: Lukas Wunner S: Maintained -- cgit v1.2.3 From 77943f4d2de0c5fa284013b97967e6c271c04310 Mon Sep 17 00:00:00 2001 From: "Ricardo B. Marliere" Date: Thu, 8 Feb 2024 17:02:04 -0300 Subject: vfio: mdev: make mdev_bus_type const Now that the driver core can properly handle constant struct bus_type, move the mdev_bus_type variable to be a constant structure as well, placing it into read-only memory which can not be modified at runtime. Cc: Greg Kroah-Hartman Suggested-by: Greg Kroah-Hartman Signed-off-by: Ricardo B. Marliere Reviewed-by: Greg Kroah-Hartman Reviewed-by: Jason Gunthorpe Reviewed-by: Kirti Wankhede Link: https://lore.kernel.org/r/20240208-bus_cleanup-vfio-v1-1-ed5da3019949@marliere.net Signed-off-by: Alex Williamson --- drivers/vfio/mdev/mdev_driver.c | 2 +- drivers/vfio/mdev/mdev_private.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/vfio/mdev/mdev_driver.c b/drivers/vfio/mdev/mdev_driver.c index 7825d83a55f8..b98322966b3e 100644 --- a/drivers/vfio/mdev/mdev_driver.c +++ b/drivers/vfio/mdev/mdev_driver.c @@ -40,7 +40,7 @@ static int mdev_match(struct device *dev, struct device_driver *drv) return 0; } -struct bus_type mdev_bus_type = { +const struct bus_type mdev_bus_type = { .name = "mdev", .probe = mdev_probe, .remove = mdev_remove, diff --git a/drivers/vfio/mdev/mdev_private.h b/drivers/vfio/mdev/mdev_private.h index af457b27f607..63a1316b08b7 100644 --- a/drivers/vfio/mdev/mdev_private.h +++ b/drivers/vfio/mdev/mdev_private.h @@ -13,7 +13,7 @@ int mdev_bus_register(void); void mdev_bus_unregister(void); -extern struct bus_type mdev_bus_type; +extern const struct bus_type mdev_bus_type; extern const struct attribute_group *mdev_device_groups[]; #define to_mdev_type_attr(_attr) \ -- cgit v1.2.3 From 4de676d494cd8fb2b4c65e58c19ebbdb36673957 Mon Sep 17 00:00:00 2001 From: Ankit Agrawal Date: Tue, 20 Feb 2024 17:20:53 +0530 Subject: vfio/pci: rename and export do_io_rw() do_io_rw() is used to read/write to the device MMIO. The grace hopper VFIO PCI variant driver require this functionality to read/write to its memory. Rename this as vfio_pci_core functions and export as GPL. Reviewed-by: Kevin Tian Reviewed-by: Yishai Hadas Signed-off-by: Ankit Agrawal Link: https://lore.kernel.org/r/20240220115055.23546-2-ankita@nvidia.com Signed-off-by: Alex Williamson --- drivers/vfio/pci/vfio_pci_rdwr.c | 16 +++++++++------- include/linux/vfio_pci_core.h | 5 ++++- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c index 07fea08ea8a2..03b8f7ada1ac 100644 --- a/drivers/vfio/pci/vfio_pci_rdwr.c +++ b/drivers/vfio/pci/vfio_pci_rdwr.c @@ -96,10 +96,10 @@ VFIO_IOREAD(32) * reads with -1. This is intended for handling MSI-X vector tables and * leftover space for ROM BARs. */ -static ssize_t do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem, - void __iomem *io, char __user *buf, - loff_t off, size_t count, size_t x_start, - size_t x_end, bool iswrite) +ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem, + void __iomem *io, char __user *buf, + loff_t off, size_t count, size_t x_start, + size_t x_end, bool iswrite) { ssize_t done = 0; int ret; @@ -201,6 +201,7 @@ static ssize_t do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem, return done; } +EXPORT_SYMBOL_GPL(vfio_pci_core_do_io_rw); int vfio_pci_core_setup_barmap(struct vfio_pci_core_device *vdev, int bar) { @@ -279,8 +280,8 @@ ssize_t vfio_pci_bar_rw(struct vfio_pci_core_device *vdev, char __user *buf, x_end = vdev->msix_offset + vdev->msix_size; } - done = do_io_rw(vdev, res->flags & IORESOURCE_MEM, io, buf, pos, - count, x_start, x_end, iswrite); + done = vfio_pci_core_do_io_rw(vdev, res->flags & IORESOURCE_MEM, io, buf, pos, + count, x_start, x_end, iswrite); if (done >= 0) *ppos += done; @@ -348,7 +349,8 @@ ssize_t vfio_pci_vga_rw(struct vfio_pci_core_device *vdev, char __user *buf, * probing, so we don't currently worry about access in relation * to the memory enable bit in the command register. */ - done = do_io_rw(vdev, false, iomem, buf, off, count, 0, 0, iswrite); + done = vfio_pci_core_do_io_rw(vdev, false, iomem, buf, off, count, + 0, 0, iswrite); vga_put(vdev->pdev, rsrc); diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h index 85e84b92751b..cf9480a31f3e 100644 --- a/include/linux/vfio_pci_core.h +++ b/include/linux/vfio_pci_core.h @@ -130,7 +130,10 @@ void vfio_pci_core_finish_enable(struct vfio_pci_core_device *vdev); int vfio_pci_core_setup_barmap(struct vfio_pci_core_device *vdev, int bar); pci_ers_result_t vfio_pci_core_aer_err_detected(struct pci_dev *pdev, pci_channel_state_t state); - +ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem, + void __iomem *io, char __user *buf, + loff_t off, size_t count, size_t x_start, + size_t x_end, bool iswrite); #define VFIO_IOWRITE_DECLATION(size) \ int vfio_pci_core_iowrite##size(struct vfio_pci_core_device *vdev, \ bool test_mem, u##size val, void __iomem *io); -- cgit v1.2.3 From 30e920e1debb437e5aea7a4ccdab61634354297a Mon Sep 17 00:00:00 2001 From: Ankit Agrawal Date: Tue, 20 Feb 2024 17:20:54 +0530 Subject: vfio/pci: rename and export range_intersect_range range_intersect_range determines an overlap between two ranges. If an overlap, the helper function returns the overlapping offset and size. The VFIO PCI variant driver emulates the PCI config space BAR offset registers. These offset may be accessed for read/write with a variety of lengths including sub-word sizes from sub-word offsets. The driver makes use of this helper function to read/write the targeted part of the emulated register. Make this a vfio_pci_core function, rename and export as GPL. Also update references in virtio driver. Reviewed-by: Kevin Tian Reviewed-by: Yishai Hadas Signed-off-by: Ankit Agrawal Link: https://lore.kernel.org/r/20240220115055.23546-3-ankita@nvidia.com Signed-off-by: Alex Williamson --- drivers/vfio/pci/vfio_pci_config.c | 42 ++++++++++++++++++++++ drivers/vfio/pci/virtio/main.c | 72 ++++++++++++++------------------------ include/linux/vfio_pci_core.h | 5 +++ 3 files changed, 73 insertions(+), 46 deletions(-) diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c index 7e2e62ab0869..97422aafaa7b 100644 --- a/drivers/vfio/pci/vfio_pci_config.c +++ b/drivers/vfio/pci/vfio_pci_config.c @@ -1966,3 +1966,45 @@ ssize_t vfio_pci_config_rw(struct vfio_pci_core_device *vdev, char __user *buf, return done; } + +/** + * vfio_pci_core_range_intersect_range() - Determine overlap between a buffer + * and register offset ranges. + * @buf_start: start offset of the buffer + * @buf_cnt: number of buffer bytes + * @reg_start: start register offset + * @reg_cnt: number of register bytes + * @buf_offset: start offset of overlap in the buffer + * @intersect_count: number of overlapping bytes + * @register_offset: start offset of overlap in register + * + * Returns: true if there is overlap, false if not. + * The overlap start and size is returned through function args. + */ +bool vfio_pci_core_range_intersect_range(loff_t buf_start, size_t buf_cnt, + loff_t reg_start, size_t reg_cnt, + loff_t *buf_offset, + size_t *intersect_count, + size_t *register_offset) +{ + if (buf_start <= reg_start && + buf_start + buf_cnt > reg_start) { + *buf_offset = reg_start - buf_start; + *intersect_count = min_t(size_t, reg_cnt, + buf_start + buf_cnt - reg_start); + *register_offset = 0; + return true; + } + + if (buf_start > reg_start && + buf_start < reg_start + reg_cnt) { + *buf_offset = 0; + *intersect_count = min_t(size_t, buf_cnt, + reg_start + reg_cnt - buf_start); + *register_offset = buf_start - reg_start; + return true; + } + + return false; +} +EXPORT_SYMBOL_GPL(vfio_pci_core_range_intersect_range); diff --git a/drivers/vfio/pci/virtio/main.c b/drivers/vfio/pci/virtio/main.c index d5af683837d3..b5d3a8c5bbc9 100644 --- a/drivers/vfio/pci/virtio/main.c +++ b/drivers/vfio/pci/virtio/main.c @@ -132,33 +132,6 @@ end: return ret ? ret : count; } -static bool range_intersect_range(loff_t range1_start, size_t count1, - loff_t range2_start, size_t count2, - loff_t *start_offset, - size_t *intersect_count, - size_t *register_offset) -{ - if (range1_start <= range2_start && - range1_start + count1 > range2_start) { - *start_offset = range2_start - range1_start; - *intersect_count = min_t(size_t, count2, - range1_start + count1 - range2_start); - *register_offset = 0; - return true; - } - - if (range1_start > range2_start && - range1_start < range2_start + count2) { - *start_offset = 0; - *intersect_count = min_t(size_t, count1, - range2_start + count2 - range1_start); - *register_offset = range1_start - range2_start; - return true; - } - - return false; -} - static ssize_t virtiovf_pci_read_config(struct vfio_device *core_vdev, char __user *buf, size_t count, loff_t *ppos) @@ -178,16 +151,18 @@ static ssize_t virtiovf_pci_read_config(struct vfio_device *core_vdev, if (ret < 0) return ret; - if (range_intersect_range(pos, count, PCI_DEVICE_ID, sizeof(val16), - ©_offset, ©_count, ®ister_offset)) { + if (vfio_pci_core_range_intersect_range(pos, count, PCI_DEVICE_ID, + sizeof(val16), ©_offset, + ©_count, ®ister_offset)) { val16 = cpu_to_le16(VIRTIO_TRANS_ID_NET); if (copy_to_user(buf + copy_offset, (void *)&val16 + register_offset, copy_count)) return -EFAULT; } if ((le16_to_cpu(virtvdev->pci_cmd) & PCI_COMMAND_IO) && - range_intersect_range(pos, count, PCI_COMMAND, sizeof(val16), - ©_offset, ©_count, ®ister_offset)) { + vfio_pci_core_range_intersect_range(pos, count, PCI_COMMAND, + sizeof(val16), ©_offset, + ©_count, ®ister_offset)) { if (copy_from_user((void *)&val16 + register_offset, buf + copy_offset, copy_count)) return -EFAULT; @@ -197,16 +172,18 @@ static ssize_t virtiovf_pci_read_config(struct vfio_device *core_vdev, return -EFAULT; } - if (range_intersect_range(pos, count, PCI_REVISION_ID, sizeof(val8), - ©_offset, ©_count, ®ister_offset)) { + if (vfio_pci_core_range_intersect_range(pos, count, PCI_REVISION_ID, + sizeof(val8), ©_offset, + ©_count, ®ister_offset)) { /* Transional needs to have revision 0 */ val8 = 0; if (copy_to_user(buf + copy_offset, &val8, copy_count)) return -EFAULT; } - if (range_intersect_range(pos, count, PCI_BASE_ADDRESS_0, sizeof(val32), - ©_offset, ©_count, ®ister_offset)) { + if (vfio_pci_core_range_intersect_range(pos, count, PCI_BASE_ADDRESS_0, + sizeof(val32), ©_offset, + ©_count, ®ister_offset)) { u32 bar_mask = ~(virtvdev->bar0_virtual_buf_size - 1); u32 pci_base_addr_0 = le32_to_cpu(virtvdev->pci_base_addr_0); @@ -215,8 +192,9 @@ static ssize_t virtiovf_pci_read_config(struct vfio_device *core_vdev, return -EFAULT; } - if (range_intersect_range(pos, count, PCI_SUBSYSTEM_ID, sizeof(val16), - ©_offset, ©_count, ®ister_offset)) { + if (vfio_pci_core_range_intersect_range(pos, count, PCI_SUBSYSTEM_ID, + sizeof(val16), ©_offset, + ©_count, ®ister_offset)) { /* * Transitional devices use the PCI subsystem device id as * virtio device id, same as legacy driver always did. @@ -227,8 +205,9 @@ static ssize_t virtiovf_pci_read_config(struct vfio_device *core_vdev, return -EFAULT; } - if (range_intersect_range(pos, count, PCI_SUBSYSTEM_VENDOR_ID, sizeof(val16), - ©_offset, ©_count, ®ister_offset)) { + if (vfio_pci_core_range_intersect_range(pos, count, PCI_SUBSYSTEM_VENDOR_ID, + sizeof(val16), ©_offset, + ©_count, ®ister_offset)) { val16 = cpu_to_le16(PCI_VENDOR_ID_REDHAT_QUMRANET); if (copy_to_user(buf + copy_offset, (void *)&val16 + register_offset, copy_count)) @@ -270,19 +249,20 @@ static ssize_t virtiovf_pci_write_config(struct vfio_device *core_vdev, loff_t copy_offset; size_t copy_count; - if (range_intersect_range(pos, count, PCI_COMMAND, sizeof(virtvdev->pci_cmd), - ©_offset, ©_count, - ®ister_offset)) { + if (vfio_pci_core_range_intersect_range(pos, count, PCI_COMMAND, + sizeof(virtvdev->pci_cmd), + ©_offset, ©_count, + ®ister_offset)) { if (copy_from_user((void *)&virtvdev->pci_cmd + register_offset, buf + copy_offset, copy_count)) return -EFAULT; } - if (range_intersect_range(pos, count, PCI_BASE_ADDRESS_0, - sizeof(virtvdev->pci_base_addr_0), - ©_offset, ©_count, - ®ister_offset)) { + if (vfio_pci_core_range_intersect_range(pos, count, PCI_BASE_ADDRESS_0, + sizeof(virtvdev->pci_base_addr_0), + ©_offset, ©_count, + ®ister_offset)) { if (copy_from_user((void *)&virtvdev->pci_base_addr_0 + register_offset, buf + copy_offset, copy_count)) diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h index cf9480a31f3e..a2c8b8bba711 100644 --- a/include/linux/vfio_pci_core.h +++ b/include/linux/vfio_pci_core.h @@ -134,6 +134,11 @@ ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem, void __iomem *io, char __user *buf, loff_t off, size_t count, size_t x_start, size_t x_end, bool iswrite); +bool vfio_pci_core_range_intersect_range(loff_t buf_start, size_t buf_cnt, + loff_t reg_start, size_t reg_cnt, + loff_t *buf_offset, + size_t *intersect_count, + size_t *register_offset); #define VFIO_IOWRITE_DECLATION(size) \ int vfio_pci_core_iowrite##size(struct vfio_pci_core_device *vdev, \ bool test_mem, u##size val, void __iomem *io); -- cgit v1.2.3 From 701ab935859fcfd4a8c8a97f3ee4fb5294a9d481 Mon Sep 17 00:00:00 2001 From: Ankit Agrawal Date: Tue, 20 Feb 2024 17:20:55 +0530 Subject: vfio/nvgrace-gpu: Add vfio pci variant module for grace hopper NVIDIA's upcoming Grace Hopper Superchip provides a PCI-like device for the on-chip GPU that is the logical OS representation of the internal proprietary chip-to-chip cache coherent interconnect. The device is peculiar compared to a real PCI device in that whilst there is a real 64b PCI BAR1 (comprising region 2 & region 3) on the device, it is not used to access device memory once the faster chip-to-chip interconnect is initialized (occurs at the time of host system boot). The device memory is accessed instead using the chip-to-chip interconnect that is exposed as a contiguous physically addressable region on the host. This device memory aperture can be obtained from host ACPI table using device_property_read_u64(), according to the FW specification. Since the device memory is cache coherent with the CPU, it can be mmap into the user VMA with a cacheable mapping using remap_pfn_range() and used like a regular RAM. The device memory is not added to the host kernel, but mapped directly as this reduces memory wastage due to struct pages. There is also a requirement of a minimum reserved 1G uncached region (termed as resmem) to support the Multi-Instance GPU (MIG) feature [1]. This is to work around a HW defect. Based on [2], the requisite properties (uncached, unaligned access) can be achieved through a VM mapping (S1) of NORMAL_NC and host (S2) mapping with MemAttr[2:0]=0b101. To provide a different non-cached property to the reserved 1G region, it needs to be carved out from the device memory and mapped as a separate region in Qemu VMA with pgprot_writecombine(). pgprot_writecombine() sets the Qemu VMA page properties (pgprot) as NORMAL_NC. Provide a VFIO PCI variant driver that adapts the unique device memory representation into a more standard PCI representation facing userspace. The variant driver exposes these two regions - the non-cached reserved (resmem) and the cached rest of the device memory (termed as usemem) as separate VFIO 64b BAR regions. This is divergent from the baremetal approach, where the device memory is exposed as a device memory region. The decision for a different approach was taken in view of the fact that it would necessiate additional code in Qemu to discover and insert those regions in the VM IPA, along with the additional VM ACPI DSDT changes to communicate the device memory region IPA to the VM workloads. Moreover, this behavior would have to be added to a variety of emulators (beyond top of tree Qemu) out there desiring grace hopper support. Since the device implements 64-bit BAR0, the VFIO PCI variant driver maps the uncached carved out region to the next available PCI BAR (i.e. comprising of region 2 and 3). The cached device memory aperture is assigned BAR region 4 and 5. Qemu will then naturally generate a PCI device in the VM with the uncached aperture reported as BAR2 region, the cacheable as BAR4. The variant driver provides emulation for these fake BARs' PCI config space offset registers. The hardware ensures that the system does not crash when the memory is accessed with the memory enable turned off. It synthesis ~0 reads and dropped writes on such access. So there is no need to support the disablement/enablement of BAR through PCI_COMMAND config space register. The memory layout on the host looks like the following: devmem (memlength) |--------------------------------------------------| |-------------cached------------------------|--NC--| | | usemem.memphys resmem.memphys PCI BARs need to be aligned to the power-of-2, but the actual memory on the device may not. A read or write access to the physical address from the last device PFN up to the next power-of-2 aligned physical address results in reading ~0 and dropped writes. Note that the GPU device driver [6] is capable of knowing the exact device memory size through separate means. The device memory size is primarily kept in the system ACPI tables for use by the VFIO PCI variant module. Note that the usemem memory is added by the VM Nvidia device driver [5] to the VM kernel as memblocks. Hence make the usable memory size memblock (MEMBLK_SIZE) aligned. This is a hardwired ABI value between the GPU FW and VFIO driver. The VM device driver make use of the same value for its calculation to determine USEMEM size. Currently there is no provision in KVM for a S2 mapping with MemAttr[2:0]=0b101, but there is an ongoing effort to provide the same [3]. As previously mentioned, resmem is mapped pgprot_writecombine(), that sets the Qemu VMA page properties (pgprot) as NORMAL_NC. Using the proposed changes in [3] and [4], KVM marks the region with MemAttr[2:0]=0b101 in S2. If the device memory properties are not present, the driver registers the vfio-pci-core function pointers. Since there are no ACPI memory properties generated for the VM, the variant driver inside the VM will only use the vfio-pci-core ops and hence try to map the BARs as non cached. This is not a problem as the CPUs have FWB enabled which blocks the VM mapping's ability to override the cacheability set by the host mapping. This goes along with a qemu series [6] to provides the necessary implementation of the Grace Hopper Superchip firmware specification so that the guest operating system can see the correct ACPI modeling for the coherent GPU device. Verified with the CUDA workload in the VM. [1] https://www.nvidia.com/en-in/technologies/multi-instance-gpu/ [2] section D8.5.5 of https://developer.arm.com/documentation/ddi0487/latest/ [3] https://lore.kernel.org/all/20240211174705.31992-1-ankita@nvidia.com/ [4] https://lore.kernel.org/all/20230907181459.18145-2-ankita@nvidia.com/ [5] https://github.com/NVIDIA/open-gpu-kernel-modules [6] https://lore.kernel.org/all/20231203060245.31593-1-ankita@nvidia.com/ Reviewed-by: Kevin Tian Reviewed-by: Yishai Hadas Reviewed-by: Zhi Wang Signed-off-by: Aniket Agashe Signed-off-by: Ankit Agrawal Link: https://lore.kernel.org/r/20240220115055.23546-4-ankita@nvidia.com Signed-off-by: Alex Williamson --- MAINTAINERS | 6 + drivers/vfio/pci/Kconfig | 2 + drivers/vfio/pci/Makefile | 2 + drivers/vfio/pci/nvgrace-gpu/Kconfig | 10 + drivers/vfio/pci/nvgrace-gpu/Makefile | 3 + drivers/vfio/pci/nvgrace-gpu/main.c | 879 ++++++++++++++++++++++++++++++++++ 6 files changed, 902 insertions(+) create mode 100644 drivers/vfio/pci/nvgrace-gpu/Kconfig create mode 100644 drivers/vfio/pci/nvgrace-gpu/Makefile create mode 100644 drivers/vfio/pci/nvgrace-gpu/main.c diff --git a/MAINTAINERS b/MAINTAINERS index 9d0ca59f1ddc..7625911ec2f1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -23089,6 +23089,12 @@ L: kvm@vger.kernel.org S: Maintained F: drivers/vfio/pci/mlx5/ +VFIO NVIDIA GRACE GPU DRIVER +M: Ankit Agrawal +L: kvm@vger.kernel.org +S: Supported +F: drivers/vfio/pci/nvgrace-gpu/ + VFIO PCI DEVICE SPECIFIC DRIVERS R: Jason Gunthorpe R: Yishai Hadas diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig index 18c397df566d..15821a2d77d2 100644 --- a/drivers/vfio/pci/Kconfig +++ b/drivers/vfio/pci/Kconfig @@ -67,4 +67,6 @@ source "drivers/vfio/pci/pds/Kconfig" source "drivers/vfio/pci/virtio/Kconfig" +source "drivers/vfio/pci/nvgrace-gpu/Kconfig" + endmenu diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile index 046139a4eca5..ce7a61f1d912 100644 --- a/drivers/vfio/pci/Makefile +++ b/drivers/vfio/pci/Makefile @@ -15,3 +15,5 @@ obj-$(CONFIG_HISI_ACC_VFIO_PCI) += hisilicon/ obj-$(CONFIG_PDS_VFIO_PCI) += pds/ obj-$(CONFIG_VIRTIO_VFIO_PCI) += virtio/ + +obj-$(CONFIG_NVGRACE_GPU_VFIO_PCI) += nvgrace-gpu/ diff --git a/drivers/vfio/pci/nvgrace-gpu/Kconfig b/drivers/vfio/pci/nvgrace-gpu/Kconfig new file mode 100644 index 000000000000..a7f624b37e41 --- /dev/null +++ b/drivers/vfio/pci/nvgrace-gpu/Kconfig @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: GPL-2.0-only +config NVGRACE_GPU_VFIO_PCI + tristate "VFIO support for the GPU in the NVIDIA Grace Hopper Superchip" + depends on ARM64 || (COMPILE_TEST && 64BIT) + select VFIO_PCI_CORE + help + VFIO support for the GPU in the NVIDIA Grace Hopper Superchip is + required to assign the GPU device to userspace using KVM/qemu/etc. + + If you don't know what to do here, say N. diff --git a/drivers/vfio/pci/nvgrace-gpu/Makefile b/drivers/vfio/pci/nvgrace-gpu/Makefile new file mode 100644 index 000000000000..3ca8c187897a --- /dev/null +++ b/drivers/vfio/pci/nvgrace-gpu/Makefile @@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0-only +obj-$(CONFIG_NVGRACE_GPU_VFIO_PCI) += nvgrace-gpu-vfio-pci.o +nvgrace-gpu-vfio-pci-y := main.o diff --git a/drivers/vfio/pci/nvgrace-gpu/main.c b/drivers/vfio/pci/nvgrace-gpu/main.c new file mode 100644 index 000000000000..25814006352d --- /dev/null +++ b/drivers/vfio/pci/nvgrace-gpu/main.c @@ -0,0 +1,879 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved + */ + +#include +#include + +/* + * The device memory usable to the workloads running in the VM is cached + * and showcased as a 64b device BAR (comprising of BAR4 and BAR5 region) + * to the VM and is represented as usemem. + * Moreover, the VM GPU device driver needs a non-cacheable region to + * support the MIG feature. This region is also exposed as a 64b BAR + * (comprising of BAR2 and BAR3 region) and represented as resmem. + */ +#define RESMEM_REGION_INDEX VFIO_PCI_BAR2_REGION_INDEX +#define USEMEM_REGION_INDEX VFIO_PCI_BAR4_REGION_INDEX + +/* Memory size expected as non cached and reserved by the VM driver */ +#define RESMEM_SIZE SZ_1G + +/* A hardwired and constant ABI value between the GPU FW and VFIO driver. */ +#define MEMBLK_SIZE SZ_512M + +/* + * The state of the two device memory region - resmem and usemem - is + * saved as struct mem_region. + */ +struct mem_region { + phys_addr_t memphys; /* Base physical address of the region */ + size_t memlength; /* Region size */ + size_t bar_size; /* Reported region BAR size */ + __le64 bar_val; /* Emulated BAR offset registers */ + union { + void *memaddr; + void __iomem *ioaddr; + }; /* Base virtual address of the region */ +}; + +struct nvgrace_gpu_pci_core_device { + struct vfio_pci_core_device core_device; + /* Cached and usable memory for the VM. */ + struct mem_region usemem; + /* Non cached memory carved out from the end of device memory */ + struct mem_region resmem; + /* Lock to control device memory kernel mapping */ + struct mutex remap_lock; +}; + +static void nvgrace_gpu_init_fake_bar_emu_regs(struct vfio_device *core_vdev) +{ + struct nvgrace_gpu_pci_core_device *nvdev = + container_of(core_vdev, struct nvgrace_gpu_pci_core_device, + core_device.vdev); + + nvdev->resmem.bar_val = 0; + nvdev->usemem.bar_val = 0; +} + +/* Choose the structure corresponding to the fake BAR with a given index. */ +static struct mem_region * +nvgrace_gpu_memregion(int index, + struct nvgrace_gpu_pci_core_device *nvdev) +{ + if (index == USEMEM_REGION_INDEX) + return &nvdev->usemem; + + if (index == RESMEM_REGION_INDEX) + return &nvdev->resmem; + + return NULL; +} + +static int nvgrace_gpu_open_device(struct vfio_device *core_vdev) +{ + struct vfio_pci_core_device *vdev = + container_of(core_vdev, struct vfio_pci_core_device, vdev); + struct nvgrace_gpu_pci_core_device *nvdev = + container_of(core_vdev, struct nvgrace_gpu_pci_core_device, + core_device.vdev); + int ret; + + ret = vfio_pci_core_enable(vdev); + if (ret) + return ret; + + if (nvdev->usemem.memlength) { + nvgrace_gpu_init_fake_bar_emu_regs(core_vdev); + mutex_init(&nvdev->remap_lock); + } + + vfio_pci_core_finish_enable(vdev); + + return 0; +} + +static void nvgrace_gpu_close_device(struct vfio_device *core_vdev) +{ + struct nvgrace_gpu_pci_core_device *nvdev = + container_of(core_vdev, struct nvgrace_gpu_pci_core_device, + core_device.vdev); + + /* Unmap the mapping to the device memory cached region */ + if (nvdev->usemem.memaddr) { + memunmap(nvdev->usemem.memaddr); + nvdev->usemem.memaddr = NULL; + } + + /* Unmap the mapping to the device memory non-cached region */ + if (nvdev->resmem.ioaddr) { + iounmap(nvdev->resmem.ioaddr); + nvdev->resmem.ioaddr = NULL; + } + + mutex_destroy(&nvdev->remap_lock); + + vfio_pci_core_close_device(core_vdev); +} + +static int nvgrace_gpu_mmap(struct vfio_device *core_vdev, + struct vm_area_struct *vma) +{ + struct nvgrace_gpu_pci_core_device *nvdev = + container_of(core_vdev, struct nvgrace_gpu_pci_core_device, + core_device.vdev); + struct mem_region *memregion; + unsigned long start_pfn; + u64 req_len, pgoff, end; + unsigned int index; + int ret = 0; + + index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT); + + memregion = nvgrace_gpu_memregion(index, nvdev); + if (!memregion) + return vfio_pci_core_mmap(core_vdev, vma); + + /* + * Request to mmap the BAR. Map to the CPU accessible memory on the + * GPU using the memory information gathered from the system ACPI + * tables. + */ + pgoff = vma->vm_pgoff & + ((1U << (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT)) - 1); + + if (check_sub_overflow(vma->vm_end, vma->vm_start, &req_len) || + check_add_overflow(PHYS_PFN(memregion->memphys), pgoff, &start_pfn) || + check_add_overflow(PFN_PHYS(pgoff), req_len, &end)) + return -EOVERFLOW; + + /* + * Check that the mapping request does not go beyond available device + * memory size + */ + if (end > memregion->memlength) + return -EINVAL; + + /* + * The carved out region of the device memory needs the NORMAL_NC + * property. Communicate as such to the hypervisor. + */ + if (index == RESMEM_REGION_INDEX) + vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot); + + /* + * Perform a PFN map to the memory and back the device BAR by the + * GPU memory. + * + * The available GPU memory size may not be power-of-2 aligned. The + * remainder is only backed by vfio_device_ops read/write handlers. + * + * During device reset, the GPU is safely disconnected to the CPU + * and access to the BAR will be immediately returned preventing + * machine check. + */ + ret = remap_pfn_range(vma, vma->vm_start, start_pfn, + req_len, vma->vm_page_prot); + if (ret) + return ret; + + vma->vm_pgoff = start_pfn; + + return 0; +} + +static long +nvgrace_gpu_ioctl_get_region_info(struct vfio_device *core_vdev, + unsigned long arg) +{ + struct nvgrace_gpu_pci_core_device *nvdev = + container_of(core_vdev, struct nvgrace_gpu_pci_core_device, + core_device.vdev); + unsigned long minsz = offsetofend(struct vfio_region_info, offset); + struct vfio_info_cap caps = { .buf = NULL, .size = 0 }; + struct vfio_region_info_cap_sparse_mmap *sparse; + struct vfio_region_info info; + struct mem_region *memregion; + u32 size; + int ret; + + if (copy_from_user(&info, (void __user *)arg, minsz)) + return -EFAULT; + + if (info.argsz < minsz) + return -EINVAL; + + /* + * Request to determine the BAR region information. Send the + * GPU memory information. + */ + memregion = nvgrace_gpu_memregion(info.index, nvdev); + if (!memregion) + return vfio_pci_core_ioctl(core_vdev, + VFIO_DEVICE_GET_REGION_INFO, arg); + + size = struct_size(sparse, areas, 1); + + /* + * Setup for sparse mapping for the device memory. Only the + * available device memory on the hardware is shown as a + * mappable region. + */ + sparse = kzalloc(size, GFP_KERNEL); + if (!sparse) + return -ENOMEM; + + sparse->nr_areas = 1; + sparse->areas[0].offset = 0; + sparse->areas[0].size = memregion->memlength; + sparse->header.id = VFIO_REGION_INFO_CAP_SPARSE_MMAP; + sparse->header.version = 1; + + ret = vfio_info_add_capability(&caps, &sparse->header, size); + kfree(sparse); + if (ret) + return ret; + + info.offset = VFIO_PCI_INDEX_TO_OFFSET(info.index); + /* + * The region memory size may not be power-of-2 aligned. + * Given that the memory as a BAR and may not be + * aligned, roundup to the next power-of-2. + */ + info.size = memregion->bar_size; + info.flags = VFIO_REGION_INFO_FLAG_READ | + VFIO_REGION_INFO_FLAG_WRITE | + VFIO_REGION_INFO_FLAG_MMAP; + + if (caps.size) { + info.flags |= VFIO_REGION_INFO_FLAG_CAPS; + if (info.argsz < sizeof(info) + caps.size) { + info.argsz = sizeof(info) + caps.size; + info.cap_offset = 0; + } else { + vfio_info_cap_shift(&caps, sizeof(info)); + if (copy_to_user((void __user *)arg + + sizeof(info), caps.buf, + caps.size)) { + kfree(caps.buf); + return -EFAULT; + } + info.cap_offset = sizeof(info); + } + kfree(caps.buf); + } + return copy_to_user((void __user *)arg, &info, minsz) ? + -EFAULT : 0; +} + +static long nvgrace_gpu_ioctl(struct vfio_device *core_vdev, + unsigned int cmd, unsigned long arg) +{ + switch (cmd) { + case VFIO_DEVICE_GET_REGION_INFO: + return nvgrace_gpu_ioctl_get_region_info(core_vdev, arg); + case VFIO_DEVICE_IOEVENTFD: + return -ENOTTY; + case VFIO_DEVICE_RESET: + nvgrace_gpu_init_fake_bar_emu_regs(core_vdev); + fallthrough; + default: + return vfio_pci_core_ioctl(core_vdev, cmd, arg); + } +} + +static __le64 +nvgrace_gpu_get_read_value(size_t bar_size, u64 flags, __le64 val64) +{ + u64 tmp_val; + + tmp_val = le64_to_cpu(val64); + tmp_val &= ~(bar_size - 1); + tmp_val |= flags; + + return cpu_to_le64(tmp_val); +} + +/* + * Both the usable (usemem) and the reserved (resmem) device memory region + * are exposed as a 64b fake device BARs in the VM. These fake BARs must + * respond to the accesses on their respective PCI config space offsets. + * + * resmem BAR owns PCI_BASE_ADDRESS_2 & PCI_BASE_ADDRESS_3. + * usemem BAR owns PCI_BASE_ADDRESS_4 & PCI_BASE_ADDRESS_5. + */ +static ssize_t +nvgrace_gpu_read_config_emu(struct vfio_device *core_vdev, + char __user *buf, size_t count, loff_t *ppos) +{ + struct nvgrace_gpu_pci_core_device *nvdev = + container_of(core_vdev, struct nvgrace_gpu_pci_core_device, + core_device.vdev); + u64 pos = *ppos & VFIO_PCI_OFFSET_MASK; + struct mem_region *memregion = NULL; + __le64 val64; + size_t register_offset; + loff_t copy_offset; + size_t copy_count; + int ret; + + ret = vfio_pci_core_read(core_vdev, buf, count, ppos); + if (ret < 0) + return ret; + + if (vfio_pci_core_range_intersect_range(pos, count, PCI_BASE_ADDRESS_2, + sizeof(val64), + ©_offset, ©_count, + ®ister_offset)) + memregion = nvgrace_gpu_memregion(RESMEM_REGION_INDEX, nvdev); + else if (vfio_pci_core_range_intersect_range(pos, count, + PCI_BASE_ADDRESS_4, + sizeof(val64), + ©_offset, ©_count, + ®ister_offset)) + memregion = nvgrace_gpu_memregion(USEMEM_REGION_INDEX, nvdev); + + if (memregion) { + val64 = nvgrace_gpu_get_read_value(memregion->bar_size, + PCI_BASE_ADDRESS_MEM_TYPE_64 | + PCI_BASE_ADDRESS_MEM_PREFETCH, + memregion->bar_val); + if (copy_to_user(buf + copy_offset, + (void *)&val64 + register_offset, copy_count)) { + /* + * The position has been incremented in + * vfio_pci_core_read. Reset the offset back to the + * starting position. + */ + *ppos -= count; + return -EFAULT; + } + } + + return count; +} + +static ssize_t +nvgrace_gpu_write_config_emu(struct vfio_device *core_vdev, + const char __user *buf, size_t count, loff_t *ppos) +{ + struct nvgrace_gpu_pci_core_device *nvdev = + container_of(core_vdev, struct nvgrace_gpu_pci_core_device, + core_device.vdev); + u64 pos = *ppos & VFIO_PCI_OFFSET_MASK; + struct mem_region *memregion = NULL; + size_t register_offset; + loff_t copy_offset; + size_t copy_count; + + if (vfio_pci_core_range_intersect_range(pos, count, PCI_BASE_ADDRESS_2, + sizeof(u64), ©_offset, + ©_count, ®ister_offset)) + memregion = nvgrace_gpu_memregion(RESMEM_REGION_INDEX, nvdev); + else if (vfio_pci_core_range_intersect_range(pos, count, PCI_BASE_ADDRESS_4, + sizeof(u64), ©_offset, + ©_count, ®ister_offset)) + memregion = nvgrace_gpu_memregion(USEMEM_REGION_INDEX, nvdev); + + if (memregion) { + if (copy_from_user((void *)&memregion->bar_val + register_offset, + buf + copy_offset, copy_count)) + return -EFAULT; + *ppos += copy_count; + return copy_count; + } + + return vfio_pci_core_write(core_vdev, buf, count, ppos); +} + +/* + * Ad hoc map the device memory in the module kernel VA space. Primarily needed + * as vfio does not require the userspace driver to only perform accesses through + * mmaps of the vfio-pci BAR regions and such accesses should be supported using + * vfio_device_ops read/write implementations. + * + * The usemem region is cacheable memory and hence is memremaped. + * The resmem region is non-cached and is mapped using ioremap_wc (NORMAL_NC). + */ +static int +nvgrace_gpu_map_device_mem(int index, + struct nvgrace_gpu_pci_core_device *nvdev) +{ + struct mem_region *memregion; + int ret = 0; + + memregion = nvgrace_gpu_memregion(index, nvdev); + if (!memregion) + return -EINVAL; + + mutex_lock(&nvdev->remap_lock); + + if (memregion->memaddr) + goto unlock; + + if (index == USEMEM_REGION_INDEX) + memregion->memaddr = memremap(memregion->memphys, + memregion->memlength, + MEMREMAP_WB); + else + memregion->ioaddr = ioremap_wc(memregion->memphys, + memregion->memlength); + + if (!memregion->memaddr) + ret = -ENOMEM; + +unlock: + mutex_unlock(&nvdev->remap_lock); + + return ret; +} + +/* + * Read the data from the device memory (mapped either through ioremap + * or memremap) into the user buffer. + */ +static int +nvgrace_gpu_map_and_read(struct nvgrace_gpu_pci_core_device *nvdev, + char __user *buf, size_t mem_count, loff_t *ppos) +{ + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos); + u64 offset = *ppos & VFIO_PCI_OFFSET_MASK; + int ret; + + if (!mem_count) + return 0; + + /* + * Handle read on the BAR regions. Map to the target device memory + * physical address and copy to the request read buffer. + */ + ret = nvgrace_gpu_map_device_mem(index, nvdev); + if (ret) + return ret; + + if (index == USEMEM_REGION_INDEX) { + if (copy_to_user(buf, + (u8 *)nvdev->usemem.memaddr + offset, + mem_count)) + ret = -EFAULT; + } else { + /* + * The hardware ensures that the system does not crash when + * the device memory is accessed with the memory enable + * turned off. It synthesizes ~0 on such read. So there is + * no need to check or support the disablement/enablement of + * BAR through PCI_COMMAND config space register. Pass + * test_mem flag as false. + */ + ret = vfio_pci_core_do_io_rw(&nvdev->core_device, false, + nvdev->resmem.ioaddr, + buf, offset, mem_count, + 0, 0, false); + } + + return ret; +} + +/* + * Read count bytes from the device memory at an offset. The actual device + * memory size (available) may not be a power-of-2. So the driver fakes + * the size to a power-of-2 (reported) when exposing to a user space driver. + * + * Reads starting beyond the reported size generate -EINVAL; reads extending + * beyond the actual device size is filled with ~0; reads extending beyond + * the reported size are truncated. + */ +static ssize_t +nvgrace_gpu_read_mem(struct nvgrace_gpu_pci_core_device *nvdev, + char __user *buf, size_t count, loff_t *ppos) +{ + u64 offset = *ppos & VFIO_PCI_OFFSET_MASK; + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos); + struct mem_region *memregion; + size_t mem_count, i; + u8 val = 0xFF; + int ret; + + /* No need to do NULL check as caller does. */ + memregion = nvgrace_gpu_memregion(index, nvdev); + + if (offset >= memregion->bar_size) + return -EINVAL; + + /* Clip short the read request beyond reported BAR size */ + count = min(count, memregion->bar_size - (size_t)offset); + + /* + * Determine how many bytes to be actually read from the device memory. + * Read request beyond the actual device memory size is filled with ~0, + * while those beyond the actual reported size is skipped. + */ + if (offset >= memregion->memlength) + mem_count = 0; + else + mem_count = min(count, memregion->memlength - (size_t)offset); + + ret = nvgrace_gpu_map_and_read(nvdev, buf, mem_count, ppos); + if (ret) + return ret; + + /* + * Only the device memory present on the hardware is mapped, which may + * not be power-of-2 aligned. A read to an offset beyond the device memory + * size is filled with ~0. + */ + for (i = mem_count; i < count; i++) { + ret = put_user(val, (unsigned char __user *)(buf + i)); + if (ret) + return ret; + } + + *ppos += count; + return count; +} + +static ssize_t +nvgrace_gpu_read(struct vfio_device *core_vdev, + char __user *buf, size_t count, loff_t *ppos) +{ + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos); + struct nvgrace_gpu_pci_core_device *nvdev = + container_of(core_vdev, struct nvgrace_gpu_pci_core_device, + core_device.vdev); + + if (nvgrace_gpu_memregion(index, nvdev)) + return nvgrace_gpu_read_mem(nvdev, buf, count, ppos); + + if (index == VFIO_PCI_CONFIG_REGION_INDEX) + return nvgrace_gpu_read_config_emu(core_vdev, buf, count, ppos); + + return vfio_pci_core_read(core_vdev, buf, count, ppos); +} + +/* + * Write the data to the device memory (mapped either through ioremap + * or memremap) from the user buffer. + */ +static int +nvgrace_gpu_map_and_write(struct nvgrace_gpu_pci_core_device *nvdev, + const char __user *buf, size_t mem_count, + loff_t *ppos) +{ + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos); + loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK; + int ret; + + if (!mem_count) + return 0; + + ret = nvgrace_gpu_map_device_mem(index, nvdev); + if (ret) + return ret; + + if (index == USEMEM_REGION_INDEX) { + if (copy_from_user((u8 *)nvdev->usemem.memaddr + pos, + buf, mem_count)) + return -EFAULT; + } else { + /* + * The hardware ensures that the system does not crash when + * the device memory is accessed with the memory enable + * turned off. It drops such writes. So there is no need to + * check or support the disablement/enablement of BAR + * through PCI_COMMAND config space register. Pass test_mem + * flag as false. + */ + ret = vfio_pci_core_do_io_rw(&nvdev->core_device, false, + nvdev->resmem.ioaddr, + (char __user *)buf, pos, mem_count, + 0, 0, true); + } + + return ret; +} + +/* + * Write count bytes to the device memory at a given offset. The actual device + * memory size (available) may not be a power-of-2. So the driver fakes the + * size to a power-of-2 (reported) when exposing to a user space driver. + * + * Writes extending beyond the reported size are truncated; writes starting + * beyond the reported size generate -EINVAL. + */ +static ssize_t +nvgrace_gpu_write_mem(struct nvgrace_gpu_pci_core_device *nvdev, + size_t count, loff_t *ppos, const char __user *buf) +{ + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos); + u64 offset = *ppos & VFIO_PCI_OFFSET_MASK; + struct mem_region *memregion; + size_t mem_count; + int ret = 0; + + /* No need to do NULL check as caller does. */ + memregion = nvgrace_gpu_memregion(index, nvdev); + + if (offset >= memregion->bar_size) + return -EINVAL; + + /* Clip short the write request beyond reported BAR size */ + count = min(count, memregion->bar_size - (size_t)offset); + + /* + * Determine how many bytes to be actually written to the device memory. + * Do not write to the offset beyond available size. + */ + if (offset >= memregion->memlength) + goto exitfn; + + /* + * Only the device memory present on the hardware is mapped, which may + * not be power-of-2 aligned. Drop access outside the available device + * memory on the hardware. + */ + mem_count = min(count, memregion->memlength - (size_t)offset); + + ret = nvgrace_gpu_map_and_write(nvdev, buf, mem_count, ppos); + if (ret) + return ret; + +exitfn: + *ppos += count; + return count; +} + +static ssize_t +nvgrace_gpu_write(struct vfio_device *core_vdev, + const char __user *buf, size_t count, loff_t *ppos) +{ + struct nvgrace_gpu_pci_core_device *nvdev = + container_of(core_vdev, struct nvgrace_gpu_pci_core_device, + core_device.vdev); + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos); + + if (nvgrace_gpu_memregion(index, nvdev)) + return nvgrace_gpu_write_mem(nvdev, count, ppos, buf); + + if (index == VFIO_PCI_CONFIG_REGION_INDEX) + return nvgrace_gpu_write_config_emu(core_vdev, buf, count, ppos); + + return vfio_pci_core_write(core_vdev, buf, count, ppos); +} + +static const struct vfio_device_ops nvgrace_gpu_pci_ops = { + .name = "nvgrace-gpu-vfio-pci", + .init = vfio_pci_core_init_dev, + .release = vfio_pci_core_release_dev, + .open_device = nvgrace_gpu_open_device, + .close_device = nvgrace_gpu_close_device, + .ioctl = nvgrace_gpu_ioctl, + .device_feature = vfio_pci_core_ioctl_feature, + .read = nvgrace_gpu_read, + .write = nvgrace_gpu_write, + .mmap = nvgrace_gpu_mmap, + .request = vfio_pci_core_request, + .match = vfio_pci_core_match, + .bind_iommufd = vfio_iommufd_physical_bind, + .unbind_iommufd = vfio_iommufd_physical_unbind, + .attach_ioas = vfio_iommufd_physical_attach_ioas, + .detach_ioas = vfio_iommufd_physical_detach_ioas, +}; + +static const struct vfio_device_ops nvgrace_gpu_pci_core_ops = { + .name = "nvgrace-gpu-vfio-pci-core", + .init = vfio_pci_core_init_dev, + .release = vfio_pci_core_release_dev, + .open_device = nvgrace_gpu_open_device, + .close_device = vfio_pci_core_close_device, + .ioctl = vfio_pci_core_ioctl, + .device_feature = vfio_pci_core_ioctl_feature, + .read = vfio_pci_core_read, + .write = vfio_pci_core_write, + .mmap = vfio_pci_core_mmap, + .request = vfio_pci_core_request, + .match = vfio_pci_core_match, + .bind_iommufd = vfio_iommufd_physical_bind, + .unbind_iommufd = vfio_iommufd_physical_unbind, + .attach_ioas = vfio_iommufd_physical_attach_ioas, + .detach_ioas = vfio_iommufd_physical_detach_ioas, +}; + +static int +nvgrace_gpu_fetch_memory_property(struct pci_dev *pdev, + u64 *pmemphys, u64 *pmemlength) +{ + int ret; + + /* + * The memory information is present in the system ACPI tables as DSD + * properties nvidia,gpu-mem-base-pa and nvidia,gpu-mem-size. + */ + ret = device_property_read_u64(&pdev->dev, "nvidia,gpu-mem-base-pa", + pmemphys); + if (ret) + return ret; + + if (*pmemphys > type_max(phys_addr_t)) + return -EOVERFLOW; + + ret = device_property_read_u64(&pdev->dev, "nvidia,gpu-mem-size", + pmemlength); + if (ret) + return ret; + + if (*pmemlength > type_max(size_t)) + return -EOVERFLOW; + + /* + * If the C2C link is not up due to an error, the coherent device + * memory size is returned as 0. Fail in such case. + */ + if (*pmemlength == 0) + return -ENOMEM; + + return ret; +} + +static int +nvgrace_gpu_init_nvdev_struct(struct pci_dev *pdev, + struct nvgrace_gpu_pci_core_device *nvdev, + u64 memphys, u64 memlength) +{ + int ret = 0; + + /* + * The VM GPU device driver needs a non-cacheable region to support + * the MIG feature. Since the device memory is mapped as NORMAL cached, + * carve out a region from the end with a different NORMAL_NC + * property (called as reserved memory and represented as resmem). This + * region then is exposed as a 64b BAR (region 2 and 3) to the VM, while + * exposing the rest (termed as usable memory and represented using usemem) + * as cacheable 64b BAR (region 4 and 5). + * + * devmem (memlength) + * |-------------------------------------------------| + * | | + * usemem.memphys resmem.memphys + */ + nvdev->usemem.memphys = memphys; + + /* + * The device memory exposed to the VM is added to the kernel by the + * VM driver module in chunks of memory block size. Only the usable + * memory (usemem) is added to the kernel for usage by the VM + * workloads. Make the usable memory size memblock aligned. + */ + if (check_sub_overflow(memlength, RESMEM_SIZE, + &nvdev->usemem.memlength)) { + ret = -EOVERFLOW; + goto done; + } + + /* + * The USEMEM part of the device memory has to be MEMBLK_SIZE + * aligned. This is a hardwired ABI value between the GPU FW and + * VFIO driver. The VM device driver is also aware of it and make + * use of the value for its calculation to determine USEMEM size. + */ + nvdev->usemem.memlength = round_down(nvdev->usemem.memlength, + MEMBLK_SIZE); + if (nvdev->usemem.memlength == 0) { + ret = -EINVAL; + goto done; + } + + if ((check_add_overflow(nvdev->usemem.memphys, + nvdev->usemem.memlength, + &nvdev->resmem.memphys)) || + (check_sub_overflow(memlength, nvdev->usemem.memlength, + &nvdev->resmem.memlength))) { + ret = -EOVERFLOW; + goto done; + } + + /* + * The memory regions are exposed as BARs. Calculate and save + * the BAR size for them. + */ + nvdev->usemem.bar_size = roundup_pow_of_two(nvdev->usemem.memlength); + nvdev->resmem.bar_size = roundup_pow_of_two(nvdev->resmem.memlength); +done: + return ret; +} + +static int nvgrace_gpu_probe(struct pci_dev *pdev, + const struct pci_device_id *id) +{ + const struct vfio_device_ops *ops = &nvgrace_gpu_pci_core_ops; + struct nvgrace_gpu_pci_core_device *nvdev; + u64 memphys, memlength; + int ret; + + ret = nvgrace_gpu_fetch_memory_property(pdev, &memphys, &memlength); + if (!ret) + ops = &nvgrace_gpu_pci_ops; + + nvdev = vfio_alloc_device(nvgrace_gpu_pci_core_device, core_device.vdev, + &pdev->dev, ops); + if (IS_ERR(nvdev)) + return PTR_ERR(nvdev); + + dev_set_drvdata(&pdev->dev, &nvdev->core_device); + + if (ops == &nvgrace_gpu_pci_ops) { + /* + * Device memory properties are identified in the host ACPI + * table. Set the nvgrace_gpu_pci_core_device structure. + */ + ret = nvgrace_gpu_init_nvdev_struct(pdev, nvdev, + memphys, memlength); + if (ret) + goto out_put_vdev; + } + + ret = vfio_pci_core_register_device(&nvdev->core_device); + if (ret) + goto out_put_vdev; + + return ret; + +out_put_vdev: + vfio_put_device(&nvdev->core_device.vdev); + return ret; +} + +static void nvgrace_gpu_remove(struct pci_dev *pdev) +{ + struct vfio_pci_core_device *core_device = dev_get_drvdata(&pdev->dev); + + vfio_pci_core_unregister_device(core_device); + vfio_put_device(&core_device->vdev); +} + +static const struct pci_device_id nvgrace_gpu_vfio_pci_table[] = { + /* GH200 120GB */ + { PCI_DRIVER_OVERRIDE_DEVICE_VFIO(PCI_VENDOR_ID_NVIDIA, 0x2342) }, + /* GH200 480GB */ + { PCI_DRIVER_OVERRIDE_DEVICE_VFIO(PCI_VENDOR_ID_NVIDIA, 0x2345) }, + {} +}; + +MODULE_DEVICE_TABLE(pci, nvgrace_gpu_vfio_pci_table); + +static struct pci_driver nvgrace_gpu_vfio_pci_driver = { + .name = KBUILD_MODNAME, + .id_table = nvgrace_gpu_vfio_pci_table, + .probe = nvgrace_gpu_probe, + .remove = nvgrace_gpu_remove, + .err_handler = &vfio_pci_core_err_handlers, + .driver_managed_dma = true, +}; + +module_pci_driver(nvgrace_gpu_vfio_pci_driver); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Ankit Agrawal "); +MODULE_AUTHOR("Aniket Agashe "); +MODULE_DESCRIPTION("VFIO NVGRACE GPU PF - User Level driver for NVIDIA devices with CPU coherently accessible device memory"); -- cgit v1.2.3 From 8512ed256334f6637fc0699ce794792c357544ec Mon Sep 17 00:00:00 2001 From: Brett Creeley Date: Tue, 27 Feb 2024 16:32:04 -0800 Subject: vfio/pds: Always clear the save/restore FDs on reset After reset the VFIO device state will always be put in VFIO_DEVICE_STATE_RUNNING, but the save/restore files will only be cleared if the previous state was VFIO_DEVICE_STATE_ERROR. This can/will cause the restore/save files to be leaked if/when the migration state machine transitions through the states that re-allocates these files. Fix this by always clearing the restore/save files for resets. Fixes: 7dabb1bcd177 ("vfio/pds: Add support for firmware recovery") Cc: stable@vger.kernel.org Signed-off-by: Brett Creeley Reviewed-by: Shannon Nelson Reviewed-by: Kevin Tian Link: https://lore.kernel.org/r/20240228003205.47311-2-brett.creeley@amd.com Signed-off-by: Alex Williamson --- drivers/vfio/pci/pds/vfio_dev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/vfio/pci/pds/vfio_dev.c b/drivers/vfio/pci/pds/vfio_dev.c index 4c351c59d05a..a286ebcc7112 100644 --- a/drivers/vfio/pci/pds/vfio_dev.c +++ b/drivers/vfio/pci/pds/vfio_dev.c @@ -32,9 +32,9 @@ again: mutex_lock(&pds_vfio->reset_mutex); if (pds_vfio->deferred_reset) { pds_vfio->deferred_reset = false; + pds_vfio_put_restore_file(pds_vfio); + pds_vfio_put_save_file(pds_vfio); if (pds_vfio->state == VFIO_DEVICE_STATE_ERROR) { - pds_vfio_put_restore_file(pds_vfio); - pds_vfio_put_save_file(pds_vfio); pds_vfio_dirty_disable(pds_vfio, false); } pds_vfio->state = pds_vfio->deferred_reset_state; -- cgit v1.2.3 From ec29d22caea85d9b391f9df780a0e61597f18778 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Mon, 26 Feb 2024 12:09:26 +0100 Subject: vfio: amba: Rename pl330_ids[] to vfio_amba_ids[] Obviously drivers/vfio/platform/vfio_amba.c started its life as a simplified copy of drivers/dma/pl330.c, but not all variable names were updated. Signed-off-by: Geert Uytterhoeven Reviewed-by: Jason Gunthorpe Link: https://lore.kernel.org/r/1d1b873b59b208547439225aee1f24d6f2512a1f.1708945194.git.geert+renesas@glider.be Signed-off-by: Alex Williamson --- drivers/vfio/platform/vfio_amba.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/vfio/platform/vfio_amba.c b/drivers/vfio/platform/vfio_amba.c index 6464b3939ebc..485c6f9161a9 100644 --- a/drivers/vfio/platform/vfio_amba.c +++ b/drivers/vfio/platform/vfio_amba.c @@ -122,16 +122,16 @@ static const struct vfio_device_ops vfio_amba_ops = { .detach_ioas = vfio_iommufd_physical_detach_ioas, }; -static const struct amba_id pl330_ids[] = { +static const struct amba_id vfio_amba_ids[] = { { 0, 0 }, }; -MODULE_DEVICE_TABLE(amba, pl330_ids); +MODULE_DEVICE_TABLE(amba, vfio_amba_ids); static struct amba_driver vfio_amba_driver = { .probe = vfio_amba_probe, .remove = vfio_amba_remove, - .id_table = pl330_ids, + .id_table = vfio_amba_ids, .drv = { .name = "vfio-amba", .owner = THIS_MODULE, -- cgit v1.2.3 From 81617c17bf58f008a57da74b97e60a0bf8e971fd Mon Sep 17 00:00:00 2001 From: Ankit Agrawal Date: Thu, 29 Feb 2024 19:39:34 +0000 Subject: vfio/nvgrace-gpu: Convey kvm to map device memory region as noncached The NVIDIA Grace Hopper GPUs have device memory that is supposed to be used as a regular RAM. It is accessible through CPU-GPU chip-to-chip cache coherent interconnect and is present in the system physical address space. The device memory is split into two regions - termed as usemem and resmem - in the system physical address space, with each region mapped and exposed to the VM as a separate fake device BAR [1]. Owing to a hardware defect for Multi-Instance GPU (MIG) feature [2], there is a requirement - as a workaround - for the resmem BAR to display uncached memory characteristics. Based on [3], on system with FWB enabled such as Grace Hopper, the requisite properties (uncached, unaligned access) can be achieved through a VM mapping (S1) of NORMAL_NC and host mapping (S2) of MT_S2_FWB_NORMAL_NC. KVM currently maps the MMIO region in S2 as MT_S2_FWB_DEVICE_nGnRE by default. The fake device BARs thus displays DEVICE_nGnRE behavior in the VM. The following table summarizes the behavior for the various S1 and S2 mapping combinations for systems with FWB enabled [3]. S1 | S2 | Result NORMAL_NC | NORMAL_NC | NORMAL_NC NORMAL_NC | DEVICE_nGnRE | DEVICE_nGnRE Recently a change was added that modifies this default behavior and make KVM map MMIO as MT_S2_FWB_NORMAL_NC when a VMA flag VM_ALLOW_ANY_UNCACHED is set [4]. Setting S2 as MT_S2_FWB_NORMAL_NC provides the desired behavior (uncached, unaligned access) for resmem. To use VM_ALLOW_ANY_UNCACHED flag, the platform must guarantee that no action taken on the MMIO mapping can trigger an uncontained failure. The Grace Hopper satisfies this requirement. So set the VM_ALLOW_ANY_UNCACHED flag in the VMA. Applied over next-20240227. base-commit: 22ba90670a51 Link: https://lore.kernel.org/all/20240220115055.23546-4-ankita@nvidia.com/ [1] Link: https://www.nvidia.com/en-in/technologies/multi-instance-gpu/ [2] Link: https://developer.arm.com/documentation/ddi0487/latest/ section D8.5.5 [3] Link: https://lore.kernel.org/all/20240224150546.368-1-ankita@nvidia.com/ [4] Cc: Alex Williamson Cc: Kevin Tian Cc: Jason Gunthorpe Cc: Vikram Sethi Cc: Zhi Wang Signed-off-by: Ankit Agrawal Reviewed-by: Jason Gunthorpe Link: https://lore.kernel.org/r/20240229193934.2417-1-ankita@nvidia.com Signed-off-by: Alex Williamson --- drivers/vfio/pci/nvgrace-gpu/main.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/vfio/pci/nvgrace-gpu/main.c b/drivers/vfio/pci/nvgrace-gpu/main.c index 25814006352d..a7fd018aa548 100644 --- a/drivers/vfio/pci/nvgrace-gpu/main.c +++ b/drivers/vfio/pci/nvgrace-gpu/main.c @@ -160,8 +160,17 @@ static int nvgrace_gpu_mmap(struct vfio_device *core_vdev, * The carved out region of the device memory needs the NORMAL_NC * property. Communicate as such to the hypervisor. */ - if (index == RESMEM_REGION_INDEX) + if (index == RESMEM_REGION_INDEX) { + /* + * The nvgrace-gpu module has no issues with uncontained + * failures on NORMAL_NC accesses. VM_ALLOW_ANY_UNCACHED is + * set to communicate to the KVM to S2 map as NORMAL_NC. + * This opens up guest usage of NORMAL_NC for this mapping. + */ + vm_flags_set(vma, VM_ALLOW_ANY_UNCACHED); + vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot); + } /* * Perform a PFN map to the memory and back the device BAR by the -- cgit v1.2.3 From 5b992412776cdf2ec88b5b5138112e6b36e47995 Mon Sep 17 00:00:00 2001 From: Alex Williamson Date: Thu, 29 Feb 2024 15:35:40 -0700 Subject: Revert "vfio/type1: Unpin zero pages" This reverts commit 873aefb376bbc0ed1dd2381ea1d6ec88106fdbd4. This was a heinous workaround and it turns out it's been fixed in mm twice since it was introduced. Most recently, commit c8070b787519 ("mm: Don't pin ZERO_PAGE in pin_user_pages()") would have prevented running up the zeropage refcount, but even before that commit 84209e87c696 ("mm/gup: reliable R/O long-term pinning in COW mappings") avoids the vfio use case from pinning the zeropage at all, instead replacing it with exclusive anonymous pages. Remove this now useless overhead. Suggested-by: David Hildenbrand Reviewed-by: David Hildenbrand Reviewed-by: Kevin Tian Link: https://lore.kernel.org/r/20240229223544.257207-1-alex.williamson@redhat.com Signed-off-by: Alex Williamson --- drivers/vfio/vfio_iommu_type1.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c index b2854d7939ce..b5c15fe8f9fc 100644 --- a/drivers/vfio/vfio_iommu_type1.c +++ b/drivers/vfio/vfio_iommu_type1.c @@ -567,18 +567,6 @@ static int vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr, ret = pin_user_pages_remote(mm, vaddr, npages, flags | FOLL_LONGTERM, pages, NULL); if (ret > 0) { - int i; - - /* - * The zero page is always resident, we don't need to pin it - * and it falls into our invalid/reserved test so we don't - * unpin in put_pfn(). Unpin all zero pages in the batch here. - */ - for (i = 0 ; i < ret; i++) { - if (unlikely(is_zero_pfn(page_to_pfn(pages[i])))) - unpin_user_page(pages[i]); - } - *pfn = page_to_pfn(pages[0]); goto done; } -- cgit v1.2.3 From fd94213e1417d28b933d4a3d41ad02eac1b29ec2 Mon Sep 17 00:00:00 2001 From: Shameer Kolothum Date: Thu, 29 Feb 2024 09:11:52 +0000 Subject: hisi_acc_vfio_pci: Remove the deferred_reset logic The deferred_reset logic was added to vfio migration drivers to prevent a circular locking dependency with respect to mm_lock and state mutex. This is mainly because of the copy_to/from_user() functions(which takes mm_lock) invoked under state mutex. But for HiSilicon driver, the only place where we now hold the state mutex for copy_to_user is during the PRE_COPY IOCTL. So for pre_copy, release the lock as soon as we have updated the data and perform copy_to_user without state mutex. By this, we can get rid of the deferred_reset logic. Link: https://lore.kernel.org/kvm/20240220132459.GM13330@nvidia.com/ Signed-off-by: Shameer Kolothum Reviewed-by: Brett Creeley Reviewed-by: Kevin Tian Reviewed-by: Jason Gunthorpe Link: https://lore.kernel.org/r/20240229091152.56664-1-shameerali.kolothum.thodi@huawei.com Signed-off-by: Alex Williamson --- drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 48 +++++++------------------- drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h | 6 ++-- 2 files changed, 14 insertions(+), 40 deletions(-) diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c index 4d27465c8f1a..9a3e97108ace 100644 --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c @@ -630,25 +630,11 @@ static void hisi_acc_vf_disable_fds(struct hisi_acc_vf_core_device *hisi_acc_vde } } -/* - * This function is called in all state_mutex unlock cases to - * handle a 'deferred_reset' if exists. - */ -static void -hisi_acc_vf_state_mutex_unlock(struct hisi_acc_vf_core_device *hisi_acc_vdev) +static void hisi_acc_vf_reset(struct hisi_acc_vf_core_device *hisi_acc_vdev) { -again: - spin_lock(&hisi_acc_vdev->reset_lock); - if (hisi_acc_vdev->deferred_reset) { - hisi_acc_vdev->deferred_reset = false; - spin_unlock(&hisi_acc_vdev->reset_lock); - hisi_acc_vdev->vf_qm_state = QM_NOT_READY; - hisi_acc_vdev->mig_state = VFIO_DEVICE_STATE_RUNNING; - hisi_acc_vf_disable_fds(hisi_acc_vdev); - goto again; - } - mutex_unlock(&hisi_acc_vdev->state_mutex); - spin_unlock(&hisi_acc_vdev->reset_lock); + hisi_acc_vdev->vf_qm_state = QM_NOT_READY; + hisi_acc_vdev->mig_state = VFIO_DEVICE_STATE_RUNNING; + hisi_acc_vf_disable_fds(hisi_acc_vdev); } static void hisi_acc_vf_start_device(struct hisi_acc_vf_core_device *hisi_acc_vdev) @@ -804,8 +790,10 @@ static long hisi_acc_vf_precopy_ioctl(struct file *filp, info.dirty_bytes = 0; info.initial_bytes = migf->total_length - *pos; + mutex_unlock(&migf->lock); + mutex_unlock(&hisi_acc_vdev->state_mutex); - ret = copy_to_user((void __user *)arg, &info, minsz) ? -EFAULT : 0; + return copy_to_user((void __user *)arg, &info, minsz) ? -EFAULT : 0; out: mutex_unlock(&migf->lock); mutex_unlock(&hisi_acc_vdev->state_mutex); @@ -1071,7 +1059,7 @@ hisi_acc_vfio_pci_set_device_state(struct vfio_device *vdev, break; } } - hisi_acc_vf_state_mutex_unlock(hisi_acc_vdev); + mutex_unlock(&hisi_acc_vdev->state_mutex); return res; } @@ -1092,7 +1080,7 @@ hisi_acc_vfio_pci_get_device_state(struct vfio_device *vdev, mutex_lock(&hisi_acc_vdev->state_mutex); *curr_state = hisi_acc_vdev->mig_state; - hisi_acc_vf_state_mutex_unlock(hisi_acc_vdev); + mutex_unlock(&hisi_acc_vdev->state_mutex); return 0; } @@ -1104,21 +1092,9 @@ static void hisi_acc_vf_pci_aer_reset_done(struct pci_dev *pdev) VFIO_MIGRATION_STOP_COPY) return; - /* - * As the higher VFIO layers are holding locks across reset and using - * those same locks with the mm_lock we need to prevent ABBA deadlock - * with the state_mutex and mm_lock. - * In case the state_mutex was taken already we defer the cleanup work - * to the unlock flow of the other running context. - */ - spin_lock(&hisi_acc_vdev->reset_lock); - hisi_acc_vdev->deferred_reset = true; - if (!mutex_trylock(&hisi_acc_vdev->state_mutex)) { - spin_unlock(&hisi_acc_vdev->reset_lock); - return; - } - spin_unlock(&hisi_acc_vdev->reset_lock); - hisi_acc_vf_state_mutex_unlock(hisi_acc_vdev); + mutex_lock(&hisi_acc_vdev->state_mutex); + hisi_acc_vf_reset(hisi_acc_vdev); + mutex_unlock(&hisi_acc_vdev->state_mutex); } static int hisi_acc_vf_qm_init(struct hisi_acc_vf_core_device *hisi_acc_vdev) diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h index dcabfeec6ca1..5bab46602fad 100644 --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h @@ -98,8 +98,8 @@ struct hisi_acc_vf_migration_file { struct hisi_acc_vf_core_device { struct vfio_pci_core_device core_device; - u8 match_done:1; - u8 deferred_reset:1; + u8 match_done; + /* For migration state */ struct mutex state_mutex; enum vfio_device_mig_state mig_state; @@ -109,8 +109,6 @@ struct hisi_acc_vf_core_device { struct hisi_qm vf_qm; u32 vf_qm_state; int vf_id; - /* For reset handler */ - spinlock_t reset_lock; struct hisi_acc_vf_migration_file *resuming_migf; struct hisi_acc_vf_migration_file *saving_migf; }; -- cgit v1.2.3 From 1f92d6a7c65f390617463532b408030a5a168097 Mon Sep 17 00:00:00 2001 From: "Ricardo B. Marliere" Date: Fri, 1 Mar 2024 14:51:47 -0300 Subject: vfio/mdpy: make mdpy_class constant Since commit 43a7206b0963 ("driver core: class: make class_register() take a const *"), the driver core allows for struct class to be in read-only memory, so move the mdpy_class structure to be declared at build time placing it into read-only memory, instead of having to be dynamically allocated at boot time. Cc: Greg Kroah-Hartman Suggested-by: Greg Kroah-Hartman Signed-off-by: Ricardo B. Marliere Link: https://lore.kernel.org/r/20240301-class_cleanup-vfio-v1-1-9236d69083f5@marliere.net Signed-off-by: Alex Williamson --- samples/vfio-mdev/mdpy.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/samples/vfio-mdev/mdpy.c b/samples/vfio-mdev/mdpy.c index 72ea5832c927..27795501de6e 100644 --- a/samples/vfio-mdev/mdpy.c +++ b/samples/vfio-mdev/mdpy.c @@ -84,7 +84,9 @@ static struct mdev_type *mdpy_mdev_types[] = { }; static dev_t mdpy_devt; -static struct class *mdpy_class; +static const struct class mdpy_class = { + .name = MDPY_CLASS_NAME, +}; static struct cdev mdpy_cdev; static struct device mdpy_dev; static struct mdev_parent mdpy_parent; @@ -709,13 +711,10 @@ static int __init mdpy_dev_init(void) if (ret) goto err_cdev; - mdpy_class = class_create(MDPY_CLASS_NAME); - if (IS_ERR(mdpy_class)) { - pr_err("Error: failed to register mdpy_dev class\n"); - ret = PTR_ERR(mdpy_class); + ret = class_register(&mdpy_class); + if (ret) goto err_driver; - } - mdpy_dev.class = mdpy_class; + mdpy_dev.class = &mdpy_class; mdpy_dev.release = mdpy_device_release; dev_set_name(&mdpy_dev, "%s", MDPY_NAME); @@ -735,7 +734,7 @@ err_device: device_del(&mdpy_dev); err_put: put_device(&mdpy_dev); - class_destroy(mdpy_class); + class_unregister(&mdpy_class); err_driver: mdev_unregister_driver(&mdpy_driver); err_cdev: @@ -753,8 +752,7 @@ static void __exit mdpy_dev_exit(void) mdev_unregister_driver(&mdpy_driver); cdev_del(&mdpy_cdev); unregister_chrdev_region(mdpy_devt, MINORMASK + 1); - class_destroy(mdpy_class); - mdpy_class = NULL; + class_unregister(&mdpy_class); } module_param_named(count, mdpy_driver.max_instances, int, 0444); -- cgit v1.2.3 From 626f534d774caaccd6861fbccee45a1a6a8781b3 Mon Sep 17 00:00:00 2001 From: "Ricardo B. Marliere" Date: Fri, 1 Mar 2024 14:51:48 -0300 Subject: vfio/mbochs: make mbochs_class constant Since commit 43a7206b0963 ("driver core: class: make class_register() take a const *"), the driver core allows for struct class to be in read-only memory, so move the mbochs_class structure to be declared at build time placing it into read-only memory, instead of having to be dynamically allocated at boot time. Cc: Greg Kroah-Hartman Suggested-by: Greg Kroah-Hartman Signed-off-by: Ricardo B. Marliere Link: https://lore.kernel.org/r/20240301-class_cleanup-vfio-v1-2-9236d69083f5@marliere.net Signed-off-by: Alex Williamson --- samples/vfio-mdev/mbochs.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c index 93405264ff23..9062598ea03d 100644 --- a/samples/vfio-mdev/mbochs.c +++ b/samples/vfio-mdev/mbochs.c @@ -133,7 +133,9 @@ static struct mdev_type *mbochs_mdev_types[] = { }; static dev_t mbochs_devt; -static struct class *mbochs_class; +static const struct class mbochs_class = { + .name = MBOCHS_CLASS_NAME, +}; static struct cdev mbochs_cdev; static struct device mbochs_dev; static struct mdev_parent mbochs_parent; @@ -1422,13 +1424,10 @@ static int __init mbochs_dev_init(void) if (ret) goto err_cdev; - mbochs_class = class_create(MBOCHS_CLASS_NAME); - if (IS_ERR(mbochs_class)) { - pr_err("Error: failed to register mbochs_dev class\n"); - ret = PTR_ERR(mbochs_class); + ret = class_register(&mbochs_class); + if (ret) goto err_driver; - } - mbochs_dev.class = mbochs_class; + mbochs_dev.class = &mbochs_class; mbochs_dev.release = mbochs_device_release; dev_set_name(&mbochs_dev, "%s", MBOCHS_NAME); @@ -1448,7 +1447,7 @@ err_device: device_del(&mbochs_dev); err_put: put_device(&mbochs_dev); - class_destroy(mbochs_class); + class_unregister(&mbochs_class); err_driver: mdev_unregister_driver(&mbochs_driver); err_cdev: @@ -1466,8 +1465,7 @@ static void __exit mbochs_dev_exit(void) mdev_unregister_driver(&mbochs_driver); cdev_del(&mbochs_cdev); unregister_chrdev_region(mbochs_devt, MINORMASK + 1); - class_destroy(mbochs_class); - mbochs_class = NULL; + class_unregister(&mbochs_class); } MODULE_IMPORT_NS(DMA_BUF); -- cgit v1.2.3 From 821b8f6bf84897aeceb230ff4d446b8aa5336acd Mon Sep 17 00:00:00 2001 From: Yishai Hadas Date: Wed, 6 Mar 2024 12:56:24 +0200 Subject: vfio/mlx5: Enforce PRE_COPY support Enable live migration only once the firmware supports PRE_COPY. PRE_COPY has been supported by the firmware for a long time already [1] and is required to achieve a low downtime upon live migration. This lets us clean up some old code that is not applicable those days while PRE_COPY is fully supported by the firmware. [1] The minimum firmware version that supports PRE_COPY is 28.36.1010, it was released in January 2023. No firmware without PRE_COPY support ever available to users. Signed-off-by: Yishai Hadas Link: https://lore.kernel.org/r/20240306105624.114830-1-yishaih@nvidia.com Signed-off-by: Alex Williamson --- drivers/vfio/pci/mlx5/cmd.c | 83 ++++++++++++++++++++++++-------- drivers/vfio/pci/mlx5/cmd.h | 6 --- drivers/vfio/pci/mlx5/main.c | 109 ++++--------------------------------------- 3 files changed, 71 insertions(+), 127 deletions(-) diff --git a/drivers/vfio/pci/mlx5/cmd.c b/drivers/vfio/pci/mlx5/cmd.c index c54bcd5d0917..41a4b0cf4297 100644 --- a/drivers/vfio/pci/mlx5/cmd.c +++ b/drivers/vfio/pci/mlx5/cmd.c @@ -233,6 +233,10 @@ void mlx5vf_cmd_set_migratable(struct mlx5vf_pci_core_device *mvdev, if (!MLX5_CAP_GEN(mvdev->mdev, migration)) goto end; + if (!(MLX5_CAP_GEN_2(mvdev->mdev, migration_multi_load) && + MLX5_CAP_GEN_2(mvdev->mdev, migration_tracking_state))) + goto end; + mvdev->vf_id = pci_iov_vf_id(pdev); if (mvdev->vf_id < 0) goto end; @@ -262,17 +266,14 @@ void mlx5vf_cmd_set_migratable(struct mlx5vf_pci_core_device *mvdev, mvdev->migrate_cap = 1; mvdev->core_device.vdev.migration_flags = VFIO_MIGRATION_STOP_COPY | - VFIO_MIGRATION_P2P; + VFIO_MIGRATION_P2P | + VFIO_MIGRATION_PRE_COPY; + mvdev->core_device.vdev.mig_ops = mig_ops; init_completion(&mvdev->tracker_comp); if (MLX5_CAP_GEN(mvdev->mdev, adv_virtualization)) mvdev->core_device.vdev.log_ops = log_ops; - if (MLX5_CAP_GEN_2(mvdev->mdev, migration_multi_load) && - MLX5_CAP_GEN_2(mvdev->mdev, migration_tracking_state)) - mvdev->core_device.vdev.migration_flags |= - VFIO_MIGRATION_PRE_COPY; - if (MLX5_CAP_GEN_2(mvdev->mdev, migration_in_chunks)) mvdev->chunk_mode = 1; @@ -414,6 +415,50 @@ void mlx5vf_free_data_buffer(struct mlx5_vhca_data_buffer *buf) kfree(buf); } +static int mlx5vf_add_migration_pages(struct mlx5_vhca_data_buffer *buf, + unsigned int npages) +{ + unsigned int to_alloc = npages; + struct page **page_list; + unsigned long filled; + unsigned int to_fill; + int ret; + + to_fill = min_t(unsigned int, npages, PAGE_SIZE / sizeof(*page_list)); + page_list = kvzalloc(to_fill * sizeof(*page_list), GFP_KERNEL_ACCOUNT); + if (!page_list) + return -ENOMEM; + + do { + filled = alloc_pages_bulk_array(GFP_KERNEL_ACCOUNT, to_fill, + page_list); + if (!filled) { + ret = -ENOMEM; + goto err; + } + to_alloc -= filled; + ret = sg_alloc_append_table_from_pages( + &buf->table, page_list, filled, 0, + filled << PAGE_SHIFT, UINT_MAX, SG_MAX_SINGLE_ALLOC, + GFP_KERNEL_ACCOUNT); + + if (ret) + goto err; + buf->allocated_length += filled * PAGE_SIZE; + /* clean input for another bulk allocation */ + memset(page_list, 0, filled * sizeof(*page_list)); + to_fill = min_t(unsigned int, to_alloc, + PAGE_SIZE / sizeof(*page_list)); + } while (to_alloc > 0); + + kvfree(page_list); + return 0; + +err: + kvfree(page_list); + return ret; +} + struct mlx5_vhca_data_buffer * mlx5vf_alloc_data_buffer(struct mlx5_vf_migration_file *migf, size_t length, @@ -680,22 +725,20 @@ int mlx5vf_cmd_save_vhca_state(struct mlx5vf_pci_core_device *mvdev, goto err_out; } - if (MLX5VF_PRE_COPY_SUPP(mvdev)) { - if (async_data->stop_copy_chunk) { - u8 header_idx = buf->stop_copy_chunk_num ? - buf->stop_copy_chunk_num - 1 : 0; + if (async_data->stop_copy_chunk) { + u8 header_idx = buf->stop_copy_chunk_num ? + buf->stop_copy_chunk_num - 1 : 0; - header_buf = migf->buf_header[header_idx]; - migf->buf_header[header_idx] = NULL; - } + header_buf = migf->buf_header[header_idx]; + migf->buf_header[header_idx] = NULL; + } - if (!header_buf) { - header_buf = mlx5vf_get_data_buffer(migf, - sizeof(struct mlx5_vf_migration_header), DMA_NONE); - if (IS_ERR(header_buf)) { - err = PTR_ERR(header_buf); - goto err_free; - } + if (!header_buf) { + header_buf = mlx5vf_get_data_buffer(migf, + sizeof(struct mlx5_vf_migration_header), DMA_NONE); + if (IS_ERR(header_buf)) { + err = PTR_ERR(header_buf); + goto err_free; } } diff --git a/drivers/vfio/pci/mlx5/cmd.h b/drivers/vfio/pci/mlx5/cmd.h index 707393df36c4..df421dc6de04 100644 --- a/drivers/vfio/pci/mlx5/cmd.h +++ b/drivers/vfio/pci/mlx5/cmd.h @@ -13,9 +13,6 @@ #include #include -#define MLX5VF_PRE_COPY_SUPP(mvdev) \ - ((mvdev)->core_device.vdev.migration_flags & VFIO_MIGRATION_PRE_COPY) - enum mlx5_vf_migf_state { MLX5_MIGF_STATE_ERROR = 1, MLX5_MIGF_STATE_PRE_COPY_ERROR, @@ -25,7 +22,6 @@ enum mlx5_vf_migf_state { }; enum mlx5_vf_load_state { - MLX5_VF_LOAD_STATE_READ_IMAGE_NO_HEADER, MLX5_VF_LOAD_STATE_READ_HEADER, MLX5_VF_LOAD_STATE_PREP_HEADER_DATA, MLX5_VF_LOAD_STATE_READ_HEADER_DATA, @@ -228,8 +224,6 @@ struct mlx5_vhca_data_buffer * mlx5vf_get_data_buffer(struct mlx5_vf_migration_file *migf, size_t length, enum dma_data_direction dma_dir); void mlx5vf_put_data_buffer(struct mlx5_vhca_data_buffer *buf); -int mlx5vf_add_migration_pages(struct mlx5_vhca_data_buffer *buf, - unsigned int npages); struct page *mlx5vf_get_migration_page(struct mlx5_vhca_data_buffer *buf, unsigned long offset); void mlx5vf_state_mutex_unlock(struct mlx5vf_pci_core_device *mvdev); diff --git a/drivers/vfio/pci/mlx5/main.c b/drivers/vfio/pci/mlx5/main.c index 3982fcf60cf2..61d9b0f9146d 100644 --- a/drivers/vfio/pci/mlx5/main.c +++ b/drivers/vfio/pci/mlx5/main.c @@ -65,50 +65,6 @@ mlx5vf_get_migration_page(struct mlx5_vhca_data_buffer *buf, return NULL; } -int mlx5vf_add_migration_pages(struct mlx5_vhca_data_buffer *buf, - unsigned int npages) -{ - unsigned int to_alloc = npages; - struct page **page_list; - unsigned long filled; - unsigned int to_fill; - int ret; - - to_fill = min_t(unsigned int, npages, PAGE_SIZE / sizeof(*page_list)); - page_list = kvzalloc(to_fill * sizeof(*page_list), GFP_KERNEL_ACCOUNT); - if (!page_list) - return -ENOMEM; - - do { - filled = alloc_pages_bulk_array(GFP_KERNEL_ACCOUNT, to_fill, - page_list); - if (!filled) { - ret = -ENOMEM; - goto err; - } - to_alloc -= filled; - ret = sg_alloc_append_table_from_pages( - &buf->table, page_list, filled, 0, - filled << PAGE_SHIFT, UINT_MAX, SG_MAX_SINGLE_ALLOC, - GFP_KERNEL_ACCOUNT); - - if (ret) - goto err; - buf->allocated_length += filled * PAGE_SIZE; - /* clean input for another bulk allocation */ - memset(page_list, 0, filled * sizeof(*page_list)); - to_fill = min_t(unsigned int, to_alloc, - PAGE_SIZE / sizeof(*page_list)); - } while (to_alloc > 0); - - kvfree(page_list); - return 0; - -err: - kvfree(page_list); - return ret; -} - static void mlx5vf_disable_fd(struct mlx5_vf_migration_file *migf) { mutex_lock(&migf->lock); @@ -777,36 +733,6 @@ mlx5vf_append_page_to_mig_buf(struct mlx5_vhca_data_buffer *vhca_buf, return 0; } -static int -mlx5vf_resume_read_image_no_header(struct mlx5_vhca_data_buffer *vhca_buf, - loff_t requested_length, - const char __user **buf, size_t *len, - loff_t *pos, ssize_t *done) -{ - int ret; - - if (requested_length > MAX_LOAD_SIZE) - return -ENOMEM; - - if (vhca_buf->allocated_length < requested_length) { - ret = mlx5vf_add_migration_pages( - vhca_buf, - DIV_ROUND_UP(requested_length - vhca_buf->allocated_length, - PAGE_SIZE)); - if (ret) - return ret; - } - - while (*len) { - ret = mlx5vf_append_page_to_mig_buf(vhca_buf, buf, len, pos, - done); - if (ret) - return ret; - } - - return 0; -} - static ssize_t mlx5vf_resume_read_image(struct mlx5_vf_migration_file *migf, struct mlx5_vhca_data_buffer *vhca_buf, @@ -1038,13 +964,6 @@ static ssize_t mlx5vf_resume_write(struct file *filp, const char __user *buf, migf->load_state = MLX5_VF_LOAD_STATE_READ_IMAGE; break; } - case MLX5_VF_LOAD_STATE_READ_IMAGE_NO_HEADER: - ret = mlx5vf_resume_read_image_no_header(vhca_buf, - requested_length, - &buf, &len, pos, &done); - if (ret) - goto out_unlock; - break; case MLX5_VF_LOAD_STATE_READ_IMAGE: ret = mlx5vf_resume_read_image(migf, vhca_buf, migf->record_size, @@ -1114,21 +1033,16 @@ mlx5vf_pci_resume_device_data(struct mlx5vf_pci_core_device *mvdev) } migf->buf[0] = buf; - if (MLX5VF_PRE_COPY_SUPP(mvdev)) { - buf = mlx5vf_alloc_data_buffer(migf, - sizeof(struct mlx5_vf_migration_header), DMA_NONE); - if (IS_ERR(buf)) { - ret = PTR_ERR(buf); - goto out_buf; - } - - migf->buf_header[0] = buf; - migf->load_state = MLX5_VF_LOAD_STATE_READ_HEADER; - } else { - /* Initial state will be to read the image */ - migf->load_state = MLX5_VF_LOAD_STATE_READ_IMAGE_NO_HEADER; + buf = mlx5vf_alloc_data_buffer(migf, + sizeof(struct mlx5_vf_migration_header), DMA_NONE); + if (IS_ERR(buf)) { + ret = PTR_ERR(buf); + goto out_buf; } + migf->buf_header[0] = buf; + migf->load_state = MLX5_VF_LOAD_STATE_READ_HEADER; + stream_open(migf->filp->f_inode, migf->filp); mutex_init(&migf->lock); INIT_LIST_HEAD(&migf->buf_list); @@ -1262,13 +1176,6 @@ mlx5vf_pci_step_device_state_locked(struct mlx5vf_pci_core_device *mvdev, } if (cur == VFIO_DEVICE_STATE_RESUMING && new == VFIO_DEVICE_STATE_STOP) { - if (!MLX5VF_PRE_COPY_SUPP(mvdev)) { - ret = mlx5vf_cmd_load_vhca_state(mvdev, - mvdev->resuming_migf, - mvdev->resuming_migf->buf[0]); - if (ret) - return ERR_PTR(ret); - } mlx5vf_disable_fds(mvdev, NULL); return NULL; } -- cgit v1.2.3 From 9b27b117e29f63d0db3fcc474f645d6f4af6d3e4 Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Fri, 8 Mar 2024 09:51:19 +0100 Subject: vfio/platform: Convert to platform remove callback returning void MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The .remove() callback for a platform driver returns an int which makes many driver authors wrongly assume it's possible to do error handling by returning an error code. However the value returned is ignored (apart from emitting a warning) and this typically results in resource leaks. To improve here there is a quest to make the remove callback return void. In the first step of this quest all drivers are converted to .remove_new(), which already returns void. Eventually after all drivers are converted, .remove_new() will be renamed to .remove(). Trivially convert this driver from always returning zero in the remove callback to the void returning variant. Signed-off-by: Uwe Kleine-König Reviewed-by: Jason Gunthorpe Reviewed-by: Eric Auger Link: https://lore.kernel.org/r/79d3df42fe5b359a05b8061631e72e5ed249b234.1709886922.git.u.kleine-koenig@pengutronix.de Signed-off-by: Alex Williamson --- drivers/vfio/platform/vfio_platform.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c index 8cf22fa65baa..42d1462c5e19 100644 --- a/drivers/vfio/platform/vfio_platform.c +++ b/drivers/vfio/platform/vfio_platform.c @@ -85,14 +85,13 @@ static void vfio_platform_release_dev(struct vfio_device *core_vdev) vfio_platform_release_common(vdev); } -static int vfio_platform_remove(struct platform_device *pdev) +static void vfio_platform_remove(struct platform_device *pdev) { struct vfio_platform_device *vdev = dev_get_drvdata(&pdev->dev); vfio_unregister_group_dev(&vdev->vdev); pm_runtime_disable(vdev->device); vfio_put_device(&vdev->vdev); - return 0; } static const struct vfio_device_ops vfio_platform_ops = { @@ -113,7 +112,7 @@ static const struct vfio_device_ops vfio_platform_ops = { static struct platform_driver vfio_platform_driver = { .probe = vfio_platform_probe, - .remove = vfio_platform_remove, + .remove_new = vfio_platform_remove, .driver = { .name = "vfio-platform", }, -- cgit v1.2.3 From 457f7308254756b6e4b8fc3876cb770dcf0e7cc7 Mon Sep 17 00:00:00 2001 From: Brett Creeley Date: Fri, 8 Mar 2024 10:21:48 -0800 Subject: vfio/pds: Make sure migration file isn't accessed after reset It's possible the migration file is accessed after reset when it has been cleaned up, especially when it's initiated by the device. This is because the driver doesn't rip out the filep when cleaning up it only frees the related page structures and sets its local struct pds_vfio_lm_file pointer to NULL. This can cause a NULL pointer dereference, which is shown in the example below during a restore after a device initiated reset: BUG: kernel NULL pointer dereference, address: 000000000000000c PF: supervisor read access in kernel mode PF: error_code(0x0000) - not-present page PGD 0 P4D 0 Oops: 0000 [#1] PREEMPT SMP NOPTI RIP: 0010:pds_vfio_get_file_page+0x5d/0xf0 [pds_vfio_pci] [...] Call Trace: pds_vfio_restore_write+0xf6/0x160 [pds_vfio_pci] vfs_write+0xc9/0x3f0 ? __fget_light+0xc9/0x110 ksys_write+0xb5/0xf0 __x64_sys_write+0x1a/0x20 do_syscall_64+0x38/0x90 entry_SYSCALL_64_after_hwframe+0x63/0xcd [...] Add a disabled flag to the driver's struct pds_vfio_lm_file that gets set during cleanup. Then make sure to check the flag when the migration file is accessed via its file_operations. By default this flag will be false as the memory for struct pds_vfio_lm_file is kzalloc'd, which means the struct pds_vfio_lm_file is enabled and accessible. Also, since the file_operations and driver's migration file cleanup happen under the protection of the same pds_vfio_lm_file.lock, using this flag is thread safe. Fixes: 8512ed256334 ("vfio/pds: Always clear the save/restore FDs on reset") Reviewed-by: Shannon Nelson Signed-off-by: Brett Creeley Link: https://lore.kernel.org/r/20240308182149.22036-2-brett.creeley@amd.com Signed-off-by: Alex Williamson --- drivers/vfio/pci/pds/lm.c | 13 +++++++++++++ drivers/vfio/pci/pds/lm.h | 1 + 2 files changed, 14 insertions(+) diff --git a/drivers/vfio/pci/pds/lm.c b/drivers/vfio/pci/pds/lm.c index 79fe2e66bb49..6b94cc0bf45b 100644 --- a/drivers/vfio/pci/pds/lm.c +++ b/drivers/vfio/pci/pds/lm.c @@ -92,8 +92,10 @@ static void pds_vfio_put_lm_file(struct pds_vfio_lm_file *lm_file) { mutex_lock(&lm_file->lock); + lm_file->disabled = true; lm_file->size = 0; lm_file->alloc_size = 0; + lm_file->filep->f_pos = 0; /* Free scatter list of file pages */ sg_free_table(&lm_file->sg_table); @@ -183,6 +185,12 @@ static ssize_t pds_vfio_save_read(struct file *filp, char __user *buf, pos = &filp->f_pos; mutex_lock(&lm_file->lock); + + if (lm_file->disabled) { + done = -ENODEV; + goto out_unlock; + } + if (*pos > lm_file->size) { done = -EINVAL; goto out_unlock; @@ -283,6 +291,11 @@ static ssize_t pds_vfio_restore_write(struct file *filp, const char __user *buf, mutex_lock(&lm_file->lock); + if (lm_file->disabled) { + done = -ENODEV; + goto out_unlock; + } + while (len) { size_t page_offset; struct page *page; diff --git a/drivers/vfio/pci/pds/lm.h b/drivers/vfio/pci/pds/lm.h index 13be893198b7..9511b1afc6a1 100644 --- a/drivers/vfio/pci/pds/lm.h +++ b/drivers/vfio/pci/pds/lm.h @@ -27,6 +27,7 @@ struct pds_vfio_lm_file { struct scatterlist *last_offset_sg; /* Iterator */ unsigned int sg_last_entry; unsigned long last_offset; + bool disabled; }; struct pds_vfio_pci_device; -- cgit v1.2.3 From 6a7e448c6b238c9b256b01eb198b0af93ae3157e Mon Sep 17 00:00:00 2001 From: Brett Creeley Date: Fri, 8 Mar 2024 10:21:49 -0800 Subject: vfio/pds: Refactor/simplify reset logic The current logic for handling resets is more complicated than it needs to be. The deferred_reset flag is used to indicate a reset is needed and the deferred_reset_state is the requested, post-reset, state. Also, the deferred_reset logic was added to vfio migration drivers to prevent a circular locking dependency with respect to mm_lock and state mutex. This is mainly because of the copy_to/from_user() functions(which takes mm_lock) invoked under state mutex. Remove all of the deferred reset logic and just pass the requested next state to pds_vfio_reset() so it can be used for VMM and DSC initiated resets. This removes the need for pds_vfio_state_mutex_lock(), so remove that and replace its use with a simple mutex_unlock(). Also, remove the reset_mutex as it's no longer needed since the state_mutex can be the driver's primary protector. Suggested-by: Shameer Kolothum Reviewed-by: Shannon Nelson Signed-off-by: Brett Creeley Link: https://lore.kernel.org/r/20240308182149.22036-3-brett.creeley@amd.com Signed-off-by: Alex Williamson --- drivers/vfio/pci/pds/dirty.c | 6 +++--- drivers/vfio/pci/pds/pci_drv.c | 27 +++++-------------------- drivers/vfio/pci/pds/vfio_dev.c | 45 +++++++++-------------------------------- drivers/vfio/pci/pds/vfio_dev.h | 8 ++------ 4 files changed, 19 insertions(+), 67 deletions(-) diff --git a/drivers/vfio/pci/pds/dirty.c b/drivers/vfio/pci/pds/dirty.c index 8ddf4346fcd5..68e8f006dfdb 100644 --- a/drivers/vfio/pci/pds/dirty.c +++ b/drivers/vfio/pci/pds/dirty.c @@ -607,7 +607,7 @@ int pds_vfio_dma_logging_report(struct vfio_device *vdev, unsigned long iova, mutex_lock(&pds_vfio->state_mutex); err = pds_vfio_dirty_sync(pds_vfio, dirty, iova, length); - pds_vfio_state_mutex_unlock(pds_vfio); + mutex_unlock(&pds_vfio->state_mutex); return err; } @@ -624,7 +624,7 @@ int pds_vfio_dma_logging_start(struct vfio_device *vdev, mutex_lock(&pds_vfio->state_mutex); pds_vfio_send_host_vf_lm_status_cmd(pds_vfio, PDS_LM_STA_IN_PROGRESS); err = pds_vfio_dirty_enable(pds_vfio, ranges, nnodes, page_size); - pds_vfio_state_mutex_unlock(pds_vfio); + mutex_unlock(&pds_vfio->state_mutex); return err; } @@ -637,7 +637,7 @@ int pds_vfio_dma_logging_stop(struct vfio_device *vdev) mutex_lock(&pds_vfio->state_mutex); pds_vfio_dirty_disable(pds_vfio, true); - pds_vfio_state_mutex_unlock(pds_vfio); + mutex_unlock(&pds_vfio->state_mutex); return 0; } diff --git a/drivers/vfio/pci/pds/pci_drv.c b/drivers/vfio/pci/pds/pci_drv.c index a34dda516629..16e93b11ab1b 100644 --- a/drivers/vfio/pci/pds/pci_drv.c +++ b/drivers/vfio/pci/pds/pci_drv.c @@ -21,16 +21,13 @@ static void pds_vfio_recovery(struct pds_vfio_pci_device *pds_vfio) { - bool deferred_reset_needed = false; - /* * Documentation states that the kernel migration driver must not * generate asynchronous device state transitions outside of * manipulation by the user or the VFIO_DEVICE_RESET ioctl. * * Since recovery is an asynchronous event received from the device, - * initiate a deferred reset. Issue a deferred reset in the following - * situations: + * initiate a reset in the following situations: * 1. Migration is in progress, which will cause the next step of * the migration to fail. * 2. If the device is in a state that will be set to @@ -42,24 +39,8 @@ static void pds_vfio_recovery(struct pds_vfio_pci_device *pds_vfio) pds_vfio->state != VFIO_DEVICE_STATE_ERROR) || (pds_vfio->state == VFIO_DEVICE_STATE_RUNNING && pds_vfio_dirty_is_enabled(pds_vfio))) - deferred_reset_needed = true; + pds_vfio_reset(pds_vfio, VFIO_DEVICE_STATE_ERROR); mutex_unlock(&pds_vfio->state_mutex); - - /* - * On the next user initiated state transition, the device will - * transition to the VFIO_DEVICE_STATE_ERROR. At this point it's the user's - * responsibility to reset the device. - * - * If a VFIO_DEVICE_RESET is requested post recovery and before the next - * state transition, then the deferred reset state will be set to - * VFIO_DEVICE_STATE_RUNNING. - */ - if (deferred_reset_needed) { - mutex_lock(&pds_vfio->reset_mutex); - pds_vfio->deferred_reset = true; - pds_vfio->deferred_reset_state = VFIO_DEVICE_STATE_ERROR; - mutex_unlock(&pds_vfio->reset_mutex); - } } static int pds_vfio_pci_notify_handler(struct notifier_block *nb, @@ -185,7 +166,9 @@ static void pds_vfio_pci_aer_reset_done(struct pci_dev *pdev) { struct pds_vfio_pci_device *pds_vfio = pds_vfio_pci_drvdata(pdev); - pds_vfio_reset(pds_vfio); + mutex_lock(&pds_vfio->state_mutex); + pds_vfio_reset(pds_vfio, VFIO_DEVICE_STATE_RUNNING); + mutex_unlock(&pds_vfio->state_mutex); } static const struct pci_error_handlers pds_vfio_pci_err_handlers = { diff --git a/drivers/vfio/pci/pds/vfio_dev.c b/drivers/vfio/pci/pds/vfio_dev.c index a286ebcc7112..76a80ae7087b 100644 --- a/drivers/vfio/pci/pds/vfio_dev.c +++ b/drivers/vfio/pci/pds/vfio_dev.c @@ -26,37 +26,14 @@ struct pds_vfio_pci_device *pds_vfio_pci_drvdata(struct pci_dev *pdev) vfio_coredev); } -void pds_vfio_state_mutex_unlock(struct pds_vfio_pci_device *pds_vfio) +void pds_vfio_reset(struct pds_vfio_pci_device *pds_vfio, + enum vfio_device_mig_state state) { -again: - mutex_lock(&pds_vfio->reset_mutex); - if (pds_vfio->deferred_reset) { - pds_vfio->deferred_reset = false; - pds_vfio_put_restore_file(pds_vfio); - pds_vfio_put_save_file(pds_vfio); - if (pds_vfio->state == VFIO_DEVICE_STATE_ERROR) { - pds_vfio_dirty_disable(pds_vfio, false); - } - pds_vfio->state = pds_vfio->deferred_reset_state; - pds_vfio->deferred_reset_state = VFIO_DEVICE_STATE_RUNNING; - mutex_unlock(&pds_vfio->reset_mutex); - goto again; - } - mutex_unlock(&pds_vfio->state_mutex); - mutex_unlock(&pds_vfio->reset_mutex); -} - -void pds_vfio_reset(struct pds_vfio_pci_device *pds_vfio) -{ - mutex_lock(&pds_vfio->reset_mutex); - pds_vfio->deferred_reset = true; - pds_vfio->deferred_reset_state = VFIO_DEVICE_STATE_RUNNING; - if (!mutex_trylock(&pds_vfio->state_mutex)) { - mutex_unlock(&pds_vfio->reset_mutex); - return; - } - mutex_unlock(&pds_vfio->reset_mutex); - pds_vfio_state_mutex_unlock(pds_vfio); + pds_vfio_put_restore_file(pds_vfio); + pds_vfio_put_save_file(pds_vfio); + if (state == VFIO_DEVICE_STATE_ERROR) + pds_vfio_dirty_disable(pds_vfio, false); + pds_vfio->state = state; } static struct file * @@ -97,8 +74,7 @@ pds_vfio_set_device_state(struct vfio_device *vdev, break; } } - pds_vfio_state_mutex_unlock(pds_vfio); - /* still waiting on a deferred_reset */ + mutex_unlock(&pds_vfio->state_mutex); if (pds_vfio->state == VFIO_DEVICE_STATE_ERROR) res = ERR_PTR(-EIO); @@ -114,7 +90,7 @@ static int pds_vfio_get_device_state(struct vfio_device *vdev, mutex_lock(&pds_vfio->state_mutex); *current_state = pds_vfio->state; - pds_vfio_state_mutex_unlock(pds_vfio); + mutex_unlock(&pds_vfio->state_mutex); return 0; } @@ -156,7 +132,6 @@ static int pds_vfio_init_device(struct vfio_device *vdev) pds_vfio->vf_id = vf_id; mutex_init(&pds_vfio->state_mutex); - mutex_init(&pds_vfio->reset_mutex); vdev->migration_flags = VFIO_MIGRATION_STOP_COPY | VFIO_MIGRATION_P2P; vdev->mig_ops = &pds_vfio_lm_ops; @@ -178,7 +153,6 @@ static void pds_vfio_release_device(struct vfio_device *vdev) vfio_coredev.vdev); mutex_destroy(&pds_vfio->state_mutex); - mutex_destroy(&pds_vfio->reset_mutex); vfio_pci_core_release_dev(vdev); } @@ -194,7 +168,6 @@ static int pds_vfio_open_device(struct vfio_device *vdev) return err; pds_vfio->state = VFIO_DEVICE_STATE_RUNNING; - pds_vfio->deferred_reset_state = VFIO_DEVICE_STATE_RUNNING; vfio_pci_core_finish_enable(&pds_vfio->vfio_coredev); diff --git a/drivers/vfio/pci/pds/vfio_dev.h b/drivers/vfio/pci/pds/vfio_dev.h index e7b01080a1ec..803d99d69c73 100644 --- a/drivers/vfio/pci/pds/vfio_dev.h +++ b/drivers/vfio/pci/pds/vfio_dev.h @@ -18,20 +18,16 @@ struct pds_vfio_pci_device { struct pds_vfio_dirty dirty; struct mutex state_mutex; /* protect migration state */ enum vfio_device_mig_state state; - struct mutex reset_mutex; /* protect reset_done flow */ - u8 deferred_reset; - enum vfio_device_mig_state deferred_reset_state; struct notifier_block nb; int vf_id; u16 client_id; }; -void pds_vfio_state_mutex_unlock(struct pds_vfio_pci_device *pds_vfio); - const struct vfio_device_ops *pds_vfio_ops_info(void); struct pds_vfio_pci_device *pds_vfio_pci_drvdata(struct pci_dev *pdev); -void pds_vfio_reset(struct pds_vfio_pci_device *pds_vfio); +void pds_vfio_reset(struct pds_vfio_pci_device *pds_vfio, + enum vfio_device_mig_state state); struct pci_dev *pds_vfio_to_pci_dev(struct pds_vfio_pci_device *pds_vfio); struct device *pds_vfio_to_dev(struct pds_vfio_pci_device *pds_vfio); -- cgit v1.2.3 From fe9a7082684eb059b925c535682e68c34d487d43 Mon Sep 17 00:00:00 2001 From: Alex Williamson Date: Fri, 8 Mar 2024 16:05:22 -0700 Subject: vfio/pci: Disable auto-enable of exclusive INTx IRQ Currently for devices requiring masking at the irqchip for INTx, ie. devices without DisINTx support, the IRQ is enabled in request_irq() and subsequently disabled as necessary to align with the masked status flag. This presents a window where the interrupt could fire between these events, resulting in the IRQ incrementing the disable depth twice. This would be unrecoverable for a user since the masked flag prevents nested enables through vfio. Instead, invert the logic using IRQF_NO_AUTOEN such that exclusive INTx is never auto-enabled, then unmask as required. Cc: Fixes: 89e1f7d4c66d ("vfio: Add PCI device driver") Reviewed-by: Kevin Tian Reviewed-by: Eric Auger Link: https://lore.kernel.org/r/20240308230557.805580-2-alex.williamson@redhat.com Signed-off-by: Alex Williamson --- drivers/vfio/pci/vfio_pci_intrs.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c index 237beac83809..136101179fcb 100644 --- a/drivers/vfio/pci/vfio_pci_intrs.c +++ b/drivers/vfio/pci/vfio_pci_intrs.c @@ -296,8 +296,15 @@ static int vfio_intx_set_signal(struct vfio_pci_core_device *vdev, int fd) ctx->trigger = trigger; + /* + * Devices without DisINTx support require an exclusive interrupt, + * IRQ masking is performed at the IRQ chip. The masked status is + * protected by vdev->irqlock. Setup the IRQ without auto-enable and + * unmask as necessary below under lock. DisINTx is unmodified by + * the IRQ configuration and may therefore use auto-enable. + */ if (!vdev->pci_2_3) - irqflags = 0; + irqflags = IRQF_NO_AUTOEN; ret = request_irq(pdev->irq, vfio_intx_handler, irqflags, ctx->name, vdev); @@ -308,13 +315,9 @@ static int vfio_intx_set_signal(struct vfio_pci_core_device *vdev, int fd) return ret; } - /* - * INTx disable will stick across the new irq setup, - * disable_irq won't. - */ spin_lock_irqsave(&vdev->irqlock, flags); - if (!vdev->pci_2_3 && ctx->masked) - disable_irq_nosync(pdev->irq); + if (!vdev->pci_2_3 && !ctx->masked) + enable_irq(pdev->irq); spin_unlock_irqrestore(&vdev->irqlock, flags); return 0; -- cgit v1.2.3 From 810cd4bb53456d0503cc4e7934e063835152c1b7 Mon Sep 17 00:00:00 2001 From: Alex Williamson Date: Fri, 8 Mar 2024 16:05:23 -0700 Subject: vfio/pci: Lock external INTx masking ops Mask operations through config space changes to DisINTx may race INTx configuration changes via ioctl. Create wrappers that add locking for paths outside of the core interrupt code. In particular, irq_type is updated holding igate, therefore testing is_intx() requires holding igate. For example clearing DisINTx from config space can otherwise race changes of the interrupt configuration. This aligns interfaces which may trigger the INTx eventfd into two camps, one side serialized by igate and the other only enabled while INTx is configured. A subsequent patch introduces synchronization for the latter flows. Cc: Fixes: 89e1f7d4c66d ("vfio: Add PCI device driver") Reported-by: Reinette Chatre Reviewed-by: Kevin Tian Reviewed-by: Reinette Chatre Reviewed-by: Eric Auger Link: https://lore.kernel.org/r/20240308230557.805580-3-alex.williamson@redhat.com Signed-off-by: Alex Williamson --- drivers/vfio/pci/vfio_pci_intrs.c | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c index 136101179fcb..75c85eec21b3 100644 --- a/drivers/vfio/pci/vfio_pci_intrs.c +++ b/drivers/vfio/pci/vfio_pci_intrs.c @@ -99,13 +99,15 @@ static void vfio_send_intx_eventfd(void *opaque, void *unused) } /* Returns true if the INTx vfio_pci_irq_ctx.masked value is changed. */ -bool vfio_pci_intx_mask(struct vfio_pci_core_device *vdev) +static bool __vfio_pci_intx_mask(struct vfio_pci_core_device *vdev) { struct pci_dev *pdev = vdev->pdev; struct vfio_pci_irq_ctx *ctx; unsigned long flags; bool masked_changed = false; + lockdep_assert_held(&vdev->igate); + spin_lock_irqsave(&vdev->irqlock, flags); /* @@ -143,6 +145,17 @@ out_unlock: return masked_changed; } +bool vfio_pci_intx_mask(struct vfio_pci_core_device *vdev) +{ + bool mask_changed; + + mutex_lock(&vdev->igate); + mask_changed = __vfio_pci_intx_mask(vdev); + mutex_unlock(&vdev->igate); + + return mask_changed; +} + /* * If this is triggered by an eventfd, we can't call eventfd_signal * or else we'll deadlock on the eventfd wait queue. Return >0 when @@ -194,12 +207,21 @@ out_unlock: return ret; } -void vfio_pci_intx_unmask(struct vfio_pci_core_device *vdev) +static void __vfio_pci_intx_unmask(struct vfio_pci_core_device *vdev) { + lockdep_assert_held(&vdev->igate); + if (vfio_pci_intx_unmask_handler(vdev, NULL) > 0) vfio_send_intx_eventfd(vdev, NULL); } +void vfio_pci_intx_unmask(struct vfio_pci_core_device *vdev) +{ + mutex_lock(&vdev->igate); + __vfio_pci_intx_unmask(vdev); + mutex_unlock(&vdev->igate); +} + static irqreturn_t vfio_intx_handler(int irq, void *dev_id) { struct vfio_pci_core_device *vdev = dev_id; @@ -563,11 +585,11 @@ static int vfio_pci_set_intx_unmask(struct vfio_pci_core_device *vdev, return -EINVAL; if (flags & VFIO_IRQ_SET_DATA_NONE) { - vfio_pci_intx_unmask(vdev); + __vfio_pci_intx_unmask(vdev); } else if (flags & VFIO_IRQ_SET_DATA_BOOL) { uint8_t unmask = *(uint8_t *)data; if (unmask) - vfio_pci_intx_unmask(vdev); + __vfio_pci_intx_unmask(vdev); } else if (flags & VFIO_IRQ_SET_DATA_EVENTFD) { struct vfio_pci_irq_ctx *ctx = vfio_irq_ctx_get(vdev, 0); int32_t fd = *(int32_t *)data; @@ -594,11 +616,11 @@ static int vfio_pci_set_intx_mask(struct vfio_pci_core_device *vdev, return -EINVAL; if (flags & VFIO_IRQ_SET_DATA_NONE) { - vfio_pci_intx_mask(vdev); + __vfio_pci_intx_mask(vdev); } else if (flags & VFIO_IRQ_SET_DATA_BOOL) { uint8_t mask = *(uint8_t *)data; if (mask) - vfio_pci_intx_mask(vdev); + __vfio_pci_intx_mask(vdev); } else if (flags & VFIO_IRQ_SET_DATA_EVENTFD) { return -ENOTTY; /* XXX implement me */ } -- cgit v1.2.3 From b620ecbd17a03cacd06f014a5d3f3a11285ce053 Mon Sep 17 00:00:00 2001 From: Alex Williamson Date: Fri, 8 Mar 2024 16:05:24 -0700 Subject: vfio: Introduce interface to flush virqfd inject workqueue In order to synchronize changes that can affect the thread callback, introduce an interface to force a flush of the inject workqueue. The irqfd pointer is only valid under spinlock, but the workqueue cannot be flushed under spinlock. Therefore the flush work for the irqfd is queued under spinlock. The vfio_irqfd_cleanup_wq workqueue is re-used for queuing this work such that flushing the workqueue is also ordered relative to shutdown. Reviewed-by: Kevin Tian Reviewed-by: Reinette Chatre Reviewed-by: Eric Auger Link: https://lore.kernel.org/r/20240308230557.805580-4-alex.williamson@redhat.com Signed-off-by: Alex Williamson --- drivers/vfio/virqfd.c | 21 +++++++++++++++++++++ include/linux/vfio.h | 2 ++ 2 files changed, 23 insertions(+) diff --git a/drivers/vfio/virqfd.c b/drivers/vfio/virqfd.c index 29c564b7a6e1..532269133801 100644 --- a/drivers/vfio/virqfd.c +++ b/drivers/vfio/virqfd.c @@ -101,6 +101,13 @@ static void virqfd_inject(struct work_struct *work) virqfd->thread(virqfd->opaque, virqfd->data); } +static void virqfd_flush_inject(struct work_struct *work) +{ + struct virqfd *virqfd = container_of(work, struct virqfd, flush_inject); + + flush_work(&virqfd->inject); +} + int vfio_virqfd_enable(void *opaque, int (*handler)(void *, void *), void (*thread)(void *, void *), @@ -124,6 +131,7 @@ int vfio_virqfd_enable(void *opaque, INIT_WORK(&virqfd->shutdown, virqfd_shutdown); INIT_WORK(&virqfd->inject, virqfd_inject); + INIT_WORK(&virqfd->flush_inject, virqfd_flush_inject); irqfd = fdget(fd); if (!irqfd.file) { @@ -213,3 +221,16 @@ void vfio_virqfd_disable(struct virqfd **pvirqfd) flush_workqueue(vfio_irqfd_cleanup_wq); } EXPORT_SYMBOL_GPL(vfio_virqfd_disable); + +void vfio_virqfd_flush_thread(struct virqfd **pvirqfd) +{ + unsigned long flags; + + spin_lock_irqsave(&virqfd_lock, flags); + if (*pvirqfd && (*pvirqfd)->thread) + queue_work(vfio_irqfd_cleanup_wq, &(*pvirqfd)->flush_inject); + spin_unlock_irqrestore(&virqfd_lock, flags); + + flush_workqueue(vfio_irqfd_cleanup_wq); +} +EXPORT_SYMBOL_GPL(vfio_virqfd_flush_thread); diff --git a/include/linux/vfio.h b/include/linux/vfio.h index 89b265bc6ec3..8b1a29820409 100644 --- a/include/linux/vfio.h +++ b/include/linux/vfio.h @@ -356,6 +356,7 @@ struct virqfd { wait_queue_entry_t wait; poll_table pt; struct work_struct shutdown; + struct work_struct flush_inject; struct virqfd **pvirqfd; }; @@ -363,5 +364,6 @@ int vfio_virqfd_enable(void *opaque, int (*handler)(void *, void *), void (*thread)(void *, void *), void *data, struct virqfd **pvirqfd, int fd); void vfio_virqfd_disable(struct virqfd **pvirqfd); +void vfio_virqfd_flush_thread(struct virqfd **pvirqfd); #endif /* VFIO_H */ -- cgit v1.2.3 From 18c198c96a815c962adc2b9b77909eec0be7df4d Mon Sep 17 00:00:00 2001 From: Alex Williamson Date: Fri, 8 Mar 2024 16:05:25 -0700 Subject: vfio/pci: Create persistent INTx handler A vulnerability exists where the eventfd for INTx signaling can be deconfigured, which unregisters the IRQ handler but still allows eventfds to be signaled with a NULL context through the SET_IRQS ioctl or through unmask irqfd if the device interrupt is pending. Ideally this could be solved with some additional locking; the igate mutex serializes the ioctl and config space accesses, and the interrupt handler is unregistered relative to the trigger, but the irqfd path runs asynchronous to those. The igate mutex cannot be acquired from the atomic context of the eventfd wake function. Disabling the irqfd relative to the eventfd registration is potentially incompatible with existing userspace. As a result, the solution implemented here moves configuration of the INTx interrupt handler to track the lifetime of the INTx context object and irq_type configuration, rather than registration of a particular trigger eventfd. Synchronization is added between the ioctl path and eventfd_signal() wrapper such that the eventfd trigger can be dynamically updated relative to in-flight interrupts or irqfd callbacks. Cc: Fixes: 89e1f7d4c66d ("vfio: Add PCI device driver") Reported-by: Reinette Chatre Reviewed-by: Kevin Tian Reviewed-by: Reinette Chatre Reviewed-by: Eric Auger Link: https://lore.kernel.org/r/20240308230557.805580-5-alex.williamson@redhat.com Signed-off-by: Alex Williamson --- drivers/vfio/pci/vfio_pci_intrs.c | 145 ++++++++++++++++++++------------------ 1 file changed, 78 insertions(+), 67 deletions(-) diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c index 75c85eec21b3..fb5392b749ff 100644 --- a/drivers/vfio/pci/vfio_pci_intrs.c +++ b/drivers/vfio/pci/vfio_pci_intrs.c @@ -90,11 +90,15 @@ static void vfio_send_intx_eventfd(void *opaque, void *unused) if (likely(is_intx(vdev) && !vdev->virq_disabled)) { struct vfio_pci_irq_ctx *ctx; + struct eventfd_ctx *trigger; ctx = vfio_irq_ctx_get(vdev, 0); if (WARN_ON_ONCE(!ctx)) return; - eventfd_signal(ctx->trigger); + + trigger = READ_ONCE(ctx->trigger); + if (likely(trigger)) + eventfd_signal(trigger); } } @@ -253,100 +257,100 @@ static irqreturn_t vfio_intx_handler(int irq, void *dev_id) return ret; } -static int vfio_intx_enable(struct vfio_pci_core_device *vdev) +static int vfio_intx_enable(struct vfio_pci_core_device *vdev, + struct eventfd_ctx *trigger) { + struct pci_dev *pdev = vdev->pdev; struct vfio_pci_irq_ctx *ctx; + unsigned long irqflags; + char *name; + int ret; if (!is_irq_none(vdev)) return -EINVAL; - if (!vdev->pdev->irq) + if (!pdev->irq) return -ENODEV; + name = kasprintf(GFP_KERNEL_ACCOUNT, "vfio-intx(%s)", pci_name(pdev)); + if (!name) + return -ENOMEM; + ctx = vfio_irq_ctx_alloc(vdev, 0); if (!ctx) return -ENOMEM; + ctx->name = name; + ctx->trigger = trigger; + /* - * If the virtual interrupt is masked, restore it. Devices - * supporting DisINTx can be masked at the hardware level - * here, non-PCI-2.3 devices will have to wait until the - * interrupt is enabled. + * Fill the initial masked state based on virq_disabled. After + * enable, changing the DisINTx bit in vconfig directly changes INTx + * masking. igate prevents races during setup, once running masked + * is protected via irqlock. + * + * Devices supporting DisINTx also reflect the current mask state in + * the physical DisINTx bit, which is not affected during IRQ setup. + * + * Devices without DisINTx support require an exclusive interrupt. + * IRQ masking is performed at the IRQ chip. Again, igate protects + * against races during setup and IRQ handlers and irqfds are not + * yet active, therefore masked is stable and can be used to + * conditionally auto-enable the IRQ. + * + * irq_type must be stable while the IRQ handler is registered, + * therefore it must be set before request_irq(). */ ctx->masked = vdev->virq_disabled; - if (vdev->pci_2_3) - pci_intx(vdev->pdev, !ctx->masked); + if (vdev->pci_2_3) { + pci_intx(pdev, !ctx->masked); + irqflags = IRQF_SHARED; + } else { + irqflags = ctx->masked ? IRQF_NO_AUTOEN : 0; + } vdev->irq_type = VFIO_PCI_INTX_IRQ_INDEX; + ret = request_irq(pdev->irq, vfio_intx_handler, + irqflags, ctx->name, vdev); + if (ret) { + vdev->irq_type = VFIO_PCI_NUM_IRQS; + kfree(name); + vfio_irq_ctx_free(vdev, ctx, 0); + return ret; + } + return 0; } -static int vfio_intx_set_signal(struct vfio_pci_core_device *vdev, int fd) +static int vfio_intx_set_signal(struct vfio_pci_core_device *vdev, + struct eventfd_ctx *trigger) { struct pci_dev *pdev = vdev->pdev; - unsigned long irqflags = IRQF_SHARED; struct vfio_pci_irq_ctx *ctx; - struct eventfd_ctx *trigger; - unsigned long flags; - int ret; + struct eventfd_ctx *old; ctx = vfio_irq_ctx_get(vdev, 0); if (WARN_ON_ONCE(!ctx)) return -EINVAL; - if (ctx->trigger) { - free_irq(pdev->irq, vdev); - kfree(ctx->name); - eventfd_ctx_put(ctx->trigger); - ctx->trigger = NULL; - } - - if (fd < 0) /* Disable only */ - return 0; - - ctx->name = kasprintf(GFP_KERNEL_ACCOUNT, "vfio-intx(%s)", - pci_name(pdev)); - if (!ctx->name) - return -ENOMEM; - - trigger = eventfd_ctx_fdget(fd); - if (IS_ERR(trigger)) { - kfree(ctx->name); - return PTR_ERR(trigger); - } + old = ctx->trigger; - ctx->trigger = trigger; + WRITE_ONCE(ctx->trigger, trigger); - /* - * Devices without DisINTx support require an exclusive interrupt, - * IRQ masking is performed at the IRQ chip. The masked status is - * protected by vdev->irqlock. Setup the IRQ without auto-enable and - * unmask as necessary below under lock. DisINTx is unmodified by - * the IRQ configuration and may therefore use auto-enable. - */ - if (!vdev->pci_2_3) - irqflags = IRQF_NO_AUTOEN; - - ret = request_irq(pdev->irq, vfio_intx_handler, - irqflags, ctx->name, vdev); - if (ret) { - ctx->trigger = NULL; - kfree(ctx->name); - eventfd_ctx_put(trigger); - return ret; + /* Releasing an old ctx requires synchronizing in-flight users */ + if (old) { + synchronize_irq(pdev->irq); + vfio_virqfd_flush_thread(&ctx->unmask); + eventfd_ctx_put(old); } - spin_lock_irqsave(&vdev->irqlock, flags); - if (!vdev->pci_2_3 && !ctx->masked) - enable_irq(pdev->irq); - spin_unlock_irqrestore(&vdev->irqlock, flags); - return 0; } static void vfio_intx_disable(struct vfio_pci_core_device *vdev) { + struct pci_dev *pdev = vdev->pdev; struct vfio_pci_irq_ctx *ctx; ctx = vfio_irq_ctx_get(vdev, 0); @@ -354,10 +358,13 @@ static void vfio_intx_disable(struct vfio_pci_core_device *vdev) if (ctx) { vfio_virqfd_disable(&ctx->unmask); vfio_virqfd_disable(&ctx->mask); + free_irq(pdev->irq, vdev); + if (ctx->trigger) + eventfd_ctx_put(ctx->trigger); + kfree(ctx->name); + vfio_irq_ctx_free(vdev, ctx, 0); } - vfio_intx_set_signal(vdev, -1); vdev->irq_type = VFIO_PCI_NUM_IRQS; - vfio_irq_ctx_free(vdev, ctx, 0); } /* @@ -641,19 +648,23 @@ static int vfio_pci_set_intx_trigger(struct vfio_pci_core_device *vdev, return -EINVAL; if (flags & VFIO_IRQ_SET_DATA_EVENTFD) { + struct eventfd_ctx *trigger = NULL; int32_t fd = *(int32_t *)data; int ret; - if (is_intx(vdev)) - return vfio_intx_set_signal(vdev, fd); + if (fd >= 0) { + trigger = eventfd_ctx_fdget(fd); + if (IS_ERR(trigger)) + return PTR_ERR(trigger); + } - ret = vfio_intx_enable(vdev); - if (ret) - return ret; + if (is_intx(vdev)) + ret = vfio_intx_set_signal(vdev, trigger); + else + ret = vfio_intx_enable(vdev, trigger); - ret = vfio_intx_set_signal(vdev, fd); - if (ret) - vfio_intx_disable(vdev); + if (ret && trigger) + eventfd_ctx_put(trigger); return ret; } -- cgit v1.2.3 From fcdc0d3d40bc26c105acf8467f7d9018970944ae Mon Sep 17 00:00:00 2001 From: Alex Williamson Date: Fri, 8 Mar 2024 16:05:26 -0700 Subject: vfio/platform: Disable virqfds on cleanup irqfds for mask and unmask that are not specifically disabled by the user are leaked. Remove any irqfds during cleanup Cc: Eric Auger Cc: Fixes: a7fa7c77cf15 ("vfio/platform: implement IRQ masking/unmasking via an eventfd") Reviewed-by: Kevin Tian Reviewed-by: Eric Auger Link: https://lore.kernel.org/r/20240308230557.805580-6-alex.williamson@redhat.com Signed-off-by: Alex Williamson --- drivers/vfio/platform/vfio_platform_irq.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c index 61a1bfb68ac7..e5dcada9e86c 100644 --- a/drivers/vfio/platform/vfio_platform_irq.c +++ b/drivers/vfio/platform/vfio_platform_irq.c @@ -321,8 +321,11 @@ void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev) { int i; - for (i = 0; i < vdev->num_irqs; i++) + for (i = 0; i < vdev->num_irqs; i++) { + vfio_virqfd_disable(&vdev->irqs[i].mask); + vfio_virqfd_disable(&vdev->irqs[i].unmask); vfio_set_trigger(vdev, i, -1, NULL); + } vdev->num_irqs = 0; kfree(vdev->irqs); -- cgit v1.2.3 From 675daf435e9f8e5a5eab140a9864dfad6668b375 Mon Sep 17 00:00:00 2001 From: Alex Williamson Date: Fri, 8 Mar 2024 16:05:27 -0700 Subject: vfio/platform: Create persistent IRQ handlers The vfio-platform SET_IRQS ioctl currently allows loopback triggering of an interrupt before a signaling eventfd has been configured by the user, which thereby allows a NULL pointer dereference. Rather than register the IRQ relative to a valid trigger, register all IRQs in a disabled state in the device open path. This allows mask operations on the IRQ to nest within the overall enable state governed by a valid eventfd signal. This decouples @masked, protected by the @locked spinlock from @trigger, protected via the @igate mutex. In doing so, it's guaranteed that changes to @trigger cannot race the IRQ handlers because the IRQ handler is synchronously disabled before modifying the trigger, and loopback triggering of the IRQ via ioctl is safe due to serialization with trigger changes via igate. For compatibility, request_irq() failures are maintained to be local to the SET_IRQS ioctl rather than a fatal error in the open device path. This allows, for example, a userspace driver with polling mode support to continue to work regardless of moving the request_irq() call site. This necessarily blocks all SET_IRQS access to the failed index. Cc: Eric Auger Cc: Fixes: 57f972e2b341 ("vfio/platform: trigger an interrupt via eventfd") Reviewed-by: Kevin Tian Reviewed-by: Eric Auger Link: https://lore.kernel.org/r/20240308230557.805580-7-alex.williamson@redhat.com Signed-off-by: Alex Williamson --- drivers/vfio/platform/vfio_platform_irq.c | 100 ++++++++++++++++++++---------- 1 file changed, 68 insertions(+), 32 deletions(-) diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c index e5dcada9e86c..ef41ecef83af 100644 --- a/drivers/vfio/platform/vfio_platform_irq.c +++ b/drivers/vfio/platform/vfio_platform_irq.c @@ -136,6 +136,16 @@ static int vfio_platform_set_irq_unmask(struct vfio_platform_device *vdev, return 0; } +/* + * The trigger eventfd is guaranteed valid in the interrupt path + * and protected by the igate mutex when triggered via ioctl. + */ +static void vfio_send_eventfd(struct vfio_platform_irq *irq_ctx) +{ + if (likely(irq_ctx->trigger)) + eventfd_signal(irq_ctx->trigger); +} + static irqreturn_t vfio_automasked_irq_handler(int irq, void *dev_id) { struct vfio_platform_irq *irq_ctx = dev_id; @@ -155,7 +165,7 @@ static irqreturn_t vfio_automasked_irq_handler(int irq, void *dev_id) spin_unlock_irqrestore(&irq_ctx->lock, flags); if (ret == IRQ_HANDLED) - eventfd_signal(irq_ctx->trigger); + vfio_send_eventfd(irq_ctx); return ret; } @@ -164,52 +174,40 @@ static irqreturn_t vfio_irq_handler(int irq, void *dev_id) { struct vfio_platform_irq *irq_ctx = dev_id; - eventfd_signal(irq_ctx->trigger); + vfio_send_eventfd(irq_ctx); return IRQ_HANDLED; } static int vfio_set_trigger(struct vfio_platform_device *vdev, int index, - int fd, irq_handler_t handler) + int fd) { struct vfio_platform_irq *irq = &vdev->irqs[index]; struct eventfd_ctx *trigger; - int ret; if (irq->trigger) { - irq_clear_status_flags(irq->hwirq, IRQ_NOAUTOEN); - free_irq(irq->hwirq, irq); - kfree(irq->name); + disable_irq(irq->hwirq); eventfd_ctx_put(irq->trigger); irq->trigger = NULL; } if (fd < 0) /* Disable only */ return 0; - irq->name = kasprintf(GFP_KERNEL_ACCOUNT, "vfio-irq[%d](%s)", - irq->hwirq, vdev->name); - if (!irq->name) - return -ENOMEM; trigger = eventfd_ctx_fdget(fd); - if (IS_ERR(trigger)) { - kfree(irq->name); + if (IS_ERR(trigger)) return PTR_ERR(trigger); - } irq->trigger = trigger; - irq_set_status_flags(irq->hwirq, IRQ_NOAUTOEN); - ret = request_irq(irq->hwirq, handler, 0, irq->name, irq); - if (ret) { - kfree(irq->name); - eventfd_ctx_put(trigger); - irq->trigger = NULL; - return ret; - } - - if (!irq->masked) - enable_irq(irq->hwirq); + /* + * irq->masked effectively provides nested disables within the overall + * enable relative to trigger. Specifically request_irq() is called + * with NO_AUTOEN, therefore the IRQ is initially disabled. The user + * may only further disable the IRQ with a MASK operations because + * irq->masked is initially false. + */ + enable_irq(irq->hwirq); return 0; } @@ -228,7 +226,7 @@ static int vfio_platform_set_irq_trigger(struct vfio_platform_device *vdev, handler = vfio_irq_handler; if (!count && (flags & VFIO_IRQ_SET_DATA_NONE)) - return vfio_set_trigger(vdev, index, -1, handler); + return vfio_set_trigger(vdev, index, -1); if (start != 0 || count != 1) return -EINVAL; @@ -236,7 +234,7 @@ static int vfio_platform_set_irq_trigger(struct vfio_platform_device *vdev, if (flags & VFIO_IRQ_SET_DATA_EVENTFD) { int32_t fd = *(int32_t *)data; - return vfio_set_trigger(vdev, index, fd, handler); + return vfio_set_trigger(vdev, index, fd); } if (flags & VFIO_IRQ_SET_DATA_NONE) { @@ -260,6 +258,14 @@ int vfio_platform_set_irqs_ioctl(struct vfio_platform_device *vdev, unsigned start, unsigned count, uint32_t flags, void *data) = NULL; + /* + * For compatibility, errors from request_irq() are local to the + * SET_IRQS path and reflected in the name pointer. This allows, + * for example, polling mode fallback for an exclusive IRQ failure. + */ + if (IS_ERR(vdev->irqs[index].name)) + return PTR_ERR(vdev->irqs[index].name); + switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) { case VFIO_IRQ_SET_ACTION_MASK: func = vfio_platform_set_irq_mask; @@ -280,7 +286,7 @@ int vfio_platform_set_irqs_ioctl(struct vfio_platform_device *vdev, int vfio_platform_irq_init(struct vfio_platform_device *vdev) { - int cnt = 0, i; + int cnt = 0, i, ret = 0; while (vdev->get_irq(vdev, cnt) >= 0) cnt++; @@ -292,29 +298,54 @@ int vfio_platform_irq_init(struct vfio_platform_device *vdev) for (i = 0; i < cnt; i++) { int hwirq = vdev->get_irq(vdev, i); + irq_handler_t handler = vfio_irq_handler; - if (hwirq < 0) + if (hwirq < 0) { + ret = -EINVAL; goto err; + } spin_lock_init(&vdev->irqs[i].lock); vdev->irqs[i].flags = VFIO_IRQ_INFO_EVENTFD; - if (irq_get_trigger_type(hwirq) & IRQ_TYPE_LEVEL_MASK) + if (irq_get_trigger_type(hwirq) & IRQ_TYPE_LEVEL_MASK) { vdev->irqs[i].flags |= VFIO_IRQ_INFO_MASKABLE | VFIO_IRQ_INFO_AUTOMASKED; + handler = vfio_automasked_irq_handler; + } vdev->irqs[i].count = 1; vdev->irqs[i].hwirq = hwirq; vdev->irqs[i].masked = false; + vdev->irqs[i].name = kasprintf(GFP_KERNEL_ACCOUNT, + "vfio-irq[%d](%s)", hwirq, + vdev->name); + if (!vdev->irqs[i].name) { + ret = -ENOMEM; + goto err; + } + + ret = request_irq(hwirq, handler, IRQF_NO_AUTOEN, + vdev->irqs[i].name, &vdev->irqs[i]); + if (ret) { + kfree(vdev->irqs[i].name); + vdev->irqs[i].name = ERR_PTR(ret); + } } vdev->num_irqs = cnt; return 0; err: + for (--i; i >= 0; i--) { + if (!IS_ERR(vdev->irqs[i].name)) { + free_irq(vdev->irqs[i].hwirq, &vdev->irqs[i]); + kfree(vdev->irqs[i].name); + } + } kfree(vdev->irqs); - return -EINVAL; + return ret; } void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev) @@ -324,7 +355,12 @@ void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev) for (i = 0; i < vdev->num_irqs; i++) { vfio_virqfd_disable(&vdev->irqs[i].mask); vfio_virqfd_disable(&vdev->irqs[i].unmask); - vfio_set_trigger(vdev, i, -1, NULL); + if (!IS_ERR(vdev->irqs[i].name)) { + free_irq(vdev->irqs[i].hwirq, &vdev->irqs[i]); + if (vdev->irqs[i].trigger) + eventfd_ctx_put(vdev->irqs[i].trigger); + kfree(vdev->irqs[i].name); + } } vdev->num_irqs = 0; -- cgit v1.2.3 From 7447d911af699a15f8d050dfcb7c680a86f87012 Mon Sep 17 00:00:00 2001 From: Alex Williamson Date: Fri, 8 Mar 2024 16:05:28 -0700 Subject: vfio/fsl-mc: Block calling interrupt handler without trigger The eventfd_ctx trigger pointer of the vfio_fsl_mc_irq object is initially NULL and may become NULL if the user sets the trigger eventfd to -1. The interrupt handler itself is guaranteed that trigger is always valid between request_irq() and free_irq(), but the loopback testing mechanisms to invoke the handler function need to test the trigger. The triggering and setting ioctl paths both make use of igate and are therefore mutually exclusive. The vfio-fsl-mc driver does not make use of irqfds, nor does it support any sort of masking operations, therefore unlike vfio-pci and vfio-platform, the flow can remain essentially unchanged. Cc: Diana Craciun Cc: Fixes: cc0ee20bd969 ("vfio/fsl-mc: trigger an interrupt via eventfd") Reviewed-by: Kevin Tian Reviewed-by: Eric Auger Link: https://lore.kernel.org/r/20240308230557.805580-8-alex.williamson@redhat.com Signed-off-by: Alex Williamson --- drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c b/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c index d62fbfff20b8..82b2afa9b7e3 100644 --- a/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c @@ -141,13 +141,14 @@ static int vfio_fsl_mc_set_irq_trigger(struct vfio_fsl_mc_device *vdev, irq = &vdev->mc_irqs[index]; if (flags & VFIO_IRQ_SET_DATA_NONE) { - vfio_fsl_mc_irq_handler(hwirq, irq); + if (irq->trigger) + eventfd_signal(irq->trigger); } else if (flags & VFIO_IRQ_SET_DATA_BOOL) { u8 trigger = *(u8 *)data; - if (trigger) - vfio_fsl_mc_irq_handler(hwirq, irq); + if (trigger && irq->trigger) + eventfd_signal(irq->trigger); } return 0; -- cgit v1.2.3