On Mon, 2020-12-14 at 15:20 +0000, Joao Martins wrote:
> On 12/14/20 2:58 PM, David Woodhouse wrote:
> > On Mon, 2020-12-14 at 13:29 +0000, Joao Martins wrote:
> > > We might be missing the case where only shared_info is registered. Something like:
> > >
> > > if (vcpu->xen.shinfo_set && !vcpu->xen.vcpu_info_set) {
> > > offset = offsetof(struct compat_vcpu_info, time);
> > > offset += offsetof(struct shared_info, vcpu_info);
> > > offset += (v - kvm_get_vcpu_by_id(0)) * sizeof(struct compat_vcpu_info);
> > >
> > > kvm_setup_pvclock_page(v, &vcpu->xen.shinfo_cache, offset);
> > > }
> > >
> > > Part of the reason I had a kvm_xen_setup_pvclock_page() was to handle this all these
> > > combinations i.e. 1) shared_info && !vcpu_info 2) vcpu_info and unilaterially updating
> > > secondary time info.
> > >
> > > But maybe introducing this xen_vcpu_info() helper to accommodate all this.
> >
> > There was complexity.
> >
> > I don't like complexity.
> >
> > I made it go away.
> >
>
> Considering what you said earlier, yes, it would be unnecessary complexity.
To be fair I don't really make it go away completely; I push it up into
userspace. Which now has to keep track of whether a given vCPU has an
explicitly-set vcpu_info page, or whether it has just used the default
one in the shared_info.
And if the shared_info *changes*, as it might on a kexec or just guest
weirdness, the VMM should ideally change only those vCPUs which were at
the default location.
When it was just v->vcpu_info?:shinfo->vcpu_info[N] in the kernel that
part was slightly simpler. It was just slightly hampered by the fact
that the kernel has no way of knowing what N should be :)
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
