Re: [RFC 4/6] capabilities: introduce console io as a domain capability

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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 <dpsmith@xxxxxxxxxxxxxxxxxxxx>

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
>