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

2016-05-09 Thread Wei Yang
On Mon, May 09, 2016 at 01:24:02PM +0200, Joerg Roedel wrote:
>On Sun, May 08, 2016 at 01:22:53PM +, 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
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


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

2016-05-09 Thread Joerg Roedel
On Sun, May 08, 2016 at 01:22:53PM +, 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

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


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

2016-05-08 Thread Wei Yang
Ping~

On Thu, Apr 14, 2016 at 02:55:06PM +, 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
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


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

2016-04-14 Thread Wei Yang
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

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu