* [PATCH v2 00/12] refactor dts and add support for more boards
From: Matthias Brugger @ 2018-05-11 16:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1523434757.git.sean.wang@mediatek.com>
On 04/11/2018 10:53 AM, sean.wang at mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
>
> Changes since v1:
> - Dropped several patches which have been merged.
> - Rebased to linux-next-20180410 where those dependent patches including
> [1] and [2] all have been got merged.
> - Revised according to suggestions and comments on v1, where the major work is
> to put board-specific nodes into DTS files and to leave the common nodes
> among boards in DTSI files as done in patch 7,9,10,11 and 12.
> - Merged Ryder Lee's patch for update audio nodes reflecting the latest
> bindings.
> - Revised commit message.
>
> The series is to refactor current .dts for allowing more boards can properly
> join and reuse almost the same device node definitions.
>
> Before we're doing that, some works about enhancements and fixups are being
> made on the dt-binding document and .dts files. In addition, the series has to
> depend on the patch [1][2]. Otherwise, certain build failure would have an
> occurrence.
>
> [1] arm: dts: mt7623: add PCIe related nodes
> http://lists.infradead.org/pipermail/linux-mediatek/2018-February/012053.html
>
> [2] dt-bindings: soc: add header files required for MT7623A SCPSYS dt-binding
> http://lists.infradead.org/pipermail/linux-mediatek/2018-February/011930.html
>
> Ryder Lee (1):
> arm: dts: mediatek: modify audio related nodes for both MT2701 and
> MT7623
>
> Sean Wang (11):
> dt-bindings: cpufreq: mediatek: use - instead of _ in examples
> arm: dts: mediatek: converted to using SPDX identifiers
> arm: dts: mt7623: fix invalid memory node being generated
> arm: dts: mt7623: fix available memory size on bananapi-r2
> arm: dts: mt7623: fix all Warnings (unit_address_vs_reg)
> arm: dts: mt7623: add BTIF, HSDMA and SPI-NOR device nodes
> arm: dts: mt6323: move node mt6323 leds to mt6323.dtsi
> arm: dts: mt7623: extend common file reused by all boards with MT7623
> SoCs
> arm: dts: mt7623: add MT7623A SoC level DTS
> arm: dts: mt7623: add MT7623A reference boards
> arm: dts: mt7623: add MT7623N reference board with eMMC
>
I merged all but 1/12 which needs to go through rafaels tree and 8/12 which was
already applied from a different series.
You can find them in v4.17-next/soc on git.kernel.org
Regards,
Matthias
> .../bindings/cpufreq/cpufreq-mediatek.txt | 6 +-
> arch/arm/boot/dts/Makefile | 3 +
> arch/arm/boot/dts/mt2701-evb.dts | 9 +-
> arch/arm/boot/dts/mt2701.dtsi | 197 ++++----
> arch/arm/boot/dts/mt6323.dtsi | 17 +-
> arch/arm/boot/dts/mt6580-evbp1.dts | 9 +-
> arch/arm/boot/dts/mt6580.dtsi | 9 +-
> arch/arm/boot/dts/mt6589-aquaris5.dts | 10 +-
> arch/arm/boot/dts/mt6589.dtsi | 12 +-
> arch/arm/boot/dts/mt6592-evb.dts | 9 +-
> arch/arm/boot/dts/mt6592.dtsi | 9 +-
> arch/arm/boot/dts/mt7623.dtsi | 537 ++++++++++++++++-----
> arch/arm/boot/dts/mt7623a-rfb-emmc.dts | 291 +++++++++++
> arch/arm/boot/dts/mt7623a-rfb-nand.dts | 337 +++++++++++++
> arch/arm/boot/dts/mt7623a.dtsi | 44 ++
> arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 279 ++---------
> arch/arm/boot/dts/mt7623n-rfb-emmc.dts | 326 +++++++++++++
> arch/arm/boot/dts/mt7623n-rfb-nand.dts | 40 +-
> arch/arm/boot/dts/mt7623n-rfb.dtsi | 12 +-
> arch/arm/boot/dts/mt8127-moose.dts | 9 +-
> arch/arm/boot/dts/mt8127.dtsi | 9 +-
> arch/arm/boot/dts/mt8135-evbp1.dts | 9 +-
> arch/arm/boot/dts/mt8135.dtsi | 9 +-
> 23 files changed, 1576 insertions(+), 616 deletions(-)
> create mode 100644 arch/arm/boot/dts/mt7623a-rfb-emmc.dts
> create mode 100644 arch/arm/boot/dts/mt7623a-rfb-nand.dts
> create mode 100644 arch/arm/boot/dts/mt7623a.dtsi
> create mode 100644 arch/arm/boot/dts/mt7623n-rfb-emmc.dts
>
^ permalink raw reply
* [PATCH v2 1/4] drm/rockchip: add transfer function for cdn-dp
From: Enric Balletbo Serra @ 2018-05-11 16:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1525861364-26323-1-git-send-email-hl@rock-chips.com>
Hi Lin,
2018-05-09 12:22 GMT+02:00 Lin Huang <hl@rock-chips.com>:
> From: Chris Zhong <zyw@rock-chips.com>
>
> We may support training outside firmware, so we need support
> dpcd read/write to get the message or do some setting with
> display.
>
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> ---
>
> Changes in v2:
> - update patch following Enric suggest
>
> drivers/gpu/drm/rockchip/cdn-dp-core.c | 55 ++++++++++++++++++++++++----
> drivers/gpu/drm/rockchip/cdn-dp-core.h | 1 +
> drivers/gpu/drm/rockchip/cdn-dp-reg.c | 67 ++++++++++++++++++++++++++++++----
> drivers/gpu/drm/rockchip/cdn-dp-reg.h | 14 ++++++-
> 4 files changed, 120 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> index c6fbdcd..cce64c1 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> @@ -176,8 +176,8 @@ static int cdn_dp_get_sink_count(struct cdn_dp_device *dp, u8 *sink_count)
> u8 value;
>
> *sink_count = 0;
> - ret = cdn_dp_dpcd_read(dp, DP_SINK_COUNT, &value, 1);
> - if (ret)
> + ret = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, &value, 1);
> + if (ret < 0)
> return ret;
>
> *sink_count = DP_GET_SINK_COUNT(value);
> @@ -374,9 +374,9 @@ static int cdn_dp_get_sink_capability(struct cdn_dp_device *dp)
> if (!cdn_dp_check_sink_connection(dp))
> return -ENODEV;
>
> - ret = cdn_dp_dpcd_read(dp, DP_DPCD_REV, dp->dpcd,
> - DP_RECEIVER_CAP_SIZE);
> - if (ret) {
> + ret = drm_dp_dpcd_read(&dp->aux, DP_DPCD_REV, dp->dpcd,
> + sizeof(dp->dpcd));
> + if (ret < 0) {
> DRM_DEV_ERROR(dp->dev, "Failed to get caps %d\n", ret);
> return ret;
> }
> @@ -582,8 +582,8 @@ static bool cdn_dp_check_link_status(struct cdn_dp_device *dp)
> if (!port || !dp->link.rate || !dp->link.num_lanes)
> return false;
>
> - if (cdn_dp_dpcd_read(dp, DP_LANE0_1_STATUS, link_status,
> - DP_LINK_STATUS_SIZE)) {
> + if (drm_dp_dpcd_read_link_status(&dp->aux, link_status) !=
> + DP_LINK_STATUS_SIZE) {
> DRM_ERROR("Failed to get link status\n");
> return false;
> }
> @@ -1012,6 +1012,40 @@ static int cdn_dp_pd_event(struct notifier_block *nb,
> return NOTIFY_DONE;
> }
>
> +static ssize_t cdn_dp_aux_transfer(struct drm_dp_aux *aux,
> + struct drm_dp_aux_msg *msg)
> +{
> + struct cdn_dp_device *dp = container_of(aux, struct cdn_dp_device, aux);
> + int ret;
> + u8 status;
> +
> + switch (msg->request & ~DP_AUX_I2C_MOT) {
> + case DP_AUX_NATIVE_WRITE:
> + case DP_AUX_I2C_WRITE:
> + case DP_AUX_I2C_WRITE_STATUS_UPDATE:
> + ret = cdn_dp_dpcd_write(dp, msg->address, msg->buffer,
> + msg->size);
> + break;
> + case DP_AUX_NATIVE_READ:
> + case DP_AUX_I2C_READ:
> + ret = cdn_dp_dpcd_read(dp, msg->address, msg->buffer,
> + msg->size);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + status = cdn_dp_get_aux_status(dp);
> + if (status == AUX_STATUS_ACK)
> + msg->reply = DP_AUX_NATIVE_REPLY_ACK;
> + else if (status == AUX_STATUS_NACK)
> + msg->reply = DP_AUX_NATIVE_REPLY_NACK;
> + else if (status == AUX_STATUS_DEFER)
> + msg->reply = DP_AUX_NATIVE_REPLY_DEFER;
> +
> + return ret;
> +}
> +
> static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
> {
> struct cdn_dp_device *dp = dev_get_drvdata(dev);
> @@ -1030,6 +1064,13 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
> dp->active = false;
> dp->active_port = -1;
> dp->fw_loaded = false;
> + dp->aux.name = "DP-AUX";
> + dp->aux.transfer = cdn_dp_aux_transfer;
> + dp->aux.dev = dev;
> +
> + ret = drm_dp_aux_register(&dp->aux);
> + if (ret)
> + return ret;
>
> INIT_WORK(&dp->event_work, cdn_dp_pd_event_work);
>
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h b/drivers/gpu/drm/rockchip/cdn-dp-core.h
> index f57e296..46159b2 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.h
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h
> @@ -78,6 +78,7 @@ struct cdn_dp_device {
> struct platform_device *audio_pdev;
> struct work_struct event_work;
> struct edid *edid;
> + struct drm_dp_aux aux;
>
> struct mutex lock;
> bool connected;
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.c b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> index eb3042c..afdfda0 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> @@ -221,7 +221,11 @@ static int cdn_dp_reg_write_bit(struct cdn_dp_device *dp, u16 addr,
> sizeof(field), field);
> }
>
> -int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
> +/*
> + * Returns the number of bytes transferred on success, or a negative error
> + * code on failure. -ETIMEDOUT is returned if mailbox message not send success;
nit: -ETIMEDOUT is returned if mailbox message was not sent successfully.
> + */
> +ssize_t cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
> {
> u8 msg[5], reg[5];
> int ret;
> @@ -247,24 +251,40 @@ int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
> goto err_dpcd_read;
>
> ret = cdn_dp_mailbox_read_receive(dp, data, len);
> + if (!ret)
> + return len;
>
> err_dpcd_read:
> + DRM_DEV_ERROR(dp->dev, "dpcd read failed: %d\n", ret);
> return ret;
> }
>
> -int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
> +#define CDN_AUX_HEADER_SIZE 5
> +#define CDN_AUX_MSG_SIZE 20
> +/*
> + * Returns the number of bytes transferred on success, or a negative error
> + * code on failure. -ETIMEDOUT is returned if mailbox message not send success;
> + * -EINVAL is return if get the wrong data size after message send.
nit:
-ETIMEDOUT is returned if mailbox message was not sent successfully;
-EINVAL is returned if you get the wrong data size after the message
is sent.
> + */
> +ssize_t cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
> {
> - u8 msg[6], reg[5];
> + u8 msg[CDN_AUX_MSG_SIZE + CDN_AUX_HEADER_SIZE];
> + u8 reg[CDN_AUX_HEADER_SIZE];
> int ret;
>
> - msg[0] = 0;
> - msg[1] = 1;
> + if (WARN_ON(len > CDN_AUX_MSG_SIZE) || WARN_ON(len <= 0))
> + return -EINVAL;
> +
> + msg[0] = (len >> 8) & 0xff;
> + msg[1] = len & 0xff;
> msg[2] = (addr >> 16) & 0xff;
> msg[3] = (addr >> 8) & 0xff;
> msg[4] = addr & 0xff;
> - msg[5] = value;
> +
> + memcpy(msg + CDN_AUX_HEADER_SIZE, data, len);
> +
> ret = cdn_dp_mailbox_send(dp, MB_MODULE_ID_DP_TX, DPTX_WRITE_DPCD,
> - sizeof(msg), msg);
> + CDN_AUX_HEADER_SIZE + len, msg);
> if (ret)
> goto err_dpcd_write;
>
> @@ -277,8 +297,12 @@ int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
> if (ret)
> goto err_dpcd_write;
>
> - if (addr != (reg[2] << 16 | reg[3] << 8 | reg[4]))
> + if ((len != (reg[0] << 8 | reg[1])) ||
> + (addr != (reg[2] << 16 | reg[3] << 8 | reg[4]))) {
> ret = -EINVAL;
> + } else {
> + return len;
> + }
>
> err_dpcd_write:
> if (ret)
> @@ -286,6 +310,33 @@ int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
> return ret;
> }
>
> +int cdn_dp_get_aux_status(struct cdn_dp_device *dp)
> +{
> + u8 status;
> + int ret;
> +
> + ret = cdn_dp_mailbox_send(dp, MB_MODULE_ID_DP_TX,
> + DPTX_GET_LAST_AUX_STAUS, 0, NULL);
> + if (ret)
> + goto err_get_hpd;
> +
> + ret = cdn_dp_mailbox_validate_receive(dp, MB_MODULE_ID_DP_TX,
> + DPTX_GET_LAST_AUX_STAUS,
> + sizeof(status));
> + if (ret)
> + goto err_get_hpd;
> +
> + ret = cdn_dp_mailbox_read_receive(dp, &status, sizeof(status));
> + if (ret)
> + goto err_get_hpd;
> +
> + return status;
> +
> +err_get_hpd:
> + DRM_DEV_ERROR(dp->dev, "get aux status failed: %d\n", ret);
> + return ret;
> +}
> +
> int cdn_dp_load_firmware(struct cdn_dp_device *dp, const u32 *i_mem,
> u32 i_size, const u32 *d_mem, u32 d_size)
> {
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.h b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> index c4bbb4a83..6580b11 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> @@ -328,6 +328,13 @@
> #define GENERAL_BUS_SETTINGS 0x03
> #define GENERAL_TEST_ACCESS 0x04
>
> +/* AUX status*/
> +#define AUX_STATUS_ACK 0
> +#define AUX_STATUS_NACK 1
> +#define AUX_STATUS_DEFER 2
> +#define AUX_STATUS_SINK_ERROR 3
> +#define AUX_STATUS_BUS_ERROR 4
> +
> #define DPTX_SET_POWER_MNG 0x00
> #define DPTX_SET_HOST_CAPABILITIES 0x01
> #define DPTX_GET_EDID 0x02
> @@ -469,8 +476,11 @@ int cdn_dp_set_host_cap(struct cdn_dp_device *dp, u8 lanes, bool flip);
> int cdn_dp_event_config(struct cdn_dp_device *dp);
> u32 cdn_dp_get_event(struct cdn_dp_device *dp);
> int cdn_dp_get_hpd_status(struct cdn_dp_device *dp);
> -int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value);
> -int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len);
> +ssize_t cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr,
> + u8 *data, u16 len);
> +ssize_t cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr,
> + u8 *data, u16 len);
> +int cdn_dp_get_aux_status(struct cdn_dp_device *dp);
> int cdn_dp_get_edid_block(void *dp, u8 *edid,
> unsigned int block, size_t length);
> int cdn_dp_train_link(struct cdn_dp_device *dp);
> --
> 2.7.4
>
Apart from the two nits, the patch looks good to me and works.
Reviewed-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Thanks,
Enric
^ permalink raw reply
* [PATCH] ARM: dts: BCM5301X: Switch D-Link DIR-885L to the new partitions syntax
From: Florian Fainelli @ 2018-05-11 16:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180510212000.11110-1-zajec5@gmail.com>
On Thu, 10 May 2018 23:20:00 +0200, Rafa? Mi?ecki <zajec5@gmail.com> wrote:
> From: Rafa? Mi?ecki <rafal@milecki.pl>
>
> This new syntax is slightly better designed & uses "compatible" string.
> For details see Documentation/devicetree/bindings/mtd/partition.txt .
>
> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
> ---
Applied to devicetree/next, thanks!
--
Florian
^ permalink raw reply
* [PATCH 2/2] arm64: dts: mt7622: add audio related device nodes
From: Matthias Brugger @ 2018-05-11 16:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1525233238.14792.14.camel@mtkswgap22>
On 05/02/2018 05:53 AM, Sean Wang wrote:
> Hi, Matthias
>
> On Wed, 2018-05-02 at 11:41 +0800, sean.wang at mediatek.com wrote:
>> From: Ryder Lee <ryder.lee@mediatek.com>
>>
>> Add audio device nodes and its proper setup for all used pins
>>
>> Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
>> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
>> ---
>> arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts | 11 +++-
>> arch/arm64/boot/dts/mediatek/mt7622.dtsi | 89 ++++++++++++++++++++++++++++
>> 2 files changed, 98 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts b/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
>> index 45d8655..b783764 100644
>> --- a/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
>> +++ b/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
>> @@ -18,7 +18,7 @@
>> compatible = "mediatek,mt7622-rfb1", "mediatek,mt7622";
>>
>> chosen {
>> - bootargs = "console=ttyS0,115200n1";
>> + bootargs = "console=ttyS0,115200n1 swiotlb=512";
>> };
>>
>> cpus {
>> @@ -163,10 +163,17 @@
>> i2s1_pins: i2s1-pins {
>> mux {
>> function = "i2s";
>> - groups = "i2s_out_bclk_ws_mclk",
>> + groups = "i2s_out_mclk_bclk_ws",
>
> The old one ("i2s_out_bclk_ws_mclk") should be the indecisive value when
> I initially worked on the pinctrl device.
>
> It has to be corrected with "i2s_out_mclk_bclk_ws" to comply with the
> final dt-bindings.
>
>> "i2s1_in_data",
>> "i2s1_out_data";
>> };
>> +
>> + conf {
>> + pins = "I2S1_IN", "I2S1_OUT", "I2S_BCLK",
>> + "I2S_WS", "I2S_MCLK";
>> + drive-strength = <12>;
>> + bias-pull-down;
>> + };
>> };
>>
>> irrx_pins: irrx-pins {
>> diff --git a/arch/arm64/boot/dts/mediatek/mt7622.dtsi b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
>> index 6bbabb6..9213c96 100644
>> --- a/arch/arm64/boot/dts/mediatek/mt7622.dtsi
>> +++ b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
>> @@ -527,6 +527,95 @@
>> status = "disabled";
>> };
>>
>> + audsys: clock-controller at 11220000 {
>> + compatible = "mediatek,mt7622-audsys", "syscon";
>> + reg = <0 0x11220000 0 0x2000>;
>> + #clock-cells = <1>;
>
> The binding have been through broonie/sound.git around a week ago
>
Thanks for the clarifications!
Pushed to v4.17-next/dts64
Regards,
Matthias
>> +
>> + afe: audio-controller {
>> + compatible = "mediatek,mt7622-audio";
>> + interrupts = <GIC_SPI 144 IRQ_TYPE_LEVEL_LOW>,
>> + <GIC_SPI 145 IRQ_TYPE_LEVEL_LOW>;
>> + interrupt-names = "afe", "asys";
>> +
>> + clocks = <&infracfg CLK_INFRA_AUDIO_PD>,
>> + <&topckgen CLK_TOP_AUD1_SEL>,
>> + <&topckgen CLK_TOP_AUD2_SEL>,
>> + <&topckgen CLK_TOP_A1SYS_HP_DIV_PD>,
>> + <&topckgen CLK_TOP_A2SYS_HP_DIV_PD>,
>> + <&topckgen CLK_TOP_I2S0_MCK_SEL>,
>> + <&topckgen CLK_TOP_I2S1_MCK_SEL>,
>> + <&topckgen CLK_TOP_I2S2_MCK_SEL>,
>> + <&topckgen CLK_TOP_I2S3_MCK_SEL>,
>> + <&topckgen CLK_TOP_I2S0_MCK_DIV>,
>> + <&topckgen CLK_TOP_I2S1_MCK_DIV>,
>> + <&topckgen CLK_TOP_I2S2_MCK_DIV>,
>> + <&topckgen CLK_TOP_I2S3_MCK_DIV>,
>> + <&topckgen CLK_TOP_I2S0_MCK_DIV_PD>,
>> + <&topckgen CLK_TOP_I2S1_MCK_DIV_PD>,
>> + <&topckgen CLK_TOP_I2S2_MCK_DIV_PD>,
>> + <&topckgen CLK_TOP_I2S3_MCK_DIV_PD>,
>> + <&audsys CLK_AUDIO_I2SO1>,
>> + <&audsys CLK_AUDIO_I2SO2>,
>> + <&audsys CLK_AUDIO_I2SO3>,
>> + <&audsys CLK_AUDIO_I2SO4>,
>> + <&audsys CLK_AUDIO_I2SIN1>,
>> + <&audsys CLK_AUDIO_I2SIN2>,
>> + <&audsys CLK_AUDIO_I2SIN3>,
>> + <&audsys CLK_AUDIO_I2SIN4>,
>> + <&audsys CLK_AUDIO_ASRCO1>,
>> + <&audsys CLK_AUDIO_ASRCO2>,
>> + <&audsys CLK_AUDIO_ASRCO3>,
>> + <&audsys CLK_AUDIO_ASRCO4>,
>> + <&audsys CLK_AUDIO_AFE>,
>> + <&audsys CLK_AUDIO_AFE_CONN>,
>> + <&audsys CLK_AUDIO_A1SYS>,
>> + <&audsys CLK_AUDIO_A2SYS>;
>> +
>> + clock-names = "infra_sys_audio_clk",
>> + "top_audio_mux1_sel",
>> + "top_audio_mux2_sel",
>> + "top_audio_a1sys_hp",
>> + "top_audio_a2sys_hp",
>> + "i2s0_src_sel",
>> + "i2s1_src_sel",
>> + "i2s2_src_sel",
>> + "i2s3_src_sel",
>> + "i2s0_src_div",
>> + "i2s1_src_div",
>> + "i2s2_src_div",
>> + "i2s3_src_div",
>> + "i2s0_mclk_en",
>> + "i2s1_mclk_en",
>> + "i2s2_mclk_en",
>> + "i2s3_mclk_en",
>> + "i2so0_hop_ck",
>> + "i2so1_hop_ck",
>> + "i2so2_hop_ck",
>> + "i2so3_hop_ck",
>> + "i2si0_hop_ck",
>> + "i2si1_hop_ck",
>> + "i2si2_hop_ck",
>> + "i2si3_hop_ck",
>> + "asrc0_out_ck",
>> + "asrc1_out_ck",
>> + "asrc2_out_ck",
>> + "asrc3_out_ck",
>> + "audio_afe_pd",
>> + "audio_afe_conn_pd",
>> + "audio_a1sys_pd",
>> + "audio_a2sys_pd";
>> +
>> + assigned-clocks = <&topckgen CLK_TOP_A1SYS_HP_SEL>,
>> + <&topckgen CLK_TOP_A2SYS_HP_SEL>,
>> + <&topckgen CLK_TOP_A1SYS_HP_DIV>,
>> + <&topckgen CLK_TOP_A2SYS_HP_DIV>;
>> + assigned-clock-parents = <&topckgen CLK_TOP_AUD1PLL>,
>> + <&topckgen CLK_TOP_AUD2PLL>;
>> + assigned-clock-rates = <0>, <0>, <49152000>, <45158400>;
>> + };
>> + };
>> +
>> mmc0: mmc at 11230000 {
>> compatible = "mediatek,mt7622-mmc";
>> reg = <0 0x11230000 0 0x1000>;
>
>
^ permalink raw reply
* [PATCH 3/9] soc: fsl: set rcpm bit for FTM
From: Leo Li @ 2018-05-11 17:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511033530.7931-3-yinbo.zhu@nxp.com>
> -----Original Message-----
> From: Yinbo Zhu [mailto:yinbo.zhu at nxp.com]
> Sent: Thursday, May 10, 2018 10:35 PM
> To: Yinbo Zhu <yinbo.zhu@nxp.com>; Rob Herring <robh+dt@kernel.org>;
> Mark Rutland <mark.rutland@arm.com>; Catalin Marinas )
> <catalin.marinas@arm.com>; Will Deacon ) <will.deacon@arm.com>;
> Lorenzo Pieralisi ) <lorenzo.pieralisi@arm.com>; Leo Li <leoyang.li@nxp.com>
> Cc: Xiaobo Xie <xiaobo.xie@nxp.com>; Ran Wang <ran.wang_1@nxp.com>;
> Daniel Lezcano <daniel.lezcano@linaro.org>; Thomas Gleixner
> <tglx@linutronix.de>; Shawn Guo <shawnguo@kernel.org>; Madalin-cristian
> Bucur <madalin.bucur@nxp.com>; Z.q. Hou <zhiqiang.hou@nxp.com>; Jerry
> Huang <jerry.huang@nxp.com>; M.h. Lian <minghuan.lian@nxp.com>;
> Qiang Zhao <qiang.zhao@nxp.com>; Fabio Estevam
> <fabio.estevam@nxp.com>; Jiaheng Fan <jiaheng.fan@nxp.com>; Po Liu
> <po.liu@nxp.com>; Nipun Gupta <nipun.gupta@nxp.com>; Horia Geant?
> <horia.geanta@nxp.com>; Priyanka Jain <priyanka.jain@nxp.com>; Sumit
> Garg <sumit.garg@nxp.com>; costi <constantin.tudor@freescale.com>;
> Bogdan Purcareata <bogdan.purcareata@nxp.com>; Meng Yi
> <meng.yi@nxp.com>; Wang Dongsheng <dongsheng.wang@nxp.com>; open
> list:CLOCKSOURCE, CLOCKEVENT DRIVERS <linux-kernel@vger.kernel.org>;
> open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> <devicetree@vger.kernel.org>; linux-arm-kernel at lists.infradead.org; open
> list:FREESCALE SOC DRIVERS <linuxppc-dev@lists.ozlabs.org>; Andy Tang
> <andy.tang@nxp.com>; Ying Zhang <ying.zhang22455@nxp.com>
> Subject: [PATCH 3/9] soc: fsl: set rcpm bit for FTM
>
> From: Zhang Ying-22455 <ying.zhang22455@nxp.com>
>
> Set RCPM for FTM when using FTM as wakeup source. Because the RCPM
> module of each platform has different big-end and little-end mode, there
> need to set RCPM depending on the platform.
>
> Signed-off-by: Zhang Ying-22455 <ying.zhang22455@nxp.com>
> Signed-off-by: Yinbo Zhu <yinbo.zhu@nxp.com>
> ---
> .../devicetree/bindings/timer/fsl,ftm-timer.txt | 7 ++
> drivers/soc/fsl/layerscape/ftm_alarm.c | 92 ++++++++++++++++++-
> 2 files changed, 94 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt
> b/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt
> index aa8c402..15ead58 100644
> --- a/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt
> +++ b/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt
> @@ -3,6 +3,13 @@ Freescale FlexTimer Module (FTM) Timer Required
> properties:
>
> - compatible : should be "fsl,ftm-timer"
Hi Yingbo,
This is a change that breaks backward compatibility and not acceptable.
> + Possible compatibles for ARM:
> + "fsl,ls1012a-ftm"
> + "fsl,ls1021a-ftm"
> + "fsl,ls1043a-ftm"
> + "fsl,ls1046a-ftm"
> + "fsl,ls1088a-ftm"
> + "fsl,ls208xa-ftm"
> - reg : Specifies base physical address and size of the register sets for the
> clock event device and clock source device.
> - interrupts : Should be the clock event device interrupt.
> diff --git a/drivers/soc/fsl/layerscape/ftm_alarm.c
> b/drivers/soc/fsl/layerscape/ftm_alarm.c
> index 6f9882f..811dcfa 100644
> --- a/drivers/soc/fsl/layerscape/ftm_alarm.c
> +++ b/drivers/soc/fsl/layerscape/ftm_alarm.c
There is no such file in the mainline kernel. So it looks like the patch set is based on some internal git repo instead of the upstream Linux kernel. This kind of patches shouldn't be sent to the upstream mailing list for review.
Regards,
Leo
^ permalink raw reply
* [PATCH 0/6] hwspinlock: Convert to use SPDX license identifiers
From: Suman Anna @ 2018-05-11 17:03 UTC (permalink / raw)
To: linux-arm-kernel
Hi Bjorn,
The following series switches over the current licensing text in
various HwSpinlock drivers to the SPDX licensing format. Please
let me know if you need any changes. If you are ok with the changes,
I will make similar changes to the remoteproc and rpmsg drivers and
post them at the earliest so that they can be picked up for 4.18.
regards
Suman
Suman Anna (6):
hwspinlock/core: Switch to SPDX license identifier
hwspinlock/omap: Switch to SPDX license identifier
hwspinlock: qcom: Switch to SPDX license identifier
hwspinlock/sirf: Switch to SPDX license identifier
hwspinlock/u8500: Switch to SPDX license identifier
hwspinlock: sprd: Switch to SPDX license identifier
drivers/hwspinlock/Kconfig | 1 +
drivers/hwspinlock/hwspinlock_core.c | 10 +---------
drivers/hwspinlock/hwspinlock_internal.h | 10 +---------
drivers/hwspinlock/omap_hwspinlock.c | 10 +---------
drivers/hwspinlock/qcom_hwspinlock.c | 10 +---------
drivers/hwspinlock/sirf_hwspinlock.c | 3 +--
drivers/hwspinlock/sprd_hwspinlock.c | 10 +---------
drivers/hwspinlock/u8500_hsem.c | 10 +---------
include/linux/hwspinlock.h | 10 +---------
9 files changed, 9 insertions(+), 65 deletions(-)
--
2.17.0
^ permalink raw reply
* [PATCH 1/6] hwspinlock/core: Switch to SPDX license identifier
From: Suman Anna @ 2018-05-11 17:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511170321.2452-1-s-anna@ti.com>
Use the appropriate SPDX license identifier in the Hwspinlock core
driver source files and drop the previous boilerplate license text.
Signed-off-by: Suman Anna <s-anna@ti.com>
---
drivers/hwspinlock/Kconfig | 1 +
drivers/hwspinlock/hwspinlock_core.c | 10 +---------
drivers/hwspinlock/hwspinlock_internal.h | 10 +---------
include/linux/hwspinlock.h | 10 +---------
4 files changed, 4 insertions(+), 27 deletions(-)
diff --git a/drivers/hwspinlock/Kconfig b/drivers/hwspinlock/Kconfig
index f0f467983960..e895d29500ee 100644
--- a/drivers/hwspinlock/Kconfig
+++ b/drivers/hwspinlock/Kconfig
@@ -1,3 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
#
# Generic HWSPINLOCK framework
#
diff --git a/drivers/hwspinlock/hwspinlock_core.c b/drivers/hwspinlock/hwspinlock_core.c
index 4074441444fe..a41bc79aaa15 100644
--- a/drivers/hwspinlock/hwspinlock_core.c
+++ b/drivers/hwspinlock/hwspinlock_core.c
@@ -1,18 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Hardware spinlock framework
*
* Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
*
* Contact: Ohad Ben-Cohen <ohad@wizery.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 in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
*/
#define pr_fmt(fmt) "%s: " fmt, __func__
diff --git a/drivers/hwspinlock/hwspinlock_internal.h b/drivers/hwspinlock/hwspinlock_internal.h
index d26f78b8f214..9eb6bd020dc7 100644
--- a/drivers/hwspinlock/hwspinlock_internal.h
+++ b/drivers/hwspinlock/hwspinlock_internal.h
@@ -1,18 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
/*
* Hardware spinlocks internal header
*
* Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
*
* Contact: Ohad Ben-Cohen <ohad@wizery.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 in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
*/
#ifndef __HWSPINLOCK_HWSPINLOCK_H
diff --git a/include/linux/hwspinlock.h b/include/linux/hwspinlock.h
index 859d673d98c8..cd6cd7fbd3c9 100644
--- a/include/linux/hwspinlock.h
+++ b/include/linux/hwspinlock.h
@@ -1,18 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
/*
* Hardware spinlock public header
*
* Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
*
* Contact: Ohad Ben-Cohen <ohad@wizery.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 in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
*/
#ifndef __LINUX_HWSPINLOCK_H
--
2.17.0
^ permalink raw reply related
* [PATCH 2/6] hwspinlock/omap: Switch to SPDX license identifier
From: Suman Anna @ 2018-05-11 17:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511170321.2452-1-s-anna@ti.com>
Use the appropriate SPDX license identifier in the OMAP hwspinlock
driver source file and drop the previous boilerplate license text.
Signed-off-by: Suman Anna <s-anna@ti.com>
---
drivers/hwspinlock/omap_hwspinlock.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/hwspinlock/omap_hwspinlock.c b/drivers/hwspinlock/omap_hwspinlock.c
index ad2f8cac8487..79a4030e89d8 100644
--- a/drivers/hwspinlock/omap_hwspinlock.c
+++ b/drivers/hwspinlock/omap_hwspinlock.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* OMAP hardware spinlock driver
*
@@ -6,15 +7,6 @@
* Contact: Simon Que <sque@ti.com>
* Hari Kanigeri <h-kanigeri2@ti.com>
* Ohad Ben-Cohen <ohad@wizery.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 in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * General Public License for more details.
*/
#include <linux/kernel.h>
--
2.17.0
^ permalink raw reply related
* [PATCH 3/6] hwspinlock: qcom: Switch to SPDX license identifier
From: Suman Anna @ 2018-05-11 17:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511170321.2452-1-s-anna@ti.com>
Use the appropriate SPDX license identifier in the Qualcomm Hwspinlock
driver source file and drop the previous boilerplate license text.
Signed-off-by: Suman Anna <s-anna@ti.com>
---
drivers/hwspinlock/qcom_hwspinlock.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/hwspinlock/qcom_hwspinlock.c b/drivers/hwspinlock/qcom_hwspinlock.c
index fa6880b8060a..6da7447d277d 100644
--- a/drivers/hwspinlock/qcom_hwspinlock.c
+++ b/drivers/hwspinlock/qcom_hwspinlock.c
@@ -1,15 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (c) 2013, The Linux Foundation. All rights reserved.
* Copyright (c) 2015, Sony Mobile Communications AB
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
*/
#include <linux/hwspinlock.h>
--
2.17.0
^ permalink raw reply related
* [PATCH 4/6] hwspinlock/sirf: Switch to SPDX license identifier
From: Suman Anna @ 2018-05-11 17:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511170321.2452-1-s-anna@ti.com>
Use the appropriate SPDX license identifier in the CSR's SIRF hardware
spinlock driver source file and drop the previous boilerplate license text.
Cc: Wei Chen <wei.chen@csr.com>
Cc: Barry Song <baohua@kernel.org>
Signed-off-by: Suman Anna <s-anna@ti.com>
---
drivers/hwspinlock/sirf_hwspinlock.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/hwspinlock/sirf_hwspinlock.c b/drivers/hwspinlock/sirf_hwspinlock.c
index 16018544d431..02ef6e543cd1 100644
--- a/drivers/hwspinlock/sirf_hwspinlock.c
+++ b/drivers/hwspinlock/sirf_hwspinlock.c
@@ -1,9 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* SIRF hardware spinlock driver
*
* Copyright (c) 2015 Cambridge Silicon Radio Limited, a CSR plc group company.
- *
- * Licensed under GPLv2.
*/
#include <linux/kernel.h>
--
2.17.0
^ permalink raw reply related
* [PATCH 5/6] hwspinlock/u8500: Switch to SPDX license identifier
From: Suman Anna @ 2018-05-11 17:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511170321.2452-1-s-anna@ti.com>
Use the appropriate SPDX license identifier in the U8500 HWSEM
driver source file and drop the previous boilerplate license text.
Cc: Mathieu J. Poirier <mathieu.poirier@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Suman Anna <s-anna@ti.com>
---
drivers/hwspinlock/u8500_hsem.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/hwspinlock/u8500_hsem.c b/drivers/hwspinlock/u8500_hsem.c
index e93eabbd660f..84a92d424e8d 100644
--- a/drivers/hwspinlock/u8500_hsem.c
+++ b/drivers/hwspinlock/u8500_hsem.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* u8500 HWSEM driver
*
@@ -10,15 +11,6 @@
* Simon Que <sque@ti.com>
* Hari Kanigeri <h-kanigeri2@ti.com>
* Ohad Ben-Cohen <ohad@wizery.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 in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * General Public License for more details.
*/
#include <linux/module.h>
--
2.17.0
^ permalink raw reply related
* [PATCH 6/6] hwspinlock: sprd: Switch to SPDX license identifier
From: Suman Anna @ 2018-05-11 17:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511170321.2452-1-s-anna@ti.com>
Use the appropriate SPDX license identifiers in the Spreadtrum hardware
spinlock driver source file and drop the previous boilerplate license text.
Cc: Baolin Wang <baolin.wang@spreadtrum.com>
Signed-off-by: Suman Anna <s-anna@ti.com>
---
drivers/hwspinlock/sprd_hwspinlock.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/hwspinlock/sprd_hwspinlock.c b/drivers/hwspinlock/sprd_hwspinlock.c
index 638e64ac18f5..dc42bf51f3e6 100644
--- a/drivers/hwspinlock/sprd_hwspinlock.c
+++ b/drivers/hwspinlock/sprd_hwspinlock.c
@@ -1,15 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Spreadtrum hardware spinlock driver
* Copyright (C) 2017 Spreadtrum - http://www.spreadtrum.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 in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * General Public License for more details.
*/
#include <linux/bitops.h>
--
2.17.0
^ permalink raw reply related
* [PATCH v9 03/11] arm64: kexec_file: invoke the kernel without purgatory
From: James Morse @ 2018-05-11 17:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180507052208.GD11326@linaro.org>
Hi Akashi,
On 07/05/18 06:22, AKASHI Takahiro wrote:
> On Tue, May 01, 2018 at 06:46:06PM +0100, James Morse wrote:
>> On 25/04/18 07:26, AKASHI Takahiro wrote:
>>> diff --git a/arch/arm64/kernel/machine_kexec.c b/arch/arm64/kernel/machine_kexec.c
>>> index f76ea92dff91..f7dbba00be10 100644
>>> --- a/arch/arm64/kernel/machine_kexec.c
>>> +++ b/arch/arm64/kernel/machine_kexec.c
>>> @@ -205,10 +205,17 @@ void machine_kexec(struct kimage *kimage)
>>> cpu_soft_restart(kimage != kexec_crash_image,
>>> - reboot_code_buffer_phys, kimage->head, kimage->start, 0);
>>> + reboot_code_buffer_phys, kimage->head, kimage->start,
>>> +#ifdef CONFIG_KEXEC_FILE
>>> + kimage->purgatory_info.purgatory_buf ?
>>> + 0 : kimage->arch.dtb_mem);
>>> +#else
>>> + 0);
>>> +#endif
>> purgatory_buf seems to only be set in kexec_purgatory_setup_kbuf(), called from
>> kexec_load_purgatory(), which we don't use. How does this get a value?
>>
>> Would it be better to always use kimage->arch.dtb_mem, and ensure that is 0 for
>> regular kexec (as we can't know where the dtb is)? (image_arg may then be a
>> better name).
>
> The problem is arch.dtb_mem is currently defined only if CONFIG_KEXEC_FILE.
I thought it was ARCH_HAS_KIMAGE_ARCH, which we can define all the time if
that's what we want.
> So I would like to
> - merge this patch with patch#8
> - change the condition
> #ifdef CONFIG_KEXEC_FILE
> kimage->file_mode ? kimage->arch.dtb_mem : 0);
> #else
> 0);
> #endif
If we can avoid even this #ifdef by always having kimage->arch, I'd prefer that.
If we do that 'dtb_mem' would need some thing that indicates its for kexec_file,
as kexec has a DTB too, we just don't know where it is...
Thanks,
James
^ permalink raw reply
* [PATCH v9 06/11] arm64: kexec_file: allow for loading Image-format kernel
From: James Morse @ 2018-05-11 17:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180507072139.GF11326@linaro.org>
Hi Akashi,
On 07/05/18 08:21, AKASHI Takahiro wrote:
> On Tue, May 01, 2018 at 06:46:11PM +0100, James Morse wrote:
>> On 25/04/18 07:26, AKASHI Takahiro wrote:
>>> This patch provides kexec_file_ops for "Image"-format kernel. In this
>>> implementation, a binary is always loaded with a fixed offset identified
>>> in text_offset field of its header.
>>> diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
>>> index e4de1223715f..3cba4161818a 100644
>>> --- a/arch/arm64/include/asm/kexec.h
>>> +++ b/arch/arm64/include/asm/kexec.h
>>> @@ -102,6 +102,56 @@ struct kimage_arch {
>>> void *dtb_buf;
>>> };
>>>
>>> +/**
>>> + * struct arm64_image_header - arm64 kernel image header
>>> + *
>>> + * @pe_sig: Optional PE format 'MZ' signature
>>> + * @branch_code: Instruction to branch to stext
>>> + * @text_offset: Image load offset, little endian
>>> + * @image_size: Effective image size, little endian
>>> + * @flags:
>>> + * Bit 0: Kernel endianness. 0=little endian, 1=big endian
>>
>> Page size? What about 'phys_base'?, (whatever that is...)
>> Probably best to refer to Documentation/arm64/booting.txt here, its the
>> authoritative source of what these fields mean.
>
> While we don't care other bit fields for now, I will add the reference
> to the Documentation file.
Thanks, I don't want to create a second, incomplete set of documentation!
>>> + u64 reserved[3];
>>> + u8 magic[4];
>>> + u32 pe_header;
>>> +};
>>
>> I'm surprised we don't have a definition for this already, I guess its always
>> done in asm. We have kernel/image.h that holds some of this stuff, if we are
>> going to validate the flags, is it worth adding the code there, (and moving it
>> to include/asm)?
>
> A comment at the beginning of this file says,
> #ifndef LINKER_SCRIPT
> #error This file should only be included in vmlinux.lds.S
> #endif
> Let me think about.
Ah, I missed that.
Having two definitions of something makes me nervous that they can become
different... looks like that header belongs to the linker, and shouldn't be used
here then.
>> I guess you skip the MZ prefix as its not present for !EFI?
>
> CONFIG_KEXEC_IMAGE_VERIFY_SIG depends on the fact that the file
> format is PE (that is, EFI is enabled).
So if the signature checking is enabled, its already been checked.
>> Could we check branch_code is non-zero, and text-offset points within image-size?
>
> We could do it, but I don't think this check is very useful.
>
>>
>> We could check that this platform supports the page-size/endian config that this
>> Image was built with... We get a message from the EFI stub if the page-size
>> can't be supported, it would be nice to do the same here (as we can).
>
> There is no restriction on page-size or endianness for kexec.
No, but it won't boot if the hardware doesn't support it. The kernel will spin
at a magic address that is, difficult, to debug without JTAG. The bug report
will be "it didn't boot".
> What will be the purpose of this check?
These values are in the header so that the bootloader can check them, then print
a meaningful error. Here, kexec_file_load() is playing the part of the bootloader.
I'm assuming kexec_file_load() can only be used to kexec linux... unlike regular
kexec. Is this where I'm going wrong?
>>> diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
>>> new file mode 100644
>>> index 000000000000..4dd524ad6611
>>> --- /dev/null
>>> +++ b/arch/arm64/kernel/kexec_image.c
>>> @@ -0,0 +1,79 @@
>>
>>> +static void *image_load(struct kimage *image,
>>> + char *kernel, unsigned long kernel_len,
>>> + char *initrd, unsigned long initrd_len,
>>> + char *cmdline, unsigned long cmdline_len)
>>> +{
>>> + struct kexec_buf kbuf;
>>> + struct arm64_image_header *h = (struct arm64_image_header *)kernel;
>>> + unsigned long text_offset;
>>> + int ret;
>>> +
>>> + /* Load the kernel */
>>> + kbuf.image = image;
>>> + kbuf.buf_min = 0;
>>> + kbuf.buf_max = ULONG_MAX;
>>> + kbuf.top_down = false;
>>> +
>>> + kbuf.buffer = kernel;
>>> + kbuf.bufsz = kernel_len;
>>> + kbuf.memsz = le64_to_cpu(h->image_size);
>>> + text_offset = le64_to_cpu(h->text_offset);
>>> + kbuf.buf_align = SZ_2M;
>>
>>> + /* Adjust kernel segment with TEXT_OFFSET */
>>> + kbuf.memsz += text_offset;
>>> +
>>> + ret = kexec_add_buffer(&kbuf);
>>> + if (ret)
>>> + goto out;
>>> +
>>> + image->arch.kern_segment = image->nr_segments - 1;
>>
>> You only seem to use kern_segment here, and in load_other_segments() called
>> below. Could it not be a local variable passed in? Instead of arch-specific data
>> we keep forever?
>
> No, kern_segment is also used in load_other_segments() in machine_kexec_file.c.
> To optimize memory hole allocation logic in locate_mem_hole_callback(),
> we need to know the exact range of kernel image (start and end).
That's the second user. My badly-made point is one calls the other, but passes
the data via some until-kexec lifetime struct. (its not important, just an
indicator this worked differently in the past and hasn't been cleaned up).
I meant something like [0].
Thanks,
James
[0] a diff is worth a thousand words:
--------------------%<--------------------
diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_
kexec_file.c
index 762f9102899c..c50ce844f09e 100644
--- a/arch/arm64/kernel/machine_kexec_file.c
+++ b/arch/arm64/kernel/machine_kexec_file.c
@@ -325,11 +325,10 @@ static int prepare_elf_headers(void **addr, unsigned long *sz)
return ret;
}
-int load_other_segments(struct kimage *image,
+int load_other_segments(struct kimage *image, struct kexec_segment *kern_seg,
char *initrd, unsigned long initrd_len,
char *cmdline, unsigned long cmdline_len)
{
- struct kexec_segment *kern_seg;
struct kexec_buf kbuf;
void *hdrs_addr;
unsigned long hdrs_sz;
@@ -368,7 +367,6 @@ int load_other_segments(struct kimage *image,
image->arch.elf_load_addr, hdrs_sz, hdrs_sz);
}
- kern_seg = &image->segment[image->arch.kern_segment];
kbuf.image = image;
/* not allocate anything below the kernel */
kbuf.buf_min = kern_seg->mem + kern_seg->memsz;
diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index 891f2484969d..085cb69293ca 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -173,8 +172,10 @@ static inline int arm64_header_check_pe_sig(const struct ar
m64_image_header *h)
extern const struct kexec_file_ops kexec_image_ops;
struct kimage;
+struct kexec_segment;
extern int load_other_segments(struct kimage *image,
+ struct kexec_segment *kern_seg,
char *initrd, unsigned long initrd_len,
char *cmdline, unsigned long cmdline_len);
#endif
diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
index 7c11beefe65f..0e032d30a79c 100644
--- a/arch/arm64/kernel/kexec_image.c
+++ b/arch/arm64/kernel/kexec_image.c
@@ -37,6 +37,7 @@ static void *image_load(struct kimage *image,
char *cmdline, unsigned long cmdline_len)
{
struct kexec_buf kbuf;
+ struct kexec_segment *kern_seg;
struct arm64_image_header *h = (struct arm64_image_header *)kernel;
unsigned long text_offset;
int ret;
@@ -65,17 +66,17 @@ static void *image_load(struct kimage *image,
if (ret)
goto out;
- image->arch.kern_segment = image->nr_segments - 1;
- image->segment[image->arch.kern_segment].mem += text_offset;
- image->segment[image->arch.kern_segment].memsz -= text_offset;
- image->start = image->segment[image->arch.kern_segment].mem;
+ kern_seg = &image->segment[image->nr_segments - 1];
+ kern_seg->mem += text_offset;
+ kern_seg->memsz -= text_offset;
+ image->start = kern_seg->mem;
pr_debug("Loaded kernel at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
- image->segment[image->arch.kern_segment].mem,
+ kern_seg->mem,
kbuf.bufsz, kbuf.memsz);
/* Load additional data */
- ret = load_other_segments(image, initrd, initrd_len,
+ ret = load_other_segments(image, kern_seg, initrd, initrd_len,
cmdline, cmdline_len);
out:
--------------------%<--------------------
^ permalink raw reply related
* [GIT PULL] arm64: fixes for -rc5
From: Will Deacon @ 2018-05-11 17:30 UTC (permalink / raw)
To: linux-arm-kernel
Hi Linus,
Please pull these arm64 fixes for -rc5. There's a small memblock accounting
problem when freeing the initrd and a Spectre-v2 mitigation for NVIDIA Denver
CPUs which just requires a match on the CPU ID register.
Thanks,
Will
--->8
The following changes since commit 6da6c0db5316275015e8cc2959f12a17584aeb64:
Linux v4.17-rc3 (2018-04-29 14:17:42 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git tags/arm64-fixes
for you to fetch changes up to 0583a4ef05987f7e0f3a7bdd3365e5dc36ca306d:
arm64: capabilities: Add NVIDIA Denver CPU to bp_harden list (2018-05-09 14:28:28 +0100)
----------------------------------------------------------------
arm64 fixes:
- Mitigate Spectre-v2 for NVIDIA Denver CPUs
- Free memblocks corresponding to freed initrd area
----------------------------------------------------------------
CHANDAN VN (1):
arm64: To remove initrd reserved area entry from memblock
David Gilhooley (2):
arm64: Add MIDR encoding for NVIDIA CPUs
arm64: capabilities: Add NVIDIA Denver CPU to bp_harden list
arch/arm64/include/asm/cputype.h | 6 ++++++
arch/arm64/kernel/cpu_errata.c | 1 +
arch/arm64/mm/init.c | 4 +++-
3 files changed, 10 insertions(+), 1 deletion(-)
^ permalink raw reply
* [PATCH 3/3] iommu: armsmmu: set iommu ops for rpmsg bus
From: Robin Murphy @ 2018-05-11 18:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOCOHw4RDwmbCySN==-zT7cQ6abzLzKUhBHBSdv_EvWnU4g4Pg@mail.gmail.com>
On 07/05/18 20:28, Bjorn Andersson wrote:
> On Fri, Mar 2, 2018 at 8:59 AM, Robin Murphy <robin.murphy@arm.com> wrote:
>> On 02/03/18 14:55, srinivas.kandagatla at linaro.org wrote:
>>>
>>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>>
>>> On Qualcomm SoCs, ADSP exposes many functions like audio and
>>> others. These services need iommu access to allocate any
>>> memory for the DSP. As these drivers are childeren of
>>> rpmsg bus, able to allocate memory from iommus is basic
>>> requirement. So set arm smmu iommu ops for this bus type.
>>
>
> Forgot to answer this and the dma_ops patch seems to be going in the
> right direction.
>
>>
>> Documentation/rpmsg.txt: "Every rpmsg device is a communication channel with
>> a remote processor (thus rpmsg devices are called channels)."
>>
>> I'd instinctively assume that a remote processor already has its own memory,
>> and that a communication channel doesn't somehow go directly through an
>> IOMMU, so that "basic requirement" seems like a pretty big assumption.
>>
>
> As of today rpmsg exclusively uses system memory for implementing the
> communication fifos, but this memory is owned/handled by the rpmsg
> bus. The need here is for drivers on top of the rpmsg_bus,
> implementing some application-level protocol that requires indirection
> buffers; e.g. to achieve zero copy of audio or image buffers that the
> remote processor is expected to operate on. In this case the device
> sitting on top of the rpmsg bus will have to map the buffer to the
> appropriate context and can then send application specific control
> requests referencing this mapping.
Right, but that's more or less what I was getting at - rpmsg can be used
as a means to signal some DMA master device to start doing a thing, but
that thing itself is unrelated to rpmsg, and it by no means implies that
everything which rpmsg can talk to is always capable of system-wide DMA.
It's no different if that communication channel is a hardware mailbox or
an I2C/SPI/USB/etc. link, rather than virtio; we wouldn't automatically
consider devices on the other end of those to be directly connected to
an IOMMU either.
IOMMU and DMA operations are highly dependent on the physical hardware
topology, which is why I really don't like trying to shoehorn them into
software constructs without modelling the actual hardware reasonably
accurately. For instance it's not unheard of for remote processors in a
SoC to see a different physical memory map from the main application
processors - how would rpmsg try to describe that? What even is the
address space of the rpmsg "bus"?
> As different parts of the firmware might operate in different contexts
> it's not feasible to utilize the parent's (the rpmsg_bus) context for
> these indirection buffers.
Indeed, and I maintain that that wouldn't be the right thing to do
anyway. As before, I think the most accurate way to model the situation
with the tools we have available is to have the actual hardware function
represented by a platform device, which is associated with a
corresponding rpmsg endpoint. Then the driver can manage communication
in the rpmsg context, and physical DMA setup in the 'real' hardware
context, and everything looks sane without questionable abstraction
breakage. Since this looked to be more or less what is actually
implemented anyway, it doesn't seem all that hard to refine; if there
are multiple DMA master functions identified distinctly to the IOMMU,
then they could either be represented as separate platform devices with
explicit IOMMU specifiers, or you could model the actual DSP subsystem
hardware as its own bus-like arrangement with an iommu-map arrangement
translating function identifiers to IOMMU identifiers.
What I don't like is forcing IOMMU drivers to pretend that some data in
a shared memory buffer is itself directly capable of generating
transactions on the interconnect. If other 'indirect' bus abstractions
like CoreSight can get this right, I don't see why rpmsg deserves to be
special.
>>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>> ---
>>> drivers/iommu/arm-smmu.c | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>>> index e6920d32ac9e..9b63489af15c 100644
>>> --- a/drivers/iommu/arm-smmu.c
>>> +++ b/drivers/iommu/arm-smmu.c
>>> @@ -53,6 +53,7 @@
>>> #include <linux/spinlock.h>
>>> #include <linux/amba/bus.h>
>>> +#include <linux/rpmsg.h>
>>> #include "io-pgtable.h"
>>> #include "arm-smmu-regs.h"
>>> @@ -2168,6 +2169,10 @@ static void arm_smmu_bus_init(void)
>>> bus_set_iommu(&pci_bus_type, &arm_smmu_ops);
>>> }
>>> #endif
>>> +#ifdef CONFIG_RPMSG
>>
>>
>> Ah, so this will at least build OK with RPMSG=m, but I doubt it does what
>> you want it to in that case.
>>
>
> Things have been refactored but the core has remained tristate,
> causing extra head aches in various areas. I think it's very
> reasonable to review the rpmsg config options and make CONFIG_RPMSG
> bool.
>
> So with the addition of making CONFIG_RPMSG bool the patch has my Acked-by.
>
> That said I'm generally concerned about the first probed iommu
> implementation assigning itself as the sole iommu implementation for
> all busses, but I guess we haven't yet hit the point where there are
> different iommu implementations in a single SoC?
As it happens I do know of one such SoC already - Rockchip RK3288 seems
to have an undocumented Arm SMMU alongside all the rockchip-iommu
instances, but it's not used by Linux (and I have no idea what it was
intended for; I just went and poked the intriguing "peripheral MMU"
region of the memory map and found what looks an awful lot like an Arm
MMU-400). More realistically, I also know of folks using the Arm Juno
dev board with an MMU-600 in the add-on FPGA tile, which would have that
driver-probe-order fight with the MMU-401 instances in the SoC, but I
figure they were either using an older firmware which didn't enable the
latter or just got lucky with not having the SMMUv2 driver enabled.
But yes, the per-bus ops thing is awful and I've been complaining about
it for years now. Since iommu_fwspec we at least have the foundations
for per-device ops in place now, but as is often the case, getting 80%
of the way there is simple[1], whilst the last 20% (like replacing
iommu_domain_alloc(), and where to call iommu_{add,remove}_device()
from) is really hard.
Robin.
[1]
https://www.mail-archive.com/iommu at lists.linux-foundation.org/msg14576.html
^ permalink raw reply
* [PATCH v2 00/40] Shared Virtual Addressing for the IOMMU
From: Jean-Philippe Brucker @ 2018-05-11 19:06 UTC (permalink / raw)
To: linux-arm-kernel
This is version 2 of the Shared Virtual Addressing (SVA) series, which
adds process address space management (1-6) and I/O page faults (7-9) to
the IOMMU core. It also includes two example users of the API: VFIO as
device driver (10-13), and Arm SMMUv3 as IOMMU driver (14-40).
The series is getting bulky and although it will remove lots of context,
I'll probably split next version into IOMMU, VFIO and SMMU changes. This
time around I also Cc'd the mm list even though get_maintainers doesn't
pick it up, because I'd like to export a few symbols in patches 10-12
and because of the recent interest in sharing mm with devices [1], which
seems somewhat related.
Major changes since v1 [2]:
* Removed bind_group(). Only bind_device() is supported. Supporting
multi-device groups is difficult and unnecessary at the moment.
* Reworked the I/O Page Fault code to support multiple queues. IOMMU
driver now registers IOPF queues as needed (for example one per IOMMU
device), then attach devices that require IOPF (for example, in
sva_device_init)
* mm_exit callback is allowed to sleep. It is now registered with
init_sva_device_init() instead of a separate function.
* Remove IOMMU_SVA_FEAT_PASID, making PASID support mandatory for now.
If a future series implements !PASID, it will have to introduce flag
IOMMU_SVA_FEAT_NO_PASID.
* Removed the atomic/blocking handler distinction for now. It is a bit
irrelevant here and can be added back later.
* Tried to address all comments. Please let me know if I missed
something.
The series relies on Jacob Pan's fault reporting work, currently under
discussion [3].
You can pull everything from:
git://linux-arm.org/linux-jpb.git sva/v2
Branch sva/debug contains tracepoints and other tools that I found
useful for validation.
[1] https://lwn.net/Articles/753481/
[2] https://www.mail-archive.com/iommu at lists.linux-foundation.org/msg21813.html
[3] IOMMU and VT-d driver support for Shared Virtual Address (SVA)
https://www.mail-archive.com/iommu at lists.linux-foundation.org/msg22640.html
---
If you're unfamiliar with SVM/SVA, a wordy description copied from
previous version follows.
Shared Virtual Addressing (SVA) is the ability to share process address
spaces with devices. It is called "SVM" (Shared Virtual Memory) by OpenCL
and some IOMMU architectures, but since that abbreviation is already used
for AMD virtualisation in Linux (Secure Virtual Machine), we prefer the
less ambiguous "SVA".
Sharing process address spaces with devices allows to rely on core kernel
memory management for DMA, removing some complexity from application and
device drivers. After binding to a device, applications can instruct it to
perform DMA on buffers obtained with malloc.
The device, bus and IOMMU must support the following features:
* Multiple address spaces per device, for example using the PCI PASID
(Process Address Space ID) extension. The IOMMU driver allocates a
PASID and the device uses it in DMA transactions.
* I/O Page Faults (IOPF), for example PCI PRI (Page Request Interface) or
Arm SMMU stall. The core mm handles translation faults from the IOMMU.
* MMU and IOMMU implement compatible page table formats.
This series requires to support all three features. Upcoming patches will
enable private PASID management, which doesn't share page tables and
augments the map/unmap API with PASIDs.
Although we don't have any performance measurement at the moment, SVA will
likely be slower than classical DMA since it relies on page faults,
whereas classical DMA pins all pages in memory. SVA mostly aims at
simplifying DMA management, but also improves security by isolating
address spaces in devices.
Jean-Philippe Brucker (40):
iommu: Introduce Shared Virtual Addressing API
iommu/sva: Bind process address spaces to devices
iommu/sva: Manage process address spaces
iommu/sva: Add a mm_exit callback for device drivers
iommu/sva: Track mm changes with an MMU notifier
iommu/sva: Search mm by PASID
iommu: Add a page fault handler
iommu/iopf: Handle mm faults
iommu/sva: Register page fault handler
mm: export symbol mm_access
mm: export symbol find_get_task_by_vpid
mm: export symbol mmput_async
vfio: Add support for Shared Virtual Addressing
dt-bindings: document stall and PASID properties for IOMMU masters
iommu/of: Add stall and pasid properties to iommu_fwspec
arm64: mm: Pin down ASIDs for sharing mm with devices
iommu/arm-smmu-v3: Link domains and devices
iommu/io-pgtable-arm: Factor out ARM LPAE register defines
iommu: Add generic PASID table library
iommu/arm-smmu-v3: Move context descriptor code
iommu/arm-smmu-v3: Add support for Substream IDs
iommu/arm-smmu-v3: Add second level of context descriptor table
iommu/arm-smmu-v3: Share process page tables
iommu/arm-smmu-v3: Seize private ASID
iommu/arm-smmu-v3: Add support for VHE
iommu/arm-smmu-v3: Enable broadcast TLB maintenance
iommu/arm-smmu-v3: Add SVA feature checking
iommu/arm-smmu-v3: Implement mm operations
iommu/arm-smmu-v3: Add support for Hardware Translation Table Update
iommu/arm-smmu-v3: Register I/O Page Fault queue
iommu/arm-smmu-v3: Improve add_device error handling
iommu/arm-smmu-v3: Maintain a SID->device structure
iommu/arm-smmu-v3: Add stall support for platform devices
ACPI/IORT: Check ATS capability in root complex nodes
iommu/arm-smmu-v3: Add support for PCI ATS
iommu/arm-smmu-v3: Hook up ATC invalidation to mm ops
iommu/arm-smmu-v3: Disable tagged pointers
PCI: Make "PRG Response PASID Required" handling common
iommu/arm-smmu-v3: Add support for PRI
iommu/arm-smmu-v3: Add support for PCI PASID
.../devicetree/bindings/iommu/iommu.txt | 24 +
MAINTAINERS | 6 +-
arch/arm64/include/asm/mmu.h | 1 +
arch/arm64/include/asm/mmu_context.h | 11 +-
arch/arm64/mm/context.c | 92 +-
drivers/acpi/arm64/iort.c | 11 +
drivers/iommu/Kconfig | 29 +
drivers/iommu/Makefile | 4 +
drivers/iommu/amd_iommu.c | 19 +-
drivers/iommu/arm-smmu-v3-context.c | 700 ++++++++
drivers/iommu/arm-smmu-v3.c | 1438 ++++++++++++++---
drivers/iommu/io-pgfault.c | 445 +++++
drivers/iommu/io-pgtable-arm.c | 49 +-
drivers/iommu/io-pgtable-arm.h | 54 +
drivers/iommu/iommu-pasid-table.c | 52 +
drivers/iommu/iommu-pasid-table.h | 177 ++
drivers/iommu/iommu-sva.c | 792 +++++++++
drivers/iommu/iommu.c | 84 +
drivers/iommu/of_iommu.c | 12 +
drivers/pci/ats.c | 17 +
drivers/vfio/vfio_iommu_type1.c | 449 ++++-
include/linux/iommu.h | 184 +++
include/linux/pci-ats.h | 8 +
include/uapi/linux/pci_regs.h | 1 +
include/uapi/linux/vfio.h | 76 +
kernel/fork.c | 15 +
kernel/pid.c | 1 +
27 files changed, 4469 insertions(+), 282 deletions(-)
create mode 100644 drivers/iommu/arm-smmu-v3-context.c
create mode 100644 drivers/iommu/io-pgfault.c
create mode 100644 drivers/iommu/io-pgtable-arm.h
create mode 100644 drivers/iommu/iommu-pasid-table.c
create mode 100644 drivers/iommu/iommu-pasid-table.h
create mode 100644 drivers/iommu/iommu-sva.c
--
2.17.0
^ permalink raw reply
* [PATCH v2 01/40] iommu: Introduce Shared Virtual Addressing API
From: Jean-Philippe Brucker @ 2018-05-11 19:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511190641.23008-1-jean-philippe.brucker@arm.com>
Shared Virtual Addressing (SVA) provides a way for device drivers to bind
process address spaces to devices. This requires the IOMMU to support page
table format and features compatible with the CPUs, and usually requires
the system to support I/O Page Faults (IOPF) and Process Address Space ID
(PASID). When all of these are available, DMA can access virtual addresses
of a process. A PASID is allocated for each process, and the device driver
programs it into the device in an implementation-specific way.
Add a new API for sharing process page tables with devices. Introduce two
IOMMU operations, sva_device_init() and sva_device_shutdown(), that
prepare the IOMMU driver for SVA. For example allocate PASID tables and
fault queues. Subsequent patches will implement the bind() and unbind()
operations.
Support for I/O Page Faults will be added in a later patch using a new
feature bit (IOMMU_SVA_FEAT_IOPF). With the current API users must pin
down all shared mappings. Other feature bits that may be added in the
future are IOMMU_SVA_FEAT_PRIVATE, to support private PASID address
spaces, and IOMMU_SVA_FEAT_NO_PASID, to bind the whole device address
space to a process.
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
v1->v2:
* Add sva_param structure to iommu_param
* CONFIG option is only selectable by IOMMU drivers
---
drivers/iommu/Kconfig | 4 ++
drivers/iommu/Makefile | 1 +
drivers/iommu/iommu-sva.c | 110 ++++++++++++++++++++++++++++++++++++++
include/linux/iommu.h | 32 +++++++++++
4 files changed, 147 insertions(+)
create mode 100644 drivers/iommu/iommu-sva.c
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 7564237f788d..cca8e06903c7 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -74,6 +74,10 @@ config IOMMU_DMA
select IOMMU_IOVA
select NEED_SG_DMA_LENGTH
+config IOMMU_SVA
+ bool
+ select IOMMU_API
+
config FSL_PAMU
bool "Freescale IOMMU support"
depends on PCI
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 1fb695854809..1dbcc89ebe4c 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_IOMMU_API) += iommu.o
obj-$(CONFIG_IOMMU_API) += iommu-traces.o
obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o
obj-$(CONFIG_IOMMU_DMA) += dma-iommu.o
+obj-$(CONFIG_IOMMU_SVA) += iommu-sva.o
obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
obj-$(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) += io-pgtable-arm-v7s.o
obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
new file mode 100644
index 000000000000..8b4afb7c63ae
--- /dev/null
+++ b/drivers/iommu/iommu-sva.c
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Manage PASIDs and bind process address spaces to devices.
+ *
+ * Copyright (C) 2018 ARM Ltd.
+ */
+
+#include <linux/iommu.h>
+#include <linux/slab.h>
+
+/**
+ * iommu_sva_device_init() - Initialize Shared Virtual Addressing for a device
+ * @dev: the device
+ * @features: bitmask of features that need to be initialized
+ * @max_pasid: max PASID value supported by the device
+ *
+ * Users of the bind()/unbind() API must call this function to initialize all
+ * features required for SVA.
+ *
+ * The device must support multiple address spaces (e.g. PCI PASID). By default
+ * the PASID allocated during bind() is limited by the IOMMU capacity, and by
+ * the device PASID width defined in the PCI capability or in the firmware
+ * description. Setting @max_pasid to a non-zero value smaller than this limit
+ * overrides it.
+ *
+ * The device should not be performing any DMA while this function is running,
+ * otherwise the behavior is undefined.
+ *
+ * Return 0 if initialization succeeded, or an error.
+ */
+int iommu_sva_device_init(struct device *dev, unsigned long features,
+ unsigned int max_pasid)
+{
+ int ret;
+ struct iommu_sva_param *param;
+ struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
+
+ if (!domain || !domain->ops->sva_device_init)
+ return -ENODEV;
+
+ if (features)
+ return -EINVAL;
+
+ param = kzalloc(sizeof(*param), GFP_KERNEL);
+ if (!param)
+ return -ENOMEM;
+
+ param->features = features;
+ param->max_pasid = max_pasid;
+
+ /*
+ * IOMMU driver updates the limits depending on the IOMMU and device
+ * capabilities.
+ */
+ ret = domain->ops->sva_device_init(dev, param);
+ if (ret)
+ goto err_free_param;
+
+ mutex_lock(&dev->iommu_param->lock);
+ if (dev->iommu_param->sva_param)
+ ret = -EEXIST;
+ else
+ dev->iommu_param->sva_param = param;
+ mutex_unlock(&dev->iommu_param->lock);
+ if (ret)
+ goto err_device_shutdown;
+
+ return 0;
+
+err_device_shutdown:
+ if (domain->ops->sva_device_shutdown)
+ domain->ops->sva_device_shutdown(dev, param);
+
+err_free_param:
+ kfree(param);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_sva_device_init);
+
+/**
+ * iommu_sva_device_shutdown() - Shutdown Shared Virtual Addressing for a device
+ * @dev: the device
+ *
+ * Disable SVA. Device driver should ensure that the device isn't performing any
+ * DMA while this function is running.
+ */
+int iommu_sva_device_shutdown(struct device *dev)
+{
+ struct iommu_sva_param *param;
+ struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
+
+ if (!domain)
+ return -ENODEV;
+
+ mutex_lock(&dev->iommu_param->lock);
+ param = dev->iommu_param->sva_param;
+ dev->iommu_param->sva_param = NULL;
+ mutex_unlock(&dev->iommu_param->lock);
+ if (!param)
+ return -ENODEV;
+
+ if (domain->ops->sva_device_shutdown)
+ domain->ops->sva_device_shutdown(dev, param);
+
+ kfree(param);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(iommu_sva_device_shutdown);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 0933f726d2e6..2efe7738bedb 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -212,6 +212,12 @@ struct page_response_msg {
u64 private_data;
};
+struct iommu_sva_param {
+ unsigned long features;
+ unsigned int min_pasid;
+ unsigned int max_pasid;
+};
+
/**
* struct iommu_ops - iommu ops and capabilities
* @capable: check capability
@@ -219,6 +225,8 @@ struct page_response_msg {
* @domain_free: free iommu domain
* @attach_dev: attach device to an iommu domain
* @detach_dev: detach device from an iommu domain
+ * @sva_device_init: initialize Shared Virtual Adressing for a device
+ * @sva_device_shutdown: shutdown Shared Virtual Adressing for a device
* @map: map a physically contiguous memory region to an iommu domain
* @unmap: unmap a physically contiguous memory region from an iommu domain
* @map_sg: map a scatter-gather list of physically contiguous memory chunks
@@ -256,6 +264,10 @@ struct iommu_ops {
int (*attach_dev)(struct iommu_domain *domain, struct device *dev);
void (*detach_dev)(struct iommu_domain *domain, struct device *dev);
+ int (*sva_device_init)(struct device *dev,
+ struct iommu_sva_param *param);
+ void (*sva_device_shutdown)(struct device *dev,
+ struct iommu_sva_param *param);
int (*map)(struct iommu_domain *domain, unsigned long iova,
phys_addr_t paddr, size_t size, int prot);
size_t (*unmap)(struct iommu_domain *domain, unsigned long iova,
@@ -413,6 +425,7 @@ struct iommu_fault_param {
* struct iommu_param - collection of per-device IOMMU data
*
* @fault_param: IOMMU detected device fault reporting data
+ * @sva_param: SVA parameters
*
* TODO: migrate other per device data pointers under iommu_dev_data, e.g.
* struct iommu_group *iommu_group;
@@ -421,6 +434,7 @@ struct iommu_fault_param {
struct iommu_param {
struct mutex lock;
struct iommu_fault_param *fault_param;
+ struct iommu_sva_param *sva_param;
};
int iommu_device_register(struct iommu_device *iommu);
@@ -920,4 +934,22 @@ static inline int iommu_sva_invalidate(struct iommu_domain *domain,
#endif /* CONFIG_IOMMU_API */
+#ifdef CONFIG_IOMMU_SVA
+extern int iommu_sva_device_init(struct device *dev, unsigned long features,
+ unsigned int max_pasid);
+extern int iommu_sva_device_shutdown(struct device *dev);
+#else /* CONFIG_IOMMU_SVA */
+static inline int iommu_sva_device_init(struct device *dev,
+ unsigned long features,
+ unsigned int max_pasid)
+{
+ return -ENODEV;
+}
+
+static inline int iommu_sva_device_shutdown(struct device *dev)
+{
+ return -ENODEV;
+}
+#endif /* CONFIG_IOMMU_SVA */
+
#endif /* __LINUX_IOMMU_H */
--
2.17.0
^ permalink raw reply related
* [PATCH v2 02/40] iommu/sva: Bind process address spaces to devices
From: Jean-Philippe Brucker @ 2018-05-11 19:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511190641.23008-1-jean-philippe.brucker@arm.com>
Add bind() and unbind() operations to the IOMMU API. Bind() returns a
PASID that drivers can program in hardware, to let their devices access an
mm. This patch only adds skeletons for the device driver API, most of the
implementation is still missing.
IOMMU groups with more than one device aren't supported for SVA at the
moment. There may be P2P traffic between devices within a group, which
cannot be seen by an IOMMU (note that supporting PASID doesn't add any
form of isolation with regard to P2P). Supporting groups would require
calling bind() for all bound processes every time a device is added to a
group, to perform sanity checks (e.g. ensure that new devices support
PASIDs at least as big as those already allocated in the group). It also
means making sure that reserved ranges (IOMMU_RESV_*) of all devices are
carved out of processes. This is already tricky with single devices, but
becomes very difficult with groups. Since SVA-capable devices are expected
to be cleanly isolated, and since we don't have any way to test groups or
hot-plug, we only allow singular groups for now.
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
v1->v2: remove iommu_sva_bind/unbind_group
---
drivers/iommu/iommu-sva.c | 27 +++++++++++++
drivers/iommu/iommu.c | 83 +++++++++++++++++++++++++++++++++++++++
include/linux/iommu.h | 37 +++++++++++++++++
3 files changed, 147 insertions(+)
diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
index 8b4afb7c63ae..8d98f9c09864 100644
--- a/drivers/iommu/iommu-sva.c
+++ b/drivers/iommu/iommu-sva.c
@@ -93,6 +93,8 @@ int iommu_sva_device_shutdown(struct device *dev)
if (!domain)
return -ENODEV;
+ __iommu_sva_unbind_dev_all(dev);
+
mutex_lock(&dev->iommu_param->lock);
param = dev->iommu_param->sva_param;
dev->iommu_param->sva_param = NULL;
@@ -108,3 +110,28 @@ int iommu_sva_device_shutdown(struct device *dev)
return 0;
}
EXPORT_SYMBOL_GPL(iommu_sva_device_shutdown);
+
+int __iommu_sva_bind_device(struct device *dev, struct mm_struct *mm,
+ int *pasid, unsigned long flags, void *drvdata)
+{
+ return -ENOSYS; /* TODO */
+}
+EXPORT_SYMBOL_GPL(__iommu_sva_bind_device);
+
+int __iommu_sva_unbind_device(struct device *dev, int pasid)
+{
+ return -ENOSYS; /* TODO */
+}
+EXPORT_SYMBOL_GPL(__iommu_sva_unbind_device);
+
+/**
+ * __iommu_sva_unbind_dev_all() - Detach all address spaces from this device
+ * @dev: the device
+ *
+ * When detaching @device from a domain, IOMMU drivers should use this helper.
+ */
+void __iommu_sva_unbind_dev_all(struct device *dev)
+{
+ /* TODO */
+}
+EXPORT_SYMBOL_GPL(__iommu_sva_unbind_dev_all);
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 9e28d88c8074..bd2819deae5b 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2261,3 +2261,86 @@ int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids)
return 0;
}
EXPORT_SYMBOL_GPL(iommu_fwspec_add_ids);
+
+/**
+ * iommu_sva_bind_device() - Bind a process address space to a device
+ * @dev: the device
+ * @mm: the mm to bind, caller must hold a reference to it
+ * @pasid: valid address where the PASID will be stored
+ * @flags: bond properties
+ * @drvdata: private data passed to the mm exit handler
+ *
+ * Create a bond between device and task, allowing the device to access the mm
+ * using the returned PASID. If unbind() isn't called first, a subsequent bind()
+ * for the same device and mm fails with -EEXIST.
+ *
+ * iommu_sva_device_init() must be called first, to initialize the required SVA
+ * features. @flags is a subset of these features.
+ *
+ * The caller must pin down using get_user_pages*() all mappings shared with the
+ * device. mlock() isn't sufficient, as it doesn't prevent minor page faults
+ * (e.g. copy-on-write).
+ *
+ * On success, 0 is returned and @pasid contains a valid ID. Otherwise, an error
+ * is returned.
+ */
+int iommu_sva_bind_device(struct device *dev, struct mm_struct *mm, int *pasid,
+ unsigned long flags, void *drvdata)
+{
+ int ret = -EINVAL;
+ struct iommu_group *group;
+
+ if (!pasid)
+ return -EINVAL;
+
+ group = iommu_group_get(dev);
+ if (!group)
+ return -ENODEV;
+
+ /* Ensure device count and domain don't change while we're binding */
+ mutex_lock(&group->mutex);
+ if (iommu_group_device_count(group) != 1)
+ goto out_unlock;
+
+ ret = __iommu_sva_bind_device(dev, mm, pasid, flags, drvdata);
+
+out_unlock:
+ mutex_unlock(&group->mutex);
+ iommu_group_put(group);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_sva_bind_device);
+
+/**
+ * iommu_sva_unbind_device() - Remove a bond created with iommu_sva_bind_device
+ * @dev: the device
+ * @pasid: the pasid returned by bind()
+ *
+ * Remove bond between device and address space identified by @pasid. Users
+ * should not call unbind() if the corresponding mm exited (as the PASID might
+ * have been reallocated for another process).
+ *
+ * The device must not be issuing any more transaction for this PASID. All
+ * outstanding page requests for this PASID must have been flushed to the IOMMU.
+ *
+ * Returns 0 on success, or an error value
+ */
+int iommu_sva_unbind_device(struct device *dev, int pasid)
+{
+ int ret = -EINVAL;
+ struct iommu_group *group;
+
+ group = iommu_group_get(dev);
+ if (!group)
+ return -ENODEV;
+
+ mutex_lock(&group->mutex);
+ ret = __iommu_sva_unbind_device(dev, pasid);
+ mutex_unlock(&group->mutex);
+
+ iommu_group_put(group);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_sva_unbind_device);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 2efe7738bedb..da59c20c4f12 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -613,6 +613,10 @@ void iommu_fwspec_free(struct device *dev);
int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids);
const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode);
+extern int iommu_sva_bind_device(struct device *dev, struct mm_struct *mm,
+ int *pasid, unsigned long flags, void *drvdata);
+extern int iommu_sva_unbind_device(struct device *dev, int pasid);
+
#else /* CONFIG_IOMMU_API */
struct iommu_ops {};
@@ -932,12 +936,29 @@ static inline int iommu_sva_invalidate(struct iommu_domain *domain,
return -EINVAL;
}
+static inline int iommu_sva_bind_device(struct device *dev,
+ struct mm_struct *mm, int *pasid,
+ unsigned long flags, void *drvdata)
+{
+ return -ENODEV;
+}
+
+static inline int iommu_sva_unbind_device(struct device *dev, int pasid)
+{
+ return -ENODEV;
+}
+
#endif /* CONFIG_IOMMU_API */
#ifdef CONFIG_IOMMU_SVA
extern int iommu_sva_device_init(struct device *dev, unsigned long features,
unsigned int max_pasid);
extern int iommu_sva_device_shutdown(struct device *dev);
+extern int __iommu_sva_bind_device(struct device *dev, struct mm_struct *mm,
+ int *pasid, unsigned long flags,
+ void *drvdata);
+extern int __iommu_sva_unbind_device(struct device *dev, int pasid);
+extern void __iommu_sva_unbind_dev_all(struct device *dev);
#else /* CONFIG_IOMMU_SVA */
static inline int iommu_sva_device_init(struct device *dev,
unsigned long features,
@@ -950,6 +971,22 @@ static inline int iommu_sva_device_shutdown(struct device *dev)
{
return -ENODEV;
}
+
+static inline int __iommu_sva_bind_device(struct device *dev,
+ struct mm_struct *mm, int *pasid,
+ unsigned long flags, void *drvdata)
+{
+ return -ENODEV;
+}
+
+static inline int __iommu_sva_unbind_device(struct device *dev, int pasid)
+{
+ return -ENODEV;
+}
+
+static inline void __iommu_sva_unbind_dev_all(struct device *dev)
+{
+}
#endif /* CONFIG_IOMMU_SVA */
#endif /* __LINUX_IOMMU_H */
--
2.17.0
^ permalink raw reply related
* [PATCH v2 03/40] iommu/sva: Manage process address spaces
From: Jean-Philippe Brucker @ 2018-05-11 19:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511190641.23008-1-jean-philippe.brucker@arm.com>
Allocate IOMMU mm structures and binding them to devices. Four operations
are added to IOMMU drivers:
* mm_alloc(): to create an io_mm structure and perform architecture-
specific operations required to grab the process (for instance on ARM,
pin down the CPU ASID so that the process doesn't get assigned a new
ASID on rollover).
There is a single valid io_mm structure per Linux mm. Future extensions
may also use io_mm for kernel-managed address spaces, populated with
map()/unmap() calls instead of bound to process address spaces. This
patch focuses on "shared" io_mm.
* mm_attach(): attach an mm to a device. The IOMMU driver checks that the
device is capable of sharing an address space, and writes the PASID
table entry to install the pgd.
Some IOMMU drivers will have a single PASID table per domain, for
convenience. Other can implement it differently but to help these
drivers, mm_attach and mm_detach take 'attach_domain' and
'detach_domain' parameters, that tell whether they need to set and clear
the PASID entry or only send the required TLB invalidations.
* mm_detach(): detach an mm from a device. The IOMMU driver removes the
PASID table entry and invalidates the IOTLBs.
* mm_free(): free a structure allocated by mm_alloc(), and let arch
release the process.
mm_attach and mm_detach operations are serialized with a spinlock. When
trying to optimize this code, we should at least prevent concurrent
attach()/detach() on the same domain (so multi-level PASID table code can
allocate tables lazily). mm_alloc() can sleep, but mm_free must not
(because we'll have to call it from call_srcu later on).
At the moment we use an IDR for allocating PASIDs and retrieving contexts.
We also use a single spinlock. These can be refined and optimized later (a
custom allocator will be needed for top-down PASID allocation).
Keeping track of address spaces requires the use of MMU notifiers.
Handling process exit with regard to unbind() is tricky, so it is left for
another patch and we explicitly fail mm_alloc() for the moment.
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
v1->v2: sanity-check of flags
---
drivers/iommu/iommu-sva.c | 380 +++++++++++++++++++++++++++++++++++++-
drivers/iommu/iommu.c | 1 +
include/linux/iommu.h | 28 +++
3 files changed, 406 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
index 8d98f9c09864..6ac679c48f3c 100644
--- a/drivers/iommu/iommu-sva.c
+++ b/drivers/iommu/iommu-sva.c
@@ -5,8 +5,298 @@
* Copyright (C) 2018 ARM Ltd.
*/
+#include <linux/idr.h>
#include <linux/iommu.h>
+#include <linux/sched/mm.h>
#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+/**
+ * DOC: io_mm model
+ *
+ * The io_mm keeps track of process address spaces shared between CPU and IOMMU.
+ * The following example illustrates the relation between structures
+ * iommu_domain, io_mm and iommu_bond. An iommu_bond is a link between io_mm and
+ * device. A device can have multiple io_mm and an io_mm may be bound to
+ * multiple devices.
+ * ___________________________
+ * | IOMMU domain A |
+ * | ________________ |
+ * | | IOMMU group | +------- io_pgtables
+ * | | | |
+ * | | dev 00:00.0 ----+------- bond --- io_mm X
+ * | |________________| \ |
+ * | '----- bond ---.
+ * |___________________________| \
+ * ___________________________ \
+ * | IOMMU domain B | io_mm Y
+ * | ________________ | / /
+ * | | IOMMU group | | / /
+ * | | | | / /
+ * | | dev 00:01.0 ------------ bond -' /
+ * | | dev 00:01.1 ------------ bond --'
+ * | |________________| |
+ * | +------- io_pgtables
+ * |___________________________|
+ *
+ * In this example, device 00:00.0 is in domain A, devices 00:01.* are in domain
+ * B. All devices within the same domain access the same address spaces. Device
+ * 00:00.0 accesses address spaces X and Y, each corresponding to an mm_struct.
+ * Devices 00:01.* only access address space Y. In addition each
+ * IOMMU_DOMAIN_DMA domain has a private address space, io_pgtable, that is
+ * managed with iommu_map()/iommu_unmap(), and isn't shared with the CPU MMU.
+ *
+ * To obtain the above configuration, users would for instance issue the
+ * following calls:
+ *
+ * iommu_sva_bind_device(dev 00:00.0, mm X, ...) -> PASID 1
+ * iommu_sva_bind_device(dev 00:00.0, mm Y, ...) -> PASID 2
+ * iommu_sva_bind_device(dev 00:01.0, mm Y, ...) -> PASID 2
+ * iommu_sva_bind_device(dev 00:01.1, mm Y, ...) -> PASID 2
+ *
+ * A single Process Address Space ID (PASID) is allocated for each mm. In the
+ * example, devices use PASID 1 to read/write into address space X and PASID 2
+ * to read/write into address space Y.
+ *
+ * Hardware tables describing this configuration in the IOMMU would typically
+ * look like this:
+ *
+ * PASID tables
+ * of domain A
+ * .->+--------+
+ * / 0 | |-------> io_pgtable
+ * / +--------+
+ * Device tables / 1 | |-------> pgd X
+ * +--------+ / +--------+
+ * 00:00.0 | A |-' 2 | |--.
+ * +--------+ +--------+ \
+ * : : 3 | | \
+ * +--------+ +--------+ --> pgd Y
+ * 00:01.0 | B |--. /
+ * +--------+ \ |
+ * 00:01.1 | B |----+ PASID tables |
+ * +--------+ \ of domain B |
+ * '->+--------+ |
+ * 0 | |-- | --> io_pgtable
+ * +--------+ |
+ * 1 | | |
+ * +--------+ |
+ * 2 | |---'
+ * +--------+
+ * 3 | |
+ * +--------+
+ *
+ * With this model, a single call binds all devices in a given domain to an
+ * address space. Other devices in the domain will get the same bond implicitly.
+ * However, users must issue one bind() for each device, because IOMMUs may
+ * implement SVA differently. Furthermore, mandating one bind() per device
+ * allows the driver to perform sanity-checks on device capabilities.
+ *
+ * On Arm and AMD IOMMUs, entry 0 of the PASID table can be used to hold
+ * non-PASID translations. In this case PASID 0 is reserved and entry 0 points
+ * to the io_pgtable base. On Intel IOMMU, the io_pgtable base would be held in
+ * the device table and PASID 0 would be available to the allocator.
+ */
+
+struct iommu_bond {
+ struct io_mm *io_mm;
+ struct device *dev;
+ struct iommu_domain *domain;
+
+ struct list_head mm_head;
+ struct list_head dev_head;
+ struct list_head domain_head;
+
+ void *drvdata;
+};
+
+/*
+ * Because we're using an IDR, PASIDs are limited to 31 bits (the sign bit is
+ * used for returning errors). In practice implementations will use at most 20
+ * bits, which is the PCI limit.
+ */
+static DEFINE_IDR(iommu_pasid_idr);
+
+/*
+ * For the moment this is an all-purpose lock. It serializes
+ * access/modifications to bonds, access/modifications to the PASID IDR, and
+ * changes to io_mm refcount as well.
+ */
+static DEFINE_SPINLOCK(iommu_sva_lock);
+
+static struct io_mm *
+io_mm_alloc(struct iommu_domain *domain, struct device *dev,
+ struct mm_struct *mm, unsigned long flags)
+{
+ int ret;
+ int pasid;
+ struct io_mm *io_mm;
+ struct iommu_sva_param *param = dev->iommu_param->sva_param;
+
+ if (!domain->ops->mm_alloc || !domain->ops->mm_free)
+ return ERR_PTR(-ENODEV);
+
+ io_mm = domain->ops->mm_alloc(domain, mm, flags);
+ if (IS_ERR(io_mm))
+ return io_mm;
+ if (!io_mm)
+ return ERR_PTR(-ENOMEM);
+
+ /*
+ * The mm must not be freed until after the driver frees the io_mm
+ * (which may involve unpinning the CPU ASID for instance, requiring a
+ * valid mm struct.)
+ */
+ mmgrab(mm);
+
+ io_mm->flags = flags;
+ io_mm->mm = mm;
+ io_mm->release = domain->ops->mm_free;
+ INIT_LIST_HEAD(&io_mm->devices);
+
+ idr_preload(GFP_KERNEL);
+ spin_lock(&iommu_sva_lock);
+ pasid = idr_alloc(&iommu_pasid_idr, io_mm, param->min_pasid,
+ param->max_pasid + 1, GFP_ATOMIC);
+ io_mm->pasid = pasid;
+ spin_unlock(&iommu_sva_lock);
+ idr_preload_end();
+
+ if (pasid < 0) {
+ ret = pasid;
+ goto err_free_mm;
+ }
+
+ /* TODO: keep track of mm. For the moment, abort. */
+ ret = -ENOSYS;
+ spin_lock(&iommu_sva_lock);
+ idr_remove(&iommu_pasid_idr, io_mm->pasid);
+ spin_unlock(&iommu_sva_lock);
+
+err_free_mm:
+ domain->ops->mm_free(io_mm);
+ mmdrop(mm);
+
+ return ERR_PTR(ret);
+}
+
+static void io_mm_free(struct io_mm *io_mm)
+{
+ struct mm_struct *mm = io_mm->mm;
+
+ io_mm->release(io_mm);
+ mmdrop(mm);
+}
+
+static void io_mm_release(struct kref *kref)
+{
+ struct io_mm *io_mm;
+
+ io_mm = container_of(kref, struct io_mm, kref);
+ WARN_ON(!list_empty(&io_mm->devices));
+
+ idr_remove(&iommu_pasid_idr, io_mm->pasid);
+
+ io_mm_free(io_mm);
+}
+
+/*
+ * Returns non-zero if a reference to the io_mm was successfully taken.
+ * Returns zero if the io_mm is being freed and should not be used.
+ */
+static int io_mm_get_locked(struct io_mm *io_mm)
+{
+ if (io_mm)
+ return kref_get_unless_zero(&io_mm->kref);
+
+ return 0;
+}
+
+static void io_mm_put_locked(struct io_mm *io_mm)
+{
+ kref_put(&io_mm->kref, io_mm_release);
+}
+
+static void io_mm_put(struct io_mm *io_mm)
+{
+ spin_lock(&iommu_sva_lock);
+ io_mm_put_locked(io_mm);
+ spin_unlock(&iommu_sva_lock);
+}
+
+static int io_mm_attach(struct iommu_domain *domain, struct device *dev,
+ struct io_mm *io_mm, void *drvdata)
+{
+ int ret;
+ bool attach_domain = true;
+ int pasid = io_mm->pasid;
+ struct iommu_bond *bond, *tmp;
+ struct iommu_sva_param *param = dev->iommu_param->sva_param;
+
+ if (!domain->ops->mm_attach || !domain->ops->mm_detach)
+ return -ENODEV;
+
+ if (pasid > param->max_pasid || pasid < param->min_pasid)
+ return -ERANGE;
+
+ bond = kzalloc(sizeof(*bond), GFP_KERNEL);
+ if (!bond)
+ return -ENOMEM;
+
+ bond->domain = domain;
+ bond->io_mm = io_mm;
+ bond->dev = dev;
+ bond->drvdata = drvdata;
+
+ spin_lock(&iommu_sva_lock);
+ /*
+ * Check if this io_mm is already bound to the domain. In which case the
+ * IOMMU driver doesn't have to install the PASID table entry.
+ */
+ list_for_each_entry(tmp, &domain->mm_list, domain_head) {
+ if (tmp->io_mm == io_mm) {
+ attach_domain = false;
+ break;
+ }
+ }
+
+ ret = domain->ops->mm_attach(domain, dev, io_mm, attach_domain);
+ if (ret) {
+ kfree(bond);
+ spin_unlock(&iommu_sva_lock);
+ return ret;
+ }
+
+ list_add(&bond->mm_head, &io_mm->devices);
+ list_add(&bond->domain_head, &domain->mm_list);
+ list_add(&bond->dev_head, ¶m->mm_list);
+ spin_unlock(&iommu_sva_lock);
+
+ return 0;
+}
+
+static void io_mm_detach_locked(struct iommu_bond *bond)
+{
+ struct iommu_bond *tmp;
+ bool detach_domain = true;
+ struct iommu_domain *domain = bond->domain;
+
+ list_for_each_entry(tmp, &domain->mm_list, domain_head) {
+ if (tmp->io_mm == bond->io_mm && tmp->dev != bond->dev) {
+ detach_domain = false;
+ break;
+ }
+ }
+
+ domain->ops->mm_detach(domain, bond->dev, bond->io_mm, detach_domain);
+
+ list_del(&bond->mm_head);
+ list_del(&bond->domain_head);
+ list_del(&bond->dev_head);
+ io_mm_put_locked(bond->io_mm);
+
+ kfree(bond);
+}
/**
* iommu_sva_device_init() - Initialize Shared Virtual Addressing for a device
@@ -47,6 +337,7 @@ int iommu_sva_device_init(struct device *dev, unsigned long features,
param->features = features;
param->max_pasid = max_pasid;
+ INIT_LIST_HEAD(¶m->mm_list);
/*
* IOMMU driver updates the limits depending on the IOMMU and device
@@ -114,13 +405,87 @@ EXPORT_SYMBOL_GPL(iommu_sva_device_shutdown);
int __iommu_sva_bind_device(struct device *dev, struct mm_struct *mm,
int *pasid, unsigned long flags, void *drvdata)
{
- return -ENOSYS; /* TODO */
+ int i, ret = 0;
+ struct io_mm *io_mm = NULL;
+ struct iommu_domain *domain;
+ struct iommu_bond *bond = NULL, *tmp;
+ struct iommu_sva_param *param = dev->iommu_param->sva_param;
+
+ domain = iommu_get_domain_for_dev(dev);
+ if (!domain)
+ return -EINVAL;
+
+ /*
+ * The device driver does not call sva_device_init/shutdown and
+ * bind/unbind concurrently, so no need to take the param lock.
+ */
+ if (WARN_ON_ONCE(!param) || (flags & ~param->features))
+ return -EINVAL;
+
+ /* If an io_mm already exists, use it */
+ spin_lock(&iommu_sva_lock);
+ idr_for_each_entry(&iommu_pasid_idr, io_mm, i) {
+ if (io_mm->mm == mm && io_mm_get_locked(io_mm)) {
+ /* ... Unless it's already bound to this device */
+ list_for_each_entry(tmp, &io_mm->devices, mm_head) {
+ if (tmp->dev == dev) {
+ bond = tmp;
+ io_mm_put_locked(io_mm);
+ break;
+ }
+ }
+ break;
+ }
+ }
+ spin_unlock(&iommu_sva_lock);
+
+ if (bond)
+ return -EEXIST;
+
+ /* Require identical features within an io_mm for now */
+ if (io_mm && (flags != io_mm->flags)) {
+ io_mm_put(io_mm);
+ return -EDOM;
+ }
+
+ if (!io_mm) {
+ io_mm = io_mm_alloc(domain, dev, mm, flags);
+ if (IS_ERR(io_mm))
+ return PTR_ERR(io_mm);
+ }
+
+ ret = io_mm_attach(domain, dev, io_mm, drvdata);
+ if (ret)
+ io_mm_put(io_mm);
+ else
+ *pasid = io_mm->pasid;
+
+ return ret;
}
EXPORT_SYMBOL_GPL(__iommu_sva_bind_device);
int __iommu_sva_unbind_device(struct device *dev, int pasid)
{
- return -ENOSYS; /* TODO */
+ int ret = -ESRCH;
+ struct iommu_domain *domain;
+ struct iommu_bond *bond = NULL;
+ struct iommu_sva_param *param = dev->iommu_param->sva_param;
+
+ domain = iommu_get_domain_for_dev(dev);
+ if (!param || WARN_ON(!domain))
+ return -EINVAL;
+
+ spin_lock(&iommu_sva_lock);
+ list_for_each_entry(bond, ¶m->mm_list, dev_head) {
+ if (bond->io_mm->pasid == pasid) {
+ io_mm_detach_locked(bond);
+ ret = 0;
+ break;
+ }
+ }
+ spin_unlock(&iommu_sva_lock);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(__iommu_sva_unbind_device);
@@ -132,6 +497,15 @@ EXPORT_SYMBOL_GPL(__iommu_sva_unbind_device);
*/
void __iommu_sva_unbind_dev_all(struct device *dev)
{
- /* TODO */
+ struct iommu_sva_param *param;
+ struct iommu_bond *bond, *next;
+
+ param = dev->iommu_param->sva_param;
+ if (param) {
+ spin_lock(&iommu_sva_lock);
+ list_for_each_entry_safe(bond, next, ¶m->mm_list, dev_head)
+ io_mm_detach_locked(bond);
+ spin_unlock(&iommu_sva_lock);
+ }
}
EXPORT_SYMBOL_GPL(__iommu_sva_unbind_dev_all);
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index bd2819deae5b..333801e1519c 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1463,6 +1463,7 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
domain->type = type;
/* Assume all sizes by default; the driver may override this later */
domain->pgsize_bitmap = bus->iommu_ops->pgsize_bitmap;
+ INIT_LIST_HEAD(&domain->mm_list);
return domain;
}
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index da59c20c4f12..d5f21719a5a0 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -100,6 +100,20 @@ struct iommu_domain {
void *handler_token;
struct iommu_domain_geometry geometry;
void *iova_cookie;
+
+ struct list_head mm_list;
+};
+
+struct io_mm {
+ int pasid;
+ /* IOMMU_SVA_FEAT_* */
+ unsigned long flags;
+ struct list_head devices;
+ struct kref kref;
+ struct mm_struct *mm;
+
+ /* Release callback for this mm */
+ void (*release)(struct io_mm *io_mm);
};
enum iommu_cap {
@@ -216,6 +230,7 @@ struct iommu_sva_param {
unsigned long features;
unsigned int min_pasid;
unsigned int max_pasid;
+ struct list_head mm_list;
};
/**
@@ -227,6 +242,11 @@ struct iommu_sva_param {
* @detach_dev: detach device from an iommu domain
* @sva_device_init: initialize Shared Virtual Adressing for a device
* @sva_device_shutdown: shutdown Shared Virtual Adressing for a device
+ * @mm_alloc: allocate io_mm
+ * @mm_free: free io_mm
+ * @mm_attach: attach io_mm to a device. Install PASID entry if necessary
+ * @mm_detach: detach io_mm from a device. Remove PASID entry and
+ * flush associated TLB entries.
* @map: map a physically contiguous memory region to an iommu domain
* @unmap: unmap a physically contiguous memory region from an iommu domain
* @map_sg: map a scatter-gather list of physically contiguous memory chunks
@@ -268,6 +288,14 @@ struct iommu_ops {
struct iommu_sva_param *param);
void (*sva_device_shutdown)(struct device *dev,
struct iommu_sva_param *param);
+ struct io_mm *(*mm_alloc)(struct iommu_domain *domain,
+ struct mm_struct *mm,
+ unsigned long flags);
+ void (*mm_free)(struct io_mm *io_mm);
+ int (*mm_attach)(struct iommu_domain *domain, struct device *dev,
+ struct io_mm *io_mm, bool attach_domain);
+ void (*mm_detach)(struct iommu_domain *domain, struct device *dev,
+ struct io_mm *io_mm, bool detach_domain);
int (*map)(struct iommu_domain *domain, unsigned long iova,
phys_addr_t paddr, size_t size, int prot);
size_t (*unmap)(struct iommu_domain *domain, unsigned long iova,
--
2.17.0
^ permalink raw reply related
* [PATCH v2 04/40] iommu/sva: Add a mm_exit callback for device drivers
From: Jean-Philippe Brucker @ 2018-05-11 19:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511190641.23008-1-jean-philippe.brucker@arm.com>
When an mm exits, devices that were bound to it must stop performing DMA
on its PASID. Let device drivers register a callback to be notified on mm
exit. Add the callback to the sva_param structure attached to struct
device.
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
v1->v2: use iommu_sva_device_init instead of a separate function
---
drivers/iommu/iommu-sva.c | 11 ++++++++++-
include/linux/iommu.h | 9 +++++++--
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
index 6ac679c48f3c..0700893c679d 100644
--- a/drivers/iommu/iommu-sva.c
+++ b/drivers/iommu/iommu-sva.c
@@ -303,6 +303,7 @@ static void io_mm_detach_locked(struct iommu_bond *bond)
* @dev: the device
* @features: bitmask of features that need to be initialized
* @max_pasid: max PASID value supported by the device
+ * @mm_exit: callback to notify the device driver of an mm exiting
*
* Users of the bind()/unbind() API must call this function to initialize all
* features required for SVA.
@@ -313,13 +314,20 @@ static void io_mm_detach_locked(struct iommu_bond *bond)
* description. Setting @max_pasid to a non-zero value smaller than this limit
* overrides it.
*
+ * If the driver intends to share process address spaces, it should pass a valid
+ * @mm_exit handler. Otherwise @mm_exit can be NULL. After @mm_exit returns, the
+ * device must not issue any more transaction with the PASID given as argument.
+ * The handler gets an opaque pointer corresponding to the drvdata passed as
+ * argument of bind().
+ *
* The device should not be performing any DMA while this function is running,
* otherwise the behavior is undefined.
*
* Return 0 if initialization succeeded, or an error.
*/
int iommu_sva_device_init(struct device *dev, unsigned long features,
- unsigned int max_pasid)
+ unsigned int max_pasid,
+ iommu_mm_exit_handler_t mm_exit)
{
int ret;
struct iommu_sva_param *param;
@@ -337,6 +345,7 @@ int iommu_sva_device_init(struct device *dev, unsigned long features,
param->features = features;
param->max_pasid = max_pasid;
+ param->mm_exit = mm_exit;
INIT_LIST_HEAD(¶m->mm_list);
/*
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index d5f21719a5a0..439c8fffd836 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -61,6 +61,7 @@ struct iommu_fault_event;
typedef int (*iommu_fault_handler_t)(struct iommu_domain *,
struct device *, unsigned long, int, void *);
typedef int (*iommu_dev_fault_handler_t)(struct iommu_fault_event *, void *);
+typedef int (*iommu_mm_exit_handler_t)(struct device *dev, int pasid, void *);
struct iommu_domain_geometry {
dma_addr_t aperture_start; /* First address that can be mapped */
@@ -231,6 +232,7 @@ struct iommu_sva_param {
unsigned int min_pasid;
unsigned int max_pasid;
struct list_head mm_list;
+ iommu_mm_exit_handler_t mm_exit;
};
/**
@@ -980,17 +982,20 @@ static inline int iommu_sva_unbind_device(struct device *dev, int pasid)
#ifdef CONFIG_IOMMU_SVA
extern int iommu_sva_device_init(struct device *dev, unsigned long features,
- unsigned int max_pasid);
+ unsigned int max_pasid,
+ iommu_mm_exit_handler_t mm_exit);
extern int iommu_sva_device_shutdown(struct device *dev);
extern int __iommu_sva_bind_device(struct device *dev, struct mm_struct *mm,
int *pasid, unsigned long flags,
void *drvdata);
extern int __iommu_sva_unbind_device(struct device *dev, int pasid);
extern void __iommu_sva_unbind_dev_all(struct device *dev);
+
#else /* CONFIG_IOMMU_SVA */
static inline int iommu_sva_device_init(struct device *dev,
unsigned long features,
- unsigned int max_pasid)
+ unsigned int max_pasid,
+ iommu_mm_exit_handler_t mm_exit)
{
return -ENODEV;
}
--
2.17.0
^ permalink raw reply related
* [PATCH v2 05/40] iommu/sva: Track mm changes with an MMU notifier
From: Jean-Philippe Brucker @ 2018-05-11 19:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511190641.23008-1-jean-philippe.brucker@arm.com>
When creating an io_mm structure, register an MMU notifier that informs
us when the virtual address space changes and disappears.
Add a new operation to the IOMMU driver, mm_invalidate, called when a
range of addresses is unmapped to let the IOMMU driver send ATC
invalidations. mm_invalidate cannot sleep.
Adding the notifier complicates io_mm release. In one case device
drivers free the io_mm explicitly by calling unbind (or detaching the
device from its domain). In the other case the process could crash
before unbind, in which case the release notifier has to do all the
work.
Allowing the device driver's mm_exit() handler to sleep adds another
complication, but it will greatly simplify things for users. For example
VFIO can take the IOMMU mutex and remove any trace of io_mm, instead of
introducing complex synchronization to delicatly handle this race. But
relaxing the user side does force unbind() to sleep and wait for all
pending mm_exit() calls to finish.
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
v1->v2:
* Unbind() waits for mm_exit to finish
* mm_exit can sleep
---
drivers/iommu/Kconfig | 1 +
drivers/iommu/iommu-sva.c | 248 +++++++++++++++++++++++++++++++++++---
include/linux/iommu.h | 10 ++
3 files changed, 244 insertions(+), 15 deletions(-)
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index cca8e06903c7..38434899e283 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -77,6 +77,7 @@ config IOMMU_DMA
config IOMMU_SVA
bool
select IOMMU_API
+ select MMU_NOTIFIER
config FSL_PAMU
bool "Freescale IOMMU support"
diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
index 0700893c679d..e9afae2537a2 100644
--- a/drivers/iommu/iommu-sva.c
+++ b/drivers/iommu/iommu-sva.c
@@ -7,6 +7,7 @@
#include <linux/idr.h>
#include <linux/iommu.h>
+#include <linux/mmu_notifier.h>
#include <linux/sched/mm.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
@@ -106,6 +107,9 @@ struct iommu_bond {
struct list_head mm_head;
struct list_head dev_head;
struct list_head domain_head;
+ refcount_t refs;
+ struct wait_queue_head mm_exit_wq;
+ bool mm_exit_active;
void *drvdata;
};
@@ -124,6 +128,8 @@ static DEFINE_IDR(iommu_pasid_idr);
*/
static DEFINE_SPINLOCK(iommu_sva_lock);
+static struct mmu_notifier_ops iommu_mmu_notifier;
+
static struct io_mm *
io_mm_alloc(struct iommu_domain *domain, struct device *dev,
struct mm_struct *mm, unsigned long flags)
@@ -151,6 +157,7 @@ io_mm_alloc(struct iommu_domain *domain, struct device *dev,
io_mm->flags = flags;
io_mm->mm = mm;
+ io_mm->notifier.ops = &iommu_mmu_notifier;
io_mm->release = domain->ops->mm_free;
INIT_LIST_HEAD(&io_mm->devices);
@@ -167,8 +174,29 @@ io_mm_alloc(struct iommu_domain *domain, struct device *dev,
goto err_free_mm;
}
- /* TODO: keep track of mm. For the moment, abort. */
- ret = -ENOSYS;
+ ret = mmu_notifier_register(&io_mm->notifier, mm);
+ if (ret)
+ goto err_free_pasid;
+
+ /*
+ * Now that the MMU notifier is valid, we can allow users to grab this
+ * io_mm by setting a valid refcount. Before that it was accessible in
+ * the IDR but invalid.
+ *
+ * The following barrier ensures that users, who obtain the io_mm with
+ * kref_get_unless_zero, don't read uninitialized fields in the
+ * structure.
+ */
+ smp_wmb();
+ kref_init(&io_mm->kref);
+
+ return io_mm;
+
+err_free_pasid:
+ /*
+ * Even if the io_mm is accessible from the IDR at this point, kref is
+ * 0 so no user could get a reference to it. Free it manually.
+ */
spin_lock(&iommu_sva_lock);
idr_remove(&iommu_pasid_idr, io_mm->pasid);
spin_unlock(&iommu_sva_lock);
@@ -180,9 +208,13 @@ io_mm_alloc(struct iommu_domain *domain, struct device *dev,
return ERR_PTR(ret);
}
-static void io_mm_free(struct io_mm *io_mm)
+static void io_mm_free(struct rcu_head *rcu)
{
- struct mm_struct *mm = io_mm->mm;
+ struct io_mm *io_mm;
+ struct mm_struct *mm;
+
+ io_mm = container_of(rcu, struct io_mm, rcu);
+ mm = io_mm->mm;
io_mm->release(io_mm);
mmdrop(mm);
@@ -197,7 +229,22 @@ static void io_mm_release(struct kref *kref)
idr_remove(&iommu_pasid_idr, io_mm->pasid);
- io_mm_free(io_mm);
+ /*
+ * If we're being released from mm exit, the notifier callback ->release
+ * has already been called. Otherwise we don't need ->release, the io_mm
+ * isn't attached to anything anymore. Hence no_release.
+ */
+ mmu_notifier_unregister_no_release(&io_mm->notifier, io_mm->mm);
+
+ /*
+ * We can't free the structure here, because if mm exits during
+ * unbind(), then ->release might be attempting to grab the io_mm
+ * concurrently. And in the other case, if ->release is calling
+ * io_mm_release, then __mmu_notifier_release expects to still have a
+ * valid mn when returning. So free the structure when it's safe, after
+ * the RCU grace period elapsed.
+ */
+ mmu_notifier_call_srcu(&io_mm->rcu, io_mm_free);
}
/*
@@ -206,8 +253,14 @@ static void io_mm_release(struct kref *kref)
*/
static int io_mm_get_locked(struct io_mm *io_mm)
{
- if (io_mm)
- return kref_get_unless_zero(&io_mm->kref);
+ if (io_mm && kref_get_unless_zero(&io_mm->kref)) {
+ /*
+ * kref_get_unless_zero doesn't provide ordering for reads. This
+ * barrier pairs with the one in io_mm_alloc.
+ */
+ smp_rmb();
+ return 1;
+ }
return 0;
}
@@ -233,7 +286,8 @@ static int io_mm_attach(struct iommu_domain *domain, struct device *dev,
struct iommu_bond *bond, *tmp;
struct iommu_sva_param *param = dev->iommu_param->sva_param;
- if (!domain->ops->mm_attach || !domain->ops->mm_detach)
+ if (!domain->ops->mm_attach || !domain->ops->mm_detach ||
+ !domain->ops->mm_invalidate)
return -ENODEV;
if (pasid > param->max_pasid || pasid < param->min_pasid)
@@ -247,6 +301,8 @@ static int io_mm_attach(struct iommu_domain *domain, struct device *dev,
bond->io_mm = io_mm;
bond->dev = dev;
bond->drvdata = drvdata;
+ refcount_set(&bond->refs, 1);
+ init_waitqueue_head(&bond->mm_exit_wq);
spin_lock(&iommu_sva_lock);
/*
@@ -275,12 +331,37 @@ static int io_mm_attach(struct iommu_domain *domain, struct device *dev,
return 0;
}
-static void io_mm_detach_locked(struct iommu_bond *bond)
+static void io_mm_detach_locked(struct iommu_bond *bond, bool wait)
{
struct iommu_bond *tmp;
bool detach_domain = true;
struct iommu_domain *domain = bond->domain;
+ if (wait) {
+ bool do_detach = true;
+ /*
+ * If we're unbind() then we're deleting the bond no matter
+ * what. Tell the mm_exit thread that we're cleaning up, and
+ * wait until it finishes using the bond.
+ *
+ * refs is guaranteed to be one or more, otherwise it would
+ * already have been removed from the list. Check is someone is
+ * already waiting, in which case we wait but do not free.
+ */
+ if (refcount_read(&bond->refs) > 1)
+ do_detach = false;
+
+ refcount_inc(&bond->refs);
+ wait_event_lock_irq(bond->mm_exit_wq, !bond->mm_exit_active,
+ iommu_sva_lock);
+ if (!do_detach)
+ return;
+
+ } else if (!refcount_dec_and_test(&bond->refs)) {
+ /* unbind() is waiting to free the bond */
+ return;
+ }
+
list_for_each_entry(tmp, &domain->mm_list, domain_head) {
if (tmp->io_mm == bond->io_mm && tmp->dev != bond->dev) {
detach_domain = false;
@@ -298,6 +379,129 @@ static void io_mm_detach_locked(struct iommu_bond *bond)
kfree(bond);
}
+static int iommu_signal_mm_exit(struct iommu_bond *bond)
+{
+ struct device *dev = bond->dev;
+ struct io_mm *io_mm = bond->io_mm;
+ struct iommu_sva_param *param = dev->iommu_param->sva_param;
+
+ /*
+ * We can't hold the device's param_lock. If we did and the device
+ * driver used a global lock around io_mm, we would risk getting the
+ * following deadlock:
+ *
+ * exit_mm() | Shutdown SVA
+ * mutex_lock(param->lock) | mutex_lock(glob lock)
+ * param->mm_exit() | sva_device_shutdown()
+ * mutex_lock(glob lock) | mutex_lock(param->lock)
+ *
+ * Fortunately unbind() waits for us to finish, and sva_device_shutdown
+ * requires that any bond is removed, so we can safely access mm_exit
+ * and drvdata without taking any lock.
+ */
+ if (!param || !param->mm_exit)
+ return 0;
+
+ return param->mm_exit(dev, io_mm->pasid, bond->drvdata);
+}
+
+/* Called when the mm exits. Can race with unbind(). */
+static void iommu_notifier_release(struct mmu_notifier *mn, struct mm_struct *mm)
+{
+ struct iommu_bond *bond, *next;
+ struct io_mm *io_mm = container_of(mn, struct io_mm, notifier);
+
+ /*
+ * If the mm is exiting then devices are still bound to the io_mm.
+ * A few things need to be done before it is safe to release:
+ *
+ * - As the mmu notifier doesn't hold any reference to the io_mm when
+ * calling ->release(), try to take a reference.
+ * - Tell the device driver to stop using this PASID.
+ * - Clear the PASID table and invalidate TLBs.
+ * - Drop all references to this io_mm by freeing the bonds.
+ */
+ spin_lock(&iommu_sva_lock);
+ if (!io_mm_get_locked(io_mm)) {
+ /* Someone's already taking care of it. */
+ spin_unlock(&iommu_sva_lock);
+ return;
+ }
+
+ list_for_each_entry_safe(bond, next, &io_mm->devices, mm_head) {
+ /*
+ * Release the lock to let the handler sleep. We need to be
+ * careful about concurrent modifications to the list and to the
+ * bond. Tell unbind() not to free the bond until we're done.
+ */
+ bond->mm_exit_active = true;
+ spin_unlock(&iommu_sva_lock);
+
+ if (iommu_signal_mm_exit(bond))
+ dev_WARN(bond->dev, "possible leak of PASID %u",
+ io_mm->pasid);
+
+ spin_lock(&iommu_sva_lock);
+ next = list_next_entry(bond, mm_head);
+
+ /* If someone is waiting, let them delete the bond now */
+ bond->mm_exit_active = false;
+ wake_up_all(&bond->mm_exit_wq);
+
+ /* Otherwise, do it ourselves */
+ io_mm_detach_locked(bond, false);
+ }
+ spin_unlock(&iommu_sva_lock);
+
+ /*
+ * We're now reasonably certain that no more fault is being handled for
+ * this io_mm, since we just flushed them all out of the fault queue.
+ * Release the last reference to free the io_mm.
+ */
+ io_mm_put(io_mm);
+}
+
+static void iommu_notifier_invalidate_range(struct mmu_notifier *mn,
+ struct mm_struct *mm,
+ unsigned long start,
+ unsigned long end)
+{
+ struct iommu_bond *bond;
+ struct io_mm *io_mm = container_of(mn, struct io_mm, notifier);
+
+ spin_lock(&iommu_sva_lock);
+ list_for_each_entry(bond, &io_mm->devices, mm_head) {
+ struct iommu_domain *domain = bond->domain;
+
+ domain->ops->mm_invalidate(domain, bond->dev, io_mm, start,
+ end - start);
+ }
+ spin_unlock(&iommu_sva_lock);
+}
+
+static int iommu_notifier_clear_flush_young(struct mmu_notifier *mn,
+ struct mm_struct *mm,
+ unsigned long start,
+ unsigned long end)
+{
+ iommu_notifier_invalidate_range(mn, mm, start, end);
+ return 0;
+}
+
+static void iommu_notifier_change_pte(struct mmu_notifier *mn,
+ struct mm_struct *mm,
+ unsigned long address, pte_t pte)
+{
+ iommu_notifier_invalidate_range(mn, mm, address, address + PAGE_SIZE);
+}
+
+static struct mmu_notifier_ops iommu_mmu_notifier = {
+ .release = iommu_notifier_release,
+ .clear_flush_young = iommu_notifier_clear_flush_young,
+ .change_pte = iommu_notifier_change_pte,
+ .invalidate_range = iommu_notifier_invalidate_range,
+};
+
/**
* iommu_sva_device_init() - Initialize Shared Virtual Addressing for a device
* @dev: the device
@@ -320,6 +524,12 @@ static void io_mm_detach_locked(struct iommu_bond *bond)
* The handler gets an opaque pointer corresponding to the drvdata passed as
* argument of bind().
*
+ * The @mm_exit handler is allowed to sleep. Be careful about the locks taken in
+ * @mm_exit, because they might lead to deadlocks if they are also held when
+ * dropping references to the mm. Consider the following call chain:
+ * mutex_lock(A); mmput(mm) -> exit_mm() -> @mm_exit() -> mutex_lock(A)
+ * Using mmput_async() prevents this scenario.
+ *
* The device should not be performing any DMA while this function is running,
* otherwise the behavior is undefined.
*
@@ -484,15 +694,16 @@ int __iommu_sva_unbind_device(struct device *dev, int pasid)
if (!param || WARN_ON(!domain))
return -EINVAL;
- spin_lock(&iommu_sva_lock);
+ /* spin_lock_irq matches the one in wait_event_lock_irq */
+ spin_lock_irq(&iommu_sva_lock);
list_for_each_entry(bond, ¶m->mm_list, dev_head) {
if (bond->io_mm->pasid == pasid) {
- io_mm_detach_locked(bond);
+ io_mm_detach_locked(bond, true);
ret = 0;
break;
}
}
- spin_unlock(&iommu_sva_lock);
+ spin_unlock_irq(&iommu_sva_lock);
return ret;
}
@@ -503,18 +714,25 @@ EXPORT_SYMBOL_GPL(__iommu_sva_unbind_device);
* @dev: the device
*
* When detaching @device from a domain, IOMMU drivers should use this helper.
+ * This function may sleep while waiting for bonds to be released.
*/
void __iommu_sva_unbind_dev_all(struct device *dev)
{
struct iommu_sva_param *param;
struct iommu_bond *bond, *next;
+ /*
+ * io_mm_detach_locked might wait, so we shouldn't call it with the dev
+ * param lock held. It's fine to read sva_param outside the lock because
+ * it can only be freed by iommu_sva_device_shutdown when there are no
+ * more bonds in the list.
+ */
param = dev->iommu_param->sva_param;
if (param) {
- spin_lock(&iommu_sva_lock);
+ spin_lock_irq(&iommu_sva_lock);
list_for_each_entry_safe(bond, next, ¶m->mm_list, dev_head)
- io_mm_detach_locked(bond);
- spin_unlock(&iommu_sva_lock);
+ io_mm_detach_locked(bond, true);
+ spin_unlock_irq(&iommu_sva_lock);
}
}
EXPORT_SYMBOL_GPL(__iommu_sva_unbind_dev_all);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 439c8fffd836..caa6f79785b9 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -24,6 +24,7 @@
#include <linux/types.h>
#include <linux/errno.h>
#include <linux/err.h>
+#include <linux/mmu_notifier.h>
#include <linux/of.h>
#include <uapi/linux/iommu.h>
@@ -111,10 +112,15 @@ struct io_mm {
unsigned long flags;
struct list_head devices;
struct kref kref;
+#if defined(CONFIG_MMU_NOTIFIER)
+ struct mmu_notifier notifier;
+#endif
struct mm_struct *mm;
/* Release callback for this mm */
void (*release)(struct io_mm *io_mm);
+ /* For postponed release */
+ struct rcu_head rcu;
};
enum iommu_cap {
@@ -249,6 +255,7 @@ struct iommu_sva_param {
* @mm_attach: attach io_mm to a device. Install PASID entry if necessary
* @mm_detach: detach io_mm from a device. Remove PASID entry and
* flush associated TLB entries.
+ * @mm_invalidate: Invalidate a range of mappings for an mm
* @map: map a physically contiguous memory region to an iommu domain
* @unmap: unmap a physically contiguous memory region from an iommu domain
* @map_sg: map a scatter-gather list of physically contiguous memory chunks
@@ -298,6 +305,9 @@ struct iommu_ops {
struct io_mm *io_mm, bool attach_domain);
void (*mm_detach)(struct iommu_domain *domain, struct device *dev,
struct io_mm *io_mm, bool detach_domain);
+ void (*mm_invalidate)(struct iommu_domain *domain, struct device *dev,
+ struct io_mm *io_mm, unsigned long vaddr,
+ size_t size);
int (*map)(struct iommu_domain *domain, unsigned long iova,
phys_addr_t paddr, size_t size, int prot);
size_t (*unmap)(struct iommu_domain *domain, unsigned long iova,
--
2.17.0
^ permalink raw reply related
* [PATCH v2 06/40] iommu/sva: Search mm by PASID
From: Jean-Philippe Brucker @ 2018-05-11 19:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511190641.23008-1-jean-philippe.brucker@arm.com>
The fault handler will need to find an mm given its PASID. This is the
reason we have an IDR for storing address spaces, so hook it up.
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
drivers/iommu/iommu-sva.c | 26 ++++++++++++++++++++++++++
include/linux/iommu.h | 6 ++++++
2 files changed, 32 insertions(+)
diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
index e9afae2537a2..5abe0f0b445c 100644
--- a/drivers/iommu/iommu-sva.c
+++ b/drivers/iommu/iommu-sva.c
@@ -736,3 +736,29 @@ void __iommu_sva_unbind_dev_all(struct device *dev)
}
}
EXPORT_SYMBOL_GPL(__iommu_sva_unbind_dev_all);
+
+/**
+ * iommu_sva_find() - Find mm associated to the given PASID
+ * @pasid: Process Address Space ID assigned to the mm
+ *
+ * Returns the mm corresponding to this PASID, or NULL if not found. A reference
+ * to the mm is taken, and must be released with mmput().
+ */
+struct mm_struct *iommu_sva_find(int pasid)
+{
+ struct io_mm *io_mm;
+ struct mm_struct *mm = NULL;
+
+ spin_lock(&iommu_sva_lock);
+ io_mm = idr_find(&iommu_pasid_idr, pasid);
+ if (io_mm && io_mm_get_locked(io_mm)) {
+ if (mmget_not_zero(io_mm->mm))
+ mm = io_mm->mm;
+
+ io_mm_put_locked(io_mm);
+ }
+ spin_unlock(&iommu_sva_lock);
+
+ return mm;
+}
+EXPORT_SYMBOL_GPL(iommu_sva_find);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index caa6f79785b9..faf3390ce89d 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -1001,6 +1001,7 @@ extern int __iommu_sva_bind_device(struct device *dev, struct mm_struct *mm,
extern int __iommu_sva_unbind_device(struct device *dev, int pasid);
extern void __iommu_sva_unbind_dev_all(struct device *dev);
+extern struct mm_struct *iommu_sva_find(int pasid);
#else /* CONFIG_IOMMU_SVA */
static inline int iommu_sva_device_init(struct device *dev,
unsigned long features,
@@ -1030,6 +1031,11 @@ static inline int __iommu_sva_unbind_device(struct device *dev, int pasid)
static inline void __iommu_sva_unbind_dev_all(struct device *dev)
{
}
+
+static inline struct mm_struct *iommu_sva_find(int pasid)
+{
+ return NULL;
+}
#endif /* CONFIG_IOMMU_SVA */
#endif /* __LINUX_IOMMU_H */
--
2.17.0
^ permalink raw reply related
* [PATCH v2 07/40] iommu: Add a page fault handler
From: Jean-Philippe Brucker @ 2018-05-11 19:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511190641.23008-1-jean-philippe.brucker@arm.com>
Some systems allow devices to handle I/O Page Faults in the core mm. For
example systems implementing the PCI PRI extension or Arm SMMU stall
model. Infrastructure for reporting these recoverable page faults was
recently added to the IOMMU core for SVA virtualisation. Add a page fault
handler for host SVA.
IOMMU driver can now instantiate several fault workqueues and link them to
IOPF-capable devices. Drivers can choose between a single global
workqueue, one per IOMMU device, one per low-level fault queue, one per
domain, etc.
When it receives a fault event, supposedly in an IRQ handler, the IOMMU
driver reports the fault using iommu_report_device_fault(), which calls
the registered handler. The page fault handler then calls the mm fault
handler, and reports either success or failure with iommu_page_response().
When the handler succeeded, the IOMMU retries the access.
The iopf_param pointer could be embedded into iommu_fault_param. But
putting iopf_param into the iommu_param structure allows us not to care
about ordering between calls to iopf_queue_add_device() and
iommu_register_device_fault_handler().
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
v1->v2: multiple queues registered by IOMMU driver
---
drivers/iommu/Kconfig | 4 +
drivers/iommu/Makefile | 1 +
drivers/iommu/io-pgfault.c | 363 +++++++++++++++++++++++++++++++++++++
include/linux/iommu.h | 58 ++++++
4 files changed, 426 insertions(+)
create mode 100644 drivers/iommu/io-pgfault.c
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 38434899e283..09f13a7c4b60 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -79,6 +79,10 @@ config IOMMU_SVA
select IOMMU_API
select MMU_NOTIFIER
+config IOMMU_PAGE_FAULT
+ bool
+ select IOMMU_API
+
config FSL_PAMU
bool "Freescale IOMMU support"
depends on PCI
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 1dbcc89ebe4c..4b744e399a1b 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_IOMMU_API) += iommu-traces.o
obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o
obj-$(CONFIG_IOMMU_DMA) += dma-iommu.o
obj-$(CONFIG_IOMMU_SVA) += iommu-sva.o
+obj-$(CONFIG_IOMMU_PAGE_FAULT) += io-pgfault.o
obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
obj-$(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) += io-pgtable-arm-v7s.o
obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c
new file mode 100644
index 000000000000..321c00dd3a3d
--- /dev/null
+++ b/drivers/iommu/io-pgfault.c
@@ -0,0 +1,363 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Handle device page faults
+ *
+ * Copyright (C) 2018 ARM Ltd.
+ */
+
+#include <linux/iommu.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+#include <linux/workqueue.h>
+
+/**
+ * struct iopf_queue - IO Page Fault queue
+ * @wq: the fault workqueue
+ * @flush: low-level flush callback
+ * @flush_arg: flush() argument
+ * @refs: references to this structure taken by producers
+ */
+struct iopf_queue {
+ struct workqueue_struct *wq;
+ iopf_queue_flush_t flush;
+ void *flush_arg;
+ refcount_t refs;
+};
+
+/**
+ * struct iopf_device_param - IO Page Fault data attached to a device
+ * @queue: IOPF queue
+ * @partial: faults that are part of a Page Request Group for which the last
+ * request hasn't been submitted yet.
+ */
+struct iopf_device_param {
+ struct iopf_queue *queue;
+ struct list_head partial;
+};
+
+struct iopf_context {
+ struct device *dev;
+ struct iommu_fault_event evt;
+ struct list_head head;
+};
+
+struct iopf_group {
+ struct iopf_context last_fault;
+ struct list_head faults;
+ struct work_struct work;
+};
+
+static int iopf_complete(struct device *dev, struct iommu_fault_event *evt,
+ enum page_response_code status)
+{
+ struct page_response_msg resp = {
+ .addr = evt->addr,
+ .pasid = evt->pasid,
+ .pasid_present = evt->pasid_valid,
+ .page_req_group_id = evt->page_req_group_id,
+ .private_data = evt->iommu_private,
+ .resp_code = status,
+ };
+
+ return iommu_page_response(dev, &resp);
+}
+
+static enum page_response_code
+iopf_handle_single(struct iopf_context *fault)
+{
+ /* TODO */
+ return -ENODEV;
+}
+
+static void iopf_handle_group(struct work_struct *work)
+{
+ struct iopf_group *group;
+ struct iopf_context *fault, *next;
+ enum page_response_code status = IOMMU_PAGE_RESP_SUCCESS;
+
+ group = container_of(work, struct iopf_group, work);
+
+ list_for_each_entry_safe(fault, next, &group->faults, head) {
+ struct iommu_fault_event *evt = &fault->evt;
+ /*
+ * Errors are sticky: don't handle subsequent faults in the
+ * group if there is an error.
+ */
+ if (status == IOMMU_PAGE_RESP_SUCCESS)
+ status = iopf_handle_single(fault);
+
+ if (!evt->last_req)
+ kfree(fault);
+ }
+
+ iopf_complete(group->last_fault.dev, &group->last_fault.evt, status);
+ kfree(group);
+}
+
+/**
+ * iommu_queue_iopf - IO Page Fault handler
+ * @evt: fault event
+ * @cookie: struct device, passed to iommu_register_device_fault_handler.
+ *
+ * Add a fault to the device workqueue, to be handled by mm.
+ */
+int iommu_queue_iopf(struct iommu_fault_event *evt, void *cookie)
+{
+ struct iopf_group *group;
+ struct iopf_context *fault, *next;
+ struct iopf_device_param *iopf_param;
+
+ struct device *dev = cookie;
+ struct iommu_param *param = dev->iommu_param;
+
+ if (WARN_ON(!mutex_is_locked(¶m->lock)))
+ return -EINVAL;
+
+ if (evt->type != IOMMU_FAULT_PAGE_REQ)
+ /* Not a recoverable page fault */
+ return IOMMU_PAGE_RESP_CONTINUE;
+
+ /*
+ * As long as we're holding param->lock, the queue can't be unlinked
+ * from the device and therefore cannot disappear.
+ */
+ iopf_param = param->iopf_param;
+ if (!iopf_param)
+ return -ENODEV;
+
+ if (!evt->last_req) {
+ fault = kzalloc(sizeof(*fault), GFP_KERNEL);
+ if (!fault)
+ return -ENOMEM;
+
+ fault->evt = *evt;
+ fault->dev = dev;
+
+ /* Non-last request of a group. Postpone until the last one */
+ list_add(&fault->head, &iopf_param->partial);
+
+ return IOMMU_PAGE_RESP_HANDLED;
+ }
+
+ group = kzalloc(sizeof(*group), GFP_KERNEL);
+ if (!group)
+ return -ENOMEM;
+
+ group->last_fault.evt = *evt;
+ group->last_fault.dev = dev;
+ INIT_LIST_HEAD(&group->faults);
+ list_add(&group->last_fault.head, &group->faults);
+ INIT_WORK(&group->work, iopf_handle_group);
+
+ /* See if we have partial faults for this group */
+ list_for_each_entry_safe(fault, next, &iopf_param->partial, head) {
+ if (fault->evt.page_req_group_id == evt->page_req_group_id)
+ /* Insert *before* the last fault */
+ list_move(&fault->head, &group->faults);
+ }
+
+ queue_work(iopf_param->queue->wq, &group->work);
+
+ /* Postpone the fault completion */
+ return IOMMU_PAGE_RESP_HANDLED;
+}
+EXPORT_SYMBOL_GPL(iommu_queue_iopf);
+
+/**
+ * iopf_queue_flush_dev - Ensure that all queued faults have been processed
+ * @dev: the endpoint whose faults need to be flushed.
+ *
+ * Users must call this function when releasing a PASID, to ensure that all
+ * pending faults for this PASID have been handled, and won't hit the address
+ * space of the next process that uses this PASID.
+ *
+ * Return 0 on success.
+ */
+int iopf_queue_flush_dev(struct device *dev)
+{
+ int ret = 0;
+ struct iopf_queue *queue;
+ struct iommu_param *param = dev->iommu_param;
+
+ if (!param)
+ return -ENODEV;
+
+ /*
+ * It is incredibly easy to find ourselves in a deadlock situation if
+ * we're not careful, because we're taking the opposite path as
+ * iommu_queue_iopf:
+ *
+ * iopf_queue_flush_dev() | PRI queue handler
+ * lock(mutex) | iommu_queue_iopf()
+ * queue->flush() | lock(mutex)
+ * wait PRI queue empty |
+ *
+ * So we can't hold the device param lock while flushing. We don't have
+ * to, because the queue or the device won't disappear until all flush
+ * are finished.
+ */
+ mutex_lock(¶m->lock);
+ if (param->iopf_param) {
+ queue = param->iopf_param->queue;
+ } else {
+ ret = -ENODEV;
+ }
+ mutex_unlock(¶m->lock);
+ if (ret)
+ return ret;
+
+ queue->flush(queue->flush_arg, dev);
+
+ /*
+ * No need to clear the partial list. All PRGs containing the PASID that
+ * needs to be decommissioned are whole (the device driver made sure of
+ * it before this function was called). They have been submitted to the
+ * queue by the above flush().
+ */
+ flush_workqueue(queue->wq);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(iopf_queue_flush_dev);
+
+/**
+ * iopf_queue_add_device - Add producer to the fault queue
+ * @queue: IOPF queue
+ * @dev: device to add
+ */
+int iopf_queue_add_device(struct iopf_queue *queue, struct device *dev)
+{
+ int ret = -EINVAL;
+ struct iopf_device_param *iopf_param;
+ struct iommu_param *param = dev->iommu_param;
+
+ if (!param)
+ return -ENODEV;
+
+ iopf_param = kzalloc(sizeof(*iopf_param), GFP_KERNEL);
+ if (!iopf_param)
+ return -ENOMEM;
+
+ INIT_LIST_HEAD(&iopf_param->partial);
+ iopf_param->queue = queue;
+
+ mutex_lock(¶m->lock);
+ if (!param->iopf_param) {
+ refcount_inc(&queue->refs);
+ param->iopf_param = iopf_param;
+ ret = 0;
+ }
+ mutex_unlock(¶m->lock);
+
+ if (ret)
+ kfree(iopf_param);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iopf_queue_add_device);
+
+/**
+ * iopf_queue_remove_device - Remove producer from fault queue
+ * @dev: device to remove
+ *
+ * Caller makes sure that no more fault is reported for this device, and no more
+ * flush is scheduled for this device.
+ *
+ * Note: safe to call unconditionally on a cleanup path, even if the device
+ * isn't registered to any IOPF queue.
+ *
+ * Return 0 if the device was attached to the IOPF queue
+ */
+int iopf_queue_remove_device(struct device *dev)
+{
+ struct iopf_context *fault, *next;
+ struct iopf_device_param *iopf_param;
+ struct iommu_param *param = dev->iommu_param;
+
+ if (!param)
+ return -EINVAL;
+
+ mutex_lock(¶m->lock);
+ iopf_param = param->iopf_param;
+ if (iopf_param) {
+ refcount_dec(&iopf_param->queue->refs);
+ param->iopf_param = NULL;
+ }
+ mutex_unlock(¶m->lock);
+ if (!iopf_param)
+ return -EINVAL;
+
+ list_for_each_entry_safe(fault, next, &iopf_param->partial, head)
+ kfree(fault);
+
+ /*
+ * No more flush is scheduled, and the caller removed all bonds from
+ * this device. unbind() waited until any concurrent mm_exit() finished,
+ * therefore there is no flush() running anymore and we can free the
+ * param.
+ */
+ kfree(iopf_param);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(iopf_queue_remove_device);
+
+/**
+ * iopf_queue_alloc - Allocate and initialize a fault queue
+ * @name: a unique string identifying the queue (for workqueue)
+ * @flush: a callback that flushes the low-level queue
+ * @cookie: driver-private data passed to the flush callback
+ *
+ * The callback is called before the workqueue is flushed. The IOMMU driver must
+ * commit all faults that are pending in its low-level queues at the time of the
+ * call, into the IOPF queue (with iommu_report_device_fault). The callback
+ * takes a device pointer as argument, hinting what endpoint is causing the
+ * flush. When the device is NULL, all faults should be committed.
+ */
+struct iopf_queue *
+iopf_queue_alloc(const char *name, iopf_queue_flush_t flush, void *cookie)
+{
+ struct iopf_queue *queue;
+
+ queue = kzalloc(sizeof(*queue), GFP_KERNEL);
+ if (!queue)
+ return NULL;
+
+ /*
+ * The WQ is unordered because the low-level handler enqueues faults by
+ * group. PRI requests within a group have to be ordered, but once
+ * that's dealt with, the high-level function can handle groups out of
+ * order.
+ */
+ queue->wq = alloc_workqueue("iopf_queue/%s", WQ_UNBOUND, 0, name);
+ if (!queue->wq) {
+ kfree(queue);
+ return NULL;
+ }
+
+ queue->flush = flush;
+ queue->flush_arg = cookie;
+ refcount_set(&queue->refs, 1);
+
+ return queue;
+}
+EXPORT_SYMBOL_GPL(iopf_queue_alloc);
+
+/**
+ * iopf_queue_free - Free IOPF queue
+ * @queue: queue to free
+ *
+ * Counterpart to iopf_queue_alloc(). Caller must make sure that all producers
+ * have been removed.
+ */
+void iopf_queue_free(struct iopf_queue *queue)
+{
+
+ /* Caller should have removed all producers first */
+ if (WARN_ON(!refcount_dec_and_test(&queue->refs)))
+ return;
+
+ destroy_workqueue(queue->wq);
+ kfree(queue);
+}
+EXPORT_SYMBOL_GPL(iopf_queue_free);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index faf3390ce89d..fad3a60e1c14 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -461,11 +461,20 @@ struct iommu_fault_param {
void *data;
};
+/**
+ * iopf_queue_flush_t - Flush low-level page fault queue
+ *
+ * Report all faults currently pending in the low-level page fault queue
+ */
+struct iopf_queue;
+typedef int (*iopf_queue_flush_t)(void *cookie, struct device *dev);
+
/**
* struct iommu_param - collection of per-device IOMMU data
*
* @fault_param: IOMMU detected device fault reporting data
* @sva_param: SVA parameters
+ * @iopf_param: I/O Page Fault queue and data
*
* TODO: migrate other per device data pointers under iommu_dev_data, e.g.
* struct iommu_group *iommu_group;
@@ -475,6 +484,7 @@ struct iommu_param {
struct mutex lock;
struct iommu_fault_param *fault_param;
struct iommu_sva_param *sva_param;
+ struct iopf_device_param *iopf_param;
};
int iommu_device_register(struct iommu_device *iommu);
@@ -874,6 +884,12 @@ static inline int iommu_report_device_fault(struct device *dev, struct iommu_fau
return 0;
}
+static inline int iommu_page_response(struct device *dev,
+ struct page_response_msg *msg)
+{
+ return -ENODEV;
+}
+
static inline int iommu_group_id(struct iommu_group *group)
{
return -ENODEV;
@@ -1038,4 +1054,46 @@ static inline struct mm_struct *iommu_sva_find(int pasid)
}
#endif /* CONFIG_IOMMU_SVA */
+#ifdef CONFIG_IOMMU_PAGE_FAULT
+extern int iommu_queue_iopf(struct iommu_fault_event *evt, void *cookie);
+
+extern int iopf_queue_add_device(struct iopf_queue *queue, struct device *dev);
+extern int iopf_queue_remove_device(struct device *dev);
+extern int iopf_queue_flush_dev(struct device *dev);
+extern struct iopf_queue *
+iopf_queue_alloc(const char *name, iopf_queue_flush_t flush, void *cookie);
+extern void iopf_queue_free(struct iopf_queue *queue);
+#else /* CONFIG_IOMMU_PAGE_FAULT */
+static inline int iommu_queue_iopf(struct iommu_fault_event *evt, void *cookie)
+{
+ return -ENODEV;
+}
+
+static inline int iopf_queue_add_device(struct iopf_queue *queue,
+ struct device *dev)
+{
+ return -ENODEV;
+}
+
+static inline int iopf_queue_remove_device(struct device *dev)
+{
+ return -ENODEV;
+}
+
+static inline int iopf_queue_flush_dev(struct device *dev)
+{
+ return -ENODEV;
+}
+
+static inline struct iopf_queue *
+iopf_queue_alloc(const char *name, iopf_queue_flush_t flush, void *cookie)
+{
+ return NULL;
+}
+
+static inline void iopf_queue_free(struct iopf_queue *queue)
+{
+}
+#endif /* CONFIG_IOMMU_PAGE_FAULT */
+
#endif /* __LINUX_IOMMU_H */
--
2.17.0
^ permalink raw reply related
* [PATCH v2 08/40] iommu/iopf: Handle mm faults
From: Jean-Philippe Brucker @ 2018-05-11 19:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511190641.23008-1-jean-philippe.brucker@arm.com>
When a recoverable page fault is handled by the fault workqueue, find the
associated mm and call handle_mm_fault.
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
v1->v2: let IOMMU drivers deal with Stop PASID
---
drivers/iommu/io-pgfault.c | 86 +++++++++++++++++++++++++++++++++++++-
1 file changed, 84 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c
index 321c00dd3a3d..dd2639e5c03b 100644
--- a/drivers/iommu/io-pgfault.c
+++ b/drivers/iommu/io-pgfault.c
@@ -7,6 +7,7 @@
#include <linux/iommu.h>
#include <linux/list.h>
+#include <linux/sched/mm.h>
#include <linux/slab.h>
#include <linux/workqueue.h>
@@ -65,8 +66,65 @@ static int iopf_complete(struct device *dev, struct iommu_fault_event *evt,
static enum page_response_code
iopf_handle_single(struct iopf_context *fault)
{
- /* TODO */
- return -ENODEV;
+ int ret;
+ struct mm_struct *mm;
+ struct vm_area_struct *vma;
+ unsigned int access_flags = 0;
+ unsigned int fault_flags = FAULT_FLAG_REMOTE;
+ struct iommu_fault_event *evt = &fault->evt;
+ enum page_response_code status = IOMMU_PAGE_RESP_INVALID;
+
+ if (!evt->pasid_valid)
+ return status;
+
+ mm = iommu_sva_find(evt->pasid);
+ if (!mm)
+ return status;
+
+ down_read(&mm->mmap_sem);
+
+ vma = find_extend_vma(mm, evt->addr);
+ if (!vma)
+ /* Unmapped area */
+ goto out_put_mm;
+
+ if (evt->prot & IOMMU_FAULT_READ)
+ access_flags |= VM_READ;
+
+ if (evt->prot & IOMMU_FAULT_WRITE) {
+ access_flags |= VM_WRITE;
+ fault_flags |= FAULT_FLAG_WRITE;
+ }
+
+ if (evt->prot & IOMMU_FAULT_EXEC) {
+ access_flags |= VM_EXEC;
+ fault_flags |= FAULT_FLAG_INSTRUCTION;
+ }
+
+ if (!(evt->prot & IOMMU_FAULT_PRIV))
+ fault_flags |= FAULT_FLAG_USER;
+
+ if (access_flags & ~vma->vm_flags)
+ /* Access fault */
+ goto out_put_mm;
+
+ ret = handle_mm_fault(vma, evt->addr, fault_flags);
+ status = ret & VM_FAULT_ERROR ? IOMMU_PAGE_RESP_INVALID :
+ IOMMU_PAGE_RESP_SUCCESS;
+
+out_put_mm:
+ up_read(&mm->mmap_sem);
+
+ /*
+ * If the process exits while we're handling the fault on its mm, we
+ * can't do mmput(). exit_mmap() would release the MMU notifier, calling
+ * iommu_notifier_release(), which has to flush the fault queue that
+ * we're executing on... So mmput_async() moves the release of the mm to
+ * another thread, if we're the last user.
+ */
+ mmput_async(mm);
+
+ return status;
}
static void iopf_handle_group(struct work_struct *work)
@@ -100,6 +158,30 @@ static void iopf_handle_group(struct work_struct *work)
* @cookie: struct device, passed to iommu_register_device_fault_handler.
*
* Add a fault to the device workqueue, to be handled by mm.
+ *
+ * This module doesn't handle PCI PASID Stop Marker; IOMMU drivers must discard
+ * them before reporting faults. A PASID Stop Marker (LRW = 0b100) doesn't
+ * expect a response. It may be generated when disabling a PASID (issuing a
+ * PASID stop request) by some PCI devices.
+ *
+ * The PASID stop request is triggered by the mm_exit() callback. When the
+ * callback returns from the device driver, no page request is generated for
+ * this PASID anymore and outstanding ones have been pushed to the IOMMU (as per
+ * PCIe 4.0r1.0 - 6.20.1 and 10.4.1.2 - Managing PASID TLP Prefix Usage). Some
+ * PCI devices will wait for all outstanding page requests to come back with a
+ * response before completing the PASID stop request. Others do not wait for
+ * page responses, and instead issue this Stop Marker that tells us when the
+ * PASID can be reallocated.
+ *
+ * It is safe to discard the Stop Marker because it is an optimization.
+ * a. Page requests, which are posted requests, have been flushed to the IOMMU
+ * when mm_exit() returns,
+ * b. We flush all fault queues after mm_exit() returns and before freeing the
+ * PASID.
+ *
+ * So even though the Stop Marker might be issued by the device *after* the stop
+ * request completes, outstanding faults will have been dealt with by the time
+ * we free the PASID.
*/
int iommu_queue_iopf(struct iommu_fault_event *evt, void *cookie)
{
--
2.17.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox