From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45527) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b7JJo-00039N-Rc for qemu-devel@nongnu.org; Mon, 30 May 2016 05:17:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b7JJk-00080O-M6 for qemu-devel@nongnu.org; Mon, 30 May 2016 05:17:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39009) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b7JJk-00080E-Dp for qemu-devel@nongnu.org; Mon, 30 May 2016 05:17:00 -0400 Date: Mon, 30 May 2016 17:16:49 +0800 From: Peter Xu Message-ID: <20160530091649.GD6656@pxdev.xzpeter.org> References: <1463469353-25642-1-git-send-email-peterx@redhat.com> <1463469353-25642-8-git-send-email-peterx@redhat.com> <20160530054509.GB6656@pxdev.xzpeter.org> <574BD600.4040601@web.de> <20160530081409.GC6656@pxdev.xzpeter.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v7 07/25] intel_iommu: define several structs for IOMMU IR List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Kiarie Cc: Jan Kiszka , QEMU Developers , imammedo@redhat.com, rth@twiddle.net, ehabkost@redhat.com, jasowang@redhat.com, Marcel Apfelbaum , "Michael S. Tsirkin" , pbonzini@redhat.com, rkrcmar@redhat.com, alex.williamson@redhat.com, wexu@redhat.com On Mon, May 30, 2016 at 11:54:52AM +0300, David Kiarie wrote: > On Mon, May 30, 2016 at 11:14 AM, Peter Xu wrote: > > On Mon, May 30, 2016 at 07:56:16AM +0200, Jan Kiszka wrote: > >> On 2016-05-30 07:45, Peter Xu wrote: [...] > >> > > >> > I assume you mean when host cpu is big endian. x86 was little endian, > >> > and I was testing on x86. > >> > > >> > I think you are right. I should do conditional byte swap for all > >> > uint{16/32/64} cases within the fields. For example, index_l field in > >> > above VTD_IR_MSIAddress. And there are several other cases that need > >> > special treatment in the patchset. Will go over and fix corresponding > >> > issues in next version. > >> > >> You actually need bit-swap with bit fields, see e.g. hw/net/vmxnet3.h. > > > > Not noticed about bit-field ordering before... So maybe I need both? > > Yes, I think we will need both though, I think, byte swapping the > whole struct will break the code but swapping individual fields is > what we need. > > Myself, I'm defining bitfields as below: > > struct CMDCompletionWait { > > #ifdef __BIG_ENDIAN_BITFIELD > uint32_t type:4; /* command type */ > uint32_t reserved:8; > uint64_t store_addr:49; /* addr to write */ > uint32_t completion_flush:1; /* allow more executions */ > uint32_t completion_int:1; /* set MMIOWAITINT */ > uint32_t completion_store:1; /* write data to address */ I guess what we need might be this one: uint64_t type:4; /* command type */ uint64_t reserved:8; uint64_t store_addr:49; /* addr to write */ uint64_t completion_flush:1; /* allow more executions */ uint64_t completion_int:1; /* set MMIOWAITINT */ uint64_t completion_store:1; /* write data to address */ IIUC, if we define type:4 as uint32_t rather than uint64_t, it should be bits [29:32] of the struct on big endian machines, not bits [61:64]. Thanks, -- peterx