Re: [PATCH 1/2] mm/cma: remove unsupported gfp_mask parameter from cma_alloc()
2018-07-11 17:54 GMT+09:00 Michal Hocko :
> On Wed 11-07-18 16:35:28, Joonsoo Kim wrote:
>> 2018-07-10 18:50 GMT+09:00 Michal Hocko :
>> > On Tue 10-07-18 16:19:32, Joonsoo Kim wrote:
>> >> Hello, Marek.
>> >>
>> >> 2018-07-09 21:19 GMT+09:00 Marek Szyprowski :
>> >> > cma_alloc() function doesn't really support gfp flags other than
>> >> > __GFP_NOWARN, so convert gfp_mask parameter to boolean no_warn
>> >> > parameter.
>> >>
>> >> Although gfp_mask isn't used in cma_alloc() except no_warn, it can be used
>> >> in alloc_contig_range(). For example, if passed gfp mask has no __GFP_FS,
>> >> compaction(isolation) would work differently. Do you have considered
>> >> such a case?
>> >
>> > Does any of cma_alloc users actually care about GFP_NO{FS,IO}?
>>
>> I don't know. My guess is that cma_alloc() is used for DMA allocation so
>> block device would use it, too. If fs/block subsystem initiates the
>> request for the device,
>> it would be possible that cma_alloc() is called with such a flag.
>> Again, I don't know
>> much about those subsystem so I would be wrong.
>
> The patch converts existing users and none of them really tries to use
> anything other than GFP_KERNEL [|__GFP_NOWARN] so this doesn't seem to
> be the case. Should there be a new user requiring more restricted
> gfp_mask we should carefuly re-evaluate and think how to support it.
One of existing user is general DMA layer and it takes gfp flags that is
provided by user. I don't check all the DMA allocation sites but how do
you convince that none of them try to use anything other
than GFP_KERNEL [|__GFP_NOWARN]?
Thanks.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] iommu: Add config option to set passthrough as default
Hi, Olof,
Tired of changing command line. I like this patch.
Thanks.
Shunyong.
On Wed, 2018-07-11 at 13:59 -0700, Olof Johansson wrote:
> This allows the default behavior to be controlled by a kernel config
> option instead of changing the commandline for the kernel to include
> "iommu.passthrough=on" on machines where this is desired.
>
> Signed-off-by: Olof Johansson
> ---
> drivers/iommu/Kconfig | 10 ++
> drivers/iommu/iommu.c | 4
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index 689ffe538370..a9bb1a5b5e43 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -60,6 +60,16 @@ config IOMMU_IO_PGTABLE_ARMV7S_SELFTEST
>
> endmenu
>
> +config IOMMU_DEFAULT_PASSTHROUGH
> + bool "IOMMU passthrough by default"
> + depends on IOMMU_API
> +help
> + Enable passthrough by default (removing the need to pass
> in
> + iommu.passthrough=on through command line). If this is
> enabled,
> + you can still disable with iommu.passthrough=off
> +
> + If unsure, say N here.
> +
> config IOMMU_IOVA
> tristate
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 63b37563db7e..ab8fc54467e0 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -36,7 +36,11 @@
>
> static struct kset *iommu_group_kset;
> static DEFINE_IDA(iommu_group_ida);
> +#ifdef CONFIG_IOMMU_DEFAULT_PASSTHROUGH
> +static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_IDENTITY;
> +#else
> static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_DMA;
> +#endif
>
> struct iommu_callback_data {
> const struct iommu_ops *ops;
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/2] iommu: add sysfs attribyte for domain type
While we could print it at setup time, this is an easier way to match
each device to their default IOMMU allocation type.
Signed-off-by: Olof Johansson
---
drivers/iommu/iommu.c | 32
1 file changed, 32 insertions(+)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index ab8fc54467e0..53164107620c 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -298,11 +298,39 @@ static ssize_t iommu_group_show_resv_regions(struct
iommu_group *group,
return (str - buf);
}
+static ssize_t iommu_group_show_type(struct iommu_group *group,
+char *buf)
+{
+ char *type = "unknown\n";
+
+ if (group->default_domain) {
+ switch (group->default_domain->type) {
+ case IOMMU_DOMAIN_BLOCKED:
+ type = "blocked\n";
+ break;
+ case IOMMU_DOMAIN_IDENTITY:
+ type = "identity\n";
+ break;
+ case IOMMU_DOMAIN_UNMANAGED:
+ type = "unmanaged\n";
+ break;
+ case IOMMU_DOMAIN_DMA:
+ type = "DMA";
+ break;
+ }
+ }
+ strcpy(buf, type);
+
+ return strlen(type);
+}
+
static IOMMU_GROUP_ATTR(name, S_IRUGO, iommu_group_show_name, NULL);
static IOMMU_GROUP_ATTR(reserved_regions, 0444,
iommu_group_show_resv_regions, NULL);
+static IOMMU_GROUP_ATTR(type, 0444, iommu_group_show_type, NULL);
+
static void iommu_group_release(struct kobject *kobj)
{
struct iommu_group *group = to_iommu_group(kobj);
@@ -384,6 +412,10 @@ struct iommu_group *iommu_group_alloc(void)
if (ret)
return ERR_PTR(ret);
+ ret = iommu_group_create_file(group, _group_attr_type);
+ if (ret)
+ return ERR_PTR(ret);
+
pr_debug("Allocated group %d\n", group->id);
return group;
--
2.11.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/2] iommu: Add config option to set passthrough as default
This allows the default behavior to be controlled by a kernel config
option instead of changing the commandline for the kernel to include
"iommu.passthrough=on" on machines where this is desired.
Signed-off-by: Olof Johansson
---
drivers/iommu/Kconfig | 10 ++
drivers/iommu/iommu.c | 4
2 files changed, 14 insertions(+)
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 689ffe538370..a9bb1a5b5e43 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -60,6 +60,16 @@ config IOMMU_IO_PGTABLE_ARMV7S_SELFTEST
endmenu
+config IOMMU_DEFAULT_PASSTHROUGH
+ bool "IOMMU passthrough by default"
+ depends on IOMMU_API
+help
+ Enable passthrough by default (removing the need to pass in
+ iommu.passthrough=on through command line). If this is enabled,
+ you can still disable with iommu.passthrough=off
+
+ If unsure, say N here.
+
config IOMMU_IOVA
tristate
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 63b37563db7e..ab8fc54467e0 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -36,7 +36,11 @@
static struct kset *iommu_group_kset;
static DEFINE_IDA(iommu_group_ida);
+#ifdef CONFIG_IOMMU_DEFAULT_PASSTHROUGH
+static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_IDENTITY;
+#else
static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_DMA;
+#endif
struct iommu_callback_data {
const struct iommu_ops *ops;
--
2.11.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v12 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops
On Wed, Jul 11, 2018 at 3:40 PM, Marek Szyprowski
wrote:
> Hi Tomasz,
>
> On 2018-07-11 14:51, Tomasz Figa wrote:
>> On Wed, Jul 11, 2018 at 8:11 PM Rafael J. Wysocki wrote:
>>> On Wed, Jul 11, 2018 at 12:55 PM, Vivek Gautam
>>> wrote:
On Wed, Jul 11, 2018 at 3:20 PM, Rafael J. Wysocki
wrote:
> On Sunday, July 8, 2018 7:34:10 PM CEST Vivek Gautam wrote:
>> From: Sricharan R
>>
>> The smmu needs to be functional only when the respective
>> master's using it are active. The device_link feature
>> helps to track such functional dependencies, so that the
>> iommu gets powered when the master device enables itself
>> using pm_runtime. So by adapting the smmu driver for
>> runtime pm, above said dependency can be addressed.
>>
>> This patch adds the pm runtime/sleep callbacks to the
>> driver and also the functions to parse the smmu clocks
>> from DT and enable them in resume/suspend.
>>
>> Signed-off-by: Sricharan R
>> Signed-off-by: Archit Taneja
>> [vivek: Clock rework to request bulk of clocks]
>> Signed-off-by: Vivek Gautam
>> Reviewed-by: Tomasz Figa
>> ---
>>
>> - No change since v11.
>>
>> drivers/iommu/arm-smmu.c | 60
>> ++--
>> 1 file changed, 58 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index f7a96bcf94a6..a01d0dde21dd 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -48,6 +48,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>>
>> @@ -205,6 +206,8 @@ struct arm_smmu_device {
>>u32 num_global_irqs;
>>u32 num_context_irqs;
>>unsigned int*irqs;
>> + struct clk_bulk_data*clks;
>> + int num_clks;
>>
>>u32 cavium_id_base; /* Specific to
>> Cavium */
>>
>> @@ -1897,10 +1900,12 @@ static int arm_smmu_device_cfg_probe(struct
>> arm_smmu_device *smmu)
>> struct arm_smmu_match_data {
>>enum arm_smmu_arch_version version;
>>enum arm_smmu_implementation model;
>> + const char * const *clks;
>> + int num_clks;
>> };
>>
>> #define ARM_SMMU_MATCH_DATA(name, ver, imp) \
>> -static struct arm_smmu_match_data name = { .version = ver, .model = imp
>> }
>> +static const struct arm_smmu_match_data name = { .version = ver, .model
>> = imp }
>>
>> ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
>> ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
>> @@ -1919,6 +1924,23 @@ static const struct of_device_id
>> arm_smmu_of_match[] = {
>> };
>> MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
>>
>> +static void arm_smmu_fill_clk_data(struct arm_smmu_device *smmu,
>> +const char * const *clks)
>> +{
>> + int i;
>> +
>> + if (smmu->num_clks < 1)
>> + return;
>> +
>> + smmu->clks = devm_kcalloc(smmu->dev, smmu->num_clks,
>> + sizeof(*smmu->clks), GFP_KERNEL);
>> + if (!smmu->clks)
>> + return;
>> +
>> + for (i = 0; i < smmu->num_clks; i++)
>> + smmu->clks[i].id = clks[i];
>> +}
>> +
>> #ifdef CONFIG_ACPI
>> static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu)
>> {
>> @@ -2001,6 +2023,9 @@ static int arm_smmu_device_dt_probe(struct
>> platform_device *pdev,
>>data = of_device_get_match_data(dev);
>>smmu->version = data->version;
>>smmu->model = data->model;
>> + smmu->num_clks = data->num_clks;
>> +
>> + arm_smmu_fill_clk_data(smmu, data->clks);
>>
>>parse_driver_options(smmu);
>>
>> @@ -2099,6 +2124,14 @@ static int arm_smmu_device_probe(struct
>> platform_device *pdev)
>>smmu->irqs[i] = irq;
>>}
>>
>> + err = devm_clk_bulk_get(smmu->dev, smmu->num_clks, smmu->clks);
>> + if (err)
>> + return err;
>> +
>> + err = clk_bulk_prepare(smmu->num_clks, smmu->clks);
>> + if (err)
>> + return err;
>> +
>>err = arm_smmu_device_cfg_probe(smmu);
>>if (err)
>>return err;
>> @@ -2181,6 +2214,9 @@ static int arm_smmu_device_remove(struct
>> platform_device *pdev)
>>
>>/* Turn the thing off */
>>writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
>> +
>> + clk_bulk_unprepare(smmu->num_clks,
Re: [PATCH] swiotlb: Clean up reporting
On Tue, Jul 10, 2018 at 04:22:22PM -0700, Kees Cook wrote:
> This removes needless use of '%p', and refactors the printk calls to
> use pr_*() helpers instead.
>
> Signed-off-by: Kees Cook
Reviewed-by: Konrad Rzeszutek Wilk
Christoph, volunteered to pick this patch up in his tree. Thank you!
> ---
> kernel/dma/swiotlb.c | 18 --
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index 904541055792..4f8a6dbf0b60 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -17,6 +17,8 @@
> * 08/12/11 beckyb Add highmem support
> */
>
> +#define pr_fmt(fmt) "software IO TLB: " fmt
> +
> #include
> #include
> #include
> @@ -162,20 +164,16 @@ static bool no_iotlb_memory;
> void swiotlb_print_info(void)
> {
> unsigned long bytes = io_tlb_nslabs << IO_TLB_SHIFT;
> - unsigned char *vstart, *vend;
>
> if (no_iotlb_memory) {
> - pr_warn("software IO TLB: No low mem\n");
> + pr_warn("No low mem\n");
> return;
> }
>
> - vstart = phys_to_virt(io_tlb_start);
> - vend = phys_to_virt(io_tlb_end);
> -
> - printk(KERN_INFO "software IO TLB [mem %#010llx-%#010llx] (%luMB)
> mapped at [%p-%p]\n",
> + pr_info("mapped [mem %#010llx-%#010llx] (%luMB)\n",
> (unsigned long long)io_tlb_start,
> (unsigned long long)io_tlb_end,
> -bytes >> 20, vstart, vend - 1);
> +bytes >> 20);
> }
>
> /*
> @@ -275,7 +273,7 @@ swiotlb_init(int verbose)
> if (io_tlb_start)
> memblock_free_early(io_tlb_start,
> PAGE_ALIGN(io_tlb_nslabs << IO_TLB_SHIFT));
> - pr_warn("Cannot allocate SWIOTLB buffer");
> + pr_warn("Cannot allocate buffer");
> no_iotlb_memory = true;
> }
>
> @@ -317,8 +315,8 @@ swiotlb_late_init_with_default_size(size_t default_size)
> return -ENOMEM;
> }
> if (order != get_order(bytes)) {
> - printk(KERN_WARNING "Warning: only able to allocate %ld MB "
> -"for software IO TLB\n", (PAGE_SIZE << order) >> 20);
> + pr_warn("only able to allocate %ld MB\n",
> + (PAGE_SIZE << order) >> 20);
> io_tlb_nslabs = SLABS_PER_PAGE << order;
> }
> rc = swiotlb_late_init_with_tbl(vstart, io_tlb_nslabs);
> --
> 2.17.1
>
>
> --
> Kees Cook
> Pixel Security
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] xtensa: use generic dma_noncoherent_ops
On Wed, Jul 11, 2018 at 11:00 AM, Christoph Hellwig wrote: > On Wed, Jul 11, 2018 at 10:57:30AM -0700, Max Filippov wrote: >> > config XTENSA >> > def_bool y >> > select ARCH_NO_COHERENT_DMA_MMAP if !MMU >> > + select ARCH_HAS_SYNC_DMA_FOR_CPU >> > + select ARCH_HAS_SYNC_DMA_FOR_DEVICE >> >> This breaks alphabetical order of selects. Otherwise LGTM. >> Sorry for the delay. >> Acked-by: Max Filippov >> >> Will you submit it further or should I take it into the xtensa tree? > > Either way is fine with me, just let me know which way you prefer. I'll take it then. Thank you! -- Max ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 2/4] ACPI/IORT: Set bus DMA mask as appropriate
On 10/07/18 19:04, Christoph Hellwig wrote: On Tue, Jul 10, 2018 at 06:17:17PM +0100, Robin Murphy wrote: When an explicit DMA limit is described by firmware, we need to remember it regardless of how drivers might subsequently update their devices' masks. The new bus_dma_mask field does that. Shouldn't we also stop presetting the dma mask after this? I guess initialising the device masks here only really has any effect if drivers fail to set their own, so if we're getting stricter about that then it would make sense to stop; I'll add a couple of patches on top to clean that up. Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] xtensa: use generic dma_noncoherent_ops
Hi Christoph, On Tue, Jun 19, 2018 at 12:03 AM, Christoph Hellwig wrote: > Switch to the generic noncoherent direct mapping implementation. > > Signed-off-by: Christoph Hellwig > --- > arch/xtensa/Kconfig | 3 + > arch/xtensa/include/asm/Kbuild| 1 + > arch/xtensa/include/asm/dma-mapping.h | 26 -- > arch/xtensa/kernel/pci-dma.c | 130 +++--- > 4 files changed, 19 insertions(+), 141 deletions(-) > delete mode 100644 arch/xtensa/include/asm/dma-mapping.h > > diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig > index d575e8701955..373708c77367 100644 > --- a/arch/xtensa/Kconfig > +++ b/arch/xtensa/Kconfig > @@ -5,11 +5,14 @@ config ZONE_DMA > config XTENSA > def_bool y > select ARCH_NO_COHERENT_DMA_MMAP if !MMU > + select ARCH_HAS_SYNC_DMA_FOR_CPU > + select ARCH_HAS_SYNC_DMA_FOR_DEVICE This breaks alphabetical order of selects. Otherwise LGTM. Sorry for the delay. Acked-by: Max Filippov Will you submit it further or should I take it into the xtensa tree? -- Thanks. -- Max ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] xtensa: use generic dma_noncoherent_ops
On Wed, Jul 11, 2018 at 10:57:30AM -0700, Max Filippov wrote: > > config XTENSA > > def_bool y > > select ARCH_NO_COHERENT_DMA_MMAP if !MMU > > + select ARCH_HAS_SYNC_DMA_FOR_CPU > > + select ARCH_HAS_SYNC_DMA_FOR_DEVICE > > This breaks alphabetical order of selects. Otherwise LGTM. > Sorry for the delay. > Acked-by: Max Filippov > > Will you submit it further or should I take it into the xtensa tree? Either way is fine with me, just let me know which way you prefer. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 1/4] dma-mapping: Generalise dma_32bit_limit flag
On 10/07/18 19:04, Christoph Hellwig wrote: On Tue, Jul 10, 2018 at 06:17:16PM +0100, Robin Murphy wrote: diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c index 8be8106270c2..95e185347e34 100644 --- a/kernel/dma/direct.c +++ b/kernel/dma/direct.c @@ -183,7 +183,7 @@ int dma_direct_supported(struct device *dev, u64 mask) * Various PCI/PCIe bridges have broken support for > 32bit DMA even * if the device itself might support it. */ - if (dev->dma_32bit_limit && mask > DMA_BIT_MASK(32)) + if (dev->bus_dma_mask && mask > dev->bus_dma_mask) return 0; The comment above this check needs an updated (or just be removed). Right, I'll give it a tweak. I could also do with actually getting the field name correct in via_no_dac_cb()... Also we still have a few architectures not using dma-direct. I guess most were doing fine without such limits anyway, but at least arm will probably need an equivalent check. Indeed, once we've found an approach that everyone's happy with we can have a more thorough audit of exactly where else it needs to be applied. FWIW I'm not aware of any 32-bit Arm systems affected by this*, but if they do exist then at least there's no risk of regression since they've always been busted. Robin. * Not counting the somewhat-similar StrongArm DMA controller bug where one bit in the *middle* of the mask is unusable. Let's keep that confined to the Arm dmabounce code and never speak of it... ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 0/4] Stop losing firmware-set DMA masks
On 11/07/18 15:40, Rob Herring wrote:
On Tue, Jul 10, 2018 at 12:43 PM Robin Murphy wrote:
Whilst the common firmware code invoked by dma_configure() initialises
devices' DMA masks according to limitations described by the respective
properties ("dma-ranges" for OF and _DMA/IORT for ACPI), the nature of
the dma_set_mask() API leads to that information getting lost when
well-behaved drivers probe and set a 64-bit mask, since in general
there's no way to tell the difference between a firmware-described mask
(which should be respected) and whatever default may have come from the
bus code (which should be replaced outright). This can break DMA on
systems with certain IOMMU topologies (e.g. [1]) where the IOMMU driver
only knows its maximum supported address size, not how many of those
address bits might actually be wired up between any of its input
interfaces and the associated DMA master devices. Similarly, some PCIe
root complexes only have a 32-bit native interface on their host bridge,
which leads to the same DMA-address-truncation problem in systems with a
larger physical memory map and RAM above 4GB (e.g. [2]).
These patches attempt to deal with this in the simplest way possible by
generalising the specific quirk for 32-bit bridges into an arbitrary
mask which can then also be plumbed into the firmware code. In the
interest of being minimally invasive, I've only included a point fix
for the IOMMU issue as seen on arm64 - there may be further tweaks
needed in DMA ops to catch all possible incarnations of this problem,
but this initial RFC is mostly about the impact beyond the dma-mapping
subsystem itself.
Couldn't you set and use the device's parent's dma_mask instead. At
least for DT, we should always have a parent device representing the
bus. That would avoid further bloating of struct device.
But then if the parent device did have a non-trivial driver which calls
dma_set_mask(), we'd be back at square 1 :/
More realistically, I don't think that's viable for ACPI, at least with
IORT, since the memory address size limit belongs to the endpoint
itself, thus two devices with the same nominal parent in the Linux
device model could still have different limits (where in DT you'd have
to have to insert intermediate simple-bus nodes to model the same
topology with dma-ranges). Plus either way it seems somewhat fragile for
PCI where the host bridge may be some distance up the hierarchy.
Robin.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: convert parisc to the generic dma-noncoherent code
ping? Any comments? On Tue, Jun 19, 2018 at 09:04:52AM +0200, Christoph Hellwig wrote: > This should address all the comments raised last time. > > > ___ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu ---end quoted text--- ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] xtensa: use generic dma_noncoherent_ops
ping? Any comments?
On Tue, Jun 19, 2018 at 09:03:16AM +0200, Christoph Hellwig wrote:
> Switch to the generic noncoherent direct mapping implementation.
>
> Signed-off-by: Christoph Hellwig
> ---
> arch/xtensa/Kconfig | 3 +
> arch/xtensa/include/asm/Kbuild| 1 +
> arch/xtensa/include/asm/dma-mapping.h | 26 --
> arch/xtensa/kernel/pci-dma.c | 130 +++---
> 4 files changed, 19 insertions(+), 141 deletions(-)
> delete mode 100644 arch/xtensa/include/asm/dma-mapping.h
>
> diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig
> index d575e8701955..373708c77367 100644
> --- a/arch/xtensa/Kconfig
> +++ b/arch/xtensa/Kconfig
> @@ -5,11 +5,14 @@ config ZONE_DMA
> config XTENSA
> def_bool y
> select ARCH_NO_COHERENT_DMA_MMAP if !MMU
> + select ARCH_HAS_SYNC_DMA_FOR_CPU
> + select ARCH_HAS_SYNC_DMA_FOR_DEVICE
> select ARCH_WANT_FRAME_POINTERS
> select ARCH_WANT_IPC_PARSE_VERSION
> select BUILDTIME_EXTABLE_SORT
> select CLONE_BACKWARDS
> select COMMON_CLK
> + select DMA_NONCOHERENT_OPS
> select GENERIC_ATOMIC64
> select GENERIC_CLOCKEVENTS
> select GENERIC_IRQ_SHOW
> diff --git a/arch/xtensa/include/asm/Kbuild b/arch/xtensa/include/asm/Kbuild
> index e5e1e61c538c..82c756431b49 100644
> --- a/arch/xtensa/include/asm/Kbuild
> +++ b/arch/xtensa/include/asm/Kbuild
> @@ -3,6 +3,7 @@ generic-y += compat.h
> generic-y += device.h
> generic-y += div64.h
> generic-y += dma-contiguous.h
> +generic-y += dma-mapping.h
> generic-y += emergency-restart.h
> generic-y += exec.h
> generic-y += extable.h
> diff --git a/arch/xtensa/include/asm/dma-mapping.h
> b/arch/xtensa/include/asm/dma-mapping.h
> deleted file mode 100644
> index 44098800dad7..
> --- a/arch/xtensa/include/asm/dma-mapping.h
> +++ /dev/null
> @@ -1,26 +0,0 @@
> -/*
> - * This file is subject to the terms and conditions of the GNU General Public
> - * License. See the file "COPYING" in the main directory of this archive
> - * for more details.
> - *
> - * Copyright (C) 2003 - 2005 Tensilica Inc.
> - * Copyright (C) 2015 Cadence Design Systems Inc.
> - */
> -
> -#ifndef _XTENSA_DMA_MAPPING_H
> -#define _XTENSA_DMA_MAPPING_H
> -
> -#include
> -#include
> -
> -#include
> -#include
> -
> -extern const struct dma_map_ops xtensa_dma_map_ops;
> -
> -static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type
> *bus)
> -{
> - return _dma_map_ops;
> -}
> -
> -#endif /* _XTENSA_DMA_MAPPING_H */
> diff --git a/arch/xtensa/kernel/pci-dma.c b/arch/xtensa/kernel/pci-dma.c
> index 392b4a80ebc2..a83d60e92908 100644
> --- a/arch/xtensa/kernel/pci-dma.c
> +++ b/arch/xtensa/kernel/pci-dma.c
> @@ -16,26 +16,24 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> #include
> -#include
> -#include
> -#include
> #include
> #include
> #include
>
> -static void do_cache_op(dma_addr_t dma_handle, size_t size,
> +static void do_cache_op(phys_addr_t paddr, size_t size,
> void (*fn)(unsigned long, unsigned long))
> {
> - unsigned long off = dma_handle & (PAGE_SIZE - 1);
> - unsigned long pfn = PFN_DOWN(dma_handle);
> + unsigned long off = paddr & (PAGE_SIZE - 1);
> + unsigned long pfn = PFN_DOWN(paddr);
> struct page *page = pfn_to_page(pfn);
>
> if (!PageHighMem(page))
> - fn((unsigned long)bus_to_virt(dma_handle), size);
> + fn((unsigned long)phys_to_virt(paddr), size);
> else
> while (size > 0) {
> size_t sz = min_t(size_t, size, PAGE_SIZE - off);
> @@ -49,14 +47,13 @@ static void do_cache_op(dma_addr_t dma_handle, size_t
> size,
> }
> }
>
> -static void xtensa_sync_single_for_cpu(struct device *dev,
> -dma_addr_t dma_handle, size_t size,
> -enum dma_data_direction dir)
> +void arch_sync_dma_for_cpu(struct device *dev, phys_addr_t paddr,
> + size_t size, enum dma_data_direction dir)
> {
> switch (dir) {
> case DMA_BIDIRECTIONAL:
> case DMA_FROM_DEVICE:
> - do_cache_op(dma_handle, size, __invalidate_dcache_range);
> + do_cache_op(paddr, size, __invalidate_dcache_range);
> break;
>
> case DMA_NONE:
> @@ -68,15 +65,14 @@ static void xtensa_sync_single_for_cpu(struct device *dev,
> }
> }
>
> -static void xtensa_sync_single_for_device(struct device *dev,
> - dma_addr_t dma_handle, size_t size,
> - enum dma_data_direction dir)
> +void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr,
> + size_t size, enum dma_data_direction dir)
> {
> switch (dir) {
> case DMA_BIDIRECTIONAL:
> case DMA_TO_DEVICE:
> if (XCHAL_DCACHE_IS_WRITEBACK)
> -
Re: [PATCH] nios2: use generic dma_noncoherent_ops
ping? Any comments?
On Tue, Jun 19, 2018 at 09:01:37AM +0200, Christoph Hellwig wrote:
> Switch to the generic noncoherent direct mapping implementation.
>
> Signed-off-by: Christoph Hellwig
> ---
> arch/nios2/Kconfig | 3 +
> arch/nios2/include/asm/Kbuild| 1 +
> arch/nios2/include/asm/dma-mapping.h | 20
> arch/nios2/mm/dma-mapping.c | 139 +++
> 4 files changed, 17 insertions(+), 146 deletions(-)
> delete mode 100644 arch/nios2/include/asm/dma-mapping.h
>
> diff --git a/arch/nios2/Kconfig b/arch/nios2/Kconfig
> index 3d4ec88f1db1..92035042cf62 100644
> --- a/arch/nios2/Kconfig
> +++ b/arch/nios2/Kconfig
> @@ -1,6 +1,9 @@
> # SPDX-License-Identifier: GPL-2.0
> config NIOS2
> def_bool y
> + select ARCH_HAS_SYNC_DMA_FOR_CPU
> + select ARCH_HAS_SYNC_DMA_FOR_DEVICE
> + select DMA_NONCOHERENT_OPS
> select TIMER_OF
> select GENERIC_ATOMIC64
> select GENERIC_CLOCKEVENTS
> diff --git a/arch/nios2/include/asm/Kbuild b/arch/nios2/include/asm/Kbuild
> index 64ed3d656956..8fde4fa2c34f 100644
> --- a/arch/nios2/include/asm/Kbuild
> +++ b/arch/nios2/include/asm/Kbuild
> @@ -9,6 +9,7 @@ generic-y += current.h
> generic-y += device.h
> generic-y += div64.h
> generic-y += dma.h
> +generic-y += dma-mapping.h
> generic-y += emergency-restart.h
> generic-y += exec.h
> generic-y += extable.h
> diff --git a/arch/nios2/include/asm/dma-mapping.h
> b/arch/nios2/include/asm/dma-mapping.h
> deleted file mode 100644
> index 6ceb92251da0..
> --- a/arch/nios2/include/asm/dma-mapping.h
> +++ /dev/null
> @@ -1,20 +0,0 @@
> -/*
> - * Copyright (C) 2011 Tobias Klauser
> - * Copyright (C) 2009 Wind River Systems Inc
> - *
> - * This file is subject to the terms and conditions of the GNU General
> - * Public License. See the file COPYING in the main directory of this
> - * archive for more details.
> - */
> -
> -#ifndef _ASM_NIOS2_DMA_MAPPING_H
> -#define _ASM_NIOS2_DMA_MAPPING_H
> -
> -extern const struct dma_map_ops nios2_dma_ops;
> -
> -static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type
> *bus)
> -{
> - return _dma_ops;
> -}
> -
> -#endif /* _ASM_NIOS2_DMA_MAPPING_H */
> diff --git a/arch/nios2/mm/dma-mapping.c b/arch/nios2/mm/dma-mapping.c
> index 4be815519dd4..4af9e5b5ba1c 100644
> --- a/arch/nios2/mm/dma-mapping.c
> +++ b/arch/nios2/mm/dma-mapping.c
> @@ -12,18 +12,18 @@
>
> #include
> #include
> -#include
> #include
> -#include
> #include
> #include
> #include
> #include
>
> -static inline void __dma_sync_for_device(void *vaddr, size_t size,
> - enum dma_data_direction direction)
> +void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr,
> + size_t size, enum dma_data_direction dir)
> {
> - switch (direction) {
> + void *vaddr = phys_to_virt(paddr);
> +
> + switch (dir) {
> case DMA_FROM_DEVICE:
> invalidate_dcache_range((unsigned long)vaddr,
> (unsigned long)(vaddr + size));
> @@ -42,10 +42,12 @@ static inline void __dma_sync_for_device(void *vaddr,
> size_t size,
> }
> }
>
> -static inline void __dma_sync_for_cpu(void *vaddr, size_t size,
> - enum dma_data_direction direction)
> +void arch_sync_dma_for_cpu(struct device *dev, phys_addr_t paddr,
> + size_t size, enum dma_data_direction dir)
> {
> - switch (direction) {
> + void *vaddr = phys_to_virt(paddr);
> +
> + switch (dir) {
> case DMA_BIDIRECTIONAL:
> case DMA_FROM_DEVICE:
> invalidate_dcache_range((unsigned long)vaddr,
> @@ -58,8 +60,8 @@ static inline void __dma_sync_for_cpu(void *vaddr, size_t
> size,
> }
> }
>
> -static void *nios2_dma_alloc(struct device *dev, size_t size,
> - dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
> +void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
> + gfp_t gfp, unsigned long attrs)
> {
> void *ret;
>
> @@ -80,125 +82,10 @@ static void *nios2_dma_alloc(struct device *dev, size_t
> size,
> return ret;
> }
>
> -static void nios2_dma_free(struct device *dev, size_t size, void *vaddr,
> +void arch_dma_free(struct device *dev, size_t size, void *vaddr,
> dma_addr_t dma_handle, unsigned long attrs)
> {
> unsigned long addr = (unsigned long) CAC_ADDR((unsigned long) vaddr);
>
> free_pages(addr, get_order(size));
> }
> -
> -static int nios2_dma_map_sg(struct device *dev, struct scatterlist *sg,
> - int nents, enum dma_data_direction direction,
> - unsigned long attrs)
> -{
> - int i;
> -
> - for_each_sg(sg, sg, nents, i) {
> - void *addr = sg_virt(sg);
> -
> - if (!addr)
> - continue;
> -
> - sg->dma_address = sg_phys(sg);
> -
> - if (attrs & DMA_ATTR_SKIP_CPU_SYNC)
Re: [RFC PATCH 0/4] Stop losing firmware-set DMA masks
On Tue, Jul 10, 2018 at 12:43 PM Robin Murphy wrote:
>
> Whilst the common firmware code invoked by dma_configure() initialises
> devices' DMA masks according to limitations described by the respective
> properties ("dma-ranges" for OF and _DMA/IORT for ACPI), the nature of
> the dma_set_mask() API leads to that information getting lost when
> well-behaved drivers probe and set a 64-bit mask, since in general
> there's no way to tell the difference between a firmware-described mask
> (which should be respected) and whatever default may have come from the
> bus code (which should be replaced outright). This can break DMA on
> systems with certain IOMMU topologies (e.g. [1]) where the IOMMU driver
> only knows its maximum supported address size, not how many of those
> address bits might actually be wired up between any of its input
> interfaces and the associated DMA master devices. Similarly, some PCIe
> root complexes only have a 32-bit native interface on their host bridge,
> which leads to the same DMA-address-truncation problem in systems with a
> larger physical memory map and RAM above 4GB (e.g. [2]).
>
> These patches attempt to deal with this in the simplest way possible by
> generalising the specific quirk for 32-bit bridges into an arbitrary
> mask which can then also be plumbed into the firmware code. In the
> interest of being minimally invasive, I've only included a point fix
> for the IOMMU issue as seen on arm64 - there may be further tweaks
> needed in DMA ops to catch all possible incarnations of this problem,
> but this initial RFC is mostly about the impact beyond the dma-mapping
> subsystem itself.
Couldn't you set and use the device's parent's dma_mask instead. At
least for DT, we should always have a parent device representing the
bus. That would avoid further bloating of struct device.
Rob
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v12 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops
Hi Tomasz,
On 2018-07-11 14:51, Tomasz Figa wrote:
> On Wed, Jul 11, 2018 at 8:11 PM Rafael J. Wysocki wrote:
>> On Wed, Jul 11, 2018 at 12:55 PM, Vivek Gautam
>> wrote:
>>> On Wed, Jul 11, 2018 at 3:20 PM, Rafael J. Wysocki
>>> wrote:
On Sunday, July 8, 2018 7:34:10 PM CEST Vivek Gautam wrote:
> From: Sricharan R
>
> The smmu needs to be functional only when the respective
> master's using it are active. The device_link feature
> helps to track such functional dependencies, so that the
> iommu gets powered when the master device enables itself
> using pm_runtime. So by adapting the smmu driver for
> runtime pm, above said dependency can be addressed.
>
> This patch adds the pm runtime/sleep callbacks to the
> driver and also the functions to parse the smmu clocks
> from DT and enable them in resume/suspend.
>
> Signed-off-by: Sricharan R
> Signed-off-by: Archit Taneja
> [vivek: Clock rework to request bulk of clocks]
> Signed-off-by: Vivek Gautam
> Reviewed-by: Tomasz Figa
> ---
>
> - No change since v11.
>
> drivers/iommu/arm-smmu.c | 60
> ++--
> 1 file changed, 58 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index f7a96bcf94a6..a01d0dde21dd 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -48,6 +48,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -205,6 +206,8 @@ struct arm_smmu_device {
>u32 num_global_irqs;
>u32 num_context_irqs;
>unsigned int*irqs;
> + struct clk_bulk_data*clks;
> + int num_clks;
>
>u32 cavium_id_base; /* Specific to
> Cavium */
>
> @@ -1897,10 +1900,12 @@ static int arm_smmu_device_cfg_probe(struct
> arm_smmu_device *smmu)
> struct arm_smmu_match_data {
>enum arm_smmu_arch_version version;
>enum arm_smmu_implementation model;
> + const char * const *clks;
> + int num_clks;
> };
>
> #define ARM_SMMU_MATCH_DATA(name, ver, imp) \
> -static struct arm_smmu_match_data name = { .version = ver, .model = imp }
> +static const struct arm_smmu_match_data name = { .version = ver, .model
> = imp }
>
> ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
> ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
> @@ -1919,6 +1924,23 @@ static const struct of_device_id
> arm_smmu_of_match[] = {
> };
> MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
>
> +static void arm_smmu_fill_clk_data(struct arm_smmu_device *smmu,
> +const char * const *clks)
> +{
> + int i;
> +
> + if (smmu->num_clks < 1)
> + return;
> +
> + smmu->clks = devm_kcalloc(smmu->dev, smmu->num_clks,
> + sizeof(*smmu->clks), GFP_KERNEL);
> + if (!smmu->clks)
> + return;
> +
> + for (i = 0; i < smmu->num_clks; i++)
> + smmu->clks[i].id = clks[i];
> +}
> +
> #ifdef CONFIG_ACPI
> static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu)
> {
> @@ -2001,6 +2023,9 @@ static int arm_smmu_device_dt_probe(struct
> platform_device *pdev,
>data = of_device_get_match_data(dev);
>smmu->version = data->version;
>smmu->model = data->model;
> + smmu->num_clks = data->num_clks;
> +
> + arm_smmu_fill_clk_data(smmu, data->clks);
>
>parse_driver_options(smmu);
>
> @@ -2099,6 +2124,14 @@ static int arm_smmu_device_probe(struct
> platform_device *pdev)
>smmu->irqs[i] = irq;
>}
>
> + err = devm_clk_bulk_get(smmu->dev, smmu->num_clks, smmu->clks);
> + if (err)
> + return err;
> +
> + err = clk_bulk_prepare(smmu->num_clks, smmu->clks);
> + if (err)
> + return err;
> +
>err = arm_smmu_device_cfg_probe(smmu);
>if (err)
>return err;
> @@ -2181,6 +2214,9 @@ static int arm_smmu_device_remove(struct
> platform_device *pdev)
>
>/* Turn the thing off */
>writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
> +
> + clk_bulk_unprepare(smmu->num_clks, smmu->clks);
> +
>return 0;
> }
>
> @@ -2197,7 +2233,27 @@ static int __maybe_unused
> arm_smmu_pm_resume(struct device *dev)
>return 0;
> }
>
Re: [PATCH 12/25] MIPS: loongson: untangle dma implementations
On Wed, 11 Jul 2018, Christoph Hellwig wrote: > > SiByte should too though, at least for those boards, such as the SWARM > > and the BigSur, that can have DRAM over 4GiB (and 32-bit PCI devices > > plugged). > > Only in this case refers to loonson boards. Right! > > I never got to have the wiring of swiotlb completed for these boards as > > I got distracted with getting set up to debug a DRAM controller issue > > observed in the form of memory data corruption with the banks fully > > populated (which might have to do something with the parameters of bank > > interleaving enabled in such a configuration, as replacing a single > > module with a smaller-sized one and therefore disabling interleaving, > > which can only work with all modules being the same size, makes the > > problem go away). > > After this series enabling swiotlb for another board is trivial as all > the code has been consolidated. Just select SWIOTLB and add a call to > swiotlb_init to the board setup code. I had that feeling too, thanks for confirming. And for doing this work in the first place! Maciej ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 12/25] MIPS: loongson: untangle dma implementations
On Fri, 25 May 2018, Christoph Hellwig wrote: > Only loongson-3 is DMA coherent and uses swiotlb. So move the dma > address translations stubs directly to the loongson-3 code, and remove > a few Kconfig indirections. SiByte should too though, at least for those boards, such as the SWARM and the BigSur, that can have DRAM over 4GiB (and 32-bit PCI devices plugged). I never got to have the wiring of swiotlb completed for these boards as I got distracted with getting set up to debug a DRAM controller issue observed in the form of memory data corruption with the banks fully populated (which might have to do something with the parameters of bank interleaving enabled in such a configuration, as replacing a single module with a smaller-sized one and therefore disabling interleaving, which can only work with all modules being the same size, makes the problem go away). FWIW, Maciej ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v12 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops
On Wed, Jul 11, 2018 at 8:11 PM Rafael J. Wysocki wrote:
>
> On Wed, Jul 11, 2018 at 12:55 PM, Vivek Gautam
> wrote:
> > Hi Rafael,
> >
> >
> > On Wed, Jul 11, 2018 at 3:20 PM, Rafael J. Wysocki
> > wrote:
> >> On Sunday, July 8, 2018 7:34:10 PM CEST Vivek Gautam wrote:
> >>> From: Sricharan R
> >>>
> >>> The smmu needs to be functional only when the respective
> >>> master's using it are active. The device_link feature
> >>> helps to track such functional dependencies, so that the
> >>> iommu gets powered when the master device enables itself
> >>> using pm_runtime. So by adapting the smmu driver for
> >>> runtime pm, above said dependency can be addressed.
> >>>
> >>> This patch adds the pm runtime/sleep callbacks to the
> >>> driver and also the functions to parse the smmu clocks
> >>> from DT and enable them in resume/suspend.
> >>>
> >>> Signed-off-by: Sricharan R
> >>> Signed-off-by: Archit Taneja
> >>> [vivek: Clock rework to request bulk of clocks]
> >>> Signed-off-by: Vivek Gautam
> >>> Reviewed-by: Tomasz Figa
> >>> ---
> >>>
> >>> - No change since v11.
> >>>
> >>> drivers/iommu/arm-smmu.c | 60
> >>> ++--
> >>> 1 file changed, 58 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> >>> index f7a96bcf94a6..a01d0dde21dd 100644
> >>> --- a/drivers/iommu/arm-smmu.c
> >>> +++ b/drivers/iommu/arm-smmu.c
> >>> @@ -48,6 +48,7 @@
> >>> #include
> >>> #include
> >>> #include
> >>> +#include
> >>> #include
> >>> #include
> >>>
> >>> @@ -205,6 +206,8 @@ struct arm_smmu_device {
> >>> u32 num_global_irqs;
> >>> u32 num_context_irqs;
> >>> unsigned int*irqs;
> >>> + struct clk_bulk_data*clks;
> >>> + int num_clks;
> >>>
> >>> u32 cavium_id_base; /* Specific to
> >>> Cavium */
> >>>
> >>> @@ -1897,10 +1900,12 @@ static int arm_smmu_device_cfg_probe(struct
> >>> arm_smmu_device *smmu)
> >>> struct arm_smmu_match_data {
> >>> enum arm_smmu_arch_version version;
> >>> enum arm_smmu_implementation model;
> >>> + const char * const *clks;
> >>> + int num_clks;
> >>> };
> >>>
> >>> #define ARM_SMMU_MATCH_DATA(name, ver, imp) \
> >>> -static struct arm_smmu_match_data name = { .version = ver, .model = imp }
> >>> +static const struct arm_smmu_match_data name = { .version = ver, .model
> >>> = imp }
> >>>
> >>> ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
> >>> ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
> >>> @@ -1919,6 +1924,23 @@ static const struct of_device_id
> >>> arm_smmu_of_match[] = {
> >>> };
> >>> MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
> >>>
> >>> +static void arm_smmu_fill_clk_data(struct arm_smmu_device *smmu,
> >>> +const char * const *clks)
> >>> +{
> >>> + int i;
> >>> +
> >>> + if (smmu->num_clks < 1)
> >>> + return;
> >>> +
> >>> + smmu->clks = devm_kcalloc(smmu->dev, smmu->num_clks,
> >>> + sizeof(*smmu->clks), GFP_KERNEL);
> >>> + if (!smmu->clks)
> >>> + return;
> >>> +
> >>> + for (i = 0; i < smmu->num_clks; i++)
> >>> + smmu->clks[i].id = clks[i];
> >>> +}
> >>> +
> >>> #ifdef CONFIG_ACPI
> >>> static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu)
> >>> {
> >>> @@ -2001,6 +2023,9 @@ static int arm_smmu_device_dt_probe(struct
> >>> platform_device *pdev,
> >>> data = of_device_get_match_data(dev);
> >>> smmu->version = data->version;
> >>> smmu->model = data->model;
> >>> + smmu->num_clks = data->num_clks;
> >>> +
> >>> + arm_smmu_fill_clk_data(smmu, data->clks);
> >>>
> >>> parse_driver_options(smmu);
> >>>
> >>> @@ -2099,6 +2124,14 @@ static int arm_smmu_device_probe(struct
> >>> platform_device *pdev)
> >>> smmu->irqs[i] = irq;
> >>> }
> >>>
> >>> + err = devm_clk_bulk_get(smmu->dev, smmu->num_clks, smmu->clks);
> >>> + if (err)
> >>> + return err;
> >>> +
> >>> + err = clk_bulk_prepare(smmu->num_clks, smmu->clks);
> >>> + if (err)
> >>> + return err;
> >>> +
> >>> err = arm_smmu_device_cfg_probe(smmu);
> >>> if (err)
> >>> return err;
> >>> @@ -2181,6 +2214,9 @@ static int arm_smmu_device_remove(struct
> >>> platform_device *pdev)
> >>>
> >>> /* Turn the thing off */
> >>> writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
> >>> +
> >>> + clk_bulk_unprepare(smmu->num_clks, smmu->clks);
> >>> +
> >>> return 0;
> >>> }
> >>>
> >>> @@ -2197,7 +2233,27 @@ static int __maybe_unused
> >>> arm_smmu_pm_resume(struct device *dev)
> >>> return 0;
> >>> }
> >>>
> >>> -static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL,
Re: [PATCH 12/25] MIPS: loongson: untangle dma implementations
On Wed, Jul 11, 2018 at 01:46:31PM +0100, Maciej W. Rozycki wrote: > > Only loongson-3 is DMA coherent and uses swiotlb. So move the dma > > address translations stubs directly to the loongson-3 code, and remove > > a few Kconfig indirections. > > SiByte should too though, at least for those boards, such as the SWARM > and the BigSur, that can have DRAM over 4GiB (and 32-bit PCI devices > plugged). Only in this case refers to loonson boards. > I never got to have the wiring of swiotlb completed for these boards as > I got distracted with getting set up to debug a DRAM controller issue > observed in the form of memory data corruption with the banks fully > populated (which might have to do something with the parameters of bank > interleaving enabled in such a configuration, as replacing a single > module with a smaller-sized one and therefore disabling interleaving, > which can only work with all modules being the same size, makes the > problem go away). After this series enabling swiotlb for another board is trivial as all the code has been consolidated. Just select SWIOTLB and add a call to swiotlb_init to the board setup code. > > FWIW, > > Maciej ---end quoted text--- ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v12 2/4] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
On 7/11/2018 4:29 PM, Rafael J. Wysocki wrote:
On Wed, Jul 11, 2018 at 12:05 PM, Tomasz Figa wrote:
Hi Rafael,
Thanks for review.
On Wed, Jul 11, 2018 at 6:53 PM Rafael J. Wysocki wrote:
On Sunday, July 8, 2018 7:34:11 PM CEST Vivek Gautam wrote:
From: Sricharan R
The smmu device probe/remove and add/remove master device callbacks
gets called when the smmu is not linked to its master, that is without
the context of the master device. So calling runtime apis in those places
separately.
Signed-off-by: Sricharan R
[vivek: Cleanup pm runtime calls]
Signed-off-by: Vivek Gautam
Reviewed-by: Tomasz Figa
---
- Change since v11
* Replaced pm_runtime_disable() with pm_runtime_force_suspend()
to avoid warning about " Unpreparing enabled clock".
Full warning text mentioned in cover patch.
drivers/iommu/arm-smmu.c | 92 +++-
1 file changed, 84 insertions(+), 8 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index a01d0dde21dd..09265e206e2d 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -268,6 +268,20 @@ static struct arm_smmu_option_prop arm_smmu_options[] = {
{ 0, NULL},
};
+static inline int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
+{
+ if (pm_runtime_enabled(smmu->dev))
Why do you need the pm_runtime_enabled() checks here and below?
pm_runtime_get_sync() and pm_runtime_put() should work just fine if
runtime PM is not enabled.
Because pm_runtime_get_sync() acquires a spin lock, even if only for
the short time of checking if runtime PM is enabled and SMMU driver
maintainers didn't want any spin locks in certain IOMMU API code paths
on hardware implementations that don't need runtime PM, while we still
need to be able to control runtime PM there on hardware
implementations that need so.
OK, so it is an optimization. It would be good to put a comment in
there to that effect.
Yea, actually there's a comment placed in arm_smmu_device_probe()
where the runtime PM is conditionally enabled.
I can add comments for these wrappers too if you would like.
Thanks & Regards
Vivek
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v12 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops
On Wed, Jul 11, 2018 at 12:55 PM, Vivek Gautam
wrote:
> Hi Rafael,
>
>
> On Wed, Jul 11, 2018 at 3:20 PM, Rafael J. Wysocki wrote:
>> On Sunday, July 8, 2018 7:34:10 PM CEST Vivek Gautam wrote:
>>> From: Sricharan R
>>>
>>> The smmu needs to be functional only when the respective
>>> master's using it are active. The device_link feature
>>> helps to track such functional dependencies, so that the
>>> iommu gets powered when the master device enables itself
>>> using pm_runtime. So by adapting the smmu driver for
>>> runtime pm, above said dependency can be addressed.
>>>
>>> This patch adds the pm runtime/sleep callbacks to the
>>> driver and also the functions to parse the smmu clocks
>>> from DT and enable them in resume/suspend.
>>>
>>> Signed-off-by: Sricharan R
>>> Signed-off-by: Archit Taneja
>>> [vivek: Clock rework to request bulk of clocks]
>>> Signed-off-by: Vivek Gautam
>>> Reviewed-by: Tomasz Figa
>>> ---
>>>
>>> - No change since v11.
>>>
>>> drivers/iommu/arm-smmu.c | 60
>>> ++--
>>> 1 file changed, 58 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>>> index f7a96bcf94a6..a01d0dde21dd 100644
>>> --- a/drivers/iommu/arm-smmu.c
>>> +++ b/drivers/iommu/arm-smmu.c
>>> @@ -48,6 +48,7 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>> #include
>>> #include
>>>
>>> @@ -205,6 +206,8 @@ struct arm_smmu_device {
>>> u32 num_global_irqs;
>>> u32 num_context_irqs;
>>> unsigned int*irqs;
>>> + struct clk_bulk_data*clks;
>>> + int num_clks;
>>>
>>> u32 cavium_id_base; /* Specific to Cavium
>>> */
>>>
>>> @@ -1897,10 +1900,12 @@ static int arm_smmu_device_cfg_probe(struct
>>> arm_smmu_device *smmu)
>>> struct arm_smmu_match_data {
>>> enum arm_smmu_arch_version version;
>>> enum arm_smmu_implementation model;
>>> + const char * const *clks;
>>> + int num_clks;
>>> };
>>>
>>> #define ARM_SMMU_MATCH_DATA(name, ver, imp) \
>>> -static struct arm_smmu_match_data name = { .version = ver, .model = imp }
>>> +static const struct arm_smmu_match_data name = { .version = ver, .model =
>>> imp }
>>>
>>> ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
>>> ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
>>> @@ -1919,6 +1924,23 @@ static const struct of_device_id arm_smmu_of_match[]
>>> = {
>>> };
>>> MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
>>>
>>> +static void arm_smmu_fill_clk_data(struct arm_smmu_device *smmu,
>>> +const char * const *clks)
>>> +{
>>> + int i;
>>> +
>>> + if (smmu->num_clks < 1)
>>> + return;
>>> +
>>> + smmu->clks = devm_kcalloc(smmu->dev, smmu->num_clks,
>>> + sizeof(*smmu->clks), GFP_KERNEL);
>>> + if (!smmu->clks)
>>> + return;
>>> +
>>> + for (i = 0; i < smmu->num_clks; i++)
>>> + smmu->clks[i].id = clks[i];
>>> +}
>>> +
>>> #ifdef CONFIG_ACPI
>>> static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu)
>>> {
>>> @@ -2001,6 +2023,9 @@ static int arm_smmu_device_dt_probe(struct
>>> platform_device *pdev,
>>> data = of_device_get_match_data(dev);
>>> smmu->version = data->version;
>>> smmu->model = data->model;
>>> + smmu->num_clks = data->num_clks;
>>> +
>>> + arm_smmu_fill_clk_data(smmu, data->clks);
>>>
>>> parse_driver_options(smmu);
>>>
>>> @@ -2099,6 +2124,14 @@ static int arm_smmu_device_probe(struct
>>> platform_device *pdev)
>>> smmu->irqs[i] = irq;
>>> }
>>>
>>> + err = devm_clk_bulk_get(smmu->dev, smmu->num_clks, smmu->clks);
>>> + if (err)
>>> + return err;
>>> +
>>> + err = clk_bulk_prepare(smmu->num_clks, smmu->clks);
>>> + if (err)
>>> + return err;
>>> +
>>> err = arm_smmu_device_cfg_probe(smmu);
>>> if (err)
>>> return err;
>>> @@ -2181,6 +2214,9 @@ static int arm_smmu_device_remove(struct
>>> platform_device *pdev)
>>>
>>> /* Turn the thing off */
>>> writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
>>> +
>>> + clk_bulk_unprepare(smmu->num_clks, smmu->clks);
>>> +
>>> return 0;
>>> }
>>>
>>> @@ -2197,7 +2233,27 @@ static int __maybe_unused arm_smmu_pm_resume(struct
>>> device *dev)
>>> return 0;
>>> }
>>>
>>> -static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL, arm_smmu_pm_resume);
>>> +static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
>>> +{
>>> + struct arm_smmu_device *smmu = dev_get_drvdata(dev);
>>> +
>>> + return clk_bulk_enable(smmu->num_clks, smmu->clks);
>>> +}
>>> +
>>> +static int __maybe_unused arm_smmu_runtime_suspend(struct device *dev)
>>> +{
>>> + struct
Re: [PATCH v12 2/4] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
On Wed, Jul 11, 2018 at 12:05 PM, Tomasz Figa wrote:
> Hi Rafael,
>
> Thanks for review.
>
> On Wed, Jul 11, 2018 at 6:53 PM Rafael J. Wysocki wrote:
>>
>> On Sunday, July 8, 2018 7:34:11 PM CEST Vivek Gautam wrote:
>> > From: Sricharan R
>> >
>> > The smmu device probe/remove and add/remove master device callbacks
>> > gets called when the smmu is not linked to its master, that is without
>> > the context of the master device. So calling runtime apis in those places
>> > separately.
>> >
>> > Signed-off-by: Sricharan R
>> > [vivek: Cleanup pm runtime calls]
>> > Signed-off-by: Vivek Gautam
>> > Reviewed-by: Tomasz Figa
>> > ---
>> >
>> > - Change since v11
>> >* Replaced pm_runtime_disable() with pm_runtime_force_suspend()
>> > to avoid warning about " Unpreparing enabled clock".
>> > Full warning text mentioned in cover patch.
>> >
>> > drivers/iommu/arm-smmu.c | 92
>> > +++-
>> > 1 file changed, 84 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> > index a01d0dde21dd..09265e206e2d 100644
>> > --- a/drivers/iommu/arm-smmu.c
>> > +++ b/drivers/iommu/arm-smmu.c
>> > @@ -268,6 +268,20 @@ static struct arm_smmu_option_prop arm_smmu_options[]
>> > = {
>> > { 0, NULL},
>> > };
>> >
>> > +static inline int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
>> > +{
>> > + if (pm_runtime_enabled(smmu->dev))
>>
>> Why do you need the pm_runtime_enabled() checks here and below?
>>
>> pm_runtime_get_sync() and pm_runtime_put() should work just fine if
>> runtime PM is not enabled.
>
> Because pm_runtime_get_sync() acquires a spin lock, even if only for
> the short time of checking if runtime PM is enabled and SMMU driver
> maintainers didn't want any spin locks in certain IOMMU API code paths
> on hardware implementations that don't need runtime PM, while we still
> need to be able to control runtime PM there on hardware
> implementations that need so.
OK, so it is an optimization. It would be good to put a comment in
there to that effect.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v12 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops
Hi Rafael,
On Wed, Jul 11, 2018 at 3:20 PM, Rafael J. Wysocki wrote:
> On Sunday, July 8, 2018 7:34:10 PM CEST Vivek Gautam wrote:
>> From: Sricharan R
>>
>> The smmu needs to be functional only when the respective
>> master's using it are active. The device_link feature
>> helps to track such functional dependencies, so that the
>> iommu gets powered when the master device enables itself
>> using pm_runtime. So by adapting the smmu driver for
>> runtime pm, above said dependency can be addressed.
>>
>> This patch adds the pm runtime/sleep callbacks to the
>> driver and also the functions to parse the smmu clocks
>> from DT and enable them in resume/suspend.
>>
>> Signed-off-by: Sricharan R
>> Signed-off-by: Archit Taneja
>> [vivek: Clock rework to request bulk of clocks]
>> Signed-off-by: Vivek Gautam
>> Reviewed-by: Tomasz Figa
>> ---
>>
>> - No change since v11.
>>
>> drivers/iommu/arm-smmu.c | 60
>> ++--
>> 1 file changed, 58 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index f7a96bcf94a6..a01d0dde21dd 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -48,6 +48,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>>
>> @@ -205,6 +206,8 @@ struct arm_smmu_device {
>> u32 num_global_irqs;
>> u32 num_context_irqs;
>> unsigned int*irqs;
>> + struct clk_bulk_data*clks;
>> + int num_clks;
>>
>> u32 cavium_id_base; /* Specific to Cavium
>> */
>>
>> @@ -1897,10 +1900,12 @@ static int arm_smmu_device_cfg_probe(struct
>> arm_smmu_device *smmu)
>> struct arm_smmu_match_data {
>> enum arm_smmu_arch_version version;
>> enum arm_smmu_implementation model;
>> + const char * const *clks;
>> + int num_clks;
>> };
>>
>> #define ARM_SMMU_MATCH_DATA(name, ver, imp) \
>> -static struct arm_smmu_match_data name = { .version = ver, .model = imp }
>> +static const struct arm_smmu_match_data name = { .version = ver, .model =
>> imp }
>>
>> ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
>> ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
>> @@ -1919,6 +1924,23 @@ static const struct of_device_id arm_smmu_of_match[]
>> = {
>> };
>> MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
>>
>> +static void arm_smmu_fill_clk_data(struct arm_smmu_device *smmu,
>> +const char * const *clks)
>> +{
>> + int i;
>> +
>> + if (smmu->num_clks < 1)
>> + return;
>> +
>> + smmu->clks = devm_kcalloc(smmu->dev, smmu->num_clks,
>> + sizeof(*smmu->clks), GFP_KERNEL);
>> + if (!smmu->clks)
>> + return;
>> +
>> + for (i = 0; i < smmu->num_clks; i++)
>> + smmu->clks[i].id = clks[i];
>> +}
>> +
>> #ifdef CONFIG_ACPI
>> static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu)
>> {
>> @@ -2001,6 +2023,9 @@ static int arm_smmu_device_dt_probe(struct
>> platform_device *pdev,
>> data = of_device_get_match_data(dev);
>> smmu->version = data->version;
>> smmu->model = data->model;
>> + smmu->num_clks = data->num_clks;
>> +
>> + arm_smmu_fill_clk_data(smmu, data->clks);
>>
>> parse_driver_options(smmu);
>>
>> @@ -2099,6 +2124,14 @@ static int arm_smmu_device_probe(struct
>> platform_device *pdev)
>> smmu->irqs[i] = irq;
>> }
>>
>> + err = devm_clk_bulk_get(smmu->dev, smmu->num_clks, smmu->clks);
>> + if (err)
>> + return err;
>> +
>> + err = clk_bulk_prepare(smmu->num_clks, smmu->clks);
>> + if (err)
>> + return err;
>> +
>> err = arm_smmu_device_cfg_probe(smmu);
>> if (err)
>> return err;
>> @@ -2181,6 +2214,9 @@ static int arm_smmu_device_remove(struct
>> platform_device *pdev)
>>
>> /* Turn the thing off */
>> writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
>> +
>> + clk_bulk_unprepare(smmu->num_clks, smmu->clks);
>> +
>> return 0;
>> }
>>
>> @@ -2197,7 +2233,27 @@ static int __maybe_unused arm_smmu_pm_resume(struct
>> device *dev)
>> return 0;
>> }
>>
>> -static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL, arm_smmu_pm_resume);
>> +static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
>> +{
>> + struct arm_smmu_device *smmu = dev_get_drvdata(dev);
>> +
>> + return clk_bulk_enable(smmu->num_clks, smmu->clks);
>> +}
>> +
>> +static int __maybe_unused arm_smmu_runtime_suspend(struct device *dev)
>> +{
>> + struct arm_smmu_device *smmu = dev_get_drvdata(dev);
>> +
>> + clk_bulk_disable(smmu->num_clks, smmu->clks);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct dev_pm_ops arm_smmu_pm_ops = {
>> +
Re: [PATCH v12 3/4] iommu/arm-smmu: Add the device_link between masters and smmu
Hi Rafael,
On 7/11/2018 3:23 PM, Rafael J. Wysocki wrote:
On Sunday, July 8, 2018 7:34:12 PM CEST Vivek Gautam wrote:
From: Sricharan R
Finally add the device link between the master device and
smmu, so that the smmu gets runtime enabled/disabled only when the
master needs it. This is done from add_device callback which gets
called once when the master is added to the smmu.
Signed-off-by: Sricharan R
Signed-off-by: Vivek Gautam
Reviewed-by: Tomasz Figa
Cc: Rafael J. Wysocki
Cc: Lukas Wunner
---
- Change since v11
* Replaced DL_FLAG_AUTOREMOVE flag with DL_FLAG_AUTOREMOVE_SUPPLIER.
drivers/iommu/arm-smmu.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 09265e206e2d..916cde4954d2 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1461,8 +1461,20 @@ static int arm_smmu_add_device(struct device *dev)
iommu_device_link(>iommu, dev);
+ if (pm_runtime_enabled(smmu->dev) &&
Why does the creation of the link depend on whether or not runtime PM
is enabled for the MMU device?
The main purpose of this device link is to handle the runtime PM
synchronization
between the supplier (iommu) and consumer (client devices, such as
GPU/display).
Moreover, the runtime pm is conditionally enabled for smmu devices that
support
such [1].
What about system-wide PM and system shutdown? Are they always guaranteed
to happen in the right order without the link?
When there's no runtime PM, there's no clocks, and other resources to be
handled.
So, we don't need device link for system-wide PM and system shutdown to
work correctly.
That's the case with current arm-smmu driver.
Is it something that i am missing here?
[1] https://lkml.org/lkml/2018/3/8/775
Thanks
Vivek
+ !device_link_add(dev, smmu->dev,
+ DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOREMOVE_SUPPLIER)) {
+ dev_err(smmu->dev, "Unable to add link to the consumer %s\n",
+ dev_name(dev));
+ ret = -ENODEV;
+ goto out_unlink;
+ }
+
return 0;
+out_unlink:
+ iommu_device_unlink(>iommu, dev);
+ arm_smmu_master_free_smes(fwspec);
out_cfg_free:
kfree(cfg);
out_free:
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v12 2/4] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
Hi Rafael,
Thanks for review.
On Wed, Jul 11, 2018 at 6:53 PM Rafael J. Wysocki wrote:
>
> On Sunday, July 8, 2018 7:34:11 PM CEST Vivek Gautam wrote:
> > From: Sricharan R
> >
> > The smmu device probe/remove and add/remove master device callbacks
> > gets called when the smmu is not linked to its master, that is without
> > the context of the master device. So calling runtime apis in those places
> > separately.
> >
> > Signed-off-by: Sricharan R
> > [vivek: Cleanup pm runtime calls]
> > Signed-off-by: Vivek Gautam
> > Reviewed-by: Tomasz Figa
> > ---
> >
> > - Change since v11
> >* Replaced pm_runtime_disable() with pm_runtime_force_suspend()
> > to avoid warning about " Unpreparing enabled clock".
> > Full warning text mentioned in cover patch.
> >
> > drivers/iommu/arm-smmu.c | 92
> > +++-
> > 1 file changed, 84 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> > index a01d0dde21dd..09265e206e2d 100644
> > --- a/drivers/iommu/arm-smmu.c
> > +++ b/drivers/iommu/arm-smmu.c
> > @@ -268,6 +268,20 @@ static struct arm_smmu_option_prop arm_smmu_options[]
> > = {
> > { 0, NULL},
> > };
> >
> > +static inline int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
> > +{
> > + if (pm_runtime_enabled(smmu->dev))
>
> Why do you need the pm_runtime_enabled() checks here and below?
>
> pm_runtime_get_sync() and pm_runtime_put() should work just fine if
> runtime PM is not enabled.
Because pm_runtime_get_sync() acquires a spin lock, even if only for
the short time of checking if runtime PM is enabled and SMMU driver
maintainers didn't want any spin locks in certain IOMMU API code paths
on hardware implementations that don't need runtime PM, while we still
need to be able to control runtime PM there on hardware
implementations that need so.
Best regards,
Tomasz
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v12 3/4] iommu/arm-smmu: Add the device_link between masters and smmu
On Sunday, July 8, 2018 7:34:12 PM CEST Vivek Gautam wrote:
> From: Sricharan R
>
> Finally add the device link between the master device and
> smmu, so that the smmu gets runtime enabled/disabled only when the
> master needs it. This is done from add_device callback which gets
> called once when the master is added to the smmu.
>
> Signed-off-by: Sricharan R
> Signed-off-by: Vivek Gautam
> Reviewed-by: Tomasz Figa
> Cc: Rafael J. Wysocki
> Cc: Lukas Wunner
> ---
>
> - Change since v11
>* Replaced DL_FLAG_AUTOREMOVE flag with DL_FLAG_AUTOREMOVE_SUPPLIER.
>
> drivers/iommu/arm-smmu.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 09265e206e2d..916cde4954d2 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -1461,8 +1461,20 @@ static int arm_smmu_add_device(struct device *dev)
>
> iommu_device_link(>iommu, dev);
>
> + if (pm_runtime_enabled(smmu->dev) &&
Why does the creation of the link depend on whether or not runtime PM
is enabled for the MMU device?
What about system-wide PM and system shutdown? Are they always guaranteed
to happen in the right order without the link?
> + !device_link_add(dev, smmu->dev,
> + DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOREMOVE_SUPPLIER)) {
> + dev_err(smmu->dev, "Unable to add link to the consumer %s\n",
> + dev_name(dev));
> + ret = -ENODEV;
> + goto out_unlink;
> + }
> +
> return 0;
>
> +out_unlink:
> + iommu_device_unlink(>iommu, dev);
> + arm_smmu_master_free_smes(fwspec);
> out_cfg_free:
> kfree(cfg);
> out_free:
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v12 2/4] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
On Sunday, July 8, 2018 7:34:11 PM CEST Vivek Gautam wrote:
> From: Sricharan R
>
> The smmu device probe/remove and add/remove master device callbacks
> gets called when the smmu is not linked to its master, that is without
> the context of the master device. So calling runtime apis in those places
> separately.
>
> Signed-off-by: Sricharan R
> [vivek: Cleanup pm runtime calls]
> Signed-off-by: Vivek Gautam
> Reviewed-by: Tomasz Figa
> ---
>
> - Change since v11
>* Replaced pm_runtime_disable() with pm_runtime_force_suspend()
> to avoid warning about " Unpreparing enabled clock".
> Full warning text mentioned in cover patch.
>
> drivers/iommu/arm-smmu.c | 92
> +++-
> 1 file changed, 84 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index a01d0dde21dd..09265e206e2d 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -268,6 +268,20 @@ static struct arm_smmu_option_prop arm_smmu_options[] = {
> { 0, NULL},
> };
>
> +static inline int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
> +{
> + if (pm_runtime_enabled(smmu->dev))
Why do you need the pm_runtime_enabled() checks here and below?
pm_runtime_get_sync() and pm_runtime_put() should work just fine if
runtime PM is not enabled.
> + return pm_runtime_get_sync(smmu->dev);
> +
> + return 0;
> +}
> +
> +static inline void arm_smmu_rpm_put(struct arm_smmu_device *smmu)
> +{
> + if (pm_runtime_enabled(smmu->dev))
> + pm_runtime_put(smmu->dev);
> +}
> +
> static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
> {
> return container_of(dom, struct arm_smmu_domain, domain);
> @@ -913,11 +927,15 @@ static void arm_smmu_destroy_domain_context(struct
> iommu_domain *domain)
> struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> struct arm_smmu_cfg *cfg = _domain->cfg;
> - int irq;
> + int ret, irq;
>
> if (!smmu || domain->type == IOMMU_DOMAIN_IDENTITY)
> return;
>
> + ret = arm_smmu_rpm_get(smmu);
> + if (ret < 0)
> + return;
> +
> /*
>* Disable the context bank and free the page tables before freeing
>* it.
> @@ -932,6 +950,8 @@ static void arm_smmu_destroy_domain_context(struct
> iommu_domain *domain)
>
> free_io_pgtable_ops(smmu_domain->pgtbl_ops);
> __arm_smmu_free_bitmap(smmu->context_map, cfg->cbndx);
> +
> + arm_smmu_rpm_put(smmu);
> }
>
> static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
> @@ -1213,10 +1233,15 @@ static int arm_smmu_attach_dev(struct iommu_domain
> *domain, struct device *dev)
> return -ENODEV;
>
> smmu = fwspec_smmu(fwspec);
> +
> + ret = arm_smmu_rpm_get(smmu);
> + if (ret < 0)
> + return ret;
> +
> /* Ensure that the domain is finalised */
> ret = arm_smmu_init_domain_context(domain, smmu);
> if (ret < 0)
> - return ret;
> + goto rpm_put;
>
> /*
>* Sanity check the domain. We don't support domains across
> @@ -1226,33 +1251,50 @@ static int arm_smmu_attach_dev(struct iommu_domain
> *domain, struct device *dev)
> dev_err(dev,
> "cannot attach to SMMU %s whilst already attached to
> domain on SMMU %s\n",
> dev_name(smmu_domain->smmu->dev), dev_name(smmu->dev));
> - return -EINVAL;
> + ret = -EINVAL;
> + goto rpm_put;
> }
>
> /* Looks ok, so add the device to the domain */
> - return arm_smmu_domain_add_master(smmu_domain, fwspec);
> + ret = arm_smmu_domain_add_master(smmu_domain, fwspec);
> +
> +rpm_put:
> + arm_smmu_rpm_put(smmu);
> + return ret;
> }
>
> static int arm_smmu_map(struct iommu_domain *domain, unsigned long iova,
> phys_addr_t paddr, size_t size, int prot)
> {
> struct io_pgtable_ops *ops = to_smmu_domain(domain)->pgtbl_ops;
> + struct arm_smmu_device *smmu = to_smmu_domain(domain)->smmu;
> + int ret;
>
> if (!ops)
> return -ENODEV;
>
> - return ops->map(ops, iova, paddr, size, prot);
> + arm_smmu_rpm_get(smmu);
> + ret = ops->map(ops, iova, paddr, size, prot);
> + arm_smmu_rpm_put(smmu);
> +
> + return ret;
> }
>
> static size_t arm_smmu_unmap(struct iommu_domain *domain, unsigned long iova,
>size_t size)
> {
> struct io_pgtable_ops *ops = to_smmu_domain(domain)->pgtbl_ops;
> + struct arm_smmu_device *smmu = to_smmu_domain(domain)->smmu;
> + size_t ret;
>
> if (!ops)
> return 0;
>
> - return ops->unmap(ops, iova, size);
> + arm_smmu_rpm_get(smmu);
> + ret = ops->unmap(ops, iova, size);
> + arm_smmu_rpm_put(smmu);
> +
> +
Re: [PATCH 1/2] mm/cma: remove unsupported gfp_mask parameter from cma_alloc()
On Wed 11-07-18 16:35:28, Joonsoo Kim wrote:
> 2018-07-10 18:50 GMT+09:00 Michal Hocko :
> > On Tue 10-07-18 16:19:32, Joonsoo Kim wrote:
> >> Hello, Marek.
> >>
> >> 2018-07-09 21:19 GMT+09:00 Marek Szyprowski :
> >> > cma_alloc() function doesn't really support gfp flags other than
> >> > __GFP_NOWARN, so convert gfp_mask parameter to boolean no_warn parameter.
> >>
> >> Although gfp_mask isn't used in cma_alloc() except no_warn, it can be used
> >> in alloc_contig_range(). For example, if passed gfp mask has no __GFP_FS,
> >> compaction(isolation) would work differently. Do you have considered
> >> such a case?
> >
> > Does any of cma_alloc users actually care about GFP_NO{FS,IO}?
>
> I don't know. My guess is that cma_alloc() is used for DMA allocation so
> block device would use it, too. If fs/block subsystem initiates the
> request for the device,
> it would be possible that cma_alloc() is called with such a flag.
> Again, I don't know
> much about those subsystem so I would be wrong.
The patch converts existing users and none of them really tries to use
anything other than GFP_KERNEL [|__GFP_NOWARN] so this doesn't seem to
be the case. Should there be a new user requiring more restricted
gfp_mask we should carefuly re-evaluate and think how to support it.
Until then I would simply stick with the proposed approach because my
experience tells me that a wrong gfp mask usage is way too easy so the
simpler the api is the less likely we will see an abuse.
--
Michal Hocko
SUSE Labs
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 6/9] iommu/vt-d: Per PCI device pasid table interfaces
On Wed, Jul 11, 2018 at 03:26:21PM +0800, Lu Baolu wrote:
[...]
> >> +int intel_pasid_alloc_table(struct device *dev)
> >> +{
> >> + struct device_domain_info *info;
> >> + struct pasid_table *pasid_table;
> >> + struct pasid_table_opaque data;
> >> + struct page *pages;
> >> + size_t size, count;
> >> + int ret, order;
> >> +
> >> + info = dev->archdata.iommu;
> >> + if (WARN_ON(!info || !dev_is_pci(dev) ||
> >> + !info->pasid_supported || info->pasid_table))
> >> + return -EINVAL;
> >> +
> >> + /* DMA alias device already has a pasid table, use it: */
> >> + data.pasid_table = _table;
> >> + ret = pci_for_each_dma_alias(to_pci_dev(dev),
> >> + _alias_pasid_table, );
> >> + if (ret)
> >> + goto attach_out;
> >> +
> >> + pasid_table = kzalloc(sizeof(*pasid_table), GFP_ATOMIC);
> > Do we need to take some lock here (e.g., the pasid lock)? Otherwise
> > what if two devices (that are sharing the same DMA alias) call the
> > function intel_pasid_alloc_table() concurrently, then could it
> > possible that we create one table for each of the device while AFAIU
> > we should let them share a single pasid table?
>
> The only place where this function is called is in a single-thread context
> (protected by a spinlock of device_domain_lock with local interrupt disabled).
>
> So we don't need an extra lock here. But anyway, I should put a comment
> here.
Yeah, that would be nice too! Or add a comment for both of the
functions:
/* Must be with device_domain_lock held */
Regards,
--
Peter Xu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] mm/cma: remove unsupported gfp_mask parameter from cma_alloc()
2018-07-10 18:50 GMT+09:00 Michal Hocko :
> On Tue 10-07-18 16:19:32, Joonsoo Kim wrote:
>> Hello, Marek.
>>
>> 2018-07-09 21:19 GMT+09:00 Marek Szyprowski :
>> > cma_alloc() function doesn't really support gfp flags other than
>> > __GFP_NOWARN, so convert gfp_mask parameter to boolean no_warn parameter.
>>
>> Although gfp_mask isn't used in cma_alloc() except no_warn, it can be used
>> in alloc_contig_range(). For example, if passed gfp mask has no __GFP_FS,
>> compaction(isolation) would work differently. Do you have considered
>> such a case?
>
> Does any of cma_alloc users actually care about GFP_NO{FS,IO}?
I don't know. My guess is that cma_alloc() is used for DMA allocation so
block device would use it, too. If fs/block subsystem initiates the
request for the device,
it would be possible that cma_alloc() is called with such a flag.
Again, I don't know
much about those subsystem so I would be wrong.
Thanks.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 6/9] iommu/vt-d: Per PCI device pasid table interfaces
Hi,
On 07/11/2018 10:18 AM, Peter Xu wrote:
> On Mon, Jul 09, 2018 at 01:22:55PM +0800, Lu Baolu wrote:
>> This patch adds the interfaces for per PCI device pasid
>> table management. Currently we allocate one pasid table
>> for all PCI devices under the scope of an IOMMU. It's
>> insecure in some cases where multiple devices under one
>> single IOMMU unit support PASID features. With per PCI
>> device pasid table, we can achieve finer protection and
>> isolation granularity.
>>
>> Cc: Ashok Raj
>> Cc: Jacob Pan
>> Cc: Kevin Tian
>> Cc: Liu Yi L
>> Suggested-by: Ashok Raj
>> Signed-off-by: Lu Baolu
>> Reviewed-by: Liu Yi L
>> ---
>> drivers/iommu/intel-iommu.c | 1 +
>> drivers/iommu/intel-pasid.c | 178
>>
>> drivers/iommu/intel-pasid.h | 18 +
>> drivers/iommu/intel-svm.c | 4 -
>> include/linux/intel-iommu.h | 2 +
>> 5 files changed, 199 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
>> index 3d802c5..a930918 100644
>> --- a/drivers/iommu/intel-iommu.c
>> +++ b/drivers/iommu/intel-iommu.c
>> @@ -2450,6 +2450,7 @@ static struct dmar_domain
>> *dmar_insert_one_dev_info(struct intel_iommu *iommu,
>> info->dev = dev;
>> info->domain = domain;
>> info->iommu = iommu;
>> +info->pasid_table = NULL;
>>
>> if (dev && dev_is_pci(dev)) {
>> struct pci_dev *pdev = to_pci_dev(info->dev);
>> diff --git a/drivers/iommu/intel-pasid.c b/drivers/iommu/intel-pasid.c
>> index e918fe0..1b61942 100644
>> --- a/drivers/iommu/intel-pasid.c
>> +++ b/drivers/iommu/intel-pasid.c
>> @@ -13,6 +13,8 @@
>> #include
>> #include
>> #include
>> +#include
>> +#include
>> #include
>>
>> #include "intel-pasid.h"
>> @@ -58,3 +60,179 @@ void *intel_pasid_lookup_id(int pasid)
>>
>> return p;
>> }
>> +
>> +/*
>> + * Per device pasid table management:
>> + */
>> +static inline void
>> +device_attach_pasid_table(struct device_domain_info *info,
>> + struct pasid_table *pasid_table)
>> +{
>> +info->pasid_table = pasid_table;
>> +list_add(>table, _table->dev);
>> +}
>> +
>> +static inline void
>> +device_detach_pasid_table(struct device_domain_info *info,
>> + struct pasid_table *pasid_table)
>> +{
>> +info->pasid_table = NULL;
>> +list_del(>table);
>> +}
>> +
>> +struct pasid_table_opaque {
>> +struct pasid_table **pasid_table;
>> +int segment;
>> +int bus;
>> +int devfn;
>> +};
>> +
>> +static int search_pasid_table(struct device_domain_info *info, void *opaque)
>> +{
>> +struct pasid_table_opaque *data = opaque;
>> +
>> +if (info->iommu->segment == data->segment &&
>> +info->bus == data->bus &&
>> +info->devfn == data->devfn &&
>> +info->pasid_table) {
>> +*data->pasid_table = info->pasid_table;
>> +return 1;
>> +}
>> +
>> +return 0;
>> +}
>> +
>> +static int get_alias_pasid_table(struct pci_dev *pdev, u16 alias, void
>> *opaque)
>> +{
>> +struct pasid_table_opaque *data = opaque;
>> +
>> +data->segment = pci_domain_nr(pdev->bus);
>> +data->bus = PCI_BUS_NUM(alias);
>> +data->devfn = alias & 0xff;
>> +
>> +return for_each_device_domain(_pasid_table, data);
>> +}
>> +
>> +int intel_pasid_alloc_table(struct device *dev)
>> +{
>> +struct device_domain_info *info;
>> +struct pasid_table *pasid_table;
>> +struct pasid_table_opaque data;
>> +struct page *pages;
>> +size_t size, count;
>> +int ret, order;
>> +
>> +info = dev->archdata.iommu;
>> +if (WARN_ON(!info || !dev_is_pci(dev) ||
>> +!info->pasid_supported || info->pasid_table))
>> +return -EINVAL;
>> +
>> +/* DMA alias device already has a pasid table, use it: */
>> +data.pasid_table = _table;
>> +ret = pci_for_each_dma_alias(to_pci_dev(dev),
>> + _alias_pasid_table, );
>> +if (ret)
>> +goto attach_out;
>> +
>> +pasid_table = kzalloc(sizeof(*pasid_table), GFP_ATOMIC);
> Do we need to take some lock here (e.g., the pasid lock)? Otherwise
> what if two devices (that are sharing the same DMA alias) call the
> function intel_pasid_alloc_table() concurrently, then could it
> possible that we create one table for each of the device while AFAIU
> we should let them share a single pasid table?
The only place where this function is called is in a single-thread context
(protected by a spinlock of device_domain_lock with local interrupt disabled).
So we don't need an extra lock here. But anyway, I should put a comment
here.
>
>> +if (!pasid_table)
>> +return -ENOMEM;
>> +INIT_LIST_HEAD(_table->dev);
>> +
>> +size = sizeof(struct pasid_entry);
>> +count = min_t(int, pci_max_pasids(to_pci_dev(dev)), intel_pasid_max_id);
>> +order =
Re: [PATCH v4 9/9] iommu/vt-d: Remove the obsolete per iommu pasid tables
Hi Peter,
On 07/11/2018 10:45 AM, Peter Xu wrote:
> On Mon, Jul 09, 2018 at 01:22:58PM +0800, Lu Baolu wrote:
>> The obsolete per iommu pasid tables are no longer used. Hence,
>> clean up them.
>>
>> Cc: Ashok Raj
>> Cc: Jacob Pan
>> Cc: Kevin Tian
>> Cc: Liu Yi L
>> Signed-off-by: Lu Baolu
>> Reviewed-by: Liu Yi L
>> ---
>> drivers/iommu/intel-iommu.c | 6 +++---
>> drivers/iommu/intel-svm.c | 17 ++---
>> include/linux/intel-iommu.h | 5 ++---
>> 3 files changed, 7 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
>> index f8609b5..9a88081 100644
>> --- a/drivers/iommu/intel-iommu.c
>> +++ b/drivers/iommu/intel-iommu.c
>> @@ -1755,7 +1755,7 @@ static void free_dmar_iommu(struct intel_iommu *iommu)
>> if (pasid_enabled(iommu)) {
>> if (ecap_prs(iommu->ecap))
>> intel_svm_finish_prq(iommu);
>> -intel_svm_free_pasid_tables(iommu);
>> +intel_svm_exit(iommu);
>> }
>> #endif
>> }
>> @@ -3336,7 +3336,7 @@ static int __init init_dmars(void)
>> hw_pass_through = 0;
>> #ifdef CONFIG_INTEL_IOMMU_SVM
>> if (pasid_enabled(iommu))
>> -intel_svm_alloc_pasid_tables(iommu);
>> +intel_svm_init(iommu);
>> #endif
>> }
>>
>> @@ -4330,7 +4330,7 @@ static int intel_iommu_add(struct dmar_drhd_unit
>> *dmaru)
>>
>> #ifdef CONFIG_INTEL_IOMMU_SVM
>> if (pasid_enabled(iommu))
>> -intel_svm_alloc_pasid_tables(iommu);
>> +intel_svm_init(iommu);
>> #endif
>>
>> if (dmaru->ignored) {
>> diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
>> index a253cde..eb30836 100644
>> --- a/drivers/iommu/intel-svm.c
>> +++ b/drivers/iommu/intel-svm.c
>> @@ -38,7 +38,7 @@ struct pasid_state_entry {
>> u64 val;
>> };
>>
>> -int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu)
>> +int intel_svm_init(struct intel_iommu *iommu)
>> {
>> struct page *pages;
>> int order;
>> @@ -63,15 +63,6 @@ int intel_svm_alloc_pasid_tables(struct intel_iommu
>> *iommu)
>> iommu->pasid_max = 0x2;
>>
>> order = get_order(sizeof(struct pasid_entry) * iommu->pasid_max);
>> -pages = alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
>> -if (!pages) {
>> -pr_warn("IOMMU: %s: Failed to allocate PASID table\n",
>> -iommu->name);
>> -return -ENOMEM;
>> -}
>> -iommu->pasid_table = page_address(pages);
>> -pr_info("%s: Allocated order %d PASID table.\n", iommu->name, order);
>> -
>> if (ecap_dis(iommu->ecap)) {
>> /* Just making it explicit... */
>> BUILD_BUG_ON(sizeof(struct pasid_entry) != sizeof(struct
>> pasid_state_entry));
> Then do we still need the so-called pasid state table? I tried to
> find out what it did before but I failed since I see that the bit 27
> of the extended cap is marked as reserved now in the latest vt-d spec
> (then ecap_dis could be meaningless too).
>
> Asked since if we don't need both the per-iommu pasid table and the
> pasid state table, then maybe we don't need intel_svm_init() at all?
>
>> @@ -86,14 +77,10 @@ int intel_svm_alloc_pasid_tables(struct intel_iommu
>> *iommu)
>> return 0;
>> }
>>
>> -int intel_svm_free_pasid_tables(struct intel_iommu *iommu)
>> +int intel_svm_exit(struct intel_iommu *iommu)
> Same thing applies to this exit() function - after we removed the
> per-iommu idr, per-iommu pasid table, and (possibly obsolete)
> per-iommu pasid state table, do we need that at all?
>
> Thanks,
>
Good eyes!
Yes, we will cleanup this with a following patch series which enables
features defined in the vt-d v3.0.
Best regards,
Lu Baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/9] iommu/vt-d: Global PASID name space
Hi Peter, Thanks for looking into my patches. On 07/11/2018 10:48 AM, Peter Xu wrote: > On Mon, Jul 09, 2018 at 01:22:50PM +0800, Lu Baolu wrote: > > [...] > >> +#ifndef __INTEL_PASID_H >> +#define __INTEL_PASID_H >> + >> +#define PASID_MIN 0x1 >> +#define PASID_MAX 0x2 > Could I ask whether there's a reason to explicitly use 0x2 for the > max value? Asked since I saw that the example in the spec gave 20 > bits for PASID (please refer to spec ver 3.0 section 3.4.3 figure > 3-8). Also I believe that's what I was told by Kevin. > > I saw that the old per-iommu max value is set to 0x2, though I'm > not sure whether that's still needed since if we're going to have > two-level pasid table then AFAIU we don't need physically continuous > memory any more (though I saw that we don't yet have two-level pasid > table implemented): > > /* Eventually I'm promised we will get a multi-level PASID table >* and it won't have to be physically contiguous. Until then, >* limit the size because 8MiB contiguous allocations can be hard >* to come by. The limit of 0x2, which is 1MiB for each of >* the PASID and PASID-state tables, is somewhat arbitrary. */ > if (iommu->pasid_max > 0x2) > iommu->pasid_max = 0x2; You are right. With the scalable mode defined in vt-d v3.0, wecould use the full 20 bit pasid. Previous max pasid was intended to save contiguous physical memory. Best regards, Lu Baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
