Re: [PATCH 35/38] unicore32: clean-up uapi Kbuild file
Hi James. > > +# Wapper from asm-generic > > generic-y += kvm_para.h > > > > You dropped the generic-y += kvm_para.h for arm64 & c6x, but not for > unicore, avr32 & ia64 patch, which seems a bit inconsistent. Was there > some reason behind that? I was fooling around with kvm_para.h and in both cases dropping them was wrong. kvm_para.h shows the current nightmare when you want to add a header file that for most architectures has a generic implementation, but in a few cases have a dedicated implementation. This is the same point that Arnd raises. I will fix it up so kvm_para.h is again specified for arm64 + c6x. And then will address the whole mess later if I manage to come up with something that is acceptable (simple enough). Thanks for the review! Sam -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm: make asm-generic/kvm_para.h have an ifdef __KERNEL__ block
On Fri, May 18, 2012 at 01:59:39PM -0400, Paul Gortmaker wrote: > There are two functions in this asm-generic file. Looking at > other arch which do not use the generic version, these two fcns > are within an #ifdef __KERNEL__ block, so make the generic one > consistent with those. > > Signed-off-by: Paul Gortmaker > > diff --git a/include/asm-generic/kvm_para.h b/include/asm-generic/kvm_para.h > index 9a7bbad..5cba37f 100644 > --- a/include/asm-generic/kvm_para.h > +++ b/include/asm-generic/kvm_para.h > @@ -1,6 +1,7 @@ > #ifndef _ASM_GENERIC_KVM_PARA_H > #define _ASM_GENERIC_KVM_PARA_H > > +#ifdef __KERNEL__ > > /* > * This function is used by architectures that support kvm to avoid issuing > @@ -16,4 +17,6 @@ static inline unsigned int kvm_arch_para_features(void) > return 0; > } > > +#endif /* _KERNEL__ */ > + > #endif I do not have this file in my tree... But in general we should try to separate out exporeted stuff in their own headers, so no new files should ever use __KERNEL__. Either everything or nothing is exported. Sam -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: asm/kvm_para.h from asm-generic (was: Re: linux-next: triage for April 19, 2012)
> >> > >> I just noticed include/asm-generic/Kbuild.asm already had > >> > >> ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/kvm_para.h \ > >> $(srctree)/include/asm-$(SRCARCH)/kvm_para.h),) > >> header-y += kvm_para.h > >> endif > >> > >> but this doesn't seem to work. > >> > >> Kbuild people: which one is correct? Any use of wildcards in include/asm-generic/Kbuild.asm is bogus. We should: 1) add explicit header-y for kvm.h + a.out.h to the relevant architectures. 2) add explicit header-y for kvm_para.h for the archictectures that needs this file 3) and drop header-y / dummy file fro the others. Anything else is just papering over the initial wrong doings. I actually coded up "header-n" support to explicitly say: do not include this file for this arch - but looking at the actual problem being solved this is just bogus so I killed it off again. Sam -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: asm/kvm_para.h from asm-generic (was: Re: linux-next: triage for April 19, 2012)
On Tue, Jun 19, 2012 at 08:48:03PM +0200, Sam Ravnborg wrote: > > >> > > >> I just noticed include/asm-generic/Kbuild.asm already had > > >> > > >> ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/kvm_para.h \ > > >> $(srctree)/include/asm-$(SRCARCH)/kvm_para.h),) > > >> header-y += kvm_para.h > > >> endif > > >> > > >> but this doesn't seem to work. > > >> > > >> Kbuild people: which one is correct? > > Any use of wildcards in include/asm-generic/Kbuild.asm is bogus. > We should: > 1) add explicit header-y for kvm.h + a.out.h to the relevant architectures. > 2) add explicit header-y for kvm_para.h for the archictectures that needs > this file > 3) and drop header-y / dummy file fro the others. > > Anything else is just papering over the initial wrong doings. > > I actually coded up "header-n" support to explicitly say: > do not include this file for this arch - but looking at the > actual problem being solved this is just bogus so I killed > it off again. Just to be clear - I know this will break headers_check_all. I will try to take a look soon for a better generic solution. Sam -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [KVM PATCH] KVM: introduce "xinterface" API for external interaction with guests
> diff --git a/arch/x86/Kbuild b/arch/x86/Kbuild > index ad8ec35..9f50cc3 100644 > --- a/arch/x86/Kbuild > +++ b/arch/x86/Kbuild > @@ -1,5 +1,7 @@ > > -obj-$(CONFIG_KVM) += kvm/ > +ifdef CONFIG_KVM > +obj-y += kvm/ > +endif What was wrong with the old version? If this is because xinterface.o is builtin to the kernel then we should always visit kvm/ and use: obj-y += kvm/ unconditionally. > > # Xen paravirtualization support > obj-$(CONFIG_XEN) += xen/ > diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile > index afaaa76..80d951d 100644 > --- a/arch/x86/kvm/Makefile > +++ b/arch/x86/kvm/Makefile > @@ -17,3 +17,7 @@ kvm-amd-y += svm.o > obj-$(CONFIG_KVM)+= kvm.o > obj-$(CONFIG_KVM_INTEL) += kvm-intel.o > obj-$(CONFIG_KVM_AMD)+= kvm-amd.o > + > +ifdef CONFIG_KVM > +obj-y += $(addprefix ../../../virt/kvm/, xinterface.o) > +endif And when we always visit kvm/ the above code snippet makes sense. Before we never visited kvm/ unless CONFIG_KVM was defined. Sam -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -tip -v12 02/11] x86: x86 instruction decoder build-time selftest
On Thu, Jul 16, 2009 at 11:57:06AM -0400, Masami Hiramatsu wrote:
> Add a user-space selftest of x86 instruction decoder at kernel build time.
> When CONFIG_X86_DECODER_SELFTEST=y, Kbuild builds a test harness of x86
> instruction decoder and performs it after building vmlinux.
> The test compares the results of objdump and x86 instruction decoder
> code and check there are no differences.
Long overdue review from my side...
> arch/x86/scripts/Makefile | 19 +++
> arch/x86/scripts/distill.awk| 42 +
> arch/x86/scripts/test_get_len.c | 99
> +++
> arch/x86/scripts/user_include.h | 49 +++
Hmmm, we have two architectures that uses scripts/ and three that
uses tools/.
I prefer the latter name as what we have ere is beyound what
I generally recognize as a script.
we have scripts/ in top-level and we do not rename this
as we have this hardcoded too many places - but no reason to
use the wrong name here.
> diff --git a/arch/x86/include/asm/inat.h b/arch/x86/include/asm/inat.h
> index 01e079a..9090665 100644
> --- a/arch/x86/include/asm/inat.h
> +++ b/arch/x86/include/asm/inat.h
> @@ -20,7 +20,9 @@
> * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
> *
> */
> +#ifdef __KERNEL__
> #include
> +#endif
>
> /* Instruction attributes */
> typedef u32 insn_attr_t;
Why this?
If you need this to use this file from userspace then could we do some
other trick to make this OK?
I see it repeated several times below.
[If this has already been discussed I have missed it - sorry].
> diff --git a/arch/x86/scripts/Makefile b/arch/x86/scripts/Makefile
> new file mode 100644
> index 000..f08859e
> --- /dev/null
> +++ b/arch/x86/scripts/Makefile
> @@ -0,0 +1,19 @@
> +PHONY += posttest
> +quiet_cmd_posttest = TEST$@
> + cmd_posttest = objdump -d $(objtree)/vmlinux | awk -f
> $(srctree)/arch/x86/scripts/distill.awk | $(obj)/test_get_len
> +
You are using the native objdump here.
But I assume this fails miserably when you build x86 on a powerpc host.
In other words - you broke an allyesconfig build for -next...
We have $(OBJDUMP) for this.
> +posttest: $(obj)/test_get_len vmlinux
> + $(call cmd,posttest)
> +
> +test_get_len_SRC = $(srctree)/arch/x86/scripts/test_get_len.c
> $(srctree)/arch/x86/lib/insn.c $(srctree)/arch/x86/lib/inat.c
> +test_get_len_INC = $(srctree)/arch/x86/include/asm/inat.h
> $(srctree)/arch/x86/include/asm/insn.h $(objtree)/arch/x86/lib/inat-tables.c
> +
> +quiet_cmd_test_get_len = CC $@
> + cmd_test_get_len = $(CC) -Wall $(test_get_len_SRC)
> -I$(objtree)/arch/x86/lib/ -I$(srctree)/arch/x86/include -include
> $(srctree)/arch/x86/scripts/user_include.h -o $@
Is there a specific reason why you cannot use the standard hostprogs-y for this?
It will take care of dependency tracking etc.
What you have above is a hopeless incomplete list of dependencies.
You need to use HOST_EXTRACFLAGS to set additional -I options and the -include.
> +
> +static void usage()
> +{
> + fprintf(stderr, "usage: %s < distilled_disassembly\n", prog);
> + exit(1);
> +}
It would be nice to tell the user what the program is supposed to do.
I know this is a bit unusual but no reason to copy bad practice.
> index 000..3bdcc55
> --- /dev/null
> +++ b/arch/x86/scripts/user_include.h
> @@ -0,0 +1,49 @@
> +#ifndef __USER_TYPES_H
> +#define __USER_TYPES_H
> +
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
> + *
> + * Copyright (C) IBM Corporation, 2009
> + */
> +
> +#include
> +
> +#ifdef __x86_64__
> +#define CONFIG_X86_64
> +#else
> +#define CONFIG_X86_32
> +#endif
> +typedef unsigned char u8;
> +typedef unsigned short u16;
> +typedef unsigned int u32;
> +typedef unsigned long long u64;
> +
> +typedef signed char s8;
> +typedef short s16;
> +typedef int s32;
> +typedef long long s64;
> +
> +typedef enum bool { false = 0, true } bool;
> +
> +/* any harmless file-scope decl */
> +#define NOP_DECL struct __nop
> +#define EXPORT_SYMBOL_GPL(symbol) NOP_DECL
> +#define MODULE_LICENSE(gpl) NOP_DECL
> +
> +#define WARN_ON(cond) do { } while (0)
> +#define unlikely(cond) (cond)
> +
So this is a file that alows you to include the other files without dragging in
a massive a
Re: [PATCH -tip -v12 01/11] x86: instruction decoder API
> > diff --git a/arch/x86/include/asm/inat.h b/arch/x86/include/asm/inat.h > new file mode 100644 > index 000..01e079a > --- /dev/null > +++ b/arch/x86/include/asm/inat.h > @@ -0,0 +1,125 @@ > +#ifndef _ASM_INAT_INAT_H > +#define _ASM_INAT_INAT_H [With reference to comment on patch 2/12...] You create inat.h here. Could you investigave what is needed to factor out the stuff needed from userspace so we can avoid the ugly havk where you redefine types.h? Maybe create a inat_types.h + inat.h as we do in other cases? Same for the other files that requred the types.h hack. Sam -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -tip -v12 02/11] x86: x86 instruction decoder build-time selftest
> >> + cmd_posttest = objdump -d $(objtree)/vmlinux | awk -f > >> $(srctree)/arch/x86/scripts/distill.awk | $(obj)/test_get_len > >> + > > > > You are using the native objdump here. > > But I assume this fails miserably when you build x86 on a powerpc host. > > In other words - you broke an allyesconfig build for -next... > > We have $(OBJDUMP) for this. > > Ah, I see... Would you know actual name of x86-objdump on the powerpc > (or any other crosscompiling host)? I just set "OBJDUMP=objdump" is OK? > I'm not so sure about cross-compiling kernel... Replacing objdump with $(OBJDUMP) will do the trick. We set OBJDUMP to the correct value in the top-level makefile. Are there any parts of your user-space program that rely on the host is little-endian? If it does then it would fail on a power-pc target despite using the correct objdump. Sam -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -tip -v12 01/11] x86: instruction decoder API
On Thu, Jul 16, 2009 at 01:28:54PM -0400, Masami Hiramatsu wrote: > Sam Ravnborg wrote: > >> diff --git a/arch/x86/include/asm/inat.h b/arch/x86/include/asm/inat.h > >> new file mode 100644 > >> index 000..01e079a > >> --- /dev/null > >> +++ b/arch/x86/include/asm/inat.h > >> @@ -0,0 +1,125 @@ > >> +#ifndef _ASM_INAT_INAT_H > >> +#define _ASM_INAT_INAT_H > > > > [With reference to comment on patch 2/12...] > > You create inat.h here. > > Could you investigave what is needed to factor out the stuff > > needed from userspace so we can avoid the ugly havk where > > you redefine types.h? > > Sorry, I'm a bit confusing. > Would you mean that I should break down user_include.h and > add those redefined types in inat.h? No - try to factor out what is needed for your program so you can avoid user_include.h entirely. > > > Maybe create a inat_types.h + inat.h as we do in other cases? > > And inat_types.h has two parts, one for kernel, and one for > userspace(which is moved from user_include.h), is that right? More like inat_types.h include pure definitions and inat.h define all the macros (that would be much nicer if expressed as static inlines). The real thing to consider is what is needed from your userspace program and is also required by the kernel. I did not event remotely try to find out - as I guess you know it. So try to isolate these bits somehow and you have then nicely dropped a lot of dependencies on the remainign headers and can thus hopefully get rid of the ugly usser_include.h hack. Sam -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [2.6 patch] remove dummy asm/kvm.h files
On Tue, Jul 01, 2008 at 07:27:16PM +0300, Adrian Bunk wrote: > This patch removes the dummy asm/kvm.h files on architectures not (yet) > supporting KVM and uses the same conditional headers installation as > already used for a.out.h . > > Also removed are superfluous install rules in the s390 and x86 Kbuild > files (they are already in Kbuild.asm). > > Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]> I really do not like the way we implement these checks and think this should be a kconfig option. But we do not (yet) require a configured kernel when exporting to userspace so this is not feasible right now. So despite the above this patch has my: Acked-by: Sam Ravnborg <[EMAIL PROTECTED]> Same for the asm/a.out patch you posted the other day. David - I assume you take these via your tree? Sam -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [2.6 patch] remove dummy asm/kvm.h files
On Tue, Jul 01, 2008 at 08:53:20PM +0100, David Woodhouse wrote: > On Tue, 2008-07-01 at 21:32 +0200, Sam Ravnborg wrote: > > On Tue, Jul 01, 2008 at 07:27:16PM +0300, Adrian Bunk wrote: > > > This patch removes the dummy asm/kvm.h files on architectures not (yet) > > > supporting KVM and uses the same conditional headers installation as > > > already used for a.out.h . > > > > > > Also removed are superfluous install rules in the s390 and x86 Kbuild > > > files (they are already in Kbuild.asm). > > > > > > Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]> > > > > I really do not like the way we implement these checks and > > think this should be a kconfig option. > > But we do not (yet) require a configured kernel when exporting > > to userspace so this is not feasible right now. > > I'm _really_ dubious about doing this kind of thing through Kconfig. > > In the general case, we really don't want exported headers to have any > differences based on the kernel configuration. > > Yes, I suppose it would be OK if we are very careful not to let anyone > use config symbols that aren't hard-coded for the given architecture. > But in practice I don't think there's any real chance that we'd manage > to be careful _enough_. We'd find people using symbols they shouldn't > be. We have two cases now and I think we can manage it. But if we have only a few cases then we can also accept use of ARCH. > > > So despite the above this patch has my: > > Acked-by: Sam Ravnborg <[EMAIL PROTECTED]> > > > > Same for the asm/a.out patch you posted the other day. > > > > David - I assume you take these via your tree? > > I can do. Thanks, Sam -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm: ppc: fix Kconfig constraints
On Sun, Nov 09, 2008 at 12:05:02PM +0200, Avi Kivity wrote: > Hollis Blanchard wrote: > >kvm: ppc: fix Kconfig constraints > > > >Make sure that CONFIG_KVM cannot be selected without processor support > >(currently, 440 is the only processor implementation available). > > > >Signed-off-by: Hollis Blanchard <[EMAIL PROTECTED]> > > > >diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig > >--- a/arch/powerpc/kvm/Kconfig > >+++ b/arch/powerpc/kvm/Kconfig > >@@ -15,24 +15,22 @@ if VIRTUALIZATION > > if VIRTUALIZATION > > > > config KVM > >-bool "Kernel-based Virtual Machine (KVM) support" > >-depends on EXPERIMENTAL > >+bool > > select PREEMPT_NOTIFIERS > > select ANON_INODES > >+ > >+config KVM_440 > >+bool "KVM support for PowerPC 440 processors" > >+depends on EXPERIMENTAL && 44x > >+select KVM > > > > I have a vague memory that 'select' will not propagate (what's the word? > transitive?) - -that is, the 'select KVM' will enable KVM but will not > cause ANON_INODES and PREEMPT_NOTIFIERS to be set. I'm glad to say your memory in this case is wrong. Select works recursively so the usage above is OK. If KVM had a dependency such as EXPERIMENTAL then select would enable KVM no matter the value of EXPERIMENTAL. Sam -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: Avoid using CONFIG_ in userspace visible headers
> > btw., would be nice to somehow untangle consciously-exported interface > definitions from kernel side bits, and standardize these > feature/capability flags like __KVM_HAVE_IOAPIC, etc. > > Right now we have this body of 75,000 lines of code spread out in 600+ > header files that are so-called 'exported' to user-space, but 95% of that > interface definition code is never being relied on by any user-space bit! > They are exported due to cargo-cult mentality or due to dependencies. With the exported headers we at least have a clear definition what part of the headers are actually exported. So what we have now is a much better base to cut off from. And there is plenty of room for improvements but it requires someone with understanding of the kernel side and the user side to do this. I for once find it hard to judge what is really used by user space and not. Diving into glibc does not help as userspace is much more than glibc although glibc is an important 'customer' here. Sam -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: Avoid using CONFIG_ in userspace visible headers
On Mon, Jan 19, 2009 at 03:00:50PM +0200, Avi Kivity wrote: > Kconfig symbols are not available in userspace, and are not stripped by > headers-install. Avoid their use by adding #defines in to > suit each architecture. > > Signed-off-by: Avi Kivity Looked through the patch and everything looked fine. Sam -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
