From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755395AbbLDWbs (ORCPT ); Fri, 4 Dec 2015 17:31:48 -0500 Received: from smtp-outbound-2.vmware.com ([208.91.2.13]:42677 "EHLO smtp-outbound-2.vmware.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751869AbbLDWbq (ORCPT ); Fri, 4 Dec 2015 17:31:46 -0500 From: "Sinclair Yeh" Date: Fri, 4 Dec 2015 14:33:08 -0800 To: "H. Peter Anvin" Cc: x86@kernel.org, Thomas Gleixner , Ingo Molnar , Alok Kataria , linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, Xavier Deguillard Subject: Re: [PATCH 1/6] x86: Add VMWare Host Communication Macros Message-ID: <20151204223308.GA28421@syeh-linux> References: <1449008047-8252-1-git-send-email-syeh@vmware.com> <1449008332-9394-1-git-send-email-syeh@vmware.com> <565E23E7.5090800@zytor.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <565E23E7.5090800@zytor.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Thanks Peter. On Tue, Dec 01, 2015 at 02:49:11PM -0800, H. Peter Anvin wrote: > On 12/01/15 14:18, Sinclair Yeh wrote: > > These macros will be used by multiple VMWare modules for handling > > host communication. > > > + __asm__ __volatile__ ("inl %%dx" : \ > > This is odd at best; the standard assembly form of this instruction is: > > inl (%dx),%eax Ok, I'm not sure why this worked and compiled before. Fixed. > > Also, we don't need the underscored forms of asm and volatile for kernel > code. > > > + __asm__ __volatile__ ("movq %13, %%rbp;" \ > > + "cld; rep outsb; " \ > > + "movq %%rbp, %6" : \ > > cld shouldn't be necessary here, DF=0 is part of the normal ABI environment. > > You also don't save/restore %rbp here, but you do below? Seems very odd. Good catch. Fixed. > > It might be better do so something like: > > +#define VMW_PORT_HB_OUT(in1, in2, port_num, magic, \ > + eax, ebx, ecx, edx, si, di, bp) \ > +({ \ > + __asm__ __volatile__ ("xchgq %6, %%rbp;" \ > + "cld; rep outsb; " \ > + "xchgq %%rbp, %6" : \ > + "=a"(eax), \ > + "=b"(ebx), \ > + "=c"(ecx), \ > + "=d"(edx), \ > + "+S"(si), \ > + "+D"(di), \ > + "+r"(bp) : \ > + "a"(magic), \ > + "b"(in1), \ > + "c"(in2), \ > + "d"(port_num) : \ > + "memory", "cc"); \ > +}) This is great. Changed. Updated patch set incoming. Sinclair From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Sinclair Yeh" Subject: Re: [PATCH 1/6] x86: Add VMWare Host Communication Macros Date: Fri, 4 Dec 2015 14:33:08 -0800 Message-ID: <20151204223308.GA28421@syeh-linux> References: <1449008047-8252-1-git-send-email-syeh@vmware.com> <1449008332-9394-1-git-send-email-syeh@vmware.com> <565E23E7.5090800@zytor.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <565E23E7.5090800@zytor.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org To: "H. Peter Anvin" Cc: Xavier Deguillard , x86@kernel.org, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, Ingo Molnar , Thomas Gleixner , Alok Kataria List-Id: virtualization@lists.linuxfoundation.org Thanks Peter. On Tue, Dec 01, 2015 at 02:49:11PM -0800, H. Peter Anvin wrote: > On 12/01/15 14:18, Sinclair Yeh wrote: > > These macros will be used by multiple VMWare modules for handling > > host communication. > > > + __asm__ __volatile__ ("inl %%dx" : \ > > This is odd at best; the standard assembly form of this instruction is: > > inl (%dx),%eax Ok, I'm not sure why this worked and compiled before. Fixed. > > Also, we don't need the underscored forms of asm and volatile for kernel > code. > > > + __asm__ __volatile__ ("movq %13, %%rbp;" \ > > + "cld; rep outsb; " \ > > + "movq %%rbp, %6" : \ > > cld shouldn't be necessary here, DF=0 is part of the normal ABI environment. > > You also don't save/restore %rbp here, but you do below? Seems very odd. Good catch. Fixed. > > It might be better do so something like: > > +#define VMW_PORT_HB_OUT(in1, in2, port_num, magic, \ > + eax, ebx, ecx, edx, si, di, bp) \ > +({ \ > + __asm__ __volatile__ ("xchgq %6, %%rbp;" \ > + "cld; rep outsb; " \ > + "xchgq %%rbp, %6" : \ > + "=a"(eax), \ > + "=b"(ebx), \ > + "=c"(ecx), \ > + "=d"(edx), \ > + "+S"(si), \ > + "+D"(di), \ > + "+r"(bp) : \ > + "a"(magic), \ > + "b"(in1), \ > + "c"(in2), \ > + "d"(port_num) : \ > + "memory", "cc"); \ > +}) This is great. Changed. Updated patch set incoming. Sinclair