> On 20 Sep 2017, at 12:09, Dave Martin <dave.martin@xxxxxxxxxxxx> wrote: > > On Wed, Sep 20, 2017 at 10:59:55AM +0000, Alan Hayward wrote: >> (Resending without disclaimer) >> >>> On 31 Aug 2017, at 18:00, Dave Martin <Dave.Martin@xxxxxxx> wrote: >> >>> >>> +int sve_set_vector_length(struct task_struct *task, >>> + unsigned long vl, unsigned long flags) >>> +{ >>> + WARN_ON(task == current && preemptible()); >>> + >>> + if (flags & ~(unsigned long)(PR_SVE_VL_INHERIT | >>> + PR_SVE_SET_VL_ONEXEC)) >>> + return -EINVAL; >>> + >>> + if (!sve_vl_valid(vl)) >>> + return -EINVAL; >>> + >>> + /* >>> + * Clamp to the maximum vector length that VL-agnostic SVE code can >>> + * work with. A flag may be assigned in the future to allow setting >>> + * of larger vector lengths without confusing older software. >>> + */ >>> + if (vl > SVE_VL_ARCH_MAX) >>> + vl = SVE_VL_ARCH_MAX; >>> + >>> + vl = find_supported_vector_length(vl); >>> + >> >> >> Given, sve_set_vector_length is called when setting the vector length in >> PTRACE_SETREGSET, it looks to me like if you set VL to a value that’s not >> supported by the hardware, then it’s going to round down to the previous value. >> Is that correct? I’m not sure if that’s explained in the docs? > > Does this cover it? > > "On success, the calling thread's vector length is changed to the > largest value supported by the system that is less than or equal to vl." > > (For ptrace, I just cross-reference the PR_SVE_SET_VL behaviour, above.) For ptrace is it worth mentioning user should do a GET after a SET to confirm what VL value was actually set? > >> What happens if you give a vl value lower than the min supported value in the >> hardware? > > This is impossible, unless vl < SVE_VL_MIN (which is rejected explicitly > by the !sve_vl_valid() check in sve_set_vector_length()). > > The architecture required support for all power-of-two vector lengths > less than the maximum supported vector length, so by construction > SVE_VL_MIN is supported by all hardware. Ok, I’m happy with that. > > To be defensive, if we fail to detect support for SVE_VL_MIN, I set the > corresponding bit in sve_vq_map and WARN. This is just to help ensure > find_supported_vector_length doesn't fall off the end of sve_vq_map. > > > Does that sounds correct? There may be a clearer way of achieving this. > > Cheers > ---Dave > >> >> >>> +/* >>> + * All vector length selection from userspace comes through here. >>> + * We're on a slow path, so some sanity-checks are included. >>> + * If things go wrong there's a bug somewhere, but try to fall back to a >>> + * safe choice. >>> + */ >>> +static unsigned int find_supported_vector_length(unsigned int vl) >>> +{ >>> + int bit; >>> + int max_vl = sve_max_vl; >>> + >>> + if (WARN_ON(!sve_vl_valid(vl))) >>> + vl = SVE_VL_MIN; >>> + >>> + if (WARN_ON(!sve_vl_valid(max_vl))) >>> + max_vl = SVE_VL_MIN; >>> + >>> + if (vl > max_vl) >>> + vl = max_vl; >>> + >>> + bit = find_next_bit(sve_vq_map, SVE_VQ_MAX, >>> + vq_to_bit(sve_vq_from_vl(vl))); >>> + return sve_vl_from_vq(bit_to_vq(bit)); >>> +} >>> + >> >> >> Thanks, >> Alan. >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel