On 23.11.2017 11:08, Cornelia Huck wrote:
> On Thu, 23 Nov 2017 11:01:23 +0100
> Thomas Huth <th...@redhat.com> wrote:
>
>> On 23.11.2017 10:49, Cornelia Huck wrote:
>>> On Thu, 23 Nov 2017 09:48:41 +0100
>>> Thomas Huth <th...@redhat.com> wrote:
>>>> On 22.11.2017 23:05, Pierre Morel wrote:
[...]
>>>>> +/**
>>>>> + * Swap data contained in s390x big endian registers to little endian
>>>>> + * PCI bars.
>>>>> + *
>>>>> + * @ptr: a pointer to a uint64_t data field
>>>>> + * @len: the length of the valid data, must be 1,2,4 or 8
>>>>> + */
>>>>> +static int zpci_endian_swap(uint64_t *ptr, uint8_t len)
>>>>> +{
>>>>> + uint64_t data = *ptr;
>>>>> +
>>>>> + switch (len) {
>>>>> + case 1:
>>>>> + break;
>>>>> + case 2:
>>>>> + data = bswap16(data);
>>>>> + break;
>>>>> + case 4:
>>>>> + data = bswap32(data);
>>>>> + break;
>>>>> + case 8:
>>>>> + data = bswap64(data);
>>>>> + break;
>>>>> + default:
>>>>> + return -EINVAL;
>>>>> + }
>>>>> + *ptr = data;
>>>>> + return 0;
>>>>> +}
>>>>
>>>> While you're at it, I think that should rather be leXX_to_cpu() instead
>>>> of bswapXX() here,
>>>
>>> I don't think that's correct, as this is supposed to swap BE registers
>>> to LE PCI bars.
>>
>> Yes, but for the CPU emulation, the registers are stored in the host's
>> endianness in the CPUS390XState structure. Or why do we byte-swap them
>> again with cpu_to_be64() during s390_store_status(), for example?
>
> Gah, endian conversion is eating my brain...
>
> So, is the content we get BE or not? I thought in our last discussion
> we came to the conclusion that it is.
data is read from / written to env->regs[r1], so this is host endian, as
far as I know. PCI is little endian, so using le32_to_cpu() /
cpu_to_le32() should IMHO be the right way to go here.
By the way, if we want to use both, cpu_to_le and le_to_cpu, depending
on whether we read from or write to PCI, we should maybe *not* put this
code into a separate function?
> [I really need to continue working on wiring up zpci in tcg, but I keep
> getting sidetracked.]
Maybe best if you get it running on a big endian host first ... if it is
then not working on a little endian host, you know that you have to look
for things like these "bswapXX()" statements...
Thomas
