[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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





On 21/02/2022 21:10, Ayan Kumar Halder wrote:
Hi Julien,

Hi,


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.

This looks good to me with one remark below.


<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.
          */

I would shorten to:

If the instruction syndrome was invalid, then we already checked if this was due to a P2M fault. So no point to check again as the result will be the same.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.