* [Qemu-devel] [PATCH v5 0/3] Force the C standard to gnu99
@ 2019-01-11 8:17 Thomas Huth
2019-01-11 8:17 ` [Qemu-devel] [PATCH v5 1/3] ppc: Move spapr-related prototypes from xics.h into a seperate header file Thomas Huth
` (2 more replies)
0 siblings, 3 replies; 23+ messages in thread
From: Thomas Huth @ 2019-01-11 8:17 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
pbonzini, qemu-ppc, Cédric Le Goater, Greg Kurz,
David Gibson
Different versions of GCC and Clang use different versions of the C
standard by default. To avoid compilation problems with different
compilers in the future, we should enforce one language level
for all compilers. Since "gnu99" is the only usable option right now
(later versions are still marked as experimental in GCC v4.8),
we enforce compiling with -std=gnu99 now (in the third patch).
This caused some new warnings/erros to appear with clang, which are
fixed by the first two patches.
v5:
- Prefer to include headers instead of plain struct forward declarations
in the second patch.
v4:
- Avoid circular inclusion of headers and introduce xics_spapr.h
v3:
- Compile C++ code with -std=gnu++98
v2:
- Use gnu99 instead of gnu11
Thomas Huth (3):
ppc: Move spapr-related prototypes from xics.h into a seperate header
file
ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99
mode
configure: Force the C standard to gnu99
configure | 5 ++++-
hw/intc/xics_kvm.c | 1 +
hw/intc/xics_spapr.c | 1 +
hw/ppc/spapr_irq.c | 1 +
include/hw/ppc/spapr.h | 5 +++--
include/hw/ppc/spapr_xive.h | 2 --
include/hw/ppc/xics.h | 7 -------
include/hw/ppc/xics_spapr.h | 37 +++++++++++++++++++++++++++++++++++++
target/ppc/cpu.h | 13 +++++++++----
9 files changed, 56 insertions(+), 16 deletions(-)
create mode 100644 include/hw/ppc/xics_spapr.h
--
1.8.3.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v5 1/3] ppc: Move spapr-related prototypes from xics.h into a seperate header file
2019-01-11 8:17 [Qemu-devel] [PATCH v5 0/3] Force the C standard to gnu99 Thomas Huth
@ 2019-01-11 8:17 ` Thomas Huth
2019-01-11 9:22 ` Daniel P. Berrangé
2019-01-11 12:58 ` Philippe Mathieu-Daudé
2019-01-11 8:17 ` [Qemu-devel] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode Thomas Huth
2019-01-11 8:17 ` [Qemu-devel] [PATCH v5 3/3] configure: Force the C standard to gnu99 Thomas Huth
2 siblings, 2 replies; 23+ messages in thread
From: Thomas Huth @ 2019-01-11 8:17 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
pbonzini, qemu-ppc, Cédric Le Goater, Greg Kurz,
David Gibson
When compiling with Clang in -std=gnu99 mode, there is a warning/error:
CC ppc64-softmmu/hw/intc/xics_spapr.o
In file included from /home/thuth/devel/qemu/hw/intc/xics_spapr.c:34:
/home/thuth/devel/qemu/include/hw/ppc/xics.h:203:34: error: redefinition of typedef 'sPAPRMachineState' is a C11 feature
[-Werror,-Wtypedef-redefinition]
typedef struct sPAPRMachineState sPAPRMachineState;
^
/home/thuth/devel/qemu/include/hw/ppc/spapr_irq.h:25:34: note: previous definition is here
typedef struct sPAPRMachineState sPAPRMachineState;
^
We have to remove the duplicated typedef here and include "spapr.h" instead.
But "spapr.h" should not be included for the pnv machine files. So move
the spapr-related prototypes into a new file called "xics_spapr.h" instead.
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
hw/intc/xics_kvm.c | 1 +
hw/intc/xics_spapr.c | 1 +
hw/ppc/spapr_irq.c | 1 +
include/hw/ppc/xics.h | 7 -------
include/hw/ppc/xics_spapr.h | 37 +++++++++++++++++++++++++++++++++++++
5 files changed, 40 insertions(+), 7 deletions(-)
create mode 100644 include/hw/ppc/xics_spapr.h
diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
index ac94594..dff1330 100644
--- a/hw/intc/xics_kvm.c
+++ b/hw/intc/xics_kvm.c
@@ -34,6 +34,7 @@
#include "sysemu/kvm.h"
#include "hw/ppc/spapr.h"
#include "hw/ppc/xics.h"
+#include "hw/ppc/xics_spapr.h"
#include "kvm_ppc.h"
#include "qemu/config-file.h"
#include "qemu/error-report.h"
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index 9c1a90d..de6cc15 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -32,6 +32,7 @@
#include "qemu/timer.h"
#include "hw/ppc/spapr.h"
#include "hw/ppc/xics.h"
+#include "hw/ppc/xics_spapr.h"
#include "hw/ppc/fdt.h"
#include "qapi/visitor.h"
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index 5fce72f..1da7a32 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -14,6 +14,7 @@
#include "hw/ppc/spapr.h"
#include "hw/ppc/spapr_xive.h"
#include "hw/ppc/xics.h"
+#include "hw/ppc/xics_spapr.h"
#include "sysemu/kvm.h"
#include "trace.h"
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 07508cb..fad786e 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -200,13 +200,6 @@ void ics_pic_print_info(ICSState *ics, Monitor *mon);
void ics_resend(ICSState *ics);
void icp_resend(ICPState *ss);
-typedef struct sPAPRMachineState sPAPRMachineState;
-
-void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
- uint32_t phandle);
-int xics_kvm_init(sPAPRMachineState *spapr, Error **errp);
-void xics_spapr_init(sPAPRMachineState *spapr);
-
Object *icp_create(Object *cpu, const char *type, XICSFabric *xi,
Error **errp);
diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h
new file mode 100644
index 0000000..b1ab27d
--- /dev/null
+++ b/include/hw/ppc/xics_spapr.h
@@ -0,0 +1,37 @@
+/*
+ * QEMU PowerPC pSeries Logical Partition (aka sPAPR) hardware System Emulator
+ *
+ * PAPR Virtualized Interrupt System, aka ICS/ICP aka xics
+ *
+ * Copyright (c) 2010, 2011 David Gibson, IBM Corporation.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef XICS_SPAPR_H
+#define XICS_SPAPR_H
+
+#include "hw/ppc/spapr.h"
+
+void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
+ uint32_t phandle);
+int xics_kvm_init(sPAPRMachineState *spapr, Error **errp);
+void xics_spapr_init(sPAPRMachineState *spapr);
+
+#endif /* XICS_SPAPR_H */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode
2019-01-11 8:17 [Qemu-devel] [PATCH v5 0/3] Force the C standard to gnu99 Thomas Huth
2019-01-11 8:17 ` [Qemu-devel] [PATCH v5 1/3] ppc: Move spapr-related prototypes from xics.h into a seperate header file Thomas Huth
@ 2019-01-11 8:17 ` Thomas Huth
2019-01-11 8:30 ` Cédric Le Goater
` (4 more replies)
2019-01-11 8:17 ` [Qemu-devel] [PATCH v5 3/3] configure: Force the C standard to gnu99 Thomas Huth
2 siblings, 5 replies; 23+ messages in thread
From: Thomas Huth @ 2019-01-11 8:17 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
pbonzini, qemu-ppc, Cédric Le Goater, Greg Kurz,
David Gibson
When compiling the ppc code with clang and -std=gnu99, there are a
couple of warnings/errors like this one:
CC ppc64-softmmu/hw/intc/xics.o
In file included from hw/intc/xics.c:35:
include/hw/ppc/xics.h:43:25: error: redefinition of typedef 'ICPState' is a C11 feature
[-Werror,-Wtypedef-redefinition]
typedef struct ICPState ICPState;
^
target/ppc/cpu.h:1181:25: note: previous definition is here
typedef struct ICPState ICPState;
^
Work around the problems by including the proper headers instead.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
include/hw/ppc/spapr.h | 5 +++--
include/hw/ppc/spapr_xive.h | 2 --
target/ppc/cpu.h | 13 +++++++++----
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 9e01a5a..a947a0a 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -8,15 +8,16 @@
#include "hw/mem/pc-dimm.h"
#include "hw/ppc/spapr_ovec.h"
#include "hw/ppc/spapr_irq.h"
+#include "hw/ppc/spapr_xive.h" /* For sPAPRXive */
+#include "hw/ppc/xics.h" /* For ICSState */
struct VIOsPAPRBus;
struct sPAPRPHBState;
struct sPAPRNVRAM;
+
typedef struct sPAPREventLogEntry sPAPREventLogEntry;
typedef struct sPAPREventSource sPAPREventSource;
typedef struct sPAPRPendingHPT sPAPRPendingHPT;
-typedef struct ICSState ICSState;
-typedef struct sPAPRXive sPAPRXive;
#define HPTE64_V_HPTE_DIRTY 0x0000000000000040ULL
#define SPAPR_ENTRY_POINT 0x100
diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
index 7fdc250..9bec919 100644
--- a/include/hw/ppc/spapr_xive.h
+++ b/include/hw/ppc/spapr_xive.h
@@ -41,8 +41,6 @@ bool spapr_xive_irq_claim(sPAPRXive *xive, uint32_t lisn, bool lsi);
bool spapr_xive_irq_free(sPAPRXive *xive, uint32_t lisn);
void spapr_xive_pic_print_info(sPAPRXive *xive, Monitor *mon);
-typedef struct sPAPRMachineState sPAPRMachineState;
-
void spapr_xive_hcall_init(sPAPRMachineState *spapr);
void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
uint32_t phandle);
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 486abaf..bde4dff 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -23,6 +23,11 @@
#include "qemu-common.h"
#include "qemu/int128.h"
+#ifndef CONFIG_USER_ONLY
+#include "hw/ppc/xive.h" /* for XiveTCTX */
+#include "hw/ppc/xics.h" /* for ICPState */
+#endif
+
//#define PPC_EMULATE_32BITS_HYPV
#if defined (TARGET_PPC64)
@@ -1177,8 +1182,6 @@ do { \
typedef struct PPCVirtualHypervisor PPCVirtualHypervisor;
typedef struct PPCVirtualHypervisorClass PPCVirtualHypervisorClass;
-typedef struct XiveTCTX XiveTCTX;
-typedef struct ICPState ICPState;
/**
* PowerPCCPU:
@@ -1197,11 +1200,13 @@ struct PowerPCCPU {
int vcpu_id;
uint32_t compat_pvr;
PPCVirtualHypervisor *vhyp;
- ICPState *icp;
- XiveTCTX *tctx;
void *machine_data;
int32_t node_id; /* NUMA node this CPU belongs to */
PPCHash64Options *hash64_opts;
+#ifndef CONFIG_USER_ONLY
+ ICPState *icp;
+ XiveTCTX *tctx;
+#endif
/* Fields related to migration compatibility hacks */
bool pre_2_8_migration;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v5 3/3] configure: Force the C standard to gnu99
2019-01-11 8:17 [Qemu-devel] [PATCH v5 0/3] Force the C standard to gnu99 Thomas Huth
2019-01-11 8:17 ` [Qemu-devel] [PATCH v5 1/3] ppc: Move spapr-related prototypes from xics.h into a seperate header file Thomas Huth
2019-01-11 8:17 ` [Qemu-devel] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode Thomas Huth
@ 2019-01-11 8:17 ` Thomas Huth
2019-01-11 8:44 ` Greg Kurz
` (2 more replies)
2 siblings, 3 replies; 23+ messages in thread
From: Thomas Huth @ 2019-01-11 8:17 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
pbonzini, qemu-ppc, Cédric Le Goater, Greg Kurz,
David Gibson
Different versions of GCC and Clang use different versions of the C standard.
This repeatedly caused problems already, e.g. with duplicated typedefs:
https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg05829.html
or with for-loop variable initializers:
https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg00237.html
To avoid these problems, we should enforce the C language version to the
same level for all compilers. Since our minimum compiler versions is
GCC v4.8, our best option is "gnu99" for C code right now ("gnu17" is not
available there yet, and "gnu11" is marked as "experimental"), and "gnu++98"
for the few C++ code that we have in the repository.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
configure | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index cf763d4..f0b5dcd 100755
--- a/configure
+++ b/configure
@@ -107,6 +107,9 @@ update_cxxflags() {
-Wstrict-prototypes|-Wmissing-prototypes|-Wnested-externs|\
-Wold-style-declaration|-Wold-style-definition|-Wredundant-decls)
;;
+ -std=gnu99)
+ QEMU_CXXFLAGS=${QEMU_CXXFLAGS:+$QEMU_CXXFLAGS }"-std=gnu++98"
+ ;;
*)
QEMU_CXXFLAGS=${QEMU_CXXFLAGS:+$QEMU_CXXFLAGS }$arg
;;
@@ -585,7 +588,7 @@ ARFLAGS="${ARFLAGS-rv}"
# left shift of signed integers is well defined and has the expected
# 2s-complement style results. (Both clang and gcc agree that it
# provides these semantics.)
-QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv $QEMU_CFLAGS"
+QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv -std=gnu99 $QEMU_CFLAGS"
QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS"
QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS"
QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS"
--
1.8.3.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode
2019-01-11 8:17 ` [Qemu-devel] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode Thomas Huth
@ 2019-01-11 8:30 ` Cédric Le Goater
2019-01-11 8:40 ` Greg Kurz
` (3 subsequent siblings)
4 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2019-01-11 8:30 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
Cc: Daniel P. Berrangé, pbonzini, qemu-ppc, Greg Kurz, David Gibson
On 1/11/19 9:17 AM, Thomas Huth wrote:
> When compiling the ppc code with clang and -std=gnu99, there are a
> couple of warnings/errors like this one:
>
> CC ppc64-softmmu/hw/intc/xics.o
> In file included from hw/intc/xics.c:35:
> include/hw/ppc/xics.h:43:25: error: redefinition of typedef 'ICPState' is a C11 feature
> [-Werror,-Wtypedef-redefinition]
> typedef struct ICPState ICPState;
> ^
> target/ppc/cpu.h:1181:25: note: previous definition is here
> typedef struct ICPState ICPState;
> ^
> Work around the problems by including the proper headers instead.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks for sorting this out,
C.
> ---
> include/hw/ppc/spapr.h | 5 +++--
> include/hw/ppc/spapr_xive.h | 2 --
> target/ppc/cpu.h | 13 +++++++++----
> 3 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 9e01a5a..a947a0a 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -8,15 +8,16 @@
> #include "hw/mem/pc-dimm.h"
> #include "hw/ppc/spapr_ovec.h"
> #include "hw/ppc/spapr_irq.h"
> +#include "hw/ppc/spapr_xive.h" /* For sPAPRXive */
> +#include "hw/ppc/xics.h" /* For ICSState */
>
> struct VIOsPAPRBus;
> struct sPAPRPHBState;
> struct sPAPRNVRAM;
> +
> typedef struct sPAPREventLogEntry sPAPREventLogEntry;
> typedef struct sPAPREventSource sPAPREventSource;
> typedef struct sPAPRPendingHPT sPAPRPendingHPT;
> -typedef struct ICSState ICSState;
> -typedef struct sPAPRXive sPAPRXive;
>
> #define HPTE64_V_HPTE_DIRTY 0x0000000000000040ULL
> #define SPAPR_ENTRY_POINT 0x100
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index 7fdc250..9bec919 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -41,8 +41,6 @@ bool spapr_xive_irq_claim(sPAPRXive *xive, uint32_t lisn, bool lsi);
> bool spapr_xive_irq_free(sPAPRXive *xive, uint32_t lisn);
> void spapr_xive_pic_print_info(sPAPRXive *xive, Monitor *mon);
>
> -typedef struct sPAPRMachineState sPAPRMachineState;
> -
> void spapr_xive_hcall_init(sPAPRMachineState *spapr);
> void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> uint32_t phandle);
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 486abaf..bde4dff 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -23,6 +23,11 @@
> #include "qemu-common.h"
> #include "qemu/int128.h"
>
> +#ifndef CONFIG_USER_ONLY
> +#include "hw/ppc/xive.h" /* for XiveTCTX */
> +#include "hw/ppc/xics.h" /* for ICPState */
> +#endif
> +
> //#define PPC_EMULATE_32BITS_HYPV
>
> #if defined (TARGET_PPC64)
> @@ -1177,8 +1182,6 @@ do { \
>
> typedef struct PPCVirtualHypervisor PPCVirtualHypervisor;
> typedef struct PPCVirtualHypervisorClass PPCVirtualHypervisorClass;
> -typedef struct XiveTCTX XiveTCTX;
> -typedef struct ICPState ICPState;
>
> /**
> * PowerPCCPU:
> @@ -1197,11 +1200,13 @@ struct PowerPCCPU {
> int vcpu_id;
> uint32_t compat_pvr;
> PPCVirtualHypervisor *vhyp;
> - ICPState *icp;
> - XiveTCTX *tctx;
> void *machine_data;
> int32_t node_id; /* NUMA node this CPU belongs to */
> PPCHash64Options *hash64_opts;
> +#ifndef CONFIG_USER_ONLY
> + ICPState *icp;
> + XiveTCTX *tctx;
> +#endif
>
> /* Fields related to migration compatibility hacks */
> bool pre_2_8_migration;
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode
2019-01-11 8:17 ` [Qemu-devel] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode Thomas Huth
2019-01-11 8:30 ` Cédric Le Goater
@ 2019-01-11 8:40 ` Greg Kurz
2019-01-11 9:22 ` Daniel P. Berrangé
` (2 subsequent siblings)
4 siblings, 0 replies; 23+ messages in thread
From: Greg Kurz @ 2019-01-11 8:40 UTC (permalink / raw)
To: Thomas Huth
Cc: qemu-devel, Daniel P. Berrangé,
pbonzini, qemu-ppc, Cédric Le Goater, David Gibson
On Fri, 11 Jan 2019 09:17:51 +0100
Thomas Huth <thuth@redhat.com> wrote:
> When compiling the ppc code with clang and -std=gnu99, there are a
> couple of warnings/errors like this one:
>
> CC ppc64-softmmu/hw/intc/xics.o
> In file included from hw/intc/xics.c:35:
> include/hw/ppc/xics.h:43:25: error: redefinition of typedef 'ICPState' is a C11 feature
> [-Werror,-Wtypedef-redefinition]
> typedef struct ICPState ICPState;
> ^
> target/ppc/cpu.h:1181:25: note: previous definition is here
> typedef struct ICPState ICPState;
> ^
> Work around the problems by including the proper headers instead.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> include/hw/ppc/spapr.h | 5 +++--
> include/hw/ppc/spapr_xive.h | 2 --
> target/ppc/cpu.h | 13 +++++++++----
> 3 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 9e01a5a..a947a0a 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -8,15 +8,16 @@
> #include "hw/mem/pc-dimm.h"
> #include "hw/ppc/spapr_ovec.h"
> #include "hw/ppc/spapr_irq.h"
> +#include "hw/ppc/spapr_xive.h" /* For sPAPRXive */
> +#include "hw/ppc/xics.h" /* For ICSState */
>
> struct VIOsPAPRBus;
> struct sPAPRPHBState;
> struct sPAPRNVRAM;
> +
> typedef struct sPAPREventLogEntry sPAPREventLogEntry;
> typedef struct sPAPREventSource sPAPREventSource;
> typedef struct sPAPRPendingHPT sPAPRPendingHPT;
> -typedef struct ICSState ICSState;
> -typedef struct sPAPRXive sPAPRXive;
>
> #define HPTE64_V_HPTE_DIRTY 0x0000000000000040ULL
> #define SPAPR_ENTRY_POINT 0x100
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index 7fdc250..9bec919 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -41,8 +41,6 @@ bool spapr_xive_irq_claim(sPAPRXive *xive, uint32_t lisn, bool lsi);
> bool spapr_xive_irq_free(sPAPRXive *xive, uint32_t lisn);
> void spapr_xive_pic_print_info(sPAPRXive *xive, Monitor *mon);
>
> -typedef struct sPAPRMachineState sPAPRMachineState;
> -
> void spapr_xive_hcall_init(sPAPRMachineState *spapr);
> void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> uint32_t phandle);
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 486abaf..bde4dff 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -23,6 +23,11 @@
> #include "qemu-common.h"
> #include "qemu/int128.h"
>
> +#ifndef CONFIG_USER_ONLY
> +#include "hw/ppc/xive.h" /* for XiveTCTX */
> +#include "hw/ppc/xics.h" /* for ICPState */
> +#endif
> +
> //#define PPC_EMULATE_32BITS_HYPV
>
> #if defined (TARGET_PPC64)
> @@ -1177,8 +1182,6 @@ do { \
>
> typedef struct PPCVirtualHypervisor PPCVirtualHypervisor;
> typedef struct PPCVirtualHypervisorClass PPCVirtualHypervisorClass;
> -typedef struct XiveTCTX XiveTCTX;
> -typedef struct ICPState ICPState;
>
> /**
> * PowerPCCPU:
> @@ -1197,11 +1200,13 @@ struct PowerPCCPU {
> int vcpu_id;
> uint32_t compat_pvr;
> PPCVirtualHypervisor *vhyp;
> - ICPState *icp;
> - XiveTCTX *tctx;
> void *machine_data;
> int32_t node_id; /* NUMA node this CPU belongs to */
> PPCHash64Options *hash64_opts;
> +#ifndef CONFIG_USER_ONLY
> + ICPState *icp;
> + XiveTCTX *tctx;
> +#endif
>
> /* Fields related to migration compatibility hacks */
> bool pre_2_8_migration;
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/3] configure: Force the C standard to gnu99
2019-01-11 8:17 ` [Qemu-devel] [PATCH v5 3/3] configure: Force the C standard to gnu99 Thomas Huth
@ 2019-01-11 8:44 ` Greg Kurz
2019-01-11 9:23 ` Daniel P. Berrangé
2019-01-11 10:55 ` Alex Bennée
2 siblings, 0 replies; 23+ messages in thread
From: Greg Kurz @ 2019-01-11 8:44 UTC (permalink / raw)
To: Thomas Huth
Cc: qemu-devel, Daniel P. Berrangé,
pbonzini, qemu-ppc, Cédric Le Goater, David Gibson
On Fri, 11 Jan 2019 09:17:52 +0100
Thomas Huth <thuth@redhat.com> wrote:
> Different versions of GCC and Clang use different versions of the C standard.
> This repeatedly caused problems already, e.g. with duplicated typedefs:
>
> https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg05829.html
>
> or with for-loop variable initializers:
>
> https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg00237.html
>
> To avoid these problems, we should enforce the C language version to the
> same level for all compilers. Since our minimum compiler versions is
> GCC v4.8, our best option is "gnu99" for C code right now ("gnu17" is not
> available there yet, and "gnu11" is marked as "experimental"), and "gnu++98"
> for the few C++ code that we have in the repository.
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> configure | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index cf763d4..f0b5dcd 100755
> --- a/configure
> +++ b/configure
> @@ -107,6 +107,9 @@ update_cxxflags() {
> -Wstrict-prototypes|-Wmissing-prototypes|-Wnested-externs|\
> -Wold-style-declaration|-Wold-style-definition|-Wredundant-decls)
> ;;
> + -std=gnu99)
> + QEMU_CXXFLAGS=${QEMU_CXXFLAGS:+$QEMU_CXXFLAGS }"-std=gnu++98"
> + ;;
> *)
> QEMU_CXXFLAGS=${QEMU_CXXFLAGS:+$QEMU_CXXFLAGS }$arg
> ;;
> @@ -585,7 +588,7 @@ ARFLAGS="${ARFLAGS-rv}"
> # left shift of signed integers is well defined and has the expected
> # 2s-complement style results. (Both clang and gcc agree that it
> # provides these semantics.)
> -QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv $QEMU_CFLAGS"
> +QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv -std=gnu99 $QEMU_CFLAGS"
> QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS"
> QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS"
> QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS"
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v5 1/3] ppc: Move spapr-related prototypes from xics.h into a seperate header file
2019-01-11 8:17 ` [Qemu-devel] [PATCH v5 1/3] ppc: Move spapr-related prototypes from xics.h into a seperate header file Thomas Huth
@ 2019-01-11 9:22 ` Daniel P. Berrangé
2019-01-11 12:58 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 23+ messages in thread
From: Daniel P. Berrangé @ 2019-01-11 9:22 UTC (permalink / raw)
To: Thomas Huth
Cc: qemu-devel, pbonzini, qemu-ppc, Cédric Le Goater, Greg Kurz,
David Gibson
On Fri, Jan 11, 2019 at 09:17:50AM +0100, Thomas Huth wrote:
> When compiling with Clang in -std=gnu99 mode, there is a warning/error:
>
> CC ppc64-softmmu/hw/intc/xics_spapr.o
> In file included from /home/thuth/devel/qemu/hw/intc/xics_spapr.c:34:
> /home/thuth/devel/qemu/include/hw/ppc/xics.h:203:34: error: redefinition of typedef 'sPAPRMachineState' is a C11 feature
> [-Werror,-Wtypedef-redefinition]
> typedef struct sPAPRMachineState sPAPRMachineState;
> ^
> /home/thuth/devel/qemu/include/hw/ppc/spapr_irq.h:25:34: note: previous definition is here
> typedef struct sPAPRMachineState sPAPRMachineState;
> ^
>
> We have to remove the duplicated typedef here and include "spapr.h" instead.
> But "spapr.h" should not be included for the pnv machine files. So move
> the spapr-related prototypes into a new file called "xics_spapr.h" instead.
>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> hw/intc/xics_kvm.c | 1 +
> hw/intc/xics_spapr.c | 1 +
> hw/ppc/spapr_irq.c | 1 +
> include/hw/ppc/xics.h | 7 -------
> include/hw/ppc/xics_spapr.h | 37 +++++++++++++++++++++++++++++++++++++
> 5 files changed, 40 insertions(+), 7 deletions(-)
> create mode 100644 include/hw/ppc/xics_spapr.h
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode
2019-01-11 8:17 ` [Qemu-devel] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode Thomas Huth
2019-01-11 8:30 ` Cédric Le Goater
2019-01-11 8:40 ` Greg Kurz
@ 2019-01-11 9:22 ` Daniel P. Berrangé
2019-01-11 12:59 ` Philippe Mathieu-Daudé
2019-01-16 11:43 ` Cédric Le Goater
4 siblings, 0 replies; 23+ messages in thread
From: Daniel P. Berrangé @ 2019-01-11 9:22 UTC (permalink / raw)
To: Thomas Huth
Cc: qemu-devel, pbonzini, qemu-ppc, Cédric Le Goater, Greg Kurz,
David Gibson
On Fri, Jan 11, 2019 at 09:17:51AM +0100, Thomas Huth wrote:
> When compiling the ppc code with clang and -std=gnu99, there are a
> couple of warnings/errors like this one:
>
> CC ppc64-softmmu/hw/intc/xics.o
> In file included from hw/intc/xics.c:35:
> include/hw/ppc/xics.h:43:25: error: redefinition of typedef 'ICPState' is a C11 feature
> [-Werror,-Wtypedef-redefinition]
> typedef struct ICPState ICPState;
> ^
> target/ppc/cpu.h:1181:25: note: previous definition is here
> typedef struct ICPState ICPState;
> ^
> Work around the problems by including the proper headers instead.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> include/hw/ppc/spapr.h | 5 +++--
> include/hw/ppc/spapr_xive.h | 2 --
> target/ppc/cpu.h | 13 +++++++++----
> 3 files changed, 12 insertions(+), 8 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/3] configure: Force the C standard to gnu99
2019-01-11 8:17 ` [Qemu-devel] [PATCH v5 3/3] configure: Force the C standard to gnu99 Thomas Huth
2019-01-11 8:44 ` Greg Kurz
@ 2019-01-11 9:23 ` Daniel P. Berrangé
2019-01-11 10:55 ` Alex Bennée
2 siblings, 0 replies; 23+ messages in thread
From: Daniel P. Berrangé @ 2019-01-11 9:23 UTC (permalink / raw)
To: Thomas Huth
Cc: qemu-devel, pbonzini, qemu-ppc, Cédric Le Goater, Greg Kurz,
David Gibson
On Fri, Jan 11, 2019 at 09:17:52AM +0100, Thomas Huth wrote:
> Different versions of GCC and Clang use different versions of the C standard.
> This repeatedly caused problems already, e.g. with duplicated typedefs:
>
> https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg05829.html
>
> or with for-loop variable initializers:
>
> https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg00237.html
>
> To avoid these problems, we should enforce the C language version to the
> same level for all compilers. Since our minimum compiler versions is
> GCC v4.8, our best option is "gnu99" for C code right now ("gnu17" is not
> available there yet, and "gnu11" is marked as "experimental"), and "gnu++98"
> for the few C++ code that we have in the repository.
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> configure | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/3] configure: Force the C standard to gnu99
2019-01-11 8:17 ` [Qemu-devel] [PATCH v5 3/3] configure: Force the C standard to gnu99 Thomas Huth
2019-01-11 8:44 ` Greg Kurz
2019-01-11 9:23 ` Daniel P. Berrangé
@ 2019-01-11 10:55 ` Alex Bennée
2 siblings, 0 replies; 23+ messages in thread
From: Alex Bennée @ 2019-01-11 10:55 UTC (permalink / raw)
To: qemu-devel
Cc: Greg Kurz, qemu-ppc, Cédric Le Goater, pbonzini, David Gibson
Thomas Huth <thuth@redhat.com> writes:
> Different versions of GCC and Clang use different versions of the C standard.
> This repeatedly caused problems already, e.g. with duplicated typedefs:
>
> https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg05829.html
>
> or with for-loop variable initializers:
>
> https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg00237.html
>
> To avoid these problems, we should enforce the C language version to the
> same level for all compilers. Since our minimum compiler versions is
> GCC v4.8, our best option is "gnu99" for C code right now ("gnu17" is not
> available there yet, and "gnu11" is marked as "experimental"), and "gnu++98"
> for the few C++ code that we have in the repository.
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> configure | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index cf763d4..f0b5dcd 100755
> --- a/configure
> +++ b/configure
> @@ -107,6 +107,9 @@ update_cxxflags() {
> -Wstrict-prototypes|-Wmissing-prototypes|-Wnested-externs|\
> -Wold-style-declaration|-Wold-style-definition|-Wredundant-decls)
> ;;
> + -std=gnu99)
> + QEMU_CXXFLAGS=${QEMU_CXXFLAGS:+$QEMU_CXXFLAGS }"-std=gnu++98"
> + ;;
> *)
> QEMU_CXXFLAGS=${QEMU_CXXFLAGS:+$QEMU_CXXFLAGS }$arg
> ;;
> @@ -585,7 +588,7 @@ ARFLAGS="${ARFLAGS-rv}"
> # left shift of signed integers is well defined and has the expected
> # 2s-complement style results. (Both clang and gcc agree that it
> # provides these semantics.)
> -QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv $QEMU_CFLAGS"
> +QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv -std=gnu99 $QEMU_CFLAGS"
> QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS"
> QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS"
> QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS"
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v5 1/3] ppc: Move spapr-related prototypes from xics.h into a seperate header file
2019-01-11 8:17 ` [Qemu-devel] [PATCH v5 1/3] ppc: Move spapr-related prototypes from xics.h into a seperate header file Thomas Huth
2019-01-11 9:22 ` Daniel P. Berrangé
@ 2019-01-11 12:58 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-01-11 12:58 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
Cc: Greg Kurz, qemu-ppc, Cédric Le Goater, pbonzini, David Gibson
On 1/11/19 9:17 AM, Thomas Huth wrote:
> When compiling with Clang in -std=gnu99 mode, there is a warning/error:
>
> CC ppc64-softmmu/hw/intc/xics_spapr.o
> In file included from /home/thuth/devel/qemu/hw/intc/xics_spapr.c:34:
> /home/thuth/devel/qemu/include/hw/ppc/xics.h:203:34: error: redefinition of typedef 'sPAPRMachineState' is a C11 feature
> [-Werror,-Wtypedef-redefinition]
> typedef struct sPAPRMachineState sPAPRMachineState;
> ^
> /home/thuth/devel/qemu/include/hw/ppc/spapr_irq.h:25:34: note: previous definition is here
> typedef struct sPAPRMachineState sPAPRMachineState;
> ^
>
> We have to remove the duplicated typedef here and include "spapr.h" instead.
> But "spapr.h" should not be included for the pnv machine files. So move
> the spapr-related prototypes into a new file called "xics_spapr.h" instead.
>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/intc/xics_kvm.c | 1 +
> hw/intc/xics_spapr.c | 1 +
> hw/ppc/spapr_irq.c | 1 +
> include/hw/ppc/xics.h | 7 -------
> include/hw/ppc/xics_spapr.h | 37 +++++++++++++++++++++++++++++++++++++
> 5 files changed, 40 insertions(+), 7 deletions(-)
> create mode 100644 include/hw/ppc/xics_spapr.h
>
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index ac94594..dff1330 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -34,6 +34,7 @@
> #include "sysemu/kvm.h"
> #include "hw/ppc/spapr.h"
> #include "hw/ppc/xics.h"
> +#include "hw/ppc/xics_spapr.h"
> #include "kvm_ppc.h"
> #include "qemu/config-file.h"
> #include "qemu/error-report.h"
> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> index 9c1a90d..de6cc15 100644
> --- a/hw/intc/xics_spapr.c
> +++ b/hw/intc/xics_spapr.c
> @@ -32,6 +32,7 @@
> #include "qemu/timer.h"
> #include "hw/ppc/spapr.h"
> #include "hw/ppc/xics.h"
> +#include "hw/ppc/xics_spapr.h"
> #include "hw/ppc/fdt.h"
> #include "qapi/visitor.h"
>
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 5fce72f..1da7a32 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -14,6 +14,7 @@
> #include "hw/ppc/spapr.h"
> #include "hw/ppc/spapr_xive.h"
> #include "hw/ppc/xics.h"
> +#include "hw/ppc/xics_spapr.h"
> #include "sysemu/kvm.h"
>
> #include "trace.h"
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index 07508cb..fad786e 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -200,13 +200,6 @@ void ics_pic_print_info(ICSState *ics, Monitor *mon);
> void ics_resend(ICSState *ics);
> void icp_resend(ICPState *ss);
>
> -typedef struct sPAPRMachineState sPAPRMachineState;
> -
> -void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> - uint32_t phandle);
> -int xics_kvm_init(sPAPRMachineState *spapr, Error **errp);
> -void xics_spapr_init(sPAPRMachineState *spapr);
> -
> Object *icp_create(Object *cpu, const char *type, XICSFabric *xi,
> Error **errp);
>
> diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h
> new file mode 100644
> index 0000000..b1ab27d
> --- /dev/null
> +++ b/include/hw/ppc/xics_spapr.h
> @@ -0,0 +1,37 @@
> +/*
> + * QEMU PowerPC pSeries Logical Partition (aka sPAPR) hardware System Emulator
> + *
> + * PAPR Virtualized Interrupt System, aka ICS/ICP aka xics
> + *
> + * Copyright (c) 2010, 2011 David Gibson, IBM Corporation.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#ifndef XICS_SPAPR_H
> +#define XICS_SPAPR_H
> +
> +#include "hw/ppc/spapr.h"
> +
> +void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> + uint32_t phandle);
> +int xics_kvm_init(sPAPRMachineState *spapr, Error **errp);
> +void xics_spapr_init(sPAPRMachineState *spapr);
> +
> +#endif /* XICS_SPAPR_H */
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode
2019-01-11 8:17 ` [Qemu-devel] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode Thomas Huth
` (2 preceding siblings ...)
2019-01-11 9:22 ` Daniel P. Berrangé
@ 2019-01-11 12:59 ` Philippe Mathieu-Daudé
2019-01-16 11:43 ` Cédric Le Goater
4 siblings, 0 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-01-11 12:59 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
Cc: Greg Kurz, qemu-ppc, Cédric Le Goater, pbonzini, David Gibson
On 1/11/19 9:17 AM, Thomas Huth wrote:
> When compiling the ppc code with clang and -std=gnu99, there are a
> couple of warnings/errors like this one:
>
> CC ppc64-softmmu/hw/intc/xics.o
> In file included from hw/intc/xics.c:35:
> include/hw/ppc/xics.h:43:25: error: redefinition of typedef 'ICPState' is a C11 feature
> [-Werror,-Wtypedef-redefinition]
> typedef struct ICPState ICPState;
> ^
> target/ppc/cpu.h:1181:25: note: previous definition is here
> typedef struct ICPState ICPState;
> ^
> Work around the problems by including the proper headers instead.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> include/hw/ppc/spapr.h | 5 +++--
> include/hw/ppc/spapr_xive.h | 2 --
> target/ppc/cpu.h | 13 +++++++++----
> 3 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 9e01a5a..a947a0a 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -8,15 +8,16 @@
> #include "hw/mem/pc-dimm.h"
> #include "hw/ppc/spapr_ovec.h"
> #include "hw/ppc/spapr_irq.h"
> +#include "hw/ppc/spapr_xive.h" /* For sPAPRXive */
> +#include "hw/ppc/xics.h" /* For ICSState */
>
> struct VIOsPAPRBus;
> struct sPAPRPHBState;
> struct sPAPRNVRAM;
> +
> typedef struct sPAPREventLogEntry sPAPREventLogEntry;
> typedef struct sPAPREventSource sPAPREventSource;
> typedef struct sPAPRPendingHPT sPAPRPendingHPT;
> -typedef struct ICSState ICSState;
> -typedef struct sPAPRXive sPAPRXive;
>
> #define HPTE64_V_HPTE_DIRTY 0x0000000000000040ULL
> #define SPAPR_ENTRY_POINT 0x100
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index 7fdc250..9bec919 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -41,8 +41,6 @@ bool spapr_xive_irq_claim(sPAPRXive *xive, uint32_t lisn, bool lsi);
> bool spapr_xive_irq_free(sPAPRXive *xive, uint32_t lisn);
> void spapr_xive_pic_print_info(sPAPRXive *xive, Monitor *mon);
>
> -typedef struct sPAPRMachineState sPAPRMachineState;
> -
> void spapr_xive_hcall_init(sPAPRMachineState *spapr);
> void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> uint32_t phandle);
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 486abaf..bde4dff 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -23,6 +23,11 @@
> #include "qemu-common.h"
> #include "qemu/int128.h"
>
> +#ifndef CONFIG_USER_ONLY
> +#include "hw/ppc/xive.h" /* for XiveTCTX */
> +#include "hw/ppc/xics.h" /* for ICPState */
> +#endif
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> +
> //#define PPC_EMULATE_32BITS_HYPV
>
> #if defined (TARGET_PPC64)
> @@ -1177,8 +1182,6 @@ do { \
>
> typedef struct PPCVirtualHypervisor PPCVirtualHypervisor;
> typedef struct PPCVirtualHypervisorClass PPCVirtualHypervisorClass;
> -typedef struct XiveTCTX XiveTCTX;
> -typedef struct ICPState ICPState;
>
> /**
> * PowerPCCPU:
> @@ -1197,11 +1200,13 @@ struct PowerPCCPU {
> int vcpu_id;
> uint32_t compat_pvr;
> PPCVirtualHypervisor *vhyp;
> - ICPState *icp;
> - XiveTCTX *tctx;
> void *machine_data;
> int32_t node_id; /* NUMA node this CPU belongs to */
> PPCHash64Options *hash64_opts;
> +#ifndef CONFIG_USER_ONLY
> + ICPState *icp;
> + XiveTCTX *tctx;
> +#endif
>
> /* Fields related to migration compatibility hacks */
> bool pre_2_8_migration;
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode
2019-01-11 8:17 ` [Qemu-devel] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode Thomas Huth
` (3 preceding siblings ...)
2019-01-11 12:59 ` Philippe Mathieu-Daudé
@ 2019-01-16 11:43 ` Cédric Le Goater
2019-01-16 11:47 ` Thomas Huth
4 siblings, 1 reply; 23+ messages in thread
From: Cédric Le Goater @ 2019-01-16 11:43 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
Cc: Daniel P. Berrangé, pbonzini, qemu-ppc, Greg Kurz, David Gibson
On 1/11/19 9:17 AM, Thomas Huth wrote:
> When compiling the ppc code with clang and -std=gnu99, there are a
> couple of warnings/errors like this one:
>
> CC ppc64-softmmu/hw/intc/xics.o
> In file included from hw/intc/xics.c:35:
> include/hw/ppc/xics.h:43:25: error: redefinition of typedef 'ICPState' is a C11 feature
> [-Werror,-Wtypedef-redefinition]
> typedef struct ICPState ICPState;
> ^
> target/ppc/cpu.h:1181:25: note: previous definition is here
> typedef struct ICPState ICPState;
> ^
> Work around the problems by including the proper headers instead.
Thomas,
After a closer look, I think we should use 'void *' under PowerPCCPU
as it was the case before I introduced the second interrupt presenter.
That's a bigger change reverting bits of already merged patches. I can
take care of it if you prefer.
I use a f29 for dev. Which compiler should I install ?
C.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> include/hw/ppc/spapr.h | 5 +++--
> include/hw/ppc/spapr_xive.h | 2 --
> target/ppc/cpu.h | 13 +++++++++----
> 3 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 9e01a5a..a947a0a 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -8,15 +8,16 @@
> #include "hw/mem/pc-dimm.h"
> #include "hw/ppc/spapr_ovec.h"
> #include "hw/ppc/spapr_irq.h"
> +#include "hw/ppc/spapr_xive.h" /* For sPAPRXive */
> +#include "hw/ppc/xics.h" /* For ICSState */
>
> struct VIOsPAPRBus;
> struct sPAPRPHBState;
> struct sPAPRNVRAM;
> +
> typedef struct sPAPREventLogEntry sPAPREventLogEntry;
> typedef struct sPAPREventSource sPAPREventSource;
> typedef struct sPAPRPendingHPT sPAPRPendingHPT;
> -typedef struct ICSState ICSState;
> -typedef struct sPAPRXive sPAPRXive;
>
> #define HPTE64_V_HPTE_DIRTY 0x0000000000000040ULL
> #define SPAPR_ENTRY_POINT 0x100
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index 7fdc250..9bec919 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -41,8 +41,6 @@ bool spapr_xive_irq_claim(sPAPRXive *xive, uint32_t lisn, bool lsi);
> bool spapr_xive_irq_free(sPAPRXive *xive, uint32_t lisn);
> void spapr_xive_pic_print_info(sPAPRXive *xive, Monitor *mon);
>
> -typedef struct sPAPRMachineState sPAPRMachineState;
> -
> void spapr_xive_hcall_init(sPAPRMachineState *spapr);
> void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> uint32_t phandle);
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 486abaf..bde4dff 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -23,6 +23,11 @@
> #include "qemu-common.h"
> #include "qemu/int128.h"
>
> +#ifndef CONFIG_USER_ONLY
> +#include "hw/ppc/xive.h" /* for XiveTCTX */
> +#include "hw/ppc/xics.h" /* for ICPState */
> +#endif
> +
> //#define PPC_EMULATE_32BITS_HYPV
>
> #if defined (TARGET_PPC64)
> @@ -1177,8 +1182,6 @@ do { \
>
> typedef struct PPCVirtualHypervisor PPCVirtualHypervisor;
> typedef struct PPCVirtualHypervisorClass PPCVirtualHypervisorClass;
> -typedef struct XiveTCTX XiveTCTX;
> -typedef struct ICPState ICPState;
>
> /**
> * PowerPCCPU:
> @@ -1197,11 +1200,13 @@ struct PowerPCCPU {
> int vcpu_id;
> uint32_t compat_pvr;
> PPCVirtualHypervisor *vhyp;
> - ICPState *icp;
> - XiveTCTX *tctx;
> void *machine_data;
> int32_t node_id; /* NUMA node this CPU belongs to */
> PPCHash64Options *hash64_opts;
> +#ifndef CONFIG_USER_ONLY
> + ICPState *icp;
> + XiveTCTX *tctx;
> +#endif
>
> /* Fields related to migration compatibility hacks */
> bool pre_2_8_migration;
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode
2019-01-16 11:43 ` Cédric Le Goater
@ 2019-01-16 11:47 ` Thomas Huth
2019-01-16 13:23 ` Greg Kurz
2019-01-16 13:29 ` Cédric Le Goater
0 siblings, 2 replies; 23+ messages in thread
From: Thomas Huth @ 2019-01-16 11:47 UTC (permalink / raw)
To: Cédric Le Goater, qemu-devel
Cc: Daniel P. Berrangé, pbonzini, qemu-ppc, Greg Kurz, David Gibson
On 2019-01-16 12:43, Cédric Le Goater wrote:
> On 1/11/19 9:17 AM, Thomas Huth wrote:
>> When compiling the ppc code with clang and -std=gnu99, there are a
>> couple of warnings/errors like this one:
>>
>> CC ppc64-softmmu/hw/intc/xics.o
>> In file included from hw/intc/xics.c:35:
>> include/hw/ppc/xics.h:43:25: error: redefinition of typedef 'ICPState' is a C11 feature
>> [-Werror,-Wtypedef-redefinition]
>> typedef struct ICPState ICPState;
>> ^
>> target/ppc/cpu.h:1181:25: note: previous definition is here
>> typedef struct ICPState ICPState;
>> ^
>> Work around the problems by including the proper headers instead.
>
> Thomas,
>
>
> After a closer look, I think we should use 'void *' under PowerPCCPU
> as it was the case before I introduced the second interrupt presenter.
If you don't like the #includes, why not simply do anonymous struct
forward declarations here? I think that would be better than "void *".
> That's a bigger change reverting bits of already merged patches. I can
> take care of it if you prefer.
Could I keep the current patch in my series so that I can get the
patches finally merged? You could then do any clean up that you like on
top of it, ok?
> I use a f29 for dev. Which compiler should I install ?
Any version of Clang with -std=gnu99 should do the job here, I think.
Thomas
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode
2019-01-16 11:47 ` Thomas Huth
@ 2019-01-16 13:23 ` Greg Kurz
2019-01-16 13:44 ` Thomas Huth
2019-01-16 13:29 ` Cédric Le Goater
1 sibling, 1 reply; 23+ messages in thread
From: Greg Kurz @ 2019-01-16 13:23 UTC (permalink / raw)
To: Thomas Huth
Cc: Cédric Le Goater, qemu-devel, Daniel P. Berrangé,
pbonzini, qemu-ppc, David Gibson
On Wed, 16 Jan 2019 12:47:36 +0100
Thomas Huth <thuth@redhat.com> wrote:
> On 2019-01-16 12:43, Cédric Le Goater wrote:
> > On 1/11/19 9:17 AM, Thomas Huth wrote:
> >> When compiling the ppc code with clang and -std=gnu99, there are a
> >> couple of warnings/errors like this one:
> >>
> >> CC ppc64-softmmu/hw/intc/xics.o
> >> In file included from hw/intc/xics.c:35:
> >> include/hw/ppc/xics.h:43:25: error: redefinition of typedef 'ICPState' is a C11 feature
> >> [-Werror,-Wtypedef-redefinition]
> >> typedef struct ICPState ICPState;
> >> ^
> >> target/ppc/cpu.h:1181:25: note: previous definition is here
> >> typedef struct ICPState ICPState;
> >> ^
> >> Work around the problems by including the proper headers instead.
> >
> > Thomas,
> >
> >
> > After a closer look, I think we should use 'void *' under PowerPCCPU
> > as it was the case before I introduced the second interrupt presenter.
>
> If you don't like the #includes, why not simply do anonymous struct
> forward declarations here? I think that would be better than "void *".
>
That's questionable. These two fields are only used by the machine code and
the interrupt controller code.
$ git grep -E '(icp|tctx)' target/ppc/
target/ppc/cpu.h: ICPState *icp;
target/ppc/cpu.h: XiveTCTX *tctx;
$ git grep -E 'cpu\->(icp|tctx)'
hw/intc/spapr_xive_kvm.c: kvmppc_xive_cpu_set_state(cpu->tctx, &local_err);
hw/intc/spapr_xive_kvm.c: kvmppc_xive_cpu_connect(cpu->tctx, &local_err);
hw/intc/xics_kvm.c: icp_kvm_connect(cpu->icp, &local_err);
hw/intc/xics_kvm.c: icp_set_kvm_state(cpu->icp, 1);
hw/intc/xics_spapr.c: icp_set_cppr(cpu->icp, cppr);
hw/intc/xics_spapr.c: uint32_t xirr = icp_accept(cpu->icp);
hw/intc/xics_spapr.c: uint32_t xirr = icp_accept(cpu->icp);
hw/intc/xics_spapr.c: icp_eoi(cpu->icp, xirr);
hw/intc/xics_spapr.c: uint32_t xirr = icp_ipoll(cpu->icp, &mfrr);
hw/intc/xive.c: XiveTCTX *tctx = cpu->tctx;
hw/intc/xive.c: XiveTCTX *tctx = cpu->tctx;
hw/intc/xive.c: XiveTCTX *tctx = cpu->tctx;
hw/ppc/pnv.c: cpu->icp = ICP(obj);
hw/ppc/pnv.c: return cpu ? cpu->icp : NULL;
hw/ppc/pnv.c: icp_pic_print_info(cpu->icp, mon);
hw/ppc/pnv_core.c: object_unparent(OBJECT(cpu->icp));
hw/ppc/spapr.c: return cpu ? cpu->icp : NULL;
hw/ppc/spapr_cpu_core.c: if (cpu->icp) {
hw/ppc/spapr_cpu_core.c: object_unparent(OBJECT(cpu->icp));
hw/ppc/spapr_cpu_core.c: if (cpu->tctx) {
hw/ppc/spapr_cpu_core.c: object_unparent(OBJECT(cpu->tctx));
hw/ppc/spapr_irq.c: icp_pic_print_info(cpu->icp, mon);
hw/ppc/spapr_irq.c: cpu->icp = ICP(obj);
hw/ppc/spapr_irq.c: icp_resend(cpu->icp);
hw/ppc/spapr_irq.c: xive_tctx_pic_print_info(cpu->tctx, mon);
hw/ppc/spapr_irq.c: cpu->tctx = XIVE_TCTX(obj);
hw/ppc/spapr_irq.c: spapr_xive_set_tctx_os_cam(cpu->tctx);
hw/ppc/spapr_irq.c: spapr_xive_set_tctx_os_cam(cpu->tctx);
hw/ppc/spapr_irq.c: spapr_xive_set_tctx_os_cam(cpu->tctx);
It thus looks wrong to expose their type in target/ppc/cpu.h. I guess
they should be hidden behind an opaque data pointer (maybe the existing
void *machine_data ?)
> > That's a bigger change reverting bits of already merged patches. I can
> > take care of it if you prefer.
>
> Could I keep the current patch in my series so that I can get the
> patches finally merged? You could then do any clean up that you like on
> top of it, ok?
>
> > I use a f29 for dev. Which compiler should I install ?
>
> Any version of Clang with -std=gnu99 should do the job here, I think.
>
> Thomas
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode
2019-01-16 11:47 ` Thomas Huth
2019-01-16 13:23 ` Greg Kurz
@ 2019-01-16 13:29 ` Cédric Le Goater
2019-01-17 7:01 ` [Qemu-devel] [Qemu-ppc] " Thomas Huth
1 sibling, 1 reply; 23+ messages in thread
From: Cédric Le Goater @ 2019-01-16 13:29 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
Cc: Daniel P. Berrangé, pbonzini, qemu-ppc, Greg Kurz, David Gibson
On 1/16/19 12:47 PM, Thomas Huth wrote:
> On 2019-01-16 12:43, Cédric Le Goater wrote:
>> On 1/11/19 9:17 AM, Thomas Huth wrote:
>>> When compiling the ppc code with clang and -std=gnu99, there are a
>>> couple of warnings/errors like this one:
>>>
>>> CC ppc64-softmmu/hw/intc/xics.o
>>> In file included from hw/intc/xics.c:35:
>>> include/hw/ppc/xics.h:43:25: error: redefinition of typedef 'ICPState' is a C11 feature
>>> [-Werror,-Wtypedef-redefinition]
>>> typedef struct ICPState ICPState;
>>> ^
>>> target/ppc/cpu.h:1181:25: note: previous definition is here
>>> typedef struct ICPState ICPState;
>>> ^
>>> Work around the problems by including the proper headers instead.
>>
>> Thomas,
>>
>>
>> After a closer look, I think we should use 'void *' under PowerPCCPU
>> as it was the case before I introduced the second interrupt presenter.
>
> If you don't like the #includes, why not simply do anonymous struct
> forward declarations here? I think that would be better than "void *".
yes.
>> That's a bigger change reverting bits of already merged patches. I can
>> take care of it if you prefer.
>
> Could I keep the current patch in my series so that I can get the
> patches finally merged? You could then do any clean up that you like on
> top of it, ok?
OK.
See below the patch I would propose. Compiled tested with clang -std=gnu99.
>> I use a f29 for dev. Which compiler should I install ?
>
> Any version of Clang with -std=gnu99 should do the job here, I think.
It does.
Thanks,
C.
>From 0119d5b2f6fa963f5951897bbadd638f94ecdf4b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= <clg@kaod.org>
Date: Wed, 16 Jan 2019 14:27:25 +0100
Subject: [PATCH] ppc: remove inclusion of hw/ppc header file
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
and use struct forward declarations instead.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
target/ppc/cpu.h | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index bde4dff1ba80..ca5f7f16a820 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -23,11 +23,6 @@
#include "qemu-common.h"
#include "qemu/int128.h"
-#ifndef CONFIG_USER_ONLY
-#include "hw/ppc/xive.h" /* for XiveTCTX */
-#include "hw/ppc/xics.h" /* for ICPState */
-#endif
-
//#define PPC_EMULATE_32BITS_HYPV
#if defined (TARGET_PPC64)
@@ -1204,8 +1199,8 @@ struct PowerPCCPU {
int32_t node_id; /* NUMA node this CPU belongs to */
PPCHash64Options *hash64_opts;
#ifndef CONFIG_USER_ONLY
- ICPState *icp;
- XiveTCTX *tctx;
+ struct ICPState *icp;
+ struct XiveTCTX *tctx;
#endif
/* Fields related to migration compatibility hacks */
--
2.20.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode
2019-01-16 13:23 ` Greg Kurz
@ 2019-01-16 13:44 ` Thomas Huth
2019-01-16 15:11 ` Greg Kurz
0 siblings, 1 reply; 23+ messages in thread
From: Thomas Huth @ 2019-01-16 13:44 UTC (permalink / raw)
To: Greg Kurz
Cc: Cédric Le Goater, qemu-devel, Daniel P. Berrangé,
pbonzini, qemu-ppc, David Gibson
On 2019-01-16 14:23, Greg Kurz wrote:
> On Wed, 16 Jan 2019 12:47:36 +0100
> Thomas Huth <thuth@redhat.com> wrote:
>
>> On 2019-01-16 12:43, Cédric Le Goater wrote:
>>> On 1/11/19 9:17 AM, Thomas Huth wrote:
>>>> When compiling the ppc code with clang and -std=gnu99, there are a
>>>> couple of warnings/errors like this one:
>>>>
>>>> CC ppc64-softmmu/hw/intc/xics.o
>>>> In file included from hw/intc/xics.c:35:
>>>> include/hw/ppc/xics.h:43:25: error: redefinition of typedef 'ICPState' is a C11 feature
>>>> [-Werror,-Wtypedef-redefinition]
>>>> typedef struct ICPState ICPState;
>>>> ^
>>>> target/ppc/cpu.h:1181:25: note: previous definition is here
>>>> typedef struct ICPState ICPState;
>>>> ^
>>>> Work around the problems by including the proper headers instead.
>>>
>>> Thomas,
>>>
>>>
>>> After a closer look, I think we should use 'void *' under PowerPCCPU
>>> as it was the case before I introduced the second interrupt presenter.
>>
>> If you don't like the #includes, why not simply do anonymous struct
>> forward declarations here? I think that would be better than "void *".
>>
>
> That's questionable. These two fields are only used by the machine code and
> the interrupt controller code.
>
> $ git grep -E '(icp|tctx)' target/ppc/
> target/ppc/cpu.h: ICPState *icp;
> target/ppc/cpu.h: XiveTCTX *tctx;
>
> $ git grep -E 'cpu\->(icp|tctx)'
> hw/intc/spapr_xive_kvm.c: kvmppc_xive_cpu_set_state(cpu->tctx, &local_err);
> hw/intc/spapr_xive_kvm.c: kvmppc_xive_cpu_connect(cpu->tctx, &local_err);
> hw/intc/xics_kvm.c: icp_kvm_connect(cpu->icp, &local_err);
> hw/intc/xics_kvm.c: icp_set_kvm_state(cpu->icp, 1);
> hw/intc/xics_spapr.c: icp_set_cppr(cpu->icp, cppr);
> hw/intc/xics_spapr.c: uint32_t xirr = icp_accept(cpu->icp);
> hw/intc/xics_spapr.c: uint32_t xirr = icp_accept(cpu->icp);
> hw/intc/xics_spapr.c: icp_eoi(cpu->icp, xirr);
> hw/intc/xics_spapr.c: uint32_t xirr = icp_ipoll(cpu->icp, &mfrr);
> hw/intc/xive.c: XiveTCTX *tctx = cpu->tctx;
> hw/intc/xive.c: XiveTCTX *tctx = cpu->tctx;
> hw/intc/xive.c: XiveTCTX *tctx = cpu->tctx;
> hw/ppc/pnv.c: cpu->icp = ICP(obj);
> hw/ppc/pnv.c: return cpu ? cpu->icp : NULL;
> hw/ppc/pnv.c: icp_pic_print_info(cpu->icp, mon);
> hw/ppc/pnv_core.c: object_unparent(OBJECT(cpu->icp));
> hw/ppc/spapr.c: return cpu ? cpu->icp : NULL;
> hw/ppc/spapr_cpu_core.c: if (cpu->icp) {
> hw/ppc/spapr_cpu_core.c: object_unparent(OBJECT(cpu->icp));
> hw/ppc/spapr_cpu_core.c: if (cpu->tctx) {
> hw/ppc/spapr_cpu_core.c: object_unparent(OBJECT(cpu->tctx));
> hw/ppc/spapr_irq.c: icp_pic_print_info(cpu->icp, mon);
> hw/ppc/spapr_irq.c: cpu->icp = ICP(obj);
> hw/ppc/spapr_irq.c: icp_resend(cpu->icp);
> hw/ppc/spapr_irq.c: xive_tctx_pic_print_info(cpu->tctx, mon);
> hw/ppc/spapr_irq.c: cpu->tctx = XIVE_TCTX(obj);
> hw/ppc/spapr_irq.c: spapr_xive_set_tctx_os_cam(cpu->tctx);
> hw/ppc/spapr_irq.c: spapr_xive_set_tctx_os_cam(cpu->tctx);
> hw/ppc/spapr_irq.c: spapr_xive_set_tctx_os_cam(cpu->tctx);
>
> It thus looks wrong to expose their type in target/ppc/cpu.h. I guess
> they should be hidden behind an opaque data pointer (maybe the existing
> void *machine_data ?)
While that's maybe cleaner at a quick glance first, you still have to
deal with the fact that it's used by two different machines (pseries and
pnv) ... not sure whether that easily distinguishable in xive.c for example?
Thomas
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode
2019-01-16 13:44 ` Thomas Huth
@ 2019-01-16 15:11 ` Greg Kurz
2019-01-16 17:26 ` Cédric Le Goater
0 siblings, 1 reply; 23+ messages in thread
From: Greg Kurz @ 2019-01-16 15:11 UTC (permalink / raw)
To: Thomas Huth
Cc: Cédric Le Goater, qemu-devel, Daniel P. Berrangé,
pbonzini, qemu-ppc, David Gibson
On Wed, 16 Jan 2019 14:44:55 +0100
Thomas Huth <thuth@redhat.com> wrote:
> On 2019-01-16 14:23, Greg Kurz wrote:
> > On Wed, 16 Jan 2019 12:47:36 +0100
> > Thomas Huth <thuth@redhat.com> wrote:
> >
> >> On 2019-01-16 12:43, Cédric Le Goater wrote:
> >>> On 1/11/19 9:17 AM, Thomas Huth wrote:
> >>>> When compiling the ppc code with clang and -std=gnu99, there are a
> >>>> couple of warnings/errors like this one:
> >>>>
> >>>> CC ppc64-softmmu/hw/intc/xics.o
> >>>> In file included from hw/intc/xics.c:35:
> >>>> include/hw/ppc/xics.h:43:25: error: redefinition of typedef 'ICPState' is a C11 feature
> >>>> [-Werror,-Wtypedef-redefinition]
> >>>> typedef struct ICPState ICPState;
> >>>> ^
> >>>> target/ppc/cpu.h:1181:25: note: previous definition is here
> >>>> typedef struct ICPState ICPState;
> >>>> ^
> >>>> Work around the problems by including the proper headers instead.
> >>>
> >>> Thomas,
> >>>
> >>>
> >>> After a closer look, I think we should use 'void *' under PowerPCCPU
> >>> as it was the case before I introduced the second interrupt presenter.
> >>
> >> If you don't like the #includes, why not simply do anonymous struct
> >> forward declarations here? I think that would be better than "void *".
> >>
> >
> > That's questionable. These two fields are only used by the machine code and
> > the interrupt controller code.
> >
> > $ git grep -E '(icp|tctx)' target/ppc/
> > target/ppc/cpu.h: ICPState *icp;
> > target/ppc/cpu.h: XiveTCTX *tctx;
> >
> > $ git grep -E 'cpu\->(icp|tctx)'
> > hw/intc/spapr_xive_kvm.c: kvmppc_xive_cpu_set_state(cpu->tctx, &local_err);
> > hw/intc/spapr_xive_kvm.c: kvmppc_xive_cpu_connect(cpu->tctx, &local_err);
> > hw/intc/xics_kvm.c: icp_kvm_connect(cpu->icp, &local_err);
> > hw/intc/xics_kvm.c: icp_set_kvm_state(cpu->icp, 1);
> > hw/intc/xics_spapr.c: icp_set_cppr(cpu->icp, cppr);
> > hw/intc/xics_spapr.c: uint32_t xirr = icp_accept(cpu->icp);
> > hw/intc/xics_spapr.c: uint32_t xirr = icp_accept(cpu->icp);
> > hw/intc/xics_spapr.c: icp_eoi(cpu->icp, xirr);
> > hw/intc/xics_spapr.c: uint32_t xirr = icp_ipoll(cpu->icp, &mfrr);
> > hw/intc/xive.c: XiveTCTX *tctx = cpu->tctx;
> > hw/intc/xive.c: XiveTCTX *tctx = cpu->tctx;
> > hw/intc/xive.c: XiveTCTX *tctx = cpu->tctx;
> > hw/ppc/pnv.c: cpu->icp = ICP(obj);
> > hw/ppc/pnv.c: return cpu ? cpu->icp : NULL;
> > hw/ppc/pnv.c: icp_pic_print_info(cpu->icp, mon);
> > hw/ppc/pnv_core.c: object_unparent(OBJECT(cpu->icp));
> > hw/ppc/spapr.c: return cpu ? cpu->icp : NULL;
> > hw/ppc/spapr_cpu_core.c: if (cpu->icp) {
> > hw/ppc/spapr_cpu_core.c: object_unparent(OBJECT(cpu->icp));
> > hw/ppc/spapr_cpu_core.c: if (cpu->tctx) {
> > hw/ppc/spapr_cpu_core.c: object_unparent(OBJECT(cpu->tctx));
> > hw/ppc/spapr_irq.c: icp_pic_print_info(cpu->icp, mon);
> > hw/ppc/spapr_irq.c: cpu->icp = ICP(obj);
> > hw/ppc/spapr_irq.c: icp_resend(cpu->icp);
> > hw/ppc/spapr_irq.c: xive_tctx_pic_print_info(cpu->tctx, mon);
> > hw/ppc/spapr_irq.c: cpu->tctx = XIVE_TCTX(obj);
> > hw/ppc/spapr_irq.c: spapr_xive_set_tctx_os_cam(cpu->tctx);
> > hw/ppc/spapr_irq.c: spapr_xive_set_tctx_os_cam(cpu->tctx);
> > hw/ppc/spapr_irq.c: spapr_xive_set_tctx_os_cam(cpu->tctx);
> >
> > It thus looks wrong to expose their type in target/ppc/cpu.h. I guess
> > they should be hidden behind an opaque data pointer (maybe the existing
> > void *machine_data ?)
>
> While that's maybe cleaner at a quick glance first, you still have to
> deal with the fact that it's used by two different machines (pseries and
> pnv) ... not sure whether that easily distinguishable in xive.c for example?
>
FWIW xive.c is only used by pseries for the moment but this is likely to
change in the future. Cedric already has patches for this.
Anyway, your concern is legitimate, so let's have a closer look at those
cpu->icp and cpu->tctx users. Most of them are something like:
CPU_FOREACH(cs) {
PowerPCCPU *cpu = POWERPC_CPU(cs);
// Do something with cpu->icp or cpu->tctx exclusively,
// not any other field of PowerPCCPU.
}
<thinking aloud>
I guess these could be addressed by having a list of XiveTCTX and a
list of ICPState instead of _hijacking_ the CPU list.
</thinking aloud>
Then we have the hcalls in xics_spapr.c. Since we know this is
pseries, we could rely on the existing void *machine_data, ie,
move the cpu->icp to sPAPRCPUState and access it thanks
to spapr_cpu_state().
And then, we have the xive_tm_ops in xive.c, which currently
does:
static void xive_tm_write(void *opaque, hwaddr offset,
uint64_t value, unsigned size)
{
PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
XiveTCTX *tctx = cpu->tctx;
It could do something like:
XiveTCTX *tctx = xive_router_get_tm_tctx(opaque, offset);
with a new XiveRouterClass ->get_tm_tctx() method:
XiveTCTX *xive_router_get_tm_tctx(XiveRouter *xrtr, hwaddr offset)
{
XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr);
return xrc->get_tm_tctx(xrtr, offset);
}
On pseries, if we move cpu->tctx to sPAPRCPUState, then we just
need to implement the ->get_tm_tctx() in spapr_xive.c as:
static XiveTCTX *spapr_xive_get_tm_tctx(XiveRouter *xrtr, hwaddr offset)
{
return spapr_cpu_state(POWERPC_CPU(current_cpu))->tctx;
}
Something similar should be done for pnv.
Cheers,
--
Greg
> Thomas
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode
2019-01-16 15:11 ` Greg Kurz
@ 2019-01-16 17:26 ` Cédric Le Goater
0 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2019-01-16 17:26 UTC (permalink / raw)
To: Greg Kurz, Thomas Huth
Cc: qemu-devel, Daniel P. Berrangé, pbonzini, qemu-ppc, David Gibson
On 1/16/19 4:11 PM, Greg Kurz wrote:
> On Wed, 16 Jan 2019 14:44:55 +0100
> Thomas Huth <thuth@redhat.com> wrote:
>
>> On 2019-01-16 14:23, Greg Kurz wrote:
>>> On Wed, 16 Jan 2019 12:47:36 +0100
>>> Thomas Huth <thuth@redhat.com> wrote:
>>>
>>>> On 2019-01-16 12:43, Cédric Le Goater wrote:
>>>>> On 1/11/19 9:17 AM, Thomas Huth wrote:
>>>>>> When compiling the ppc code with clang and -std=gnu99, there are a
>>>>>> couple of warnings/errors like this one:
>>>>>>
>>>>>> CC ppc64-softmmu/hw/intc/xics.o
>>>>>> In file included from hw/intc/xics.c:35:
>>>>>> include/hw/ppc/xics.h:43:25: error: redefinition of typedef 'ICPState' is a C11 feature
>>>>>> [-Werror,-Wtypedef-redefinition]
>>>>>> typedef struct ICPState ICPState;
>>>>>> ^
>>>>>> target/ppc/cpu.h:1181:25: note: previous definition is here
>>>>>> typedef struct ICPState ICPState;
>>>>>> ^
>>>>>> Work around the problems by including the proper headers instead.
>>>>>
>>>>> Thomas,
>>>>>
>>>>>
>>>>> After a closer look, I think we should use 'void *' under PowerPCCPU
>>>>> as it was the case before I introduced the second interrupt presenter.
>>>>
>>>> If you don't like the #includes, why not simply do anonymous struct
>>>> forward declarations here? I think that would be better than "void *".
>>>>
>>>
>>> That's questionable. These two fields are only used by the machine code and
>>> the interrupt controller code.
>>>
>>> $ git grep -E '(icp|tctx)' target/ppc/
>>> target/ppc/cpu.h: ICPState *icp;
>>> target/ppc/cpu.h: XiveTCTX *tctx;
>>>
>>> $ git grep -E 'cpu\->(icp|tctx)'
>>> hw/intc/spapr_xive_kvm.c: kvmppc_xive_cpu_set_state(cpu->tctx, &local_err);
>>> hw/intc/spapr_xive_kvm.c: kvmppc_xive_cpu_connect(cpu->tctx, &local_err);
>>> hw/intc/xics_kvm.c: icp_kvm_connect(cpu->icp, &local_err);
>>> hw/intc/xics_kvm.c: icp_set_kvm_state(cpu->icp, 1);
>>> hw/intc/xics_spapr.c: icp_set_cppr(cpu->icp, cppr);
>>> hw/intc/xics_spapr.c: uint32_t xirr = icp_accept(cpu->icp);
>>> hw/intc/xics_spapr.c: uint32_t xirr = icp_accept(cpu->icp);
>>> hw/intc/xics_spapr.c: icp_eoi(cpu->icp, xirr);
>>> hw/intc/xics_spapr.c: uint32_t xirr = icp_ipoll(cpu->icp, &mfrr);
>>> hw/intc/xive.c: XiveTCTX *tctx = cpu->tctx;
>>> hw/intc/xive.c: XiveTCTX *tctx = cpu->tctx;
>>> hw/intc/xive.c: XiveTCTX *tctx = cpu->tctx;
>>> hw/ppc/pnv.c: cpu->icp = ICP(obj);
>>> hw/ppc/pnv.c: return cpu ? cpu->icp : NULL;
>>> hw/ppc/pnv.c: icp_pic_print_info(cpu->icp, mon);
>>> hw/ppc/pnv_core.c: object_unparent(OBJECT(cpu->icp));
>>> hw/ppc/spapr.c: return cpu ? cpu->icp : NULL;
>>> hw/ppc/spapr_cpu_core.c: if (cpu->icp) {
>>> hw/ppc/spapr_cpu_core.c: object_unparent(OBJECT(cpu->icp));
>>> hw/ppc/spapr_cpu_core.c: if (cpu->tctx) {
>>> hw/ppc/spapr_cpu_core.c: object_unparent(OBJECT(cpu->tctx));
>>> hw/ppc/spapr_irq.c: icp_pic_print_info(cpu->icp, mon);
>>> hw/ppc/spapr_irq.c: cpu->icp = ICP(obj);
>>> hw/ppc/spapr_irq.c: icp_resend(cpu->icp);
>>> hw/ppc/spapr_irq.c: xive_tctx_pic_print_info(cpu->tctx, mon);
>>> hw/ppc/spapr_irq.c: cpu->tctx = XIVE_TCTX(obj);
>>> hw/ppc/spapr_irq.c: spapr_xive_set_tctx_os_cam(cpu->tctx);
>>> hw/ppc/spapr_irq.c: spapr_xive_set_tctx_os_cam(cpu->tctx);
>>> hw/ppc/spapr_irq.c: spapr_xive_set_tctx_os_cam(cpu->tctx);
>>>
>>> It thus looks wrong to expose their type in target/ppc/cpu.h. I guess
>>> they should be hidden behind an opaque data pointer (maybe the existing
>>> void *machine_data ?)
yes. That was sort of a mid term plan but I guess it has become short
term now.
Anyhow, I don't think we should block Thomas gnu99 patchset because
it's not ready yet.
>> While that's maybe cleaner at a quick glance first, you still have to
>> deal with the fact that it's used by two different machines (pseries and
>> pnv) ... not sure whether that easily distinguishable in xive.c for example?
>>
>
> FWIW xive.c is only used by pseries for the moment but this is likely to
> change in the future. Cedric already has patches for this.
Yes. Because XIVE on baremetal has a way to indirectly perform load/store
on the TIMA. So we can not just use the current_cpu anymore and we need
to peek at the XIVE controller registers to know on behalf of which
CPU we are accessing memory. This requires an extra method in the
XiveRouter that we could use for the cleanups you are proposing.
> Anyway, your concern is legitimate, so let's have a closer look at those
> cpu->icp and cpu->tctx users. Most of them are something like:
>
> CPU_FOREACH(cs) {
> PowerPCCPU *cpu = POWERPC_CPU(cs);
>
> // Do something with cpu->icp or cpu->tctx exclusively,
> // not any other field of PowerPCCPU.
> }
>
> <thinking aloud>
> I guess these could be addressed by having a list of XiveTCTX and a
> list of ICPState instead of _hijacking_ the CPU list.
> </thinking aloud>
We removed the list of ICPState in 2.9 I think because it was
a problem at the time. I don't remember exactly which but adding it
back does not seem a good idea to me.
> Then we have the hcalls in xics_spapr.c. Since we know this is
> pseries, we could rely on the existing void *machine_data, ie,
> move the cpu->icp to sPAPRCPUState and access it thanks
> to spapr_cpu_state().
yes.
> And then, we have the xive_tm_ops in xive.c, which currently
> does:
>
> static void xive_tm_write(void *opaque, hwaddr offset,
> uint64_t value, unsigned size)
> {
> PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
> XiveTCTX *tctx = cpu->tctx;
>
> It could do something like:
>
> XiveTCTX *tctx = xive_router_get_tm_tctx(opaque, offset);
>
> with a new XiveRouterClass ->get_tm_tctx() method:
>
> XiveTCTX *xive_router_get_tm_tctx(XiveRouter *xrtr, hwaddr offset)
> {
> XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr);
>
> return xrc->get_tm_tctx(xrtr, offset);
> }
Yes. The Pnv method is nearly doing that today.
> On pseries, if we move cpu->tctx to sPAPRCPUState, then we just
> need to implement the ->get_tm_tctx() in spapr_xive.c as:
>
> static XiveTCTX *spapr_xive_get_tm_tctx(XiveRouter *xrtr, hwaddr offset)
> {
> return spapr_cpu_state(POWERPC_CPU(current_cpu))->tctx;
> }
>
> Something similar should be done for pnv.
yes.
It seems like it would be a good cleanup before introducing more
changes for KVM and Pnv. The CPU_FOREACH() is the only problem
I see. I will take a look at it.
Thanks,
C.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode
2019-01-16 13:29 ` Cédric Le Goater
@ 2019-01-17 7:01 ` Thomas Huth
2019-01-17 7:58 ` Greg Kurz
2019-01-17 8:02 ` Cédric Le Goater
0 siblings, 2 replies; 23+ messages in thread
From: Thomas Huth @ 2019-01-17 7:01 UTC (permalink / raw)
To: Cédric Le Goater, qemu-devel
Cc: qemu-ppc, pbonzini, Daniel P. Berrangé, Greg Kurz, David Gibson
On 2019-01-16 14:29, Cédric Le Goater wrote:
> On 1/16/19 12:47 PM, Thomas Huth wrote:
>> On 2019-01-16 12:43, Cédric Le Goater wrote:
>>> On 1/11/19 9:17 AM, Thomas Huth wrote:
>>>> When compiling the ppc code with clang and -std=gnu99, there are a
>>>> couple of warnings/errors like this one:
>>>>
>>>> CC ppc64-softmmu/hw/intc/xics.o
>>>> In file included from hw/intc/xics.c:35:
>>>> include/hw/ppc/xics.h:43:25: error: redefinition of typedef 'ICPState' is a C11 feature
>>>> [-Werror,-Wtypedef-redefinition]
>>>> typedef struct ICPState ICPState;
>>>> ^
>>>> target/ppc/cpu.h:1181:25: note: previous definition is here
>>>> typedef struct ICPState ICPState;
>>>> ^
>>>> Work around the problems by including the proper headers instead.
>>>
>>> Thomas,
>>>
>>>
>>> After a closer look, I think we should use 'void *' under PowerPCCPU
>>> as it was the case before I introduced the second interrupt presenter.
>>
>> If you don't like the #includes, why not simply do anonymous struct
>> forward declarations here? I think that would be better than "void *".
>
> yes.
>
>>> That's a bigger change reverting bits of already merged patches. I can
>>> take care of it if you prefer.
>>
>> Could I keep the current patch in my series so that I can get the
>> patches finally merged? You could then do any clean up that you like on
>> top of it, ok?
>
> OK.
>
> See below the patch I would propose. Compiled tested with clang -std=gnu99.
[...]
> @@ -1204,8 +1199,8 @@ struct PowerPCCPU {
> int32_t node_id; /* NUMA node this CPU belongs to */
> PPCHash64Options *hash64_opts;
> #ifndef CONFIG_USER_ONLY
> - ICPState *icp;
> - XiveTCTX *tctx;
> + struct ICPState *icp;
> + struct XiveTCTX *tctx;
> #endif
That's pretty much what I had in an earlier version of my patch:
https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg01810.html
But Greg did not like it:
https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg01893.html
Thomas
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode
2019-01-17 7:01 ` [Qemu-devel] [Qemu-ppc] " Thomas Huth
@ 2019-01-17 7:58 ` Greg Kurz
2019-01-17 8:02 ` Cédric Le Goater
1 sibling, 0 replies; 23+ messages in thread
From: Greg Kurz @ 2019-01-17 7:58 UTC (permalink / raw)
To: Thomas Huth
Cc: Cédric Le Goater, qemu-devel, qemu-ppc, pbonzini,
Daniel P. Berrangé,
David Gibson
On Thu, 17 Jan 2019 08:01:25 +0100
Thomas Huth <thuth@redhat.com> wrote:
> On 2019-01-16 14:29, Cédric Le Goater wrote:
> > On 1/16/19 12:47 PM, Thomas Huth wrote:
> >> On 2019-01-16 12:43, Cédric Le Goater wrote:
> >>> On 1/11/19 9:17 AM, Thomas Huth wrote:
> >>>> When compiling the ppc code with clang and -std=gnu99, there are a
> >>>> couple of warnings/errors like this one:
> >>>>
> >>>> CC ppc64-softmmu/hw/intc/xics.o
> >>>> In file included from hw/intc/xics.c:35:
> >>>> include/hw/ppc/xics.h:43:25: error: redefinition of typedef 'ICPState' is a C11 feature
> >>>> [-Werror,-Wtypedef-redefinition]
> >>>> typedef struct ICPState ICPState;
> >>>> ^
> >>>> target/ppc/cpu.h:1181:25: note: previous definition is here
> >>>> typedef struct ICPState ICPState;
> >>>> ^
> >>>> Work around the problems by including the proper headers instead.
> >>>
> >>> Thomas,
> >>>
> >>>
> >>> After a closer look, I think we should use 'void *' under PowerPCCPU
> >>> as it was the case before I introduced the second interrupt presenter.
> >>
> >> If you don't like the #includes, why not simply do anonymous struct
> >> forward declarations here? I think that would be better than "void *".
> >
> > yes.
> >
> >>> That's a bigger change reverting bits of already merged patches. I can
> >>> take care of it if you prefer.
> >>
> >> Could I keep the current patch in my series so that I can get the
> >> patches finally merged? You could then do any clean up that you like on
> >> top of it, ok?
> >
> > OK.
> >
> > See below the patch I would propose. Compiled tested with clang -std=gnu99.
> [...]
> > @@ -1204,8 +1199,8 @@ struct PowerPCCPU {
> > int32_t node_id; /* NUMA node this CPU belongs to */
> > PPCHash64Options *hash64_opts;
> > #ifndef CONFIG_USER_ONLY
> > - ICPState *icp;
> > - XiveTCTX *tctx;
> > + struct ICPState *icp;
> > + struct XiveTCTX *tctx;
> > #endif
>
> That's pretty much what I had in an earlier version of my patch:
>
> https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg01810.html
>
> But Greg did not like it:
>
> https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg01893.html
>
Yeah I didn't but the #includes bring even more troubles so I won't object
if we go for struct :) For the long term, I still think that icp and tctx
should be hidden behind void *machine_data in a per-machine struct.
> Thomas
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode
2019-01-17 7:01 ` [Qemu-devel] [Qemu-ppc] " Thomas Huth
2019-01-17 7:58 ` Greg Kurz
@ 2019-01-17 8:02 ` Cédric Le Goater
1 sibling, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2019-01-17 8:02 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
Cc: qemu-ppc, pbonzini, Daniel P. Berrangé, Greg Kurz, David Gibson
On 1/17/19 8:01 AM, Thomas Huth wrote:
> On 2019-01-16 14:29, Cédric Le Goater wrote:
>> On 1/16/19 12:47 PM, Thomas Huth wrote:
>>> On 2019-01-16 12:43, Cédric Le Goater wrote:
>>>> On 1/11/19 9:17 AM, Thomas Huth wrote:
>>>>> When compiling the ppc code with clang and -std=gnu99, there are a
>>>>> couple of warnings/errors like this one:
>>>>>
>>>>> CC ppc64-softmmu/hw/intc/xics.o
>>>>> In file included from hw/intc/xics.c:35:
>>>>> include/hw/ppc/xics.h:43:25: error: redefinition of typedef 'ICPState' is a C11 feature
>>>>> [-Werror,-Wtypedef-redefinition]
>>>>> typedef struct ICPState ICPState;
>>>>> ^
>>>>> target/ppc/cpu.h:1181:25: note: previous definition is here
>>>>> typedef struct ICPState ICPState;
>>>>> ^
>>>>> Work around the problems by including the proper headers instead.
>>>>
>>>> Thomas,
>>>>
>>>>
>>>> After a closer look, I think we should use 'void *' under PowerPCCPU
>>>> as it was the case before I introduced the second interrupt presenter.
>>>
>>> If you don't like the #includes, why not simply do anonymous struct
>>> forward declarations here? I think that would be better than "void *".
>>
>> yes.
>>
>>>> That's a bigger change reverting bits of already merged patches. I can
>>>> take care of it if you prefer.
>>>
>>> Could I keep the current patch in my series so that I can get the
>>> patches finally merged? You could then do any clean up that you like on
>>> top of it, ok?
>>
>> OK.
>>
>> See below the patch I would propose. Compiled tested with clang -std=gnu99.
> [...]
>> @@ -1204,8 +1199,8 @@ struct PowerPCCPU {
>> int32_t node_id; /* NUMA node this CPU belongs to */
>> PPCHash64Options *hash64_opts;
>> #ifndef CONFIG_USER_ONLY
>> - ICPState *icp;
>> - XiveTCTX *tctx;
>> + struct ICPState *icp;
>> + struct XiveTCTX *tctx;
>> #endif
>
> That's pretty much what I had in an earlier version of my patch:
>
> https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg01810.html
>
> But Greg did not like it:
>
> https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg01893.html
yeah :/ Sorry for all the fuss. We have been tackling this problem for
years now.
I think the patchset I just sent is good step forward and I checked
compilation with clang -std=gn99 !
Thanks,
C.
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2019-01-17 8:03 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-11 8:17 [Qemu-devel] [PATCH v5 0/3] Force the C standard to gnu99 Thomas Huth
2019-01-11 8:17 ` [Qemu-devel] [PATCH v5 1/3] ppc: Move spapr-related prototypes from xics.h into a seperate header file Thomas Huth
2019-01-11 9:22 ` Daniel P. Berrangé
2019-01-11 12:58 ` Philippe Mathieu-Daudé
2019-01-11 8:17 ` [Qemu-devel] [PATCH v5 2/3] ppc: Fix duplicated typedefs to be able to compile with Clang in gnu99 mode Thomas Huth
2019-01-11 8:30 ` Cédric Le Goater
2019-01-11 8:40 ` Greg Kurz
2019-01-11 9:22 ` Daniel P. Berrangé
2019-01-11 12:59 ` Philippe Mathieu-Daudé
2019-01-16 11:43 ` Cédric Le Goater
2019-01-16 11:47 ` Thomas Huth
2019-01-16 13:23 ` Greg Kurz
2019-01-16 13:44 ` Thomas Huth
2019-01-16 15:11 ` Greg Kurz
2019-01-16 17:26 ` Cédric Le Goater
2019-01-16 13:29 ` Cédric Le Goater
2019-01-17 7:01 ` [Qemu-devel] [Qemu-ppc] " Thomas Huth
2019-01-17 7:58 ` Greg Kurz
2019-01-17 8:02 ` Cédric Le Goater
2019-01-11 8:17 ` [Qemu-devel] [PATCH v5 3/3] configure: Force the C standard to gnu99 Thomas Huth
2019-01-11 8:44 ` Greg Kurz
2019-01-11 9:23 ` Daniel P. Berrangé
2019-01-11 10:55 ` Alex Bennée
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.