[Qemu-devel] [PATCH v2 0/7] Poison some more target-specific defines

[Qemu-devel] [PATCH v2 0/7] Poison some more target-specific defines

[Thomas Huth]

This series marks some more #defines as poisoned, which are
target-specific (declared in config-target.h) and thus must
not be used in common code.

v2:
 - First two patches are the same as in v1
 - Reworked the CONFIG_KVM patches according to Paolo's review feedback
 - Added two new patches to finally poison CONFIG_SOFTMMU, too
 - Added a final patch to move bootdevice.o to common-obj now
   (based on an earlier patch where I also tried to move numa.o and
   balloon.o, too - but these files are indirectly target-dependent as
   I now know, so they can't be moved)

Thomas Huth (7):
  include/exec/poison: Add missing TARGET defines
  include/exec/poison: Mark some CONFIG defines as poisoned, too
  Move CONFIG_KVM related definitions to kvm_i386.h
  include/exec/poison: Mark CONFIG_KVM as poisoned, too
  cpu: Introduce a wrapper for tlb_flush() that can be used in common
    code
  include/exec/poison: Mark CONFIG_SOFTMMU as poisoned
  Makefile: Move bootdevice.o to common-obj-y

 Makefile.objs             |  2 +-
 Makefile.target           |  2 +-
 bootdevice.c              |  2 +-
 hw/acpi/ich9.c            |  1 -
 hw/i386/pc_q35.c          |  1 +
 include/exec/cpu-common.h |  2 ++
 include/exec/poison.h     | 33 +++++++++++++++++++++++++++++++++
 include/hw/i386/pc.h      | 13 -------------
 include/qom/cpu.h         |  8 ++++++++
 include/sysemu/kvm.h      | 31 ++++++++++++-------------------
 qom/cpu.c                 |  5 ++---
 target/i386/kvm_i386.h    | 23 +++++++++++++++++++++++
 translate-all.c           |  8 ++++++++
 13 files changed, 92 insertions(+), 39 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/7] include/exec/poison: Add missing TARGET defines

[Thomas Huth]

Since we've got some new CPU targets in QEMU during the last months
and years, we've got some new TARGET_xxx defines now which should
be marked as poisoned for common code.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 include/exec/poison.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/exec/poison.h b/include/exec/poison.h
index 3ca7929..9356d5f 100644
--- a/include/exec/poison.h
+++ b/include/exec/poison.h
@@ -12,17 +12,28 @@
 #pragma GCC poison TARGET_CRIS
 #pragma GCC poison TARGET_LM32
 #pragma GCC poison TARGET_M68K
+#pragma GCC poison TARGET_MICROBLAZE
 #pragma GCC poison TARGET_MIPS
+#pragma GCC poison TARGET_ABI_MIPSO32
 #pragma GCC poison TARGET_MIPS64
+#pragma GCC poison TARGET_ABI_MIPSN64
+#pragma GCC poison TARGET_MOXIE
+#pragma GCC poison TARGET_NIOS2
 #pragma GCC poison TARGET_OPENRISC
 #pragma GCC poison TARGET_PPC
 #pragma GCC poison TARGET_PPCEMB
 #pragma GCC poison TARGET_PPC64
 #pragma GCC poison TARGET_ABI32
+#pragma GCC poison TARGET_S390X
 #pragma GCC poison TARGET_SH4
 #pragma GCC poison TARGET_SPARC
 #pragma GCC poison TARGET_SPARC64
+#pragma GCC poison TARGET_TRICORE
+#pragma GCC poison TARGET_UNICORE32
+#pragma GCC poison TARGET_XTENSA
 
+#pragma GCC poison TARGET_NAME
+#pragma GCC poison TARGET_SUPPORTS_MTTCG
 #pragma GCC poison TARGET_WORDS_BIGENDIAN
 #pragma GCC poison BSWAP_NEEDED
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/7] include/exec/poison: Mark some CONFIG defines as poisoned, too

[Thomas Huth]

These are defined in config-target.h and thus should never be
used in common code.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 include/exec/poison.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/include/exec/poison.h b/include/exec/poison.h
index 9356d5f..5ffed4d 100644
--- a/include/exec/poison.h
+++ b/include/exec/poison.h
@@ -61,5 +61,25 @@
 #pragma GCC poison CPU_INTERRUPT_TGT_INT_1
 #pragma GCC poison CPU_INTERRUPT_TGT_INT_2
 
+#pragma GCC poison CONFIG_ALPHA_DIS
+#pragma GCC poison CONFIG_ARM_A64_DIS
+#pragma GCC poison CONFIG_ARM_DIS
+#pragma GCC poison CONFIG_CRIS_DIS
+#pragma GCC poison CONFIG_I386_DIS
+#pragma GCC poison CONFIG_LM32_DIS
+#pragma GCC poison CONFIG_M68K_DIS
+#pragma GCC poison CONFIG_MICROBLAZE_DIS
+#pragma GCC poison CONFIG_MIPS_DIS
+#pragma GCC poison CONFIG_MOXIE_DIS
+#pragma GCC poison CONFIG_NIOS2_DIS
+#pragma GCC poison CONFIG_PPC_DIS
+#pragma GCC poison CONFIG_S390_DIS
+#pragma GCC poison CONFIG_SH4_DIS
+#pragma GCC poison CONFIG_SPARC_DIS
+#pragma GCC poison CONFIG_XTENSA_DIS
+
+#pragma GCC poison CONFIG_LINUX_USER
+#pragma GCC poison CONFIG_VHOST_NET
+
 #endif
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/7] Move CONFIG_KVM related definitions to kvm_i386.h

[Thomas Huth]

pc.h and sysemu/kvm.h are also included from common code (where
CONFIG_KVM is not available), so the #defines that depend on CONFIG_KVM
should not be declared here to avoid that anybody is using them in a
wrong way. Since we're also going to poison CONFIG_KVM for common code,
let's move them to kvm_i386.h instead. Most of the dummy definitions
from sysemu/kvm.h are also unused since the code that uses them is
only compiled for CONFIG_KVM (e.g. target/i386/kvm.c), so the unused
defines are also simply dropped here instead of being moved.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/i386/pc_q35.c       |  1 +
 include/hw/i386/pc.h   | 13 -------------
 include/sysemu/kvm.h   | 15 ---------------
 target/i386/kvm_i386.h | 23 +++++++++++++++++++++++
 4 files changed, 24 insertions(+), 28 deletions(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 1523ef3..8f696b7 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -36,6 +36,7 @@
 #include "hw/timer/mc146818rtc.h"
 #include "hw/xen/xen.h"
 #include "sysemu/kvm.h"
+#include "kvm_i386.h"
 #include "hw/kvm/clock.h"
 #include "hw/pci-host/q35.h"
 #include "exec/address-spaces.h"
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index d071c9c..a31f7aa 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -20,19 +20,6 @@
 
 #define HPET_INTCAP "hpet-intcap"
 
-#ifdef CONFIG_KVM
-#define kvm_pit_in_kernel() \
-    (kvm_irqchip_in_kernel() && !kvm_irqchip_is_split())
-#define kvm_pic_in_kernel()  \
-    (kvm_irqchip_in_kernel() && !kvm_irqchip_is_split())
-#define kvm_ioapic_in_kernel() \
-    (kvm_irqchip_in_kernel() && !kvm_irqchip_is_split())
-#else
-#define kvm_pit_in_kernel()      0
-#define kvm_pic_in_kernel()      0
-#define kvm_ioapic_in_kernel()   0
-#endif
-
 /**
  * PCMachineState:
  * @acpi_dev: link to ACPI PM device that performs ACPI hotplug handling
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 1e91613..ca40b6e 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -22,21 +22,6 @@
 #ifdef CONFIG_KVM
 #include <linux/kvm.h>
 #include <linux/kvm_para.h>
-#else
-/* These constants must never be used at runtime if kvm_enabled() is false.
- * They exist so we don't need #ifdefs around KVM-specific code that already
- * checks kvm_enabled() properly.
- */
-#define KVM_CPUID_SIGNATURE      0
-#define KVM_CPUID_FEATURES       0
-#define KVM_FEATURE_CLOCKSOURCE  0
-#define KVM_FEATURE_NOP_IO_DELAY 0
-#define KVM_FEATURE_MMU_OP       0
-#define KVM_FEATURE_CLOCKSOURCE2 0
-#define KVM_FEATURE_ASYNC_PF     0
-#define KVM_FEATURE_STEAL_TIME   0
-#define KVM_FEATURE_PV_EOI       0
-#define KVM_FEATURE_CLOCKSOURCE_STABLE_BIT 0
 #endif
 
 extern bool kvm_allowed;
diff --git a/target/i386/kvm_i386.h b/target/i386/kvm_i386.h
index bfce427..1de9876 100644
--- a/target/i386/kvm_i386.h
+++ b/target/i386/kvm_i386.h
@@ -15,6 +15,29 @@
 
 #define kvm_apic_in_kernel() (kvm_irqchip_in_kernel())
 
+#ifdef CONFIG_KVM
+
+#define kvm_pit_in_kernel() \
+    (kvm_irqchip_in_kernel() && !kvm_irqchip_is_split())
+#define kvm_pic_in_kernel()  \
+    (kvm_irqchip_in_kernel() && !kvm_irqchip_is_split())
+#define kvm_ioapic_in_kernel() \
+    (kvm_irqchip_in_kernel() && !kvm_irqchip_is_split())
+
+#else
+
+#define kvm_pit_in_kernel()      0
+#define kvm_pic_in_kernel()      0
+#define kvm_ioapic_in_kernel()   0
+
+/* These constants must never be used at runtime if kvm_enabled() is false.
+ * They exist so we don't need #ifdefs around KVM-specific code that already
+ * checks kvm_enabled() properly.
+ */
+#define KVM_CPUID_FEATURES       0
+
+#endif  /* CONFIG_KVM */
+
 bool kvm_allows_irq0_override(void);
 bool kvm_has_smm(void);
 bool kvm_has_adjust_clock_stable(void);
-- 
1.8.3.1

[Qemu-devel] [PATCH 4/7] include/exec/poison: Mark CONFIG_KVM as poisoned, too

[Thomas Huth]

CONFIG_KVM is only defined for target-specific code, so nobody should
use it by accident in common code. To avoid such subtle bugs,
CONFIG_KVM is now marked as poisoned in common code. The header
include/sysemu/kvm.h is somewhat special since it is included
all over the place from common code, too, so we need some extra
logic via "#ifdef NEED_CPU_H" here to make sure that we can
compile all files without problems.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/acpi/ich9.c        |  1 -
 include/exec/poison.h |  1 +
 include/sysemu/kvm.h  | 18 +++++++++++++-----
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 5c279bb..c5d8646 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -33,7 +33,6 @@
 #include "sysemu/sysemu.h"
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/tco.h"
-#include "sysemu/kvm.h"
 #include "exec/address-spaces.h"
 
 #include "hw/i386/ich9.h"
diff --git a/include/exec/poison.h b/include/exec/poison.h
index 5ffed4d..540fc70 100644
--- a/include/exec/poison.h
+++ b/include/exec/poison.h
@@ -80,6 +80,7 @@
 
 #pragma GCC poison CONFIG_LINUX_USER
 #pragma GCC poison CONFIG_VHOST_NET
+#pragma GCC poison CONFIG_KVM
 
 #endif
 #endif
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index ca40b6e..052e11f 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -19,11 +19,18 @@
 #include "exec/memattrs.h"
 #include "hw/irq.h"
 
-#ifdef CONFIG_KVM
-#include <linux/kvm.h>
-#include <linux/kvm_para.h>
+#ifdef NEED_CPU_H
+# ifdef CONFIG_KVM
+#  include <linux/kvm.h>
+#  include <linux/kvm_para.h>
+#  define CONFIG_KVM_IS_POSSIBLE
+# endif
+#else
+# define CONFIG_KVM_IS_POSSIBLE
 #endif
 
+#ifdef CONFIG_KVM_IS_POSSIBLE
+
 extern bool kvm_allowed;
 extern bool kvm_kernel_irqchip;
 extern bool kvm_split_irqchip;
@@ -40,7 +47,6 @@ extern bool kvm_direct_msi_allowed;
 extern bool kvm_ioeventfd_any_length_allowed;
 extern bool kvm_msi_use_devid;
 
-#if defined CONFIG_KVM || !defined NEED_CPU_H
 #define kvm_enabled()           (kvm_allowed)
 /**
  * kvm_irqchip_in_kernel:
@@ -163,6 +169,7 @@ extern bool kvm_msi_use_devid;
 #define kvm_msi_devid_required() (kvm_msi_use_devid)
 
 #else
+
 #define kvm_enabled()           (0)
 #define kvm_irqchip_in_kernel() (false)
 #define kvm_irqchip_is_split() (false)
@@ -178,7 +185,8 @@ extern bool kvm_msi_use_devid;
 #define kvm_direct_msi_enabled() (false)
 #define kvm_ioeventfd_any_length_enabled() (false)
 #define kvm_msi_devid_required() (false)
-#endif
+
+#endif  /* CONFIG_KVM_IS_POSSIBLE */
 
 struct kvm_run;
 struct kvm_lapic_state;
-- 
1.8.3.1

[Qemu-devel] [PATCH 5/7] cpu: Introduce a wrapper for tlb_flush() that can be used in common code

[Thomas Huth]

Commit 1f5c00cfdb8114c ("qom/cpu: move tlb_flush to cpu_common_reset")
moved the call to tlb_flush() from the target-specific reset handlers
into the common code qom/cpu.c file, and protected the call with
"#ifdef CONFIG_SOFTMMU" to avoid that it is called for linux-user
only targets. But since qom/cpu.c is common code, CONFIG_SOFTMMU is
*never* defined here, so the tlb_flush() was simply never executed
anymore. Fix it by introducing a wrapper for tlb_flush() in a file
that is re-compiled for each target, i.e. in translate-all.c.

Fixes: 1f5c00cfdb8114c1e3a13426588ceb64f82c9ddb
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 include/exec/cpu-common.h | 2 ++
 qom/cpu.c                 | 5 ++---
 translate-all.c           | 8 ++++++++
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 4d45a72..74341b1 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -28,6 +28,8 @@ void qemu_init_cpu_list(void);
 void cpu_list_lock(void);
 void cpu_list_unlock(void);
 
+void tcg_flush_softmmu_tlb(CPUState *cs);
+
 #if !defined(CONFIG_USER_ONLY)
 
 enum device_endian {
diff --git a/qom/cpu.c b/qom/cpu.c
index 5069876..303eb42 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -26,6 +26,7 @@
 #include "qemu/notify.h"
 #include "qemu/log.h"
 #include "exec/log.h"
+#include "exec/cpu-common.h"
 #include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
 #include "hw/qdev-properties.h"
@@ -296,9 +297,7 @@ static void cpu_common_reset(CPUState *cpu)
             atomic_set(&cpu->tb_jmp_cache[i], NULL);
         }
 
-#ifdef CONFIG_SOFTMMU
-        tlb_flush(cpu, 0);
-#endif
+        tcg_flush_softmmu_tlb(cpu);
     }
 }
 
diff --git a/translate-all.c b/translate-all.c
index b3ee876..a45480f 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -2219,3 +2219,11 @@ int page_unprotect(target_ulong address, uintptr_t pc)
     return 0;
 }
 #endif /* CONFIG_USER_ONLY */
+
+/* This is a wrapper for common code that can not use CONFIG_SOFTMMU */
+void tcg_flush_softmmu_tlb(CPUState *cs)
+{
+#ifdef CONFIG_SOFTMMU
+    tlb_flush(cs);
+#endif
+}
-- 
1.8.3.1

[Qemu-devel] [PATCH 6/7] include/exec/poison: Mark CONFIG_SOFTMMU as poisoned

[Thomas Huth]

CONFIG_SOFTMMU should never be used in common code, so mark
it as poisoned, too.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 include/exec/poison.h | 1 +
 include/qom/cpu.h     | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/include/exec/poison.h b/include/exec/poison.h
index 540fc70..c932d6f 100644
--- a/include/exec/poison.h
+++ b/include/exec/poison.h
@@ -81,6 +81,7 @@
 #pragma GCC poison CONFIG_LINUX_USER
 #pragma GCC poison CONFIG_VHOST_NET
 #pragma GCC poison CONFIG_KVM
+#pragma GCC poison CONFIG_SOFTMMU
 
 #endif
 #endif
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 89ddb68..c41e1e3 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -783,6 +783,8 @@ bool cpu_throttle_active(void);
  */
 int cpu_throttle_get_percentage(void);
 
+#ifdef NEED_CPU_H
+
 #ifndef CONFIG_USER_ONLY
 
 typedef void (*CPUInterruptHandler)(CPUState *, int);
@@ -829,6 +831,8 @@ static inline void cpu_unaligned_access(CPUState *cpu, vaddr addr,
 }
 #endif
 
+#endif /* NEED_CPU_H */
+
 /**
  * cpu_set_pc:
  * @cpu: The CPU to set the program counter for.
@@ -1005,6 +1009,8 @@ void cpu_exec_initfn(CPUState *cpu);
 void cpu_exec_realizefn(CPUState *cpu, Error **errp);
 void cpu_exec_unrealizefn(CPUState *cpu);
 
+#ifdef NEED_CPU_H
+
 #ifdef CONFIG_SOFTMMU
 extern const struct VMStateDescription vmstate_cpu_common;
 #else
@@ -1019,6 +1025,8 @@ extern const struct VMStateDescription vmstate_cpu_common;
     .offset = 0,                                                            \
 }
 
+#endif /* NEED_CPU_H */
+
 #define UNASSIGNED_CPU_INDEX -1
 
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH 7/7] Makefile: Move bootdevice.o to common-obj-y

[Thomas Huth]

There does not seem to be any target specific code in this file, so
we can put it into "common-obj" instead of "obj" to compile it only
once for all targets.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 Makefile.objs   | 2 +-
 Makefile.target | 2 +-
 bootdevice.c    | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index 0575802..e5bdae9 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -40,7 +40,7 @@ io-obj-y = io/
 
 ifeq ($(CONFIG_SOFTMMU),y)
 common-obj-y = blockdev.o blockdev-nbd.o block/
-common-obj-y += iothread.o
+common-obj-y += bootdevice.o iothread.o
 common-obj-y += net/
 common-obj-y += qdev-monitor.o device-hotplug.o
 common-obj-$(CONFIG_WIN32) += os-win32.o
diff --git a/Makefile.target b/Makefile.target
index ce8dfe4..0759176 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -140,7 +140,7 @@ endif #CONFIG_BSD_USER
 # System emulator target
 ifdef CONFIG_SOFTMMU
 obj-y += arch_init.o cpus.o monitor.o gdbstub.o balloon.o ioport.o numa.o
-obj-y += qtest.o bootdevice.o
+obj-y += qtest.o
 obj-y += hw/
 obj-$(CONFIG_KVM) += kvm-all.o
 obj-y += memory.o cputlb.o
diff --git a/bootdevice.c b/bootdevice.c
index 33e3029..1141009 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -27,7 +27,7 @@
 #include "sysemu/sysemu.h"
 #include "qapi/visitor.h"
 #include "qemu/error-report.h"
-#include "hw/hw.h"
+#include "sysemu/reset.h"
 #include "hw/qdev-core.h"
 
 typedef struct FWBootEntry FWBootEntry;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 0/7] Poison some more target-specific defines

[Paolo Bonzini]

On 16/06/2017 16:59, Thomas Huth wrote:
> This series marks some more #defines as poisoned, which are
> target-specific (declared in config-target.h) and thus must
> not be used in common code.
> 
> v2:
>  - First two patches are the same as in v1
>  - Reworked the CONFIG_KVM patches according to Paolo's review feedback
>  - Added two new patches to finally poison CONFIG_SOFTMMU, too
>  - Added a final patch to move bootdevice.o to common-obj now
>    (based on an earlier patch where I also tried to move numa.o and
>    balloon.o, too - but these files are indirectly target-dependent as
>    I now know, so they can't be moved)

Why do you say they can't be moved?  They can, and your series should
change nothing about it, thanks to what you're doing now with
CONFIG_KVM_IS_POSSIBLE (which was also done before with NEED_CPU_H).

If it compiles, it's perfect. :)  (Almost---your patches 5-6 show it's
not entirely true, but poisoning helps).

Paolo

> Thomas Huth (7):
>   include/exec/poison: Add missing TARGET defines
>   include/exec/poison: Mark some CONFIG defines as poisoned, too
>   Move CONFIG_KVM related definitions to kvm_i386.h
>   include/exec/poison: Mark CONFIG_KVM as poisoned, too
>   cpu: Introduce a wrapper for tlb_flush() that can be used in common
>     code
>   include/exec/poison: Mark CONFIG_SOFTMMU as poisoned
>   Makefile: Move bootdevice.o to common-obj-y
> 
>  Makefile.objs             |  2 +-
>  Makefile.target           |  2 +-
>  bootdevice.c              |  2 +-
>  hw/acpi/ich9.c            |  1 -
>  hw/i386/pc_q35.c          |  1 +
>  include/exec/cpu-common.h |  2 ++
>  include/exec/poison.h     | 33 +++++++++++++++++++++++++++++++++
>  include/hw/i386/pc.h      | 13 -------------
>  include/qom/cpu.h         |  8 ++++++++
>  include/sysemu/kvm.h      | 31 ++++++++++++-------------------
>  qom/cpu.c                 |  5 ++---
>  target/i386/kvm_i386.h    | 23 +++++++++++++++++++++++
>  translate-all.c           |  8 ++++++++
>  13 files changed, 92 insertions(+), 39 deletions(-)
> 

Re: [Qemu-devel] [PATCH v2 0/7] Poison some more target-specific defines

[Thomas Huth]

On 16.06.2017 17:05, Paolo Bonzini wrote:
> On 16/06/2017 16:59, Thomas Huth wrote:
>> This series marks some more #defines as poisoned, which are
>> target-specific (declared in config-target.h) and thus must
>> not be used in common code.
>>
>> v2:
>>  - First two patches are the same as in v1
>>  - Reworked the CONFIG_KVM patches according to Paolo's review feedback
>>  - Added two new patches to finally poison CONFIG_SOFTMMU, too
>>  - Added a final patch to move bootdevice.o to common-obj now
>>    (based on an earlier patch where I also tried to move numa.o and
>>    balloon.o, too - but these files are indirectly target-dependent as
>>    I now know, so they can't be moved)
> 
> Why do you say they can't be moved?  They can, and your series should
> change nothing about it, thanks to what you're doing now with
> CONFIG_KVM_IS_POSSIBLE (which was also done before with NEED_CPU_H).

numa.c and balloon.c both use ram_addr_t (numa.c uses it directly, and
balloon.c indirectly via the QEMUBalloonEvent typedef from the header).
And ram_addr_t is currently target-specific - it depends on
CONFIG_XEN_BACKEND. So this could break in certain subtle cases, e.g.
when compiling on a 32-bit host with XEN enabled.

 Thomas

Re: [Qemu-devel] [PATCH v2 0/7] Poison some more target-specific defines

[Paolo Bonzini]

On 16/06/2017 17:21, Thomas Huth wrote:
>> Why do you say they can't be moved?  They can, and your series should
>> change nothing about it, thanks to what you're doing now with
>> CONFIG_KVM_IS_POSSIBLE (which was also done before with NEED_CPU_H).
> numa.c and balloon.c both use ram_addr_t (numa.c uses it directly, and
> balloon.c indirectly via the QEMUBalloonEvent typedef from the header).
> And ram_addr_t is currently target-specific - it depends on
> CONFIG_XEN_BACKEND. So this could break in certain subtle cases, e.g.
> when compiling on a 32-bit host with XEN enabled.

Can ram_addr_t be poisoned on target-independent files?

Paolo

Re: [Qemu-devel] [PATCH v2 0/7] Poison some more target-specific defines

[Thomas Huth]

On 16.06.2017 17:55, Paolo Bonzini wrote:
> 
> 
> On 16/06/2017 17:21, Thomas Huth wrote:
>>> Why do you say they can't be moved?  They can, and your series should
>>> change nothing about it, thanks to what you're doing now with
>>> CONFIG_KVM_IS_POSSIBLE (which was also done before with NEED_CPU_H).
>> numa.c and balloon.c both use ram_addr_t (numa.c uses it directly, and
>> balloon.c indirectly via the QEMUBalloonEvent typedef from the header).
>> And ram_addr_t is currently target-specific - it depends on
>> CONFIG_XEN_BACKEND. So this could break in certain subtle cases, e.g.
>> when compiling on a 32-bit host with XEN enabled.
> 
> Can ram_addr_t be poisoned on target-independent files?

This is quite similar to trying to poison CONFIG_USER_ONLY ... every
common file that depends on include/exec/memory.h or
/include/exec/cpu-common.h then does not compile anymore - and as far as
I can see, this can't be fixed with some trivial "#ifdef NEED_CPU_H"s in
most cases anymore.

Since you've asked on IRC for a list of files which do not compile
anymore in this case, here we go (some of them are also quite easy to
fix, I think, but some need some bigger reworks, I guess):

stubs/xen-common.o
stubs/xen-hvm.o
stubs/qmp_pc_dimm_device_list.o
stubs/pc_madt_cpu_entry.o
device-hotplug.o
qdev-monitor.o
accel.o
dma-helpers.o
device_tree.o
qmp.o
cpus-common.o
hmp.o
vl.o
audio/audio.o
audio/wavcapture.o
backends/hostmem-ram.o
backends/hostmem.o
backends/hostmem-file.o
backends/cryptodev.o
backends/cryptodev-builtin.o
hw/9pfs/9p.o
hw/acpi/core.o
hw/acpi/pcihp.o
hw/acpi/piix4.o
hw/acpi/ich9.o
hw/acpi/cpu_hotplug.o
hw/acpi/memory_hotplug.o
hw/acpi/tco.o
hw/acpi/cpu.o
hw/acpi/nvdimm.o
hw/acpi/acpi_interface.o
hw/acpi/vmgenid.o
hw/acpi/aml-build.o
hw/acpi/ipmi.o
hw/audio/sb16.o
hw/acpi/acpi-stub.o
hw/audio/es1370.o
hw/audio/ac97.o
hw/audio/adlib.o
hw/audio/gus.o
hw/audio/cs4231a.o
hw/audio/pcspk.o
hw/audio/hda-codec.o
hw/audio/intel-hda.o
hw/audio/wm8750.o
hw/audio/lm4549.o
hw/audio/pl041.o
hw/audio/cs4231.o
hw/audio/marvell_88w8618.o
hw/audio/milkymist-ac97.o
hw/block/cdrom.o
hw/audio/soundhw.o
hw/block/fdc.o
hw/block/m25p80.o
hw/block/nand.o
hw/block/pflash_cfi01.o
hw/block/pflash_cfi02.o
hw/block/ecc.o
hw/block/onenand.o
hw/block/nvme.o
hw/char/ipoctal232.o
hw/bt/hci.o
hw/char/parallel.o
hw/char/pl011.o
hw/char/serial.o
hw/char/escc.o
hw/char/serial-isa.o
hw/char/serial-pci.o
hw/char/xilinx_uartlite.o
hw/char/virtio-console.o
hw/char/etraxfs_ser.o
hw/char/cadence_uart.o
hw/char/debugcon.o
hw/char/grlib_apbuart.o
hw/char/imx_serial.o
hw/char/lm32_juart.o
hw/char/lm32_uart.o
hw/char/milkymist-uart.o
hw/char/sclpconsole.o
hw/char/sclpconsole-lm.o
hw/core/qdev-properties.o
hw/core/qdev.o
hw/core/bus.o
hw/core/empty_slot.o
hw/core/ptimer.o
hw/core/sysbus.o
hw/core/machine.o
hw/core/loader.o
hw/core/loader-fit.o
hw/core/qdev-properties-system.o
hw/core/register.o
hw/core/or-irq.o
hw/core/platform-bus.o
hw/cpu/core.o
hw/display/ads7846.o
hw/display/g364fb.o
hw/display/jazz_led.o
hw/display/cirrus_vga.o
hw/display/ssd0303.o
hw/display/ssd0323.o
hw/display/pl110.o
hw/display/vga-pci.o
hw/display/vga-isa.o
hw/display/vga-isa-mm.o
hw/display/vmware_vga.o
hw/display/exynos4210_fimd.o
hw/display/framebuffer.o
hw/display/milkymist-vgafb.o
hw/dma/puv3_dma.o
hw/display/tc6393xb.o
hw/display/milkymist-tmu2.o
hw/dma/rc4030.o
hw/dma/pl080.o
hw/dma/i82374.o
hw/dma/i8257.o
hw/dma/pl330.o
hw/dma/xilinx_axidma.o
hw/dma/xlnx-zynq-devcfg.o
hw/dma/etraxfs_dma.o
hw/dma/sparc32_dma.o
hw/dma/sun4m_iommu.o
hw/gpio/max7310.o
hw/gpio/puv3_gpio.o
hw/gpio/pl061.o
hw/gpio/zaurus.o
hw/gpio/mpc8xxx.o
hw/gpio/gpio_key.o
hw/i2c/core.o
hw/i2c/smbus.o
hw/i2c/smbus_eeprom.o
hw/i2c/i2c-ddc.o
hw/i2c/versatile_i2c.o
hw/i2c/smbus_ich9.o
hw/i2c/pm_smbus.o
hw/i2c/bitbang_i2c.o
hw/i2c/exynos4210_i2c.o
hw/i2c/imx_i2c.o
hw/i2c/aspeed_i2c.o
hw/ide/core.o
hw/ide/atapi.o
hw/ide/qdev.o
hw/ide/pci.o
hw/ide/isa.o
hw/ide/piix.o
hw/ide/cmd646.o
hw/ide/macio.o
hw/ide/mmio.o
hw/ide/via.o
hw/ide/microdrive.o
hw/ide/ahci.o
hw/ide/ich.o
hw/input/adb.o
hw/input/pckbd.o
hw/input/hid.o
hw/input/lm832x.o
hw/input/pl050.o
hw/input/stellaris_input.o
hw/input/ps2.o
hw/input/tsc2005.o
hw/input/vmmouse.o
hw/input/virtio-input.o
hw/input/virtio-input-host.o
hw/intc/heathrow_pic.o
hw/input/virtio-input-hid.o
hw/intc/i8259_common.o
hw/intc/i8259.o
hw/intc/pl190.o
hw/intc/puv3_intc.o
hw/intc/xilinx_intc.o
hw/intc/etraxfs_pic.o
hw/intc/imx_avic.o
hw/intc/lm32_pic.o
hw/intc/realview_gic.o
hw/intc/ioapic_common.o
hw/intc/slavio_intctl.o
hw/intc/arm_gic_common.o
hw/intc/arm_gic.o
hw/intc/arm_gicv2m.o
hw/intc/arm_gicv3_common.o
hw/intc/arm_gicv3.o
hw/intc/arm_gicv3_dist.o
hw/intc/arm_gicv3_its_common.o
hw/intc/arm_gicv3_redist.o
hw/intc/openpic.o
hw/ipack/ipack.o
hw/ipack/tpci200.o
hw/ipmi/ipmi.o
hw/ipmi/ipmi_bmc_sim.o
hw/ipmi/ipmi_bmc_extern.o
hw/ipmi/isa_ipmi_kcs.o
hw/ipmi/isa_ipmi_bt.o
hw/isa/apm.o
hw/isa/isa-bus.o
hw/isa/i82378.o
hw/isa/pc87312.o
hw/isa/vt82c686.o
hw/isa/piix4.o
hw/mem/pc-dimm.o
hw/mem/nvdimm.o
hw/misc/max111x.o
hw/misc/tmp105.o
hw/misc/tmp421.o
hw/misc/applesmc.o
hw/misc/debugexit.o
hw/misc/sga.o
hw/misc/pc-testdev.o
hw/misc/pci-testdev.o
hw/misc/unimp.o
hw/misc/arm_l2x0.o
hw/misc/arm_integrator_debug.o
hw/misc/a9scu.o
hw/misc/puv3_pm.o
hw/misc/arm11scu.o
hw/misc/macio/macio.o
hw/misc/macio/cuda.o
hw/misc/macio/mac_dbdma.o
hw/net/dp8393x.o
hw/net/ne2000.o
hw/net/eepro100.o
hw/net/pcnet-pci.o
hw/net/pcnet.o
hw/net/e1000x_common.o
hw/net/e1000.o
hw/net/net_tx_pkt.o
hw/net/e1000e.o
hw/net/rtl8139.o
hw/net/e1000e_core.o
hw/net/smc91c111.o
hw/net/vmxnet3.o
hw/net/lan9118.o
hw/net/ne2000-isa.o
hw/net/opencores_eth.o
hw/net/xgmac.o
hw/net/mipsnet.o
hw/net/xilinx_axienet.o
hw/net/allwinner_emac.o
hw/net/imx_fec.o
hw/net/stellaris_enet.o
hw/net/cadence_gem.o
hw/net/lance.o
hw/net/ftgmac100.o
hw/net/rocker/rocker.o
hw/net/rocker/rocker_desc.o
hw/nvram/ds1225y.o
hw/nvram/eeprom93xx.o
hw/nvram/fw_cfg.o
hw/nvram/chrp_nvram.o
hw/nvram/mac_nvram.o
hw/pci-bridge/pci_bridge_dev.o
hw/pci-bridge/pcie_root_port.o
hw/pci-bridge/gen_pcie_root_port.o
hw/pci-bridge/xio3130_upstream.o
hw/pci-bridge/pci_expander_bridge.o
hw/pci-bridge/xio3130_downstream.o
hw/pci-bridge/ioh3420.o
hw/pci-bridge/dec.o
hw/pci-bridge/i82801b11.o
hw/pci-host/pam.o
hw/pci-host/prep.o
hw/pci-host/grackle.o
hw/pci-host/uninorth.o
hw/pci-host/ppce500.o
hw/pci-host/versatile.o
hw/pci-host/apb.o
hw/pci-host/bonito.o
hw/pci-host/piix.o
hw/pci-host/q35.o
hw/pci-host/gpex.o
hw/pci-host/xilinx-pcie.o
hw/pci/pci.o
hw/pci/pci_bridge.o
hw/pci/msix.o
hw/pci/msi.o
hw/pci/shpc.o
hw/pci/slotid_cap.o
hw/pci/pci_host.o
hw/pci/pcie_host.o
hw/pci/pcie.o
hw/pci/pcie_port.o
hw/pci/pcie_aer.o
hw/pci/pci-stub.o
hw/pcmcia/pcmcia.o
hw/scsi/scsi-generic.o
hw/scsi/scsi-disk.o
hw/scsi/scsi-bus.o
hw/scsi/lsi53c895a.o
hw/scsi/mptsas.o
hw/scsi/mptconfig.o
hw/scsi/mptendian.o
hw/scsi/megasas.o
hw/scsi/esp.o
hw/scsi/vmw_pvscsi.o
hw/scsi/esp-pci.o
hw/sd/pl181.o
hw/sd/ssi-sd.o
hw/sd/sd.o
hw/sd/core.o
hw/sd/sdhci.o
hw/smbios/smbios_type_38.o
hw/smbios/smbios.o
hw/ssi/pl022.o
hw/ssi/ssi.o
hw/ssi/xilinx_spi.o
hw/ssi/aspeed_smc.o
hw/ssi/xilinx_spips.o
hw/ssi/stm32f2xx_spi.o
hw/timer/arm_timer.o
hw/timer/arm_mptimer.o
hw/timer/armv7m_systick.o
hw/timer/a9gtimer.o
hw/timer/cadence_ttc.o
hw/timer/ds1338.o
hw/timer/i8254_common.o
hw/timer/i8254.o
hw/timer/hpet.o
hw/timer/m48t59.o
hw/timer/m48t59-isa.o
hw/timer/pl031.o
hw/timer/puv3_ost.o
hw/timer/xilinx_timer.o
hw/timer/slavio_timer.o
hw/timer/twl92230.o
hw/timer/etraxfs_timer.o
hw/timer/grlib_gptimer.o
hw/timer/imx_epit.o
hw/timer/imx_gpt.o
hw/timer/lm32_timer.o
hw/timer/milkymist-sysctl.o
hw/timer/stm32f2xx_timer.o
hw/timer/aspeed_timer.o
hw/timer/sun4v-rtc.o
hw/tpm/tpm_tis.o
hw/tpm/tpm_passthrough.o
hw/tpm/tpm_util.o
hw/usb/core.o
hw/usb/libhw.o
hw/usb/combined-packet.o
hw/usb/bus.o
hw/usb/desc.o
hw/usb/desc-msos.o
hw/usb/hcd-uhci.o
hw/usb/hcd-ohci.o
hw/usb/hcd-ehci-pci.o
hw/usb/hcd-ehci-sysbus.o
hw/usb/hcd-ehci.o
hw/usb/hcd-xhci-nec.o
hw/usb/hcd-xhci.o
hw/usb/hcd-musb.o
hw/usb/dev-hub.o
hw/usb/dev-hid.o
hw/usb/dev-wacom.o
hw/usb/dev-storage.o
hw/usb/dev-uas.o
hw/usb/dev-audio.o
hw/usb/dev-serial.o
hw/usb/dev-network.o
hw/usb/dev-bluetooth.o
hw/usb/dev-smartcard-reader.o
hw/usb/dev-mtp.o
hw/usb/host-legacy.o
hw/usb/host-libusb.o
hw/virtio/virtio-rng.o
hw/virtio/virtio-bus.o
hw/virtio/virtio-mmio.o
hw/virtio/vhost-stub.o
hw/virtio/virtio-pci.o
hw/watchdog/wdt_i6300esb.o
hw/watchdog/wdt_ib700.o
hw/watchdog/wdt_diag288.o
hw/watchdog/wdt_aspeed.o
migration/socket.o
migration/migration.o
migration/tls.o
migration/channel.o
migration/savevm.o
migration/colo-comm.o
migration/colo.o
migration/colo-failover.o
migration/vmstate.o
migration/vmstate-types.o
migration/qemu-file.o
migration/qemu-file-channel.o
migration/postcopy-ram.o
migration/block.o
net/net.o
net/vhost-user.o
net/filter.o
net/tap.o
qom/cpu.o
slirp/slirp.o
ui/console.o
ui/input.o
ui/vnc.o

 Thomas

Re: [Qemu-devel] [PATCH v2 0/7] Poison some more target-specific defines

[Paolo Bonzini]

On 16/06/2017 18:15, Thomas Huth wrote:
> This is quite similar to trying to poison CONFIG_USER_ONLY ... every
> common file that depends on include/exec/memory.h or
> /include/exec/cpu-common.h then does not compile anymore - and as far as
> I can see, this can't be fixed with some trivial "#ifdef NEED_CPU_H"s in
> most cases anymore.

Oh, indeed. :(  I'll put removing ram_addr_t from memory.h on my todo
list.  I think most of those can go under NEED_CPU_H or can be replaced
with size_t.

Those parts of cpu-common.h instead should move to
include/exec/ram_addr.h sooner rather than later.

Paolo

> Since you've asked on IRC for a list of files which do not compile
> anymore in this case, here we go (some of them are also quite easy to
> fix, I think, but some need some bigger reworks, I guess):

Re: [Qemu-devel] [PATCH 5/7] cpu: Introduce a wrapper for tlb_flush() that can be used in common code

[Alex Bennée]

Thomas Huth <thuth@redhat.com> writes:

> Commit 1f5c00cfdb8114c ("qom/cpu: move tlb_flush to cpu_common_reset")
> moved the call to tlb_flush() from the target-specific reset handlers
> into the common code qom/cpu.c file, and protected the call with
> "#ifdef CONFIG_SOFTMMU" to avoid that it is called for linux-user
> only targets. But since qom/cpu.c is common code, CONFIG_SOFTMMU is
> *never* defined here, so the tlb_flush() was simply never executed
> anymore. Fix it by introducing a wrapper for tlb_flush() in a file
> that is re-compiled for each target, i.e. in translate-all.c.
>
> Fixes: 1f5c00cfdb8114c1e3a13426588ceb64f82c9ddb
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  include/exec/cpu-common.h | 2 ++
>  qom/cpu.c                 | 5 ++---
>  translate-all.c           | 8 ++++++++
>  3 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> index 4d45a72..74341b1 100644
> --- a/include/exec/cpu-common.h
> +++ b/include/exec/cpu-common.h
> @@ -28,6 +28,8 @@ void qemu_init_cpu_list(void);
>  void cpu_list_lock(void);
>  void cpu_list_unlock(void);
>
> +void tcg_flush_softmmu_tlb(CPUState *cs);
> +
>  #if !defined(CONFIG_USER_ONLY)
>
>  enum device_endian {
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 5069876..303eb42 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -26,6 +26,7 @@
>  #include "qemu/notify.h"
>  #include "qemu/log.h"
>  #include "exec/log.h"
> +#include "exec/cpu-common.h"
>  #include "qemu/error-report.h"
>  #include "sysemu/sysemu.h"
>  #include "hw/qdev-properties.h"
> @@ -296,9 +297,7 @@ static void cpu_common_reset(CPUState *cpu)
>              atomic_set(&cpu->tb_jmp_cache[i], NULL);
>          }
>
> -#ifdef CONFIG_SOFTMMU
> -        tlb_flush(cpu, 0);
> -#endif
> +        tcg_flush_softmmu_tlb(cpu);
>      }
>  }
>
> diff --git a/translate-all.c b/translate-all.c
> index b3ee876..a45480f 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -2219,3 +2219,11 @@ int page_unprotect(target_ulong address, uintptr_t pc)
>      return 0;
>  }
>  #endif /* CONFIG_USER_ONLY */
> +
> +/* This is a wrapper for common code that can not use CONFIG_SOFTMMU */
> +void tcg_flush_softmmu_tlb(CPUState *cs)
> +{
> +#ifdef CONFIG_SOFTMMU
> +    tlb_flush(cs);
> +#endif
> +}

Don't you usually have a empty inline for the stub in the headers so the
non-SoftMMU build can optimize away rather than link to an empty function?

--
Alex Bennée

Re: [Qemu-devel] [PATCH 5/7] cpu: Introduce a wrapper for tlb_flush() that can be used in common code

[Paolo Bonzini]

On 16/06/2017 19:07, Alex Bennée wrote:
> 
> Thomas Huth <thuth@redhat.com> writes:
> 
>> Commit 1f5c00cfdb8114c ("qom/cpu: move tlb_flush to cpu_common_reset")
>> moved the call to tlb_flush() from the target-specific reset handlers
>> into the common code qom/cpu.c file, and protected the call with
>> "#ifdef CONFIG_SOFTMMU" to avoid that it is called for linux-user
>> only targets. But since qom/cpu.c is common code, CONFIG_SOFTMMU is
>> *never* defined here, so the tlb_flush() was simply never executed
>> anymore. Fix it by introducing a wrapper for tlb_flush() in a file
>> that is re-compiled for each target, i.e. in translate-all.c.
>>
>> Fixes: 1f5c00cfdb8114c1e3a13426588ceb64f82c9ddb
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  include/exec/cpu-common.h | 2 ++
>>  qom/cpu.c                 | 5 ++---
>>  translate-all.c           | 8 ++++++++
>>  3 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
>> index 4d45a72..74341b1 100644
>> --- a/include/exec/cpu-common.h
>> +++ b/include/exec/cpu-common.h
>> @@ -28,6 +28,8 @@ void qemu_init_cpu_list(void);
>>  void cpu_list_lock(void);
>>  void cpu_list_unlock(void);
>>
>> +void tcg_flush_softmmu_tlb(CPUState *cs);
>> +
>>  #if !defined(CONFIG_USER_ONLY)
>>
>>  enum device_endian {
>> diff --git a/qom/cpu.c b/qom/cpu.c
>> index 5069876..303eb42 100644
>> --- a/qom/cpu.c
>> +++ b/qom/cpu.c
>> @@ -26,6 +26,7 @@
>>  #include "qemu/notify.h"
>>  #include "qemu/log.h"
>>  #include "exec/log.h"
>> +#include "exec/cpu-common.h"
>>  #include "qemu/error-report.h"
>>  #include "sysemu/sysemu.h"
>>  #include "hw/qdev-properties.h"
>> @@ -296,9 +297,7 @@ static void cpu_common_reset(CPUState *cpu)
>>              atomic_set(&cpu->tb_jmp_cache[i], NULL);
>>          }
>>
>> -#ifdef CONFIG_SOFTMMU
>> -        tlb_flush(cpu, 0);
>> -#endif
>> +        tcg_flush_softmmu_tlb(cpu);
>>      }
>>  }
>>
>> diff --git a/translate-all.c b/translate-all.c
>> index b3ee876..a45480f 100644
>> --- a/translate-all.c
>> +++ b/translate-all.c
>> @@ -2219,3 +2219,11 @@ int page_unprotect(target_ulong address, uintptr_t pc)
>>      return 0;
>>  }
>>  #endif /* CONFIG_USER_ONLY */
>> +
>> +/* This is a wrapper for common code that can not use CONFIG_SOFTMMU */
>> +void tcg_flush_softmmu_tlb(CPUState *cs)
>> +{
>> +#ifdef CONFIG_SOFTMMU
>> +    tlb_flush(cs);
>> +#endif
>> +}
> 
> Don't you usually have a empty inline for the stub in the headers so the
> non-SoftMMU build can optimize away rather than link to an empty function?

Optimizing reset doesn't seem worthwhile. :)

Paolo