On 2016/1/8 5:40, Daniel De Graaf wrote:

On 01/07/2016 05:50 AM, Jan Beulich wrote:

quoted

quoted

quoted

quoted

On 07.01.16 at 10:11, [off-list ref] wrote:

Hi Jan,

On 2016/1/7 15:45, Jan Beulich wrote:

quoted

quoted

quoted

quoted

On 07.01.16 at 07:58, [off-list ref] wrote:

On 2015/11/17 19:04, Jan Beulich wrote:

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

On 17.11.15 at 10:40, [off-list ref] wrote:

--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c

@@ -1138,6 +1138,10 @@ int xenmem_add_to_physmap_one(
          rcu_unlock_domain(od);
          break;
      }
+    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. 

Yeah, since it's hard to know the MMIO range from the DSDT in XEN, we
permit full mmio capabilities for Dom0 and deny mmio access for some
devices e.g. uart. Then when Dom0 add those devices, call
XENMEM_add_to_physmap to map their MMIO ranges. This looks similar with
what x86 does.

    /* The hardware domain is initially permitted full I/O capabilities. */
    rc |= ioports_permit_access(d, 0, 0xFFFF);
    rc |= iomem_permit_access(d, 0UL, ~0UL);
    rc |= irqs_permit_access(d, 1, nr_irqs_gsi - 1);

The iomem_permit_access call there also seems
to be redundant because it is the same range that was just checked.

Ah, I'll drop this at next version.

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.

Thanks,
-- 
Shannon