* [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.