On Thu, 8 May 2025 at 12:50, Alex Bennée <alex.ben...@linaro.org> wrote:
>
> Peter Maydell <peter.mayd...@linaro.org> writes:
>
> > On Wed, 7 May 2025 at 17:58, Alex Bennée <alex.ben...@linaro.org> wrote:
> >>
> >> Before this we suppress all ARM_CP_NORAW registers being listed under
> >> GDB. This includes useful registers like CurrentEL which gets tagged
> >> as ARM_CP_NO_RAW because it is one of the ARM_CP_SPECIAL_MASK
> >> registers. These are registers TCG can directly compute because we
> >> have the information at compile time but until now with no readfn.
> >>
> >> Add a .readfn to return the CurrentEL and then loosen the restrictions
> >> in arm_register_sysreg_for_feature to allow ARM_CP_NORAW registers to
> >> be read if there is a readfn available.
> >
> > The primary use case for NO_RAW is "system instructions" like
> > the TLB maintenance insns. These don't make sense to expose
> > to a debugger.
>
> I think we could re-think the logic:
>
> /*
> * By convention, for wildcarded registers only the first
> * entry is used for migration; the others are marked as
> * ALIAS so we don't try to transfer the register
> * multiple times. Special registers (ie NOP/WFI) are
> * never migratable and not even raw-accessible.
> */
> if (r2->type & ARM_CP_SPECIAL_MASK) {
> r2->type |= ARM_CP_NO_RAW;
> }
Well, we definitely don't want WFI or the DC ZVA etc
"registers" to be exposed to GDB or for us to try to handle
them in KVM state sync or migration... ARM_CP_NOP is less
clear because we use it pretty widely for more than one
purpose. The main one is "system instruction that we don't
need to implement". (CP_NOP + a readable register is a
questionable combination since the guest will expect it to
update the general-purpose destreg, not leave it untouched,
but we do have some.)
> > If we want the gdbstub access to system registers to be
> > more than our current "we provide the ones that are easy",
> > then I think I'd like to see a bit more up-front analysis of
> > what the gdbstub needs and whether we've got into a bit of
> > a mess with our ARM_CP_* flags that we could straighten out.
>
> Yeah - hence the RFC. CurrentEL is a super useful one to expose though
> when you are debugging complex hypervisor setups.
One problem with this patch is the one that the reporter of
https://gitlab.com/qemu-project/qemu/-/issues/2760 noted
in the conversation there: it will allow the debugger to
read registers which have a side-effect on read, like
ICC_IAR1_EL1: we almost certainly do not want to allow
the debugger to acknowledge an interrupt by doing a sysreg
read.
thanks
-- PMM
