Hi Steve,

That was super helpful. I'm now a step closer to solving this!

Your suggestion of =ssz, lead me to search for the uses of that in x86, and
it turns out that almost all of other stack instructions have dataSize=ssz.
So, I added both dataSize=ssz and addressSize=ssz to the call instruction,
though I think only the addressSize is actually needed, but I'm not certain.

Now, the address is passed to the Request object correctly, but the program
still fails. The problem now is that the request object is getting
the AddrSizeFlagBit set to true, because machInst.legacy.addr is true.
Thus, the TLB only uses the lower 32 bits of the 64-bit address.

Any ideas on how to change the instruction's memFlags from the macro-op
definition? They are set on
http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/x86/isa/microops/ldstop.isa#l334
.

It would be nice if we could fix this in the decoder. I think the logic
should be "if the address prefix is set and this is an implicit stack
operation, ignore the address prefix". However, I'm not sure how to tell if
the instruction is "an implicit stack operation" from the decoder.

Thanks,
Jason

On Tue, Jan 24, 2017 at 9:05 AM Steve Reinhardt <ste...@gmail.com> wrote:

My recollection of how all this works is that the arguments to the 'st'
micro-op get turned into arguments to a call to the StoreOp constructor:

   597 <http://repo.gem5.o, and that address doesn't
existrg/gem5/file/cd7f3a1dbf55/src/arch/x86/isa/microops/ldstop.isa#l597
<http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/x86/isa/microops/ldstop.isa#l597>
>
        class StoreOp(LdStOp):
   598 <
http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/x86/isa/microops/ldstop.isa#l598
>
            def __init__(self, data, segment, addr, disp = 0,
   599 <
http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/x86/isa/microops/ldstop.isa#l599
>
                    dataSize="env.dataSize",
   600 <
http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/x86/isa/microops/ldstop.isa#l600
>
                    addressSize="env.addressSize",
   601 <
http://repo.gem5.org/gem5/file/cd7f3a1dbf55/src/arch/x86/isa/microops/ldstop.isa#l601
>
                    atCPL0=False, nonSpec=False):

so the "-env.dataSize" you see is actually the displacement for the store,
not the dataSize or addressSize.  I think the problem is that the
addressSize is using the env,addressSize that's calculated the "normal"
way, i.e., including the effects of the override prefix.

Poking around some more, there's an 'ssz' symbol that maps to env.stackSize
[1] which gets used a lot in stack operations; for example, right after the
store in CALL_NEAR_I, 'ssz' is subtracted off of the stack pointer. The
calculation of env.stackSize seems to follow the rule you mentioned about
being fixed at 64 bits in 64-bit mode [2, 3].

So it might be sufficient to add ', addressSize=ssz' to the end of the 'st'
micro-op. Oddly though I don't see addressSize being set on any other stack
ops (just dataSize), so I wonder if this is a problem with other stack
instructions or not. If so, it might be useful to have an override
hardwired in at some lower level to check if the segment is ss and force
the address size to be stackSize (rather than adding this extra parameter
in a dozen different places). I wouldn't know where best to do that though,
so the manual override seems best for now.

Steve

[1] http://grok.gem5.org/xref/gem5/src/arch/x86/isa/microasm.isa#107
[2] http://grok.gem5.org/xref/gem5/src/arch/x86/isa.cc#91
[3] http://grok.gem5.org/xref/gem5/src/arch/x86/decoder.cc#400




On Mon, Jan 23, 2017 at 4:04 PM, Jason Lowe-Power <ja...@lowepower.com>
wrote:

> To those of you with more x86 ISA implementation knowledge than I have:
>
> I've been working through a bug one of our users found (thanks
Sanchayan!).
> It looks like current versions of ld use the 0x67 instruction prefix
> (address size override) as an optimization instead of using a nop. See
> https://www.sourceware.org/ml/binutils/2016-05/msg00323.html.
>
> This causes the call instruction to be decoded with with the "address size
> override prefix", which is correct, in a sense. From what I can tell, this
> is passed to the call instruction via "-env.dataSize" (see call
instruction
> implementation below).
>
> def macroop CALL_NEAR_I
> {
>     # Make the default data size of calls 64 bits in 64 bit mode
>     .adjust_env oszIn64Override
>     .function_call
>
>     limm t1, imm
>     rdip t7
>     # Check target of call
>     st t7, ss, [0, t0, rsp], "-env.dataSize"
>     subi rsp, rsp, ssz
>     wrip t7, t1
> };
>
> Now, the bug is, according to the x86 manual, "For instructions that
> implicitly access the stack segment (SS), the address size for stack
> accesses is determined by the D (default) bit in the stack-segment
> descriptor. In 64-bit mode, the D bit is ignored, and all stack references
> have a 64-bit address size." See
> https://support.amd.com/TechDocs/24594.pdf page
> 9.
>
> Thoughts on how to fix this?
>
> Thanks,
> Jason
> _______________________________________________
> gem5-dev mailing list
> gem5-dev@gem5.org
> http://m5sim.org/mailman/listinfo/gem5-dev
>
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

-- 

Jason
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to