[Qemu-devel] Re: [PATCH] Fix up pxe boot

[Glauber Costa <glommer@redhat.com>]

On Tue, Sep 02, 2008 at 06:20:55PM +0300, Avi Kivity wrote:
> Glauber Costa wrote:
>> On Tue, Sep 2, 2008 at 5:39 AM, Avi Kivity <avi@qumranet.com> wrote:
>>   
>>> Glauber Costa wrote:
>>>     
>>>> diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
>>>> index 0b5fdc0..433aa3f 100644
>>>> --- a/target-i386/op_helper.c
>>>> +++ b/target-i386/op_helper.c
>>>> @@ -600,7 +600,7 @@ do {\
>>>>  #define PUSHL(ssp, sp, sp_mask, val)\
>>>>  {\
>>>>     sp -= 4;\
>>>> -    stl_kernel((ssp) + (sp & (sp_mask)), (val));\
>>>> +    stl_kernel((uint32_t)((ssp) + (sp & (sp_mask))), (uint32_t)(val));\
>>>>  }
>>>>
>>>>       
>>> Surly it is better to push this into the underlying virtual->physical
>>> translation functions, so it applies everywhere?
>>>
>>> btw, the cast is wrong for x86-64, so it must be qualified for 32-bit
>>> operating modes.
>>>     
>> The tests were all done with x86_64. This is a PUSHL macro, so it's
>> 32-bit anyway.
>> A x86_64-only PUSHQ seems to do the right thing.
>>
>>   
>
> Right.
>
> It's still odd to see this in an op helper rather than in somewhere generic.

After a second look, here's what it seems to me:

It's not in a generic place, such as ldl, because in general, we may want to grab
a 32-bit value from a 64-bit address. This is perfectly valid.

It's a specifity that the pop instruction, when not in long mode (manual says that in 64-bit mode
no 32-bit operand is valid, but then again, qemu should use the POPQ macro), that ssp:sp may overflow,
but we don't want it.

It would be possible to do something more generic if we had a segment_to_linear() function, that returned
the linear address, but we don't.

Does it make more sense to you?
>
> -- 
> error compiling committee.c: too many arguments to function
>