* [bug report] perf/x86: Add Intel Tiger Lake uncore support
@ 2022-01-25 7:43 Dan Carpenter
2022-01-25 13:32 ` Liang, Kan
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2022-01-25 7:43 UTC (permalink / raw)
To: kan.liang, linux-perf-users
Hello Kan Liang,
The patch fdb64822443e: "perf/x86: Add Intel Tiger Lake uncore
support" from Feb 6, 2020, leads to the following Smatch static
checker warning:
arch/x86/events/intel/uncore_snb.c:1486 __uncore_imc_init_box()
warn: cast after binop
arch/x86/events/intel/uncore_snb.c
1465 static void __uncore_imc_init_box(struct intel_uncore_box *box,
1466 unsigned int base_offset)
1467 {
1468 struct pci_dev *pdev = tgl_uncore_get_mc_dev();
1469 struct intel_uncore_pmu *pmu = box->pmu;
1470 struct intel_uncore_type *type = pmu->type;
1471 resource_size_t addr;
1472 u32 mch_bar;
1473
1474 if (!pdev) {
1475 pr_warn("perf uncore: Cannot find matched IMC device.\n");
1476 return;
1477 }
1478
1479 pci_read_config_dword(pdev, SNB_UNCORE_PCI_IMC_BAR_OFFSET, &mch_bar);
1480 /* MCHBAR is disabled */
1481 if (!(mch_bar & BIT(0))) {
1482 pr_warn("perf uncore: MCHBAR is disabled. Failed to map IMC free-running counters.\n");
1483 return;
1484 }
1485 mch_bar &= ~BIT(0);
--> 1486 addr = (resource_size_t)(mch_bar + TGL_UNCORE_MMIO_IMC_MEM_OFFSET * pmu->pmu_idx);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
All of these are 32 bit values. If we really need a 64bit type, these
will overflow before the cast to resource_size_t. This code is 2 years
old, so I suspect the cast is only required on line 1490 and that one is
correct.
1487
1488 #ifdef CONFIG_PHYS_ADDR_T_64BIT
1489 pci_read_config_dword(pdev, SNB_UNCORE_PCI_IMC_BAR_OFFSET + 4, &mch_bar);
1490 addr |= ((resource_size_t)mch_bar << 32);
1491 #endif
1492
1493 addr += base_offset;
1494 box->io_addr = ioremap(addr, type->mmio_map_size);
1495 if (!box->io_addr)
1496 pr_warn("perf uncore: Failed to ioremap for %s.\n", type->name);
1497 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [bug report] perf/x86: Add Intel Tiger Lake uncore support
2022-01-25 7:43 [bug report] perf/x86: Add Intel Tiger Lake uncore support Dan Carpenter
@ 2022-01-25 13:32 ` Liang, Kan
0 siblings, 0 replies; 2+ messages in thread
From: Liang, Kan @ 2022-01-25 13:32 UTC (permalink / raw)
To: Dan Carpenter, linux-perf-users
Hi Dan
On 1/25/2022 2:43 AM, Dan Carpenter wrote:
> Hello Kan Liang,
>
> The patch fdb64822443e: "perf/x86: Add Intel Tiger Lake uncore
> support" from Feb 6, 2020, leads to the following Smatch static
> checker warning:
>
> arch/x86/events/intel/uncore_snb.c:1486 __uncore_imc_init_box()
> warn: cast after binop
>
> arch/x86/events/intel/uncore_snb.c
> 1465 static void __uncore_imc_init_box(struct intel_uncore_box *box,
> 1466 unsigned int base_offset)
> 1467 {
> 1468 struct pci_dev *pdev = tgl_uncore_get_mc_dev();
> 1469 struct intel_uncore_pmu *pmu = box->pmu;
> 1470 struct intel_uncore_type *type = pmu->type;
> 1471 resource_size_t addr;
> 1472 u32 mch_bar;
> 1473
> 1474 if (!pdev) {
> 1475 pr_warn("perf uncore: Cannot find matched IMC device.\n");
> 1476 return;
> 1477 }
> 1478
> 1479 pci_read_config_dword(pdev, SNB_UNCORE_PCI_IMC_BAR_OFFSET, &mch_bar);
> 1480 /* MCHBAR is disabled */
> 1481 if (!(mch_bar & BIT(0))) {
> 1482 pr_warn("perf uncore: MCHBAR is disabled. Failed to map IMC free-running counters.\n");
> 1483 return;
> 1484 }
> 1485 mch_bar &= ~BIT(0);
> --> 1486 addr = (resource_size_t)(mch_bar + TGL_UNCORE_MMIO_IMC_MEM_OFFSET * pmu->pmu_idx);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> All of these are 32 bit values. If we really need a 64bit type, these
> will overflow before the cast to resource_size_t. This code is 2 years
> old, so I suspect the cast is only required on line 1490 and that one is
> correct.
>
Thanks for the report.
For 32bit system, I'm not sure whether it's OK to remove the
resource_size_t for 1486?
Shouldn't we explicitly cast to resource_size_t for the possible
overflow part for a 32bit system as below?
diff --git a/arch/x86/events/intel/uncore_snb.c
b/arch/x86/events/intel/uncore_snb.c
index 36b6c20..fc6cfcb 100644
--- a/arch/x86/events/intel/uncore_snb.c
+++ b/arch/x86/events/intel/uncore_snb.c
@@ -1720,7 +1720,7 @@ static void __uncore_imc_init_box(struct
intel_uncore_box *box,
return;
}
mch_bar &= ~BIT(0);
- addr = (resource_size_t)(mch_bar +
TGL_UNCORE_MMIO_IMC_MEM_OFFSET * pmu->pmu_idx);
+ addr = mch_bar +
(resource_size_t)(TGL_UNCORE_MMIO_IMC_MEM_OFFSET * pmu->pmu_idx);
#ifdef CONFIG_PHYS_ADDR_T_64BIT
pci_read_config_dword(pdev, SNB_UNCORE_PCI_IMC_BAR_OFFSET + 4,
&mch_bar);
Thanks,
Kan
> 1487
> 1488 #ifdef CONFIG_PHYS_ADDR_T_64BIT
> 1489 pci_read_config_dword(pdev, SNB_UNCORE_PCI_IMC_BAR_OFFSET + 4, &mch_bar);
> 1490 addr |= ((resource_size_t)mch_bar << 32);
> 1491 #endif
> 1492
> 1493 addr += base_offset;
> 1494 box->io_addr = ioremap(addr, type->mmio_map_size);
> 1495 if (!box->io_addr)
> 1496 pr_warn("perf uncore: Failed to ioremap for %s.\n", type->name);
> 1497 }
>
> regards,
> dan carpenter
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-01-25 13:35 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-25 7:43 [bug report] perf/x86: Add Intel Tiger Lake uncore support Dan Carpenter
2022-01-25 13:32 ` Liang, Kan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).