Re: [Xen-devel] [PATCH v3 8/9] viridian: add implementation of synthetic timers

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.03.19 at 14:09, <Paul.Durrant@xxxxxxxxxx> wrote:
>> From: Paul Durrant
>> Sent: 06 March 2019 13:06
>> 
>> > From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> > Sent: 06 March 2019 12:57
>> > Subject: RE: [PATCH v3 8/9] viridian: add implementation of synthetic 
>> > timers
>> >
>> > >>> On 06.03.19 at 12:23, <Paul.Durrant@xxxxxxxxxx> wrote:
>> > >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> > >> Sent: 25 February 2019 14:54
>> > >>
>> > >> >>> On 31.01.19 at 11:47, <paul.durrant@xxxxxxxxxx> wrote:
>> > >> > +{
>> > >> > +    const struct viridian_page *rt = 
>> > >> > &d->arch.hvm.viridian->reference_tsc;
>> > >> > +    HV_REFERENCE_TSC_PAGE *p = rt->ptr;
>> > >> > +    uint32_t start, end;
>> > >> > +    __int128_t tsc;
>> > >> > +    __int128_t scale;
>> > >>
>> > >> I don't think you need both of them be 128 bits wide. I also don't
>> > >> see why either would want to be of a signed type.
>> > >
>> > > The spec says (as in the comment below):
>> > >
>> > > "The partition reference time is computed by the following formula:
>> > >
>> > > ReferenceTime = ((VirtualTsc * TscScale) >> 64) + TscOffset
>> > >
>> > > The multiplication is a 64 bit multiplication, which results in a 128 bit
>> > > number which is then shifted 64 times to the right to obtain the high 64
>> > > bits.TscScale"
>> >
>> > Well, yes, you want a 128-bit result. But for that you don't need to
>> > multiply 128-bit quantities. See e.g. our own scale_delta() or
>> > hvm_scale_tsc().
>> 
>> Testing showed that by not casting first things were broken. I assumed this 
>> was because the result of
>> the multiplication was being truncated to 64-bits before assignment, but I 
>> can check the generated
>> code. I'll also have a look at the examples you cite.
> 
> Both those examples do the multiplication by inline asm (presumably to avoid 
> truncation). Is that what you'd prefer?

That would imo be best, not the least because of making us independent
of possible issues with 128-bit arithmetic on older compilers.

Jan



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