From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42874) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VCpEA-0003Hr-87 for qemu-devel@nongnu.org; Fri, 23 Aug 2013 07:08:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VCpE4-00043H-Pc for qemu-devel@nongnu.org; Fri, 23 Aug 2013 07:08:26 -0400 Received: from cantor2.suse.de ([195.135.220.15]:40206 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VCpE4-00041J-FN for qemu-devel@nongnu.org; Fri, 23 Aug 2013 07:08:20 -0400 Message-ID: <5217429F.4060606@suse.de> Date: Fri, 23 Aug 2013 13:08:15 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1377244791-56856-1-git-send-email-leon.alrae@imgtec.com> In-Reply-To: <1377244791-56856-1-git-send-email-leon.alrae@imgtec.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] mips/malta: prevent writes to reset flash mapping faulting List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Leon Alrae Cc: james.hogan@imgtec.com, paul.burton@imgtec.com, qemu-devel@nongnu.org, yongbok.kim@imgtec.com, cristian.cuna@imgtec.com, aurelien@aurel32.net Am 23.08.2013 09:59, schrieb Leon Alrae: > From: James Hogan >=20 > Commit a427338 (mips_malta: correct reading MIPS revision at 0x1fc00010= ) > altered the behaviour of the monitor flash mapping at the reset address > by making it read only. However this causes data bus error exceptions > when it is written to since it is effectively unassigned memory for > writes. This isn't how the real hardware behaves. That memory can be > written to (even with the MFWR jumper not fitted) and the new value rea= d > back from, but it doesn't get written back to the monitor flash so is > volatile. >=20 > This is fixed by converting the bios copy from read only ram to a bios > device with a nop write callback. That sounds like a contradiction: The nop write will not have reads return the new value, will it? Why not just remove the _set_readonly and have it reloaded on reset for volatility? Anyway, having a MemoryRegionOps with just a .write looks dangerous, but I guess you've tested read to work. We had been seeing assertions elsewhere when either was missing. Regards, Andreas >=20 > Signed-off-by: James Hogan > Cc: Paul Burton > Cc: Leon Alrae > Cc: Aurelien Jarno > Signed-off-by: Leon Alrae > --- > hw/mips/mips_malta.c | 14 ++++++++++++-- > 1 files changed, 12 insertions(+), 2 deletions(-) >=20 > diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c > index f8d064c..9e721d3 100644 > --- a/hw/mips/mips_malta.c > +++ b/hw/mips/mips_malta.c > @@ -873,6 +873,16 @@ static void cpu_request_exit(void *opaque, int irq= , int level) > } > } > =20 > +static void monflash_copy_mem_write(void *opaque, hwaddr ram_addr, > + uint64_t val, unsigned size) > +{ > +} > + > +static const MemoryRegionOps monflash_copy_mem_ops =3D { > + .write =3D monflash_copy_mem_write, > + .endianness =3D DEVICE_NATIVE_ENDIAN, > +}; > + > static > void mips_malta_init(QEMUMachineInitArgs *args) > { > @@ -1043,13 +1053,13 @@ void mips_malta_init(QEMUMachineInitArgs *args) > * handled by an overlapping region as the resulting ROM code subp= age > * regions are not executable. > */ > - memory_region_init_ram(bios_copy, NULL, "bios.1fc", BIOS_SIZE); > + memory_region_init_rom_device(bios_copy, NULL, &monflash_copy_mem_= ops, NULL, > + "bios.1fc", BIOS_SIZE); > if (!rom_copy(memory_region_get_ram_ptr(bios_copy), > FLASH_ADDRESS, BIOS_SIZE)) { > memcpy(memory_region_get_ram_ptr(bios_copy), > memory_region_get_ram_ptr(bios), BIOS_SIZE); > } > - memory_region_set_readonly(bios_copy, true); > memory_region_add_subregion(system_memory, RESET_ADDRESS, bios_cop= y); > =20 > /* Board ID =3D 0x420 (Malta Board with CoreLV) */ >=20 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg