applesmc: add sysfs file to report OSK
Subsystems (2)
PATCH 1/1 applesmc: add sysfs file to report OSK
The AppleSMC contains two char[32] keys, OSK0 and OSK1, which are not reported in the key count and index by default. These keys are used by the OS X boot sequence, and normally don't matter when running Linux.
This patch creates a sysfs entry which reports the value of these keys as an ASCII string, to help emulators (such as QEMU) load OS X when running on genuine Apple hardware.
---
For extra context: To boot OS X as a guest, QEMU must (among others) emulate the AppleSMC. To boot successfully, OS X insists on querying the (emulated) SMC for the value of OSK0 and OSK1. Currently, these values must be supplied on the QEMU command line as
-device applesmc,osk="...concatenated values of OSK0 and OSK1..."
With the availability of /sys/devices/platform/applesmc.768/osk, the emulated QEMU AppleSMC could acquire this string directly from the (Apple-manufactured) host machine.
Thanks,
Gabriel
drivers/hwmon/applesmc.c | 18 ++++++++++++++++++ 1 files changed, 18 insertions(+), 0 deletions(-)
drivers/hwmon/applesmc.c +18 -0
@@ -1013,6 +1013,23 @@ static ssize_t applesmc_key_at_index_store(struct device *dev,return count; } +static ssize_t applesmc_osk_show (struct device *dev, +struct device_attribute *attr,char *sysfsbuf) +{ +int fail; + +mutex_lock (&smcreg.mutex); + fail =read_smc (APPLESMC_READ_CMD ,"OSK0" , sysfsbuf,32 ) || +read_smc (APPLESMC_READ_CMD ,"OSK1" , sysfsbuf +32 ,32 ); +mutex_unlock (&smcreg.mutex); +if (fail) +return -1 ; + + sysfsbuf[64 ] ='\n' ; + sysfsbuf[65 ] ='\0' ; +return 65 ; +} +static struct led_classdev applesmc_backlight = { .name ="smc::kbd_backlight" , .default_trigger ="nand-disk" ,
@@ -1027,6 +1044,7 @@ static struct applesmc_node_group info_group[] = { { "key_at_index_type", applesmc_key_at_index_type_show }, { "key_at_index_data_length", applesmc_key_at_index_data_length_show }, { "key_at_index_data", applesmc_key_at_index_read_show }, + { "osk", applesmc_osk_show }, { } };
Signature · 7 lines
1.7.7.6 _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Quoted 9 lines · Gabriel L. Somlo
The AppleSMC contains two char[32] keys, OSK0 and OSK1, which are not reported in the key count and index by default. These keys are used by the OS X boot sequence, and normally don't matter when running Linux.
This patch creates a sysfs entry which reports the value of these keys as an ASCII string, to help emulators (such as QEMU) load OS X when running on genuine Apple hardware.
Signed-off-by Gabriel L. Somlo <[email protected]>
Alex
Quoted 58 lines
---
For extra context: To boot OS X as a guest, QEMU must (among others) emulate the AppleSMC. To boot successfully, OS X insists on querying the (emulated) SMC for the value of OSK0 and OSK1. Currently, these values must be supplied on the QEMU command line as
-device applesmc,osk="...concatenated values of OSK0 and OSK1..."
With the availability of
/sys/devices/platform/applesmc.768/osk, the emulated QEMU AppleSMC could acquire this string directly from the (Apple-manufactured) host machine.Thanks,
Gabriel
drivers/hwmon/applesmc.c| 18 ++++++++++++++++++1 files changed, 18 insertions(+), 0 deletions(-)drivers/hwmon/applesmc.c +18
-0@@ -1013,6 +1013,23 @@ static ssize_t applesmc_key_at_index_store(struct device *dev,return count; } +static ssize_t applesmc_osk_show (struct device *dev, +struct device_attribute *attr,char *sysfsbuf) +{ +int fail; + +mutex_lock (&smcreg.mutex); + fail =read_smc (APPLESMC_READ_CMD ,"OSK0" , sysfsbuf,32 ) || +read_smc (APPLESMC_READ_CMD ,"OSK1" , sysfsbuf +32 ,32 ); +mutex_unlock (&smcreg.mutex); +if (fail) +return -1 ; + + sysfsbuf[64 ] ='\n' ; + sysfsbuf[65 ] ='\0' ; +return 65 ; +} +static struct led_classdev applesmc_backlight = { .name ="smc::kbd_backlight" , .default_trigger ="nand-disk" ,@@ -1027,6 +1044,7 @@ static struct applesmc_node_group info_group[] = { { "key_at_index_type", applesmc_key_at_index_type_show }, { "key_at_index_data_length", applesmc_key_at_index_data_length_show }, { "key_at_index_data", applesmc_key_at_index_read_show }, + { "osk", applesmc_osk_show }, { } };Signature · 1 line
1.7.7.6
Quoted 16 lines · Gabriel L. Somlo
The AppleSMC contains two char[32] keys, OSK0 and OSK1, which are not reported in the key count and index by default. These keys are used by the OS X boot sequence, and normally don't matter when running Linux.
This patch creates a sysfs entry which reports the value of these keys as an ASCII string, to help emulators (such as QEMU) load OS X when running on genuine Apple hardware.
Signed-off-by Gabriel L. Somlo <[email protected]>---
For extra context: To boot OS X as a guest, QEMU must (among others) emulate the AppleSMC. To boot successfully, OS X insists on querying the (emulated) SMC for the value of OSK0 and OSK1. Currently, these values must be supplied on the QEMU command line as
Quoted 6 lines
-device applesmc,osk="...concatenated values of OSK0 and OSK1..."
With the availability of
/sys/devices/platform/applesmc.768/osk, the emulated QEMU AppleSMC could acquire this string directly from the (Apple-manufactured) host machine.
Hmm ... this is a non-hwmon attribute which doesn't really belong into hwmon in the first place ... like several other attributes in the same driver.
So I'll leave it up to the maintainer to decide if we should accept it. Henrik ?
Thanks,
Guenter
Quoted 46 lines
Thanks,
Gabrieldrivers/hwmon/applesmc.c | 18 ++++++++++++++++++ 1 files changed, 18 insertions(+), 0 deletions(-)drivers/hwmon/applesmc.c +18
-0@@ -1013,6 +1013,23 @@ static ssize_t applesmc_key_at_index_store(struct device *dev,return count; } +static ssize_t applesmc_osk_show (struct device *dev, +struct device_attribute *attr,char *sysfsbuf) +{ +int fail; + +mutex_lock (&smcreg.mutex); + fail =read_smc (APPLESMC_READ_CMD ,"OSK0" , sysfsbuf,32 ) || +read_smc (APPLESMC_READ_CMD ,"OSK1" , sysfsbuf +32 ,32 ); +mutex_unlock (&smcreg.mutex); +if (fail) +return -1 ; + + sysfsbuf[64 ] ='\n' ; + sysfsbuf[65 ] ='\0' ; +return 65 ; +} +static struct led_classdev applesmc_backlight = { .name ="smc::kbd_backlight" , .default_trigger ="nand-disk" ,@@ -1027,6 +1044,7 @@ static struct applesmc_node_group info_group[] = { { "key_at_index_type", applesmc_key_at_index_type_show }, { "key_at_index_data_length", applesmc_key_at_index_data_length_show }, { "key_at_index_data", applesmc_key_at_index_read_show }, + { "osk", applesmc_osk_show }, { } };Signature · 1 line
1.7.7.6
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Hi Guenter,
Quoted 19 lines · Gabriel L. Somlo
Quoted 18 lines
The AppleSMC contains two char[32] keys, OSK0 and OSK1, which are not reported in the key count and index by default. These keys are used by the OS X boot sequence, and normally don't matter when running Linux.
This patch creates a sysfs entry which reports the value of these keys as an ASCII string, to help emulators (such as QEMU) load OS X when running on genuine Apple hardware.
Signed-off-by Gabriel L. Somlo <[email protected]>---
For extra context: To boot OS X as a guest, QEMU must (among others) emulate the AppleSMC. To boot successfully, OS X insists on querying the (emulated) SMC for the value of OSK0 and OSK1. Currently, these values must be supplied on the QEMU command line as
-device applesmc,osk="...concatenated values of OSK0 and OSK1..."
Quoted 8 lines
Quoted 4 lines
With the availability of
/sys/devices/platform/applesmc.768/osk, the emulated QEMU AppleSMC could acquire this string directly from the (Apple-manufactured) host machine.Hmm ... this is a non-hwmon attribute which doesn't really belong into hwmon in the first place ... like several other attributes in the same driver.
So I'll leave it up to the maintainer to decide if we should accept it. Henrik ?
Indeed, the reaons against this patch are too many. I was just about to reply with the below:
Gabriel,
The OSK string seems constant accross machines, which renders the patch rather pointless, no? And even if the OSK differs between a couple of machines, the emulator could easily handle it gracefully.
There are also some technical issues with the patch below, to keep in mind for future submissions.
Quoted 8 lines
drivers/hwmon/applesmc.c | 18 ++++++++++++++++++ 1 files changed, 18 insertions(+), 0 deletions(-)drivers/hwmon/applesmc.c +0
-0
drivers/hwmon/applesmc.c static ssize_t applesmc_key_at_index_store(struct device *dev, · line 1019
Quoted 7 lines
drivers/hwmon/applesmc.c +4
-0@@ -1013,6 +1013,23 @@ static ssize_t applesmc_key_at_index_store(struct device *dev,return count; } +static ssize_t applesmc_osk_show (struct device *dev, +struct device_attribute *attr,char *sysfsbuf) +{ +int fail;
All other functions use 'ret' here...
drivers/hwmon/applesmc.c static ssize_t applesmc_key_at_index_store(struct device *dev, · line 1023
Quoted 4 lines
drivers/hwmon/applesmc.c +4
-0+ +mutex_lock (&smcreg.mutex); + fail =read_smc (APPLESMC_READ_CMD ,"OSK0" , sysfsbuf,32 ) || +read_smc (APPLESMC_READ_CMD ,"OSK1" , sysfsbuf +32 ,32 );
The read function should propagate error messages, i.e., keep the return values here. And please read to buffers instead.
drivers/hwmon/applesmc.c static ssize_t applesmc_key_at_index_store(struct device *dev, · line 1026
Quoted 3 lines
drivers/hwmon/applesmc.c +3
-0+mutex_unlock (&smcreg.mutex); +if (fail) +return -1 ;
Return error here.
drivers/hwmon/applesmc.c static ssize_t applesmc_key_at_index_store(struct device *dev, · line 1030
Quoted 4 lines
drivers/hwmon/applesmc.c +4
-0+ + sysfsbuf[64 ] ='\n' ; + sysfsbuf[65 ] ='\0' ; +return 65 ;
A snprintf here, please.
drivers/hwmon/applesmc.c static struct applesmc_node_group info_group[] = { · line 1047
Quoted 10 lines
drivers/hwmon/applesmc.c +3
-0+} +static struct led_classdev applesmc_backlight = { .name ="smc::kbd_backlight" , .default_trigger ="nand-disk" ,@@ -1027,6 +1044,7 @@ static struct applesmc_node_group info_group[] = { { "key_at_index_type", applesmc_key_at_index_type_show }, { "key_at_index_data_length", applesmc_key_at_index_data_length_show }, { "key_at_index_data", applesmc_key_at_index_read_show }, + { "osk", applesmc_osk_show },
Unfortunately this is not a good place to put random things going forward.
Quoted 6 lines
drivers/hwmon/applesmc.c +0
-0{ } };Signature · 1 line
1.7.7.6
Given the above issues together with the weak rationale for the patch in the first place, this patch will not be applied.
Thanks.
Henrik
Quoted 30 lines · Henrik Rydberg
Hi Guenter,
Quoted 19 lines · Gabriel L. Somlo
Quoted 18 lines
The AppleSMC contains two char[32] keys, OSK0 and OSK1, which are not reported in the key count and index by default. These keys are used by the OS X boot sequence, and normally don't matter when running Linux.
This patch creates a sysfs entry which reports the value of these keys as an ASCII string, to help emulators (such as QEMU) load OS X when running on genuine Apple hardware.
Signed-off-by Gabriel L. Somlo <[email protected]>---
For extra context: To boot OS X as a guest, QEMU must (among others) emulate the AppleSMC. To boot successfully, OS X insists on querying the (emulated) SMC for the value of OSK0 and OSK1. Currently, these values must be supplied on the QEMU command line as
-device applesmc,osk="...concatenated values of OSK0 and OSK1..."
Quoted 8 lines
Quoted 4 lines
With the availability of
/sys/devices/platform/applesmc.768/osk, the emulated QEMU AppleSMC could acquire this string directly from the (Apple-manufactured) host machine.Hmm ... this is a non-hwmon attribute which doesn't really belong into hwmon in the first place ... like several other attributes in the same driver.
So I'll leave it up to the maintainer to decide if we should accept it. Henrik ?
Quoted 8 lines
Indeed, the reaons against this patch are too many. I was just about to reply with the below:
Gabriel,
The OSK string seems constant accross machines, which renders the patch rather pointless, no? And even if the OSK differs between a couple of machines, the emulator could easily handle it gracefully.
The point is that the return value of the OSK is a copyrighted string, we can not include in any other layer. The only way to make this legally savvy is to read the key from the host.
Quoted 3 lines
There are also some technical issues with the patch below, to keep in mind for future submissions.
Sigh - most of the comments below go back to earlier review from me. He basically had a version almost exactly like what you're asking him to do :). Funny how code style taste differs.
Alex
Quoted 61 lines
Quoted 9 lines
drivers/hwmon/applesmc.c| 18 ++++++++++++++++++1 files changed, 18 insertions(+), 0 deletions(-)drivers/hwmon/applesmc.c +0
-0drivers/hwmon/applesmc.c static ssize_t applesmc_key_at_index_store(struct device *dev, · line 1019
Quoted 7 lines
drivers/hwmon/applesmc.c +4
-0@@ -1013,6 +1013,23 @@ static ssize_t applesmc_key_at_index_store(struct device *dev,return count; } +static ssize_t applesmc_osk_show (struct device *dev, +struct device_attribute *attr,char *sysfsbuf) +{ +int fail;All other functions use 'ret' here...
drivers/hwmon/applesmc.c static ssize_t applesmc_key_at_index_store(struct device *dev, · line 1023
Quoted 4 lines
drivers/hwmon/applesmc.c +4
-0+ +mutex_lock (&smcreg.mutex); + fail =read_smc (APPLESMC_READ_CMD ,"OSK0" , sysfsbuf,32 ) || +read_smc (APPLESMC_READ_CMD ,"OSK1" , sysfsbuf +32 ,32 );The read function should propagate error messages, i.e., keep the return values here. And please read to buffers instead.
drivers/hwmon/applesmc.c static ssize_t applesmc_key_at_index_store(struct device *dev, · line 1026
Quoted 3 lines
drivers/hwmon/applesmc.c +3
-0+mutex_unlock (&smcreg.mutex); +if (fail) +return -1 ;Return error here.
drivers/hwmon/applesmc.c static ssize_t applesmc_key_at_index_store(struct device *dev, · line 1030
Quoted 4 lines
drivers/hwmon/applesmc.c +4
-0+ + sysfsbuf[64 ] ='\n' ; + sysfsbuf[65 ] ='\0' ; +return 65 ;A snprintf here, please.
drivers/hwmon/applesmc.c static struct applesmc_node_group info_group[] = { · line 1047
Quoted 10 lines
drivers/hwmon/applesmc.c +3
-0+} +static struct led_classdev applesmc_backlight = { .name ="smc::kbd_backlight" , .default_trigger ="nand-disk" ,@@ -1027,6 +1044,7 @@ static struct applesmc_node_group info_group[] = { { "key_at_index_type", applesmc_key_at_index_type_show }, { "key_at_index_data_length", applesmc_key_at_index_data_length_show }, { "key_at_index_data", applesmc_key_at_index_read_show }, + { "osk", applesmc_osk_show },Unfortunately this is not a good place to put random things going forward.
Quoted 6 lines
drivers/hwmon/applesmc.c +0
-0{ } };Signature · 1 line
1.7.7.6
Quoted 5 lines
Given the above issues together with the weak rationale for the patch in the first place, this patch will not be applied.
Thanks.
Henrik
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Quoted 41 lines · Alexander Graf
Quoted 29 lines · Henrik Rydberg
Hi Guenter,
Quoted 19 lines · Gabriel L. Somlo
Quoted 18 lines
The AppleSMC contains two char[32] keys, OSK0 and OSK1, which are not reported in the key count and index by default. These keys are used by the OS X boot sequence, and normally don't matter when running Linux.
This patch creates a sysfs entry which reports the value of these keys as an ASCII string, to help emulators (such as QEMU) load OS X when running on genuine Apple hardware.
Signed-off-by Gabriel L. Somlo <[email protected]>---
For extra context: To boot OS X as a guest, QEMU must (among others) emulate the AppleSMC. To boot successfully, OS X insists on querying the (emulated) SMC for the value of OSK0 and OSK1. Currently, these values must be supplied on the QEMU command line as
-device applesmc,osk="...concatenated values of OSK0 and OSK1..."
Quoted 7 lines
Quoted 3 lines
With the availability of
/sys/devices/platform/applesmc.768/osk, the emulated QEMU AppleSMC could acquire this string directly from the (Apple-manufactured) host machine.Hmm ... this is a non-hwmon attribute which doesn't really belong into hwmon in the first place ... like several other attributes in the same driver.
So I'll leave it up to the maintainer to decide if we should accept it. Henrik ?
Quoted 8 lines
Indeed, the reaons against this patch are too many. I was just about to reply with the below:
Gabriel,
The OSK string seems constant accross machines, which renders the patch rather pointless, no? And even if the OSK differs between a couple of machines, the emulator could easily handle it gracefully.
Quoted 7 lines
The point is that the return value of the OSK is a copyrighted string, we can not include in any other layer. The only way to make this legally savvy is to read the key from the host.
Quoted 3 lines
There are also some technical issues with the patch below, to keep in mind for future submissions.
Sigh - most of the comments below go back to earlier review from me. He basically had a version almost exactly like what you're asking him to do :). Funny how code style taste differs.
And this is exactly the reason why I'm less and less motivated to waste my lifetime with upstream work ...
Quoted 68 lines
Alex
Quoted 65 lines
Quoted 9 lines
drivers/hwmon/applesmc.c| 18 ++++++++++++++++++1 files changed, 18 insertions(+), 0 deletions(-)drivers/hwmon/applesmc.c +0
-0drivers/hwmon/applesmc.c static ssize_t applesmc_key_at_index_store(struct device *dev, · line 1019
Quoted 7 lines
drivers/hwmon/applesmc.c +4
-0@@ -1013,6 +1013,23 @@ static ssize_t applesmc_key_at_index_store(struct device *dev,return count; } +static ssize_t applesmc_osk_show (struct device *dev, +struct device_attribute *attr,char *sysfsbuf) +{ +int fail;All other functions use 'ret' here...
drivers/hwmon/applesmc.c static ssize_t applesmc_key_at_index_store(struct device *dev, · line 1023
Quoted 4 lines
drivers/hwmon/applesmc.c +4
-0+ +mutex_lock (&smcreg.mutex); + fail =read_smc (APPLESMC_READ_CMD ,"OSK0" , sysfsbuf,32 ) || +read_smc (APPLESMC_READ_CMD ,"OSK1" , sysfsbuf +32 ,32 );The read function should propagate error messages, i.e., keep the return values here. And please read to buffers instead.
drivers/hwmon/applesmc.c static ssize_t applesmc_key_at_index_store(struct device *dev, · line 1026
Quoted 3 lines
drivers/hwmon/applesmc.c +3
-0+mutex_unlock (&smcreg.mutex); +if (fail) +return -1 ;Return error here.
drivers/hwmon/applesmc.c static ssize_t applesmc_key_at_index_store(struct device *dev, · line 1030
Quoted 4 lines
drivers/hwmon/applesmc.c +4
-0+ + sysfsbuf[64 ] ='\n' ; + sysfsbuf[65 ] ='\0' ; +return 65 ;A snprintf here, please.
drivers/hwmon/applesmc.c static struct applesmc_node_group info_group[] = { · line 1047
Quoted 10 lines
drivers/hwmon/applesmc.c +3
-0+} +static struct led_classdev applesmc_backlight = { .name ="smc::kbd_backlight" , .default_trigger ="nand-disk" ,@@ -1027,6 +1044,7 @@ static struct applesmc_node_group info_group[] = { { "key_at_index_type", applesmc_key_at_index_type_show }, { "key_at_index_data_length", applesmc_key_at_index_data_length_show }, { "key_at_index_data", applesmc_key_at_index_read_show }, + { "osk", applesmc_osk_show },Unfortunately this is not a good place to put random things going forward.
Quoted 5 lines
drivers/hwmon/applesmc.c +0
-0{ } };Signature · 1 line
1.7.7.6Given the above issues together with the weak rationale for the patch in the first place, this patch will not be applied.
Thanks.
Henrik
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Quoted 47 lines · Rene Rebe
Quoted 44 lines · Alexander Graf
Quoted 29 lines · Henrik Rydberg
Hi Guenter,
Quoted 19 lines · Gabriel L. Somlo
Quoted 18 lines
The AppleSMC contains two char[32] keys, OSK0 and OSK1, which are not reported in the key count and index by default. These keys are used by the OS X boot sequence, and normally don't matter when running Linux.
This patch creates a sysfs entry which reports the value of these keys as an ASCII string, to help emulators (such as QEMU) load OS X when running on genuine Apple hardware.
Signed-off-by Gabriel L. Somlo <[email protected]>---
For extra context: To boot OS X as a guest, QEMU must (among others) emulate the AppleSMC. To boot successfully, OS X insists on querying the (emulated) SMC for the value of OSK0 and OSK1. Currently, these values must be supplied on the QEMU command line as
-device applesmc,osk="...concatenated values of OSK0 and OSK1..."
Quoted 7 lines
Quoted 3 lines
With the availability of
/sys/devices/platform/applesmc.768/osk, the emulated QEMU AppleSMC could acquire this string directly from the (Apple-manufactured) host machine.Hmm ... this is a non-hwmon attribute which doesn't really belong into hwmon in the first place ... like several other attributes in the same driver.
So I'll leave it up to the maintainer to decide if we should accept it. Henrik ?
Quoted 8 lines
Indeed, the reaons against this patch are too many. I was just about to reply with the below:
Gabriel,
The OSK string seems constant accross machines, which renders the patch rather pointless, no? And even if the OSK differs between a couple of machines, the emulator could easily handle it gracefully.
The point is that the return value of the OSK is a copyrighted string, we can not include in any other layer. The only way to make this legally savvy is to read the key from the host.
Quoted 1 line
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
There are also some technical issues with the patch below, to keep in mind for future submissions.
Sigh - most of the comments below go back to earlier review from me. He basically had a version almost exactly like what you're asking him to do :). Funny how code style taste differs.
And this is exactly the reason why I'm less and less motivated to waste my lifetime with upstream work ...
It's nit that bad really. Gabriel can just send his earlier version again with the ret variable name changed and then it shouldn't be an issue to get it in.
Reading the key really is an important bit in creating a legally safe and easy method to virtualize Mac OS X on Linux. Just believe us on this one :).
Alex
Quoted 50 lines · Alexander Graf
Quoted 48 lines · Rene Rebe
Quoted 47 lines · Alexander Graf
Quoted 29 lines · Henrik Rydberg
Hi Guenter,
Quoted 19 lines · Gabriel L. Somlo
Quoted 18 lines
The AppleSMC contains two char[32] keys, OSK0 and OSK1, which are not reported in the key count and index by default. These keys are used by the OS X boot sequence, and normally don't matter when running Linux.
This patch creates a sysfs entry which reports the value of these keys as an ASCII string, to help emulators (such as QEMU) load OS X when running on genuine Apple hardware.
Signed-off-by Gabriel L. Somlo <[email protected]>---
For extra context: To boot OS X as a guest, QEMU must (among others) emulate the AppleSMC. To boot successfully, OS X insists on querying the (emulated) SMC for the value of OSK0 and OSK1. Currently, these values must be supplied on the QEMU command line as
-device applesmc,osk="...concatenated values of OSK0 and OSK1..."
Quoted 7 lines
Quoted 3 lines
With the availability of
/sys/devices/platform/applesmc.768/osk, the emulated QEMU AppleSMC could acquire this string directly from the (Apple-manufactured) host machine.Hmm ... this is a non-hwmon attribute which doesn't really belong into hwmon in the first place ... like several other attributes in the same driver.
So I'll leave it up to the maintainer to decide if we should accept it. Henrik ?
Quoted 8 lines
Indeed, the reaons against this patch are too many. I was just about to reply with the below:
Gabriel,
The OSK string seems constant accross machines, which renders the patch rather pointless, no? And even if the OSK differs between a couple of machines, the emulator could easily handle it gracefully.
The point is that the return value of the OSK is a copyrighted string, we can not include in any other layer. The only way to make this legally savvy is to read the key from the host.
Quoted 3 lines
There are also some technical issues with the patch below, to keep in mind for future submissions.
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Sigh - most of the comments below go back to earlier review from me. He basically had a version almost exactly like what you're asking him to do :). Funny how code style taste differs.
And this is exactly the reason why I'm less and less motivated to waste my lifetime with upstream work ...
It's nit that bad really. Gabriel can just send his earlier version again with the ret variable name changed and then it shouldn't be an issue to get it in.
Reading the key really is an important bit in creating a legally safe and easy method to virtualize Mac OS X on Linux. Just believe us on this one :).
Sure, you do not need to convince me about that ;-)
Signature · 8 lines
René Rebe, ExactCODE GmbH, Jaegerstr. 67, DE-10117 Berlin http://exactcode.com | http://t2-project.org | http://rene.rebe.de _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
PATCH v2 1/1 applesmc: add sysfs file to report OSK
In reply to Henrik RydbergThe AppleSMC contains two char[32] keys, OSK0 and OSK1, which are not reported in the key count and index by default. These keys are used by the OS X boot sequence, and normally don't matter when running Linux.
This patch creates a sysfs entry which reports the value of these keys as an ASCII string, to help emulators (such as QEMU) load OS X when running on genuine Apple hardware.
---
Henrik,
Quoted 10 lines · Alexander Graf
Quoted 4 lines · Henrik Rydberg
The OSK string seems constant accross machines, which renders the patch rather pointless, no? And even if the OSK differs between a couple of machines, the emulator could easily handle it gracefully.
The point is that the return value of the OSK is a copyrighted string, we can not include in any other layer. The only way to make this
legally savvy is to read the key from the host.
I believe this second version addresses your stylistic objections.
As Alex pointed out, hardcoding the OSK in any open-source project might lead to legal/copyright related issues, whereas simply reporting whatever the the hardware happens to contain should keep everyone safe.
As for struct applesmc_node_group info_group[]:
Quoted 2 lines
Quoted 2 lines
Unfortunately this is not a good place to put random things going forward.
I figured "info_group" for the "least bad" place to put the OSK, given that it's more or less "information from/about the SMC". If you could be persuaded by the above arguments to accept the patch, and have a better place in mind, I'd be happy to move it there...
Thanks again,
Gabriel
drivers/hwmon/applesmc.c | 19 +++++++++++++++++++ 1 files changed, 19 insertions(+), 0 deletions(-)
drivers/hwmon/applesmc.c +19 -0
@@ -1013,6 +1013,24 @@ static ssize_t applesmc_key_at_index_store(struct device *dev,return count; } +static ssize_t applesmc_osk_show (struct device *dev, +struct device_attribute *attr,char *sysfsbuf) +{ +int ret; +char buf[65 ]; + +mutex_lock (&smcreg.mutex); + ret =read_smc (APPLESMC_READ_CMD ,"OSK0" , buf,32 ); +if (!ret) + ret =read_smc (APPLESMC_READ_CMD ,"OSK1" , buf +32 ,32 ); +mutex_unlock (&smcreg.mutex); +if (ret) +return ret; + + buf[64 ] ='\0' ; +return snprintf (sysfsbuf,PAGE_SIZE ,"%s\n" , buf); +} +static struct led_classdev applesmc_backlight = { .name ="smc::kbd_backlight" , .default_trigger ="nand-disk" ,
@@ -1027,6 +1045,7 @@ static struct applesmc_node_group info_group[] = { { "key_at_index_type", applesmc_key_at_index_type_show }, { "key_at_index_data_length", applesmc_key_at_index_data_length_show }, { "key_at_index_data", applesmc_key_at_index_read_show }, + { "osk", applesmc_osk_show }, { } };
Signature · 7 lines
1.7.7.6 _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Henrik, I agree with you that it's silly to poke the hardware for a string we KNOW will NEVER change :)
That being said, I too would love to live in a world where technical common sense prevailed over bullshit lawyerly tactics, "corporate strategies", and the ensuing FUD.
The way things stand today, in order to boot Mac OS X on QEMU/KVM/Linux (running on Mac hardware of course, in compliance with Apple's license) requires the end user supplying the OSK on the QEMU command line via the -osk="..." argument.
I would like to make it easy for people who run Linux natively on Apple hardware (of which I am one) to run OS X from QEMU "out of the box".
I could try to hardcode the OSK inside QEMU, or I could try to include it as a default config file entry, but I'm quite certain the QEMU project would be uncomfortable "distributing" a string on which Apple claims copyright. Even if that happened, distros might then balk at shipping QEMU as part of their package repositories, for the exact same reason.
Currently, we're all beating around the bush about the OSK, with vague hints and hoops to jump through for end users to acquire the OSK and pass it to QEMU on the command line. This process will have the unfortunate side effect of filtering out "normal" users :(
The only viable (from a legal CYA standpoint) thing I can think of is to make it easy to acquire the OSK automatically, on demand, directly from the hardware. Right now, the logical place for that is applesmc.ko. It already controls access to the SMC, and already reports values for various keys.
Yes it may feel silly to "go through the motions" when you KNOW it's always going to be the same value that gets returned. But this patch is mostly about reassuring people that lawyers will continue not to have a reason to get involved...
Thanks for reading, and hope I'm being sufficiently persuasive :) --Gabriel
Quoted 42 lines
drivers/hwmon/applesmc.c | 19 +++++++++++++++++++ 1 files changed, 19 insertions(+), 0 deletions(-)drivers/hwmon/applesmc.c +19
-0@@ -1013,6 +1013,24 @@ static ssize_t applesmc_key_at_index_store(struct device *dev,return count; } +static ssize_t applesmc_osk_show (struct device *dev, +struct device_attribute *attr,char *sysfsbuf) +{ +int ret; +char buf[65 ]; + +mutex_lock (&smcreg.mutex); + ret =read_smc (APPLESMC_READ_CMD ,"OSK0" , buf,32 ); +if (!ret) + ret =read_smc (APPLESMC_READ_CMD ,"OSK1" , buf +32 ,32 ); +mutex_unlock (&smcreg.mutex); +if (ret) +return ret; + + buf[64 ] ='\0' ; +return snprintf (sysfsbuf,PAGE_SIZE ,"%s\n" , buf); +} +static struct led_classdev applesmc_backlight = { .name ="smc::kbd_backlight" , .default_trigger ="nand-disk" ,@@ -1027,6 +1045,7 @@ static struct applesmc_node_group info_group[] = { { "key_at_index_type", applesmc_key_at_index_type_show }, { "key_at_index_data_length", applesmc_key_at_index_data_length_show }, { "key_at_index_data", applesmc_key_at_index_read_show }, + { "osk", applesmc_osk_show }, { } };Signature · 1 line
1.7.7.6
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Hi Gabriel,
Quoted 5 lines
I could try to hardcode the OSK inside QEMU, or I could try to include it as a default config file entry, but I'm quite certain the QEMU project would be uncomfortable "distributing" a string on which Apple claims copyright. Even if that happened, distros might then balk at shipping QEMU as part of their package repositories, for the exact same reason.
I understand this is frustrating, and I believe you have made a good case for the rationale. However, the technical issues remain. Also, there might still be other, simpler, solutions.
Quoted 5 lines
The only viable (from a legal CYA standpoint) thing I can think of is to make it easy to acquire the OSK automatically, on demand, directly from the hardware. Right now, the logical place for that is applesmc.ko. It already controls access to the SMC, and already reports values for various keys.
How about encrypting the string with a key only found on an Apple computer? There are strings available in both ACPI and EFI that could serve such a purpose.
Regarding the patch, I agree with Guenter that putting more unrelated things into the hwmon subsystem makes no sense. Most of the information in applesmc should go into the hwmon, thermal, backlight and input subsystems, but some strings should go somewhere else (maybe /sys/firmware/smc/?). The reluctance you experience here is a technical one; someone will need to make an effort to create a good place for your string, and it does not help that the string is, in fact, a constant. :-)
So, don't give up hope, but please do not expect an immediate solution.
Thanks.
Henrik
Quoted 9 lines · Henrik Rydberg
Quoted 5 lines
The only viable (from a legal CYA standpoint) thing I can think of is to make it easy to acquire the OSK automatically, on demand, directly from the hardware. Right now, the logical place for that is applesmc.ko. It already controls access to the SMC, and already reports values for various keys.
How about encrypting the string with a key only found on an Apple computer? There are strings available in both ACPI and EFI that could serve such a purpose.
I'd still be distributing the copyrighted string. I don't expect encryption to placate the proverbial lawyers, nor the project/distro maintainers who want to avoid them :)
Plus, now I'd also be taking on the responsibility of deciding when it's (legally) OK to let QEMU boot OS X and when it isn't.
Besides, the most obvious Apple-only key that fits your description is the OSK itself ! :)
Quoted 10 lines
Regarding the patch, I agree with Guenter that putting more unrelated things into the hwmon subsystem makes no sense. Most of the information in applesmc should go into the hwmon, thermal, backlight and input subsystems, but some strings should go somewhere else (maybe
/sys/firmware/smc/?). The reluctance you experience here is a technical one; someone will need to make an effort to create a good place for your string, and it does not help that the string is, in fact, a constant. :-)So, don't give up hope, but please do not expect an immediate solution.
OK, so if we agree that for legal-CYA reasons we must pretend to ignore the constant-ness of the key value, are we now talking about simply where in the sysfs hierarchy to place the node that will spit it out ?
Right now it's under /sys/devices/platform/applesmc.768/ (i.e., it's a device, it's platform specific, and it's name is applesmc-something).
The only ties to "hwmon" are that:
- there's a "/sys/class/hwmon/hwmonX" symlink to the above directory (among others, such as the graphics card and two instances of "coretemp", at least on my MacPro5,1);
- *most* of the files in it are indeed reporting sensor readings;
- the applesmc.c device driver source file itself lives under drivers/hwmon/ in the kernel source tree.
Is it common practice to split off a subset of the values reported by such a chip and make them show up elsehere (i.e. besides the /sys/devices/platform/<chipname> folder) ? Does the relative size of the subsets involved count as input into that decision proces ? :)
I personally don't know enough to have an opinion on where the best place for this might be. You suggested "/sys/firmware/smc/", and I'd be perfectly happy with that one too. Do you know who would decide whether that's an acceptable alternative to /sys/devices/platform/applesmc ?
Thanks again,
--Gabriel
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors