ron minnich [mailto:rminnich@gmail.com] wrote:
]Thanks, that's a great explanation. Generally, we've tried to avoid ]too much hardware setup in coreboot; that's the job of the kernel.
The PIC mode interrupt routing configuration must be done by BIOS because proprietary southbridge registers are used. The register definitions are not even consistent across different AMD southbridge models. An OEM BIOS does this configuration.
]I'm not really happy that we're doing all this PIC setup for one OS, ]written in assembly.
This is needed for all operating systems that support PIC mode. For example, Ubuntu 13.10 with boot option acpi=off fails. With the attached revised patch, it works.
]It's been quite some time since I've had to use PIC mode at all. ] ]Why can't the Kolibrios just use modern standards? And why all this ]effort for an OS written in assembly anyway? ] ]Unix v6 kernels were the same size as Kolibrios. They were written in ]C. That was 40 years ago. It's bizarre, to say the least, to be ]booting a kernel written in assembly from firmware written in C. ] ]Hence, I find it hard to believe that we want this patch. But I'm ]wrong a lot, so if I am here too, just let me know.
It is OK with me to take no action on the patch. The reason is that the patch is here in the mailing list archives, and anyone who really wants PIC mode will find it. The patch probably needs sanitization and additional testing anyway.
]ron
Patch revisions: 1) With the current code, CONFIG_GENERATE_ACPI_TABLES=1 causes the PCI interrupts to be omitted from the mptable. With the revised patch, the mptable always includes PCI interrupts. This solves the Ubuntu acpi=off problem of "can't find IRQ for PCI INT A; probably buggy MP table". 2) Add PIC mode PCI interrupt values for SATA and IDE devices. Without this, Ubuntu setup cannot read the CD-ROM. 3) Use 00 for unused PIC mode routing entries rather than 1F. This is for consistency with the APIC mode table.
Thanks, Scott
diff --git a/src/mainboard/asrock/e350m1/mptable.c b/src/mainboard/asrock/e350m1/mptable.c index 6444be5..f45385e 100644 --- a/src/mainboard/asrock/e350m1/mptable.c +++ b/src/mainboard/asrock/e350m1/mptable.c @@ -35,6 +35,7 @@ extern u32 apicid_sb800; extern u32 bus_type[256]; extern u32 sbdn_sb800;
+// SB800 interrupt routing register values: APIC mode u8 intr_data[] = { [0x00] = 0x10,0x11,0x12,0x13,0x14,0x15,0x16,0x17, /* INTA# - INTH# */ [0x08] = 0x00,0x00,0x00,0x00,0x1F,0x1F,0x1F,0x1F, /* Misc-nil,0,1,2, INT from Serial irq */ @@ -45,6 +46,20 @@ u8 intr_data[] = { 0x10,0x11,0x12,0x13 };
+// SB800 interrupt routing register values: PIC mode +u8 intr_data_pic[] = { + 0x0B, 0x0A, 0x0B, 0x0A, 0x1F, 0x1F, 0x1F, 0x1F, // 0x00 + 0x00, 0xF1, 0x00, 0x00, 0x1F, 0x1F, 0x1F, 0x1F, // 0x08 + 0x1F, 0x1F, 0x1F, 0x1F, 0x1F, 0x1F, 0x1F, 0x00, // 0x10 + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // 0x18 + 0x1F, 0x1F, 0x1F, 0x1F, 0x1F, 0x1F, 0x00, 0x00, // 0x20 + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // 0x28 + 0x0B, 0x0A, 0x0B, 0x0A, 0x0B, 0x0A, 0x0B, 0x00, // 0x30 + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // 0x38 + 0x0A, 0x0A, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // 0x40 + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // 0x48 + 0x0B, 0x0A, 0x0B, 0x0A}; // 0x50 + static void *smp_write_config_table(void *v) { struct mp_config_table *mc; @@ -75,6 +90,12 @@ static void *smp_write_config_table(void *v) outb(intr_data[byte], 0xC01); }
+ // program the SB800 PIC mode interrupt routing register values + for (byte = 0x0; byte < sizeof(intr_data_pic); byte ++) { + outb(byte, 0xC00); + outb(intr_data_pic[byte], 0xC01); + } + /* I/O Ints: Type Polarity Trigger Bus ID IRQ APIC ID PIN# */ #define IO_LOCAL_INT(type, intr, apicid, pin) \ smp_write_lintsrc(mc, (type), MP_IRQ_TRIGGER_EDGE | MP_IRQ_POLARITY_HIGH, bus_isa, (intr), (apicid), (pin)); @@ -84,12 +105,8 @@ static void *smp_write_config_table(void *v) /* PCI interrupts are level triggered, and are * associated with a specific bus/device/function tuple. */ -#if !CONFIG_GENERATE_ACPI_TABLES #define PCI_INT(bus, dev, fn, pin) \ - smp_write_intsrc(mc, mp_INT, MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, (bus), (((dev)<<2)|(fn)), apicid_sb800, (pin)) -#else -#define PCI_INT(bus, dev, fn, pin) -#endif + smp_write_intsrc(mc, mp_INT, MP_IRQ_TRIGGER_LEVEL|MP_IRQ_POLARITY_LOW, (bus), (((dev)<<2)|(fn)), apicid_sb800, (pin))
/* APU Internal Graphic Device*/ PCI_INT(0x0, 0x01, 0x0, intr_data[0x02]); @@ -149,6 +166,25 @@ static void *smp_write_config_table(void *v) IO_LOCAL_INT(mp_NMI, 0x0, MP_APIC_ALL, 0x1); /* There is no extension information... */
+ // program interrupt line registers for legacy OS use + pci_write_config32(dev_find_slot(0, PCI_DEVFN(0x01, 0)), 0x3C, 0x0B); + pci_write_config32(dev_find_slot(0, PCI_DEVFN(0x01, 1)), 0x3C, 0x0A); + pci_write_config32(dev_find_slot(0, PCI_DEVFN(0x04, 0)), 0x3C, 0x0B); + pci_write_config32(dev_find_slot(0, PCI_DEVFN(0x11, 0)), 0x3C, 0x0A); + pci_write_config32(dev_find_slot(0, PCI_DEVFN(0x12, 0)), 0x3C, 0x0B); + pci_write_config32(dev_find_slot(0, PCI_DEVFN(0x12, 2)), 0x3C, 0x0A); + pci_write_config32(dev_find_slot(0, PCI_DEVFN(0x13, 0)), 0x3C, 0x0B); + pci_write_config32(dev_find_slot(0, PCI_DEVFN(0x13, 2)), 0x3C, 0x0A); + pci_write_config32(dev_find_slot(0, PCI_DEVFN(0x14, 1)), 0x3C, 0x0A); + pci_write_config32(dev_find_slot(0, PCI_DEVFN(0x14, 2)), 0x3C, 0x0B); + pci_write_config32(dev_find_slot(0, PCI_DEVFN(0x14, 5)), 0x3C, 0x0B); + pci_write_config32(dev_find_slot(0, PCI_DEVFN(0x16, 0)), 0x3C, 0x0B); + pci_write_config32(dev_find_slot(0, PCI_DEVFN(0x16, 2)), 0x3C, 0x0A); + pci_write_config32(dev_find_slot(3, PCI_DEVFN(0x00, 0)), 0x3C, 0x0B); + + // program slave PIC edge-level control register + outb (0x0C, 0x4D1); + /* Compute the checksums */ return mptable_finalize(mc); }
ron minnich wrote:
I'm not really happy that we're doing all this PIC setup for one OS,
It's not for one OS, Ron. PIC setup is part of the PC legacy.
Please remember that coreboot is more than Chrome OS' firmware of choice.
It's been quite some time since I've had to use PIC mode at all.
Thanks to the efforts of Paul and Scott we were using it quite a bit at LinuxTag. KolibriOS booting from flash in a blink of an eye is an impressive demo. You should try it in qemu. It makes the coreboot point extremely clear, and was quite a valuable things to have.
Why can't the Kolibrios just use modern standards?
Please stay on point; PC firmware. Another way to explain what I mean is with a counter-question:
Why shouldn't coreboot do legacy initialization? What is the reason to be *less* compatible than possible?
It's bizarre, to say the least, to be booting a kernel written in assembly from firmware written in C.
I think it makes a lot of sense. It demonstrates how complexity has shifted from OS into firmware over the decades.
I find it hard to believe that we want this patch.
I want this patch and more. I want a data and API model within coreboot which allows common code for the three reporting mechanisms to call platform-specific code to program them automatically, when the respective tables have been enabled by Kconfig. They need to be enabled by default and perhaps hidden behind CONFIG_EXPERT.
Scott Duplichan wrote:
This is needed for all operating systems that support PIC mode. For example, Ubuntu 13.10 with boot option acpi=off fails. With the attached revised patch, it works.
To me it's a no-brainer to accept this patch and build on it. This is an important aspect where coreboot does a much worse job than it could, for no real reason.
It is OK with me to take no action on the patch.
I disagree. I think this is a very important patch. Thank you.
the patch is here in the mailing list archives, and anyone who really wants PIC mode will find it. The patch probably needs sanitization and additional testing anyway.
I would love for this patch to be pushed to Gerrit. Scott, can you do that easily, or would you prefer if I do it?
I want you as the author to give a +2 before it is submitted. If this policy hasn't already been implemented then a workaround is for you to give a -2 immediately after pushing the commit.
Thank you!
//Peter
On Mon, May 12, 2014 at 09:13:26PM +0200, Peter Stuge wrote:
ron minnich wrote:
I'm not really happy that we're doing all this PIC setup for one OS,
It's not for one OS, Ron. PIC setup is part of the PC legacy.
Please remember that coreboot is more than Chrome OS' firmware of choice.
It's been quite some time since I've had to use PIC mode at all.
Thanks to the efforts of Paul and Scott we were using it quite a bit at LinuxTag. KolibriOS booting from flash in a blink of an eye is an impressive demo. You should try it in qemu. It makes the coreboot point extremely clear, and was quite a valuable things to have.
I can can confirm that it was an impressive demo for our visitors! Thanks to Scott and Paul also from me. I think this patch should go in.
Regards, Björn
On Mon, May 12, 2014 at 12:13 PM, Peter Stuge peter@stuge.se wrote:
Why shouldn't coreboot do legacy initialization? What is the reason to be *less* compatible than possible?
The main question I had was whether enabling this set of interrupts could negatively impact other payloads. The goal of linuxbios and coreboot was always to do as little as possible, not act like a BIOS.
Simple example: if we enable all these interrupts, and a non-kolibrios payload boots, is there a chance that a broadcast packet could be picked up by the NIC and interrupt the non-kolibrios payload? Is there anything in there that is a one-way initialization that might make it harder for for _MP_, ACPI, MSI, or MSI-X?
I just want to hear that the answer is "no". I have yet to hear it. It's a pretty simple question.
And the question remains: why is it kolibrios can't just read the $PIR and/or _MP_ like everything else has somehow managed to do for 15 years? Why are we doing these config writes in coreboot when the OS should do them? And why are we doing this for *one* OS?
Especially given this grotesque example:
The NIC bus number is hard-coded at the moment. This needs fixing if the NIC bus number can change.
Are you seriously telling me you want coreboot to hardware the bus number for a NIC? That's a terrible idea.
In general, we've tended not to set up too much interrupt hardware for a simple reason: we do have lots of payloads, and the odds are very good if you do too much setup - you're wasting time - you're doing the wrong thing for the payload - it may confuse the payload you eventually boot.
So, I'd like to hear the answer to my first question.
Finally, we actually always tried from the beginning to no setup up IRQs. The emphasis was on creating the tables that let the payload do the right thing.
Communicating IRQ info to the kernel is what we've done; actually configuring the PIC is a violation of the early design goals.
ron