Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1396435 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V4 0/4] Code refine for Intel IOMMU Wei Yang <richard.weiyang@gmail.com> - 2016-05-08 15:30 +0200
    Re: [PATCH V4 0/4] Code refine for Intel IOMMU Joerg Roedel <joro@8bytes.org> - 2016-05-09 13:30 +0200
      Re: [PATCH V4 0/4] Code refine for Intel IOMMU Wei Yang <richard.weiyang@gmail.com> - 2016-05-10 00:30 +0200

#1396435 — Re: [PATCH V4 0/4] Code refine for Intel IOMMU

Ping~

On Thu, Apr 14, 2016 at 02:55:06PM +0000, Wei Yang wrote:
>These four patches try to refine the Intel IOMMU.                                                                                     
>                                                                                                                                      
>Patch 1/2 tries to make it more user friendly by add a zero-sized array in                                                            
>some dmar data structure.                                                                                                             
>Patch 3 move the ckeck of Register Base Address ahead to avoid cleanup when it                                                        
>is NULL.                                                                                                                              
>Patch 4 re-use dmar_walk_dmar_table() to make the code easy to understand.
>
>V4:
>  * add similar change for struct dmar_atsr_unit in patch 1
>  * add similar change for rmrr and atsr in patch 2
>
>V3:
>  * change hdr to drhd from type acpi_dmar_header to acpi_dmar_hardware_unit 
>  * add reason in changelog for the change in Patch 1
>
>V2:
>  * add patch 3 and 4
>
>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()
>
> drivers/iommu/dmar.c                |  129 +++++++++++++++++------------------
> drivers/iommu/intel-iommu.c         |   26 +++----
> drivers/iommu/intel_irq_remapping.c |   10 ++-
> include/acpi/actbl2.h               |   33 +++++----
> include/linux/dmar.h                |    3 +-
> 5 files changed, 102 insertions(+), 99 deletions(-)
>
>-- 
>1.7.9.5

-- 
Wei Yang
Help you, Help me

[toc] | [next] | [standalone]

#1396949

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.



	Joerg

[toc] | [prev] | [next] | [standalone]

#1397469

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web