On 09/13/12 16:57, Anthony Liguori wrote:
"Michael S. Tsirkin" <m...@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.
How important is the big endian support?
lsi53c895a.c says:
/* ??? Need to check if the {read,write}[wl] routines work properly on
big-endian targets. */
I could do the same, I.E. code it up and submit it un-tested on big
endian soon. Or since I currently do not have access to any real big
endian hardware; do all testing on QEMU on QEMU.
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.
I have re-worked the variable names. For example:
if (next_chain_offset) {
MptSGEntryChain sgec;
cpu_physical_memory_read(seg_start_pa + next_chain_offset,
&sgec, sizeof(MptSGEntryChain));
assert(sgec.element_type == MPTSGENTRYTYPE_CHAIN);
next_sge_pa = sgec.segment_address_low;
if (sgec.bit_address) {
next_sge_pa |=
((uint64_t)sgec.segment_address_high) << 32;
}
seg_start_pa = next_sge_pa;
next_chain_offset = sgec.next_chain_offset *
sizeof(uint32_t);
Does this look better?
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
-Don Slutz
