On Mon Jul 29, 2024 at 1:53 PM UTC, Vitaly Kuznetsov wrote: > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe. > Nicolas Saenz Julienne <nsaenz@xxxxxxxxxx> writes: > > > Hi Vitaly, > > Thanks for having a look at this. > > > > On Mon Jul 8, 2024 at 2:59 PM UTC, Vitaly Kuznetsov wrote: > >> Nicolas Saenz Julienne <nsaenz@xxxxxxxxxx> writes: > >> > >> > Prepare infrastructure to be able to return data through the XMM > >> > registers when Hyper-V hypercalls are issues in fast mode. The XMM > >> > registers are exposed to user-space through KVM_EXIT_HYPERV_HCALL and > >> > restored on successful hypercall completion. > >> > > >> > Signed-off-by: Nicolas Saenz Julienne <nsaenz@xxxxxxxxxx> > >> > > >> > --- > >> > > >> > There was some discussion in the RFC about whether growing 'struct > >> > kvm_hyperv_exit' is ABI breakage. IMO it isn't: > >> > - There is padding in 'struct kvm_run' that ensures that a bigger > >> > 'struct kvm_hyperv_exit' doesn't alter the offsets within that struct. > >> > - Adding a new field at the bottom of the 'hcall' field within the > >> > 'struct kvm_hyperv_exit' should be fine as well, as it doesn't alter > >> > the offsets within that struct either. > >> > - Ultimately, previous updates to 'struct kvm_hyperv_exit's hint that > >> > its size isn't part of the uABI. It already grew when syndbg was > >> > introduced. > >> > >> Yes but SYNDBG exit comes with KVM_EXIT_HYPERV_SYNDBG. While I don't see > >> any immediate issues with the current approach, we may want to introduce > >> something like KVM_EXIT_HYPERV_HCALL_XMM: the userspace must be prepared > >> to handle this new information anyway and it is better to make > >> unprepared userspace fail with 'unknown exit' then to mishandle a > >> hypercall by ignoring XMM portion of the data. > > > > OK, I'll go that way. Just wanted to get a better understanding of why > > you felt it was necessary. > > > > (sorry for delayed reply, I was on vacation) > > I don't think it's an absolute must but it appears as a cleaner approach > to me. > > Imagine there's some userspace which handles KVM_EXIT_HYPERV_HCALL today > and we want to add XMM handling there. How would we know if xmm portion > of the data is actually filled by KVM or not? With your patch, we can of > course check for HV_X64_HYPERCALL_XMM_OUTPUT_AVAILABLE in > KVM_GET_SUPPORTED_HV_CPUID but this is not really straightforward, is > it? Checking the size is not good either. E.g. think about downstream > versions of KVM which may or may not have certain backports. In case we > (theoretically) do several additions to 'struct kvm_hyperv_exit', it > will quickly become a nightmare. > > On the contrary, KVM_EXIT_HYPERV_HCALL_XMM (or just > KVM_EXIT_HYPERV_HCALL2) approach looks cleaner: once userspace sees it, > it knows that 'xmm' portion of the data can be relied upon. Makes sense, thanks for the explanation. Nicolas
