Re: [PATCH v2 3/7] x86: shrink struct arch_{vcpu,domain} when !HVM

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.08.2020 19:14, Andrew Cooper wrote:
> On 07/08/2020 12:33, Jan Beulich wrote:
>> While this won't affect overall memory overhead (struct vcpu as well as
>> struct domain get allocated as single pages) nor code size (the offsets
>> into the base structures are too large to be representable as signed 8-
>> bit displacements), it'll allow the tail of struct pv_{domain,vcpu} to
>> share a cache line with subsequent struct arch_{domain,vcpu} fields.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> RFC: There is a risk associated with this: If we still have code
>>      somewhere accessing the HVM parts of the structures without a prior
>>      type check of the guest, this going to end up worse than the so far
>>      not uncommon case of the access simply going to space unused by PV.
>>      We may therefore want to consider whether to further restrict when
>>      this conversion to union gets done.
>>      And of course there's also the risk of future compilers complaining
>>      about this abuse of unions. But this is limited to code that's dead
>>      in !HVM configs, so only an apparent problem.
>>
>> --- a/xen/include/asm-x86/hvm/domain.h
>> +++ b/xen/include/asm-x86/hvm/domain.h
>> @@ -99,7 +99,13 @@ struct hvm_pi_ops {
>>  
>>  #define MAX_NR_IOREQ_SERVERS 8
>>  
>> -struct hvm_domain {
>> +typedef
>> +#ifdef CONFIG_HVM
>> +struct
>> +#else
>> +union
>> +#endif
>> +hvm_domain {
>>      /* Guest page range used for non-default ioreq servers */
>>      struct {
>>          unsigned long base;
>> @@ -203,7 +209,7 @@ struct hvm_domain {
>>  #ifdef CONFIG_MEM_SHARING
>>      struct mem_sharing_domain mem_sharing;
>>  #endif
>> -};
>> +} hvm_domain_t;
> 
> Honestly, I'd say no to this patch even it resulted in a 100x speedup,
> because I am totally lost for words about this construct, and the effect
> it comprehensibility of our code.

Okay - away it goes.

Jan