From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id E6693374A16; Wed, 30 Sep 2026 16:41:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790786497; cv=none; b=BgjtRkgRreswMkypstEmXgjTcie016fkq2RyWx5YQnRbHovJvboibw4w91VdMruZ0uT/57VpfDoWX0JA1MUZeIO6xBJa/E95siUXMK4G4SQdeAbkaWMP/YtcqrAlTk78j2ZlY+xMXpOEy+5YCbj1UbiOb44DcO9gj5FJjKzhgqs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790786497; c=relaxed/simple; bh=zIybdkD3SEiIpHFkwMna4obYsUSyKvnYrWjR1/XxsAI=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=R1H++p7eHUJzaV10KgrvEImkk0ZAMu3tnlRDh8yBjv0/khSdfdLWMIdgktf7Qm9SPVcNe/n013GV6gty3D+TsjY4TsF+87thTKpc0bPzA4NqtDcmw7PRheHklJfPftJMqiMNv+r1mvrK5FeJzlYAH3cYH+fg8wzKlMlRqKDTJdQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b=bywaKIpC; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b="bywaKIpC" Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 68E8B497; Wed, 30 Sep 2026 09:41:30 -0700 (PDT) Received: from [10.57.11.217] (unknown [10.57.11.217]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 521153F85F; Wed, 30 Sep 2026 09:41:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=arm.com; s=foss; t=1790786493; bh=zIybdkD3SEiIpHFkwMna4obYsUSyKvnYrWjR1/XxsAI=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=bywaKIpClqDxFUujRdn49tJkf6bDl2U1HXjFnnn27wkFTcXfLxpdQMvHoMyU5sH8p Pznv0wx7zx8UiQml44Jw7YmE0427PiAqVRhqQ8v4gFJEBFum9yURknlyixlaHgu0Z7 PyfPpdgAoHXdlCH0nFjqGdcL3W4GlODRyVA1Fq/A= Message-ID: <0c1797bc-0325-44ad-b43f-83353edf81eb@arm.com> Date: Wed, 30 Sep 2026 18:41:28 +0200 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v11 13/13] arm_mpam: detect and enable MPAM-Fb PCC support To: Ben Horgan , Lorenzo Pieralisi , Hanjun Guo , Sudeep Holla , Catalin Marinas , Will Deacon , "Rafael J . Wysocki" , Len Brown , James Morse , Reinette Chatre , Fenghua Yu Cc: Jonathan Cameron , Srivathsa L Rao , Ganapatrao Kulkarni , Trilok Soni , Srinivas Ramana , Niyas Sait , Lee Trager , Ritwick Sharma , Gavin Shan , linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org References: <20260924152739.2865510-1-andre.przywara@arm.com> <20260924152739.2865510-14-andre.przywara@arm.com> Content-Language: en-GB From: Andre Przywara In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi, On 9/30/26 16:15, Ben Horgan wrote: > Hi Andre, > > On 24/09/2026 16:27, Andre Przywara wrote: >> The Arm MPAM-Fb specification [1] describes a protocol to access MSC >> registers through a firmware interface. This requires a shared memory >> region to hold the message, and a mailbox to trigger the access. >> For ACPI this is wrapped as a PCC channel, described using existing >> ACPI abstractions. >> >> Add code to parse those PCC table descriptions associated with an MSC, >> and store the parsed information in the MSC struct. >> There can be multiple PCC channels, and each channel can serve multiple >> MSCs, so we need to keep track of the channel usage, using a list and >> a refcount. >> >> This will be used by the MPAM-Fb access wrapper code. >> >> [1] https://developer.arm.com/documentation/den0144/latest >> >> Signed-off-by: Andre Przywara >> Tested-by: Ritwick Sharma # on Arm AGI CPU >> Reviewed-by: Jonathan Cameron >> Tested-by: Srivathsa L Rao >> Reviewed-by: Srivathsa L Rao >> Reviewed-by: Gavin Shan >> --- >> drivers/resctrl/Kconfig | 1 + >> drivers/resctrl/mpam_devices.c | 119 +++++++++++++++++++++++++++++++- >> drivers/resctrl/mpam_fb.c | 41 +++++++++++ >> drivers/resctrl/mpam_internal.h | 2 + >> 4 files changed, 161 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/resctrl/Kconfig b/drivers/resctrl/Kconfig >> index 9591d792736e5..f5ff9b1f6c48e 100644 >> --- a/drivers/resctrl/Kconfig >> +++ b/drivers/resctrl/Kconfig >> @@ -2,6 +2,7 @@ menuconfig ARM64_MPAM_DRIVER >> bool "MPAM driver" >> depends on ARM64 && ARM64_MPAM >> select ACPI_MPAM if ACPI >> + select PCC if ACPI >> select MAILBOX >> help >> Memory System Resource Partitioning and Monitoring (MPAM) driver for >> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c >> index ce20dc44d5d40..ca7e0fe75f24f 100644 >> --- a/drivers/resctrl/mpam_devices.c >> +++ b/drivers/resctrl/mpam_devices.c >> @@ -20,14 +20,19 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> #include >> >> +#include >> +#include >> + >> #include "mpam_internal.h" >> >> /* Values for the T241 errata workaround */ >> @@ -50,6 +55,87 @@ static LIST_HEAD(mpam_all_msc); >> >> struct srcu_struct mpam_srcu; >> >> +/* PCC channels might be serving multiple MSCs, so keep a refcounted list. */ >> +static DEFINE_MUTEX(pcc_chan_list_lock); >> +static LIST_HEAD(pcc_chan_list); >> + >> +static void mpam_pcc_chan_release(struct kref *ref) >> +{ >> + struct mpam_pcc_chan *cur = container_of(ref, struct mpam_pcc_chan, >> + refcount); >> + >> + pcc_mbox_free_channel(cur->pcc_chan); >> + list_del(&cur->pcc_chans); >> + mutex_destroy(&cur->pcc_chan_lock); >> + kfree(cur); >> +} >> + >> +static struct mpam_pcc_chan *mpam_pcc_chan_get(struct device *dev, >> + int subspace_id) >> +{ >> + struct mpam_pcc_chan *cur; >> + >> + guard(mutex)(&pcc_chan_list_lock); >> + >> + list_for_each_entry(cur, &pcc_chan_list, pcc_chans) { >> + if (cur->subspace_id == subspace_id) { >> + kref_get(&cur->refcount); >> + >> + return cur; >> + } >> + } >> + >> + cur = kzalloc_obj(*cur); >> + if (!cur) >> + return ERR_PTR(-ENOMEM); >> + >> + cur->pcc_cl.tx_block = true; >> + >> + cur->pcc_chan = pcc_mbox_request_channel(&cur->pcc_cl, subspace_id); >> + if (IS_ERR(cur->pcc_chan)) { >> + long err = PTR_ERR(cur->pcc_chan); >> + >> + kfree(cur); >> + return ERR_PTR(err); >> + } >> + >> + /* >> + * Timeout based on the "nominal latency" in us, from the >> + * PCC ACPI table. tx_tout is in ms. >> + * Add some margin here to be on the safe side. >> + */ >> + cur->pcc_cl.tx_tout = DIV_ROUND_UP(cur->pcc_chan->latency * 5, 1000); >> + >> + mutex_init(&cur->pcc_chan_lock); >> + >> + cur->subspace_id = subspace_id; >> + kref_init(&cur->refcount); >> + >> + list_add_tail(&cur->pcc_chans, &pcc_chan_list); >> + >> + return cur; >> +} >> + >> +static int mpam_pcc_chan_put(struct mpam_pcc_chan *pcc_chan) >> +{ >> + struct mpam_pcc_chan *cur, *tmp; >> + >> + if (!pcc_chan) >> + return 0; >> + >> + guard(mutex)(&pcc_chan_list_lock); >> + >> + list_for_each_entry_safe(cur, tmp, &pcc_chan_list, pcc_chans) { >> + if (cur == pcc_chan) { >> + kref_put(&cur->refcount, mpam_pcc_chan_release); >> + >> + return 0; >> + } >> + } >> + >> + return -ENOENT; >> +} >> + >> /* >> * Number of MSCs that have been probed. Once all MSCs have been probed MPAM >> * can be enabled. >> @@ -2272,6 +2358,8 @@ static void mpam_msc_destroy(struct mpam_msc *msc) >> list_for_each_entry_safe(ris, tmp, &msc->ris, msc_list) >> mpam_ris_destroy(ris); >> >> + mpam_pcc_chan_put(msc->pcc_chan); >> + > > Last one from Sashiko [1] and I haven't spotted anything else either in the patches or from giving > it a go on the arm agi cpu. :) > > Does the mpam_pcc_chan_put() need to go after the list_del_rcu() and wait for a srcu synchronization > to avioid a race between iterating through the list while acessing the msc and tearing down the channel. I wonder if we can really do any MPAM-Fb requests after mpam_disable() has been called, because that function first thing clears the static branch. But regardless, I swapped mpam_pcc_chan_put() and list_del_rcu(), and also added a synchronize_rcu() call inbetween. Not sure how far we want to go here, though, seems like a very pathological case to me? Cheers, Andre > >> list_del_rcu(&msc->all_msc_list); >> platform_set_drvdata(pdev, NULL); >> >> @@ -2295,7 +2383,7 @@ static void mpam_msc_drv_remove(struct platform_device *pdev) >> static struct mpam_msc *do_mpam_msc_drv_probe(struct platform_device *pdev) >> { >> int err; >> - u32 tmp; >> + u32 pcc_subspace_id; >> struct mpam_msc *msc; >> struct resource *msc_res; >> struct device *dev = &pdev->dev; >> @@ -2339,7 +2427,7 @@ static struct mpam_msc *do_mpam_msc_drv_probe(struct platform_device *pdev) >> if (err) >> return ERR_PTR(err); >> >> - if (device_property_read_u32(&pdev->dev, "pcc-channel", &tmp)) >> + if (device_property_read_u32(dev, "pcc-channel", &pcc_subspace_id)) >> msc->iface = MPAM_IFACE_MMIO; >> else >> msc->iface = MPAM_IFACE_PCC; >> @@ -2360,6 +2448,33 @@ static struct mpam_msc *do_mpam_msc_drv_probe(struct platform_device *pdev) >> } >> msc->mapped_hwpage_sz = msc_res->end - msc_res->start; >> msc->mapped_hwpage = io; >> + } else if (msc->iface == MPAM_IFACE_PCC) { >> + u32 mpam_fb_msc_id; >> + >> + msc->pcc_chan = mpam_pcc_chan_get(dev, pcc_subspace_id); >> + if (IS_ERR(msc->pcc_chan)) { >> + pr_err("Failed to request MSC PCC channel\n"); >> + return ERR_CAST(msc->pcc_chan); >> + } >> + >> + err = mpam_fb_check_shared_buffer_size(msc); >> + if (err) { >> + mpam_pcc_chan_put(msc->pcc_chan); >> + >> + return ERR_PTR(err); >> + } >> + >> + err = mpam_fb_check_protocol_version(msc); >> + if (err) { >> + mpam_pcc_chan_put(msc->pcc_chan); >> + >> + return ERR_PTR(err); >> + } >> + >> + if (device_property_read_u32(&pdev->dev, "mpam-fb-msc-id", >> + &mpam_fb_msc_id)) >> + mpam_fb_msc_id = msc->id; >> + msc->fb_id = mpam_fb_msc_id; >> } else { >> return ERR_PTR(-EINVAL); >> } >> diff --git a/drivers/resctrl/mpam_fb.c b/drivers/resctrl/mpam_fb.c >> index 8298fcedd2be7..d90888944abbd 100644 >> --- a/drivers/resctrl/mpam_fb.c >> +++ b/drivers/resctrl/mpam_fb.c >> @@ -20,10 +20,15 @@ >> #define MPAM_FB_PROTOCOL_ID 0x1a >> >> #define MPAM_PROTOCOL_VERSION_CMD 0x0 >> +#define MPAM_FB_VERSION_MAJOR_MASK GENMASK_U32(31, 16) >> +#define MPAM_FB_VERSION_MINOR_MASK GENMASK_U32(15, 0) >> + >> #define MPAM_MSC_READ_CMD 0x4 >> #define MPAM_MSC_WRITE_CMD 0x5 >> >> #define MPAM_FB_PROT_HEADER_LEN sizeof(u32) >> +/* The longest message is MPAM_MSC_WRITE, with 4 parameters. */ >> +#define MPAM_FB_MAX_MSG_SIZE (4 * sizeof(u32)) >> >> #define MPAM_FB_SUCCESS 0 >> #define MPAM_FB_ERR_NOT_SUPPORTED -1 >> @@ -210,3 +215,39 @@ int mpam_fb_send_write_request(struct mpam_msc *msc, u16 reg, u32 value) >> return mpam_fb_send_request(msc, msc->fb_id, reg, &value, >> MPAM_MSC_WRITE_CMD); >> } >> + >> +/* We only support MPAM-Fb protocol version 1.x */ >> +int mpam_fb_check_protocol_version(struct mpam_msc *msc) >> +{ >> + u32 version; >> + int ret; >> + >> + ret = mpam_fb_send_request(msc, 0, 0, &version, >> + MPAM_PROTOCOL_VERSION_CMD); >> + if (ret) >> + return ret; >> + >> + if (FIELD_GET(MPAM_FB_VERSION_MAJOR_MASK, version) != 1) { >> + pr_err("Incompatible MPAM-Fb protocol version %d.%d\n", >> + FIELD_GET(MPAM_FB_VERSION_MAJOR_MASK, version), >> + FIELD_GET(MPAM_FB_VERSION_MINOR_MASK, version)); >> + >> + return -EINVAL; >> + } >> + >> + return 0; >> +} >> + >> +int mpam_fb_check_shared_buffer_size(struct mpam_msc *msc) >> +{ >> + int min_buffer_size = MPAM_FB_MAX_MSG_SIZE + >> + sizeof(struct acpi_pcct_ext_pcc_shared_memory); >> + >> + if (msc->pcc_chan->pcc_chan->shmem_size < min_buffer_size) { >> + pr_err("MPAM-Fb PCC channel size too small.\n"); >> + >> + return -ENOMEM; >> + } >> + >> + return 0; >> +} >> diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h >> index 99057d6abee1e..ee7c3953a9b6c 100644 >> --- a/drivers/resctrl/mpam_internal.h >> +++ b/drivers/resctrl/mpam_internal.h >> @@ -538,6 +538,8 @@ static inline void mpam_resctrl_teardown_class(struct mpam_class *class) { } >> /* MPAM-Fb Firmware-backed protocol wrappers */ >> int mpam_fb_send_read_request(struct mpam_msc *msc, u16 reg, u32 *result); >> int mpam_fb_send_write_request(struct mpam_msc *msc, u16 reg, u32 value); >> +int mpam_fb_check_protocol_version(struct mpam_msc *msc); >> +int mpam_fb_check_shared_buffer_size(struct mpam_msc *msc); >> >> /* >> * MPAM MSCs have the following register layout. See: >