2020-02-17 17:01:22

by Qian Cai

[permalink] [raw]
Subject: [PATCH v2] kvm/emulate: fix a -Werror=cast-function-type

arch/x86/kvm/emulate.c: In function 'x86_emulate_insn':
arch/x86/kvm/emulate.c:5686:22: error: cast between incompatible
function types from 'int (*)(struct x86_emulate_ctxt *)' to 'void
(*)(struct fastop *)' [-Werror=cast-function-type]
rc = fastop(ctxt, (fastop_t)ctxt->execute);

Fix it by using an unnamed union of a (*execute) function pointer and a
(*fastop) function pointer.

Fixes: 3009afc6e39e ("KVM: x86: Use a typedef for fastop functions")
Suggested-by: Paolo Bonzini <[email protected]>
Signed-off-by: Qian Cai <[email protected]>
---

v2: use an unnamed union.

arch/x86/include/asm/kvm_emulate.h | 13 ++++++++++++-
arch/x86/kvm/emulate.c | 36 ++++++++++++++----------------------
2 files changed, 26 insertions(+), 23 deletions(-)

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)
--
1.8.3.1


2020-02-17 17:14:08

by Paolo Bonzini

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

On 17/02/20 17:48, Qian Cai wrote:
> arch/x86/kvm/emulate.c: In function 'x86_emulate_insn':
> arch/x86/kvm/emulate.c:5686:22: error: cast between incompatible
> function types from 'int (*)(struct x86_emulate_ctxt *)' to 'void
> (*)(struct fastop *)' [-Werror=cast-function-type]
> rc = fastop(ctxt, (fastop_t)ctxt->execute);
>
> Fix it by using an unnamed union of a (*execute) function pointer and a
> (*fastop) function pointer.
>
> Fixes: 3009afc6e39e ("KVM: x86: Use a typedef for fastop functions")
> Suggested-by: Paolo Bonzini <[email protected]>
> Signed-off-by: Qian Cai <[email protected]>
> ---
>
> v2: use an unnamed union.
>
> arch/x86/include/asm/kvm_emulate.h | 13 ++++++++++++-
> arch/x86/kvm/emulate.c | 36 ++++++++++++++----------------------
> 2 files changed, 26 insertions(+), 23 deletions(-)
>
> 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)
>

Queued, thank you very much.

Paolo