[PATCHv3] Move IOMMU faults handling into softirq for AMD-Vi.

[PATCHv3] Move IOMMU faults handling into softirq for AMD-Vi.

[Dario Faggioli]

[-- Attachment #1.1.1: Type: text/plain, Size: 3553 bytes --]

Hello Wei,

Here it is, and it seems to work for me, but of course I'm not testing
PPR. If you could give this a go and let me know... I'll repost in a
separate thread if it happens to be fine.

Thanks again,
Dario

--

Move IOMMU faults handling into softirq for AMD-Vi.
                  
Dealing with interrupts from AMD-Vi IOMMU(s) is deferred to a softirq-tasklet,
raised by the actual IRQ handler. To avoid more interrupts being generated
(because of further faults), they must be masked in the IOMMU within the low
level IRQ handler and enabled back in the tasklet body. Notice that this may
cause the log to overflow, but none of the existing entry will be overwritten.
                                 
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

diff -r 15ab61865ecb xen/drivers/passthrough/amd/iommu_init.c
--- a/xen/drivers/passthrough/amd/iommu_init.c	Tue Jan 17 12:40:52 2012 +0000
+++ b/xen/drivers/passthrough/amd/iommu_init.c	Wed Jan 18 13:01:23 2012 +0100
@@ -32,6 +32,8 @@
 
 static int __initdata nr_amd_iommus;
 
+static struct tasklet amd_iommu_irq_tasklet;
+
 unsigned short ivrs_bdf_entries;
 static struct radix_tree_root ivrs_maps;
 struct list_head amd_iommu_head;
@@ -689,14 +691,48 @@ static void iommu_check_ppr_log(struct a
     spin_unlock_irqrestore(&iommu->lock, flags);
 }
 
+static void do_amd_iommu_irq(unsigned long data)
+{
+    struct amd_iommu *iommu;
+
+    if ( !iommu_found() )
+    {
+        AMD_IOMMU_DEBUG("no device found, something must be very wrong!\n");
+        return;
+    }
+
+    /*
+     * No matter from where the interrupt came from, check all the
+     * IOMMUs present in the system. This allows for having just one
+     * tasklet (instead of one per each IOMMUs).
+     */
+    for_each_amd_iommu ( iommu ) {
+        iommu_check_event_log(iommu);
+
+        if ( iommu->ppr_log.buffer != NULL )
+            iommu_check_ppr_log(iommu);
+    }
+}
+
 static void iommu_interrupt_handler(int irq, void *dev_id,
                                     struct cpu_user_regs *regs)
 {
+    u32 entry;
+    unsigned long flags;
     struct amd_iommu *iommu = dev_id;
-    iommu_check_event_log(iommu);
 
-    if ( iommu->ppr_log.buffer != NULL )
-        iommu_check_ppr_log(iommu);
+    spin_lock_irqsave(&iommu->lock, flags);
+
+    /* Silence interrupts from both event and PPR logging */
+    entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
+    iommu_clear_bit(&entry, IOMMU_STATUS_EVENT_LOG_INT_SHIFT);
+    iommu_clear_bit(&entry, IOMMU_STATUS_PPR_LOG_INT_SHIFT);
+    writel(entry, iommu->mmio_base+IOMMU_STATUS_MMIO_OFFSET);
+
+    spin_unlock_irqrestore(&iommu->lock, flags);
+
+    /* It is the tasklet that will clear the logs and re-enable interrupts */
+    tasklet_schedule(&amd_iommu_irq_tasklet);
 }
 
 static int __init set_iommu_interrupt_handler(struct amd_iommu *iommu)
@@ -876,6 +912,8 @@ static int __init amd_iommu_init_one(str
     printk("AMD-Vi: IOMMU %d Enabled.\n", nr_amd_iommus );
     nr_amd_iommus++;
 
+    softirq_tasklet_init(&amd_iommu_irq_tasklet, do_amd_iommu_irq, 0);
+
     return 0;
 
 error_out:


-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-------------------------------------------------------------------
Dario Faggioli, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
PhD Candidate, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa (Italy)


[-- Attachment #1.1.2: iommu-fault-tasklet_amd.patch --]
[-- Type: text/x-patch, Size: 3058 bytes --]

# HG changeset patch
# Parent 15ab61865ecbd146f6ce65fbea5bf49bfd9c6cb1
Move IOMMU faults handling into softirq for AMD-Vi.
                  
Dealing with interrupts from AMD-Vi IOMMU(s) is deferred to a softirq-tasklet,
raised by the actual IRQ handler. To avoid more interrupts being generated
(because of further faults), they must be masked in the IOMMU within the low
level IRQ handler and enabled back in the tasklet body. Notice that this may
cause the log to overflow, but none of the existing entry will be overwritten.
                                 
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

diff -r 15ab61865ecb xen/drivers/passthrough/amd/iommu_init.c
--- a/xen/drivers/passthrough/amd/iommu_init.c	Tue Jan 17 12:40:52 2012 +0000
+++ b/xen/drivers/passthrough/amd/iommu_init.c	Wed Jan 18 13:01:23 2012 +0100
@@ -32,6 +32,8 @@
 
 static int __initdata nr_amd_iommus;
 
+static struct tasklet amd_iommu_irq_tasklet;
+
 unsigned short ivrs_bdf_entries;
 static struct radix_tree_root ivrs_maps;
 struct list_head amd_iommu_head;
@@ -689,14 +691,48 @@ static void iommu_check_ppr_log(struct a
     spin_unlock_irqrestore(&iommu->lock, flags);
 }
 
+static void do_amd_iommu_irq(unsigned long data)
+{
+    struct amd_iommu *iommu;
+
+    if ( !iommu_found() )
+    {
+        AMD_IOMMU_DEBUG("no device found, something must be very wrong!\n");
+        return;
+    }
+
+    /*
+     * No matter from where the interrupt came from, check all the
+     * IOMMUs present in the system. This allows for having just one
+     * tasklet (instead of one per each IOMMUs).
+     */
+    for_each_amd_iommu ( iommu ) {
+        iommu_check_event_log(iommu);
+
+        if ( iommu->ppr_log.buffer != NULL )
+            iommu_check_ppr_log(iommu);
+    }
+}
+
 static void iommu_interrupt_handler(int irq, void *dev_id,
                                     struct cpu_user_regs *regs)
 {
+    u32 entry;
+    unsigned long flags;
     struct amd_iommu *iommu = dev_id;
-    iommu_check_event_log(iommu);
 
-    if ( iommu->ppr_log.buffer != NULL )
-        iommu_check_ppr_log(iommu);
+    spin_lock_irqsave(&iommu->lock, flags);
+
+    /* Silence interrupts from both event and PPR logging */
+    entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
+    iommu_clear_bit(&entry, IOMMU_STATUS_EVENT_LOG_INT_SHIFT);
+    iommu_clear_bit(&entry, IOMMU_STATUS_PPR_LOG_INT_SHIFT);
+    writel(entry, iommu->mmio_base+IOMMU_STATUS_MMIO_OFFSET);
+
+    spin_unlock_irqrestore(&iommu->lock, flags);
+
+    /* It is the tasklet that will clear the logs and re-enable interrupts */
+    tasklet_schedule(&amd_iommu_irq_tasklet);
 }
 
 static int __init set_iommu_interrupt_handler(struct amd_iommu *iommu)
@@ -876,6 +912,8 @@ static int __init amd_iommu_init_one(str
     printk("AMD-Vi: IOMMU %d Enabled.\n", nr_amd_iommus );
     nr_amd_iommus++;
 
+    softirq_tasklet_init(&amd_iommu_irq_tasklet, do_amd_iommu_irq, 0);
+
     return 0;
 
 error_out:

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCHv3] Move IOMMU faults handling into softirq for AMD-Vi.

[Wei Wang]

On 01/18/2012 02:51 PM, Dario Faggioli wrote:
> Hello Wei,
>
> Here it is, and it seems to work for me, but of course I'm not testing
> PPR. If you could give this a go and let me know... I'll repost in a
> separate thread if it happens to be fine.

Hi,
It works on my machine with PPR log enabled.
Thanks,
Wei

Tested-by: Wei Wang <wei.wang2@amd.com>

> Thanks again,
> Dario
>
> --
>
> Move IOMMU faults handling into softirq for AMD-Vi.
>
> Dealing with interrupts from AMD-Vi IOMMU(s) is deferred to a softirq-tasklet,
> raised by the actual IRQ handler. To avoid more interrupts being generated
> (because of further faults), they must be masked in the IOMMU within the low
> level IRQ handler and enabled back in the tasklet body. Notice that this may
> cause the log to overflow, but none of the existing entry will be overwritten.
>
> Signed-off-by: Dario Faggioli<dario.faggioli@citrix.com>
>
> diff -r 15ab61865ecb xen/drivers/passthrough/amd/iommu_init.c
> --- a/xen/drivers/passthrough/amd/iommu_init.c	Tue Jan 17 12:40:52 2012 +0000
> +++ b/xen/drivers/passthrough/amd/iommu_init.c	Wed Jan 18 13:01:23 2012 +0100
> @@ -32,6 +32,8 @@
>
>   static int __initdata nr_amd_iommus;
>
> +static struct tasklet amd_iommu_irq_tasklet;
> +
>   unsigned short ivrs_bdf_entries;
>   static struct radix_tree_root ivrs_maps;
>   struct list_head amd_iommu_head;
> @@ -689,14 +691,48 @@ static void iommu_check_ppr_log(struct a
>       spin_unlock_irqrestore(&iommu->lock, flags);
>   }
>
> +static void do_amd_iommu_irq(unsigned long data)
> +{
> +    struct amd_iommu *iommu;
> +
> +    if ( !iommu_found() )
> +    {
> +        AMD_IOMMU_DEBUG("no device found, something must be very wrong!\n");
> +        return;
> +    }
> +
> +    /*
> +     * No matter from where the interrupt came from, check all the
> +     * IOMMUs present in the system. This allows for having just one
> +     * tasklet (instead of one per each IOMMUs).
> +     */
> +    for_each_amd_iommu ( iommu ) {
> +        iommu_check_event_log(iommu);
> +
> +        if ( iommu->ppr_log.buffer != NULL )
> +            iommu_check_ppr_log(iommu);
> +    }
> +}
> +
>   static void iommu_interrupt_handler(int irq, void *dev_id,
>                                       struct cpu_user_regs *regs)
>   {
> +    u32 entry;
> +    unsigned long flags;
>       struct amd_iommu *iommu = dev_id;
> -    iommu_check_event_log(iommu);
>
> -    if ( iommu->ppr_log.buffer != NULL )
> -        iommu_check_ppr_log(iommu);
> +    spin_lock_irqsave(&iommu->lock, flags);
> +
> +    /* Silence interrupts from both event and PPR logging */
> +    entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
> +    iommu_clear_bit(&entry, IOMMU_STATUS_EVENT_LOG_INT_SHIFT);
> +    iommu_clear_bit(&entry, IOMMU_STATUS_PPR_LOG_INT_SHIFT);
> +    writel(entry, iommu->mmio_base+IOMMU_STATUS_MMIO_OFFSET);
> +
> +    spin_unlock_irqrestore(&iommu->lock, flags);
> +
> +    /* It is the tasklet that will clear the logs and re-enable interrupts */
> +    tasklet_schedule(&amd_iommu_irq_tasklet);
>   }
>
>   static int __init set_iommu_interrupt_handler(struct amd_iommu *iommu)
> @@ -876,6 +912,8 @@ static int __init amd_iommu_init_one(str
>       printk("AMD-Vi: IOMMU %d Enabled.\n", nr_amd_iommus );
>       nr_amd_iommus++;
>
> +    softirq_tasklet_init(&amd_iommu_irq_tasklet, do_amd_iommu_irq, 0);
> +
>       return 0;
>
>   error_out:
>
>

[PATCHv3] Move IOMMU faults handling into softirq for AMD-Vi.

[Dario Faggioli]

[-- Attachment #1.1.1: Type: text/plain, Size: 3247 bytes --]

Dealing with interrupts from AMD-Vi IOMMU(s) is deferred to a softirq-tasklet,
raised by the actual IRQ handler. To avoid more interrupts being generated
(because of further faults), they must be masked in the IOMMU within the low
level IRQ handler and enabled back in the tasklet body. Notice that this may
cause the log to overflow, but none of the existing entry will be overwritten.
                                 
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

diff -r 15ab61865ecb xen/drivers/passthrough/amd/iommu_init.c
--- a/xen/drivers/passthrough/amd/iommu_init.c	Tue Jan 17 12:40:52 2012 +0000
+++ b/xen/drivers/passthrough/amd/iommu_init.c	Wed Jan 18 13:01:23 2012 +0100
@@ -32,6 +32,8 @@
 
 static int __initdata nr_amd_iommus;
 
+static struct tasklet amd_iommu_irq_tasklet;
+
 unsigned short ivrs_bdf_entries;
 static struct radix_tree_root ivrs_maps;
 struct list_head amd_iommu_head;
@@ -689,14 +691,48 @@ static void iommu_check_ppr_log(struct a
     spin_unlock_irqrestore(&iommu->lock, flags);
 }
 
+static void do_amd_iommu_irq(unsigned long data)
+{
+    struct amd_iommu *iommu;
+
+    if ( !iommu_found() )
+    {
+        AMD_IOMMU_DEBUG("no device found, something must be very wrong!\n");
+        return;
+    }
+
+    /*
+     * No matter from where the interrupt came from, check all the
+     * IOMMUs present in the system. This allows for having just one
+     * tasklet (instead of one per each IOMMUs).
+     */
+    for_each_amd_iommu ( iommu ) {
+        iommu_check_event_log(iommu);
+
+        if ( iommu->ppr_log.buffer != NULL )
+            iommu_check_ppr_log(iommu);
+    }
+}
+
 static void iommu_interrupt_handler(int irq, void *dev_id,
                                     struct cpu_user_regs *regs)
 {
+    u32 entry;
+    unsigned long flags;
     struct amd_iommu *iommu = dev_id;
-    iommu_check_event_log(iommu);
 
-    if ( iommu->ppr_log.buffer != NULL )
-        iommu_check_ppr_log(iommu);
+    spin_lock_irqsave(&iommu->lock, flags);
+
+    /* Silence interrupts from both event and PPR logging */
+    entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
+    iommu_clear_bit(&entry, IOMMU_STATUS_EVENT_LOG_INT_SHIFT);
+    iommu_clear_bit(&entry, IOMMU_STATUS_PPR_LOG_INT_SHIFT);
+    writel(entry, iommu->mmio_base+IOMMU_STATUS_MMIO_OFFSET);
+
+    spin_unlock_irqrestore(&iommu->lock, flags);
+
+    /* It is the tasklet that will clear the logs and re-enable interrupts */
+    tasklet_schedule(&amd_iommu_irq_tasklet);
 }
 
 static int __init set_iommu_interrupt_handler(struct amd_iommu *iommu)
@@ -876,6 +912,8 @@ static int __init amd_iommu_init_one(str
     printk("AMD-Vi: IOMMU %d Enabled.\n", nr_amd_iommus );
     nr_amd_iommus++;
 
+    softirq_tasklet_init(&amd_iommu_irq_tasklet, do_amd_iommu_irq, 0);
+
     return 0;
 
 error_out:

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-------------------------------------------------------------------
Dario Faggioli, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
PhD Candidate, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa (Italy)


[-- Attachment #1.1.2: iommu-fault-tasklet_amd.patch --]
[-- Type: text/x-patch, Size: 3058 bytes --]

# HG changeset patch
# Parent 15ab61865ecbd146f6ce65fbea5bf49bfd9c6cb1
Move IOMMU faults handling into softirq for AMD-Vi.
                  
Dealing with interrupts from AMD-Vi IOMMU(s) is deferred to a softirq-tasklet,
raised by the actual IRQ handler. To avoid more interrupts being generated
(because of further faults), they must be masked in the IOMMU within the low
level IRQ handler and enabled back in the tasklet body. Notice that this may
cause the log to overflow, but none of the existing entry will be overwritten.
                                 
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

diff -r 15ab61865ecb xen/drivers/passthrough/amd/iommu_init.c
--- a/xen/drivers/passthrough/amd/iommu_init.c	Tue Jan 17 12:40:52 2012 +0000
+++ b/xen/drivers/passthrough/amd/iommu_init.c	Wed Jan 18 13:01:23 2012 +0100
@@ -32,6 +32,8 @@
 
 static int __initdata nr_amd_iommus;
 
+static struct tasklet amd_iommu_irq_tasklet;
+
 unsigned short ivrs_bdf_entries;
 static struct radix_tree_root ivrs_maps;
 struct list_head amd_iommu_head;
@@ -689,14 +691,48 @@ static void iommu_check_ppr_log(struct a
     spin_unlock_irqrestore(&iommu->lock, flags);
 }
 
+static void do_amd_iommu_irq(unsigned long data)
+{
+    struct amd_iommu *iommu;
+
+    if ( !iommu_found() )
+    {
+        AMD_IOMMU_DEBUG("no device found, something must be very wrong!\n");
+        return;
+    }
+
+    /*
+     * No matter from where the interrupt came from, check all the
+     * IOMMUs present in the system. This allows for having just one
+     * tasklet (instead of one per each IOMMUs).
+     */
+    for_each_amd_iommu ( iommu ) {
+        iommu_check_event_log(iommu);
+
+        if ( iommu->ppr_log.buffer != NULL )
+            iommu_check_ppr_log(iommu);
+    }
+}
+
 static void iommu_interrupt_handler(int irq, void *dev_id,
                                     struct cpu_user_regs *regs)
 {
+    u32 entry;
+    unsigned long flags;
     struct amd_iommu *iommu = dev_id;
-    iommu_check_event_log(iommu);
 
-    if ( iommu->ppr_log.buffer != NULL )
-        iommu_check_ppr_log(iommu);
+    spin_lock_irqsave(&iommu->lock, flags);
+
+    /* Silence interrupts from both event and PPR logging */
+    entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
+    iommu_clear_bit(&entry, IOMMU_STATUS_EVENT_LOG_INT_SHIFT);
+    iommu_clear_bit(&entry, IOMMU_STATUS_PPR_LOG_INT_SHIFT);
+    writel(entry, iommu->mmio_base+IOMMU_STATUS_MMIO_OFFSET);
+
+    spin_unlock_irqrestore(&iommu->lock, flags);
+
+    /* It is the tasklet that will clear the logs and re-enable interrupts */
+    tasklet_schedule(&amd_iommu_irq_tasklet);
 }
 
 static int __init set_iommu_interrupt_handler(struct amd_iommu *iommu)
@@ -876,6 +912,8 @@ static int __init amd_iommu_init_one(str
     printk("AMD-Vi: IOMMU %d Enabled.\n", nr_amd_iommus );
     nr_amd_iommus++;
 
+    softirq_tasklet_init(&amd_iommu_irq_tasklet, do_amd_iommu_irq, 0);
+
     return 0;
 
 error_out:

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCHv3] Move IOMMU faults handling into softirq for AMD-Vi.

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 509 bytes --]

On Wed, 2012-01-18 at 16:53 +0100, Wei Wang wrote:
> Hi,
> It works on my machine with PPR log enabled.
>
Ok then, patch resent in a separate thread right now.

Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-------------------------------------------------------------------
Dario Faggioli, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
PhD Candidate, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa (Italy)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel