netdev - Re: [PATCH net-next 01/14] qed: Add CONFIG_QED

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 10 May 2016 14:09:50 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	alexander.duyck@...il.com
Cc:	Yuval.Mintz@...gic.com, netdev@...r.kernel.org,
	Ariel.Elior@...gic.com
Subject: Re: [PATCH net-next 01/14] qed: Add CONFIG_QED_SRIOV

From: Alexander Duyck <alexander.duyck@...il.com>
Date: Tue, 10 May 2016 11:02:09 -0700

> On Tue, May 10, 2016 at 10:16 AM, Yuval Mintz <Yuval.Mintz@...gic.com> wrote:
>>  > From: Yuval Mintz <Yuval.Mintz@...gic.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.