On 17.03.2023 14:19, Luca Fancellu wrote:
> @@ -744,6 +773,9 @@ int arch_domain_create(struct domain *d,
> if ( (rc = domain_vpci_init(d)) != 0 )
> goto fail;
>
> + /* Copy and decode sve_vl from the domain configuration */
> + d->arch.sve_vl_bits = domainconfig_decode_vl(config->arch.sve_vl);
Considering that you now "encode" and "decode" the value when coming in /
going out for a hypercall, wouldn't it make sense to also have in internally
stored value in the same more compact format?
> --- a/xen/arch/arm/include/asm/arm64/sve.h
> +++ b/xen/arch/arm/include/asm/arm64/sve.h
> @@ -13,10 +13,23 @@
> /* Vector length must be multiple of 128 */
> #define SVE_VL_MULTIPLE_VAL (128U)
>
> +static inline bool is_vl_valid(uint16_t vl)
> +{
> + /* SVE vector length is multiple of 128 and maximum 2048 */
> + return ((vl % SVE_VL_MULTIPLE_VAL) == 0) && (vl <= SVE_VL_MAX_BITS);
> +}
> +
> +static inline uint16_t domainconfig_decode_vl(uint8_t sve_vl)
> +{
> + /* SVE vector length is stored as VL/128 in xen_arch_domainconfig */
> + return sve_vl * SVE_VL_MULTIPLE_VAL;
> +}
> +
> #ifdef CONFIG_ARM64_SVE
>
> register_t compute_max_zcr(void);
> register_t vl_to_zcr(uint16_t vl);
> +uint16_t get_sys_vl_len(void);
>
> #else /* !CONFIG_ARM64_SVE */
>
> @@ -30,6 +43,11 @@ static inline register_t vl_to_zcr(uint16_t vl)
> return 0;
> }
>
> +static inline uint16_t get_sys_vl_len(void)
> +{
> + return 0;
> +}
Throughout here: Style - please limit the use of fixed width types to
cases where they're actually necessary to use to achieve a certain
effect (see ./CODING_STYLE). None of the cases above look to match that
criteria, merely ...
> @@ -114,6 +116,9 @@ struct arch_domain
> void *tee;
> #endif
>
> + /* max SVE vector length in bits */
> + uint16_t sve_vl_bits;
... this may be justified (for space efficiency), while ...
> +
> } __cacheline_aligned;
(nit: stray insertion of a blank line)
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -300,6 +300,8 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t);
> struct xen_arch_domainconfig {
> /* IN/OUT */
> uint8_t gic_version;
> + /* IN - Contains SVE vector length divided by 128 */
> + uint8_t sve_vl;
... in the public interface it's of course mandatory to use.
Jan
