[PATCH] x86emul: pad blob-execution "okay" messages

Jan Beulich posted 1 patch 2 years, 10 months ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/3250a871-e49d-d3c4-333a-eff435e092c2@suse.com
[PATCH] x86emul: pad blob-execution "okay" messages
Posted by Jan Beulich 2 years, 10 months ago
We already do so in the native execution case, and a few descriptions (I
did notice this with SHA ones) are short enough for the output to look
slightly odd.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Many descriptions are longer than 37 characters, so I wonder whether we
wouldn't want to bump the padding to 50, 60, or even 70. And this
perhaps despite then going out of sync with the individual insn tests
earlier on (which I wouldn't want to touch).

--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -5181,6 +5181,8 @@ int main(int argc, char **argv)
 
     for ( j = 0; j < ARRAY_SIZE(blobs); j++ )
     {
+        unsigned int nr;
+
         if ( blobs[j].check_cpu && !blobs[j].check_cpu() )
             continue;
 
@@ -5196,7 +5198,8 @@ int main(int argc, char **argv)
 
         if ( ctxt.addr_size == sizeof(void *) * CHAR_BIT )
         {
-            i = printf("Testing %s native execution...", blobs[j].name);
+            nr = printf("Testing %s native execution...", blobs[j].name);
+
             if ( blobs[j].set_regs )
                 blobs[j].set_regs(&regs);
             asm volatile (
@@ -5212,11 +5215,13 @@ int main(int argc, char **argv)
             );
             if ( !blobs[j].check_regs(&regs) )
                 goto fail;
-            printf("%*sokay\n", i < 40 ? 40 - i : 0, "");
+
+            printf("%*sokay\n", nr < 40 ? 40 - nr : 0, "");
         }
 
-        printf("Testing %s %u-bit code sequence",
-               blobs[j].name, ctxt.addr_size);
+        nr = printf("Testing %s %u-bit code sequence",
+                    blobs[j].name, ctxt.addr_size);
+
         if ( blobs[j].set_regs )
             blobs[j].set_regs(&regs);
         regs.eip = (unsigned long)res;
@@ -5233,7 +5238,10 @@ int main(int argc, char **argv)
                 regs.eip < (unsigned long)res + blobs[j].size )
         {
             if ( (i++ & 8191) == 0 )
+            {
                 printf(".");
+                ++nr;
+            }
             rc = x86_emulate(&ctxt, &emulops);
             if ( rc != X86EMUL_OKAY )
             {
@@ -5242,13 +5250,17 @@ int main(int argc, char **argv)
                 return 1;
             }
         }
-        for ( ; i < 2 * 8192; i += 8192 )
+        for ( ; i < 2 * 8192; i += 8192 ) {
             printf(".");
+            ++nr;
+        }
+
         if ( (regs.eip != 0x12345678) ||
              (regs.esp != ((unsigned long)res + MMAP_SZ)) ||
              !blobs[j].check_regs(&regs) )
             goto fail;
-        printf("okay\n");
+
+        printf("%*sokay\n", nr < 40 ? 40 - nr : 0, "");
     }
 
     return 0;


Ping: [PATCH] x86emul: pad blob-execution "okay" messages
Posted by Jan Beulich 2 years, 9 months ago
On 02.06.2021 16:38, Jan Beulich wrote:
> We already do so in the native execution case, and a few descriptions (I
> did notice this with SHA ones) are short enough for the output to look
> slightly odd.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Again - anyone?

Thanks, Jan

> ---
> Many descriptions are longer than 37 characters, so I wonder whether we
> wouldn't want to bump the padding to 50, 60, or even 70. And this
> perhaps despite then going out of sync with the individual insn tests
> earlier on (which I wouldn't want to touch).
> 
> --- a/tools/tests/x86_emulator/test_x86_emulator.c
> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
> @@ -5181,6 +5181,8 @@ int main(int argc, char **argv)
>  
>      for ( j = 0; j < ARRAY_SIZE(blobs); j++ )
>      {
> +        unsigned int nr;
> +
>          if ( blobs[j].check_cpu && !blobs[j].check_cpu() )
>              continue;
>  
> @@ -5196,7 +5198,8 @@ int main(int argc, char **argv)
>  
>          if ( ctxt.addr_size == sizeof(void *) * CHAR_BIT )
>          {
> -            i = printf("Testing %s native execution...", blobs[j].name);
> +            nr = printf("Testing %s native execution...", blobs[j].name);
> +
>              if ( blobs[j].set_regs )
>                  blobs[j].set_regs(&regs);
>              asm volatile (
> @@ -5212,11 +5215,13 @@ int main(int argc, char **argv)
>              );
>              if ( !blobs[j].check_regs(&regs) )
>                  goto fail;
> -            printf("%*sokay\n", i < 40 ? 40 - i : 0, "");
> +
> +            printf("%*sokay\n", nr < 40 ? 40 - nr : 0, "");
>          }
>  
> -        printf("Testing %s %u-bit code sequence",
> -               blobs[j].name, ctxt.addr_size);
> +        nr = printf("Testing %s %u-bit code sequence",
> +                    blobs[j].name, ctxt.addr_size);
> +
>          if ( blobs[j].set_regs )
>              blobs[j].set_regs(&regs);
>          regs.eip = (unsigned long)res;
> @@ -5233,7 +5238,10 @@ int main(int argc, char **argv)
>                  regs.eip < (unsigned long)res + blobs[j].size )
>          {
>              if ( (i++ & 8191) == 0 )
> +            {
>                  printf(".");
> +                ++nr;
> +            }
>              rc = x86_emulate(&ctxt, &emulops);
>              if ( rc != X86EMUL_OKAY )
>              {
> @@ -5242,13 +5250,17 @@ int main(int argc, char **argv)
>                  return 1;
>              }
>          }
> -        for ( ; i < 2 * 8192; i += 8192 )
> +        for ( ; i < 2 * 8192; i += 8192 ) {
>              printf(".");
> +            ++nr;
> +        }
> +
>          if ( (regs.eip != 0x12345678) ||
>               (regs.esp != ((unsigned long)res + MMAP_SZ)) ||
>               !blobs[j].check_regs(&regs) )
>              goto fail;
> -        printf("okay\n");
> +
> +        printf("%*sokay\n", nr < 40 ? 40 - nr : 0, "");
>      }
>  
>      return 0;
> 
> 


Re: Ping: [PATCH] x86emul: pad blob-execution "okay" messages
Posted by Jan Beulich 2 years, 9 months ago
On 28.06.2021 14:15, Jan Beulich wrote:
> On 02.06.2021 16:38, Jan Beulich wrote:
>> We already do so in the native execution case, and a few descriptions (I
>> did notice this with SHA ones) are short enough for the output to look
>> slightly odd.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Again - anyone?

Okay, this is trivial enough to fall under "lazy consensus" if I don't hear
back by Thursday (i.e. including the Community Call as an option to voice
objections).

Jan