mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v4 0/4] ACPI: LoongArch: Add IOVT table support
@ 2026-09-22 12:10 Bibo Mao
  2026-09-22 12:10 ` [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure Bibo Mao
                   ` (3 more replies)
  0 siblings, 4 replies; 22+ messages in thread
From: Bibo Mao @ 2026-09-22 12:10 UTC (permalink / raw)
  To: Huacai Chen, Rafael J . Wysocki, Maciej Wieczor-Retman,
	Pawel Chmielewski
  Cc: Len Brown, linux-kernel, loongarch, linux-acpi, acpica-devel, lixianglai

I/O Virtualization Table is ACPI table for LoongArch IOMMU devices, it
is merged on ACPI Spec 6.6, and it can be located at
  https://www.loongson.cn/uploads/images/2024110517404135188.LoongArch-IO-Virtualization-Table-Specification.pdf

Here add IOVT table parsing on LoongArch system, it is for LoongArch
IOMMU driver use in future.
---
v3 ... v4:
  1. Move kernel option ACPI_IOVT from patch 4 to patch 2.
  2. Move function call with iovt_iommu_configure_id() before function
     viot_iommu_configure() in acpi_iommu_configure_id().
  3. Rename LoongArch with LOONGARCH like other architecture in file
     MAINTAINERS.
  4. Set Huacai as one of Maintainers.

v2 ... v3:
  1. Add separate patch with header file include/linux/acpi_iovt.h,
     after it is merged in acpica tree.
  2. Modify patch set since macro name is changed in acpi_iovt.h
  3. Merge patch 1 and patch 2 together.

v1 ... v2:
  1. Reorganize the patch, patch 1/4 are merged, patch 3/5/6 are merged,
     add patch 2/8 are merged together.
  2. Enrich error printk information in iovt_get_pci_iommu_fwnode().
---
Bibo Mao (4):
  ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT
    IOMMU structure
  ACPI: LoongArch: Add acpi_arch_[late]_init()
  ACPI: scan: Add support for LoongArch in acpi_iommu_configure_id()
  MAINTAINERS: Add entry for drivers/acpi/loongarch

 MAINTAINERS                     |   9 +
 arch/loongarch/Kconfig          |   1 +
 drivers/acpi/Kconfig            |   4 +
 drivers/acpi/Makefile           |   2 +-
 drivers/acpi/bus.c              |   2 +
 drivers/acpi/loongarch/Kconfig  |   7 +
 drivers/acpi/loongarch/Makefile |   3 +
 drivers/acpi/loongarch/init.c   |  16 ++
 drivers/acpi/loongarch/init.h   |   5 +
 drivers/acpi/loongarch/iovt.c   | 280 ++++++++++++++++++++++++++++++++
 drivers/acpi/scan.c             |   3 +
 include/acpi/actbl2.h           |   7 +
 include/linux/acpi.h            |   1 +
 include/linux/acpi_iovt.h       |  13 ++
 14 files changed, 352 insertions(+), 1 deletion(-)
 create mode 100644 drivers/acpi/loongarch/Kconfig
 create mode 100644 drivers/acpi/loongarch/Makefile
 create mode 100644 drivers/acpi/loongarch/init.c
 create mode 100644 drivers/acpi/loongarch/init.h
 create mode 100644 drivers/acpi/loongarch/iovt.c
 create mode 100644 include/linux/acpi_iovt.h


base-commit: 93f51579e7df248780214094418f205253383cc5
-- 
2.39.3


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure
  2026-09-22 12:10 [PATCH v4 0/4] ACPI: LoongArch: Add IOVT table support Bibo Mao
@ 2026-09-22 12:10 ` Bibo Mao
  2026-10-12  7:18   ` Bibo Mao
  2026-09-22 12:10 ` [PATCH v4 2/4] ACPI: LoongArch: Add acpi_arch_[late]_init() Bibo Mao
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 22+ messages in thread
From: Bibo Mao @ 2026-09-22 12:10 UTC (permalink / raw)
  To: Huacai Chen, Rafael J . Wysocki, Maciej Wieczor-Retman,
	Pawel Chmielewski
  Cc: Len Brown, linux-kernel, loongarch, linux-acpi, acpica-devel, lixianglai

According to the IOVT specification, define macros for each bit of the
Flags field in the IOVT IOMMU structure.

Link: https://github.com/open-acpica/acpica/commit/352f750e4cc0
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 include/acpi/actbl2.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index baef525367b5..76e4a5bc79b4 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -919,6 +919,13 @@ struct acpi_iovt_iommu {
 	u32 device_entry_offset;
 };
 
+/* Masks for Flags field above */
+#define ACPI_IOVT_IOMMU_PCI_DEVICE		(1<<0)
+#define ACPI_IOVT_IOMMU_PXM_VALID		(1<<1)
+#define ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT	(1<<2)
+#define ACPI_IOVT_IOMMU_HW_CAP_SUPPORT		(1<<3)
+#define ACPI_IOVT_IOMMU_MSI_INT_BYPASS		(1<<4)
+
 struct acpi_iovt_device_entry {
 	u8 type;
 	u8 length;
-- 
2.39.3


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v4 2/4] ACPI: LoongArch: Add acpi_arch_[late]_init()
  2026-09-22 12:10 [PATCH v4 0/4] ACPI: LoongArch: Add IOVT table support Bibo Mao
  2026-09-22 12:10 ` [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure Bibo Mao
@ 2026-09-22 12:10 ` Bibo Mao
  2026-09-22 12:10 ` [PATCH v4 3/4] ACPI: scan: Add support for LoongArch in acpi_iommu_configure_id() Bibo Mao
  2026-09-22 12:10 ` [PATCH v4 4/4] MAINTAINERS: Add entry for drivers/acpi/loongarch Bibo Mao
  3 siblings, 0 replies; 22+ messages in thread
From: Bibo Mao @ 2026-09-22 12:10 UTC (permalink / raw)
  To: Huacai Chen, Rafael J . Wysocki, Maciej Wieczor-Retman,
	Pawel Chmielewski
  Cc: Len Brown, linux-kernel, loongarch, linux-acpi, acpica-devel, lixianglai

LoongArch IOMMU scanning flow is similar with virtio-iommu,
there is early init and late_init. In early init function, PCI
ACS is enabled if there is IOMMU table, and late_init is called
after PCI device scanning, where IOVT table scanning function is
executed.

Here add acpi_arch_init() and acpi_arch_late_init() API on LoongArch,
function acpi_iovt_init() is to enable PCI ACS function if IOMMU device
exists in IOVT table, and function acpi_iovt_late_init() is to scan
IOVT table, add IOMMU devices.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 arch/loongarch/Kconfig          |   1 +
 drivers/acpi/Kconfig            |   4 +
 drivers/acpi/Makefile           |   2 +-
 drivers/acpi/bus.c              |   2 +
 drivers/acpi/loongarch/Kconfig  |   7 +
 drivers/acpi/loongarch/Makefile |   3 +
 drivers/acpi/loongarch/init.c   |  16 +++
 drivers/acpi/loongarch/init.h   |   5 +
 drivers/acpi/loongarch/iovt.c   | 219 ++++++++++++++++++++++++++++++++
 include/linux/acpi.h            |   1 +
 10 files changed, 259 insertions(+), 1 deletion(-)
 create mode 100644 drivers/acpi/loongarch/Kconfig
 create mode 100644 drivers/acpi/loongarch/Makefile
 create mode 100644 drivers/acpi/loongarch/init.c
 create mode 100644 drivers/acpi/loongarch/init.h
 create mode 100644 drivers/acpi/loongarch/iovt.c

diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index 2067d1f2ad7a..d7f8e4fec952 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -6,6 +6,7 @@ config LOONGARCH
 	select ACPI_GENERIC_GSI if ACPI
 	select ACPI_MCFG if ACPI
 	select ACPI_HOTPLUG_CPU if ACPI_PROCESSOR && HOTPLUG_CPU
+	select ACPI_IOVT if ACPI
 	select ACPI_PPTT if ACPI
 	select ACPI_SYSTEM_POWER_STATES_SUPPORT	if ACPI
 	select ARCH_BINFMT_ELF_STATE
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index f165d14cf61a..192e6f18eda1 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -550,6 +550,10 @@ if ARM64
 source "drivers/acpi/arm64/Kconfig"
 endif
 
+if LOONGARCH
+source "drivers/acpi/loongarch/Kconfig"
+endif
+
 if RISCV
 source "drivers/acpi/riscv/Kconfig"
 endif
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index d1b0affb844f..91584a3c1e01 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -131,6 +131,6 @@ obj-y				+= dptf/
 obj-$(CONFIG_ARM64)		+= arm64/
 
 obj-$(CONFIG_ACPI_VIOT)		+= viot.o
-
+obj-$(CONFIG_LOONGARCH)		+= loongarch/
 obj-$(CONFIG_RISCV)		+= riscv/
 obj-$(CONFIG_X86)		+= x86/
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 808c6746be14..33d752eacde3 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1465,6 +1465,7 @@ struct kobject *acpi_kobj;
 EXPORT_SYMBOL_GPL(acpi_kobj);
 
 void __weak __init acpi_arch_init(void) { }
+void __weak __init acpi_arch_late_init(void) { }
 
 static int __init acpi_init(void)
 {
@@ -1503,6 +1504,7 @@ static int __init acpi_init(void)
 	acpi_wakeup_device_init();
 	acpi_debugger_init();
 	acpi_setup_sb_notify_handler();
+	acpi_arch_late_init();
 	acpi_viot_init();
 	return 0;
 }
diff --git a/drivers/acpi/loongarch/Kconfig b/drivers/acpi/loongarch/Kconfig
new file mode 100644
index 000000000000..91ba3c35b9bf
--- /dev/null
+++ b/drivers/acpi/loongarch/Kconfig
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# ACPI Configuration for LOONGARCH
+#
+
+config ACPI_IOVT
+	bool
diff --git a/drivers/acpi/loongarch/Makefile b/drivers/acpi/loongarch/Makefile
new file mode 100644
index 000000000000..c338d3102fc1
--- /dev/null
+++ b/drivers/acpi/loongarch/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0-only
+obj-y					+= init.o
+obj-$(CONFIG_ACPI_IOVT)			+= iovt.o
diff --git a/drivers/acpi/loongarch/init.c b/drivers/acpi/loongarch/init.c
new file mode 100644
index 000000000000..e7272ee197c7
--- /dev/null
+++ b/drivers/acpi/loongarch/init.c
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/acpi.h>
+#include "init.h"
+
+void __init acpi_arch_init(void)
+{
+	if (IS_ENABLED(CONFIG_ACPI_IOVT))
+		acpi_iovt_init();
+}
+
+void __init acpi_arch_late_init(void)
+{
+	if (IS_ENABLED(CONFIG_ACPI_IOVT))
+		acpi_iovt_late_init();
+}
diff --git a/drivers/acpi/loongarch/init.h b/drivers/acpi/loongarch/init.h
new file mode 100644
index 000000000000..85e5197553ad
--- /dev/null
+++ b/drivers/acpi/loongarch/init.h
@@ -0,0 +1,5 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#include <linux/init.h>
+
+void __init acpi_iovt_init(void);
+void __init acpi_iovt_late_init(void);
diff --git a/drivers/acpi/loongarch/iovt.c b/drivers/acpi/loongarch/iovt.c
new file mode 100644
index 000000000000..302ca6a75905
--- /dev/null
+++ b/drivers/acpi/loongarch/iovt.c
@@ -0,0 +1,219 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/acpi.h>
+#include <linux/pci.h>
+#include "init.h"
+
+struct iovt_device_entry {
+	struct list_head list;
+	int start_devid;
+	int end_devid;
+};
+
+struct iovt_fwnode {
+	struct list_head list;
+	struct fwnode_handle *fwnode;
+	int flag;
+	int segment;
+	int devid;
+	int nid;
+	struct list_head ep_list;
+};
+
+/* Root pointer to the mapped IOVT table */
+static LIST_HEAD(iovt_fwnode_list);
+static DEFINE_SPINLOCK(iovt_fwnode_lock);
+
+#ifdef CONFIG_PCI
+static void __init iovt_enable_acs(struct acpi_iovt_iommu *iommu)
+{
+	static bool acs_enabled __initdata;
+
+	if (acs_enabled)
+		return;
+
+	/* IOMMU V1 only supports PCI device management */
+	if ((iommu->header.type == ACPI_IOVT_IOMMU_V1) ||
+	    (iommu->flags & (ACPI_IOVT_IOMMU_PCI_DEVICE | ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT))) {
+		pci_request_acs();
+		acs_enabled = true;
+	}
+}
+#else
+static inline void iovt_enable_acs(struct acpi_iovt_iommu *iommu) { }
+#endif
+
+static int __init iovt_get_pci_iommu_fwnode(struct iovt_fwnode *np, u16 segment, u16 bdf)
+{
+	struct pci_dev *pdev;
+	struct fwnode_handle *fwnode;
+
+	pdev = pci_get_domain_bus_and_slot(segment, PCI_BUS_NUM(bdf), bdf & 0xff);
+	if (!pdev) {
+		pr_err("No PCI IOMMU found for segment 0x%x bdf 0x%x\n", segment, bdf);
+		return -ENODEV;
+	}
+
+	fwnode = dev_fwnode(&pdev->dev);
+	if (!fwnode) {
+		/*
+		 * PCI devices aren't necessarily described by ACPI. Create a
+		 * fwnode so the IOMMU subsystem can identify this device.
+		 */
+		fwnode = acpi_alloc_fwnode_static();
+		if (!fwnode) {
+			pci_dev_put(pdev);
+			return -ENOMEM;
+		}
+		set_primary_fwnode(&pdev->dev, fwnode);
+	}
+
+	np->fwnode = dev_fwnode(&pdev->dev);
+	if (np->flag & ACPI_IOVT_IOMMU_PXM_VALID)
+		set_dev_node(&pdev->dev, np->nid);
+	pci_dev_put(pdev);
+	return 0;
+}
+
+static int __init iovt_add_iommu(struct acpi_iovt_iommu *iommu)
+{
+	struct iovt_fwnode *np;
+	struct fwnode_handle *fwnode;
+	struct acpi_iovt_device_entry *ep;
+	struct iovt_device_entry *entry;
+	int i, ret, start_devid;
+	bool is_start = false;
+
+	np = kzalloc_obj(struct iovt_fwnode, GFP_ATOMIC);
+	if (WARN_ON(!np))
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&np->list);
+	np->flag = iommu->flags;
+	np->segment = iommu->segment;
+	if (np->flag & ACPI_IOVT_IOMMU_PXM_VALID)
+		np->nid = pxm_to_node(iommu->proximity_domain);
+
+	if (np->flag & ACPI_IOVT_IOMMU_PCI_DEVICE) {
+		np->devid = iommu->device_id;
+		ret = iovt_get_pci_iommu_fwnode(np, np->segment, np->devid);
+		if (ret) {
+			kfree(np);
+			return ret;
+		}
+
+	} else {
+		fwnode = acpi_alloc_fwnode_static();
+		if (!fwnode) {
+			kfree(np);
+			return -ENOMEM;
+		}
+
+		np->fwnode = fwnode;
+	}
+
+	/* All devices in the segment are managed by this IOMMU */
+	if (np->flag & ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT)
+		goto skip;
+
+	INIT_LIST_HEAD(&np->ep_list);
+	ep = ACPI_ADD_PTR(struct acpi_iovt_device_entry, iommu, iommu->device_entry_offset);
+	for (i = 0; i < iommu->device_entry_num; i++) {
+		switch (ep->type) {
+		case ACPI_IOVT_DEVICE_ENTRY_START:
+			is_start = true;
+			start_devid = ep->device_id;
+			break;
+		case ACPI_IOVT_DEVICE_ENTRY_END:
+			if (!is_start)
+				break;
+
+			entry = kzalloc_obj(struct iovt_device_entry, GFP_ATOMIC);
+			if (!entry)
+				return -ENOMEM;
+
+			entry->start_devid = start_devid;
+			entry->end_devid = ep->device_id;
+			list_add_tail(&entry->list, &np->ep_list);
+			is_start = false;
+			break;
+		case ACPI_IOVT_DEVICE_ENTRY_SINGLE:
+			entry = kzalloc_obj(struct iovt_device_entry, GFP_ATOMIC);
+			if (!entry)
+				return -ENOMEM;
+
+			entry->start_devid = ep->device_id;
+			entry->end_devid = ep->device_id;
+			list_add_tail(&entry->list, &np->ep_list);
+			is_start = false;
+			break;
+		default:
+			break;
+		}
+		ep = ACPI_ADD_PTR(struct acpi_iovt_device_entry, ep, ep->length);
+	}
+
+skip:
+	spin_lock(&iovt_fwnode_lock);
+	list_add_tail(&np->list, &iovt_fwnode_list);
+	spin_unlock(&iovt_fwnode_lock);
+	return 0;
+}
+
+static void __init iovt_init_devices(struct acpi_table_header *header)
+{
+	struct acpi_iovt_iommu *iommu;
+	struct acpi_table_iovt *iovt;
+	int i;
+
+	/* Get the first IOVT node */
+	iovt = (struct acpi_table_iovt *)header;
+	iommu = ACPI_ADD_PTR(struct acpi_iovt_iommu, iovt, iovt->iommu_offset);
+	for (i = 0; i < iovt->iommu_count; i++) {
+		iovt_add_iommu(iommu);
+		iommu = ACPI_ADD_PTR(struct acpi_iovt_iommu, iommu, iommu->header.length);
+	}
+}
+
+void __init acpi_iovt_init(void)
+{
+	acpi_status status;
+	struct acpi_table_header *hdr;
+	struct acpi_table_iovt *iovt;
+	struct acpi_iovt_iommu *iommu;
+	int i;
+
+	status = acpi_get_table(ACPI_SIG_IOVT, 0, &hdr);
+	if (ACPI_FAILURE(status)) {
+		if (status != AE_NOT_FOUND)
+			pr_err("Failed to get table, %s\n", acpi_format_exception(status));
+
+		return;
+	}
+
+	iovt = (struct acpi_table_iovt *)&hdr;
+	iommu = ACPI_ADD_PTR(struct acpi_iovt_iommu, iovt, iovt->iommu_offset);
+	for (i = 0; i < iovt->iommu_count; i++) {
+		iovt_enable_acs(iommu);
+		iommu = ACPI_ADD_PTR(struct acpi_iovt_iommu, iommu, iommu->header.length);
+	}
+
+	acpi_put_table(hdr);
+}
+
+void __init acpi_iovt_late_init(void)
+{
+	acpi_status status;
+	struct acpi_table_header *hdr;
+
+	status = acpi_get_table(ACPI_SIG_IOVT, 0, &hdr);
+	if (ACPI_FAILURE(status)) {
+		if (status != AE_NOT_FOUND)
+			pr_err("Failed to get table, %s\n", acpi_format_exception(status));
+
+		return;
+	}
+
+	iovt_init_devices(hdr);
+	acpi_put_table(hdr);
+}
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index ddacac812094..2a621c095b8b 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1642,6 +1642,7 @@ static inline int acpi_pptt_get_cpumask_from_cache_id(u32 cache_id,
 #endif
 
 void acpi_arch_init(void);
+void acpi_arch_late_init(void);
 
 #ifdef CONFIG_ACPI_PCC
 void acpi_init_pcc(void);
-- 
2.39.3


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v4 3/4] ACPI: scan: Add support for LoongArch in acpi_iommu_configure_id()
  2026-09-22 12:10 [PATCH v4 0/4] ACPI: LoongArch: Add IOVT table support Bibo Mao
  2026-09-22 12:10 ` [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure Bibo Mao
  2026-09-22 12:10 ` [PATCH v4 2/4] ACPI: LoongArch: Add acpi_arch_[late]_init() Bibo Mao
@ 2026-09-22 12:10 ` Bibo Mao
  2026-09-22 12:10 ` [PATCH v4 4/4] MAINTAINERS: Add entry for drivers/acpi/loongarch Bibo Mao
  3 siblings, 0 replies; 22+ messages in thread
From: Bibo Mao @ 2026-09-22 12:10 UTC (permalink / raw)
  To: Huacai Chen, Rafael J . Wysocki, Maciej Wieczor-Retman,
	Pawel Chmielewski
  Cc: Len Brown, linux-kernel, loongarch, linux-acpi, acpica-devel, lixianglai

acpi_iommu_configure_id() currently supports only IORT and VIOT. Add
support for IOVT as well.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
Acked-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>
---
 drivers/acpi/loongarch/iovt.c | 61 +++++++++++++++++++++++++++++++++++
 drivers/acpi/scan.c           |  3 ++
 include/linux/acpi_iovt.h     | 13 ++++++++
 3 files changed, 77 insertions(+)
 create mode 100644 include/linux/acpi_iovt.h

diff --git a/drivers/acpi/loongarch/iovt.c b/drivers/acpi/loongarch/iovt.c
index 302ca6a75905..df14ce48eac6 100644
--- a/drivers/acpi/loongarch/iovt.c
+++ b/drivers/acpi/loongarch/iovt.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 
 #include <linux/acpi.h>
+#include <linux/acpi_iovt.h>
 #include <linux/pci.h>
 #include "init.h"
 
@@ -217,3 +218,63 @@ void __init acpi_iovt_late_init(void)
 	iovt_init_devices(hdr);
 	acpi_put_table(hdr);
 }
+
+static int iovt_pci_dev_iommu_init(struct pci_dev *pdev, u16 dev_id, void *data)
+{
+	u32 sbdf;
+	struct iovt_fwnode *np;
+	struct device *aliased_dev = data;
+	u32 domain = pci_domain_nr(pdev->bus);
+	struct iovt_device_entry *entry;
+	bool found = false;
+
+	list_for_each_entry(np, &iovt_fwnode_list, list) {
+		if (domain != np->segment)
+			continue;
+
+		/* We're not translating ourself */
+		if (dev_id == np->devid)
+			return -EINVAL;
+
+		if (np->flag & ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT) {
+			found = true;
+			break;
+		}
+
+		list_for_each_entry(entry, &np->ep_list, list) {
+			if (dev_id < entry->start_devid)
+				continue;
+
+			if (dev_id > entry->end_devid)
+				continue;
+
+			found = true;
+			break;
+		}
+
+		if (found)
+			break;
+	}
+
+	if (!found)
+		return -ENODEV;
+
+	sbdf = (domain << 16) + dev_id;
+	return acpi_iommu_fwspec_init(aliased_dev, sbdf, np->fwnode);
+}
+
+/*
+ * iovt_iommu_configure_id - Set-up IOMMU configuration for a device.
+ *
+ * @dev: device to configure
+ * @id_in: optional input id const value pointer
+ *
+ * Returns: 0 on success, <0 on failure
+ */
+int iovt_iommu_configure_id(struct device *dev, const u32 *id_in)
+{
+	if (!dev_is_pci(dev))
+		return -ENODEV;
+
+	return pci_for_each_dma_alias(to_pci_dev(dev), iovt_pci_dev_iommu_init, dev);
+}
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 163a3cccf197..827cd526a099 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -13,6 +13,7 @@
 #include <linux/kernel.h>
 #include <linux/acpi.h>
 #include <linux/acpi_iort.h>
+#include <linux/acpi_iovt.h>
 #include <linux/acpi_rimt.h>
 #include <linux/acpi_viot.h>
 #include <linux/iommu.h>
@@ -1626,6 +1627,8 @@ static int acpi_iommu_configure_id(struct device *dev, const u32 *id_in)
 	err = iort_iommu_configure_id(dev, id_in);
 	if (err && err != -EPROBE_DEFER)
 		err = rimt_iommu_configure_id(dev, id_in);
+	if (err && err != -EPROBE_DEFER)
+		err = iovt_iommu_configure_id(dev, id_in);
 	if (err && err != -EPROBE_DEFER)
 		err = viot_iommu_configure(dev);
 
diff --git a/include/linux/acpi_iovt.h b/include/linux/acpi_iovt.h
new file mode 100644
index 000000000000..25a846b96260
--- /dev/null
+++ b/include/linux/acpi_iovt.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __ACPI_IOVT_H__
+#define __ACPI_IOVT_H__
+
+#if defined(CONFIG_IOMMU_API) && defined(CONFIG_ACPI_IOVT)
+int iovt_iommu_configure_id(struct device *dev, const u32 *id_in);
+#else
+static inline int iovt_iommu_configure_id(struct device *dev, const u32 *id_in)
+{ return -ENODEV; }
+#endif
+
+#endif
-- 
2.39.3


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v4 4/4] MAINTAINERS: Add entry for drivers/acpi/loongarch
  2026-09-22 12:10 [PATCH v4 0/4] ACPI: LoongArch: Add IOVT table support Bibo Mao
                   ` (2 preceding siblings ...)
  2026-09-22 12:10 ` [PATCH v4 3/4] ACPI: scan: Add support for LoongArch in acpi_iommu_configure_id() Bibo Mao
@ 2026-09-22 12:10 ` Bibo Mao
  3 siblings, 0 replies; 22+ messages in thread
From: Bibo Mao @ 2026-09-22 12:10 UTC (permalink / raw)
  To: Huacai Chen, Rafael J . Wysocki, Maciej Wieczor-Retman,
	Pawel Chmielewski
  Cc: Len Brown, linux-kernel, loongarch, linux-acpi, acpica-devel, lixianglai

ACPI defines few LoongArch specific tables which need parsing code
added in drivers/acpi/loongarch. Add maintainer entries for this newly
created folder.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index cc3cae2e378b..f9ae86a42dec 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -347,6 +347,15 @@ S:	Maintained
 F:	drivers/acpi/riscv/
 F:	include/linux/acpi_rimt.h
 
+ACPI FOR LOONGARCH (ACPI/LoongArch)
+M:	Bibo Mao <maobibo@loongson.cn>
+M:	Huacai Chen <chenhuacai@kernel.org>
+L:	linux-acpi@vger.kernel.org
+L:	loongarch@lists.linux.dev
+S:	Maintained
+F:	drivers/acpi/loongarch/
+F:	include/linux/acpi_iovt.h
+
 ACPI PCC(Platform Communication Channel) MAILBOX DRIVER
 M:	Sudeep Holla <sudeep.holla@kernel.org>
 L:	linux-acpi@vger.kernel.org
-- 
2.39.3


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure
  2026-09-22 12:10 ` [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure Bibo Mao
@ 2026-10-12  7:18   ` Bibo Mao
  2026-10-12  7:20     ` Huacai Chen
  2026-10-23  7:59     ` Huacai Chen
  0 siblings, 2 replies; 22+ messages in thread
From: Bibo Mao @ 2026-10-12  7:18 UTC (permalink / raw)
  To: rafael, linux-acpi
  Cc: acpica-devel, chenhuacai, lenb, linux-kernel, lixianglai,
	loongarch, maciej.wieczor-retman, pawel.chmielewski, maobibo

This change is already merged in acpica tree.

slightly ping.

On Tue, Sep 22, 2026 at 08:10:55PM +0800, Bibo Mao wrote:
 > According to the IOVT specification, define macros for each bit of the
 > Flags field in the IOVT IOMMU structure.
 >
 > Link: https://github.com/open-acpica/acpica/commit/352f750e4cc0
 > Signed-off-by: Bibo Mao <maobibo@loongson.cn>
 > ---
 >  include/acpi/actbl2.h | 7 +++++++
 >  1 file changed, 7 insertions(+)
 >
 > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
 > index baef525367b5..76e4a5bc79b4 100644
 > --- a/include/acpi/actbl2.h
 > +++ b/include/acpi/actbl2.h
 > @@ -919,6 +919,13 @@ struct acpi_iovt_iommu {
 >       u32 device_entry_offset;
 >  };
 >
 > +/* Masks for Flags field above */
 > +#define ACPI_IOVT_IOMMU_PCI_DEVICE           (1<<0)
 > +#define ACPI_IOVT_IOMMU_PXM_VALID            (1<<1)
 > +#define ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT    (1<<2)
 > +#define ACPI_IOVT_IOMMU_HW_CAP_SUPPORT               (1<<3)
 > +#define ACPI_IOVT_IOMMU_MSI_INT_BYPASS               (1<<4)
 > +
 >  struct acpi_iovt_device_entry {
 >       u8 type;
 >       u8 length;
 > --
 > 2.39.3
 >


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure
  2026-10-12  7:18   ` Bibo Mao
@ 2026-10-12  7:20     ` Huacai Chen
  2026-10-12 14:47       ` Rafael J. Wysocki (Intel)
  2026-10-23  7:59     ` Huacai Chen
  1 sibling, 1 reply; 22+ messages in thread
From: Huacai Chen @ 2026-10-12  7:20 UTC (permalink / raw)
  To: Bibo Mao
  Cc: rafael, linux-acpi, acpica-devel, lenb, linux-kernel, lixianglai,
	loongarch, maciej.wieczor-retman, pawel.chmielewski

On Mon, Oct 12, 2026 at 3:16 PM Bibo Mao <maobibo@loongson.cn> wrote:
>
> This change is already merged in acpica tree.
>
> slightly ping.
I will merge the last three patches once the acpica changes are synced
to git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git.

Huacai

>
> On Tue, Sep 22, 2026 at 08:10:55PM +0800, Bibo Mao wrote:
>  > According to the IOVT specification, define macros for each bit of the
>  > Flags field in the IOVT IOMMU structure.
>  >
>  > Link: https://github.com/open-acpica/acpica/commit/352f750e4cc0
>  > Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>  > ---
>  >  include/acpi/actbl2.h | 7 +++++++
>  >  1 file changed, 7 insertions(+)
>  >
>  > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
>  > index baef525367b5..76e4a5bc79b4 100644
>  > --- a/include/acpi/actbl2.h
>  > +++ b/include/acpi/actbl2.h
>  > @@ -919,6 +919,13 @@ struct acpi_iovt_iommu {
>  >       u32 device_entry_offset;
>  >  };
>  >
>  > +/* Masks for Flags field above */
>  > +#define ACPI_IOVT_IOMMU_PCI_DEVICE           (1<<0)
>  > +#define ACPI_IOVT_IOMMU_PXM_VALID            (1<<1)
>  > +#define ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT    (1<<2)
>  > +#define ACPI_IOVT_IOMMU_HW_CAP_SUPPORT               (1<<3)
>  > +#define ACPI_IOVT_IOMMU_MSI_INT_BYPASS               (1<<4)
>  > +
>  >  struct acpi_iovt_device_entry {
>  >       u8 type;
>  >       u8 length;
>  > --
>  > 2.39.3
>  >
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure
  2026-10-12  7:20     ` Huacai Chen
@ 2026-10-12 14:47       ` Rafael J. Wysocki (Intel)
  2026-10-13  8:39         ` Huacai Chen
  0 siblings, 1 reply; 22+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-10-12 14:47 UTC (permalink / raw)
  To: Huacai Chen, Bibo Mao
  Cc: linux-acpi, acpica-devel, lenb, linux-kernel, lixianglai,
	loongarch, maciej.wieczor-retman, pawel.chmielewski

On Mon, Oct 12, 2026 at 9:21 AM Huacai Chen <chenhuacai@kernel.org> wrote:
>
> On Mon, Oct 12, 2026 at 3:16 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >
> > This change is already merged in acpica tree.
> >
> > slightly ping.
> I will merge the last three patches once the acpica changes are synced
> to git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git.

I've applied the $subject patch as 7.4 material.

When it propagates to linux-next, I can expose it on an immutable
branch if that helps, in which case please let me know.

Thanks!

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure
  2026-10-12 14:47       ` Rafael J. Wysocki (Intel)
@ 2026-10-13  8:39         ` Huacai Chen
  2026-10-13 12:00           ` Rafael J. Wysocki (Intel)
  0 siblings, 1 reply; 22+ messages in thread
From: Huacai Chen @ 2026-10-13  8:39 UTC (permalink / raw)
  To: Rafael J. Wysocki (Intel)
  Cc: Bibo Mao, linux-acpi, acpica-devel, lenb, linux-kernel,
	lixianglai, loongarch, maciej.wieczor-retman, pawel.chmielewski

On Mon, Oct 12, 2026 at 10:49 PM Rafael J. Wysocki (Intel)
<rafael@kernel.org> wrote:
>
> On Mon, Oct 12, 2026 at 9:21 AM Huacai Chen <chenhuacai@kernel.org> wrote:
> >
> > On Mon, Oct 12, 2026 at 3:16 PM Bibo Mao <maobibo@loongson.cn> wrote:
> > >
> > > This change is already merged in acpica tree.
> > >
> > > slightly ping.
> > I will merge the last three patches once the acpica changes are synced
> > to git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git.
>
> I've applied the $subject patch as 7.4 material.
>
> When it propagates to linux-next, I can expose it on an immutable
> branch if that helps, in which case please let me know.
If that commit is contained by the tag such as 'acpi-7.4-rc1', then it
is enough for me. The loongarch pull request is usually after the acpi
pull request.

Huacai

>
> Thanks!

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure
  2026-10-13  8:39         ` Huacai Chen
@ 2026-10-13 12:00           ` Rafael J. Wysocki (Intel)
  0 siblings, 0 replies; 22+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-10-13 12:00 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Rafael J. Wysocki (Intel),
	Bibo Mao, linux-acpi, acpica-devel, lenb, linux-kernel,
	lixianglai, loongarch, maciej.wieczor-retman, pawel.chmielewski

On Tue, Oct 13, 2026 at 10:40 AM Huacai Chen <chenhuacai@kernel.org> wrote:
>
> On Mon, Oct 12, 2026 at 10:49 PM Rafael J. Wysocki (Intel)
> <rafael@kernel.org> wrote:
> >
> > On Mon, Oct 12, 2026 at 9:21 AM Huacai Chen <chenhuacai@kernel.org> wrote:
> > >
> > > On Mon, Oct 12, 2026 at 3:16 PM Bibo Mao <maobibo@loongson.cn> wrote:
> > > >
> > > > This change is already merged in acpica tree.
> > > >
> > > > slightly ping.
> > > I will merge the last three patches once the acpica changes are synced
> > > to git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git.
> >
> > I've applied the $subject patch as 7.4 material.
> >
> > When it propagates to linux-next, I can expose it on an immutable
> > branch if that helps, in which case please let me know.
> If that commit is contained by the tag such as 'acpi-7.4-rc1', then it
> is enough for me. The loongarch pull request is usually after the acpi
> pull request.

OK

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure
  2026-10-12  7:18   ` Bibo Mao
  2026-10-12  7:20     ` Huacai Chen
@ 2026-10-23  7:59     ` Huacai Chen
  2026-11-04  8:30       ` Bibo Mao
  2026-11-05  9:15       ` Bibo Mao
  1 sibling, 2 replies; 22+ messages in thread
From: Huacai Chen @ 2026-10-23  7:59 UTC (permalink / raw)
  To: Bibo Mao
  Cc: rafael, linux-acpi, acpica-devel, lenb, linux-kernel, lixianglai,
	loongarch, maciej.wieczor-retman, pawel.chmielewski

Bibo,

Now I have applied this series but with some modifications, please
double check and test the code.

https://git.kernel.org/pub/scm/linux/kernel/git/chenhuacai/linux-loongson.git/log/?h=loongarch-next

Huacai

On Mon, Oct 12, 2026 at 3:16 PM Bibo Mao <maobibo@loongson.cn> wrote:
>
> This change is already merged in acpica tree.
>
> slightly ping.
>
> On Tue, Sep 22, 2026 at 08:10:55PM +0800, Bibo Mao wrote:
>  > According to the IOVT specification, define macros for each bit of the
>  > Flags field in the IOVT IOMMU structure.
>  >
>  > Link: https://github.com/open-acpica/acpica/commit/352f750e4cc0
>  > Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>  > ---
>  >  include/acpi/actbl2.h | 7 +++++++
>  >  1 file changed, 7 insertions(+)
>  >
>  > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
>  > index baef525367b5..76e4a5bc79b4 100644
>  > --- a/include/acpi/actbl2.h
>  > +++ b/include/acpi/actbl2.h
>  > @@ -919,6 +919,13 @@ struct acpi_iovt_iommu {
>  >       u32 device_entry_offset;
>  >  };
>  >
>  > +/* Masks for Flags field above */
>  > +#define ACPI_IOVT_IOMMU_PCI_DEVICE           (1<<0)
>  > +#define ACPI_IOVT_IOMMU_PXM_VALID            (1<<1)
>  > +#define ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT    (1<<2)
>  > +#define ACPI_IOVT_IOMMU_HW_CAP_SUPPORT               (1<<3)
>  > +#define ACPI_IOVT_IOMMU_MSI_INT_BYPASS               (1<<4)
>  > +
>  >  struct acpi_iovt_device_entry {
>  >       u8 type;
>  >       u8 length;
>  > --
>  > 2.39.3
>  >
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure
  2026-10-23  7:59     ` Huacai Chen
@ 2026-11-04  8:30       ` Bibo Mao
  2026-11-05  9:15       ` Bibo Mao
  1 sibling, 0 replies; 22+ messages in thread
From: Bibo Mao @ 2026-11-04  8:30 UTC (permalink / raw)
  To: Huacai Chen
  Cc: rafael, linux-acpi, acpica-devel, lenb, linux-kernel, lixianglai,
	loongarch, maciej.wieczor-retman, pawel.chmielewski



On 2026/10/23 下午3:59, Huacai Chen wrote:
> Bibo,
> 
> Now I have applied this series but with some modifications, please
> double check and test the code.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/chenhuacai/linux-loongson.git/log/?h=loongarch-next

Thanks, it works well on 7.4.0-rc1 version.

Regards
Bibo Mao
> 
> Huacai
> 
> On Mon, Oct 12, 2026 at 3:16 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>> This change is already merged in acpica tree.
>>
>> slightly ping.
>>
>> On Tue, Sep 22, 2026 at 08:10:55PM +0800, Bibo Mao wrote:
>>   > According to the IOVT specification, define macros for each bit of the
>>   > Flags field in the IOVT IOMMU structure.
>>   >
>>   > Link: https://github.com/open-acpica/acpica/commit/352f750e4cc0
>>   > Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>   > ---
>>   >  include/acpi/actbl2.h | 7 +++++++
>>   >  1 file changed, 7 insertions(+)
>>   >
>>   > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
>>   > index baef525367b5..76e4a5bc79b4 100644
>>   > --- a/include/acpi/actbl2.h
>>   > +++ b/include/acpi/actbl2.h
>>   > @@ -919,6 +919,13 @@ struct acpi_iovt_iommu {
>>   >       u32 device_entry_offset;
>>   >  };
>>   >
>>   > +/* Masks for Flags field above */
>>   > +#define ACPI_IOVT_IOMMU_PCI_DEVICE           (1<<0)
>>   > +#define ACPI_IOVT_IOMMU_PXM_VALID            (1<<1)
>>   > +#define ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT    (1<<2)
>>   > +#define ACPI_IOVT_IOMMU_HW_CAP_SUPPORT               (1<<3)
>>   > +#define ACPI_IOVT_IOMMU_MSI_INT_BYPASS               (1<<4)
>>   > +
>>   >  struct acpi_iovt_device_entry {
>>   >       u8 type;
>>   >       u8 length;
>>   > --
>>   > 2.39.3
>>   >
>>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure
  2026-10-23  7:59     ` Huacai Chen
  2026-11-04  8:30       ` Bibo Mao
@ 2026-11-05  9:15       ` Bibo Mao
  2026-11-05  9:31         ` Huacai Chen
  1 sibling, 1 reply; 22+ messages in thread
From: Bibo Mao @ 2026-11-05  9:15 UTC (permalink / raw)
  To: Huacai Chen
  Cc: rafael, linux-acpi, acpica-devel, lenb, linux-kernel, lixianglai,
	loongarch, maciej.wieczor-retman, pawel.chmielewski



On 2026/10/23 下午3:59, Huacai Chen wrote:
> Bibo,
> 
> Now I have applied this series but with some modifications, please
> double check and test the code.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/chenhuacai/linux-loongson.git/log/?h=loongarch-next
In my v4 submit, it is:
+               case ACPI_IOVT_DEVICE_ENTRY_START:
+                       is_start = true;
+                       start_devid = ep->device_id;
+                       break;
however in rc1, it is modified as:
                 case ACPI_IOVT_DEVICE_ENTRY_START:
                         is_start = true;
                         entry->start_devid = ep->device_id;
                         break;
Here entry is not allocated and NULL pointer, it is allocated in 
ACPI_IOVT_DEVICE_ENTRY_END switch case.

I really do not know what is aid to modify it and let others to test.

> 
> Huacai
> 
> On Mon, Oct 12, 2026 at 3:16 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>> This change is already merged in acpica tree.
>>
>> slightly ping.
>>
>> On Tue, Sep 22, 2026 at 08:10:55PM +0800, Bibo Mao wrote:
>>   > According to the IOVT specification, define macros for each bit of the
>>   > Flags field in the IOVT IOMMU structure.
>>   >
>>   > Link: https://github.com/open-acpica/acpica/commit/352f750e4cc0
>>   > Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>   > ---
>>   >  include/acpi/actbl2.h | 7 +++++++
>>   >  1 file changed, 7 insertions(+)
>>   >
>>   > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
>>   > index baef525367b5..76e4a5bc79b4 100644
>>   > --- a/include/acpi/actbl2.h
>>   > +++ b/include/acpi/actbl2.h
>>   > @@ -919,6 +919,13 @@ struct acpi_iovt_iommu {
>>   >       u32 device_entry_offset;
>>   >  };
>>   >
>>   > +/* Masks for Flags field above */
>>   > +#define ACPI_IOVT_IOMMU_PCI_DEVICE           (1<<0)
>>   > +#define ACPI_IOVT_IOMMU_PXM_VALID            (1<<1)
>>   > +#define ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT    (1<<2)
>>   > +#define ACPI_IOVT_IOMMU_HW_CAP_SUPPORT               (1<<3)
>>   > +#define ACPI_IOVT_IOMMU_MSI_INT_BYPASS               (1<<4)
>>   > +
>>   >  struct acpi_iovt_device_entry {
>>   >       u8 type;
>>   >       u8 length;
>>   > --
>>   > 2.39.3
>>   >
>>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure
  2026-11-05  9:15       ` Bibo Mao
@ 2026-11-05  9:31         ` Huacai Chen
  2026-11-05  9:50           ` Bibo Mao
  0 siblings, 1 reply; 22+ messages in thread
From: Huacai Chen @ 2026-11-05  9:31 UTC (permalink / raw)
  To: Bibo Mao
  Cc: rafael, linux-acpi, acpica-devel, lenb, linux-kernel, lixianglai,
	loongarch, maciej.wieczor-retman, pawel.chmielewski

On Thu, Nov 5, 2026 at 5:14 PM Bibo Mao <maobibo@loongson.cn> wrote:
>
>
>
> On 2026/10/23 下午3:59, Huacai Chen wrote:
> > Bibo,
> >
> > Now I have applied this series but with some modifications, please
> > double check and test the code.
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/chenhuacai/linux-loongson.git/log/?h=loongarch-next
> In my v4 submit, it is:
> +               case ACPI_IOVT_DEVICE_ENTRY_START:
> +                       is_start = true;
> +                       start_devid = ep->device_id;
> +                       break;
> however in rc1, it is modified as:
>                  case ACPI_IOVT_DEVICE_ENTRY_START:
>                          is_start = true;
>                          entry->start_devid = ep->device_id;
>                          break;
> Here entry is not allocated and NULL pointer, it is allocated in
> ACPI_IOVT_DEVICE_ENTRY_END switch case.
>
> I really do not know what is aid to modify it and let others to test.
Sorry, it's my fault.

In theory I should point out the modification (eliminating the local
variable start_devid) and refresh a new version. But we have no time
to do this because when the first patch is merged into the ACPI repo
it is near the merge window.

If you agree, I will submit such modifications to fix it.

diff --git a/drivers/acpi/loongarch/iovt.c b/drivers/acpi/loongarch/iovt.c
index 87cee4f0fc80..4cdac28a1fd0 100644
--- a/drivers/acpi/loongarch/iovt.c
+++ b/drivers/acpi/loongarch/iovt.c
@@ -115,6 +115,10 @@ static int __init iovt_add_iommu(struct
acpi_iovt_iommu *iommu)
        for (i = 0; i < iommu->device_entry_num; i++) {
                switch (ep->type) {
                case ACPI_IOVT_DEVICE_ENTRY_START:
+                       entry = kzalloc_obj(struct iovt_device_entry,
GFP_ATOMIC);
+                       if (!entry)
+                               return -ENOMEM;
+
                        is_start = true;
                        entry->start_devid = ep->device_id;
                        break;
@@ -122,10 +126,6 @@ static int __init iovt_add_iommu(struct
acpi_iovt_iommu *iommu)
                        if (!is_start)
                                break;

-                       entry = kzalloc_obj(struct iovt_device_entry,
GFP_ATOMIC);
-                       if (!entry)
-                               return -ENOMEM;
-
                        is_start = false;
                        entry->end_devid = ep->device_id;
                        list_add_tail(&entry->list, &np->ep_list);

Huacai

>
> >
> > Huacai
> >
> > On Mon, Oct 12, 2026 at 3:16 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>
> >> This change is already merged in acpica tree.
> >>
> >> slightly ping.
> >>
> >> On Tue, Sep 22, 2026 at 08:10:55PM +0800, Bibo Mao wrote:
> >>   > According to the IOVT specification, define macros for each bit of the
> >>   > Flags field in the IOVT IOMMU structure.
> >>   >
> >>   > Link: https://github.com/open-acpica/acpica/commit/352f750e4cc0
> >>   > Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >>   > ---
> >>   >  include/acpi/actbl2.h | 7 +++++++
> >>   >  1 file changed, 7 insertions(+)
> >>   >
> >>   > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> >>   > index baef525367b5..76e4a5bc79b4 100644
> >>   > --- a/include/acpi/actbl2.h
> >>   > +++ b/include/acpi/actbl2.h
> >>   > @@ -919,6 +919,13 @@ struct acpi_iovt_iommu {
> >>   >       u32 device_entry_offset;
> >>   >  };
> >>   >
> >>   > +/* Masks for Flags field above */
> >>   > +#define ACPI_IOVT_IOMMU_PCI_DEVICE           (1<<0)
> >>   > +#define ACPI_IOVT_IOMMU_PXM_VALID            (1<<1)
> >>   > +#define ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT    (1<<2)
> >>   > +#define ACPI_IOVT_IOMMU_HW_CAP_SUPPORT               (1<<3)
> >>   > +#define ACPI_IOVT_IOMMU_MSI_INT_BYPASS               (1<<4)
> >>   > +
> >>   >  struct acpi_iovt_device_entry {
> >>   >       u8 type;
> >>   >       u8 length;
> >>   > --
> >>   > 2.39.3
> >>   >
> >>
>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure
  2026-11-05  9:31         ` Huacai Chen
@ 2026-11-05  9:50           ` Bibo Mao
  2026-11-05 10:31             ` Huacai Chen
  0 siblings, 1 reply; 22+ messages in thread
From: Bibo Mao @ 2026-11-05  9:50 UTC (permalink / raw)
  To: Huacai Chen
  Cc: rafael, linux-acpi, acpica-devel, lenb, linux-kernel, lixianglai,
	loongarch, maciej.wieczor-retman, pawel.chmielewski



On 2026/11/5 下午5:31, Huacai Chen wrote:
> On Thu, Nov 5, 2026 at 5:14 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2026/10/23 下午3:59, Huacai Chen wrote:
>>> Bibo,
>>>
>>> Now I have applied this series but with some modifications, please
>>> double check and test the code.
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/chenhuacai/linux-loongson.git/log/?h=loongarch-next
>> In my v4 submit, it is:
>> +               case ACPI_IOVT_DEVICE_ENTRY_START:
>> +                       is_start = true;
>> +                       start_devid = ep->device_id;
>> +                       break;
>> however in rc1, it is modified as:
>>                   case ACPI_IOVT_DEVICE_ENTRY_START:
>>                           is_start = true;
>>                           entry->start_devid = ep->device_id;
>>                           break;
>> Here entry is not allocated and NULL pointer, it is allocated in
>> ACPI_IOVT_DEVICE_ENTRY_END switch case.
>>
>> I really do not know what is aid to modify it and let others to test.
> Sorry, it's my fault.
> 
> In theory I should point out the modification (eliminating the local
> variable start_devid) and refresh a new version. But we have no time
> to do this because when the first patch is merged into the ACPI repo
> it is near the merge window.
Maybe our work style is different, I do not care much for the coding style.
> 
> If you agree, I will submit such modifications to fix it.
fix is welcome.
> 
> diff --git a/drivers/acpi/loongarch/iovt.c b/drivers/acpi/loongarch/iovt.c
> index 87cee4f0fc80..4cdac28a1fd0 100644
> --- a/drivers/acpi/loongarch/iovt.c
> +++ b/drivers/acpi/loongarch/iovt.c
> @@ -115,6 +115,10 @@ static int __init iovt_add_iommu(struct
> acpi_iovt_iommu *iommu)
>          for (i = 0; i < iommu->device_entry_num; i++) {
>                  switch (ep->type) {
>                  case ACPI_IOVT_DEVICE_ENTRY_START:
> +                       entry = kzalloc_obj(struct iovt_device_entry,
> GFP_ATOMIC);
There will be memory leak problem if there are START/START/END in the table.

Regards
Bibo Mao
> +                       if (!entry)
> +                               return -ENOMEM;
> +
>                          is_start = true;
>                          entry->start_devid = ep->device_id;
>                          break;
> @@ -122,10 +126,6 @@ static int __init iovt_add_iommu(struct
> acpi_iovt_iommu *iommu)
>                          if (!is_start)
>                                  break;
> 
> -                       entry = kzalloc_obj(struct iovt_device_entry,
> GFP_ATOMIC);
> -                       if (!entry)
> -                               return -ENOMEM;
> -
>                          is_start = false;
>                          entry->end_devid = ep->device_id;
>                          list_add_tail(&entry->list, &np->ep_list);
> 
> Huacai
> 
>>
>>>
>>> Huacai
>>>
>>> On Mon, Oct 12, 2026 at 3:16 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>
>>>> This change is already merged in acpica tree.
>>>>
>>>> slightly ping.
>>>>
>>>> On Tue, Sep 22, 2026 at 08:10:55PM +0800, Bibo Mao wrote:
>>>>    > According to the IOVT specification, define macros for each bit of the
>>>>    > Flags field in the IOVT IOMMU structure.
>>>>    >
>>>>    > Link: https://github.com/open-acpica/acpica/commit/352f750e4cc0
>>>>    > Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>>    > ---
>>>>    >  include/acpi/actbl2.h | 7 +++++++
>>>>    >  1 file changed, 7 insertions(+)
>>>>    >
>>>>    > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
>>>>    > index baef525367b5..76e4a5bc79b4 100644
>>>>    > --- a/include/acpi/actbl2.h
>>>>    > +++ b/include/acpi/actbl2.h
>>>>    > @@ -919,6 +919,13 @@ struct acpi_iovt_iommu {
>>>>    >       u32 device_entry_offset;
>>>>    >  };
>>>>    >
>>>>    > +/* Masks for Flags field above */
>>>>    > +#define ACPI_IOVT_IOMMU_PCI_DEVICE           (1<<0)
>>>>    > +#define ACPI_IOVT_IOMMU_PXM_VALID            (1<<1)
>>>>    > +#define ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT    (1<<2)
>>>>    > +#define ACPI_IOVT_IOMMU_HW_CAP_SUPPORT               (1<<3)
>>>>    > +#define ACPI_IOVT_IOMMU_MSI_INT_BYPASS               (1<<4)
>>>>    > +
>>>>    >  struct acpi_iovt_device_entry {
>>>>    >       u8 type;
>>>>    >       u8 length;
>>>>    > --
>>>>    > 2.39.3
>>>>    >
>>>>
>>
>>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure
  2026-11-05  9:50           ` Bibo Mao
@ 2026-11-05 10:31             ` Huacai Chen
  2026-11-05 11:23               ` Bibo Mao
  0 siblings, 1 reply; 22+ messages in thread
From: Huacai Chen @ 2026-11-05 10:31 UTC (permalink / raw)
  To: Bibo Mao
  Cc: rafael, linux-acpi, acpica-devel, lenb, linux-kernel, lixianglai,
	loongarch, maciej.wieczor-retman, pawel.chmielewski

On Thu, Nov 5, 2026 at 5:49 PM Bibo Mao <maobibo@loongson.cn> wrote:
>
>
>
> On 2026/11/5 下午5:31, Huacai Chen wrote:
> > On Thu, Nov 5, 2026 at 5:14 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>
> >>
> >>
> >> On 2026/10/23 下午3:59, Huacai Chen wrote:
> >>> Bibo,
> >>>
> >>> Now I have applied this series but with some modifications, please
> >>> double check and test the code.
> >>>
> >>> https://git.kernel.org/pub/scm/linux/kernel/git/chenhuacai/linux-loongson.git/log/?h=loongarch-next
> >> In my v4 submit, it is:
> >> +               case ACPI_IOVT_DEVICE_ENTRY_START:
> >> +                       is_start = true;
> >> +                       start_devid = ep->device_id;
> >> +                       break;
> >> however in rc1, it is modified as:
> >>                   case ACPI_IOVT_DEVICE_ENTRY_START:
> >>                           is_start = true;
> >>                           entry->start_devid = ep->device_id;
> >>                           break;
> >> Here entry is not allocated and NULL pointer, it is allocated in
> >> ACPI_IOVT_DEVICE_ENTRY_END switch case.
> >>
> >> I really do not know what is aid to modify it and let others to test.
> > Sorry, it's my fault.
> >
> > In theory I should point out the modification (eliminating the local
> > variable start_devid) and refresh a new version. But we have no time
> > to do this because when the first patch is merged into the ACPI repo
> > it is near the merge window.
> Maybe our work style is different, I do not care much for the coding style.
> >
> > If you agree, I will submit such modifications to fix it.
> fix is welcome.
> >
> > diff --git a/drivers/acpi/loongarch/iovt.c b/drivers/acpi/loongarch/iovt.c
> > index 87cee4f0fc80..4cdac28a1fd0 100644
> > --- a/drivers/acpi/loongarch/iovt.c
> > +++ b/drivers/acpi/loongarch/iovt.c
> > @@ -115,6 +115,10 @@ static int __init iovt_add_iommu(struct
> > acpi_iovt_iommu *iommu)
> >          for (i = 0; i < iommu->device_entry_num; i++) {
> >                  switch (ep->type) {
> >                  case ACPI_IOVT_DEVICE_ENTRY_START:
> > +                       entry = kzalloc_obj(struct iovt_device_entry,
> > GFP_ATOMIC);
> There will be memory leak problem if there are START/START/END in the table.
That means a broken table? I think we don't need to consider broken
tables, because there may be kinds of broken pattern rather than just
a START/START/END.

Huacai

>
> Regards
> Bibo Mao
> > +                       if (!entry)
> > +                               return -ENOMEM;
> > +
> >                          is_start = true;
> >                          entry->start_devid = ep->device_id;
> >                          break;
> > @@ -122,10 +126,6 @@ static int __init iovt_add_iommu(struct
> > acpi_iovt_iommu *iommu)
> >                          if (!is_start)
> >                                  break;
> >
> > -                       entry = kzalloc_obj(struct iovt_device_entry,
> > GFP_ATOMIC);
> > -                       if (!entry)
> > -                               return -ENOMEM;
> > -
> >                          is_start = false;
> >                          entry->end_devid = ep->device_id;
> >                          list_add_tail(&entry->list, &np->ep_list);
> >
> > Huacai
> >
> >>
> >>>
> >>> Huacai
> >>>
> >>> On Mon, Oct 12, 2026 at 3:16 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>
> >>>> This change is already merged in acpica tree.
> >>>>
> >>>> slightly ping.
> >>>>
> >>>> On Tue, Sep 22, 2026 at 08:10:55PM +0800, Bibo Mao wrote:
> >>>>    > According to the IOVT specification, define macros for each bit of the
> >>>>    > Flags field in the IOVT IOMMU structure.
> >>>>    >
> >>>>    > Link: https://github.com/open-acpica/acpica/commit/352f750e4cc0
> >>>>    > Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >>>>    > ---
> >>>>    >  include/acpi/actbl2.h | 7 +++++++
> >>>>    >  1 file changed, 7 insertions(+)
> >>>>    >
> >>>>    > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> >>>>    > index baef525367b5..76e4a5bc79b4 100644
> >>>>    > --- a/include/acpi/actbl2.h
> >>>>    > +++ b/include/acpi/actbl2.h
> >>>>    > @@ -919,6 +919,13 @@ struct acpi_iovt_iommu {
> >>>>    >       u32 device_entry_offset;
> >>>>    >  };
> >>>>    >
> >>>>    > +/* Masks for Flags field above */
> >>>>    > +#define ACPI_IOVT_IOMMU_PCI_DEVICE           (1<<0)
> >>>>    > +#define ACPI_IOVT_IOMMU_PXM_VALID            (1<<1)
> >>>>    > +#define ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT    (1<<2)
> >>>>    > +#define ACPI_IOVT_IOMMU_HW_CAP_SUPPORT               (1<<3)
> >>>>    > +#define ACPI_IOVT_IOMMU_MSI_INT_BYPASS               (1<<4)
> >>>>    > +
> >>>>    >  struct acpi_iovt_device_entry {
> >>>>    >       u8 type;
> >>>>    >       u8 length;
> >>>>    > --
> >>>>    > 2.39.3
> >>>>    >
> >>>>
> >>
> >>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure
  2026-11-05 10:31             ` Huacai Chen
@ 2026-11-05 11:23               ` Bibo Mao
  2026-11-06 14:58                 ` Huacai Chen
  0 siblings, 1 reply; 22+ messages in thread
From: Bibo Mao @ 2026-11-05 11:23 UTC (permalink / raw)
  To: Huacai Chen
  Cc: rafael, linux-acpi, acpica-devel, lenb, linux-kernel, lixianglai,
	loongarch, maciej.wieczor-retman, pawel.chmielewski



On 2026/11/5 下午6:31, Huacai Chen wrote:
> On Thu, Nov 5, 2026 at 5:49 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2026/11/5 下午5:31, Huacai Chen wrote:
>>> On Thu, Nov 5, 2026 at 5:14 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>
>>>>
>>>>
>>>> On 2026/10/23 下午3:59, Huacai Chen wrote:
>>>>> Bibo,
>>>>>
>>>>> Now I have applied this series but with some modifications, please
>>>>> double check and test the code.
>>>>>
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/chenhuacai/linux-loongson.git/log/?h=loongarch-next
>>>> In my v4 submit, it is:
>>>> +               case ACPI_IOVT_DEVICE_ENTRY_START:
>>>> +                       is_start = true;
>>>> +                       start_devid = ep->device_id;
>>>> +                       break;
>>>> however in rc1, it is modified as:
>>>>                    case ACPI_IOVT_DEVICE_ENTRY_START:
>>>>                            is_start = true;
>>>>                            entry->start_devid = ep->device_id;
>>>>                            break;
>>>> Here entry is not allocated and NULL pointer, it is allocated in
>>>> ACPI_IOVT_DEVICE_ENTRY_END switch case.
>>>>
>>>> I really do not know what is aid to modify it and let others to test.
>>> Sorry, it's my fault.
>>>
>>> In theory I should point out the modification (eliminating the local
>>> variable start_devid) and refresh a new version. But we have no time
>>> to do this because when the first patch is merged into the ACPI repo
>>> it is near the merge window.
>> Maybe our work style is different, I do not care much for the coding style.
>>>
>>> If you agree, I will submit such modifications to fix it.
>> fix is welcome.
>>>
>>> diff --git a/drivers/acpi/loongarch/iovt.c b/drivers/acpi/loongarch/iovt.c
>>> index 87cee4f0fc80..4cdac28a1fd0 100644
>>> --- a/drivers/acpi/loongarch/iovt.c
>>> +++ b/drivers/acpi/loongarch/iovt.c
>>> @@ -115,6 +115,10 @@ static int __init iovt_add_iommu(struct
>>> acpi_iovt_iommu *iommu)
>>>           for (i = 0; i < iommu->device_entry_num; i++) {
>>>                   switch (ep->type) {
>>>                   case ACPI_IOVT_DEVICE_ENTRY_START:
>>> +                       entry = kzalloc_obj(struct iovt_device_entry,
>>> GFP_ATOMIC);
>> There will be memory leak problem if there are START/START/END in the table.
> That means a broken table? I think we don't need to consider broken
> tables, because there may be kinds of broken pattern rather than just
> a START/START/END.
yes, START/END/START, START/START/END, START/SINGLE/END. what is the 
relationship between "don't need to consider" and kinds of broken pattern?

what is the policy from your side with such BIOS table? let memory leak 
and continue?

> 
> Huacai
> 
>>
>> Regards
>> Bibo Mao
>>> +                       if (!entry)
>>> +                               return -ENOMEM;
>>> +
>>>                           is_start = true;
>>>                           entry->start_devid = ep->device_id;
>>>                           break;
>>> @@ -122,10 +126,6 @@ static int __init iovt_add_iommu(struct
>>> acpi_iovt_iommu *iommu)
>>>                           if (!is_start)
>>>                                   break;
>>>
>>> -                       entry = kzalloc_obj(struct iovt_device_entry,
>>> GFP_ATOMIC);
>>> -                       if (!entry)
>>> -                               return -ENOMEM;
>>> -
>>>                           is_start = false;
>>>                           entry->end_devid = ep->device_id;
>>>                           list_add_tail(&entry->list, &np->ep_list);
>>>
>>> Huacai
>>>
>>>>
>>>>>
>>>>> Huacai
>>>>>
>>>>> On Mon, Oct 12, 2026 at 3:16 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>
>>>>>> This change is already merged in acpica tree.
>>>>>>
>>>>>> slightly ping.
>>>>>>
>>>>>> On Tue, Sep 22, 2026 at 08:10:55PM +0800, Bibo Mao wrote:
>>>>>>     > According to the IOVT specification, define macros for each bit of the
>>>>>>     > Flags field in the IOVT IOMMU structure.
>>>>>>     >
>>>>>>     > Link: https://github.com/open-acpica/acpica/commit/352f750e4cc0
>>>>>>     > Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>>>>     > ---
>>>>>>     >  include/acpi/actbl2.h | 7 +++++++
>>>>>>     >  1 file changed, 7 insertions(+)
>>>>>>     >
>>>>>>     > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
>>>>>>     > index baef525367b5..76e4a5bc79b4 100644
>>>>>>     > --- a/include/acpi/actbl2.h
>>>>>>     > +++ b/include/acpi/actbl2.h
>>>>>>     > @@ -919,6 +919,13 @@ struct acpi_iovt_iommu {
>>>>>>     >       u32 device_entry_offset;
>>>>>>     >  };
>>>>>>     >
>>>>>>     > +/* Masks for Flags field above */
>>>>>>     > +#define ACPI_IOVT_IOMMU_PCI_DEVICE           (1<<0)
>>>>>>     > +#define ACPI_IOVT_IOMMU_PXM_VALID            (1<<1)
>>>>>>     > +#define ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT    (1<<2)
>>>>>>     > +#define ACPI_IOVT_IOMMU_HW_CAP_SUPPORT               (1<<3)
>>>>>>     > +#define ACPI_IOVT_IOMMU_MSI_INT_BYPASS               (1<<4)
>>>>>>     > +
>>>>>>     >  struct acpi_iovt_device_entry {
>>>>>>     >       u8 type;
>>>>>>     >       u8 length;
>>>>>>     > --
>>>>>>     > 2.39.3
>>>>>>     >
>>>>>>
>>>>
>>>>
>>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure
  2026-11-05 11:23               ` Bibo Mao
@ 2026-11-06 14:58                 ` Huacai Chen
  2026-11-07  1:45                   ` Bibo Mao
  0 siblings, 1 reply; 22+ messages in thread
From: Huacai Chen @ 2026-11-06 14:58 UTC (permalink / raw)
  To: Bibo Mao
  Cc: rafael, linux-acpi, acpica-devel, lenb, linux-kernel, lixianglai,
	loongarch, maciej.wieczor-retman, pawel.chmielewski

On Thu, Nov 5, 2026 at 7:22 PM Bibo Mao <maobibo@loongson.cn> wrote:
>
>
>
> On 2026/11/5 下午6:31, Huacai Chen wrote:
> > On Thu, Nov 5, 2026 at 5:49 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>
> >>
> >>
> >> On 2026/11/5 下午5:31, Huacai Chen wrote:
> >>> On Thu, Nov 5, 2026 at 5:14 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 2026/10/23 下午3:59, Huacai Chen wrote:
> >>>>> Bibo,
> >>>>>
> >>>>> Now I have applied this series but with some modifications, please
> >>>>> double check and test the code.
> >>>>>
> >>>>> https://git.kernel.org/pub/scm/linux/kernel/git/chenhuacai/linux-loongson.git/log/?h=loongarch-next
> >>>> In my v4 submit, it is:
> >>>> +               case ACPI_IOVT_DEVICE_ENTRY_START:
> >>>> +                       is_start = true;
> >>>> +                       start_devid = ep->device_id;
> >>>> +                       break;
> >>>> however in rc1, it is modified as:
> >>>>                    case ACPI_IOVT_DEVICE_ENTRY_START:
> >>>>                            is_start = true;
> >>>>                            entry->start_devid = ep->device_id;
> >>>>                            break;
> >>>> Here entry is not allocated and NULL pointer, it is allocated in
> >>>> ACPI_IOVT_DEVICE_ENTRY_END switch case.
> >>>>
> >>>> I really do not know what is aid to modify it and let others to test.
> >>> Sorry, it's my fault.
> >>>
> >>> In theory I should point out the modification (eliminating the local
> >>> variable start_devid) and refresh a new version. But we have no time
> >>> to do this because when the first patch is merged into the ACPI repo
> >>> it is near the merge window.
> >> Maybe our work style is different, I do not care much for the coding style.
> >>>
> >>> If you agree, I will submit such modifications to fix it.
> >> fix is welcome.
> >>>
> >>> diff --git a/drivers/acpi/loongarch/iovt.c b/drivers/acpi/loongarch/iovt.c
> >>> index 87cee4f0fc80..4cdac28a1fd0 100644
> >>> --- a/drivers/acpi/loongarch/iovt.c
> >>> +++ b/drivers/acpi/loongarch/iovt.c
> >>> @@ -115,6 +115,10 @@ static int __init iovt_add_iommu(struct
> >>> acpi_iovt_iommu *iommu)
> >>>           for (i = 0; i < iommu->device_entry_num; i++) {
> >>>                   switch (ep->type) {
> >>>                   case ACPI_IOVT_DEVICE_ENTRY_START:
> >>> +                       entry = kzalloc_obj(struct iovt_device_entry,
> >>> GFP_ATOMIC);
> >> There will be memory leak problem if there are START/START/END in the table.
> > That means a broken table? I think we don't need to consider broken
> > tables, because there may be kinds of broken pattern rather than just
> > a START/START/END.
> yes, START/END/START, START/START/END, START/SINGLE/END. what is the
> relationship between "don't need to consider" and kinds of broken pattern?
>
> what is the policy from your side with such BIOS table? let memory leak
> and continue?
The driver can only work with a good table or no table, but not with a
broken table.

Take START1/START2/END as an example of broken table: in your original
code, the END entry will be pairing with START2. But for a broken
table who can ensure they are the correct pair? If the real correct
pair is START1 and END, then (entry->start_devid, entry->end_devid)
will be a wrong device range. Such mistakes are much worse than a
memery leak.

If you want to say "I don't care about correct pairing, memory leak is
bad in any case.", OK, we can do it like this:

diff --git a/drivers/acpi/loongarch/iovt.c b/drivers/acpi/loongarch/iovt.c
index 87cee4f0fc80..ebd11e22c9be 100644
--- a/drivers/acpi/loongarch/iovt.c
+++ b/drivers/acpi/loongarch/iovt.c
@@ -115,6 +115,13 @@ static int __init iovt_add_iommu(struct
acpi_iovt_iommu *iommu)
        for (i = 0; i < iommu->device_entry_num; i++) {
                switch (ep->type) {
                case ACPI_IOVT_DEVICE_ENTRY_START:
+                       if (is_start)
+                               break;
+
+                       entry = kzalloc_obj(struct iovt_device_entry,
GFP_ATOMIC);
+                       if (!entry)
+                               return -ENOMEM;
+
                        is_start = true;
                        entry->start_devid = ep->device_id;
                        break;
@@ -122,10 +129,6 @@ static int __init iovt_add_iommu(struct
acpi_iovt_iommu *iommu)
                        if (!is_start)
                                break;

-                       entry = kzalloc_obj(struct iovt_device_entry,
GFP_ATOMIC);
-                       if (!entry)
-                               return -ENOMEM;
-
                        is_start = false;
                        entry->end_devid = ep->device_id;
                        list_add_tail(&entry->list, &np->ep_list);

Huacai

>
> >
> > Huacai
> >
> >>
> >> Regards
> >> Bibo Mao
> >>> +                       if (!entry)
> >>> +                               return -ENOMEM;
> >>> +
> >>>                           is_start = true;
> >>>                           entry->start_devid = ep->device_id;
> >>>                           break;
> >>> @@ -122,10 +126,6 @@ static int __init iovt_add_iommu(struct
> >>> acpi_iovt_iommu *iommu)
> >>>                           if (!is_start)
> >>>                                   break;
> >>>
> >>> -                       entry = kzalloc_obj(struct iovt_device_entry,
> >>> GFP_ATOMIC);
> >>> -                       if (!entry)
> >>> -                               return -ENOMEM;
> >>> -
> >>>                           is_start = false;
> >>>                           entry->end_devid = ep->device_id;
> >>>                           list_add_tail(&entry->list, &np->ep_list);
> >>>
> >>> Huacai
> >>>
> >>>>
> >>>>>
> >>>>> Huacai
> >>>>>
> >>>>> On Mon, Oct 12, 2026 at 3:16 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>>>
> >>>>>> This change is already merged in acpica tree.
> >>>>>>
> >>>>>> slightly ping.
> >>>>>>
> >>>>>> On Tue, Sep 22, 2026 at 08:10:55PM +0800, Bibo Mao wrote:
> >>>>>>     > According to the IOVT specification, define macros for each bit of the
> >>>>>>     > Flags field in the IOVT IOMMU structure.
> >>>>>>     >
> >>>>>>     > Link: https://github.com/open-acpica/acpica/commit/352f750e4cc0
> >>>>>>     > Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >>>>>>     > ---
> >>>>>>     >  include/acpi/actbl2.h | 7 +++++++
> >>>>>>     >  1 file changed, 7 insertions(+)
> >>>>>>     >
> >>>>>>     > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> >>>>>>     > index baef525367b5..76e4a5bc79b4 100644
> >>>>>>     > --- a/include/acpi/actbl2.h
> >>>>>>     > +++ b/include/acpi/actbl2.h
> >>>>>>     > @@ -919,6 +919,13 @@ struct acpi_iovt_iommu {
> >>>>>>     >       u32 device_entry_offset;
> >>>>>>     >  };
> >>>>>>     >
> >>>>>>     > +/* Masks for Flags field above */
> >>>>>>     > +#define ACPI_IOVT_IOMMU_PCI_DEVICE           (1<<0)
> >>>>>>     > +#define ACPI_IOVT_IOMMU_PXM_VALID            (1<<1)
> >>>>>>     > +#define ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT    (1<<2)
> >>>>>>     > +#define ACPI_IOVT_IOMMU_HW_CAP_SUPPORT               (1<<3)
> >>>>>>     > +#define ACPI_IOVT_IOMMU_MSI_INT_BYPASS               (1<<4)
> >>>>>>     > +
> >>>>>>     >  struct acpi_iovt_device_entry {
> >>>>>>     >       u8 type;
> >>>>>>     >       u8 length;
> >>>>>>     > --
> >>>>>>     > 2.39.3
> >>>>>>     >
> >>>>>>
> >>>>
> >>>>
> >>
>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure
  2026-11-06 14:58                 ` Huacai Chen
@ 2026-11-07  1:45                   ` Bibo Mao
  2026-11-07 12:25                     ` Huacai Chen
  0 siblings, 1 reply; 22+ messages in thread
From: Bibo Mao @ 2026-11-07  1:45 UTC (permalink / raw)
  To: Huacai Chen
  Cc: rafael, linux-acpi, acpica-devel, lenb, linux-kernel, lixianglai,
	loongarch, maciej.wieczor-retman, pawel.chmielewski



On 2026/11/6 下午10:58, Huacai Chen wrote:
> On Thu, Nov 5, 2026 at 7:22 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2026/11/5 下午6:31, Huacai Chen wrote:
>>> On Thu, Nov 5, 2026 at 5:49 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>
>>>>
>>>>
>>>> On 2026/11/5 下午5:31, Huacai Chen wrote:
>>>>> On Thu, Nov 5, 2026 at 5:14 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 2026/10/23 下午3:59, Huacai Chen wrote:
>>>>>>> Bibo,
>>>>>>>
>>>>>>> Now I have applied this series but with some modifications, please
>>>>>>> double check and test the code.
>>>>>>>
>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/chenhuacai/linux-loongson.git/log/?h=loongarch-next
>>>>>> In my v4 submit, it is:
>>>>>> +               case ACPI_IOVT_DEVICE_ENTRY_START:
>>>>>> +                       is_start = true;
>>>>>> +                       start_devid = ep->device_id;
>>>>>> +                       break;
>>>>>> however in rc1, it is modified as:
>>>>>>                     case ACPI_IOVT_DEVICE_ENTRY_START:
>>>>>>                             is_start = true;
>>>>>>                             entry->start_devid = ep->device_id;
>>>>>>                             break;
>>>>>> Here entry is not allocated and NULL pointer, it is allocated in
>>>>>> ACPI_IOVT_DEVICE_ENTRY_END switch case.
>>>>>>
>>>>>> I really do not know what is aid to modify it and let others to test.
>>>>> Sorry, it's my fault.
>>>>>
>>>>> In theory I should point out the modification (eliminating the local
>>>>> variable start_devid) and refresh a new version. But we have no time
>>>>> to do this because when the first patch is merged into the ACPI repo
>>>>> it is near the merge window.
>>>> Maybe our work style is different, I do not care much for the coding style.
>>>>>
>>>>> If you agree, I will submit such modifications to fix it.
>>>> fix is welcome.
>>>>>
>>>>> diff --git a/drivers/acpi/loongarch/iovt.c b/drivers/acpi/loongarch/iovt.c
>>>>> index 87cee4f0fc80..4cdac28a1fd0 100644
>>>>> --- a/drivers/acpi/loongarch/iovt.c
>>>>> +++ b/drivers/acpi/loongarch/iovt.c
>>>>> @@ -115,6 +115,10 @@ static int __init iovt_add_iommu(struct
>>>>> acpi_iovt_iommu *iommu)
>>>>>            for (i = 0; i < iommu->device_entry_num; i++) {
>>>>>                    switch (ep->type) {
>>>>>                    case ACPI_IOVT_DEVICE_ENTRY_START:
>>>>> +                       entry = kzalloc_obj(struct iovt_device_entry,
>>>>> GFP_ATOMIC);
>>>> There will be memory leak problem if there are START/START/END in the table.
>>> That means a broken table? I think we don't need to consider broken
>>> tables, because there may be kinds of broken pattern rather than just
>>> a START/START/END.
>> yes, START/END/START, START/START/END, START/SINGLE/END. what is the
>> relationship between "don't need to consider" and kinds of broken pattern?
>>
>> what is the policy from your side with such BIOS table? let memory leak
>> and continue?
> The driver can only work with a good table or no table, but not with a
> broken table.
With a broken table, software can discard the whole table, or discard 
the single table entry.
> 
> Take START1/START2/END as an example of broken table: in your original
> code, the END entry will be pairing with START2. But for a broken
> table who can ensure they are the correct pair? If the real correct
> pair is START1 and END, then (entry->start_devid, entry->end_devid)
there is only one policy, software does not know START1/END is right or 
START2/END is right.
> will be a wrong device range. Such mistakes are much worse than a
> memery leak.
> 
> If you want to say "I don't care about correct pairing, memory leak is
> bad in any case.", OK, we can do it like this:
> 
> diff --git a/drivers/acpi/loongarch/iovt.c b/drivers/acpi/loongarch/iovt.c
> index 87cee4f0fc80..ebd11e22c9be 100644
> --- a/drivers/acpi/loongarch/iovt.c
> +++ b/drivers/acpi/loongarch/iovt.c
> @@ -115,6 +115,13 @@ static int __init iovt_add_iommu(struct
> acpi_iovt_iommu *iommu)
>          for (i = 0; i < iommu->device_entry_num; i++) {
>                  switch (ep->type) {
>                  case ACPI_IOVT_DEVICE_ENTRY_START:
> +                       if (is_start)
> +                               break;
there is still memory leak with START/SINGLE/END pairs, is_start set 
with false with ACPI_IOVT_DEVICE_ENTRY_SINGLE case.

Regards
Bibo Mao
> +
> +                       entry = kzalloc_obj(struct iovt_device_entry,
> GFP_ATOMIC);
> +                       if (!entry)
> +                               return -ENOMEM;
> +
>                          is_start = true;
>                          entry->start_devid = ep->device_id;
>                          break;
> @@ -122,10 +129,6 @@ static int __init iovt_add_iommu(struct
> acpi_iovt_iommu *iommu)
>                          if (!is_start)
>                                  break;
> 
> -                       entry = kzalloc_obj(struct iovt_device_entry,
> GFP_ATOMIC);
> -                       if (!entry)
> -                               return -ENOMEM;
> -
>                          is_start = false;
>                          entry->end_devid = ep->device_id;
>                          list_add_tail(&entry->list, &np->ep_list);
> 
> Huacai
> 
>>
>>>
>>> Huacai
>>>
>>>>
>>>> Regards
>>>> Bibo Mao
>>>>> +                       if (!entry)
>>>>> +                               return -ENOMEM;
>>>>> +
>>>>>                            is_start = true;
>>>>>                            entry->start_devid = ep->device_id;
>>>>>                            break;
>>>>> @@ -122,10 +126,6 @@ static int __init iovt_add_iommu(struct
>>>>> acpi_iovt_iommu *iommu)
>>>>>                            if (!is_start)
>>>>>                                    break;
>>>>>
>>>>> -                       entry = kzalloc_obj(struct iovt_device_entry,
>>>>> GFP_ATOMIC);
>>>>> -                       if (!entry)
>>>>> -                               return -ENOMEM;
>>>>> -
>>>>>                            is_start = false;
>>>>>                            entry->end_devid = ep->device_id;
>>>>>                            list_add_tail(&entry->list, &np->ep_list);
>>>>>
>>>>> Huacai
>>>>>
>>>>>>
>>>>>>>
>>>>>>> Huacai
>>>>>>>
>>>>>>> On Mon, Oct 12, 2026 at 3:16 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>>>
>>>>>>>> This change is already merged in acpica tree.
>>>>>>>>
>>>>>>>> slightly ping.
>>>>>>>>
>>>>>>>> On Tue, Sep 22, 2026 at 08:10:55PM +0800, Bibo Mao wrote:
>>>>>>>>      > According to the IOVT specification, define macros for each bit of the
>>>>>>>>      > Flags field in the IOVT IOMMU structure.
>>>>>>>>      >
>>>>>>>>      > Link: https://github.com/open-acpica/acpica/commit/352f750e4cc0
>>>>>>>>      > Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>>>>>>      > ---
>>>>>>>>      >  include/acpi/actbl2.h | 7 +++++++
>>>>>>>>      >  1 file changed, 7 insertions(+)
>>>>>>>>      >
>>>>>>>>      > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
>>>>>>>>      > index baef525367b5..76e4a5bc79b4 100644
>>>>>>>>      > --- a/include/acpi/actbl2.h
>>>>>>>>      > +++ b/include/acpi/actbl2.h
>>>>>>>>      > @@ -919,6 +919,13 @@ struct acpi_iovt_iommu {
>>>>>>>>      >       u32 device_entry_offset;
>>>>>>>>      >  };
>>>>>>>>      >
>>>>>>>>      > +/* Masks for Flags field above */
>>>>>>>>      > +#define ACPI_IOVT_IOMMU_PCI_DEVICE           (1<<0)
>>>>>>>>      > +#define ACPI_IOVT_IOMMU_PXM_VALID            (1<<1)
>>>>>>>>      > +#define ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT    (1<<2)
>>>>>>>>      > +#define ACPI_IOVT_IOMMU_HW_CAP_SUPPORT               (1<<3)
>>>>>>>>      > +#define ACPI_IOVT_IOMMU_MSI_INT_BYPASS               (1<<4)
>>>>>>>>      > +
>>>>>>>>      >  struct acpi_iovt_device_entry {
>>>>>>>>      >       u8 type;
>>>>>>>>      >       u8 length;
>>>>>>>>      > --
>>>>>>>>      > 2.39.3
>>>>>>>>      >
>>>>>>>>
>>>>>>
>>>>>>
>>>>
>>
>>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure
  2026-11-07  1:45                   ` Bibo Mao
@ 2026-11-07 12:25                     ` Huacai Chen
  2026-11-09  1:12                       ` Bibo Mao
  2026-11-10  3:39                       ` Bibo Mao
  0 siblings, 2 replies; 22+ messages in thread
From: Huacai Chen @ 2026-11-07 12:25 UTC (permalink / raw)
  To: Bibo Mao
  Cc: rafael, linux-acpi, acpica-devel, lenb, linux-kernel, lixianglai,
	loongarch, maciej.wieczor-retman, pawel.chmielewski

On Sat, Nov 7, 2026 at 9:43 AM Bibo Mao <maobibo@loongson.cn> wrote:
>
>
>
> On 2026/11/6 下午10:58, Huacai Chen wrote:
> > On Thu, Nov 5, 2026 at 7:22 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>
> >>
> >>
> >> On 2026/11/5 下午6:31, Huacai Chen wrote:
> >>> On Thu, Nov 5, 2026 at 5:49 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 2026/11/5 下午5:31, Huacai Chen wrote:
> >>>>> On Thu, Nov 5, 2026 at 5:14 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On 2026/10/23 下午3:59, Huacai Chen wrote:
> >>>>>>> Bibo,
> >>>>>>>
> >>>>>>> Now I have applied this series but with some modifications, please
> >>>>>>> double check and test the code.
> >>>>>>>
> >>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/chenhuacai/linux-loongson.git/log/?h=loongarch-next
> >>>>>> In my v4 submit, it is:
> >>>>>> +               case ACPI_IOVT_DEVICE_ENTRY_START:
> >>>>>> +                       is_start = true;
> >>>>>> +                       start_devid = ep->device_id;
> >>>>>> +                       break;
> >>>>>> however in rc1, it is modified as:
> >>>>>>                     case ACPI_IOVT_DEVICE_ENTRY_START:
> >>>>>>                             is_start = true;
> >>>>>>                             entry->start_devid = ep->device_id;
> >>>>>>                             break;
> >>>>>> Here entry is not allocated and NULL pointer, it is allocated in
> >>>>>> ACPI_IOVT_DEVICE_ENTRY_END switch case.
> >>>>>>
> >>>>>> I really do not know what is aid to modify it and let others to test.
> >>>>> Sorry, it's my fault.
> >>>>>
> >>>>> In theory I should point out the modification (eliminating the local
> >>>>> variable start_devid) and refresh a new version. But we have no time
> >>>>> to do this because when the first patch is merged into the ACPI repo
> >>>>> it is near the merge window.
> >>>> Maybe our work style is different, I do not care much for the coding style.
> >>>>>
> >>>>> If you agree, I will submit such modifications to fix it.
> >>>> fix is welcome.
> >>>>>
> >>>>> diff --git a/drivers/acpi/loongarch/iovt.c b/drivers/acpi/loongarch/iovt.c
> >>>>> index 87cee4f0fc80..4cdac28a1fd0 100644
> >>>>> --- a/drivers/acpi/loongarch/iovt.c
> >>>>> +++ b/drivers/acpi/loongarch/iovt.c
> >>>>> @@ -115,6 +115,10 @@ static int __init iovt_add_iommu(struct
> >>>>> acpi_iovt_iommu *iommu)
> >>>>>            for (i = 0; i < iommu->device_entry_num; i++) {
> >>>>>                    switch (ep->type) {
> >>>>>                    case ACPI_IOVT_DEVICE_ENTRY_START:
> >>>>> +                       entry = kzalloc_obj(struct iovt_device_entry,
> >>>>> GFP_ATOMIC);
> >>>> There will be memory leak problem if there are START/START/END in the table.
> >>> That means a broken table? I think we don't need to consider broken
> >>> tables, because there may be kinds of broken pattern rather than just
> >>> a START/START/END.
> >> yes, START/END/START, START/START/END, START/SINGLE/END. what is the
> >> relationship between "don't need to consider" and kinds of broken pattern?
> >>
> >> what is the policy from your side with such BIOS table? let memory leak
> >> and continue?
> > The driver can only work with a good table or no table, but not with a
> > broken table.
> With a broken table, software can discard the whole table, or discard
> the single table entry.
Discard a single entry is dangerous because the pairing may be wrong,
so if we should handle a broken table, we should discard the whole
table.

Discarding the whole table need a rework, the is_start variable is not
enough. You can consider the below solution:

diff --git a/drivers/acpi/loongarch/iovt.c b/drivers/acpi/loongarch/iovt.c
index 87cee4f0fc80..8381de278e3a 100644
--- a/drivers/acpi/loongarch/iovt.c
+++ b/drivers/acpi/loongarch/iovt.c
@@ -73,10 +73,44 @@ static int __init iovt_get_pci_iommu_fwnode(struct
iovt_fwnode *np, u16 segment,
        return 0;
 }

+static int __init iovt_validate_entries(struct acpi_iovt_iommu *iommu)
+{
+       int i, done = 1;
+       struct acpi_iovt_device_entry *ep;
+
+       ep = ACPI_ADD_PTR(struct acpi_iovt_device_entry, iommu,
iommu->device_entry_offset);
+       for (i = 0; i < iommu->device_entry_num; i++) {
+               switch (ep->type) {
+               case ACPI_IOVT_DEVICE_ENTRY_START:
+                       if (!done)
+                               return -EINVAL;
+
+                       done = 0;
+                       break;
+               case ACPI_IOVT_DEVICE_ENTRY_END:
+                       if (done)
+                               return -EINVAL;
+
+                       done = 1;
+                       break;
+               case ACPI_IOVT_DEVICE_ENTRY_SINGLE:
+                       if (!done)
+                               return -EINVAL;
+
+                       done = 1;
+                       break;
+               default:
+                       return -EINVAL;
+               }
+               ep = ACPI_ADD_PTR(struct acpi_iovt_device_entry, ep,
ep->length);
+       }
+
+       return done ? 0 : -EINVAL;
+}
+
 static int __init iovt_add_iommu(struct acpi_iovt_iommu *iommu)
 {
        int i, ret;
-       bool is_start = false;
        struct iovt_fwnode *np;
        struct iovt_device_entry *entry;
        struct acpi_iovt_device_entry *ep;
@@ -110,23 +144,22 @@ static int __init iovt_add_iommu(struct
acpi_iovt_iommu *iommu)
        if (np->flag & ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT)
                goto skip;

+       if (iovt_validate_entries(iommu) < 0)
+               return -EINVAL;
+
        INIT_LIST_HEAD(&np->ep_list);
        ep = ACPI_ADD_PTR(struct acpi_iovt_device_entry, iommu,
iommu->device_entry_offset);
        for (i = 0; i < iommu->device_entry_num; i++) {
                switch (ep->type) {
                case ACPI_IOVT_DEVICE_ENTRY_START:
-                       is_start = true;
-                       entry->start_devid = ep->device_id;
-                       break;
-               case ACPI_IOVT_DEVICE_ENTRY_END:
-                       if (!is_start)
-                               break;
-
                        entry = kzalloc_obj(struct iovt_device_entry,
GFP_ATOMIC);
                        if (!entry)
                                return -ENOMEM;

-                       is_start = false;
+                       entry->start_devid = ep->device_id;
+                       break;
+               case ACPI_IOVT_DEVICE_ENTRY_END:
+                       /* START & END are paired */
                        entry->end_devid = ep->device_id;
                        list_add_tail(&entry->list, &np->ep_list);
                        break;
@@ -135,13 +168,12 @@ static int __init iovt_add_iommu(struct
acpi_iovt_iommu *iommu)
                        if (!entry)
                                return -ENOMEM;

-                       is_start = false;
                        entry->start_devid = ep->device_id;
                        entry->end_devid = ep->device_id;
                        list_add_tail(&entry->list, &np->ep_list);
                        break;
                default:
-                       break;
+                       return -EINVAL;
                }
                ep = ACPI_ADD_PTR(struct acpi_iovt_device_entry, ep,
ep->length);
        }

> >
> > Take START1/START2/END as an example of broken table: in your original
> > code, the END entry will be pairing with START2. But for a broken
> > table who can ensure they are the correct pair? If the real correct
> > pair is START1 and END, then (entry->start_devid, entry->end_devid)
> there is only one policy, software does not know START1/END is right or
> START2/END is right.
> > will be a wrong device range. Such mistakes are much worse than a
> > memery leak.
> >
> > If you want to say "I don't care about correct pairing, memory leak is
> > bad in any case.", OK, we can do it like this:
> >
> > diff --git a/drivers/acpi/loongarch/iovt.c b/drivers/acpi/loongarch/iovt.c
> > index 87cee4f0fc80..ebd11e22c9be 100644
> > --- a/drivers/acpi/loongarch/iovt.c
> > +++ b/drivers/acpi/loongarch/iovt.c
> > @@ -115,6 +115,13 @@ static int __init iovt_add_iommu(struct
> > acpi_iovt_iommu *iommu)
> >          for (i = 0; i < iommu->device_entry_num; i++) {
> >                  switch (ep->type) {
> >                  case ACPI_IOVT_DEVICE_ENTRY_START:
> > +                       if (is_start)
> > +                               break;
> there is still memory leak with START/SINGLE/END pairs, is_start set
> with false with ACPI_IOVT_DEVICE_ENTRY_SINGLE case.
>
> Regards
> Bibo Mao
> > +
> > +                       entry = kzalloc_obj(struct iovt_device_entry,
> > GFP_ATOMIC);
> > +                       if (!entry)
> > +                               return -ENOMEM;
> > +
> >                          is_start = true;
> >                          entry->start_devid = ep->device_id;
> >                          break;
> > @@ -122,10 +129,6 @@ static int __init iovt_add_iommu(struct
> > acpi_iovt_iommu *iommu)
> >                          if (!is_start)
> >                                  break;
> >
> > -                       entry = kzalloc_obj(struct iovt_device_entry,
> > GFP_ATOMIC);
> > -                       if (!entry)
> > -                               return -ENOMEM;
> > -
> >                          is_start = false;
> >                          entry->end_devid = ep->device_id;
> >                          list_add_tail(&entry->list, &np->ep_list);
> >
> > Huacai
> >
> >>
> >>>
> >>> Huacai
> >>>
> >>>>
> >>>> Regards
> >>>> Bibo Mao
> >>>>> +                       if (!entry)
> >>>>> +                               return -ENOMEM;
> >>>>> +
> >>>>>                            is_start = true;
> >>>>>                            entry->start_devid = ep->device_id;
> >>>>>                            break;
> >>>>> @@ -122,10 +126,6 @@ static int __init iovt_add_iommu(struct
> >>>>> acpi_iovt_iommu *iommu)
> >>>>>                            if (!is_start)
> >>>>>                                    break;
> >>>>>
> >>>>> -                       entry = kzalloc_obj(struct iovt_device_entry,
> >>>>> GFP_ATOMIC);
> >>>>> -                       if (!entry)
> >>>>> -                               return -ENOMEM;
> >>>>> -
> >>>>>                            is_start = false;
> >>>>>                            entry->end_devid = ep->device_id;
> >>>>>                            list_add_tail(&entry->list, &np->ep_list);
> >>>>>
> >>>>> Huacai
> >>>>>
> >>>>>>
> >>>>>>>
> >>>>>>> Huacai
> >>>>>>>
> >>>>>>> On Mon, Oct 12, 2026 at 3:16 PM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>>>>>
> >>>>>>>> This change is already merged in acpica tree.
> >>>>>>>>
> >>>>>>>> slightly ping.
> >>>>>>>>
> >>>>>>>> On Tue, Sep 22, 2026 at 08:10:55PM +0800, Bibo Mao wrote:
> >>>>>>>>      > According to the IOVT specification, define macros for each bit of the
> >>>>>>>>      > Flags field in the IOVT IOMMU structure.
> >>>>>>>>      >
> >>>>>>>>      > Link: https://github.com/open-acpica/acpica/commit/352f750e4cc0
> >>>>>>>>      > Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >>>>>>>>      > ---
> >>>>>>>>      >  include/acpi/actbl2.h | 7 +++++++
> >>>>>>>>      >  1 file changed, 7 insertions(+)
> >>>>>>>>      >
> >>>>>>>>      > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> >>>>>>>>      > index baef525367b5..76e4a5bc79b4 100644
> >>>>>>>>      > --- a/include/acpi/actbl2.h
> >>>>>>>>      > +++ b/include/acpi/actbl2.h
> >>>>>>>>      > @@ -919,6 +919,13 @@ struct acpi_iovt_iommu {
> >>>>>>>>      >       u32 device_entry_offset;
> >>>>>>>>      >  };
> >>>>>>>>      >
> >>>>>>>>      > +/* Masks for Flags field above */
> >>>>>>>>      > +#define ACPI_IOVT_IOMMU_PCI_DEVICE           (1<<0)
> >>>>>>>>      > +#define ACPI_IOVT_IOMMU_PXM_VALID            (1<<1)
> >>>>>>>>      > +#define ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT    (1<<2)
> >>>>>>>>      > +#define ACPI_IOVT_IOMMU_HW_CAP_SUPPORT               (1<<3)
> >>>>>>>>      > +#define ACPI_IOVT_IOMMU_MSI_INT_BYPASS               (1<<4)
> >>>>>>>>      > +
> >>>>>>>>      >  struct acpi_iovt_device_entry {
> >>>>>>>>      >       u8 type;
> >>>>>>>>      >       u8 length;
> >>>>>>>>      > --
> >>>>>>>>      > 2.39.3
> >>>>>>>>      >
> >>>>>>>>
> >>>>>>
> >>>>>>
> >>>>
> >>
> >>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure
  2026-11-07 12:25                     ` Huacai Chen
@ 2026-11-09  1:12                       ` Bibo Mao
  2026-11-10  3:39                       ` Bibo Mao
  1 sibling, 0 replies; 22+ messages in thread
From: Bibo Mao @ 2026-11-09  1:12 UTC (permalink / raw)
  To: Huacai Chen
  Cc: rafael, linux-acpi, acpica-devel, lenb, linux-kernel, lixianglai,
	loongarch, maciej.wieczor-retman, pawel.chmielewski



On 2026/11/7 下午8:25, Huacai Chen wrote:
> On Sat, Nov 7, 2026 at 9:43 AM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2026/11/6 下午10:58, Huacai Chen wrote:
>>> On Thu, Nov 5, 2026 at 7:22 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>
>>>>
>>>>
>>>> On 2026/11/5 下午6:31, Huacai Chen wrote:
>>>>> On Thu, Nov 5, 2026 at 5:49 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 2026/11/5 下午5:31, Huacai Chen wrote:
>>>>>>> On Thu, Nov 5, 2026 at 5:14 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2026/10/23 下午3:59, Huacai Chen wrote:
>>>>>>>>> Bibo,
>>>>>>>>>
>>>>>>>>> Now I have applied this series but with some modifications, please
>>>>>>>>> double check and test the code.
>>>>>>>>>
>>>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/chenhuacai/linux-loongson.git/log/?h=loongarch-next
>>>>>>>> In my v4 submit, it is:
>>>>>>>> +               case ACPI_IOVT_DEVICE_ENTRY_START:
>>>>>>>> +                       is_start = true;
>>>>>>>> +                       start_devid = ep->device_id;
>>>>>>>> +                       break;
>>>>>>>> however in rc1, it is modified as:
>>>>>>>>                      case ACPI_IOVT_DEVICE_ENTRY_START:
>>>>>>>>                              is_start = true;
>>>>>>>>                              entry->start_devid = ep->device_id;
>>>>>>>>                              break;
>>>>>>>> Here entry is not allocated and NULL pointer, it is allocated in
>>>>>>>> ACPI_IOVT_DEVICE_ENTRY_END switch case.
>>>>>>>>
>>>>>>>> I really do not know what is aid to modify it and let others to test.
>>>>>>> Sorry, it's my fault.
>>>>>>>
>>>>>>> In theory I should point out the modification (eliminating the local
>>>>>>> variable start_devid) and refresh a new version. But we have no time
>>>>>>> to do this because when the first patch is merged into the ACPI repo
>>>>>>> it is near the merge window.
>>>>>> Maybe our work style is different, I do not care much for the coding style.
>>>>>>>
>>>>>>> If you agree, I will submit such modifications to fix it.
>>>>>> fix is welcome.
>>>>>>>
>>>>>>> diff --git a/drivers/acpi/loongarch/iovt.c b/drivers/acpi/loongarch/iovt.c
>>>>>>> index 87cee4f0fc80..4cdac28a1fd0 100644
>>>>>>> --- a/drivers/acpi/loongarch/iovt.c
>>>>>>> +++ b/drivers/acpi/loongarch/iovt.c
>>>>>>> @@ -115,6 +115,10 @@ static int __init iovt_add_iommu(struct
>>>>>>> acpi_iovt_iommu *iommu)
>>>>>>>             for (i = 0; i < iommu->device_entry_num; i++) {
>>>>>>>                     switch (ep->type) {
>>>>>>>                     case ACPI_IOVT_DEVICE_ENTRY_START:
>>>>>>> +                       entry = kzalloc_obj(struct iovt_device_entry,
>>>>>>> GFP_ATOMIC);
>>>>>> There will be memory leak problem if there are START/START/END in the table.
>>>>> That means a broken table? I think we don't need to consider broken
>>>>> tables, because there may be kinds of broken pattern rather than just
>>>>> a START/START/END.
>>>> yes, START/END/START, START/START/END, START/SINGLE/END. what is the
>>>> relationship between "don't need to consider" and kinds of broken pattern?
>>>>
>>>> what is the policy from your side with such BIOS table? let memory leak
>>>> and continue?
>>> The driver can only work with a good table or no table, but not with a
>>> broken table.
>> With a broken table, software can discard the whole table, or discard
>> the single table entry.
> Discard a single entry is dangerous because the pairing may be wrong,
> so if we should handle a broken table, we should discard the whole
> table.
> 
> Discarding the whole table need a rework, the is_start variable is not
> enough. You can consider the below solution:
Discarding the whole table is reasonable for me.
> 
> diff --git a/drivers/acpi/loongarch/iovt.c b/drivers/acpi/loongarch/iovt.c
> index 87cee4f0fc80..8381de278e3a 100644
> --- a/drivers/acpi/loongarch/iovt.c
> +++ b/drivers/acpi/loongarch/iovt.c
> @@ -73,10 +73,44 @@ static int __init iovt_get_pci_iommu_fwnode(struct
> iovt_fwnode *np, u16 segment,
>          return 0;
>   }
> 
> +static int __init iovt_validate_entries(struct acpi_iovt_iommu *iommu)
> +{
> +       int i, done = 1;
> +       struct acpi_iovt_device_entry *ep;
> +
> +       ep = ACPI_ADD_PTR(struct acpi_iovt_device_entry, iommu,
> iommu->device_entry_offset);
> +       for (i = 0; i < iommu->device_entry_num; i++) {
> +               switch (ep->type) {
> +               case ACPI_IOVT_DEVICE_ENTRY_START:
> +                       if (!done)
> +                               return -EINVAL;
> +
> +                       done = 0;
> +                       break;
> +               case ACPI_IOVT_DEVICE_ENTRY_END:
> +                       if (done)
> +                               return -EINVAL;
> +
> +                       done = 1;
> +                       break;
> +               case ACPI_IOVT_DEVICE_ENTRY_SINGLE:
> +                       if (!done)
> +                               return -EINVAL;
> +
> +                       done = 1;
> +                       break;
> +               default:
> +                       return -EINVAL;
> +               }
> +               ep = ACPI_ADD_PTR(struct acpi_iovt_device_entry, ep,
> ep->length);
> +       }
> +
> +       return done ? 0 : -EINVAL;
> +}
> +
>   static int __init iovt_add_iommu(struct acpi_iovt_iommu *iommu)
>   {
>          int i, ret;
> -       bool is_start = false;
>          struct iovt_fwnode *np;
>          struct iovt_device_entry *entry;
>          struct acpi_iovt_device_entry *ep;
> @@ -110,23 +144,22 @@ static int __init iovt_add_iommu(struct
> acpi_iovt_iommu *iommu)
>          if (np->flag & ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT)
>                  goto skip;
> 
> +       if (iovt_validate_entries(iommu) < 0)
> +               return -EINVAL;
> +
the logic about adding iovt_validate_entries() is OK, here kfree(np); 
should be added to prevent memory leak.

Regards
Bibo Mao
>          INIT_LIST_HEAD(&np->ep_list);
>          ep = ACPI_ADD_PTR(struct acpi_iovt_device_entry, iommu,
> iommu->device_entry_offset);
>          for (i = 0; i < iommu->device_entry_num; i++) {
>                  switch (ep->type) {
>                  case ACPI_IOVT_DEVICE_ENTRY_START:
> -                       is_start = true;
> -                       entry->start_devid = ep->device_id;
> -                       break;
> -               case ACPI_IOVT_DEVICE_ENTRY_END:
> -                       if (!is_start)
> -                               break;
> -
>                          entry = kzalloc_obj(struct iovt_device_entry,
> GFP_ATOMIC);
>                          if (!entry)
>                                  return -ENOMEM;
> 
> -                       is_start = false;
> +                       entry->start_devid = ep->device_id;
> +                       break;
> +               case ACPI_IOVT_DEVICE_ENTRY_END:
> +                       /* START & END are paired */
>                          entry->end_devid = ep->device_id;
>                          list_add_tail(&entry->list, &np->ep_list);
>                          break;
> @@ -135,13 +168,12 @@ static int __init iovt_add_iommu(struct
> acpi_iovt_iommu *iommu)
>                          if (!entry)
>                                  return -ENOMEM;
> 
> -                       is_start = false;
>                          entry->start_devid = ep->device_id;
>                          entry->end_devid = ep->device_id;
>                          list_add_tail(&entry->list, &np->ep_list);
>                          break;
>                  default:
> -                       break;
> +                       return -EINVAL;
>                  }
>                  ep = ACPI_ADD_PTR(struct acpi_iovt_device_entry, ep,
> ep->length);
>          }
> 
>>>
>>> Take START1/START2/END as an example of broken table: in your original
>>> code, the END entry will be pairing with START2. But for a broken
>>> table who can ensure they are the correct pair? If the real correct
>>> pair is START1 and END, then (entry->start_devid, entry->end_devid)
>> there is only one policy, software does not know START1/END is right or
>> START2/END is right.
>>> will be a wrong device range. Such mistakes are much worse than a
>>> memery leak.
>>>
>>> If you want to say "I don't care about correct pairing, memory leak is
>>> bad in any case.", OK, we can do it like this:
>>>
>>> diff --git a/drivers/acpi/loongarch/iovt.c b/drivers/acpi/loongarch/iovt.c
>>> index 87cee4f0fc80..ebd11e22c9be 100644
>>> --- a/drivers/acpi/loongarch/iovt.c
>>> +++ b/drivers/acpi/loongarch/iovt.c
>>> @@ -115,6 +115,13 @@ static int __init iovt_add_iommu(struct
>>> acpi_iovt_iommu *iommu)
>>>           for (i = 0; i < iommu->device_entry_num; i++) {
>>>                   switch (ep->type) {
>>>                   case ACPI_IOVT_DEVICE_ENTRY_START:
>>> +                       if (is_start)
>>> +                               break;
>> there is still memory leak with START/SINGLE/END pairs, is_start set
>> with false with ACPI_IOVT_DEVICE_ENTRY_SINGLE case.
>>
>> Regards
>> Bibo Mao
>>> +
>>> +                       entry = kzalloc_obj(struct iovt_device_entry,
>>> GFP_ATOMIC);
>>> +                       if (!entry)
>>> +                               return -ENOMEM;
>>> +
>>>                           is_start = true;
>>>                           entry->start_devid = ep->device_id;
>>>                           break;
>>> @@ -122,10 +129,6 @@ static int __init iovt_add_iommu(struct
>>> acpi_iovt_iommu *iommu)
>>>                           if (!is_start)
>>>                                   break;
>>>
>>> -                       entry = kzalloc_obj(struct iovt_device_entry,
>>> GFP_ATOMIC);
>>> -                       if (!entry)
>>> -                               return -ENOMEM;
>>> -
>>>                           is_start = false;
>>>                           entry->end_devid = ep->device_id;
>>>                           list_add_tail(&entry->list, &np->ep_list);
>>>
>>> Huacai
>>>
>>>>
>>>>>
>>>>> Huacai
>>>>>
>>>>>>
>>>>>> Regards
>>>>>> Bibo Mao
>>>>>>> +                       if (!entry)
>>>>>>> +                               return -ENOMEM;
>>>>>>> +
>>>>>>>                             is_start = true;
>>>>>>>                             entry->start_devid = ep->device_id;
>>>>>>>                             break;
>>>>>>> @@ -122,10 +126,6 @@ static int __init iovt_add_iommu(struct
>>>>>>> acpi_iovt_iommu *iommu)
>>>>>>>                             if (!is_start)
>>>>>>>                                     break;
>>>>>>>
>>>>>>> -                       entry = kzalloc_obj(struct iovt_device_entry,
>>>>>>> GFP_ATOMIC);
>>>>>>> -                       if (!entry)
>>>>>>> -                               return -ENOMEM;
>>>>>>> -
>>>>>>>                             is_start = false;
>>>>>>>                             entry->end_devid = ep->device_id;
>>>>>>>                             list_add_tail(&entry->list, &np->ep_list);
>>>>>>>
>>>>>>> Huacai
>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Huacai
>>>>>>>>>
>>>>>>>>> On Mon, Oct 12, 2026 at 3:16 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>>>>>
>>>>>>>>>> This change is already merged in acpica tree.
>>>>>>>>>>
>>>>>>>>>> slightly ping.
>>>>>>>>>>
>>>>>>>>>> On Tue, Sep 22, 2026 at 08:10:55PM +0800, Bibo Mao wrote:
>>>>>>>>>>       > According to the IOVT specification, define macros for each bit of the
>>>>>>>>>>       > Flags field in the IOVT IOMMU structure.
>>>>>>>>>>       >
>>>>>>>>>>       > Link: https://github.com/open-acpica/acpica/commit/352f750e4cc0
>>>>>>>>>>       > Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>>>>>>>>       > ---
>>>>>>>>>>       >  include/acpi/actbl2.h | 7 +++++++
>>>>>>>>>>       >  1 file changed, 7 insertions(+)
>>>>>>>>>>       >
>>>>>>>>>>       > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
>>>>>>>>>>       > index baef525367b5..76e4a5bc79b4 100644
>>>>>>>>>>       > --- a/include/acpi/actbl2.h
>>>>>>>>>>       > +++ b/include/acpi/actbl2.h
>>>>>>>>>>       > @@ -919,6 +919,13 @@ struct acpi_iovt_iommu {
>>>>>>>>>>       >       u32 device_entry_offset;
>>>>>>>>>>       >  };
>>>>>>>>>>       >
>>>>>>>>>>       > +/* Masks for Flags field above */
>>>>>>>>>>       > +#define ACPI_IOVT_IOMMU_PCI_DEVICE           (1<<0)
>>>>>>>>>>       > +#define ACPI_IOVT_IOMMU_PXM_VALID            (1<<1)
>>>>>>>>>>       > +#define ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT    (1<<2)
>>>>>>>>>>       > +#define ACPI_IOVT_IOMMU_HW_CAP_SUPPORT               (1<<3)
>>>>>>>>>>       > +#define ACPI_IOVT_IOMMU_MSI_INT_BYPASS               (1<<4)
>>>>>>>>>>       > +
>>>>>>>>>>       >  struct acpi_iovt_device_entry {
>>>>>>>>>>       >       u8 type;
>>>>>>>>>>       >       u8 length;
>>>>>>>>>>       > --
>>>>>>>>>>       > 2.39.3
>>>>>>>>>>       >
>>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>>>
>>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure
  2026-11-07 12:25                     ` Huacai Chen
  2026-11-09  1:12                       ` Bibo Mao
@ 2026-11-10  3:39                       ` Bibo Mao
  1 sibling, 0 replies; 22+ messages in thread
From: Bibo Mao @ 2026-11-10  3:39 UTC (permalink / raw)
  To: Huacai Chen
  Cc: rafael, linux-acpi, acpica-devel, lenb, linux-kernel, lixianglai,
	loongarch, maciej.wieczor-retman, pawel.chmielewski



On 2026/11/7 下午8:25, Huacai Chen wrote:
> On Sat, Nov 7, 2026 at 9:43 AM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2026/11/6 下午10:58, Huacai Chen wrote:
>>> On Thu, Nov 5, 2026 at 7:22 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>
>>>>
>>>>
>>>> On 2026/11/5 下午6:31, Huacai Chen wrote:
>>>>> On Thu, Nov 5, 2026 at 5:49 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 2026/11/5 下午5:31, Huacai Chen wrote:
>>>>>>> On Thu, Nov 5, 2026 at 5:14 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2026/10/23 下午3:59, Huacai Chen wrote:
>>>>>>>>> Bibo,
>>>>>>>>>
>>>>>>>>> Now I have applied this series but with some modifications, please
>>>>>>>>> double check and test the code.
>>>>>>>>>
>>>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/chenhuacai/linux-loongson.git/log/?h=loongarch-next
>>>>>>>> In my v4 submit, it is:
>>>>>>>> +               case ACPI_IOVT_DEVICE_ENTRY_START:
>>>>>>>> +                       is_start = true;
>>>>>>>> +                       start_devid = ep->device_id;
>>>>>>>> +                       break;
>>>>>>>> however in rc1, it is modified as:
>>>>>>>>                      case ACPI_IOVT_DEVICE_ENTRY_START:
>>>>>>>>                              is_start = true;
>>>>>>>>                              entry->start_devid = ep->device_id;
>>>>>>>>                              break;
>>>>>>>> Here entry is not allocated and NULL pointer, it is allocated in
>>>>>>>> ACPI_IOVT_DEVICE_ENTRY_END switch case.
>>>>>>>>
>>>>>>>> I really do not know what is aid to modify it and let others to test.
>>>>>>> Sorry, it's my fault.
>>>>>>>
>>>>>>> In theory I should point out the modification (eliminating the local
>>>>>>> variable start_devid) and refresh a new version. But we have no time
>>>>>>> to do this because when the first patch is merged into the ACPI repo
>>>>>>> it is near the merge window.
>>>>>> Maybe our work style is different, I do not care much for the coding style.
>>>>>>>
>>>>>>> If you agree, I will submit such modifications to fix it.
>>>>>> fix is welcome.
>>>>>>>
>>>>>>> diff --git a/drivers/acpi/loongarch/iovt.c b/drivers/acpi/loongarch/iovt.c
>>>>>>> index 87cee4f0fc80..4cdac28a1fd0 100644
>>>>>>> --- a/drivers/acpi/loongarch/iovt.c
>>>>>>> +++ b/drivers/acpi/loongarch/iovt.c
>>>>>>> @@ -115,6 +115,10 @@ static int __init iovt_add_iommu(struct
>>>>>>> acpi_iovt_iommu *iommu)
>>>>>>>             for (i = 0; i < iommu->device_entry_num; i++) {
>>>>>>>                     switch (ep->type) {
>>>>>>>                     case ACPI_IOVT_DEVICE_ENTRY_START:
>>>>>>> +                       entry = kzalloc_obj(struct iovt_device_entry,
>>>>>>> GFP_ATOMIC);
>>>>>> There will be memory leak problem if there are START/START/END in the table.
>>>>> That means a broken table? I think we don't need to consider broken
>>>>> tables, because there may be kinds of broken pattern rather than just
>>>>> a START/START/END.
>>>> yes, START/END/START, START/START/END, START/SINGLE/END. what is the
>>>> relationship between "don't need to consider" and kinds of broken pattern?
>>>>
>>>> what is the policy from your side with such BIOS table? let memory leak
>>>> and continue?
>>> The driver can only work with a good table or no table, but not with a
>>> broken table.
>> With a broken table, software can discard the whole table, or discard
>> the single table entry.
> Discard a single entry is dangerous because the pairing may be wrong,
> so if we should handle a broken table, we should discard the whole
> table.
> 
> Discarding the whole table need a rework, the is_start variable is not
> enough. You can consider the below solution:
> 
> diff --git a/drivers/acpi/loongarch/iovt.c b/drivers/acpi/loongarch/iovt.c
> index 87cee4f0fc80..8381de278e3a 100644
> --- a/drivers/acpi/loongarch/iovt.c
> +++ b/drivers/acpi/loongarch/iovt.c
> @@ -73,10 +73,44 @@ static int __init iovt_get_pci_iommu_fwnode(struct
> iovt_fwnode *np, u16 segment,
>          return 0;
>   }
> 
> +static int __init iovt_validate_entries(struct acpi_iovt_iommu *iommu)
> +{
> +       int i, done = 1;
> +       struct acpi_iovt_device_entry *ep;
> +
> +       ep = ACPI_ADD_PTR(struct acpi_iovt_device_entry, iommu,
> iommu->device_entry_offset);
> +       for (i = 0; i < iommu->device_entry_num; i++) {
> +               switch (ep->type) {
> +               case ACPI_IOVT_DEVICE_ENTRY_START:
> +                       if (!done)
> +                               return -EINVAL;
> +
> +                       done = 0;
> +                       break;
> +               case ACPI_IOVT_DEVICE_ENTRY_END:
> +                       if (done)
> +                               return -EINVAL;
> +
> +                       done = 1;
> +                       break;
> +               case ACPI_IOVT_DEVICE_ENTRY_SINGLE:
> +                       if (!done)
> +                               return -EINVAL;
> +
> +                       done = 1;
> +                       break;
> +               default:
one small bit about default case, which is reserved for future use. I 
prefer to skip this type rather than treat it as error,such as:
  +               case ACPI_IOVT_DEVICE_ENTRY_SINGLE:
  +               default:
  +                       if (!done)
  +                               return -EINVAL;
  +
  +                       done = 1;
  +                       break;

Because if iovt table spec is updated and new type is added, the old 
kernel will skip the new type and continue to run.

Regards
Bibo Mao
> +                       return -EINVAL;
> +               }
> +               ep = ACPI_ADD_PTR(struct acpi_iovt_device_entry, ep,
> ep->length);
> +       }
> +
> +       return done ? 0 : -EINVAL;
> +}
> +
>   static int __init iovt_add_iommu(struct acpi_iovt_iommu *iommu)
>   {
>          int i, ret;
> -       bool is_start = false;
>          struct iovt_fwnode *np;
>          struct iovt_device_entry *entry;
>          struct acpi_iovt_device_entry *ep;
> @@ -110,23 +144,22 @@ static int __init iovt_add_iommu(struct
> acpi_iovt_iommu *iommu)
>          if (np->flag & ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT)
>                  goto skip;
> 
> +       if (iovt_validate_entries(iommu) < 0)
> +               return -EINVAL;
> +
>          INIT_LIST_HEAD(&np->ep_list);
>          ep = ACPI_ADD_PTR(struct acpi_iovt_device_entry, iommu,
> iommu->device_entry_offset);
>          for (i = 0; i < iommu->device_entry_num; i++) {
>                  switch (ep->type) {
>                  case ACPI_IOVT_DEVICE_ENTRY_START:
> -                       is_start = true;
> -                       entry->start_devid = ep->device_id;
> -                       break;
> -               case ACPI_IOVT_DEVICE_ENTRY_END:
> -                       if (!is_start)
> -                               break;
> -
>                          entry = kzalloc_obj(struct iovt_device_entry,
> GFP_ATOMIC);
>                          if (!entry)
>                                  return -ENOMEM;
> 
> -                       is_start = false;
> +                       entry->start_devid = ep->device_id;
> +                       break;
> +               case ACPI_IOVT_DEVICE_ENTRY_END:
> +                       /* START & END are paired */
>                          entry->end_devid = ep->device_id;
>                          list_add_tail(&entry->list, &np->ep_list);
>                          break;
> @@ -135,13 +168,12 @@ static int __init iovt_add_iommu(struct
> acpi_iovt_iommu *iommu)
>                          if (!entry)
>                                  return -ENOMEM;
> 
> -                       is_start = false;
>                          entry->start_devid = ep->device_id;
>                          entry->end_devid = ep->device_id;
>                          list_add_tail(&entry->list, &np->ep_list);
>                          break;
>                  default:
> -                       break;
> +                       return -EINVAL;
>                  }
>                  ep = ACPI_ADD_PTR(struct acpi_iovt_device_entry, ep,
> ep->length);
>          }
> 
>>>
>>> Take START1/START2/END as an example of broken table: in your original
>>> code, the END entry will be pairing with START2. But for a broken
>>> table who can ensure they are the correct pair? If the real correct
>>> pair is START1 and END, then (entry->start_devid, entry->end_devid)
>> there is only one policy, software does not know START1/END is right or
>> START2/END is right.
>>> will be a wrong device range. Such mistakes are much worse than a
>>> memery leak.
>>>
>>> If you want to say "I don't care about correct pairing, memory leak is
>>> bad in any case.", OK, we can do it like this:
>>>
>>> diff --git a/drivers/acpi/loongarch/iovt.c b/drivers/acpi/loongarch/iovt.c
>>> index 87cee4f0fc80..ebd11e22c9be 100644
>>> --- a/drivers/acpi/loongarch/iovt.c
>>> +++ b/drivers/acpi/loongarch/iovt.c
>>> @@ -115,6 +115,13 @@ static int __init iovt_add_iommu(struct
>>> acpi_iovt_iommu *iommu)
>>>           for (i = 0; i < iommu->device_entry_num; i++) {
>>>                   switch (ep->type) {
>>>                   case ACPI_IOVT_DEVICE_ENTRY_START:
>>> +                       if (is_start)
>>> +                               break;
>> there is still memory leak with START/SINGLE/END pairs, is_start set
>> with false with ACPI_IOVT_DEVICE_ENTRY_SINGLE case.
>>
>> Regards
>> Bibo Mao
>>> +
>>> +                       entry = kzalloc_obj(struct iovt_device_entry,
>>> GFP_ATOMIC);
>>> +                       if (!entry)
>>> +                               return -ENOMEM;
>>> +
>>>                           is_start = true;
>>>                           entry->start_devid = ep->device_id;
>>>                           break;
>>> @@ -122,10 +129,6 @@ static int __init iovt_add_iommu(struct
>>> acpi_iovt_iommu *iommu)
>>>                           if (!is_start)
>>>                                   break;
>>>
>>> -                       entry = kzalloc_obj(struct iovt_device_entry,
>>> GFP_ATOMIC);
>>> -                       if (!entry)
>>> -                               return -ENOMEM;
>>> -
>>>                           is_start = false;
>>>                           entry->end_devid = ep->device_id;
>>>                           list_add_tail(&entry->list, &np->ep_list);
>>>
>>> Huacai
>>>
>>>>
>>>>>
>>>>> Huacai
>>>>>
>>>>>>
>>>>>> Regards
>>>>>> Bibo Mao
>>>>>>> +                       if (!entry)
>>>>>>> +                               return -ENOMEM;
>>>>>>> +
>>>>>>>                             is_start = true;
>>>>>>>                             entry->start_devid = ep->device_id;
>>>>>>>                             break;
>>>>>>> @@ -122,10 +126,6 @@ static int __init iovt_add_iommu(struct
>>>>>>> acpi_iovt_iommu *iommu)
>>>>>>>                             if (!is_start)
>>>>>>>                                     break;
>>>>>>>
>>>>>>> -                       entry = kzalloc_obj(struct iovt_device_entry,
>>>>>>> GFP_ATOMIC);
>>>>>>> -                       if (!entry)
>>>>>>> -                               return -ENOMEM;
>>>>>>> -
>>>>>>>                             is_start = false;
>>>>>>>                             entry->end_devid = ep->device_id;
>>>>>>>                             list_add_tail(&entry->list, &np->ep_list);
>>>>>>>
>>>>>>> Huacai
>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Huacai
>>>>>>>>>
>>>>>>>>> On Mon, Oct 12, 2026 at 3:16 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>>>>>>>
>>>>>>>>>> This change is already merged in acpica tree.
>>>>>>>>>>
>>>>>>>>>> slightly ping.
>>>>>>>>>>
>>>>>>>>>> On Tue, Sep 22, 2026 at 08:10:55PM +0800, Bibo Mao wrote:
>>>>>>>>>>       > According to the IOVT specification, define macros for each bit of the
>>>>>>>>>>       > Flags field in the IOVT IOMMU structure.
>>>>>>>>>>       >
>>>>>>>>>>       > Link: https://github.com/open-acpica/acpica/commit/352f750e4cc0
>>>>>>>>>>       > Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>>>>>>>>       > ---
>>>>>>>>>>       >  include/acpi/actbl2.h | 7 +++++++
>>>>>>>>>>       >  1 file changed, 7 insertions(+)
>>>>>>>>>>       >
>>>>>>>>>>       > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
>>>>>>>>>>       > index baef525367b5..76e4a5bc79b4 100644
>>>>>>>>>>       > --- a/include/acpi/actbl2.h
>>>>>>>>>>       > +++ b/include/acpi/actbl2.h
>>>>>>>>>>       > @@ -919,6 +919,13 @@ struct acpi_iovt_iommu {
>>>>>>>>>>       >       u32 device_entry_offset;
>>>>>>>>>>       >  };
>>>>>>>>>>       >
>>>>>>>>>>       > +/* Masks for Flags field above */
>>>>>>>>>>       > +#define ACPI_IOVT_IOMMU_PCI_DEVICE           (1<<0)
>>>>>>>>>>       > +#define ACPI_IOVT_IOMMU_PXM_VALID            (1<<1)
>>>>>>>>>>       > +#define ACPI_IOVT_IOMMU_MAGAGE_BY_SEGMENT    (1<<2)
>>>>>>>>>>       > +#define ACPI_IOVT_IOMMU_HW_CAP_SUPPORT               (1<<3)
>>>>>>>>>>       > +#define ACPI_IOVT_IOMMU_MSI_INT_BYPASS               (1<<4)
>>>>>>>>>>       > +
>>>>>>>>>>       >  struct acpi_iovt_device_entry {
>>>>>>>>>>       >       u8 type;
>>>>>>>>>>       >       u8 length;
>>>>>>>>>>       > --
>>>>>>>>>>       > 2.39.3
>>>>>>>>>>       >
>>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>>>
>>


^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2026-11-10  3:38 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-22 12:10 [PATCH v4 0/4] ACPI: LoongArch: Add IOVT table support Bibo Mao
2026-09-22 12:10 ` [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure Bibo Mao
2026-10-12  7:18   ` Bibo Mao
2026-10-12  7:20     ` Huacai Chen
2026-10-12 14:47       ` Rafael J. Wysocki (Intel)
2026-10-13  8:39         ` Huacai Chen
2026-10-13 12:00           ` Rafael J. Wysocki (Intel)
2026-10-23  7:59     ` Huacai Chen
2026-11-04  8:30       ` Bibo Mao
2026-11-05  9:15       ` Bibo Mao
2026-11-05  9:31         ` Huacai Chen
2026-11-05  9:50           ` Bibo Mao
2026-11-05 10:31             ` Huacai Chen
2026-11-05 11:23               ` Bibo Mao
2026-11-06 14:58                 ` Huacai Chen
2026-11-07  1:45                   ` Bibo Mao
2026-11-07 12:25                     ` Huacai Chen
2026-11-09  1:12                       ` Bibo Mao
2026-11-10  3:39                       ` Bibo Mao
2026-09-22 12:10 ` [PATCH v4 2/4] ACPI: LoongArch: Add acpi_arch_[late]_init() Bibo Mao
2026-09-22 12:10 ` [PATCH v4 3/4] ACPI: scan: Add support for LoongArch in acpi_iommu_configure_id() Bibo Mao
2026-09-22 12:10 ` [PATCH v4 4/4] MAINTAINERS: Add entry for drivers/acpi/loongarch Bibo Mao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®