| Message ID | 1462811099-16897-1-git-send-email-mario_limonciello@dell.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
> More complex SMI requests can return data that exceeds the 4 32 bit > arguments that are traditionally part of a request. > > To support more complex requests, the first input argument can be a > 32 bit physical address with a buffer properly built in advance. > > This new method prepares the buffer as the BIOS will look for > it in these requests. I guess rephrasing this sentence might improve clarity, e.g.: This patch adds a new method which prepares the buffer as required by the BIOS and then issues the specified SMI request. > Signed-off-by: Mario Limonciello <mario_limonciello@dell.com> > --- > drivers/platform/x86/dell-smbios.c | 43 ++++++++++++++++++++++++++++++++++++++ > drivers/platform/x86/dell-smbios.h | 2 ++ > 2 files changed, 45 insertions(+) > > diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c > index d2412ab..e3bbbb3 100644 > --- a/drivers/platform/x86/dell-smbios.c > +++ b/drivers/platform/x86/dell-smbios.c > @@ -33,12 +33,14 @@ struct calling_interface_structure { > } __packed; > > static struct calling_interface_buffer *buffer; > +static unsigned char *extended_buffer; > static DEFINE_MUTEX(buffer_mutex); > > static int da_command_address; > static int da_command_code; > static int da_num_tokens; > static struct calling_interface_token *da_tokens; > +static const char extended_key[4] = {'D', 'S', 'C', 'I'}; Is there any reason why you preferred declaring a char array over using a string literal in the memcpy() call below? > > int dell_smbios_error(int value) > { > @@ -92,6 +94,38 @@ void dell_smbios_send_request(int class, int select) > } > EXPORT_SYMBOL_GPL(dell_smbios_send_request); > > +/* More complex requests are served by sending > + * a pointer to a pre-allocated buffer > + * Bytes 0:3 are the size of the return value > + * Bytes 4:length are the returned value > + * > + * The return value is the data of the extended buffer request > + * The value of length will be updated to the length of the actual > + * buffer content > + * > + */ Nit: could you please format this comment so that it resembles the rest of the kernel in terms of whitespace placement, i.e.: -> /* * More complex requests are served by sending ... * The value of length will be updated to the length of the actual * buffer content -> */ > +unsigned char *dell_smbios_send_extended_request(int class, int select, > + size_t *length) Nice touch with length being an in/out parameter, I like it. > +{ > + u32 i; > + u32 *buffer_length = (u32 *)extended_buffer; Please swap these two lines so that local variable declarations are in "Reverse Christmas Tree" order. > + > + if (*length < 5 || *length - 4 > PAGE_SIZE) > + return NULL; > + > + *buffer_length = *length - 4; > + for (i = 4; i < *length; i += 4) > + if (*length - i > 4) > + memcpy(&extended_buffer[i], &extended_key, 4); I guess a comment just before the for loop might be useful for documenting the format expected by the BIOS, e.g.: /* * BIOS will reject the buffer with error code -5 if it does not * contain repeating "DSCI" strings from byte 4 onward */ > + > + *length = buffer_length[0]; This assignment should be moved below the dell_smbios_send_request() call as at this point the BIOS hasn't yet had a chance to update the buffer. By the way, I think it would be nice to use the dereference operator here instead of the subscript operator for consistency with a similar assignment 4 lines earlier. > + buffer->input[0] = virt_to_phys(extended_buffer); > + dell_smbios_send_request(class, select); > + > + return &extended_buffer[4]; > +} > +EXPORT_SYMBOL_GPL(dell_smbios_send_extended_request); > + > struct calling_interface_token *dell_smbios_find_token(int tokenid) > { > int i; > @@ -170,8 +204,16 @@ static int __init dell_smbios_init(void) > goto fail_buffer; > } > > + extended_buffer = (void *)__get_free_page(GFP_KERNEL | GFP_DMA32); > + if (!extended_buffer) { > + ret = -ENOMEM; > + goto fail_extended_buffer; > + } > + > return 0; > > +fail_extended_buffer: > + kfree(buffer); Please use free_page() instead, as in dell_smbios_exit().
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c index d2412ab..e3bbbb3 100644 --- a/drivers/platform/x86/dell-smbios.c +++ b/drivers/platform/x86/dell-smbios.c @@ -33,12 +33,14 @@ struct calling_interface_structure { } __packed; static struct calling_interface_buffer *buffer; +static unsigned char *extended_buffer; static DEFINE_MUTEX(buffer_mutex); static int da_command_address; static int da_command_code; static int da_num_tokens; static struct calling_interface_token *da_tokens; +static const char extended_key[4] = {'D', 'S', 'C', 'I'}; int dell_smbios_error(int value) { @@ -92,6 +94,38 @@ void dell_smbios_send_request(int class, int select) } EXPORT_SYMBOL_GPL(dell_smbios_send_request); +/* More complex requests are served by sending + * a pointer to a pre-allocated buffer + * Bytes 0:3 are the size of the return value + * Bytes 4:length are the returned value + * + * The return value is the data of the extended buffer request + * The value of length will be updated to the length of the actual + * buffer content + * + */ +unsigned char *dell_smbios_send_extended_request(int class, int select, + size_t *length) +{ + u32 i; + u32 *buffer_length = (u32 *)extended_buffer; + + if (*length < 5 || *length - 4 > PAGE_SIZE) + return NULL; + + *buffer_length = *length - 4; + for (i = 4; i < *length; i += 4) + if (*length - i > 4) + memcpy(&extended_buffer[i], &extended_key, 4); + + *length = buffer_length[0]; + buffer->input[0] = virt_to_phys(extended_buffer); + dell_smbios_send_request(class, select); + + return &extended_buffer[4]; +} +EXPORT_SYMBOL_GPL(dell_smbios_send_extended_request); + struct calling_interface_token *dell_smbios_find_token(int tokenid) { int i; @@ -170,8 +204,16 @@ static int __init dell_smbios_init(void) goto fail_buffer; } + extended_buffer = (void *)__get_free_page(GFP_KERNEL | GFP_DMA32); + if (!extended_buffer) { + ret = -ENOMEM; + goto fail_extended_buffer; + } + return 0; +fail_extended_buffer: + kfree(buffer); fail_buffer: kfree(da_tokens); return ret; @@ -181,6 +223,7 @@ static void __exit dell_smbios_exit(void) { kfree(da_tokens); free_page((unsigned long)buffer); + free_page((unsigned long)extended_buffer); } subsys_initcall(dell_smbios_init); diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h index ec7d40a..c0f4395 100644 --- a/drivers/platform/x86/dell-smbios.h +++ b/drivers/platform/x86/dell-smbios.h @@ -41,6 +41,8 @@ struct calling_interface_buffer *dell_smbios_get_buffer(void); void dell_smbios_clear_buffer(void); void dell_smbios_release_buffer(void); void dell_smbios_send_request(int class, int select); +unsigned char *dell_smbios_send_extended_request(int class, int select, + size_t *length); struct calling_interface_token *dell_smbios_find_token(int tokenid); #endif
More complex SMI requests can return data that exceeds the 4 32 bit arguments that are traditionally part of a request. To support more complex requests, the first input argument can be a 32 bit physical address with a buffer properly built in advance. This new method prepares the buffer as the BIOS will look for it in these requests. Signed-off-by: Mario Limonciello <mario_limonciello@dell.com> --- drivers/platform/x86/dell-smbios.c | 43 ++++++++++++++++++++++++++++++++++++++ drivers/platform/x86/dell-smbios.h | 2 ++ 2 files changed, 45 insertions(+)