xen-shell-cmd: Allow config selection for domain creation #57

Deedone · 2024-04-01T10:01:33Z

Add linker script to collect all domain configs in one place. Add macro DECL_CONFIG to mark domain config structures. Specific config can be selected for domain creation by name via the shell command line parameter.

xen-shell-cmd/src/xen_cmds.c

xakep-amatop · 2024-04-01T13:04:15Z

xen-shell-cmd/src/xen_cmds.c

@@ -15,7 +16,8 @@

 LOG_MODULE_REGISTER(xen_shell);

-extern struct xen_domain_cfg domd_cfg;


it would be nice to put domd_cfg in domain_configs
or mark this struct that it should be placed in the section

The intention of this change is to remove the direct dependency on external variable. Section attributes should be added in the same place where the config struct is declared, which is currently in the AOS project.

I understand. I mean, could you open a pull request with the necessary changes so that we can use the 'xu create' command with domD as well?

Deedone · 2024-04-04T10:47:34Z

Rebased and implemented suggestions.

oleksiimoisieiev · 2024-04-17T13:07:51Z

Checkpatch check was failed

Deedone · 2024-04-17T13:16:27Z

Checkpatch check was failed

It is a false positive.

GrygiriiS · 2024-04-22T12:17:31Z

Why not create _init(struct xen_domain_cfg*) for library? It really should be problem of Zephyr app and not a library where to place/get dom_cfgs.

Deedone · 2024-04-24T12:40:44Z

Depends on #74

Deedone · 2024-04-24T12:40:56Z

Why not create _init(struct xen_domain_cfg*) for library? It really should be problem of Zephyr app and not a library where to place/get dom_cfgs.

This will force every application developer to come up with a similar system. I believe it is better to hide the complexity within a library, instead of forcing it onto its users.

GrygiriiS · 2024-04-24T14:55:41Z

Why not create _init(struct xen_domain_cfg*) for library? It really should be problem of Zephyr app and not a library where to place/get dom_cfgs.

This will force every application developer to come up with a similar system. I believe it is better to hide the complexity within a library, instead of forcing it onto its users.

It forces applications to use some linker sections which in turn adds:

platform specific issues/dependencies like cons & not const, XIP vs not XIP
prevents placing/retrieving dom_cfg form anywhere like shared mem, or placing cfg in FIP image for example (FIP can be attached to Zephyr binary and prepared outside of Zephyr build system)
prevents form building and using dynamic cfg list (i do not like it but could happen)

Playing with linker in Zephyr is powerful tool from one side, but painful from another - ask @oleksiimoisieiev.

Really, simplest possible solution for library is xenlib_int().

lorc · 2024-04-24T21:33:16Z

I agree with @GrygiriiS here. App should provide own callbacks for obtaining configs, images, etc. We can provide some generic implementation in xenlib for a default use case. But it should be strictly optional.

GrygiriiS · 2024-04-26T06:54:53Z

I agree with @GrygiriiS here. App should provide own callbacks for obtaining configs, images, etc. We can provide some generic implementation in xenlib for a default use case. But it should be strictly optional.

Hope I understood you idea right, add

typedef int  (*domain_get_cfg_cb_t)(const uint32_t domid,  const struct xen_domain_cfg **dom_cfg);

int domain_set_get_cfg_cb(domain_get_cfg_cb_t cb);

The main user for now is XEN shell. A small question about func naming, but I like the Idea.

Deedone · 2024-04-26T08:48:43Z

Added additional way for apps to provide their configs
Added config option to disable additional section creation

Waiting for merge of #74 to update cmdline interface

GrygiriiS · 2024-04-30T08:42:01Z

xen-shell-cmd/src/xen_cmds.c

@@ -151,6 +198,9 @@ SHELL_STATIC_SUBCMD_SET_CREATE(
 		      " Unpause Xen domain\n"
 		      " Usage: unpause -d <domid>\n",
 		      domu_unpause, 3, 0),
+	SHELL_CMD_ARG(config_list, NULL,
+		      " List available domain configurations\n",
+		      xen_config_list, 1, 0),


How about "config list"? With possibility to have "config dump" in the future, for example?

Thank you for the suggestion but I am not sure if this change will provide any actual benefit.

GrygiriiS · 2024-04-30T08:43:12Z

xen-shell-cmd/src/xen_cmds.c

+
+	for (i = 0; i < domain_get_user_cfg_count(); i++) {
+		cfg = domain_get_user_cfg(i);
+		shell_print(shell, "%s", cfg->name);


Could you add index in print, pls? "%d %s" <- i, name

No, because it will add confusion suggesting to users that they can somehow use this number somewhere.

GrygiriiS · 2024-04-30T08:43:29Z

xen-shell-cmd/src/xen_cmds.c

+
+#ifdef CONFIG_XEN_DOMCFG_SECTION
+	for (cfg = _domain_configs_start; cfg < _domain_configs_end; cfg++) {
+		shell_print(shell, "%s", cfg->name);


Could you add index in print, pls? "%d %s" <- i, name

GrygiriiS · 2024-05-07T16:13:33Z

Tested-by: Grygorii Strashko <[email protected]>

lorc · 2024-05-09T10:24:49Z

xen-dom-mgmt/linker.ld

+  _domain_configs_start = .;
+  KEEP(*(.domain_configs))
+  _domain_configs_end = .;
+} > RAM


In which section it will reside at the end? I expect it to get somewhere in .rodata

no :P Not exactly :) Now it can't be made const. More over, it depends on iomems, dtdevs, dt_passthrough.
Now it can be only in RAM

I kinda did experiment to build dom_cfg.c with one config as .o outside Zephyr - results are not encouraging.

It's a pity. On other hand, I can imagine that someone might want to alter configuration in run-time. So maybe, in the end, this is not as bad as it seems.

Also, back_cfg is stored in xen_domain_cfg struct, it is filled manually by app or via xrun external configuration before domain is created. Thus we definitely need it to be r/w or rework xrun + app processing to separate structure.

It is at the end of data_region

lorc · 2024-05-09T10:25:11Z

xen-dom-mgmt/src/xen-dom-mgmt.c

@@ -616,6 +617,21 @@ struct xen_domain *domid_to_domain(uint32_t domid)
 	return NULL;
 }

+struct xen_domain_cfg *domain_find_config(char *name)


const char *name

lorc

Thanks

Reviewed-by: Volodymyr Babchuk <[email protected]>

firscity

Reviewed-by: Dmytro Firsov <[email protected]>

Add linker script to collect all domain configs in one place. Add macro DECL_CONFIG to mark domain config structures. Specific config can be selected for domain creation by name via the shell command line parameter. Signed-off-by: Mykyta Poturai <[email protected]> Tested-by: Grygorii Strashko <[email protected]> Reviewed-by: Volodymyr Babchuk <[email protected]> Reviewed-by: Dmytro Firsov <[email protected]>

Provide weak functions for user configurations in the xen-dom-mgmt allowing apps to extend the list of configurations for a domain. Signed-off-by: Mykyta Poturai <[email protected]> Tested-by: Grygorii Strashko <[email protected]> Reviewed-by: Volodymyr Babchuk <[email protected]> Reviewed-by: Dmytro Firsov <[email protected]>

Add a new command to list available domain configurations. Signed-off-by: Mykyta Poturai <[email protected]> Tested-by: Grygorii Strashko <[email protected]> Reviewed-by: Volodymyr Babchuk <[email protected]> Reviewed-by: Dmytro Firsov <[email protected]>

Deedone force-pushed the multiconfig branch from ce00583 to cf37117 Compare April 1, 2024 10:18

firscity reviewed Apr 1, 2024

View reviewed changes

xen-shell-cmd/src/xen_cmds.c Outdated Show resolved Hide resolved

xen-shell-cmd/src/xen_cmds.c Outdated Show resolved Hide resolved

xakep-amatop reviewed Apr 1, 2024

View reviewed changes

Deedone force-pushed the multiconfig branch from cf37117 to 33b74a8 Compare April 2, 2024 08:30

Deedone mentioned this pull request Apr 2, 2024

domd: Switch to multiconfig declarations aosedge/aos_core_zephyr#140

Open

Deedone force-pushed the multiconfig branch from 33b74a8 to 3264d27 Compare April 4, 2024 10:46

Deedone force-pushed the multiconfig branch from 3264d27 to d3f02da Compare April 17, 2024 10:39

lorc mentioned this pull request Apr 17, 2024

Rework command line interface #69

Closed

Deedone force-pushed the multiconfig branch from d3f02da to c7b51be Compare April 26, 2024 08:47

Deedone force-pushed the multiconfig branch from c7b51be to 057fcbf Compare April 26, 2024 09:01

GrygiriiS reviewed Apr 30, 2024

View reviewed changes

GrygiriiS mentioned this pull request May 3, 2024

RFC: zephyr-dom0-xt: add possibility to use many domain configs xen-troops/zephyr-dom0-xt#11

Closed

Deedone force-pushed the multiconfig branch from 057fcbf to 02fe427 Compare May 7, 2024 08:33

lorc reviewed May 9, 2024

View reviewed changes

Deedone force-pushed the multiconfig branch 2 times, most recently from 501b007 to e28b8d2 Compare May 10, 2024 08:41

lorc approved these changes May 10, 2024

View reviewed changes

firscity approved these changes May 13, 2024

View reviewed changes

Deedone added 2 commits May 13, 2024 14:45

Deedone force-pushed the multiconfig branch from e28b8d2 to 8527f93 Compare May 13, 2024 11:46

firscity merged commit 531d52b into xen-troops:main May 13, 2024
4 of 5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

xen-shell-cmd: Allow config selection for domain creation #57

xen-shell-cmd: Allow config selection for domain creation #57

Deedone commented Apr 1, 2024

xakep-amatop Apr 1, 2024 •

edited

Loading

Deedone Apr 2, 2024

xakep-amatop Apr 2, 2024

Deedone commented Apr 4, 2024

oleksiimoisieiev commented Apr 17, 2024

Deedone commented Apr 17, 2024

GrygiriiS commented Apr 22, 2024 •

edited

Loading

Deedone commented Apr 24, 2024

Deedone commented Apr 24, 2024

GrygiriiS commented Apr 24, 2024

lorc commented Apr 24, 2024

GrygiriiS commented Apr 26, 2024

Deedone commented Apr 26, 2024

GrygiriiS Apr 30, 2024

Deedone Apr 30, 2024

GrygiriiS Apr 30, 2024

Deedone Apr 30, 2024

GrygiriiS Apr 30, 2024

Deedone Apr 30, 2024

GrygiriiS commented May 7, 2024

lorc May 9, 2024

GrygiriiS May 9, 2024 •

edited

Loading

lorc May 9, 2024

firscity May 9, 2024

Deedone May 10, 2024

lorc May 9, 2024

lorc left a comment

firscity left a comment

		@@ -15,7 +16,8 @@

		LOG_MODULE_REGISTER(xen_shell);

		extern struct xen_domain_cfg domd_cfg;

xen-shell-cmd: Allow config selection for domain creation #57

xen-shell-cmd: Allow config selection for domain creation #57

Conversation

Deedone commented Apr 1, 2024

xakep-amatop Apr 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Deedone commented Apr 4, 2024

oleksiimoisieiev commented Apr 17, 2024

Deedone commented Apr 17, 2024

GrygiriiS commented Apr 22, 2024 • edited Loading

Deedone commented Apr 24, 2024

Deedone commented Apr 24, 2024

GrygiriiS commented Apr 24, 2024

lorc commented Apr 24, 2024

GrygiriiS commented Apr 26, 2024

Deedone commented Apr 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GrygiriiS commented May 7, 2024

Choose a reason for hiding this comment

GrygiriiS May 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lorc left a comment

Choose a reason for hiding this comment

firscity left a comment

Choose a reason for hiding this comment

xakep-amatop Apr 1, 2024 •

edited

Loading

GrygiriiS commented Apr 22, 2024 •

edited

Loading

GrygiriiS May 9, 2024 •

edited

Loading