From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:55416)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <anthony@codemonkey.ws>) id 1TCGTO-0003xx-VR
	for qemu-devel@nongnu.org; Thu, 13 Sep 2012 16:57:19 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <anthony@codemonkey.ws>) id 1TCGTN-0004Sn-W6
	for qemu-devel@nongnu.org; Thu, 13 Sep 2012 16:57:18 -0400
Received: from mail-ob0-f173.google.com ([209.85.214.173]:51376)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <anthony@codemonkey.ws>) id 1TCGTN-0004Si-RU
	for qemu-devel@nongnu.org; Thu, 13 Sep 2012 16:57:17 -0400
Received: by obbta14 with SMTP id ta14so4862428obb.4
	for <qemu-devel@nongnu.org>; Thu, 13 Sep 2012 13:57:17 -0700 (PDT)
From: Anthony Liguori <anthony@codemonkey.ws>
In-Reply-To: <20120913135426.GB7640@redhat.com>
References: <1347382813-5662-1-git-send-email-Don@CloudSwitch.com>
	<20120913135426.GB7640@redhat.com>
Date: Thu, 13 Sep 2012 15:57:12 -0500
Message-ID: <87ligdbpzr.fsf@codemonkey.ws>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Subject: Re: [Qemu-devel] [PATCH] hw: Add support for new LSI Logic devices.
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Michael S. Tsirkin" <mst@redhat.com>, Don Slutz <Don@cloudswitch.com>
Cc: qemu-devel@nongnu.org

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Tue, Sep 11, 2012 at 01:00:13PM -0400, Don Slutz wrote:
>> +            if (next_chain_offset) {
>> +                MptSGEntryChain sgec;
>> +                cpu_physical_memory_read(seg_start_pa + next_chain_offset,
>> +                        &sgec, sizeof(MptSGEntryChain));
>> +                assert(sgec.u2ElementType == MPTSGENTRYTYPE_CHAIN);
>> +                next_sge_pa = sgec.u32SegmentAddressLow;
>> +                if (sgec.f64BitAddress) {
>> +                    next_sge_pa |=
>> +                        ((uint64_t)sgec.u32SegmentAddressHigh) << 32;
>> +                }
>> +                seg_start_pa = next_sge_pa;
>> +                next_chain_offset = sgec.u8NextChainOffset * sizeof(uint32_t);
>
> BTW all this logic seems wrong on big endian.
> Maybe we don't care short term but we do long term.

I think we care short term.  It's easy enough to copy and past but i'm
not inclined to believe this will be maintained longer term if someone
makes the investment to de-uglify things.

I can tolerate a lot, but I'm not going to pull something with variable
names of 'u8NextChainOffset' :-)  Changing this in tree is just
unnecessary churn.

Regards,

Anthony Liguori

> I think you need to fix it up using le_to_cpu or something.
> And in particular this likely means bitfields can not be used cleanly,
> so you will not be able to resync lsilogic.h from virtualbox.
> The implication I guess is that we should just fix up the style
> to match qemu.
>
> -- 
> MST