[Qemu-devel] [PATCH 0/7] ppc: Get rid of some CONFIG_KVM guards

[Qemu-devel] [PATCH 0/7] ppc: Get rid of some CONFIG_KVM guards

[Greg Kurz]

There are several places where CONFIG_KVM is used to guard code that
should only be built when KVM is supported. It is generally preferable
to avoid that and leave such guards in header files for improved
readability.

In many cases, the execution of the code is also conditionned by
kvm_enabled() which expands to (0) when CONFIG_KVM is not defined.
This is likely to cause the compiler to optimize the code out,
and if it doesn't, the right way to address compiling issues is
to add stubs.

Successfuly compile tested on x86_64 and ppc64le linux. Travis shows
this builds fine on OSX as well.

--
Greg

---

Greg Kurz (7):
      spapr_pci: Drop useless CONFIG_KVM ifdefery
      hw/ppc/mac_oldworld: Drop useless CONFIG_KVM ifdefery
      hw/ppc/mac_newworld: Drop useless CONFIG_KVM ifdefery
      hw/ppc/prep: Drop useless CONFIG_KVM ifdefery
      hw/ppc: Drop useless CONFIG_KVM ifdefery
      ppc: Introduce kvmppc_set_reg_tb_offset() helper
      target/ppc/machine: Add kvmppc_pvr_workaround_required() stub


 hw/ppc/mac_newworld.c |    4 ----
 hw/ppc/mac_oldworld.c |    2 --
 hw/ppc/ppc.c          |    7 +------
 hw/ppc/prep.c         |    2 --
 hw/ppc/spapr_pci.c    |    2 --
 target/ppc/kvm.c      |    9 +++++++++
 target/ppc/kvm_ppc.h  |   10 ++++++++++
 target/ppc/machine.c  |    2 --
 8 files changed, 20 insertions(+), 18 deletions(-)

[Qemu-devel] [PATCH 1/7] spapr_pci: Drop useless CONFIG_KVM ifdefery

[Greg Kurz]

kvm_enabled() expands to (0) when CONFIG_KVM is not defined.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_pci.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index fbeb1c90ee6c..00d9f2cfe464 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1944,11 +1944,9 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
      * For KVM we want to ensure that this memory is a full page so that
      * our memory slot is of page size granularity.
      */
-#ifdef CONFIG_KVM
     if (kvm_enabled()) {
         msi_window_size = getpagesize();
     }
-#endif
 
     memory_region_init_io(&sphb->msiwindow, OBJECT(sphb), &spapr_msi_ops, spapr,
                           "msi", msi_window_size);

[Qemu-devel] [PATCH 2/7] hw/ppc/mac_oldworld: Drop useless CONFIG_KVM ifdefery

[Greg Kurz]

kvm_enabled() expands to (0) when CONFIG_KVM is not defined. It is
likely that the compiler will optimize the code out. And even if
it doesn't, we have a stub for kvmppc_get_hypercall().

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/mac_oldworld.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index eddd005a7ce4..da751addc495 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -345,14 +345,12 @@ static void ppc_heathrow_init(MachineState *machine)
 
     fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_IS_KVM, kvm_enabled());
     if (kvm_enabled()) {
-#ifdef CONFIG_KVM
         uint8_t *hypercall;
 
         hypercall = g_malloc(16);
         kvmppc_get_hypercall(env, hypercall, 16);
         fw_cfg_add_bytes(fw_cfg, FW_CFG_PPC_KVM_HC, hypercall, 16);
         fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_KVM_PID, getpid());
-#endif
     }
     fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, tbfreq);
     /* Mac OS X requires a "known good" clock-frequency value; pass it one. */

[Qemu-devel] [PATCH 3/7] hw/ppc/mac_newworld: Drop useless CONFIG_KVM ifdefery

[Greg Kurz]

kvm_enabled() expands to (0) when CONFIG_KVM is not defined. The first
CONFIG_KVM guard is thus useless and it is likely that the compiler
will optimize the code out in the case of the second guard. And even
if it doesn't, we have a stub for kvmppc_get_hypercall().

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/mac_newworld.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 4d835f32b536..c8d324552470 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -437,13 +437,11 @@ static void ppc_core99_init(MachineState *machine)
     }
 
     /* The NewWorld NVRAM is not located in the MacIO device */
-#ifdef CONFIG_KVM
     if (kvm_enabled() && getpagesize() > 4096) {
         /* We can't combine read-write and read-only in a single page, so
            move the NVRAM out of ROM again for KVM */
         nvram_addr = 0xFFE00000;
     }
-#endif
     dev = qdev_create(NULL, TYPE_MACIO_NVRAM);
     qdev_prop_set_uint32(dev, "size", 0x2000);
     qdev_prop_set_uint32(dev, "it_shift", 1);
@@ -488,14 +486,12 @@ static void ppc_core99_init(MachineState *machine)
 
     fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_IS_KVM, kvm_enabled());
     if (kvm_enabled()) {
-#ifdef CONFIG_KVM
         uint8_t *hypercall;
 
         hypercall = g_malloc(16);
         kvmppc_get_hypercall(env, hypercall, 16);
         fw_cfg_add_bytes(fw_cfg, FW_CFG_PPC_KVM_HC, hypercall, 16);
         fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_KVM_PID, getpid());
-#endif
     }
     fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, tbfreq);
     /* Mac OS X requires a "known good" clock-frequency value; pass it one. */

[Qemu-devel] [PATCH 4/7] hw/ppc/prep: Drop useless CONFIG_KVM ifdefery

[Greg Kurz]

kvm_enabled() expands to (0) when CONFIG_KVM is not defined. It is
likely that the compiler will optimize the code out. And even if
it doesn't, we have a stub for kvmppc_get_hypercall().

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/prep.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 2a8009e20b46..a248ce480d57 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -780,7 +780,6 @@ static void ibm_40p_init(MachineState *machine)
 
     fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_IS_KVM, kvm_enabled());
     if (kvm_enabled()) {
-#ifdef CONFIG_KVM
         uint8_t *hypercall;
 
         fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, kvmppc_get_tbfreq());
@@ -788,7 +787,6 @@ static void ibm_40p_init(MachineState *machine)
         kvmppc_get_hypercall(env, hypercall, 16);
         fw_cfg_add_bytes(fw_cfg, FW_CFG_PPC_KVM_HC, hypercall, 16);
         fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_KVM_PID, getpid());
-#endif
     } else {
         fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, NANOSECONDS_PER_SECOND);
     }

[Qemu-devel] [PATCH 5/7] hw/ppc: Drop useless CONFIG_KVM ifdefery

[Greg Kurz]

kvmppc_set_interrupt() has a stub that does nothing when CONFIG_KVM is
not defined.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/ppc.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 9d91e8481b32..288196dfa67a 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -80,9 +80,7 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)
     }
 
     if (old_pending != env->pending_interrupts) {
-#ifdef CONFIG_KVM
         kvmppc_set_interrupt(cpu, n_IRQ, level);
-#endif
     }
 
 

[Qemu-devel] [PATCH 6/7] ppc: Introduce kvmppc_set_reg_tb_offset() helper

[Greg Kurz]

Introduce a KVM helper and its stub instead of guarding the code with
CONFIG_KVM.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/ppc.c         |    5 +----
 target/ppc/kvm.c     |    9 +++++++++
 target/ppc/kvm_ppc.h |    5 +++++
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 288196dfa67a..a9e508c496de 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -1034,10 +1034,7 @@ static void timebase_load(PPCTimebase *tb)
     CPU_FOREACH(cpu) {
         PowerPCCPU *pcpu = POWERPC_CPU(cpu);
         pcpu->env.tb_env->tb_offset = tb_off_adj;
-#if defined(CONFIG_KVM)
-        kvm_set_one_reg(cpu, KVM_REG_PPC_TB_OFFSET,
-                        &pcpu->env.tb_env->tb_offset);
-#endif
+        kvmppc_set_reg_tb_offset(pcpu, pcpu->env.tb_env->tb_offset);
     }
 }
 
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index d4107dd70d21..cde39904510a 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2939,3 +2939,12 @@ void kvmppc_set_reg_ppc_online(PowerPCCPU *cpu, unsigned int online)
         kvm_set_one_reg(cs, KVM_REG_PPC_ONLINE, &online);
     }
 }
+
+void kvmppc_set_reg_tb_offset(PowerPCCPU *cpu, int64_t tb_offset)
+{
+    CPUState *cs = CPU(cpu);
+
+    if (kvm_enabled()) {
+        kvm_set_one_reg(cs, KVM_REG_PPC_TB_OFFSET, &tb_offset);
+    }
+}
diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index 45776cad79d9..e642aaaf9226 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -80,6 +80,7 @@ bool kvmppc_pvr_workaround_required(PowerPCCPU *cpu);
 bool kvmppc_hpt_needs_host_contiguous_pages(void);
 void kvm_check_mmu(PowerPCCPU *cpu, Error **errp);
 void kvmppc_set_reg_ppc_online(PowerPCCPU *cpu, unsigned int online);
+void kvmppc_set_reg_tb_offset(PowerPCCPU *cpu, int64_t tb_offset);
 
 #else
 
@@ -206,6 +207,10 @@ static inline void kvmppc_set_reg_ppc_online(PowerPCCPU *cpu,
     return;
 }
 
+static inline void kvmppc_set_reg_tb_offset(PowerPCCPU *cpu, int64_t tb_offset)
+{
+}
+
 #ifndef CONFIG_USER_ONLY
 static inline bool kvmppc_spapr_use_multitce(void)
 {

[Qemu-devel] [PATCH 7/7] target/ppc/machine: Add kvmppc_pvr_workaround_required() stub

[Greg Kurz]

This allows to drop the CONFIG_KVM guard from the code.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 target/ppc/kvm_ppc.h |    5 +++++
 target/ppc/machine.c |    2 --
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index e642aaaf9226..98bd7d5da6d6 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -399,6 +399,11 @@ static inline int kvmppc_resize_hpt_commit(PowerPCCPU *cpu,
     return -ENOSYS;
 }
 
+static inline bool kvmppc_pvr_workaround_required(PowerPCCPU *cpu)
+{
+    return false;
+}
+
 #endif
 
 #ifndef CONFIG_KVM
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index 5ad7b40f4533..e82f5de9db7c 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -378,11 +378,9 @@ static int cpu_post_load(void *opaque, int version_id)
      * receive the PVR it expects as a workaround.
      *
      */
-#if defined(CONFIG_KVM)
     if (kvmppc_pvr_workaround_required(cpu)) {
         env->spr[SPR_PVR] = env->spr_cb[SPR_PVR].default_value;
     }
-#endif
 
     env->lr = env->spr[SPR_LR];
     env->ctr = env->spr[SPR_CTR];

Re: [Qemu-devel] [PATCH 0/7] ppc: Get rid of some CONFIG_KVM guards

[David Gibson]

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

On Fri, Jun 14, 2019 at 01:08:44PM +0200, Greg Kurz wrote:
> There are several places where CONFIG_KVM is used to guard code that
> should only be built when KVM is supported. It is generally preferable
> to avoid that and leave such guards in header files for improved
> readability.
> 
> In many cases, the execution of the code is also conditionned by
> kvm_enabled() which expands to (0) when CONFIG_KVM is not defined.
> This is likely to cause the compiler to optimize the code out,
> and if it doesn't, the right way to address compiling issues is
> to add stubs.
> 
> Successfuly compile tested on x86_64 and ppc64le linux. Travis shows
> this builds fine on OSX as well.

1..5 applied.  Still need to look at 6..7 more closely.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH 6/7] ppc: Introduce kvmppc_set_reg_tb_offset() helper

[David Gibson]

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

On Fri, Jun 14, 2019 at 01:09:17PM +0200, Greg Kurz wrote:
> Introduce a KVM helper and its stub instead of guarding the code with
> CONFIG_KVM.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied, thanks.

> ---
>  hw/ppc/ppc.c         |    5 +----
>  target/ppc/kvm.c     |    9 +++++++++
>  target/ppc/kvm_ppc.h |    5 +++++
>  3 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index 288196dfa67a..a9e508c496de 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -1034,10 +1034,7 @@ static void timebase_load(PPCTimebase *tb)
>      CPU_FOREACH(cpu) {
>          PowerPCCPU *pcpu = POWERPC_CPU(cpu);
>          pcpu->env.tb_env->tb_offset = tb_off_adj;
> -#if defined(CONFIG_KVM)
> -        kvm_set_one_reg(cpu, KVM_REG_PPC_TB_OFFSET,
> -                        &pcpu->env.tb_env->tb_offset);
> -#endif
> +        kvmppc_set_reg_tb_offset(pcpu, pcpu->env.tb_env->tb_offset);
>      }
>  }
>  
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index d4107dd70d21..cde39904510a 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -2939,3 +2939,12 @@ void kvmppc_set_reg_ppc_online(PowerPCCPU *cpu, unsigned int online)
>          kvm_set_one_reg(cs, KVM_REG_PPC_ONLINE, &online);
>      }
>  }
> +
> +void kvmppc_set_reg_tb_offset(PowerPCCPU *cpu, int64_t tb_offset)
> +{
> +    CPUState *cs = CPU(cpu);
> +
> +    if (kvm_enabled()) {
> +        kvm_set_one_reg(cs, KVM_REG_PPC_TB_OFFSET, &tb_offset);
> +    }
> +}
> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> index 45776cad79d9..e642aaaf9226 100644
> --- a/target/ppc/kvm_ppc.h
> +++ b/target/ppc/kvm_ppc.h
> @@ -80,6 +80,7 @@ bool kvmppc_pvr_workaround_required(PowerPCCPU *cpu);
>  bool kvmppc_hpt_needs_host_contiguous_pages(void);
>  void kvm_check_mmu(PowerPCCPU *cpu, Error **errp);
>  void kvmppc_set_reg_ppc_online(PowerPCCPU *cpu, unsigned int online);
> +void kvmppc_set_reg_tb_offset(PowerPCCPU *cpu, int64_t tb_offset);
>  
>  #else
>  
> @@ -206,6 +207,10 @@ static inline void kvmppc_set_reg_ppc_online(PowerPCCPU *cpu,
>      return;
>  }
>  
> +static inline void kvmppc_set_reg_tb_offset(PowerPCCPU *cpu, int64_t tb_offset)
> +{
> +}
> +
>  #ifndef CONFIG_USER_ONLY
>  static inline bool kvmppc_spapr_use_multitce(void)
>  {
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH 7/7] target/ppc/machine: Add kvmppc_pvr_workaround_required() stub

[David Gibson]

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

On Fri, Jun 14, 2019 at 01:09:22PM +0200, Greg Kurz wrote:
> This allows to drop the CONFIG_KVM guard from the code.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied, thanks.

> ---
>  target/ppc/kvm_ppc.h |    5 +++++
>  target/ppc/machine.c |    2 --
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> index e642aaaf9226..98bd7d5da6d6 100644
> --- a/target/ppc/kvm_ppc.h
> +++ b/target/ppc/kvm_ppc.h
> @@ -399,6 +399,11 @@ static inline int kvmppc_resize_hpt_commit(PowerPCCPU *cpu,
>      return -ENOSYS;
>  }
>  
> +static inline bool kvmppc_pvr_workaround_required(PowerPCCPU *cpu)
> +{
> +    return false;
> +}
> +
>  #endif
>  
>  #ifndef CONFIG_KVM
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index 5ad7b40f4533..e82f5de9db7c 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -378,11 +378,9 @@ static int cpu_post_load(void *opaque, int version_id)
>       * receive the PVR it expects as a workaround.
>       *
>       */
> -#if defined(CONFIG_KVM)
>      if (kvmppc_pvr_workaround_required(cpu)) {
>          env->spr[SPR_PVR] = env->spr_cb[SPR_PVR].default_value;
>      }
> -#endif
>  
>      env->lr = env->spr[SPR_LR];
>      env->ctr = env->spr[SPR_CTR];
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]