On 06.06.2024 10:39, Sergiy Kibrik wrote:
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -1160,6 +1160,7 @@ long arch_do_domctl(
> break;
> }
>
> +#ifdef CONFIG_INTEL
> case XEN_DOMCTL_psr_cmt_op:
> if ( !psr_cmt_enabled() )
> {
> @@ -1262,6 +1263,7 @@ long arch_do_domctl(
> }
>
> break;
> +#endif
Imo the result thereof shouldn't be -ENOSYS, but -EOPNOTSUPP (at least
for XEN_DOMCTL_psr_alloc; for XEN_DOMCTL_psr_cmt_op it shouldn't change,
even if I consider this wrong, but that's a separate topic). Wouldn't it
be possible here to reduce the #ifdef scope a little to just most of the
inner switch()es (i.e. requiring it to be split into two regions), whose
"default" case is already (kind of) doing what we want?
> --- a/xen/arch/x86/include/asm/psr.h
> +++ b/xen/arch/x86/include/asm/psr.h
> @@ -72,6 +72,8 @@ static inline bool psr_cmt_enabled(void)
> return !!psr_cmt;
> }
>
> +#ifdef CONFIG_INTEL
> +
> int psr_alloc_rmid(struct domain *d);
> void psr_free_rmid(struct domain *d);
> void psr_ctxt_switch_to(struct domain *d);
> @@ -86,6 +88,19 @@ int psr_set_val(struct domain *d, unsigned int socket,
> void psr_domain_init(struct domain *d);
> void psr_domain_free(struct domain *d);
>
> +#else
> +
> +static inline void psr_ctxt_switch_to(struct domain *d)
> +{
> +}
> +static inline void psr_domain_init(struct domain *d)
> +{
> +}
> +static inline void psr_domain_free(struct domain *d)
> +{
> +}
> +#endif /* CONFIG_INTEL */
As I think I did mention elsewhere, such stubs can have the braces on the
same line the the function specifier.
> @@ -169,6 +171,7 @@ long arch_do_sysctl(
> }
> break;
>
> +#ifdef CONFIG_INTEL
> case XEN_SYSCTL_psr_cmt_op:
> if ( !psr_cmt_enabled() )
> return -ENODEV;
> @@ -286,6 +289,7 @@ long arch_do_sysctl(
> }
> break;
> }
> +#endif
Like for domctl I think you want to reduce the #ifdef scope some, even if
for XEN_SYSCTL_psr_cmt_op that'll still result in -ENOSYS. At least we'll
then be consistent in clearing sysctl->u.psr_cmt_op.u.data there.
Jan
