| Message ID | 20231115151242.184645-4-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/uefi: add uefi variable service | expand |
On 11/15/23 16:12, Gerd Hoffmann wrote: > Add state structs and function declarations for the uefi-vars device. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > include/hw/uefi/var-service.h | 119 ++++++++++++++++++++++++++++++++++ > 1 file changed, 119 insertions(+) > create mode 100644 include/hw/uefi/var-service.h > > diff --git a/include/hw/uefi/var-service.h b/include/hw/uefi/var-service.h > new file mode 100644 > index 000000000000..2b8d3052e59f > --- /dev/null > +++ b/include/hw/uefi/var-service.h > @@ -0,0 +1,119 @@ > +/* > + * SPDX-License-Identifier: GPL-2.0-or-later > + * > + * uefi-vars device - state struct and function prototypes > + */ > +#ifndef QEMU_UEFI_VAR_SERVICE_H > +#define QEMU_UEFI_VAR_SERVICE_H > + > +#include "qemu/uuid.h" > +#include "qemu/queue.h" > + > +#include "hw/uefi/var-service-edk2.h" > + > +#define MAX_BUFFER_SIZE (64 * 1024) > + > +typedef struct uefi_variable uefi_variable; > +typedef struct uefi_var_policy uefi_var_policy; > +typedef struct uefi_vars_state uefi_vars_state; > + > +struct uefi_variable { > + QemuUUID guid; > + uint16_t *name; > + uint32_t name_size; > + uint32_t attributes; > + void *data; > + uint32_t data_size; > + QTAILQ_ENTRY(uefi_variable) next; > +}; > + > +struct uefi_var_policy { > + variable_policy_entry *entry; > + uint32_t entry_size; > + uint16_t *name; > + uint32_t name_size; > + uint32_t hashmarks; > + QTAILQ_ENTRY(uefi_var_policy) next; > +}; - I wonder if the size fields should be size_t. uint32_t is not wrong either; we'll just have to be careful when doing comparisons etc. - care to explain (in a comment) hashmarks? I think it's related to the wildcard policy stuff, but a hint would be appreciated. > + > +struct uefi_vars_state { > + MemoryRegion mr; > + uint16_t sts; > + uint32_t buf_size; > + uint32_t buf_addr_lo; > + uint32_t buf_addr_hi; spelling out endianness here would be useful IMO > + uint8_t *buffer; > + QTAILQ_HEAD(, uefi_variable) variables; > + QTAILQ_HEAD(, uefi_var_policy) var_policies; > + > + /* boot phases */ > + bool end_of_dxe; > + bool ready_to_boot; > + bool exit_boot_service; There are some variations of the 8 possible that don't make sense. at the same time, a single enum could be too limiting. depends on what the code will do with these. > + bool policy_locked; > + > + /* storage accounting */ > + uint64_t max_storage; > + uint64_t used_storage; > + > + char *jsonfile; > + int jsonfd; > +}; > + > +/* vars-service-guid.c */ > +extern QemuUUID EfiGlobalVariable; > +extern QemuUUID EfiImageSecurityDatabase; > +extern QemuUUID EfiCustomModeEnable; > +extern QemuUUID EfiSecureBootEnableDisable; > +extern QemuUUID EfiSmmVariableProtocolGuid; > +extern QemuUUID VarCheckPolicyLibMmiHandlerGuid; > +extern QemuUUID EfiEndOfDxeEventGroupGuid; > +extern QemuUUID EfiEventReadyToBootGuid; > +extern QemuUUID EfiEventExitBootServicesGuid; the spelling of these names appears a bit questionable: - camelcase is idiomatic in edk2, but (I think?) not in QEMU, for variables - the "Guid" suffix is inconsistently used / carried over from edk2 > + > +/* vars-service-core.c */ > +extern const VMStateDescription vmstate_uefi_vars; > +size_t uefi_strlen(const uint16_t *str, size_t len); > +gboolean uefi_str_equal(const uint16_t *a, size_t alen, > + const uint16_t *b, size_t blen); > +char *uefi_ucs2_to_ascii(const uint16_t *ucs2, uint64_t ucs2_size); > +void uefi_trace_variable(const char *action, QemuUUID guid, > + const uint16_t *name, uint64_t name_size); > +void uefi_trace_status(const char *action, efi_status status); > +void uefi_vars_init(Object *obj, uefi_vars_state *uv); > +void uefi_vars_realize(uefi_vars_state *uv, Error **errp); > +void uefi_vars_hard_reset(uefi_vars_state *uv); > + > +/* vars-service-json.c */ > +void uefi_vars_json_init(uefi_vars_state *uv, Error **errp); > +void uefi_vars_json_save(uefi_vars_state *uv); > +void uefi_vars_json_load(uefi_vars_state *uv, Error **errp); > + > +/* vars-service-vars.c */ > +extern const VMStateDescription vmstate_uefi_variable; > +uefi_variable *uefi_vars_find_variable(uefi_vars_state *uv, QemuUUID guid, > + const uint16_t *name, > + uint64_t name_size); > +void uefi_vars_set_variable(uefi_vars_state *uv, QemuUUID guid, > + const uint16_t *name, uint64_t name_size, > + uint32_t attributes, > + void *data, uint64_t data_size); > +void uefi_vars_clear_volatile(uefi_vars_state *uv); > +void uefi_vars_clear_all(uefi_vars_state *uv); > +void uefi_vars_update_storage(uefi_vars_state *uv); > +uint32_t uefi_vars_mm_vars_proto(uefi_vars_state *uv); > + > +/* vars-service-auth.c */ > +void uefi_vars_auth_init(uefi_vars_state *uv); > + > +/* vars-service-policy.c */ > +extern const VMStateDescription vmstate_uefi_var_policy; > +efi_status uefi_vars_policy_check(uefi_vars_state *uv, > + uefi_variable *var, > + gboolean is_newvar); > +void uefi_vars_policies_clear(uefi_vars_state *uv); > +uefi_var_policy *uefi_vars_add_policy(uefi_vars_state *uv, > + variable_policy_entry *pe); > +uint32_t uefi_vars_mm_check_policy_proto(uefi_vars_state *uv); > + > +#endif /* QEMU_UEFI_VAR_SERVICE_H */ I guess I'll have to see these in use to think anything of them. (I prefer a more "functional" structuring for a series, where the thing sort of builds & works from patch#1 onwards, it's only the actual functionality that is introduced layer by layer. But, that's not an objection; this patch certainly works as the collection of APIs the rest is going to implement and call later.) Again we'll have to keep an eye on the integer types. with some comments inserted: Reviewed-by: Laszlo Ersek <lersek@redhat.com> Laszlo
Hi, > > +struct uefi_var_policy { > > + variable_policy_entry *entry; > > + uint32_t entry_size; > > + uint16_t *name; > > + uint32_t name_size; > > + uint32_t hashmarks; > > + QTAILQ_ENTRY(uefi_var_policy) next; > > +}; > > - I wonder if the size fields should be size_t. uint32_t is not wrong > either; we'll just have to be careful when doing comparisons etc. I can't store size_t in vmstate ... > - care to explain (in a comment) hashmarks? I think it's related to the > wildcard policy stuff, but a hint would be appreciated. Yes. It's the number of hashmarks, used when sorting entries by priority. Most specific match, i.e. smallest number of wildcard characters, goes first. I'll add a comment. > > +struct uefi_vars_state { > > + MemoryRegion mr; > > + uint16_t sts; > > + uint32_t buf_size; > > + uint32_t buf_addr_lo; > > + uint32_t buf_addr_hi; > > spelling out endianness here would be useful IMO Don't think this is needed, qemu handles this for us. The memory region is declared to be little endian, qemu will convert reads/writes to native endian for us, so there are no endian worries for the register interface (the transfer buffer in guest ram is a different story). > > + /* boot phases */ > > + bool end_of_dxe; > > + bool ready_to_boot; > > + bool exit_boot_service; > > There are some variations of the 8 possible that don't make sense. at > the same time, a single enum could be too limiting. depends on what the > code will do with these. end-of-dxe: used by variable policies (they are enforced only after that event). ready-to-boot: not used yet (other than setting it when the event arrives). exit-boot-service: access control (RT vs. BT etc). > > +/* vars-service-guid.c */ > > +extern QemuUUID EfiGlobalVariable; > > +extern QemuUUID EfiImageSecurityDatabase; > > +extern QemuUUID EfiCustomModeEnable; > > +extern QemuUUID EfiSecureBootEnableDisable; > > +extern QemuUUID EfiSmmVariableProtocolGuid; > > +extern QemuUUID VarCheckPolicyLibMmiHandlerGuid; > > +extern QemuUUID EfiEndOfDxeEventGroupGuid; > > +extern QemuUUID EfiEventReadyToBootGuid; > > +extern QemuUUID EfiEventExitBootServicesGuid; > > the spelling of these names appears a bit questionable: > > - camelcase is idiomatic in edk2, but (I think?) not in QEMU, for variables > > - the "Guid" suffix is inconsistently used / carried over from edk2 Yes. It's carried over from edk2. We don't have to keep the names in qemu, in fact I've renamed the structs because that would have been too much of a contrast to the qemu code style, so the edk2 name is only noted in a comment in the vars-service-edk2.h header file. For the #defines and GUIDs I've kept the edk2 names as-is, so it's easier to follow the control flow for people which are familiar with edk2. We have already have simliar stuff else, for example the struct names in hw/usb/desc.h follow the usb spec not qemu code style. take care, Gerd
diff --git a/include/hw/uefi/var-service.h b/include/hw/uefi/var-service.h new file mode 100644 index 000000000000..2b8d3052e59f --- /dev/null +++ b/include/hw/uefi/var-service.h @@ -0,0 +1,119 @@ +/* + * SPDX-License-Identifier: GPL-2.0-or-later + * + * uefi-vars device - state struct and function prototypes + */ +#ifndef QEMU_UEFI_VAR_SERVICE_H +#define QEMU_UEFI_VAR_SERVICE_H + +#include "qemu/uuid.h" +#include "qemu/queue.h" + +#include "hw/uefi/var-service-edk2.h" + +#define MAX_BUFFER_SIZE (64 * 1024) + +typedef struct uefi_variable uefi_variable; +typedef struct uefi_var_policy uefi_var_policy; +typedef struct uefi_vars_state uefi_vars_state; + +struct uefi_variable { + QemuUUID guid; + uint16_t *name; + uint32_t name_size; + uint32_t attributes; + void *data; + uint32_t data_size; + QTAILQ_ENTRY(uefi_variable) next; +}; + +struct uefi_var_policy { + variable_policy_entry *entry; + uint32_t entry_size; + uint16_t *name; + uint32_t name_size; + uint32_t hashmarks; + QTAILQ_ENTRY(uefi_var_policy) next; +}; + +struct uefi_vars_state { + MemoryRegion mr; + uint16_t sts; + uint32_t buf_size; + uint32_t buf_addr_lo; + uint32_t buf_addr_hi; + uint8_t *buffer; + QTAILQ_HEAD(, uefi_variable) variables; + QTAILQ_HEAD(, uefi_var_policy) var_policies; + + /* boot phases */ + bool end_of_dxe; + bool ready_to_boot; + bool exit_boot_service; + bool policy_locked; + + /* storage accounting */ + uint64_t max_storage; + uint64_t used_storage; + + char *jsonfile; + int jsonfd; +}; + +/* vars-service-guid.c */ +extern QemuUUID EfiGlobalVariable; +extern QemuUUID EfiImageSecurityDatabase; +extern QemuUUID EfiCustomModeEnable; +extern QemuUUID EfiSecureBootEnableDisable; +extern QemuUUID EfiSmmVariableProtocolGuid; +extern QemuUUID VarCheckPolicyLibMmiHandlerGuid; +extern QemuUUID EfiEndOfDxeEventGroupGuid; +extern QemuUUID EfiEventReadyToBootGuid; +extern QemuUUID EfiEventExitBootServicesGuid; + +/* vars-service-core.c */ +extern const VMStateDescription vmstate_uefi_vars; +size_t uefi_strlen(const uint16_t *str, size_t len); +gboolean uefi_str_equal(const uint16_t *a, size_t alen, + const uint16_t *b, size_t blen); +char *uefi_ucs2_to_ascii(const uint16_t *ucs2, uint64_t ucs2_size); +void uefi_trace_variable(const char *action, QemuUUID guid, + const uint16_t *name, uint64_t name_size); +void uefi_trace_status(const char *action, efi_status status); +void uefi_vars_init(Object *obj, uefi_vars_state *uv); +void uefi_vars_realize(uefi_vars_state *uv, Error **errp); +void uefi_vars_hard_reset(uefi_vars_state *uv); + +/* vars-service-json.c */ +void uefi_vars_json_init(uefi_vars_state *uv, Error **errp); +void uefi_vars_json_save(uefi_vars_state *uv); +void uefi_vars_json_load(uefi_vars_state *uv, Error **errp); + +/* vars-service-vars.c */ +extern const VMStateDescription vmstate_uefi_variable; +uefi_variable *uefi_vars_find_variable(uefi_vars_state *uv, QemuUUID guid, + const uint16_t *name, + uint64_t name_size); +void uefi_vars_set_variable(uefi_vars_state *uv, QemuUUID guid, + const uint16_t *name, uint64_t name_size, + uint32_t attributes, + void *data, uint64_t data_size); +void uefi_vars_clear_volatile(uefi_vars_state *uv); +void uefi_vars_clear_all(uefi_vars_state *uv); +void uefi_vars_update_storage(uefi_vars_state *uv); +uint32_t uefi_vars_mm_vars_proto(uefi_vars_state *uv); + +/* vars-service-auth.c */ +void uefi_vars_auth_init(uefi_vars_state *uv); + +/* vars-service-policy.c */ +extern const VMStateDescription vmstate_uefi_var_policy; +efi_status uefi_vars_policy_check(uefi_vars_state *uv, + uefi_variable *var, + gboolean is_newvar); +void uefi_vars_policies_clear(uefi_vars_state *uv); +uefi_var_policy *uefi_vars_add_policy(uefi_vars_state *uv, + variable_policy_entry *pe); +uint32_t uefi_vars_mm_check_policy_proto(uefi_vars_state *uv); + +#endif /* QEMU_UEFI_VAR_SERVICE_H */
Add state structs and function declarations for the uefi-vars device. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- include/hw/uefi/var-service.h | 119 ++++++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 include/hw/uefi/var-service.h