[Xen-ia64-devel] RE: Rebased xen-ia64 tree with VT-i support

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

Hi, Dan,
        Thanks for feedback, and see comments below.

Thanks,
Kevin
>> >Patch files:
>> >
>> >1) move vmx_ia64_handle_irq out of irq_ia64.c and
>> >   into xenirq.c
>>
>> This is just to sync with ia64_handle_irq, which still exists
>> in irq_ia64.c. If you move xen specific version like
>> xen_handle_irq, then its easy for us to follow that
>> direction, since they same type should be in same place
>
>Maybe I misunderstand, but xen_handle_irq was recently
>moved to xenirq.c.  I was suggesting that vmx_ia64_handle_irq
>move there also.

Maybe I lose sync with bk tree? I "bk pull" and still see
ia64_handle_irq is the entry point for interrupt handler, which is in
irq_ia64.c. What's new in xenirq.c is some xen specific guest irq handle
(xen_do_irq) called by ia64_handle_irq. So for CONFIG_VTI, our interrupt
handler is vmx_ia64_handle_irq, which is in same level as
ia64_handle_irq, not as xen_do_irq. Hope it clear now. :)

>
>> I'm concern whether it's worthy of time to create so many new
>> files just because we have some new generic definitions.
>> Ideally, I believe finally XEN/IPF will go to flatten after
>> removing about 99% unused Linux stuff. At that time, saying
>> kregs.h, it becomes xen's kregs.h and free to add new stuff
>> without "#ifdef XEN" any more. Why bother to waste time to
>> add xen_kregs.h now and then remove and merge them again
>> later? Either one patch file and one new file, you need to
>> add one. Especially such definition seldom changes and should
>> go where it should. It is different as new xentime.c which
>> differs on logic. :)
>
>For header patches that require only a few lines of change
>and are expected to rarely or never change, I agree it is
>a waste of time to create new files.  

Agree.

>For header patches with
>dozens or hundreds of lines changed or lines that are expected
>to change somewhat frequently, the "sub-include" may be
>better.  This will make it more clear what new code
>is added to support Xen... and also reduce complaints from Arun
>about the extra step required to checkin changes to patched
>files ;-)

80% agree based on current model, if you're sure this patched approach
will last for a long time. :)

>
>Let's settle on splitting out the code from processor.h
>and system.h into separate xen_processor.h and xen_system.h
>(and perhaps gcc_intrin.h unless you're sure this is stable).

OK, I can start to make that change. gcc_inirin.h should be very stable.


But as a note, the patch to system.h and processor.h will still exist
which may simply remove original definition and include "xen_***"
specific header to contain new definition. The reason is the difficulty
to handle multiple redefinitions without any change to original file.
But that patch will be stable, and later changes only happen in
"xen_***" as you expected.

>
>If your estimate of 99% is anywhere close to accurate,
>flattening would be best.  I think it is probably closer
>to 10-20% and flattening sacrifices long-term maintainablity
>and functionality leverage for short-term developer
>convenience.

You should have more accurate data about the number. :) Actually I
didn't want to influence current model for short-term developer, with
just belief that at some point in the future, complete code clean has to
be made to split xen from Linux pool completely for better
maintainability, performance and development base. We can just take
Linux as a big library for many useful and existing features (Steal word
from Arun :), but not as functional and performance model for a VMM...

Thanks,
Kevin

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