Subject: [PATCH v1] ACPI: sysfs: Enable ACPI sysfs support for CCEL records

The Confidential Computing Event Log (CCEL) table provides the address
and length of the CCEL records area in UEFI reserved memory. To access
these records, userspace can use /dev/mem to retrieve them. But
'/dev/mem' is not enabled on many systems for security reasons.

So to allow user space access these event log records without the
/dev/mem interface, add support to access it via sysfs interface. The
ACPI driver has provided read only access to BERT records area via
'/sys/firmware/acpi/tables/data/BERT' in sysfs. So follow the same way,
and add support for /sys/firmware/acpi/tables/data/CCEL to enable
read-only access to the CCEL recorids area.

More details about the CCEL table can be found in ACPI specification
r6.5, sec titled "CC Event Log ACPI Table".

Original-patch-by: Haibo Xu <[email protected]>
[Original patch is for TDEL table, modified it for CCEL support]
Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
---
drivers/acpi/sysfs.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
index 7db3b530279b..afeac925b31b 100644
--- a/drivers/acpi/sysfs.c
+++ b/drivers/acpi/sysfs.c
@@ -458,11 +458,28 @@ static int acpi_bert_data_init(void *th, struct acpi_data_attr *data_attr)
return sysfs_create_bin_file(tables_data_kobj, &data_attr->attr);
}

+static int acpi_ccel_data_init(void *th, struct acpi_data_attr *data_attr)
+{
+ struct acpi_table_ccel *ccel = th;
+
+ if (ccel->header.length < sizeof(struct acpi_table_ccel) ||
+ !(ccel->log_area_start_address) || !(ccel->log_area_minimum_length)) {
+ kfree(data_attr);
+ return -EINVAL;
+ }
+ data_attr->addr = ccel->log_area_start_address;
+ data_attr->attr.size = ccel->log_area_minimum_length;
+ data_attr->attr.attr.name = "CCEL";
+
+ return sysfs_create_bin_file(tables_data_kobj, &data_attr->attr);
+}
+
static struct acpi_data_obj {
char *name;
int (*fn)(void *, struct acpi_data_attr *);
} acpi_data_objs[] = {
{ ACPI_SIG_BERT, acpi_bert_data_init },
+ { ACPI_SIG_CCEL, acpi_ccel_data_init },
};

#define NUM_ACPI_DATA_OBJS ARRAY_SIZE(acpi_data_objs)
--
2.34.1



Subject: Re: [PATCH v1] ACPI: sysfs: Enable ACPI sysfs support for CCEL records

Hi Rafael,

Gentle ping! Any comments on this patch?

On 3/1/23 11:13 PM, Kuppuswamy Sathyanarayanan wrote:
> The Confidential Computing Event Log (CCEL) table provides the address
> and length of the CCEL records area in UEFI reserved memory. To access
> these records, userspace can use /dev/mem to retrieve them. But
> '/dev/mem' is not enabled on many systems for security reasons.
>
> So to allow user space access these event log records without the
> /dev/mem interface, add support to access it via sysfs interface. The
> ACPI driver has provided read only access to BERT records area via
> '/sys/firmware/acpi/tables/data/BERT' in sysfs. So follow the same way,
> and add support for /sys/firmware/acpi/tables/data/CCEL to enable
> read-only access to the CCEL recorids area.
>
> More details about the CCEL table can be found in ACPI specification
> r6.5, sec titled "CC Event Log ACPI Table".
>
> Original-patch-by: Haibo Xu <[email protected]>
> [Original patch is for TDEL table, modified it for CCEL support]
> Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
> ---
> drivers/acpi/sysfs.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
> index 7db3b530279b..afeac925b31b 100644
> --- a/drivers/acpi/sysfs.c
> +++ b/drivers/acpi/sysfs.c
> @@ -458,11 +458,28 @@ static int acpi_bert_data_init(void *th, struct acpi_data_attr *data_attr)
> return sysfs_create_bin_file(tables_data_kobj, &data_attr->attr);
> }
>
> +static int acpi_ccel_data_init(void *th, struct acpi_data_attr *data_attr)
> +{
> + struct acpi_table_ccel *ccel = th;
> +
> + if (ccel->header.length < sizeof(struct acpi_table_ccel) ||
> + !(ccel->log_area_start_address) || !(ccel->log_area_minimum_length)) {
> + kfree(data_attr);
> + return -EINVAL;
> + }
> + data_attr->addr = ccel->log_area_start_address;
> + data_attr->attr.size = ccel->log_area_minimum_length;
> + data_attr->attr.attr.name = "CCEL";
> +
> + return sysfs_create_bin_file(tables_data_kobj, &data_attr->attr);
> +}
> +
> static struct acpi_data_obj {
> char *name;
> int (*fn)(void *, struct acpi_data_attr *);
> } acpi_data_objs[] = {
> { ACPI_SIG_BERT, acpi_bert_data_init },
> + { ACPI_SIG_CCEL, acpi_ccel_data_init },
> };
>
> #define NUM_ACPI_DATA_OBJS ARRAY_SIZE(acpi_data_objs)

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

2023-03-20 17:21:11

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v1] ACPI: sysfs: Enable ACPI sysfs support for CCEL records

On Thu, Mar 2, 2023 at 8:13 AM Kuppuswamy Sathyanarayanan
<[email protected]> wrote:
>
> The Confidential Computing Event Log (CCEL) table provides the address
> and length of the CCEL records area in UEFI reserved memory. To access
> these records, userspace can use /dev/mem to retrieve them. But
> '/dev/mem' is not enabled on many systems for security reasons.
>
> So to allow user space access these event log records without the
> /dev/mem interface, add support to access it via sysfs interface. The
> ACPI driver has provided read only access to BERT records area via
> '/sys/firmware/acpi/tables/data/BERT' in sysfs. So follow the same way,
> and add support for /sys/firmware/acpi/tables/data/CCEL to enable
> read-only access to the CCEL recorids area.
>
> More details about the CCEL table can be found in ACPI specification
> r6.5, sec titled "CC Event Log ACPI Table".
>
> Original-patch-by: Haibo Xu <[email protected]>
> [Original patch is for TDEL table, modified it for CCEL support]
> Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
> ---
> drivers/acpi/sysfs.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
> index 7db3b530279b..afeac925b31b 100644
> --- a/drivers/acpi/sysfs.c
> +++ b/drivers/acpi/sysfs.c
> @@ -458,11 +458,28 @@ static int acpi_bert_data_init(void *th, struct acpi_data_attr *data_attr)
> return sysfs_create_bin_file(tables_data_kobj, &data_attr->attr);
> }
>
> +static int acpi_ccel_data_init(void *th, struct acpi_data_attr *data_attr)
> +{
> + struct acpi_table_ccel *ccel = th;
> +
> + if (ccel->header.length < sizeof(struct acpi_table_ccel) ||
> + !(ccel->log_area_start_address) || !(ccel->log_area_minimum_length)) {

The inner parens in this line are not necessary AFAICS.

Otherwise I have no objections.

> + kfree(data_attr);
> + return -EINVAL;
> + }
> + data_attr->addr = ccel->log_area_start_address;
> + data_attr->attr.size = ccel->log_area_minimum_length;
> + data_attr->attr.attr.name = "CCEL";
> +
> + return sysfs_create_bin_file(tables_data_kobj, &data_attr->attr);
> +}
> +
> static struct acpi_data_obj {
> char *name;
> int (*fn)(void *, struct acpi_data_attr *);
> } acpi_data_objs[] = {
> { ACPI_SIG_BERT, acpi_bert_data_init },
> + { ACPI_SIG_CCEL, acpi_ccel_data_init },
> };
>
> #define NUM_ACPI_DATA_OBJS ARRAY_SIZE(acpi_data_objs)
> --

2023-03-20 17:40:38

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v1] ACPI: sysfs: Enable ACPI sysfs support for CCEL records

On Mon, Mar 20, 2023 at 6:34 PM Sathyanarayanan Kuppuswamy
<[email protected]> wrote:
>
> Hi Rafael,
>
> On 3/20/23 10:15 AM, Rafael J. Wysocki wrote:
> > On Thu, Mar 2, 2023 at 8:13 AM Kuppuswamy Sathyanarayanan
> > <[email protected]> wrote:
> >>
> >> The Confidential Computing Event Log (CCEL) table provides the address
> >> and length of the CCEL records area in UEFI reserved memory. To access
> >> these records, userspace can use /dev/mem to retrieve them. But
> >> '/dev/mem' is not enabled on many systems for security reasons.
> >>
> >> So to allow user space access these event log records without the
> >> /dev/mem interface, add support to access it via sysfs interface. The
> >> ACPI driver has provided read only access to BERT records area via
> >> '/sys/firmware/acpi/tables/data/BERT' in sysfs. So follow the same way,
> >> and add support for /sys/firmware/acpi/tables/data/CCEL to enable
> >> read-only access to the CCEL recorids area.
> >>
> >> More details about the CCEL table can be found in ACPI specification
> >> r6.5, sec titled "CC Event Log ACPI Table".
> >>
> >> Original-patch-by: Haibo Xu <[email protected]>
> >> [Original patch is for TDEL table, modified it for CCEL support]
> >> Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
> >> ---
> >> drivers/acpi/sysfs.c | 17 +++++++++++++++++
> >> 1 file changed, 17 insertions(+)
> >>
> >> diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
> >> index 7db3b530279b..afeac925b31b 100644
> >> --- a/drivers/acpi/sysfs.c
> >> +++ b/drivers/acpi/sysfs.c
> >> @@ -458,11 +458,28 @@ static int acpi_bert_data_init(void *th, struct acpi_data_attr *data_attr)
> >> return sysfs_create_bin_file(tables_data_kobj, &data_attr->attr);
> >> }
> >>
> >> +static int acpi_ccel_data_init(void *th, struct acpi_data_attr *data_attr)
> >> +{
> >> + struct acpi_table_ccel *ccel = th;
> >> +
> >> + if (ccel->header.length < sizeof(struct acpi_table_ccel) ||
> >> + !(ccel->log_area_start_address) || !(ccel->log_area_minimum_length)) {
> >
> > The inner parens in this line are not necessary AFAICS.
> >
> > Otherwise I have no objections.
>
> Yes. We can do without it. Shall I submit v2 with this change, or you want to
> fix it when applying?

I would appreciate a v2.

> >
> >> + kfree(data_attr);
> >> + return -EINVAL;
> >> + }
> >> + data_attr->addr = ccel->log_area_start_address;
> >> + data_attr->attr.size = ccel->log_area_minimum_length;
> >> + data_attr->attr.attr.name = "CCEL";
> >> +
> >> + return sysfs_create_bin_file(tables_data_kobj, &data_attr->attr);
> >> +}
> >> +
> >> static struct acpi_data_obj {
> >> char *name;
> >> int (*fn)(void *, struct acpi_data_attr *);
> >> } acpi_data_objs[] = {
> >> { ACPI_SIG_BERT, acpi_bert_data_init },
> >> + { ACPI_SIG_CCEL, acpi_ccel_data_init },
> >> };
> >>
> >> #define NUM_ACPI_DATA_OBJS ARRAY_SIZE(acpi_data_objs)
> >> --
>
> --

Subject: Re: [PATCH v1] ACPI: sysfs: Enable ACPI sysfs support for CCEL records

Hi Rafael,

On 3/20/23 10:15 AM, Rafael J. Wysocki wrote:
> On Thu, Mar 2, 2023 at 8:13 AM Kuppuswamy Sathyanarayanan
> <[email protected]> wrote:
>>
>> The Confidential Computing Event Log (CCEL) table provides the address
>> and length of the CCEL records area in UEFI reserved memory. To access
>> these records, userspace can use /dev/mem to retrieve them. But
>> '/dev/mem' is not enabled on many systems for security reasons.
>>
>> So to allow user space access these event log records without the
>> /dev/mem interface, add support to access it via sysfs interface. The
>> ACPI driver has provided read only access to BERT records area via
>> '/sys/firmware/acpi/tables/data/BERT' in sysfs. So follow the same way,
>> and add support for /sys/firmware/acpi/tables/data/CCEL to enable
>> read-only access to the CCEL recorids area.
>>
>> More details about the CCEL table can be found in ACPI specification
>> r6.5, sec titled "CC Event Log ACPI Table".
>>
>> Original-patch-by: Haibo Xu <[email protected]>
>> [Original patch is for TDEL table, modified it for CCEL support]
>> Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
>> ---
>> drivers/acpi/sysfs.c | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
>> index 7db3b530279b..afeac925b31b 100644
>> --- a/drivers/acpi/sysfs.c
>> +++ b/drivers/acpi/sysfs.c
>> @@ -458,11 +458,28 @@ static int acpi_bert_data_init(void *th, struct acpi_data_attr *data_attr)
>> return sysfs_create_bin_file(tables_data_kobj, &data_attr->attr);
>> }
>>
>> +static int acpi_ccel_data_init(void *th, struct acpi_data_attr *data_attr)
>> +{
>> + struct acpi_table_ccel *ccel = th;
>> +
>> + if (ccel->header.length < sizeof(struct acpi_table_ccel) ||
>> + !(ccel->log_area_start_address) || !(ccel->log_area_minimum_length)) {
>
> The inner parens in this line are not necessary AFAICS.
>
> Otherwise I have no objections.

Yes. We can do without it. Shall I submit v2 with this change, or you want to
fix it when applying?

>
>> + kfree(data_attr);
>> + return -EINVAL;
>> + }
>> + data_attr->addr = ccel->log_area_start_address;
>> + data_attr->attr.size = ccel->log_area_minimum_length;
>> + data_attr->attr.attr.name = "CCEL";
>> +
>> + return sysfs_create_bin_file(tables_data_kobj, &data_attr->attr);
>> +}
>> +
>> static struct acpi_data_obj {
>> char *name;
>> int (*fn)(void *, struct acpi_data_attr *);
>> } acpi_data_objs[] = {
>> { ACPI_SIG_BERT, acpi_bert_data_init },
>> + { ACPI_SIG_CCEL, acpi_ccel_data_init },
>> };
>>
>> #define NUM_ACPI_DATA_OBJS ARRAY_SIZE(acpi_data_objs)
>> --

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer