Xen devel - Sep 2008 - [PATCH] Share the IO_APIC_route_entry with iosapic

The patch moves the struct IO_APIC_route_entry to a common place.
This allows us to share the struct with iosapic.

Thanks,
-- Dexuan





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

If the code using this definition will be shared too, that''s one thing.
If
you''re actually going to put all the code under arch/ia64 (which you
seem to
imply), then duplicating this structure in include/asm-ia64 is okay.
It''s
not like it''s changing, ever. I personally think that striving to share
architectural definitions in all cases, even where that may twist the header
#includes, is not necessarily a good idea. So it depends what your end goal
is.

 -- Keir

On 28/9/08 07:52, "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
> The patch moves the struct IO_APIC_route_entry to a common place.
> This allows us to share the struct with iosapic.
> 
> Thanks,
> -- Dexuan
> 
> 
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi Keir,
The iosapic continues to stay inside arch/ia64/; on the other hand, I think the
struct 'IO_APIC_route_entry' should be placed to a common place so that
IPF and x86 can share most of the common codes, like interrupt remapping.
To make the most use most use of the current x86 VT-d code, I personally think
this movement of thedefinition of the struct IO_APIC_route_entry is necessary
here.
Could you please comment this more?

Thanks,
-- Dexuan


-----Original Message-----
From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
Sent: 2008年9月28日 17:36
To: Cui, Dexuan; 'xen-devel@lists.xensource.com'
Subject: Re: [PATCH] Share the IO_APIC_route_entry with iosapic

If the code using this definition will be shared too, that's one thing. If
you're actually going to put all the code under arch/ia64 (which you seem to
imply), then duplicating this structure in include/asm-ia64 is okay. It's
not like it's changing, ever. I personally think that striving to share
architectural definitions in all cases, even where that may twist the header
#includes, is not necessarily a good idea. So it depends what your end goal
is.

 -- Keir

On 28/9/08 07:52, "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
> The patch moves the struct IO_APIC_route_entry to a common place.
> This allows us to share the struct with iosapic.
>
> Thanks,
> -- Dexuan
>
>
>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 7/10/08 08:24, "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
> Hi Keir,
> The iosapic continues to stay inside arch/ia64/; on the other hand, I think
> the struct ''IO_APIC_route_entry'' should be placed to a
common place so that
> IPF and x86 can share most of the common codes, like interrupt remapping.
> To make the most use most use of the current x86 VT-d code, I personally
think
> this movement of thedefinition of the struct IO_APIC_route_entry is
necessary
> here.
> Could you please comment this more?
Okay. Please move all arch-neutral definitions to xen/io_apic.h though --
that will at a minimum be all IO_APIC_reg_xx structures.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

The IO_APIC_reg_xx structures are not used in IPF side.
Let me double check if we also need to move them.

Thanks,
-- Dexuan


-----Original Message-----
From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
Sent: 2008年10月7日 15:34
To: Cui, Dexuan; 'xen-devel@lists.xensource.com'; Xu, Anthony
Subject: Re: [PATCH] Share the IO_APIC_route_entry with iosapic

On 7/10/08 08:24, "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
> Hi Keir,
> The iosapic continues to stay inside arch/ia64/; on the other hand, I think
> the struct 'IO_APIC_route_entry' should be placed to a common place
so that
> IPF and x86 can share most of the common codes, like interrupt remapping.
> To make the most use of the current x86 VT-d codes, I personally think
> this movement of the definition of the struct IO_APIC_route_entry is
necessary
> here.
> Could you please comment this more?
Okay. Please move all arch-neutral definitions to xen/io_apic.h though --
that will at a minimum be all IO_APIC_reg_xx structures.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

If you''re making an arch-neutral io-apic header, you should move them.
If
you just want a convenient stash for vt-d struct definitions, duplicate the
route_entry structure in a header under drivers/passthrough.

 -- Keir

On 7/10/08 08:49, "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
> The IO_APIC_reg_xx structures are not used in IPF side.
> Let me double check if we also need to move them.
> 
> Thanks,
> -- Dexuan
> 
> 
> -----Original Message-----
> From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
> Sent: 2008年10月7日 15:34
> To: Cui, Dexuan; ''xen-devel@lists.xensource.com''; Xu,
Anthony
> Subject: Re: [PATCH] Share the IO_APIC_route_entry with iosapic
> 
> On 7/10/08 08:24, "Cui, Dexuan" <dexuan.cui@intel.com>
wrote:
> 
>> Hi Keir,
>> The iosapic continues to stay inside arch/ia64/; on the other hand, I
think
>> the struct ''IO_APIC_route_entry'' should be placed to
a common place so that
>> IPF and x86 can share most of the common codes, like interrupt
remapping.
>> To make the most use of the current x86 VT-d codes, I personally think
>> this movement of the definition of the struct IO_APIC_route_entry is
>> necessary
>> here.
>> Could you please comment this more?
> 
> Okay. Please move all arch-neutral definitions to xen/io_apic.h though --
> that will at a minimum be all IO_APIC_reg_xx structures.
> 
>  -- Keir
> 
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi Keir,
I think the idea "making an arch-neutral io-apic header" may need too
many lines of code.

I tried the idea "duplicating the IO_APIC_route_entry structure in a header
under drivers/passthrough" and find it's not ok:
xen/drivers/passthrough/vtd/intremap.c includes xen/sched.h which has the
IO_APIC_route_entry defined in include/asm-x86/io_apic.h.
To use the newly-duplicated definition in intremap.c, I have to remove the
inclusion of xen/sched.h and I find doing this causes many header files
compilation issue which seems not easy to resolve.
Another drawback of this idea is: we have 2 different definitions for the same
struct name -- this can cause great confusion, I think.

A better idea may be:
we duplicate the IO_APIC_route_entry structure in a header under
drivers/passthrough/ and rename it to IO_xAPIC_route_entry -- this new name will
only be used by files in drivers/passthrough/.
This requires the least changes and avoids confusion.
How do you like this?

Thanks,
-- Dexuan


-----Original Message-----
From: xen-devel-bounces@lists.xensource.com
[mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Keir Fraser
Sent: 2008年10月7日 16:07
To: Cui, Dexuan; 'xen-devel@lists.xensource.com'; Xu, Anthony
Subject: [Xen-devel] Re: [PATCH] Share the IO_APIC_route_entry with iosapic

If you're making an arch-neutral io-apic header, you should move them. If
you just want a convenient stash for vt-d struct definitions, duplicate the
route_entry structure in a header under drivers/passthrough.

 -- Keir

On 7/10/08 08:49, "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
> The IO_APIC_reg_xx structures are not used in IPF side.
> Let me double check if we also need to move them.
>
> Thanks,
> -- Dexuan
>
>
> -----Original Message-----
> From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
> Sent: 2008年10月7日 15:34
> To: Cui, Dexuan; 'xen-devel@lists.xensource.com'; Xu, Anthony
> Subject: Re: [PATCH] Share the IO_APIC_route_entry with iosapic
>
> On 7/10/08 08:24, "Cui, Dexuan" <dexuan.cui@intel.com>
wrote:
>
>> Hi Keir,
>> The iosapic continues to stay inside arch/ia64/; on the other hand, I
think
>> the struct 'IO_APIC_route_entry' should be placed to a common
place so that
>> IPF and x86 can share most of the common codes, like interrupt
remapping.
>> To make the most use of the current x86 VT-d codes, I personally think
>> this movement of the definition of the struct IO_APIC_route_entry is
>> necessary
>> here.
>> Could you please comment this more?
>
> Okay. Please move all arch-neutral definitions to xen/io_apic.h though --
> that will at a minimum be all IO_APIC_reg_xx structures.
>
>  -- Keir
>
>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 9/10/08 09:27, "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
> A better idea may be:
> we duplicate the IO_APIC_route_entry structure in a header under
> drivers/passthrough/ and rename it to IO_xAPIC_route_entry -- this new name
> will only be used by files in drivers/passthrough/.
> This requires the least changes and avoids confusion.
> How do you like this?
Sounds okay.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel