[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH 2/3] Assume PPC64 host on PPC32 KVM
From: |
Jan Kiszka |
Subject: |
[Qemu-devel] Re: [PATCH 2/3] Assume PPC64 host on PPC32 KVM |
Date: |
Fri, 24 Jul 2009 13:51:32 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
Alexander Graf wrote:
>
> On 24.07.2009, at 13:17, Jan Kiszka wrote:
>
>> Alexander Graf wrote:
>>>
>>> On 24.07.2009, at 12:59, Jan Kiszka wrote:
>>>
>>>> Alexander Graf wrote:
>>>>> When talking to the kernel about dirty maps, we need to find out which
>>>>> bits were actually set. This is done by set_bit and test_bit like
>>>>> functiontality which uses the "long" variable type.
>>>>>
>>>>> Now, with PPC32 userspace and PPC64 kernel space (which is pretty
>>>>> common),
>>>>> we can't interpret the bits properly anymore, because we think long is
>>>>> 32 bits wide.
>>>>>
>>>>> So for PPC dirty bitmap analysis, let's just assume we're always
>>>>> running
>>>>> on a PPC64 host. Currently there is no dirty bitmap implementation for
>>>>> PPC32 / PPCEMB anyways.
>>>>>
>>>>> Unbreaks dirty logging on PPC.
>>>>>
>>>>> Signed-off-by: Alexander Graf <address@hidden>
>>>>> ---
>>>>> kvm-all.c | 6 ++++++
>>>>> 1 files changed, 6 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/kvm-all.c b/kvm-all.c
>>>>> index 824bb4c..bfaa623 100644
>>>>> --- a/kvm-all.c
>>>>> +++ b/kvm-all.c
>>>>> @@ -357,7 +357,13 @@ int
>>>>> kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
>>>>> for (phys_addr = mem->start_addr, addr = mem->phys_offset;
>>>>> phys_addr < mem->start_addr + mem->memory_size;
>>>>> phys_addr += TARGET_PAGE_SIZE, addr += TARGET_PAGE_SIZE) {
>>>>> +#ifdef HOST_PPC
>>>>> + /* Big endian keeps us from having different long sizes
>>>>> in user and
>>>>> + * kernel space, so assume we're always on ppc64. */
>>>>> + uint64_t *bitmap = (uint64_t *)d.dirty_bitmap;
>>>>> +#else
>>>>> unsigned long *bitmap = (unsigned long *)d.dirty_bitmap;
>>>>> +#endif
>>>>> unsigned nr = (phys_addr - mem->start_addr) >>
>>>>> TARGET_PAGE_BITS;
>>>>> unsigned word = nr / (sizeof(*bitmap) * 8);
>>>>> unsigned bit = nr % (sizeof(*bitmap) * 8);
>>>>
>>>> This rather screams for a generic fix. Current code assumes
>>>> sizeof(unsigned long) == 8. That should already break on 32-bit x86
>>>> hosts. So either do (sizeof(*bitmap) * sizeof(unsigned long)) or switch
>>>> to uint64_t - but for ALL hosts.
>>>
>>> I don't see where that would break. The kernel treats the array as
>>> ulong*, userspace treats it as ulong* and set_bit in kernel does
>>> bitmap[word] |= (1 << bit). So as long as userspace long and kernel long
>>> are the same, it works.
>>>
>>> In fact - it should even work out with little endian and different ulong
>>> sizes. It just breaks on BE.
>>
>> Err, yes, forget it.
>>
>> But let's help me understanding the actual problem: Do you have
>> different ulong sizes in your scenario? Why? Is it a compat issue of
>> 32-bit userland on 64-bit kernel?
>
> 32-bit userland on 64-bit kernel.
OK. So this is an issue due to an underspecified KVM ABI, right?
>
> kernel: sizeof(ulong) = 8
> userspace: sizeof(ulong) = 4
>
> now, with big endian, a "1" is on the rightmost byte - which means
> looking at the bytes it's
>
> kernel: byte[7]
> userspace: byte[3]
>
> So if you set bit nr "1" with the current logic, the kernel would set
> bit "1" (in the first 8 bytes), userspace would read bit "1" in the
> second byte, thus 32 + 1.
>
> On little endian, the lower word is on the first 4 bytes, so it would
> still be bit "1" in the first byte.
>
Big endian machines require us to agree on the word size of the bitmap
so that 32-on-64-bit works - and 32-on-32 doesn't break. I think the
latter would be the case with your patch, no? Or don't we have 32-bit
KVM PowerPC kernels?
In any case, I suggest to pin down the word size and use it for all hosts.
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
- [Qemu-devel] [PATCH 2/3] Assume PPC64 host on PPC32 KVM, (continued)
- [Qemu-devel] Re: [PATCH 2/3] Assume PPC64 host on PPC32 KVM, Jan Kiszka, 2009/07/24
- [Qemu-devel] Re: [PATCH 2/3] Assume PPC64 host on PPC32 KVM, Alexander Graf, 2009/07/24
- [Qemu-devel] Re: [PATCH 2/3] Assume PPC64 host on PPC32 KVM, Jan Kiszka, 2009/07/24
- [Qemu-devel] Re: [PATCH 2/3] Assume PPC64 host on PPC32 KVM, Alexander Graf, 2009/07/24
- [Qemu-devel] Re: [PATCH 2/3] Assume PPC64 host on PPC32 KVM,
Jan Kiszka <=
- [Qemu-devel] Re: [PATCH 2/3] Assume PPC64 host on PPC32 KVM, Alexander Graf, 2009/07/24
- [Qemu-devel] Re: [PATCH 2/3] Assume PPC64 host on PPC32 KVM, Jan Kiszka, 2009/07/24
- [Qemu-devel] Re: [PATCH 2/3] Assume PPC64 host on PPC32 KVM, Alexander Graf, 2009/07/24
- [Qemu-devel] Re: [PATCH 2/3] Assume PPC64 host on PPC32 KVM, Jan Kiszka, 2009/07/24
- [Qemu-devel] Re: [PATCH 2/3] Assume PPC64 host on PPC32 KVM, Alexander Graf, 2009/07/24