kvm: drop wrong assertion creating problems with pflash

[Qemu-devel] [PATCH v1] kvm: drop wrong assertion creating problems with pflash

Posted by David Hildenbrand 8 years, 1 month ago

pflash toggles mr->romd_mode. So this assert does not always hold.

1) a device was added with !mr->romd_mode, therefore effectively not
   creating a kvm slot as we want to trap every access (add = false).
2) mr->romd_mode was toggled on before remove it. There is now
   actually no slot to remove and the asser is wrong.

So let's just drop the assert.

Reported-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 accel/kvm/kvm-all.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index b0181d7220..4f1997deec 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -722,7 +722,6 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
     mem = kvm_lookup_matching_slot(kml, start_addr, size);
     if (!add) {
         if (!mem) {
-            g_assert(!memory_region_is_ram(mr) && !writeable && !mr->romd_mode);
             return;
         }
         if (mem->flags & KVM_MEM_LOG_DIRTY_PAGES) {
-- 
2.13.5

Re: [Qemu-devel] [PATCH v1] kvm: drop wrong assertion creating problems with pflash

Posted by Gerd Hoffmann 8 years, 1 month ago

On Wed, 2017-09-20 at 16:50 +0200, David Hildenbrand wrote:
> pflash toggles mr->romd_mode. So this assert does not always hold.
> 
> 1) a device was added with !mr->romd_mode, therefore effectively not
>    creating a kvm slot as we want to trap every access (add = false).
> 2) mr->romd_mode was toggled on before remove it. There is now
>    actually no slot to remove and the asser is wrong.
> 
> So let's just drop the assert.

Works without any obvious bad side effects.

Tested-by: Gerd Hoffmann <kraxel@redhat.com>

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v1] kvm: drop wrong assertion creating problems with pflash

Posted by Paolo Bonzini 8 years, 1 month ago

On 21/09/2017 08:49, Gerd Hoffmann wrote:
> On Wed, 2017-09-20 at 16:50 +0200, David Hildenbrand wrote:
>> pflash toggles mr->romd_mode. So this assert does not always hold.
>>
>> 1) a device was added with !mr->romd_mode, therefore effectively not
>>    creating a kvm slot as we want to trap every access (add = false).
>> 2) mr->romd_mode was toggled on before remove it. There is now
>>    actually no slot to remove and the asser is wrong.
>>
>> So let's just drop the assert.
> 
> Works without any obvious bad side effects.
> 
> Tested-by: Gerd Hoffmann <kraxel@redhat.com>

Queued, thanks.

Paolo