All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Richard Henderson <richard.henderson@linaro.org>,
	qemu-arm <qemu-arm@nongnu.org>,
	QEMU Developers <qemu-devel@nongnu.org>
Subject: Re: [Qemu-arm] [Qemu-devel] [PATCH v4 1/5] linux-user: Implement aarch64 PR_SVE_SET/GET_VL
Date: Tue, 06 Mar 2018 14:03:27 +0000	[thread overview]
Message-ID: <877eqpti6o.fsf@linaro.org> (raw)
In-Reply-To: <CAFEAcA8hNzeCSe2DNwUfzeKOSonHY5uoc31m68F2YQsvcyjJLQ@mail.gmail.com>


Peter Maydell <peter.maydell@linaro.org> writes:

> On 6 March 2018 at 12:28, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Richard Henderson <richard.henderson@linaro.org> writes:
>>
>>> As an implementation choice, widening VL has zeroed the
>>> previously inaccessible portion of the sve registers.
>>>
>>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>
>>> +                int old_vq = (env->vfp.zcr_el[1] & 0xf) + 1;
>>> +                int vq = MAX(arg2 / 16, 1);
>>> +
>>> +                if (vq < old_vq) {
>>> +                    aarch64_sve_narrow_vq(env, vq);
>>> +                }
>>> +                env->vfp.zcr_el[1] = vq - 1;
>>
>> It seems odd not to have setting this inside cpu64.c. Won't a similar
>> manipulation need to be made for system mode? I'd keep all the logic
>> together in aarch64_sve_narrow_vq (or maybe call it aarch64_sve_set_vq
>> and pass it the current exception level).
>
> I think I asked Richard to put it into linux-user because it was
> in target/arm in an earlier version of this series. The manipulation
> that's happening here is kind of linux-specific (if it were for
> system mode we'd need to think about ZCR_EL2 and ZCR_EL3 as well),
> and the analogy is with cpu_set_tls/cpu_get_tls which are in
> linux-user/arm/target_cpu.h.

Fair enough.

>
> NB: I've already put this series in target-arm.next -- do you want
> me to drop them ? (That would mean they won't go in 2.12, given
> RTH is away.)

No it's fine. We can always fix up minor nits later when system mode is
done.

Acked-by: Alex Bennée <alex.bennee@linaro.org>

>
> thanks
> -- PMM


--
Alex Bennée

WARNING: multiple messages have this Message-ID (diff)
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Richard Henderson <richard.henderson@linaro.org>,
	qemu-arm <qemu-arm@nongnu.org>,
	QEMU Developers <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [Qemu-arm] [PATCH v4 1/5] linux-user: Implement aarch64 PR_SVE_SET/GET_VL
Date: Tue, 06 Mar 2018 14:03:27 +0000	[thread overview]
Message-ID: <877eqpti6o.fsf@linaro.org> (raw)
In-Reply-To: <CAFEAcA8hNzeCSe2DNwUfzeKOSonHY5uoc31m68F2YQsvcyjJLQ@mail.gmail.com>


Peter Maydell <peter.maydell@linaro.org> writes:

> On 6 March 2018 at 12:28, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Richard Henderson <richard.henderson@linaro.org> writes:
>>
>>> As an implementation choice, widening VL has zeroed the
>>> previously inaccessible portion of the sve registers.
>>>
>>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>
>>> +                int old_vq = (env->vfp.zcr_el[1] & 0xf) + 1;
>>> +                int vq = MAX(arg2 / 16, 1);
>>> +
>>> +                if (vq < old_vq) {
>>> +                    aarch64_sve_narrow_vq(env, vq);
>>> +                }
>>> +                env->vfp.zcr_el[1] = vq - 1;
>>
>> It seems odd not to have setting this inside cpu64.c. Won't a similar
>> manipulation need to be made for system mode? I'd keep all the logic
>> together in aarch64_sve_narrow_vq (or maybe call it aarch64_sve_set_vq
>> and pass it the current exception level).
>
> I think I asked Richard to put it into linux-user because it was
> in target/arm in an earlier version of this series. The manipulation
> that's happening here is kind of linux-specific (if it were for
> system mode we'd need to think about ZCR_EL2 and ZCR_EL3 as well),
> and the analogy is with cpu_set_tls/cpu_get_tls which are in
> linux-user/arm/target_cpu.h.

Fair enough.

>
> NB: I've already put this series in target-arm.next -- do you want
> me to drop them ? (That would mean they won't go in 2.12, given
> RTH is away.)

No it's fine. We can always fix up minor nits later when system mode is
done.

Acked-by: Alex Bennée <alex.bennee@linaro.org>

>
> thanks
> -- PMM


--
Alex Bennée

  reply	other threads:[~2018-03-06 14:03 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-03 14:38 [Qemu-devel] [PATCH v4 0/5] target/arm linux-user changes for sve Richard Henderson
2018-03-03 14:38 ` [Qemu-devel] [PATCH v4 1/5] linux-user: Implement aarch64 PR_SVE_SET/GET_VL Richard Henderson
2018-03-06 12:28   ` Alex Bennée
2018-03-06 12:58     ` [Qemu-arm] " Peter Maydell
2018-03-06 12:58       ` [Qemu-devel] [Qemu-arm] " Peter Maydell
2018-03-06 14:03       ` Alex Bennée [this message]
2018-03-06 14:03         ` Alex Bennée
2018-03-03 14:38 ` [Qemu-devel] [PATCH v4 2/5] aarch64-linux-user: Split out helpers for guest signal handling Richard Henderson
2018-03-06 14:11   ` Alex Bennée
2018-03-03 14:38 ` [Qemu-devel] [PATCH v4 3/5] aarch64-linux-user: Remove struct target_aux_context Richard Henderson
2018-03-06 14:16   ` Alex Bennée
2018-03-03 14:38 ` [Qemu-arm] [PATCH v4 4/5] aarch64-linux-user: Add support for EXTRA signal frame records Richard Henderson
2018-03-03 14:38   ` [Qemu-devel] " Richard Henderson
2018-03-06 14:26   ` [Qemu-arm] " Alex Bennée
2018-03-06 14:26     ` [Qemu-devel] " Alex Bennée
2018-03-03 14:38 ` [Qemu-arm] [PATCH v4 5/5] aarch64-linux-user: Add support for SVE " Richard Henderson
2018-03-03 14:38   ` [Qemu-devel] " Richard Henderson
2018-03-05 15:44   ` [Qemu-arm] " Peter Maydell
2018-03-05 15:44     ` [Qemu-devel] " Peter Maydell
2018-03-06 14:27   ` Alex Bennée
2018-03-06 14:27     ` [Qemu-devel] " Alex Bennée
2018-03-05 15:42 ` [Qemu-arm] [Qemu-devel] [PATCH v4 0/5] target/arm linux-user changes for sve Peter Maydell
2018-03-05 15:42   ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=877eqpti6o.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.