Hi, thanks, I committed this, but also have some questions below.

On Fri, Aug 31, 2018 at 8:34 PM, Waldemar Kozaczuk <jwkozac...@gmail.com>
wrote:

> This patch revives old boot32/loader.bin mode which
> apparently was used to boot OSv in multiboot mode
> (aka -kernel with qemu).
>
> Please note that there are some gaps in this multiboot
> implementation. For example cmdline does not get
> properly passed

and it only works with multiboot loaders
> that put multiboot info structure into the low memory.
>

I think you already had code fixing this?
Will something like that work also here?


> Signed-off-by: Waldemar Kozaczuk <jwkozac...@gmail.com>
> ---
>  Makefile          |  5 +++--
>  arch/x64/boot32.S |  2 +-
>  scripts/run.py    | 16 ++++++++++++++--
>  3 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 770b28d0..0ca95c64 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -131,7 +131,7 @@ endif
>  quiet = $(if $V, $1, @echo " $2"; $1)
>  very-quiet = $(if $V, $1, @$1)
>
> -all: $(out)/loader.img links
> +all: $(out)/loader.img links $(out)/loader.bin
>

This is not a huge issue, but now every "make" will make yet another copy
(or even two, in boot32.o and loader.bin) of the kernel.
I'm not sure what to suggest instead, though.

One option is to just not put loader.bin in "all", and ask a user to
manually "make" it (probably should add a comment on this target explaining
what it is).
Another option is to have a separate script, like we have scripts/convert,
which "converts" loader.img into loader.bin.
I guess we can consider these options later. Or leave it like it is now.

>
>  .PHONY: all
>
>  links:
> @@ -442,7 +442,8 @@ $(out)/loader.bin: $(out)/arch/x64/boot32.o
> arch/x64/loader32.ld
>         $(call quiet, $(LD) -nostartfiles -static -nodefaultlibs -o $@ \
>                         $(filter-out %.bin, $(^:%.ld=-T %.ld)), LD $@)
>
> -$(out)/arch/x64/boot32.o: $(out)/loader.elf
> +$(out)/arch/x64/boot32.o: $(out)/loader-stripped.elf
> +$(out)/arch/x64/boot32.o: ASFLAGS += -I$(out)
>
>  $(out)/fastlz/fastlz.o:
>         $(makedir)
> diff --git a/arch/x64/boot32.S b/arch/x64/boot32.S
> index 00298fba..33597e6d 100644
> --- a/arch/x64/boot32.S
> +++ b/arch/x64/boot32.S
> @@ -22,4 +22,4 @@ start32:
>  .align 4096
>
>  elf64_header:
> -.incbin "loader.elf"
> +.incbin "loader-stripped.elf"
> diff --git a/scripts/run.py b/scripts/run.py
> index e5a76f1d..92cd4e5f 100755
> --- a/scripts/run.py
> +++ b/scripts/run.py
> @@ -63,6 +63,7 @@ def set_imgargs(options):
>      if options.sampler:
>          execute = '--sampler=%d %s' % (int(options.sampler), execute)
>
> +    options.osv_cmdline = execute
>      cmdline = ["scripts/imgedit.py", "setargs", options.image_file,
> execute]
>      if options.dry_run:
>          print(format_args(cmdline))
> @@ -108,6 +109,14 @@ def start_osv_qemu(options):
>          args += [
>          "-display", "sdl"]
>
> +    if options.kernel:
> +        boot_index = ""
> +        args += [
> +        "-kernel", options.kernel_file,
> +        "-append", options.osv_cmdline]
> +    else:
> +        boot_index = ",bootindex=0"
> +
>      if options.sata:
>          args += [
>          "-machine", "q35",
> @@ -117,13 +126,13 @@ def start_osv_qemu(options):
>          args += [
>          "-device", "virtio-scsi-pci,id=scsi0",
>          "-drive", "file=%s,if=none,id=hd0,media=disk,%s" %
> (options.image_file, aio),
> -        "-device", "scsi-hd,bus=scsi0.0,drive=
> hd0,scsi-id=1,lun=0,bootindex=0"]
> +        "-device", "scsi-hd,bus=scsi0.0,drive=hd0,scsi-id=1,lun=0%s" %
> boot_index]
>      elif options.ide:
>          args += [
>          "-hda", options.image_file]
>      else:
>          args += [
> -        "-device", "virtio-blk-pci,id=blk0,bootindex=0,drive=hd0,scsi=
> off",
> +        "-device", "virtio-blk-pci,id=blk0,drive=hd0,scsi=off%s" %
> boot_index,
>          "-drive", "file=%s,if=none,id=hd0,%s" % (options.image_file, aio)]
>
>      if options.cloud_init_image:
> @@ -489,9 +498,12 @@ if __name__ == "__main__":
>                          help="XEN define configuration script for vif")
>      parser.add_argument("--cloud-init-image", action="store",
>                          help="Path to the optional cloud-init image that
> should be attached to the instance")
> +    parser.add_argument("-k", "--kernel", action="store_true",
> +                        help="Run OSv in QEMU kernel mode as multiboot.")
>      cmdargs = parser.parse_args()
>      cmdargs.opt_path = "debug" if cmdargs.debug else "release" if
> cmdargs.release else "last"
>      cmdargs.image_file = os.path.abspath(cmdargs.image or
> "build/%s/usr.img" % cmdargs.opt_path)
> +    cmdargs.kernel_file = "build/%s/loader.bin" % cmdargs.opt_path
>      if not os.path.exists(cmdargs.image_file):
>          raise Exception('Image file %s does not exist.' %
> cmdargs.image_file)
>      if cmdargs.cloud_init_image:
> --
> 2.17.1
>
> --
> You received this message because you are subscribed to the Google Groups
> "OSv Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to osv-dev+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to