mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Will Deacon <will@kernel.org>
To: "liwei (GF)" <liwei391@huawei.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Anshuman Khandual <anshuman.khandual@arm.com>,
	Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Marc Zyngier <maz@kernel.org>,
	Ionela Voinescu <ionela.voinescu@arm.com>,
	Ard Biesheuvel <ardb@kernel.org>,
	Amit Daniel Kachhap <amit.kachhap@arm.com>,
	Vladimir Murzin <vladimir.murzin@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, guohanjun@huawei.com
Subject: Re: [PATCH v3] drivers/perf: Add support for ARMv8.3-SPE
Date: Thu, 3 Dec 2020 09:19:22 +0000	[thread overview]
Message-ID: <20201203091922.GA30817@willie-the-truck> (raw)
In-Reply-To: <47201e85-7cf3-7c94-d1be-9c83ef8c3416@huawei.com>

Hi,

On Thu, Dec 03, 2020 at 04:42:20PM +0800, liwei (GF) wrote:
> On 2020/11/30 18:06, Will Deacon wrote:
> > On Fri, Nov 27, 2020 at 02:03:22PM +0800, Wei Li wrote:
> >>  /* Perf callbacks */
> >>  static int arm_spe_pmu_event_init(struct perf_event *event)
> >>  {
> >> @@ -670,7 +686,7 @@ static int arm_spe_pmu_event_init(struct perf_event *event)
> >>  	    !cpumask_test_cpu(event->cpu, &spe_pmu->supported_cpus))
> >>  		return -ENOENT;
> >>  
> >> -	if (arm_spe_event_to_pmsevfr(event) & SYS_PMSEVFR_EL1_RES0)
> >> +	if (arm_spe_event_to_pmsevfr(event) & arm_spe_pmsevfr_res0(spe_pmu->pmsver))
> >>  		return -EOPNOTSUPP;
> >>  
> >>  	if (attr->exclude_idle)
> >> @@ -937,6 +953,7 @@ static void __arm_spe_pmu_dev_probe(void *info)
> >>  			fld, smp_processor_id());
> >>  		return;
> >>  	}
> >> +	spe_pmu->pmsver = (u16)fld;
> > 
> > ... which also means we should clamp this value now that we expose it to
> > userspace. Otherwise, userspace can't rely on this field for anything.
> > 
> > That said -- please can you tell me what userspace intends to do with
> > this version number?
> > 
> 
> In fact, it is only used in our testcase for now, which needs to know the real version the
> chip has implemented, as these is no other way to get the info now. So i added it in the
> probe message in v1 at first.

If it's not needed, then let's not expose it yet. ABIs aren't cheap!

> Especially we use the of_device_id "arm,statistical-profiling-extension-v1" and the
> platform_device_id "arm,spe-v1". It's a little weird to ARMv8.3-SPE.
> 

What exactly breaks? It looks like it should work fine to me, but I don't
have any SPE systems to test on. The "v1" in the compatible string doesn't
really do anything at the moment because we can probe everything other than
the interrupt topology at runtime, so I wouldn't get hung up on that.

> Further more i am wondering if we need to add the 'spe-v2' device_id. If
> not, i think it's broken on a big.LITTLE system [1] as we can register
> only one device and the driver will work incorrectly if the big and LITTLE
> cores have different version of SPE.

This is only an issue for ACPI, right? I don't think that was ever intended
to work with big.LITTLE. See:

	pr_warn("ACPI: SPE must be homogeneous\n");

DT should handle it though.

Anyway, please can you post a new version of this patch without the ABI
changes? In other words, just including the RES0 part.

Thanks,

Will

      reply	other threads:[~2020-12-03  9:20 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-27  6:03 Wei Li
2020-11-27 12:03 ` Suzuki K Poulose
2020-11-30  9:55   ` Will Deacon
2020-11-30 10:06 ` Will Deacon
2020-12-03  8:42   ` liwei (GF)
2020-12-03  9:19     ` Will Deacon [this message]

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=20201203091922.GA30817@willie-the-truck \
    --to=will@kernel.org \
    --cc=amit.kachhap@arm.com \
    --cc=anshuman.khandual@arm.com \
    --cc=ardb@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=guohanjun@huawei.com \
    --cc=ionela.voinescu@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liwei391@huawei.com \
    --cc=mark.rutland@arm.com \
    --cc=maz@kernel.org \
    --cc=suzuki.poulose@arm.com \
    --cc=vincenzo.frascino@arm.com \
    --cc=vladimir.murzin@arm.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®