Re: [RFC PATCH v4 11/28] x86: Add support to determine the E820 type of an address
On Tue, Feb 28, 2017 at 04:34:39PM -0600, Tom Lendacky wrote: > Or if we want to guard against ACPI adding a type 0 in the future, I > could make the function return an int and then return -EINVAL if an e820 > entry isn't found. This might be the better option. Yap, think so too. I don't trust specs anyway :) -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 22/30] iommu: Bind/unbind tasks to/from devices
On Mon, 2017-02-27 at 19:54 +, Jean-Philippe Brucker wrote: > Add three functions to the IOMMU API. iommu_bind_task takes a device and a > task as argument. If the IOMMU, the device and the bus support it, attach > task to device and create a Process Address Space ID (PASID) unique to the > device. DMA from the device can then use the PASID to read or write into > the address space. iommu_unbind_task removes a bond created with > iommu_bind_task. iommu_set_svm_ops allows a device driver to set some > callbacks for specific SVM-related operations. > > Try to accommodate current implementations (AMD, Intel and ARM), by > letting the IOMMU driver do all the work, but attempt by the same occasion > to find intersections between implementations. > > * amd_iommu_v2 expects the device to allocate a PASID and pass it to the > IOMMU. The driver also provides separate functions to register callbacks > that handles failed PRI requests and invalidate PASIDs. > > int amd_iommu_bind_pasid(struct pci_dev *pdev, int pasid, > struct task_struct *task) > void amd_iommu_unbind_pasid(struct pci_dev *pdev, int pasid) > int amd_iommu_set_invalid_ppr_cb(struct pci_dev *pdev, > amd_iommu_invalid_ppr_cb cb) > int amd_iommu_set_invalidate_ctx_cb(struct pci_dev *pdev, > amd_iommu_invalidate_ctx cb) > > * intel-svm allocates a PASID, and requires the driver to pass > "svm_dev_ops", which currently contains a fault callback. It also > doesn't take a task as argument, but uses 'current'. > > int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, > struct svm_dev_ops *ops) > int intel_svm_unbind_mm(struct device *dev, int pasid) > > * For arm-smmu-v3, PASID must be allocated by the SMMU driver since it > indexes contexts in an array handled by the SMMU device. Right. The Intel version was designed with all of the above three in mind. It was discussed at the Kernel Summit and LPC on more than one occasion as it took shape, and what I implemented for Intel basicall represents the consensus of what we thought it should look like. I meant to convert the AMD driver to the same API, but don't have access to test hardware. Note that the amdkfd code will need careful attention here. Intel slightly deviates from the "one PASID per process" vision too, because it currently has a PASID allocator idr per IOMMU. That wants making system-wide. And probably not Intel-specific. Some other comments... The callbacks and fault handlers could perhaps be deprecated. In an ideal world nobody would ever use them — the device itself is supposed to be able to communicate with its driver about the request that failed; we don't need a dirty hook into the IOMMU code from when *it* handles the fault. In the Intel IOMMU fault reports, there are some additional bits in the descriptor which are 'context private' bits. For built-in devices like the graphics engine, this contains further information about precisely which context was performing the failing access. But again I don't think we should need it in an ideal world. It's a horrid thing to have to feed through a generic IOMMU API. One thing which might help us *avoid* needing it is the SVM_FLAG_PRIVATE_PASID option, which asks for a *new* PASID. So a single process can have more than one PASID. That's still OK on ARM, isn't it? As long as they're all allocated from the same pool and we never use a given PASID for more than one address space simultaneously on different devices. We also have SVM_FLAG_SUPERVISOR_MODE, which gives access to kernel address space. Yes, people use it. > PASID invalidation > -- > > Next, we need to let the IOMMU driver notify the device driver before it > attempts to unbind a PASID. Subsequent patches discuss PASID invalidation > in more details, so we'll simply propose the following interface for now. > > AMD has: > > void (*amd_iommu_invalidate_ctx)(struct pci_dev *pdev, int pasid); > > We put the following in iommu_svm_ops: > > int (*invalidate_pasid)(struct device *dev, int pasid, void *priv); These can basically take for ever, right? You're asking the *device* to tell you when it's finished using that PASID. > Capability detection > > ... > > int iommu_svm_capable(struct device *dev, int flags); We already had this for Intel. It basically goes through *all* the enabling checks that it needs to for really setting up SVM, and that's why it's actually the *same* call, but with a NULL pasid argument: #define intel_svm_available(dev) (!intel_svm_bind_mm((dev), NULL, 0, NULL)) smime.p7s Description: S/MIME cryptographic signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/9] Docs: dt: document qcom iommu bindings
On Fri, Mar 3, 2017 at 1:21 AM, Rob Herring wrote:
> On Wed, Mar 01, 2017 at 12:42:52PM -0500, Rob Clark wrote:
>
> Nit: use "dt-bindings: iommu: ..." for subject. And a commit message
> would be nice.
>
>> Cc: devicet...@vger.kernel.org
>> Signed-off-by: Rob Clark
>> ---
>> .../devicetree/bindings/iommu/qcom,iommu.txt | 106
>> +
>> 1 file changed, 106 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/iommu/qcom,iommu.txt
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/qcom,iommu.txt
>> b/Documentation/devicetree/bindings/iommu/qcom,iommu.txt
>> new file mode 100644
>> index 000..2e69b78
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iommu/qcom,iommu.txt
>> @@ -0,0 +1,106 @@
>> +* QCOM IOMMU v1 Implementation
>> +
>> +Qualcomm "B" family devices which are not compatible with arm-smmu have
>> +a similar looking IOMMU but without access to the global register space,
>> +and optionally requiring additional configuration to route context irqs
>> +to non-secure vs secure interrupt line.
>> +
>> +** Required properties:
>> +
>> +- compatible : Should be "qcom,msm-iommu-v1".
>
> Fine as a fallback, but this needs chip specific compatibles.
ok, so maybe:
compatible = "qcom,msm8916-iommu", "qcom,msm-iommu-v1";
>> +- clocks : The interface clock (iface_clk) and bus clock
>> (bus_clk).
>
> The names need to be documented under clock-names prop.
>
> '_clk' is redundant.
ok
>> +- #address-cells : must be 1.
>> +- #size-cells : must be 1.
>> +- #iommu-cells : Must be 1.
>> +- ranges : Base address and size of the iommu context banks.
>> +- qcom,iommu-secure-id : secure-id.
>> +
>> +- List of sub-nodes, one per translation context bank. Each sub-node
>> + has the following required properties:
>> +
>> + - compatible : Should be one of:
>> +- "qcom,msm-iommu-v1-ns" : non-secure context bank
>> +- "qcom,msm-iommu-v1-sec" : secure context bank
>
> These are okay without chip specific strings.
>
>> + - reg: Base address and size of context bank within the iommu
>> + - interrupts : The context fault irq.
>> +
>> +** Optional properties:
>> +
>> +- reg : Base address and size of the SMMU local base, should
>> + be only specified if the iommu requires configuration
>> + for routing of context bank irq's to secure vs non-
>> + secure lines. (Ie. if the iommu contains secure
>> + context banks)
>> +
>> +
>> +** Examples:
>> +
>> + apps_iommu: msm-iommu-v1@1e2 {
>
> iommu@...
>
> And this should be the reg address, not the ranges address.
ok.. but I'm not entirely sure what to do w/ gpu_iommu, which doesn't
have a reg address.
I guess I could have a required reg address (which is the unaccessible
global register space), and make the "SMMU local base" thing a 2nd
optional address. Not sure if that is weird, since we can't actually
do anything with the global register space.
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + #iommu-cells = <1>;
>> + compatible = "qcom,msm-iommu-v1";
>> + ranges = <0 0x1e2 0x4>;
>> + reg = <0x1ef 0x3000>;
>> + clocks = <&gcc GCC_SMMU_CFG_CLK>,
>> + <&gcc GCC_APSS_TCU_CLK>;
>> + clock-names = "iface_clk", "bus_clk";
>> + qcom,iommu-secure-id = <17>;
>> +
>> + // mdp_0:
>> + msm-iommu-v1-ctx@4000 {
>
> iommu@...
it's not weird to have:
iommu@1e2 {
...
iommu@4000 {
...
};
};
??
BR,
-R
>> + compatible = "qcom,msm-iommu-v1-ns";
>> + reg = <0x4000 0x1000>;
>> + interrupts = ;
>> + };
>> +
>> + // venus_ns:
>> + msm-iommu-v1-ctx@5000 {
>> + compatible = "qcom,msm-iommu-v1-sec";
>> + reg = <0x5000 0x1000>;
>> + interrupts = ;
>> + };
>> + };
>> +
>> + gpu_iommu: msm-iommu-v1@1f08000 {
>
> ditto.
>
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + #iommu-cells = <1>;
>> + compatible = "qcom,msm-iommu-v1";
>> + ranges = <0 0x1f08000 0x1>;
>> + clocks = <&gcc GCC_SMMU_CFG_CLK>,
>> + <&gcc GCC_GFX_TCU_CLK>;
>> + clock-names = "iface_clk", "bus_clk";
>> + qcom,iommu-secure-id = <18>;
>> +
>> + // gfx3d_user:
>> + msm-iommu-v1-ctx@1f09000 {
>> + compatible = "qcom,msm-iommu-v1-ns";
>> + reg = <0x1000 0x1000>;
>> + interrupts = ;
>> + };
>> +
>> + // gfx3d_priv:
>> + msm-iommu-v1-ctx@1f0a000 {
>> + compatible =
Re: [RFC PATCH 22/30] iommu: Bind/unbind tasks to/from devices
Hi David, Good to hear back from you! On Fri, Mar 03, 2017 at 09:40:44AM +, David Woodhouse wrote: > > Intel slightly deviates from the "one PASID per process" vision too, > because it currently has a PASID allocator idr per IOMMU. That wants > making system-wide. And probably not Intel-specific. Intel version is getting the system wide pasid. We are actively working on it and should be out shortly. > > Some other comments... > > We also have SVM_FLAG_SUPERVISOR_MODE, which gives access to kernel > address space. Yes, people use it. There are some gaps with the current implementation, esp related to vmalloc. We have something internally, these will also be posted soon after some testing. Cheers, Ashok ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 22/30] iommu: Bind/unbind tasks to/from devices
Hi David, On Fri, Mar 03, 2017 at 09:40:44AM +, David Woodhouse wrote: > On Mon, 2017-02-27 at 19:54 +, Jean-Philippe Brucker wrote: > > Add three functions to the IOMMU API. iommu_bind_task takes a device and a > > task as argument. If the IOMMU, the device and the bus support it, attach > > task to device and create a Process Address Space ID (PASID) unique to the > > device. DMA from the device can then use the PASID to read or write into > > the address space. iommu_unbind_task removes a bond created with > > iommu_bind_task. iommu_set_svm_ops allows a device driver to set some > > callbacks for specific SVM-related operations. > > > > Try to accommodate current implementations (AMD, Intel and ARM), by > > letting the IOMMU driver do all the work, but attempt by the same occasion > > to find intersections between implementations. > > > > * amd_iommu_v2 expects the device to allocate a PASID and pass it to the > > IOMMU. The driver also provides separate functions to register callbacks > > that handles failed PRI requests and invalidate PASIDs. > > > > int amd_iommu_bind_pasid(struct pci_dev *pdev, int pasid, > > struct task_struct *task) > > void amd_iommu_unbind_pasid(struct pci_dev *pdev, int pasid) > > int amd_iommu_set_invalid_ppr_cb(struct pci_dev *pdev, > > amd_iommu_invalid_ppr_cb cb) > > int amd_iommu_set_invalidate_ctx_cb(struct pci_dev *pdev, > > amd_iommu_invalidate_ctx cb) > > > > * intel-svm allocates a PASID, and requires the driver to pass > > "svm_dev_ops", which currently contains a fault callback. It also > > doesn't take a task as argument, but uses 'current'. > > > > int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, > > struct svm_dev_ops *ops) > > int intel_svm_unbind_mm(struct device *dev, int pasid) > > > > * For arm-smmu-v3, PASID must be allocated by the SMMU driver since it > > indexes contexts in an array handled by the SMMU device. > > Right. The Intel version was designed with all of the above three in > mind. It was discussed at the Kernel Summit and LPC on more than one > occasion as it took shape, and what I implemented for Intel basicall > represents the consensus of what we thought it should look like. Good to know. I didn't intend to deviate much from the Intel version. > I meant to convert the AMD driver to the same API, but don't have > access to test hardware. Note that the amdkfd code will need careful > attention here. > > Intel slightly deviates from the "one PASID per process" vision too, > because it currently has a PASID allocator idr per IOMMU. That wants > making system-wide. And probably not Intel-specific. Yes, it would be nice to have a common PASID allocator. But I don't think that a system-wide PASID space is workable for us. At the moment systems might have a few identical devices all supporting 20 bits of PASID. But consider the case where one odd device can only handle four address spaces, and supports a maximum of two PASID bits. We'd quickly run out of PASIDs to hand to such devices, even though we could easily have one PASID space per endpoint (from a quick glance at the specs, I assume that both Intel and AMD IOMMUs offer one PASID table per RID.) For example, we could have three devices, all with different PASID capabilities, attached to process B. Device X was already bound to process A, so it gets PASID 2 for process B. Devices Y and Z get PASID 1 for process B: .-- PASID 1 - process A device X -+-- PASID 2 ---. device Y PASID 1 ---+- process B device Z PASID 1 ---' This is the model I implemented for ARM, as I wanted to keep the RFC simple before getting a chance to discuss the API. This is the finest granularity, but history shows that some systems cannot isolate all devices/RIDs from each others, be it by accident (bug, missing ACS) or by design (NTB). Arguably this hasn't come up with SVM yet, and it might never do. However, bringing IOMMU groups in the picture early seems preferable than having to redesign everything five years from now. We would have PASID spaces for each group, capped to the lowest common denominator in the group, pasid_bits = min(pasid_bits of each device): .- PASID 1 - process A device W -+- group -+- PASID 2 ---. device X -' | device X --- group --- PASID 1 ---+- process B device Z --- group --- PASID 1 ---' Finally, to get in line with the core API, we could make the iommu_domain hold the PASID space instead of the group. The API would then look like this: int iommu_bind(domain, task, &pasid, flags, priv) int iommu_unbind(domain, task, flags) IOMMU drivers would then call into a common PASID allocator and return a PASID unique to the domain. We can accept any number o
Re: [RFC PATCH 03/30] PCI: Move ATS declarations outside of CONFIG_PCI
On Mon, Feb 27, 2017 at 07:54:14PM +, Jean-Philippe Brucker wrote:
> Currently ATS helpers like pci_enable_ats are only defined when CONFIG_PCI
> is enabled. The ARM SMMU driver might get built with CONFIG_PCI disabled.
> It would thus have to wrap any use of ATS helpers around #ifdef
> CONFIG_PCI, which isn't ideal.
>
> A nicer solution is to always define these helpers. Since CONFIG_PCI_ATS
> is only enabled in association with CONFIG_PCI, move defines outside of
> CONFIG_PCI to prevent build failure when PCI is disabled.
>
> Signed-off-by: Jean-Philippe Brucker
I don't think there's any reason to make a pci_ats_init() stub when
CONFIG_PCI is not enabled, because it's only called from the PCI core.
But it does make some sense to keep them all together in one place.
I think you could also remove the #ifdef CONFIG_PCI_ATS in
arm_smmu_enable_ats() ("[RFC PATCH 04/30] iommu/arm-smmu-v3: Add
support for PCI ATS"), right?
If you remove the #ifdef, we'll call pci_enable_ats(), and it will
fail if !pdev->ats_cap.
Acked-by: Bjorn Helgaas
> ---
> include/linux/pci.h | 26 +-
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 282ed32244ce..e606f289bf5f 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1418,19 +1418,6 @@ int ht_create_irq(struct pci_dev *dev, int idx);
> void ht_destroy_irq(unsigned int irq);
> #endif /* CONFIG_HT_IRQ */
>
> -#ifdef CONFIG_PCI_ATS
> -/* Address Translation Service */
> -void pci_ats_init(struct pci_dev *dev);
> -int pci_enable_ats(struct pci_dev *dev, int ps);
> -void pci_disable_ats(struct pci_dev *dev);
> -int pci_ats_queue_depth(struct pci_dev *dev);
> -#else
> -static inline void pci_ats_init(struct pci_dev *d) { }
> -static inline int pci_enable_ats(struct pci_dev *d, int ps) { return
> -ENODEV; }
> -static inline void pci_disable_ats(struct pci_dev *d) { }
> -static inline int pci_ats_queue_depth(struct pci_dev *d) { return -ENODEV; }
> -#endif
> -
> #ifdef CONFIG_PCIE_PTM
> int pci_enable_ptm(struct pci_dev *dev, u8 *granularity);
> #else
> @@ -1616,6 +1603,19 @@ static inline int pci_get_new_domain_nr(void) { return
> -ENOSYS; }
> #define dev_is_pf(d) (false)
> #endif /* CONFIG_PCI */
>
> +#ifdef CONFIG_PCI_ATS
> +/* Address Translation Service */
> +void pci_ats_init(struct pci_dev *dev);
> +int pci_enable_ats(struct pci_dev *dev, int ps);
> +void pci_disable_ats(struct pci_dev *dev);
> +int pci_ats_queue_depth(struct pci_dev *dev);
> +#else
> +static inline void pci_ats_init(struct pci_dev *d) { }
> +static inline int pci_enable_ats(struct pci_dev *d, int ps) { return
> -ENODEV; }
> +static inline void pci_disable_ats(struct pci_dev *d) { }
> +static inline int pci_ats_queue_depth(struct pci_dev *d) { return -ENODEV; }
> +#endif
> +
> /* Include architecture-dependent settings and functions */
>
> #include
> --
> 2.11.0
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 18/30] PCI: Make "PRG Response PASID Required" handling common
On Mon, Feb 27, 2017 at 07:54:29PM +, Jean-Philippe Brucker wrote:
> The PASID ECN to the PCIe spec added a bit in the PRI status register that
> allows a Function to declare whether a PRG Response should contain the
> PASID prefix or not.
>
> Move the helper that accesses it from amd_iommu into the PCI subsystem,
> renaming it to something more consistent with the spec, and introducing
> another obscure acronym to make it all fit.
Maybe mention the acronym itelf and spell it out here?
> Signed-off-by: Jean-Philippe Brucker
Acked-by: Bjorn Helgaas
> ---
> drivers/iommu/amd_iommu.c | 19 +--
> drivers/pci/ats.c | 17 +
> include/linux/pci-ats.h | 8
> include/uapi/linux/pci_regs.h | 1 +
> 4 files changed, 27 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index 98940d1392cb..c5c598bf4ba3 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -2024,23 +2024,6 @@ static int pdev_iommuv2_enable(struct pci_dev *pdev)
> return ret;
> }
>
> -/* FIXME: Move this to PCI code */
> -#define PCI_PRI_TLP_OFF (1 << 15)
> -
> -static bool pci_pri_tlp_required(struct pci_dev *pdev)
> -{
> - u16 status;
> - int pos;
> -
> - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI);
> - if (!pos)
> - return false;
> -
> - pci_read_config_word(pdev, pos + PCI_PRI_STATUS, &status);
> -
> - return (status & PCI_PRI_TLP_OFF) ? true : false;
> -}
> -
> /*
> * If a device is not yet associated with a domain, this function
> * assigns it visible for the hardware
> @@ -2069,7 +2052,7 @@ static int attach_device(struct device *dev,
>
> dev_data->ats.enabled = true;
> dev_data->ats.qdep= pci_ats_queue_depth(pdev);
> - dev_data->pri_tlp = pci_pri_tlp_required(pdev);
> + dev_data->pri_tlp =
> pci_prg_resp_requires_prefix(pdev);
> }
> } else if (amd_iommu_iotlb_sup &&
> pci_enable_ats(pdev, PAGE_SHIFT) == 0) {
> diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
> index eeb9fb2b47aa..331376e9bb8b 100644
> --- a/drivers/pci/ats.c
> +++ b/drivers/pci/ats.c
> @@ -334,3 +334,20 @@ int pci_max_pasids(struct pci_dev *pdev)
> }
> EXPORT_SYMBOL_GPL(pci_max_pasids);
> #endif /* CONFIG_PCI_PASID */
> +
> +#if defined(CONFIG_PCI_PASID) && defined(CONFIG_PCI_PRI)
> +bool pci_prg_resp_requires_prefix(struct pci_dev *pdev)
> +{
> + u16 status;
> + int pos;
> +
> + pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI);
> + if (!pos)
> + return false;
> +
> + pci_read_config_word(pdev, pos + PCI_PRI_STATUS, &status);
> +
> + return !!(status & PCI_PRI_STATUS_PRPR);
> +}
> +EXPORT_SYMBOL_GPL(pci_prg_resp_requires_prefix);
> +#endif /* CONFIG_PCI_PASID && CONFIG_PCI_PRI */
> diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h
> index 57e0b8250947..e21bcacbe80c 100644
> --- a/include/linux/pci-ats.h
> +++ b/include/linux/pci-ats.h
> @@ -57,5 +57,13 @@ static inline int pci_max_pasids(struct pci_dev *pdev)
>
> #endif /* CONFIG_PCI_PASID */
>
> +#if defined(CONFIG_PCI_PASID) && defined(CONFIG_PCI_PRI)
> +bool pci_prg_resp_requires_prefix(struct pci_dev *pdev);
> +#else
> +static inline bool pci_prg_resp_requires_prefix(struct pci_dev *pdev)
> +{
> + return false;
> +}
> +#endif /* CONFIG_PCI_PASID && CONFIG_PCI_PRI */
>
> #endif /* LINUX_PCI_ATS_H*/
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index 634c9c44ed6c..bae815876be6 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -864,6 +864,7 @@
> #define PCI_PRI_STATUS_RF 0x001 /* Response Failure */
> #define PCI_PRI_STATUS_UPRGI0x002 /* Unexpected PRG index */
> #define PCI_PRI_STATUS_STOPPED 0x100 /* PRI Stopped */
> +#define PCI_PRI_STATUS_PRPR 0x8000 /* PRG Response requires PASID prefix */
> #define PCI_PRI_MAX_REQ 0x08/* PRI max reqs supported */
> #define PCI_PRI_ALLOC_REQ0x0c/* PRI max reqs allowed */
> #define PCI_EXT_CAP_PRI_SIZEOF 16
> --
> 2.11.0
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 19/30] PCI: Cache PRI and PASID bits in pci_dev
On Mon, Feb 27, 2017 at 07:54:30PM +, Jean-Philippe Brucker wrote:
> Device drivers need to check if an IOMMU enabled ATS, PRI and PASID in
> order to know when they can use the SVM API. Cache PRI and PASID bits in
> the pci_dev structure, similarly to what is currently done for ATS.
>
> Signed-off-by: Jean-Philippe Brucker
Acked-by: Bjorn Helgaas
> ---
> drivers/pci/ats.c | 23 +++
> include/linux/pci.h | 2 ++
> 2 files changed, 25 insertions(+)
>
> diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
> index 331376e9bb8b..486dc2208119 100644
> --- a/drivers/pci/ats.c
> +++ b/drivers/pci/ats.c
> @@ -153,6 +153,9 @@ int pci_enable_pri(struct pci_dev *pdev, u32 reqs)
> u32 max_requests;
> int pos;
>
> + if (WARN_ON(pdev->pri_enabled))
> + return -EBUSY;
> +
> pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI);
> if (!pos)
> return -EINVAL;
> @@ -170,6 +173,8 @@ int pci_enable_pri(struct pci_dev *pdev, u32 reqs)
> control |= PCI_PRI_CTRL_ENABLE;
> pci_write_config_word(pdev, pos + PCI_PRI_CTRL, control);
>
> + pdev->pri_enabled = 1;
> +
> return 0;
> }
> EXPORT_SYMBOL_GPL(pci_enable_pri);
> @@ -185,6 +190,9 @@ void pci_disable_pri(struct pci_dev *pdev)
> u16 control;
> int pos;
>
> + if (WARN_ON(!pdev->pri_enabled))
> + return;
> +
> pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI);
> if (!pos)
> return;
> @@ -192,6 +200,8 @@ void pci_disable_pri(struct pci_dev *pdev)
> pci_read_config_word(pdev, pos + PCI_PRI_CTRL, &control);
> control &= ~PCI_PRI_CTRL_ENABLE;
> pci_write_config_word(pdev, pos + PCI_PRI_CTRL, control);
> +
> + pdev->pri_enabled = 0;
> }
> EXPORT_SYMBOL_GPL(pci_disable_pri);
>
> @@ -207,6 +217,9 @@ int pci_reset_pri(struct pci_dev *pdev)
> u16 control;
> int pos;
>
> + if (WARN_ON(pdev->pri_enabled))
> + return -EBUSY;
> +
> pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI);
> if (!pos)
> return -EINVAL;
> @@ -239,6 +252,9 @@ int pci_enable_pasid(struct pci_dev *pdev, int features)
> u16 control, supported;
> int pos;
>
> + if (WARN_ON(pdev->pasid_enabled))
> + return -EBUSY;
> +
> pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID);
> if (!pos)
> return -EINVAL;
> @@ -259,6 +275,8 @@ int pci_enable_pasid(struct pci_dev *pdev, int features)
>
> pci_write_config_word(pdev, pos + PCI_PASID_CTRL, control);
>
> + pdev->pasid_enabled = 1;
> +
> return 0;
> }
> EXPORT_SYMBOL_GPL(pci_enable_pasid);
> @@ -273,11 +291,16 @@ void pci_disable_pasid(struct pci_dev *pdev)
> u16 control = 0;
> int pos;
>
> + if (WARN_ON(!pdev->pasid_enabled))
> + return;
> +
> pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID);
> if (!pos)
> return;
>
> pci_write_config_word(pdev, pos + PCI_PASID_CTRL, control);
> +
> + pdev->pasid_enabled = 0;
> }
> EXPORT_SYMBOL_GPL(pci_disable_pasid);
>
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index e606f289bf5f..47c353ca9957 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -351,6 +351,8 @@ struct pci_dev {
> unsigned intmsix_enabled:1;
> unsigned intari_enabled:1; /* ARI forwarding */
> unsigned intats_enabled:1; /* Address Translation Service */
> + unsigned intpasid_enabled:1;/* Process Address Space ID */
> + unsigned intpri_enabled:1; /* Page Request Interface */
> unsigned intis_managed:1;
> unsigned intneeds_freset:1; /* Dev requires fundamental reset */
> unsigned intstate_saved:1;
> --
> 2.11.0
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
