lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <SJ0PR12MB56763459EE6CE776680E0762A0559@SJ0PR12MB5676.namprd12.prod.outlook.com>
Date:   Tue, 27 Sep 2022 03:59:22 +0000
From:   Besar Wicaksono <bwicaksono@...dia.com>
To:     Will Deacon <will@...nel.org>
CC:     "suzuki.poulose@....com" <suzuki.poulose@....com>,
        "robin.murphy@....com" <robin.murphy@....com>,
        "catalin.marinas@....com" <catalin.marinas@....com>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        "sudeep.holla@....com" <sudeep.holla@....com>,
        "thanu.rangarajan@....com" <thanu.rangarajan@....com>,
        "Michael.Williams@....com" <Michael.Williams@....com>,
        Thierry Reding <treding@...dia.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Vikram Sethi <vsethi@...dia.com>,
        "mathieu.poirier@...aro.org" <mathieu.poirier@...aro.org>,
        "mike.leach@...aro.org" <mike.leach@...aro.org>,
        "leo.yan@...aro.org" <leo.yan@...aro.org>
Subject: RE: [PATCH v4 1/2] perf: arm_cspmu: Add support for ARM CoreSight PMU
 driver

Hi Will,

Thanks for the comment. Please see my response inline.

> -----Original Message-----
> From: Will Deacon <will@...nel.org>
> Sent: Thursday, September 22, 2022 8:53 AM
> To: Besar Wicaksono <bwicaksono@...dia.com>
> Cc: suzuki.poulose@....com; robin.murphy@....com;
> catalin.marinas@....com; mark.rutland@....com; linux-arm-
> kernel@...ts.infradead.org; linux-kernel@...r.kernel.org; linux-
> tegra@...r.kernel.org; sudeep.holla@....com;
> thanu.rangarajan@....com; Michael.Williams@....com; Thierry Reding
> <treding@...dia.com>; Jonathan Hunter <jonathanh@...dia.com>; Vikram
> Sethi <vsethi@...dia.com>; mathieu.poirier@...aro.org;
> mike.leach@...aro.org; leo.yan@...aro.org
> Subject: Re: [PATCH v4 1/2] perf: arm_cspmu: Add support for ARM
> CoreSight PMU driver
> 
> External email: Use caution opening links or attachments
> 
> 
> On Sun, Aug 14, 2022 at 01:23:50PM -0500, Besar Wicaksono wrote:
> > Add support for ARM CoreSight PMU driver framework and interfaces.
> > The driver provides generic implementation to operate uncore PMU based
> > on ARM CoreSight PMU architecture. The driver also provides interface
> > to get vendor/implementation specific information, for example event
> > attributes and formating.
> >
> > The specification used in this implementation can be found below:
> >  * ACPI Arm Performance Monitoring Unit table:
> >         https://developer.arm.com/documentation/den0117/latest
> >  * ARM Coresight PMU architecture:
> >         https://developer.arm.com/documentation/ihi0091/latest
> >
> > Signed-off-by: Besar Wicaksono <bwicaksono@...dia.com>
> > ---
> >  arch/arm64/configs/defconfig       |    1 +
> >  drivers/perf/Kconfig               |    2 +
> >  drivers/perf/Makefile              |    1 +
> >  drivers/perf/arm_cspmu/Kconfig     |   13 +
> >  drivers/perf/arm_cspmu/Makefile    |    6 +
> >  drivers/perf/arm_cspmu/arm_cspmu.c | 1262
> ++++++++++++++++++++++++++++
> >  drivers/perf/arm_cspmu/arm_cspmu.h |  151 ++++
> >  7 files changed, 1436 insertions(+)
> >  create mode 100644 drivers/perf/arm_cspmu/Kconfig
> >  create mode 100644 drivers/perf/arm_cspmu/Makefile
> >  create mode 100644 drivers/perf/arm_cspmu/arm_cspmu.c
> >  create mode 100644 drivers/perf/arm_cspmu/arm_cspmu.h
> 
> [...]
> 
> > diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c
> b/drivers/perf/arm_cspmu/arm_cspmu.c
> > new file mode 100644
> > index 000000000000..410876f86eb0
> > --- /dev/null
> > +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> 
> [...]
> 
> > +/*
> > + * Read 64-bit register as a pair of 32-bit registers using hi-lo-hi sequence.
> > + */
> > +static u64 read_reg64_hilohi(const void __iomem *addr)
> > +{
> > +     u32 val_lo, val_hi;
> > +     u64 val;
> > +
> > +     /* Use high-low-high sequence to avoid tearing */
> > +     do {
> > +             val_hi = readl(addr + 4);
> > +             val_lo = readl(addr);
> > +     } while (val_hi != readl(addr + 4));
> 
> Hmm, we probably want a timeout or something in here so we don't lock
> up the CPU if the device goes wonky.
> 

This function is used to read the counter register. The perf driver APIs
(read, stop) that use this function do not return an error code. I am
not sure if we can just break the loop and return 0. Any suggestions ?
Is triggering a panic acceptable ?

> With that, how about adding this a helper to
> include/linux/io-64-nonatomic-*o.h so other folks can reuse it?
> 
> > +/* Check if PMU supports 64-bit single copy atomic. */
> > +static inline bool supports_64bit_atomics(const struct arm_cspmu
> *cspmu)
> > +{
> > +     return CHECK_APMT_FLAG(cspmu->apmt_node->flags, ATOMIC,
> SUPP);
> > +}
> 
> Is this just there because the architecture permits it, or are folks
> actually hanging these things off 32-bit MMIO buses on arm64 SoCs?
> 

Yes, the PMU spec permits a system that needs to break 64-bit access into
a pair of 32-bit accesses.

> > +static int arm_cspmu_request_irq(struct arm_cspmu *cspmu)
> > +{
> > +     int irq, ret;
> > +     struct device *dev;
> > +     struct platform_device *pdev;
> > +     struct acpi_apmt_node *apmt_node;
> > +
> > +     dev = cspmu->dev;
> > +     pdev = to_platform_device(dev);
> > +     apmt_node = cspmu->apmt_node;
> > +
> > +     /* Skip IRQ request if the PMU does not support overflow interrupt. */
> > +     if (apmt_node->ovflw_irq == 0)
> > +             return 0;
> 
> Set PERF_PMU_CAP_NO_INTERRUPT?
> 

Thanks, I will apply it on the next version.

> Will

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ