lkml.org 
[lkml]   [2022]   [Jul]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v4 6/6] vfio/pci: Add support for virtual PME
From
On 7/6/2022 9:10 PM, Alex Williamson wrote:
> On Fri, 1 Jul 2022 16:38:14 +0530
> Abhishek Sahu <abhsahu@nvidia.com> wrote:
>
>> If the PCI device is in low power state and the device requires
>> wake-up, then it can generate PME (Power Management Events). Mostly
>> these PME events will be propagated to the root port and then the
>> root port will generate the system interrupt. Then the OS should
>> identify the device which generated the PME and should resume
>> the device.
>>
>> We can implement a similar virtual PME framework where if the device
>> already went into the runtime suspended state and then there is any
>> wake-up on the host side, then it will send the virtual PME
>> notification to the guest. This virtual PME will be helpful for the cases
>> where the device will not be suspended again if there is any wake-up
>> triggered by the host. Following is the overall approach regarding
>> the virtual PME.
>>
>> 1. Add one more event like VFIO_PCI_ERR_IRQ_INDEX named
>> VFIO_PCI_PME_IRQ_INDEX and do the required code changes to get/set
>> this new IRQ.
>>
>> 2. From the guest side, the guest needs to enable eventfd for the
>> virtual PME notification.
>>
>> 3. In the vfio-pci driver, the PME support bits are currently
>> virtualized and set to 0. We can set PME capability support for all
>> the power states. This PME capability support is independent of the
>> physical PME support.
>>
>> 4. The PME enable (PME_En bit in Power Management Control/Status
>> Register) and PME status (PME_Status bit in Power Management
>> Control/Status Register) are also virtualized currently.
>> The write support for PME_En bit can be enabled.
>>
>> 5. The PME_Status bit is a write-1-clear bit where the write with
>> zero value will have no effect and write with 1 value will clear the
>> bit. The write for this bit will be trapped inside
>> vfio_pm_config_write() similar to PCI_PM_CTRL write for PM_STATES.
>>
>> 6. When the host gets a request for resuming the device other than from
>> low power exit feature IOCTL, then PME_Status bit will be set.
>> According to [PCIe v5 7.5.2.2],
>> "PME_Status - This bit is Set when the Function would normally
>> generate a PME signal. The value of this bit is not affected by
>> the value of the PME_En bit."
>>
>> So even if PME_En bit is not set, we can set PME_Status bit.
>>
>> 7. If the guest has enabled PME_En and registered for PME events
>> through eventfd, then the usage count will be incremented to prevent
>> the device to go into the suspended state and notify the guest through
>> eventfd trigger.
>>
>> The virtual PME can help in handling physical PME also. When
>> physical PME comes, then also the runtime resume will be called. If
>> the guest has registered for virtual PME, then it will be sent in this
>> case also.
>>
>> * Implementation for handling the virtual PME on the hypervisor:
>>
>> If we take the implementation in Linux OS, then during runtime suspend
>> time, then it calls __pci_enable_wake(). It internally enables PME
>> through pci_pme_active() and also enables the ACPI side wake-up
>> through platform_pci_set_wakeup(). To handle the PME, the hypervisor has
>> the following two options:
>>
>> 1. Create a virtual root port for the VFIO device and trigger
>> interrupt when the PME comes. It will call pcie_pme_irq() which will
>> resume the device.
>>
>> 2. Create a virtual ACPI _PRW resource and associate it with the device
>> itself. In _PRW, any GPE (General Purpose Event) can be assigned for
>> the wake-up. When PME comes, then GPE can be triggered by the
>> hypervisor. GPE interrupt will call pci_acpi_wake_dev() function
>> internally and it will resume the device.
>
> Do we really need to implement PME emulation in the kernel or is it
> sufficient for userspace to simply register a one-shot eventfd when
> SET'ing the low power feature and QEMU can provide the PME emulation
> based on that signaling? Thanks,
>
> Alex
>

It seems it can be implemented in QEMU instead of kernel.
I will drop this patch.

Thanks,
Abhishek

>>
>> Signed-off-by: Abhishek Sahu <abhsahu@nvidia.com>
>> ---
>> drivers/vfio/pci/vfio_pci_config.c | 39 +++++++++++++++++++++------
>> drivers/vfio/pci/vfio_pci_core.c | 43 ++++++++++++++++++++++++------
>> drivers/vfio/pci/vfio_pci_intrs.c | 18 +++++++++++++
>> include/linux/vfio_pci_core.h | 2 ++
>> include/uapi/linux/vfio.h | 1 +
>> 5 files changed, 87 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
>> index 21a4743d011f..a06375a03758 100644
>> --- a/drivers/vfio/pci/vfio_pci_config.c
>> +++ b/drivers/vfio/pci/vfio_pci_config.c
>> @@ -719,6 +719,20 @@ static int vfio_pm_config_write(struct vfio_pci_core_device *vdev, int pos,
>> if (count < 0)
>> return count;
>>
>> + /*
>> + * PME_STATUS is write-1-clear bit. If PME_STATUS is 1, then clear the
>> + * bit in vconfig. The PME_STATUS is in the upper byte of the control
>> + * register and user can do single byte write also.
>> + */
>> + if (offset <= PCI_PM_CTRL + 1 && offset + count > PCI_PM_CTRL + 1) {
>> + if (le32_to_cpu(val) &
>> + (PCI_PM_CTRL_PME_STATUS >> (offset - PCI_PM_CTRL) * 8)) {
>> + __le16 *ctrl = (__le16 *)&vdev->vconfig
>> + [vdev->pm_cap_offset + PCI_PM_CTRL];
>> + *ctrl &= ~cpu_to_le16(PCI_PM_CTRL_PME_STATUS);
>> + }
>> + }
>> +
>> if (offset == PCI_PM_CTRL) {
>> pci_power_t state;
>>
>> @@ -771,14 +785,16 @@ static int __init init_pci_cap_pm_perm(struct perm_bits *perm)
>> * the user change power state, but we trap and initiate the
>> * change ourselves, so the state bits are read-only.
>> *
>> - * The guest can't process PME from D3cold so virtualize PME_Status
>> - * and PME_En bits. The vconfig bits will be cleared during device
>> - * capability initialization.
>> + * The guest can't process physical PME from D3cold so virtualize
>> + * PME_Status and PME_En bits. These bits will be used for the
>> + * virtual PME between host and guest. The vconfig bits will be
>> + * updated during device capability initialization. PME_Status is
>> + * write-1-clear bit, so it is read-only. We trap and update the
>> + * vconfig bit manually during write.
>> */
>> p_setd(perm, PCI_PM_CTRL,
>> PCI_PM_CTRL_PME_ENABLE | PCI_PM_CTRL_PME_STATUS,
>> - ~(PCI_PM_CTRL_PME_ENABLE | PCI_PM_CTRL_PME_STATUS |
>> - PCI_PM_CTRL_STATE_MASK));
>> + ~(PCI_PM_CTRL_STATE_MASK | PCI_PM_CTRL_PME_STATUS));
>>
>> return 0;
>> }
>> @@ -1454,8 +1470,13 @@ static void vfio_update_pm_vconfig_bytes(struct vfio_pci_core_device *vdev,
>> __le16 *pmc = (__le16 *)&vdev->vconfig[offset + PCI_PM_PMC];
>> __le16 *ctrl = (__le16 *)&vdev->vconfig[offset + PCI_PM_CTRL];
>>
>> - /* Clear vconfig PME_Support, PME_Status, and PME_En bits */
>> - *pmc &= ~cpu_to_le16(PCI_PM_CAP_PME_MASK);
>> + /*
>> + * Set the vconfig PME_Support bits. The PME_Status is being used for
>> + * virtual PME support and is not dependent upon the physical
>> + * PME support.
>> + */
>> + *pmc |= cpu_to_le16(PCI_PM_CAP_PME_MASK);
>> + /* Clear vconfig PME_Support and PME_En bits */
>> *ctrl &= ~cpu_to_le16(PCI_PM_CTRL_PME_ENABLE | PCI_PM_CTRL_PME_STATUS);
>> }
>>
>> @@ -1582,8 +1603,10 @@ static int vfio_cap_init(struct vfio_pci_core_device *vdev)
>> if (ret)
>> return ret;
>>
>> - if (cap == PCI_CAP_ID_PM)
>> + if (cap == PCI_CAP_ID_PM) {
>> + vdev->pm_cap_offset = pos;
>> vfio_update_pm_vconfig_bytes(vdev, pos);
>> + }
>>
>> prev = &vdev->vconfig[pos + PCI_CAP_LIST_NEXT];
>> pos = next;
>> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
>> index 1ddaaa6ccef5..6c1225bc2aeb 100644
>> --- a/drivers/vfio/pci/vfio_pci_core.c
>> +++ b/drivers/vfio/pci/vfio_pci_core.c
>> @@ -319,14 +319,35 @@ static int vfio_pci_core_runtime_resume(struct device *dev)
>> * the low power state or closed the device.
>> * - If there is device access on the host side.
>> *
>> - * For the second case, check if re-entry to the low power state is
>> - * allowed. If not, then increment the usage count so that runtime PM
>> - * framework won't suspend the device and set the 'pm_runtime_resumed'
>> - * flag.
>> + * For the second case:
>> + * - The virtual PME_STATUS bit will be set. If PME_ENABLE bit is set
>> + * and user has registered for virtual PME events, then send the PME
>> + * virtual PME event.
>> + * - Check if re-entry to the low power state is not allowed.
>> + *
>> + * For the above conditions, increment the usage count so that
>> + * runtime PM framework won't suspend the device and set the
>> + * 'pm_runtime_resumed' flag.
>> */
>> - if (vdev->pm_runtime_engaged && !vdev->pm_runtime_reentry_allowed) {
>> - pm_runtime_get_noresume(dev);
>> - vdev->pm_runtime_resumed = true;
>> + if (vdev->pm_runtime_engaged) {
>> + bool pme_triggered = false;
>> + __le16 *ctrl = (__le16 *)&vdev->vconfig
>> + [vdev->pm_cap_offset + PCI_PM_CTRL];
>> +
>> + *ctrl |= cpu_to_le16(PCI_PM_CTRL_PME_STATUS);
>> + if (le16_to_cpu(*ctrl) & PCI_PM_CTRL_PME_ENABLE) {
>> + mutex_lock(&vdev->igate);
>> + if (vdev->pme_trigger) {
>> + pme_triggered = true;
>> + eventfd_signal(vdev->pme_trigger, 1);
>> + }
>> + mutex_unlock(&vdev->igate);
>> + }
>> +
>> + if (!vdev->pm_runtime_reentry_allowed || pme_triggered) {
>> + pm_runtime_get_noresume(dev);
>> + vdev->pm_runtime_resumed = true;
>> + }
>> }
>> up_write(&vdev->memory_lock);
>>
>> @@ -586,6 +607,10 @@ void vfio_pci_core_close_device(struct vfio_device *core_vdev)
>> eventfd_ctx_put(vdev->req_trigger);
>> vdev->req_trigger = NULL;
>> }
>> + if (vdev->pme_trigger) {
>> + eventfd_ctx_put(vdev->pme_trigger);
>> + vdev->pme_trigger = NULL;
>> + }
>> mutex_unlock(&vdev->igate);
>> }
>> EXPORT_SYMBOL_GPL(vfio_pci_core_close_device);
>> @@ -639,7 +664,8 @@ static int vfio_pci_get_irq_count(struct vfio_pci_core_device *vdev, int irq_typ
>> } else if (irq_type == VFIO_PCI_ERR_IRQ_INDEX) {
>> if (pci_is_pcie(vdev->pdev))
>> return 1;
>> - } else if (irq_type == VFIO_PCI_REQ_IRQ_INDEX) {
>> + } else if (irq_type == VFIO_PCI_REQ_IRQ_INDEX ||
>> + irq_type == VFIO_PCI_PME_IRQ_INDEX) {
>> return 1;
>> }
>>
>> @@ -985,6 +1011,7 @@ long vfio_pci_core_ioctl(struct vfio_device *core_vdev, unsigned int cmd,
>> switch (info.index) {
>> case VFIO_PCI_INTX_IRQ_INDEX ... VFIO_PCI_MSIX_IRQ_INDEX:
>> case VFIO_PCI_REQ_IRQ_INDEX:
>> + case VFIO_PCI_PME_IRQ_INDEX:
>> break;
>> case VFIO_PCI_ERR_IRQ_INDEX:
>> if (pci_is_pcie(vdev->pdev))
>> diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
>> index 1a37db99df48..db4180687a74 100644
>> --- a/drivers/vfio/pci/vfio_pci_intrs.c
>> +++ b/drivers/vfio/pci/vfio_pci_intrs.c
>> @@ -639,6 +639,17 @@ static int vfio_pci_set_req_trigger(struct vfio_pci_core_device *vdev,
>> count, flags, data);
>> }
>>
>> +static int vfio_pci_set_pme_trigger(struct vfio_pci_core_device *vdev,
>> + unsigned index, unsigned start,
>> + unsigned count, uint32_t flags, void *data)
>> +{
>> + if (index != VFIO_PCI_PME_IRQ_INDEX || start != 0 || count > 1)
>> + return -EINVAL;
>> +
>> + return vfio_pci_set_ctx_trigger_single(&vdev->pme_trigger,
>> + count, flags, data);
>> +}
>> +
>> int vfio_pci_set_irqs_ioctl(struct vfio_pci_core_device *vdev, uint32_t flags,
>> unsigned index, unsigned start, unsigned count,
>> void *data)
>> @@ -688,6 +699,13 @@ int vfio_pci_set_irqs_ioctl(struct vfio_pci_core_device *vdev, uint32_t flags,
>> break;
>> }
>> break;
>> + case VFIO_PCI_PME_IRQ_INDEX:
>> + switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) {
>> + case VFIO_IRQ_SET_ACTION_TRIGGER:
>> + func = vfio_pci_set_pme_trigger;
>> + break;
>> + }
>> + break;
>> }
>>
>> if (!func)
>> diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
>> index 18cc83b767b8..ee2646d820c2 100644
>> --- a/include/linux/vfio_pci_core.h
>> +++ b/include/linux/vfio_pci_core.h
>> @@ -102,6 +102,7 @@ struct vfio_pci_core_device {
>> bool bar_mmap_supported[PCI_STD_NUM_BARS];
>> u8 *pci_config_map;
>> u8 *vconfig;
>> + u8 pm_cap_offset;
>> struct perm_bits *msi_perm;
>> spinlock_t irqlock;
>> struct mutex igate;
>> @@ -133,6 +134,7 @@ struct vfio_pci_core_device {
>> int ioeventfds_nr;
>> struct eventfd_ctx *err_trigger;
>> struct eventfd_ctx *req_trigger;
>> + struct eventfd_ctx *pme_trigger;
>> struct list_head dummy_resources_list;
>> struct mutex ioeventfds_lock;
>> struct list_head ioeventfds_list;
>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>> index 7e00de5c21ea..08170950d655 100644
>> --- a/include/uapi/linux/vfio.h
>> +++ b/include/uapi/linux/vfio.h
>> @@ -621,6 +621,7 @@ enum {
>> VFIO_PCI_MSIX_IRQ_INDEX,
>> VFIO_PCI_ERR_IRQ_INDEX,
>> VFIO_PCI_REQ_IRQ_INDEX,
>> + VFIO_PCI_PME_IRQ_INDEX,
>> VFIO_PCI_NUM_IRQS
>> };
>>
>

\
 
 \ /
  Last update: 2022-07-08 11:47    [W:0.543 / U:0.068 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site