Re: [Xen-devel] Getting the XSAVE size from userspace

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 05/11/15 12:26, Razvan Cojocaru wrote:
> On 11/05/2015 01:44 PM, Andrew Cooper wrote:
>> On 05/11/15 11:35, Andrei LUTAS wrote:
>>> The use-case is the following: whenever an EPT violation is triggered
>>> inside a monitored VM, the introspection logic needs to know how many
>>> bytes were accessed (read/written). This is done by inspecting the
>>> faulting instruction and directly inferring the size, which is not
>>> straight-forward for XSAVE/XRSTOR family. Using the maximum possible
>>> size is wrong, as in any given moment the OS may or may not desire to
>>> XSAVE/XRSTOR the entire state (and thinking that the instruction tries
>>> to access more than it actually does may yield undesired effects).
>>> Therefore, the size needed for the currently enabled features of the
>>> monitored guest is required instead. Normally, it could be done by
>>> running CPUID with eax = 0xD and ecx = i, where i >= 2 and XCR0[i] is
>>> 1 (XCR0 belongs to the monitored guest), but I am unsure if using
>>> CPUID this way would be safe/desired: will Xen expose the same CPUID
>>> features, for XSAVE related functionality, on all VMs? (using XCPUID
>>> with eax = 0xD and ecx = 0 would give us the needed size for the SVA,
>>> and like I said, using the maximum size would not be safe, even if
>>> it's the same across all VMs on a given host). Also, I'm unsure how
>>> this would get along with migration...
>> Hmm yes - there is no way to do this currently.
>>
>> Xen's CPUID handling for xsave related things is broken in levelling and
>> migration scenarios, which is why it is *still* disabled by default in
>> XenServer.
>>
>> I am working on fixing it, and will take this usecase into account
>> (although I think I had already included enough for this usecase to work).
>>
>> At the point of the xsave/xrestor trap, you need to know xcr0 and be
>> able to perfom a cpuid instruction in the context of a target domain, to
>> make use of 0xD[0].ebx to get the "current size based on xcr0".
> So then the closest thing to what we need would be to add a size field
> to struct hvm_hw_cpu_xsave, and just assign the size variable to it in
> hvm_save_cpu_xsave_states (migration aside)?
>
> 2130 static int hvm_save_cpu_xsave_states(struct domain *d,
> hvm_domain_context_t *h)
> 2131 {
> 2132     struct vcpu *v;
> 2133     struct hvm_hw_cpu_xsave *ctxt;
> 2134
> 2135     if ( !cpu_has_xsave )
> 2136         return 0;   /* do nothing */
> 2137
> 2138     for_each_vcpu ( d, v )
> 2139     {
> 2140         unsigned int size = HVM_CPU_XSAVE_SIZE(v->arch.xcr0_accum);
> 2141
> 2142         if ( !xsave_enabled(v) )
> 2143             continue;
> 2144         if ( _hvm_init_entry(h, CPU_XSAVE_CODE, v->vcpu_id, size) )
> 2145             return 1;
> 2146         ctxt = (struct hvm_hw_cpu_xsave *)&h->data[h->cur];
> 2147         h->cur += size;
> 2148
> 2149         ctxt->xfeature_mask = xfeature_mask;
> 2150         ctxt->xcr0 = v->arch.xcr0;
> 2151         ctxt->xcr0_accum = v->arch.xcr0_accum;
> 2152         memcpy(&ctxt->save_area, v->arch.xsave_area,
> 2153                size - offsetof(struct hvm_hw_cpu_xsave, save_area));
> 2154     }
> 2155
> 2156     return 0;
> 2157 }

I don't see any difference between this pasted code and the current
hvm_save_cpu_xsave_states().  What have you changed?

You can't use this size value, and it is the accumulated xcr0 over the
life of the VM, not the xcr0 in use at the time of the intercepted
instruction.

You also can't blindly modify the ctxt structure, or you will break
migration.

The xcr0 -> size mapping is static, and won't change going forwards. 
Your best bet is just to query each one and stash all the results.

~Andrew

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