[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver
From: |
Schwarz, Konrad |
Subject: |
RE: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver |
Date: |
Wed, 5 Jan 2022 13:25:51 +0000 |
Hi,
> -----Original Message-----
> From: Alistair Francis <alistair23@gmail.com>
> Sent: Tuesday, January 4, 2022 23:12
> To: Schwarz, Konrad (T CED SES-DE) <konrad.schwarz@siemens.com>
> Subject: Re: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver
>
> On Wed, Jan 5, 2022 at 1:56 AM Konrad Schwarz
> <konrad.schwarz@siemens.com> wrote:
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 9f41954894..557b4afe0e 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -3,6 +3,7 @@
> > *
> > * Copyright (c) 2016-2017 Sagar Karandikar, sagark@eecs.berkeley.edu
> > * Copyright (c) 2017-2018 SiFive, Inc.
> > + * Copyright (c) 2021 Siemens AG, konrad.schwarz@siemens.com
>
> Please don't add these to existing files. In this case you have just
> added a newline to this file
Sorry, I don't know how that slipped through.
I originally didn't have any copyright messages, to which my company objected,
and I guess I overcompensated.
> diff --git a/target/riscv/csr32-op-gdbserver.h
> b/target/riscv/csr32-op-gdbserver.h
> > new file mode 100644
> > index 0000000000..e8ec527f23
> > --- /dev/null
> > +++ b/target/riscv/csr32-op-gdbserver.h
> > @@ -0,0 +1,109 @@
> > +/* Copyright (c) 2021 Siemens AG, konrad.schwarz@siemens.com */
>
> All of these files should have the usual file boiler plate
OK, I'll try to figure out something.
> > +#if !defined(CONFIG_USER_ONLY)
> > +# ifdef TARGET_RISCV64
> > +# include "csr64-op-gdbserver.h"
> > +# elif defined TARGET_RISCV64
> > +# include "csr32-op-gdbserver.h"
>
> This doesn't look right. `if defined TARGET_RISCV64` -> `include
> "csr32-op-gdbserver.h"`?
You are quite right.
> Also this should be dynamic instead of based on the build time CPU, as
> the user could use a 32-bit CPU on a 64-bit target build.
I'm not sure. The machine itself is still a 64-bit machine; its CSRs are 64
bits long.
`csr.c', which implements the CSRs contains an instance of # ifdef
TARGET_RISCV64,
so that part of the implementation is not dynamic either.
Also, someone who is developing 32-bit system software can easily
sidestep the issue by using the 32-bit target.
Regards
Konrad
- [PATCH v1 5/5] RISC-V: Add `v' (virtualization mode) bit to the `priv' virtual debug register, (continued)
- [PATCH v1 5/5] RISC-V: Add `v' (virtualization mode) bit to the `priv' virtual debug register, Konrad Schwarz, 2022/01/02
- [PATCH v1 4/5] RISC-V: Typed CSRs in gdbserver, Konrad Schwarz, 2022/01/02
- Re: [PATCH v1 4/5] RISC-V: Typed CSRs in gdbserver, Ralf Ramsauer, 2022/01/03
- [PATCH v2 0/5] Improve RISC-V debugging support., Konrad Schwarz, 2022/01/04
- [PATCH v2 2/5] RISC-V: monitor's print register functionality, Konrad Schwarz, 2022/01/04
- [PATCH v2 3/5] RISC-V: 'info gmem' to show hypervisor guest -> physical address translations, Konrad Schwarz, 2022/01/04
- Re: [PATCH v2 3/5] RISC-V: 'info gmem' to show hypervisor guest -> physical address translations, Alistair Francis, 2022/01/04
- RE: [PATCH v2 3/5] RISC-V: 'info gmem' to show hypervisor guest -> physical address translations, Schwarz, Konrad, 2022/01/05
- [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver, Konrad Schwarz, 2022/01/04
- Re: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver, Alistair Francis, 2022/01/04
- RE: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver,
Schwarz, Konrad <=
- Re: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver, Richard Henderson, 2022/01/04
- RE: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver, Schwarz, Konrad, 2022/01/05
- Re: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver, Richard Henderson, 2022/01/05
- Re: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver, Alex Bennée, 2022/01/05
- RE: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver, Schwarz, Konrad, 2022/01/05
- Re: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver, Alex Bennée, 2022/01/05
- [PATCH v2 5/5] RISC-V: Add `v' (virtualization mode) bit to the `priv' virtual debug register, Konrad Schwarz, 2022/01/04
- [PATCH v2 1/5] RISC-V: larger and more consistent register set for 'info registers', Konrad Schwarz, 2022/01/04
- Re: [PATCH v2 1/5] RISC-V: larger and more consistent register set for 'info registers', Richard Henderson, 2022/01/04
- RE: [PATCH v2 1/5] RISC-V: larger and more consistent register set for 'info registers', Schwarz, Konrad, 2022/01/05