January 2010 - SeaBIOS - mail.coreboot.org

Bochs cvs + SeaBIOS git master + FreeDOS = slow load + invalid opcode
by Sebastian Herbszt May 16, 2010

May 16, 2010

I have tried SeaBIOS 0.5.1 on Bochs cvs with FreeDOS 1.0 Final boot floppy. Loading from floppy was utterly slow and then FreeDOS dumped Invalid Opcode at 0013 0000 0202 800F 01F3 20F4 10AA 10AA 109A 0000 0000 0000 00 00 Bad or missing Command Interpreter: A:\COMMAND.COM A:\ /E:2048 /F /MSG /P=A:\FRE EDOS\FDAUTO.BAT Enter the full shell command line: Bochs log has 00477945139i[BIOS ] Booting from Floppy... 00510608951i[BIOS ] Booting from 0000:7c00 05264161428i[FDD ] controller reset in software 05445136148i[FDD ] controller reset in software 05625186768i[FDD ] controller reset in software 05803824012i[FDD ] controller reset in software 05985259728i[FDD ] controller reset in software 05985285461i[CPU0 ] LOCK prefix unallowed (op1=0x53, attr=0x0, mod=0x0, nnn=0) Same floppy and Bochs version work fine and fast with Bochs BIOS. Any clues? - Sebastian

3 14

Re: [SeaBIOS] [PATCH] Seabios - read e820 reserve from qemu_cfg
by Kevin O'Connor Feb. 14, 2010

Feb. 14, 2010

On Mon, Jan 25, 2010 at 05:46:42PM +0100, Jes Sorensen wrote: > Hi, > > Right now KVM/QEMU relies on hard coded values in Seabios for the > reserved area for the TSS pages and the EPT page. > > I'd like to suggest we change this to pass the value from QEMU via > qemu-cfg making it possible to move it around dynamically in the future. > > Attached is a patch to Seabios for this, which defaults to the current > hard coded value if no value is provided by qemu-cfg. We can remove > the backwards compatibility later. > > I'll post the QEMU patches for upstream QEMU and QEMU-KVM in a minute. > > Comments most welcome! I like the idea, but I think it would be better to pass a list of e820 entries explicitly. That is, pass an array of: struct e820entry { u64 start; u64 size; u32 type; }; where 'type' uses the standard e820 definitions. That way, SeaBIOS can just walk through the list and add them to its e820 map. BTW, this is what SeaBIOS does when running under coreboot (coreboot passes a memory map as part of the "coreboot tables"). SeaBIOS is already smart enough to not use any high memory addresses marked as reserved. -Kevin

4 9

Re: [SeaBIOS] [PATCH] Seabios: Fix PkgLength calculation for the SSDT.
by Kevin O'Connor Feb. 9, 2010

Feb. 9, 2010

On Mon, Dec 14, 2009 at 10:25:14AM +0100, Magnus Christensson wrote: >>> --- a/src/acpi.c >>> +++ b/src/acpi.c >>> @@ -464,10 +464,12 @@ build_ssdt(void) >>> // build processor scope header >>> *(ssdt_ptr++) = 0x10; // ScopeOp >>> if (cpu_length<= 0x3e) { >>> + /* Handle 1-4 CPUs with one byte encoding */ >>> *(ssdt_ptr++) = cpu_length + 1; >>> } else { >>> - *(ssdt_ptr++) = 0x7F; >>> - *(ssdt_ptr++) = (cpu_length + 2)>> 6; >>> + /* Handle 5-314 CPUs with two byte encoding */ >>> + *(ssdt_ptr++) = 0x40 | ((cpu_length + 1)& 0xf); >>> + *(ssdt_ptr++) = (cpu_length + 1)>> 4; >>> >> Should be cpu_length + 2 as far as I can tell. The current code is >> definitely broken. >> > Right. That should be cpu_length +2 in the else-part. Can you resend the patch with the change? Thanks, -Kevin

4 5

Re: [SeaBIOS] [PATCH] vcpu hotplug: Setup vcpu add/remove infrastructure
by Kevin O'Connor Feb. 1, 2010

Feb. 1, 2010

On Sat, Jan 30, 2010 at 11:10:43PM +0800, Jiang, Yunhong wrote: > For how to setup the memory as NVS, you can refer to src/memmap.c, > maybe using add_e820(). Adding to the e820 is simple. Just align the madt to 4k: madt_size = ALIGN(madt_size, PAGE_SIZE); struct multiple_apic_table *madt = memalign_high(PAGE_SIZE, madt_size); and then add it to the e820: add_e820((u64)madt, madt_size, E820_NVS); It's not strictly necessary to align to 4k, but I think it makes sense to do that. -Kevin

2 1

Re: [SeaBIOS] [PATCH] vcpu hotplug: Setup vcpu add/remove infrastructure
by Gleb Natapov Jan. 30, 2010

Jan. 30, 2010

On Sat, Jan 30, 2010 at 04:14:29AM +0800, Liu, Jinsong wrote: > Gleb Natapov wrote: > >> > > OK. Let me try to explain it one more time. Currently MADT table > > resides in memory of type "ACPI DATA" and AML code reads and writes > > this memory during CPU hot-plug. According to ACPI spec "ACPI DATA" > > ares can be reused by an OS, so consider what will happen if OS puts > > its data into memory area where MADT was and later gets cpu hotplug > > event. Do you see the problem now? > > > > I just hesitate ' if OS puts its data ...'. What is not clear here? OS is free to reuse this memory for whatever it wants. > Per my understanding, os will not write madt after booting, only AML write madt when necessary (i.e. cpu hotplug). Your understanding is wrong. Read spec. > So is it necessary to mark madt as ACPI NVS? > ACPI VNS or reserved. The difference is that OS should save/restore ACPI NVS memory during S4 transition. Read the spec it is all there. -- Gleb.

2 1

Re: [SeaBIOS] Pci express passthough in KVM
by Kevin O'Connor Jan. 29, 2010

Jan. 29, 2010

On Thu, Jan 28, 2010 at 05:18:00PM -0500, Jean-Christophe Ducom wrote: > Dear Kevin- > Sorry to bother you but I have a quick question. > I am trying to attach a pci express Infiniband card to a guest VM via > KVM (Fedora Core12 version kvm/qemu still uses pcbios). > Unfortunately it fails. I suspect (please correct me if I'm wrong) > it's due to the lack of PCIexpress support in the bios as mentioned > in the talk of Isaku Yamahata and Akio Takebe during last Xen > summit. > If so, would seabios fix it? > Thank you for your time > Jean-Christophe Ducom I don't know. I think you need to ask the KVM folks. -Kevin

3 2

Re: [SeaBIOS] [PATCH] vcpu hotplug: Setup vcpu add/remove infrastructure
by Gleb Natapov Jan. 29, 2010

Jan. 29, 2010

On Sat, Jan 30, 2010 at 03:22:59AM +0800, Liu, Jinsong wrote: > >> You can update vcpu_hotplug_1.patch per your understanding. > >> > > I am not going to. Just not including it into seabios is my advice > > if submitter doesn't want to address issues and read specs. I want > > proper CPU hotplug support in seabios that works for Windows too, > > doesn't break MS SVVP test and support arbitrary number of CPUs. The > > code you ported from BOCHS fails on all accounts. I am OK with > > including it as temporary solution if you address at least easy to > > address issues, but you refuse to do even that. > > Seems you don't like to update my patch, it doesn't matter. > Currently issue is, through discussion, I didn't quite understand what the 'ACPI NVS issue' is and what for. > You may meet this issue before, but you didn't speak clear to make me understand. > OK. Let me try to explain it one more time. Currently MADT table resides in memory of type "ACPI DATA" and AML code reads and writes this memory during CPU hot-plug. According to ACPI spec "ACPI DATA" ares can be reused by an OS, so consider what will happen if OS puts its data into memory area where MADT was and later gets cpu hotplug event. Do you see the problem now? > I will go through ACPI sepc 4.0 to see if there are some clue. > If I find what I think 'ACPI NVS issue' is, I will confirm with you to prevent any misunderstanding. > Based on I correctly understand what you say 'ACPI NVS issue', I will then update my patch. > This is my solution to 'ACPI NVS issue'. > Thanks. -- Gleb.

2 1

Re: [SeaBIOS] [PATCH] vcpu hotplug: Setup vcpu add/remove infrastructure
by Gleb Natapov Jan. 29, 2010

Jan. 29, 2010

On Sat, Jan 30, 2010 at 02:16:11AM +0800, Liu, Jinsong wrote: > Gleb Natapov wrote: > > On Sat, Jan 30, 2010 at 01:42:49AM +0800, Liu, Jinsong wrote: > >> Gleb Natapov wrote: > >>> On Sat, Jan 30, 2010 at 01:05:04AM +0800, Liu, Jinsong wrote: > >>>> Gleb Natapov wrote: > >>>>> On Sat, Jan 30, 2010 at 12:52:54AM +0800, Liu, Jinsong wrote: > >>>>>> Gleb Natapov wrote: > >>>>>>> On Fri, Jan 29, 2010 at 11:34:24PM +0800, Liu, Jinsong wrote: > >>>>>>>> Gleb Natapov wrote: > >>>>>>>>> On Thu, Jan 28, 2010 at 10:54:48PM +0800, Liu, Jinsong wrote: > >>>>>>>>>> Connor and Gleb, > >>>>>>>>>> > >>>>>>>>>> I updated my patch according to our discussion: > >>>>>>>>>> 1. simplify scan loop according to 'maxvcpus'; > >>>>>>>>>> 2. remove unecessary global variables; > >>>>>>>>>> 3. change hardcode address to bios use only area '0x514'; > >>>>>>>>>> 4. remove simple \_PR scope from ssdt; > >>>>>>>>>> > >>>>>>>>> I still don't see that ACPI NVS issue is addressed. > >>>>>>>> > >>>>>>>> Gleb, > >>>>>>>> > >>>>>>>> I'm not quite sure Win7 BSOD issue you mentioned last email, > >>>>>>>> anyway I update my patch, define ACPI NVS per my understanding. > >>>>>>> You need checked build of Windows 7. This is not regular Win7 > >>>>>>> this is debug version that you can download from MSDN. Here is > >>>>>>> the bug report about the crash: > >>>>>>> http://sourceforge.net/tracker/?func=detail&aid=2902983&group_id=180599&ati… > >>>>>>> > >>>>>>> Anyway according to ACPI spec ACPI DATA area can be reused by an > >>>>>>> OS after reading ACPI tables so it is wrong to access this > >>>>>>> memory after an OS boot. > >>>>>>> > >>>>>> > >>>>>> Yes, it's safer to define ACPI NVS, although it's bios use only > >>>>>> area. > >>>>>> > >>>>> APCI DATA is explicitly not defined by ACPI spec as "bios use only > >>>>> area". > >>>> > >>>> Yes, it's not defined by ACPI spec, but from default x86 memory > >>>> layout, refer to Documentation/x86/boot.txt: 001000 > >>>> +------------------------+ | Reserved for MBR/BIOS | > >>>> 000800 +------------------------+ > >>>> | Typically used by MBR | > >>>> 000600 +------------------------+ > >>>> | BIOS use only | > >>>> 000000 +------------------------+ > >>>> > >>>> > >>> Only now I actually looked at you patch at it is wrong. You don't > >>> need to mark 0x512 as NVS for reason you are stating above. You > >>> need to put MADT table into ACPI NVS and that is not what your > >>> patch is doing. > >> > >> Do you mean ACPI table like MADT may be destroyed by os, so it need > >> explicitly marked as ACPI NVS ? > > Yes. That is what spec says. But we don't what all ACPI tables to be > > in ACPI NVS only those which are accessed from AML code and for now > > it's only MADT. > > I hesitate what you say. AML code access MADT is normal access, so why need marked ACPI NVS? to prevent who access MADT or avoid what? I can't parse that. > > In fact, I cannot get the conclusion ACPI NVS cause win7 build BSOD from bug report > http://sourceforge.net/tracker/?func=detail&aid=2902983&group_id=180599&ati… > it only change cmdline and then installation works OK. > If you can't get this conclusion you'll have to believe me. > I still not quite clear your ACPI NVS issue. > I remove vcpu_hotplug_2.patch. > leave vcpu_hotplug_1.pach, and it was tested linux guestos and win7 guestos, all work OK. Have you tested checked build installation of win7 like bug report says? > You can update vcpu_hotplug_1.patch per your understanding. > I am not going to. Just not including it into seabios is my advice if submitter doesn't want to address issues and read specs. I want proper CPU hotplug support in seabios that works for Windows too, doesn't break MS SVVP test and support arbitrary number of CPUs. The code you ported from BOCHS fails on all accounts. I am OK with including it as temporary solution if you address at least easy to address issues, but you refuse to do even that. -- Gleb.

2 1

Re: [SeaBIOS] [PATCH] vcpu hotplug: Setup vcpu add/remove infrastructure
by Gleb Natapov Jan. 29, 2010

Jan. 29, 2010

On Sat, Jan 30, 2010 at 01:42:49AM +0800, Liu, Jinsong wrote: > Gleb Natapov wrote: > > On Sat, Jan 30, 2010 at 01:05:04AM +0800, Liu, Jinsong wrote: > >> Gleb Natapov wrote: > >>> On Sat, Jan 30, 2010 at 12:52:54AM +0800, Liu, Jinsong wrote: > >>>> Gleb Natapov wrote: > >>>>> On Fri, Jan 29, 2010 at 11:34:24PM +0800, Liu, Jinsong wrote: > >>>>>> Gleb Natapov wrote: > >>>>>>> On Thu, Jan 28, 2010 at 10:54:48PM +0800, Liu, Jinsong wrote: > >>>>>>>> Connor and Gleb, > >>>>>>>> > >>>>>>>> I updated my patch according to our discussion: > >>>>>>>> 1. simplify scan loop according to 'maxvcpus'; > >>>>>>>> 2. remove unecessary global variables; > >>>>>>>> 3. change hardcode address to bios use only area '0x514'; > >>>>>>>> 4. remove simple \_PR scope from ssdt; > >>>>>>>> > >>>>>>> I still don't see that ACPI NVS issue is addressed. > >>>>>> > >>>>>> Gleb, > >>>>>> > >>>>>> I'm not quite sure Win7 BSOD issue you mentioned last email, > >>>>>> anyway I update my patch, define ACPI NVS per my understanding. > >>>>> You need checked build of Windows 7. This is not regular Win7 this > >>>>> is debug version that you can download from MSDN. Here is the bug > >>>>> report about the crash: > >>>>> http://sourceforge.net/tracker/?func=detail&aid=2902983&group_id=180599&ati… > >>>>> > >>>>> Anyway according to ACPI spec ACPI DATA area can be reused by an > >>>>> OS after reading ACPI tables so it is wrong to access this memory > >>>>> after an OS boot. > >>>>> > >>>> > >>>> Yes, it's safer to define ACPI NVS, although it's bios use only > >>>> area. > >>>> > >>> APCI DATA is explicitly not defined by ACPI spec as "bios use only > >>> area". > >> > >> Yes, it's not defined by ACPI spec, but from default x86 memory > >> layout, refer to Documentation/x86/boot.txt: 001000 > >> +------------------------+ | Reserved for MBR/BIOS | > >> 000800 +------------------------+ > >> | Typically used by MBR | > >> 000600 +------------------------+ > >> | BIOS use only | > >> 000000 +------------------------+ > >> > >> > > Only now I actually looked at you patch at it is wrong. You don't need > > to mark 0x512 as NVS for reason you are stating above. You need to put > > MADT table into ACPI NVS and that is not what your patch is doing. > > Do you mean ACPI table like MADT may be destroyed by os, so it need explicitly marked as ACPI NVS ? Yes. That is what spec says. But we don't what all ACPI tables to be in ACPI NVS only those which are accessed from AML code and for now it's only MADT. -- Gleb.

2 1

Re: [SeaBIOS] [PATCH] vcpu hotplug: Setup vcpu add/remove infrastructure
by Gleb Natapov Jan. 29, 2010

Jan. 29, 2010

On Sat, Jan 30, 2010 at 01:05:04AM +0800, Liu, Jinsong wrote: > Gleb Natapov wrote: > > On Sat, Jan 30, 2010 at 12:52:54AM +0800, Liu, Jinsong wrote: > >> Gleb Natapov wrote: > >>> On Fri, Jan 29, 2010 at 11:34:24PM +0800, Liu, Jinsong wrote: > >>>> Gleb Natapov wrote: > >>>>> On Thu, Jan 28, 2010 at 10:54:48PM +0800, Liu, Jinsong wrote: > >>>>>> Connor and Gleb, > >>>>>> > >>>>>> I updated my patch according to our discussion: > >>>>>> 1. simplify scan loop according to 'maxvcpus'; > >>>>>> 2. remove unecessary global variables; > >>>>>> 3. change hardcode address to bios use only area '0x514'; > >>>>>> 4. remove simple \_PR scope from ssdt; > >>>>>> > >>>>> I still don't see that ACPI NVS issue is addressed. > >>>> > >>>> Gleb, > >>>> > >>>> I'm not quite sure Win7 BSOD issue you mentioned last email, anyway > >>>> I update my patch, define ACPI NVS per my understanding. > >>> You need checked build of Windows 7. This is not regular Win7 this > >>> is debug version that you can download from MSDN. Here is the bug > >>> report about the crash: > >>> http://sourceforge.net/tracker/?func=detail&aid=2902983&group_id=180599&ati… > >>> > >>> Anyway according to ACPI spec ACPI DATA area can be reused by an OS > >>> after reading ACPI tables so it is wrong to access this memory > >>> after an OS boot. > >>> > >> > >> Yes, it's safer to define ACPI NVS, although it's bios use only area. > >> > > APCI DATA is explicitly not defined by ACPI spec as "bios use only > > area". > > Yes, it's not defined by ACPI spec, but from default x86 memory layout, refer to Documentation/x86/boot.txt: > 001000 +------------------------+ > | Reserved for MBR/BIOS | > 000800 +------------------------+ > | Typically used by MBR | > 000600 +------------------------+ > | BIOS use only | > 000000 +------------------------+ > > Only now I actually looked at you patch at it is wrong. You don't need to mark 0x512 as NVS for reason you are stating above. You need to put MADT table into ACPI NVS and that is not what your patch is doing. -- Gleb.

2 1