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 473D5CD342A for ; Tue, 3 Sep 2024 07:58:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To: Content-Transfer-Encoding:Content-Type: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=EpqgvHYsrTqtiBbD2gubTtjEbMzyUsaQNI2kDwROGw4=; b=pmq65iLgKqJk9VaZkXmJqe2O2t ddHW+p0UqAarC46LWYkAcJEPsjJO8a8pdwDRf2Dxdbu/Zk+FgKbRlsgSWAz4mmieaBIM7GKNhTZUx 4y5F31FdBHx3jz63Jx8RSAD7BPYS/czkX5yVFk1RUnme3szZNfIJ+Z/ciJHvhG2/AWjgott2JhZO5 LtFAJ6XAewlXrrnrU1ZKlXfXm7utHPxmftMuE1xX6dDM2K8sLvgFICjZrdGBnB4RjOBVseruDGwWT 2uFAjemxVHTJtG8ikwcAEtntu6Iegrj9PkxyGL20BDGxCOUIOFuZ8B5lIYJXOYEzM4o/TpJy7vbjp S6NUhZlQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1slOQO-0000000GnFq-28K0; Tue, 03 Sep 2024 07:58:04 +0000 Received: from mail-wm1-x334.google.com ([2a00:1450:4864:20::334]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1slOPU-0000000Gn3r-0hCJ for linux-arm-kernel@lists.infradead.org; Tue, 03 Sep 2024 07:57:09 +0000 Received: by mail-wm1-x334.google.com with SMTP id 5b1f17b1804b1-427fc9834deso112195e9.0 for ; Tue, 03 Sep 2024 00:57:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1725350226; x=1725955026; darn=lists.infradead.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=EpqgvHYsrTqtiBbD2gubTtjEbMzyUsaQNI2kDwROGw4=; b=MWoiFUK2zGiHeIM3rCAuS+/Zzv20a7OiQTm8zbRmDzYgjqu5iudOjEAh4dONLvgKsx 0yNanhdtN2vu/n8UhsT6CXhALj/TkyYKK5fkJOx8mwmxG8/mgb1cOxdhgRgL9saw3Ly2 rwcf9B/GfyzJRdn61G3uRwR9dggvYnf+42F9H81LhiT2lexlKS02mGisrJOb5LbPeDBc tzhmMkwxjNXik8eRywUzB9nSYz+Ttj8ROnw+tSO38SFxiVbqXN5CucoVg3wyzJ2gDghk 0QrxV+8AdkzEJFYfSwrKhKkKO4xf/gfdkDQcVKyd0eu3o7lKWk90Y5mnG06VTo0a+R7K 1b+A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725350226; x=1725955026; h=in-reply-to:content-transfer-encoding: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=EpqgvHYsrTqtiBbD2gubTtjEbMzyUsaQNI2kDwROGw4=; b=MG6AfuvqENI6vohj025REPBb0HDz+irkLiQLcwjBPP88QDxn/RlnvbEKu3PVtHeIwo 0uPfwiG2Dt6bCb/RW96yRjE1OSmb26R35jp4tZShCtjCFKzuBs6BTOHDegXT4O6FSQvY ik/YNyybuVZR/JEXUHq1PjQf/DMnEUzd4w4D3YJObCO8pyhXk6VG+T+qQW8FAAsDOgtD WBSn1uuSNiEi5UvXyQb1Oqs2/TXXjFl8vg8SkO4Z1lMfmtQvfoJVY1c1GVxEEy4kFKim zvmPvq4DQQuIgo6NPwkLnlA81kxRgx5QEawLK9356xrwcSOFFHC507+GNa2JhEfPomLH g2uw== X-Forwarded-Encrypted: i=1; AJvYcCWSJoQHH1EB1yDmX7Qn7Lsd4zRbhPHPQ0zKPH8kVPMtQfm3TzgANg9UWjPZpg2l9UA9wq88UpnsEPqxWMe162sg@lists.infradead.org X-Gm-Message-State: AOJu0Ywrr2GIz4X/xOCM7koo1ZTYpUbphSdVyYgKumnzDDA8h7KabN0k 9RYPiAjqFTpvxP8mdqJYZJvGp0GqPkAtY0Baamw97GwD/iEntRAR1g3upBhNlQ== X-Google-Smtp-Source: AGHT+IHgA6nmHG1PnRdTR7J4T9ZeBV+88BPig2e5l9HlhMUvcfgRj5uIoCkj4peI2Tcgr2az2ArssQ== X-Received: by 2002:a05:600c:1d9f:b0:428:e6eb:1340 with SMTP id 5b1f17b1804b1-42c33a5fb71mr3188345e9.4.1725350225593; Tue, 03 Sep 2024 00:57:05 -0700 (PDT) Received: from google.com (109.36.187.35.bc.googleusercontent.com. [35.187.36.109]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-42bb5969234sm165927345e9.17.2024.09.03.00.57.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Sep 2024 00:57:05 -0700 (PDT) Date: Tue, 3 Sep 2024 07:57:01 +0000 From: Mostafa Saleh To: Jason Gunthorpe Cc: acpica-devel@lists.linux.dev, Hanjun Guo , iommu@lists.linux.dev, Joerg Roedel , Kevin Tian , kvm@vger.kernel.org, Len Brown , linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Lorenzo Pieralisi , "Rafael J. Wysocki" , Robert Moore , Robin Murphy , Sudeep Holla , Will Deacon , Alex Williamson , Eric Auger , Jean-Philippe Brucker , Moritz Fischer , Michael Shavit , Nicolin Chen , patches@lists.linux.dev, Shameerali Kolothum Thodi Subject: Re: [PATCH v2 2/8] iommu/arm-smmu-v3: Use S2FWB when available Message-ID: References: <0-v2-621370057090+91fec-smmuv3_nesting_jgg@nvidia.com> <2-v2-621370057090+91fec-smmuv3_nesting_jgg@nvidia.com> <20240830164019.GU3773488@nvidia.com> <20240903000546.GD3773488@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20240903000546.GD3773488@nvidia.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240903_005708_317454_9E55167B X-CRM114-Status: GOOD ( 47.40 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Sep 02, 2024 at 09:05:46PM -0300, Jason Gunthorpe wrote: > On Mon, Sep 02, 2024 at 09:29:53AM +0000, Mostafa Saleh wrote: > > On Fri, Aug 30, 2024 at 01:40:19PM -0300, Jason Gunthorpe wrote: > > > On Fri, Aug 30, 2024 at 03:12:54PM +0000, Mostafa Saleh wrote: > > > > > + /* > > > > > + * If for some reason the HW does not support DMA coherency then using > > > > > + * S2FWB won't work. This will also disable nesting support. > > > > > + */ > > > > > + if (FIELD_GET(IDR3_FWB, reg) && > > > > > + (smmu->features & ARM_SMMU_FEAT_COHERENCY)) > > > > > + smmu->features |= ARM_SMMU_FEAT_S2FWB; > > > > I think that’s for the SMMU coherency which in theory is not related to the > > > > master which FWB overrides, so this check is not correct. > > > > > > Yes, I agree, in theory. > > > > > > However the driver today already links them together: > > > > > > case IOMMU_CAP_CACHE_COHERENCY: > > > /* Assume that a coherent TCU implies coherent TBUs */ > > > return master->smmu->features & ARM_SMMU_FEAT_COHERENCY; > > > > > > So this hunk was a continuation of that design. > > > > > > > What I meant in the previous thread that we should set FWB only for coherent > > > > masters as (in attach s2): > > > > if (smmu->features & ARM_SMMU_FEAT_S2FWB && dev_is_dma_coherent(master->dev) > > > > // set S2FWB in STE > > > > > > I think as I explained in that thread, it is not really correct > > > either. There is no reason to block using S2FWB for non-coherent > > > masters that are not used with VFIO. The page table will still place > > > the correct memattr according to the IOMMU_CACHE flag, S2FWB just > > > slightly changes the encoding. > > > > It’s not just the encoding that changes, as > > - Without FWB, stage-2 combine attributes > > - While with FWB, it overrides them. > > You mean there is some incomming attribute in the transaction > (obviously not talking PCI here) and S2FWB combines with that? Yes, stuff as cacheability (as defined by Arm spec) I am not sure about PCI, but according to the spec: “PCIe does not contain memory type attributes, and each transaction takes a system-defined memory type when it progresses into the system” > > > So a cacheable mapping in stage-2 can lead to a non-cacheable > > (or with different cachableitiy attributes) transaction based on the > > input. I am not sure though if there is such case in the kernel. > > If the kernel supplies IOMMU_CACHE then the kernel also skips all the > cache flushing. So it would be a functional problem if combining was > causing a non-cachable access through a IOMMU_CACHE S2 already. The > DMA API would fail if that was the case. Correct, but it’s not just about cacheable/non-cacheable, as I mentioned it’s about other attributes also, this is a very niche case, and again I am not sure if there are devices affected in the kernel, but I just wanted to highlight it’s not just a different encoding for stage-2. > > > > If anything should be changed then it would be the above > > > IOMMU_CAP_CACHE_COHERENCY test, and I don't know if > > > dev_is_dma_coherent() would be correct there, or if it should do some > > > ACPI inspection or what. > > > > I agree, I believe that this assumption is not accurate, I am not sure > > what is the right approach here, but in concept I think we shouldn’t > > enable FWB for non-coherent devices (using dev_is_dma_coherent() or > > other check) > > The DMA API requires that the cachability rules it sets via > IOMMU_CACHE are followed. In this way the stricter behavior of S2FWB > is a benefit, not a draw back. > > I'm still not seeing a problm here?? Basically, I believe we shouldn’t set FWB blindly just because it’s supported, I don’t see how it’s useful for stage-2 only domains. And I believe making assumptions about VFIO (which actually is not correctly enforced at the moment) is fragile, and we should only set FWB for coherent devices in nested setup only where the VMM(or hypervisor) knows better than the VM. Thanks, Mostafa > > Jason