This patch enables P2P upstream forwarding in ACS capable PCIe switches.
It solves two potential problems in virtualization environment where a PCIe
device is assigned to a guest domain using a HW iommu such as VT-d:
1) Unintentional failure caused by guest physical address programmed
into the device's DMA that happens to match the memory address range
of other downstream ports in the same PCIe switch. This causes the PCI
transaction to go to the matching downstream port instead of go to the
root complex to get translated by VT-d as it should be.
2) Malicious guest software intentionally attacks another downstream
PCIe device by programming the DMA address into the assigned device
that matches memory address range of the downstream PCIe port.
We are in process of implementing device filtering software in KVM/XEN
management software to allow device assignment of PCIe devices behind a PCIe
switch only if it has ACS capability and with the P2P upstream forwarding bits
enabled. This patch is intended to work for both KVM and Xen environments.
Changes from initial to v1:
- removed #define ACS_ENABLE and dev_info() call
- changed ctrl value setting without using if-condition
- fixed ACS #defines in pci_regs.h
Changes from v2 to v3:
- change #define indention to 2 for PCI reg and 1 for bit
position
Changes from v3 to v4:
- enable ACS only if iommu is enabled or if kernel is running as xen dom0
- changed xen_domain_type from enum to #define to allow it to be used
in pci/probe.c and to avoid excessive changes to code structure.
Signed-off-by: Allen Kay <[email protected]>
Reviewed--by: Mathew Wilcox <[email protected]>
Reviewed--by: Chris Wright <[email protected]>
Signed-off-by: Allen Kay <[email protected]>
---
arch/x86/include/asm/xen/hypervisor.h | 10 +++-----
arch/x86/xen/enlighten.c | 2 +-
drivers/pci/dmar.c | 1 +
drivers/pci/pci.c | 35 +++++++++++++++++++++++++++++++++
drivers/pci/pci.h | 10 +++++++++
drivers/pci/probe.c | 11 ++++++++++
include/linux/dmar.h | 2 +-
include/linux/pci_regs.h | 13 ++++++++++++
8 files changed, 76 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/xen/hypervisor.h b/arch/x86/include/asm/xen/hypervisor.h
index d5b7e90..a1e65dd 100644
--- a/arch/x86/include/asm/xen/hypervisor.h
+++ b/arch/x86/include/asm/xen/hypervisor.h
@@ -37,14 +37,12 @@
extern struct shared_info *HYPERVISOR_shared_info;
extern struct start_info *xen_start_info;
-enum xen_domain_type {
- XEN_NATIVE, /* running on bare hardware */
- XEN_PV_DOMAIN, /* running in a PV domain */
- XEN_HVM_DOMAIN, /* running in a Xen hvm domain */
-};
+#define XEN_NATIVE 0 /* running on bare hardware */
+#define XEN_PV_DOMAIN 1 /* running in a PV domain */
+#define XEN_HVM_DOMAIN 2 /* running in a Xen hvm domain */
#ifdef CONFIG_XEN
-extern enum xen_domain_type xen_domain_type;
+extern int xen_domain_type;
#else
#define xen_domain_type XEN_NATIVE
#endif
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 3439616..b263e85 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -62,7 +62,7 @@ EXPORT_SYMBOL_GPL(hypercall_page);
DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
DEFINE_PER_CPU(struct vcpu_info, xen_vcpu_info);
-enum xen_domain_type xen_domain_type = XEN_NATIVE;
+int xen_domain_type = XEN_NATIVE;
EXPORT_SYMBOL_GPL(xen_domain_type);
struct start_info *xen_start_info;
diff --git a/drivers/pci/dmar.c b/drivers/pci/dmar.c
index 22b02c6..814faa3 100644
--- a/drivers/pci/dmar.c
+++ b/drivers/pci/dmar.c
@@ -35,6 +35,7 @@
#include <linux/interrupt.h>
#include <linux/tboot.h>
#include <linux/dmi.h>
+#include "pci.h"
#define PREFIX "DMAR: "
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 6edecff..ec61c76 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1533,6 +1533,41 @@ void pci_enable_ari(struct pci_dev *dev)
}
/**
+ * pci_enable_acs - enable ACS if hardware support it
+ * @dev: the PCI device
+ */
+void pci_enable_acs(struct pci_dev *dev)
+{
+ int pos;
+ u16 cap;
+ u16 ctrl;
+
+ if (!dev->is_pcie)
+ return;
+
+ pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ACS);
+ if (!pos)
+ return;
+
+ pci_read_config_word(dev, pos + PCI_ACS_CAP, &cap);
+ pci_read_config_word(dev, pos + PCI_ACS_CTRL, &ctrl);
+
+ /* Source Validation */
+ ctrl |= (cap & PCI_ACS_SV);
+
+ /* P2P Request Redirect */
+ ctrl |= (cap & PCI_ACS_RR);
+
+ /* P2P Completion Redirect */
+ ctrl |= (cap & PCI_ACS_CR);
+
+ /* Upstream Forwarding */
+ ctrl |= (cap & PCI_ACS_UF);
+
+ pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
+}
+
+/**
* pci_swizzle_interrupt_pin - swizzle INTx for device behind bridge
* @dev: the PCI device
* @pin: the INTx pin (1=INTA, 2=INTB, 3=INTD, 4=INTD)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index d92d195..5b3a8f9 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -311,4 +311,14 @@ static inline int pci_resource_alignment(struct pci_dev *dev,
return resource_alignment(res);
}
+extern void pci_enable_acs(struct pci_dev *dev);
+
+#ifdef CONFIG_DMAR
+extern int iommu_detected;
+#endif
+
+#ifdef CONFIG_XEN
+extern int xen_domain_type;
+#endif
+
#endif /* DRIVERS_PCI_H */
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8105e32..32703c7 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1014,6 +1014,17 @@ static void pci_init_capabilities(struct pci_dev *dev)
/* Single Root I/O Virtualization */
pci_iov_init(dev);
+
+ /* Enable ACS P2P upstream forwarding if HW iommu is detected */
+ if (iommu_detected)
+ pci_enable_acs(dev);
+
+#ifdef CONFIG_XEN
+ /* HW iommu is not visible in xen dom0 */
+ if (xen_domain_type)
+ pci_enable_acs(dev);
+#endif
+
}
void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index 69a6fba..d13c21c 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -196,7 +196,7 @@ extern irqreturn_t dmar_fault(int irq, void *dev_id);
extern int arch_setup_dmar_msi(unsigned int irq);
#ifdef CONFIG_DMAR
-extern int iommu_detected, no_iommu;
+extern int no_iommu;
extern struct list_head dmar_rmrr_units;
struct dmar_rmrr_unit {
struct list_head list; /* list of rmrr units */
diff --git a/include/linux/pci_regs.h b/include/linux/pci_regs.h
index dd0bed4..d798770 100644
--- a/include/linux/pci_regs.h
+++ b/include/linux/pci_regs.h
@@ -502,6 +502,7 @@
#define PCI_EXT_CAP_ID_VC 2
#define PCI_EXT_CAP_ID_DSN 3
#define PCI_EXT_CAP_ID_PWR 4
+#define PCI_EXT_CAP_ID_ACS 13
#define PCI_EXT_CAP_ID_ARI 14
#define PCI_EXT_CAP_ID_ATS 15
#define PCI_EXT_CAP_ID_SRIOV 16
@@ -662,4 +663,16 @@
#define PCI_SRIOV_VFM_MO 0x2 /* Active.MigrateOut */
#define PCI_SRIOV_VFM_AV 0x3 /* Active.Available */
+/* Access Control Service */
+#define PCI_ACS_CAP 0x04 /* ACS Capability Register */
+#define PCI_ACS_SV 0x01 /* Source Validation */
+#define PCI_ACS_TB 0x02 /* Translation Blocking */
+#define PCI_ACS_RR 0x04 /* P2P Request Redirect */
+#define PCI_ACS_CR 0x08 /* P2P Completion Redirect */
+#define PCI_ACS_UF 0x10 /* Upstream Forwarding */
+#define PCI_ACS_EC 0x20 /* P2P Egress Control */
+#define PCI_ACS_DT 0x40 /* Direct Translated P2P */
+#define PCI_ACS_CTRL 0x06 /* ACS Control Register */
+#define PCI_ACS_EGRESS_CTL_V 0x08 /* ACS Egress Control Vector */
+
#endif /* LINUX_PCI_REGS_H */
--
1.6.0.6
On 10/06/09 13:33, Allen Kay wrote:
> Changes from v3 to v4:
> - enable ACS only if iommu is enabled or if kernel is running as xen dom0
> - changed xen_domain_type from enum to #define to allow it to be used
> in pci/probe.c and to avoid excessive changes to code structure.
>
Why can't it be used as an enum?
> +++ b/drivers/pci/pci.h
> @@ -311,4 +311,14 @@ static inline int pci_resource_alignment(struct pci_dev *dev,
> return resource_alignment(res);
> }
>
> +extern void pci_enable_acs(struct pci_dev *dev);
> +
> +#ifdef CONFIG_DMAR
> +extern int iommu_detected;
> +#endif
> +
> +#ifdef CONFIG_XEN
> +extern int xen_domain_type;
> +#endif
>
That's pretty ugly; duplicate externs are pretty bad idea. Why not just
include the right header? If its because its asm-x86, I'm happy to move
the definitions to somewhere more common.
> +
> #endif /* DRIVERS_PCI_H */
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 8105e32..32703c7 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1014,6 +1014,17 @@ static void pci_init_capabilities(struct pci_dev *dev)
>
> /* Single Root I/O Virtualization */
> pci_iov_init(dev);
> +
> + /* Enable ACS P2P upstream forwarding if HW iommu is detected */
> + if (iommu_detected)
> + pci_enable_acs(dev);
> +
> +#ifdef CONFIG_XEN
> + /* HW iommu is not visible in xen dom0 */
> + if (xen_domain_type)
> + pci_enable_acs(dev);
> +#endif
>
The idea is that you're supposed to use xen_domain() to test for
Xenness; it expands to a constant 0 if Xen isn't configured, so there's
no need to use #ifdefs.
> +
> }
>
> void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> index 69a6fba..d13c21c 100644
> --- a/include/linux/dmar.h
> +++ b/include/linux/dmar.h
> @@ -196,7 +196,7 @@ extern irqreturn_t dmar_fault(int irq, void *dev_id);
> extern int arch_setup_dmar_msi(unsigned int irq);
>
> #ifdef CONFIG_DMAR
> -extern int iommu_detected, no_iommu;
> +extern int no_iommu;
> extern struct list_head dmar_rmrr_units;
> struct dmar_rmrr_unit {
> struct list_head list; /* list of rmrr units */
> diff --git a/include/linux/pci_regs.h b/include/linux/pci_regs.h
> index dd0bed4..d798770 100644
> --- a/include/linux/pci_regs.h
> +++ b/include/linux/pci_regs.h
> @@ -502,6 +502,7 @@
> #define PCI_EXT_CAP_ID_VC 2
> #define PCI_EXT_CAP_ID_DSN 3
> #define PCI_EXT_CAP_ID_PWR 4
> +#define PCI_EXT_CAP_ID_ACS 13
> #define PCI_EXT_CAP_ID_ARI 14
> #define PCI_EXT_CAP_ID_ATS 15
> #define PCI_EXT_CAP_ID_SRIOV 16
> @@ -662,4 +663,16 @@
> #define PCI_SRIOV_VFM_MO 0x2 /* Active.MigrateOut */
> #define PCI_SRIOV_VFM_AV 0x3 /* Active.Available */
>
> +/* Access Control Service */
> +#define PCI_ACS_CAP 0x04 /* ACS Capability Register */
> +#define PCI_ACS_SV 0x01 /* Source Validation */
> +#define PCI_ACS_TB 0x02 /* Translation Blocking */
> +#define PCI_ACS_RR 0x04 /* P2P Request Redirect */
> +#define PCI_ACS_CR 0x08 /* P2P Completion Redirect */
> +#define PCI_ACS_UF 0x10 /* Upstream Forwarding */
> +#define PCI_ACS_EC 0x20 /* P2P Egress Control */
> +#define PCI_ACS_DT 0x40 /* Direct Translated P2P */
> +#define PCI_ACS_CTRL 0x06 /* ACS Control Register */
> +#define PCI_ACS_EGRESS_CTL_V 0x08 /* ACS Egress Control Vector */
> +
> #endif /* LINUX_PCI_REGS_H */
>
NAK
J
> That's pretty ugly; duplicate externs are pretty bad idea. Why not just
> include the right header? If its because its asm-x86, I'm happy to move
> the definitions to somewhere more common.
The main reason is lack of access of xen header files in driver/pci directory. If we can move xen header files to include/asm-x86, then the ugliness will go away.
Specifically, I would like to access header file containing xen_domain_type and associated enum definition from driver/pci directory.
-----Original Message-----
From: Jeremy Fitzhardinge [mailto:[email protected]]
Sent: Tuesday, October 06, 2009 2:38 PM
To: Kay, Allen M
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH ACS v4 1/1] Enabling PCI ACS P2P upstream forwarding
On 10/06/09 13:33, Allen Kay wrote:
> Changes from v3 to v4:
> - enable ACS only if iommu is enabled or if kernel is running as xen dom0
> - changed xen_domain_type from enum to #define to allow it to be used
> in pci/probe.c and to avoid excessive changes to code structure.
>
Why can't it be used as an enum?
> +++ b/drivers/pci/pci.h
> @@ -311,4 +311,14 @@ static inline int pci_resource_alignment(struct pci_dev *dev,
> return resource_alignment(res);
> }
>
> +extern void pci_enable_acs(struct pci_dev *dev);
> +
> +#ifdef CONFIG_DMAR
> +extern int iommu_detected;
> +#endif
> +
> +#ifdef CONFIG_XEN
> +extern int xen_domain_type;
> +#endif
>
That's pretty ugly; duplicate externs are pretty bad idea. Why not just
include the right header? If its because its asm-x86, I'm happy to move
the definitions to somewhere more common.
> +
> #endif /* DRIVERS_PCI_H */
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 8105e32..32703c7 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1014,6 +1014,17 @@ static void pci_init_capabilities(struct pci_dev *dev)
>
> /* Single Root I/O Virtualization */
> pci_iov_init(dev);
> +
> + /* Enable ACS P2P upstream forwarding if HW iommu is detected */
> + if (iommu_detected)
> + pci_enable_acs(dev);
> +
> +#ifdef CONFIG_XEN
> + /* HW iommu is not visible in xen dom0 */
> + if (xen_domain_type)
> + pci_enable_acs(dev);
> +#endif
>
The idea is that you're supposed to use xen_domain() to test for
Xenness; it expands to a constant 0 if Xen isn't configured, so there's
no need to use #ifdefs.
> +
> }
>
> void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> index 69a6fba..d13c21c 100644
> --- a/include/linux/dmar.h
> +++ b/include/linux/dmar.h
> @@ -196,7 +196,7 @@ extern irqreturn_t dmar_fault(int irq, void *dev_id);
> extern int arch_setup_dmar_msi(unsigned int irq);
>
> #ifdef CONFIG_DMAR
> -extern int iommu_detected, no_iommu;
> +extern int no_iommu;
> extern struct list_head dmar_rmrr_units;
> struct dmar_rmrr_unit {
> struct list_head list; /* list of rmrr units */
> diff --git a/include/linux/pci_regs.h b/include/linux/pci_regs.h
> index dd0bed4..d798770 100644
> --- a/include/linux/pci_regs.h
> +++ b/include/linux/pci_regs.h
> @@ -502,6 +502,7 @@
> #define PCI_EXT_CAP_ID_VC 2
> #define PCI_EXT_CAP_ID_DSN 3
> #define PCI_EXT_CAP_ID_PWR 4
> +#define PCI_EXT_CAP_ID_ACS 13
> #define PCI_EXT_CAP_ID_ARI 14
> #define PCI_EXT_CAP_ID_ATS 15
> #define PCI_EXT_CAP_ID_SRIOV 16
> @@ -662,4 +663,16 @@
> #define PCI_SRIOV_VFM_MO 0x2 /* Active.MigrateOut */
> #define PCI_SRIOV_VFM_AV 0x3 /* Active.Available */
>
> +/* Access Control Service */
> +#define PCI_ACS_CAP 0x04 /* ACS Capability Register */
> +#define PCI_ACS_SV 0x01 /* Source Validation */
> +#define PCI_ACS_TB 0x02 /* Translation Blocking */
> +#define PCI_ACS_RR 0x04 /* P2P Request Redirect */
> +#define PCI_ACS_CR 0x08 /* P2P Completion Redirect */
> +#define PCI_ACS_UF 0x10 /* Upstream Forwarding */
> +#define PCI_ACS_EC 0x20 /* P2P Egress Control */
> +#define PCI_ACS_DT 0x40 /* Direct Translated P2P */
> +#define PCI_ACS_CTRL 0x06 /* ACS Control Register */
> +#define PCI_ACS_EGRESS_CTL_V 0x08 /* ACS Egress Control Vector */
> +
> #endif /* LINUX_PCI_REGS_H */
>
NAK
J
Better move to somewhere arch independent as I'm calling this in architectural independent PCI driver code.
-----Original Message-----
From: Kay, Allen M
Sent: Tuesday, October 06, 2009 2:47 PM
To: 'Jeremy Fitzhardinge'
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: RE: [PATCH ACS v4 1/1] Enabling PCI ACS P2P upstream forwarding
> That's pretty ugly; duplicate externs are pretty bad idea. Why not just
> include the right header? If its because its asm-x86, I'm happy to move
> the definitions to somewhere more common.
The main reason is lack of access of xen header files in driver/pci directory. If we can move xen header files to include/asm-x86, then the ugliness will go away.
Specifically, I would like to access header file containing xen_domain_type and associated enum definition from driver/pci directory.
-----Original Message-----
From: Jeremy Fitzhardinge [mailto:[email protected]]
Sent: Tuesday, October 06, 2009 2:38 PM
To: Kay, Allen M
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH ACS v4 1/1] Enabling PCI ACS P2P upstream forwarding
On 10/06/09 13:33, Allen Kay wrote:
> Changes from v3 to v4:
> - enable ACS only if iommu is enabled or if kernel is running as xen dom0
> - changed xen_domain_type from enum to #define to allow it to be used
> in pci/probe.c and to avoid excessive changes to code structure.
>
Why can't it be used as an enum?
> +++ b/drivers/pci/pci.h
> @@ -311,4 +311,14 @@ static inline int pci_resource_alignment(struct pci_dev *dev,
> return resource_alignment(res);
> }
>
> +extern void pci_enable_acs(struct pci_dev *dev);
> +
> +#ifdef CONFIG_DMAR
> +extern int iommu_detected;
> +#endif
> +
> +#ifdef CONFIG_XEN
> +extern int xen_domain_type;
> +#endif
>
That's pretty ugly; duplicate externs are pretty bad idea. Why not just
include the right header? If its because its asm-x86, I'm happy to move
the definitions to somewhere more common.
> +
> #endif /* DRIVERS_PCI_H */
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 8105e32..32703c7 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1014,6 +1014,17 @@ static void pci_init_capabilities(struct pci_dev *dev)
>
> /* Single Root I/O Virtualization */
> pci_iov_init(dev);
> +
> + /* Enable ACS P2P upstream forwarding if HW iommu is detected */
> + if (iommu_detected)
> + pci_enable_acs(dev);
> +
> +#ifdef CONFIG_XEN
> + /* HW iommu is not visible in xen dom0 */
> + if (xen_domain_type)
> + pci_enable_acs(dev);
> +#endif
>
The idea is that you're supposed to use xen_domain() to test for
Xenness; it expands to a constant 0 if Xen isn't configured, so there's
no need to use #ifdefs.
> +
> }
>
> void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> index 69a6fba..d13c21c 100644
> --- a/include/linux/dmar.h
> +++ b/include/linux/dmar.h
> @@ -196,7 +196,7 @@ extern irqreturn_t dmar_fault(int irq, void *dev_id);
> extern int arch_setup_dmar_msi(unsigned int irq);
>
> #ifdef CONFIG_DMAR
> -extern int iommu_detected, no_iommu;
> +extern int no_iommu;
> extern struct list_head dmar_rmrr_units;
> struct dmar_rmrr_unit {
> struct list_head list; /* list of rmrr units */
> diff --git a/include/linux/pci_regs.h b/include/linux/pci_regs.h
> index dd0bed4..d798770 100644
> --- a/include/linux/pci_regs.h
> +++ b/include/linux/pci_regs.h
> @@ -502,6 +502,7 @@
> #define PCI_EXT_CAP_ID_VC 2
> #define PCI_EXT_CAP_ID_DSN 3
> #define PCI_EXT_CAP_ID_PWR 4
> +#define PCI_EXT_CAP_ID_ACS 13
> #define PCI_EXT_CAP_ID_ARI 14
> #define PCI_EXT_CAP_ID_ATS 15
> #define PCI_EXT_CAP_ID_SRIOV 16
> @@ -662,4 +663,16 @@
> #define PCI_SRIOV_VFM_MO 0x2 /* Active.MigrateOut */
> #define PCI_SRIOV_VFM_AV 0x3 /* Active.Available */
>
> +/* Access Control Service */
> +#define PCI_ACS_CAP 0x04 /* ACS Capability Register */
> +#define PCI_ACS_SV 0x01 /* Source Validation */
> +#define PCI_ACS_TB 0x02 /* Translation Blocking */
> +#define PCI_ACS_RR 0x04 /* P2P Request Redirect */
> +#define PCI_ACS_CR 0x08 /* P2P Completion Redirect */
> +#define PCI_ACS_UF 0x10 /* Upstream Forwarding */
> +#define PCI_ACS_EC 0x20 /* P2P Egress Control */
> +#define PCI_ACS_DT 0x40 /* Direct Translated P2P */
> +#define PCI_ACS_CTRL 0x06 /* ACS Control Register */
> +#define PCI_ACS_EGRESS_CTL_V 0x08 /* ACS Egress Control Vector */
> +
> #endif /* LINUX_PCI_REGS_H */
>
NAK
J
On 10/06/09 15:01, Kay, Allen M wrote:
> Better move to somewhere arch independent as I'm calling this in architectural independent PCI driver code.
>
I'm prepping a patch for you now, to move it to xen/xen.h.
J
Move xen_domain and related tests out of asm-x86 to xen/xen.h so they
can be included whenever they are necessary.
Signed-off-by: Jeremy Fitzhardinge <[email protected]>
--
arch/x86/include/asm/xen/hypervisor.h | 27 ---------------------------
arch/x86/xen/enlighten.c | 1 +
drivers/block/xen-blkfront.c | 1 +
drivers/char/hvc_xen.c | 2 ++
drivers/input/xen-kbdfront.c | 3 +++
drivers/net/xen-netfront.c | 1 +
drivers/video/xen-fbfront.c | 3 +++
drivers/xen/balloon.c | 2 ++
drivers/xen/cpu_hotplug.c | 1 +
drivers/xen/evtchn.c | 2 ++
drivers/xen/grant-table.c | 1 +
drivers/xen/sys-hypervisor.c | 1 +
drivers/xen/xenbus/xenbus_probe.c | 2 ++
drivers/xen/xenfs/super.c | 2 ++
include/xen/xen.h | 32 ++++++++++++++++++++++++++++++++
diff --git a/arch/x86/include/asm/xen/hypervisor.h b/arch/x86/include/asm/xen/hypervisor.h
index d5b7e90..396ff4c 100644
--- a/arch/x86/include/asm/xen/hypervisor.h
+++ b/arch/x86/include/asm/xen/hypervisor.h
@@ -37,31 +37,4 @@
extern struct shared_info *HYPERVISOR_shared_info;
extern struct start_info *xen_start_info;
-enum xen_domain_type {
- XEN_NATIVE, /* running on bare hardware */
- XEN_PV_DOMAIN, /* running in a PV domain */
- XEN_HVM_DOMAIN, /* running in a Xen hvm domain */
-};
-
-#ifdef CONFIG_XEN
-extern enum xen_domain_type xen_domain_type;
-#else
-#define xen_domain_type XEN_NATIVE
-#endif
-
-#define xen_domain() (xen_domain_type != XEN_NATIVE)
-#define xen_pv_domain() (xen_domain() && \
- xen_domain_type == XEN_PV_DOMAIN)
-#define xen_hvm_domain() (xen_domain() && \
- xen_domain_type == XEN_HVM_DOMAIN)
-
-#ifdef CONFIG_XEN_DOM0
-#include <xen/interface/xen.h>
-
-#define xen_initial_domain() (xen_pv_domain() && \
- xen_start_info->flags & SIF_INITDOMAIN)
-#else /* !CONFIG_XEN_DOM0 */
-#define xen_initial_domain() (0)
-#endif /* CONFIG_XEN_DOM0 */
-
#endif /* _ASM_X86_XEN_HYPERVISOR_H */
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index eb33aaa..f0db2b9 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -28,6 +28,7 @@
#include <linux/highmem.h>
#include <linux/console.h>
+#include <xen/xen.h>
#include <xen/interface/xen.h>
#include <xen/interface/version.h>
#include <xen/interface/physdev.h>
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index e532847..1e5baf7 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -42,6 +42,7 @@
#include <linux/module.h>
#include <linux/scatterlist.h>
+#include <xen/xen.h>
#include <xen/xenbus.h>
#include <xen/grant_table.h>
#include <xen/events.h>
diff --git a/drivers/char/hvc_xen.c b/drivers/char/hvc_xen.c
index eba999f..93d3381 100644
--- a/drivers/char/hvc_xen.c
+++ b/drivers/char/hvc_xen.c
@@ -25,6 +25,8 @@
#include <linux/types.h>
#include <asm/xen/hypervisor.h>
+
+#include <xen/xen.h>
#include <xen/page.h>
#include <xen/events.h>
#include <xen/interface/io/console.h>
diff --git a/drivers/input/xen-kbdfront.c b/drivers/input/xen-kbdfront.c
index b115726..c721c0a 100644
--- a/drivers/input/xen-kbdfront.c
+++ b/drivers/input/xen-kbdfront.c
@@ -21,7 +21,10 @@
#include <linux/errno.h>
#include <linux/module.h>
#include <linux/input.h>
+
#include <asm/xen/hypervisor.h>
+
+#include <xen/xen.h>
#include <xen/events.h>
#include <xen/page.h>
#include <xen/interface/io/fbif.h>
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 8d88dae..c2567ad 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -42,6 +42,7 @@
#include <linux/mm.h>
#include <net/ip.h>
+#include <xen/xen.h>
#include <xen/xenbus.h>
#include <xen/events.h>
#include <xen/page.h>
diff --git a/drivers/video/xen-fbfront.c b/drivers/video/xen-fbfront.c
index 54cd916..966b226 100644
--- a/drivers/video/xen-fbfront.c
+++ b/drivers/video/xen-fbfront.c
@@ -25,7 +25,10 @@
#include <linux/module.h>
#include <linux/vmalloc.h>
#include <linux/mm.h>
+
#include <asm/xen/hypervisor.h>
+
+#include <xen/xen.h>
#include <xen/events.h>
#include <xen/page.h>
#include <xen/interface/io/fbif.h>
diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index f5bbd9e..3a27e68 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -52,6 +52,8 @@
#include <asm/xen/hypervisor.h>
#include <asm/xen/hypercall.h>
+
+#include <xen/xen.h>
#include <xen/interface/xen.h>
#include <xen/interface/memory.h>
#include <xen/xenbus.h>
diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
index bdfd584..6625ffe 100644
--- a/drivers/xen/cpu_hotplug.c
+++ b/drivers/xen/cpu_hotplug.c
@@ -1,5 +1,6 @@
#include <linux/notifier.h>
+#include <xen/xen.h>
#include <xen/xenbus.h>
#include <asm/xen/hypervisor.h>
diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
index af03195..8356843 100644
--- a/drivers/xen/evtchn.c
+++ b/drivers/xen/evtchn.c
@@ -49,6 +49,8 @@
#include <linux/gfp.h>
#include <linux/mutex.h>
#include <linux/cpu.h>
+
+#include <xen/xen.h>
#include <xen/events.h>
#include <xen/evtchn.h>
#include <asm/xen/hypervisor.h>
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 7d8f531..4c6c0bd 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -37,6 +37,7 @@
#include <linux/vmalloc.h>
#include <linux/uaccess.h>
+#include <xen/xen.h>
#include <xen/interface/xen.h>
#include <xen/page.h>
#include <xen/grant_table.h>
diff --git a/drivers/xen/sys-hypervisor.c b/drivers/xen/sys-hypervisor.c
index 88a60e0..ae5cb05 100644
--- a/drivers/xen/sys-hypervisor.c
+++ b/drivers/xen/sys-hypervisor.c
@@ -14,6 +14,7 @@
#include <asm/xen/hypervisor.h>
#include <asm/xen/hypercall.h>
+#include <xen/xen.h>
#include <xen/xenbus.h>
#include <xen/interface/xen.h>
#include <xen/interface/version.h>
diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
index d42e25d..08638ad 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -49,6 +49,8 @@
#include <asm/page.h>
#include <asm/pgtable.h>
#include <asm/xen/hypervisor.h>
+
+#include <xen/xen.h>
#include <xen/xenbus.h>
#include <xen/events.h>
#include <xen/page.h>
diff --git a/drivers/xen/xenfs/super.c b/drivers/xen/xenfs/super.c
index 6559e0c..8924d93 100644
--- a/drivers/xen/xenfs/super.c
+++ b/drivers/xen/xenfs/super.c
@@ -13,6 +13,8 @@
#include <linux/fs.h>
#include <linux/magic.h>
+#include <xen/xen.h>
+
#include "xenfs.h"
#include <asm/xen/hypervisor.h>
diff --git a/include/xen/xen.h b/include/xen/xen.h
new file mode 100644
index 0000000..a164024
--- /dev/null
+++ b/include/xen/xen.h
@@ -0,0 +1,32 @@
+#ifndef _XEN_XEN_H
+#define _XEN_XEN_H
+
+enum xen_domain_type {
+ XEN_NATIVE, /* running on bare hardware */
+ XEN_PV_DOMAIN, /* running in a PV domain */
+ XEN_HVM_DOMAIN, /* running in a Xen hvm domain */
+};
+
+#ifdef CONFIG_XEN
+extern enum xen_domain_type xen_domain_type;
+#else
+#define xen_domain_type XEN_NATIVE
+#endif
+
+#define xen_domain() (xen_domain_type != XEN_NATIVE)
+#define xen_pv_domain() (xen_domain() && \
+ xen_domain_type == XEN_PV_DOMAIN)
+#define xen_hvm_domain() (xen_domain() && \
+ xen_domain_type == XEN_HVM_DOMAIN)
+
+#ifdef CONFIG_XEN_DOM0
+#include <xen/interface/xen.h>
+#include <asm/xen/hypervisor.h>
+
+#define xen_initial_domain() (xen_pv_domain() && \
+ xen_start_info->flags & SIF_INITDOMAIN)
+#else /* !CONFIG_XEN_DOM0 */
+#define xen_initial_domain() (0)
+#endif /* CONFIG_XEN_DOM0 */
+
+#endif /* _XEN_XEN_H */
Thanks! I will re-spin my patch.
-----Original Message-----
From: Jeremy Fitzhardinge [mailto:[email protected]]
Sent: Tuesday, October 06, 2009 3:05 PM
To: Kay, Allen M
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH ACS v4 1/1] Enabling PCI ACS P2P upstream forwarding
On 10/06/09 15:01, Kay, Allen M wrote:
> Better move to somewhere arch independent as I'm calling this in architectural independent PCI driver code.
>
I'm prepping a patch for you now, to move it to xen/xen.h.
J
* Allen Kay ([email protected]) wrote:
> +#ifdef CONFIG_DMAR
> +extern int iommu_detected;
> +#endif
This should not be needed.
> +
> +#ifdef CONFIG_XEN
> +extern int xen_domain_type;
> +#endif
Nor this (there's already a check for is dom0 called xen_initial_domain(),
but unclear it's relevant yet for this patch).
> +
> + /* Enable ACS P2P upstream forwarding if HW iommu is detected */
> + if (iommu_detected)
I think you'd want iommu_found() instead. To avoid, e.g., GART
triggering this one.
> + pci_enable_acs(dev);
> +
> +#ifdef CONFIG_XEN
> + /* HW iommu is not visible in xen dom0 */
> + if (xen_domain_type)
> + pci_enable_acs(dev);
could do this (xen_initial_domain()) above, but it's only relevant for
dom0 (so not needed yet?), and really seems to like it should be done by hv.
thanks,
-chris
* Chris Wright ([email protected]) wrote:
> * Allen Kay ([email protected]) wrote:
> > +#ifdef CONFIG_XEN
> > + /* HW iommu is not visible in xen dom0 */
> > + if (xen_domain_type)
> > + pci_enable_acs(dev);
>
> could do this (xen_initial_domain()) above, but it's only relevant for
> dom0 (so not needed yet?), and really seems to like it should be done by hv.
Guess it doesn't matter since dom0 could just as easily undo it, so
nevermind on the hv bit ;-)
Thanks Chris, I was not aware of these two functions. I have made the following change:
if (iommu_found() || xen_initial_domain())
pci_enable_acs(dev);
xen_intial_domain() should take care of dom0 case. We just make the rough assumption that P2P upstream forwarding is needed if we are running on top of xen HV - similar to what we are doing for native kernel case.
If this is ok, I'm going to re-spin the patch. Separate out xen_initial_domain() modification since it is predicated on Jeremy's xen.h patch.
-----Original Message-----
From: Chris Wright [mailto:[email protected]]
Sent: Tuesday, October 06, 2009 4:42 PM
To: Kay, Allen M
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH ACS v4 1/1] Enabling PCI ACS P2P upstream forwarding
* Allen Kay ([email protected]) wrote:
> +#ifdef CONFIG_DMAR
> +extern int iommu_detected;
> +#endif
This should not be needed.
> +
> +#ifdef CONFIG_XEN
> +extern int xen_domain_type;
> +#endif
Nor this (there's already a check for is dom0 called xen_initial_domain(),
but unclear it's relevant yet for this patch).
> +
> + /* Enable ACS P2P upstream forwarding if HW iommu is detected */
> + if (iommu_detected)
I think you'd want iommu_found() instead. To avoid, e.g., GART
triggering this one.
> + pci_enable_acs(dev);
> +
> +#ifdef CONFIG_XEN
> + /* HW iommu is not visible in xen dom0 */
> + if (xen_domain_type)
> + pci_enable_acs(dev);
could do this (xen_initial_domain()) above, but it's only relevant for
dom0 (so not needed yet?), and really seems to like it should be done by hv.
thanks,
-chris