Hi,

On 17/12/2021 13:23, Oleksii Moisieiev wrote:
+static int map_memory_to_domain(struct domain *d, uint64_t addr, uint64_t len)
+{
+    return iomem_permit_access(d, paddr_to_pfn(addr),
+                paddr_to_pfn(PAGE_ALIGN(addr + len -1)));
+}
+
+static int unmap_memory_from_domain(struct domain *d, uint64_t addr,
+                                     uint64_t len)
+{
+    return iomem_deny_access(d, paddr_to_pfn(addr),
+                paddr_to_pfn(PAGE_ALIGN(addr + len -1)));
+}

I wonder, why we need an extra level of indirection here. And if this is
really needed, I wonder why map(unmap)_memory* name was chosen, as there is
no memory mapping/unmapping really happens here.


I've added extra indirection to hide math like
paddr_to_pfn(PAGE_ALIGN(addr + len -1)
so you don't have to math in each call. unmap_memory_from_domain called
from 2 places, so I moved both calls to separate function.
Although, I agree that map/unmap is not perfect name. I consider
renaming it to mem_permit_acces and mam_deny_access.

I haven't looked at the rest of the series. But this discussion caught my eye. This code implies that the address is page-aligned but the length not. Is that intended?

That said, if you give permission to the domain on a full page then it means it may be able to access address it should not. Can you explain why this is fine?

Cheers,

--
Julien Grall

Reply via email to