Re: [CFT PATCH 2/2] KVM: x86: support XSAVES usage in the host

[Paolo Bonzini <pbonzini@redhat.com>]

On 24/11/2014 03:10, Wanpeng Li wrote:
> Hi Paolo,
> On Fri, Nov 21, 2014 at 07:31:18PM +0100, Paolo Bonzini wrote:
> [...]
>> +		u64 feature = valid & -valid;
>> +		int index = fls64(feature) - 1;
>> +		void *src = get_xsave_addr(xsave, feature);
>> +
>> +	        if (src) {
>> +			u32 size, offset, ecx, edx;
>> +			cpuid_count(XSTATE_CPUID, index,
>> +				    &size, &offset, &ecx, &edx);
>> +	                memcpy(dest + offset, src, size);
> 
> The offset you get is still for compact format

No, it's not, or all old software using XSAVE/XRSTOR would be broken.

The code in arch/x86/kernel/xsave.c agrees with me; compacted offsets
(xsave_comp_offsets) are computed by summing sizes, while non-compacted
offsets (xsave_offsets) come for CPUID.

> , so you almost convert compat 
> format to compat format instead of convert compact format to standard format. 
> In addition, I think convert standard format to compact format should be 
> implemented in put path.

If I do that, userspace is broken because it expects standard format.
Hence, passing XSAVE data to userspace in compact format can be done,
but has to be guarded by an explicitly enabled capability (using
KVM_ENABLE_CAP).

I do not think that's useful, since no supervisor-specific states are
defined yet, and anyway they can be passed using KVM_GET/SET_MSR because
this is not a fast path.

Paolo