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: <ZP6ADIAqZn0kkxwB@APC323>
Date:   Mon, 11 Sep 2023 10:48:44 +0800
From:   Yu-Chien Peter Lin <peterlin@...estech.com>
To:     Conor Dooley <conor@...nel.org>
CC:     <linux-riscv@...ts.infradead.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>, <linux-perf-users@...r.kernel.org>,
        <paul.walmsley@...ive.com>, <palmer@...belt.com>,
        <aou@...s.berkeley.edu>, <conor.dooley@...rochip.com>,
        <atishp@...shpatra.org>, <anup@...infault.org>,
        <prabhakar.mahadev-lad.rj@...renesas.com>,
        <ajones@...tanamicro.com>, <heiko@...ech.de>,
        <samuel@...lland.org>, <geert+renesas@...der.be>,
        <n.shubin@...ro.com>, <dminus@...estech.com>,
        <ycliang@...estech.com>, <tim609@...estech.com>,
        <locus84@...estech.com>, <dylan@...estech.com>
Subject: Re: [PATCH 3/4] riscv: errata: Add Andes PMU errata

On Thu, Sep 07, 2023 at 12:02:46PM +0100, Conor Dooley wrote:
> On Thu, Sep 07, 2023 at 10:27:03AM +0100, Conor Dooley wrote:
> > Hey,
> > 
> > On Thu, Sep 07, 2023 at 10:16:34AM +0800, Yu Chien Peter Lin wrote:
> > > Before the ratification of Sscofpmf, the Andes PMU extension
> > > implements the same mechanism and is compatible with existing
> > > SBI PMU driver of perf to support event sampling and mode
> > > filtering with programmable hardware performance counters.
> > 
> > If it actually was, you'd not need to modify the driver ;)
> > 
> > > This patch adds PMU support for Andes 45-series CPUs by
> > > introducing a CPU errata.
> > 
> > I don't really understand this in all honesty. You don't have Sscofpmf
> > support with a bug, you have something that is Sscofpmf-adjactent that
> > predates it. Why claim to support an extension that you do not, only to
> > have to come along and try to clean things up afterwards, instead of
> > accurately declaring what you do support from the outset?
> 
> Reading this again, I don't think that I have been particularly clear,
> sorry. My point is that this is not a fix for a bug in your
> implementation of Sscofpmf, but rather adding probing for what is
> effectively a custom ISA extension that happens to be very similar to
> the standard one. As it is not an implementation bug, errata should
> not be abused to support vendor extensions, and either DT or ACPI should
> be used to inform the operating system about its presence.
> 
> Cheers,
> Conor.
> 
> > 
> > (and just because someone already got away with it, doesn't mean that
> > you get a free pass on it, sorry)

Apologize for any confusion caused by the name. I thought it would make it
easier to find the related functions and files in OpenSBI, didn't expect
that it may have misled people to abuse the use of errata, you are right,
I should have chosen my words more carefully.

In my opinion, this is simply a pre-sepc solution to enable missing perf
features before the standard is finalized. The underlying logic remains the
same, so we can still use the errata to patch a few lines of CSR accesses
and align with other vendors. This way, we can make minimal changes and
avoid performance overhead to the driver.

> > Thanks,
> > Conor.
> > 
> > > Signed-off-by: Yu Chien Peter Lin <peterlin@...estech.com>
> > > Signed-off-by: Locus Wei-Han Chen <locus84@...estech.com>
> > 
> > btw, what did Locus Wei-Han Chen do here? Are you missing
> > a Co-developed-by: tag?

Yes I missed the CD-tag, will fix it.
Thanks for the review.

> > > Reviewed-by: Charles Ci-Jyun Wu <dminus@...estech.com>
> > > Reviewed-by: Leo Yu-Chi Liang <ycliang@...estech.com>
> > > ---
> > >  arch/riscv/Kconfig.errata            | 13 ++++++++
> > >  arch/riscv/errata/andes/errata.c     | 45 +++++++++++++++++++++++++++-
> > >  arch/riscv/include/asm/errata_list.h | 43 ++++++++++++++++++++++++--
> > >  drivers/perf/riscv_pmu_sbi.c         | 20 +++++++++----
> > >  4 files changed, 111 insertions(+), 10 deletions(-)

Best regards,
Peter Lin

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ