Hi, > -----Original Message----- > From: Alistair Francis <alistai...@gmail.com> > Sent: Tuesday, January 4, 2022 23:12 > To: Schwarz, Konrad (T CED SES-DE) <konrad.schw...@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.schw...@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, sag...@eecs.berkeley.edu > > * Copyright (c) 2017-2018 SiFive, Inc. > > + * Copyright (c) 2021 Siemens AG, konrad.schw...@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.schw...@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
