From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59296) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uz4RR-0004F7-N7 for qemu-devel@nongnu.org; Tue, 16 Jul 2013 08:33:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Uz4RM-00068v-Rg for qemu-devel@nongnu.org; Tue, 16 Jul 2013 08:33:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:20834) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uz4RM-00068b-EH for qemu-devel@nongnu.org; Tue, 16 Jul 2013 08:33:12 -0400 Message-ID: <51E53D7E.4060002@redhat.com> Date: Tue, 16 Jul 2013 14:33:02 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1369947836-2638-1-git-send-email-pbonzini@redhat.com> <1369947836-2638-7-git-send-email-pbonzini@redhat.com> <51E53C62.3070204@siemens.com> In-Reply-To: <51E53C62.3070204@siemens.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: qemu-devel@nongnu.org, Stefan Hajnoczi Il 16/07/2013 14:28, Jan Kiszka ha scritto: > On 2013-05-30 23:03, Paolo Bonzini wrote: >> This provides the basics for detecting accesses to unassigned memory >> as soon as they happen, and also for a simple implementation of >> address_space_access_valid. >> >> Reviewed-by: Richard Henderson >> Signed-off-by: Paolo Bonzini >> --- >> exec.c | 36 ++++++++++++------------------------ >> memory.c | 28 ++++++++++++++++++++++++++-- >> 2 files changed, 38 insertions(+), 26 deletions(-) >> >> diff --git a/exec.c b/exec.c >> index 785eeeb..c5100d6 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -1383,32 +1383,14 @@ ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr) >> return ram_addr; >> } >> >> -static uint64_t unassigned_mem_read(void *opaque, hwaddr addr, >> - unsigned size) >> +static bool unassigned_mem_accepts(void *opaque, hwaddr addr, >> + unsigned size, bool is_write) >> { >> -#ifdef DEBUG_UNASSIGNED >> - printf("Unassigned mem read " TARGET_FMT_plx "\n", addr); >> -#endif >> -#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE) >> - cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size); >> -#endif >> - return 0; >> -} >> - >> -static void unassigned_mem_write(void *opaque, hwaddr addr, >> - uint64_t val, unsigned size) >> -{ >> -#ifdef DEBUG_UNASSIGNED >> - printf("Unassigned mem write " TARGET_FMT_plx " = 0x%"PRIx64"\n", addr, val); >> -#endif >> -#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE) >> - cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size); >> -#endif >> + return false; >> } >> >> -static const MemoryRegionOps unassigned_mem_ops = { >> - .read = unassigned_mem_read, >> - .write = unassigned_mem_write, >> +const MemoryRegionOps unassigned_mem_ops = { >> + .valid.accepts = unassigned_mem_accepts, >> .endianness = DEVICE_NATIVE_ENDIAN, >> }; >> >> @@ -1442,9 +1424,15 @@ static void notdirty_mem_write(void *opaque, hwaddr ram_addr, >> tlb_set_dirty(cpu_single_env, cpu_single_env->mem_io_vaddr); >> } >> >> +static bool notdirty_mem_accepts(void *opaque, hwaddr addr, >> + unsigned size, bool is_write) >> +{ >> + return is_write; >> +} >> + >> static const MemoryRegionOps notdirty_mem_ops = { >> - .read = unassigned_mem_read, >> .write = notdirty_mem_write, >> + .valid.accepts = notdirty_mem_accepts, >> .endianness = DEVICE_NATIVE_ENDIAN, >> }; >> >> diff --git a/memory.c b/memory.c >> index 99f046d..15da877 100644 >> --- a/memory.c >> +++ b/memory.c >> @@ -814,6 +814,29 @@ void memory_region_init(MemoryRegion *mr, >> mr->flush_coalesced_mmio = false; >> } >> >> +static uint64_t unassigned_mem_read(void *opaque, hwaddr addr, >> + unsigned size) >> +{ >> +#ifdef DEBUG_UNASSIGNED >> + printf("Unassigned mem read " TARGET_FMT_plx "\n", addr); >> +#endif >> +#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE) >> + cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size); >> +#endif >> + return 0; > > This changed the value read from unassigned memory from -1 to 0. Any > particular reason or an unintentional change? Cut-and-paste (unassigned RAM used to return 0, invalid MMIO used to return -1, unifying the paths dropped the difference). I guess unassigned RAM can read as -1 just fine, so we can just change unassigned_mem_read to return -1. Paolo > Jan > >> +} >> + >> +static void unassigned_mem_write(void *opaque, hwaddr addr, >> + uint64_t val, unsigned size) >> +{ >> +#ifdef DEBUG_UNASSIGNED >> + printf("Unassigned mem write " TARGET_FMT_plx " = 0x%"PRIx64"\n", addr, val); >> +#endif >> +#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || defined(TARGET_MICROBLAZE) >> + cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size); >> +#endif >> +} >> + >> static bool memory_region_access_valid(MemoryRegion *mr, >> hwaddr addr, >> unsigned size, >> @@ -847,7 +870,7 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion *mr, >> uint64_t data = 0; >> >> if (!memory_region_access_valid(mr, addr, size, false)) { >> - return -1U; /* FIXME: better signalling */ >> + return unassigned_mem_read(mr, addr, size); >> } >> >> if (!mr->ops->read) { >> @@ -898,7 +921,8 @@ static void memory_region_dispatch_write(MemoryRegion *mr, >> unsigned size) >> { >> if (!memory_region_access_valid(mr, addr, size, true)) { >> - return; /* FIXME: better signalling */ >> + unassigned_mem_write(mr, addr, data, size); >> + return; >> } >> >> adjust_endianness(mr, &data, size); >>