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

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

Mar 12, 2023, 12:15 PM

Post #1 of 3 (41 views)

From: Chao Gao <chao.gao@intel.com>

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
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.

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
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 &&
+ fault->is_private != kvm_mem_is_private(vcpu->kvm, fault->gfn))
return kvm_do_memory_fault_exit(vcpu, fault);

if (fault->is_private)
--
2.25.1

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

yilun.xu at intel

Mar 27, 2023, 8:15 PM

Post #2 of 3 (41 views)

Permalink

On 2023-03-12 at 10:56:03 -0700, isaku.yamahata@intel.com wrote:
> From: Chao Gao <chao.gao@intel.com>
>
> 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.

> 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.

>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> ---
> 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?

Thanks,
Yilun

> + fault->is_private != kvm_mem_is_private(vcpu->kvm, fault->gfn))
> return kvm_do_memory_fault_exit(vcpu, fault);
>
> if (fault->is_private)
> --
> 2.25.1
>

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

isaku.yamahata at gmail

Mar 29, 2023, 6:15 PM

Post #3 of 3 (38 views)

Permalink

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

> On 2023-03-12 at 10:56:03 -0700, isaku.yamahata@intel.com wrote:
> > From: Chao Gao <chao.gao@intel.com>
> >
> > 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@intel.com>
> > ---
> > 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@gmail.com>