Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Received-SPF: Pass (protection.outlook.com: domain of amd.com designates
 165.204.84.17 as permitted sender) receiver=protection.outlook.com;
 client-ip=165.204.84.17; helo=SATLEXMB04.amd.com;
Date:   Sun, 24 Oct 2021 22:58:33 -0500
From:   Michael Roth <michael.roth@amd.com>
To:     Paolo Bonzini <pbonzini@redhat.com>
CC:     <linux-kselftest@vger.kernel.org>, <kvm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <x86@kernel.org>,
        Nathan Tempelman <natet@google.com>,
        Marc Orr <marcorr@google.com>,
        Steve Rutherford <srutherford@google.com>,
        Sean Christopherson <seanjc@google.com>,
        "Mingwei Zhang" <mizhang@google.com>,
        Brijesh Singh <brijesh.singh@amd.com>,
        "Tom Lendacky" <thomas.lendacky@amd.com>,
        Varad Gautam <varad.gautam@suse.com>,
        Shuah Khan <shuah@kernel.org>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        "David Woodhouse" <dwmw@amazon.co.uk>,
        Ricardo Koller <ricarkol@google.com>,
        "Jim Mattson" <jmattson@google.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        "Joerg Roedel" <joro@8bytes.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        "H . Peter Anvin" <hpa@zytor.com>
Subject: Re: [RFC 06/16] KVM: selftests: add library for creating/interacting
 with SEV guests
Message-ID: <20211025035833.yqphcnf5u3lk4zgg@amd.com>
References: <20211005234459.430873-1-michael.roth@amd.com>
 <20211006203710.13326-1-michael.roth@amd.com>
 <80a69f94-23ca-3ca9-4c77-14e09683dc7d@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <80a69f94-23ca-3ca9-4c77-14e09683dc7d@redhat.com>
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Oct 2021 04:29:30.4928
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: e64c6624-e1f9-43af-f890-08d9977009d0
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.17];Helo=[SATLEXMB04.amd.com]
X-MS-Exchange-CrossTenant-AuthSource: CO1NAM11FT033.eop-nam11.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR12MB3671
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Oct 21, 2021 at 05:39:34PM +0200, Paolo Bonzini wrote:
> On 06/10/21 22:37, Michael Roth wrote:
> > +struct sev_sync_data {
> > +	uint32_t token;
> > +	bool pending;
> > +	bool done;
> > +	bool aborted;
> > +	uint64_t info;
> > +};
> > +
> 
> Please add a comment explaining roughly the design and what the fields are
> for.  Maybe the bools can be replaced by an enum { DONE, ABORT, SYNC,
> RUNNING } (running is for pending==false)?

That makes sense. And SYNC is basically pending==true.

Though since it would be the test code calling sev_check_guest_sync() that
sets pending to true after each sync, "RUNNABLE" might be a better name
since whether it's actually running depends on a separate call to
vcpu_run().

> 
> Also, for the part that you can feel free to ignore: this seems to be
> similar to the ucall mechanism.  Is it possible to implement the ucall
> interface in terms of this one (or vice versa)?
> 
> One idea could be to:
> 
> - move ucall to the main lib/ directory
> 
> - make it use a struct of function pointers, whose default implementation
> would be in the existing lib/ARCH/ucall.c files
> 
> - add a function to register the struct for the desired implementation
> 
> - make sev.c register its own implementation

That seems doable. We'd also need to register an opaque pointer that, in the
case of SEV, would be for the shared page used for syncing. The guest would
also need the pointers for the ops/opaque, but ucall_init() could
additionally set some globals to provide the GVAs for them in the guest
environment.

Will look at that for the next spin.

> 
> Thanks,
> 
> Paolo
>