Re: [PATCH v13 039/113] KVM: x86/mmu: Assume guest MMIOs are shared

[Isaku Yamahata]

On Tue, Mar 28, 2023 at 10:39:12AM +0800,
Xu Yilun <yilun.xu@xxxxxxxxx> wrote:

> On 2023-03-12 at 10:56:03 -0700, isaku.yamahata@xxxxxxxxx wrote:
> > From: Chao Gao <chao.gao@xxxxxxxxx>
> > 
> > Current TD guest doesn't invoke MAP_GPA to convert MMIO range to shared
> > before accessing it. It implies that current TD guest assumes MMIOs are
> > shared.
> > 
> > When TD tries to access assigned device's MMIO as shared, an EPT violation
> 
> Seems the patch is dealing with emulated MMIO, not assigned device's MMIO.

That's right. Here I'm discussing about virtual device. Will drop "assigned"
word.


> > is raised first. kvm_mem_is_private() checks the page shared or private
> > attribute against the access type (shared bit in GPA). Then since no
> > MAP_GPA is called for the MMIO, KVM thinks the MMIO is private and refuses
> > shared access and doesn't set up shared EPT. Then KVM returns to TD and TD
> > just retries and this causes an infinite loop.
> > 
> > Instead of requiring guest to invoke MAP_GPA for MMIOs, assume guest MMIOs
> > are shared in KVM as well and don't expect explicit calls of MAP_GAP for
> > guest MMIOs (i.e., GPAs either have no kvm_memory_slot or are backed by
> > host MMIOs). So, allow shared access to guest MMIOs and move the page type
> > check after the corresponding pfn is available.
> 
> Didn't see the movement.
> 
> Seems the commit message needs update.

Will update.


> > Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
> > ---
> >  arch/x86/kvm/mmu/mmu.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > index 5883ab95ff07..ce8a896a3cfa 100644
> > --- a/arch/x86/kvm/mmu/mmu.c
> > +++ b/arch/x86/kvm/mmu/mmu.c
> > @@ -4314,7 +4314,12 @@ static int __kvm_faultin_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
> >  			return RET_PF_EMULATE;
> >  	}
> >  
> > -	if (fault->is_private != kvm_mem_is_private(vcpu->kvm, fault->gfn))
> > +	/*
> > +	 * !fault->slot means MMIO.  Don't require explicit GPA conversion for
> > +	 * MMIO because MMIO is assigned at the boot time.
> > +	 */
> > +	if (fault->slot &&
> 
> This only exempts emulated MMIO, how about the passthrough device's MMIO?

This patch is for virtual MMIO.  If physical device is assigned to shared
region, KVM memory slot is assigned.  EPT entry is setup to point to the HPA.
-- 
Isaku Yamahata <isaku.yamahata@xxxxxxxxx>