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. 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.
signature.asc
Description: OpenPGP digital signature