diff mbox series

[PATCH-for-5.2] hw/i386/q35: Remove unreachable Xen code on Q35 machine

Message ID 20200722082517.18708-1-philmd@redhat.com (mailing list archive)
State New, archived
Headers show
Series [PATCH-for-5.2] hw/i386/q35: Remove unreachable Xen code on Q35 machine | expand

Commit Message

Philippe Mathieu-Daudé July 22, 2020, 8:25 a.m. UTC
Xen accelerator requires specific changes to a machine to be able
to use it. See for example the 'Xen PC' machine configure its PCI
bus calling pc_xen_hvm_init_pci(). There is no 'Xen Q35' machine
declared. This code was probably added while introducing the Q35
machine, based on the existing PC machine (see commit df2d8b3ed4
"Introduce q35 pc based chipset emulator"). Remove the unreachable
code to simplify this file.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/i386/pc_q35.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

Comments

Anthony PERARD Aug. 11, 2020, 10:55 a.m. UTC | #1
On Wed, Jul 22, 2020 at 10:25:17AM +0200, Philippe Mathieu-Daudé wrote:
> Xen accelerator requires specific changes to a machine to be able
> to use it. See for example the 'Xen PC' machine configure its PCI
> bus calling pc_xen_hvm_init_pci(). There is no 'Xen Q35' machine
> declared. This code was probably added while introducing the Q35
> machine, based on the existing PC machine (see commit df2d8b3ed4
> "Introduce q35 pc based chipset emulator"). Remove the unreachable
> code to simplify this file.

This is almost correct, we can't start a xen guest with the q35 machine
due to missing setup. But we wouldn't need to declare a new xen specific
machine as setting "accel=xen" is enough.

Anyway, that patch can be reverted whenever someone takes care of
bringing q35 to xen.

> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Acked-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,
Philippe Mathieu-Daudé Aug. 22, 2020, 8:10 p.m. UTC | #2
Hi Anthony,

On 8/11/20 12:55 PM, Anthony PERARD wrote:
> On Wed, Jul 22, 2020 at 10:25:17AM +0200, Philippe Mathieu-Daudé wrote:
>> Xen accelerator requires specific changes to a machine to be able
>> to use it. See for example the 'Xen PC' machine configure its PCI
>> bus calling pc_xen_hvm_init_pci(). There is no 'Xen Q35' machine
>> declared. This code was probably added while introducing the Q35
>> machine, based on the existing PC machine (see commit df2d8b3ed4
>> "Introduce q35 pc based chipset emulator"). Remove the unreachable
>> code to simplify this file.
> 
> This is almost correct, we can't start a xen guest with the q35 machine
> due to missing setup. But we wouldn't need to declare a new xen specific
> machine as setting "accel=xen" is enough.

I'm not sure you are asking me to reword the patch description,
but since you gave your A-b, I suppose this is enough as it.

> 
> Anyway, that patch can be reverted whenever someone takes care of
> bringing q35 to xen.
> 
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Acked-by: Anthony PERARD <anthony.perard@citrix.com>

Michael, can this go via your tree?

Thanks,

Phil.
Paolo Bonzini Sept. 16, 2020, 4:55 p.m. UTC | #3
On 22/07/20 10:25, Philippe Mathieu-Daudé wrote:
> Xen accelerator requires specific changes to a machine to be able
> to use it. See for example the 'Xen PC' machine configure its PCI
> bus calling pc_xen_hvm_init_pci(). There is no 'Xen Q35' machine
> declared. This code was probably added while introducing the Q35
> machine, based on the existing PC machine (see commit df2d8b3ed4
> "Introduce q35 pc based chipset emulator"). Remove the unreachable
> code to simplify this file.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/i386/pc_q35.c | 13 ++-----------
>  1 file changed, 2 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index a3e607a544..12f5934241 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -34,9 +34,7 @@
>  #include "sysemu/arch_init.h"
>  #include "hw/i2c/smbus_eeprom.h"
>  #include "hw/rtc/mc146818rtc.h"
> -#include "hw/xen/xen.h"
>  #include "sysemu/kvm.h"
> -#include "sysemu/xen.h"
>  #include "hw/kvm/clock.h"
>  #include "hw/pci-host/q35.h"
>  #include "hw/qdev-properties.h"
> @@ -179,10 +177,6 @@ static void pc_q35_init(MachineState *machine)
>          x86ms->below_4g_mem_size = machine->ram_size;
>      }
>  
> -    if (xen_enabled()) {
> -        xen_hvm_init(pcms, &ram_memory);
> -    }
> -
>      x86_cpus_init(x86ms, pcmc->default_cpu_version);
>  
>      kvmclock_create();
> @@ -208,10 +202,7 @@ static void pc_q35_init(MachineState *machine)
>      }
>  
>      /* allocate ram and load rom/bios */
> -    if (!xen_enabled()) {
> -        pc_memory_init(pcms, get_system_memory(),
> -                       rom_memory, &ram_memory);
> -    }
> +    pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory);
>  
>      /* create pci host bus */
>      q35_host = Q35_HOST_DEVICE(qdev_new(TYPE_Q35_HOST_DEVICE));
> @@ -271,7 +262,7 @@ static void pc_q35_init(MachineState *machine)
>  
>      assert(pcms->vmport != ON_OFF_AUTO__MAX);
>      if (pcms->vmport == ON_OFF_AUTO_AUTO) {
> -        pcms->vmport = xen_enabled() ? ON_OFF_AUTO_OFF : ON_OFF_AUTO_ON;
> +        pcms->vmport = ON_OFF_AUTO_ON;
>      }
>  
>      /* init basic PC hardware */
> 

Queued, thanks.

Paolo
diff mbox series

Patch

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index a3e607a544..12f5934241 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -34,9 +34,7 @@ 
 #include "sysemu/arch_init.h"
 #include "hw/i2c/smbus_eeprom.h"
 #include "hw/rtc/mc146818rtc.h"
-#include "hw/xen/xen.h"
 #include "sysemu/kvm.h"
-#include "sysemu/xen.h"
 #include "hw/kvm/clock.h"
 #include "hw/pci-host/q35.h"
 #include "hw/qdev-properties.h"
@@ -179,10 +177,6 @@  static void pc_q35_init(MachineState *machine)
         x86ms->below_4g_mem_size = machine->ram_size;
     }
 
-    if (xen_enabled()) {
-        xen_hvm_init(pcms, &ram_memory);
-    }
-
     x86_cpus_init(x86ms, pcmc->default_cpu_version);
 
     kvmclock_create();
@@ -208,10 +202,7 @@  static void pc_q35_init(MachineState *machine)
     }
 
     /* allocate ram and load rom/bios */
-    if (!xen_enabled()) {
-        pc_memory_init(pcms, get_system_memory(),
-                       rom_memory, &ram_memory);
-    }
+    pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory);
 
     /* create pci host bus */
     q35_host = Q35_HOST_DEVICE(qdev_new(TYPE_Q35_HOST_DEVICE));
@@ -271,7 +262,7 @@  static void pc_q35_init(MachineState *machine)
 
     assert(pcms->vmport != ON_OFF_AUTO__MAX);
     if (pcms->vmport == ON_OFF_AUTO_AUTO) {
-        pcms->vmport = xen_enabled() ? ON_OFF_AUTO_OFF : ON_OFF_AUTO_ON;
+        pcms->vmport = ON_OFF_AUTO_ON;
     }
 
     /* init basic PC hardware */