Re: [PATCH v14 15/22] cpu: tcg_ops: move to tcg-cpu-ops.h, keep a pointer in CPUClass

[Claudio Fontana]

On 1/29/21 1:31 AM, Richard Henderson wrote:
> On 1/27/21 11:28 PM, Claudio Fontana wrote:
>> +    /*
>> +     * NB: this should be covered by CONFIG_TCG, but it is unsafe to do it 
>> here,
>> +     * as this header is included by both ss_specific and ss_common code,
>> +     * leading to potential differences in the data structure between 
>> modules.
>> +     * We could always keep it last, but it seems safer to just leave this
>> +     * pointer NULL for non-TCG.
>> +     */
>> +    struct TCGCPUOps *tcg_ops;
> 
> Sorry, I'm going to unqueue the patch set.
> 
> I first thought this was fixing up something done already, fixing an existing 
> bug.
> 
> But it's something done in patch 1, and therefore the patch set needs to be
> re-worked to use this pointer to begin, for the exact reasons detailed above.
> Otherwise it would appear this breaks bisection.
> 
> 
> r~
> 

Hi Richard,

I made sure that the previous patches actually work in practice,

ie the issue mentioned there does not actually happen in practice,
due to the purposeful placement of the field in the structure that might be 
seen by one module
and not the other at the end of the structure, so no problem happens, and 
bisection
is actually safe.

The warning and the change is due to the fact that changing things in the way 
this patch
suggests makes it safer from _future changes_,

in which someone might add a new field at the end of the structure, after the 
conditional fields,
which would actually break things.

Do you think I should redo the series anyway? I would have started this way in 
the first place,
but I tried not to redo Eduardo's work.

Let me know what you think,

Thanks,

Claudio