Re: [PATCH v4 01/26] xen/public: Export cpu featureset information in the public API

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 24/03/16 14:08, Jan Beulich wrote:
>>>> On 23.03.16 at 17:36, <andrew.cooper3@citrix.com> wrote:
>> --- /dev/null
>> +++ b/xen/include/asm-x86/cpufeatureset.h
>> @@ -0,0 +1,32 @@
>> +#ifndef __XEN_X86_CPUFEATURESET_H__
>> +#define __XEN_X86_CPUFEATURESET_H__
>> +
>> +#ifndef __ASSEMBLY__
>> +
>> +#define XEN_CPUFEATURE(name, value) X86_FEATURE_##name = value,
>> +enum {
>> +#include <public/arch-x86/cpufeatureset.h>
>> +#undef XEN_CPUFEATURE
>> +};
>> +
>> +#define XEN_CPUFEATURE(name, value) asm (".equ X86_FEATURE_" #name ", " #value);
>> +#include <public/arch-x86/cpufeatureset.h>
>> +
>> +#else /* !__ASSEMBLY__ */
>> +
>> +#define XEN_CPUFEATURE(name, value) .equ X86_FEATURE_##name, value
>> +#include <public/arch-x86/cpufeatureset.h>
>> +
>> +#endif /* __ASSEMBLY__ */
>> +
>> +#endif /* !__XEN_X86_CPUFEATURESET_H__ */
> While this is now no longer in a public header, I still don't like
> XEN_CPUFEATURE() remaining defined here.
>
> With that
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>
> If you agree, I could fold this in while committing.

Ok.

>
> Independently - is the asm() indeed unconditionally necessary?

Yes.  Otherwise alternative blocks in C fail to compile.  They
__stringify() the feature name, which used to turn into a number (when
the feature was a define), but stay as a string identifier because of
the new enum.

> If so, how much clutter to symbol tables does it introduce? I notice
> that public/errno.h causes some unnecessary bloat in this regard
> too. Should we perhaps add --strip-local-absolute to the assembler
> options (for both .c and .S files)?

I was wondering if there was anything we could do to fix the errno
clutter.  That sounds like a good plan.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel