Re: [PATCH v9 10/11] viridian: add implementation of synthetic timers

From: "Jan Beulich" <JBeulich@suse.com>
To: Paul Durrant <paul.durrant@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Tim Deegan <tim@xen.org>,
	george.dunlap@citrix.com, Julien Grall <julien.grall@arm.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Ian Jackson <Ian.Jackson@citrix.com>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH v9 10/11] viridian: add implementation of synthetic timers
Date: Tue, 19 Mar 2019 07:32:28 -0600	[thread overview]
Message-ID: <5C90EF6C0200007800220476@prv1-mh.provo.novell.com> (raw)
In-Reply-To: <22207b1c797348289442efcf6af882b5@AMSPEX02CL02.citrite.net>

>>> On 19.03.19 at 13:47, <Paul.Durrant@citrix.com> wrote:
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: 19 March 2019 12:18
>> 
>> So perhaps a set
>> pending bit should result in the RDMSR handling to clear the enabled
>> bit in the returned value for a non-periodic timer?
> 
> I get tied in knots every time I think about this and without waiting for a 
> pending timer to stop when it is disabled I see no way of the race, but I 
> think doing that would be prohibitively slow because windows seems to flip 
> between single-shot and periodic timers on quite a frequent basis.

I'm afraid I don't understand: Why a timer or any other complications.
All I'm thinking about is

    case HV_X64_MSR_STIMER0_CONFIG:
    case HV_X64_MSR_STIMER1_CONFIG:
    case HV_X64_MSR_STIMER2_CONFIG:
    case HV_X64_MSR_STIMER3_CONFIG:
    {
        unsigned int stimerx = (idx - HV_X64_MSR_STIMER0_CONFIG) / 2;
        const struct viridian_stimer *vs =
            &array_access_nospec(vv->stimer, stimerx);

        if ( !(viridian_feature_mask(d) & HVMPV_stimer) )
            return X86EMUL_EXCEPTION;

        *val = vs->config.raw;
        if ( !vs->config.periodic && test_bit(stimerx, vv->stimer_pending) )
            *val &= ~1;
        break;
    }

or a suitable equivalent to avoid the literal 1, plus perhaps a
helpful comment.

Jan

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