State Component 18 and Palette 1 (Re: [PATCH 16/19] kvm: x86: Introduce KVM_{G|S}ET_XSAVE2 ioctl)

From: "Nakajima, Jun" <jun.nakajima@intel.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Zhong, Yang" <yang.zhong@intel.com>,
	"x86@kernel.org" <x86@kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"bp@alien8.de" <bp@alien8.de>,
	"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
	"Christopherson,, Sean" <seanjc@google.com>,
	"Tian, Kevin" <kevin.tian@intel.com>,
	"jing2.liu@linux.intel.com" <jing2.liu@linux.intel.com>,
	"Liu, Jing2" <jing2.liu@intel.com>
Subject: State Component 18 and Palette 1 (Re: [PATCH 16/19] kvm: x86: Introduce KVM_{G|S}ET_XSAVE2 ioctl)
Date: Mon, 20 Dec 2021 17:54:09 +0000	[thread overview]
Message-ID: <24CFD156-5093-4833-8516-526A90FF350E@intel.com> (raw)
In-Reply-To: <26ea7039-3186-c23f-daba-d039bb8d6f48@redhat.com>

> On Dec 10, 2021, at 2:13 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 12/10/21 17:30, Paolo Bonzini wrote:
>>> 
>>> +static int kvm_vcpu_ioctl_x86_set_xsave2(struct kvm_vcpu *vcpu, u8 *state)
>>> +{
>>> +    if (fpstate_is_confidential(&vcpu->arch.guest_fpu))
>>> +        return 0;
>>> +
>>> +    return fpu_copy_uabi_to_guest_fpstate(&vcpu->arch.guest_fpu, state,
>>> +                          supported_xcr0, &vcpu->arch.pkru);
>>> +}
>>> +
>> I think fpu_copy_uabi_to_guest_fpstate (and therefore copy_uabi_from_kernel_to_xstate) needs to check that the size is compatible with the components in the input.
>> Also, IIUC the size of the AMX state will vary in different processors.   Is this correct?  If so, this should be handled already by KVM_GET/SET_XSAVE2 and therefore should be part of the arch/x86/kernel/fpu APIs.  In the future we want to support migrating a "small AMX" host to a "large AMX" host; and also migrating from a "large AMX" host to a "small AMX" host if the guest CPUID is compatible with the destination of the migration.
> 
> So, the size of the AMX state will depend on the active "palette" in TILECONFIG, and on the CPUID information.  I have a few questions on how Intel intends to handle future extensions to AMX:
> 
> - can we assume that, in the future, palette 1 will always have the same value (bytes_per_row=64, max_names=8, max_rows=16), and basically that the only variable value is really the number of palettes?
> 
> - how does Intel plan to handle bigger TILEDATA?  Will it use more XCR0 bits or will it rather enlarge save state 18?
> 
> If it will use more XCR0 bits, I suppose that XCR0 bits will control which palettes can be chosen by LDTILECFG.
> 
> If not, on the other hand, this will be a first case of one system's XSAVE data not being XRSTOR-able on another system even if the destination system can set XCR0 to the same value as the source system.
> 
> Likewise, if the size and offsets for save state 18 were to vary depending on the selected palette, then this would be novel, in that the save state size and offsets would not be in CPUID anymore.  It would be particularly interesting for non-compacted format, where all save states after 18 would also move forward.
> 
> So, I hope that save state 18 will be frozen to 8k.  In that case, and if palette 1 is frozen to the same values as today, implementing migration will not be a problem; it will be essentially the same as SSE->AVX (horizontal extension of existing registers) and/or AVX->AVX512 (both horizontal and vertical extension).

Hi Paolo,

I would like to confirm that the state component 18 will remain 8KB and palette 1 will remain the same. 

Thanks,
--- 
Jun