Re: [PATCH] vfio/pci: Propagate ACPI notifications to the user-space

[Alex Williamson <alex.williamson@redhat.com>]

On Tue,  7 Mar 2023 22:05:53 +0000
Grzegorz Jaszczyk <jaz@semihalf.com> wrote:

> From: Dominik Behr <dbehr@chromium.org>
> 
> Hitherto there was no support for propagating ACPI notifications to the
> guest drivers. In order to provide such support, install a handler for
> notifications on an ACPI device during vfio-pci device registration. The
> handler role is to propagate such ACPI notifications to the user-space
> via acpi netlink events, which allows VMM to receive and propagate them
> further to the VMs.
> 
> Thanks to the above, the actual driver for the pass-through device,
> which belongs to the guest, can receive and react to device specific
> notifications.

What consumes these events?  Has this been proposed to any VM
management tools like libvirt?  What sort of ACPI events are we
expecting to see here and what does userspace do with them?

> Signed-off-by: Dominik Behr <dbehr@chromium.org>
> Co-developed-by: Grzegorz Jaszczyk <jaz@semihalf.com>
> Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com>
> ---
>  drivers/vfio/pci/vfio_pci_core.c | 33 ++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index a5ab416cf476..92b8ed8d087c 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -10,6 +10,7 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> +#include <linux/acpi.h>
>  #include <linux/aperture.h>
>  #include <linux/device.h>
>  #include <linux/eventfd.h>
> @@ -2120,10 +2121,20 @@ void vfio_pci_core_release_dev(struct vfio_device *core_vdev)
>  }
>  EXPORT_SYMBOL_GPL(vfio_pci_core_release_dev);
>  
> +static void vfio_pci_core_acpi_notify(acpi_handle handle, u32 event, void *data)
> +{
> +	struct vfio_pci_core_device *vdev = (struct vfio_pci_core_device *)data;
> +	struct device *dev = &vdev->pdev->dev;
> +
> +	acpi_bus_generate_netlink_event("vfio_pci", dev_name(dev), event, 0);

Who listens to this?  Should there be an in-band means to provide
notifies related to the device?  How does a userspace driver know to
look for netlink events for a particular device?

> +}
> +
>  int vfio_pci_core_register_device(struct vfio_pci_core_device *vdev)
>  {
> +	acpi_status status;
>  	struct pci_dev *pdev = vdev->pdev;
>  	struct device *dev = &pdev->dev;
> +	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
>  	int ret;
>  
>  	/* Drivers must set the vfio_pci_core_device to their drvdata */
> @@ -2201,8 +2212,24 @@ int vfio_pci_core_register_device(struct vfio_pci_core_device *vdev)
>  	ret = vfio_register_group_dev(&vdev->vdev);
>  	if (ret)
>  		goto out_power;
> +
> +	if (!adev) {
> +		pci_info(pdev, "No ACPI companion");

This would be a log message generated for 99.99% of devices.

> +		return 0;
> +	}
> +
> +	status = acpi_install_notify_handler(adev->handle, ACPI_DEVICE_NOTIFY,
> +					vfio_pci_core_acpi_notify, (void *)vdev);

vfio-pci supports non-ACPI platforms, I don't see any !CONFIG_ACPI
prototypes for this function.  Thanks,

Alex

> +
> +	if (ACPI_FAILURE(status)) {
> +		pci_err(pdev, "Failed to install notify handler");
> +		goto out_group_register;
> +	}
> +
>  	return 0;
>  
> +out_group_register:
> +	vfio_unregister_group_dev(&vdev->vdev);
>  out_power:
>  	if (!disable_idle_d3)
>  		pm_runtime_get_noresume(dev);
> @@ -2216,6 +2243,12 @@ EXPORT_SYMBOL_GPL(vfio_pci_core_register_device);
>  
>  void vfio_pci_core_unregister_device(struct vfio_pci_core_device *vdev)
>  {
> +	struct acpi_device *adev = ACPI_COMPANION(&vdev->pdev->dev);
> +
> +	if (adev)
> +		acpi_remove_notify_handler(adev->handle, ACPI_DEVICE_NOTIFY,
> +					   vfio_pci_core_acpi_notify);
> +
>  	vfio_pci_core_sriov_configure(vdev, 0);
>  
>  	vfio_unregister_group_dev(&vdev->vdev);