Re: [PATCH v2] KVM: Specify byte order for KVM_EXIT_MMIO

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?

> 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.

>>> 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.

Thanks,
Victor

>>
>> And if we agree on the current proposal (from Alex/Peter/Me) is the
>> current suggested wording ok, or can someone suggest a better wording?
>>
>> We could for example include the the matrices as above for all known
>> architectures, but that does seem overly verbose.
>>
>> -Christoffer
>> _______________________________________________
>> kvmarm mailing list
>> kvmarm@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
--
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




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux