On Thu, 2 Feb 2023 11:03:47 +0100 "Rafael J. Wysocki" <rafael@xxxxxxxxxx> wrote: > On Thu, Feb 2, 2023 at 8:50 AM Sebastian Grzywna > <swiftgeek@xxxxxxxxx> wrote: > > > > Dnia 2023-02-01, o godz. 19:34:48 > > "Rafael J. Wysocki" <rafael@xxxxxxxxxx> napisał(a): > > > > > On Wed, Feb 1, 2023 at 12:38 AM Pedro Falcato > > > <pedro.falcato@xxxxxxxxx> wrote: > > > > > > > > Make custom_method keep its own per-file-open state instead of > > > > global state in order to avoid race conditions[1] and other > > > > possible conflicts with other concurrent users. > > > > > > > > Link: > > > > https://lore.kernel.org/linux-acpi/20221227063335.61474-1-zh.nvgt@xxxxxxxxx/ > > > > # [1] Reported-by: Hang Zhang <zh.nvgt@xxxxxxxxx> Cc: Swift Geek > > > > <swiftgeek@xxxxxxxxx> Signed-off-by: Pedro Falcato > > > > <pedro.falcato@xxxxxxxxx> --- > > > > This patch addresses Hang's problems plus the ones raised by > > > > Rafael in his review (see link above). > > > > https://lore.kernel.org/lkml/2667007.mvXUDI8C0e@kreacher/ was > > > > submitted but since there were still people that wanted this > > > > feature, I took my time to write up a patch that should fix the > > > > issues. Hopefully the linux-acpi maintainers have not decided to > > > > remove custom_method just yet. > > > > > > Well, thanks for the patch, but yes, they have. Sorry. > > > > Hi Rafael, > > Can you please explain why you don't want to keep it, given there's > > a patch? > > Because this interface was a bad idea to start with and its > implementation is questionable at the design level. > > Granted, at the time it was introduced, there was no alternative, but > there is the AML debugger in the kernel now and as far as debugging is > concerned, it is actually more powerful than custom_metod AFAICS. See > Documentation/firmware-guide/acpi/aml-debugger.rst. > > If the AML debugger has problems, I would very much prefer fixing them > to the perpetual maintenance of custom_method. > > > I find it really useful in my day-to-day as a firmware engineer. > > I don't see much happening in git history of > > drivers/acpi/custom_method.c , and I don't see anything that was > > specifically changed in it in past 10 years to keep it being > > functional. Without your more detailed explanation I have hard time > > understanding your decision to remove it, since I'm not a kernel > > developer myself. > > It's been always conceptually questionable, problematic from the > security standpoint and implemented poorly. Also its documentation is > outdated. > > The patches fixing its most apparent functional issues don't actually > address much of the above. > > The AML debugger should really be used for debug rather than > custom_method and honestly, what's the purpose of it beyond debug? I tried to investigate how to get my workflow going with tools/power/acpi/acpidbg , and after looking at drivers/acpi/acpica/dbinput.c I am a bit confused as to how should I enable CMD_LOAD properly, and if it's not in Kconfig then how supported/tested is this feature. acpica-reference_19.pdf is also confusing me a bit with mention of "Unloading the DSDT is not allowed", making me worry that live reloading of DSDT would not be possible (which would be superset of overriding just a single method). I want to be able to swap a particular method, and have it run as close to normally as possible, sometimes for prolonged period of time, but most of the time I will fail at writing a patch and will need to swap it again with another attempted fix.
