linux-kernel - Re: [PATCH V4 0/4] Code refine for Intel IOMMU

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] [day] [month] [year] [list]

Message-ID: <20160509222059.GA21601@vultr.guest>
Date:	Mon, 9 May 2016 22:20:59 +0000
From:	Wei Yang <richard.weiyang@...il.com>
To:	Joerg Roedel <joro@...tes.org>
Cc:	Wei Yang <richard.weiyang@...il.com>, jiang.liu@...ux.intel.com,
	tglx@...utronix.de, iommu@...ts.linux-foundation.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH V4 0/4] Code refine for Intel IOMMU

On Mon, May 09, 2016 at 01:24:02PM +0200, Joerg Roedel wrote:
>On Sun, May 08, 2016 at 01:22:53PM +0000, Wei Yang wrote:
>> >Wei Yang (4):
>> >  iommu/vt-d: replace *hdr with {drhd/atsr}[0] in struct
>> >    dmar_{drhd/atsr}_unit
>> >  iommu/vt-d: use zero-sized array in DMAR related ACPI structures
>> >  iommu/vt-d: check Register Base Address at the beginning of
>> >    dmar_parse_one_drhd()
>> >  iommu/vt-d: refine dmar_acpi_dev_scope_init() with
>> >    dmar_walk_dmar_table()
>
>Okay, I've ignored this so far as I don't see where you are going with
>this refactoring. The patches as-is don't make much sense to me other
>than creating conflicts with other vt-d driver changes.
>

Hi, Joerg

Thanks for your comments. 

Generally, the purpose of these patches is to make the code more audience
friendly. Below is my thoughts for each patch.

For Patch 1, the zero-sized drhd/atsr would save some space in
dmar_{drhd/atsr}_unit. As in the change log explained, before commit
<6b1972493a84> ("iommu/vt-d: Implement DMAR unit hotplug framework"), it is
necessary to have a pointer. While after this commit, we just need a place
holder.

For Patch 2, I add a zero-sized array at the end of related structures to
represent variable length elements. By doing so, our code could use this field
to be a parameter for a function instead of casting and add 1. I think this
would let audience understand the variable elements is used here instead of go
through SPEC to check which part it means, which is not that reader friendly.

For Patch 3, when you go through the code, Register Base Address will be
checked to see whether this is a valid dmar. Since this is what we have to do,
I move this step a little bit ahead, so that we can avoid related setups at
the first place.

For Patch 4, dmar_acpi_dev_scope_init() initialize dev_scope by iterate on the
remapping structure. We can see this is just what dmar_walk_dmar_table() does.
So I reuse the code in this place.

In my mind, those changes are dmar internal changes, which will not be seen
outside. Would you mind pointing me the conflicts you saw? Maybe I missed some
thing :-)

Thanks, have a good day.

>
>
>	Joerg

-- 
Wei Yang
Help you, Help me