[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 59/62] xen/arm: Add a hypercall for device mmio mapping

On 01/07/2016 05:50 AM, Jan Beulich wrote:
On 07.01.16 at 10:11, <zhaoshenglong@xxxxxxxxxx> wrote:
Hi Jan,

On 2016/1/7 15:45, Jan Beulich wrote:
On 07.01.16 at 07:58, <zhaoshenglong@xxxxxxxxxx> wrote:
On 2015/11/17 19:04, Jan Beulich wrote:
On 17.11.15 at 10:40, <shannon.zhao@xxxxxxxxxx> wrote:
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1138,6 +1138,10 @@ int xenmem_add_to_physmap_one(
+    case XENMAPSPACE_dev_mmio:
+        rc = map_dev_mmio_region(d, gpfn, 1, idx);
+        return rc;
+        break;
Blindly for any kind of domain? The XSM check in the
XENMEM_add_to_physmap_batch handler (in common code) doesn't
even know which map space is to be used...

Sorry, I know little about XSM. Could you suggest me how to add the
check for this new type here?
I'm sorry to push back here, but did you at least try to derive
what is wanted from the multitude of other XSM checks present
throughout the tree?

IIUC, you mean that it doean't need to change the XSM check itself, but
we should check if the current->domain is hardware domain and it maps
the space to itself before the XSM check, right?

No, I actually think that you need to add a new, secondary XSM
check. But you may want to consult with Daniel (who so far wasn't
even Cc-ed).

Looking at the original patch, I am not sure if I understand the
checks: it seems like the iomem_access_permitted check is being done
on the guest's page range instead of the actual IO memory, which
ends up allowing the guest to map anything as long as it maps it in
the right guest area.  The iomem_permit_access call there also seems
to be redundant because it is the same range that was just checked.

If the [start_gfn, start_gfn + nr) memory range actually describes
the physical addresses, then this operation is taking advantage of
the existing XSM checks on XEN_DOMCTL_iomem_permission, and the
only XSM check that is needed would be that current->domain has
permission to modify (d)'s mappings - and this is done by the
xsm_add_to_physmap check in XENMEM_add_to_physmap.

Daniel De Graaf
National Security Agency

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.