Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
From:   Paul Durrant <xadimgnik@gmail.com>
Message-ID: <f5eab713-fa74-2cbc-7df5-81d8d26fee0a@xen.org>
Date:   Mon, 18 Sep 2023 14:41:08 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.0
Reply-To: paul@xen.org
Subject: Re: [PATCH v2 11/12] KVM: selftests / xen: don't explicitly set the
 vcpu_info address
Content-Language: en-US
To:     David Woodhouse <dwmw2@infradead.org>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     Paul Durrant <pdurrant@amazon.com>,
        Sean Christopherson <seanjc@google.com>,
        Paolo Bonzini <pbonzini@redhat.com>
References: <20230918112148.28855-1-paul@xen.org>
 <20230918112148.28855-12-paul@xen.org>
 <f649285c0973ec59180ed51c4ee10cdc51279505.camel@infradead.org>
 <56dad458-8816-2de5-544e-a5e50c5ad2a2@xen.org>
 <c9a1961812b0cbb6e9f641dec5c6edcb21482161.camel@infradead.org>
Organization: Xen Project
In-Reply-To: <c9a1961812b0cbb6e9f641dec5c6edcb21482161.camel@infradead.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 18/09/2023 14:36, David Woodhouse wrote:
> On Mon, 2023-09-18 at 14:26 +0100, Paul Durrant wrote:
>> On 18/09/2023 14:21, David Woodhouse wrote:
>>> On Mon, 2023-09-18 at 11:21 +0000, Paul Durrant wrote:
>>>> From: Paul Durrant <pdurrant@amazon.com>
>>>>
>>>> If the vCPU id is set and the shared_info is mapped using HVA then we can
>>>> infer that KVM has the ability to use a default vcpu_info mapping. Hence
>>>> we can stop setting the address of the vcpu_info structure.
>>>
>>> Again that means we're not *testing* it any more when the test is run
>>> on newer kernels. Can we perhaps set it explicitly, after *half* the
>>> tests are done? Maybe to a *different* address than the default which
>>> is derived from the Xen vcpu_id? And check that the memcpy works right
>>> when we do?
>>>
>>
>> Ok. The VMM is currently responsible for that memcpy. Are you suggesting
>> we push that into KVM too?
> 
> Ah OK.
> 
> Hm, maybe we should?
> 
> What happened before in the case where interrupts were being delivered,
> and the vcpu_info address was changed.
> 
> In Xen, I guess it's effectively atomic? Some locking will mean that
> the event channel is delivered to the vcpu_info either *before* the
> memcpy, or *after* it, but never to the old address after the copy has
> been done, so that the event (well the index of it) gets lost?
> 
> In KVM before we did the automatic placement, it was the VMM's problem.
> 
> If there are any interrupts set up for direct delivery, I suppose the
> VMM should have *removed* the vcpu_info mapping before doing the
> memcpy, then restored it at the new address? I may have to check qemu
> gets that right.
> 
> Then again, it's a very hard race to trigger, given that a guest can
> only set the vcpu_info once. So it can move it from the shinfo to a
> separate address and attempt to trigger this race just that one time.
> 
> But in the case where auto-placement has happened, and then the guest
> sets an explicit vcpu_info location... are we saying that the VMM must
> explicitly *unmap* the vcpu_info first, then memcpy, then set it to the
> new location? Or will we handle the memcpy in-kernel?
> 

Well, if the VMM is using the default then it can't unmap it. But 
setting a vcpu_info *after* enabling any event channels would be a very 
odd thing for a guest to do and IMO it gets to keep the pieces if it 
does so.

   Paul