Re: [Xen-devel] [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-14 Thread Chen, Tiejun

The way this sort of thing is defined in the rest of domctl.h is like this:

#define _XEN_DOMCTL_CDF_hvm_guest 0
#define XEN_DOMCTL_CDF_hvm_guest  (1U_XEN_DOMCTL_CDF_hvm_guest)

So the above should be

#define _XEN_DOMCTL_DEV_RDM_RELAXED 0
#define XEN_DOMCTL_DEV_RDM_RELAXED (1U_XEN_DOMCTL_DEV_RDM_RELAXED)

And then your check in iommu_do_pci_domctl() would look like

if (flag  ~XEN_DOMCTL_DEV_RDM_RELAXED)

And if we end up adding any extra flags, we just | them into the above
conditional, as is done in, for example, the XEN_DOMCTL_createdomain
case in xen/common/domctl.c:do_domctl().



Seems Jan didn't like this way IIRC, so I hope Jan also can have a look 
at this beforehand :)


Thanks
Tiejun

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-14 Thread George Dunlap
On 07/13/2015 07:47 AM, Chen, Tiejun wrote:
 Thanks for this; a few more comments...

 
 Thanks for your time.
 
 @@ -1577,9 +1578,15 @@ int iommu_do_pci_domctl(
   seg = machine_sbdf  16;
   bus = PCI_BUS(machine_sbdf);
   devfn = PCI_DEVFN2(machine_sbdf);
 +flag = domctl-u.assign_device.flag;
 +if ( flag  XEN_DOMCTL_DEV_RDM_RELAXED )

 This is not a blocker, but a stylistic comment: I would have inverted
 the bitmask here, as that's conceptually what you're checking.  I
 won't make this a blocker for going in.
 
 What about this?
 
 diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
 index 6e23fc6..17a4206 100644
 --- a/xen/drivers/passthrough/pci.c
 +++ b/xen/drivers/passthrough/pci.c
 @@ -1579,7 +1579,7 @@ int iommu_do_pci_domctl(
  bus = PCI_BUS(machine_sbdf);
  devfn = PCI_DEVFN2(machine_sbdf);
  flag = domctl-u.assign_device.flag;
 -if ( flag  XEN_DOMCTL_DEV_RDM_RELAXED )
 +if ( flag  ~XEN_DOMCTL_DEV_RDM_MASK )
  {
  ret = -EINVAL;
  break;
 diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
 index bca25c9..07549a4 100644
 --- a/xen/include/public/domctl.h
 +++ b/xen/include/public/domctl.h
 @@ -480,6 +480,7 @@ struct xen_domctl_assign_device {
  } u;
  /* IN */
  #define XEN_DOMCTL_DEV_RDM_RELAXED  1
 +#define XEN_DOMCTL_DEV_RDM_MASK 0x1

The way this sort of thing is defined in the rest of domctl.h is like this:

#define _XEN_DOMCTL_CDF_hvm_guest 0
#define XEN_DOMCTL_CDF_hvm_guest  (1U_XEN_DOMCTL_CDF_hvm_guest)

So the above should be

#define _XEN_DOMCTL_DEV_RDM_RELAXED 0
#define XEN_DOMCTL_DEV_RDM_RELAXED (1U_XEN_DOMCTL_DEV_RDM_RELAXED)

And then your check in iommu_do_pci_domctl() would look like

if (flag  ~XEN_DOMCTL_DEV_RDM_RELAXED)

And if we end up adding any extra flags, we just | them into the above
conditional, as is done in, for example, the XEN_DOMCTL_createdomain
case in xen/common/domctl.c:do_domctl().

Thanks,
 -George


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-14 Thread George Dunlap
On 07/14/2015 11:53 AM, Chen, Tiejun wrote:
 The way this sort of thing is defined in the rest of domctl.h is like
 this:

 #define _XEN_DOMCTL_CDF_hvm_guest 0
 #define XEN_DOMCTL_CDF_hvm_guest  (1U_XEN_DOMCTL_CDF_hvm_guest)

 So the above should be

 #define _XEN_DOMCTL_DEV_RDM_RELAXED 0
 #define XEN_DOMCTL_DEV_RDM_RELAXED (1U_XEN_DOMCTL_DEV_RDM_RELAXED)

 And then your check in iommu_do_pci_domctl() would look like

 if (flag  ~XEN_DOMCTL_DEV_RDM_RELAXED)

 And if we end up adding any extra flags, we just | them into the above
 conditional, as is done in, for example, the XEN_DOMCTL_createdomain
 case in xen/common/domctl.c:do_domctl().

 
 Seems Jan didn't like this way IIRC, so I hope Jan also can have a look
 at this beforehand :)

I think Jan thought that the MASK value you defined wasn't meant to be a
single flag, but for all the flags; i.e., that if we added flags in bits
1 and 2, that MASK would become 0x7 rather than 0x1.  And I agree that
there's not much point to having such a mask defined in the public header.

But what I'm doing above is making explicit what you have already; i.e.,
you just set XEN_DOMCTL_DEV_RDM_RELAXED to '1'; the reader has to sort
of infer that the reason '1' is chosen is that it's setting bit 0.
Doing it the way I suggest makes it more clear that this is meant to be
a bitfield, and '0' has been allocated.

Please correct me if I'm wrong, Jan.

 -George

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-14 Thread George Dunlap
On 07/14/2015 12:45 PM, Jan Beulich wrote:
 On 14.07.15 at 13:30, george.dun...@eu.citrix.com wrote:
 On 07/14/2015 11:53 AM, Chen, Tiejun wrote:
 The way this sort of thing is defined in the rest of domctl.h is like
 this:

 #define _XEN_DOMCTL_CDF_hvm_guest 0
 #define XEN_DOMCTL_CDF_hvm_guest  (1U_XEN_DOMCTL_CDF_hvm_guest)

 So the above should be

 #define _XEN_DOMCTL_DEV_RDM_RELAXED 0
 #define XEN_DOMCTL_DEV_RDM_RELAXED (1U_XEN_DOMCTL_DEV_RDM_RELAXED)

 And then your check in iommu_do_pci_domctl() would look like

 if (flag  ~XEN_DOMCTL_DEV_RDM_RELAXED)

 And if we end up adding any extra flags, we just | them into the above
 conditional, as is done in, for example, the XEN_DOMCTL_createdomain
 case in xen/common/domctl.c:do_domctl().


 Seems Jan didn't like this way IIRC, so I hope Jan also can have a look
 at this beforehand :)

 I think Jan thought that the MASK value you defined wasn't meant to be a
 single flag, but for all the flags; i.e., that if we added flags in bits
 1 and 2, that MASK would become 0x7 rather than 0x1.  And I agree that
 there's not much point to having such a mask defined in the public header.

 But what I'm doing above is making explicit what you have already; i.e.,
 you just set XEN_DOMCTL_DEV_RDM_RELAXED to '1'; the reader has to sort
 of infer that the reason '1' is chosen is that it's setting bit 0.
 Doing it the way I suggest makes it more clear that this is meant to be
 a bitfield, and '0' has been allocated.

 Please correct me if I'm wrong, Jan.
 
 Indeed my primary objection was to what you describe. That said,
 I'm not too happy with what you propose now either. Not only do
 I view this (bit-pos,bit-mask) tuple as redundant unless one actually
 needs both in certain places (which doesn't seem to be the case
 here), but the naming also conflicts with the C standard (reserving
 identifiers starting with underscore and an upper case letter). Just
 like said in various other cases - we've got many examples of such
 violations already, but I'd prefer not to make the situation worse.
 
 IOW I'd prefer to go with just
 
 #define XEN_DOMCTL_DEV_RDM_RELAXED 1

Very well.

 -George

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-14 Thread Jan Beulich
 On 14.07.15 at 13:30, george.dun...@eu.citrix.com wrote:
 On 07/14/2015 11:53 AM, Chen, Tiejun wrote:
 The way this sort of thing is defined in the rest of domctl.h is like
 this:

 #define _XEN_DOMCTL_CDF_hvm_guest 0
 #define XEN_DOMCTL_CDF_hvm_guest  (1U_XEN_DOMCTL_CDF_hvm_guest)

 So the above should be

 #define _XEN_DOMCTL_DEV_RDM_RELAXED 0
 #define XEN_DOMCTL_DEV_RDM_RELAXED (1U_XEN_DOMCTL_DEV_RDM_RELAXED)

 And then your check in iommu_do_pci_domctl() would look like

 if (flag  ~XEN_DOMCTL_DEV_RDM_RELAXED)

 And if we end up adding any extra flags, we just | them into the above
 conditional, as is done in, for example, the XEN_DOMCTL_createdomain
 case in xen/common/domctl.c:do_domctl().

 
 Seems Jan didn't like this way IIRC, so I hope Jan also can have a look
 at this beforehand :)
 
 I think Jan thought that the MASK value you defined wasn't meant to be a
 single flag, but for all the flags; i.e., that if we added flags in bits
 1 and 2, that MASK would become 0x7 rather than 0x1.  And I agree that
 there's not much point to having such a mask defined in the public header.
 
 But what I'm doing above is making explicit what you have already; i.e.,
 you just set XEN_DOMCTL_DEV_RDM_RELAXED to '1'; the reader has to sort
 of infer that the reason '1' is chosen is that it's setting bit 0.
 Doing it the way I suggest makes it more clear that this is meant to be
 a bitfield, and '0' has been allocated.
 
 Please correct me if I'm wrong, Jan.

Indeed my primary objection was to what you describe. That said,
I'm not too happy with what you propose now either. Not only do
I view this (bit-pos,bit-mask) tuple as redundant unless one actually
needs both in certain places (which doesn't seem to be the case
here), but the naming also conflicts with the C standard (reserving
identifiers starting with underscore and an upper case letter). Just
like said in various other cases - we've got many examples of such
violations already, but I'd prefer not to make the situation worse.

IOW I'd prefer to go with just

#define XEN_DOMCTL_DEV_RDM_RELAXED 1

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-13 Thread Jan Beulich
 On 13.07.15 at 08:47, tiejun.c...@intel.com wrote:
 --- a/xen/include/public/domctl.h
 +++ b/xen/include/public/domctl.h
 @@ -480,6 +480,7 @@ struct xen_domctl_assign_device {
   } u;
   /* IN */
   #define XEN_DOMCTL_DEV_RDM_RELAXED  1
 +#define XEN_DOMCTL_DEV_RDM_MASK 0x1

As said before - I dislike this mask being made part of the public
interface, albeit it being a domctl thing makes it a minor issue.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-13 Thread Chen, Tiejun

Thanks for this; a few more comments...



Thanks for your time.


@@ -1577,9 +1578,15 @@ int iommu_do_pci_domctl(
  seg = machine_sbdf  16;
  bus = PCI_BUS(machine_sbdf);
  devfn = PCI_DEVFN2(machine_sbdf);
+flag = domctl-u.assign_device.flag;
+if ( flag  XEN_DOMCTL_DEV_RDM_RELAXED )


This is not a blocker, but a stylistic comment: I would have inverted
the bitmask here, as that's conceptually what you're checking.  I
won't make this a blocker for going in.


What about this?

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 6e23fc6..17a4206 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1579,7 +1579,7 @@ int iommu_do_pci_domctl(
 bus = PCI_BUS(machine_sbdf);
 devfn = PCI_DEVFN2(machine_sbdf);
 flag = domctl-u.assign_device.flag;
-if ( flag  XEN_DOMCTL_DEV_RDM_RELAXED )
+if ( flag  ~XEN_DOMCTL_DEV_RDM_MASK )
 {
 ret = -EINVAL;
 break;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index bca25c9..07549a4 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -480,6 +480,7 @@ struct xen_domctl_assign_device {
 } u;
 /* IN */
 #define XEN_DOMCTL_DEV_RDM_RELAXED  1
+#define XEN_DOMCTL_DEV_RDM_MASK 0x1
 uint32_t  flag;   /* flag of assigned device */
 };
 typedef struct xen_domctl_assign_device xen_domctl_assign_device_t;




@@ -1898,7 +1899,14 @@ static int intel_iommu_add_device(u8 devfn, struct 
pci_dev *pdev)
   PCI_BUS(bdf) == pdev-bus 
   PCI_DEVFN2(bdf) == devfn )
  {
-ret = rmrr_identity_mapping(pdev-domain, 1, rmrr);
+/*
+ * Here means we're add a device to the hardware domain
+ * so actually RMRR is always reserved on e820 so either
+ * of flag is fine for hardware domain and here we'd like
+ * to pass XEN_DOMCTL_DEV_RDM_RELAXED.
+ */


Sorry I didn't give feedback on your comment before.  I don't find it
clear enough; I'd suggest something like this:

iommu_add_device() is only called for the hardware domain (see
xen/drivers/passthrough/pci.c:pci_add_device()).  Since RMRRs are
always reserved in the e820 map for the hardware domain, there
shouldn't be a conflict.


Loos good and thanks.



I also said that if we went with anything other than STRICT that we'd
need to check to make sure that the domain really was the hardware
domain before proceeding, in case the assumption that pdev-domain ==
hardware_domain ever changed.  (Perhaps with an ASSERT -- Jan, what do
you think?)


Sounds reasonable.



Also, passing in RELAXED in locations where the flag is completely
ignored (such as when removing mappings) doesn't really make any
sense.

On the whole I think it would be better if you removed the RELAXED
flag for both removals and for hardware domains.



Just as I said in another email I agreed your STRICT way.

Thanks
Tiejun

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-13 Thread Chen, Tiejun

I was saying two things in the above paragraph:

1. For removal, there's no point in passing in anything other than '0'
for flags, since it's ignored.  Passing a non-0 value implies that the
flags will have some effect, which is misleading.

2. For places we know we're adding to hw domains, I think it makes most
sense also to pass in '0', to imply STRICT.

But if instead they insist on passing RELAXED, then please add an
ASSERT(pdev-domain == hw_domain) or something of the kind to
intel_iommu_add_device().  (If defaulting to STRICT, I don't think the
ASSERT is necessary anymore.)



I agree and also looks Jan didn't oppose this STRICT way by setting 0 
directly, so lets do this.


Thanks
Tiejun


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-12 Thread Chen, Tiejun

Also, passing in RELAXED in locations where the flag is completely
ignored (such as when removing mappings) doesn't really make any
sense.

On the whole I think it would be better if you removed the RELAXED
flag for both removals and for hardware domains.


But what would he pass instead? Or wait - iirc I had even suggested
a way to do so by combining two arguments. Would need to go dig
that out, because I think the idea got dropped without good reason.



No, I don't drop this directly.

http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg01877.html

I went there with one optional way you provided that I just need to a 
brief comment. And I also had reply at that moment.


http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg02101.html

Thanks
Tiejun

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-10 Thread Jan Beulich
 On 10.07.15 at 15:26, george.dun...@eu.citrix.com wrote:
 I also said that if we went with anything other than STRICT that we'd
 need to check to make sure that the domain really was the hardware
 domain before proceeding, in case the assumption that pdev-domain ==
 hardware_domain ever changed.  (Perhaps with an ASSERT -- Jan, what do
 you think?)

Yes, such an ASSERT() seems okay/desirable.

 Also, passing in RELAXED in locations where the flag is completely
 ignored (such as when removing mappings) doesn't really make any
 sense.
 
 On the whole I think it would be better if you removed the RELAXED
 flag for both removals and for hardware domains.

But what would he pass instead? Or wait - iirc I had even suggested
a way to do so by combining two arguments. Would need to go dig
that out, because I think the idea got dropped without good reason.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-10 Thread George Dunlap
On Thu, Jul 9, 2015 at 6:33 AM, Tiejun Chen tiejun.c...@intel.com wrote:
 This patch extends the existing hypercall to support rdm reservation policy.
 We return error or just throw out a warning message depending on whether
 the policy is strict or relaxed when reserving RDM regions in pfn space.
 Note in some special cases, e.g. add a device to hwdomain, and remove a
 device from user domain, 'relaxed' is fine enough since this is always safe
 to hwdomain.

 CC: Tim Deegan t...@xen.org
 CC: Keir Fraser k...@xen.org
 CC: Jan Beulich jbeul...@suse.com
 CC: Andrew Cooper andrew.coop...@citrix.com
 CC: Suravee Suthikulpanit suravee.suthikulpa...@amd.com
 CC: Aravind Gopalakrishnan aravind.gopalakrish...@amd.com
 CC: Ian Campbell ian.campb...@citrix.com
 CC: Stefano Stabellini stefano.stabell...@citrix.com
 CC: Yang Zhang yang.z.zh...@intel.com
 CC: Kevin Tian kevin.t...@intel.com
 Signed-off-by: Tiejun Chen tiejun.c...@intel.com
 ---
 v6 ~ v7:

 * Nothing is changed.

 v5:

 * Just leave one bit XEN_DOMCTL_DEV_RDM_RELAXED as our flag, so
   0 means strict and 1 means relaxed.

Thanks for this; a few more comments...

 @@ -1577,9 +1578,15 @@ int iommu_do_pci_domctl(
  seg = machine_sbdf  16;
  bus = PCI_BUS(machine_sbdf);
  devfn = PCI_DEVFN2(machine_sbdf);
 +flag = domctl-u.assign_device.flag;
 +if ( flag  XEN_DOMCTL_DEV_RDM_RELAXED )

This is not a blocker, but a stylistic comment: I would have inverted
the bitmask here, as that's conceptually what you're checking.  I
won't make this a blocker for going in.

 @@ -1898,7 +1899,14 @@ static int intel_iommu_add_device(u8 devfn, struct 
 pci_dev *pdev)
   PCI_BUS(bdf) == pdev-bus 
   PCI_DEVFN2(bdf) == devfn )
  {
 -ret = rmrr_identity_mapping(pdev-domain, 1, rmrr);
 +/*
 + * Here means we're add a device to the hardware domain
 + * so actually RMRR is always reserved on e820 so either
 + * of flag is fine for hardware domain and here we'd like
 + * to pass XEN_DOMCTL_DEV_RDM_RELAXED.
 + */

Sorry I didn't give feedback on your comment before.  I don't find it
clear enough; I'd suggest something like this:

iommu_add_device() is only called for the hardware domain (see
xen/drivers/passthrough/pci.c:pci_add_device()).  Since RMRRs are
always reserved in the e820 map for the hardware domain, there
shouldn't be a conflict.

I also said that if we went with anything other than STRICT that we'd
need to check to make sure that the domain really was the hardware
domain before proceeding, in case the assumption that pdev-domain ==
hardware_domain ever changed.  (Perhaps with an ASSERT -- Jan, what do
you think?)

Also, passing in RELAXED in locations where the flag is completely
ignored (such as when removing mappings) doesn't really make any
sense.

On the whole I think it would be better if you removed the RELAXED
flag for both removals and for hardware domains.

 -George

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-10 Thread George Dunlap
On 07/10/2015 04:01 PM, Jan Beulich wrote:
 On 10.07.15 at 15:26, george.dun...@eu.citrix.com wrote:
 I also said that if we went with anything other than STRICT that we'd
 need to check to make sure that the domain really was the hardware
 domain before proceeding, in case the assumption that pdev-domain ==
 hardware_domain ever changed.  (Perhaps with an ASSERT -- Jan, what do
 you think?)
 
 Yes, such an ASSERT() seems okay/desirable.
 
 Also, passing in RELAXED in locations where the flag is completely
 ignored (such as when removing mappings) doesn't really make any
 sense.

 On the whole I think it would be better if you removed the RELAXED
 flag for both removals and for hardware domains.
 
 But what would he pass instead? Or wait - iirc I had even suggested
 a way to do so by combining two arguments. Would need to go dig
 that out, because I think the idea got dropped without good reason.

No, I just meant to pass '0' for the flags (which would imply STRICT).

I was saying two things in the above paragraph:

1. For removal, there's no point in passing in anything other than '0'
for flags, since it's ignored.  Passing a non-0 value implies that the
flags will have some effect, which is misleading.

2. For places we know we're adding to hw domains, I think it makes most
sense also to pass in '0', to imply STRICT.

But if instead they insist on passing RELAXED, then please add an
ASSERT(pdev-domain == hw_domain) or something of the kind to
intel_iommu_add_device().  (If defaulting to STRICT, I don't think the
ASSERT is necessary anymore.)

 -George


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


[Xen-devel] [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-08 Thread Tiejun Chen
This patch extends the existing hypercall to support rdm reservation policy.
We return error or just throw out a warning message depending on whether
the policy is strict or relaxed when reserving RDM regions in pfn space.
Note in some special cases, e.g. add a device to hwdomain, and remove a
device from user domain, 'relaxed' is fine enough since this is always safe
to hwdomain.

CC: Tim Deegan t...@xen.org
CC: Keir Fraser k...@xen.org
CC: Jan Beulich jbeul...@suse.com
CC: Andrew Cooper andrew.coop...@citrix.com
CC: Suravee Suthikulpanit suravee.suthikulpa...@amd.com
CC: Aravind Gopalakrishnan aravind.gopalakrish...@amd.com
CC: Ian Campbell ian.campb...@citrix.com
CC: Stefano Stabellini stefano.stabell...@citrix.com
CC: Yang Zhang yang.z.zh...@intel.com
CC: Kevin Tian kevin.t...@intel.com
Signed-off-by: Tiejun Chen tiejun.c...@intel.com
---
v6 ~ v7:

* Nothing is changed.

v5:

* Just leave one bit XEN_DOMCTL_DEV_RDM_RELAXED as our flag, so
  0 means strict and 1 means relaxed.

* So make DT device ignore the flag field

* Improve the code comments

v4:

* Add code comments to describer why we fix to set a policy flag in some
  cases like adding a device to hwdomain, and removing a device from user 
domain.

* Avoid using fixed width types for the parameter of set_identity_p2m_entry()

* Fix one judging condition
  domctl-u.assign_device.flag == XEN_DOMCTL_DEV_NO_RDM
  - domctl-u.assign_device.flag != XEN_DOMCTL_DEV_NO_RDM

* Add to range check the flag passed to make future extensions possible
  (and to avoid ambiguity on what out of range values would mean).

 xen/arch/x86/mm/p2m.c   |  7 +++--
 xen/drivers/passthrough/amd/pci_amd_iommu.c |  3 ++-
 xen/drivers/passthrough/arm/smmu.c  |  2 +-
 xen/drivers/passthrough/device_tree.c   |  3 ++-
 xen/drivers/passthrough/pci.c   | 15 ---
 xen/drivers/passthrough/vtd/iommu.c | 40 +++--
 xen/include/asm-x86/p2m.h   |  2 +-
 xen/include/public/domctl.h |  3 +++
 xen/include/xen/iommu.h |  2 +-
 9 files changed, 58 insertions(+), 19 deletions(-)

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 99a26ca..47785dc 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -901,7 +901,7 @@ int set_mmio_p2m_entry(struct domain *d, unsigned long gfn, 
mfn_t mfn,
 }
 
 int set_identity_p2m_entry(struct domain *d, unsigned long gfn,
-   p2m_access_t p2ma)
+   p2m_access_t p2ma, unsigned int flag)
 {
 p2m_type_t p2mt;
 p2m_access_t a;
@@ -923,7 +923,10 @@ int set_identity_p2m_entry(struct domain *d, unsigned long 
gfn,
 ret = 0;
 else
 {
-ret = -EBUSY;
+if ( flag  XEN_DOMCTL_DEV_RDM_RELAXED )
+ret = 0;
+else
+ret = -EBUSY;
 printk(XENLOG_G_WARNING
Cannot setup identity map d%d:%lx,
 gfn already mapped to %lx.\n,
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c 
b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index e83bb35..920b35a 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -394,7 +394,8 @@ static int reassign_device(struct domain *source, struct 
domain *target,
 }
 
 static int amd_iommu_assign_device(struct domain *d, u8 devfn,
-   struct pci_dev *pdev)
+   struct pci_dev *pdev,
+   u32 flag)
 {
 struct ivrs_mappings *ivrs_mappings = get_ivrs_mappings(pdev-seg);
 int bdf = PCI_BDF2(pdev-bus, devfn);
diff --git a/xen/drivers/passthrough/arm/smmu.c 
b/xen/drivers/passthrough/arm/smmu.c
index 6cc4394..9a667e9 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2605,7 +2605,7 @@ static void arm_smmu_destroy_iommu_domain(struct 
iommu_domain *domain)
 }
 
 static int arm_smmu_assign_dev(struct domain *d, u8 devfn,
-  struct device *dev)
+  struct device *dev, u32 flag)
 {
struct iommu_domain *domain;
struct arm_smmu_xen_domain *xen_domain;
diff --git a/xen/drivers/passthrough/device_tree.c 
b/xen/drivers/passthrough/device_tree.c
index 5d3842a..7ff79f8 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));
+/* The flag field doesn't matter to DT device. */
+rc = hd-platform_ops-assign_device(d, 0, dt_to_dev(dev), 0);
 
 if ( rc )
 goto fail;
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index e30be43..6e23fc6 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1335,7