Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1650711 > unrolled thread
| Started by | Marc Zyngier <marc.zyngier@arm.com> |
|---|---|
| First post | 2017-05-25 20:10 +0200 |
| Last post | 2017-05-31 19:50 +0200 |
| Articles | 3 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH 01/10] vfio: platform: Add automasked field to vfio_platform_irq Marc Zyngier <marc.zyngier@arm.com> - 2017-05-25 20:10 +0200
Re: [PATCH 01/10] vfio: platform: Add automasked field to vfio_platform_irq Auger Eric <eric.auger@redhat.com> - 2017-05-30 14:50 +0200
Re: [PATCH 01/10] vfio: platform: Add automasked field to vfio_platform_irq Alex Williamson <alex.williamson@redhat.com> - 2017-05-31 19:50 +0200
| From | Marc Zyngier <marc.zyngier@arm.com> |
|---|---|
| Date | 2017-05-25 20:10 +0200 |
| Subject | Re: [PATCH 01/10] vfio: platform: Add automasked field to vfio_platform_irq |
| Message-ID | <tL5mV-36x-3@gated-at.bofh.it> |
Hi Eric,
On Wed, May 24 2017 at 10:13:14 pm BST, Eric Auger <eric.auger@redhat.com> wrote:
> For direct EOI modality we will need to differentiate a userspace
> masking from the IRQ handler auto-masking.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
> drivers/vfio/platform/vfio_platform_irq.c | 10 ++++++----
> drivers/vfio/platform/vfio_platform_private.h | 1 +
> 2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c
> index 46d4750..831f0b0 100644
> --- a/drivers/vfio/platform/vfio_platform_irq.c
> +++ b/drivers/vfio/platform/vfio_platform_irq.c
> @@ -29,7 +29,7 @@ static void vfio_platform_mask(struct vfio_platform_irq *irq_ctx)
>
> spin_lock_irqsave(&irq_ctx->lock, flags);
>
> - if (!irq_ctx->masked) {
> + if (!irq_ctx->masked && !irq_ctx->automasked) {
Could you please expand a bit on what this automasked variable covers?
It'd be good to document how masked and automasked differ in behaviour.
Also, it may be worth having a helper (is_masked?) to abstract both
cases.
> disable_irq_nosync(irq_ctx->hwirq);
> irq_ctx->masked = true;
> }
> @@ -89,9 +89,10 @@ static void vfio_platform_unmask(struct vfio_platform_irq *irq_ctx)
>
> spin_lock_irqsave(&irq_ctx->lock, flags);
>
> - if (irq_ctx->masked) {
> + if (irq_ctx->masked || irq_ctx->automasked) {
> enable_irq(irq_ctx->hwirq);
> irq_ctx->masked = false;
> + irq_ctx->automasked = false;
> }
>
> spin_unlock_irqrestore(&irq_ctx->lock, flags);
> @@ -152,12 +153,12 @@ static irqreturn_t vfio_automasked_irq_handler(int irq, void *dev_id)
>
> spin_lock_irqsave(&irq_ctx->lock, flags);
>
> - if (!irq_ctx->masked) {
> + if (!irq_ctx->masked && !irq_ctx->automasked) {
> ret = IRQ_HANDLED;
>
> /* automask maskable interrupts */
> disable_irq_nosync(irq_ctx->hwirq);
> - irq_ctx->masked = true;
> + irq_ctx->automasked = true;
> }
>
> spin_unlock_irqrestore(&irq_ctx->lock, flags);
> @@ -315,6 +316,7 @@ int vfio_platform_irq_init(struct vfio_platform_device *vdev)
> vdev->irqs[i].count = 1;
> vdev->irqs[i].hwirq = hwirq;
> vdev->irqs[i].masked = false;
> + vdev->irqs[i].automasked = false;
> }
>
> vdev->num_irqs = cnt;
> diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
> index 85ffe5d..8a3cfa9 100644
> --- a/drivers/vfio/platform/vfio_platform_private.h
> +++ b/drivers/vfio/platform/vfio_platform_private.h
> @@ -34,6 +34,7 @@ struct vfio_platform_irq {
> char *name;
> struct eventfd_ctx *trigger;
> bool masked;
> + bool automasked;
> spinlock_t lock;
> struct virqfd *unmask;
> struct virqfd *mask;
Thanks,
M.
--
Jazz is not dead, it just smell funny.
[toc] | [next] | [standalone]
| From | Auger Eric <eric.auger@redhat.com> |
|---|---|
| Date | 2017-05-30 14:50 +0200 |
| Subject | Re: [PATCH 01/10] vfio: platform: Add automasked field to vfio_platform_irq |
| Message-ID | <tMOL1-7oW-37@gated-at.bofh.it> |
| In reply to | #1650711 |
Hi Marc,
On 25/05/2017 20:05, Marc Zyngier wrote:
> Hi Eric,
>
> On Wed, May 24 2017 at 10:13:14 pm BST, Eric Auger <eric.auger@redhat.com> wrote:
>> For direct EOI modality we will need to differentiate a userspace
>> masking from the IRQ handler auto-masking.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> ---
>> drivers/vfio/platform/vfio_platform_irq.c | 10 ++++++----
>> drivers/vfio/platform/vfio_platform_private.h | 1 +
>> 2 files changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c
>> index 46d4750..831f0b0 100644
>> --- a/drivers/vfio/platform/vfio_platform_irq.c
>> +++ b/drivers/vfio/platform/vfio_platform_irq.c
>> @@ -29,7 +29,7 @@ static void vfio_platform_mask(struct vfio_platform_irq *irq_ctx)
>>
>> spin_lock_irqsave(&irq_ctx->lock, flags);
>>
>> - if (!irq_ctx->masked) {
>> + if (!irq_ctx->masked && !irq_ctx->automasked) {
>
> Could you please expand a bit on what this automasked variable covers?
> It'd be good to document how masked and automasked differ in behaviour.
Yes sure. So automasked is set by the physical IRQ handler only, for
level sensitive IRQ (AUTOMASKED interrupts). masked is set through the
userspace API (VFIO_DEVICE_SET_IRQS and ACTION_MASK) when masking the
IRQ. VFIO ACTION_UNMASK resets both.
>
> Also, it may be worth having a helper (is_masked?) to abstract both
> cases.
Sure
Eric
>
>> disable_irq_nosync(irq_ctx->hwirq);
>> irq_ctx->masked = true;
>> }
>> @@ -89,9 +89,10 @@ static void vfio_platform_unmask(struct vfio_platform_irq *irq_ctx)
>>
>> spin_lock_irqsave(&irq_ctx->lock, flags);
>>
>> - if (irq_ctx->masked) {
>> + if (irq_ctx->masked || irq_ctx->automasked) {
>> enable_irq(irq_ctx->hwirq);
>> irq_ctx->masked = false;
>> + irq_ctx->automasked = false;
>> }
>>
>> spin_unlock_irqrestore(&irq_ctx->lock, flags);
>> @@ -152,12 +153,12 @@ static irqreturn_t vfio_automasked_irq_handler(int irq, void *dev_id)
>>
>> spin_lock_irqsave(&irq_ctx->lock, flags);
>>
>> - if (!irq_ctx->masked) {
>> + if (!irq_ctx->masked && !irq_ctx->automasked) {
>> ret = IRQ_HANDLED;
>>
>> /* automask maskable interrupts */
>> disable_irq_nosync(irq_ctx->hwirq);
>> - irq_ctx->masked = true;
>> + irq_ctx->automasked = true;
>> }
>>
>> spin_unlock_irqrestore(&irq_ctx->lock, flags);
>> @@ -315,6 +316,7 @@ int vfio_platform_irq_init(struct vfio_platform_device *vdev)
>> vdev->irqs[i].count = 1;
>> vdev->irqs[i].hwirq = hwirq;
>> vdev->irqs[i].masked = false;
>> + vdev->irqs[i].automasked = false;
>> }
>>
>> vdev->num_irqs = cnt;
>> diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
>> index 85ffe5d..8a3cfa9 100644
>> --- a/drivers/vfio/platform/vfio_platform_private.h
>> +++ b/drivers/vfio/platform/vfio_platform_private.h
>> @@ -34,6 +34,7 @@ struct vfio_platform_irq {
>> char *name;
>> struct eventfd_ctx *trigger;
>> bool masked;
>> + bool automasked;
>> spinlock_t lock;
>> struct virqfd *unmask;
>> struct virqfd *mask;
>
> Thanks,
>
> M.
>
[toc] | [prev] | [next] | [standalone]
| From | Alex Williamson <alex.williamson@redhat.com> |
|---|---|
| Date | 2017-05-31 19:50 +0200 |
| Subject | Re: [PATCH 01/10] vfio: platform: Add automasked field to vfio_platform_irq |
| Message-ID | <tNfUR-7UB-1@gated-at.bofh.it> |
| In reply to | #1653204 |
On Tue, 30 May 2017 14:45:54 +0200
Auger Eric <eric.auger@redhat.com> wrote:
> Hi Marc,
>
> On 25/05/2017 20:05, Marc Zyngier wrote:
> > Hi Eric,
> >
> > On Wed, May 24 2017 at 10:13:14 pm BST, Eric Auger <eric.auger@redhat.com> wrote:
> >> For direct EOI modality we will need to differentiate a userspace
> >> masking from the IRQ handler auto-masking.
> >>
> >> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> >> ---
> >> drivers/vfio/platform/vfio_platform_irq.c | 10 ++++++----
> >> drivers/vfio/platform/vfio_platform_private.h | 1 +
> >> 2 files changed, 7 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c
> >> index 46d4750..831f0b0 100644
> >> --- a/drivers/vfio/platform/vfio_platform_irq.c
> >> +++ b/drivers/vfio/platform/vfio_platform_irq.c
> >> @@ -29,7 +29,7 @@ static void vfio_platform_mask(struct vfio_platform_irq *irq_ctx)
> >>
> >> spin_lock_irqsave(&irq_ctx->lock, flags);
> >>
> >> - if (!irq_ctx->masked) {
> >> + if (!irq_ctx->masked && !irq_ctx->automasked) {
> >
> > Could you please expand a bit on what this automasked variable covers?
> > It'd be good to document how masked and automasked differ in behaviour.
>
> Yes sure. So automasked is set by the physical IRQ handler only, for
> level sensitive IRQ (AUTOMASKED interrupts). masked is set through the
> userspace API (VFIO_DEVICE_SET_IRQS and ACTION_MASK) when masking the
> IRQ. VFIO ACTION_UNMASK resets both.
This would make more sense if you at the same time renamed 'masked' to
'usermasked'. Thanks,
Alex
> >
> > Also, it may be worth having a helper (is_masked?) to abstract both
> > cases.
>
> Sure
>
> Eric
> >
> >> disable_irq_nosync(irq_ctx->hwirq);
> >> irq_ctx->masked = true;
> >> }
> >> @@ -89,9 +89,10 @@ static void vfio_platform_unmask(struct vfio_platform_irq *irq_ctx)
> >>
> >> spin_lock_irqsave(&irq_ctx->lock, flags);
> >>
> >> - if (irq_ctx->masked) {
> >> + if (irq_ctx->masked || irq_ctx->automasked) {
> >> enable_irq(irq_ctx->hwirq);
> >> irq_ctx->masked = false;
> >> + irq_ctx->automasked = false;
> >> }
> >>
> >> spin_unlock_irqrestore(&irq_ctx->lock, flags);
> >> @@ -152,12 +153,12 @@ static irqreturn_t vfio_automasked_irq_handler(int irq, void *dev_id)
> >>
> >> spin_lock_irqsave(&irq_ctx->lock, flags);
> >>
> >> - if (!irq_ctx->masked) {
> >> + if (!irq_ctx->masked && !irq_ctx->automasked) {
> >> ret = IRQ_HANDLED;
> >>
> >> /* automask maskable interrupts */
> >> disable_irq_nosync(irq_ctx->hwirq);
> >> - irq_ctx->masked = true;
> >> + irq_ctx->automasked = true;
> >> }
> >>
> >> spin_unlock_irqrestore(&irq_ctx->lock, flags);
> >> @@ -315,6 +316,7 @@ int vfio_platform_irq_init(struct vfio_platform_device *vdev)
> >> vdev->irqs[i].count = 1;
> >> vdev->irqs[i].hwirq = hwirq;
> >> vdev->irqs[i].masked = false;
> >> + vdev->irqs[i].automasked = false;
> >> }
> >>
> >> vdev->num_irqs = cnt;
> >> diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
> >> index 85ffe5d..8a3cfa9 100644
> >> --- a/drivers/vfio/platform/vfio_platform_private.h
> >> +++ b/drivers/vfio/platform/vfio_platform_private.h
> >> @@ -34,6 +34,7 @@ struct vfio_platform_irq {
> >> char *name;
> >> struct eventfd_ctx *trigger;
> >> bool masked;
> >> + bool automasked;
> >> spinlock_t lock;
> >> struct virqfd *unmask;
> >> struct virqfd *mask;
> >
> > Thanks,
> >
> > M.
> >
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web