Re: [PATCH v2 2/4] build: use common stubs for debugger_trap_* functions if !CONFIG_CRASH_DEBUG

[Jan Beulich <jbeulich@suse.com>]

On 14.07.2021 22:37, Bobby Eshleman wrote:
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -16,6 +16,7 @@
>   * GNU General Public License for more details.
>   */
>  
> +#include <xen/debugger.h>

I don't think this is needed here; instead I think ...

> @@ -41,7 +42,6 @@
>  #include <asm/acpi.h>
>  #include <asm/cpuerrata.h>
>  #include <asm/cpufeature.h>
> -#include <asm/debugger.h>

... this wants to be done in patch 1 already.

> --- a/xen/common/keyhandler.c
> +++ b/xen/common/keyhandler.c
> @@ -3,6 +3,7 @@
>   */
>  
>  #include <asm/regs.h>
> +#include <xen/debugger.h>
>  #include <xen/delay.h>
>  #include <xen/keyhandler.h>
>  #include <xen/param.h>
> @@ -20,7 +21,6 @@
>  #include <xen/mm.h>
>  #include <xen/watchdog.h>
>  #include <xen/init.h>
> -#include <asm/debugger.h>
>  #include <asm/div64.h>

Not sure about this - as indicated maybe the code needing the include
instead wants to move to x86'es new debugger.c.

> --- /dev/null
> +++ b/xen/include/xen/debugger.h
> @@ -0,0 +1,69 @@
> +/******************************************************************************
> + * Generic hooks into arch-dependent Xen.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Each debugger should define three functions here:
> + *
> + * 1. debugger_trap_entry():
> + *  Called at start of any synchronous fault or trap, before any other work
> + *  is done. The idea is that if your debugger deliberately caused the trap
> + *  (e.g. to implement breakpoints or data watchpoints) then you can take
> + *  appropriate action and return a non-zero value to cause early exit from
> + *  the trap function.
> + *
> + * 2. debugger_trap_fatal():
> + *  Called when Xen is about to give up and crash. Typically you will use this
> + *  hook to drop into a debug session. It can also be used to hook off
> + *  deliberately caused traps (which you then handle and return non-zero).
> + *
> + * 3. debugger_trap_immediate():
> + *  Called if we want to drop into a debugger now.  This is essentially the
> + *  same as debugger_trap_fatal, except that we use the current register state
> + *  rather than the state which was in effect when we took the trap.
> + *  For example: if we're dying because of an unhandled exception, we call
> + *  debugger_trap_fatal; if we're dying because of a panic() we call
> + *  debugger_trap_immediate().
> + */
> +
> +#ifndef __XEN_DEBUGGER_H__
> +#define __XEN_DEBUGGER_H__
> +
> +#ifdef CONFIG_CRASH_DEBUG
> +
> +#include <asm/debugger.h>
> +
> +#else
> +
> +#include <asm/regs.h>
> +#include <asm/processor.h>

I don't think you need either of these for the stubs below.

> +static inline void domain_pause_for_debugger(void)
> +{
> +}
> +
> +static inline bool debugger_trap_fatal(
> +    unsigned int vector, const struct cpu_user_regs *regs)
> +{
> +    return false;
> +}
> +
> +static inline void debugger_trap_immediate(void)
> +{
> +}
> +
> +static inline bool debugger_trap_entry(
> +    unsigned int vector, const struct cpu_user_regs *regs)
> +{
> +    return false;
> +}

Of these stubs, after patch 1 only debugger_trap_immediate() is needed
outside of arch/x86/. Perhaps everything else wants to remain in x86'es
per-arch header?

Jan