Two entries were left in place by d638fe233cb3 ("libxl: use the cpuid
feature names from cpufeatureset.h"), despite matching the generated
names.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Permitting "psn", "ia64, "cntxid", and "perfctr_*" when the hypervisor
doesn't even know of the bits (perhaps wrongly so) is kind of odd as
well. Permitting bits like "est", which the hypervisor knows of but
doesn't expose to guests, is also questionable.

I wouldn't really call this a bug fix (the entries are simply redundant,
but nothing bad would happen with them there), hence no Fixes: tag.

--- a/tools/libs/light/libxl_cpuid.c
+++ b/tools/libs/light/libxl_cpuid.c
@@ -260,7 +260,6 @@ int libxl_cpuid_parse_config(libxl_cpuid
         {"clfsh",        0x00000001, NA, CPUID_REG_EDX, 19,  1},
         {"tm",           0x00000001, NA, CPUID_REG_EDX, 29,  1},
         {"ia64",         0x00000001, NA, CPUID_REG_EDX, 30,  1},
-        {"pbe",          0x00000001, NA, CPUID_REG_EDX, 31,  1},
 
         {"arat",         0x00000006, NA, CPUID_REG_EAX,  2,  1},
 
@@ -279,7 +278,6 @@ int libxl_cpuid_parse_config(libxl_cpuid
         {"invtsc",       0x80000007, NA, CPUID_REG_EDX,  8,  1},
 
         {"ppin",         0x80000008, NA, CPUID_REG_EBX, 23,  1},
-        {"btc-no",       0x80000008, NA, CPUID_REG_EBX, 29,  1},
 
         {"nc",           0x80000008, NA, CPUID_REG_ECX,  0,  8},
         {"apicidsize",   0x80000008, NA, CPUID_REG_ECX, 12,  4},

On Wed, Aug 23, 2023 at 09:21:26AM +0200, Jan Beulich wrote:
> Two entries were left in place by d638fe233cb3 ("libxl: use the cpuid
> feature names from cpufeatureset.h"), despite matching the generated
> names.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Anthony PERARD <anthony.perard@citrix.com>

> ---
> Permitting "psn", "ia64, "cntxid", and "perfctr_*" when the hypervisor
> doesn't even know of the bits (perhaps wrongly so) is kind of odd as
> well. Permitting bits like "est", which the hypervisor knows of but
> doesn't expose to guests, is also questionable.

I think those are just aliases, to a specific bit in a bitmap. Even if
we remove those aliases, it is still possible to instruct libxl to do
something with those bits. So there presence isn't permission, I don't
think.

Looks like "est" is just an alias for "eist", so it doesn't seems useful
to remove it either.

Thanks,

-- 
Anthony PERARD

On 23.08.2023 15:45, Anthony PERARD wrote:
> On Wed, Aug 23, 2023 at 09:21:26AM +0200, Jan Beulich wrote:
>> Two entries were left in place by d638fe233cb3 ("libxl: use the cpuid
>> feature names from cpufeatureset.h"), despite matching the generated
>> names.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Acked-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks.

>> ---
>> Permitting "psn", "ia64, "cntxid", and "perfctr_*" when the hypervisor
>> doesn't even know of the bits (perhaps wrongly so) is kind of odd as
>> well. Permitting bits like "est", which the hypervisor knows of but
>> doesn't expose to guests, is also questionable.
> 
> I think those are just aliases, to a specific bit in a bitmap. Even if
> we remove those aliases, it is still possible to instruct libxl to do
> something with those bits. So there presence isn't permission, I don't
> think.

It's not permission, but recognizing the field name when its use then
(in the best case) doesn't do anything is at least misleading.

> Looks like "est" is just an alias for "eist", so it doesn't seems useful
> to remove it either.

Well, I'm effectively also questioning the exposure of "eist". Using
INIT_FEATURE_NAMES here was likely okay as a quick first approach, but
I think we want to limit what is recognized to what actually is useful
in at least some cases (yet better would of course be to also not
recognize e.g. HVM-only options when we're dealing with a PV guest).

Jan