Re: [XEN PATCH v2 06/15] x86/p2m: guard altp2m code with CONFIG_ALTP2M option

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.05.2024 12:44, Sergiy Kibrik wrote:
> 16.05.24 14:01, Jan Beulich:
>> On 15.05.2024 11:10, Sergiy Kibrik wrote:
>>> --- a/xen/arch/x86/include/asm/hvm/hvm.h
>>> +++ b/xen/arch/x86/include/asm/hvm/hvm.h
>>> @@ -670,7 +670,7 @@ static inline bool hvm_hap_supported(void)
>>>   /* returns true if hardware supports alternate p2m's */
>>>   static inline bool hvm_altp2m_supported(void)
>>>   {
>>> -    return hvm_funcs.caps.altp2m;
>>> +    return IS_ENABLED(CONFIG_ALTP2M) && hvm_funcs.caps.altp2m;
>>
>> Which in turn raises the question whether the altp2m struct field shouldn't
>> become conditional upon CONFIG_ALTP2M too (or rather: instead, as the change
>> here then would need to be done differently). Yet maybe that would entail
>> further changes elsewhere, so may well better be left for later.
> 
>   but hvm_funcs.caps.altp2m is only a capability bit -- is it worth to 
> become conditional?

Well, the comment was more based on the overall principle than the actual
space savings that might result. Plus as said - likely that would not work
anyway without further changes elsewhere. So perhaps okay to leave as you
have it.

>>> --- a/xen/arch/x86/mm/Makefile
>>> +++ b/xen/arch/x86/mm/Makefile
>>> @@ -1,7 +1,7 @@
>>>   obj-y += shadow/
>>>   obj-$(CONFIG_HVM) += hap/
>>>   
>>> -obj-$(CONFIG_HVM) += altp2m.o
>>> +obj-$(CONFIG_ALTP2M) += altp2m.o
>>
>> This change I think wants to move to patch 5.
>>
> 
> If this moves to patch 5 then HVM=y && ALTP2M=n configuration 
> combination will break the build in between patch 5 and 6, so I've 
> decided to put it together with fixes of these build failures in patch 6.

Hmm, yes, I think I see what you mean.

> Maybe I can merge patch 5 & 6 together then ?

Perhaps more consistent that way, yes.

Jan