Re: [Xen-devel] [PATCH RFC 1/3] x86/pvclock: add setter for pvclock_pvti_cpu0_va

[Andy Lutomirski <luto@xxxxxxxxxxxxxx>]

On Mon, Dec 28, 2015 at 1:52 PM, Joao Martins <joao.m.martins@xxxxxxxxxx> wrote:
> Right now there is only a pvclock_pvti_cpu0_va() which is defined on
> kvmclock since:
>
> commit dac16fba6fc5
> ("x86/vdso: Get pvclock data from the vvar VMA instead of the fixmap")
>
> The only user of this interface so far is kvm. This commit adds a setter
> function for the pvti page and moves pvclock_pvti_cpu0_va to pvclock, which
> is a more generic place to have it; and would allow other PV clocksources
> to use it, such as Xen.
>

> +
> +void pvclock_set_pvti_cpu0_va(struct pvclock_vsyscall_time_info *pvti)
> +{
> +       pvti_cpu0_va = pvti;
> +}

IMO this either wants to be __init or wants a
WARN_ON(vclock_was_used(VCLOCK_PVCLOCK)).  The latter hasn't landed in
-tip yet, but I think it'll land next week unless the merge window
opens early.

It may pay to actually separate out the kvm-clock clocksource and
rename it rather than partially duplicating it, assuming the result
wouldn't be messy.

Can you CC me on the rest of the series for new versions?

BTW, since this seems to require hypervisor changes to be useful, it
might make sense to rethink the interface a bit.  Are you actually
planning to support per-cpu pvti for this in any useful way?  If not,
I think that this would work a whole lot better and be considerably
less code if you had a single global pvti that lived in
hypervisor-allocated memory instead of an array that lives in guest
memory.  I'd be happy to discuss next week in more detail (currently
on vacation).

--Andy

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel