linux-kernel - Re: [PATCH 1/3] iommu/core: add fault reporting mechanism

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: <20110912160252.GE11701@amd.com>
Date:	Mon, 12 Sep 2011 18:02:52 +0200
From:	"Roedel, Joerg" <Joerg.Roedel@....com>
To:	Ohad Ben-Cohen <ohad@...ery.com>
CC:	"iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
	"linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>,
	Hiroshi DOYU <Hiroshi.DOYU@...ia.com>,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	David Woodhouse <dwmw2@...radead.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	David Brown <davidb@...eaurora.org>,
	Arnd Bergmann <arnd@...db.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] iommu/core: add fault reporting mechanism

On Wed, Sep 07, 2011 at 02:53:22PM -0400, Ohad Ben-Cohen wrote:
>  struct device;
> +struct iommu_domain;
> +
> +/**
> + * enum iommu_fault_types - iommu fault types
> + *
> + * @IOMMU_ERROR: Unrecoverable error
> + *
> + * Currently we only support a generic error fault type.
> + * Future users, which will require more informative fault types, will add
> + * them as needed.
> + */
> +enum iommu_fault_types {
> +	IOMMU_ERROR,
> +};

I still don't get the need for this. It would make sense to encode
different types of faults, like page-faults or interrupt-faults. That is
what I read from the name of the enum.
When I read the comment above it sounds more like you want to encode
different error-levels, like recoverable and unrecoverable error.
The exact meaning of these values need to be clarified.

> +/**
> + * report_iommu_fault() - report about an IOMMU fault to the IOMMU framework
> + * @domain: the iommu domain where the fault has happened
> + * @dev: the device where the fault has happened
> + * @iova: the faulting address
> + * @flags: mmu fault flags (e.g. IOMMU_FAULT_READ/IOMMU_FAULT_WRITE/...)
> + * @event: the mmu fault type

Please place 'event' before iova when you keep it, and not at the end.
Then you have 'where' and 'what' of the fault first before the details
(iova, flags).

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

--
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/