Re: [Xen-devel] [PATCH v2] [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c
On 2015/1/13 18:09, David Vrabel wrote:
> On 13/01/15 02:23, Jiang Liu wrote:
>> On 2015/1/12 23:01, David Vrabel wrote:
>>> On 12/01/15 13:39, Jiang Liu wrote:
Commit b81975eade8c ("x86, irq: Clean up irqdomain transition code")
breaks xen IRQ allocation because xen_smp_prepare_cpus() doesn't invoke
setup_IO_APIC(), so no irqdomains created for IOAPICs and
mp_map_pin_to_irq() fails at the very beginning.
Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
irqdomain for IOAPICs.
>>>
>>> Having Xen call setup_IO_APIC() to initialize the irq domains then having to
>>> add special cases to it is just wrong.
>>>
>>> The bits of init deferred by mp_register_apic() are also deferred to
>>> two different places which looks odd.
>>>
>>> What about something like the following (untested) patch?
>> Hi David,
>> Really appreciate your suggestions about Xen IOAPIC:)
>> Your patch solves the issue, but it will break a coming patch
>> set to enable hierarchy irqdomain on x86.
>
> For 3.19 I would recommend this patch. Since it's been tested and it's
> pretty small. Can you spin a version of this with a proper changelog.
> please?
Hi David,
Good suggestion, I will send out the patch tomorrow.
Regards!
Gerry
>
>> To enable hierarchy irqdomain on x86, need to build
>> irqdomains in following order:
>> 1) create irqdomain for CPU vector domain
>> 2) create irqdomains for interrupt remapping unit if interrupt
>>remapping is enabled.
>> 3) create irqdomains for IOAPIC/MSI/HPET etc.
>>
>> Function arch_early_ioapic_init() is called before initializing
>> interrupt remapping subsystem. So creating IOAPIC irqdomains in
>> arch_early_ioapic_init() will break hierarchy irqdomain with
>> interrupt remapping enabled. I will post the x86 hierarchy
>> irqdomain patch soon.
>>
>> So could you please help to advice on:
>
> I don't know enough about this area to comment on these authoritatively
> and I don't have time to look into this in more detail right now, but to
> the best of my understanding.
>
>> 1) Is IOAPIC irqdomain needed for PV?
>
> PV dom0 only requires a mechanism to obtain properties of the IOAPIC
> interrupt lines. The actual physical interrupts are delivered to the
> hypervisor. It doesn't require any of the irq setup.
>
>> 2) Is IOAPIC irqdomain needed for HVM?
>
> Yes.
>
>> 3) Is IOAPIC irqdomain needed for Dom0?
>
>
> See (1) above.
>
>> 4) What's the proper hook point to create IOAPIC irqdomain for Xen
>>UP systems?
>
> Don't know.
>
> David
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c
On 13/01/15 02:23, Jiang Liu wrote:
> On 2015/1/12 23:01, David Vrabel wrote:
>> On 12/01/15 13:39, Jiang Liu wrote:
>>> Commit b81975eade8c ("x86, irq: Clean up irqdomain transition code")
>>> breaks xen IRQ allocation because xen_smp_prepare_cpus() doesn't invoke
>>> setup_IO_APIC(), so no irqdomains created for IOAPICs and
>>> mp_map_pin_to_irq() fails at the very beginning.
>>>
>>> Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
>>> irqdomain for IOAPICs.
>>
>> Having Xen call setup_IO_APIC() to initialize the irq domains then having to
>> add special cases to it is just wrong.
>>
>> The bits of init deferred by mp_register_apic() are also deferred to
>> two different places which looks odd.
>>
>> What about something like the following (untested) patch?
> Hi David,
> Really appreciate your suggestions about Xen IOAPIC:)
> Your patch solves the issue, but it will break a coming patch
> set to enable hierarchy irqdomain on x86.
For 3.19 I would recommend this patch. Since it's been tested and it's
pretty small. Can you spin a version of this with a proper changelog.
please?
> To enable hierarchy irqdomain on x86, need to build
> irqdomains in following order:
> 1) create irqdomain for CPU vector domain
> 2) create irqdomains for interrupt remapping unit if interrupt
>remapping is enabled.
> 3) create irqdomains for IOAPIC/MSI/HPET etc.
>
> Function arch_early_ioapic_init() is called before initializing
> interrupt remapping subsystem. So creating IOAPIC irqdomains in
> arch_early_ioapic_init() will break hierarchy irqdomain with
> interrupt remapping enabled. I will post the x86 hierarchy
> irqdomain patch soon.
>
> So could you please help to advice on:
I don't know enough about this area to comment on these authoritatively
and I don't have time to look into this in more detail right now, but to
the best of my understanding.
> 1) Is IOAPIC irqdomain needed for PV?
PV dom0 only requires a mechanism to obtain properties of the IOAPIC
interrupt lines. The actual physical interrupts are delivered to the
hypervisor. It doesn't require any of the irq setup.
> 2) Is IOAPIC irqdomain needed for HVM?
Yes.
> 3) Is IOAPIC irqdomain needed for Dom0?
See (1) above.
> 4) What's the proper hook point to create IOAPIC irqdomain for Xen
>UP systems?
Don't know.
David
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c
On Tue, 13 Jan 2015, Sander Eikelenboom wrote:
>
> Monday, January 12, 2015, 4:01:00 PM, you wrote:
>
> > On 12/01/15 13:39, Jiang Liu wrote:
> >> Commit b81975eade8c ("x86, irq: Clean up irqdomain transition code")
> >> breaks xen IRQ allocation because xen_smp_prepare_cpus() doesn't invoke
> >> setup_IO_APIC(), so no irqdomains created for IOAPICs and
> >> mp_map_pin_to_irq() fails at the very beginning.
> >>
> >> Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
> >> irqdomain for IOAPICs.
>
> > Having Xen call setup_IO_APIC() to initialize the irq domains then having to
> > add special cases to it is just wrong.
>
> > The bits of init deferred by mp_register_apic() are also deferred to
> > two different places which looks odd.
>
> > What about something like the following (untested) patch?
>
> Hi David / Gerry,
>
> David's patch (after fixing a few compile issues) fixes the problem.
>
> The power button now works for me on:
> - intel baremetal
> - intel xen
> - amd baremetal (no issues with the override anymore)
> - amd xen (no freeze issues anymore)
Can someone please send a proper patch with changelog?
Thanks,
tglx
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c
On 2015/1/12 23:01, David Vrabel wrote:
> On 12/01/15 13:39, Jiang Liu wrote:
>> Commit b81975eade8c ("x86, irq: Clean up irqdomain transition code")
>> breaks xen IRQ allocation because xen_smp_prepare_cpus() doesn't invoke
>> setup_IO_APIC(), so no irqdomains created for IOAPICs and
>> mp_map_pin_to_irq() fails at the very beginning.
>>
>> Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
>> irqdomain for IOAPICs.
>
> Having Xen call setup_IO_APIC() to initialize the irq domains then having to
> add special cases to it is just wrong.
>
> The bits of init deferred by mp_register_apic() are also deferred to
> two different places which looks odd.
>
> What about something like the following (untested) patch?
Hi David,
Really appreciate your suggestions about Xen IOAPIC:)
Your patch solves the issue, but it will break a coming patch
set to enable hierarchy irqdomain on x86.
To enable hierarchy irqdomain on x86, need to build
irqdomains in following order:
1) create irqdomain for CPU vector domain
2) create irqdomains for interrupt remapping unit if interrupt
remapping is enabled.
3) create irqdomains for IOAPIC/MSI/HPET etc.
Function arch_early_ioapic_init() is called before initializing
interrupt remapping subsystem. So creating IOAPIC irqdomains in
arch_early_ioapic_init() will break hierarchy irqdomain with
interrupt remapping enabled. I will post the x86 hierarchy
irqdomain patch soon.
So could you please help to advice on:
1) Is IOAPIC irqdomain needed for PV?
2) Is IOAPIC irqdomain needed for HVM?
3) Is IOAPIC irqdomain needed for Dom0?
4) What's the proper hook point to create IOAPIC irqdomain for Xen
UP systems?
I really need to get more understanding of Xen interrupt subsystem:)
Thanks!
Gerry
>
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 3f5f604..e180680 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -253,8 +253,10 @@ int __init arch_early_ioapic_init(void)
> if (!nr_legacy_irqs())
> io_apic_irqs = ~0UL;
>
> - for_each_ioapic(i)
> + for_each_ioapic(i) {
> + BUG_ON(mp_irqdomain_create(ioapic));
> alloc_ioapic_saved_registers(i);
> + }
>
> /*
>* For legacy IRQ's, start with assigning irq0 to irq15 to
> @@ -2379,8 +2381,6 @@ void __init setup_IO_APIC(void)
> io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
>
> apic_printk(APIC_VERBOSE, "ENABLING IO-APIC IRQs\n");
> - for_each_ioapic(ioapic)
> - BUG_ON(mp_irqdomain_create(ioapic));
>
> /*
> * Set up IO-APIC IRQ routing.
> @@ -2929,7 +2929,8 @@ int mp_register_ioapic(int id, u32 address, u32
> gsi_base,
> /*
>* If mp_register_ioapic() is called during early boot stage when
>* walking ACPI/SFI/DT tables, it's too early to create irqdomain,
> - * we are still using bootmem allocator. So delay it to setup_IO_APIC().
> + * we are still using bootmem allocator. So delay it to
> + * arch_early_ioapic_init().
>*/
> if (hotplug) {
> if (mp_irqdomain_create(idx)) {
>
>> --- a/arch/x86/kernel/apic/io_apic.c
>> +++ b/arch/x86/kernel/apic/io_apic.c
>> @@ -2369,6 +2381,15 @@ static void ioapic_destroy_irqdomain(int idx)
>> ioapics[idx].pin_info = NULL;
>> }
>>
>> +static void setup_IO_APIC_IDs(void)
>> +{
>> +if (xen_domain())
>> +return;
>
> This would have to xen_pv_domain().
>
> David
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c
Monday, January 12, 2015, 4:01:00 PM, you wrote:
> On 12/01/15 13:39, Jiang Liu wrote:
>> Commit b81975eade8c ("x86, irq: Clean up irqdomain transition code")
>> breaks xen IRQ allocation because xen_smp_prepare_cpus() doesn't invoke
>> setup_IO_APIC(), so no irqdomains created for IOAPICs and
>> mp_map_pin_to_irq() fails at the very beginning.
>>
>> Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
>> irqdomain for IOAPICs.
> Having Xen call setup_IO_APIC() to initialize the irq domains then having to
> add special cases to it is just wrong.
> The bits of init deferred by mp_register_apic() are also deferred to
> two different places which looks odd.
> What about something like the following (untested) patch?
Hi David / Gerry,
David's patch (after fixing a few compile issues) fixes the problem.
The power button now works for me on:
- intel baremetal
- intel xen
- amd baremetal (no issues with the override anymore)
- amd xen (no freeze issues anymore)
Big thanks David !
--
Sander
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 3f5f604..e180680 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -253,8 +253,10 @@ int __init arch_early_ioapic_init(void)
> if (!nr_legacy_irqs())
> io_apic_irqs = ~0UL;
>
> - for_each_ioapic(i)
> + for_each_ioapic(i) {
> + BUG_ON(mp_irqdomain_create(ioapic));
> alloc_ioapic_saved_registers(i);
> + }
>
> /*
> * For legacy IRQ's, start with assigning irq0 to irq15 to
> @@ -2379,8 +2381,6 @@ void __init setup_IO_APIC(void)
> io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
>
> apic_printk(APIC_VERBOSE, "ENABLING IO-APIC IRQs\n");
> - for_each_ioapic(ioapic)
> - BUG_ON(mp_irqdomain_create(ioapic));
>
> /*
> * Set up IO-APIC IRQ routing.
> @@ -2929,7 +2929,8 @@ int mp_register_ioapic(int id, u32 address, u32
> gsi_base,
> /*
> * If mp_register_ioapic() is called during early boot stage when
> * walking ACPI/SFI/DT tables, it's too early to create irqdomain,
> -* we are still using bootmem allocator. So delay it to
> setup_IO_APIC().
> +* we are still using bootmem allocator. So delay it to
> +* arch_early_ioapic_init().
> */
> if (hotplug) {
> if (mp_irqdomain_create(idx)) {
>> --- a/arch/x86/kernel/apic/io_apic.c
>> +++ b/arch/x86/kernel/apic/io_apic.c
>> @@ -2369,6 +2381,15 @@ static void ioapic_destroy_irqdomain(int idx)
>> ioapics[idx].pin_info = NULL;
>> }
>>
>> +static void setup_IO_APIC_IDs(void)
>> +{
>> + if (xen_domain())
>> + return;
> This would have to xen_pv_domain().
> David
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c
On 12/01/15 13:39, Jiang Liu wrote:
> Commit b81975eade8c ("x86, irq: Clean up irqdomain transition code")
> breaks xen IRQ allocation because xen_smp_prepare_cpus() doesn't invoke
> setup_IO_APIC(), so no irqdomains created for IOAPICs and
> mp_map_pin_to_irq() fails at the very beginning.
>
> Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
> irqdomain for IOAPICs.
Having Xen call setup_IO_APIC() to initialize the irq domains then having to
add special cases to it is just wrong.
The bits of init deferred by mp_register_apic() are also deferred to
two different places which looks odd.
What about something like the following (untested) patch?
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 3f5f604..e180680 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -253,8 +253,10 @@ int __init arch_early_ioapic_init(void)
if (!nr_legacy_irqs())
io_apic_irqs = ~0UL;
- for_each_ioapic(i)
+ for_each_ioapic(i) {
+ BUG_ON(mp_irqdomain_create(ioapic));
alloc_ioapic_saved_registers(i);
+ }
/*
* For legacy IRQ's, start with assigning irq0 to irq15 to
@@ -2379,8 +2381,6 @@ void __init setup_IO_APIC(void)
io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
apic_printk(APIC_VERBOSE, "ENABLING IO-APIC IRQs\n");
- for_each_ioapic(ioapic)
- BUG_ON(mp_irqdomain_create(ioapic));
/*
* Set up IO-APIC IRQ routing.
@@ -2929,7 +2929,8 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
/*
* If mp_register_ioapic() is called during early boot stage when
* walking ACPI/SFI/DT tables, it's too early to create irqdomain,
-* we are still using bootmem allocator. So delay it to setup_IO_APIC().
+* we are still using bootmem allocator. So delay it to
+* arch_early_ioapic_init().
*/
if (hotplug) {
if (mp_irqdomain_create(idx)) {
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -2369,6 +2381,15 @@ static void ioapic_destroy_irqdomain(int idx)
> ioapics[idx].pin_info = NULL;
> }
>
> +static void setup_IO_APIC_IDs(void)
> +{
> + if (xen_domain())
> + return;
This would have to xen_pv_domain().
David
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c
Commit b81975eade8c ("x86, irq: Clean up irqdomain transition code")
breaks xen IRQ allocation because xen_smp_prepare_cpus() doesn't invoke
setup_IO_APIC(), so no irqdomains created for IOAPICs and
mp_map_pin_to_irq() fails at the very beginning.
Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
irqdomain for IOAPICs.
Signed-off-by: Jiang Liu
---
Hi Sander,
Could you please help to test this patch against 3.19-rc3?
I have reworked it based Konrad's suggestions.
Thanks!
Gerry
---
arch/x86/kernel/apic/io_apic.c | 31 +++
arch/x86/xen/smp.c |1 +
2 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 3f5f60406ab1..81d4faeb8040 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -38,6 +38,8 @@
#include
#include
+#include
+
#include
#include
#include
@@ -2165,6 +2167,9 @@ static inline void __init check_timer(void)
unsigned long flags;
int no_pin1 = 0;
+ if (!nr_legacy_irqs())
+ return;
+
local_irq_save(flags);
/*
@@ -2185,6 +2190,13 @@ static inline void __init check_timer(void)
apic_write(APIC_LVT0, APIC_LVT_MASKED | APIC_DM_EXTINT);
legacy_pic->init(1);
+ /*
+* legacy_pic will be changed to null_legacy_pic if init() fails to
+* to detect legacy PIC hardware. Recheck again.
+*/
+ if (!nr_legacy_irqs())
+ goto out;
+
pin1 = find_isa_irq_pin(0, mp_INT);
apic1 = find_isa_irq_apic(0, mp_INT);
pin2 = ioapic_i8259.pin;
@@ -2369,6 +2381,15 @@ static void ioapic_destroy_irqdomain(int idx)
ioapics[idx].pin_info = NULL;
}
+static void setup_IO_APIC_IDs(void)
+{
+ if (xen_domain())
+ return;
+
+ x86_init.mpparse.setup_ioapic_ids();
+ sync_Arb_IDs();
+}
+
void __init setup_IO_APIC(void)
{
int ioapic;
@@ -2382,16 +2403,10 @@ void __init setup_IO_APIC(void)
for_each_ioapic(ioapic)
BUG_ON(mp_irqdomain_create(ioapic));
- /*
- * Set up IO-APIC IRQ routing.
- */
- x86_init.mpparse.setup_ioapic_ids();
-
- sync_Arb_IDs();
+ setup_IO_APIC_IDs();
setup_IO_APIC_irqs();
init_IO_APIC_traps();
- if (nr_legacy_irqs())
- check_timer();
+ check_timer();
ioapic_initialized = 1;
}
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 4c071aeb8417..9f404df64422 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -327,6 +327,7 @@ static void __init xen_smp_prepare_cpus(unsigned int
max_cpus)
xen_raw_printk(m);
panic(m);
}
+ setup_IO_APIC();
xen_init_lock_cpu(0);
smp_store_boot_cpu_info();
--
1.7.10.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
