[v2] riscv-to-apply queue

[PULL v2 03/32] riscv: Generalize CPU init routine for the base CPU

Posted by Alistair Francis 4 years, 10 months ago

From: Bin Meng <bin.meng@windriver.com>

There is no need to have two functions that have exactly the same
codes for 32-bit and 64-bit base CPUs.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 1591837729-27486-1-git-send-email-bmeng.cn@gmail.com
Message-Id: <1591837729-27486-1-git-send-email-bmeng.cn@gmail.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 3a6d202d03..81cdea8680 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -126,9 +126,7 @@ static void riscv_any_cpu_init(Object *obj)
     set_resetvec(env, DEFAULT_RSTVEC);
 }
 
-#if defined(TARGET_RISCV32)
-
-static void riscv_base32_cpu_init(Object *obj)
+static void riscv_base_cpu_init(Object *obj)
 {
     CPURISCVState *env = &RISCV_CPU(obj)->env;
     /* We set this in the realise function */
@@ -136,6 +134,8 @@ static void riscv_base32_cpu_init(Object *obj)
     set_resetvec(env, DEFAULT_RSTVEC);
 }
 
+#if defined(TARGET_RISCV32)
+
 static void rv32gcsu_priv1_10_0_cpu_init(Object *obj)
 {
     CPURISCVState *env = &RISCV_CPU(obj)->env;
@@ -173,14 +173,6 @@ static void rv32imafcu_nommu_cpu_init(Object *obj)
 
 #elif defined(TARGET_RISCV64)
 
-static void riscv_base64_cpu_init(Object *obj)
-{
-    CPURISCVState *env = &RISCV_CPU(obj)->env;
-    /* We set this in the realise function */
-    set_misa(env, 0);
-    set_resetvec(env, DEFAULT_RSTVEC);
-}
-
 static void rv64gcsu_priv1_10_0_cpu_init(Object *obj)
 {
     CPURISCVState *env = &RISCV_CPU(obj)->env;
@@ -603,13 +595,13 @@ static const TypeInfo riscv_cpu_type_infos[] = {
     },
     DEFINE_CPU(TYPE_RISCV_CPU_ANY,              riscv_any_cpu_init),
 #if defined(TARGET_RISCV32)
-    DEFINE_CPU(TYPE_RISCV_CPU_BASE32,           riscv_base32_cpu_init),
+    DEFINE_CPU(TYPE_RISCV_CPU_BASE32,           riscv_base_cpu_init),
     DEFINE_CPU(TYPE_RISCV_CPU_IBEX,             rv32imcu_nommu_cpu_init),
     DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E31,       rv32imacu_nommu_cpu_init),
     DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E34,       rv32imafcu_nommu_cpu_init),
     DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34,       rv32gcsu_priv1_10_0_cpu_init),
 #elif defined(TARGET_RISCV64)
-    DEFINE_CPU(TYPE_RISCV_CPU_BASE64,           riscv_base64_cpu_init),
+    DEFINE_CPU(TYPE_RISCV_CPU_BASE64,           riscv_base_cpu_init),
     DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51,       rv64imacu_nommu_cpu_init),
     DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54,       rv64gcsu_priv1_10_0_cpu_init),
 #endif
-- 
2.27.0

Re: [PULL v2 03/32] riscv: Generalize CPU init routine for the base CPU

Posted by Bin Meng 4 years, 10 months ago

Hi Alistair,

On Sat, Jun 20, 2020 at 1:09 AM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> There is no need to have two functions that have exactly the same
> codes for 32-bit and 64-bit base CPUs.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Message-id: 1591837729-27486-1-git-send-email-bmeng.cn@gmail.com
> Message-Id: <1591837729-27486-1-git-send-email-bmeng.cn@gmail.com>

I noticed that patches from other people than you have the
"Message-id" tags, but your patch [1] does not. Is this intentional?

(not sure why we need 2 "Message-id" tags here, with one has <> ?)

Just want to know what's the best practice here.

> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
>

[1] https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg06208.html

Regards,
Bin

Re: [PULL v2 03/32] riscv: Generalize CPU init routine for the base CPU

Posted by Markus Armbruster 4 years, 10 months ago

Bin Meng <bmeng.cn@gmail.com> writes:

> Hi Alistair,
>
> On Sat, Jun 20, 2020 at 1:09 AM Alistair Francis
> <alistair.francis@wdc.com> wrote:
>>
>> From: Bin Meng <bin.meng@windriver.com>
>>
>> There is no need to have two functions that have exactly the same
>> codes for 32-bit and 64-bit base CPUs.
>>
>> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>> Message-id: 1591837729-27486-1-git-send-email-bmeng.cn@gmail.com
>> Message-Id: <1591837729-27486-1-git-send-email-bmeng.cn@gmail.com>
>
> I noticed that patches from other people than you have the
> "Message-id" tags, but your patch [1] does not. Is this intentional?
>
> (not sure why we need 2 "Message-id" tags here, with one has <> ?)

We don't.  Looks like an accident.

> Just want to know what's the best practice here.

The Message-Id tag's purpose is connecting commits back to the mailing
list.  Useful when you want to look up their review later.

To get them into git, maintainers should use git-am -m to apply
patches.  I have

    [am]
            messageid = true

in my .gitconfig.

Maintainers may be tempted to use git-rebase or git-cherry-pick instead
for patches they already have in their local git (such as their own
patches).  No good, because we don't get the Message-Id that way.

Patch submissions (as opposed to pull requests) generally do not have
Message-Id tags in commit messages.

Hope this helps!

>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>> ---
>>  target/riscv/cpu.c | 18 +++++-------------
>>  1 file changed, 5 insertions(+), 13 deletions(-)
>>
>
> [1] https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg06208.html
>
> Regards,
> Bin

Re: [PULL v2 03/32] riscv: Generalize CPU init routine for the base CPU

Posted by Alistair Francis 4 years, 10 months ago

On Tue, Jun 23, 2020 at 2:08 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> Bin Meng <bmeng.cn@gmail.com> writes:
>
> > Hi Alistair,
> >
> > On Sat, Jun 20, 2020 at 1:09 AM Alistair Francis
> > <alistair.francis@wdc.com> wrote:
> >>
> >> From: Bin Meng <bin.meng@windriver.com>
> >>
> >> There is no need to have two functions that have exactly the same
> >> codes for 32-bit and 64-bit base CPUs.
> >>
> >> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> >> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> >> Message-id: 1591837729-27486-1-git-send-email-bmeng.cn@gmail.com
> >> Message-Id: <1591837729-27486-1-git-send-email-bmeng.cn@gmail.com>
> >
> > I noticed that patches from other people than you have the
> > "Message-id" tags, but your patch [1] does not. Is this intentional?
> >
> > (not sure why we need 2 "Message-id" tags here, with one has <> ?)
>
> We don't.  Looks like an accident.

Yeah, that must have been an accident.

>
> > Just want to know what's the best practice here.
>
> The Message-Id tag's purpose is connecting commits back to the mailing
> list.  Useful when you want to look up their review later.
>
> To get them into git, maintainers should use git-am -m to apply
> patches.  I have
>
>     [am]
>             messageid = true
>
> in my .gitconfig.
>
> Maintainers may be tempted to use git-rebase or git-cherry-pick instead
> for patches they already have in their local git (such as their own
> patches).  No good, because we don't get the Message-Id that way.

Ah, thanks for clarifying that. I was never sure. I'll be sure to not
rebase or cherry-pick my own patches in the future.

Alistair

>
> Patch submissions (as opposed to pull requests) generally do not have
> Message-Id tags in commit messages.
>
> Hope this helps!
>
> >> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> >> ---
> >>  target/riscv/cpu.c | 18 +++++-------------
> >>  1 file changed, 5 insertions(+), 13 deletions(-)
> >>
> >
> > [1] https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg06208.html
> >
> > Regards,
> > Bin
>