On Tue, 1 Aug 2023, Daniel P. Smith wrote:
> The field `is_console` suggests that the field represents a state of being or
> posession, not that it reflects the privilege to access the console. In this
^ possession
> patch the field is renamed to capabilities to encapsulate the capabilities a
> domain has been granted. The first capability being the ability to read/write
> the Xen console.
>
> Signed-off-by: Daniel P. Smith <dpsm...@apertussolutions.com>
Patch looks fine to me aside the two minor nits. I am not sure I
understand 100% the difference between capabilities and roles but I am
OK with the patch. I'd like to hear Julien's feedback on this as well.
> ---
> xen/arch/arm/domain_build.c | 4 +++-
> xen/include/xen/sched.h | 25 +++++++++++++++++++++++--
> xen/include/xsm/dummy.h | 2 +-
> 3 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 51b4daefe1..ad7432b029 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -4076,7 +4076,9 @@ void __init create_domUs(void)
> panic("Error creating domain %s (rc = %ld)\n",
> dt_node_name(node), PTR_ERR(d));
>
> - d->is_console = true;
> + if ( ! domain_set_cap(d, CAP_CONSOLE_IO) )
code style
> + printk("failed setting console_io on %pd\n", d);
> +
> dt_device_set_used_by(node, d->domain_id);
>
> rc = construct_domU(d, node);
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index ec0f9baff6..b04fbe0565 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -472,8 +472,8 @@ struct domain
> #define ROLE_HARDWARE_DOMAIN (1U<<2)
> #define ROLE_XENSTORE_DOMAIN (1U<<3)
> uint8_t role;
> - /* Can this guest access the Xen console? */
> - bool is_console;
> +#define CAP_CONSOLE_IO (1U<<0)
> + uint8_t capabilities;
> /* Is this guest being debugged by dom0? */
> bool debugger_attached;
> /*
> @@ -1146,6 +1146,27 @@ static always_inline bool is_hvm_vcpu(const struct
> vcpu *v)
> return is_hvm_domain(v->domain);
> }
>
> +static always_inline bool domain_has_cap(
> + const struct domain *d, uint8_t cap)
> +{
> + return d->capabilities & cap;
> +}
> +
> +static always_inline bool domain_set_cap(
> + struct domain *d, uint8_t cap)
> +{
> + switch ( cap )
> + {
> + case CAP_CONSOLE_IO:
> + d->capabilities |= cap;
> + break;
> + default:
> + return false;
> + }
> +
> + return domain_has_cap(d, cap);
> +}
> +
> static always_inline bool hap_enabled(const struct domain *d)
> {
> /* sanitise_domain_config() rejects HAP && !HVM */
> diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
> index 18f1ddd127..067ff1d111 100644
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -268,7 +268,7 @@ static XSM_INLINE int cf_check xsm_console_io(
> XSM_DEFAULT_ARG struct domain *d, int cmd)
> {
> XSM_ASSERT_ACTION(XSM_OTHER);
> - if ( d->is_console )
> + if ( domain_has_cap(d, CAP_CONSOLE_IO) )
> return xsm_default_action(XSM_HOOK, d, NULL);
> #ifdef CONFIG_VERBOSE_DEBUG
> if ( cmd == CONSOLEIO_write )
> --
> 2.20.1
>
