LAR currently overwrites the top 13 bytes (0xfffff3-0xffffff) of the bootblock with zeros, then it stores the ROM size in 0xfffff4-0xfffff7. The top 8 bytes are unused in that scheme. Leave the top 8 bytes as they are and allow us to store something in there during the bootblock build process. The byte at 0xfffff3 is 0xff by default on x86 and setting it to 0 is not needed.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: corebootv3-lar_dont_erase_top_8_bytes/util/lar/stream.c =================================================================== --- corebootv3-lar_dont_erase_top_8_bytes/util/lar/stream.c (Revision 1061) +++ corebootv3-lar_dont_erase_top_8_bytes/util/lar/stream.c (Arbeitskopie) @@ -245,8 +245,9 @@ }
/** - * Add the LAR archive size to the bootblock, and clean up some other params - * in what we're loosely calling the "bootblockh header" + * Add the LAR archive size to the bootblock at 0xfffffff4 + * in what we're loosely calling the "bootblock header". + * Leave anything else alone. * @param ptr Pointer to the start of the bootblock * @param size The size value to write to the bootblock header */ @@ -255,7 +256,6 @@ int i; u32 *p;
- memset(ptr + (BOOTBLOCK_SIZE - 13), 0, 13); p = (u32 *) (ptr + BOOTBLOCK_SIZE - 12); p[0] = size; }
Carl-Daniel Hailfinger wrote:
LAR currently overwrites the top 13 bytes (0xfffff3-0xffffff) of the bootblock with zeros, then it stores the ROM size in 0xfffff4-0xfffff7. The top 8 bytes are unused in that scheme. Leave the top 8 bytes as they are and allow us to store something in there during the bootblock build process. The byte at 0xfffff3 is 0xff by default on x86 and setting it to 0 is not needed.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Nack.
They're unused, and until they are used, they should be cleared to a consistent state.
On 03.12.2008 12:48, Stefan Reinauer wrote:
Carl-Daniel Hailfinger wrote:
LAR currently overwrites the top 13 bytes (0xfffff3-0xffffff) of the bootblock with zeros, then it stores the ROM size in 0xfffff4-0xfffff7. The top 8 bytes are unused in that scheme. Leave the top 8 bytes as they are and allow us to store something in there during the bootblock build process. The byte at 0xfffff3 is 0xff by default on x86 and setting it to 0 is not needed.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Nack.
They're unused, and until they are used, they should be cleared to a consistent state.
They are already cleared by arch/x86/stage0_common.S. Having to clear them in the LAR would mean the code in stage0_common.S is crap and needs to be fixed.
Regards, Carl-Daniel
Carl-Daniel Hailfinger wrote:
On 03.12.2008 12:48, Stefan Reinauer wrote:
Carl-Daniel Hailfinger wrote:
LAR currently overwrites the top 13 bytes (0xfffff3-0xffffff) of the bootblock with zeros, then it stores the ROM size in 0xfffff4-0xfffff7. The top 8 bytes are unused in that scheme. Leave the top 8 bytes as they are and allow us to store something in there during the bootblock build process. The byte at 0xfffff3 is 0xff by default on x86 and setting it to 0 is not needed.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Nack.
They're unused, and until they are used, they should be cleared to a consistent state.
They are already cleared by arch/x86/stage0_common.S. Having to clear them in the LAR would mean the code in stage0_common.S is crap and needs to be fixed.
Not necessarily. This is a userland utility, so there is no real penalty for making extra sure that the area is in a valid state. LAR shouldn't depend on the v3 code to do the right thing, nor should the v3 code depend on LAR to do the right thing.
Jordan
On 03.12.2008, at 19:00, Jordan Crouse jordan@cosmicpenguin.net wrote:
Carl-Daniel Hailfinger wrote:
On 03.12.2008 12:48, Stefan Reinauer wrote:
Carl-Daniel Hailfinger wrote:
LAR currently overwrites the top 13 bytes (0xfffff3-0xffffff) of the bootblock with zeros, then it stores the ROM size in 0xfffff4-0xfffff7. The top 8 bytes are unused in that scheme. Leave the top 8 bytes as they are and allow us to store something in there during the bootblock build process. The byte at 0xfffff3 is 0xff by default on x86 and setting it to 0 is not needed.
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net
Nack.
They're unused, and until they are used, they should be cleared to a consistent state.
They are already cleared by arch/x86/stage0_common.S. Having to clear them in the LAR would mean the code in stage0_common.S is crap and needs to be fixed.
Not necessarily. This is a userland utility, so there is no real penalty for making extra sure that the area is in a valid state. LAR shouldn't depend on the v3 code to do the right thing, nor should the v3 code depend on LAR to do the right thing.
In fact the area is not completely clean due to the binutils workaround. We had "nicer" code there, and it broke on Ron's binutils, so we went to the current version. It works, so don't waste time fixing it.
Stefan
On Wed, Dec 3, 2008 at 2:02 PM, Stefan Reinauer stepan@coresystems.de wrote:
In fact the area is not completely clean due to the binutils workaround. We had "nicer" code there, and it broke on Ron's binutils, so we went to the current version. It works, so don't waste time fixing it.
In fact, if anything, can we please fix the cn700?
these cosmetic patches are not really helping anything.
ron
On 03.12.2008 23:27, ron minnich wrote:
On Wed, Dec 3, 2008 at 2:02 PM, Stefan Reinauer stepan@coresystems.de wrote:
In fact the area is not completely clean due to the binutils workaround. We had "nicer" code there, and it broke on Ron's binutils, so we went to the current version. It works, so don't waste time fixing it.
Since the code works, it is pointless to overwrite the result of the working code in LAR.
In fact, if anything, can we please fix the cn700?
I don't have the hardware, so I can't fix it. I did fix the C7 CAR code, but beyond that I can't help.
these cosmetic patches are not really helping anything.
This patch is not cosmetic and it is needed to fix the broken heuristic in flashrom for coreboot image detection.
Regards, Carl-Daniel
Carl-Daniel Hailfinger wrote:
This patch is not cosmetic and it is needed to fix the broken heuristic in flashrom for coreboot image detection.
Not at all.
//Peter
On 04.12.2008 00:15, Peter Stuge wrote:
Carl-Daniel Hailfinger wrote:
This patch is not cosmetic and it is needed to fix the broken heuristic in flashrom for coreboot image detection.
Not at all.
OK, I give up.
I documented this bug in r1064 if someone else ever stumbles on it.
Regards, Carl-Daniel
On Wed, Dec 3, 2008 at 5:27 PM, ron minnich rminnich@gmail.com wrote:
On Wed, Dec 3, 2008 at 2:02 PM, Stefan Reinauer stepan@coresystems.de wrote:
In fact the area is not completely clean due to the binutils workaround.
We
had "nicer" code there, and it broke on Ron's binutils, so we went to the current version. It works, so don't waste time fixing it.
In fact, if anything, can we please fix the cn700?
Working on it!
Current status: a GPIO line isn't getting disabled for some reason (even though it's disabled several times), and the damn chipset reboots when the timer it controls expires. Once the board reboots, something isn't working correctly, presumably CAR, and the LAR can't be read from. I'm working from revision 1010, but I think it's the same status in current svn. I'll be spending more time with it this weekend, from the sound of the weather report I won't be able to do much else anyways :p
-Corey
these cosmetic patches are not really helping anything.
ron
-- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
On 05.12.2008 09:53, Corey Osgood wrote:
On Wed, Dec 3, 2008 at 5:27 PM, ron minnich rminnich@gmail.com wrote:
In fact, if anything, can we please fix the cn700?
Working on it!
Current status: a GPIO line isn't getting disabled for some reason (even though it's disabled several times), and the damn chipset reboots when the timer it controls expires. Once the board reboots, something isn't working correctly, presumably CAR, and the LAR can't be read from. I'm working from revision 1010, but I think it's the same status in current svn. I'll be spending more time with it this weekend, from the sound of the weather report I won't be able to do much else anyways :p
Could you give me a little more verbose info? If CAR is not working, printk should not work (or at least crash directly after the first call). Having the ROM not mapped also looks weird. I don't doubt your description, I'm just trying to understand it. In theory, the different cache behaviour of the C7 on poweron could be related... can you try issuing a reset from inside coreboot and check if the same problem appears after your deliberate reset? Oh, and please check how the processor behaves if you press the reset button. I hope to find a pattern here.
Regards, Carl-Daniel
were you able to get a working rev tested?
Just want to make sure your hardware has not gone bad on you.
ron
-
Carl-Daniel Hailfinger -
Corey Osgood -
Jordan Crouse -
Peter Stuge -
ron minnich -
Stefan Reinauer