Re: [XEN v8 2/2] xen/arm64: io: Support instructions (for which ISS is not valid) on emulated MMIO region using MMIO/ioreq handler

[Ayan Kumar Halder <ayan.kumar.halder@xilinx.com>]

Hi Julien,

On 21/02/2022 19:13, Julien Grall wrote:
>
>
> On 21/02/2022 19:05, Ayan Kumar Halder wrote:
>> If we (ie Xen) didn't decode the instruction manually, then 
>> check_p2m() has not been invoked yet.  This is because of the 
>> following (info.dabt.valid == True) :-
>>
>>          if ( !is_data || !info.dabt.valid )
>>          {
>>
>>                  ...
>>
>>                  if ( check_p2m(is_data, gpa) )
>>                      return;
>>
>>                  ...
>>
>>          }
>>
>> So, in this scenario ( !info.dabt.valid), it would not be necessary 
>> to invoke check_p2m() after try_handle_mmio().
>>
>> However, if we havenot decoded the instruction manually (ie 
>> info.dabt.valid == True), and try_handle_mmio() returns IO_UNHANDLED, 
>> then it will be necessary to invoke "check_p2m(is_data, gpa)"
>
> Hmmm you are right. But this doesn't seem to match the code you wrote 
> below. What did I miss?

My code was not correct.  I have rectified it as below. Please let me 
know if it looks sane.

<snip>

     case FSC_FLT_TRANS:
     {
         info.gpa = gpa;
         info.dabt = hsr.dabt;

         /*
          * Assumption :- Most of the times when we get a data abort and 
the ISS
          * is invalid or an instruction abort, the underlying cause is 
that the
          * page tables have not been set up correctly.
          */
         if ( !is_data || !info.dabt.valid )
         {
             if ( check_p2m(is_data, gpa) )
                 return;

             /*
              * If the instruction abort could not be resolved by 
setting the
              * appropriate bits in the translation table, then Xen should
              * forward the abort to the guest.
              */
             if ( !is_data )
                 goto inject_abt;

             try_decode_instruction(regs, &info);

             /*
              * If Xen could not decode the instruction or encountered 
an error
              * while decoding, then it should forward the abort to the 
guest.
              */
             if ( info.dabt_instr.state == INSTR_ERROR )
                 goto inject_abt;
         }

         state = try_handle_mmio(regs, &info);

         switch ( state )
         {
             case IO_ABORT:
                 goto inject_abt;
             case IO_HANDLED:
                 /*
                  * If the instruction was decoded and has executed 
successfully
                  * on the MMIO region, then Xen should execute the next 
part of
                  * the instruction. (for eg increment the rn if it is a
                  * post-indexing instruction.
                  */
                 post_increment_register(&info.dabt_instr);
                 advance_pc(regs, hsr);
                 return;
             case IO_RETRY:
                 /* finish later */
                 return;
             case IO_UNHANDLED:
                 /* IO unhandled, try another way to handle it. */
                 break;
         }

         /*
          * If the instruction was valid but Xen could not emulate the 
instruction
          * then it should configure the page tables to set the correct 
page table
          * entry corresponding to the faulting address. If it was 
successful, it
          * should return to the guest to retry the instruction (hoping 
that the
          * instruction will not be trapped to Xen again).
          * However, if Xen was not successful in setting the page 
tables, then
          * it should forward the abort to the guest.
          */
         if ( info.dabt.valid && check_p2m(is_data, gpa) )
             return;

         break;
     }
     default:

<snip>

- Ayan

>
> Cheers,
>