[PATCHv3] support T13 EDD3.0 spec

[Gleb Natapov]

Some guests (such as Linux) expect BIOS to behave according to T13
EDD3.0 spec. T13 spec is much better then Phoenix since it provides
more information about interface and device paths. This patch adds
support for the spec. If guest provides buffer with enough space for
T13 EDD info return EDD according to T13 spec otherwise use Phoenix
one.

Signed-off-by: Gleb Natapov 
diff --git a/src/disk.c b/src/disk.c
index f7bfe9c..8f7c61f 100644
--- a/src/disk.c
+++ b/src/disk.c
@@ -503,6 +503,7 @@ static void
 disk_1348(struct bregs *regs, struct drive_s *drive_g)
 {
     u16 size = GET_INT13DPT(regs, size);
+    u16 t13 = size == 74;

// Buffer is too small
     if (size < 26) {
@@ -553,8 +554,9 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
     // EDD 2.x

int bdf;
-    u16 iobase1;
-    u64 device_path;
+    u16 iobase1 = 0;
+    u64 device_path = 0;
+    u8 channel = 0;
     SET_INT13DPT(regs, size, 30);
     if (type == DTYPE_ATA || type == DTYPE_ATAPI) {
         u16 ebda_seg = get_ebda_seg();
@@ -573,6 +575,7 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
         iobase1 = GET_GLOBALFLAT(chan_gf->iobase1);
         bdf = GET_GLOBALFLAT(chan_gf->pci_bdf);
         device_path = slave;
+        channel = GET_GLOBALFLAT(chan_gf->chanid);

u16 options = 0;
         if (type == DTYPE_ATA) {
@@ -613,8 +616,6 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
         SET_INT13DPT(regs, dpte_segment, 0);
         SET_INT13DPT(regs, dpte_offset, 0);
         bdf = GET_GLOBAL(drive_g->cntl_id);
-        device_path = 0;
-        iobase1 = 0;
     }

if (size < 66) {
@@ -624,7 +625,7 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)

// EDD 3.x
     SET_INT13DPT(regs, key, 0xbedd);
-    SET_INT13DPT(regs, dpi_length, 36);
+    SET_INT13DPT(regs, dpi_length, t13 ? 44 : 36);
     SET_INT13DPT(regs, reserved1, 0);
     SET_INT13DPT(regs, reserved2, 0);

@@ -632,17 +633,20 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
         SET_INT13DPT(regs, host_bus[0], 'P');
         SET_INT13DPT(regs, host_bus[1], 'C');
         SET_INT13DPT(regs, host_bus[2], 'I');
-        SET_INT13DPT(regs, host_bus[3], 0);
+        SET_INT13DPT(regs, host_bus[3], ' ');

u32 path = (pci_bdf_to_bus(bdf) | (pci_bdf_to_dev(bdf) << 8)
                     | (pci_bdf_to_fn(bdf) << 16));
+        if (t13)
+            path |= channel << 24;
+
         SET_INT13DPT(regs, iface_path, path);
     } else {
         // ISA
         SET_INT13DPT(regs, host_bus[0], 'I');
         SET_INT13DPT(regs, host_bus[1], 'S');
         SET_INT13DPT(regs, host_bus[2], 'A');
-        SET_INT13DPT(regs, host_bus[3], 0);
+        SET_INT13DPT(regs, host_bus[3], ' ');

SET_INT13DPT(regs, iface_path, iobase1);
     }
@@ -651,22 +655,30 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
         SET_INT13DPT(regs, iface_type[0], 'A');
         SET_INT13DPT(regs, iface_type[1], 'T');
         SET_INT13DPT(regs, iface_type[2], 'A');
-        SET_INT13DPT(regs, iface_type[3], 0);
+        SET_INT13DPT(regs, iface_type[3], ' ');
     } else {
         SET_INT13DPT(regs, iface_type[0], 'S');
         SET_INT13DPT(regs, iface_type[1], 'C');
         SET_INT13DPT(regs, iface_type[2], 'S');
         SET_INT13DPT(regs, iface_type[3], 'I');
     }
-    SET_INT13DPT(regs, iface_type[4], 0);
-    SET_INT13DPT(regs, iface_type[5], 0);
-    SET_INT13DPT(regs, iface_type[6], 0);
-    SET_INT13DPT(regs, iface_type[7], 0);
+    SET_INT13DPT(regs, iface_type[4], ' ');
+    SET_INT13DPT(regs, iface_type[5], ' ');
+    SET_INT13DPT(regs, iface_type[6], ' ');
+    SET_INT13DPT(regs, iface_type[7], ' ');
+
+    if (t13) {
+        SET_INT13DPT(regs, t13.device_path[0], device_path);
+        SET_INT13DPT(regs, t13.device_path[1], 0);

-    SET_INT13DPT(regs, device_path, device_path);
+        SET_INT13DPT(regs, t13.checksum
+                     , -checksum_far(regs->ds, (void*)(regs->si+30), 43));
+    } else {
+        SET_INT13DPT(regs, phoenix.device_path, device_path);

-    SET_INT13DPT(regs, checksum
-                 , -checksum_far(regs->ds, (void*)(regs->si+30), 35));
+        SET_INT13DPT(regs, phoenix.checksum
+                     , -checksum_far(regs->ds, (void*)(regs->si+30), 35));
+    }

disk_ret(regs, DISK_RET_SUCCESS);
 }
diff --git a/src/disk.h b/src/disk.h
index f31de73..10a0051 100644
--- a/src/disk.h
+++ b/src/disk.h
@@ -63,9 +63,18 @@ struct int13dpt_s {
     u8  host_bus[4];
     u8  iface_type[8];
     u64 iface_path;
-    u64 device_path;
-    u8  reserved3;
-    u8  checksum;
+    union {
+        struct {
+            u64 device_path;
+            u8  reserved3;
+            u8  checksum;
+        } phoenix;
+        struct {
+            u64 device_path[2];
+            u8  reserved3;
+            u8  checksum;
+        } t13;
+    };
 } PACKED;

#define GET_INT13DPT(regs,var)                                          \
--
    		Gleb.

[Sebastian Herbszt]

Gleb Natapov wrote:
> Some guests (such as Linux) expect BIOS to behave according to T13

"Some software expects BIOS ..." instead? "Guests" sounds misleading since
this feature can also be expected by userland software.
> EDD3.0 spec. T13 spec is much better then Phoenix since it provides
> more information about interface and device paths. This patch adds

"T13 spec provides more information about interface and device paths."
> support for the spec. If guest provides buffer with enough space for

"If caller provides ...".
> T13 EDD info return EDD according to T13 spec otherwise use Phoenix
> one.
>
> Signed-off-by: Gleb Natapov 
> diff --git a/src/disk.c b/src/disk.c
> index f7bfe9c..8f7c61f 100644
> --- a/src/disk.c
> +++ b/src/disk.c
> @@ -503,6 +503,7 @@ static void
> disk_1348(struct bregs *regs, struct drive_s *drive_g)
> {
>     u16 size = GET_INT13DPT(regs, size);
>
> u16 t13 = size == 74;
>
> // Buffer is too small
> if (size < 26) {
>
> @@ -553,8 +554,9 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
>     // EDD 2.x
> int bdf;
> u16 iobase1;
> u64 device_path;
> u16 iobase1 = 0;
> u64 device_path = 0;
> u8 channel = 0;
> SET_INT13DPT(regs, size, 30);
> if (type == DTYPE_ATA || type == DTYPE_ATAPI) {
>    u16 ebda_seg = get_ebda_seg();
>
> @@ -573,6 +575,7 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
>         iobase1 = GET_GLOBALFLAT(chan_gf->iobase1);
>         bdf = GET_GLOBALFLAT(chan_gf->pci_bdf);
>         device_path = slave;
>    channel = GET_GLOBALFLAT(chan_gf->chanid);
>
>   u16 options = 0;
>   if (type == DTYPE_ATA) {
>
> @@ -613,8 +616,6 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
>         SET_INT13DPT(regs, dpte_segment, 0);
>         SET_INT13DPT(regs, dpte_offset, 0);
>         bdf = GET_GLOBAL(drive_g->cntl_id);
>    device_path = 0;
>    iobase1 = 0;
>
> }
>
> if (size < 66) {
>
> @@ -624,7 +625,7 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
> // EDD 3.x
> SET_INT13DPT(regs, key, 0xbedd);
> SET_INT13DPT(regs, dpi_length, 36);
> SET_INT13DPT(regs, dpi_length, t13 ? 44 : 36);
> SET_INT13DPT(regs, reserved1, 0);
> SET_INT13DPT(regs, reserved2, 0);
>
> @@ -632,17 +633,20 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
>         SET_INT13DPT(regs, host_bus[0], 'P');
>         SET_INT13DPT(regs, host_bus[1], 'C');
>         SET_INT13DPT(regs, host_bus[2], 'I');
>    SET_INT13DPT(regs, host_bus[3], 0);
>    SET_INT13DPT(regs, host_bus[3], ' ');
>
>   u32 path = (pci_bdf_to_bus(bdf) | (pci_bdf_to_dev(bdf) << 8)
>               | (pci_bdf_to_fn(bdf) << 16));
>    if (t13)
>        path |= channel << 24;
>   SET_INT13DPT(regs, iface_path, path);
>
> } else {
>       // ISA
>       SET_INT13DPT(regs, host_bus[0], 'I');
>       SET_INT13DPT(regs, host_bus[1], 'S');
>       SET_INT13DPT(regs, host_bus[2], 'A');
>    SET_INT13DPT(regs, host_bus[3], 0);
>    SET_INT13DPT(regs, host_bus[3], ' ');
>
>   SET_INT13DPT(regs, iface_path, iobase1);
>
> }
>
> @@ -651,22 +655,30 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
>         SET_INT13DPT(regs, iface_type[0], 'A');
>         SET_INT13DPT(regs, iface_type[1], 'T');
>         SET_INT13DPT(regs, iface_type[2], 'A');
>    SET_INT13DPT(regs, iface_type[3], 0);
>    SET_INT13DPT(regs, iface_type[3], ' ');
> } else {
>       SET_INT13DPT(regs, iface_type[0], 'S');
>       SET_INT13DPT(regs, iface_type[1], 'C');
>       SET_INT13DPT(regs, iface_type[2], 'S');
>       SET_INT13DPT(regs, iface_type[3], 'I');
>   }
> SET_INT13DPT(regs, iface_type[4], 0);
> SET_INT13DPT(regs, iface_type[5], 0);
> SET_INT13DPT(regs, iface_type[6], 0);
> SET_INT13DPT(regs, iface_type[7], 0);
> SET_INT13DPT(regs, iface_type[4], ' ');
> SET_INT13DPT(regs, iface_type[5], ' ');
> SET_INT13DPT(regs, iface_type[6], ' ');
> SET_INT13DPT(regs, iface_type[7], ' ');
> if (t13) {
>    SET_INT13DPT(regs, t13.device_path[0], device_path);
>    SET_INT13DPT(regs, t13.device_path[1], 0);
>
> SET_INT13DPT(regs, device_path, device_path);
>    SET_INT13DPT(regs, t13.checksum
>                 , -checksum_far(regs->ds, (void*)(regs->si+30), 43));
> } else {
>    SET_INT13DPT(regs, phoenix.device_path, device_path);
>
> SET_INT13DPT(regs, checksum
>             , -checksum_far(regs->ds, (void*)(regs->si+30), 35));
>    SET_INT13DPT(regs, phoenix.checksum
>                 , -checksum_far(regs->ds, (void*)(regs->si+30), 35));
> }

Should the 0 to space padding part be a separate commit? Some pre-T13 spec
BIOS do space padding too.

Sebastian
> disk_ret(regs, DISK_RET_SUCCESS);
>
> }
> diff --git a/src/disk.h b/src/disk.h
> index f31de73..10a0051 100644
> --- a/src/disk.h
> +++ b/src/disk.h
> @@ -63,9 +63,18 @@ struct int13dpt_s {
>     u8  host_bus[4];
>     u8  iface_type[8];
>     u64 iface_path;
> u64 device_path;
> u8  reserved3;
> u8  checksum;
> union {
>    struct {
>        u64 device_path;
>        u8  reserved3;
>        u8  checksum;
>    } phoenix;
>    struct {
>        u64 device_path[2];
>        u8  reserved3;
>        u8  checksum;
>    } t13;
> };
>
> } PACKED;
> #define GET_INT13DPT(regs,var)                                          \
>
> Gleb.
>
> SeaBIOS mailing list
> [e-mail]
> http://www.seabios.org/mailman/listinfo/seabios

[Kevin O'Connor]

On Mon, Jan 10, 2011 at 10:50:27AM +0200, Gleb Natapov wrote:
> Some guests (such as Linux) expect BIOS to behave according to T13
> EDD3.0 spec. T13 spec is much better then Phoenix since it provides
> more information about interface and device paths. This patch adds
> support for the spec. If guest provides buffer with enough space for
> T13 EDD info return EDD according to T13 spec otherwise use Phoenix
> one.

It looks okay to me.

-Kevin

[Gleb Natapov]

On Mon, Jan 10, 2011 at 09:15:19PM +0100, Sebastian Herbszt wrote:
> Gleb Natapov wrote:
> > Some guests (such as Linux) expect BIOS to behave according to T13
>
> "Some software expects BIOS ..." instead? "Guests" sounds misleading since
> this feature can also be expected by userland software.
> > EDD3.0 spec. T13 spec is much better then Phoenix since it provides
> > more information about interface and device paths. This patch adds
>
> "T13 spec provides more information about interface and device paths."
> > support for the spec. If guest provides buffer with enough space for
>
> "If caller provides ...".

Yeah, I have to admit my head is full of vitalization ;)
> > T13 EDD info return EDD according to T13 spec otherwise use Phoenix
> > one.
> >
> > Signed-off-by: Gleb Natapov 
> > diff --git a/src/disk.c b/src/disk.c
> > index f7bfe9c..8f7c61f 100644
> > --- a/src/disk.c
> > +++ b/src/disk.c
> > @@ -503,6 +503,7 @@ static void
> > disk_1348(struct bregs *regs, struct drive_s *drive_g)
> > {
> >    u16 size = GET_INT13DPT(regs, size);
> >
> > u16 t13 = size == 74;
> >
> > // Buffer is too small
> > if (size < 26) {
> >
> > @@ -553,8 +554,9 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
> >    // EDD 2.x
> >
> > int bdf;
> > u16 iobase1;
> > u64 device_path;
> > u16 iobase1 = 0;
> > u64 device_path = 0;
> > u8 channel = 0;
> > SET_INT13DPT(regs, size, 30);
> > if (type == DTYPE_ATA || type == DTYPE_ATAPI) {
> >   u16 ebda_seg = get_ebda_seg();
> >
> > @@ -573,6 +575,7 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
> >        iobase1 = GET_GLOBALFLAT(chan_gf->iobase1);
> >        bdf = GET_GLOBALFLAT(chan_gf->pci_bdf);
> >        device_path = slave;
> >    channel = GET_GLOBALFLAT(chan_gf->chanid);
> >
> >  u16 options = 0;
> >  if (type == DTYPE_ATA) {
> >
> > @@ -613,8 +616,6 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
> >        SET_INT13DPT(regs, dpte_segment, 0);
> >        SET_INT13DPT(regs, dpte_offset, 0);
> >        bdf = GET_GLOBAL(drive_g->cntl_id);
> >    device_path = 0;
> >    iobase1 = 0;
> >
> > }
> >
> > if (size < 66) {
> >
> > @@ -624,7 +625,7 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
> >
> > // EDD 3.x
> >    SET_INT13DPT(regs, key, 0xbedd);
> > SET_INT13DPT(regs, dpi_length, 36);
> > SET_INT13DPT(regs, dpi_length, t13 ? 44 : 36);
> > SET_INT13DPT(regs, reserved1, 0);
> > SET_INT13DPT(regs, reserved2, 0);
> >
> > @@ -632,17 +633,20 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
> >        SET_INT13DPT(regs, host_bus[0], 'P');
> >        SET_INT13DPT(regs, host_bus[1], 'C');
> >        SET_INT13DPT(regs, host_bus[2], 'I');
> >    SET_INT13DPT(regs, host_bus[3], 0);
> >    SET_INT13DPT(regs, host_bus[3], ' ');
> >
> >  u32 path = (pci_bdf_to_bus(bdf) | (pci_bdf_to_dev(bdf) << 8)
> >              | (pci_bdf_to_fn(bdf) << 16));
> >    if (t13)
> >        path |= channel << 24;
> >  SET_INT13DPT(regs, iface_path, path);
> >
> > } else {
> >      // ISA
> >      SET_INT13DPT(regs, host_bus[0], 'I');
> >      SET_INT13DPT(regs, host_bus[1], 'S');
> >      SET_INT13DPT(regs, host_bus[2], 'A');
> >    SET_INT13DPT(regs, host_bus[3], 0);
> >    SET_INT13DPT(regs, host_bus[3], ' ');
> >
> >  SET_INT13DPT(regs, iface_path, iobase1);
> >
> > }
> >
> > @@ -651,22 +655,30 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
> >        SET_INT13DPT(regs, iface_type[0], 'A');
> >        SET_INT13DPT(regs, iface_type[1], 'T');
> >        SET_INT13DPT(regs, iface_type[2], 'A');
> >    SET_INT13DPT(regs, iface_type[3], 0);
> >    SET_INT13DPT(regs, iface_type[3], ' ');
> > } else {
> >      SET_INT13DPT(regs, iface_type[0], 'S');
> >      SET_INT13DPT(regs, iface_type[1], 'C');
> >      SET_INT13DPT(regs, iface_type[2], 'S');
> >      SET_INT13DPT(regs, iface_type[3], 'I');
> >  }
> > SET_INT13DPT(regs, iface_type[4], 0);
> > SET_INT13DPT(regs, iface_type[5], 0);
> > SET_INT13DPT(regs, iface_type[6], 0);
> > SET_INT13DPT(regs, iface_type[7], 0);
> > SET_INT13DPT(regs, iface_type[4], ' ');
> > SET_INT13DPT(regs, iface_type[5], ' ');
> > SET_INT13DPT(regs, iface_type[6], ' ');
> > SET_INT13DPT(regs, iface_type[7], ' ');
> > if (t13) {
> >    SET_INT13DPT(regs, t13.device_path[0], device_path);
> >    SET_INT13DPT(regs, t13.device_path[1], 0);
> >
> > SET_INT13DPT(regs, device_path, device_path);
> >    SET_INT13DPT(regs, t13.checksum
> >                 , -checksum_far(regs->ds, (void*)(regs->si+30), 43));
> > } else {
> >    SET_INT13DPT(regs, phoenix.device_path, device_path);
> >
> > SET_INT13DPT(regs, checksum
> >             , -checksum_far(regs->ds, (void*)(regs->si+30), 35));
> >    SET_INT13DPT(regs, phoenix.checksum
> >                 , -checksum_far(regs->ds, (void*)(regs->si+30), 35));
> > }
>
> Should the 0 to space padding part be a separate commit? Some pre-T13 spec
> BIOS do space padding too.

Unless we are planing to port space padding fix to stable branch I do
not see the need in separate commit.
> Sebastian
> > disk_ret(regs, DISK_RET_SUCCESS);
> > }
> > diff --git a/src/disk.h b/src/disk.h
> > index f31de73..10a0051 100644
> > --- a/src/disk.h
> > +++ b/src/disk.h
> > @@ -63,9 +63,18 @@ struct int13dpt_s {
> >    u8  host_bus[4];
> >    u8  iface_type[8];
> >    u64 iface_path;
> > u64 device_path;
> > u8  reserved3;
> > u8  checksum;
> > union {
> >    struct {
> >        u64 device_path;
> >        u8  reserved3;
> >        u8  checksum;
> >    } phoenix;
> >    struct {
> >        u64 device_path[2];
> >        u8  reserved3;
> >        u8  checksum;
> >    } t13;
> > };
> >
> > } PACKED;
> > #define GET_INT13DPT(regs,var)                                          \
> >
> > Gleb.
> >
> > SeaBIOS mailing list
> > [e-mail]
> > http://www.seabios.org/mailman/listinfo/seabios

[Gleb Natapov]

On Mon, Jan 10, 2011 at 11:28:26PM -0500, Kevin O'Connor wrote:
> On Mon, Jan 10, 2011 at 10:50:27AM +0200, Gleb Natapov wrote:
> > Some guests (such as Linux) expect BIOS to behave according to T13
> > EDD3.0 spec. T13 spec is much better then Phoenix since it provides
> > more information about interface and device paths. This patch adds
> > support for the spec. If guest provides buffer with enough space for
> > T13 EDD info return EDD according to T13 spec otherwise use Phoenix
> > one.
>
> It looks okay to me.

Do you want me to rewrite commit message to drop references to "guest" like
Sebastian suggested, or will you do it yourself?

[Sebastian Herbszt]

Gleb Natapov wrote:
> On Mon, Jan 10, 2011 at 09:15:19PM +0100, Sebastian Herbszt wrote:
> > Gleb Natapov wrote:
> > > Some guests (such as Linux) expect BIOS to behave according to T13
> >
> > "Some software expects BIOS ..." instead? "Guests" sounds misleading since
> > this feature can also be expected by userland software.
> > > EDD3.0 spec. T13 spec is much better then Phoenix since it provides
> > > more information about interface and device paths. This patch adds
> >
> > "T13 spec provides more information about interface and device paths."
> > > support for the spec. If guest provides buffer with enough space for
> >
> > "If caller provides ...".
>
> Yeah, I have to admit my head is full of vitalization ;)
> > > T13 EDD info return EDD according to T13 spec otherwise use Phoenix
> > > one.
> > >
> > > Signed-off-by: Gleb Natapov 
> > > diff --git a/src/disk.c b/src/disk.c
> > > index f7bfe9c..8f7c61f 100644
> > > --- a/src/disk.c
> > > +++ b/src/disk.c
> > > @@ -503,6 +503,7 @@ static void
> > > disk_1348(struct bregs *regs, struct drive_s *drive_g)
> > > {
> > >    u16 size = GET_INT13DPT(regs, size);
> > >
> > > u16 t13 = size == 74;
> > >
> > > // Buffer is too small
> > > if (size < 26) {
> > >
> > > @@ -553,8 +554,9 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
> > >    // EDD 2.x
> > >
> > > int bdf;
> > > u16 iobase1;
> > > u64 device_path;
> > > u16 iobase1 = 0;
> > > u64 device_path = 0;
> > > u8 channel = 0;
> > > SET_INT13DPT(regs, size, 30);
> > > if (type == DTYPE_ATA || type == DTYPE_ATAPI) {
> > >   u16 ebda_seg = get_ebda_seg();
> > >
> > > @@ -573,6 +575,7 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
> > >        iobase1 = GET_GLOBALFLAT(chan_gf->iobase1);
> > >        bdf = GET_GLOBALFLAT(chan_gf->pci_bdf);
> > >        device_path = slave;
> > >    channel = GET_GLOBALFLAT(chan_gf->chanid);
> > >
> > >  u16 options = 0;
> > >  if (type == DTYPE_ATA) {
> > >
> > > @@ -613,8 +616,6 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
> > >        SET_INT13DPT(regs, dpte_segment, 0);
> > >        SET_INT13DPT(regs, dpte_offset, 0);
> > >        bdf = GET_GLOBAL(drive_g->cntl_id);
> > >    device_path = 0;
> > >    iobase1 = 0;
> > >
> > > }
> > >
> > > if (size < 66) {
> > >
> > > @@ -624,7 +625,7 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
> > >
> > > // EDD 3.x
> > >    SET_INT13DPT(regs, key, 0xbedd);
> > > SET_INT13DPT(regs, dpi_length, 36);
> > > SET_INT13DPT(regs, dpi_length, t13 ? 44 : 36);
> > > SET_INT13DPT(regs, reserved1, 0);
> > > SET_INT13DPT(regs, reserved2, 0);
> > >
> > > @@ -632,17 +633,20 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
> > >        SET_INT13DPT(regs, host_bus[0], 'P');
> > >        SET_INT13DPT(regs, host_bus[1], 'C');
> > >        SET_INT13DPT(regs, host_bus[2], 'I');
> > >    SET_INT13DPT(regs, host_bus[3], 0);
> > >    SET_INT13DPT(regs, host_bus[3], ' ');
> > >
> > >  u32 path = (pci_bdf_to_bus(bdf) | (pci_bdf_to_dev(bdf) << 8)
> > >              | (pci_bdf_to_fn(bdf) << 16));
> > >    if (t13)
> > >        path |= channel << 24;
> > >  SET_INT13DPT(regs, iface_path, path);
> > >
> > > } else {
> > >      // ISA
> > >      SET_INT13DPT(regs, host_bus[0], 'I');
> > >      SET_INT13DPT(regs, host_bus[1], 'S');
> > >      SET_INT13DPT(regs, host_bus[2], 'A');
> > >    SET_INT13DPT(regs, host_bus[3], 0);
> > >    SET_INT13DPT(regs, host_bus[3], ' ');
> > >
> > >  SET_INT13DPT(regs, iface_path, iobase1);
> > >
> > > }
> > >
> > > @@ -651,22 +655,30 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
> > >        SET_INT13DPT(regs, iface_type[0], 'A');
> > >        SET_INT13DPT(regs, iface_type[1], 'T');
> > >        SET_INT13DPT(regs, iface_type[2], 'A');
> > >    SET_INT13DPT(regs, iface_type[3], 0);
> > >    SET_INT13DPT(regs, iface_type[3], ' ');
> > > } else {
> > >      SET_INT13DPT(regs, iface_type[0], 'S');
> > >      SET_INT13DPT(regs, iface_type[1], 'C');
> > >      SET_INT13DPT(regs, iface_type[2], 'S');
> > >      SET_INT13DPT(regs, iface_type[3], 'I');
> > >  }
> > > SET_INT13DPT(regs, iface_type[4], 0);
> > > SET_INT13DPT(regs, iface_type[5], 0);
> > > SET_INT13DPT(regs, iface_type[6], 0);
> > > SET_INT13DPT(regs, iface_type[7], 0);
> > > SET_INT13DPT(regs, iface_type[4], ' ');
> > > SET_INT13DPT(regs, iface_type[5], ' ');
> > > SET_INT13DPT(regs, iface_type[6], ' ');
> > > SET_INT13DPT(regs, iface_type[7], ' ');
> > > if (t13) {
> > >    SET_INT13DPT(regs, t13.device_path[0], device_path);
> > >    SET_INT13DPT(regs, t13.device_path[1], 0);
> > >
> > > SET_INT13DPT(regs, device_path, device_path);
> > >    SET_INT13DPT(regs, t13.checksum
> > >                 , -checksum_far(regs->ds, (void*)(regs->si+30), 43));
> > > } else {
> > >    SET_INT13DPT(regs, phoenix.device_path, device_path);
> > >
> > > SET_INT13DPT(regs, checksum
> > >             , -checksum_far(regs->ds, (void*)(regs->si+30), 35));
> > >    SET_INT13DPT(regs, phoenix.checksum
> > >                 , -checksum_far(regs->ds, (void*)(regs->si+30), 35));
> > > }
> >
> > Should the 0 to space padding part be a separate commit? Some pre-T13 spec
> > BIOS do space padding too.
>
> Unless we are planing to port space padding fix to stable branch I do
> not see the need in separate commit.

That change is not directly linked to the T13 EDD 3.0 spec and a separate commit
would point that out. Also bisect would blame the correct change.

Sebastian

[Kevin O'Connor]

On Tue, Jan 11, 2011 at 09:37:42AM +0200, Gleb Natapov wrote:
> On Mon, Jan 10, 2011 at 11:28:26PM -0500, Kevin O'Connor wrote:
> > On Mon, Jan 10, 2011 at 10:50:27AM +0200, Gleb Natapov wrote:
> > > Some guests (such as Linux) expect BIOS to behave according to T13
> > > EDD3.0 spec. T13 spec is much better then Phoenix since it provides
> > > more information about interface and device paths. This patch adds
> > > support for the spec. If guest provides buffer with enough space for
> > > T13 EDD info return EDD according to T13 spec otherwise use Phoenix
> > > one.
> >
> > It looks okay to me.
>
> Do you want me to rewrite commit message to drop references to "guest" like
> Sebastian suggested, or will you do it yourself?

I don't think it's that big a deal - lets just note this for future
commits - okay?

-Kevin

[Gleb Natapov]

On Tue, Jan 11, 2011 at 10:32:24PM +0100, Sebastian Herbszt wrote:
> Gleb Natapov wrote:
> > On Mon, Jan 10, 2011 at 09:15:19PM +0100, Sebastian Herbszt wrote:
> > > Gleb Natapov wrote:
> > > > Some guests (such as Linux) expect BIOS to behave according to T13
> > >
> > > "Some software expects BIOS ..." instead? "Guests" sounds misleading since
> > > this feature can also be expected by userland software.
> > > > EDD3.0 spec. T13 spec is much better then Phoenix since it provides
> > > > more information about interface and device paths. This patch adds
> > >
> > > "T13 spec provides more information about interface and device paths."
> > > > support for the spec. If guest provides buffer with enough space for
> > >
> > > "If caller provides ...".
> >
> > Yeah, I have to admit my head is full of vitalization ;)
> > > > T13 EDD info return EDD according to T13 spec otherwise use Phoenix
> > > > one.
> > > >
> > > > Signed-off-by: Gleb Natapov 
> > > > diff --git a/src/disk.c b/src/disk.c
> > > > index f7bfe9c..8f7c61f 100644
> > > > --- a/src/disk.c
> > > > +++ b/src/disk.c
> > > > @@ -503,6 +503,7 @@ static void
> > > > disk_1348(struct bregs *regs, struct drive_s *drive_g)
> > > > {
> > > >    u16 size = GET_INT13DPT(regs, size);
> > > >
> > > > u16 t13 = size == 74;
> > > >
> > > > // Buffer is too small
> > > > if (size < 26) {
> > > >
> > > > @@ -553,8 +554,9 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
> > > >    // EDD 2.x
> > > >
> > > > int bdf;
> > > > u16 iobase1;
> > > > u64 device_path;
> > > > u16 iobase1 = 0;
> > > > u64 device_path = 0;
> > > > u8 channel = 0;
> > > > SET_INT13DPT(regs, size, 30);
> > > > if (type == DTYPE_ATA || type == DTYPE_ATAPI) {
> > > >   u16 ebda_seg = get_ebda_seg();
> > > >
> > > > @@ -573,6 +575,7 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
> > > >        iobase1 = GET_GLOBALFLAT(chan_gf->iobase1);
> > > >        bdf = GET_GLOBALFLAT(chan_gf->pci_bdf);
> > > >        device_path = slave;
> > > >    channel = GET_GLOBALFLAT(chan_gf->chanid);
> > > >
> > > >  u16 options = 0;
> > > >  if (type == DTYPE_ATA) {
> > > >
> > > > @@ -613,8 +616,6 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
> > > >        SET_INT13DPT(regs, dpte_segment, 0);
> > > >        SET_INT13DPT(regs, dpte_offset, 0);
> > > >        bdf = GET_GLOBAL(drive_g->cntl_id);
> > > >    device_path = 0;
> > > >    iobase1 = 0;
> > > >
> > > > }
> > > >
> > > > if (size < 66) {
> > > >
> > > > @@ -624,7 +625,7 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
> > > >
> > > > // EDD 3.x
> > > >    SET_INT13DPT(regs, key, 0xbedd);
> > > > SET_INT13DPT(regs, dpi_length, 36);
> > > > SET_INT13DPT(regs, dpi_length, t13 ? 44 : 36);
> > > > SET_INT13DPT(regs, reserved1, 0);
> > > > SET_INT13DPT(regs, reserved2, 0);
> > > >
> > > > @@ -632,17 +633,20 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
> > > >        SET_INT13DPT(regs, host_bus[0], 'P');
> > > >        SET_INT13DPT(regs, host_bus[1], 'C');
> > > >        SET_INT13DPT(regs, host_bus[2], 'I');
> > > >    SET_INT13DPT(regs, host_bus[3], 0);
> > > >    SET_INT13DPT(regs, host_bus[3], ' ');
> > > >
> > > >  u32 path = (pci_bdf_to_bus(bdf) | (pci_bdf_to_dev(bdf) << 8)
> > > >              | (pci_bdf_to_fn(bdf) << 16));
> > > >    if (t13)
> > > >        path |= channel << 24;
> > > >  SET_INT13DPT(regs, iface_path, path);
> > > >
> > > > } else {
> > > >      // ISA
> > > >      SET_INT13DPT(regs, host_bus[0], 'I');
> > > >      SET_INT13DPT(regs, host_bus[1], 'S');
> > > >      SET_INT13DPT(regs, host_bus[2], 'A');
> > > >    SET_INT13DPT(regs, host_bus[3], 0);
> > > >    SET_INT13DPT(regs, host_bus[3], ' ');
> > > >
> > > >  SET_INT13DPT(regs, iface_path, iobase1);
> > > >
> > > > }
> > > >
> > > > @@ -651,22 +655,30 @@ disk_1348(struct bregs *regs, struct drive_s *drive_g)
> > > >        SET_INT13DPT(regs, iface_type[0], 'A');
> > > >        SET_INT13DPT(regs, iface_type[1], 'T');
> > > >        SET_INT13DPT(regs, iface_type[2], 'A');
> > > >    SET_INT13DPT(regs, iface_type[3], 0);
> > > >    SET_INT13DPT(regs, iface_type[3], ' ');
> > > > } else {
> > > >      SET_INT13DPT(regs, iface_type[0], 'S');
> > > >      SET_INT13DPT(regs, iface_type[1], 'C');
> > > >      SET_INT13DPT(regs, iface_type[2], 'S');
> > > >      SET_INT13DPT(regs, iface_type[3], 'I');
> > > >  }
> > > > SET_INT13DPT(regs, iface_type[4], 0);
> > > > SET_INT13DPT(regs, iface_type[5], 0);
> > > > SET_INT13DPT(regs, iface_type[6], 0);
> > > > SET_INT13DPT(regs, iface_type[7], 0);
> > > > SET_INT13DPT(regs, iface_type[4], ' ');
> > > > SET_INT13DPT(regs, iface_type[5], ' ');
> > > > SET_INT13DPT(regs, iface_type[6], ' ');
> > > > SET_INT13DPT(regs, iface_type[7], ' ');
> > > > if (t13) {
> > > >    SET_INT13DPT(regs, t13.device_path[0], device_path);
> > > >    SET_INT13DPT(regs, t13.device_path[1], 0);
> > > >
> > > > SET_INT13DPT(regs, device_path, device_path);
> > > >    SET_INT13DPT(regs, t13.checksum
> > > >                 , -checksum_far(regs->ds, (void*)(regs->si+30), 43));
> > > > } else {
> > > >    SET_INT13DPT(regs, phoenix.device_path, device_path);
> > > >
> > > > SET_INT13DPT(regs, checksum
> > > >             , -checksum_far(regs->ds, (void*)(regs->si+30), 35));
> > > >    SET_INT13DPT(regs, phoenix.checksum
> > > >                 , -checksum_far(regs->ds, (void*)(regs->si+30), 35));
> > > > }
> > >
> > > Should the 0 to space padding part be a separate commit? Some pre-T13 spec
> > > BIOS do space padding too.
> >
> > Unless we are planing to port space padding fix to stable branch I do
> > not see the need in separate commit.
>
> That change is not directly linked to the T13 EDD 3.0 spec and a separate commit
> would point that out. Also bisect would blame the correct change.

I'll leave that for Kevin to decide. If he wants me to resubmit as two
patches I will.

[Kevin O'Connor]

On Wed, Jan 12, 2011 at 06:23:26PM +0200, Gleb Natapov wrote:
> On Tue, Jan 11, 2011 at 10:32:24PM +0100, Sebastian Herbszt wrote:
> > Gleb Natapov wrote:
> > > On Mon, Jan 10, 2011 at 09:15:19PM +0100, Sebastian Herbszt wrote:
> > > > Should the 0 to space padding part be a separate commit? Some pre-T13 spec
> > > > BIOS do space padding too.
> > >
> > > Unless we are planing to port space padding fix to stable branch I do
> > > not see the need in separate commit.
> >
> > That change is not directly linked to the T13 EDD 3.0 spec and a separate commit
> > would point that out. Also bisect would blame the correct change.
>
> I'll leave that for Kevin to decide. If he wants me to resubmit as two
> patches I will.

If you or Sebastian wish to send a broken out patch, then great.
Otherwise I'll commit your existing patch.

-Kevin

Question about linker building seabios

[Marc Bertens]

Hi Kevin,

I'm adding a VT100 serial console to the seabios code. 
Which can be set to port COM1 or COM2. 
I added the interrupt vector in the serial_setup() routine. 
And made the interrupt service routine. The compiling goes 
fine, no problems there. But in the linking stage an error 
is reported to the line of the interrupt service routine:

make
  Compiling whole program out/ccode.16.s
  Compiling (16bit) out/code16.o
  Compiling whole program out/ccode32flat.o
  Building ld scripts (version "pre-0.6.2-20110115_103541-mbertens-desktop")
Fixed space: 0xe05b-0x10000  total: 8101  slack: 15  Percent slack: 0.2%
16bit size:           37264
32bit segmented size: 2217
32bit flat size:      14007
32bit flat init size: 38848
  Linking out/rom16.o
  Stripping out/rom16.strip.o
  Linking out/rom32seg.o
  Stripping out/rom32seg.strip.o
  Linking out/rom.o
`.discard.func16.src/serial.c.261' referenced in section `.text.init_hw.54839' of out/code32flat.o: defined in discarded section `.discard.func16.src/serial.c.261' of out/code32flat.o
make: *** [out/rom.o] Error 1

Can you tell me what i'm doing wrong or missing to do?

Below here are the code snippets:

// INT(3/4)h : Serial Hardware Service Entry Point (for keyboard
handling)
    void VISIBLE16 handle_console_serial( void )
    {
        ...    // my code
        ...
    done:
        eoi_pic1();
    }

void
    serial_setup(void)
    {
        ...  // orginal code

...
        #if CONFIG_KEYBOARD_SERIAL
            con_addr = GET_BDA( port_com[ CONFIG_KEYBOARD_SERIAL -
1 ] );
            dprintf( 3, "keyboard on serial port %x\n", con_addr ); 
        # if CONFIG_KEYBOARD_SERIAL == 1 || CONFIG_KEYBOARD_SERIAL == 3 
            enable_hwirq( 4, FUNC16( handle_console_serial ) );
        # else
            enable_hwirq( 3, FUNC16( handle_console_serial ) );
        # endif
        #endif
    }

Regards,

Marc Bertens

Question about linker building seabios

[Kevin O'Connor]

On Sat, Jan 15, 2011 at 10:56:43AM +0100, Marc Bertens wrote:
> Hi Kevin,

Hi - see below.
> I'm adding a VT100 serial console to the seabios code. 
> Which can be set to port COM1 or COM2. 
> I added the interrupt vector in the serial_setup() routine. 
> And made the interrupt service routine. The compiling goes 
> fine, no problems there. But in the linking stage an error 
> is reported to the line of the interrupt service routine:
>
> make
>   Compiling whole program out/ccode.16.s
>   Compiling (16bit) out/code16.o
>   Compiling whole program out/ccode32flat.o
>   Building ld scripts (version "pre-0.6.2-20110115_103541-mbertens-desktop")
> Fixed space: 0xe05b-0x10000  total: 8101  slack: 15  Percent slack: 0.2%
> 16bit size:           37264
> 32bit segmented size: 2217
> 32bit flat size:      14007
> 32bit flat init size: 38848
>   Linking out/rom16.o
>   Stripping out/rom16.strip.o
>   Linking out/rom32seg.o
>   Stripping out/rom32seg.strip.o
>   Linking out/rom.o
> `.discard.func16.src/serial.c.261' referenced in section `.text.init_hw.54839' of out/code32flat.o: defined in discarded section `.discard.func16.src/serial.c.261' of out/code32flat.o
> make: *** [out/rom.o] Error 1
>
> Can you tell me what i'm doing wrong or missing to do?
>
> Below here are the code snippets:
> // INT(3/4)h : Serial Hardware Service Entry Point (for keyboard
>
> handling)
>     void VISIBLE16 handle_console_serial( void )
>     {
>         ...    // my code
>         ...
>     done:
>         eoi_pic1();
>     }
> void
> serial_setup(void)
> {
>     ...  // orginal code
>
>     ...
>     #if CONFIG_KEYBOARD_SERIAL
>         con_addr = GET_BDA( port_com[ CONFIG_KEYBOARD_SERIAL -
>
> 1 ] );
>             dprintf( 3, "keyboard on serial port %x\n", con_addr ); 
>         # if CONFIG_KEYBOARD_SERIAL == 1 || CONFIG_KEYBOARD_SERIAL == 3 
>             enable_hwirq( 4, FUNC16( handle_console_serial ) );
>         # else
>             enable_hwirq( 3, FUNC16( handle_console_serial ) );
>         # endif

You need to define an assembler stub (see romlayout.S) that calls
handle_console_serial.  Only this assembler stub should be registered
with enable_hwirq().  Looks like you probably just need to add
"DECL_IRQ_ENTRY console_serial", which will allow you do
"enable_hwirq(3, FUNC16(entry_console_serial));"

-Kevin

Question about linker building seabios

[Kevin O'Connor]

On Sat, Jan 15, 2011 at 10:56:43AM +0100, Marc Bertens wrote:
> Hi Kevin,
>
> I'm adding a VT100 serial console to the seabios code. 
> Which can be set to port COM1 or COM2. 
> I added the interrupt vector in the serial_setup() routine. 
> And made the interrupt service routine. The compiling goes 
> fine, no problems there. But in the linking stage an error 
> is reported to the line of the interrupt service routine:

BTW, are you familiar with sgabios?  It sounds like it does something
similar:

http://www.coreboot.org/SeaBIOS#Adding_sgabios_support

-Kevin

Question about linker building seabios

[Marc Bertens]

Kevin,

yea i know about sga bios, i used it for a long time, but not any more.
And the part i'm adding is basically to pick up the keyboard from the
serial console. And that is not is sgabios.

Marc

-----Original Message-----
From: Kevin O'Connor 
To: Marc Bertens 
Cc: [e-mail]
Subject: Re: Question about linker building seabios
Date: Sat, 15 Jan 2011 11:36:20 -0500

On Sat, Jan 15, 2011 at 10:56:43AM +0100, Marc Bertens wrote:
> Hi Kevin,
>
> I'm adding a VT100 serial console to the seabios code. 
> Which can be set to port COM1 or COM2. 
> I added the interrupt vector in the serial_setup() routine. 
> And made the interrupt service routine. The compiling goes 
> fine, no problems there. But in the linking stage an error 
> is reported to the line of the interrupt service routine:

BTW, are you familiar with sgabios?  It sounds like it does something
similar:

http://www.coreboot.org/SeaBIOS#Adding_sgabios_support

-Kevin

Question about linker building seabios

[Kevin O'Connor]

On Sat, Jan 15, 2011 at 05:40:11PM +0100, Marc Bertens wrote:
> Kevin,
>
> yea i know about sga bios, i used it for a long time, but not any more.

Okay.
> And the part i'm adding is basically to pick up the keyboard from the
> serial console. And that is not is sgabios.

FYI, sgabios does have code to read the serial port and insert keys
into the keyboard buffer.

-Kevin

Question about linker building seabios

[Marc Bertens]

Hi Kevin,

Fixed the error on the linker, but now i got another error which i don't
understand, can you help me ?

make
  Compiling whole program out/ccode.16.s
  Compiling (16bit) out/code16.o
  Compiling whole program out/ccode32flat.o
  Building ld scripts (version
"pre-0.6.2-20110115_181202-mbertens-desktop")
Fixed space: 0xe05b-0x10000  total: 8101  slack: 15  Percent slack: 0.2%
16bit size:           37904
32bit segmented size: 2217
32bit flat size:      14007
32bit flat init size: 38848
Traceback (most recent call last):
  File "./tools/layoutrom.py", line 579, in <module>
    main()
  File "./tools/layoutrom.py", line 576, in main
    writeLinkerScripts(sections, entrysym, genreloc, out16, out32seg,
out32flat)
  File "./tools/layoutrom.py", line 257, in writeLinkerScripts
    + COMMONTRAILER)
TypeError: %x format: a number is required, not NoneType
make: *** [out/romlayout16.lds] Error 1

Regards,

Marc

-----Original Message-----
From: Marc Bertens 
To: Kevin O'Connor 
Cc: [e-mail]
Subject: [SeaBIOS] Question about linker building seabios
Date: Sat, 15 Jan 2011 10:56:43 +0100

Hi Kevin,

I'm adding a VT100 serial console to the seabios code. 
Which can be set to port COM1 or COM2. 
I added the interrupt vector in the serial_setup() routine. 
And made the interrupt service routine. The compiling goes 
fine, no problems there. But in the linking stage an error 
is reported to the line of the interrupt service routine:

make
  Compiling whole program out/ccode.16.s
  Compiling (16bit) out/code16.o
  Compiling whole program out/ccode32flat.o
  Building ld scripts (version "pre-0.6.2-20110115_103541-mbertens-desktop")
Fixed space: 0xe05b-0x10000  total: 8101  slack: 15  Percent slack: 0.2%
16bit size:           37264
32bit segmented size: 2217
32bit flat size:      14007
32bit flat init size: 38848
  Linking out/rom16.o
  Stripping out/rom16.strip.o
  Linking out/rom32seg.o
  Stripping out/rom32seg.strip.o
  Linking out/rom.o
`.discard.func16.src/serial.c.261' referenced in section `.text.init_hw.54839' of out/code32flat.o: defined in discarded section `.discard.func16.src/serial.c.261' of out/code32flat.o
make: *** [out/rom.o] Error 1

Can you tell me what i'm doing wrong or missing to do?

Below here are the code snippets:

// INT(3/4)h : Serial Hardware Service Entry Point (for keyboard
handling)
    void VISIBLE16 handle_console_serial( void )
    {
        ...    // my code
        ...
    done:
        eoi_pic1();
    }

void
    serial_setup(void)
    {
        ...  // orginal code

...
        #if CONFIG_KEYBOARD_SERIAL
            con_addr = GET_BDA( port_com[ CONFIG_KEYBOARD_SERIAL -
1 ] );
            dprintf( 3, "keyboard on serial port %x\n", con_addr ); 
        # if CONFIG_KEYBOARD_SERIAL == 1 || CONFIG_KEYBOARD_SERIAL == 3 
            enable_hwirq( 4, FUNC16( handle_console_serial ) );
        # else
            enable_hwirq( 3, FUNC16( handle_console_serial ) );
        # endif
        #endif
    }

Regards,

Marc Bertens

_______________________________________________
SeaBIOS mailing list
[e-mail]
http://www.seabios.org/mailman/listinfo/seabios

Question about linker building seabios

[Kevin O'Connor]

On Sat, Jan 15, 2011 at 09:07:55PM +0100, Marc Bertens wrote:
> Hi Kevin,
>
> Fixed the error on the linker, but now i got another error which i don't
> understand, can you help me ?
>
> make
>   Compiling whole program out/ccode.16.s
>   Compiling (16bit) out/code16.o
>   Compiling whole program out/ccode32flat.o
>   Building ld scripts (version
> "pre-0.6.2-20110115_181202-mbertens-desktop")
> Fixed space: 0xe05b-0x10000  total: 8101  slack: 15  Percent slack: 0.2%
> 16bit size:           37904
> 32bit segmented size: 2217
> 32bit flat size:      14007
> 32bit flat init size: 38848
> Traceback (most recent call last):
>   File "./tools/layoutrom.py", line 579, in <module>
>     main()
>   File "./tools/layoutrom.py", line 576, in main
>     writeLinkerScripts(sections, entrysym, genreloc, out16, out32seg,
> out32flat)
>   File "./tools/layoutrom.py", line 257, in writeLinkerScripts
>     + COMMONTRAILER)
> TypeError: %x format: a number is required, not NoneType
> make: *** [out/romlayout16.lds] Error 1

Something went wrong with the build.  Try "make clean ; make" - if
that doesn't fix it, send the patch you've tried to apply.

-Kevin

Question about linker building seabios

[Marc Bertens]

Keven,

here is the patch, i hope yoiu can see what i'm doing wrong :-)

Marc

-----Original Message-----
From: Kevin O'Connor 
To: Marc Bertens 
Cc: [e-mail]
Subject: Re: [SeaBIOS] Question about linker building seabios
Date: Sat, 15 Jan 2011 15:16:38 -0500

On Sat, Jan 15, 2011 at 09:07:55PM +0100, Marc Bertens wrote:
> Hi Kevin,
>
> Fixed the error on the linker, but now i got another error which i don't
> understand, can you help me ?
>
> make
>   Compiling whole program out/ccode.16.s
>   Compiling (16bit) out/code16.o
>   Compiling whole program out/ccode32flat.o
>   Building ld scripts (version
> "pre-0.6.2-20110115_181202-mbertens-desktop")
> Fixed space: 0xe05b-0x10000  total: 8101  slack: 15  Percent slack: 0.2%
> 16bit size:           37904
> 32bit segmented size: 2217
> 32bit flat size:      14007
> 32bit flat init size: 38848
> Traceback (most recent call last):
>   File "./tools/layoutrom.py", line 579, in <module>
>     main()
>   File "./tools/layoutrom.py", line 576, in main
>     writeLinkerScripts(sections, entrysym, genreloc, out16, out32seg,
> out32flat)
>   File "./tools/layoutrom.py", line 257, in writeLinkerScripts
>     + COMMONTRAILER)
> TypeError: %x format: a number is required, not NoneType
> make: *** [out/romlayout16.lds] Error 1

Something went wrong with the build.  Try "make clean ; make" - if
that doesn't fix it, send the patch you've tried to apply.

-Kevin

Question about linker building seabios

[Marc Bertens]

Hi Kevin,

Can you tell me what the short 'HDL' stands for in for example
'DEBUG_HDL_pmm'

Currently i'm building a kconfig for seabios

Marc

Question about linker building seabios

[Kevin O'Connor]

On Sat, Jan 15, 2011 at 10:51:11PM +0100, Marc Bertens wrote:
> Hi Kevin,
>
> Can you tell me what the short 'HDL' stands for in for example
> 'DEBUG_HDL_pmm'

I believe it was shorthand for "handle".
> Currently i'm building a kconfig for seabios

That would be great!

-Kevin

Question about linker building seabios

[Kevin O'Connor]

On Sat, Jan 15, 2011 at 09:29:07PM +0100, Marc Bertens wrote:
> Keven,
>
> here is the patch, i hope yoiu can see what i'm doing wrong :-)
>
> Marc

The patch had no problem linking on my machine.  Can you re-clone,
apply the patch, and verify you still have the failure?  If so, please
tar up the repo after the failed "make" and email it to me.

-Kevin

Question about linker building seabios

[Marc Bertens]

Kevin,

Yeah I was thinking of for time, but first needed to figure out how the
kconfig build system works.
But I cloned the coreboot version, but now i have it almost on some
minor details ready. And I tested
it on my system, tomorrow i will build a complete image including
coreboot to see if my nokia will 
run correctly with seabios generated from kconfig.

Currently i've implemented such a way that you can still use it "old
school" by editing the 'hconfig.h'
file. But when all accept the new kconfig, than this can be dropped
easily.

Marc

-----Original Message-----
From: Kevin O'Connor 
To: Marc Bertens 
Cc: [e-mail]
Subject: Re: [SeaBIOS] Question about linker building seabios
Date: Sat, 15 Jan 2011 17:38:03 -0500

On Sat, Jan 15, 2011 at 10:51:11PM +0100, Marc Bertens wrote:
> Hi Kevin,
>
> Can you tell me what the short 'HDL' stands for in for example
> 'DEBUG_HDL_pmm'

I believe it was shorthand for "handle".
> Currently i'm building a kconfig for seabios

That would be great!

-Kevin

Question about linker building seabios

[Marc Bertens]

I will try that tommorrow :-)

Marc

-----Original Message-----
From: Kevin O'Connor 
To: Marc Bertens 
Cc: [e-mail]
Subject: Re: [SeaBIOS] Question about linker building seabios
Date: Sat, 15 Jan 2011 17:42:06 -0500

On Sat, Jan 15, 2011 at 09:29:07PM +0100, Marc Bertens wrote:
> Keven,
>
> here is the patch, i hope yoiu can see what i'm doing wrong :-)
>
> Marc

The patch had no problem linking on my machine.  Can you re-clone,
apply the patch, and verify you still have the failure?  If so, please
tar up the repo after the failed "make" and email it to me.

-Kevin

[Sebastian Herbszt]

Kevin O'Connor wrote:
> On Wed, Jan 12, 2011 at 06:23:26PM +0200, Gleb Natapov wrote:
> > On Tue, Jan 11, 2011 at 10:32:24PM +0100, Sebastian Herbszt wrote:
> > > Gleb Natapov wrote:
> > > > On Mon, Jan 10, 2011 at 09:15:19PM +0100, Sebastian Herbszt wrote:
> > > > > Should the 0 to space padding part be a separate commit? Some pre-T13 spec
> > > > > BIOS do space padding too.
> > > >
> > > > Unless we are planing to port space padding fix to stable branch I do
> > > > not see the need in separate commit.
> > >
> > > That change is not directly linked to the T13 EDD 3.0 spec and a separate commit
> > > would point that out. Also bisect would blame the correct change.
> >
> > I'll leave that for Kevin to decide. If he wants me to resubmit as two
> > patches I will.
>
> If you or Sebastian wish to send a broken out patch, then great.
> Otherwise I'll commit your existing patch.
>
> -Kevin

Kevin,

please commit the existing patch.

Acked-by: Sebastian Herbszt

Sebastian

[Kevin O'Connor]

On Sun, Jan 16, 2011 at 03:01:41PM +0100, Sebastian Herbszt wrote:
> please commit the existing patch.
>
> Acked-by: Sebastian Herbszt

I have committed the patch.

-Kevin