[J-core] Illegal instruction handling (was: Re: PC-relative loads and delay slots)

[Robert Ou]

On Tue, Jul 19, 2016 at 11:40 PM, Robert Ou <rqou at robertou.com> wrote:
> On Tue, Jul 19, 2016 at 10:01 AM, Geoff Salmon
> <gsalmon at se-instruments.com> wrote:
>> On 16-07-19 11:01 AM, Rich Felker wrote:
>>>
>>> I agree and I would assume this was the motivation for the sh4 change.
>>> This is a good time to mention that it would be really nice if illegal
>>> slot instruction and undefined instruction exceptions actually worked.
>>
>>
>> The illegal slot instruction detection is implemented. I don't know why it's
>> not working. Someone will have to look in simulation to see why.
>>
>> It uses this function to detect instructions that don't belong in delay
>> slots.
>>
>>     function check_illegal_delay_slot (code : std_logic_vector(15 downto 0))
>> return std_logic is
>>     begin
>>         -- Check for instructions that assign to PC:
>>         -- RTE, RTS, JMP @Rm, JSR @Rm, BRAF Rm, BSRF Rm, BF label, BF /S
>> label, BT label, BT /S label, BRA label, BSR label, TRAPA #imm
>>         if ((code(15 downto 12) = "0000" and code(3 downto 2) = "00" and
>> code(0) = '1') or (code(15 downto 14) = "10" and code(12 downto 11) = "01"
>> and code(8) = '1') or code(15 downto 13) = "101" or (code(15) = '1' and
>> code(13 downto 8) = "000011") or (code(15) = '0' and code(13 downto 12) =
>> "00" and code(4 downto 0) = "01011")) then
>>             return '1';
>>         else
>>             return '0';
>>         end if;
>>     end;
>>
>> That may be an incomplete list.
>>
>>
>> The illegal/undefined instruction test is woefully incomplete.
>>
>>     function check_illegal_instruction (code : std_logic_vector(15 downto
>> 0)) return std_logic is
>>     begin
>>         -- TODO: Improve detection of illegal instructions
>>         if code(15 downto 8) = x"ff" then
>>             return '1';
>>         else
>>             return '0';
>>         end if;
>>     end;
>>
>> We weren't sure how to implement it without a large amount of logic. At one
>> point I was trying to generate it from the instruction spreadsheet. I forget
>> why that didn't work, or maybe it did work but the result was too large. If
>> the instruction space is full enough, it may be easy to hand-write the
>> check_illegal_instruction function to cover all the gaps. Whatever is
>> preventing the illegal delay slot test from having an effect will also
>> probably stop the illegal instruction test as well though.
>>
>> - Geoff
>>
>> _______________________________________________
>> J-core mailing list
>> J-core at lists.j-core.org
>> http://lists.j-core.org/mailman/listinfo/j-core
>
> I just tested illegal instruction handling, and it does "work" with
> some caveats:
>
> a) Illegal instruction checking is very limited as Geoff just explained.
>
> b) The vector used by illegal instructions is always hardcoded to 0.
> This happens on line 78 of decode_core.vhm:
> cd := x"0" & system_instr_codes(instr) & x"00";
> If you change the x"00" to something else, then all "system operations
> that are not external events" (slot illegal, general illegal, and
> break) will then use that new vector.
>
> c) You need to explicitly set VBR to 0 in your program if you are
> simulating (this should not be necessary on real hardware). The reason
> for this is quite interesting. If you look at the microcode
> spreadsheet for the reset logic, the CPU tries to clear VBR to 0 by
> xor-ing TEMP1 with itself and storing that in VBR. On a real device,
> this should work. However, the register file in simulation is not
> initialized with anything, so it gets a special value of "U" for
> uninitialized. Apparently, xor-ing "U" with "U" doesn't give 0 but
> instead gives another "U". When an illegal instruction happens and the
> CPU tries to read the vector table, it then adds some constant to the
> value of VBR. "U" plus something then gives "X" for unknown/invalid,
> and the CPU then tries to read from an address "XXXXXXXX" which
> obviously doesn't work.
>
> Robert
> _______________________________________________
> J-core mailing list
> J-core at lists.j-core.org
> http://lists.j-core.org/mailman/listinfo/j-core

I have just written a patch that checks for illegal instructions
completely. It is attached. Note that it may have mistakes. I tested
it briefly in simulation and smoke-tested booting the kernel, but I
didn't test it very extensively (you can see the simulation in the
illegal_test branch of my MyHDL+J-core demonstration). Unfortunately,
I didn't test Linux actually properly handling a program that has an
illegal instruction because I couldn't figure out from a quick search
which vector an illegal instruction is actually supposed to raise, nor
could I figure out where in the kernel VBR actually gets set.

The logic in this patch was written mostly by hand. I wrote a program
to enumerate all illegal instructions and group them by the first and
last nybble. I then manually cleaned up and simplified the cases.

When I compiled this patch for the Mimas v2 board, it didn't seem to
make the timing report any noticeable amount worse than before (I
don't think the "dq_pads" constraint has ever succeeded for me even on
an unmodified source tree). The PAR report is also attached. Note that
this source tree has both this illegal instruction patch and my
previous "intuitive delay slot" patch.

Incidentally, I may have discovered a bug in binutils. sh2elf-objdump
(Rob Landley's binary version) disassembles 0x0048 as clrs and 0x0058
as sets even when given the -msh2 option. These instructions were
(according to http://www.shared-ptr.com/sh_insns.html) only introduced
in SH3.

Robert
-------------- next part --------------
A non-text attachment was scrubbed...
Name: illegal-instruction-fix.patch
Type: text/x-patch
Size: 3015 bytes
Desc: not available
URL: <http://lists.j-core.org/pipermail/j-core/attachments/20160720/b545d415/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mimas_v2_par.par
Type: application/octet-stream
Size: 18904 bytes
Desc: not available
URL: <http://lists.j-core.org/pipermail/j-core/attachments/20160720/b545d415/attachment-0001.obj>