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: <IA1PR11MB6097ED9919B408116EFE161E9BC69@IA1PR11MB6097.namprd11.prod.outlook.com>
Date:   Tue, 17 Jan 2023 18:29:18 +0000
From:   "Yu, Fenghua" <fenghua.yu@...el.com>
To:     James Morse <james.morse@....com>,
        "x86@...nel.org" <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     "Chatre, Reinette" <reinette.chatre@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        H Peter Anvin <hpa@...or.com>,
        Babu Moger <Babu.Moger@....com>,
        "shameerali.kolothum.thodi@...wei.com" 
        <shameerali.kolothum.thodi@...wei.com>,
        D Scott Phillips OS <scott@...amperecomputing.com>,
        "carl@...amperecomputing.com" <carl@...amperecomputing.com>,
        "lcherian@...vell.com" <lcherian@...vell.com>,
        "bobo.shaobowang@...wei.com" <bobo.shaobowang@...wei.com>,
        "tan.shaopeng@...itsu.com" <tan.shaopeng@...itsu.com>,
        "xingxin.hx@...nanolis.org" <xingxin.hx@...nanolis.org>,
        "baolin.wang@...ux.alibaba.com" <baolin.wang@...ux.alibaba.com>,
        Jamie Iles <quic_jiles@...cinc.com>,
        Xin Hao <xhao@...ux.alibaba.com>,
        "peternewman@...gle.com" <peternewman@...gle.com>
Subject: RE: [PATCH v2 08/18] x86/resctrl: Queue mon_event_read() instead of
 sending an IPI

Hi, James,

> x86 is blessed with an abundance of monitors, one per RMID, that can be read
> from any CPU in the domain. MPAMs monitors reside in the MMIO MSC, the
> number implemented is up to the manufacturer. This means when there are
> fewer monitors than needed, they need to be allocated and freed.
> 
> Worse, the domain may be broken up into slices, and the MMIO accesses for
> each slice may need performing from different CPUs.
> 
> These two details mean MPAMs monitor code needs to be able to sleep, and IPI
> another CPU in the domain to read from a resource that has been sliced.
> 
> mon_event_read() already invokes mon_event_count() via IPI, which means this
> isn't possible.
> 
> Change mon_event_read() to schedule mon_event_count() on a remote CPU
> and wait, instead of sending an IPI. This function is only used in response to a
> user-space filesystem request (not the timing sensitive overflow code).

But mkdir mon group needs mon_event_count() to reset RMID state.
If mon_event_count() is called much later, the RMID state may be used
before it's reset. E.g. prev_msr might be non-0 value. That will cause
overflow code failure.

Seems this may happen on both x86 and arm64. At least need to make sure
RMID state reset happens before it's used.

> 
> This allows MPAM to hide the slice behaviour from resctrl, and to keep the
> monitor-allocation in monitor.c.
> 
> Tested-by: Shaopeng Tan <tan.shaopeng@...itsu.com>
> Signed-off-by: James Morse <james.morse@....com>
> ---
>  arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 7 +++++--
>  arch/x86/kernel/cpu/resctrl/internal.h    | 2 +-
>  arch/x86/kernel/cpu/resctrl/monitor.c     | 6 ++++--
>  3 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> index 1df0e3262bca..4ee3da6dced7 100644
> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> @@ -532,8 +532,11 @@ void mon_event_read(struct rmid_read *rr, struct
> rdt_resource *r,
>  		    struct rdt_domain *d, struct rdtgroup *rdtgrp,
>  		    int evtid, int first)
>  {
> +	/* When picking a cpu from cpu_mask, ensure it can't race with cpuhp
> */
> +	lockdep_assert_held(&rdtgroup_mutex);
> +
>  	/*
> -	 * setup the parameters to send to the IPI to read the data.
> +	 * setup the parameters to pass to mon_event_count() to read the data.
>  	 */
>  	rr->rgrp = rdtgrp;
>  	rr->evtid = evtid;
> @@ -542,7 +545,7 @@ void mon_event_read(struct rmid_read *rr, struct
> rdt_resource *r,
>  	rr->val = 0;
>  	rr->first = first;
> 
> -	smp_call_function_any(&d->cpu_mask, mon_event_count, rr, 1);
> +	smp_call_on_cpu(cpumask_any(&d->cpu_mask), mon_event_count, rr,
> +false);
>  }
> 
>  int rdtgroup_mondata_show(struct seq_file *m, void *arg) diff --git
> a/arch/x86/kernel/cpu/resctrl/internal.h
> b/arch/x86/kernel/cpu/resctrl/internal.h
> index adae6231324f..1f90a10b75a1 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -514,7 +514,7 @@ void closid_free(int closid);  int alloc_rmid(u32 closid);
> void free_rmid(u32 closid, u32 rmid);  int rdt_get_mon_l3_config(struct
> rdt_resource *r); -void mon_event_count(void *info);
> +int mon_event_count(void *info);
>  int rdtgroup_mondata_show(struct seq_file *m, void *arg);  void
> mon_event_read(struct rmid_read *rr, struct rdt_resource *r,
>  		    struct rdt_domain *d, struct rdtgroup *rdtgrp, diff --git
> a/arch/x86/kernel/cpu/resctrl/monitor.c
> b/arch/x86/kernel/cpu/resctrl/monitor.c
> index 190ac183139e..d309b830aeb2 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -509,10 +509,10 @@ static void mbm_bw_count(u32 closid, u32 rmid,
> struct rmid_read *rr)  }
> 
>  /*
> - * This is called via IPI to read the CQM/MBM counters
> + * This is scheduled by mon_event_read() to read the CQM/MBM counters
>   * on a domain.
>   */
> -void mon_event_count(void *info)
> +int mon_event_count(void *info)
>  {
>  	struct rdtgroup *rdtgrp, *entry;
>  	struct rmid_read *rr = info;
> @@ -545,6 +545,8 @@ void mon_event_count(void *info)
>  	 */
>  	if (ret == 0)
>  		rr->err = 0;
> +
> +	return 0;
>  }
> 
>  /*
> --
> 2.30.2

Thanks.

-Fenghua

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ