On Tue, Sep 4, 2018 at 2:36 AM Nadav Har'El <[email protected]> wrote:

> Hi, thanks, I committed this, but also have some questions below.
>
>
> On Fri, Aug 31, 2018 at 8:34 PM, Waldemar Kozaczuk <[email protected]>
> 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?
>
Yes I have code that fixes it and is part of the patch I submitted earlier
but decided to abandon. Instead I have decided to take smaller step by step
approach and this fix will be part of the next patch.

>
>
>> Signed-off-by: Waldemar Kozaczuk <[email protected]>
>> ---
>>  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.
>
I will try to eventually address some way, either by following your
suggestion or making lzloader handle multiboot.

>
>>
>>  .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 [email protected].
>> 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 [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to