From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C2886C67861 for ; Tue, 9 Apr 2024 10:20:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=xN7/IUCXDc1u/ts1VswhVXfi54lY9vtAsPnF3vy2gog=; b=aKA88VQkYKiS+z BwVBHpYkZZGD0BT10ydnR4i8ZpxMu3wHI9sSdigNrpuNjhngzk/IXySdMQGzUwN57cN+Mj1umxuiX jhqv/9G/oAqB0Xsj490mUmmtKrLVB/Vvx+7u20dBenzIg2RZ8S02SC4Ia0aFjPtGbHZz5flmt/Moa IQVDcMrm0oMdb/g7ehy5ezL7qVeeE7j4FcsP17myhYVnm5N20QH2HcJDoBpmFKhgjc7wcNSnc0tOu YkKaT8sBoTndv6iC5jxsnSCmG6EYFxXr9V7exi6LtovZGhMY92tECnC79bBW56dBHoeQiGzY4jUU/ T4OJYO2J1m8xv9OKc2Hw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1ru8a9-00000001QWO-2suP; Tue, 09 Apr 2024 10:20:01 +0000 Received: from mail-wm1-x32e.google.com ([2a00:1450:4864:20::32e]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1ru8a6-00000001QT2-25QR for linux-arm-kernel@lists.infradead.org; Tue, 09 Apr 2024 10:20:00 +0000 Received: by mail-wm1-x32e.google.com with SMTP id 5b1f17b1804b1-41641a889ccso49615e9.1 for ; Tue, 09 Apr 2024 03:19:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1712657991; x=1713262791; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=J8juUJwLhUaK5YyOPk/qzvJU59mosWxulrYVn92pfJQ=; b=eCLZ710z+iQ5XEf5sgPG955dHlRyOgoxWEfJweChLUSjWFBd4ry+oCvF/QGC5QjrlW EENnoWPFyqGL5rvae1gmXpyTmXW5ewc1x9i2kIthtUMZ06xXxs+MMQMpi96BUsnr5UBU AZ0jAk6kv6g3/xcWY42rMFXrbTJ9ZGQvA+bvwDTyTXCW7yhXadRRi1+Bn4Z4RgD9wWBQ P6AUfKej684ayZ8olA07ULSaqw5ZVm8tPeRFLrNCMyeUB8FLZv99+v7U4UyQGwsmcvXX 6bHckR1Wly/aD2KppmrhhqCY10iEzEfXXmpnzYGyJ9O9vN+9Sa+XCfqv+68ptiYI5KhY xd5w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712657991; x=1713262791; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=J8juUJwLhUaK5YyOPk/qzvJU59mosWxulrYVn92pfJQ=; b=NwPP9ZI6d3JD4f+zFBn0o2NeZS2vaK7sfrN5kmPb4dfNkEGj7wNBlA1he61edFsUUZ cHdW9ImYtlYRPgUjeB4Rm+79cTDopoR1gZ19A4bcQ7Btp6RR9zFK1BGvG3w629tO8THy GgETrvc898cSlpFmZD0qYsElWeP5QRIFLpHXxHcdzSuqfF9pMUJtV+iFjplFTUqsYNNv xEZ0VyoeJRp7J8WUfItoGAuDq8jrI+XxHPd+y9Ib47D8aNKlhdB/HawoXpKB0DbCKO4Y Lb/F4Hxt2AYpION8TjA78nHkVQ2X9+mlP1usT/irsX9JdDL1XzgDe0mcsryhiGcfxMIC fz7w== X-Forwarded-Encrypted: i=1; AJvYcCVtftNsUw1v4rXSFfr7fAuc0IOj/ZaLGoU+Iou59K91m93y6YXgI75tPCLDzHsNBoWO3bM9cKdTU/SZTibX07bJmhfobgcxFz0YGxZhCPpQZgBrOx0= X-Gm-Message-State: AOJu0Yw/7fxOvsffgjH5p5vkejyWtTeLTSZ6LEaMhbpDFRN1FtAqv0wC uZc99QRfGMV29IaW82CyFu7DyuYkcNrn6aO0WHFQqgI2MmkZ6MItJ2P0SSgjFg== X-Google-Smtp-Source: AGHT+IEQxud1V1Zbs0fuLsx1yc6EmCyxxeCAV4JhVRqNIvaKVbkdx6uI4b69x0IaG5bE+zD8pN4CYw== X-Received: by 2002:a05:600c:3b8c:b0:415:4436:2a12 with SMTP id n12-20020a05600c3b8c00b0041544362a12mr107628wms.3.1712657990564; Tue, 09 Apr 2024 03:19:50 -0700 (PDT) Received: from google.com (180.232.140.34.bc.googleusercontent.com. [34.140.232.180]) by smtp.gmail.com with ESMTPSA id dm14-20020a0560000bce00b003437a76565asm11065958wrb.25.2024.04.09.03.19.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 09 Apr 2024 03:19:50 -0700 (PDT) Date: Tue, 9 Apr 2024 10:19:46 +0000 From: Mostafa Saleh To: Robin Murphy Cc: will@kernel.org, joro@8bytes.org, iommu@lists.linux.dev, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH] iommu/arm-smmu-v3: Retire disable_bypass parameter Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240409_031958_660601_36DB3FE0 X-CRM114-Status: GOOD ( 34.26 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Robin, On Fri, Apr 05, 2024 at 05:52:07PM +0100, Robin Murphy wrote: > The disable_bypass parameter has been mostly meaningless for a long time > since the introduction of default domains. Its original intent is now > fulfilled by the controls users have over the default domain type, and > its remaining effect in the brief window between Stream Table > initialisation and default domain creation hardly seems worth the > complication. Furthermore, thanks to 2-level Stream Tables, disabling > disable_bypass (there's another reason not to like it right there) has > never guaranteed that any particular StreamID *will* bypass anyway - any > device which might actually care about that wants RMRs - so there's not > really much lost by taking away that option (which has already been > non-default for nearing 6 years now). > > As part of this, also remove the weird behaviour where we "successfully" > probe and register a non-functional SMMU if the DT "#iommu-cells" > property is wrong. I have no memory of what possessed me to think that > was a good idea at the time, and by now I suspect it's likely to break > things worse than simply failing probe would. > > Signed-off-by: Robin Murphy Reviewed-by: Mostafa Saleh > --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 46 ++++++--------------- > 1 file changed, 13 insertions(+), 33 deletions(-) > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > index 41f93c3ab160..4eb74f0ad13b 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > @@ -30,11 +30,6 @@ > #include "arm-smmu-v3.h" > #include "../../dma-iommu.h" > > -static bool disable_bypass = true; > -module_param(disable_bypass, bool, 0444); > -MODULE_PARM_DESC(disable_bypass, > - "Disable bypass streams such that incoming transactions from devices that are not attached to an iommu domain will report an abort back to the device and will not be allowed to pass through the SMMU."); > - > static bool disable_msipolling; > module_param(disable_msipolling, bool, 0444); > MODULE_PARM_DESC(disable_msipolling, > @@ -1567,17 +1562,13 @@ static void arm_smmu_make_s2_domain_ste(struct arm_smmu_ste *target, > * This can safely directly manipulate the STE memory without a sync sequence > * because the STE table has not been installed in the SMMU yet. > */ > -static void arm_smmu_init_initial_stes(struct arm_smmu_device *smmu, > - struct arm_smmu_ste *strtab, > +static void arm_smmu_init_initial_stes(struct arm_smmu_ste *strtab, > unsigned int nent) > { > unsigned int i; > > for (i = 0; i < nent; ++i) { > - if (disable_bypass) > - arm_smmu_make_abort_ste(strtab); > - else > - arm_smmu_make_bypass_ste(smmu, strtab); > + arm_smmu_make_abort_ste(strtab); > strtab++; > } > } > @@ -1605,7 +1596,7 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid) > return -ENOMEM; > } > > - arm_smmu_init_initial_stes(smmu, desc->l2ptr, 1 << STRTAB_SPLIT); > + arm_smmu_init_initial_stes(desc->l2ptr, 1 << STRTAB_SPLIT); > arm_smmu_write_strtab_l1_desc(strtab, desc); > return 0; > } > @@ -2915,10 +2906,10 @@ static void arm_smmu_release_device(struct device *dev) > iopf_queue_remove_device(master->smmu->evtq.iopf, dev); > > /* Put the STE back to what arm_smmu_init_strtab() sets */ > - if (disable_bypass && !dev->iommu->require_direct) > - arm_smmu_attach_dev_blocked(&arm_smmu_blocked_domain, dev); > - else > + if (dev->iommu->require_direct) > arm_smmu_attach_dev_identity(&arm_smmu_identity_domain, dev); > + else > + arm_smmu_attach_dev_blocked(&arm_smmu_blocked_domain, dev); > > arm_smmu_disable_pasid(master); > arm_smmu_remove_master(master); > @@ -3273,7 +3264,7 @@ static int arm_smmu_init_strtab_linear(struct arm_smmu_device *smmu) > reg |= FIELD_PREP(STRTAB_BASE_CFG_LOG2SIZE, smmu->sid_bits); > cfg->strtab_base_cfg = reg; > > - arm_smmu_init_initial_stes(smmu, strtab, cfg->num_l1_ents); > + arm_smmu_init_initial_stes(strtab, cfg->num_l1_ents); > return 0; > } > > @@ -3503,7 +3494,7 @@ static int arm_smmu_device_disable(struct arm_smmu_device *smmu) > return ret; > } > > -static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass) > +static int arm_smmu_device_reset(struct arm_smmu_device *smmu) > { > int ret; > u32 reg, enables; > @@ -3513,7 +3504,6 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass) > reg = readl_relaxed(smmu->base + ARM_SMMU_CR0); > if (reg & CR0_SMMUEN) { > dev_warn(smmu->dev, "SMMU currently enabled! Resetting...\n"); > - WARN_ON(is_kdump_kernel() && !disable_bypass); > arm_smmu_update_gbpa(smmu, GBPA_ABORT, 0); > } > > @@ -3620,14 +3610,8 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass) > if (is_kdump_kernel()) > enables &= ~(CR0_EVTQEN | CR0_PRIQEN); > > - /* Enable the SMMU interface, or ensure bypass */ > - if (!bypass || disable_bypass) { > - enables |= CR0_SMMUEN; > - } else { > - ret = arm_smmu_update_gbpa(smmu, 0, GBPA_ABORT); > - if (ret) > - return ret; > - } > + /* Enable the SMMU interface */ > + enables |= CR0_SMMUEN; > ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0, > ARM_SMMU_CR0ACK); > if (ret) { > @@ -4019,7 +4003,6 @@ static int arm_smmu_device_probe(struct platform_device *pdev) > resource_size_t ioaddr; > struct arm_smmu_device *smmu; > struct device *dev = &pdev->dev; > - bool bypass; > > smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL); > if (!smmu) > @@ -4030,12 +4013,9 @@ static int arm_smmu_device_probe(struct platform_device *pdev) > ret = arm_smmu_device_dt_probe(pdev, smmu); > } else { > ret = arm_smmu_device_acpi_probe(pdev, smmu); > - if (ret == -ENODEV) > - return ret; > } > - > - /* Set bypass mode according to firmware probing result */ > - bypass = !!ret; > + if (ret) > + return ret; > > /* Base address */ > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > @@ -4099,7 +4079,7 @@ static int arm_smmu_device_probe(struct platform_device *pdev) > arm_smmu_rmr_install_bypass_ste(smmu); > > /* Reset the device */ > - ret = arm_smmu_device_reset(smmu, bypass); > + ret = arm_smmu_device_reset(smmu); > if (ret) > return ret; > > -- > 2.39.2.101.g768bb238c484.dirty > Thanks, Mostafa _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel