Re: [patch 5/9 v3] efi: export more efi table variable to sysfs

2013-11-23 Thread Borislav Petkov 
On Fri, Nov 22, 2013 at 10:48:50AM +0800, Dave Young wrote:
  efi.config_table = (unsigned long)efi.systab-tables;
  efi.fw_vendor= (unsigned long)efi.systab-fw_vendor;
  efi.runtime  = (unsigned long)efi.systab-runtime;
 
 Hmm, UEFI spec mentions the them like below so I use the order:

I'm sure by now you know you should not really trust the UEFI spec, or
any other spec for that matter :)

 Several fields of the EFI System Table must be converted from
 physical pointers to virtual pointers using the ConvertPointer()
 service. These fields include FirmwareVendor, RuntimeServices,
 and ConfigurationTable.
 
 But since you like the reverse I can change it in next version.

The reverse was simply a suggestion. The vertical alignment was more
what I aimed at because it makes this chunk much more readable IMO.

-- 
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 5/9 v3] efi: export more efi table variable to sysfs

2013-11-21 Thread Borislav Petkov 
On Thu, Nov 21, 2013 at 02:17:09PM +0800, dyo...@redhat.com wrote:
 --- efi.orig/arch/x86/platform/efi/efi.c
 +++ efi/arch/x86/platform/efi/efi.c
 @@ -653,6 +653,10 @@ void __init efi_init(void)
  
   set_bit(EFI_SYSTEM_TABLES, x86_efi_facility);
  
 + efi.fw_vendor = (unsigned long)efi.systab-fw_vendor;
 + efi.runtime = (unsigned long)efi.systab-runtime;
 + efi.config_table = (unsigned long)efi.systab-tables;

A bit more readable:

efi.config_table = (unsigned long)efi.systab-tables;
efi.fw_vendor= (unsigned long)efi.systab-fw_vendor;
efi.runtime  = (unsigned long)efi.systab-runtime;

-- 
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 5/9 v3] efi: export more efi table variable to sysfs

2013-11-21 Thread Greg KH 
On Thu, Nov 21, 2013 at 02:17:09PM +0800, dyo...@redhat.com wrote:
 Export fw_vendor, runtime and config tables physical
 addresses to /sys/firmware/efi/systab becaue kexec
 kernel will need them.
 
 From EFI spec these 3 variables will be updated to
 virtual address after entering virtual mode. But
 kernel startup code will need the physical address.
 
 changelog:
 Greg: add standalone sysfs files instead of add lines to systab
 Document them as testing ABI
 
 Signed-off-by: Dave Young dyo...@redhat.com
 ---
  Documentation/ABI/testing/sysfs-firmware-efi |   26 +
  arch/x86/platform/efi/efi.c  |4 +
  drivers/firmware/efi/efi.c   |   71 
 +--
  include/linux/efi.h  |3 +
  4 files changed, 101 insertions(+), 3 deletions(-)
 
 --- efi.orig/drivers/firmware/efi/efi.c
 +++ efi/drivers/firmware/efi/efi.c
 @@ -32,6 +32,9 @@ struct efi __read_mostly efi = {
   .hcdp   = EFI_INVALID_TABLE_ADDR,
   .uga= EFI_INVALID_TABLE_ADDR,
   .uv_systab  = EFI_INVALID_TABLE_ADDR,
 + .fw_vendor  = EFI_INVALID_TABLE_ADDR,
 + .runtime= EFI_INVALID_TABLE_ADDR,
 + .config_table  = EFI_INVALID_TABLE_ADDR,
  };
  EXPORT_SYMBOL(efi);
  
 @@ -71,6 +74,31 @@ static ssize_t systab_show(struct kobjec
  static struct kobj_attribute efi_attr_systab =
   __ATTR(systab, 0400, systab_show, NULL);
  
 +static ssize_t fw_vendor_show(struct kobject *kobj,
 + struct kobj_attribute *attr, char *buf)
 +{
 + return sprintf(buf, 0x%lx\n, efi.fw_vendor);
 +}
 +
 +static ssize_t runtime_show(struct kobject *kobj,
 + struct kobj_attribute *attr, char *buf)
 +{
 + return sprintf(buf, 0x%lx\n, efi.runtime);
 +}
 +
 +static ssize_t config_table_show(struct kobject *kobj,
 + struct kobj_attribute *attr, char *buf)
 +{
 + return sprintf(buf, 0x%lx\n, efi.config_table);
 +}
 +
 +static struct kobj_attribute efi_attr_fw_vendor =
 + __ATTR(fw_vendor, 0400, fw_vendor_show, NULL);
 +static struct kobj_attribute efi_attr_runtime =
 + __ATTR(runtime, 0400, runtime_show, NULL);
 +static struct kobj_attribute efi_attr_config_table =
 + __ATTR(config_table, 0400, config_table_show, NULL);
 +
  static struct attribute *efi_subsys_attrs[] = {
   efi_attr_systab.attr,
   NULL,   /* maybe more in the future? */
 @@ -123,21 +151,58 @@ static int __init efisubsys_init(void)
  
   error = sysfs_create_group(efi_kobj, efi_subsys_attr_group);
   if (error) {
 - pr_err(efi: Sysfs attribute export failed with error %d.\n,
 -error);
 + pr_err(efi: Sysfs attribute %s export failed with error %d.\n,
 +efi_attr_systab.attr.name, error);
   goto err_unregister;
   }
  
 + if (efi.fw_vendor != EFI_INVALID_TABLE_ADDR) {
 + error = sysfs_create_file(efi_kobj, efi_attr_fw_vendor.attr);
 + if (error) {
 + pr_err(efi: Sysfs attribute %s export failed with 
 error %d.\n,
 + efi_attr_fw_vendor.attr.name, error);
 + goto err_remove_group;
 + }
 + }
 +
 + if (efi.runtime != EFI_INVALID_TABLE_ADDR) {
 + error = sysfs_create_file(efi_kobj, efi_attr_runtime.attr);
 + if (error) {
 + pr_err(efi: Sysfs attribute %s export failed with 
 error %d.\n,
 + efi_attr_runtime.attr.name, error);
 + goto err_remove_fw_vendor;
 + }
 + }
 +
 + if (efi.config_table != EFI_INVALID_TABLE_ADDR) {
 + error = sysfs_create_file(efi_kobj,
 + efi_attr_config_table.attr);
 + if (error) {
 + pr_err(efi: Sysfs attribute %s export failed with 
 error %d.\n,
 + efi_attr_config_table.attr.name, error);
 + goto err_remove_runtime;
 + }
 + }

You don't need to do this if SOMETHING then create the file, just use
the is_visible attribute in the group to do this as a callback to
determine this when the group is registered.

That should clean up this logic a bunch.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 5/9 v3] efi: export more efi table variable to sysfs

2013-11-21 Thread Dave Young 
On 11/21/13 at 05:57pm, Borislav Petkov wrote:
 On Thu, Nov 21, 2013 at 02:17:09PM +0800, dyo...@redhat.com wrote:
  --- efi.orig/arch/x86/platform/efi/efi.c
  +++ efi/arch/x86/platform/efi/efi.c
  @@ -653,6 +653,10 @@ void __init efi_init(void)
   
  set_bit(EFI_SYSTEM_TABLES, x86_efi_facility);
   
  +   efi.fw_vendor = (unsigned long)efi.systab-fw_vendor;
  +   efi.runtime = (unsigned long)efi.systab-runtime;
  +   efi.config_table = (unsigned long)efi.systab-tables;
 
 A bit more readable:
 
   efi.config_table = (unsigned long)efi.systab-tables;
   efi.fw_vendor= (unsigned long)efi.systab-fw_vendor;
   efi.runtime  = (unsigned long)efi.systab-runtime;

Hmm, UEFI spec mentions the them like below so I use the order:

Several fields of the EFI System Table must be converted from
physical pointers to virtual pointers using the ConvertPointer()
service. These fields include FirmwareVendor, RuntimeServices,
and ConfigurationTable.

But since you like the reverse I can change it in next version.

--
Thanks for review
Dave
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[patch 5/9 v3] efi: export more efi table variable to sysfs

2013-11-20 Thread dyoung 
Export fw_vendor, runtime and config tables physical
addresses to /sys/firmware/efi/systab becaue kexec
kernel will need them.

From EFI spec these 3 variables will be updated to
virtual address after entering virtual mode. But
kernel startup code will need the physical address.

changelog:
Greg: add standalone sysfs files instead of add lines to systab
Document them as testing ABI

Signed-off-by: Dave Young dyo...@redhat.com
---
 Documentation/ABI/testing/sysfs-firmware-efi |   26 +
 arch/x86/platform/efi/efi.c  |4 +
 drivers/firmware/efi/efi.c   |   71 +--
 include/linux/efi.h  |3 +
 4 files changed, 101 insertions(+), 3 deletions(-)

--- efi.orig/drivers/firmware/efi/efi.c
+++ efi/drivers/firmware/efi/efi.c
@@ -32,6 +32,9 @@ struct efi __read_mostly efi = {
.hcdp   = EFI_INVALID_TABLE_ADDR,
.uga= EFI_INVALID_TABLE_ADDR,
.uv_systab  = EFI_INVALID_TABLE_ADDR,
+   .fw_vendor  = EFI_INVALID_TABLE_ADDR,
+   .runtime= EFI_INVALID_TABLE_ADDR,
+   .config_table  = EFI_INVALID_TABLE_ADDR,
 };
 EXPORT_SYMBOL(efi);
 
@@ -71,6 +74,31 @@ static ssize_t systab_show(struct kobjec
 static struct kobj_attribute efi_attr_systab =
__ATTR(systab, 0400, systab_show, NULL);
 
+static ssize_t fw_vendor_show(struct kobject *kobj,
+   struct kobj_attribute *attr, char *buf)
+{
+   return sprintf(buf, 0x%lx\n, efi.fw_vendor);
+}
+
+static ssize_t runtime_show(struct kobject *kobj,
+   struct kobj_attribute *attr, char *buf)
+{
+   return sprintf(buf, 0x%lx\n, efi.runtime);
+}
+
+static ssize_t config_table_show(struct kobject *kobj,
+   struct kobj_attribute *attr, char *buf)
+{
+   return sprintf(buf, 0x%lx\n, efi.config_table);
+}
+
+static struct kobj_attribute efi_attr_fw_vendor =
+   __ATTR(fw_vendor, 0400, fw_vendor_show, NULL);
+static struct kobj_attribute efi_attr_runtime =
+   __ATTR(runtime, 0400, runtime_show, NULL);
+static struct kobj_attribute efi_attr_config_table =
+   __ATTR(config_table, 0400, config_table_show, NULL);
+
 static struct attribute *efi_subsys_attrs[] = {
efi_attr_systab.attr,
NULL,   /* maybe more in the future? */
@@ -123,21 +151,58 @@ static int __init efisubsys_init(void)
 
error = sysfs_create_group(efi_kobj, efi_subsys_attr_group);
if (error) {
-   pr_err(efi: Sysfs attribute export failed with error %d.\n,
-  error);
+   pr_err(efi: Sysfs attribute %s export failed with error %d.\n,
+  efi_attr_systab.attr.name, error);
goto err_unregister;
}
 
+   if (efi.fw_vendor != EFI_INVALID_TABLE_ADDR) {
+   error = sysfs_create_file(efi_kobj, efi_attr_fw_vendor.attr);
+   if (error) {
+   pr_err(efi: Sysfs attribute %s export failed with 
error %d.\n,
+   efi_attr_fw_vendor.attr.name, error);
+   goto err_remove_group;
+   }
+   }
+
+   if (efi.runtime != EFI_INVALID_TABLE_ADDR) {
+   error = sysfs_create_file(efi_kobj, efi_attr_runtime.attr);
+   if (error) {
+   pr_err(efi: Sysfs attribute %s export failed with 
error %d.\n,
+   efi_attr_runtime.attr.name, error);
+   goto err_remove_fw_vendor;
+   }
+   }
+
+   if (efi.config_table != EFI_INVALID_TABLE_ADDR) {
+   error = sysfs_create_file(efi_kobj,
+   efi_attr_config_table.attr);
+   if (error) {
+   pr_err(efi: Sysfs attribute %s export failed with 
error %d.\n,
+   efi_attr_config_table.attr.name, error);
+   goto err_remove_runtime;
+   }
+   }
+
/* and the standard mountpoint for efivarfs */
efivars_kobj = kobject_create_and_add(efivars, efi_kobj);
if (!efivars_kobj) {
pr_err(efivars: Subsystem registration failed.\n);
error = -ENOMEM;
-   goto err_remove_group;
+   goto err_remove_config_table;
}
 
return 0;
 
+err_remove_config_table:
+   if (efi.config_table != EFI_INVALID_TABLE_ADDR)
+   sysfs_remove_file(efi_kobj, efi_attr_config_table.attr);
+err_remove_runtime:
+   if (efi.runtime != EFI_INVALID_TABLE_ADDR)
+   sysfs_remove_file(efi_kobj, efi_attr_runtime.attr);
+err_remove_fw_vendor:
+   if (efi.fw_vendor != EFI_INVALID_TABLE_ADDR)
+   sysfs_remove_file(efi_kobj, efi_attr_fw_vendor.attr);
 err_remove_group:
sysfs_remove_group(efi_kobj, efi_subsys_attr_group);