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

From: Jan Beulich <jbeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
	Xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH 9/9] RFC: Everything else
Date: Thu, 30 Mar 2023 14:55:13 +0200	[thread overview]
Message-ID: <77067988-9389-87b5-22e7-398a37bef1ef@suse.com> (raw)
In-Reply-To: <cbacbc0b-60a0-35ea-c813-e4d620f6406c@citrix.com>

On 30.03.2023 14:31, Andrew Cooper wrote:
> On 30/03/2023 1:06 pm, Jan Beulich wrote:
>> On 30.03.2023 14:01, Andrew Cooper wrote:
>>> On 30/03/2023 11:16 am, Jan Beulich wrote:
>>>>> --- 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.
> 
> So its the other way around and you're saying you don't want the field
> name to change, and you don't want to see
> 
> -#define vcpu_has_fpu()         (ctxt->cpuid->basic.fpu)
> +#define vcpu_has_fpu()         (ctxt->cpu_policy->basic.fpu)
> 
> in the resulting diff ?

Neither. I want the change as seen in the diff further up, but indeed
I'd prefer if the two quoted diff lines in your most recent reply
weren't there. This would again be by way of using a union, this time
in struct x86_emulate_ctxt:

    /* CPU Policy for the domain. */
    union {
        const struct cpu_policy *cpuid;
        const struct cpu_policy *cpu_policy;
    };

Having said that: I realize that this is against what C mandates for
the use of unions, but since we (ab)use unions in similar ways in many
other places, I don't think we need to be concerned. Furthermore this
specific use could, aiui, be "legalized" by making the declaration

    /* CPU Policy for the domain. */
    union {
        struct {
            const struct cpu_policy *cpuid;
        };
        struct {
            const struct cpu_policy *cpu_policy;
        };
    };

Jan