diff mbox series

[v1,04/14] xen/riscv: add <asm/csr.h> header

Message ID afc53b9bee58b5d386f105ee8f23a411d5a15bed.1674226563.git.oleksii.kurochko@gmail.com (mailing list archive)
State Superseded
Headers show
Series RISCV basic exception handling implementation | expand

Commit Message

Oleksii Kurochko Jan. 20, 2023, 2:59 p.m. UTC
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/arch/riscv/include/asm/csr.h | 82 ++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100644 xen/arch/riscv/include/asm/csr.h

Comments

Alistair Francis Jan. 22, 2023, 11:25 p.m. UTC | #1
On Sat, Jan 21, 2023 at 1:00 AM Oleksii Kurochko
<oleksii.kurochko@gmail.com> wrote:
>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  xen/arch/riscv/include/asm/csr.h | 82 ++++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
>  create mode 100644 xen/arch/riscv/include/asm/csr.h
>
> diff --git a/xen/arch/riscv/include/asm/csr.h b/xen/arch/riscv/include/asm/csr.h
> new file mode 100644
> index 0000000000..1a879c6c4d
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/csr.h
> @@ -0,0 +1,82 @@
> +/*
> + * Take from Linux.
> + *
> + * SPDX-License-Identifier: GPL-2.0-only
> + *
> + * Copyright (C) 2015 Regents of the University of California
> + */
> +
> +#ifndef _ASM_RISCV_CSR_H
> +#define _ASM_RISCV_CSR_H
> +
> +#include <asm/asm.h>
> +#include <xen/const.h>
> +#include <asm/riscv_encoding.h>
> +
> +#ifndef __ASSEMBLY__
> +
> +#define csr_read(csr)                                          \
> +({                                                             \
> +       register unsigned long __v;                             \
> +       __asm__ __volatile__ ("csrr %0, " __ASM_STR(csr)        \
> +                             : "=r" (__v) :                    \
> +                             : "memory");                      \
> +       __v;                                                    \
> +})
> +
> +#define csr_write(csr, val)                                    \
> +({                                                             \
> +       unsigned long __v = (unsigned long)(val);               \
> +       __asm__ __volatile__ ("csrw " __ASM_STR(csr) ", %0"     \
> +                             : : "rK" (__v)                    \
> +                             : "memory");                      \
> +})
> +
> +/*
> +#define csr_swap(csr, val)                                     \
> +({                                                             \
> +       unsigned long __v = (unsigned long)(val);               \
> +       __asm__ __volatile__ ("csrrw %0, " __ASM_STR(csr) ", %1"\
> +                             : "=r" (__v) : "rK" (__v)         \
> +                             : "memory");                      \
> +       __v;                                                    \
> +})
> +
> +#define csr_read_set(csr, val)                                 \
> +({                                                             \
> +       unsigned long __v = (unsigned long)(val);               \
> +       __asm__ __volatile__ ("csrrs %0, " __ASM_STR(csr) ", %1"\
> +                             : "=r" (__v) : "rK" (__v)         \
> +                             : "memory");                      \
> +       __v;                                                    \
> +})
> +
> +#define csr_set(csr, val)                                      \
> +({                                                             \
> +       unsigned long __v = (unsigned long)(val);               \
> +       __asm__ __volatile__ ("csrs " __ASM_STR(csr) ", %0"     \
> +                             : : "rK" (__v)                    \
> +                             : "memory");                      \
> +})
> +
> +#define csr_read_clear(csr, val)                               \
> +({                                                             \
> +       unsigned long __v = (unsigned long)(val);               \
> +       __asm__ __volatile__ ("csrrc %0, " __ASM_STR(csr) ", %1"\
> +                             : "=r" (__v) : "rK" (__v)         \
> +                             : "memory");                      \
> +       __v;                                                    \
> +})
> +
> +#define csr_clear(csr, val)                                    \
> +({                                                             \
> +       unsigned long __v = (unsigned long)(val);               \
> +       __asm__ __volatile__ ("csrc " __ASM_STR(csr) ", %0"     \
> +                             : : "rK" (__v)                    \
> +                             : "memory");                      \
> +})
> +*/
> +
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* _ASM_RISCV_CSR_H */
> --
> 2.39.0
>
>
Jan Beulich Jan. 23, 2023, 1:57 p.m. UTC | #2
On 20.01.2023 15:59, Oleksii Kurochko wrote:
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/csr.h
> @@ -0,0 +1,82 @@
> +/*
> + * Take from Linux.

This again means you want an Origin: tag. Whether the comment itself is
useful depends on how much customization you expect there to be down
the road. But wait - the header here is quite dissimilar from Linux'es,
so the description wants to go into further detail. That would then want
to include why 5 of the 7 functions are actually commented out at this
point.

Jan
Oleksii Kurochko Jan. 23, 2023, 2:23 p.m. UTC | #3
On Mon, 2023-01-23 at 14:57 +0100, Jan Beulich wrote:
> On 20.01.2023 15:59, Oleksii Kurochko wrote:
> > --- /dev/null
> > +++ b/xen/arch/riscv/include/asm/csr.h
> > @@ -0,0 +1,82 @@
> > +/*
> > + * Take from Linux.
> 
> This again means you want an Origin: tag. Whether the comment itself
> is
> useful depends on how much customization you expect there to be down
> the road. But wait - the header here is quite dissimilar from
> Linux'es,
> so the description wants to go into further detail. That would then
> want
> to include why 5 of the 7 functions are actually commented out at
> this
> point.
> 
I forgot two remove them. They were commented as they aren't used now.
But probably there is a sense to add them from the start.

I am curious if "Take from Linux" is needed at all?
Should it be described what was removed from the original header [1] ?

[1]
https://elixir.bootlin.com/linux/latest/source/arch/riscv/include/asm/csr.h
> Jan
Jan Beulich Jan. 23, 2023, 2:31 p.m. UTC | #4
On 23.01.2023 15:23, Oleksii wrote:
> On Mon, 2023-01-23 at 14:57 +0100, Jan Beulich wrote:
>> On 20.01.2023 15:59, Oleksii Kurochko wrote:
>>> --- /dev/null
>>> +++ b/xen/arch/riscv/include/asm/csr.h
>>> @@ -0,0 +1,82 @@
>>> +/*
>>> + * Take from Linux.
>>
>> This again means you want an Origin: tag. Whether the comment itself
>> is
>> useful depends on how much customization you expect there to be down
>> the road. But wait - the header here is quite dissimilar from
>> Linux'es,
>> so the description wants to go into further detail. That would then
>> want
>> to include why 5 of the 7 functions are actually commented out at
>> this
>> point.
>>
> I forgot two remove them. They were commented as they aren't used now.
> But probably there is a sense to add them from the start.
> 
> I am curious if "Take from Linux" is needed at all?

I said, I was wondering too. The fewer you take from Linux (and the more
you add on top), the less useful such a comment is going to be.

> Should it be described what was removed from the original header [1] ?

In the description, yes (or, if it's very little, simply say that much
more is present there). Doing so in the leading comment in the header
is risking to go stale very quickly.

Jan
diff mbox series

Patch

diff --git a/xen/arch/riscv/include/asm/csr.h b/xen/arch/riscv/include/asm/csr.h
new file mode 100644
index 0000000000..1a879c6c4d
--- /dev/null
+++ b/xen/arch/riscv/include/asm/csr.h
@@ -0,0 +1,82 @@ 
+/*
+ * Take from Linux.
+ *
+ * SPDX-License-Identifier: GPL-2.0-only
+ *
+ * Copyright (C) 2015 Regents of the University of California
+ */
+
+#ifndef _ASM_RISCV_CSR_H
+#define _ASM_RISCV_CSR_H
+
+#include <asm/asm.h>
+#include <xen/const.h>
+#include <asm/riscv_encoding.h>
+
+#ifndef __ASSEMBLY__
+
+#define csr_read(csr)						\
+({								\
+	register unsigned long __v;				\
+	__asm__ __volatile__ ("csrr %0, " __ASM_STR(csr)	\
+			      : "=r" (__v) :			\
+			      : "memory");			\
+	__v;							\
+})
+
+#define csr_write(csr, val)					\
+({								\
+	unsigned long __v = (unsigned long)(val);		\
+	__asm__ __volatile__ ("csrw " __ASM_STR(csr) ", %0"	\
+			      : : "rK" (__v)			\
+			      : "memory");			\
+})
+
+/*
+#define csr_swap(csr, val)					\
+({								\
+	unsigned long __v = (unsigned long)(val);		\
+	__asm__ __volatile__ ("csrrw %0, " __ASM_STR(csr) ", %1"\
+			      : "=r" (__v) : "rK" (__v)		\
+			      : "memory");			\
+	__v;							\
+})
+
+#define csr_read_set(csr, val)					\
+({								\
+	unsigned long __v = (unsigned long)(val);		\
+	__asm__ __volatile__ ("csrrs %0, " __ASM_STR(csr) ", %1"\
+			      : "=r" (__v) : "rK" (__v)		\
+			      : "memory");			\
+	__v;							\
+})
+
+#define csr_set(csr, val)					\
+({								\
+	unsigned long __v = (unsigned long)(val);		\
+	__asm__ __volatile__ ("csrs " __ASM_STR(csr) ", %0"	\
+			      : : "rK" (__v)			\
+			      : "memory");			\
+})
+
+#define csr_read_clear(csr, val)				\
+({								\
+	unsigned long __v = (unsigned long)(val);		\
+	__asm__ __volatile__ ("csrrc %0, " __ASM_STR(csr) ", %1"\
+			      : "=r" (__v) : "rK" (__v)		\
+			      : "memory");			\
+	__v;							\
+})
+
+#define csr_clear(csr, val)					\
+({								\
+	unsigned long __v = (unsigned long)(val);		\
+	__asm__ __volatile__ ("csrc " __ASM_STR(csr) ", %0"	\
+			      : : "rK" (__v)			\
+			      : "memory");			\
+})
+*/
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* _ASM_RISCV_CSR_H */