| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 12 Apr 2016 10:37:43 +0200
From: Vitaly Kuznetsov <vkuznets@...hat.com>
To: Stefano Stabellini <sstabellini@...nel.org>
Cc: x86@...nel.org, Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
"H. Peter Anvin" <hpa@...or.com>,
Jiang Liu <jiang.liu@...ux.intel.com>,
"K. Y. Srinivasan" <kys@...rosoft.com>,
linux-kernel@...r.kernel.org, konrad.wilk@...cle.com,
boris.ostrovsky@...cle.com, david.vrabel@...rix.com,
jgross@...e.com
Subject: Re: Xen regression, Was: [PATCH] x86/irq: Probe for PIC presence before allocating descs for legacy IRQs
Stefano Stabellini <sstabellini@...nel.org> writes:
> Hi all,
>
> Unfortunately this patch (now commit
> 8c058b0b9c34d8c8d7912880956543769323e2d8) causes a regression on Xen
> when running on top of QEMU: the number of PIT irqs get set to 0 by
> probe_8259A but actually there are 16.
>
How would one see the regression?
8c058b0b9c34d8c8d7912880956543769323e2d8 is present since 4.4 and HVM
guests seem to work.
Other than that, why does probe_8259A() lie?
> Any suggestions on how to fix this?
>
> 1) we could revert 8c058b0b9c34d8c8d7912880956543769323e2d8
This would re-introduce the original issue I was fixing.
> 2) we could introduce an 'if (!xen_domain())' in probe_8259A
> 3) suggestions welcome
I'd suggest we make probe_8259A() work. It can only return 0 if PIC
probe by outb()/inb() fails. Why does it fail on QEMU?
>
> On Mon, 2 Nov 2015, Vitaly Kuznetsov wrote:
>> Commit d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain
>> interfaces") brought a regression for Hyper-V Gen2 instances. These
>> instances don't have i8259 legacy PIC but they use legacy IRQs for serial
>> port, rtc, and acpi. With this commit included we end up with these IRQs
>> not initialized. Earlier, there was a special workaround for legacy IRQs
>> in mp_map_pin_to_irq() doing mp_irqdomain_map() without looking at
>> nr_legacy_irqs() and now we fail in __irq_domain_alloc_irqs() when
>> irq_domain_alloc_descs() returns -EEXIST.
>>
>> The essence of the issue seems to be that early_irq_init() calls
>> arch_probe_nr_irqs() to figure out the number of legacy IRQs before
>> we probe for i8259 and gets 16. Later when init_8259A() is called we switch
>> to NULL legacy PIC and nr_legacy_irqs() starts to return 0 but we already
>> have 16 descs allocated.
>>
>> Solve the issue by separating i8259 probe from init and calling it in
>> arch_probe_nr_irqs() before we actually use nr_legacy_irqs() information.
>>
>> Fixes: d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces")
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@...hat.com>
>> ---
>> arch/x86/include/asm/i8259.h | 1 +
>> arch/x86/kernel/apic/vector.c | 3 +++
>> arch/x86/kernel/i8259.c | 24 ++++++++++++++++--------
>> 3 files changed, 20 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/i8259.h b/arch/x86/include/asm/i8259.h
>> index ccffa53..bd55a77 100644
>> --- a/arch/x86/include/asm/i8259.h
>> +++ b/arch/x86/include/asm/i8259.h
>> @@ -60,6 +60,7 @@ struct legacy_pic {
>> void (*mask_all)(void);
>> void (*restore_mask)(void);
>> void (*init)(int auto_eoi);
>> + void (*probe)(void);
>> int (*irq_pending)(unsigned int irq);
>> void (*make_irq)(unsigned int irq);
>> };
>> diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
>> index 836d11b..aadd7ae 100644
>> --- a/arch/x86/kernel/apic/vector.c
>> +++ b/arch/x86/kernel/apic/vector.c
>> @@ -361,6 +361,9 @@ int __init arch_probe_nr_irqs(void)
>> if (nr < nr_irqs)
>> nr_irqs = nr;
>>
>> + /* nr_legecy_irqs() depends on the PIC presence */
>> + legacy_pic->probe();
>> +
>> return nr_legacy_irqs();
>> }
>>
>> diff --git a/arch/x86/kernel/i8259.c b/arch/x86/kernel/i8259.c
>> index 16cb827..96f1562 100644
>> --- a/arch/x86/kernel/i8259.c
>> +++ b/arch/x86/kernel/i8259.c
>> @@ -295,16 +295,11 @@ static void unmask_8259A(void)
>> raw_spin_unlock_irqrestore(&i8259A_lock, flags);
>> }
>>
>> -static void init_8259A(int auto_eoi)
>> +static void probe_8259A(void)
>> {
>> unsigned long flags;
>> unsigned char probe_val = ~(1 << PIC_CASCADE_IR);
>> unsigned char new_val;
>> -
>> - i8259A_auto_eoi = auto_eoi;
>> -
>> - raw_spin_lock_irqsave(&i8259A_lock, flags);
>> -
>> /*
>> * Check to see if we have a PIC.
>> * Mask all except the cascade and read
>> @@ -312,16 +307,27 @@ static void init_8259A(int auto_eoi)
>> * have a PIC, we will read 0xff as opposed to the
>> * value we wrote.
>> */
>> + raw_spin_lock_irqsave(&i8259A_lock, flags);
>> +
>> outb(0xff, PIC_SLAVE_IMR); /* mask all of 8259A-2 */
>> outb(probe_val, PIC_MASTER_IMR);
>> new_val = inb(PIC_MASTER_IMR);
>> if (new_val != probe_val) {
>> printk(KERN_INFO "Using NULL legacy PIC\n");
>> legacy_pic = &null_legacy_pic;
>> - raw_spin_unlock_irqrestore(&i8259A_lock, flags);
>> - return;
>> }
>>
>> + raw_spin_unlock_irqrestore(&i8259A_lock, flags);
>> +}
>> +
>> +static void init_8259A(int auto_eoi)
>> +{
>> + unsigned long flags;
>> +
>> + i8259A_auto_eoi = auto_eoi;
>> +
>> + raw_spin_lock_irqsave(&i8259A_lock, flags);
>> +
>> outb(0xff, PIC_MASTER_IMR); /* mask all of 8259A-1 */
>>
>> /*
>> @@ -388,6 +394,7 @@ struct legacy_pic null_legacy_pic = {
>> .mask_all = legacy_pic_noop,
>> .restore_mask = legacy_pic_noop,
>> .init = legacy_pic_int_noop,
>> + .probe = legacy_pic_noop,
>> .irq_pending = legacy_pic_irq_pending_noop,
>> .make_irq = legacy_pic_uint_noop,
>> };
>> @@ -400,6 +407,7 @@ struct legacy_pic default_legacy_pic = {
>> .mask_all = mask_8259A,
>> .restore_mask = unmask_8259A,
>> .init = init_8259A,
>> + .probe = probe_8259A,
>> .irq_pending = i8259A_irq_pending,
>> .make_irq = make_8259A_irq,
>> };
>> --
>> 2.4.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@...r.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>>
--
Vitaly
Powered by blists - more mailing lists