RE: [PATCH 1/1] X86: Probe for PIC and set legacy_pic appropriately
> -Original Message-
> From: H. Peter Anvin [mailto:h...@zytor.com]
> Sent: Friday, April 11, 2014 3:36 PM
> To: KY Srinivasan; x...@kernel.org; gre...@linuxfoundation.org; linux-
> ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de;
> a...@canonical.com; jasow...@redhat.com; t...@linutronix.de;
> jbeul...@suse.com; b...@alien8.de
> Cc: Thomas Gleixner
> Subject: Re: [PATCH 1/1] X86: Probe for PIC and set legacy_pic appropriately
>
> On 04/11/2014 04:02 PM, K. Y. Srinivasan wrote:
> >
> > diff --git a/arch/x86/kernel/i8259.c b/arch/x86/kernel/i8259.c index
> > 2e977b5..0a57a19 100644
> > --- a/arch/x86/kernel/i8259.c
> > +++ b/arch/x86/kernel/i8259.c
> > @@ -299,11 +299,22 @@ static void unmask_8259A(void) static void
> > init_8259A(int auto_eoi) {
> > unsigned long flags;
> > + unsigned char probe_val = 0xfb;
> >
> > i8259A_auto_eoi = auto_eoi;
> >
> > raw_spin_lock_irqsave(_lock, flags);
> >
> > + /* Check to see if we have a PIC */
> > + outb(probe_val, PIC_MASTER_IMR);
> > + probe_val = inb(PIC_MASTER_IMR);
> > + if (probe_val == 0xff) {
> > + printk(KERN_INFO "Using NULL legacy PIC\n");
> > + legacy_pic = _legacy_pic;
> > + raw_spin_unlock_irqrestore(_lock, flags);
> > + return;
> > + }
> > +
> > outb(0xff, PIC_MASTER_IMR); /* mask all of 8259A-1 */
> > outb(0xff, PIC_SLAVE_IMR); /* mask all of 8259A-2 */
> >
>
> I think it is important to document what this actually means here. 0xfb is
> "mask all except for the cascade."
>
> Arguably, we should do this after masking the slave PIC.
>
> On a whole other subject... the whole __byte() logic is a just plain horrific
> piece of crap. Clearly whomever wrote it had never heard of a union.
>
> I'm still horrifically confused, though, why we mask IRQ 2 (the cascade
> interrupt) in i8259.c, but we explicitly do *not* mask IRQ 2 in boot/pm.c (nor
> in the assembly code on which boot/pm.c was based.) In fact, it looks like we
> never unmask it. I'm guessing that the masking status of the cascade input
> simply doesn't matter.
>
> At the very least, though, we shouldn't have 0xfb as a magic number, but use
> ~(1U << PIC_CASCADE_IR).
I will get rid of this magic number as you have suggested.
K. Y
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
Re: [PATCH 1/1] X86: Probe for PIC and set legacy_pic appropriately
On 04/11/2014 04:02 PM, K. Y. Srinivasan wrote:
>
> diff --git a/arch/x86/kernel/i8259.c b/arch/x86/kernel/i8259.c
> index 2e977b5..0a57a19 100644
> --- a/arch/x86/kernel/i8259.c
> +++ b/arch/x86/kernel/i8259.c
> @@ -299,11 +299,22 @@ static void unmask_8259A(void)
> static void init_8259A(int auto_eoi)
> {
> unsigned long flags;
> + unsigned char probe_val = 0xfb;
>
> i8259A_auto_eoi = auto_eoi;
>
> raw_spin_lock_irqsave(_lock, flags);
>
> + /* Check to see if we have a PIC */
> + outb(probe_val, PIC_MASTER_IMR);
> + probe_val = inb(PIC_MASTER_IMR);
> + if (probe_val == 0xff) {
> + printk(KERN_INFO "Using NULL legacy PIC\n");
> + legacy_pic = _legacy_pic;
> + raw_spin_unlock_irqrestore(_lock, flags);
> + return;
> + }
> +
> outb(0xff, PIC_MASTER_IMR); /* mask all of 8259A-1 */
> outb(0xff, PIC_SLAVE_IMR); /* mask all of 8259A-2 */
>
I think it is important to document what this actually means here. 0xfb
is "mask all except for the cascade."
Arguably, we should do this after masking the slave PIC.
On a whole other subject... the whole __byte() logic is a just plain
horrific piece of crap. Clearly whomever wrote it had never heard of a
union.
I'm still horrifically confused, though, why we mask IRQ 2 (the cascade
interrupt) in i8259.c, but we explicitly do *not* mask IRQ 2 in
boot/pm.c (nor in the assembly code on which boot/pm.c was based.) In
fact, it looks like we never unmask it. I'm guessing that the masking
status of the cascade input simply doesn't matter.
At the very least, though, we shouldn't have 0xfb as a magic number, but
use ~(1U << PIC_CASCADE_IR).
-hpa
--
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/
[PATCH 1/1] X86: Probe for PIC and set legacy_pic appropriately
Probe for the existance of legacy PIC, if one does not exist, use the
null_legacy_pic.
This patch implements the proposal put forth by H. Peter Anvin
.
Signed-off-by: K. Y. Srinivasan
---
arch/x86/kernel/cpu/mshyperv.c |2 --
arch/x86/kernel/i8259.c| 11 +++
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 47359f7..9ca9587 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -133,8 +133,6 @@ static void __init ms_hyperv_init_platform(void)
printk(KERN_INFO "HyperV: LAPIC Timer Frequency: %#x\n",
lapic_timer_frequency);
- printk(KERN_INFO "HyperV: Using null_legacy_pic\n");
- legacy_pic = _legacy_pic;
}
#endif
diff --git a/arch/x86/kernel/i8259.c b/arch/x86/kernel/i8259.c
index 2e977b5..0a57a19 100644
--- a/arch/x86/kernel/i8259.c
+++ b/arch/x86/kernel/i8259.c
@@ -299,11 +299,22 @@ static void unmask_8259A(void)
static void init_8259A(int auto_eoi)
{
unsigned long flags;
+ unsigned char probe_val = 0xfb;
i8259A_auto_eoi = auto_eoi;
raw_spin_lock_irqsave(_lock, flags);
+ /* Check to see if we have a PIC */
+ outb(probe_val, PIC_MASTER_IMR);
+ probe_val = inb(PIC_MASTER_IMR);
+ if (probe_val == 0xff) {
+ printk(KERN_INFO "Using NULL legacy PIC\n");
+ legacy_pic = _legacy_pic;
+ raw_spin_unlock_irqrestore(_lock, flags);
+ return;
+ }
+
outb(0xff, PIC_MASTER_IMR); /* mask all of 8259A-1 */
outb(0xff, PIC_SLAVE_IMR); /* mask all of 8259A-2 */
--
1.7.4.1
--
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/
[PATCH 1/1] X86: Probe for PIC and set legacy_pic appropriately
Probe for the existance of legacy PIC, if one does not exist, use the
null_legacy_pic.
This patch implements the proposal put forth by H. Peter Anvin
h...@linux.intel.com.
Signed-off-by: K. Y. Srinivasan k...@microsoft.com
---
arch/x86/kernel/cpu/mshyperv.c |2 --
arch/x86/kernel/i8259.c| 11 +++
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 47359f7..9ca9587 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -133,8 +133,6 @@ static void __init ms_hyperv_init_platform(void)
printk(KERN_INFO HyperV: LAPIC Timer Frequency: %#x\n,
lapic_timer_frequency);
- printk(KERN_INFO HyperV: Using null_legacy_pic\n);
- legacy_pic = null_legacy_pic;
}
#endif
diff --git a/arch/x86/kernel/i8259.c b/arch/x86/kernel/i8259.c
index 2e977b5..0a57a19 100644
--- a/arch/x86/kernel/i8259.c
+++ b/arch/x86/kernel/i8259.c
@@ -299,11 +299,22 @@ static void unmask_8259A(void)
static void init_8259A(int auto_eoi)
{
unsigned long flags;
+ unsigned char probe_val = 0xfb;
i8259A_auto_eoi = auto_eoi;
raw_spin_lock_irqsave(i8259A_lock, flags);
+ /* Check to see if we have a PIC */
+ outb(probe_val, PIC_MASTER_IMR);
+ probe_val = inb(PIC_MASTER_IMR);
+ if (probe_val == 0xff) {
+ printk(KERN_INFO Using NULL legacy PIC\n);
+ legacy_pic = null_legacy_pic;
+ raw_spin_unlock_irqrestore(i8259A_lock, flags);
+ return;
+ }
+
outb(0xff, PIC_MASTER_IMR); /* mask all of 8259A-1 */
outb(0xff, PIC_SLAVE_IMR); /* mask all of 8259A-2 */
--
1.7.4.1
--
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/
Re: [PATCH 1/1] X86: Probe for PIC and set legacy_pic appropriately
On 04/11/2014 04:02 PM, K. Y. Srinivasan wrote:
diff --git a/arch/x86/kernel/i8259.c b/arch/x86/kernel/i8259.c
index 2e977b5..0a57a19 100644
--- a/arch/x86/kernel/i8259.c
+++ b/arch/x86/kernel/i8259.c
@@ -299,11 +299,22 @@ static void unmask_8259A(void)
static void init_8259A(int auto_eoi)
{
unsigned long flags;
+ unsigned char probe_val = 0xfb;
i8259A_auto_eoi = auto_eoi;
raw_spin_lock_irqsave(i8259A_lock, flags);
+ /* Check to see if we have a PIC */
+ outb(probe_val, PIC_MASTER_IMR);
+ probe_val = inb(PIC_MASTER_IMR);
+ if (probe_val == 0xff) {
+ printk(KERN_INFO Using NULL legacy PIC\n);
+ legacy_pic = null_legacy_pic;
+ raw_spin_unlock_irqrestore(i8259A_lock, flags);
+ return;
+ }
+
outb(0xff, PIC_MASTER_IMR); /* mask all of 8259A-1 */
outb(0xff, PIC_SLAVE_IMR); /* mask all of 8259A-2 */
I think it is important to document what this actually means here. 0xfb
is mask all except for the cascade.
Arguably, we should do this after masking the slave PIC.
On a whole other subject... the whole __byte() logic is a just plain
horrific piece of crap. Clearly whomever wrote it had never heard of a
union.
I'm still horrifically confused, though, why we mask IRQ 2 (the cascade
interrupt) in i8259.c, but we explicitly do *not* mask IRQ 2 in
boot/pm.c (nor in the assembly code on which boot/pm.c was based.) In
fact, it looks like we never unmask it. I'm guessing that the masking
status of the cascade input simply doesn't matter.
At the very least, though, we shouldn't have 0xfb as a magic number, but
use ~(1U PIC_CASCADE_IR).
-hpa
--
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/
RE: [PATCH 1/1] X86: Probe for PIC and set legacy_pic appropriately
-Original Message-
From: H. Peter Anvin [mailto:h...@zytor.com]
Sent: Friday, April 11, 2014 3:36 PM
To: KY Srinivasan; x...@kernel.org; gre...@linuxfoundation.org; linux-
ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de;
a...@canonical.com; jasow...@redhat.com; t...@linutronix.de;
jbeul...@suse.com; b...@alien8.de
Cc: Thomas Gleixner
Subject: Re: [PATCH 1/1] X86: Probe for PIC and set legacy_pic appropriately
On 04/11/2014 04:02 PM, K. Y. Srinivasan wrote:
diff --git a/arch/x86/kernel/i8259.c b/arch/x86/kernel/i8259.c index
2e977b5..0a57a19 100644
--- a/arch/x86/kernel/i8259.c
+++ b/arch/x86/kernel/i8259.c
@@ -299,11 +299,22 @@ static void unmask_8259A(void) static void
init_8259A(int auto_eoi) {
unsigned long flags;
+ unsigned char probe_val = 0xfb;
i8259A_auto_eoi = auto_eoi;
raw_spin_lock_irqsave(i8259A_lock, flags);
+ /* Check to see if we have a PIC */
+ outb(probe_val, PIC_MASTER_IMR);
+ probe_val = inb(PIC_MASTER_IMR);
+ if (probe_val == 0xff) {
+ printk(KERN_INFO Using NULL legacy PIC\n);
+ legacy_pic = null_legacy_pic;
+ raw_spin_unlock_irqrestore(i8259A_lock, flags);
+ return;
+ }
+
outb(0xff, PIC_MASTER_IMR); /* mask all of 8259A-1 */
outb(0xff, PIC_SLAVE_IMR); /* mask all of 8259A-2 */
I think it is important to document what this actually means here. 0xfb is
mask all except for the cascade.
Arguably, we should do this after masking the slave PIC.
On a whole other subject... the whole __byte() logic is a just plain horrific
piece of crap. Clearly whomever wrote it had never heard of a union.
I'm still horrifically confused, though, why we mask IRQ 2 (the cascade
interrupt) in i8259.c, but we explicitly do *not* mask IRQ 2 in boot/pm.c (nor
in the assembly code on which boot/pm.c was based.) In fact, it looks like we
never unmask it. I'm guessing that the masking status of the cascade input
simply doesn't matter.
At the very least, though, we shouldn't have 0xfb as a magic number, but use
~(1U PIC_CASCADE_IR).
I will get rid of this magic number as you have suggested.
K. Y
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf��^jǫy�m��@A�a���
0��h���i
