| Message ID | 1506049330-11196-4-git-send-email-tianyu.lan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Sep 21, 2017 at 11:01:44PM -0400, Lan Tianyu wrote: > This patch is to introduce create, destroy and query capabilities > command for vIOMMU. vIOMMU layer will deal with requests and call > arch vIOMMU ops. > > Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> > --- > xen/common/domctl.c | 6 ++++++ > xen/common/viommu.c | 30 ++++++++++++++++++++++++++++++ > xen/include/public/domctl.h | 42 ++++++++++++++++++++++++++++++++++++++++++ > xen/include/xen/viommu.h | 2 ++ > 4 files changed, 80 insertions(+) > > diff --git a/xen/common/domctl.c b/xen/common/domctl.c > index 42658e5..7e28237 100644 > --- a/xen/common/domctl.c > +++ b/xen/common/domctl.c > @@ -1149,6 +1149,12 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) > copyback = 1; > break; > > +#ifdef CONFIG_VIOMMU > + case XEN_DOMCTL_viommu_op: > + ret = viommu_domctl(d, &op->u.viommu_op, ©back); IMHO, I'm not really sure if it's worth to pass the copyback parameter around. Can you just do the copy if !ret? > + break; > +#endif Instead of guarding every call to a viommu related function with CONFIG_VIOMMU I would rather add dummy replacements for them in the !CONFIG_VIOMMU case in the viommu.h header. > + > default: > ret = arch_do_domctl(op, d, u_domctl); > break; > diff --git a/xen/common/viommu.c b/xen/common/viommu.c > index 64d91e6..55feb5d 100644 > --- a/xen/common/viommu.c > +++ b/xen/common/viommu.c > @@ -133,6 +133,36 @@ static int viommu_create(struct domain *d, uint64_t type, > return 0; > } > > +int viommu_domctl(struct domain *d, struct xen_domctl_viommu_op *op, > + bool *need_copy) > +{ > + int rc = -EINVAL; Why do you need to set rc to EINVAL? AFAICT there's no path that would return rc without being initialized. > + > + if ( !viommu_enabled() ) > + return -ENODEV; > + > + switch ( op->cmd ) > + { > + case XEN_DOMCTL_create_viommu: > + rc = viommu_create(d, op->u.create.viommu_type, > + op->u.create.base_address, > + op->u.create.capabilities, > + &op->u.create.viommu_id); > + if ( !rc ) > + *need_copy = true; > + break; > + > + case XEN_DOMCTL_destroy_viommu: > + rc = viommu_destroy_domain(d); > + break; > + > + default: > + return -ENOSYS; > + } > + > + return rc; > +} > + > /* > * Local variables: > * mode: C > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h > index 50ff58f..68854b6 100644 > --- a/xen/include/public/domctl.h > +++ b/xen/include/public/domctl.h > @@ -1163,6 +1163,46 @@ struct xen_domctl_psr_cat_op { > typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t; > DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t); > > +/* vIOMMU helper > + * > + * vIOMMU interface can be used to create/destroy vIOMMU and > + * query vIOMMU capabilities. > + */ > + > +/* vIOMMU type - specify vendor vIOMMU device model */ > +#define VIOMMU_TYPE_INTEL_VTD 0 > + > +/* vIOMMU capabilities */ > +#define VIOMMU_CAP_IRQ_REMAPPING (1u << 0) Please put those two defines next to the fields they belong to. > + > +struct xen_domctl_viommu_op { > + uint32_t cmd; > +#define XEN_DOMCTL_create_viommu 0 > +#define XEN_DOMCTL_destroy_viommu 1 Would be nice if the values where right aligned. > + union { > + struct { > + /* IN - vIOMMU type */ > + uint64_t viommu_type; > + /* > + * IN - MMIO base address of vIOMMU. vIOMMU device models > + * are in charge of to check base_address. > + */ > + uint64_t base_address; > + /* IN - Capabilities with which we want to create */ > + uint64_t capabilities; > + /* OUT - vIOMMU identity */ > + uint32_t viommu_id; > + } create; > + > + struct { > + /* IN - vIOMMU identity */ > + uint32_t viommu_id; > + } destroy; > + } u; > +}; See my comments about the struct in patch 01/29. Thanks, Roger.
On 2017年10月18日 22:18, Roger Pau Monné wrote: > On Thu, Sep 21, 2017 at 11:01:44PM -0400, Lan Tianyu wrote: >> This patch is to introduce create, destroy and query capabilities >> command for vIOMMU. vIOMMU layer will deal with requests and call >> arch vIOMMU ops. >> >> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> >> --- >> xen/common/domctl.c | 6 ++++++ >> xen/common/viommu.c | 30 ++++++++++++++++++++++++++++++ >> xen/include/public/domctl.h | 42 ++++++++++++++++++++++++++++++++++++++++++ >> xen/include/xen/viommu.h | 2 ++ >> 4 files changed, 80 insertions(+) >> >> diff --git a/xen/common/domctl.c b/xen/common/domctl.c >> index 42658e5..7e28237 100644 >> --- a/xen/common/domctl.c >> +++ b/xen/common/domctl.c >> @@ -1149,6 +1149,12 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) >> copyback = 1; >> break; >> >> +#ifdef CONFIG_VIOMMU >> + case XEN_DOMCTL_viommu_op: >> + ret = viommu_domctl(d, &op->u.viommu_op, ©back); > > IMHO, I'm not really sure if it's worth to pass the copyback parameter > around. Can you just do the copy if !ret? Yes, will update. > >> + break; >> +#endif > > Instead of guarding every call to a viommu related function with > CONFIG_VIOMMU I would rather add dummy replacements for them in the > !CONFIG_VIOMMU case in the viommu.h header. OK. > >> + >> default: >> ret = arch_do_domctl(op, d, u_domctl); >> break; >> /* >> * Local variables: >> * mode: C >> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h >> index 50ff58f..68854b6 100644 >> --- a/xen/include/public/domctl.h >> +++ b/xen/include/public/domctl.h >> @@ -1163,6 +1163,46 @@ struct xen_domctl_psr_cat_op { >> typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t; >> DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t); >> >> +/* vIOMMU helper >> + * >> + * vIOMMU interface can be used to create/destroy vIOMMU and >> + * query vIOMMU capabilities. >> + */ >> + >> +/* vIOMMU type - specify vendor vIOMMU device model */ >> +#define VIOMMU_TYPE_INTEL_VTD 0 >> + >> +/* vIOMMU capabilities */ >> +#define VIOMMU_CAP_IRQ_REMAPPING (1u << 0) > > Please put those two defines next to the fields they belong to. OK.
diff --git a/xen/common/domctl.c b/xen/common/domctl.c index 42658e5..7e28237 100644 --- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -1149,6 +1149,12 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) copyback = 1; break; +#ifdef CONFIG_VIOMMU + case XEN_DOMCTL_viommu_op: + ret = viommu_domctl(d, &op->u.viommu_op, ©back); + break; +#endif + default: ret = arch_do_domctl(op, d, u_domctl); break; diff --git a/xen/common/viommu.c b/xen/common/viommu.c index 64d91e6..55feb5d 100644 --- a/xen/common/viommu.c +++ b/xen/common/viommu.c @@ -133,6 +133,36 @@ static int viommu_create(struct domain *d, uint64_t type, return 0; } +int viommu_domctl(struct domain *d, struct xen_domctl_viommu_op *op, + bool *need_copy) +{ + int rc = -EINVAL; + + if ( !viommu_enabled() ) + return -ENODEV; + + switch ( op->cmd ) + { + case XEN_DOMCTL_create_viommu: + rc = viommu_create(d, op->u.create.viommu_type, + op->u.create.base_address, + op->u.create.capabilities, + &op->u.create.viommu_id); + if ( !rc ) + *need_copy = true; + break; + + case XEN_DOMCTL_destroy_viommu: + rc = viommu_destroy_domain(d); + break; + + default: + return -ENOSYS; + } + + return rc; +} + /* * Local variables: * mode: C diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index 50ff58f..68854b6 100644 --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -1163,6 +1163,46 @@ struct xen_domctl_psr_cat_op { typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t; DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t); +/* vIOMMU helper + * + * vIOMMU interface can be used to create/destroy vIOMMU and + * query vIOMMU capabilities. + */ + +/* vIOMMU type - specify vendor vIOMMU device model */ +#define VIOMMU_TYPE_INTEL_VTD 0 + +/* vIOMMU capabilities */ +#define VIOMMU_CAP_IRQ_REMAPPING (1u << 0) + +struct xen_domctl_viommu_op { + uint32_t cmd; +#define XEN_DOMCTL_create_viommu 0 +#define XEN_DOMCTL_destroy_viommu 1 + union { + struct { + /* IN - vIOMMU type */ + uint64_t viommu_type; + /* + * IN - MMIO base address of vIOMMU. vIOMMU device models + * are in charge of to check base_address. + */ + uint64_t base_address; + /* IN - Capabilities with which we want to create */ + uint64_t capabilities; + /* OUT - vIOMMU identity */ + uint32_t viommu_id; + } create; + + struct { + /* IN - vIOMMU identity */ + uint32_t viommu_id; + } destroy; + } u; +}; +typedef struct xen_domctl_viommu_op xen_domctl_viommu_op; +DEFINE_XEN_GUEST_HANDLE(xen_domctl_viommu_op); + struct xen_domctl { uint32_t cmd; #define XEN_DOMCTL_createdomain 1 @@ -1240,6 +1280,7 @@ struct xen_domctl { #define XEN_DOMCTL_monitor_op 77 #define XEN_DOMCTL_psr_cat_op 78 #define XEN_DOMCTL_soft_reset 79 +#define XEN_DOMCTL_viommu_op 80 #define XEN_DOMCTL_gdbsx_guestmemio 1000 #define XEN_DOMCTL_gdbsx_pausevcpu 1001 #define XEN_DOMCTL_gdbsx_unpausevcpu 1002 @@ -1302,6 +1343,7 @@ struct xen_domctl { struct xen_domctl_psr_cmt_op psr_cmt_op; struct xen_domctl_monitor_op monitor_op; struct xen_domctl_psr_cat_op psr_cat_op; + struct xen_domctl_viommu_op viommu_op; uint8_t pad[128]; } u; }; diff --git a/xen/include/xen/viommu.h b/xen/include/xen/viommu.h index 636a2a3..baa8ab7 100644 --- a/xen/include/xen/viommu.h +++ b/xen/include/xen/viommu.h @@ -43,6 +43,8 @@ static inline bool viommu_enabled(void) int viommu_register_type(uint64_t type, struct viommu_ops *ops); int viommu_destroy_domain(struct domain *d); +int viommu_domctl(struct domain *d, struct xen_domctl_viommu_op *op, + bool_t *need_copy); #else static inline int viommu_register_type(uint64_t type, struct viommu_ops *ops) {
This patch is to introduce create, destroy and query capabilities command for vIOMMU. vIOMMU layer will deal with requests and call arch vIOMMU ops. Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> --- xen/common/domctl.c | 6 ++++++ xen/common/viommu.c | 30 ++++++++++++++++++++++++++++++ xen/include/public/domctl.h | 42 ++++++++++++++++++++++++++++++++++++++++++ xen/include/xen/viommu.h | 2 ++ 4 files changed, 80 insertions(+)