| Message ID | 20230201143148.1744093-46-dwmw2@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Xen HVM support under KVM | expand |
On 01/02/2023 14:31, David Woodhouse wrote: > From: David Woodhouse <dwmw@amazon.co.uk> > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> > --- > hw/i386/kvm/xen_gnttab.c | 31 ++++++++++++++++++++ > hw/i386/kvm/xen_gnttab.h | 5 ++++ > target/i386/kvm/xen-emu.c | 60 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 96 insertions(+) > > diff --git a/hw/i386/kvm/xen_gnttab.c b/hw/i386/kvm/xen_gnttab.c > index cd8c3ae60d..9dfce91f6a 100644 > --- a/hw/i386/kvm/xen_gnttab.c > +++ b/hw/i386/kvm/xen_gnttab.c > @@ -181,3 +181,34 @@ int xen_gnttab_map_page(uint64_t idx, uint64_t gfn) > return 0; > } > > +int xen_gnttab_set_version_op(struct gnttab_set_version *set) > +{ > + int ret; > + > + switch (set->version) { > + case 1: > + ret = 0; > + break; > + > + case 2: > + /* Behave as before set_version was introduced. */ > + ret = -ENOSYS; > + break; > + > + default: > + ret = -EINVAL; > + } > + > + set->version = 1; Seems like an odd semantic. Only a version of 1 can result in a non-error exit. I suspect no harm will come from setting it in the error case though so... Reviewed-by: Paul Durrant <paul@xen.org>
On 14 February 2023 10:59:12 CET, Paul Durrant <xadimgnik@gmail.com> wrote: >On 01/02/2023 14:31, David Woodhouse wrote: >> From: David Woodhouse <dwmw@amazon.co.uk> >> >> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> >> --- >> hw/i386/kvm/xen_gnttab.c | 31 ++++++++++++++++++++ >> hw/i386/kvm/xen_gnttab.h | 5 ++++ >> target/i386/kvm/xen-emu.c | 60 +++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 96 insertions(+) >> >> diff --git a/hw/i386/kvm/xen_gnttab.c b/hw/i386/kvm/xen_gnttab.c >> index cd8c3ae60d..9dfce91f6a 100644 >> --- a/hw/i386/kvm/xen_gnttab.c >> +++ b/hw/i386/kvm/xen_gnttab.c >> @@ -181,3 +181,34 @@ int xen_gnttab_map_page(uint64_t idx, uint64_t gfn) >> return 0; >> } >> +int xen_gnttab_set_version_op(struct gnttab_set_version *set) >> +{ >> + int ret; >> + >> + switch (set->version) { >> + case 1: >> + ret = 0; >> + break; >> + >> + case 2: >> + /* Behave as before set_version was introduced. */ >> + ret = -ENOSYS; >> + break; >> + >> + default: >> + ret = -EINVAL; >> + } >> + >> + set->version = 1; > >Seems like an odd semantic. Only a version of 1 can result in a non-error exit. I suspect no harm will come from setting it in the error case though so... Yeah, this is behaviour I saw in an actual Xen code base when gnttab v2 was disabled. The logic (in the comment) seemed sane enough.
diff --git a/hw/i386/kvm/xen_gnttab.c b/hw/i386/kvm/xen_gnttab.c index cd8c3ae60d..9dfce91f6a 100644 --- a/hw/i386/kvm/xen_gnttab.c +++ b/hw/i386/kvm/xen_gnttab.c @@ -181,3 +181,34 @@ int xen_gnttab_map_page(uint64_t idx, uint64_t gfn) return 0; } +int xen_gnttab_set_version_op(struct gnttab_set_version *set) +{ + int ret; + + switch (set->version) { + case 1: + ret = 0; + break; + + case 2: + /* Behave as before set_version was introduced. */ + ret = -ENOSYS; + break; + + default: + ret = -EINVAL; + } + + set->version = 1; + return ret; +} + +int xen_gnttab_get_version_op(struct gnttab_get_version *get) +{ + if (get->dom != DOMID_SELF && get->dom != xen_domid) { + return -ESRCH; + } + + get->version = 1; + return 0; +} diff --git a/hw/i386/kvm/xen_gnttab.h b/hw/i386/kvm/xen_gnttab.h index a7caa94c83..79579677ba 100644 --- a/hw/i386/kvm/xen_gnttab.h +++ b/hw/i386/kvm/xen_gnttab.h @@ -15,4 +15,9 @@ void xen_gnttab_create(void); int xen_gnttab_map_page(uint64_t idx, uint64_t gfn); +struct gnttab_set_version; +struct gnttab_get_version; +int xen_gnttab_set_version_op(struct gnttab_set_version *set); +int xen_gnttab_get_version_op(struct gnttab_get_version *get); + #endif /* QEMU_XEN_GNTTAB_H */ diff --git a/target/i386/kvm/xen-emu.c b/target/i386/kvm/xen-emu.c index 41976e85af..e35b2d5557 100644 --- a/target/i386/kvm/xen-emu.c +++ b/target/i386/kvm/xen-emu.c @@ -34,6 +34,7 @@ #include "hw/xen/interface/hvm/params.h" #include "hw/xen/interface/vcpu.h" #include "hw/xen/interface/event_channel.h" +#include "hw/xen/interface/grant_table.h" #include "xen-compat.h" @@ -1166,6 +1167,61 @@ static bool kvm_xen_hcall_sched_op(struct kvm_xen_exit *exit, X86CPU *cpu, return true; } +static bool kvm_xen_hcall_gnttab_op(struct kvm_xen_exit *exit, X86CPU *cpu, + int cmd, uint64_t arg, int count) +{ + CPUState *cs = CPU(cpu); + int err; + + switch (cmd) { + case GNTTABOP_set_version: { + struct gnttab_set_version set; + + qemu_build_assert(sizeof(set) == 4); + if (kvm_copy_from_gva(cs, arg, &set, sizeof(set))) { + err = -EFAULT; + break; + } + + err = xen_gnttab_set_version_op(&set); + if (!err && kvm_copy_to_gva(cs, arg, &set, sizeof(set))) { + err = -EFAULT; + } + break; + } + case GNTTABOP_get_version: { + struct gnttab_get_version get; + + qemu_build_assert(sizeof(get) == 8); + if (kvm_copy_from_gva(cs, arg, &get, sizeof(get))) { + err = -EFAULT; + break; + } + + err = xen_gnttab_get_version_op(&get); + if (!err && kvm_copy_to_gva(cs, arg, &get, sizeof(get))) { + err = -EFAULT; + } + break; + } + case GNTTABOP_query_size: + case GNTTABOP_setup_table: + case GNTTABOP_copy: + case GNTTABOP_map_grant_ref: + case GNTTABOP_unmap_grant_ref: + case GNTTABOP_swap_grant_ref: + return false; + + default: + /* Xen explicitly returns -ENOSYS to HVM guests for all others */ + err = -ENOSYS; + break; + } + + exit->u.hcall.result = err; + return true; +} + static bool do_kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit) { uint16_t code = exit->u.hcall.input; @@ -1176,6 +1232,10 @@ static bool do_kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit) } switch (code) { + case __HYPERVISOR_grant_table_op: + return kvm_xen_hcall_gnttab_op(exit, cpu, exit->u.hcall.params[0], + exit->u.hcall.params[1], + exit->u.hcall.params[2]); case __HYPERVISOR_sched_op: return kvm_xen_hcall_sched_op(exit, cpu, exit->u.hcall.params[0], exit->u.hcall.params[1]);