ron minnich wrote:
On Dec 12, 2007 9:49 PM, Marc Jones marc.jones@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.
Yes, someone could do that. This is why I think it would be a good idea to return to CAR init code to disable CAR. It forces you back to a known state. Not much has happened prior to CAR being enabled.
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.
Yes, so don't copy 4KB but just up to %esp (or esp rounded to the next dword).
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.
A similar thing is done with the K8 sysinfo structure. It isn't on the stack but it is in CAR and has to be moved to memory. As I mentioned above, if you return to the CAR code nothing can really be on the stack. This forces platform maintainers to design structures and explicitly move them to memory. They can worry about the pointer problem. Yes, it is work for them but they are in control and it shouldn't be that hard.
Or, more simply, disable_car is this:
void disable_car(void (*chain)(void), void *newstack)
- disable CAR
- 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.
I think that this would work. I don't think that disable CAR has to return. I just don't think it should be called from a mainboard file like it is in V2.
Marc