Bug 1207

Summary:	evaluate compatibility with microwatt's sc
Product:	Libre-SOC's first SoC	Reporter:	Jacob Lifshay <programmerjake>
Component:	Source Code	Assignee:	Jacob Lifshay <programmerjake>
Status:	RESOLVED FIXED
Severity:	enhancement	CC:	libre-soc-bugs, lkcl, programmerjake
Priority:	---
Version:	unspecified
Hardware:	PC
OS:	Linux
NLnet milestone:	---	total budget (EUR) for completion of task and all subtasks:	0
budget (EUR) for this task, excluding subtasks' budget:	0	parent task for budget allocation:
child tasks for budget allocation:		The table of payments (in EUR) for this task; TOML format:
Bug Depends on:
Bug Blocks:	1177

Description Jacob Lifshay 2023-11-07 09:01:23 GMT

copied from:
https://bugs.libre-soc.org/show_bug.cgi?id=1177#c22

(In reply to Luke Kenneth Casson Leighton from bug #1177 comment #21)
> (In reply to Jacob Lifshay from bug #1177 comment #20)
> > I also fixed sc and the previously-working tests for it, since SRR1 is
> > specified to have bits 33:36 set to 0, 42:43 set to LEV (always 0 for now),
> > and 44:47 set to 0. SRR1[TRAP] is *not* set.
> 
> please revert aaaalll of them. we are compatible with MICROWATT.
> i keep having to tell you this.

well, my changes make us *compatible* with microwatt again:
on sc-test branch (based off latest master):
https://git.libre-soc.org/?p=microwatt.git;a=commitdiff;h=44eab364aee1a7cb6198cb148d68a065715a434a

make -C sc_test
ln -sf sc_test/hello_world.bin main_ram.bin
make DOCKER=1
./core_tb > /dev/null

gives the following output on my computer:
before sc:
SRR0 = 0x0000000012345678 SRR1 = 0x0000000000005678 MSR = 0x90000000783F2903
sc
after sc:
SRR0 = 0x000000000000112C SRR1 = 0x8000000000002903 MSR = 0x80000000783F0001
before sc:
SRR0 = 0x0000000012345678 SRR1 = 0x0000000000005678 MSR = 0x9000000000000003
sc
after sc:
SRR0 = 0x000000000000112C SRR1 = 0x8000000000000003 MSR = 0x8000000000000001
<snip>

as you can see, microwatt sc doesn't set SRR1[TRAP] (which would be 0x20000), and it also clears all the SRR1 bits that sc is supposed to
(basically SRR1 &= ~0x783f0000)

Comment 1 Jacob Lifshay 2023-11-07 09:02:58 GMT

oops, accidentally removed blocks

Comment 2 Jacob Lifshay 2023-11-09 02:29:30 GMT

(In reply to Luke Kenneth Casson Leighton from bug #1177 comment #23)
> (In reply to Jacob Lifshay from bug #1177 comment #22)
> > (edit: please continue conversation on sc on bug #1207)

apparently you missed that...

> > well, my changes make us *compatible* with microwatt again:
> 
> okaay,interesting. can you check that they don't set the LEV bit,
> in OP_SC? implementing Hypervisor is completely off-limits.

yup. they ignore LEV.

https://git.libre-soc.org/?p=microwatt.git;a=commit;h=5c5456ef20448a09f661bfe70420f7d3f683a8dc

before sc:
SRR0 = 0x0000000012345678 SRR1 = 0x0000000000005678 MSR = 0x90000000783F2903
sc 0
after sc:
SRR0 = 0x0000000000001020 SRR1 = 0x8000000000002903 MSR = 0x80000000783F0001
before sc:
SRR0 = 0x0000000012345678 SRR1 = 0x0000000000005678 MSR = 0x9000000000000003
sc 0
after sc:
SRR0 = 0x0000000000001020 SRR1 = 0x8000000000000003 MSR = 0x8000000000000001
before sc:
SRR0 = 0x0000000012345678 SRR1 = 0x0000000000005678 MSR = 0x9000000000000003
sc 1
after sc:
SRR0 = 0x000000000000105C SRR1 = 0x8000000000000003 MSR = 0x8000000000000001
before sc:
SRR0 = 0x0000000012345678 SRR1 = 0x0000000000005678 MSR = 0x9000000000000003
sc 2
after sc:
SRR0 = 0x0000000000001098 SRR1 = 0x8000000000000003 MSR = 0x8000000000000001

> 
> > on sc-test branch (based off latest master):
> > https://git.libre-soc.org/?p=microwatt.git;a=commitdiff;
> > h=44eab364aee1a7cb6198cb148d68a065715a434a
> 
> oh! good idea. niice teeest :)

thx!

> yes use a "None" option but
> make sure it is documented.

done in:
https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=7062e72765b37640e435a8551f19fcbaef81aa3b

+++ b/src/openpower/decoder/isa/caller.py
@@ -1357,6 +1357,11 @@ class ISACaller(ISACallerHelper, ISAFPHelpers, StepLoop):
         TRAP function is callable from inside the pseudocode itself,
         hence the default arguments.  when calling from inside ISACaller
         it is best to use call_trap()
+
+        trap_addr: int | SelectableInt
+            the address to go to (before any modifications from `KAIVB`)
+        trap_bit: int | None
+            the bit in `SRR1` to set, `None` means don't set any bits.
         """
         if isinstance(trap_addr, SelectableInt):
             trap_addr = trap_addr.value

Comment 3 Luke Kenneth Casson Leighton 2023-11-09 02:50:21 GMT

(In reply to Jacob Lifshay from comment #2)

> apparently you missed that...

yes. i am on the move (again).

> +++ b/src/openpower/decoder/isa/caller.py
> @@ -1357,6 +1357,11 @@ class ISACaller(ISACallerHelper, ISAFPHelpers,
> StepLoop):
>          TRAP function is callable from inside the pseudocode itself,
>          hence the default arguments.  when calling from inside ISACaller
>          it is best to use call_trap()
> +
> +        trap_addr: int | SelectableInt
> +            the address to go to (before any modifications from `KAIVB`)
> +        trap_bit: int | None
> +            the bit in `SRR1` to set, `None` means don't set any bits.
>          """
>          if isinstance(trap_addr, SelectableInt):
>              trap_addr = trap_addr.value

ahh looks really good. the default wasn't changed so td/tw etc are fine.

l.