Some improvements to EFI GOP support

Some improvements to EFI GOP support

[Matthew Garrett]

Add support for grabbing the EDID on GOP devices, along with picking the
better GOP device when we have more than one (Thanks, Apple).

[PATCH 1/4] Add PCI protocols

[Matthew Garrett]

There's various UEFI protocols for handling PCI devices. Add support for them.
---
 ChangeLog              |    4 +
 include/grub/efi/pci.h |  319 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 323 insertions(+), 0 deletions(-)
 create mode 100644 include/grub/efi/pci.h

diff --git a/ChangeLog b/ChangeLog
index ede7f8e..ca07786 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2012-02-08  Matthew Garrett  <mjg@redhat.com>
+
+	* include/grub/efi/pci.h: New file to define EFI PCI protocols.
+
 2012-02-07  Vladimir Serbinenko  <phcoder@gmail.com>
 
 	* grub-core/lib/i386/relocator16.S: Revert moving A20 code into PM
diff --git a/include/grub/efi/pci.h b/include/grub/efi/pci.h
new file mode 100644
index 0000000..b172455
--- /dev/null
+++ b/include/grub/efi/pci.h
@@ -0,0 +1,319 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2006,2007,2008  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef GRUB_EFI_PCI_HEADER
+#define GRUB_EFI_PCI_HEADER	1
+
+#include <grub/symbol.h>
+
+#define GRUB_EFI_PCI_IO_GUID \
+  { 0x4cf5b200, 0x68b8, 0x4ca5, { 0x9e, 0xec, 0xb2, 0x3e, 0x3f, 0x50, 0x02, 0x9a }}
+
+#define GRUB_EFI_PCI_ROOT_IO_GUID \
+  { 0x2F707EBB, 0x4A1A, 0x11d4, { 0x9A, 0x38, 0x00, 0x90, 0x27, 0x3F, 0xC1, 0x4D }}
+
+typedef enum
+  {
+    GRUB_EFI_PCI_IO_WIDTH_UINT8,
+    GRUB_EFI_PCI_IO_WIDTH_UINT16,
+    GRUB_EFI_PCI_IO_WIDTH_UINT32,
+    GRUB_EFI_PCI_IO_WIDTH_UINT64,
+    GRUB_EFI_PCI_IO_WIDTH_FIFO_UINT8,
+    GRUB_EFI_PCI_IO_WIDTH_FIFO_UINT16,
+    GRUB_EFI_PCI_IO_WIDTH_FIFO_UINT32,
+    GRUB_EFI_PCI_IO_WIDTH_FIFO_UINT64,
+    GRUB_EFI_PCI_IO_WIDTH_FILL_UINT8,
+    GRUB_EFI_PCI_IO_WIDTH_FILL_UINT16,
+    GRUB_EFI_PCI_IO_WIDTH_FILL_UINT32,
+    GRUB_EFI_PCI_IO_WIDTH_FILL_UINT64,
+    GRUB_EFI_PCI_IO_WIDTH_MAXIMUM,
+  }
+  grub_efi_pci_io_width_t;
+
+struct grub_efi_pci_io;
+
+typedef grub_efi_status_t
+(*grub_efi_pci_io_mem_t) (struct grub_efi_pci_io *this,
+			  grub_efi_pci_io_width_t width,
+			  grub_efi_uint8_t bar_index,
+			  grub_efi_uint64_t offset,
+			  grub_efi_uintn_t count,
+			  void *buffer);
+
+typedef grub_efi_status_t
+(*grub_efi_pci_io_config_t) (struct grub_efi_pci_io *this,
+			     grub_efi_pci_io_width_t width,
+			     grub_efi_uint32_t offset,
+			     grub_efi_uintn_t count,
+			     void *buffer);
+typedef struct
+{
+  grub_efi_pci_io_mem_t read;
+  grub_efi_pci_io_mem_t write;
+} grub_efi_pci_io_access_t;
+
+typedef struct
+{
+  grub_efi_pci_io_config_t read;
+  grub_efi_pci_io_config_t write;
+} grub_efi_pci_io_config_access_t;
+
+typedef enum
+  {
+    GRUB_EFI_PCI_IO_OPERATION_BUS_MASTER_READ,
+    GRUB_EFI_PCI_IO_OPERATION_BUS_MASTER_WRITE,
+    GRUB_EFI_PCI_IO_OPERATION_BUS_MASTER_COMMON_BUFFER,
+    GRUB_EFI_PCI_IO_OPERATION_BUS_MASTER_MAXIMUM
+  }
+  grub_efi_pci_io_operation_t;
+
+#define GRUB_EFI_PCI_IO_ATTRIBUTE_ISA_IO               0x0002
+#define GRUB_EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO       0x0004
+#define GRUB_EFI_PCI_IO_ATTRIBUTE_VGA_MEMORY           0x0008
+#define GRUB_EFI_PCI_IO_ATTRIBUTE_VGA_IO               0x0010
+#define GRUB_EFI_PCI_IO_ATTRIBUTE_IDE_PRIMARY_IO       0x0020
+#define GRUB_EFI_PCI_IO_ATTRIBUTE_IDE_SECONDARY_IO     0x0040
+#define GRUB_EFI_PCI_IO_ATTRIBUTE_MEMORY_WRITE_COMBINE 0x0080
+#define GRUB_EFI_PCI_IO_ATTRIBUTE_IO                   0x0100
+#define GRUB_EFI_PCI_IO_ATTRIBUTE_MEMORY               0x0200
+#define GRUB_EFI_PCI_IO_ATTRIBUTE_BUS_MASTER           0x0400
+#define GRUB_EFI_PCI_IO_ATTRIBUTE_MEMORY_CACHED        0x0800
+#define GRUB_EFI_PCI_IO_ATTRIBUTE_MEMORY_DISABLE       0x1000
+#define GRUB_EFI_PCI_IO_ATTRIBUTE_EMBEDDED_DEVICE      0x2000
+#define GRUB_EFI_PCI_IO_ATTRIBUTE_EMBEDDED_ROM         0x4000
+#define GRUB_EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE   0x8000
+#define GRUB_EFI_PCI_IO_ATTRIBUTE_ISA_IO_16            0x10000
+#define GRUB_EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO_16    0x20000
+#define GRUB_EFI_PCI_IO_ATTRIBUTE_VGA_IO_16            0x40000
+
+typedef enum
+  {
+    GRUB_EFI_PCI_IO_ATTRIBUTE_OPERATION_GET,
+    GRUB_EFI_PCI_IO_ATTRIBUTE_OPERATION_SET,
+    GRUB_EFI_PCI_IO_ATTRIBUTE_OPERATION_ENABLE,
+    GRUB_EFI_PCI_IO_ATTRIBUTE_OPERATION_DISABLE,
+    GRUB_EFI_PCI_IO_ATTRIBUTE_OPERATION_SUPPORTED,
+    GRUB_EFI_PCI_IO_ATTRIBUTE_OPERATION_MAXIMUM
+  }
+  grub_efi_pci_io_attribute_operation_t;
+
+typedef grub_efi_status_t
+(*grub_efi_pci_io_poll_io_mem_t) (struct grub_efi_pci_io *this,
+				  grub_efi_pci_io_width_t  width,
+				  grub_efi_uint8_t bar_ndex,
+				  grub_efi_uint64_t offset,
+				  grub_efi_uint64_t mask,
+				  grub_efi_uint64_t value,
+				  grub_efi_uint64_t delay,
+				  grub_efi_uint64_t *result);
+
+typedef grub_efi_status_t
+(*grub_efi_pci_io_copy_mem_t) (struct grub_efi_pci_io *this,
+			       grub_efi_pci_io_width_t width,
+			       grub_efi_uint8_t dest_bar_index,
+			       grub_efi_uint64_t dest_offset,
+			       grub_efi_uint8_t src_bar_index,
+			       grub_efi_uint64_t src_offset,
+			       grub_efi_uintn_t count);
+
+typedef grub_efi_status_t
+(*grub_efi_pci_io_map_t) (struct grub_efi_pci_io *this,
+			  grub_efi_pci_io_operation_t operation,
+			  void *host_address,
+			  grub_efi_uintn_t *number_of_bytes,
+			  grub_efi_uint64_t *device_address,
+			  void **mapping);
+
+typedef grub_efi_status_t
+(*grub_efi_pci_io_unmap_t) (struct grub_efi_pci_io *this,
+			    void *mapping);
+
+typedef grub_efi_status_t
+(*grub_efi_pci_io_allocate_buffer_t) (struct grub_efi_pci_io *this,
+				      grub_efi_allocate_type_t type,
+				      grub_efi_memory_type_t memory_type,
+				      grub_efi_uintn_t pages,
+				      void **host_address,
+				      grub_efi_uint64_t attributes);
+
+typedef grub_efi_status_t
+(*grub_efi_pci_io_free_buffer_t) (struct grub_efi_pci_io *this,
+				  grub_efi_allocate_type_t type,
+				  grub_efi_memory_type_t memory_type,
+				  grub_efi_uintn_t pages,
+				  void **host_address,
+				  grub_efi_uint64_t attributes);
+
+typedef grub_efi_status_t
+(*grub_efi_pci_io_flush_t) (struct grub_efi_pci_io *this);
+
+typedef grub_efi_status_t
+(*grub_efi_pci_io_get_location_t) (struct grub_efi_pci_io *this,
+				   grub_efi_uintn_t *segment_number,
+				   grub_efi_uintn_t *bus_number,
+				   grub_efi_uintn_t *device_number,
+				   grub_efi_uintn_t *function_number);
+
+typedef grub_efi_status_t
+(*grub_efi_pci_io_attributes_t) (struct grub_efi_pci_io *this,
+				 grub_efi_pci_io_attribute_operation_t operation,
+				 grub_efi_uint64_t attributes,
+				 grub_efi_uint64_t *result);
+
+typedef grub_efi_status_t
+(*grub_efi_pci_io_get_bar_attributes_t) (struct grub_efi_pci_io *this,
+					 grub_efi_uint8_t bar_index,
+					 grub_efi_uint64_t *supports,
+					 void **resources);
+
+typedef grub_efi_status_t
+(*grub_efi_pci_io_set_bar_attributes_t) (struct grub_efi_pci_io *this,
+					 grub_efi_uint64_t attributes,
+					 grub_efi_uint8_t bar_index,
+					 grub_efi_uint64_t *offset,
+					 grub_efi_uint64_t *length);
+struct grub_efi_pci_io {
+  grub_efi_pci_io_poll_io_mem_t poll_mem;
+  grub_efi_pci_io_poll_io_mem_t poll_io;
+  grub_efi_pci_io_access_t mem;
+  grub_efi_pci_io_access_t io;
+  grub_efi_pci_io_config_access_t pci;
+  grub_efi_pci_io_copy_mem_t copy_mem;
+  grub_efi_pci_io_map_t map;
+  grub_efi_pci_io_unmap_t unmap;
+  grub_efi_pci_io_allocate_buffer_t allocate_buffer;
+  grub_efi_pci_io_free_buffer_t free_buffer;
+  grub_efi_pci_io_flush_t flush;
+  grub_efi_pci_io_get_location_t get_location;
+  grub_efi_pci_io_attributes_t attributes;
+  grub_efi_pci_io_get_bar_attributes_t get_bar_attributes;
+  grub_efi_pci_io_set_bar_attributes_t set_bar_attributes;
+  grub_efi_uint64_t rom_size;
+  void *rom_image;
+};
+typedef struct grub_efi_pci_io grub_efi_pci_io_t;
+
+struct grub_efi_pci_root_io;
+
+typedef struct
+{
+  grub_efi_status_t(*read) (struct grub_efi_pci_root_io *this,
+			    grub_efi_pci_io_width_t width,
+			    grub_efi_uint64_t address,
+			    grub_efi_uintn_t count,
+			    void *buffer);
+  grub_efi_status_t(*write) (struct grub_efi_pci_root_io *this,
+			     grub_efi_pci_io_width_t width,
+			     grub_efi_uint64_t address,
+			     grub_efi_uintn_t count,
+			     void *buffer);
+} grub_efi_pci_root_io_access_t;
+
+typedef enum
+  {
+    GRUB_EFI_PCI_ROOT_IO_OPERATION_BUS_MASTER_READ,
+    GRUB_EFI_PCI_ROOT_IO_OPERATION_BUS_MASTER_WRITE,
+    GRUB_EFI_PCI_ROOT_IO_OPERATION_BUS_MASTER_COMMON_BUFFER,
+    GRUB_EFI_PCI_ROOT_IO_OPERATION_BUS_MASTER_READ_64,
+    GRUB_EFI_PCI_ROOT_IO_OPERATION_BUS_MASTER_WRITE_64,
+    GRUB_EFI_PCI_ROOT_IO_OPERATION_BUS_MASTER_COMMON_BUFFER_64,
+    GRUB_EFI_PCI_ROOT_IO_OPERATION_BUS_MASTER_MAXIMUM
+  }
+  grub_efi_pci_root_io_operation_t;
+
+typedef grub_efi_status_t
+(*grub_efi_pci_root_io_poll_io_mem_t) (struct grub_efi_pci_root_io *this,
+				       grub_efi_pci_io_width_t  width,
+				       grub_efi_uint64_t address,
+				       grub_efi_uint64_t mask,
+				       grub_efi_uint64_t value,
+				       grub_efi_uint64_t delay,
+				       grub_efi_uint64_t *result);
+
+typedef grub_efi_status_t
+(*grub_efi_pci_root_io_copy_mem_t) (struct grub_efi_pci_root_io *this,
+				    grub_efi_pci_io_width_t width,
+				    grub_efi_uint64_t dest_offset,
+				    grub_efi_uint64_t src_offset,
+				    grub_efi_uintn_t count);
+
+
+typedef grub_efi_status_t
+(*grub_efi_pci_root_io_map_t) (struct grub_efi_pci_root_io *this,
+				grub_efi_pci_root_io_operation_t operation,
+			       void *host_address,
+			       grub_efi_uintn_t *number_of_bytes,
+			       grub_efi_uint64_t *device_address,
+			       void **mapping);
+
+typedef grub_efi_status_t
+(*grub_efi_pci_root_io_unmap_t) (struct grub_efi_pci_root_io *this,
+				 void *mapping);
+
+typedef grub_efi_status_t
+(*grub_efi_pci_root_io_allocate_buffer_t) (struct grub_efi_pci_root_io *this,
+					   grub_efi_allocate_type_t type,
+					   grub_efi_memory_type_t memory_type,
+					   grub_efi_uintn_t pages,
+					   void **host_address,
+					   grub_efi_uint64_t attributes);
+
+typedef grub_efi_status_t
+(*grub_efi_pci_root_io_free_buffer_t) (struct grub_efi_pci_root_io *this,
+				       grub_efi_uintn_t pages,
+				       void **host_address);
+
+typedef grub_efi_status_t
+(*grub_efi_pci_root_io_flush_t) (struct grub_efi_pci_root_io *this);
+
+typedef grub_efi_status_t
+(*grub_efi_pci_root_io_get_attributes_t) (struct grub_efi_pci_root_io *this,
+					  grub_efi_uint64_t *supports,
+					  void **resources);
+
+typedef grub_efi_status_t
+(*grub_efi_pci_root_io_set_attributes_t) (struct grub_efi_pci_root_io *this,
+					  grub_efi_uint64_t attributes,
+					  grub_efi_uint64_t *offset,
+					  grub_efi_uint64_t *length);
+
+typedef grub_efi_status_t
+(*grub_efi_pci_root_io_configuration_t) (struct grub_efi_pci_root_io *this,
+					 void **resources);
+
+struct grub_efi_pci_root_io {
+  grub_efi_handle_t parent;
+  grub_efi_pci_root_io_poll_io_mem_t poll_mem;
+  grub_efi_pci_root_io_poll_io_mem_t poll_io;
+  grub_efi_pci_root_io_access_t mem;
+  grub_efi_pci_root_io_access_t io;
+  grub_efi_pci_root_io_access_t pci;
+  grub_efi_pci_root_io_copy_mem_t copy_mem;
+  grub_efi_pci_root_io_map_t map;
+  grub_efi_pci_root_io_unmap_t unmap;
+  grub_efi_pci_root_io_allocate_buffer_t allocate_buffer;
+  grub_efi_pci_root_io_free_buffer_t free_buffer;
+  grub_efi_pci_root_io_flush_t flush;
+  grub_efi_pci_root_io_get_attributes_t get_attributes;
+  grub_efi_pci_root_io_set_attributes_t set_attributes;
+  grub_efi_pci_root_io_configuration_t configuration;
+};
+
+typedef struct grub_efi_pci_root_io grub_efi_pci_root_io_t;
+
+#endif  /* !GRUB_EFI_PCI_HEADER */
-- 
1.7.7.6

[PATCH 2/4] Add grub_efi_get_variable

[Matthew Garrett]

Code may wish to obtain system information from EFI variables. Add support
for making the platform call.
---
 ChangeLog                |    6 ++++++
 grub-core/kern/efi/efi.c |   32 ++++++++++++++++++++++++++++++++
 include/grub/efi/api.h   |    4 ++++
 include/grub/efi/efi.h   |    3 ++-
 4 files changed, 44 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index ca07786..e662f3d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2012-02-08  Matthew Garrett  <mjg@redhat.com>
 
+	* grub-core/kern/efi/efi.c (grub_efi_get_variable): Add new function.
+	* include/grub/efi/efi.h: Likewise.
+	* include/grub/efi/api.h: Add guid for EFI-specified variables.
+
+2012-02-08  Matthew Garrett  <mjg@redhat.com>
+
 	* include/grub/efi/pci.h: New file to define EFI PCI protocols.
 
 2012-02-07  Vladimir Serbinenko  <phcoder@gmail.com>
diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index 9a2c5e6..d366f8a 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -182,6 +182,38 @@ grub_efi_set_virtual_address_map (grub_efi_uintn_t memory_map_size,
   return grub_error (GRUB_ERR_IO, "set_virtual_address_map failed");
 }
 
+void *
+grub_efi_get_variable(grub_uint8_t *var, grub_efi_guid_t *guid)
+{
+  grub_efi_status_t status;
+  grub_efi_uintn_t datasize = 0;
+  grub_efi_runtime_services_t *r;
+  grub_efi_char16_t *var16;
+  int i;
+  void *data;
+
+  var16 = grub_malloc((grub_strlen((char *)var) +1) * 2);
+
+  for (i=0; i<(int)grub_strlen((char *)var); i++)
+    var16[i] = var[i];
+  var16[i] = '\0';
+
+  r = grub_efi_system_table->runtime_services;
+
+  status = efi_call_5 (r->get_variable, var16, guid, NULL, &datasize, NULL);
+
+  data = grub_malloc(datasize);
+
+  status = efi_call_5 (r->get_variable, var16, guid, NULL, &datasize, data);
+
+  grub_free(var16);
+
+  if (status == GRUB_EFI_SUCCESS)
+      return data;
+
+  return NULL;
+}
+
 grub_uint64_t
 grub_rtc_get_time_ms (void)
 {
diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
index b3ec663..c954d03 100644
--- a/include/grub/efi/api.h
+++ b/include/grub/efi/api.h
@@ -1025,6 +1025,10 @@ struct grub_efi_runtime_services
   grub_efi_status_t
   (*convert_pointer) (grub_efi_uintn_t debug_disposition, void **address);
 
+#define GRUB_EFI_GLOBAL_VARIABLE_GUID \
+  { 0x8BE4DF61, 0x93CA, 0x11d2, { 0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B,0x8C }}
+
+
   grub_efi_status_t
   (*get_variable) (grub_efi_char16_t *variable_name,
 		   grub_efi_guid_t *vendor_guid,
diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
index 7ecda58..3182972 100644
--- a/include/grub/efi/efi.h
+++ b/include/grub/efi/efi.h
@@ -61,7 +61,8 @@ grub_err_t EXPORT_FUNC (grub_efi_set_virtual_address_map) (grub_efi_uintn_t memo
 							   grub_efi_uintn_t descriptor_size,
 							   grub_efi_uint32_t descriptor_version,
 							   grub_efi_memory_descriptor_t *virtual_map);
-
+void* EXPORT_FUNC (grub_efi_get_variable) (grub_uint8_t *variable,
+						grub_efi_guid_t *guid);
 int
 EXPORT_FUNC (grub_efi_compare_device_paths) (const grub_efi_device_path_t *dp1,
 					     const grub_efi_device_path_t *dp2);
-- 
1.7.7.6

[PATCH 3/4] Prefer GOP devices which implement the pci_io protocol

[Matthew Garrett]

Some systems (especially Apple) implement multiple GOP devices representing
the same hardware. The preferred device is the one that also implements the
PCI io protocol.
---
 ChangeLog                 |    5 +++++
 grub-core/video/efi_gop.c |   38 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 42 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index e662f3d..26d779b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2012-02-08  Matthew Garrett  <mjg@redhat.com>
 
+	* grub-core/video/efi_gop.c (check_protocol): Prefer GOP devices which
+	implement the pci_io protocol.
+
+2012-02-08  Matthew Garrett  <mjg@redhat.com>
+
 	* grub-core/kern/efi/efi.c (grub_efi_get_variable): Add new function.
 	* include/grub/efi/efi.h: Likewise.
 	* include/grub/efi/api.h: Add guid for EFI-specified variables.
diff --git a/grub-core/video/efi_gop.c b/grub-core/video/efi_gop.c
index d14ae98..47e3ee9 100644
--- a/grub-core/video/efi_gop.c
+++ b/grub-core/video/efi_gop.c
@@ -28,11 +28,14 @@
 #include <grub/efi/api.h>
 #include <grub/efi/efi.h>
 #include <grub/efi/graphics_output.h>
+#include <grub/efi/pci.h>
 
 GRUB_MOD_LICENSE ("GPLv3+");
 
 static grub_efi_guid_t graphics_output_guid = GRUB_EFI_GOP_GUID;
+static grub_efi_guid_t pci_io_guid = GRUB_EFI_PCI_IO_GUID;
 static struct grub_efi_gop *gop;
+static grub_efi_handle_t gop_handle;
 static unsigned old_mode;
 static int restore_needed;
 
@@ -47,7 +50,40 @@ static struct
 static int
 check_protocol (void)
 {
-  gop = grub_efi_locate_protocol (&graphics_output_guid, 0);
+  grub_efi_handle_t *handle, *handles;
+  grub_efi_uintn_t num_handles;
+
+  handles = grub_efi_locate_handle (GRUB_EFI_BY_PROTOCOL,
+				    &pci_io_guid, NULL, &num_handles);
+  if (!num_handles || !handles)
+    return 0;
+
+  for (handle = handles; num_handles--; handle++)
+    {
+      gop = grub_efi_open_protocol (*handle, &graphics_output_guid,
+				    GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+      if (gop)
+	{
+	  gop_handle = *handle;
+	  break;
+	}
+    }
+
+  grub_free(handles);
+
+  if (!gop)
+    {
+      handles = grub_efi_locate_handle (GRUB_EFI_BY_PROTOCOL,
+					&graphics_output_guid, NULL, &num_handles);
+      gop = grub_efi_open_protocol (*handles, &graphics_output_guid,
+				    GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+      if (gop)
+	{
+	  gop_handle = *handles;
+	}
+      grub_free(handles);
+    }
+
   if (gop)
     return 1;
 
-- 
1.7.7.6

[PATCH 4/4] Add support for getting EDID via EFI

[Matthew Garrett]

EFI gives a couple of defined methods for retrieving the EDID, so make use
of them. Some Apple devices don't provide these but do stash the EDID in an
nvram variable - grab it from there if it exists.
---
 ChangeLog                 |    8 +++++
 grub-core/video/efi_gop.c |   69 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+), 0 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 26d779b..d46b3d1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2012-02-08  Matthew Garrett  <mjg@redhat.com>
 
+	* grub-core/video/efi_gop.c (grub_video_gop_get_edid): Add support for
+	retrieving the EDID via EFI. Implements the active and discovered
+	protocols, falling back to a variable for Apple devices.
+	(grub_video_gop_get_preferred_mode): Retrieve the EDID preferred mode
+	when possible.
+
+2012-02-08  Matthew Garrett  <mjg@redhat.com>
+
 	* grub-core/video/efi_gop.c (check_protocol): Prefer GOP devices which
 	implement the pci_io protocol.
 
diff --git a/grub-core/video/efi_gop.c b/grub-core/video/efi_gop.c
index 47e3ee9..f4c563d 100644
--- a/grub-core/video/efi_gop.c
+++ b/grub-core/video/efi_gop.c
@@ -28,12 +28,16 @@
 #include <grub/efi/api.h>
 #include <grub/efi/efi.h>
 #include <grub/efi/graphics_output.h>
+#include <grub/efi/edid.h>
 #include <grub/efi/pci.h>
 
 GRUB_MOD_LICENSE ("GPLv3+");
 
 static grub_efi_guid_t graphics_output_guid = GRUB_EFI_GOP_GUID;
+static grub_efi_guid_t active_edid_guid = GRUB_EFI_EDID_ACTIVE_GUID;
+static grub_efi_guid_t discovered_edid_guid = GRUB_EFI_EDID_DISCOVERED_GUID;
 static grub_efi_guid_t pci_io_guid = GRUB_EFI_PCI_IO_GUID;
+static grub_efi_guid_t efi_var_guid = GRUB_EFI_GLOBAL_VARIABLE_GUID;
 static struct grub_efi_gop *gop;
 static grub_efi_handle_t gop_handle;
 static unsigned old_mode;
@@ -257,6 +261,53 @@ grub_video_gop_iterate (int (*hook) (const struct grub_video_mode_info *info))
 }
 
 static grub_err_t
+grub_video_gop_get_edid (struct grub_video_edid_info *edid_info)
+{
+  struct grub_efi_active_edid *edid;
+  grub_uint8_t edidname[] = "agp-internal-edid";
+  grub_uint8_t *data;
+
+  edid = grub_efi_open_protocol(gop_handle, &active_edid_guid,
+				GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+  if (!edid || edid->size_of_edid == 0) {
+    edid = grub_efi_open_protocol(gop_handle, &discovered_edid_guid,
+				  GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+  }
+
+  if (!edid || edid->size_of_edid == 0) {
+    data = grub_efi_get_variable(edidname, &efi_var_guid);
+    if (data)
+      {
+	grub_memcpy(edid_info, data + 16, sizeof(*edid_info));
+	grub_free(data);
+	return GRUB_ERR_NONE;
+      }
+    return grub_error (GRUB_ERR_BAD_DEVICE, "EDID information not available");
+  }
+
+  grub_memcpy (&edid_info, edid->edid, sizeof(edid_info));
+
+  return GRUB_ERR_NONE;
+}
+
+static grub_err_t
+grub_gop_get_preferred_mode (unsigned int *width, unsigned int *height)
+{
+  struct grub_video_edid_info edid_info;
+
+  if (grub_video_gop_get_edid(&edid_info) == GRUB_ERR_NONE)
+    {
+      if (grub_video_edid_checksum (&edid_info) == GRUB_ERR_NONE
+	  && grub_video_edid_preferred_mode (&edid_info, width, height)
+	  == GRUB_ERR_NONE)
+	return GRUB_ERR_NONE;
+      else
+	grub_dprintf("video", "invalid edid");
+    }
+  return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "cannot get preferred mode");
+}
+
+static grub_err_t
 grub_video_gop_setup (unsigned int width, unsigned int height,
 		      unsigned int mode_type,
 		      unsigned int mode_mask __attribute__ ((unused)))
@@ -268,10 +319,18 @@ grub_video_gop_setup (unsigned int width, unsigned int height,
   unsigned bpp;
   int found = 0;
   unsigned long long best_volume = 0;
+  int preferred_mode = 0;
 
   depth = (mode_type & GRUB_VIDEO_MODE_TYPE_DEPTH_MASK)
     >> GRUB_VIDEO_MODE_TYPE_DEPTH_POS;
 
+  if (width == 0 && height == 0)
+    {
+      grub_gop_get_preferred_mode (&width, &height);
+      if (grub_errno == GRUB_ERR_NONE)
+	  preferred_mode = 1;
+    }
+
   /* Keep current mode if possible.  */
   if (gop->mode->info)
     {
@@ -306,6 +365,15 @@ grub_video_gop_setup (unsigned int width, unsigned int height,
 	  grub_dprintf ("video", "GOP: mode %d: %dx%d\n", mode, info->width,
 			info->height);
 
+	  if (preferred_mode)
+	    {
+	      if (info->width > width || info->height > height)
+		{
+		  grub_dprintf ("video", "GOP: mode %d: too large\n", mode);
+		  continue;
+		}
+	    }
+
 	  bpp = grub_video_gop_get_bpp (info);
 	  if (!bpp)
 	    {
@@ -437,6 +505,7 @@ static struct grub_video_adapter grub_video_gop_adapter =
     .setup = grub_video_gop_setup,
     .get_info = grub_video_fb_get_info,
     .get_info_and_fini = grub_video_gop_get_info_and_fini,
+    .get_edid = grub_video_gop_get_edid,
     .set_palette = grub_video_fb_set_palette,
     .get_palette = grub_video_fb_get_palette,
     .set_viewport = grub_video_fb_set_viewport,
-- 
1.7.7.6

Re: Some improvements to EFI GOP support

[Keshav P R]

On Wed, Feb 8, 2012 at 22:21, Matthew Garrett <mjg@redhat.com> wrote:
> Add support for grabbing the EDID on GOP devices, along with picking the
> better GOP device when we have more than one (Thanks, Apple).
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Patch?

- Keshav

Re: [PATCH 1/4] Add PCI protocols

[Vladimir 'φ-coder/phcoder' Serbinenko]

Go ahead.
As a note: it may be useful for ia64 in future.
On 08.02.2012 17:51, Matthew Garrett wrote:
> There's various UEFI protocols for handling PCI devices. Add support for them.
> ---
>   ChangeLog              |    4 +
>   include/grub/efi/pci.h |  319 ++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 323 insertions(+), 0 deletions(-)
>   create mode 100644 include/grub/efi/pci.h
>
> diff --git a/ChangeLog b/ChangeLog
> index ede7f8e..ca07786 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,7 @@
> +2012-02-08  Matthew Garrett<mjg@redhat.com>
> +
> +	* include/grub/efi/pci.h: New file to define EFI PCI protocols.
> +
>   2012-02-07  Vladimir Serbinenko<phcoder@gmail.com>
>
>   	* grub-core/lib/i386/relocator16.S: Revert moving A20 code into PM
> diff --git a/include/grub/efi/pci.h b/include/grub/efi/pci.h
> new file mode 100644
> index 0000000..b172455
> --- /dev/null
> +++ b/include/grub/efi/pci.h
> @@ -0,0 +1,319 @@
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2006,2007,2008  Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see<http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef GRUB_EFI_PCI_HEADER
> +#define GRUB_EFI_PCI_HEADER	1
> +
> +#include<grub/symbol.h>
> +
> +#define GRUB_EFI_PCI_IO_GUID \
> +  { 0x4cf5b200, 0x68b8, 0x4ca5, { 0x9e, 0xec, 0xb2, 0x3e, 0x3f, 0x50, 0x02, 0x9a }}
> +
> +#define GRUB_EFI_PCI_ROOT_IO_GUID \
> +  { 0x2F707EBB, 0x4A1A, 0x11d4, { 0x9A, 0x38, 0x00, 0x90, 0x27, 0x3F, 0xC1, 0x4D }}
> +
> +typedef enum
> +  {
> +    GRUB_EFI_PCI_IO_WIDTH_UINT8,
> +    GRUB_EFI_PCI_IO_WIDTH_UINT16,
> +    GRUB_EFI_PCI_IO_WIDTH_UINT32,
> +    GRUB_EFI_PCI_IO_WIDTH_UINT64,
> +    GRUB_EFI_PCI_IO_WIDTH_FIFO_UINT8,
> +    GRUB_EFI_PCI_IO_WIDTH_FIFO_UINT16,
> +    GRUB_EFI_PCI_IO_WIDTH_FIFO_UINT32,
> +    GRUB_EFI_PCI_IO_WIDTH_FIFO_UINT64,
> +    GRUB_EFI_PCI_IO_WIDTH_FILL_UINT8,
> +    GRUB_EFI_PCI_IO_WIDTH_FILL_UINT16,
> +    GRUB_EFI_PCI_IO_WIDTH_FILL_UINT32,
> +    GRUB_EFI_PCI_IO_WIDTH_FILL_UINT64,
> +    GRUB_EFI_PCI_IO_WIDTH_MAXIMUM,
> +  }
> +  grub_efi_pci_io_width_t;
> +
> +struct grub_efi_pci_io;
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_io_mem_t) (struct grub_efi_pci_io *this,
> +			  grub_efi_pci_io_width_t width,
> +			  grub_efi_uint8_t bar_index,
> +			  grub_efi_uint64_t offset,
> +			  grub_efi_uintn_t count,
> +			  void *buffer);
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_io_config_t) (struct grub_efi_pci_io *this,
> +			     grub_efi_pci_io_width_t width,
> +			     grub_efi_uint32_t offset,
> +			     grub_efi_uintn_t count,
> +			     void *buffer);
> +typedef struct
> +{
> +  grub_efi_pci_io_mem_t read;
> +  grub_efi_pci_io_mem_t write;
> +} grub_efi_pci_io_access_t;
> +
> +typedef struct
> +{
> +  grub_efi_pci_io_config_t read;
> +  grub_efi_pci_io_config_t write;
> +} grub_efi_pci_io_config_access_t;
> +
> +typedef enum
> +  {
> +    GRUB_EFI_PCI_IO_OPERATION_BUS_MASTER_READ,
> +    GRUB_EFI_PCI_IO_OPERATION_BUS_MASTER_WRITE,
> +    GRUB_EFI_PCI_IO_OPERATION_BUS_MASTER_COMMON_BUFFER,
> +    GRUB_EFI_PCI_IO_OPERATION_BUS_MASTER_MAXIMUM
> +  }
> +  grub_efi_pci_io_operation_t;
> +
> +#define GRUB_EFI_PCI_IO_ATTRIBUTE_ISA_IO               0x0002
> +#define GRUB_EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO       0x0004
> +#define GRUB_EFI_PCI_IO_ATTRIBUTE_VGA_MEMORY           0x0008
> +#define GRUB_EFI_PCI_IO_ATTRIBUTE_VGA_IO               0x0010
> +#define GRUB_EFI_PCI_IO_ATTRIBUTE_IDE_PRIMARY_IO       0x0020
> +#define GRUB_EFI_PCI_IO_ATTRIBUTE_IDE_SECONDARY_IO     0x0040
> +#define GRUB_EFI_PCI_IO_ATTRIBUTE_MEMORY_WRITE_COMBINE 0x0080
> +#define GRUB_EFI_PCI_IO_ATTRIBUTE_IO                   0x0100
> +#define GRUB_EFI_PCI_IO_ATTRIBUTE_MEMORY               0x0200
> +#define GRUB_EFI_PCI_IO_ATTRIBUTE_BUS_MASTER           0x0400
> +#define GRUB_EFI_PCI_IO_ATTRIBUTE_MEMORY_CACHED        0x0800
> +#define GRUB_EFI_PCI_IO_ATTRIBUTE_MEMORY_DISABLE       0x1000
> +#define GRUB_EFI_PCI_IO_ATTRIBUTE_EMBEDDED_DEVICE      0x2000
> +#define GRUB_EFI_PCI_IO_ATTRIBUTE_EMBEDDED_ROM         0x4000
> +#define GRUB_EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE   0x8000
> +#define GRUB_EFI_PCI_IO_ATTRIBUTE_ISA_IO_16            0x10000
> +#define GRUB_EFI_PCI_IO_ATTRIBUTE_VGA_PALETTE_IO_16    0x20000
> +#define GRUB_EFI_PCI_IO_ATTRIBUTE_VGA_IO_16            0x40000
> +
> +typedef enum
> +  {
> +    GRUB_EFI_PCI_IO_ATTRIBUTE_OPERATION_GET,
> +    GRUB_EFI_PCI_IO_ATTRIBUTE_OPERATION_SET,
> +    GRUB_EFI_PCI_IO_ATTRIBUTE_OPERATION_ENABLE,
> +    GRUB_EFI_PCI_IO_ATTRIBUTE_OPERATION_DISABLE,
> +    GRUB_EFI_PCI_IO_ATTRIBUTE_OPERATION_SUPPORTED,
> +    GRUB_EFI_PCI_IO_ATTRIBUTE_OPERATION_MAXIMUM
> +  }
> +  grub_efi_pci_io_attribute_operation_t;
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_io_poll_io_mem_t) (struct grub_efi_pci_io *this,
> +				  grub_efi_pci_io_width_t  width,
> +				  grub_efi_uint8_t bar_ndex,
> +				  grub_efi_uint64_t offset,
> +				  grub_efi_uint64_t mask,
> +				  grub_efi_uint64_t value,
> +				  grub_efi_uint64_t delay,
> +				  grub_efi_uint64_t *result);
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_io_copy_mem_t) (struct grub_efi_pci_io *this,
> +			       grub_efi_pci_io_width_t width,
> +			       grub_efi_uint8_t dest_bar_index,
> +			       grub_efi_uint64_t dest_offset,
> +			       grub_efi_uint8_t src_bar_index,
> +			       grub_efi_uint64_t src_offset,
> +			       grub_efi_uintn_t count);
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_io_map_t) (struct grub_efi_pci_io *this,
> +			  grub_efi_pci_io_operation_t operation,
> +			  void *host_address,
> +			  grub_efi_uintn_t *number_of_bytes,
> +			  grub_efi_uint64_t *device_address,
> +			  void **mapping);
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_io_unmap_t) (struct grub_efi_pci_io *this,
> +			    void *mapping);
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_io_allocate_buffer_t) (struct grub_efi_pci_io *this,
> +				      grub_efi_allocate_type_t type,
> +				      grub_efi_memory_type_t memory_type,
> +				      grub_efi_uintn_t pages,
> +				      void **host_address,
> +				      grub_efi_uint64_t attributes);
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_io_free_buffer_t) (struct grub_efi_pci_io *this,
> +				  grub_efi_allocate_type_t type,
> +				  grub_efi_memory_type_t memory_type,
> +				  grub_efi_uintn_t pages,
> +				  void **host_address,
> +				  grub_efi_uint64_t attributes);
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_io_flush_t) (struct grub_efi_pci_io *this);
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_io_get_location_t) (struct grub_efi_pci_io *this,
> +				   grub_efi_uintn_t *segment_number,
> +				   grub_efi_uintn_t *bus_number,
> +				   grub_efi_uintn_t *device_number,
> +				   grub_efi_uintn_t *function_number);
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_io_attributes_t) (struct grub_efi_pci_io *this,
> +				 grub_efi_pci_io_attribute_operation_t operation,
> +				 grub_efi_uint64_t attributes,
> +				 grub_efi_uint64_t *result);
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_io_get_bar_attributes_t) (struct grub_efi_pci_io *this,
> +					 grub_efi_uint8_t bar_index,
> +					 grub_efi_uint64_t *supports,
> +					 void **resources);
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_io_set_bar_attributes_t) (struct grub_efi_pci_io *this,
> +					 grub_efi_uint64_t attributes,
> +					 grub_efi_uint8_t bar_index,
> +					 grub_efi_uint64_t *offset,
> +					 grub_efi_uint64_t *length);
> +struct grub_efi_pci_io {
> +  grub_efi_pci_io_poll_io_mem_t poll_mem;
> +  grub_efi_pci_io_poll_io_mem_t poll_io;
> +  grub_efi_pci_io_access_t mem;
> +  grub_efi_pci_io_access_t io;
> +  grub_efi_pci_io_config_access_t pci;
> +  grub_efi_pci_io_copy_mem_t copy_mem;
> +  grub_efi_pci_io_map_t map;
> +  grub_efi_pci_io_unmap_t unmap;
> +  grub_efi_pci_io_allocate_buffer_t allocate_buffer;
> +  grub_efi_pci_io_free_buffer_t free_buffer;
> +  grub_efi_pci_io_flush_t flush;
> +  grub_efi_pci_io_get_location_t get_location;
> +  grub_efi_pci_io_attributes_t attributes;
> +  grub_efi_pci_io_get_bar_attributes_t get_bar_attributes;
> +  grub_efi_pci_io_set_bar_attributes_t set_bar_attributes;
> +  grub_efi_uint64_t rom_size;
> +  void *rom_image;
> +};
> +typedef struct grub_efi_pci_io grub_efi_pci_io_t;
> +
> +struct grub_efi_pci_root_io;
> +
> +typedef struct
> +{
> +  grub_efi_status_t(*read) (struct grub_efi_pci_root_io *this,
> +			    grub_efi_pci_io_width_t width,
> +			    grub_efi_uint64_t address,
> +			    grub_efi_uintn_t count,
> +			    void *buffer);
> +  grub_efi_status_t(*write) (struct grub_efi_pci_root_io *this,
> +			     grub_efi_pci_io_width_t width,
> +			     grub_efi_uint64_t address,
> +			     grub_efi_uintn_t count,
> +			     void *buffer);
> +} grub_efi_pci_root_io_access_t;
> +
> +typedef enum
> +  {
> +    GRUB_EFI_PCI_ROOT_IO_OPERATION_BUS_MASTER_READ,
> +    GRUB_EFI_PCI_ROOT_IO_OPERATION_BUS_MASTER_WRITE,
> +    GRUB_EFI_PCI_ROOT_IO_OPERATION_BUS_MASTER_COMMON_BUFFER,
> +    GRUB_EFI_PCI_ROOT_IO_OPERATION_BUS_MASTER_READ_64,
> +    GRUB_EFI_PCI_ROOT_IO_OPERATION_BUS_MASTER_WRITE_64,
> +    GRUB_EFI_PCI_ROOT_IO_OPERATION_BUS_MASTER_COMMON_BUFFER_64,
> +    GRUB_EFI_PCI_ROOT_IO_OPERATION_BUS_MASTER_MAXIMUM
> +  }
> +  grub_efi_pci_root_io_operation_t;
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_root_io_poll_io_mem_t) (struct grub_efi_pci_root_io *this,
> +				       grub_efi_pci_io_width_t  width,
> +				       grub_efi_uint64_t address,
> +				       grub_efi_uint64_t mask,
> +				       grub_efi_uint64_t value,
> +				       grub_efi_uint64_t delay,
> +				       grub_efi_uint64_t *result);
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_root_io_copy_mem_t) (struct grub_efi_pci_root_io *this,
> +				    grub_efi_pci_io_width_t width,
> +				    grub_efi_uint64_t dest_offset,
> +				    grub_efi_uint64_t src_offset,
> +				    grub_efi_uintn_t count);
> +
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_root_io_map_t) (struct grub_efi_pci_root_io *this,
> +				grub_efi_pci_root_io_operation_t operation,
> +			       void *host_address,
> +			       grub_efi_uintn_t *number_of_bytes,
> +			       grub_efi_uint64_t *device_address,
> +			       void **mapping);
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_root_io_unmap_t) (struct grub_efi_pci_root_io *this,
> +				 void *mapping);
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_root_io_allocate_buffer_t) (struct grub_efi_pci_root_io *this,
> +					   grub_efi_allocate_type_t type,
> +					   grub_efi_memory_type_t memory_type,
> +					   grub_efi_uintn_t pages,
> +					   void **host_address,
> +					   grub_efi_uint64_t attributes);
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_root_io_free_buffer_t) (struct grub_efi_pci_root_io *this,
> +				       grub_efi_uintn_t pages,
> +				       void **host_address);
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_root_io_flush_t) (struct grub_efi_pci_root_io *this);
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_root_io_get_attributes_t) (struct grub_efi_pci_root_io *this,
> +					  grub_efi_uint64_t *supports,
> +					  void **resources);
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_root_io_set_attributes_t) (struct grub_efi_pci_root_io *this,
> +					  grub_efi_uint64_t attributes,
> +					  grub_efi_uint64_t *offset,
> +					  grub_efi_uint64_t *length);
> +
> +typedef grub_efi_status_t
> +(*grub_efi_pci_root_io_configuration_t) (struct grub_efi_pci_root_io *this,
> +					 void **resources);
> +
> +struct grub_efi_pci_root_io {
> +  grub_efi_handle_t parent;
> +  grub_efi_pci_root_io_poll_io_mem_t poll_mem;
> +  grub_efi_pci_root_io_poll_io_mem_t poll_io;
> +  grub_efi_pci_root_io_access_t mem;
> +  grub_efi_pci_root_io_access_t io;
> +  grub_efi_pci_root_io_access_t pci;
> +  grub_efi_pci_root_io_copy_mem_t copy_mem;
> +  grub_efi_pci_root_io_map_t map;
> +  grub_efi_pci_root_io_unmap_t unmap;
> +  grub_efi_pci_root_io_allocate_buffer_t allocate_buffer;
> +  grub_efi_pci_root_io_free_buffer_t free_buffer;
> +  grub_efi_pci_root_io_flush_t flush;
> +  grub_efi_pci_root_io_get_attributes_t get_attributes;
> +  grub_efi_pci_root_io_set_attributes_t set_attributes;
> +  grub_efi_pci_root_io_configuration_t configuration;
> +};
> +
> +typedef struct grub_efi_pci_root_io grub_efi_pci_root_io_t;
> +
> +#endif  /* !GRUB_EFI_PCI_HEADER */
> -- 1.7.7.6 _______________________________________________ Grub-devel 
> mailing list Grub-devel@gnu.org 
> https://lists.gnu.org/mailman/listinfo/grub-devel


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

Re: [PATCH 2/4] Add grub_efi_get_variable

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 08.02.2012 17:51, Matthew Garrett wrote:
> Code may wish to obtain system information from EFI variables. Add support
> for making the platform call.
> ---
>   ChangeLog                |    6 ++++++
>   grub-core/kern/efi/efi.c |   32 ++++++++++++++++++++++++++++++++
>   include/grub/efi/api.h   |    4 ++++
>   include/grub/efi/efi.h   |    3 ++-
>   4 files changed, 44 insertions(+), 1 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index ca07786..e662f3d 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,11 @@
>   2012-02-08  Matthew Garrett<mjg@redhat.com>
>
> +	* grub-core/kern/efi/efi.c (grub_efi_get_variable): Add new function.
> +	* include/grub/efi/efi.h: Likewise.
> +	* include/grub/efi/api.h: Add guid for EFI-specified variables.
> +
> +2012-02-08  Matthew Garrett<mjg@redhat.com>
> +
>   	* include/grub/efi/pci.h: New file to define EFI PCI protocols.
>
>   2012-02-07  Vladimir Serbinenko<phcoder@gmail.com>
> diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
> index 9a2c5e6..d366f8a 100644
> --- a/grub-core/kern/efi/efi.c
> +++ b/grub-core/kern/efi/efi.c
> @@ -182,6 +182,38 @@ grub_efi_set_virtual_address_map (grub_efi_uintn_t memory_map_size,
>     return grub_error (GRUB_ERR_IO, "set_virtual_address_map failed");
>   }
>
> +void *
> +grub_efi_get_variable(grub_uint8_t *var, grub_efi_guid_t *guid)
> +{
> +  grub_efi_status_t status;
> +  grub_efi_uintn_t datasize = 0;
> +  grub_efi_runtime_services_t *r;
> +  grub_efi_char16_t *var16;
> +  int i;
> +  void *data;
> +
> +  var16 = grub_malloc((grub_strlen((char *)var) +1) * 2);
> +
Indentation.
You also don't check that malloc succeeded. Also I recommend using 
intermediary variable for length
> +  for (i=0; i<(int)grub_strlen((char *)var); i++)
> +    var16[i] = var[i];
> +  var16[i] = '\0';
> +
We use grub_utf8_to_utf16. Also don't forget to multiply the malloc 
length by GRUB_MAX_UTF16_PER_UTF8 ((GRUB_MAX_UTF16_PER_UTF8 * length + 
1) * sizeof (var16[0]))
> +  r = grub_efi_system_table->runtime_services;
> +
> +  status = efi_call_5 (r->get_variable, var16, guid, NULL,&datasize, NULL);
> +
> +  data = grub_malloc(datasize);
> +
Again: no check for success.
> +  status = efi_call_5 (r->get_variable, var16, guid, NULL,&datasize, data);
> +
> +  grub_free(var16);
> +
> +  if (status == GRUB_EFI_SUCCESS)
> +      return data;


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

Re: [PATCH 3/4] Prefer GOP devices which implement the pci_io protocol

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 08.02.2012 17:51, Matthew Garrett wrote:
> Some systems (especially Apple) implement multiple GOP devices representing
> the same hardware. The preferred device is the one that also implements the
> PCI io protocol.
>
Why not check which modes are supported by GOP rather than something we 
don't use otherwise?

-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

Re: [PATCH 2/4] Add grub_efi_get_variable

[Matthew Garrett]

On Wed, Feb 08, 2012 at 08:55:39PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> >+  for (i=0; i<(int)grub_strlen((char *)var); i++)
> >+    var16[i] = var[i];
> >+  var16[i] = '\0';
> >+
> We use grub_utf8_to_utf16. Also don't forget to multiply the malloc
> length by GRUB_MAX_UTF16_PER_UTF8 ((GRUB_MAX_UTF16_PER_UTF8 * length
> + 1) * sizeof (var16[0]))

That's not currently exported. Any problem with changing that?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 4/4] Add support for getting EDID via EFI

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 08.02.2012 17:51, Matthew Garrett wrote:
> EFI gives a couple of defined methods for retrieving the EDID, so make use
> of them. Some Apple devices don't provide these but do stash the EDID in an
> nvram variable - grab it from there if it exists.
> ---
>   ChangeLog                 |    8 +++++
>   grub-core/video/efi_gop.c |   69 +++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 77 insertions(+), 0 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index 26d779b..d46b3d1 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,13 @@
>   2012-02-08  Matthew Garrett<mjg@redhat.com>
>
> +  }
> +
> +  grub_memcpy (&edid_info, edid->edid, sizeof(edid_info));
> +
This code may copy more than actually present. You need to take minimum 
of both length and fill the rest with zeros
> +  return GRUB_ERR_NONE;
> +}
> +
> +static grub_err_t
> +grub_gop_get_preferred_mode (unsigned int *width, unsigned int *height)
> +{
> +  struct grub_video_edid_info edid_info;
> +
> +  if (grub_video_gop_get_edid(&edid_info) == GRUB_ERR_NONE)
> +    {
> +      if (grub_video_edid_checksum (&edid_info) == GRUB_ERR_NONE
> +	&&  grub_video_edid_preferred_mode (&edid_info, width, height)
> +	  == GRUB_ERR_NONE)
> +	return GRUB_ERR_NONE;
> +      else
> +	grub_dprintf("video", "invalid edid");
> +    }
> +  return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "cannot get preferred mode");
Please reuse the error which actually happened. E.g:
err = grub_video_gop_get_edid(&edid_info);
if (err)
    return err;
err =  grub_video_edid_checksum (&edid_info);
if (err)
   return err;
...
> +}
> +
> +static grub_err_t
>   grub_video_gop_setup (unsigned int width, unsigned int height,
>   		      unsigned int mode_type,
>   		      unsigned int mode_mask __attribute__ ((unused)))
> @@ -268,10 +319,18 @@ grub_video_gop_setup (unsigned int width, unsigned int height,
>     unsigned bpp;
>     int found = 0;
>     unsigned long long best_volume = 0;
> +  int preferred_mode = 0;
>
>     depth = (mode_type&  GRUB_VIDEO_MODE_TYPE_DEPTH_MASK)
>       >>  GRUB_VIDEO_MODE_TYPE_DEPTH_POS;
>
> +  if (width == 0&&  height == 0)
> +    {
> +      grub_gop_get_preferred_mode (&width,&height);
> +      if (grub_errno == GRUB_ERR_NONE)
> +	  preferred_mode = 1;
> +    }
> +
You need to discard error if you continue
>     /* Keep current mode if possible.  */
>     if (gop->mode->info)
>       {
> @@ -306,6 +365,15 @@ grub_video_gop_setup (


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

Re: [PATCH 2/4] Add grub_efi_get_variable

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 08.02.2012 21:04, Matthew Garrett wrote:
> On Wed, Feb 08, 2012 at 08:55:39PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>>> +  for (i=0; i<(int)grub_strlen((char *)var); i++)
>>> +    var16[i] = var[i];
>>> +  var16[i] = '\0';
>>> +
>> We use grub_utf8_to_utf16. Also don't forget to multiply the malloc
>> length by GRUB_MAX_UTF16_PER_UTF8 ((GRUB_MAX_UTF16_PER_UTF8 * length
>> + 1) * sizeof (var16[0]))
> That's not currently exported. Any problem with changing that?
It already is. The prototype and the define is in grub/charset.h


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

Re: [PATCH 2/4] Add grub_efi_get_variable

[Matthew Garrett]

On Wed, Feb 08, 2012 at 09:09:23PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 08.02.2012 21:04, Matthew Garrett wrote:
> >On Wed, Feb 08, 2012 at 08:55:39PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> >>>+  for (i=0; i<(int)grub_strlen((char *)var); i++)
> >>>+    var16[i] = var[i];
> >>>+  var16[i] = '\0';
> >>>+
> >>We use grub_utf8_to_utf16. Also don't forget to multiply the malloc
> >>length by GRUB_MAX_UTF16_PER_UTF8 ((GRUB_MAX_UTF16_PER_UTF8 * length
> >>+ 1) * sizeof (var16[0]))
> >That's not currently exported. Any problem with changing that?
> It already is. The prototype and the define is in grub/charset.h

Oh, I was thinking of modules, but then this is already in core. Yes, 
I'll do that.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 2/4] Add grub_efi_get_variable

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 08.02.2012 21:19, Matthew Garrett wrote:
> On Wed, Feb 08, 2012 at 09:09:23PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>> On 08.02.2012 21:04, Matthew Garrett wrote:
>>> On Wed, Feb 08, 2012 at 08:55:39PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>>>>> +  for (i=0; i<(int)grub_strlen((char *)var); i++)
>>>>> +    var16[i] = var[i];
>>>>> +  var16[i] = '\0';
>>>>> +
>>>> We use grub_utf8_to_utf16. Also don't forget to multiply the malloc
>>>> length by GRUB_MAX_UTF16_PER_UTF8 ((GRUB_MAX_UTF16_PER_UTF8 * length
>>>> + 1) * sizeof (var16[0]))
>>> That's not currently exported. Any problem with changing that?
>> It already is. The prototype and the define is in grub/charset.h
> Oh, I was thinking of modules, but then this is already in core. Yes,
> I'll do that.
grub_utf8_to_utf16 is in normal.mod. The easiest way to fix this problem 
is to inline grub_utf8_to_utf16. It's used only in 2 other different places.
>


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko