public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dell-laptop: Use buffer with 32-bit physical address
@ 2009-10-21 19:04 Stuart_Hayes
  2009-10-22  1:57 ` Matthew Garrett
  2009-10-22 15:12 ` Bjorn Helgaas
  0 siblings, 2 replies; 6+ messages in thread
From: Stuart_Hayes @ 2009-10-21 19:04 UTC (permalink / raw)
  To: linux-acpi; +Cc: mjg, Rezwanul_Kabir, Stuart_Hayes

[-- Attachment #1: Type: text/plain, Size: 452 bytes --]


Calls to communicate with system firmware via a SMI (using dcdbas) need
to use a buffer that has a physical address of 4GB or less.  Currently
the dell-laptop driver does not guarantee this, and when the buffer
address is higher than 4GB, the address is truncated to 32 bits and the
SMI handler writes to the wrong memory address.

Signed-off-by: Stuart Hayes <stuart_hayes@dell.com>

 
 <<dell-laptop-buffer-below-4G-for-2.6.31.4.patch>> 

[-- Attachment #2: dell-laptop-buffer-below-4G-for-2.6.31.4.patch --]
[-- Type: application/octet-stream, Size: 5704 bytes --]

--- linux-2.6.31/drivers/platform/x86/dell-laptop.c.orig	2009-10-19 15:37:24.000000000 -0500
+++ linux-2.6.31/drivers/platform/x86/dell-laptop.c	2009-10-19 17:45:51.000000000 -0500
@@ -23,6 +23,7 @@
 #include <linux/power_supply.h>
 #include <linux/acpi.h>
 #include <linux/input.h>
+#include <linux/mm.h>
 #include "../../firmware/dcdbas.h"
 
 #define BRIGHTNESS_TOKEN 0x7d
@@ -64,6 +65,10 @@ static struct rfkill *wifi_rfkill;
 static struct rfkill *bluetooth_rfkill;
 static struct rfkill *wwan_rfkill;
 
+static struct page *bufferpage;
+static struct calling_interface_buffer *buffer;
+DEFINE_MUTEX(buffer_mutex);
+
 static const struct dmi_system_id __initdata dell_device_table[] = {
 	{
 		.ident = "Dell laptop",
@@ -75,6 +80,17 @@ static const struct dmi_system_id __init
 	{ }
 };
 
+static void get_buffer(void)
+{
+	mutex_lock(&buffer_mutex);
+	memset(buffer, 0, sizeof(struct calling_interface_buffer));
+}
+
+static void release_buffer(void)
+{
+	mutex_unlock(&buffer_mutex);
+}
+
 static void parse_da_table(const struct dmi_header *dm)
 {
 	/* Final token is a terminator, so we don't want to copy it */
@@ -181,29 +197,29 @@ dell_send_request(struct calling_interfa
 
 static int dell_rfkill_set(void *data, bool blocked)
 {
-	struct calling_interface_buffer buffer;
 	int disable = blocked ? 1 : 0;
 	unsigned long radio = (unsigned long)data;
 
 	printk(KERN_WARNING "MATCH, disable is %d\n", disable);
 	printk(KERN_WARNING "MATCH, radio is %x\n", radio);
 
-	memset(&buffer, 0, sizeof(struct calling_interface_buffer));
-	buffer.input[0] = (1 | (radio<<8) | (disable << 16));
-	dell_send_request(&buffer, 17, 11);
+	get_buffer();
+	buffer->input[0] = (1 | (radio<<8) | (disable << 16));
+	dell_send_request(buffer, 17, 11);
+	release_buffer();
 
 	return 0;
 }
 
 static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 {
-	struct calling_interface_buffer buffer;
 	int status;
 	int bit = (unsigned long)data + 16;
 
-	memset(&buffer, 0, sizeof(struct calling_interface_buffer));
-	dell_send_request(&buffer, 17, 11);
-	status = buffer.output[1];
+	get_buffer();
+	dell_send_request(buffer, 17, 11);
+	status = buffer->output[1];
+	release_buffer();
 
 	printk(KERN_WARNING "MATCH, status is %x\n", status);
 
@@ -228,13 +244,13 @@ static void dell_rfkill_update(void)
 
 static int dell_setup_rfkill(void)
 {
-	struct calling_interface_buffer buffer;
 	int status;
 	int ret;
 
-	memset(&buffer, 0, sizeof(struct calling_interface_buffer));
-	dell_send_request(&buffer, 17, 11);
-	status = buffer.output[1];
+	get_buffer();
+	dell_send_request(buffer, 17, 11);
+	status = buffer->output[1];
+	release_buffer();
 
 	printk(KERN_WARNING "MATCH, SMBIOS CI return is %x\n", status);
 
@@ -294,39 +310,44 @@ err_wifi:
 
 static int dell_send_intensity(struct backlight_device *bd)
 {
-	struct calling_interface_buffer buffer;
-
-	memset(&buffer, 0, sizeof(struct calling_interface_buffer));
-	buffer.input[0] = find_token_location(BRIGHTNESS_TOKEN);
-	buffer.input[1] = bd->props.brightness;
+	get_buffer();
+	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
+	buffer->input[1] = bd->props.brightness;
 
-	if (buffer.input[0] == -1)
+	if (buffer->input[0] == -1) {
+		release_buffer();
 		return -ENODEV;
+	}
 
 	if (power_supply_is_system_supplied() > 0)
-		dell_send_request(&buffer, 1, 2);
+		dell_send_request(buffer, 1, 2);
 	else
-		dell_send_request(&buffer, 1, 1);
+		dell_send_request(buffer, 1, 1);
 
+	release_buffer();
 	return 0;
 }
 
 static int dell_get_intensity(struct backlight_device *bd)
 {
-	struct calling_interface_buffer buffer;
+	int ret;
 
-	memset(&buffer, 0, sizeof(struct calling_interface_buffer));
-	buffer.input[0] = find_token_location(BRIGHTNESS_TOKEN);
+	get_buffer();
+	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
 
-	if (buffer.input[0] == -1)
+	if (buffer->input[0] == -1) {
+		release_buffer();
 		return -ENODEV;
+	}
 
 	if (power_supply_is_system_supplied() > 0)
-		dell_send_request(&buffer, 0, 2);
+		dell_send_request(buffer, 0, 2);
 	else
-		dell_send_request(&buffer, 0, 1);
+		dell_send_request(buffer, 0, 1);
 
-	return buffer.output[1];
+	ret = buffer->output[1];
+	release_buffer();
+	return ret;
 }
 
 static struct backlight_ops dell_ops = {
@@ -420,7 +441,6 @@ static struct input_handler dell_input_h
 
 static int __init dell_init(void)
 {
-	struct calling_interface_buffer buffer;
 	int max_intensity = 0;
 	int ret;
 
@@ -434,6 +454,13 @@ static int __init dell_init(void)
 		return -ENODEV;
 	}
 
+	if (!(bufferpage = alloc_page(GFP_KERNEL | GFP_DMA32)))
+		return -ENOMEM;
+	buffer = page_address(bufferpage);
+	mutex_init(&buffer_mutex);
+
+	printk("MATCH--alloc got %p for buffer\n",buffer);
+
 	ret = dell_setup_rfkill();
 
 	if (ret) {
@@ -457,13 +484,14 @@ static int __init dell_init(void)
 		return 0;
 #endif
 
-	memset(&buffer, 0, sizeof(struct calling_interface_buffer));
-	buffer.input[0] = find_token_location(BRIGHTNESS_TOKEN);
+	get_buffer();
+	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
 
-	if (buffer.input[0] != -1) {
-		dell_send_request(&buffer, 0, 2);
-		max_intensity = buffer.output[3];
+	if (buffer->input[0] != -1) {
+		dell_send_request(buffer, 0, 2);
+		max_intensity = buffer->output[3];
 	}
+	release_buffer();
 
 	if (max_intensity) {
 		dell_backlight_device = backlight_device_register(
@@ -492,6 +520,8 @@ out:
 	if (wwan_rfkill)
 		rfkill_unregister(wwan_rfkill);
 	kfree(da_tokens);
+	__free_page(bufferpage);
 	return ret;
 }
 
@@ -505,6 +535,8 @@ static void __exit dell_exit(void)
 	if (wwan_rfkill)
 		rfkill_unregister(wwan_rfkill);
 	input_unregister_handler(&dell_input_handler);
+	__free_page(bufferpage);
 }
 
 module_init(dell_init);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] dell-laptop: Use buffer with 32-bit physical address
  2009-10-21 19:04 [PATCH] dell-laptop: Use buffer with 32-bit physical address Stuart_Hayes
@ 2009-10-22  1:57 ` Matthew Garrett
  2009-10-22 16:27   ` Stuart_Hayes
  2009-10-22 15:12 ` Bjorn Helgaas
  1 sibling, 1 reply; 6+ messages in thread
From: Matthew Garrett @ 2009-10-22  1:57 UTC (permalink / raw)
  To: Stuart_Hayes; +Cc: linux-acpi, Rezwanul_Kabir

On Wed, Oct 21, 2009 at 02:04:09PM -0500, Stuart_Hayes@Dell.com wrote:
> 
> Calls to communicate with system firmware via a SMI (using dcdbas) need
> to use a buffer that has a physical address of 4GB or less.  Currently
> the dell-laptop driver does not guarantee this, and when the buffer
> address is higher than 4GB, the address is truncated to 32 bits and the
> SMI handler writes to the wrong memory address.
> 
> Signed-off-by: Stuart Hayes <stuart_hayes@dell.com>

It looks like you've got a printk in there that should be dropped, but 
if you resubmit without that then feel free to add

Acked-by: Matthew Garrett <mjg@redhat.com>

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] dell-laptop: Use buffer with 32-bit physical address
  2009-10-21 19:04 [PATCH] dell-laptop: Use buffer with 32-bit physical address Stuart_Hayes
  2009-10-22  1:57 ` Matthew Garrett
@ 2009-10-22 15:12 ` Bjorn Helgaas
  1 sibling, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2009-10-22 15:12 UTC (permalink / raw)
  To: Stuart_Hayes; +Cc: linux-acpi, mjg, Rezwanul_Kabir

On Wednesday 21 October 2009 01:04:09 pm Stuart_Hayes@dell.com wrote:
> 
> Calls to communicate with system firmware via a SMI (using dcdbas) need
> to use a buffer that has a physical address of 4GB or less.  Currently
> the dell-laptop driver does not guarantee this, and when the buffer
> address is higher than 4GB, the address is truncated to 32 bits and the
> SMI handler writes to the wrong memory address.
> 
> Signed-off-by: Stuart Hayes <stuart_hayes@dell.com>

+static void get_buffer(void)
+{
+	mutex_lock(&buffer_mutex);
+	memset(buffer, 0, sizeof(struct calling_interface_buffer));
+}
+
+static void release_buffer(void)
+{
+	mutex_unlock(&buffer_mutex);
+}


 static int dell_rfkill_set(void *data, bool blocked)
 {
-	struct calling_interface_buffer buffer;
 	int disable = blocked ? 1 : 0;
 	unsigned long radio = (unsigned long)data;
 
 	printk(KERN_WARNING "MATCH, disable is %d\n", disable);
 	printk(KERN_WARNING "MATCH, radio is %x\n", radio);
 
-	memset(&buffer, 0, sizeof(struct calling_interface_buffer));
-	buffer.input[0] = (1 | (radio<<8) | (disable << 16));
-	dell_send_request(&buffer, 17, 11);
+	get_buffer();
+	buffer->input[0] = (1 | (radio<<8) | (disable << 16));
+	dell_send_request(buffer, 17, 11);
+	release_buffer();


I think it would be slightly nicer to do something like this:

    static struct calling_interface_buffer *get_buffer(void);

    static int dell_rfkill_set(void *data, bool blocked)
    {
	struct calling_interface_buffer *buffer;

	buffer = get_buffer();
	...
	release_buffer(buffer);
    }

    static struct calling_interface_buffer *buffer;
    DEFINE_MUTEX(buffer_mutex);

    static struct calling_interface_buffer *get_buffer(void)
    {
	mutex_lock(&buffer_mutex);
	memset(buffer, 0, sizeof(struct calling_interface_buffer));
	return buffer;
    }

because it makes it a little harder to mistakenly use the global
buffer without acquiring the mutex.  With a little work, the
global buffer could probably even be made static to get_buffer(),
to make it *impossible* to get the pointer outside the mutex.

Bjorn

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] dell-laptop: Use buffer with 32-bit physical address
  2009-10-22  1:57 ` Matthew Garrett
@ 2009-10-22 16:27   ` Stuart_Hayes
  0 siblings, 0 replies; 6+ messages in thread
From: Stuart_Hayes @ 2009-10-22 16:27 UTC (permalink / raw)
  To: mjg59; +Cc: linux-acpi, Rezwanul_Kabir

[-- Attachment #1: Type: text/plain, Size: 585 bytes --]


Oops... I grabbed the wrong patch file last time.  This is almost the
same, but not *exactly* the same as the previous patch without the
printk, so I didn't want to add your ack.


Calls to communicate with system firmware via a SMI (using dcdbas) 
need to use a buffer that has a physical address of 4GB or less.  
Currently the dell-laptop driver does not guarantee this, and when the 
buffer address is higher than 4GB, the address is truncated to 32 bits 
and the SMI handler writes to the wrong memory address.

Signed-off-by: Stuart Hayes <stuart_hayes@dell.com>


[-- Attachment #2: dell-laptop-buffer-below-4G-for-2.6.31.4-try2.patch --]
[-- Type: application/octet-stream, Size: 5006 bytes --]

--- linux-2.6.31.4/drivers/platform/x86/dell-laptop.c.virgin	2009-10-21 10:12:38.000000000 -0500
+++ linux-2.6.31.4/drivers/platform/x86/dell-laptop.c	2009-10-21 10:42:55.000000000 -0500
@@ -22,6 +22,7 @@
 #include <linux/rfkill.h>
 #include <linux/power_supply.h>
 #include <linux/acpi.h>
+#include <linux/mm.h>
 #include "../../firmware/dcdbas.h"
 
 #define BRIGHTNESS_TOKEN 0x7d
@@ -74,6 +75,20 @@ static const struct dmi_system_id __init
 	{ }
 };
 
+static struct calling_interface_buffer *buffer;
+DEFINE_MUTEX(buffer_mutex);
+
+static void get_buffer(void)
+{
+	mutex_lock(&buffer_mutex);
+	memset(buffer, 0, sizeof(struct calling_interface_buffer));
+}
+
+static void release_buffer(void)
+{
+	mutex_unlock(&buffer_mutex);
+}
+
 static void parse_da_table(const struct dmi_header *dm)
 {
 	/* Final token is a terminator, so we don't want to copy it */
@@ -176,26 +191,26 @@ dell_send_request(struct calling_interfa
 
 static int dell_rfkill_set(void *data, bool blocked)
 {
-	struct calling_interface_buffer buffer;
 	int disable = blocked ? 1 : 0;
 	unsigned long radio = (unsigned long)data;
 
-	memset(&buffer, 0, sizeof(struct calling_interface_buffer));
-	buffer.input[0] = (1 | (radio<<8) | (disable << 16));
-	dell_send_request(&buffer, 17, 11);
+	get_buffer();
+	buffer->input[0] = (1 | (radio<<8) | (disable << 16));
+	dell_send_request(buffer, 17, 11);
+	release_buffer();
 
 	return 0;
 }
 
 static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 {
-	struct calling_interface_buffer buffer;
 	int status;
 	int bit = (unsigned long)data + 16;
 
-	memset(&buffer, 0, sizeof(struct calling_interface_buffer));
-	dell_send_request(&buffer, 17, 11);
-	status = buffer.output[1];
+	get_buffer();
+	dell_send_request(buffer, 17, 11);
+	status = buffer->output[1];
+	release_buffer();
 
 	if (status & BIT(bit))
 		rfkill_set_hw_state(rfkill, !!(status & BIT(16)));
@@ -270,39 +285,45 @@ err_wifi:
 
 static int dell_send_intensity(struct backlight_device *bd)
 {
-	struct calling_interface_buffer buffer;
 
-	memset(&buffer, 0, sizeof(struct calling_interface_buffer));
-	buffer.input[0] = find_token_location(BRIGHTNESS_TOKEN);
-	buffer.input[1] = bd->props.brightness;
+	get_buffer();
+	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
+	buffer->input[1] = bd->props.brightness;
 
-	if (buffer.input[0] == -1)
+	if (buffer->input[0] == -1) {
+		release_buffer();
 		return -ENODEV;
+	}
 
 	if (power_supply_is_system_supplied() > 0)
-		dell_send_request(&buffer, 1, 2);
+		dell_send_request(buffer, 1, 2);
 	else
-		dell_send_request(&buffer, 1, 1);
+		dell_send_request(buffer, 1, 1);
 
+	release_buffer();
 	return 0;
 }
 
 static int dell_get_intensity(struct backlight_device *bd)
 {
-	struct calling_interface_buffer buffer;
+	int ret;
 
-	memset(&buffer, 0, sizeof(struct calling_interface_buffer));
-	buffer.input[0] = find_token_location(BRIGHTNESS_TOKEN);
+	get_buffer();
+	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
 
-	if (buffer.input[0] == -1)
+	if (buffer->input[0] == -1) {
+		release_buffer();
 		return -ENODEV;
+	}
 
 	if (power_supply_is_system_supplied() > 0)
-		dell_send_request(&buffer, 0, 2);
+		dell_send_request(buffer, 0, 2);
 	else
-		dell_send_request(&buffer, 0, 1);
+		dell_send_request(buffer, 0, 1);
 
-	return buffer.output[1];
+	ret = buffer->output[1];
+	release_buffer();
+	return ret;
 }
 
 static struct backlight_ops dell_ops = {
@@ -312,9 +333,9 @@ static struct backlight_ops dell_ops = {
 
 static int __init dell_init(void)
 {
-	struct calling_interface_buffer buffer;
 	int max_intensity = 0;
 	int ret;
+	struct page *bufferpage;
 
 	if (!dmi_check_system(dell_device_table))
 		return -ENODEV;
@@ -326,6 +347,15 @@ static int __init dell_init(void)
 		return -ENODEV;
 	}
 
+	/*
+	 * Allocate buffer below 4GB for SMI data--only 32-bit physical addr
+	 * is passed to SMI handler.
+	 */
+	if (!(bufferpage = alloc_page(GFP_KERNEL | GFP_DMA32)))
+		return -ENOMEM;
+	buffer = page_address(bufferpage);
+	mutex_init(&buffer_mutex);
+
 	ret = dell_setup_rfkill();
 
 	if (ret) {
@@ -341,13 +371,14 @@ static int __init dell_init(void)
 		return 0;
 #endif
 
-	memset(&buffer, 0, sizeof(struct calling_interface_buffer));
-	buffer.input[0] = find_token_location(BRIGHTNESS_TOKEN);
+	get_buffer();
+	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
 
-	if (buffer.input[0] != -1) {
-		dell_send_request(&buffer, 0, 2);
-		max_intensity = buffer.output[3];
+	if (buffer->input[0] != -1) {
+		dell_send_request(buffer, 0, 2);
+		max_intensity = buffer->output[3];
 	}
+	release_buffer();
 
 	if (max_intensity) {
 		dell_backlight_device = backlight_device_register(
@@ -376,6 +407,7 @@ out:
 	if (wwan_rfkill)
 		rfkill_unregister(wwan_rfkill);
 	kfree(da_tokens);
+	free_page(buffer);
 	return ret;
 }
 
@@ -388,6 +420,7 @@ static void __exit dell_exit(void)
 		rfkill_unregister(bluetooth_rfkill);
 	if (wwan_rfkill)
 		rfkill_unregister(wwan_rfkill);
+	free_page(buffer);
 }
 
 module_init(dell_init);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] dell-laptop: Use buffer with 32-bit physical address
@ 2010-02-04 22:56 Matthew Garrett
  2010-02-06 15:38 ` Henrique de Moraes Holschuh
  0 siblings, 1 reply; 6+ messages in thread
From: Matthew Garrett @ 2010-02-04 22:56 UTC (permalink / raw)
  To: linux-acpi; +Cc: lenb, stuart_hayes

From: Stuart Hayes <stuart_hayes@dell.com>

Calls to communicate with system firmware via a SMI (using dcdbas)
need to use a buffer that has a physical address of 4GB or less.
Currently the dell-laptop driver does not guarantee this, and when the
buffer address is higher than 4GB, the address is truncated to 32 bits
and the SMI handler writes to the wrong memory address.

Signed-off-by: Stuart Hayes <stuart_hayes@dell.com>
Acked-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/platform/x86/dell-laptop.c |   89 ++++++++++++++++++++++++-----------
 1 files changed, 61 insertions(+), 28 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 3780994..08d62c9 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -22,6 +22,7 @@
 #include <linux/rfkill.h>
 #include <linux/power_supply.h>
 #include <linux/acpi.h>
+#include <linux/mm.h>
 #include "../../firmware/dcdbas.h"
 
 #define BRIGHTNESS_TOKEN 0x7d
@@ -82,6 +83,20 @@ static const struct dmi_system_id __initdata dell_device_table[] = {
 	{ }
 };
 
+static struct calling_interface_buffer *buffer;
+DEFINE_MUTEX(buffer_mutex);
+
+static void get_buffer(void)
+{
+	mutex_lock(&buffer_mutex);
+	memset(buffer, 0, sizeof(struct calling_interface_buffer));
+}
+
+static void release_buffer(void)
+{
+	mutex_unlock(&buffer_mutex);
+}
+
 static void __init parse_da_table(const struct dmi_header *dm)
 {
 	/* Final token is a terminator, so we don't want to copy it */
@@ -184,26 +199,26 @@ dell_send_request(struct calling_interface_buffer *buffer, int class,
 
 static int dell_rfkill_set(void *data, bool blocked)
 {
-	struct calling_interface_buffer buffer;
 	int disable = blocked ? 1 : 0;
 	unsigned long radio = (unsigned long)data;
 
-	memset(&buffer, 0, sizeof(struct calling_interface_buffer));
-	buffer.input[0] = (1 | (radio<<8) | (disable << 16));
-	dell_send_request(&buffer, 17, 11);
+	get_buffer();
+	buffer->input[0] = (1 | (radio<<8) | (disable << 16));
+	dell_send_request(buffer, 17, 11);
+	release_buffer();
 
 	return 0;
 }
 
 static void dell_rfkill_query(struct rfkill *rfkill, void *data)
 {
-	struct calling_interface_buffer buffer;
 	int status;
 	int bit = (unsigned long)data + 16;
 
-	memset(&buffer, 0, sizeof(struct calling_interface_buffer));
-	dell_send_request(&buffer, 17, 11);
-	status = buffer.output[1];
+	get_buffer();
+	dell_send_request(buffer, 17, 11);
+	status = buffer->output[1];
+	release_buffer();
 
 	rfkill_set_sw_state(rfkill, !!(status & BIT(bit)));
 	rfkill_set_hw_state(rfkill, !(status & BIT(16)));
@@ -298,39 +313,45 @@ static void dell_cleanup_rfkill(void)
 
 static int dell_send_intensity(struct backlight_device *bd)
 {
-	struct calling_interface_buffer buffer;
 
-	memset(&buffer, 0, sizeof(struct calling_interface_buffer));
-	buffer.input[0] = find_token_location(BRIGHTNESS_TOKEN);
-	buffer.input[1] = bd->props.brightness;
+	get_buffer();
+	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
+	buffer->input[1] = bd->props.brightness;
 
-	if (buffer.input[0] == -1)
+	if (buffer->input[0] == -1) {
+		release_buffer();
 		return -ENODEV;
+	}
 
 	if (power_supply_is_system_supplied() > 0)
-		dell_send_request(&buffer, 1, 2);
+		dell_send_request(buffer, 1, 2);
 	else
-		dell_send_request(&buffer, 1, 1);
+		dell_send_request(buffer, 1, 1);
 
+	release_buffer();
 	return 0;
 }
 
 static int dell_get_intensity(struct backlight_device *bd)
 {
-	struct calling_interface_buffer buffer;
+	int ret;
 
-	memset(&buffer, 0, sizeof(struct calling_interface_buffer));
-	buffer.input[0] = find_token_location(BRIGHTNESS_TOKEN);
+	get_buffer();
+	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
 
-	if (buffer.input[0] == -1)
+	if (buffer->input[0] == -1) {
+		release_buffer();
 		return -ENODEV;
+	}
 
 	if (power_supply_is_system_supplied() > 0)
-		dell_send_request(&buffer, 0, 2);
+		dell_send_request(buffer, 0, 2);
 	else
-		dell_send_request(&buffer, 0, 1);
+		dell_send_request(buffer, 0, 1);
 
-	return buffer.output[1];
+	ret = buffer->output[1];
+	release_buffer();
+	return ret;
 }
 
 static struct backlight_ops dell_ops = {
@@ -340,9 +361,9 @@ static struct backlight_ops dell_ops = {
 
 static int __init dell_init(void)
 {
-	struct calling_interface_buffer buffer;
 	int max_intensity = 0;
 	int ret;
+	struct page *bufferpage;
 
 	if (!dmi_check_system(dell_device_table))
 		return -ENODEV;
@@ -366,6 +387,15 @@ static int __init dell_init(void)
 	if (ret)
 		goto fail_platform_device2;
 
+	/*
+	 * Allocate buffer below 4GB for SMI data--only 32-bit physical addr
+	 * is passed to SMI handler.
+	 */
+	if (!(bufferpage = alloc_page(GFP_KERNEL | GFP_DMA32)))
+		return -ENOMEM;
+	buffer = page_address(bufferpage);
+	mutex_init(&buffer_mutex);
+
 	ret = dell_setup_rfkill();
 
 	if (ret) {
@@ -381,13 +411,14 @@ static int __init dell_init(void)
 		return 0;
 #endif
 
-	memset(&buffer, 0, sizeof(struct calling_interface_buffer));
-	buffer.input[0] = find_token_location(BRIGHTNESS_TOKEN);
+	get_buffer();
+	buffer->input[0] = find_token_location(BRIGHTNESS_TOKEN);
 
-	if (buffer.input[0] != -1) {
-		dell_send_request(&buffer, 0, 2);
-		max_intensity = buffer.output[3];
+	if (buffer->input[0] != -1) {
+		dell_send_request(buffer, 0, 2);
+		max_intensity = buffer->output[3];
 	}
+	release_buffer();
 
 	if (max_intensity) {
 		dell_backlight_device = backlight_device_register(
@@ -419,6 +450,7 @@ fail_platform_device1:
 	platform_driver_unregister(&platform_driver);
 fail_platform_driver:
 	kfree(da_tokens);
+	free_page(buffer);
 	return ret;
 }
 
@@ -426,6 +458,7 @@ static void __exit dell_exit(void)
 {
 	backlight_device_unregister(dell_backlight_device);
 	dell_cleanup_rfkill();
+	free_page(buffer);
 }
 
 module_init(dell_init);
-- 
1.6.6


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] dell-laptop: Use buffer with 32-bit physical address
  2010-02-04 22:56 Matthew Garrett
@ 2010-02-06 15:38 ` Henrique de Moraes Holschuh
  0 siblings, 0 replies; 6+ messages in thread
From: Henrique de Moraes Holschuh @ 2010-02-06 15:38 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: linux-acpi, lenb, stuart_hayes

On Thu, 04 Feb 2010, Matthew Garrett wrote:
> Currently the dell-laptop driver does not guarantee this, and when the
> buffer address is higher than 4GB, the address is truncated to 32 bits
> and the SMI handler writes to the wrong memory address.

Hmm, this is a very dangerous bug, with the potential to cause
page-cache/file system corruption and data loss, isn't it?

What kernel versions does it affect?  How easy is it to trigger?  Shouldn't
it go to -stable post-haste as soon as it is merged in mainline?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-02-06 15:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-21 19:04 [PATCH] dell-laptop: Use buffer with 32-bit physical address Stuart_Hayes
2009-10-22  1:57 ` Matthew Garrett
2009-10-22 16:27   ` Stuart_Hayes
2009-10-22 15:12 ` Bjorn Helgaas
  -- strict thread matches above, loose matches on Subject: below --
2010-02-04 22:56 Matthew Garrett
2010-02-06 15:38 ` Henrique de Moraes Holschuh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox