[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 0/9] add LOG_UNSUPP log type + mark hcalls as unsupp
From: |
Daniel Henrique Barboza |
Subject: |
Re: [PATCH 0/9] add LOG_UNSUPP log type + mark hcalls as unsupp |
Date: |
Mon, 7 Mar 2022 19:00:35 -0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 |
On 3/7/22 17:21, Peter Maydell wrote:
On Mon, 7 Mar 2022 at 19:19, Daniel Henrique Barboza
<danielhb413@gmail.com> wrote:
Hi,
I got a lot of noise trying to debug an AIX guest in a pseries machine when
running with
'-d unimp'. The reason is that there is no distinction between features
(in my case, hypercalls) that are unimplemented because we never considered,
versus features that we made a design choice not to implement.
This series adds a new log type, LOG_UNSUPP, as a way to filter the
second case. After changing the log level of existing unsupported
pseries hypercalls, -d unimp was reporting just the ones that I need to
worry about and decide whether we should implement it or mark as
unsupported in our model. After this series there's still one hypercall
thgat is being thrown by AIX. We'll deal with that another day.
So the intention of the distinction is:
LOG_UNIMP: we don't implement this, but we should
LOG_UNSUPP: we don't implement this, and that's OK because it's optional
?
The idea is that LOG_UNIMP is too broad and it's used to indicate features that
are
unknown to QEMU and also features that QEMU knows about but does not support
it. It's
not necessarily a way of telling "we should implement this" but more like "we
know/do
not know what this is".
I think I'd be happier about adding a new log category if we had
some examples of where we should be using it other than just in
the spapr hcall code, to indicate that it's a bit more broadly
useful. If this is a distinction that only makes sense for that
narrow use case, then as Philippe says a tracepoint might be a
better choice.
target/arm/translate.c, do_coproc_insn():
/* Unknown register; this might be a guest error or a QEMU
* unimplemented feature.
*/
if (is64) {
qemu_log_mask(LOG_UNIMP, "%s access to unsupported AArch32 "
"64 bit system register cp:%d opc1: %d crm:%d "
"(%s)\n",
isread ? "read" : "write", cpnum, opc1, crm,
s->ns ? "non-secure" : "secure");
} else {
qemu_log_mask(LOG_UNIMP, "%s access to unsupported AArch32 "
"system register cp:%d opc1:%d crn:%d crm:%d opc2:%d "
"(%s)\n",
isread ? "read" : "write", cpnum, opc1, crn, crm, opc2,
s->ns ? "non-secure" : "secure");
}
This use of LOG_UNIMP is logging something that we don't know about, it's
unknown.
And hw/arm/smmuv3.c, decode_ste():
if (STE_CFG_S2_ENABLED(config)) {
qemu_log_mask(LOG_UNIMP, "SMMUv3 does not support stage 2 yet\n");
goto bad_ste;
}
if (STE_S1CDMAX(ste) != 0) {
qemu_log_mask(LOG_UNIMP,
"SMMUv3 does not support multiple context descriptors
yet\n");
goto bad_ste;
}
if (STE_S1STALLD(ste)) {
qemu_log_mask(LOG_UNIMP,
"SMMUv3 S1 stalling fault model not allowed yet\n");
goto bad_ste;
}
This is something we know what it is and are deciding not to support it. Both
are being
logged as LOG_UNIMP. This is the distinction I was trying to achieve with this
new
log type. The example in decode_ste() could be logged as LOG_UNSUPP.
Perhaps an easier sell would be adding a "LOG_UNKNOWN" for the
features/regs/etc that
we do not know about. The semantics/intention seems clearer than what I'm
trying to
achieve with LOG_UNSUPP.
And yeah, if this is too convoluted to push forward I can always tracepoint
like Philippe
suggested as well.
Thanks,
Daniel
thanks
-- PMM
- [PATCH 2/9] hw/ppc/spapr_hcall.c: log h_clean_slb() as unsupported, (continued)
- [PATCH 2/9] hw/ppc/spapr_hcall.c: log h_clean_slb() as unsupported, Daniel Henrique Barboza, 2022/03/07
- [PATCH 3/9] hw/ppc/spapr_hcall.c: log h_invalidate_pid() as unsupported, Daniel Henrique Barboza, 2022/03/07
- [PATCH 4/9] hw/ppc/spapr_hcall.c: log h_copy_tofrom_guest() as unsupported, Daniel Henrique Barboza, 2022/03/07
- [PATCH 5/9] hw/ppc/spapr_hcall.c: log H_GET_EM_PARMS as unsupported, Daniel Henrique Barboza, 2022/03/07
- [PATCH 6/9] hw/ppc/spapr_hcall.c: log H_BEST_ENERGY as unsupported, Daniel Henrique Barboza, 2022/03/07
- [PATCH 7/9] hw/ppc/spapr_hcall.c: log H_QUERY_VAS_CAPABILITIES as unsupported, Daniel Henrique Barboza, 2022/03/07
- [PATCH 8/9] hw/ppc/spapr_hcall.c: log H_GET_PPP as unsupported, Daniel Henrique Barboza, 2022/03/07
- [PATCH 9/9] hw/ppc/spapr_hcall.c: log H_VIOCTL as unsupported, Daniel Henrique Barboza, 2022/03/07
- Re: [PATCH 0/9] add LOG_UNSUPP log type + mark hcalls as unsupp, Philippe Mathieu-Daudé, 2022/03/07
- Re: [PATCH 0/9] add LOG_UNSUPP log type + mark hcalls as unsupp, Peter Maydell, 2022/03/07
- Re: [PATCH 0/9] add LOG_UNSUPP log type + mark hcalls as unsupp,
Daniel Henrique Barboza <=