From: Dan Carpenter <dan.carpenter@linaro.org>
To: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: platform-driver-x86@vger.kernel.org
Subject: [bug report] platform/x86/intel-uncore-freq: Add attributes to show die_id
Date: Fri, 6 Jun 2025 12:04:24 +0300 [thread overview]
Message-ID: <aEKvGCLd1qmX04Tc@stanley.mountain> (raw)
Hello Srinivas Pandruvada,
Commit 247b43fcd872 ("platform/x86/intel-uncore-freq: Add attributes
to show die_id") from May 8, 2025 (linux-next), leads to the
following Smatch static checker warning:
drivers/platform/x86/intel/uncore-frequency/uncore-frequency-tpmi.c:607 uncore_probe()
error: we previously assumed 'plat_info' could be null (see line 514)
drivers/platform/x86/intel/uncore-frequency/uncore-frequency-tpmi.c
465 static int uncore_probe(struct auxiliary_device *auxdev, const struct auxiliary_device_id *id)
466 {
467 bool read_blocked = 0, write_blocked = 0;
468 struct intel_tpmi_plat_info *plat_info;
469 struct tpmi_uncore_struct *tpmi_uncore;
470 bool uncore_sysfs_added = false;
471 int ret, i, pkg = 0;
472 int num_resources;
473
474 ret = tpmi_get_feature_status(auxdev, TPMI_ID_UNCORE, &read_blocked, &write_blocked);
475 if (ret)
476 dev_info(&auxdev->dev, "Can't read feature status: ignoring blocked status\n");
477
478 if (read_blocked) {
479 dev_info(&auxdev->dev, "Firmware has blocked reads, exiting\n");
480 return -ENODEV;
481 }
482
483 /* Get number of power domains, which is equal to number of resources */
484 num_resources = tpmi_get_resource_count(auxdev);
485 if (!num_resources)
486 return -EINVAL;
487
488 /* Register callbacks to uncore core */
489 ret = uncore_freq_common_init(uncore_read, uncore_write);
490 if (ret)
491 return ret;
492
493 /* Allocate uncore instance per package */
494 tpmi_uncore = devm_kzalloc(&auxdev->dev, sizeof(*tpmi_uncore), GFP_KERNEL);
495 if (!tpmi_uncore) {
496 ret = -ENOMEM;
497 goto err_rem_common;
498 }
499
500 /* Allocate memory for all power domains in a package */
501 tpmi_uncore->pd_info = devm_kcalloc(&auxdev->dev, num_resources,
502 sizeof(*tpmi_uncore->pd_info),
503 GFP_KERNEL);
504 if (!tpmi_uncore->pd_info) {
505 ret = -ENOMEM;
506 goto err_rem_common;
507 }
508
509 tpmi_uncore->power_domain_count = num_resources;
510 tpmi_uncore->write_blocked = write_blocked;
511
512 /* Get the package ID from the TPMI core */
513 plat_info = tpmi_get_platform_data(auxdev);
514 if (plat_info)
The old code assumes plat_info can be NULL.
515 pkg = plat_info->package_id;
516 else
517 dev_info(&auxdev->dev, "Platform information is NULL\n");
518
519 for (i = 0; i < num_resources; ++i) {
520 struct tpmi_uncore_power_domain_info *pd_info;
521 struct resource *res;
522 u64 cluster_offset;
523 u8 cluster_mask;
524 int mask, j;
525 u64 header;
526
527 res = tpmi_get_resource_at_index(auxdev, i);
528 if (!res)
529 continue;
530
531 pd_info = &tpmi_uncore->pd_info[i];
532
533 pd_info->uncore_base = devm_ioremap_resource(&auxdev->dev, res);
534 if (IS_ERR(pd_info->uncore_base)) {
535 ret = PTR_ERR(pd_info->uncore_base);
536 /*
537 * Set to NULL so that clean up can still remove other
538 * entries already created if any by
539 * remove_cluster_entries()
540 */
541 pd_info->uncore_base = NULL;
542 goto remove_clusters;
543 }
544
545 /* Check for version and skip this resource if there is mismatch */
546 header = readq(pd_info->uncore_base);
547 pd_info->ufs_header_ver = header & UNCORE_VERSION_MASK;
548
549 if (pd_info->ufs_header_ver == TPMI_VERSION_INVALID)
550 continue;
551
552 if (TPMI_MAJOR_VERSION(pd_info->ufs_header_ver) != UNCORE_MAJOR_VERSION) {
553 dev_err(&auxdev->dev, "Uncore: Unsupported major version:%lx\n",
554 TPMI_MAJOR_VERSION(pd_info->ufs_header_ver));
555 ret = -ENODEV;
556 goto remove_clusters;
557 }
558
559 if (TPMI_MINOR_VERSION(pd_info->ufs_header_ver) > UNCORE_MINOR_VERSION)
560 dev_info(&auxdev->dev, "Uncore: Ignore: Unsupported minor version:%lx\n",
561 TPMI_MINOR_VERSION(pd_info->ufs_header_ver));
562
563 /* Get Cluster ID Mask */
564 cluster_mask = FIELD_GET(UNCORE_LOCAL_FABRIC_CLUSTER_ID_MASK, header);
565 if (!cluster_mask) {
566 dev_info(&auxdev->dev, "Uncore: Invalid cluster mask:%x\n", cluster_mask);
567 continue;
568 }
569
570 /* Find out number of clusters in this resource */
571 pd_info->cluster_count = hweight8(cluster_mask);
572
573 pd_info->cluster_infos = devm_kcalloc(&auxdev->dev, pd_info->cluster_count,
574 sizeof(struct tpmi_uncore_cluster_info),
575 GFP_KERNEL);
576 if (!pd_info->cluster_infos) {
577 ret = -ENOMEM;
578 goto remove_clusters;
579 }
580 /*
581 * Each byte in the register point to status and control
582 * registers belonging to cluster id 0-8.
583 */
584 cluster_offset = readq(pd_info->uncore_base +
585 UNCORE_FABRIC_CLUSTER_OFFSET);
586
587 for (j = 0; j < pd_info->cluster_count; ++j) {
588 struct tpmi_uncore_cluster_info *cluster_info;
589
590 /* Get the offset for this cluster */
591 mask = (cluster_offset & UNCORE_CLUSTER_OFF_MASK);
592 /* Offset in QWORD, so change to bytes */
593 mask <<= 3;
594
595 cluster_info = &pd_info->cluster_infos[j];
596
597 cluster_info->cluster_base = pd_info->uncore_base + mask;
598
599 uncore_set_agent_type(cluster_info);
600
601 cluster_info->uncore_data.package_id = pkg;
602 /* There are no dies like Cascade Lake */
603 cluster_info->uncore_data.die_id = 0;
604 cluster_info->uncore_data.domain_id = i;
605 cluster_info->uncore_data.cluster_id = j;
606
--> 607 set_cdie_id(i, cluster_info, plat_info);
^^^^^^^^^
But the patch adds an unchecked dereference.
608
609 cluster_info->uncore_root = tpmi_uncore;
610
611 if (TPMI_MINOR_VERSION(pd_info->ufs_header_ver) >= UNCORE_ELC_SUPPORTED_VERSION)
612 cluster_info->elc_supported = true;
613
614 ret = uncore_freq_add_entry(&cluster_info->uncore_data, 0);
615 if (ret) {
616 cluster_info->cluster_base = NULL;
617 goto remove_clusters;
618 }
619 /* Point to next cluster offset */
620 cluster_offset >>= UNCORE_MAX_CLUSTER_PER_DOMAIN;
621 uncore_sysfs_added = true;
622 }
623 }
624
625 if (!uncore_sysfs_added) {
626 ret = -ENODEV;
627 goto remove_clusters;
628 }
629
630 auxiliary_set_drvdata(auxdev, tpmi_uncore);
631
632 if (topology_max_dies_per_package() > 1)
633 return 0;
634
635 tpmi_uncore->root_cluster.root_domain = true;
636 tpmi_uncore->root_cluster.uncore_root = tpmi_uncore;
637
638 tpmi_uncore->root_cluster.uncore_data.package_id = pkg;
639 tpmi_uncore->root_cluster.uncore_data.domain_id = UNCORE_DOMAIN_ID_INVALID;
640 ret = uncore_freq_add_entry(&tpmi_uncore->root_cluster.uncore_data, 0);
641 if (ret)
642 goto remove_clusters;
643
644 return 0;
645
646 remove_clusters:
647 remove_cluster_entries(tpmi_uncore);
648 err_rem_common:
649 uncore_freq_common_exit();
650
651 return ret;
652 }
regards,
dan carpenter
reply other threads:[~2025-06-06 9:04 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aEKvGCLd1qmX04Tc@stanley.mountain \
--to=dan.carpenter@linaro.org \
--cc=platform-driver-x86@vger.kernel.org \
--cc=srinivas.pandruvada@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.