[PATCH] Fix arguments passed to SHADOW_PRINTK

[PATCH] Fix arguments passed to SHADOW_PRINTK

[Yoshiaki Tamura]

Hi.

When I compiled xen-3.3-testing with DEBUG_TRACE_DUMP in xen/include/xen/lib.h
turned on, I got some errors at SHADOW_PRINTK.
The following patch will fix the arguments passed to SHADOW_PRINTK in
xen/arch/x86/mm/shadow/common.c and xen/arch/x86/mm/shadow/multi.c.

Although I haven't tested, it is the same for xen-unstable.hg

Thanks,

Yoshi

Signed-off-by: Yoshi Tamura <tamura.yoshiaki@lab.ntt.co.jp>

diff -r bfd1157dd315 xen/arch/x86/mm/shadow/common.c
--- a/xen/arch/x86/mm/shadow/common.c	Thu Sep 11 08:06:48 2008 +0900
+++ b/xen/arch/x86/mm/shadow/common.c	Thu Sep 11 09:25:28 2008 +0900
@@ -701,7 +701,8 @@ static void _sh_resync(struct vcpu *v, m
     ASSERT(!sh_page_has_multiple_shadows(mfn_to_page(gmfn)));

     SHADOW_PRINTK("d=%d, v=%d, gmfn=%05lx, va=%lx\n",
-                  v->domain->domain_id, v->vcpu_id, mfn_x(gmfn), va);
+                  v->domain->domain_id, v->vcpu_id, mfn_x(gmfn),
+                  sh_map_domain_page(gmfn));

     /* Need to pull write access so the page *stays* in sync. */
     if ( oos_remove_write_access(v, gmfn, fixup) )
@@ -953,7 +954,8 @@ int sh_unsync(struct vcpu *v, mfn_t gmfn
     ASSERT(shadow_locked_by_me(v->domain));

     SHADOW_PRINTK("d=%d, v=%d, gmfn=%05lx va %lx\n",
-                  v->domain->domain_id, v->vcpu_id, mfn_x(gmfn), va);
+                  v->domain->domain_id, v->vcpu_id, mfn_x(gmfn),
+                  sh_map_domain_page(gmfn));

     pg = mfn_to_page(gmfn);

diff -r bfd1157dd315 xen/arch/x86/mm/shadow/multi.c
--- a/xen/arch/x86/mm/shadow/multi.c	Thu Sep 11 08:06:48 2008 +0900
+++ b/xen/arch/x86/mm/shadow/multi.c	Thu Sep 11 09:49:09 2008 +0900
@@ -3041,9 +3041,14 @@ static int sh_page_fault(struct vcpu *v,
     int fast_emul = 0;
 #endif

+#ifdef __x86_64__
     SHADOW_PRINTK("d:v=%u:%u va=%#lx err=%u, rip=%lx\n",
                   v->domain->domain_id, v->vcpu_id, va, regs->error_code,
                   regs->rip);
+#elif defined(__i386__)
+    SHADOW_PRINTK("d:v=%u:%u va=%#lx err=%u\n",
+                  v->domain->domain_id, v->vcpu_id, va, regs->error_code);
+#endif /* __i386__ */

     perfc_incr(shadow_fault);

-- 
TAMURA, Yoshiaki

NTT Cyber Space Labs
OSS Computing Project
Kernel Group
E-mail: tamura.yoshiaki@lab.ntt.co.jp
TEL: +81-46-859-2771
FAX: +81-46-855-1152
Address: 1-1 Hikarinooka, Yokosuka
	 Kanagawa 239-0847 JAPAN

Re: [PATCH] Fix arguments passed to SHADOW_PRINTK

[Gianluca Guida]

Hi,


Yoshiaki Tamura wrote:
> Hi.
> 
> When I compiled xen-3.3-testing with DEBUG_TRACE_DUMP in xen/include/xen/lib.h
> turned on, I got some errors at SHADOW_PRINTK.
> The following patch will fix the arguments passed to SHADOW_PRINTK in
> xen/arch/x86/mm/shadow/common.c and xen/arch/x86/mm/shadow/multi.c.

My fault, the va argument was removed with recent changes in the OOS
code and it is unneeded now.

>      SHADOW_PRINTK("d=%d, v=%d, gmfn=%05lx, va=%lx\n",
> -                  v->domain->domain_id, v->vcpu_id, mfn_x(gmfn), va);
> +                  v->domain->domain_id, v->vcpu_id, mfn_x(gmfn),
> +                  sh_map_domain_page(gmfn));
> 
>      /* Need to pull write access so the page *stays* in sync. */
>      if ( oos_remove_write_access(v, gmfn, fixup) )
> @@ -953,7 +954,8 @@ int sh_unsync(struct vcpu *v, mfn_t gmfn
>      ASSERT(shadow_locked_by_me(v->domain));
> 
>      SHADOW_PRINTK("d=%d, v=%d, gmfn=%05lx va %lx\n",
> -                  v->domain->domain_id, v->vcpu_id, mfn_x(gmfn), va);
> +                  v->domain->domain_id, v->vcpu_id, mfn_x(gmfn),
> +                  sh_map_domain_page(gmfn));

These will likely break Xen in 32 bit, since you should unmap the gmfn
in those architecture. As I said, the va argument is unneeded now, so
you should remove it.

Thanks,
Gianluca

Re: [PATCH] Fix arguments passed to SHADOW_PRINTK

[Keir Fraser]

On 11/9/08 11:44, "Yoshiaki Tamura" <tamura.yoshiaki@lab.ntt.co.jp> wrote:

> When I compiled xen-3.3-testing with DEBUG_TRACE_DUMP in xen/include/xen/lib.h
> turned on, I got some errors at SHADOW_PRINTK.
> The following patch will fix the arguments passed to SHADOW_PRINTK in
> xen/arch/x86/mm/shadow/common.c and xen/arch/x86/mm/shadow/multi.c.
> 
> Although I haven't tested, it is the same for xen-unstable.hg

First chunks of patch: If there is no longer a va to print, then don't
manufacture one to print; just remove it entirely from the printk. Apart
from anything else, using sh_map_domain_page() without
sh_unmap_domain_page() will leak mappings. And the va printed will not be
useful anyway.

Final chunk of patch: print regs->eip? That should work on both x86_64 and
i386.

Please fix and re-send.

 Thanks,
 Keir

Re: [PATCH] Fix arguments passed to SHADOW_PRINTK

[Yoshiaki Tamura]

Keir and Gianluca,

Keir Fraser wrote:
> On 11/9/08 11:44, "Yoshiaki Tamura" <tamura.yoshiaki@lab.ntt.co.jp> wrote:
> 
>> When I compiled xen-3.3-testing with DEBUG_TRACE_DUMP in xen/include/xen/lib.h
>> turned on, I got some errors at SHADOW_PRINTK.
>> The following patch will fix the arguments passed to SHADOW_PRINTK in
>> xen/arch/x86/mm/shadow/common.c and xen/arch/x86/mm/shadow/multi.c.
>>
>> Although I haven't tested, it is the same for xen-unstable.hg
> 
> First chunks of patch: If there is no longer a va to print, then don't
> manufacture one to print; just remove it entirely from the printk. Apart
> from anything else, using sh_map_domain_page() without
> sh_unmap_domain_page() will leak mappings. And the va printed will not be
> useful anyway.

Sorry, that was my mistake.

> Final chunk of patch: print regs->eip? That should work on both x86_64 and
> i386.

When I compiled xen-3.3-testing with DEBUG_TRACE_DUMP turned on,
I got the following errors. Would you please tell me how to work around?

multi.c: In function 'sh_page_fault__guest_2':
multi.c:3045: error: 'struct cpu_user_regs' has no member named 'rip'
multi.c: In function 'sh_page_fault__guest_3':
multi.c:3045: error: 'struct cpu_user_regs' has no member named 'rip'
make[6]: *** [guest_3.o] Error 1
make[6]: *** Waiting for unfinished jobs....
make[6]: *** [guest_2.o] Error 1
make[5]: *** [shadow/built_in.o] Error 2
make[4]: *** [mm/built_in.o] Error 2
make[3]: *** [/home/tamura/Developer/kemari-v1-devel.hg/xen/arch/x86/built_in.o] 
Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [/home/tamura/Developer/kemari-v1-devel.hg/xen/xen] Error 2
make[1]: *** [install] Error 2
make: *** [install-xen] Error 2

> 
> Please fix and re-send.
> 
>  Thanks,
>  Keir
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
> 
> 
> 


-- 
TAMURA, Yoshiaki

NTT Cyber Space Labs
OSS Computing Project
Kernel Group
E-mail: tamura.yoshiaki@lab.ntt.co.jp
TEL: +81-46-859-2771
FAX: +81-46-855-1152
Address: 1-1 Hikarinooka, Yokosuka
	 Kanagawa 239-0847 JAPAN

Re: [PATCH] Fix arguments passed to SHADOW_PRINTK

[Gianluca Guida]

Yoshiaki Tamura wrote:
> Keir and Gianluca,
> 
> When I compiled xen-3.3-testing with DEBUG_TRACE_DUMP turned on,
> I got the following errors. Would you please tell me how to work around?
> 
> multi.c: In function 'sh_page_fault__guest_2':
> multi.c:3045: error: 'struct cpu_user_regs' has no member named 'rip'

Using regs->eip should work both on 32 and 64 bit.

Thanks,
Gianluca

Re: [PATCH] Fix arguments passed to SHADOW_PRINTK

[Yoshiaki Tamura]

Gianluca,

Thanks for your comment.
The following is the fixed patch.

Thanks,

Yoshi

Signed-off-by: Yoshi Tamura <tamura.yoshiaki@lab.ntt.co.jp>

diff -r bfd1157dd315 xen/arch/x86/mm/shadow/common.c
--- a/xen/arch/x86/mm/shadow/common.c	Thu Sep 11 08:06:48 2008 +0900
+++ b/xen/arch/x86/mm/shadow/common.c	Thu Sep 11 11:42:40 2008 +0900
@@ -700,8 +700,8 @@ static void _sh_resync(struct vcpu *v, m
               & ~SHF_L1_ANY));
      ASSERT(!sh_page_has_multiple_shadows(mfn_to_page(gmfn)));

-    SHADOW_PRINTK("d=%d, v=%d, gmfn=%05lx, va=%lx\n",
-                  v->domain->domain_id, v->vcpu_id, mfn_x(gmfn), va);
+    SHADOW_PRINTK("d=%d, v=%d, gmfn=%05lx\n",
+                  v->domain->domain_id, v->vcpu_id, mfn_x(gmfn));

      /* Need to pull write access so the page *stays* in sync. */
      if ( oos_remove_write_access(v, gmfn, fixup) )
@@ -952,8 +952,8 @@ int sh_unsync(struct vcpu *v, mfn_t gmfn

      ASSERT(shadow_locked_by_me(v->domain));

-    SHADOW_PRINTK("d=%d, v=%d, gmfn=%05lx va %lx\n",
-                  v->domain->domain_id, v->vcpu_id, mfn_x(gmfn), va);
+    SHADOW_PRINTK("d=%d, v=%d, gmfn=%05lx\n",
+                  v->domain->domain_id, v->vcpu_id, mfn_x(gmfn));

      pg = mfn_to_page(gmfn);

diff -r bfd1157dd315 xen/arch/x86/mm/shadow/multi.c
--- a/xen/arch/x86/mm/shadow/multi.c	Thu Sep 11 08:06:48 2008 +0900
+++ b/xen/arch/x86/mm/shadow/multi.c	Thu Sep 11 11:46:48 2008 +0900
@@ -3041,9 +3041,9 @@ static int sh_page_fault(struct vcpu *v,
      int fast_emul = 0;
  #endif

-    SHADOW_PRINTK("d:v=%u:%u va=%#lx err=%u, rip=%lx\n",
+    SHADOW_PRINTK("d:v=%u:%u va=%#lx err=%u, eip=%lx\n",
                    v->domain->domain_id, v->vcpu_id, va, regs->error_code,
-                  regs->rip);
+                  regs->eip);

      perfc_incr(shadow_fault);


Gianluca Guida wrote:
> 
> 
> Yoshiaki Tamura wrote:
>> Keir and Gianluca,
>>
>> When I compiled xen-3.3-testing with DEBUG_TRACE_DUMP turned on,
>> I got the following errors. Would you please tell me how to work around?
>>
>> multi.c: In function 'sh_page_fault__guest_2':
>> multi.c:3045: error: 'struct cpu_user_regs' has no member named 'rip'
> 
> Using regs->eip should work both on 32 and 64 bit.
> 
> Thanks,
> Gianluca
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
> 
> 
> 


-- 
TAMURA, Yoshiaki

NTT Cyber Space Labs
OSS Computing Project
Kernel Group
E-mail: tamura.yoshiaki@lab.ntt.co.jp
TEL: +81-46-859-2771
FAX: +81-46-855-1152
Address: 1-1 Hikarinooka, Yokosuka
	 Kanagawa 239-0847 JAPAN