[Qemu-devel] [uq/master][PATCH 0/3] Fix initialization bugs in kvmvapic

[Qemu-devel] [uq/master][PATCH 0/3] Fix initialization bugs in kvmvapic

[Jan Kiszka]

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


CC: qemu-stable@nongnu.org

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(-)

-- 
1.8.1.1.298.ge7eed54

[Qemu-devel] [uq/master][PATCH 1/3] kvmvapic: Catch invalid ROM size

[Jan Kiszka]

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: qemu-stable@nongnu.org
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/i386/kvmvapic.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 15beb80..7ac0fe1 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -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;
-- 
1.8.1.1.298.ge7eed54

[Qemu-devel] [uq/master][PATCH 2/3] kvmvapic: Enter inactive state on hardware reset

[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: qemu-stable@nongnu.org
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/i386/kvmvapic.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 7ac0fe1..f2e335d 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -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);
 }
 
-- 
1.8.1.1.298.ge7eed54

[Qemu-devel] [uq/master][PATCH 3/3] kvmvapic: Clear also physical ROM address when entering INACTIVE state

[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: qemu-stable@nongnu.org
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/i386/kvmvapic.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index f2e335d..cf6c714 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -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;
-- 
1.8.1.1.298.ge7eed54

Re: [Qemu-devel] [uq/master][PATCH 0/3] Fix initialization bugs in kvmvapic

[Paolo Bonzini]

Il 03/09/2013 18:08, Jan Kiszka ha scritto:
> Addresses the issue Daniel reported in
> http://thread.gmane.org/gmane.comp.emulators.qemu/231577
> 
> 
> CC: qemu-stable@nongnu.org
> 
> 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