Re: [Xen-devel] [PATCH v4 2/2] x86/PV: support data breakpoint extension registers

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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).

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.

Jan


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