[PATCH] ia64 kvm fixes for O=... builds

[PATCH] ia64 kvm fixes for O=... builds

[Al Viro]

* EXTRA_CFLAGS do not apply for *.S
* don't bother with symlinks to ../lib/mem*.S, just add ../lib/mem*.o
  to object list

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 arch/ia64/kvm/Makefile |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/ia64/kvm/Makefile b/arch/ia64/kvm/Makefile
index 41b034f..5235339 100644
--- a/arch/ia64/kvm/Makefile
+++ b/arch/ia64/kvm/Makefile
@@ -42,10 +42,7 @@ $(obj)/$(offsets-file): arch/ia64/kvm/asm-offsets.s
 #
 
 EXTRA_CFLAGS += -Ivirt/kvm -Iarch/ia64/kvm/
-
-$(addprefix $(objtree)/,$(obj)/memcpy.S $(obj)/memset.S):
-	$(shell ln -snf ../lib/memcpy.S $(src)/memcpy.S)
-	$(shell ln -snf ../lib/memset.S $(src)/memset.S)
+EXTRA_AFLAGS += -Ivirt/kvm -Iarch/ia64/kvm/
 
 common-objs = $(addprefix ../../../virt/kvm/, kvm_main.o ioapic.o)
 
@@ -57,5 +54,5 @@ EXTRA_CFLAGS_vcpu.o += -mfixed-range=f2-f5,f12-f127
 kvm-intel-objs = vmm.o vmm_ivt.o trampoline.o vcpu.o optvfault.o mmio.o \
 	vtlb.o process.o
 #Add link memcpy and memset to avoid possible structure assignment error
-kvm-intel-objs += memset.o memcpy.o
+kvm-intel-objs += ../lib/memset.o ../lib/memcpy.o
 obj-$(CONFIG_KVM_INTEL) += kvm-intel.o
-- 
1.5.3.GIT

RE: [kvm-ia64-devel] [PATCH] ia64 kvm fixes for O=... builds

[Zhang, Xiantao]

Al Viro wrote:
> * EXTRA_CFLAGS do not apply for *.S
> * don't bother with symlinks to ../lib/mem*.S, just add ../lib/mem*.o
>   to object list
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  arch/ia64/kvm/Makefile |    7 ++-----
>  1 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/ia64/kvm/Makefile b/arch/ia64/kvm/Makefile
> index 41b034f..5235339 100644
> --- a/arch/ia64/kvm/Makefile
> +++ b/arch/ia64/kvm/Makefile
> @@ -42,10 +42,7 @@ $(obj)/$(offsets-file): arch/ia64/kvm/asm-offsets.s
>  #
> 
>  EXTRA_CFLAGS += -Ivirt/kvm -Iarch/ia64/kvm/
> -
> -$(addprefix $(objtree)/,$(obj)/memcpy.S $(obj)/memset.S):
> -	$(shell ln -snf ../lib/memcpy.S $(src)/memcpy.S)
> -	$(shell ln -snf ../lib/memset.S $(src)/memset.S)
> +EXTRA_AFLAGS += -Ivirt/kvm -Iarch/ia64/kvm/
> 
>  common-objs = $(addprefix ../../../virt/kvm/, kvm_main.o ioapic.o)
> 
> @@ -57,5 +54,5 @@ EXTRA_CFLAGS_vcpu.o += -mfixed-range=f2-f5,f12-f127
>  kvm-intel-objs = vmm.o vmm_ivt.o trampoline.o vcpu.o optvfault.o
>  	mmio.o \ vtlb.o process.o
>  #Add link memcpy and memset to avoid possible structure assignment
> error -kvm-intel-objs += memset.o memcpy.o
> +kvm-intel-objs += ../lib/memset.o ../lib/memcpy.o
>  obj-$(CONFIG_KVM_INTEL) += kvm-intel.o

I don't think it is workable for kvm-ia64.  Currently,  kvm-intel module
needs to be relocated at insertion time, so the code is
position-indenpent. Howerver, lib/*.memset.o(memcoy.o) are not compiled
as position independent code, they can't be linked into the module
correctly in your way. That is why we use symbol link instead of linking
the objects under /lib. 

Xiantao

Re: [kvm-ia64-devel] [PATCH] ia64 kvm fixes for O=... builds

[Al Viro]

On Mon, Apr 28, 2008 at 09:46:44PM +0800, Zhang, Xiantao wrote:
> >  kvm-intel-objs = vmm.o vmm_ivt.o trampoline.o vcpu.o optvfault.o
> >  	mmio.o \ vtlb.o process.o
> >  #Add link memcpy and memset to avoid possible structure assignment
> > error -kvm-intel-objs += memset.o memcpy.o
> > +kvm-intel-objs += ../lib/memset.o ../lib/memcpy.o
> >  obj-$(CONFIG_KVM_INTEL) += kvm-intel.o
> 
> I don't think it is workable for kvm-ia64.  Currently,  kvm-intel module
> needs to be relocated at insertion time, so the code is
> position-indenpent. Howerver, lib/*.memset.o(memcoy.o) are not compiled
> as position independent code, they can't be linked into the module
> correctly in your way. That is why we use symbol link instead of linking
> the objects under /lib. 

symlinks won't work as you use them.  If you really need separately
compiled, use file with a couple of #include "../lib/......S" and
be done with that.

Re: [kvm-ia64-devel] [PATCH] ia64 kvm fixes for O=... builds

[Al Viro]

On Mon, Apr 28, 2008 at 03:14:02PM +0100, Al Viro wrote:
> On Mon, Apr 28, 2008 at 09:46:44PM +0800, Zhang, Xiantao wrote:
> > >  kvm-intel-objs = vmm.o vmm_ivt.o trampoline.o vcpu.o optvfault.o
> > >  	mmio.o \ vtlb.o process.o
> > >  #Add link memcpy and memset to avoid possible structure assignment
> > > error -kvm-intel-objs += memset.o memcpy.o
> > > +kvm-intel-objs += ../lib/memset.o ../lib/memcpy.o
> > >  obj-$(CONFIG_KVM_INTEL) += kvm-intel.o
> > 
> > I don't think it is workable for kvm-ia64.  Currently,  kvm-intel module
> > needs to be relocated at insertion time, so the code is
> > position-indenpent. Howerver, lib/*.memset.o(memcoy.o) are not compiled
> > as position independent code, they can't be linked into the module
> > correctly in your way. That is why we use symbol link instead of linking
> > the objects under /lib. 
> 
> symlinks won't work as you use them.  If you really need separately
> compiled, use file with a couple of #include "../lib/......S" and
> be done with that.

... like this, perhaps?  I still wonder what will happen with either this
or the original tree if you make CONFIG_KVM_INTEL=y, though - won't either
variant give you duplicate symbols?

diff --git a/arch/ia64/kvm/Makefile b/arch/ia64/kvm/Makefile
index 5235339..e7b9432 100644
--- a/arch/ia64/kvm/Makefile
+++ b/arch/ia64/kvm/Makefile
@@ -54,5 +54,5 @@ EXTRA_CFLAGS_vcpu.o += -mfixed-range=f2-f5,f12-f127
 kvm-intel-objs = vmm.o vmm_ivt.o trampoline.o vcpu.o optvfault.o mmio.o \
 	vtlb.o process.o
 #Add link memcpy and memset to avoid possible structure assignment error
-kvm-intel-objs += ../lib/memset.o ../lib/memcpy.o
+kvm-intel-objs += memset.o memcpy.o
 obj-$(CONFIG_KVM_INTEL) += kvm-intel.o
diff --git a/arch/ia64/kvm/memcpy.S b/arch/ia64/kvm/memcpy.S
new file mode 100644
index 0000000..c04cdbe
--- /dev/null
+++ b/arch/ia64/kvm/memcpy.S
@@ -0,0 +1 @@
+#include "../lib/memcpy.S"
diff --git a/arch/ia64/kvm/memset.S b/arch/ia64/kvm/memset.S
new file mode 100644
index 0000000..83c3066
--- /dev/null
+++ b/arch/ia64/kvm/memset.S
@@ -0,0 +1 @@
+#include "../lib/memset.S"

RE: [kvm-ia64-devel] [PATCH] ia64 kvm fixes for O=... builds

[Zhang, Xiantao]

Hi, Airo
    Currenlty,  We only allow CONFIG_KVM_INTEL = m , since it needs to
be relocated at insertion time. 
Xiantao
Al Viro wrote:
> On Mon, Apr 28, 2008 at 03:14:02PM +0100, Al Viro wrote:
>> On Mon, Apr 28, 2008 at 09:46:44PM +0800, Zhang, Xiantao wrote:
>>>>  kvm-intel-objs = vmm.o vmm_ivt.o trampoline.o vcpu.o optvfault.o 
>>>>  mmio.o \ vtlb.o process.o #Add link memcpy and memset to avoid
>>>> possible structure assignment error -kvm-intel-objs += memset.o
>>>> memcpy.o +kvm-intel-objs += ../lib/memset.o ../lib/memcpy.o
>>>>  obj-$(CONFIG_KVM_INTEL) += kvm-intel.o
>>> 
>>> I don't think it is workable for kvm-ia64.  Currently,  kvm-intel
>>> module needs to be relocated at insertion time, so the code is
>>> position-indenpent. Howerver, lib/*.memset.o(memcoy.o) are not
>>> compiled as position independent code, they can't be linked into
>>> the module correctly in your way. That is why we use symbol link
>>> instead of linking the objects under /lib.
>> 
>> symlinks won't work as you use them.  If you really need separately
>> compiled, use file with a couple of #include "../lib/......S" and
>> be done with that.
> 
> ... like this, perhaps?  I still wonder what will happen with either
> this or the original tree if you make CONFIG_KVM_INTEL=y, though -
> won't either variant give you duplicate symbols?
> 
> diff --git a/arch/ia64/kvm/Makefile b/arch/ia64/kvm/Makefile
> index 5235339..e7b9432 100644
> --- a/arch/ia64/kvm/Makefile
> +++ b/arch/ia64/kvm/Makefile
> @@ -54,5 +54,5 @@ EXTRA_CFLAGS_vcpu.o += -mfixed-range=f2-f5,f12-f127
>  kvm-intel-objs = vmm.o vmm_ivt.o trampoline.o vcpu.o optvfault.o
>  	mmio.o \ vtlb.o process.o
>  #Add link memcpy and memset to avoid possible structure assignment
> error -kvm-intel-objs += ../lib/memset.o ../lib/memcpy.o
> +kvm-intel-objs += memset.o memcpy.o
>  obj-$(CONFIG_KVM_INTEL) += kvm-intel.o
> diff --git a/arch/ia64/kvm/memcpy.S b/arch/ia64/kvm/memcpy.S
> new file mode 100644
> index 0000000..c04cdbe
> --- /dev/null
> +++ b/arch/ia64/kvm/memcpy.S
> @@ -0,0 +1 @@
> +#include "../lib/memcpy.S"
> diff --git a/arch/ia64/kvm/memset.S b/arch/ia64/kvm/memset.S
> new file mode 100644
> index 0000000..83c3066
> --- /dev/null
> +++ b/arch/ia64/kvm/memset.S
> @@ -0,0 +1 @@
> +#include "../lib/memset.S"

Re: [kvm-ia64-devel] [PATCH] ia64 kvm fixes for O=... builds

[Avi Kivity]

Zhang, Xiantao wrote:
> Hi, Airo
>     Currenlty,  We only allow CONFIG_KVM_INTEL = m , since it needs to
> be relocated at insertion time. 
>   

In many ways this module is more like firmware than a standard kernel 
module, no?

-- 
error compiling committee.c: too many arguments to function

RE: [kvm-ia64-devel] [PATCH] ia64 kvm fixes for O=... builds

[Zhang, Xiantao]

Avi Kivity wrote:
> Zhang, Xiantao wrote:
>> Hi, Airo
>>     Currenlty,  We only allow CONFIG_KVM_INTEL = m , since it needs
>> to be relocated at insertion time.
>> 
> 
> In many ways this module is more like firmware than a standard kernel
> module, no?

Right, this module doesn't work in kernel context. Instead, it is
responsible for switching world between host and guest after relocation.
In this way, likely to be a firmware. 
Xiantao