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 8E2B5515886; Wed, 30 Sep 2026 16:46:48 +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=1790786810; cv=none; b=hYeapn+P47RVVzxS366rc6pL+uA21xM37eKhzAcPSbuUYbRa7Mp0eDM97L3q4nk2NfIaDOERccRbwbD2YI9IORPxrBQke1S3+PqibSgAnlHOszMP3/Ifx05HDSYlQhRio2bQX4q6bsWCcs6aY3+vgfbrf6KHWwaPWEp8eTrslmw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790786810; c=relaxed/simple; bh=kKBg+uqDCN6OdagaZrIkjfwPQjhfoAvskrjZJG1JJkU=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=neTs+Tg8S/Rl2TWeRBEEWndwdZovW+kyr02YQDYfUrTxuf6D1YQV4ZobS8+EKa0KG42oPGfc4teewu8ulHW/OcN63ikmVBiu0BiSxRao6YTHQSDdtoClfqAOxOnLELz354hrokhAAtvjX/548zYLMyWzloMtL2Rx7sOxLnPYHSM= 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=phx+TFtw; 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="phx+TFtw" 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 6D509497; Wed, 30 Sep 2026 09:46:44 -0700 (PDT) Received: from [10.2.212.8] (e134344.arm.com [10.2.212.8]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 870883F85F; Wed, 30 Sep 2026 09:46:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=arm.com; s=foss; t=1790786807; bh=kKBg+uqDCN6OdagaZrIkjfwPQjhfoAvskrjZJG1JJkU=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=phx+TFtw3Lpv/kpd9E7P1CLx8cCthWANhS4oyu2MI6X4AXfHlZnOgpbH4f2K8dAUh IFF+6+S6lMCXzTvKn631KWke3gWxqpnMtF8QNqKPEefH6KMFYqzmpm8z4zCbOs1LQv f92CVWZ0KoFBNluIidvmonbCBImNGzPppZrCCKj8= Message-ID: Date: Wed, 30 Sep 2026 17:46:43 +0100 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: Andre Przywara , 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> <0c1797bc-0325-44ad-b43f-83353edf81eb@arm.com> Content-Language: en-US From: Ben Horgan In-Reply-To: <0c1797bc-0325-44ad-b43f-83353edf81eb@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hi Andre, On 30/09/2026 17:41, Andre Przywara wrote: > 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? Thanks :) I do want to clean up the teardown paths in the driver up so that unbind is posible and we're not relying on everything happening at once. That's something for another day though. Cheers, Ben > > 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: >> >