* [PATCH v2 0/2] PCI: aardvark: Cleanup
From: Bjorn Helgaas @ 2016-10-12 16:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161012123001.22186.4053.stgit@bhelgaas-glaptop2.roam.corp.google.com>
On Wed, Oct 12, 2016 at 07:36:21AM -0500, Bjorn Helgaas wrote:
> Add local "dev" pointers to reduce repetition of things like "&pdev->dev"
> and remove unused drvdata.
>
> Changes from v1:
> I dropped the following because they added a lot of churn for
> questionable benefit:
> PCI: aardvark: Name private struct pointer "advk" consistently
> PCI: aardvark: Reorder accessor functions
> PCI: aardvark: Swap order of advk_write() reg/val arguments
>
> ---
>
> Bjorn Helgaas (2):
> PCI: aardvark: Add local struct device pointers
> PCI: aardvark: Remove unused platform data
I applied these to pci/host-aardvark for v4.9. I hope to ask Linus to
pull them tomorrow, so if you see any issues, let me know soon.
^ permalink raw reply
* [PATCH V3 01/10] acpi: apei: read ack upon ghes record consumption
From: Punit Agrawal @ 2016-10-12 15:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475875882-2604-2-git-send-email-tbaicar@codeaurora.org>
Hi Tyler,
A few comments below.
Tyler Baicar <tbaicar@codeaurora.org> writes:
> A RAS (Reliability, Availability, Serviceability) controller
> may be a separate processor running in parallel with OS
> execution, and may generate error records for consumption by
> the OS. If the RAS controller produces multiple error records,
> then they may be overwritten before the OS has consumed them.
>
> The Generic Hardware Error Source (GHES) v2 structure
> introduces the capability for the OS to acknowledge the
> consumption of the error record generated by the RAS
> controller. A RAS controller supporting GHESv2 shall wait for
> the acknowledgment before writing a new error record, thus
> eliminating the race condition.
>
> Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
> Signed-off-by: Richard Ruigrok <rruigrok@codeaurora.org>
> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
> Signed-off-by: Naveen Kaje <nkaje@codeaurora.org>
> ---
> drivers/acpi/apei/ghes.c | 41 +++++++++++++++++++++++++++++++++++++++++
> drivers/acpi/apei/hest.c | 7 +++++--
> include/acpi/ghes.h | 1 +
> 3 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 60746ef..3021f0e 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -45,6 +45,7 @@
> #include <linux/aer.h>
> #include <linux/nmi.h>
>
> +#include <acpi/actbl1.h>
> #include <acpi/ghes.h>
> #include <acpi/apei.h>
> #include <asm/tlbflush.h>
> @@ -244,10 +245,22 @@ static struct ghes *ghes_new(struct acpi_hest_generic *generic)
> struct ghes *ghes;
> unsigned int error_block_length;
> int rc;
> + struct acpi_hest_header *hest_hdr;
>
> ghes = kzalloc(sizeof(*ghes), GFP_KERNEL);
> if (!ghes)
> return ERR_PTR(-ENOMEM);
> +
> + hest_hdr = (struct acpi_hest_header *)generic;
> + if (hest_hdr->type == ACPI_HEST_TYPE_GENERIC_ERROR_V2) {
> + ghes->generic_v2 = (struct acpi_hest_generic_v2 *)generic;
> + rc = apei_map_generic_address(
> + &ghes->generic_v2->read_ack_register);
> + if (rc)
> + goto err_unmap;
> + } else
> + ghes->generic_v2 = NULL;
Since you kzalloc ghes, shouldn't ghes->generic_v2 be NULL already?
> +
> ghes->generic = generic;
> rc = apei_map_generic_address(&generic->error_status_address);
> if (rc)
> @@ -270,6 +283,9 @@ static struct ghes *ghes_new(struct acpi_hest_generic *generic)
>
> err_unmap:
> apei_unmap_generic_address(&generic->error_status_address);
> + if (ghes->generic_v2)
> + apei_unmap_generic_address(
> + &ghes->generic_v2->read_ack_register);
> err_free:
> kfree(ghes);
> return ERR_PTR(rc);
> @@ -279,6 +295,9 @@ static void ghes_fini(struct ghes *ghes)
> {
> kfree(ghes->estatus);
> apei_unmap_generic_address(&ghes->generic->error_status_address);
> + if (ghes->generic_v2)
> + apei_unmap_generic_address(
> + &ghes->generic_v2->read_ack_register);
> }
>
> static inline int ghes_severity(int severity)
> @@ -648,6 +667,22 @@ static void ghes_estatus_cache_add(
> rcu_read_unlock();
> }
>
> +static int ghes_do_read_ack(struct acpi_hest_generic_v2 *generic_v2)
> +{
> + int rc;
> + u64 val = 0;
> +
> + rc = apei_read(&val, &generic_v2->read_ack_register);
> + if (rc)
> + return rc;
> + val &= generic_v2->read_ack_preserve <<
> + generic_v2->read_ack_register.bit_offset;
> + val |= generic_v2->read_ack_write;
Reading the spec, it is not clear whether you need the left shift
above.
Having said that, if you do it for read_ack_preserve, do you also need
to left shift read_ack_write by read_ack_register.bit_offset?
> + rc = apei_write(val, &generic_v2->read_ack_register);
> +
> + return rc;
> +}
> +
> static int ghes_proc(struct ghes *ghes)
> {
> int rc;
> @@ -660,6 +695,12 @@ static int ghes_proc(struct ghes *ghes)
> ghes_estatus_cache_add(ghes->generic, ghes->estatus);
> }
> ghes_do_proc(ghes, ghes->estatus);
> +
> + if (ghes->generic_v2) {
> + rc = ghes_do_read_ack(ghes->generic_v2);
> + if (rc)
> + return rc;
> + }
> out:
> ghes_clear_estatus(ghes);
> return 0;
> diff --git a/drivers/acpi/apei/hest.c b/drivers/acpi/apei/hest.c
> index 792a0d9..ef725a9 100644
> --- a/drivers/acpi/apei/hest.c
> +++ b/drivers/acpi/apei/hest.c
> @@ -52,6 +52,7 @@ static const int hest_esrc_len_tab[ACPI_HEST_TYPE_RESERVED] = {
> [ACPI_HEST_TYPE_AER_ENDPOINT] = sizeof(struct acpi_hest_aer),
> [ACPI_HEST_TYPE_AER_BRIDGE] = sizeof(struct acpi_hest_aer_bridge),
> [ACPI_HEST_TYPE_GENERIC_ERROR] = sizeof(struct acpi_hest_generic),
> + [ACPI_HEST_TYPE_GENERIC_ERROR_V2] = sizeof(struct acpi_hest_generic_v2),
> };
>
> static int hest_esrc_len(struct acpi_hest_header *hest_hdr)
> @@ -146,7 +147,8 @@ static int __init hest_parse_ghes_count(struct acpi_hest_header *hest_hdr, void
> {
> int *count = data;
>
> - if (hest_hdr->type == ACPI_HEST_TYPE_GENERIC_ERROR)
> + if (hest_hdr->type == ACPI_HEST_TYPE_GENERIC_ERROR ||
> + hest_hdr->type == ACPI_HEST_TYPE_GENERIC_ERROR_V2)
> (*count)++;
> return 0;
> }
> @@ -157,7 +159,8 @@ static int __init hest_parse_ghes(struct acpi_hest_header *hest_hdr, void *data)
> struct ghes_arr *ghes_arr = data;
> int rc, i;
>
> - if (hest_hdr->type != ACPI_HEST_TYPE_GENERIC_ERROR)
> + if (hest_hdr->type != ACPI_HEST_TYPE_GENERIC_ERROR &&
> + hest_hdr->type != ACPI_HEST_TYPE_GENERIC_ERROR_V2)
> return 0;
>
> if (!((struct acpi_hest_generic *)hest_hdr)->enabled)
> diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
> index 720446c..d0108b6 100644
> --- a/include/acpi/ghes.h
> +++ b/include/acpi/ghes.h
> @@ -14,6 +14,7 @@
>
> struct ghes {
> struct acpi_hest_generic *generic;
> + struct acpi_hest_generic_v2 *generic_v2;
You either have a GHES or a GHESv2 structure. Instead of duplication,
could this be represented as a union?
Thanks,
Punit
> struct acpi_hest_generic_status *estatus;
> u64 buffer_paddr;
> unsigned long flags;
^ permalink raw reply
* [bug report] perf: xgene: Add APM X-Gene SoC Performance Monitoring Unit driver
From: Tai Tri Nguyen @ 2016-10-12 15:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161012152326.GB21489@remoulade>
Hi Mark,
Sure I'm working on it.
Regards,
Tai
On Wed, Oct 12, 2016 at 8:23 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi Dan,
>
> Thanks for the report.
>
> On Wed, Oct 12, 2016 at 02:32:29PM +0300, Dan Carpenter wrote:
>> The patch 832c927d119b: "perf: xgene: Add APM X-Gene SoC Performance
>> Monitoring Unit driver" from Jul 15, 2016, leads to the following
>> static checker warning:
>>
>> drivers/perf/xgene_pmu.c:1014 acpi_get_pmu_hw_inf()
>> warn: '&res' isn't an ERR_PTR
>>
>> drivers/perf/xgene_pmu.c
>> 992 static struct
>> 993 xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu *xgene_pmu,
>> 994 struct acpi_device *adev, u32 type)
>> 995 {
>> 996 struct device *dev = xgene_pmu->dev;
>> 997 struct list_head resource_list;
>> 998 struct xgene_pmu_dev_ctx *ctx;
>> 999 const union acpi_object *obj;
>> 1000 struct hw_pmu_info *inf;
>> 1001 void __iomem *dev_csr;
>> 1002 struct resource res;
>> 1003 int enable_bit;
>> 1004 int rc;
>> 1005
>> 1006 ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
>> 1007 if (!ctx)
>> 1008 return NULL;
>> 1009
>> 1010 INIT_LIST_HEAD(&resource_list);
>> 1011 rc = acpi_dev_get_resources(adev, &resource_list,
>> 1012 acpi_pmu_dev_add_resource, &res);
>> 1013 acpi_dev_free_resource_list(&resource_list);
>> 1014 if (rc < 0 || IS_ERR(&res)) {
>> ^^^^
>> Obviously this is a stack address and not an error pointer. I'm not
>> sure what was intended here.
>
> Urrgh; I should have caught that in review. It's been this way since v1 [1], so
> I can't reverse-engineer the intent. However, I don't believe that it's
> necessary to check anything other than the rc value here.
>
> This should always evaluate as false, and shouldn't result in a functional
> problem, but it is completely bogus.
>
> Tai, can you plase send a patch deleting The IS_ERR() check here?
>
> Thanks,
> Mark.
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-March/419173.html
--
Tai
^ permalink raw reply
* [bug report] perf: xgene: Add APM X-Gene SoC Performance Monitoring Unit driver
From: Mark Rutland @ 2016-10-12 15:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161012113229.GA16447@mwanda>
Hi Dan,
Thanks for the report.
On Wed, Oct 12, 2016 at 02:32:29PM +0300, Dan Carpenter wrote:
> The patch 832c927d119b: "perf: xgene: Add APM X-Gene SoC Performance
> Monitoring Unit driver" from Jul 15, 2016, leads to the following
> static checker warning:
>
> drivers/perf/xgene_pmu.c:1014 acpi_get_pmu_hw_inf()
> warn: '&res' isn't an ERR_PTR
>
> drivers/perf/xgene_pmu.c
> 992 static struct
> 993 xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu *xgene_pmu,
> 994 struct acpi_device *adev, u32 type)
> 995 {
> 996 struct device *dev = xgene_pmu->dev;
> 997 struct list_head resource_list;
> 998 struct xgene_pmu_dev_ctx *ctx;
> 999 const union acpi_object *obj;
> 1000 struct hw_pmu_info *inf;
> 1001 void __iomem *dev_csr;
> 1002 struct resource res;
> 1003 int enable_bit;
> 1004 int rc;
> 1005
> 1006 ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> 1007 if (!ctx)
> 1008 return NULL;
> 1009
> 1010 INIT_LIST_HEAD(&resource_list);
> 1011 rc = acpi_dev_get_resources(adev, &resource_list,
> 1012 acpi_pmu_dev_add_resource, &res);
> 1013 acpi_dev_free_resource_list(&resource_list);
> 1014 if (rc < 0 || IS_ERR(&res)) {
> ^^^^
> Obviously this is a stack address and not an error pointer. I'm not
> sure what was intended here.
Urrgh; I should have caught that in review. It's been this way since v1 [1], so
I can't reverse-engineer the intent. However, I don't believe that it's
necessary to check anything other than the rc value here.
This should always evaluate as false, and shouldn't result in a functional
problem, but it is completely bogus.
Tai, can you plase send a patch deleting The IS_ERR() check here?
Thanks,
Mark.
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-March/419173.html
^ permalink raw reply
* [PATCH v3 2/5] arm64: mm: replace 'block_mappings_allowed' with 'page_mappings_only'
From: Mark Rutland @ 2016-10-12 15:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476271425-19401-3-git-send-email-ard.biesheuvel@linaro.org>
On Wed, Oct 12, 2016 at 12:23:42PM +0100, Ard Biesheuvel wrote:
> In preparation of adding support for contiguous PTE and PMD mappings,
> let's replace 'block_mappings_allowed' with 'page_mappings_only', which
> will be a more accurate description of the nature of the setting once we
> add such contiguous mappings into the mix.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Regardless of the contiguous bit stuff, I think this makes the code
clearer. As far as I can tell, this is correct. So FWIW:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
> ---
> arch/arm64/include/asm/mmu.h | 2 +-
> arch/arm64/kernel/efi.c | 8 ++---
> arch/arm64/mm/mmu.c | 32 ++++++++++----------
> 3 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> index 8d9fce037b2f..a81454ad5455 100644
> --- a/arch/arm64/include/asm/mmu.h
> +++ b/arch/arm64/include/asm/mmu.h
> @@ -34,7 +34,7 @@ extern void __iomem *early_io_map(phys_addr_t phys, unsigned long virt);
> extern void init_mem_pgprot(void);
> extern void create_pgd_mapping(struct mm_struct *mm, phys_addr_t phys,
> unsigned long virt, phys_addr_t size,
> - pgprot_t prot, bool allow_block_mappings);
> + pgprot_t prot, bool page_mappings_only);
> extern void *fixmap_remap_fdt(phys_addr_t dt_phys);
>
> #endif
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index ba9bee389fd5..5d17f377d905 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -62,8 +62,8 @@ struct screen_info screen_info __section(.data);
> int __init efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md)
> {
> pteval_t prot_val = create_mapping_protection(md);
> - bool allow_block_mappings = (md->type != EFI_RUNTIME_SERVICES_CODE &&
> - md->type != EFI_RUNTIME_SERVICES_DATA);
> + bool page_mappings_only = (md->type == EFI_RUNTIME_SERVICES_CODE ||
> + md->type == EFI_RUNTIME_SERVICES_DATA);
>
> if (!PAGE_ALIGNED(md->phys_addr) ||
> !PAGE_ALIGNED(md->num_pages << EFI_PAGE_SHIFT)) {
> @@ -76,12 +76,12 @@ int __init efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md)
> * from the MMU routines. So avoid block mappings altogether in
> * that case.
> */
> - allow_block_mappings = false;
> + page_mappings_only = true;
> }
>
> create_pgd_mapping(mm, md->phys_addr, md->virt_addr,
> md->num_pages << EFI_PAGE_SHIFT,
> - __pgprot(prot_val | PTE_NG), allow_block_mappings);
> + __pgprot(prot_val | PTE_NG), page_mappings_only);
> return 0;
> }
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index e1c34e5a1d7d..bf1d71b62c4f 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -139,7 +139,7 @@ static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
> static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
> phys_addr_t phys, pgprot_t prot,
> phys_addr_t (*pgtable_alloc)(void),
> - bool allow_block_mappings)
> + bool page_mappings_only)
> {
> pmd_t *pmd;
> unsigned long next;
> @@ -166,7 +166,7 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
>
> /* try section mapping first */
> if (((addr | next | phys) & ~SECTION_MASK) == 0 &&
> - allow_block_mappings) {
> + !page_mappings_only) {
> pmd_set_huge(pmd, phys, prot);
>
> /*
> @@ -204,7 +204,7 @@ static inline bool use_1G_block(unsigned long addr, unsigned long next,
> static void alloc_init_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
> phys_addr_t phys, pgprot_t prot,
> phys_addr_t (*pgtable_alloc)(void),
> - bool allow_block_mappings)
> + bool page_mappings_only)
> {
> pud_t *pud;
> unsigned long next;
> @@ -226,7 +226,7 @@ static void alloc_init_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
> /*
> * For 4K granule only, attempt to put down a 1GB block
> */
> - if (use_1G_block(addr, next, phys) && allow_block_mappings) {
> + if (use_1G_block(addr, next, phys) && !page_mappings_only) {
> pud_set_huge(pud, phys, prot);
>
> /*
> @@ -238,7 +238,7 @@ static void alloc_init_pud(pgd_t *pgd, unsigned long addr, unsigned long end,
> ~modifiable_attr_mask) != 0);
> } else {
> alloc_init_pmd(pud, addr, next, phys, prot,
> - pgtable_alloc, allow_block_mappings);
> + pgtable_alloc, page_mappings_only);
>
> BUG_ON(pud_val(old_pud) != 0 &&
> pud_val(old_pud) != pud_val(*pud));
> @@ -253,7 +253,7 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
> unsigned long virt, phys_addr_t size,
> pgprot_t prot,
> phys_addr_t (*pgtable_alloc)(void),
> - bool allow_block_mappings)
> + bool page_mappings_only)
> {
> unsigned long addr, length, end, next;
> pgd_t *pgd = pgd_offset_raw(pgdir, virt);
> @@ -273,7 +273,7 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
> do {
> next = pgd_addr_end(addr, end);
> alloc_init_pud(pgd, addr, next, phys, prot, pgtable_alloc,
> - allow_block_mappings);
> + page_mappings_only);
> phys += next - addr;
> } while (pgd++, addr = next, addr != end);
> }
> @@ -302,17 +302,17 @@ static void __init create_mapping_noalloc(phys_addr_t phys, unsigned long virt,
> &phys, virt);
> return;
> }
> - __create_pgd_mapping(init_mm.pgd, phys, virt, size, prot, NULL, true);
> + __create_pgd_mapping(init_mm.pgd, phys, virt, size, prot, NULL, false);
> }
>
> void __init create_pgd_mapping(struct mm_struct *mm, phys_addr_t phys,
> unsigned long virt, phys_addr_t size,
> - pgprot_t prot, bool allow_block_mappings)
> + pgprot_t prot, bool page_mappings_only)
> {
> BUG_ON(mm == &init_mm);
>
> __create_pgd_mapping(mm->pgd, phys, virt, size, prot,
> - pgd_pgtable_alloc, allow_block_mappings);
> + pgd_pgtable_alloc, page_mappings_only);
> }
>
> static void create_mapping_late(phys_addr_t phys, unsigned long virt,
> @@ -325,7 +325,7 @@ static void create_mapping_late(phys_addr_t phys, unsigned long virt,
> }
>
> __create_pgd_mapping(init_mm.pgd, phys, virt, size, prot,
> - NULL, !debug_pagealloc_enabled());
> + NULL, debug_pagealloc_enabled());
> }
>
> static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end)
> @@ -343,7 +343,7 @@ static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end
> __create_pgd_mapping(pgd, start, __phys_to_virt(start),
> end - start, PAGE_KERNEL,
> early_pgtable_alloc,
> - !debug_pagealloc_enabled());
> + debug_pagealloc_enabled());
> return;
> }
>
> @@ -356,13 +356,13 @@ static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end
> __phys_to_virt(start),
> kernel_start - start, PAGE_KERNEL,
> early_pgtable_alloc,
> - !debug_pagealloc_enabled());
> + debug_pagealloc_enabled());
> if (kernel_end < end)
> __create_pgd_mapping(pgd, kernel_end,
> __phys_to_virt(kernel_end),
> end - kernel_end, PAGE_KERNEL,
> early_pgtable_alloc,
> - !debug_pagealloc_enabled());
> + debug_pagealloc_enabled());
>
> /*
> * Map the linear alias of the [_text, __init_begin) interval as
> @@ -372,7 +372,7 @@ static void __init __map_memblock(pgd_t *pgd, phys_addr_t start, phys_addr_t end
> */
> __create_pgd_mapping(pgd, kernel_start, __phys_to_virt(kernel_start),
> kernel_end - kernel_start, PAGE_KERNEL_RO,
> - early_pgtable_alloc, !debug_pagealloc_enabled());
> + early_pgtable_alloc, debug_pagealloc_enabled());
> }
>
> static void __init map_mem(pgd_t *pgd)
> @@ -422,7 +422,7 @@ static void __init map_kernel_segment(pgd_t *pgd, void *va_start, void *va_end,
> BUG_ON(!PAGE_ALIGNED(size));
>
> __create_pgd_mapping(pgd, pa_start, (unsigned long)va_start, size, prot,
> - early_pgtable_alloc, !debug_pagealloc_enabled());
> + early_pgtable_alloc, debug_pagealloc_enabled());
>
> vma->addr = va_start;
> vma->phys_addr = pa_start;
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v3 1/5] arm64: mm: BUG on unsupported manipulations of live kernel mappings
From: Catalin Marinas @ 2016-10-12 15:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476271425-19401-2-git-send-email-ard.biesheuvel@linaro.org>
On Wed, Oct 12, 2016 at 12:23:41PM +0100, Ard Biesheuvel wrote:
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -28,8 +28,6 @@
> #include <linux/memblock.h>
> #include <linux/fs.h>
> #include <linux/io.h>
> -#include <linux/slab.h>
> -#include <linux/stop_machine.h>
>
> #include <asm/barrier.h>
> #include <asm/cputype.h>
> @@ -95,6 +93,12 @@ static phys_addr_t __init early_pgtable_alloc(void)
> return phys;
> }
>
> +/*
> + * The following mapping attributes may be updated in live
> + * kernel mappings without the need for break-before-make.
> + */
> +static const pteval_t modifiable_attr_mask = PTE_PXN | PTE_RDONLY | PTE_WRITE;
> +
> static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
> unsigned long end, unsigned long pfn,
> pgprot_t prot,
> @@ -115,8 +119,18 @@ static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
>
> pte = pte_set_fixmap_offset(pmd, addr);
> do {
> + pte_t old_pte = *pte;
> +
> set_pte(pte, pfn_pte(pfn, prot));
> pfn++;
> +
> + /*
> + * After the PTE entry has been populated once, we
> + * only allow updates to the permission attributes.
> + */
> + BUG_ON(pte_val(old_pte) != 0 &&
> + ((pte_val(old_pte) ^ pte_val(*pte)) &
> + ~modifiable_attr_mask) != 0);
Please turn this check into a single macro. You have it in three places
already (though with different types but a macro would do). Something
like below (feel free to come up with a better macro name):
BUG_ON(!safe_pgattr_change(old_pte, *pte));
--
Catalin
^ permalink raw reply
* [PATCH/RFT 07/12] USB: ohci-da8xx: Request gpios and handle interrupt in the driver
From: Axel Haslam @ 2016-10-12 15:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <22f1532a-b651-3330-b2ce-9a64a35c85af@lechnology.com>
Hi David
On Tue, Oct 11, 2016 at 1:18 AM, David Lechner <david@lechnology.com> wrote:
> On 10/07/2016 11:42 AM, ahaslam at baylibre.com wrote:
>>
>> From: Axel Haslam <ahaslam@baylibre.com>
>>
>> Currently requesting the vbus and overcurrent gpio is handled on
>> the board specific file. But this does not play well moving to
>> device tree.
>>
>> In preparation to migrate to a device tree boot, handle requesting
>> gpios and overcurrent interrupt on the usb driver itself, thus avoiding
>> callbacks to arch/mach*
>>
>
> Instead of using gpios, it seems like it would be better to use a regulator
> here. I don't know of any real-life cases, but who is to say someone
> not design a board that uses a regulator controlled by I2C instead of gpios
> or something like that.
>
> Then, boards that don't have gpios can just use a fixed regulator (or you
> can make the regulator optional). Using a regulator would also allow users
> to decide how to respond to overcurrent events (by supplying their own
> regulator driver) instead of the behavior being dictated by the ohci driver.
I agree that we should use a regulator for the vbus power.
i will make that change. However, im not so sure about using the
regulator for the overcurrent handling. There seems to be no other
driver doing this, and as you mention, we would need to change the regulator
framework, which might not be justifiable. I think there is not another way
to handle the over current notification other than powering the port off.
>
> In my particular area of interest (LEGO MINDSTORMS EV3), the 5V (hardware)
> regulator for VBUS does use gpios, but the 5V is also shared with the LEGO
> input and output ports. So what I would ultimately like to be able to do is
> have userspace notified of an overcurrent event and let userspace decided
> when to turn the vbus back on. For example, someone might plug something
> into one of the LEGO input or output ports that causes a short circuit. I
> would like to display a notification to the user and wait for them to
> correct the problem and then press a button to turn the power back on.
>
how about using regulator for vbus, but keeping gpio for overcurrent
notifications?
For the usersapce handling you describe above, maybe we should be able to
listen for an usb overcurrent uevent in userspace? it seems this
question was asked
a couple of years back[1], but im not sure what the conclusion was. In any case,
we could have DT and non-DT based ohci-da8xx working,
And could work on a uevent notification for the scenario you describe
above which
i think is not specific to the ohci-da8xx.
[1]http://linux-usb.vger.kernel.narkive.com/SjcUB5hk/how-best-to-get-over-current-notification-to-user-application
Regards
Axel
> This will require some modifications to the regulator subsystem though. I
> actually started work on this a while back, but haven't had the time to
> pursue it any farther.
>
> Here are my WIP patches in case there is any interest:
> *
> https://github.com/dlech/ev3dev-kernel/commit/541a42b3b8ed639e95bbc835df3292f80190c789
> *
> https://github.com/dlech/ev3dev-kernel/commit/2ba99b1ad6a06c944dd33a073f54044e71b75ae6
> *
> https://github.com/dlech/ev3dev-kernel/commit/cdb03caa50e64931d4f2836c648739aa4385ed3b
> *
> https://github.com/dlech/ev3dev-kernel/commit/9d6b50cde34b51309c74d97c26b1430c7ff6aa0f
^ permalink raw reply
* [linux-sunxi] [PATCH 3/5] Input: add driver for Ilitek ili2139 touch IC
From: Hans de Goede @ 2016-10-12 14:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161011132149.LZ40KToV@smtp1h.mail.yandex.net>
Hi,
On 10/11/2016 12:21 PM, Icenowy Zheng wrote:
>
> 2016?10?11? ??5:37? Hans de Goede <hdegoede@redhat.com>???
>>
>> Hi,
>
> I have a request: could you please test this driver on your E708 Q1? (According to the info you published on github, your E708 has also ili)
I'm afraid the touchscreen on my E708 Q1 is broken (does not even work in android),
so I cannot test this.
>
>>
>> On 10/11/2016 02:33 AM, Icenowy Zheng wrote:
>>> This driver adds support for Ilitek ili2139 touch IC, which is used in
>>> several Colorfly tablets (for example, Colorfly E708 Q1, which is an
>>> Allwinner A31s tablet with mainline kernel support).
>>>
>>> Theortically it may support more Ilitek touch ICs, however, only ili2139
>>> is used in any mainlined device.
>>>
>>> It supports device tree enumeration, with screen resolution and axis
>>> quirks configurable.
>>>
>>> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>>> ---
>>> drivers/input/touchscreen/Kconfig | 14 ++
>>> drivers/input/touchscreen/Makefile | 1 +
>>> drivers/input/touchscreen/ili2139.c | 320 ++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 335 insertions(+)
>>> create mode 100644 drivers/input/touchscreen/ili2139.c
>>>
>>> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
>>> index 5079813..bb4d9d2 100644
>>> --- a/drivers/input/touchscreen/Kconfig
>>> +++ b/drivers/input/touchscreen/Kconfig
>>> @@ -348,6 +348,20 @@ config TOUCHSCREEN_ILI210X
>>> To compile this driver as a module, choose M here: the
>>> module will be called ili210x.
>>>
>>> +config TOUCHSCREEN_ILI2139
>>> + tristate "Ilitek ILI2139 based touchscreen"
>>> + depends on I2C
>>> + depends on OF
>>> + help
>>> + Say Y here if you have a ILI2139 based touchscreen
>>> + controller. Such kind of chipsets can be found in several
>>> + Colorfly tablets.
>>> +
>>> + If unsure, say N.
>>> +
>>> + To compile this driver as a module, choose M here; the
>>> + module will be called ili2139.
>>> +
>>> config TOUCHSCREEN_IPROC
>>> tristate "IPROC touch panel driver support"
>>> depends on ARCH_BCM_IPROC || COMPILE_TEST
>>> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
>>> index 81b8645..930b5e2 100644
>>> --- a/drivers/input/touchscreen/Makefile
>>> +++ b/drivers/input/touchscreen/Makefile
>>> @@ -40,6 +40,7 @@ obj-$(CONFIG_TOUCHSCREEN_EGALAX_SERIAL) += egalax_ts_serial.o
>>> obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
>>> obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix.o
>>> obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
>>> +obj-$(CONFIG_TOUCHSCREEN_ILI2139) += ili2139.o
>>> obj-$(CONFIG_TOUCHSCREEN_IMX6UL_TSC) += imx6ul_tsc.o
>>> obj-$(CONFIG_TOUCHSCREEN_INEXIO) += inexio.o
>>> obj-$(CONFIG_TOUCHSCREEN_INTEL_MID) += intel-mid-touch.o
>>> diff --git a/drivers/input/touchscreen/ili2139.c b/drivers/input/touchscreen/ili2139.c
>>> new file mode 100644
>>> index 0000000..65c2dea
>>> --- /dev/null
>>> +++ b/drivers/input/touchscreen/ili2139.c
>>> @@ -0,0 +1,320 @@
>>> +/* -------------------------------------------------------------------------
>>> + * Copyright (C) 2016, Icenowy Zheng <icenowy@aosc.xyz>
>>> + *
>>> + * Derived from:
>>> + * ili210x.c
>>> + * Copyright (C) Olivier Sobrie <olivier@sobrie.be>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + * -------------------------------------------------------------------------
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/i2c.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/input.h>
>>> +#include <linux/input/mt.h>
>>> +#include <linux/input/touchscreen.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/workqueue.h>
>>> +
>>> +#define DEFAULT_POLL_PERIOD 20
>>> +
>>> +#define MAX_TOUCHES 10
>>> +#define COMPATIBLE_TOUCHES 2
>>> +
>>> +/* Touchscreen commands */
>>> +#define REG_TOUCHDATA 0x10
>>> +#define REG_TOUCHSUBDATA 0x11
>>> +#define REG_PANEL_INFO 0x20
>>> +#define REG_FIRMWARE_VERSION 0x40
>>> +#define REG_PROTO_VERSION 0x42
>>> +
>>> +#define SUBDATA_STATUS_TOUCH_POINT 0x80
>>> +#define SUBDATA_STATUS_RELEASE_POINT 0x00
>>> +
>>> +struct finger {
>>> + u8 x_low;
>>> + u8 x_high;
>>> + u8 y_low;
>>> + u8 y_high;
>>> +} __packed;
>>> +
>>> +struct touchdata {
>>> + u8 length;
>>> + struct finger finger[COMPATIBLE_TOUCHES];
>>> +} __packed;
>>> +
>>> +struct touch_subdata {
>>> + u8 status;
>>> + struct finger finger;
>>> +} __packed;
>>> +
>>> +struct panel_info {
>>> + struct finger finger_max;
>>> + u8 xchannel_num;
>>> + u8 ychannel_num;
>>> +} __packed;
>>> +
>>> +struct firmware_version {
>>> + u8 id;
>>> + u8 major;
>>> + u8 minor;
>>> +} __packed;
>>> +
>>> +struct ili2139 {
>>> + struct i2c_client *client;
>>> + struct input_dev *input;
>>> + unsigned int poll_period;
>>> + struct delayed_work dwork;
>>> + struct touchscreen_properties prop;
>>> + int slots[MAX_TOUCHES];
>>> + int ids[MAX_TOUCHES];
>>> + struct input_mt_pos pos[MAX_TOUCHES];
>>> +};
>>> +
>>> +static int ili2139_read_reg(struct i2c_client *client, u8 reg, void *buf,
>>> + size_t len)
>>> +{
>>> + struct i2c_msg msg[2] = {
>>> + {
>>> + .addr = client->addr,
>>> + .flags = 0,
>>> + .len = 1,
>>> + .buf = ®,
>>> + },
>>> + {
>>> + .addr = client->addr,
>>> + .flags = I2C_M_RD,
>>> + .len = len,
>>> + .buf = buf,
>>> + }
>>> + };
>>> +
>>> + if (i2c_transfer(client->adapter, msg, 2) != 2) {
>>> + dev_err(&client->dev, "i2c transfer failed\n");
>>> + return -EIO;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>
>> This just i2c_smbus_read_i2c_block_data, please use that instead.
>>
>>> +static void ili2139_work(struct work_struct *work)
>>> +{
>>> + int id;
>>> + struct ili2139 *priv = container_of(work, struct ili2139,
>>> + dwork.work);
>>> + struct i2c_client *client = priv->client;
>>> + struct touchdata touchdata;
>>> + struct touch_subdata subdata;
>>> + int error;
>>> +
>>> + error = ili2139_read_reg(client, REG_TOUCHDATA,
>>> + &touchdata, sizeof(touchdata));
>>> + if (error) {
>>> + dev_err(&client->dev,
>>> + "Unable to get touchdata, err = %d\n", error);
>>> + return;
>>> + }
>>> +
>>> + for (id = 0; id < touchdata.length; id++) {
>>> + error = ili2139_read_reg(client, REG_TOUCHSUBDATA, &subdata,
>>> + sizeof(subdata));
>>> + if (error) {
>>> + dev_err(&client->dev,
>>> + "Unable to get touch subdata, err = %d\n",
>>> + error);
>>> + return;
>>> + }
>>> +
>>> + priv->ids[id] = subdata.status & 0x3F;
>>> +
>>> + /* The sequence changed in the v2 subdata protocol. */
>>> + touchscreen_set_mt_pos(&priv->pos[id], &priv->prop,
>>> + (subdata.finger.x_high | (subdata.finger.x_low << 8)),
>>> + (subdata.finger.y_high | (subdata.finger.y_low << 8)));
>>> + }
>>> +
>>> + input_mt_assign_slots(priv->input, priv->slots, priv->pos,
>>> + touchdata.length, 0);
>>> +
>>> + for (id = 0; id < touchdata.length; id++) {
>>> + input_mt_slot(priv->input, priv->slots[id]);
>>> + input_mt_report_slot_state(priv->input, MT_TOOL_FINGER,
>>> + subdata.status &
>>> + SUBDATA_STATUS_TOUCH_POINT);
>>> + input_report_abs(priv->input, ABS_MT_POSITION_X,
>>> + priv->pos[id].x);
>>> + input_report_abs(priv->input, ABS_MT_POSITION_Y,
>>> + priv->pos[id].y);
>>> + }
>>> +
>>> + input_mt_sync_frame(priv->input);
>>> + input_sync(priv->input);
>>> +
>>> + schedule_delayed_work(&priv->dwork,
>>> + msecs_to_jiffies(priv->poll_period));
>>
>> If the irq is working properly there should be no need for this,
>> can you try with this schedule call removed ?
>
> Thanks. I'm glad to know this.
> The driver that I modelled after is too old and unmaintained, and even nearly not usable now (as it requires platform data)
>
>>
>>> +}
>>> +
>>> +static irqreturn_t ili2139_irq(int irq, void *irq_data)
>>> +{
>>> + struct ili2139 *priv = irq_data;
>>> +
>>> + schedule_delayed_work(&priv->dwork, 0);
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +static int ili2139_i2c_probe(struct i2c_client *client,
>>> + const struct i2c_device_id *id)
>>> +{
>>> + struct device *dev = &client->dev;
>>> + struct ili2139 *priv;
>>> + struct input_dev *input;
>>> + struct panel_info panel;
>>> + struct firmware_version firmware;
>>> + int xmax, ymax;
>>> + int error;
>>> +
>>> + dev_dbg(dev, "Probing for ILI2139 I2C Touschreen driver");
>>> +
>>> + if (client->irq <= 0) {
>>> + dev_err(dev, "No IRQ!\n");
>>> + return -ENODEV;
>>> + }
>>> +
>>> + /* Get firmware version */
>>> + error = ili2139_read_reg(client, REG_FIRMWARE_VERSION,
>>> + &firmware, sizeof(firmware));
>>> + if (error) {
>>> + dev_err(dev, "Failed to get firmware version, err: %d\n",
>>> + error);
>>> + return error;
>>> + }
>>> +
>>> + /* get panel info */
>>> + error = ili2139_read_reg(client, REG_PANEL_INFO, &panel, sizeof(panel));
>>> + if (error) {
>>> + dev_err(dev, "Failed to get panel information, err: %d\n",
>>> + error);
>>> + return error;
>>> + }
>>> +
>>> + xmax = panel.finger_max.x_low | (panel.finger_max.x_high << 8);
>>> + ymax = panel.finger_max.y_low | (panel.finger_max.y_high << 8);
>>> +
>>> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>> + input = devm_input_allocate_device(dev);
>>> + if (!priv || !input)
>>> + return -ENOMEM;
>>> +
>>> + priv->client = client;
>>> + priv->input = input;
>>> + priv->poll_period = DEFAULT_POLL_PERIOD;
>>> + INIT_DELAYED_WORK(&priv->dwork, ili2139_work);
>>> +
>>> + /* Setup input device */
>>> + input->name = "ILI2139 Touchscreen";
>>> + input->id.bustype = BUS_I2C;
>>> + input->dev.parent = dev;
>>> +
>>> + __set_bit(EV_SYN, input->evbit);
>>> + __set_bit(EV_KEY, input->evbit);
>>> + __set_bit(EV_ABS, input->evbit);
>>> +
>>> + /* Multi touch */
>>> + input_mt_init_slots(input, MAX_TOUCHES, INPUT_MT_DIRECT |
>>> + INPUT_MT_DROP_UNUSED | INPUT_MT_TRACK);
>>> + input_set_abs_params(input, ABS_MT_POSITION_X, 0, xmax, 0, 0);
>>> + input_set_abs_params(input, ABS_MT_POSITION_Y, 0, ymax, 0, 0);
>>> +
>>> + touchscreen_parse_properties(input, true, &priv->prop);
>>> +
>>> + input_set_drvdata(input, priv);
>>> + i2c_set_clientdata(client, priv);
>>> +
>>> + error = devm_request_irq(dev, client->irq, ili2139_irq,
>>> + IRQF_TRIGGER_FALLING, client->name, priv);
>>
>> If things work with the re-scheduleing of the delayed work
>> from the work removed, then you can use request_threaded_irq here,
>> pass in ili2139_irq as the threaded handler (and NULL as the non threaded
>> handler) and do all the i2c reading directly in ili2139_irq without needing
>> to use any work struct at all.
>
> I'll test it, and remember request_threaded_irq function. I've seen the usage of the function in silead.c.
Good :)
Regards,
Hans
>
>>
>> Regards,
>>
>> Hans
>>
>>
>>> + if (error) {
>>> + dev_err(dev, "Unable to request touchscreen IRQ, err: %d\n",
>>> + error);
>>> + return error;
>>> + }
>>> +
>>> + error = input_register_device(priv->input);
>>> + if (error) {
>>> + dev_err(dev, "Cannot register input device, err: %d\n", error);
>>> + return error;
>>> + }
>>> +
>>> + device_init_wakeup(&client->dev, 1);
>>> +
>>> + dev_dbg(dev,
>>> + "ILI2139 initialized (IRQ: %d), firmware version %d.%d.%d",
>>> + client->irq, firmware.id, firmware.major, firmware.minor);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int ili2139_i2c_remove(struct i2c_client *client)
>>> +{
>>> + struct ili2139 *priv = i2c_get_clientdata(client);
>>> +
>>> + cancel_delayed_work_sync(&priv->dwork);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int __maybe_unused ili2139_i2c_suspend(struct device *dev)
>>> +{
>>> + struct i2c_client *client = to_i2c_client(dev);
>>> +
>>> + if (device_may_wakeup(&client->dev))
>>> + enable_irq_wake(client->irq);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int __maybe_unused ili2139_i2c_resume(struct device *dev)
>>> +{
>>> + struct i2c_client *client = to_i2c_client(dev);
>>> +
>>> + if (device_may_wakeup(&client->dev))
>>> + disable_irq_wake(client->irq);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static SIMPLE_DEV_PM_OPS(ili2139_i2c_pm,
>>> + ili2139_i2c_suspend, ili2139_i2c_resume);
>>> +
>>> +static const struct i2c_device_id ili2139_i2c_id[] = {
>>> + { "ili2139", 0 },
>>> + { }
>>> +};
>>> +MODULE_DEVICE_TABLE(i2c, ili2139_i2c_id);
>>> +
>>> +static struct i2c_driver ili2139_ts_driver = {
>>> + .driver = {
>>> + .name = "ili2139_i2c",
>>> + .pm = &ili2139_i2c_pm,
>>> + },
>>> + .id_table = ili2139_i2c_id,
>>> + .probe = ili2139_i2c_probe,
>>> + .remove = ili2139_i2c_remove,
>>> +};
>>> +
>>> +module_i2c_driver(ili2139_ts_driver);
>>> +
>>> +MODULE_AUTHOR("Olivier Sobrie <olivier@sobrie.be>");
>>> +MODULE_DESCRIPTION("ILI2139 I2C Touchscreen Driver");
>>> +MODULE_LICENSE("GPL");
>>>
^ permalink raw reply
* [bug report] perf: xgene: Add APM X-Gene SoC Performance Monitoring Unit driver
From: Mason @ 2016-10-12 14:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161012113229.GA16447@mwanda>
On 12/10/2016 13:32, Dan Carpenter wrote:
> Hello Tai Nguyen,
>
> The patch 832c927d119b: "perf: xgene: Add APM X-Gene SoC Performance
> Monitoring Unit driver" from Jul 15, 2016, leads to the following
> static checker warning:
>
> drivers/perf/xgene_pmu.c:1014 acpi_get_pmu_hw_inf()
> warn: '&res' isn't an ERR_PTR
>
> drivers/perf/xgene_pmu.c
> 992 static struct
> 993 xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu *xgene_pmu,
> 994 struct acpi_device *adev, u32 type)
> 995 {
> 996 struct device *dev = xgene_pmu->dev;
> 997 struct list_head resource_list;
> 998 struct xgene_pmu_dev_ctx *ctx;
> 999 const union acpi_object *obj;
> 1000 struct hw_pmu_info *inf;
> 1001 void __iomem *dev_csr;
> 1002 struct resource res;
> 1003 int enable_bit;
> 1004 int rc;
> 1005
> 1006 ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> 1007 if (!ctx)
> 1008 return NULL;
> 1009
> 1010 INIT_LIST_HEAD(&resource_list);
> 1011 rc = acpi_dev_get_resources(adev, &resource_list,
> 1012 acpi_pmu_dev_add_resource, &res);
> 1013 acpi_dev_free_resource_list(&resource_list);
> 1014 if (rc < 0 || IS_ERR(&res)) {
> ^^^^
> Obviously this is a stack address and not an error pointer. I'm not
> sure what was intended here.
Reminds me of 287980e49ffc0f6d911601e7e352a812ed27768e
("remove lots of IS_ERR_VALUE abuses")
Regards.
^ permalink raw reply
* Question: Re: [PATCH] drm/bridge: analogix: protect power when get_modes or detect
From: Sean Paul @ 2016-10-12 14:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57FE0EF5.7010207@rock-chips.com>
On Wed, Oct 12, 2016 at 6:22 AM, Mark yao <mark.yao@rock-chips.com> wrote:
>
> I'm not familiar with the analogix driver, maybe use a power reference count
> would better then direct power on/off analogix_dp.
>
> Does anyone has the idea to protect detect and get_modes context?
>
I'm not sure a reference count is going to help here. The common
pattern is to call detect() followed by get_modes() and then modeset.
However, it's not guaranteed that any one of those functions will be
called after the other. So, if you leave things on after detect or
get_modes, you might be wasting power (or worse).
I recently ran into this exact problem with a panel we're using. Check
out "0b8b059a7: drm/bridge: analogix_dp: Ensure the panel is properly
prepared/unprepared". Perhaps you can piggyback on that function to
add your pm_runtime and plat_data callbacks (since using dpms_mode
might be racey).
Sean
> I found many other connector driver also direct access register on detect or
> get_modes, no problem for it?
>
> On 2016?10?12? 18:00, Mark Yao wrote:
>
> The drm callback ->detect and ->get_modes seems is not power safe,
> they may be called when device is power off, do register access on
> detect or get_modes will cause system die.
>
> Here is the path call ->detect before analogix_dp power on
> [<ffffff800843babc>] analogix_dp_detect+0x44/0xdc
> [<ffffff80083fd840>]
> drm_helper_probe_single_connector_modes_merge_bits+0xe8/0x41c
> [<ffffff80083fdb84>] drm_helper_probe_single_connector_modes+0x10/0x18
> [<ffffff8008418d24>] drm_mode_getconnector+0xf4/0x304
> [<ffffff800840cff0>] drm_ioctl+0x23c/0x390
> [<ffffff80081a8adc>] do_vfs_ioctl+0x4b8/0x58c
> [<ffffff80081a8c10>] SyS_ioctl+0x60/0x88
>
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> Cc: "Ville Syrj?l?" <ville.syrjala@linux.intel.com>
>
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 28
> ++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index efac8ab..09dece2 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1062,6 +1062,13 @@ int analogix_dp_get_modes(struct drm_connector
> *connector)
> return 0;
> }
>
> + if (dp->dpms_mode != DRM_MODE_DPMS_ON) {
> + pm_runtime_get_sync(dp->dev);
> +
> + if (dp->plat_data->power_on)
> + dp->plat_data->power_on(dp->plat_data);
> + }
> +
> if (analogix_dp_handle_edid(dp) == 0) {
> drm_mode_connector_update_edid_property(&dp->connector, edid);
> num_modes += drm_add_edid_modes(&dp->connector, edid);
> @@ -1073,6 +1080,13 @@ int analogix_dp_get_modes(struct drm_connector
> *connector)
> if (dp->plat_data->get_modes)
> num_modes += dp->plat_data->get_modes(dp->plat_data, connector);
>
> + if (dp->dpms_mode != DRM_MODE_DPMS_ON) {
> + if (dp->plat_data->power_off)
> + dp->plat_data->power_off(dp->plat_data);
> +
> + pm_runtime_put_sync(dp->dev);
> + }
> +
> ret = analogix_dp_prepare_panel(dp, false, false);
> if (ret)
> DRM_ERROR("Failed to unprepare panel (%d)\n", ret);
> @@ -1106,9 +1120,23 @@ analogix_dp_detect(struct drm_connector *connector,
> bool force)
> return connector_status_disconnected;
> }
>
> + if (dp->dpms_mode != DRM_MODE_DPMS_ON) {
> + pm_runtime_get_sync(dp->dev);
> +
> + if (dp->plat_data->power_on)
> + dp->plat_data->power_on(dp->plat_data);
> + }
> +
> if (!analogix_dp_detect_hpd(dp))
> status = connector_status_connected;
>
> + if (dp->dpms_mode != DRM_MODE_DPMS_ON) {
> + if (dp->plat_data->power_off)
> + dp->plat_data->power_off(dp->plat_data);
> +
> + pm_runtime_put_sync(dp->dev);
> + }
> +
> ret = analogix_dp_prepare_panel(dp, false, false);
> if (ret)
> DRM_ERROR("Failed to unprepare panel (%d)\n", ret);
>
>
>
> --
> ?ark Yao
^ permalink raw reply
* [PATCH 2/3] arm64: hw_breakpoint: Handle inexact watchpoint addresses
From: Pavel Labath @ 2016-10-12 13:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHB_GupR4sJtAGYdRwvCwu7891KM8TEByZCgqMCrYrdt=cJuYw@mail.gmail.com>
Hello Pratyush,
I am sorry about the delay. I have finally had a chance to try out
your changes today. Response inline.
On 8 October 2016 at 06:10, Pratyush Anand <panand@redhat.com> wrote:
> On Fri, Oct 7, 2016 at 10:54 PM, Pavel Labath <labath@google.com> wrote:
>> The thing is, I have observed different behavior here depending on the
>> exact hardware used. I don't have the exact parameters with me now,
>> but I can look it up next week.
>>
>> The thing is that the spec is imprecise about what exact address the
>> hardware can report for the watchpoint hit. I presume that is
>> deliberate to give some leeway to implementers. The spec says the
>> address can be anywhere in the range from the lowest memory address
>> accessed by the instruction to the highest address watched by the
>> watchpoint,
>
> I think, my patches should be able to take care of the above condition.
Unfortunately, the patch does not solve the problem for my hardware,
because of the leeway you give in watchpoint_handler is not big
enough. It does work however, if I change the line
> if (addr + 7 < val + lens || addr > val + lene)
to
> if (addr + 15 < val + lens || addr > val + lene)
I do not think we can assume that address reported by the hardware
will be at most 7 bytes off from the address we put the watchpoint at.
There is nothing in the spec that guarantees that, and it does not
seem to be enough for some hardware. In fact, I am not sure we can
assume 15 is enough either, but maybe it can do for now, until we can
find hardware that does not work with that (I haven't yet tried what
happens it the watchpoint is triggered by cache management
instructions, which can access much larger blocks of memory).
For reference, the hardware in question is:
> Processor : AArch64 Processor rev 0 (aarch64)
> processor : 0
> min_vddcx : 400000
> min_vddmx : 490000
> BogoMIPS : 38.00
> Features : fp asimd evtstrm aes pmull sha1 sha2 crc32
> CPU implementer : 0x51
> CPU architecture: 8
> CPU variant : 0x2
> CPU part : 0x201
> CPU revision : 0
> CPU param : 300 468 468 621 939 299 445 445 621 1077
> Hardware : Qualcomm Technologies, Inc MSM8996pro
And this is how it behaves:
The output of the test app triggering the watchpoint (I have set a
single-byte watchpoint at 555556705f)
>
> Writing to: 555556705f, size: 1
> Writing to: 555556705e, size: 2
> Writing to: 555556705c, size: 4
> Writing to: 5555567058, size: 8
> Writing to: 5555567050, size: 16
> Writing to: 5555567040, size: 32
The addresses received by the kernel:
[ 251.812166] c1 3780 hw-breakpoint: watchpoint_handler: addr:
555556705f, val+lens: 555556705f, val+lene: 555556705f
[ 251.820341] c1 3781 hw-breakpoint: watchpoint_handler: addr:
555556705e, val+lens: 555556705f, val+lene: 555556705f
[ 251.825572] c0 3782 hw-breakpoint: watchpoint_handler: addr:
555556705c, val+lens: 555556705f, val+lene: 555556705f
[ 251.831085] c0 3783 hw-breakpoint: watchpoint_handler: addr:
5555567058, val+lens: 555556705f, val+lene: 555556705f
[ 251.835804] c0 3784 hw-breakpoint: watchpoint_handler: addr:
5555567050, val+lens: 555556705f, val+lene: 555556705f
[ 251.841350] c0 3785 hw-breakpoint: watchpoint_handler: addr:
5555567050, val+lens: 555556705f, val+lene: 555556705f
Note that for the case of 16 and 32-byte access it returns the address
5555567050 -- this is why the "+15" is sufficient for me.
The other thing I am not so sure about in your patch is that it has
potential to mis-attribute the watchpoint hit if we have two
watchpoints close together. For example, if I have first watchpoint on
0x1008-0x100f and a second one on 0x1000-0x1007, *and* the application
writes one byte to 0x1004, then your code will still attribute the hit
to the first watchpoint, even though it was not really triggered. This
is the reason I implemented my fix as a two-stage process, first
looking for exact hits, and then falling back to the nearest one. That
said, I don't know enough about the codebase to say if this is a real
problem.
On the plus side, I like the fact that we can watch arbitrary memory
regions now, and the feature is working perfectly. :)
My proposal would be to combine the two patches - take the byte mask
handling code from yours, and the hit-attribution code from my patch.
What do you think?
regards,
pavel
^ permalink raw reply
* [PATCH v14 16/16] vfio/type1: Introduce MSI_RESV capability
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>
This patch allows the user-space to retrieve the MSI reserved region
requirements, if any. The implementation is based on capability chains,
now also added to VFIO_IOMMU_GET_INFO.
The returned info comprises the size and the alignment requirements
In case the userspace must provide the IOVA aperture, we currently report
a size/alignment based on all the doorbells registered by the host kernel.
This may exceed the actual needs.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v13 -> v14:
- new capability struct
- change the padding in vfio_iommu_type1_info
v11 -> v12:
- msi_doorbell_pages was renamed msi_doorbell_calc_pages
v9 -> v10:
- move cap_offset after iova_pgsizes
- replace __u64 alignment by __u32 order
- introduce __u32 flags in vfio_iommu_type1_info_cap_msi_geometry and
fix alignment
- call msi-doorbell API to compute the size/alignment
v8 -> v9:
- use iommu_msi_supported flag instead of programmable
- replace IOMMU_INFO_REQUIRE_MSI_MAP flag by a more sophisticated
capability chain, reporting the MSI geometry
v7 -> v8:
- use iommu_domain_msi_geometry
v6 -> v7:
- remove the computation of the number of IOVA pages to be provisionned.
This number depends on the domain/group/device topology which can
dynamically change. Let's rely instead rely on an arbitrary max depending
on the system
v4 -> v5:
- move msi_info and ret declaration within the conditional code
v3 -> v4:
- replace former vfio_domains_require_msi_mapping by
more complex computation of MSI mapping requirements, especially the
number of pages to be provided by the user-space.
- reword patch title
RFC v1 -> v1:
- derived from
[RFC PATCH 3/6] vfio: Extend iommu-info to return MSIs automap state
- renamed allow_msi_reconfig into require_msi_mapping
- fixed VFIO_IOMMU_GET_INFO
---
drivers/vfio/vfio_iommu_type1.c | 67 ++++++++++++++++++++++++++++++++++++++++-
include/uapi/linux/vfio.h | 20 +++++++++++-
2 files changed, 85 insertions(+), 2 deletions(-)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index c18ba9d..6775da3 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -1147,6 +1147,46 @@ static int vfio_domains_have_iommu_cache(struct vfio_iommu *iommu)
return ret;
}
+static int msi_resv_caps(struct vfio_iommu *iommu, struct vfio_info_cap *caps)
+{
+ struct iommu_domain_msi_resv msi_resv = {.size = 0, .alignment = 0};
+ struct vfio_iommu_type1_info_cap_msi_resv *cap;
+ struct vfio_info_cap_header *header;
+ struct iommu_domain_msi_resv iter;
+ struct vfio_domain *d;
+
+ mutex_lock(&iommu->lock);
+
+ list_for_each_entry(d, &iommu->domain_list, next) {
+ if (iommu_domain_get_attr(d->domain,
+ DOMAIN_ATTR_MSI_RESV, &iter))
+ continue;
+ if (iter.size > msi_resv.size) {
+ msi_resv.size = iter.size;
+ msi_resv.alignment = iter.alignment;
+ }
+ }
+
+ if (!msi_resv.size)
+ return 0;
+
+ mutex_unlock(&iommu->lock);
+
+ header = vfio_info_cap_add(caps, sizeof(*cap),
+ VFIO_IOMMU_TYPE1_INFO_CAP_MSI_RESV, 1);
+
+ if (IS_ERR(header))
+ return PTR_ERR(header);
+
+ cap = container_of(header, struct vfio_iommu_type1_info_cap_msi_resv,
+ header);
+
+ cap->alignment = msi_resv.alignment;
+ cap->size = msi_resv.size;
+
+ return 0;
+}
+
static long vfio_iommu_type1_ioctl(void *iommu_data,
unsigned int cmd, unsigned long arg)
{
@@ -1168,8 +1208,10 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
}
} else if (cmd == VFIO_IOMMU_GET_INFO) {
struct vfio_iommu_type1_info info;
+ struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
+ int ret;
- minsz = offsetofend(struct vfio_iommu_type1_info, iova_pgsizes);
+ minsz = offsetofend(struct vfio_iommu_type1_info, cap_offset);
if (copy_from_user(&info, (void __user *)arg, minsz))
return -EFAULT;
@@ -1181,6 +1223,29 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
info.iova_pgsizes = vfio_pgsize_bitmap(iommu);
+ ret = msi_resv_caps(iommu, &caps);
+ if (ret)
+ return ret;
+
+ if (caps.size) {
+ info.flags |= VFIO_IOMMU_INFO_CAPS;
+ if (info.argsz < sizeof(info) + caps.size) {
+ info.argsz = sizeof(info) + caps.size;
+ info.cap_offset = 0;
+ } else {
+ vfio_info_cap_shift(&caps, sizeof(info));
+ if (copy_to_user((void __user *)arg +
+ sizeof(info), caps.buf,
+ caps.size)) {
+ kfree(caps.buf);
+ return -EFAULT;
+ }
+ info.cap_offset = sizeof(info);
+ }
+
+ kfree(caps.buf);
+ }
+
return copy_to_user((void __user *)arg, &info, minsz) ?
-EFAULT : 0;
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 4a9dbc2..e34a9a6 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -488,7 +488,23 @@ struct vfio_iommu_type1_info {
__u32 argsz;
__u32 flags;
#define VFIO_IOMMU_INFO_PGSIZES (1 << 0) /* supported page sizes info */
- __u64 iova_pgsizes; /* Bitmap of supported page sizes */
+#define VFIO_IOMMU_INFO_CAPS (1 << 1) /* Info supports caps */
+ __u64 iova_pgsizes; /* Bitmap of supported page sizes */
+ __u32 cap_offset; /* Offset within info struct of first cap */
+ __u32 __resv;
+};
+
+/*
+ * The MSI_RESV capability allows to report the MSI reserved IOVA requirements:
+ * In case this capability is supported, the userspace must provide an IOVA
+ * window characterized by @size and @alignment using VFIO_IOMMU_MAP_DMA with
+ * RESERVED_MSI_IOVA flag.
+ */
+#define VFIO_IOMMU_TYPE1_INFO_CAP_MSI_RESV 1
+struct vfio_iommu_type1_info_cap_msi_resv {
+ struct vfio_info_cap_header header;
+ __u64 size; /* requested IOVA aperture size in bytes */
+ __u64 alignment; /* requested byte alignment of the window */
};
#define VFIO_IOMMU_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
@@ -503,6 +519,8 @@ struct vfio_iommu_type1_info {
* IOVA region that will be used on some platforms to map the host MSI frames.
* In that specific case, vaddr is ignored. Once registered, an MSI reserved
* IOVA region stays until the container is closed.
+ * The requirement for provisioning such reserved IOVA range can be checked by
+ * checking the VFIO_IOMMU_TYPE1_INFO_CAP_MSI_RESV capability.
*/
struct vfio_iommu_type1_dma_map {
__u32 argsz;
--
1.9.1
^ permalink raw reply related
* [PATCH v14 15/16] iommu/arm-smmu: Do not advertise IOMMU_CAP_INTR_REMAP
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>
Do not advertise IOMMU_CAP_INTR_REMAP for arm-smmu(-v3). Indeed the
irq_remapping capability is abstracted on irqchip side for ARM as
opposed to Intel IOMMU featuring IRQ remapping HW.
So to check IRQ remapping capability, the msi domain needs to be
checked instead.
This commit affects platform and PCIe device assignment use cases
on any platform featuring an unsafe MSI controller (currently the
ARM GICv2m). For those platforms the VFIO module must be loaded with
allow_unsafe_interrupts set to 1.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v9 -> v10:
- reword the commit message (allow_unsafe_interrupts)
---
drivers/iommu/arm-smmu-v3.c | 3 ++-
drivers/iommu/arm-smmu.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 572cad8..d71a955 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1371,7 +1371,8 @@ static bool arm_smmu_capable(enum iommu_cap cap)
case IOMMU_CAP_CACHE_COHERENCY:
return true;
case IOMMU_CAP_INTR_REMAP:
- return true; /* MSIs are just memory writes */
+ /* interrupt translation handled at MSI controller level */
+ return false;
case IOMMU_CAP_NOEXEC:
return true;
default:
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index ae20b9c..becad89 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1361,7 +1361,8 @@ static bool arm_smmu_capable(enum iommu_cap cap)
*/
return true;
case IOMMU_CAP_INTR_REMAP:
- return true; /* MSIs are just memory writes */
+ /* interrupt translation handled at MSI controller level */
+ return false;
case IOMMU_CAP_NOEXEC:
return true;
default:
--
1.9.1
^ permalink raw reply related
* [PATCH v14 14/16] vfio/type1: Check doorbell safety
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>
On x86 IRQ remapping is abstracted by the IOMMU. On ARM this is abstracted
by the msi controller.
Since we currently have no way to detect whether the MSI controller is
upstream or downstream to the IOMMU we rely on the MSI doorbell information
registered by the interrupt controllers. In case at least one doorbell
does not implement proper isolation, we state the assignment is unsafe
with regard to interrupts. This is a coarse assessment but should allow to
wait for a better system description.
At this point ARM sMMU still advertises IOMMU_CAP_INTR_REMAP. This is
removed in next patch.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v13 -> v15:
- check vfio_msi_resv before checking whether msi doorbell is safe
v9 -> v10:
- coarse safety assessment based on MSI doorbell info
v3 -> v4:
- rename vfio_msi_parent_irq_remapping_capable into vfio_safe_irq_domain
and irq_remapping into safe_irq_domains
v2 -> v3:
- protect vfio_msi_parent_irq_remapping_capable with
CONFIG_GENERIC_MSI_IRQ_DOMAIN
---
drivers/vfio/vfio_iommu_type1.c | 30 +++++++++++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index e0c97ef..c18ba9d 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -442,6 +442,29 @@ static void vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma)
}
/**
+ * vfio_msi_resv - Return whether any VFIO iommu domain requires
+ * MSI mapping
+ *
+ * @iommu: vfio iommu handle
+ *
+ * Return: true of MSI mapping is needed, false otherwise
+ */
+static bool vfio_msi_resv(struct vfio_iommu *iommu)
+{
+ struct iommu_domain_msi_resv msi_resv;
+ struct vfio_domain *d;
+ int ret;
+
+ list_for_each_entry(d, &iommu->domain_list, next) {
+ ret = iommu_domain_get_attr(d->domain, DOMAIN_ATTR_MSI_RESV,
+ &msi_resv);
+ if (!ret)
+ return true;
+ }
+ return false;
+}
+
+/**
* vfio_set_msi_aperture - Sets the msi aperture on all domains
* requesting MSI mapping
*
@@ -945,8 +968,13 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
INIT_LIST_HEAD(&domain->group_list);
list_add(&group->next, &domain->group_list);
+ /*
+ * to advertise safe interrupts either the IOMMU or the MSI controllers
+ * must support IRQ remapping (aka. interrupt translation)
+ */
if (!allow_unsafe_interrupts &&
- !iommu_capable(bus, IOMMU_CAP_INTR_REMAP)) {
+ (!iommu_capable(bus, IOMMU_CAP_INTR_REMAP) &&
+ !(vfio_msi_resv(iommu) && iommu_msi_doorbell_safe()))) {
pr_warn("%s: No interrupt remapping support. Use the module param \"allow_unsafe_interrupts\" to enable VFIO IOMMU support on this platform\n",
__func__);
ret = -EPERM;
--
1.9.1
^ permalink raw reply related
* [PATCH v14 13/16] vfio: Allow reserved msi iova registration
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>
The user is allowed to register a reserved MSI IOVA range by using the
DMA MAP API and setting the new flag: VFIO_DMA_MAP_FLAG_MSI_RESERVED_IOVA.
This region is stored in the vfio_dma rb tree. At that point the iova
range is not mapped to any target address yet. The host kernel will use
those iova when needed, typically when MSIs are allocated.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
v13 -> v14:
- in vfio_set_msi_aperture, unfold in case of failure
- Get DOMAIN_ATTR_MSI_RESV attribute to decide whether to set the MSI
aperture
v12 -> v13:
- use iommu_get_dma_msi_region_cookie
v9 -> v10
- use VFIO_IOVA_RESERVED_MSI enum value
v7 -> v8:
- use iommu_msi_set_aperture function. There is no notion of
unregistration anymore since the reserved msi slot remains
until the container gets closed.
v6 -> v7:
- use iommu_free_reserved_iova_domain
- convey prot attributes downto dma-reserved-iommu iova domain creation
- reserved bindings teardown now performed on iommu domain destruction
- rename VFIO_DMA_MAP_FLAG_MSI_RESERVED_IOVA into
VFIO_DMA_MAP_FLAG_RESERVED_MSI_IOVA
- change title
- pass the protection attribute to dma-reserved-iommu API
v3 -> v4:
- use iommu_alloc/free_reserved_iova_domain exported by dma-reserved-iommu
- protect vfio_register_reserved_iova_range implementation with
CONFIG_IOMMU_DMA_RESERVED
- handle unregistration by user-space and on vfio_iommu_type1 release
v1 -> v2:
- set returned value according to alloc_reserved_iova_domain result
- free the iova domains in case any error occurs
RFC v1 -> v1:
- takes into account Alex comments, based on
[RFC PATCH 1/6] vfio: Add interface for add/del reserved iova region:
- use the existing dma map/unmap ioctl interface with a flag to register
a reserved IOVA range. A single reserved iova region is allowed.
---
drivers/vfio/vfio_iommu_type1.c | 97 ++++++++++++++++++++++++++++++++++++++++-
include/uapi/linux/vfio.h | 10 ++++-
2 files changed, 105 insertions(+), 2 deletions(-)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 2108e2e..e0c97ef 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -441,6 +441,40 @@ static void vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma)
vfio_lock_acct(-unlocked);
}
+/**
+ * vfio_set_msi_aperture - Sets the msi aperture on all domains
+ * requesting MSI mapping
+ *
+ * @iommu: vfio iommu handle
+ * @iova: MSI window iova base address
+ * @size: size of the MSI reserved iova window
+ *
+ * Return: 0 if the MSI reserved region was set on at least one domain,
+ * negative value on failure
+ */
+static int vfio_set_msi_aperture(struct vfio_iommu *iommu,
+ dma_addr_t iova, size_t size)
+{
+ struct iommu_domain_msi_resv msi_resv;
+ struct vfio_domain *d;
+ int ret = -EINVAL;
+
+ list_for_each_entry(d, &iommu->domain_list, next) {
+ if (iommu_domain_get_attr(d->domain, DOMAIN_ATTR_MSI_RESV,
+ &msi_resv))
+ continue;
+ ret = iommu_get_dma_msi_region_cookie(d->domain, iova, size);
+ if (ret)
+ goto unfold;
+ }
+ return 0;
+unfold:
+ list_for_each_entry(d, &iommu->domain_list, next)
+ iommu_put_dma_cookie(d->domain);
+
+ return ret;
+}
+
static void vfio_remove_dma(struct vfio_iommu *iommu, struct vfio_dma *dma)
{
vfio_unmap_unpin(iommu, dma);
@@ -690,6 +724,63 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
return ret;
}
+static int vfio_register_msi_range(struct vfio_iommu *iommu,
+ struct vfio_iommu_type1_dma_map *map)
+{
+ dma_addr_t iova = map->iova;
+ size_t size = map->size;
+ int ret = 0;
+ struct vfio_dma *dma;
+ unsigned long order;
+ uint64_t mask;
+
+ /* Verify that none of our __u64 fields overflow */
+ if (map->size != size || map->iova != iova)
+ return -EINVAL;
+
+ order = __ffs(vfio_pgsize_bitmap(iommu));
+ mask = ((uint64_t)1 << order) - 1;
+
+ WARN_ON(mask & PAGE_MASK);
+
+ if (!size || (size | iova) & mask)
+ return -EINVAL;
+
+ /* Don't allow IOVA address wrap */
+ if (iova + size - 1 < iova)
+ return -EINVAL;
+
+ mutex_lock(&iommu->lock);
+
+ if (vfio_find_dma(iommu, iova, size, VFIO_IOVA_ANY)) {
+ ret = -EEXIST;
+ goto unlock;
+ }
+
+ dma = kzalloc(sizeof(*dma), GFP_KERNEL);
+ if (!dma) {
+ ret = -ENOMEM;
+ goto unlock;
+ }
+
+ dma->iova = iova;
+ dma->size = size;
+ dma->type = VFIO_IOVA_RESERVED_MSI;
+
+ ret = vfio_set_msi_aperture(iommu, iova, size);
+ if (ret)
+ goto free_unlock;
+
+ vfio_link_dma(iommu, dma);
+ goto unlock;
+
+free_unlock:
+ kfree(dma);
+unlock:
+ mutex_unlock(&iommu->lock);
+ return ret;
+}
+
static int vfio_bus_type(struct device *dev, void *data)
{
struct bus_type **bus = data;
@@ -1068,7 +1159,8 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
} else if (cmd == VFIO_IOMMU_MAP_DMA) {
struct vfio_iommu_type1_dma_map map;
uint32_t mask = VFIO_DMA_MAP_FLAG_READ |
- VFIO_DMA_MAP_FLAG_WRITE;
+ VFIO_DMA_MAP_FLAG_WRITE |
+ VFIO_DMA_MAP_FLAG_RESERVED_MSI_IOVA;
minsz = offsetofend(struct vfio_iommu_type1_dma_map, size);
@@ -1078,6 +1170,9 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
if (map.argsz < minsz || map.flags & ~mask)
return -EINVAL;
+ if (map.flags & VFIO_DMA_MAP_FLAG_RESERVED_MSI_IOVA)
+ return vfio_register_msi_range(iommu, &map);
+
return vfio_dma_do_map(iommu, &map);
} else if (cmd == VFIO_IOMMU_UNMAP_DMA) {
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 255a211..4a9dbc2 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -498,12 +498,19 @@ struct vfio_iommu_type1_info {
*
* Map process virtual addresses to IO virtual addresses using the
* provided struct vfio_dma_map. Caller sets argsz. READ &/ WRITE required.
+ *
+ * In case RESERVED_MSI_IOVA flag is set, the API only aims@registering an
+ * IOVA region that will be used on some platforms to map the host MSI frames.
+ * In that specific case, vaddr is ignored. Once registered, an MSI reserved
+ * IOVA region stays until the container is closed.
*/
struct vfio_iommu_type1_dma_map {
__u32 argsz;
__u32 flags;
#define VFIO_DMA_MAP_FLAG_READ (1 << 0) /* readable from device */
#define VFIO_DMA_MAP_FLAG_WRITE (1 << 1) /* writable from device */
+/* reserved iova for MSI vectors*/
+#define VFIO_DMA_MAP_FLAG_RESERVED_MSI_IOVA (1 << 2)
__u64 vaddr; /* Process virtual address */
__u64 iova; /* IO virtual address */
__u64 size; /* Size of mapping (bytes) */
@@ -519,7 +526,8 @@ struct vfio_iommu_type1_dma_map {
* Caller sets argsz. The actual unmapped size is returned in the size
* field. No guarantee is made to the user that arbitrary unmaps of iova
* or size different from those used in the original mapping call will
- * succeed.
+ * succeed. Once registered, an MSI region cannot be unmapped and stays
+ * until the container is closed.
*/
struct vfio_iommu_type1_dma_unmap {
__u32 argsz;
--
1.9.1
^ permalink raw reply related
* [PATCH v14 12/16] vfio/type1: Handle unmap/unpin and replay for VFIO_IOVA_RESERVED slots
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>
Before allowing the end-user to create VFIO_IOVA_RESERVED dma slots,
let's implement the expected behavior for removal and replay.
As opposed to user dma slots, reserved IOVAs are not systematically bound
to PAs and PAs are not pinned. VFIO just initializes the IOVA "aperture".
IOVAs are allocated outside of the VFIO framework, by the MSI layer which
is responsible to free and unmap them. The MSI mapping resources are freed
by the IOMMU driver on domain destruction.
On the creation of a new domain, the "replay" of a reserved slot simply
needs to set the MSI aperture on the new domain.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v13 -> v14:
- make iommu_get_dma_msi_region_cookie's failure not passable
- remove useless "select IOMMU_DMA" causing cyclic dependency
- set the MSI region only if needed
v12 -> v13:
- use dma-iommu iommu_get_dma_msi_region_cookie
v9 -> v10:
- replay of a reserved slot sets the MSI aperture on the new domain
- use VFIO_IOVA_RESERVED_MSI enum value instead of VFIO_IOVA_RESERVED
v7 -> v8:
- do no destroy anything anymore, just bypass unmap/unpin and iommu_map
on replay
---
drivers/vfio/vfio_iommu_type1.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 1f120f9..2108e2e 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -36,6 +36,7 @@
#include <linux/uaccess.h>
#include <linux/vfio.h>
#include <linux/workqueue.h>
+#include <linux/dma-iommu.h>
#define DRIVER_VERSION "0.2"
#define DRIVER_AUTHOR "Alex Williamson <alex.williamson@redhat.com>"
@@ -386,7 +387,7 @@ static void vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma)
struct vfio_domain *domain, *d;
long unlocked = 0;
- if (!dma->size)
+ if (!dma->size || dma->type != VFIO_IOVA_USER)
return;
/*
* We use the IOMMU to track the physical addresses, otherwise we'd
@@ -717,12 +718,24 @@ static int vfio_iommu_replay(struct vfio_iommu *iommu,
return -EINVAL;
for (; n; n = rb_next(n)) {
+ struct iommu_domain_msi_resv msi_resv;
struct vfio_dma *dma;
dma_addr_t iova;
dma = rb_entry(n, struct vfio_dma, node);
iova = dma->iova;
+ if ((dma->type == VFIO_IOVA_RESERVED_MSI) &&
+ (!iommu_domain_get_attr(domain->domain,
+ DOMAIN_ATTR_MSI_RESV,
+ &msi_resv))) {
+ ret = iommu_get_dma_msi_region_cookie(domain->domain,
+ dma->iova,
+ dma->size);
+ if (ret)
+ return ret;
+ }
+
while (iova < dma->iova + dma->size) {
phys_addr_t phys = iommu_iova_to_phys(d->domain, iova);
size_t size;
--
1.9.1
^ permalink raw reply related
* [PATCH v14 11/16] vfio/type1: Implement recursive vfio_find_dma_from_node
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>
This patch handles the case where a node is encountered, matching
@start and @size arguments but not matching the @type argument.
In that case, we need to skip that node and pursue the search in the
node's leaves. In case @start is inferior to the node's base, we
resume the search on the left leaf. If the recursive search on the left
leaves did not produce any match, we search the right leaves recursively.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Acked-by: Alex Williamson <alex.williamson@redhat.com>
---
v10: creation
---
drivers/vfio/vfio_iommu_type1.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 1bd16ff..1f120f9 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -125,7 +125,17 @@ static struct vfio_dma *vfio_find_dma_from_node(struct rb_node *top,
if (type == VFIO_IOVA_ANY || dma->type == type)
return dma;
- return NULL;
+ /* restart 2 searches skipping the current node */
+ if (start < dma->iova) {
+ dma = vfio_find_dma_from_node(node->rb_left, start,
+ size, type);
+ if (dma)
+ return dma;
+ }
+ if (start + size > dma->iova + dma->size)
+ dma = vfio_find_dma_from_node(node->rb_right, start,
+ size, type);
+ return dma;
}
/**
--
1.9.1
^ permalink raw reply related
* [PATCH v14 10/16] vfio/type1: vfio_find_dma accepting a type argument
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>
In our RB-tree we get prepared to insert slots of different types
(USER and RESERVED). It becomes useful to be able to search for dma
slots of a specific type or any type.
This patch introduces vfio_find_dma_from_node which starts the
search from a given node and stops on the first node that matches
the @start and @size parameters. If this node also matches the
@type parameter, the node is returned else NULL is returned.
At the moment we only have USER SLOTS so the type will always match.
In a separate patch, this function will be enhanced to pursue the
search recursively in case a node with a different type is
encountered.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v13 -> v14:
- remove top_node variable
---
drivers/vfio/vfio_iommu_type1.c | 52 +++++++++++++++++++++++++++++++++--------
1 file changed, 42 insertions(+), 10 deletions(-)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index a9f8b93..1bd16ff 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -94,25 +94,55 @@ struct vfio_group {
* into DMA'ble space using the IOMMU
*/
-static struct vfio_dma *vfio_find_dma(struct vfio_iommu *iommu,
- dma_addr_t start, size_t size)
+/**
+ * vfio_find_dma_from_node: looks for a dma slot intersecting a window
+ * from a given rb tree node
+ * @top: top rb tree node where the search starts (including this node)
+ * @start: window start
+ * @size: window size
+ * @type: window type
+ */
+static struct vfio_dma *vfio_find_dma_from_node(struct rb_node *top,
+ dma_addr_t start, size_t size,
+ enum vfio_iova_type type)
{
- struct rb_node *node = iommu->dma_list.rb_node;
+ struct rb_node *node = top;
+ struct vfio_dma *dma;
while (node) {
- struct vfio_dma *dma = rb_entry(node, struct vfio_dma, node);
-
+ dma = rb_entry(node, struct vfio_dma, node);
if (start + size <= dma->iova)
node = node->rb_left;
else if (start >= dma->iova + dma->size)
node = node->rb_right;
else
- return dma;
+ break;
}
+ if (!node)
+ return NULL;
+
+ /* a dma slot intersects our window, check the type also matches */
+ if (type == VFIO_IOVA_ANY || dma->type == type)
+ return dma;
return NULL;
}
+/**
+ * vfio_find_dma: find a dma slot intersecting a given window
+ * @iommu: vfio iommu handle
+ * @start: window base iova
+ * @size: window size
+ * @type: window type
+ */
+static struct vfio_dma *vfio_find_dma(struct vfio_iommu *iommu,
+ dma_addr_t start, size_t size,
+ enum vfio_iova_type type)
+{
+ return vfio_find_dma_from_node(iommu->dma_list.rb_node,
+ start, size, type);
+}
+
static void vfio_link_dma(struct vfio_iommu *iommu, struct vfio_dma *new)
{
struct rb_node **link = &iommu->dma_list.rb_node, *parent = NULL;
@@ -484,19 +514,21 @@ static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
* mappings within the range.
*/
if (iommu->v2) {
- dma = vfio_find_dma(iommu, unmap->iova, 0);
+ dma = vfio_find_dma(iommu, unmap->iova, 0, VFIO_IOVA_USER);
if (dma && dma->iova != unmap->iova) {
ret = -EINVAL;
goto unlock;
}
- dma = vfio_find_dma(iommu, unmap->iova + unmap->size - 1, 0);
+ dma = vfio_find_dma(iommu, unmap->iova + unmap->size - 1, 0,
+ VFIO_IOVA_USER);
if (dma && dma->iova + dma->size != unmap->iova + unmap->size) {
ret = -EINVAL;
goto unlock;
}
}
- while ((dma = vfio_find_dma(iommu, unmap->iova, unmap->size))) {
+ while ((dma = vfio_find_dma(iommu, unmap->iova, unmap->size,
+ VFIO_IOVA_USER))) {
if (!iommu->v2 && unmap->iova > dma->iova)
break;
unmapped += dma->size;
@@ -600,7 +632,7 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
mutex_lock(&iommu->lock);
- if (vfio_find_dma(iommu, iova, size)) {
+ if (vfio_find_dma(iommu, iova, size, VFIO_IOVA_ANY)) {
mutex_unlock(&iommu->lock);
return -EEXIST;
}
--
1.9.1
^ permalink raw reply related
* [PATCH v14 09/16] vfio: Introduce a vfio_dma type field
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>
We introduce a vfio_dma type since we will need to discriminate
different types of dma slots:
- VFIO_IOVA_USER: IOVA region used to map user vaddr
- VFIO_IOVA_RESERVED_MSI: IOVA region reserved to map MSI doorbells
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Acked-by: Alex Williamson <alex.williamson@redhat.com>
---
v9 -> v10:
- renamed VFIO_IOVA_RESERVED into VFIO_IOVA_RESERVED_MSI
- explicitly set type to VFIO_IOVA_USER on dma_map
v6 -> v7:
- add VFIO_IOVA_ANY
- do not introduce yet any VFIO_IOVA_RESERVED handling
---
drivers/vfio/vfio_iommu_type1.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 2ba1942..a9f8b93 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -53,6 +53,12 @@ module_param_named(disable_hugepages,
MODULE_PARM_DESC(disable_hugepages,
"Disable VFIO IOMMU support for IOMMU hugepages.");
+enum vfio_iova_type {
+ VFIO_IOVA_USER = 0, /* standard IOVA used to map user vaddr */
+ VFIO_IOVA_RESERVED_MSI, /* reserved to map MSI doorbells */
+ VFIO_IOVA_ANY, /* matches any IOVA type */
+};
+
struct vfio_iommu {
struct list_head domain_list;
struct mutex lock;
@@ -75,6 +81,7 @@ struct vfio_dma {
unsigned long vaddr; /* Process virtual addr */
size_t size; /* Map size (bytes) */
int prot; /* IOMMU_READ/WRITE */
+ enum vfio_iova_type type; /* type of IOVA */
};
struct vfio_group {
@@ -607,6 +614,7 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
dma->iova = iova;
dma->vaddr = vaddr;
dma->prot = prot;
+ dma->type = VFIO_IOVA_USER;
/* Insert zero-sized and grow as we map chunks of it */
vfio_link_dma(iommu, dma);
--
1.9.1
^ permalink raw reply related
* [PATCH v14 08/16] irqchip/gicv3-its: Register the MSI doorbell
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>
This patch registers the ITS global doorbell. Registered information
are needed to set up the KVM passthrough use case.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v13 -> v14:
- use iommu_msi_doorbell_alloc/free
v12 -> v13:
- use new doorbell registration prototype
v11 -> v12:
- use new irq_get_msi_doorbell_info name
- simplify error handling
v10 -> v11:
- adapt to new doorbell registration API and implement msi_doorbell_info
---
drivers/irqchip/irq-gic-v3-its.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 98ff669..b42e006 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -86,6 +86,7 @@ struct its_node {
u32 ite_size;
u32 device_ids;
int numa_node;
+ struct iommu_msi_doorbell_info *doorbell_info;
};
#define ITS_ITT_ALIGN SZ_256
@@ -1717,6 +1718,7 @@ static int __init its_probe(struct device_node *node,
if (of_property_read_bool(node, "msi-controller")) {
struct msi_domain_info *info;
+ phys_addr_t translater;
info = kzalloc(sizeof(*info), GFP_KERNEL);
if (!info) {
@@ -1724,10 +1726,21 @@ static int __init its_probe(struct device_node *node,
goto out_free_tables;
}
+ translater = its->phys_base + GITS_TRANSLATER;
+ its->doorbell_info =
+ iommu_msi_doorbell_alloc(translater, sizeof(u32), true);
+
+ if (IS_ERR(its->doorbell_info)) {
+ kfree(info);
+ goto out_free_tables;
+ }
+
+
inner_domain = irq_domain_add_tree(node, &its_domain_ops, its);
if (!inner_domain) {
err = -ENOMEM;
kfree(info);
+ iommu_msi_doorbell_free(its->doorbell_info);
goto out_free_tables;
}
--
1.9.1
^ permalink raw reply related
* [PATCH v14 07/16] irqchip/gic-v2m: Register the MSI doorbell
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>
Register the GIC V2M global doorbell. The registered information
are used to set up the KVM passthrough use case.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v13 -> v14:
- use iommu_msi_doorbell_alloc/free
v12 -> v13:
- use new msi doorbell registration prototype
- remove iommu protection attributes
- add unregistration in teardown
v11 -> v12:
- use irq_get_msi_doorbell_info new name
- simplify error handling
v10 -> v11:
- use the new registration API and re-implement the msi_doorbell_info
ops
v9 -> v10:
- introduce the registration concept in place of msi_doorbell_info
callback
v8 -> v9:
- use global_doorbell instead of percpu_doorbells
v7 -> v8:
- gicv2m_msi_doorbell_info does not return a pointer to const
- remove spurious !v2m check
- add IOMMU_MMIO flag
v7: creation
---
drivers/irqchip/irq-gic-v2m.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index 863e073..33acfe0 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -70,6 +70,7 @@ struct v2m_data {
u32 spi_offset; /* offset to be subtracted from SPI number */
unsigned long *bm; /* MSI vector bitmap */
u32 flags; /* v2m flags for specific implementation */
+ struct iommu_msi_doorbell_info *doorbell_info; /* MSI doorbell */
};
static void gicv2m_mask_msi_irq(struct irq_data *d)
@@ -254,6 +255,7 @@ static void gicv2m_teardown(void)
struct v2m_data *v2m, *tmp;
list_for_each_entry_safe(v2m, tmp, &v2m_nodes, entry) {
+ iommu_msi_doorbell_free(v2m->doorbell_info);
list_del(&v2m->entry);
kfree(v2m->bm);
iounmap(v2m->base);
@@ -370,12 +372,18 @@ static int __init gicv2m_init_one(struct fwnode_handle *fwnode,
goto err_iounmap;
}
+ v2m->doorbell_info = iommu_msi_doorbell_alloc(v2m->res.start,
+ sizeof(u32), false);
+ if (IS_ERR(v2m->doorbell_info))
+ goto err_free_bm;
+
list_add_tail(&v2m->entry, &v2m_nodes);
pr_info("range%pR, SPI[%d:%d]\n", res,
v2m->spi_start, (v2m->spi_start + v2m->nr_spis - 1));
return 0;
-
+err_free_bm:
+ kfree(v2m->bm);
err_iounmap:
iounmap(v2m->base);
err_free_v2m:
--
1.9.1
^ permalink raw reply related
* [PATCH v14 06/16] iommu/arm-smmu: Implement domain_get_attr for DOMAIN_ATTR_MSI_RESV
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>
ARM smmu and smmu-v3 translate MSI transactions so their driver
are must implement domain_get_attr for DOMAIN_ATTR_MSI_RESV.
This allows to retrieve the size and alignment requirements of
the MSI reserved IOVA window.
Also IOMMU_DMA gets selected since it exposes the API to map the
MSIs.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
drivers/iommu/Kconfig | 4 ++--
drivers/iommu/arm-smmu-v3.c | 7 +++++++
drivers/iommu/arm-smmu.c | 7 +++++++
3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 8ee54d7..f5e5e4b 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -297,7 +297,7 @@ config SPAPR_TCE_IOMMU
config ARM_SMMU
bool "ARM Ltd. System MMU (SMMU) Support"
depends on (ARM64 || ARM) && MMU
- select IOMMU_API
+ select IOMMU_DMA
select IOMMU_IO_PGTABLE_LPAE
select ARM_DMA_USE_IOMMU if ARM
help
@@ -310,7 +310,7 @@ config ARM_SMMU
config ARM_SMMU_V3
bool "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
depends on ARM64
- select IOMMU_API
+ select IOMMU_DMA
select IOMMU_IO_PGTABLE_LPAE
select GENERIC_MSI_IRQ_DOMAIN
help
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 15c01c3..572cad8 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1559,6 +1559,9 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain)
if (ret < 0)
free_io_pgtable_ops(pgtbl_ops);
+ if (domain->type == IOMMU_DOMAIN_UNMANAGED)
+ iommu_calc_msi_resv(domain);
+
return ret;
}
@@ -1840,6 +1843,10 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
case DOMAIN_ATTR_NESTING:
*(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED);
return 0;
+ case DOMAIN_ATTR_MSI_RESV:
+ *(struct iommu_domain_msi_resv *)data =
+ smmu_domain->domain.msi_resv;
+ return 0;
default:
return -ENODEV;
}
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index ac4aab9..ae20b9c 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -943,6 +943,9 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
domain->geometry.aperture_end = (1UL << ias) - 1;
domain->geometry.force_aperture = true;
+ if (domain->type == IOMMU_DOMAIN_UNMANAGED)
+ iommu_calc_msi_resv(domain);
+
/* Initialise the context bank with our page table cfg */
arm_smmu_init_context_bank(smmu_domain, &pgtbl_cfg);
@@ -1486,6 +1489,10 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
case DOMAIN_ATTR_NESTING:
*(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED);
return 0;
+ case DOMAIN_ATTR_MSI_RESV:
+ *(struct iommu_domain_msi_resv *)data =
+ smmu_domain->domain.msi_resv;
+ return 0;
default:
return -ENODEV;
}
--
1.9.1
^ permalink raw reply related
* [PATCH v14 05/16] iommu/dma: Introduce iommu_calc_msi_resv
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>
iommu_calc_msi_resv() sum up the number of iommu pages of the lowest
order supported by the iommu domain requested to map all the registered
doorbells. This function will allow to dimension the intermediate
physical address (IPA) aperture requested to map the MSI doorbells.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v13 -> v14
- name and proto changed, moved to dma-iommu
---
drivers/iommu/dma-iommu.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/dma-iommu.h | 11 +++++++-
2 files changed, 80 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index d8a7d86..3a4b73b 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -830,3 +830,73 @@ bool iommu_msi_doorbell_safe(void)
return !nb_unsafe_doorbells;
}
EXPORT_SYMBOL_GPL(iommu_msi_doorbell_safe);
+
+/**
+ * calc_region_reqs - compute the number of pages requested to map a region
+ *
+ * @addr: physical base address of the region
+ * @size: size of the region
+ * @order: the page order
+ *
+ * Return: the number of requested pages to map this region
+ */
+static int calc_region_reqs(phys_addr_t addr, size_t size, unsigned int order)
+{
+ phys_addr_t offset, granule;
+ unsigned int nb_pages;
+
+ granule = (uint64_t)(1 << order);
+ offset = addr & (granule - 1);
+ size = ALIGN(size + offset, granule);
+ nb_pages = size >> order;
+
+ return nb_pages;
+}
+
+/**
+ * calc_dbinfo_reqs - compute the number of pages requested to map a given
+ * MSI doorbell
+ *
+ * @dbi: doorbell info descriptor
+ * @order: page order
+ *
+ * Return: the number of requested pages to map this doorbell
+ */
+static int calc_dbinfo_reqs(struct iommu_msi_doorbell_info *dbi,
+ unsigned int order)
+{
+ int ret = 0;
+
+ if (!dbi->doorbell_is_percpu) {
+ ret = calc_region_reqs(dbi->global_doorbell, dbi->size, order);
+ } else {
+ phys_addr_t __percpu *pbase;
+ int cpu;
+
+ for_each_possible_cpu(cpu) {
+ pbase = per_cpu_ptr(dbi->percpu_doorbells, cpu);
+ ret += calc_region_reqs(*pbase, dbi->size, order);
+ }
+ }
+ return ret;
+}
+
+int iommu_calc_msi_resv(struct iommu_domain *domain)
+{
+ unsigned long order = __ffs(domain->pgsize_bitmap);
+ struct iommu_msi_doorbell *db;
+ phys_addr_t granule;
+ int size = 0;
+
+ mutex_lock(&iommu_msi_doorbell_mutex);
+ list_for_each_entry(db, &iommu_msi_doorbell_list, next)
+ size += calc_dbinfo_reqs(&db->info, order);
+
+ mutex_unlock(&iommu_msi_doorbell_mutex);
+
+ granule = (uint64_t)(1 << order);
+ domain->msi_resv.size = size * granule;
+ domain->msi_resv.alignment = granule;
+
+ return 0;
+}
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 9640a27..95875c8 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -97,6 +97,16 @@ void iommu_msi_doorbell_free(struct iommu_msi_doorbell_info *db);
*/
bool iommu_msi_doorbell_safe(void);
+/**
+ * iommu_calc_msi_resv - compute the number of pages of the lowest order
+ * supported by @domain, requested to map all the registered doorbells.
+ *
+ * @domain: iommu_domain
+ * @msi_resv: MSI reserved window requirements
+ *
+ */
+int iommu_calc_msi_resv(struct iommu_domain *domain);
+
#else
struct iommu_domain;
@@ -139,7 +149,6 @@ static inline bool iommu_msi_doorbell_safe(void)
{
return false;
}
-
#endif /* CONFIG_IOMMU_DMA */
#endif /* __KERNEL__ */
#endif /* __DMA_IOMMU_H */
--
1.9.1
^ permalink raw reply related
* [PATCH v14 04/16] iommu/dma: MSI doorbell alloc/free
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>
We introduce the capability to (un)register MSI doorbells.
A doorbell region is characterized by its physical address base, size,
and whether it its safe (ie. it implements IRQ remapping). A doorbell
can be per-cpu or global. We currently only care about global doorbells.
A function returns whether all registered doorbells are safe.
MSI controllers likely to work along with IOMMU that translate MSI
transaction must register their doorbells to allow device assignment
with MSI support. Otherwise the MSI transactions will cause IOMMU faults.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v13 -> v14:
- previously in msi-doorbell.h/c
---
drivers/iommu/dma-iommu.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/dma-iommu.h | 41 ++++++++++++++++++++++++++
2 files changed, 116 insertions(+)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index d45f9a0..d8a7d86 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -43,6 +43,38 @@ struct iommu_dma_cookie {
spinlock_t msi_lock;
};
+/**
+ * struct iommu_msi_doorbell_info - MSI doorbell region descriptor
+ * @percpu_doorbells: per cpu doorbell base address
+ * @global_doorbell: base address of the doorbell
+ * @doorbell_is_percpu: is the doorbell per cpu or global?
+ * @safe: true if irq remapping is implemented
+ * @size: size of the doorbell
+ */
+struct iommu_msi_doorbell_info {
+ union {
+ phys_addr_t __percpu *percpu_doorbells;
+ phys_addr_t global_doorbell;
+ };
+ bool doorbell_is_percpu;
+ bool safe;
+ size_t size;
+};
+
+struct iommu_msi_doorbell {
+ struct iommu_msi_doorbell_info info;
+ struct list_head next;
+};
+
+/* list of registered MSI doorbells */
+static LIST_HEAD(iommu_msi_doorbell_list);
+
+/* counts the number of unsafe registered doorbells */
+static uint nb_unsafe_doorbells;
+
+/* protects the list and nb_unsafe_doorbells */
+static DEFINE_MUTEX(iommu_msi_doorbell_mutex);
+
static inline struct iova_domain *cookie_iovad(struct iommu_domain *domain)
{
return &((struct iommu_dma_cookie *)domain->iova_cookie)->iovad;
@@ -755,3 +787,46 @@ int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
return 0;
}
EXPORT_SYMBOL(iommu_get_dma_msi_region_cookie);
+
+struct iommu_msi_doorbell_info *
+iommu_msi_doorbell_alloc(phys_addr_t base, size_t size, bool safe)
+{
+ struct iommu_msi_doorbell *db;
+
+ db = kzalloc(sizeof(*db), GFP_KERNEL);
+ if (!db)
+ return ERR_PTR(-ENOMEM);
+
+ db->info.global_doorbell = base;
+ db->info.size = size;
+ db->info.safe = safe;
+
+ mutex_lock(&iommu_msi_doorbell_mutex);
+ list_add(&db->next, &iommu_msi_doorbell_list);
+ if (!db->info.safe)
+ nb_unsafe_doorbells++;
+ mutex_unlock(&iommu_msi_doorbell_mutex);
+ return &db->info;
+}
+EXPORT_SYMBOL_GPL(iommu_msi_doorbell_alloc);
+
+void iommu_msi_doorbell_free(struct iommu_msi_doorbell_info *dbinfo)
+{
+ struct iommu_msi_doorbell *db;
+
+ db = container_of(dbinfo, struct iommu_msi_doorbell, info);
+
+ mutex_lock(&iommu_msi_doorbell_mutex);
+ list_del(&db->next);
+ if (!db->info.safe)
+ nb_unsafe_doorbells--;
+ mutex_unlock(&iommu_msi_doorbell_mutex);
+ kfree(db);
+}
+EXPORT_SYMBOL_GPL(iommu_msi_doorbell_free);
+
+bool iommu_msi_doorbell_safe(void)
+{
+ return !nb_unsafe_doorbells;
+}
+EXPORT_SYMBOL_GPL(iommu_msi_doorbell_safe);
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 05ab5b4..9640a27 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -19,6 +19,8 @@
#ifdef __KERNEL__
#include <asm/errno.h>
+struct iommu_msi_doorbell_info;
+
#ifdef CONFIG_IOMMU_DMA
#include <linux/iommu.h>
#include <linux/msi.h>
@@ -70,6 +72,31 @@ void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg);
int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
dma_addr_t base, u64 size);
+/**
+ * iommu_msi_doorbell_alloc - allocate a global doorbell
+ * @base: physical base address of the doorbell
+ * @size: size of the doorbell
+ * @safe: true is irq_remapping implemented for this doorbell
+ *
+ * Return: the newly allocated doorbell info or a pointer converted error
+ */
+struct iommu_msi_doorbell_info *
+iommu_msi_doorbell_alloc(phys_addr_t base, size_t size, bool safe);
+
+/**
+ * iommu_msi_doorbell_free - free a global doorbell
+ * @db: doorbell info to free
+ */
+void iommu_msi_doorbell_free(struct iommu_msi_doorbell_info *db);
+
+/**
+ * iommu_msi_doorbell_safe - return whether all registered doorbells are safe
+ *
+ * Safe doorbells are those which implement irq remapping
+ * Return: true if all doorbells are safe, false otherwise
+ */
+bool iommu_msi_doorbell_safe(void);
+
#else
struct iommu_domain;
@@ -99,6 +126,20 @@ static inline int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
return -ENODEV;
}
+static inline struct iommu_msi_doorbell_info *
+iommu_msi_doorbell_alloc(phys_addr_t base, size_t size, bool safe)
+{
+ return NULL;
+}
+
+static inline void
+iommu_msi_doorbell_free(struct msi_doorbell_info *db) {}
+
+static inline bool iommu_msi_doorbell_safe(void)
+{
+ return false;
+}
+
#endif /* CONFIG_IOMMU_DMA */
#endif /* __KERNEL__ */
#endif /* __DMA_IOMMU_H */
--
1.9.1
^ permalink raw reply related
* [PATCH v14 03/16] iommu/dma: Allow MSI-only cookies
From: Eric Auger @ 2016-10-12 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476278544-3397-1-git-send-email-eric.auger@redhat.com>
From: Robin Murphy <robin.murphy@arm.com>
IOMMU domain users such as VFIO face a similar problem to DMA API ops
with regard to mapping MSI messages in systems where the MSI write is
subject to IOMMU translation. With the relevant infrastructure now in
place for managed DMA domains, it's actually really simple for other
users to piggyback off that and reap the benefits without giving up
their own IOVA management, and without having to reinvent their own
wheel in the MSI layer.
Allow such users to opt into automatic MSI remapping by dedicating a
region of their IOVA space to a managed cookie.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v13 -> v14:
- restore reserve_iova for iova >= base + size
v1 -> v13 incorpration:
- compared to Robin's version
- add NULL last param to iommu_dma_init_domain
- set the msi_geometry aperture
- I removed
if (base < U64_MAX - size)
reserve_iova(iovad, iova_pfn(iovad, base + size), ULONG_MAX);
don't get why we would reserve something out of the scope of the iova domain?
what do I miss?
---
drivers/iommu/dma-iommu.c | 39 +++++++++++++++++++++++++++++++++++++++
include/linux/dma-iommu.h | 9 +++++++++
2 files changed, 48 insertions(+)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index c5ab866..d45f9a0 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -716,3 +716,42 @@ void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
msg->address_lo += lower_32_bits(msi_page->iova);
}
}
+
+/**
+ * iommu_get_dma_msi_region_cookie - Configure a domain for MSI remapping only
+ * @domain: IOMMU domain to prepare
+ * @base: Base address of IOVA region to use as the MSI remapping aperture
+ * @size: Size of the desired MSI aperture
+ *
+ * Users who manage their own IOVA allocation and do not want DMA API support,
+ * but would still like to take advantage of automatic MSI remapping, can use
+ * this to initialise their own domain appropriately.
+ */
+int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
+ dma_addr_t base, u64 size)
+{
+ struct iommu_dma_cookie *cookie;
+ struct iova_domain *iovad;
+ int ret;
+
+ if (domain->type == IOMMU_DOMAIN_DMA)
+ return -EINVAL;
+
+ ret = iommu_get_dma_cookie(domain);
+ if (ret)
+ return ret;
+
+ ret = iommu_dma_init_domain(domain, base, size, NULL);
+ if (ret) {
+ iommu_put_dma_cookie(domain);
+ return ret;
+ }
+
+ cookie = domain->iova_cookie;
+ iovad = &cookie->iovad;
+ if (base < U64_MAX - size)
+ reserve_iova(iovad, iova_pfn(iovad, base + size), ULONG_MAX);
+
+ return 0;
+}
+EXPORT_SYMBOL(iommu_get_dma_msi_region_cookie);
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 32c5890..05ab5b4 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -67,6 +67,9 @@ int iommu_dma_mapping_error(struct device *dev, dma_addr_t dma_addr);
/* The DMA API isn't _quite_ the whole story, though... */
void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg);
+int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
+ dma_addr_t base, u64 size);
+
#else
struct iommu_domain;
@@ -90,6 +93,12 @@ static inline void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
{
}
+static inline int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
+ dma_addr_t base, u64 size)
+{
+ return -ENODEV;
+}
+
#endif /* CONFIG_IOMMU_DMA */
#endif /* __KERNEL__ */
#endif /* __DMA_IOMMU_H */
--
1.9.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox