From: Alexander Duyck <alexander.du...@gmail.com> Date: Tue, 10 May 2016 11:02:09 -0700
> On Tue, May 10, 2016 at 10:16 AM, Yuval Mintz <yuval.mi...@qlogic.com> wrote: >> > From: Yuval Mintz <yuval.mi...@qlogic.com> >>> Date: Mon, 9 May 2016 16:19:10 +0300 >>> >>> > + /* bitmap indicating which fields hold valid values */ >>> > + aligned_u64 valid_bitmap; >>> >>> There is absolutely no reason to use aligned_u64 here. That type is for >>> handling >>> a specific issue in user facing APIs, which this is not. >> >> I'm not entirely convinced this is true; If we'll not enforce the alignment >> of this 64-bit field, it's possible there will be differences between 32-bit >> and 64-bit machines versions of this struct. >> You have to recall that this is going to be copied via DMA between PF and VF, >> so they must have the exact same representation of the structure. >> >> [If I'm wrong on the technical part here, please correct me; I vaguely >> seem to recall that this was already discussed on bnx2x's implementation >> of the hw-channel which also uses aligned u64 fields] > > I think your change does have an impact, I just don't know if you > really realize what it will get you. Specifically what using the > aligned_u64 is doing is forcing qed_bulletin_content to be u64 aligned > and introducing two holes in qed_bulletin on 32 bit platforms where > dma_addr_t might be 32 bit, and adding a 4 bytes of padding after > size. dma_addr_t is 32-bits on some 64-bit architectures too.