All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1.7] pc: get rid of builtin pvpanic for "-M pc-1.5"
@ 2013-11-04 13:30 Paolo Bonzini
  2013-11-04 19:13 ` Eric Blake
  2013-11-06  4:03 ` Anthony Liguori
  0 siblings, 2 replies; 3+ messages in thread
From: Paolo Bonzini @ 2013-11-04 13:30 UTC (permalink / raw)
  To: qemu-devel; +Cc: mst

This causes two slight backwards-incompatibilities between "-M pc-1.5"
and 1.5's "-M pc":

(1) a fw_cfg file is removed with this patch.  This is only a problem
if migration stops the virtual machine exactly during fw_cfg enumeration.

(2) after migration, a VM created without an explicit "-device pvpanic"
will stop reporting panics to management.

The first problem only occurs if migration is done at a very, very
early point (and I'm not sure it can happen in practice for reasonable-size
VMs, since it will likely take more time to send the RAM to destination,
than it will take for BIOS to scan fw_cfg).

The second problem only occurs if the guest panics _and_ has a guest
driver _and_ management knows to look at the crash event, so it is
mostly theoretical at this point in time.

Thus keep the code simple, and pretend it was never broken.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/pc_piix.c    | 7 -------
 hw/i386/pc_q35.c     | 7 -------
 hw/misc/pvpanic.c    | 5 -----
 include/hw/i386/pc.h | 3 ---
 4 files changed, 22 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c6042c7..a041e53 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -57,7 +57,6 @@ static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
 static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
 static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
 
-static bool has_pvpanic;
 static bool has_pci_info = true;
 
 /* PC hardware initialisation */
@@ -225,10 +224,6 @@ static void pc_init1(QEMUMachineInitArgs *args,
     if (pci_enabled) {
         pc_pci_device_init(pci_bus);
     }
-
-    if (has_pvpanic) {
-        pvpanic_init(isa_bus);
-    }
 }
 
 static void pc_init_pci(QEMUMachineInitArgs *args)
@@ -245,13 +240,11 @@ static void pc_compat_1_6(QEMUMachineInitArgs *args)
 static void pc_compat_1_5(QEMUMachineInitArgs *args)
 {
     pc_compat_1_6(args);
-    has_pvpanic = true;
 }
 
 static void pc_compat_1_4(QEMUMachineInitArgs *args)
 {
     pc_compat_1_5(args);
-    has_pvpanic = false;
     x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
     x86_cpu_compat_set_features("Westmere", FEAT_1_ECX, 0, CPUID_EXT_PCLMULQDQ);
 }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index ca84e1c..593ed2a 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -47,7 +47,6 @@
 /* ICH9 AHCI has 6 ports */
 #define MAX_SATA_PORTS     6
 
-static bool has_pvpanic;
 static bool has_pci_info = true;
 
 /* PC hardware initialisation */
@@ -214,10 +213,6 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     if (pci_enabled) {
         pc_pci_device_init(host_bus);
     }
-
-    if (has_pvpanic) {
-        pvpanic_init(isa_bus);
-    }
 }
 
 static void pc_compat_1_6(QEMUMachineInitArgs *args)
@@ -229,13 +224,11 @@ static void pc_compat_1_6(QEMUMachineInitArgs *args)
 static void pc_compat_1_5(QEMUMachineInitArgs *args)
 {
     pc_compat_1_6(args);
-    has_pvpanic = true;
 }
 
 static void pc_compat_1_4(QEMUMachineInitArgs *args)
 {
     pc_compat_1_5(args);
-    has_pvpanic = false;
     x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
     x86_cpu_compat_set_features("Westmere", FEAT_1_ECX, 0, CPUID_EXT_PCLMULQDQ);
 }
diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
index b64e3bb..4c85906 100644
--- a/hw/misc/pvpanic.c
+++ b/hw/misc/pvpanic.c
@@ -112,11 +112,6 @@ static void pvpanic_isa_realizefn(DeviceState *dev, Error **errp)
     isa_register_ioport(d, &s->io, s->ioport);
 }
 
-void pvpanic_init(ISABus *bus)
-{
-    isa_create_simple(bus, TYPE_ISA_PVPANIC_DEVICE);
-}
-
 static Property pvpanic_isa_properties[] = {
     DEFINE_PROP_UINT16("ioport", PVPanicState, ioport, 0x505),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 6083839..f6313b2 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -213,9 +213,6 @@ static inline bool isa_ne2000_init(ISABus *bus, int base, int irq, NICInfo *nd)
 void pc_system_firmware_init(MemoryRegion *rom_memory,
                              bool isapc_ram_fw);
 
-/* pvpanic.c */
-void pvpanic_init(ISABus *bus);
-
 /* e820 types */
 #define E820_RAM        1
 #define E820_RESERVED   2
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH 1.7] pc: get rid of builtin pvpanic for "-M pc-1.5"
  2013-11-04 13:30 [Qemu-devel] [PATCH 1.7] pc: get rid of builtin pvpanic for "-M pc-1.5" Paolo Bonzini
@ 2013-11-04 19:13 ` Eric Blake
  2013-11-06  4:03 ` Anthony Liguori
  1 sibling, 0 replies; 3+ messages in thread
From: Eric Blake @ 2013-11-04 19:13 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: mst

[-- Attachment #1: Type: text/plain, Size: 1356 bytes --]

On 11/04/2013 06:30 AM, Paolo Bonzini wrote:
> This causes two slight backwards-incompatibilities between "-M pc-1.5"
> and 1.5's "-M pc":
> 
> (1) a fw_cfg file is removed with this patch.  This is only a problem
> if migration stops the virtual machine exactly during fw_cfg enumeration.
> 
> (2) after migration, a VM created without an explicit "-device pvpanic"
> will stop reporting panics to management.
> 
> The first problem only occurs if migration is done at a very, very
> early point (and I'm not sure it can happen in practice for reasonable-size
> VMs, since it will likely take more time to send the RAM to destination,
> than it will take for BIOS to scan fw_cfg).
> 
> The second problem only occurs if the guest panics _and_ has a guest
> driver _and_ management knows to look at the crash event, so it is
> mostly theoretical at this point in time.
> 
> Thus keep the code simple, and pretend it was never broken.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/i386/pc_piix.c    | 7 -------
>  hw/i386/pc_q35.c     | 7 -------
>  hw/misc/pvpanic.c    | 5 -----
>  include/hw/i386/pc.h | 3 ---
>  4 files changed, 22 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH 1.7] pc: get rid of builtin pvpanic for "-M pc-1.5"
  2013-11-04 13:30 [Qemu-devel] [PATCH 1.7] pc: get rid of builtin pvpanic for "-M pc-1.5" Paolo Bonzini
  2013-11-04 19:13 ` Eric Blake
@ 2013-11-06  4:03 ` Anthony Liguori
  1 sibling, 0 replies; 3+ messages in thread
From: Anthony Liguori @ 2013-11-06  4:03 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: mst

Paolo Bonzini <pbonzini@redhat.com> writes:

> This causes two slight backwards-incompatibilities between "-M pc-1.5"
> and 1.5's "-M pc":

Can you rebase this?  It no longer applies because of the acpi building
changes.

Regards,

Anthony Liguori

>
> (1) a fw_cfg file is removed with this patch.  This is only a problem
> if migration stops the virtual machine exactly during fw_cfg enumeration.
>
> (2) after migration, a VM created without an explicit "-device pvpanic"
> will stop reporting panics to management.
>
> The first problem only occurs if migration is done at a very, very
> early point (and I'm not sure it can happen in practice for reasonable-size
> VMs, since it will likely take more time to send the RAM to destination,
> than it will take for BIOS to scan fw_cfg).
>
> The second problem only occurs if the guest panics _and_ has a guest
> driver _and_ management knows to look at the crash event, so it is
> mostly theoretical at this point in time.
>
> Thus keep the code simple, and pretend it was never broken.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/i386/pc_piix.c    | 7 -------
>  hw/i386/pc_q35.c     | 7 -------
>  hw/misc/pvpanic.c    | 5 -----
>  include/hw/i386/pc.h | 3 ---
>  4 files changed, 22 deletions(-)
>
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index c6042c7..a041e53 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -57,7 +57,6 @@ static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
>  static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
>  static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
>  
> -static bool has_pvpanic;
>  static bool has_pci_info = true;
>  
>  /* PC hardware initialisation */
> @@ -225,10 +224,6 @@ static void pc_init1(QEMUMachineInitArgs *args,
>      if (pci_enabled) {
>          pc_pci_device_init(pci_bus);
>      }
> -
> -    if (has_pvpanic) {
> -        pvpanic_init(isa_bus);
> -    }
>  }
>  
>  static void pc_init_pci(QEMUMachineInitArgs *args)
> @@ -245,13 +240,11 @@ static void pc_compat_1_6(QEMUMachineInitArgs *args)
>  static void pc_compat_1_5(QEMUMachineInitArgs *args)
>  {
>      pc_compat_1_6(args);
> -    has_pvpanic = true;
>  }
>  
>  static void pc_compat_1_4(QEMUMachineInitArgs *args)
>  {
>      pc_compat_1_5(args);
> -    has_pvpanic = false;
>      x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
>      x86_cpu_compat_set_features("Westmere", FEAT_1_ECX, 0, CPUID_EXT_PCLMULQDQ);
>  }
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index ca84e1c..593ed2a 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -47,7 +47,6 @@
>  /* ICH9 AHCI has 6 ports */
>  #define MAX_SATA_PORTS     6
>  
> -static bool has_pvpanic;
>  static bool has_pci_info = true;
>  
>  /* PC hardware initialisation */
> @@ -214,10 +213,6 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
>      if (pci_enabled) {
>          pc_pci_device_init(host_bus);
>      }
> -
> -    if (has_pvpanic) {
> -        pvpanic_init(isa_bus);
> -    }
>  }
>  
>  static void pc_compat_1_6(QEMUMachineInitArgs *args)
> @@ -229,13 +224,11 @@ static void pc_compat_1_6(QEMUMachineInitArgs *args)
>  static void pc_compat_1_5(QEMUMachineInitArgs *args)
>  {
>      pc_compat_1_6(args);
> -    has_pvpanic = true;
>  }
>  
>  static void pc_compat_1_4(QEMUMachineInitArgs *args)
>  {
>      pc_compat_1_5(args);
> -    has_pvpanic = false;
>      x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
>      x86_cpu_compat_set_features("Westmere", FEAT_1_ECX, 0, CPUID_EXT_PCLMULQDQ);
>  }
> diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
> index b64e3bb..4c85906 100644
> --- a/hw/misc/pvpanic.c
> +++ b/hw/misc/pvpanic.c
> @@ -112,11 +112,6 @@ static void pvpanic_isa_realizefn(DeviceState *dev, Error **errp)
>      isa_register_ioport(d, &s->io, s->ioport);
>  }
>  
> -void pvpanic_init(ISABus *bus)
> -{
> -    isa_create_simple(bus, TYPE_ISA_PVPANIC_DEVICE);
> -}
> -
>  static Property pvpanic_isa_properties[] = {
>      DEFINE_PROP_UINT16("ioport", PVPanicState, ioport, 0x505),
>      DEFINE_PROP_END_OF_LIST(),
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 6083839..f6313b2 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -213,9 +213,6 @@ static inline bool isa_ne2000_init(ISABus *bus, int base, int irq, NICInfo *nd)
>  void pc_system_firmware_init(MemoryRegion *rom_memory,
>                               bool isapc_ram_fw);
>  
> -/* pvpanic.c */
> -void pvpanic_init(ISABus *bus);
> -
>  /* e820 types */
>  #define E820_RAM        1
>  #define E820_RESERVED   2
> -- 
> 1.8.3.1

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-11-06  4:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-04 13:30 [Qemu-devel] [PATCH 1.7] pc: get rid of builtin pvpanic for "-M pc-1.5" Paolo Bonzini
2013-11-04 19:13 ` Eric Blake
2013-11-06  4:03 ` Anthony Liguori

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.