Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [v2,2/2] PCI: mediatek: Add controller support for MT7629
From: Lorenzo Pieralisi @ 2019-08-06 16:24 UTC (permalink / raw)
  To: Jianjun Wang, Bjorn Helgaas
  Cc: Mark Rutland, devicetree, Ryder Lee, linux-pci, youlin.pei,
	linux-kernel, Rob Herring, linux-mediatek, Matthias Brugger,
	linux-arm-kernel
In-Reply-To: <1564385918.17211.6.camel@mhfsdcap03>

[trim the CC list please to keep only required maintainers]

On Mon, Jul 29, 2019 at 03:38:38PM +0800, Jianjun Wang wrote:
> On Fri, 2019-06-28 at 15:34 +0800, Jianjun Wang wrote:
> > MT7629 is an ARM platform SoC which has the same PCIe IP with MT7622.
> > 
> > The HW default value of its Device ID is invalid, fix its Device ID to
> > match the hardware implementation.
> > 
> > Acked-by: Ryder Lee <ryder.lee@mediatek.com>
> > Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> > ---
> >  drivers/pci/controller/pcie-mediatek.c | 18 ++++++++++++++++++
> >  include/linux/pci_ids.h                |  1 +
> >  2 files changed, 19 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > index 80601e1b939e..e5e6740b635d 100644
> > --- a/drivers/pci/controller/pcie-mediatek.c
> > +++ b/drivers/pci/controller/pcie-mediatek.c
> > @@ -73,6 +73,7 @@
> >  #define PCIE_MSI_VECTOR		0x0c0
> >  
> >  #define PCIE_CONF_VEND_ID	0x100
> > +#define PCIE_CONF_DEVICE_ID	0x102
> >  #define PCIE_CONF_CLASS_ID	0x106
> >  
> >  #define PCIE_INT_MASK		0x420
> > @@ -141,12 +142,16 @@ struct mtk_pcie_port;
> >  /**
> >   * struct mtk_pcie_soc - differentiate between host generations
> >   * @need_fix_class_id: whether this host's class ID needed to be fixed or not
> > + * @need_fix_device_id: whether this host's Device ID needed to be fixed or not
> > + * @device_id: Device ID which this host need to be fixed
> >   * @ops: pointer to configuration access functions
> >   * @startup: pointer to controller setting functions
> >   * @setup_irq: pointer to initialize IRQ functions
> >   */
> >  struct mtk_pcie_soc {
> >  	bool need_fix_class_id;
> > +	bool need_fix_device_id;
> > +	unsigned int device_id;
> >  	struct pci_ops *ops;
> >  	int (*startup)(struct mtk_pcie_port *port);
> >  	int (*setup_irq)(struct mtk_pcie_port *port, struct device_node *node);
> > @@ -696,6 +701,9 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
> >  		writew(val, port->base + PCIE_CONF_CLASS_ID);
> >  	}
> >  
> > +	if (soc->need_fix_device_id)
> > +		writew(soc->device_id, port->base + PCIE_CONF_DEVICE_ID);
> > +
> >  	/* 100ms timeout value should be enough for Gen1/2 training */
> >  	err = readl_poll_timeout(port->base + PCIE_LINK_STATUS_V2, val,
> >  				 !!(val & PCIE_PORT_LINKUP_V2), 20,
> > @@ -1216,11 +1224,21 @@ static const struct mtk_pcie_soc mtk_pcie_soc_mt7622 = {
> >  	.setup_irq = mtk_pcie_setup_irq,
> >  };
> >  
> > +static const struct mtk_pcie_soc mtk_pcie_soc_mt7629 = {
> > +	.need_fix_class_id = true,
> > +	.need_fix_device_id = true,
> > +	.device_id = PCI_DEVICE_ID_MEDIATEK_7629,
> > +	.ops = &mtk_pcie_ops_v2,
> > +	.startup = mtk_pcie_startup_port_v2,
> > +	.setup_irq = mtk_pcie_setup_irq,
> > +};
> > +
> >  static const struct of_device_id mtk_pcie_ids[] = {
> >  	{ .compatible = "mediatek,mt2701-pcie", .data = &mtk_pcie_soc_v1 },
> >  	{ .compatible = "mediatek,mt7623-pcie", .data = &mtk_pcie_soc_v1 },
> >  	{ .compatible = "mediatek,mt2712-pcie", .data = &mtk_pcie_soc_mt2712 },
> >  	{ .compatible = "mediatek,mt7622-pcie", .data = &mtk_pcie_soc_mt7622 },
> > +	{ .compatible = "mediatek,mt7629-pcie", .data = &mtk_pcie_soc_mt7629 },
> >  	{},
> >  };
> >  
> > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> > index 70e86148cb1e..aa32962759b2 100644
> > --- a/include/linux/pci_ids.h
> > +++ b/include/linux/pci_ids.h
> > @@ -2131,6 +2131,7 @@
> >  #define PCI_VENDOR_ID_MYRICOM		0x14c1
> >  
> >  #define PCI_VENDOR_ID_MEDIATEK		0x14c3
> > +#define PCI_DEVICE_ID_MEDIATEK_7629	0x7629
> >  
> >  #define PCI_VENDOR_ID_TITAN		0x14D2
> >  #define PCI_DEVICE_ID_TITAN_010L	0x8001
> 
> Hi Bjorn & Lorenzo,
> 
> Is this patch ok or is there anything I need to fixed?

The commit log need to be fixed and I will do it, the code if
Bjorn is OK with it I can merge it.

Lorenzo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 1/2] drm: add cache support for arm64
From: Rob Clark @ 2019-08-06 16:23 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Sean Paul, Maxime Ripard, Catalin Marinas, Maarten Lankhorst,
	LKML, dri-devel, David Airlie, Rob Clark, linux-arm-kernel,
	Daniel Vetter, Greg Kroah-Hartman, Thomas Gleixner, Will Deacon,
	Allison Randal
In-Reply-To: <20190806155044.GC25050@lst.de>

On Tue, Aug 6, 2019 at 8:50 AM Christoph Hellwig <hch@lst.de> wrote:
>
> On Tue, Aug 06, 2019 at 07:11:41AM -0700, Rob Clark wrote:
> > Agreed that drm_cflush_* isn't a great API.  In this particular case
> > (IIUC), I need wb+inv so that there aren't dirty cache lines that drop
> > out to memory later, and so that I don't get a cache hit on
> > uncached/wc mmap'ing.
>
> So what is the use case here?  Allocate pages using the page allocator
> (or CMA for that matter), and then mmaping them to userspace and never
> touching them again from the kernel?

Currently, it is pages coming from tmpfs.  Ideally we want pages that
are swappable when unpinned.

CPU mappings are *mostly* just mapping to userspace.  There are a few
exceptions that are vmap'd (fbcon, and ringbuffer).

(Eventually I'd like to support pages passed in from userspace.. but
that is down the road.)

> > Tying it in w/ iommu seems a bit weird to me.. but maybe that is just
> > me, I'm certainly willing to consider proposals or to try things and
> > see how they work out.
>
> This was just my through as the fit seems easy.  But maybe you'll
> need to explain your use case(s) a bit more so that we can figure out
> what a good high level API is.

Tying it to iommu_map/unmap would be awkward, as we could need to
setup cpu mmap before it ends up mapped to iommu.  And the plan to
support per-process pagetables involved creating an iommu_domain per
userspace gl context.. some buffers would end up mapped into multiple
contexts/iommu_domains.

If the cache operation was detached from iommu_map/unmap, then it
would seem weird to be part of the iommu API.

I guess I'm not entirely sure what you had in mind, but this is why
iommu seemed to me like a bad fit.

> > Exposing the arch_sync_* API and using that directly (bypassing
> > drm_cflush_*) actually seems pretty reasonable and pragmatic.  I did
> > have one doubt, as phys_to_virt() is only valid for kernel direct
> > mapped memory (AFAIU), what happens for pages that are not in kernel
> > linear map?  Maybe it is ok to ignore those pages, since they won't
> > have an aliased mapping?
>
> They could have an aliased mapping in vmalloc/vmap space for example,
> if you created one.  We have the flush_kernel_vmap_range /
> invalidate_kernel_vmap_range APIs for those, that are implement
> on architectures with VIVT caches.

If I only have to worry about aliased mappings that I create myself,
that doesn't seem too bad..  I could track that myself easily enough.

BR,
-R

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 5/6] ARM: psci: cpuidle: Enable PSCI CPUidle driver
From: Sudeep Holla @ 2019-08-06 16:16 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Mark Rutland, Ulf Hansson, linux-pm, Catalin Marinas,
	Daniel Lezcano, Rafael J. Wysocki, LKML, Will Deacon, LAKML
In-Reply-To: <20190722153745.32446-6-lorenzo.pieralisi@arm.com>

On Mon, Jul 22, 2019 at 04:37:44PM +0100, Lorenzo Pieralisi wrote:
> Allow selection of the PSCI CPUidle in the kernel by adding
> the required Kconfig options.
> 
> Remove PSCI callbacks from ARM/ARM64 generic CPU ops
> to prevent the PSCI idle driver from clashing with the generic
> ARM CPUidle driver initialization, that relies on CPU ops
> to initialize and enter idle states.
> 
> Update the affected defconfig files to guarantee seamingless
> transition from the generic ARM CPUidle to the PSCI CPUidle
> driver on arch/platforms using it.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> ---
>  arch/arm/configs/imx_v6_v7_defconfig | 1 +
>  arch/arm64/configs/defconfig         | 1 +

Better to keep above you as separate patch, though it may cause
minor issues from bisectibility. It may be needed anyway for merging.

>  arch/arm64/kernel/cpuidle.c          | 7 ++++---
>  arch/arm64/kernel/psci.c             | 4 ----
>  drivers/cpuidle/Kconfig.arm          | 8 ++++++--
>  drivers/firmware/psci/psci.c         | 9 ---------
>  6 files changed, 12 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
> index a53b29251ed4..4174fd1b79e7 100644
> --- a/arch/arm/configs/imx_v6_v7_defconfig
> +++ b/arch/arm/configs/imx_v6_v7_defconfig
> @@ -60,6 +60,7 @@ CONFIG_ARM_IMX6Q_CPUFREQ=y
>  CONFIG_ARM_IMX_CPUFREQ_DT=y
>  CONFIG_CPU_IDLE=y
>  CONFIG_ARM_CPUIDLE=y
> +CONFIG_ARM_PSCI_CPUIDLE=y
>  CONFIG_VFP=y
>  CONFIG_NEON=y
>  CONFIG_PM_DEBUG=y
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 0e58ef02880c..c0a7cfe3aebd 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -72,6 +72,7 @@ CONFIG_RANDOMIZE_BASE=y
>  CONFIG_HIBERNATION=y
>  CONFIG_WQ_POWER_EFFICIENT_DEFAULT=y
>  CONFIG_ARM_CPUIDLE=y
> +CONFIG_ARM_PSCI_CPUIDLE=y
>  CONFIG_CPU_FREQ=y
>  CONFIG_CPU_FREQ_STAT=y
>  CONFIG_CPU_FREQ_GOV_POWERSAVE=m
> diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
> index d1048173fd8a..4bcd1bca0dfc 100644
> --- a/arch/arm64/kernel/cpuidle.c
> +++ b/arch/arm64/kernel/cpuidle.c
> @@ -11,6 +11,7 @@
>  #include <linux/cpu_pm.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/psci.h>
>  
>  #include <asm/cpuidle.h>
>  #include <asm/cpu_ops.h>
> @@ -48,15 +49,15 @@ int arm_cpuidle_suspend(int index)
>  
>  int acpi_processor_ffh_lpi_probe(unsigned int cpu)
>  {
> -	return arm_cpuidle_init(cpu);
> +	return psci_acpi_cpu_init_idle(cpu);

This will break build as psci_acpi_cpu_init_idle is introduced in next patch.
You can simply move it to next patch I assume.

>  }
>  
>  int acpi_processor_ffh_lpi_enter(struct acpi_lpi_state *lpi)
>  {
>  	if (ARM64_LPI_IS_RETENTION_STATE(lpi->arch_flags))
> -		return CPU_PM_CPU_IDLE_ENTER_RETENTION(arm_cpuidle_suspend,
> +		return CPU_PM_CPU_IDLE_ENTER_RETENTION(psci_cpu_suspend_enter,
>  						lpi->index);
>  	else
> -		return CPU_PM_CPU_IDLE_ENTER(arm_cpuidle_suspend, lpi->index);
> +		return CPU_PM_CPU_IDLE_ENTER(psci_cpu_suspend_enter, lpi->index);
>  }
>  #endif
> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
> index 85ee7d07889e..a543ab7e007c 100644
> --- a/arch/arm64/kernel/psci.c
> +++ b/arch/arm64/kernel/psci.c
> @@ -105,10 +105,6 @@ static int cpu_psci_cpu_kill(unsigned int cpu)
>  
>  const struct cpu_operations cpu_psci_ops = {
>  	.name		= "psci",
> -#ifdef CONFIG_CPU_IDLE
> -	.cpu_init_idle	= psci_cpu_init_idle,
> -	.cpu_suspend	= psci_cpu_suspend_enter,
> -#endif
>  	.cpu_init	= cpu_psci_cpu_init,
>  	.cpu_prepare	= cpu_psci_cpu_prepare,
>  	.cpu_boot	= cpu_psci_cpu_boot,
> diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
> index 929b57424ea4..b9c56c60ab98 100644
> --- a/drivers/cpuidle/Kconfig.arm
> +++ b/drivers/cpuidle/Kconfig.arm
> @@ -14,8 +14,12 @@ config ARM_CPUIDLE
>            provided by architecture code.
>  
>  config ARM_PSCI_CPUIDLE
> -	bool
> -
> +	bool "PSCI CPU idle Driver"

As mentioned in previous patch, do you see issues having just above
change in this patch and the below ones moved to the previous.

> +	depends on ARM_PSCI_FW
> +	select DT_IDLE_STATES
> +	select CPU_IDLE_MULTIPLE_DRIVERS
> +	help
> +	  Select this to enable PSCI firmware based CPUidle driver for ARM.

You need extra blank line here.

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 4/6] ARM: psci: cpuidle: Introduce PSCI CPUidle driver
From: Sudeep Holla @ 2019-08-06 16:10 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Mark Rutland, Ulf Hansson, linux-pm, Catalin Marinas,
	Daniel Lezcano, Rafael J. Wysocki, LKML, Will Deacon, LAKML
In-Reply-To: <20190722153745.32446-5-lorenzo.pieralisi@arm.com>

On Mon, Jul 22, 2019 at 04:37:43PM +0100, Lorenzo Pieralisi wrote:
> PSCI firmware is the standard power management control for
> all ARM64 based platforms and it is also deployed on some
> ARM 32 bit platforms to date.
>
> Idle state entry in PSCI is currently achieved by calling
> arm_cpuidle_init() and arm_cpuidle_suspend() in a generic
> idle driver, which in turn relies on ARM/ARM64 CPUidle back-end
> to relay the call into PSCI firmware if PSCI is the boot method.
>
> Given that PSCI is the standard idle entry method on ARM64 systems
> (which means that no other CPUidle driver are expected on ARM64
> platforms - so PSCI is already a generic idle driver), in order to
> simplify idle entry and code maintenance, it makes sense to have a PSCI
> specific idle driver so that idle code that it is currently living in
> drivers/firmware directory can be hoisted out of it and moved
> where it belongs, into a full-fledged PSCI driver, leaving PSCI code
> in drivers/firmware as a pure firmware interface, as it should be.
>
> Implement a PSCI CPUidle driver. By default it is a silent Kconfig entry
> which is left unselected, since it selection would clash with the
> generic ARM CPUidle driver that provides a PSCI based idle driver
> through the arm/arm64 arches back-ends CPU operations.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>

Once the error path issues pointed by Ulf are resolved,

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> ---
>  MAINTAINERS                    |   8 ++
>  drivers/cpuidle/Kconfig.arm    |   3 +
>  drivers/cpuidle/Makefile       |   1 +
>  drivers/cpuidle/cpuidle-psci.c | 150 +++++++++++++++++++++++++++++++++
>  4 files changed, 162 insertions(+)
>  create mode 100644 drivers/cpuidle/cpuidle-psci.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 783569e3c4b4..c2bf8ce65e83 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4286,6 +4286,14 @@ S:	Supported
>  F:	drivers/cpuidle/cpuidle-exynos.c
>  F:	arch/arm/mach-exynos/pm.c
>
> +CPUIDLE DRIVER - ARM PSCI
> +M:	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> +M:	Sudeep Holla <sudeep.holla@arm.com>
> +L:	linux-pm@vger.kernel.org
> +L:	linux-arm-kernel@lists.infradead.org
> +S:	Supported
> +F:	drivers/cpuidle/cpuidle-psci.c
> +
>  CPU IDLE TIME MANAGEMENT FRAMEWORK
>  M:	"Rafael J. Wysocki" <rjw@rjwysocki.net>
>  M:	Daniel Lezcano <daniel.lezcano@linaro.org>
> diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
> index 48cb3d4bb7d1..929b57424ea4 100644
> --- a/drivers/cpuidle/Kconfig.arm
> +++ b/drivers/cpuidle/Kconfig.arm
> @@ -13,6 +13,9 @@ config ARM_CPUIDLE
>            initialized by calling the CPU operations init idle hook
>            provided by architecture code.
>
> +config ARM_PSCI_CPUIDLE
> +	bool
> +

[nit] I understand the intention to keep it hidden, but can't we have
the dependency and selection of other config as part of this patch to
make it more complete ?

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] dma-mapping: fix page attributes for dma_mmap_*
From: Will Deacon @ 2019-08-06 16:08 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Shawn Anastasio, Michael Ellerman, linuxppc-dev, Russell King,
	linux-kernel, iommu, Catalin Marinas, Robin Murphy,
	linux-arm-kernel
In-Reply-To: <20190803064812.GA29746@lst.de>

On Sat, Aug 03, 2019 at 08:48:12AM +0200, Christoph Hellwig wrote:
> On Fri, Aug 02, 2019 at 11:38:03AM +0100, Will Deacon wrote:
> > 
> > So this boils down to a terminology mismatch. The Arm architecture doesn't have
> > anything called "write combine", so in Linux we instead provide what the Arm
> > architecture calls "Normal non-cacheable" memory for pgprot_writecombine().
> > Amongst other things, this memory type permits speculation, unaligned accesses
> > and merging of writes. I found something in the architecture spec about
> > non-cachable memory, but it's written in Armglish[1].
> > 
> > pgprot_noncached(), on the other hand, provides what the architecture calls
> > Strongly Ordered or Device-nGnRnE memory. This is intended for mapping MMIO
> > (i.e. PCI config space) and therefore forbids speculation, preserves access
> > size, requires strict alignment and also forces write responses to come from
> > the endpoint.
> > 
> > I think the naming mismatch is historical, but on arm64 we wanted to use the
> > same names as arm32 so that any drivers using these things directly would get
> > the same behaviour.
> 
> That all makes sense, but it totally needs a comment.  I'll try to draft
> one based on this.  I've also looked at the arm32 code a bit more, and
> it seems arm always (?) supported Normal non-cacheable attribute, but
> Linux only optionally uses it for arm v6+ because of fears of drivers
> missing barriers.

I think it was also to do with aliasing, but I don't recall all of the
details.

> The other really weird things is that in arm32
> pgprot_dmacoherent incudes the L_PTE_XN bit, which from my understanding
> is the no-execture bit, but pgprot_writecombine does not.  This seems to
> not very unintentional.  So minus that the whole DMA_ATTR_WRITE_COMBІNE
> seems to be about flagging old arm specific drivers as having the proper
> barriers in places and otherwise is a no-op.

I think it only matters for Armv7 CPUs, but yes, we should probably be
setting L_PTE_XN for both of these memory types.

> Here is my tentative plan:
> 
>  - respin this patch with a small fix to handle the
>    DMA_ATTR_NON_CONSISTENT (as in ignore it unless actually supported),
>    but keep the name as-is to avoid churn.  This should allow 5.3
>    inclusion and backports
>  - remove DMA_ATTR_WRITE_COMBINE support from mips, probably also 5.3
>    material.
>  - move all architectures but arm over to just define
>    pgprot_dmacoherent, including a comment with the above explanation
>    for arm64.

That would be great, thanks.

>  - make DMA_ATTR_WRITE_COMBINE a no-op and schedule it for removal,
>    thus removing the last instances of arch_dma_mmap_pgprot

All sounds good to me, although I suppose 32-bit Arm platforms without
CONFIG_ARM_DMA_MEM_BUFFERABLE may run into issues if DMA_ATTR_WRITE_COMBINE
disappears. Only one way to find out...

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v8 4/4] arm64: dts: allwinner: orange-pi-3: Enable HDMI output
From: megous @ 2019-08-06 15:57 UTC (permalink / raw)
  To: linux-sunxi, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Jernej Škrabec
  Cc: Ondrej Jirman, Mark Rutland, devicetree, David Airlie,
	linux-kernel, dri-devel, Daniel Vetter, linux-arm-kernel
In-Reply-To: <20190806155744.10263-1-megous@megous.com>

From: Ondrej Jirman <megous@megous.com>

Orange Pi 3 has a DDC_CEC_EN signal connected to PH2, that enables the DDC
I2C bus voltage shifter. Before EDID can be read, we need to pull PH2 high.
This is realized by the ddc-en-gpios property.

Signed-off-by: Ondrej Jirman <megous@megous.com>
---
 .../dts/allwinner/sun50i-h6-orangepi-3.dts    | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
index 2c6807b74ff6..01bb1bafe284 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
@@ -22,6 +22,18 @@
 		stdout-path = "serial0:115200n8";
 	};
 
+	connector {
+		compatible = "hdmi-connector";
+		ddc-en-gpios = <&pio 7 2 GPIO_ACTIVE_HIGH>; /* PH2 */
+		type = "a";
+
+		port {
+			hdmi_con_in: endpoint {
+				remote-endpoint = <&hdmi_out_con>;
+			};
+		};
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
@@ -72,6 +84,10 @@
 	cpu-supply = <&reg_dcdca>;
 };
 
+&de {
+	status = "okay";
+};
+
 &ehci0 {
 	status = "okay";
 };
@@ -91,6 +107,16 @@
 	status = "okay";
 };
 
+&hdmi {
+	status = "okay";
+};
+
+&hdmi_out {
+	hdmi_out_con: endpoint {
+		remote-endpoint = <&hdmi_con_in>;
+	};
+};
+
 &mdio {
 	ext_rgmii_phy: ethernet-phy@1 {
 		compatible = "ethernet-phy-ieee802.3-c22";
-- 
2.22.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v8 3/4] drm: sun4i: Add support for enabling DDC I2C bus to sun8i_dw_hdmi glue
From: megous @ 2019-08-06 15:57 UTC (permalink / raw)
  To: linux-sunxi, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Jernej Škrabec
  Cc: Ondrej Jirman, Mark Rutland, Jernej Skrabec, devicetree,
	David Airlie, linux-kernel, dri-devel, Daniel Vetter,
	linux-arm-kernel
In-Reply-To: <20190806155744.10263-1-megous@megous.com>

From: Ondrej Jirman <megous@megous.com>

Orange Pi 3 board requires enabling a voltage shifting circuit via GPIO
for the DDC bus to be usable.

Add support for hdmi-connector node's optional ddc-en-gpios property to
support this use case.

Signed-off-by: Ondrej Jirman <megous@megous.com>
Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net>
---
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 54 +++++++++++++++++++++++++--
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h |  2 +
 2 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
index 8ca5af0c912f..a44dca4b0219 100644
--- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
+++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
@@ -97,10 +97,34 @@ static u32 sun8i_dw_hdmi_find_possible_crtcs(struct drm_device *drm,
 	return crtcs;
 }
 
+static int sun8i_dw_hdmi_find_connector_pdev(struct device *dev,
+					     struct platform_device **pdev_out)
+{
+	struct platform_device *pdev;
+	struct device_node *remote;
+
+	remote = of_graph_get_remote_node(dev->of_node, 1, -1);
+	if (!remote)
+		return -ENODEV;
+
+	if (!of_device_is_compatible(remote, "hdmi-connector")) {
+		of_node_put(remote);
+		return -ENODEV;
+	}
+
+	pdev = of_find_device_by_node(remote);
+	of_node_put(remote);
+	if (!pdev)
+		return -ENODEV;
+
+	*pdev_out = pdev;
+	return 0;
+}
+
 static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master,
 			      void *data)
 {
-	struct platform_device *pdev = to_platform_device(dev);
+	struct platform_device *pdev = to_platform_device(dev), *connector_pdev;
 	struct dw_hdmi_plat_data *plat_data;
 	struct drm_device *drm = data;
 	struct device_node *phy_node;
@@ -150,16 +174,30 @@ static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master,
 		return PTR_ERR(hdmi->regulator);
 	}
 
+	ret = sun8i_dw_hdmi_find_connector_pdev(dev, &connector_pdev);
+	if (!ret) {
+		hdmi->ddc_en = gpiod_get_optional(&connector_pdev->dev,
+						  "ddc-en", GPIOD_OUT_HIGH);
+		platform_device_put(connector_pdev);
+
+		if (IS_ERR(hdmi->ddc_en)) {
+			dev_err(dev, "Couldn't get ddc-en gpio\n");
+			return PTR_ERR(hdmi->ddc_en);
+		}
+	}
+
 	ret = regulator_enable(hdmi->regulator);
 	if (ret) {
 		dev_err(dev, "Failed to enable regulator\n");
-		return ret;
+		goto err_unref_ddc_en;
 	}
 
+	gpiod_set_value(hdmi->ddc_en, 1);
+
 	ret = reset_control_deassert(hdmi->rst_ctrl);
 	if (ret) {
 		dev_err(dev, "Could not deassert ctrl reset control\n");
-		goto err_disable_regulator;
+		goto err_disable_ddc_en;
 	}
 
 	ret = clk_prepare_enable(hdmi->clk_tmds);
@@ -212,8 +250,12 @@ static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master,
 	clk_disable_unprepare(hdmi->clk_tmds);
 err_assert_ctrl_reset:
 	reset_control_assert(hdmi->rst_ctrl);
-err_disable_regulator:
+err_disable_ddc_en:
+	gpiod_set_value(hdmi->ddc_en, 0);
 	regulator_disable(hdmi->regulator);
+err_unref_ddc_en:
+	if (hdmi->ddc_en)
+		gpiod_put(hdmi->ddc_en);
 
 	return ret;
 }
@@ -227,7 +269,11 @@ static void sun8i_dw_hdmi_unbind(struct device *dev, struct device *master,
 	sun8i_hdmi_phy_remove(hdmi);
 	clk_disable_unprepare(hdmi->clk_tmds);
 	reset_control_assert(hdmi->rst_ctrl);
+	gpiod_set_value(hdmi->ddc_en, 0);
 	regulator_disable(hdmi->regulator);
+
+	if (hdmi->ddc_en)
+		gpiod_put(hdmi->ddc_en);
 }
 
 static const struct component_ops sun8i_dw_hdmi_ops = {
diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
index 720c5aa8adc1..d707c9171824 100644
--- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
+++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
@@ -9,6 +9,7 @@
 #include <drm/bridge/dw_hdmi.h>
 #include <drm/drm_encoder.h>
 #include <linux/clk.h>
+#include <linux/gpio/consumer.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
@@ -190,6 +191,7 @@ struct sun8i_dw_hdmi {
 	struct regulator		*regulator;
 	const struct sun8i_dw_hdmi_quirks *quirks;
 	struct reset_control		*rst_ctrl;
+	struct gpio_desc		*ddc_en;
 };
 
 static inline struct sun8i_dw_hdmi *
-- 
2.22.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v8 1/4] arm64: dts: allwinner: orange-pi-3: Enable ethernet
From: megous @ 2019-08-06 15:57 UTC (permalink / raw)
  To: linux-sunxi, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Jernej Škrabec
  Cc: Ondrej Jirman, Mark Rutland, devicetree, David Airlie,
	linux-kernel, dri-devel, Daniel Vetter, linux-arm-kernel
In-Reply-To: <20190806155744.10263-1-megous@megous.com>

From: Ondrej Jirman <megous@megous.com>

Orange Pi 3 has two regulators that power the Realtek RTL8211E. According
to the phy datasheet, both regulators need to be enabled at the same time,
but we can only specify a single phy-supply in the DT.

This can be achieved by making one regulator depedning on the other via
vin-supply. While it's not a technically correct description of the
hardware, it achieves the purpose.

All values of RX/TX delay were tested exhaustively and a middle one of the
working values was chosen.

Signed-off-by: Ondrej Jirman <megous@megous.com>
---
 .../dts/allwinner/sun50i-h6-orangepi-3.dts    | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
index 17d496990108..2c6807b74ff6 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
@@ -15,6 +15,7 @@
 
 	aliases {
 		serial0 = &uart0;
+		ethernet0 = &emac;
 	};
 
 	chosen {
@@ -44,6 +45,27 @@
 		regulator-max-microvolt = <5000000>;
 		regulator-always-on;
 	};
+
+	/*
+	 * The board uses 2.5V RGMII signalling. Power sequence to enable
+	 * the phy is to enable GMAC-2V5 and GMAC-3V (aldo2) power rails
+	 * at the same time and to wait 100ms.
+	 */
+	reg_gmac_2v5: gmac-2v5 {
+		compatible = "regulator-fixed";
+		regulator-name = "gmac-2v5";
+		regulator-min-microvolt = <2500000>;
+		regulator-max-microvolt = <2500000>;
+		startup-delay-us = <100000>;
+		enable-active-high;
+		gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
+
+		/* The real parent of gmac-2v5 is reg_vcc5v, but we need to
+		 * enable two regulators to power the phy. This is one way
+		 * to achieve that.
+		 */
+		vin-supply = <&reg_aldo2>; /* GMAC-3V */
+	};
 };
 
 &cpu0 {
@@ -58,6 +80,28 @@
 	status = "okay";
 };
 
+&emac {
+	pinctrl-names = "default";
+	pinctrl-0 = <&ext_rgmii_pins>;
+	phy-mode = "rgmii";
+	phy-handle = <&ext_rgmii_phy>;
+	phy-supply = <&reg_gmac_2v5>;
+	allwinner,rx-delay-ps = <1500>;
+	allwinner,tx-delay-ps = <700>;
+	status = "okay";
+};
+
+&mdio {
+	ext_rgmii_phy: ethernet-phy@1 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <1>;
+
+		reset-gpios = <&pio 3 14 GPIO_ACTIVE_LOW>; /* PD14 */
+		reset-assert-us = <15000>;
+		reset-deassert-us = <40000>;
+	};
+};
+
 &mmc0 {
 	vmmc-supply = <&reg_cldo1>;
 	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */
-- 
2.22.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v8 0/4] Add support for Orange Pi 3
From: megous @ 2019-08-06 15:57 UTC (permalink / raw)
  To: linux-sunxi, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Jernej Škrabec
  Cc: Ondrej Jirman, Mark Rutland, devicetree, David Airlie,
	linux-kernel, dri-devel, Daniel Vetter, linux-arm-kernel

From: Ondrej Jirman <megous@megous.com>

This series implements support for Xunlong Orange Pi 3 board. There
are only a few patches remaining.

- ethernet support - just a DT change (patch 1)
- HDMI support (patches 2-4)

For some people, ethernet doesn't work after reboot because u-boot doesn't
support AXP805 PMIC, and will not turn off the etherent PHY regulators.
So the regulator controlled by gpio will be shut down, but the other one
controlled by the AXP PMIC will not.

This is a problem only when running with a builtin driver. This needs
to be fixed in u-boot.


Please take a look.

thank you and regards,
  Ondrej Jirman

Changes in v8:
- added reviewed-by tags
- dropped already applied patches
- added more info about the phy initialization issue after reset

Changes in v7:
- dropped stored reference to connector_pdev as suggested by Jernej
- added forgotten dt-bindings reviewed-by tag

Changes in v6:
- added dt-bindings reviewed-by tag
- fix wording in stmmac commit (as suggested by Sergei)

Changes in v5:
- dropped already applied patches (pinctrl patches, mmc1 pinconf patch)
- rename GMAC-3V3 -> GMAC-3V to match the schematic (Jagan)
- changed hdmi-connector's ddc-supply property to ddc-en-gpios
  (Rob Herring)

Changes in v4:
- fix checkpatch warnings/style issues
- use enum in struct sunxi_desc_function for io_bias_cfg_variant
- collected acked-by's
- fix compile error in drivers/pinctrl/sunxi/pinctrl-sun9i-a80-r.c:156
  caused by missing conversion from has_io_bias_cfg struct member
  (I've kept the acked-by, because it's a trivial change, but feel free
  to object.) (reported by Martin A. on github)
  I did not have A80 pinctrl enabled for some reason, so I did not catch
  this sooner.
- dropped brcm firmware patch (was already applied)
- dropped the wifi dts patch (will re-send after H6 RTC gets merged,
  along with bluetooth support, in a separate series)

Changes in v3:
- dropped already applied patches
- changed pinctrl I/O bias selection constants to enum and renamed
- added /omit-if-no-ref/ to mmc1_pins
- made mmc1_pins default pinconf for mmc1 in H6 dtsi
- move ddc-supply to HDMI connector node, updated patch descriptions,
  changed dt-bindings docs

Changes in v2:
- added dt-bindings documentation for the board's compatible string
  (suggested by Clement)
- addressed checkpatch warnings and code formatting issues (on Maxime's
  suggestions)
- stmmac: dropped useless parenthesis, reworded description of the patch
  (suggested by Sergei)
- drop useles dev_info() about the selected io bias voltage
- docummented io voltage bias selection variant macros
- wifi: marked WiFi DTS patch and realted mmc1_pins as "DO NOT MERGE",
  because wifi depends on H6 RTC support that's not merged yet (suggested
  by Clement)
- added missing signed-of-bys
- changed &usb2otg dr_mode to otg, and added a note about VBUS
- improved wording of HDMI driver's DDC power supply patch

Ondrej Jirman (4):
  arm64: dts: allwinner: orange-pi-3: Enable ethernet
  dt-bindings: display: hdmi-connector: Support DDC bus enable
  drm: sun4i: Add support for enabling DDC I2C bus to sun8i_dw_hdmi glue
  arm64: dts: allwinner: orange-pi-3: Enable HDMI output

 .../display/connector/hdmi-connector.txt      |  1 +
 .../dts/allwinner/sun50i-h6-orangepi-3.dts    | 70 +++++++++++++++++++
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c         | 54 ++++++++++++--
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h         |  2 +
 4 files changed, 123 insertions(+), 4 deletions(-)

-- 
2.22.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v8 2/4] dt-bindings: display: hdmi-connector: Support DDC bus enable
From: megous @ 2019-08-06 15:57 UTC (permalink / raw)
  To: linux-sunxi, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Jernej Škrabec
  Cc: Ondrej Jirman, Mark Rutland, devicetree, David Airlie,
	linux-kernel, dri-devel, Daniel Vetter, Rob Herring,
	linux-arm-kernel
In-Reply-To: <20190806155744.10263-1-megous@megous.com>

From: Ondrej Jirman <megous@megous.com>

Some Allwinner SoC using boards (Orange Pi 3 for example) need to enable
on-board voltage shifting logic for the DDC bus using a gpio to be able
to access DDC bus. Use ddc-en-gpios property on the hdmi-connector to
model this.

Add binding documentation for optional ddc-en-gpios property.

Signed-off-by: Ondrej Jirman <megous@megous.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/display/connector/hdmi-connector.txt     | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/connector/hdmi-connector.txt b/Documentation/devicetree/bindings/display/connector/hdmi-connector.txt
index 508aee461e0d..aeb07c4bd703 100644
--- a/Documentation/devicetree/bindings/display/connector/hdmi-connector.txt
+++ b/Documentation/devicetree/bindings/display/connector/hdmi-connector.txt
@@ -9,6 +9,7 @@ Optional properties:
 - label: a symbolic name for the connector
 - hpd-gpios: HPD GPIO number
 - ddc-i2c-bus: phandle link to the I2C controller used for DDC EDID probing
+- ddc-en-gpios: signal to enable DDC bus
 
 Required nodes:
 - Video port for HDMI input
-- 
2.22.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH] arm64/kvm: remove VMID rollover I-cache maintenance
From: Mark Rutland @ 2019-08-06 15:57 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Mark Rutland, Suzuki K Poulose, Marc Zyngier, Christoffer Dall,
	James Morse, kvmarm, Julien Thierry

For VPIPT I-caches, we need I-cache maintenance on VMID rollover to
avoid an ABA problem. Consider a single vCPU VM, with a pinned stage-2,
running with an idmap VA->IPA and idmap IPA->PA. If we don't do
maintenance on rollover:

        // VMID A
        Writes insn X to PA 0xF
        Invalidates PA 0xF (for VMID A)

        I$ contains [{A,F}->X]

        [VMID ROLLOVER]

        // VMID B
        Writes insn Y to PA 0xF
        Invalidates PA 0xF (for VMID B)

        I$ contains [{A,F}->X, {B,F}->Y]

        [VMID ROLLOVER]

        // VMID A
        I$ contains [{A,F}->X, {B,F}->Y]

        Unexpectedly hits stale I$ line {A,F}->X.

However, for PIPT and VIPT I-caches, the VMID doesn't affect lookup or
constrain maintenance. Given the VMID doesn't affect PIPT and VIPT
I-caches, and given VMID rollover is independent of changes to stage-2
mappings, I-cache maintenance cannot be necessary on VMID rollover for
PIPT or VIPT I-caches.

This patch removes the maintenance on rollover for VIPT and PIPT
I-caches. At the same time, the unnecessary colons are removed from the
asm statement to make it more legible.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Christoffer Dall <christoffer.dall@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Julien Thierry <julien.thierry.kdev@gmail.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: kvmarm@lists.cs.columbia.edu
---
 arch/arm64/kvm/hyp/tlb.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/tlb.c b/arch/arm64/kvm/hyp/tlb.c
index d49a14497715..c466060b76d6 100644
--- a/arch/arm64/kvm/hyp/tlb.c
+++ b/arch/arm64/kvm/hyp/tlb.c
@@ -193,6 +193,18 @@ void __hyp_text __kvm_flush_vm_context(void)
 {
 	dsb(ishst);
 	__tlbi(alle1is);
-	asm volatile("ic ialluis" : : );
+
+	/*
+	 * VIPT and PIPT caches are not affected by VMID, so no maintenance
+	 * is necessary across a VMID rollover.
+	 *
+	 * VPIPT caches constrain lookup and maintenance to the active VMID,
+	 * so we need to invalidate lines with a stale VMID to avoid an ABA
+	 * race after multiple rollovers.
+	 *
+	 */
+	if (icache_is_vpipt())
+		asm volatile("ic ialluis");
+
 	dsb(ish);
 }
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH 3/6] drivers: firmware: psci: Decouple checker from generic ARM CPUidle
From: Sudeep Holla @ 2019-08-06 15:54 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Mark Rutland, Ulf Hansson, linux-pm, Catalin Marinas,
	Daniel Lezcano, Rafael J. Wysocki, LKML, Will Deacon, LAKML
In-Reply-To: <20190722153745.32446-4-lorenzo.pieralisi@arm.com>

On Mon, Jul 22, 2019 at 04:37:42PM +0100, Lorenzo Pieralisi wrote:
> The PSCI checker currently relies on the generic ARM CPUidle
> infrastructure to enter an idle state, which in turn creates
> a dependency that is not really needed.
> 
> The PSCI checker code to test PSCI CPU suspend is built on
> top of the CPUidle framework and can easily reuse the
> struct cpuidle_state.enter() function (previously initialized
> by an idle driver, with a PSCI back-end) to trigger an entry
> into an idle state, decoupling the PSCI checker from the
> generic ARM CPUidle infrastructure and simplyfing the code
> in the process.
> 
> Convert the PSCI checker suspend entry function to use
> the struct cpuidle_state.enter() function callback.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>

Not sure why we didn't take this path from the beginning.
Anyways make sense and much needed for later patches in the series.

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 2/6] ARM: cpuidle: Remove overzealous error logging
From: Sudeep Holla @ 2019-08-06 15:51 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Mark Rutland, Ulf Hansson, linux-pm, Catalin Marinas,
	Daniel Lezcano, Rafael J. Wysocki, LKML, Will Deacon, LAKML
In-Reply-To: <20190722153745.32446-3-lorenzo.pieralisi@arm.com>

On Mon, Jul 22, 2019 at 04:37:41PM +0100, Lorenzo Pieralisi wrote:
> CPUidle back-end operations are not implemented in some platforms
> but this should not be considered an error serious enough to be
> logged. Check the arm_cpuidle_init() return value to detect whether
> the failure must be reported or not in the kernel log and do
> not log it if the platform does not support CPUidle operations.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 1/6] ARM: cpuidle: Remove useless header include
From: Sudeep Holla @ 2019-08-06 15:51 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Mark Rutland, Ulf Hansson, linux-pm, Catalin Marinas,
	Daniel Lezcano, Rafael J. Wysocki, LKML, Will Deacon, LAKML
In-Reply-To: <20190722153745.32446-2-lorenzo.pieralisi@arm.com>

On Mon, Jul 22, 2019 at 04:37:40PM +0100, Lorenzo Pieralisi wrote:
> The generic ARM CPUidle driver includes <linux/topology.h> by mistake.
> 
> Remove the topology header include.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 1/2] drm: add cache support for arm64
From: Christoph Hellwig @ 2019-08-06 15:50 UTC (permalink / raw)
  To: Rob Clark
  Cc: Sean Paul, Maxime Ripard, Catalin Marinas, Maarten Lankhorst,
	LKML, dri-devel, David Airlie, Rob Clark, linux-arm-kernel,
	Daniel Vetter, Greg Kroah-Hartman, Thomas Gleixner, Will Deacon,
	Christoph Hellwig, Allison Randal
In-Reply-To: <CAJs_Fx6eh1w7c=crMoD5XyEOMzP6orLhqUewErE51cPGYmObBQ@mail.gmail.com>

On Tue, Aug 06, 2019 at 07:11:41AM -0700, Rob Clark wrote:
> Agreed that drm_cflush_* isn't a great API.  In this particular case
> (IIUC), I need wb+inv so that there aren't dirty cache lines that drop
> out to memory later, and so that I don't get a cache hit on
> uncached/wc mmap'ing.

So what is the use case here?  Allocate pages using the page allocator
(or CMA for that matter), and then mmaping them to userspace and never
touching them again from the kernel?

> Tying it in w/ iommu seems a bit weird to me.. but maybe that is just
> me, I'm certainly willing to consider proposals or to try things and
> see how they work out.

This was just my through as the fit seems easy.  But maybe you'll
need to explain your use case(s) a bit more so that we can figure out
what a good high level API is.

> Exposing the arch_sync_* API and using that directly (bypassing
> drm_cflush_*) actually seems pretty reasonable and pragmatic.  I did
> have one doubt, as phys_to_virt() is only valid for kernel direct
> mapped memory (AFAIU), what happens for pages that are not in kernel
> linear map?  Maybe it is ok to ignore those pages, since they won't
> have an aliased mapping?

They could have an aliased mapping in vmalloc/vmap space for example,
if you created one.  We have the flush_kernel_vmap_range /
invalidate_kernel_vmap_range APIs for those, that are implement
on architectures with VIVT caches.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 02/11] kselftest: arm64: adds first test and common utils
From: Cristian Marussi @ 2019-08-06 15:50 UTC (permalink / raw)
  To: linux-kselftest, linux-arm-kernel; +Cc: dave.martin
In-Reply-To: <20190802170300.20662-3-cristian.marussi@arm.com>

Hi

On 02/08/2019 18:02, Cristian Marussi wrote:
> Added some arm64/signal specific boilerplate and utility code to help
> further testcase development.
> 
> A simple testcase and related helpers are also introduced in this commit:
> mangle_pstate_invalid_compat_toggle is a simple mangle testcase which
> messes with the ucontext_t from within the sig_handler, trying to toggle
> PSTATE state bits to switch the system between 32bit/64bit execution state.
> Expects SIGSEGV on test PASS.
> 
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> A few fixes:
> - test_arm64_signals.sh runner script generation has been reviewed in order to
>   be safe against the .gitignore
> - using kselftest.h officially provided defines for tests' return values
> - removed SAFE_WRITE()/dump_uc()
> - looking for si_code==SEGV_ACCERR on SEGV test cases to better understand if
>   the sigfault had been directly triggered by Kernel
> ---
>  tools/testing/selftests/arm64/Makefile        |   2 +-
>  .../testing/selftests/arm64/signal/.gitignore |   6 +
>  tools/testing/selftests/arm64/signal/Makefile |  88 ++++++
>  tools/testing/selftests/arm64/signal/README   |  59 ++++
>  .../arm64/signal/test_arm64_signals.src_shell |  55 ++++
>  .../selftests/arm64/signal/test_signals.c     |  26 ++
>  .../selftests/arm64/signal/test_signals.h     | 137 +++++++++
>  .../arm64/signal/test_signals_utils.c         | 261 ++++++++++++++++++
>  .../arm64/signal/test_signals_utils.h         |  13 +
>  .../arm64/signal/testcases/.gitignore         |   1 +
>  .../mangle_pstate_invalid_compat_toggle.c     |  25 ++
>  .../arm64/signal/testcases/testcases.c        | 150 ++++++++++
>  .../arm64/signal/testcases/testcases.h        |  83 ++++++
>  13 files changed, 905 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/arm64/signal/.gitignore
>  create mode 100644 tools/testing/selftests/arm64/signal/Makefile
>  create mode 100644 tools/testing/selftests/arm64/signal/README
>  create mode 100755 tools/testing/selftests/arm64/signal/test_arm64_signals.src_shell
>  create mode 100644 tools/testing/selftests/arm64/signal/test_signals.c
>  create mode 100644 tools/testing/selftests/arm64/signal/test_signals.h
>  create mode 100644 tools/testing/selftests/arm64/signal/test_signals_utils.c
>  create mode 100644 tools/testing/selftests/arm64/signal/test_signals_utils.h
>  create mode 100644 tools/testing/selftests/arm64/signal/testcases/.gitignore
>  create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_compat_toggle.c
>  create mode 100644 tools/testing/selftests/arm64/signal/testcases/testcases.c
>  create mode 100644 tools/testing/selftests/arm64/signal/testcases/testcases.h
> 

One known issue with this patch, which I've just discovered once I upgraded my local toolchain,
is a missing include and a feature_test_macros needed by testcases.h (related to to siginfo_t)
like in:

diff --git a/tools/testing/selftests/arm64/signal/testcases/testcases.h b/tools/testing/selftests/arm64/signal/testcases/testcases.h
index 624717c71b1d..80831ffc0bbd 100644
--- a/tools/testing/selftests/arm64/signal/testcases/testcases.h
+++ b/tools/testing/selftests/arm64/signal/testcases/testcases.h
@@ -7,6 +7,7 @@
 #include <unistd.h>
 #include <ucontext.h>
 #include <assert.h>
+#include <signal.h>
 
 /* Architecture specific sigframe definitions */
 #include <asm/sigcontext.h>


diff --git a/tools/testing/selftests/arm64/signal/Makefile b/tools/testing/selftests/arm64/signal/Makefile
index b3dcf315b5a4..a32d5656bb20 100644
--- a/tools/testing/selftests/arm64/signal/Makefile
+++ b/tools/testing/selftests/arm64/signal/Makefile
@@ -16,7 +16,7 @@
 # and standalone builds
 top_srcdir = ../../../../..
 
-CFLAGS += -std=gnu99 -I. -I$(top_srcdir)/tools/testing/selftests/
+CFLAGS += -D_GNU_SOURCE -std=gnu99 -I. -I$(top_srcdir)/tools/testing/selftests/
 SRCS := $(filter-out testcases/testcases.c,$(wildcard testcases/*.c))
 PROGS := $(patsubst %.c,%,$(SRCS))

Cheers

Cristian


> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
> index 03a0d4f71218..af59dc74e0dc 100644
> --- a/tools/testing/selftests/arm64/Makefile
> +++ b/tools/testing/selftests/arm64/Makefile
> @@ -6,7 +6,7 @@ ARCH ?= $(shell uname -m)
>  ARCH := $(shell echo $(ARCH) | sed -e s/aarch64/arm64/)
>  
>  ifeq ("x$(ARCH)", "xarm64")
> -SUBDIRS :=
> +SUBDIRS := signal
>  else
>  SUBDIRS :=
>  endif
> diff --git a/tools/testing/selftests/arm64/signal/.gitignore b/tools/testing/selftests/arm64/signal/.gitignore
> new file mode 100644
> index 000000000000..434f65c15f03
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/.gitignore
> @@ -0,0 +1,6 @@
> +# Helper script's internal testcases list (TPROGS) is regenerated
> +# each time by Makefile on standalone (non KSFT driven) runs.
> +# Committing such list creates a dependency between testcases
> +# patches such that they are no more easily revertable. Just ignore.
> +test_arm64_signals.src_shell
> +test_arm64_signals.sh
> diff --git a/tools/testing/selftests/arm64/signal/Makefile b/tools/testing/selftests/arm64/signal/Makefile
> new file mode 100644
> index 000000000000..8c8d08be4b0d
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/Makefile
> @@ -0,0 +1,88 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2019 ARM Limited
> +
> +# Supports also standalone invokation out of KSFT-tree
> +# Compile standalone and run on your device with:
> +#
> +#  $ make -C tools/testing/selftests/arm64/signal INSTALL_PATH=<your-dir> install
> +#
> +# Run standalone on device with:
> +#
> +#  $ <your-device-instdir>/test_arm64_signals.sh [-k|-v]
> +#
> +# If INSTALL_PATH= is NOT provided it will default to ./install
> +
> +# A proper top_srcdir is needed both by KSFT(lib.mk)
> +# and standalone builds
> +top_srcdir = ../../../../..
> +
> +CFLAGS += -std=gnu99 -I. -I$(top_srcdir)/tools/testing/selftests/
> +SRCS := $(filter-out testcases/testcases.c,$(wildcard testcases/*.c))
> +PROGS := $(patsubst %.c,%,$(SRCS))
> +
> +# Guessing as best as we can where the Kernel headers
> +# could have been installed depending on ENV config and
> +# type of invocation.
> +ifeq ($(KBUILD_OUTPUT),)
> +khdr_dir = $(top_srcdir)/usr/include
> +else
> +ifeq (0,$(MAKELEVEL))
> +khdr_dir = $(KBUILD_OUTPUT)/usr/include
> +else
> +# the KSFT preferred location when KBUILD_OUTPUT is set
> +khdr_dir = $(KBUILD_OUTPUT)/kselftest/usr/include
> +endif
> +endif
> +
> +CFLAGS += -I$(khdr_dir)
> +
> +# Standalone run
> +ifeq (0,$(MAKELEVEL))
> +CC := $(CROSS_COMPILE)gcc
> +RUNNER_SRC = test_arm64_signals.src_shell
> +RUNNER = test_arm64_signals.sh
> +INSTALL_PATH ?= install/
> +
> +all: $(RUNNER)
> +
> +$(RUNNER): $(PROGS)
> +	cp $(RUNNER_SRC) $(RUNNER)
> +	sed -i -e 's#PROGS=.*#PROGS="$(PROGS)"#' $@
> +
> +install: all
> +	mkdir -p $(INSTALL_PATH)/testcases
> +	cp $(PROGS) $(INSTALL_PATH)/testcases
> +	cp $(RUNNER) $(INSTALL_PATH)/
> +
> +.PHONY clean:
> +	rm -f $(PROGS)
> +# KSFT run
> +else
> +# Generated binaries to be installed by top KSFT script
> +TEST_GEN_PROGS := $(notdir $(PROGS))
> +
> +# Get Kernel headers installed and use them.
> +KSFT_KHDR_INSTALL := 1
> +
> +# This include mk will also mangle the TEST_GEN_PROGS list
> +# to account for any OUTPUT target-dirs optionally provided
> +# by the toplevel makefile
> +include ../../lib.mk
> +
> +$(TEST_GEN_PROGS): $(PROGS)
> +	cp $(PROGS) $(OUTPUT)/
> +
> +clean:
> +	$(CLEAN)
> +	rm -f $(PROGS)
> +endif
> +
> +# Common test-unit targets to build common-layout test-cases executables
> +# Needs secondary expansion to properly include the testcase c-file in pre-reqs
> +.SECONDEXPANSION:
> +$(PROGS): test_signals.c test_signals_utils.c testcases/testcases.c $$@.c test_signals.h test_signals_utils.h testcases/testcases.h
> +	@if [ ! -d $(khdr_dir) ]; then \
> +		echo -n "\n!!! WARNING: $(khdr_dir) NOT FOUND."; \
> +		echo "===>  Are you sure Kernel Headers have been installed properly ?\n"; \
> +	fi
> +	$(CC) $(CFLAGS) $^ -o $@
> diff --git a/tools/testing/selftests/arm64/signal/README b/tools/testing/selftests/arm64/signal/README
> new file mode 100644
> index 000000000000..53f005f7910a
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/README
> @@ -0,0 +1,59 @@
> +KSelfTest arm64/signal/
> +=======================
> +
> +Signals Tests
> ++++++++++++++
> +
> +- Tests are built around a common main compilation unit: such shared main
> +  enforces a standard sequence of operations needed to perform a single
> +  signal-test (setup/trigger/run/result/cleanup)
> +
> +- The above mentioned ops are configurable on a test-by-test basis: each test
> +  is described (and configured) using the descriptor signals.h::struct tdescr
> +
> +- Each signal testcase is compiled into its own executable: a separate
> +  executable is used for each test since many tests complete successfully
> +  by receiving some kind of fatal signal from the Kernel, so it's safer
> +  to run each test unit in its own standalone process, so as to start each
> +  test from a clean slate.
> +
> +- New tests can be simply defined in testcases/ dir providing a proper struct
> +  tdescr overriding all the defaults we wish to change (as of now providing a
> +  custom run method is mandatory though)
> +
> +- Signals' test-cases hereafter defined belong currently to two
> +  principal families:
> +
> +  - 'mangle_' tests: a real signal (SIGUSR1) is raised and used as a trigger
> +    and then the test case code messes-up with the sigframe ucontext_t from
> +    inside the sighandler itself.
> +
> +  - 'fake_sigreturn_' tests: a brand new custom artificial sigframe structure
> +    is placed on the stack and a sigreturn syscall is called to simulate a
> +    real signal return. This kind of tests does not use a trigger usually and
> +    they are just fired using some simple included assembly trampoline code.
> +
> + - Most of these tests are successfully passing if the process gets killed by
> +   some fatal signal: usually SIGSEGV or SIGBUS. Since while writing this
> +   kind of tests it is extremely easy in fact to end-up injecting other
> +   unrelated SEGV bugs in the testcases, it becomes extremely tricky to
> +   be really sure that the tests are really addressing what they are meant
> +   to address and they are not instead falling apart due to unplanned bugs
> +   in the test code.
> +   In order to alleviate the misery of the life of such test-developer, a few
> +   helpers are provided:
> +
> +   - a couple of ASSERT_BAD/GOOD_CONTEXT() macros to easily parse a ucontext_t
> +     and verify if it is indeed GOOD or BAD (depending on what we were
> +     expecting), using the same logic/perspective as in the arm64 Kernel signals
> +     routines.
> +
> +   - a sanity mechanism to be used in 'fake_sigreturn_'-alike tests: enabled by
> +     default it takes care to verify that the test-execution had at least
> +     successfully progressed up to the stage of triggering the fake sigreturn
> +     call.
> +
> +  In both cases test results are expected in terms of:
> +   - some fatal signal sent by the Kernel to the test process
> +  or
> +  - analyzing some final regs state
> diff --git a/tools/testing/selftests/arm64/signal/test_arm64_signals.src_shell b/tools/testing/selftests/arm64/signal/test_arm64_signals.src_shell
> new file mode 100755
> index 000000000000..163e941e2997
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/test_arm64_signals.src_shell
> @@ -0,0 +1,55 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2019 ARM Limited
> +
> +ret=0
> +keep_on_fail=0
> +err_out="2> /dev/null"
> +
> +usage() {
> +	echo "Usage: `basename $0` [-v] [-k]"
> +	exit 1
> +}
> +
> +# avoiding getopt to avoid compatibility issues on targets
> +# with limited resources
> +while [ $# -gt 0 ]
> +do
> +	case $1 in
> +		"-k")
> +			keep_on_fail=1
> +			;;
> +		"-v")
> +			err_out=
> +			;;
> +		*)
> +			usage
> +			;;
> +	esac
> +	shift
> +done
> +
> +TPROGS=
> +
> +tot=$(echo $TPROGS | wc -w)
> +
> +# Tests are expected in testcases/ subdir inside the installation path
> +workdir="`dirname $0 2>/dev/null`"
> +[ -n $workdir ] && cd $workdir
> +
> +passed=0
> +run=0
> +for test in $TPROGS
> +do
> +	run=$((run + 1))
> +	eval ./$test $err_out
> +	if [ $? != 0 ]; then
> +		[ $keep_on_fail = 0 ] && echo "===>>> FAILED:: $test <<<===" && ret=1 && break
> +	else
> +		passed=$((passed + 1))
> +	fi
> +done
> +
> +echo "==>> PASSED: $passed/$run on $tot available tests."
> +
> +exit $ret
> diff --git a/tools/testing/selftests/arm64/signal/test_signals.c b/tools/testing/selftests/arm64/signal/test_signals.c
> new file mode 100644
> index 000000000000..3447d7011aec
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/test_signals.c
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
> +#include <kselftest.h>
> +
> +#include "test_signals.h"
> +#include "test_signals_utils.h"
> +
> +struct tdescr *current;
> +extern struct tdescr tde;
> +
> +int main(int argc, char *argv[])
> +{
> +	current = &tde;
> +
> +	ksft_print_msg("%s :: %s - SIG_TRIG:%d  SIG_OK:%d -- current:%p\n",
> +		       current->name, current->descr, current->sig_trig,
> +		       current->sig_ok, current);
> +	if (test_setup(current)) {
> +		if (test_run(current))
> +			test_result(current);
> +		test_cleanup(current);
> +	}
> +
> +	return current->pass ? KSFT_PASS : KSFT_FAIL;
> +}
> diff --git a/tools/testing/selftests/arm64/signal/test_signals.h b/tools/testing/selftests/arm64/signal/test_signals.h
> new file mode 100644
> index 000000000000..85db3ac44b32
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/test_signals.h
> @@ -0,0 +1,137 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
> +#ifndef __TEST_SIGNALS_H__
> +#define __TEST_SIGNALS_H__
> +
> +#include <assert.h>
> +#include <stdbool.h>
> +#include <signal.h>
> +#include <ucontext.h>
> +#include <stdint.h>
> +
> +/*
> + * Using ARCH specific and sanitized Kernel headers installed by KSFT
> + * framework since we asked for it by setting flag KSFT_KHDR_INSTALL
> + * in our Makefile.
> + */
> +#include <asm/ptrace.h>
> +#include <asm/hwcap.h>
> +
> +/* pasted from include/linux/stringify.h */
> +#define __stringify_1(x...)	#x
> +#define __stringify(x...)	__stringify_1(x)
> +
> +/*
> + * Reads a sysreg using the, possibly provided, S3_ encoding in order to
> + * avoid inject any dependency on the used toolchain regarding possibly
> + * still unsupported ARMv8 extensions.
> + *
> + * Using a standard mnemonic here to indicate the specific sysreg (like SSBS)
> + * would introduce a compile-time dependency on possibly unsupported ARMv8
> + * Extensions: you could end-up failing to build the test depending on the
> + * available toolchain.
> + * This is undesirable since some tests, even if specifically targeted at some
> + * ARMv8 Extensions, can be plausibly run even on hardware lacking the above
> + * optional ARM features. (SSBS bit preservation is an example: Kernel handles
> + * it transparently not caring at all about the effective set of supported
> + * features).
> + * On the other side we will expect to observe different behaviours if the
> + * feature is supported or not: usually getting a SIGILL when trying to use
> + * unsupported features. For this reason we have anyway in place some
> + * preliminary run-time checks about the cpu effectively supported features.
> + *
> + * This helper macro is meant to be used for regs readable at EL0, BUT some
> + * EL1 sysregs are indeed readable too through MRS emulation Kernel-mechanism
> + * if the required reg is included in the supported encoding space:
> + *
> + *  Documentation/arm64/cpu-feature-regsiters.txt
> + *
> + *  "The infrastructure emulates only the following system register space:
> + *   	Op0=3, Op1=0, CRn=0, CRm=0,4,5,6,7
> + */
> +#define get_regval(regname, out) \
> +	asm volatile("mrs %0, " __stringify(regname) : "=r" (out) :: "memory")
> +
> +/* Regs encoding and masks naming copied in from sysreg.h */
> +#define SYS_ID_AA64MMFR1_EL1	S3_0_C0_C7_1	/* MRS Emulated */
> +#define SYS_ID_AA64MMFR2_EL1	S3_0_C0_C7_2	/* MRS Emulated */
> +#define ID_AA64MMFR1_PAN_SHIFT	20
> +#define ID_AA64MMFR2_UAO_SHIFT	4
> +
> +/* Local Helpers */
> +#define IS_PAN_SUPPORTED(val) \
> +	(!!((val) & (0xfUL << ID_AA64MMFR1_PAN_SHIFT)))
> +#define IS_UAO_SUPPORTED(val) \
> +	(!!((val) & (0xfUL << ID_AA64MMFR2_UAO_SHIFT)))
> +
> +#define S3_MRS_SSBS_SYSREG		S3_3_C4_C2_6	/* EL0 supported */
> +
> +/*
> + * Feature flags used in tdescr.feats_required to specify
> + * any feature by the test
> + */
> +enum {
> +	FSSBS_BIT,
> +	FPAN_BIT,
> +	FUAO_BIT,
> +	FMAX_END
> +};
> +
> +#define FEAT_SSBS		(1UL << FSSBS_BIT)
> +#define FEAT_PAN		(1UL << FPAN_BIT)
> +#define FEAT_UAO		(1UL << FUAO_BIT)
> +
> +/*
> + * A descriptor used to describe and configure a test case.
> + * Fields with a non-trivial meaning are described inline in the following.
> + */
> +struct tdescr {
> +	/* KEEP THIS FIELD FIRST for easier lookup from assembly */
> +	void		*token;
> +	/* when disabled token based sanity checking is skipped in handler */
> +	bool		sanity_disabled;
> +	/* just a name for the test-case; manadatory field */
> +	char		*name;
> +	char		*descr;
> +	unsigned long	feats_required;
> +	/* bitmask of effectively supported feats: populated at run-time */
> +	unsigned long	feats_supported;
> +	bool		feats_ok;
> +	bool		initialized;
> +	unsigned int	minsigstksz;
> +	/* signum used as a test trigger. Zero if no trigger-signal is used */
> +	int		sig_trig;
> +	/*
> +	 * signum considered as a successful test completion.
> +	 * Zero when no signal is expected on success
> +	 */
> +	int		sig_ok;
> +	/* signum expected on unsupported CPU features. */
> +	int		sig_unsupp;
> +	/* a timeout in second for test completion */
> +	unsigned int	timeout;
> +	bool		triggered;
> +	bool		pass;
> +	/* optional sa_flags for the installed handler */
> +	int		sa_flags;
> +	ucontext_t	saved_uc;
> +
> +	/* a setup function to be called before test starts */
> +	int (*setup)(struct tdescr *td);
> +	void (*cleanup)(struct tdescr *td);
> +
> +	/* an optional function to be used as a trigger for test starting */
> +	int (*trigger)(struct tdescr *td);
> +	/*
> +	 * the actual test-core: invoked differently depending on the
> +	 * presence of the trigger function above; this is mandatory
> +	 */
> +	int (*run)(struct tdescr *td, siginfo_t *si, ucontext_t *uc);
> +
> +	/* an optional function for custom results' processing */
> +	void (*check_result)(struct tdescr *td);
> +
> +	void *priv;
> +};
> +#endif
> diff --git a/tools/testing/selftests/arm64/signal/test_signals_utils.c b/tools/testing/selftests/arm64/signal/test_signals_utils.c
> new file mode 100644
> index 000000000000..ac0055f6340b
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/test_signals_utils.c
> @@ -0,0 +1,261 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <signal.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <assert.h>
> +#include <sys/auxv.h>
> +#include <linux/auxvec.h>
> +#include <ucontext.h>
> +
> +#include "test_signals.h"
> +#include "test_signals_utils.h"
> +#include "testcases/testcases.h"
> +
> +extern struct tdescr *current;
> +
> +static char *feats_store[FMAX_END] = {
> +	"SSBS",
> +	"PAN",
> +	"UAO"
> +};
> +
> +#define MAX_FEATS_SZ	128
> +static inline char *feats_to_string(unsigned long feats)
> +{
> +	static char feats_string[MAX_FEATS_SZ];
> +
> +	for (int i = 0; i < FMAX_END && feats_store[i][0]; i++) {
> +		if (feats & 1UL << i)
> +			snprintf(feats_string, MAX_FEATS_SZ - 1, "%s %s ",
> +				 feats_string, feats_store[i]);
> +	}
> +
> +	return feats_string;
> +}
> +
> +static void unblock_signal(int signum)
> +{
> +	sigset_t sset;
> +
> +	sigemptyset(&sset);
> +	sigaddset(&sset, signum);
> +	sigprocmask(SIG_UNBLOCK, &sset, NULL);
> +}
> +
> +static void default_result(struct tdescr *td, bool force_exit)
> +{
> +	if (td->pass)
> +		fprintf(stderr, "==>> completed. PASS(1)\n");
> +	else
> +		fprintf(stdout, "==>> completed. FAIL(0)\n");
> +	if (force_exit)
> +		exit(td->pass ? EXIT_SUCCESS : EXIT_FAILURE);
> +}
> +
> +static inline bool are_feats_ok(struct tdescr *td)
> +{
> +	return td ? td->feats_required == td->feats_supported : 0;
> +}
> +
> +static void default_handler(int signum, siginfo_t *si, void *uc)
> +{
> +	if (current->sig_trig && signum == current->sig_trig) {
> +		fprintf(stderr, "Handling SIG_TRIG\n");
> +		current->triggered = 1;
> +		/* ->run was asserted NON-NULL in test_setup() already */
> +		current->run(current, si, uc);
> +	} else if (signum == SIGILL && !current->initialized) {
> +		/*
> +		 * A SIGILL here while still not initialized means we failed
> +		 * even to asses the existence of features during init
> +		 */
> +		fprintf(stdout,
> +			"Got SIGILL test_init. Marking ALL features UNSUPPORTED.\n");
> +		current->feats_supported = 0;
> +	} else if (current->sig_ok && signum == current->sig_ok) {
> +		/* it's a bug in the test code when this assert fail */
> +		assert(!current->sig_trig || current->triggered);
> +		fprintf(stderr,
> +			"SIG_OK -- SP:%p  si_addr@:0x%p  si_code:%d  token@:0x%p  offset:%ld\n",
> +			((ucontext_t *)uc)->uc_mcontext.sp,
> +			si->si_addr, si->si_code, current->token,
> +			current->token - si->si_addr);
> +		/*
> +		 * fake_sigreturn tests, which have sanity_enabled=1, set, at
> +		 * the very last time, the token field to the SP address used
> +		 * to place the fake sigframe: so token==0 means we never made
> +		 * it to the end, segfaulting well-before, and the test is
> +		 * possibly broken.
> +		 */
> +		if (!current->sanity_disabled && !current->token) {
> +			fprintf(stdout,
> +				"current->token ZEROED...test is probably broken!\n");
> +			assert(0);
> +		}
> +		/*
> +		 * Trying to narrow down the SEGV to the ones generated by
> +		 * Kernel itself via arm64_notify_segfault()
> +		 */
> +		if (current->sig_ok == SIGSEGV && si->si_code != SEGV_ACCERR) {
> +			fprintf(stdout,
> +				"si_code != SEGV_ACCERR...test is probably broken!\n");
> +			assert(0);
> +		}
> +		fprintf(stderr, "Handling SIG_OK\n");
> +		current->pass = 1;
> +		/*
> +		 * Some tests can lead to SEGV loops: in such a case we want
> +		 * to terminate immediately exiting straight away
> +		 */
> +		default_result(current, 1);
> +	} else {
> +		if (signum == current->sig_unsupp && !are_feats_ok(current)) {
> +			fprintf(stderr, "-- RX SIG_UNSUPP on unsupported feature...OK\n");
> +			current->pass = 1;
> +		} else if (signum == SIGALRM && current->timeout) {
> +			fprintf(stderr, "-- Timeout !\n");
> +		} else {
> +			fprintf(stderr,
> +				"-- RX UNEXPECTED SIGNAL: %d\n", signum);
> +		}
> +		default_result(current, 1);
> +	}
> +}
> +
> +static int default_setup(struct tdescr *td)
> +{
> +	struct sigaction sa;
> +
> +	sa.sa_sigaction = default_handler;
> +	sa.sa_flags = SA_SIGINFO;
> +	if (td->sa_flags)
> +		sa.sa_flags |= td->sa_flags;
> +	sigemptyset(&sa.sa_mask);
> +	/* uncatchable signals naturally skipped ... */
> +	for (int sig = 1; sig < 32; sig++)
> +		sigaction(sig, &sa, NULL);
> +	/*
> +	 * RT Signals default disposition is Term but they cannot be
> +	 * generated by the Kernel in response to our tests; so just catch
> +	 * them all and report them as UNEXPECTED signals.
> +	 */
> +	for (int sig = SIGRTMIN; sig <= SIGRTMAX; sig++)
> +		sigaction(sig, &sa, NULL);
> +
> +	/* just in case...unblock explicitly all we need */
> +	if (td->sig_trig)
> +		unblock_signal(td->sig_trig);
> +	if (td->sig_ok)
> +		unblock_signal(td->sig_ok);
> +	if (td->sig_unsupp)
> +		unblock_signal(td->sig_unsupp);
> +
> +	if (td->timeout) {
> +		unblock_signal(SIGALRM);
> +		alarm(td->timeout);
> +	}
> +	fprintf(stderr, "Registered handlers for all signals.\n");
> +
> +	return 1;
> +}
> +
> +static inline int default_trigger(struct tdescr *td)
> +{
> +	return !raise(td->sig_trig);
> +}
> +
> +static int test_init(struct tdescr *td)
> +{
> +	td->minsigstksz = getauxval(AT_MINSIGSTKSZ);
> +	if (!td->minsigstksz)
> +		td->minsigstksz = MINSIGSTKSZ;
> +	fprintf(stderr, "Detected MINSTKSIGSZ:%d\n", td->minsigstksz);
> +
> +	if (td->feats_required) {
> +		bool feats_ok = false;
> +		td->feats_supported = 0;
> +		/*
> +		 * Checking for CPU required features using both the
> +		 * auxval and the arm64 MRS Emulation to read sysregs.
> +		 */
> +		if (getauxval(AT_HWCAP) & HWCAP_CPUID) {
> +			uint64_t val = 0;
> +
> +			if (td->feats_required & FEAT_SSBS) {
> +				/* Uses HWCAP to check capability */
> +				if (getauxval(AT_HWCAP) & HWCAP_SSBS)
> +					td->feats_supported |= FEAT_SSBS;
> +			}
> +			if (td->feats_required & FEAT_PAN) {
> +				/* Uses MRS emulation to check capability */
> +				get_regval(SYS_ID_AA64MMFR1_EL1, val);
> +				if (IS_PAN_SUPPORTED(val))
> +					td->feats_supported |= FEAT_PAN;
> +			}
> +			if (td->feats_required & FEAT_UAO) {
> +				/* Uses MRS emulation to check capability */
> +				get_regval(SYS_ID_AA64MMFR2_EL1 , val);
> +				if (IS_UAO_SUPPORTED(val))
> +					td->feats_supported |= FEAT_UAO;
> +			}
> +		} else {
> +			fprintf(stderr,
> +				"HWCAP_CPUID NOT available. Mark ALL feats UNSUPPORTED.\n");
> +		}
> +		feats_ok = are_feats_ok(td);
> +		fprintf(stderr,
> +			"Required Features: [%s] %ssupported\n",
> +			feats_ok ? feats_to_string(td->feats_supported) :
> +		        feats_to_string(td->feats_required ^ td->feats_supported),
> +			!feats_ok ? "NOT " : "");
> +	}
> +
> +	td->initialized = 1;
> +	return 1;
> +}
> +
> +int test_setup(struct tdescr *td)
> +{
> +	/* assert core invariants symptom of a rotten testcase */
> +	assert(current);
> +	assert(td);
> +	assert(td->name);
> +	assert(td->run);
> +
> +	if (!test_init(td))
> +		return 0;
> +
> +	if (td->setup)
> +		return td->setup(td);
> +	else
> +		return default_setup(td);
> +}
> +
> +int test_run(struct tdescr *td)
> +{
> +	if (td->sig_trig) {
> +		if (td->trigger)
> +			return td->trigger(td);
> +		else
> +			return default_trigger(td);
> +	} else {
> +		return td->run(td, NULL, NULL);
> +	}
> +}
> +
> +void test_result(struct tdescr *td)
> +{
> +	if (td->check_result)
> +		td->check_result(td);
> +	default_result(td, 0);
> +}
> +
> +void test_cleanup(struct tdescr *td)
> +{
> +	if (td->cleanup)
> +		td->cleanup(td);
> +}
> diff --git a/tools/testing/selftests/arm64/signal/test_signals_utils.h b/tools/testing/selftests/arm64/signal/test_signals_utils.h
> new file mode 100644
> index 000000000000..8658d1a7d4b9
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/test_signals_utils.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
> +#ifndef __TEST_SIGNALS_UTILS_H__
> +#define __TEST_SIGNALS_UTILS_H__
> +
> +#include "test_signals.h"
> +
> +int test_setup(struct tdescr *td);
> +void test_cleanup(struct tdescr *td);
> +int test_run(struct tdescr *td);
> +void test_result(struct tdescr *td);
> +#endif
> diff --git a/tools/testing/selftests/arm64/signal/testcases/.gitignore b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> new file mode 100644
> index 000000000000..8651272e3cfc
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/.gitignore
> @@ -0,0 +1 @@
> +mangle_pstate_invalid_compat_toggle
> diff --git a/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_compat_toggle.c b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_compat_toggle.c
> new file mode 100644
> index 000000000000..971193e7501b
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_compat_toggle.c
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +static int mangle_invalid_pstate_run(struct tdescr *td, siginfo_t *si,
> +				     ucontext_t *uc)
> +{
> +	ASSERT_GOOD_CONTEXT(uc);
> +
> +	/* This config should trigger a SIGSEGV by Kernel */
> +	uc->uc_mcontext.pstate ^= PSR_MODE32_BIT;
> +
> +	return 1;
> +}
> +
> +struct tdescr tde = {
> +		.sanity_disabled = true,
> +		.name = "MANGLE_PSTATE_INVALID_STATE_TOGGLE",
> +		.descr = "Mangling uc_mcontext with INVALID STATE_TOGGLE",
> +		.sig_trig = SIGUSR1,
> +		.sig_ok = SIGSEGV,
> +		.run = mangle_invalid_pstate_run,
> +};
> diff --git a/tools/testing/selftests/arm64/signal/testcases/testcases.c b/tools/testing/selftests/arm64/signal/testcases/testcases.c
> new file mode 100644
> index 000000000000..a59785092e1f
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/testcases.c
> @@ -0,0 +1,150 @@
> +#include "testcases.h"
> +
> +struct _aarch64_ctx *get_header(struct _aarch64_ctx *head, uint32_t magic,
> +				size_t resv_sz, size_t *offset)
> +{
> +	size_t offs = 0;
> +	struct _aarch64_ctx *found = NULL;
> +
> +	if (!head || resv_sz < HDR_SZ)
> +		return found;
> +
> +	do {
> +		if (head->magic == magic) {
> +			found = head;
> +			break;
> +		}
> +		offs += head->size;
> +		head = GET_RESV_NEXT_HEAD(head);
> +	} while (offs < resv_sz - HDR_SZ);
> +
> +	if (offset)
> +		*offset = offs;
> +
> +	return found;
> +}
> +
> +bool validate_extra_context(struct extra_context *extra, char **err)
> +{
> +	struct _aarch64_ctx *term;
> +
> +	if (!extra || !err)
> +		return false;
> +
> +	fprintf(stderr, "Validating EXTRA...\n");
> +	term = GET_RESV_NEXT_HEAD(extra);
> +	if (!term || term->magic || term->size) {
> +		*err = "UN-Terminated EXTRA context";
> +		return false;
> +	}
> +	if (extra->datap & 0x0fUL)
> +		*err = "Extra DATAP misaligned";
> +	else if (extra->size & 0x0fUL)
> +		*err = "Extra SIZE misaligned";
> +	else if (extra->datap != (uint64_t)term + sizeof(*term))
> +		*err = "Extra DATAP misplaced (not contiguos)";
> +	if (*err)
> +		return false;
> +
> +	return true;
> +}
> +
> +bool validate_reserved(ucontext_t *uc, size_t resv_sz, char **err)
> +{
> +	bool terminated = false;
> +	size_t offs = 0;
> +	int flags = 0;
> +	struct extra_context *extra = NULL;
> +	struct _aarch64_ctx *head =
> +		(struct _aarch64_ctx *)uc->uc_mcontext.__reserved;
> +
> +	if (!err)
> +		return false;
> +	/* Walk till the end terminator verifying __reserved contents */
> +	while (head && !terminated && offs < resv_sz) {
> +		if ((uint64_t)head & 0x0fUL) {
> +			*err = "Misaligned HEAD";
> +			return false;
> +		}
> +
> +		switch (head->magic) {
> +			case 0:
> +				if (head->size)
> +					*err = "Bad size for MAGIC0";
> +				else
> +					terminated = true;
> +				break;
> +			case FPSIMD_MAGIC:
> +				if (flags & FPSIMD_CTX)
> +					*err = "Multiple FPSIMD_MAGIC";
> +				else if (head->size !=
> +					 sizeof(struct fpsimd_context))
> +					*err = "Bad size for fpsimd_context";
> +				flags |= FPSIMD_CTX;
> +				break;
> +			case ESR_MAGIC:
> +				if (head->size != sizeof(struct esr_context))
> +					fprintf(stderr,
> +						"Bad size for esr_context is not an error...just ignore.\n");
> +				break;
> +			case SVE_MAGIC:
> +				if (flags & SVE_CTX)
> +					*err = "Multiple SVE_MAGIC";
> +				else if (head->size !=
> +					 sizeof(struct sve_context))
> +					*err = "Bad size for sve_context";
> +				flags |= SVE_CTX;
> +				break;
> +			case EXTRA_MAGIC:
> +				if (flags & EXTRA_CTX)
> +					*err = "Multiple EXTRA_MAGIC";
> +				else if (head->size !=
> +					 sizeof(struct extra_context))
> +					*err = "Bad size for extra_context";
> +				flags |= EXTRA_CTX;
> +				extra = (struct extra_context *)head;
> +				break;
> +			case KSFT_BAD_MAGIC:
> +				/*
> +				 * This is a BAD magic header defined
> +				 * artificially by a testcase and surely
> +				 * unknown to the Kernel parse_user_sigframe().
> +				 * It MUST cause a Kernel induced SEGV
> +				 */
> +				*err = "BAD MAGIC !";
> +				break;
> +			default:
> +				/*
> +				 * A still unknown Magic: potentially freshly added
> +				 * to the Kernel code and still unknown to the
> +				 * tests.
> +				 */
> +				fprintf(stdout,
> +					"SKIP Unknown MAGIC: 0x%X - Is KSFT arm64/signal up to date ?\n",
> +					head->magic);
> +				break;
> +		}
> +
> +		if (*err)
> +			return false;
> +
> +		offs += head->size;
> +		if (resv_sz - offs < sizeof(*head)) {
> +			*err = "HEAD Overrun";
> +			return false;
> +		}
> +
> +		if (flags & EXTRA_CTX)
> +			if (!validate_extra_context(extra, err))
> +				return false;
> +
> +		head = GET_RESV_NEXT_HEAD(head);
> +	}
> +
> +	if (terminated && !(flags & FPSIMD_CTX)) {
> +		*err = "Missing FPSIMD";
> +		return false;
> +	}
> +
> +	return true;
> +}
> diff --git a/tools/testing/selftests/arm64/signal/testcases/testcases.h b/tools/testing/selftests/arm64/signal/testcases/testcases.h
> new file mode 100644
> index 000000000000..624717c71b1d
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/testcases.h
> @@ -0,0 +1,83 @@
> +#ifndef __TESTCASES_H__
> +#define __TESTCASES_H__
> +
> +#include <stdio.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <unistd.h>
> +#include <ucontext.h>
> +#include <assert.h>
> +
> +/* Architecture specific sigframe definitions */
> +#include <asm/sigcontext.h>
> +
> +#define FPSIMD_CTX	(1 << 0)
> +#define SVE_CTX		(1 << 1)
> +#define EXTRA_CTX	(1 << 2)
> +
> +#define KSFT_BAD_MAGIC	0xdeadbeef
> +
> +#define HDR_SZ \
> +	sizeof(struct _aarch64_ctx)
> +
> +#define GET_SF_RESV_HEAD(sf) \
> +	(struct _aarch64_ctx *)(&(sf).uc.uc_mcontext.__reserved)
> +
> +#define GET_SF_RESV_SIZE(sf) \
> +	sizeof((sf).uc.uc_mcontext.__reserved)
> +
> +#define GET_UCP_RESV_SIZE(ucp) \
> +	sizeof((ucp)->uc_mcontext.__reserved)
> +
> +#define ASSERT_BAD_CONTEXT(uc) do {					\
> +	char *err = NULL;						\
> +	assert(!validate_reserved((uc), GET_UCP_RESV_SIZE((uc)), &err));\
> +	if (err)							\
> +		fprintf(stderr,						\
> +			"Using badly built context - ERR: %s\n", err);	\
> +} while(0)
> +
> +#define ASSERT_GOOD_CONTEXT(uc) do {					 \
> +	char *err = NULL;						 \
> +	if (!validate_reserved((uc), GET_UCP_RESV_SIZE((uc)), &err)) {	 \
> +		if (err)						 \
> +			fprintf(stderr,					 \
> +				"Detected BAD context - ERR: %s\n", err);\
> +		assert(0);						 \
> +	} else {							 \
> +		fprintf(stderr, "uc context validated.\n");		 \
> +	}								 \
> +} while(0)
> +
> +/* head->size accounts both for payload and header _aarch64_ctx size ! */
> +#define GET_RESV_NEXT_HEAD(h) \
> +	(struct _aarch64_ctx *)((char *)(h) + (h)->size)
> +
> +struct fake_sigframe {
> +	siginfo_t	info;
> +	ucontext_t	uc;
> +};
> +
> +
> +bool validate_reserved(ucontext_t *uc, size_t resv_sz, char **err);
> +
> +bool validate_extra_context(struct extra_context *extra, char **err);
> +
> +struct _aarch64_ctx *get_header(struct _aarch64_ctx *head, uint32_t magic,
> +				size_t resv_sz, size_t *offset);
> +
> +static inline struct _aarch64_ctx *get_terminator(struct _aarch64_ctx *head,
> +						  size_t resv_sz,
> +						  size_t *offset)
> +{
> +	return get_header(head, 0, resv_sz, offset);
> +}
> +
> +static inline void write_terminator_record(struct _aarch64_ctx *tail)
> +{
> +	if (tail) {
> +		tail->magic = 0;
> +		tail->size = 0;
> +	}
> +}
> +#endif
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH 6/6] arm64: dts: meson-g12b-khadas-vim3: add initial device-tree
From: Neil Armstrong @ 2019-08-06 15:48 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: khilman, Christian Hewitt, linux-kernel, linux-arm-kernel,
	linux-amlogic
In-Reply-To: <CAFBinCAD6F=bEE8Z2MvNZLJVKZ3siPqdJ36GuCYkp=DuY3AecQ@mail.gmail.com>

On 03/08/2019 20:50, Martin Blumenstingl wrote:
> On Wed, Jul 31, 2019 at 2:44 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> [...]
>> +       memory@0 {
>> +               device_type = "memory";
>> +               reg = <0x0 0x0 0x0 0x40000000>;
> nit-pick: we typically use the memory size used in the board size with
> the lowest amount of RAM - 2GiB in this case. so I would change it to
> 0x80000000

Right

> 
> [...]
>> +       leds {
>> +               compatible = "gpio-leds";
>> +
>> +               white {
>> +                       label = "vim3:white";
> downstream sources use label="sys_led"
> should we call it vim3:white:sys?
> 

OK, no problem
Thanks for the review.

Neil

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3] tracing: Function stack size and its name mismatch in arm64
From: Joel Fernandes @ 2019-08-06 15:48 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Jiping Ma, catalin.marinas, will.deacon, linux-kernel, mingo,
	linux-arm-kernel
In-Reply-To: <20190802121124.6b41f26a@gandalf.local.home>

On Fri, Aug 02, 2019 at 12:11:24PM -0400, Steven Rostedt wrote:
> On Fri, 2 Aug 2019 12:09:20 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > On Fri, 2 Aug 2019 11:22:59 -0400
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > > I think you are not explaining the issue correctly. From looking at the
> > > document, I think what you want to say is that the LR is saved *after*
> > > the data for the function. Is that correct? If so, then yes, it would
> > > cause the stack tracing algorithm to be incorrect.
> > > 
> > 
> > [..]
> > 
> > > Can someone confirm that this is the real issue?
> > 
> > Does this patch fix your issue?
> >
> 
> Bah, I hit "attach" instead of "insert" (I wondered why it didn't
> insert). Here's the patch without the attachment.
> 
> -- Steve
> 
> diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h
> index 5ab5200b2bdc..13a4832cfb00 100644
> --- a/arch/arm64/include/asm/ftrace.h
> +++ b/arch/arm64/include/asm/ftrace.h
> @@ -13,6 +13,7 @@
>  #define HAVE_FUNCTION_GRAPH_FP_TEST
>  #define MCOUNT_ADDR		((unsigned long)_mcount)
>  #define MCOUNT_INSN_SIZE	AARCH64_INSN_SIZE
> +#define ARCH_RET_ADDR_AFTER_LOCAL_VARS 1
>  
>  #ifndef __ASSEMBLY__
>  #include <linux/compat.h>
> diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
> index 5d16f73898db..050c6bd9beac 100644
> --- a/kernel/trace/trace_stack.c
> +++ b/kernel/trace/trace_stack.c
> @@ -158,6 +158,18 @@ static void check_stack(unsigned long ip, unsigned long *stack)
>  			i++;
>  	}
>  
> +#ifdef ARCH_RET_ADDR_AFTER_LOCAL_VARS
> +	/*
> +	 * Most archs store the return address before storing the
> +	 * function's local variables. But some archs do this backwards.
> +	 */
> +	if (x > 1) {
> +		memmove(&stack_trace_index[0], &stack_trace_index[1],
> +			sizeof(stack_trace_index[0]) * (x - 1));
> +		x--;
> +	}
> +#endif
> +
>  	stack_trace_nr_entries = x;
>  
>  	if (task_stack_end_corrupted(current)) {


I am not fully understanding the fix :(. If the positions of the data and
FP/LR are swapped, then there should be a loop of some sort where the FP/LR
are copied repeatedly to undo the mess we are discussing. But in this patch
I see only one copy happening. May be I just don't understand this code well
enough. Are there any more clues for helping understand the fix?

Also, this stack trace loop (original code) is a bit hairy :) It appears
there is a call to stack_trace_save() followed by another loop that goes
through the returned entries from there and tries to generate a set of
indexes. Isn't the real issue that the entries returned by stack_trace_save()
are a out of whack? I am curious also if other users of stack_trace_save()
will suffer from the same issue.

thanks,

 - Joel


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2] iommu: arm-smmu-v3: Mark expected switch fall-through
From: Joerg Roedel @ 2019-08-06 15:28 UTC (permalink / raw)
  To: Will Deacon
  Cc: robin.murphy, iommu, Anders Roxell, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20190730152600.643mg43y6567pchi@willie-the-truck>

On Tue, Jul 30, 2019 at 04:26:01PM +0100, Will Deacon wrote:
> Joerg -- if you'd like to pick this up as a fix, feel free, otherwise I'll
> include it in my pull request for 5.4.

Applied to iommu/fixes, thanks.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH V4 11/11] docs: arm64: Add layout and 52-bit info to memory document
From: Catalin Marinas @ 2019-08-06 15:27 UTC (permalink / raw)
  To: Steve Capper
  Cc: crecklin, ard.biesheuvel, maz, bhsharma, will, linux-arm-kernel
In-Reply-To: <20190729162117.832-12-steve.capper@arm.com>

On Mon, Jul 29, 2019 at 05:21:17PM +0100, Steve Capper wrote:
> +AArch64 Linux memory layout with 4KB pages + 4 levels (48-bit)::
>  
>    Start			End			Size		Use
>    -----------------------------------------------------------------------
>    0000000000000000	0000ffffffffffff	 256TB		user
> -  ffff000000000000	ffffffffffffffff	 256TB		kernel
> -
> -
> -AArch64 Linux memory layout with 64KB pages + 2 levels::
> +  ffff000000000000	ffff7fffffffffff	 128TB		kernel logical memory map
> +  ffff800000000000	ffff9fffffffffff	  32TB		kasan shadow region
> +  ffffa00000000000	ffffa00007ffffff	 128MB		bpf jit region
> +  ffffa00008000000	ffffa0000fffffff	 128MB		modules
> +  ffffa00010000000	fffffdffbffeffff	 ~93TB		vmalloc
> +  fffffdffbfff0000	fffffdfffe5f8fff	~998MB		[guard region]
> +  fffffdfffe5f9000	fffffdfffe9fffff	4124KB		fixed mappings
> +  fffffdfffea00000	fffffdfffebfffff	   2MB		[guard region]
> +  fffffdfffec00000	fffffdffffbfffff	  16MB		PCI I/O space
> +  fffffdffffc00000	fffffdffffdfffff	   2MB		[guard region]
> +  fffffdffffe00000	ffffffffffdfffff	   2TB		vmemmap
> +  ffffffffffe00000	ffffffffffffffff	   2MB		[guard region]
[...]
> +AArch64 Linux memory layout with 64KB pages + 3 levels (52-bit with HW support)::
>  
> -For details of the virtual kernel memory layout please see the kernel
> -booting log.
> +  Start			End			Size		Use
> +  -----------------------------------------------------------------------
> +  0000000000000000	000fffffffffffff	   4PB		user
> +  fff0000000000000	fff7ffffffffffff	   2PB		kernel logical memory map
> +  fff8000000000000	fffd9fffffffffff	1440TB		[gap]
> +  fffda00000000000	ffff9fffffffffff	 512TB		kasan shadow region
> +  ffffa00000000000	ffffa00007ffffff	 128MB		bpf jit region
> +  ffffa00008000000	ffffa0000fffffff	 128MB		modules
> +  ffffa00010000000	fffff81ffffeffff	 ~88TB		vmalloc
> +  fffff81fffff0000	fffffc1ffe58ffff	  ~3TB		[guard region]
> +  fffffc1ffe590000	fffffc1ffe9fffff	4544KB		fixed mappings
> +  fffffc1ffea00000	fffffc1ffebfffff	   2MB		[guard region]
> +  fffffc1ffec00000	fffffc1fffbfffff	  16MB		PCI I/O space
> +  fffffc1fffc00000	fffffc1fffdfffff	   2MB		[guard region]
> +  fffffc1fffe00000	ffffffffffdfffff	3968GB		vmemmap
> +  ffffffffffe00000	ffffffffffffffff	   2MB		[guard region]

Since we risk getting these out of sync, I'd rather only maintain two
entries: defconfig (4K pages, 48-bit VA) and the largest (64K pages,
52-bit with HW support).


> +52-bit VA support in the kernel
> +-------------------------------
> +If the ARMv8.2-LVA optional feature is present, and we are running
> +with a 64KB page size; then it is possible to use 52-bits of address
> +space for both userspace and kernel addresses. However, any kernel
> +binary that supports 52-bit must also be able to fall back to 48-bit
> +at early boot time if the hardware feature is not present.
> +
> +This fallback mechanism necessitates the kernel .text to be in the
> +higher addresses s.t. they are invariant to 48/52-bti VAs. Due to

The 's.t.' abbreviation always confused me. Could you please change it
to "so that" in the documentation? (I'm not too bothered about the
commit logs).

Also fix s/bti/bit/.

Otherwise:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] iommu/dma: Handle MSI mappings separately
From: Joerg Roedel @ 2019-08-06 15:23 UTC (permalink / raw)
  To: Robin Murphy
  Cc: maz, iommu, Shameer Kolothum, linux-arm-kernel, Andre Przywara
In-Reply-To: <2b2595de703c60a772ebcffe248d0cf036143e6a.1564414114.git.robin.murphy@arm.com>

On Mon, Jul 29, 2019 at 04:32:38PM +0100, Robin Murphy wrote:
>  drivers/iommu/dma-iommu.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)

Applied to iommu/fixes, thanks Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 1/2] dt-bindings: net: snps, dwmac: update reg minItems maxItems
From: Rob Herring @ 2019-08-06 15:22 UTC (permalink / raw)
  To: Neil Armstrong, Maxime Ripard
  Cc: devicetree, Martin Blumenstingl, netdev,
	linux-kernel@vger.kernel.org, linux-amlogic,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20190806125041.16105-2-narmstrong@baylibre.com>

+Maxime

On Tue, Aug 6, 2019 at 6:50 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> The Amlogic Meson DWMAC glue bindings needs a second reg cells for the
> glue registers, thus update the reg minItems/maxItems to allow more
> than a single reg cell.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  Documentation/devicetree/bindings/net/snps,dwmac.yaml | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

I haven't checked, but the derivative schema could be assuming this
schema enforced reg is 1 item. I don't think that's a major issue
though.

Acked-by: Rob Herring <robh@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 2/4 v3] drm/panel: simple: Add TI nspire panel bindings
From: Rob Herring @ 2019-08-06 15:17 UTC (permalink / raw)
  To: Linus Walleij
  Cc: devicetree, Maxime Ripard, Maarten Lankhorst, dri-devel,
	Sean Paul,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20190806135437.7451-1-linus.walleij@linaro.org>

On Tue, Aug 6, 2019 at 7:55 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> Add bindings for the TI NSPIRE simple display panels.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChanegLog v2->v3:
> - Switch to GPL-2.0-only OR BSD-2-Clause license
> - Use a simple enum for the compatible
> - Use the new nifty panel-common.yaml, tested on
>   linux-next
> ChangeLog v1->v2:
> - New patch as bindings are required
> - Let's use YAML
> ---
>  .../bindings/display/panel/ti,nspire.yaml     | 36 +++++++++++++++++++
>  1 file changed, 36 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/ti,nspire.yaml

Reviewed-by: Rob Herring <robh@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 2/2] dt-bindings: net: meson-dwmac: convert to yaml
From: Rob Herring @ 2019-08-06 15:15 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: devicetree, Martin Blumenstingl, netdev,
	linux-kernel@vger.kernel.org, linux-amlogic,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20190806125041.16105-3-narmstrong@baylibre.com>

On Tue, Aug 6, 2019 at 6:50 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> Now that we have the DT validation in place, let's convert the device tree
> bindings for the Synopsys DWMAC Glue for Amlogic SoCs over to a YAML schemas.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  .../bindings/net/amlogic,meson-dwmac.yaml     | 113 ++++++++++++++++++
>  .../devicetree/bindings/net/meson-dwmac.txt   |  71 -----------
>  .../devicetree/bindings/net/snps,dwmac.yaml   |   5 +
>  3 files changed, 118 insertions(+), 71 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
>  delete mode 100644 Documentation/devicetree/bindings/net/meson-dwmac.txt

I don't love the compatible schema with 'additionalItems: true' and
contains, but I guess it is what it is. I'm hopeful schemas help limit
how many variations we end up with.

Reviewed-by: Rob Herring <robh@kernel.org>

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH V4 10/11] arm64: mm: Introduce 52-bit Kernel VAs
From: Catalin Marinas @ 2019-08-06 14:58 UTC (permalink / raw)
  To: Steve Capper
  Cc: crecklin, ard.biesheuvel, maz, bhsharma, will, linux-arm-kernel
In-Reply-To: <20190806145545.GF30716@arrakis.emea.arm.com>

On Tue, Aug 06, 2019 at 03:55:45PM +0100, Catalin Marinas wrote:
> On Mon, Jul 29, 2019 at 05:21:16PM +0100, Steve Capper wrote:
> > @@ -759,13 +759,14 @@ config ARM64_VA_BITS_47
> >  config ARM64_VA_BITS_48
> >  	bool "48-bit"
> >  
> > -config ARM64_USER_VA_BITS_52
> > -	bool "52-bit (user)"
> > +config ARM64_VA_BITS_52
> > +	bool "52-bit"
> 
> I think we should change defconfig as well to make this the default. We
> tend to make defconfig cover all the architecture features we support
> and people can disable them if they get in the way (performance).

Ignore this. It only works with 64K pages and our defconfig is 4K.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox