On Mon, Feb 26, 2024, Isaku Yamahata wrote: > On Mon, Feb 12, 2024 at 11:29:51AM +0100, > Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote: > > > On Tue, Jan 23, 2024 at 12:55 AM <isaku.yamahata@xxxxxxxxx> wrote: > > > > > > From: Chao Gao <chao.gao@xxxxxxxxx> > > > > > > Guest TD doesn't necessarily invoke MAP_GPA to convert the virtual MMIO > > > range to shared before accessing it. When TD tries to access the virtual > > > 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 guest > > > can't make progress. > > > > > > Instead of requiring the guest to invoke MAP_GPA for regions of virtual > > > 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. > > > > I'm not sure how the patch below deals with host MMIOs? > > It falls back to shared case to hit KVM_PFN_NOSLOT. It will be handled as > MMIO. > > Anyway I found it breaks SW_PROTECTED case. So I came up with the following. > I think we'd like to handle as > - SW_PROTECTED => KVM_EXIT_MEMORY_FAULT > - SNP, TDX => MMIO. > 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 *acknowledging* that you received the message DOES NOT WORK. You didn't even add a Suggested-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.1708933498.git.isaku.yamahata@xxxxxxxxx [2] https://lore.kernel.org/all/8b7380f1b02f8e3995f18bebb085e43165d5d682.1708933498.git.isaku.yamahata@xxxxxxxxx > - if (fault->is_private != kvm_mem_is_private(vcpu->kvm, fault->gfn)) { > + /* > + * !fault->slot means MMIO for SNP and TDX. Don't require explicit GPA > + * conversion for MMIO because MMIO is assigned at the boot time. Fall > + * to !is_private case to get pfn = KVM_PFN_NOSLOT. > + */ > + force_mmio = !slot && NAK, this already got shot down. https://lore.kernel.org/all/ZcUO5sFEAIH68JIA@xxxxxxxxxx > + vcpu->kvm->arch.vm_type != KVM_X86_DEFAULT_VM && > + vcpu->kvm->arch.vm_type != KVM_X86_SW_PROTECTED_VM; > + if (!force_mmio && > + fault->is_private != kvm_mem_is_private(vcpu->kvm, fault->gfn)) { > kvm_mmu_prepare_memory_fault_exit(vcpu, fault); > return -EFAULT; > } > > - if (fault->is_private) > + if (!force_mmio && fault->is_private) > return kvm_faultin_pfn_private(vcpu, fault); > > -- > Isaku Yamahata <isaku.yamahata@xxxxxxxxxxxxxxx>
