Re: [Xen-devel] [PATCH v2] [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Jiang Liu]

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 majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/