| Message ID | 20190815233618.31630-7-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v6,1/8] xen/arm: pass node to device_tree_for_each_node | expand |
Hi, On 16/08/2019 00:36, Stefano Stabellini wrote: > Don't allow reserved-memory regions to be remapped into any unprivileged > guests, until reserved-memory regions are properly supported in Xen. For > now, do not call iomem_permit_access on them, because giving > iomem_permit_access to dom0 means that the toolstack will be able to > assign the region to a domU. > > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> > --- > > Changes in v6: > - compare against "/reserved-memory/" > > Changes in v5: > - fix check condition > - use strnicmp > - return error > - improve commit message > > Changes in v4: > - compare the parent name with reserved-memory > - use dt_node_cmp > > Changes in v3: > - new patch > --- > xen/arch/arm/domain_build.c | 24 ++++++++++++++++-------- > 1 file changed, 16 insertions(+), 8 deletions(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 4c8404155a..673ffa453f 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -1155,15 +1155,23 @@ static int __init map_range_to_domain(const struct dt_device_node *dev, > bool need_mapping = !dt_device_for_passthrough(dev); > int res; > > - res = iomem_permit_access(d, paddr_to_pfn(addr), > - paddr_to_pfn(PAGE_ALIGN(addr + len - 1))); > - if ( res ) > + /* > + * Don't give iomem permissions for reserved-memory ranges to domUs In upstream Xen, this is only called for Dom0. So what are you referring to? > + * until reserved-memory support is complete. But as I pointed out before reserved-memory support is not going to happen via iomem. This is an abuse of the interface. > + */ A better comment would be: "reserved-memory regions are RAM carved out for special purpose. They are not MMIO and therefore a domain should not be able to manage them via the IOMEM interface". > + if ( strnicmp(dt_node_full_name(dev), "/reserved-memory/", > + strlen("/reserved-memory/")) != 0 ) > { > - printk(XENLOG_ERR "Unable to permit to dom%d access to" > - " 0x%"PRIx64" - 0x%"PRIx64"\n", > - d->domain_id, > - addr & PAGE_MASK, PAGE_ALIGN(addr + len) - 1); > - return res; > + res = iomem_permit_access(d, paddr_to_pfn(addr), > + paddr_to_pfn(PAGE_ALIGN(addr + len - 1))); > + if ( res ) > + { > + printk(XENLOG_ERR "Unable to permit to dom%d access to" > + " 0x%"PRIx64" - 0x%"PRIx64"\n", > + d->domain_id, > + addr & PAGE_MASK, PAGE_ALIGN(addr + len) - 1); > + return res; > + } > } > > if ( need_mapping ) > Cheers,
On Fri, 16 Aug 2019, Julien Grall wrote: > On 16/08/2019 00:36, Stefano Stabellini wrote: > > Don't allow reserved-memory regions to be remapped into any unprivileged > > guests, until reserved-memory regions are properly supported in Xen. For > > now, do not call iomem_permit_access on them, because giving > > iomem_permit_access to dom0 means that the toolstack will be able to > > assign the region to a domU. > > > > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> > > --- > > > > Changes in v6: > > - compare against "/reserved-memory/" > > > > Changes in v5: > > - fix check condition > > - use strnicmp > > - return error > > - improve commit message > > > > Changes in v4: > > - compare the parent name with reserved-memory > > - use dt_node_cmp > > > > Changes in v3: > > - new patch > > --- > > xen/arch/arm/domain_build.c | 24 ++++++++++++++++-------- > > 1 file changed, 16 insertions(+), 8 deletions(-) > > > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > > index 4c8404155a..673ffa453f 100644 > > --- a/xen/arch/arm/domain_build.c > > +++ b/xen/arch/arm/domain_build.c > > @@ -1155,15 +1155,23 @@ static int __init map_range_to_domain(const struct > > dt_device_node *dev, > > bool need_mapping = !dt_device_for_passthrough(dev); > > int res; > > - res = iomem_permit_access(d, paddr_to_pfn(addr), > > - paddr_to_pfn(PAGE_ALIGN(addr + len - 1))); > > - if ( res ) > > + /* > > + * Don't give iomem permissions for reserved-memory ranges to domUs > > In upstream Xen, this is only called for Dom0. So what are you referring to? > > > + * until reserved-memory support is complete. > > But as I pointed out before reserved-memory support is not going to happen via > iomem. This is an abuse of the interface. > > > + */ > > A better comment would be: > > "reserved-memory regions are RAM carved out for special purpose. They are not > MMIO and therefore a domain should not be able to manage them via the IOMEM > interface". Thank you for providing the comment. I'll use your version. > > + if ( strnicmp(dt_node_full_name(dev), "/reserved-memory/", > > + strlen("/reserved-memory/")) != 0 ) > > { > > - printk(XENLOG_ERR "Unable to permit to dom%d access to" > > - " 0x%"PRIx64" - 0x%"PRIx64"\n", > > - d->domain_id, > > - addr & PAGE_MASK, PAGE_ALIGN(addr + len) - 1); > > - return res; > > + res = iomem_permit_access(d, paddr_to_pfn(addr), > > + paddr_to_pfn(PAGE_ALIGN(addr + len - 1))); > > + if ( res ) > > + { > > + printk(XENLOG_ERR "Unable to permit to dom%d access to" > > + " 0x%"PRIx64" - 0x%"PRIx64"\n", > > + d->domain_id, > > + addr & PAGE_MASK, PAGE_ALIGN(addr + len) - 1); > > + return res; > > + } > > } > > if ( need_mapping ) > > > > Cheers, > > -- > Julien Grall >
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 4c8404155a..673ffa453f 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -1155,15 +1155,23 @@ static int __init map_range_to_domain(const struct dt_device_node *dev, bool need_mapping = !dt_device_for_passthrough(dev); int res; - res = iomem_permit_access(d, paddr_to_pfn(addr), - paddr_to_pfn(PAGE_ALIGN(addr + len - 1))); - if ( res ) + /* + * Don't give iomem permissions for reserved-memory ranges to domUs + * until reserved-memory support is complete. + */ + if ( strnicmp(dt_node_full_name(dev), "/reserved-memory/", + strlen("/reserved-memory/")) != 0 ) { - printk(XENLOG_ERR "Unable to permit to dom%d access to" - " 0x%"PRIx64" - 0x%"PRIx64"\n", - d->domain_id, - addr & PAGE_MASK, PAGE_ALIGN(addr + len) - 1); - return res; + res = iomem_permit_access(d, paddr_to_pfn(addr), + paddr_to_pfn(PAGE_ALIGN(addr + len - 1))); + if ( res ) + { + printk(XENLOG_ERR "Unable to permit to dom%d access to" + " 0x%"PRIx64" - 0x%"PRIx64"\n", + d->domain_id, + addr & PAGE_MASK, PAGE_ALIGN(addr + len) - 1); + return res; + } } if ( need_mapping )
Don't allow reserved-memory regions to be remapped into any unprivileged guests, until reserved-memory regions are properly supported in Xen. For now, do not call iomem_permit_access on them, because giving iomem_permit_access to dom0 means that the toolstack will be able to assign the region to a domU. Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> --- Changes in v6: - compare against "/reserved-memory/" Changes in v5: - fix check condition - use strnicmp - return error - improve commit message Changes in v4: - compare the parent name with reserved-memory - use dt_node_cmp Changes in v3: - new patch --- xen/arch/arm/domain_build.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-)