* [linux-sunxi] [PATCH RESEND 1/2] dt: bindings: add allwinner,otg-routed property for phy-sun4i-usb
From: Hans de Goede @ 2016-10-28 18:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <86c3fad4-e0c1-9aaf-76c5-b9428110464f@redhat.com>
HI,
On 26-10-16 12:14, Hans de Goede wrote:
> Hi,
>
> On 26-10-16 10:52, Icenowy Zheng wrote:
>>
>>
>> 26.10.2016, 16:28, "Hans de Goede" <hdegoede@redhat.com>:
>>> Hi,
>>>
>>> On 25-10-16 06:11, Icenowy Zheng wrote:
>>>> On some newer Allwinner SoCs (H3 or A64), the PHY0 can be either routed to
>>>> the MUSB controller (which is an OTG controller) or the OHCI/EHCI pair
>>>> (which is a Host-only controller, but more stable and easy to implement).
>>>>
>>>> This property marks whether on a certain board which controller should be
>>>> attached to the PHY.
>>>>
>>>> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>>>
>>> Icenowy, I appreciate your work on this, but we really need full otg
>>> support with dynamic switching rather then hardwiring the routing, so
>>> this cannot go in as is.
>>
>> Now I have both PHY0 controllers' drivers.
>>
>> In the tree of https://github.com/Icenowy/linux/tree/ice-a64-v6.1 , I have already
>> enabled MUSB controller.
>>
>> And this patchset is for those prefer a stable USB host implement to dual-role
>> implementation. MUSB is a good UDC, but not a good host controller. My USB
>> sound card cannot work on MUSB on A33. Even connecting a R8's MUSB (Serial
>> Gadget) to an A33's MUSB cannot work.
>
> The idea is for dual-role setups to used the MUSB in gadget mode and the EHCI/OHCI
> pair when in host mode. So for otg setups you would runtime change the mux
> from one controller to the other based on the id pin value.
>
> Take a look at drivers/phy/phy-sun4i-usb.c, around line 512:
>
> if (id_det != data->id_det) {
> ...
> }
>
> This deals with id_det changes (including the initial id_det "change"
> for hardwired host-only ports). This currently assumes that the musb
> will be used for host mode too, we will want to change this to
> something like this:
>
> if (id_det != data->id_det) {
> if (data->cfg->separate_phy0_host_controller) {
> if (id_det) {
> /* Change to gadget mode (id_det == 1), switch phy mux to musb */
> actual code to switch phy mux to musb...
> } else {
> /* Change to host mode (id_det == 0), switch phy mux to ehci/ohci */
> actual code to switch phy mux to ehci/ohci...
> }
> }
> /* old code */
> }
>
> Note this will then still rely on the musb code to actually turn
> the regulator on, so you do need to have the musb driver build and
> loaded. This can be fixed but lets start with the above.
>
> If you combine this with dr_mode = "host"; in the dts, then
> sun4i_usb_phy0_get_id_det() will return 0 so on its first run
> sun4i_usb_phy0_id_vbus_det_scan() will throw the mux to the ehci/ohci
> and everything should work as you want without needing the custom
> "allwinner,otg-routed" property, and we should be more or less
> ready to support full otg on other boards.
I've just found further proof that the musb on the H3 at least
only is intended for gadget mode and that we must dynamically
switch for host-mode. If you look at:
drivers/usb/sunxi_usb/include/sunxi_udc.h
In the h3 sdk then you will see that for the H3 a different fifo
endpoint table is used, as the total fifo space is only 4k where
as previous SoCs had 8k. This means that we need to have 2
different ep tables in drivers/usb/musb/sunxi.c and select by
compatible.
Regards,
Hans
^ permalink raw reply
* [PATCH] staging: vc04_services: call sg_init_table to init scatterlist
From: Michael Zoran @ 2016-10-28 17:58 UTC (permalink / raw)
To: linux-arm-kernel
Call the sg_init_table function to correctly initialze
the DMA scatterlist. This function is required to completely
initialize the list and is mandatory if DMA debugging is
enabled in the build configuration.
One of the purposes of sg_init_table is to set
the magic "cookie" on each list element and ensure
the chain end is marked.
Signed-off-by: Michael Zoran <mzoran@crowfest.net>
---
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index 6fa2b5a..21b26e5 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -464,6 +464,12 @@ create_pagelist(char __user *buf, size_t count, unsigned short type,
pagelist->type = type;
pagelist->offset = offset;
+ /*
+ * Initialize the scatterlist so that the magic cookie
+ * is filled if debugging is enabled
+ */
+ sg_init_table(scatterlist, num_pages);
+ /* Now set the pages for each scatterlist */
for (i = 0; i < num_pages; i++)
sg_set_page(scatterlist + i, pages[i], PAGE_SIZE, 0);
--
2.10.1
^ permalink raw reply related
* [PATCH] [ARM] Fix stack alignment when processing backtraces
From: Jason Gunthorpe @ 2016-10-28 17:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161018170510.GA12248@obsidianresearch.com>
On Tue, Oct 18, 2016 at 11:05:10AM -0600, Jason Gunthorpe wrote:
> The dumpstm helper within c_backtrace pushed 5 dwords onto the stack
> causing the stack to become unaligned and then calls printk. This
> causes memory corruption in the kernel which assumes AAPCS calling
> convention.
>
> Since this bit of asm doesn't use the standard prologue just add
> another register to restore alignment.
>
> Fixes: 7ab3f8d595a1b ("[ARM] Add ability to dump exception stacks to kernel backtraces")
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> arch/arm/lib/backtrace.S | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> In my case the kernel was hitting a WARN_ON during boot and then
> reliably failed to start the compiled-in initramfs.
>
> I'm inferring that the stack misalignment caused some kind of memory
> corruption which wiped out the unpacked initramfs.
>
> Saw with gcc 5.4.0 on a kirkwood armv5te
Since there are no comments, I will send this to RMK's patch system..
Thanks,
Jason
> diff --git a/arch/arm/lib/backtrace.S b/arch/arm/lib/backtrace.S
> index fab5a50503ae..25e1cce19991 100644
> +++ b/arch/arm/lib/backtrace.S
> @@ -116,7 +116,8 @@ ENDPROC(c_backtrace)
> #define reg r5
> #define stack r6
>
> -.Ldumpstm: stmfd sp!, {instr, reg, stack, r7, lr}
> + /* Must maintain 8 byte stack alignment */
> +.Ldumpstm: stmfd sp!, {r3, instr, reg, stack, r7, lr}
> mov stack, r0
> mov instr, r1
> mov reg, #10
> @@ -140,7 +141,7 @@ ENDPROC(c_backtrace)
> teq r7, #0
> adrne r0, .Lcr
> blne printk
> - ldmfd sp!, {instr, reg, stack, r7, pc}
> + ldmfd sp!, {r3, instr, reg, stack, r7, pc}
>
> .Lfp: .asciz " r%d:%08x%s"
> .Lcr: .asciz "\n"
^ permalink raw reply
* [PATCH] ASoC: samsung: Drop AC97 drivers
From: Mark Brown @ 2016-10-28 17:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477671072-742-1-git-send-email-s.nawrocki@samsung.com>
On Fri, Oct 28, 2016 at 06:11:12PM +0200, Sylwester Nawrocki wrote:
> The AC97 drivers are broken and it seems these have not been used
> for a long time. This patch removes the unused code, i.e. Samsung
> SoC AC97 controller driver and related machine drivers:
> ln2440sbc_alc650, smdk2443_wm9710, smdk_wm9713.
This needs rebasing against some of the recent fixes, sorry.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161028/cddf7f0f/attachment.sig>
^ permalink raw reply
* [PATCH v2 3/3] reset: Add the TI SCI reset driver
From: Mathieu Poirier @ 2016-10-28 17:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161027214941.24641-4-afd@ti.com>
On 27 October 2016 at 15:49, Andrew F. Davis <afd@ti.com> wrote:
> Some TI Keystone family of SoCs contain a system controller (like the
> Power Management Micro Controller (PMMC) on K2G SoCs) that manage the
> low-level device control (like clocks, resets etc) for the various
> hardware modules present on the SoC. These device control operations
> are provided to the host processor OS through a communication protocol
> called the TI System Control Interface (TI SCI) protocol.
>
> This patch adds a reset driver that communicates to the system
> controller over the TI SCI protocol for performing reset management
> of various devices present on the SoC. Various reset functionalities
> are achieved by the means of different TI SCI device operations
> provided by the TI SCI framework.
>
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> [s-anna at ti.com: documentation changes, revised commit message]
> Signed-off-by: Suman Anna <s-anna@ti.com>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
> MAINTAINERS | 1 +
> drivers/reset/Kconfig | 9 ++
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-ti-sci.c | 262 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 273 insertions(+)
> create mode 100644 drivers/reset/reset-ti-sci.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index accf991..b93d91a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11901,6 +11901,7 @@ F: include/dt-bindings/clock/k2g.h
> F: drivers/clk/keystone/sci-clk.c
> F: Documentation/devicetree/bindings/reset/ti,sci-reset.txt
> F: include/dt-bindings/reset/k2g.h
> +F: drivers/reset/reset-ti-sci.c
>
> THANKO'S RAREMONO AM/FM/SW RADIO RECEIVER USB DRIVER
> M: Hans Verkuil <hverkuil@xs4all.nl>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 06d9fa2..4c21c9d 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -66,6 +66,15 @@ config RESET_SUNXI
> help
> This enables the reset driver for Allwinner SoCs.
>
> +config RESET_TI_SCI
> + tristate "TI System Control Interface (TI-SCI) reset driver"
> + depends on RESET_CONTROLLER
> + depends on TI_SCI_PROTOCOL
> + help
> + This enables the reset driver support over TI System Control Interface
> + available on some new TI SoCs. If you wish to use reset resources
> + managed by the TI System Controller, say Y here. Otherwise, say N.
> +
> config TI_SYSCON_RESET
> tristate "TI SYSCON Reset Driver"
> depends on HAS_IOMEM
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index bbe7026..36321f2 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
> obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
> obj-$(CONFIG_RESET_STM32) += reset-stm32.o
> obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
> +obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
> obj-$(CONFIG_TI_SYSCON_RESET) += reset-ti-syscon.o
> obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
> obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
> diff --git a/drivers/reset/reset-ti-sci.c b/drivers/reset/reset-ti-sci.c
> new file mode 100644
> index 0000000..42ccf12
> --- /dev/null
> +++ b/drivers/reset/reset-ti-sci.c
> @@ -0,0 +1,262 @@
> +/*
> + * Texas Instrument's System Control Interface (TI-SCI) reset driver
> + *
> + * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/
> + * Andrew F. Davis <afd@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/idr.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/soc/ti/ti_sci_protocol.h>
> +
> +/**
> + * struct ti_sci_reset_control - reset control structure
> + * @dev_id: SoC-specific device identifier
> + * @reset_mask: reset mask to use for toggling reset
> + */
> +struct ti_sci_reset_control {
> + u32 dev_id;
> + u32 reset_mask;
> +};
> +
> +/**
> + * struct ti_sci_reset_data - reset controller information structure
> + * @rcdev: reset controller entity
> + * @dev: reset controller device pointer
> + * @sci: TI SCI handle used for communication with system controller
> + * @idr: idr structure for mapping ids to reset control structures
> + */
> +struct ti_sci_reset_data {
> + struct reset_controller_dev rcdev;
> + struct device *dev;
> + const struct ti_sci_handle *sci;
> + struct idr idr;
> +};
> +
> +#define to_ti_sci_reset_data(p) \
> + container_of((p), struct ti_sci_reset_data, rcdev)
> +
> +/**
> + * ti_sci_reset_set() - program a device's reset
> + * @rcdev: reset controller entity
> + * @id: ID of the reset to toggle
> + * @assert: boolean flag to indicate assert or deassert
> + *
> + * This is a common internal function used to assert or deassert a device's
> + * reset using the TI SCI protocol. The device's reset is asserted if the
> + * @assert argument is true, or deasserted if @assert argument is false.
> + * The mechanism itself is a read-modify-write procedure, the current device
> + * reset register is read using a TI SCI device operation, the new value is
> + * set or un-set using the reset's mask, and the new reset value written by
> + * using another TI SCI device operation.
> + *
> + * Return: 0 for successful request, else a corresponding error value
> + */
> +static int ti_sci_reset_set(struct reset_controller_dev *rcdev,
> + unsigned long id, bool assert)
> +{
> + struct ti_sci_reset_data *data = to_ti_sci_reset_data(rcdev);
> + const struct ti_sci_handle *sci = data->sci;
> + const struct ti_sci_dev_ops *dev_ops = &sci->ops.dev_ops;
> + struct ti_sci_reset_control *control;
> + u32 reset_state;
> + int ret;
> +
> + control = idr_find(&data->idr, id);
> + if (!control)
> + return -EINVAL;
> +
> + ret = dev_ops->get_device_resets(sci, control->dev_id,
> + &reset_state);
> + if (ret)
> + return ret;
> +
> + if (assert)
> + reset_state |= control->reset_mask;
> + else
> + reset_state &= ~control->reset_mask;
> +
> + return dev_ops->set_device_resets(sci, control->dev_id,
> + reset_state);
> +}
> +
> +/**
> + * ti_sci_reset_assert() - assert device reset
> + * @rcdev: reset controller entity
> + * @id: ID of the reset to be asserted
> + *
> + * This function implements the reset driver op to assert a device's reset
> + * using the TI SCI protocol. This invokes the function ti_sci_reset_set()
> + * with the corresponding parameters as passed in, but with the @assert
> + * argument set to true for asserting the reset.
> + *
> + * Return: 0 for successful request, else a corresponding error value
> + */
> +static int ti_sci_reset_assert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + return ti_sci_reset_set(rcdev, id, true);
> +}
> +
> +/**
> + * ti_sci_reset_deassert() - deassert device reset
> + * @rcdev: reset controller entity
> + * @id: ID of the reset to be deasserted
> + *
> + * This function implements the reset driver op to deassert a device's reset
> + * using the TI SCI protocol. This invokes the function ti_sci_reset_set()
> + * with the corresponding parameters as passed in, but with the @assert
> + * argument set to false for deasserting the reset.
> + *
> + * Return: 0 for successful request, else a corresponding error value
> + */
> +static int ti_sci_reset_deassert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + return ti_sci_reset_set(rcdev, id, false);
> +}
> +
> +/**
> + * ti_sci_reset_status() - check device reset status
> + * @rcdev: reset controller entity
> + * @id: ID of reset to be checked
> + *
> + * This function implements the reset driver op to return the status of a
> + * device's reset using the TI SCI protocol. The reset register value is read
> + * by invoking the TI SCI device opertation .get_device_resets(), and the
> + * status of the specific reset is extracted and returned using this reset's
> + * reset mask.
> + *
> + * Return: 0 if reset is deasserted, or a non-zero value if reset is asserted
> + */
> +static int ti_sci_reset_status(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct ti_sci_reset_data *data = to_ti_sci_reset_data(rcdev);
> + const struct ti_sci_handle *sci = data->sci;
> + const struct ti_sci_dev_ops *dev_ops = &sci->ops.dev_ops;
> + struct ti_sci_reset_control *control;
> + u32 reset_state;
> + int ret;
> +
> + control = idr_find(&data->idr, id);
> + if (!control)
> + return -EINVAL;
> +
> + ret = dev_ops->get_device_resets(sci, control->dev_id,
> + &reset_state);
> + if (ret)
> + return ret;
> +
> + return reset_state & control->reset_mask;
> +}
> +
> +static struct reset_control_ops ti_sci_reset_ops = {
> + .assert = ti_sci_reset_assert,
> + .deassert = ti_sci_reset_deassert,
> + .status = ti_sci_reset_status,
> +};
> +
> +/**
> + * ti_sci_reset_of_xlate() - translate a set of OF arguments to a reset ID
> + * @rcdev: reset controller entity
> + * @reset_spec: OF reset argument specifier
> + *
> + * This function performs the translation of the reset argument specifier
> + * values defined in a reset consumer device node. The function allocates a
> + * reset control structure for that device reset, and will be used by the
> + * driver for performing any reset functions on that reset. An idr structure
> + * is allocated and used to map to the reset control structure. This idr
> + * is used by the driver to do reset lookups.
> + *
> + * Return: 0 for successful request, else a corresponding error value
> + */
> +static int ti_sci_reset_of_xlate(struct reset_controller_dev *rcdev,
> + const struct of_phandle_args *reset_spec)
> +{
> + struct ti_sci_reset_data *data = to_ti_sci_reset_data(rcdev);
> + struct ti_sci_reset_control *control;
> +
> + if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
> + return -EINVAL;
> +
> + control = devm_kzalloc(data->dev, sizeof(*control), GFP_KERNEL);
> + if (!control)
> + return -ENOMEM;
> +
> + control->dev_id = reset_spec->args[0];
> + control->reset_mask = reset_spec->args[1];
> +
> + return idr_alloc(&data->idr, control, 0, 0, GFP_KERNEL);
> +}
> +
> +static const struct of_device_id ti_sci_reset_of_match[] = {
> + { .compatible = "ti,sci-reset", },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, ti_sci_reset_of_match);
> +
> +static int ti_sci_reset_probe(struct platform_device *pdev)
> +{
> + struct ti_sci_reset_data *data;
> +
> + if (!pdev->dev.of_node)
> + return -ENODEV;
> +
> + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->sci = devm_ti_sci_get_handle(&pdev->dev);
> + if (IS_ERR(data->sci))
> + return PTR_ERR(data->sci);
> +
> + data->rcdev.ops = &ti_sci_reset_ops;
> + data->rcdev.owner = THIS_MODULE;
> + data->rcdev.of_node = pdev->dev.of_node;
> + data->rcdev.of_reset_n_cells = 2;
> + data->rcdev.of_xlate = ti_sci_reset_of_xlate;
> + data->dev = &pdev->dev;
> + idr_init(&data->idr);
Hello Andrew,
For my own education, is there a specific reason to use a struct idr
as opposed to keeping a pointer to a struct ti_sci_reset_control in
truct ti_sci_reset_data? I'm not opposed to the way you've done
things but simply keeping a pointer sound more intuitive to me.
Thanks,
Mathieu
> +
> + platform_set_drvdata(pdev, data);
> +
> + return reset_controller_register(&data->rcdev);
> +}
> +
> +static int ti_sci_reset_remove(struct platform_device *pdev)
> +{
> + struct ti_sci_reset_data *data = platform_get_drvdata(pdev);
> +
> + reset_controller_unregister(&data->rcdev);
> +
> + idr_destroy(&data->idr);
> +
> + return 0;
> +}
> +
> +static struct platform_driver ti_sci_reset_driver = {
> + .probe = ti_sci_reset_probe,
> + .remove = ti_sci_reset_remove,
> + .driver = {
> + .name = "ti-sci-reset",
> + .of_match_table = ti_sci_reset_of_match,
> + },
> +};
> +module_platform_driver(ti_sci_reset_driver);
> +
> +MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>");
> +MODULE_DESCRIPTION("TI System Control Interface (TI SCI) Reset driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.10.1
>
^ permalink raw reply
* SMR masking and PCI
From: Mark Rutland @ 2016-10-28 17:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c39b785a-0f18-fc0a-ce08-7ebe3cfaf8c5@arm.com>
Hi,
On Fri, Oct 28, 2016 at 05:16:37PM +0100, Robin Murphy wrote:
> On 27/10/16 18:10, Stuart Yoder wrote:
> > A question about how the SMR masking defined in the arm,smmu binding
> > relates to the PCI iommu-map.
> >
> > The #iommu-cells property defines the number of cells an "IOMMU specifier"
> > takes and 2 is specified to be:
> >
> > SMMUs with stream matching support and complex masters
> > may use a value of 2, where the second cell represents
> > an SMR mask to combine with the ID in the first cell.
> >
> > An iommu-map entry is defined as:
> >
> > (rid-base,iommu,iommu-base,length)
> >
> > What seems to be currently missing in the iommu-map support is
> > the possibility the case where #iommu-cells=<2>.
>
> Indeed. The bindings have so far rather implicitly assumed the case of
> #{msi,iommu}-cells = 1, and the code has followed suit.
The intention was that neither the iommu or msi bindings had such a
requirement, but evidently I did not specify the intended behaviour
thoroughly enough.
I had intended that the offset was added to the final cell of the
iommu-specifier (i.e. that the iommu-specifier was treated as a large
number).
You can handle this case by adding additional entries in the map table,
with a single entry length.
> > In this case iommu-base which is an IOMMU specifier should
> > occupy 2 cells. For example on an ls2085a we would want:
> >
> > iommu-map = <0x0 0x6 0x7 0x3ff 0x1
> > 0x100 0x6 0x8 0x3ff 0x1>;
> >
> > ...to mask our stream IDs to 10 bits.
> >
> > This should work in theory and comply with the bindings, no?
>
> In theory, but now consider:
>
> iommu-map = <0x0 0x6 0x7 0x3ff 0x2>;
>
> faced with ID 1. The input base is 0, the output base is the 2-cell
> value 0x7000003ff, so the final ID value should be 0x700000400, right?
That was the intended behaviour, yes.
> > (Also, I guess that msi-map is not affected by this since it
> > is not related to the IOMMU...but we do have common code
> > handling both maps.)
>
> I'd say it's definitely affected, since #msi-cells can equally be more
> than 1, and encodes an equally opaque value.
Yes.
> It seems pretty reasonable to me that we could extend the binding to
> accommodate #cells > 1 iff length == 1. Mark?
I will try to come up with the wording to make the above explicit, for
both bindings.
Thanks,
Mark.
^ permalink raw reply
* [RFC][PATCH] arm64: Add support for CONFIG_DEBUG_VIRTUAL
From: Laura Abbott @ 2016-10-28 17:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161028144951.GI5806@leverpostej>
On 10/28/2016 07:49 AM, Mark Rutland wrote:
> Hi Laura,
>
> On Thu, Oct 27, 2016 at 05:18:12PM -0700, Laura Abbott wrote:
>> x86 has an option CONFIG_DEBUG_VIRTUAL to do additional checks
>> on virt_to_phys calls. The goal is to catch users who are calling
>> virt_to_phys on non-linear addresses immediately. As features
>> such as CONFIG_VMAP_STACK get enabled for arm64, this becomes
>> increasingly important. Add checks to catch bad virt_to_phys
>> usage.
>>
>> Signed-off-by: Laura Abbott <labbott@redhat.com>
>> ---
>> This has been on my TODO list for a while. It caught a few bugs with
>> CONFIG_VMAP_STACK on x86 so when that eventually gets added
>> for arm64 it will be useful to have. This caught one driver calling __pa on an
>> ioremapped address already.
>
> This is fantastic; thanks for putting this together!
>
>> RFC for a couple of reasons:
>>
>> 1) This is basically a direct port of the x86 approach.
>> 2) I needed some #ifndef __ASSEMBLY__ which I don't like to throw around.
>> 3) I'm not quite sure about the bounds check for the VIRTUAL_BUG_ON with KASLR,
>> specifically the _end check.
>> 4) Is it worth actually keeping this as DEBUG_VIRTUAL vs. folding it into
>> another option?
>
> I think it's worth aligning with x86, so modulo a couple of comments
> below, (1) and (4) seem fine. I think (2) just requires an additional
> shuffle.
>
>> ---
>> arch/arm64/include/asm/memory.h | 11 ++++++++++-
>> arch/arm64/mm/Makefile | 2 +-
>> arch/arm64/mm/physaddr.c | 17 +++++++++++++++++
>> lib/Kconfig.debug | 2 +-
>> mm/cma.c | 2 +-
>> 5 files changed, 30 insertions(+), 4 deletions(-)
>> create mode 100644 arch/arm64/mm/physaddr.c
>>
>> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
>> index ba62df8..9805adc 100644
>> --- a/arch/arm64/include/asm/memory.h
>> +++ b/arch/arm64/include/asm/memory.h
>> @@ -106,11 +106,19 @@
>> * private definitions which should NOT be used outside memory.h
>> * files. Use virt_to_phys/phys_to_virt/__pa/__va instead.
>> */
>> -#define __virt_to_phys(x) ({ \
>> +#define __virt_to_phys_nodebug(x) ({ \
>> phys_addr_t __x = (phys_addr_t)(x); \
>> __x & BIT(VA_BITS - 1) ? (__x & ~PAGE_OFFSET) + PHYS_OFFSET : \
>> (__x - kimage_voffset); })
>>
>> +#ifdef CONFIG_DEBUG_VIRTUAL
>> +#ifndef __ASSEMBLY__
>> +extern unsigned long __virt_to_phys(unsigned long x);
>> +#endif
>> +#else
>> +#define __virt_to_phys(x) __virt_to_phys_nodebug(x)
>> +#endif
>> +
>> #define __phys_to_virt(x) ((unsigned long)((x) - PHYS_OFFSET) | PAGE_OFFSET)
>> #define __phys_to_kimg(x) ((unsigned long)((x) + kimage_voffset))
>
> I think we can move all the existing conversion logic here (including
> into page_to_phys/phys_to_page) into the existing #ifndef __ASSEMBLY__
> block at the end of the file. Assembly clearly can't use these, and we
> already have virt_to_phys and others in that #ifndef.
>
> Assuming that works, would you mind doing that as a preparatory patch?
>
Sure.
>> @@ -202,6 +210,7 @@ static inline void *phys_to_virt(phys_addr_t x)
>> * Drivers should NOT use these either.
>> */
>> #define __pa(x) __virt_to_phys((unsigned long)(x))
>> +#define __pa_nodebug(x) __virt_to_phys_nodebug((unsigned long)(x))
>> #define __va(x) ((void *)__phys_to_virt((phys_addr_t)(x)))
>> #define pfn_to_kaddr(pfn) __va((pfn) << PAGE_SHIFT)
>> #define virt_to_pfn(x) __phys_to_pfn(__virt_to_phys(x))
>> diff --git a/arch/arm64/mm/Makefile b/arch/arm64/mm/Makefile
>> index 54bb209..bcea84e 100644
>> --- a/arch/arm64/mm/Makefile
>> +++ b/arch/arm64/mm/Makefile
>> @@ -5,6 +5,6 @@ obj-y := dma-mapping.o extable.o fault.o init.o \
>> obj-$(CONFIG_HUGETLB_PAGE) += hugetlbpage.o
>> obj-$(CONFIG_ARM64_PTDUMP) += dump.o
>> obj-$(CONFIG_NUMA) += numa.o
>> -
>> +obj-$(CONFIG_DEBUG_VIRTUAL) += physaddr.o
>> obj-$(CONFIG_KASAN) += kasan_init.o
>> KASAN_SANITIZE_kasan_init.o := n
>> diff --git a/arch/arm64/mm/physaddr.c b/arch/arm64/mm/physaddr.c
>> new file mode 100644
>> index 0000000..6c271e2
>> --- /dev/null
>> +++ b/arch/arm64/mm/physaddr.c
>> @@ -0,0 +1,17 @@
>> +#include <linux/mm.h>
>> +
>> +#include <asm/memory.h>
>> +
>> +unsigned long __virt_to_phys(unsigned long x)
>> +{
>> + phys_addr_t __x = (phys_addr_t)x;
>> +
>> + if (__x & BIT(VA_BITS - 1)) {
>> + /* The bit check ensures this is the right range */
>> + return (__x & ~PAGE_OFFSET) + PHYS_OFFSET;
>> + } else {
>> + VIRTUAL_BUG_ON(x < kimage_vaddr || x > (unsigned long)_end);
>
> IIUC, in (3) you were asking if the last check should be '>' or '>='?
>
> To match high_memory, I suspect the latter, as _end doesn't fall within
> the mapped virtual address space.
>
I was actually concerned about if _end would be correct with KASLR.
Ard confirmed that it gets fixed up to be correct. I'll change the
check to check for >=.
>> + return (__x - kimage_voffset);
>> + }
>> +}
>> +EXPORT_SYMBOL(__virt_to_phys);
>
> It's a bit annoying that we have to duplicate the logic here to add the
> VIRTUAL_BUG_ON(), but I see that x86 also do this, and I don't have a
> better suggestion.
>
>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> index 33bc56c..e5634bb 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -604,7 +604,7 @@ config DEBUG_VM_PGFLAGS
>>
>> config DEBUG_VIRTUAL
>> bool "Debug VM translations"
>> - depends on DEBUG_KERNEL && X86
>> + depends on DEBUG_KERNEL && (X86 || ARM64)
>
> I have no strong feelings about this, but perhaps it's nicer to have
> something like ARCH_HAS_DEBUG_VIRTUAL?
>
Yes, if this ever gets added for other arches this will start to get
unwieldy. I'll look at cleaning that up.
>> help
>> Enable some costly sanity checks in virtual to page code. This can
>> catch mistakes with virt_to_page() and friends.
>> diff --git a/mm/cma.c b/mm/cma.c
>> index 384c2cb..2345803 100644
>> --- a/mm/cma.c
>> +++ b/mm/cma.c
>> @@ -235,7 +235,7 @@ int __init cma_declare_contiguous(phys_addr_t base,
>> phys_addr_t highmem_start;
>> int ret = 0;
>>
>> -#ifdef CONFIG_X86
>> +#if defined(CONFIG_X86) || defined(CONFIG_ARM64)
>
> Rather than an architecture list, can we depend on DEBUG_VIRTUAL? Are
> there other checks that we're trying to avoid?
>
> ... or could we avoid ifdeffery entirely with something like:
>
> /*
> * We can't use __pa(high_memory) directly, since high_memory
> * isn't a valid direct map VA, and DEBUG_VIRTUAL will (validly)
> * complain. Find the boundary by adding one to the last valid
> * address.
> */
> highmem_start = __pa(high_memory - 1) + 1;
>
I like getting rid of the #ifdef there. Maybe future cleanup could turn
this into a #define, HIGHMEM_PHYS since it seems to be used in quite
a few places in the kernel.
> Thanks,
> Mark.
>
Thanks,
Laura
>> /*
>> * high_memory isn't direct mapped memory so retrieving its physical
>> * address isn't appropriate. But it would be useful to check the
>> --
>> 2.7.4
>>
^ permalink raw reply
* [PATCH v2 5/5] fpga manager: cyclone-ps-spi: make delay variable
From: Moritz Fischer @ 2016-10-28 17:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2239ffc8aba7d053825d2bea122a3c16cdd9f6e1.1477669745.git.stillcompiling@gmail.com>
Hi Joshua,
looks good to me; however, I think since you're adding initial support,
I'd squash this together with [3/5].
On Fri, Oct 28, 2016 at 09:56:42AM -0700, Joshua Clayton wrote:
> The status pin may not show ready in the time described in the
> Altetera manual. check the value several times before giving up
s/Altetera/Altera
> For the hardware I am working on, the status pin takes 250 us,
> 5 times as long as described by Altera.
>
> Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
> ---
> drivers/fpga/cyclone-ps-spi.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/fpga/cyclone-ps-spi.c b/drivers/fpga/cyclone-ps-spi.c
> index 4b70d5c..c368223 100644
> --- a/drivers/fpga/cyclone-ps-spi.c
> +++ b/drivers/fpga/cyclone-ps-spi.c
> @@ -20,6 +20,7 @@
>
> #define FPGA_RESET_TIME 50 /* time in usecs to trigger FPGA config */
> -#define FPGA_MIN_DELAY 250 /* min usecs to wait for config status */
> +#define FPGA_MIN_DELAY 50 /* min usecs to wait for config status */
> +#define FPGA_MAX_DELAY 1000 /* max usecs to wait for config status */
>
> struct cyclonespi_conf {
> struct gpio_desc *config;
> @@ -42,6 +43,7 @@ static int cyclonespi_write_init(struct fpga_manager *mgr, u32 flags,
> const char *buf, size_t count)
> {
> struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
> + int i;
>
> if (flags & FPGA_MGR_PARTIAL_RECONFIG) {
> dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
> @@ -56,13 +58,14 @@ static int cyclonespi_write_init(struct fpga_manager *mgr, u32 flags,
> }
>
> gpiod_set_value(conf->config, 1);
> - usleep_range(FPGA_MIN_DELAY, FPGA_MIN_DELAY + 20);
> - if (gpiod_get_value(conf->status) == 0) {
> - dev_err(&mgr->dev, "Status pin not ready.\n");
> - return -EIO;
> + for (i = 0; i < (FPGA_MAX_DELAY / FPGA_MIN_DELAY); i++) {
> + usleep_range(FPGA_MIN_DELAY, FPGA_MIN_DELAY + 20);
> + if (gpiod_get_value(conf->status))
> + return 0;
> }
>
> - return 0;
> + dev_err(&mgr->dev, "Status pin not ready.\n");
> + return -EIO;
> }
>
> static void rev_buf(void *buf, size_t len)
> --
> 2.7.4
>
Cheers,
Moritz
^ permalink raw reply
* [PATCH v5 1/7] drm: sunxi: Add a basic DRM driver for Allwinner DE2
From: Jean-Francois Moine @ 2016-10-28 17:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161027220316.y7u3h4qsfftryrmp@lukather>
On Fri, 28 Oct 2016 00:03:16 +0200
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> On Tue, Oct 25, 2016 at 04:14:41PM +0200, Jean-Francois Moine wrote:
> > > > +Display controller
> > > > +==================
> > > > +
> > > > +Required properties:
> > > > +
> > > > +- compatible: value should be one of the following
> > > > + "allwinner,sun8i-a83t-display-engine"
> > > > + "allwinner,sun8i-h3-display-engine"
> > > > +
> > > > +- clocks: must include clock specifiers corresponding to entries in the
> > > > + clock-names property.
> > > > +
> > > > +- clock-names: must contain
> > > > + "gate": for DE activation
> > > > + "clock": DE clock
> > >
> > > We've been calling them bus and mod.
> >
> > I can understand "bus" (which is better than "apb"), but why "mod"?
>
> Allwinner has been calling the clocks that are supposed to generate
> the external signals (depending on where you were looking) module or
> mod clocks (which is also why we have mod in the clock
> compatibles). The module 1 clocks being used for the audio and the
> module 0 for the rest (SPI, MMC, NAND, display, etc.)
I did not find any 'module' in the H3 documentation.
So, is it really a good name?
> > > > +
> > > > +- resets: phandle to the reset of the device
> > > > +
> > > > +- ports: phandle's to the LCD ports
> > >
> > > Please use the OF graph.
> >
> > These ports are references to the graph of nodes. See
> > http://www.kernelhub.org/?msg=911825&p=2
>
> In an OF-graph, your phandle to the LCD controller would be replaced
> by an output endpoint.
This is the DE controller. There is no endpoint link at this level.
The Device Engine just handles the planes of the LCDs, but, indeed,
the LCDs must know about the DE and the DE must know about the LCDs.
There are 2 ways to realize this knowledge in the DT:
1) either the DE has one or two phandle's to the LCDs,
2) or the LCDs have a phandle to the DE.
I chose the 1st way, the DE ports pointing to the endpoint of the LCDs
which is part of the video link (OF-graph LCD <-> connector).
It would be possible to have phandles to the LCDs themselves, but this
asks for more code.
The second way is also possible, but it also complexifies a bit the
exchanges DE <-> LCD.
> > [snip]
> > > > +struct tcon {
> > > > + u32 gctl;
> > > > +#define TCON_GCTL_TCON_En BIT(31)
[snip]
> > > > + u32 fill_ctl; /* 0x300 */
> > > > + u32 fill_start0;
> > > > + u32 fill_end0;
> > > > + u32 fill_data0;
> > > > +};
> > >
> > > Please use defines instead of the structures.
> >
> > I think that structures are more readable.
>
> That's not really the point. No one in the kernel uses it (and even
> you use defines for registers offset in some places of that
> patch). And then you have Andr? arguments.
I am not convinced, but I'll do as you said.
> > > > +void de2_disable_vblank(struct drm_device *drm, unsigned crtc)
> > > > +{
> > > > + struct priv *priv = drm->dev_private;
> > > > + struct lcd *lcd = priv->lcds[crtc];
> > > > +
> > > > + tcon_write(lcd->mmio, gint0,
> > > > + tcon_read(lcd->mmio, gint0) &
> > > > + ~TCON_GINT0_TCON1_Vb_Int_En);
> > > > +}
> > > > +
> > > > +/* panel functions */
> > >
> > > Panel functions? In the CRTC driver?
> >
> > Yes, dumb panel.
>
> What do you mean by that? Using a Parallel/RGB interface?
Sorry, I though this was a well-known name. The 'dump panel' was used
in the documentation of my previous ARM machine as the video frame sent
to the HDMI controller. 'video_frame' is OK for you?
[snip]
> > > > + ret = clk_prepare_enable(lcd->clk);
> > > > + if (ret)
> > > > + goto err2;
> > >
> > > Is there any reason not to do that in the enable / disable? Leaving
> > > clocks running while the device has no guarantee that it's going to be
> > > used seems like a waste of resources.
> >
> > If the machine does not need video (network server, router..), it is simpler
> > to prevent the video driver to be loaded (DT, module black list...).
>
> You might not have control on any of it, or you might just have no
> monitor attached for example. Recompiling the kernel or updating the
> DT when you want to plug an HDMI monitor seems like a poor UX :)
OK, I will check if this works.
> > > > +static const struct {
> > > > + char chan;
> > > > + char layer;
> > > > + char pipe;
> > > > +} plane2layer[DE2_N_PLANES] = {
> > > > + [DE2_PRIMARY_PLANE] = {0, 0, 0},
> > > > + [DE2_CURSOR_PLANE] = {1, 0, 1},
> > > > + [DE2_VI_PLANE] = {0, 1, 0},
> > > > +};
> > >
> > > Comments?
> >
> > This
> > primary plane is channel 0 (VI), layer 0, pipe 0
> > cursor plane is channel 1 (UI), layer 0, pipe 1
> > overlay plane is channel 0 (VI), layer 1, pipe 0
> > or the full explanation:
> > Constraints:
> > The VI channels can do RGB or YUV, while UI channels can do RGB
> > only.
> > The LCD 0 has 1 VI channel and 4 UI channels, while
> > LCD 1 has only 1 VI channel and 1 UI channel.
> > The cursor must go to a channel bigger than the primary channel,
> > otherwise it is not transparent.
> > First try:
> > Letting the primary plane (usually RGB) in the 2nd channel (UI),
> > as this is done in the legacy driver, asks for the cursor to go
> > to the next channel (UI), but this one does not exist in LCD1.
> > Retained layout:
> > So, we must use only 2 channels for the same behaviour on LCD0
> > (H3) and LCD1 (A83T)
> > The retained combination is:
> > - primary plane in the first channel (VI),
> > - cursor plane inthe 2nd channel (UI), and
> > - overlay plane in the 1st channel (VI).
> >
> > Note that there could be 3 overlay planes (a channel has 4
> > layers), but I am not sure that the A83T or the H3 could
> > support 3 simultaneous video streams...
>
> Do you know if the pipe works in the old display engine?
>
> Especially about the two-steps composition that wouldn't allow you to
> have alpha on all the planes?
>
> If it is similar, I think hardcoding the pipe number is pretty bad,
> because that would restrict the combination of planes and formats,
> while some other might have worked.
>From what I understood about the DE2, the pipes just define the priority
of the overlay channels (one pipe for one channel).
With the cursor constraint, there must be at least 2 channels in
order (primary, cursor). Then, with these 2 channels/pipes, there can be
6 so-called overlay planes (3 RGB/YUV and 3 RGB only).
Enabling the pipes 2 and 3 (LCD 0 only) would offer 8 more planes, but
RGB only. Then, it might be useful to have dynamic pipes.
[snip]
> > > > +void de2_de_plane_update(struct priv *priv,
> > > > + int lcd_num, int plane_ix,
> > > > + struct drm_plane_state *state,
> > > > + struct drm_plane_state *old_state)
> > > > +{
> > > > + struct drm_framebuffer *fb = state->fb;
> > > > + struct drm_gem_cma_object *gem;
> > > > + void __iomem *mux_o = priv->mmio;
> > > > + void __iomem *chan_o;
> > > > + u32 size = WH(state->crtc_w, state->crtc_h);
> > > > + u32 coord;
> > > > + u32 screen_size;
> > > > + u32 data, fcolor;
> > > > + u32 ui_sel, alpha_glob;
> > > > + int chan, layer, x, y;
> > > > + unsigned fmt;
> > > > + unsigned long flags;
> > > > +
> > > > + chan = plane2layer[plane_ix].chan;
> > > > + layer = plane2layer[plane_ix].layer;
> > > > +
> > > > + mux_o += (lcd_num == 0) ? DE_MUX0_BASE : DE_MUX1_BASE;
> > > > + chan_o = mux_o;
> > > > + chan_o += DE_MUX_CHAN_REGS + DE_MUX_CHAN_SZ * chan;
> > > > +
> > > > + x = state->crtc_x >= 0 ? state->crtc_x : 0;
> > > > + y = state->crtc_y >= 0 ? state->crtc_y : 0;
> > > > + coord = XY(x, y);
> > > > +
> > > > + /* handle the cursor move */
> > > > + if (plane_ix == DE2_CURSOR_PLANE
> > > > + && fb == old_state->fb) {
> > > > + spin_lock_irqsave(&de_lock, flags);
> > > > + de_lcd_select(priv, lcd_num, mux_o);
> > > > + if (chan == 0)
> > > > + vi_write(chan_o, cfg[layer].coord, coord);
> > > > + else
> > > > + ui_write(chan_o, cfg[layer].coord, coord);
> > > > + spin_unlock_irqrestore(&de_lock, flags);
> > > > + return;
> > > > + }
> > > > +
> > > > + gem = drm_fb_cma_get_gem_obj(fb, 0);
> > > > +
> > > > + ui_sel = alpha_glob = 0;
> > > > + switch (fb->pixel_format) {
> > > > + case DRM_FORMAT_ARGB8888:
> > > > + fmt = DE2_FORMAT_ARGB_8888;
> > > > + ui_sel = VI_CFG_ATTR_ui_sel;
> > > > + break;
> > > > + case DRM_FORMAT_BGRA8888:
> > > > + fmt = DE2_FORMAT_BGRA_8888;
> > > > + ui_sel = VI_CFG_ATTR_ui_sel;
> > > > + break;
> > > > + case DRM_FORMAT_XRGB8888:
> > > > + fmt = DE2_FORMAT_XRGB_8888;
> > > > + ui_sel = VI_CFG_ATTR_ui_sel;
> > > > + alpha_glob = (1 << UI_CFG_ATTR_alpmod_SHIFT) |
> > > > + (0xff << UI_CFG_ATTR_alpha_SHIFT);
> > > > + break;
> > > > + case DRM_FORMAT_RGB888:
> > > > + fmt = DE2_FORMAT_RGB_888;
> > > > + ui_sel = VI_CFG_ATTR_ui_sel;
> > > > + break;
> > > > + case DRM_FORMAT_BGR888:
> > > > + fmt = DE2_FORMAT_BGR_888;
> > > > + ui_sel = VI_CFG_ATTR_ui_sel;
> > > > + break;
> > > > + case DRM_FORMAT_YUYV:
> > > > + fmt = DE2_FORMAT_YUV422_I_YUYV;
> > > > + break;
> > > > + case DRM_FORMAT_YVYU:
> > > > + fmt = DE2_FORMAT_YUV422_I_YVYU;
> > > > + break;
> > > > + case DRM_FORMAT_YUV422:
> > > > + fmt = DE2_FORMAT_YUV422_P;
> > > > + break;
> > > > + case DRM_FORMAT_YUV420:
> > > > + fmt = DE2_FORMAT_YUV420_P;
> > > > + break;
> > > > + case DRM_FORMAT_UYVY:
> > > > + fmt = DE2_FORMAT_YUV422_I_UYVY;
> > > > + break;
> > > > + default:
> > > > + pr_err("format %.4s not yet treated\n",
> > > > + (char *) &fb->pixel_format);
> > > > + return;
> > > > + }
> > > > +
> > > > + spin_lock_irqsave(&de_lock, flags);
> > > > +
> > > > + screen_size = plane_ix == DE2_PRIMARY_PLANE ?
> > > > + size :
> > > > + glb_read(mux_o + DE_MUX_GLB_REGS, size);
> > > > +
> > > > + /* prepare the activation of alpha blending (1 bit per plane) */
> > > > + fcolor = bld_read(mux_o + DE_MUX_BLD_REGS, fcolor_ctl)
> > > > + | (0x100 << plane2layer[plane_ix].pipe);
> > > > +
> > > > + de_lcd_select(priv, lcd_num, mux_o);
> > > > +
> > > > + if (chan == 0) { /* VI channel */
> > > > + int i;
> > > > +
> > > > + data = VI_CFG_ATTR_en | (fmt << VI_CFG_ATTR_fmt_SHIFT) |
> > > > + ui_sel;
> > > > + vi_write(chan_o, cfg[layer].attr, data);
> > > > + vi_write(chan_o, cfg[layer].size, size);
> > > > + vi_write(chan_o, cfg[layer].coord, coord);
> > > > + for (i = 0; i < VI_N_PLANES; i++) {
> > > > + vi_write(chan_o, cfg[layer].pitch[i],
> > > > + fb->pitches[i] ? fb->pitches[i] :
> > > > + fb->pitches[0]);
> > > > + vi_write(chan_o, cfg[layer].top_laddr[i],
> > > > + gem->paddr + fb->offsets[i]);
> > > > + vi_write(chan_o, fcolor[layer], 0xff000000);
> > > > + }
> > > > + if (layer == 0)
> > > > + vi_write(chan_o, ovl_size[0], screen_size);
> > > > +
> > > > + } else { /* UI channel */
> > > > + data = UI_CFG_ATTR_en | (fmt << UI_CFG_ATTR_fmt_SHIFT) |
> > > > + alpha_glob;
> > > > + ui_write(chan_o, cfg[layer].attr, data);
> > > > + ui_write(chan_o, cfg[layer].size, size);
> > > > + ui_write(chan_o, cfg[layer].coord, coord);
> > > > + ui_write(chan_o, cfg[layer].pitch, fb->pitches[0]);
> > > > + ui_write(chan_o, cfg[layer].top_laddr,
> > > > + gem->paddr + fb->offsets[0]);
> > > > + if (layer == 0)
> > > > + ui_write(chan_o, ovl_size, screen_size);
> > > > + }
> > > > + bld_write(mux_o + DE_MUX_BLD_REGS, fcolor_ctl, fcolor);
> > > > +
> > > > + spin_unlock_irqrestore(&de_lock, flags);
> > > > +}
> > >
> > > Splitting that into functions would make it a bit more trivial and
> > > readable.
> >
> > Not sure: there is a lot of common data and different I/O accesses.
>
> You could still have different ones to set the buffers, formats and
> coordinates for example.
I will have a look...
[snip]
> > > > +static int __init de2_drm_init(void)
> > > > +{
> > > > + int ret;
> > > > +
> > > > +/* uncomment to activate the drm traces at startup time */
> > > > +/* drm_debug = DRM_UT_CORE | DRM_UT_DRIVER | DRM_UT_KMS |
> > > > + DRM_UT_PRIME | DRM_UT_ATOMIC; */
> > >
> > > That's useless.
> >
> > Right, but it seems that some people don't know how to debug a DRM
> > driver. This is only a reminder.
> >
> > > > + DRM_DEBUG_DRIVER("\n");
> > > > +
> > > > + ret = platform_driver_register(&de2_lcd_platform_driver);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > +
> > > > + ret = platform_driver_register(&de2_drm_platform_driver);
> > > > + if (ret < 0)
> > > > + platform_driver_unregister(&de2_lcd_platform_driver);
> > > > +
> > > > + return ret;
> > > > +}
> > >
> > > And that really shouldn't be done that way.
> >
> > May you explain?
>
> This goes against the whole idea of the device and driver
> model. Drivers should only register themselves, device should be
> created by buses (or by using some external components if the bus
> can't: DT, ACPI, etc.). If there's a match, you get probed.
>
> A driver that creates its own device just to probe itself violates
> that.
In this function (module init), there is no driver yet.
The module contains 2 drivers: the DE (planes) and the LCD (CRTC),
and there is no macro to handle such modules.
> > > > +int de2_plane_init(struct drm_device *drm, struct lcd *lcd)
> > > > +{
> > > > + int ret, possible_crtcs = 1 << lcd->crtc_idx;
> > > > +
> > > > + ret = de2_one_plane_init(drm, &lcd->planes[DE2_PRIMARY_PLANE],
> > > > + DRM_PLANE_TYPE_PRIMARY, possible_crtcs,
> > > > + ui_formats, ARRAY_SIZE(ui_formats));
> > > > + if (ret >= 0)
> > > > + ret = de2_one_plane_init(drm, &lcd->planes[DE2_CURSOR_PLANE],
> > > > + DRM_PLANE_TYPE_CURSOR, possible_crtcs,
> > > > + ui_formats, ARRAY_SIZE(ui_formats));
> > >
> > > Nothing looks really special about that cursor plane. Any reasion not
> > > to make it an overlay?
> >
> > As explained above (channel/layer/pipe plane definitions), the cursor
> > cannot go in a channel lower or equal to the one of the primary plane.
> > Then, it must be known and, so, have an explicit plane.
>
> If you were to make it a plane, you could use atomic_check to check
> this and make sure this doesn't happen. And you would gain a generic
> plane that can be used for other purposes if needed.
The function drm_crtc_init_with_planes() offers a cursor plane for free.
On the other side, having 6 overlay planes is more than the SoCs can
support.
--
Ken ar c'henta? | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
^ permalink raw reply
* [PATCH v4 8/8] arm64: Enable CONFIG_ARM64_SW_TTBR0_PAN
From: Catalin Marinas @ 2016-10-28 17:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477675636-3957-1-git-send-email-catalin.marinas@arm.com>
This patch adds the Kconfig option to enable support for TTBR0 PAN
emulation. The option is default off because of a slight performance hit
when enabled, caused by the additional TTBR0_EL1 switching during user
access operations or exception entry/exit code.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/Kconfig | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 969ef880d234..6b9a8446fc43 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -790,6 +790,14 @@ config SETEND_EMULATION
If unsure, say Y
endif
+config ARM64_SW_TTBR0_PAN
+ bool "Emulate Privileged Access Never using TTBR0_EL1 switching"
+ help
+ Enabling this option prevents the kernel from accessing
+ user-space memory directly by pointing TTBR0_EL1 to a reserved
+ zeroed area and reserved ASID. The user access routines
+ restore the valid TTBR0_EL1 temporarily.
+
menu "ARMv8.1 architectural features"
config ARM64_HW_AFDBM
^ permalink raw reply related
* [PATCH v4 7/8] arm64: xen: Enable user access before a privcmd hvc call
From: Catalin Marinas @ 2016-10-28 17:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477675636-3957-1-git-send-email-catalin.marinas@arm.com>
Privcmd calls are issued by the userspace. The kernel needs to enable
access to TTBR0_EL1 as the hypervisor would issue stage 1 translations
to user memory via AT instructions. Since AT instructions are not
affected by the PAN bit (ARMv8.1), we only need the explicit
uaccess_enable/disable if the TTBR0 PAN option is enabled.
Reviewed-by: Julien Grall <julien.grall@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/xen/hypercall.S | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/arm64/xen/hypercall.S b/arch/arm64/xen/hypercall.S
index 329c8027b0a9..b41aff25426d 100644
--- a/arch/arm64/xen/hypercall.S
+++ b/arch/arm64/xen/hypercall.S
@@ -49,6 +49,7 @@
#include <linux/linkage.h>
#include <asm/assembler.h>
+#include <asm/uaccess.h>
#include <xen/interface/xen.h>
@@ -91,6 +92,20 @@ ENTRY(privcmd_call)
mov x2, x3
mov x3, x4
mov x4, x5
+ /*
+ * Privcmd calls are issued by the userspace. The kernel needs to
+ * enable access to TTBR0_EL1 as the hypervisor would issue stage 1
+ * translations to user memory via AT instructions. Since AT
+ * instructions are not affected by the PAN bit (ARMv8.1), we only
+ * need the explicit uaccess_enable/disable if the TTBR0 PAN emulation
+ * is enabled (it implies that hardware UAO and PAN disabled).
+ */
+ uaccess_ttbr0_enable x6, x7
hvc XEN_IMM
+
+ /*
+ * Disable userspace access from kernel once the hyp call completed.
+ */
+ uaccess_ttbr0_disable x6
ret
ENDPROC(privcmd_call);
^ permalink raw reply related
* [PATCH v4 6/8] arm64: Handle faults caused by inadvertent user access with PAN enabled
From: Catalin Marinas @ 2016-10-28 17:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477675636-3957-1-git-send-email-catalin.marinas@arm.com>
When TTBR0_EL1 is set to the reserved page, an erroneous kernel access
to user space would generate a translation fault. This patch adds the
checks for the software-set PSR_PAN_BIT to emulate a permission fault
and report it accordingly.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/mm/fault.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index d035cc594445..a78a5c401806 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -269,13 +269,19 @@ static int __do_page_fault(struct mm_struct *mm, unsigned long addr,
return fault;
}
-static inline bool is_permission_fault(unsigned int esr)
+static inline bool is_permission_fault(unsigned int esr, struct pt_regs *regs)
{
unsigned int ec = ESR_ELx_EC(esr);
unsigned int fsc_type = esr & ESR_ELx_FSC_TYPE;
- return (ec == ESR_ELx_EC_DABT_CUR && fsc_type == ESR_ELx_FSC_PERM) ||
- (ec == ESR_ELx_EC_IABT_CUR && fsc_type == ESR_ELx_FSC_PERM);
+ if (ec != ESR_ELx_EC_DABT_CUR && ec != ESR_ELx_EC_IABT_CUR)
+ return false;
+
+ if (system_uses_ttbr0_pan())
+ return fsc_type == ESR_ELx_FSC_FAULT &&
+ (regs->pstate & PSR_PAN_BIT);
+ else
+ return fsc_type == ESR_ELx_FSC_PERM;
}
static bool is_el0_instruction_abort(unsigned int esr)
@@ -315,7 +321,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
mm_flags |= FAULT_FLAG_WRITE;
}
- if (is_permission_fault(esr) && (addr < USER_DS)) {
+ if (addr < USER_DS && is_permission_fault(esr, regs)) {
/* regs->orig_addr_limit may be 0 if we entered from EL0 */
if (regs->orig_addr_limit == KERNEL_DS)
die("Accessing user space memory with fs=KERNEL_DS", regs, esr);
^ permalink raw reply related
* [PATCH v4 5/8] arm64: Disable TTBR0_EL1 during normal kernel execution
From: Catalin Marinas @ 2016-10-28 17:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477675636-3957-1-git-send-email-catalin.marinas@arm.com>
When the TTBR0 PAN feature is enabled, the kernel entry points need to
disable access to TTBR0_EL1. The PAN status of the interrupted context
is stored as part of the saved pstate, reusing the PSR_PAN_BIT (22).
Restoring access to TTBR0_EL1 is done on exception return if returning
to user or returning to a context where PAN was disabled.
Context switching via switch_mm() must defer the update of TTBR0_EL1
until a return to user or an explicit uaccess_enable() call.
Special care needs to be taken for two cases where TTBR0_EL1 is set
outside the normal kernel context switch operation: EFI run-time
services (via efi_set_pgd) and CPU suspend (via cpu_(un)install_idmap).
Code has been added to avoid deferred TTBR0_EL1 switching as in
switch_mm() and restore the reserved TTBR0_EL1 when uninstalling the
special TTBR0_EL1.
User cache maintenance (user_cache_maint_handler and
__flush_cache_user_range) needs the TTBR0_EL1 re-instated since the
operations are performed by user virtual address.
This patch also removes a stale comment on the switch_mm() function.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/include/asm/efi.h | 26 ++++++++++++++-
arch/arm64/include/asm/mmu_context.h | 53 ++++++++++++++++++++++--------
arch/arm64/kernel/entry.S | 63 ++++++++++++++++++++++++++++++++++++
arch/arm64/kernel/setup.c | 9 ++++++
arch/arm64/kernel/traps.c | 9 ++++--
arch/arm64/mm/cache.S | 6 +++-
arch/arm64/mm/context.c | 7 +++-
7 files changed, 153 insertions(+), 20 deletions(-)
diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index a9e54aad15ef..3a405dccb6cf 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -1,6 +1,7 @@
#ifndef _ASM_EFI_H
#define _ASM_EFI_H
+#include <asm/cpufeature.h>
#include <asm/io.h>
#include <asm/mmu_context.h>
#include <asm/neon.h>
@@ -75,7 +76,30 @@ static inline void efifb_setup_from_dmi(struct screen_info *si, const char *opt)
static inline void efi_set_pgd(struct mm_struct *mm)
{
- switch_mm(NULL, mm, NULL);
+ __switch_mm(mm);
+
+ if (system_uses_ttbr0_pan()) {
+ if (mm != current->active_mm) {
+ /*
+ * Update the current thread's saved ttbr0 since it is
+ * restored as part of a return from exception. Set
+ * the hardware TTBR0_EL1 using cpu_switch_mm()
+ * directly to enable potential errata workarounds.
+ */
+ update_saved_ttbr0(current, mm);
+ cpu_switch_mm(mm->pgd, mm);
+ } else {
+ /*
+ * Defer the switch to the current thread's TTBR0_EL1
+ * until uaccess_enable(). Restore the current
+ * thread's saved ttbr0 corresponding to its active_mm
+ * (if different from init_mm).
+ */
+ cpu_set_reserved_ttbr0();
+ if (current->active_mm != &init_mm)
+ update_saved_ttbr0(current, current->active_mm);
+ }
+ }
}
void efi_virtmap_load(void);
diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
index a50185375f09..0363fe80455c 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -23,6 +23,7 @@
#include <linux/sched.h>
#include <asm/cacheflush.h>
+#include <asm/cpufeature.h>
#include <asm/proc-fns.h>
#include <asm-generic/mm_hooks.h>
#include <asm/cputype.h>
@@ -103,7 +104,7 @@ static inline void cpu_uninstall_idmap(void)
local_flush_tlb_all();
cpu_set_default_tcr_t0sz();
- if (mm != &init_mm)
+ if (mm != &init_mm && !system_uses_ttbr0_pan())
cpu_switch_mm(mm->pgd, mm);
}
@@ -163,20 +164,26 @@ enter_lazy_tlb(struct mm_struct *mm, struct task_struct *tsk)
{
}
-/*
- * This is the actual mm switch as far as the scheduler
- * is concerned. No registers are touched. We avoid
- * calling the CPU specific function when the mm hasn't
- * actually changed.
- */
-static inline void
-switch_mm(struct mm_struct *prev, struct mm_struct *next,
- struct task_struct *tsk)
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+static inline void update_saved_ttbr0(struct task_struct *tsk,
+ struct mm_struct *mm)
{
- unsigned int cpu = smp_processor_id();
+ if (system_uses_ttbr0_pan()) {
+ BUG_ON(mm->pgd == swapper_pg_dir);
+ task_thread_info(tsk)->ttbr0 =
+ virt_to_phys(mm->pgd) | ASID(mm) << 48;
+ }
+}
+#else
+static inline void update_saved_ttbr0(struct task_struct *tsk,
+ struct mm_struct *mm)
+{
+}
+#endif
- if (prev == next)
- return;
+static inline void __switch_mm(struct mm_struct *next)
+{
+ unsigned int cpu = smp_processor_id();
/*
* init_mm.pgd does not contain any user mappings and it is always
@@ -190,8 +197,26 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
check_and_switch_context(next, cpu);
}
+static inline void
+switch_mm(struct mm_struct *prev, struct mm_struct *next,
+ struct task_struct *tsk)
+{
+ if (prev != next)
+ __switch_mm(next);
+
+ /*
+ * Update the saved TTBR0_EL1 of the scheduled-in task as the previous
+ * value may have not been initialised yet (activate_mm caller) or the
+ * ASID has changed since the last run (following the context switch
+ * of another thread of the same process). Avoid setting the reserved
+ * TTBR0_EL1 to swapper_pg_dir (init_mm; e.g. via idle_task_exit).
+ */
+ if (next != &init_mm)
+ update_saved_ttbr0(tsk, next);
+}
+
#define deactivate_mm(tsk,mm) do { } while (0)
-#define activate_mm(prev,next) switch_mm(prev, next, NULL)
+#define activate_mm(prev,next) switch_mm(prev, next, current)
void verify_cpu_asid_bits(void);
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 099e43d1adb5..6869db9cfec9 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -29,7 +29,9 @@
#include <asm/esr.h>
#include <asm/irq.h>
#include <asm/memory.h>
+#include <asm/ptrace.h>
#include <asm/thread_info.h>
+#include <asm/uaccess.h>
#include <asm/unistd.h>
/*
@@ -109,6 +111,32 @@
mrs x22, elr_el1
mrs x23, spsr_el1
stp lr, x21, [sp, #S_LR]
+
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+ /*
+ * Set the TTBR0 PAN bit in SPSR. When the exception is taken from
+ * EL0, there is no need to check the state of TTBR0_EL1 since
+ * accesses are always enabled.
+ * Note that the meaning of this bit differs from the ARMv8.1 PAN
+ * feature as all TTBR0_EL1 accesses are disabled, not just those to
+ * user mappings.
+ */
+alternative_if ARM64_HAS_PAN
+ b 1f // skip TTBR0 PAN
+alternative_else_nop_endif
+
+ .if \el != 0
+ mrs x21, ttbr0_el1
+ tst x21, #0xffff << 48 // Check for the reserved ASID
+ orr x23, x23, #PSR_PAN_BIT // Set the emulated PAN in the saved SPSR
+ b.eq 1f // TTBR0 access already disabled
+ and x23, x23, #~PSR_PAN_BIT // Clear the emulated PAN in the saved SPSR
+ .endif
+
+ __uaccess_ttbr0_disable x21
+1:
+#endif
+
stp x22, x23, [sp, #S_PC]
/*
@@ -147,6 +175,40 @@
ldp x21, x22, [sp, #S_PC] // load ELR, SPSR
.if \el == 0
ct_user_enter
+ .endif
+
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+ /*
+ * Restore access to TTBR0_EL1. If returning to EL0, no need for SPSR
+ * PAN bit checking.
+ */
+alternative_if ARM64_HAS_PAN
+ b 2f // skip TTBR0 PAN
+alternative_else_nop_endif
+
+ .if \el != 0
+ tbnz x22, #22, 1f // Skip re-enabling TTBR0 access if the PSR_PAN_BIT is set
+ .endif
+
+ __uaccess_ttbr0_enable x0
+
+ .if \el == 0
+ /*
+ * Enable errata workarounds only if returning to user. The only
+ * workaround currently required for TTBR0_EL1 changes are for the
+ * Cavium erratum 27456 (broadcast TLBI instructions may cause I-cache
+ * corruption).
+ */
+ post_ttbr0_update_workaround
+ .endif
+1:
+ .if \el != 0
+ and x22, x22, #~PSR_PAN_BIT // ARMv8.0 CPUs do not understand this bit
+ .endif
+2:
+#endif
+
+ .if \el == 0
ldr x23, [sp, #S_SP] // load return stack pointer
msr sp_el0, x23
#ifdef CONFIG_ARM64_ERRATUM_845719
@@ -162,6 +224,7 @@ alternative_if ARM64_WORKAROUND_845719
alternative_else_nop_endif
#endif
.endif
+
msr elr_el1, x21 // set up the return data
msr spsr_el1, x22
ldp x0, x1, [sp, #16 * 0]
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index f534f492a268..f7545a7f6f29 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -291,6 +291,15 @@ void __init setup_arch(char **cmdline_p)
smp_init_cpus();
smp_build_mpidr_hash();
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+ /*
+ * Make sure init_thread_info.ttbr0 always generates translation
+ * faults in case uaccess_enable() is inadvertently called by the init
+ * thread.
+ */
+ init_thread_info.ttbr0 = virt_to_phys(empty_zero_page);
+#endif
+
#ifdef CONFIG_VT
#if defined(CONFIG_VGA_CONSOLE)
conswitchp = &vga_con;
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index c9986b3e0a96..827c02d042a0 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -435,9 +435,10 @@ int cpu_enable_cache_maint_trap(void *__unused)
}
#define __user_cache_maint(insn, address, res) \
- if (untagged_addr(address) >= user_addr_max()) \
+ if (untagged_addr(address) >= user_addr_max()) { \
res = -EFAULT; \
- else \
+ } else { \
+ uaccess_ttbr0_enable(); \
asm volatile ( \
"1: " insn ", %1\n" \
" mov %w0, #0\n" \
@@ -449,7 +450,9 @@ int cpu_enable_cache_maint_trap(void *__unused)
" .popsection\n" \
_ASM_EXTABLE(1b, 3b) \
: "=r" (res) \
- : "r" (address), "i" (-EFAULT) )
+ : "r" (address), "i" (-EFAULT)); \
+ uaccess_ttbr0_disable(); \
+ }
static void user_cache_maint_handler(unsigned int esr, struct pt_regs *regs)
{
diff --git a/arch/arm64/mm/cache.S b/arch/arm64/mm/cache.S
index 58b5a906ff78..da9576932322 100644
--- a/arch/arm64/mm/cache.S
+++ b/arch/arm64/mm/cache.S
@@ -23,6 +23,7 @@
#include <asm/assembler.h>
#include <asm/cpufeature.h>
#include <asm/alternative.h>
+#include <asm/uaccess.h>
/*
* flush_icache_range(start,end)
@@ -48,6 +49,7 @@ ENTRY(flush_icache_range)
* - end - virtual end address of region
*/
ENTRY(__flush_cache_user_range)
+ uaccess_ttbr0_enable x2, x3
dcache_line_size x2, x3
sub x3, x2, #1
bic x4, x0, x3
@@ -69,10 +71,12 @@ USER(9f, ic ivau, x4 ) // invalidate I line PoU
dsb ish
isb
mov x0, #0
+1:
+ uaccess_ttbr0_disable x1
ret
9:
mov x0, #-EFAULT
- ret
+ b 1b
ENDPROC(flush_icache_range)
ENDPROC(__flush_cache_user_range)
diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
index efcf1f7ef1e4..4c63cb154859 100644
--- a/arch/arm64/mm/context.c
+++ b/arch/arm64/mm/context.c
@@ -221,7 +221,12 @@ void check_and_switch_context(struct mm_struct *mm, unsigned int cpu)
raw_spin_unlock_irqrestore(&cpu_asid_lock, flags);
switch_mm_fastpath:
- cpu_switch_mm(mm->pgd, mm);
+ /*
+ * Defer TTBR0_EL1 setting for user threads to uaccess_enable() when
+ * emulating PAN.
+ */
+ if (!system_uses_ttbr0_pan())
+ cpu_switch_mm(mm->pgd, mm);
}
static int asids_init(void)
^ permalink raw reply related
* [PATCH v4 4/8] arm64: Introduce uaccess_{disable, enable} functionality based on TTBR0_EL1
From: Catalin Marinas @ 2016-10-28 17:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477675636-3957-1-git-send-email-catalin.marinas@arm.com>
This patch adds the uaccess macros/functions to disable access to user
space by setting TTBR0_EL1 to a reserved zeroed page. Since the value
written to TTBR0_EL1 must be a physical address, for simplicity this
patch introduces a reserved_ttbr0 page at a constant offset from
swapper_pg_dir. The uaccess_disable code uses the ttbr1_el1 value
adjusted by the reserved_ttbr0 offset.
Enabling access to user is done by restoring TTBR0_EL1 with the value
from the struct thread_info ttbr0 variable. Interrupts must be disabled
during the uaccess_ttbr0_enable code to ensure the atomicity of the
thread_info.ttbr0 read and TTBR0_EL1 write. This patch also moves the
get_thread_info asm macro from entry.S to assembler.h for reuse in the
uaccess_ttbr0_* macros.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/include/asm/assembler.h | 16 +++++
arch/arm64/include/asm/cpufeature.h | 6 ++
arch/arm64/include/asm/kernel-pgtable.h | 7 +++
arch/arm64/include/asm/thread_info.h | 3 +
arch/arm64/include/asm/uaccess.h | 108 ++++++++++++++++++++++++++++++--
arch/arm64/kernel/asm-offsets.c | 3 +
arch/arm64/kernel/cpufeature.c | 1 +
arch/arm64/kernel/entry.S | 4 --
arch/arm64/kernel/head.S | 6 +-
arch/arm64/kernel/vmlinux.lds.S | 5 ++
10 files changed, 146 insertions(+), 13 deletions(-)
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index ab87006ff2fb..359d9d268dac 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -41,6 +41,15 @@
msr daifclr, #2
.endm
+ .macro save_and_disable_irq, flags
+ mrs \flags, daif
+ msr daifset, #2
+ .endm
+
+ .macro restore_irq, flags
+ msr daif, \flags
+ .endm
+
/*
* Enable and disable debug exceptions.
*/
@@ -396,6 +405,13 @@ alternative_endif
.endm
/*
+ * Return the current thread_info.
+ */
+ .macro get_thread_info, rd
+ mrs \rd, sp_el0
+ .endm
+
+/*
* Errata workaround post TTBR0_EL1 update.
*/
.macro post_ttbr0_update_workaround
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index a27c3245ba21..e5cfb73f65e1 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -226,6 +226,12 @@ static inline bool system_supports_mixed_endian_el0(void)
return id_aa64mmfr0_mixed_endian_el0(read_system_reg(SYS_ID_AA64MMFR0_EL1));
}
+static inline bool system_uses_ttbr0_pan(void)
+{
+ return IS_ENABLED(CONFIG_ARM64_SW_TTBR0_PAN) &&
+ !cpus_have_cap(ARM64_HAS_PAN);
+}
+
#endif /* __ASSEMBLY__ */
#endif
diff --git a/arch/arm64/include/asm/kernel-pgtable.h b/arch/arm64/include/asm/kernel-pgtable.h
index 7e51d1b57c0c..7803343e5881 100644
--- a/arch/arm64/include/asm/kernel-pgtable.h
+++ b/arch/arm64/include/asm/kernel-pgtable.h
@@ -19,6 +19,7 @@
#ifndef __ASM_KERNEL_PGTABLE_H
#define __ASM_KERNEL_PGTABLE_H
+#include <asm/pgtable.h>
#include <asm/sparsemem.h>
/*
@@ -54,6 +55,12 @@
#define SWAPPER_DIR_SIZE (SWAPPER_PGTABLE_LEVELS * PAGE_SIZE)
#define IDMAP_DIR_SIZE (IDMAP_PGTABLE_LEVELS * PAGE_SIZE)
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+#define RESERVED_TTBR0_SIZE (PAGE_SIZE)
+#else
+#define RESERVED_TTBR0_SIZE (0)
+#endif
+
/* Initial memory map size */
#if ARM64_SWAPPER_USES_SECTION_MAPS
#define SWAPPER_BLOCK_SHIFT SECTION_SHIFT
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index e9ea5a6bd449..33e454e4b94b 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -47,6 +47,9 @@ typedef unsigned long mm_segment_t;
struct thread_info {
unsigned long flags; /* low level flags */
mm_segment_t addr_limit; /* address limit */
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+ u64 ttbr0; /* saved TTBR0_EL1 */
+#endif
struct task_struct *task; /* main task structure */
int preempt_count; /* 0 => preemptable, <0 => bug */
int cpu; /* cpu */
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 154659509afb..1b7274f77cbe 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -19,6 +19,7 @@
#define __ASM_UACCESS_H
#include <asm/alternative.h>
+#include <asm/kernel-pgtable.h>
#include <asm/sysreg.h>
#ifndef __ASSEMBLY__
@@ -125,16 +126,71 @@ static inline void set_fs(mm_segment_t fs)
/*
* User access enabling/disabling.
*/
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+static inline void __uaccess_ttbr0_disable(void)
+{
+ unsigned long ttbr;
+
+ /* reserved_ttbr0 placed at the end of swapper_pg_dir */
+ ttbr = read_sysreg(ttbr1_el1) + SWAPPER_DIR_SIZE;
+ write_sysreg(ttbr, ttbr0_el1);
+ isb();
+}
+
+static inline void __uaccess_ttbr0_enable(void)
+{
+ unsigned long flags;
+
+ /*
+ * Disable interrupts to avoid preemption between reading the 'ttbr0'
+ * variable and the MSR. A context switch could trigger an ASID
+ * roll-over and an update of 'ttbr0'.
+ */
+ local_irq_save(flags);
+ write_sysreg(current_thread_info()->ttbr0, ttbr0_el1);
+ isb();
+ local_irq_restore(flags);
+}
+
+static inline bool uaccess_ttbr0_disable(void)
+{
+ if (!system_uses_ttbr0_pan())
+ return false;
+ __uaccess_ttbr0_disable();
+ return true;
+}
+
+static inline bool uaccess_ttbr0_enable(void)
+{
+ if (!system_uses_ttbr0_pan())
+ return false;
+ __uaccess_ttbr0_enable();
+ return true;
+}
+#else
+static inline bool uaccess_ttbr0_disable(void)
+{
+ return false;
+}
+
+static inline bool uaccess_ttbr0_enable(void)
+{
+ return false;
+}
+#endif
+
#define __uaccess_disable(alt) \
do { \
- asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), alt, \
- CONFIG_ARM64_PAN)); \
+ if (!uaccess_ttbr0_disable()) \
+ asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), alt, \
+ CONFIG_ARM64_PAN)); \
} while (0)
#define __uaccess_enable(alt) \
do { \
- asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
- CONFIG_ARM64_PAN)); \
+ if (uaccess_ttbr0_enable()) \
+ asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
+ CONFIG_ARM64_PAN)); \
} while (0)
static inline void uaccess_disable(void)
@@ -373,16 +429,56 @@ extern __must_check long strnlen_user(const char __user *str, long n);
#include <asm/assembler.h>
/*
- * User access enabling/disabling macros. These are no-ops when UAO is
- * present.
+ * User access enabling/disabling macros.
+ */
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+ .macro __uaccess_ttbr0_disable, tmp1
+ mrs \tmp1, ttbr1_el1 // swapper_pg_dir
+ add \tmp1, \tmp1, #SWAPPER_DIR_SIZE // reserved_ttbr0 at the end of swapper_pg_dir
+ msr ttbr0_el1, \tmp1 // set reserved TTBR0_EL1
+ isb
+ .endm
+
+ .macro __uaccess_ttbr0_enable, tmp1
+ get_thread_info \tmp1
+ ldr \tmp1, [\tmp1, #TI_TTBR0] // load saved TTBR0_EL1
+ msr ttbr0_el1, \tmp1 // set the non-PAN TTBR0_EL1
+ isb
+ .endm
+
+ .macro uaccess_ttbr0_disable, tmp1
+alternative_if_not ARM64_HAS_PAN
+ __uaccess_ttbr0_disable \tmp1
+alternative_else_nop_endif
+ .endm
+
+ .macro uaccess_ttbr0_enable, tmp1, tmp2
+alternative_if_not ARM64_HAS_PAN
+ save_and_disable_irq \tmp2 // avoid preemption
+ __uaccess_ttbr0_enable \tmp1
+ restore_irq \tmp2
+alternative_else_nop_endif
+ .endm
+#else
+ .macro uaccess_ttbr0_disable, tmp1
+ .endm
+
+ .macro uaccess_ttbr0_enable, tmp1, tmp2
+ .endm
+#endif
+
+/*
+ * These macros are no-ops when UAO is present.
*/
.macro uaccess_disable_not_uao, tmp1
+ uaccess_ttbr0_disable \tmp1
alternative_if ARM64_ALT_PAN_NOT_UAO
SET_PSTATE_PAN(1)
alternative_else_nop_endif
.endm
.macro uaccess_enable_not_uao, tmp1, tmp2
+ uaccess_ttbr0_enable \tmp1, \tmp2
alternative_if ARM64_ALT_PAN_NOT_UAO
SET_PSTATE_PAN(0)
alternative_else_nop_endif
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 4a2f0f0fef32..e55532111c68 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -39,6 +39,9 @@ int main(void)
DEFINE(TI_FLAGS, offsetof(struct thread_info, flags));
DEFINE(TI_PREEMPT, offsetof(struct thread_info, preempt_count));
DEFINE(TI_ADDR_LIMIT, offsetof(struct thread_info, addr_limit));
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+ DEFINE(TI_TTBR0, offsetof(struct thread_info, ttbr0));
+#endif
DEFINE(TI_TASK, offsetof(struct thread_info, task));
DEFINE(TI_CPU, offsetof(struct thread_info, cpu));
BLANK();
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index c02504ea304b..debae34a19b0 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -47,6 +47,7 @@ unsigned int compat_elf_hwcap2 __read_mostly;
#endif
DECLARE_BITMAP(cpu_hwcaps, ARM64_NCAPS);
+EXPORT_SYMBOL(cpu_hwcaps);
DEFINE_STATIC_KEY_ARRAY_FALSE(cpu_hwcap_keys, ARM64_NCAPS);
EXPORT_SYMBOL(cpu_hwcap_keys);
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 223d54a4d66b..099e43d1adb5 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -184,10 +184,6 @@ alternative_else_nop_endif
eret // return to kernel
.endm
- .macro get_thread_info, rd
- mrs \rd, sp_el0
- .endm
-
.macro irq_stack_entry
mov x19, sp // preserve the original sp
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 332e33193ccf..c7d26bb87a9a 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -326,14 +326,14 @@ __create_page_tables:
* dirty cache lines being evicted.
*/
adrp x0, idmap_pg_dir
- adrp x1, swapper_pg_dir + SWAPPER_DIR_SIZE
+ adrp x1, swapper_pg_dir + SWAPPER_DIR_SIZE + RESERVED_TTBR0_SIZE
bl __inval_cache_range
/*
* Clear the idmap and swapper page tables.
*/
adrp x0, idmap_pg_dir
- adrp x6, swapper_pg_dir + SWAPPER_DIR_SIZE
+ adrp x6, swapper_pg_dir + SWAPPER_DIR_SIZE + RESERVED_TTBR0_SIZE
1: stp xzr, xzr, [x0], #16
stp xzr, xzr, [x0], #16
stp xzr, xzr, [x0], #16
@@ -412,7 +412,7 @@ __create_page_tables:
* tables again to remove any speculatively loaded cache lines.
*/
adrp x0, idmap_pg_dir
- adrp x1, swapper_pg_dir + SWAPPER_DIR_SIZE
+ adrp x1, swapper_pg_dir + SWAPPER_DIR_SIZE + RESERVED_TTBR0_SIZE
dmb sy
bl __inval_cache_range
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index 1105aab1e6d6..b8deffa9e1bf 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -216,6 +216,11 @@ SECTIONS
swapper_pg_dir = .;
. += SWAPPER_DIR_SIZE;
+#ifdef CONFIG_ARM64_SW_TTBR0_PAN
+ reserved_ttbr0 = .;
+ . += RESERVED_TTBR0_SIZE;
+#endif
+
_end = .;
STABS_DEBUG
^ permalink raw reply related
* [PATCH v4 3/8] arm64: Factor out TTBR0_EL1 post-update workaround into a specific asm macro
From: Catalin Marinas @ 2016-10-28 17:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477675636-3957-1-git-send-email-catalin.marinas@arm.com>
This patch takes the errata workaround code out of cpu_do_switch_mm into
a dedicated post_ttbr0_update_workaround macro which will be reused in a
subsequent patch.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/include/asm/assembler.h | 13 +++++++++++++
arch/arm64/mm/proc.S | 6 +-----
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index 28bfe6132eb6..ab87006ff2fb 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -395,4 +395,17 @@ alternative_endif
movk \reg, :abs_g0_nc:\val
.endm
+/*
+ * Errata workaround post TTBR0_EL1 update.
+ */
+ .macro post_ttbr0_update_workaround
+#ifdef CONFIG_CAVIUM_ERRATUM_27456
+alternative_if ARM64_WORKAROUND_CAVIUM_27456
+ ic iallu
+ dsb nsh
+ isb
+alternative_else_nop_endif
+#endif
+ .endm
+
#endif /* __ASM_ASSEMBLER_H */
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 352c73b6a59e..c2adb0cb952a 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -136,11 +136,7 @@ ENTRY(cpu_do_switch_mm)
bfi x0, x1, #48, #16 // set the ASID
msr ttbr0_el1, x0 // set TTBR0
isb
-alternative_if ARM64_WORKAROUND_CAVIUM_27456
- ic iallu
- dsb nsh
- isb
-alternative_else_nop_endif
+ post_ttbr0_update_workaround
ret
ENDPROC(cpu_do_switch_mm)
^ permalink raw reply related
* [PATCH v4 2/8] arm64: Factor out PAN enabling/disabling into separate uaccess_* macros
From: Catalin Marinas @ 2016-10-28 17:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477675636-3957-1-git-send-email-catalin.marinas@arm.com>
This patch moves the directly coded alternatives for turning PAN on/off
into separate uaccess_{enable,disable} macros or functions. The asm
macros take a few arguments which will be used in subsequent patches.
Note that any (unlikely) access that the compiler might generate between
uaccess_enable() and uaccess_disable(), other than those explicitly
specified by the user access code, will not be protected by PAN.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/include/asm/futex.h | 17 ++++----
arch/arm64/include/asm/uaccess.h | 79 +++++++++++++++++++++++++++++++-----
arch/arm64/kernel/armv8_deprecated.c | 11 +++--
arch/arm64/lib/clear_user.S | 11 ++---
arch/arm64/lib/copy_from_user.S | 11 ++---
arch/arm64/lib/copy_in_user.S | 11 ++---
arch/arm64/lib/copy_to_user.S | 11 ++---
7 files changed, 93 insertions(+), 58 deletions(-)
diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
index f2585cdd32c2..85c4a8981d47 100644
--- a/arch/arm64/include/asm/futex.h
+++ b/arch/arm64/include/asm/futex.h
@@ -21,15 +21,12 @@
#include <linux/futex.h>
#include <linux/uaccess.h>
-#include <asm/alternative.h>
-#include <asm/cpufeature.h>
#include <asm/errno.h>
-#include <asm/sysreg.h>
#define __futex_atomic_op(insn, ret, oldval, uaddr, tmp, oparg) \
+do { \
+ uaccess_enable(); \
asm volatile( \
- ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN, \
- CONFIG_ARM64_PAN) \
" prfm pstl1strm, %2\n" \
"1: ldxr %w1, %2\n" \
insn "\n" \
@@ -44,11 +41,11 @@
" .popsection\n" \
_ASM_EXTABLE(1b, 4b) \
_ASM_EXTABLE(2b, 4b) \
- ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_HAS_PAN, \
- CONFIG_ARM64_PAN) \
: "=&r" (ret), "=&r" (oldval), "+Q" (*uaddr), "=&r" (tmp) \
: "r" (oparg), "Ir" (-EFAULT) \
- : "memory")
+ : "memory"); \
+ uaccess_disable(); \
+} while (0)
static inline int
futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr)
@@ -118,8 +115,8 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
return -EFAULT;
+ uaccess_enable();
asm volatile("// futex_atomic_cmpxchg_inatomic\n"
-ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN, CONFIG_ARM64_PAN)
" prfm pstl1strm, %2\n"
"1: ldxr %w1, %2\n"
" sub %w3, %w1, %w4\n"
@@ -134,10 +131,10 @@ ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN, CONFIG_ARM64_PAN)
" .popsection\n"
_ASM_EXTABLE(1b, 4b)
_ASM_EXTABLE(2b, 4b)
-ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_HAS_PAN, CONFIG_ARM64_PAN)
: "+r" (ret), "=&r" (val), "+Q" (*uaddr), "=&r" (tmp)
: "r" (oldval), "r" (newval), "Ir" (-EFAULT)
: "memory");
+ uaccess_disable();
*uval = val;
return ret;
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 55d0adbf6509..154659509afb 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -18,6 +18,11 @@
#ifndef __ASM_UACCESS_H
#define __ASM_UACCESS_H
+#include <asm/alternative.h>
+#include <asm/sysreg.h>
+
+#ifndef __ASSEMBLY__
+
/*
* User space memory access functions
*/
@@ -26,10 +31,8 @@
#include <linux/string.h>
#include <linux/thread_info.h>
-#include <asm/alternative.h>
#include <asm/cpufeature.h>
#include <asm/ptrace.h>
-#include <asm/sysreg.h>
#include <asm/errno.h>
#include <asm/memory.h>
#include <asm/compiler.h>
@@ -120,6 +123,44 @@ static inline void set_fs(mm_segment_t fs)
" .popsection\n"
/*
+ * User access enabling/disabling.
+ */
+#define __uaccess_disable(alt) \
+do { \
+ asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), alt, \
+ CONFIG_ARM64_PAN)); \
+} while (0)
+
+#define __uaccess_enable(alt) \
+do { \
+ asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
+ CONFIG_ARM64_PAN)); \
+} while (0)
+
+static inline void uaccess_disable(void)
+{
+ __uaccess_disable(ARM64_HAS_PAN);
+}
+
+static inline void uaccess_enable(void)
+{
+ __uaccess_enable(ARM64_HAS_PAN);
+}
+
+/*
+ * These functions are no-ops when UAO is present.
+ */
+static inline void uaccess_disable_not_uao(void)
+{
+ __uaccess_disable(ARM64_ALT_PAN_NOT_UAO);
+}
+
+static inline void uaccess_enable_not_uao(void)
+{
+ __uaccess_enable(ARM64_ALT_PAN_NOT_UAO);
+}
+
+/*
* The "__xxx" versions of the user access functions do not verify the address
* space - it must have been done previously with a separate "access_ok()"
* call.
@@ -146,8 +187,7 @@ static inline void set_fs(mm_segment_t fs)
do { \
unsigned long __gu_val; \
__chk_user_ptr(ptr); \
- asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_ALT_PAN_NOT_UAO,\
- CONFIG_ARM64_PAN)); \
+ uaccess_enable_not_uao(); \
switch (sizeof(*(ptr))) { \
case 1: \
__get_user_asm("ldrb", "ldtrb", "%w", __gu_val, (ptr), \
@@ -168,9 +208,8 @@ do { \
default: \
BUILD_BUG(); \
} \
+ uaccess_disable_not_uao(); \
(x) = (__force __typeof__(*(ptr)))__gu_val; \
- asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_ALT_PAN_NOT_UAO,\
- CONFIG_ARM64_PAN)); \
} while (0)
#define __get_user(x, ptr) \
@@ -215,8 +254,7 @@ do { \
do { \
__typeof__(*(ptr)) __pu_val = (x); \
__chk_user_ptr(ptr); \
- asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_ALT_PAN_NOT_UAO,\
- CONFIG_ARM64_PAN)); \
+ uaccess_enable_not_uao(); \
switch (sizeof(*(ptr))) { \
case 1: \
__put_user_asm("strb", "sttrb", "%w", __pu_val, (ptr), \
@@ -237,8 +275,7 @@ do { \
default: \
BUILD_BUG(); \
} \
- asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_ALT_PAN_NOT_UAO,\
- CONFIG_ARM64_PAN)); \
+ uaccess_disable_not_uao(); \
} while (0)
#define __put_user(x, ptr) \
@@ -331,4 +368,26 @@ extern long strncpy_from_user(char *dest, const char __user *src, long count);
extern __must_check long strlen_user(const char __user *str);
extern __must_check long strnlen_user(const char __user *str, long n);
+#else /* __ASSEMBLY__ */
+
+#include <asm/assembler.h>
+
+/*
+ * User access enabling/disabling macros. These are no-ops when UAO is
+ * present.
+ */
+ .macro uaccess_disable_not_uao, tmp1
+alternative_if ARM64_ALT_PAN_NOT_UAO
+ SET_PSTATE_PAN(1)
+alternative_else_nop_endif
+ .endm
+
+ .macro uaccess_enable_not_uao, tmp1, tmp2
+alternative_if ARM64_ALT_PAN_NOT_UAO
+ SET_PSTATE_PAN(0)
+alternative_else_nop_endif
+ .endm
+
+#endif /* __ASSEMBLY__ */
+
#endif /* __ASM_UACCESS_H */
diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index b0988bb1bf64..bdb35b92003e 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -14,7 +14,6 @@
#include <linux/slab.h>
#include <linux/sysctl.h>
-#include <asm/alternative.h>
#include <asm/cpufeature.h>
#include <asm/insn.h>
#include <asm/opcodes.h>
@@ -285,10 +284,10 @@ static void __init register_insn_emulation_sysctl(struct ctl_table *table)
#define __SWP_LL_SC_LOOPS 4
#define __user_swpX_asm(data, addr, res, temp, temp2, B) \
+do { \
+ uaccess_enable(); \
__asm__ __volatile__( \
" mov %w3, %w7\n" \
- ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN, \
- CONFIG_ARM64_PAN) \
"0: ldxr"B" %w2, [%4]\n" \
"1: stxr"B" %w0, %w1, [%4]\n" \
" cbz %w0, 2f\n" \
@@ -306,12 +305,12 @@ static void __init register_insn_emulation_sysctl(struct ctl_table *table)
" .popsection" \
_ASM_EXTABLE(0b, 4b) \
_ASM_EXTABLE(1b, 4b) \
- ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_HAS_PAN, \
- CONFIG_ARM64_PAN) \
: "=&r" (res), "+r" (data), "=&r" (temp), "=&r" (temp2) \
: "r" (addr), "i" (-EAGAIN), "i" (-EFAULT), \
"i" (__SWP_LL_SC_LOOPS) \
- : "memory")
+ : "memory"); \
+ uaccess_disable(); \
+} while (0)
#define __user_swp_asm(data, addr, res, temp, temp2) \
__user_swpX_asm(data, addr, res, temp, temp2, "")
diff --git a/arch/arm64/lib/clear_user.S b/arch/arm64/lib/clear_user.S
index 5d1cad3ce6d6..d7150e30438a 100644
--- a/arch/arm64/lib/clear_user.S
+++ b/arch/arm64/lib/clear_user.S
@@ -17,10 +17,7 @@
*/
#include <linux/linkage.h>
-#include <asm/alternative.h>
-#include <asm/assembler.h>
-#include <asm/cpufeature.h>
-#include <asm/sysreg.h>
+#include <asm/uaccess.h>
.text
@@ -33,8 +30,7 @@
* Alignment fixed up by hardware.
*/
ENTRY(__clear_user)
-ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(0)), ARM64_ALT_PAN_NOT_UAO, \
- CONFIG_ARM64_PAN)
+ uaccess_enable_not_uao x2, x3
mov x2, x1 // save the size for fixup return
subs x1, x1, #8
b.mi 2f
@@ -54,8 +50,7 @@ uao_user_alternative 9f, strh, sttrh, wzr, x0, 2
b.mi 5f
uao_user_alternative 9f, strb, sttrb, wzr, x0, 0
5: mov x0, #0
-ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(1)), ARM64_ALT_PAN_NOT_UAO, \
- CONFIG_ARM64_PAN)
+ uaccess_disable_not_uao x2
ret
ENDPROC(__clear_user)
diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S
index 4fd67ea03bb0..cfe13396085b 100644
--- a/arch/arm64/lib/copy_from_user.S
+++ b/arch/arm64/lib/copy_from_user.S
@@ -16,11 +16,8 @@
#include <linux/linkage.h>
-#include <asm/alternative.h>
-#include <asm/assembler.h>
#include <asm/cache.h>
-#include <asm/cpufeature.h>
-#include <asm/sysreg.h>
+#include <asm/uaccess.h>
/*
* Copy from user space to a kernel buffer (alignment handled by the hardware)
@@ -67,12 +64,10 @@
end .req x5
ENTRY(__arch_copy_from_user)
-ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(0)), ARM64_ALT_PAN_NOT_UAO, \
- CONFIG_ARM64_PAN)
+ uaccess_enable_not_uao x3, x4
add end, x0, x2
#include "copy_template.S"
-ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(1)), ARM64_ALT_PAN_NOT_UAO, \
- CONFIG_ARM64_PAN)
+ uaccess_disable_not_uao x3
mov x0, #0 // Nothing to copy
ret
ENDPROC(__arch_copy_from_user)
diff --git a/arch/arm64/lib/copy_in_user.S b/arch/arm64/lib/copy_in_user.S
index f7292dd08c84..718b1c4e2f85 100644
--- a/arch/arm64/lib/copy_in_user.S
+++ b/arch/arm64/lib/copy_in_user.S
@@ -18,11 +18,8 @@
#include <linux/linkage.h>
-#include <asm/alternative.h>
-#include <asm/assembler.h>
#include <asm/cache.h>
-#include <asm/cpufeature.h>
-#include <asm/sysreg.h>
+#include <asm/uaccess.h>
/*
* Copy from user space to user space (alignment handled by the hardware)
@@ -68,12 +65,10 @@
end .req x5
ENTRY(__copy_in_user)
-ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(0)), ARM64_ALT_PAN_NOT_UAO, \
- CONFIG_ARM64_PAN)
+ uaccess_enable_not_uao x3, x4
add end, x0, x2
#include "copy_template.S"
-ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(1)), ARM64_ALT_PAN_NOT_UAO, \
- CONFIG_ARM64_PAN)
+ uaccess_disable_not_uao x3
mov x0, #0
ret
ENDPROC(__copy_in_user)
diff --git a/arch/arm64/lib/copy_to_user.S b/arch/arm64/lib/copy_to_user.S
index 7a7efe255034..e99e31c9acac 100644
--- a/arch/arm64/lib/copy_to_user.S
+++ b/arch/arm64/lib/copy_to_user.S
@@ -16,11 +16,8 @@
#include <linux/linkage.h>
-#include <asm/alternative.h>
-#include <asm/assembler.h>
#include <asm/cache.h>
-#include <asm/cpufeature.h>
-#include <asm/sysreg.h>
+#include <asm/uaccess.h>
/*
* Copy to user space from a kernel buffer (alignment handled by the hardware)
@@ -66,12 +63,10 @@
end .req x5
ENTRY(__arch_copy_to_user)
-ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(0)), ARM64_ALT_PAN_NOT_UAO, \
- CONFIG_ARM64_PAN)
+ uaccess_enable_not_uao x3, x4
add end, x0, x2
#include "copy_template.S"
-ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(1)), ARM64_ALT_PAN_NOT_UAO, \
- CONFIG_ARM64_PAN)
+ uaccess_disable_not_uao x3
mov x0, #0
ret
ENDPROC(__arch_copy_to_user)
^ permalink raw reply related
* [PATCH v4 1/8] arm64: Update the synchronous external abort fault description
From: Catalin Marinas @ 2016-10-28 17:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477675636-3957-1-git-send-email-catalin.marinas@arm.com>
This patch updates the description of the synchronous external aborts on
translation table walks.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/mm/fault.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 0f8788374815..d035cc594445 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -507,10 +507,10 @@ static const struct fault_info {
{ do_bad, SIGBUS, 0, "unknown 17" },
{ do_bad, SIGBUS, 0, "unknown 18" },
{ do_bad, SIGBUS, 0, "unknown 19" },
- { do_bad, SIGBUS, 0, "synchronous abort (translation table walk)" },
- { do_bad, SIGBUS, 0, "synchronous abort (translation table walk)" },
- { do_bad, SIGBUS, 0, "synchronous abort (translation table walk)" },
- { do_bad, SIGBUS, 0, "synchronous abort (translation table walk)" },
+ { do_bad, SIGBUS, 0, "synchronous external abort (translation table walk)" },
+ { do_bad, SIGBUS, 0, "synchronous external abort (translation table walk)" },
+ { do_bad, SIGBUS, 0, "synchronous external abort (translation table walk)" },
+ { do_bad, SIGBUS, 0, "synchronous external abort (translation table walk)" },
{ do_bad, SIGBUS, 0, "synchronous parity error" },
{ do_bad, SIGBUS, 0, "unknown 25" },
{ do_bad, SIGBUS, 0, "unknown 26" },
^ permalink raw reply related
* [PATCH v4 0/8] arm64: Privileged Access Never using TTBR0_EL1 switching
From: Catalin Marinas @ 2016-10-28 17:27 UTC (permalink / raw)
To: linux-arm-kernel
This is the fourth version of the arm64 PAN emulation using TTBR0_EL1
switching. The series does not include the empty_zero_page patch from
Ard B. as I didn't have an up to date patch, nor figured out where the
discussion was left.
The patches are available on this branch:
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux ttbr0-pan
Changes since v3:
- Xen hyp call now only enables TTBR0_EL1 if needed without clearing PAN
- Fix for user space cache maintenance trapping/emulation
- Simplified asm macros based on Mark R's "alternative_else_nop_endif"
- Separate patch for fixing the synchronous external abort description
- #includes clean-up
Catalin Marinas (8):
arm64: Update the synchronous external abort fault description
arm64: Factor out PAN enabling/disabling into separate uaccess_*
macros
arm64: Factor out TTBR0_EL1 post-update workaround into a specific asm
macro
arm64: Introduce uaccess_{disable,enable} functionality based on
TTBR0_EL1
arm64: Disable TTBR0_EL1 during normal kernel execution
arm64: Handle faults caused by inadvertent user access with PAN
enabled
arm64: xen: Enable user access before a privcmd hvc call
arm64: Enable CONFIG_ARM64_SW_TTBR0_PAN
arch/arm64/Kconfig | 8 ++
arch/arm64/include/asm/assembler.h | 29 ++++++
arch/arm64/include/asm/cpufeature.h | 6 ++
arch/arm64/include/asm/efi.h | 26 ++++-
arch/arm64/include/asm/futex.h | 17 ++--
arch/arm64/include/asm/kernel-pgtable.h | 7 ++
arch/arm64/include/asm/mmu_context.h | 53 +++++++---
arch/arm64/include/asm/thread_info.h | 3 +
arch/arm64/include/asm/uaccess.h | 175 ++++++++++++++++++++++++++++++--
arch/arm64/kernel/armv8_deprecated.c | 11 +-
arch/arm64/kernel/asm-offsets.c | 3 +
arch/arm64/kernel/cpufeature.c | 1 +
arch/arm64/kernel/entry.S | 67 +++++++++++-
arch/arm64/kernel/head.S | 6 +-
arch/arm64/kernel/setup.c | 9 ++
arch/arm64/kernel/traps.c | 9 +-
arch/arm64/kernel/vmlinux.lds.S | 5 +
arch/arm64/lib/clear_user.S | 11 +-
arch/arm64/lib/copy_from_user.S | 11 +-
arch/arm64/lib/copy_in_user.S | 11 +-
arch/arm64/lib/copy_to_user.S | 11 +-
arch/arm64/mm/cache.S | 6 +-
arch/arm64/mm/context.c | 7 +-
arch/arm64/mm/fault.c | 22 ++--
arch/arm64/mm/proc.S | 6 +-
arch/arm64/xen/hypercall.S | 15 +++
26 files changed, 437 insertions(+), 98 deletions(-)
^ permalink raw reply
* [RFC][PATCH] arm64: Add support for CONFIG_DEBUG_VIRTUAL
From: Laura Abbott @ 2016-10-28 17:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu9-Pm_j-3shoiGZY+8zMBjTpNOXOphwea+tdC4KPbie3Q@mail.gmail.com>
On 10/28/2016 12:52 AM, Ard Biesheuvel wrote:
> Hi Laura,
>
> On 28 October 2016 at 01:18, Laura Abbott <labbott@redhat.com> wrote:
>> x86 has an option CONFIG_DEBUG_VIRTUAL to do additional checks
>> on virt_to_phys calls. The goal is to catch users who are calling
>> virt_to_phys on non-linear addresses immediately. As features
>> such as CONFIG_VMAP_STACK get enabled for arm64, this becomes
>> increasingly important. Add checks to catch bad virt_to_phys
>> usage.
>>
>
> I think this is a useful thing to have. However, the Kconfig
> description talks about virt to page translations, not virt to phys.
> Of course, this is a shift away from being equivalent on x86, but not
> so much on arm64. Any concerns there?
>
I think the description is vague. I'm guessing there were problems
with virt_to_page but the actual check ends up going in virt_to_phys
because that's where the actual problem occurs.
>> Signed-off-by: Laura Abbott <labbott@redhat.com>
>> ---
>> This has been on my TODO list for a while. It caught a few bugs with
>> CONFIG_VMAP_STACK on x86 so when that eventually gets added
>> for arm64 it will be useful to have. This caught one driver calling __pa on an
>> ioremapped address already. RFC for a couple of reasons:
>>
>> 1) This is basically a direct port of the x86 approach.
>> 2) I needed some #ifndef __ASSEMBLY__ which I don't like to throw around.
>> 3) I'm not quite sure about the bounds check for the VIRTUAL_BUG_ON with KASLR,
>> specifically the _end check.
>> 4) Is it worth actually keeping this as DEBUG_VIRTUAL vs. folding it into
>> another option?
>>
>> ---
>> arch/arm64/include/asm/memory.h | 11 ++++++++++-
>> arch/arm64/mm/Makefile | 2 +-
>> arch/arm64/mm/physaddr.c | 17 +++++++++++++++++
>> lib/Kconfig.debug | 2 +-
>> mm/cma.c | 2 +-
>> 5 files changed, 30 insertions(+), 4 deletions(-)
>> create mode 100644 arch/arm64/mm/physaddr.c
>>
>> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
>> index ba62df8..9805adc 100644
>> --- a/arch/arm64/include/asm/memory.h
>> +++ b/arch/arm64/include/asm/memory.h
>> @@ -106,11 +106,19 @@
>> * private definitions which should NOT be used outside memory.h
>> * files. Use virt_to_phys/phys_to_virt/__pa/__va instead.
>> */
>> -#define __virt_to_phys(x) ({ \
>> +#define __virt_to_phys_nodebug(x) ({ \
>> phys_addr_t __x = (phys_addr_t)(x); \
>> __x & BIT(VA_BITS - 1) ? (__x & ~PAGE_OFFSET) + PHYS_OFFSET : \
>> (__x - kimage_voffset); })
>>
>> +#ifdef CONFIG_DEBUG_VIRTUAL
>> +#ifndef __ASSEMBLY__
>> +extern unsigned long __virt_to_phys(unsigned long x);
>> +#endif
>> +#else
>> +#define __virt_to_phys(x) __virt_to_phys_nodebug(x)
>> +#endif
>> +
>
> Couldn't you move this whole block into the #ifndef __ASSEMBLY__
> section lower down in the file?
>
Yes, I think that makes more sense.
>> #define __phys_to_virt(x) ((unsigned long)((x) - PHYS_OFFSET) | PAGE_OFFSET)
>> #define __phys_to_kimg(x) ((unsigned long)((x) + kimage_voffset))
>>
>> @@ -202,6 +210,7 @@ static inline void *phys_to_virt(phys_addr_t x)
>> * Drivers should NOT use these either.
>> */
>> #define __pa(x) __virt_to_phys((unsigned long)(x))
>> +#define __pa_nodebug(x) __virt_to_phys_nodebug((unsigned long)(x))
>> #define __va(x) ((void *)__phys_to_virt((phys_addr_t)(x)))
>> #define pfn_to_kaddr(pfn) __va((pfn) << PAGE_SHIFT)
>> #define virt_to_pfn(x) __phys_to_pfn(__virt_to_phys(x))
>> diff --git a/arch/arm64/mm/Makefile b/arch/arm64/mm/Makefile
>> index 54bb209..bcea84e 100644
>> --- a/arch/arm64/mm/Makefile
>> +++ b/arch/arm64/mm/Makefile
>> @@ -5,6 +5,6 @@ obj-y := dma-mapping.o extable.o fault.o init.o \
>> obj-$(CONFIG_HUGETLB_PAGE) += hugetlbpage.o
>> obj-$(CONFIG_ARM64_PTDUMP) += dump.o
>> obj-$(CONFIG_NUMA) += numa.o
>> -
>> +obj-$(CONFIG_DEBUG_VIRTUAL) += physaddr.o
>> obj-$(CONFIG_KASAN) += kasan_init.o
>> KASAN_SANITIZE_kasan_init.o := n
>> diff --git a/arch/arm64/mm/physaddr.c b/arch/arm64/mm/physaddr.c
>> new file mode 100644
>> index 0000000..6c271e2
>> --- /dev/null
>> +++ b/arch/arm64/mm/physaddr.c
>> @@ -0,0 +1,17 @@
>> +#include <linux/mm.h>
>> +
>> +#include <asm/memory.h>
>> +
>> +unsigned long __virt_to_phys(unsigned long x)
>> +{
>> + phys_addr_t __x = (phys_addr_t)x;
>> +
>> + if (__x & BIT(VA_BITS - 1)) {
>> + /* The bit check ensures this is the right range */
>
> Could you expand the comment to clarify that the linear region starts
> right in the middle of the kernel virtual address space, and so we
> only have to test the top bit?
>
Sure.
>> + return (__x & ~PAGE_OFFSET) + PHYS_OFFSET;
>> + } else {l
>> + VIRTUAL_BUG_ON(x < kimage_vaddr || x > (unsigned long)_end);
>
> This looks fine. References to _end are relocated at boot to refer to
> the actual runtime offset.
>
Thanks for clarifying.
>> + return (__x - kimage_voffset);
>> + }
>> +}
>> +EXPORT_SYMBOL(__virt_to_phys);
>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> index 33bc56c..e5634bb 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -604,7 +604,7 @@ config DEBUG_VM_PGFLAGS
>>
>> config DEBUG_VIRTUAL
>> bool "Debug VM translations"
>> - depends on DEBUG_KERNEL && X86
>> + depends on DEBUG_KERNEL && (X86 || ARM64)
>> help
>> Enable some costly sanity checks in virtual to page code. This can
>> catch mistakes with virt_to_page() and friends.
>> diff --git a/mm/cma.c b/mm/cma.c
>> index 384c2cb..2345803 100644
>> --- a/mm/cma.c
>> +++ b/mm/cma.c
>> @@ -235,7 +235,7 @@ int __init cma_declare_contiguous(phys_addr_t base,
>> phys_addr_t highmem_start;
>> int ret = 0;
>>
>> -#ifdef CONFIG_X86
>> +#if defined(CONFIG_X86) || defined(CONFIG_ARM64)
>> /*
>> * high_memory isn't direct mapped memory so retrieving its physical
>> * address isn't appropriate. But it would be useful to check the
>> --
>> 2.7.4
>>
^ permalink raw reply
* [PATCH v6 4/5] ARM: DTS: da850: Add cfgchip syscon node
From: David Lechner @ 2016-10-28 17:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <m2fungcses.fsf@baylibre.com>
On 10/28/2016 12:08 PM, Kevin Hilman wrote:
> Sekhar Nori <nsekhar@ti.com> writes:
>
>> On Wednesday 26 October 2016 09:38 PM, David Lechner wrote:
>>> On 10/25/2016 10:06 PM, David Lechner wrote:
>>>> Add a syscon node for the SoC CFGCHIPn registers. This is needed for
>>>> the new usb phy driver.
>>>>
>>>> Signed-off-by: David Lechner <david@lechnology.com>
>>>> ---
>>>> arch/arm/boot/dts/da850.dtsi | 4 ++++
>>>> 1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>>> index f79e1b9..6bbf20d 100644
>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>> @@ -188,6 +188,10 @@
>>>> };
>>>>
>>>> };
>>>> + cfgchip: cfgchip at 1417c {
>>>
>>> I wonder if there is a more generic name instead of cfgchip at . Is there a
>>> preferred generic name for syscon nodes?
>>
>> I did not find anything in ePAPR, but chip-controller might be more
>> appropriate.
>>
>>>
>>>> + compatible = "ti,da830-cfgchip", "syscon";
>>
>> Looks like we need "simple-mfd" too in the compatible list?
>>
>> I think we can also fold patch 5/5 into this patch and add the cfgchip
>> along with USB phy child node included.
>>
>> If you respin the patch, I can drop 4/5 and 5/5 that I have queued and
>> included the updated patch instead.
>
> Sekhar, what's your opinion of having this syscon just for CFGCHIP* vs
> a single syscon for the whole SYSCFG0 region.
>
> The drivers/bus driver from Bartosz is also using SYSCFG0 registers, and
> proposing a sysconf ro this region, but it will need to exclude the
> CFGCHIPn registers if we also have this syscon.
What about the pinmux registers, which are already being used separately
too?
>
> I tend to think we should just have one for the whole SYSCFG0 which
> this series could use.
>
> Unfortunately, the PHY driver is already merged and it references the
> syscon by compatible. The PHY driver should probably be fixed to find
> its syscon by phandle, and then maybe we could move to a single syscon
> for SYSCFG0?
I agree that this should be change, but I was thinking we should use
syscon_node_to_regmap(np->parent) since the phy node should be a child
of the syscon node.
>
> Let us know your preference, I don't have a very strong feeling either
> way, but since we're already part way down the path of the CFGCHIP
> syscon, we should keep it and later migrate it to one for all of
> SYSCFG0.
>
> Kevin
>
^ permalink raw reply
* pinctrl: mediatek: build failure if CONFIG_IRQ_DOMAIN is not set
From: Paul Bolle @ 2016-10-28 17:20 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
0) A rather spartan build, on x86_64, which did include
drivers/pinctrl/mediatek/pinctrl-mtk-common.o failed like this:
drivers/pinctrl/mediatek/pinctrl-mtk-common.c: In function ?mtk_gpio_to_irq?:
drivers/pinctrl/mediatek/pinctrl-mtk-common.c:838:8: error: implicit declaration of function ?irq_find_mapping? [-Werror=implicit-function-declaration]
irq = irq_find_mapping(pctl->domain, pin->eint.eintnum);
^~~~~~~~~~~~~~~~
drivers/pinctrl/mediatek/pinctrl-mtk-common.c: In function ?mtk_pctrl_init?:
drivers/pinctrl/mediatek/pinctrl-mtk-common.c:1474:17: error: implicit declaration of function ?irq_domain_add_linear? [-Werror=implicit-function-declaration]
pctl->domain = irq_domain_add_linear(np,
^~~~~~~~~~~~~~~~~~~~~
drivers/pinctrl/mediatek/pinctrl-mtk-common.c:1475:27: error: ?irq_domain_simple_ops? undeclared (first use in this function)
pctl->devdata->ap_num, &irq_domain_simple_ops, NULL);
^~~~~~~~~~~~~~~~~~~~~
drivers/pinctrl/mediatek/pinctrl-mtk-common.c:1475:27: note: each undeclared identifier is reported only once for each function it appears in
drivers/pinctrl/mediatek/pinctrl-mtk-common.c:1484:14: error: implicit declaration of function ?irq_create_mapping? [-Werror=implicit-function-declaration]
int virq = irq_create_mapping(pctl->domain, i);
^~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
make[3]: *** [drivers/pinctrl/mediatek/pinctrl-mtk-common.o] Error 1
make[2]: *** [drivers/pinctrl/mediatek] Error 2
make[1]: *** [drivers/pinctrl] Error 2
make: *** [drivers] Error 2
1) That build had CONFIG_COMPILE_TEST set (obviously) but
CONFIG_IRQ_DOMAIN not set.
2) This quick hack fixes that for me:
diff --git a/drivers/pinctrl/mediatek/Kconfig b/drivers/pinctrl/mediatek/Kconfig
index 419ea4d5964d..066087156dcc 100644
--- a/drivers/pinctrl/mediatek/Kconfig
+++ b/drivers/pinctrl/mediatek/Kconfig
@@ -7,6 +7,7 @@ config PINCTRL_MTK
select GENERIC_PINCONF
select GPIOLIB
select OF_GPIO
+ select IRQ_DOMAIN
# For ARMv7 SoCs
config PINCTRL_MT2701
3) Would you like me to submit a proper (but lightly tested) patch or
do you prefer to fix this yourself?
Thanks,
Paul Bolle
^ permalink raw reply related
* [RFC PATCH 0/6] media: davinci: VPIF: add DT support
From: Kevin Hilman @ 2016-10-28 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161025235536.7342-1-khilman@baylibre.com>
Kevin Hilman <khilman@baylibre.com> writes:
> This series attempts to add DT support to the davinci VPIF capture
> driver.
>
> I'm not sure I've completely grasped the proper use of the ports and
> endpoints stuff, so this RFC is primarily to get input on whether I'm
> on the right track.
>
> The last patch is the one where all my questions are, the rest are
> just prep work to ge there.
>
> Tested on da850-lcdk and was able to do basic frame capture from the
> composite input.
>
> Series applies on v4.9-rc1
And FYI for anyone wanting to test, it needs a few config options
enabled[1] that are not (yet) part of davinci_all_defconfig. I'll
update the defconfig when I'm ready to send non-RFC patches.
Kevin
[1]
CONFIG_MEDIA_SUPPORT=y
CONFIG_MEDIA_CAMERA_SUPPORT=y
CONFIG_VIDEO_DEV=y
CONFIG_VIDEO_V4L2=y
CONFIG_VIDEO_V4L2_SUBDEV_API=y
CONFIG_V4L_PLATFORM_DRIVERS=y
CONFIG_VIDEO_DAVINCI_VPIF_CAPTURE=y
# manually select codecs
CONFIG_MEDIA_SUBDRV_AUTOSELECT=n
# da850-lcdk
CONFIG_VIDEO_TVP514X=y
CONFIG_VIDEO_ADV7343=y
^ permalink raw reply
* SMR masking and PCI
From: Stuart Yoder @ 2016-10-28 17:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c39b785a-0f18-fc0a-ce08-7ebe3cfaf8c5@arm.com>
> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy at arm.com]
> Sent: Friday, October 28, 2016 11:17 AM
> To: Stuart Yoder <stuart.yoder@nxp.com>; Mark Rutland <mark.rutland@arm.com>
> Cc: linux-arm-kernel at lists.infradead.org; Will Deacon <will.deacon@arm.com>; Diana Madalina Craciun
> <diana.craciun@nxp.com>; Nipun Gupta <nipun.gupta@nxp.com>; iommu at lists.linux-foundation.org
> Subject: Re: SMR masking and PCI
>
> Hi Stuart,
>
> On 27/10/16 18:10, Stuart Yoder wrote:
> > Hi Robin,
> >
> > A question about how the SMR masking defined in the arm,smmu binding
> > relates to the PCI iommu-map.
> >
> > The #iommu-cells property defines the number of cells an "IOMMU specifier"
> > takes and 2 is specified to be:
> >
> > SMMUs with stream matching support and complex masters
> > may use a value of 2, where the second cell represents
> > an SMR mask to combine with the ID in the first cell.
> >
> > An iommu-map entry is defined as:
> >
> > (rid-base,iommu,iommu-base,length)
> >
> > What seems to be currently missing in the iommu-map support is
> > the possibility the case where #iommu-cells=<2>.
>
> Indeed. The bindings have so far rather implicitly assumed the case of
> #{msi,iommu}-cells = 1, and the code has followed suit.
>
> > In this case iommu-base which is an IOMMU specifier should
> > occupy 2 cells. For example on an ls2085a we would want:
> >
> > iommu-map = <0x0 0x6 0x7 0x3ff 0x1
> > 0x100 0x6 0x8 0x3ff 0x1>;
> >
> > ...to mask our stream IDs to 10 bits.
> >
> > This should work in theory and comply with the bindings, no?
>
> In theory, but now consider:
>
> iommu-map = <0x0 0x6 0x7 0x3ff 0x2>;
>
> faced with ID 1. The input base is 0, the output base is the 2-cell
> value 0x7000003ff, so the final ID value should be 0x700000400, right?
No. The second cell as per the SMMU binding is the SMR mask...applied
by the SMMU before matching stream IDs.
In our case we want to mask off the upper TBU ID bits that the SMMU tags
onto the stream ID in our RID->SID LUT table.
RID = 0
SID in LUT and seen by SMMU = 7
SMMU-500 TBU appends bits, making SID something like: 0xC07
SMR mask of 0x3ff should be applied making the SID: 0x7
> > of_pci_map_rid() seems to have a hardcoded assumption that
> > each field in the map is 4 bytes.
>
> It does. I guess we should explicitly check that #{msi,iommu}-cells = 1
> on the target node, and maybe clarify in the binding that that cell
> should represent a plain linear ID value (although that's pretty obvious
> from context IMO).
>
> > (Also, I guess that msi-map is not affected by this since it
> > is not related to the IOMMU...but we do have common code
> > handling both maps.)
>
> I'd say it's definitely affected, since #msi-cells can equally be more
> than 1, and encodes an equally opaque value.
>
> It seems pretty reasonable to me that we could extend the binding to
> accommodate #cells > 1 iff length == 1. Mark?
I'm not following why the length matters.
> That said, is there a concrete need for this, i.e. do you actually have
> one device with a single requester ID, which maps to multiple output IDs
> (which differ only in the upper bits) in a non-predictable manner?
Devices behind an fsl-mc bus instance share the same stream ID (and GIC
ITS device ID) but may be behind different TBUs.
(see drivers/staging/fsl-mc/README.txt for overview info on fsl-mc)
So we could have a bus instance with all devices having a stream ID
of 0x5. But the TBU ID appending causes the TCU to see incoming
stream IDs from different devices with values of 0x105 and 0x205.
We need to get those upper bits masked off.
I don't see the above being an issue for PCI and platform devices. But
we don't want to expose TBU topography and IDs. Those are treated as
a microarchitecture detail in our SoC and are not even documented.
The SMMU programming model does not expose the TCU/TBU split.
Just treating the stream ID namespace as a clean linear space is
much easier to understand without having to grasp the existence
of TBUs at all. Masking SMRs keeps things clean.
As far as msi-map goes, I don't think there is an issue there. I
don't think the TBU appended bits are propagated to the GIC ITS
(as the device ID) so I don't think any masking is needed. Perhaps
you or Marc know.
It seems like perhaps what we need is a new argument passed to
of_pci_map_rid() that tells the function how many cells the
base iommu/msi identifier is. The caller supplies it.
Thanks,
Stuart
^ permalink raw reply
* [PATCH] staging: vc04_services: setup DMA and coherent mask
From: Michael Zoran @ 2016-10-28 17:11 UTC (permalink / raw)
To: linux-arm-kernel
Setting the DMA mask is optional on 32 bit but
is mandatory on 64 bit. Set the DMA mask and coherent
to force all DMA to be in the 32 bit address space.
This is considered a "good practice" and most drivers
already do this.
Signed-off-by: Michael Zoran <mzoran@crowfest.net>
---
.../staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index a5afcc5..6fa2b5a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -97,6 +97,16 @@ int vchiq_platform_init(struct platform_device *pdev, VCHIQ_STATE_T *state)
int slot_mem_size, frag_mem_size;
int err, irq, i;
+ /*
+ * Setting the DMA mask is necessary in the 64 bit environment.
+ * It isn't necessary in a 32 bit environment but is considered
+ * a good practice.
+ */
+ err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+
+ if (err < 0)
+ return err;
+
(void)of_property_read_u32(dev->of_node, "cache-line-size",
&g_cache_line_size);
g_fragments_size = 2 * g_cache_line_size;
--
2.10.1
^ permalink raw reply related
* [GIT PULL] Broadcom ARM64 Device Tree fixes for 4.9
From: Florian Fainelli @ 2016-10-28 17:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477331005-32163-1-git-send-email-f.fainelli@gmail.com>
On 10/24/2016 10:43 AM, Florian Fainelli wrote:
> The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
>
> Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
>
> are available in the git repository at:
>
> http://github.com/Broadcom/stblinux.git tags/arm-soc/for-4.9/devicetree-arm64-fixes
>
> for you to fetch changes up to 963d790468a2f581abf039b45edac79af5e16e55:
>
> arm64: dts: Updated NAND DT properties for NS2 SVK (2016-10-23 14:50:20 -0700)
>
> ----------------------------------------------------------------
> This pull request contains a single fix for Broadcom ARM64-based SoCs:
>
> - Ray adds the required bus width and OOB sector size properties to the
> Northstar 2 SVK reference board in order for the NAND controller to work
> properly
>
> ----------------------------------------------------------------
> Ray Jui (1):
> arm64: dts: Updated NAND DT properties for NS2 SVK
>
> arch/arm64/boot/dts/broadcom/ns2-svk.dts | 2 ++
> 1 file changed, 2 insertions(+)
Arnd, Kevin, Olof,
Can you take this change? Thanks!
--
Florian
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox