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

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

[Philippe Mathieu-Daudé]

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 */
-- 
2.21.3

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

[Anthony PERARD via]

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,

-- 
Anthony PERARD

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

[Philippe Mathieu-Daudé]

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.

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

[Paolo Bonzini]

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