Received-SPF: pass (google.com: domain of linux-kernel+bounces-52692-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Date: Mon, 5 Feb 2024 15:11:22 +0200 (EET)
To: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
cc: fenghua.yu@intel.com, Reinette Chatre <reinette.chatre@intel.com>, 
    shuah@kernel.org, linux-kselftest@vger.kernel.org, 
    LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 5/5] selftests/resctrl: Add non-contiguous CBMs CAT
 test
In-Reply-To: <b341ecc8d1143932307708aff44ab90db3a91564.1707130307.git.maciej.wieczor-retman@intel.com>
Message-ID: <81064977-28cd-10bf-6721-c5a0916dee85@linux.intel.com>
References: <cover.1707130307.git.maciej.wieczor-retman@intel.com> <b341ecc8d1143932307708aff44ab90db3a91564.1707130307.git.maciej.wieczor-retman@intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323328-2116838435-1707138682=:1027"

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323328-2116838435-1707138682=:1027
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: QUOTED-PRINTABLE

On Mon, 5 Feb 2024, Maciej Wieczor-Retman wrote:

> Add tests for both L2 and L3 CAT to verify the return values
> generated by writing non-contiguous CBMs don't contradict the
> reported non-contiguous support information.
>=20
> Use a logical XOR to confirm return value of write_schemata() and
> non-contiguous CBMs support information match.
>=20
> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> ---
> Changelog v4:
> - Return failure instead of error on check of cpuid against sparse_masks
>   and on contiguous write_schemata fail. (Reinette)
>=20
> Changelog v3:
> - Roll back __cpuid_count part. (Reinette)
> - Update function name to read sparse_masks file.
> - Roll back get_cache_level() changes.
> - Add ksft_print_msg() to contiguous schemata write error handling
>   (Reinette).
>=20
> Changelog v2:
> - Redo the patch message. (Ilpo)
> - Tidy up __cpuid_count calls. (Ilpo)
> - Remove redundant AND in noncont_mask calculations (Ilpo)
> - Fix bit_center offset.
> - Add newline before function return. (Ilpo)
> - Group non-contiguous tests with CAT tests. (Ilpo)
> - Use a helper for reading sparse_masks file. (Ilpo)
> - Make get_cache_level() available in other source files. (Ilpo)
>=20
>  tools/testing/selftests/resctrl/cat_test.c    | 81 +++++++++++++++++++
>  tools/testing/selftests/resctrl/resctrl.h     |  2 +
>  .../testing/selftests/resctrl/resctrl_tests.c |  2 +
>  3 files changed, 85 insertions(+)
>=20
> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/s=
elftests/resctrl/cat_test.c
> index 39fc9303b8e8..20eb978e624b 100644
> --- a/tools/testing/selftests/resctrl/cat_test.c
> +++ b/tools/testing/selftests/resctrl/cat_test.c
> @@ -294,6 +294,71 @@ static int cat_run_test(const struct resctrl_test *t=
est, const struct user_param
>  =09return ret;
>  }
> =20
> +static int noncont_cat_run_test(const struct resctrl_test *test,
> +=09=09=09=09const struct user_params *uparams)
> +{
> +=09unsigned long full_cache_mask, cont_mask, noncont_mask;
> +=09unsigned int eax, ebx, ecx, edx, ret, sparse_masks;
> +=09char schemata[64];
> +=09int bit_center;
> +
> +=09/* Check to compare sparse_masks content to CPUID output. */
> +=09ret =3D resource_info_unsigned_get(test->resource, "sparse_masks", &s=
parse_masks);
> +=09if (ret)
> +=09=09return ret;
> +
> +=09if (!strcmp(test->resource, "L3"))
> +=09=09__cpuid_count(0x10, 1, eax, ebx, ecx, edx);
> +=09else if (!strcmp(test->resource, "L2"))
> +=09=09__cpuid_count(0x10, 2, eax, ebx, ecx, edx);
> +=09else
> +=09=09return -EINVAL;
> +
> +=09if (sparse_masks !=3D ((ecx >> 3) & 1)) {
> +=09=09ksft_print_msg("CPUID output doesn't match 'sparse_masks' file con=
tent!\n");
> +=09=09return 1;
> +=09}
> +
> +=09/* Write checks initialization. */
> +=09ret =3D get_full_cbm(test->resource, &full_cache_mask);
> +=09if (ret < 0)
> +=09=09return ret;
> +=09bit_center =3D count_bits(full_cache_mask) / 2;
> +=09cont_mask =3D full_cache_mask >> bit_center;
> +
> +=09/* Contiguous mask write check. */
> +=09snprintf(schemata, sizeof(schemata), "%lx", cont_mask);
> +=09ret =3D write_schemata("", schemata, uparams->cpu, test->resource);
> +=09if (ret) {
> +=09=09ksft_print_msg("Write of contiguous CBM failed\n");
> +=09=09return 1;
> +=09}
> +
> +=09/*
> +=09 * Non-contiguous mask write check. CBM has a 0xf hole approximately =
in the middle.
> +=09 * Output is compared with support information to catch any edge case=
 errors.
> +=09 */
> +=09noncont_mask =3D ~(0xf << (bit_center - 2)) & full_cache_mask;

To be on the safe side, I think the types could be made to match here=20
with 0xfUL to avoid sizeof(int) vs sizeof(unsigned long) related bit=20
drops in the & (although it feel somewhat theoretical given the bitmask=20
sizes we are currently seeing).

Reviewed-by: Ilpo J=E4rvinen <ilpo.jarvinen@linux.intel.com>


--=20
 i.

--8323328-2116838435-1707138682=:1027--