Re: [Xen-devel] [PATCH v11 08/13] x86/boot: implement early command line parser in C

2016-12-09 Thread Daniel Kiper 
On Fri, Dec 09, 2016 at 01:19:24AM -0700, Jan Beulich wrote:
> >>> On 09.12.16 at 00:08,  wrote:

[...]

> > I have checked it. It requires at least some changes made by patch #1 which
> > has "Reviewed-by: Jan Beulich ". Of course I can change
> > this but then I think that I should drop your Reviewed-by from #1 and your
> > Acked-by from #8.
>
> Unless you change the structure of the code you move, I don't think
> either of the tags would require dropping.

I think that code move will be sufficient and logic should not change.
So, if it works in that way I will leave your tags as is.

> > Does it pays? I think that we can do that in a bit different
> > way. If there are no more comments please apply everything as is.
>
> I think Andrew and I are in agreement that we're not at the point
> yet where everything can go in as is. Please be patient - this has
> taken so long to get where it is now that I don't think there's a
> reason to rush anything now. And you may have noticed that
> there have been quite a few other patch submissions which also
> all want dealing with. Andrew and I should also be allowed some
> room to actually do some work of our own ...

I understand that. This is not a problem for me. However, I would like to see
my patches in 4.9. Otherwise I am spinning around and cannot efficiently
continue my work which partially depends on this patch series.

Daniel

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v11 08/13] x86/boot: implement early command line parser in C

2016-12-09 Thread Jan Beulich 
>>> On 09.12.16 at 00:08,  wrote:
> On Wed, Dec 07, 2016 at 06:27:58PM +0100, Daniel Kiper wrote:
>> On Wed, Dec 07, 2016 at 06:43:40AM -0700, Jan Beulich wrote:
>> > >>> On 05.12.16 at 23:25,  wrote:
>> > > Current early command line parser implementation in assembler
>> > > is very difficult to change to relocatable stuff using segment
>> > > registers. This requires a lot of changes in very weird and
>> > > fragile code. So, reimplement this functionality in C. This
>> > > way code will be relocatable out of the box (without playing
>> > > with segment registers) and much easier to maintain.
>> > >
>> > > Additionally, put all common cmdline.c and reloc.c definitions
>> > > into defs.h header. This way we do not duplicate needlessly
>> > > some stuff.
>> > >
>> > > And finally remove unused xen/include/asm-x86/config.h
>> > > header from reloc.c dependencies.
>> > >
>> > > Suggested-by: Andrew Cooper 
>> > > Signed-off-by: Daniel Kiper 
>> > > Acked-by: Jan Beulich 
>> >
>> > As you may have seen I've applied patches 2..4. I would also
>>
>> Great! Thanks a lot!
>>
>> > have applied this one, but it fails to apply cleanly. Whether
>> > that's because it needs re-basing or because it can't be applied
>> > out of order I can't tell. In order for you to not have to re-submit
> 
> I have checked it. It requires at least some changes made by patch #1 which
> has "Reviewed-by: Jan Beulich ". Of course I can change
> this but then I think that I should drop your Reviewed-by from #1 and your
> Acked-by from #8.

Unless you change the structure of the code you move, I don't think
either of the tags would require dropping.

> Does it pays? I think that we can do that in a bit different
> way. If there are no more comments please apply everything as is.

I think Andrew and I are in agreement that we're not at the point
yet where everything can go in as is. Please be patient - this has
taken so long to get where it is now that I don't think there's a
reason to rush anything now. And you may have noticed that
there have been quite a few other patch submissions which also
all want dealing with. Andrew and I should also be allowed some
room to actually do some work of our own ...

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v11 08/13] x86/boot: implement early command line parser in C

2016-12-08 Thread Daniel Kiper 
On Wed, Dec 07, 2016 at 06:27:58PM +0100, Daniel Kiper wrote:
> On Wed, Dec 07, 2016 at 06:43:40AM -0700, Jan Beulich wrote:
> > >>> On 05.12.16 at 23:25,  wrote:
> > > Current early command line parser implementation in assembler
> > > is very difficult to change to relocatable stuff using segment
> > > registers. This requires a lot of changes in very weird and
> > > fragile code. So, reimplement this functionality in C. This
> > > way code will be relocatable out of the box (without playing
> > > with segment registers) and much easier to maintain.
> > >
> > > Additionally, put all common cmdline.c and reloc.c definitions
> > > into defs.h header. This way we do not duplicate needlessly
> > > some stuff.
> > >
> > > And finally remove unused xen/include/asm-x86/config.h
> > > header from reloc.c dependencies.
> > >
> > > Suggested-by: Andrew Cooper 
> > > Signed-off-by: Daniel Kiper 
> > > Acked-by: Jan Beulich 
> >
> > As you may have seen I've applied patches 2..4. I would also
>
> Great! Thanks a lot!
>
> > have applied this one, but it fails to apply cleanly. Whether
> > that's because it needs re-basing or because it can't be applied
> > out of order I can't tell. In order for you to not have to re-submit

I have checked it. It requires at least some changes made by patch #1 which
has "Reviewed-by: Jan Beulich ". Of course I can change
this but then I think that I should drop your Reviewed-by from #1 and your
Acked-by from #8. Does it pays? I think that we can do that in a bit different
way. If there are no more comments please apply everything as is. If any new
comment, worth taking into account, pop up I can rearrange patches order and
drop your Reviewed-by/Acked-by too. Does it work for you?

Daniel

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v11 08/13] x86/boot: implement early command line parser in C

2016-12-07 Thread Daniel Kiper 
On Wed, Dec 07, 2016 at 06:43:40AM -0700, Jan Beulich wrote:
> >>> On 05.12.16 at 23:25,  wrote:
> > Current early command line parser implementation in assembler
> > is very difficult to change to relocatable stuff using segment
> > registers. This requires a lot of changes in very weird and
> > fragile code. So, reimplement this functionality in C. This
> > way code will be relocatable out of the box (without playing
> > with segment registers) and much easier to maintain.
> >
> > Additionally, put all common cmdline.c and reloc.c definitions
> > into defs.h header. This way we do not duplicate needlessly
> > some stuff.
> >
> > And finally remove unused xen/include/asm-x86/config.h
> > header from reloc.c dependencies.
> >
> > Suggested-by: Andrew Cooper 
> > Signed-off-by: Daniel Kiper 
> > Acked-by: Jan Beulich 
>
> As you may have seen I've applied patches 2..4. I would also

Great! Thanks a lot!

> have applied this one, but it fails to apply cleanly. Whether
> that's because it needs re-basing or because it can't be applied
> out of order I can't tell. In order for you to not have to re-submit
> the whole series all the time it would help if you moved
> uncontroversial patches which make sense to be applied without
> the rest of the series to the beginning of the series.

Will do!

Daniel

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v11 08/13] x86/boot: implement early command line parser in C

2016-12-07 Thread Jan Beulich 
>>> On 05.12.16 at 23:25,  wrote:
> Current early command line parser implementation in assembler
> is very difficult to change to relocatable stuff using segment
> registers. This requires a lot of changes in very weird and
> fragile code. So, reimplement this functionality in C. This
> way code will be relocatable out of the box (without playing
> with segment registers) and much easier to maintain.
> 
> Additionally, put all common cmdline.c and reloc.c definitions
> into defs.h header. This way we do not duplicate needlessly
> some stuff.
> 
> And finally remove unused xen/include/asm-x86/config.h
> header from reloc.c dependencies.
> 
> Suggested-by: Andrew Cooper 
> Signed-off-by: Daniel Kiper 
> Acked-by: Jan Beulich 

As you may have seen I've applied patches 2..4. I would also
have applied this one, but it fails to apply cleanly. Whether
that's because it needs re-basing or because it can't be applied
out of order I can't tell. In order for you to not have to re-submit
the whole series all the time it would help if you moved
uncontroversial patches which make sense to be applied without
the rest of the series to the beginning of the series.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v11 08/13] x86/boot: implement early command line parser in C

2016-12-05 Thread Daniel Kiper 
Current early command line parser implementation in assembler
is very difficult to change to relocatable stuff using segment
registers. This requires a lot of changes in very weird and
fragile code. So, reimplement this functionality in C. This
way code will be relocatable out of the box (without playing
with segment registers) and much easier to maintain.

Additionally, put all common cmdline.c and reloc.c definitions
into defs.h header. This way we do not duplicate needlessly
some stuff.

And finally remove unused xen/include/asm-x86/config.h
header from reloc.c dependencies.

Suggested-by: Andrew Cooper 
Signed-off-by: Daniel Kiper 
Acked-by: Jan Beulich 
---
v7 - suggestions/fixes:
   - add min() macro
 (suggested by Jan Beulich),
   - add padding to early_boot_opts_t
 in more standard way
 (suggested by Jan Beulich),
   - simplify defs.h dependencies
 (suggested by Jan Beulich).

v6 - suggestions/fixes:
   - put common cmdline.c and reloc.c
 definitions into defs.h header
 (suggested by Jan Beulich),
   - use xen/include/xen/stdbool.h
 and bool type from it instead
 of own defined bool_t
 (suggested by Jan Beulich),
   - define delim_chars as constant
 (suggested by Jan Beulich),
   - properly align trampoline.S:early_boot_opts struct
 (suggested by Jan Beulich),
   - fix overflow check in strtoui()
 (suggested by Jan Beulich),
   - remove unused xen/include/asm-x86/config.h
 header from reloc.c dependencies,
   - improve commit message.

v4 - suggestions/fixes:
   - move to stdcall calling convention
 (suggested by Jan Beulich),
   - define bool_t and use it properly
 (suggested by Jan Beulich),
   - put list of delimiter chars into
 static const char[]
 (suggested by Jan Beulich),
   - use strlen() instead of strlen_opt()
 (suggested by Jan Beulich),
   - change strtoi() to strtoui() and
 optimize it a bit
 (suggested by Jan Beulich),
   - define strchr() and use it in strtoui()
 (suggested by Jan Beulich),
   - optimize vga_parse()
 (suggested by Jan Beulich),
   - move !cmdline check from assembly to C
 (suggested by Jan Beulich),
   - remove my name from copyright (Oracle requirement)
 (suggested by Konrad Rzeszutek Wilk).

v3 - suggestions/fixes:
   - optimize some code
 (suggested by Jan Beulich),
   - put VESA data into early_boot_opts_t members
 (suggested by Jan Beulich),
   - rename some functions and variables
 (suggested by Jan Beulich),
   - move around video.h include in xen/arch/x86/boot/trampoline.S
 (suggested by Jan Beulich),
   - fix coding style
 (suggested by Jan Beulich),
   - fix build with older GCC
 (suggested by Konrad Rzeszutek Wilk),
   - remove redundant comments
 (suggested by Jan Beulich),
   - add some comments
   - improve commit message
 (suggested by Jan Beulich).
---
 .gitignore |5 +-
 xen/arch/x86/Makefile  |2 +-
 xen/arch/x86/boot/Makefile |   11 +-
 xen/arch/x86/boot/build32.mk   |2 +
 xen/arch/x86/boot/cmdline.S|  367 
 xen/arch/x86/boot/cmdline.c|  340 +
 xen/arch/x86/boot/defs.h   |   58 +++
 xen/arch/x86/boot/edd.S|3 -
 xen/arch/x86/boot/head.S   |8 +
 xen/arch/x86/boot/reloc.c  |   13 +-
 xen/arch/x86/boot/trampoline.S |   15 ++
 xen/arch/x86/boot/video.S  |7 -
 12 files changed, 437 insertions(+), 394 deletions(-)
 delete mode 100644 xen/arch/x86/boot/cmdline.S
 create mode 100644 xen/arch/x86/boot/cmdline.c
 create mode 100644 xen/arch/x86/boot/defs.h

diff --git a/.gitignore b/.gitignore
index a2f34a1..d2967d8 100644
--- a/.gitignore
+++ b/.gitignore
@@ -250,9 +250,10 @@ xen/arch/arm/xen.lds
 xen/arch/x86/asm-offsets.s
 xen/arch/x86/boot/mkelf32
 xen/arch/x86/xen.lds
+xen/arch/x86/boot/cmdline.S
 xen/arch/x86/boot/reloc.S
-xen/arch/x86/boot/reloc.bin
-xen/arch/x86/boot/reloc.lnk
+xen/arch/x86/boot/*.bin
+xen/arch/x86/boot/*.lnk
 xen/arch/x86/efi.lds
 xen/arch/x86/efi/check.efi
 xen/arch/x86/efi/disabled
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 2a0781a..e74fe62 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -220,5 +220,5 @@ clean::
rm -f asm-offsets.s *.lds boot/*.o boot/*~ boot/core boot/mkelf32
rm -f $(BASEDIR)/.xen-syms.[0-9]* boot/.*.d
rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.efi efi/disabled efi/mkreloc
-   rm -f boot/reloc.S boot/reloc.lnk boot/reloc.bin
+   rm -f boot/cmdline.S boot/reloc.S boot/*.lnk boot/*.bin
rm -f note.o
diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
index 06893d8..c6246c8 100644
--- a/xen/arch/x86/boot/Makefile
+++ b/xen/arch/x86/boot/Makefile
@@ -1,9 +1,16 @@
 obj-bin-y += head.o
 
-RELOC_DEPS = $(BASEDIR)/include/asm-x86/config.h