Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
From:   Paul Durrant <xadimgnik@gmail.com>
Message-ID: <34457a97-516f-4122-a9d7-920eb3b3c2e0@xen.org>
Date:   Fri, 22 Sep 2023 16:49:49 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Reply-To: paul@xen.org
Subject: Re: [PATCH v5 07/10] KVM: xen: allow vcpu_info to be mapped by fixed
 HVA
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>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        "H. Peter Anvin" <hpa@zytor.com>, x86@kernel.org
References: <20230922150009.3319-1-paul@xen.org>
 <20230922150009.3319-8-paul@xen.org>
 <8f61e1618f23e975f30e552c09787c5f82ee89f3.camel@infradead.org>
Organization: Xen Project
In-Reply-To: <8f61e1618f23e975f30e552c09787c5f82ee89f3.camel@infradead.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 22/09/2023 16:44, David Woodhouse wrote:
> On Fri, 2023-09-22 at 15:00 +0000, Paul Durrant wrote:
>> From: Paul Durrant <pdurrant@amazon.com>
>>
>> If the guest does not explicitly set the GPA of vcpu_info structure in
>> memory then, for guests with 32 vCPUs or fewer, the vcpu_info embedded
>> in the shared_info page may be used. As described in a previous commit,
>> the shared_info page is an overlay at a fixed HVA within the VMM, so in
>> this case it also more optimal to activate the vcpu_info cache with a
>> fixed HVA to avoid unnecessary invalidation if the guest memory layout
>> is modified.
>>
>> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> 
> But it should *only* be defined as an HVA in the case where it's the
> one in the shinfo. Otherwise, it's defined by its GPA.
> 
> Which almost makes me want to see a sanity check that it precisely
> equals &shinfo->vcpu_info[vcpu->arch.xen.vcpu_id].
> 
> Which brings me back around the circle again to wonder why we don't
> just *default* to it.... you hate me, don't you?
> 
> Your previous set of patches did that, and it did end requiring that
> the VMM restore both VCPU_INFO and VCPU_ID for each vCPU *before*
> restoring the SHARED_INFO_HVA on resume, but wasn't that OK?
> 

No. It was painful and overly complex, with too many corner cases that 
were making my brain hurt. Given the pain of the last week, leaving the 
default handling in the VMM is preferable. It means I don't need to
impose rules about attribute ordering and hence get caught by needing
one thread to wait for another inside the VMM. It's better and more 
robust this way.

   Paul