| Message ID | 20230302071327.557734-1-sathyanarayanan.kuppuswamy@linux.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v1] ACPI: sysfs: Enable ACPI sysfs support for CCEL records | expand |
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 <haibo1.xu@intel.com> > [Original patch is for TDEL table, modified it for CCEL support] > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > --- > 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)
On Thu, Mar 2, 2023 at 8:13 AM Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> 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 <haibo1.xu@intel.com> > [Original patch is for TDEL table, modified it for CCEL support] > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > --- > 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) > --
Hi Rafael, On 3/20/23 10:15 AM, Rafael J. Wysocki wrote: > On Thu, Mar 2, 2023 at 8:13 AM Kuppuswamy Sathyanarayanan > <sathyanarayanan.kuppuswamy@linux.intel.com> 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 <haibo1.xu@intel.com> >> [Original patch is for TDEL table, modified it for CCEL support] >> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> >> --- >> 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) >> --
On Mon, Mar 20, 2023 at 6:34 PM Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@linux.intel.com> 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 > > <sathyanarayanan.kuppuswamy@linux.intel.com> 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 <haibo1.xu@intel.com> > >> [Original patch is for TDEL table, modified it for CCEL support] > >> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > >> --- > >> 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) > >> -- > > --
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)
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 <haibo1.xu@intel.com> [Original patch is for TDEL table, modified it for CCEL support] Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> --- drivers/acpi/sysfs.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)