Skip to contentSkip to conversation

Fix initialization bugs in kvmvapic

PATCH Applied Jan Kiszka 2 participants 5 messages Sep 3, 2013 – Sep 12, 2013 KVM

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

cover Fix initialization bugs in kvmvapic

Addresses the issue Daniel reported in
http://thread.gmane.org/gmane.comp.emulators.qemu/231577

CC [email protected]

Jan Kiszka (3):
kvmvapic: Catch invalid ROM size
kvmvapic: Enter inactive state on hardware reset
kvmvapic: Clear also physical ROM address when entering INACTIVE state

 hw/i386/kvmvapic.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)
Signature · 1 line
1.8.1.1.298.ge7eed54
author
#1 lore raw

PATCH 1/3 kvmvapic: Catch invalid ROM size

If not caught early, a zero-length ROM will cause a NULL-pointer access later on in patch_hypercalls when allocating a zero-length ROM copy and trying to read from it.

CC [email protected]
Signed-off-by Jan Kiszka <[email protected]>
---
 hw/i386/kvmvapic.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
hw/i386/kvmvapic.c +9 -2
index 15beb80..7ac0fe1 100644
@@ -578,7 +578,7 @@ static int patch_hypercalls(VAPICROMState *s)
  * enable write access to the option ROM so that variables can be updated by
  * the guest.
  */
-static void vapic_map_rom_writable(VAPICROMState *s)
+static int vapic_map_rom_writable(VAPICROMState *s)
 {
     hwaddr rom_paddr = s->rom_state_paddr & ROM_BLOCK_MASK;
     MemoryRegionSection section;

@@ -599,6 +599,9 @@ static void vapic_map_rom_writable(VAPICROMState *s)
     /* read ROM size from RAM region */
     ram = memory_region_get_ram_ptr(section.mr);
     rom_size = ram[rom_paddr + 2] * ROM_BLOCK_SIZE;
+    if (rom_size == 0) {
+        return -1;
+    }
     s->rom_size = rom_size;
 
     /* We need to round to avoid creating subpages

@@ -612,11 +615,15 @@ static void vapic_map_rom_writable(VAPICROMState *s)
     memory_region_add_subregion_overlap(as, rom_paddr, &s->rom, 1000);
     s->rom_mapped_writable = true;
     memory_region_unref(section.mr);
+
+    return 0;
 }
 
 static int vapic_prepare(VAPICROMState *s)
 {
-    vapic_map_rom_writable(s);
+    if (vapic_map_rom_writable(s) < 0) {
+        return -1;
+    }
 
     if (patch_hypercalls(s) < 0) {
         return -1;
Signature · 1 line
1.8.1.1.298.ge7eed54
author
#2 lore raw

PATCH 2/3 kvmvapic: Enter inactive state on hardware reset

In reply to Jan Kiszka

ROM layout may change after reset of devices are hotplugged, so we have to pick up the physical address again when the ROM is initialized. This is best achieved by resetting the state to INACTIVE.

CC [email protected]
Signed-off-by Jan Kiszka <[email protected]>
---
 hw/i386/kvmvapic.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
hw/i386/kvmvapic.c +1 -3
index 7ac0fe1..f2e335d 100644
@@ -510,9 +510,7 @@ static void vapic_reset(DeviceState *dev)
 {
     VAPICROMState *s = VAPIC(dev);
 
-    if (s->state == VAPIC_ACTIVE) {
-        s->state = VAPIC_STANDBY;
-    }
+    s->state = VAPIC_INACTIVE;
     vapic_enable_tpr_reporting(false);
 }
Signature · 1 line
1.8.1.1.298.ge7eed54
author
#3 lore raw

PATCH 3/3 kvmvapic: Clear also physical ROM address when entering INACTIVE state

In reply to Jan Kiszka

To avoid misinterpreting INACTIVE after migration as old qemu-kvm's STANDBY, also clear rom_state_paddr when going back to this state.

CC [email protected]
Signed-off-by Jan Kiszka <[email protected]>
---
 hw/i386/kvmvapic.c | 2 ++
 1 file changed, 2 insertions(+)
hw/i386/kvmvapic.c +2 -0
index f2e335d..cf6c714 100644
@@ -511,6 +511,7 @@ static void vapic_reset(DeviceState *dev)
     VAPICROMState *s = VAPIC(dev);
 
     s->state = VAPIC_INACTIVE;
+    s->rom_state_paddr = 0;
     vapic_enable_tpr_reporting(false);
 }
 

@@ -664,6 +665,7 @@ static void vapic_write(void *opaque, hwaddr addr, uint64_t data,
         }
         if (vapic_prepare(s) < 0) {
             s->state = VAPIC_INACTIVE;
+            s->rom_state_paddr = 0;
             break;
         }
         break;
Signature · 1 line
1.8.1.1.298.ge7eed54
maintainer Applied
#4 lore raw
In reply to Jan Kiszka

Il 03/09/2013 18:08, Jan Kiszka ha scritto:

Quoted 6 lines

Addresses the issue Daniel reported in
http://thread.gmane.org/gmane.comp.emulators.qemu/231577

CC [email protected]
Quoted 8 lines

Jan Kiszka (3):
kvmvapic: Catch invalid ROM size
kvmvapic: Enter inactive state on hardware reset
kvmvapic: Clear also physical ROM address when entering INACTIVE state

 hw/i386/kvmvapic.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

Applied to uq/master, thanks.

Paolo