Re: [PATCH 9/9] RFC: Everything else

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.03.2023 14:01, Andrew Cooper wrote:
> On 30/03/2023 11:16 am, Jan Beulich wrote:
>> On 29.03.2023 22:51, Andrew Cooper wrote:
>>> Looking at this diff, I'm wondering whether keeping
>>>
>>>     union {
>>>         struct cpu_policy *cpuid;
>>>         struct cpu_policy *cpu_policy;
>>>     };
>>>
>>> permentantly might be a good idea, because d->arch.cpuid->$X reads rather
>>> better than d->arch.cpu_policy->$X
>>>
>>> Thoughts?
>> I'm not overly fussed, but perhaps yes.
> 
> If we were to do this, we ought to keep d->arch.msr too for the same reason.

I'm not sure this is necessarily a consequence.

> (Honestly - I'm still undecided on whether this is a good idea or not...)
> 
>>  Nevertheless e.g. ...
>>
>>> --- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
>>> +++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
>>> @@ -893,7 +893,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data_p, 
>>> size_t size)
>>>      struct x86_emulate_ctxt ctxt = {
>>>          .data = &state,
>>>          .regs = &input.regs,
>>> -        .cpuid = &cp,
>>> +        .cpu_policy = &cp,
>> ... this and ...
>>
>>> --- a/tools/tests/x86_emulator/test_x86_emulator.c
>>> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
>>> @@ -909,7 +909,7 @@ int main(int argc, char **argv)
>>>  
>>>      ctxt.regs = &regs;
>>>      ctxt.force_writeback = 0;
>>> -    ctxt.cpuid     = &cp;
>>> +    ctxt.cpu_policy = &cp;
>> ... this imo want keeping as you have it here.
> 
> Ok, so that's a firm "switch to using cpu_policy for emul_ctxt" then.
> 
> Which is fine - in fact it's one I'd already started splitting out of
> this patch.

Hmm, wait - CPUID "basic" and "feat" and alike uses I still would prefer
to see using "cpuid". It's really only such initializers which (imo
even logically) want to use the name with the wider coverage.

Jan