| Message ID | 20240420-dev-charlie-support_thead_vector_6_9-v3-6-67cff4271d1d@rivosinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | riscv: Support vendor extensions and xtheadvector | expand |
Hi Charlie, On 21/04/2024 03:04, Charlie Jenkins wrote: > This loop is supposed to check if ext->subset_ext_ids[j] is valid, rather > than if ext->subset_ext_ids[i] is valid, before setting the extension > id ext->subset_ext_ids[j] in isainfo->isa. > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > Fixes: 0d8295ed975b ("riscv: add ISA extension parsing for scalar crypto") > --- > arch/riscv/kernel/cpufeature.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c > index 48874aac4871..b537731cadef 100644 > --- a/arch/riscv/kernel/cpufeature.c > +++ b/arch/riscv/kernel/cpufeature.c > @@ -609,7 +609,7 @@ static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap) > > if (ext->subset_ext_size) { > for (int j = 0; j < ext->subset_ext_size; j++) { > - if (riscv_isa_extension_check(ext->subset_ext_ids[i])) > + if (riscv_isa_extension_check(ext->subset_ext_ids[j])) > set_bit(ext->subset_ext_ids[j], isainfo->isa); > } > } > I think this should go into -fixes, let's check with Palmer if he wants to take this patch only or if you should send the patch on its own. You can add: Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com> Thanks, Alex
On Wed, Apr 24, 2024 at 04:22:02PM +0200, Alexandre Ghiti wrote: > Hi Charlie, > > On 21/04/2024 03:04, Charlie Jenkins wrote: > > This loop is supposed to check if ext->subset_ext_ids[j] is valid, rather > > than if ext->subset_ext_ids[i] is valid, before setting the extension > > id ext->subset_ext_ids[j] in isainfo->isa. > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > > Fixes: 0d8295ed975b ("riscv: add ISA extension parsing for scalar crypto") > > --- > > arch/riscv/kernel/cpufeature.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c > > index 48874aac4871..b537731cadef 100644 > > --- a/arch/riscv/kernel/cpufeature.c > > +++ b/arch/riscv/kernel/cpufeature.c > > @@ -609,7 +609,7 @@ static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap) > > if (ext->subset_ext_size) { > > for (int j = 0; j < ext->subset_ext_size; j++) { > > - if (riscv_isa_extension_check(ext->subset_ext_ids[i])) > > + if (riscv_isa_extension_check(ext->subset_ext_ids[j])) > > set_bit(ext->subset_ext_ids[j], isainfo->isa); > > } > > } > > > > I think this should go into -fixes, let's check with Palmer if he wants to > take this patch only or if you should send the patch on its own. I think splitting out this and patch 1 into a new series targeting fixes would probably make things clearer?
On Wed, Apr 24, 2024 at 03:51:54PM +0100, Conor Dooley wrote: > On Wed, Apr 24, 2024 at 04:22:02PM +0200, Alexandre Ghiti wrote: > > Hi Charlie, > > > > On 21/04/2024 03:04, Charlie Jenkins wrote: > > > This loop is supposed to check if ext->subset_ext_ids[j] is valid, rather > > > than if ext->subset_ext_ids[i] is valid, before setting the extension > > > id ext->subset_ext_ids[j] in isainfo->isa. > > > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > > > Fixes: 0d8295ed975b ("riscv: add ISA extension parsing for scalar crypto") > > > --- > > > arch/riscv/kernel/cpufeature.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c > > > index 48874aac4871..b537731cadef 100644 > > > --- a/arch/riscv/kernel/cpufeature.c > > > +++ b/arch/riscv/kernel/cpufeature.c > > > @@ -609,7 +609,7 @@ static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap) > > > if (ext->subset_ext_size) { > > > for (int j = 0; j < ext->subset_ext_size; j++) { > > > - if (riscv_isa_extension_check(ext->subset_ext_ids[i])) > > > + if (riscv_isa_extension_check(ext->subset_ext_ids[j])) > > > set_bit(ext->subset_ext_ids[j], isainfo->isa); > > > } > > > } > > > > > > > I think this should go into -fixes, let's check with Palmer if he wants to > > take this patch only or if you should send the patch on its own. > > I think splitting out this and patch 1 into a new series targeting fixes > would probably make things clearer? Okay I can do that. I will give it a bit more time before I send this series split into two to allow time for the rest of the patches to gather comments so I avoid sending too many duplicate patches. - Charlie
On Wed, Apr 24, 2024 at 11:13:40AM -0400, Charlie Jenkins wrote: > On Wed, Apr 24, 2024 at 03:51:54PM +0100, Conor Dooley wrote: > > On Wed, Apr 24, 2024 at 04:22:02PM +0200, Alexandre Ghiti wrote: > > > Hi Charlie, > > > > > > On 21/04/2024 03:04, Charlie Jenkins wrote: > > > > This loop is supposed to check if ext->subset_ext_ids[j] is valid, rather > > > > than if ext->subset_ext_ids[i] is valid, before setting the extension > > > > id ext->subset_ext_ids[j] in isainfo->isa. > > > > > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > > > > Fixes: 0d8295ed975b ("riscv: add ISA extension parsing for scalar crypto") > > > > --- > > > > arch/riscv/kernel/cpufeature.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c > > > > index 48874aac4871..b537731cadef 100644 > > > > --- a/arch/riscv/kernel/cpufeature.c > > > > +++ b/arch/riscv/kernel/cpufeature.c > > > > @@ -609,7 +609,7 @@ static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap) > > > > if (ext->subset_ext_size) { > > > > for (int j = 0; j < ext->subset_ext_size; j++) { > > > > - if (riscv_isa_extension_check(ext->subset_ext_ids[i])) > > > > + if (riscv_isa_extension_check(ext->subset_ext_ids[j])) > > > > set_bit(ext->subset_ext_ids[j], isainfo->isa); > > > > } > > > > } > > > > > > > > > > I think this should go into -fixes, let's check with Palmer if he wants to > > > take this patch only or if you should send the patch on its own. > > > > I think splitting out this and patch 1 into a new series targeting fixes > > would probably make things clearer? > > Okay I can do that. I will give it a bit more time before I send this > series split into two to allow time for the rest of the patches to > gather comments so I avoid sending too many duplicate patches. Ye, I do hope to get back to this series later in the week when I have time to actually read through all of the patches in detail. However, you wouldn't have to resend both parts of the series - you can just split out the fixes portion and send that, leaving the rest of the series sitting on the list to gather comments.
On Wed, Apr 24, 2024 at 04:21:05PM +0100, Conor Dooley wrote: > On Wed, Apr 24, 2024 at 11:13:40AM -0400, Charlie Jenkins wrote: > > On Wed, Apr 24, 2024 at 03:51:54PM +0100, Conor Dooley wrote: > > > On Wed, Apr 24, 2024 at 04:22:02PM +0200, Alexandre Ghiti wrote: > > > > Hi Charlie, > > > > > > > > On 21/04/2024 03:04, Charlie Jenkins wrote: > > > > > This loop is supposed to check if ext->subset_ext_ids[j] is valid, rather > > > > > than if ext->subset_ext_ids[i] is valid, before setting the extension > > > > > id ext->subset_ext_ids[j] in isainfo->isa. > > > > > > > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > > > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > > > > > Fixes: 0d8295ed975b ("riscv: add ISA extension parsing for scalar crypto") > > > > > --- > > > > > arch/riscv/kernel/cpufeature.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c > > > > > index 48874aac4871..b537731cadef 100644 > > > > > --- a/arch/riscv/kernel/cpufeature.c > > > > > +++ b/arch/riscv/kernel/cpufeature.c > > > > > @@ -609,7 +609,7 @@ static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap) > > > > > if (ext->subset_ext_size) { > > > > > for (int j = 0; j < ext->subset_ext_size; j++) { > > > > > - if (riscv_isa_extension_check(ext->subset_ext_ids[i])) > > > > > + if (riscv_isa_extension_check(ext->subset_ext_ids[j])) > > > > > set_bit(ext->subset_ext_ids[j], isainfo->isa); > > > > > } > > > > > } > > > > > > > > > > > > > I think this should go into -fixes, let's check with Palmer if he wants to > > > > take this patch only or if you should send the patch on its own. > > > > > > I think splitting out this and patch 1 into a new series targeting fixes > > > would probably make things clearer? > > > > Okay I can do that. I will give it a bit more time before I send this > > series split into two to allow time for the rest of the patches to > > gather comments so I avoid sending too many duplicate patches. > > Ye, I do hope to get back to this series later in the week when I have > time to actually read through all of the patches in detail. > > However, you wouldn't have to resend both parts of the series - you can > just split out the fixes portion and send that, leaving the rest of the > series sitting on the list to gather comments. Oh cool, I will send those two patches out in their own series then. - Charlie
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index 48874aac4871..b537731cadef 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -609,7 +609,7 @@ static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap) if (ext->subset_ext_size) { for (int j = 0; j < ext->subset_ext_size; j++) { - if (riscv_isa_extension_check(ext->subset_ext_ids[i])) + if (riscv_isa_extension_check(ext->subset_ext_ids[j])) set_bit(ext->subset_ext_ids[j], isainfo->isa); } }