From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43440) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4GFP-000292-HN for qemu-devel@nongnu.org; Wed, 02 Dec 2015 17:51:40 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a4GFO-00016W-Nd for qemu-devel@nongnu.org; Wed, 02 Dec 2015 17:51:39 -0500 Message-ID: <565F75F4.1080402@gmail.com> Date: Wed, 02 Dec 2015 17:51:32 -0500 From: Michael Davidsaver MIME-Version: 1.0 References: <1447031505-12477-1-git-send-email-mdavidsaver@gmail.com> <1447031505-12477-2-git-send-email-mdavidsaver@gmail.com> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 01/18] armv7m: MRS/MSR handle unprivileged access List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Peter Crosthwaite , qemu-arm@nongnu.org, QEMU Developers On 11/17/2015 12:09 PM, Peter Maydell wrote: > On 9 November 2015 at 01:11, Michael Davidsaver wrote: >> The MRS and MSR instruction handling isn't checking >> the current permission level. >> >> Signed-off-by: Michael Davidsaver >> --- >> target-arm/helper.c | 79 +++++++++++++++++++++++++---------------------------- >> 1 file changed, 37 insertions(+), 42 deletions(-) > > This patch looks good overall, but there's one style nit: > >> + case 0 ... 7: /* xPSR sub-fields */ >> + mask = 0; >> + if ((reg&1) && el) { > > you want spaces around operators, so "reg & 1" here and elsewhere. Would be nice if checkpatch.pl caught these, but I understand that this would be quite difficult to do well. I've tried to catch this with grep and sort through the false positives. I think I got them all. > It would also be good to mention in the commit message the > other things this patch is fixing: > * privileged attempts to write EPSR should do nothing > * accessing an unknown special register now triggers a > guest-error warning rather than aborting QEMU Will do.