The alignment constraint is defined in the CSM specifications as
"Bit mapped.  First non-zero bit from the right is the alignment."

Use __fls() to sanitise the alignment given that definition, since
passing a non-power-of-two alignment to _malloc() isn't going to work
well. And cope with being passed zero, which was happening for the
E820 table allocation from EDK2.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
---
 src/fw/csm.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/fw/csm.c b/src/fw/csm.c
index 03b4bb8..bf7b8f5 100644
--- a/src/fw/csm.c
+++ b/src/fw/csm.c
@@ -264,6 +264,13 @@ handle_csm_0006(struct bregs *regs)
     dprintf(3, "Legacy16GetTableAddress size %x align %x region %d\n",
         size, align, region);
 
+    // DX = Required address alignment. Bit mapped.
+    // First non-zero bit from the right is the alignment.*/
+    if (align)
+	    align = 1 << __ffs(align);
+    else
+	    align = 1;
+
     if (region & 2)
         chunk = _malloc(&ZoneLow, size, align);
     if (!chunk && (region & 1))

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org

On Wed, Jun 12, 2019 at 05:56:23PM +0100, David Woodhouse wrote:
> The alignment constraint is defined in the CSM specifications as
> "Bit mapped.  First non-zero bit from the right is the alignment."
> 
> Use __fls() to sanitise the alignment given that definition, since
> passing a non-power-of-two alignment to _malloc() isn't going to work
> well. And cope with being passed zero, which was happening for the
> E820 table allocation from EDK2.
> 
> Signed-off-by: David Woodhouse <dwmw2@infradead.org>
> ---
>  src/fw/csm.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/fw/csm.c b/src/fw/csm.c
> index 03b4bb8..bf7b8f5 100644
> --- a/src/fw/csm.c
> +++ b/src/fw/csm.c
> @@ -264,6 +264,13 @@ handle_csm_0006(struct bregs *regs)
>      dprintf(3, "Legacy16GetTableAddress size %x align %x region %d\n",
>          size, align, region);
>  
> +    // DX = Required address alignment. Bit mapped.
> +    // First non-zero bit from the right is the alignment.*/
> +    if (align)
> +	    align = 1 << __ffs(align);
> +    else
> +	    align = 1;
> +

Thanks.  This should also enforce "align >= MALLOC_MIN_ALIGN".

-Kevin
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org

The alignment constraint is defined in the CSM specifications as
"Bit mapped.  First non-zero bit from the right is the alignment."

Use __fls() to sanitise the alignment given that definition, since
passing a non-power-of-two alignment to _malloc() isn't going to work
well. And cope with being passed zero, which was happening for the
E820 table allocation from EDK2.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
---
v2: Enforce MALLOC_MIN_ALIGN

 src/fw/csm.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/fw/csm.c b/src/fw/csm.c
index 03b4bb8..3fcc252 100644
--- a/src/fw/csm.c
+++ b/src/fw/csm.c
@@ -258,11 +258,21 @@ handle_csm_0006(struct bregs *regs)
     u16 region = regs->bx; // (1 for F000 seg, 2 for E000 seg, 0 for either)
     void *chunk = NULL;
 
+    dprintf(3, "Legacy16GetTableAddress size %x align %x region %d\n",
+        size, align, region);
+
     if (!region)
         region = 3;
 
-    dprintf(3, "Legacy16GetTableAddress size %x align %x region %d\n",
-        size, align, region);
+    // DX = Required address alignment. Bit mapped.
+    // First non-zero bit from the right is the alignment.*/
+    if (align) {
+        align = 1 << __ffs(align);
+        if (align < MALLOC_MIN_ALIGN)
+            align = MALLOC_MIN_ALIGN;
+    } else {
+        align = MALLOC_MIN_ALIGN;
+    }
 
     if (region & 2)
         chunk = _malloc(&ZoneLow, size, align);

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org

On Thu, Jun 13, 2019 at 04:25:13PM +0100, David Woodhouse wrote:
> The alignment constraint is defined in the CSM specifications as
> "Bit mapped.  First non-zero bit from the right is the alignment."
> 
> Use __fls() to sanitise the alignment given that definition, since
> passing a non-power-of-two alignment to _malloc() isn't going to work
> well. And cope with being passed zero, which was happening for the
> E820 table allocation from EDK2.

Thanks.  I committed this change.

-Kevin
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org