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

2015-07-16 Thread George Dunlap 
On Thu, Jul 16, 2015 at 7:52 AM, Tiejun Chen  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 
> CC: Keir Fraser 
> CC: Jan Beulich 
> CC: Andrew Cooper 
> CC: Suravee Suthikulpanit 
> CC: Aravind Gopalakrishnan 
> CC: Ian Campbell 
> CC: Stefano Stabellini 
> CC: Yang Zhang 
> CC: Kevin Tian 
> Signed-off-by: Tiejun Chen 

With or without the "flags &" change:

Reviewed-by: George Dunlap 

> ---
> v8:
>
> * Force to pass "0"(strict) when add or move a device in hardware domain,
>   and improve some associated code comments.
>
> 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 | 37 
> ++---
>  xen/include/asm-x86/p2m.h   |  2 +-
>  xen/include/public/domctl.h |  3 +++
>  xen/include/xen/iommu.h |  2 +-
>  9 files changed, 55 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->pla

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

2015-07-16 Thread Jan Beulich 
>>> On 16.07.15 at 09:48,  wrote:
> On 2015/7/16 15:40, Jan Beulich wrote:
> On 16.07.15 at 08:52,  wrote:
>>> @@ -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 )
>>
>> Didn't we settle on flag & ~XEN_DOMCTL_DEV_RDM_RELAXED?
> 
> Sorry its my fault to miss this merge.
> 
> BTW, could I resend this patch separately to get your Ack? If you don't 
> have other objections.

Actually if we were to commit (parts of) this version, I'd even be fine
with doing the adjustment upon commit. So I'd suggest making the
change in your local copy so it would be as intended on v9, should
that need sending.

Jan


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

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

2015-07-16 Thread Chen, Tiejun 

On 2015/7/16 15:40, Jan Beulich wrote:

On 16.07.15 at 08:52,  wrote:

@@ -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 )


Didn't we settle on flag & ~XEN_DOMCTL_DEV_RDM_RELAXED?


Sorry its my fault to miss this merge.

BTW, could I resend this patch separately to get your Ack? If you don't 
have other objections.


Thanks
Tiejun

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

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

2015-07-16 Thread Jan Beulich 
>>> On 16.07.15 at 08:52,  wrote:
> @@ -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 )

Didn't we settle on flag & ~XEN_DOMCTL_DEV_RDM_RELAXED?

Jan


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

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

2015-07-15 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 
CC: Keir Fraser 
CC: Jan Beulich 
CC: Andrew Cooper 
CC: Suravee Suthikulpanit 
CC: Aravind Gopalakrishnan 
CC: Ian Campbell 
CC: Stefano Stabellini 
CC: Yang Zhang 
CC: Kevin Tian 
Signed-off-by: Tiejun Chen 
---
v8:

* Force to pass "0"(strict) when add or move a device in hardware domain,
  and improve some associated code comments.

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 | 37 ++---
 xen/include/asm-x86/p2m.h   |  2 +-
 xen/include/public/domctl.h |  3 +++
 xen/include/xen/iommu.h |  2 +-
 9 files changed, 55 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 +1335,7 @@ static int device_assigned(u16 seg, u8 bus, u8 devfn)
 return pdev ? 0 : -EBUSY