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 smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (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 64B11CCA479 for ; Fri, 17 Jun 2022 14:02:42 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id DED9583EA0; Fri, 17 Jun 2022 14:02:41 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org DED9583EA0 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id cgi_ZgTUYTiL; Fri, 17 Jun 2022 14:02:41 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp1.osuosl.org (Postfix) with ESMTPS id 8960383E83; Fri, 17 Jun 2022 14:02:40 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 8960383E83 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 11B92C0039; Fri, 17 Jun 2022 14:02:40 +0000 (UTC) Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 691E2C002D for ; Fri, 17 Jun 2022 14:02:38 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 2D80183E55 for ; Fri, 17 Jun 2022 14:02:38 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 2D80183E55 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id IL_ztRXOUi_Y for ; Fri, 17 Jun 2022 14:02:37 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 12CB7834AE Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp1.osuosl.org (Postfix) with ESMTP id 12CB7834AE for ; Fri, 17 Jun 2022 14:02:36 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 3CA0712FC; Fri, 17 Jun 2022 07:02:36 -0700 (PDT) Received: from [10.57.84.65] (unknown [10.57.84.65]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 40BA13F7D8; Fri, 17 Jun 2022 07:02:33 -0700 (PDT) Message-ID: Date: Fri, 17 Jun 2022 15:02:27 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:91.0) Gecko/20100101 Thunderbird/91.10.0 Subject: Re: [PATCH v2 2/3] scsi: BusLogic remove bus_to_virt Content-Language: en-GB To: Arnd Bergmann , linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org References: <20220617125750.728590-1-arnd@kernel.org> <20220617125750.728590-3-arnd@kernel.org> From: Robin Murphy In-Reply-To: <20220617125750.728590-3-arnd@kernel.org> Cc: linux-arch@vger.kernel.org, Miquel van Smoorenburg , linux-parisc@vger.kernel.org, Arnd Bergmann , iommu@lists.linux-foundation.org, Denis Efremov , Mark Salyzyn , Christoph Hellwig , linux-m68k@lists.linux-m68k.org, Matt Wang , linux-alpha@vger.kernel.org, Jakub Kicinski , Khalid Aziz , linuxppc-dev@lists.ozlabs.org, "Maciej W . Rozycki" X-BeenThere: iommu@lists.linux-foundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Development issues for Linux IOMMU support List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: iommu-bounces@lists.linux-foundation.org Sender: "iommu" On 2022-06-17 13:57, Arnd Bergmann wrote: > From: Arnd Bergmann > > The BusLogic driver is the last remaining driver that relies on the > deprecated bus_to_virt() function, which in turn only works on a few > architectures, and is incompatible with both swiotlb and iommu support. > > Before commit 391e2f25601e ("[SCSI] BusLogic: Port driver to 64-bit."), > the driver had a dependency on x86-32, presumably because of this > problem. However, the change introduced another bug that made it still > impossible to use the driver on any 64-bit machine. > > This was in turn fixed in commit 56f396146af2 ("scsi: BusLogic: Fix > 64-bit system enumeration error for Buslogic"), 8 years later, which > shows that there are not a lot of users. > > Maciej is still using the driver on 32-bit hardware, and Khalid mentioned > that the driver works with the device emulation used in VirtualBox > and VMware. Both of those only emulate it for Windows 2000 and older > operating systems that did not ship with the better LSI logic driver. > > Do a minimum fix that searches through the list of descriptors to find > one that matches the bus address. This is clearly as inefficient as > was indicated in the code comment about the lack of a bus_to_virt() > replacement. A better fix would likely involve changing out the entire > descriptor allocation for a simpler one, but that would be much > more invasive. FWIW, if efficiency *is* a practical concern, even under the current allocation scheme it looks like there are only 4 actual DMA allocations to search through. From a quick scan (since it's too hot here not to get distracted...), if I wanted to optimise this in future I'd probably remove the semi-redundant allocgrp_* fields from struct blogic_ccb and hang a dedicated list of the block allocations off the adapter - at that point the lookup could likely already be more efficient than a theoretical dma_to_virt() interface would be if it had to go off and walk an IOMMU pagetable. Then the next question would be whether it's viable to make a single 32KB allocation rather 4*8KB, so it's no longer even a list. For now, though, I agree with the simple change that's clear and easy to reason about: Reviewed-by: Robin Murphy Thanks, Robin. > Cc: Maciej W. Rozycki > Cc: Matt Wang > Cc: Khalid Aziz > Signed-off-by: Arnd Bergmann > --- > drivers/scsi/BusLogic.c | 27 ++++++++++++++++----------- > drivers/scsi/Kconfig | 2 +- > 2 files changed, 17 insertions(+), 12 deletions(-) > > diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c > index a897c8f914cf..d057abfcdd5c 100644 > --- a/drivers/scsi/BusLogic.c > +++ b/drivers/scsi/BusLogic.c > @@ -2515,12 +2515,26 @@ static int blogic_resultcode(struct blogic_adapter *adapter, > return (hoststatus << 16) | tgt_status; > } > > +/* > + * turn the dma address from an inbox into a ccb pointer > + * This is rather inefficient. > + */ > +static struct blogic_ccb * > +blogic_inbox_to_ccb(struct blogic_adapter *adapter, struct blogic_inbox *inbox) > +{ > + struct blogic_ccb *ccb; > + > + for (ccb = adapter->all_ccbs; ccb; ccb = ccb->next_all) > + if (inbox->ccb == ccb->dma_handle) > + break; > + > + return ccb; > +} > > /* > blogic_scan_inbox scans the Incoming Mailboxes saving any > Incoming Mailbox entries for completion processing. > */ > - > static void blogic_scan_inbox(struct blogic_adapter *adapter) > { > /* > @@ -2540,16 +2554,7 @@ static void blogic_scan_inbox(struct blogic_adapter *adapter) > enum blogic_cmplt_code comp_code; > > while ((comp_code = next_inbox->comp_code) != BLOGIC_INBOX_FREE) { > - /* > - We are only allowed to do this because we limit our > - architectures we run on to machines where bus_to_virt( > - actually works. There *needs* to be a dma_addr_to_virt() > - in the new PCI DMA mapping interface to replace > - bus_to_virt() or else this code is going to become very > - innefficient. > - */ > - struct blogic_ccb *ccb = > - (struct blogic_ccb *) bus_to_virt(next_inbox->ccb); > + struct blogic_ccb *ccb = blogic_inbox_to_ccb(adapter, adapter->next_inbox); > if (comp_code != BLOGIC_CMD_NOTFOUND) { > if (ccb->status == BLOGIC_CCB_ACTIVE || > ccb->status == BLOGIC_CCB_RESET) { > diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig > index cf75588a2587..56bdc08d0b77 100644 > --- a/drivers/scsi/Kconfig > +++ b/drivers/scsi/Kconfig > @@ -513,7 +513,7 @@ config SCSI_HPTIOP > > config SCSI_BUSLOGIC > tristate "BusLogic SCSI support" > - depends on PCI && SCSI && VIRT_TO_BUS > + depends on PCI && SCSI > help > This is support for BusLogic MultiMaster and FlashPoint SCSI Host > Adapters. Consult the SCSI-HOWTO, available from _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu