[PATCH] x86emul: support clzero

[PATCH] x86emul: support clzero

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 4407 bytes --]

... in anticipation of this possibly going to get used by guests for
basic thinks like memset() or clearing or pages.

Since the emulation doesn't use clzero itself, checking the guest's
CPUID for the feature to be exposed is (intentionally) being avoided
here. All that's required is sensible guest side data for the clflush
line size.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -82,6 +82,12 @@ static int cpuid(
     return X86EMUL_OKAY;
 }
 
+#define cache_line_size() ({ \
+    unsigned int eax = 1, ebx, ecx = 0, edx; \
+    cpuid(&eax, &ebx, &ecx, &edx, NULL); \
+    edx & (1U << 19) ? (ebx >> 5) & 0x7f8 : 0; \
+})
+
 #define cpu_has_mmx ({ \
     unsigned int eax = 1, ecx = 0, edx; \
     cpuid(&eax, &ecx, &ecx, &edx, NULL); \
@@ -873,6 +879,35 @@ int main(int argc, char **argv)
 #undef set_insn
 #undef check_eip
 
+    j = cache_line_size();
+    snprintf(instr, (char *)res + MMAP_SZ - instr,
+             "Testing clzero (%u-byte line)...", j);
+    printf("%-40s", instr);
+    if ( j >= sizeof(*res) && j <= MMAP_SZ / 4 )
+    {
+        instr[0] = 0x0f; instr[1] = 0x01; instr[2] = 0xfc;
+        regs.eflags = 0x200;
+        regs.eip    = (unsigned long)&instr[0];
+        regs.eax    = (unsigned long)res + MMAP_SZ / 2 + j - 1;
+        memset((void *)res + MMAP_SZ / 4, ~0, 3 * MMAP_SZ / 4);
+        rc = x86_emulate(&ctxt, &emulops);
+        if ( (rc != X86EMUL_OKAY) ||
+             (regs.eax != (unsigned long)res + MMAP_SZ / 2 + j - 1) ||
+             (regs.eflags != 0x200) ||
+             (regs.eip != (unsigned long)&instr[3]) ||
+             (res[MMAP_SZ / 2 / sizeof(*res) - 1] != ~0U) ||
+             (res[(MMAP_SZ / 2 + j) / sizeof(*res)] != ~0U) )
+            goto fail;
+        for ( i = 0; i < j; i += sizeof(*res) )
+            if ( res[(MMAP_SZ / 2 + i) / sizeof(*res)] )
+                break;
+        if ( i < j )
+            goto fail;
+        printf("okay\n");
+    }
+    else
+        printf("skipped\n");
+
     for ( j = 1; j <= 2; j++ )
     {
 #if defined(__i386__)
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1069,6 +1069,7 @@ static bool_t vcpu_has(
     return rc == X86EMUL_OKAY;
 }
 
+#define vcpu_has_clflush() vcpu_has(       1, EDX, 19, ctxt, ops)
 #define vcpu_has_lzcnt() vcpu_has(0x80000001, ECX,  5, ctxt, ops)
 #define vcpu_has_bmi1()  vcpu_has(0x00000007, EBX,  3, ctxt, ops)
 
@@ -3841,6 +3842,45 @@ x86_emulate(
             if ( (rc = ops->vmfunc(ctxt) != X86EMUL_OKAY) )
                 goto done;
             goto no_writeback;
+	case 0xfc: /* clzero */ {
+            unsigned int eax = 1, ebx = 0, dummy = 0;
+            unsigned long zero = 0;
+
+            base = ad_bytes == 8 ? _regs.eax :
+                   ad_bytes == 4 ? (uint32_t)_regs.eax : (uint16_t)_regs.eax;
+            limit = 0;
+            if ( vcpu_has_clflush() &&
+                 ops->cpuid(&eax, &ebx, &dummy, &dummy, ctxt) == X86EMUL_OKAY )
+                limit = ((ebx >> 8) & 0xff) * 8;
+            generate_exception_if(limit < sizeof(long) ||
+                                  (limit & (limit - 1)), EXC_UD, -1);
+            base &= ~(limit - 1);
+            if ( override_seg == -1 )
+                override_seg = x86_seg_ds;
+            if ( ops->rep_stos )
+            {
+                unsigned long nr_reps = limit / sizeof(zero);
+
+                rc = ops->rep_stos(&zero, override_seg, base, sizeof(zero),
+                                   &nr_reps, ctxt);
+                if ( rc == X86EMUL_OKAY )
+                {
+                    base += nr_reps * sizeof(zero);
+                    limit -= nr_reps * sizeof(zero);
+                }
+                else if ( rc != X86EMUL_UNHANDLEABLE )
+                    goto done;
+            }
+            while ( limit )
+            {
+                rc = ops->write(override_seg, base, &zero, sizeof(zero), ctxt);
+                if ( rc != X86EMUL_OKAY )
+                    goto done;
+                base += sizeof(zero);
+                limit -= sizeof(zero);
+            }
+            goto no_writeback;
+        }
         }
 
         switch ( modrm_reg & 7 )



[-- Attachment #2: x86emul-clzero.patch --]
[-- Type: text/plain, Size: 4430 bytes --]

x86emul: support clzero

... in anticipation of this possibly going to get used by guests for
basic thinks like memset() or clearing or pages.

Since the emulation doesn't use clzero itself, checking the guest's
CPUID for the feature to be exposed is (intentionally) being avoided
here. All that's required is sensible guest side data for the clflush
line size.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -82,6 +82,12 @@ static int cpuid(
     return X86EMUL_OKAY;
 }
 
+#define cache_line_size() ({ \
+    unsigned int eax = 1, ebx, ecx = 0, edx; \
+    cpuid(&eax, &ebx, &ecx, &edx, NULL); \
+    edx & (1U << 19) ? (ebx >> 5) & 0x7f8 : 0; \
+})
+
 #define cpu_has_mmx ({ \
     unsigned int eax = 1, ecx = 0, edx; \
     cpuid(&eax, &ecx, &ecx, &edx, NULL); \
@@ -873,6 +879,35 @@ int main(int argc, char **argv)
 #undef set_insn
 #undef check_eip
 
+    j = cache_line_size();
+    snprintf(instr, (char *)res + MMAP_SZ - instr,
+             "Testing clzero (%u-byte line)...", j);
+    printf("%-40s", instr);
+    if ( j >= sizeof(*res) && j <= MMAP_SZ / 4 )
+    {
+        instr[0] = 0x0f; instr[1] = 0x01; instr[2] = 0xfc;
+        regs.eflags = 0x200;
+        regs.eip    = (unsigned long)&instr[0];
+        regs.eax    = (unsigned long)res + MMAP_SZ / 2 + j - 1;
+        memset((void *)res + MMAP_SZ / 4, ~0, 3 * MMAP_SZ / 4);
+        rc = x86_emulate(&ctxt, &emulops);
+        if ( (rc != X86EMUL_OKAY) ||
+             (regs.eax != (unsigned long)res + MMAP_SZ / 2 + j - 1) ||
+             (regs.eflags != 0x200) ||
+             (regs.eip != (unsigned long)&instr[3]) ||
+             (res[MMAP_SZ / 2 / sizeof(*res) - 1] != ~0U) ||
+             (res[(MMAP_SZ / 2 + j) / sizeof(*res)] != ~0U) )
+            goto fail;
+        for ( i = 0; i < j; i += sizeof(*res) )
+            if ( res[(MMAP_SZ / 2 + i) / sizeof(*res)] )
+                break;
+        if ( i < j )
+            goto fail;
+        printf("okay\n");
+    }
+    else
+        printf("skipped\n");
+
     for ( j = 1; j <= 2; j++ )
     {
 #if defined(__i386__)
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1069,6 +1069,7 @@ static bool_t vcpu_has(
     return rc == X86EMUL_OKAY;
 }
 
+#define vcpu_has_clflush() vcpu_has(       1, EDX, 19, ctxt, ops)
 #define vcpu_has_lzcnt() vcpu_has(0x80000001, ECX,  5, ctxt, ops)
 #define vcpu_has_bmi1()  vcpu_has(0x00000007, EBX,  3, ctxt, ops)
 
@@ -3841,6 +3842,45 @@ x86_emulate(
             if ( (rc = ops->vmfunc(ctxt) != X86EMUL_OKAY) )
                 goto done;
             goto no_writeback;
+	case 0xfc: /* clzero */ {
+            unsigned int eax = 1, ebx = 0, dummy = 0;
+            unsigned long zero = 0;
+
+            base = ad_bytes == 8 ? _regs.eax :
+                   ad_bytes == 4 ? (uint32_t)_regs.eax : (uint16_t)_regs.eax;
+            limit = 0;
+            if ( vcpu_has_clflush() &&
+                 ops->cpuid(&eax, &ebx, &dummy, &dummy, ctxt) == X86EMUL_OKAY )
+                limit = ((ebx >> 8) & 0xff) * 8;
+            generate_exception_if(limit < sizeof(long) ||
+                                  (limit & (limit - 1)), EXC_UD, -1);
+            base &= ~(limit - 1);
+            if ( override_seg == -1 )
+                override_seg = x86_seg_ds;
+            if ( ops->rep_stos )
+            {
+                unsigned long nr_reps = limit / sizeof(zero);
+
+                rc = ops->rep_stos(&zero, override_seg, base, sizeof(zero),
+                                   &nr_reps, ctxt);
+                if ( rc == X86EMUL_OKAY )
+                {
+                    base += nr_reps * sizeof(zero);
+                    limit -= nr_reps * sizeof(zero);
+                }
+                else if ( rc != X86EMUL_UNHANDLEABLE )
+                    goto done;
+            }
+            while ( limit )
+            {
+                rc = ops->write(override_seg, base, &zero, sizeof(zero), ctxt);
+                if ( rc != X86EMUL_OKAY )
+                    goto done;
+                base += sizeof(zero);
+                limit -= sizeof(zero);
+            }
+            goto no_writeback;
+        }
         }
 
         switch ( modrm_reg & 7 )

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] x86emul: support clzero

[Andrew Cooper]

On 22/09/15 14:06, Jan Beulich wrote:
> ... in anticipation of this possibly going to get used by guests for
> basic thinks like memset() or clearing or pages.
>
> Since the emulation doesn't use clzero itself, checking the guest's
> CPUID for the feature to be exposed is (intentionally) being avoided
> here. All that's required is sensible guest side data for the clflush
> line size.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Where have you found this instruction?  Googling, I have found a
presentation talking about it being new in the new AMD Zen cores, but I
still can't locate any technical documentation on the matter.

~Andrew

Re: [PATCH] x86emul: support clzero

[Jan Beulich]

>>> On 23.09.15 at 19:37, <andrew.cooper3@citrix.com> wrote:
> On 22/09/15 14:06, Jan Beulich wrote:
>> ... in anticipation of this possibly going to get used by guests for
>> basic thinks like memset() or clearing or pages.
>>
>> Since the emulation doesn't use clzero itself, checking the guest's
>> CPUID for the feature to be exposed is (intentionally) being avoided
>> here. All that's required is sensible guest side data for the clflush
>> line size.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Where have you found this instruction?  Googling, I have found a
> presentation talking about it being new in the new AMD Zen cores, but I
> still can't locate any technical documentation on the matter.

Sadly no technical documentation so far, despite me pinging for it
after the respective binutils patch
(https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=029f3522619e8b77a7b848be23f4c13e50087d8b)
got posted and went in.

Jan

Re: [PATCH] x86emul: support clzero

[Andrew Cooper]

On 24/09/15 09:02, Jan Beulich wrote:
>>>> On 23.09.15 at 19:37, <andrew.cooper3@citrix.com> wrote:
>> On 22/09/15 14:06, Jan Beulich wrote:
>>> ... in anticipation of this possibly going to get used by guests for
>>> basic thinks like memset() or clearing or pages.
>>>
>>> Since the emulation doesn't use clzero itself, checking the guest's
>>> CPUID for the feature to be exposed is (intentionally) being avoided
>>> here. All that's required is sensible guest side data for the clflush
>>> line size.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Where have you found this instruction?  Googling, I have found a
>> presentation talking about it being new in the new AMD Zen cores, but I
>> still can't locate any technical documentation on the matter.
> Sadly no technical documentation so far, despite me pinging for it
> after the respective binutils patch
> (https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=029f3522619e8b77a7b848be23f4c13e50087d8b)
> got posted and went in.

While I don't see an obvious issue with your patch, I can't claim to
have reviewed it without some documentation to refer to.

Aravind/Suravee: Any ideas when the AMD manuals might be updated to
include this?

~Andrew

Re: [PATCH] x86emul: support clzero

[Jan Beulich]

>>> On 24.09.15 at 13:59, <andrew.cooper3@citrix.com> wrote:
> On 24/09/15 09:02, Jan Beulich wrote:
>>>>> On 23.09.15 at 19:37, <andrew.cooper3@citrix.com> wrote:
>>> On 22/09/15 14:06, Jan Beulich wrote:
>>>> ... in anticipation of this possibly going to get used by guests for
>>>> basic thinks like memset() or clearing or pages.
>>>>
>>>> Since the emulation doesn't use clzero itself, checking the guest's
>>>> CPUID for the feature to be exposed is (intentionally) being avoided
>>>> here. All that's required is sensible guest side data for the clflush
>>>> line size.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> Where have you found this instruction?  Googling, I have found a
>>> presentation talking about it being new in the new AMD Zen cores, but I
>>> still can't locate any technical documentation on the matter.
>> Sadly no technical documentation so far, despite me pinging for it
>> after the respective binutils patch
>> 
> (https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=029f3 
> 522619e8b77a7b848be23f4c13e50087d8b)
>> got posted and went in.
> 
> While I don't see an obvious issue with your patch, I can't claim to
> have reviewed it without some documentation to refer to.

Understood. Depending on the actual semantics the patch may allow
the instruction to be used (emulated) in more cases than on actual
hardware, which I don't see as an issue. That's mainly due to the
undefinedness of "cache line" for memory types not using the cache
(i.e. the instruction may not do what one might expect on WC or UC
memory, which is what I've been trying to find out since said
binutils posting, but I'm pretty certain it would at best be undefined;
us giving it defined behavior would not violate that).

Jan

Re: [PATCH] x86emul: support clzero

[Jan Beulich]

>>> On 24.09.15 at 10:02, <JBeulich@suse.com> wrote:
>>>> On 23.09.15 at 19:37, <andrew.cooper3@citrix.com> wrote:
>> On 22/09/15 14:06, Jan Beulich wrote:
>>> ... in anticipation of this possibly going to get used by guests for
>>> basic thinks like memset() or clearing or pages.
>>>
>>> Since the emulation doesn't use clzero itself, checking the guest's
>>> CPUID for the feature to be exposed is (intentionally) being avoided
>>> here. All that's required is sensible guest side data for the clflush
>>> line size.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> 
>> Where have you found this instruction?  Googling, I have found a
>> presentation talking about it being new in the new AMD Zen cores, but I
>> still can't locate any technical documentation on the matter.
> 
> Sadly no technical documentation so far, despite me pinging for it
> after the respective binutils patch
> (https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=029f 
> 3522619e8b77a7b848be23f4c13e50087d8b)
> got posted and went in.

Sherry,

any chance you could help out here making this make some forward
progress?

Jan

Re: [PATCH] x86emul: support clzero

[Aravind Gopalakrishnan]

On 9/22/2015 8:06 AM, Jan Beulich wrote:
> ... in anticipation of this possibly going to get used by guests for
> basic thinks like memset() or clearing or pages.
>
> Since the emulation doesn't use clzero itself, checking the guest's
> CPUID for the feature to be exposed is (intentionally) being avoided
> here. All that's required is sensible guest side data for the clflush
> line size.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
>

Reviewed-by: Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>

Thanks,
-Aravind.

Re: [PATCH] x86emul: support clzero

[Andrew Cooper]

On 11/01/16 15:55, Aravind Gopalakrishnan wrote:
> On 9/22/2015 8:06 AM, Jan Beulich wrote:
>> ... in anticipation of this possibly going to get used by guests for
>> basic thinks like memset() or clearing or pages.
>>
>> Since the emulation doesn't use clzero itself, checking the guest's
>> CPUID for the feature to be exposed is (intentionally) being avoided
>> here. All that's required is sensible guest side data for the clflush
>> line size.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>>
>
> Reviewed-by: Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

I would still like to see some instruction documentation as the soonest
possible opportunity, to give the implementation an independent review.

~Andrew