[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 2/2] x86/PV: support data breakpoint extension registers
>>> On 23.04.14 at 14:23, <Ian.Campbell@xxxxxxxxxxxxx> wrote: > On Wed, 2014-04-23 at 12:52 +0100, Jan Beulich wrote: >> >>> On 23.04.14 at 12:23, <Ian.Campbell@xxxxxxxxxxxxx> wrote: >> > On Wed, 2014-04-16 at 15:34 +0100, Jan Beulich wrote: >> >> @@ -583,6 +593,7 @@ struct xen_domctl_ext_vcpucontext { >> >> uint16_t sysenter_callback_cs; >> >> uint8_t syscall32_disables_events; >> >> uint8_t sysenter_disables_events; >> >> + uint16_t msr_count; >> >> #if defined(__GNUC__) >> >> union { >> >> uint64_aligned_t mcg_cap; >> >> @@ -591,6 +602,7 @@ struct xen_domctl_ext_vcpucontext { >> >> #else >> >> struct hvm_vmce_vcpu vmce; >> >> #endif >> >> + XEN_GUEST_HANDLE_64(xen_domctl_ext_vcpu_msr_t) msrs; >> > >> > I must be missing something because I can't see where the tools are >> > initialising msrs, nor does the hypervisor appear to check it is valid >> > before trying to save stuff to it (although that would be caught by the >> > copy_to_user I expect). >> > >> > Also how does one go about determining the correct msr_count before >> > retrieving this state? >> >> When msr_count is zero and MSRs are there that need storing, the >> call will return -ENOBUFS and set msr_count to the required (minimum) >> value. Furthermore the field is only being looked at if the size stored >> inside the structure covers the entire msrs field. And yes, if >> msr_count is non-zero but msrs doesn't point to a valid memory block, >> copy_to_guest() will catch this (as usual). > > All makes sense. Worth a comment though? Not sure - it's no more subtle than other code in the handling of that specific sub-hypercall. But yes, by my own argumentation elsewhere maybe I shouldn't be extending badness - if only I saw ways of describing things like this without just converting C to human language (which doesn't seem all that helpful)... >> So as is the tools are fine not explicitly setting msr_count (it's being >> implicitly set to zero) - state save will fail in that case. It's the 3rd >> (unfinished and now delayed until after Andrew's re-write) patch >> that would be dealing with that error, re-issuing the call after >> allocating a suitable array. > > That would be a bisection hazard wouldn't it? Only with guests using the extension and wanting to be migrated. > The tools side needs to be > part of this patch I think. In any case I suppose this series should > wait until the 3rd patch is available? That might mean I'd have to carry this for months. I'd rather want to get this in as is, and live with save/migration not working for guests using the extension. Even more so that I'm hoping to get someone more fluent in libxc speak to take care of that. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |