Florian Hofhammer <[email protected]> writes:

> On 16/12/2025 10:27, Florian Hofhammer wrote:
>>>> diff --git a/plugins/api.c b/plugins/api.c
>>>> index eac04cc1f6..fc19bdb40b 100644
>>>> --- a/plugins/api.c
>>>> +++ b/plugins/api.c
>>>> @@ -41,6 +41,7 @@
>>>>  #include "qemu/log.h"
>>>>  #include "system/memory.h"
>>>>  #include "tcg/tcg.h"
>>>> +#include "exec/cpu-common.h"
>>>>  #include "exec/gdbstub.h"
>>>>  #include "exec/target_page.h"
>>>>  #include "exec/translation-block.h"
>>>> @@ -450,13 +451,27 @@ int qemu_plugin_write_register(struct 
>>>> qemu_plugin_register *reg,
>>>>  {
>>>>      g_assert(current_cpu);
>>>>  
>>>> -    if (buf->len == 0 || qemu_plugin_get_cb_flags() != 
>>>> QEMU_PLUGIN_CB_RW_REGS) {
>>>> +    if (buf->len == 0 || (
>>>> +                qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS
>>>> +                && qemu_plugin_get_cb_flags() != 
>>>> QEMU_PLUGIN_CB_RW_REGS_PC)) {
>>>>          return -1;
>>>>      }
>>>
>>> If we are exposing a specific qemu_plugin_set_pc we should probably
>>> forbid setting it via write_register. Can we filter out the PC from the
>>> register list?
>> 
>> The qemu_plugin_write_register API silently swallows writes to the PC
>> even though such a write doesn't actually have an effect, so excluding
>> the PC here might make sense and I'm happy to update the patch
>> accordingly.
>> Are there other registers that should be excluded as well?
>> General-purpose register writes are visible to the guest immediately,
>> but what about special registers? Do writes to those actually always
>> have the intended effect or should they also be excluded here?
>
> Actually, after looking into this for a bit, I don't think it's easily
> feasible without a big revamp (but please correct me if I'm wrong or
> simply missing something).
>
> The problem is that the opaque handle passed to the plugin API only
> encodes a register offset, not its name or type. So to exclude the PC,
> we'd need to expose the register name to the API (which would
> likely require either duplication of the name in both `struct
> qemu_plugin_reg_descriptor` and `struct qemu_plugin_register` or
> changing the API) and then filter in the API based on the register name
> ("pc", "rip", "eip", ...). This seems rather hacky and fragile to me.

I agree - although I don't think there are too many more variations.
Most of the other arches use pc.

>
> Alternatively, we could encode whether a register is to be filtered out
> or not in the opaque handle itself (e.g., by setting the topmost bit).
> This still requires getting this information from somewhere, though.
> Currently, registers are created by parsing the target XML in `gdb-xml/`
> at compile time. The PC is generally marked as `type="code_ptr"` in the
> XML, so we could expose this type information to the code and adjust the
> register handle if it's a code pointer. The problem here is that it's
> not just the PC that is marked as `code_ptr` in the XML; there may be
> other registers as well (e.g., `lr` or `ra` for Arm and RISC-V).

Hmm thats interesting. Looking at the gdb docs:

  code_ptr
  data_ptr

      Pointers to unspecified code and data. The program counter and any
      dedicated return address register may be marked as code pointers;
      printing a code pointer converts it into a symbolic address. The
      stack pointer and any dedicated address registers may be marked as
      data pointers.

I'm not sure we can necessarily infer PC like behaviour based on that.

I have been messing with a systrace plugin and I can track the values of
el1_elr and they seem sane:

  CPU: 0 taking host call from 0xffffffc08003a35c to 0xffffffc08003a35c
    FROM: 0xffffffc08003a358 hvc #0       (lepc)
    LAST SYSREG: 0xffffffc0809d3d38 msr daif, x23
    REG: id_aa64pfr0_el1 is 1100000011110012 (previously 1100000010110012, 0 to 
1 hits)
    REG: sctlr_el1 is 0200000034f4d91d (previously 0000000000000000, 0 to 3 
hits)
    REG: spsr_el1 is 00000000000003c5 (previously 0000000000000000, 0 to 1 hits)
    REG: elr_el1 is 0000000041406100 (previously 0000000000000000, 0 to 1 hits)
    REG: vbar_el1 is ffffffc080010800 (previously 0000000000000000, 0 to 1 hits)
    REG: mdscr_el1 is 0000000000001000 (previously 0000000000000000, 0 to 1 
hits)
    REG: ttbr0_el1 is 0000000041408000 (previously 0000000000000000, 0 to 2 
hits)
    REG: ttbr1_el1 is 0000000041409000 (previously 0000000000000000, 0 to 5 
hits)
    REG: tcr_el1 is 001000f5b5593519 (previously 0000000000000000, 0 to 3 hits)
    REG: mair_el1 is 000000040044ffff (previously 0000000000000000, 0 to 1 hits)
    REG: midr_el1 is 00000000414fd0c1 (previously 0000000000000000, 0 to 2 hits)
  CPU: 0 taking host call from 0xffffffc08003a35c to 0xffffffc08003a35c
    FROM: 0xffffffc08003a35c ldr x4, [sp] (fpc)
    LAST SYSREG: 0xffffffc0800cd6dc msr daif, x21
  CPU: 0 taking host call from 0xffffffc08003a35c to 0xffffffc08003a35c
    FROM: 0xffffffc08003a35c ldr x4, [sp] (fpc)
    LAST SYSREG: 0xffffffc08010fe9c msr daif, x27
  CPU: 0 taking host call from 0xffffffc08003a35c to 0xffffffc08003a35c
    FROM: 0xffffffc08003a35c ldr x4, [sp] (fpc)
    LAST SYSREG: 0xffffffc08010fe9c msr daif, x27
  CPU: 0 taking host call from 0xffffffc08003a35c to 0xffffffc08003a35c
    FROM: 0xffffffc08003a35c ldr x4, [sp] (fpc)
    LAST SYSREG: 0xffffffc08010fe9c msr daif, x27
  CPU: 0 taking host call from 0xffffffc08003a35c to 0xffffffc08003a35c
    FROM: 0xffffffc08003a35c ldr x4, [sp] (fpc)
    LAST SYSREG: 0xffffffc08010fe9c msr daif, x27
  CPU: 0 taking host call from 0xffffffc08003a35c to 0xffffffc08003a35c
    FROM: 0xffffffc08003a35c ldr x4, [sp] (fpc)
    LAST SYSREG: 0xffffffc08010fe9c msr daif, x27
  CPU: 0 taking irq from 0xffffffc08121102c to 0xffffffc080010a80
    FROM: 0xffffffc081211028 msr daifclr, #3      (lepc)
    LAST SYSREG: 0xffffffc081211028 msr daifclr, #3
    REG: cntkctl_el1 is 0000000000000002 (previously 0000000000000000, 0 to 2 
hits)
    REG: tpidr_el1 is ffffffbfbe084000 (previously 0000000000000000, 513 to 
4454 hits)
  CPU: 0 taking irq from 0xffffffc08121102c to 0xffffffc080010a80
    FROM: 0xffffffc0800120c8 eret         (lepc)
    LAST SYSREG: 0xffffffc08001207c msr spsr_el1, x22
    REG: spsr_el1 is 0000000000000005 (previously 00000000000003c5, 1 to 3 hits)
    REG: elr_el1 is ffffffc08121102c (previously 0000000041406100, 1 to 3 hits)

But due to the way I track the changes I don't see changes that are
implicit due to branch instructions, only when the kernel directly
manipulates them.

We could extend the XML itself but that makes things tricky when we sync
with gdb.

> I'm not sure whether the above explanation is clear enough, so please
> let me know if you'd like me to clarify anything or point you to the
> exact spots in the code.
>
> Thanks,
> Florian

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Reply via email to