On 26.01.2014, at 06:43, Victor Kamensky <victor.kamensky@xxxxxxxxxx> wrote:
> On 25 January 2014 19:46, Victor Kamensky <victor.kamensky@xxxxxxxxxx> wrote:
>> On 25 January 2014 10:31, Christoffer Dall <christoffer.dall@xxxxxxxxxx> wrote:
>>> On Sat, Jan 25, 2014 at 04:23:00PM +0000, Peter Maydell wrote:
>>>> On 25 January 2014 02:15, Alexander Graf <agraf@xxxxxxx> wrote:
>>>>> Ok, let's go through the combinations for a 32-bit write of 0x01020304 on PPC and what data[] looks like
>>
>> Alex, could you please for the clarification purpose, tell
>> us what instructions BE guest and LE guest run in below
>> cases. Suppose r0 register has device address, and r1
>> register has word as 0x01020304. And suppose we have
>> regular memory instead of io at r0 address. How do you
>> see 'unsigned char data[4]' will look like if 'data'
>> address is at r0.
>>
>> For this, for a moment, I think, we can forget about KVM,
>> just consider BE guest and LE guest running on regular
>> h/w.
>>
>> Next we will consider cases as below when the same
>> BE guest and LE guest runs on top of BE host or LE
>> host KVM in different proposals combinations. Effectively
>> defining non-virtual examples first and consider how they
>> will work under KVM.
>>
>> I was thinking that that you consider
>>
>> BE guest:
>> stw r1, 0, r0
>> just write r1 at r0 address
>>
>> LE guest
>> stwbrx r1, 0, r0
>> stwbrx - word byteswap r1 value and write it at r0 address
>>
>> which would produce at r0 address
>> data[] = {0x01, 0x02, 0x03, 0x04}
>> for both above cases, and have the same effect
>> on device if its register is mapped at r0 address.
>>
>> But judging by your other response in previous email, I
>> think you have something different in mind. Please
>> describe it.
>
> Alex, I reread your tables again. I think that just
> 'stw r1, 0, r0' for both LE and BE will match them. As
> far as data[] concerned in BE it will be data[] = {0x01,
> 0x02, 0x03, 0x04}, and in LE it wil lbe data[] = {0x04,
> 0x03, 0x02, 0x01}.
>
> Do we agree that these are two different memory
> transactions, as far as device concerned?
Not sure I understand what you mean by that. "stw" is "stw" regardless of the MSR.LE setting. The only reason we care about all of this in KVM is that we implement an instruction emulator that needs to do what the CPU does - and that one interprets MSR.LE.
>
>> Thanks,
>> Victor
>>
>>>>> your proposal:
>>>>>
>>>>> BE guest, BE host: { 0x01, 0x02, 0x03, 0x04 }
>>>>> LE guest, BE host: { 0x04, 0x03, 0x02, 0x01 }
>>>>> BE guest, LE host: { 0x01, 0x02, 0x03, 0x04 }
>>>>> LE guest, LE host: { 0x04, 0x03, 0x02, 0x01 }
>>>>>
>>>>> -> ldw_p() will give us the correct value to work with
>>>>>
>>>>> current proposal:
>>>>>
>>>>> BE guest, BE host: { 0x01, 0x02, 0x03, 0x04 }
>>>>> LE guest, BE host: { 0x04, 0x03, 0x02, 0x01 }
>>>>> BE guest, LE host: { 0x04, 0x03, 0x02, 0x01 }
>>>>> LE guest, LE host: { 0x01, 0x02, 0x03, 0x04 }
>>>>>
>>>>> -> *(uint32_t*)data will give us the correct value to work with
>>>>
>>>> For completeness, ditto, ARM:
>>>> Scott's proposal (you end up with the same values in
>>>> the data array as for PPC, so userspace has to know the
>>>> "default" endianness so it can do a byteswap
>
> Actually userland need to mange mismatch between
> native internal data of simulated device and device
> endianity. If native endianity mismatches device qemu
> need to do byteswap. Otherwise it will be just copy it
> into mmio.data as bytes array.
> In qemu there is already infrastructure that handles
> that. My ARM BE KVM patches use Scott's mmio.
> data[] definition and I did not do anything in qemu
> to make that work.
Right, that's what I indicated with my original email. Scott's proposal makes current QEMU "just work" while our proposal needs QEMU changes.
>
>>>> if the
>>>> process endianness doesn't match it; on QEMU
>>>> ldl_p() handles this for us, as you say):
>>>>
>>>> BE guest, BE host: { 0x01, 0x02, 0x03, 0x04 }
>>>> LE guest, BE host: { 0x04, 0x03, 0x02, 0x01 }
>>>> BE guest, LE host: { 0x01, 0x02, 0x03, 0x04 }
>>>> LE guest, LE host: { 0x04, 0x03, 0x02, 0x01 }
>
> Peter, thank you for adding ARM case. It really
> helps to see the difference between definitions.
>
> Don't you agree that Scott's definition has advantage
> because its interpretation does not depend on
> CPU type?
>
> It is always the same for the same 'device access
> operation', guest endianity, host endianity. If one
> look at 'stw r1, 0, r0' in BE guest, and 'stwbrx r1,
> 0, r0' in LE guest, which is the same memory
> transaction as device concerned. Table using
> Scott's definition will always look like
>
> LE guest, BE host: { 0x01, 0x02, 0x03, 0x04 }
> LE guest, BE host: { 0x01, 0x02, 0x03, 0x04 }
> BE guest, LE host: { 0x01, 0x02, 0x03, 0x04 }
> LE guest, LE host: { 0x01, 0x02, 0x03, 0x04 }
>
> if ARM would work with this device to do the
> same thing, its table will look exactly the same.
>
>>>>
>>>> current proposal:
>>>>
>>>> BE guest, BE host: { 0x04, 0x03, 0x02, 0x01 }
>>>> LE guest, BE host: { 0x01, 0x02, 0x03, 0x04 }
>>>> BE guest, LE host: { 0x01, 0x02, 0x03, 0x04 }
>>>> LE guest, LE host: { 0x04, 0x03, 0x02, 0x01 }
>>>>
>>>> The values in the data array are different than on
>>>> PPC, reflecting the fact that the "default" endianness
>>>> is different; userspace does
>>>>
>>>> -> *(uint32_t*)data will give us the correct value to work with
>>>>
>>>> regardless of what the guest CPU arch is.
>>>>
>>>>> There are pros and cons for both approaches.
>>>>>
>>>>> Pro approach 1 is that it fits the way data[] is read today,
>>>>> so no QEMU changes are required. However, it means
>>>>> that user space needs to have awareness of the
>>>>> "default endianness".
>>>>> With approach 2 you don't care about endianness at all
>>>>> anymore - you just get a payload that the host process
>>>>> can read in.
>>>>>
>>>>> Obviously both approaches would work as long as they're
>>>>> properly defined :).
>>>>
>>>> Agreed with all of that.
>>>>
>>> Agreed as well.
>>>
>>> How do we settle on one versus the other?
>
> Not surprisingly, I vote for Scott's proposal :). ARM BE KVM
> patches previously posted use this definition of mmio.data[].
>
> Scott's definition interpretation does not depend on CPU type.
> It is much simpler. It does not use notion not very well defined
> like "real bus". it does not use word 'endianness', byteswap,
> cast example, which IMHO makes it more robust.
The more I think about it the more I tend to agree.
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
