On 4/2/2013 9:33 AM, Joerg Roedel wrote:
Hi Suravee,
On Wed, Mar 27, 2013 at 06:51:23PM -0500, suravee.suthikulpa...@amd.com wrote:
From: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
Add logic to decode AMD IOMMU event flag based on information from AMD IOMMU
specification.
This should simplify debugging IOMMU errors. Also, dump DTE information in
additional cases.
The patch in general makes sense to have, but I have a couple of
comments.
+static void dump_flags(int flags, int ev_type)
+{
+ struct _event_log_flags *p = (struct _event_log_flags *) &flags;
+ u32 err_type = p->type;
+
+ pr_err("AMD-Vi: Flags details:\n");
+ pr_err("AMD-Vi: Guest / Nested : %u\n", p->gn);
+ pr_err("AMD-Vi: No Execute : %u\n", p->nx);
+ pr_err("AMD-Vi: User-Supervisor : %u\n", p->us);
+ pr_err("AMD-Vi: Interrupt : %u\n", p->i);
+ pr_err("AMD-Vi: Present : %u\n", p->pr);
+ pr_err("AMD-Vi: Read / Write : %u\n", p->rw);
+ pr_err("AMD-Vi: Permission : %u\n", p->pe);
+ pr_err("AMD-Vi: Reserv bit not zero / Illegal level encoding : %u\n",
+ p->rz);
+ pr_err("AMD-Vi: Translation / Transaction : %u\n",
+ p->tr);
+ pr_err("AMD-Vi: Type of error : (0x%x) ", err_type);
Printing the flags multi-line is much too noisy for IOMMU events. Just
print a char-shortcut for each flag set on the same line.
I will make the changes and send in for new patch for review.
+
+ if ((ev_type == EVENT_TYPE_DEV_TAB_ERR) ||
+ (ev_type == EVENT_TYPE_PAGE_TAB_ERR) ||
+ (ev_type == EVENT_TYPE_CMD_HARD_ERR)) {
+ /* Only supports up to 2 bits */
+ err_type &= 0x3;
+ switch (err_type) {
+ case 0:
+ pr_err("Reserved\n");
+ break;
+ case 1:
+ pr_err("Master Abort\n");
+ break;
+ case 2:
+ pr_err("Target Abort\n");
+ break;
+ case 3:
+ pr_err("Data Error\n");
+ break;
+ }
Why do you create string-arrays for other type-encodings but not for
this one?
I can do the same way if that's preferred.
Thanks,
Suravee
+ } else if (ev_type == EVENT_TYPE_INV_DEV_REQ) {
+ if (p->tr == 0) {
+ if (err_type < ARRAY_SIZE(_invalid_translation_desc))
+ printk("%s\n",
+ _invalid_translation_desc[err_type]);
+ } else {
+ if (err_type < ARRAY_SIZE(_invalid_transaction_desc))
+ printk("%s\n",
+ _invalid_transaction_desc[err_type]);
pr_cont instead of printk.
Joerg
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
