[RFC,4/6] libstb: add opal runtime services for secure boot key management

This patch provides the OPAL runtime service frontend for the host OS to
retrieve secure variables, and append new ones for processing on the
next reboot.

Included are the four minimum required services for basic manipulation:
 - opal_secvar_read()
 - opal_secvar_read_next()
 - opal_secvar_enqueue()
 - opal_secvar_info()

opal_secvar_read() takes in an name string and a vendor buffer (likely a
GUID), and returns the corresponding blob of data if there is a matching
name + vendor pair in the variable bank. This runtime service only
operates on the variable bank.

opal_secvar_read_next() provides a method to iterate through the
variables in the variable bank. If provided a name and vendor that match a
variable in the variable bank, the function will return the name and
vendor of the next variable in the bank in sequence. When provided an
empty string, it will begin from the top of the list. This runtime service
only operates on the variable bank.

opal_secvar_enqueue() provides a method for the host OS to submit a new
secure variable for processing on next boot, by appending it to the
update bank, or "update queue". As this does not affect the variable bank,
appending a variable via this runtime service will not affect the output
of the previous two. The update queue will only ever be processed during
the initialization of skiboot, and is part of a forthcoming patch
series.

opal_secvar_info() has not been implemented yet. It will return
status on the size of, and remaining storage space for secure variables.

Signed-off-by: Eric Richter <erichte@linux.ibm.com>
---
 include/opal-api.h  |   6 +-
 libstb/Makefile.inc |   2 +-
 libstb/secvar_api.c | 203 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 209 insertions(+), 2 deletions(-)
 create mode 100644 libstb/secvar_api.c

On Fri, Mar 29, 2019 at 9:19 AM Eric Richter <erichte@linux.ibm.com> wrote:
>
> This patch provides the OPAL runtime service frontend for the host OS to
> retrieve secure variables, and append new ones for processing on the
> next reboot.
>
> Included are the four minimum required services for basic manipulation:
>  - opal_secvar_read()
>  - opal_secvar_read_next()
>  - opal_secvar_enqueue()
>  - opal_secvar_info()
>
> opal_secvar_read() takes in an name string and a vendor buffer (likely a
> GUID), and returns the corresponding blob of data if there is a matching
> name + vendor pair in the variable bank. This runtime service only
> operates on the variable bank.
>
> opal_secvar_read_next() provides a method to iterate through the
> variables in the variable bank. If provided a name and vendor that match a
> variable in the variable bank, the function will return the name and
> vendor of the next variable in the bank in sequence. When provided an
> empty string, it will begin from the top of the list. This runtime service
> only operates on the variable bank.
>
> opal_secvar_enqueue() provides a method for the host OS to submit a new
> secure variable for processing on next boot, by appending it to the
> update bank, or "update queue". As this does not affect the variable bank,
> appending a variable via this runtime service will not affect the output
> of the previous two. The update queue will only ever be processed during
> the initialization of skiboot, and is part of a forthcoming patch
> series.
>
> opal_secvar_info() has not been implemented yet. It will return
> status on the size of, and remaining storage space for secure variables.
>
> Signed-off-by: Eric Richter <erichte@linux.ibm.com>
> ---
>  include/opal-api.h  |   6 +-
>  libstb/Makefile.inc |   2 +-
>  libstb/secvar_api.c | 203 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 209 insertions(+), 2 deletions(-)
>  create mode 100644 libstb/secvar_api.c
>
> diff --git a/include/opal-api.h b/include/opal-api.h
> index 73f86f9a..8e398073 100644
> --- a/include/opal-api.h
> +++ b/include/opal-api.h
> @@ -226,7 +226,11 @@
>  #define OPAL_NX_COPROC_INIT                    167
>  #define OPAL_NPU_SET_RELAXED_ORDER             168
>  #define OPAL_NPU_GET_RELAXED_ORDER             169
> -#define OPAL_LAST                              169
> +#define OPAL_SECVAR_GET                                170
> +#define OPAL_SECVAR_GET_NEXT                   171

> +#define OPAL_SECVAR_ENQUEUE                    172

Call it ENQUEUE_UPDATE or something. This is a bit vague.

> +#define OPAL_SECVAR_INFO                       173
> +#define OPAL_LAST                              173
>
>  #define QUIESCE_HOLD                   1 /* Spin all calls at entry */
>  #define QUIESCE_REJECT                 2 /* Fail all calls with OPAL_BUSY */
> diff --git a/libstb/Makefile.inc b/libstb/Makefile.inc
> index 2bc9e91b..f3a7e716 100644
> --- a/libstb/Makefile.inc
> +++ b/libstb/Makefile.inc
> @@ -4,7 +4,7 @@ LIBSTB_DIR = libstb
>
>  SUBDIRS += $(LIBSTB_DIR)
>
> -LIBSTB_SRCS = container.c tpm_chip.c cvc.c secureboot.c trustedboot.c secvar.c
> +LIBSTB_SRCS = container.c tpm_chip.c cvc.c secureboot.c trustedboot.c secvar.c secvar_api.c
>  LIBSTB_OBJS = $(LIBSTB_SRCS:%.c=%.o)
>  LIBSTB = $(LIBSTB_DIR)/built-in.a
>
> diff --git a/libstb/secvar_api.c b/libstb/secvar_api.c
> new file mode 100644
> index 00000000..c5de3bdd
> --- /dev/null
> +++ b/libstb/secvar_api.c
> @@ -0,0 +1,203 @@
> +/* Copyright 2019 IBM Corp.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + *      http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> + * implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#ifndef pr_fmt
> +#define pr_fmt(fmt) "SECVAR_API: " fmt
> +#endif
> +
> +#include <opal.h>
> +#include "secvar.h"
> +
> +
> +static int64_t opal_secvar_get(uint64_t k_name, uint64_t k_vendor, uint64_t k_attributes, uint64_t k_data_size, uint64_t k_data)
> +{
> +       struct secvar *var;
> +
> +       char16_t *name = (char16_t *) k_name;
> +       guid_t *vendor = (guid_t *) k_vendor;
> +       uint32_t *attributes = (uint32_t *) k_attributes;       // TODO: make sure this size is correct
> +       size_t *data_size = (size_t *) k_data_size;
> +       void *data = (void *) k_data;
> +
> +       if (!secvar_enabled)
> +               return OPAL_HARDWARE;
> +       if (!name)
> +               return OPAL_PARAMETER;
> +       if (!vendor)
> +               return OPAL_PARAMETER;
> +       if (!data_size)
> +               return OPAL_PARAMETER;
> +       if ((*data_size == 0) && (data))        // Data should be NULL when size is zero
> +               return OPAL_PARAMETER;
> +       if ((*data_size) && (!data))
> +               return OPAL_PARAMETER;          // Data can't be NULL with nonzero size
> +       if (str16nlen(name, SECVAR_MAX_NAME_SIZE) <= 0)
> +               return OPAL_PARAMETER;          // Empty or erroneous name is useless
> +
> +       var = find_secvar_by_name_vendor(name, vendor, &variable_bank);
> +
> +       if (!var)
> +               return OPAL_EMPTY;
> +
> +       if (!data) {
> +               *data_size = var->data_size;
> +               return OPAL_SUCCESS; // Size check
> +       }
> +
> +       if (var->data_size > *data_size) {
> +               *data_size = var->data_size;
> +               return OPAL_PARTIAL;
> +       }
> +
> +       memcpy(data, var->data, var->data_size);
> +       *data_size = var->data_size;
> +

> +       if (attributes)
> +               *attributes = var->attributes;

Moving this above the !data check might be a good idea. That would
allow the kernel to query the size and attributes simultaneously.


> +
> +
> +       return OPAL_SUCCESS;
> +}
> +opal_call(OPAL_SECVAR_GET, opal_secvar_get, 5);
> +
> +
> +static int64_t opal_secvar_get_next(uint64_t k_name_size, uint64_t k_name, uint64_t k_vendor)
> +{
> +       struct secvar *var;
> +       int namelen;
> +
> +       size_t *name_size = (size_t *) k_name_size;
> +       char16_t *name = (char16_t *) k_name;
> +       guid_t *vendor = (guid_t *) k_vendor;
> +
> +       if (!secvar_enabled)
> +               return OPAL_HARDWARE;
> +       if (!name_size || (*name_size == 0))
> +               return OPAL_PARAMETER;
> +       if (!name)
> +               return OPAL_PARAMETER;
> +       if (!vendor)
> +               return OPAL_PARAMETER;
> +
> +       namelen = str16nlen(name, SECVAR_MAX_NAME_SIZE);
> +       if (namelen < 0)
> +               return OPAL_PARAMETER;  // Erroneous name buffer
> +
> +       if (namelen != 0) {
> +               // Non-empty string
> +               var = find_secvar_by_name_vendor(name, vendor, &variable_bank);
> +               if (!var)
> +                       return OPAL_PARAMETER;
> +
> +               var = list_next(&variable_bank, var, link);
> +       }
> +       else {
> +               // Empty string was found, returning first entry
> +               var = list_top(&variable_bank, struct secvar, link);
> +       }
> +
> +       if (!var)
> +               return OPAL_EMPTY;
> +
> +       // Return name buffer is too small
> +       namelen = str16nlen(var->name, SECVAR_MAX_NAME_SIZE);
> +       if (namelen <= 0)
> +               return OPAL_HARDWARE; // This should not be reached
> +
> +       if ((namelen*2) > *name_size) {
> +               *name_size = namelen*2;
> +               return OPAL_PARTIAL;
> +       }
> +
> +
> +       memcpy(name, var->name, namelen*2);
> +       memcpy(vendor, var->vendor, SECVAR_VENDOR_SIZE);
> +
> +       return OPAL_SUCCESS;
> +}
> +opal_call(OPAL_SECVAR_GET_NEXT, opal_secvar_get_next, 3);
> +
> +static int64_t opal_secvar_enqueue(uint64_t k_name, uint64_t k_vendor, uint64_t k_attributes, uint64_t k_data_size, uint64_t k_data)
> +{
> +       struct secvar *var;
> +       int namelen;
> +
> +       char16_t *name = (char16_t *) k_name;
> +       guid_t *vendor = (guid_t *) k_vendor;
> +       uint32_t attributes = (uint32_t) k_attributes;
> +       size_t data_size = (size_t) k_data_size;
> +       void *data = (void *) k_data;
> +
> +       if (!secvar_enabled)
> +               return OPAL_HARDWARE;
> +       if (!name)
> +               return OPAL_PARAMETER;
> +       if (!vendor)
> +               return OPAL_PARAMETER;
> +       if (!data)
> +               return OPAL_PARAMETER;

> +       if (data_size == 0)
> +               return OPAL_UNSUPPORTED;        // we don't support variable deletion this way

How does variable deletion work?

> +       if (data_size > SECVAR_MAX_DATA_SIZE)
> +               return OPAL_PARAMETER;
> +
> +       namelen = str16nlen(name, SECVAR_MAX_NAME_SIZE);
> +       if (namelen <= 0)
> +               return OPAL_PARAMETER;          // empty or erroneous name is useless
> +

> +       var = zalloc(sizeof(struct secvar));
> +       if (!var)
> +               return OPAL_NO_MEM;
> +
> +       var->data = malloc(data_size);
> +       if (!var->data) {
> +               free(var);
> +               return OPAL_NO_MEM;
> +       }

You can use a trailing struct member to do this in a single
allocation. Not a big deal, but it makes the code a bit cleaner.

> +
> +
> +       memcpy(var->name, name, namelen*2);
> +       memcpy(var->vendor, vendor, SECVAR_VENDOR_SIZE);
> +       var->attributes = attributes;
> +       var->data_size = data_size;
> +       memcpy(var->data, data, data_size);
> +

> +       // TODO: Prevalidate here?

What does pre-validation involve?

> +
> +       list_add_tail(&update_bank, &var->link);
> +
> +       if (!secvar_write_update_bank)
> +               return OPAL_HARDWARE;
> +
> +       secvar_write_update_bank(&update_bank);
> +
> +       return OPAL_SUCCESS;
> +}
> +opal_call(OPAL_SECVAR_ENQUEUE, opal_secvar_enqueue, 5);
> +
> +
> +// TODO: not implemented yet, out of scope for now
> +static int64_t opal_secvar_info(uint64_t attributes, uint64_t *storage_space, uint64_t *remaining_space, uint64_t *max_variable_size)
> +{
> +       (void) attributes;
> +       (void) storage_space;
> +       (void) remaining_space;

> +       (void) max_variable_size;

Is the max variable size a static value or does it depend on how much
space we have left? If it's static then this should be put in the
device-tree rather than being queried with an OPAL call.

> +
> +       return OPAL_UNSUPPORTED;
> +}
> +opal_call(OPAL_SECVAR_INFO, opal_secvar_info, 4);
> --
> 2.17.2
>
> _______________________________________________
> Skiboot mailing list
> Skiboot@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot

On 3/28/19 9:24 PM, Oliver wrote:
> On Fri, Mar 29, 2019 at 9:19 AM Eric Richter <erichte@linux.ibm.com> wrote:
>>
>> This patch provides the OPAL runtime service frontend for the host OS to
>> retrieve secure variables, and append new ones for processing on the
>> next reboot.
>>
>> Included are the four minimum required services for basic manipulation:
>>   - opal_secvar_read()
>>   - opal_secvar_read_next()
>>   - opal_secvar_enqueue()
>>   - opal_secvar_info()
>>

I just noticed I failed to update this patch description from a previous 
version, should be s/read/get/

>> opal_secvar_read() takes in an name string and a vendor buffer (likely a
>> GUID), and returns the corresponding blob of data if there is a matching
>> name + vendor pair in the variable bank. This runtime service only
>> operates on the variable bank.
>>
>> opal_secvar_read_next() provides a method to iterate through the
>> variables in the variable bank. If provided a name and vendor that match a
>> variable in the variable bank, the function will return the name and
>> vendor of the next variable in the bank in sequence. When provided an
>> empty string, it will begin from the top of the list. This runtime service
>> only operates on the variable bank.
>>
>> opal_secvar_enqueue() provides a method for the host OS to submit a new
>> secure variable for processing on next boot, by appending it to the
>> update bank, or "update queue". As this does not affect the variable bank,
>> appending a variable via this runtime service will not affect the output
>> of the previous two. The update queue will only ever be processed during
>> the initialization of skiboot, and is part of a forthcoming patch
>> series.
>>
>> opal_secvar_info() has not been implemented yet. It will return
>> status on the size of, and remaining storage space for secure variables.
>>
>> Signed-off-by: Eric Richter <erichte@linux.ibm.com>
>> ---
>>   include/opal-api.h  |   6 +-
>>   libstb/Makefile.inc |   2 +-
>>   libstb/secvar_api.c | 203 ++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 209 insertions(+), 2 deletions(-)
>>   create mode 100644 libstb/secvar_api.c
>>
>> diff --git a/include/opal-api.h b/include/opal-api.h
>> index 73f86f9a..8e398073 100644
>> --- a/include/opal-api.h
>> +++ b/include/opal-api.h
>> @@ -226,7 +226,11 @@
>>   #define OPAL_NX_COPROC_INIT                    167
>>   #define OPAL_NPU_SET_RELAXED_ORDER             168
>>   #define OPAL_NPU_GET_RELAXED_ORDER             169
>> -#define OPAL_LAST                              169
>> +#define OPAL_SECVAR_GET                                170
>> +#define OPAL_SECVAR_GET_NEXT                   171
> 
>> +#define OPAL_SECVAR_ENQUEUE                    172
> 
> Call it ENQUEUE_UPDATE or something. This is a bit vague. >
>> +#define OPAL_SECVAR_INFO                       173
>> +#define OPAL_LAST                              173
>>
>>   #define QUIESCE_HOLD                   1 /* Spin all calls at entry */
>>   #define QUIESCE_REJECT                 2 /* Fail all calls with OPAL_BUSY */
>> diff --git a/libstb/Makefile.inc b/libstb/Makefile.inc
>> index 2bc9e91b..f3a7e716 100644
>> --- a/libstb/Makefile.inc
>> +++ b/libstb/Makefile.inc
>> @@ -4,7 +4,7 @@ LIBSTB_DIR = libstb
>>
>>   SUBDIRS += $(LIBSTB_DIR)
>>
>> -LIBSTB_SRCS = container.c tpm_chip.c cvc.c secureboot.c trustedboot.c secvar.c
>> +LIBSTB_SRCS = container.c tpm_chip.c cvc.c secureboot.c trustedboot.c secvar.c secvar_api.c
>>   LIBSTB_OBJS = $(LIBSTB_SRCS:%.c=%.o)
>>   LIBSTB = $(LIBSTB_DIR)/built-in.a
>>
>> diff --git a/libstb/secvar_api.c b/libstb/secvar_api.c
>> new file mode 100644
>> index 00000000..c5de3bdd
>> --- /dev/null
>> +++ b/libstb/secvar_api.c
>> @@ -0,0 +1,203 @@
>> +/* Copyright 2019 IBM Corp.
>> + *
>> + * Licensed under the Apache License, Version 2.0 (the "License");
>> + * you may not use this file except in compliance with the License.
>> + * You may obtain a copy of the License at
>> + *
>> + *      http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing, software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>> + * implied.
>> + * See the License for the specific language governing permissions and
>> + * limitations under the License.
>> + */
>> +
>> +#ifndef pr_fmt
>> +#define pr_fmt(fmt) "SECVAR_API: " fmt
>> +#endif
>> +
>> +#include <opal.h>
>> +#include "secvar.h"
>> +
>> +
>> +static int64_t opal_secvar_get(uint64_t k_name, uint64_t k_vendor, uint64_t k_attributes, uint64_t k_data_size, uint64_t k_data)
>> +{
>> +       struct secvar *var;
>> +
>> +       char16_t *name = (char16_t *) k_name;
>> +       guid_t *vendor = (guid_t *) k_vendor;
>> +       uint32_t *attributes = (uint32_t *) k_attributes;       // TODO: make sure this size is correct
>> +       size_t *data_size = (size_t *) k_data_size;
>> +       void *data = (void *) k_data;
>> +
>> +       if (!secvar_enabled)
>> +               return OPAL_HARDWARE;
>> +       if (!name)
>> +               return OPAL_PARAMETER;
>> +       if (!vendor)
>> +               return OPAL_PARAMETER;
>> +       if (!data_size)
>> +               return OPAL_PARAMETER;
>> +       if ((*data_size == 0) && (data))        // Data should be NULL when size is zero
>> +               return OPAL_PARAMETER;
>> +       if ((*data_size) && (!data))
>> +               return OPAL_PARAMETER;          // Data can't be NULL with nonzero size
>> +       if (str16nlen(name, SECVAR_MAX_NAME_SIZE) <= 0)
>> +               return OPAL_PARAMETER;          // Empty or erroneous name is useless
>> +
>> +       var = find_secvar_by_name_vendor(name, vendor, &variable_bank);
>> +
>> +       if (!var)
>> +               return OPAL_EMPTY;
>> +
>> +       if (!data) {
>> +               *data_size = var->data_size;
>> +               return OPAL_SUCCESS; // Size check
>> +       }
>> +
>> +       if (var->data_size > *data_size) {
>> +               *data_size = var->data_size;
>> +               return OPAL_PARTIAL;
>> +       }
>> +
>> +       memcpy(data, var->data, var->data_size);
>> +       *data_size = var->data_size;
>> +
> 
>> +       if (attributes)
>> +               *attributes = var->attributes;
> 
> Moving this above the !data check might be a good idea. That would
> allow the kernel to query the size and attributes simultaneously.
> 
> 
>> +
>> +
>> +       return OPAL_SUCCESS;
>> +}
>> +opal_call(OPAL_SECVAR_GET, opal_secvar_get, 5);
>> +
>> +
>> +static int64_t opal_secvar_get_next(uint64_t k_name_size, uint64_t k_name, uint64_t k_vendor)
>> +{
>> +       struct secvar *var;
>> +       int namelen;
>> +
>> +       size_t *name_size = (size_t *) k_name_size;
>> +       char16_t *name = (char16_t *) k_name;
>> +       guid_t *vendor = (guid_t *) k_vendor;
>> +
>> +       if (!secvar_enabled)
>> +               return OPAL_HARDWARE;
>> +       if (!name_size || (*name_size == 0))
>> +               return OPAL_PARAMETER;
>> +       if (!name)
>> +               return OPAL_PARAMETER;
>> +       if (!vendor)
>> +               return OPAL_PARAMETER;
>> +
>> +       namelen = str16nlen(name, SECVAR_MAX_NAME_SIZE);
>> +       if (namelen < 0)
>> +               return OPAL_PARAMETER;  // Erroneous name buffer
>> +
>> +       if (namelen != 0) {
>> +               // Non-empty string
>> +               var = find_secvar_by_name_vendor(name, vendor, &variable_bank);
>> +               if (!var)
>> +                       return OPAL_PARAMETER;
>> +
>> +               var = list_next(&variable_bank, var, link);
>> +       }
>> +       else {
>> +               // Empty string was found, returning first entry
>> +               var = list_top(&variable_bank, struct secvar, link);
>> +       }
>> +
>> +       if (!var)
>> +               return OPAL_EMPTY;
>> +
>> +       // Return name buffer is too small
>> +       namelen = str16nlen(var->name, SECVAR_MAX_NAME_SIZE);
>> +       if (namelen <= 0)
>> +               return OPAL_HARDWARE; // This should not be reached
>> +
>> +       if ((namelen*2) > *name_size) {
>> +               *name_size = namelen*2;
>> +               return OPAL_PARTIAL;
>> +       }
>> +
>> +
>> +       memcpy(name, var->name, namelen*2);
>> +       memcpy(vendor, var->vendor, SECVAR_VENDOR_SIZE);
>> +
>> +       return OPAL_SUCCESS;
>> +}
>> +opal_call(OPAL_SECVAR_GET_NEXT, opal_secvar_get_next, 3);
>> +
>> +static int64_t opal_secvar_enqueue(uint64_t k_name, uint64_t k_vendor, uint64_t k_attributes, uint64_t k_data_size, uint64_t k_data)
>> +{
>> +       struct secvar *var;
>> +       int namelen;
>> +
>> +       char16_t *name = (char16_t *) k_name;
>> +       guid_t *vendor = (guid_t *) k_vendor;
>> +       uint32_t attributes = (uint32_t) k_attributes;
>> +       size_t data_size = (size_t) k_data_size;
>> +       void *data = (void *) k_data;
>> +
>> +       if (!secvar_enabled)
>> +               return OPAL_HARDWARE;
>> +       if (!name)
>> +               return OPAL_PARAMETER;
>> +       if (!vendor)
>> +               return OPAL_PARAMETER;
>> +       if (!data)
>> +               return OPAL_PARAMETER;
> 
>> +       if (data_size == 0)
>> +               return OPAL_UNSUPPORTED;        // we don't support variable deletion this way
> 
> How does variable deletion work?
> 

At the moment, we don't support that. In order for a variable to be 
deleted, it would have to pass through the update queue, and thus would 
need to be signed. Signing nothing, or some fixed value would be 
vulnerable to a replay attack, so for now we don't support this.

The kernel however, expects a zero data_size as a deletion operation. 
This is included as we explicitly don't support that method.

>> +       if (data_size > SECVAR_MAX_DATA_SIZE)
>> +               return OPAL_PARAMETER;
>> +
>> +       namelen = str16nlen(name, SECVAR_MAX_NAME_SIZE);
>> +       if (namelen <= 0)
>> +               return OPAL_PARAMETER;          // empty or erroneous name is useless
>> +
> 
>> +       var = zalloc(sizeof(struct secvar));
>> +       if (!var)
>> +               return OPAL_NO_MEM;
>> +
>> +       var->data = malloc(data_size);
>> +       if (!var->data) {
>> +               free(var);
>> +               return OPAL_NO_MEM;
>> +       }
> 
> You can use a trailing struct member to do this in a single
> allocation. Not a big deal, but it makes the code a bit cleaner.
> 
>> +
>> +
>> +       memcpy(var->name, name, namelen*2);
>> +       memcpy(var->vendor, vendor, SECVAR_VENDOR_SIZE);
>> +       var->attributes = attributes;
>> +       var->data_size = data_size;
>> +       memcpy(var->data, data, data_size);
>> +
> 
>> +       // TODO: Prevalidate here?
> 
> What does pre-validation involve?
> 

We can only process variable updates on initial boot, so ideally we 
should catch bad variable updates here before going for a full reboot 
(invalid signature, etc).

This would not actually accept the new variable into the variable bank, 
but attempt to filter bad ones that will fail anyway on boot.

>> +
>> +       list_add_tail(&update_bank, &var->link);
>> +
>> +       if (!secvar_write_update_bank)
>> +               return OPAL_HARDWARE;
>> +
>> +       secvar_write_update_bank(&update_bank);
>> +
>> +       return OPAL_SUCCESS;
>> +}
>> +opal_call(OPAL_SECVAR_ENQUEUE, opal_secvar_enqueue, 5);
>> +
>> +
>> +// TODO: not implemented yet, out of scope for now
>> +static int64_t opal_secvar_info(uint64_t attributes, uint64_t *storage_space, uint64_t *remaining_space, uint64_t *max_variable_size)
>> +{
>> +       (void) attributes;
>> +       (void) storage_space;
>> +       (void) remaining_space;
> 
>> +       (void) max_variable_size;
> 
> Is the max variable size a static value or does it depend on how much
> space we have left? If it's static then this should be put in the
> device-tree rather than being queried with an OPAL call.
> 
It is (likely) a static size, however this is a call the kernel 
implementation is expecting.

>> +
>> +       return OPAL_UNSUPPORTED;
>> +}
>> +opal_call(OPAL_SECVAR_INFO, opal_secvar_info, 4);
>> --
>> 2.17.2
>>
>> _______________________________________________
>> Skiboot mailing list
>> Skiboot@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/skiboot
>