* [PATCH] platform/chrome: chromeos_acpi: print hex string for ACPI_TYPE_BUFFER
@ 2023-07-26 7:31 Tzung-Bi Shih
2023-08-01 3:32 ` Tzung-Bi Shih
0 siblings, 1 reply; 6+ messages in thread
From: Tzung-Bi Shih @ 2023-07-26 7:31 UTC (permalink / raw)
To: bleung, groeck
Cc: chrome-platform, guillaume.tucker, denys.f, ricardo.canuelo,
tzungbi
`element->buffer.pointer` should be binary blob. `%s` doesn't work perfect
for them.
Print hex string for ACPI_TYPE_BUFFER.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/platform/chrome/chromeos_acpi.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/chrome/chromeos_acpi.c b/drivers/platform/chrome/chromeos_acpi.c
index 50d8a4d4352d..a60d344c54e4 100644
--- a/drivers/platform/chrome/chromeos_acpi.c
+++ b/drivers/platform/chrome/chromeos_acpi.c
@@ -90,7 +90,25 @@ static int chromeos_acpi_handle_package(struct device *dev, union acpi_object *o
case ACPI_TYPE_STRING:
return sysfs_emit(buf, "%s\n", element->string.pointer);
case ACPI_TYPE_BUFFER:
- return sysfs_emit(buf, "%s\n", element->buffer.pointer);
+ {
+ int i, n;
+ const int char_per_line = 16;
+
+ for (i = 0, n = 0; i < element->buffer.length; ++i) {
+ n += sysfs_emit_at(buf, n, " %2.2x", element->buffer.pointer[i]);
+
+ if (((i + 1) % char_per_line) == 0)
+ n += sysfs_emit_at(buf, n, "\n");
+ }
+ n += sysfs_emit_at(buf, n, "\n");
+
+ if (n == PAGE_SIZE - 1) {
+ dev_info(dev, "truncating sysfs content for %s\n", name);
+ sysfs_emit_at(buf, n - 3, "..\n");
+ }
+
+ return n;
+ }
default:
dev_err(dev, "element type %d not supported\n", element->type);
return -EINVAL;
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] platform/chrome: chromeos_acpi: print hex string for ACPI_TYPE_BUFFER
2023-07-26 7:31 [PATCH] platform/chrome: chromeos_acpi: print hex string for ACPI_TYPE_BUFFER Tzung-Bi Shih
@ 2023-08-01 3:32 ` Tzung-Bi Shih
2023-08-01 21:53 ` Brian Norris
0 siblings, 1 reply; 6+ messages in thread
From: Tzung-Bi Shih @ 2023-08-01 3:32 UTC (permalink / raw)
To: bleung, groeck
Cc: chrome-platform, guillaume.tucker, denys.f, ricardo.canuelo,
usama.anjum
On Wed, Jul 26, 2023 at 03:31:27PM +0800, Tzung-Bi Shih wrote:
> `element->buffer.pointer` should be binary blob. `%s` doesn't work perfect
> for them.
>
> Print hex string for ACPI_TYPE_BUFFER.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
Curious about does this patch make sense to folks on the mailing list?
> ---
> drivers/platform/chrome/chromeos_acpi.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/chrome/chromeos_acpi.c b/drivers/platform/chrome/chromeos_acpi.c
> index 50d8a4d4352d..a60d344c54e4 100644
> --- a/drivers/platform/chrome/chromeos_acpi.c
> +++ b/drivers/platform/chrome/chromeos_acpi.c
> @@ -90,7 +90,25 @@ static int chromeos_acpi_handle_package(struct device *dev, union acpi_object *o
> case ACPI_TYPE_STRING:
> return sysfs_emit(buf, "%s\n", element->string.pointer);
> case ACPI_TYPE_BUFFER:
> - return sysfs_emit(buf, "%s\n", element->buffer.pointer);
> + {
> + int i, n;
> + const int char_per_line = 16;
> +
> + for (i = 0, n = 0; i < element->buffer.length; ++i) {
> + n += sysfs_emit_at(buf, n, " %2.2x", element->buffer.pointer[i]);
> +
> + if (((i + 1) % char_per_line) == 0)
> + n += sysfs_emit_at(buf, n, "\n");
> + }
> + n += sysfs_emit_at(buf, n, "\n");
> +
> + if (n == PAGE_SIZE - 1) {
> + dev_info(dev, "truncating sysfs content for %s\n", name);
> + sysfs_emit_at(buf, n - 3, "..\n");
> + }
> +
> + return n;
> + }
> default:
> dev_err(dev, "element type %d not supported\n", element->type);
> return -EINVAL;
> --
> 2.41.0.487.g6d72f3e995-goog
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] platform/chrome: chromeos_acpi: print hex string for ACPI_TYPE_BUFFER
2023-08-01 3:32 ` Tzung-Bi Shih
@ 2023-08-01 21:53 ` Brian Norris
2023-08-02 10:06 ` Tzung-Bi Shih
0 siblings, 1 reply; 6+ messages in thread
From: Brian Norris @ 2023-08-01 21:53 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: bleung, groeck, chrome-platform, guillaume.tucker, denys.f,
ricardo.canuelo, usama.anjum
On Tue, Aug 01, 2023 at 11:32:40AM +0800, Tzung-Bi Shih wrote:
> On Wed, Jul 26, 2023 at 03:31:27PM +0800, Tzung-Bi Shih wrote:
> > `element->buffer.pointer` should be binary blob. `%s` doesn't work perfect
> > for them.
> >
> > Print hex string for ACPI_TYPE_BUFFER.
> >
> > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
>
> Curious about does this patch make sense to folks on the mailing list?
Since you asked.... (sorry if the questions are dumb; I'm not extremely
familiar with this one)
Are there any ABI concerns here? As in, did this perhaps work for some
use cases that will be broken now by this change in format? If not, it
would be nice to note your thought process on that. [1]
Also, is it possible to use some standard formatting, like
hex_dump_to_buffer()?
Or, would it make more sense to make these into binary attributes
(struct bin_attribute)?
NB: the docs (Documentation/ABI/testing/sysfs-driver-chromeos-acpi)
suggest that some attributes (like VDAT) are indeed binary, and so we
should probably maintain or fix that.
Brian
[1] My guess: no real user space actually uses this driver yet, since
ChromiumOS still has its own downstream variant on its active kernels?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] platform/chrome: chromeos_acpi: print hex string for ACPI_TYPE_BUFFER
2023-08-01 21:53 ` Brian Norris
@ 2023-08-02 10:06 ` Tzung-Bi Shih
2023-08-02 18:59 ` Brian Norris
0 siblings, 1 reply; 6+ messages in thread
From: Tzung-Bi Shih @ 2023-08-02 10:06 UTC (permalink / raw)
To: Brian Norris
Cc: bleung, groeck, chrome-platform, guillaume.tucker, denys.f,
ricardo.canuelo, usama.anjum
On Tue, Aug 01, 2023 at 02:53:31PM -0700, Brian Norris wrote:
> Are there any ABI concerns here? As in, did this perhaps work for some
> use cases that will be broken now by this change in format? If not, it
> would be nice to note your thought process on that. [1]
There is an existing use case from userland program[2]. It works with
downstream chromeos_acpi driver but not the upstream one.
[2]: https://crrev.com/48a12071a43813e5bf0694f99e4024468ea900b6/host/arch/x86/lib/crossystem_arch.c#232
> Also, is it possible to use some standard formatting, like
> hex_dump_to_buffer()?
Ack, fix in v2[3].
[3]: https://patchwork.kernel.org/project/chrome-platform/patch/20230802095736.3079963-1-tzungbi@kernel.org/
> Or, would it make more sense to make these into binary attributes
> (struct bin_attribute)?
>
> NB: the docs (Documentation/ABI/testing/sysfs-driver-chromeos-acpi)
> suggest that some attributes (like VDAT) are indeed binary, and so we
> should probably maintain or fix that.
No. Unless we also want to change userland program in [2].
> [1] My guess: no real user space actually uses this driver yet, since
> ChromiumOS still has its own downstream variant on its active kernels?
Correct, AFAIK, there has no use cases for the upstream chromeos_acpi driver
yet.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] platform/chrome: chromeos_acpi: print hex string for ACPI_TYPE_BUFFER
2023-08-02 10:06 ` Tzung-Bi Shih
@ 2023-08-02 18:59 ` Brian Norris
2023-08-03 1:18 ` Tzung-Bi Shih
0 siblings, 1 reply; 6+ messages in thread
From: Brian Norris @ 2023-08-02 18:59 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: bleung, groeck, chrome-platform, guillaume.tucker, denys.f,
ricardo.canuelo, usama.anjum
On Wed, Aug 02, 2023 at 06:06:30PM +0800, Tzung-Bi Shih wrote:
> On Tue, Aug 01, 2023 at 02:53:31PM -0700, Brian Norris wrote:
> > NB: the docs (Documentation/ABI/testing/sysfs-driver-chromeos-acpi)
> > suggest that some attributes (like VDAT) are indeed binary, and so we
> > should probably maintain or fix that.
>
> No. Unless we also want to change userland program in [2].
I mean, I think you need to do one of those two -- either keep this
(maintain) as "binary", or update (fix) the doc to describe the hex dump
instead.
i.e., I think this piece of
Documentation/ABI/testing/sysfs-driver-chromeos-acpi is now wrong:
What: /sys/bus/platform/devices/GGL0001:*/VDAT
Date: May 2022
KernelVersion: 5.19
Description:
Returns the verified boot data block shared between the
firmware verification step and the kernel verification step
(binary).
But, the userland program already doesn't know how to handle the
upstream driver. So it'd be possible to change both at the same time.
Anyway, hex-encoded is fine with me too; we just need to get the docs to
match.
Regards,
Brian
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] platform/chrome: chromeos_acpi: print hex string for ACPI_TYPE_BUFFER
2023-08-02 18:59 ` Brian Norris
@ 2023-08-03 1:18 ` Tzung-Bi Shih
0 siblings, 0 replies; 6+ messages in thread
From: Tzung-Bi Shih @ 2023-08-03 1:18 UTC (permalink / raw)
To: Brian Norris
Cc: bleung, groeck, chrome-platform, guillaume.tucker, denys.f,
ricardo.canuelo, usama.anjum
On Wed, Aug 02, 2023 at 11:59:59AM -0700, Brian Norris wrote:
> On Wed, Aug 02, 2023 at 06:06:30PM +0800, Tzung-Bi Shih wrote:
> > On Tue, Aug 01, 2023 at 02:53:31PM -0700, Brian Norris wrote:
> > > NB: the docs (Documentation/ABI/testing/sysfs-driver-chromeos-acpi)
> > > suggest that some attributes (like VDAT) are indeed binary, and so we
> > > should probably maintain or fix that.
> >
> > No. Unless we also want to change userland program in [2].
>
> I mean, I think you need to do one of those two -- either keep this
> (maintain) as "binary", or update (fix) the doc to describe the hex dump
> instead.
>
> i.e., I think this piece of
> Documentation/ABI/testing/sysfs-driver-chromeos-acpi is now wrong:
>
> What: /sys/bus/platform/devices/GGL0001:*/VDAT
> Date: May 2022
> KernelVersion: 5.19
> Description:
> Returns the verified boot data block shared between the
> firmware verification step and the kernel verification step
> (binary).
>
> But, the userland program already doesn't know how to handle the
> upstream driver. So it'd be possible to change both at the same time.
>
> Anyway, hex-encoded is fine with me too; we just need to get the docs to
> match.
Ack, fix in v3 (https://patchwork.kernel.org/project/chrome-platform/patch/20230803011245.3773756-1-tzungbi@kernel.org/).
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-08-03 1:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-26 7:31 [PATCH] platform/chrome: chromeos_acpi: print hex string for ACPI_TYPE_BUFFER Tzung-Bi Shih
2023-08-01 3:32 ` Tzung-Bi Shih
2023-08-01 21:53 ` Brian Norris
2023-08-02 10:06 ` Tzung-Bi Shih
2023-08-02 18:59 ` Brian Norris
2023-08-03 1:18 ` Tzung-Bi Shih
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox