[LinuxBIOS] patch for making system run past disable_car

[Marc Jones]

ron minnich wrote:
> 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.
> 
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)
> 
> 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.
> 

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


-- 
Marc Jones
Senior Firmware Engineer
(970) 226-9684 Office
mailto:Marc.Jones at amd.com
http://www.amd.com/embeddedprocessors