>>> #include "irq_remapping.h" >>> >>> +typedef int (*dmar_res_handler_t)(struct acpi_dmar_header *, void *); >>> +struct dmar_res_callback { >>> + dmar_res_handler_t cb[ACPI_DMAR_TYPE_RESERVED]; >>> + void *arg[ACPI_DMAR_TYPE_RESERVED]; >>> + bool ignore_unhandled; >>> + bool print_entry; >> >> Why do we need a switch to control print ? > We will walk DMAR entries several times during hotplug and only > want to print once. Fine, thanks for your explanation. > >> >>> +}; >>> + >>> >>> +static int dmar_walk_resources(struct acpi_dmar_header *start, size_t len, >>> + struct dmar_res_callback *cb) >> >> The name dmar_walk_resources easily make people think this is related with I/O or memory resources. >> Do you have a better idea of this ? What about dmar_walk_remapping_entry() or dmar_walk_remapping_structure() ? > Good suggestion, I like dmar_walk_remapping_entries(). >> >>> +{ >>> + int ret = 0; >>> + struct acpi_dmar_header *iter, *next; >>> + struct acpi_dmar_header *end = ((void *)start) + len; >>> + >>> + for (iter = start; iter < end && ret == 0; iter = next) { >>> + next = (void *)iter + iter->length; >>> + if (iter->length == 0) { >>> + /* Avoid looping forever on bad ACPI tables */ >>> + pr_debug(FW_BUG "Invalid 0-length structure\n"); >> >> What about use pr_warn() instead of pr_debug(), pr_debug() default is off. > It seems a common practice for BIOS engineer to mark the last entry with > zero length. So it will be annoying if we generate this debug message > on product kernel. OK. > >> >>> + break; >>> + } else if (next > end) { >>> + /* Avoid passing table end */ >>> + pr_warn(FW_BUG "record passes table end\n"); >>> + ret = -EINVAL; >>> + break; >>> + } >>> + >>> + if (cb->print_entry) >>> + dmar_table_print_dmar_entry(iter); >>> + >>> + if (iter->type >= ACPI_DMAR_TYPE_RESERVED) { >>> + /* continue for forward compatibility */ >>> + pr_debug("Unknown DMAR structure type %d\n", >>> + iter->type); >> >> Same as above. > This is typically caused by new DMAR specification. It will also be > annoying too if we also generate this debug message on an newer > hardware platform with older linux kernel. OK. > >> >>> + } else if (cb->cb[iter->type]) { >>> + ret = cb->cb[iter->type](iter, cb->arg[iter->type]); >>> + } else if (!cb->ignore_unhandled) { >>> + pr_warn("No handler for DMAR structure type %d\n", -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html