Re: [Devel] [PATCH v2] tools/power/acpi: exclude tools/* from .gitignore pattern
by Rafael J. Wysocki
On Wednesday, May 1, 2019 3:53:05 PM CEST Masahiro Yamada wrote:
> tools/power/acpi/.gitignore has the following entries:
>
> acpidbg
> acpidump
> ec
>
> They are intended to ignore the following build artifacts:
>
> tools/power/acpi/acpidbg
> tools/power/acpi/acpidump
> tools/power/acpi/ec
>
> However, those .gitignore entries are effective not only for the
> current directory, but also for any sub-directories.
>
> So, from the point of .gitignore grammar, the following check-in
> directories are also considered to be ignored:
>
> tools/power/acpi/tools/acpidbg
> tools/power/acpi/tools/acpidump
> tools/power/acpi/tools/ec
>
> As the manual gitignore(5) says "Files already tracked by Git are not
> affected", this is not a problem as far as Git is concerned.
>
> However, Git is not the only program that parses .gitignore because
> .gitignore is useful to distinguish build artifacts from source files.
>
> For example, tar(1) supports the --exclude-vcs-ignore option. As of
> writing, this option does not work perfectly, but it intends to create
> a tarball excluding files specified by .gitignore.
>
> So, I believe it is better to fix this issue.
>
> You can fix it by prefixing the pattern with a slash; the leading slash
> means the specified pattern is relative to the current directory.
>
> I also prefixed the "include" consistently. IMHO, it is safer when you
> intend to ignore specific files or directories.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro(a)socionext.com>
> ---
>
> Changes in v2:
> - Add more information to the commit log to clarify my main motivation
>
> tools/power/acpi/.gitignore | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/power/acpi/.gitignore b/tools/power/acpi/.gitignore
> index cba3d99..f698a0e 100644
> --- a/tools/power/acpi/.gitignore
> +++ b/tools/power/acpi/.gitignore
> @@ -1,4 +1,4 @@
> -acpidbg
> -acpidump
> -ec
> -include
> +/acpidbg
> +/acpidump
> +/ec
> +/include/
>
I actually have applied this one, not the v1, sorry for the confusion.
3 years, 4 months
Re: [Devel] [PATCH] tools/power/acpi: exclude tools/* from .gitignore pattern
by Rafael J. Wysocki
On Monday, April 29, 2019 4:45:06 PM CEST Masahiro Yamada wrote:
> tools/power/acpi/.gitignore has the following entries:
>
> acpidbg
> acpidump
> ec
>
> They are intended to ignore the following build artifacts:
>
> tools/power/acpi/acpidbg
> tools/power/acpi/acpidump
> tools/power/acpi/ec
>
> However, those .gitignore entries are effective not only for the
> current directory, but also for any sub-directories.
>
> So, the following directories are also considered to be ignored:
>
> tools/power/acpi/tools/acpidbg
> tools/power/acpi/tools/acpidump
> tools/power/acpi/tools/ec
>
> They are obviously version-controlled, so should be excluded from the
> .gitignore patterns.
>
> You can fix it by prefixing the patterns with '/', which means they
> are only effective in the current directory.
>
> I also prefixed the "include" consistently. IMHO, '/' prefixing is
> safer when you intend to ignore specific files or directories.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro(a)socionext.com>
> ---
>
> tools/power/acpi/.gitignore | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/power/acpi/.gitignore b/tools/power/acpi/.gitignore
> index cba3d99..f698a0e 100644
> --- a/tools/power/acpi/.gitignore
> +++ b/tools/power/acpi/.gitignore
> @@ -1,4 +1,4 @@
> -acpidbg
> -acpidump
> -ec
> -include
> +/acpidbg
> +/acpidump
> +/ec
> +/include/
>
Applied, thanks!
3 years, 4 months
Re: [Devel] [PATCH 10/10] docs: fix broken documentation links
by Rafael J. Wysocki
On Mon, May 20, 2019 at 4:48 PM Mauro Carvalho Chehab
<mchehab+samsung(a)kernel.org> wrote:
>
> Mostly due to x86 and acpi conversion, several documentation
> links are still pointing to the old file. Fix them.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung(a)kernel.org>
For the ACPI part:
Acked-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com>
3 years, 4 months
Re: [Devel] [PATCH v2] ACPI / device_sysfs: change _ADR representation to 64 bits
by Rafael J. Wysocki
On Thu, May 2, 2019 at 6:58 AM Vinod Koul <vkoul(a)kernel.org> wrote:
>
> On 01-05-19, 07:53, Pierre-Louis Bossart wrote:
> > Standards such as the MIPI DisCo for SoundWire 1.0 specification
> > assume the _ADR field is 64 bits.
> >
> > _ADR is defined as an "Integer" represented as 64 bits since ACPI 2.0
> > released in 2002. The low levels already use _ADR as 64 bits, e.g. in
> > struct acpi_device_info.
> >
> > This patch bumps the representation used for sysfs to 64 bits. To
> > avoid any compatibility/ABI issues, the printf format is only extended
> > to 16 characters when the actual _ADR value exceeds the 32 bit
> > maximum.
> >
> > Example with a SoundWire device, the results show the complete
> > vendorID and linkID which were omitted before:
> >
> > Before:
> > $ more /sys/bus/acpi/devices/device\:38/adr
> > 0x5d070000
> > After:
> > $ more /sys/bus/acpi/devices/device\:38/adr
> > 0x000010025d070000
> >
> > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com>
> > ---
> > v2: only use 64 bits when required to avoid compatibility issues
> > (feedback from Vinod and Rafael)
> >
> > drivers/acpi/device_sysfs.c | 6 ++++--
> > include/acpi/acpi_bus.h | 2 +-
> > 2 files changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
> > index 8940054d6250..7dda0ee05cd1 100644
> > --- a/drivers/acpi/device_sysfs.c
> > +++ b/drivers/acpi/device_sysfs.c
> > @@ -428,8 +428,10 @@ static ssize_t acpi_device_adr_show(struct device *dev,
> > {
> > struct acpi_device *acpi_dev = to_acpi_device(dev);
> >
> > - return sprintf(buf, "0x%08x\n",
> > - (unsigned int)(acpi_dev->pnp.bus_address));
> > + if (acpi_dev->pnp.bus_address > 0xFFFFFFFF)
>
> Would prefer to use U32_MAX instead of 0xFFFFFFFF
I would.
3 years, 4 months
ACPICA version 20190509 released
by Moore, Robert
09 May 2019. Summary of changes for version 20190509:
This release is available at https://acpica.org/downloads
1) ACPICA kernel-resident subsystem:
Revert commit 6c43e1a ("ACPICA: Clear status of GPEs before enabling them") that causes problems with Thunderbolt controllers to occur if a dock device is connected at init time (the xhci_hcd and thunderbolt modules crash which prevents peripherals connected through them from working). Commit 6c43e1a effectively causes commit ecc1165b8b74 ("ACPICA: Dispatch active GPEs at init time") to get undone, so the problem addressed by commit ecc1165b8b74 appears again as a result of it.
2) iASL Compiler/Disassembler and ACPICA tools:
Reverted iASL: Additional forward reference detection. This change reverts forward reference detection for field declarations. The feature unintentionally emitted AML bytecode with incorrect package lengths for some ASL code related to Fields and OperationRegions. This malformed AML can cause systems to crash
during boot. The malformed AML bytecode is emitted in iASL version 20190329 and 20190405.
iASL: improve forward reference detection. This change improves forward reference detection for named objects inside of scopes. If a parse object has the OP_NOT_FOUND_DURING_LOAD set, it means that Op is a reference to a named object that is declared later in the AML bytecode. This is allowed if the reference is inside of a method and the declaration is outside of a method like so:
DefinitionBlock(...)
{
Method (TEST)
{
Return (NUM0)
}
Name (NUM0,0)
}
However, if the declaration and reference are both in the same method or outside any methods, this is a forward reference and should be marked as an error because it would result in runtime errors.
DefinitionBlock(...)
{
Name (BUFF, Buffer (NUM0) {}) // Forward reference
Name (NUM0, 0x0)
Method (TEST)
{
Local0 = NUM1
Name (NUM1, 0x1) // Forward reference
return (Local0)
}
}
iASL: Implemented additional buffer overflow analysis for BufferField declarations. Check if a buffer index argument to a create buffer field operation is beyond the end of the target buffer.
This affects these AML operators:
AML_CREATE_FIELD_OP
AML_CREATE_BIT_FIELD_OP
AML_CREATE_BYTE_FIELD_OP
AML_CREATE_WORD_FIELD_OP
AML_CREATE_DWORD_FIELD_OP
AML_CREATE_QWORD_FIELD_OP
There are three conditions that must be satisfied in order to allow this validation at compile time:
1) The length of the target buffer must be an integer constant
2) The index specified in the create* must be an integer constant
3) For CreateField, the bit length argument must be non-zero.
Example:
Name (BUF1, Buffer() {1,2})
CreateField (BUF1, 7, 9, CF03) // 3: ERR
dsdt.asl 14: CreateField (BUF1, 7, 9, CF03) // 3: ERR
Error 6165 - ^ Buffer index beyond end of target buffer
3 years, 4 months
Re: [Devel] [PATCH] ACPICA: acpica: Fix possible NULL pointer dereference in acpi_ut_remove_reference
by Schmauss, Erik
> -----Original Message-----
> From: YueHaibing [mailto:yuehaibing@huawei.com]
> Sent: Wednesday, May 8, 2019 8:07 AM
> To: Moore, Robert <robert.moore(a)intel.com>; Schmauss, Erik
> <erik.schmauss(a)intel.com>; Wysocki, Rafael J <rafael.j.wysocki(a)intel.com>;
> lenb(a)kernel.org
> Cc: linux-kernel(a)vger.kernel.org; devel(a)acpica.org; linux-
> acpi(a)vger.kernel.org; YueHaibing <yuehaibing(a)huawei.com>
> Subject: [PATCH] ACPICA: acpica: Fix possible NULL pointer dereference in
> acpi_ut_remove_reference
>
> BUG: kernel NULL pointer dereference, address: 0000000000000000
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x0000) - not-present page PGD 0 P4D 0
> Oops: 0000 [#1
> CPU: 0 PID: 7393 Comm: modprobe Not tainted 5.1.0+ #34 Hardware name:
> QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.9.3-0-ge2fc41e-
> prebuilt.qemu-project.org 04/01/2014
> RIP: 0010:acpi_ut_update_object_reference+0xda/0x1e8
> Code: 4c 89 e7 eb ea 48 8b 7b 18 48 85 ff 0f 84 95 00 00 00 4c 8b 67 38 44 89 ee
> e8 dd fb ff ff 4c 89 e7 eb e6 48 8b 43 18 44 89 e2 <48> 8b 3c d0 48 85 ff 75 0b 41
> ff c4 44 3b 63 2c 72 e7 eb 66 8a 47
> RSP: 0018:ffffc90001c9f550 EFLAGS: 00010283
> RAX: 0000000000000000 RBX: ffff8882310d7288 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: 0000000000000001 RDI: ffff8882310d7288
> RBP: ffffc90001c9f580 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000001 R11: 000000003ef29b78 R12: 0000000000000000
> R13: 0000000000000001 R14: ffff88823122e000 R15: 0000000000000000
> FS: 00007f4469ead540(0000) GS:ffff888237a00000(0000)
> knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000000 CR3: 000000022c2b5000 CR4: 00000000000006f0 Call
> Trace:
> acpi_ut_remove_reference+0x29/0x2c
> acpi_ut_copy_iobject_to_iobject+0xd7/0xee
> acpi_ds_store_object_to_local+0x9a/0x181
> acpi_ex_store+0x233/0x279
> ? acpi_ds_create_operands+0x74/0xdb
> acpi_ex_opcode_1A_1T_1R+0x3c3/0x4fc
> acpi_ds_exec_end_op+0xd1/0x419
> acpi_ps_parse_loop+0x532/0x5d0
> acpi_ps_parse_aml+0x93/0x2c8
> acpi_ps_execute_method+0x16d/0x1b2
> acpi_ns_evaluate+0x1c1/0x26c
> acpi_ut_evaluate_object+0x7d/0x1a4
> acpi_rs_get_prt_method_data+0x30/0x66
> acpi_get_irq_routing_table+0x3d/0x56
> acpi_pci_irq_find_prt_entry+0x8d/0x300
> ? trace_hardirqs_on+0x3f/0x110
> acpi_pci_irq_lookup+0x35/0x1f0
> acpi_pci_irq_enable+0x72/0x1e0
> ? pci_read_config_word+0x2e/0x30
> pcibios_enable_device+0x2e/0x40
> do_pci_enable_device+0x5c/0x100
> pci_enable_device_flags+0xe0/0x130
> pci_enable_device+0xe/0x10
> e1000_probe+0xd2/0xfc0 [e1000
> ? trace_hardirqs_on+0x3f/0x110
> local_pci_probe+0x41/0x90
> pci_device_probe+0x14c/0x1b0
> really_probe+0x1d4/0x2d0
> driver_probe_device+0x50/0xf0
> device_driver_attach+0x54/0x60
> __driver_attach+0x7e/0xd0
> ? device_driver_attach+0x60/0x60
> bus_for_each_dev+0x68/0xc0
> driver_attach+0x19/0x20
> bus_add_driver+0x15e/0x200
> driver_register+0x5b/0xf0
> __pci_register_driver+0x66/0x70
> ? 0xffffffffa0179000
> e1000_init_module+0x50/0x1000 [e1000
> ? 0xffffffffa0179000
> do_one_initcall+0x6c/0x3cc
> ? do_init_module+0x22/0x207
> ? rcu_read_lock_sched_held+0x97/0xb0
> ? kmem_cache_alloc_trace+0x325/0x3b0
> do_init_module+0x5b/0x207
> load_module+0x1e34/0x2560
> ? m_show+0x1d0/0x1d0
> __do_sys_finit_module+0xc5/0xd0
> __x64_sys_finit_module+0x15/0x20
> do_syscall_64+0x6b/0x1d0
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> In acpi_ut_copy_iobject_to_iobject, if
> acpi_ut_copy_ipackage_to_ipackage failed with AE_NO_MEMORY,
> acpi_ut_remove_reference will be called and in which calls
> acpi_ut_update_object_reference, then it try to dereference 'object-
> >package.elements[i]'
> which trigger NULL pointer dereference.
>
> Reported-by: Hulk Robot <hulkci(a)huawei.com>
> Fixes: 8aa5e56eeb61 ("ACPICA: Utilities: Fix memory leak in
> acpi_ut_copy_iobject_to_iobject")
> Signed-off-by: YueHaibing <yuehaibing(a)huawei.com>
> ---
> drivers/acpi/acpica/utcopy.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/acpica/utcopy.c b/drivers/acpi/acpica/utcopy.c index
> 1fb8327..038d518 100644
> --- a/drivers/acpi/acpica/utcopy.c
> +++ b/drivers/acpi/acpica/utcopy.c
> @@ -895,7 +895,6 @@
>
> dest_obj->common.type = source_obj->common.type;
> dest_obj->common.flags = source_obj->common.flags;
> - dest_obj->package.count = source_obj->package.count;
>
> /*
> * Create the object array and walk the source package tree @@ -
> 909,6 +908,8 @@
> return_ACPI_STATUS(AE_NO_MEMORY);
> }
>
> + dest_obj->package.count = source_obj->package.count;
> +
> /*
> * Copy the package element-by-element by walking the package
> "tree".
> * This handles nested packages of arbitrary depth.
> --
> 1.8.3.1
>
Please provide the acpidump as well as the dmesg
Thanks,
Erik
3 years, 4 months
iasl -d and duplicate symbols (ACPI Error: AE_ALREADY_EXISTS)
by Anthony Jenkins
Hi all,
I am trying to patch the ACPI tables on my Dell XPS 15 9570 running
FreeBSD 13.0-CURRENT @ git commit 68c8581f772. 'acpidump -d -t' gives
error AE_ALREADY_EXISTS when trying to add symbol
\_SB.PCI0.XHC.RHUB.HS01._UPC.?? Google says this is because my BIOS' set
of ACPI tables contains two duplicate tables, and it fails to add
symbols from the 2nd table because they already exist from the 1st.
Q: By "duplicate table", does this mean the entire body of the table
(excluding its header) is duplicated?
What's the standard practice for handling this error? I assume I have to:
1. Identify the two duplicate tables
2. Tell 'iasl -d' (which is what 'acpidump' calls to do the
disassembly) to exclude one of the two tables
I have no idea how to do either of these two tasks with the set of
acpica tools I have (iasl version 20190108)...anyone have any pointers?
Is there a way to extract (using acpica tools) the body of a single
named ACPI table?
I have some ideas for patches to acpica to help with this increasingly
common issue:
* Patch iasl(1) to emit more information about the origins of the
symbols it parses, such that an AE_ALREADY_EXISTS error would also
emit the origin (table name) of the existing symbol and that of the
current table it's trying to add.
* Patch iasl(1) to add a user option (flag) to ignore tables with
duplicate symbols, possibly adding a parameter indicating number of
duplicates or percentage of duplication before dropping that table.
Would any/all of these patches be useful/considered for acceptance into
acpica?
Thanks in advance,
Anthony Jenkins
3 years, 4 months
Re: [Devel] [PATCH v2] ACPI / device_sysfs: change _ADR representation to 64 bits
by Moore, Robert
> -----Original Message-----
> From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
> Sent: Wednesday, May 1, 2019 5:53 AM
> To: alsa-devel(a)alsa-project.org
> Cc: linux-kernel(a)vger.kernel.org; tiwai(a)suse.de; broonie(a)kernel.org;
> vkoul(a)kernel.org; gregkh(a)linuxfoundation.org;
> liam.r.girdwood(a)linux.intel.com; jank(a)cadence.com; joe(a)perches.com;
> srinivas.kandagatla(a)linaro.org; Pierre-Louis Bossart <pierre-
> louis.bossart(a)linux.intel.com>; Rafael J. Wysocki <rjw(a)rjwysocki.net>;
> Len Brown <lenb(a)kernel.org>; Moore, Robert <robert.moore(a)intel.com>;
> Schmauss, Erik <erik.schmauss(a)intel.com>; open list:ACPI <linux-
> acpi(a)vger.kernel.org>; open list:ACPI COMPONENT ARCHITECTURE (ACPICA)
> <devel(a)acpica.org>
> Subject: [PATCH v2] ACPI / device_sysfs: change _ADR representation to
> 64 bits
>
> Standards such as the MIPI DisCo for SoundWire 1.0 specification assume
> the _ADR field is 64 bits.
>
> _ADR is defined as an "Integer" represented as 64 bits since ACPI 2.0
> released in 2002. The low levels already use _ADR as 64 bits, e.g. in
> struct acpi_device_info.
>
[Moore, Robert]
Just to be precise: since acpi 2.0 the integer width is either 32 bits or 64 bits, depending on the version number of the DSDT (1-->32, 2 or greater --> 64).
> This patch bumps the representation used for sysfs to 64 bits. To avoid
> any compatibility/ABI issues, the printf format is only extended to 16
> characters when the actual _ADR value exceeds the 32 bit maximum.
>
> Example with a SoundWire device, the results show the complete vendorID
> and linkID which were omitted before:
>
> Before:
> $ more /sys/bus/acpi/devices/device\:38/adr
> 0x5d070000
> After:
> $ more /sys/bus/acpi/devices/device\:38/adr
> 0x000010025d070000
>
> Signed-off-by: Pierre-Louis Bossart <pierre-
> louis.bossart(a)linux.intel.com>
> ---
> v2: only use 64 bits when required to avoid compatibility issues
> (feedback from Vinod and Rafael)
>
> drivers/acpi/device_sysfs.c | 6 ++++--
> include/acpi/acpi_bus.h | 2 +-
> 2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
> index 8940054d6250..7dda0ee05cd1 100644
> --- a/drivers/acpi/device_sysfs.c
> +++ b/drivers/acpi/device_sysfs.c
> @@ -428,8 +428,10 @@ static ssize_t acpi_device_adr_show(struct device
> *dev, {
> struct acpi_device *acpi_dev = to_acpi_device(dev);
>
> - return sprintf(buf, "0x%08x\n",
> - (unsigned int)(acpi_dev->pnp.bus_address));
> + if (acpi_dev->pnp.bus_address > 0xFFFFFFFF)
> + return sprintf(buf, "0x%016llx\n", acpi_dev-
> >pnp.bus_address);
> + else
> + return sprintf(buf, "0x%08llx\n", acpi_dev->pnp.bus_address);
> }
> static DEVICE_ATTR(adr, 0444, acpi_device_adr_show, NULL);
>
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index
> f7981751ac77..9075e28ea60a 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -230,7 +230,7 @@ struct acpi_device_dir {
> /* Plug and Play */
>
> typedef char acpi_bus_id[8];
> -typedef unsigned long acpi_bus_address;
> +typedef u64 acpi_bus_address;
> typedef char acpi_device_name[40];
> typedef char acpi_device_class[20];
>
> --
> 2.17.1
3 years, 4 months
Re: [Devel] [alsa-devel] [PATCH] ACPI / device_sysfs: change _ADR representation to 64 bits
by Rafael J. Wysocki
On Tue, Apr 30, 2019 at 8:23 PM Pierre-Louis Bossart
<pierre-louis.bossart(a)linux.intel.com> wrote:
>
>
>
> On 4/16/19 3:09 AM, Rafael J. Wysocki wrote:
> > On Tue, Apr 16, 2019 at 5:29 AM Vinod Koul <vkoul(a)kernel.org> wrote:
> >>
> >> On 15-04-19, 10:18, Pierre-Louis Bossart wrote:
> >>> Standards such as the MIPI DisCo for SoundWire 1.0 specification
> >>> assume the _ADR field is 64 bits.
> >>>
> >>> _ADR is defined as an "Integer" represented as 64 bits since ACPI 2.0
> >>> released in 2002. The low levels already use _ADR as 64 bits, e.g. in
> >>> struct acpi_device_info.
> >>>
> >>> This patch bumps the representation used for sysfs to 64 bits.
> >>>
> >>> Example with a SoundWire device, the results show the complete
> >>> vendorID and linkID which were omitted before:
> >>>
> >>> Before:
> >>> $ more /sys/bus/acpi/devices/device\:38/adr
> >>> 0x5d070000
> >>> After:
> >>> $ more /sys/bus/acpi/devices/device\:38/adr
> >>> 0x000010025d070000
> >>
> >> This looks fine but the sysfs file is an ABI. Not sure if we can modify
> >> the value returned this way.. Though it should not cause userspace
> >> reading 32bits to break...
> >
> > Well, IIRC using "08" instead of "016" in the format field would
> > preserve the existing behavior for 32-bit values, wouldn't it?
>
> yes, but it makes the 64-bit address not aligned depending on the number
> of leading zeroes, see below. I get a migraine just looking at the results.
Well, scripts reading them won't get that, but fair enough.
> Maybe add a test to use 08 for values that are below 0xFFFFFFFF and 16
> for addresses who really need the full range, typically because of an
> encoding?
That would be fine by me.
3 years, 5 months