[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v5 02/12] xen/arm: add SVE vector length field to the domain
> On 13 Apr 2023, at 20:52, Julien Grall <julien@xxxxxxx> wrote: > > Hi Luca, > > On 13/04/2023 15:05, Luca Fancellu wrote: >>> On 13 Apr 2023, at 14:30, Julien Grall <julien@xxxxxxx> wrote: >>> >>> >>> >>> On 13/04/2023 14:24, Luca Fancellu wrote: >>>> Hi Julien, >>> >>> Hi Luca, >>> >>>>>> @@ -594,6 +597,7 @@ int arch_sanitise_domain_config(struct >>>>>> xen_domctl_createdomain *config) >>>>>> unsigned int max_vcpus; >>>>>> unsigned int flags_required = (XEN_DOMCTL_CDF_hvm | >>>>>> XEN_DOMCTL_CDF_hap); >>>>>> unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | >>>>>> XEN_DOMCTL_CDF_vpmu); >>>>>> + unsigned int sve_vl_bits = sve_decode_vl(config->arch.sve_vl); >>>>>> if ( (config->flags & ~flags_optional) != flags_required ) >>>>>> { >>>>>> @@ -602,6 +606,26 @@ int arch_sanitise_domain_config(struct >>>>>> xen_domctl_createdomain *config) >>>>>> return -EINVAL; >>>>>> } >>>>>> + /* Check feature flags */ >>>>>> + if ( sve_vl_bits > 0 ) >>>>>> + { >>>>>> + unsigned int zcr_max_bits = get_sys_vl_len(); >>>>>> + >>>>>> + if ( !zcr_max_bits ) >>>>>> + { >>>>>> + dprintk(XENLOG_INFO, "SVE is unsupported on this >>>>>> machine.\n"); >>>>>> + return -EINVAL; >>>>>> + } >>>>>> + >>>>>> + if ( sve_vl_bits > zcr_max_bits ) >>>>>> + { >>>>>> + dprintk(XENLOG_INFO, >>>>>> + "Requested SVE vector length (%u) > supported >>>>>> length (%u)\n", >>>>>> + sve_vl_bits, zcr_max_bits); >>>>>> + return -EINVAL; >>>>>> + } >>>>> >>>>> Is SVE supported for 32-bit guest? If not, then you should had a check >>>>> here to prevent the creation of the domain if sve_vl_bits is set. >>>> No SVE is not supported for 32 bit guests, here I think we will get “SVE >>>> is unsupported on this machine” because get_sys_vl_len() will return 0. >>> >>> From my understanding, get_sys_vl_len() will return the len supported by >>> the hosts. So if you run a 32-bit guest on top of a 64-bit hosts, then I >>> believe get_sys_vl_len() will be non-zero. >> Yes you are right, I realise that I need the domain type information and I >> can’t have it in arch_sanitise_domain_config, however they might have sense >> there, and I can do a check >> like this afterwards: >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> index c1f0d1d78431..ce1235c25769 100644 >> --- a/xen/arch/arm/domain_build.c >> +++ b/xen/arch/arm/domain_build.c >> @@ -3694,6 +3694,12 @@ static int __init construct_domain(struct domain *d, >> struct kernel_info *kinfo) >> return -EINVAL; >> } >> + if ( d->arch.sve_vl && (kinfo->type == DOMAIN_32BIT) ) >> + { >> + printk("SVE is not available for 32-bit domain\n"); >> + return -EINVAL; >> + } >> + >> if ( is_64bit_domain(d) ) >> vcpu_switch_to_aarch64_mode(v); >> Would it be ok for you? > > construct_domain() is only going to be used for domains created by Xen. You > would need the same check for the ones created by the toolstack. > > Do you need to know the SVE length when the domain is created? If not, then I > would suggest to create a new domctl that would be called after we switch the > domain to 32-bit. Hi Julien, Yes that’s true, we would like to prevent who is using hyper calls to have guests with SVE but 32 bits, I think that with this check it’s possible to avoid them: diff --git a/xen/arch/arm/arm64/domctl.c b/xen/arch/arm/arm64/domctl.c index 0de89b42c448..b7189e8dbbb5 100644 --- a/xen/arch/arm/arm64/domctl.c +++ b/xen/arch/arm/arm64/domctl.c @@ -43,6 +43,9 @@ long subarch_do_domctl(struct xen_domctl *domctl, struct domain *d, case 32: if ( !cpu_has_el1_32 ) return -EINVAL; + /* SVE is not supported for 32 bit domain */ + if ( is_sve_domain(d) ) + return -EOPNOTSUPP; return switch_mode(d, DOMAIN_32BIT); case 64: return switch_mode(d, DOMAIN_64BIT); It’s a bit late in the guest creation, but we don’t have the domain type information before, this check together with the check above in construct_domain would be enough. What do you think? > > Cheers, > > -- > Julien Grall
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |