Re: [PATCH v2 3/7] iommu/vt-d: Expose ISA direct mapping region via iommu_get_resv_regions
Hi James, On 3/25/19 8:57 PM, James Sewart wrote: Theres an issue that if we choose to alloc a new resv_region with type IOMMU_RESV_DIRECT, we will need to refactor intel_iommu_put_resv_regions to free this entry type which means refactoring the rmrr regions in get_resv_regions. Should this work be in this patchset? Do you mean the rmrr regions are not allocated in get_resv_regions, but are freed in put_resv_regions? I think we should fix this in this patch set since this might impact the device passthrough if we don't do it. They’re not allocated and not freed currently, only type IOMMU_RESV_MSI is freed in put_resv_regions. If we allocate a new resv_region with type IOMMU_RESV_DIRECT for the isa region, then it won’t be freed. If we modify put_resv_regions to free type IOMMU_RESV_DIRECT, then we will try to free the static RMRR regions. Either the ISA region is static and not freed as with my implementation, or the RMRR regions are converted to be allocated on each call to get_resv_regions and freed in put_resv_regions. By the way, there's another way in my mind. Let's add a new region type for LPC devices, e.x. IOMMU_RESV_LPC, and then handle it in the same way as those MSI regions. Just FYI. Best regards, Lu Baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/7] iommu/vt-d: Expose ISA direct mapping region via iommu_get_resv_regions
Hi,
On 3/25/19 8:57 PM, James Sewart wrote:
Hey Lu,
On 25 Mar 2019, at 02:03, Lu Baolu wrote:
Hi James,
On 3/22/19 5:57 PM, James Sewart wrote:
Hey Lu,
On 15 Mar 2019, at 02:19, Lu Baolu wrote:
Hi James,
On 3/14/19 7:58 PM, James Sewart wrote:
To support mapping ISA region via iommu_group_create_direct_mappings,
make sure its exposed by iommu_get_resv_regions. This allows
deduplication of reserved region mappings
Signed-off-by: James Sewart
---
drivers/iommu/intel-iommu.c | 42 +
1 file changed, 33 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 8e0a4e2ff77f..2e00e8708f06 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -337,6 +337,8 @@ static LIST_HEAD(dmar_rmrr_units);
#define for_each_rmrr_units(rmrr) \
list_for_each_entry(rmrr, _rmrr_units, list)
+static struct iommu_resv_region *isa_resv_region;
+
/* bitmap for indexing intel_iommus */
static int g_num_of_iommus;
@@ -2780,26 +2782,34 @@ static inline int iommu_prepare_rmrr_dev(struct
dmar_rmrr_unit *rmrr,
rmrr->end_address);
}
+static inline struct iommu_resv_region *iommu_get_isa_resv_region(void)
+{
+ if (!isa_resv_region)
+ isa_resv_region = iommu_alloc_resv_region(0,
+ 16*1024*1024,
+ 0, IOMMU_RESV_DIRECT);
+
+ return isa_resv_region;
+}
+
#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA
-static inline void iommu_prepare_isa(void)
+static inline void iommu_prepare_isa(struct pci_dev *pdev)
{
- struct pci_dev *pdev;
int ret;
+ struct iommu_resv_region *reg = iommu_get_isa_resv_region();
- pdev = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, NULL);
- if (!pdev)
+ if (!reg)
return;
pr_info("Prepare 0-16MiB unity mapping for LPC\n");
- ret = iommu_prepare_identity_map(>dev, 0, 16*1024*1024 - 1);
+ ret = iommu_prepare_identity_map(>dev, reg->start,
+ reg->start + reg->length - 1);
if (ret)
pr_err("Failed to create 0-16MiB identity map - floppy might not
work\n");
-
- pci_dev_put(pdev);
}
#else
-static inline void iommu_prepare_isa(void)
+static inline void iommu_prepare_isa(struct pci_dev *pdev)
{
return;
}
@@ -3289,6 +3299,7 @@ static int __init init_dmars(void)
struct dmar_rmrr_unit *rmrr;
bool copied_tables = false;
struct device *dev;
+ struct pci_dev *pdev;
struct intel_iommu *iommu;
int i, ret;
@@ -3469,7 +3480,11 @@ static int __init init_dmars(void)
}
}
- iommu_prepare_isa();
+ pdev = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, NULL);
+ if (pdev) {
+ iommu_prepare_isa(pdev);
+ pci_dev_put(pdev);
+ }
domains_done:
@@ -5266,6 +5281,7 @@ static void intel_iommu_get_resv_regions(struct device
*device,
struct iommu_resv_region *reg;
struct dmar_rmrr_unit *rmrr;
struct device *i_dev;
+ struct pci_dev *pdev;
int i;
rcu_read_lock();
@@ -5280,6 +5296,14 @@ static void intel_iommu_get_resv_regions(struct device
*device,
}
rcu_read_unlock();
+ if (dev_is_pci(device)) {
+ pdev = to_pci_dev(device);
+ if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) {
+ reg = iommu_get_isa_resv_region();
+ list_add_tail(>list, head);
+ }
+ }
+
Just wondering why not just
+#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA
+ if (dev_is_pci(device)) {
+ pdev = to_pci_dev(device);
+ if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) {
+ reg = iommu_alloc_resv_region(0,
+ 16*1024*1024,
+ 0, IOMMU_RESV_DIRECT);
+ if (reg)
+ list_add_tail(>list, head);
+ }
+ }
+#endif
and, remove all other related code?
At this point in the patchset if we remove iommu_prepare_isa then the ISA
region won’t be mapped to the device. Only once the dma domain is allocable
will the reserved regions be mapped by iommu_group_create_direct_mappings.
Yes. So if we put the allocation code here, it won't impact anything and
will take effect as soon as the dma domain is allocatable.
Theres an issue that if we choose to alloc a new resv_region with type
IOMMU_RESV_DIRECT, we will need to refactor intel_iommu_put_resv_regions
to free this entry type which means refactoring the rmrr regions in
get_resv_regions. Should this work be in this patchset?
Do you mean the rmrr regions are not allocated in get_resv_regions, but
are freed in put_resv_regions? I think we should fix this in this patch
set since this might impact the device
Re: [PATCH v2 4/4] iommu/arm-smmu: Add support to handle Qcom's TLBI serialization errata
On Sun 09 Sep 23:25 PDT 2018, Vivek Gautam wrote:
> Qcom's implementation of arm,mmu-500 require to serialize all
> TLB invalidations for context banks.
> In case the TLB invalidation requests don't go through the first
> time, there's a way to disable/enable the wait for safe logic.
> Disabling this logic expadites the TLBIs.
>
Reading this I get a feeling that this will give a slight performance
boost, but after finally narrowing the last weeks performance hunt down
I've concluded that without this patch we loose 1000x throughput on UFS
and USB (even I2C is completely useless) when the display is on.
So this is definitely needed for SDM845 devices, such as the Dragonboard
845c and MTP, to be functional.
> Different bootloaders with their access control policies allow this
> register access differntly. With one, we should be able to directly
> make qcom-scm call to do io read/write, while with other we should
> use the specific SCM command to send request to do the complete
> register configuration.
> A separate device tree flag for arm-smmu will allow to identify
> which firmware configuration of the two mentioned above we use.
>
> Signed-off-by: Vivek Gautam
Tested-by: Bjorn Andersson
Regards,
Bjorn
> ---
> drivers/iommu/arm-smmu-regs.h | 2 +
> drivers/iommu/arm-smmu.c | 133
> +-
> 2 files changed, 133 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-regs.h b/drivers/iommu/arm-smmu-regs.h
> index a1226e4ab5f8..71662cae9806 100644
> --- a/drivers/iommu/arm-smmu-regs.h
> +++ b/drivers/iommu/arm-smmu-regs.h
> @@ -177,6 +177,8 @@ enum arm_smmu_s2cr_privcfg {
> #define ARM_SMMU_CB_ATS1PR 0x800
> #define ARM_SMMU_CB_ATSR 0x8f0
>
> +#define ARM_SMMU_GID_QCOM_CUSTOM_CFG 0x300
> +
> #define SCTLR_S1_ASIDPNE (1 << 12)
> #define SCTLR_CFCFG (1 << 7)
> #define SCTLR_CFIE (1 << 6)
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 411e5ac57c64..de9c4a5bf686 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -49,6 +49,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -181,7 +182,8 @@ struct arm_smmu_device {
> #define ARM_SMMU_FEAT_EXIDS (1 << 12)
> u32 features;
>
> -#define ARM_SMMU_OPT_SECURE_CFG_ACCESS (1 << 0)
> +#define ARM_SMMU_OPT_SECURE_CFG_ACCESS(1 << 0)
> +#define ARM_SMMU_OPT_QCOM_FW_IMPL_ERRATA (1 << 1)
> u32 options;
> enum arm_smmu_arch_version version;
> enum arm_smmu_implementationmodel;
> @@ -266,6 +268,7 @@ static bool using_legacy_binding, using_generic_binding;
>
> static struct arm_smmu_option_prop arm_smmu_options[] = {
> { ARM_SMMU_OPT_SECURE_CFG_ACCESS, "calxeda,smmu-secure-config-access" },
> + { ARM_SMMU_OPT_QCOM_FW_IMPL_ERRATA, "qcom,smmu-500-fw-impl-errata" },
> { 0, NULL},
> };
>
> @@ -531,12 +534,134 @@ static void arm_smmu_tlb_inv_vmid_nosync(unsigned long
> iova, size_t size,
> writel_relaxed(smmu_domain->cfg.vmid, base + ARM_SMMU_GR0_TLBIVMID);
> }
>
> +#define CUSTOM_CFG_MDP_SAFE_ENABLE BIT(15)
> +#define CUSTOM_CFG_IFE1_SAFE_ENABLE BIT(14)
> +#define CUSTOM_CFG_IFE0_SAFE_ENABLE BIT(13)
> +
> +static int __qsmmu500_wait_safe_toggle(struct arm_smmu_device *smmu, int en)
> +{
> + int ret;
> + u32 val, gid_phys_base;
> + phys_addr_t reg;
> + struct vm_struct *vm;
> +
> + /* We want physical address of SMMU, so the vm_area */
> + vm = find_vm_area(smmu->base);
> +
> + /*
> + * GID (implementation defined address space) is located at
> + * SMMU_BASE + (2 × PAGESIZE).
> + */
> + gid_phys_base = vm->phys_addr + (2 << (smmu)->pgshift);
> + reg = gid_phys_base + ARM_SMMU_GID_QCOM_CUSTOM_CFG;
> +
> + ret = qcom_scm_io_readl_atomic(reg, );
> + if (ret)
> + return ret;
> +
> + if (en)
> + val |= CUSTOM_CFG_MDP_SAFE_ENABLE |
> +CUSTOM_CFG_IFE0_SAFE_ENABLE |
> +CUSTOM_CFG_IFE1_SAFE_ENABLE;
> + else
> + val &= ~(CUSTOM_CFG_MDP_SAFE_ENABLE |
> + CUSTOM_CFG_IFE0_SAFE_ENABLE |
> + CUSTOM_CFG_IFE1_SAFE_ENABLE);
> +
> + ret = qcom_scm_io_writel_atomic(reg, val);
> +
> + return ret;
> +}
> +
> +static int qsmmu500_wait_safe_toggle(struct arm_smmu_device *smmu,
> + int en, bool is_fw_impl)
> +{
> + if (is_fw_impl)
> + return qcom_scm_qsmmu500_wait_safe_toggle(en);
> + else
> + return __qsmmu500_wait_safe_toggle(smmu, en);
> +}
> +
> +static void qcom_errata_tlb_sync(struct arm_smmu_domain *smmu_domain)
> +{
> + struct arm_smmu_device *smmu = smmu_domain->smmu;
> + void __iomem *base = ARM_SMMU_CB(smmu, smmu_domain->cfg.cbndx);
>
Re: [PATCH v2 3/4] firmware/qcom_scm: Add scm call to handle smmu errata
On Sun 09 Sep 23:25 PDT 2018, Vivek Gautam wrote:
> Qcom's smmu-500 needs to toggle wait-for-safe sequence to
> handle TLB invalidation sync's.
> Few firmwares allow doing that through SCM interface.
> Add API to toggle wait for safe from firmware through a
> SCM call.
>
> Signed-off-by: Vivek Gautam
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> ---
> drivers/firmware/qcom_scm-32.c | 5 +
> drivers/firmware/qcom_scm-64.c | 13 +
> drivers/firmware/qcom_scm.c| 6 ++
> drivers/firmware/qcom_scm.h| 5 +
> include/linux/qcom_scm.h | 2 ++
> 5 files changed, 31 insertions(+)
>
> diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
> index 7293e5efad69..2d301ad053f8 100644
> --- a/drivers/firmware/qcom_scm-32.c
> +++ b/drivers/firmware/qcom_scm-32.c
> @@ -639,3 +639,8 @@ int __qcom_scm_io_writel_atomic(struct device *dev,
> phys_addr_t addr,
> {
> return -ENODEV;
> }
> +
> +int __qcom_scm_qsmmu500_wait_safe_toggle(struct device *dev, bool enable)
> +{
> + return -ENODEV;
> +}
> diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
> index 6bf55403f6e3..f13bcabc5d78 100644
> --- a/drivers/firmware/qcom_scm-64.c
> +++ b/drivers/firmware/qcom_scm-64.c
> @@ -590,3 +590,16 @@ int __qcom_scm_io_writel_atomic(struct device *dev,
> phys_addr_t addr,
> return qcom_scm_call_atomic(dev, QCOM_SCM_SVC_IO, QCOM_SCM_IO_WRITE,
> , );
> }
> +
> +int __qcom_scm_qsmmu500_wait_safe_toggle(struct device *dev, bool en)
> +{
> + struct qcom_scm_desc desc = {0};
> + struct arm_smccc_res res;
> +
> + desc.args[0] = QCOM_SCM_CONFIG_ERRATA1_CLIENT_ALL;
> + desc.args[1] = en;
> + desc.arginfo = QCOM_SCM_ARGS(2);
> +
> + return qcom_scm_call_atomic(dev, QCOM_SCM_SVC_SMMU_PROGRAM,
> + QCOM_SCM_CONFIG_ERRATA1, , );
> +}
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index 36dab37f..5f15cc2e9f69 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -353,6 +353,12 @@ int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size,
> u32 spare)
> }
> EXPORT_SYMBOL(qcom_scm_iommu_secure_ptbl_init);
>
> +int qcom_scm_qsmmu500_wait_safe_toggle(bool en)
> +{
> + return __qcom_scm_qsmmu500_wait_safe_toggle(__scm->dev, en);
> +}
> +EXPORT_SYMBOL(qcom_scm_qsmmu500_wait_safe_toggle);
> +
> int qcom_scm_io_readl(phys_addr_t addr, unsigned int *val)
> {
> return __qcom_scm_io_readl(__scm->dev, addr, val);
> diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
> index bb176107f51e..89a822c23e33 100644
> --- a/drivers/firmware/qcom_scm.h
> +++ b/drivers/firmware/qcom_scm.h
> @@ -103,10 +103,15 @@ extern int __qcom_scm_restore_sec_cfg(struct device
> *dev, u32 device_id,
> u32 spare);
> #define QCOM_SCM_IOMMU_SECURE_PTBL_SIZE 3
> #define QCOM_SCM_IOMMU_SECURE_PTBL_INIT 4
> +#define QCOM_SCM_SVC_SMMU_PROGRAM0x15
> +#define QCOM_SCM_CONFIG_ERRATA1 0x3
> +#define QCOM_SCM_CONFIG_ERRATA1_CLIENT_ALL 0x2
> extern int __qcom_scm_iommu_secure_ptbl_size(struct device *dev, u32 spare,
>size_t *size);
> extern int __qcom_scm_iommu_secure_ptbl_init(struct device *dev, u64 addr,
>u32 size, u32 spare);
> +extern int __qcom_scm_qsmmu500_wait_safe_toggle(struct device *dev,
> + bool enable);
> #define QCOM_MEM_PROT_ASSIGN_ID 0x16
> extern int __qcom_scm_assign_mem(struct device *dev,
> phys_addr_t mem_region, size_t mem_sz,
> diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
> index 6a5d0c98b328..46e6b1692998 100644
> --- a/include/linux/qcom_scm.h
> +++ b/include/linux/qcom_scm.h
> @@ -62,6 +62,7 @@ extern int qcom_scm_set_remote_state(u32 state, u32 id);
> extern int qcom_scm_restore_sec_cfg(u32 device_id, u32 spare);
> extern int qcom_scm_iommu_secure_ptbl_size(u32 spare, size_t *size);
> extern int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32 spare);
> +extern int qcom_scm_qsmmu500_wait_safe_toggle(bool en);
> extern int qcom_scm_io_readl(phys_addr_t addr, unsigned int *val);
> extern int qcom_scm_io_writel(phys_addr_t addr, unsigned int val);
> extern int qcom_scm_io_readl_atomic(phys_addr_t addr, unsigned int *val);
> @@ -100,6 +101,7 @@ qcom_scm_set_remote_state(u32 state,u32 id) { return
> -ENODEV; }
> static inline int qcom_scm_restore_sec_cfg(u32 device_id, u32 spare) {
> return -ENODEV; }
> static inline int qcom_scm_iommu_secure_ptbl_size(u32 spare, size_t *size) {
> return -ENODEV; }
> static inline int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32
> spare) { return -ENODEV; }
> +static inline int qcom_scm_qsmmu500_wait_safe_toggle(bool en) { return
> -ENODEV; }
> static inline int
Re: [PATCH v2 2/4] firmware/qcom_scm: Add atomic version of io read/write APIs
On Sun 09 Sep 23:25 PDT 2018, Vivek Gautam wrote:
> Add atomic versions of qcom_scm_io_readl/writel to enable
> reading/writing secure registers from atomic context.
>
> Signed-off-by: Vivek Gautam
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> ---
> drivers/firmware/qcom_scm-32.c | 12
> drivers/firmware/qcom_scm-64.c | 32
> drivers/firmware/qcom_scm.c| 12
> drivers/firmware/qcom_scm.h| 4
> include/linux/qcom_scm.h | 4
> 5 files changed, 64 insertions(+)
>
> diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
> index 4e24e591ae74..7293e5efad69 100644
> --- a/drivers/firmware/qcom_scm-32.c
> +++ b/drivers/firmware/qcom_scm-32.c
> @@ -627,3 +627,15 @@ int __qcom_scm_io_writel(struct device *dev, phys_addr_t
> addr, unsigned int val)
> return qcom_scm_call_atomic2(QCOM_SCM_SVC_IO, QCOM_SCM_IO_WRITE,
>addr, val);
> }
> +
> +int __qcom_scm_io_readl_atomic(struct device *dev, phys_addr_t addr,
> +unsigned int *val)
> +{
> + return -ENODEV;
> +}
> +
> +int __qcom_scm_io_writel_atomic(struct device *dev, phys_addr_t addr,
> + unsigned int val)
> +{
> + return -ENODEV;
> +}
> diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
> index 3a8c867cdf51..6bf55403f6e3 100644
> --- a/drivers/firmware/qcom_scm-64.c
> +++ b/drivers/firmware/qcom_scm-64.c
> @@ -558,3 +558,35 @@ int __qcom_scm_io_writel(struct device *dev, phys_addr_t
> addr, unsigned int val)
> return qcom_scm_call(dev, QCOM_SCM_SVC_IO, QCOM_SCM_IO_WRITE,
>, );
> }
> +
> +int __qcom_scm_io_readl_atomic(struct device *dev, phys_addr_t addr,
> +unsigned int *val)
> +{
> + struct qcom_scm_desc desc = {0};
> + struct arm_smccc_res res;
> + int ret;
> +
> + desc.args[0] = addr;
> + desc.arginfo = QCOM_SCM_ARGS(1);
> +
> + ret = qcom_scm_call_atomic(dev, QCOM_SCM_SVC_IO, QCOM_SCM_IO_READ,
> +, );
> + if (ret >= 0)
> + *val = res.a1;
> +
> + return ret < 0 ? ret : 0;
> +}
> +
> +int __qcom_scm_io_writel_atomic(struct device *dev, phys_addr_t addr,
> + unsigned int val)
> +{
> + struct qcom_scm_desc desc = {0};
> + struct arm_smccc_res res;
> +
> + desc.args[0] = addr;
> + desc.args[1] = val;
> + desc.arginfo = QCOM_SCM_ARGS(2);
> +
> + return qcom_scm_call_atomic(dev, QCOM_SCM_SVC_IO, QCOM_SCM_IO_WRITE,
> + , );
> +}
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index e778af766fae..36dab37f 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -365,6 +365,18 @@ int qcom_scm_io_writel(phys_addr_t addr, unsigned int
> val)
> }
> EXPORT_SYMBOL(qcom_scm_io_writel);
>
> +int qcom_scm_io_readl_atomic(phys_addr_t addr, unsigned int *val)
> +{
> + return __qcom_scm_io_readl_atomic(__scm->dev, addr, val);
> +}
> +EXPORT_SYMBOL(qcom_scm_io_readl_atomic);
> +
> +int qcom_scm_io_writel_atomic(phys_addr_t addr, unsigned int val)
> +{
> + return __qcom_scm_io_writel_atomic(__scm->dev, addr, val);
> +}
> +EXPORT_SYMBOL(qcom_scm_io_writel_atomic);
> +
> static void qcom_scm_set_download_mode(bool enable)
> {
> bool avail;
> diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
> index dcd7f7917fc7..bb176107f51e 100644
> --- a/drivers/firmware/qcom_scm.h
> +++ b/drivers/firmware/qcom_scm.h
> @@ -37,6 +37,10 @@ extern void __qcom_scm_cpu_power_down(u32 flags);
> #define QCOM_SCM_IO_WRITE0x2
> extern int __qcom_scm_io_readl(struct device *dev, phys_addr_t addr,
> unsigned int *val);
> extern int __qcom_scm_io_writel(struct device *dev, phys_addr_t addr,
> unsigned int val);
> +extern int __qcom_scm_io_readl_atomic(struct device *dev, phys_addr_t addr,
> + unsigned int *val);
> +extern int __qcom_scm_io_writel_atomic(struct device *dev, phys_addr_t addr,
> +unsigned int val);
>
> #define QCOM_SCM_SVC_INFO0x6
> #define QCOM_IS_CALL_AVAIL_CMD 0x1
> diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
> index 5d65521260b3..6a5d0c98b328 100644
> --- a/include/linux/qcom_scm.h
> +++ b/include/linux/qcom_scm.h
> @@ -64,6 +64,8 @@ extern int qcom_scm_iommu_secure_ptbl_size(u32 spare,
> size_t *size);
> extern int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32 spare);
> extern int qcom_scm_io_readl(phys_addr_t addr, unsigned int *val);
> extern int qcom_scm_io_writel(phys_addr_t addr, unsigned int val);
> +extern int qcom_scm_io_readl_atomic(phys_addr_t addr, unsigned int *val);
> +extern int qcom_scm_io_writel_atomic(phys_addr_t addr, unsigned int val);
> #else
> static inline
>
Re: [PATCH v2 1/4] firmware: qcom_scm-64: Add atomic version of qcom_scm_call
On Sun 09 Sep 23:25 PDT 2018, Vivek Gautam wrote:
> There are scnenarios where drivers are required to make a
> scm call in atomic context, such as in one of the qcom's
> arm-smmu-500 errata [1].
>
> [1] ("https://source.codeaurora.org/quic/la/kernel/msm-4.9/
> tree/drivers/iommu/arm-smmu.c?h=msm-4.9#n4842")
>
> Signed-off-by: Vivek Gautam
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> ---
> drivers/firmware/qcom_scm-64.c | 136
> -
> 1 file changed, 92 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
> index 688525dd4aee..3a8c867cdf51 100644
> --- a/drivers/firmware/qcom_scm-64.c
> +++ b/drivers/firmware/qcom_scm-64.c
> @@ -70,32 +70,71 @@ static DEFINE_MUTEX(qcom_scm_lock);
> #define FIRST_EXT_ARG_IDX 3
> #define N_REGISTER_ARGS (MAX_QCOM_SCM_ARGS - N_EXT_QCOM_SCM_ARGS + 1)
>
> -/**
> - * qcom_scm_call() - Invoke a syscall in the secure world
> - * @dev: device
> - * @svc_id: service identifier
> - * @cmd_id: command identifier
> - * @desc:Descriptor structure containing arguments and return values
> - *
> - * Sends a command to the SCM and waits for the command to finish processing.
> - * This should *only* be called in pre-emptible context.
> -*/
> -static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
> - const struct qcom_scm_desc *desc,
> - struct arm_smccc_res *res)
> +static void __qcom_scm_call_do(const struct qcom_scm_desc *desc,
> +struct arm_smccc_res *res, u32 fn_id,
> +u64 x5, u32 type)
> +{
> + u64 cmd;
> + struct arm_smccc_quirk quirk = {.id = ARM_SMCCC_QUIRK_QCOM_A6};
> +
> + cmd = ARM_SMCCC_CALL_VAL(type, qcom_smccc_convention,
> + ARM_SMCCC_OWNER_SIP, fn_id);
> +
> + quirk.state.a6 = 0;
> +
> + do {
> + arm_smccc_smc_quirk(cmd, desc->arginfo, desc->args[0],
> + desc->args[1], desc->args[2], x5,
> + quirk.state.a6, 0, res, );
> +
> + if (res->a0 == QCOM_SCM_INTERRUPTED)
> + cmd = res->a0;
> +
> + } while (res->a0 == QCOM_SCM_INTERRUPTED);
> +}
> +
> +static void qcom_scm_call_do(const struct qcom_scm_desc *desc,
> + struct arm_smccc_res *res, u32 fn_id,
> + u64 x5, bool atomic)
> +{
> + int retry_count = 0;
> +
> + if (!atomic) {
> + do {
> + mutex_lock(_scm_lock);
> +
> + __qcom_scm_call_do(desc, res, fn_id, x5,
> +ARM_SMCCC_STD_CALL);
> +
> + mutex_unlock(_scm_lock);
> +
> + if (res->a0 == QCOM_SCM_V2_EBUSY) {
> + if (retry_count++ > QCOM_SCM_EBUSY_MAX_RETRY)
> + break;
> + msleep(QCOM_SCM_EBUSY_WAIT_MS);
> + }
> + } while (res->a0 == QCOM_SCM_V2_EBUSY);
> + } else {
> + __qcom_scm_call_do(desc, res, fn_id, x5, ARM_SMCCC_FAST_CALL);
> + }
> +}
> +
> +static int ___qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
> + const struct qcom_scm_desc *desc,
> + struct arm_smccc_res *res, bool atomic)
> {
> int arglen = desc->arginfo & 0xf;
> - int retry_count = 0, i;
> + int i;
> u32 fn_id = QCOM_SCM_FNID(svc_id, cmd_id);
> - u64 cmd, x5 = desc->args[FIRST_EXT_ARG_IDX];
> + u64 x5 = desc->args[FIRST_EXT_ARG_IDX];
> dma_addr_t args_phys = 0;
> void *args_virt = NULL;
> size_t alloc_len;
> - struct arm_smccc_quirk quirk = {.id = ARM_SMCCC_QUIRK_QCOM_A6};
> + gfp_t flag = atomic ? GFP_ATOMIC : GFP_KERNEL;
>
> if (unlikely(arglen > N_REGISTER_ARGS)) {
> alloc_len = N_EXT_QCOM_SCM_ARGS * sizeof(u64);
> - args_virt = kzalloc(PAGE_ALIGN(alloc_len), GFP_KERNEL);
> + args_virt = kzalloc(PAGE_ALIGN(alloc_len), flag);
>
> if (!args_virt)
> return -ENOMEM;
> @@ -125,33 +164,7 @@ static int qcom_scm_call(struct device *dev, u32 svc_id,
> u32 cmd_id,
> x5 = args_phys;
> }
>
> - do {
> - mutex_lock(_scm_lock);
> -
> - cmd = ARM_SMCCC_CALL_VAL(ARM_SMCCC_STD_CALL,
> - qcom_smccc_convention,
> - ARM_SMCCC_OWNER_SIP, fn_id);
> -
> - quirk.state.a6 = 0;
> -
> - do {
> - arm_smccc_smc_quirk(cmd, desc->arginfo, desc->args[0],
> - desc->args[1], desc->args[2], x5,
> - quirk.state.a6, 0, res, );
> -
> - if (res->a0 == QCOM_SCM_INTERRUPTED)
> -
Re: [PATCH v3 09/10] NTB: Add MSI interrupt support to ntb_transport
On 2019-03-22 10:47 a.m., Bjorn Helgaas wrote: > Hi Logan, > > Drive-by nits: Thanks Bjorn! I've updated my patches for v4 which I'll send out in a week or two. Logan ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 RFC/RFT] dma-contiguous: Get normal pages for single-page allocations
On Mon, Mar 25, 2019 at 12:14:37PM +, Catalin Marinas wrote:
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index fcdb23e8d2fc..8955ba6f52fc 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -111,8 +111,7 @@ struct page *__dma_direct_alloc_pages(struct device *dev,
> size_t size,
> again:
> /* CMA can be used only in the context which permits sleeping */
> if (gfpflags_allow_blocking(gfp)) {
> - page = dma_alloc_from_contiguous(dev, count, page_order,
> - gfp & __GFP_NOWARN);
> + page = dma_alloc_from_contiguous(dev, count, page_order, gfp);
Oh...yea...It should have this part. I messed up with some
thing else so couldn't get it. Thanks for the reply.
I will go for the previous change by returning NULL in the
dma_alloc_from_contiguous() so the no_warm parameter would
not be touched.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/4] ACPI/IORT: Check ATS capability in root complex nodes
On 21/03/2019 16:00, Sinan Kaya wrote: > On 3/20/2019 1:36 PM, Jean-Philippe Brucker wrote: >> err = pci_for_each_dma_alias(to_pci_dev(dev), >> iort_pci_iommu_init, ); >> + >> + if (!err && !iort_pci_rc_supports_ats(node)) >> + dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_NO_ATS; > > Is it possible to remove !err check here. ATS supported is just a flag > in IORT table. Does this decision have to rely on having a correct dma > alias? iort_pci_iommu_init() allocates and initializes dev->iommu_fwspec, so checking !err ensures that dev->iommu_fwspec->flags is valid. Thanks, Jean ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/4] iommu/arm-smmu-v3: Add support for PCI ATS
On 21/03/2019 15:52, Sinan Kaya wrote: > On 3/20/2019 1:36 PM, Jean-Philippe Brucker wrote: >> pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_ATS); >> + if (!pos) >> + return -ENOSYS; >> + > > You don't need this. pci_enable_ats() validates this via. > > if (!dev->ats_cap) > return -EINVAL; Right, I think I wanted to differentiate lack of ATS capability from actual error in pci_enable_ats(), in which case we'd issue a dev_err(). But the only other errors in pci_enable_ats() are related to invalid STU parameter, which are valid in our case, so the dev_err() is pretty useless. I'll remove this. Thanks, Jean ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/1] iommu: Bind process address spaces to devices
Hey Jean-Philippe,
thanks for the patch, I think we are on the finish line with this
interface. Just one small question below.
On Wed, Mar 20, 2019 at 03:02:58PM +, Jean-Philippe Brucker wrote:
> +int iommu_sva_set_ops(struct iommu_sva *handle,
> + const struct iommu_sva_ops *sva_ops)
> +{
> + const struct iommu_ops *ops = handle->dev->bus->iommu_ops;
> +
> + if (!ops || !ops->sva_set_ops)
> + return -ENODEV;
> +
> + return ops->sva_set_ops(handle, sva_ops);
> +}
What is the purpose of the sva_set_ops call-back in iommu-ops? Is the
IOMMU driver supposed to do some extra setup work with the provided ops?
Otherwise we can just store the pointer in 'struct iommu_sva' without
calling into the iommu driver.
Regards,
Joerg
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] svm/avic: iommu/amd: Flush IOMMU IRT after update all entries
On Wed, Mar 20, 2019 at 08:14:56AM +, Suthikulpanit, Suravee wrote: > When AVIC is enabled and the VM has discrete device assignment, > the interrupt remapping table (IRT) is used to keep track of which > destination APIC ID the IOMMU will inject the device interrput to. > > This means every time a vcpu is blocked or context-switched (i.e. > vcpu_blocking/unblocking() and vcpu_load/put()), the information > in IRT must be updated and the IOMMU IRT flush command must be > issued. > > The current implementation flushes IOMMU IRT every time an entry > is modified. If the assigned device has large number of interrupts > (hence large number of entries), this would add large amount of > overhead to vcpu context-switch. Instead, this can be optmized by > only flush IRT once per vcpu context-switch per device after all > IRT entries are modified. > > The function amd_iommu_update_ga() is refactored to only update > IRT entry, while the amd_iommu_sync_ga() is introduced to allow > IRT flushing to be done separately. > > Cc: Joerg Roedel > Cc: Radim Krčmář > Cc: Paolo Bonzini > Signed-off-by: Suravee Suthikulpanit > --- > arch/x86/kvm/svm.c| 35 ++- > drivers/iommu/amd_iommu.c | 20 +--- > include/linux/amd-iommu.h | 13 ++--- > 3 files changed, 61 insertions(+), 7 deletions(-) For the IOMMU parts: Acked-by: Joerg Roedel ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/1] iommu: Remove iommu_callback_data
On Wed, Mar 20, 2019 at 09:40:24AM +0800, Lu Baolu wrote: > The iommu_callback_data is not used anywhere, remove it to make > the code more concise. > > Signed-off-by: Lu Baolu > --- > drivers/iommu/iommu.c | 11 ++- > 1 file changed, 2 insertions(+), 9 deletions(-) Applied, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: Don't print warning when IOMMU driver only supports unmanaged domains
On Fri, Mar 22, 2019 at 05:04:26PM +0100, Joerg Roedel wrote:
> From: Joerg Roedel
>
> Print the warning about the fall-back to IOMMU_DOMAIN_DMA in
> iommu_group_get_for_dev() only when such a domain was
> actually allocated.
>
> Otherwise the user will get misleading warnings in the
> kernel log when the iommu driver used doesn't support
> IOMMU_DOMAIN_DMA and IOMMU_DOMAIN_IDENTITY.
>
> Fixes: fccb4e3b8ab09 ('iommu: Allow default domain type to be set on the
> kernel command line')
> Signed-off-by: Joerg Roedel
> ---
> drivers/iommu/iommu.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
Applied.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/7] iommu/vt-d: Expose ISA direct mapping region via iommu_get_resv_regions
Hey Lu,
> On 25 Mar 2019, at 02:03, Lu Baolu wrote:
>
> Hi James,
>
> On 3/22/19 5:57 PM, James Sewart wrote:
>> Hey Lu,
>>> On 15 Mar 2019, at 02:19, Lu Baolu wrote:
>>>
>>> Hi James,
>>>
>>> On 3/14/19 7:58 PM, James Sewart wrote:
To support mapping ISA region via iommu_group_create_direct_mappings,
make sure its exposed by iommu_get_resv_regions. This allows
deduplication of reserved region mappings
Signed-off-by: James Sewart
---
drivers/iommu/intel-iommu.c | 42 +
1 file changed, 33 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 8e0a4e2ff77f..2e00e8708f06 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -337,6 +337,8 @@ static LIST_HEAD(dmar_rmrr_units);
#define for_each_rmrr_units(rmrr) \
list_for_each_entry(rmrr, _rmrr_units, list)
+static struct iommu_resv_region *isa_resv_region;
+
/* bitmap for indexing intel_iommus */
static int g_num_of_iommus;
@@ -2780,26 +2782,34 @@ static inline int iommu_prepare_rmrr_dev(struct
dmar_rmrr_unit *rmrr,
rmrr->end_address);
}
+static inline struct iommu_resv_region *iommu_get_isa_resv_region(void)
+{
+ if (!isa_resv_region)
+ isa_resv_region = iommu_alloc_resv_region(0,
+ 16*1024*1024,
+ 0, IOMMU_RESV_DIRECT);
+
+ return isa_resv_region;
+}
+
#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA
-static inline void iommu_prepare_isa(void)
+static inline void iommu_prepare_isa(struct pci_dev *pdev)
{
- struct pci_dev *pdev;
int ret;
+ struct iommu_resv_region *reg = iommu_get_isa_resv_region();
- pdev = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, NULL);
- if (!pdev)
+ if (!reg)
return;
pr_info("Prepare 0-16MiB unity mapping for LPC\n");
- ret = iommu_prepare_identity_map(>dev, 0, 16*1024*1024 - 1);
+ ret = iommu_prepare_identity_map(>dev, reg->start,
+ reg->start + reg->length - 1);
if (ret)
pr_err("Failed to create 0-16MiB identity map - floppy might
not work\n");
-
- pci_dev_put(pdev);
}
#else
-static inline void iommu_prepare_isa(void)
+static inline void iommu_prepare_isa(struct pci_dev *pdev)
{
return;
}
@@ -3289,6 +3299,7 @@ static int __init init_dmars(void)
struct dmar_rmrr_unit *rmrr;
bool copied_tables = false;
struct device *dev;
+ struct pci_dev *pdev;
struct intel_iommu *iommu;
int i, ret;
@@ -3469,7 +3480,11 @@ static int __init init_dmars(void)
}
}
- iommu_prepare_isa();
+ pdev = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, NULL);
+ if (pdev) {
+ iommu_prepare_isa(pdev);
+ pci_dev_put(pdev);
+ }
domains_done:
@@ -5266,6 +5281,7 @@ static void intel_iommu_get_resv_regions(struct
device *device,
struct iommu_resv_region *reg;
struct dmar_rmrr_unit *rmrr;
struct device *i_dev;
+ struct pci_dev *pdev;
int i;
rcu_read_lock();
@@ -5280,6 +5296,14 @@ static void intel_iommu_get_resv_regions(struct
device *device,
}
rcu_read_unlock();
+ if (dev_is_pci(device)) {
+ pdev = to_pci_dev(device);
+ if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) {
+ reg = iommu_get_isa_resv_region();
+ list_add_tail(>list, head);
+ }
+ }
+
>>>
>>> Just wondering why not just
>>>
>>> +#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA
>>> + if (dev_is_pci(device)) {
>>> + pdev = to_pci_dev(device);
>>> + if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) {
>>> + reg = iommu_alloc_resv_region(0,
>>> + 16*1024*1024,
>>> + 0, IOMMU_RESV_DIRECT);
>>> + if (reg)
>>> + list_add_tail(>list, head);
>>> + }
>>> + }
>>> +#endif
>>>
>>> and, remove all other related code?
>> At this point in the patchset if we remove iommu_prepare_isa then the ISA
>> region won’t be mapped to the device. Only once the dma domain is allocable
>> will the reserved regions be mapped by iommu_group_create_direct_mappings.
>
> Yes. So if we put the allocation code here, it won't impact anything and
> will take effect as soon as the dma domain is allocatable.
>
>> Theres an issue that if we choose to alloc a new resv_region with type
>> IOMMU_RESV_DIRECT, we will need to refactor intel_iommu_put_resv_regions
>> to free this entry type which means refactoring the
Re: [PATCH v2 RFC/RFT] dma-contiguous: Get normal pages for single-page allocations
On Fri, Mar 22, 2019 at 01:09:26PM -0700, Nicolin Chen wrote:
> On Fri, Mar 22, 2019 at 10:57:13AM +, Catalin Marinas wrote:
> > > > Do you have any numbers to back this up? You don't seem to address
> > > > dma_direct_alloc() either but, as I said above, it's not trivial since
> > > > some platforms expect certain physical range for DMA allocations.
> > >
> > > What's the dma_direct_alloc() here about? Mind elaborating?
> >
> > I just did a grep for dma_alloc_from_contiguous() in the 5.1-rc1 kernel
> > and came up with __dma_direct_alloc_pages(). Should your patch cover
> > this as well?
>
> I don't get the meaning of "cover this" here. What missing part
> do you refer to?
What I meant, do you need this hunk as well in your patch?
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index fcdb23e8d2fc..8955ba6f52fc 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -111,8 +111,7 @@ struct page *__dma_direct_alloc_pages(struct device *dev,
size_t size,
again:
/* CMA can be used only in the context which permits sleeping */
if (gfpflags_allow_blocking(gfp)) {
- page = dma_alloc_from_contiguous(dev, count, page_order,
-gfp & __GFP_NOWARN);
+ page = dma_alloc_from_contiguous(dev, count, page_order, gfp);
if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
dma_release_from_contiguous(dev, page, count);
page = NULL;
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
