From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] build: convert CONFIG_COMPAT to Kconfig Date: Sun, 20 Dec 2015 15:47:33 +0000 Message-ID: <5676CD95.6050201@citrix.com> References: <1450469218-24860-1-git-send-email-cardoe@cardoe.com> <56747C2C.5020408@citrix.com> <56747F86.3030602@cardoe.com> <5674840E.4020101@citrix.com> <5676B037.1070405@cardoe.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5676B037.1070405@cardoe.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Doug Goldstein , xen-devel@lists.xen.org Cc: Keir Fraser , Jan Beulich List-Id: xen-devel@lists.xenproject.org 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 >>>>> CC: Jan Beulich >>>>> CC: Andrew Cooper >>>>> Signed-off-by: Doug Goldstein >>>> Reviewed-by: Andrew Cooper , although I have >>>> a slight quibble. >>>> >>>>> --- >>>>> This was previously Acked-by: Jan Beulich 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