subject:"\[PATCH v2 6\/8\] xen\/lib\: Add topology generator for x86"

Re: [PATCH v2 6/8] xen/lib: Add topology generator for x86

2024-05-28 Thread Alejandro Vallejo

On 23/05/2024 17:50, Roger Pau Monné wrote:
> On Wed, May 08, 2024 at 01:39:25PM +0100, Alejandro Vallejo wrote:
>> Add a helper to populate topology leaves in the cpu policy from
>> threads/core and cores/package counts.
>>
>> No functional change, as it's not connected to anything yet.
> 
> There is a functional change in test-cpu-policy.c.
> 
> Maybe the commit message needs to be updated to reflect the added
> testing to test-cpu-policy.c using the newly introduced helper to
> generate topologies?
> 

Sure to this and all formatting feedback below.

>>
>> Signed-off-by: Alejandro Vallejo 
>> ---
>> v2:
>>   * New patch. Extracted from v1/patch6
>> ---
>>  tools/tests/cpu-policy/test-cpu-policy.c | 128 +++
>>  xen/include/xen/lib/x86/cpu-policy.h |  16 +++
>>  xen/lib/x86/policy.c |  86 +++
>>  3 files changed, 230 insertions(+)
>>
>> diff --git a/tools/tests/cpu-policy/test-cpu-policy.c 
>> b/tools/tests/cpu-policy/test-cpu-policy.c
>> index 301df2c00285..0ba8c418b1b3 100644
>> --- a/tools/tests/cpu-policy/test-cpu-policy.c
>> +++ b/tools/tests/cpu-policy/test-cpu-policy.c
>> @@ -650,6 +650,132 @@ static void test_is_compatible_failure(void)
>>  }
>>  }
>>  
>> +static void test_topo_from_parts(void)
>> +{
>> +static const struct test {
>> +unsigned int threads_per_core;
>> +unsigned int cores_per_pkg;
>> +struct cpu_policy policy;
>> +} tests[] = {
>> +{
>> +.threads_per_core = 3, .cores_per_pkg = 1,
>> +.policy = {
>> +.x86_vendor = X86_VENDOR_AMD,
>> +.topo.subleaf = {
>> +[0] = { .nr_logical = 3, .level = 0, .type = 1, 
>> .id_shift = 2, },
>> +[1] = { .nr_logical = 1, .level = 1, .type = 2, 
>> .id_shift = 2, },
>> +},
>> +},
>> +},
>> +{
>> +.threads_per_core = 1, .cores_per_pkg = 3,
>> +.policy = {
>> +.x86_vendor = X86_VENDOR_AMD,
>> +.topo.subleaf = {
>> +[0] = { .nr_logical = 1, .level = 0, .type = 1, 
>> .id_shift = 0, },
>> +[1] = { .nr_logical = 3, .level = 1, .type = 2, 
>> .id_shift = 2, },
>> +},
>> +},
>> +},
>> +{
>> +.threads_per_core = 7, .cores_per_pkg = 5,
>> +.policy = {
>> +.x86_vendor = X86_VENDOR_AMD,
>> +.topo.subleaf = {
>> +[0] = { .nr_logical = 7, .level = 0, .type = 1, 
>> .id_shift = 3, },
>> +[1] = { .nr_logical = 5, .level = 1, .type = 2, 
>> .id_shift = 6, },
>> +},
>> +},
>> +},
>> +{
>> +.threads_per_core = 2, .cores_per_pkg = 128,
>> +.policy = {
>> +.x86_vendor = X86_VENDOR_AMD,
>> +.topo.subleaf = {
>> +[0] = { .nr_logical = 2, .level = 0, .type = 1, 
>> .id_shift = 1, },
>> +[1] = { .nr_logical = 128, .level = 1, .type = 2, 
>> .id_shift = 8, },
>> +},
>> +},
>> +},
>> +{
>> +.threads_per_core = 3, .cores_per_pkg = 1,
>> +.policy = {
>> +.x86_vendor = X86_VENDOR_INTEL,
>> +.topo.subleaf = {
>> +[0] = { .nr_logical = 3, .level = 0, .type = 1, 
>> .id_shift = 2, },
>> +[1] = { .nr_logical = 3, .level = 1, .type = 2, 
>> .id_shift = 2, },
>> +},
>> +},
>> +},
>> +{
>> +.threads_per_core = 1, .cores_per_pkg = 3,
>> +.policy = {
>> +.x86_vendor = X86_VENDOR_INTEL,
>> +.topo.subleaf = {
>> +[0] = { .nr_logical = 1, .level = 0, .type = 1, 
>> .id_shift = 0, },
>> +[1] = { .nr_logical = 3, .level = 1, .type = 2, 
>> .id_shift = 2, },
>> +},
>> +},
>> +},
>> +{
>> +.threads_per_core = 7, .cores_per_pkg = 5,
>> +.policy = {
>> +.x86_vendor = X86_VENDOR_INTEL,
>> +.topo.subleaf = {
>> +[0] = { .nr_logical = 7, .level = 0, .type = 1, 
>> .id_shift = 3, },
>> +[1] = { .nr_logical = 35, .level = 1, .type = 2, 
>> .id_shift = 6, },
>> +},
>> +},
>> +},
>> +{
>> +.threads_per_core = 2, .cores_per_pkg = 128,
>> +.policy = {
>> +.x86_vendor = X86_VENDOR_INTEL,
>> +.topo.subleaf = {
>> +[0] = { .nr_logical = 2, .level = 0, .type = 1, 
>> .id_shift = 1, },
>> +[1] = { .nr_logical = 256, .level = 1, .type = 2, 
>> .id_shift = 8, },
> 
> You don't need the array index in the initialization:
> 
> .topo.subleaf = {

Re: [PATCH v2 6/8] xen/lib: Add topology generator for x86

2024-05-23 Thread Roger Pau Monné

On Wed, May 08, 2024 at 01:39:25PM +0100, Alejandro Vallejo wrote:
> Add a helper to populate topology leaves in the cpu policy from
> threads/core and cores/package counts.
> 
> No functional change, as it's not connected to anything yet.

There is a functional change in test-cpu-policy.c.

Maybe the commit message needs to be updated to reflect the added
testing to test-cpu-policy.c using the newly introduced helper to
generate topologies?

> 
> Signed-off-by: Alejandro Vallejo 
> ---
> v2:
>   * New patch. Extracted from v1/patch6
> ---
>  tools/tests/cpu-policy/test-cpu-policy.c | 128 +++
>  xen/include/xen/lib/x86/cpu-policy.h |  16 +++
>  xen/lib/x86/policy.c |  86 +++
>  3 files changed, 230 insertions(+)
> 
> diff --git a/tools/tests/cpu-policy/test-cpu-policy.c 
> b/tools/tests/cpu-policy/test-cpu-policy.c
> index 301df2c00285..0ba8c418b1b3 100644
> --- a/tools/tests/cpu-policy/test-cpu-policy.c
> +++ b/tools/tests/cpu-policy/test-cpu-policy.c
> @@ -650,6 +650,132 @@ static void test_is_compatible_failure(void)
>  }
>  }
>  
> +static void test_topo_from_parts(void)
> +{
> +static const struct test {
> +unsigned int threads_per_core;
> +unsigned int cores_per_pkg;
> +struct cpu_policy policy;
> +} tests[] = {
> +{
> +.threads_per_core = 3, .cores_per_pkg = 1,
> +.policy = {
> +.x86_vendor = X86_VENDOR_AMD,
> +.topo.subleaf = {
> +[0] = { .nr_logical = 3, .level = 0, .type = 1, 
> .id_shift = 2, },
> +[1] = { .nr_logical = 1, .level = 1, .type = 2, 
> .id_shift = 2, },
> +},
> +},
> +},
> +{
> +.threads_per_core = 1, .cores_per_pkg = 3,
> +.policy = {
> +.x86_vendor = X86_VENDOR_AMD,
> +.topo.subleaf = {
> +[0] = { .nr_logical = 1, .level = 0, .type = 1, 
> .id_shift = 0, },
> +[1] = { .nr_logical = 3, .level = 1, .type = 2, 
> .id_shift = 2, },
> +},
> +},
> +},
> +{
> +.threads_per_core = 7, .cores_per_pkg = 5,
> +.policy = {
> +.x86_vendor = X86_VENDOR_AMD,
> +.topo.subleaf = {
> +[0] = { .nr_logical = 7, .level = 0, .type = 1, 
> .id_shift = 3, },
> +[1] = { .nr_logical = 5, .level = 1, .type = 2, 
> .id_shift = 6, },
> +},
> +},
> +},
> +{
> +.threads_per_core = 2, .cores_per_pkg = 128,
> +.policy = {
> +.x86_vendor = X86_VENDOR_AMD,
> +.topo.subleaf = {
> +[0] = { .nr_logical = 2, .level = 0, .type = 1, 
> .id_shift = 1, },
> +[1] = { .nr_logical = 128, .level = 1, .type = 2, 
> .id_shift = 8, },
> +},
> +},
> +},
> +{
> +.threads_per_core = 3, .cores_per_pkg = 1,
> +.policy = {
> +.x86_vendor = X86_VENDOR_INTEL,
> +.topo.subleaf = {
> +[0] = { .nr_logical = 3, .level = 0, .type = 1, 
> .id_shift = 2, },
> +[1] = { .nr_logical = 3, .level = 1, .type = 2, 
> .id_shift = 2, },
> +},
> +},
> +},
> +{
> +.threads_per_core = 1, .cores_per_pkg = 3,
> +.policy = {
> +.x86_vendor = X86_VENDOR_INTEL,
> +.topo.subleaf = {
> +[0] = { .nr_logical = 1, .level = 0, .type = 1, 
> .id_shift = 0, },
> +[1] = { .nr_logical = 3, .level = 1, .type = 2, 
> .id_shift = 2, },
> +},
> +},
> +},
> +{
> +.threads_per_core = 7, .cores_per_pkg = 5,
> +.policy = {
> +.x86_vendor = X86_VENDOR_INTEL,
> +.topo.subleaf = {
> +[0] = { .nr_logical = 7, .level = 0, .type = 1, 
> .id_shift = 3, },
> +[1] = { .nr_logical = 35, .level = 1, .type = 2, 
> .id_shift = 6, },
> +},
> +},
> +},
> +{
> +.threads_per_core = 2, .cores_per_pkg = 128,
> +.policy = {
> +.x86_vendor = X86_VENDOR_INTEL,
> +.topo.subleaf = {
> +[0] = { .nr_logical = 2, .level = 0, .type = 1, 
> .id_shift = 1, },
> +[1] = { .nr_logical = 256, .level = 1, .type = 2, 
> .id_shift = 8, },

You don't need the array index in the initialization:

.topo.subleaf = {
{ .nr_logical = 2, .level = 0, .type = 1, .id_shift = 1, },
{ .nr_logical = 256, .level = 1, .type = 2,
  .id_shift = 8, },
}

And lines should be limited to 80

[PATCH v2 6/8] xen/lib: Add topology generator for x86

2024-05-08 Thread Alejandro Vallejo

Add a helper to populate topology leaves in the cpu policy from
threads/core and cores/package counts.

No functional change, as it's not connected to anything yet.

Signed-off-by: Alejandro Vallejo 
---
v2:
  * New patch. Extracted from v1/patch6
---
 tools/tests/cpu-policy/test-cpu-policy.c | 128 +++
 xen/include/xen/lib/x86/cpu-policy.h |  16 +++
 xen/lib/x86/policy.c |  86 +++
 3 files changed, 230 insertions(+)

diff --git a/tools/tests/cpu-policy/test-cpu-policy.c 
b/tools/tests/cpu-policy/test-cpu-policy.c
index 301df2c00285..0ba8c418b1b3 100644
--- a/tools/tests/cpu-policy/test-cpu-policy.c
+++ b/tools/tests/cpu-policy/test-cpu-policy.c
@@ -650,6 +650,132 @@ static void test_is_compatible_failure(void)
 }
 }
 
+static void test_topo_from_parts(void)
+{
+static const struct test {
+unsigned int threads_per_core;
+unsigned int cores_per_pkg;
+struct cpu_policy policy;
+} tests[] = {
+{
+.threads_per_core = 3, .cores_per_pkg = 1,
+.policy = {
+.x86_vendor = X86_VENDOR_AMD,
+.topo.subleaf = {
+[0] = { .nr_logical = 3, .level = 0, .type = 1, .id_shift 
= 2, },
+[1] = { .nr_logical = 1, .level = 1, .type = 2, .id_shift 
= 2, },
+},
+},
+},
+{
+.threads_per_core = 1, .cores_per_pkg = 3,
+.policy = {
+.x86_vendor = X86_VENDOR_AMD,
+.topo.subleaf = {
+[0] = { .nr_logical = 1, .level = 0, .type = 1, .id_shift 
= 0, },
+[1] = { .nr_logical = 3, .level = 1, .type = 2, .id_shift 
= 2, },
+},
+},
+},
+{
+.threads_per_core = 7, .cores_per_pkg = 5,
+.policy = {
+.x86_vendor = X86_VENDOR_AMD,
+.topo.subleaf = {
+[0] = { .nr_logical = 7, .level = 0, .type = 1, .id_shift 
= 3, },
+[1] = { .nr_logical = 5, .level = 1, .type = 2, .id_shift 
= 6, },
+},
+},
+},
+{
+.threads_per_core = 2, .cores_per_pkg = 128,
+.policy = {
+.x86_vendor = X86_VENDOR_AMD,
+.topo.subleaf = {
+[0] = { .nr_logical = 2, .level = 0, .type = 1, .id_shift 
= 1, },
+[1] = { .nr_logical = 128, .level = 1, .type = 2, 
.id_shift = 8, },
+},
+},
+},
+{
+.threads_per_core = 3, .cores_per_pkg = 1,
+.policy = {
+.x86_vendor = X86_VENDOR_INTEL,
+.topo.subleaf = {
+[0] = { .nr_logical = 3, .level = 0, .type = 1, .id_shift 
= 2, },
+[1] = { .nr_logical = 3, .level = 1, .type = 2, .id_shift 
= 2, },
+},
+},
+},
+{
+.threads_per_core = 1, .cores_per_pkg = 3,
+.policy = {
+.x86_vendor = X86_VENDOR_INTEL,
+.topo.subleaf = {
+[0] = { .nr_logical = 1, .level = 0, .type = 1, .id_shift 
= 0, },
+[1] = { .nr_logical = 3, .level = 1, .type = 2, .id_shift 
= 2, },
+},
+},
+},
+{
+.threads_per_core = 7, .cores_per_pkg = 5,
+.policy = {
+.x86_vendor = X86_VENDOR_INTEL,
+.topo.subleaf = {
+[0] = { .nr_logical = 7, .level = 0, .type = 1, .id_shift 
= 3, },
+[1] = { .nr_logical = 35, .level = 1, .type = 2, .id_shift 
= 6, },
+},
+},
+},
+{
+.threads_per_core = 2, .cores_per_pkg = 128,
+.policy = {
+.x86_vendor = X86_VENDOR_INTEL,
+.topo.subleaf = {
+[0] = { .nr_logical = 2, .level = 0, .type = 1, .id_shift 
= 1, },
+[1] = { .nr_logical = 256, .level = 1, .type = 2, 
.id_shift = 8, },
+},
+},
+},
+};
+
+printf("Testing topology synthesis from parts:\n");
+
+for ( size_t i = 0; i < ARRAY_SIZE(tests); ++i )
+{
+const struct test *t = [i];
+struct cpu_policy actual = { .x86_vendor = t->policy.x86_vendor };
+int rc = x86_topo_from_parts(, t->threads_per_core, 
t->cores_per_pkg);
+
+if ( rc || memcmp(, >policy.topo, sizeof(actual.topo)) )
+{
+#define TOPO(n) topo.subleaf[(n)]
+fail("FAIL[%d] - '%s %u t/c, %u c/p'\n",
+ rc,
+ x86_cpuid_vendor_to_str(t->policy.x86_vendor),
+ t->threads_per_core, t->cores_per_pkg);
+printf("  subleaf=%u  expected_n=%u actual_n=%u\n"
+   " expected_lvl=%u actual_lvl=%u\n"
+   "

Re: [PATCH v2 6/8] xen/lib: Add topology generator for x86

Re: [PATCH v2 6/8] xen/lib: Add topology generator for x86

[PATCH v2 6/8] xen/lib: Add topology generator for x86

3 matches

Site Navigation

Mail list logo

Footer information