| Message ID | 20240411-dev-charlie-support_thead_vector_6_9-v1-6-4af9815ec746@rivosinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | riscv: Support vendor extensions and xtheadvector | expand |
On Thu, Apr 11, 2024 at 09:11:12PM -0700, Charlie Jenkins wrote: > static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct riscv_isainfo *isainfo, > - unsigned long *isa2hwcap, const char *isa) > + struct riscv_isainfo *isavendorinfo, unsigned long vendorid, > + unsigned long *isa2hwcap, const char *isa) > { > /* > * For all possible cpus, we have already validated in > @@ -349,8 +384,30 @@ static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct risc > const char *ext = isa++; > const char *ext_end = isa; > bool ext_long = false, ext_err = false; > + struct riscv_isainfo *selected_isainfo = isainfo; > + const struct riscv_isa_ext_data *selected_riscv_isa_ext = riscv_isa_ext; > + size_t selected_riscv_isa_ext_count = riscv_isa_ext_count; > + unsigned int id_offset = 0; > > switch (*ext) { > + case 'x': > + case 'X': One quick remark is that we should not go and support this stuff via riscv,isa in my opinion, only allowing it for the riscv,isa-extensions parsing. We don't have a way to define meanings for vendor extensions in this way. ACPI also uses this codepath and at the moment the kernel's docs say we're gonna follow isa string parsing rules in a specific version of the ISA manual. While that manual provides a format for the string and meanings for standard extensions, there's nothing in there that allows us to get consistent meanings for specific vendor extensions, so I think we should avoid intentionally supporting this here. I'd probably go as far as to actively skip vendor extensions in riscv_parse_isa_string() to avoid any potential issues. > + bool found; > + > + found = get_isa_vendor_ext(vendorid, > + &selected_riscv_isa_ext, > + &selected_riscv_isa_ext_count); > + selected_isainfo = isavendorinfo; > + id_offset = RISCV_ISA_VENDOR_EXT_BASE; > + if (!found) { > + pr_warn("No associated vendor extensions with vendor id: %lx\n", > + vendorid); This should not be a warning, anything we don't understand should be silently ignored to avoid spamming just because the kernel has not grown support for it yet. Thanks, Conor. > + for (; *isa && *isa != '_'; ++isa) > + ; > + ext_err = true; > + break; > + } > + fallthrough; > case 's': > /* > * Workaround for invalid single-letter 's' & 'u' (QEMU). > @@ -366,8 +423,6 @@ static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct risc > } > fallthrough; > case 'S': > - case 'x': > - case 'X': > case 'z': > case 'Z': > /* > @@ -476,8 +531,10 @@ static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct risc > set_bit(nr, isainfo->isa); > } > } else { > - for (int i = 0; i < riscv_isa_ext_count; i++) > - match_isa_ext(&riscv_isa_ext[i], ext, ext_end, isainfo); > + for (int i = 0; i < selected_riscv_isa_ext_count; i++) > + match_isa_ext(&selected_riscv_isa_ext[i], ext, > + ext_end, selected_isainfo, > + id_offset); > } > } > }
Hi Charlie,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 4cece764965020c22cff7665b18a012006359095]
url: https://github.com/intel-lab-lkp/linux/commits/Charlie-Jenkins/dt-bindings-riscv-Add-vendorid-and-archid/20240412-121709
base: 4cece764965020c22cff7665b18a012006359095
patch link: https://lore.kernel.org/r/20240411-dev-charlie-support_thead_vector_6_9-v1-6-4af9815ec746%40rivosinc.com
patch subject: [PATCH 06/19] riscv: Extend cpufeature.c to detect vendor extensions
config: riscv-defconfig (https://download.01.org/0day-ci/archive/20240412/202404122206.TkXKhj29-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 8b3b4a92adee40483c27f26c478a384cd69c6f05)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240412/202404122206.TkXKhj29-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404122206.TkXKhj29-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from arch/riscv/kernel/cpufeature.c:20:
In file included from arch/riscv/include/asm/cacheflush.h:9:
In file included from include/linux/mm.h:2208:
include/linux/vmstat.h:522:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
522 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> arch/riscv/kernel/cpufeature.c:395:4: warning: label followed by a declaration is a C23 extension [-Wc23-extensions]
395 | bool found;
| ^
2 warnings generated.
vim +395 arch/riscv/kernel/cpufeature.c
370
371 static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct riscv_isainfo *isainfo,
372 struct riscv_isainfo *isavendorinfo, unsigned long vendorid,
373 unsigned long *isa2hwcap, const char *isa)
374 {
375 /*
376 * For all possible cpus, we have already validated in
377 * the boot process that they at least contain "rv" and
378 * whichever of "32"/"64" this kernel supports, and so this
379 * section can be skipped.
380 */
381 isa += 4;
382
383 while (*isa) {
384 const char *ext = isa++;
385 const char *ext_end = isa;
386 bool ext_long = false, ext_err = false;
387 struct riscv_isainfo *selected_isainfo = isainfo;
388 const struct riscv_isa_ext_data *selected_riscv_isa_ext = riscv_isa_ext;
389 size_t selected_riscv_isa_ext_count = riscv_isa_ext_count;
390 unsigned int id_offset = 0;
391
392 switch (*ext) {
393 case 'x':
394 case 'X':
> 395 bool found;
396
397 found = get_isa_vendor_ext(vendorid,
398 &selected_riscv_isa_ext,
399 &selected_riscv_isa_ext_count);
400 selected_isainfo = isavendorinfo;
401 id_offset = RISCV_ISA_VENDOR_EXT_BASE;
402 if (!found) {
403 pr_warn("No associated vendor extensions with vendor id: %lx\n",
404 vendorid);
405 for (; *isa && *isa != '_'; ++isa)
406 ;
407 ext_err = true;
408 break;
409 }
410 fallthrough;
411 case 's':
412 /*
413 * Workaround for invalid single-letter 's' & 'u' (QEMU).
414 * No need to set the bit in riscv_isa as 's' & 'u' are
415 * not valid ISA extensions. It works unless the first
416 * multi-letter extension in the ISA string begins with
417 * "Su" and is not prefixed with an underscore.
418 */
419 if (ext[-1] != '_' && ext[1] == 'u') {
420 ++isa;
421 ext_err = true;
422 break;
423 }
424 fallthrough;
425 case 'S':
426 case 'z':
427 case 'Z':
428 /*
429 * Before attempting to parse the extension itself, we find its end.
430 * As multi-letter extensions must be split from other multi-letter
431 * extensions with an "_", the end of a multi-letter extension will
432 * either be the null character or the "_" at the start of the next
433 * multi-letter extension.
434 *
435 * Next, as the extensions version is currently ignored, we
436 * eliminate that portion. This is done by parsing backwards from
437 * the end of the extension, removing any numbers. This may be a
438 * major or minor number however, so the process is repeated if a
439 * minor number was found.
440 *
441 * ext_end is intended to represent the first character *after* the
442 * name portion of an extension, but will be decremented to the last
443 * character itself while eliminating the extensions version number.
444 * A simple re-increment solves this problem.
445 */
446 ext_long = true;
447 for (; *isa && *isa != '_'; ++isa)
448 if (unlikely(!isalnum(*isa)))
449 ext_err = true;
450
451 ext_end = isa;
452 if (unlikely(ext_err))
453 break;
454
455 if (!isdigit(ext_end[-1]))
456 break;
457
458 while (isdigit(*--ext_end))
459 ;
460
461 if (tolower(ext_end[0]) != 'p' || !isdigit(ext_end[-1])) {
462 ++ext_end;
463 break;
464 }
465
466 while (isdigit(*--ext_end))
467 ;
468
469 ++ext_end;
470 break;
471 default:
472 /*
473 * Things are a little easier for single-letter extensions, as they
474 * are parsed forwards.
475 *
476 * After checking that our starting position is valid, we need to
477 * ensure that, when isa was incremented at the start of the loop,
478 * that it arrived at the start of the next extension.
479 *
480 * If we are already on a non-digit, there is nothing to do. Either
481 * we have a multi-letter extension's _, or the start of an
482 * extension.
483 *
484 * Otherwise we have found the current extension's major version
485 * number. Parse past it, and a subsequent p/minor version number
486 * if present. The `p` extension must not appear immediately after
487 * a number, so there is no fear of missing it.
488 *
489 */
490 if (unlikely(!isalpha(*ext))) {
491 ext_err = true;
492 break;
493 }
494
495 if (!isdigit(*isa))
496 break;
497
498 while (isdigit(*++isa))
499 ;
500
501 if (tolower(*isa) != 'p')
502 break;
503
504 if (!isdigit(*++isa)) {
505 --isa;
506 break;
507 }
508
509 while (isdigit(*++isa))
510 ;
511
512 break;
513 }
514
515 /*
516 * The parser expects that at the start of an iteration isa points to the
517 * first character of the next extension. As we stop parsing an extension
518 * on meeting a non-alphanumeric character, an extra increment is needed
519 * where the succeeding extension is a multi-letter prefixed with an "_".
520 */
521 if (*isa == '_')
522 ++isa;
523
524 if (unlikely(ext_err))
525 continue;
526 if (!ext_long) {
527 int nr = tolower(*ext) - 'a';
528
529 if (riscv_isa_extension_check(nr)) {
530 *this_hwcap |= isa2hwcap[nr];
531 set_bit(nr, isainfo->isa);
532 }
533 } else {
534 for (int i = 0; i < selected_riscv_isa_ext_count; i++)
535 match_isa_ext(&selected_riscv_isa_ext[i], ext,
536 ext_end, selected_isainfo,
537 id_offset);
538 }
539 }
540 }
541
On Fri, Apr 12, 2024 at 01:30:08PM +0100, Conor Dooley wrote: > On Thu, Apr 11, 2024 at 09:11:12PM -0700, Charlie Jenkins wrote: > > static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct riscv_isainfo *isainfo, > > > - unsigned long *isa2hwcap, const char *isa) > > + struct riscv_isainfo *isavendorinfo, unsigned long vendorid, > > + unsigned long *isa2hwcap, const char *isa) > > { > > /* > > * For all possible cpus, we have already validated in > > @@ -349,8 +384,30 @@ static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct risc > > const char *ext = isa++; > > const char *ext_end = isa; > > bool ext_long = false, ext_err = false; > > + struct riscv_isainfo *selected_isainfo = isainfo; > > + const struct riscv_isa_ext_data *selected_riscv_isa_ext = riscv_isa_ext; > > + size_t selected_riscv_isa_ext_count = riscv_isa_ext_count; > > + unsigned int id_offset = 0; > > > > switch (*ext) { > > + case 'x': > > + case 'X': > > One quick remark is that we should not go and support this stuff via > riscv,isa in my opinion, only allowing it for the riscv,isa-extensions > parsing. We don't have a way to define meanings for vendor extensions in > this way. ACPI also uses this codepath and at the moment the kernel's > docs say we're gonna follow isa string parsing rules in a specific version > of the ISA manual. While that manual provides a format for the string and > meanings for standard extensions, there's nothing in there that allows us > to get consistent meanings for specific vendor extensions, so I think we > should avoid intentionally supporting this here. Getting a "consistent meaning" is managed by a vendor. If a vendor supports a vendor extension and puts it in their DT/ACPI table it's up to them to ensure that it works. How does riscv,isa-extensions allow for a consistent meaning? > > I'd probably go as far as to actively skip vendor extensions in > riscv_parse_isa_string() to avoid any potential issues. > > > + bool found; > > + > > + found = get_isa_vendor_ext(vendorid, > > + &selected_riscv_isa_ext, > > + &selected_riscv_isa_ext_count); > > + selected_isainfo = isavendorinfo; > > + id_offset = RISCV_ISA_VENDOR_EXT_BASE; > > + if (!found) { > > + pr_warn("No associated vendor extensions with vendor id: %lx\n", > > + vendorid); > > This should not be a warning, anything we don't understand should be > silently ignored to avoid spamming just because the kernel has not grown > support for it yet. Sounds good. - Charlie > > Thanks, > Conor. > > > + for (; *isa && *isa != '_'; ++isa) > > + ; > > + ext_err = true; > > + break; > > + } > > + fallthrough; > > case 's': > > /* > > * Workaround for invalid single-letter 's' & 'u' (QEMU). > > @@ -366,8 +423,6 @@ static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct risc > > } > > fallthrough; > > case 'S': > > - case 'x': > > - case 'X': > > case 'z': > > case 'Z': > > /* > > @@ -476,8 +531,10 @@ static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct risc > > set_bit(nr, isainfo->isa); > > } > > } else { > > - for (int i = 0; i < riscv_isa_ext_count; i++) > > - match_isa_ext(&riscv_isa_ext[i], ext, ext_end, isainfo); > > + for (int i = 0; i < selected_riscv_isa_ext_count; i++) > > + match_isa_ext(&selected_riscv_isa_ext[i], ext, > > + ext_end, selected_isainfo, > > + id_offset); > > } > > } > > }
On Fri, Apr 12, 2024 at 09:58:04AM -0700, Charlie Jenkins wrote: > On Fri, Apr 12, 2024 at 01:30:08PM +0100, Conor Dooley wrote: > > On Thu, Apr 11, 2024 at 09:11:12PM -0700, Charlie Jenkins wrote: > > > static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct riscv_isainfo *isainfo, > > > > > - unsigned long *isa2hwcap, const char *isa) > > > + struct riscv_isainfo *isavendorinfo, unsigned long vendorid, > > > + unsigned long *isa2hwcap, const char *isa) > > > { > > > /* > > > * For all possible cpus, we have already validated in > > > @@ -349,8 +384,30 @@ static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct risc > > > const char *ext = isa++; > > > const char *ext_end = isa; > > > bool ext_long = false, ext_err = false; > > > + struct riscv_isainfo *selected_isainfo = isainfo; > > > + const struct riscv_isa_ext_data *selected_riscv_isa_ext = riscv_isa_ext; > > > + size_t selected_riscv_isa_ext_count = riscv_isa_ext_count; > > > + unsigned int id_offset = 0; > > > > > > switch (*ext) { > > > + case 'x': > > > + case 'X': > > > > One quick remark is that we should not go and support this stuff via > > riscv,isa in my opinion, only allowing it for the riscv,isa-extensions > > parsing. We don't have a way to define meanings for vendor extensions in > > this way. ACPI also uses this codepath and at the moment the kernel's > > docs say we're gonna follow isa string parsing rules in a specific version > > of the ISA manual. While that manual provides a format for the string and > > meanings for standard extensions, there's nothing in there that allows us > > to get consistent meanings for specific vendor extensions, so I think we > > should avoid intentionally supporting this here. > > Getting a "consistent meaning" is managed by a vendor. IOW, there's absolutely no guarantee of a consistent meaning. > If a vendor > supports a vendor extension and puts it in their DT/ACPI table it's up > to them to ensure that it works. How does riscv,isa-extensions allow for > a consistent meaning? The definitions for each string contain links to exact versions of specifications that they correspond to. > > > > I'd probably go as far as to actively skip vendor extensions in > > riscv_parse_isa_string() to avoid any potential issues. > > > > > + bool found; > > > + > > > + found = get_isa_vendor_ext(vendorid, > > > + &selected_riscv_isa_ext, > > > + &selected_riscv_isa_ext_count); > > > + selected_isainfo = isavendorinfo; > > > + id_offset = RISCV_ISA_VENDOR_EXT_BASE; > > > + if (!found) { > > > + pr_warn("No associated vendor extensions with vendor id: %lx\n", > > > + vendorid); > > > > This should not be a warning, anything we don't understand should be > > silently ignored to avoid spamming just because the kernel has not grown > > support for it yet. > > Sounds good. > > - Charlie > > > > > Thanks, > > Conor. > > > > > + for (; *isa && *isa != '_'; ++isa) > > > + ; > > > + ext_err = true; > > > + break; > > > + } > > > + fallthrough; > > > case 's': > > > /* > > > * Workaround for invalid single-letter 's' & 'u' (QEMU). > > > @@ -366,8 +423,6 @@ static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct risc > > > } > > > fallthrough; > > > case 'S': > > > - case 'x': > > > - case 'X': > > > case 'z': > > > case 'Z': > > > /* > > > @@ -476,8 +531,10 @@ static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct risc > > > set_bit(nr, isainfo->isa); > > > } > > > } else { > > > - for (int i = 0; i < riscv_isa_ext_count; i++) > > > - match_isa_ext(&riscv_isa_ext[i], ext, ext_end, isainfo); > > > + for (int i = 0; i < selected_riscv_isa_ext_count; i++) > > > + match_isa_ext(&selected_riscv_isa_ext[i], ext, > > > + ext_end, selected_isainfo, > > > + id_offset); > > > } > > > } > > > } > >
Hi Charlie, kernel test robot noticed the following build errors: [auto build test ERROR on 4cece764965020c22cff7665b18a012006359095] url: https://github.com/intel-lab-lkp/linux/commits/Charlie-Jenkins/dt-bindings-riscv-Add-vendorid-and-archid/20240412-121709 base: 4cece764965020c22cff7665b18a012006359095 patch link: https://lore.kernel.org/r/20240411-dev-charlie-support_thead_vector_6_9-v1-6-4af9815ec746%40rivosinc.com patch subject: [PATCH 06/19] riscv: Extend cpufeature.c to detect vendor extensions config: riscv-randconfig-r133-20240413 (https://download.01.org/0day-ci/archive/20240414/202404140621.x9B02eF8-lkp@intel.com/config) compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18) reproduce: (https://download.01.org/0day-ci/archive/20240414/202404140621.x9B02eF8-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202404140621.x9B02eF8-lkp@intel.com/ All errors (new ones prefixed by >>): >> arch/riscv/kernel/cpufeature.c:395:4: error: expected expression 395 | bool found; | ^ >> arch/riscv/kernel/cpufeature.c:397:4: error: use of undeclared identifier 'found' 397 | found = get_isa_vendor_ext(vendorid, | ^ arch/riscv/kernel/cpufeature.c:402:9: error: use of undeclared identifier 'found' 402 | if (!found) { | ^ 3 errors generated. vim +395 arch/riscv/kernel/cpufeature.c 370 371 static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct riscv_isainfo *isainfo, 372 struct riscv_isainfo *isavendorinfo, unsigned long vendorid, 373 unsigned long *isa2hwcap, const char *isa) 374 { 375 /* 376 * For all possible cpus, we have already validated in 377 * the boot process that they at least contain "rv" and 378 * whichever of "32"/"64" this kernel supports, and so this 379 * section can be skipped. 380 */ 381 isa += 4; 382 383 while (*isa) { 384 const char *ext = isa++; 385 const char *ext_end = isa; 386 bool ext_long = false, ext_err = false; 387 struct riscv_isainfo *selected_isainfo = isainfo; 388 const struct riscv_isa_ext_data *selected_riscv_isa_ext = riscv_isa_ext; 389 size_t selected_riscv_isa_ext_count = riscv_isa_ext_count; 390 unsigned int id_offset = 0; 391 392 switch (*ext) { 393 case 'x': 394 case 'X': > 395 bool found; 396 > 397 found = get_isa_vendor_ext(vendorid, 398 &selected_riscv_isa_ext, 399 &selected_riscv_isa_ext_count); 400 selected_isainfo = isavendorinfo; 401 id_offset = RISCV_ISA_VENDOR_EXT_BASE; 402 if (!found) { 403 pr_warn("No associated vendor extensions with vendor id: %lx\n", 404 vendorid); 405 for (; *isa && *isa != '_'; ++isa) 406 ; 407 ext_err = true; 408 break; 409 } 410 fallthrough; 411 case 's': 412 /* 413 * Workaround for invalid single-letter 's' & 'u' (QEMU). 414 * No need to set the bit in riscv_isa as 's' & 'u' are 415 * not valid ISA extensions. It works unless the first 416 * multi-letter extension in the ISA string begins with 417 * "Su" and is not prefixed with an underscore. 418 */ 419 if (ext[-1] != '_' && ext[1] == 'u') { 420 ++isa; 421 ext_err = true; 422 break; 423 } 424 fallthrough; 425 case 'S': 426 case 'z': 427 case 'Z': 428 /* 429 * Before attempting to parse the extension itself, we find its end. 430 * As multi-letter extensions must be split from other multi-letter 431 * extensions with an "_", the end of a multi-letter extension will 432 * either be the null character or the "_" at the start of the next 433 * multi-letter extension. 434 * 435 * Next, as the extensions version is currently ignored, we 436 * eliminate that portion. This is done by parsing backwards from 437 * the end of the extension, removing any numbers. This may be a 438 * major or minor number however, so the process is repeated if a 439 * minor number was found. 440 * 441 * ext_end is intended to represent the first character *after* the 442 * name portion of an extension, but will be decremented to the last 443 * character itself while eliminating the extensions version number. 444 * A simple re-increment solves this problem. 445 */ 446 ext_long = true; 447 for (; *isa && *isa != '_'; ++isa) 448 if (unlikely(!isalnum(*isa))) 449 ext_err = true; 450 451 ext_end = isa; 452 if (unlikely(ext_err)) 453 break; 454 455 if (!isdigit(ext_end[-1])) 456 break; 457 458 while (isdigit(*--ext_end)) 459 ; 460 461 if (tolower(ext_end[0]) != 'p' || !isdigit(ext_end[-1])) { 462 ++ext_end; 463 break; 464 } 465 466 while (isdigit(*--ext_end)) 467 ; 468 469 ++ext_end; 470 break; 471 default: 472 /* 473 * Things are a little easier for single-letter extensions, as they 474 * are parsed forwards. 475 * 476 * After checking that our starting position is valid, we need to 477 * ensure that, when isa was incremented at the start of the loop, 478 * that it arrived at the start of the next extension. 479 * 480 * If we are already on a non-digit, there is nothing to do. Either 481 * we have a multi-letter extension's _, or the start of an 482 * extension. 483 * 484 * Otherwise we have found the current extension's major version 485 * number. Parse past it, and a subsequent p/minor version number 486 * if present. The `p` extension must not appear immediately after 487 * a number, so there is no fear of missing it. 488 * 489 */ 490 if (unlikely(!isalpha(*ext))) { 491 ext_err = true; 492 break; 493 } 494 495 if (!isdigit(*isa)) 496 break; 497 498 while (isdigit(*++isa)) 499 ; 500 501 if (tolower(*isa) != 'p') 502 break; 503 504 if (!isdigit(*++isa)) { 505 --isa; 506 break; 507 } 508 509 while (isdigit(*++isa)) 510 ; 511 512 break; 513 } 514 515 /* 516 * The parser expects that at the start of an iteration isa points to the 517 * first character of the next extension. As we stop parsing an extension 518 * on meeting a non-alphanumeric character, an extra increment is needed 519 * where the succeeding extension is a multi-letter prefixed with an "_". 520 */ 521 if (*isa == '_') 522 ++isa; 523 524 if (unlikely(ext_err)) 525 continue; 526 if (!ext_long) { 527 int nr = tolower(*ext) - 'a'; 528 529 if (riscv_isa_extension_check(nr)) { 530 *this_hwcap |= isa2hwcap[nr]; 531 set_bit(nr, isainfo->isa); 532 } 533 } else { 534 for (int i = 0; i < selected_riscv_isa_ext_count; i++) 535 match_isa_ext(&selected_riscv_isa_ext[i], ext, 536 ext_end, selected_isainfo, 537 id_offset); 538 } 539 } 540 } 541
diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h index 347805446151..b5f4eedcfa86 100644 --- a/arch/riscv/include/asm/cpufeature.h +++ b/arch/riscv/include/asm/cpufeature.h @@ -26,11 +26,18 @@ struct riscv_isainfo { DECLARE_BITMAP(isa, RISCV_ISA_EXT_MAX); }; +struct riscv_isavendorinfo { + DECLARE_BITMAP(isa, RISCV_ISA_VENDOR_EXT_SIZE); +}; + DECLARE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo); /* Per-cpu ISA extensions. */ extern struct riscv_isainfo hart_isa[NR_CPUS]; +/* Per-cpu ISA vendor extensions. */ +extern struct riscv_isainfo hart_isa_vendor[NR_CPUS]; + void riscv_user_isa_enable(void); #if defined(CONFIG_RISCV_MISALIGNED) diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h index e17d0078a651..38157be5becd 100644 --- a/arch/riscv/include/asm/hwcap.h +++ b/arch/riscv/include/asm/hwcap.h @@ -87,6 +87,29 @@ #define RISCV_ISA_EXT_MAX 128 #define RISCV_ISA_EXT_INVALID U32_MAX +/* + * These macros represent the logical IDs of each vendor RISC-V ISA extension + * and are used in each vendor ISA bitmap. The logical IDs start from + * RISCV_ISA_VENDOR_EXT_BASE, which allows the 0-0x7999 range to be + * reserved for non-vendor extensions. The maximum, RISCV_ISA_VENDOR_EXT_MAX, + * is defined in order to allocate the bitmap and may be increased when + * necessary. + * + * Values are expected to overlap between vendors. + * + * New extensions should just be added to the bottom of the respective vendor, + * rather than added alphabetically, in order to avoid unnecessary shuffling. + * + */ +#define RISCV_ISA_VENDOR_EXT_BASE 0x8000 + +/* THead Vendor Extensions */ +#define RISCV_ISA_VENDOR_EXT_XTHEADVECTOR 0x8000 + +#define RISCV_ISA_VENDOR_EXT_MAX 0x8080 +#define RISCV_ISA_VENDOR_EXT_SIZE (RISCV_ISA_VENDOR_EXT_MAX - RISCV_ISA_VENDOR_EXT_BASE) +#define RISCV_ISA_VENDOR_EXT_INVALID U32_MAX + #ifdef CONFIG_RISCV_M_MODE #define RISCV_ISA_EXT_SxAIA RISCV_ISA_EXT_SMAIA #else diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index 5eb52d270a9a..f72fbdd0d7f5 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -32,9 +32,15 @@ unsigned long elf_hwcap __read_mostly; /* Host ISA bitmap */ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly; +/* Host ISA vendor bitmap */ +static DECLARE_BITMAP(riscv_isa_vendor, RISCV_ISA_VENDOR_EXT_SIZE) __read_mostly; + /* Per-cpu ISA extensions. */ struct riscv_isainfo hart_isa[NR_CPUS]; +/* Per-cpu ISA vendor extensions. */ +struct riscv_isainfo hart_isa_vendor[NR_CPUS]; + /** * riscv_isa_extension_base() - Get base extension word * @@ -309,8 +315,15 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = { const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext); +const struct riscv_isa_ext_data riscv_isa_vendor_ext_thead[] = { + __RISCV_ISA_EXT_DATA(xtheadvector, RISCV_ISA_VENDOR_EXT_XTHEADVECTOR), +}; + +const size_t riscv_isa_vendor_ext_count_thead = ARRAY_SIZE(riscv_isa_vendor_ext_thead); + static void __init match_isa_ext(const struct riscv_isa_ext_data *ext, const char *name, - const char *name_end, struct riscv_isainfo *isainfo) + const char *name_end, struct riscv_isainfo *isainfo, + unsigned int id_offset) { if ((name_end - name == strlen(ext->name)) && !strncasecmp(name, ext->name, name_end - name)) { @@ -321,7 +334,7 @@ static void __init match_isa_ext(const struct riscv_isa_ext_data *ext, const cha if (ext->subset_ext_size) { for (int i = 0; i < ext->subset_ext_size; i++) { if (riscv_isa_extension_check(ext->subset_ext_ids[i])) - set_bit(ext->subset_ext_ids[i], isainfo->isa); + set_bit(ext->subset_ext_ids[i] - id_offset, isainfo->isa); } } @@ -330,12 +343,34 @@ static void __init match_isa_ext(const struct riscv_isa_ext_data *ext, const cha * (rejected by riscv_isa_extension_check()). */ if (riscv_isa_extension_check(ext->id)) - set_bit(ext->id, isainfo->isa); + set_bit(ext->id - id_offset, isainfo->isa); + } +} + +static bool __init get_isa_vendor_ext(unsigned long vendorid, + const struct riscv_isa_ext_data **isa_vendor_ext, + size_t *count) +{ + bool found_vendor = true; + + switch (vendorid) { + case THEAD_VENDOR_ID: + *isa_vendor_ext = riscv_isa_vendor_ext_thead; + *count = riscv_isa_vendor_ext_count_thead; + break; + default: + *isa_vendor_ext = NULL; + *count = 0; + found_vendor = false; + break; } + + return found_vendor; } static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct riscv_isainfo *isainfo, - unsigned long *isa2hwcap, const char *isa) + struct riscv_isainfo *isavendorinfo, unsigned long vendorid, + unsigned long *isa2hwcap, const char *isa) { /* * For all possible cpus, we have already validated in @@ -349,8 +384,30 @@ static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct risc const char *ext = isa++; const char *ext_end = isa; bool ext_long = false, ext_err = false; + struct riscv_isainfo *selected_isainfo = isainfo; + const struct riscv_isa_ext_data *selected_riscv_isa_ext = riscv_isa_ext; + size_t selected_riscv_isa_ext_count = riscv_isa_ext_count; + unsigned int id_offset = 0; switch (*ext) { + case 'x': + case 'X': + bool found; + + found = get_isa_vendor_ext(vendorid, + &selected_riscv_isa_ext, + &selected_riscv_isa_ext_count); + selected_isainfo = isavendorinfo; + id_offset = RISCV_ISA_VENDOR_EXT_BASE; + if (!found) { + pr_warn("No associated vendor extensions with vendor id: %lx\n", + vendorid); + for (; *isa && *isa != '_'; ++isa) + ; + ext_err = true; + break; + } + fallthrough; case 's': /* * Workaround for invalid single-letter 's' & 'u' (QEMU). @@ -366,8 +423,6 @@ static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct risc } fallthrough; case 'S': - case 'x': - case 'X': case 'z': case 'Z': /* @@ -476,8 +531,10 @@ static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct risc set_bit(nr, isainfo->isa); } } else { - for (int i = 0; i < riscv_isa_ext_count; i++) - match_isa_ext(&riscv_isa_ext[i], ext, ext_end, isainfo); + for (int i = 0; i < selected_riscv_isa_ext_count; i++) + match_isa_ext(&selected_riscv_isa_ext[i], ext, + ext_end, selected_isainfo, + id_offset); } } } @@ -490,8 +547,8 @@ static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap) struct acpi_table_header *rhct; acpi_status status; unsigned int cpu; - u64 boot_vendorid; - u64 boot_archid; + u64 boot_vendorid = ULL(-1), vendorid; + u64 boot_archid = ULL(-1); if (!acpi_disabled) { status = acpi_get_table(ACPI_SIG_RHCT, 0, &rhct); @@ -499,11 +556,9 @@ static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap) return; } - boot_vendorid = riscv_get_mvendorid(); - boot_archid = riscv_get_marchid(); - for_each_possible_cpu(cpu) { struct riscv_isainfo *isainfo = &hart_isa[cpu]; + struct riscv_isainfo *isavendorinfo = &hart_isa_vendor[cpu]; unsigned long this_hwcap = 0; u64 this_vendorid; u64 this_archid; @@ -523,11 +578,19 @@ static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap) } if (of_property_read_u64(node, "riscv,vendorid", &this_vendorid) < 0) { pr_warn("Unable to find \"riscv,vendorid\" devicetree entry, using boot hart mvendorid instead\n"); + + if (boot_vendorid == -1) + this_vendorid = riscv_get_mvendorid(); + this_vendorid = boot_vendorid; } if (of_property_read_u64(node, "riscv,archid", &this_archid) < 0) { pr_warn("Unable to find \"riscv,vendorid\" devicetree entry, using boot hart marchid instead\n"); + + if (boot_archid == -1) + boot_archid = riscv_get_marchid(); + this_archid = boot_archid; } } else { @@ -540,7 +603,8 @@ static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap) this_archid = boot_archid; } - riscv_parse_isa_string(&this_hwcap, isainfo, isa2hwcap, isa); + riscv_parse_isa_string(&this_hwcap, isainfo, isavendorinfo, + this_vendorid, isa2hwcap, isa); /* * These ones were as they were part of the base ISA when the @@ -582,21 +646,77 @@ static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap) bitmap_copy(riscv_isa, isainfo->isa, RISCV_ISA_EXT_MAX); else bitmap_and(riscv_isa, riscv_isa, isainfo->isa, RISCV_ISA_EXT_MAX); + + /* + * All harts must have the same vendor to have compatible + * vendor extensions. + */ + if (bitmap_empty(riscv_isa_vendor, RISCV_ISA_VENDOR_EXT_SIZE)) { + vendorid = this_vendorid; + bitmap_copy(riscv_isa_vendor, isavendorinfo->isa, + RISCV_ISA_VENDOR_EXT_SIZE); + } else if (vendorid != this_vendorid) { + vendorid = -1ULL; + bitmap_clear(riscv_isa_vendor, 0, RISCV_ISA_VENDOR_EXT_SIZE); + } else { + bitmap_and(riscv_isa_vendor, riscv_isa_vendor, + isavendorinfo->isa, + RISCV_ISA_VENDOR_EXT_SIZE); + } } if (!acpi_disabled && rhct) acpi_put_table((struct acpi_table_header *)rhct); } +static void __init riscv_add_cpu_ext(struct device_node *cpu_node, + unsigned long *this_hwcap, + unsigned long *isa2hwcap, + const struct riscv_isa_ext_data *riscv_isa_ext_data, + struct riscv_isainfo *isainfo, + unsigned int id_offset, + size_t riscv_isa_ext_count) +{ + for (int i = 0; i < riscv_isa_ext_count; i++) { + const struct riscv_isa_ext_data ext = riscv_isa_ext_data[i]; + + if (of_property_match_string(cpu_node, "riscv,isa-extensions", + ext.property) < 0) + continue; + + if (ext.subset_ext_size) { + for (int j = 0; j < ext.subset_ext_size; j++) { + if (riscv_isa_extension_check(ext.subset_ext_ids[j])) + set_bit(ext.subset_ext_ids[j] - id_offset, isainfo->isa); + } + } + + if (riscv_isa_extension_check(ext.id)) { + set_bit(ext.id - id_offset, isainfo->isa); + + /* Only single letter extensions get set in hwcap */ + if (strnlen(ext.name, 2) == 1) + *this_hwcap |= isa2hwcap[ext.id]; + } + } +} + static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap) { unsigned int cpu; + u64 boot_vendorid, vendorid; for_each_possible_cpu(cpu) { unsigned long this_hwcap = 0; struct device_node *cpu_node; struct riscv_isainfo *isainfo = &hart_isa[cpu]; + struct riscv_isainfo *isavendorinfo = &hart_isa_vendor[cpu]; + size_t riscv_isa_vendor_ext_count; + const struct riscv_isa_ext_data *riscv_isa_vendor_ext; + u64 this_vendorid; + bool found_vendor; + cpu_node = of_cpu_device_node_get(cpu); if (!cpu_node) { pr_warn("Unable to find cpu node\n"); @@ -608,28 +728,28 @@ static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap) continue; } - for (int i = 0; i < riscv_isa_ext_count; i++) { - const struct riscv_isa_ext_data *ext = &riscv_isa_ext[i]; + riscv_add_cpu_ext(cpu_node, &this_hwcap, isa2hwcap, + riscv_isa_ext, isainfo, 0, + riscv_isa_ext_count); - if (of_property_match_string(cpu_node, "riscv,isa-extensions", - ext->property) < 0) - continue; - - if (ext->subset_ext_size) { - for (int j = 0; j < ext->subset_ext_size; j++) { - if (riscv_isa_extension_check(ext->subset_ext_ids[j])) - set_bit(ext->subset_ext_ids[j], isainfo->isa); - } - } + if (of_property_read_u64(cpu_node, "riscv,vendorid", &this_vendorid) < 0) { + pr_warn("Unable to find \"riscv,vendorid\" devicetree entry, using boot hart mvendorid instead\n"); + if (boot_vendorid == -1) + boot_vendorid = riscv_get_mvendorid(); + this_vendorid = boot_vendorid; + } - if (riscv_isa_extension_check(ext->id)) { - set_bit(ext->id, isainfo->isa); + found_vendor = get_isa_vendor_ext(this_vendorid, + &riscv_isa_vendor_ext, + &riscv_isa_vendor_ext_count); - /* Only single letter extensions get set in hwcap */ - if (strnlen(riscv_isa_ext[i].name, 2) == 1) - this_hwcap |= isa2hwcap[riscv_isa_ext[i].id]; - } - } + if (found_vendor) + riscv_add_cpu_ext(cpu_node, &this_hwcap, isa2hwcap, + riscv_isa_vendor_ext, isavendorinfo, + RISCV_ISA_VENDOR_EXT_BASE, riscv_isa_vendor_ext_count); + else + pr_warn("No associated vendor extensions with vendor id: %llx\n", + vendorid); of_node_put(cpu_node); @@ -646,6 +766,23 @@ static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap) bitmap_copy(riscv_isa, isainfo->isa, RISCV_ISA_EXT_MAX); else bitmap_and(riscv_isa, riscv_isa, isainfo->isa, RISCV_ISA_EXT_MAX); + + /* + * All harts must have the same vendorid to have compatible + * vendor extensions. + */ + if (bitmap_empty(riscv_isa_vendor, RISCV_ISA_VENDOR_EXT_SIZE)) { + vendorid = this_vendorid; + bitmap_copy(riscv_isa_vendor, isavendorinfo->isa, + RISCV_ISA_VENDOR_EXT_SIZE); + } else if (vendorid != this_vendorid) { + vendorid = -1ULL; + bitmap_clear(riscv_isa_vendor, 0, + RISCV_ISA_VENDOR_EXT_SIZE); + } else { + bitmap_and(riscv_isa_vendor, riscv_isa_vendor, + isavendorinfo->isa, RISCV_ISA_VENDOR_EXT_SIZE); + } } if (bitmap_empty(riscv_isa, RISCV_ISA_EXT_MAX))
Create a private namespace for each vendor above 0x8000. During the probing of hardware capabilities, the vendorid of each hart is used to resolve the vendor extension compatibility. Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> --- arch/riscv/include/asm/cpufeature.h | 7 ++ arch/riscv/include/asm/hwcap.h | 23 ++++ arch/riscv/kernel/cpufeature.c | 203 ++++++++++++++++++++++++++++++------ 3 files changed, 200 insertions(+), 33 deletions(-)