On 05.07.17 19:47, Heinrich Schuchardt wrote:
Up to now the boot time supported only a single event. This patch now allows four events.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
include/efi_api.h | 8 ++- include/efi_loader.h | 23 ++++++ lib/efi_loader/efi_boottime.c | 161 ++++++++++++++++++++++++++---------------- 3 files changed, 131 insertions(+), 61 deletions(-)
diff --git a/include/efi_api.h b/include/efi_api.h index 42cd47ff08..18bef722c6 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -28,8 +28,12 @@ enum efi_event_type { EFI_TIMER_RELATIVE = 2 };
-#define EVT_NOTIFY_WAIT 0x00000100 -#define EVT_NOTIFY_SIGNAL 0x00000200 +#define EVT_TIMER 0x80000000 +#define EVT_RUNTIME 0x40000000 +#define EVT_NOTIFY_WAIT 0x00000100 +#define EVT_NOTIFY_SIGNAL 0x00000200 +#define EVT_SIGNAL_EXIT_BOOT_SERVICES 0x00000201 +#define EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE 0x60000202
/* EFI Boot Services table */ struct efi_boot_services { diff --git a/include/efi_loader.h b/include/efi_loader.h index c620652307..a35b971f7e 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -71,6 +71,29 @@ struct efi_object { void *handle; };
+/**
- struct efi_event
- @type: Type of event
- @trigger_type: Type of timer
- @trigger_time: Period of the timer
- @trigger_next: Next time to trigger the timer
- @nofify_function: Function to call when the event is triggered
- @notify_context: Data to be passed to the notify function
- @signaled: The notify function was already called
- */
+struct efi_event {
- u32 type;
- unsigned long notify_tpl;
- void (EFIAPI *notify_function)(void *event, void *context);
- void *notify_context;
- u64 trigger_next;
- u32 trigger_time;
- enum efi_event_type trigger_type;
- int signaled;
+};
- /* This list contains all UEFI objects we know of */ extern struct list_head efi_obj_list;
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 3060c25a2a..ef3e7d9d52 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -162,21 +162,10 @@ static efi_status_t EFIAPI efi_free_pool_ext(void *buffer) }
/*
- Our event capabilities are very limited. Only support a single
- event to exist, so we don't need to maintain lists.
- Our event capabilities are very limited. Only a small limited
*/
- number of events is allowed to coexist.
-static struct {
- enum efi_event_type type;
- u32 trigger_type;
- u32 trigger_time;
- u64 trigger_next;
- unsigned long notify_tpl;
- void (EFIAPI *notify_function) (void *event, void *context);
- void *notify_context;
-} efi_event = {
- /* Disable timers on bootup */
- .trigger_next = -1ULL,
-}; +static struct efi_event efi_events[16];
static efi_status_t EFIAPI efi_create_event( enum efi_event_type type, ulong notify_tpl, @@ -184,13 +173,10 @@ static efi_status_t EFIAPI efi_create_event( void *context), void *notify_context, void **event) {
- int i;
- EFI_ENTRY("%d, 0x%lx, %p, %p", type, notify_tpl, notify_function, notify_context);
- if (efi_event.notify_function) {
/* We only support one event at a time */return EFI_EXIT(EFI_OUT_OF_RESOURCES);- }
- if (event == NULL) return EFI_EXIT(EFI_INVALID_PARAMETER);
@@ -201,13 +187,20 @@ static efi_status_t EFIAPI efi_create_event( notify_function == NULL) return EFI_EXIT(EFI_INVALID_PARAMETER);
- efi_event.type = type;
- efi_event.notify_tpl = notify_tpl;
- efi_event.notify_function = notify_function;
- efi_event.notify_context = notify_context;
- *event = &efi_event;
- return EFI_EXIT(EFI_SUCCESS);
- for (i = 0; i < ARRAY_SIZE(efi_events); ++i) {
if (efi_events[i].type)
Please explicitly check for EFI_TIMER_STOP here.
continue;efi_events[i].type = type;efi_events[i].notify_tpl = notify_tpl;efi_events[i].notify_function = notify_function;efi_events[i].notify_context = notify_context;/* Disable timers on bootup */efi_events[i].trigger_next = -1ULL;efi_events[i].signaled = 0;*event = &efi_events[i];return EFI_EXIT(EFI_SUCCESS);}
return EFI_EXIT(EFI_OUT_OF_RESOURCES); }
/*
@@ -216,17 +209,25 @@ static efi_status_t EFIAPI efi_create_event( */ void efi_timer_check(void) {
- int i; u64 now = timer_get_us();
- if (now >= efi_event.trigger_next) {
/* Triggering! */if (efi_event.trigger_type == EFI_TIMER_PERIODIC)efi_event.trigger_next += efi_event.trigger_time / 10;if (efi_event.type & (EVT_NOTIFY_WAIT | EVT_NOTIFY_SIGNAL))efi_event.notify_function(&efi_event,efi_event.notify_context);
- for (i = 0; i < ARRAY_SIZE(efi_events); ++i) {
if (!(efi_events[i].type & EVT_TIMER) ||efi_events[i].trigger_type == EFI_TIMER_STOP ||now < efi_events[i].trigger_next)continue;
This if clause is quite hard to read. I don't think it would hurt to just unfold it into individual cases? You can then also document for each why they should get ignored.
if (efi_events[i].trigger_type == EFI_TIMER_PERIODIC)efi_events[i].trigger_next +=efi_events[i].trigger_time / 10;
I stumbled over my own code here - awesome :). Can you please put the division into a static function that tells the reader from which number space into which number space we're converting (100ns)?
efi_events[i].signaled = 1;if (efi_events[i].type & EVT_NOTIFY_SIGNAL) {EFI_EXIT(EFI_SUCCESS);
This is quite dangerous...
efi_events[i].notify_function(&efi_events[i],efi_events[i].notify_context);efi_restore_gd();
... I see that you do restore gd but please document this heavily with a big, obvious comment and ideally even use EFI_ENTRY() again here instead of hard coding efi_restore_gd().
}
wrong indentation
}
- WATCHDOG_RESET(); }
@@ -236,67 +237,109 @@ static efi_status_t EFIAPI efi_set_timer(void *event, int type, /* We don't have 64bit division available everywhere, so limit timer * distances to 32bit bits. */ u32 trigger32 = trigger_time;
int i;
EFI_ENTRY("%p, %d, %"PRIx64, event, type, trigger_time);
if (trigger32 < trigger_time) {
trigger32 = 0xffffffff;
This change should be a separate patch I suppose. Makes bisecting things easier.
printf("WARNING: Truncating timer from %"PRIx64" to %x\n", trigger_time, trigger32);}
- if (event != &efi_event) {
/* We only support one event at a time */return EFI_EXIT(EFI_INVALID_PARAMETER);- }
- for (i = 0; i < ARRAY_SIZE(efi_events); ++i) {
if (event != &efi_events[i])continue;
- switch (type) {
- case EFI_TIMER_STOP:
efi_event.trigger_next = -1ULL;break;- case EFI_TIMER_PERIODIC:
- case EFI_TIMER_RELATIVE:
efi_event.trigger_next = timer_get_us() + (trigger32 / 10);break;- default:
return EFI_EXIT(EFI_INVALID_PARAMETER);
switch (type) {case EFI_TIMER_STOP:efi_events[i].trigger_next = -1ULL;break;case EFI_TIMER_PERIODIC:case EFI_TIMER_RELATIVE:efi_events[i].trigger_next =timer_get_us() + (trigger32 / 10);break;default:return EFI_EXIT(EFI_INVALID_PARAMETER);}efi_events[i].trigger_type = type;efi_events[i].trigger_time = trigger_time;return EFI_EXIT(EFI_SUCCESS);
- efi_event.trigger_type = type;
- efi_event.trigger_time = trigger_time;
- return EFI_EXIT(EFI_SUCCESS);
return EFI_EXIT(EFI_INVALID_PARAMETER); }
static efi_status_t EFIAPI efi_wait_for_event(unsigned long num_events, void *event, unsigned long *index) {
- u64 now;
int i;
EFI_ENTRY("%ld, %p, %p", num_events, event, index);
- now = timer_get_us();
- while (now < efi_event.trigger_next) { }
- efi_timer_check();
- for (i = 0; i < ARRAY_SIZE(efi_events); ++i) {
if (event != &efi_events[i])continue;
We also need to check if EVT_NOTIFY_SIGNAL is set and error out in that case, no?
while (!efi_events[i].signaled)efi_timer_check();
Ah, nice, so your new code also resets the watchdog. Very good :).
efi_events[i].signaled = 0;
Please document that line with reference to the spec.
break;} return EFI_EXIT(EFI_SUCCESS); }
static efi_status_t EFIAPI efi_signal_event(void *event) {
int i;
EFI_ENTRY("%p", event);
for (i = 0; i < ARRAY_SIZE(efi_events); ++i) {
if (event != &efi_events[i])continue;if (efi_events[i].signaled)break;efi_events[i].signaled = 1;if (efi_events[i].type & EVT_NOTIFY_SIGNAL) {EFI_EXIT(EFI_SUCCESS);efi_events[i].notify_function(&efi_events[i],efi_events[i].notify_context);efi_restore_gd();
This looks like code repetition. Can you fold it in with the same code above?
}break;} return EFI_EXIT(EFI_SUCCESS); }
static efi_status_t EFIAPI efi_close_event(void *event) {
int i;
EFI_ENTRY("%p", event);
- efi_event.trigger_next = -1ULL;
- for (i = 0; i < ARRAY_SIZE(efi_events); ++i) {
if (event != &efi_events[i])continue;efi_events[i].type = 0;
Use STOPPED here.
efi_events[i].trigger_next = -1ULL;efi_events[i].signaled = 0;break;} return EFI_EXIT(EFI_SUCCESS); }
static efi_status_t EFIAPI efi_check_event(void *event) {
int i;
EFI_ENTRY("%p", event);
- return EFI_EXIT(EFI_NOT_READY);
- efi_timer_check();
- for (i = 0; i < ARRAY_SIZE(efi_events); ++i) {
Here to we need to check for EVT_NOTIFY_SIGNAL and error out, no?
Alex
if (event != &efi_events[i])continue;if (efi_events[i].signaled)return EFI_EXIT(EFI_SUCCESS);return EFI_EXIT(EFI_NOT_READY);}
return EFI_EXIT(EFI_INVALID_PARAMETER); }
static efi_status_t EFIAPI efi_install_protocol_interface(void **handle,