[Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Lucien Anti-Spam via Qemu-devel]

Hi folks,
Does anyone have any knowledge why
gen_exception(s, s->base.pc_next, EXCP_RTE);

Re: [Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Lucien Anti-Spam via Qemu-devel]

 Hi Laurent / Richard,
(resent email )
Does anyone have any knowledge why    gen_exception(s, s->base.pc_next, EXCP_RTE);

is generated for "RTE" instruction, where as the "RTS" goes a gen_jmp?( note see target/m68k/translate.c in functions DISAS_INSN(rte) and DISAS_INSN(rts)

Cheers,Luc  

Re: [Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Richard Henderson]

On 6/27/19 3:22 PM, Lucien Anti-Spam wrote:
> Hi Laurent / Richard,
> (resent email )
> 
> Does anyone have any knowledge why
>     gen_exception(s, s->base.pc_next, EXCP_RTE);
> 
> is generated for "RTE" instruction, where as the "RTS" goes a gen_jmp?
> ( note see target/m68k/translate.c in functions DISAS_INSN(rte) and DISAS_INSN(rts)

History, it would seem.  Paul Brook implemented it that way in 2007.

I think that it should not be implemented as an exception.  It should be a call
to one of two different helpers (cf and m68k), followed by either a normal exit
to main loop (to recognize the new interrupt state) or a debug exception.

This sort of modification should be fairly easy to perform, if you have the time.


r~

Re: [Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Lucien Murray-Pitts]

On Thu, Jun 27, 2019 at 07:09:39PM +0200, Richard Henderson wrote:
> On 6/27/19 3:22 PM, Lucien Anti-Spam wrote:
> > Hi Laurent / Richard,
> > (resent email )
> > 
> > Does anyone have any knowledge why
> >     gen_exception(s, s->base.pc_next, EXCP_RTE);
> > 
> > is generated for "RTE" instruction, where as the "RTS" goes a gen_jmp?
> > ( note see target/m68k/translate.c in functions DISAS_INSN(rte) and DISAS_INSN(rts)
> 
> History, it would seem.  Paul Brook implemented it that way in 2007.

Ok, thank you I wanted to make sure RTE wasnt being one like this as a special case.

> 
> I think that it should not be implemented as an exception.  It should be a call
> to one of two different helpers (cf and m68k), followed by either a normal exit
> to main loop (to recognize the new interrupt state) or a debug exception.
> 
> This sort of modification should be fairly easy to perform, if you have the time.
> 

The original way of handling it was causing single step to malfunction, I dont
rightly know why but the effect was that step would step twice and end up
inside the ISR function again OR just stepping past the RTE as if it didnt
exist.

I have made a quick hack to implement it the way you suggest and confirm that
works better.

HOWEVER, the "return" address is the instruction that causes the exception.
So it immediately does return to the ISR.

This is a different issue, but I think interrelated to the original problem.

Further single stepping INTO the failing instruction results in ending up
at the ISR +1 instruction

I will look at these but so far a little lost on the why for of them.

Cheers,
Luc

> 
> r~

Re: [Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Richard Henderson]

On 6/28/19 2:27 AM, Lucien Murray-Pitts wrote:
> The original way of handling it was causing single step to malfunction, I dont
> rightly know why but the effect was that step would step twice and end up
> inside the ISR function again OR just stepping past the RTE as if it didnt
> exist.
> 
> I have made a quick hack to implement it the way you suggest and confirm that
> works better.
> 
> HOWEVER, the "return" address is the instruction that causes the exception.
> So it immediately does return to the ISR.
> 
> This is a different issue, but I think interrelated to the original problem.

Is this a synchronous exception, like a SIGSEGV, that the instruction is
causing?  I have made attempts at fixing asynchronous interrupts, like the
clock, in the presence of single-stepping.  I haven't tested that in a while
and I hope it's still working...

> Further single stepping INTO the failing instruction results in ending up
> at the ISR +1 instruction

For a synchronous exception, that sounds like a real bug.

Probably within cpu_handle_exception,

  #else
          if (replay_exception()) {
              CPUClass *cc = CPU_GET_CLASS(cpu);
              qemu_mutex_lock_iothread();
              cc->do_interrupt(cpu);
              qemu_mutex_unlock_iothread();
+             /* Single-step into the exception handler.  */
+             if (cpu->singlestep_enabled) {
+                 cpu_handle_debug_exception(cpu);
+             }
              cpu->exception_index = -1;
          } else if (!replay_has_interrupt()) {


r~

Re: [Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Lucien Murray-Pitts]

On Fri, Jun 28, 2019 at 11:35:48AM +0200, Richard Henderson wrote:
> On 6/28/19 2:27 AM, Lucien Murray-Pitts wrote:
> > 
> > [snip] ... the "return" address is the instruction that causes the exception.
> > So it immediately does return to the ISR.
> > This is a different issue, but I think interrelated to the original problem.
> 
> Is this a synchronous exception, like a SIGSEGV, that the instruction is
> causing?  I have made attempts at fixing asynchronous interrupts, like the
> clock, in the presence of single-stepping.  I haven't tested that in a while
> and I hope it's still working...

This issue is to do with the stack frame generation storing the retaddr
which is the current PC, I cant find any way to obtain the next PC when
not inside the instructions (say a bus handler).  So RTE will return to
the instruction causing SIGSEGV

 op_helper.c
   static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
   ...
     if (cs->exception_index == EXCP_ACCESS) {
      ...
      do_stack_frame(env, &sp, 7, oldsr, 0, retaddr /*LMP: BROKEN - needs PC NEXT*/);

Actually according to the MC68000 manuals the "return address" (the PC saved on
the stack) can be upto 5 instructions later due to prefetch. So some pc_next would best
be used here.

I am not clear how this should work in the presence of an MMU though.

I am triggering this from inside my device by doing the following, since that memory address
should dynamically cause a bus error (I hope this is the right way to do it)
   cpuclass->do_unassigned_access( s->cpu, /*addr*/0x0, /*is_write*/1, /*is_exec*/0, opaque, /*size*/4);

Something similar with TRAP#0 / RTE combination will happen.
Stepping on the TRAP#0 does correctly get me to the ISR, but a subsequent RTE will
step me +1 past the return whilst a break point and run will land in the right place.
I need to experiment a bit more with a solid setup.


> > Further single stepping INTO the failing instruction results in ending up
> > at the ISR +1 instruction
> 
> For a synchronous exception, that sounds like a real bug.
> 
> Probably within cpu_handle_exception,
> 
>   #else
>           if (replay_exception()) {
>               CPUClass *cc = CPU_GET_CLASS(cpu);
>               qemu_mutex_lock_iothread();
>               cc->do_interrupt(cpu);
>               qemu_mutex_unlock_iothread();
> +             /* Single-step into the exception handler.  */
> +             if (cpu->singlestep_enabled) {
> +                 cpu_handle_debug_exception(cpu);
> +             }
>               cpu->exception_index = -1;
>           } else if (!replay_has_interrupt()) {
> 
> 
> r~

I see the function but in 4.0 its been mangled a bit more, so I will have to get
back to you.  Seems that the issue persists in 2.x so maybe this is something new.

Cheers,
Luc

Re: [Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Richard Henderson]

On 6/28/19 5:50 PM, Lucien Murray-Pitts wrote:
>  op_helper.c
>    static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
>    ...
>      if (cs->exception_index == EXCP_ACCESS) {
>       ...
>       do_stack_frame(env, &sp, 7, oldsr, 0, retaddr /*LMP: BROKEN - needs PC NEXT*/);
> 
> Actually according to the MC68000 manuals the "return address" (the PC saved on
> the stack) can be upto 5 instructions later due to prefetch. So some pc_next
> would best be used here.

The way I read it from the 68040 manual, it's "the pc of the instruction
executing at the time the fault was detected".  Well, we did in fact detect the
fault at "retaddr", so that seems to be the right answer.  The fact that real
hardware has a different pipeline and detects the fault later seems immaterial,
and largely irrelevant, since the programmer wasn't given any guarantees for
what sort of value appears in that slot.

> I am triggering this from inside my device by doing the following, since that memory address
> should dynamically cause a bus error (I hope this is the right way to do it)
>    cpuclass->do_unassigned_access( s->cpu, /*addr*/0x0, /*is_write*/1, /*is_exec*/0, opaque, /*size*/4);


For a device to raise a bus error, it should return MEMTX_ERROR (or something).
 This eventually reaches cpu_transaction_failed, which has all of the data that
you seem to be missing above.


r~

Re: [Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Lucien Murray-Pitts]

On Sat, Jun 29, 2019 at 12:15:44PM +0200, Richard Henderson wrote:
> On 6/28/19 5:50 PM, Lucien Murray-Pitts wrote:
> >  op_helper.c
> >    static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
> >    ...
> >      if (cs->exception_index == EXCP_ACCESS) {
> >       ...
> >       do_stack_frame(env, &sp, 7, oldsr, 0, retaddr /*LMP: BROKEN - needs PC NEXT*/);
> > 
> > Actually according to the MC68000 manuals the "return address" (the PC saved on
> > the stack) can be upto 5 instructions later due to prefetch. So some pc_next
> > would best be used here.
> 
> The way I read it from the 68040 manual, it's "the pc of the instruction
> executing at the time the fault was detected".  Well, we did in fact detect the
> fault at "retaddr", so that seems to be the right answer.  The fact that real
> hardware has a different pipeline and detects the fault later seems immaterial,
> and largely irrelevant, since the programmer wasn't given any guarantees for
> what sort of value appears in that slot.
> 

I was reading the 68000/68020, and based on your suggestion now the 68060 manual
The 68000 is pretty rough, but I agree you could expect it to more likely be the
next or even upto 5 or so instructions away.

MC68000UM.pdf, 5.4.1 Bus Error Operation
  ....the following information is placed on the supervisor stack:
    1. Status register
    2. Program counter (two words, which may be up to five words past the
       instruction being executed)
....
MC68000UM.pdf, 6.3.9.1 BUS ERROR.
  ...value saved for the program counter is advanced 2–10 bytes beyond the
  address of the first word of the instruction that made the reference causing
  the bus error. If the bus error occurred during the fetch of the next
  instruction, the saved program counter has a value in the vicinity of the
  current instruction, even if the current instruction is a branch, a jump, or
  a return instruction ...

MC68020UM.pdf, 6.1.2 Bus Error Exception
  The saved PC value is the logical address of the instruction that was
  executing at the time the fault was detected. This is not necessarily the
  instruction that initiated the bus cycle since the processor overlaps
  execution of instructions
  (See 6.4 Bus Fault REcovery and 6.3.11 Return From Exception)
  
MC68060UM.pdf, 8.4.4.1 Program Counter (PC).
On read access faults, the PC points to the instruction that caused the
access error. This instruction is restarted when an RTE is executed, hence,
the read cycle is re-executed. On read access errors on the second or later
of misaligned reads, the read cycles that are successful prior to the access
error are re-executed since the processor uses a restart model for recovery
from exceptions.

So it would seem the m68k was rather rough, but with the introduction
of MMUs the 68010 and beyond handle it differently.  68010/20 have
pipeline stage retries, and 68060 just returns to retry the instruction.

In my case I think the original firmware expects to return after the
faulting instruction, and the retry of the bus io is to be ignored
(this is a memory map probe routine).

So I think it would take significant work to fake the pipeline retry
in the RTE instruction - so I will hack somethign into the memory region
so it passes the second time the instruciton is exected.

What are your thoughts?


> > I am triggering this from inside my device by doing the following, since that memory address
> > should dynamically cause a bus error (I hope this is the right way to do it)
> >    cpuclass->do_unassigned_access( s->cpu, /*addr*/0x0, /*is_write*/1, /*is_exec*/0, opaque, /*size*/4);
> 
> 
> For a device to raise a bus error, it should return MEMTX_ERROR (or something).
>  This eventually reaches cpu_transaction_failed, which has all of the data that
> you seem to be missing above.
> 

I was originally using this but it wasnt doing anything, now that you recommend it I see why -
thank you for your help.

qemu/accel/tcg/cputlb.c
   ...
   r = memory_region_dispatch_read(mr, mr_offset,
                                    &val, size, iotlbentry->attrs);
    if (r != MEMTX_OK) {
        hwaddr physaddr = mr_offset +
            section->offset_within_address_space -
            section->offset_within_region;

        cpu_transaction_failed(cpu, physaddr, addr, size, access_type,
                               mmu_idx, iotlbentry->attrs, r, retaddr);
    }
    ...

As you say this call directly flows through to CPUClass->transaction_failed
( as found in the struct for CPUClass in qemu/include/qom/cpu.h )

However for the m68k the do_transaction_failed function pointer field
has not been implemented.

I implemented it in a rudamentary fashion copying from already existing
m68k_cpu_unassigned_access.  I really dont know if this is the right way.

  void m68k_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
                                  unsigned size, MMUAccessType access_type,
                                  int mmu_idx, MemTxAttrs attrs,
                                  MemTxResult response, uintptr_t retaddr)
  {
      cs->exception_index = EXCP_ACCESS;
      cpu_loop_exit(cs);
  }

Then I reverted my device to the "mem with attributes" io with a return of
MEMTX_ERROR and found that the behavior during single-step were the same as
before.

I end up ISR+1 instruction stepped in.

I will have to dig more.  See some other peoples implementation and methods.

If you have time I would appreciate your input, I am still very hazy on how
the complete interaction, sepcially with gdb happens.

Cheers,
Luc

Re: [Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Richard Henderson]

On 6/29/19 6:36 PM, Lucien Murray-Pitts wrote:
> On Sat, Jun 29, 2019 at 12:15:44PM +0200, Richard Henderson wrote:
>> On 6/28/19 5:50 PM, Lucien Murray-Pitts wrote:
>>>  op_helper.c
>>>    static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
>>>    ...
>>>      if (cs->exception_index == EXCP_ACCESS) {
>>>       ...
>>>       do_stack_frame(env, &sp, 7, oldsr, 0, retaddr /*LMP: BROKEN - needs PC NEXT*/);
>>>
>>> Actually according to the MC68000 manuals the "return address" (the PC saved on
>>> the stack) can be upto 5 instructions later due to prefetch. So some pc_next
>>> would best be used here.
>>
>> The way I read it from the 68040 manual, it's "the pc of the instruction
>> executing at the time the fault was detected".  Well, we did in fact detect the
>> fault at "retaddr", so that seems to be the right answer.  The fact that real
>> hardware has a different pipeline and detects the fault later seems immaterial,
>> and largely irrelevant, since the programmer wasn't given any guarantees for
>> what sort of value appears in that slot.
>>
> 
> I was reading the 68000/68020, and based on your suggestion now the 68060 manual
> The 68000 is pretty rough, but I agree you could expect it to more likely be the
> next or even upto 5 or so instructions away.
> 
> MC68000UM.pdf, 5.4.1 Bus Error Operation
>   ....the following information is placed on the supervisor stack:
>     1. Status register
>     2. Program counter (two words, which may be up to five words past the
>        instruction being executed)
> ....
> MC68000UM.pdf, 6.3.9.1 BUS ERROR.
>   ...value saved for the program counter is advanced 2–10 bytes beyond the
>   address of the first word of the instruction that made the reference causing
>   the bus error. If the bus error occurred during the fetch of the next
>   instruction, the saved program counter has a value in the vicinity of the
>   current instruction, even if the current instruction is a branch, a jump, or
>   a return instruction ...
> 
> MC68020UM.pdf, 6.1.2 Bus Error Exception
>   The saved PC value is the logical address of the instruction that was
>   executing at the time the fault was detected. This is not necessarily the
>   instruction that initiated the bus cycle since the processor overlaps
>   execution of instructions
>   (See 6.4 Bus Fault REcovery and 6.3.11 Return From Exception)
>   
> MC68060UM.pdf, 8.4.4.1 Program Counter (PC).
> On read access faults, the PC points to the instruction that caused the
> access error. This instruction is restarted when an RTE is executed, hence,
> the read cycle is re-executed. On read access errors on the second or later
> of misaligned reads, the read cycles that are successful prior to the access
> error are re-executed since the processor uses a restart model for recovery
> from exceptions.

Interesting.  Thanks for digging through the other manuals.

> In my case I think the original firmware expects to return after the
> faulting instruction, and the retry of the bus io is to be ignored
> (this is a memory map probe routine).

Oh my.  Ok, well, if there's software that relies on some advance, I guess we
can conditionally do that.  It'll take a bit of work...

In

  https://patchwork.ozlabs.org/patch/1072596/

I arrange for the length of the current instruction to be stored in qemu's
exception unwind data.  Within restore_state_to_opc, the "ilen" is stored into
an existing slot in s390's env structure; you'd have to add such a slot for m68k.

Given the length of the insn currently executing, you can reliably compute the
pc of the next instruction.  Which would let you advance the pc for this case,
for the appropriate cpu type.

> However for the m68k the do_transaction_failed function pointer field
> has not been implemented.
> 
> I implemented it in a rudamentary fashion copying from already existing
> m68k_cpu_unassigned_access.  I really dont know if this is the right way.
> 
>   void m68k_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
>                                   unsigned size, MMUAccessType access_type,
>                                   int mmu_idx, MemTxAttrs attrs,
>                                   MemTxResult response, uintptr_t retaddr)
>   {
>       cs->exception_index = EXCP_ACCESS;
>       cpu_loop_exit(cs);
>   }

At minimum that's going to need

  cpu_loop_exit_restore(cs, retaddr);

But usually store some of the addr and access_type data into fields so that you
can properly fill in the exception frame later.

> Then I reverted my device to the "mem with attributes" io with a return of
> MEMTX_ERROR and found that the behavior during single-step were the same as
> before.
> 
> I end up ISR+1 instruction stepped in.

So, earlier, I suggested adding a call to cpu_handle_debug_exception within
cpu_handle_exception.  Did you try that?


r~

Re: [Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Peter Maydell]

On Sat, 29 Jun 2019 at 17:37, Lucien Murray-Pitts
<lucienmp.qemu@gmail.com> wrote:
> However for the m68k the do_transaction_failed function pointer field
> has not been implemented.

Er, I implemented that in commit e1aaf3a88e95ab007. Are
you working with an out-of-date version of QEMU ?

thanks
-- PMM

Re: [Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Lucien Anti-Spam via Qemu-devel]

 

   >On Monday, July 1, 2019, 06:10:55 PM GMT+9, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Sat, 29 Jun 2019 at 17:37, Lucien Murray-Pitts> > <lucienmp.qemu@gmail.com> wrote:
> > However for the m68k the do_transaction_failed function pointer field
> > has not been implemented.>Er, I implemented that in commit e1aaf3a88e95ab007. Are
>you working with an out-of-date version of QEMU ?

Sorry not pulled in a long time, you are right that is there now - I dont generally check the development list outside of replies, and was focused on the stepping issue - I will be more careful of that in future.  Thanks for the heads up.
Further to my initial problem I noticed that TRAP #0 also jumps to the handlers +1 instruction.  Same behavior can also be seen with ARM "SWI #0".    (PC shows 0x0C vs the expected 0x08)
Putting a "BRA $" / "B $" so that it loops on the first address of the handler results in the step stopping, of course, at the expected "first instruction" of the vector handler.
So it would seem this maybe a wider problem than just the m68K.Since I dont really understand the TCG complete execution method, and how it fits in with the GNU RSP "s" step command I am going to take some time to work this out.
I appreciate any hints people can provide, but I dont mind plugging away - I am learning and surprising myself how much there is to this.
Cheers,Luc

  

Re: [Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Peter Maydell]

On Mon, 1 Jul 2019 at 13:04, Lucien Anti-Spam
<lucienmp_antispam@yahoo.com> wrote:
> Further to my initial problem I noticed that TRAP #0 also jumps to the handlers +1 instruction.
> Same behavior can also be seen with ARM "SWI #0".    (PC shows 0x0C vs the expected 0x08)

Yes, that's a known bug for arm -- we treat "single step" as
"execute one instruction", whereas I think most debuggers will
treat "we took an exception" as a reason to stop the single step
and return control to the user, rather than executing the insn at
the exception entry point as the one instruction of the step.
(You can see similar odd behaviour if you try to single step a
load instruction which causes a data abort, for instance -- on
arm at least we will execute the first insn of the data abort
handler before completing the step.)

thanks
-- PMM

Re: [Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Lucien Murray-Pitts]

On Mon, Jul 1, 2019 at 9:11 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> > On Mon, 1 Jul 2019 at 13:04, Lucien Anti-Spam
> > <lucienmp_antispam@yahoo.com> wrote:
> > > Further to my initial problem I noticed that TRAP #0 also jumps to the
> handlers +1 instruction.
> > > Same behavior can also be seen with ARM "SWI #0".    (PC shows 0x0C vs
> the expected 0x08)
> >
> > Yes, that's a known bug for arm -- we treat "single step" as
> > "execute one instruction", whereas I think most debuggers will
> > treat "we took an exception" as a reason to stop the single step
> > and return control to the user, rather than executing the insn at
> > the exception entry point as the one instruction of the step.
> > (You can see similar odd behaviour if you try to single step a
> > load instruction which causes a data abort, for instance -- on
> > arm at least we will execute the first insn of the data abort
> > handler before completing the step.)
> > thanks
> > -- PMM
>

As recommended in the previous email this is fixable with a call to handle
debug
when were in single step -  I will submit that patch if nobody else it
working on this?

I also then found the m68k stack frame is fouled for 68010/68020 (wrong
frame type,
and it does not honor the special status word aka SSW).

In real hardware the handler code should alter the stack frame chaning the
SSW.
RTE should then start/or-not-start the "pipeline" again from the setting in
the SSW.
This allows for stage B/C stage re-runs, thus a retry on the read
instruction.
I suspect it will keep looping, and retrying until the exception handler
decides to turn
off the rerun.

I am thinking the easiest method would be to check the re-run bits in the
SSW and
jump back to next_pc instead of pc inside the RTE instruction handler.

Any suggestions on how to obtain pc_next from the "m68k_cpu_do_interrupt(
CPUState *cs)" ?

What do we think of this approach?

Cheers,
Luc

Re: [Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Peter Maydell]

On Tue, 9 Jul 2019 at 17:58, Lucien Murray-Pitts
<lucienmp.qemu@gmail.com> wrote:
> On Mon, Jul 1, 2019 at 9:11 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>> > Yes, that's a known bug for arm -- we treat "single step" as
>> > "execute one instruction", whereas I think most debuggers will
>> > treat "we took an exception" as a reason to stop the single step
>> > and return control to the user, rather than executing the insn at
>> > the exception entry point as the one instruction of the step.
>> > (You can see similar odd behaviour if you try to single step a
>> > load instruction which causes a data abort, for instance -- on
>> > arm at least we will execute the first insn of the data abort
>> > handler before completing the step.)

> As recommended in the previous email this is fixable with a call to handle debug
> when were in single step -  I will submit that patch if nobody else it working on this?

I don't think anybody else is, so go ahead.

> Any suggestions on how to obtain pc_next from the "m68k_cpu_do_interrupt( CPUState *cs)" ?

You can't -- pc_next exists only at translate time, and the do_interrupt
function is called at run time. What do_interrupt needs to do is look
at (a) the current state of the CPU and (b) whatever has been stashed
in the exception frame, and then update the state of the CPU accordingly.

thanks
-- PMM

Re: [Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Richard Henderson]

On 7/9/19 6:58 PM, Lucien Murray-Pitts wrote:
> Any suggestions on how to obtain pc_next from the "m68k_cpu_do_interrupt(
> CPUState *cs)" ?

I did have a suggestion.  It was fairly detailed.

https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg06522.html


r~

Re: [Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Lucien Murray-Pitts]

Hi Richard,

I am so sorry, I was too naive when you suggested this and I misunderstood
the meaning of what you were telling me.

After spending most nights digging through the RTE/Exception process for
the M68000 I now actually get what you
were trying to tell me and its simply genius  - I hope it serves others
well too when they need to do something similar.

I still dont have all the tools in my chest to understand what I have, but
I will keep digging.

Many thanks,
Luc


On Wed, Jul 10, 2019 at 4:04 AM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 7/9/19 6:58 PM, Lucien Murray-Pitts wrote:
> > Any suggestions on how to obtain pc_next from the "m68k_cpu_do_interrupt(
> > CPUState *cs)" ?
>
> I did have a suggestion.  It was fairly detailed.
>
> https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg06522.html
>
>
> r~
>

Re: [Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Lucien Murray-Pitts]

> On Wed, Jul 10, 2019 at 4:04 AM Richard Henderson <
richard.henderson@linaro.org> wrote:

> > I did have a suggestion.  It was fairly detailed.
> > https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg06522.html
>
> Your solution is elegant at about 10 lines that return getl_ilen(pc), but
it seems the s390 has a far simpler
instruction word format than the m68k.

However then that got me to thinking, it seems that we can call a portion
of the TCG system to disassemble a single instruction.
    TranslationBlock tb;
    tb.pc = env->pc;
    gen_intermediate_code(cs, &tb, /* max isn */ 1);
    int ilen = tb.size;
    printf( "PC: %08x sz:%08x\n", env->pc, tb, ilen ) ;

I am very new to TCG, so it does seem there is a lot of code in the
translator_loop that appears to be interacting with the CPU model/state.
Should I be worried about this, or is this a safe function to call outside
of the translator core proper?
(if everyone is too busy I can dig by myself but I think its going to take
some time)

Cheers,
Luc

Re: [Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Alex Bennée]

Lucien Murray-Pitts <lucienmp.qemu@gmail.com> writes:

>> On Wed, Jul 10, 2019 at 4:04 AM Richard Henderson <
> richard.henderson@linaro.org> wrote:
>
>> > I did have a suggestion.  It was fairly detailed.
>> > https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg06522.html
>>
>> Your solution is elegant at about 10 lines that return getl_ilen(pc), but
> it seems the s390 has a far simpler
> instruction word format than the m68k.
>
> However then that got me to thinking, it seems that we can call a portion
> of the TCG system to disassemble a single instruction.
>     TranslationBlock tb;
>     tb.pc = env->pc;
>     gen_intermediate_code(cs, &tb, /* max isn */ 1);
>     int ilen = tb.size;
>     printf( "PC: %08x sz:%08x\n", env->pc, tb, ilen ) ;
>
> I am very new to TCG, so it does seem there is a lot of code in the
> translator_loop that appears to be interacting with the CPU model/state.
> Should I be worried about this, or is this a safe function to call outside
> of the translator core proper?

I would recommend against it - the time to do stuff like this would be
during translation phase where you can save the data. Don't re-invoke the
translator while trying to process an exception.

Is the instruction format that irregular that you can't do a simple
disassembly in a helper?

> (if everyone is too busy I can dig by myself but I think its going to take
> some time)
>
> Cheers,
> Luc


--
Alex Bennée

Re: [Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Peter Maydell]

On Wed, 10 Jul 2019 at 19:15, Alex Bennée <alex.bennee@linaro.org> wrote:
> Lucien Murray-Pitts <lucienmp.qemu@gmail.com> writes:
> > I am very new to TCG, so it does seem there is a lot of code in the
> > translator_loop that appears to be interacting with the CPU model/state.
> > Should I be worried about this, or is this a safe function to call outside
> > of the translator core proper?
>
> I would recommend against it - the time to do stuff like this would be
> during translation phase where you can save the data. Don't re-invoke the
> translator while trying to process an exception.
>
> Is the instruction format that irregular that you can't do a simple
> disassembly in a helper?

For anything moderately complicated, the use of the tcg_set_insn_param()
and restore_state_to_opc() that RTH recommends is definitely the
way to go. That way you can just save the right information when
you translate the code and don't have two separate bits of decoding
logic that need to agree.

For a more complicated example than the s390 one, you can look
at target/arm, which uses this to generate the 'syndrome' register
value which can include things like "which register was this faulting
load trying to load to".

thanks
-- PMM

Re: [Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Richard Henderson]

On 7/10/19 7:50 PM, Lucien Murray-Pitts wrote:
> 
> 
>> On Wed, Jul 10, 2019 at 4:04 AM Richard Henderson
> <richard.henderson@linaro.org <mailto:richard.henderson@linaro.org>> wrote:
> 
>     > I did have a suggestion.  It was fairly detailed.
>     > https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg06522.html
> 
> Your solution is elegant at about 10 lines that return getl_ilen(pc), but it
> seems the s390 has a far simpler 
> instruction word format than the m68k. 

S390 is simpler, in that it encodes the total length into the first two bits of
the opcode.  That said, the same technique can apply.  The only difference is
the location at which you would record the length.

For s390, we do this immediately after loading the entire instruction, having
parsed the length as you saw.

For m68k, we would do this after decoding the entire instruction, as
pc_at_insn_end - pc_at_insn_start.

> However then that got me to thinking, it seems that we can call a portion of
> the TCG system to disassemble a single instruction.
>     TranslationBlock tb;
>     tb.pc = env->pc;
>     gen_intermediate_code(cs, &tb, /* max isn */ 1);
>     int ilen = tb.size;
>     printf( "PC: %08x sz:%08x\n", env->pc, tb, ilen ) ;
> 
> I am very new to TCG, so it does seem there is a lot of code in the
> translator_loop that appears to be interacting with the CPU model/state.
> Should I be worried about this, or is this a safe function to call outside of
> the translator core proper? 

No, it is not safe to call outside of the translator core, because of how this
interacts with emitting tcg opcodes.


r~

Re: [Qemu-devel] RFC: Why does target/m68k RTE insn. use gen_exception

[Lucien Murray-Pitts]

On 7/10/19 4:04 AM, Richard Henderson wrote:
> On 7/9/19 6:58 PM, Lucien Murray-Pitts wrote:
>> Any suggestions on how to obtain pc_next from the "m68k_cpu_do_interrupt(
>> CPUState *cs)" ?
test
> I did have a suggestion.  It was fairly detailed.
>
> https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg06522.html
>
>
> r~
test