| Message ID | 20221108125855.2984751-1-harshit.m.mogalapalli@oracle.com |
|---|---|
| State | New |
| Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org>
Delivered-To: ouuuleilei@gmail.com
Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp2691304wru;
Tue, 8 Nov 2022 05:05:15 -0800 (PST)
X-Google-Smtp-Source:
AMsMyM6GondSOedTM71i+Hcy4IZgHNEHc8vd2WQhVWkAl2SjyFhSgTLOZDmnVPvSb0Ri8pw38OE5
X-Received: by 2002:a65:458a:0:b0:46f:33df:46b0 with SMTP id
o10-20020a65458a000000b0046f33df46b0mr47316083pgq.564.1667912715319;
Tue, 08 Nov 2022 05:05:15 -0800 (PST)
ARC-Seal: i=1; a=rsa-sha256; t=1667912715; cv=none;
d=google.com; s=arc-20160816;
b=PbQXgjHAOrIf5rHe5tzWo3oWm4eBbgWIkr8Grk8jz55QaXr988QHBLiBUMMtbZIR0V
P7DOEHbOwZngFNHUdnUEryOUb87SY+xJOXKEfUcKymCbGnSBzOdtENOfZR/okoz6YlE1
mcqXZ28YKouRQ5lnqXi0siGiJZmIjCkkoBU3dJWPjezc1oa5+WTYS1XbFkVQra+0ZQ04
WehYPGf74biTgWgC+Q/nd4LyGet8wh5pd+LqZiwNRLWqV1b1HcSxySJEtXgvuNqTYqXh
VEtc9TZic+1/y20WslGqpFTAnCxon6/U3FLoxT1G+x3Q1TXQrP0kfcpN7d3DiHeWY/38
6vjw==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;
s=arc-20160816;
h=list-id:precedence:to:content-transfer-encoding:mime-version
:message-id:date:subject:cc:from:dkim-signature;
bh=FkA06drFvoohZOAdtjadciWDeXolGizxDkgRyYuiORY=;
b=n8EjVVOyHvzTKXMGnnPQXd7HThQLhGHlC3zbENEAUHJvl1yueBpQ0gMXlTH+Is/5d7
jgQ7oiyA0fy+pgK+JnYLDx1JPA02hxgFaCb099qijh6eKcq3+PGfh2ItwJFvXNE3pL3Y
KumEx0vDc+j5wtv7WUomOgmwdosVJZDfbxxGqkW9zrgPS9VWrsIo0aWO7ZnjLaSznPkQ
Kr4Dry8N85nHYajCIh0A8BwxJIHCtaMeEUbNM6hqjt9RWPGwQUrgZMMa4YxMmegxehzh
CzgP8vcpcknVdl/4huSTZtjK5GF1iV9T9NKD0Elu7hBqJWGp/ieDVNY+lbVltRR3llGM
KwKw==
ARC-Authentication-Results: i=1; mx.google.com;
dkim=fail header.i=@oracle.com header.s=corp-2022-7-12
header.b=qPZh7p9D;
spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
smtp.mailfrom=linux-kernel-owner@vger.kernel.org;
dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=oracle.com
Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20])
by mx.google.com with ESMTP id
z36-20020a630a64000000b0045a5d846af1si14585336pgk.134.2022.11.08.05.04.59;
Tue, 08 Nov 2022 05:05:15 -0800 (PST)
Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
client-ip=2620:137:e000::1:20;
Authentication-Results: mx.google.com;
dkim=fail header.i=@oracle.com header.s=corp-2022-7-12
header.b=qPZh7p9D;
spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
smtp.mailfrom=linux-kernel-owner@vger.kernel.org;
dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=oracle.com
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
id S234303AbiKHNAb (ORCPT <rfc822;hjfbswb@gmail.com> + 99 others);
Tue, 8 Nov 2022 08:00:31 -0500
Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43174 "EHLO
lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
with ESMTP id S234224AbiKHNAa (ORCPT
<rfc822;linux-kernel@vger.kernel.org>);
Tue, 8 Nov 2022 08:00:30 -0500
Received: from mx0a-00069f02.pphosted.com (mx0a-00069f02.pphosted.com
[205.220.165.32])
by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6748514D00
for <linux-kernel@vger.kernel.org>;
Tue, 8 Nov 2022 05:00:29 -0800 (PST)
Received: from pps.filterd (m0246629.ppops.net [127.0.0.1])
by mx0b-00069f02.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id
2A8C9Or5009799;
Tue, 8 Nov 2022 12:59:29 GMT
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com;
h=from : to : cc :
subject : date : message-id : mime-version : content-transfer-encoding;
s=corp-2022-7-12; bh=FkA06drFvoohZOAdtjadciWDeXolGizxDkgRyYuiORY=;
b=qPZh7p9DtLx44ReKOkmgNZH8JVJHDp9qwcIPpOOBAunnsT14EjuaIlzJr2euVtCgOgpS
zq461cPoL9ieX+5eab6yjY+Xu1VlhLMQZht4ITlEXzmxZh3gLV1BXzp4KHsHCTh39aAv
CVzcMgpUC/dro2fC4F/9079yZICWmde3AEiq29RPGuS2KxxwZaDwEBNgGcUtuiaI2gpU
VTM4pFRYaMK4gZR7CkPo/KqnsUoewf6KedJA8zwnIGamgSwvbJUhkhNjqcvIV72mAZWO
J7WhdqhImUl6AVfkyYy8F2aQ4XMc8xFYwYelL9oB1omHTz0xAMDX/JROjmefjMeFJ19m GQ==
Received: from phxpaimrmta02.imrmtpd1.prodappphxaev1.oraclevcn.com
(phxpaimrmta02.appoci.oracle.com [147.154.114.232])
by mx0b-00069f02.pphosted.com (PPS) with ESMTPS id 3kngreq23d-1
(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256
verify=OK);
Tue, 08 Nov 2022 12:59:28 +0000
Received: from pps.filterd
(phxpaimrmta02.imrmtpd1.prodappphxaev1.oraclevcn.com [127.0.0.1])
by phxpaimrmta02.imrmtpd1.prodappphxaev1.oraclevcn.com
(8.17.1.5/8.17.1.5) with ESMTP id 2A8A5mNr030074;
Tue, 8 Nov 2022 12:59:28 GMT
Received: from pps.reinject (localhost [127.0.0.1])
by phxpaimrmta02.imrmtpd1.prodappphxaev1.oraclevcn.com (PPS) with
ESMTPS id 3kpctkc73x-1
(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256
verify=OK);
Tue, 08 Nov 2022 12:59:28 +0000
Received: from phxpaimrmta02.imrmtpd1.prodappphxaev1.oraclevcn.com
(phxpaimrmta02.imrmtpd1.prodappphxaev1.oraclevcn.com [127.0.0.1])
by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 2A8CxRG4027177;
Tue, 8 Nov 2022 12:59:27 GMT
Received: from ca-dev112.us.oracle.com (ca-dev112.us.oracle.com
[10.129.136.47])
by phxpaimrmta02.imrmtpd1.prodappphxaev1.oraclevcn.com (PPS) with
ESMTP id 3kpctkc73p-1;
Tue, 08 Nov 2022 12:59:27 +0000
From: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
Cc: harshit.m.mogalapalli@oracle.com, konrad.wilk@oracle.com,
harshit.m.mogalapalli@gmail.com,
David Woodhouse <dwmw2@infradead.org>,
Lu Baolu <baolu.lu@linux.intel.com>,
Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>,
Robin Murphy <robin.murphy@arm.com>, iommu@lists.linux.dev,
linux-kernel@vger.kernel.org
Subject: [RFC] iommu/vt-d: set default value of INTEL_IOMMU_FLOPPY_WA to n
Date: Tue, 8 Nov 2022 04:58:55 -0800
Message-Id: <20221108125855.2984751-1-harshit.m.mogalapalli@oracle.com>
X-Mailer: git-send-email 2.31.1
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-Proofpoint-Virus-Version: vendor=baseguard
engine=ICAP:2.0.219,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1
definitions=2022-11-07_11,2022-11-08_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999
spamscore=0 bulkscore=0
adultscore=0 mlxscore=0 suspectscore=0 malwarescore=0 phishscore=0
classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2210170000
definitions=main-2211080076
X-Proofpoint-ORIG-GUID: QcCHY4EIHo5OTCgg4UQclcgiztWbC31S
X-Proofpoint-GUID: QcCHY4EIHo5OTCgg4UQclcgiztWbC31S
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIMWL_WL_MED,
DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham
autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
lindbergh.monkeyblade.net
To: unlisted-recipients:; (no To-header on input)
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?=
X-GMAIL-THRID: =?utf-8?q?1748933243654995450?=
X-GMAIL-MSGID: =?utf-8?q?1748933243654995450?=
|
| Series |
[RFC] iommu/vt-d: set default value of INTEL_IOMMU_FLOPPY_WA to n
|
|
Commit Message
Harshit Mogalapalli
Nov. 8, 2022, 12:58 p.m. UTC
It is likely that modern intel motherboard will not ship with a
floppy connection anymore, so let us disable it by default, as it
gets turned on when we do a make defconfig.
Suggested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
drivers/iommu/intel/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On 2022/11/8 20:58, Harshit Mogalapalli wrote: > It is likely that modern intel motherboard will not ship with a > floppy connection anymore, so let us disable it by default, as it > gets turned on when we do a make defconfig. > > Suggested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> > --- > drivers/iommu/intel/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig > index b7dff5092fd2..c783ae85ca9b 100644 > --- a/drivers/iommu/intel/Kconfig > +++ b/drivers/iommu/intel/Kconfig > @@ -76,7 +76,7 @@ config INTEL_IOMMU_BROKEN_GFX_WA > option is removed in the 2.6.32 kernel. > > config INTEL_IOMMU_FLOPPY_WA > - def_bool y > + def_bool n > depends on X86 > help > Floppy disk drivers are known to bypass DMA API calls Nobody selects or depends on this. How about removing this bool? Only less than 10 lines of code are impacted and are not in any performance path. diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig index b7dff5092fd2..5e077d1c5f5d 100644 --- a/drivers/iommu/intel/Kconfig +++ b/drivers/iommu/intel/Kconfig @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA to use physical addresses for DMA, at least until this option is removed in the 2.6.32 kernel. -config INTEL_IOMMU_FLOPPY_WA - def_bool y - depends on X86 - help - Floppy disk drivers are known to bypass DMA API calls - thereby failing to work when IOMMU is enabled. This - workaround will setup a 1:1 mapping for the first - 16MiB to make floppy (an ISA device) work. - config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON bool "Enable Intel IOMMU scalable mode by default" default y diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index 48cdcd0a5cf3..22801850f339 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -4567,7 +4567,6 @@ static void intel_iommu_get_resv_regions(struct device *device, } rcu_read_unlock(); -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA if (dev_is_pci(device)) { struct pci_dev *pdev = to_pci_dev(device); @@ -4579,7 +4578,6 @@ static void intel_iommu_get_resv_regions(struct device *device, list_add_tail(®->list, head); } } -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, IOAPIC_RANGE_END - IOAPIC_RANGE_START + 1, Best regards, baolu
On 09/11/22 12:35 pm, Baolu Lu wrote: > On 2022/11/8 20:58, Harshit Mogalapalli wrote: >> It is likely that modern intel motherboard will not ship with a >> floppy connection anymore, so let us disable it by default, as it >> gets turned on when we do a make defconfig. >> >> Suggested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> >> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> >> --- >> drivers/iommu/intel/Kconfig | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig >> index b7dff5092fd2..c783ae85ca9b 100644 >> --- a/drivers/iommu/intel/Kconfig >> +++ b/drivers/iommu/intel/Kconfig >> @@ -76,7 +76,7 @@ config INTEL_IOMMU_BROKEN_GFX_WA >> option is removed in the 2.6.32 kernel. >> config INTEL_IOMMU_FLOPPY_WA >> - def_bool y >> + def_bool n >> depends on X86 >> help >> Floppy disk drivers are known to bypass DMA API calls > > Nobody selects or depends on this. How about removing this bool? Only > less than 10 lines of code are impacted and are not in any performance > path. > > diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig > index b7dff5092fd2..5e077d1c5f5d 100644 > --- a/drivers/iommu/intel/Kconfig > +++ b/drivers/iommu/intel/Kconfig > @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA > to use physical addresses for DMA, at least until this > option is removed in the 2.6.32 kernel. > > -config INTEL_IOMMU_FLOPPY_WA > - def_bool y > - depends on X86 > - help > - Floppy disk drivers are known to bypass DMA API calls > - thereby failing to work when IOMMU is enabled. This > - workaround will setup a 1:1 mapping for the first > - 16MiB to make floppy (an ISA device) work. > - > config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON > bool "Enable Intel IOMMU scalable mode by default" > default y > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c > index 48cdcd0a5cf3..22801850f339 100644 > --- a/drivers/iommu/intel/iommu.c > +++ b/drivers/iommu/intel/iommu.c > @@ -4567,7 +4567,6 @@ static void intel_iommu_get_resv_regions(struct > device *device, > } > rcu_read_unlock(); > > -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA > if (dev_is_pci(device)) { > struct pci_dev *pdev = to_pci_dev(device); > > @@ -4579,7 +4578,6 @@ static void intel_iommu_get_resv_regions(struct > device *device, > list_add_tail(®->list, head); > } > } > -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ > > reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, > IOAPIC_RANGE_END - IOAPIC_RANGE_START + 1, > Hi Baolu, I have a question: Shouldn't we remove the code between ifdef-endif statements? I mean something like this: diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig index b7dff5092fd2..5e077d1c5f5d 100644 --- a/drivers/iommu/intel/Kconfig +++ b/drivers/iommu/intel/Kconfig @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA to use physical addresses for DMA, at least until this option is removed in the 2.6.32 kernel. -config INTEL_IOMMU_FLOPPY_WA - def_bool y - depends on X86 - help - Floppy disk drivers are known to bypass DMA API calls - thereby failing to work when IOMMU is enabled. This - workaround will setup a 1:1 mapping for the first - 16MiB to make floppy (an ISA device) work. - config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON bool "Enable Intel IOMMU scalable mode by default" default y diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index 48cdcd0a5cf3..2c416ad3204e 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -4567,20 +4567,6 @@ static void intel_iommu_get_resv_regions(struct device *device, } rcu_read_unlock(); -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA - if (dev_is_pci(device)) { - struct pci_dev *pdev = to_pci_dev(device); - - if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) { - reg = iommu_alloc_resv_region(0, 1UL << 24, prot, - IOMMU_RESV_DIRECT_RELAXABLE, - GFP_KERNEL); - if (reg) - list_add_tail(®->list, head); - } - } -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ - reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, IOAPIC_RANGE_END - IOAPIC_RANGE_START + 1, 0, IOMMU_RESV_MSI, GFP_KERNEL); This code is introduced in Commit d850c2ee5fe2 ("iommu/vt-d: Expose ISA direct mapping region via iommu_get_resv_regions") If others don't have an objection I can send a patch with the above diff. Note: I am unsure about the code internals. Thanks, Harshit > Best regards, > baolu
On 2022/11/9 20:16, Harshit Mogalapalli wrote: > > > On 09/11/22 12:35 pm, Baolu Lu wrote: >> On 2022/11/8 20:58, Harshit Mogalapalli wrote: >>> It is likely that modern intel motherboard will not ship with a >>> floppy connection anymore, so let us disable it by default, as it >>> gets turned on when we do a make defconfig. >>> >>> Suggested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> >>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> >>> --- >>> drivers/iommu/intel/Kconfig | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig >>> index b7dff5092fd2..c783ae85ca9b 100644 >>> --- a/drivers/iommu/intel/Kconfig >>> +++ b/drivers/iommu/intel/Kconfig >>> @@ -76,7 +76,7 @@ config INTEL_IOMMU_BROKEN_GFX_WA >>> option is removed in the 2.6.32 kernel. >>> config INTEL_IOMMU_FLOPPY_WA >>> - def_bool y >>> + def_bool n >>> depends on X86 >>> help >>> Floppy disk drivers are known to bypass DMA API calls >> >> Nobody selects or depends on this. How about removing this bool? Only >> less than 10 lines of code are impacted and are not in any performance >> path. >> >> diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig >> index b7dff5092fd2..5e077d1c5f5d 100644 >> --- a/drivers/iommu/intel/Kconfig >> +++ b/drivers/iommu/intel/Kconfig >> @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA >> to use physical addresses for DMA, at least until this >> option is removed in the 2.6.32 kernel. >> >> -config INTEL_IOMMU_FLOPPY_WA >> - def_bool y >> - depends on X86 >> - help >> - Floppy disk drivers are known to bypass DMA API calls >> - thereby failing to work when IOMMU is enabled. This >> - workaround will setup a 1:1 mapping for the first >> - 16MiB to make floppy (an ISA device) work. >> - >> config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON >> bool "Enable Intel IOMMU scalable mode by default" >> default y >> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c >> index 48cdcd0a5cf3..22801850f339 100644 >> --- a/drivers/iommu/intel/iommu.c >> +++ b/drivers/iommu/intel/iommu.c >> @@ -4567,7 +4567,6 @@ static void intel_iommu_get_resv_regions(struct >> device *device, >> } >> rcu_read_unlock(); >> >> -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA >> if (dev_is_pci(device)) { >> struct pci_dev *pdev = to_pci_dev(device); >> >> @@ -4579,7 +4578,6 @@ static void intel_iommu_get_resv_regions(struct >> device *device, >> list_add_tail(®->list, head); >> } >> } >> -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ >> >> reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, >> IOAPIC_RANGE_END - IOAPIC_RANGE_START + 1, >> > > Hi Baolu, > > I have a question: > Shouldn't we remove the code between ifdef-endif statements? > > I mean something like this: > > diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig > index b7dff5092fd2..5e077d1c5f5d 100644 > --- a/drivers/iommu/intel/Kconfig > +++ b/drivers/iommu/intel/Kconfig > @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA > to use physical addresses for DMA, at least until this > option is removed in the 2.6.32 kernel. > > -config INTEL_IOMMU_FLOPPY_WA > - def_bool y > - depends on X86 > - help > - Floppy disk drivers are known to bypass DMA API calls > - thereby failing to work when IOMMU is enabled. This > - workaround will setup a 1:1 mapping for the first > - 16MiB to make floppy (an ISA device) work. > - > config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON > bool "Enable Intel IOMMU scalable mode by default" > default y > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c > index 48cdcd0a5cf3..2c416ad3204e 100644 > --- a/drivers/iommu/intel/iommu.c > +++ b/drivers/iommu/intel/iommu.c > @@ -4567,20 +4567,6 @@ static void intel_iommu_get_resv_regions(struct > device *device, > } > rcu_read_unlock(); > > -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA > - if (dev_is_pci(device)) { > - struct pci_dev *pdev = to_pci_dev(device); > - > - if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) { > - reg = iommu_alloc_resv_region(0, 1UL << 24, prot, > - IOMMU_RESV_DIRECT_RELAXABLE, > - GFP_KERNEL); > - if (reg) > - list_add_tail(®->list, head); > - } > - } > -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ > - > reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, > IOAPIC_RANGE_END - > IOAPIC_RANGE_START + 1, > 0, IOMMU_RESV_MSI, GFP_KERNEL); > > This code is introduced in Commit d850c2ee5fe2 ("iommu/vt-d: Expose ISA > direct mapping region via iommu_get_resv_regions") As long as floppy driver exists in the tree, we have to include above code. Otherwise, floppy drivers don't work. At least we can easily find drivers/block/floppy.c which is still maintained (check MAINTAINERS).:-) Best regards, baolu
On 09/11/2022 12:16 pm, Harshit Mogalapalli wrote: > > > On 09/11/22 12:35 pm, Baolu Lu wrote: >> On 2022/11/8 20:58, Harshit Mogalapalli wrote: >>> It is likely that modern intel motherboard will not ship with a >>> floppy connection anymore, so let us disable it by default, as it >>> gets turned on when we do a make defconfig. >>> >>> Suggested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> >>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> >>> --- >>> drivers/iommu/intel/Kconfig | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig >>> index b7dff5092fd2..c783ae85ca9b 100644 >>> --- a/drivers/iommu/intel/Kconfig >>> +++ b/drivers/iommu/intel/Kconfig >>> @@ -76,7 +76,7 @@ config INTEL_IOMMU_BROKEN_GFX_WA >>> option is removed in the 2.6.32 kernel. >>> config INTEL_IOMMU_FLOPPY_WA >>> - def_bool y >>> + def_bool n >>> depends on X86 >>> help >>> Floppy disk drivers are known to bypass DMA API calls >> >> Nobody selects or depends on this. How about removing this bool? Only >> less than 10 lines of code are impacted and are not in any performance >> path. >> >> diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig >> index b7dff5092fd2..5e077d1c5f5d 100644 >> --- a/drivers/iommu/intel/Kconfig >> +++ b/drivers/iommu/intel/Kconfig >> @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA >> to use physical addresses for DMA, at least until this >> option is removed in the 2.6.32 kernel. >> >> -config INTEL_IOMMU_FLOPPY_WA >> - def_bool y >> - depends on X86 >> - help >> - Floppy disk drivers are known to bypass DMA API calls >> - thereby failing to work when IOMMU is enabled. This >> - workaround will setup a 1:1 mapping for the first >> - 16MiB to make floppy (an ISA device) work. >> - >> config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON >> bool "Enable Intel IOMMU scalable mode by default" >> default y >> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c >> index 48cdcd0a5cf3..22801850f339 100644 >> --- a/drivers/iommu/intel/iommu.c >> +++ b/drivers/iommu/intel/iommu.c >> @@ -4567,7 +4567,6 @@ static void intel_iommu_get_resv_regions(struct >> device *device, >> } >> rcu_read_unlock(); >> >> -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA >> if (dev_is_pci(device)) { >> struct pci_dev *pdev = to_pci_dev(device); >> >> @@ -4579,7 +4578,6 @@ static void intel_iommu_get_resv_regions(struct >> device *device, >> list_add_tail(®->list, head); >> } >> } >> -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ >> >> reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, >> IOAPIC_RANGE_END - IOAPIC_RANGE_START + 1, >> > > Hi Baolu, > > I have a question: > Shouldn't we remove the code between ifdef-endif statements? No, machines with floppy drives may be uncommon, but almost certainly some are still in use and being updated with new kernels, so we should continue to support them. Do you have any practical issues with the ISA reserved region being present, or is the only motivation here to save a few bytes of code when it won't be needed? In the latter case, making it "def_bool BLK_DEV_FD" might be sensible. (For the original patch, note that "def_bool n" is always a bit redundant since plain "bool" implicitly defaults to "n" anyway) Thanks, Robin. > I mean something like this: > > diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig > index b7dff5092fd2..5e077d1c5f5d 100644 > --- a/drivers/iommu/intel/Kconfig > +++ b/drivers/iommu/intel/Kconfig > @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA > to use physical addresses for DMA, at least until this > option is removed in the 2.6.32 kernel. > > -config INTEL_IOMMU_FLOPPY_WA > - def_bool y > - depends on X86 > - help > - Floppy disk drivers are known to bypass DMA API calls > - thereby failing to work when IOMMU is enabled. This > - workaround will setup a 1:1 mapping for the first > - 16MiB to make floppy (an ISA device) work. > - > config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON > bool "Enable Intel IOMMU scalable mode by default" > default y > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c > index 48cdcd0a5cf3..2c416ad3204e 100644 > --- a/drivers/iommu/intel/iommu.c > +++ b/drivers/iommu/intel/iommu.c > @@ -4567,20 +4567,6 @@ static void intel_iommu_get_resv_regions(struct > device *device, > } > rcu_read_unlock(); > > -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA > - if (dev_is_pci(device)) { > - struct pci_dev *pdev = to_pci_dev(device); > - > - if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) { > - reg = iommu_alloc_resv_region(0, 1UL << 24, prot, > - IOMMU_RESV_DIRECT_RELAXABLE, > - GFP_KERNEL); > - if (reg) > - list_add_tail(®->list, head); > - } > - } > -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ > - > reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, > IOAPIC_RANGE_END - > IOAPIC_RANGE_START + 1, > 0, IOMMU_RESV_MSI, GFP_KERNEL); > > This code is introduced in Commit d850c2ee5fe2 ("iommu/vt-d: Expose ISA > direct mapping region via iommu_get_resv_regions") > > If others don't have an objection I can send a patch with the above diff. > > Note: I am unsure about the code internals. > > Thanks, > Harshit > > >> Best regards, >> baolu
On 09/11/22 6:56 pm, Robin Murphy wrote: > On 09/11/2022 12:16 pm, Harshit Mogalapalli wrote: >> >> >> On 09/11/22 12:35 pm, Baolu Lu wrote: >>> On 2022/11/8 20:58, Harshit Mogalapalli wrote: >>>> It is likely that modern intel motherboard will not ship with a >>>> floppy connection anymore, so let us disable it by default, as it >>>> gets turned on when we do a make defconfig. >>>> >>>> Suggested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> >>>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> >>>> --- >>>> drivers/iommu/intel/Kconfig | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig >>>> index b7dff5092fd2..c783ae85ca9b 100644 >>>> --- a/drivers/iommu/intel/Kconfig >>>> +++ b/drivers/iommu/intel/Kconfig >>>> @@ -76,7 +76,7 @@ config INTEL_IOMMU_BROKEN_GFX_WA >>>> option is removed in the 2.6.32 kernel. >>>> config INTEL_IOMMU_FLOPPY_WA >>>> - def_bool y >>>> + def_bool n >>>> depends on X86 >>>> help >>>> Floppy disk drivers are known to bypass DMA API calls >>> >>> Nobody selects or depends on this. How about removing this bool? Only >>> less than 10 lines of code are impacted and are not in any performance >>> path. >>> >>> diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig >>> index b7dff5092fd2..5e077d1c5f5d 100644 >>> --- a/drivers/iommu/intel/Kconfig >>> +++ b/drivers/iommu/intel/Kconfig >>> @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA >>> to use physical addresses for DMA, at least until this >>> option is removed in the 2.6.32 kernel. >>> >>> -config INTEL_IOMMU_FLOPPY_WA >>> - def_bool y >>> - depends on X86 >>> - help >>> - Floppy disk drivers are known to bypass DMA API calls >>> - thereby failing to work when IOMMU is enabled. This >>> - workaround will setup a 1:1 mapping for the first >>> - 16MiB to make floppy (an ISA device) work. >>> - >>> config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON >>> bool "Enable Intel IOMMU scalable mode by default" >>> default y >>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c >>> index 48cdcd0a5cf3..22801850f339 100644 >>> --- a/drivers/iommu/intel/iommu.c >>> +++ b/drivers/iommu/intel/iommu.c >>> @@ -4567,7 +4567,6 @@ static void intel_iommu_get_resv_regions(struct >>> device *device, >>> } >>> rcu_read_unlock(); >>> >>> -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA >>> if (dev_is_pci(device)) { >>> struct pci_dev *pdev = to_pci_dev(device); >>> >>> @@ -4579,7 +4578,6 @@ static void intel_iommu_get_resv_regions(struct >>> device *device, >>> list_add_tail(®->list, head); >>> } >>> } >>> -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ >>> >>> reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, >>> IOAPIC_RANGE_END - IOAPIC_RANGE_START + 1, >>> >> >> Hi Baolu, >> >> I have a question: >> Shouldn't we remove the code between ifdef-endif statements? > Hi Baolu and Robin, > No, machines with floppy drives may be uncommon, but almost certainly > some are still in use and being updated with new kernels, so we should > continue to support them. > Thanks a lot for explaining. I misunderstood the code, I thought when we are removing ifdef-endif statement, the code inside(within ifdef-endif) becomes redundant, thanks for correcting me. > Do you have any practical issues with the ISA reserved region being > present, or is the only motivation here to save a few bytes of code when > it won't be needed? In the latter case, making it "def_bool BLK_DEV_FD" > might be sensible. > No practical issues seen. > (For the original patch, note that "def_bool n" is always a bit > redundant since plain "bool" implicitly defaults to "n" anyway) > Okay. How about Baolu's version of the patch, where we are removing the CONFIG item from Kconfig and ifdef statement from the code in iommu.c? Thanks, Harshit > Thanks, > Robin. > >> I mean something like this: >> >> diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig >> index b7dff5092fd2..5e077d1c5f5d 100644 >> --- a/drivers/iommu/intel/Kconfig >> +++ b/drivers/iommu/intel/Kconfig >> @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA >> to use physical addresses for DMA, at least until this >> option is removed in the 2.6.32 kernel. >> >> -config INTEL_IOMMU_FLOPPY_WA >> - def_bool y >> - depends on X86 >> - help >> - Floppy disk drivers are known to bypass DMA API calls >> - thereby failing to work when IOMMU is enabled. This >> - workaround will setup a 1:1 mapping for the first >> - 16MiB to make floppy (an ISA device) work. >> - >> config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON >> bool "Enable Intel IOMMU scalable mode by default" >> default y >> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c >> index 48cdcd0a5cf3..2c416ad3204e 100644 >> --- a/drivers/iommu/intel/iommu.c >> +++ b/drivers/iommu/intel/iommu.c >> @@ -4567,20 +4567,6 @@ static void intel_iommu_get_resv_regions(struct >> device *device, >> } >> rcu_read_unlock(); >> >> -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA >> - if (dev_is_pci(device)) { >> - struct pci_dev *pdev = to_pci_dev(device); >> - >> - if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) { >> - reg = iommu_alloc_resv_region(0, 1UL << 24, prot, >> - IOMMU_RESV_DIRECT_RELAXABLE, >> - GFP_KERNEL); >> - if (reg) >> - list_add_tail(®->list, head); >> - } >> - } >> -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ >> - >> reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, >> IOAPIC_RANGE_END - >> IOAPIC_RANGE_START + 1, >> 0, IOMMU_RESV_MSI, GFP_KERNEL); >> >> This code is introduced in Commit d850c2ee5fe2 ("iommu/vt-d: Expose >> ISA direct mapping region via iommu_get_resv_regions") >> >> If others don't have an objection I can send a patch with the above diff. >> >> Note: I am unsure about the code internals. >> >> Thanks, >> Harshit >> >> >>> Best regards, >>> baolu
On Wed, Nov 09, 2022 at 09:16:53PM +0800, Baolu Lu wrote: > On 2022/11/9 20:16, Harshit Mogalapalli wrote: > > > > > > On 09/11/22 12:35 pm, Baolu Lu wrote: > > > On 2022/11/8 20:58, Harshit Mogalapalli wrote: > > > > It is likely that modern intel motherboard will not ship with a > > > > floppy connection anymore, so let us disable it by default, as it > > > > gets turned on when we do a make defconfig. > > > > > > > > Suggested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > > > Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> > > > > --- > > > > drivers/iommu/intel/Kconfig | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig > > > > index b7dff5092fd2..c783ae85ca9b 100644 > > > > --- a/drivers/iommu/intel/Kconfig > > > > +++ b/drivers/iommu/intel/Kconfig > > > > @@ -76,7 +76,7 @@ config INTEL_IOMMU_BROKEN_GFX_WA > > > > option is removed in the 2.6.32 kernel. > > > > config INTEL_IOMMU_FLOPPY_WA > > > > - def_bool y > > > > + def_bool n > > > > depends on X86 > > > > help > > > > Floppy disk drivers are known to bypass DMA API calls > > > > > > Nobody selects or depends on this. How about removing this bool? Only > > > less than 10 lines of code are impacted and are not in any performance > > > path. > > > > > > diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig > > > index b7dff5092fd2..5e077d1c5f5d 100644 > > > --- a/drivers/iommu/intel/Kconfig > > > +++ b/drivers/iommu/intel/Kconfig > > > @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA > > > to use physical addresses for DMA, at least until this > > > option is removed in the 2.6.32 kernel. > > > > > > -config INTEL_IOMMU_FLOPPY_WA > > > - def_bool y > > > - depends on X86 > > > - help > > > - Floppy disk drivers are known to bypass DMA API calls > > > - thereby failing to work when IOMMU is enabled. This > > > - workaround will setup a 1:1 mapping for the first > > > - 16MiB to make floppy (an ISA device) work. > > > - > > > config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON > > > bool "Enable Intel IOMMU scalable mode by default" > > > default y > > > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c > > > index 48cdcd0a5cf3..22801850f339 100644 > > > --- a/drivers/iommu/intel/iommu.c > > > +++ b/drivers/iommu/intel/iommu.c > > > @@ -4567,7 +4567,6 @@ static void > > > intel_iommu_get_resv_regions(struct device *device, > > > } > > > rcu_read_unlock(); > > > > > > -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA > > > if (dev_is_pci(device)) { > > > struct pci_dev *pdev = to_pci_dev(device); > > > > > > @@ -4579,7 +4578,6 @@ static void > > > intel_iommu_get_resv_regions(struct device *device, > > > list_add_tail(®->list, head); > > > } > > > } > > > -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ > > > > > > reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, > > > IOAPIC_RANGE_END - IOAPIC_RANGE_START + 1, > > > > > > > Hi Baolu, > > > > I have a question: > > Shouldn't we remove the code between ifdef-endif statements? > > > > I mean something like this: > > > > diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig > > index b7dff5092fd2..5e077d1c5f5d 100644 > > --- a/drivers/iommu/intel/Kconfig > > +++ b/drivers/iommu/intel/Kconfig > > @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA > > to use physical addresses for DMA, at least until this > > option is removed in the 2.6.32 kernel. > > > > -config INTEL_IOMMU_FLOPPY_WA > > - def_bool y > > - depends on X86 > > - help > > - Floppy disk drivers are known to bypass DMA API calls > > - thereby failing to work when IOMMU is enabled. This > > - workaround will setup a 1:1 mapping for the first > > - 16MiB to make floppy (an ISA device) work. > > - > > config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON > > bool "Enable Intel IOMMU scalable mode by default" > > default y > > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c > > index 48cdcd0a5cf3..2c416ad3204e 100644 > > --- a/drivers/iommu/intel/iommu.c > > +++ b/drivers/iommu/intel/iommu.c > > @@ -4567,20 +4567,6 @@ static void intel_iommu_get_resv_regions(struct > > device *device, > > } > > rcu_read_unlock(); > > > > -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA > > - if (dev_is_pci(device)) { > > - struct pci_dev *pdev = to_pci_dev(device); > > - > > - if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) { > > - reg = iommu_alloc_resv_region(0, 1UL << 24, prot, > > - IOMMU_RESV_DIRECT_RELAXABLE, > > - GFP_KERNEL); > > - if (reg) > > - list_add_tail(®->list, head); > > - } > > - } > > -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ > > - > > reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, > > IOAPIC_RANGE_END - > > IOAPIC_RANGE_START + 1, > > 0, IOMMU_RESV_MSI, GFP_KERNEL); > > > > This code is introduced in Commit d850c2ee5fe2 ("iommu/vt-d: Expose ISA > > direct mapping region via iommu_get_resv_regions") > > As long as floppy driver exists in the tree, we have to include above > code. Otherwise, floppy drivers don't work. At least we can easily find > drivers/block/floppy.c which is still maintained (check MAINTAINERS).:-) But this requires a machine with Intel IOMMU and ISA: > > > - 16MiB to make floppy (an ISA device) work. ISA device? I don't believe there are any Intel machines with an IOMMU and an ISA device?
On 2022/11/10 4:17, Konrad Rzeszutek Wilk wrote: > On Wed, Nov 09, 2022 at 09:16:53PM +0800, Baolu Lu wrote: >> On 2022/11/9 20:16, Harshit Mogalapalli wrote: >>> >>> >>> On 09/11/22 12:35 pm, Baolu Lu wrote: >>>> On 2022/11/8 20:58, Harshit Mogalapalli wrote: >>>>> It is likely that modern intel motherboard will not ship with a >>>>> floppy connection anymore, so let us disable it by default, as it >>>>> gets turned on when we do a make defconfig. >>>>> >>>>> Suggested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> >>>>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> >>>>> --- >>>>> drivers/iommu/intel/Kconfig | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig >>>>> index b7dff5092fd2..c783ae85ca9b 100644 >>>>> --- a/drivers/iommu/intel/Kconfig >>>>> +++ b/drivers/iommu/intel/Kconfig >>>>> @@ -76,7 +76,7 @@ config INTEL_IOMMU_BROKEN_GFX_WA >>>>> option is removed in the 2.6.32 kernel. >>>>> config INTEL_IOMMU_FLOPPY_WA >>>>> - def_bool y >>>>> + def_bool n >>>>> depends on X86 >>>>> help >>>>> Floppy disk drivers are known to bypass DMA API calls >>>> >>>> Nobody selects or depends on this. How about removing this bool? Only >>>> less than 10 lines of code are impacted and are not in any performance >>>> path. >>>> >>>> diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig >>>> index b7dff5092fd2..5e077d1c5f5d 100644 >>>> --- a/drivers/iommu/intel/Kconfig >>>> +++ b/drivers/iommu/intel/Kconfig >>>> @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA >>>> to use physical addresses for DMA, at least until this >>>> option is removed in the 2.6.32 kernel. >>>> >>>> -config INTEL_IOMMU_FLOPPY_WA >>>> - def_bool y >>>> - depends on X86 >>>> - help >>>> - Floppy disk drivers are known to bypass DMA API calls >>>> - thereby failing to work when IOMMU is enabled. This >>>> - workaround will setup a 1:1 mapping for the first >>>> - 16MiB to make floppy (an ISA device) work. >>>> - >>>> config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON >>>> bool "Enable Intel IOMMU scalable mode by default" >>>> default y >>>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c >>>> index 48cdcd0a5cf3..22801850f339 100644 >>>> --- a/drivers/iommu/intel/iommu.c >>>> +++ b/drivers/iommu/intel/iommu.c >>>> @@ -4567,7 +4567,6 @@ static void >>>> intel_iommu_get_resv_regions(struct device *device, >>>> } >>>> rcu_read_unlock(); >>>> >>>> -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA >>>> if (dev_is_pci(device)) { >>>> struct pci_dev *pdev = to_pci_dev(device); >>>> >>>> @@ -4579,7 +4578,6 @@ static void >>>> intel_iommu_get_resv_regions(struct device *device, >>>> list_add_tail(®->list, head); >>>> } >>>> } >>>> -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ >>>> >>>> reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, >>>> IOAPIC_RANGE_END - IOAPIC_RANGE_START + 1, >>>> >>> >>> Hi Baolu, >>> >>> I have a question: >>> Shouldn't we remove the code between ifdef-endif statements? >>> >>> I mean something like this: >>> >>> diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig >>> index b7dff5092fd2..5e077d1c5f5d 100644 >>> --- a/drivers/iommu/intel/Kconfig >>> +++ b/drivers/iommu/intel/Kconfig >>> @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA >>> to use physical addresses for DMA, at least until this >>> option is removed in the 2.6.32 kernel. >>> >>> -config INTEL_IOMMU_FLOPPY_WA >>> - def_bool y >>> - depends on X86 >>> - help >>> - Floppy disk drivers are known to bypass DMA API calls >>> - thereby failing to work when IOMMU is enabled. This >>> - workaround will setup a 1:1 mapping for the first >>> - 16MiB to make floppy (an ISA device) work. >>> - >>> config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON >>> bool "Enable Intel IOMMU scalable mode by default" >>> default y >>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c >>> index 48cdcd0a5cf3..2c416ad3204e 100644 >>> --- a/drivers/iommu/intel/iommu.c >>> +++ b/drivers/iommu/intel/iommu.c >>> @@ -4567,20 +4567,6 @@ static void intel_iommu_get_resv_regions(struct >>> device *device, >>> } >>> rcu_read_unlock(); >>> >>> -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA >>> - if (dev_is_pci(device)) { >>> - struct pci_dev *pdev = to_pci_dev(device); >>> - >>> - if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) { >>> - reg = iommu_alloc_resv_region(0, 1UL << 24, prot, >>> - IOMMU_RESV_DIRECT_RELAXABLE, >>> - GFP_KERNEL); >>> - if (reg) >>> - list_add_tail(®->list, head); >>> - } >>> - } >>> -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ >>> - >>> reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, >>> IOAPIC_RANGE_END - >>> IOAPIC_RANGE_START + 1, >>> 0, IOMMU_RESV_MSI, GFP_KERNEL); >>> >>> This code is introduced in Commit d850c2ee5fe2 ("iommu/vt-d: Expose ISA >>> direct mapping region via iommu_get_resv_regions") >> >> As long as floppy driver exists in the tree, we have to include above >> code. Otherwise, floppy drivers don't work. At least we can easily find >> drivers/block/floppy.c which is still maintained (check MAINTAINERS).:-) > > But this requires a machine with Intel IOMMU and ISA: > >>>> - 16MiB to make floppy (an ISA device) work. > > ISA device? I don't believe there are any Intel machines with an IOMMU > and an ISA device? This workaround was introduced by commit 49a0429e53f2 ("Intel IOMMU: Iommu floppy workaround") in 2007. I can't remember what happened 15 years ago, but I believe there must have been corresponding hardware configurations at that time, and the Linux kernel has been maintained it to now. Best regards, baolu
On Thu, Nov 10, 2022 at 02:39:53PM +0800, Baolu Lu wrote: > On 2022/11/10 4:17, Konrad Rzeszutek Wilk wrote: > > On Wed, Nov 09, 2022 at 09:16:53PM +0800, Baolu Lu wrote: > > > On 2022/11/9 20:16, Harshit Mogalapalli wrote: > > > > > > > > > > > > On 09/11/22 12:35 pm, Baolu Lu wrote: > > > > > On 2022/11/8 20:58, Harshit Mogalapalli wrote: > > > > > > It is likely that modern intel motherboard will not ship with a > > > > > > floppy connection anymore, so let us disable it by default, as it > > > > > > gets turned on when we do a make defconfig. > > > > > > > > > > > > Suggested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > > > > > Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> > > > > > > --- > > > > > > drivers/iommu/intel/Kconfig | 2 +- > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig > > > > > > index b7dff5092fd2..c783ae85ca9b 100644 > > > > > > --- a/drivers/iommu/intel/Kconfig > > > > > > +++ b/drivers/iommu/intel/Kconfig > > > > > > @@ -76,7 +76,7 @@ config INTEL_IOMMU_BROKEN_GFX_WA > > > > > > option is removed in the 2.6.32 kernel. > > > > > > config INTEL_IOMMU_FLOPPY_WA > > > > > > - def_bool y > > > > > > + def_bool n > > > > > > depends on X86 > > > > > > help > > > > > > Floppy disk drivers are known to bypass DMA API calls > > > > > > > > > > Nobody selects or depends on this. How about removing this bool? Only > > > > > less than 10 lines of code are impacted and are not in any performance > > > > > path. > > > > > > > > > > diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig > > > > > index b7dff5092fd2..5e077d1c5f5d 100644 > > > > > --- a/drivers/iommu/intel/Kconfig > > > > > +++ b/drivers/iommu/intel/Kconfig > > > > > @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA > > > > > to use physical addresses for DMA, at least until this > > > > > option is removed in the 2.6.32 kernel. > > > > > > > > > > -config INTEL_IOMMU_FLOPPY_WA > > > > > - def_bool y > > > > > - depends on X86 > > > > > - help > > > > > - Floppy disk drivers are known to bypass DMA API calls > > > > > - thereby failing to work when IOMMU is enabled. This > > > > > - workaround will setup a 1:1 mapping for the first > > > > > - 16MiB to make floppy (an ISA device) work. > > > > > - > > > > > config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON > > > > > bool "Enable Intel IOMMU scalable mode by default" > > > > > default y > > > > > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c > > > > > index 48cdcd0a5cf3..22801850f339 100644 > > > > > --- a/drivers/iommu/intel/iommu.c > > > > > +++ b/drivers/iommu/intel/iommu.c > > > > > @@ -4567,7 +4567,6 @@ static void > > > > > intel_iommu_get_resv_regions(struct device *device, > > > > > } > > > > > rcu_read_unlock(); > > > > > > > > > > -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA > > > > > if (dev_is_pci(device)) { > > > > > struct pci_dev *pdev = to_pci_dev(device); > > > > > > > > > > @@ -4579,7 +4578,6 @@ static void > > > > > intel_iommu_get_resv_regions(struct device *device, > > > > > list_add_tail(®->list, head); > > > > > } > > > > > } > > > > > -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ > > > > > > > > > > reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, > > > > > IOAPIC_RANGE_END - IOAPIC_RANGE_START + 1, > > > > > > > > > > > > > Hi Baolu, > > > > > > > > I have a question: > > > > Shouldn't we remove the code between ifdef-endif statements? > > > > > > > > I mean something like this: > > > > > > > > diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig > > > > index b7dff5092fd2..5e077d1c5f5d 100644 > > > > --- a/drivers/iommu/intel/Kconfig > > > > +++ b/drivers/iommu/intel/Kconfig > > > > @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA > > > > to use physical addresses for DMA, at least until this > > > > option is removed in the 2.6.32 kernel. > > > > > > > > -config INTEL_IOMMU_FLOPPY_WA > > > > - def_bool y > > > > - depends on X86 > > > > - help > > > > - Floppy disk drivers are known to bypass DMA API calls > > > > - thereby failing to work when IOMMU is enabled. This > > > > - workaround will setup a 1:1 mapping for the first > > > > - 16MiB to make floppy (an ISA device) work. > > > > - > > > > config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON > > > > bool "Enable Intel IOMMU scalable mode by default" > > > > default y > > > > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c > > > > index 48cdcd0a5cf3..2c416ad3204e 100644 > > > > --- a/drivers/iommu/intel/iommu.c > > > > +++ b/drivers/iommu/intel/iommu.c > > > > @@ -4567,20 +4567,6 @@ static void intel_iommu_get_resv_regions(struct > > > > device *device, > > > > } > > > > rcu_read_unlock(); > > > > > > > > -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA > > > > - if (dev_is_pci(device)) { > > > > - struct pci_dev *pdev = to_pci_dev(device); > > > > - > > > > - if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) { > > > > - reg = iommu_alloc_resv_region(0, 1UL << 24, prot, > > > > - IOMMU_RESV_DIRECT_RELAXABLE, > > > > - GFP_KERNEL); > > > > - if (reg) > > > > - list_add_tail(®->list, head); > > > > - } > > > > - } > > > > -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ > > > > - > > > > reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, > > > > IOAPIC_RANGE_END - > > > > IOAPIC_RANGE_START + 1, > > > > 0, IOMMU_RESV_MSI, GFP_KERNEL); > > > > > > > > This code is introduced in Commit d850c2ee5fe2 ("iommu/vt-d: Expose ISA > > > > direct mapping region via iommu_get_resv_regions") > > > > > > As long as floppy driver exists in the tree, we have to include above > > > code. Otherwise, floppy drivers don't work. At least we can easily find > > > drivers/block/floppy.c which is still maintained (check MAINTAINERS).:-) > > > > But this requires a machine with Intel IOMMU and ISA: > > > > > > > - 16MiB to make floppy (an ISA device) work. > > > > ISA device? I don't believe there are any Intel machines with an IOMMU > > and an ISA device? > > This workaround was introduced by commit 49a0429e53f2 ("Intel IOMMU: > Iommu floppy workaround") in 2007. I can't remember what happened 15 > years ago, but I believe there must have been corresponding hardware > configurations at that time, and the Linux kernel has been maintained it > to now. At what point can this be removed then?
On 2022/11/11 5:00, Konrad Rzeszutek Wilk wrote: > On Thu, Nov 10, 2022 at 02:39:53PM +0800, Baolu Lu wrote: >> On 2022/11/10 4:17, Konrad Rzeszutek Wilk wrote: >>> On Wed, Nov 09, 2022 at 09:16:53PM +0800, Baolu Lu wrote: >>>> On 2022/11/9 20:16, Harshit Mogalapalli wrote: >>>>> >>>>> >>>>> On 09/11/22 12:35 pm, Baolu Lu wrote: >>>>>> On 2022/11/8 20:58, Harshit Mogalapalli wrote: >>>>>>> It is likely that modern intel motherboard will not ship with a >>>>>>> floppy connection anymore, so let us disable it by default, as it >>>>>>> gets turned on when we do a make defconfig. >>>>>>> >>>>>>> Suggested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> >>>>>>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> >>>>>>> --- >>>>>>> drivers/iommu/intel/Kconfig | 2 +- >>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig >>>>>>> index b7dff5092fd2..c783ae85ca9b 100644 >>>>>>> --- a/drivers/iommu/intel/Kconfig >>>>>>> +++ b/drivers/iommu/intel/Kconfig >>>>>>> @@ -76,7 +76,7 @@ config INTEL_IOMMU_BROKEN_GFX_WA >>>>>>> option is removed in the 2.6.32 kernel. >>>>>>> config INTEL_IOMMU_FLOPPY_WA >>>>>>> - def_bool y >>>>>>> + def_bool n >>>>>>> depends on X86 >>>>>>> help >>>>>>> Floppy disk drivers are known to bypass DMA API calls >>>>>> >>>>>> Nobody selects or depends on this. How about removing this bool? Only >>>>>> less than 10 lines of code are impacted and are not in any performance >>>>>> path. >>>>>> >>>>>> diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig >>>>>> index b7dff5092fd2..5e077d1c5f5d 100644 >>>>>> --- a/drivers/iommu/intel/Kconfig >>>>>> +++ b/drivers/iommu/intel/Kconfig >>>>>> @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA >>>>>> to use physical addresses for DMA, at least until this >>>>>> option is removed in the 2.6.32 kernel. >>>>>> >>>>>> -config INTEL_IOMMU_FLOPPY_WA >>>>>> - def_bool y >>>>>> - depends on X86 >>>>>> - help >>>>>> - Floppy disk drivers are known to bypass DMA API calls >>>>>> - thereby failing to work when IOMMU is enabled. This >>>>>> - workaround will setup a 1:1 mapping for the first >>>>>> - 16MiB to make floppy (an ISA device) work. >>>>>> - >>>>>> config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON >>>>>> bool "Enable Intel IOMMU scalable mode by default" >>>>>> default y >>>>>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c >>>>>> index 48cdcd0a5cf3..22801850f339 100644 >>>>>> --- a/drivers/iommu/intel/iommu.c >>>>>> +++ b/drivers/iommu/intel/iommu.c >>>>>> @@ -4567,7 +4567,6 @@ static void >>>>>> intel_iommu_get_resv_regions(struct device *device, >>>>>> } >>>>>> rcu_read_unlock(); >>>>>> >>>>>> -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA >>>>>> if (dev_is_pci(device)) { >>>>>> struct pci_dev *pdev = to_pci_dev(device); >>>>>> >>>>>> @@ -4579,7 +4578,6 @@ static void >>>>>> intel_iommu_get_resv_regions(struct device *device, >>>>>> list_add_tail(®->list, head); >>>>>> } >>>>>> } >>>>>> -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ >>>>>> >>>>>> reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, >>>>>> IOAPIC_RANGE_END - IOAPIC_RANGE_START + 1, >>>>>> >>>>> >>>>> Hi Baolu, >>>>> >>>>> I have a question: >>>>> Shouldn't we remove the code between ifdef-endif statements? >>>>> >>>>> I mean something like this: >>>>> >>>>> diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig >>>>> index b7dff5092fd2..5e077d1c5f5d 100644 >>>>> --- a/drivers/iommu/intel/Kconfig >>>>> +++ b/drivers/iommu/intel/Kconfig >>>>> @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA >>>>> to use physical addresses for DMA, at least until this >>>>> option is removed in the 2.6.32 kernel. >>>>> >>>>> -config INTEL_IOMMU_FLOPPY_WA >>>>> - def_bool y >>>>> - depends on X86 >>>>> - help >>>>> - Floppy disk drivers are known to bypass DMA API calls >>>>> - thereby failing to work when IOMMU is enabled. This >>>>> - workaround will setup a 1:1 mapping for the first >>>>> - 16MiB to make floppy (an ISA device) work. >>>>> - >>>>> config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON >>>>> bool "Enable Intel IOMMU scalable mode by default" >>>>> default y >>>>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c >>>>> index 48cdcd0a5cf3..2c416ad3204e 100644 >>>>> --- a/drivers/iommu/intel/iommu.c >>>>> +++ b/drivers/iommu/intel/iommu.c >>>>> @@ -4567,20 +4567,6 @@ static void intel_iommu_get_resv_regions(struct >>>>> device *device, >>>>> } >>>>> rcu_read_unlock(); >>>>> >>>>> -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA >>>>> - if (dev_is_pci(device)) { >>>>> - struct pci_dev *pdev = to_pci_dev(device); >>>>> - >>>>> - if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) { >>>>> - reg = iommu_alloc_resv_region(0, 1UL << 24, prot, >>>>> - IOMMU_RESV_DIRECT_RELAXABLE, >>>>> - GFP_KERNEL); >>>>> - if (reg) >>>>> - list_add_tail(®->list, head); >>>>> - } >>>>> - } >>>>> -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ >>>>> - >>>>> reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, >>>>> IOAPIC_RANGE_END - >>>>> IOAPIC_RANGE_START + 1, >>>>> 0, IOMMU_RESV_MSI, GFP_KERNEL); >>>>> >>>>> This code is introduced in Commit d850c2ee5fe2 ("iommu/vt-d: Expose ISA >>>>> direct mapping region via iommu_get_resv_regions") >>>> >>>> As long as floppy driver exists in the tree, we have to include above >>>> code. Otherwise, floppy drivers don't work. At least we can easily find >>>> drivers/block/floppy.c which is still maintained (check MAINTAINERS).:-) >>> >>> But this requires a machine with Intel IOMMU and ISA: >>> >>>>>> - 16MiB to make floppy (an ISA device) work. >>> >>> ISA device? I don't believe there are any Intel machines with an IOMMU >>> and an ISA device? >> >> This workaround was introduced by commit 49a0429e53f2 ("Intel IOMMU: >> Iommu floppy workaround") in 2007. I can't remember what happened 15 >> years ago, but I believe there must have been corresponding hardware >> configurations at that time, and the Linux kernel has been maintained it >> to now. > > At what point can this be removed then? No floppy block drivers in the tree or all floppy drivers' DMA going through the kernel DMA APIs. Best regards, baolu
On 2022-11-11 02:31, Baolu Lu wrote: > On 2022/11/11 5:00, Konrad Rzeszutek Wilk wrote: >> On Thu, Nov 10, 2022 at 02:39:53PM +0800, Baolu Lu wrote: >>> On 2022/11/10 4:17, Konrad Rzeszutek Wilk wrote: >>>> On Wed, Nov 09, 2022 at 09:16:53PM +0800, Baolu Lu wrote: >>>>> On 2022/11/9 20:16, Harshit Mogalapalli wrote: >>>>>> >>>>>> >>>>>> On 09/11/22 12:35 pm, Baolu Lu wrote: >>>>>>> On 2022/11/8 20:58, Harshit Mogalapalli wrote: >>>>>>>> It is likely that modern intel motherboard will not ship with a >>>>>>>> floppy connection anymore, so let us disable it by default, as it >>>>>>>> gets turned on when we do a make defconfig. >>>>>>>> >>>>>>>> Suggested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> >>>>>>>> Signed-off-by: Harshit Mogalapalli >>>>>>>> <harshit.m.mogalapalli@oracle.com> >>>>>>>> --- >>>>>>>> drivers/iommu/intel/Kconfig | 2 +- >>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git a/drivers/iommu/intel/Kconfig >>>>>>>> b/drivers/iommu/intel/Kconfig >>>>>>>> index b7dff5092fd2..c783ae85ca9b 100644 >>>>>>>> --- a/drivers/iommu/intel/Kconfig >>>>>>>> +++ b/drivers/iommu/intel/Kconfig >>>>>>>> @@ -76,7 +76,7 @@ config INTEL_IOMMU_BROKEN_GFX_WA >>>>>>>> option is removed in the 2.6.32 kernel. >>>>>>>> config INTEL_IOMMU_FLOPPY_WA >>>>>>>> - def_bool y >>>>>>>> + def_bool n >>>>>>>> depends on X86 >>>>>>>> help >>>>>>>> Floppy disk drivers are known to bypass DMA API calls >>>>>>> >>>>>>> Nobody selects or depends on this. How about removing this bool? >>>>>>> Only >>>>>>> less than 10 lines of code are impacted and are not in any >>>>>>> performance >>>>>>> path. >>>>>>> >>>>>>> diff --git a/drivers/iommu/intel/Kconfig >>>>>>> b/drivers/iommu/intel/Kconfig >>>>>>> index b7dff5092fd2..5e077d1c5f5d 100644 >>>>>>> --- a/drivers/iommu/intel/Kconfig >>>>>>> +++ b/drivers/iommu/intel/Kconfig >>>>>>> @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA >>>>>>> to use physical addresses for DMA, at least until this >>>>>>> option is removed in the 2.6.32 kernel. >>>>>>> >>>>>>> -config INTEL_IOMMU_FLOPPY_WA >>>>>>> - def_bool y >>>>>>> - depends on X86 >>>>>>> - help >>>>>>> - Floppy disk drivers are known to bypass DMA API calls >>>>>>> - thereby failing to work when IOMMU is enabled. This >>>>>>> - workaround will setup a 1:1 mapping for the first >>>>>>> - 16MiB to make floppy (an ISA device) work. >>>>>>> - >>>>>>> config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON >>>>>>> bool "Enable Intel IOMMU scalable mode by default" >>>>>>> default y >>>>>>> diff --git a/drivers/iommu/intel/iommu.c >>>>>>> b/drivers/iommu/intel/iommu.c >>>>>>> index 48cdcd0a5cf3..22801850f339 100644 >>>>>>> --- a/drivers/iommu/intel/iommu.c >>>>>>> +++ b/drivers/iommu/intel/iommu.c >>>>>>> @@ -4567,7 +4567,6 @@ static void >>>>>>> intel_iommu_get_resv_regions(struct device *device, >>>>>>> } >>>>>>> rcu_read_unlock(); >>>>>>> >>>>>>> -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA >>>>>>> if (dev_is_pci(device)) { >>>>>>> struct pci_dev *pdev = to_pci_dev(device); >>>>>>> >>>>>>> @@ -4579,7 +4578,6 @@ static void >>>>>>> intel_iommu_get_resv_regions(struct device *device, >>>>>>> list_add_tail(®->list, head); >>>>>>> } >>>>>>> } >>>>>>> -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ >>>>>>> >>>>>>> reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, >>>>>>> IOAPIC_RANGE_END - IOAPIC_RANGE_START + 1, >>>>>>> >>>>>> >>>>>> Hi Baolu, >>>>>> >>>>>> I have a question: >>>>>> Shouldn't we remove the code between ifdef-endif statements? >>>>>> >>>>>> I mean something like this: >>>>>> >>>>>> diff --git a/drivers/iommu/intel/Kconfig >>>>>> b/drivers/iommu/intel/Kconfig >>>>>> index b7dff5092fd2..5e077d1c5f5d 100644 >>>>>> --- a/drivers/iommu/intel/Kconfig >>>>>> +++ b/drivers/iommu/intel/Kconfig >>>>>> @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA >>>>>> to use physical addresses for DMA, at least until this >>>>>> option is removed in the 2.6.32 kernel. >>>>>> >>>>>> -config INTEL_IOMMU_FLOPPY_WA >>>>>> - def_bool y >>>>>> - depends on X86 >>>>>> - help >>>>>> - Floppy disk drivers are known to bypass DMA API calls >>>>>> - thereby failing to work when IOMMU is enabled. This >>>>>> - workaround will setup a 1:1 mapping for the first >>>>>> - 16MiB to make floppy (an ISA device) work. >>>>>> - >>>>>> config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON >>>>>> bool "Enable Intel IOMMU scalable mode by default" >>>>>> default y >>>>>> diff --git a/drivers/iommu/intel/iommu.c >>>>>> b/drivers/iommu/intel/iommu.c >>>>>> index 48cdcd0a5cf3..2c416ad3204e 100644 >>>>>> --- a/drivers/iommu/intel/iommu.c >>>>>> +++ b/drivers/iommu/intel/iommu.c >>>>>> @@ -4567,20 +4567,6 @@ static void >>>>>> intel_iommu_get_resv_regions(struct >>>>>> device *device, >>>>>> } >>>>>> rcu_read_unlock(); >>>>>> >>>>>> -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA >>>>>> - if (dev_is_pci(device)) { >>>>>> - struct pci_dev *pdev = to_pci_dev(device); >>>>>> - >>>>>> - if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) { >>>>>> - reg = iommu_alloc_resv_region(0, 1UL << >>>>>> 24, prot, >>>>>> - IOMMU_RESV_DIRECT_RELAXABLE, >>>>>> - GFP_KERNEL); >>>>>> - if (reg) >>>>>> - list_add_tail(®->list, head); >>>>>> - } >>>>>> - } >>>>>> -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ >>>>>> - >>>>>> reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, >>>>>> IOAPIC_RANGE_END - >>>>>> IOAPIC_RANGE_START + 1, >>>>>> 0, IOMMU_RESV_MSI, >>>>>> GFP_KERNEL); >>>>>> >>>>>> This code is introduced in Commit d850c2ee5fe2 ("iommu/vt-d: >>>>>> Expose ISA >>>>>> direct mapping region via iommu_get_resv_regions") >>>>> >>>>> As long as floppy driver exists in the tree, we have to include above >>>>> code. Otherwise, floppy drivers don't work. At least we can easily >>>>> find >>>>> drivers/block/floppy.c which is still maintained (check >>>>> MAINTAINERS).:-) >>>> >>>> But this requires a machine with Intel IOMMU and ISA: >>>> >>>>>>> - 16MiB to make floppy (an ISA device) work. >>>> >>>> ISA device? I don't believe there are any Intel machines with an IOMMU >>>> and an ISA device? >>> >>> This workaround was introduced by commit 49a0429e53f2 ("Intel IOMMU: >>> Iommu floppy workaround") in 2007. I can't remember what happened 15 >>> years ago, but I believe there must have been corresponding hardware >>> configurations at that time, and the Linux kernel has been maintained it >>> to now. >> >> At what point can this be removed then? > > No floppy block drivers in the tree or all floppy drivers' DMA going > through the kernel DMA APIs. Presumably the point of the config is that ISA bridges aren't expected on IA-64, so a tiny code saving can be made there, but at this point is anyone really that bothered about any more? There's already tons more code all through the driver to support newer features that aren't meaningful to old IA-64 hardware, so in my opinion, meh. Given that this already won't affect systems that truly can't have any ISA devices, I don't see any issue with simply making it unconditional. Note that plenty of chipsets still have an "ISA" bridge to an LPC interface, so ISA DMA is not necessarily as dead as one might like to think. Robin.
On Friday, 2022-11-11 at 12:36:48 UTC, Robin Murphy wrote: > On 2022-11-11 02:31, Baolu Lu wrote: >> On 2022/11/11 5:00, Konrad Rzeszutek Wilk wrote: >>> On Thu, Nov 10, 2022 at 02:39:53PM +0800, Baolu Lu wrote: >>>> On 2022/11/10 4:17, Konrad Rzeszutek Wilk wrote: >>>>> On Wed, Nov 09, 2022 at 09:16:53PM +0800, Baolu Lu wrote: >>>>>> On 2022/11/9 20:16, Harshit Mogalapalli wrote: >>>>>>> >>>>>>> >>>>>>> On 09/11/22 12:35 pm, Baolu Lu wrote: >>>>>>>> On 2022/11/8 20:58, Harshit Mogalapalli wrote: >>>>>>>>> It is likely that modern intel motherboard will not ship with a >>>>>>>>> floppy connection anymore, so let us disable it by default, as it >>>>>>>>> gets turned on when we do a make defconfig. >>>>>>>>> >>>>>>>>> Suggested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> >>>>>>>>> Signed-off-by: Harshit Mogalapalli >>>>>>>>> <harshit.m.mogalapalli@oracle.com> >>>>>>>>> --- >>>>>>>>> drivers/iommu/intel/Kconfig | 2 +- >>>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>>>> >>>>>>>>> diff --git a/drivers/iommu/intel/Kconfig >>>>>>>>> b/drivers/iommu/intel/Kconfig >>>>>>>>> index b7dff5092fd2..c783ae85ca9b 100644 >>>>>>>>> --- a/drivers/iommu/intel/Kconfig >>>>>>>>> +++ b/drivers/iommu/intel/Kconfig >>>>>>>>> @@ -76,7 +76,7 @@ config INTEL_IOMMU_BROKEN_GFX_WA >>>>>>>>> option is removed in the 2.6.32 kernel. >>>>>>>>> config INTEL_IOMMU_FLOPPY_WA >>>>>>>>> - def_bool y >>>>>>>>> + def_bool n >>>>>>>>> depends on X86 >>>>>>>>> help >>>>>>>>> Floppy disk drivers are known to bypass DMA API calls >>>>>>>> >>>>>>>> Nobody selects or depends on this. How about removing this bool? >>>>>>>> Only >>>>>>>> less than 10 lines of code are impacted and are not in any >>>>>>>> performance >>>>>>>> path. >>>>>>>> >>>>>>>> diff --git a/drivers/iommu/intel/Kconfig >>>>>>>> b/drivers/iommu/intel/Kconfig >>>>>>>> index b7dff5092fd2..5e077d1c5f5d 100644 >>>>>>>> --- a/drivers/iommu/intel/Kconfig >>>>>>>> +++ b/drivers/iommu/intel/Kconfig >>>>>>>> @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA >>>>>>>> to use physical addresses for DMA, at least until this >>>>>>>> option is removed in the 2.6.32 kernel. >>>>>>>> >>>>>>>> -config INTEL_IOMMU_FLOPPY_WA >>>>>>>> - def_bool y >>>>>>>> - depends on X86 >>>>>>>> - help >>>>>>>> - Floppy disk drivers are known to bypass DMA API calls >>>>>>>> - thereby failing to work when IOMMU is enabled. This >>>>>>>> - workaround will setup a 1:1 mapping for the first >>>>>>>> - 16MiB to make floppy (an ISA device) work. >>>>>>>> - >>>>>>>> config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON >>>>>>>> bool "Enable Intel IOMMU scalable mode by default" >>>>>>>> default y >>>>>>>> diff --git a/drivers/iommu/intel/iommu.c >>>>>>>> b/drivers/iommu/intel/iommu.c >>>>>>>> index 48cdcd0a5cf3..22801850f339 100644 >>>>>>>> --- a/drivers/iommu/intel/iommu.c >>>>>>>> +++ b/drivers/iommu/intel/iommu.c >>>>>>>> @@ -4567,7 +4567,6 @@ static void >>>>>>>> intel_iommu_get_resv_regions(struct device *device, >>>>>>>> } >>>>>>>> rcu_read_unlock(); >>>>>>>> >>>>>>>> -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA >>>>>>>> if (dev_is_pci(device)) { >>>>>>>> struct pci_dev *pdev = to_pci_dev(device); >>>>>>>> >>>>>>>> @@ -4579,7 +4578,6 @@ static void >>>>>>>> intel_iommu_get_resv_regions(struct device *device, >>>>>>>> list_add_tail(®->list, head); >>>>>>>> } >>>>>>>> } >>>>>>>> -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ >>>>>>>> >>>>>>>> reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, >>>>>>>> IOAPIC_RANGE_END - IOAPIC_RANGE_START + 1, >>>>>>>> >>>>>>> >>>>>>> Hi Baolu, >>>>>>> >>>>>>> I have a question: >>>>>>> Shouldn't we remove the code between ifdef-endif statements? >>>>>>> >>>>>>> I mean something like this: >>>>>>> >>>>>>> diff --git a/drivers/iommu/intel/Kconfig >>>>>>> b/drivers/iommu/intel/Kconfig >>>>>>> index b7dff5092fd2..5e077d1c5f5d 100644 >>>>>>> --- a/drivers/iommu/intel/Kconfig >>>>>>> +++ b/drivers/iommu/intel/Kconfig >>>>>>> @@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA >>>>>>> to use physical addresses for DMA, at least until this >>>>>>> option is removed in the 2.6.32 kernel. >>>>>>> >>>>>>> -config INTEL_IOMMU_FLOPPY_WA >>>>>>> - def_bool y >>>>>>> - depends on X86 >>>>>>> - help >>>>>>> - Floppy disk drivers are known to bypass DMA API calls >>>>>>> - thereby failing to work when IOMMU is enabled. This >>>>>>> - workaround will setup a 1:1 mapping for the first >>>>>>> - 16MiB to make floppy (an ISA device) work. >>>>>>> - >>>>>>> config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON >>>>>>> bool "Enable Intel IOMMU scalable mode by default" >>>>>>> default y >>>>>>> diff --git a/drivers/iommu/intel/iommu.c >>>>>>> b/drivers/iommu/intel/iommu.c >>>>>>> index 48cdcd0a5cf3..2c416ad3204e 100644 >>>>>>> --- a/drivers/iommu/intel/iommu.c >>>>>>> +++ b/drivers/iommu/intel/iommu.c >>>>>>> @@ -4567,20 +4567,6 @@ static void >>>>>>> intel_iommu_get_resv_regions(struct >>>>>>> device *device, >>>>>>> } >>>>>>> rcu_read_unlock(); >>>>>>> >>>>>>> -#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA >>>>>>> - if (dev_is_pci(device)) { >>>>>>> - struct pci_dev *pdev = to_pci_dev(device); >>>>>>> - >>>>>>> - if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) { >>>>>>> - reg = iommu_alloc_resv_region(0, 1UL << >>>>>>> 24, prot, >>>>>>> - IOMMU_RESV_DIRECT_RELAXABLE, >>>>>>> - GFP_KERNEL); >>>>>>> - if (reg) >>>>>>> - list_add_tail(®->list, head); >>>>>>> - } >>>>>>> - } >>>>>>> -#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */ >>>>>>> - >>>>>>> reg = iommu_alloc_resv_region(IOAPIC_RANGE_START, >>>>>>> IOAPIC_RANGE_END - >>>>>>> IOAPIC_RANGE_START + 1, >>>>>>> 0, IOMMU_RESV_MSI, >>>>>>> GFP_KERNEL); >>>>>>> >>>>>>> This code is introduced in Commit d850c2ee5fe2 ("iommu/vt-d: >>>>>>> Expose ISA >>>>>>> direct mapping region via iommu_get_resv_regions") >>>>>> >>>>>> As long as floppy driver exists in the tree, we have to include above >>>>>> code. Otherwise, floppy drivers don't work. At least we can easily >>>>>> find >>>>>> drivers/block/floppy.c which is still maintained (check >>>>>> MAINTAINERS).:-) >>>>> >>>>> But this requires a machine with Intel IOMMU and ISA: >>>>> >>>>>>>> - 16MiB to make floppy (an ISA device) work. >>>>> >>>>> ISA device? I don't believe there are any Intel machines with an IOMMU >>>>> and an ISA device? >>>> >>>> This workaround was introduced by commit 49a0429e53f2 ("Intel IOMMU: >>>> Iommu floppy workaround") in 2007. I can't remember what happened 15 >>>> years ago, but I believe there must have been corresponding hardware >>>> configurations at that time, and the Linux kernel has been maintained it >>>> to now. >>> >>> At what point can this be removed then? >> >> No floppy block drivers in the tree or all floppy drivers' DMA going >> through the kernel DMA APIs. > > Presumably the point of the config is that ISA bridges aren't expected > on IA-64, so a tiny code saving can be made there, but at this point is > anyone really that bothered about any more? There's already tons more > code all through the driver to support newer features that aren't > meaningful to old IA-64 hardware, so in my opinion, meh. Given that this > already won't affect systems that truly can't have any ISA devices, I > don't see any issue with simply making it unconditional. > > Note that plenty of chipsets still have an "ISA" bridge to an LPC > interface, so ISA DMA is not necessarily as dead as one might like to think. > Based on what you're saying then, the fix that you agree with is what Baolu first suggested, which is to remove it from Kconfig and just remove the #ifdef/#endif, but not the code that it contains. Is that correct? Thanks, Darren.
diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig index b7dff5092fd2..c783ae85ca9b 100644 --- a/drivers/iommu/intel/Kconfig +++ b/drivers/iommu/intel/Kconfig @@ -76,7 +76,7 @@ config INTEL_IOMMU_BROKEN_GFX_WA option is removed in the 2.6.32 kernel. config INTEL_IOMMU_FLOPPY_WA - def_bool y + def_bool n depends on X86 help Floppy disk drivers are known to bypass DMA API calls