[PATCH v2 0/3] Add support for Fujitsu A64FX processor

[PATCH v2 0/3] Add support for Fujitsu A64FX processor

[Shuuichirou Ishii]

This is the v2 patch series.

v2:
No features have been added or removed from the v1 patch series. Removal
of unused definitions that were added in excess, and consolidation of
patches for the purpose of functional consistency.

For patch 1, ARM_FEATURE_A64FX is not used in the v1 patch series, so it
was deleted this time, and will be added again when it is used.

For patch 2, since the a64fx_cp_reginfo structure is not used in the v1
patch series, I deleted the empty definition and added the TODO in the
aarch64_a64fx_initfn function. Also fixed the appearance, and cleaned up
and removed some things for patch consistency.

For patch 3, a64fx was added to docs/system/arm/virt.rst and
hw/arm/virt.c respectively, as a modification to the patch consistency
cleanup done in patch 2.

Shuuichirou Ishii (3):
  target-arm: delete ARM_FEATURE_A64FX
  target-arm: cpu64: Add support for Fujitsu A64FX
  target-arm: Add A64FX processor support to virt machine

 hw/arm/virt.c      |  2 +-
 target/arm/cpu.h   |  1 -
 target/arm/cpu64.c | 10 +++-------
 3 files changed, 4 insertions(+), 9 deletions(-)

-- 
2.27.0

[PATCH v2 1/3] target-arm: delete ARM_FEATURE_A64FX

[Shuuichirou Ishii]

The ARM_FEATURE_A64FX property was added,
but there is no function that uses this property yet,
so it will be removed until a function that uses it is added.

Signed-off-by: Shuuichirou Ishii <ishii.shuuichir@fujitsu.com>
---
 target/arm/cpu.h   | 1 -
 target/arm/cpu64.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 1b0c7b91ec..9f0a5f84d5 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2145,7 +2145,6 @@ enum arm_features {
     ARM_FEATURE_M_SECURITY, /* M profile Security Extension */
     ARM_FEATURE_M_MAIN, /* M profile Main Extension */
     ARM_FEATURE_V8_1M, /* M profile extras only in v8.1M and later */
-    ARM_FEATURE_A64FX, /* Fujitsu A64FX processor HPC extensions support */
 };
 
 static inline int arm_feature(CPUARMState *env, int feature)
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index a15f9c0c55..dd72300e88 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -856,7 +856,6 @@ static void aarch64_a64fx_initfn(Object *obj)
     ARMCPU *cpu = ARM_CPU(obj);
 
     cpu->dtb_compatible = "arm,a64fx";
-    set_feature(&cpu->env, ARM_FEATURE_A64FX);
     set_feature(&cpu->env, ARM_FEATURE_V8);
     set_feature(&cpu->env, ARM_FEATURE_NEON);
     set_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER);
-- 
2.27.0

Re: [PATCH v2 1/3] target-arm: delete ARM_FEATURE_A64FX

[Peter Maydell]

On Fri, 30 Jul 2021 at 04:08, Shuuichirou Ishii
<ishii.shuuichir@fujitsu.com> wrote:
>
> The ARM_FEATURE_A64FX property was added,
> but there is no function that uses this property yet,
> so it will be removed until a function that uses it is added.
>
> Signed-off-by: Shuuichirou Ishii <ishii.shuuichir@fujitsu.com>
> ---
>  target/arm/cpu.h   | 1 -
>  target/arm/cpu64.c | 1 -
>  2 files changed, 2 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 1b0c7b91ec..9f0a5f84d5 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -2145,7 +2145,6 @@ enum arm_features {
>      ARM_FEATURE_M_SECURITY, /* M profile Security Extension */
>      ARM_FEATURE_M_MAIN, /* M profile Main Extension */
>      ARM_FEATURE_V8_1M, /* M profile extras only in v8.1M and later */
> -    ARM_FEATURE_A64FX, /* Fujitsu A64FX processor HPC extensions support */
>  };

This feature doesn't exist in upstream QEMU, so this won't apply.

For a v2 of a patch, the patches should be based on upstream, not
on top of the v1 series.

thanks
-- PMM

RE: [PATCH v2 1/3] target-arm: delete ARM_FEATURE_A64FX

[ishii.shuuichir]

> This feature doesn't exist in upstream QEMU, so this won't apply.
> For a v2 of a patch, the patches should be based on upstream, not on top of the v1
> series.

Thank you for your comment.
I understood your point.

Best regards.
> -----Original Message-----
> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Friday, July 30, 2021 7:35 PM
> To: Ishii, Shuuichirou/石井 周一郎 <ishii.shuuichir@fujitsu.com>
> Cc: qemu-arm <qemu-arm@nongnu.org>; QEMU Developers
> <qemu-devel@nongnu.org>
> Subject: Re: [PATCH v2 1/3] target-arm: delete ARM_FEATURE_A64FX
> 
> On Fri, 30 Jul 2021 at 04:08, Shuuichirou Ishii <ishii.shuuichir@fujitsu.com>
> wrote:
> >
> > The ARM_FEATURE_A64FX property was added, but there is no function
> > that uses this property yet, so it will be removed until a function
> > that uses it is added.
> >
> > Signed-off-by: Shuuichirou Ishii <ishii.shuuichir@fujitsu.com>
> > ---
> >  target/arm/cpu.h   | 1 -
> >  target/arm/cpu64.c | 1 -
> >  2 files changed, 2 deletions(-)
> >
> > diff --git a/target/arm/cpu.h b/target/arm/cpu.h index
> > 1b0c7b91ec..9f0a5f84d5 100644
> > --- a/target/arm/cpu.h
> > +++ b/target/arm/cpu.h
> > @@ -2145,7 +2145,6 @@ enum arm_features {
> >      ARM_FEATURE_M_SECURITY, /* M profile Security Extension */
> >      ARM_FEATURE_M_MAIN, /* M profile Main Extension */
> >      ARM_FEATURE_V8_1M, /* M profile extras only in v8.1M and later */
> > -    ARM_FEATURE_A64FX, /* Fujitsu A64FX processor HPC extensions
> support */
> >  };
> 
> This feature doesn't exist in upstream QEMU, so this won't apply.
> 
> For a v2 of a patch, the patches should be based on upstream, not on top of the v1
> series.
> 
> thanks
> -- PMM

Re: [PATCH v2 1/3] target-arm: delete ARM_FEATURE_A64FX

[Alex Bennée]

Shuuichirou Ishii <ishii.shuuichir@fujitsu.com> writes:

> The ARM_FEATURE_A64FX property was added,
> but there is no function that uses this property yet,
> so it will be removed until a function that uses it is added.
>
> Signed-off-by: Shuuichirou Ishii <ishii.shuuichir@fujitsu.com>
> ---
>  target/arm/cpu.h   | 1 -
>  target/arm/cpu64.c | 1 -
>  2 files changed, 2 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 1b0c7b91ec..9f0a5f84d5 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -2145,7 +2145,6 @@ enum arm_features {
>      ARM_FEATURE_M_SECURITY, /* M profile Security Extension */
>      ARM_FEATURE_M_MAIN, /* M profile Main Extension */
>      ARM_FEATURE_V8_1M, /* M profile extras only in v8.1M and later */
> -    ARM_FEATURE_A64FX, /* Fujitsu A64FX processor HPC extensions
> support */

This is confusing because I can't see this feature flag in the mainline
branch. Have you inadvertently based this series from an internal branch? 

>  };
>  
>  static inline int arm_feature(CPUARMState *env, int feature)
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index a15f9c0c55..dd72300e88 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -856,7 +856,6 @@ static void aarch64_a64fx_initfn(Object *obj)
>      ARMCPU *cpu = ARM_CPU(obj);
>  
>      cpu->dtb_compatible = "arm,a64fx";
> -    set_feature(&cpu->env, ARM_FEATURE_A64FX);
>      set_feature(&cpu->env, ARM_FEATURE_V8);
>      set_feature(&cpu->env, ARM_FEATURE_NEON);
>      set_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER);


-- 
Alex Bennée

RE: [PATCH v2 1/3] target-arm: delete ARM_FEATURE_A64FX

[ishii.shuuichir]

> This is confusing because I can't see this feature flag in the mainline branch. Have
> you inadvertently based this series from an internal branch?

I'm sorry for the confusion.

My lack of understanding of how to handle v2 patches has led me to create a v2 patch series
based on patches that have not been merged into upstream.
I will repost this so that the implementation can be closed in one patch series.

Best regards.
> -----Original Message-----
> From: Alex Bennée <alex.bennee@linaro.org>
> Sent: Friday, July 30, 2021 7:36 PM
> To: Ishii, Shuuichirou/石井 周一郎 <ishii.shuuichir@fujitsu.com>
> Cc: peter.maydell@linaro.org; qemu-devel@nongnu.org; qemu-arm@nongnu.org
> Subject: Re: [PATCH v2 1/3] target-arm: delete ARM_FEATURE_A64FX
> 
> 
> Shuuichirou Ishii <ishii.shuuichir@fujitsu.com> writes:
> 
> > The ARM_FEATURE_A64FX property was added, but there is no function
> > that uses this property yet, so it will be removed until a function
> > that uses it is added.
> >
> > Signed-off-by: Shuuichirou Ishii <ishii.shuuichir@fujitsu.com>
> > ---
> >  target/arm/cpu.h   | 1 -
> >  target/arm/cpu64.c | 1 -
> >  2 files changed, 2 deletions(-)
> >
> > diff --git a/target/arm/cpu.h b/target/arm/cpu.h index
> > 1b0c7b91ec..9f0a5f84d5 100644
> > --- a/target/arm/cpu.h
> > +++ b/target/arm/cpu.h
> > @@ -2145,7 +2145,6 @@ enum arm_features {
> >      ARM_FEATURE_M_SECURITY, /* M profile Security Extension */
> >      ARM_FEATURE_M_MAIN, /* M profile Main Extension */
> >      ARM_FEATURE_V8_1M, /* M profile extras only in v8.1M and later */
> > -    ARM_FEATURE_A64FX, /* Fujitsu A64FX processor HPC extensions
> > support */
> 
> This is confusing because I can't see this feature flag in the mainline branch. Have
> you inadvertently based this series from an internal branch?
> 
> >  };
> >
> >  static inline int arm_feature(CPUARMState *env, int feature) diff
> > --git a/target/arm/cpu64.c b/target/arm/cpu64.c index
> > a15f9c0c55..dd72300e88 100644
> > --- a/target/arm/cpu64.c
> > +++ b/target/arm/cpu64.c
> > @@ -856,7 +856,6 @@ static void aarch64_a64fx_initfn(Object *obj)
> >      ARMCPU *cpu = ARM_CPU(obj);
> >
> >      cpu->dtb_compatible = "arm,a64fx";
> > -    set_feature(&cpu->env, ARM_FEATURE_A64FX);
> >      set_feature(&cpu->env, ARM_FEATURE_V8);
> >      set_feature(&cpu->env, ARM_FEATURE_NEON);
> >      set_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER);
> 
> 
> --
> Alex Bennée

[PATCH v2 2/3] target-arm: cpu64: Add support for Fujitsu A64FX

[Shuuichirou Ishii]

Remove unused definitions, change of appearance and fix for patch consistency
https://lists.gnu.org/archive/html/qemu-devel/2021-07/msg04789.html
https://lists.gnu.org/archive/html/qemu-devel/2021-07/msg04790.html

Signed-off-by: Shuuichirou Ishii <ishii.shuuichir@fujitsu.com>
---
 docs/system/arm/virt.rst | 1 -
 hw/arm/virt.c            | 1 -
 target/arm/cpu64.c       | 9 +++------
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
index 5329e952cf..27652adfae 100644
--- a/docs/system/arm/virt.rst
+++ b/docs/system/arm/virt.rst
@@ -55,7 +55,6 @@ Supported guest CPU types:
 - ``cortex-a53`` (64-bit)
 - ``cortex-a57`` (64-bit)
 - ``cortex-a72`` (64-bit)
-- ``a64fx`` (64-bit)
 - ``host`` (with KVM only)
 - ``max`` (same as ``host`` for KVM; best possible emulation with TCG)
 
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 3b8a26a420..81eda46b0b 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -202,7 +202,6 @@ static const char *valid_cpus[] = {
     ARM_CPU_TYPE_NAME("cortex-a72"),
     ARM_CPU_TYPE_NAME("host"),
     ARM_CPU_TYPE_NAME("max"),
-    ARM_CPU_TYPE_NAME("a64fx"),
 };
 
 static bool cpu_type_valid(const char *cpu)
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index dd72300e88..2b66e30133 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -847,10 +847,6 @@ static void aarch64_max_initfn(Object *obj)
                         cpu_max_set_sve_max_vq, NULL, NULL);
 }
 
-static const ARMCPRegInfo a64fx_cp_reginfo[] = {
-    /* TODO  Add A64FX specific HPC extensinos registers */
-    REGINFO_SENTINEL
-};
 static void aarch64_a64fx_initfn(Object *obj)
 {
     ARMCPU *cpu = ARM_CPU(obj);
@@ -887,19 +883,20 @@ static void aarch64_a64fx_initfn(Object *obj)
     cpu->gic_num_lrs = 4;
     cpu->gic_vpribits = 5;
     cpu->gic_vprebits = 5;
-    define_arm_cp_regs(cpu, a64fx_cp_reginfo);
+    /* TODO:  Add A64FX specific HPC extension registers */
 
     aarch64_add_sve_properties(obj);
     object_property_add(obj, "sve-max-vq", "uint32", cpu_max_get_sve_max_vq,
                         cpu_max_set_sve_max_vq, NULL, NULL);
+
 }
 
 static const ARMCPUInfo aarch64_cpus[] = {
     { .name = "cortex-a57",         .initfn = aarch64_a57_initfn },
     { .name = "cortex-a53",         .initfn = aarch64_a53_initfn },
     { .name = "cortex-a72",         .initfn = aarch64_a72_initfn },
-    { .name = "max",                .initfn = aarch64_max_initfn },
     { .name = "a64fx",              .initfn = aarch64_a64fx_initfn },
+    { .name = "max",                .initfn = aarch64_max_initfn },
 };
 
 static bool aarch64_cpu_get_aarch64(Object *obj, Error **errp)
-- 
2.27.0

[PATCH v2 3/3] target-arm: Add A64FX processor support to virt machine

[Shuuichirou Ishii]

Fix for patch consistency.
https://lists.gnu.org/archive/html/qemu-devel/2021-07/msg06993.html

Signed-off-by: Shuuichirou Ishii <ishii.shuuichir@fujitsu.com>
---
 docs/system/arm/virt.rst | 1 +
 hw/arm/virt.c            | 1 +
 2 files changed, 2 insertions(+)

diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
index 27652adfae..5329e952cf 100644
--- a/docs/system/arm/virt.rst
+++ b/docs/system/arm/virt.rst
@@ -55,6 +55,7 @@ Supported guest CPU types:
 - ``cortex-a53`` (64-bit)
 - ``cortex-a57`` (64-bit)
 - ``cortex-a72`` (64-bit)
+- ``a64fx`` (64-bit)
 - ``host`` (with KVM only)
 - ``max`` (same as ``host`` for KVM; best possible emulation with TCG)
 
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 81eda46b0b..10286d3fd6 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -200,6 +200,7 @@ static const char *valid_cpus[] = {
     ARM_CPU_TYPE_NAME("cortex-a53"),
     ARM_CPU_TYPE_NAME("cortex-a57"),
     ARM_CPU_TYPE_NAME("cortex-a72"),
+    ARM_CPU_TYPE_NAME("a64fx"),
     ARM_CPU_TYPE_NAME("host"),
     ARM_CPU_TYPE_NAME("max"),
 };
-- 
2.27.0

Re: [PATCH v2 3/3] target-arm: Add A64FX processor support to virt machine

[Peter Maydell]

On Fri, 30 Jul 2021 at 04:08, Shuuichirou Ishii
<ishii.shuuichir@fujitsu.com> wrote:
>
> Fix for patch consistency.
> https://lists.gnu.org/archive/html/qemu-devel/2021-07/msg06993.html

Commit messages should describe what the patch is doing and why,
so the reader can understand it without having to cross-reference
old mailing list threads.

> Signed-off-by: Shuuichirou Ishii <ishii.shuuichir@fujitsu.com>
> ---
>  docs/system/arm/virt.rst | 1 +
>  hw/arm/virt.c            | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
> index 27652adfae..5329e952cf 100644
> --- a/docs/system/arm/virt.rst
> +++ b/docs/system/arm/virt.rst
> @@ -55,6 +55,7 @@ Supported guest CPU types:
>  - ``cortex-a53`` (64-bit)
>  - ``cortex-a57`` (64-bit)
>  - ``cortex-a72`` (64-bit)
> +- ``a64fx`` (64-bit)
>  - ``host`` (with KVM only)
>  - ``max`` (same as ``host`` for KVM; best possible emulation with TCG)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 81eda46b0b..10286d3fd6 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -200,6 +200,7 @@ static const char *valid_cpus[] = {
>      ARM_CPU_TYPE_NAME("cortex-a53"),
>      ARM_CPU_TYPE_NAME("cortex-a57"),
>      ARM_CPU_TYPE_NAME("cortex-a72"),
> +    ARM_CPU_TYPE_NAME("a64fx"),
>      ARM_CPU_TYPE_NAME("host"),
>      ARM_CPU_TYPE_NAME("max"),
>  };

thanks
-- PMM

RE: [PATCH v2 3/3] target-arm: Add A64FX processor support to virt machine

[ishii.shuuichir]

> Commit messages should describe what the patch is doing and why, so the reader
> can understand it without having to cross-reference old mailing list threads.

Thank you for your comment.
I understood your point.

Best regards.
> -----Original Message-----
> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Friday, July 30, 2021 7:36 PM
> To: Ishii, Shuuichirou <ishii.shuuichir@fujitsu.com>
> Cc: qemu-arm <qemu-arm@nongnu.org>; QEMU Developers
> <qemu-devel@nongnu.org>
> Subject: Re: [PATCH v2 3/3] target-arm: Add A64FX processor support to virt
> machine
> 
> On Fri, 30 Jul 2021 at 04:08, Shuuichirou Ishii <ishii.shuuichir@fujitsu.com>
> wrote:
> >
> > Fix for patch consistency.
> > https://lists.gnu.org/archive/html/qemu-devel/2021-07/msg06993.html
> 
> Commit messages should describe what the patch is doing and why, so the reader
> can understand it without having to cross-reference old mailing list threads.
> 
> > Signed-off-by: Shuuichirou Ishii <ishii.shuuichir@fujitsu.com>
> > ---
> >  docs/system/arm/virt.rst | 1 +
> >  hw/arm/virt.c            | 1 +
> >  2 files changed, 2 insertions(+)
> >
> > diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst index
> > 27652adfae..5329e952cf 100644
> > --- a/docs/system/arm/virt.rst
> > +++ b/docs/system/arm/virt.rst
> > @@ -55,6 +55,7 @@ Supported guest CPU types:
> >  - ``cortex-a53`` (64-bit)
> >  - ``cortex-a57`` (64-bit)
> >  - ``cortex-a72`` (64-bit)
> > +- ``a64fx`` (64-bit)
> >  - ``host`` (with KVM only)
> >  - ``max`` (same as ``host`` for KVM; best possible emulation with
> > TCG)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c index
> > 81eda46b0b..10286d3fd6 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -200,6 +200,7 @@ static const char *valid_cpus[] = {
> >      ARM_CPU_TYPE_NAME("cortex-a53"),
> >      ARM_CPU_TYPE_NAME("cortex-a57"),
> >      ARM_CPU_TYPE_NAME("cortex-a72"),
> > +    ARM_CPU_TYPE_NAME("a64fx"),
> >      ARM_CPU_TYPE_NAME("host"),
> >      ARM_CPU_TYPE_NAME("max"),
> >  };
> 
> thanks
> -- PMM

Re: [PATCH v2 0/3] Add support for Fujitsu A64FX processor

[Peter Maydell]

On Fri, 30 Jul 2021 at 04:08, Shuuichirou Ishii
<ishii.shuuichir@fujitsu.com> wrote:
>
> This is the v2 patch series.
>
> v2:
> No features have been added or removed from the v1 patch series. Removal
> of unused definitions that were added in excess, and consolidation of
> patches for the purpose of functional consistency.
>
> For patch 1, ARM_FEATURE_A64FX is not used in the v1 patch series, so it
> was deleted this time, and will be added again when it is used.
>
> For patch 2, since the a64fx_cp_reginfo structure is not used in the v1
> patch series, I deleted the empty definition and added the TODO in the
> aarch64_a64fx_initfn function. Also fixed the appearance, and cleaned up
> and removed some things for patch consistency.
>
> For patch 3, a64fx was added to docs/system/arm/virt.rst and
> hw/arm/virt.c respectively, as a modification to the patch consistency
> cleanup done in patch 2.

I'm afraid this isn't the way a v2 patchseries should be structured.
The idea is that a v2 series should be complete in itself, not based
on whatever v1 was. So when you make the changes requested in review
of v1, you update the commits in your local git branch, and then you
send out the patches as the v2. v2 should apply cleanly on to master,
and all the patches in it should be logically separated out changes
(with no "patch 1 makes a change and then patch 2 changes the code
that was added in patch 1" effects).

thanks
-- PMM

RE: [PATCH v2 0/3] Add support for Fujitsu A64FX processor

[ishii.shuuichir]

> I'm afraid this isn't the way a v2 patchseries should be structured.
> The idea is that a v2 series should be complete in itself, not based on whatever v1
> was. So when you make the changes requested in review of v1, you update the
> commits in your local git branch, and then you send out the patches as the v2. v2
> should apply cleanly on to master, and all the patches in it should be logically
> separated out changes (with no "patch 1 makes a change and then patch 2
> changes the code that was added in patch 1" effects).

Thank you for comments.
We apologize for the inconvenience caused by our lack of understanding.
I understood your point.

Just to confirm, 
should I update to v3 and resubmit it as a patch series based on the points you mentioned?

Best regards.

> -----Original Message-----
> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Friday, July 30, 2021 7:39 PM
> To: Ishii, Shuuichirou/石井 周一郎 <ishii.shuuichir@fujitsu.com>
> Cc: qemu-arm <qemu-arm@nongnu.org>; QEMU Developers
> <qemu-devel@nongnu.org>
> Subject: Re: [PATCH v2 0/3] Add support for Fujitsu A64FX processor
> 
> On Fri, 30 Jul 2021 at 04:08, Shuuichirou Ishii <ishii.shuuichir@fujitsu.com>
> wrote:
> >
> > This is the v2 patch series.
> >
> > v2:
> > No features have been added or removed from the v1 patch series.
> > Removal of unused definitions that were added in excess, and
> > consolidation of patches for the purpose of functional consistency.
> >
> > For patch 1, ARM_FEATURE_A64FX is not used in the v1 patch series, so
> > it was deleted this time, and will be added again when it is used.
> >
> > For patch 2, since the a64fx_cp_reginfo structure is not used in the
> > v1 patch series, I deleted the empty definition and added the TODO in
> > the aarch64_a64fx_initfn function. Also fixed the appearance, and
> > cleaned up and removed some things for patch consistency.
> >
> > For patch 3, a64fx was added to docs/system/arm/virt.rst and
> > hw/arm/virt.c respectively, as a modification to the patch consistency
> > cleanup done in patch 2.
> 
> I'm afraid this isn't the way a v2 patchseries should be structured.
> The idea is that a v2 series should be complete in itself, not based on whatever v1
> was. So when you make the changes requested in review of v1, you update the
> commits in your local git branch, and then you send out the patches as the v2. v2
> should apply cleanly on to master, and all the patches in it should be logically
> separated out changes (with no "patch 1 makes a change and then patch 2
> changes the code that was added in patch 1" effects).
> 
> thanks
> -- PMM

Re: [PATCH v2 0/3] Add support for Fujitsu A64FX processor

[Peter Maydell]

On Tue, 3 Aug 2021 at 01:37, ishii.shuuichir@fujitsu.com
<ishii.shuuichir@fujitsu.com> wrote:
>
> > I'm afraid this isn't the way a v2 patchseries should be structured.
> > The idea is that a v2 series should be complete in itself, not based on whatever v1
> > was. So when you make the changes requested in review of v1, you update the
> > commits in your local git branch, and then you send out the patches as the v2. v2
> > should apply cleanly on to master, and all the patches in it should be logically
> > separated out changes (with no "patch 1 makes a change and then patch 2
> > changes the code that was added in patch 1" effects).
>
> Thank you for comments.
> We apologize for the inconvenience caused by our lack of understanding.
> I understood your point.
>
> Just to confirm,
> should I update to v3 and resubmit it as a patch series based on the points you mentioned?

Yes, please.

thanks
-- PMM