Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available

2013-02-18 Thread Igor Mammedov
On Thu, 14 Feb 2013 11:51:59 -0200
Eduardo Habkost  wrote:

> On Thu, Feb 14, 2013 at 02:31:38PM +0100, Paolo Bonzini wrote:
> > Il 14/02/2013 14:24, Eduardo Habkost ha scritto:
> > > On Thu, Feb 14, 2013 at 01:13:18PM +0100, Paolo Bonzini wrote:
> > >> Il 14/02/2013 12:18, Eduardo Habkost ha scritto:
> > > qemu boots from disk image 3 times faster than direct kernel load.
> > >>> That's surprising. Do you have any idea why that happens?
> > >>
> > >> Because kernel load uses MMIO (from fw_cfg), while booting from disk
> > >> uses at worst PCI DMA and at best virtio.
> > > 
> > > Is it something worth trying to optimize
> > 
> > I think that, within the limits of what the spec makes legal, Gleb
> > optimized all that he could out of it.  The alternative is to make
> > fw_cfg do DMA, which in the past was rejected because it doesn't look
> > like what real ISA hardware would do.
> > 
> > > , or a reasonable solution would
> > > be so similar to having a disk+bootloader that's easier to simply
> > > recommend people to set up a real disk with a real bootloader if they
> > > care about speed?
> > 
> > In the end it's a pity, but yeah that's the easiest thing to do with
> > distro kernels and big all-drivers initrd.  -kernel is still useful and
> > fast enough if you have a custom-built kernel, possibly with no initrd
> > at all.
> 
> The patch that originated this thread wasn't even for distro kernels and
> big initrds. Our test case lods a very small test kernel (11 KB), and it
> is taking almost 15 seconds to boot.
> 
> Maybe our test case should create a custom BIOS image to be loaded on
> ROM, instead?

Problem was that gcc generated 2 LOAD sections with a hole ~128Mb between
them. And current implementation of ELF multiboot loader in QEMU just
allocates flat buffer of size=highest-lowest loaded address from ELF which
includes possible holes.

Possible fix is to exclude holes and pass a packed buffer to seabios, which
would require fixing multiboot rom as well (not sure if it could be made
compatible with migration to old qemu/seabios).



this hole.




Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available

2013-02-14 Thread Lucas Meneghel Rodrigues

On 02/14/2013 05:57 PM, Eduardo Habkost wrote:

On Thu, Feb 14, 2013 at 08:28:03PM +0100, Igor Mammedov wrote:

On Thu, 14 Feb 2013 13:13:18 +0100
Paolo Bonzini  wrote:


Il 14/02/2013 12:18, Eduardo Habkost ha scritto:

qemu boots from disk image 3 times faster than direct kernel load.

That's surprising. Do you have any idea why that happens?


Because kernel load uses MMIO (from fw_cfg), while booting from disk
uses at worst PCI DMA and at best virtio.

Paolo

I've tried Paolo's suggestion to run in TCG mode with instruction logging,
and it spends too much time reading 11K kernel from qemu compared to
kvm-unit-test kernel. I'll need debug it further to find out why.
But this patch looks unnecessary if I could fix qemu/seabios or test kernel
to reach the same load time as kvm-unit-test kernel.


Agreed.

Lucas, please don't apply this patch yet, until we find the actual cause
of the slow booting.


Okey dokey.




Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available

2013-02-14 Thread Eduardo Habkost
On Thu, Feb 14, 2013 at 08:28:03PM +0100, Igor Mammedov wrote:
> On Thu, 14 Feb 2013 13:13:18 +0100
> Paolo Bonzini  wrote:
> 
> > Il 14/02/2013 12:18, Eduardo Habkost ha scritto:
> > >> > qemu boots from disk image 3 times faster than direct kernel load.
> > > That's surprising. Do you have any idea why that happens?
> > 
> > Because kernel load uses MMIO (from fw_cfg), while booting from disk
> > uses at worst PCI DMA and at best virtio.
> > 
> > Paolo
> I've tried Paolo's suggestion to run in TCG mode with instruction logging,
> and it spends too much time reading 11K kernel from qemu compared to
> kvm-unit-test kernel. I'll need debug it further to find out why.
> But this patch looks unnecessary if I could fix qemu/seabios or test kernel
> to reach the same load time as kvm-unit-test kernel.

Agreed.

Lucas, please don't apply this patch yet, until we find the actual cause
of the slow booting.

-- 
Eduardo



Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available

2013-02-14 Thread Igor Mammedov
On Thu, 14 Feb 2013 13:13:18 +0100
Paolo Bonzini  wrote:

> Il 14/02/2013 12:18, Eduardo Habkost ha scritto:
> >> > qemu boots from disk image 3 times faster than direct kernel load.
> > That's surprising. Do you have any idea why that happens?
> 
> Because kernel load uses MMIO (from fw_cfg), while booting from disk
> uses at worst PCI DMA and at best virtio.
> 
> Paolo
I've tried Paolo's suggestion to run in TCG mode with instruction logging,
and it spends too much time reading 11K kernel from qemu compared to
kvm-unit-test kernel. I'll need debug it further to find out why.
But this patch looks unnecessary if I could fix qemu/seabios or test kernel
to reach the same load time as kvm-unit-test kernel.

-- 
Regards,
  Igor



Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available

2013-02-14 Thread Eduardo Habkost
On Thu, Feb 14, 2013 at 02:31:38PM +0100, Paolo Bonzini wrote:
> Il 14/02/2013 14:24, Eduardo Habkost ha scritto:
> > On Thu, Feb 14, 2013 at 01:13:18PM +0100, Paolo Bonzini wrote:
> >> Il 14/02/2013 12:18, Eduardo Habkost ha scritto:
> > qemu boots from disk image 3 times faster than direct kernel load.
> >>> That's surprising. Do you have any idea why that happens?
> >>
> >> Because kernel load uses MMIO (from fw_cfg), while booting from disk
> >> uses at worst PCI DMA and at best virtio.
> > 
> > Is it something worth trying to optimize
> 
> I think that, within the limits of what the spec makes legal, Gleb
> optimized all that he could out of it.  The alternative is to make
> fw_cfg do DMA, which in the past was rejected because it doesn't look
> like what real ISA hardware would do.
> 
> > , or a reasonable solution would
> > be so similar to having a disk+bootloader that's easier to simply
> > recommend people to set up a real disk with a real bootloader if they
> > care about speed?
> 
> In the end it's a pity, but yeah that's the easiest thing to do with
> distro kernels and big all-drivers initrd.  -kernel is still useful and
> fast enough if you have a custom-built kernel, possibly with no initrd
> at all.

The patch that originated this thread wasn't even for distro kernels and
big initrds. Our test case lods a very small test kernel (11 KB), and it
is taking almost 15 seconds to boot.

Maybe our test case should create a custom BIOS image to be loaded on
ROM, instead?

-- 
Eduardo



Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available

2013-02-14 Thread Lucas Meneghel Rodrigues

On 02/14/2013 11:41 AM, Eduardo Habkost wrote:

I'd glad to amend patch, if there are suggestions how to generalize it and
improve "create boot image" process for distros that doesn't have
grub2-mkrescue.


A reusable tool/script would be useful even outside virt-test. Anybody
who uses -kernel/-initrd today may want a more efficient solution (IIRC,
virt-install uses -kernel/-initrd when installing from a Fedora
repository URL).

But if we made that an independent tool we would have the additional
problem of having the new tool packaged and installed on the host. So
having something specific inside virt-test makes sense by now.


Absolutely, agreed.




Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available

2013-02-14 Thread Eduardo Habkost

(CCing virt-tools list in case they have any existing solutions or ideas
on how to make this easier for everybody)


On Thu, Feb 14, 2013 at 02:20:45PM +0100, Igor Mammedov wrote:
> On Thu, 14 Feb 2013 09:18:53 -0200
> Eduardo Habkost  wrote:
> 
> > On Wed, Feb 13, 2013 at 05:17:44PM +0100, Igor Mammedov wrote:
> > > qemu boots from disk image 3 times faster than direct kernel load.
> > 
> > That's surprising. Do you have any idea why that happens?
> > 
> > (CCing qemu-devel in case other QEMU developers can explain it)
> > 
> > 
> > > Use grub2-mkrescue tool to create boot image with test kernel if available
> > > and do image boot then.
> > > On FC17 it reduces 1 VM run from ~15 sec to ~5sec.
> > > 
> > > Signed-off-by: Igor Mammedov 
> > > ---
> > >  qemu/tests/cpuid.py|   11 +--
> > >  shared/deps/cpuid_test_kernel/Makefile |   18 --
> > >  shared/deps/cpuid_test_kernel/grub.cfg |8 
> > >  3 files changed, 33 insertions(+), 4 deletions(-)
> > >  create mode 100644 shared/deps/cpuid_test_kernel/grub.cfg
> > > 
> > > diff --git a/qemu/tests/cpuid.py b/qemu/tests/cpuid.py
> > > index 5065c6a..731411c 100644
> > > --- a/qemu/tests/cpuid.py
> > > +++ b/qemu/tests/cpuid.py
> > > @@ -118,13 +118,20 @@ def run_cpuid(test, params, env):
> > > "cpuid_test_kernel")
> > >  os.chdir(test_kernel_dir)
> > >  utils.make("cpuid_dump_kernel.bin")
> > > +utils.make("boot.img", ignore_status=True)
> > > +cmdres = utils.run("cd %s && ls boot.img" % test_kernel_dir,
> > > ignore_status=True)
> > 
> > Why fork a shell just to check if a file exists? You can simply use
> > "os.path.exists('%s/boot.img' % (test_kernel_dir))" or
> > "os.path.exists(os.path.join(test_kernel_dir, 'boot.img'))".
> Thanks, I'll fix it.
> 
> > 
> > The rest of the patch looks good to me.
> > 
> > I wonder if we could make this a generic "boot kernel image" function,
> > that uses grub2-mkrescue if available, and -kernel otherwise. I believe
> > other test cases may benefit for a general mechanism to boot test
> > kernels.
> I'd make something like make_boot_image() a generic, which would
>   * take kernel, (optional initrd, kernel options), root tree directory
>   * create boot image using host available tools (which is greatly distro
> specific depending on available tools)
> and leave decision what boot method to use to test cases.
> 
> But due to lack of free time, I went through easy road.

I understand. Maybe one day somebody will volunteer to write a more
generic solution.  :-)


> I'd glad to amend patch, if there are suggestions how to generalize it and
> improve "create boot image" process for distros that doesn't have
> grub2-mkrescue.

A reusable tool/script would be useful even outside virt-test. Anybody
who uses -kernel/-initrd today may want a more efficient solution (IIRC,
virt-install uses -kernel/-initrd when installing from a Fedora
repository URL).

But if we made that an independent tool we would have the additional
problem of having the new tool packaged and installed on the host. So
having something specific inside virt-test makes sense by now.

-- 
Eduardo



Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available

2013-02-14 Thread Paolo Bonzini
Il 14/02/2013 14:24, Eduardo Habkost ha scritto:
> On Thu, Feb 14, 2013 at 01:13:18PM +0100, Paolo Bonzini wrote:
>> Il 14/02/2013 12:18, Eduardo Habkost ha scritto:
> qemu boots from disk image 3 times faster than direct kernel load.
>>> That's surprising. Do you have any idea why that happens?
>>
>> Because kernel load uses MMIO (from fw_cfg), while booting from disk
>> uses at worst PCI DMA and at best virtio.
> 
> Is it something worth trying to optimize

I think that, within the limits of what the spec makes legal, Gleb
optimized all that he could out of it.  The alternative is to make
fw_cfg do DMA, which in the past was rejected because it doesn't look
like what real ISA hardware would do.

> , or a reasonable solution would
> be so similar to having a disk+bootloader that's easier to simply
> recommend people to set up a real disk with a real bootloader if they
> care about speed?

In the end it's a pity, but yeah that's the easiest thing to do with
distro kernels and big all-drivers initrd.  -kernel is still useful and
fast enough if you have a custom-built kernel, possibly with no initrd
at all.

Paolo



Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available

2013-02-14 Thread Eduardo Habkost
On Thu, Feb 14, 2013 at 01:13:18PM +0100, Paolo Bonzini wrote:
> Il 14/02/2013 12:18, Eduardo Habkost ha scritto:
> >> > qemu boots from disk image 3 times faster than direct kernel load.
> > That's surprising. Do you have any idea why that happens?
> 
> Because kernel load uses MMIO (from fw_cfg), while booting from disk
> uses at worst PCI DMA and at best virtio.

Is it something worth trying to optimize, or a reasonable solution would
be so similar to having a disk+bootloader that's easier to simply
recommend people to set up a real disk with a real bootloader if they
care about speed?

-- 
Eduardo



Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available

2013-02-14 Thread Igor Mammedov
On Thu, 14 Feb 2013 09:18:53 -0200
Eduardo Habkost  wrote:

> On Wed, Feb 13, 2013 at 05:17:44PM +0100, Igor Mammedov wrote:
> > qemu boots from disk image 3 times faster than direct kernel load.
> 
> That's surprising. Do you have any idea why that happens?
> 
> (CCing qemu-devel in case other QEMU developers can explain it)
> 
> 
> > Use grub2-mkrescue tool to create boot image with test kernel if available
> > and do image boot then.
> > On FC17 it reduces 1 VM run from ~15 sec to ~5sec.
> > 
> > Signed-off-by: Igor Mammedov 
> > ---
> >  qemu/tests/cpuid.py|   11 +--
> >  shared/deps/cpuid_test_kernel/Makefile |   18 --
> >  shared/deps/cpuid_test_kernel/grub.cfg |8 
> >  3 files changed, 33 insertions(+), 4 deletions(-)
> >  create mode 100644 shared/deps/cpuid_test_kernel/grub.cfg
> > 
> > diff --git a/qemu/tests/cpuid.py b/qemu/tests/cpuid.py
> > index 5065c6a..731411c 100644
> > --- a/qemu/tests/cpuid.py
> > +++ b/qemu/tests/cpuid.py
> > @@ -118,13 +118,20 @@ def run_cpuid(test, params, env):
> > "cpuid_test_kernel")
> >  os.chdir(test_kernel_dir)
> >  utils.make("cpuid_dump_kernel.bin")
> > +utils.make("boot.img", ignore_status=True)
> > +cmdres = utils.run("cd %s && ls boot.img" % test_kernel_dir,
> > ignore_status=True)
> 
> Why fork a shell just to check if a file exists? You can simply use
> "os.path.exists('%s/boot.img' % (test_kernel_dir))" or
> "os.path.exists(os.path.join(test_kernel_dir, 'boot.img'))".
Thanks, I'll fix it.

> 
> The rest of the patch looks good to me.
> 
> I wonder if we could make this a generic "boot kernel image" function,
> that uses grub2-mkrescue if available, and -kernel otherwise. I believe
> other test cases may benefit for a general mechanism to boot test
> kernels.
I'd make something like make_boot_image() a generic, which would
  * take kernel, (optional initrd, kernel options), root tree directory
  * create boot image using host available tools (which is greatly distro
specific depending on available tools)
and leave decision what boot method to use to test cases.

But due to lack of free time, I went through easy road.
I'd glad to amend patch, if there are suggestions how to generalize it and
improve "create boot image" process for distros that doesn't have
grub2-mkrescue.


> 
> >  
> >  vm_name = params.get('main_vm')
> >  params_b = params.copy()
> > -params_b["kernel"] = os.path.join(test_kernel_dir,
> > "cpuid_dump_kernel.bin")
> > +if cmdres.exit_status == 0:
> > +params_b["image_name_custom"] =
> > os.path.join(test_kernel_dir, "boot.img")
> > +params_b["image_format_custom"] = ""
> > +params_b["images"] = "custom"
> > +else:
> > +params_b["kernel"] = os.path.join(test_kernel_dir,
> > "cpuid_dump_kernel.bin")
> > +del params_b["images"]
> >  params_b["cpu_model"] = cpu_model
> >  params_b["cpu_model_flags"] = feature
> > -del params_b["images"]
> >  del params_b["nics"]
> >  env_process.preprocess_vm(self, params_b, env, vm_name)
> >  vm = env.get_vm(vm_name)
> > diff --git a/shared/deps/cpuid_test_kernel/Makefile
> > b/shared/deps/cpuid_test_kernel/Makefile index 0e34c43..5999804 100644
> > --- a/shared/deps/cpuid_test_kernel/Makefile
> > +++ b/shared/deps/cpuid_test_kernel/Makefile
> > @@ -1,10 +1,24 @@
> >  CFLAGS  := -m32 -fno-stack-protector -fno-builtin -nostdinc -O -g -Wall
> > -I. LDFLAGS := -nostdlib -Wl,-N -Wl,-Ttext -Wl,10
> >  
> > -all: cpuid_dump_kernel.bin
> > +boot := $(shell sh -c 'which grub2-mkrescue 2>/dev/null && echo
> > grub2boot.img') +
> > +all: cpuid_dump_kernel.bin boot.img
> >  
> >  cpuid_dump_kernel.bin: boot.S main.c test.c
> > $(CC) -o $@ $(CFLAGS) $^ $(LDFLAGS)
> > +   ln -s $@ kernel.bin
> > +
> > +boot.img: $(boot)
> > +
> > +
> > +grub2boot.img:
> > +   mkdir -p iso/boot/grub
> > +   cp -f grub.cfg iso/boot/grub
> > +   cp -f kernel.bin iso/
> > +   grub2-mkrescue -o boot.img iso
> >  
> >  clean:
> > -   rm -f *.o *.bin
> > +   rm -rf *.o *.bin iso *.img
> > +
> > +.PHONY: grub2boot.img
> > diff --git a/shared/deps/cpuid_test_kernel/grub.cfg
> > b/shared/deps/cpuid_test_kernel/grub.cfg new file mode 100644
> > index 000..aeb5b2b
> > --- /dev/null
> > +++ b/shared/deps/cpuid_test_kernel/grub.cfg
> > @@ -0,0 +1,8 @@
> > +set timeout=0
> > +set default=0
> > +
> > +menuentry "test" {
> > +   set root=(hd0)
> > +   multiboot /kernel.bin
> > +   boot
> > +}
> > -- 
> > 1.7.1
> > 
> 




Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available

2013-02-14 Thread Paolo Bonzini
Il 14/02/2013 12:18, Eduardo Habkost ha scritto:
>> > qemu boots from disk image 3 times faster than direct kernel load.
> That's surprising. Do you have any idea why that happens?

Because kernel load uses MMIO (from fw_cfg), while booting from disk
uses at worst PCI DMA and at best virtio.

Paolo



Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available

2013-02-14 Thread Eduardo Habkost
On Wed, Feb 13, 2013 at 05:17:44PM +0100, Igor Mammedov wrote:
> qemu boots from disk image 3 times faster than direct kernel load.

That's surprising. Do you have any idea why that happens?

(CCing qemu-devel in case other QEMU developers can explain it)


> Use grub2-mkrescue tool to create boot image with test kernel if available
> and do image boot then.
> On FC17 it reduces 1 VM run from ~15 sec to ~5sec.
> 
> Signed-off-by: Igor Mammedov 
> ---
>  qemu/tests/cpuid.py|   11 +--
>  shared/deps/cpuid_test_kernel/Makefile |   18 --
>  shared/deps/cpuid_test_kernel/grub.cfg |8 
>  3 files changed, 33 insertions(+), 4 deletions(-)
>  create mode 100644 shared/deps/cpuid_test_kernel/grub.cfg
> 
> diff --git a/qemu/tests/cpuid.py b/qemu/tests/cpuid.py
> index 5065c6a..731411c 100644
> --- a/qemu/tests/cpuid.py
> +++ b/qemu/tests/cpuid.py
> @@ -118,13 +118,20 @@ def run_cpuid(test, params, env):
> "cpuid_test_kernel")
>  os.chdir(test_kernel_dir)
>  utils.make("cpuid_dump_kernel.bin")
> +utils.make("boot.img", ignore_status=True)
> +cmdres = utils.run("cd %s && ls boot.img" % test_kernel_dir, 
> ignore_status=True)

Why fork a shell just to check if a file exists? You can simply use
"os.path.exists('%s/boot.img' % (test_kernel_dir))" or
"os.path.exists(os.path.join(test_kernel_dir, 'boot.img'))".

The rest of the patch looks good to me.

I wonder if we could make this a generic "boot kernel image" function,
that uses grub2-mkrescue if available, and -kernel otherwise. I believe
other test cases may benefit for a general mechanism to boot test
kernels.

>  
>  vm_name = params.get('main_vm')
>  params_b = params.copy()
> -params_b["kernel"] = os.path.join(test_kernel_dir, 
> "cpuid_dump_kernel.bin")
> +if cmdres.exit_status == 0:
> +params_b["image_name_custom"] = os.path.join(test_kernel_dir, 
> "boot.img")
> +params_b["image_format_custom"] = ""
> +params_b["images"] = "custom"
> +else:
> +params_b["kernel"] = os.path.join(test_kernel_dir, 
> "cpuid_dump_kernel.bin")
> +del params_b["images"]
>  params_b["cpu_model"] = cpu_model
>  params_b["cpu_model_flags"] = feature
> -del params_b["images"]
>  del params_b["nics"]
>  env_process.preprocess_vm(self, params_b, env, vm_name)
>  vm = env.get_vm(vm_name)
> diff --git a/shared/deps/cpuid_test_kernel/Makefile 
> b/shared/deps/cpuid_test_kernel/Makefile
> index 0e34c43..5999804 100644
> --- a/shared/deps/cpuid_test_kernel/Makefile
> +++ b/shared/deps/cpuid_test_kernel/Makefile
> @@ -1,10 +1,24 @@
>  CFLAGS  := -m32 -fno-stack-protector -fno-builtin -nostdinc -O -g -Wall -I.
>  LDFLAGS := -nostdlib -Wl,-N -Wl,-Ttext -Wl,10
>  
> -all: cpuid_dump_kernel.bin
> +boot := $(shell sh -c 'which grub2-mkrescue 2>/dev/null && echo 
> grub2boot.img')
> +
> +all: cpuid_dump_kernel.bin boot.img
>  
>  cpuid_dump_kernel.bin: boot.S main.c test.c
>   $(CC) -o $@ $(CFLAGS) $^ $(LDFLAGS)
> + ln -s $@ kernel.bin
> +
> +boot.img: $(boot)
> +
> +
> +grub2boot.img:
> + mkdir -p iso/boot/grub
> + cp -f grub.cfg iso/boot/grub
> + cp -f kernel.bin iso/
> + grub2-mkrescue -o boot.img iso
>  
>  clean:
> - rm -f *.o *.bin
> + rm -rf *.o *.bin iso *.img
> +
> +.PHONY: grub2boot.img
> diff --git a/shared/deps/cpuid_test_kernel/grub.cfg 
> b/shared/deps/cpuid_test_kernel/grub.cfg
> new file mode 100644
> index 000..aeb5b2b
> --- /dev/null
> +++ b/shared/deps/cpuid_test_kernel/grub.cfg
> @@ -0,0 +1,8 @@
> +set timeout=0
> +set default=0
> +
> +menuentry "test" {
> +   set root=(hd0)
> +   multiboot /kernel.bin
> +   boot
> +}
> -- 
> 1.7.1
> 

-- 
Eduardo