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

["Luis R. Rodriguez" <mcgrof@xxxxxxxxxxxxxxxx>]

On Mon, Feb 16, 2015 at 11:39 PM, Juergen Gross <jgross@xxxxxxxx> wrote:
> On 02/17/2015 01:20 AM, Luis R. Rodriguez wrote:
>>
>> On Thu, Feb 12, 2015 at 3:07 AM, David Vrabel <david.vrabel@xxxxxxxxxx>
>> wrote:
>>>
>>> On 12/02/15 06:03, Luis R. Rodriguez wrote:
>>>>
>>>> From: "Luis R. Rodriguez" <mcgrof@xxxxxxxx>
>>>>
>>>> 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.

OK I'll roll in this change.

>> 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.

This was caused for a few reasons for this, firstly because HVC_XEN
depends on XEN and also the entire file drivers/xen/Kconfig is wrapped
with "if XEN" so obviously dropping in XEN_FRONTEND there will make it
depend on XEN. This means we need to expose XEN_FRONTEND outside of
the "if XEN", but likewise we also need to do the same for
XEN_BUS_FRONTEND.

So although we can test building front end drivers without CONFIG_XEN
right now I'd prefer to not do this as it doesn't work in the first
place right now and just doing these changes:

  * Make HVC_XEN select XEN
  * Take out both XEN_XENBUS_FRONTEND and XEN_FRONTEND from "if XEN"
roll on the Kconfig file. We upkeep the "select XEN" on XEN_FRONTEND,
but users will still be able to see XEN_FRONTEND even if XEN hasn't
been selected at least.

I'll test building all front end drivers with this but so far am
running into issues with just building XEN_FRONTEND so it seems there
are other things that we need to address. The issues seem to be
related to the item 3) below.

>> 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.

OK I'll keep xen_memory_setup() and its requirements then only if
XEN_PV is enabled but not XEN_PVH. As for the rest of enlighten.c and
friends -- this is just collateral. What files should we just need for
just XEN_PVHVM when XEN_PV and XEN_PVH are both disabled?

  Luis

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel