Re: [Xen-devel] [PATCH 04/13] libx86: introduce a libx86 shared library

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.07.18 at 22:55, <andrew.cooper3@xxxxxxxxxx> wrote:
> From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> Move x86_cpuid_lookup_deep_deps() into the shared library, removing the
> individual copies from the hypervisor and libxc respectively.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>
> CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> ---
>  tools/libxc/Makefile               |  5 +++++
>  tools/libxc/include/xenctrl.h      |  1 -
>  tools/libxc/xc_cpuid_x86.c         | 29 +------------------------
>  xen/arch/x86/cpu/common.c          |  2 +-
>  xen/arch/x86/cpuid.c               | 32 +--------------------------
>  xen/common/Makefile                |  1 +
>  xen/common/libx86/Makefile         |  1 +
>  xen/common/libx86/cpuid.c          | 44 
> ++++++++++++++++++++++++++++++++++++++
>  xen/common/libx86/libx86-private.h | 42 ++++++++++++++++++++++++++++++++++++

I'm pretty opposed to redundancies in names like this - why can't
this simply be private.h - its placement in the tree makes clear
whose private header this is.

> --- /dev/null
> +++ b/xen/common/libx86/libx86-private.h
> @@ -0,0 +1,42 @@
> +#ifndef XEN_LIBX86_PRIVATE_H
> +#define XEN_LIBX86_PRIVATE_H
> +
> +#ifdef __XEN__
> +
> +#include <xen/bitops.h>
> +#include <xen/kernel.h>
> +#include <xen/lib.h>
> +#include <xen/types.h>
> +
> +#else
> +
> +#include <inttypes.h>
> +#include <stdbool.h>
> +#include <stddef.h>
> +
> +#include <xen-tools/libs.h>
> +
> +#define MAX(x, y) ((x) > (y) ? (x) : (y))
> +
> +#define ARRAY_SIZE(x) (sizeof x / sizeof *x)
> +
> +static inline bool test_bit(unsigned int bit, const void *vaddr)
> +{
> +    const char *addr = vaddr;
> +
> +    return addr[bit / 8] & (1u << (bit % 8));
> +}
> +
> +#endif /* __XEN__ */
> +
> +#endif /* XEN_LIBX86_PRIVATE_H */

Did you consider making this x86-independent but still shared?
None of the above is x86-specific. libelf could then possibly use
this too, for exampled.

Also I think we pretty consistently use sizeof(x), not sizeof x, so
I'd like to ask that ARRAY_SIZE() be adjusted accordingly.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel