From mboxrd@z Thu Jan  1 00:00:00 1970
From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Subject: Re: [PATCH for-4.5 v6 04/16] xen: Add vmware_port
	support
Date: Tue, 23 Sep 2014 13:16:57 -0400
Message-ID: <5421AB09.6040402@oracle.com>
References: <1411236447-7435-1-git-send-email-dslutz@verizon.com>
	<1411236447-7435-5-git-send-email-dslutz@verizon.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1411236447-7435-5-git-send-email-dslutz@verizon.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Don Slutz <dslutz@verizon.com>, xen-devel@lists.xen.org
Cc: Kevin Tian <kevin.tian@intel.com>, Keir Fraser <keir@xen.org>, Ian Campbell <ian.campbell@citrix.com>, Stefano Stabellini <stefano.stabellini@eu.citrix.com>, Jun Nakajima <jun.nakajima@intel.com>, Eddie Dong <eddie.dong@intel.com>, Ian Jackson <ian.jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>, George Dunlap <George.Dunlap@eu.citrix.com>, Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>, Jan Beulich <jbeulich@suse.com>, Andrew Cooper <andrew.cooper3@citrix.com>, Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
List-Id: xen-devel@lists.xenproject.org

On 09/20/2014 02:07 PM, Don Slutz wrote:
> @@ -2064,6 +2065,42 @@ svm_vmexit_do_vmsave(struct vmcb_struct *vmcb,
>       return;
>   }
>   
> +static void svm_vmexit_gp_intercept(struct cpu_user_regs *regs,
> +                                    struct vcpu *v)
> +{
> +    struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
> +    /*
> +     * Just use 15 for the instruction length; vmport_gp_check will
> +     * adjust it.  This is because
> +     * __get_instruction_length_from_list() has issues, and may
> +     * require a double read of the instruction bytes.  At some
> +     * point a new routine could be added that is based on the code
> +     * in vmport_gp_check with extensions to make it more general.
> +     * Since that routine is the only user of this code this can be
> +     * done later.
> +     */
> +    unsigned long inst_len = 15;

Can you add a comment describing why you chose 15?

Also, saying that __get_instruction_length_from_list() has issues I 
think requires a bit more details (e.g. that when called from #GP 
handler NRIP is not available, or that NRIP may not be available at all 
on a particular HW, leading to the need read the instruction twice --- 
once in __get_instruction_length_from_list() and then again in 
vmport_gp_check(). Which is bad because memory may change between the 
reads. Or something like that.).

-boris

> +    unsigned long inst_addr = svm_rip2pointer(v);
> +    int rc;
> +
> +    rc = vmport_gp_check(regs, v, &inst_len, inst_addr,
> +                         vmcb->exitinfo1, vmcb->exitinfo2);
> +    if ( !rc )
> +        __update_guest_eip(regs, inst_len);
> +    else
> +    {
> +        VMPORT_DBG_LOG(VMPORT_LOG_GP_UNKNOWN,
> +                       "gp: rc=%d ei1=0x%lx ei2=0x%lx ec=0x%x ip=%"PRIx64
> +                       " (0x%lx,%ld) ax=%"PRIx64" bx=%"PRIx64" cx=%"PRIx64
> +                       " dx=%"PRIx64" si=%"PRIx64" di=%"PRIx64, rc,
> +                       (unsigned long)vmcb->exitinfo1,
> +                       (unsigned long)vmcb->exitinfo2, regs->error_code,
> +                       regs->rip, inst_addr, inst_len, regs->rax, regs->rbx,
> +                       regs->rcx, regs->rdx, regs->rsi, regs->rdi);
> +        hvm_inject_hw_exception(TRAP_gp_fault, vmcb->exitinfo1);
> +    }
> +}
> +