[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 12/14] xen/riscv: introduce an implementation of macros from <asm/bug.h>



On 30.01.23 12:35, Oleksii wrote:
Hi Julien,
On Fri, 2023-01-27 at 16:02 +0000, Julien Grall wrote:
Hi Oleksii,

On 27/01/2023 13:59, Oleksii Kurochko wrote:
The patch introduces macros: BUG(), WARN(), run_in_exception(),
assert_failed.

The implementation uses "ebreak" instruction in combination with
diffrent bug frame tables (for each type) which contains useful
information.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
---
Changes:
    - Remove __ in define namings
    - Update run_in_exception_handler() with
      register void *fn_ asm(__stringify(BUG_FN_REG)) = (fn);
    - Remove bug_instr_t type and change it's usage to uint32_t
---
   xen/arch/riscv/include/asm/bug.h | 118
++++++++++++++++++++++++++++
   xen/arch/riscv/traps.c           | 128
+++++++++++++++++++++++++++++++
   xen/arch/riscv/xen.lds.S         |  10 +++
   3 files changed, 256 insertions(+)
   create mode 100644 xen/arch/riscv/include/asm/bug.h

diff --git a/xen/arch/riscv/include/asm/bug.h
b/xen/arch/riscv/include/asm/bug.h
new file mode 100644
index 0000000000..4b15d8eba6
--- /dev/null
+++ b/xen/arch/riscv/include/asm/bug.h
@@ -0,0 +1,118 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2012 Regents of the University of California
+ * Copyright (C) 2021-2023 Vates

I have to question the two copyrights here given that the majority of
the code seems to be taken from the arm implementation (see
arch/arm/include/asm/bug.h).

With that said, we should consolidate the code rather than
duplicating
it on every architecture.

Copyrights should be removed. They were taken from the previous
implementation of bug.h for RISC-V so I just forgot to remove them.

It looks like we should have common bug.h for ARM and RISCV but I am
not sure that I know how to do that better.
Probably the code wants to be moved to xen/include/bug.h and using
ifdef ARM && RISCV ...
But still I am not sure that this is the best one option as at least we
have different implementation for x86_64.

There are already a lot of duplicated #defines in the Arm and x86 asm/bug.h
files.

I'd create xen/include/xen/bug.h including asm/bug.h first and then adding
all the common stuff.

In case 2 archs are sharing some #define FOO you could #define FOO in the
asm/bug.h for the arch not using the common definition and do #ifndef FOO
in xen/include/xen/bug.h around the variant shared by the other archs.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.