On Wed, Aug 22, 2012 at 09:05:52AM +0200, Jan Kiszka wrote: > On 2012-08-22 08:47, Jan Kiszka wrote: > > On 2012-08-22 07:57, David Gibson wrote: > >> On Wed, Aug 22, 2012 at 07:55:31AM +0200, Alexander Graf wrote: > >>> > >>> On 22.08.2012, at 06:59, David Gibson wrote: > >>> > >>>> cpu_physical_memory_write_rom(), despite the name, can also be used to > >>>> write images into RAM - and will often be used that way if the machine > >>>> uses load_image_targphys() into RAM addresses. > >>>> > >>>> However, cpu_physical_memory_write_rom(), unlike cpu_physical_memory_rw() > >>>> does invalidate any cached TBs which might be affected by the region > >>>> written. > >>>> > >>>> This was breaking reset (under full emu) on the pseries machine - we > >>>> loaded > >>>> our firmware image into RAM, and while executing it rewrite the code at > >>>> the entry point (correctly causing a TB invalidate/refresh). When we > >>>> reset the firmware image was reloaded, but the TB from the rewrite was > >>>> still active and caused us to get an illegal instruction trap. > >>>> > >>>> This patch fixes the bug by duplicating the tb invalidate code from > >>>> cpu_physical_memory_rw() in cpu_physical_memory_write_rom(). > >>>> > >>>> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> > >>>> --- > >>>> exec.c | 7 +++++++ > >>>> 1 file changed, 7 insertions(+) > >>>> > >>>> diff --git a/exec.c b/exec.c > >>>> index 5834766..eff40d7 100644 > >>>> --- a/exec.c > >>>> +++ b/exec.c > >>>> @@ -3523,6 +3523,13 @@ void > >>>> cpu_physical_memory_write_rom(target_phys_addr_t addr, > >>>> /* ROM/RAM case */ > >>>> ptr = qemu_get_ram_ptr(addr1); > >>>> memcpy(ptr, buf, l); > >>>> + if (!cpu_physical_memory_is_dirty(addr1)) { > >>>> + /* invalidate code */ > >>>> + tb_invalidate_phys_page_range(addr1, addr1 + l, 0); > >>>> + /* set dirty bit */ > >>>> + cpu_physical_memory_set_dirty_flags( > >>>> + addr1, (0xff & ~CODE_DIRTY_FLAG)); > >>>> + } > >>> > >>> Can't we just call cpu_physical_memory_rw in the RAM case? The > >>> function only tries to not do MMIO accesses on ROM pages, right? > >> > >> Maybe. It's not clear at all to me what cases > >> cpu_physical_memory_write_rom() is supposed to be for, as opposed to > >> just using cpu_physical_memory_rw(). > > > > write_rom ignores write protection - that you usually find on ROMs. That > > makes no difference under KVM so far as there we lack read-only > > sections. But that will be fixed soon, patches are on the list. > > In fact, it does make a difference also for KVM mode as > cpu_physical_memory_rw works from userspace while the limitation only > affects guest code running under KVM control.
Ok, so IIUC, that means we do need the cpu_physical_memory_write_rom() version for load_image_targphys(), and so my original patch is basically the right fix. > Jan > > PS: I'm still facing a bogus Mail-Followup-To tag in your postings, > David, thus you easily fall from the To list on reply. Ah, yes, thanks for the reminder. I think I finally found the right option to stop mutt from doing that. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
signature.asc
Description: Digital signature