From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:35801) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gzHPM-0005V7-Sv for qemu-devel@nongnu.org; Thu, 28 Feb 2019 03:51:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gzHB8-00067e-6y for qemu-devel@nongnu.org; Thu, 28 Feb 2019 03:36:31 -0500 References: <20190226113915.20150-1-david@redhat.com> <20190226113915.20150-15-david@redhat.com> From: David Hildenbrand Message-ID: Date: Thu, 28 Feb 2019 09:36:20 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v1 14/33] s390x/tcg: Implement VECTOR LOAD MULTIPLE List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson , qemu-devel@nongnu.org Cc: qemu-s390x@nongnu.org, Cornelia Huck , Thomas Huth , Richard Henderson On 27.02.19 17:02, Richard Henderson wrote: > On 2/26/19 3:38 AM, David Hildenbrand wrote: >> Also fairly easy to implement. One issue we have is that exceptions wi= ll >> result in some vectors already being modified. At least handle it >> consistently per vector by using a temporary vector. Good enough for >> now, add a FIXME. >> >> Signed-off-by: David Hildenbrand >> --- >> target/s390x/insn-data.def | 2 ++ >> target/s390x/translate_vx.inc.c | 26 ++++++++++++++++++++++++++ >> 2 files changed, 28 insertions(+) >=20 > I suppose the fixme is good enough. For the record, I think you could = do the > check with just two loads -- the first and last quadword. After that, = none of > the other loads can fault, and you can store everything else into the > destination vectors as you read them. Aren't such approaches prone to races if other VCPUs invalidate page tables/TLB entries? (or am I messing up things and the MMU of this VCPU won't be touched while in this block and once we touched all applicable pages, it cannot fail anymore?) >=20 > Also missing for the fixme: MO_ALIGN{,_16}. Just like the other occurrence, I think MO_ALIGN would be wrong. "Setting the alignment hint to a non-zero value that doesn=E2=80=99t correspond to the alignment of the second operand may reduce performance on some models." >=20 > Reviewed-by: Richard Henderson >=20 Thanks! >=20 > r~ >=20 --=20 Thanks, David / dhildenb