From: David Mosberger <davidm@napali.hpl.hp.com>
To: linux-ia64@vger.kernel.org
Subject: fix memory corruption/crash for physical-mode EFI calls
Date: Sat, 10 Jul 2004 04:39:00 +0000 [thread overview]
Message-ID: <16623.29412.381259.793452@napali.hpl.hp.com> (raw)
Jesse,
I think you'll want to try out the patch below. If my guess is
correct and the SGI firmware doesn't support switching into virtual
mode, then this may fix the boot-problem you are seeing on SN2.
We found this problem after I noticed that the Ski simulator always
wanted to fsck its filesystem. That turned out to be because the
phys_get_time() in efi.c used __pa() to convert the address of a
stack-variable to a physical address. Only problem was that the
stack-variable was on the init-task's stack, so it was in region 5.
Effectively, this ended up writing the correct time to a bogus memory
address. In the simulator, that was harmless apart from not returning
the correct time, but in a real machine, it would likely lead to a
machine-check. We never saw this problem on tiger or zx1-based
machines because by the time efi_get_time() is called, they have
switched EFI into virtual mode, which obviates the need to do the
virtual->physical conversion.
The patch below looks bigger than what's really going on: all it does
is convert __pa() to ia64_tpa(), with some extra code to allow NULL
pointers for optional arguments.
Happy hacking,
--david
=== arch/ia64/kernel/efi.c 1.34 vs edited ==--- 1.34/arch/ia64/kernel/efi.c 2004-05-10 11:38:38 -07:00
+++ edited/arch/ia64/kernel/efi.c 2004-07-09 21:24:38 -07:00
@@ -43,18 +43,20 @@
#define efi_call_virt(f, args...) (*(f))(args)
-#define STUB_GET_TIME(prefix, adjust_arg) \
-static efi_status_t \
-prefix##_get_time (efi_time_t *tm, efi_time_cap_t *tc) \
-{ \
- struct ia64_fpreg fr[6]; \
- efi_status_t ret; \
- \
- ia64_save_scratch_fpregs(fr); \
- ret = efi_call_##prefix((efi_get_time_t *) __va(runtime->get_time), adjust_arg(tm), \
- adjust_arg(tc)); \
- ia64_load_scratch_fpregs(fr); \
- return ret; \
+#define STUB_GET_TIME(prefix, adjust_arg) \
+static efi_status_t \
+prefix##_get_time (efi_time_t *tm, efi_time_cap_t *tc) \
+{ \
+ struct ia64_fpreg fr[6]; \
+ efi_time_cap_t *atc = 0; \
+ efi_status_t ret; \
+ \
+ if (tc) \
+ atc = adjust_arg(tc); \
+ ia64_save_scratch_fpregs(fr); \
+ ret = efi_call_##prefix((efi_get_time_t *) __va(runtime->get_time), adjust_arg(tm), atc); \
+ ia64_load_scratch_fpregs(fr); \
+ return ret; \
}
#define STUB_SET_TIME(prefix, adjust_arg) \
@@ -89,11 +91,14 @@
prefix##_set_wakeup_time (efi_bool_t enabled, efi_time_t *tm) \
{ \
struct ia64_fpreg fr[6]; \
+ efi_time_t *atm = 0; \
efi_status_t ret; \
\
+ if (tm) \
+ atm = adjust_arg(tm); \
ia64_save_scratch_fpregs(fr); \
ret = efi_call_##prefix((efi_set_wakeup_time_t *) __va(runtime->set_wakeup_time), \
- enabled, adjust_arg(tm)); \
+ enabled, atm); \
ia64_load_scratch_fpregs(fr); \
return ret; \
}
@@ -104,11 +109,14 @@
unsigned long *data_size, void *data) \
{ \
struct ia64_fpreg fr[6]; \
+ u32 *aattr = 0; \
efi_status_t ret; \
\
+ if (attr) \
+ aattr = adjust_arg(attr); \
ia64_save_scratch_fpregs(fr); \
ret = efi_call_##prefix((efi_get_variable_t *) __va(runtime->get_variable), \
- adjust_arg(name), adjust_arg(vendor), adjust_arg(attr), \
+ adjust_arg(name), adjust_arg(vendor), aattr, \
adjust_arg(data_size), adjust_arg(data)); \
ia64_load_scratch_fpregs(fr); \
return ret; \
@@ -164,33 +172,41 @@
unsigned long data_size, efi_char16_t *data) \
{ \
struct ia64_fpreg fr[6]; \
+ efi_char16_t *adata = 0; \
+ \
+ if (data) \
+ adata = adjust_arg(data); \
\
ia64_save_scratch_fpregs(fr); \
efi_call_##prefix((efi_reset_system_t *) __va(runtime->reset_system), \
- reset_type, status, data_size, adjust_arg(data)); \
+ reset_type, status, data_size, adata); \
/* should not return, but just in case... */ \
ia64_load_scratch_fpregs(fr); \
}
-STUB_GET_TIME(phys, __pa)
-STUB_SET_TIME(phys, __pa)
-STUB_GET_WAKEUP_TIME(phys, __pa)
-STUB_SET_WAKEUP_TIME(phys, __pa)
-STUB_GET_VARIABLE(phys, __pa)
-STUB_GET_NEXT_VARIABLE(phys, __pa)
-STUB_SET_VARIABLE(phys, __pa)
-STUB_GET_NEXT_HIGH_MONO_COUNT(phys, __pa)
-STUB_RESET_SYSTEM(phys, __pa)
-
-STUB_GET_TIME(virt, )
-STUB_SET_TIME(virt, )
-STUB_GET_WAKEUP_TIME(virt, )
-STUB_SET_WAKEUP_TIME(virt, )
-STUB_GET_VARIABLE(virt, )
-STUB_GET_NEXT_VARIABLE(virt, )
-STUB_SET_VARIABLE(virt, )
-STUB_GET_NEXT_HIGH_MONO_COUNT(virt, )
-STUB_RESET_SYSTEM(virt, )
+#define phys_ptr(arg) ((__typeof__(arg)) ia64_tpa(arg))
+
+STUB_GET_TIME(phys, phys_ptr)
+STUB_SET_TIME(phys, phys_ptr)
+STUB_GET_WAKEUP_TIME(phys, phys_ptr)
+STUB_SET_WAKEUP_TIME(phys, phys_ptr)
+STUB_GET_VARIABLE(phys, phys_ptr)
+STUB_GET_NEXT_VARIABLE(phys, phys_ptr)
+STUB_SET_VARIABLE(phys, phys_ptr)
+STUB_GET_NEXT_HIGH_MONO_COUNT(phys, phys_ptr)
+STUB_RESET_SYSTEM(phys, phys_ptr)
+
+#define id(arg) arg
+
+STUB_GET_TIME(virt, id)
+STUB_SET_TIME(virt, id)
+STUB_GET_WAKEUP_TIME(virt, id)
+STUB_SET_WAKEUP_TIME(virt, id)
+STUB_GET_VARIABLE(virt, id)
+STUB_GET_NEXT_VARIABLE(virt, id)
+STUB_SET_VARIABLE(virt, id)
+STUB_GET_NEXT_HIGH_MONO_COUNT(virt, id)
+STUB_RESET_SYSTEM(virt, id)
void
efi_gettimeofday (struct timespec *ts)
next reply other threads:[~2004-07-10 4:39 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2004-07-10 4:39 David Mosberger [this message]
2004-07-10 16:54 ` fix memory corruption/crash for physical-mode EFI calls Jesse Barnes
2004-07-12 20:41 ` David Mosberger
2004-07-13 14:54 ` Jack Steiner
2004-07-13 15:18 ` Jesse Barnes
2004-07-13 22:00 ` David Mosberger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=16623.29412.381259.793452@napali.hpl.hp.com \
--to=davidm@napali.hpl.hp.com \
--cc=linux-ia64@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox