This is a first example of how to build (e.g.) an smm blob.
ron
ron minnich wrote:
+++ arch/x86/Makefile (working copy) @@ -48,6 +48,13 @@ COMPRESSFLAG := -C nrv2b endif
+# all depends on coreboot.rom and BLOBS. +# BLOBS may be empty. They are things such as smm that are searched for at runtime. +# They depend on the coreboot rom existing so that they can be placed there. +# The name of the blob is determined by the various blob rules. There is so much possible +# variation in them that it is not really possible to put a standard rule in this file. +rom:: $(obj)/coreboot.rom $(BLOBS)
$(obj)/coreboot.rom $(obj)/coreboot.map: $(obj)/coreboot.bootblock $(obj)/util/lar/lar lzma nrv2b $(obj)/coreboot.initram $(obj)/coreboot.stage2 $(obj)/option_table $(Q)printf " LAR $(subst $(shell pwd)/,,$(@))\n" $(Q)rm -f $(obj)/coreboot.rom @@ -306,4 +313,4 @@ $(Q)cp cscope.proj $(obj)/mainboard/$(MAINBOARDDIR)/kscope $(Q)sh util/mkdep $@ $(INITCFLAGS) "--" $(ALLSRC) # $(Q)sort -u -o $@ $@
+.PHONY: rom
I don't like to add PHONY targets, nor special purpose toplevel dependencies. Could the boards that need them simply depend on smm.elf?
I remember discussing blobs being sort-of stage3/4. Do we call them that instead?
//Peter
On Wed, Nov 26, 2008 at 2:19 PM, Peter Stuge peter@stuge.se wrote:
ron minnich wrote:
+++ arch/x86/Makefile (working copy) @@ -48,6 +48,13 @@ COMPRESSFLAG := -C nrv2b endif
+# all depends on coreboot.rom and BLOBS. +# BLOBS may be empty. They are things such as smm that are searched for at runtime. +# They depend on the coreboot rom existing so that they can be placed there. +# The name of the blob is determined by the various blob rules. There is so much possible +# variation in them that it is not really possible to put a standard rule in this file. +rom:: $(obj)/coreboot.rom $(BLOBS)
$(obj)/coreboot.rom $(obj)/coreboot.map: $(obj)/coreboot.bootblock $(obj)/util/lar/lar lzma nrv2b $(obj)/coreboot.initram $(obj)/coreboot.stage2 $(obj)/option_table $(Q)printf " LAR $(subst $(shell pwd)/,,$(@))\n" $(Q)rm -f $(obj)/coreboot.rom @@ -306,4 +313,4 @@ $(Q)cp cscope.proj $(obj)/mainboard/$(MAINBOARDDIR)/kscope $(Q)sh util/mkdep $@ $(INITCFLAGS) "--" $(ALLSRC) # $(Q)sort -u -o $@ $@
+.PHONY: rom
I don't like to add PHONY targets, nor special purpose toplevel dependencies. Could the boards that need them simply depend on smm.elf?
we have a bunch already. I don't see a big deal with rom as the target. I can take the PHONY out however.
I remember discussing blobs being sort-of stage3/4. Do we call them that instead?
no, that makes no sense. Blobs are all over. VSA blob is in stage1. SMM is installed in stage2 phase 6. They are not classifiable.
ron
ron minnich wrote:
I remember discussing blobs being sort-of stage3/4. Do we call them that instead?
no, that makes no sense. Blobs are all over. VSA blob is in stage1. SMM is installed in stage2 phase 6. They are not classifiable.
I don't particularly get excited about the name "blob", either. In the lar, everything is a blob, unless we pack a text file in there. Can we find a better name?
3rd-party?
smm should just be in the main "directory" it's not more a blob than stage2 is.
On Wed, Nov 26, 2008 at 3:49 PM, Stefan Reinauer stepan@coresystems.de wrote:
smm should just be in the main "directory" it's not more a blob than stage2 is.
I'm not particular on the name. I tihnk you can use the makeifle to build the smm.elf, and then it's a few lines of makefile foo to get the names that way you all would like them.
We should remove the 'cp coreboot.rom bios.bin' in the makefile btw. make bios.bin a target or yank it.
Anyway, I could use some help wrapping this up, so if anyone can beat on it, that would be helpful.
thanks
ron
On 27.11.2008 00:51, ron minnich wrote:
We should remove the 'cp coreboot.rom bios.bin' in the makefile btw. make bios.bin a target or yank it.
That's a helper for qemu. Basically, qemu expects bios.bin, and the copy operation reduces the amount of stuff you have to do before you can boot Qemu with the new ROM. I have to admit it does not make sense for anything besides Qemu. Making bios.bin a separate target would have the Qemu target diverge from all other targets. And "make bios.bin" is really not much an improvement over "make; cp build/coreboot.rom build/bios.bin".
I'm open to removing it completely, but I don't feel strongly for either removal or unchanged keeping as long as invoking the build process for Qemu doesn't diverge.
Tell me what you want and I'll cook up a patch.
Regards, Carl-Daniel
On Thu, Nov 27, 2008 at 7:25 PM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Tell me what you want and I'll cook up a patch.
let's let it go. I put too many things in that patch. My bad.
ron
On 26.11.2008 23:41, ron minnich wrote:
On Wed, Nov 26, 2008 at 2:19 PM, Peter Stuge peter@stuge.se wrote:
ron minnich wrote:
+++ arch/x86/Makefile (working copy) @@ -48,6 +48,13 @@ COMPRESSFLAG := -C nrv2b endif
+# all depends on coreboot.rom and BLOBS. +# BLOBS may be empty. They are things such as smm that are searched for at runtime. +# They depend on the coreboot rom existing so that they can be placed there. +# The name of the blob is determined by the various blob rules. There is so much possible +# variation in them that it is not really possible to put a standard rule in this file. +rom:: $(obj)/coreboot.rom $(BLOBS)
$(obj)/coreboot.rom $(obj)/coreboot.map: $(obj)/coreboot.bootblock $(obj)/util/lar/lar lzma nrv2b $(obj)/coreboot.initram $(obj)/coreboot.stage2 $(obj)/option_table $(Q)printf " LAR $(subst $(shell pwd)/,,$(@))\n" $(Q)rm -f $(obj)/coreboot.rom @@ -306,4 +313,4 @@ $(Q)cp cscope.proj $(obj)/mainboard/$(MAINBOARDDIR)/kscope $(Q)sh util/mkdep $@ $(INITCFLAGS) "--" $(ALLSRC) # $(Q)sort -u -o $@ $@
+.PHONY: rom
I don't like to add PHONY targets, nor special purpose toplevel dependencies. Could the boards that need them simply depend on smm.elf?
we have a bunch already. I don't see a big deal with rom as the target. I can take the PHONY out however.
I remember discussing blobs being sort-of stage3/4. Do we call them that instead?
no, that makes no sense. Blobs are all over. VSA blob is in stage1. SMM is installed in stage2 phase 6. They are not classifiable.
Do we want fallback/normal SMM? From a BIOS update POV, it would make sense.
Regards, Carl-Daniel
On Wed, Nov 26, 2008 at 1:49 PM, ron minnich rminnich@gmail.com wrote:
This is a first example of how to build (e.g.) an smm blob.
I just verified that kontron boots with these changes.
I am going radio silent very soon, have to work on blue gene/p next week, so somebody will have to pick this up (or kill it) if we don't get it done today.
ron
On 26.11.2008 22:49, ron minnich wrote:
Index: arch/x86/intel/core2/stage1.c
--- arch/x86/intel/core2/stage1.c (revision 1059) +++ arch/x86/intel/core2/stage1.c (working copy) @@ -2,6 +2,7 @@
- This file is part of the coreboot project.
- Copyright (C) 2008 Carl-Daniel Hailfinger
- Copyright (C) 2008 coresystems GmbH
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
@@ -29,30 +30,23 @@ #include <string.h> #include <mtrr.h>
-/**
- Disable Cache As RAM (CAR) after memory is setup.
- */
void disable_car(void) {
- printk(BIOS_DEBUG, "disable_car entry\n");
- /* Determine new global variable location. Stack organization from top
* Top 4 bytes are reserved* Pointer to global variables* Global variables** Align the result to 8 bytes*/- struct global_vars *const newlocation = (struct global_vars *)((RAM_STACK_BASE - sizeof(struct global_vars *) - sizeof(struct global_vars)) & ~0x7);
- /* Copy global variables to new location. */
- memcpy(newlocation, global_vars(), sizeof(struct global_vars));
- printk(BIOS_DEBUG, "disable_car global_vars copy done\n");
- /* Set the new global variable pointer. */
- *(struct global_vars **)(RAM_STACK_BASE - sizeof(struct global_vars *)) = newlocation;
- struct global_vars *const new_globals =
(struct global_vars *)((RAM_STACK_BASE - sizeof(struct global_vars *) - sizeof(struct global_vars)) & ~0x7);
- printk(BIOS_DEBUG, "disable_car global_vars pointer adjusted\n");
- printk(BIOS_DEBUG, "entering asm code now\n");
/* Copy global variables */
memcpy(new_globals, global_vars(), sizeof(struct global_vars));
/* Update global variable pointer */
*(struct global_vars **)(RAM_STACK_BASE - sizeof(struct global_vars *))
= new_globals;printk(BIOS_DEBUG, "Disabling cache-as-ram\n");
__asm__ __volatile__(
+"movb $0x30, %%al\noutb %%al, $0x80\n"
" movl %[newesp], %%esp \n"
/* We don't need cache as ram for now on */
@@ -61,6 +55,8 @@ " orl $(0x1<<30),%%eax \n" " movl %%eax, %%cr0 \n"
+"movb $0x31, %%al\noutb %%al, $0x80\n"
- /* disable fixed mtrr from now on, it will be enabled by coreboot_ram again*/ /* clear sth */ " xorl %%eax, %%eax \n"
@@ -70,6 +66,7 @@ " movl $0x200, %%ecx \n" " wrmsr \n"
+"movb $0x32, %%al\noutb %%al, $0x80\n" /* Set the default memory type and disable fixed and enable variable MTRRs */ " movl %[_MTRRdefType_MSR], %%ecx \n" " xorl %%edx, %%edx \n" @@ -77,17 +74,22 @@ " movl $0x00000800, %%eax \n" " wrmsr \n"
+"movb $0x33, %%al\noutb %%al, $0x80\n" /* enable cache */ " movl %%cr0, %%eax \n" " andl $0x9fffffff,%%eax \n" " movl %%eax, %%cr0 \n"
- " wbinvd \n"
+"movb $0x34, %%al\noutb %%al, $0x80\n"
- " invd \n"
+"movb $0x35, %%al\noutb %%al, $0x80\n" " call stage1_phase3 \n"
- :: [newesp] "i" (newlocation),
- :: [newesp] "i" (new_globals), [_MTRRdefType_MSR] "i" (MTRRdefType_MSR) : "memory");
- printk(BIOS_DEBUG, "Something went wrong. Still here.\n");
}
void stop_ap(void)
That part of the patch is really funny. It rewrites a few comments and printk messages (no effect on execution), renames one variable and changes some whitespace. A few POST codes are added for good measure.
The only _functional_ change (except cosmetics and debugging) is the change from wbinvd to invd.
May I suggest to drop all arch/x86/intel/core2/stage1.c changes except the one below (and this wbinvd->invd change should have no effect according to the scarce publicly available docs).
Index: arch/x86/intel/core2/stage1.c =================================================================== --- arch/x86/intel/core2/stage1.c (Revision 1061) +++ arch/x86/intel/core2/stage1.c (Arbeitskopie) @@ -82,7 +82,7 @@ " andl $0x9fffffff,%%eax \n" " movl %%eax, %%cr0 \n"
- " wbinvd \n" + " invd \n"
" call stage1_phase3 \n" :: [newesp] "i" (newlocation),
This keeps the changes to a minium (and Stefan opposed changing whitespace, printk and comments just for the sake of it).
If the machine boots even without the change, we could drop it altogether. Having disable_car implementations diverge without reasons enforced by the hardware is suboptimal.
Thanks.
Regards, Carl-Daniel
Carl-Daniel Hailfinger wrote:
This keeps the changes to a minium (and Stefan opposed changing whitespace, printk and comments just for the sake of it).
It's ok to change whitespace when you touch code and clean it up. It's not ok to do indent orgies.
If the machine boots even without the change, we could drop it altogether.
It does not, hence the change.
-
Carl-Daniel Hailfinger -
Peter Stuge -
ron minnich -
Stefan Reinauer