Re: [Qemu-devel] [qemu-s390x] [PATCH v3 5/9] s390x/tcg: properly implement the TOD

[Cornelia Huck <cohuck@redhat.com>]

On Tue, 26 Jun 2018 14:28:18 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 26.06.2018 14:27, Cornelia Huck wrote:
> > On Tue, 26 Jun 2018 14:06:00 +0200
> > David Hildenbrand <david@redhat.com> wrote:

> >>>>  /* Set Clock Comparator */
> >>>>  void HELPER(sckc)(CPUS390XState *env, uint64_t time)
> >>>>  {
> >>>> +    S390TODState *td = s390_get_todstate();
> >>>> +    S390TODClass *tdc = S390_TOD_GET_CLASS(td);
> >>>> +    S390TOD tod_base;
> >>>> +
> >>>>      if (time == -1ULL) {
> >>>>          return;
> >>>>      }
> >>>>  
> >>>>      env->ckc = time;
> >>>>  
> >>>> +    tdc->get(td, &tod_base, &error_abort);
> >>>> +    tod_base.low -= time2tod(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));    
> >>>
> >>> So the tdc->get first adds the time2tod, and then you subtract it here
> >>> again? Can't you simply use td->base.low directly instead?    
> >>
> >> That might be a good idea, previously I had tdc->get_base(), but dropped
> >> it because it cannot be implemented for KVM.
> >>
> >> Simply accessing the member here should be fine. Thanks!  
> > 
> > So, we're guaranteed to have td->base.low at the correct value? (Sorry,
> > having a bit of a hard time following around here.)
> >   
> 
> Yes, just verified, changed and tested. We will have the value at that
> place.
> 
> I'll give some more time to review the other parts, then I can resend.

Ok, thx.