On 16.10.2017 10:52, Thomas Huth wrote:
> 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 <david@xxxxxxxxxx>
>>>>> ---
>>>>> 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
>
This one was easy to fix, there was just one place where the asssumption
"only called with known sections" was wrong.
See "memory: call log_start after region_add"
--
Thanks,
David
