kvm tools: A few fixes
cover kvm tools: A few fixes
Nothing serious, please review. Thanks.
Cyrill
PATCH 1/5 kvm tools: Options parser to handle hex numbers
Some kernel parameters are convenient if passed in hex form so our options parser should handle even such form of input.
--- tools/kvm/util/parse-options.c | 102 ++++++++++++++++++++++++++++++++--------- 1 file changed, 82 insertions(+), 20 deletions(-)
tools/kvm/util/parse-options.c +82 -20
@@ -39,6 +39,84 @@ static int get_arg(struct parse_opt_ctx_return 0 ; } +#define numvalue (c) \ + ((c) >= 'a' ? (c) - 'a' + 10 : \ + (c) >= 'A' ? (c) - 'A' + 10 : (c) - '0') + +static u64 readhex (const char *str,bool *error) +{ +char *pos =strchr (str,'x' ) +1 ; +u64 res =0 ; + +while (*pos) { +unsigned int v =numvalue (*pos); +if (v >16 ) { + *error = true; +return 0 ; + } + + res = (res *16 ) + v; + pos++; + } + + *error = false; +return res; +} + +static int readnum (const struct option *opt,int flags, +const char *str,char **end) +{ +if (strchr (str,'x' )) { +bool error; +u64 value; + + value =readhex (str, &error); +if (error) + goto enotnum; + +switch (opt->type) { +case OPTION_INTEGER : + *(int *)opt->value = value; +break ; +case OPTION_UINTEGER : + *(unsigned int *)opt->value = value; +break ; +case OPTION_LONG : + *(long *)opt->value = value; +break ; +case OPTION_U64 : + *(u64 *)opt->value = value; +break ; +default : + goto invcall; + } + }else { +switch (opt->type) { +case OPTION_INTEGER : + *(int *)opt->value =strtol (str, end,10 ); +break ; +case OPTION_UINTEGER : + *(unsigned int *)opt->value =strtol (str, end,10 ); +break ; +case OPTION_LONG : + *(long *)opt->value =strtol (str, end,10 ); +break ; +case OPTION_U64 : + *(u64 *)opt->value =strtoull (str, end,10 ); +break ; +default : + goto invcall; + } + } + +return 0 ; + +enotnum: +return opterror (opt,"expects a numerical value" , flags); +invcall: +return opterror (opt,"invalid numeric conversion" , flags); +} +static int get_value (struct parse_opt_ctx_t *p,const struct option *opt,int flags) {
@@ -131,11 +209,7 @@ static int get_value(struct parse_opt_ct }if (get_arg (p, opt, flags, &arg))return -1 ; - *(int *)opt->value =strtol (arg, (char **)&s,10 ); -if (*s) -return opterror (opt,"expects a numerical value" , - flags); -return 0 ; +return readnum (opt, flags, arg, (char **)&s);case OPTION_UINTEGER :if (unset) {
@@ -148,11 +222,7 @@ static int get_value(struct parse_opt_ct }if (get_arg (p, opt, flags, &arg))return -1 ; - *(unsigned int *)opt->value =strtol (arg, (char **)&s,10 ); -if (*s) -return opterror (opt, -"expects a numerical value" , flags); -return 0 ; +return readnum (opt, flags, arg, (char **)&s);case OPTION_LONG :if (unset) {
@@ -165,11 +235,7 @@ static int get_value(struct parse_opt_ct }if (get_arg (p, opt, flags, &arg))return -1 ; - *(long *)opt->value =strtol (arg, (char **)&s,10 ); -if (*s) -return opterror (opt, -"expects a numerical value" , flags); -return 0 ; +return readnum (opt, flags, arg, (char **)&s);case OPTION_U64 :if (unset) {
@@ -182,11 +248,7 @@ static int get_value(struct parse_opt_ct }if (get_arg (p, opt, flags, &arg))return -1 ; - *(u64 *)opt->value =strtoull (arg, (char **)&s,10 ); -if (*s) -return opterror (opt, -"expects a numerical value" , flags); -return 0 ; +return readnum (opt, flags, arg, (char **)&s);case OPTION_END :case OPTION_ARGUMENT :
PATCH 2/5 kvm tools: Introduce vidmode parmeter
In reply to Cyrill GorcunovUsually this might be set by loader but since
we're the loader lets allow to specify vesa
mode as well.
--- tools/kvm/kvm-run.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
tools/kvm/kvm-run.c +7 -5
@@ -80,6 +80,7 @@ extern int active_console;bool do_debug_print = false;static int nrcpus; +static int vidmode =0x312 ;static const char *const run_usage[] = {"kvm run [<options>] [<kernel image>]" ,
@@ -139,6 +140,10 @@ static const struct option options[] = {OPT_STRING ('\0' ,"tapscript" , &script,"Script path" ,"Assign a script to process created tap device" ), +OPT_GROUP ("BIOS options:" ), +OPT_INTEGER ('\0' ,"vidmode" , &vidmode, +"Video mode" ), +OPT_GROUP ("Debug options:" ),OPT_BOOLEAN ('\0' ,"debug" , &do_debug_print, "Enable debug messages"),
@@ -434,7 +439,6 @@ int kvm_cmd_run(int argc, const char **astruct framebuffer *fb =NULL ;unsigned int nr_online_cpus;int exit_code =0 ; -u16 vidmode =0 ;int max_cpus;char *hi;int i;
@@ -541,12 +545,10 @@ int kvm_cmd_run(int argc, const char **amemset (real_cmdline,0 ,sizeof (real_cmdline));strcpy (real_cmdline,"notsc noapic noacpi pci=conf1" ); -if (vnc || sdl) { +if (vnc || sdl)strcat (real_cmdline," video=vesafb console=tty0" ); - vidmode =0x312 ; - }else { +else strcat (real_cmdline," console=ttyS0 earlyprintk=serial" ); - }strcat (real_cmdline," " );if (kernel_cmdline)strlcat (real_cmdline, kernel_cmdline,sizeof (real_cmdline));
Re: [patch 2/5] kvm tools: Introduce vidmode parmeter
Quoted 5 lines · Cyrill Gorcunov
Usually this might be set by loader but since
we're the loader lets allow to specify vesa
mode as well.Signed-off-by Cyrill Gorcunov <[email protected]>
This patch causes 'make check' to go crazy and print out bunch of these:
Warning: Ignoring MMIO write at 00000000d0031f40 (length 4)
Warning: Ignoring MMIO write at 00000000d0031f44 (length 4)
Warning: Ignoring MMIO write at 00000000d0031f48 (length 4)
Warning: Ignoring MMIO write at 00000000d0031f4c (length 4)
Warning: Ignoring MMIO write at 00000000d0031f50 (length 4)
Warning: Ignoring MMIO write at 00000000d0031f54 (length 4)
Warning: Ignoring MMIO write at 00000000d0031f58 (length 4)
Warning: Ignoring MMIO write at 00000000d0031f5c (length 4)
Warning: Ignoring MMIO write at 00000000d0031f60 (length 4)
Warning: Ignoring MMIO write at 00000000d0031f64 (length 4)
Warning: Ignoring MMIO write at 00000000d0031f68 (length 4)
Warning: Ignoring MMIO write at 00000000d0031f6c (length 4)
Warning: Ignoring MMIO write at 00000000d0031f70 (length 4)
Warning: Ignoring MMIO write at 00000000d0031f74 (length 4)
Warning: Ignoring MMIO write at 00000000d0031f78 (length 4)
Warning: Ignoring MMIO write at 00000000d0031f7c (length 4)
Re: [patch 2/5] kvm tools: Introduce vidmode parmeter
Quoted 11 lines · Pekka Enberg
Quoted 5 lines · Cyrill Gorcunov
Usually this might be set by loader but since
we're the loader lets allow to specify vesa
mode as well.Signed-off-by Cyrill Gorcunov <[email protected]>This patch causes 'make check' to go crazy and print out bunch of these:
Warning: Ignoring MMIO write at
00000000d0031f40(length 4)
Hmm, weird...
Cyrill
Re: [patch 2/5] kvm tools: Introduce vidmode parmeter
In reply to Pekka EnbergQuoted 9 lines · Pekka Enberg
Quoted 5 lines · Cyrill Gorcunov
Usually this might be set by loader but since
we're the loader lets allow to specify vesa
mode as well.Signed-off-by Cyrill Gorcunov <[email protected]>This patch causes 'make check' to go crazy and print out bunch of these:
Pekka, are you sure it's because of _this_ particular patch?
Cyrill
Re: [patch 2/5] kvm tools: Introduce vidmode parmeter
Quoted 6 lines · Cyrill Gorcunov
Quoted 5 lines · Pekka Enberg
Quoted 4 lines · Cyrill Gorcunov
Usually this might be set by loader but since
we're the loader lets allow to specify vesa
mode as well.
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Cyrill Gorcunov <[email protected]>This patch causes 'make check' to go crazy and print out bunch of these:
Pekka, are you sure it's because of _this_ particular patch?
Cyrill
This one should do the trick, cant say I like it, we probably need some default values from options parser, ie to extend it.
Cyrill
---
kvm tools: Introduce vidmode parmeter v2
Usually this might be set by loader but since
we're the loader lets allow to specify vesa
mode as well.
v2: Pekka spotted the default value was being compromised,
so revert it back and set only if specified.
--- tools/kvm/kvm-run.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)
tools/kvm/kvm-run.c +16 -4
@@ -80,6 +80,7 @@ extern int active_console;bool do_debug_print = false;static int nrcpus; +static int vidmode =-1 ;static const char *const run_usage[] = {"kvm run [<options>] [<kernel image>]" ,
@@ -139,6 +140,10 @@ static const struct option options[] = {OPT_STRING ('\0' ,"tapscript" , &script,"Script path" ,"Assign a script to process created tap device" ), +OPT_GROUP ("BIOS options:" ), +OPT_INTEGER ('\0' ,"vidmode" , &vidmode, +"Video mode" ), +OPT_GROUP ("Debug options:" ),OPT_BOOLEAN ('\0' ,"debug" , &do_debug_print, "Enable debug messages"),
@@ -434,7 +439,6 @@ int kvm_cmd_run(int argc, const char **astruct framebuffer *fb =NULL ;unsigned int nr_online_cpus;int exit_code =0 ; -u16 vidmode =0 ;int max_cpus;char *hi;int i;
@@ -539,14 +543,22 @@ int kvm_cmd_run(int argc, const char **a kvm->nrcpus = nrcpus; +/* +* vidmode should be either specified +* either set by default +*/ +if (vnc || sdl) { +if (vidmode ==-1 ) + vidmode =0x312 ; + }else + vidmode =0 ; +memset (real_cmdline,0 ,sizeof (real_cmdline));strcpy (real_cmdline,"notsc noapic noacpi pci=conf1" );if (vnc || sdl) {strcat (real_cmdline," video=vesafb console=tty0" ); - vidmode =0x312 ; - }else { + }else strcat (real_cmdline," console=ttyS0 earlyprintk=serial" ); - }strcat (real_cmdline," " );if (kernel_cmdline)strlcat (real_cmdline, kernel_cmdline,sizeof (real_cmdline));
PATCH 3/5 kvm tools: Delete dangling cursor from int10
In reply to Cyrill GorcunovNoone use it anymore. Also cleanup comment on
int10 as well, int10_handler routine do all
the hard work.
--- tools/kvm/bios/bios-rom.S | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-)
tools/kvm/bios/bios-rom.S +1 -13
@@ -18,13 +18,7 @@ ENTRY(bios_intfake) ENTRY_END(bios_intfake) /* - * int 10 - video - write character and advance cursor (tty write) - * ah = 0eh - * al = character - * bh = display page (alpha modes) - * bl = foreground color (graphics modes) - * - * We ignore bx settings + * int 10 - video - service */ ENTRY(bios_int10) pushw %fs
@@ -55,12 +49,6 @@ ENTRY(bios_int10) popw %fs IRET - - -/* - * private IRQ data - */ -cursor: .long 0 ENTRY_END(bios_int10) #define EFLAGS_CF (1 << 0)
PATCH 4/5 kvm tools: Get rid of spaces in ld script
In reply to Cyrill Gorcunov--- tools/kvm/bios/rom.ld.S | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
tools/kvm/bios/rom.ld.S +2 -2
@@ -11,7 +11,7 @@ PHDRS { } SECTIONS { - . = 0; - .text : { *(.text) } :text = 0x9090 + . = 0; + .text : { *(.text) } :text = 0x9090 }
PATCH 5/5 kvm tools: Reform bios make fules
In reply to Cyrill GorcunovPut bios code into bios.s and adjust makefile rules accordingly. It's more natural than bios-rom.S (which is now simply a container over real bios code).
Also improve bios deps in Makefile.
--- tools/kvm/Makefile | 29 +++++++----- tools/kvm/bios/bios-rom.S | 95 +++--------------------------------------- tools/kvm/bios/bios.S | 95 ++++++++++++++++++++++++++++++++++++++---- tools/kvm/bios/gen-offsets.sh | 3 - 4 files changed, 115 insertions(+), 107 deletions(-)
tools/kvm/Makefile +18 -11
@@ -82,7 +82,7 @@ DEPS := $(patsubst %.o,%.d,$(OBJS)) # Exclude BIOS object files from header dependencies. OBJS += bios.o -OBJS += bios/bios.o +OBJS += bios/bios-rom.o LIBS += -lrt LIBS += -lpthread
@@ -165,20 +165,27 @@ BIOS_CFLAGS += -m32 BIOS_CFLAGS += -march=i386 BIOS_CFLAGS += -mregparm=3 -bios.o: bios/bios-rom.bin -bios/bios.o: bios/bios.S bios/bios-rom.bin - $(E) " CC " $@ - $(Q) $(CC) -c $(CFLAGS) bios/bios.S -o bios/bios.o - -bios/bios-rom.bin: bios/bios-rom.S bios/e820.c - $(E) " CC " $@ +bios.o: bios/bios.bin bios/bios-rom.h + +bios/bios.bin.elf: bios/bios.S bios/e820.c bios/int10.c bios/rom.ld.S + $(E) " CC bios/e820.o" $(Q) $(CC) -include code16gcc.h $(CFLAGS) $(BIOS_CFLAGS) -c -s bios/e820.c -o bios/e820.o + $(E) " CC bios/int10.o" $(Q) $(CC) -include code16gcc.h $(CFLAGS) $(BIOS_CFLAGS) -c -s bios/int10.c -o bios/int10.o - $(Q) $(CC) $(CFLAGS) $(BIOS_CFLAGS) -c -s bios/bios-rom.S -o bios/bios-rom.o + $(E) " CC bios/bios.o" + $(Q) $(CC) $(CFLAGS) $(BIOS_CFLAGS) -c -s bios/bios.S -o bios/bios.o $(E) " LD " $@ - $(Q) ld -T bios/rom.ld.S -o bios/bios-rom.bin.elf bios/bios-rom.o bios/e820.o bios/int10.o + $(Q) ld -T bios/rom.ld.S -o bios/bios.bin.elf bios/bios.o bios/e820.o bios/int10.o + +bios/bios.bin: bios/bios.bin.elf $(E) " OBJCOPY " $@ - $(Q) objcopy -O binary -j .text bios/bios-rom.bin.elf bios/bios-rom.bin + $(Q) objcopy -O binary -j .text bios/bios.bin.elf bios/bios.bin + +bios/bios-rom.o: bios/bios-rom.S bios/bios.bin bios/bios-rom.h + $(E) " CC " $@ + $(Q) $(CC) -c $(CFLAGS) bios/bios-rom.S -o bios/bios-rom.o + +bios/bios-rom.h: bios/bios.bin.elf $(E) " NM " $@ $(Q) cd bios && sh gen-offsets.sh > bios-rom.h && cd ..
tools/kvm/bios/bios-rom.S +9 -86
98 lines not on this page · raw message
tools/kvm/bios/bios.S +86 -9
98 lines not on this page · raw message
tools/kvm/bios/gen-offsets.sh +2 -1
@@ -8,6 +8,7 @@ echo "" echo "#define BIOS_ENTRY_SIZE(name) (name##_end - name)" echo "" -nm bios-rom.bin.elf | grep ' [Tt] ' | awk '{ print "#define BIOS_OFFSET__" $3 " 0x" $1; }' +nm bios.bin.elf | grep ' [Tt] ' | awk '{ print "#define BIOS_OFFSET__" $3 " 0x" $1; }' +echo "" echo "#endif"