From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59871) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bkyLl-0002VU-E8 for qemu-devel@nongnu.org; Fri, 16 Sep 2016 14:59:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bkyLh-0000qa-BK for qemu-devel@nongnu.org; Fri, 16 Sep 2016 14:59:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60738) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bkyLh-0000qW-1Q for qemu-devel@nongnu.org; Fri, 16 Sep 2016 14:58:57 -0400 Date: Fri, 16 Sep 2016 21:58:54 +0300 From: "Michael S. Tsirkin" Message-ID: <20160916215555-mutt-send-email-mst@kernel.org> References: <1472660263-4524-1-git-send-email-davidkiarie4@gmail.com> <1472660263-4524-4-git-send-email-davidkiarie4@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1472660263-4524-4-git-send-email-davidkiarie4@gmail.com> Subject: Re: [Qemu-devel] [V17 3/4] hw/i386: Introduce AMD IOMMU List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Kiarie Cc: qemu-devel@nongnu.org, jan.kiszka@web.de, rkrcmar@redhat.com, peterx@redhat.com, ehabkost@redhat.com On Wed, Aug 31, 2016 at 07:17:42PM +0300, David Kiarie wrote: > +/* serialize IOMMU command processing */ > +typedef struct QEMU_PACKED { > +#ifdef HOST_WORDS_BIGENDIAN > + 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 */ > +#else > + uint64_t completion_store:1; > + uint64_t completion_int:1; > + uint64_t completion_flush:1; > + uint64_t store_addr:49; > + uint64_t reserved:8; > + uint64_t type:4; > +#endif /* __BIG_ENDIAN_BITFIELD */ > + uint64_t store_data; /* data to write */ > +} CMDCompletionWait; > + > +/* invalidate internal caches for devid */ > +typedef struct QEMU_PACKED { > +#ifdef HOST_WORDS_BIGENDIAN > + uint64_t devid:16; /* device to invalidate */ > + uint64_t reserved_1:44; > + uint64_t type:4; /* command type */ > +#else > + uint64_t devid:16; > + uint64_t reserved_1:44; > + uint64_t type:4; > +#endif /* __BIG_ENDIAN_BITFIELD */ > + uint64_t reserved_2; > +} CMDInvalDevEntry; > + > +/* invalidate a range of entries in IOMMU translation cache for devid */ > +typedef struct QEMU_PACKED { > +#ifdef HOST_WORDS_BIGENDIAN > + uint64_t type:4; /* command type */ > + uint64_t reserved_2:12 > + uint64_t domid:16; /* domain to inval for */ > + uint64_t reserved_1:12; > + uint64_t pasid:20; > +#else > + uint64_t pasid:20; > + uint64_t reserved_1:12; > + uint64_t domid:16; > + uint64_t reserved_2:12; > + uint64_t type:4; > +#endif /* __BIG_ENDIAN_BITFIELD */ > + > +#ifdef HOST_WORDS_BIGENDIAN > + uint64_t address:51; /* address to invalidate */ > + uint64_t reserved_3:10; > + uint64_t guest:1; /* G/N invalidation */ > + uint64_t pde:1; /* invalidate cached ptes */ > + uint64_t size:1 /* size of invalidation */ > +#else > + uint64_t size:1; > + uint64_t pde:1; > + uint64_t guest:1; > + uint64_t reserved_3:10; > + uint64_t address:51; > +#endif /* __BIG_ENDIAN_BITFIELD */ > +} CMDInvalIommuPages; > + > +/* inval specified address for devid from remote IOTLB */ > +typedef struct QEMU_PACKED { > +#ifdef HOST_WORDS_BIGENDIAN > + uint64_t type:4; /* command type */ > + uint64_t pasid_19_6:4; > + uint64_t pasid_7_0:8; > + uint64_t queuid:16; > + uint64_t maxpend:8; > + uint64_t pasid_15_8; > + uint64_t devid:16; /* related devid */ > +#else > + uint64_t devid:16; > + uint64_t pasid_15_8:8; > + uint64_t maxpend:8; > + uint64_t queuid:16; > + uint64_t pasid_7_0:8; > + uint64_t pasid_19_6:4; > + uint64_t type:4; > +#endif /* __BIG_ENDIAN_BITFIELD */ > + > +#ifdef HOST_WORDS_BIGENDIAN > + uint64_t address:52; /* invalidate addr */ > + uint64_t reserved_2:9; > + uint64_t guest:1; /* G/N invalidate */ > + uint64_t reserved_1:1; > + uint64_t size:1; /* size of invalidation */ > +#else > + uint64_t size:1; > + uint64_t reserved_1:1; > + uint64_t guest:1; > + uint64_t reserved_2:9; > + uint64_t address:52; > +#endif /* __BIG_ENDIAN_BITFIELD */ > +} CMDInvalIOTLBPages; > + > +/* invalidate all cached interrupt info for devid */ > +typedef struct QEMU_PACKED { > +#ifdef HOST_WORDS_BIGENDIAN > + uint64_t type:4; /* command type */ > + uint64_t reserved_1:44; > + uint64_t devid:16; /* related devid */ > +#else > + uint64_t devid:16; > + uint64_t reserved_1:44; > + uint64_t type:4; > +#endif /* __BIG_ENDIAN_BITFIELD */ > + uint64_t reserved_2; > +} CMDInvalIntrTable; > + > +/* load adddress translation info for devid into translation cache */ > +typedef struct QEMU_PACKED { > +#ifdef HOST_WORDS_BIGENDIAN > + uint64_t type:4; /* command type */ > + uint64_t reserved_2:8; > + uint64_t pasid_19_0:20; > + uint64_t pfcount_7_0:8; > + uint64_t reserved_1:8; > + uint64_t devid:16; /* related devid */ > +#else > + uint64_t devid:16; > + uint64_t reserved_1:8; > + uint64_t pfcount_7_0:8; > + uint64_t pasid_19_0:20; > + uint64_t reserved_2:8; > + uint64_t type:4; > +#endif /* __BIG_ENDIAN_BITFIELD */ > + > +#ifdef HOST_WORDS_BIGENDIAN > + uint64_t address:52; /* invalidate address */ > + uint64_t reserved_5:7; > + uint64_t inval:1; /* inval matching entries */ > + uint64_t reserved_4:1; > + uint64_t guest:1; /* G/N invalidate */ > + uint64_t reserved_3:1; > + uint64_t size:1; /* prefetched page size */ > +#else > + uint64_t size:1; > + uint64_t reserved_3:1; > + uint64_t guest:1; > + uint64_t reserved_4:1; > + uint64_t inval:1; > + uint64_t reserved_5:7; > + uint64_t address:52; > +#endif /* __BIG_ENDIAN_BITFIELD */ > +} CMDPrefetchPages; > + > +/* clear all address translation/interrupt remapping caches */ > +typedef struct QEMU_PACKED { > +#ifdef HOST_WORDS_BIGENDIAN > + uint64_t type:4; /* command type */ > + uint64_t reserved_1:60; > +#else > + uint64_t reserved_1:60; > + uint64_t type:4; > +#endif /* __BIG_ENDIAN_BITFIELD */ > + uint64_t reserved_2; > +} CMDInvalIommuAll; > + > +/* issue a PCIe completion packet for devid */ > +typedef struct QEMU_PACKED { > + uint32_t reserved_1:16; > + uint32_t devid:16; > + > +#ifdef HOST_WORDS_BIGENDIAN > + uint32_t type:4; /* command type */ > + uint32_t reserved_2:8; > + uint32_t pasid_19_0:20 > +#else > + uint32_t pasid_19_0:20; > + uint32_t reserved_2:8; > + uint32_t type:4; > +#endif /* __BIG_ENDIAN_BITFIELD */ > + > +#ifdef HOST_WORDS_BIGENDIAN > + uint32_t reserved_3:29; > + uint32_t guest:1; > + uint32_t reserved_4:2; > +#else > + uint32_t reserved_3:2; > + uint32_t guest:1; > + uint32_t reserved_4:29; > +#endif /* __BIG_ENDIAN_BITFIELD */ > + > + uint32_t completion_tag:16; /* PCIe PRI information */ > + uint32_t reserved_5:16; > +} CMDCompletePPR; So as Peter points, out, we don't do this kind of thing. Pls drop this ifdefery and rework using masks and cpu_to_le. E.g. > +#ifdef HOST_WORDS_BIGENDIAN > + uint32_t reserved_3:29; > + uint32_t guest:1; > + uint32_t reserved_4:2; > +#else > + uint32_t reserved_3:2; > + uint32_t guest:1; > + uint32_t reserved_4:29; > +#endif /* __BIG_ENDIAN_BITFIELD */ guest = 1; -> #define GUEST cpu_to_le32(0x1 << 2) uint32_t guest; guest = GUEST; -- MST