[Qemu-devel] Re: [PATCH] remove smaller slots if registering a bigger one

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

Jan Kiszka wrote:
> Glauber Costa wrote:
>> On Wed, Feb 11, 2009 at 03:23:07PM +0100, Jan Kiszka wrote:
>>> Glauber Costa wrote:
>>>> It's like a shark eating a bunch of small fishes:
>>>> in some situations (vga linear frame buffer mapping,
>>>> for example), we need to register a new slot in place
>>>> of older, smaller ones. This patch handles this case
>>>>
>>>> Signed-off-by: Glauber Costa <glommer@redhat.com>
>>>> ---
>>>>  kvm-all.c |   10 ++++++++++
>>>>  1 files changed, 10 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/kvm-all.c b/kvm-all.c
>>>> index 9fb295c..53aca0a 100644
>>>> --- a/kvm-all.c
>>>> +++ b/kvm-all.c
>>>> @@ -582,6 +582,16 @@ void kvm_set_phys_mem(target_phys_addr_t start_addr,
>>>>                  kvm_set_phys_mem(mem_start, mem_size, mem_offset);
>>>>  
>>>>              return;
>>>> +        } else if (start_addr <= mem->start_addr &&
>>>> +                   (start_addr + size) >= (mem->start_addr +
>>>> +                                           mem->memory_size)) {
>>>> +            KVMSlot slot;
>>>> +            /* unregister whole slot */
>>>> +            memcpy(&slot, mem, sizeof(slot));
>>>> +            mem->memory_size = 0;
>>>> +            kvm_set_user_memory_region(s, mem);
>>>> +
>>>> +            kvm_set_phys_mem(start_addr, size, phys_offset);
>>> That may solve some problems, but...
>>>
>>>>          } else {
>>>>              printf("Registering overlapping slot\n");
>>>>              abort();
>>>             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>> ...as long as this line exists, issues will remain. IIRC, the mapping
>>> the i440 tries to re-establish after reboot will hit this case.
>> Which is fine. I'd prefer it to be here, so we can analyse it case by case.
>> The old memory code for kvm was totally messy, in part because we tried to
>> hug the world at once, with some code paths that were almost never hit.
>>
>> Slot management can easily get very complicated. and trying to come up
>> with a solution that accounts for all problems at once may backfire on us.
>>
> 
> Well, then "fix" all users...

BTW, if you want to play with some problematic case, apply this and
reboot a guest while using -enable-kvm:

diff --git a/vl.c b/vl.c
index ce80690..d53611e 100644
--- a/vl.c
+++ b/vl.c
@@ -3557,6 +3557,8 @@ void qemu_system_reset(void)
     for(re = first_reset_entry; re != NULL; re = re->next) {
         re->func(re->opaque);
     }
+    if (kvm_enabled())
+        kvm_sync_vcpus();
 }
 
 void qemu_system_reset_request(void)

Jan

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