| Message ID | 1550519559-15915-8-git-send-email-Dave.Martin@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: SVE guest support | expand |
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 <Dave.Martin@arm.com> > --- > 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@arm.com>
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 <Dave.Martin@arm.com> Reviewed-by: Julien Grall <julien.grall@arm.com> Cheers,
On Wed, Feb 20, 2019 at 11:43:24AM +0000, 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 <Dave.Martin@arm.com> > > --- > > 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 <julien.thierry@arm.com> Thanks ---Dave
On Thu, Feb 21, 2019 at 01:46:46PM +0000, 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 <Dave.Martin@arm.com> > > Reviewed-by: Julien Grall <julien.grall@arm.com> Thanks ---Dave
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);
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 <Dave.Martin@arm.com> --- arch/arm64/kernel/fpsimd.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)