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

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



07.10.2013 17:02, Julien Grall:
I think checking !page_fault is nearly everywhere is error-prone when
this function will be modified.

Can you do something like this?

if ( page_fault )
   // Your code to handle page fault
else
{
   // handle_mmio
}

It will avoid && !page_fault.
Yes, but I think at page fault condition we should check whether address belong MMIO regions (page fault at MMIO addr is possible, but we don't need that memory to transfer)

>          goto bad_data_abort;
> > rc = gva_to_ipa(info.gva, &info.gpa);
>-    if ( rc == -EFAULT )
>+    if ( rc == -EFAULT && !page_fault )
>          goto bad_data_abort;
> > /* XXX: Decode the instruction if ISS is not valid */
>-    if ( !dabt.valid )
>+    if ( !dabt.valid && !page_fault )
>          goto bad_data_abort;
> > /*
>       * Erratum 766422: Thumb store translation fault to Hypervisor may
>       * not have correct HSR Rt value.
>       */
>-    if ( cpu_has_erratum_766422() && (regs->cpsr & PSR_THUMB) && dabt.write )
>+    if ( cpu_has_erratum_766422() && (regs->cpsr & PSR_THUMB) && dabt.write
>+            && !page_fault)
>      {
>          rc = decode_instruction(regs, &info.dabt);
>          if ( rc )
>@@ -1358,6 +1361,16 @@ static void do_trap_data_abort_guest(struct 
cpu_user_regs *regs,
>          return;
>      }
> >+ /* handle permission fault on write */
>+    if ( page_fault )
>+    {
>+        if ( current->domain->arch.dirty.mode == 0 )
>+           goto bad_data_abort;
>+
>+        handle_page_fault(current->domain, info.gpa);
You must call advance_pc(regs, hsr) here.

Let me explain. I think the difference between handle_page_fault and handle_mmio is that the ongoing memory operation (that trapped here) should be repeated after handle_page_fault (the page fault handler clears read-only bit), but should be stepped out after handle_mmio (to do this we call advance_pc to increase the pc register). So, advance_pc is intentionally missed.

Best regards,
Evgeny.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

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