Received-SPF: pass (google.com: domain of linux-kernel+bounces-82146-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Mon, 26 Feb 2024 10:39:19 -0800
In-Reply-To: <20240226180626.GE177224@ls.amr.corp.intel.com>
Precedence: bulk
Mime-Version: 1.0
References: <cover.1705965634.git.isaku.yamahata@intel.com>
 <cce34006a4db0e1995ce007c917f834b117b12af.1705965635.git.isaku.yamahata@intel.com>
 <CABgObfbZRO3yiXoHAoHSsBp4sKQY9r4GTLt-SRqevz2c8wOqbQ@mail.gmail.com> <20240226180626.GE177224@ls.amr.corp.intel.com>
Message-ID: <Zdza1xJNlvhl8OZb@google.com>
Subject: Re: [PATCH v18 044/121] KVM: x86/mmu: Assume guest MMIOs are shared
From: Sean Christopherson <seanjc@google.com>
To: Isaku Yamahata <isaku.yamahata@linux.intel.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>, isaku.yamahata@intel.com, kvm@vger.kernel.org, 
	linux-kernel@vger.kernel.org, isaku.yamahata@gmail.com, erdemaktas@google.com, 
	Sagi Shahar <sagis@google.com>, Kai Huang <kai.huang@intel.com>, chen.bo@intel.com, 
	hang.yuan@intel.com, tina.zhang@intel.com, Chao Gao <chao.gao@intel.com>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Feb 26, 2024, Isaku Yamahata wrote:
> On Mon, Feb 12, 2024 at 11:29:51AM +0100,
> Paolo Bonzini <pbonzini@redhat.com> wrote:
>=20
> > On Tue, Jan 23, 2024 at 12:55=E2=80=AFAM <isaku.yamahata@intel.com> wro=
te:
> > >
> > > From: Chao Gao <chao.gao@intel.com>
> > >
> > > Guest TD doesn't necessarily invoke MAP_GPA to convert the virtual MM=
IO
> > > range to shared before accessing it.  When TD tries to access the vir=
tual
> > > device's MMIO as shared, an EPT violation is raised first.
> > > kvm_mem_is_private() checks whether the GFN is shared or private.  If
> > > MAP_GPA is not called for the GPA, KVM thinks the GPA is private and
> > > refuses shared access, and doesn't set up shared EPT entry.  The gues=
t
> > > can't make progress.
> > >
> > > Instead of requiring the guest to invoke MAP_GPA for regions of virtu=
al
> > > MMIOs assume regions of virtual MMIOs are shared in KVM as well (i.e.=
, GPAs
> > > either have no kvm_memory_slot or are backed by host MMIOs). So that =
guests
> > > can access those MMIO regions.
> >=20
> > I'm not sure how the patch below deals with host MMIOs?
>=20
> It falls back to shared case to hit KVM_PFN_NOSLOT. It will be handled as
> MMIO.
>=20
> Anyway I found it breaks SW_PROTECTED case.  So I came up with the follow=
ing.
> I think we'd like to handle as
>   - SW_PROTECTED =3D> KVM_EXIT_MEMORY_FAULT
>   - SNP, TDX =3D> MMIO.
>  =20

FFS.  Stop lobbing patch bombs and start having actual conversations.

Seriously, the whole point of using mailing lists is to have *discussions* =
and
to coordinate development.  Throwing patches at kvm@ and then walking away =
DOES
NOT WORK.

Putting a "TODO: Drop this patch once the common patch is merged." in the
changelog[1] is not helpful.

Dropping a proposed common uAPI[2] into a 121 patch series without even *ac=
knowledging*
that you received the message DOES NOT WORK.  You didn't even add a Suggest=
ed-by
or Cc: the people who expressed interest.  I can't read minds, and AFAIK no=
 one
else working on KVM is a telepath either.

I do not know to make it any clearer: for TDX support to go anywhere, there=
 needs
to be a _lot_ more communication.

[1] https://lore.kernel.org/all/b2e5c92fd66a0113b472dd602220346d3d435732.17=
08933498.git.isaku.yamahata@intel.com
[2] https://lore.kernel.org/all/8b7380f1b02f8e3995f18bebb085e43165d5d682.17=
08933498.git.isaku.yamahata@intel.com

> -       if (fault->is_private !=3D kvm_mem_is_private(vcpu->kvm, fault->g=
fn)) {
> +       /*
> +        * !fault->slot means MMIO for SNP and TDX.  Don't require explic=
it GPA
> +        * conversion for MMIO because MMIO is assigned at the boot time.=
  Fall
> +        * to !is_private case to get pfn =3D KVM_PFN_NOSLOT.
> +        */
> +       force_mmio =3D !slot &&

NAK, this already got shot down.

https://lore.kernel.org/all/ZcUO5sFEAIH68JIA@google.com

> +               vcpu->kvm->arch.vm_type !=3D KVM_X86_DEFAULT_VM &&
> +               vcpu->kvm->arch.vm_type !=3D KVM_X86_SW_PROTECTED_VM;
> +       if (!force_mmio &&
> +           fault->is_private !=3D kvm_mem_is_private(vcpu->kvm, fault->g=
fn)) {
>                 kvm_mmu_prepare_memory_fault_exit(vcpu, fault);
>                 return -EFAULT;
>         }
> =20
> -       if (fault->is_private)
> +       if (!force_mmio && fault->is_private)
>                 return kvm_faultin_pfn_private(vcpu, fault);
>=20
> --=20
> Isaku Yamahata <isaku.yamahata@linux.intel.com>