And while at it, unify it as clock frequency in Hz, to match the value in
grub_serial_config struct and do the division by 16 in one common place.
This will simplify adding SPCR support.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This adds the ability for the driver to access UARTs via MMIO instead
of PIO selectively at runtime, and exposes a new function to add an
MMIO port.
In an ideal world, MMIO accessors would be generic and have architecture
specific memory barriers. However, existing drivers don't have them and
most of those "bare metal" drivers tend to be for x86 which doesn't need
them. If necessary, those can be added later.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Coordinates passed to screen_write_char() did not have any checks to
ensure they are not out-of-bounds. This adds an if statement to prevent
out-of-bounds writes to the VGA text buffer.
Signed-off-by: Ryan Cohen <rcohenprogramming@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The GRUB is failing to build with GCC-12 in many places like this:
In function 'init_cbfsdisk',
inlined from 'grub_mod_init' at ../../grub-core/fs/cbfs.c:391:3:
../../grub-core/fs/cbfs.c:345:7: error: array subscript 0 is outside array bounds of 'grub_uint32_t[0]' {aka 'unsigned int[]'} [-Werror=array-bounds]
345 | ptr = *(grub_uint32_t *) 0xfffffffc;
| ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This is caused by GCC regression in 11/12 [1]. In a nut shell, the
warning is about detected invalid accesses at non-zero offsets to NULL
pointers. Since hardwired constant address is treated as NULL plus an
offset in the same underlying code, the warning is therefore triggered.
Instead of inserting #pragma all over the places where literal pointers
are accessed to avoid diagnosing array-bounds, we can try to borrow the
idea from Linux kernel that the absolute_pointer() macro [2][3] is used
to disconnect a pointer using literal address from it's original object,
hence GCC won't be able to make assumptions on the boundary while doing
pointer arithmetic. With that we can greatly reduce the code we have to
cover up by making initial literal pointer assignment to use the new
wrapper but not having to track everywhere literal pointers are
accessed. This also makes code looks cleaner.
Please note the grub_absolute_pointer() macro requires to be invoked in
a function as long as it is compound expression. Some global variables
with literal pointers has been changed to local ones in order to use
grub_absolute_pointer() to initialize it. The shuffling is basically done
in a selective and careful way that the variable's scope doesn't matter
being local or global, for example, the global variable must not get
modified at run time throughout. For the record, here's the list of
global variables got shuffled in this patch:
grub-core/commands/i386/pc/drivemap.c:int13slot
grub-core/term/i386/pc/console.c:bios_data_area
grub-core/term/ns8250.c:serial_hw_io_addr
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578
[2] https://elixir.bootlin.com/linux/v5.16.14/source/include/linux/compiler.h#L180
[3] https://elixir.bootlin.com/linux/v5.16.14/source/include/linux/compiler-gcc.h#L31
Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
To allow flickerfree boot the EFI console code does not call
grub_efi_set_text_mode(1) until some text is actually output. Depending
on if the output text is because of an error loading, e.g. the .cfg
file, or because of showing the menu the cursor needs to be on or off
when the first text is shown. So far the cursor was hardcoded to being
on, but this is causing drawing artifacts + slow drawing of the menu as
reported here: https://bugzilla.redhat.com/show_bug.cgi?id=1946969
Handle the cursorstate in the same way as the colorstate to fix this,
when no text has been output yet, just cache the cursorstate and then
use the last set value when the first text is output.
Fixes: 2d7c3abd871f (efi/console: Do not set text-mode until we actually need it)
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1946969
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
GRUB_MOD_INIT(normal) does an unconditional:
grub_env_set ("color_normal", "light-gray/black");
which triggers a grub_term_setcolorstate() call. The original version
of the "efi/console: Do not set text-mode until we actually need it" patch,
https://lists.gnu.org/archive/html/grub-devel/2018-03/msg00125.html,
protected against this by caching the requested state in
grub_console_setcolorstate() and then only applying it when the first
text output actually happens. During refactoring to move the
grub_console_setcolorstate() up higher in the grub-core/term/efi/console.c
file the code to cache the color-state + bail early was accidentally dropped.
Restore the cache the color-state + bail early behavior from the original.
Fixes: 2d7c3abd871f (efi/console: Do not set text-mode until we actually need it)
Cc: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Additionally, fix the terminfo spelling mistake in
the GRUB development documentation.
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Catch the case where we have a font so big that it causes the number of
rows or columns to be 0. Currently we continue and allocate a
virtual_screen.text_buffer of size 0. We then try to use that for glpyhs
and things go badly.
On the emu platform, malloc() may give us a valid pointer, in which case
we'll access heap memory which we shouldn't. Alternatively, it may give us
NULL, in which case we'll crash. For other platforms, if I understand
grub_memalign() correctly, we will receive a valid but small allocation
that we will very likely later overrun.
Prevent the creation of a virtual screen that isn't at least 40 cols
by 12 rows. This is arbitrary, but it seems that if your width or height
is half a standard 80x24 terminal, you're probably going to struggle to
read anything anyway.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
When building with --target=arm-linux-gnu --with-platform=coreboot
a linking error occurs caused by multiple definitions of the
ps2_state variable.
Mark them as static since they aren't used outside their compilation unit.
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This requires a very weird input from the serial interface but can cause
an overflow in input_buf (keys) overwriting the next variable (npending)
with the user choice:
(pahole output)
struct grub_terminfo_input_state {
int input_buf[6]; /* 0 24 */
int npending; /* 24 4 */ <- CORRUPT
...snip...
The magic string requires causing this is "ESC,O,],0,1,2,q" and we overflow
npending with "q" (aka increase npending to 161). The simplest fix is to
just to disallow overwrites input_buf, which exactly what this patch does.
Fixes: CID 292449
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
If we're running with a hidden menu we may never need text mode, so do not
change the video-mode to text until we actually need it.
This allows to boot a machine without unnecessary graphical transitions and
provide a seamless boot experience to users.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Implement getkeystatus() support in the EFI console driver.
This is needed because the logic to determine if a key was pressed to make
the menu countdown stop will be changed by a later patch to also take into
account the SHIFT key being held down.
For this reason the EFI console driver has to support getkeystatus() to
allow detecting that event.
Note that if a non-modifier key gets pressed and repeated calls to
getkeystatus() are made then it will return the modifier status at the
time of the non-modifier key, until that key-press gets consumed by a
getkey() call.
This is a side-effect of how the EFI simple-text-input protocol works
and cannot be avoided.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This is a preparatory patch for adding getkeystatus() support to the
EFI console driver.
We can get modifier status through the simple_text_input read_key_stroke()
method, but if a non-modifier key is (also) pressed the read_key_stroke()
call will consume that key from the firmware's queue.
The new grub_console_read_key_stroke() helper buffers upto 1 key-stroke.
If it has a non-modifier key buffered, it will return that one, if its
buffer is empty, it will fills its buffer by getting a new key-stroke.
If called with consume=1 it will empty its buffer after copying the
key-data to the callers buffer, this is how getkey() will use it.
If called with consume=0 it will keep the last key-stroke buffered, this
is how getkeystatus() will call it. This means that if a non-modifier
key gets pressed, repeated getkeystatus() calls will return the modifiers
of that key-press until it is consumed by a getkey() call.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This is just a preparatory patch to move the functions higher in the file,
since these will be called by the grub_prepare_for_text_output() function
that will be introduced in a later patch.
The logic is unchanged by this patch. Functions definitions are just moved
to avoid a forward declaration in a later patch, keeping the code clean.
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Currently the string functions grub_strtol(), grub_strtoul(), and
grub_strtoull() don't declare the "end" pointer in such a way as to
require the pointer itself or the character array to be immutable to the
implementation, nor does the C standard do so in its similar functions,
though it does require us not to change any of it.
The typical declarations of these functions follow this pattern:
long
strtol(const char * restrict nptr, char ** restrict endptr, int base);
Much of the reason for this is historic, and a discussion of that
follows below, after the explanation of this change. (GRUB currently
does not include the "restrict" qualifiers, and we name the arguments a
bit differently.)
The implementation is semantically required to treat the character array
as immutable, but such accidental modifications aren't stopped by the
compiler, and the semantics for both the callers and the implementation
of these functions are sometimes also helped by adding that requirement.
This patch changes these declarations to follow this pattern instead:
long
strtol(const char * restrict nptr,
const char ** const restrict endptr,
int base);
This means that if any modification to these functions accidentally
introduces either an errant modification to the underlying character
array, or an accidental assignment to endptr rather than *endptr, the
compiler should generate an error. (The two uses of "restrict" in this
case basically mean strtol() isn't allowed to modify the character array
by going through *endptr, and endptr isn't allowed to point inside the
array.)
It also means the typical use case changes to:
char *s = ...;
const char *end;
long l;
l = strtol(s, &end, 10);
Or even:
const char *p = str;
while (p && *p) {
long l = strtol(p, &p, 10);
...
}
This fixes 26 places where we discard our attempts at treating the data
safely by doing:
const char *p = str;
long l;
l = strtol(p, (char **)&ptr, 10);
It also adds 5 places where we do:
char *p = str;
while (p && *p) {
long l = strtol(p, (const char ** const)&p, 10);
...
/* more calls that need p not to be pointer-to-const */
}
While moderately distasteful, this is a better problem to have.
With one minor exception, I have tested that all of this compiles
without relevant warnings or errors, and that /much/ of it behaves
correctly, with gcc 9 using 'gcc -W -Wall -Wextra'. The one exception
is the changes in grub-core/osdep/aros/hostdisk.c , which I have no idea
how to build.
Because the C standard defined type-qualifiers in a way that can be
confusing, in the past there's been a slow but fairly regular stream of
churn within our patches, which add and remove the const qualifier in many
of the users of these functions. This change should help avoid that in
the future, and in order to help ensure this, I've added an explanation
in misc.h so that when someone does get a compiler warning about a type
error, they have the fix at hand.
The reason we don't have "const" in these calls in the standard is
purely anachronistic: C78 (de facto) did not have type qualifiers in the
syntax, and the "const" type qualifier was added for C89 (I think; it
may have been later). strtol() appears to date from 4.3BSD in 1986,
which means it could not be added to those functions in the standard
without breaking compatibility, which is usually avoided.
The syntax chosen for type qualifiers is what has led to the churn
regarding usage of const, and is especially confusing on string
functions due to the lack of a string type. Quoting from C99, the
syntax is:
declarator:
pointer[opt] direct-declarator
direct-declarator:
identifier
( declarator )
direct-declarator [ type-qualifier-list[opt] assignment-expression[opt] ]
...
direct-declarator [ type-qualifier-list[opt] * ]
...
pointer:
* type-qualifier-list[opt]
* type-qualifier-list[opt] pointer
type-qualifier-list:
type-qualifier
type-qualifier-list type-qualifier
...
type-qualifier:
const
restrict
volatile
So the examples go like:
const char foo; // immutable object
const char *foo; // mutable pointer to object
char * const foo; // immutable pointer to mutable object
const char * const foo; // immutable pointer to immutable object
const char const * const foo; // XXX extra const keyword in the middle
const char * const * const foo; // immutable pointer to immutable
// pointer to immutable object
const char ** const foo; // immutable pointer to mutable pointer
// to immutable object
Making const left-associative for * and right-associative for everything
else may not have been the best choice ever, but here we are, and the
inevitable result is people using trying to use const (as they should!),
putting it at the wrong place, fighting with the compiler for a bit, and
then either removing it or typecasting something in a bad way. I won't
go into describing restrict, but its syntax has exactly the same issue
as with const.
Anyway, the last example above actually represents the *behavior* that's
required of strtol()-like functions, so that's our choice for the "end"
pointer.
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This patch fixes an issue that prevented the at_keyboard module to work
(for me). The cause was a bad/wrong return value in the
grub_at_keyboard_getkey() function in grub-core/term/at_keyboard.c file
at line 237. My symptoms were to have an unresponsive keyboard. Keys
needed to be pressed 10x and more to effectively be printed sometimes
generating multiple key presses (after 1 or 2 sec of no printing). It
was very problematic when typing passphrase in early stage (with
GRUB_ENABLE_CRYPTODISK). When switched to "console" terminal input
keyboard worked perfectly. It also worked great with the GRUB 2.02
packaged by Debian (2.02+dfsg1-20). It was not an output issue but an
input one.
I've managed to analyze the issue and found that it came from the commit
216950a4e (at_keyboard: Split protocol from controller code.). Three
lines where moved from the fetch_key() function in
grub-core/term/at_keyboard.c file to the beginning of
grub_at_keyboard_getkey() function (same file). However, returning -1
made sense when it happened in fetch_key() function but not anymore in
grub_at_keyboard_getkey() function which should return GRUB_TERM_NO_KEY.
I think it was just an incomplete cut-paste missing a small manual
correction. Let's fix it.
Note: Commit message updated by Daniel Kiper.
Signed-off-by: Michael Bideau <mica.devel@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Most 8" or 7" x86 Windows 10 tablets come with volume up/down buttons and
a power-button. In their UEFI these are almost always mapped to arrow
up/down and enter.
Pressing the volume buttons (sometimes by accident) will stop the
menu countdown, but the power-button / "enter" key was not being recognized
as enter, so the user would be stuck at the grub menu.
The problem is that these tablets send scan_code 13 or 0x0d for the
power-button, which officialy maps to the F3 key. They also set
unicode_char to 0x0d.
This commit recognizes the special case of both scan_code and unicode_char
being set to 0x0d and treats this as an enter key press.
This fixes things getting stuck at the grub-menu and allows the user
to choice a grub-menu entry using the buttons on the tablet.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
If ascent is bigger than height - 2, then we draw over character box but then
to clear cursor we only draw over character box. So trim ascent if necessarry.
