svn commit: r267651 - in head: share/man/man4 sys/dev/cpuctl sys/sys usr.sbin/cpucontrol

[Konstantin Belousov]

On Thu, Jun 19, 2014 at 09:54:41PM +0000, Attilio Rao wrote:
> Author: attilio
> Date: Thu Jun 19 21:54:41 2014
> New Revision: 267651
> URL: http://svnweb.freebsd.org/changeset/base/267651
> 
> Log:
>   Following comments in r242565 add the possibility to specify ecx when
>   performing cpuid calls.
>   Add also a new way to specify the level type to cpucontrol(8) as
>   reported in the manpage.
>   
>   Sponsored by:	EMC / Isilon storage division
>   Reviewed by:	bdrewery, gcooper
>   Testerd by:	bdrewery
> Modified: head/sys/sys/cpuctl.h
> ==============================================================================
> --- head/sys/sys/cpuctl.h	Thu Jun 19 21:05:07 2014	(r267650)
> +++ head/sys/sys/cpuctl.h	Thu Jun 19 21:54:41 2014	(r267651)
> @@ -35,7 +35,8 @@ typedef struct {
>  } cpuctl_msr_args_t;
>  
>  typedef struct {
> -	int		level;	/* CPUID level */
> +	int		level;		/* CPUID level */
> +	int		level_type;	/* CPUID level type */
>  	uint32_t	data[4];
>  } cpuctl_cpuid_args_t;
>  
> @@ -50,5 +51,6 @@ typedef struct {
>  #define	CPUCTL_UPDATE	_IOWR('c', 4, cpuctl_update_args_t)
>  #define	CPUCTL_MSRSBIT	_IOWR('c', 5, cpuctl_msr_args_t)
>  #define	CPUCTL_MSRCBIT	_IOWR('c', 6, cpuctl_msr_args_t)
> +#define	CPUCTL_CPUID_COUNT _IOWR('c', 7, cpuctl_cpuid_args_t)
>  
>  #endif /* _CPUCTL_H_ */

The cpuctl(4) is used by third-party code, and this change breaks its
ABI. The numeric value for CPUCTL_CPUID is changed, which means that
old binaries call non-existing ioctl now. This is at least a visible
breakage, since the argument for the ioctl changed the layout as well.

The following patch restored the CPUCTL_CPUID for me.  I considered
naming its argument differently, instead of renaming the argument
of CPUCTL_CPUID_COUNT (which you tried to do ?), but decided not,
to preserve the API as well.

Do you agree with the change ?

diff --git a/sys/dev/cpuctl/cpuctl.c b/sys/dev/cpuctl/cpuctl.c
index 63187bd..9832933 100644
--- a/sys/dev/cpuctl/cpuctl.c
+++ b/sys/dev/cpuctl/cpuctl.c
@@ -69,7 +69,7 @@ static int cpuctl_do_msr(int cpu, cpuctl_msr_args_t *data, u_long cmd,
     struct thread *td);
 static int cpuctl_do_cpuid(int cpu, cpuctl_cpuid_args_t *data,
     struct thread *td);
-static int cpuctl_do_cpuid_count(int cpu, cpuctl_cpuid_args_t *data,
+static int cpuctl_do_cpuid_count(int cpu, cpuctl_cpuid_count_args_t *data,
     struct thread *td);
 static int cpuctl_do_update(int cpu, cpuctl_update_args_t *data,
     struct thread *td);
@@ -180,8 +180,8 @@ cpuctl_ioctl(struct cdev *dev, u_long cmd, caddr_t data,
 		ret = cpuctl_do_update(cpu, (cpuctl_update_args_t *)data, td);
 		break;
 	case CPUCTL_CPUID_COUNT:
-		ret = cpuctl_do_cpuid_count(cpu, (cpuctl_cpuid_args_t *)data,
-		    td);
+		ret = cpuctl_do_cpuid_count(cpu,
+		    (cpuctl_cpuid_count_args_t *)data, td);
 		break;
 	default:
 		ret = EINVAL;
@@ -195,7 +195,8 @@ fail:
  * Actually perform cpuid operation.
  */
 static int
-cpuctl_do_cpuid_count(int cpu, cpuctl_cpuid_args_t *data, struct thread *td)
+cpuctl_do_cpuid_count(int cpu, cpuctl_cpuid_count_args_t *data,
+    struct thread *td)
 {
 	int is_bound = 0;
 	int oldcpu;
@@ -218,10 +219,15 @@ cpuctl_do_cpuid_count(int cpu, cpuctl_cpuid_args_t *data, struct thread *td)
 static int
 cpuctl_do_cpuid(int cpu, cpuctl_cpuid_args_t *data, struct thread *td)
 {
+	cpuctl_cpuid_count_args_t cdata;
+	int error;
 
+	cdata.level = data->level;
 	/* Override the level type. */
-	data->level_type = 0;
-	return (cpuctl_do_cpuid_count(cpu, data, td));
+	cdata.level_type = 0;
+	error = cpuctl_do_cpuid_count(cpu, &cdata, td);
+	bcopy(cdata.data, data->data, sizeof(data->data)); /* Ignore error */
+	return (error);
 }
 
 /*
diff --git a/sys/sys/cpuctl.h b/sys/sys/cpuctl.h
index 4220dee..30af524 100644
--- a/sys/sys/cpuctl.h
+++ b/sys/sys/cpuctl.h
@@ -36,11 +36,16 @@ typedef struct {
 
 typedef struct {
 	int		level;		/* CPUID level */
-	int		level_type;	/* CPUID level type */
 	uint32_t	data[4];
 } cpuctl_cpuid_args_t;
 
 typedef struct {
+	int		level;		/* CPUID level */
+	int		level_type;	/* CPUID level type */
+	uint32_t	data[4];
+} cpuctl_cpuid_count_args_t;
+
+typedef struct {
 	void	*data;
 	size_t	size;
 } cpuctl_update_args_t;
@@ -51,6 +56,6 @@ typedef struct {
 #define	CPUCTL_UPDATE	_IOWR('c', 4, cpuctl_update_args_t)
 #define	CPUCTL_MSRSBIT	_IOWR('c', 5, cpuctl_msr_args_t)
 #define	CPUCTL_MSRCBIT	_IOWR('c', 6, cpuctl_msr_args_t)
-#define	CPUCTL_CPUID_COUNT _IOWR('c', 7, cpuctl_cpuid_args_t)
+#define	CPUCTL_CPUID_COUNT _IOWR('c', 7, cpuctl_cpuid_count_args_t)
 
 #endif /* _CPUCTL_H_ */
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-head/attachments/20140620/c1d66913/attachment-0001.sig>