Yanfeng Liu <yfliu2...@qq.com> writes: > On Wed, 2024-12-04 at 17:03 +0100, Mario Fleischmann wrote: >> Hi everyone, >> >> I'd like to chime in here because we are sitting on a similar patch >> which I wanted to send to the mailing list as soon as riscv-debug-spec >> v1.0.0 becomes ratified. >> >> For hypervisor support, `(qemu) info registers` isn't enough. We need to >> have both read and write access to the V-bit. >> >> On 04.12.2024 14:43, Yanfeng Liu wrote: >> > On Fri, 2024-11-29 at 09:59 +0000, Alex Bennée wrote: >> > > Yanfeng <yfliu2...@qq.com> writes: >> > > >> > > > On Thu, 2024-11-28 at 14:21 +0000, Alex Bennée wrote: >> > > > > Yanfeng Liu <yfliu2...@qq.com> writes: >> > > > > >> > > > > > This adds `virt` virtual register on debug interface so that users >> > > > > > can access current virtualization mode for debugging purposes. >> > > > > > >> > > > > > Signed-off-by: Yanfeng Liu <yfliu2...@qq.com> >> > > > > > --- >> > > > > > gdb-xml/riscv-32bit-virtual.xml | 1 + >> > > > > > gdb-xml/riscv-64bit-virtual.xml | 1 + >> > > > > > target/riscv/gdbstub.c | 18 ++++++++++++------ >> > > > > > 3 files changed, 14 insertions(+), 6 deletions(-) >> > > > > > >> > > > > > diff --git a/gdb-xml/riscv-32bit-virtual.xml b/gdb-xml/riscv-32bit- >> > > > > > virtual.xml >> > > > > > index 905f1c555d..d44b6ca2dc 100644 >> > > > > > --- a/gdb-xml/riscv-32bit-virtual.xml >> > > > > > +++ b/gdb-xml/riscv-32bit-virtual.xml >> > > > > > @@ -8,4 +8,5 @@ >> > > > > > <!DOCTYPE feature SYSTEM "gdb-target.dtd"> >> > > > > > <feature name="org.gnu.gdb.riscv.virtual"> >> > > > > > <reg name="priv" bitsize="32"/> >> > > > > > + <reg name="virt" bitsize="32"/> >> > > > > > </feature> >> > > > > > diff --git a/gdb-xml/riscv-64bit-virtual.xml b/gdb-xml/riscv-64bit- >> > > > > > virtual.xml >> > > > > > index 62d86c237b..7c9b63d5b6 100644 >> > > > > > --- a/gdb-xml/riscv-64bit-virtual.xml >> > > > > > +++ b/gdb-xml/riscv-64bit-virtual.xml >> > > > > > @@ -8,4 +8,5 @@ >> > > > > > <!DOCTYPE feature SYSTEM "gdb-target.dtd"> >> > > > > > <feature name="org.gnu.gdb.riscv.virtual"> >> > > > > > <reg name="priv" bitsize="64"/> >> > > > > > + <reg name="virt" bitsize="64"/> >> > > > > > </feature> >> > > > > >> > > > > I assume these are mirrored in gdb not a QEMU only extension? >> > > > >> > > > So far I think it is a QEMU extension and the `gdb-multiarch` doesn't >> > > > treat >> > > > is >> > > > specially. My tests shows it basically works: >> > > > >> > > > ``` >> > > > (gdb) ir virt >> > > > priv 0x3 prv:3 [Machine] >> > > > virt 0x0 0 >> > > > (gdb) set $priv = 2 >> > > > (gdb) ir virt >> > > > priv 0x1 prv:1 [Supervisor] >> > > > virt 0x0 0 >> > > > (gdb) set $virt = 1 >> > > > (gdb) ir virt >> > > > priv 0x1 prv:1 [Supervisor] >> > > > virt 0x1 1 >> > > > (gdb) set $virt = 0 >> > > > (gdb) ir virt >> > > > priv 0x1 prv:1 [Supervisor] >> > > > virt 0x0 0 >> > > > (gdb) set $virt = 1 >> > > > (gdb) ir virt >> > > > priv 0x1 prv:1 [Supervisor] >> > > > virt 0x1 1 >> > > > (gdb) set $priv = 3 >> > > > (gdb) ir virt >> > > > priv 0x3 prv:3 [Machine] >> > > > virt 0x0 0 >> > > > ``` >> > > >> > > A gdbstub test case would be useful for this although I don't know if >> > > the RiscV check-tcg tests switch mode at all. >> > > >> > > > >> > > > As I am rather new to QEMU, please teach how we can add it as a QEMU >> > > > only >> > > > extension. >> > > >> > > You don't need to extend the XML from GDB, you can build a specific one >> > > for QEMU extensions. For example: >> > > >> > > gdb_feature_builder_init(¶m.builder, >> > > &cpu->dyn_sysreg_feature.desc, >> > > "org.qemu.gdb.arm.sys.regs", >> > > "system-registers.xml", >> > > base_reg); >> > > >> > > This exports all the system registers QEMU knows about and GDB can >> > > access generically. Note the id is org.qemu..., indicating its our >> > > schema not gdbs. >> > Thanks for teaching, I need time to digest. I guess more feature builder >> > APIs >> > are needed (like append_reg) and the getter/setter callbacks might be at a >> > different place. >> > >> > BTW, compared to adding virtual register `virt`, how do you think if we >> > share >> > the V bit as part of existing `priv` register? >> >> IMHO this is a very good idea since the latest release candidate of >> riscv-debug-spec also includes the V bit in priv:2. >> > > Thanks for this information, I noticed the bit(2) of `priv` register is for > the > V bit as per section 4.10.1. > >> > Or maybe we shall talk to GDB community to get their opinions? If they >> > agree >> > to >> > add a few words about V bit here >> > https://sourceware.org/gdb/current/onlinedocs/gdb.html/RISC_002dV-Features.html >> > , >> > then it saves us a lot. >> >> Except being currently not supported by GDB >> >> (gdb) info register $priv >> priv 0x5 prv:5 [INVALID] >> >> are there any reasons from QEMU's side that would speak against >> including V in priv? >> > > My v1 patch used `bit(8)` to avoid seeing the `[INVALID]` thing at GDB side, > though that is due to GDB isn't in line with its own manual (i.e. use the two > lowest bits only). > > Without a doc or specification. we felt people may not know `bit(8)` in v1 > patch > was for the V bit, so I drafted patch v2 as Alistair suggested. However, as > Alex > pointed out, directly adding `virt` register in "org.gnu.gdb.riscv.virtual" > XML > is improper. I also wanted to raise this in GDB side but my application to > join > the mail list is still pending. > > Alex and Alistair, now I am wondering if we can follow the RiscV debug > specification to use `bit(2)` of `priv` virtual register? My test shows except > for the `[INVALID]` label, both set/get access seems working.
I guess the INVALID just means gdb needs teaching about the format of the register. > > > Regards, > yf -- Alex Bennée Virtualisation Tech Lead @ Linaro
