* [PATCH v12 5/5] arm64: Allow huge io mappings again
From: Will Deacon @ 2018-06-04 12:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527856758-27169-6-git-send-email-cpandya@codeaurora.org>
On Fri, Jun 01, 2018 at 06:09:18PM +0530, Chintan Pandya wrote:
> Huge mappings have had stability issues due to stale
> TLB entry and memory leak issues. Since, those are
> addressed in this series of patches, it is now safe
> to allow huge mappings.
>
> Signed-off-by: Chintan Pandya <cpandya@codeaurora.org>
> ---
> arch/arm64/mm/mmu.c | 18 ++----------------
> 1 file changed, 2 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 6e7e16c..c65abc4 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -934,15 +934,8 @@ int pud_set_huge(pud_t *pudp, phys_addr_t phys, pgprot_t prot)
> {
> pgprot_t sect_prot = __pgprot(PUD_TYPE_SECT |
> pgprot_val(mk_sect_prot(prot)));
> - pud_t new_pud = pfn_pud(__phys_to_pfn(phys), sect_prot);
> -
> - /* Only allow permission changes for now */
> - if (!pgattr_change_is_safe(READ_ONCE(pud_val(*pudp)),
> - pud_val(new_pud)))
> - return 0;
Do you actually need to remove these checks? If we're doing
break-before-make properly, then the check won't fire but it would be
good to keep it there so we can catch misuse of these in future.
In other words, can we drop this patch?
Will
^ permalink raw reply
* [PATCH v12 3/5] arm64: pgtable: Add p*d_page_vaddr helper macros
From: Will Deacon @ 2018-06-04 12:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527856758-27169-4-git-send-email-cpandya@codeaurora.org>
On Fri, Jun 01, 2018 at 06:09:16PM +0530, Chintan Pandya wrote:
> Add helper macros to give virtual references to page
> tables. These will be used while freeing dangling
> page tables.
>
> Signed-off-by: Chintan Pandya <cpandya@codeaurora.org>
> ---
> arch/arm64/include/asm/pgtable.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index 7c4c8f3..ef4047f 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -580,6 +580,9 @@ static inline phys_addr_t pgd_page_paddr(pgd_t pgd)
>
> #endif /* CONFIG_PGTABLE_LEVELS > 3 */
>
> +#define pmd_page_vaddr(pmd) __va(pmd_page_paddr(pmd))
> +#define pud_page_vaddr(pud) __va(pud_page_paddr(pud))
Are these actually needed, or do pte_offset_kernel and pmd_offset do the
job already?
Will
^ permalink raw reply
* [PATCH v12 4/5] arm64: Implement page table free interfaces
From: Will Deacon @ 2018-06-04 12:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527856758-27169-5-git-send-email-cpandya@codeaurora.org>
On Fri, Jun 01, 2018 at 06:09:17PM +0530, Chintan Pandya wrote:
> Implement pud_free_pmd_page() and pmd_free_pte_page().
>
> Implementation requires,
> 1) Clearing off the current pud/pmd entry
> 2) Invalidate TLB which could have previously
> valid but not stale entry
> 3) Freeing of the un-used next level page tables
Please can you rewrite this describing the problem that you're solving,
rather than a brief summary of some requirements?
> Signed-off-by: Chintan Pandya <cpandya@codeaurora.org>
> ---
> arch/arm64/mm/mmu.c | 38 ++++++++++++++++++++++++++++++++++----
> 1 file changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 8ae5d7a..6e7e16c 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -45,6 +45,7 @@
> #include <asm/memblock.h>
> #include <asm/mmu_context.h>
> #include <asm/ptdump.h>
> +#include <asm/tlbflush.h>
>
> #define NO_BLOCK_MAPPINGS BIT(0)
> #define NO_CONT_MAPPINGS BIT(1)
> @@ -977,12 +978,41 @@ int pmd_clear_huge(pmd_t *pmdp)
> return 1;
> }
>
> -int pud_free_pmd_page(pud_t *pud, unsigned long addr)
> +int pmd_free_pte_page(pmd_t *pmdp, unsigned long addr)
> {
> - return pud_none(*pud);
> + pte_t *table;
> + pmd_t pmd;
> +
> + pmd = READ_ONCE(*pmdp);
> + if (pmd_present(pmd)) {
> + table = pmd_page_vaddr(pmd);
> + pmd_clear(pmdp);
> + __flush_tlb_kernel_pgtable(addr);
> + pte_free_kernel(NULL, table);
> + }
> + return 1;
> }
>
> -int pmd_free_pte_page(pmd_t *pmd, unsigned long addr)
> +int pud_free_pmd_page(pud_t *pudp, unsigned long addr)
> {
> - return pmd_none(*pmd);
> + pmd_t *table;
> + pmd_t *entry;
> + pud_t pud;
> + unsigned long next, end;
> +
> + pud = READ_ONCE(*pudp);
> + if (pud_present(pud)) {
Just some stylistic stuff, but please can you rewrite this as:
if (!pud_present(pud) || VM_WARN_ON(!pud_table(pud)))
return 1;
similarly for the pmd/pte code above.
> + table = pud_page_vaddr(pud);
> + entry = table;
Could you rename entry -> pmdp, please?
> + next = addr;
> + end = addr + PUD_SIZE;
> + do {
> + pmd_free_pte_page(entry, next);
> + } while (entry++, next += PMD_SIZE, next != end);
> +
> + pud_clear(pudp);
> + __flush_tlb_kernel_pgtable(addr);
> + pmd_free(NULL, table);
> + }
> + return 1;
So with these patches, we only ever return 1 from these helpers. It looks
like the same is true for x86, so how about we make them void and move the
calls inside the conditionals in lib/ioremap.c? Obviously, this would be a
separate patch on the end.
Will
^ permalink raw reply
* [PATCH v3 5/8] media: rcar-vin: Handle data-enable polarity
From: Niklas Söderlund @ 2018-06-04 12:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527606359-19261-6-git-send-email-jacopo+renesas@jmondi.org>
Hi Jacopo,
Thanks for your work.
On 2018-05-29 17:05:56 +0200, Jacopo Mondi wrote:
> Handle data-enable signal polarity. If the polarity is not specifically
> requested to be active low, use the active high default.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Acked-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> v3:
> - use new property to set the CES bit
> ---
> drivers/media/platform/rcar-vin/rcar-dma.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> index d2b7002..9145b56 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -123,6 +123,7 @@
> /* Video n Data Mode Register 2 bits */
> #define VNDMR2_VPS (1 << 30)
> #define VNDMR2_HPS (1 << 29)
> +#define VNDMR2_CES (1 << 28)
> #define VNDMR2_FTEV (1 << 17)
> #define VNDMR2_VLV(n) ((n & 0xf) << 12)
>
> @@ -698,6 +699,10 @@ static int rvin_setup(struct rvin_dev *vin)
> /* Vsync Signal Polarity Select */
> if (!(vin->parallel->mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
> dmr2 |= VNDMR2_VPS;
> +
> + /* Data Enable Polarity Select */
> + if (vin->parallel->mbus_flags & V4L2_MBUS_DATA_ENABLE_LOW)
> + dmr2 |= VNDMR2_CES;
> }
>
> /*
> --
> 2.7.4
>
--
Regards,
Niklas S?derlund
^ permalink raw reply
* [PATCH v3 4/8] dt-bindings: media: rcar-vin: Add 'data-enable-active'
From: Niklas Söderlund @ 2018-06-04 12:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527606359-19261-5-git-send-email-jacopo+renesas@jmondi.org>
Hi Jacopo,
Thanks for your patch.
On 2018-05-29 17:05:55 +0200, Jacopo Mondi wrote:
> Describe optional endpoint property 'data-enable-active' for R-Car VIN.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> v3:
> - new patch
> ---
>
> Documentation/devicetree/bindings/media/rcar_vin.txt | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index 4d91a36..ff53226 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -58,6 +58,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
> - Optional properties for endpoint nodes of port at 0:
> - hsync-active: see [1] for description. Default is active high.
> - vsync-active: see [1] for description. Default is active high.
> + - data-enable-active: polarity of CLKENB signal, see [1] for
> + description. Default is active high.
>
> If both HSYNC and VSYNC polarities are not specified, embedded
> synchronization is selected.
> --
> 2.7.4
>
--
Regards,
Niklas S?derlund
^ permalink raw reply
* [PATCH v3 3/8] media: v4l2-fwnode: parse 'data-enable-active' prop
From: Niklas Söderlund @ 2018-06-04 12:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527606359-19261-4-git-send-email-jacopo+renesas@jmondi.org>
Hi Jacopo,
Thanks for your patch.
On 2018-05-29 17:05:54 +0200, Jacopo Mondi wrote:
> Parse the newly defined 'data-enable-active' property in parallel endpoint
> parsing function.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
>
> ---
> v3:
> - new patch
> ---
> drivers/media/v4l2-core/v4l2-fwnode.c | 4 ++++
> include/media/v4l2-mediabus.h | 2 ++
> 2 files changed, 6 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 3f77aa3..6105191 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -154,6 +154,10 @@ static void v4l2_fwnode_endpoint_parse_parallel_bus(
> flags |= v ? V4L2_MBUS_VIDEO_SOG_ACTIVE_HIGH :
> V4L2_MBUS_VIDEO_SOG_ACTIVE_LOW;
>
> + if (!fwnode_property_read_u32(fwnode, "data-enable-active", &v))
> + flags |= v ? V4L2_MBUS_DATA_ENABLE_HIGH :
> + V4L2_MBUS_DATA_ENABLE_LOW;
> +
> bus->flags = flags;
>
> }
> diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
> index 4d8626c..4bbb5f3 100644
> --- a/include/media/v4l2-mediabus.h
> +++ b/include/media/v4l2-mediabus.h
> @@ -45,6 +45,8 @@
> /* Active state of Sync-on-green (SoG) signal, 0/1 for LOW/HIGH respectively. */
> #define V4L2_MBUS_VIDEO_SOG_ACTIVE_HIGH BIT(12)
> #define V4L2_MBUS_VIDEO_SOG_ACTIVE_LOW BIT(13)
> +#define V4L2_MBUS_DATA_ENABLE_HIGH BIT(14)
> +#define V4L2_MBUS_DATA_ENABLE_LOW BIT(15)
>
> /* Serial flags */
> /* How many lanes the client can use */
> --
> 2.7.4
>
--
Regards,
Niklas S?derlund
^ permalink raw reply
* [PATCH] rtc: sunxi: fix possible race condition
From: Alexandre Belloni @ 2018-06-04 12:05 UTC (permalink / raw)
To: linux-arm-kernel
The IRQ is requested before the struct rtc is allocated and registered, but
this struct is used in the IRQ handler. This may lead to a NULL pointer
dereference.
Switch to devm_rtc_allocate_device/rtc_register_device to allocate the rtc
before requesting the IRQ.
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
drivers/rtc/rtc-sunxi.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/rtc/rtc-sunxi.c b/drivers/rtc/rtc-sunxi.c
index dadbf8b324ad..21865d3d8fe8 100644
--- a/drivers/rtc/rtc-sunxi.c
+++ b/drivers/rtc/rtc-sunxi.c
@@ -445,6 +445,10 @@ static int sunxi_rtc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, chip);
chip->dev = &pdev->dev;
+ chip->rtc = devm_rtc_allocate_device(&pdev->dev);
+ if (IS_ERR(chip->rtc))
+ return PTR_ERR(chip->rtc);
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
chip->base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(chip->base))
@@ -481,11 +485,12 @@ static int sunxi_rtc_probe(struct platform_device *pdev)
writel(SUNXI_ALRM_IRQ_STA_CNT_IRQ_PEND, chip->base +
SUNXI_ALRM_IRQ_STA);
- chip->rtc = rtc_device_register("rtc-sunxi", &pdev->dev,
- &sunxi_rtc_ops, THIS_MODULE);
- if (IS_ERR(chip->rtc)) {
+ chip->rtc->ops = &sunxi_rtc_ops;
+
+ ret = rtc_register_device(chip->rtc);
+ if (ret) {
dev_err(&pdev->dev, "unable to register device\n");
- return PTR_ERR(chip->rtc);
+ return ret;
}
dev_info(&pdev->dev, "RTC enabled\n");
@@ -493,18 +498,8 @@ static int sunxi_rtc_probe(struct platform_device *pdev)
return 0;
}
-static int sunxi_rtc_remove(struct platform_device *pdev)
-{
- struct sunxi_rtc_dev *chip = platform_get_drvdata(pdev);
-
- rtc_device_unregister(chip->rtc);
-
- return 0;
-}
-
static struct platform_driver sunxi_rtc_driver = {
.probe = sunxi_rtc_probe,
- .remove = sunxi_rtc_remove,
.driver = {
.name = "sunxi-rtc",
.of_match_table = sunxi_rtc_dt_ids,
--
2.17.1
^ permalink raw reply related
* [PATCH 5/7] iommu/dma: add support for non-strict mode
From: Leizhen (ThunderTown) @ 2018-06-04 12:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <65cfe2f9-eb23-d81c-270e-ae80e96b6009@arm.com>
On 2018/5/31 21:04, Robin Murphy wrote:
> On 31/05/18 08:42, Zhen Lei wrote:
>> 1. Save the related domain pointer in struct iommu_dma_cookie, make iovad
>> capable call domain->ops->flush_iotlb_all to flush TLB.
>> 2. Define a new iommu capable: IOMMU_CAP_NON_STRICT, which used to indicate
>> that the iommu domain support non-strict mode.
>> 3. During the iommu domain initialization phase, call capable() to check
>> whether it support non-strcit mode. If so, call init_iova_flush_queue
>> to register iovad->flush_cb callback.
>> 4. All unmap(contains iova-free) APIs will finally invoke __iommu_dma_unmap
>> -->iommu_dma_free_iova. Use iovad->flush_cb to check whether its related
>> iommu support non-strict mode or not, and call IOMMU_DOMAIN_IS_STRICT to
>> make sure the IOMMU_DOMAIN_UNMANAGED domain always follow strict mode.
>
> Once again, this is a whole load of complexity for a property which could just be statically encoded at allocation, e.g. in the cookie type.
That's right. Pass domain to the static function iommu_dma_free_iova will be better.
>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>> drivers/iommu/dma-iommu.c | 29 ++++++++++++++++++++++++++---
>> include/linux/iommu.h | 3 +++
>> 2 files changed, 29 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>> index 4e885f7..2e116d9 100644
>> --- a/drivers/iommu/dma-iommu.c
>> +++ b/drivers/iommu/dma-iommu.c
>> @@ -55,6 +55,7 @@ struct iommu_dma_cookie {
>> };
>> struct list_head msi_page_list;
>> spinlock_t msi_lock;
>> + struct iommu_domain *domain;
>> };
>> static inline size_t cookie_msi_granule(struct iommu_dma_cookie *cookie)
>> @@ -64,7 +65,8 @@ static inline size_t cookie_msi_granule(struct iommu_dma_cookie *cookie)
>> return PAGE_SIZE;
>> }
>> -static struct iommu_dma_cookie *cookie_alloc(enum iommu_dma_cookie_type type)
>> +static struct iommu_dma_cookie *cookie_alloc(struct iommu_domain *domain,
>> + enum iommu_dma_cookie_type type)
>> {
>> struct iommu_dma_cookie *cookie;
>> @@ -73,6 +75,7 @@ static struct iommu_dma_cookie *cookie_alloc(enum iommu_dma_cookie_type type)
>> spin_lock_init(&cookie->msi_lock);
>> INIT_LIST_HEAD(&cookie->msi_page_list);
>> cookie->type = type;
>> + cookie->domain = domain;
>> }
>> return cookie;
>> }
>> @@ -94,7 +97,7 @@ int iommu_get_dma_cookie(struct iommu_domain *domain)
>> if (domain->iova_cookie)
>> return -EEXIST;
>> - domain->iova_cookie = cookie_alloc(IOMMU_DMA_IOVA_COOKIE);
>> + domain->iova_cookie = cookie_alloc(domain, IOMMU_DMA_IOVA_COOKIE);
>> if (!domain->iova_cookie)
>> return -ENOMEM;
>> @@ -124,7 +127,7 @@ int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
>> if (domain->iova_cookie)
>> return -EEXIST;
>> - cookie = cookie_alloc(IOMMU_DMA_MSI_COOKIE);
>> + cookie = cookie_alloc(domain, IOMMU_DMA_MSI_COOKIE);
>> if (!cookie)
>> return -ENOMEM;
>> @@ -261,6 +264,17 @@ static int iova_reserve_iommu_regions(struct device *dev,
>> return ret;
>> }
>> +static void iova_flush_iotlb_all(struct iova_domain *iovad)
>
> iommu_dma_flush...
OK
>
>> +{
>> + struct iommu_dma_cookie *cookie;
>> + struct iommu_domain *domain;
>> +
>> + cookie = container_of(iovad, struct iommu_dma_cookie, iovad);
>> + domain = cookie->domain;
>> +
>> + domain->ops->flush_iotlb_all(domain);
>> +}
>> +
>> /**
>> * iommu_dma_init_domain - Initialise a DMA mapping domain
>> * @domain: IOMMU domain previously prepared by iommu_get_dma_cookie()
>> @@ -276,6 +290,7 @@ static int iova_reserve_iommu_regions(struct device *dev,
>> int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>> u64 size, struct device *dev)
>> {
>> + const struct iommu_ops *ops = domain->ops;
>> struct iommu_dma_cookie *cookie = domain->iova_cookie;
>> struct iova_domain *iovad = &cookie->iovad;
>> unsigned long order, base_pfn, end_pfn;
>> @@ -313,6 +328,11 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>> init_iova_domain(iovad, 1UL << order, base_pfn);
>> + if (ops->capable && ops->capable(IOMMU_CAP_NON_STRICT)) {
>> + BUG_ON(!ops->flush_iotlb_all);
>> + init_iova_flush_queue(iovad, iova_flush_iotlb_all, NULL);
>> + }
>> +
>> return iova_reserve_iommu_regions(dev, domain);
>> }
>> EXPORT_SYMBOL(iommu_dma_init_domain);
>> @@ -392,6 +412,9 @@ static void iommu_dma_free_iova(struct iommu_dma_cookie *cookie,
>> /* The MSI case is only ever cleaning up its most recent allocation */
>> if (cookie->type == IOMMU_DMA_MSI_COOKIE)
>> cookie->msi_iova -= size;
>> + else if (!IOMMU_DOMAIN_IS_STRICT(cookie->domain) && iovad->flush_cb)
>> + queue_iova(iovad, iova_pfn(iovad, iova),
>> + size >> iova_shift(iovad), 0);
>> else
>> free_iova_fast(iovad, iova_pfn(iovad, iova),
>> size >> iova_shift(iovad));
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index 39b3150..01ff569 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -87,6 +87,8 @@ struct iommu_domain_geometry {
>> __IOMMU_DOMAIN_DMA_API)
>> #define IOMMU_STRICT 1
>> +#define IOMMU_DOMAIN_IS_STRICT(domain) \
>> + (domain->type == IOMMU_DOMAIN_UNMANAGED)
>> struct iommu_domain {
>> unsigned type;
>> @@ -103,6 +105,7 @@ enum iommu_cap {
>> transactions */
>> IOMMU_CAP_INTR_REMAP, /* IOMMU supports interrupt isolation */
>> IOMMU_CAP_NOEXEC, /* IOMMU_NOEXEC flag */
>> + IOMMU_CAP_NON_STRICT, /* IOMMU supports non-strict mode */
>
> This isn't a property of the IOMMU, it depends purely on the driver implementation. I think it also doesn't matter anyway - if a caller asks for lazy unmapping on their domain but the IOMMU driver just does strict unmaps anyway because that's all it supports, there's no actual harm done.
>
> Robin.
>
>> };
>> /*
>>
>
> .
>
--
Thanks!
BestRegards
^ permalink raw reply
* [PATCH 06/15] drm/sun4i: tcon: Add support for tcon-top
From: Maxime Ripard @ 2018-06-04 11:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v667pdjfHXYpBk1ER5sC8vgpcaOFKEbEByWoD1zh9Z0cyg@mail.gmail.com>
On Fri, Jun 01, 2018 at 09:19:43AM -0700, Chen-Yu Tsai wrote:
> On Fri, Jun 1, 2018 at 8:29 AM, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > On Thu, May 31, 2018 at 07:54:08PM +0200, Jernej ?krabec wrote:
> >> Dne ?etrtek, 31. maj 2018 ob 11:21:33 CEST je Maxime Ripard napisal(a):
> >> > On Thu, May 24, 2018 at 03:01:09PM -0700, Chen-Yu Tsai wrote:
> >> > > >> > > + if (tcon->quirks->needs_tcon_top) {
> >> > > >> > > + struct device_node *np;
> >> > > >> > > +
> >> > > >> > > + np = of_parse_phandle(dev->of_node, "allwinner,tcon-top",
> >> > > >> > > 0);
> >> > > >> > > + if (np) {
> >> > > >> > > + struct platform_device *pdev;
> >> > > >> > > +
> >> > > >> > > + pdev = of_find_device_by_node(np);
> >> > > >> > > + if (pdev)
> >> > > >> > > + tcon->tcon_top =
> >> > > >> > > platform_get_drvdata(pdev);
> >> > > >> > > + of_node_put(np);
> >> > > >> > > +
> >> > > >> > > + if (!tcon->tcon_top)
> >> > > >> > > + return -EPROBE_DEFER;
> >> > > >> > > + }
> >> > > >> > > + }
> >> > > >> > > +
> >> > > >> >
> >> > > >> > I might have missed it, but I've not seen the bindings additions for
> >> > > >> > that property. This shouldn't really be done that way anyway, instead
> >> > > >> > of using a direct phandle, you should be using the of-graph, with the
> >> > > >> > TCON-top sitting where it belongs in the flow of data.
> >> > > >>
> >> > > >> Just to answer to the first question, it did describe it in "[PATCH
> >> > > >> 07/15] dt- bindings: display: sun4i-drm: Add R40 HDMI pipeline".
> >> > > >>
> >> > > >> As why I designed it that way - HW representation could be described
> >> > > >> that way> >>
> >> > > >> (ASCII art makes sense when fixed width font is used to view it):
> >> > > >> / LCD0/LVDS0
> >> > > >>
> >> > > >> / TCON-LCD0
> >> > > >>
> >> > > >> | \ MIPI DSI
> >> > > >>
> >> > > >> mixer0 |
> >> > > >>
> >> > > >> \ / TCON-LCD1 - LCD1/LVDS1
> >> > > >>
> >> > > >> TCON-TOP
> >> > > >>
> >> > > >> / \ TCON-TV0 - TVE0/RGB
> >> > > >>
> >> > > >> mixer1 | \
> >> > > >>
> >> > > >> | TCON-TOP - HDMI
> >> > > >> |
> >> > > >> | /
> >> > > >>
> >> > > >> \ TCON-TV1 - TVE1/RGB
> >> > > >>
> >> > > >> This is a bit simplified, since there is also TVE-TOP, which is
> >> > > >> responsible
> >> > > >> for sharing 4 DACs between both TVE encoders. You can have two TV outs
> >> > > >> (PAL/ NTSC) or TVE0 as TV out and TVE1 as RGB or vice versa. It even
> >> > > >> seems that you can arbitrarly choose which DAC is responsible for
> >> > > >> which signal, so there is a ton of possible end combinations, but I'm
> >> > > >> not 100% sure.
> >> > > >>
> >> > > >> Even though I wrote TCON-TOP twice, this is same unit in HW. R40 manual
> >> > > >> suggest more possibilities, although some of them seem wrong, like RGB
> >> > > >> feeding from LCD TCON. That is confirmed to be wrong when checking BSP
> >> > > >> code.
> >> > > >>
> >> > > >> Additionally, TCON-TOP comes in the middle of TVE0 and LCD0, TVE1 and
> >> > > >> LCD1 for pin muxing, although I'm not sure why is that needed at all,
> >> > > >> since according to R40 datasheet, TVE0 and TVE1 pins are dedicated and
> >> > > >> not on PORT D and PORT H, respectively, as TCON-TOP documentation
> >> > > >> suggest. However, HSYNC and PSYNC lines might be shared between TVE
> >> > > >> (when it works in RGB mode) and LCD. But that is just my guess since
> >> > > >> I'm not really familiar with RGB and LCD interfaces.
> >> > > >>
> >> > > >> I'm really not sure what would be the best representation in OF-graph.
> >> > > >> Can you suggest one?
> >> > > >
> >> > > > Rob might disagree on this one, but I don't see anything wrong with
> >> > > > having loops in the graph. If the TCON-TOP can be both the input and
> >> > > > output of the TCONs, then so be it, and have it described that way in
> >> > > > the graph.
> >> > > >
> >> > > > The code is already able to filter out nodes that have already been
> >> > > > added to the list of devices we need to wait for in the component
> >> > > > framework, so that should work as well.
> >> > > >
> >> > > > And we'd need to describe TVE-TOP as well, even though we don't have a
> >> > > > driver for it yet. That will simplify the backward compatibility later
> >> > > > on.
> >> > >
> >> > > I'm getting the feeling that TCON-TOP / TVE-TOP is the glue layer that
> >> > > binds everything together, and provides signal routing, kind of like
> >> > > DE-TOP on A64. So the signal mux controls that were originally found
> >> > > in TCON0 and TVE0 were moved out.
> >> > >
> >> > > The driver needs to know about that, but the graph about doesn't make
> >> > > much sense directly. Without looking at the manual, I understand it to
> >> > > likely be one mux between the mixers and TCONs, and one between the
> >> > > TCON-TVs and HDMI. Would it make more sense to just have the graph
> >> > > connections between the muxed components, and remove TCON-TOP from
> >> > > it, like we had in the past? A phandle could be used to reference
> >> > > the TCON-TOP for mux controls, in addition to the clocks and resets.
> >> > >
> >> > > For TVE, we would need something to represent each of the output pins,
> >> > > so the device tree can actually describe what kind of signal, be it
> >> > > each component of RGB/YUV or composite video, is wanted on each pin,
> >> > > if any. This is also needed on the A20 for the Cubietruck, so we can
> >> > > describe which pins are tied to the VGA connector, and which one does
> >> > > R, G, or B.
> >> >
> >> > I guess we'll see how the DT maintainers feel about this, but my
> >> > impression is that the OF graph should model the flow of data between
> >> > the devices. If there's a mux somewhere, then the data is definitely
> >> > going through it, and as such it should be part of the graph.
> >>
> >> I concur, but I spent few days thinking how to represent this sanely in graph,
> >> but I didn't find any good solution. I'll represent here my idea and please
> >> tell your opinion before I start implementing it.
> >>
> >> First, let me be clear that mixer0 and mixer1 don't have same capabilities
> >> (different number of planes, mixer0 supports writeback, mixer1 does not,
> >> etc.). Thus, it does matter which mixer is connected to which TCON/encoder.
> >> mixer0 is meant to be connected to main display and mixer1 to auxiliary. That
> >> obviously depends on end system.
> >>
> >> So, TCON TOP has 3 muxes, which have to be represented in graph. Two of them
> >> are for mixer/TCON relationship (each of them 1 input and 4 possible outputs)
> >> and one for TV TCON/HDMI pair selection (2 possible inputs, 1 output).
> >>
> >> According to current practice in sun4i-drm driver, graph has to have port 0,
> >> representing input and port 1, representing output. This would mean that graph
> >> looks something like that:
> >>
> >> tcon_top: tcon-top at 1c70000 {
> >> compatible = "allwinner,sun8i-r40-tcon-top";
> >> ...
> >> ports {
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >>
> >> tcon_top_in: port at 0 {
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >> reg = <0>;
> >>
> >> tcon_top_in_mixer0: endpoint at 0 {
> >> reg = <0>;
> >> remote-endpoint = <&mixer0_out_tcon_top>;
> >> };
> >>
> >> tcon_top_in_mixer1: endpoint at 1 {
> >> reg = <1>;
> >> remote-endpoint = <&mixer1_out_tcon_top>;
> >> };
> >>
> >> tcon_top_in_tcon_tv: endpoint at 2 {
> >> reg = <2>;
> >> // here is HDMI input connection, part of board DTS
> >> remote-endpoint = <board specific phandle to TV TCON output>;
> >> };
> >> };
> >>
> >> tcon_top_out: port at 1 {
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >> reg = <1>;
> >>
> >> tcon_top_out_tcon0: endpoint at 0 {
> >> reg = <0>;
> >> // here is mixer0 output connection, part of board DTS
> >> remote-endpoint = <board specific phandle to TCON>;
> >> };
> >>
> >> tcon_top_out_tcon1: endpoint at 1 {
> >> reg = <1>;
> >> // here is mixer1 output connection, part of board DTS
> >> remote-endpoint = <board specific phandle to TCON>;
> >> };
> >>
> >> tcon_top_out_hdmi: endpoint at 2 {
> >> reg = <2>;
> >> remote-endpoint = <&hdmi_in_tcon_top>;
> >> };
> >> };
> >> };
> >> };
> >
> > IIRC, each port is supposed to be one route for the data, so we would
> > have multiple ports, one for the mixers in input and for the tcon in
> > output, and one for the TCON in input and for the HDMI/TV in
> > output. Rob might correct me here.
> >
> >> tcon_tv0: lcd-controller at 1c73000 {
> >> compatible = "allwinner,sun8i-r40-tcon-tv-0";
> >> ...
> >> ports {
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >>
> >> tcon_tv0_in: port at 0 {
> >> reg = <0>;
> >>
> >> tcon_tv0_in_tcon_top: endpoint {
> >> // endpoint depends on board, part of board DTS
> >> remote-endpoint = <phandle to one of tcon_top_out_tcon>;
> >
> > Just curious, what would be there?
> >
> >> };
> >> };
> >>
> >> tcon_tv0_out: port at 1 {
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >> reg = <1>;
> >>
> >> // endpoints to TV TOP and TCON TOP HDMI input
> >> ...
> >> };
> >> };
> >> };
> >>
> >> tcon_tv1: lcd-controller at 1c74000 {
> >> compatible = "allwinner,sun8i-r40-tcon-tv-1";
> >> ...
> >> ports {
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >>
> >> tcon_tv1_in: port at 0 {
> >> reg = <0>;
> >>
> >> tcon_tv1_in_tcon_top: endpoint {
> >> // endpoint depends on board, part of board DTS
> >> remote-endpoint = <phandle to one of tcon_top_out_tcon>;
> >> };
> >> };
> >>
> >> tcon_tv1_out: port at 1 {
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >> reg = <1>;
> >>
> >> // endpoints to TV TOP and TCON TOP HDMI input
> >> ...
> >> };
> >> };
> >> };
> >>
> >> tcon_lcd0 and tcon_lcd1 would have similar connections, except that for
> >> outputs would be LCD or LVDS panels or MIPI DSI encoder.
> >>
> >> Please note that each TCON (there are 4 of them) would need to have unique
> >> compatible and have HW index stored in quirks data. It would be used by TCON
> >> TOP driver for configuring muxes.
> >
> > Can't we use the port/endpoint ID instead? If the mux is the only
> > thing that changes, the compatible has no reason to. It's the same IP,
> > and the only thing that changes is something that is not part of that
> > IP.
>
> I agree. Endpoint IDs should provide that information. I'm still not
> sure How to encode multiple in/out mux groups in a device node though.
I guess we can do that through different ports?
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180604/39f48a03/attachment-0001.sig>
^ permalink raw reply
* [PATCH 0/4] arm64/mm: migrate swapper_pg_dir
From: Jun Yao @ 2018-06-04 11:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <41dbb7d5-c9a5-3ed2-c0fe-a8bb8a3e487f@arm.com>
On Fri, Jun 01, 2018 at 10:42:10AM +0100, Robin Murphy wrote:
> On 01/06/18 09:08, Jun Yao wrote:
> > Currently, The offset between swapper_pg_dir and _text is
> > fixed. When attackers know the address of _text(no KASLR or
> > breaking KASLR), they can caculate the address of
> > swapper_pg_dir. Then KSMA(Kernel Space Mirroring Attack) can
> > be applied.
> >
> > The principle of KSMA is to insert a carefully constructed PGD
> > entry into the translation table. The type of this entry is
>
> Out of interest, how does that part work? AFAICS, modifying a PGD entry
> involves writing to kernel memory, which would mean the implication of KSMA
> is "userspace can gain write permission to kernel memory by writing to
> kernel memory" - that doesn't sound like an attack in itself, more just a
> convenience for ease of exploiting whatever successful attack got you in
> there in the first place.
>
> That's not to say that it isn't still worth mitigating, I'm just questioning
> the given rationale here.
>
> Robin.
Yes, you are right. KSMA is just a convenience for ease of exploiting. I think
that the biggest role of KSMA is to covert an arbitrary write to multiple
arbitrary writes. In the past, to accomplish this, a function
pointer(e.g. ptmx_fops) is modified to point to gadget, which can r/w kernel
memory. However, PAN makes this more difficult. And KSMA becomes a new way to
do that.
For details on KSMA, you can refer to:
https://www.blackhat.com/docs/asia-18/asia-18-WANG-KSMA-Breaking-Android-kernel-isolation-and-Rooting-with-ARM-MMU-features.pdf
thanks,
Jun
^ permalink raw reply
* [PATCH 4/7] iommu/amd: make sure TLB to be flushed before IOVA freed
From: Leizhen (ThunderTown) @ 2018-06-04 11:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <ccc47903-d946-13ed-3c68-b264bc44b216@arm.com>
On 2018/5/31 21:04, Robin Murphy wrote:
> On 31/05/18 08:42, Zhen Lei wrote:
>> Although the mapping has already been removed in the page table, it maybe
>> still exist in TLB. Suppose the freed IOVAs is reused by others before the
>> flush operation completed, the new user can not correctly access to its
>> meomory.
>
> This change seems reasonable in isolation, but why is it right in the middle of a series which has nothing to do with x86?
Because I described more in the previous patch, which may help this patch to be understood well.
You're right, I will repost this patch separately.
>
> Robin.
>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>> drivers/iommu/amd_iommu.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
>> index 8fb8c73..93aa389 100644
>> --- a/drivers/iommu/amd_iommu.c
>> +++ b/drivers/iommu/amd_iommu.c
>> @@ -2402,9 +2402,9 @@ static void __unmap_single(struct dma_ops_domain *dma_dom,
>> }
>> if (amd_iommu_unmap_flush) {
>> - dma_ops_free_iova(dma_dom, dma_addr, pages);
>> domain_flush_tlb(&dma_dom->domain);
>> domain_flush_complete(&dma_dom->domain);
>> + dma_ops_free_iova(dma_dom, dma_addr, pages);
>> } else {
>> pages = __roundup_pow_of_two(pages);
>> queue_iova(&dma_dom->iovad, dma_addr >> PAGE_SHIFT, pages, 0);
>>
>
> .
>
--
Thanks!
BestRegards
^ permalink raw reply
* [PATCH 3/7] iommu: prepare for the non-strict mode support
From: Leizhen (ThunderTown) @ 2018-06-04 11:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f6b2b67e-2178-91a1-1e54-c15fc8c0a7c6@arm.com>
On 2018/5/31 21:04, Robin Murphy wrote:
> On 31/05/18 08:42, Zhen Lei wrote:
>> In common, a IOMMU unmap operation follow the below steps:
>> 1. remove the mapping in page table of the specified iova range
>> 2. execute tlbi command to invalid the mapping which is cached in TLB
>> 3. wait for the above tlbi operation to be finished
>> 4. free the IOVA resource
>> 5. free the physical memory resource
>>
>> This maybe a problem when unmap is very frequently, the combination of tlbi
>> and wait operation will consume a lot of time. A feasible method is put off
>> tlbi and iova-free operation, when accumulating to a certain number or
>> reaching a specified time, execute only one tlbi_all command to clean up
>> TLB, then free the backup IOVAs. Mark as non-strict mode.
>>
>> But it must be noted that, although the mapping has already been removed in
>> the page table, it maybe still exist in TLB. And the freed physical memory
>> may also be reused for others. So a attacker can persistent access to memory
>> based on the just freed IOVA, to obtain sensible data or corrupt memory. So
>> the VFIO should always choose the strict mode.
>>
>> This patch just add a new parameter for the unmap operation, to help the
>> upper functions capable choose which mode to be applied.
>
> This seems like it might be better handled by a flag in io_pgtable_cfg->quirks. This interface change on its own looks rather invasive, and teh fact that it ends up only being used to pass through a constant property of the domain (which is already known by the point io_pgtable_alloc() is called) implies that it is indeed the wrong level of abstraction.
>
Sound good. Thanks for your suggestion, I will try it in v2.
>> No functional changes.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>> drivers/iommu/arm-smmu-v3.c | 2 +-
>> drivers/iommu/arm-smmu.c | 2 +-
>> drivers/iommu/io-pgtable-arm-v7s.c | 6 +++---
>> drivers/iommu/io-pgtable-arm.c | 6 +++---
>> drivers/iommu/io-pgtable.h | 2 +-
>> drivers/iommu/ipmmu-vmsa.c | 2 +-
>> drivers/iommu/msm_iommu.c | 2 +-
>> drivers/iommu/mtk_iommu.c | 2 +-
>> drivers/iommu/qcom_iommu.c | 2 +-
>> include/linux/iommu.h | 2 ++
>
> Plus things specific to io-pgtable shouldn't really be spilling into the core API header either.
>
> Robin.
>
>> 10 files changed, 15 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index 4402187..59b3387 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -1767,7 +1767,7 @@ static int arm_smmu_map(struct iommu_domain *domain, unsigned long iova,
>> if (!ops)
>> return 0;
>> - return ops->unmap(ops, iova, size);
>> + return ops->unmap(ops, iova, size, IOMMU_STRICT);
>> }
>> static void arm_smmu_flush_iotlb_all(struct iommu_domain *domain)
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 69e7c60..253e807 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -1249,7 +1249,7 @@ static size_t arm_smmu_unmap(struct iommu_domain *domain, unsigned long iova,
>> if (!ops)
>> return 0;
>> - return ops->unmap(ops, iova, size);
>> + return ops->unmap(ops, iova, size, IOMMU_STRICT);
>> }
>> static void arm_smmu_iotlb_sync(struct iommu_domain *domain)
>> diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c
>> index 10e4a3d..799eced 100644
>> --- a/drivers/iommu/io-pgtable-arm-v7s.c
>> +++ b/drivers/iommu/io-pgtable-arm-v7s.c
>> @@ -658,7 +658,7 @@ static size_t __arm_v7s_unmap(struct arm_v7s_io_pgtable *data,
>> }
>> static size_t arm_v7s_unmap(struct io_pgtable_ops *ops, unsigned long iova,
>> - size_t size)
>> + size_t size, int strict)
>> {
>> struct arm_v7s_io_pgtable *data = io_pgtable_ops_to_data(ops);
>> @@ -883,7 +883,7 @@ static int __init arm_v7s_do_selftests(void)
>> size = 1UL << __ffs(cfg.pgsize_bitmap);
>> while (i < loopnr) {
>> iova_start = i * SZ_16M;
>> - if (ops->unmap(ops, iova_start + size, size) != size)
>> + if (ops->unmap(ops, iova_start + size, size, IOMMU_STRICT) != size)
>> return __FAIL(ops);
>> /* Remap of partial unmap */
>> @@ -902,7 +902,7 @@ static int __init arm_v7s_do_selftests(void)
>> while (i != BITS_PER_LONG) {
>> size = 1UL << i;
>> - if (ops->unmap(ops, iova, size) != size)
>> + if (ops->unmap(ops, iova, size, IOMMU_STRICT) != size)
>> return __FAIL(ops);
>> if (ops->iova_to_phys(ops, iova + 42))
>> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
>> index 39c2a05..e0f52db 100644
>> --- a/drivers/iommu/io-pgtable-arm.c
>> +++ b/drivers/iommu/io-pgtable-arm.c
>> @@ -624,7 +624,7 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
>> }
>> static size_t arm_lpae_unmap(struct io_pgtable_ops *ops, unsigned long iova,
>> - size_t size)
>> + size_t size, int strict)
>> {
>> struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
>> arm_lpae_iopte *ptep = data->pgd;
>> @@ -1108,7 +1108,7 @@ static int __init arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
>> /* Partial unmap */
>> size = 1UL << __ffs(cfg->pgsize_bitmap);
>> - if (ops->unmap(ops, SZ_1G + size, size) != size)
>> + if (ops->unmap(ops, SZ_1G + size, size, IOMMU_STRICT) != size)
>> return __FAIL(ops, i);
>> /* Remap of partial unmap */
>> @@ -1124,7 +1124,7 @@ static int __init arm_lpae_run_tests(struct io_pgtable_cfg *cfg)
>> while (j != BITS_PER_LONG) {
>> size = 1UL << j;
>> - if (ops->unmap(ops, iova, size) != size)
>> + if (ops->unmap(ops, iova, size, IOMMU_STRICT) != size)
>> return __FAIL(ops, i);
>> if (ops->iova_to_phys(ops, iova + 42))
>> diff --git a/drivers/iommu/io-pgtable.h b/drivers/iommu/io-pgtable.h
>> index 2df7909..2908806 100644
>> --- a/drivers/iommu/io-pgtable.h
>> +++ b/drivers/iommu/io-pgtable.h
>> @@ -120,7 +120,7 @@ struct io_pgtable_ops {
>> int (*map)(struct io_pgtable_ops *ops, unsigned long iova,
>> phys_addr_t paddr, size_t size, int prot);
>> size_t (*unmap)(struct io_pgtable_ops *ops, unsigned long iova,
>> - size_t size);
>> + size_t size, int strict);
>> phys_addr_t (*iova_to_phys)(struct io_pgtable_ops *ops,
>> unsigned long iova);
>> };
>> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
>> index 40ae6e8..e6d9e11 100644
>> --- a/drivers/iommu/ipmmu-vmsa.c
>> +++ b/drivers/iommu/ipmmu-vmsa.c
>> @@ -716,7 +716,7 @@ static size_t ipmmu_unmap(struct iommu_domain *io_domain, unsigned long iova,
>> {
>> struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
>> - return domain->iop->unmap(domain->iop, iova, size);
>> + return domain->iop->unmap(domain->iop, iova, size, IOMMU_STRICT);
>> }
>> static void ipmmu_iotlb_sync(struct iommu_domain *io_domain)
>> diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
>> index 0d33504..180fa3d 100644
>> --- a/drivers/iommu/msm_iommu.c
>> +++ b/drivers/iommu/msm_iommu.c
>> @@ -532,7 +532,7 @@ static size_t msm_iommu_unmap(struct iommu_domain *domain, unsigned long iova,
>> unsigned long flags;
>> spin_lock_irqsave(&priv->pgtlock, flags);
>> - len = priv->iop->unmap(priv->iop, iova, len);
>> + len = priv->iop->unmap(priv->iop, iova, len, IOMMU_STRICT);
>> spin_unlock_irqrestore(&priv->pgtlock, flags);
>> return len;
>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
>> index f2832a1..54661ed 100644
>> --- a/drivers/iommu/mtk_iommu.c
>> +++ b/drivers/iommu/mtk_iommu.c
>> @@ -386,7 +386,7 @@ static size_t mtk_iommu_unmap(struct iommu_domain *domain,
>> size_t unmapsz;
>> spin_lock_irqsave(&dom->pgtlock, flags);
>> - unmapsz = dom->iop->unmap(dom->iop, iova, size);
>> + unmapsz = dom->iop->unmap(dom->iop, iova, size, IOMMU_STRICT);
>> spin_unlock_irqrestore(&dom->pgtlock, flags);
>> return unmapsz;
>> diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
>> index 65b9c99..90abde1 100644
>> --- a/drivers/iommu/qcom_iommu.c
>> +++ b/drivers/iommu/qcom_iommu.c
>> @@ -444,7 +444,7 @@ static size_t qcom_iommu_unmap(struct iommu_domain *domain, unsigned long iova,
>> */
>> pm_runtime_get_sync(qcom_domain->iommu->dev);
>> spin_lock_irqsave(&qcom_domain->pgtbl_lock, flags);
>> - ret = ops->unmap(ops, iova, size);
>> + ret = ops->unmap(ops, iova, size, IOMMU_STRICT);
>> spin_unlock_irqrestore(&qcom_domain->pgtbl_lock, flags);
>> pm_runtime_put_sync(qcom_domain->iommu->dev);
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index 19938ee..39b3150 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -86,6 +86,8 @@ struct iommu_domain_geometry {
>> #define IOMMU_DOMAIN_DMA (__IOMMU_DOMAIN_PAGING | \
>> __IOMMU_DOMAIN_DMA_API)
>> +#define IOMMU_STRICT 1
>> +
>> struct iommu_domain {
>> unsigned type;
>> const struct iommu_ops *ops;
>>
>
> .
>
--
Thanks!
BestRegards
^ permalink raw reply
* [PATCH v2] ASoC: dapm: delete dapm_kcontrol_data paths list before freeing it
From: Srinivas Kandagatla @ 2018-06-04 11:13 UTC (permalink / raw)
To: linux-arm-kernel
dapm_kcontrol_data is freed as part of dapm_kcontrol_free(), leaving the
paths pointer dangling in the list.
This leads to system crash when we try to unload and reload sound card.
I hit this bug during ADSP crash/reboot test case on Dragon board DB410c.
Without this patch, on SLAB Poisoning enabled build, kernel crashes with
"BUG kmalloc-128 (Tainted: G W ): Poison overwritten"
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
Changes since v1:
-remove unnecessary very long bug trace.
sound/soc/soc-dapm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 1e9a36389667..36a39ba30226 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -433,6 +433,8 @@ static int dapm_kcontrol_data_alloc(struct snd_soc_dapm_widget *widget,
static void dapm_kcontrol_free(struct snd_kcontrol *kctl)
{
struct dapm_kcontrol_data *data = snd_kcontrol_chip(kctl);
+
+ list_del(&data->paths);
kfree(data->wlist);
kfree(data);
}
--
2.16.2
^ permalink raw reply related
* [PATCH 1/7] iommu/dma: fix trival coding style mistake
From: Leizhen (ThunderTown) @ 2018-06-04 11:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <b473059d-fee6-1627-4749-192771038741@arm.com>
On 2018/5/31 21:03, Robin Murphy wrote:
> On 31/05/18 08:42, Zhen Lei wrote:
>> The static function iova_reserve_iommu_regions is only called by function
>> iommu_dma_init_domain, and the 'if (!dev)' check in iommu_dma_init_domain
>> affect it only, so we can safely move the check into it. I think it looks
>> more natural.
>
> As before, I disagree - the logic of iommu_dma_init_domain() is "we expect to have a valid device, but stop here if we don't", and moving the check just needlessly obfuscates that. It is not a coincidence that the arguments of both functions are in effective order of importance.
OK
>
>> In addition, the local variable 'ret' is only assigned in the branch of
>> 'if (region->type == IOMMU_RESV_MSI)', so the 'if (ret)' should also only
>> take effect in the branch, add a brace to enclose it.
>
> 'ret' is clearly also assigned at its declaration, to cover the (very likely) case where we don't enter the loop at all. Thus testing it in the loop is harmless, and cluttering that up with extra tabs and braces is just noise.
OK, I will drop this patch in v2
>
> Robin.
>
>> No functional changes.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>> drivers/iommu/dma-iommu.c | 12 +++++++-----
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>> index ddcbbdb..4e885f7 100644
>> --- a/drivers/iommu/dma-iommu.c
>> +++ b/drivers/iommu/dma-iommu.c
>> @@ -231,6 +231,9 @@ static int iova_reserve_iommu_regions(struct device *dev,
>> LIST_HEAD(resv_regions);
>> int ret = 0;
>> + if (!dev)
>> + return 0;
>> +
>> if (dev_is_pci(dev))
>> iova_reserve_pci_windows(to_pci_dev(dev), iovad);
>> @@ -246,11 +249,12 @@ static int iova_reserve_iommu_regions(struct device *dev,
>> hi = iova_pfn(iovad, region->start + region->length - 1);
>> reserve_iova(iovad, lo, hi);
>> - if (region->type == IOMMU_RESV_MSI)
>> + if (region->type == IOMMU_RESV_MSI) {
>> ret = cookie_init_hw_msi_region(cookie, region->start,
>> region->start + region->length);
>> - if (ret)
>> - break;
>> + if (ret)
>> + break;
>> + }
>> }
>> iommu_put_resv_regions(dev, &resv_regions);
>> @@ -308,8 +312,6 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>> }
>> init_iova_domain(iovad, 1UL << order, base_pfn);
>> - if (!dev)
>> - return 0;
>> return iova_reserve_iommu_regions(dev, domain);
>> }
>>
>
> .
>
--
Thanks!
BestRegards
^ permalink raw reply
* [PATCH] ASoC: dapm: delete dapm_kcontrol_data paths entry before freeing
From: Mark Brown @ 2018-06-04 11:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <e37bf559-b040-db34-6273-d4f8ea1e2284@linaro.org>
On Mon, Jun 04, 2018 at 11:55:10AM +0100, Srinivas Kandagatla wrote:
> I agree, Do you want me to resend the patch removing irrelevant sections in
> log?
I didn't actually apply yet so yes please.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180604/a71b3ace/attachment-0001.sig>
^ permalink raw reply
* [PATCH] ASoC: dapm: delete dapm_kcontrol_data paths entry before freeing
From: Srinivas Kandagatla @ 2018-06-04 10:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180604104541.GB7536@sirena.org.uk>
On 04/06/18 11:45, Mark Brown wrote:
> On Fri, Jun 01, 2018 at 11:53:34PM +0100, Srinivas Kandagatla wrote:
>
>> Below is the kernel BUG with SLAB Poisoning
>
>> =============================================================================
>> BUG kmalloc-128 (Tainted: G W ): Poison overwritten
>> -----------------------------------------------------------------------------
>>
>> Disabling lock debugging due to kernel taint
>> INFO: 0xffff80003cf1c310-0xffff80003cf1c31f. First byte 0x10 instead of 0x6b
>> INFO: Allocated in dapm_kcontrol_data_alloc.isra.37+0x34/0x2a8 age=6929 cpu=0 pid=50
>> __slab_alloc.isra.24+0x24/0x38
>> kmem_cache_alloc+0x190/0x1d8
>
> Please think hard before including complete backtraces in upstream
> reports, they are very large and contain almost no useful information
> relative to their size so often obscure the relevant content in your
> message. If part of the backtrace is usefully illustrative then it's
> usually better to pull out the relevant sections.
I agree, Do you want me to resend the patch removing irrelevant sections
in log?
thanks,
srini
>
^ permalink raw reply
* linux-next: manual merge of the regulator tree with the arm-soc tree
From: Mark Brown @ 2018-06-04 10:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2564792.sYBGPVz9u3@z50>
On Fri, Jun 01, 2018 at 12:49:53AM +0200, Janusz Krzysztofik wrote:
> I confirm the fix by Stephen works for me, however, the conflicting patch by
> Linus breaks things a bit.
> Lookup tables added to board files use function name "enable" while the
> regulator uses NULL. As a result, GPIO descriptor is not matched and not
> assigned to the regulator which ends up running with no control over GPIO pin.
> Either the regulator driver should use the function name "enable" or that name
> should be removed from lookup tables.
I'll revert this one as well :(
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180604/ab8c1fa9/attachment.sig>
^ permalink raw reply
* [PATCH] ASoC: dapm: delete dapm_kcontrol_data paths entry before freeing
From: Mark Brown @ 2018-06-04 10:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180601225334.19064-1-srinivas.kandagatla@linaro.org>
On Fri, Jun 01, 2018 at 11:53:34PM +0100, Srinivas Kandagatla wrote:
> Below is the kernel BUG with SLAB Poisoning
> =============================================================================
> BUG kmalloc-128 (Tainted: G W ): Poison overwritten
> -----------------------------------------------------------------------------
>
> Disabling lock debugging due to kernel taint
> INFO: 0xffff80003cf1c310-0xffff80003cf1c31f. First byte 0x10 instead of 0x6b
> INFO: Allocated in dapm_kcontrol_data_alloc.isra.37+0x34/0x2a8 age=6929 cpu=0 pid=50
> __slab_alloc.isra.24+0x24/0x38
> kmem_cache_alloc+0x190/0x1d8
Please think hard before including complete backtraces in upstream
reports, they are very large and contain almost no useful information
relative to their size so often obscure the relevant content in your
message. If part of the backtrace is usefully illustrative then it's
usually better to pull out the relevant sections.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180604/2726e3d4/attachment.sig>
^ permalink raw reply
* [PATCH 5/5] arm64: topology: rename llc_siblings to align with other struct members
From: Sudeep Holla @ 2018-06-04 10:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528108797-13743-1-git-send-email-sudeep.holla@arm.com>
Similar to core_sibling and thread_sibling, it's better to align and
rename llc_siblings to llc_sibling.
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
arch/arm64/include/asm/topology.h | 2 +-
arch/arm64/kernel/topology.c | 12 ++++++------
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/include/asm/topology.h b/arch/arm64/include/asm/topology.h
index fb996f454305..dda4b6dba6b4 100644
--- a/arch/arm64/include/asm/topology.h
+++ b/arch/arm64/include/asm/topology.h
@@ -11,7 +11,7 @@ struct cpu_topology {
int llc_id;
cpumask_t thread_sibling;
cpumask_t core_sibling;
- cpumask_t llc_siblings;
+ cpumask_t llc_sibling;
};
extern struct cpu_topology cpu_topology[NR_CPUS];
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index bf522bcf40ec..082f76f7a94d 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -223,8 +223,8 @@ const struct cpumask *cpu_coregroup_mask(int cpu)
core_mask = &cpu_topology[cpu].core_sibling;
}
if (cpu_topology[cpu].llc_id != -1) {
- if (cpumask_subset(&cpu_topology[cpu].llc_siblings, core_mask))
- core_mask = &cpu_topology[cpu].llc_siblings;
+ if (cpumask_subset(&cpu_topology[cpu].llc_sibling, core_mask))
+ core_mask = &cpu_topology[cpu].llc_sibling;
}
return core_mask;
@@ -240,7 +240,7 @@ static void update_siblings_masks(unsigned int cpuid)
cpu_topo = &cpu_topology[cpu];
if (cpuid_topo->llc_id == cpu_topo->llc_id)
- cpumask_set_cpu(cpu, &cpuid_topo->llc_siblings);
+ cpumask_set_cpu(cpu, &cpuid_topo->llc_sibling);
if (cpuid_topo->package_id != cpu_topo->package_id)
continue;
@@ -305,13 +305,13 @@ static void clear_cpu_topology(int cpu, bool reset)
cpu_topo->package_id = -1;
cpu_topo->llc_id = -1;
- cpumask_clear(&cpu_topo->llc_siblings);
+ cpumask_clear(&cpu_topo->llc_sibling);
cpumask_clear(&cpu_topo->core_sibling);
cpumask_clear(&cpu_topo->thread_sibling);
if (reset) {
cpu_topo->core_id = 0;
- cpumask_set_cpu(cpu, &cpu_topo->llc_siblings);
+ cpumask_set_cpu(cpu, &cpu_topo->llc_sibling);
cpumask_set_cpu(cpu, &cpu_topo->core_sibling);
cpumask_set_cpu(cpu, &cpu_topo->thread_sibling);
}
@@ -325,7 +325,7 @@ static void __init reset_cpu_topology(void)
clear_cpu_topology(cpu, true);
}
-#define cpu_llc_shared_mask(cpu) (&cpu_topology[cpu].llc_siblings)
+#define cpu_llc_shared_mask(cpu) (&cpu_topology[cpu].llc_sibling)
void remove_cpu_topology(unsigned int cpu)
{
int sibling;
--
2.7.4
^ permalink raw reply related
* [PATCH 4/5] arm64: smp: remove cpu and numa topology information when hotplugging out CPU
From: Sudeep Holla @ 2018-06-04 10:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528108797-13743-1-git-send-email-sudeep.holla@arm.com>
We already repopulate the information on CPU hotplug-in, so we can safely
remove the CPU topology and NUMA cpumap information during CPU hotplug
out operation. This will help to provide the correct cpumask for
scheduler domains.
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
arch/arm64/kernel/smp.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 49a021e30dfb..63a40ba3cd37 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -279,6 +279,9 @@ int __cpu_disable(void)
if (ret)
return ret;
+ remove_cpu_topology(cpu);
+ numa_remove_cpu(cpu);
+
/*
* Take this CPU offline. Once we clear this, we can't return,
* and we must not schedule until we're ready to give up the cpu.
--
2.7.4
^ permalink raw reply related
* [PATCH 3/5] arm64: numa: separate out updates to percpu nodeid and NUMA node cpumap
From: Sudeep Holla @ 2018-06-04 10:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528108797-13743-1-git-send-email-sudeep.holla@arm.com>
Currently numa_clear_node removes both cpu information from the NUMA
node cpumap as well as the NUMA node id from the cpu. Similarly
numa_store_cpu_info updates both percpu nodeid and NUMA cpumap.
However we need to retain the numa node id for the cpu and only remove
the cpu information from the numa node cpumap during CPU hotplug out.
The same can be extended for hotplugging in the CPU.
This patch separates out numa_{add,remove}_cpu from numa_clear_node and
numa_store_cpu_info.
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
arch/arm64/include/asm/numa.h | 4 ++++
arch/arm64/kernel/smp.c | 2 ++
arch/arm64/mm/numa.c | 29 +++++++++++++++++++++--------
3 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/include/asm/numa.h b/arch/arm64/include/asm/numa.h
index 01bc46d5b43a..626ad01e83bf 100644
--- a/arch/arm64/include/asm/numa.h
+++ b/arch/arm64/include/asm/numa.h
@@ -35,10 +35,14 @@ void __init numa_set_distance(int from, int to, int distance);
void __init numa_free_distance(void);
void __init early_map_cpu_to_node(unsigned int cpu, int nid);
void numa_store_cpu_info(unsigned int cpu);
+void numa_add_cpu(unsigned int cpu);
+void numa_remove_cpu(unsigned int cpu);
#else /* CONFIG_NUMA */
static inline void numa_store_cpu_info(unsigned int cpu) { }
+static inline void numa_add_cpu(unsigned int cpu) { }
+static inline void numa_remove_cpu(unsigned int cpu) { }
static inline void arm64_numa_init(void) { }
static inline void early_map_cpu_to_node(unsigned int cpu, int nid) { }
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index f3e2e3aec0b0..49a021e30dfb 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -225,6 +225,7 @@ asmlinkage void secondary_start_kernel(void)
notify_cpu_starting(cpu);
store_cpu_topology(cpu);
+ numa_add_cpu(cpu);
/*
* OK, now it's safe to let the boot CPU continue. Wait for
@@ -679,6 +680,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
this_cpu = smp_processor_id();
store_cpu_topology(this_cpu);
numa_store_cpu_info(this_cpu);
+ numa_add_cpu(this_cpu);
/*
* If UP is mandated by "nosmp" (which implies "maxcpus=0"), don't set
diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
index dad128ba98bf..43cc669bc7bc 100644
--- a/arch/arm64/mm/numa.c
+++ b/arch/arm64/mm/numa.c
@@ -70,19 +70,32 @@ EXPORT_SYMBOL(cpumask_of_node);
#endif
-static void map_cpu_to_node(unsigned int cpu, int nid)
+static void numa_update_cpu(unsigned int cpu, bool remove)
{
- set_cpu_numa_node(cpu, nid);
- if (nid >= 0)
+ int nid = cpu_to_node(cpu);
+
+ if (nid < 0)
+ return;
+
+ if (remove)
+ cpumask_clear_cpu(cpu, node_to_cpumask_map[nid]);
+ else
cpumask_set_cpu(cpu, node_to_cpumask_map[nid]);
}
-void numa_clear_node(unsigned int cpu)
+void numa_add_cpu(unsigned int cpu)
{
- int nid = cpu_to_node(cpu);
+ numa_update_cpu(cpu, false);
+}
- if (nid >= 0)
- cpumask_clear_cpu(cpu, node_to_cpumask_map[nid]);
+void numa_remove_cpu(unsigned int cpu)
+{
+ numa_update_cpu(cpu, true);
+}
+
+void numa_clear_node(unsigned int cpu)
+{
+ numa_remove_cpu(cpu);
set_cpu_numa_node(cpu, NUMA_NO_NODE);
}
@@ -116,7 +129,7 @@ static void __init setup_node_to_cpumask_map(void)
*/
void numa_store_cpu_info(unsigned int cpu)
{
- map_cpu_to_node(cpu, cpu_to_node_map[cpu]);
+ set_cpu_numa_node(cpu, cpu_to_node_map[cpu]);
}
void __init early_map_cpu_to_node(unsigned int cpu, int nid)
--
2.7.4
^ permalink raw reply related
* [PATCH 2/5] arm64: topology: add support to remove cpu topology
From: Sudeep Holla @ 2018-06-04 10:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528108797-13743-1-git-send-email-sudeep.holla@arm.com>
This patch adds support to remove all the CPU topology information using
clear_cpu_topology and also resetting the sibling information on other
sibling CPUs. This will be used in cpu_disable so that all the topology
information is removed on CPU hotplug out.
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
arch/arm64/include/asm/topology.h | 1 +
arch/arm64/kernel/topology.c | 15 +++++++++++++++
2 files changed, 16 insertions(+)
diff --git a/arch/arm64/include/asm/topology.h b/arch/arm64/include/asm/topology.h
index df48212f767b..fb996f454305 100644
--- a/arch/arm64/include/asm/topology.h
+++ b/arch/arm64/include/asm/topology.h
@@ -23,6 +23,7 @@ extern struct cpu_topology cpu_topology[NR_CPUS];
void init_cpu_topology(void);
void store_cpu_topology(unsigned int cpuid);
+void remove_cpu_topology(unsigned int cpuid);
const struct cpumask *cpu_coregroup_mask(int cpu);
#ifdef CONFIG_NUMA
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index f10babcc112b..bf522bcf40ec 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -325,6 +325,21 @@ static void __init reset_cpu_topology(void)
clear_cpu_topology(cpu, true);
}
+#define cpu_llc_shared_mask(cpu) (&cpu_topology[cpu].llc_siblings)
+void remove_cpu_topology(unsigned int cpu)
+{
+ int sibling;
+
+ for_each_cpu(sibling, topology_core_cpumask(cpu))
+ cpumask_clear_cpu(cpu, topology_core_cpumask(sibling));
+ for_each_cpu(sibling, topology_sibling_cpumask(cpu))
+ cpumask_clear_cpu(cpu, topology_sibling_cpumask(sibling));
+ for_each_cpu(sibling, cpu_llc_shared_mask(cpu))
+ cpumask_clear_cpu(cpu, cpu_llc_shared_mask(sibling));
+
+ clear_cpu_topology(cpu, false);
+}
+
#ifdef CONFIG_ACPI
/*
* Propagate the topology information of the processor_topology_node tree to the
--
2.7.4
^ permalink raw reply related
* [PATCH 1/5] arm64: topology: refactor reset_cpu_topology to add support for removing topology
From: Sudeep Holla @ 2018-06-04 10:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528108797-13743-1-git-send-email-sudeep.holla@arm.com>
Currently reset_cpu_topology clears all the CPU topology information
and resets to default values. However we may need to just clear the
information when we hotplig out the CPU. In preparation to add the
support the same, let's refactor reset_cpu_topology to clear out the
information and reset them only if explicitly requested.
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
arch/arm64/kernel/topology.c | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 7415c166281f..f10babcc112b 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -296,28 +296,35 @@ void store_cpu_topology(unsigned int cpuid)
update_siblings_masks(cpuid);
}
-static void __init reset_cpu_topology(void)
+static void clear_cpu_topology(int cpu, bool reset)
{
- unsigned int cpu;
+ struct cpu_topology *cpu_topo = &cpu_topology[cpu];
- for_each_possible_cpu(cpu) {
- struct cpu_topology *cpu_topo = &cpu_topology[cpu];
+ cpu_topo->core_id = -1;
+ cpu_topo->thread_id = -1;
+ cpu_topo->package_id = -1;
+ cpu_topo->llc_id = -1;
- cpu_topo->thread_id = -1;
- cpu_topo->core_id = 0;
- cpu_topo->package_id = -1;
+ cpumask_clear(&cpu_topo->llc_siblings);
+ cpumask_clear(&cpu_topo->core_sibling);
+ cpumask_clear(&cpu_topo->thread_sibling);
- cpu_topo->llc_id = -1;
- cpumask_clear(&cpu_topo->llc_siblings);
+ if (reset) {
+ cpu_topo->core_id = 0;
cpumask_set_cpu(cpu, &cpu_topo->llc_siblings);
-
- cpumask_clear(&cpu_topo->core_sibling);
cpumask_set_cpu(cpu, &cpu_topo->core_sibling);
- cpumask_clear(&cpu_topo->thread_sibling);
cpumask_set_cpu(cpu, &cpu_topo->thread_sibling);
}
}
+static void __init reset_cpu_topology(void)
+{
+ unsigned int cpu;
+
+ for_each_possible_cpu(cpu)
+ clear_cpu_topology(cpu, true);
+}
+
#ifdef CONFIG_ACPI
/*
* Propagate the topology information of the processor_topology_node tree to the
--
2.7.4
^ permalink raw reply related
* [PATCH 0/5] arm64: numa/topology/smp: fix the cpumasks for CPU hotplug
From: Sudeep Holla @ 2018-06-04 10:39 UTC (permalink / raw)
To: linux-arm-kernel
Hi Will, Catalin, Jeremy, Morten,
This is the fix I could come up for the issues we are seeing with arm64
for-next branch, in particular with commit 37c3ec2d810f ("arm64: topology:
divorce MC scheduling domain from core_siblings").
The solution is to update the CPU topology during CPU hotplug operations
similar to other architectures like x86 and PPC. This is also inline
with the expection from the scheduler.
I have cc-ed few Cavium and Huawei guys as they seem to have added or
modified the numa related code in the past.
Regards,
Sudeep
Sudeep Holla (5):
arm64: topology: refactor reset_cpu_topology to add support for removing topology
arm64: topology: add support to remove cpu topology
arm64: numa: separate out updates to percpu nodeid and NUMA node cpumap
arm64: smp: remove cpu and numa topology information when hotplugging out CPU
arm64: topology: rename llc_siblings to align with other struct members
arch/arm64/include/asm/numa.h | 4 +++
arch/arm64/include/asm/topology.h | 3 ++-
arch/arm64/kernel/smp.c | 5 ++++
arch/arm64/kernel/topology.c | 54 +++++++++++++++++++++++++++------------
arch/arm64/mm/numa.c | 29 +++++++++++++++------
5 files changed, 70 insertions(+), 25 deletions(-)
--
2.7.4
^ permalink raw reply
* [RFC PATCH 2/8] coresight: Fix remote endpoint parsing
From: Suzuki K Poulose @ 2018-06-04 10:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CANLsYkzY-1z4NBmZECczMisDixgPb2UkJL0k9_LmDNEiqUS1=g@mail.gmail.com>
On 06/01/2018 08:46 PM, Mathieu Poirier wrote:
> On 1 June 2018 at 13:38, Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
>> On Fri, Jun 01, 2018 at 02:16:01PM +0100, Suzuki K Poulose wrote:
>>> When parsing the remote endpoint of an output port, we do :
>>> rport = of_graph_get_remote_port(ep);
>>> rparent = of_graph_get_remote_port_parent(ep);
>>>
>>> and then parse the "remote_port" as if it was the remote endpoint,
>>> which is wrong. The code worked fine because we used endpoint number
>>> as the port number. Let us fix it and optimise a bit as:
>>>
>>> remote_ep = of_graph_get_remote_endpoint(ep);
>>> if (remote_ep)
>>> remote_parent = of_graph_get_port_parent(remote_ep);
>>>
>>> and then, parse the remote_ep for the port/endpoint details.
>>>
>>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> ---
>>> drivers/hwtracing/coresight/of_coresight.c | 19 ++++++++++---------
>>> 1 file changed, 10 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c
>>> index 7c37544..e0deab0 100644
>>> --- a/drivers/hwtracing/coresight/of_coresight.c
>>> +++ b/drivers/hwtracing/coresight/of_coresight.c
>>> @@ -128,7 +128,7 @@ of_get_coresight_platform_data(struct device *dev,
>>> struct device *rdev;
>>> struct device_node *ep = NULL;
>>> struct device_node *rparent = NULL;
>>> - struct device_node *rport = NULL;
>>> + struct device_node *rep = NULL;
>>>
>>> pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>>> if (!pdata)
>>> @@ -169,16 +169,17 @@ of_get_coresight_platform_data(struct device *dev,
>>> pdata->outports[i] = endpoint.port;
>>>
>>> /*
>>> - * Get a handle on the remote port and parent
>>> - * attached to it.
>>> + * Get a handle on the remote endpoint and the device
>>> + * it is attached to.
>>> */
>>> - rparent = of_graph_get_remote_port_parent(ep);
>>> - rport = of_graph_get_remote_port(ep);
>>> -
>>> - if (!rparent || !rport)
>>> + rep = of_graph_get_remote_endpoint(ep);
>>> + if (!rep)
>>> + continue;
>>> + rparent = of_graph_get_port_parent(rep);
>>> + if (!rparent)
>>> continue;
>>>
>>> - if (of_graph_parse_endpoint(rport, &rendpoint))
>>> + if (of_graph_parse_endpoint(rep, &rendpoint))
>>> continue;
>>
>> You are correct and I'm out to lunch.
>>
>>>
>>> rdev = of_coresight_get_endpoint_device(rparent);
>>> @@ -186,7 +187,7 @@ of_get_coresight_platform_data(struct device *dev,
>>> return ERR_PTR(-EPROBE_DEFER);
>>>
>>> pdata->child_names[i] = dev_name(rdev);
>>> - pdata->child_ports[i] = rendpoint.id;
>>> + pdata->child_ports[i] = rendpoint.port;
>>
>> You need to do a of_node_put() here for both rep and rparent.
>
> Same thing for the "continue" and error condition above.
Mathieu,
Thanks for pointing that out. I see that we were missing them for the
existing code as well. I will clean all this up.
Cheers
Suzuki
^ permalink raw reply
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