Re: [Qemu-devel] [PATCH v1 5/6] kvm: kvm_log_start/stop are only called with known sections

[Thomas Huth]

On 16.10.2017 09:16, David Hildenbrand wrote:
> On 10.10.2017 15:29, Paolo Bonzini wrote:
>> On 10/10/2017 11:06, Thomas Huth wrote:
>>> On 11.09.2017 19:49, David Hildenbrand wrote:
>>>> Let's properly align the sections first and bail out if we would ever
>>>> get called with a memory section we don't know yet.
>>>>
>>>> Signed-off-by: David Hildenbrand <address@hidden>
>>>> ---
>>>>  accel/kvm/kvm-all.c | 18 ++++++++++++------
>>>>  1 file changed, 12 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
>>>> index b677d1b13e..2ae459453d 100644
>>>> --- a/accel/kvm/kvm-all.c
>>>> +++ b/accel/kvm/kvm-all.c
>>>> @@ -411,15 +411,21 @@ static int kvm_slot_update_flags(KVMMemoryListener 
>>>> *kml, KVMSlot *mem,
>>>>  static int kvm_section_update_flags(KVMMemoryListener *kml,
>>>>                                      MemoryRegionSection *section)
>>>>  {
>>>> -    hwaddr phys_addr = section->offset_within_address_space;
>>>> -    ram_addr_t size = int128_get64(section->size);
>>>> -    KVMSlot *mem = kvm_lookup_matching_slot(kml, phys_addr, size);
>>>> +    hwaddr start_addr, size;
>>>> +    KVMSlot *mem;
>>>>  
>>>> -    if (mem == NULL)  {
>>>> +    size = kvm_align_section(section, &start_addr);
>>>> +    if (!size) {
>>>>          return 0;
>>>> -    } else {
>>>> -        return kvm_slot_update_flags(kml, mem, section->mr);
>>>>      }
>>>> +
>>>> +    mem = kvm_lookup_matching_slot(kml, start_addr, size);
>>>> +    if (!mem) {
>>>> +        fprintf(stderr, "%s: error finding slot\n", __func__);
>>>> +        abort();
>>>> +    }
>>>
>>> FYI, this abort now triggers with the "isa-vga" device:
>>>
>>> $ x86_64-softmmu/qemu-system-x86_64 -S -accel kvm -device isa-vga
>>> kvm_section_update_flags: error finding slot
>>> Aborted (core dumped)
>>>
>>> Any ideas how to fix this?
>>
>> Reverting, unless David has some time to look into it.
>>
>> Paolo
>>
> 
> Just returned from vacation, I'll have a look this week.
> 
> Thanks for that nice reproducer Thomas, that should help a lot!

FWIW, I've found the problem with the scripts/device-crash-test script,
so you might want to run that, too, before submitting a new version.

 Thomas