| Message ID | 20201005094905.2929-4-paul@xen.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iommu page-table memory pool | expand |
On Mon, Oct 05, 2020 at 10:49:03AM +0100, Paul Durrant wrote: > From: Paul Durrant <pdurrant@amazon.com> [...] > diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c > index 6ec6c27c83..9631974dd6 100644 > --- a/tools/libs/light/libxl_x86.c > +++ b/tools/libs/light/libxl_x86.c > @@ -520,6 +520,16 @@ int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config, > NULL, 0, &shadow, 0, NULL); > } > > + if (d_config->b_info.iommu_memkb) { > + unsigned int nr_pages = DIV_ROUNDUP(d_config->b_info.iommu_memkb, 4); > + Please use XC_PAGE_SHIFT / XC_PAGE_SIZE for the calculation. Wei.
Hi Paul, On 05/10/2020 10:49, Paul Durrant wrote: > From: Paul Durrant <pdurrant@amazon.com> > > ... sub-operation of XEN_DOMCTL_iommu_ctl. > > This patch adds a new sub-operation into the domctl. The code in iommu_ctl() > is extended to call a new arch-specific iommu_set_allocation() function which > will be called with the IOMMU page-table overhead (in 4k pages) in response Why 4KB? Wouldn't it be better to use the hypervisor page size instead? > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h > index 75e855625a..6402678838 100644 > --- a/xen/include/public/domctl.h > +++ b/xen/include/public/domctl.h > @@ -1138,8 +1138,16 @@ struct xen_domctl_vuart_op { > > #define XEN_DOMCTL_IOMMU_INVALID 0 > > +#define XEN_DOMCTL_IOMMU_SET_ALLOCATION 1 > +struct xen_domctl_iommu_set_allocation { > + uint32_t nr_pages; Shouldn't this be a 64-bit value? Cheers,
> -----Original Message----- > From: Julien Grall <julien@xen.org> > Sent: 16 October 2020 16:55 > To: Paul Durrant <paul@xen.org>; xen-devel@lists.xenproject.org > Cc: Paul Durrant <pdurrant@amazon.com>; Ian Jackson <iwj@xenproject.org>; Wei Liu <wl@xen.org>; Andrew > Cooper <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>; Jan Beulich > <jbeulich@suse.com>; Stefano Stabellini <sstabellini@kernel.org>; Anthony PERARD > <anthony.perard@citrix.com>; Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Roger Pau Monné > <roger.pau@citrix.com> > Subject: Re: [PATCH 3/5] libxl / iommu / domctl: introduce XEN_DOMCTL_IOMMU_SET_ALLOCATION... > > Hi Paul, > > On 05/10/2020 10:49, Paul Durrant wrote: > > From: Paul Durrant <pdurrant@amazon.com> > > > > ... sub-operation of XEN_DOMCTL_iommu_ctl. > > > > This patch adds a new sub-operation into the domctl. The code in iommu_ctl() > > is extended to call a new arch-specific iommu_set_allocation() function which > > will be called with the IOMMU page-table overhead (in 4k pages) in response > > Why 4KB? Wouldn't it be better to use the hypervisor page size instead? > I think I'll follow the shadow/hap code more closely and just pass a value in MB, then any issue with page size is left inside Xen. > > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h > > index 75e855625a..6402678838 100644 > > --- a/xen/include/public/domctl.h > > +++ b/xen/include/public/domctl.h > > @@ -1138,8 +1138,16 @@ struct xen_domctl_vuart_op { > > > > #define XEN_DOMCTL_IOMMU_INVALID 0 > > > > +#define XEN_DOMCTL_IOMMU_SET_ALLOCATION 1 > > +struct xen_domctl_iommu_set_allocation { > > + uint32_t nr_pages; > > Shouldn't this be a 64-bit value? If I pass the value in MB then 32-bits will cover it, I think. I do need to add padding though. Paul > > Cheers, > > -- > Julien Grall
On 05.10.2020 11:49, Paul Durrant wrote: Just two nits, in case the op is really needed: > --- a/xen/drivers/passthrough/iommu.c > +++ b/xen/drivers/passthrough/iommu.c > @@ -515,6 +515,14 @@ static int iommu_ctl( > > switch ( ctl->op ) > { > + case XEN_DOMCTL_IOMMU_SET_ALLOCATION: > + { > + struct xen_domctl_iommu_set_allocation *set_allocation = > + &ctl->u.set_allocation; const please, or drop the local variable. > + rc = iommu_set_allocation(d, set_allocation->nr_pages); > + break; > + } > default: Blank line above here please. Jan
diff --git a/tools/libs/ctrl/include/xenctrl.h b/tools/libs/ctrl/include/xenctrl.h index 3796425e1e..4d6c9d44bc 100644 --- a/tools/libs/ctrl/include/xenctrl.h +++ b/tools/libs/ctrl/include/xenctrl.h @@ -2650,6 +2650,11 @@ int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout, uint32 int xc_domain_cacheflush(xc_interface *xch, uint32_t domid, xen_pfn_t start_pfn, xen_pfn_t nr_pfns); +/* IOMMU control operations */ + +int xc_iommu_set_allocation(xc_interface *xch, uint32_t domid, + unsigned int nr_pages); + /* Compat shims */ #include "xenctrl_compat.h" diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c index e7cea4a17d..0b20a8f2ee 100644 --- a/tools/libs/ctrl/xc_domain.c +++ b/tools/libs/ctrl/xc_domain.c @@ -2185,6 +2185,22 @@ int xc_domain_soft_reset(xc_interface *xch, domctl.domain = domid; return do_domctl(xch, &domctl); } + +int xc_iommu_set_allocation(xc_interface *xch, uint32_t domid, + unsigned int nr_pages) +{ + DECLARE_DOMCTL; + + memset(&domctl, 0, sizeof(domctl)); + + domctl.cmd = XEN_DOMCTL_iommu_ctl; + domctl.domain = domid; + domctl.u.iommu_ctl.op = XEN_DOMCTL_IOMMU_SET_ALLOCATION; + domctl.u.iommu_ctl.u.set_allocation.nr_pages = nr_pages; + + return do_domctl(xch, &domctl); +} + /* * Local variables: * mode: C diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c index 6ec6c27c83..9631974dd6 100644 --- a/tools/libs/light/libxl_x86.c +++ b/tools/libs/light/libxl_x86.c @@ -520,6 +520,16 @@ int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config, NULL, 0, &shadow, 0, NULL); } + if (d_config->b_info.iommu_memkb) { + unsigned int nr_pages = DIV_ROUNDUP(d_config->b_info.iommu_memkb, 4); + + ret = xc_iommu_set_allocation(ctx->xch, domid, nr_pages); + if (ret) { + LOGED(ERROR, domid, "Failed to set IOMMU allocation"); + goto out; + } + } + if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV && libxl_defbool_val(d_config->b_info.u.pv.e820_host)) { ret = libxl__e820_alloc(gc, domid, d_config); diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c index bef0405984..642d5c8331 100644 --- a/xen/drivers/passthrough/iommu.c +++ b/xen/drivers/passthrough/iommu.c @@ -515,6 +515,14 @@ static int iommu_ctl( switch ( ctl->op ) { + case XEN_DOMCTL_IOMMU_SET_ALLOCATION: + { + struct xen_domctl_iommu_set_allocation *set_allocation = + &ctl->u.set_allocation; + + rc = iommu_set_allocation(d, set_allocation->nr_pages); + break; + } default: rc = -EOPNOTSUPP; break; diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c index f17b1820f4..b168073f10 100644 --- a/xen/drivers/passthrough/x86/iommu.c +++ b/xen/drivers/passthrough/x86/iommu.c @@ -134,6 +134,11 @@ void __hwdom_init arch_iommu_check_autotranslated_hwdom(struct domain *d) panic("PVH hardware domain iommu must be set in 'strict' mode\n"); } +int iommu_set_allocation(struct domain *d, unsigned nr_pages) +{ + return 0; +} + int arch_iommu_domain_init(struct domain *d) { struct domain_iommu *hd = dom_iommu(d); diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h index 937edc8373..2e4735bace 100644 --- a/xen/include/asm-arm/iommu.h +++ b/xen/include/asm-arm/iommu.h @@ -33,6 +33,12 @@ int __must_check arm_iommu_map_page(struct domain *d, dfn_t dfn, mfn_t mfn, int __must_check arm_iommu_unmap_page(struct domain *d, dfn_t dfn, unsigned int *flush_flags); +static inline int iommu_set_allocation(struct domain *d, + unsigned int nr_pages) +{ + return -EOPNOTSUPP; +} + #endif /* __ARCH_ARM_IOMMU_H__ */ /* diff --git a/xen/include/asm-x86/iommu.h b/xen/include/asm-x86/iommu.h index 970eb06ffa..d086f564af 100644 --- a/xen/include/asm-x86/iommu.h +++ b/xen/include/asm-x86/iommu.h @@ -138,6 +138,8 @@ int pi_update_irte(const struct pi_desc *pi_desc, const struct pirq *pirq, int __must_check iommu_free_pgtables(struct domain *d); struct page_info *__must_check iommu_alloc_pgtable(struct domain *d); +int __must_check iommu_set_allocation(struct domain *d, unsigned int nr_pages); + #endif /* !__ARCH_X86_IOMMU_H__ */ /* * Local variables: diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index 75e855625a..6402678838 100644 --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -1138,8 +1138,16 @@ struct xen_domctl_vuart_op { #define XEN_DOMCTL_IOMMU_INVALID 0 +#define XEN_DOMCTL_IOMMU_SET_ALLOCATION 1 +struct xen_domctl_iommu_set_allocation { + uint32_t nr_pages; +}; + struct xen_domctl_iommu_ctl { uint32_t op; /* XEN_DOMCTL_IOMMU_* */ + union { + struct xen_domctl_iommu_set_allocation set_allocation; + } u; }; struct xen_domctl {