Re: [PATCH v4 8/9] xen/arm: Implement hypercall for dirty page tracing

[Eugene Fedotov <e.fedotov@samsung.com>]

If we move necessary checking into handle_page_fault routine, so the 
resulting patch for traps.c will look more simple:

--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1313,6 +1313,8 @@ static void do_trap_data_abort_guest(struct 
cpu_user_regs *regs,
      const char *msg;
      int rc, level = -1;
      mmio_info_t info;
+    int page_fault = ( (dabt.dfsc & FSC_MASK) ==
+                          (FSC_FLT_PERM | FSC_3D_LEVEL) && dabt.write );

      if ( !check_conditional_instr(regs, hsr) )
      {
@@ -1334,6 +1336,13 @@ static void do_trap_data_abort_guest(struct 
cpu_user_regs *regs,
      if ( rc == -EFAULT )
          goto bad_data_abort;

+    /* domU page fault handling for guest live migration */
+    /* dabt.valid can be 0 here */
+    if ( page_fault && handle_page_fault(current->domain, info.gpa) )
+    {
+        /* Do not modify pc after page fault to repeat memory operation */
+        return;
+    }
      /* XXX: Decode the instruction if ISS is not valid */
      if ( !dabt.valid )
          goto bad_data_abort;

Will it be acceptable, or you think "else" statement looks more better?

Where handle_page_fault returns zero if dirty page tracing is not 
enabled for domain (dirty.mode==0) or address is not valid for domain 
(having memory map we check it), so MMIO and faults from dom0 are not 
handled by handle_page_fault.

The handle_page_fault routine (see  [PATCH v4 7/9] xen/arm: Implement 
virtual-linear page table for guest p2m mapping in live migration) will be:

  int handle_page_fault(struct domain *d, paddr_t addr)
{

     lpae_t *vlp2m_pte = 0;
     vaddr_t start, end;

     if (!d->arch.dirty.mode) return 0;
     /* Ensure that addr is inside guest's RAM */
     get_gma_start_end(d, &start, &end);
     if ( addr < start || addr > end ) return 0;

     vlp2m_pte = get_vlpt_3lvl_pte(addr);
     if ( vlp2m_pte->p2m.valid && vlp2m_pte->p2m.write == 0 )
     {
         lpae_t pte = *vlp2m_pte;
         pte.p2m.write = 1;
         write_pte(vlp2m_pte, pte);
         flush_tlb_local();

         /* in order to remove mappings in cleanup stage */
         add_mapped_vaddr(d, addr);
     }

     return 1;
}

Best,
Evgeny.