[Xen-devel] RE: [PATCH][1/2] pvops-dom0: Xen acpi processor logic cleanup

["Yu, Ke" <ke.yu@xxxxxxxxx>]

>-----Original Message-----
>From: Jeremy Fitzhardinge [mailto:jeremy@xxxxxxxx]
>Sent: Thursday, November 26, 2009 4:45 PM
>To: Yu, Ke
>Cc: 'xen-devel@xxxxxxxxxxxxxxxxxxx'; Brown, Len
>Subject: Re: [PATCH][1/2] pvops-dom0: Xen acpi processor logic cleanup
>
>On 11/26/09 00:29, Yu, Ke wrote:
>>>    * Split this into 3 patches:
>>>          o revert the old Xen-specific changes
>>>          o add the new common code (which may just be
>>>            is_processor_apic_enabled())
>>>          o add the new Xen-specific code in a new file
>>>
>> Thanks for the comments. I split the original patch into three patch as you
>suggested. However, after it is done, I notice the original patch has already 
>been
>checked in. so I just attach the three patch for your information, in case you 
>still
>need that.
>>
>
>Yes, that's why I'd like to see the revert patch separate.  Later on we
>can fold together the patches to clean them up, but for now lots of
>incremental delta patches is best.   And if we can avoid mixing Xen and
>non-Xen changes into the one file, then its easier for subsystem
>maintainers to see what we're doing to their code without getting stuck
>in the details of the Xen-specific parts.

Yes, this is more easy for maintenance. Will follow this way in later patches.

>
>And can we split all the Xen-specific code into a new file?  I don't
>want it mixed in with the generic ACPI code.

Yes. This is what I plan to do next step.

>
>>>    * Make acpi_processor_driver a pointer to the preferred driver
>>>      structure which defaults to the normal driver, and have some Xen
>>>      code re-point it to the Xen one during Xen setup, rather than
>>>      having an explicit Xen test in the core ACPI code
>>>
>> This approach will introduce another dependency between xen setup code and
>acpi processor driver module, and again cannot pass compiling when
>CONFIG_ACPI_PROCESSOR=m. so I would suggest to keep it as it is. How do you
>think?
>>
>
>Well, if ACPI_PROCESSOR=m then the corresponding Xen code would also
>need to be modular.  But aside from that, it would work OK, wouldn't
>it?  Rather than exposing the variable directly, it might be better to
>wrap it with acpi_set_processor_driver() or something.  Or am I missing
>something?
>
>    J

I tried making xen code modular before, the issue here is the two-way 
dependency among acpi processor driver (driver/acpi/processor_core.c) and xen 
driver (driver/xen/acpi_processor.c). firstly, acpi processor driver depend on 
xen driver routine processor_cntl_xen_notify(). Secondly, if xen driver depend 
on some routine of acpi processor driver (e.g. acpi_set_processor_driver as you 
mentioned), then there is two way dependency between these two modules, which 
cannot pass compilation. so what I have done in last patch is moving related 
code from xen part to acpi processor driver part, to eliminate the xen->acpi 
processor dependency. 

Best Regards
Ke

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