diff mbox

[5/5,V2] kvm tools: Initialize and use VESA and VNC

Message ID 1306149553-26793-5-git-send-email-levinsasha928@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sasha Levin May 23, 2011, 11:19 a.m. UTC
Requirements - Kernel compiled with:
CONFIG_FB_BOOT_VESA_SUPPORT=y
CONFIG_FB_VESA=y
CONFIG_FRAMEBUFFER_CONSOLE=y

Start VNC server by starting kvm tools with "--vnc".
Connect to the VNC server by running: "vncviewer :0".

Since there is no support for input devices at this time,
it may be useful starting kvm tools with an additional
' -p "console=ttyS0" ' parameter so that it would be possible
to use a serial console alongside with a graphic one.

Signed-off-by: John Floren <john@jfloren.net>
[ turning code into patches and cleanup ]
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/kvm-run.c |   17 +++++++++++++++--
 1 files changed, 15 insertions(+), 2 deletions(-)

Comments

Ingo Molnar May 23, 2011, 11:38 a.m. UTC | #1
* Sasha Levin <levinsasha928@gmail.com> wrote:

> @@ -511,7 +515,13 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
>  	kvm->nrcpus = nrcpus;
>  
>  	memset(real_cmdline, 0, sizeof(real_cmdline));
> -	strcpy(real_cmdline, "notsc noapic noacpi pci=conf1 console=ttyS0 earlyprintk=serial");
> +	strcpy(real_cmdline, "notsc noapic noacpi pci=conf1");
> +	if (vnc) {
> +		strcat(real_cmdline, " video=vesafb console=tty0");
> +		vidmode = 0x312;
> +	} else {
> +		strcat(real_cmdline, " console=ttyS0 earlyprintk=serial");
> +	}

Hm, i think all the kernel parameter handling code wants to move into driver 
specific routines as well. Something like:

	serial_init(kvm, real_cmdline);

where serial_init() would append to real_cmdline if needed.

This removes a bit of serial-driver specific knowledge from kvm-run.c.

Same goes for the VESA driver and the above video mode flag logic.

> @@ -597,6 +607,9 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
>  
>  	kvm__init_ram(kvm);
>  
> +	if (vnc)
> +		vesa__init(kvm);

Shouldnt vesa__init() itself know about whether it's active (i.e. the 'vnc' 
flag is set) and return early if it's not set?

That way this could become more encapsulated and self-sufficient:

	vesa__init(kvm);

With no VESA driver specific state exposed to the generic kvm_cmd_run() 
function.

Ideally kvm_cmd_run() hould just be a series of:

	serial_init(kvm, real_cmdline);
	vesa_init(kvm, real_cmdline);
	...

initialization routines. Later on even this could be removed: using section 
tricks we can put init functions into a section and drivers could register 
their init function like initcall(func) functions are registered within the 
kernel. kvm_cmd_run() could thus iterate over that (build time constructed) 
section like this:

extern initcall_t __initcall_start[], __initcall_end[], __early_initcall_end[];

static void __init do_initcalls(void)
{
        initcall_t *fn;

        for (fn = __early_initcall_end; fn < __initcall_end; fn++)
                do_one_initcall(*fn);
}

and would not actually have *any* knowledge about what drivers were built in.

Currently it's fine to initialize everything explicitly - but this would be the 
long term model to work towards ...

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pekka Enberg May 23, 2011, 11:45 a.m. UTC | #2
On 5/23/11 2:38 PM, Ingo Molnar wrote:
> * Sasha Levin<levinsasha928@gmail.com>  wrote:
>
>> @@ -511,7 +515,13 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
>>   	kvm->nrcpus = nrcpus;
>>
>>   	memset(real_cmdline, 0, sizeof(real_cmdline));
>> -	strcpy(real_cmdline, "notsc noapic noacpi pci=conf1 console=ttyS0 earlyprintk=serial");
>> +	strcpy(real_cmdline, "notsc noapic noacpi pci=conf1");
>> +	if (vnc) {
>> +		strcat(real_cmdline, " video=vesafb console=tty0");
>> +		vidmode = 0x312;
>> +	} else {
>> +		strcat(real_cmdline, " console=ttyS0 earlyprintk=serial");
>> +	}
> Hm, i think all the kernel parameter handling code wants to move into driver
> specific routines as well. Something like:
>
> 	serial_init(kvm, real_cmdline);
>
> where serial_init() would append to real_cmdline if needed.
>
> This removes a bit of serial-driver specific knowledge from kvm-run.c.
>
> Same goes for the VESA driver and the above video mode flag logic.
>
>> @@ -597,6 +607,9 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
>>
>>   	kvm__init_ram(kvm);
>>
>> +	if (vnc)
>> +		vesa__init(kvm);
> Shouldnt vesa__init() itself know about whether it's active (i.e. the 'vnc'
> flag is set) and return early if it's not set?
>
> That way this could become more encapsulated and self-sufficient:
>
> 	vesa__init(kvm);
>
> With no VESA driver specific state exposed to the generic kvm_cmd_run()
> function.
>
> Ideally kvm_cmd_run() hould just be a series of:
>
> 	serial_init(kvm, real_cmdline);
> 	vesa_init(kvm, real_cmdline);
> 	...
>
> initialization routines. Later on even this could be removed: using section
> tricks we can put init functions into a section and drivers could register
> their init function like initcall(func) functions are registered within the
> kernel. kvm_cmd_run() could thus iterate over that (build time constructed)
> section like this:
>
> extern initcall_t __initcall_start[], __initcall_end[], __early_initcall_end[];
>
> static void __init do_initcalls(void)
> {
>          initcall_t *fn;
>
>          for (fn = __early_initcall_end; fn<  __initcall_end; fn++)
>                  do_one_initcall(*fn);
> }
>
> and would not actually have *any* knowledge about what drivers were built in.
>
> Currently it's fine to initialize everything explicitly - but this would be the
> long term model to work towards ...

Prasad, didn't you have patches to do exactly that?
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pekka Enberg May 23, 2011, 2:10 p.m. UTC | #3
On Mon, May 23, 2011 at 2:19 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
> Requirements - Kernel compiled with:
> CONFIG_FB_BOOT_VESA_SUPPORT=y
> CONFIG_FB_VESA=y
> CONFIG_FRAMEBUFFER_CONSOLE=y

Dunno if it's possible but it would be nice to have a more readable
error message if you don't have that compiled in:

penberg@tiger:~/linux/tools/kvm$ ./kvm run --vnc -d
~/images/debian_squeeze_amd64_standard.img
  # kvm run -k ../../arch/x86/boot/bzImage -m 320 -c 2
  Warning: Config tap device error. Are you root?
23/05/2011 17:08:19 Listening for VNC connections on TCP port 5900
Undefined video mode number: 312
Press <ENTER> to see video modes available, <SPACE> to continue, or wait 30 sec
Killed

This obviously isn't an issue for merging this patch.

                       Pekka
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paolo Bonzini May 24, 2011, 8:37 a.m. UTC | #4
On 05/23/2011 01:38 PM, Ingo Molnar wrote:
> Later on even this could be removed: using section
> tricks we can put init functions into a section

This is not kernel space, the C library provides a way to do that with 
__attribute__((constructor))...

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ingo Molnar May 24, 2011, 8:50 a.m. UTC | #5
* Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 05/23/2011 01:38 PM, Ingo Molnar wrote:

> > Later on even this could be removed: using section tricks we can put init 
> > functions into a section
> 
> This is not kernel space, the C library provides a way to do that
> with __attribute__((constructor))...

Yeah, that would certainly work for simple things but there's several reasons 
why explicit control over initcalls is preferred in a tool like tools/kvm/ over 
__attribute__((constructor)):

 - C constructors run before main() so any generic environment that tools/kvm/
   might want to set up is not available yet - including but not limited to the
   command line arguments.

 - C constructors have random limitations like apparently not being executed if
   the constructor is linked into a .a static library.

 - It has advantages to have explicit control over initcalls - that way
   debugging and other instrumentation can be added freely. For example the
   kernel has a debug_initcall boot parameter to print the initcalls as they
   are executed. They can also be traced.

 - The kernel has several classes of initcalls with different call priority,
   i'm not aware of an equivalent __attribute__((constructor)) capability.
   We could also do more sophisticated layers of initcalls or an explicit
   initcall dependency tree, should the need arise.

Using .section tricks directly isnt particularly complex and the result is 
rather flexible, well-defined and visible.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Cyrill Gorcunov May 24, 2011, 8:51 a.m. UTC | #6
On 05/24/2011 12:37 PM, Paolo Bonzini wrote:
> On 05/23/2011 01:38 PM, Ingo Molnar wrote:
>> Later on even this could be removed: using section
>> tricks we can put init functions into a section
> 
> This is not kernel space, the C library provides a way to do that with __attribute__((constructor))...
> 
> Paolo

Ingo meant to use the same trick as we do in kernel space for late initcalls.
Paolo Bonzini May 24, 2011, 9:10 a.m. UTC | #7
On 05/24/2011 10:50 AM, Ingo Molnar wrote:
> Yeah, that would certainly work for simple things but there's several reasons
> why explicit control over initcalls is preferred in a tool like tools/kvm/ over
> __attribute__((constructor)):

Some advantages you mention are real indeed, but they are general; 
there's no reason why they apply only to tools/kvm.  You can achieve the 
same by doing only minimal work (registering your 
subsystems/devices/whatever in a linked list) in the constructors.  Then 
you iterate on the list and call function pointers.

I know portability is not relevant to tools/kvm/, but using unportable 
tricks for the sake of using them is a direct way to NIH.  But oh well 
all of tools/kvm/ is NIH after all. :)

>   - The kernel has several classes of initcalls with different call priority,
>     i'm not aware of an equivalent __attribute__((constructor)) capability.
>     We could also do more sophisticated layers of initcalls or an explicit
>     initcall dependency tree, should the need arise.

Constructors and destructors can have a priority (low runs first for 
constructors, low runs last for destructors).

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paolo Bonzini May 24, 2011, 9:18 a.m. UTC | #8
On 05/24/2011 10:50 AM, Ingo Molnar wrote:
>   - C constructors have random limitations like apparently not being executed if
>     the constructor is linked into a .a static library.

Ah, forgot about this.  Given _why_ this happens (for static libraries, 
the linker omits object modules that are not required to fulfill 
undefined references in previous objects), I'd be surprised if explicit 
section tricks do not have the same limitation.  After all the 
constructor attribute is only syntactic sugar for section tricks.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pekka Enberg May 24, 2011, 9:55 a.m. UTC | #9
On Tue, May 24, 2011 at 12:10 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> I know portability is not relevant to tools/kvm/, but using unportable
> tricks for the sake of using them is a direct way to NIH.  But oh well all
> of tools/kvm/ is NIH after all. :)

Hmm?

The point is to follow Linux kernel conventions and idioms (and share
code) as much as possible so it's familiar to devs who are already
working on the kernel. That's why section tricks seem more appropriate
than using constructor to me. Or is there some technical advantage to
using constructors?

                       Pekka
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity May 24, 2011, 11:22 a.m. UTC | #10
On 05/24/2011 12:55 PM, Pekka Enberg wrote:
> On Tue, May 24, 2011 at 12:10 PM, Paolo Bonzini<pbonzini@redhat.com>  wrote:
> >  I know portability is not relevant to tools/kvm/, but using unportable
> >  tricks for the sake of using them is a direct way to NIH.  But oh well all
> >  of tools/kvm/ is NIH after all. :)
>
> Hmm?
>
> The point is to follow Linux kernel conventions and idioms (and share
> code) as much as possible so it's familiar to devs who are already
> working on the kernel. That's why section tricks seem more appropriate
> than using constructor to me. Or is there some technical advantage to
> using constructors?

You get to reuse infrastructure that's already there.

Things like using sections and s/uint64_t/u64/ look anti-reuse to me.  
Userspace isn't the kernel, for better or for worse.
Pekka Enberg May 24, 2011, 11:26 a.m. UTC | #11
Hi Avi,

On Tue, May 24, 2011 at 2:22 PM, Avi Kivity <avi@redhat.com> wrote:
>> The point is to follow Linux kernel conventions and idioms (and share
>> code) as much as possible so it's familiar to devs who are already
>> working on the kernel. That's why section tricks seem more appropriate
>> than using constructor to me. Or is there some technical advantage to
>> using constructors?
>
> You get to reuse infrastructure that's already there.
>
> Things like using sections and s/uint64_t/u64/ look anti-reuse to me.
>  Userspace isn't the kernel, for better or for worse.

Not really. The type thing is pretty much required once you start
using kernel code (as we learned the hard way).

Btw, constructor attribute doesn't really seem like a good fit for
"late_initcall" type of thing:

  The constructor attribute causes the function to be called
automatically before execution enters main ()

What am I missing here?

                        Pekka
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sasha Levin May 24, 2011, 11:30 a.m. UTC | #12
On Tue, 2011-05-24 at 14:26 +0300, Pekka Enberg wrote:
> Hi Avi,
> 
> On Tue, May 24, 2011 at 2:22 PM, Avi Kivity <avi@redhat.com> wrote:
> >> The point is to follow Linux kernel conventions and idioms (and share
> >> code) as much as possible so it's familiar to devs who are already
> >> working on the kernel. That's why section tricks seem more appropriate
> >> than using constructor to me. Or is there some technical advantage to
> >> using constructors?
> >
> > You get to reuse infrastructure that's already there.
> >
> > Things like using sections and s/uint64_t/u64/ look anti-reuse to me.
> >  Userspace isn't the kernel, for better or for worse.
> 
> Not really. The type thing is pretty much required once you start
> using kernel code (as we learned the hard way).
> 
> Btw, constructor attribute doesn't really seem like a good fit for
> "late_initcall" type of thing:
> 
>   The constructor attribute causes the function to be called
> automatically before execution enters main ()

You could add a small constructor function that'll add a pointer to the
real initialization function to a list which will get called after we
get everything initialized and ready.
Avi Kivity May 24, 2011, 11:30 a.m. UTC | #13
On 05/24/2011 02:26 PM, Pekka Enberg wrote:
> Hi Avi,
>
> On Tue, May 24, 2011 at 2:22 PM, Avi Kivity<avi@redhat.com>  wrote:
> >>  The point is to follow Linux kernel conventions and idioms (and share
> >>  code) as much as possible so it's familiar to devs who are already
> >>  working on the kernel. That's why section tricks seem more appropriate
> >>  than using constructor to me. Or is there some technical advantage to
> >>  using constructors?
> >
> >  You get to reuse infrastructure that's already there.
> >
> >  Things like using sections and s/uint64_t/u64/ look anti-reuse to me.
> >    Userspace isn't the kernel, for better or for worse.
>
> Not really. The type thing is pretty much required once you start
> using kernel code (as we learned the hard way).

What happens when you start using userspace libraries?

Eventually you'll have a lot more of that than kernel code.

> Btw, constructor attribute doesn't really seem like a good fit for
> "late_initcall" type of thing:
>
>    The constructor attribute causes the function to be called
> automatically before execution enters main ()
>
> What am I missing here?

Like Paolo said, you can have the constructor register a function or 
structure to be called any time you like.
Pekka Enberg May 24, 2011, 11:38 a.m. UTC | #14
Hi Avi,

On Tue, May 24, 2011 at 2:30 PM, Avi Kivity <avi@redhat.com> wrote:
> What happens when you start using userspace libraries?
>
> Eventually you'll have a lot more of that than kernel code.

I don't quite understand what problems you think we might have. We're
already using userspace libraries (libvnc) and most of us code is
non-kernel code.

We switched to u64 and friends for two reasons: (1) using uint*_t
turned out to be painful when using kernel headers (e.g., mptables,
e820, etc.) and (2) we want to be as close as possible to the coding
style of tools/perf to be able to reuse their code in the future.

On Tue, May 24, 2011 at 2:30 PM, Avi Kivity <avi@redhat.com> wrote:
> Like Paolo said, you can have the constructor register a function or
> structure to be called any time you like.

Oh, okay. We should probably start out with that and switch to section
tricks if we have to.

                     Pekka
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity May 24, 2011, 11:41 a.m. UTC | #15
On 05/24/2011 02:38 PM, Pekka Enberg wrote:
> Hi Avi,
>
> On Tue, May 24, 2011 at 2:30 PM, Avi Kivity<avi@redhat.com>  wrote:
> >  What happens when you start using userspace libraries?
> >
> >  Eventually you'll have a lot more of that than kernel code.
>
> I don't quite understand what problems you think we might have. We're
> already using userspace libraries (libvnc) and most of us code is
> non-kernel code.

If uint64_t is defined differently than u64, you won't be able to pass 
it by reference if an external library expects it.

> We switched to u64 and friends for two reasons: (1) using uint*_t
> turned out to be painful when using kernel headers (e.g., mptables,
> e820, etc.) and (2) we want to be as close as possible to the coding
> style of tools/perf to be able to reuse their code in the future.
>

Regarding this reuse, I see it's done by copy/paste.  Won't it be better 
to have tools/lib and have tools/perf and tools/kvm use that?
Pekka Enberg May 24, 2011, 11:56 a.m. UTC | #16
Hi Avi,

On Tue, May 24, 2011 at 2:41 PM, Avi Kivity <avi@redhat.com> wrote:
>> I don't quite understand what problems you think we might have. We're
>> already using userspace libraries (libvnc) and most of us code is
>> non-kernel code.
>
> If uint64_t is defined differently than u64, you won't be able to pass it by
> reference if an external library expects it.

Oh, the sizes must be the same for obvious reasons so that shouldn't
be a problem and it's better to do the casting in the external API
calls than in core code.

On Tue, May 24, 2011 at 2:41 PM, Avi Kivity <avi@redhat.com> wrote:
> Regarding this reuse, I see it's done by copy/paste.  Won't it be better to
> have tools/lib and have tools/perf and tools/kvm use that?

Yes, absolutely. We're keeping things well-contained under tools/kvm
with copy-paste until we hit Linus' tree in the future, though, for
merge reasons.

                       Pekka
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paolo Bonzini May 24, 2011, 12:27 p.m. UTC | #17
On 05/24/2011 01:56 PM, Pekka Enberg wrote:
>> >  If uint64_t is defined differently than u64, you won't be able to pass it by
>> >  reference if an external library expects it.
>
> Oh, the sizes must be the same for obvious reasons so that shouldn't
> be a problem and it's better to do the casting in the external API
> calls than in core code.

Be careful about unsigned long vs. unsigned long long.  If you want to 
use the u64 spelling that's fine, but I would use a custom header that 
defines it from uint64_t.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity May 24, 2011, 2:37 p.m. UTC | #18
On 05/24/2011 02:41 PM, Avi Kivity wrote:
>
>> We switched to u64 and friends for two reasons: (1) using uint*_t
>> turned out to be painful when using kernel headers (e.g., mptables,
>> e820, etc.) and (2) we want to be as close as possible to the coding
>> style of tools/perf to be able to reuse their code in the future.
>>
>
> Regarding this reuse, I see it's done by copy/paste.  Won't it be 
> better to have tools/lib and have tools/perf and tools/kvm use that?
>

Another thing, I believe reuse of actual kernel code has limited 
utility.  The kernel has vast infrastructure and porting all of it to 
userspace would be a huge undertaking.  It's pretty common for code to 
use printk(), kmalloc(), slab caches, rcu, various locks, per-cpu 
variables.  Are you going to port all of it?  and maintain it when it 
changes?

It's a lot easier to use the native userspace stacks.
Avi Kivity May 24, 2011, 2:38 p.m. UTC | #19
On 05/24/2011 03:27 PM, Paolo Bonzini wrote:
> On 05/24/2011 01:56 PM, Pekka Enberg wrote:
>>> >  If uint64_t is defined differently than u64, you won't be able to 
>>> pass it by
>>> >  reference if an external library expects it.
>>
>> Oh, the sizes must be the same for obvious reasons so that shouldn't
>> be a problem and it's better to do the casting in the external API
>> calls than in core code.
>
> Be careful about unsigned long vs. unsigned long long. 

Yes, that's the issue I had in mind.

> If you want to use the u64 spelling that's fine, but I would use a 
> custom header that defines it from uint64_t.

And now you'll have printf/printk format mismatches if they happen to be 
different.
Pekka Enberg May 24, 2011, 2:54 p.m. UTC | #20
Hi Avi,

On Tue, May 24, 2011 at 5:37 PM, Avi Kivity <avi@redhat.com> wrote:
> Another thing, I believe reuse of actual kernel code has limited utility.

OK, I don't agree with that.

On Tue, May 24, 2011 at 5:37 PM, Avi Kivity <avi@redhat.com> wrote:
>  The kernel has vast infrastructure and porting all of it to userspace would
> be a huge undertaking.  It's pretty common for code to use printk(),
> kmalloc(), slab caches, rcu, various locks, per-cpu variables.  Are you
> going to port all of it?  and maintain it when it changes?

We will port whatever is useful and makes sense. Nothing new here -
we're following perf's lead, that's all.

As for the specific issues you mention, only RCU and locking
mechanisms seem like something that are not trivial to port (although
userspace RCU already exists).

[ Porting lockdep to userspace would be one cool feature, though. ]

On Tue, May 24, 2011 at 5:37 PM, Avi Kivity <avi@redhat.com> wrote:
> It's a lot easier to use the native userspace stacks.

We will be using userspace libs where appropriate (like with libvnc).

                        Pekka
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ingo Molnar May 24, 2011, 7 p.m. UTC | #21
* Pekka Enberg <penberg@kernel.org> wrote:

> We switched to u64 and friends for two reasons: (1) using uint*_t turned out 
> to be painful when using kernel headers (e.g., mptables, e820, etc.) and (2) 
> we want to be as close as possible to the coding style of tools/perf to be 
> able to reuse their code in the future.

3)

uint*_t caused frequent type mismatches and breakages with format strings if 
accidentally an uint*_t was stuck into a regular printf format - it would 
trigger a warning only on 64-bit or only on 32-bit systems, causing frequent 
flux. This is not a very good data type model.

4)

uint*_t also causes absolute brain-damaged printf format hacks like:

                fprintf(stderr, "%s: %" PRIu64 " %" PRIu64 " %" PRIu64 "\n",
                        event_name(counter), count[0], count[1], count[2]);

instead of the much cleaner:

                fprintf(stderr, "%s: %Lu %Lu %Lu\n",
                        event_name(counter), count[0], count[1], count[2]);

So we can use uint*_t where absolutely necessary due to external ABI 
constraints, but otherwise it's discouraged for tools/kvm/ internal code.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ingo Molnar May 24, 2011, 7:03 p.m. UTC | #22
* Pekka Enberg <penberg@kernel.org> wrote:

> As for the specific issues you mention, only RCU and locking mechanisms seem 
> like something that are not trivial to port (although userspace RCU already 
> exists).

Indeed.

> [ Porting lockdep to userspace would be one cool feature, though. ]

Agreed - we already ran into a few locking bugs that were IMO needlessly 
difficult to debug - our expectations got spoiled by lockdep i guess! :-)

Porting RCU would also have its distinct advantages in terms of scalability. 
While tools/perf/ probably wont need it (all its scalability-relevant code runs 
in the kernel), tools/kvm/ definitely looks like a candidate for good 
user-space RCU primitives.

> On Tue, May 24, 2011 at 5:37 PM, Avi Kivity <avi@redhat.com> wrote:
> > It's a lot easier to use the native userspace stacks.
> 
> We will be using userspace libs where appropriate (like with libvnc).

We are using a fair amount of userspace libraries in perf too: -ldw, -lelf, 
-lnewt, -lslang, we used -laudit too for some time.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ingo Molnar May 24, 2011, 7:16 p.m. UTC | #23
* Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 05/24/2011 10:50 AM, Ingo Molnar wrote:
> >Yeah, that would certainly work for simple things but there's several reasons
> >why explicit control over initcalls is preferred in a tool like tools/kvm/ over
> >__attribute__((constructor)):
> 
> Some advantages you mention are real indeed, but they are general;
> there's no reason why they apply only to tools/kvm.  You can achieve
> the same by doing only minimal work (registering your
> subsystems/devices/whatever in a linked list) in the constructors.
> Then you iterate on the list and call function pointers.

Well, the plain fact that __attribute__((constructor)) gets called on binary 
and shared library startup before main() is called is a show-stopper for us as 
i described it in my previous mail, so why are we even arguing about it?

((constructor)) has showstopper properties:

 - We don't have access to the program arguments

 - stdio is probably not set up yet (this is undefined AFAICS)

 - Also, over the years i have grown to be suspicious of GCC defined 
   extensions. More often than not the GCC project is fixing regressions not by 
   fixing the compiler but by changing the documentation ;-) We got bitten by 
   regressions in asm() behavior in the kernel rather often.

   In that sense ((section)) is way more robust: there's not really that many 
   ways to screw that up. Fiddling with the ((constructor)) environment on the 
   other hand ...

Note that in terms of explicit iterations we do that with 
__attribute__((section)) as well: except that *we* define where and how the 
functions get called.

> I know portability is not relevant to tools/kvm/, but using unportable tricks 
> for the sake of using them is a direct way to NIH. But oh well all of 
> tools/kvm/ is NIH after all. :)

__attribute__((constructor)) is not particularly portable to begin with: does 
the MSVC compiler support it for example?

So __attribute__ ((section)), which is used by the initcall() machinery is 
similarly portable: GCC and LLVM/Clang support it.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ingo Molnar May 24, 2011, 7:40 p.m. UTC | #24
* Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 05/24/2011 10:50 AM, Ingo Molnar wrote:
> >  - C constructors have random limitations like apparently not being executed if
> >    the constructor is linked into a .a static library.
> 
> Ah, forgot about this.  Given _why_ this happens (for static libraries, the 
> linker omits object modules that are not required to fulfill undefined 
> references in previous objects), I'd be surprised if explicit section tricks 
> do not have the same limitation.  [...]

Since we create an actual array (data) and iterate it the worst i can imagine 
is the linker dropping those sections - in which case the build will fail and 
we are alerted to the problem.

With ((constructor)) the program will misbehave silently - not good.

> [...] After all the constructor attribute is only syntactic sugar for section 
> tricks.

Yeah but not just syntactic sugar but also code built into glibc to execute 
them at some point before main().

And this last bit really matters as well.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paolo Bonzini May 25, 2011, 8:21 a.m. UTC | #25
On 05/24/2011 09:40 PM, Ingo Molnar wrote:
> >  Ah, forgot about this.  Given_why_  this happens (for static libraries, the
> >  linker omits object modules that are not required to fulfill undefined
> >  references in previous objects), I'd be surprised if explicit section tricks
> >  do not have the same limitation.  [...]
>
> Since we create an actual array (data)

Are you?  I figured it was like this example from Linux:

         .x86_cpu_dev.init : AT(ADDR(.x86_cpu_dev.init) - LOAD_OFFSET) {
                 __x86_cpu_dev_start = .;
                 *(.x86_cpu_dev.init)
                 __x86_cpu_dev_end = .;
         }


   extern const struct cpu_dev *const __x86_cpu_dev_start[],
                               *const __x86_cpu_dev_end[];

   for (cdev = __x86_cpu_dev_start; cdev < __x86_cpu_dev_end; cdev++) {

This is pretty much the same as the linker script for ELF:

   .init_array     : {
      __init_array_start = .;
      *(.init_array)
      *(SORT(.init_array$*))
      __init_array_end = .;
   }

   extern void (*__init_array_start []) (int, char **, char **)
     attribute_hidden;
   extern void (*__init_array_end []) (int, char **, char **)
     attribute_hidden;

   const size_t size = __init_array_end - __init_array_start;
   for (size_t i = 0; i < size; i++)
       (*__init_array_start [i]) (argc, argv, envp);


> ((constructor)) has showstopper properties:
>
>  - We don't have access to the program arguments
>
>  - stdio is probably not set up yet (this is undefined AFAICS)

As I said, you can do this even better by doing only minimal work in the 
constructor.  Create a struct with several callbacks (pre_init, 
late_init, destroy, reset, whatever) and possibly other information (a 
human-readable device name and command-line argument to access it, for 
example).  In the constructor you just build a linked list of said 
structs, and then you can walk it whenever you see fit: do comparisons, 
call a function, whatever.  This is similar to the cpudev example from 
the kernel above.

A simple example from QEMU:

   static void virtio_pci_register_devices(void)
   {
       pci_qdev_register_many(virtio_info);
   }

   /* This macro wraps ((constructor)).  */
   device_init(virtio_pci_register_devices)

>    In that sense ((section)) is way more robust: there's not really that many
>    ways to screw that up. Fiddling with the ((constructor)) environment on the
>    other hand ...

Sorry, this is textbook FUD.

> __attribute__((constructor)) is not particularly portable to begin with: does
> the MSVC compiler support it for example?

No, but GCC supports it on non-ELF platforms, where you would need a 
similar but different linker script.  (Also, the differences between 
MSVC and GCC can easily be abstracted with a macro).

More practically, is your linker script supported by gold?

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ingo Molnar May 25, 2011, 8:32 a.m. UTC | #26
* Paolo Bonzini <pbonzini@redhat.com> wrote:

> >   In that sense ((section)) is way more robust: there's not really that many
> >   ways to screw that up. Fiddling with the ((constructor)) environment on the
> >   other hand ...
> 
> Sorry, this is textbook FUD.

I specifically cited the problem of static libraries. They *do not work* with 
((constructor)).

That's not FUD but justified scepticism backed by hard evidence.

> > __attribute__((constructor)) is not particularly portable to begin with: 
> > does the MSVC compiler support it for example?
> 
> No, but GCC supports it on non-ELF platforms, [...]

So you claim that ((section)) is 'not portable' but concede that it does in 
fact not port to one of the most widely used compilers.

Instead you clarify that under 'not portable' you really meant to say that 
((section)) does not work on non-ELF binary platforms.

That is a pretty inconsistent position and has nothing to do with 'portability' 
in itself but with being portable to what *you* consider important.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ingo Molnar May 25, 2011, 8:38 a.m. UTC | #27
* Paolo Bonzini <pbonzini@redhat.com> wrote:

> > ((constructor)) has showstopper properties:
> >
> > - We don't have access to the program arguments
> >
> > - stdio is probably not set up yet (this is undefined AFAICS)
> 
> As I said, you can do this even better by doing only minimal work in
> the constructor. [...]

The costs of doing that:

  - extra per init entry data structures

  - extra code being run during the constructor phase

  - the risk of not getting the ((constructor)) init run at all,
    such as on static libraries

The only benefit is:

  - ((constructor)) is a non-portable GCC extension so the only portability
    win is to possibly support non-ELF targets. Which ones do we care about?

At this point it is a rather legitimate technical decision for us to say that 
we do not want to bear those cost for that limited benefit, wouldn't you agree?

If someone adds non-ELF support then it can all be switched over to the bit 
more bloated ((constructor)) approach rather easily - so this all is a 
non-issue really.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paolo Bonzini May 25, 2011, 9:15 a.m. UTC | #28
On 05/25/2011 10:32 AM, Ingo Molnar wrote:
>
> * Paolo Bonzini<pbonzini@redhat.com>  wrote:
>
>>>    In that sense ((section)) is way more robust: there's not really that many
>>>    ways to screw that up. Fiddling with the ((constructor)) environment on the
>>>    other hand ...
>>
>> Sorry, this is textbook FUD.
>
> I specifically cited the problem of static libraries. They *do not work* with
> ((constructor)).

((constructor)) has easily explained semantics: it's the same semantics 
as C++ global constructors.  If you don't like those, that's another story.

But anyway, I specifically cited the reason why ((section)) is not any 
better, AFAIU how you would use it.  Now I didn't trust myself and tried it.

f.ld:
	SECTIONS {
		.paolo : {
		        PROVIDE(__paolo_start = .);
		        *(.paolo)
		        PROVIDE(__paolo_end = .);
		}
	}

f.c:
	int x __attribute__((section(".paolo"))) = 0x12345678;

	int main()
	{
		extern int __paolo_start[];
		extern int __paolo_end[];
	
		int *p;
		for (p = __paolo_start; p < __paolo_end; p++) {
			printf ("%x\n", *p);
		}
	}

f1.c:
	int y __attribute__((section(".paolo"))) = 0x76543210;

compilation with objects:
	$ gcc f.c -o f.o -c
	$ gcc f1.c -o f1.o -c
	$ ld -r f.ld f.o f1.o -o f2.o
	$ gcc f2.o -o a.out
	$ ./a.out
	12345678
	76543210

compilation with static libraries:
	$ gcc f.c -o f.o -c
	$ gcc f1.c -o f1.o -c
	$ ar cr f1.a f1.o
	$ ld -r f.ld f.o f1.a -o f2.o
	$ gcc f2.o -o a.out
	$ ./a.out
	12345678

Since this is userspace, you do not have complete control on the build 
and you still need to use gcc to drive the final link.

So you get exactly the same semantics as ((constructor)), plus one extra 
build step that spits out a warning ("ld: warning: f.ld contains output 
sections; did you forget -T?").

>>> __attribute__((constructor)) is not particularly portable to begin with:
>>> does the MSVC compiler support it for example?
>>
>> No, but GCC supports it on non-ELF platforms, [...]
>
> So you claim that ((section)) is 'not portable' but concede that it does in
> fact not port to one of the most widely used compilers.

Can you please avoid trimming messages?  I said "the differences between 
MSVC and GCC can easily be abstracted with a macro".  This is not unlike 
other differences between the two compilers (e.g. __forceinline vs. the 
((always_inline)) attribute), and it is not true of linker scripts.

> Instead you clarify that under 'not portable' you really meant to say that
> ((section)) does not work on non-ELF binary platforms.
>
> That is a pretty inconsistent position and has nothing to do with 'portability'
> in itself but with being portable to what *you* consider important.

My definition of "(reasonably) portable" includes having to write a 
5-line macro that works across _all_ GCC targets and _all_ MSVC targets.

Having to change the build process for every compiler target (see the 
above "ld -r" step) is a bit more unportable, though not much. 
Potentially having to modify the custom linker script for every targeted 
architecture (the above doesn't work for mingw32, it needs an extra _ in 
front of the start/end symbols) is pretty much my definition of 
"unportable".

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ingo Molnar May 25, 2011, 9:36 a.m. UTC | #29
* Paolo Bonzini <pbonzini@redhat.com> wrote:

> > So you claim that ((section)) is 'not portable' but concede that 
> > it does in fact not port to one of the most widely used 
> > compilers.
> 
> Can you please avoid trimming messages?  I said "the differences 
> between MSVC and GCC can easily be abstracted with a macro".  This 
> is not unlike other differences between the two compilers (e.g. 
> __forceinline vs. the ((always_inline)) attribute), and it is not 
> true of linker scripts.

So ((constructor)) is unportable but it can be wrapped. So can other 
portability problems be solved, such as linker scripts can be written 
for non-ELF targets (should that ever become necessary), so what's 
your point?

We want to use ELF sections in the future *anyway* to make tools/kvm/ 
scale even better, for example to use alternative instruction fixups 
(for which no GCC extension exists) so a linker script will be there 
anyway. ( We might also want to use sections to speed up exception 
handling in tools/kvm/. )

Fact is that neither ((section)) *NOR* ((constructor)) is portable: 
*both* are GCC extensions - while you falsely claimed that 
((constructor)) was somehow portable and that ((section)) was an 
'unportable trick'. Let me quote your initial claim:

>>>>> I know portability is not relevant to tools/kvm/, but using 
>>>>> unportable tricks for the sake of using them is a direct way to 
>>>>> NIH. But oh well all of tools/kvm/ is NIH after all. :)

Btw., that NIH claim was rather unfair and uncalled for as well.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ingo Molnar May 25, 2011, 9:49 a.m. UTC | #30
* Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 05/25/2011 10:32 AM, Ingo Molnar wrote:
> >
> >* Paolo Bonzini<pbonzini@redhat.com>  wrote:
> >
> >>>   In that sense ((section)) is way more robust: there's not really that many
> >>>   ways to screw that up. Fiddling with the ((constructor)) environment on the
> >>>   other hand ...
> >>
> >> Sorry, this is textbook FUD.
> >
> > I specifically cited the problem of static libraries. They *do 
> > not work* with ((constructor)).
> 
> ((constructor)) has easily explained semantics: it's the same 
> semantics as C++ global constructors.  If you don't like those, 
> that's another story.

It is a plain *bug* if device initialization is not being executed if 
kvm is linked into a static library using the regular way of how 
libraries are created ...

No amount of arguing about 'semantics' will change that simple fact: 
it breaks code so it's a bug. We don't want to rely on a facility 
that handles boundary conditions in such a poor way.

See my prior point:

 - Also, over the years i have grown to be suspicious of GCC defined
   extensions. More often than not the GCC project is fixing regressions not by
   fixing the compiler but by changing the documentation ;-) We got bitten by
   regressions in asm() behavior in the kernel rather often.

   In that sense ((section)) is way more robust: there's not really that many
   ways to screw that up. Fiddling with the ((constructor)) environment on the
   other hand ...

You are demonstrating this phenomenon rather well. You argue against 
plain old bugs with 'but these are well-defined semantics'.

That's not how we deal with bugs in tools/kvm/ really.

And then you argue that the bug can be worked around by writing a 
linker script:

 > compilation with static libraries:
 >       $ gcc f.c -o f.o -c
 >       $ gcc f1.c -o f1.o -c
 >       $ ar cr f1.a f1.o
 >       $ ld -r f.ld f.o f1.a -o f2.o
 >       $ gcc f2.o -o a.out
 >       $ ./a.out
 >       12345678

I will rather use linker scripts and less erratic facilities straight 
away.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paolo Bonzini May 25, 2011, 10:01 a.m. UTC | #31
On 05/25/2011 11:36 AM, Ingo Molnar wrote:
> So can other portability problems be solved, such as linker scripts
> can be written for non-ELF targets (should that ever become
> necessary)

I know that's not going to be the case, and I mentioned that in my first 
message---portability is not relevant to tools/kvm/ just like it is not 
relevant say to systemd.

> so what's your point?

Using this kind of trick makes it harder to share code with other 
libraries that may require a higher standard of portability (not 
"better" or "worse", just "higher").  QEMU _does_ have this problem BTW, 
we have our own event loop and AIO, our own JSON parser, our own thread 
abstractions, and so on...

> We want to use ELF sections in the future *anyway* to make
> tools/kvm/ scale even better, for example to use alternative
> instruction fixups (for which no GCC extension exists) so a linker
> script will be there anyway.

Since you're not writing the hypervisor, but only the device model, I
doubt that this is needed (and you'd likely run into troubles with
SELinux).  QEMU scales to dozens of VCPUs with a big lock around all 
userspace device model code.  But it's premature to say that, and I'd be 
happy if I were proven wrong.

> Fact is that neither ((section)) *NOR* ((constructor)) is portable:
> *both* are GCC extensions - while you falsely claimed that
> ((constructor)) was somehow portable and that ((section)) was an
> 'unportable trick'.

Fair enough.  I agree that ((constructor)) has limited scope and is 
compiler-unportable.  ((section)) is more powerful and has the cost of 
using also target-unportable linker scripts, and requiring changes to 
the build system.  Both have the same problem with static libraries 
(please reread my message).

>>>>>> I know portability is not relevant to tools/kvm/, but
>>>>>> using unportable tricks for the sake of using them is a
>>>>>> direct way to NIH. But oh well all of tools/kvm/ is NIH
>>>>>> after all. :)
>
> Btw., that NIH claim was rather unfair and uncalled for as well.

Hey hey I put a smiley for a reason!

Anyway I think we both agree that this debate is pointless.  I learnt 
something (I wasn't aware of interaction between ((constructor)) and 
static libraries), you learnt something (it's the same with ((section)), 
and it's intrinsic in how static libraries work).

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ingo Molnar May 25, 2011, 10:17 a.m. UTC | #32
* Paolo Bonzini <pbonzini@redhat.com> wrote:

> > so what's your point?
> 
> Using this kind of trick makes it harder to share code with other 
> libraries that may require a higher standard of portability (not 
> "better" or "worse", just "higher"). [...]

That's an complication but should be fixable, should it ever happen.

As things stand today we:

 - Are *already* using an ELF linker script, see tools/kvm/bios/rom.ld.S

 - Have multiple valid reasons not to use ((constructor))

 - Want to use sections to implement other useful features as well

If the *only* linker script use would be the init facility then you'd 
probably have a valid point - although the possible code flow 
fragility with ((constructor)) is still a problem: we still would 
want to know when no constructors were executed.

Also it's not clear why ((constructor)) was written in the way it 
was: why apparently no access is given to the array of init functions 
and why it's not possible to turn the auto-execution off but still 
have the array generated, for legitimate cases that want to use data 
driven constructor execution.

> >>>>>> I know portability is not relevant to tools/kvm/, but using 
> >>>>>> unportable tricks for the sake of using them is a direct way 
> >>>>>> to NIH. But oh well all of tools/kvm/ is NIH after all. :)
> >
> > Btw., that NIH claim was rather unfair and uncalled for as well.
> 
> Hey hey I put a smiley for a reason!

Well after two insults in a single paragraph you need to put in at 
least two smileys! Or not write the insults in a technical discssion 
to begin with, especially if you are criticising a patch rather 
forcefully. It will be easily misunderstood as a real insult, despite 
the smiley ;-)

> Anyway I think we both agree that this debate is pointless.  I 
> learnt something (I wasn't aware of interaction between 
> ((constructor)) and static libraries), you learnt something (it's 
> the same with ((section)), and it's intrinsic in how static 
> libraries work).

While i did not know whether static libraries would work with a 
linker script (never tried it - and your experiment suggests that 
they wont), the ((section)) approach we could create a clear runtime 
BUG_ON() assert for a zero-sized array of init function pointers, 
while ((constructor)) will silently not execute initialization 
functions.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paolo Bonzini May 25, 2011, 10:44 a.m. UTC | #33
On 05/25/2011 12:17 PM, Ingo Molnar wrote:
> Also it's not clear why ((constructor)) was written in the way it
> was: why apparently no access is given to the array of init functions

It is accessible---glibc uses it.

> and why it's not possible to turn the auto-execution off but still
> have the array generated, for legitimate cases that want to use data
> driven constructor execution.

The compiler doesn't care about what is done with the data.  It simply 
provides the table for the runtime library to use it.  ((constructor)) 
is a veneer over the same infrastructure used for C++ global 
constructors; that explains its design pretty well.

> the ((section)) approach we could create a clear runtime
> BUG_ON() assert for a zero-sized array of init function pointers,

Not really.  The problem is that f1.o is not pulled from the static 
library _because the linker thinks it is not necessary_.  If f1.o 
defines another symbol, and f.o uses it, then the constructor is pulled 
in together with the rest of f.o.  As soon as _one_ constructor is 
pulled in (even from outside the static library), your BUG() would not 
detect silently missing imports anymore.

It turns out a way to get the semantics you want for ((section)) is to 
use --whole-archive (and possibly --no-whole-archive) on the "ld -r" 
command line.  You can do the same on the link line for ((constructor)) 
or C++ constructors too.  However, since you're effectively using a C++ 
feature, static libraries are probably a bad idea just like C++ static 
libraries are.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ingo Molnar May 25, 2011, 12:53 p.m. UTC | #34
* Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 05/25/2011 12:17 PM, Ingo Molnar wrote:
>
> > Also it's not clear why ((constructor)) was written in the way it 
> > was: why apparently no access is given to the array of init 
> > functions
> 
> It is accessible---glibc uses it.
> 
> > and why it's not possible to turn the auto-execution off but 
> > still have the array generated, for legitimate cases that want to 
> > use data driven constructor execution.
> 
> The compiler doesn't care about what is done with the data.  It 
> simply provides the table for the runtime library to use it. 
> ((constructor)) is a veneer over the same infrastructure used for 
> C++ global constructors; that explains its design pretty well.

Obviously the compiler did not provide this feature into a vacuum, it 
expected the C library to execute constructors, right?

So the above dodges my question of why there is no method (in glibc) 
to turn auto-execution off and let the app do it.

If all that was possible then ((constructor)) could certainly be used 
as a generalized shortcut for certain ((section)) uses.

> > the ((section)) approach we could create a clear runtime BUG_ON() 
> > assert for a zero-sized array of init function pointers,
> 
> Not really.  The problem is that f1.o is not pulled from the static 
> library _because the linker thinks it is not necessary_.  If f1.o 
> defines another symbol, and f.o uses it, then the constructor is 
> pulled in together with the rest of f.o.  As soon as _one_ 
> constructor is pulled in (even from outside the static library), 
> your BUG() would not detect silently missing imports anymore.

Yeah, good point.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paolo Bonzini May 25, 2011, 3:37 p.m. UTC | #35
On 05/25/2011 02:53 PM, Ingo Molnar wrote:
> Obviously the compiler did not provide this feature into a vacuum, it
> expected the C library to execute constructors, right?
>
> So the above dodges my question of why there is no method (in glibc)
> to turn auto-execution off and let the app do it.

Well, I wasn't there. :)

I suppose they simply chose those semantics for C++ constructors which 
were easiest to implement (i.e. execute before main).  ((constructor)) 
then followed suit.

Any further extension in the direction you suggest would have to be 
designed very carefully in order to coexist with C++ constructors, or it 
would have to be something completely separate... in the latter case, 
((constructor)) already provides the ingredients to do this, even if at 
a small cost in efficiency.  So there is no big incentive to add this 
further extension to the compiler and/or the C library.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/tools/kvm/kvm-run.c b/tools/kvm/kvm-run.c
index 288e1fb..adbb25b 100644
--- a/tools/kvm/kvm-run.c
+++ b/tools/kvm/kvm-run.c
@@ -28,6 +28,7 @@ 
 #include <kvm/barrier.h>
 #include <kvm/symbol.h>
 #include <kvm/virtio-9p.h>
+#include <kvm/vesa.h>
 
 /* header files for gitish interface  */
 #include <kvm/kvm-run.h>
@@ -66,6 +67,7 @@  static const char *virtio_9p_dir;
 static bool single_step;
 static bool readonly_image[MAX_DISK_IMAGES];
 static bool virtio_rng;
+static bool vnc;
 extern bool ioport_debug;
 extern int  active_console;
 
@@ -110,6 +112,7 @@  static const struct option options[] = {
 	OPT_STRING('\0', "kvm-dev", &kvm_dev, "kvm-dev", "KVM device file"),
 	OPT_STRING('\0', "virtio-9p", &virtio_9p_dir, "root dir",
 			"Enable 9p over virtio"),
+	OPT_BOOLEAN('\0', "vnc", &vnc, "Enable VNC framebuffer"),
 
 	OPT_GROUP("Kernel options:"),
 	OPT_STRING('k', "kernel", &kernel_filename, "kernel",
@@ -413,6 +416,7 @@  int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 	char *hi;
 	int i;
 	void *ret;
+	u16 vidmode = 0;
 
 	signal(SIGALRM, handle_sigalrm);
 	signal(SIGQUIT, handle_sigquit);
@@ -511,7 +515,13 @@  int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 	kvm->nrcpus = nrcpus;
 
 	memset(real_cmdline, 0, sizeof(real_cmdline));
-	strcpy(real_cmdline, "notsc noapic noacpi pci=conf1 console=ttyS0 earlyprintk=serial");
+	strcpy(real_cmdline, "notsc noapic noacpi pci=conf1");
+	if (vnc) {
+		strcat(real_cmdline, " video=vesafb console=tty0");
+		vidmode = 0x312;
+	} else {
+		strcat(real_cmdline, " console=ttyS0 earlyprintk=serial");
+	}
 	strcat(real_cmdline, " ");
 	if (kernel_cmdline)
 		strlcat(real_cmdline, kernel_cmdline, sizeof(real_cmdline));
@@ -543,7 +553,7 @@  int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 	printf("  # kvm run -k %s -m %Lu -c %d\n", kernel_filename, ram_size / 1024 / 1024, nrcpus);
 
 	if (!kvm__load_kernel(kvm, kernel_filename, initrd_filename,
-				real_cmdline))
+				real_cmdline, vidmode))
 		die("unable to load kernel %s", kernel_filename);
 
 	kvm->vmlinux		= vmlinux_filename;
@@ -597,6 +607,9 @@  int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 
 	kvm__init_ram(kvm);
 
+	if (vnc)
+		vesa__init(kvm);
+
 	thread_pool__init(nr_online_cpus);
 
 	for (i = 0; i < nrcpus; i++) {