Re: [Xen-devel] Bug in x86 instruction emulator?
On Wed, 06 Apr 2016 01:38:32 +0200 wo...@openmailbox.org wrote: > I'm running Xen 4.6.1 with Alpine Linux 3.3.3 in dom0. In a HVM domU > with vga="qxl", Xorg will segfault instantly if tried started. Multiple > Linux distros have been tested and Xorg segfaults in all. > > Attached are a full backtrace from domU generated by Xorg, and a > assembler dump of function 'sse2_blt'. > > According to Xen IRC channel, the cause could be a bug in the x86 > instruction emulator related to SSE. I don't believe the x86 emulator is complete wrt the SSE instruction set. But I do wonder why, in your case, these instructions need emulation at all. Unless touching the video RAM requires emulation. Can you try using a different video driver? I see xorg picked up qxl, maybe try vesa? -- Mihai Donțu ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Bug in x86 instruction emulator?
On Wed, 6 Apr 2016 02:57:35 +0300 Mihai Donțu wrote: > On Wed, 06 Apr 2016 01:38:32 +0200 wo...@openmailbox.org wrote: > > I'm running Xen 4.6.1 with Alpine Linux 3.3.3 in dom0. In a HVM domU > > with vga="qxl", Xorg will segfault instantly if tried started. Multiple > > Linux distros have been tested and Xorg segfaults in all. > > > > Attached are a full backtrace from domU generated by Xorg, and a > > assembler dump of function 'sse2_blt'. > > > > According to Xen IRC channel, the cause could be a bug in the x86 > > instruction emulator related to SSE. > > I don't believe the x86 emulator is complete wrt the SSE instruction > set. But I do wonder why, in your case, these instructions need > emulation at all. Unless touching the video RAM requires emulation. Can > you try using a different video driver? I see xorg picked up qxl, maybe > try vesa? This would appear to be an old issue with qxl and xen: http://www.gossamer-threads.com/lists/xen/devel/358125 -- Mihai Donțu ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Side channel attack
On Fri, 15 Apr 2016 15:49:20 +0800 Zakirasafi wrote: > The following code is for side channel attack on xen hypevisor. In this > code I am having problem in understanding the highlighted red line. In the > line what ".byte 15, byte 49" do??? You can use this trick in the future: $ printf "\xf\x31" | ndisasm -b 64 - 0F31 rdtsc -- Mihai Donțu ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/5] x86/emulate: add support for {, v}movq xmm, xmm/m64
On Thursday 08 September 2016 07:45:19 Jan Beulich wrote:
> From: Mihai Donțu
>
> Signed-off-by: Mihai Donțu
> Signed-off-by: Jan Beulich
> ---
> v4: Re-base on decoding changes. Address my own review comments (where
> still applicable). #UD when vex.l is set. Various adjustments to
> the test tool change.
Thank you! They were in my queue for too long and I was struggling to
find a window of time to get them in shape.
> --- a/tools/tests/x86_emulator/test_x86_emulator.c
> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
> @@ -713,6 +713,54 @@ int main(int argc, char **argv)
> else
> printf("skipped\n");
>
> +printf("%-40s", "Testing movq %%xmm0,32(%%ecx)...");
> +if ( stack_exec && cpu_has_sse2 )
> +{
> +decl_insn(movq_to_mem2);
> +
> +asm volatile ( "pcmpgtb %%xmm0, %%xmm0\n"
> + put_insn(movq_to_mem2, "movq %%xmm0, 32(%0)")
> + :: "c" (NULL) );
> +
> +memset(res, 0xbd, 64);
> +set_insn(movq_to_mem2);
> +regs.ecx = (unsigned long)res;
> +regs.edx = 0;
> +rc = x86_emulate(&ctxt, &emulops);
> +if ( rc != X86EMUL_OKAY || !check_eip(movq_to_mem2) ||
> + *((uint64_t *)res + 4) ||
> + memcmp(res, res + 10, 24) ||
> + memcmp(res, res + 6, 8) )
> +goto fail;
> +printf("okay\n");
> +}
> +else
> +printf("skipped\n");
> +
> +printf("%-40s", "Testing vmovq %%xmm1,32(%%edx)...");
> +if ( stack_exec && cpu_has_avx )
> +{
> +decl_insn(vmovq_to_mem);
> +
> +asm volatile ( "pcmpgtb %%xmm1, %%xmm1\n"
> + put_insn(vmovq_to_mem, "vmovq %%xmm1, 32(%0)")
> + :: "d" (NULL) );
> +
> +memset(res, 0xdb, 64);
> +set_insn(vmovq_to_mem);
> +regs.ecx = 0;
> +regs.edx = (unsigned long)res;
> +rc = x86_emulate(&ctxt, &emulops);
> +if ( rc != X86EMUL_OKAY || !check_eip(vmovq_to_mem) ||
> + *((uint64_t *)res + 4) ||
> + memcmp(res, res + 10, 24) ||
> + memcmp(res, res + 6, 8) )
> +goto fail;
> +printf("okay\n");
> +}
> +else
> +printf("skipped\n");
> +
> printf("%-40s", "Testing movdqu %xmm2,(%ecx)...");
> if ( stack_exec && cpu_has_sse2 )
> {
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -269,7 +269,7 @@ static const opcode_desc_t twobyte_table
> ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
> ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
> /* 0xD0 - 0xDF */
> -ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, ModRM,
> +ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, ImplicitOps|ModRM, ModRM,
> ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, ModRM,
> /* 0xE0 - 0xEF */
> ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, ImplicitOps|ModRM,
> @@ -4779,6 +4779,8 @@ x86_emulate(
> case X86EMUL_OPC_F3(0x0f, 0x7f): /* movdqu xmm,xmm/m128 */
> case X86EMUL_OPC_VEX_F3(0x0f, 0x7f): /* vmovdqu xmm,xmm/m128 */
> /* vmovdqu ymm,ymm/m256 */
> +case X86EMUL_OPC_66(0x0f, 0xd6): /* movq xmm,xmm/m64 */
> +case X86EMUL_OPC_VEX_66(0x0f, 0xd6): /* vmovq xmm,xmm/m64 */
> {
> uint8_t *buf = get_stub(stub);
> struct fpu_insn_ctxt fic = { .insn_bytes = 5 };
> @@ -4796,7 +4798,8 @@ x86_emulate(
> case vex_66:
> case vex_f3:
> host_and_vcpu_must_have(sse2);
> -buf[0] = 0x66; /* movdqa */
> +/* Converting movdqu to movdqa here: Our buffer is aligned.
> */
> +buf[0] = 0x66;
> get_fpu(X86EMUL_FPU_xmm, &fic);
> ea.bytes = 16;
> break;
> @@ -4819,6 +4822,11 @@ x86_emulate(
> get_fpu(X86EMUL_FPU_ymm, &fic);
> ea.bytes = 16 << vex.l;
> }
> +if ( b == 0xd6 )
> +{
> +generate_exception_if(vex.l, EXC_UD, -1);
> +ea.bytes = 8;
> +}
> if ( ea.type == OP_MEM )
> {
> generate_exception_if((vex.pfx == vex_66) &&
>
--
Mihai DONȚU
pgpGMsA4XM4H2.pgp
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] BUG_ON() vs ASSERT()
On Tue, 13 Sep 2016 09:10:32 -0400 Konrad Rzeszutek Wilk wrote: > On Mon, Sep 12, 2016 at 09:23:41AM -0600, Jan Beulich wrote: > > All, > > > > in > > https://lists.xenproject.org/archives/html/xen-devel/2016-09/msg01201.html > > and > > https://lists.xenproject.org/archives/html/xen-devel/2016-09/msg01210.html > > Andrew basically suggests that we should switch away from using > > ASSERT() and over to BUG_ON() in perhaps quite broad a set of > > cases. And honestly I'm not convinced of this: We've been adding > > quite a few ASSERT()s over the last years with the aim of doing > > sanity checking in debug builds, without adding overhead to non- > > debug builds. I can certainly see possible cases where using > > BUG_ON() to prevent further possible damage is appropriate, but > > I don't think we should overdo here. > > > > Thanks for other's opinions, > > I am in the mindset that ASSERTS are in the cases where a check > has been done earlier and the ASSERT is more of a catch if we ended up > somehow in the wrong state. We can then slowly follow the breadcrumbs to > see what changed the state. In other words - something that the hypervisor > has checked for and that invariant should have not changed. > > But a BUG_ON is in the same category - it should not have happend. > > Perhaps the distinction is that for ASSERTS() it is to catch me messing > things up. While BUG_ON() is something (or somebody) else messing things up. > > It is kind of hard to describe the semantic of an ASSERT vs BUG_ON now > that I think of it .. I would see ASSERT() used to check for conditions that have immediate and visible consequences, like the ones that lead to lack of functionality (like a hw feature misdetection) or straight crashes (like NULL-dereference). BUG_ON(), on the other hand, would be an early warning for subtle corruptions that can lead to a hypervisor crash or corrupted data after many hours of use (close to impossible to track down). For example, a while ago I posted a small patch that would BUG_ON() when it detected that the heap chunks were not properly linked. That's the type of bug that's a pain to detect. -- Mihai Donțu ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] BUG_ON() vs ASSERT()
On Tue, 13 Sep 2016 19:25:54 +0100 Andrew Cooper wrote: > On 13/09/16 14:46, Mihai Donțu wrote: > > On Tue, 13 Sep 2016 09:10:32 -0400 Konrad Rzeszutek Wilk wrote: > >> On Mon, Sep 12, 2016 at 09:23:41AM -0600, Jan Beulich wrote: > >>> All, > >>> > >>> in > >>> https://lists.xenproject.org/archives/html/xen-devel/2016-09/msg01201.html > >>> and > >>> https://lists.xenproject.org/archives/html/xen-devel/2016-09/msg01210.html > >>> Andrew basically suggests that we should switch away from using > >>> ASSERT() and over to BUG_ON() in perhaps quite broad a set of > >>> cases. And honestly I'm not convinced of this: We've been adding > >>> quite a few ASSERT()s over the last years with the aim of doing > >>> sanity checking in debug builds, without adding overhead to non- > >>> debug builds. I can certainly see possible cases where using > >>> BUG_ON() to prevent further possible damage is appropriate, but > >>> I don't think we should overdo here. > >>> > >>> Thanks for other's opinions, > >> I am in the mindset that ASSERTS are in the cases where a check > >> has been done earlier and the ASSERT is more of a catch if we ended up > >> somehow in the wrong state. We can then slowly follow the breadcrumbs to > >> see what changed the state. In other words - something that the hypervisor > >> has checked for and that invariant should have not changed. > >> > >> But a BUG_ON is in the same category - it should not have happend. > >> > >> Perhaps the distinction is that for ASSERTS() it is to catch me messing > >> things up. While BUG_ON() is something (or somebody) else messing things > >> up. > >> > >> It is kind of hard to describe the semantic of an ASSERT vs BUG_ON now > >> that I think of it .. > > I would see ASSERT() used to check for conditions that have immediate > > and visible consequences, like the ones that lead to lack of > > functionality (like a hw feature misdetection) or straight crashes > > (like NULL-dereference). > > NULL dereferences in the context of PV guests are also a security issue, > as the PV guest controls what is mapped at 0. > > > BUG_ON(), on the other hand, would be an early > > warning for subtle corruptions that can lead to a hypervisor crash or > > corrupted data after many hours of use (close to impossible to track > > down). > > > > For example, a while ago I posted a small patch that would BUG_ON() > > when it detected that the heap chunks were not properly linked. That's > > the type of bug that's a pain to detect. > > Speaking of, what happened to that patch? I did not post and updated version after the last review because I wanted to produce some numbers too (wrt performance impact) ... and I stopped there as I got distracted by other issues. I have a good mind to update it, write a quick test and publish them. I hope I can do this all sometime next week. -- Mihai Donțu ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 2/3] x86/emulate: add support of emulating SSE2 instruction {, v}movd mm, r32/m32 and {, v}movq mm, r64
Found that Windows driver was using a SSE2 instruction MOVD.
Signed-off-by: Zhi Wang
Signed-off-by: Mihai Donțu
---
Picked from the XenServer 7 patch queue, as suggested by Andrew Cooper
Changed since v2:
* handle the case where the destination is a GPR
---
xen/arch/x86/x86_emulate/x86_emulate.c | 38 +++---
1 file changed, 35 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
b/xen/arch/x86/x86_emulate/x86_emulate.c
index 44de3b6..9f89ada 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -204,7 +204,7 @@ static uint8_t twobyte_table[256] = {
/* 0x60 - 0x6F */
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ImplicitOps|ModRM,
/* 0x70 - 0x7F */
-0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ImplicitOps|ModRM,
+0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ImplicitOps|ModRM,
ImplicitOps|ModRM,
/* 0x80 - 0x87 */
ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
@@ -4409,6 +4409,10 @@ x86_emulate(
case 0x6f: /* movq mm/m64,mm */
/* {,v}movdq{a,u} xmm/m128,xmm */
/* vmovdq{a,u} ymm/m256,ymm */
+case 0x7e: /* movd mm,r/m32 */
+ /* movq mm,r/m64 */
+ /* {,v}movd xmm,r/m32 */
+ /* {,v}movq xmm,r/m64 */
case 0x7f: /* movq mm,mm/m64 */
/* {,v}movdq{a,u} xmm,xmm/m128 */
/* vmovdq{a,u} ymm,ymm/m256 */
@@ -4432,7 +4436,17 @@ x86_emulate(
host_and_vcpu_must_have(sse2);
buf[0] = 0x66; /* SSE */
get_fpu(X86EMUL_FPU_xmm, &fic);
-ea.bytes = (b == 0xd6 ? 8 : 16);
+switch ( b )
+{
+case 0x7e:
+ea.bytes = 4;
+break;
+case 0xd6:
+ea.bytes = 8;
+break;
+default:
+ea.bytes = 16;
+}
break;
case vex_none:
if ( b != 0xe7 )
@@ -4452,7 +4466,17 @@ x86_emulate(
((vex.pfx != vex_66) && (vex.pfx != vex_f3)));
host_and_vcpu_must_have(avx);
get_fpu(X86EMUL_FPU_ymm, &fic);
-ea.bytes = (b == 0xd6 ? 8 : (16 << vex.l));
+switch ( b )
+{
+case 0x7e:
+ea.bytes = 4;
+break;
+case 0xd6:
+ea.bytes = 8;
+break;
+default:
+ea.bytes = 16 << vex.l;
+}
}
if ( ea.type == OP_MEM )
{
@@ -4468,6 +4492,14 @@ x86_emulate(
vex.b = 1;
buf[4] &= 0x38;
}
+else if ( b == 0x7e )
+{
+/* convert the GPR destination to (%rAX) */
+*((unsigned long *)&mmvalp) = (unsigned long)ea.reg;
+rex_prefix &= ~REX_B;
+vex.b = 1;
+buf[4] &= 0x38;
+}
if ( !rc )
{
copy_REX_VEX(buf, rex_prefix, vex);
--
2.9.2
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 3/3] x86/emulate: added tests for {, v}movd mm, r32/m32 and {, v}movq xmm, r64/m64
Signed-off-by: Mihai Donțu
---
Changed since v2:
* added tests for {,v}movq xmm,r64
---
tools/tests/x86_emulator/test_x86_emulator.c | 120 +++
1 file changed, 120 insertions(+)
diff --git a/tools/tests/x86_emulator/test_x86_emulator.c
b/tools/tests/x86_emulator/test_x86_emulator.c
index 8994149..fb59b0f 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -650,6 +650,88 @@ int main(int argc, char **argv)
#define check_eip(which) (regs.eip == (unsigned long)instr + \
(unsigned long)which##_len)
+printf("%-40s", "Testing movd %%mm3,32(%%eax)...");
+if ( stack_exec && cpu_has_mmx )
+{
+decl_insn(movd_to_mem32);
+
+asm volatile ( "pcmpeqb %%mm3, %%mm3\n"
+ put_insn(movd_to_mem32, "movd %%mm3, 32(%%eax)")
+ :: );
+
+*(res + 8) = 0xbdbdbdbd;
+set_insn(movd_to_mem32);
+regs.eax = (unsigned long)res;
+rc = x86_emulate(&ctxt, &emulops);
+if ( (rc != X86EMUL_OKAY) || *(res + 8) != 0x ||
+ !check_eip(movd_to_mem32) )
+goto fail;
+printf("okay\n");
+}
+else
+printf("skipped\n");
+
+printf("%-40s", "Testing movd %%mm3,%%eax...");
+if ( stack_exec && cpu_has_mmx )
+{
+decl_insn(movd_to_reg);
+
+asm volatile ( "pcmpeqb %%mm3, %%mm3\n"
+ put_insn(movd_to_reg, "movd %%mm3, %%eax")
+ :: );
+
+set_insn(movd_to_reg);
+regs.rax = 0xbdbdbdbdbdbdbdbd;
+rc = x86_emulate(&ctxt, &emulops);
+if ( (rc != X86EMUL_OKAY) || regs.eax != 0xbdbdbdbd ||
+ !check_eip(movd_to_reg) )
+goto fail;
+printf("okay\n");
+}
+else
+printf("skipped\n");
+
+printf("%-40s", "Testing vmovd %%xmm1,32(%%eax)...");
+if ( stack_exec && cpu_has_avx )
+{
+decl_insn(vmovd_to_mem32);
+
+asm volatile ( "pcmpeqb %%xmm1, %%xmm1\n"
+ put_insn(vmovd_to_mem32, "vmovd %%xmm1, 32(%%eax)")
+ :: );
+
+*(res + 8) = 0xbdbdbdbd;
+set_insn(vmovd_to_mem32);
+regs.eax = (unsigned long)res;
+rc = x86_emulate(&ctxt, &emulops);
+if ( (rc != X86EMUL_OKAY) || *(res + 8) != 0x ||
+ !check_eip(vmovd_to_mem32) )
+goto fail;
+printf("okay\n");
+}
+else
+printf("skipped\n");
+
+printf("%-40s", "Testing vmovd %%xmm1,%%eax...");
+if ( stack_exec && cpu_has_avx )
+{
+decl_insn(vmovd_to_reg);
+
+asm volatile ( "pcmpeqb %%xmm1, %%xmm1\n"
+ put_insn(vmovd_to_reg, "vmovd %%xmm1, %%eax")
+ :: );
+
+set_insn(vmovd_to_reg);
+regs.rax = 0xbdbdbdbdbdbdbdbd;
+rc = x86_emulate(&ctxt, &emulops);
+if ( (rc != X86EMUL_OKAY) || regs.rax != 0xbdbdbdbd ||
+ !check_eip(vmovd_to_reg) )
+goto fail;
+printf("okay\n");
+}
+else
+printf("skipped\n");
+
printf("%-40s", "Testing movq %mm3,(%ecx)...");
if ( stack_exec && cpu_has_mmx )
{
@@ -719,6 +801,25 @@ int main(int argc, char **argv)
else
printf("skipped\n");
+printf("%-40s", "Testing movq %%xmm0,%%rax...");
+if ( stack_exec && cpu_has_sse )
+{
+decl_insn(movq_to_reg);
+
+asm volatile ( "pcmpgtb %%xmm0, %%xmm0\n"
+ put_insn(movq_to_reg, "movq %%xmm0, %%rax")
+ :: );
+
+set_insn(movq_to_reg);
+regs.rax = 0xbdbdbdbdbdbdbdbd;
+rc = x86_emulate(&ctxt, &emulops);
+if ( rc != X86EMUL_OKAY || regs.rax || !check_eip(movq_to_reg) )
+goto fail;
+printf("okay\n");
+}
+else
+printf("skipped\n");
+
printf("%-40s", "Testing vmovq %%xmm1,32(%%eax)...");
if ( stack_exec && cpu_has_avx )
{
@@ -741,6 +842,25 @@ int main(int argc, char **argv)
else
printf("skipped\n");
+printf("%-40s", "Testing vmovq %%xmm1,%%rax...");
+if ( stack_exec && cpu_has_avx )
+{
+decl_insn(vmovq_to_reg);
+
+asm volatile ( "pcmpgtb %%xmm1, %%xmm1\n"
+ put_insn(vmovq_to_reg, "vmovq %%xmm1, %%rax")
+ :: );
+
+set_insn(vmovq_to_reg);
+regs.rax = 0xbdbdbdbdbdbdbdbd;
+rc = x86_emulate(&ctxt, &emulops);
+if ( rc != X86EMUL_OKAY || regs.rax || !check_eip(vmovq_to_reg) )
+goto fail;
+printf("okay\n");
+}
+else
+printf("skipped\n");
+
printf("%-40s", "Testing movdqu %xmm2,(%ecx)...");
if ( stack_exec && cpu_has_sse2 )
{
--
2.9.2
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 1/3] x86/emulate: add support for {, v}movq xmm, xmm/m64
Signed-off-by: Mihai Donțu
---
Changed since v2:
* minor change to make it more obvious what the code does
Changed since v1:
* added a test for vmovq
* made the tests depend on SSE and AVX, respectively
* added emulator support for vmovq (0xd6 forces the operand size to
64bit)
---
tools/tests/x86_emulator/test_x86_emulator.c | 44
xen/arch/x86/x86_emulate/x86_emulate.c | 9 +++---
2 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/tools/tests/x86_emulator/test_x86_emulator.c
b/tools/tests/x86_emulator/test_x86_emulator.c
index c7f572a..8994149 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -697,6 +697,50 @@ int main(int argc, char **argv)
else
printf("skipped\n");
+printf("%-40s", "Testing movq %%xmm0,32(%%eax)...");
+if ( stack_exec && cpu_has_sse )
+{
+decl_insn(movq_to_mem2);
+
+asm volatile ( "pcmpgtb %%xmm0, %%xmm0\n"
+ put_insn(movq_to_mem2, "movq %%xmm0, 32(%%eax)")
+ :: );
+
+*((unsigned long *)res + 4) = 0xbdbdbdbdbdbdbdbd;
+set_insn(movq_to_mem2);
+regs.eax = (unsigned long)res;
+rc = x86_emulate(&ctxt, &emulops);
+if ( rc != X86EMUL_OKAY || !check_eip(movq_to_mem2) )
+goto fail;
+if ( *((unsigned long *)res + 4) )
+goto fail;
+printf("okay\n");
+}
+else
+printf("skipped\n");
+
+printf("%-40s", "Testing vmovq %%xmm1,32(%%eax)...");
+if ( stack_exec && cpu_has_avx )
+{
+decl_insn(vmovq_to_mem);
+
+asm volatile ( "pcmpgtb %%xmm1, %%xmm1\n"
+ put_insn(vmovq_to_mem, "vmovq %%xmm1, 32(%%eax)")
+ :: );
+
+*((unsigned long *)res + 4) = 0xbdbdbdbdbdbdbdbd;
+set_insn(vmovq_to_mem);
+regs.eax = (unsigned long)res;
+rc = x86_emulate(&ctxt, &emulops);
+if ( rc != X86EMUL_OKAY || !check_eip(vmovq_to_mem) )
+goto fail;
+if ( *((unsigned long *)res + 4) )
+goto fail;
+printf("okay\n");
+}
+else
+printf("skipped\n");
+
printf("%-40s", "Testing movdqu %xmm2,(%ecx)...");
if ( stack_exec && cpu_has_sse2 )
{
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
b/xen/arch/x86/x86_emulate/x86_emulate.c
index fe594ba..44de3b6 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -245,7 +245,7 @@ static uint8_t twobyte_table[256] = {
ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
/* 0xD0 - 0xDF */
-0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+0, 0, 0, 0, 0, 0, ImplicitOps|ModRM, 0, 0, 0, 0, 0, 0, 0, 0, 0,
/* 0xE0 - 0xEF */
0, 0, 0, 0, 0, 0, 0, ImplicitOps|ModRM, 0, 0, 0, 0, 0, 0, 0, 0,
/* 0xF0 - 0xFF */
@@ -4412,6 +4412,7 @@ x86_emulate(
case 0x7f: /* movq mm,mm/m64 */
/* {,v}movdq{a,u} xmm,xmm/m128 */
/* vmovdq{a,u} ymm,ymm/m256 */
+case 0xd6: /* {,v}movq xmm,xmm/m64 */
{
uint8_t *buf = get_stub(stub);
struct fpu_insn_ctxt fic = { .insn_bytes = 5 };
@@ -4429,9 +4430,9 @@ x86_emulate(
case vex_66:
case vex_f3:
host_and_vcpu_must_have(sse2);
-buf[0] = 0x66; /* movdqa */
+buf[0] = 0x66; /* SSE */
get_fpu(X86EMUL_FPU_xmm, &fic);
-ea.bytes = 16;
+ea.bytes = (b == 0xd6 ? 8 : 16);
break;
case vex_none:
if ( b != 0xe7 )
@@ -4451,7 +4452,7 @@ x86_emulate(
((vex.pfx != vex_66) && (vex.pfx != vex_f3)));
host_and_vcpu_must_have(avx);
get_fpu(X86EMUL_FPU_ymm, &fic);
-ea.bytes = 16 << vex.l;
+ea.bytes = (b == 0xd6 ? 8 : (16 << vex.l));
}
if ( ea.type == OP_MEM )
{
--
2.9.2
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 3/3] x86/emulate: added tests for {, v}movd mm, r32/m32 and {, v}movq xmm, r64/m64
On Monday 01 August 2016 10:54:10 Andrew Cooper wrote:
> On 01/08/16 03:52, Mihai Donțu wrote:
> > Signed-off-by: Mihai Donțu
> > ---
> > Changed since v2:
> > * added tests for {,v}movq xmm,r64
> > ---
> > tools/tests/x86_emulator/test_x86_emulator.c | 120
> > +++
> > 1 file changed, 120 insertions(+)
> >
> > diff --git a/tools/tests/x86_emulator/test_x86_emulator.c
> > b/tools/tests/x86_emulator/test_x86_emulator.c
> > index 8994149..fb59b0f 100644
> > --- a/tools/tests/x86_emulator/test_x86_emulator.c
> > +++ b/tools/tests/x86_emulator/test_x86_emulator.c
> > @@ -650,6 +650,88 @@ int main(int argc, char **argv)
> > #define check_eip(which) (regs.eip == (unsigned long)instr + \
> >(unsigned long)which##_len)
> >
> > +printf("%-40s", "Testing movd %%mm3,32(%%eax)...");
> > +if ( stack_exec && cpu_has_mmx )
> > +{
> > +decl_insn(movd_to_mem32);
> > +
> > +asm volatile ( "pcmpeqb %%mm3, %%mm3\n"
> > + put_insn(movd_to_mem32, "movd %%mm3, 32(%%eax)")
> > + :: );
> > +
> > +*(res + 8) = 0xbdbdbdbd;
> > +set_insn(movd_to_mem32);
> > +regs.eax = (unsigned long)res;
> > +rc = x86_emulate(&ctxt, &emulops);
> > +if ( (rc != X86EMUL_OKAY) || *(res + 8) != 0x ||
> > + !check_eip(movd_to_mem32) )
> > +goto fail;
> > +printf("okay\n");
> > +}
> > +else
> > + printf("skipped\n");
> > +
> > +printf("%-40s", "Testing movd %%mm3,%%eax...");
>
> Could we possibly change this to %ebx instead of %eax?
Sure thing!
> That case is far more likely to go bang in a debug build if the emulator
> is wrong.
>
> Otherwise, Revewed-by: Andrew Cooper
--
Mihai DONȚU
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/3] x86/emulate: add support of emulating SSE2 instruction {, v}movd mm, r32/m32 and {, v}movq mm, r64
On Monday 01 August 2016 10:52:12 Andrew Cooper wrote:
> On 01/08/16 03:52, Mihai Donțu wrote:
> > Found that Windows driver was using a SSE2 instruction MOVD.
> >
> > Signed-off-by: Zhi Wang
> > Signed-off-by: Mihai Donțu
> > ---
> > Picked from the XenServer 7 patch queue, as suggested by Andrew Cooper
> >
> > Changed since v2:
> > * handle the case where the destination is a GPR
> > ---
> > xen/arch/x86/x86_emulate/x86_emulate.c | 38
> > +++---
> > 1 file changed, 35 insertions(+), 3 deletions(-)
> >
> > diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
> > b/xen/arch/x86/x86_emulate/x86_emulate.c
> > index 44de3b6..9f89ada 100644
> > --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> > +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> > @@ -204,7 +204,7 @@ static uint8_t twobyte_table[256] = {
> > /* 0x60 - 0x6F */
> > 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ImplicitOps|ModRM,
> > /* 0x70 - 0x7F */
> > -0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ImplicitOps|ModRM,
> > +0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ImplicitOps|ModRM,
> > ImplicitOps|ModRM,
> > /* 0x80 - 0x87 */
> > ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
> > ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
> > @@ -4409,6 +4409,10 @@ x86_emulate(
> > case 0x6f: /* movq mm/m64,mm */
> > /* {,v}movdq{a,u} xmm/m128,xmm */
> > /* vmovdq{a,u} ymm/m256,ymm */
> > +case 0x7e: /* movd mm,r/m32 */
> > + /* movq mm,r/m64 */
> > + /* {,v}movd xmm,r/m32 */
> > + /* {,v}movq xmm,r/m64 */
> > case 0x7f: /* movq mm,mm/m64 */
> > /* {,v}movdq{a,u} xmm,xmm/m128 */
> > /* vmovdq{a,u} ymm,ymm/m256 */
> > @@ -4432,7 +4436,17 @@ x86_emulate(
> > host_and_vcpu_must_have(sse2);
> > buf[0] = 0x66; /* SSE */
> > get_fpu(X86EMUL_FPU_xmm, &fic);
> > -ea.bytes = (b == 0xd6 ? 8 : 16);
> > +switch ( b )
> > +{
> > +case 0x7e:
> > +ea.bytes = 4;
> > +break;
> > +case 0xd6:
> > +ea.bytes = 8;
> > +break;
> > +default:
> > +ea.bytes = 16;
> > +}
> > break;
> > case vex_none:
> > if ( b != 0xe7 )
> > @@ -4452,7 +4466,17 @@ x86_emulate(
> > ((vex.pfx != vex_66) && (vex.pfx != vex_f3)));
> > host_and_vcpu_must_have(avx);
> > get_fpu(X86EMUL_FPU_ymm, &fic);
> > -ea.bytes = (b == 0xd6 ? 8 : (16 << vex.l));
> > +switch ( b )
> > +{
> > +case 0x7e:
> > +ea.bytes = 4;
> > +break;
> > +case 0xd6:
> > +ea.bytes = 8;
> > +break;
> > +default:
> > +ea.bytes = 16 << vex.l;
> > +}
> > }
> > if ( ea.type == OP_MEM )
> > {
> > @@ -4468,6 +4492,14 @@ x86_emulate(
> > vex.b = 1;
> > buf[4] &= 0x38;
> > }
> > +else if ( b == 0x7e )
> > +{
> > +/* convert the GPR destination to (%rAX) */
> > +*((unsigned long *)&mmvalp) = (unsigned long)ea.reg;
> > +rex_prefix &= ~REX_B;
> > +vex.b = 1;
> > +buf[4] &= 0x38;
> > +}
>
> Thankyou for doing this. However, looking at it, it has some code in
> common with the "ea.type == OP_MEM" clause.
>
> Would this work?
>
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
> b/xen/arch/x86/x86_emulate/x86_emulate.c
> index fe594ba..90db067 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -4453,16 +4453,25 @@ x86_emulate(
> get_fpu(X86EMUL_FPU_ymm, &fic);
> ea.bytes = 16 << vex.l;
> }
> -if ( ea.type == OP_MEM )
> +if ( ea.type == OP_MEM || ea.type == OP_REG )
> {
> -/* XXX enable once there is ops->ea() or equivalent
> -generate_exception_if((vex.pfx == vex_66) &&
> -
Re: [Xen-devel] [PATCH v3 2/3] x86/emulate: add support of emulating SSE2 instruction {, v}movd mm, r32/m32 and {, v}movq mm, r64
On Monday 01 August 2016 15:53:27 Mihai Donțu wrote:
> On Monday 01 August 2016 10:52:12 Andrew Cooper wrote:
> > On 01/08/16 03:52, Mihai Donțu wrote:
> > > Found that Windows driver was using a SSE2 instruction MOVD.
> > >
> > > Signed-off-by: Zhi Wang
> > > Signed-off-by: Mihai Donțu
> > > ---
> > > Picked from the XenServer 7 patch queue, as suggested by Andrew Cooper
> > >
> > > Changed since v2:
> > > * handle the case where the destination is a GPR
> > > ---
> > > xen/arch/x86/x86_emulate/x86_emulate.c | 38
> > > +++---
> > > 1 file changed, 35 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
> > > b/xen/arch/x86/x86_emulate/x86_emulate.c
> > > index 44de3b6..9f89ada 100644
> > > --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> > > +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> > > @@ -204,7 +204,7 @@ static uint8_t twobyte_table[256] = {
> > > /* 0x60 - 0x6F */
> > > 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ImplicitOps|ModRM,
> > > /* 0x70 - 0x7F */
> > > -0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ImplicitOps|ModRM,
> > > +0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ImplicitOps|ModRM,
> > > ImplicitOps|ModRM,
> > > /* 0x80 - 0x87 */
> > > ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
> > > ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
> > > @@ -4409,6 +4409,10 @@ x86_emulate(
> > > case 0x6f: /* movq mm/m64,mm */
> > > /* {,v}movdq{a,u} xmm/m128,xmm */
> > > /* vmovdq{a,u} ymm/m256,ymm */
> > > +case 0x7e: /* movd mm,r/m32 */
> > > + /* movq mm,r/m64 */
> > > + /* {,v}movd xmm,r/m32 */
> > > + /* {,v}movq xmm,r/m64 */
> > > case 0x7f: /* movq mm,mm/m64 */
> > > /* {,v}movdq{a,u} xmm,xmm/m128 */
> > > /* vmovdq{a,u} ymm,ymm/m256 */
> > > @@ -4432,7 +4436,17 @@ x86_emulate(
> > > host_and_vcpu_must_have(sse2);
> > > buf[0] = 0x66; /* SSE */
> > > get_fpu(X86EMUL_FPU_xmm, &fic);
> > > -ea.bytes = (b == 0xd6 ? 8 : 16);
> > > +switch ( b )
> > > +{
> > > +case 0x7e:
> > > +ea.bytes = 4;
> > > +break;
> > > +case 0xd6:
> > > +ea.bytes = 8;
> > > +break;
> > > +default:
> > > +ea.bytes = 16;
> > > +}
> > > break;
> > > case vex_none:
> > > if ( b != 0xe7 )
> > > @@ -4452,7 +4466,17 @@ x86_emulate(
> > > ((vex.pfx != vex_66) && (vex.pfx != vex_f3)));
> > > host_and_vcpu_must_have(avx);
> > > get_fpu(X86EMUL_FPU_ymm, &fic);
> > > -ea.bytes = (b == 0xd6 ? 8 : (16 << vex.l));
> > > +switch ( b )
> > > +{
> > > +case 0x7e:
> > > +ea.bytes = 4;
> > > +break;
> > > +case 0xd6:
> > > +ea.bytes = 8;
> > > +break;
> > > +default:
> > > +ea.bytes = 16 << vex.l;
> > > +}
> > > }
> > > if ( ea.type == OP_MEM )
> > > {
> > > @@ -4468,6 +4492,14 @@ x86_emulate(
> > > vex.b = 1;
> > > buf[4] &= 0x38;
> > > }
> > > +else if ( b == 0x7e )
> > > +{
> > > +/* convert the GPR destination to (%rAX) */
> > > +*((unsigned long *)&mmvalp) = (unsigned long)ea.reg;
> > > +rex_prefix &= ~REX_B;
> > > +vex.b = 1;
> > > +buf[4] &= 0x38;
> > > +}
> >
> > Thankyou for doing this. However, looking at it, it has some code in
> > common with the "ea.type == OP_MEM" clause.
> >
> > Would this work?
> >
> > diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
> > b/xen/arch/x86/x86_emulate/x86_emulate.c
>
Re: [Xen-devel] [PATCH v3 1/3] x86/emulate: add support for {, v}movq xmm, xmm/m64
On Monday 01 August 2016 07:19:51 Jan Beulich wrote:
> >>> On 01.08.16 at 04:52, wrote:
> > @@ -4412,6 +4412,7 @@ x86_emulate(
> > case 0x7f: /* movq mm,mm/m64 */
> > /* {,v}movdq{a,u} xmm,xmm/m128 */
> > /* vmovdq{a,u} ymm,ymm/m256 */
> > +case 0xd6: /* {,v}movq xmm,xmm/m64 */
> > {
> > uint8_t *buf = get_stub(stub);
> > struct fpu_insn_ctxt fic = { .insn_bytes = 5 };
> > @@ -4429,9 +4430,9 @@ x86_emulate(
> > case vex_66:
> > case vex_f3:
> > host_and_vcpu_must_have(sse2);
> > -buf[0] = 0x66; /* movdqa */
> > +buf[0] = 0x66; /* SSE */
>
> The comment change here indicates a problem: So far it was indicating
> that despite the possible F3 prefix (movdqu) we encode a 66 one
> (movdqa). Opcode D6 prefixed with F3, however, is movq2dq, which
> you then either don't emulate correctly, or if it happens to be
> emulated correctly you should include in the comment accompanying
> the case label. And its AVX counterpart should then produce #UD.
I see. Thanks for the hint. I'll try to write a test case and see if it
"just works" or needs extra handling.
--
Mihai DONȚU
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/3] x86/emulate: add support of emulating SSE2 instruction {, v}movd mm, r32/m32 and {, v}movq mm, r64
On Monday 01 August 2016 06:59:08 Jan Beulich wrote:
> >>> On 01.08.16 at 14:53, wrote:
> > On Monday 01 August 2016 10:52:12 Andrew Cooper wrote:
> >> On 01/08/16 03:52, Mihai Donțu wrote:
> >> > Found that Windows driver was using a SSE2 instruction MOVD.
> >> >
> >> > Signed-off-by: Zhi Wang
> >> > Signed-off-by: Mihai Donțu
> >> > ---
> >> > Picked from the XenServer 7 patch queue, as suggested by Andrew Cooper
> >> >
> >> > Changed since v2:
> >> > * handle the case where the destination is a GPR
> >> > ---
> >> > xen/arch/x86/x86_emulate/x86_emulate.c | 38
> > +++---
> >> > 1 file changed, 35 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
> > b/xen/arch/x86/x86_emulate/x86_emulate.c
> >> > index 44de3b6..9f89ada 100644
> >> > --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> >> > +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> >> > @@ -204,7 +204,7 @@ static uint8_t twobyte_table[256] = {
> >> > /* 0x60 - 0x6F */
> >> > 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ImplicitOps|ModRM,
> >> > /* 0x70 - 0x7F */
> >> > -0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ImplicitOps|ModRM,
> >> > +0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ImplicitOps|ModRM,
> > ImplicitOps|ModRM,
> >> > /* 0x80 - 0x87 */
> >> > ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
> >> > ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
> >> > @@ -4409,6 +4409,10 @@ x86_emulate(
> >> > case 0x6f: /* movq mm/m64,mm */
> >> > /* {,v}movdq{a,u} xmm/m128,xmm */
> >> > /* vmovdq{a,u} ymm/m256,ymm */
> >> > +case 0x7e: /* movd mm,r/m32 */
> >> > + /* movq mm,r/m64 */
> >> > + /* {,v}movd xmm,r/m32 */
> >> > + /* {,v}movq xmm,r/m64 */
> >> > case 0x7f: /* movq mm,mm/m64 */
> >> > /* {,v}movdq{a,u} xmm,xmm/m128 */
> >> > /* vmovdq{a,u} ymm,ymm/m256 */
> >> > @@ -4432,7 +4436,17 @@ x86_emulate(
> >> > host_and_vcpu_must_have(sse2);
> >> > buf[0] = 0x66; /* SSE */
> >> > get_fpu(X86EMUL_FPU_xmm, &fic);
> >> > -ea.bytes = (b == 0xd6 ? 8 : 16);
> >> > +switch ( b )
> >> > +{
> >> > +case 0x7e:
> >> > +ea.bytes = 4;
> >> > +break;
> >> > +case 0xd6:
> >> > +ea.bytes = 8;
> >> > +break;
> >> > +default:
> >> > +ea.bytes = 16;
> >> > +}
> >> > break;
> >> > case vex_none:
> >> > if ( b != 0xe7 )
> >> > @@ -4452,7 +4466,17 @@ x86_emulate(
> >> > ((vex.pfx != vex_66) && (vex.pfx != vex_f3)));
> >> > host_and_vcpu_must_have(avx);
> >> > get_fpu(X86EMUL_FPU_ymm, &fic);
> >> > -ea.bytes = (b == 0xd6 ? 8 : (16 << vex.l));
> >> > +switch ( b )
> >> > +{
> >> > +case 0x7e:
> >> > +ea.bytes = 4;
> >> > +break;
> >> > +case 0xd6:
> >> > +ea.bytes = 8;
> >> > +break;
> >> > +default:
> >> > +ea.bytes = 16 << vex.l;
> >> > +}
> >> > }
> >> > if ( ea.type == OP_MEM )
> >> > {
> >> > @@ -4468,6 +4492,14 @@ x86_emulate(
> >> > vex.b = 1;
> >> > buf[4] &= 0x38;
> >> > }
> >> > +else if ( b == 0x7e )
> >> > +{
> >> > +/* convert the GPR destination to (%rAX) */
> >> > +*((unsigned long *)&mmvalp) = (unsigned long)ea.reg;
> >> > +rex_prefix &= ~REX_B;
> >> > +vex.b = 1;
>
Re: [Xen-devel] [PATCH v3 2/3] x86/emulate: add support of emulating SSE2 instruction {, v}movd mm, r32/m32 and {, v}movq mm, r64
On Monday 01 August 2016 07:43:20 Jan Beulich wrote: > > > > Your suggestion makes sense, but I'm starting to doubt my initial > > > > patch. :-) I'm testing "movq xmm1, xmm1" and noticing that it takes the > > > > GPR-handling route and I can't seem to be able to easily prevent it > > > > with !(rex_prefix & REX_B), as rex_prefix == 0 and vex.b == 1. I need > > > > to take a harder look at how that class of instructions is coded. > > > > > > You obviously need to distinguish the two kinds of register sources/ > > > destinations: GPRs need suitable re-writing of the instruction (without > > > having looked at the most recent version of the patch yet I btw doubt > > > converting register to memory operands is the most efficient model), > > > while MMs, XMMs, and YMMs can retain their register encoding. > > > > Regarding efficiency, I'm not married with the approach I've proposed. > > If you can give me a few more hints, I can give it a try. > > I'd rather pick a fixed register and update the regs->... field from that > after the stub was executed. E.g. using rAX and treating it just like a > return value of the "call". But maybe I'm imagining this easier than it > really is; as an alternative I'd then suggest really following what Andrew > said - use a pointer into regs->, not mmvalp. But (as said in the review > mail) you'd then have the problem of the missing zero-extension for > writes to 32-bit GPRs I thought that by re-using (hijacking, really) mmvalp, the patch will look less intrusive and thus not add too much to an already complex code. Assuming I'll just pass to the stub "a"(ea.reg), would it be a good idea to just zero-out the 64bit register before that? It does not appear to be any instructions that write just the low dword. Or am I misunderstanding the zero-extension concept? -- Mihai DONȚU ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/3] x86/emulate: add support of emulating SSE2 instruction {, v}movd mm, r32/m32 and {, v}movq mm, r64
On Monday 01 August 2016 17:48:33 Mihai Donțu wrote: > On Monday 01 August 2016 07:43:20 Jan Beulich wrote: > > > > > Your suggestion makes sense, but I'm starting to doubt my initial > > > > > patch. :-) I'm testing "movq xmm1, xmm1" and noticing that it takes > > > > > the > > > > > GPR-handling route and I can't seem to be able to easily prevent it > > > > > with !(rex_prefix & REX_B), as rex_prefix == 0 and vex.b == 1. I need > > > > > to take a harder look at how that class of instructions is coded. > > > > > > > > > > > > > You obviously need to distinguish the two kinds of register sources/ > > > > destinations: GPRs need suitable re-writing of the instruction (without > > > > having looked at the most recent version of the patch yet I btw doubt > > > > converting register to memory operands is the most efficient model), > > > > while MMs, XMMs, and YMMs can retain their register encoding. > > > > > > Regarding efficiency, I'm not married with the approach I've proposed. > > > If you can give me a few more hints, I can give it a try. > > > > I'd rather pick a fixed register and update the regs->... field from that > > after the stub was executed. E.g. using rAX and treating it just like a > > return value of the "call". But maybe I'm imagining this easier than it > > really is; as an alternative I'd then suggest really following what Andrew > > said - use a pointer into regs->, not mmvalp. But (as said in the review > > mail) you'd then have the problem of the missing zero-extension for > > writes to 32-bit GPRs > > I thought that by re-using (hijacking, really) mmvalp, the patch will > look less intrusive and thus not add too much to an already complex > code. > > Assuming I'll just pass to the stub "a"(ea.reg), would it be a good > idea to just zero-out the 64bit register before that? It does not > appear to be any instructions that write just the low dword. Or am I > misunderstanding the zero-extension concept? > Just to be sure I'm making myself understood, ea.reg contains the output of decode_register() which, in turn, returns a pointer in regs. -- Mihai DONȚU ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/3] x86/emulate: add support of emulating SSE2 instruction {, v}movd mm, r32/m32 and {, v}movq mm, r64
On Monday 01 August 2016 15:53:56 Andrew Cooper wrote: > On 01/08/16 15:48, Mihai Donțu wrote: > > > I'd rather pick a fixed register and update the regs->... field from that > > > after the stub was executed. E.g. using rAX and treating it just like a > > > return value of the "call". But maybe I'm imagining this easier than it > > > really is; as an alternative I'd then suggest really following what Andrew > > > said - use a pointer into regs->, not mmvalp. But (as said in the review > > > mail) you'd then have the problem of the missing zero-extension for > > > writes to 32-bit GPRs > > > > I thought that by re-using (hijacking, really) mmvalp, the patch will > > look less intrusive and thus not add too much to an already complex > > code. > > > > Assuming I'll just pass to the stub "a"(ea.reg), would it be a good > > idea to just zero-out the 64bit register before that? It does not > > appear to be any instructions that write just the low dword. Or am I > > misunderstanding the zero-extension concept? > > Any write to a 32bit GPR zero extends it to 64 bits. This is specified > by AMD64 and only applies to 32bit writes. 8 and 16 bit writes leave > the upper bits alone. > > Therefore movd %mm, %eax will move 32bits from %mm to eax, then zero > extend the upper 32 bits of %rax. ... and given that the stub has (%rAX) as destination, the zero-extension is not implicit and I have to do it myself. Which also means two of my tests in a previous patch are wrong (0xbdbdbdbd checks). Darn! :-) -- Mihai DONȚU pgptzOnLAl5Rd.pgp Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 1/3] x86/emulate: add support for {, v}movq xmm, xmm/m64
On Monday 01 August 2016 07:19:51 Jan Beulich wrote:
> >>> On 01.08.16 at 04:52, wrote:
> > @@ -4412,6 +4412,7 @@ x86_emulate(
> > case 0x7f: /* movq mm,mm/m64 */
> > /* {,v}movdq{a,u} xmm,xmm/m128 */
> > /* vmovdq{a,u} ymm,ymm/m256 */
> > +case 0xd6: /* {,v}movq xmm,xmm/m64 */
> > {
> > uint8_t *buf = get_stub(stub);
> > struct fpu_insn_ctxt fic = { .insn_bytes = 5 };
> > @@ -4429,9 +4430,9 @@ x86_emulate(
> > case vex_66:
> > case vex_f3:
> > host_and_vcpu_must_have(sse2);
> > -buf[0] = 0x66; /* movdqa */
> > +buf[0] = 0x66; /* SSE */
>
> The comment change here indicates a problem: So far it was indicating
> that despite the possible F3 prefix (movdqu) we encode a 66 one
> (movdqa). Opcode D6 prefixed with F3, however, is movq2dq, which
> you then either don't emulate correctly, or if it happens to be
> emulated correctly you should include in the comment accompanying
> the case label. And its AVX counterpart should then produce #UD.
I fiddled with this for a while and the attached patch (adjusted)
appears to be doing the right thing: ie. movq2dq gets emulated
correctly too. copy_REX_VEX() does not work OK with movq2dq, but it
looked easy to single out this case.
All tests pass, including for {,v}movq xmm/m64 and movq2dq. There does
not appear to be an AVX variant for the latter, or I'm not reading the
Intel SDM right (or binutils' as is lying to me).
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
b/xen/arch/x86/x86_emulate/x86_emulate.c
index fe594ba..d6c199b 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -245,7 +245,7 @@ static uint8_t twobyte_table[256] = {
ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
/* 0xD0 - 0xDF */
-0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+0, 0, 0, 0, 0, 0, ImplicitOps|ModRM, 0, 0, 0, 0, 0, 0, 0, 0, 0,
/* 0xE0 - 0xEF */
0, 0, 0, 0, 0, 0, 0, ImplicitOps|ModRM, 0, 0, 0, 0, 0, 0, 0, 0,
/* 0xF0 - 0xFF */
@@ -4412,6 +4412,8 @@ x86_emulate(
case 0x7f: /* movq mm,mm/m64 */
/* {,v}movdq{a,u} xmm,xmm/m128 */
/* vmovdq{a,u} ymm,ymm/m256 */
+case 0xd6: /* {,v}movq xmm,xmm/m64 */
+ /* movq2dq mm,xmm */
{
uint8_t *buf = get_stub(stub);
struct fpu_insn_ctxt fic = { .insn_bytes = 5 };
@@ -4431,7 +4433,7 @@ x86_emulate(
host_and_vcpu_must_have(sse2);
buf[0] = 0x66; /* movdqa */
get_fpu(X86EMUL_FPU_xmm, &fic);
-ea.bytes = 16;
+ea.bytes = (b == 0xd6 ? 8 : 16);
break;
case vex_none:
if ( b != 0xe7 )
@@ -4451,7 +4453,7 @@ x86_emulate(
((vex.pfx != vex_66) && (vex.pfx != vex_f3)));
host_and_vcpu_must_have(avx);
get_fpu(X86EMUL_FPU_ymm, &fic);
-ea.bytes = 16 << vex.l;
+ea.bytes = (b == 0xd6 ? 8 : (16 << vex.l));
}
if ( ea.type == OP_MEM )
{
@@ -4469,7 +4471,11 @@ x86_emulate(
}
if ( !rc )
{
- copy_REX_VEX(buf, rex_prefix, vex);
+ /* try to preserve the mandatory prefix for movq2dq */
+ if ( !rex_prefix && vex.opcx == vex_none && vex.pfx == vex_f3 )
+ buf[0] = 0xf3;
+ else
+ copy_REX_VEX(buf, rex_prefix, vex);
asm volatile ( "call *%0" : : "r" (stub.func), "a" (mmvalp)
: "memory" );
}
--
Mihai DONȚU
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 1/3] x86/emulate: add support for {, v}movq xmm, xmm/m64
On Tuesday 02 August 2016 00:19:22 Jan Beulich wrote:
> > > > On 02.08.16 at 01:19, wrote:
> > > > @@ -4412,6 +4412,7 @@ x86_emulate(
> > > > case 0x7f: /* movq mm,mm/m64 */
> > > > /* {,v}movdq{a,u} xmm,xmm/m128 */
> > > > /* vmovdq{a,u} ymm,ymm/m256 */
> > > > +case 0xd6: /* {,v}movq xmm,xmm/m64 */
> > > > {
> > > > uint8_t *buf = get_stub(stub);
> > > > struct fpu_insn_ctxt fic = { .insn_bytes = 5 };
> > > > @@ -4429,9 +4430,9 @@ x86_emulate(
> > > > case vex_66:
> > > > case vex_f3:
> > > > host_and_vcpu_must_have(sse2);
> > > > -buf[0] = 0x66; /* movdqa */
> > > > +buf[0] = 0x66; /* SSE */
> > >
> > > The comment change here indicates a problem: So far it was indicating
> > > that despite the possible F3 prefix (movdqu) we encode a 66 one
> > > (movdqa). Opcode D6 prefixed with F3, however, is movq2dq, which
> > > you then either don't emulate correctly, or if it happens to be
> > > emulated correctly you should include in the comment accompanying
> > > the case label. And its AVX counterpart should then produce #UD.
> >
> > I fiddled with this for a while and the attached patch (adjusted)
> > appears to be doing the right thing: ie. movq2dq gets emulated
> > correctly too. copy_REX_VEX() does not work OK with movq2dq, but it
> > looked easy to single out this case.
>
> Except that you can't really avoid it (see below). Without you being
> more explicit I also can't tell what it is that doesn't work right in that
> case.
Sorry about that. In x86_emulate(), after buf[0] == 0x66 and
copy_REX_VEX():
f3 0f d6 d1 movq2dq %mm1,%xmm2
becomes:
66 40 0f d6 d1 rex movq %xmm2,%xmm1
Now that I slept over it, I can see it's not really a problem with
copy_REX_VEX().
> > All tests pass, including for {,v}movq xmm/m64 and movq2dq. There does
> > not appear to be an AVX variant for the latter, or I'm not reading the
> > Intel SDM right (or binutils' as is lying to me).
>
> Well, that's what I said earlier on (still visible above), and what you
> still fail to deal with.
>
> > @@ -4469,7 +4471,11 @@ x86_emulate(
> > }
> > if ( !rc )
> > {
> > - copy_REX_VEX(buf, rex_prefix, vex);
> > + /* try to preserve the mandatory prefix for movq2dq */
> > + if ( !rex_prefix && vex.opcx == vex_none && vex.pfx == vex_f3 )
> > + buf[0] = 0xf3;
> > + else
> > + copy_REX_VEX(buf, rex_prefix, vex);
>
> So what about a movq2dq having a REX prefix to encode XMM8..15
> for one or both of its operands? The writing of the F3 prefix really
> needs to go elsewhere - probably the best place is where the 66
> one gets written unconditionally right now. And afaict then
> copy_REX_VEX() will work fine here too.
--
Mihai DONȚU
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] "Tried to do a paging op on itself"
On Mon, 8 Aug 2016 13:35:42 -0700 (MST) Alina wrote: > When I try to enable shadow page table features using the shadow_control > function, I get this message in the hypervisor logs: "Tried to do a paging > op on itself". After backtracking to the source of this message, I > discovered it comes from the condition: "if ( unlikely(d == current->domain) > )", which means that the hypervisor does not consider the current domain as > the same domain I give as parameter, even though I checked and their > domain_id is the same. > > Does anyone have any idea why the hypervisor does not see them as being > equal? You are misreading the code: a domain cannot call xc_shadow_control() on itself. I've never played with shadow paging, but you'd probably have more luck running your code from dom0 against a user domain. -- Mihai Donțu ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] Enabling vm_event for a guest with more VCPUs than available ring buffer slots freezes the virtual machine
>> 169 online++;
> > >> 170 ved->blocked--;
> > >> 171 ved->last_vcpu_wake_up = k;
> > >> 172 }
> > >> 173 }
> > >> 174 }
> > >>
> > >> at "if ( !(ved->blocked) || online >= avail_req )". At this point,
> > >> nothing ever gets unblocked. It's hard to believe that this is
> > desired
> > >> behaviour, as I don't know what could possibly happen for that
> > condition
> > >> to become false once all the online VCPUs are stuck (especially
> > when the
> > >> guest has just started booting).
> >
> >
> > Ah I see what happens. During boot vCPU 0 generates an event and gets
> > marked blocked because the number of vCPUs is so high. The other vCPUs
> > are still unblocked since they are idle, but this test here will still
> > be true (online >= avail_req) and thus we can never unblock vCPU0. And
> > then the boot process is hanging because vCPU0 never resumes. I would
> > argue that this test should be changed to check that there is at least 1
> > spot on the ring and only break if that is not the case anymore (ie.
> > instead of incrementing online we should be decrementing avail_req).
>
> That is exactly what happens. And it can't really be fixed just by
> increasing the ring buffer (although that definitely helps a lot and
> would be a smart move): no matter how large it is, we can always ask the
> domain to use more VCPUs than there are slots in the buffer.
>
> > >
> > > I wouldn't bet that this logic has ever been tested. If you
> > recall, the
> > > addition of register state into the vm_event ring made each entry far
> > > larger, which in turns makes it more likely to hit this condition.
> > >
> > > However, simply fixing the logic to re-online the cpus isn't a good
> > > solution either, as having $N vcpus paused at any one time because of
> > > ring contention is not conducive good system performance.
> > >
> > > Realistically, the ring size needs to be max_cpus * sizeof(largest
> > > vm_event) at an absolute minimum, and I guess this is now beyond 1
> > page?
> >
> > Yes, of course the reason this triggers earlier now is the growth of the
> > request's size. Yes, using e.g. 20 VCPUs in the guest's setup will
> > exceed a page's number of slots.
> >
> > And yes, ideally we should have multi-page ring buffers - however that
> > is a long-term project that requires design changes in other parts of
> > Xen as well (Andrew, CCd here, was recently talking about one).
> >
> > However, even with a one-page ring buffer, surely it's not good to end
> > up in this situation, especially for guests such as mine, which never
> > actually bring more than 2 VCPUs online. But even if they were to use
> > more, blocking the guest on vm_event init is completely pointless - we
> > might as well return some kind of error if max_vcpus > available slots.
> >
> > I don't follow the system performance argument. Surely completely
> > blocking the guest is worse.
> >
> >
> > I also don't see the point in marking a vCPU blocked if it is already
> > paused. I think this behavior of blocking vCPUs makes only sense for
> > asynchronous events. Razvan, could you test what happens if
> > vm_event_mark_and_pause is only called if the vCPU is unpaused?
>
> It works for me with this change (using Xen 4.7 sources here):
>
> @@ -318,7 +329,11 @@ void vm_event_put_request(struct domain *d,
> * on how this mechanism works to avoid waiting. */
> avail_req = vm_event_ring_available(ved);
> if( current->domain == d && avail_req < d->max_vcpus )
> -vm_event_mark_and_pause(current, ved);
> +{
> +if ( !atomic_read( ¤t->vm_event_pause_count ) )
> +vm_event_mark_and_pause(current, ved);
> +}
If I'm reading the code correctly, when max_vcpus is greater than the
number of slots available in the ring, a race appears that can lead to
a ring corruption (in debug mode ASSERT(free_req > 0) will trigger).
For example, when a single slot is available, two vCPUs can race to
vm_event_put_request() after both being given a green light in
__vm_event_claim_slot(), whose return depends only on
vm_event_ring_available() returning non-zero (which it can do, for both
vCPUs at the same time).
As it turns out, the bug being talked about prevented this from showing
up.
PS:
https://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=3643a961195f76ba849a213628c1979240e6fbdd
--
Mihai Donțu
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] Enabling vm_event for a guest with more VCPUs than available ring buffer slots freezes the virtual machine
On Wed, 8 Feb 2017 00:09:52 +0200 Mihai Donțu wrote:
> On Tue, 7 Feb 2017 22:41:57 +0200 Razvan Cojocaru wrote:
> > On 02/07/2017 10:20 PM, Tamas K Lengyel wrote:
> > > On Tue, Feb 7, 2017 at 11:57 AM, Razvan Cojocaru wrote:
> > > On 02/07/2017 08:39 PM, Andrew Cooper wrote:
> > > > On 07/02/17 18:31, Razvan Cojocaru wrote:
> > > >> On 02/07/2017 08:15 PM, Tamas K Lengyel wrote:
> > > >>> On Tue, Feb 7, 2017 at 9:53 AM, Razvan Cojocaru wrote:
> > > >>> Hello,
> > > >>>
> > > >>> Setting, e.g. 16 VCPUs for a HVM guest, ends up blocking the
> > >
> > > guest
> > > >>> completely when subscribing to vm_events, apparently because
> > >
> > > of this
> > > >>> code in xen/common/vm_event.c:
> > > >>>
> > > >>> 315 /* Give this vCPU a black eye if necessary, on the
> > > way out.
> > > >>> 316 * See the comments above wake_blocked() for more
> > > information
> > > >>> 317 * on how this mechanism works to avoid waiting. */
> > > >>> 318 avail_req = vm_event_ring_available(ved);
> > > >>> 319 if( current->domain == d && avail_req < d->max_vcpus )
> > > >>> 320 vm_event_mark_and_pause(current, ved);
> > > >>>
> > > >>> It would appear that even if the guest only has 2 online
> > > VCPUs, the
> > > >>> "avail_req < d->max_vcpus" condition will pause current, and
> > > we
> > > >>> eventually end up with all the VCPUs paused.
> > > >>>
> > > >>> An ugly hack ("avail_req < 2") has allowed booting a guest
> > > with many
> > > >>> VCPUs (max_vcpus, the guest only brings 2 VCPUs online),
> > > however that's
> > > >>> just to prove that that was the culprit - a real solution to
> > >
> > > this needs
> > > >>> more in-depth understading of the issue and potential
> > > solution. That's
> > > >>> basically very old code (pre-2012 at least) that got moved
> > > around into
> > > >>> the current shape of Xen today - please CC anyone relevant
> > > to the
> > > >>> discussion that you're aware of.
> > > >>>
> > > >>> Thoughts?
> > > >>>
> > > >>>
> > > >>> I think is a side-effect of the growth of the vm_event structure
> > >
> > > and the
> > > >>> fact that we have a single page ring. The check effectively sets a
> > > >>> threshold of having enough space for each vCPU to place at least
> > > one
> > > >>> more event on the ring, and if that's not the case it gets
> > > paused. OTOH
> > > >>> I think this would only have an effect on asynchronous events,
> > > for all
> > > >>> other events the vCPU is already paused. Is that the case you
> > > have?
> > > >> No, on the contrary, all my events are synchronous (the VCPU is
> > > paused
> > > >> waiting for the vm_event reply).
> > > >>
> > > >> I've debugged this a bit, and the problem seems to be that
> > > >> vm_event_wake_blocked() breaks here:
> > > >>
> > > >> 150 /* We remember which vcpu last woke up to avoid scanning
> > >
> > > always
> > > >> linearly
> > > >> 151 * from zero and starving higher-numbered vcpus under
> > > high load */
> > > >> 152 if ( d->vcpu )
> > > >> 153 {
> > > >> 154 int i, j, k;
> > > >> 155
> > > >> 156 for (i = ved->last_vcpu_wake_up + 1, j = 0; j <
> > > >> d->max_vcpus; i++, j++)
> > > >> 157 {
> > > >> 158
Re: [Xen-devel] [PATCH 1/4] hvmloader: Fixup pci_write* macros
On Mon, 15 Jun 2015 16:23:13 +0100 Jan Beulich wrote: > >>> On 15.06.15 at 16:35, wrote: > > On 15/06/15 15:30, Don Slutz wrote: > >> On 06/15/15 10:19, Andrew Cooper wrote: > >>> Furthermore, what about devfn or reg? > >>> > >> devfn and reg do not need the bracketing since they are just passed, > >> but I have no issue with adding the extra brackets. > > > > Macros, under all circumstances, should have all of their parameters > > bracketed for safety. > > No, as this harms readability: > > #define macro(x) function((x)) > > would completely pointlessly be having an extra set of parentheses. > And > > #define macro(x, y) function(x, y) > > is just the logical extension to that: There is nothing the expressions > passed as arguments could contain to require parenthesization, as > there's no operator of precedence lower than comma (and if either > argument contains a comma expression, it needs to be > parenthesized at the invocation site anyway). I think parenthesis are just long term guards against classic surprises such as: #define macro(x) function(x * 2) ... macro(N + 3) You could treat this case separately though, but people often go for the "good practice". -- Mihai Donțu ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4] xmalloc: add support for checking the pool integrity
On Wednesday 07 January 2015 19:20:38 Andrew Cooper wrote:
> On 16/12/14 19:33, Mihai Donțu wrote:
> > Implemented xmem_pool_check(), xmem_pool_check_locked() and
> > xmem_pool_check_unlocked() to verity the integrity of the TLSF matrix.
> >
> > Signed-off-by: Mihai Donțu
>
> This review supersedes (and is adjusted accordingly for) the two
> discussion threads which happened off my first review.
>
> >
> > ---
> > Changes since v3:
> > - try harder to respect the 80 column limit
> > - use 'unsigned int' instead of 'int' where possible
> > - made the logged messages even shorter
> > - dropped the use of goto (didn't really make sense)
> >
> > Changes since v2:
> > - print the name of the corrupted pool
> > - adjusted the messages to better fit within 80 columns
> > - minor style change (a ? a : b -> a ?: b)
> >
> > Changes since v1:
> > - fixed the codingstyle
> > - swaped _locked/_unlocked naming
> > - reworked __xmem_pool_check_locked() a bit
> > - used bool_t where appropriate
> > - made xmem_pool_check() take a pool argument which can be NULL
> > ---
> > xen/common/xmalloc_tlsf.c | 121
> > +-
> > xen/include/xen/xmalloc.h | 7 +++
> > 2 files changed, 126 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c
> > index a5769c9..eca4f1c 100644
> > --- a/xen/common/xmalloc_tlsf.c
> > +++ b/xen/common/xmalloc_tlsf.c
> > @@ -120,9 +120,122 @@ struct xmem_pool {
> > char name[MAX_POOL_NAME_LEN];
> > };
> >
> > +static struct xmem_pool *xenpool;
> > +
> > +static inline void MAPPING_INSERT(unsigned long r, int *fl, int *sl);
> > +
> > /*
> > * Helping functions
> > */
> > +#ifndef NDEBUG
> > +static bool_t xmem_pool_check_size(const struct xmem_pool *pool,
> > + int fl, int sl)
> > +{
> > +const struct bhdr *b = pool->matrix[fl][sl];
> > +
> > +while ( b )
> > +{
> > +int __fl;
> > +int __sl;
> > +
> > +MAPPING_INSERT(b->size, &__fl, &__sl);
> > +if ( __fl != fl || __sl != sl )
> > +{
> > +printk(XENLOG_ERR
> > + "xmem_pool: %s: misplaced block %p:%u ({%d,%d} ->
> > {%d,%d})\n",
> > + pool->name, b, b->size, fl, sl, __fl, __sl);
>
> Is it liable to get confusing with a hex number and a decimal number
> combined with just a colon?
>
> Is b even a useful pointer to print? You have the pool name, indicies
> and size which seem to be the salient information.
>
I think you are right. I went ahead and printed all information
available just because I had access to it thinking someone other than
myself might find it valuable.
> > +return 0;
> > +}
> > +b = b->ptr.free_ptr.next;
> > +}
> > +return 1;
> > +}
> > +
> > +/*
> > + * This function must be called from a context where pool->lock is
> > + * already acquired.
> > + *
> > + * Returns true if the pool has been corrupted, false otherwise
> > + */
> > +#define xmem_pool_check_locked(pool) \
> > +__xmem_pool_check_locked(__FILE__, __LINE__, pool)
> > +static bool_t __xmem_pool_check_locked(const char *file, int line,
> > + const struct xmem_pool *pool)
> > +{
> > +unsigned int i;
> > +static bool_t once = 1;
> > +
> > +if ( !once )
> > +return 1;
> > +for ( i = 0; i < REAL_FLI; i++ )
> > +{
> > +int fl = (pool->fl_bitmap & (1 << i)) ? i : -1;
> > +
> > +if ( fl >= 0 )
> > +{
> > +unsigned int j;
> > +
> > +if ( !pool->sl_bitmap[fl] )
> > +{
> > +printk(XENLOG_ERR
> > + "xmem_pool: %s: TLSF bitmap corrupt (!empty FL,
> > empty SL)\n",
> > + pool->name);
> > +__warn(file, line);
> > +once = 0;
> > +break;
> > +}
> > +for ( j = 0; j < MAX_SLI; j++ )
> > +{
> > +int sl = (pool->sl_bitmap[fl] & (1 << j)) ? j : -1;
> > +
> > +if ( sl < 0 )
> > +conti
[Xen-devel] [PATCH] x86/emulate: add support for movq xmm,xmm/m64
Signed-off-by: Mihai Donțu
---
tools/tests/x86_emulator/test_x86_emulator.c | 23 +++
xen/arch/x86/x86_emulate/x86_emulate.c | 7 ---
2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/tools/tests/x86_emulator/test_x86_emulator.c
b/tools/tests/x86_emulator/test_x86_emulator.c
index c7f572a..d18b2d2 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -697,6 +697,29 @@ int main(int argc, char **argv)
else
printf("skipped\n");
+printf("%-40s", "Testing movq %%xmm0, 32(%%eax)...");
+if ( stack_exec && cpu_has_mmx )
+{
+decl_insn(movq_to_mem2);
+
+asm volatile ( "pcmpgtb %%xmm0, %%xmm0\n"
+put_insn(movq_to_mem2, "movq %%xmm0, 32(%%eax)")
+:: );
+
+*((unsigned long *)res + 4) = 0xbdbdbdbdbdbdbdbd;
+set_insn(movq_to_mem2);
+regs.ecx= 0;
+regs.eax= (unsigned long)res;
+rc = x86_emulate(&ctxt, &emulops);
+if ( rc != X86EMUL_OKAY || !check_eip(movq_to_mem2) )
+goto fail;
+if ( *((unsigned long *)res + 4) )
+goto fail;
+printf("okay\n");
+}
+else
+printf("skipped\n");
+
printf("%-40s", "Testing movdqu %xmm2,(%ecx)...");
if ( stack_exec && cpu_has_sse2 )
{
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
b/xen/arch/x86/x86_emulate/x86_emulate.c
index fe594ba..fcf5694 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -245,7 +245,7 @@ static uint8_t twobyte_table[256] = {
ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
/* 0xD0 - 0xDF */
-0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+0, 0, 0, 0, 0, 0, ImplicitOps|ModRM, 0, 0, 0, 0, 0, 0, 0, 0, 0,
/* 0xE0 - 0xEF */
0, 0, 0, 0, 0, 0, 0, ImplicitOps|ModRM, 0, 0, 0, 0, 0, 0, 0, 0,
/* 0xF0 - 0xFF */
@@ -4412,6 +4412,7 @@ x86_emulate(
case 0x7f: /* movq mm,mm/m64 */
/* {,v}movdq{a,u} xmm,xmm/m128 */
/* vmovdq{a,u} ymm,ymm/m256 */
+case 0xd6: /* movq xmm,xmm/m64 */
{
uint8_t *buf = get_stub(stub);
struct fpu_insn_ctxt fic = { .insn_bytes = 5 };
@@ -4429,9 +4430,9 @@ x86_emulate(
case vex_66:
case vex_f3:
host_and_vcpu_must_have(sse2);
-buf[0] = 0x66; /* movdqa */
+buf[0] = 0x66; /* movq, movdqa */
get_fpu(X86EMUL_FPU_xmm, &fic);
-ea.bytes = 16;
+ea.bytes = (b == 0xd6 ? 8 : 16);
break;
case vex_none:
if ( b != 0xe7 )
--
2.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 3/3] x86/emulate: added tests for {, v}movd mm, m32
Signed-off-by: Mihai Donțu
---
tools/tests/x86_emulator/test_x86_emulator.c | 42
1 file changed, 42 insertions(+)
diff --git a/tools/tests/x86_emulator/test_x86_emulator.c
b/tools/tests/x86_emulator/test_x86_emulator.c
index 8994149..e2bd7ce 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -650,6 +650,48 @@ int main(int argc, char **argv)
#define check_eip(which) (regs.eip == (unsigned long)instr + \
(unsigned long)which##_len)
+printf("%-40s", "Testing movd %%mm3,32(%%eax)...");
+if ( stack_exec && cpu_has_mmx )
+{
+decl_insn(movd_to_mem32);
+
+asm volatile ( "pcmpeqb %%mm3, %%mm3\n"
+ put_insn(movd_to_mem32, "movd %%mm3, 32(%%eax)")
+ :: );
+
+*(res + 8) = 0xbdbdbdbd;
+set_insn(movd_to_mem32);
+regs.eax = (unsigned long)res;
+rc = x86_emulate(&ctxt, &emulops);
+if ( (rc != X86EMUL_OKAY) || *(res + 8) != 0x ||
+ !check_eip(movd_to_mem32) )
+goto fail;
+printf("okay\n");
+}
+else
+printf("skipped\n");
+
+printf("%-40s", "Testing vmovd %%xmm1,32(%%eax)...");
+if ( stack_exec && cpu_has_avx )
+{
+decl_insn(vmovd_to_mem32);
+
+asm volatile ( "pcmpeqb %%xmm1, %%xmm1\n"
+ put_insn(vmovd_to_mem32, "vmovd %%xmm1, 32(%%eax)")
+ :: );
+
+*(res + 8) = 0xbdbdbdbd;
+set_insn(vmovd_to_mem32);
+regs.eax = (unsigned long)res;
+rc = x86_emulate(&ctxt, &emulops);
+if ( (rc != X86EMUL_OKAY) || *(res + 8) != 0x ||
+ !check_eip(vmovd_to_mem32) )
+goto fail;
+printf("okay\n");
+}
+else
+printf("skipped\n");
+
printf("%-40s", "Testing movq %mm3,(%ecx)...");
if ( stack_exec && cpu_has_mmx )
{
--
2.9.2
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 2/3] x86/emulate: add support of emulating SSE2 instruction {, v}movd mm, m32
Found that Windows driver was using a SSE2 instruction MOVD.
Signed-off-by: Zhi Wang
Signed-off-by: Mihai Donțu
---
Picked from the XenServer 7 patch queue, as suggested by Andrew Cooper
---
xen/arch/x86/x86_emulate/x86_emulate.c | 30 +++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
b/xen/arch/x86/x86_emulate/x86_emulate.c
index 0301235..2a56a67 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -204,7 +204,7 @@ static uint8_t twobyte_table[256] = {
/* 0x60 - 0x6F */
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ImplicitOps|ModRM,
/* 0x70 - 0x7F */
-0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ImplicitOps|ModRM,
+0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ImplicitOps|ModRM,
ImplicitOps|ModRM,
/* 0x80 - 0x87 */
ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
@@ -4409,6 +4409,10 @@ x86_emulate(
case 0x6f: /* movq mm/m64,mm */
/* {,v}movdq{a,u} xmm/m128,xmm */
/* vmovdq{a,u} ymm/m256,ymm */
+case 0x7e: /* movd mm,r/m32 */
+ /* movq mm,r/m64 */
+ /* {,v}movd xmm,r/m32 */
+ /* {,v}movq xmm,r/m64 */
case 0x7f: /* movq mm,mm/m64 */
/* {,v}movdq{a,u} xmm,xmm/m128 */
/* vmovdq{a,u} ymm,ymm/m256 */
@@ -4432,7 +4436,17 @@ x86_emulate(
host_and_vcpu_must_have(sse2);
buf[0] = 0x66; /* SSE */
get_fpu(X86EMUL_FPU_xmm, &fic);
-ea.bytes = (b == 0xd6 ? 8 : 16);
+switch ( b )
+{
+case 0x7e:
+ea.bytes = 4;
+break;
+case 0xd6:
+ea.bytes = 8;
+break;
+default:
+ea.bytes = 16;
+}
break;
case vex_none:
if ( b != 0xe7 )
@@ -4452,7 +4466,17 @@ x86_emulate(
((vex.pfx != vex_66) && (vex.pfx != vex_f3)));
host_and_vcpu_must_have(avx);
get_fpu(X86EMUL_FPU_ymm, &fic);
-ea.bytes = (b == 0xd6 ? 8 : 16 << vex.l);
+switch ( b )
+{
+case 0x7e:
+ea.bytes = 4;
+break;
+case 0xd6:
+ea.bytes = 8;
+break;
+default:
+ea.bytes = 16 << vex.l;
+}
}
if ( ea.type == OP_MEM )
{
--
2.9.2
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 1/3] x86/emulate: add support for {, v}movq xmm, xmm/m64
Signed-off-by: Mihai Donțu
---
Changed since v1:
* added a test for vmovq
* made the tests depend on SSE and AVX, respectively
* added emulator support for vmovq (0xd6 forces the operand size to
64bit)
---
tools/tests/x86_emulator/test_x86_emulator.c | 44
xen/arch/x86/x86_emulate/x86_emulate.c | 9 +++---
2 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/tools/tests/x86_emulator/test_x86_emulator.c
b/tools/tests/x86_emulator/test_x86_emulator.c
index c7f572a..8994149 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -697,6 +697,50 @@ int main(int argc, char **argv)
else
printf("skipped\n");
+printf("%-40s", "Testing movq %%xmm0,32(%%eax)...");
+if ( stack_exec && cpu_has_sse )
+{
+decl_insn(movq_to_mem2);
+
+asm volatile ( "pcmpgtb %%xmm0, %%xmm0\n"
+ put_insn(movq_to_mem2, "movq %%xmm0, 32(%%eax)")
+ :: );
+
+*((unsigned long *)res + 4) = 0xbdbdbdbdbdbdbdbd;
+set_insn(movq_to_mem2);
+regs.eax = (unsigned long)res;
+rc = x86_emulate(&ctxt, &emulops);
+if ( rc != X86EMUL_OKAY || !check_eip(movq_to_mem2) )
+goto fail;
+if ( *((unsigned long *)res + 4) )
+goto fail;
+printf("okay\n");
+}
+else
+printf("skipped\n");
+
+printf("%-40s", "Testing vmovq %%xmm1,32(%%eax)...");
+if ( stack_exec && cpu_has_avx )
+{
+decl_insn(vmovq_to_mem);
+
+asm volatile ( "pcmpgtb %%xmm1, %%xmm1\n"
+ put_insn(vmovq_to_mem, "vmovq %%xmm1, 32(%%eax)")
+ :: );
+
+*((unsigned long *)res + 4) = 0xbdbdbdbdbdbdbdbd;
+set_insn(vmovq_to_mem);
+regs.eax = (unsigned long)res;
+rc = x86_emulate(&ctxt, &emulops);
+if ( rc != X86EMUL_OKAY || !check_eip(vmovq_to_mem) )
+goto fail;
+if ( *((unsigned long *)res + 4) )
+goto fail;
+printf("okay\n");
+}
+else
+printf("skipped\n");
+
printf("%-40s", "Testing movdqu %xmm2,(%ecx)...");
if ( stack_exec && cpu_has_sse2 )
{
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
b/xen/arch/x86/x86_emulate/x86_emulate.c
index fe594ba..0301235 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -245,7 +245,7 @@ static uint8_t twobyte_table[256] = {
ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
/* 0xD0 - 0xDF */
-0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+0, 0, 0, 0, 0, 0, ImplicitOps|ModRM, 0, 0, 0, 0, 0, 0, 0, 0, 0,
/* 0xE0 - 0xEF */
0, 0, 0, 0, 0, 0, 0, ImplicitOps|ModRM, 0, 0, 0, 0, 0, 0, 0, 0,
/* 0xF0 - 0xFF */
@@ -4412,6 +4412,7 @@ x86_emulate(
case 0x7f: /* movq mm,mm/m64 */
/* {,v}movdq{a,u} xmm,xmm/m128 */
/* vmovdq{a,u} ymm,ymm/m256 */
+case 0xd6: /* {,v}movq xmm,xmm/m64 */
{
uint8_t *buf = get_stub(stub);
struct fpu_insn_ctxt fic = { .insn_bytes = 5 };
@@ -4429,9 +4430,9 @@ x86_emulate(
case vex_66:
case vex_f3:
host_and_vcpu_must_have(sse2);
-buf[0] = 0x66; /* movdqa */
+buf[0] = 0x66; /* SSE */
get_fpu(X86EMUL_FPU_xmm, &fic);
-ea.bytes = 16;
+ea.bytes = (b == 0xd6 ? 8 : 16);
break;
case vex_none:
if ( b != 0xe7 )
@@ -4451,7 +4452,7 @@ x86_emulate(
((vex.pfx != vex_66) && (vex.pfx != vex_f3)));
host_and_vcpu_must_have(avx);
get_fpu(X86EMUL_FPU_ymm, &fic);
-ea.bytes = 16 << vex.l;
+ea.bytes = (b == 0xd6 ? 8 : 16 << vex.l);
}
if ( ea.type == OP_MEM )
{
--
2.9.2
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 2/3] x86/emulate: add support of emulating SSE2 instruction {, v}movd mm, m32
On Monday 18 July 2016 15:57:09 Andrew Cooper wrote:
> On 18/07/16 15:30, Mihai Donțu wrote:
> > @@ -4409,6 +4409,10 @@ x86_emulate(
> > case 0x6f: /* movq mm/m64,mm */
> > /* {,v}movdq{a,u} xmm/m128,xmm */
> > /* vmovdq{a,u} ymm/m256,ymm */
> > +case 0x7e: /* movd mm,r/m32 */
> > + /* movq mm,r/m64 */
> > + /* {,v}movd xmm,r/m32 */
> > + /* {,v}movq xmm,r/m64 */
>
> This exposes a vulnerability where a guest can clobber local state in
> x86_emulate, by specifying registers such as %ebx as the destination.
>
> You must either
> 1) Move this case up above the fail_if(ea.type != OP_MEM); check, or
> 2) modify the stub logic to convert a GPR destination to a memory
> address pointing into _regs.
I'm taking a look at (2) as it feels like the best approach. If I'm not
making any good progress in the coming days, I'll fallback to (1).
Thank you,
--
Mihai DONȚU
pgpIf9CfX1gMD.pgp
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] Xen 4.6 Development Update (two months reminder)
On Thu, 12 Mar 2015 10:21:56 + wei.l...@citrix.com wrote: > We are now two months into 4.6 development window. This is an email to keep > track of all the patch series I gathered. It is by no means complete and / or > acurate. Feel free to reply this email with new projects or correct my > misunderstanding. > > = Timeline = > > We are planning on a 9-month release cycle, but we could also release a bit > earlier if everything goes well (no blocker, no critical bug). > > * Development start: 6 Jan 2015 > <=== We are here ===> > * Feature Freeze: 10 Jul 2015 > * RCs: TBD > * Release Date: 9 Oct 2015 (could release earlier) > > The RCs and release will of course depend on stability and bugs, and > will therefore be fairly unpredictable. > > Bug-fixes, if Acked-by by maintainer, can go anytime before the First > RC. Later on we will need to figure out the risk of regression/reward > to eliminate the possiblity of a bug introducing another bug. > > = Prognosis = > > The states are: none -> fair -> ok -> good -> done > > none - nothing yet > fair - still working on it, patches are prototypes or RFC > ok - patches posted, acting on review > good - some last minute pieces > done - all done, might have bugs > > = Bug Fixes = > > Bug fixes can be checked in without a freeze exception throughout the > freeze, unless the maintainer thinks they are particularly high > risk. In later RC's, we may even begin rejecting bug fixes if the > broken functionality is small and the risk to other functionality is > high. > > Document changes can go in anytime if the maintainer is OK with it. > > These are guidelines and principles to give you an idea where we're coming > from; if you think there's a good reason why making an exception for you will > help us make Xen better than not doing so, feel free to make your case. > > [...] I have been meaning to write this email for a while now, just to let everyone know we're working on a couple more patches related to VM introspection. They are not as big as our initial ones, but they do bring in new functionality. Răzvan Cojocaru will post a series based on Tamas' changes, as soon as it stabilizes some more or (even better) gets merged. I'll let him choose the right moment. I will also pick up some of my [oldish] patches with reviews from Andrew and Ian and also try to tackle some of the x86 emulator challenges that we talked about some months ago. We've adjusted our plans and aim for a to-the-point set of changes that allow one to built a security solution (at least) the we envision it, in terms of isolation and monitoring capability. Regards, -- Mihai Donțu pgpGS4KQ6pQ3j.pgp Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Xen 4.6 Development Update (two months reminder)
On Monday 16 March 2015 18:18:22 Razvan Cojocaru wrote: > On 03/16/2015 06:00 PM, Lars Kurth wrote: > > > >> On 16 Mar 2015, at 13:01, Mihai Donțu wrote: > >> > >> On Thu, 12 Mar 2015 10:21:56 + wei.l...@citrix.com wrote: > >>> We are now two months into 4.6 development window. This is an email to > >>> keep > >>> track of all the patch series I gathered. It is by no means complete and > >>> / or > >>> acurate. Feel free to reply this email with new projects or correct my > >>> misunderstanding. > >>> > >>> = Timeline = > >>> > >>> We are planning on a 9-month release cycle, but we could also release a > >>> bit > >>> earlier if everything goes well (no blocker, no critical bug). > >>> > >>> * Development start: 6 Jan 2015 > >>> <=== We are here ===> > >>> * Feature Freeze: 10 Jul 2015 > >>> * RCs: TBD > >>> * Release Date: 9 Oct 2015 (could release earlier) > >>> > >>> The RCs and release will of course depend on stability and bugs, and > >>> will therefore be fairly unpredictable. > >>> > >>> Bug-fixes, if Acked-by by maintainer, can go anytime before the First > >>> RC. Later on we will need to figure out the risk of regression/reward > >>> to eliminate the possiblity of a bug introducing another bug. > >>> > >>> = Prognosis = > >>> > >>> The states are: none -> fair -> ok -> good -> done > >>> > >>> none - nothing yet > >>> fair - still working on it, patches are prototypes or RFC > >>> ok - patches posted, acting on review > >>> good - some last minute pieces > >>> done - all done, might have bugs > >>> > >>> = Bug Fixes = > >>> > >>> Bug fixes can be checked in without a freeze exception throughout the > >>> freeze, unless the maintainer thinks they are particularly high > >>> risk. In later RC's, we may even begin rejecting bug fixes if the > >>> broken functionality is small and the risk to other functionality is > >>> high. > >>> > >>> Document changes can go in anytime if the maintainer is OK with it. > >>> > >>> These are guidelines and principles to give you an idea where we're coming > >>> from; if you think there's a good reason why making an exception for you > >>> will > >>> help us make Xen better than not doing so, feel free to make your case. > >>> > >>> [...] > >> > >> I have been meaning to write this email for a while now, just to let > >> everyone know we're working on a couple more patches related to VM > >> introspection. They are not as big as our initial ones, but they do > >> bring in new functionality. > > > > Mihai, > > it would make Wei's life easier if you could provide headlines for those > > patches. That way they can be tracked before you post them > > For my part, the patches are: > > 1. xen: Add support for XSETBV vm_events > > This is basically VMX support for sending out an event on VMEXIT / > EXIT_REASON_XSETBV. Additional information sent out is the XCR (the > value of ECX). > > 2. xen: Support hybernating guests > > This patch cover two areas: A) send data (just a regular blob / buffer) > back to the HV in the vm_event response, and B) have that data returned > by the read function when emulating an instruction. Unless we do this, > monitored guests won't be able to properly wake up from hybernation. > > 3. xen: Support for VMCALL-based vm_events > > This is a modification of the VMCALL patch in my original RFC series, > which got dropped last year. The modification takes into account Andrew > Cooper's suggestion to just use a hypercall: > > http://lists.xen.org/archives/html/xen-devel/2014-07/msg01677.html > > 4. xen: Deny MSR writes if refused by the vm_event reply > > Preempt MSR writes that the monitoring application decides are evil. > > 5. xen: Implement actual write of CR values on xc_vcpu_setcontext() > > Although libxc's API leads one to believe that all info in the context > will be set for the guest, the CR values were actually ignored for HVM > guests. This patch addresses that problem. > > Hope this helps, Mihai will complete the picture with the rest. 6. xmalloc: pool integrity This is a simple mechanism that gives an early heads-up that the
[Xen-devel] [PATCH] xmalloc: add support for checking the pool integrity
Implemented xmem_pool_check(), xmem_pool_check_locked() and
xmem_pool_check_unlocked() to verity the integrity of the TLSF matrix.
Signed-off-by: Mihai Donțu
---
xen/common/xmalloc_tlsf.c | 119 +-
xen/include/xen/xmalloc.h | 7 +++
2 files changed, 124 insertions(+), 2 deletions(-)
diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c
index a5769c9..009ba60 100644
--- a/xen/common/xmalloc_tlsf.c
+++ b/xen/common/xmalloc_tlsf.c
@@ -120,9 +120,120 @@ struct xmem_pool {
char name[MAX_POOL_NAME_LEN];
};
+static struct xmem_pool *xenpool;
+
+static inline void MAPPING_INSERT(unsigned long r, int *fl, int *sl);
+
/*
* Helping functions
*/
+#ifndef NDEBUG
+static int xmem_pool_check_size(const struct bhdr *b, int fl, int sl)
+{
+while ( b )
+{
+int __fl;
+int __sl;
+
+MAPPING_INSERT(b->size, &__fl, &__sl);
+if ( __fl != fl || __sl != sl )
+{
+printk(XENLOG_ERR "xmem_pool: for block %p size = %u, { fl = %d,
sl = %d } should be { fl = %d, sl = %d }\n", b, b->size, fl, sl, __fl, __sl);
+return 0;
+}
+b = b->ptr.free_ptr.next;
+}
+return 1;
+}
+
+/*
+ * This function must be called from a context where pool->lock is
+ * already acquired
+ */
+#define xmem_pool_check_unlocked(__pool) __xmem_pool_check_unlocked(__FILE__,
__LINE__, __pool)
+static int __xmem_pool_check_unlocked(const char *file, int line, const struct
xmem_pool *pool)
+{
+int i;
+int woops = 0;
+static int once = 1;
+
+for ( i = 0; i < REAL_FLI; i++ )
+{
+int fl = ( pool->fl_bitmap & (1 << i) ) ? i : -1;
+
+if ( fl >= 0 )
+{
+int j;
+int bitmap_empty = 1;
+int matrix_empty = 1;
+
+for ( j = 0; j < MAX_SLI; j++ )
+{
+int sl = ( pool->sl_bitmap[fl] & (1 << j) ) ? j : -1;
+
+if ( sl < 0 )
+continue;
+
+if ( once && !pool->matrix[fl][sl] )
+{
+/* The bitmap is corrupted */
+printk(XENLOG_ERR "xmem_pool:%s:%d the TLSF bitmap is
corrupted\n", file, line);
+__warn((char *)file, line);
+once = 0;
+woops = 1;
+}
+else if ( once && !xmem_pool_check_size(pool->matrix[fl][sl],
fl, sl))
+{
+printk(XENLOG_ERR "xmem_pool:%s:%d the TLSF chunk matrix
is corrupted\n", file, line);
+__warn((char *)file, line);
+once = 0;
+woops = 1;
+}
+if ( pool->matrix[fl][sl] )
+matrix_empty = 0;
+bitmap_empty = 0;
+}
+if ( once && bitmap_empty )
+{
+/* The bitmap is corrupted */
+printk(XENLOG_ERR "xmem_pool:%s:%d the TLSF bitmap is
corrupted (non-empty FL with empty SL)\n", file, line);
+__warn((char *)file, line);
+once = 0;
+woops = 1;
+}
+if ( once && matrix_empty )
+{
+/* The bitmap is corrupted */
+printk(XENLOG_ERR "xmem_pool:%s:%d the TLSF bitmap is
corrupted (empty matrix)\n", file, line);
+__warn((char *)file, line);
+once = 0;
+woops = 1;
+}
+}
+}
+
+return woops;
+}
+
+#define xmem_pool_check_locked(__pool) __xmem_pool_check_locked(__FILE__,
__LINE__, __pool)
+static int __xmem_pool_check_locked(const char *file, int line, struct
xmem_pool *pool)
+{
+int err;
+
+spin_lock(&pool->lock);
+err = __xmem_pool_check_unlocked(file, line, pool);
+spin_unlock(&pool->lock);
+return err;
+}
+
+int __xmem_pool_check(const char *file, int line)
+{
+return __xmem_pool_check_locked(file, line, xenpool);
+}
+#else
+#define xmem_pool_check_locked(__pool) do { if ( 0 && (__pool) ); } while (0)
+#define xmem_pool_check_unlocked(__pool) do { if ( 0 && (__pool) ); } while (0)
+#endif
/**
* Returns indexes (fl, sl) of the list used to serve request of size r
@@ -381,6 +492,8 @@ void *xmem_pool_alloc(unsigned long size, struct xmem_pool
*pool)
int fl, sl;
unsigned long tmp_size;
+xmem_pool_check_locked(pool);
+
if ( pool->init_region == NULL )
{
if ( (region = pool->get_mem(pool->init_size)) == NULL )
@@ -442,11 +555,13 @@ void *xmem_pool_alloc(unsigned long size, struct
xmem_pool *pool)
pool->used_size += (b->size & BLOCK_SIZE_MASK) + BHDR_OVERHEAD;
+xmem_pool_check_unlocked(pool);
spin_unlock(&pool-&g
Re: [Xen-devel] [PATCH] xmalloc: add support for checking the pool integrity
On Thursday 04 December 2014 19:01:40 Mihai Donțu wrote:
> Implemented xmem_pool_check(), xmem_pool_check_locked() and
> xmem_pool_check_unlocked() to verity the integrity of the TLSF matrix.
>
> Signed-off-by: Mihai Donțu
> ---
> xen/common/xmalloc_tlsf.c | 119
> +-
> xen/include/xen/xmalloc.h | 7 +++
> 2 files changed, 124 insertions(+), 2 deletions(-)
>
> diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c
> index a5769c9..009ba60 100644
> --- a/xen/common/xmalloc_tlsf.c
> +++ b/xen/common/xmalloc_tlsf.c
> @@ -120,9 +120,120 @@ struct xmem_pool {
> char name[MAX_POOL_NAME_LEN];
> };
>
> +static struct xmem_pool *xenpool;
> +
> +static inline void MAPPING_INSERT(unsigned long r, int *fl, int *sl);
> +
> /*
> * Helping functions
> */
> +#ifndef NDEBUG
> +static int xmem_pool_check_size(const struct bhdr *b, int fl, int sl)
> +{
> +while ( b )
> +{
> +int __fl;
> +int __sl;
> +
> +MAPPING_INSERT(b->size, &__fl, &__sl);
> +if ( __fl != fl || __sl != sl )
> +{
> +printk(XENLOG_ERR "xmem_pool: for block %p size = %u, { fl = %d,
> sl = %d } should be { fl = %d, sl = %d }\n", b, b->size, fl, sl, __fl, __sl);
> +return 0;
> +}
> +b = b->ptr.free_ptr.next;
> +}
> +return 1;
> +}
> +
> +/*
> + * This function must be called from a context where pool->lock is
> + * already acquired
> + */
> +#define xmem_pool_check_unlocked(__pool)
> __xmem_pool_check_unlocked(__FILE__, __LINE__, __pool)
> +static int __xmem_pool_check_unlocked(const char *file, int line, const
> struct xmem_pool *pool)
> +{
> +int i;
> +int woops = 0;
> +static int once = 1;
> +
> +for ( i = 0; i < REAL_FLI; i++ )
> +{
> +int fl = ( pool->fl_bitmap & (1 << i) ) ? i : -1;
> +
> +if ( fl >= 0 )
> +{
> +int j;
> +int bitmap_empty = 1;
> +int matrix_empty = 1;
> +
> +for ( j = 0; j < MAX_SLI; j++ )
> +{
> +int sl = ( pool->sl_bitmap[fl] & (1 << j) ) ? j : -1;
> +
> +if ( sl < 0 )
> +continue;
> +
> +if ( once && !pool->matrix[fl][sl] )
> +{
> +/* The bitmap is corrupted */
> +printk(XENLOG_ERR "xmem_pool:%s:%d the TLSF bitmap is
> corrupted\n", file, line);
> +__warn((char *)file, line);
> +once = 0;
> +woops = 1;
> +}
> +else if ( once &&
> !xmem_pool_check_size(pool->matrix[fl][sl], fl, sl))
> +{
> +printk(XENLOG_ERR "xmem_pool:%s:%d the TLSF chunk matrix
> is corrupted\n", file, line);
> +__warn((char *)file, line);
> +once = 0;
> +woops = 1;
> +}
> +if ( pool->matrix[fl][sl] )
> +matrix_empty = 0;
> +bitmap_empty = 0;
> +}
> +if ( once && bitmap_empty )
> +{
> +/* The bitmap is corrupted */
> +printk(XENLOG_ERR "xmem_pool:%s:%d the TLSF bitmap is
> corrupted (non-empty FL with empty SL)\n", file, line);
> +__warn((char *)file, line);
> +once = 0;
> +woops = 1;
> +}
> +if ( once && matrix_empty )
> +{
> +/* The bitmap is corrupted */
> +printk(XENLOG_ERR "xmem_pool:%s:%d the TLSF bitmap is
> corrupted (empty matrix)\n", file, line);
> +__warn((char *)file, line);
> +once = 0;
> +woops = 1;
> +}
> +}
> +}
> +
> +return woops;
> +}
> +
> +#define xmem_pool_check_locked(__pool) __xmem_pool_check_locked(__FILE__,
> __LINE__, __pool)
> +static int __xmem_pool_check_locked(const char *file, int line, struct
> xmem_pool *pool)
> +{
> +int err;
> +
> +spin_lock(&pool->lock);
> +err = __xmem_pool_check_unlocked(file, line, pool);
> +spin_unlock(&pool->lock);
> +return err;
> +}
> +
> +int __xmem_pool_check(const char *file, int line)
> +{
> +return __xmem_pool_check_locked(file, line, xenpool);
> +}
> +#else
> +#define xmem_pool_check_locked(__p
Re: [Xen-devel] [PATCH] xmalloc: add support for checking the pool integrity
On Friday 05 December 2014 12:09:02 Jan Beulich wrote:
> >>> On 04.12.14 at 18:01, wrote:
> > --- a/xen/common/xmalloc_tlsf.c
> > +++ b/xen/common/xmalloc_tlsf.c
> > @@ -120,9 +120,120 @@ struct xmem_pool {
> > char name[MAX_POOL_NAME_LEN];
> > };
> >
> > +static struct xmem_pool *xenpool;
> > +
> > +static inline void MAPPING_INSERT(unsigned long r, int *fl, int *sl);
> > +
> > /*
> > * Helping functions
> > */
> > +#ifndef NDEBUG
> > +static int xmem_pool_check_size(const struct bhdr *b, int fl, int sl)
> > +{
> > +while ( b )
> > +{
> > +int __fl;
> > +int __sl;
> > +
> > +MAPPING_INSERT(b->size, &__fl, &__sl);
> > +if ( __fl != fl || __sl != sl )
> > +{
> > +printk(XENLOG_ERR "xmem_pool: for block %p size = %u, { fl =
> > %d, sl = %d } should be { fl = %d, sl = %d }\n", b, b->size, fl, sl, __fl,
> > __sl);
>
> Long line. Only the format message alone is allowed to exceed 80
> characters.
>
> > +return 0;
> > +}
> > +b = b->ptr.free_ptr.next;
> > +}
> > +return 1;
> > +}
> > +
> > +/*
> > + * This function must be called from a context where pool->lock is
> > + * already acquired
> > + */
> > +#define xmem_pool_check_unlocked(__pool)
> > __xmem_pool_check_unlocked(__FILE__, __LINE__, __pool)
>
> No need for the double underscores on the macro parameter.
>
> > +static int __xmem_pool_check_unlocked(const char *file, int line, const
> > struct xmem_pool *pool)
> > +{
> > +int i;
> > +int woops = 0;
> > +static int once = 1;
>
> bool_t
>
> > +
> > +for ( i = 0; i < REAL_FLI; i++ )
> > +{
> > +int fl = ( pool->fl_bitmap & (1 << i) ) ? i : -1;
>
> Bogus spaces inside parentheses.
>
> > +
> > +if ( fl >= 0 )
> > +{
> > +int j;
> > +int bitmap_empty = 1;
> > +int matrix_empty = 1;
>
> For any of the int-s here and above - can they really all become
> negative? If not, they ought to be unsigned int or bool_t.
>
> > +
> > +for ( j = 0; j < MAX_SLI; j++ )
> > +{
> > +int sl = ( pool->sl_bitmap[fl] & (1 << j) ) ? j : -1;
> > +
> > +if ( sl < 0 )
> > +continue;
> > +
> > +if ( once && !pool->matrix[fl][sl] )
> > +{
> > +/* The bitmap is corrupted */
> > +printk(XENLOG_ERR "xmem_pool:%s:%d the TLSF bitmap is
> > corrupted\n", file, line);
> > +__warn((char *)file, line);
>
> Please constify the first parameter of __warn() instead of adding
> fragile casts. I also don't see why file and line need printing twice.
>
> > +static int __xmem_pool_check_locked(const char *file, int line, struct
> > xmem_pool *pool)
> > +{
> > +int err;
> > +
> > +spin_lock(&pool->lock);
> > +err = __xmem_pool_check_unlocked(file, line, pool);
>
> Inversed naming: The caller here should be _unlocked, and the
> callee _locked.
>
> > +#define xmem_pool_check_locked(__pool) do { if ( 0 && (__pool) ); } while
> > (0)
> > +#define xmem_pool_check_unlocked(__pool) do { if ( 0 && (__pool) ); }
> > while (0)
>
> ((void)(pool)) or at least drop the "0 &&" - after all you _want_ the
> macro argument to be evaluated (in order to carry out side effects).
>
> > --- a/xen/include/xen/xmalloc.h
> > +++ b/xen/include/xen/xmalloc.h
> > @@ -123,4 +123,11 @@ unsigned long xmem_pool_get_used_size(struct xmem_pool
> > *pool);
> > */
> > unsigned long xmem_pool_get_total_size(struct xmem_pool *pool);
> >
> > +#ifndef NDEBUG
> > +#define xmem_pool_check() __xmem_pool_check(__FILE__, __LINE__)
> > +int __xmem_pool_check(const char *file, int line);
> > +#else
> > +#define xmem_pool_check() do { if ( 0 ); } while (0)
>
> ((void)0)
>
> or
>
> do {} while (0)
>
> Also perhaps __xmem_pool_check() should have a pool parameter,
> with NULL meaning the default one.
>
Thank you for your review Jan. I'll follow up with an update soon, as
well as a patch for __warn() (and __bug() while I'm at it).
--
Mihai DONȚU
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] console: const-ify the arguments for __warn() and __bug()
Both __warn() and __bug() take as first parameter the file name of the
current compilation unit (__FILE__). Mark that parameter as constant to
better reflect that.
Signed-off-by: Mihai Donțu
---
xen/drivers/char/console.c | 4 ++--
xen/include/xen/lib.h | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 2f03259..7807cf2 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -1138,7 +1138,7 @@ void panic(const char *fmt, ...)
machine_restart(5000);
}
-void __bug(char *file, int line)
+void __bug(const char *file, int line)
{
console_start_sync();
printk("Xen BUG at %s:%d\n", file, line);
@@ -1146,7 +1146,7 @@ void __bug(char *file, int line)
panic("Xen BUG at %s:%d", file, line);
}
-void __warn(char *file, int line)
+void __warn(const char *file, int line)
{
printk("Xen WARN at %s:%d\n", file, line);
dump_execution_state();
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index f11b49e..8f9cadb 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -8,8 +8,8 @@
#include
#include
-void noreturn __bug(char *file, int line);
-void __warn(char *file, int line);
+void noreturn __bug(const char *file, int line);
+void __warn(const char *file, int line);
#define BUG_ON(p) do { if (unlikely(p)) BUG(); } while (0)
#define WARN_ON(p) do { if (unlikely(p)) WARN(); } while (0)
--
2.2.0
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] xmalloc: add support for checking the pool integrity
Implemented xmem_pool_check(), xmem_pool_check_locked() and
xmem_pool_check_unlocked() to verity the integrity of the TLSF matrix.
Signed-off-by: Mihai Donțu
---
Changes since v1:
- fixed the codingstyle
- swaped _locked/_unlocked naming
- reworked __xmem_pool_check_locked() a bit
- used bool_t where appropriate
- made xmem_pool_check() take a pool argument which can be NULL
---
xen/common/xmalloc_tlsf.c | 110 +-
xen/include/xen/xmalloc.h | 7 +++
2 files changed, 115 insertions(+), 2 deletions(-)
diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c
index a5769c9..8681185 100644
--- a/xen/common/xmalloc_tlsf.c
+++ b/xen/common/xmalloc_tlsf.c
@@ -120,9 +120,111 @@ struct xmem_pool {
char name[MAX_POOL_NAME_LEN];
};
+static struct xmem_pool *xenpool;
+
+static inline void MAPPING_INSERT(unsigned long r, int *fl, int *sl);
+
/*
* Helping functions
*/
+#ifndef NDEBUG
+static bool_t xmem_pool_check_size(const struct bhdr *b, int fl, int sl)
+{
+while ( b )
+{
+int __fl;
+int __sl;
+
+MAPPING_INSERT(b->size, &__fl, &__sl);
+if ( __fl != fl || __sl != sl )
+{
+printk(XENLOG_ERR "xmem_pool: for block %p size = %u, { fl = %d,
sl = %d } should be { fl = %d, sl = %d }\n",
+ b, b->size, fl, sl, __fl, __sl);
+return 0;
+}
+b = b->ptr.free_ptr.next;
+}
+return 1;
+}
+
+/*
+ * This function must be called from a context where pool->lock is
+ * already acquired.
+ *
+ * Returns true if the pool has been corrupted, false otherwise
+ */
+#define xmem_pool_check_locked(pool) __xmem_pool_check_locked(__FILE__,
__LINE__, pool)
+static bool_t __xmem_pool_check_locked(const char *file, int line, const
struct xmem_pool *pool)
+{
+int i;
+static bool_t once = 1;
+
+if ( !once )
+goto out;
+for ( i = 0; i < REAL_FLI; i++ )
+{
+int fl = (pool->fl_bitmap & (1 << i)) ? i : -1;
+
+if ( fl >= 0 )
+{
+int j;
+
+if ( !pool->sl_bitmap[fl] )
+{
+printk(XENLOG_ERR "xmem_pool: the TLSF bitmap is corrupted
(non-empty FL with empty SL)\n");
+__warn(file, line);
+once = 0;
+break;
+}
+for ( j = 0; j < MAX_SLI; j++ )
+{
+int sl = (pool->sl_bitmap[fl] & (1 << j)) ? j : -1;
+
+if ( sl < 0 )
+continue;
+if ( !pool->matrix[fl][sl] )
+{
+printk(XENLOG_ERR "xmem_pool: the TLSF bitmap is corrupted
(matrix[%d][%d] is NULL)\n",
+fl, sl);
+__warn(file, line);
+once = 0;
+break;
+}
+if ( !xmem_pool_check_size(pool->matrix[fl][sl], fl, sl) )
+{
+printk(XENLOG_ERR "xmem_pool: the TLSF chunk matrix is
corrupted\n");
+__warn(file, line);
+once = 0;
+break;
+}
+}
+if ( !once )
+break;
+}
+}
+out:
+return !once;
+}
+
+#define xmem_pool_check_unlocked(pool) __xmem_pool_check_unlocked(__FILE__,
__LINE__, pool)
+static bool_t __xmem_pool_check_unlocked(const char *file, int line, struct
xmem_pool *pool)
+{
+bool_t oops;
+
+spin_lock(&pool->lock);
+oops = __xmem_pool_check_locked(file, line, pool);
+spin_unlock(&pool->lock);
+return oops;
+}
+
+bool_t __xmem_pool_check(const char *file, int line, struct xmem_pool *pool)
+{
+return __xmem_pool_check_unlocked(file, line, pool ? pool : xenpool);
+}
+#else
+#define xmem_pool_check_locked(pool) ((void)(pool))
+#define xmem_pool_check_unlocked(pool) ((void)(pool))
+#endif
/**
* Returns indexes (fl, sl) of the list used to serve request of size r
@@ -381,6 +483,8 @@ void *xmem_pool_alloc(unsigned long size, struct xmem_pool
*pool)
int fl, sl;
unsigned long tmp_size;
+xmem_pool_check_unlocked(pool);
+
if ( pool->init_region == NULL )
{
if ( (region = pool->get_mem(pool->init_size)) == NULL )
@@ -442,11 +546,13 @@ void *xmem_pool_alloc(unsigned long size, struct
xmem_pool *pool)
pool->used_size += (b->size & BLOCK_SIZE_MASK) + BHDR_OVERHEAD;
+xmem_pool_check_locked(pool);
spin_unlock(&pool->lock);
return (void *)b->ptr.buffer;
/* Failed alloc */
out_locked:
+xmem_pool_check_locked(pool);
spin_unlock(&pool->lock);
out:
@@ -464,6 +570,7 @@ void xmem_pool_free(void *ptr, struct xmem_pool *pool)
b = (struct bhdr *)((char *) ptr - BHDR_OVERHEAD);
spin_lock(&pool->lock);
+xmem_pool_
Re: [Xen-devel] [PATCH v2] xmalloc: add support for checking the pool integrity
On Mon, 8 Dec 2014 04:30:48 +0200 Mihai Donțu wrote:
> Implemented xmem_pool_check(), xmem_pool_check_locked() and
> xmem_pool_check_unlocked() to verity the integrity of the TLSF matrix.
>
> Signed-off-by: Mihai Donțu
>
> ---
> Changes since v1:
> - fixed the codingstyle
> - swaped _locked/_unlocked naming
> - reworked __xmem_pool_check_locked() a bit
> - used bool_t where appropriate
> - made xmem_pool_check() take a pool argument which can be NULL
> ---
> xen/common/xmalloc_tlsf.c | 110
> +-
> xen/include/xen/xmalloc.h | 7 +++
> 2 files changed, 115 insertions(+), 2 deletions(-)
>
> diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c
> index a5769c9..8681185 100644
> --- a/xen/common/xmalloc_tlsf.c
> +++ b/xen/common/xmalloc_tlsf.c
> @@ -120,9 +120,111 @@ struct xmem_pool {
> char name[MAX_POOL_NAME_LEN];
> };
>
> +static struct xmem_pool *xenpool;
> +
> +static inline void MAPPING_INSERT(unsigned long r, int *fl, int *sl);
> +
> /*
> * Helping functions
> */
> +#ifndef NDEBUG
> +static bool_t xmem_pool_check_size(const struct bhdr *b, int fl, int sl)
> +{
> +while ( b )
> +{
> +int __fl;
> +int __sl;
> +
> +MAPPING_INSERT(b->size, &__fl, &__sl);
> +if ( __fl != fl || __sl != sl )
> +{
> +printk(XENLOG_ERR "xmem_pool: for block %p size = %u, { fl = %d,
> sl = %d } should be { fl = %d, sl = %d }\n",
> + b, b->size, fl, sl, __fl, __sl);
> +return 0;
> +}
> +b = b->ptr.free_ptr.next;
> +}
> +return 1;
> +}
> +
> +/*
> + * This function must be called from a context where pool->lock is
> + * already acquired.
> + *
> + * Returns true if the pool has been corrupted, false otherwise
> + */
> +#define xmem_pool_check_locked(pool) __xmem_pool_check_locked(__FILE__,
> __LINE__, pool)
> +static bool_t __xmem_pool_check_locked(const char *file, int line, const
> struct xmem_pool *pool)
> +{
> +int i;
> +static bool_t once = 1;
> +
> +if ( !once )
> +goto out;
> +for ( i = 0; i < REAL_FLI; i++ )
> +{
> +int fl = (pool->fl_bitmap & (1 << i)) ? i : -1;
> +
> +if ( fl >= 0 )
> +{
> +int j;
> +
> +if ( !pool->sl_bitmap[fl] )
> +{
> +printk(XENLOG_ERR "xmem_pool: the TLSF bitmap is corrupted
> (non-empty FL with empty SL)\n");
> +__warn(file, line);
> +once = 0;
> +break;
> +}
> +for ( j = 0; j < MAX_SLI; j++ )
> +{
> +int sl = (pool->sl_bitmap[fl] & (1 << j)) ? j : -1;
> +
> +if ( sl < 0 )
> +continue;
> +if ( !pool->matrix[fl][sl] )
> +{
> +printk(XENLOG_ERR "xmem_pool: the TLSF bitmap is
> corrupted (matrix[%d][%d] is NULL)\n",
> +fl, sl);
> +__warn(file, line);
> +once = 0;
> +break;
> +}
> +if ( !xmem_pool_check_size(pool->matrix[fl][sl], fl, sl) )
> +{
> +printk(XENLOG_ERR "xmem_pool: the TLSF chunk matrix is
> corrupted\n");
> +__warn(file, line);
> +once = 0;
> +break;
> +}
> +}
> +if ( !once )
> +break;
> +}
> +}
> +out:
> +return !once;
> +}
> +
> +#define xmem_pool_check_unlocked(pool) __xmem_pool_check_unlocked(__FILE__,
> __LINE__, pool)
> +static bool_t __xmem_pool_check_unlocked(const char *file, int line, struct
> xmem_pool *pool)
> +{
> +bool_t oops;
> +
> +spin_lock(&pool->lock);
> +oops = __xmem_pool_check_locked(file, line, pool);
> +spin_unlock(&pool->lock);
> +return oops;
> +}
> +
> +bool_t __xmem_pool_check(const char *file, int line, struct xmem_pool *pool)
> +{
> +return __xmem_pool_check_unlocked(file, line, pool ? pool : xenpool);
> +}
> +#else
> +#define xmem_pool_check_locked(pool) ((void)(pool))
> +#define xmem_pool_check_unlocked(pool) ((void)(pool))
> +#endif
>
> /**
> * Returns indexes (fl, sl) of the list used to serve request of size r
> @@ -381,6 +483,8 @@ void *xmem_pool_alloc(unsigned long size, struct
> xmem_pool *pool)
> int fl
Re: [Xen-devel] [PATCH v2] xmalloc: add support for checking the pool integrity
On Monday 08 December 2014 10:18:01 Jan Beulich wrote:
> >>> On 08.12.14 at 03:30, wrote:
> > +#ifndef NDEBUG
> > +static bool_t xmem_pool_check_size(const struct bhdr *b, int fl, int sl)
> > +{
> > +while ( b )
> > +{
> > +int __fl;
> > +int __sl;
> > +
> > +MAPPING_INSERT(b->size, &__fl, &__sl);
> > +if ( __fl != fl || __sl != sl )
> > +{
> > +printk(XENLOG_ERR "xmem_pool: for block %p size = %u, { fl =
> > %d, sl = %d } should be { fl = %d, sl = %d }\n",
>
> Quoting my reply to v1: "Long line. Only the format message alone
> is allowed to exceed 80 characters."
>
Just so I don't send another faulty patch, you would see that printk()
being:
printk(XENLOG_ERR
"xmem_pool: for block %p size = %u, { fl = %d, sl = %d } should be {
fl = %d, sl = %d }\n",
b, b->size, fl, sl, __fl, __sl);
?
> Also with there potentially being multiple pools, shouldn't all of the
> log messages the patch issues be extended to allow identifying the
> offending one?
>
I think I can insert the pool name in that message too. Something like:
printk(XENLOG_ERR
"xmem_pool: %s: for block [...]\n",
pool->name, b, b->size [...]);
would do? A quick preview:
[2014-12-04 15:41:23] (XEN) [ 1374.507125] xmem_pool: xmalloc: for block
8304004fb9b0 size = 0, { fl = 3, sl = 9 } should be { fl = 0, sl = 0 }
[2014-12-04 15:41:23] (XEN) [ 1374.507127] xmem_pool: xmalloc: the TLSF chunk
matrix is corrupted
> > +bool_t __xmem_pool_check(const char *file, int line, struct xmem_pool
> > *pool)
> > +{
> > +return __xmem_pool_check_unlocked(file, line, pool ? pool : xenpool);
>
> For brevity, the shorter "pool ?: xenpool" is generally preferable. The
> only place using this is not allowed are the public headers.
>
Will do.
Thank you,
--
Mihai DONȚU
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xmalloc: add support for checking the pool integrity
On Monday 08 December 2014 16:04:55 Ian Campbell wrote:
> On Mon, 2014-12-08 at 18:00 +0200, Mihai Donțu wrote:
> > On Monday 08 December 2014 10:18:01 Jan Beulich wrote:
> > > >>> On 08.12.14 at 03:30, wrote:
> > > > +#ifndef NDEBUG
> > > > +static bool_t xmem_pool_check_size(const struct bhdr *b, int fl, int
> > > > sl)
> > > > +{
> > > > +while ( b )
> > > > +{
> > > > +int __fl;
> > > > +int __sl;
> > > > +
> > > > +MAPPING_INSERT(b->size, &__fl, &__sl);
> > > > +if ( __fl != fl || __sl != sl )
> > > > +{
> > > > +printk(XENLOG_ERR "xmem_pool: for block %p size = %u, { fl
> > > > = %d, sl = %d } should be { fl = %d, sl = %d }\n",
> > >
> > > Quoting my reply to v1: "Long line. Only the format message alone
> > > is allowed to exceed 80 characters."
> > >
> >
> > Just so I don't send another faulty patch, you would see that printk()
> > being:
> >
> > printk(XENLOG_ERR
> > "xmem_pool: for block %p size = %u, { fl = %d, sl = %d } should be
> > { fl = %d, sl = %d }\n",
> > b, b->size, fl, sl, __fl, __sl);
> >
> > ?
>
> The log message here is going to be substantially more than 80
> characters (the format string by itself already is). Could you find a
> more compact representation of the useful info?
>
Ah! I see. I apologize for being so slow. :-) How about:
printk(XENLOG_ERR "xmem_pool: %s: misplaced block %p:%u ({%d,%d} -> {%d,%d})\n",
pool->name, b, b->size, fl, sl, __fl, __sl);
Looks a bit cryptic, but the TLSF itself is pretty complex and for
brave souls wishing to debug it, the message format will be the last
thing on their minds. :-)
Preview:
[2014-12-04 15:41:23] (XEN) [ 1374.507125] xmem_pool: xmalloc: misplaced block
8304004fb9b0:0 ({3,9} -> {0,0})
[2014-12-04 15:41:23] (XEN) [ 1374.507127] xmem_pool: xmalloc: the TLSF chunk
matrix is corrupted
Thanks,
--
Mihai DONȚU
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] console: const-ify the arguments for __warn() and __bug()
On Monday 08 December 2014 10:28:41 Andrew Cooper wrote:
> On 08/12/14 00:19, Mihai Donțu wrote:
> > Both __warn() and __bug() take as first parameter the file name of the
> > current compilation unit (__FILE__). Mark that parameter as constant to
> > better reflect that.
> >
> > Signed-off-by: Mihai Donțu
>
> This seems reasonable, but for 4.6 at this point.
>
> Reviewed-by: Andrew Cooper
>
> In future, please CC the relevant maintainers (scrips/get_maintainer.pl
> should help)
Thank you Andrew.
Should I resend the patch with your Reviewed-by and the proper people in
CC?
> > ---
> > xen/drivers/char/console.c | 4 ++--
> > xen/include/xen/lib.h | 4 ++--
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> > index 2f03259..7807cf2 100644
> > --- a/xen/drivers/char/console.c
> > +++ b/xen/drivers/char/console.c
> > @@ -1138,7 +1138,7 @@ void panic(const char *fmt, ...)
> > machine_restart(5000);
> > }
> >
> > -void __bug(char *file, int line)
> > +void __bug(const char *file, int line)
> > {
> > console_start_sync();
> > printk("Xen BUG at %s:%d\n", file, line);
> > @@ -1146,7 +1146,7 @@ void __bug(char *file, int line)
> > panic("Xen BUG at %s:%d", file, line);
> > }
> >
> > -void __warn(char *file, int line)
> > +void __warn(const char *file, int line)
> > {
> > printk("Xen WARN at %s:%d\n", file, line);
> > dump_execution_state();
> > diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
> > index f11b49e..8f9cadb 100644
> > --- a/xen/include/xen/lib.h
> > +++ b/xen/include/xen/lib.h
> > @@ -8,8 +8,8 @@
> > #include
> > #include
> >
> > -void noreturn __bug(char *file, int line);
> > -void __warn(char *file, int line);
> > +void noreturn __bug(const char *file, int line);
> > +void __warn(const char *file, int line);
> >
> > #define BUG_ON(p) do { if (unlikely(p)) BUG(); } while (0)
> > #define WARN_ON(p) do { if (unlikely(p)) WARN(); } while (0)
--
Mihai DONȚU
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3] xmalloc: add support for checking the pool integrity
Implemented xmem_pool_check(), xmem_pool_check_locked() and
xmem_pool_check_unlocked() to verity the integrity of the TLSF matrix.
Signed-off-by: Mihai Donțu
---
Changes since v2:
- print the name of the corrupted pool
- adjusted the messages to better fit within 80 columns
- minor style change (a ? a : b -> a ?: b)
Changes since v1:
- fixed the codingstyle
- swaped _locked/_unlocked naming
- reworked __xmem_pool_check_locked() a bit
- used bool_t where appropriate
- made xmem_pool_check() take a pool argument which can be NULL
---
xen/common/xmalloc_tlsf.c | 117 +-
xen/include/xen/xmalloc.h | 7 +++
2 files changed, 122 insertions(+), 2 deletions(-)
diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c
index a5769c9..1dea137 100644
--- a/xen/common/xmalloc_tlsf.c
+++ b/xen/common/xmalloc_tlsf.c
@@ -120,9 +120,118 @@ struct xmem_pool {
char name[MAX_POOL_NAME_LEN];
};
+static struct xmem_pool *xenpool;
+
+static inline void MAPPING_INSERT(unsigned long r, int *fl, int *sl);
+
/*
* Helping functions
*/
+#ifndef NDEBUG
+static bool_t xmem_pool_check_size(const struct xmem_pool *pool, int fl, int
sl)
+{
+const struct bhdr *b = pool->matrix[fl][sl];
+
+while ( b )
+{
+int __fl;
+int __sl;
+
+MAPPING_INSERT(b->size, &__fl, &__sl);
+if ( __fl != fl || __sl != sl )
+{
+printk(XENLOG_ERR
+ "xmem_pool: %s: misplaced block %p:%u ({%d,%d} ->
{%d,%d})\n",
+ pool->name, b, b->size, fl, sl, __fl, __sl);
+return 0;
+}
+b = b->ptr.free_ptr.next;
+}
+return 1;
+}
+
+/*
+ * This function must be called from a context where pool->lock is
+ * already acquired.
+ *
+ * Returns true if the pool has been corrupted, false otherwise
+ */
+#define xmem_pool_check_locked(pool) __xmem_pool_check_locked(__FILE__,
__LINE__, pool)
+static bool_t __xmem_pool_check_locked(const char *file, int line, const
struct xmem_pool *pool)
+{
+int i;
+static bool_t once = 1;
+
+if ( !once )
+goto out;
+for ( i = 0; i < REAL_FLI; i++ )
+{
+int fl = (pool->fl_bitmap & (1 << i)) ? i : -1;
+
+if ( fl >= 0 )
+{
+int j;
+
+if ( !pool->sl_bitmap[fl] )
+{
+printk(XENLOG_ERR
+ "xmem_pool: %s: the TLSF bitmap is corrupted (non-empty
FL with empty SL)\n",
+ pool->name);
+__warn(file, line);
+once = 0;
+break;
+}
+for ( j = 0; j < MAX_SLI; j++ )
+{
+int sl = (pool->sl_bitmap[fl] & (1 << j)) ? j : -1;
+
+if ( sl < 0 )
+continue;
+if ( !pool->matrix[fl][sl] )
+{
+printk(XENLOG_ERR
+ "xmem_pool: %s: the TLSF bitmap is corrupted
(matrix[%d][%d] is NULL)\n",
+ pool->name, fl, sl);
+__warn(file, line);
+once = 0;
+break;
+}
+if ( !xmem_pool_check_size(pool, fl, sl) )
+{
+printk(XENLOG_ERR "xmem_pool: %s: the TLSF chunk matrix is
corrupted\n",
+ pool->name);
+__warn(file, line);
+once = 0;
+break;
+}
+}
+if ( !once )
+break;
+}
+}
+out:
+return !once;
+}
+
+#define xmem_pool_check_unlocked(pool) __xmem_pool_check_unlocked(__FILE__,
__LINE__, pool)
+static bool_t __xmem_pool_check_unlocked(const char *file, int line, struct
xmem_pool *pool)
+{
+bool_t oops;
+
+spin_lock(&pool->lock);
+oops = __xmem_pool_check_locked(file, line, pool);
+spin_unlock(&pool->lock);
+return oops;
+}
+
+bool_t __xmem_pool_check(const char *file, int line, struct xmem_pool *pool)
+{
+return __xmem_pool_check_unlocked(file, line, pool ?: xenpool);
+}
+#else
+#define xmem_pool_check_locked(pool) ((void)(pool))
+#define xmem_pool_check_unlocked(pool) ((void)(pool))
+#endif
/**
* Returns indexes (fl, sl) of the list used to serve request of size r
@@ -381,6 +490,8 @@ void *xmem_pool_alloc(unsigned long size, struct xmem_pool
*pool)
int fl, sl;
unsigned long tmp_size;
+xmem_pool_check_unlocked(pool);
+
if ( pool->init_region == NULL )
{
if ( (region = pool->get_mem(pool->init_size)) == NULL )
@@ -442,11 +553,13 @@ void *xmem_pool_alloc(unsigned long size, struct
xmem_pool *pool)
pool->used_size += (b->size & BLOCK_SIZE_MASK) + BHDR_OVERHEAD;
+xmem_pool_check_locked(pool);
[Xen-devel] [PATCH] console: const-ify the arguments for __warn() and __bug()
Both __warn() and __bug() take as first parameter the file name of the
current compilation unit (__FILE__). Mark that parameter as constant to
better reflect that.
Signed-off-by: Mihai Donțu
Reviewed-by: Andrew Cooper
---
xen/drivers/char/console.c | 4 ++--
xen/include/xen/lib.h | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 2f03259..7807cf2 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -1138,7 +1138,7 @@ void panic(const char *fmt, ...)
machine_restart(5000);
}
-void __bug(char *file, int line)
+void __bug(const char *file, int line)
{
console_start_sync();
printk("Xen BUG at %s:%d\n", file, line);
@@ -1146,7 +1146,7 @@ void __bug(char *file, int line)
panic("Xen BUG at %s:%d", file, line);
}
-void __warn(char *file, int line)
+void __warn(const char *file, int line)
{
printk("Xen WARN at %s:%d\n", file, line);
dump_execution_state();
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index f11b49e..8f9cadb 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -8,8 +8,8 @@
#include
#include
-void noreturn __bug(char *file, int line);
-void __warn(char *file, int line);
+void noreturn __bug(const char *file, int line);
+void __warn(const char *file, int line);
#define BUG_ON(p) do { if (unlikely(p)) BUG(); } while (0)
#define WARN_ON(p) do { if (unlikely(p)) WARN(); } while (0)
--
2.2.0
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3] xmalloc: add support for checking the pool integrity
On Wed, 10 Dec 2014 14:13:58 +0200 Mihai Donțu wrote:
> Implemented xmem_pool_check(), xmem_pool_check_locked() and
> xmem_pool_check_unlocked() to verity the integrity of the TLSF matrix.
>
> Signed-off-by: Mihai Donțu
>
> ---
> Changes since v2:
> - print the name of the corrupted pool
> - adjusted the messages to better fit within 80 columns
> - minor style change (a ? a : b -> a ?: b)
>
> Changes since v1:
> - fixed the codingstyle
> - swaped _locked/_unlocked naming
> - reworked __xmem_pool_check_locked() a bit
> - used bool_t where appropriate
> - made xmem_pool_check() take a pool argument which can be NULL
> ---
> xen/common/xmalloc_tlsf.c | 117
> +-
> xen/include/xen/xmalloc.h | 7 +++
> 2 files changed, 122 insertions(+), 2 deletions(-)
>
> diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c
> index a5769c9..1dea137 100644
> --- a/xen/common/xmalloc_tlsf.c
> +++ b/xen/common/xmalloc_tlsf.c
> @@ -120,9 +120,118 @@ struct xmem_pool {
> char name[MAX_POOL_NAME_LEN];
> };
>
> +static struct xmem_pool *xenpool;
> +
> +static inline void MAPPING_INSERT(unsigned long r, int *fl, int *sl);
> +
> /*
> * Helping functions
> */
> +#ifndef NDEBUG
> +static bool_t xmem_pool_check_size(const struct xmem_pool *pool, int fl, int
> sl)
> +{
> +const struct bhdr *b = pool->matrix[fl][sl];
> +
> +while ( b )
> +{
> +int __fl;
> +int __sl;
> +
> +MAPPING_INSERT(b->size, &__fl, &__sl);
> +if ( __fl != fl || __sl != sl )
> +{
> +printk(XENLOG_ERR
> + "xmem_pool: %s: misplaced block %p:%u ({%d,%d} ->
> {%d,%d})\n",
> + pool->name, b, b->size, fl, sl, __fl, __sl);
> +return 0;
> +}
> +b = b->ptr.free_ptr.next;
> +}
> +return 1;
> +}
> +
> +/*
> + * This function must be called from a context where pool->lock is
> + * already acquired.
> + *
> + * Returns true if the pool has been corrupted, false otherwise
> + */
> +#define xmem_pool_check_locked(pool) __xmem_pool_check_locked(__FILE__,
> __LINE__, pool)
> +static bool_t __xmem_pool_check_locked(const char *file, int line, const
> struct xmem_pool *pool)
> +{
> +int i;
> +static bool_t once = 1;
> +
> +if ( !once )
> +goto out;
> +for ( i = 0; i < REAL_FLI; i++ )
> +{
> +int fl = (pool->fl_bitmap & (1 << i)) ? i : -1;
> +
> +if ( fl >= 0 )
> +{
> +int j;
> +
> +if ( !pool->sl_bitmap[fl] )
> +{
> +printk(XENLOG_ERR
> + "xmem_pool: %s: the TLSF bitmap is corrupted
> (non-empty FL with empty SL)\n",
> + pool->name);
> +__warn(file, line);
> +once = 0;
> +break;
> +}
> +for ( j = 0; j < MAX_SLI; j++ )
> +{
> +int sl = (pool->sl_bitmap[fl] & (1 << j)) ? j : -1;
> +
> +if ( sl < 0 )
> +continue;
> +if ( !pool->matrix[fl][sl] )
> +{
> +printk(XENLOG_ERR
> + "xmem_pool: %s: the TLSF bitmap is corrupted
> (matrix[%d][%d] is NULL)\n",
> + pool->name, fl, sl);
> +__warn(file, line);
> +once = 0;
> +break;
> +}
> +if ( !xmem_pool_check_size(pool, fl, sl) )
> +{
> +printk(XENLOG_ERR "xmem_pool: %s: the TLSF chunk matrix
> is corrupted\n",
> + pool->name);
> +__warn(file, line);
> +once = 0;
> +break;
> +}
> +}
> +if ( !once )
> +break;
> +}
> +}
> +out:
> +return !once;
> +}
> +
> +#define xmem_pool_check_unlocked(pool) __xmem_pool_check_unlocked(__FILE__,
> __LINE__, pool)
> +static bool_t __xmem_pool_check_unlocked(const char *file, int line, struct
> xmem_pool *pool)
> +{
> +bool_t oops;
> +
> +spin_lock(&pool->lock);
> +oops = __xmem_pool_check_locked(file, line, pool);
> +spin_unlock(&pool->lock);
> +return oops;
> +}
> +
> +bool_t __xmem_pool_check(const char *file, int line, struct xmem_pool *pool)
[Xen-devel] [PATCH v4] xmalloc: add support for checking the pool integrity
Implemented xmem_pool_check(), xmem_pool_check_locked() and
xmem_pool_check_unlocked() to verity the integrity of the TLSF matrix.
Signed-off-by: Mihai Donțu
---
Changes since v3:
- try harder to respect the 80 column limit
- use 'unsigned int' instead of 'int' where possible
- made the logged messages even shorter
- dropped the use of goto (didn't really make sense)
Changes since v2:
- print the name of the corrupted pool
- adjusted the messages to better fit within 80 columns
- minor style change (a ? a : b -> a ?: b)
Changes since v1:
- fixed the codingstyle
- swaped _locked/_unlocked naming
- reworked __xmem_pool_check_locked() a bit
- used bool_t where appropriate
- made xmem_pool_check() take a pool argument which can be NULL
---
xen/common/xmalloc_tlsf.c | 121 +-
xen/include/xen/xmalloc.h | 7 +++
2 files changed, 126 insertions(+), 2 deletions(-)
diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c
index a5769c9..eca4f1c 100644
--- a/xen/common/xmalloc_tlsf.c
+++ b/xen/common/xmalloc_tlsf.c
@@ -120,9 +120,122 @@ struct xmem_pool {
char name[MAX_POOL_NAME_LEN];
};
+static struct xmem_pool *xenpool;
+
+static inline void MAPPING_INSERT(unsigned long r, int *fl, int *sl);
+
/*
* Helping functions
*/
+#ifndef NDEBUG
+static bool_t xmem_pool_check_size(const struct xmem_pool *pool,
+ int fl, int sl)
+{
+const struct bhdr *b = pool->matrix[fl][sl];
+
+while ( b )
+{
+int __fl;
+int __sl;
+
+MAPPING_INSERT(b->size, &__fl, &__sl);
+if ( __fl != fl || __sl != sl )
+{
+printk(XENLOG_ERR
+ "xmem_pool: %s: misplaced block %p:%u ({%d,%d} ->
{%d,%d})\n",
+ pool->name, b, b->size, fl, sl, __fl, __sl);
+return 0;
+}
+b = b->ptr.free_ptr.next;
+}
+return 1;
+}
+
+/*
+ * This function must be called from a context where pool->lock is
+ * already acquired.
+ *
+ * Returns true if the pool has been corrupted, false otherwise
+ */
+#define xmem_pool_check_locked(pool) \
+__xmem_pool_check_locked(__FILE__, __LINE__, pool)
+static bool_t __xmem_pool_check_locked(const char *file, int line,
+ const struct xmem_pool *pool)
+{
+unsigned int i;
+static bool_t once = 1;
+
+if ( !once )
+return 1;
+for ( i = 0; i < REAL_FLI; i++ )
+{
+int fl = (pool->fl_bitmap & (1 << i)) ? i : -1;
+
+if ( fl >= 0 )
+{
+unsigned int j;
+
+if ( !pool->sl_bitmap[fl] )
+{
+printk(XENLOG_ERR
+ "xmem_pool: %s: TLSF bitmap corrupt (!empty FL, empty
SL)\n",
+ pool->name);
+__warn(file, line);
+once = 0;
+break;
+}
+for ( j = 0; j < MAX_SLI; j++ )
+{
+int sl = (pool->sl_bitmap[fl] & (1 << j)) ? j : -1;
+
+if ( sl < 0 )
+continue;
+if ( !pool->matrix[fl][sl] )
+{
+printk(XENLOG_ERR
+ "xmem_pool: %s: TLSF bitmap corrupt
(!matrix[%d][%d])\n",
+ pool->name, fl, sl);
+__warn(file, line);
+once = 0;
+break;
+}
+if ( !xmem_pool_check_size(pool, fl, sl) )
+{
+printk(XENLOG_ERR "xmem_pool: %s: TLSF chunk matrix
corrupt\n",
+ pool->name);
+__warn(file, line);
+once = 0;
+break;
+}
+}
+if ( !once )
+break;
+}
+}
+return !once;
+}
+
+#define xmem_pool_check_unlocked(pool) \
+__xmem_pool_check_unlocked(__FILE__, __LINE__, pool)
+static bool_t __xmem_pool_check_unlocked(const char *file, int line,
+ struct xmem_pool *pool)
+{
+bool_t oops;
+
+spin_lock(&pool->lock);
+oops = __xmem_pool_check_locked(file, line, pool);
+spin_unlock(&pool->lock);
+return oops;
+}
+
+bool_t __xmem_pool_check(const char *file, int line, struct xmem_pool *pool)
+{
+return __xmem_pool_check_unlocked(file, line, pool ?: xenpool);
+}
+#else
+#define xmem_pool_check_locked(pool) ((void)(pool))
+#define xmem_pool_check_unlocked(pool) ((void)(pool))
+#endif
/**
* Returns indexes (fl, sl) of the list used to serve request of size r
@@ -381,6 +494,8 @@ void *xmem_pool_alloc(unsigned long size, struct xmem_pool
*pool)
int fl, sl;
unsigned long tmp_size;
+xmem_pool_check_unlocked
Re: [Xen-devel] Interfering with memory accesses
On Tue, 14 Jul 2015 21:24:04 +0500 Bilal Bakht Ahmad wrote: > Is there anyway to intercept memory accesses from the guest OS in the > hypervisor? Say one wants to detect memory access in a range of addresses > or such. Without hiding pages and page faults because that would induce a > large time penalty. Have you looked at http://libvmi.com/ ? -- Mihai Donțu ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
