Re: [RFC v1 0/8] xen: kconfig changes

[Juergen Gross <jgross@suse.com>]

On 02/17/2015 01:20 AM, Luis R. Rodriguez wrote:
> On Thu, Feb 12, 2015 at 3:07 AM, David Vrabel <david.vrabel@citrix.com> wrote:
>> On 12/02/15 06:03, Luis R. Rodriguez wrote:
>>> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>>>
>>> Here's the first shot at the Kconfig changes for Xen as discussed
>>> on the mailing list a little while ago [0]. Let me know if you spot
>>> any issues or if you'd like things split differently. I tried to
>>> make things as atomic as possible, but not being too rediculous
>>> on the atomicity of the changes, for instance the HVC changes
>>> were reasonable to just fold into the other change it touched.
>>>
>>> Haven't gone to war with testing the Kconfig changes yet given this
>>> is just the first RFC. If things look good please look for major
>>> issues and let me know.#
>>
>> Can you spin a v2 and make a git branch available, please?  I would like
>> people to be able to easily try out the changes rather than looking at
>> the diffs.
>>
>> If I haven't comment on a specific patch it's because I thought it
>> looked ok.
>
> Sure thing, before that I should address now what I have found as
> issues with the Kconfig changes and what we need. What I see so far:
>
> 1) due to a recursive dependency it seems we should consider having
> XEN_DOM0 select SWIOTLB_XEN instead of depend on it? That fixes it:
>
> diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
> index d930574..c25e12b 100644
> --- a/arch/x86/xen/Kconfig
> +++ b/arch/x86/xen/Kconfig
> @@ -14,7 +14,8 @@ config XEN
>   config XEN_DOM0
>          def_bool y
>          select XEN_BACKEND
> -       depends on XEN && PCI_XEN && SWIOTLB_XEN
> +       select SWIOTLB_XEN
> +       depends on XEN && PCI_XEN
>          depends on X86_LOCAL_APIC && X86_IO_APIC && ACPI && PCI
>          depends on XEN_PV || XEN_PVH

I'm fine with this.

> 2) due to a recursive-dependency it doesn't seem we should have
> XEN_FRONTEND select on CONFIG_XEN -- with that in place we end up
> with:
>
> arch/x86/xen/Kconfig:5:error: recursive dependency detected!
> arch/x86/xen/Kconfig:5: symbol XEN is selected by XEN_FRONTEND
> drivers/xen/Kconfig:82: symbol XEN_FRONTEND depends on XEN
>
> If we remove the select XEN from XEN_FRONTEND that fixes it. Not sure
> what is ideal here though.
>
> XEN_FRONTEND does not depend on XEN but the select seems to imply it.
>
> 3) The simple memory setup build issue:
>
> As it is per our agreed upon changes we can in theory enable a
> XEN_PVHVM system without XEN_PV or XEN_PVH. If this is indeed
> desirable this poses an issue at build time at
> arch/x86/xen/enlighten.c on xen_start_kernel() here:
>
>          if (xen_feature(XENFEAT_auto_translated_physmap))
>                  x86_init.resources.memory_setup = xen_auto_xlated_memory_setup;
>          else
>                  x86_init.resources.memory_setup = xen_memory_setup;

If we have neither XEN_PV nor XEN_PVH set, why do we have to build
enlighten.c? It will never be used. Same should apply to several other
files in arch/x86/xen.

Juergen