>>> On 03.08.16 at 20:41, <tamas.leng...@zentific.com> wrote:
> The two functions monitor_traps and mem_access_send_req duplicate some of the
> same functionality. The mem_access_send_req however leaves a lot of the
> standard vm_event fields to be filled by other functions.
>
> Remove mem_access_send_req() completely, making use of monitor_traps() to
> put
> requests into the monitor ring. This in turn causes some cleanup around the
> old callsites of mem_access_send_req(). We also update monitor_traps to now
> include setting the common vcpu_id field so that all other call-sites can
> ommit
> this step.
>
> Finally, this change identifies that errors from mem_access_send_req() were
> never checked. As errors constitute a problem with the monitor ring,
> crashing the domain is the most appropriate action to take.
>
> Signed-off-by: Tamas K Lengyel <tamas.leng...@zentific.com>
> Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>
> Acked-by: Razvan Cojocaru <rcojoc...@bitdefender.com>
> ---
> Cc: Stefano Stabellini <sstabell...@kernel.org>
> Cc: Julien Grall <julien.gr...@arm.com>
> Cc: Jan Beulich <jbeul...@suse.com>
> Cc: George Dunlap <george.dun...@eu.citrix.com>
>
> v3: reduce the code movement and sanitization performed to a minimum
Doesn't this invalidate prior reviews and acks?
> @@ -1846,11 +1846,15 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned
> long gla,
> }
> }
>
> - if ( p2m_mem_access_check(gpa, gla, npfec, &req_ptr) )
> - {
> + sync = p2m_mem_access_check(gpa, gla, npfec, &req_ptr);
> +
> + if ( !sync )
> fall_through = 1;
> - } else {
> - /* Rights not promoted, vcpu paused, work here is done */
> + else
> + {
> + /*
> + * Rights not promoted (aka. sync event), work here is done
> + */
Comment style.
> @@ -1750,23 +1745,12 @@ bool_t p2m_mem_access_check(paddr_t gpa, unsigned
> long gla,
> req->u.mem_access.flags |= npfec.read_access ? MEM_ACCESS_R : 0;
> req->u.mem_access.flags |= npfec.write_access ? MEM_ACCESS_W : 0;
> req->u.mem_access.flags |= npfec.insn_fetch ? MEM_ACCESS_X : 0;
> - req->vcpu_id = v->vcpu_id;
> -
> - vm_event_fill_regs(req);
> -
> - if ( altp2m_active(v->domain) )
> - {
> - req->flags |= VM_EVENT_FLAG_ALTERNATE_P2M;
> - req->altp2m_idx = vcpu_altp2m(v).p2midx;
> - }
> }
>
> - /* Pause the current VCPU */
> - if ( p2ma != p2m_access_n2rwx )
> - vm_event_vcpu_pause(v);
> -
> - /* VCPU may be paused, return whether we promoted automatically */
> - return (p2ma == p2m_access_n2rwx);
> + /*
> + * Return whether vCPU pause is required (aka. sync event)
> + */
Again.
> + return (p2ma != p2m_access_n2rwx);
Pointless parentheses.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
