From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38670) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFHAe-00036j-GD for qemu-devel@nongnu.org; Wed, 15 Jul 2015 03:32:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZFHAb-00072W-A1 for qemu-devel@nongnu.org; Wed, 15 Jul 2015 03:32:00 -0400 Received: from mxout-1k.itc.hs-rm.de ([195.72.102.133]:56627) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFHAb-00071q-1L for qemu-devel@nongnu.org; Wed, 15 Jul 2015 03:31:57 -0400 Message-ID: <55A60C68.1090308@hs-rm.de> Date: Wed, 15 Jul 2015 09:31:52 +0200 From: =?UTF-8?B?QWxleCBaw7xwa2U=?= MIME-Version: 1.0 References: <1436293553-15575-1-git-send-email-alexander.zuepke@hs-rm.de> <7a50cb9f-dd9b-482e-ae59-9ddec755b7cb@EXCHANGE-3K.hds.local> In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 6/6] ARM: enable PMSAv7-style MPU on Cortex-M3/M4 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: QEMU Developers Am 14.07.2015 um 19:49 schrieb Peter Maydell: > On 7 July 2015 at 19:25, Alex Zuepke wrote: > > A commit message that wasn't just the one-line summary would > be nice. Sometimes a patch really is trivial enough that it's > not worth describing in more than just a single line, but > those situations are the exception rather than the rule. OK. > [snip] >> @@ -406,6 +436,57 @@ static void nvic_writel(nvic_state *s, uint32_t offset, uint32_t value) >> qemu_log_mask(LOG_UNIMP, >> "NVIC: AUX fault status registers unimplemented\n"); >> break; >> + case 0xd94: /* MPU control register. */ >> + cpu = ARM_CPU(current_cpu); >> + if (cpu->pmsav7_dregion == 0) >> + break; >> + cpu->env.v7m.mpu_ctrl = value & 0x7; >> + if (cpu->env.v7m.mpu_ctrl & MPU_CTRL_ENABLE) >> + cpu->env.cp15.sctlr_ns |= SCTLR_M; >> + else >> + cpu->env.cp15.sctlr_ns &= ~SCTLR_M; >> + /* TODO: mimic MPU_CTRL_HFNMIENA */ > > That will be interesting to implement. > >> + if (cpu->env.v7m.mpu_ctrl & MPU_CTRL_PRIVDEFENA) >> + cpu->env.cp15.sctlr_ns |= SCTLR_BR; >> + else >> + cpu->env.cp15.sctlr_ns &= ~SCTLR_BR; > > This is kind of ugly. Wouldn't it be nicer to make the code > which tests for MMU-enabled and privdefena be M-profile aware? Yes, but I wanted to keep the impact as small as possible ... As a general question: is it OK to (mis-)use the existing cp15 register infrastructure for that, or should I put the MPU enable bits, region number, etc into dedicated MPU-related sub structures in CPUARMState? > [snip] >> @@ -445,6 +526,19 @@ static uint64_t nvic_sysreg_read(void *opaque, hwaddr addr, >> return val & 0xffff; >> } >> break; >> + case 0xda0 ... 0xdb7: /* MPU_RSAR and aliases. */ >> + cpu = ARM_CPU(current_cpu); >> + if (cpu->pmsav7_dregion == 0) >> + break; >> + if ((size == 2) && (offset & 7) == 0) { >> + val = cpu->env.pmsav7.drsr[cpu->env.cp15.c6_rgnr]; >> + return val & 0xffff; >> + } >> + if ((size == 2) && (offset & 7) == 2) { >> + val = cpu->env.pmsav7.dracr[cpu->env.cp15.c6_rgnr]; >> + return val & 0xffff; >> + } > > RASR is UNPREDICTABLE for non-word-size access, so we don't need > this at all. It's from ARM recommended sample code: http://infocenter.arm.com/help/topic/com.arm.doc.dui0553a/BIHHHDDJ.html Thanks a lot for the reviews! I'll send a new set of patches next week. Best regards Alex