Re: [PATCH 11/24] target/i386: Restrict cpu_exec_interrupt() handler to sysemu

From: Warner Losh <imp@bsdimp.com>
To: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Bin Meng <bin.meng@windriver.com>,
	Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Max Filippov <jcmvbkbc@gmail.com>,
	Alistair Francis <alistair.francis@wdc.com>,
	"Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
	Marek Vasut <marex@denx.de>,
	Yoshinori Sato <ysato@users.sourceforge.jp>,
	qemu-ppc <qemu-ppc@nongnu.org>,
	Artyom Tarasenko <atar4qemu@gmail.com>,
	Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	Kyle Evans <kevans@freebsd.org>,
	Richard Henderson <richard.henderson@linaro.org>,
	Greg Kurz <groug@kaod.org>, qemu-arm <qemu-arm@nongnu.org>,
	Michael Rolnik <mrolnik@gmail.com>,
	Stafford Horne <shorne@gmail.com>,
	David Gibson <david@gibson.dropbear.id.au>,
	qemu-riscv@nongnu.org, Chris Wulff <crwulff@gmail.com>,
	Laurent Vivier <laurent@vivier.eu>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Aurelien Jarno <aurelien@aurel32.net>
Subject: Re: [PATCH 11/24] target/i386: Restrict cpu_exec_interrupt() handler to sysemu
Date: Thu, 2 Sep 2021 14:18:34 -0600	[thread overview]
Message-ID: <CANCZdfoM82TXdpRzqZ46coTuy441MQ2n7xJoBNk4gZF-6HObuQ@mail.gmail.com> (raw)
In-Reply-To: <20210902151715.383678-12-f4bug@amsat.org>

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

On Thu, Sep 2, 2021 at 9:18 AM Philippe Mathieu-Daudé <f4bug@amsat.org>
wrote:

> Restrict cpu_exec_interrupt() and its callees to sysemu.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/i386/tcg/helper-tcg.h |  2 ++
>  target/i386/tcg/seg_helper.c | 10 ++--------
>  target/i386/tcg/tcg-cpu.c    |  2 +-
>  3 files changed, 5 insertions(+), 9 deletions(-)
>

Reviewed-by: Warner Losh <imp@bsdimp.com>

> diff --git a/target/i386/tcg/helper-tcg.h b/target/i386/tcg/helper-tcg.h
> index 2510cc244e9..60ca09e95eb 100644
> --- a/target/i386/tcg/helper-tcg.h
> +++ b/target/i386/tcg/helper-tcg.h
> @@ -38,7 +38,9 @@ QEMU_BUILD_BUG_ON(TCG_PHYS_ADDR_BITS >
> TARGET_PHYS_ADDR_SPACE_BITS);
>   * @cpu: vCPU the interrupt is to be handled by.
>   */
>  void x86_cpu_do_interrupt(CPUState *cpu);
> +#ifndef CONFIG_USER_ONLY
>  bool x86_cpu_exec_interrupt(CPUState *cpu, int int_req);
> +#endif
>
>  /* helper.c */
>  bool x86_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
> index dee7bef68c6..13c6e6ee62e 100644
> --- a/target/i386/tcg/seg_helper.c
> +++ b/target/i386/tcg/seg_helper.c
> @@ -1110,6 +1110,7 @@ void do_interrupt_x86_hardirq(CPUX86State *env, int
> intno, int is_hw)
>      do_interrupt_all(env_archcpu(env), intno, 0, 0, 0, is_hw);
>  }
>
> +#ifndef CONFIG_USER_ONLY
>  bool x86_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>  {
>      X86CPU *cpu = X86_CPU(cs);
> @@ -1125,23 +1126,17 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int
> interrupt_request)
>       * This is required to make icount-driven execution deterministic.
>       */
>      switch (interrupt_request) {
> -#if !defined(CONFIG_USER_ONLY)
>      case CPU_INTERRUPT_POLL:
>          cs->interrupt_request &= ~CPU_INTERRUPT_POLL;
>          apic_poll_irq(cpu->apic_state);
>          break;
> -#endif
>      case CPU_INTERRUPT_SIPI:
>          do_cpu_sipi(cpu);
>          break;
>      case CPU_INTERRUPT_SMI:
>          cpu_svm_check_intercept_param(env, SVM_EXIT_SMI, 0, 0);
>          cs->interrupt_request &= ~CPU_INTERRUPT_SMI;
> -#ifdef CONFIG_USER_ONLY
> -        cpu_abort(CPU(cpu), "SMI interrupt: cannot enter SMM in
> user-mode");
> -#else
>          do_smm_enter(cpu);
> -#endif /* CONFIG_USER_ONLY */
>          break;
>      case CPU_INTERRUPT_NMI:
>          cpu_svm_check_intercept_param(env, SVM_EXIT_NMI, 0, 0);
> @@ -1162,7 +1157,6 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int
> interrupt_request)
>                        "Servicing hardware INT=0x%02x\n", intno);
>          do_interrupt_x86_hardirq(env, intno, 1);
>          break;
> -#if !defined(CONFIG_USER_ONLY)
>      case CPU_INTERRUPT_VIRQ:
>          /* FIXME: this should respect TPR */
>          cpu_svm_check_intercept_param(env, SVM_EXIT_VINTR, 0, 0);
> @@ -1173,12 +1167,12 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int
> interrupt_request)
>          do_interrupt_x86_hardirq(env, intno, 1);
>          cs->interrupt_request &= ~CPU_INTERRUPT_VIRQ;
>          break;
> -#endif
>      }
>
>      /* Ensure that no TB jump will be modified as the program flow was
> changed.  */
>      return true;
>  }
> +#endif /* CONFIG_USER_ONLY */
>
>  void helper_lldt(CPUX86State *env, int selector)
>  {
> diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c
> index dce800a8953..fd86daf93d2 100644
> --- a/target/i386/tcg/tcg-cpu.c
> +++ b/target/i386/tcg/tcg-cpu.c
> @@ -72,12 +72,12 @@ static const struct TCGCPUOps x86_tcg_ops = {
>      .synchronize_from_tb = x86_cpu_synchronize_from_tb,
>      .cpu_exec_enter = x86_cpu_exec_enter,
>      .cpu_exec_exit = x86_cpu_exec_exit,
> -    .cpu_exec_interrupt = x86_cpu_exec_interrupt,
>      .tlb_fill = x86_cpu_tlb_fill,
>  #ifdef CONFIG_USER_ONLY
>      .fake_user_exception = x86_cpu_do_interrupt,
>  #else
>      .do_interrupt = x86_cpu_do_interrupt,
> +    .cpu_exec_interrupt = x86_cpu_exec_interrupt,
>      .debug_excp_handler = breakpoint_handler,
>      .debug_check_breakpoint = x86_debug_check_breakpoint,
>  #endif /* !CONFIG_USER_ONLY */
> --
> 2.31.1
>
>

[-- Attachment #2: Type: text/html, Size: 5392 bytes --]