From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] x86/domctl: Adjust size calculations for XEN_DOMCTL_get{_ext_vcpucontext, vcpuextstate} Date: Mon, 28 Apr 2014 14:53:40 +0100 Message-ID: <535E5D64.2040800@citrix.com> References: <1398678232-32733-1-git-send-email-andrew.cooper3@citrix.com> <535E4AE3020000780000CD7B@nat28.tlf.novell.com> <535E34A8.7050105@citrix.com> <535E59B3020000780000CE4E@nat28.tlf.novell.com> <535E490E.2060800@citrix.com> <535E7783020000780000CFD0@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <535E7783020000780000CFD0@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Keir Fraser , IanJackson , Ian Campbell , Xen-devel List-Id: xen-devel@lists.xenproject.org On 28/04/14 14:45, Jan Beulich wrote: >>>> On 28.04.14 at 14:26, wrote: >> On 28/04/14 12:37, Jan Beulich wrote: >>>>>> On 28.04.14 at 12:59, wrote: >>>> On 28/04/14 11:34, Jan Beulich wrote: >>>>>>>> On 28.04.14 at 11:43, wrote: >>>>>> XEN_DOMCTL_get_ext_vcpucontext suffers from the same issue but while trying >> to >>>>>> fix that in similar way, I discovered that it had a genuine bug when >> returning >>>>>> the count of MSRs to the toolstack. When running the hypercall on an active >>>>>> vcpu, the vcpu can arbitrarily alter the count returned to the toolstack by >>>>>> clearing and setting relevant MSRs. >>>>> Did you perhaps overlook the vcpu_pause() there? >>>> There is a vcpu pause in the hypercall, so for the duration of the >>>> hypercall the returned value will be consistent. >>>> >>>> However without the toolstack pausing the domain, issuing this hypercall >>>> twice, first to get the size and second to get the data might still >>>> result in -ENOBUFS if the vcpu suddenly writes non-0 values to the MSRs. >>> And in what way is this different from e.g. XEN_DOMCTL_get_vcpuextstate? >> As xcr0_accum is strictly increasing and only in a few possible steps, >> the size returned can never decrease. As it is context switch material, >> the chances are very good that it will reach the maximum the guest >> kernel is willing to use a long time before migration happens. > Chances you say. But we need guarantees, or rely on the tool stack > knowing to re-issue such requests upon certain kinds of failures (or > accept that migration may not work occasionally, with a retry helping). Yes - that is the fix I intend to use. In the case of -EINVAL and size is now larger, realloc the buffer to the new size and retry. > >>>>> I'm also not really in favor of forcing the tools to allocate memory >>>>> for the array if in fact no MSRs are being used by the guest. >>>> If there are no msrs to receive, then passing a NULL guest handle is >>>> still fine. >>> But the caller can't know whether the count was non-zero just because >>> that's the theoretical maximum or because some MSR really is in use. >> Why is that a problem? > The problem is with the first half of your earlier reply: "If there are > no msrs to receive ..." - the caller just can't tell this with your change > in place. > >> If the toolstack wants to save any possible MSRs the guest is using, >> then it is going to have to provide a buffer large enough for any >> eventual number of MSRs. In the case that the buffer is sufficiently >> sized, Xen writes back msr_count with the number of MSRs written, so the >> toolstack can detect when fewer MSRs have been written back. > In the end all I want to be assured is that migration would fail at the > sending side if there are MSRs that need transmitting. > > Jan > Ah I see. Given the one sole caller in xc_domain_save(), I will add a hunk in v2 which explicitly fails the migration if MSRs would need transmitting, making this safe for the short period before proper MSR transmission can be added. ~Andrew