Hi Penny,
On 20/06/2022 06:11, Penny Zheng wrote:
If owner property is defined, then owner domain of a static shared memory
region is not the default dom_io anymore, but a specific domain.
This commit implements allocating static shared memory to a specific domain
when owner property is defined.
Coding flow for dealing borrower domain will be introduced later in the
following commits.
Signed-off-by: Penny Zheng <penny.zh...@arm.com>
Reviewed-by: Stefano Stabellini <sstabell...@kernel.org>
---
v5 change:
- no change
---
v4 change:
- no changes
---
v3 change:
- simplify the code since o_gbase is not used if the domain is dom_io
---
v2 change:
- P2M mapping is restricted to normal domain
- in-code comment fix
---
xen/arch/arm/domain_build.c | 44 +++++++++++++++++++++++++++----------
1 file changed, 33 insertions(+), 11 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 91a5ace851..d4fd64e2bd 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -805,9 +805,11 @@ static mfn_t __init acquire_shared_memory_bank(struct
domain *d,
*/
static int __init allocate_shared_memory(struct domain *d,
u32 addr_cells, u32 size_cells,
- paddr_t pbase, paddr_t psize)
+ paddr_t pbase, paddr_t psize,
+ paddr_t gbase)
{
mfn_t smfn;
+ int ret = 0;
dprintk(XENLOG_INFO,
"Allocate static shared memory BANK %#"PRIpaddr"-%#"PRIpaddr".\n",
@@ -822,8 +824,18 @@ static int __init allocate_shared_memory(struct domain *d,
* DOMID_IO is the domain, like DOMID_XEN, that is not auto-translated.
* It sees RAM 1:1 and we do not need to create P2M mapping for it
*/
- ASSERT(d == dom_io);
- return 0;
+ if ( d != dom_io )
+ {
+ ret = guest_physmap_add_pages(d, gaddr_to_gfn(gbase), smfn,
PFN_DOWN(psize));
Coding style: this line is over 80 characters. And...
+ if ( ret )
+ {
+ printk(XENLOG_ERR
+ "Failed to map shared memory to %pd.\n", d);
... this line could be merged with the previous one.
+ return ret;
+ }
+ }
+
+ return ret;
}
static int __init process_shm(struct domain *d,
@@ -836,6 +848,8 @@ static int __init process_shm(struct domain *d,
u32 shm_id;
u32 addr_cells, size_cells;
paddr_t gbase, pbase, psize;
+ const char *role_str;
+ bool owner_dom_io = true;
I think it would be best if role_str and owner_dom_io are defined within
the loop. Same goes for all the other declarations.
dt_for_each_child_node(node, shm_node)
{
@@ -862,19 +876,27 @@ static int __init process_shm(struct domain *d,
ASSERT(IS_ALIGNED(pbase, PAGE_SIZE) && IS_ALIGNED(psize, PAGE_SIZE));
gbase = dt_read_number(cells, addr_cells);
- /* TODO: Consider owner domain is not the default dom_io. */
+ /*
+ * "role" property is optional and if it is defined explicitly,
+ * then the owner domain is not the default "dom_io" domain.
+ */
+ if ( dt_property_read_string(shm_node, "role", &role_str) == 0 )
+ owner_dom_io = false
IIUC, the role is per-region. However, owner_dom_io is first initialized
to false outside the loop. Therefore, the variable may not be correct on
the next region.
So I think you want to write:
owner_dom_io = !dt_property_read_string(...);
This can also be avoided if you reduce the scope of the variable (it is
meant to only be used in the loop).
+
/*
* Per static shared memory region could be shared between multiple
* domains.
- * In case re-allocating the same shared memory region, we check
- * if it is already allocated to the default owner dom_io before
- * the actual allocation.
+ * So when owner domain is the default dom_io, in case re-allocating
+ * the same shared memory region, we check if it is already allocated
+ * to the default owner dom_io before the actual allocation.
*/
- if ( !is_shm_allocated_to_domio(pbase) )
+ if ( (owner_dom_io && !is_shm_allocated_to_domio(pbase)) ||
+ (!owner_dom_io && strcmp(role_str, "owner") == 0) )
{
- /* Allocate statically shared pages to the default owner dom_io. */
- ret = allocate_shared_memory(dom_io, addr_cells, size_cells,
- pbase, psize);
+ /* Allocate statically shared pages to the owner domain. */
+ ret = allocate_shared_memory(owner_dom_io ? dom_io : d,
+ addr_cells, size_cells,
+ pbase, psize, gbase);
if ( ret )
return ret;
}
Cheers,
--
Julien Grall