* [PATCH 0/1] ACPI / PCI: Fix "Operation not permitted" error reading sysfs acpi_index and label files.
@ 2014-06-18 14:51 Simone Gotti
2014-06-18 14:55 ` [PATCH 1/1] " Simone Gotti
0 siblings, 1 reply; 4+ messages in thread
From: Simone Gotti @ 2014-06-18 14:51 UTC (permalink / raw)
To: linux-pci; +Cc: linux-acpi, Jiang Liu, Narendra_K
Hi everyone,
starting from kernel 3.14 I noticed that the on board interfaces names on a Dell
PowerEdge E420 where renamed (by udev net_id internal) from eno1/eno2 to
enp2s0f0/enp2s0f1.
The root cause is that reading from the exported sysfs informations (acpi_index
and label) from drivers/pci/pci-label.c returns an "Operation not permitted"
error.
This machine supports PCI firmware specification 3.1 so the acpi methods are
preferred over the smbios one.
Debugging the problem I noticed that the call to the _DSM method returned the
second element with ACPI_TYPE_BUFFER type instead of ACPI_TYPE_STRING as
expected. The commit that forced this check is:
commit 1d0fcef732832432aee47cb75bf4a2cb3107be64 Author: Jiang Liu
<jiang.liu@linux.intel.com> Date: Thu Dec 19 20:38:13 2013 +0800
ACPI / PCI: replace open-coded _DSM code with helper functions
Previously the elements' types weren't checked and an ACPI_TYPE_BUFFER for the
second element was accepted.
The disassembled _DSM acpi method uses the ASL Unicode() macro that returns an
unicode string contained in a buffer.
Additionaly I noticed that an ascii string (as it should be the one of type
ACPI_TYPE_STRING) will be wrongly converted to utf8 from
dsm_label_utf16s_to_utf8s as it expects an utf16 string (I reproduced this
problem with a modified DSDT that returned an acpi string as label).
I don't have the PCI firmware specification so I don't know if an Unicode string
is correct or the bios is not compliant but I'm assuming this is possible. If
so, the following patch tries to fix this.
Thanks a lot.
Simone Gotti (1):
Fix "Operation not permitted" error reading sysfs acpi_index and label
files.
drivers/pci/pci-label.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
--
2.0.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/1] ACPI / PCI: Fix "Operation not permitted" error reading sysfs acpi_index and label files.
2014-06-18 14:51 [PATCH 0/1] ACPI / PCI: Fix "Operation not permitted" error reading sysfs acpi_index and label files Simone Gotti
@ 2014-06-18 14:55 ` Simone Gotti
2014-06-18 16:22 ` Jiang Liu
2014-07-02 22:19 ` Bjorn Helgaas
0 siblings, 2 replies; 4+ messages in thread
From: Simone Gotti @ 2014-06-18 14:55 UTC (permalink / raw)
To: linux-pci; +Cc: linux-acpi, Jiang Liu, Narendra_K
If the _DSM method returns an unicode string then the acpi type is not
ACPI_TYPE_STRING but ACPI_TYPE_BUFFER.
Before commit 1d0fcef732832432aee47cb75bf4a2cb3107be64 the dsm_get_label
function wasn't checking the second element type and it was correctly working only with
ACPI_TYPE_BUFFER as the call to dsm_label_utf16s_to_utf8s will return bad
encoding for an ascii string.
Also fix dsm_label_utf16s_to_utf8s to use acpi_object->buffer instead of
acpi_object->string.
This fixes changed onboard interfaces names generated by udev net_id internal as
reading from the acpi_index file returned "Operation not permitted" error.
Signed-off-by: Simone Gotti <simone.gotti@gmail.com>
---
drivers/pci/pci-label.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/pci-label.c b/drivers/pci/pci-label.c
index a3fbe20..2ab1b47 100644
--- a/drivers/pci/pci-label.c
+++ b/drivers/pci/pci-label.c
@@ -161,8 +161,8 @@ enum acpi_attr_enum {
static void dsm_label_utf16s_to_utf8s(union acpi_object *obj, char *buf)
{
int len;
- len = utf16s_to_utf8s((const wchar_t *)obj->string.pointer,
- obj->string.length,
+ len = utf16s_to_utf8s((const wchar_t *)obj->buffer.pointer,
+ obj->buffer.length,
UTF16_LITTLE_ENDIAN,
buf, PAGE_SIZE);
buf[len] = '\n';
@@ -187,16 +187,22 @@ static int dsm_get_label(struct device *dev, char *buf,
tmp = obj->package.elements;
if (obj->type == ACPI_TYPE_PACKAGE && obj->package.count == 2 &&
tmp[0].type == ACPI_TYPE_INTEGER &&
- tmp[1].type == ACPI_TYPE_STRING) {
+ (tmp[1].type == ACPI_TYPE_STRING ||
+ tmp[1].type == ACPI_TYPE_BUFFER)) {
/*
* The second string element is optional even when
* this _DSM is implemented; when not implemented,
* this entry must return a null string.
*/
- if (attr == ACPI_ATTR_INDEX_SHOW)
+ if (attr == ACPI_ATTR_INDEX_SHOW) {
scnprintf(buf, PAGE_SIZE, "%llu\n", tmp->integer.value);
- else if (attr == ACPI_ATTR_LABEL_SHOW)
- dsm_label_utf16s_to_utf8s(tmp + 1, buf);
+ } else if (attr == ACPI_ATTR_LABEL_SHOW) {
+ if (tmp[1].type == ACPI_TYPE_STRING)
+ scnprintf(buf, PAGE_SIZE, "%s\n",
+ tmp[1].string.pointer);
+ else if (tmp[1].type == ACPI_TYPE_BUFFER)
+ dsm_label_utf16s_to_utf8s(tmp + 1, buf);
+ }
len = strlen(buf) > 0 ? strlen(buf) : -1;
}
--
2.0.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] ACPI / PCI: Fix "Operation not permitted" error reading sysfs acpi_index and label files.
2014-06-18 14:55 ` [PATCH 1/1] " Simone Gotti
@ 2014-06-18 16:22 ` Jiang Liu
2014-07-02 22:19 ` Bjorn Helgaas
1 sibling, 0 replies; 4+ messages in thread
From: Jiang Liu @ 2014-06-18 16:22 UTC (permalink / raw)
To: Simone Gotti, linux-pci; +Cc: linux-acpi, Narendra_K
Hi Simone,
Thanks for fixing this, you may add
Reviewed-by: Jiang Liu <jiang.liu@linux.intel.com>
On 2014/6/18 22:55, Simone Gotti wrote:
> If the _DSM method returns an unicode string then the acpi type is not
> ACPI_TYPE_STRING but ACPI_TYPE_BUFFER.
>
> Before commit 1d0fcef732832432aee47cb75bf4a2cb3107be64 the dsm_get_label
> function wasn't checking the second element type and it was correctly working only with
> ACPI_TYPE_BUFFER as the call to dsm_label_utf16s_to_utf8s will return bad
> encoding for an ascii string.
>
> Also fix dsm_label_utf16s_to_utf8s to use acpi_object->buffer instead of
> acpi_object->string.
>
> This fixes changed onboard interfaces names generated by udev net_id internal as
> reading from the acpi_index file returned "Operation not permitted" error.
>
> Signed-off-by: Simone Gotti <simone.gotti@gmail.com>
> ---
> drivers/pci/pci-label.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/pci-label.c b/drivers/pci/pci-label.c
> index a3fbe20..2ab1b47 100644
> --- a/drivers/pci/pci-label.c
> +++ b/drivers/pci/pci-label.c
> @@ -161,8 +161,8 @@ enum acpi_attr_enum {
> static void dsm_label_utf16s_to_utf8s(union acpi_object *obj, char *buf)
> {
> int len;
> - len = utf16s_to_utf8s((const wchar_t *)obj->string.pointer,
> - obj->string.length,
> + len = utf16s_to_utf8s((const wchar_t *)obj->buffer.pointer,
> + obj->buffer.length,
> UTF16_LITTLE_ENDIAN,
> buf, PAGE_SIZE);
> buf[len] = '\n';
> @@ -187,16 +187,22 @@ static int dsm_get_label(struct device *dev, char *buf,
> tmp = obj->package.elements;
> if (obj->type == ACPI_TYPE_PACKAGE && obj->package.count == 2 &&
> tmp[0].type == ACPI_TYPE_INTEGER &&
> - tmp[1].type == ACPI_TYPE_STRING) {
> + (tmp[1].type == ACPI_TYPE_STRING ||
> + tmp[1].type == ACPI_TYPE_BUFFER)) {
> /*
> * The second string element is optional even when
> * this _DSM is implemented; when not implemented,
> * this entry must return a null string.
> */
> - if (attr == ACPI_ATTR_INDEX_SHOW)
> + if (attr == ACPI_ATTR_INDEX_SHOW) {
> scnprintf(buf, PAGE_SIZE, "%llu\n", tmp->integer.value);
> - else if (attr == ACPI_ATTR_LABEL_SHOW)
> - dsm_label_utf16s_to_utf8s(tmp + 1, buf);
> + } else if (attr == ACPI_ATTR_LABEL_SHOW) {
> + if (tmp[1].type == ACPI_TYPE_STRING)
> + scnprintf(buf, PAGE_SIZE, "%s\n",
> + tmp[1].string.pointer);
> + else if (tmp[1].type == ACPI_TYPE_BUFFER)
> + dsm_label_utf16s_to_utf8s(tmp + 1, buf);
> + }
> len = strlen(buf) > 0 ? strlen(buf) : -1;
> }
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] ACPI / PCI: Fix "Operation not permitted" error reading sysfs acpi_index and label files.
2014-06-18 14:55 ` [PATCH 1/1] " Simone Gotti
2014-06-18 16:22 ` Jiang Liu
@ 2014-07-02 22:19 ` Bjorn Helgaas
1 sibling, 0 replies; 4+ messages in thread
From: Bjorn Helgaas @ 2014-07-02 22:19 UTC (permalink / raw)
To: Simone Gotti; +Cc: linux-pci, linux-acpi, Jiang Liu, Narendra_K
On Wed, Jun 18, 2014 at 04:55:30PM +0200, Simone Gotti wrote:
> If the _DSM method returns an unicode string then the acpi type is not
> ACPI_TYPE_STRING but ACPI_TYPE_BUFFER.
>
> Before commit 1d0fcef732832432aee47cb75bf4a2cb3107be64 the dsm_get_label
> function wasn't checking the second element type and it was correctly working only with
> ACPI_TYPE_BUFFER as the call to dsm_label_utf16s_to_utf8s will return bad
> encoding for an ascii string.
>
> Also fix dsm_label_utf16s_to_utf8s to use acpi_object->buffer instead of
> acpi_object->string.
>
> This fixes changed onboard interfaces names generated by udev net_id internal as
> reading from the acpi_index file returned "Operation not permitted" error.
>
> Signed-off-by: Simone Gotti <simone.gotti@gmail.com>
Applied with Jiang's reviewed-by to pci/misc for v3.17. I also added a
stable tag for v3.14+. Thanks!
> ---
> drivers/pci/pci-label.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/pci-label.c b/drivers/pci/pci-label.c
> index a3fbe20..2ab1b47 100644
> --- a/drivers/pci/pci-label.c
> +++ b/drivers/pci/pci-label.c
> @@ -161,8 +161,8 @@ enum acpi_attr_enum {
> static void dsm_label_utf16s_to_utf8s(union acpi_object *obj, char *buf)
> {
> int len;
> - len = utf16s_to_utf8s((const wchar_t *)obj->string.pointer,
> - obj->string.length,
> + len = utf16s_to_utf8s((const wchar_t *)obj->buffer.pointer,
> + obj->buffer.length,
> UTF16_LITTLE_ENDIAN,
> buf, PAGE_SIZE);
> buf[len] = '\n';
> @@ -187,16 +187,22 @@ static int dsm_get_label(struct device *dev, char *buf,
> tmp = obj->package.elements;
> if (obj->type == ACPI_TYPE_PACKAGE && obj->package.count == 2 &&
> tmp[0].type == ACPI_TYPE_INTEGER &&
> - tmp[1].type == ACPI_TYPE_STRING) {
> + (tmp[1].type == ACPI_TYPE_STRING ||
> + tmp[1].type == ACPI_TYPE_BUFFER)) {
> /*
> * The second string element is optional even when
> * this _DSM is implemented; when not implemented,
> * this entry must return a null string.
> */
> - if (attr == ACPI_ATTR_INDEX_SHOW)
> + if (attr == ACPI_ATTR_INDEX_SHOW) {
> scnprintf(buf, PAGE_SIZE, "%llu\n", tmp->integer.value);
> - else if (attr == ACPI_ATTR_LABEL_SHOW)
> - dsm_label_utf16s_to_utf8s(tmp + 1, buf);
> + } else if (attr == ACPI_ATTR_LABEL_SHOW) {
> + if (tmp[1].type == ACPI_TYPE_STRING)
> + scnprintf(buf, PAGE_SIZE, "%s\n",
> + tmp[1].string.pointer);
> + else if (tmp[1].type == ACPI_TYPE_BUFFER)
> + dsm_label_utf16s_to_utf8s(tmp + 1, buf);
> + }
> len = strlen(buf) > 0 ? strlen(buf) : -1;
> }
>
> --
> 2.0.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-07-02 22:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-18 14:51 [PATCH 0/1] ACPI / PCI: Fix "Operation not permitted" error reading sysfs acpi_index and label files Simone Gotti
2014-06-18 14:55 ` [PATCH 1/1] " Simone Gotti
2014-06-18 16:22 ` Jiang Liu
2014-07-02 22:19 ` Bjorn Helgaas
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.