From: Eric Auger <eric.auger@linaro.org> To: Alex Williamson <alex.williamson@redhat.com> Cc: eric.auger@st.com, christoffer.dall@linaro.org, marc.zyngier@arm.com, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, patches@linaro.org, pbonzini@redhat.com, kim.phillips@freescale.com, b.reynal@virtualopensystems.com, feng.wu@intel.com Subject: Re: [RFC v5 06/13] VFIO: platform: add vfio_external_{mask|is_active|set_automasked} Date: Wed, 01 Apr 2015 14:20:38 +0200 [thread overview] Message-ID: <551BE296.2080509@linaro.org> (raw) In-Reply-To: <1427822405.5567.163.camel@redhat.com> On 03/31/2015 07:20 PM, Alex Williamson wrote: > On Thu, 2015-03-19 at 15:55 +0100, Eric Auger wrote: >> Introduces 3 new external functions aimed at doining some actions >> on VFIO platform devices: >> - mask a VFIO IRQ >> - get the active status of a VFIO IRQ (active at interrupt >> controller level or masked by the level-sensitive automasking). >> - change the automasked property and the VFIO handler >> >> Note there is no way to discriminate between user-space >> masking and automasked handler masking. As a consequence, is_active >> will return true in case the IRQ was masked by the user-space. >> >> Signed-off-by: Eric Auger <eric.auger@linaro.org> >> >> --- >> >> V4: creation >> --- >> drivers/vfio/platform/vfio_platform_irq.c | 43 +++++++++++++++++++++++++++++++ >> include/linux/vfio.h | 14 ++++++++++ >> 2 files changed, 57 insertions(+) >> >> diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c >> index 8eb65c1..49994cb 100644 >> --- a/drivers/vfio/platform/vfio_platform_irq.c >> +++ b/drivers/vfio/platform/vfio_platform_irq.c >> @@ -231,6 +231,49 @@ static int vfio_set_trigger(struct vfio_platform_device *vdev, int index, >> return 0; >> } >> >> +void vfio_external_mask(struct vfio_platform_device *vdev, int index) >> +{ >> + vfio_platform_mask(&vdev->irqs[index]); >> +} >> +EXPORT_SYMBOL_GPL(vfio_external_mask); >> + >> +bool vfio_external_is_active(struct vfio_platform_device *vdev, int index) >> +{ >> + unsigned long flags; >> + struct vfio_platform_irq *irq = &vdev->irqs[index]; >> + bool active, masked, outstanding; >> + int ret; >> + >> + spin_lock_irqsave(&irq->lock, flags); >> + >> + ret = irq_get_irqchip_state(irq->hwirq, IRQCHIP_STATE_ACTIVE, &active); >> + BUG_ON(ret); >> + masked = irq->masked; >> + outstanding = active || masked; >> + >> + spin_unlock_irqrestore(&irq->lock, flags); >> + return outstanding; >> +} >> +EXPORT_SYMBOL_GPL(vfio_external_is_active); >> + >> +void vfio_external_set_automasked(struct vfio_platform_device *vdev, >> + int index, bool automasked) >> +{ >> + unsigned long flags; >> + struct vfio_platform_irq *irq = &vdev->irqs[index]; >> + >> + spin_lock_irqsave(&irq->lock, flags); >> + if (automasked) { >> + irq->flags |= VFIO_IRQ_INFO_AUTOMASKED; >> + irq->handler = vfio_automasked_irq_handler; >> + } else { >> + irq->flags &= ~VFIO_IRQ_INFO_AUTOMASKED; >> + irq->handler = vfio_irq_handler; >> + } >> + spin_unlock_irqrestore(&irq->lock, flags); >> +} >> +EXPORT_SYMBOL_GPL(vfio_external_set_automasked); >> + > > > This is where the abstraction breaks down. These are > vfio_external_foo() interfaces, yet they assume a specific type of > device, a vfio platform device. Either the name should reflect that or > they should be hosted in vfio-core with a callout to the device specific > implementations. Can we make kvm-vfio deal only in struct vfio_device > and struct device? Hi Alex, I will follow your guidelines and intend to extend vfio_platform_ops to store those new callbacks. Indeed kvm-vfio should be able to be vfio_platform_device independent. Thank you for the review. Best Regards Eric > >> static int vfio_platform_set_irq_trigger(struct vfio_platform_device *vdev, >> unsigned index, unsigned start, >> unsigned count, uint32_t flags, >> diff --git a/include/linux/vfio.h b/include/linux/vfio.h >> index b18c38f..7aa6330 100644 >> --- a/include/linux/vfio.h >> +++ b/include/linux/vfio.h >> @@ -105,6 +105,20 @@ extern struct vfio_device *vfio_device_get_external_user(struct file *filep); >> extern void vfio_device_put_external_user(struct vfio_device *vdev); >> extern struct device *vfio_external_base_device(struct vfio_device *vdev); >> >> +struct vfio_platform_device; >> +extern void vfio_external_mask(struct vfio_platform_device *vdev, int index); >> +/* >> + * returns whether the VFIO IRQ is active: >> + * true if not yet deactivated at interrupt controller level or if >> + * automasked (level sensitive IRQ). Unfortunately there is no way to >> + * discriminate between handler auto-masking and user-space masking >> + */ >> +extern bool vfio_external_is_active(struct vfio_platform_device *vdev, >> + int index); >> + >> +extern void vfio_external_set_automasked(struct vfio_platform_device *vdev, >> + int index, bool automasked); >> + >> struct pci_dev; >> #ifdef CONFIG_EEH >> extern void vfio_spapr_pci_eeh_open(struct pci_dev *pdev); > > >
WARNING: multiple messages have this Message-ID (diff)
From: eric.auger@linaro.org (Eric Auger) To: linux-arm-kernel@lists.infradead.org Subject: [RFC v5 06/13] VFIO: platform: add vfio_external_{mask|is_active|set_automasked} Date: Wed, 01 Apr 2015 14:20:38 +0200 [thread overview] Message-ID: <551BE296.2080509@linaro.org> (raw) In-Reply-To: <1427822405.5567.163.camel@redhat.com> On 03/31/2015 07:20 PM, Alex Williamson wrote: > On Thu, 2015-03-19 at 15:55 +0100, Eric Auger wrote: >> Introduces 3 new external functions aimed at doining some actions >> on VFIO platform devices: >> - mask a VFIO IRQ >> - get the active status of a VFIO IRQ (active at interrupt >> controller level or masked by the level-sensitive automasking). >> - change the automasked property and the VFIO handler >> >> Note there is no way to discriminate between user-space >> masking and automasked handler masking. As a consequence, is_active >> will return true in case the IRQ was masked by the user-space. >> >> Signed-off-by: Eric Auger <eric.auger@linaro.org> >> >> --- >> >> V4: creation >> --- >> drivers/vfio/platform/vfio_platform_irq.c | 43 +++++++++++++++++++++++++++++++ >> include/linux/vfio.h | 14 ++++++++++ >> 2 files changed, 57 insertions(+) >> >> diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c >> index 8eb65c1..49994cb 100644 >> --- a/drivers/vfio/platform/vfio_platform_irq.c >> +++ b/drivers/vfio/platform/vfio_platform_irq.c >> @@ -231,6 +231,49 @@ static int vfio_set_trigger(struct vfio_platform_device *vdev, int index, >> return 0; >> } >> >> +void vfio_external_mask(struct vfio_platform_device *vdev, int index) >> +{ >> + vfio_platform_mask(&vdev->irqs[index]); >> +} >> +EXPORT_SYMBOL_GPL(vfio_external_mask); >> + >> +bool vfio_external_is_active(struct vfio_platform_device *vdev, int index) >> +{ >> + unsigned long flags; >> + struct vfio_platform_irq *irq = &vdev->irqs[index]; >> + bool active, masked, outstanding; >> + int ret; >> + >> + spin_lock_irqsave(&irq->lock, flags); >> + >> + ret = irq_get_irqchip_state(irq->hwirq, IRQCHIP_STATE_ACTIVE, &active); >> + BUG_ON(ret); >> + masked = irq->masked; >> + outstanding = active || masked; >> + >> + spin_unlock_irqrestore(&irq->lock, flags); >> + return outstanding; >> +} >> +EXPORT_SYMBOL_GPL(vfio_external_is_active); >> + >> +void vfio_external_set_automasked(struct vfio_platform_device *vdev, >> + int index, bool automasked) >> +{ >> + unsigned long flags; >> + struct vfio_platform_irq *irq = &vdev->irqs[index]; >> + >> + spin_lock_irqsave(&irq->lock, flags); >> + if (automasked) { >> + irq->flags |= VFIO_IRQ_INFO_AUTOMASKED; >> + irq->handler = vfio_automasked_irq_handler; >> + } else { >> + irq->flags &= ~VFIO_IRQ_INFO_AUTOMASKED; >> + irq->handler = vfio_irq_handler; >> + } >> + spin_unlock_irqrestore(&irq->lock, flags); >> +} >> +EXPORT_SYMBOL_GPL(vfio_external_set_automasked); >> + > > > This is where the abstraction breaks down. These are > vfio_external_foo() interfaces, yet they assume a specific type of > device, a vfio platform device. Either the name should reflect that or > they should be hosted in vfio-core with a callout to the device specific > implementations. Can we make kvm-vfio deal only in struct vfio_device > and struct device? Hi Alex, I will follow your guidelines and intend to extend vfio_platform_ops to store those new callbacks. Indeed kvm-vfio should be able to be vfio_platform_device independent. Thank you for the review. Best Regards Eric > >> static int vfio_platform_set_irq_trigger(struct vfio_platform_device *vdev, >> unsigned index, unsigned start, >> unsigned count, uint32_t flags, >> diff --git a/include/linux/vfio.h b/include/linux/vfio.h >> index b18c38f..7aa6330 100644 >> --- a/include/linux/vfio.h >> +++ b/include/linux/vfio.h >> @@ -105,6 +105,20 @@ extern struct vfio_device *vfio_device_get_external_user(struct file *filep); >> extern void vfio_device_put_external_user(struct vfio_device *vdev); >> extern struct device *vfio_external_base_device(struct vfio_device *vdev); >> >> +struct vfio_platform_device; >> +extern void vfio_external_mask(struct vfio_platform_device *vdev, int index); >> +/* >> + * returns whether the VFIO IRQ is active: >> + * true if not yet deactivated at interrupt controller level or if >> + * automasked (level sensitive IRQ). Unfortunately there is no way to >> + * discriminate between handler auto-masking and user-space masking >> + */ >> +extern bool vfio_external_is_active(struct vfio_platform_device *vdev, >> + int index); >> + >> +extern void vfio_external_set_automasked(struct vfio_platform_device *vdev, >> + int index, bool automasked); >> + >> struct pci_dev; >> #ifdef CONFIG_EEH >> extern void vfio_spapr_pci_eeh_open(struct pci_dev *pdev); > > >
next prev parent reply other threads:[~2015-04-01 12:23 UTC|newest] Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-03-19 14:55 [RFC v5 00/13] KVM-VFIO IRQ forward control Eric Auger 2015-03-19 14:55 ` Eric Auger 2015-03-19 14:55 ` [RFC v5 01/13] KVM: arm/arm64: Enable the KVM-VFIO device Eric Auger 2015-03-19 14:55 ` Eric Auger 2015-03-19 14:55 ` [RFC v5 02/13] VFIO: platform: test forwarded state when selecting IRQ handler Eric Auger 2015-03-19 14:55 ` Eric Auger 2015-03-19 14:55 ` [RFC v5 03/13] VFIO: platform: single handler using function pointer Eric Auger 2015-03-19 14:55 ` Eric Auger 2015-03-19 14:55 ` [RFC v5 04/13] KVM: kvm-vfio: User API for IRQ forwarding Eric Auger 2015-03-19 14:55 ` Eric Auger 2015-03-19 14:55 ` [RFC v5 05/13] VFIO: external user API for interaction with vfio devices Eric Auger 2015-03-19 14:55 ` Eric Auger 2015-03-19 14:55 ` [RFC v5 06/13] VFIO: platform: add vfio_external_{mask|is_active|set_automasked} Eric Auger 2015-03-19 14:55 ` Eric Auger 2015-03-31 17:20 ` Alex Williamson 2015-03-31 17:20 ` Alex Williamson 2015-04-01 12:20 ` Eric Auger [this message] 2015-04-01 12:20 ` Eric Auger 2015-03-19 14:55 ` [RFC v5 07/13] KVM: kvm-vfio: wrappers to VFIO external API device helpers Eric Auger 2015-03-19 14:55 ` Eric Auger 2015-03-19 14:55 ` [RFC v5 08/13] KVM: kvm-vfio: wrappers for vfio_external_{mask|is_active|set_automasked} Eric Auger 2015-03-19 14:55 ` Eric Auger 2015-03-31 17:20 ` Alex Williamson 2015-03-31 17:20 ` Alex Williamson 2015-03-19 14:55 ` [RFC v5 09/13] KVM: arm: rename pause into power_off Eric Auger 2015-03-19 14:55 ` Eric Auger 2015-03-19 14:55 ` [RFC v5 10/13] kvm: introduce kvm_arch_halt_guest and kvm_arch_resume_guest Eric Auger 2015-03-19 14:55 ` Eric Auger 2015-03-19 14:55 ` [RFC v5 11/13] kvm: arm/arm64: implement " Eric Auger 2015-03-19 14:55 ` Eric Auger 2015-03-19 14:55 ` [RFC v5 12/13] KVM: kvm-vfio: generic forwarding control Eric Auger 2015-03-19 14:55 ` Eric Auger 2015-03-31 17:20 ` Alex Williamson 2015-03-31 17:20 ` Alex Williamson 2015-03-19 14:55 ` [RFC v5 13/13] KVM: arm/arm64: vgic: " Eric Auger 2015-03-19 14:55 ` Eric Auger
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=551BE296.2080509@linaro.org \ --to=eric.auger@linaro.org \ --cc=alex.williamson@redhat.com \ --cc=b.reynal@virtualopensystems.com \ --cc=christoffer.dall@linaro.org \ --cc=eric.auger@st.com \ --cc=feng.wu@intel.com \ --cc=kim.phillips@freescale.com \ --cc=kvm@vger.kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=marc.zyngier@arm.com \ --cc=patches@linaro.org \ --cc=pbonzini@redhat.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.