All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH][VTD] Fix apic pin to interrupt remapping table index
@ 2009-06-03  9:28 Han, Weidong
  2009-06-03 12:02 ` Keir Fraser
  0 siblings, 1 reply; 8+ messages in thread
From: Han, Weidong @ 2009-06-03  9:28 UTC (permalink / raw)
  To: 'Keir Fraser'; +Cc: 'xen-devel', 'Jan Beulich'

[-- Attachment #1: Type: text/plain, Size: 412 bytes --]

Originally, it calls xmalloc to set index in ioapic_rte_to_remap_entry(). When make with debug=y, it may trigger spinlock BUG_ON because allocate memory with interrupt disabled.

This patch doesn't allocate list_head entry in ioapic_rte_to_remap_entry(), instead allocate the array in enable_intremap() to avoid allocating memory with interrupt disabled.


Signed-off-by: Weidong Han <weidong.han@intel.com>

[-- Attachment #2: apic-pin-index.patch --]
[-- Type: application/octet-stream, Size: 4312 bytes --]

diff -r 200fc86a4258 xen/drivers/passthrough/vtd/intremap.c
--- a/xen/drivers/passthrough/vtd/intremap.c	Wed Jun 03 11:22:43 2009 +0800
+++ b/xen/drivers/passthrough/vtd/intremap.c	Wed Jun 03 14:59:24 2009 +0800
@@ -41,62 +41,32 @@
  */
 #define MAX_IOAPIC_PIN_NUM  256
 
-struct ioapicid_pin_intremap_index {
-	struct list_head list;
-	unsigned int ioapic_id;
-	unsigned int pin;
-	int intremap_index;
-};
+/*
+ * Array of interrupt remapping table index for ioapic pins,
+ * The size is MAX_IO_APICS * MAX_IOAPIC_PIN_NUM;
+ */
+typedef int apic_pin_2_ir_idx_t[MAX_IOAPIC_PIN_NUM];
+static apic_pin_2_ir_idx_t *apic_pin_2_ir_idx;
 
-static struct list_head ioapic_pin_to_intremap_index[MAX_IOAPIC_PIN_NUM];
-
-static int init_ioapic_pin_intremap_index(void)
+static int init_apic_pin_2_ir_idx(void)
 {
     static int initialized = 0;
-    int i;
+    int i, j;
 
     if ( initialized == 1 )
         return 0;
 
-    for ( i = 0; i < MAX_IOAPIC_PIN_NUM; i++ )
-        INIT_LIST_HEAD(&ioapic_pin_to_intremap_index[i]);
+    /* allocate apic_pin_2_ir_idx, and initialize to -1 */
+    apic_pin_2_ir_idx = xmalloc_array(apic_pin_2_ir_idx_t, MAX_IO_APICS);
+
+    if ( !apic_pin_2_ir_idx )
+        return -ENOMEM;
+
+    for ( i = 0; i < MAX_IO_APICS; i++ )
+        for ( j = 0; j < MAX_IOAPIC_PIN_NUM; j++ )
+            apic_pin_2_ir_idx[i][j] = -1;
 
     initialized = 1;
-    return 0;
-}
-
-static int get_ioapic_pin_intremap_index(unsigned int ioapic_id,
-                                         unsigned int pin)
-{
-    struct ioapicid_pin_intremap_index *entry;
-    struct list_head *pos, *tmp;
-
-    list_for_each_safe ( pos, tmp, &ioapic_pin_to_intremap_index[pin] )
-    {
-        entry = list_entry(pos, struct ioapicid_pin_intremap_index, list);
-        if ( entry->ioapic_id == ioapic_id )
-            return entry->intremap_index;
-    }
-
-    return -1;
-}
-
-static int set_ioapic_pin_intremap_index(unsigned int ioapic_id,
-                                         unsigned int pin,
-                                         int index)
-{
-    struct ioapicid_pin_intremap_index *entry;
-
-    entry = xmalloc(struct ioapicid_pin_intremap_index);
-    if ( !entry )
-        return -ENOMEM;
-
-    entry->ioapic_id = ioapic_id;
-    entry->pin = pin;
-    entry->intremap_index = index;
-
-    list_add_tail(&entry->list, &ioapic_pin_to_intremap_index[pin]);
-
     return 0;
 }
 
@@ -160,7 +130,7 @@
 }
 
 static int ioapic_rte_to_remap_entry(struct iommu *iommu,
-    int apic_id, unsigned int ioapic_pin, struct IO_xAPIC_route_entry *old_rte,
+    int apic, unsigned int ioapic_pin, struct IO_xAPIC_route_entry *old_rte,
     unsigned int rte_upper, unsigned int value)
 {
     struct iremap_entry *iremap_entry = NULL, *iremap_entries;
@@ -174,12 +144,12 @@
     remap_rte = (struct IO_APIC_route_remap_entry *) old_rte;
     spin_lock_irqsave(&ir_ctrl->iremap_lock, flags);
 
-    index = get_ioapic_pin_intremap_index(apic_id, ioapic_pin);
+    index = apic_pin_2_ir_idx[apic][ioapic_pin];
     if ( index < 0 )
     {
         ir_ctrl->iremap_index++;
         index = ir_ctrl->iremap_index;
-        set_ioapic_pin_intremap_index(apic_id, ioapic_pin, index);
+        apic_pin_2_ir_idx[apic][ioapic_pin] = index;
     }
 
     if ( index > IREMAP_ENTRY_NR - 1 )
@@ -218,7 +188,7 @@
         new_ire.lo.res_1 = 0;
         new_ire.lo.vector = new_rte.vector;
         new_ire.lo.res_2 = 0;
-        new_ire.hi.sid = apicid_to_bdf(apic_id);
+        new_ire.hi.sid = apicid_to_bdf(IO_APIC_ID(apic));
 
         new_ire.hi.sq = 0;    /* comparing all 16-bit of SID */
         new_ire.hi.svt = 1;   /* requestor ID verification SID/SQ */
@@ -357,7 +327,7 @@
     remap_rte->mask = saved_mask;
 
     ASSERT(ioapic_pin < MAX_IOAPIC_PIN_NUM);
-    if ( ioapic_rte_to_remap_entry(iommu, IO_APIC_ID(apic), ioapic_pin,
+    if ( ioapic_rte_to_remap_entry(iommu, apic, ioapic_pin,
                                    &old_rte, rte_upper, value) )
     {
         *IO_APIC_BASE(apic) = rte_upper ? (reg + 1) : reg;
@@ -628,9 +598,7 @@
     /* After set SIRTP, we should do globally invalidate the IEC */
     iommu_flush_iec_global(iommu);
 
-    init_ioapic_pin_intremap_index();
-
-    return 0;
+    return init_apic_pin_2_ir_idx();
 }
 
 void disable_intremap(struct iommu *iommu)

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

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][VTD] Fix apic pin to interrupt remapping table index
  2009-06-03  9:28 [PATCH][VTD] Fix apic pin to interrupt remapping table index Han, Weidong
@ 2009-06-03 12:02 ` Keir Fraser
  2009-06-04  1:16   ` Han, Weidong
  2009-06-04  3:59   ` Isaku Yamahata
  0 siblings, 2 replies; 8+ messages in thread
From: Keir Fraser @ 2009-06-03 12:02 UTC (permalink / raw)
  To: Han, Weidong; +Cc: Isaku Yamahata, 'xen-devel', 'Jan Beulich'

Wasteful of memory, so I checked in a modified version as c/s 19707, which
dynamically sizes the array. Please take a look and check it's okay.

It probably breaks ia64 build due to undefined nr_ioapics and
nr_ioapic_registers[], but I think yours broke ia64 too so we're even. :-)

Isaku: can you suggest ia64 equivalents for nr_ioapics and
nr_ioapic_registers[]? We can do some ifdef magic at the top of intremap.c,
including defining a nr_ioapic_registers() macro, if that helps.

 Thanks,
 Keir

On 03/06/2009 10:28, "Han, Weidong" <weidong.han@intel.com> wrote:

> Originally, it calls xmalloc to set index in ioapic_rte_to_remap_entry(). When
> make with debug=y, it may trigger spinlock BUG_ON because allocate memory with
> interrupt disabled.
> 
> This patch doesn't allocate list_head entry in ioapic_rte_to_remap_entry(),
> instead allocate the array in enable_intremap() to avoid allocating memory
> with interrupt disabled.
> 
> 
> Signed-off-by: Weidong Han <weidong.han@intel.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH][VTD] Fix apic pin to interrupt remapping table index
  2009-06-03 12:02 ` Keir Fraser
@ 2009-06-04  1:16   ` Han, Weidong
  2009-06-04  4:18     ` Cui, Dexuan
  2009-06-04  7:36     ` Keir Fraser
  2009-06-04  3:59   ` Isaku Yamahata
  1 sibling, 2 replies; 8+ messages in thread
From: Han, Weidong @ 2009-06-04  1:16 UTC (permalink / raw)
  To: 'Keir Fraser'
  Cc: 'Isaku Yamahata', 'xen-devel',
	'Jan Beulich',
	Cui, Dexuan

Keir,

Yes, your modified patch saves memory. I wanted to do like it. But Dexuan is working on a x2apic patch, which will move interrupt remapping enabling before IOAPIC setup. So I'm wondering ioapic stuffs (nr_ioapic_registers[], nr_ioapics, etc.) aren't ready when enable interrupt remapping after that moving. Dexuan, can you have a look at it?

Regards,
Weidong

Keir Fraser wrote:
> Wasteful of memory, so I checked in a modified version as c/s 19707,
> which dynamically sizes the array. Please take a look and check it's
> okay. 
> 
> It probably breaks ia64 build due to undefined nr_ioapics and
> nr_ioapic_registers[], but I think yours broke ia64 too so we're
> even. :-) 
> 
> Isaku: can you suggest ia64 equivalents for nr_ioapics and
> nr_ioapic_registers[]? We can do some ifdef magic at the top of
> intremap.c, including defining a nr_ioapic_registers() macro, if that
> helps. 
> 
>  Thanks,
>  Keir
> 
> On 03/06/2009 10:28, "Han, Weidong" <weidong.han@intel.com> wrote:
> 
>> Originally, it calls xmalloc to set index in
>> ioapic_rte_to_remap_entry(). When make with debug=y, it may trigger
>> spinlock BUG_ON because allocate memory with interrupt disabled. 
>> 
>> This patch doesn't allocate list_head entry in
>> ioapic_rte_to_remap_entry(), instead allocate the array in
>> enable_intremap() to avoid allocating memory with interrupt
>> disabled.  
>> 
>> 
>> Signed-off-by: Weidong Han <weidong.han@intel.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][VTD] Fix apic pin to interrupt remapping table index
  2009-06-03 12:02 ` Keir Fraser
  2009-06-04  1:16   ` Han, Weidong
@ 2009-06-04  3:59   ` Isaku Yamahata
  2009-06-04  5:39     ` Han, Weidong
  1 sibling, 1 reply; 8+ messages in thread
From: Isaku Yamahata @ 2009-06-04  3:59 UTC (permalink / raw)
  To: Keir Fraser; +Cc: 'xen-devel', Han, Weidong, 'Jan Beulich'

On Wed, Jun 03, 2009 at 01:02:59PM +0100, Keir Fraser wrote:
> Wasteful of memory, so I checked in a modified version as c/s 19707, which
> dynamically sizes the array. Please take a look and check it's okay.
> 
> It probably breaks ia64 build due to undefined nr_ioapics and
> nr_ioapic_registers[], but I think yours broke ia64 too so we're even. :-)
> 
> Isaku: can you suggest ia64 equivalents for nr_ioapics and
> nr_ioapic_registers[]? We can do some ifdef magic at the top of intremap.c,
> including defining a nr_ioapic_registers() macro, if that helps.

Here is the patch. I'm going to test it, though. 
 
[] operator is an obstacle to use CPP magic. 
So I did just quick hack to send this patch as soon as possible. 
You may want to wrap it with a function. 
 
thanks, 

vtd: ia64 fix of intremap.c

19707:07cf79dfb59c caused compilation error on ia64.
This patch fixes it.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

diff --git a/xen/arch/ia64/linux-xen/iosapic.c b/xen/arch/ia64/linux-xen/iosapic.c
--- a/xen/arch/ia64/linux-xen/iosapic.c
+++ b/xen/arch/ia64/linux-xen/iosapic.c
@@ -1275,4 +1275,22 @@ int iosapic_guest_write(unsigned long ph
 	spin_unlock_irqrestore(&irq_descp(vec)->lock, flags);
 	return 0;
 }
+
+/* for vtd interrupt remapping. xen/drivers/vtd/intremap.c */
+int iosapic_get_nr_iosapics(void)
+{
+	int index;
+
+	for (index = NR_IOSAPICS - 1; index >= 0; index--) {
+		if (iosapic_lists[index].addr)
+			break;
+	}
+
+	return index + 1;
+}
+
+int iosapic_get_nr_pins(int index)
+{
+	return iosapic_lists[index].num_rte;
+}
 #endif /* XEN */
diff --git a/xen/drivers/passthrough/vtd/intremap.c b/xen/drivers/passthrough/vtd/intremap.c
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -33,6 +33,10 @@
 
 #ifdef __ia64__
 #define dest_SMI -1
+#define nr_ioapics              iosapic_get_nr_iosapics()
+#define nr_ioapic_registers(i)  iosapic_get_nr_pins(i)
+#else
+#define nr_ioapic_registers(i)  nr_ioapic_registers[i]
 #endif
 
 /* apic_pin_2_ir_idx[apicid][pin] = interrupt remapping table index */
@@ -45,7 +49,7 @@ static int init_apic_pin_2_ir_idx(void)
 
     nr_pins = 0;
     for ( i = 0; i < nr_ioapics; i++ )
-        nr_pins += nr_ioapic_registers[i];
+        nr_pins += nr_ioapic_registers(i);
 
     _apic_pin_2_ir_idx = xmalloc_array(unsigned int, nr_pins);
     apic_pin_2_ir_idx = xmalloc_array(unsigned int *, nr_ioapics);
@@ -63,7 +67,7 @@ static int init_apic_pin_2_ir_idx(void)
     for ( i = 0; i < nr_ioapics; i++ )
     {
         apic_pin_2_ir_idx[i] = &_apic_pin_2_ir_idx[nr_pins];
-        nr_pins += nr_ioapic_registers[i];
+        nr_pins += nr_ioapic_registers(i);
     }
 
     return 0;
diff --git a/xen/include/asm-ia64/linux-xen/asm/iosapic.h b/xen/include/asm-ia64/linux-xen/asm/iosapic.h
--- a/xen/include/asm-ia64/linux-xen/asm/iosapic.h
+++ b/xen/include/asm-ia64/linux-xen/asm/iosapic.h
@@ -186,6 +186,9 @@ struct rte_entry {
 #define IOSAPIC_RTEINDEX(reg)	(((reg) - 0x10) >> 1)
 extern unsigned long ia64_vector_mask[];
 extern unsigned long ia64_xen_vector[];
+
+int iosapic_get_nr_iosapics(void);
+int iosapic_get_nr_pins(int index);
 #endif /* XEN */
 
 #define IO_APIC_BASE(idx) ((unsigned int *)iosapic_lists[idx].addr)

-- 
yamahata

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH][VTD] Fix apic pin to interrupt remapping table index
  2009-06-04  1:16   ` Han, Weidong
@ 2009-06-04  4:18     ` Cui, Dexuan
  2009-06-04  7:36     ` Keir Fraser
  1 sibling, 0 replies; 8+ messages in thread
From: Cui, Dexuan @ 2009-06-04  4:18 UTC (permalink / raw)
  To: Han, Weidong, 'Keir Fraser'
  Cc: 'Isaku Yamahata', 'xen-devel', 'Jan Beulich'

Yes, I'm organizing the order. I can further change the code. :-)

Thanks,
-- Dexuan

-----Original Message-----
From: Han, Weidong 
Sent: 2009?6?4? 9:16
To: 'Keir Fraser'
Cc: 'xen-devel'; 'Jan Beulich'; 'Isaku Yamahata'; Cui, Dexuan
Subject: RE: [xen-devel][PATCH][VTD] Fix apic pin to interrupt remapping table index

Keir,

Yes, your modified patch saves memory. I wanted to do like it. But Dexuan is working on a x2apic patch, which will move interrupt remapping enabling before IOAPIC setup. So I'm wondering ioapic stuffs (nr_ioapic_registers[], nr_ioapics, etc.) aren't ready when enable interrupt remapping after that moving. Dexuan, can you have a look at it?

Regards,
Weidong

Keir Fraser wrote:
> Wasteful of memory, so I checked in a modified version as c/s 19707,
> which dynamically sizes the array. Please take a look and check it's
> okay. 
> 
> It probably breaks ia64 build due to undefined nr_ioapics and
> nr_ioapic_registers[], but I think yours broke ia64 too so we're
> even. :-) 
> 
> Isaku: can you suggest ia64 equivalents for nr_ioapics and
> nr_ioapic_registers[]? We can do some ifdef magic at the top of
> intremap.c, including defining a nr_ioapic_registers() macro, if that
> helps. 
> 
>  Thanks,
>  Keir
> 
> On 03/06/2009 10:28, "Han, Weidong" <weidong.han@intel.com> wrote:
> 
>> Originally, it calls xmalloc to set index in
>> ioapic_rte_to_remap_entry(). When make with debug=y, it may trigger
>> spinlock BUG_ON because allocate memory with interrupt disabled. 
>> 
>> This patch doesn't allocate list_head entry in
>> ioapic_rte_to_remap_entry(), instead allocate the array in
>> enable_intremap() to avoid allocating memory with interrupt
>> disabled.  
>> 
>> 
>> Signed-off-by: Weidong Han <weidong.han@intel.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH][VTD] Fix apic pin to interrupt remapping table index
  2009-06-04  3:59   ` Isaku Yamahata
@ 2009-06-04  5:39     ` Han, Weidong
  0 siblings, 0 replies; 8+ messages in thread
From: Han, Weidong @ 2009-06-04  5:39 UTC (permalink / raw)
  To: 'Isaku Yamahata', 'Keir Fraser'
  Cc: 'xen-devel', 'Jan Beulich'

Look fine for me. Thanks.

Regards,
Weidong

Isaku Yamahata wrote:
> On Wed, Jun 03, 2009 at 01:02:59PM +0100, Keir Fraser wrote:
>> Wasteful of memory, so I checked in a modified version as c/s 19707,
>> which dynamically sizes the array. Please take a look and check it's
>> okay. 
>> 
>> It probably breaks ia64 build due to undefined nr_ioapics and
>> nr_ioapic_registers[], but I think yours broke ia64 too so we're
>> even. :-) 
>> 
>> Isaku: can you suggest ia64 equivalents for nr_ioapics and
>> nr_ioapic_registers[]? We can do some ifdef magic at the top of
>> intremap.c, including defining a nr_ioapic_registers() macro, if
>> that helps. 
> 
> Here is the patch. I'm going to test it, though.
> 
> [] operator is an obstacle to use CPP magic.
> So I did just quick hack to send this patch as soon as possible.
> You may want to wrap it with a function.
> 
> thanks,
> 
> vtd: ia64 fix of intremap.c
> 
> 19707:07cf79dfb59c caused compilation error on ia64.
> This patch fixes it.
> 
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> 
> diff --git a/xen/arch/ia64/linux-xen/iosapic.c
> b/xen/arch/ia64/linux-xen/iosapic.c ---
> a/xen/arch/ia64/linux-xen/iosapic.c +++
> b/xen/arch/ia64/linux-xen/iosapic.c @@ -1275,4 +1275,22 @@ int
>  	iosapic_guest_write(unsigned long ph
>  	spin_unlock_irqrestore(&irq_descp(vec)->lock, flags); return 0;
>  }
> +
> +/* for vtd interrupt remapping. xen/drivers/vtd/intremap.c */
> +int iosapic_get_nr_iosapics(void)
> +{
> +	int index;
> +
> +	for (index = NR_IOSAPICS - 1; index >= 0; index--) {
> +		if (iosapic_lists[index].addr)
> +			break;
> +	}
> +
> +	return index + 1;
> +}
> +
> +int iosapic_get_nr_pins(int index)
> +{
> +	return iosapic_lists[index].num_rte;
> +}
>  #endif /* XEN */
> diff --git a/xen/drivers/passthrough/vtd/intremap.c
> b/xen/drivers/passthrough/vtd/intremap.c ---
> a/xen/drivers/passthrough/vtd/intremap.c +++
> b/xen/drivers/passthrough/vtd/intremap.c @@ -33,6 +33,10 @@
> 
>  #ifdef __ia64__
>  #define dest_SMI -1
> +#define nr_ioapics              iosapic_get_nr_iosapics()
> +#define nr_ioapic_registers(i)  iosapic_get_nr_pins(i)
> +#else
> +#define nr_ioapic_registers(i)  nr_ioapic_registers[i]
>  #endif
> 
>  /* apic_pin_2_ir_idx[apicid][pin] = interrupt remapping table index
> */ @@ -45,7 +49,7 @@ static int init_apic_pin_2_ir_idx(void)
> 
>      nr_pins = 0;
>      for ( i = 0; i < nr_ioapics; i++ )
> -        nr_pins += nr_ioapic_registers[i];
> +        nr_pins += nr_ioapic_registers(i);
> 
>      _apic_pin_2_ir_idx = xmalloc_array(unsigned int, nr_pins);
>      apic_pin_2_ir_idx = xmalloc_array(unsigned int *, nr_ioapics);
> @@ -63,7 +67,7 @@ static int init_apic_pin_2_ir_idx(void)
>      for ( i = 0; i < nr_ioapics; i++ )
>      {
>          apic_pin_2_ir_idx[i] = &_apic_pin_2_ir_idx[nr_pins];
> -        nr_pins += nr_ioapic_registers[i];
> +        nr_pins += nr_ioapic_registers(i);
>      }
> 
>      return 0;
> diff --git a/xen/include/asm-ia64/linux-xen/asm/iosapic.h
> b/xen/include/asm-ia64/linux-xen/asm/iosapic.h ---
> a/xen/include/asm-ia64/linux-xen/asm/iosapic.h +++
> b/xen/include/asm-ia64/linux-xen/asm/iosapic.h @@ -186,6 +186,9 @@
>  struct rte_entry { #define IOSAPIC_RTEINDEX(reg)	(((reg) - 0x10) >>
>  1) extern unsigned long ia64_vector_mask[];
>  extern unsigned long ia64_xen_vector[];
> +
> +int iosapic_get_nr_iosapics(void);
> +int iosapic_get_nr_pins(int index);
>  #endif /* XEN */
> 
>  #define IO_APIC_BASE(idx) ((unsigned int *)iosapic_lists[idx].addr)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][VTD] Fix apic pin to interrupt remapping table index
  2009-06-04  1:16   ` Han, Weidong
  2009-06-04  4:18     ` Cui, Dexuan
@ 2009-06-04  7:36     ` Keir Fraser
  2009-06-04  7:47       ` Han, Weidong
  1 sibling, 1 reply; 8+ messages in thread
From: Keir Fraser @ 2009-06-04  7:36 UTC (permalink / raw)
  To: Han, Weidong
  Cc: 'Isaku Yamahata', 'xen-devel',
	'Jan Beulich',
	Cui, Dexuan

Ok, we do already support x2apic though, so I don't know what extra Dexuan's
patch will do? Is x2apic+intremap currently broken?

 -- Keir

On 04/06/2009 02:16, "Han, Weidong" <weidong.han@intel.com> wrote:

> Keir,
> 
> Yes, your modified patch saves memory. I wanted to do like it. But Dexuan is
> working on a x2apic patch, which will move interrupt remapping enabling before
> IOAPIC setup. So I'm wondering ioapic stuffs (nr_ioapic_registers[],
> nr_ioapics, etc.) aren't ready when enable interrupt remapping after that
> moving. Dexuan, can you have a look at it?
> 
> Regards,
> Weidong
> 
> Keir Fraser wrote:
>> Wasteful of memory, so I checked in a modified version as c/s 19707,
>> which dynamically sizes the array. Please take a look and check it's
>> okay. 
>> 
>> It probably breaks ia64 build due to undefined nr_ioapics and
>> nr_ioapic_registers[], but I think yours broke ia64 too so we're
>> even. :-) 
>> 
>> Isaku: can you suggest ia64 equivalents for nr_ioapics and
>> nr_ioapic_registers[]? We can do some ifdef magic at the top of
>> intremap.c, including defining a nr_ioapic_registers() macro, if that
>> helps. 
>> 
>>  Thanks,
>>  Keir
>> 
>> On 03/06/2009 10:28, "Han, Weidong" <weidong.han@intel.com> wrote:
>> 
>>> Originally, it calls xmalloc to set index in
>>> ioapic_rte_to_remap_entry(). When make with debug=y, it may trigger
>>> spinlock BUG_ON because allocate memory with interrupt disabled.
>>> 
>>> This patch doesn't allocate list_head entry in
>>> ioapic_rte_to_remap_entry(), instead allocate the array in
>>> enable_intremap() to avoid allocating memory with interrupt
>>> disabled.  
>>> 
>>> 
>>> Signed-off-by: Weidong Han <weidong.han@intel.com>
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH][VTD] Fix apic pin to interrupt remapping table index
  2009-06-04  7:36     ` Keir Fraser
@ 2009-06-04  7:47       ` Han, Weidong
  0 siblings, 0 replies; 8+ messages in thread
From: Han, Weidong @ 2009-06-04  7:47 UTC (permalink / raw)
  To: 'Keir Fraser'
  Cc: 'Isaku Yamahata', 'xen-devel',
	'Jan Beulich',
	Cui, Dexuan

Keir Fraser wrote:
> Ok, we do already support x2apic though, so I don't know what extra
> Dexuan's patch will do? Is x2apic+intremap currently broken?
> 

No break now. I think current x2apic implementation is incomplete. Per spec, interrupt remapping is pre-requirement for x2apic. So should enable interrupt remapping before x2apic. I think your modified patch is fine for now.  We can change it when it's really necessary.

Regards,
Weidong

>  -- Keir
> 
> On 04/06/2009 02:16, "Han, Weidong" <weidong.han@intel.com> wrote:
> 
>> Keir,
>> 
>> Yes, your modified patch saves memory. I wanted to do like it. But
>> Dexuan is working on a x2apic patch, which will move interrupt
>> remapping enabling before IOAPIC setup. So I'm wondering ioapic
>> stuffs (nr_ioapic_registers[], nr_ioapics, etc.) aren't ready when
>> enable interrupt remapping after that moving. Dexuan, can you have a
>> look at it? 
>> 
>> Regards,
>> Weidong
>> 
>> Keir Fraser wrote:
>>> Wasteful of memory, so I checked in a modified version as c/s 19707,
>>> which dynamically sizes the array. Please take a look and check
>>> it's okay. 
>>> 
>>> It probably breaks ia64 build due to undefined nr_ioapics and
>>> nr_ioapic_registers[], but I think yours broke ia64 too so we're
>>> even. :-) 
>>> 
>>> Isaku: can you suggest ia64 equivalents for nr_ioapics and
>>> nr_ioapic_registers[]? We can do some ifdef magic at the top of
>>> intremap.c, including defining a nr_ioapic_registers() macro, if
>>> that helps. 
>>> 
>>>  Thanks,
>>>  Keir
>>> 
>>> On 03/06/2009 10:28, "Han, Weidong" <weidong.han@intel.com> wrote:
>>> 
>>>> Originally, it calls xmalloc to set index in
>>>> ioapic_rte_to_remap_entry(). When make with debug=y, it may trigger
>>>> spinlock BUG_ON because allocate memory with interrupt disabled.
>>>> 
>>>> This patch doesn't allocate list_head entry in
>>>> ioapic_rte_to_remap_entry(), instead allocate the array in
>>>> enable_intremap() to avoid allocating memory with interrupt
>>>> disabled. 
>>>> 
>>>> 
>>>> Signed-off-by: Weidong Han <weidong.han@intel.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2009-06-04  7:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-03  9:28 [PATCH][VTD] Fix apic pin to interrupt remapping table index Han, Weidong
2009-06-03 12:02 ` Keir Fraser
2009-06-04  1:16   ` Han, Weidong
2009-06-04  4:18     ` Cui, Dexuan
2009-06-04  7:36     ` Keir Fraser
2009-06-04  7:47       ` Han, Weidong
2009-06-04  3:59   ` Isaku Yamahata
2009-06-04  5:39     ` Han, Weidong

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.