[Qemu-devel] Re: [PATCH 6/7] Set slots more carefully

[Jan Kiszka <jan.kiszka@siemens.com>]

Alexander Graf wrote:
> 
> On 17.07.2009, at 15:48, Jan Kiszka wrote:
> 
>> Alexander Graf wrote:
>>> KVM only supports slot sizes of PAGE_SIZE granilarity. On PPC the OS
>>> sets the framebuffer to some odd size though, causing the current code
>>> to simply abort().
>>>
>>> So let's bet graceful here. We can just allocate memory sizes that
>>> are of
>>> PAGE_SIZE granularity and everything that exceeds that comes in as
>>> MMIO and
>>> gets handled too - just slower.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> ---
>>> kvm-all.c |    2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/kvm-all.c b/kvm-all.c
>>> index 961fa32..60b76cf 100644
>>> --- a/kvm-all.c
>>> +++ b/kvm-all.c
>>> @@ -134,7 +134,7 @@ static int kvm_set_user_memory_region(KVMState
>>> *s, KVMSlot *slot)
>>>
>>>     mem.slot = slot->slot;
>>>     mem.guest_phys_addr = slot->start_addr;
>>> -    mem.memory_size = slot->memory_size;
>>> +    mem.memory_size = slot->memory_size & ~(TARGET_PAGE_SIZE - 1);
>>
>> TARGET_PAGE_MASK? And I bet you want to round up here...
> 
> 
> Eh - yeah. Same thing, no?

Just look at its definition...

> 
> And no, I don't want to round up. After the memory backed slot could
> easily be real MMIO space. I had such strange configurations with the
> ESCC overlapping in the same page as graphic memory once.

Overlapping is handled by the kvm layer in user space. For sure, if
there is a mapping conflict, we are in trouble (kvm-wise) as the second
request overwrites the mapping type of the overlapping page(s).

> 
> Better be safe than sorry.

No, something wrong. If you cut off the odd "overhead", you effectively
exclude that page. So either the caller should not include it in the
first place as it is unused or kvm should try to map the whole page just
like the rest of the slot. If there remains an unresolvable conflict, we
need to enhance the slot management with some sub-page dispatching
mechanism.

> 
>> Does the caller use the odd size consistently (i.e. also for dirty log
>> enable/disable)? Otherwise we may run into troubles while looking up
>> that slot as it will be registered in user space via its odd end
>> address. Not sure yet, but maybe it's better to round up at the call
>> sites of kvm_set_user_memory_region.
> 
> Well if the destroyer does not call with the same region it's totally
> broken, no?

Yes, but I was more concerned about consistency between registration and
(potentially partial) logging updates.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux