2024-03-11 08:19:00

by Rex Nie

[permalink] [raw]
Subject: [PATCH] fs/resctrl: Uniform data type of component_id/domid/id/cache_id

This patch uniform data type of component_id/domid/id/cache_id to
u32 to avoid type confusion. According to ACPI for mpam, cache id
is used as locator for cache MSC. Reference to RD_PPTT_CACHE_ID
definition from edk2-platforms, u32 is enough for cache_id.

( \
(((PackageId) & 0xF) << 20) | (((ClusterId) & 0xFF) << 12) | \
(((CoreId) & 0xFF) << 4) | ((CacheType) & 0xF) \
)

refs:
1. ACPI for mpam: https://developer.arm.com/documentation/den0065/latest/
2. RD_PPTT_CACHE_ID from edk2-platforms: https://github.com/tianocore/edk2-platforms/blob/master/Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h#L202

Signed-off-by: Rex Nie <[email protected]>
---
drivers/platform/mpam/mpam_devices.c | 8 ++++----
include/linux/arm_mpam.h | 2 +-
include/linux/resctrl.h | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/mpam/mpam_devices.c b/drivers/platform/mpam/mpam_devices.c
index c2457c36ab12..335ca9e441da 100644
--- a/drivers/platform/mpam/mpam_devices.c
+++ b/drivers/platform/mpam/mpam_devices.c
@@ -205,7 +205,7 @@ int mpam_register_requestor(u16 partid_max, u8 pmg_max)
EXPORT_SYMBOL(mpam_register_requestor);

static struct mpam_component *
-mpam_component_alloc(struct mpam_class *class, int id, gfp_t gfp)
+mpam_component_alloc(struct mpam_class *class, u32 id, gfp_t gfp)
{
struct mpam_component *comp;

@@ -227,7 +227,7 @@ mpam_component_alloc(struct mpam_class *class, int id, gfp_t gfp)
}

static struct mpam_component *
-mpam_component_get(struct mpam_class *class, int id, bool alloc, gfp_t gfp)
+mpam_component_get(struct mpam_class *class, u32 id, bool alloc, gfp_t gfp)
{
struct mpam_component *comp;

@@ -476,7 +476,7 @@ static int mpam_ris_get_affinity(struct mpam_msc *msc, cpumask_t *affinity,

static int mpam_ris_create_locked(struct mpam_msc *msc, u8 ris_idx,
enum mpam_class_types type, u8 class_id,
- int component_id, gfp_t gfp)
+ u32 component_id, gfp_t gfp)
{
int err;
struct mpam_msc_ris *ris;
@@ -525,7 +525,7 @@ static int mpam_ris_create_locked(struct mpam_msc *msc, u8 ris_idx,
}

int mpam_ris_create(struct mpam_msc *msc, u8 ris_idx,
- enum mpam_class_types type, u8 class_id, int component_id)
+ enum mpam_class_types type, u8 class_id, u32 component_id)
{
int err;

diff --git a/include/linux/arm_mpam.h b/include/linux/arm_mpam.h
index d70e4e726fe6..0a4ac76682b5 100644
--- a/include/linux/arm_mpam.h
+++ b/include/linux/arm_mpam.h
@@ -48,7 +48,7 @@ static inline int acpi_mpam_count_msc(void) { return -EINVAL; }
int mpam_register_requestor(u16 partid_max, u8 pmg_max);

int mpam_ris_create(struct mpam_msc *msc, u8 ris_idx,
- enum mpam_class_types type, u8 class_id, int component_id);
+ enum mpam_class_types type, u8 class_id, u32 component_id);

static inline unsigned int resctrl_arch_round_mon_val(unsigned int val)
{
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index dd34523469a5..b00a89addf91 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -108,7 +108,7 @@ struct resctrl_staged_config {
*/
struct rdt_domain {
struct list_head list;
- int id;
+ u32 id;
struct cpumask cpu_mask;
unsigned long *rmid_busy_llc;
struct mbm_state *mbm_total;
--
2.34.1



2024-03-11 17:55:14

by James Morse

[permalink] [raw]
Subject: Re: [PATCH] fs/resctrl: Uniform data type of component_id/domid/id/cache_id

Hi Rex Nie,

(for those following along at home - this is a patch against the MPAM tree, not mainline)

On 11/03/2024 08:18, Rex Nie wrote:
> This patch uniform data type of component_id/domid/id/cache_id to
> u32 to avoid type confusion. According to ACPI for mpam, cache id
> is used as locator for cache MSC. Reference to RD_PPTT_CACHE_ID
> definition from edk2-platforms, u32 is enough for cache_id.
>
> ( \
> (((PackageId) & 0xF) << 20) | (((ClusterId) & 0xFF) << 12) | \
> (((CoreId) & 0xFF) << 4) | ((CacheType) & 0xF) \
> )

Aha, this is where those numbers are coming from! Thanks for digging that out.


> refs:
> 1. ACPI for mpam: https://developer.arm.com/documentation/den0065/latest/
> 2. RD_PPTT_CACHE_ID from edk2-platforms: https://github.com/tianocore/edk2-platforms/blob/master/Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h#L202

Just to check - you don't see any side effects from doing this, its just cleaner.
I agree - today this is only an int because that's what it is in struct rdt_domain.


> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index dd34523469a5..b00a89addf91 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -108,7 +108,7 @@ struct resctrl_staged_config {
> */
> struct rdt_domain {
> struct list_head list;
> - int id;
> + u32 id;
> struct cpumask cpu_mask;
> unsigned long *rmid_busy_llc;
> struct mbm_state *mbm_total;


We should probably only make this change if we clean this up in restrl, not just the MPAM
driver.


I'll pick the MPAM bits of this up for the MPAM tree. This will eventually get merged with
the patch that adds the original code as there is no point preserving the history of code
that isn't merged yet. I'll add you to 'CC' of those patches. (The joke is 'CC' also
stands for Celebrate Contribution!)


Thanks!

James

2024-03-12 02:52:22

by Rex Nie

[permalink] [raw]
Subject: 答复: [PATCH] fs/resctrl: Uniform data type of component_id/domid/id/cache_id

HI James Morse,
Thanks for your reply. Please check my inline reply.
BTW, can I know the progress/roadmap of mpam driver upstream?
Best regards
Rex

> -----邮件原件-----
> 发件人: James Morse <[email protected]>
> 发送时间: 2024年3月12日 1:55
> 收件人: Rex Nie <[email protected]>
> 抄送: [email protected]; [email protected];
> [email protected]; [email protected]
> 主题: Re: [PATCH] fs/resctrl: Uniform data type of
> component_id/domid/id/cache_id
>
> Hi Rex Nie,
>
> (for those following along at home - this is a patch against the MPAM tree, not
> mainline)
>
> On 11/03/2024 08:18, Rex Nie wrote:
> > This patch uniform data type of component_id/domid/id/cache_id to
> > u32 to avoid type confusion. According to ACPI for mpam, cache id is
> > used as locator for cache MSC. Reference to RD_PPTT_CACHE_ID
> > definition from edk2-platforms, u32 is enough for cache_id.
> >
> >
> (
> \
> > (((PackageId) & 0xF) << 20) | (((ClusterId) & 0xFF) << 12) | \
> > (((CoreId) & 0xFF) << 4) | ((CacheType) & 0xF) \
> > )
>
> Aha, this is where those numbers are coming from! Thanks for digging that
> out.
>
>
> > refs:
> > 1. ACPI for mpam:
> > https://developer.arm.com/documentation/den0065/latest/
> > 2. RD_PPTT_CACHE_ID from edk2-platforms:
> > https://github.com/tianocore/edk2-platforms/blob/master/Platform/ARM/S
> > giPkg/Include/SgiAcpiHeader.h#L202
>
> Just to check - you don't see any side effects from doing this, its just cleaner.
> I agree - today this is only an int because that's what it is in struct rdt_domain.
>
>
> > diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h index
> > dd34523469a5..b00a89addf91 100644
> > --- a/include/linux/resctrl.h
> > +++ b/include/linux/resctrl.h
> > @@ -108,7 +108,7 @@ struct resctrl_staged_config {
> > */
> > struct rdt_domain {
> > struct list_head list;
> > - int id;
> > + u32 id;
> > struct cpumask cpu_mask;
> > unsigned long *rmid_busy_llc;
> > struct mbm_state *mbm_total;
>
>
> We should probably only make this change if we clean this up in restrl, not just
> the MPAM driver.
>
I agree. The cleanup should in top of resctrl, considering different platforms
>
> I'll pick the MPAM bits of this up for the MPAM tree. This will eventually get
> merged with the patch that adds the original code as there is no point
> preserving the history of code that isn't merged yet. I'll add you to 'CC' of those
> patches. (The joke is 'CC' also stands for Celebrate Contribution!)
>
Thanks!
>
> Thanks!
>
> James

2024-03-18 17:33:37

by James Morse

[permalink] [raw]
Subject: Re: 答复: [PATCH] fs/resctrl: Uniform da ta type of component_id/domid/id/cache_id

Hi Rex,

On 12/03/2024 02:52, Rex Nie wrote:
> Thanks for your reply. Please check my inline reply.
> BTW, can I know the progress/roadmap of mpam driver upstream?

The series to change the monitor code in a way that allows MPAM to work, and split the
locks has been merged for rc1. I plan to post the next series shortly. Once that is
reviewed and merged the refactoring of resctrl will be finished and I can start posting
the MPAM driver. (it has a few small dependencies on cacheinfo and PPTT parsing code).
I anticipate the MPAM driver will be merged fairly quickly as it won't regress existing
systems.

Any help reviewing the x86 changes would be appreciated - these are the changes that can
affect existing systems. (Shall I CC you on the series?)



Thanks,

James

2024-03-19 03:21:52

by Rex Nie

[permalink] [raw]
Subject: 答复: 答复: [PATCH] fs/resctrl: Uniform d ata type of component_id/domid/id/cache_id


> -----邮件原件-----
> 发件人: James Morse <[email protected]>
> 发送时间: 2024年3月19日 1:33
> 收件人: Rex Nie <[email protected]>
> 抄送: [email protected]; [email protected];
> [email protected]; [email protected]
> 主题: Re: 答复: [PATCH] fs/resctrl: Uniform data type of
> component_id/domid/id/cache_id
>
> Hi Rex,
>
> On 12/03/2024 02:52, Rex Nie wrote:
> > Thanks for your reply. Please check my inline reply.
> > BTW, can I know the progress/roadmap of mpam driver upstream?
>
> The series to change the monitor code in a way that allows MPAM to work,
> and split the locks has been merged for rc1. I plan to post the next series
> shortly. Once that is reviewed and merged the refactoring of resctrl will be
> finished and I can start posting the MPAM driver. (it has a few small
> dependencies on cacheinfo and PPTT parsing code).
> I anticipate the MPAM driver will be merged fairly quickly as it won't regress
> existing systems.
>
> Any help reviewing the x86 changes would be appreciated - these are the
> changes that can affect existing systems. (Shall I CC you on the series?)
>
HI James,
Thanks for showing me the roadmap of mpam upstream and CC me on the patchset series. Mpam is an extremely important feature for our product.
I also Loop angus who is familiar with x86 and arm.

BRs
Rex
>
> Thanks,
>
> James