[SeaBIOS] Re: [PATCH 3/4] romlayout32flag.lds: Use `. +=` instead of `. =`

2020-04-06 Thread Kevin O'Connor
On Wed, Apr 01, 2020 at 10:29:14AM -0700, Fangrui Song wrote:
> This improves the portability of the linker script and allows lld to link 
> rom.o
> 
> Dot assignment inside an output section has an inconsistent behavior
> which makes lld difficult to implement.
> See https://bugs.llvm.org/show_bug.cgi?id=43083
> 
> Dropping `. =` turns out to be beneficial to older GNU ld as well
> because we can delete an ld check detecting "cannot move location
> counter backwards".
> 
> Signed-off-by: Fangrui Song 
> ---
>  scripts/layoutrom.py | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
> index 4c55390..94d4412 100755
> --- a/scripts/layoutrom.py
> +++ b/scripts/layoutrom.py
> @@ -339,14 +339,19 @@ def outRelSections(sections, startsym, useseg=0):
>  if section.finalloc is not None]
>  sections.sort(key=operator.itemgetter(0))
>  out = ""
> +location = "_reloc_init_end"
>  for addr, section in sections:
>  loc = section.finalloc
>  if useseg:
>  loc = section.finalsegloc
> -out += ". = ( 0x%x - %s ) ;\n" % (loc, startsym)
> +if location == "_reloc_init_end":
> +out += ". += 0x%x - %s ;\n" % (loc, location)
> +elif location < loc:
> +out += ". += 0x%x ;\n" % (loc-location,)
>  if section.name in ('.rodata.str1.1', '.rodata'):
>  out += "_rodata%s = . ;\n" % (section.fileid,)
>  out += "*%s.*(%s)\n" % (section.fileid, section.name)
> +location = loc + section.size

I'm finding this code confusing.  I would recommend "location" always
be a string or always be an integer - mixing them makes the code hard
to follow.  Also, this code removes the only reference to startsym,
but doesn't update the function signature.  Similarly, it hardcodes
"_reloc_init_end" when I think that should be passed to the function.

-Kevin


>  return out
>  
>  # Build linker script output for a list of relocations.
> -- 
> 2.26.0.rc2.310.g2932bb562d-goog
> ___
> SeaBIOS mailing list -- seabios@seabios.org
> To unsubscribe send an email to seabios-le...@seabios.org
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org


[SeaBIOS] Re: [PATCH 1/4] Make rom16.o linkable with lld

2020-04-06 Thread Fangrui Song

On 2020-04-06, Kevin O'Connor wrote:

On Wed, Apr 01, 2020 at 10:29:12AM -0700, Fangrui Song wrote:

(1) In romlayout.S, .fixedaddr.\addr sections do have not the SHF_ALLOC flag.
It does not make sense to reference a SHF_ALLOC section from a non-SHF_ALLOC 
section via R_386_PC16.
GNU ld allows it but lld will warn. Add the SHF_ALLOC flag.

(2) lld requires output section descriptions to be sorted by address.
 Just sort the addresses beforehand.


This looks like it should be two separate patches.

I know it's a pain to redo patches, but separating out changes helps
greatly when tracking down regressions via "git bisect".


I would hope [PATCH 2/4] Makefile: Change ET_EXEC to ET_REL so that lld can 
link bios.bin.elf
can be applied first it it is acceptable.

Like I said, these 1~3 have no dependency at all.
Redoing a patch series and resending as a whole may mess up the list.



Signed-off-by: Fangrui Song 
---
 scripts/layoutrom.py | 4 
 src/romlayout.S  | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
index 6616721..4c55390 100755
--- a/scripts/layoutrom.py
+++ b/scripts/layoutrom.py
@@ -321,6 +321,10 @@ def outXRefs(sections, useseg=0, exportsyms=[], 
forcedelta=0):

 # Write LD script includes for the given sections
 def outSections(sections, useseg=0):
+if useseg:
+sections.sort(key=lambda x: x.finalsegloc)
+else:
+sections.sort(key=lambda x: x.finalloc)


This looks odd to me - there shouldn't be a need to change the sort
order based on useseg, as finalloc should always have the same order
as finalsegloc.  Also, this code alters the input list which is
confusing - perhaps use "sections = sorted(sections, key=...)".


Just to confirm, I should use:

if not useseg:
sections = sorted(sections, key=lambda x: x.finalloc)



 out = ""
 for section in sections:
 loc = section.finalloc
diff --git a/src/romlayout.S b/src/romlayout.S
index c4a4635..a854783 100644
--- a/src/romlayout.S
+++ b/src/romlayout.S
@@ -587,7 +587,7 @@ entry_18:

 // Specify a location in the fixed part of bios area.
 .macro ORG addr
-.section .fixedaddr.\addr
+.section .fixedaddr.\addr,"a"
 .endm

 ORG 0xe05b
--
2.26.0.rc2.310.g2932bb562d-goog
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org

___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org


[SeaBIOS] [PATCH v3 4/5] romlayout32flag.lds: Use `. +=` instead of `. =`

2020-04-06 Thread Fangrui Song
This improves the portability of the linker script and allows lld to link rom.o

Dot assignment inside an output section has an inconsistent behavior
which makes lld difficult to implement.
See https://bugs.llvm.org/show_bug.cgi?id=43083

Dropping `. =` turns out to be beneficial to older GNU ld as well
because we can delete an ld check detecting "cannot move location
counter backwards".

--
Changes v2 -> v3
* Add `first` to avoid overloading `location`
* Delete startsym from the parameters

Signed-off-by: Fangrui Song 
---
 scripts/layoutrom.py | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
index caed387..caa182a 100755
--- a/scripts/layoutrom.py
+++ b/scripts/layoutrom.py
@@ -331,19 +331,25 @@ def outSections(sections, useseg=0):
 return out
 
 # Write LD script includes for the given sections using relative offsets
-def outRelSections(sections, startsym, useseg=0):
+def outRelSections(sections, useseg=0):
 sections = [(section.finalloc, section) for section in sections
 if section.finalloc is not None]
 sections.sort(key=operator.itemgetter(0))
 out = ""
+first = True
 for addr, section in sections:
 loc = section.finalloc
 if useseg:
 loc = section.finalsegloc
-out += ". = ( 0x%x - %s ) ;\n" % (loc, startsym)
+if first:
+out += ". += 0x%x - _reloc_init_end ;\n" % (loc,)
+first = False
+elif location != loc:
+out += ". += 0x%x ;\n" % (loc-location,)
 if section.name in ('.rodata.str1.1', '.rodata'):
 out += "_rodata%s = . ;\n" % (section.fileid,)
 out += "*%s.*(%s)\n" % (section.fileid, section.name)
+location = loc + section.size
 return out
 
 # Build linker script output for a list of relocations.
@@ -444,7 +450,7 @@ def writeLinkerScripts(li, out16, out32seg, out32flat):
sec32all_start,
multiboot_header,
relocstr,
-   outRelSections(li.sections, 'code32flat_start'))
+   outRelSections(li.sections))
 out = COMMONHEADER + out + COMMONTRAILER + """
 ENTRY(%s)
 PHDRS
-- 
2.26.0.292.g33ef6b2f38-goog
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org


[SeaBIOS] Re: [PATCH 7/9] acpi: add dsdt parser

2020-04-06 Thread Kevin O'Connor
On Fri, Apr 03, 2020 at 10:31:19AM +0200, Gerd Hoffmann wrote:
> Create a list of devices found in the DSDT table.  Add helper functions
> to find devices, walk the list and figure device informations like mmio
> ranges and irqs.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  src/util.h  |  10 +
>  src/fw/biostables.c | 622 
>  src/post.c  |   2 +
>  src/Kconfig |   7 +
>  4 files changed, 641 insertions(+)
> 
> diff --git a/src/util.h b/src/util.h
> index 4f27fc307439..8ee0370492b8 100644
> --- a/src/util.h
> +++ b/src/util.h
> @@ -94,6 +94,16 @@ void display_uuid(void);
>  void copy_table(void *pos);
>  void smbios_setup(void);
>  
> +struct acpi_device;
> +void acpi_dsdt_parse(void);
> +struct acpi_device *acpi_dsdt_find_string(struct acpi_device *prev, const 
> char *hid);
> +struct acpi_device *acpi_dsdt_find_eisaid(struct acpi_device *prev, u16 
> eisaid);
> +char *acpi_dsdt_name(struct acpi_device *dev);
> +int acpi_dsdt_present_eisaid(u16 eisaid);
> +int acpi_dsdt_find_io(struct acpi_device *dev, u64 *min, u64 *max);
> +int acpi_dsdt_find_mem(struct acpi_device *dev, u64 *min, u64 *max);
> +int acpi_dsdt_find_irq(struct acpi_device *dev, u64 *irq);
> +
>  // fw/coreboot.c
>  extern const char *CBvendor, *CBpart;
>  struct cbfs_file;
> diff --git a/src/fw/biostables.c b/src/fw/biostables.c
> index 0d4fdb9c22e8..ebe1c90fca5e 100644
> --- a/src/fw/biostables.c
> +++ b/src/fw/biostables.c
> @@ -17,6 +17,7 @@
>  #include "std/smbios.h" // struct smbios_entry_point
>  #include "string.h" // memcpy
>  #include "util.h" // copy_table
> +#include "list.h" // hlist_*
>  #include "x86.h" // outb
>  
>  struct pir_header *PirAddr VARFSEG;
> @@ -509,3 +510,624 @@ copy_table(void *pos)
>  copy_acpi_rsdp(pos);
>  copy_smbios(pos);
>  }
> +
> +/
> + * DSDT parser
> + /

I think this code is sufficiently large to demand it's own C file -
for example src/fw/dsdt_parser.c .

> +
> +struct acpi_device {
> +struct hlist_node node;
> +char name[16];
> +u8 *hid_aml;
> +u8 *sta_aml;
> +u8 *crs_data;
> +int crs_size;
> +};
> +static struct hlist_head acpi_devices;

It would be good to add VARVERIFY32INIT to this global.

> +
> +static int parse_error = 0;
> +static int parse_dumptree = 0;
> +static char parse_name[32];
> +static struct acpi_device *parse_dev;

I think it would be preferable to not use global variables for
temporary state.  I think the above could be moved into a new "struct
dsdt_parsing_state" and passed between functions.  (I suspect the "u8
*ptr" could be moved into that struct as well.)

> +
> +static void parse_termlist(u8 *ptr, int offset, int pkglength);

I'm a little concerned about the unbounded recursion in this parsing
code.  The main SeaBIOS execution stack is pretty large, but nothing
stops the dsdt table from doing something goofy.  I think a sanity
check on recursion depth may be worthwhile.

> +
> +static void hex(const u8 *ptr, int count, int lvl, const char *item)
> +{
> +int l = 0, i;
> +
> +do {
> +dprintf(lvl, "%s: %04x:  ", item, l);
> +for (i = l; i < l+16; i += 4)
> +dprintf(lvl, "%02x %02x %02x %02x  ",
> +ptr[i+0], ptr[i+1], ptr[i+2], ptr[i+3]);
> +for (i = l; i < l+16; i++)
> +dprintf(lvl, "%c", (ptr[i] > 0x20 && ptr[i] < 0x80) ? ptr[i] : 
> '.');
> +dprintf(lvl, "\n");
> +l += 16;
> +} while (l < count);
> +}
> +
> +static u64 parse_resource_int(u8 *ptr, int count)
> +{
> +u64 value = 0;
> +int index = 0;
> +
> +for (index = 0; index < count; index++)
> +value |= (u64)ptr[index] << (index * 8);
> +return value;
> +}
> +
> +static int parse_resource_bit(u8 *ptr, int count)
> +{
> +int bit;
> +
> +for (bit = 0; bit < count*8; bit++)
> +if (ptr[bit/8] & (1 << (bit%8)))
> +return bit;
> +return 0;
> +}
> +
> +static int parse_resource(u8 *ptr, int length, int *type, u64 *min, u64 *max)
> +{
> +int rname, rsize;
> +u64 len;
> +
> +*type = -1;
> +*min = 0;
> +*max = 0;
> +len = 0;
> +if (!(ptr[0] & 0x80)) {
> +/* small resource */
> +rname = (ptr[0] >> 3) & 0x0f;
> +rsize = ptr[0] & 0x07;
> +rsize++;
> +switch (rname) {
> +case 0x04: /* irq */
> +*min = parse_resource_bit(ptr + 1, rsize);
> +*max = *min;
> +*type = 3;
> +break;
> +case 0x0f: /* end marker */
> +return 0;
> +case 0x08: /* io */
> +*min = parse_resource_int(ptr + 2, 2);
> +*max = parse_resource_int(ptr + 4, 2);
> +if (*min == *max) {
> +*max = *min + ptr[7] - 1;
> +*type = 1;
> +}
> +break;
> +case 

[SeaBIOS] Re: [PATCH 4/4] test-build.sh: Delete unneeded LD capability test

2020-04-06 Thread Fangrui Song

On 2020-04-06, Kevin O'Connor wrote:

On Wed, Apr 01, 2020 at 10:29:15AM -0700, Fangrui Song wrote:

The previous commit changed romlayout32flag.lds to use `. += ` instead
of `. =`.
We no longer need the LD capability test checking
https://sourceware.org/bugzilla/show_bug.cgi?id=12726


Thanks.  It would be great to remove this test.  Were you able to
bring up that old version of Ubuntu and verify that it produces
bootable code (or definitely errors) with the new code though?  I'd
like to make sure the new code doesn't result in silently broken code.

-Kevin


I cannot test it easily. Can you do that for me, please?





Signed-off-by: Fangrui Song 
---
 scripts/test-build.sh | 42 +-
 1 file changed, 1 insertion(+), 41 deletions(-)

diff --git a/scripts/test-build.sh b/scripts/test-build.sh
index 25cc2f2..8b35d6f 100755
--- a/scripts/test-build.sh
+++ b/scripts/test-build.sh
@@ -4,50 +4,10 @@
 mkdir -p ${OUT}
 TMPFILE1=${OUT}/tmp_testcompile1.c
 TMPFILE1o=${OUT}/tmp_testcompile1.o
-TMPFILE1_ld=${OUT}/tmp_testcompile1.lds
 TMPFILE2=${OUT}/tmp_testcompile2.c
 TMPFILE2o=${OUT}/tmp_testcompile2.o
 TMPFILE3o=${OUT}/tmp_testcompile3.o

-# Test if ld's alignment handling is correct.  This is a known problem
-# with the linker that ships with Ubuntu 11.04.
-cat - > $TMPFILE1 < $TMPFILE1_ld < /dev/null 2>&1
-if [ $? -ne 0 ]; then
-echo "Unable to execute the C compiler ($CC)." >&2
-echo "" >&2
-echo "Please install a working compiler and retry." >&2
-echo -1
-exit 0
-fi
-$LD -T $TMPFILE1_ld $TMPFILE1o -o $TMPFILE2o > /dev/null 2>&1
-if [ $? -ne 0 ]; then
-echo "The version of LD on this system ($LD) does not properly handle" >&2
-echo "alignments.  As a result, this project can not be built." >&2
-echo "" >&2
-echo "The problem may be the result of this LD bug report:" >&2
-echo " http://sourceware.org/bugzilla/show_bug.cgi?id=12726; >&2
-echo "" >&2
-echo "Please update to a working version of binutils and retry." >&2
-echo -1
-exit 0
-fi
-
 # Test for "-fwhole-program".  Older versions of gcc (pre v4.1) don't
 # support the whole-program optimization - detect that.
 $CC -fwhole-program -S -o /dev/null -xc /dev/null > /dev/null 2>&1
@@ -87,4 +47,4 @@ echo 0
 # "ebp" register is clobberred in an "asm" statement.  The code has
 # been modified to not clobber "ebp" - no test is available yet.

-rm -f $TMPFILE1 $TMPFILE1o $TMPFILE1_ld $TMPFILE2 $TMPFILE2o $TMPFILE3o
+rm -f $TMPFILE1 $TMPFILE1o $TMPFILE2 $TMPFILE2o $TMPFILE3o
--
2.26.0.rc2.310.g2932bb562d-goog
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org

___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org


[SeaBIOS] Re: [PATCH 2/4] Makefile: Change ET_EXEC to ET_REL so that lld can link bios.bin.elf

2020-04-06 Thread Fangrui Song

On 2020-04-06, Kevin O'Connor wrote:

On Wed, Apr 01, 2020 at 10:29:13AM -0700, Fangrui Song wrote:

Accepting ET_EXEC as an input file is an extremely rare GNU ld feature
that lld does not intend to support, because this is error-prone.

See Linux kernel commit 90ceddcb495008ac8ba7a3dce297841efcd7d584 for
another use of the dd trick.

--
Changes v1 -> v2
* Add status=none to the dd command line. This suppresses dd's stderr output.
* Move dd command to a separate command

Signed-off-by: Fangrui Song 
---
 Makefile | 4 
 1 file changed, 4 insertions(+)

diff --git a/Makefile b/Makefile
index 5f7d537..118dec0 100644
--- a/Makefile
+++ b/Makefile
@@ -177,10 +177,14 @@ $(OUT)romlayout32seg.lds $(OUT)romlayout32flat.lds 
$(OUT)code32flat.o $(OUT)code
 $(OUT)rom16.o: $(OUT)code16.o $(OUT)romlayout16.lds
@echo "  Linking $@"
$(Q)$(LD) -T $(OUT)romlayout16.lds $< -o $@
+   # Change e_type to ET_REL so that it can be used to link rom.o.
+   # Unlike GNU ld, lld does not allow an ET_EXEC input.
+   printf '\1' | dd of=$@ conv=notrunc bs=1 seek=16 status=none

 $(OUT)rom32seg.o: $(OUT)code32seg.o $(OUT)romlayout32seg.lds
@echo "  Linking $@"
$(Q)$(LD) -T $(OUT)romlayout32seg.lds $< -o $@
+   printf '\1' | dd of=$@ conv=notrunc bs=1 seek=16 status=none


My high-level feedback is that the above is very fragile.  I'd be
reluctant to adopt that hack.  What is the underlying issue that needs
to be addressed?

-Kevin


lld does not take ET_EXEC as input. This is a deliberate choice.
GNU gold does not accept ET_EXEC as well:

 % gold a
 gold: error: a: unsupported ELF file type 2

1 ET_REL represents object files (.o)
2 ET_EXEC represents position-dependent executables.
3 ET_DYN represents shared objects (.so) (or PIE; for linking purposes, PIE 
cannot be accepted)
4 ET_CORE represents core files. For linking purposes, they cannot be accepted.

I don't know how GNU ld ends up accepting ET_EXEC. I am not even sure it
is an intentional decision. A lot of sections will not be meaningful
to the linker and accidentally mixing an ET_EXEC can likely lead to
hard-to-debug linking issues.

I made a similar change to Linux recently.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=90ceddcb495008ac8ba7a3dce297841efcd7d584



 $(OUT)rom.o: $(OUT)rom16.strip.o $(OUT)rom32seg.strip.o $(OUT)code32flat.o 
$(OUT)romlayout32flat.lds
@echo "  Linking $@"
--
2.26.0.rc2.310.g2932bb562d-goog
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org

___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org


[SeaBIOS] Re: [PATCH 8/9] acpi: skip kbd init if not present

2020-04-06 Thread Kevin O'Connor
On Fri, Apr 03, 2020 at 10:31:20AM +0200, Gerd Hoffmann wrote:
> Don't initialize the ps/2 keyboard in case the device is not
> listed in the ACPi DSDT table.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  src/hw/ps2port.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/hw/ps2port.c b/src/hw/ps2port.c
> index 2c334c06b7eb..c82521b42e16 100644
> --- a/src/hw/ps2port.c
> +++ b/src/hw/ps2port.c
> @@ -542,6 +542,10 @@ ps2port_setup(void)
>  ASSERT32FLAT();
>  if (! CONFIG_PS2PORT)
>  return;
> +if (acpi_dsdt_present_eisaid(0x0303) == 0) {
> +dprintf(1, "ACPI: no PS/2 keyboard present\n");
> +return;
> +}

Unless I'm missing something, if the dsdt parser is enabled, but the
dsdt is not actually found, this would turn off the keyboard.  I think
it should only disable the keyboard detection if the dsdt is actually
found and doesn't contain a ps2 port entry.

-Kevin


>  dprintf(3, "init ps2port\n");
>  
>  enable_hwirq(1, FUNC16(entry_09));
> -- 
> 2.18.2
> ___
> SeaBIOS mailing list -- seabios@seabios.org
> To unsubscribe send an email to seabios-le...@seabios.org
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org


[SeaBIOS] Re: [PATCH 2/4] Makefile: Change ET_EXEC to ET_REL so that lld can link bios.bin.elf

2020-04-06 Thread Kevin O'Connor
On Wed, Apr 01, 2020 at 10:29:13AM -0700, Fangrui Song wrote:
> Accepting ET_EXEC as an input file is an extremely rare GNU ld feature
> that lld does not intend to support, because this is error-prone.
> 
> See Linux kernel commit 90ceddcb495008ac8ba7a3dce297841efcd7d584 for
> another use of the dd trick.
> 
> --
> Changes v1 -> v2
> * Add status=none to the dd command line. This suppresses dd's stderr output.
> * Move dd command to a separate command
> 
> Signed-off-by: Fangrui Song 
> ---
>  Makefile | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 5f7d537..118dec0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -177,10 +177,14 @@ $(OUT)romlayout32seg.lds $(OUT)romlayout32flat.lds 
> $(OUT)code32flat.o $(OUT)code
>  $(OUT)rom16.o: $(OUT)code16.o $(OUT)romlayout16.lds
>   @echo "  Linking $@"
>   $(Q)$(LD) -T $(OUT)romlayout16.lds $< -o $@
> + # Change e_type to ET_REL so that it can be used to link rom.o.
> + # Unlike GNU ld, lld does not allow an ET_EXEC input.
> + printf '\1' | dd of=$@ conv=notrunc bs=1 seek=16 status=none
>  
>  $(OUT)rom32seg.o: $(OUT)code32seg.o $(OUT)romlayout32seg.lds
>   @echo "  Linking $@"
>   $(Q)$(LD) -T $(OUT)romlayout32seg.lds $< -o $@
> + printf '\1' | dd of=$@ conv=notrunc bs=1 seek=16 status=none

My high-level feedback is that the above is very fragile.  I'd be
reluctant to adopt that hack.  What is the underlying issue that needs
to be addressed?

-Kevin


>  
>  $(OUT)rom.o: $(OUT)rom16.strip.o $(OUT)rom32seg.strip.o $(OUT)code32flat.o 
> $(OUT)romlayout32flat.lds
>   @echo "  Linking $@"
> -- 
> 2.26.0.rc2.310.g2932bb562d-goog
> ___
> SeaBIOS mailing list -- seabios@seabios.org
> To unsubscribe send an email to seabios-le...@seabios.org
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org


[SeaBIOS] Re: [PATCH 4/4] test-build.sh: Delete unneeded LD capability test

2020-04-06 Thread Kevin O'Connor
On Wed, Apr 01, 2020 at 10:29:15AM -0700, Fangrui Song wrote:
> The previous commit changed romlayout32flag.lds to use `. += ` instead
> of `. =`.
> We no longer need the LD capability test checking
> https://sourceware.org/bugzilla/show_bug.cgi?id=12726

Thanks.  It would be great to remove this test.  Were you able to
bring up that old version of Ubuntu and verify that it produces
bootable code (or definitely errors) with the new code though?  I'd
like to make sure the new code doesn't result in silently broken code.

-Kevin


> 
> Signed-off-by: Fangrui Song 
> ---
>  scripts/test-build.sh | 42 +-
>  1 file changed, 1 insertion(+), 41 deletions(-)
> 
> diff --git a/scripts/test-build.sh b/scripts/test-build.sh
> index 25cc2f2..8b35d6f 100755
> --- a/scripts/test-build.sh
> +++ b/scripts/test-build.sh
> @@ -4,50 +4,10 @@
>  mkdir -p ${OUT}
>  TMPFILE1=${OUT}/tmp_testcompile1.c
>  TMPFILE1o=${OUT}/tmp_testcompile1.o
> -TMPFILE1_ld=${OUT}/tmp_testcompile1.lds
>  TMPFILE2=${OUT}/tmp_testcompile2.c
>  TMPFILE2o=${OUT}/tmp_testcompile2.o
>  TMPFILE3o=${OUT}/tmp_testcompile3.o
>  
> -# Test if ld's alignment handling is correct.  This is a known problem
> -# with the linker that ships with Ubuntu 11.04.
> -cat - > $TMPFILE1 < -const char v1[] __attribute__((section(".text.v1"))) = "0123456789";
> -const char v2[] __attribute__((section(".text.v2"))) = "0123456789";
> -EOF
> -cat - > $TMPFILE1_ld < -SECTIONS
> -{
> - .mysection 0x88f0 : {
> -. = 0x10 ;
> -*(.text.v1)
> -. = 0x20 ;
> -*(.text.v2)
> -. = 0x30 ;
> - }
> -}
> -EOF
> -$CC -O -g -c $TMPFILE1 -o $TMPFILE1o > /dev/null 2>&1
> -if [ $? -ne 0 ]; then
> -echo "Unable to execute the C compiler ($CC)." >&2
> -echo "" >&2
> -echo "Please install a working compiler and retry." >&2
> -echo -1
> -exit 0
> -fi
> -$LD -T $TMPFILE1_ld $TMPFILE1o -o $TMPFILE2o > /dev/null 2>&1
> -if [ $? -ne 0 ]; then
> -echo "The version of LD on this system ($LD) does not properly handle" 
> >&2
> -echo "alignments.  As a result, this project can not be built." >&2
> -echo "" >&2
> -echo "The problem may be the result of this LD bug report:" >&2
> -echo " http://sourceware.org/bugzilla/show_bug.cgi?id=12726; >&2
> -echo "" >&2
> -echo "Please update to a working version of binutils and retry." >&2
> -echo -1
> -exit 0
> -fi
> -
>  # Test for "-fwhole-program".  Older versions of gcc (pre v4.1) don't
>  # support the whole-program optimization - detect that.
>  $CC -fwhole-program -S -o /dev/null -xc /dev/null > /dev/null 2>&1
> @@ -87,4 +47,4 @@ echo 0
>  # "ebp" register is clobberred in an "asm" statement.  The code has
>  # been modified to not clobber "ebp" - no test is available yet.
>  
> -rm -f $TMPFILE1 $TMPFILE1o $TMPFILE1_ld $TMPFILE2 $TMPFILE2o $TMPFILE3o
> +rm -f $TMPFILE1 $TMPFILE1o $TMPFILE2 $TMPFILE2o $TMPFILE3o
> -- 
> 2.26.0.rc2.310.g2932bb562d-goog
> ___
> SeaBIOS mailing list -- seabios@seabios.org
> To unsubscribe send an email to seabios-le...@seabios.org
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org


[SeaBIOS] Re: [PATCH 1/4] Make rom16.o linkable with lld

2020-04-06 Thread Kevin O'Connor
On Mon, Apr 06, 2020 at 05:26:35PM -0700, Fangrui Song wrote:
> On 2020-04-06, Kevin O'Connor wrote:
> > On Wed, Apr 01, 2020 at 10:29:12AM -0700, Fangrui Song wrote:
> > > (1) In romlayout.S, .fixedaddr.\addr sections do have not the SHF_ALLOC 
> > > flag.
> > > It does not make sense to reference a SHF_ALLOC section from a 
> > > non-SHF_ALLOC section via R_386_PC16.
> > > GNU ld allows it but lld will warn. Add the SHF_ALLOC flag.
> > > 
> > > (2) lld requires output section descriptions to be sorted by address.
> > >  Just sort the addresses beforehand.
> > 
> > This looks like it should be two separate patches.
> > 
> > I know it's a pain to redo patches, but separating out changes helps
> > greatly when tracking down regressions via "git bisect".
> 
> I would hope [PATCH 2/4] Makefile: Change ET_EXEC to ET_REL so that lld can 
> link bios.bin.elf
> can be applied first it it is acceptable.
> 
> Like I said, these 1~3 have no dependency at all.
> Redoing a patch series and resending as a whole may mess up the list.
> 
> > > 
> > > Signed-off-by: Fangrui Song 
> > > ---
> > >  scripts/layoutrom.py | 4 
> > >  src/romlayout.S  | 2 +-
> > >  2 files changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
> > > index 6616721..4c55390 100755
> > > --- a/scripts/layoutrom.py
> > > +++ b/scripts/layoutrom.py
> > > @@ -321,6 +321,10 @@ def outXRefs(sections, useseg=0, exportsyms=[], 
> > > forcedelta=0):
> > > 
> > >  # Write LD script includes for the given sections
> > >  def outSections(sections, useseg=0):
> > > +if useseg:
> > > +sections.sort(key=lambda x: x.finalsegloc)
> > > +else:
> > > +sections.sort(key=lambda x: x.finalloc)
> > 
> > This looks odd to me - there shouldn't be a need to change the sort
> > order based on useseg, as finalloc should always have the same order
> > as finalsegloc.  Also, this code alters the input list which is
> > confusing - perhaps use "sections = sorted(sections, key=...)".
> 
> Just to confirm, I should use:
> 
> if not useseg:
> sections = sorted(sections, key=lambda x: x.finalloc)

I was proposing to unconditionally use:
  sections = sorted(sections, key=lambda x: x.finalloc)

-Kevin


> 
> 
> > >  out = ""
> > >  for section in sections:
> > >  loc = section.finalloc
> > > diff --git a/src/romlayout.S b/src/romlayout.S
> > > index c4a4635..a854783 100644
> > > --- a/src/romlayout.S
> > > +++ b/src/romlayout.S
> > > @@ -587,7 +587,7 @@ entry_18:
> > > 
> > >  // Specify a location in the fixed part of bios area.
> > >  .macro ORG addr
> > > -.section .fixedaddr.\addr
> > > +.section .fixedaddr.\addr,"a"
> > >  .endm
> > > 
> > >  ORG 0xe05b
> > > --
> > > 2.26.0.rc2.310.g2932bb562d-goog
> > > ___
> > > SeaBIOS mailing list -- seabios@seabios.org
> > > To unsubscribe send an email to seabios-le...@seabios.org
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org


[SeaBIOS] Re: [PATCH 3/4] romlayout32flag.lds: Use `. +=` instead of `. =`

2020-04-06 Thread Fangrui Song

On 2020-04-06, Kevin O'Connor wrote:

On Wed, Apr 01, 2020 at 10:29:14AM -0700, Fangrui Song wrote:

This improves the portability of the linker script and allows lld to link rom.o

Dot assignment inside an output section has an inconsistent behavior
which makes lld difficult to implement.
See https://bugs.llvm.org/show_bug.cgi?id=43083

Dropping `. =` turns out to be beneficial to older GNU ld as well
because we can delete an ld check detecting "cannot move location
counter backwards".

Signed-off-by: Fangrui Song 
---
 scripts/layoutrom.py | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
index 4c55390..94d4412 100755
--- a/scripts/layoutrom.py
+++ b/scripts/layoutrom.py
@@ -339,14 +339,19 @@ def outRelSections(sections, startsym, useseg=0):
 if section.finalloc is not None]
 sections.sort(key=operator.itemgetter(0))
 out = ""
+location = "_reloc_init_end"
 for addr, section in sections:
 loc = section.finalloc
 if useseg:
 loc = section.finalsegloc
-out += ". = ( 0x%x - %s ) ;\n" % (loc, startsym)
+if location == "_reloc_init_end":
+out += ". += 0x%x - %s ;\n" % (loc, location)
+elif location < loc:
+out += ". += 0x%x ;\n" % (loc-location,)
 if section.name in ('.rodata.str1.1', '.rodata'):
 out += "_rodata%s = . ;\n" % (section.fileid,)
 out += "*%s.*(%s)\n" % (section.fileid, section.name)
+location = loc + section.size


I'm finding this code confusing.  I would recommend "location" always
be a string or always be an integer - mixing them makes the code hard
to follow.  Also, this code removes the only reference to startsym,
but doesn't update the function signature.  Similarly, it hardcodes
"_reloc_init_end" when I think that should be passed to the function.

-Kevin


Thanks. Addressed in [PATCH v3 4/5] romlayout32flag.lds: Use `. +=` instead of 
`. =`

Non-zero useseg does not appear to be used, but I don't understand it
enough to fix it.




 return out

 # Build linker script output for a list of relocations.
--
2.26.0.rc2.310.g2932bb562d-goog
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org

___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org


[SeaBIOS] Re: [PATCH 1/9] virtio-mmio: device registry

2020-04-06 Thread Kevin O'Connor
On Fri, Apr 03, 2020 at 10:31:13AM +0200, Gerd Hoffmann wrote:
> Add a new virtio-mmio.c source file, providing a function
> to register virtio-mmio devices.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  Makefile |  2 +-
>  src/hw/virtio-mmio.h |  6 ++
>  src/hw/virtio-mmio.c | 30 ++
>  3 files changed, 37 insertions(+), 1 deletion(-)
>  create mode 100644 src/hw/virtio-mmio.h
>  create mode 100644 src/hw/virtio-mmio.c
> 
> diff --git a/Makefile b/Makefile
> index 5f7d5370198a..985ef591a13b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -43,7 +43,7 @@ SRC32FLAT=$(SRCBOTH) post.c e820map.c malloc.c romfile.c 
> x86.c optionroms.c \
>  fw/coreboot.c fw/lzmadecode.c fw/multiboot.c fw/csm.c fw/biostables.c \
>  fw/paravirt.c fw/shadow.c fw/pciinit.c fw/smm.c fw/smp.c fw/mtrr.c 
> fw/xen.c \
>  fw/acpi.c fw/mptable.c fw/pirtable.c fw/smbios.c fw/romfile_loader.c \
> -hw/virtio-ring.c hw/virtio-pci.c hw/virtio-blk.c hw/virtio-scsi.c \
> +hw/virtio-ring.c hw/virtio-pci.c hw/virtio-mmio.c hw/virtio-blk.c 
> hw/virtio-scsi.c \
>  hw/tpm_drivers.c hw/nvme.c
>  SRC32SEG=string.c output.c pcibios.c apm.c stacks.c hw/pci.c hw/serialio.c
>  DIRS=src src/hw src/fw vgasrc
> diff --git a/src/hw/virtio-mmio.h b/src/hw/virtio-mmio.h
> new file mode 100644
> index ..751984241f49
> --- /dev/null
> +++ b/src/hw/virtio-mmio.h
> @@ -0,0 +1,6 @@
> +#ifndef _VIRTIO_MMIO_H
> +#define _VIRTIO_MMIO_H
> +
> +void virtio_mmio_register(u64 mmio);
> +
> +#endif /* _VIRTIO_MMIO_H */
> diff --git a/src/hw/virtio-mmio.c b/src/hw/virtio-mmio.c
> new file mode 100644
> index ..6c969d787ec7
> --- /dev/null
> +++ b/src/hw/virtio-mmio.c
> @@ -0,0 +1,30 @@
> +#include "config.h" // CONFIG_DEBUG_LEVEL
> +#include "malloc.h" // free
> +#include "output.h" // dprintf
> +#include "virtio-pci.h"
> +#include "virtio-ring.h"
> +
> +/* qemu microvm supports 8 virtio-mmio devices */
> +static u64 devs[8];

It would be preferable to avoid using global variables for temporary
state.  Because of all the weird linker rules and segment rules, the
use of global variables is conceptually harder in SeaBIOS.

If I understand this patch series correctly, the ultimate result is an
acpi parser that walks the dsdt and calls virtio_mmio_register().
Could that code directly launch the appropriate hardware registration
directly?

-Kevin


> +
> +void virtio_mmio_register(u64 mmio)
> +{
> +int i;
> +
> +for (i = 0; i < ARRAY_SIZE(devs); i++) {
> +if (devs[i] == mmio) {
> +/*
> + * This can happen in case we have multiple scsi devices
> + * attached to a single virtio-scsi controller
> + */
> +dprintf(3, "virtio-mmio: duplicate device at 0x%llx, 
> ignoring\n", mmio);
> +return;
> +}
> +if (devs[i] == 0) {
> +dprintf(3, "virtio-mmio: register device at 0x%llx\n", mmio);
> +devs[i] = mmio;
> +return;
> +}
> +}
> +dprintf(1, "virtio-mmio: device list full\n");
> +}
> -- 
> 2.18.2
> ___
> SeaBIOS mailing list -- seabios@seabios.org
> To unsubscribe send an email to seabios-le...@seabios.org
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org


[SeaBIOS] Re: [PATCH v2] Use readelf -WSrs instead of objdump -thr so that llvm-readelf can be used as a replacement

2020-04-06 Thread Kevin O'Connor
On Wed, Apr 01, 2020 at 06:12:09PM -0700, Fangrui Song wrote:
> objdump -h relies heavily on BFD internals and the BFD flags are difficult to 
> emulate in llvm-objdump.
> llvm-objdump -h has a different output (https://reviews.llvm.org/D57146)
> 
> Switch to readelf, which is generally better than objdump when dumping ELF 
> section/symbol information.
> 
> Signed-off-by: Fangrui Song 

At a high-level, this change makes sense to me.  What Linux
distros/versions have you tested this on?  (Or, do you have reason to
believe the readelf output has been stable for many years?)

-Kevin
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org


[SeaBIOS] Re: [PATCH 1/4] Make rom16.o linkable with lld

2020-04-06 Thread Kevin O'Connor
On Wed, Apr 01, 2020 at 10:29:12AM -0700, Fangrui Song wrote:
> (1) In romlayout.S, .fixedaddr.\addr sections do have not the SHF_ALLOC flag.
> It does not make sense to reference a SHF_ALLOC section from a non-SHF_ALLOC 
> section via R_386_PC16.
> GNU ld allows it but lld will warn. Add the SHF_ALLOC flag.
> 
> (2) lld requires output section descriptions to be sorted by address.
>  Just sort the addresses beforehand.

This looks like it should be two separate patches.

I know it's a pain to redo patches, but separating out changes helps
greatly when tracking down regressions via "git bisect".

> 
> Signed-off-by: Fangrui Song 
> ---
>  scripts/layoutrom.py | 4 
>  src/romlayout.S  | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
> index 6616721..4c55390 100755
> --- a/scripts/layoutrom.py
> +++ b/scripts/layoutrom.py
> @@ -321,6 +321,10 @@ def outXRefs(sections, useseg=0, exportsyms=[], 
> forcedelta=0):
>  
>  # Write LD script includes for the given sections
>  def outSections(sections, useseg=0):
> +if useseg:
> +sections.sort(key=lambda x: x.finalsegloc)
> +else:
> +sections.sort(key=lambda x: x.finalloc)

This looks odd to me - there shouldn't be a need to change the sort
order based on useseg, as finalloc should always have the same order
as finalsegloc.  Also, this code alters the input list which is
confusing - perhaps use "sections = sorted(sections, key=...)".

>  out = ""
>  for section in sections:
>  loc = section.finalloc
> diff --git a/src/romlayout.S b/src/romlayout.S
> index c4a4635..a854783 100644
> --- a/src/romlayout.S
> +++ b/src/romlayout.S
> @@ -587,7 +587,7 @@ entry_18:
>  
>  // Specify a location in the fixed part of bios area.
>  .macro ORG addr
> -.section .fixedaddr.\addr
> +.section .fixedaddr.\addr,"a"
>  .endm
>  
>  ORG 0xe05b
> -- 
> 2.26.0.rc2.310.g2932bb562d-goog
> ___
> SeaBIOS mailing list -- seabios@seabios.org
> To unsubscribe send an email to seabios-le...@seabios.org
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org


[SeaBIOS] Re: [PATCH v2] Use readelf -WSrs instead of objdump -thr so that llvm-readelf can be used as a replacement

2020-04-06 Thread Fangrui Song

On 2020-04-06, Kevin O'Connor wrote:

On Wed, Apr 01, 2020 at 06:12:09PM -0700, Fangrui Song wrote:

objdump -h relies heavily on BFD internals and the BFD flags are difficult to 
emulate in llvm-objdump.
llvm-objdump -h has a different output (https://reviews.llvm.org/D57146)

Switch to readelf, which is generally better than objdump when dumping ELF 
section/symbol information.

Signed-off-by: Fangrui Song 


At a high-level, this change makes sense to me.  What Linux
distros/versions have you tested this on?  (Or, do you have reason to
believe the readelf output has been stable for many years?)

-Kevin


Tested on Debian testing.

readelf -S -r -s output is stable. It just prints raw ELF fields.
objdump -t -h -r output is not. The output goes through various BFD 
abstractions.
 "CONTENTS, ALLOC, LOAD, READONLY, DATA" -> these are really BFD_* constants.
  The genuine ELF values are SHT_PROGBITS, SHF_ALLOC, non-SHF_WRITE, etc.

  The LMA column is not really recorded in ELF. objdump internally does
  some translation like sh_addr-p_vaddr+p_paddr.

I made llvm-objdump's output closer to objdump as long as it is reasonable.
https://github.com/llvm/llvm-project/commits/master/llvm/tools/llvm-objdump/llvm-objdump.cpp?author=maskray
These BFD flags may be difficult to implement and may not provide enough value.
I actually claim myself as a maintainer of llvm-{readobj,objdump,...}
and have reported many binutils issues as well.

llvm-readelf, on the other side, has great compatibility with GNU readelf.
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org


[SeaBIOS] [PATCH v3 3/5] Makefile: Change ET_EXEC to ET_REL so that lld can link bios.bin.elf

2020-04-06 Thread Fangrui Song
Accepting ET_EXEC as an input file is an extremely rare GNU ld feature
that lld does not intend to support, because this is error-prone.

See Linux kernel commit 90ceddcb495008ac8ba7a3dce297841efcd7d584 for
another use of the dd trick.

--
Changes v1 -> v2
* Add status=none to the dd command line. This suppresses dd's stderr output.
* Move dd command to a separate command

Signed-off-by: Fangrui Song 
---
 Makefile | 4 
 1 file changed, 4 insertions(+)

diff --git a/Makefile b/Makefile
index 5f7d537..118dec0 100644
--- a/Makefile
+++ b/Makefile
@@ -177,10 +177,14 @@ $(OUT)romlayout32seg.lds $(OUT)romlayout32flat.lds 
$(OUT)code32flat.o $(OUT)code
 $(OUT)rom16.o: $(OUT)code16.o $(OUT)romlayout16.lds
@echo "  Linking $@"
$(Q)$(LD) -T $(OUT)romlayout16.lds $< -o $@
+   # Change e_type to ET_REL so that it can be used to link rom.o.
+   # Unlike GNU ld, lld does not allow an ET_EXEC input.
+   printf '\1' | dd of=$@ conv=notrunc bs=1 seek=16 status=none
 
 $(OUT)rom32seg.o: $(OUT)code32seg.o $(OUT)romlayout32seg.lds
@echo "  Linking $@"
$(Q)$(LD) -T $(OUT)romlayout32seg.lds $< -o $@
+   printf '\1' | dd of=$@ conv=notrunc bs=1 seek=16 status=none
 
 $(OUT)rom.o: $(OUT)rom16.strip.o $(OUT)rom32seg.strip.o $(OUT)code32flat.o 
$(OUT)romlayout32flat.lds
@echo "  Linking $@"
-- 
2.26.0.292.g33ef6b2f38-goog
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org


[SeaBIOS] [PATCH v3 0/5] Make seabios linkable with lld

2020-04-06 Thread Fangrui Song
This patch series make seabios linkable with lld.

This is beneficial for FreeBSD ports as well
https://svnweb.freebsd.org/ports/head/misc/seabios/Makefile
as they can drop an LLD_UNSAFE

As a maintainer of lld ELF, I have triaged numerous pieces of software.
seabios is the only one making use of 
This drops the only instance 
https://sourceware.org/bugzilla/show_bug.cgi?id=12726

1, 2, 3 and 4 are really independent and can be applied in arbitrary order.
5 depends on 4. I originally combined 4 and 5 and that was why I don't think a 
patch series made sense.


Fangrui Song (5):
  romlayout.S: Add missing SHF_ALLOC flag to .fixedaddr.\addr
  Make rom16.o linkable with lld
  Makefile: Change ET_EXEC to ET_REL so that lld can link bios.bin.elf
  romlayout32flag.lds: Use `. +=` instead of `. =`
  test-build.sh: Delete unneeded LD capability test

 Makefile  |  4 
 scripts/layoutrom.py  | 13 ++---
 scripts/test-build.sh | 42 +-
 src/romlayout.S   |  2 +-
 4 files changed, 16 insertions(+), 45 deletions(-)

-- 
2.26.0.292.g33ef6b2f38-goog
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org


[SeaBIOS] [PATCH v3 5/5] test-build.sh: Delete unneeded LD capability test

2020-04-06 Thread Fangrui Song
The previous commit changed romlayout32flag.lds to use `. += ` instead
of `. =`.
We no longer need the LD capability test checking
https://sourceware.org/bugzilla/show_bug.cgi?id=12726

Signed-off-by: Fangrui Song 
---
 scripts/test-build.sh | 42 +-
 1 file changed, 1 insertion(+), 41 deletions(-)

diff --git a/scripts/test-build.sh b/scripts/test-build.sh
index 25cc2f2..8b35d6f 100755
--- a/scripts/test-build.sh
+++ b/scripts/test-build.sh
@@ -4,50 +4,10 @@
 mkdir -p ${OUT}
 TMPFILE1=${OUT}/tmp_testcompile1.c
 TMPFILE1o=${OUT}/tmp_testcompile1.o
-TMPFILE1_ld=${OUT}/tmp_testcompile1.lds
 TMPFILE2=${OUT}/tmp_testcompile2.c
 TMPFILE2o=${OUT}/tmp_testcompile2.o
 TMPFILE3o=${OUT}/tmp_testcompile3.o
 
-# Test if ld's alignment handling is correct.  This is a known problem
-# with the linker that ships with Ubuntu 11.04.
-cat - > $TMPFILE1 < $TMPFILE1_ld < /dev/null 2>&1
-if [ $? -ne 0 ]; then
-echo "Unable to execute the C compiler ($CC)." >&2
-echo "" >&2
-echo "Please install a working compiler and retry." >&2
-echo -1
-exit 0
-fi
-$LD -T $TMPFILE1_ld $TMPFILE1o -o $TMPFILE2o > /dev/null 2>&1
-if [ $? -ne 0 ]; then
-echo "The version of LD on this system ($LD) does not properly handle" >&2
-echo "alignments.  As a result, this project can not be built." >&2
-echo "" >&2
-echo "The problem may be the result of this LD bug report:" >&2
-echo " http://sourceware.org/bugzilla/show_bug.cgi?id=12726; >&2
-echo "" >&2
-echo "Please update to a working version of binutils and retry." >&2
-echo -1
-exit 0
-fi
-
 # Test for "-fwhole-program".  Older versions of gcc (pre v4.1) don't
 # support the whole-program optimization - detect that.
 $CC -fwhole-program -S -o /dev/null -xc /dev/null > /dev/null 2>&1
@@ -87,4 +47,4 @@ echo 0
 # "ebp" register is clobberred in an "asm" statement.  The code has
 # been modified to not clobber "ebp" - no test is available yet.
 
-rm -f $TMPFILE1 $TMPFILE1o $TMPFILE1_ld $TMPFILE2 $TMPFILE2o $TMPFILE3o
+rm -f $TMPFILE1 $TMPFILE1o $TMPFILE2 $TMPFILE2o $TMPFILE3o
-- 
2.26.0.292.g33ef6b2f38-goog
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org


[SeaBIOS] [PATCH v3 2/5] Make rom16.o linkable with lld

2020-04-06 Thread Fangrui Song
lld requires output section descriptions to be sorted by address.
Just sort the addresses beforehand.

--
Changes v2 -> v3
* Sort sections by finalloc unconditionally

Signed-off-by: Fangrui Song 
---
 scripts/layoutrom.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
index 6616721..caed387 100755
--- a/scripts/layoutrom.py
+++ b/scripts/layoutrom.py
@@ -321,6 +321,7 @@ def outXRefs(sections, useseg=0, exportsyms=[], 
forcedelta=0):
 
 # Write LD script includes for the given sections
 def outSections(sections, useseg=0):
+sections = sorted(sections, key=lambda x: x.finalloc)
 out = ""
 for section in sections:
 loc = section.finalloc
-- 
2.26.0.292.g33ef6b2f38-goog
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org


[SeaBIOS] [PATCH v3 1/5] romlayout.S: Add missing SHF_ALLOC flag to .fixedaddr.\addr

2020-04-06 Thread Fangrui Song
It does not make sense to reference a SHF_ALLOC section from a
non-SHF_ALLOC section via R_386_PC16. Conceptually, even if a
non-SHF_ALLOC is loaded as part of the memory image, the distance
between it and a SHF_ALLOC section may not be a constant, so the linker
cannot reasonably resolve the relocation.

GNU ld allows it but lld will warn. Add the SHF_ALLOC flag.

Signed-off-by: Fangrui Song 
---
 src/romlayout.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/romlayout.S b/src/romlayout.S
index c4a4635..a854783 100644
--- a/src/romlayout.S
+++ b/src/romlayout.S
@@ -587,7 +587,7 @@ entry_18:
 
 // Specify a location in the fixed part of bios area.
 .macro ORG addr
-.section .fixedaddr.\addr
+.section .fixedaddr.\addr,"a"
 .endm
 
 ORG 0xe05b
-- 
2.26.0.292.g33ef6b2f38-goog
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org


[SeaBIOS] Handling SMI problem

2020-04-06 Thread lowa-88
Hello, recently I've found the-sea-watcher project and now I'm trying to repeat 
it. 
https://scumjr.github.io/2016/01/10/from-smm-to-userland-in-a-few-bytes/
But I have a strange problem when I trigger a SMI my VM instantly reboot. I use 
that function to send SMI:

#define PORT_SMI_CMD 0x00b2
int main(void)
{
if (ioperm(PORT_SMI_CMD, 1, 1) != 0)
err(EXIT_FAILURE, "ioperm");
outb(0x61, PORT_SMI_CMD);
printf("done\n");
return EXIT_SUCCESS;
}

I have rebuild SeaBios with CONFIG_DEBUG_LEVEL=8 and found out that SMI's 
reboot VM with a rather big chance. This is a part of SeaBios log:

VBE current mode=3
stub vbe_104fXX:406:
   a=4f09  b=0001  c=0100  d= ds=0040 es= ss=0100
  si= di=2000 bp= sp=1000 cs= ip=0600  f=3200
VBE mode set: c18f
set VGA mode 18f//at this point Ubuntu is loaded
handle_smi cmd=61 smbase=0x000a //I send first
handle_smi cmd=61 smbase=0x000a  //and second SMI
In resume (status=0) //have reboot on third
In 32bit resume
Attempting a hard reboot
enabling shadow ram
SeaBIOS (version 
rel-1.13.0-15-gde88a96-dirty-20200406_111431-test-Lenovo-ideapad-320S-13IKB)
BUILD: gcc: (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0 binutils: (GNU Binutils for 
Ubuntu) 2.30

I've started to explore the source code, but I don't found anything that could 
help me.
Can you give me an advice or a direction for my research?

Best regards.
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org