Re: [v4][PATCH 04/19] xen/passthrough: extend hypercall to support rdm reservation policy

["Chen, Tiejun" <tiejun.chen@intel.com>]

> When you say "not tools", I take it to mean that you're not exposing
> that option through the libxl interface?

Yes.

>
> tools/libxc/xc_domain.c:xc_assign_dt_device() most certainly does pass
> it in, and that's the level I'm talking about.  Someone reviewing this
> patch series needs to know, when xc or libxl set NO_RDM, what will be
> the effect?  The fact that libxc *shouldn't* set NO_RDM for PCI devices
> doesn't mean it won't happen.
>
> Now looking at the end of the series and grepping for
> "XEN_DOMCTL_DEV.*RDM", these values are *read and acted on* in exactly
> two places:
>
> xen/arch/x86/mm/p2m.c: The whole point of this series; if the p2m is
> occupied already, and flag == RDM_STRICT, return an error; otherwise
> ignore it.
>
> xen/drivers/passthrough/device_tree.c: If flag != NO_RDM, return an error.
>
> So the meaning of the flags is:
> For pci devices:
>   - RDM_RELAXED, NO_RDM: ignore conflicts in set_identity_p2m_entry()
>   - RDM_STRICT: error on conflicts in set_identity_p2m_entry()
> for dt devices:
>   - Error if not NO_RDM

Correct.

>
> It doesn't look to me like the NO_RDM setting actually adds any semantic
> meaning.
>
> What I see in the list of references you gave is a request from the list
> below is Julien saying this:
>
> "I would also add a XEN_DOMCTL_DEV_NO_RDM that would be use for non-PCI
> assignment."
>
> It looks a bit like what you did is said, "Well Julien asked for a
> NO_RDM setting, so here's a NO_RDM setting."  Which while perhaps
> understandable, doesn't make the value have any more usefulness.
>
> It seems to me that the real problem is that you had two values to begin
> with, rather than actually having flags (as the name would imply).
>
> This what I would suggest.  Make a single flag:
>
> #define _XEN_DOMCTL_DEV_RDM_RELAXED     0
> #define XEN_DOMCTL_DEV_RDM_RELAXED      (1U<<_XEN_DOMCTL_DEV_RDM_RELAXED)
>
> Then make the meaning of the flags as follows:
> * for pci devices:
>   - RDM_RELAXED flag SET: ignore conflicts in set_identity_p2m_entry()
>   - RDM_RELAXED flag CLEAR: error on conflicts in set_identity_p2m_entry()

No problem.

> * for dt devices:
>   - Ignore this flag entirely

But we still a flag to assign_device() like this,

diff --git a/xen/drivers/passthrough/device_tree.c 
b/xen/drivers/passthrough/device_tree.c
index 5d3842a..a182487 100644
--- a/xen/drivers/passthrough/device_tree.c
+++ b/xen/drivers/passthrough/device_tree.c
@@ -52,7 +52,8 @@ int iommu_assign_dt_device(struct domain *d, struct 
dt_device_node *dev)
              goto fail;
      }

-    rc = hd->platform_ops->assign_device(d, 0, dt_to_dev(dev));
+    rc = hd->platform_ops->assign_device(d, 0, dt_to_dev(dev),
+                                         XEN_DOMCTL_DEV_RDM_RELAXED);

      if ( rc )
          goto fail;

Or rc = hd->platform_ops->assign_device(d, 0, dt_to_dev(dev), 0)?

Thanks
Tiejun