[V1 PATCH] PVH: avoid call to handle_mmio

[V1 PATCH] PVH: avoid call to handle_mmio

[Mukesh Rathor]

Last try on this. If xen wants to wait till #VE support is implemented,
maybe vendors/testers would wanna pick this up in the meantime. IMO it will
help with debug to have this. In the absense of this, people with ept
violation will just report vioapic_range crash in xen.

thanks
Mukesh

[V1 PATCH] PVH: avoid call to handle_mmio

[Mukesh Rathor]

handle_mmio() is currently unsafe for pvh guests. A call to it would
result in call to vioapic_range that will crash xen since the vioapic
ptr in struct hvm_domain is not initialized for pvh guests.

However, one path exists for such a call. If a pvh guest, dom0 or domU,
unintentionally touches non-existing memory, an EPT violation would occur.
This would result in unconditional call to hvm_hap_nested_page_fault. In
that function, because get_gfn_type_access returns p2m_mmio_dm for non
existing mfns by default, handle_mmio() will get called. This would result
in xen crash instead of the guest crash. This patch addresses that.

Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
---
 xen/arch/x86/hvm/hvm.c | 5 +++++
 xen/arch/x86/hvm/io.c  | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index efbf6d9..51089b3 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2079,6 +2079,11 @@ int hvm_hap_nested_page_fault(paddr_t gpa,
          (access_w && (p2mt == p2m_ram_ro)) )
     {
         put_gfn(p2m->domain, gfn);
+
+        rc = 0;
+        if ( unlikely(is_pvh_vcpu(v)) )
+            goto out;
+
         if ( !handle_mmio() )
             hvm_inject_hw_exception(TRAP_gp_fault, 0);
         rc = 1;
diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
index e6cb5e2..b53395e 100644
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -84,6 +84,8 @@ int handle_mmio(void)
     struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
     int rc;
 
+    ASSERT(!is_pvh_vcpu(current));
+
     hvm_emulate_prepare(&ctxt, guest_cpu_user_regs());
 
     rc = hvm_emulate_one(&ctxt);
-- 
1.8.3.1

Re: [V1 PATCH] PVH: avoid call to handle_mmio

[Jan Beulich]

>>> On 04.06.14 at 00:00, <mukesh.rathor@oracle.com> wrote:
> handle_mmio() is currently unsafe for pvh guests. A call to it would
> result in call to vioapic_range that will crash xen since the vioapic
> ptr in struct hvm_domain is not initialized for pvh guests.
> 
> However, one path exists for such a call. If a pvh guest, dom0 or domU,
> unintentionally touches non-existing memory, an EPT violation would occur.
> This would result in unconditional call to hvm_hap_nested_page_fault. In
> that function, because get_gfn_type_access returns p2m_mmio_dm for non
> existing mfns by default, handle_mmio() will get called. This would result
> in xen crash instead of the guest crash. This patch addresses that.

Yes, we definitely want this until being properly handled, no matter
that crashing the guest here doesn't seem to be the right thing either
(normal x86 behavior would be to drop writes and return all ones for
reads).

Jan

Re: [V1 PATCH] PVH: avoid call to handle_mmio

[Mukesh Rathor]

On Wed, 04 Jun 2014 08:24:15 +0100
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 04.06.14 at 00:00, <mukesh.rathor@oracle.com> wrote:
> > handle_mmio() is currently unsafe for pvh guests. A call to it would
> > result in call to vioapic_range that will crash xen since the
> > vioapic ptr in struct hvm_domain is not initialized for pvh guests.
> > 
> > However, one path exists for such a call. If a pvh guest, dom0 or
> > domU, unintentionally touches non-existing memory, an EPT violation
> > would occur. This would result in unconditional call to
> > hvm_hap_nested_page_fault. In that function, because
> > get_gfn_type_access returns p2m_mmio_dm for non existing mfns by
> > default, handle_mmio() will get called. This would result in xen
> > crash instead of the guest crash. This patch addresses that.
> 
> Yes, we definitely want this until being properly handled, no matter
> that crashing the guest here doesn't seem to be the right thing either
> (normal x86 behavior would be to drop writes and return all ones for
> reads).

How about doing the same we do for HVM which is inject GP. Then handle_mmio
would just return 0 for pvh, and hvm_hap_nested_page_fault would not need
to be modified.

Mukesh

Re: [V1 PATCH] PVH: avoid call to handle_mmio

[Jan Beulich]

>>> On 05.06.14 at 01:52, <mukesh.rathor@oracle.com> wrote:
> On Wed, 04 Jun 2014 08:24:15 +0100
> "Jan Beulich" <JBeulich@suse.com> wrote:
> 
>> >>> On 04.06.14 at 00:00, <mukesh.rathor@oracle.com> wrote:
>> > handle_mmio() is currently unsafe for pvh guests. A call to it would
>> > result in call to vioapic_range that will crash xen since the
>> > vioapic ptr in struct hvm_domain is not initialized for pvh guests.
>> > 
>> > However, one path exists for such a call. If a pvh guest, dom0 or
>> > domU, unintentionally touches non-existing memory, an EPT violation
>> > would occur. This would result in unconditional call to
>> > hvm_hap_nested_page_fault. In that function, because
>> > get_gfn_type_access returns p2m_mmio_dm for non existing mfns by
>> > default, handle_mmio() will get called. This would result in xen
>> > crash instead of the guest crash. This patch addresses that.
>> 
>> Yes, we definitely want this until being properly handled, no matter
>> that crashing the guest here doesn't seem to be the right thing either
>> (normal x86 behavior would be to drop writes and return all ones for
>> reads).
> 
> How about doing the same we do for HVM which is inject GP. Then handle_mmio
> would just return 0 for pvh, and hvm_hap_nested_page_fault would not need
> to be modified.

The fundamentally wrong thing is that real hardware wouldn't
surface #GP on any wrong physical address - with one exception,
the only possibility would be #MC, and I don't think this would
ever happen for truly unpopulated ranges. (The exception being
on AMD, where #PF gets surfaced when trying to access a page
referring to the HT reserved address range.) IOW even on HVM
it is wrong for us to inject #GP in cases like this.

Jan

Re: [V1 PATCH] PVH: avoid call to handle_mmio

[Mukesh Rathor]

On Thu, 05 Jun 2014 07:28:30 +0100
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 05.06.14 at 01:52, <mukesh.rathor@oracle.com> wrote:
> > On Wed, 04 Jun 2014 08:24:15 +0100
> > "Jan Beulich" <JBeulich@suse.com> wrote:
> > 
> >> >>> On 04.06.14 at 00:00, <mukesh.rathor@oracle.com> wrote:
> >> > handle_mmio() is currently unsafe for pvh guests. A call to it
> >> > would result in call to vioapic_range that will crash xen since
> >> > the vioapic ptr in struct hvm_domain is not initialized for pvh
> >> > guests.
> >> > 
> >> > However, one path exists for such a call. If a pvh guest, dom0 or
> >> > domU, unintentionally touches non-existing memory, an EPT
> >> > violation would occur. This would result in unconditional call to
> >> > hvm_hap_nested_page_fault. In that function, because
> >> > get_gfn_type_access returns p2m_mmio_dm for non existing mfns by
> >> > default, handle_mmio() will get called. This would result in xen
> >> > crash instead of the guest crash. This patch addresses that.
> >> 
> >> Yes, we definitely want this until being properly handled, no
> >> matter that crashing the guest here doesn't seem to be the right
> >> thing either (normal x86 behavior would be to drop writes and
> >> return all ones for reads).
> > 
> > How about doing the same we do for HVM which is inject GP. Then
> > handle_mmio would just return 0 for pvh, and
> > hvm_hap_nested_page_fault would not need to be modified.
> 
> The fundamentally wrong thing is that real hardware wouldn't
> surface #GP on any wrong physical address - with one exception,
> the only possibility would be #MC, and I don't think this would
> ever happen for truly unpopulated ranges. (The exception being
> on AMD, where #PF gets surfaced when trying to access a page
> referring to the HT reserved address range.) IOW even on HVM
> it is wrong for us to inject #GP in cases like this.

We, internally here, discussed the idea of injecting #PF instead of #GP. That 
makes sense from baremetal perspective. IOW, if we look at it from the 
prespective that the guest kernel put a bad pte entry, then on baremetal
it will get a #PF. Moreover, a PV guest returns 0 for mfn if the p2m
lookup fails, and in some paths it appears the code does do_mmu_update
subsequently.... which should result in guest getting a NULL ptr later. 

-Mukesh

Re: [V1 PATCH] PVH: avoid call to handle_mmio

[Jan Beulich]

>>> On 06.06.14 at 21:42, <mukesh.rathor@oracle.com> wrote:
> On Thu, 05 Jun 2014 07:28:30 +0100
> "Jan Beulich" <JBeulich@suse.com> wrote:
>> The fundamentally wrong thing is that real hardware wouldn't
>> surface #GP on any wrong physical address - with one exception,
>> the only possibility would be #MC, and I don't think this would
>> ever happen for truly unpopulated ranges. (The exception being
>> on AMD, where #PF gets surfaced when trying to access a page
>> referring to the HT reserved address range.) IOW even on HVM
>> it is wrong for us to inject #GP in cases like this.
> 
> We, internally here, discussed the idea of injecting #PF instead of #GP. 
> That 
> makes sense from baremetal perspective. IOW, if we look at it from the 
> prespective that the guest kernel put a bad pte entry, then on baremetal
> it will get a #PF.

This very much depends on what's wrong with the PTE - reserved
bits being set would yield #PF, but an out-of-(valid-)range physical
address wouldn't normally. And it's that which we've been discussing
here.

Jan