* [PATCH] kvm-userspace: x86: Support for breakpoints in ROM code
@ 2009-03-16 18:48 Jan Kiszka
2009-03-16 18:53 ` Jan Kiszka
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2009-03-16 18:48 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm-devel
Align qemu-kvm-x86 with upstream kvm support /wrt software breakpoint
support in ROM code.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
qemu/qemu-kvm-x86.c | 29 ++++++++++++++++++++++++-----
1 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/qemu/qemu-kvm-x86.c b/qemu/qemu-kvm-x86.c
index 06ef775..05a9c4c 100644
--- a/qemu/qemu-kvm-x86.c
+++ b/qemu/qemu-kvm-x86.c
@@ -680,12 +680,32 @@ void kvm_arch_cpu_reset(CPUState *env)
}
}
-int kvm_arch_insert_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint *bp)
+#ifdef KVM_CAP_SET_GUEST_DEBUG
+static int kvm_patch_opcode_byte(CPUState *env, target_ulong addr, uint8_t val)
{
- uint8_t int3 = 0xcc;
+ target_phys_addr_t phys_page_addr;
+ unsigned long pd;
+ uint8_t *ptr;
+
+ phys_page_addr = cpu_get_phys_page_debug(env, addr & TARGET_PAGE_MASK);
+ if (phys_page_addr == -1)
+ return -EINVAL;
+
+ pd = cpu_get_physical_page_desc(phys_page_addr);
+ if ((pd & ~TARGET_PAGE_MASK) != IO_MEM_RAM &&
+ (pd & ~TARGET_PAGE_MASK) != IO_MEM_ROM && !(pd & IO_MEM_ROMD))
+ return -EINVAL;
+ ptr = phys_ram_base + (pd & TARGET_PAGE_MASK)
+ + (addr & ~TARGET_PAGE_MASK);
+ *ptr = val;
+ return 0;
+}
+
+int kvm_arch_insert_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint *bp)
+{
if (cpu_memory_rw_debug(env, bp->pc, (uint8_t *)&bp->saved_insn, 1, 0) ||
- cpu_memory_rw_debug(env, bp->pc, &int3, 1, 1))
+ kvm_patch_opcode_byte(env, bp->pc, 0xcc))
return -EINVAL;
return 0;
}
@@ -695,12 +715,11 @@ int kvm_arch_remove_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint *bp)
uint8_t int3;
if (cpu_memory_rw_debug(env, bp->pc, &int3, 1, 0) || int3 != 0xcc ||
- cpu_memory_rw_debug(env, bp->pc, (uint8_t *)&bp->saved_insn, 1, 1))
+ kvm_patch_opcode_byte(env, bp->pc, bp->saved_insn))
return -EINVAL;
return 0;
}
-#ifdef KVM_CAP_SET_GUEST_DEBUG
static struct {
target_ulong addr;
int len;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] kvm-userspace: x86: Support for breakpoints in ROM code
2009-03-16 18:48 [PATCH] kvm-userspace: x86: Support for breakpoints in ROM code Jan Kiszka
@ 2009-03-16 18:53 ` Jan Kiszka
2009-03-17 9:33 ` Avi Kivity
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2009-03-16 18:53 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm-devel
Jan Kiszka wrote:
> Align qemu-kvm-x86 with upstream kvm support /wrt software breakpoint
> support in ROM code.
Hmm, this might not be needed as kvm-userspace is not protecting its
ROM. That's why I didn't pushed this so far. However, aligning isn't
bad. But dropping this duplication would be better...
Jan
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>
> qemu/qemu-kvm-x86.c | 29 ++++++++++++++++++++++++-----
> 1 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/qemu/qemu-kvm-x86.c b/qemu/qemu-kvm-x86.c
> index 06ef775..05a9c4c 100644
> --- a/qemu/qemu-kvm-x86.c
> +++ b/qemu/qemu-kvm-x86.c
> @@ -680,12 +680,32 @@ void kvm_arch_cpu_reset(CPUState *env)
> }
> }
>
> -int kvm_arch_insert_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint *bp)
> +#ifdef KVM_CAP_SET_GUEST_DEBUG
> +static int kvm_patch_opcode_byte(CPUState *env, target_ulong addr, uint8_t val)
> {
> - uint8_t int3 = 0xcc;
> + target_phys_addr_t phys_page_addr;
> + unsigned long pd;
> + uint8_t *ptr;
> +
> + phys_page_addr = cpu_get_phys_page_debug(env, addr & TARGET_PAGE_MASK);
> + if (phys_page_addr == -1)
> + return -EINVAL;
> +
> + pd = cpu_get_physical_page_desc(phys_page_addr);
> + if ((pd & ~TARGET_PAGE_MASK) != IO_MEM_RAM &&
> + (pd & ~TARGET_PAGE_MASK) != IO_MEM_ROM && !(pd & IO_MEM_ROMD))
> + return -EINVAL;
>
> + ptr = phys_ram_base + (pd & TARGET_PAGE_MASK)
> + + (addr & ~TARGET_PAGE_MASK);
> + *ptr = val;
> + return 0;
> +}
> +
> +int kvm_arch_insert_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint *bp)
> +{
> if (cpu_memory_rw_debug(env, bp->pc, (uint8_t *)&bp->saved_insn, 1, 0) ||
> - cpu_memory_rw_debug(env, bp->pc, &int3, 1, 1))
> + kvm_patch_opcode_byte(env, bp->pc, 0xcc))
> return -EINVAL;
> return 0;
> }
> @@ -695,12 +715,11 @@ int kvm_arch_remove_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint *bp)
> uint8_t int3;
>
> if (cpu_memory_rw_debug(env, bp->pc, &int3, 1, 0) || int3 != 0xcc ||
> - cpu_memory_rw_debug(env, bp->pc, (uint8_t *)&bp->saved_insn, 1, 1))
> + kvm_patch_opcode_byte(env, bp->pc, bp->saved_insn))
> return -EINVAL;
> return 0;
> }
>
> -#ifdef KVM_CAP_SET_GUEST_DEBUG
> static struct {
> target_ulong addr;
> int len;
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kvm-userspace: x86: Support for breakpoints in ROM code
2009-03-16 18:53 ` Jan Kiszka
@ 2009-03-17 9:33 ` Avi Kivity
2009-03-17 9:44 ` Jan Kiszka
0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2009-03-17 9:33 UTC (permalink / raw)
To: Jan Kiszka; +Cc: kvm-devel
Jan Kiszka wrote:
> Jan Kiszka wrote:
>
>> Align qemu-kvm-x86 with upstream kvm support /wrt software breakpoint
>> support in ROM code.
>>
>
> Hmm, this might not be needed as kvm-userspace is not protecting its
> ROM. That's why I didn't pushed this so far. However, aligning isn't
> bad. But dropping this duplication would be better...
>
Aligning is a good thing (and duplication can be fixed in upstream and
merged here). But maybe cpu_memory_rw_debug() should use
cpu_physical_memory_write_rom() to write, instead of this hack?
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kvm-userspace: x86: Support for breakpoints in ROM code
2009-03-17 9:33 ` Avi Kivity
@ 2009-03-17 9:44 ` Jan Kiszka
2009-03-17 9:51 ` Avi Kivity
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2009-03-17 9:44 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm-devel
Avi Kivity wrote:
> Jan Kiszka wrote:
>> Jan Kiszka wrote:
>>
>>> Align qemu-kvm-x86 with upstream kvm support /wrt software breakpoint
>>> support in ROM code.
>>>
>>
>> Hmm, this might not be needed as kvm-userspace is not protecting its
>> ROM. That's why I didn't pushed this so far. However, aligning isn't
>> bad. But dropping this duplication would be better...
>>
>
> Aligning is a good thing (and duplication can be fixed in upstream and
> merged here). But maybe cpu_memory_rw_debug() should use
> cpu_physical_memory_write_rom() to write, instead of this hack?
This is not a hack (it shouldn't have been merged upstream otherwise):
cpu_physical_memory_write_rom() takes system-wide physical addresses
while kvm_patch_opcode_byte() works with per-CPU linear addresses.
And IMHO, the duplication needs to be fixed here by switching to
upstream's kvm layer - at some point in the future. As a first step, we
may have to look in making them compatible so that you can use the both
layers at the same time.
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kvm-userspace: x86: Support for breakpoints in ROM code
2009-03-17 9:44 ` Jan Kiszka
@ 2009-03-17 9:51 ` Avi Kivity
2009-03-17 10:43 ` Jan Kiszka
0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2009-03-17 9:51 UTC (permalink / raw)
To: Jan Kiszka; +Cc: kvm-devel
Jan Kiszka wrote:
> This is not a hack (it shouldn't have been merged upstream otherwise):
> cpu_physical_memory_write_rom() takes system-wide physical addresses
> while kvm_patch_opcode_byte() works with per-CPU linear addresses.
>
From exec.c:
> /* virtual memory access for debug */
> int cpu_memory_rw_debug(CPUState *env, target_ulong addr,
> uint8_t *buf, int len, int is_write)
> {
> int l;
> target_phys_addr_t phys_addr;
> target_ulong page;
>
> while (len > 0) {
> page = addr & TARGET_PAGE_MASK;
> phys_addr = cpu_get_phys_page_debug(env, page);
> /* if no physical page mapped, return an error */
> if (phys_addr == -1)
> return -1;
> l = (page + TARGET_PAGE_SIZE) - addr;
> if (l > len)
> l = len;
> cpu_physical_memory_rw(phys_addr + (addr & ~TARGET_PAGE_MASK),
> buf, l, is_write);
> len -= l;
> buf += l;
> addr += l;
> }
> return 0;
> }
I'm talking about replacing cpu_physical_memory_rw() with
cpu_physical_memory_write_rom() (for the write case). Is there a case
where the debugger should be prevented from writing into ROM? If so,
maybe cpu_memory_rw_debug_rom() for breakpoints?
We shouldn't juggle page descriptors in *kvm*.c.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kvm-userspace: x86: Support for breakpoints in ROM code
2009-03-17 9:51 ` Avi Kivity
@ 2009-03-17 10:43 ` Jan Kiszka
0 siblings, 0 replies; 6+ messages in thread
From: Jan Kiszka @ 2009-03-17 10:43 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm-devel
Avi Kivity wrote:
> Jan Kiszka wrote:
>> This is not a hack (it shouldn't have been merged upstream otherwise):
>> cpu_physical_memory_write_rom() takes system-wide physical addresses
>> while kvm_patch_opcode_byte() works with per-CPU linear addresses.
>>
>
> From exec.c:
>
>> /* virtual memory access for debug */
>> int cpu_memory_rw_debug(CPUState *env, target_ulong addr,
>> uint8_t *buf, int len, int is_write)
>> {
>> int l;
>> target_phys_addr_t phys_addr;
>> target_ulong page;
>>
>> while (len > 0) {
>> page = addr & TARGET_PAGE_MASK;
>> phys_addr = cpu_get_phys_page_debug(env, page);
>> /* if no physical page mapped, return an error */
>> if (phys_addr == -1)
>> return -1;
>> l = (page + TARGET_PAGE_SIZE) - addr;
>> if (l > len)
>> l = len;
>> cpu_physical_memory_rw(phys_addr + (addr & ~TARGET_PAGE_MASK),
>> buf, l, is_write);
>> len -= l;
>> buf += l;
>> addr += l;
>> }
>> return 0;
>> }
>
> I'm talking about replacing cpu_physical_memory_rw() with
> cpu_physical_memory_write_rom() (for the write case).
Ah, miss-read your point.
> Is there a case
> where the debugger should be prevented from writing into ROM? If so,
> maybe cpu_memory_rw_debug_rom() for breakpoints?
Good and valid question. So far only the 'M' gdb packet uses
cpu_physical_memory_rw in write mode, and I don't know if there is any
reason to deny modifying ROM code that way. Hmm, will check with
upstream and change there first. So forget about this patch for now.
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-03-17 10:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-03-16 18:48 [PATCH] kvm-userspace: x86: Support for breakpoints in ROM code Jan Kiszka
2009-03-16 18:53 ` Jan Kiszka
2009-03-17 9:33 ` Avi Kivity
2009-03-17 9:44 ` Jan Kiszka
2009-03-17 9:51 ` Avi Kivity
2009-03-17 10:43 ` Jan Kiszka
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.