Re: [Qemu-riscv] [Qemu-devel] [PATCH v7 00/42] Invert Endian bit in SPARCv9 MMU TTE

[tony.nguyen]

Hi Phillippe,

On 8/16/19 7:58 PM, Philippe Mathieu-Daudé wrote:

>On 8/16/19 8:28 AM, address@hidden wrote:

>> This patchset implements the IE (Invert Endian) bit in SPARCv9 MMU TTE.

>> 

>> v7:

>[...]

>> - Re-declared many native endian devices as little or big endian. This is why

>>   v7 has +16 patches.

>

>Why are you doing that? What is the rational?

While collapsing the byte swaps, it was suggested in patch #11 of v5 that

consistent use of MemOp simplified endian comparisons. This lead to the

deprecation of enum device_endian by MemOp.

As MO_TE is conditional upon NEED_CPU_H, the s/DEVICE_NATIVE_ENDIAN/MO_TE/

required changing some device object files from common-obj-* to obj-*. In patch

#15 of v6 Paolo noted that most devices should not of been DEVICE_NATIVE_ENDIAN

and hinted at a clean up.

The +16 patches in v7 is the clean up effort.

>Anyhow if this not required by your series, you should split it out of

>it, and send it on your principal changes are merged.

>I'm worried because this these new patches involve many subsystems (thus

>maintainers) and reviewing them will now take a fair amount of time.

Yes, lets split these patches out. They are very much a tangent to the series

purpose.

>> For each device declared with DEVICE_NATIVE_ENDIAN, find the set of

>> targets from the set of target/hw/*/device.o.

>>

>> If the set of targets are all little or all big endian, re-declare

>> the device endianness as DEVICE_LITTLE_ENDIAN or DEVICE_BIG_ENDIAN

>> respectively.

>

>If only little endian targets use a device, that doesn't mean the device

>is designed in little endian...

>

>Then if a big endian target plan to use this device, it will require

>more work and you might have introduced regressions...

>

>I'm not sure this is a safe move.

>

>> This *naive* deduction may result in genuinely native endian devices

>> being incorrectly declared as little or big endian, but should not

>> introduce regressions for current targets.

>

Roger. Evidently too naive. TBH, most devices I've never heard of...

Regards,

Tony