linux-kernel - Re: [PATCH 1/3] iommu/amd: Add logic to decode AMD IOMMU event flag

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <515AED9E.80204@amd.com>
Date:	Tue, 2 Apr 2013 09:39:26 -0500
From:	Suravee Suthikulanit <suravee.suthikulpanit@....com>
To:	Joerg Roedel <joro@...tes.org>
CC:	<iommu@...ts.linux-foundation.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] iommu/amd: Add logic to decode AMD IOMMU event flag

On 4/2/2013 9:33 AM, Joerg Roedel wrote:
> Hi Suravee,
>
> On Wed, Mar 27, 2013 at 06:51:23PM -0500, suravee.suthikulpanit@....com wrote:
>> From: Suravee Suthikulpanit <suravee.suthikulpanit@....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
>
>
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/