Patchwork xen: Restrict build to x86 targets

login
register
mail settings
Submitter Jan Kiszka
Date Nov. 28, 2010, 3:59 p.m.
Message ID <4CF27C6D.9000401@web.de>
Download mbox | patch
Permalink /patch/73340/
State New
Headers show

Comments

Jan Kiszka - Nov. 28, 2010, 3:59 p.m.
From: Jan Kiszka <jan.kiszka@siemens.com>

Xen target bits in qemu are intended for x86. Let the build system
reflect this and avoid useless building/linking for other targets.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 Makefile.objs   |    4 ++--
 Makefile.target |    4 ++--
 configure       |    4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)
Alexander Graf - Nov. 29, 2010, 12:24 p.m.
On 28.11.2010, at 16:59, Jan Kiszka wrote:

> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Xen target bits in qemu are intended for x86. Let the build system
> reflect this and avoid useless building/linking for other targets.

Not sure I understand the split. Xen is x86 only, yes. But why split it into host and target? Target usually defines the guest. The piece you marked as _HOST are target specific.


Alex
Jan Kiszka - Nov. 29, 2010, 12:30 p.m.
Am 29.11.2010 13:24, Alexander Graf wrote:
> 
> On 28.11.2010, at 16:59, Jan Kiszka wrote:
> 
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Xen target bits in qemu are intended for x86. Let the build system
>> reflect this and avoid useless building/linking for other targets.
> 
> Not sure I understand the split. Xen is x86 only, yes. But why split it into host and target? Target usually defines the guest. The piece you marked as _HOST are target specific.

At least so far, the HOST part is build once for all targets into the
host backend library. As this step injected CONFIG_XEN into all target
builds, even non-x86 targets built xen_machine_pv and xen_domainbuild.
That's addressed by the patch.

Jan
Alexander Graf - Nov. 29, 2010, 12:40 p.m.
On 29.11.2010, at 13:30, Jan Kiszka wrote:

> Am 29.11.2010 13:24, Alexander Graf wrote:
>> 
>> On 28.11.2010, at 16:59, Jan Kiszka wrote:
>> 
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>> 
>>> Xen target bits in qemu are intended for x86. Let the build system
>>> reflect this and avoid useless building/linking for other targets.
>> 
>> Not sure I understand the split. Xen is x86 only, yes. But why split it into host and target? Target usually defines the guest. The piece you marked as _HOST are target specific.
> 
> At least so far, the HOST part is build once for all targets into the
> host backend library. As this step injected CONFIG_XEN into all target
> builds, even non-x86 targets built xen_machine_pv and xen_domainbuild.
> That's addressed by the patch.

I still don't understand the need for that split. The device drivers should be built only once, as do the xen_machine_pv parts. Both are useless on non-x86. CONFIG_XEN should simply always be a target specific option.


Alex
Jan Kiszka - Nov. 29, 2010, 12:44 p.m.
Am 29.11.2010 13:40, Alexander Graf wrote:
> 
> On 29.11.2010, at 13:30, Jan Kiszka wrote:
> 
>> Am 29.11.2010 13:24, Alexander Graf wrote:
>>>
>>> On 28.11.2010, at 16:59, Jan Kiszka wrote:
>>>
>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>
>>>> Xen target bits in qemu are intended for x86. Let the build system
>>>> reflect this and avoid useless building/linking for other targets.
>>>
>>> Not sure I understand the split. Xen is x86 only, yes. But why split it into host and target? Target usually defines the guest. The piece you marked as _HOST are target specific.
>>
>> At least so far, the HOST part is build once for all targets into the
>> host backend library. As this step injected CONFIG_XEN into all target
>> builds, even non-x86 targets built xen_machine_pv and xen_domainbuild.
>> That's addressed by the patch.
> 
> I still don't understand the need for that split.

Enable Xen and build some non-x86 targets, then you see the need.

> The device drivers should be built only once, as do the xen_machine_pv parts. Both are useless on non-x86. CONFIG_XEN should simply always be a target specific option.

Maybe the split-up between the "generic" host-side interfaces and
xen_machine_pv/xen_domainbuild is the problem. You know the dependencies
better than me, maybe you find a better fix.

Jan
Alexander Graf - Nov. 29, 2010, 2:15 p.m.
On 29.11.2010, at 13:44, Jan Kiszka wrote:

> Am 29.11.2010 13:40, Alexander Graf wrote:
>> 
>> On 29.11.2010, at 13:30, Jan Kiszka wrote:
>> 
>>> Am 29.11.2010 13:24, Alexander Graf wrote:
>>>> 
>>>> On 28.11.2010, at 16:59, Jan Kiszka wrote:
>>>> 
>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>> 
>>>>> Xen target bits in qemu are intended for x86. Let the build system
>>>>> reflect this and avoid useless building/linking for other targets.
>>>> 
>>>> Not sure I understand the split. Xen is x86 only, yes. But why split it into host and target? Target usually defines the guest. The piece you marked as _HOST are target specific.
>>> 
>>> At least so far, the HOST part is build once for all targets into the
>>> host backend library. As this step injected CONFIG_XEN into all target
>>> builds, even non-x86 targets built xen_machine_pv and xen_domainbuild.
>>> That's addressed by the patch.
>> 
>> I still don't understand the need for that split.
> 
> Enable Xen and build some non-x86 targets, then you see the need.
> 
>> The device drivers should be built only once, as do the xen_machine_pv parts. Both are useless on non-x86. CONFIG_XEN should simply always be a target specific option.
> 
> Maybe the split-up between the "generic" host-side interfaces and
> xen_machine_pv/xen_domainbuild is the problem. You know the dependencies
> better than me, maybe you find a better fix.

Should be enough to just replace obj-$(CONFIG_XEN) by obj-i386-$(CONFIG_XEN). Unless it's very urgent, please wait with this patch until qemu-dm and xenner are in. It's pretty suboptimal to have 3 patches flying around that hit the exact same code spot :).


Alex
Jan Kiszka - Nov. 29, 2010, 2:27 p.m.
Am 29.11.2010 15:15, Alexander Graf wrote:
> 
> On 29.11.2010, at 13:44, Jan Kiszka wrote:
> 
>> Am 29.11.2010 13:40, Alexander Graf wrote:
>>>
>>> On 29.11.2010, at 13:30, Jan Kiszka wrote:
>>>
>>>> Am 29.11.2010 13:24, Alexander Graf wrote:
>>>>>
>>>>> On 28.11.2010, at 16:59, Jan Kiszka wrote:
>>>>>
>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>
>>>>>> Xen target bits in qemu are intended for x86. Let the build system
>>>>>> reflect this and avoid useless building/linking for other targets.
>>>>>
>>>>> Not sure I understand the split. Xen is x86 only, yes. But why split it into host and target? Target usually defines the guest. The piece you marked as _HOST are target specific.
>>>>
>>>> At least so far, the HOST part is build once for all targets into the
>>>> host backend library. As this step injected CONFIG_XEN into all target
>>>> builds, even non-x86 targets built xen_machine_pv and xen_domainbuild.
>>>> That's addressed by the patch.
>>>
>>> I still don't understand the need for that split.
>>
>> Enable Xen and build some non-x86 targets, then you see the need.
>>
>>> The device drivers should be built only once, as do the xen_machine_pv parts. Both are useless on non-x86. CONFIG_XEN should simply always be a target specific option.
>>
>> Maybe the split-up between the "generic" host-side interfaces and
>> xen_machine_pv/xen_domainbuild is the problem. You know the dependencies
>> better than me, maybe you find a better fix.
> 
> Should be enough to just replace obj-$(CONFIG_XEN) by obj-i386-$(CONFIG_XEN).

Indeed (as long as qemu's xen remains x86-only).

> Unless it's very urgent, please wait with this patch until qemu-dm and xenner are in. It's pretty suboptimal to have 3 patches flying around that hit the exact same code spot :).

It isn't urgent. If patches series refactor the stuff and fix the
dependency, I'm happy to wait for them.

Jan
Alexander Graf - Nov. 29, 2010, 2:32 p.m.
On 29.11.2010, at 15:27, Jan Kiszka wrote:

> Am 29.11.2010 15:15, Alexander Graf wrote:
>> 
>> On 29.11.2010, at 13:44, Jan Kiszka wrote:
>> 
>>> Am 29.11.2010 13:40, Alexander Graf wrote:
>>>> 
>>>> On 29.11.2010, at 13:30, Jan Kiszka wrote:
>>>> 
>>>>> Am 29.11.2010 13:24, Alexander Graf wrote:
>>>>>> 
>>>>>> On 28.11.2010, at 16:59, Jan Kiszka wrote:
>>>>>> 
>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>> 
>>>>>>> Xen target bits in qemu are intended for x86. Let the build system
>>>>>>> reflect this and avoid useless building/linking for other targets.
>>>>>> 
>>>>>> Not sure I understand the split. Xen is x86 only, yes. But why split it into host and target? Target usually defines the guest. The piece you marked as _HOST are target specific.
>>>>> 
>>>>> At least so far, the HOST part is build once for all targets into the
>>>>> host backend library. As this step injected CONFIG_XEN into all target
>>>>> builds, even non-x86 targets built xen_machine_pv and xen_domainbuild.
>>>>> That's addressed by the patch.
>>>> 
>>>> I still don't understand the need for that split.
>>> 
>>> Enable Xen and build some non-x86 targets, then you see the need.
>>> 
>>>> The device drivers should be built only once, as do the xen_machine_pv parts. Both are useless on non-x86. CONFIG_XEN should simply always be a target specific option.
>>> 
>>> Maybe the split-up between the "generic" host-side interfaces and
>>> xen_machine_pv/xen_domainbuild is the problem. You know the dependencies
>>> better than me, maybe you find a better fix.
>> 
>> Should be enough to just replace obj-$(CONFIG_XEN) by obj-i386-$(CONFIG_XEN).
> 
> Indeed (as long as qemu's xen remains x86-only).

I'm 99.9% sure it will :).

>> Unless it's very urgent, please wait with this patch until qemu-dm and xenner are in. It's pretty suboptimal to have 3 patches flying around that hit the exact same code spot :).
> 
> It isn't urgent. If patches series refactor the stuff and fix the
> dependency, I'm happy to wait for them.

They don't really refactor it, but add a lot more dependencies. Anthony, since you probably need yet another round for the 4.0 compile stuff, feel like putting this on your TODO list too? (Sorry this takes so long :( - I'll try to review stuff quicker next time around)


Alex
Anthony PERARD - Dec. 1, 2010, 10:21 a.m.
On Mon, 29 Nov 2010, Alexander Graf wrote:

>
> On 29.11.2010, at 15:27, Jan Kiszka wrote:
>
> > Am 29.11.2010 15:15, Alexander Graf wrote:
> >>
> >> On 29.11.2010, at 13:44, Jan Kiszka wrote:
> >>
> >>> Am 29.11.2010 13:40, Alexander Graf wrote:
> >>>>
> >>>> On 29.11.2010, at 13:30, Jan Kiszka wrote:
> >>>>
> >>>>> Am 29.11.2010 13:24, Alexander Graf wrote:
> >>>>>>
> >>>>>> On 28.11.2010, at 16:59, Jan Kiszka wrote:
> >>>>>>
> >>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>>>>>>
> >>>>>>> Xen target bits in qemu are intended for x86. Let the build system
> >>>>>>> reflect this and avoid useless building/linking for other targets.
> >>>>>>
> >>>>>> Not sure I understand the split. Xen is x86 only, yes. But why split it into host and target? Target usually defines the guest. The piece you marked as _HOST are target specific.
> >>>>>
> >>>>> At least so far, the HOST part is build once for all targets into the
> >>>>> host backend library. As this step injected CONFIG_XEN into all target
> >>>>> builds, even non-x86 targets built xen_machine_pv and xen_domainbuild.
> >>>>> That's addressed by the patch.
> >>>>
> >>>> I still don't understand the need for that split.
> >>>
> >>> Enable Xen and build some non-x86 targets, then you see the need.
> >>>
> >>>> The device drivers should be built only once, as do the xen_machine_pv parts. Both are useless on non-x86. CONFIG_XEN should simply always be a target specific option.
> >>>
> >>> Maybe the split-up between the "generic" host-side interfaces and
> >>> xen_machine_pv/xen_domainbuild is the problem. You know the dependencies
> >>> better than me, maybe you find a better fix.
> >>
> >> Should be enough to just replace obj-$(CONFIG_XEN) by obj-i386-$(CONFIG_XEN).
> >
> > Indeed (as long as qemu's xen remains x86-only).
>
> I'm 99.9% sure it will :).
>
> >> Unless it's very urgent, please wait with this patch until qemu-dm and xenner are in. It's pretty suboptimal to have 3 patches flying around that hit the exact same code spot :).
> >
> > It isn't urgent. If patches series refactor the stuff and fix the
> > dependency, I'm happy to wait for them.
>
> They don't really refactor it, but add a lot more dependencies. Anthony, since you probably need yet another round for the 4.0 compile stuff, feel like putting this on your TODO list too? (Sorry this takes so long :( - I'll try to review stuff quicker next time around)

OK, but I will go a little further by putting all Xen stuff in x86 only
target, not only xenpv.
Alexander Graf - Dec. 1, 2010, 10:25 a.m.
On 01.12.2010, at 11:21, Anthony PERARD wrote:

> On Mon, 29 Nov 2010, Alexander Graf wrote:
> 
>> 
>> On 29.11.2010, at 15:27, Jan Kiszka wrote:
>> 
>>> Am 29.11.2010 15:15, Alexander Graf wrote:
>>>> 
>>>> On 29.11.2010, at 13:44, Jan Kiszka wrote:
>>>> 
>>>>> Am 29.11.2010 13:40, Alexander Graf wrote:
>>>>>> 
>>>>>> On 29.11.2010, at 13:30, Jan Kiszka wrote:
>>>>>> 
>>>>>>> Am 29.11.2010 13:24, Alexander Graf wrote:
>>>>>>>> 
>>>>>>>> On 28.11.2010, at 16:59, Jan Kiszka wrote:
>>>>>>>> 
>>>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>>>> 
>>>>>>>>> Xen target bits in qemu are intended for x86. Let the build system
>>>>>>>>> reflect this and avoid useless building/linking for other targets.
>>>>>>>> 
>>>>>>>> Not sure I understand the split. Xen is x86 only, yes. But why split it into host and target? Target usually defines the guest. The piece you marked as _HOST are target specific.
>>>>>>> 
>>>>>>> At least so far, the HOST part is build once for all targets into the
>>>>>>> host backend library. As this step injected CONFIG_XEN into all target
>>>>>>> builds, even non-x86 targets built xen_machine_pv and xen_domainbuild.
>>>>>>> That's addressed by the patch.
>>>>>> 
>>>>>> I still don't understand the need for that split.
>>>>> 
>>>>> Enable Xen and build some non-x86 targets, then you see the need.
>>>>> 
>>>>>> The device drivers should be built only once, as do the xen_machine_pv parts. Both are useless on non-x86. CONFIG_XEN should simply always be a target specific option.
>>>>> 
>>>>> Maybe the split-up between the "generic" host-side interfaces and
>>>>> xen_machine_pv/xen_domainbuild is the problem. You know the dependencies
>>>>> better than me, maybe you find a better fix.
>>>> 
>>>> Should be enough to just replace obj-$(CONFIG_XEN) by obj-i386-$(CONFIG_XEN).
>>> 
>>> Indeed (as long as qemu's xen remains x86-only).
>> 
>> I'm 99.9% sure it will :).
>> 
>>>> Unless it's very urgent, please wait with this patch until qemu-dm and xenner are in. It's pretty suboptimal to have 3 patches flying around that hit the exact same code spot :).
>>> 
>>> It isn't urgent. If patches series refactor the stuff and fix the
>>> dependency, I'm happy to wait for them.
>> 
>> They don't really refactor it, but add a lot more dependencies. Anthony, since you probably need yet another round for the 4.0 compile stuff, feel like putting this on your TODO list too? (Sorry this takes so long :( - I'll try to review stuff quicker next time around)
> 
> OK, but I will go a little further by putting all Xen stuff in x86 only
> target, not only xenpv.

Yes, please. That's perfect :).


Alex

Patch

diff --git a/Makefile.objs b/Makefile.objs
index 13ba26f..eabb032 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -148,8 +148,8 @@  slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o
 common-obj-$(CONFIG_SLIRP) += $(addprefix slirp/, $(slirp-obj-y))
 
 # xen backend driver support
-common-obj-$(CONFIG_XEN) += xen_backend.o xen_devconfig.o
-common-obj-$(CONFIG_XEN) += xen_console.o xenfb.o xen_disk.o xen_nic.o
+common-obj-$(CONFIG_XEN_HOST) += xen_backend.o xen_devconfig.o
+common-obj-$(CONFIG_XEN_HOST) += xen_console.o xenfb.o xen_disk.o xen_nic.o
 
 ######################################################################
 # libuser
diff --git a/Makefile.target b/Makefile.target
index 5784844..0863d5c 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -203,8 +203,8 @@  QEMU_CFLAGS += $(VNC_SASL_CFLAGS)
 QEMU_CFLAGS += $(VNC_JPEG_CFLAGS)
 QEMU_CFLAGS += $(VNC_PNG_CFLAGS)
 
-# xen backend driver support
-obj-$(CONFIG_XEN) += xen_machine_pv.o xen_domainbuild.o
+# xen target support
+obj-$(CONFIG_XEN_TARGET) += xen_machine_pv.o xen_domainbuild.o
 
 # USB layer
 obj-$(CONFIG_USB_OHCI) += usb-ohci.o
diff --git a/configure b/configure
index 2917874..3dd252a 100755
--- a/configure
+++ b/configure
@@ -2565,7 +2565,7 @@  if test "$bluez" = "yes" ; then
   echo "BLUEZ_CFLAGS=$bluez_cflags" >> $config_host_mak
 fi
 if test "$xen" = "yes" ; then
-  echo "CONFIG_XEN=y" >> $config_host_mak
+  echo "CONFIG_XEN_HOST=y" >> $config_host_mak
 fi
 if test "$io_thread" = "yes" ; then
   echo "CONFIG_IOTHREAD=y" >> $config_host_mak
@@ -2903,7 +2903,7 @@  echo "TARGET_ABI_DIR=$TARGET_ABI_DIR" >> $config_target_mak
 case "$target_arch2" in
   i386|x86_64)
     if test "$xen" = "yes" -a "$target_softmmu" = "yes" ; then
-      echo "CONFIG_XEN=y" >> $config_target_mak
+      echo "CONFIG_XEN_TARGET=y" >> $config_target_mak
     fi
 esac
 case "$target_arch2" in