Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <ed92111b-3b80-0cde-1821-0a491dee2dcf@redhat.com>
Date:   Fri, 29 Apr 2022 15:52:02 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.0
Subject: Re: [PATCH for-5.18] KVM: fix bad user ABI for KVM_EXIT_SYSTEM_EVENT
Content-Language: en-US
To:     Oliver Upton <oupton@google.com>
Cc:     linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        Will Deacon <will@kernel.org>, Marc Zyngier <maz@kernel.org>,
        Peter Gonda <pgonda@google.com>,
        Sean Christopherson <seanjc@google.com>
References: <20220422103013.34832-1-pbonzini@redhat.com>
 <Ymtr2mfyujoxLsDR@google.com>
From:   Paolo Bonzini <pbonzini@redhat.com>
In-Reply-To: <Ymtr2mfyujoxLsDR@google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 4/29/22 06:38, Oliver Upton wrote:
>> +                        __u64 data[16];
> This is out of sync with the union { flags; data; } now.

Yes, that's intentional.  The flags member is mentioned below:

+Previous versions of Linux defined a `flags` member in this struct.  The
+field is now aliased to `data[0]`.  Userspace can assume that it is only
+written if ndata is greater than 0.

but I don't want projects to believe it is different in any way from
`data[0]`.  In particular, `flags` should also be considered valid only
if the cap is present (unless crosvm wants ARM to be grandfathered in).

> IMO, we should put a giant disclaimer on all of this to*not*  use the
> flags field and instead only use data. I imagine we wont want to persist
> the union forever as it is quite ugly, but necessary.


>> +/* #define KVM_CAP_VM_TSC_CONTROL 214 */
> 
> This sticks out a bit. Couldn't the VM TSC control patch just use a
> different number? It seems that there will be a conflict anyway, if only to
> delete this comment.

I don't want to change cap numbers once things have landed in
kvm/next, because that's when userspace projects pick them.

Paolo