2004-04-20 22:13:10

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH] add some EFI device smarts

This patch against current 2.6 BK adds efi_get_variable() and
efi_uart_console_only().

The first thing I want to use this for is smarter console detection.
In the absence of a "console=" argument, we currently always default
to a VGA console. But if the EFI ConOut variable includes only UART
devices, we ought to take advantage of that and use a serial console
instead.

This is based on previous work by Khalid Aziz and Alex Williamson.

(Like much of the EFI stuff, this really isn't ia64-specific. Maybe
it's time to move some of it under drivers/efi? If there's interest,
I can look at doing that.)

===== arch/ia64/kernel/efi.c 1.31 vs edited =====
--- 1.31/arch/ia64/kernel/efi.c Mon Apr 12 04:21:11 2004
+++ edited/arch/ia64/kernel/efi.c Tue Apr 20 14:46:28 2004
@@ -747,6 +747,71 @@
return 0;
}

+int
+efi_get_variable(char *name, unsigned char *data, unsigned long *size)
+{
+ efi_status_t status;
+ efi_guid_t vendor_guid;
+ efi_char16_t name_unicode[128];
+ char name_utf8[ARRAY_SIZE(name_unicode) + 1];
+ unsigned long name_size;
+ int i;
+
+ memset(name_unicode, 0, sizeof(name_unicode));
+ for (;;) {
+ name_size = sizeof(name_unicode);
+ status = efi.get_next_variable(&name_size, name_unicode,
+ &vendor_guid);
+ if (status != EFI_SUCCESS)
+ return -EINVAL;
+
+ /* Convert Unicode to normal chars */
+ for (i = 0; i < (name_size/sizeof(name_unicode[0])); i++)
+ name_utf8[i] = name_unicode[i] & 0xff;
+ name_utf8[i] = 0;
+
+ if (strcmp(name_utf8, name))
+ continue;
+
+ status = efi.get_variable(name_unicode, &vendor_guid, NULL,
+ size, data);
+ if (status != EFI_SUCCESS)
+ return -EINVAL;
+
+ return 0;
+ }
+}
+
+int __init
+efi_uart_console_only(void)
+{
+ unsigned char data[1024];
+ unsigned long size = sizeof(data);
+ efi_generic_dev_path_t *hdr, *end_addr;
+ int uart = 0;
+
+ if (efi_get_variable("ConOut", data, &size) < 0)
+ return 0;
+
+ hdr = (efi_generic_dev_path_t *) data;
+ end_addr = (efi_generic_dev_path_t *) ((u8 *) data + size);
+ while (hdr < end_addr) {
+ if (hdr->type == EFI_DEV_MSG &&
+ hdr->sub_type == EFI_DEV_MSG_UART)
+ uart = 1;
+ else if (hdr->type == EFI_DEV_END_PATH ||
+ hdr->type == EFI_DEV_END_PATH2) {
+ if (!uart)
+ return 0;
+ if (hdr->sub_type == EFI_DEV_END_ENTIRE)
+ return 1;
+ uart = 0;
+ }
+ hdr = (efi_generic_dev_path_t *) ((u8 *) hdr + hdr->length);
+ }
+ return 0; /* malformed path (no end) */
+}
+
static void __exit
efivars_exit (void)
{
===== include/linux/efi.h 1.7 vs edited =====
--- 1.7/include/linux/efi.h Tue Feb 3 22:29:14 2004
+++ edited/include/linux/efi.h Tue Apr 20 14:47:46 2004
@@ -294,6 +294,8 @@
extern u64 efi_get_iobase (void);
extern u32 efi_mem_type (unsigned long phys_addr);
extern u64 efi_mem_attributes (unsigned long phys_addr);
+extern int efi_get_variable (char *name, unsigned char *data, unsigned long *size);
+extern int __init efi_uart_console_only (void);
extern void efi_initialize_iomem_resources(struct resource *code_resource,
struct resource *data_resource);
extern efi_status_t phys_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc);
@@ -322,6 +324,49 @@
#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0000000000000002
#define EFI_VARIABLE_RUNTIME_ACCESS 0x0000000000000004

+/*
+ * EFI Device Path information
+ */
+#define EFI_DEV_HW 0x01
+#define EFI_DEV_PCI 1
+#define EFI_DEV_PCCARD 2
+#define EFI_DEV_MEM_MAPPED 3
+#define EFI_DEV_VENDOR 4
+#define EFI_DEV_CONTROLLER 5
+#define EFI_DEV_ACPI 0x02
+#define EFI_DEV_BASIC_ACPI 1
+#define EFI_DEV_EXPANDED_ACPI 2
+#define EFI_DEV_MSG 0x03
+#define EFI_DEV_MSG_ATAPI 1
+#define EFI_DEV_MSG_SCSI 2
+#define EFI_DEV_MSG_FC 3
+#define EFI_DEV_MSG_1394 4
+#define EFI_DEV_MSG_USB 5
+#define EFI_DEV_MSG_USB_CLASS 15
+#define EFI_DEV_MSG_I20 6
+#define EFI_DEV_MSG_MAC 11
+#define EFI_DEV_MSG_IPV4 12
+#define EFI_DEV_MSG_IPV6 13
+#define EFI_DEV_MSG_INFINIBAND 9
+#define EFI_DEV_MSG_UART 14
+#define EFI_DEV_MSG_VENDOR 10
+#define EFI_DEV_MEDIA 0x04
+#define EFI_DEV_MEDIA_HARD_DRIVE 1
+#define EFI_DEV_MEDIA_CDROM 2
+#define EFI_DEV_MEDIA_VENDOR 3
+#define EFI_DEV_MEDIA_FILE 4
+#define EFI_DEV_MEDIA_PROTOCOL 5
+#define EFI_DEV_BIOS_BOOT 0x05
+#define EFI_DEV_END_PATH 0x7F
+#define EFI_DEV_END_PATH2 0xFF
+#define EFI_DEV_END_INSTANCE 0x01
+#define EFI_DEV_END_ENTIRE 0xFF
+
+typedef struct {
+ u8 type;
+ u8 sub_type;
+ u16 length;
+} efi_generic_dev_path_t;

/*
* efi_dir is allocated in arch/ia64/kernel/efi.c.


2004-04-20 23:53:44

by Matt Domsch

[permalink] [raw]
Subject: Re: [PATCH] add some EFI device smarts

On Tue, Apr 20, 2004 at 04:00:26PM -0600, Bjorn Helgaas wrote:
> (Like much of the EFI stuff, this really isn't ia64-specific. Maybe
> it's time to move some of it under drivers/efi? If there's interest,
> I can look at doing that.)

Matt T. had done the work to move it under drivers/efi, though now
that there's a drivers/firmware, that's more appropraite. It also
converted it to use sysfs instead of proc. There was a bug in
efivars_exit() where it was removing stuff (which could sleep) while
holding a spinlock which wasn't good, but that was about the only
issue anyone had with it.

> + /* Convert Unicode to normal chars */
> + for (i = 0; i < (name_size/sizeof(name_unicode[0])); i++)
> + name_utf8[i] = name_unicode[i] & 0xff;
> + name_utf8[i] = 0;

I've never had a clear understanding of this. It's not really UTF8
(else straight ASCII text could be used), but more like UCS2. (Yeah,
I'm sure I named it wrong myself too in the rest of the file...)

> +
> + if (strcmp(name_utf8, name))
> + continue;

This ignores the fact that someone could create a variable with the
same name but a different vendor GUID, and it would return the first
one found. Unfortunately, you need to request both pieces
specifically -

+int
+efi_get_variable(char *name, efi_variable_t *guid, unsigned char *data, unsigned long *size)

and do a guidcmp() on them as well as the strcmp() on the name.

> +int __init
> +efi_uart_console_only(void)

So to be useful, efivars can't be build modular anymore, right? Then
Kconfig needs to change as well. It's module_init(), is that early
enough to be used? Where is efi_uart_console_only() called from?
It's not in this patch.

> +typedef struct {
> + u8 type;
> + u8 sub_type;
> + u16 length;
> +} efi_generic_dev_path_t;

No typedefs, just struct efi_generic_dev_path, and
__attribute__((packed)) please just to be safe.


Thanks,
Matt

--
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & http://www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com


Attachments:
(No filename) (2.01 kB)
(No filename) (189.00 B)
Download all attachments

2004-04-21 07:05:46

by Tolentino, Matthew E

[permalink] [raw]
Subject: RE: [PATCH] add some EFI device smarts

> On Tue, Apr 20, 2004 at 04:00:26PM -0600, Bjorn Helgaas wrote:
> > (Like much of the EFI stuff, this really isn't ia64-specific. Maybe
> > it's time to move some of it under drivers/efi? If there's
> interest,
> > I can look at doing that.)
>
> Matt T. had done the work to move it under drivers/efi, though now
> that there's a drivers/firmware, that's more appropraite. It also
> converted it to use sysfs instead of proc. There was a bug in
> efivars_exit() where it was removing stuff (which could sleep) while
> holding a spinlock which wasn't good, but that was about the only
> issue anyone had with it.

Indeed. I fixed that and one other small issue Greg pointed out
a while ago. I'll resend updated efivars driver patches in a separate
mail shortly.

> +int
> +efi_get_variable(char *name, efi_variable_t *guid, unsigned
> char *data, unsigned long *size)
>
> and do a guidcmp() on them as well as the strcmp() on the name.
>
> > +int __init
> > +efi_uart_console_only(void)
>
> So to be useful, efivars can't be build modular anymore, right? Then
> Kconfig needs to change as well. It's module_init(), is that early
> enough to be used? Where is efi_uart_console_only() called from?
> It's not in this patch.

This part isn't clear to me. It looks like you are including a
duplicate efi_get_variable function in efi.c in order to have
access to EFI variable services (and perform the checks) very early
before console initialization and/or efivars is available ... is
that correct?

thanks,
matt

2004-04-21 12:59:57

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH] add some EFI device smarts

On Tuesday 20 April 2004 5:50 pm, Matt Domsch wrote:
> Matt T. had done the work to move it under drivers/efi, though now
> that there's a drivers/firmware, that's more appropraite.

Sounds good. I'll let Matt T. worry about that for now.

> > + /* Convert Unicode to normal chars */
> > + for (i = 0; i < (name_size/sizeof(name_unicode[0])); i++)
> > + name_utf8[i] = name_unicode[i] & 0xff;
> > + name_utf8[i] = 0;
>
> I've never had a clear understanding of this. It's not really UTF8
> (else straight ASCII text could be used), but more like UCS2.

I don't understand Unicode either. Maybe the attached is a little
closer? The EFI spec (1.10, table 2-2) says strings are stored in
UTF-16, and I think that UTF-16 is the same as ASCII for
(0 < c <= 0x7f).

I also stuck in the GUID stuff and removed the typedef (I prefer
the plain structs myself, but was just following the existing style ;-)).

Oh, BTW, my changes are in efi.c, not efivars.c, so no Kconfig
or modular build implications.

I'll try to post the patch that calls efi_uart_console_only() today.
I just didn't want these EFI changes to get buried in the rest of
that. Thanks a lot for the feedback!

Bjorn

===== arch/ia64/kernel/efi.c 1.31 vs edited =====
--- 1.31/arch/ia64/kernel/efi.c Mon Apr 12 04:21:11 2004
+++ edited/arch/ia64/kernel/efi.c Wed Apr 21 06:31:33 2004
@@ -747,6 +747,72 @@
return 0;
}

+int
+efi_get_variable(char *name, efi_guid_t guid, unsigned char *data,
+ unsigned long *size, u32 *attributes)
+{
+ efi_status_t status;
+ efi_guid_t vendor_guid;
+ efi_char16_t name_utf16[128];
+ char name_ascii[ARRAY_SIZE(name_utf16) + 1];
+ unsigned long name_size;
+ int i;
+
+ memset(name_utf16, 0, sizeof(name_utf16));
+ for (;;) {
+ name_size = sizeof(name_utf16);
+ status = efi.get_next_variable(&name_size, name_utf16,
+ &vendor_guid);
+ if (status != EFI_SUCCESS)
+ return -EINVAL;
+
+ /* Convert UTF-16 to ASCII */
+ for (i = 0; i < (name_size/sizeof(name_utf16[0])); i++)
+ name_ascii[i] = name_utf16[i] & 0x7f;
+ name_ascii[i] = 0;
+
+ if (strcmp(name_ascii, name) || efi_guidcmp(guid, vendor_guid))
+ continue;
+
+ status = efi.get_variable(name_utf16, &guid, attributes,
+ size, data);
+ if (status != EFI_SUCCESS)
+ return -EINVAL;
+
+ return 0;
+ }
+}
+
+int __init
+efi_uart_console_only(void)
+{
+ unsigned char data[1024];
+ unsigned long size = sizeof(data);
+ struct efi_generic_dev_path *hdr, *end_addr;
+ int uart = 0;
+
+ if (efi_get_variable("ConOut", EFI_GLOBAL_VARIABLE_GUID, data, &size, NULL) < 0)
+ return 0;
+
+ hdr = (struct efi_generic_dev_path *) data;
+ end_addr = (struct efi_generic_dev_path *) ((u8 *) data + size);
+ while (hdr < end_addr) {
+ if (hdr->type == EFI_DEV_MSG &&
+ hdr->sub_type == EFI_DEV_MSG_UART)
+ uart = 1;
+ else if (hdr->type == EFI_DEV_END_PATH ||
+ hdr->type == EFI_DEV_END_PATH2) {
+ if (!uart)
+ return 0;
+ if (hdr->sub_type == EFI_DEV_END_ENTIRE)
+ return 1;
+ uart = 0;
+ }
+ hdr = (struct efi_generic_dev_path *) ((u8 *) hdr + hdr->length);
+ }
+ return 0; /* malformed path (no end) */
+}
+
static void __exit
efivars_exit (void)
{
===== arch/ia64/kernel/efivars.c 1.15 vs edited =====
--- 1.15/arch/ia64/kernel/efivars.c Tue Feb 10 19:51:27 2004
+++ edited/arch/ia64/kernel/efivars.c Wed Apr 21 06:41:25 2004
@@ -191,10 +191,9 @@
variable_name_size);
memcpy(&(new_efivar->var.VendorGuid), vendor_guid, sizeof(efi_guid_t));

- /* Convert Unicode to normal chars (assume top bits are 0),
- ala UTF-8 */
+ /* Convert UTF-16 to ASCII (assume top bits are 0) */
for (i=0; i< (int) (variable_name_size / sizeof(efi_char16_t)); i++) {
- short_name[i] = variable_name[i] & 0xFF;
+ short_name[i] = variable_name[i] & 0x7F;
}

/* This is ugly, but necessary to separate one vendor's
===== include/linux/efi.h 1.7 vs edited =====
--- 1.7/include/linux/efi.h Tue Feb 3 22:29:14 2004
+++ edited/include/linux/efi.h Wed Apr 21 06:25:22 2004
@@ -212,6 +212,9 @@
#define UGA_IO_PROTOCOL_GUID \
EFI_GUID( 0x61a4d49e, 0x6f68, 0x4f1b, 0xb9, 0x22, 0xa8, 0x6e, 0xed, 0xb, 0x7, 0xa2 )

+#define EFI_GLOBAL_VARIABLE_GUID \
+ EFI_GUID( 0x8be4df61, 0x93ca, 0x11d2, 0xaa, 0x0d, 0x00, 0xe0, 0x98, 0x03, 0x2b, 0x8c )
+
typedef struct {
efi_guid_t guid;
unsigned long table;
@@ -294,6 +297,8 @@
extern u64 efi_get_iobase (void);
extern u32 efi_mem_type (unsigned long phys_addr);
extern u64 efi_mem_attributes (unsigned long phys_addr);
+extern int efi_get_variable (char *name, efi_guid_t guid, unsigned char *data, unsigned long *size, u32 *attributes);
+extern int __init efi_uart_console_only (void);
extern void efi_initialize_iomem_resources(struct resource *code_resource,
struct resource *data_resource);
extern efi_status_t phys_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc);
@@ -322,6 +327,49 @@
#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0000000000000002
#define EFI_VARIABLE_RUNTIME_ACCESS 0x0000000000000004

+/*
+ * EFI Device Path information
+ */
+#define EFI_DEV_HW 0x01
+#define EFI_DEV_PCI 1
+#define EFI_DEV_PCCARD 2
+#define EFI_DEV_MEM_MAPPED 3
+#define EFI_DEV_VENDOR 4
+#define EFI_DEV_CONTROLLER 5
+#define EFI_DEV_ACPI 0x02
+#define EFI_DEV_BASIC_ACPI 1
+#define EFI_DEV_EXPANDED_ACPI 2
+#define EFI_DEV_MSG 0x03
+#define EFI_DEV_MSG_ATAPI 1
+#define EFI_DEV_MSG_SCSI 2
+#define EFI_DEV_MSG_FC 3
+#define EFI_DEV_MSG_1394 4
+#define EFI_DEV_MSG_USB 5
+#define EFI_DEV_MSG_USB_CLASS 15
+#define EFI_DEV_MSG_I20 6
+#define EFI_DEV_MSG_MAC 11
+#define EFI_DEV_MSG_IPV4 12
+#define EFI_DEV_MSG_IPV6 13
+#define EFI_DEV_MSG_INFINIBAND 9
+#define EFI_DEV_MSG_UART 14
+#define EFI_DEV_MSG_VENDOR 10
+#define EFI_DEV_MEDIA 0x04
+#define EFI_DEV_MEDIA_HARD_DRIVE 1
+#define EFI_DEV_MEDIA_CDROM 2
+#define EFI_DEV_MEDIA_VENDOR 3
+#define EFI_DEV_MEDIA_FILE 4
+#define EFI_DEV_MEDIA_PROTOCOL 5
+#define EFI_DEV_BIOS_BOOT 0x05
+#define EFI_DEV_END_PATH 0x7F
+#define EFI_DEV_END_PATH2 0xFF
+#define EFI_DEV_END_INSTANCE 0x01
+#define EFI_DEV_END_ENTIRE 0xFF
+
+struct efi_generic_dev_path {
+ u8 type;
+ u8 sub_type;
+ u16 length;
+} __attribute ((packed));

/*
* efi_dir is allocated in arch/ia64/kernel/efi.c.

2004-04-21 14:22:42

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH] add some EFI device smarts

On Wed, Apr 21, 2004 at 06:59:22AM -0600, Bjorn Helgaas wrote:
> > > + /* Convert Unicode to normal chars */
> > > + for (i = 0; i < (name_size/sizeof(name_unicode[0])); i++)
> > > + name_utf8[i] = name_unicode[i] & 0xff;
> > > + name_utf8[i] = 0;
> >
> > I've never had a clear understanding of this. It's not really UTF8
> > (else straight ASCII text could be used), but more like UCS2.
>
> I don't understand Unicode either. Maybe the attached is a little
> closer? The EFI spec (1.10, table 2-2) says strings are stored in
> UTF-16, and I think that UTF-16 is the same as ASCII for
> (0 < c <= 0x7f).

Ah, I know this one ...

UTF-16 is like UCS-2 except that it has escapes to let you use past the
first plane of Unicode. That is, by and large it's 2-bytes long, but
sometimes it can be 4 or more bytes long. How to convert? Let's see ...

for (i = 0, j = 0; i < (name_size/sizeof(name_unicode[0])); i++) {
unsigned int rune = name_unicode[i];
if (0xD7FF < rune && rune < 0xE000) {
rune = (rune & 0x7ff) << 10;
rune |= name_unicode[++i] & 0x3ff;
}
if (rune < 0x80) {
name_utf8[j++] = rune;
} else if (rune < 0x800) {
name_utf8[j++] = 0xc0 | (rune >> 6);
name_utf8[j++] = 0x80 | (rune & 0x3f);
} else if (rune < 0x10000) {
name_utf8[j++] = 0xe0 | (rune >> 12);
name_utf8[j++] = 0x80 | ((rune >> 6) & 0x3f);
name_utf8[j++] = 0x80 | (rune & 0x3f);
} else {
name_utf8[j++] = 0xf0 | (rune >> 18);
name_utf8[j++] = 0x80 | ((rune >> 12) & 0x3f);
name_utf8[j++] = 0x80 | ((rune >> 6) & 0x3f);
name_utf8[j++] = 0x80 | (rune & 0x3f);
}
}

ftp://ftp.rfc-editor.org/in-notes/rfc2279.txt
ftp://ftp.rfc-editor.org/in-notes/rfc2781.txt

Haven't even tried to compile it ...

--
"Next the statesmen will invent cheap lies, putting the blame upon
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince
himself that the war is just, and will thank God for the better sleep
he enjoys after this process of grotesque self-deception." -- Mark Twain