To avoid the need for a forward declaration of pit_load_count() in a
subsequent change, move it earlier in the file (along with its helper
callback).

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/emul-i8254.c
+++ b/xen/arch/x86/emul-i8254.c
@@ -87,6 +87,57 @@ static int pit_get_count(PITState *pit,
     return counter;
 }
 
+static void cf_check pit_time_fired(struct vcpu *v, void *priv)
+{
+    uint64_t *count_load_time = priv;
+    TRACE_0D(TRC_HVM_EMUL_PIT_TIMER_CB);
+    *count_load_time = get_guest_time(v);
+}
+
+static void pit_load_count(PITState *pit, int channel, int val)
+{
+    u32 period;
+    struct hvm_hw_pit_channel *s = &pit->hw.channels[channel];
+    struct vcpu *v = vpit_vcpu(pit);
+
+    ASSERT(spin_is_locked(&pit->lock));
+
+    if ( val == 0 )
+        val = 0x10000;
+
+    if ( v == NULL )
+        pit->count_load_time[channel] = 0;
+    else
+        pit->count_load_time[channel] = get_guest_time(v);
+    s->count = val;
+    period = DIV_ROUND(val * SYSTEM_TIME_HZ, PIT_FREQ);
+
+    if ( (v == NULL) || !is_hvm_vcpu(v) || (channel != 0) )
+        return;
+
+    switch ( s->mode )
+    {
+    case 2:
+    case 3:
+        /* Periodic timer. */
+        TRACE_2D(TRC_HVM_EMUL_PIT_START_TIMER, period, period);
+        create_periodic_time(v, &pit->pt0, period, period, 0, pit_time_fired,
+                             &pit->count_load_time[channel], false);
+        break;
+    case 1:
+    case 4:
+        /* One-shot timer. */
+        TRACE_2D(TRC_HVM_EMUL_PIT_START_TIMER, period, 0);
+        create_periodic_time(v, &pit->pt0, period, 0, 0, pit_time_fired,
+                             &pit->count_load_time[channel], false);
+        break;
+    default:
+        TRACE_0D(TRC_HVM_EMUL_PIT_STOP_TIMER);
+        destroy_periodic_time(&pit->pt0);
+        break;
+    }
+}
+
 static int pit_get_out(PITState *pit, int channel)
 {
     struct hvm_hw_pit_channel *s = &pit->hw.channels[channel];
@@ -156,57 +207,6 @@ static int pit_get_gate(PITState *pit, i
     return pit->hw.channels[channel].gate;
 }
 
-static void cf_check pit_time_fired(struct vcpu *v, void *priv)
-{
-    uint64_t *count_load_time = priv;
-    TRACE_0D(TRC_HVM_EMUL_PIT_TIMER_CB);
-    *count_load_time = get_guest_time(v);
-}
-
-static void pit_load_count(PITState *pit, int channel, int val)
-{
-    u32 period;
-    struct hvm_hw_pit_channel *s = &pit->hw.channels[channel];
-    struct vcpu *v = vpit_vcpu(pit);
-
-    ASSERT(spin_is_locked(&pit->lock));
-
-    if ( val == 0 )
-        val = 0x10000;
-
-    if ( v == NULL )
-        pit->count_load_time[channel] = 0;
-    else
-        pit->count_load_time[channel] = get_guest_time(v);
-    s->count = val;
-    period = DIV_ROUND(val * SYSTEM_TIME_HZ, PIT_FREQ);
-
-    if ( (v == NULL) || !is_hvm_vcpu(v) || (channel != 0) )
-        return;
-
-    switch ( s->mode )
-    {
-    case 2:
-    case 3:
-        /* Periodic timer. */
-        TRACE_2D(TRC_HVM_EMUL_PIT_START_TIMER, period, period);
-        create_periodic_time(v, &pit->pt0, period, period, 0, pit_time_fired, 
-                             &pit->count_load_time[channel], false);
-        break;
-    case 1:
-    case 4:
-        /* One-shot timer. */
-        TRACE_2D(TRC_HVM_EMUL_PIT_START_TIMER, period, 0);
-        create_periodic_time(v, &pit->pt0, period, 0, 0, pit_time_fired,
-                             &pit->count_load_time[channel], false);
-        break;
-    default:
-        TRACE_0D(TRC_HVM_EMUL_PIT_STOP_TIMER);
-        destroy_periodic_time(&pit->pt0);
-        break;
-    }
-}
-
 static void pit_latch_count(PITState *pit, int channel)
 {
     struct hvm_hw_pit_channel *c = &pit->hw.channels[channel];

On Tue, May 30, 2023 at 05:30:02PM +0200, Jan Beulich wrote:
> To avoid the need for a forward declaration of pit_load_count() in a
> subsequent change, move it earlier in the file (along with its helper
> callback).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Just a couple of nits, which you might also noticed but decided to not
fix given this is just code movement.

> 
> --- a/xen/arch/x86/emul-i8254.c
> +++ b/xen/arch/x86/emul-i8254.c
> @@ -87,6 +87,57 @@ static int pit_get_count(PITState *pit,
>      return counter;
>  }
>  
> +static void cf_check pit_time_fired(struct vcpu *v, void *priv)

Seems like v could be constified?

> +{
> +    uint64_t *count_load_time = priv;
> +    TRACE_0D(TRC_HVM_EMUL_PIT_TIMER_CB);
> +    *count_load_time = get_guest_time(v);
> +}
> +
> +static void pit_load_count(PITState *pit, int channel, int val)
> +{
> +    u32 period;

uint32_t

Thanks, Roger.

On 01.06.2023 11:17, Roger Pau Monné wrote:
> On Tue, May 30, 2023 at 05:30:02PM +0200, Jan Beulich wrote:
>> To avoid the need for a forward declaration of pit_load_count() in a
>> subsequent change, move it earlier in the file (along with its helper
>> callback).
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

> Just a couple of nits, which you might also noticed but decided to not
> fix given this is just code movement.

Indeed, I meant this to be pure code movement. Nevertheless I'd be happy
to take care of style issues, if that's deemed okay in a "pure code
movement" patch. However, ...

>> --- a/xen/arch/x86/emul-i8254.c
>> +++ b/xen/arch/x86/emul-i8254.c
>> @@ -87,6 +87,57 @@ static int pit_get_count(PITState *pit,
>>      return counter;
>>  }
>>  
>> +static void cf_check pit_time_fired(struct vcpu *v, void *priv)
> 
> Seems like v could be constified?

... the function being used as a callback, I doubt adding const would
be possible. Otoh ...

>> +{
>> +    uint64_t *count_load_time = priv;

... there's a blank line missing here, if I was to go for style
adjustments.

Jan

>> +    TRACE_0D(TRC_HVM_EMUL_PIT_TIMER_CB);
>> +    *count_load_time = get_guest_time(v);
>> +}
>> +
>> +static void pit_load_count(PITState *pit, int channel, int val)
>> +{
>> +    u32 period;
> 
> uint32_t
> 
> Thanks, Roger.

On Thu, Jun 01, 2023 at 11:56:12AM +0200, Jan Beulich wrote:
> On 01.06.2023 11:17, Roger Pau Monné wrote:
> > On Tue, May 30, 2023 at 05:30:02PM +0200, Jan Beulich wrote:
> >> To avoid the need for a forward declaration of pit_load_count() in a
> >> subsequent change, move it earlier in the file (along with its helper
> >> callback).
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> > 
> > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Thanks.
> 
> > Just a couple of nits, which you might also noticed but decided to not
> > fix given this is just code movement.
> 
> Indeed, I meant this to be pure code movement. Nevertheless I'd be happy
> to take care of style issues, if that's deemed okay in a "pure code
> movement" patch. However, ...

It's just small style issues, so it would be OK for me.

> >> --- a/xen/arch/x86/emul-i8254.c
> >> +++ b/xen/arch/x86/emul-i8254.c
> >> @@ -87,6 +87,57 @@ static int pit_get_count(PITState *pit,
> >>      return counter;
> >>  }
> >>  
> >> +static void cf_check pit_time_fired(struct vcpu *v, void *priv)
> > 
> > Seems like v could be constified?
> 
> ... the function being used as a callback, I doubt adding const would
> be possible. Otoh ...

Oh, I see.

> >> +{
> >> +    uint64_t *count_load_time = priv;
> 
> ... there's a blank line missing here, if I was to go for style
> adjustments.

Sure.

Thanks, Roger.

On 01.06.2023 13:50, Roger Pau Monné wrote:
> On Thu, Jun 01, 2023 at 11:56:12AM +0200, Jan Beulich wrote:
>> On 01.06.2023 11:17, Roger Pau Monné wrote:
>>> On Tue, May 30, 2023 at 05:30:02PM +0200, Jan Beulich wrote:
>>>> To avoid the need for a forward declaration of pit_load_count() in a
>>>> subsequent change, move it earlier in the file (along with its helper
>>>> callback).
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
>>
>> Thanks.
>>
>>> Just a couple of nits, which you might also noticed but decided to not
>>> fix given this is just code movement.
>>
>> Indeed, I meant this to be pure code movement. Nevertheless I'd be happy
>> to take care of style issues, if that's deemed okay in a "pure code
>> movement" patch. However, ...
> 
> It's just small style issues, so it would be OK for me.

So I've done the obvious ones. There's a further signed/unsigned issue
which isn't quite as clear whether to take care of "on the fly": The
function's 2nd and 3rd parameters both ought to be unsigned, yet
throughout the full file the same issue exists many more times. So I
guess I'll leave those untouched for now.

Jan

On Thu, Jun 01, 2023 at 04:06:53PM +0200, Jan Beulich wrote:
> On 01.06.2023 13:50, Roger Pau Monné wrote:
> > On Thu, Jun 01, 2023 at 11:56:12AM +0200, Jan Beulich wrote:
> >> On 01.06.2023 11:17, Roger Pau Monné wrote:
> >>> On Tue, May 30, 2023 at 05:30:02PM +0200, Jan Beulich wrote:
> >>>> To avoid the need for a forward declaration of pit_load_count() in a
> >>>> subsequent change, move it earlier in the file (along with its helper
> >>>> callback).
> >>>>
> >>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >>>
> >>> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> >>
> >> Thanks.
> >>
> >>> Just a couple of nits, which you might also noticed but decided to not
> >>> fix given this is just code movement.
> >>
> >> Indeed, I meant this to be pure code movement. Nevertheless I'd be happy
> >> to take care of style issues, if that's deemed okay in a "pure code
> >> movement" patch. However, ...
> > 
> > It's just small style issues, so it would be OK for me.
> 
> So I've done the obvious ones. There's a further signed/unsigned issue
> which isn't quite as clear whether to take care of "on the fly": The
> function's 2nd and 3rd parameters both ought to be unsigned, yet
> throughout the full file the same issue exists many more times. So I
> guess I'll leave those untouched for now.

No strong opinion regarding those, I'm fine if you want to adjust also
to unsigned.

Thanks, Roger.