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 X-Spam-Level: X-Spam-Status: No, score=-6.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8D59DC2B9F4 for ; Tue, 22 Jun 2021 20:16:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 67AF161107 for ; Tue, 22 Jun 2021 20:16:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230157AbhFVUSZ (ORCPT ); Tue, 22 Jun 2021 16:18:25 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40674 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229786AbhFVUSZ (ORCPT ); Tue, 22 Jun 2021 16:18:25 -0400 Received: from mail-qt1-x832.google.com (mail-qt1-x832.google.com [IPv6:2607:f8b0:4864:20::832]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3BF35C061574 for ; Tue, 22 Jun 2021 13:16:08 -0700 (PDT) Received: by mail-qt1-x832.google.com with SMTP id l2so401037qtq.10 for ; Tue, 22 Jun 2021 13:16:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=FEaj5TzKzhC7h9IFSKKjMs8DSuzea5RNxJgmYoQLtDo=; b=gAVB6WdGFfSF8M3fXNSIPpQmdJeqZQsRwA717UsvPbCaHtHJwO67jeInsCAEu8fZ9h uHT5p2OxZYu2RG7C998+c+PSzH1bWfNmR/AKE4JJasXBiXiAvt9eghjBffcqzfsWT6O8 itPCAH6dnbArXOZBiLxZWkyFcZYrakdwz1qMs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=FEaj5TzKzhC7h9IFSKKjMs8DSuzea5RNxJgmYoQLtDo=; b=ccz1icIAlBBEhoAm/TY0ExYO7jBDONW5JCZogyXj+lN8N188NWzuWx/z24B0N5BrQV zhqXOc4ZMMrzopY2wLrSFXP2jRDa4fuEsskuNP75CrBoZEqZlyExB0w9ZiWUB/BfnoPL FRuP+Ct+olAcEGRcAL6tP15qZUdtzx52ac27K92ugLFEl+Z0bsQF9BRIVYgl0RFgMQUv cfnpN9zt6B2rymN7lc74dRV4AEM8EDbfu1g0V+Dx/+QM2UHiIv/0aUdLJuYyt77qqBVQ 5N4xlS8+QgPeNQGxQaRs3RsYU2My/ASJ/reBNWKbeCBK/tIAFjvH8mc+1zCZ6lRl12Zc cOeA== X-Gm-Message-State: AOAM5302W1AgCXo1MSu3c6VROXw/0dpBKih9MsECQ6lbnfJvvpKM7LAS 8d1y8eQKiPrw9aGro/gtb7TuTcCB0VvQsQ== X-Google-Smtp-Source: ABdhPJxzsbPKcxRkH3tu/hyK/tUpOtq07PIzlZZu3Zvcmom9VQzgNNTyQ26KC4cHUqMJXiQAwNS03w== X-Received: by 2002:ac8:590a:: with SMTP id 10mr528556qty.276.1624392967053; Tue, 22 Jun 2021 13:16:07 -0700 (PDT) Received: from mail-qk1-f176.google.com (mail-qk1-f176.google.com. [209.85.222.176]) by smtp.gmail.com with ESMTPSA id x12sm1587038qtw.20.2021.06.22.13.16.06 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 22 Jun 2021 13:16:06 -0700 (PDT) Received: by mail-qk1-f176.google.com with SMTP id w21so29034721qkb.9 for ; Tue, 22 Jun 2021 13:16:06 -0700 (PDT) X-Received: by 2002:a25:80d4:: with SMTP id c20mr7216839ybm.345.1624392645386; Tue, 22 Jun 2021 13:10:45 -0700 (PDT) MIME-Version: 1.0 References: <20210621235248.2521620-1-dianders@chromium.org> <067dd86d-da7f-ac83-6ce6-b8fd5aba0b6f@arm.com> <20210622200219.GA28722@robh.at.kernel.org> In-Reply-To: From: Doug Anderson Date: Tue, 22 Jun 2021 13:10:33 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 0/6] iommu: Enable devices to request non-strict DMA, starting with QCom SD/MMC To: Saravana Kannan Cc: Rob Herring , Robin Murphy , Greg Kroah-Hartman , "Rafael J. Wysocki" , "Rafael J. Wysocki" , Will Deacon , Joerg Roedel , Bjorn Andersson , Ulf Hansson , Adrian Hunter , Bjorn Helgaas , Rob Clark , linux-arm-msm , linux-pci@vger.kernel.org, quic_c_gdjako@quicinc.com, "list@263.net:IOMMU DRIVERS , Joerg Roedel ," , Sonny Rao , Sai Prakash Ranjan , Linux MMC List , Veerabhadrarao Badiganti , Rajat Jain , Joel Fernandes , Andy Gross , Bartosz Golaszewski , Dan Williams , Geert Uytterhoeven , Heikki Krogerus , Randy Dunlap , LKML Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org Hi, On Tue, Jun 22, 2021 at 1:06 PM Saravana Kannan wrote: > > On Tue, Jun 22, 2021 at 1:02 PM Rob Herring wrote: > > > > On Tue, Jun 22, 2021 at 09:06:02AM -0700, Doug Anderson wrote: > > > Hi, > > > > > > On Tue, Jun 22, 2021 at 4:35 AM Robin Murphy wrote: > > > > > > > > Hi Doug, > > > > > > > > On 2021-06-22 00:52, Douglas Anderson wrote: > > > > > > > > > > This patch attempts to put forward a proposal for enabling non-strict > > > > > DMA on a device-by-device basis. The patch series requests non-strict > > > > > DMA for the Qualcomm SDHCI controller as a first device to enable, > > > > > getting a nice bump in performance with what's believed to be a very > > > > > small drop in security / safety (see the patch for the full argument). > > > > > > > > > > As part of this patch series I am end up slightly cleaning up some of > > > > > the interactions between the PCI subsystem and the IOMMU subsystem but > > > > > I don't go all the way to fully remove all the tentacles. Specifically > > > > > this patch series only concerns itself with a single aspect: strict > > > > > vs. non-strict mode for the IOMMU. I'm hoping that this will be easier > > > > > to talk about / reason about for more subsystems compared to overall > > > > > deciding what it means for a device to be "external" or "untrusted". > > > > > > > > > > If something like this patch series ends up being landable, it will > > > > > undoubtedly need coordination between many maintainers to land. I > > > > > believe it's fully bisectable but later patches in the series > > > > > definitely depend on earlier ones. Sorry for the long CC list. :( > > > > > > > > Unfortunately, this doesn't work. In normal operation, the default > > > > domains should be established long before individual drivers are even > > > > loaded (if they are modules), let alone anywhere near probing. The fact > > > > that iommu_probe_device() sometimes gets called far too late off the > > > > back of driver probe is an unfortunate artefact of the original > > > > probe-deferral scheme, and causes other problems like potentially > > > > malformed groups - I've been forming a plan to fix that for a while now, > > > > so I for one really can't condone anything trying to rely on it. > > > > Non-deterministic behaviour based on driver probe order for multi-device > > > > groups is part of the existing problem, and your proposal seems equally > > > > vulnerable to that too. > > > > > > Doh! :( I definitely can't say I understand the iommu subsystem > > > amazingly well. It was working for me, but I could believe that I was > > > somehow violating a rule somewhere. > > > > > > I'm having a bit of a hard time understanding where the problem is > > > though. Is there any chance that you missed the part of my series > > > where I introduced a "pre_probe" step? Specifically, I see this: > > > > > > * really_probe() is called w/ a driver and a device. > > > * -> calls dev->bus->dma_configure() w/ a "struct device *" > > > * -> eventually calls iommu_probe_device() w/ the device. > > > * -> calls iommu_alloc_default_domain() w/ the device > > > * -> calls iommu_group_alloc_default_domain() > > > * -> always allocates a new domain > > > > > > ...so we always have a "struct device" when a domain is allocated if > > > that domain is going to be associated with a device. > > > > > > I will agree that iommu_probe_device() is called before the driver > > > probe, but unless I missed something it's after the device driver is > > > loaded. ...and assuming something like patch #1 in this series looks > > > OK then iommu_probe_device() will be called after "pre_probe". > > > > > > So assuming I'm not missing something, I'm not actually relying the > > > IOMMU getting init off the back of driver probe. > > > > > > > > > > FWIW we already have a go-faster knob for people who want to tweak the > > > > security/performance compromise for specific devices, namely the sysfs > > > > interface for changing a group's domain type before binding the relevant > > > > driver(s). Is that something you could use in your application, say from > > > > an initramfs script? > > > > > > We've never had an initramfs script in Chrome OS. I don't know all the > > > history of why (I'm trying to check), but I'm nearly certain it was a > > > conscious decision. Probably it has to do with the fact that we're not > > > trying to build a generic distribution where a single boot source can > > > boot a huge variety of hardware. We generally have one kernel for a > > > class of devices. I believe avoiding the initramfs just keeps things > > > simpler. > > > > > > I think trying to revamp Chrome OS to switch to an initramfs type > > > system would be a pretty big undertaking since (as I understand it) > > > you can't just run a little command and then return to the normal boot > > > flow. Once you switch to initramfs you're committing to finding / > > > setting up the rootfs yourself and on Chrome OS I believe that means a > > > whole bunch of dm-verity work. > > > > > > > > > ...so probably the initramfs is a no-go for me, but I'm still crossing > > > my fingers that the pre_probe() might be legit if you take a second > > > look at it? > > > > Couldn't you have a driver flag that has the same effect as twiddling > > sysfs? At the being of probe, check the flag and go set the underlying > > sysfs setting in the device. > > My understanding of what Robin is saying is that we'd need this info > well before the driver is even available. The pre_probe() is > effectively doing the same thing you are suggesting. Right, I was just about to respond with the same. ;-) So overall right now we're blocked waiting for someone to point out the error in my logic. ;-) > > Though you may want this to be per device, not per driver. To do that > > early, I think you'd need a DT property. I wouldn't be totally opposed > > to that and I appreciate you not starting there. :) > > Which is what I'm suggest elsewhere in the thread: > > https://lore.kernel.org/lkml/CAGETcx83qCZF5JN5cqXxdSFiEgfc4jYESJg-RepL2wJXJv0Eww@mail.gmail.com/ Rob: I'd be happy if you wanted to comment on that thread. If you say that it's fine to add a generic device tree property to control strictness then I'm more than happy to add support for it. I've been going on the theory that you'd NAK such a property but I'm totally good with being wrong. ;-) I'd be more than happy if you could suggest what you'd envision such a property to be named. -Doug