[LinuxBIOS] patch for making system run past disable_car

ron minnich rminnich at gmail.com
Thu Dec 13 18:08:02 CET 2007


On Dec 12, 2007 9:49 PM, Marc Jones <marc.jones at amd.com> wrote:

> I don't think that moving the stack should be a problem. All access
> should be push/pop or ss/esp/ebp relative. I also don't think you need
> to copy the stack back (like K8) and I would only copy the amount of
> stack that is being used. Not the entire space.

actually it could be a huge problem. If at some future time someone
does something such as  & a stack variable, that's no longer an
esp-relative reference. You can't move the stack at that point. Or you
can but that pointer is now wrong.

Also, in general, it is hard to know how much of the stack is being
used ... you really have to save from %esp to top of stack, whatever
that is.

Here's my concern. If we return from disable_car(), and we have moved
the stack, we're making a guarantee that the state of the stack is
good for anything on the stack. We either meet that guarantee or we
don't. If we meet it halfway, it's going to cause trouble for
somebody in future, and the error is going to be quite obscure, since
some things work and some don't. Possibly a comment in the calling
code would help ("don't use pointers").

You really should copy the whole stack back IMHO as somebody might in
future put a big struct on the stack. They have to put it on stack,
since memory is not working at that point.

Or, more simply, disable_car is this:

void disable_car(void (*chain)(void), void *newstack)

1. disable CAR
2. set up new stack and execution environment in ram; if newstack is
NULL, a machine-specific default is used
3. call chain(), which does not return.

disable_car does not return. This disable_car is simple and involves a
lot less magic. People don't have unrealistic expectations of what is
going to happen. I realize the other magic seems easier, but it's a
lot harder to debug when things start to go wrong. We need code that
we don't have to try to debug 2 years from now.

> The savestack variable won't be a problem. It will either be in a
> register or on the stack. Once the stack is move to real memory and the
> registers adjusted the invd won't change anything. The register
> adjustment will have to be inline asm with no push, pops, or stack
> accesses. For and example, look in cpu/amd/car/post_cache_as_ram.c for %esp.

As I say, we can do this, but it won't really preserve the stack, and
the likely result is a strange error the first time a pointer is used.
It worries me. But let's see what people think.

ron




More information about the coreboot mailing list