[PATCH 2/2] vgabios: Limit the range of the VBE number of "pages" parameter.

[Kevin O'Connor]

Looking at the output of other VGA BIOS implementations, it appears
that the number of available video pages reported is always between 1
and 127.

Signed-off-by: Kevin O'Connor 
---
 vgasrc/vbe.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/vgasrc/vbe.c b/vgasrc/vbe.c
index d962333..2c08736 100644
--- a/vgasrc/vbe.c
+++ b/vgasrc/vbe.c
@@ -144,6 +144,10 @@ vbe_104f01(struct bregs *regs)
             mode_attr |= VBE_MODE_ATTRIBUTE_LINEAR_FRAME_BUFFER_MODE;
         break;
     }
+    if (pages > 128)
+        pages = 128;
+    if (pages < 2)
+        pages++;
     SET_FARVAR(seg, info->mode_attributes, mode_attr);
     SET_FARVAR(seg, info->planes, planes);
     SET_FARVAR(seg, info->pages, pages - 1);

[Paul Menzel]

Am Freitag, den 13.09.2013, 16:24 -0400 schrieb Kevin O'Connor:
> Looking at the output of other VGA BIOS implementations, it appears
> that the number of available video pages reported is always between 1
> and 127.

That is including 1 and 127, right?
> Signed-off-by: Kevin O'Connor
>
> vgasrc/vbe.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/vgasrc/vbe.c b/vgasrc/vbe.c
> index d962333..2c08736 100644
> --- a/vgasrc/vbe.c
> +++ b/vgasrc/vbe.c
> @@ -144,6 +144,10 @@ vbe_104f01(struct bregs *regs)
>              mode_attr |= VBE_MODE_ATTRIBUTE_LINEAR_FRAME_BUFFER_MODE;
>          break;
>      }
> if (pages > 128)
>    pages = 128;

As 1 gets subtracted (below in diff), in this case 127 is included.
> if (pages < 2)
>    pages++;

Can pages be 0? If yes, it would be 0 again after substracting 1. Should
it be `pages = 1` instead of `pages++`?
>  SET_FARVAR(seg, info->mode_attributes, mode_attr);
>  SET_FARVAR(seg, info->planes, planes);
>  SET_FARVAR(seg, info->pages, pages - 1);

Thanks,

Paul

[PATCH 2/2] vgabios: Limit the range of the VBE number of "pages" parameter.

[Peter Stuge]

Paul Menzel wrote:
> > if (pages < 2)
> >    pages++;
>
> Can pages be 0? If yes, it would be 0 again after substracting 1. Should
> it be `pages = 1` instead of `pages++`?

pages=2;

//Peter

[Kevin O'Connor]

On Sat, Sep 14, 2013 at 09:11:02AM +0200, Paul Menzel wrote:
> Am Freitag, den 13.09.2013, 16:24 -0400 schrieb Kevin O'Connor:
> > Looking at the output of other VGA BIOS implementations, it appears
> > that the number of available video pages reported is always between 1
> > and 127.
>
> That is including 1 and 127, right?
> > Signed-off-by: Kevin O'Connor
> >
> > vgasrc/vbe.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/vgasrc/vbe.c b/vgasrc/vbe.c
> > index d962333..2c08736 100644
> > --- a/vgasrc/vbe.c
> > +++ b/vgasrc/vbe.c
> > @@ -144,6 +144,10 @@ vbe_104f01(struct bregs *regs)
> >              mode_attr |= VBE_MODE_ATTRIBUTE_LINEAR_FRAME_BUFFER_MODE;
> >          break;
> >      }
> > if (pages > 128)
> >    pages = 128;
>
> As 1 gets subtracted (below in diff), in this case 127 is included.
> > if (pages < 2)
> >    pages++;
>
> Can pages be 0? If yes, it would be 0 again after substracting 1. Should
> it be `pages = 1` instead of `pages++`?

The goal was to report 0 if there isn't enough memory for even a
single image.  Ideally that would not happen in practice as it
shouldn't even be presented as a valid video mode if there isn't
enough memory for it.

Otherwise, the goal is to report between 1-127 (inclusive) for the
number of pages.  So, if there is only enough ram for 1 image, then
the code would report 1 (which means 2 images).  If there is enough
ram for more than 128 images, then the code would report 127 (which
means 128).  This is what the AMD VGABIOS appears to be doing.

-Kevin