Re: [PATCH v2 05/13] libx86: introduce a libx86 shared library

From: "Jan Beulich" <JBeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Sergey Dyasli <sergey.dyasli@citrix.com>,
	Xen-devel <xen-devel@lists.xen.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH v2 05/13] libx86: introduce a libx86 shared library
Date: Mon, 16 Jul 2018 04:52:36 -0600	[thread overview]
Message-ID: <5B4C78F402000078001D44E5@prv1-mh.provo.novell.com> (raw)
In-Reply-To: <4be341fd-a04e-a5e3-a383-cf6afeb544e1@citrix.com>

>>> On 16.07.18 at 12:35, <andrew.cooper3@citrix.com> wrote:
> On 16/07/18 11:17, Jan Beulich wrote:
>>>>> On 13.07.18 at 22:03, <andrew.cooper3@citrix.com> wrote:
>>>  tools/libxc/Makefile           |  6 ++++++
>>>  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/private.h    | 38 ++++++++++++++++++++++++++++++++++++
>> I can certainly accept libx86 headers to live in xen/include/xen/libx86/
>> for now if the suggested better places cause trouble, but xen/common/
>> is what its name says - common (i.e. architecture independent) code.
>> This wants to go into lib/x86 or xen/lib/x86 or some such, or in the worst
>> case under xen/arch/x86/, the more that I doubt the same issue as with
>> the header files applies here.
> 
> The C files are trivial to move in comparison to the header files.  All
> that needs changing is the single vpath in libxc's build.
> 
> I'll move the C files into xen/lib/x86/

Thanks, and with that then
Acked-by: Jan Beulich <jbeulich@suse.com>

>>> --- /dev/null
>>> +++ b/xen/common/libx86/private.h
>>> @@ -0,0 +1,38 @@
>>> +#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>
>>> +
>>> +static inline bool test_bit(unsigned int bit, const void *vaddr)
>>> +{
>>> +    const char *addr = vaddr;
>>> +
>>> +    return addr[bit / 8] & (1u << (bit % 8));
>>> +}
>> There's nowhere in the tools that this could be taken from? xc_bitops.h
>> has one - couldn't this be lifted to xen-tools/libs.h?
> 
> There are already 2 different copies in tools, with different APIs, and
> adding this into xen-tools/libs.h broke the build in more complicated
> ways than I have time to fix at this point.

Oh, ouch.

Jan

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