Re: [Qemu-devel] [PATCH 2/3] pc: get rid of builtin pvpanic

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Aug 21, 2013 at 06:43:15PM +0200, Paolo Bonzini wrote:
> It is a source of pain, and the previous patch anyway changed the
> behavior of "-M pc-1.5" compared to the real 1.5.
> 
> This also makes it clear that "-device pvpanic" is not enough:
> it will not expose pvpanic in fw_cfg properly.
> 
> No idea how to fix that.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

That should be the first step. Make the device work properly.
We'll discuss compatibility when that is clear.

> ---
>  hw/i386/pc_piix.c    |  8 --------
>  hw/i386/pc_q35.c     |  6 ------
>  hw/misc/pvpanic.c    | 14 ++------------
>  include/hw/i386/pc.h |  3 ---
>  4 files changed, 2 insertions(+), 29 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index b58c255..b80f9a3 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -56,7 +56,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 = true;
>  static bool has_pci_info = true;
>  
>  /* PC hardware initialisation */
> @@ -240,10 +239,6 @@ static void pc_init1(MemoryRegion *system_memory,
>      if (pci_enabled) {
>          pc_pci_device_init(pci_bus);
>      }
> -
> -    if (has_pvpanic) {
> -        pvpanic_init(isa_bus);
> -    }
>  }
>  
>  static void pc_init_pci(QEMUMachineInitArgs *args)
> @@ -269,7 +264,6 @@ static void pc_init_pci_1_5(QEMUMachineInitArgs *args)
>  
>  static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
>  {
> -    has_pvpanic = false;
>      x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
>      pc_init_pci_1_5(args);
>  }
> @@ -302,7 +296,6 @@ static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs *args)
>      const char *kernel_cmdline = args->kernel_cmdline;
>      const char *initrd_filename = args->initrd_filename;
>      const char *boot_device = args->boot_device;
> -    has_pvpanic = false;
>      has_pci_info = false;
>      disable_kvm_pv_eoi();
>      enable_compat_apic_id_mode();
> @@ -321,7 +314,6 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
>      const char *kernel_cmdline = args->kernel_cmdline;
>      const char *initrd_filename = args->initrd_filename;
>      const char *boot_device = args->boot_device;
> -    has_pvpanic = false;
>      has_pci_info = false;
>      if (cpu_model == NULL)
>          cpu_model = "486";
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 0b1d2e3..fb403b8 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -46,7 +46,6 @@
>  /* ICH9 AHCI has 6 ports */
>  #define MAX_SATA_PORTS     6
>  
> -static bool has_pvpanic = true;
>  static bool has_pci_info = true;
>  
>  /* PC hardware initialisation */
> @@ -210,10 +209,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_q35_init_1_5(QEMUMachineInitArgs *args)
> @@ -224,7 +219,6 @@ static void pc_q35_init_1_5(QEMUMachineInitArgs *args)
>  
>  static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
>  {
> -    has_pvpanic = false;
>      x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
>      pc_q35_init_1_5(args);
>  }
> diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
> index 7bb49a5..1928cc9 100644
> --- a/hw/misc/pvpanic.c
> +++ b/hw/misc/pvpanic.c
> @@ -101,7 +101,8 @@ static void pvpanic_isa_realizefn(DeviceState *dev, Error **errp)
>      isa_register_ioport(d, &s->io, s->ioport);
>  }
>  
> -static void pvpanic_fw_cfg(ISADevice *dev, FWCfgState *fw_cfg)
> +static void __attribute__((unused)) pvpanic_fw_cfg(ISADevice *dev,
> +                                                   FWCfgState *fw_cfg)
>  {
>      PVPanicState *s = ISA_PVPANIC_DEVICE(dev);
>      uint16_t *pvpanic_port = g_malloc(sizeof(*pvpanic_port));
> @@ -111,17 +112,6 @@ static void pvpanic_fw_cfg(ISADevice *dev, FWCfgState *fw_cfg)
>                      sizeof(*pvpanic_port));
>  }
>  
> -void pvpanic_init(ISABus *bus)
> -{
> -    ISADevice *dev;
> -    FWCfgState *fw_cfg = fw_cfg_find();
> -    if (!fw_cfg) {
> -        return;
> -    }
> -    dev = isa_create_simple (bus, TYPE_ISA_PVPANIC_DEVICE);
> -    pvpanic_fw_cfg(dev, fw_cfg);
> -}
> -
>  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 7fb97b0..064cc81 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -194,9 +194,6 @@ static inline bool isa_ne2000_init(ISABus *bus, int base, int irq, NICInfo *nd)
>  /* pc_sysfw.c */
>  void pc_system_firmware_init(MemoryRegion *rom_memory);
>  
> -/* pvpanic.c */
> -void pvpanic_init(ISABus *bus);
> -
>  /* e820 types */
>  #define E820_RAM        1
>  #define E820_RESERVED   2
> -- 
> 1.8.3.1
>