Re: [PATCH v4 1/4] ACPICA: actbl2.h: Add bit constants of the Flags field in the IOVT IOMMU structure
From: Bibo Mao
Date: Fri Nov 06 2026 - 20:43:59 EST
On 2026/11/6 下午10:58, Huacai Chen wrote:
On Thu, Nov 5, 2026 at 7:22 PM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:With a broken table, software can discard the whole table, or discard the single table entry.
The driver can only work with a good table or no table, but not with a
On 2026/11/5 下午6:31, Huacai Chen wrote:
On Thu, Nov 5, 2026 at 5:49 PM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:yes, START/END/START, START/START/END, START/SINGLE/END. what is the
That means a broken table? I think we don't need to consider broken
On 2026/11/5 下午5:31, Huacai Chen wrote:
On Thu, Nov 5, 2026 at 5:14 PM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:Maybe our work style is different, I do not care much for the coding style.
Sorry, it's my fault.
On 2026/10/23 下午3:59, Huacai Chen wrote:
Bibo,In my v4 submit, it is:
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
+ 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.
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.
fix is welcome.
If you agree, I will submit such modifications to fix it.
There will be memory leak problem if there are START/START/END in the table.
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);
tables, because there may be kinds of broken pattern rather than just
a START/START/END.
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?
broken table.
there is only one policy, software does not know START1/END is right or START2/END is right.
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 athere is still memory leak with START/SINGLE/END pairs, is_start set with false with ACPI_IOVT_DEVICE_ENTRY_SINGLE case.
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;
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@xxxxxxxxxxx> 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@xxxxxxxxxxx>
> ---
> 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
>