Re: [PATCH v5 07/26] arm64/sve: Clarify role of the VQ map maintenance functions
On Wed, Feb 20, 2019 at 11:43:24AM +, Julien Thierry wrote:
>
>
> On 18/02/2019 19:52, Dave Martin wrote:
> > The roles of sve_init_vq_map(), sve_update_vq_map() and
> > sve_verify_vq_map() are highly non-obvious to anyone who has not dug
> > through cpufeatures.c in detail.
> >
> > Since the way these functions interact with each other is more
> > important here than a full understanding of the cpufeatures code, this
> > patch adds comments to make the functions' roles clearer.
> >
> > No functional change.
> >
> > Signed-off-by: Dave Martin
> > ---
> > arch/arm64/kernel/fpsimd.c | 10 +-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> > index 64729e2..92c2331 100644
> > --- a/arch/arm64/kernel/fpsimd.c
> > +++ b/arch/arm64/kernel/fpsimd.c
> > @@ -647,6 +647,10 @@ static void sve_probe_vqs(DECLARE_BITMAP(map,
> > SVE_VQ_MAX))
> > }
> > }
> >
> > +/*
> > + * Initialise the set of known supported VQs for the boot CPU.
> > + * This is called during kernel boot, before secondary CPUs are brought up.
> > + */
> > void __init sve_init_vq_map(void)
> > {
> > sve_probe_vqs(sve_vq_map);
> > @@ -656,6 +660,7 @@ void __init sve_init_vq_map(void)
> > /*
> > * If we haven't committed to the set of supported VQs yet, filter out
> > * those not supported by the current CPU.
> > + * This function is called during the bring-up of early secondary CPUs
> > only.
> > */
> > void sve_update_vq_map(void)
> > {
> > @@ -666,7 +671,10 @@ void sve_update_vq_map(void)
> > bitmap_or(sve_vq_partial_map, sve_vq_partial_map, tmp_map, SVE_VQ_MAX);
> > }
> >
> > -/* Check whether the current CPU supports all VQs in the committed set */
> > +/*
> > + * Check whether the current CPU supports all VQs in the committed set.
> > + * This function is called during the bring-up of late secondary CPUs only.
>
> Oh I see, this is for late CPUs. So you can probably disregard my
> comment on the warning in the previous patch.
>
> If you respin this series, I feel it would be more useful to have this
> patch before the current patch 6.
Agreed, I'll swap them over. Looking at the two patches, I'm not sure
how they ended up this way round. It may have been an unintended side-
effect of a previous rebase.
> Reviewed-by: Julien Thierry
Thanks
---Dave
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 07/26] arm64/sve: Clarify role of the VQ map maintenance functions
On Thu, Feb 21, 2019 at 01:46:46PM +, Julien Grall wrote: > Hi Dave, > > On 18/02/2019 19:52, Dave Martin wrote: > >The roles of sve_init_vq_map(), sve_update_vq_map() and > >sve_verify_vq_map() are highly non-obvious to anyone who has not dug > >through cpufeatures.c in detail. > > > >Since the way these functions interact with each other is more > >important here than a full understanding of the cpufeatures code, this > >patch adds comments to make the functions' roles clearer. > > > >No functional change. > > > >Signed-off-by: Dave Martin > > Reviewed-by: Julien Grall Thanks ---Dave ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 07/26] arm64/sve: Clarify role of the VQ map maintenance functions
Hi Dave, On 18/02/2019 19:52, Dave Martin wrote: The roles of sve_init_vq_map(), sve_update_vq_map() and sve_verify_vq_map() are highly non-obvious to anyone who has not dug through cpufeatures.c in detail. Since the way these functions interact with each other is more important here than a full understanding of the cpufeatures code, this patch adds comments to make the functions' roles clearer. No functional change. Signed-off-by: Dave Martin Reviewed-by: Julien Grall Cheers, -- Julien Grall ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 07/26] arm64/sve: Clarify role of the VQ map maintenance functions
On 18/02/2019 19:52, Dave Martin wrote:
> The roles of sve_init_vq_map(), sve_update_vq_map() and
> sve_verify_vq_map() are highly non-obvious to anyone who has not dug
> through cpufeatures.c in detail.
>
> Since the way these functions interact with each other is more
> important here than a full understanding of the cpufeatures code, this
> patch adds comments to make the functions' roles clearer.
>
> No functional change.
>
> Signed-off-by: Dave Martin
> ---
> arch/arm64/kernel/fpsimd.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index 64729e2..92c2331 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -647,6 +647,10 @@ static void sve_probe_vqs(DECLARE_BITMAP(map,
> SVE_VQ_MAX))
> }
> }
>
> +/*
> + * Initialise the set of known supported VQs for the boot CPU.
> + * This is called during kernel boot, before secondary CPUs are brought up.
> + */
> void __init sve_init_vq_map(void)
> {
> sve_probe_vqs(sve_vq_map);
> @@ -656,6 +660,7 @@ void __init sve_init_vq_map(void)
> /*
> * If we haven't committed to the set of supported VQs yet, filter out
> * those not supported by the current CPU.
> + * This function is called during the bring-up of early secondary CPUs only.
> */
> void sve_update_vq_map(void)
> {
> @@ -666,7 +671,10 @@ void sve_update_vq_map(void)
> bitmap_or(sve_vq_partial_map, sve_vq_partial_map, tmp_map, SVE_VQ_MAX);
> }
>
> -/* Check whether the current CPU supports all VQs in the committed set */
> +/*
> + * Check whether the current CPU supports all VQs in the committed set.
> + * This function is called during the bring-up of late secondary CPUs only.
Oh I see, this is for late CPUs. So you can probably disregard my
comment on the warning in the previous patch.
If you respin this series, I feel it would be more useful to have this
patch before the current patch 6.
Reviewed-by: Julien Thierry
--
Julien Thierry
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v5 07/26] arm64/sve: Clarify role of the VQ map maintenance functions
The roles of sve_init_vq_map(), sve_update_vq_map() and
sve_verify_vq_map() are highly non-obvious to anyone who has not dug
through cpufeatures.c in detail.
Since the way these functions interact with each other is more
important here than a full understanding of the cpufeatures code, this
patch adds comments to make the functions' roles clearer.
No functional change.
Signed-off-by: Dave Martin
---
arch/arm64/kernel/fpsimd.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 64729e2..92c2331 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -647,6 +647,10 @@ static void sve_probe_vqs(DECLARE_BITMAP(map, SVE_VQ_MAX))
}
}
+/*
+ * Initialise the set of known supported VQs for the boot CPU.
+ * This is called during kernel boot, before secondary CPUs are brought up.
+ */
void __init sve_init_vq_map(void)
{
sve_probe_vqs(sve_vq_map);
@@ -656,6 +660,7 @@ void __init sve_init_vq_map(void)
/*
* If we haven't committed to the set of supported VQs yet, filter out
* those not supported by the current CPU.
+ * This function is called during the bring-up of early secondary CPUs only.
*/
void sve_update_vq_map(void)
{
@@ -666,7 +671,10 @@ void sve_update_vq_map(void)
bitmap_or(sve_vq_partial_map, sve_vq_partial_map, tmp_map, SVE_VQ_MAX);
}
-/* Check whether the current CPU supports all VQs in the committed set */
+/*
+ * Check whether the current CPU supports all VQs in the committed set.
+ * This function is called during the bring-up of late secondary CPUs only.
+ */
int sve_verify_vq_map(void)
{
DECLARE_BITMAP(tmp_map, SVE_VQ_MAX);
--
2.1.4
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
