Re: [PATCH] kvm/emulate: fix a -Werror=cast-function-type

[Qian Cai]

On Fri, 2020-02-14 at 20:33 +0100, Paolo Bonzini wrote:
> On 14/02/20 20:14, Qian Cai wrote:
> > > It seems misguided to define a local variable just to get an implicit
> > > cast from (void *) to (fastop_t). Sean's first suggestion gives you
> > > the same implicit cast without the local variable. The second
> > > suggestion makes both casts explicit.
> > 
> > OK, I'll do a v2 using the first suggestion which looks simpler once it passed
> > compilations.
> > 
> 
> Another interesting possibility is to use an unnamed union of a
> (*execute) function pointer and a (*fastop) function pointer.
> 

This?

diff --git a/arch/x86/include/asm/kvm_emulate.h
b/arch/x86/include/asm/kvm_emulate.h
index 03946eb3e2b9..2a8f2bd2e5cf 100644
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -292,6 +292,14 @@ enum x86emul_mode {
Â#define X86EMUL_SMM_MASKÂÂÂÂÂÂÂÂÂÂÂÂÂ(1 << 6)
Â#define X86EMUL_SMM_INSIDE_NMI_MASKÂÂ(1 << 7)
Â
+/*
+ * fastop functions are declared as taking a never-defined fastop parameter,
+ * so they can't be called from C directly.
+ */
+struct fastop;
+
+typedef void (*fastop_t)(struct fastop *);
+
Âstruct x86_emulate_ctxt {
Â	const struct x86_emulate_ops *ops;
Â
@@ -324,7 +332,10 @@ struct x86_emulate_ctxt {
Â	struct operand src;
Â	struct operand src2;
Â	struct operand dst;
-	int (*execute)(struct x86_emulate_ctxt *ctxt);
+	union {
+		int (*execute)(struct x86_emulate_ctxt *ctxt);
+		fastop_t fop;
+	};
Â	int (*check_perm)(struct x86_emulate_ctxt *ctxt);
Â	/*
Â	Â* The following six fields are cleared together,
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index ddbc61984227..dd19fb3539e0 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -191,25 +191,6 @@
Â#define NR_FASTOP (ilog2(sizeof(ulong)) + 1)
Â#define FASTOP_SIZE 8
Â
-/*
- * fastop functions have a special calling convention:
- *
- * dst:ÂÂÂÂraxÂÂÂÂÂÂÂÂ(in/out)
- * src:ÂÂÂÂrdxÂÂÂÂÂÂÂÂ(in/out)
- * src2:ÂÂÂrcxÂÂÂÂÂÂÂÂ(in)
- * flags:ÂÂrflagsÂÂÂÂÂ(in/out)
- * ex:ÂÂÂÂÂrsiÂÂÂÂÂÂÂÂ(in:fastop pointer, out:zero if exception)
- *
- * Moreover, they are all exactly FASTOP_SIZE bytes long, so functions for
- * different operand sizes can be reached by calculation, rather than a jump
- * table (which would be bigger than the code).
- *
- * fastop functions are declared as taking a never-defined fastop parameter,
- * so they can't be called from C directly.
- */
-
-struct fastop;
-
Âstruct opcode {
Â	u64 flags : 56;
Â	u64 intercept : 8;
@@ -311,8 +292,19 @@ static void invalidate_registers(struct x86_emulate_ctxt
*ctxt)
Â#define ON64(x)
Â#endif
Â
-typedef void (*fastop_t)(struct fastop *);
-
+/*
+ * fastop functions have a special calling convention:
+ *
+ * dst:ÂÂÂÂraxÂÂÂÂÂÂÂÂ(in/out)
+ * src:ÂÂÂÂrdxÂÂÂÂÂÂÂÂ(in/out)
+ * src2:ÂÂÂrcxÂÂÂÂÂÂÂÂ(in)
+ * flags:ÂÂrflagsÂÂÂÂÂ(in/out)
+ * ex:ÂÂÂÂÂrsiÂÂÂÂÂÂÂÂ(in:fastop pointer, out:zero if exception)
+ *
+ * Moreover, they are all exactly FASTOP_SIZE bytes long, so functions for
+ * different operand sizes can be reached by calculation, rather than a jump
+ * table (which would be bigger than the code).
+ */
Âstatic int fastop(struct x86_emulate_ctxt *ctxt, fastop_t fop);
Â
Â#define __FOP_FUNC(name) \
@@ -5683,7 +5675,7 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
Â
Â	if (ctxt->execute) {
Â		if (ctxt->d & Fastop)
-			rc = fastop(ctxt, (fastop_t)ctxt->execute);
+			rc = fastop(ctxt, ctxt->fop);
Â		else
Â			rc = ctxt->execute(ctxt);
Â		if (rc != X86EMUL_CONTINUE)