[Qemu-devel] [PATCH 0/1] Fix cacheline detection on FreeBSD/powerpc

[Qemu-devel] [PATCH 0/1] Fix cacheline detection on FreeBSD/powerpc

[Laurent Vivier]

This is the patch originally sent by Justin, modified
to change the parameter size on FreeBSD only.

Justin, could you review and test on FreeBSD?
Peter, could you run "make check" on your MacOS X host?

Thanks,
Laurent

Justin Hibbits (1):
  Fix cacheline detection on FreeBSD/powerpc.

 util/cacheinfo.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

-- 
2.21.0

[Qemu-devel] [PATCH 1/1] Fix cacheline detection on FreeBSD/powerpc.

[Laurent Vivier]

From: Justin Hibbits <chmeeedalf@gmail.com>

machdep.cacheline_size is an integer, not a long.  Since PowerPC is
big-endian this causes sysctlbyname() to fill in the upper bits of the
argument, rather than the correct 'lower bits' of the word.  Specify the
correct type to fix this.

Fixes: b255b2c8a548 ("util: add cacheinfo")
Signed-off-by: Justin Hibbits <chmeeedalf@gmail.com>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 util/cacheinfo.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/util/cacheinfo.c b/util/cacheinfo.c
index eebe1ce9c5d2..ea6f3e99bf4a 100644
--- a/util/cacheinfo.c
+++ b/util/cacheinfo.c
@@ -65,25 +65,28 @@ static void sys_cache_info(int *isize, int *dsize)
     g_free(buf);
 }
 
-#elif defined(__APPLE__) \
-      || defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
+#elif defined(__APPLE__)
 # include <sys/sysctl.h>
-# if defined(__APPLE__)
-#  define SYSCTL_CACHELINE_NAME "hw.cachelinesize"
-# else
-#  define SYSCTL_CACHELINE_NAME "machdep.cacheline_size"
-# endif
-
 static void sys_cache_info(int *isize, int *dsize)
 {
     /* There's only a single sysctl for both I/D cache line sizes.  */
     long size;
     size_t len = sizeof(size);
-    if (!sysctlbyname(SYSCTL_CACHELINE_NAME, &size, &len, NULL, 0)) {
+    if (!sysctlbyname("hw.cachelinesize", &size, &len, NULL, 0)) {
+        *isize = *dsize = size;
+    }
+}
+#elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
+# include <sys/sysctl.h>
+static void sys_cache_info(int *isize, int *dsize)
+{
+    /* There's only a single sysctl for both I/D cache line sizes.  */
+    int size;
+    size_t len = sizeof(size);
+    if (!sysctlbyname("machdep.cacheline_size", &size, &len, NULL, 0)) {
         *isize = *dsize = size;
     }
 }
-
 #else
 /* POSIX */
 
-- 
2.21.0

Re: [Qemu-devel] [PATCH 0/1] Fix cacheline detection on FreeBSD/powerpc

[Justin Hibbits]

On Wed, 21 Aug 2019 10:25:45 +0200
Laurent Vivier <laurent@vivier.eu> wrote:

> This is the patch originally sent by Justin, modified
> to change the parameter size on FreeBSD only.
> 
> Justin, could you review and test on FreeBSD?
> Peter, could you run "make check" on your MacOS X host?
> 
> Thanks,
> Laurent
> 
> Justin Hibbits (1):
>   Fix cacheline detection on FreeBSD/powerpc.
> 
>  util/cacheinfo.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 

Hi,

Sorry for the delay, I'll get to testing it tonight or tomorrow.  The
patch looks good from inspection.

- Justin

Re: [Qemu-devel] [PATCH 0/1] Fix cacheline detection on FreeBSD/powerpc

[Justin Hibbits]

On Wed, 21 Aug 2019 10:25:45 +0200
Laurent Vivier <laurent@vivier.eu> wrote:

> This is the patch originally sent by Justin, modified
> to change the parameter size on FreeBSD only.
> 
> Justin, could you review and test on FreeBSD?
> Peter, could you run "make check" on your MacOS X host?
> 
> Thanks,
> Laurent
> 
> Justin Hibbits (1):
>   Fix cacheline detection on FreeBSD/powerpc.
> 
>  util/cacheinfo.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 

Hi Laurent,

I applied the patch against the FreeBSD emulators/qemu-devel (qemu
3.1.0) port, and it was successful.

All good on this end!

- Justin

Re: [Qemu-devel] [PATCH 1/1] Fix cacheline detection on FreeBSD/powerpc.

[Laurent Vivier]

Le 21/08/2019 à 10:25, Laurent Vivier a écrit :
> From: Justin Hibbits <chmeeedalf@gmail.com>
> 
> machdep.cacheline_size is an integer, not a long.  Since PowerPC is
> big-endian this causes sysctlbyname() to fill in the upper bits of the
> argument, rather than the correct 'lower bits' of the word.  Specify the
> correct type to fix this.
> 
> Fixes: b255b2c8a548 ("util: add cacheinfo")
> Signed-off-by: Justin Hibbits <chmeeedalf@gmail.com>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  util/cacheinfo.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/util/cacheinfo.c b/util/cacheinfo.c
> index eebe1ce9c5d2..ea6f3e99bf4a 100644
> --- a/util/cacheinfo.c
> +++ b/util/cacheinfo.c
> @@ -65,25 +65,28 @@ static void sys_cache_info(int *isize, int *dsize)
>      g_free(buf);
>  }
>  
> -#elif defined(__APPLE__) \
> -      || defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
> +#elif defined(__APPLE__)
>  # include <sys/sysctl.h>
> -# if defined(__APPLE__)
> -#  define SYSCTL_CACHELINE_NAME "hw.cachelinesize"
> -# else
> -#  define SYSCTL_CACHELINE_NAME "machdep.cacheline_size"
> -# endif
> -
>  static void sys_cache_info(int *isize, int *dsize)
>  {
>      /* There's only a single sysctl for both I/D cache line sizes.  */
>      long size;
>      size_t len = sizeof(size);
> -    if (!sysctlbyname(SYSCTL_CACHELINE_NAME, &size, &len, NULL, 0)) {
> +    if (!sysctlbyname("hw.cachelinesize", &size, &len, NULL, 0)) {
> +        *isize = *dsize = size;
> +    }
> +}
> +#elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
> +# include <sys/sysctl.h>
> +static void sys_cache_info(int *isize, int *dsize)
> +{
> +    /* There's only a single sysctl for both I/D cache line sizes.  */
> +    int size;
> +    size_t len = sizeof(size);
> +    if (!sysctlbyname("machdep.cacheline_size", &size, &len, NULL, 0)) {
>          *isize = *dsize = size;
>      }
>  }
> -
>  #else
>  /* POSIX */
>  
> 

Applied to my trivial-patches branch.

Thanks,
Laurent