Re: [Xen-devel] [PATCH v2 19/23] x86: PIT emulation is common to both PV and HVM

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.08.18 at 16:58, <wei.liu2@xxxxxxxxxx> wrote:
> On Tue, Aug 28, 2018 at 03:51:51PM +0100, Andrew Cooper wrote:
>> On 28/08/18 15:48, Wei Liu wrote:
>> > On Tue, Aug 28, 2018 at 08:36:37AM -0600, Jan Beulich wrote:
>> >>>>> On 28.08.18 at 15:19, <wei.liu2@xxxxxxxxxx> wrote:
>> >>> On Tue, Aug 28, 2018 at 05:44:51AM -0600, Jan Beulich wrote:
>> >>>>>>> On 26.08.18 at 14:19, <wei.liu2@xxxxxxxxxx> wrote:
>> >>>>> Move the file to x86 common code and change its name to emul-i8254.c.
>> >>>>>
>> >>>>> Put HVM only code under CONFIG_HVM or is_hvm_domain.
>> >>>>>
>> >>>>> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
>> >>>>> ---
>> >>>>> v2: move the whole file.
>> >>>>> ---
>> >>>>>  xen/arch/x86/Makefile     |   1 +-
>> >>>>>  xen/arch/x86/emul-i8254.c | 609 
>> >>>>> +++++++++++++++++++++++++++++++++++++++-
>> >>>>>  xen/arch/x86/hvm/Makefile |   1 +-
>> >>>>>  xen/arch/x86/hvm/i8254.c  | 597 
>> >>>>> +--------------------------------------
>> >>>>>  4 files changed, 610 insertions(+), 598 deletions(-)
>> >>>>>  create mode 100644 xen/arch/x86/emul-i8254.c
>> >>>>>  delete mode 100644 xen/arch/x86/hvm/i8254.c
>> >>>> On the assumption that the changes to the file are only CONFIG_HVM
>> >>>> additions
>> >>> No only that. There are a few is_hvm_domain calls as well.
>> >>>
>> >>>> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
>> >>> Does this still stand?
>> >> Yes, albeit looking at the full diff of the files I'm then a little
>> >> puzzled about is_hvm_domain() uses: Why does pit_load_count()
>> >> need an #ifdef added at all, with the is_hvm_vcpu() check
>> >> right before it?
>> > Because if I don't do that we get "defined but not used" warnings.
>> 
>> (hopefully) not with the newer is_hvm_vcpu() which evaluates its
>> arguments properly.
> 
> The real reason is I put pit_time_fired under CONFIG_HVM because it is
> only used as a HVM only callback -- passed as a parameter to
> create_periodic_time.
> 
> If I remove CONFIG_HVM around pit_time_fired, I can remove CONFIG_HVM in
> pit_load_count.  Let me know if you prefer this.

Oh, I see. I'd prefer this, yes, but I'm unconvinced DCE would
eliminate pit_time_fired() in that case. If you find it does, I think
that would be better indeed (less #ifdef clutter).

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel