Re: [PATCH] build: convert CONFIG_COMPAT to Kconfig

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 20/12/15 13:42, Doug Goldstein wrote:
> On 12/18/15 4:09 PM, Andrew Cooper wrote:
>> On 18/12/2015 21:49, Doug Goldstein wrote:
>>> On 12/18/15 3:35 PM, Andrew Cooper wrote:
>>>> On 18/12/2015 20:06, Doug Goldstein wrote:
>>>>> Use the Kconfig generated CONFIG_COMPAT defines in the code base.
>>>>>
>>>>> CC: Keir Fraser <keir@xen.org>
>>>>> CC: Jan Beulich <jbeulich@suse.com>
>>>>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
>>>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, although I have
>>>> a slight quibble.
>>>>
>>>>> ---
>>>>> This was previously Acked-by: Jan Beulich <jbeulich@suse.com> but then
>>>>> there was a request to change it to xen/common/Kconfig from
>>>>> xen/arch/x86/Kconfig. Unfortunately a small typo ('def_bool y' instead of
>>>>> 'bool') caused it to break on ARM. This resolves the issue and should be
>>>>> ready to merge.
>>>>> ---
>>>>>  config/x86_64.mk     | 1 -
>>>>>  xen/arch/x86/Kconfig | 1 +
>>>>>  xen/common/Kconfig   | 7 +++++++
>>>>>  3 files changed, 8 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/config/x86_64.mk b/config/x86_64.mk
>>>>> index f12d549..85fa27c 100644
>>>>> --- a/config/x86_64.mk
>>>>> +++ b/config/x86_64.mk
>>>>> @@ -2,7 +2,6 @@ CONFIG_X86 := y
>>>>>  CONFIG_X86_64 := y
>>>>>  CONFIG_X86_$(XEN_OS) := y
>>>>>  
>>>>> -CONFIG_COMPAT := y
>>>>>  CONFIG_MIGRATE := y
>>>>>  CONFIG_XCUTILS := y
>>>>>  
>>>>> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
>>>>> index 07e366d..7d2ed96 100644
>>>>> --- a/xen/arch/x86/Kconfig
>>>>> +++ b/xen/arch/x86/Kconfig
>>>>> @@ -3,6 +3,7 @@ config X86_64
>>>>>  
>>>>>  config X86
>>>>>  	def_bool y
>>>>> +	select COMPAT
>>>>>  	select HAS_ACPI
>>>>>  	select HAS_CPUFREQ
>>>>>  	select HAS_EHCI
>>>>> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
>>>>> index 7d0e9a9..046e257 100644
>>>>> --- a/xen/common/Kconfig
>>>>> +++ b/xen/common/Kconfig
>>>>> @@ -1,6 +1,13 @@
>>>>>  
>>>>>  menu "Common Features"
>>>>>  
>>>>> +config COMPAT
>>>>> +	bool
>>>>> +	help
>>>>> +	  32-bit interface support on 64-bit Xen which is used for both
>>>>> +	  HVM and PV guests. HVMLoader makes 32-bit hypercalls irrespective
>>>>> +	  of the destination runmode of the guest.
>>>> As this is now common, probably want to specify x86 HVM and PV guests. 
>>>> Arm guests are technically HVM, although the term is rather less common
>>>> on their side.
>>>>
>>>> ~Andrew
>>>>
>>> How about:
>>>
>>> 32-bit interface support on 64-bit Xen which is used by x86 HVM and PV
>>> guests and ARM HVM guests. The reason this is used for HVM guests is
>>> that HVMLoader makes 32-bit hypercalls irrespective of the destination
>>> run mode of the guest.
>>>
>> The complication here is that arm doesn't yet support compat.  There is
>> a hope to (which is, I guess, why Jan asked for it to be common), but it
>> shouldn't give any implication that it is available on ARM yet.
> So maybe just drop the "and ARM HVM guests" part of that statement and
> when that gets added we can expand the language?
>
>> I am not overly familiar with the kconfig grammar.  Is there a sensible
>> way to limit the option to being visible only on x86, and only on 64bit
>> builds?  (even if arm supported compat, it wouldn't make sense to offer
>> it to arm32).
>>
>> Or am I just talking my way back around to suggesting that it be an arch
>> specific option?
>>
>> ~Andrew
>>
> Not necessarily. Currently its not visible to users at all and x86 just
> enables it. Should it be made to be visible we could keep it in the
> common place and add depends like "depends on X86_64" or "depends on
> 64BIT" (both x86_64 and arm64).

Ah ok.  In which case dropping the ARM bit is probably best, and leaving
everything else as it stands.

~Andrew