Re: [Xen-devel] [PATCH V7 for-4.5 4/4] xen: Handle resumed instruction based on previous mem_event reply

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.09.14 at 15:15, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> @@ -1448,6 +1449,28 @@ bool_t p2m_mem_access_check(paddr_t gpa, unsigned long 
> gla,
>          }
>      }
>  
> +    /* The previous mem_event reply does not match the current state. */
> +    if ( v->arch.mem_event.gpa != gpa || v->arch.mem_event.eip != eip )
> +    {
> +        /* Don't emulate the current instruction, send a new mem_event. */
> +        v->arch.mem_event.emulate_flags = 0;
> +
> +        /* Make sure to mark the current state to match it again against
> +         * the new mem_event about to be sent. */

Coding style.

> +        v->arch.mem_event.gpa = gpa;
> +        v->arch.mem_event.eip = eip;
> +    }
> +
> +    if ( v->arch.mem_event.emulate_flags )
> +    {
> +        hvm_mem_event_emulate_one((v->arch.mem_event.emulate_flags &
> +                                   MEM_EVENT_FLAG_EMULATE_NOWRITE) != 0,
> +                                  TRAP_invalid_op, 
> HVM_DELIVER_NO_ERROR_CODE);
> +
> +        v->arch.mem_event.emulate_flags = 0;
> +        return 1;
> +    }
> +
>      *req_ptr = NULL;
>      req = xzalloc(mem_event_request_t);
>      if ( req )
> @@ -1502,6 +1525,59 @@ void p2m_mem_access_resume(struct domain *d)
>  
>          v = d->vcpu[rsp.vcpu_id];
>  
> +        /* Mark vcpu for skipping one instruction upon rescheduling */

Missing stop at end of sentence.

> +        if ( rsp.flags & MEM_EVENT_FLAG_EMULATE )
> +        {
> +            xenmem_access_t access;
> +            bool_t violation = 1;
> +
> +            v->arch.mem_event.emulate_flags = 0;

Do you really need to write this once here and ...

> +
> +            if ( p2m_get_mem_access(d, rsp.gfn, &access) == 0 )
> +            {
> +                switch ( access )
> +                {
> +                case XENMEM_access_n:
> +                case XENMEM_access_n2rwx:
> +                default:
> +                    violation = rsp.access_r || rsp.access_w || rsp.access_x;
> +                    break;
> +
> +                case XENMEM_access_r:
> +                    violation = rsp.access_w || rsp.access_x;
> +                    break;
> +
> +                case XENMEM_access_w:
> +                    violation = rsp.access_r || rsp.access_x;
> +                    break;
> +
> +                case XENMEM_access_x:
> +                    violation = rsp.access_r || rsp.access_w;
> +                    break;
> +
> +                case XENMEM_access_rx:
> +                case XENMEM_access_rx2rw:
> +                    violation = rsp.access_w;
> +                    break;
> +
> +                case XENMEM_access_wx:
> +                    violation = rsp.access_r;
> +                    break;
> +
> +                case XENMEM_access_rw:
> +                    violation = rsp.access_x;
> +                    break;
> +
> +                case XENMEM_access_rwx:
> +                    violation = 0;
> +                    break;
> +                }
> +            }
> +
> +            if ( violation )
> +                v->arch.mem_event.emulate_flags = rsp.flags;

... a second time here (rather making this one simply a conditional
expression)?

And I further wonder whether all the MEM_EVENT_FLAG_* values are
really potentially useful in v->arch.mem_event.emulate_flags - right
now it rather looks like this field could be a simple bool_t (likely with
a different name), which would at once make the
hvm_mem_event_emulate_one() a little better readable.

> --- a/xen/include/asm-x86/domain.h
> +++ b/xen/include/asm-x86/domain.h
> @@ -458,6 +458,15 @@ struct arch_vcpu
>  
>      /* A secondary copy of the vcpu time info. */
>      XEN_GUEST_HANDLE(vcpu_time_info_t) time_info_guest;
> +
> +    /* Should we emulate the next matching instruction on VCPU resume
> +     * after a mem_event? */

Coding style again.

Jan


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