[Qemu-devel] [PATCH] cpu_physical_memory_write_rom() needs to do TB invalidates
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() doesn't 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(+)
I've sent this before, and there was some discussion about the
details, but as far as I can tell the conclusion was that this patch
was as good as any, at least for fixing the existing bug in 1.2 /
stable. However, it still hasn't been merged.
Please apply for stable as well as mainline.
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));
+}
qemu_put_ram_ptr(ptr);
}
len -= l;
--
1.7.10.4
Re: [Qemu-devel] [PATCH] cpu_physical_memory_write_rom() needs to do TB invalidates
On 22.08.2012, at 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().
I can only guess, but the code looks to me as if it wants to be a nop on ROM
pages, while basically doing cpu_physical_memory_rw for RAM pages. Usually in
QEMU, every non-RAM page gets treated as MMIO which might eventually lead to
machine checks.
Alex
Re: [Qemu-devel] [PATCH] cpu_physical_memory_write_rom() needs to do TB invalidates
On Wed, Aug 22, 2012 at 08:02:11AM +0200, Alexander Graf wrote:
On 22.08.2012, at 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().
I can only guess, but the code looks to me as if it wants to be a
nop on ROM pages, while basically doing cpu_physical_memory_rw for
RAM pages. Usually in QEMU, every non-RAM page gets treated as MMIO
which might eventually lead to machine checks.
Maybe. Anthony, can you make a ruling on this, or tell me who can. I
don't really care how I fix it, but it's definitely broken right now.
--
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
Re: [Qemu-devel] [PATCH] cpu_physical_memory_write_rom() needs to do TB invalidates
On 22.08.2012, at 08:10, David Gibson wrote:
On Wed, Aug 22, 2012 at 08:02:11AM +0200, Alexander Graf wrote:
On 22.08.2012, at 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().
I can only guess, but the code looks to me as if it wants to be a
nop on ROM pages, while basically doing cpu_physical_memory_rw for
RAM pages. Usually in QEMU, every non-RAM page gets treated as MMIO
which might eventually lead to machine checks.
Maybe. Anthony, can you make a ruling on this, or tell me who can. I
don't really care how I fix it, but it's definitely broken right now.
Yeah, and for a 1.2 quick fix your original patch should be perfectly fine too.
Alex
Re: [Qemu-devel] [PATCH] cpu_physical_memory_write_rom() needs to do TB invalidates
On 22.08.2012, at 08:10, David Gibson wrote:
On Wed, Aug 22, 2012 at 08:02:11AM +0200, Alexander Graf wrote:
On 22.08.2012, at 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().
I can only guess, but the code looks to me as if it wants to be a
nop on ROM pages, while basically doing cpu_physical_memory_rw for
RAM pages. Usually in QEMU, every non-RAM page gets treated as MMIO
which might eventually lead to machine checks.
Maybe. Anthony, can you make a ruling on this, or tell me who can. I
don't really care how I fix it, but it's definitely broken right now.
Also, does tb_invalidate_phys_page_range() do an icache flush on KVM?
Alex
Re: [Qemu-devel] [PATCH] cpu_physical_memory_write_rom() needs to do TB invalidates
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.
Jan
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] cpu_physical_memory_write_rom() needs to do TB invalidates
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
Re: [Qemu-devel] [PATCH] cpu_physical_memory_write_rom() needs to do TB invalidates
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
Re: [Qemu-devel] [PATCH] cpu_physical_memory_write_rom() needs to do TB invalidates
On 22.08.2012, at 13:38, David Gibson wrote:
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.
Sure it is, I don't think anyone argued about that :). But it's duplicating
code in a slow path. So my proposal was instead of doing the write manually in
the this is read-write RAM case, just fall back to the known-to-work
cpu_physical_memory_rw for those pages. That would make the rom function
smaller, more obvious and duplicate less code.
Alex
Re: [Qemu-devel] [PATCH] cpu_physical_memory_write_rom() needs to do TB invalidates
On 08/22/2012 02:47 PM, Alexander Graf wrote: 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. Sure it is, I don't think anyone argued about that :). But it's duplicating code in a slow path. So my proposal was instead of doing the write manually in the this is read-write RAM case, just fall back to the known-to-work cpu_physical_memory_rw for those pages. That would make the rom function smaller, more obvious and duplicate less code. I think there were patches (from Xen) extracting that snippet into a helper. -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH] cpu_physical_memory_write_rom() needs to do TB invalidates
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));
+}
qemu_put_ram_ptr(ptr);
}
len -= l;
--
1.7.10.4
Re: [Qemu-devel] [PATCH] cpu_physical_memory_write_rom() needs to do TB invalidates
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?
Alex
Re: [Qemu-devel] [PATCH] cpu_physical_memory_write_rom() needs to do TB invalidates
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().
--
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
