Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian

[Alexander Graf <agraf@suse.de>]

On 05/07/2014 02:40 PM, Greg Kurz wrote:
> On Wed, 07 May 2014 13:54:36 +0200
> Alexander Graf <agraf@suse.de> wrote:
>> On 05/07/2014 12:19 PM, Greg Kurz wrote:
>>> On Wed, 7 May 2014 11:41:10 +0200
>>> Alexander Graf <agraf@suse.de> wrote:
>>>
>>>>> Am 07.05.2014 um 11:26 schrieb Peter Maydell <peter.maydell@linaro.org>:
>>>>>
>>>>>> On 7 May 2014 10:09, Alexander Graf <agraf@suse.de> wrote:
>>>>>> I don't think we should overengineer hacks for legacy virtio.
>>>>> Agreed. So what's our final conclusion: virtio endianness
>>>>> is the endianness of the guest kernel at the point where
>>>>> it triggers a reset of the virtio device, yes?
>>>> I just realized we're talking about virtio in a non-virtio thread. This patch set is about core dump support which is different from virtio bi-endian support. While both may end up at the same logic, I don't like the idea to mix them. This function is PPC internal.
>>>>
>>>> Alex
>>>>
>>> Correct and now I have this feeling about using LPCR_ILE versus MSR_LE...
>>>
>>> LPCR_ILE reflects the interrupt vector endianness. It is set during early boot
>>> by the guest kernel according to the desired endianness. MSR_LE gives the
>>> current endian mode for the cpu.
>>>
>>> The idea is that you need to rely on LPCR_ILE when you peek from the host
>>> because you lack context and MSR_LE may be have been temporarily changed.
>>> This is clearly the case for dump support.
>>>
>>> Now when it comes to virtio, we cache the endianness at device reset time: MSR_LE from
>>> current_cpu should reflect the guest kernel endianness, no ?
>>>
>>> In this case we could end up like what's being currently discussed with ARM:
>>>
>>> http://www.spinics.net/lists/kvm-arm/msg09099.html
>>> http://www.spinics.net/lists/kvm-arm/msg09091.html
>>>
>>> Alex,
>>>
>>> If we agree that current_cpu->MSR_LE does the job when the guest kernel resets
>>> the device, then I guess we don't even need this patch...
>> Either one works for me as long as we put it into a spec. No solution
>> will be able to fulfill all cases.
>>
>> The uglyness about the current_cpu bit is that devices are usually not
>> supposed to know about the cpu accesses come from usually. But then
>> again devices shouldn't know about the endianness of a cpu either so I
>> guess it's ok to breach layers here.
>>
>> Rusty, do you have strong feelings either way?
>>
>>
>> Alex
>>
> Coming back to the initial subject. What about changing the last sentence
> in the commit message to the following ?
>
> "And when it comes to dumping a guest, the information is needed to write
>   ELF headers using the kernel endianness."
>
> This would make the patch a PPC64 dump only business and end the debate. :)
>
> Tell me if you want me to repost.

Let's wait for Tom's ack first - or if you can show me that you fully 
grasped that we're doing the "right thing" in all host/guest big/little 
combinations with AVX registers I'm happy too :)


Alex