NetBSD Problem Report #47967

From riz@slash.lan  Wed Jun 26 17:32:15 2013
Return-Path: <riz@slash.lan>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 9ECE77157D
	for <gnats-bugs@gnats.NetBSD.org>; Wed, 26 Jun 2013 17:32:15 +0000 (UTC)
Message-Id: <20130626161054.6B801CEC12@slash.lan>
Date: Wed, 26 Jun 2013 09:10:54 -0700 (PDT)
From: riz@NetBSD.org
Reply-To: riz@NetBSD.org
To: gnats-bugs@NetBSD.org
Subject: DTrace does not work while running under QEMU
X-Send-Pr-Version: 3.95

>Number:         47967
>Category:       port-amd64
>Synopsis:       tsc_freq is 0 when running under QEMU so DTrace crashes at load
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    port-amd64-maintainer
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Jun 26 17:35:00 +0000 2013
>Closed-Date:    Tue Jan 31 23:42:04 +0000 2023
>Last-Modified:  Tue Jan 31 23:42:04 +0000 2023
>Originator:     Jeff Rizzo
>Release:        NetBSD 6.99.22
>Organization:

>Environment:


System: NetBSD slash.lan 6.99.22 NetBSD 6.99.22 (DTRACE) #37: Sat Jun 22 16:37:43 PDT 2013 riz@slash.lan:/build/obj/sys/arch/amd64/compile/DTRACE amd64
Architecture: x86_64
Machine: amd64
>Description:
	QEMU does not implement MSR_TSC, so reads there with rdmsr() always
	return 0, which breaks cpu_counter_serializing(), and in turn
	breaks DTrace under QEMU.

	Things work ok if cpu_counter_serializing() is made to use
	cpu_counter() when running under QEMU.

>How-To-Repeat:
	Install system built with MKDTRACE=yes under QEMU, and
	try to "modload dtrace".  Boom.
>Fix:

This patch makes things work for me, though I'm not convinced it's
entirely correct:

Index: sys/arch/x86/x86/identcpu.c
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/x86/identcpu.c,v
retrieving revision 1.32
diff -u -r1.32 identcpu.c
--- sys/arch/x86/x86/identcpu.c 16 Jun 2012 17:30:19 -0000      1.32
+++ sys/arch/x86/x86/identcpu.c 26 Jun 2013 16:08:01 -0000
@@ -563,6 +563,17 @@
 }

 static void
+cpu_probe_qemu(struct cpu_info *ci)
+{
+       if (memcmp("QEMU Virtual CPU", cpu_brand_string, 16) != 0)
+               return;
+
+       /* if QEMU does not implement MSR_TSC, disable the TSC */
+       ci->ci_feat_val[0] &= ~CPUID_MSR;
+       cpu_feature[0] &= ~CPUID_MSR;
+}
+
+static void
 cpu_probe_vortex86(struct cpu_info *ci)
 {
 #define PCI_MODE1_ADDRESS_REG  0x0cf8
@@ -709,6 +720,7 @@
        cpu_probe_c3(ci);
        cpu_probe_geode(ci);
        cpu_probe_vortex86(ci);
+       cpu_probe_qemu(ci);

        x86_cpu_topology(ci);

>Release-Note:

>Audit-Trail:
From: Jukka Ruohonen <jruohonen@iki.fi>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-amd64/47967: DTrace does not work while running under QEMU
Date: Wed, 26 Jun 2013 20:49:01 +0300

 On Wed, Jun 26, 2013 at 05:35:00PM +0000, riz@NetBSD.org wrote:
 > +       if (memcmp("QEMU Virtual CPU", cpu_brand_string, 16) != 0)
 > +               return;

 Wouldn't it be better to patch Qemu? Vague string matching in the kernel
 does not seem like a good approach. As our test setup shows, trying match
 Qemu is a bog...

 - Jukka.

From: Jeff Rizzo <riz@NetBSD.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-amd64/47967: DTrace does not work while running under QEMU
Date: Wed, 26 Jun 2013 11:00:18 -0700

 On 6/26/13 10:50 AM, Jukka Ruohonen wrote:
 > The following reply was made to PR port-amd64/47967; it has been noted by GNATS.
 >
 >   On Wed, Jun 26, 2013 at 05:35:00PM +0000, riz@NetBSD.org wrote:
 >   > +       if (memcmp("QEMU Virtual CPU", cpu_brand_string, 16) != 0)
 >   > +               return;
 >   
 >   Wouldn't it be better to patch Qemu? Vague string matching in the kernel
 >   does not seem like a good approach. As our test setup shows, trying match
 >   Qemu is a bog...
 >

 Patch QEMU to do what, exactly?  Implement MSR_TSC?  Sure - if you want 
 to dig into QEMU and do that, go ahead, but this is a workaround for a 
 known problem in a known "cpu".  The brand string also includes a 
 version number, should qemu ever fix this issue, so it will be possible 
 to support multiple versions. It doesn't seem to me to be any different 
 from any other CPU we work around in that file.

 My only concern was, does turning off CPUID_MSR have any other side 
 effects that I'm missing, and should we be using a different test 
 altogether?

From: Jukka Ruohonen <jruohonen@iki.fi>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-amd64/47967: DTrace does not work while running under QEMU
Date: Wed, 26 Jun 2013 21:02:44 +0300

 On Wed, Jun 26, 2013 at 05:35:00PM +0000, riz@NetBSD.org wrote:
 > 	QEMU does not implement MSR_TSC, so reads there with rdmsr() always
 > 	Install system built with MKDTRACE=yes under QEMU, and

 Alternatively, something like rdmsr_safe(9) somewhere in arch/x86/tsc.c
 seems more appropriate. But it is unclear whether this is a real bug in
 Qemu, given that the TSC MSR should be available since Pentium XXX, I think
 (without actually checking this from the documentation).

 - Jukka.

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/47967 CVS commit: src/sys/arch/x86/x86
Date: Wed, 26 Jun 2013 16:52:28 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Wed Jun 26 20:52:28 UTC 2013

 Modified Files:
 	src/sys/arch/x86/x86: identcpu.c

 Log Message:
 PR/47967: Jeff Rizzo: Add a probe for QEMU to disable it from claiming it
 has MSR_TSC. Fixes DTRACE crashing because it returned a frequency of 0.


 To generate a diff of this commit:
 cvs rdiff -u -r1.32 -r1.33 src/sys/arch/x86/x86/identcpu.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Jeff Rizzo <riz@NetBSD.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-amd64/47967: DTrace does not work while running under QEMU
Date: Wed, 26 Jun 2013 14:14:22 -0700

 I filed a bug report with the QEMU folks:

 https://bugs.launchpad.net/qemu/+bug/1195012

 Hopefully they'll fix it and this workaround can be limited to older 
 versions of QEMU.

From: David Laight <david@l8s.co.uk>
To: gnats-bugs@NetBSD.org
Cc: port-amd64-maintainer@netbsd.org, gnats-admin@netbsd.org,
	netbsd-bugs@netbsd.org
Subject: Re: port-amd64/47967: DTrace does not work while running under QEMU
Date: Wed, 26 Jun 2013 22:30:00 +0100

 On Wed, Jun 26, 2013 at 05:35:00PM +0000, riz@NetBSD.org wrote:
 > >Synopsis:       tsc_freq is 0 when running under QEMU so DTrace crashes at load
 ...
 >  static void
 > +cpu_probe_qemu(struct cpu_info *ci)
 > +{
 > +       if (memcmp("QEMU Virtual CPU", cpu_brand_string, 16) != 0)
 > +               return;
 > +
 > +       /* if QEMU does not implement MSR_TSC, disable the TSC */
 > +       ci->ci_feat_val[0] &= ~CPUID_MSR;
 > +       cpu_feature[0] &= ~CPUID_MSR;
 > +}

 Surely it would be much better to probe for the symptom, rather
 than a random string.

 	David

 -- 
 David Laight: david@l8s.co.uk

From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, port-amd64-maintainer@netbsd.org, 
	gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, riz@NetBSD.org
Cc: 
Subject: Re: port-amd64/47967: DTrace does not work while running under QEMU
Date: Wed, 26 Jun 2013 18:28:08 -0400

 On Jun 26,  9:20pm, david@l8s.co.uk (David Laight) wrote:
 -- Subject: Re: port-amd64/47967: DTrace does not work while running under QE

 |  Surely it would be much better to probe for the symptom, rather
 |  than a random string.

 Ok, how about:

 Index: identcpu.c
 ===================================================================
 RCS file: /cvsroot/src/sys/arch/x86/x86/identcpu.c,v
 retrieving revision 1.33
 diff -u -p -u -r1.33 identcpu.c
 --- identcpu.c	26 Jun 2013 20:52:28 -0000	1.33
 +++ identcpu.c	26 Jun 2013 22:19:26 -0000
 @@ -58,6 +58,7 @@ static const struct x86_cache_info amd_c
  	AMD_L3CACHE_INFO;

  int cpu_vendor;
 +int cpu_msr_tsc;
  char cpu_brand_string[49];

  /*
 @@ -563,14 +564,17 @@ cpu_probe_geode(struct cpu_info *ci)
  }

  static void
 -cpu_probe_qemu(struct cpu_info *ci)
 +cpu_probe_msr_tsc(struct cpu_info *ci)
  {
 -       if (memcmp("QEMU Virtual CPU", cpu_brand_string, 16) != 0)
 -               return;
 +	cpu_msr_tsc = (cpu_feature[0] & CPUID_MSR) != 0;
 +	if (!cpu_msr_tsc)
 +		return;
 +
 +	if (rdmsr(MSR_TSC) != 0)
 +		return;

 -       /* if QEMU does not implement MSR_TSC, disable the TSC */
 -       ci->ci_feat_val[0] &= ~CPUID_MSR;
 -       cpu_feature[0] &= ~CPUID_MSR;
 +	aprint_error_dev(ci->ci_dev, "Bad MSR_TSC support");
 +	cpu_msr_tsc = 0;
  }

  static void
 @@ -720,7 +724,7 @@ cpu_probe(struct cpu_info *ci)
  	cpu_probe_c3(ci);
  	cpu_probe_geode(ci);
  	cpu_probe_vortex86(ci);
 -	cpu_probe_qemu(ci);
 +	cpu_probe_msr_tsc(ci);

  	x86_cpu_topology(ci);

 Index: tsc.c
 ===================================================================
 RCS file: /cvsroot/src/sys/arch/x86/x86/tsc.c,v
 retrieving revision 1.30
 diff -u -p -u -r1.30 tsc.c
 --- tsc.c	8 Aug 2011 17:00:23 -0000	1.30
 +++ tsc.c	26 Jun 2013 22:19:26 -0000
 @@ -258,10 +258,12 @@ cpu_hascounter(void)
  	return cpu_feature[0] & CPUID_TSC;
  }

 +extern int cpu_msr_tsc;
 +
  uint64_t
  cpu_counter_serializing(void)
  {
 -	if (cpu_feature[0] & CPUID_MSR)
 +	if (cpu_msr_tsc)
  		return rdmsr(MSR_TSC);
  	else
  		return cpu_counter();

From: Jukka Ruohonen <jruohonen@iki.fi>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-amd64/47967: DTrace does not work while running under QEMU
Date: Thu, 27 Jun 2013 02:54:53 +0300

 On Wed, Jun 26, 2013 at 10:30:01PM +0000, Christos Zoulas wrote:
 >  Ok, how about:

 Why the heck it needs to be Qemu?

 Test for the problem, not for the string that may vary according to the
 random guy who writes it to a chip or some code section in an emulator. 
 Besides, we have strong prior evidence that Qemu-guys keep chaing their
 strings.

 Well, whatever, if CPUs are nowadays identified by strings, go for it. 
 Maybe instructions will some day follow.

 PS. Please consider also the performance considerations w.r.t TSC.

 - Jukka.


 >  
 >  Index: identcpu.c
 >  ===================================================================
 >  RCS file: /cvsroot/src/sys/arch/x86/x86/identcpu.c,v
 >  retrieving revision 1.33
 >  diff -u -p -u -r1.33 identcpu.c
 >  --- identcpu.c	26 Jun 2013 20:52:28 -0000	1.33
 >  +++ identcpu.c	26 Jun 2013 22:19:26 -0000
 >  @@ -58,6 +58,7 @@ static const struct x86_cache_info amd_c
 >   	AMD_L3CACHE_INFO;
 >   
 >   int cpu_vendor;
 >  +int cpu_msr_tsc;
 >   char cpu_brand_string[49];
 >   
 >   /*
 >  @@ -563,14 +564,17 @@ cpu_probe_geode(struct cpu_info *ci)
 >   }
 >   
 >   static void
 >  -cpu_probe_qemu(struct cpu_info *ci)
 >  +cpu_probe_msr_tsc(struct cpu_info *ci)
 >   {
 >  -       if (memcmp("QEMU Virtual CPU", cpu_brand_string, 16) != 0)
 >  -               return;
 >  +	cpu_msr_tsc = (cpu_feature[0] & CPUID_MSR) != 0;
 >  +	if (!cpu_msr_tsc)
 >  +		return;
 >  +
 >  +	if (rdmsr(MSR_TSC) != 0)
 >  +		return;
 >   
 >  -       /* if QEMU does not implement MSR_TSC, disable the TSC */
 >  -       ci->ci_feat_val[0] &= ~CPUID_MSR;
 >  -       cpu_feature[0] &= ~CPUID_MSR;
 >  +	aprint_error_dev(ci->ci_dev, "Bad MSR_TSC support");
 >  +	cpu_msr_tsc = 0;
 >   }
 >   
 >   static void
 >  @@ -720,7 +724,7 @@ cpu_probe(struct cpu_info *ci)
 >   	cpu_probe_c3(ci);
 >   	cpu_probe_geode(ci);
 >   	cpu_probe_vortex86(ci);
 >  -	cpu_probe_qemu(ci);
 >  +	cpu_probe_msr_tsc(ci);
 >   
 >   	x86_cpu_topology(ci);
 >   
 >  Index: tsc.c
 >  ===================================================================
 >  RCS file: /cvsroot/src/sys/arch/x86/x86/tsc.c,v
 >  retrieving revision 1.30
 >  diff -u -p -u -r1.30 tsc.c
 >  --- tsc.c	8 Aug 2011 17:00:23 -0000	1.30
 >  +++ tsc.c	26 Jun 2013 22:19:26 -0000
 >  @@ -258,10 +258,12 @@ cpu_hascounter(void)
 >   	return cpu_feature[0] & CPUID_TSC;
 >   }
 >   
 >  +extern int cpu_msr_tsc;
 >  +
 >   uint64_t
 >   cpu_counter_serializing(void)
 >   {
 >  -	if (cpu_feature[0] & CPUID_MSR)
 >  +	if (cpu_msr_tsc)
 >   		return rdmsr(MSR_TSC);
 >   	else
 >   		return cpu_counter();
 >  

From: Jukka Ruohonen <jruohonen@iki.fi>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-amd64/47967: DTrace does not work while running under QEMU
Date: Thu, 27 Jun 2013 03:01:28 +0300

 Sorry, 

 wrote before thinking, which is usual, sorry. But nevertheless.

 On Wed, Jun 26, 2013 at 10:30:01PM +0000, Christos Zoulas wrote:
 > The following reply was made to PR port-amd64/47967; it has been noted by GNATS.

 >  +	if (rdmsr(MSR_TSC) != 0)
 >  +		return;
 >   
 >  -       /* if QEMU does not implement MSR_TSC, disable the TSC */
 >  -       ci->ci_feat_val[0] &= ~CPUID_MSR;
 >  -       cpu_feature[0] &= ~CPUID_MSR;
 >  +	aprint_error_dev(ci->ci_dev, "Bad MSR_TSC support");
 >  +	cpu_msr_tsc = 0;

 This hardly seems right. Now TSC determines whether something is capable of
 MSRs? Those are just registers, even for emulators.  TM: fixing bugs in the
 kernel that are not really bugs, i.e.  never has there been in NetBSD actual
 x86 hardware for which this wouldn't work.

 - Jukka.

From: Jukka Ruohonen <jruohonen@iki.fi>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-amd64/47967: DTrace does not work while running under QEMU
Date: Thu, 27 Jun 2013 03:20:59 +0300

 On Wed, Jun 26, 2013 at 10:30:01PM +0000, Christos Zoulas wrote:
 > The following reply was made to PR port-amd64/47967; it has been noted by GNATS.
 > 
 > From: christos@zoulas.com (Christos Zoulas)
 > To: gnats-bugs@NetBSD.org, port-amd64-maintainer@netbsd.org, 
 > 	gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, riz@NetBSD.org
 > Cc: 
 > Subject: Re: port-amd64/47967: DTrace does not work while running under QEMU
 > Date: Wed, 26 Jun 2013 18:28:08 -0400
 > 
 >  On Jun 26,  9:20pm, david@l8s.co.uk (David Laight) wrote:
 >  -- Subject: Re: port-amd64/47967: DTrace does not work while running under QE

 So, just that I wouldn't just rant:

 1) Rename the ridiculous constant; call it, say, HAVE_MSR.

 2) Probe it during initialization of TSC. No need to add quirks for whatever
    emulator is hot at the moment.

 3) Be done with it.

 - Jukka.

State-Changed-From-To: open->feedback
State-Changed-By: gson@NetBSD.org
State-Changed-When: Tue, 01 Jan 2019 11:28:39 +0000
State-Changed-Why:
Is this still an issue?  "modload dtrace" works for me using -current
under qemu 3.1.0, modulo a warning message "XXX dtrace_dof_property not
implemented (name=dof-data-0)".


State-Changed-From-To: feedback->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Tue, 31 Jan 2023 23:42:04 +0000
State-Changed-Why:
Assume no longer an issue; qemu's changed a lot since 2013


>Unformatted:

Home	PR Database Search