Re: [Xen-devel] [PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy
On 23.07.15 at 13:45, ian.jack...@eu.citrix.com wrote: Ian Jackson writes ([PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy): From: Tiejun Chen tiejun.c...@intel.com 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. This patch breaks the build on ARM: gcc -O1 -fno-omit-frame-pointer -marm -g -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs -I/local/scratch/ianj/xen.git/xen/include -fno-stack-protector -fno-exceptions -Wnested-externs -msoft-float -mcpu=cortex-a15 -DGCC_HAS_VISIBILITY_ATTRIBUTE -fno-builtin -fno-common -Werror -Wredundant-decls -Wno-pointer-arith -pipe -g -D__XEN__ -include /local/scratch/ianj/xen.git/xen/include/xen/config.h -nostdinc -fno-optimize-sibling-calls -DVERBOSE -DHAS_PASSTHROUGH -DHAS_DEVICE_TREE -DHAS_MEM_ACCESS -DHAS_PDX -fno-omit-frame-pointer -DCONFIG_FRAME_POINTER -MMD -MF .smmu.o.d -c smmu.c -o smmu.o smmu.c: In function 'arm_smmu_reassign_dev': smmu.c:2712:9: error: too few arguments to function 'arm_smmu_assign_dev' ret = arm_smmu_assign_dev(t, devfn, dev); ^ smmu.c:2607:12: note: declared here static int arm_smmu_assign_dev(struct domain *d, u8 devfn, ^ /local/scratch/ianj/xen.git/xen/Rules.mk:168: recipe for target 'smmu.o' failed make[6]: *** [smmu.o] Error 1 I had a quick look but it's not a simple matter of plumbing through an additional flags parameter becuase the reassign_device method apparently doesn't take flags. Considering that the parameter is ignored anyway, I'd suggest following what was done for various of the rmrr_identity_mapping() callers - just pass zero. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy
Ian Jackson writes ([PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy): From: Tiejun Chen tiejun.c...@intel.com 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. This patch breaks the build on ARM: gcc -O1 -fno-omit-frame-pointer -marm -g -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs -I/local/scratch/ianj/xen.git/xen/include -fno-stack-protector -fno-exceptions -Wnested-externs -msoft-float -mcpu=cortex-a15 -DGCC_HAS_VISIBILITY_ATTRIBUTE -fno-builtin -fno-common -Werror -Wredundant-decls -Wno-pointer-arith -pipe -g -D__XEN__ -include /local/scratch/ianj/xen.git/xen/include/xen/config.h -nostdinc -fno-optimize-sibling-calls -DVERBOSE -DHAS_PASSTHROUGH -DHAS_DEVICE_TREE -DHAS_MEM_ACCESS -DHAS_PDX -fno-omit-frame-pointer -DCONFIG_FRAME_POINTER -MMD -MF .smmu.o.d -c smmu.c -o smmu.o smmu.c: In function 'arm_smmu_reassign_dev': smmu.c:2712:9: error: too few arguments to function 'arm_smmu_assign_dev' ret = arm_smmu_assign_dev(t, devfn, dev); ^ smmu.c:2607:12: note: declared here static int arm_smmu_assign_dev(struct domain *d, u8 devfn, ^ /local/scratch/ianj/xen.git/xen/Rules.mk:168: recipe for target 'smmu.o' failed make[6]: *** [smmu.o] Error 1 I had a quick look but it's not a simple matter of plumbing through an additional flags parameter becuase the reassign_device method apparently doesn't take flags. Tiejun, please can you send a patch to fix this up. Please send just a revised version of this patch. I think the rest of the series will rebase just fine on top of it. (If I'm wrong then we will need to do something more complex.) Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy
On Thu, 2015-07-23 at 12:45 +0100, Ian Jackson wrote: I had a quick look but it's not a simple matter of plumbing through an additional flags parameter becuase the reassign_device method apparently doesn't take flags. The vtd version (reassign_device_ownership) does: /* * Any RMRR flag is always ignored when remove a device, * but its always safe and strict to set 0. */ ret = rmrr_identity_mapping(source, 0, rmrr, 0); Where that second 0 there was the flag when it was called from (intel_iommu_assign_device). Given that arm_smmu_assign_dev currently ignores the flag, and that ARM doesn't currently have RDM/RMRR at all I think just passing 0 in arm_smmu_reassign_dev would be tolerable. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy
On Thu, 2015-07-23 at 13:19 +0100, Ian Jackson wrote: Chen, Tiejun writes (Re: [PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy): Tiejun, please can you send a patch to fix this up. Please send just a revised version of this patch. I think the rest of the series will rebase just fine on top of it. (If I'm wrong then we will need to do something more complex.) Sorry to this. On ARM side the flag field doesn't take any affect. Signed-off-by: Tiejun Chen tiejun.c...@intel.com OK, thanks. I wasn't sure that just passing 0 was right. I will take Jan's and Ian's suggestions as acks for your patch, which looks correct to me to implement them. Can you avoid the mention of DT in the comment please, since PCI will eventually go that path. Something like No flags are defined for ARM would suffice I think. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy
Tiejun, please can you send a patch to fix this up. Please send just
a revised version of this patch. I think the rest of the series will
rebase just fine on top of it. (If I'm wrong then we will need to do
something more complex.)
Sorry to this.
On ARM side the flag field doesn't take any affect.
Signed-off-by: Tiejun Chen tiejun.c...@intel.com
diff --git a/xen/drivers/passthrough/arm/smmu.c
b/xen/drivers/passthrough/arm/smmu.c
index 9a667e9..b62c8cf 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2709,7 +2709,8 @@ static int arm_smmu_reassign_dev(struct domain *s,
struct domain *t,
return ret;
if (t) {
- ret = arm_smmu_assign_dev(t, devfn, dev);
+ /* The flag field doesn't matter to DT device. */
+ ret = arm_smmu_assign_dev(t, devfn, dev, 0);
if (ret)
return ret;
}
Thanks
Tiejun
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy
Can you avoid the mention of DT in the comment please, since PCI will eventually go that path. Something like No flags are defined for ARM would suffice I think. Works for me. But in some way 0 also represents one valid flag. So what about this ? No flags are defined for ARM so its always safe to set 0. Thanks Tiejun ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy
Chen, Tiejun writes (Re: [PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy): Tiejun, please can you send a patch to fix this up. Please send just a revised version of this patch. I think the rest of the series will rebase just fine on top of it. (If I'm wrong then we will need to do something more complex.) Sorry to this. On ARM side the flag field doesn't take any affect. Signed-off-by: Tiejun Chen tiejun.c...@intel.com OK, thanks. I wasn't sure that just passing 0 was right. I will take Jan's and Ian's suggestions as acks for your patch, which looks correct to me to implement them. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy
Ian Campbell writes (Re: [PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy): Can you avoid the mention of DT in the comment please, since PCI will eventually go that path. Something like No flags are defined for ARM would suffice I think. Right. I have substituted that for the commment. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy
From: Tiejun Chen tiejun.c...@intel.com
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
Reviewed-by: George Dunlap george.dun...@eu.citrix.com
Acked-by: Jan Beulich jbeul...@suse.com
---
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 1e763dc..89616b7 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..c7bbf6e 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;
}
-static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
+static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
{
struct hvm_iommu *hd = domain_hvm_iommu(d);
struct pci_dev *pdev;
@@ -1371,7 +1371,7 @@ static int assign_device(struct domain *d, u16 seg, u8
bus, u8 devfn)
pdev-fault.count = 0;
-if ( (rc = hd-platform_ops-assign_device(d, devfn, pci_to_dev(pdev))) )
+if ( (rc = hd-platform_ops-assign_device(d, devfn,
