| Message ID | 20221209095612.689243-11-dwmw2@infradead.org |
|---|---|
| State | New |
| Headers | show |
| Series | Xen HVM support under KVM | expand |
On 09/12/2022 09:56, David Woodhouse wrote: > From: Joao Martins <joao.m.martins@oracle.com> > > This means handling the new exit reason for Xen but still > crashing on purpose. As we implement each of the hypercalls > we will then return the right return code. > > Signed-off-by: Joao Martins <joao.m.martins@oracle.com> > [dwmw2: Add CPL to hypercall tracing, disallow hypercalls from CPL > 0] > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> > --- > target/i386/kvm/kvm.c | 5 +++++ > target/i386/trace-events | 3 +++ > target/i386/xen.c | 39 +++++++++++++++++++++++++++++++++++++++ > target/i386/xen.h | 1 + > 4 files changed, 48 insertions(+) > [snip] > +int kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit) > +{ > + if (exit->type != KVM_EXIT_XEN_HCALL) > + return -1; > + > + if (!__kvm_xen_handle_exit(cpu, exit)) { > + /* Some hypercalls will be deliberately "implemented" by returning > + * -ENOSYS. This case is for hypercalls which are unexpected. */ > + exit->u.hcall.result = -ENOSYS; > + qemu_log_mask(LOG_GUEST_ERROR, "Unimplemented Xen hypercall %" > + PRId64 " (0x%" PRIx64 " 0x%" PRIx64 " 0x%" PRIx64 ")\n", > + (uint64_t)exit->u.hcall.input, (uint64_t)exit->u.hcall.params[0], > + (uint64_t)exit->u.hcall.params[1], (uint64_t)exit->u.hcall.params[1]); This could get a little noisy; would a trace not be better? Then only those interested in it need be bothered by it. As we know, some ancient guests attempt to use some hypercalls which have long been consigned to the waste-bin of history. Paul > + } > + > + trace_kvm_xen_hypercall(CPU(cpu)->cpu_index, exit->u.hcall.cpl, > + exit->u.hcall.input, exit->u.hcall.params[0], > + exit->u.hcall.params[1], exit->u.hcall.params[2], > + exit->u.hcall.result); > + return 0; > +}
On Mon, 2022-12-12 at 14:11 +0000, Paul Durrant wrote: > On 09/12/2022 09:56, David Woodhouse wrote: > > From: Joao Martins < > > joao.m.martins@oracle.com > > > > > > > This means handling the new exit reason for Xen but still > > crashing on purpose. As we implement each of the hypercalls > > we will then return the right return code. > > > > Signed-off-by: Joao Martins < > > joao.m.martins@oracle.com > > > > > [dwmw2: Add CPL to hypercall tracing, disallow hypercalls from CPL > 0] > > Signed-off-by: David Woodhouse < > > dwmw@amazon.co.uk > > > > > --- > > target/i386/kvm/kvm.c | 5 +++++ > > target/i386/trace-events | 3 +++ > > target/i386/xen.c | 39 +++++++++++++++++++++++++++++++++++++++ > > target/i386/xen.h | 1 + > > 4 files changed, 48 insertions(+) > > > > [snip] > > +int kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit) > > +{ > > + if (exit->type != KVM_EXIT_XEN_HCALL) > > + return -1; > > + > > + if (!__kvm_xen_handle_exit(cpu, exit)) { > > + /* Some hypercalls will be deliberately "implemented" by returning > > + * -ENOSYS. This case is for hypercalls which are unexpected. */ > > + exit->u.hcall.result = -ENOSYS; > > + qemu_log_mask(LOG_GUEST_ERROR, "Unimplemented Xen hypercall %" > > + PRId64 " (0x%" PRIx64 " 0x%" PRIx64 " 0x%" PRIx64 ")\n", > > + (uint64_t)exit->u.hcall.input, (uint64_t)exit->u.hcall.params[0], > > + (uint64_t)exit->u.hcall.params[1], (uint64_t)exit->u.hcall.params[1]); > > This could get a little noisy; would a trace not be better? Then only > those interested in it need be bothered by it. As we know, some ancient > guests attempt to use some hypercalls which have long been consigned to > the waste-bin of history. By the time I'm done here, qemu is going to get the benefit of all those things that "we know", and is going to have implementations of those ancient hypercalls which intentionally return -ENOSYS and don't trigger this warning. So this is worth reporting in *addition* to the trace (which does also exist). I'll change it to LOG_UNIMP though, as I think I mentioned in the cover message.
On 12/9/22 10:56, David Woodhouse wrote: > +static bool __kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit) No double underscores in userspace. > +{ > + uint16_t code = exit->u.hcall.input; > + > + if (exit->u.hcall.cpl > 0) { > + exit->u.hcall.result = -EPERM; > + return true; > + } > + > + switch (code) { > + default: > + return false; > + } > +} > + > +int kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit) BTW, please name this file either target/i386/xen-emu.c or target/i386/kvm/xen-emu.c (probably the latter is better, since XEN_EMU depends on KVM.) Paolo
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 0d3eddf9de..ebde6bc204 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -5471,6 +5471,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run) assert(run->msr.reason == KVM_MSR_EXIT_REASON_FILTER); ret = kvm_handle_wrmsr(cpu, run); break; +#ifdef CONFIG_XEN_EMU + case KVM_EXIT_XEN: + ret = kvm_xen_handle_exit(cpu, &run->xen); + break; +#endif default: fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason); ret = -1; diff --git a/target/i386/trace-events b/target/i386/trace-events index 2cd8726eeb..1bf9558811 100644 --- a/target/i386/trace-events +++ b/target/i386/trace-events @@ -11,3 +11,6 @@ kvm_sev_launch_measurement(const char *value) "data %s" kvm_sev_launch_finish(void) "" kvm_sev_launch_secret(uint64_t hpa, uint64_t hva, uint64_t secret, int len) "hpa 0x%" PRIx64 " hva 0x%" PRIx64 " data 0x%" PRIx64 " len %d" kvm_sev_attestation_report(const char *mnonce, const char *data) "mnonce %s data %s" + +# target/i386/xen.c +kvm_xen_hypercall(int cpu, uint8_t cpl, uint64_t input, uint64_t a0, uint64_t a1, uint64_t a2, uint64_t ret) "xen_hypercall: cpu %d cpl %d input %" PRIu64 " a0 0x%" PRIx64 " a1 0x%" PRIx64 " a2 0x%" PRIx64" ret 0x%" PRIx64 diff --git a/target/i386/xen.c b/target/i386/xen.c index bc183dce4e..708ab908a0 100644 --- a/target/i386/xen.c +++ b/target/i386/xen.c @@ -10,8 +10,10 @@ */ #include "qemu/osdep.h" +#include "qemu/log.h" #include "kvm/kvm_i386.h" #include "xen.h" +#include "trace.h" int kvm_xen_init(KVMState *s, uint32_t xen_version) { @@ -47,3 +49,40 @@ int kvm_xen_init(KVMState *s, uint32_t xen_version) return 0; } + +static bool __kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit) +{ + uint16_t code = exit->u.hcall.input; + + if (exit->u.hcall.cpl > 0) { + exit->u.hcall.result = -EPERM; + return true; + } + + switch (code) { + default: + return false; + } +} + +int kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit) +{ + if (exit->type != KVM_EXIT_XEN_HCALL) + return -1; + + if (!__kvm_xen_handle_exit(cpu, exit)) { + /* Some hypercalls will be deliberately "implemented" by returning + * -ENOSYS. This case is for hypercalls which are unexpected. */ + exit->u.hcall.result = -ENOSYS; + qemu_log_mask(LOG_GUEST_ERROR, "Unimplemented Xen hypercall %" + PRId64 " (0x%" PRIx64 " 0x%" PRIx64 " 0x%" PRIx64 ")\n", + (uint64_t)exit->u.hcall.input, (uint64_t)exit->u.hcall.params[0], + (uint64_t)exit->u.hcall.params[1], (uint64_t)exit->u.hcall.params[1]); + } + + trace_kvm_xen_hypercall(CPU(cpu)->cpu_index, exit->u.hcall.cpl, + exit->u.hcall.input, exit->u.hcall.params[0], + exit->u.hcall.params[1], exit->u.hcall.params[2], + exit->u.hcall.result); + return 0; +} diff --git a/target/i386/xen.h b/target/i386/xen.h index ae880c47bc..9134d78685 100644 --- a/target/i386/xen.h +++ b/target/i386/xen.h @@ -23,5 +23,6 @@ #define XEN_VERSION(maj, min) ((maj) << 16 | (min)) int kvm_xen_init(KVMState *s, uint32_t xen_version); +int kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit); #endif /* QEMU_I386_XEN_H */