On 9/6/21 2:31 PM, Andrew Cooper wrote:
On 03/09/2021 20:06, Daniel P. Smith wrote:
This renames the `struct xsm_operations` to the shorter `struct xsm_ops` and
converts the global xsm_ops from being a pointer to an explicit instance. As
part of this conversion, it reworks the XSM modules init function to return
their xsm_ops struct which is copied in to the global xsm_ops instance.
Signed-off-by: Daniel P. Smith <dpsm...@apertussolutions.com>
Acked-by: Andrew Cooper <andrew.coop...@citrix.com>
However, some suggestions...
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 55483292c5..859af3fe9a 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -28,9 +28,17 @@
#include <asm/setup.h>
#endif
-#define XSM_FRAMEWORK_VERSION "1.0.0"
+#define XSM_FRAMEWORK_VERSION "1.0.1"
-struct xsm_operations *xsm_ops;
+struct xsm_ops __read_mostly xsm_ops;
+
+enum xsm_ops_state {
+ XSM_OPS_UNREGISTERED,
+ XSM_OPS_REG_FAILED,
+ XSM_OPS_REGISTERED,
+};
+
+static enum xsm_ops_state xsm_ops_registered = XSM_OPS_UNREGISTERED;
__read_mostly, or can this be __initdata ?
Apologies, I think you may have suggested this before. It would be good
to be able to check this later but currently since I just introduced
this and it is only used during init, it could be made __initdata for
now and later if it gets exposed, then it can be moved to __read_mostly.
Do you agree?
@@ -87,25 +88,35 @@ static int __init xsm_core_init(const void *policy_buffer,
size_t policy_size)
}
#endif
- if ( verify(&dummy_xsm_ops) )
+ if ( xsm_ops_registered != XSM_OPS_UNREGISTERED )
{
- printk(XENLOG_ERR "Could not verify dummy_xsm_ops structure\n");
+ printk(XENLOG_ERR
+ "Could not init XSM, xsm_ops register already attempted\n");
return -EIO;
}
- xsm_ops = &dummy_xsm_ops;
-
switch ( xsm_bootparam )
{
case XSM_BOOTPARAM_DUMMY:
+ xsm_ops_registered = XSM_OPS_REGISTERED;
break;
case XSM_BOOTPARAM_FLASK:
- flask_init(policy_buffer, policy_size);
+ ops = flask_init(policy_buffer, policy_size);
+ if ( ops )
+ {
+ xsm_ops_registered = XSM_OPS_REGISTERED;
+ xsm_ops = *ops;
+ }
break;
case XSM_BOOTPARAM_SILO:
- silo_init();
+ ops = silo_init();
+ if ( ops )
+ {
+ xsm_ops_registered = XSM_OPS_REGISTERED;
+ xsm_ops = *ops;
+ }
This if ( ops ) block can be deduplicated by moving after the switch()
statement. It's going to be common to all everything except dummy.
Good call. I will rework to remove the duplication.
v/r,
dps
