Re: [Xen-devel] [PATCH 15/15] xen/arm: traps: Move the implementation of GUEST_BUG_ON in traps.h
On Mon, 20 Aug 2018, Julien Grall wrote: > Hi Stefano, > > On Thu, 16 Aug 2018, 19:17 Stefano Stabellini, wrote: > On Thu, 16 Aug 2018, Julien Grall wrote: > > Hi Stefano, > > > > On 08/14/2018 10:43 PM, Stefano Stabellini wrote: > > > On Mon, 16 Jul 2018, Julien Grall wrote: > > > > GUEST_BUG_ON may be used in other files doing guest emulation. > > > > > > > > Signed-off-by: Julien Grall > > > > > > Given that GUEST_BUG_ON is not actually used in any other files in > this > > > patch series, I assume you are referring to one of your future > series? > > > > It is going to be used later. However, I don't really refer to any > series in > > particular. It is just that this macros could be helpful in any > emulation code > > to catch what we think is a invalid architectural behavior. > > It is only that a concrete use-case helps. The idea of moving > GUEST_BUG_ON is good, but where to? For instance, wouldn't it be better > to move it to guest_access.h? I am just trying to point to an existing > header which is more widely used across the emulators (vpl011, > vgic-v3-its.c, hvm.c, ...) > > > Definitely not in guest_access.h or any wider in includes. If you look at the > implementation and documentation, it will crash the > hypervisor because the goal is to catch hardware bug or misunderstanding of > the spec. This is not meant to be used in emulation. > We should not crash the guest/hypervisor in bad behavior but inject an > exception. > > So trap.h is the best places for it as hardware trap are now split accros > multiples files. OK ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 15/15] xen/arm: traps: Move the implementation of GUEST_BUG_ON in traps.h
Hi Stefano, On Thu, 16 Aug 2018, 19:17 Stefano Stabellini, wrote: > On Thu, 16 Aug 2018, Julien Grall wrote: > > Hi Stefano, > > > > On 08/14/2018 10:43 PM, Stefano Stabellini wrote: > > > On Mon, 16 Jul 2018, Julien Grall wrote: > > > > GUEST_BUG_ON may be used in other files doing guest emulation. > > > > > > > > Signed-off-by: Julien Grall > > > > > > Given that GUEST_BUG_ON is not actually used in any other files in this > > > patch series, I assume you are referring to one of your future series? > > > > It is going to be used later. However, I don't really refer to any > series in > > particular. It is just that this macros could be helpful in any > emulation code > > to catch what we think is a invalid architectural behavior. > > It is only that a concrete use-case helps. The idea of moving > GUEST_BUG_ON is good, but where to? For instance, wouldn't it be better > to move it to guest_access.h? I am just trying to point to an existing > header which is more widely used across the emulators (vpl011, > vgic-v3-its.c, hvm.c, ...) > Definitely not in guest_access.h or any wider in includes. If you look at the implementation and documentation, it will crash the hypervisor because the goal is to catch hardware bug or misunderstanding of the spec. This is not meant to be used in emulation. We should not crash the guest/hypervisor in bad behavior but inject an exception. So trap.h is the best places for it as hardware trap are now split accros multiples files. Cheers, ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 15/15] xen/arm: traps: Move the implementation of GUEST_BUG_ON in traps.h
On Thu, 16 Aug 2018, Julien Grall wrote: > Hi Stefano, > > On 08/14/2018 10:43 PM, Stefano Stabellini wrote: > > On Mon, 16 Jul 2018, Julien Grall wrote: > > > GUEST_BUG_ON may be used in other files doing guest emulation. > > > > > > Signed-off-by: Julien Grall > > > > Given that GUEST_BUG_ON is not actually used in any other files in this > > patch series, I assume you are referring to one of your future series? > > It is going to be used later. However, I don't really refer to any series in > particular. It is just that this macros could be helpful in any emulation code > to catch what we think is a invalid architectural behavior. It is only that a concrete use-case helps. The idea of moving GUEST_BUG_ON is good, but where to? For instance, wouldn't it be better to move it to guest_access.h? I am just trying to point to an existing header which is more widely used across the emulators (vpl011, vgic-v3-its.c, hvm.c, ...) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 15/15] xen/arm: traps: Move the implementation of GUEST_BUG_ON in traps.h
Hi Stefano, On 08/14/2018 10:43 PM, Stefano Stabellini wrote: On Mon, 16 Jul 2018, Julien Grall wrote: GUEST_BUG_ON may be used in other files doing guest emulation. Signed-off-by: Julien Grall Given that GUEST_BUG_ON is not actually used in any other files in this patch series, I assume you are referring to one of your future series? It is going to be used later. However, I don't really refer to any series in particular. It is just that this macros could be helpful in any emulation code to catch what we think is a invalid architectural behavior. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 15/15] xen/arm: traps: Move the implementation of GUEST_BUG_ON in traps.h
On Mon, 16 Jul 2018, Julien Grall wrote:
> GUEST_BUG_ON may be used in other files doing guest emulation.
>
> Signed-off-by: Julien Grall
Given that GUEST_BUG_ON is not actually used in any other files in this
patch series, I assume you are referring to one of your future series?
> ---
> xen/arch/arm/traps.c| 24
> xen/include/asm-arm/traps.h | 24
> 2 files changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index d1bf69b245..6751e4d754 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -68,30 +68,6 @@ static inline void check_stack_alignment_constraints(void)
> {
> #endif
> }
>
> -/*
> - * GUEST_BUG_ON is intended for checking that the guest state has not been
> - * corrupted in hardware and/or that the hardware behaves as we
> - * believe it should (i.e. that certain traps can only occur when the
> - * guest is in a particular mode).
> - *
> - * The intention is to limit the damage such h/w bugs (or spec
> - * misunderstandings) can do by turning them into Denial of Service
> - * attacks instead of e.g. information leaks or privilege escalations.
> - *
> - * GUEST_BUG_ON *MUST* *NOT* be used to check for guest controllable state!
> - *
> - * Compared with regular BUG_ON it dumps the guest vcpu state instead
> - * of Xen's state.
> - */
> -#define guest_bug_on_failed(p) \
> -do {\
> -show_execution_state(guest_cpu_user_regs());\
> -panic("Guest Bug: %pv: '%s', line %d, file %s\n", \
> - current, p, __LINE__, __FILE__); \
> -} while (0)
> -#define GUEST_BUG_ON(p) \
> -do { if ( unlikely(p) ) guest_bug_on_failed(#p); } while (0)
> -
> #ifdef CONFIG_ARM_32
> static int debug_stack_lines = 20;
> #define stack_words_per_line 8
> diff --git a/xen/include/asm-arm/traps.h b/xen/include/asm-arm/traps.h
> index 70b52d1d16..0acf7de67d 100644
> --- a/xen/include/asm-arm/traps.h
> +++ b/xen/include/asm-arm/traps.h
> @@ -9,6 +9,30 @@
> # include
> #endif
>
> +/*
> + * GUEST_BUG_ON is intended for checking that the guest state has not been
> + * corrupted in hardware and/or that the hardware behaves as we
> + * believe it should (i.e. that certain traps can only occur when the
> + * guest is in a particular mode).
> + *
> + * The intention is to limit the damage such h/w bugs (or spec
> + * misunderstandings) can do by turning them into Denial of Service
> + * attacks instead of e.g. information leaks or privilege escalations.
> + *
> + * GUEST_BUG_ON *MUST* *NOT* be used to check for guest controllable state!
> + *
> + * Compared with regular BUG_ON it dumps the guest vcpu state instead
> + * of Xen's state.
> + */
> +#define guest_bug_on_failed(p) \
> +do {\
> +show_execution_state(guest_cpu_user_regs());\
> +panic("Guest Bug: %pv: '%s', line %d, file %s\n", \
> + current, p, __LINE__, __FILE__); \
> +} while (0)
> +#define GUEST_BUG_ON(p) \
> +do { if ( unlikely(p) ) guest_bug_on_failed(#p); } while (0)
> +
> int check_conditional_instr(struct cpu_user_regs *regs, const union hsr hsr);
>
> void advance_pc(struct cpu_user_regs *regs, const union hsr hsr);
> --
> 2.11.0
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 15/15] xen/arm: traps: Move the implementation of GUEST_BUG_ON in traps.h
GUEST_BUG_ON may be used in other files doing guest emulation.
Signed-off-by: Julien Grall
---
xen/arch/arm/traps.c| 24
xen/include/asm-arm/traps.h | 24
2 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index d1bf69b245..6751e4d754 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -68,30 +68,6 @@ static inline void check_stack_alignment_constraints(void) {
#endif
}
-/*
- * GUEST_BUG_ON is intended for checking that the guest state has not been
- * corrupted in hardware and/or that the hardware behaves as we
- * believe it should (i.e. that certain traps can only occur when the
- * guest is in a particular mode).
- *
- * The intention is to limit the damage such h/w bugs (or spec
- * misunderstandings) can do by turning them into Denial of Service
- * attacks instead of e.g. information leaks or privilege escalations.
- *
- * GUEST_BUG_ON *MUST* *NOT* be used to check for guest controllable state!
- *
- * Compared with regular BUG_ON it dumps the guest vcpu state instead
- * of Xen's state.
- */
-#define guest_bug_on_failed(p) \
-do {\
-show_execution_state(guest_cpu_user_regs());\
-panic("Guest Bug: %pv: '%s', line %d, file %s\n", \
- current, p, __LINE__, __FILE__); \
-} while (0)
-#define GUEST_BUG_ON(p) \
-do { if ( unlikely(p) ) guest_bug_on_failed(#p); } while (0)
-
#ifdef CONFIG_ARM_32
static int debug_stack_lines = 20;
#define stack_words_per_line 8
diff --git a/xen/include/asm-arm/traps.h b/xen/include/asm-arm/traps.h
index 70b52d1d16..0acf7de67d 100644
--- a/xen/include/asm-arm/traps.h
+++ b/xen/include/asm-arm/traps.h
@@ -9,6 +9,30 @@
# include
#endif
+/*
+ * GUEST_BUG_ON is intended for checking that the guest state has not been
+ * corrupted in hardware and/or that the hardware behaves as we
+ * believe it should (i.e. that certain traps can only occur when the
+ * guest is in a particular mode).
+ *
+ * The intention is to limit the damage such h/w bugs (or spec
+ * misunderstandings) can do by turning them into Denial of Service
+ * attacks instead of e.g. information leaks or privilege escalations.
+ *
+ * GUEST_BUG_ON *MUST* *NOT* be used to check for guest controllable state!
+ *
+ * Compared with regular BUG_ON it dumps the guest vcpu state instead
+ * of Xen's state.
+ */
+#define guest_bug_on_failed(p) \
+do {\
+show_execution_state(guest_cpu_user_regs());\
+panic("Guest Bug: %pv: '%s', line %d, file %s\n", \
+ current, p, __LINE__, __FILE__); \
+} while (0)
+#define GUEST_BUG_ON(p) \
+do { if ( unlikely(p) ) guest_bug_on_failed(#p); } while (0)
+
int check_conditional_instr(struct cpu_user_regs *regs, const union hsr hsr);
void advance_pc(struct cpu_user_regs *regs, const union hsr hsr);
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
