Re: [V9 PATCH 7/8] pvh dom0: check for vioapic null ptr in vioapic_range

[Mukesh Rathor <mukesh.rathor@oracle.com>]

On Thu, 24 Apr 2014 07:49:44 +0100
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 23.04.14 at 23:18, <mukesh.rathor@oracle.com> wrote:
> > On Wed, 23 Apr 2014 10:07:25 +0100
> > "Jan Beulich" <JBeulich@suse.com> wrote:
> > 
> >> >>> On 23.04.14 at 02:11, <mukesh.rathor@oracle.com> wrote:
> >> > On Tue, 22 Apr 2014 08:33:29 +0100
> >> > "Jan Beulich" <JBeulich@suse.com> wrote:
> >> >> >>> On 22.04.14 at 02:59, <mukesh.rathor@oracle.com> wrote:
> > ......
> > 
> >> >   So it must have been the third one that I had observed the
> >> >   vioapic_range crash in a while ago, and had made note of it.
> >> > Looking at it:
> >> > 
> >> >     if ( (p2mt == p2m_mmio_dm) ||
> >> >          (access_w && (p2mt == p2m_ram_ro)) )
> >> >     {
> >> >         put_gfn(p2m->domain, gfn);
> >> >         if ( !handle_mmio() )
> >> > 
> >> > doesn't seem apply to domu. Unfortunately, I can't reproduce it
> >> > now so maybe it was an ept violation due to some bug, and a
> >> > crash in vioapic_range before printing the gfn/mfns etc by
> >> > ept_handle_violation made me make a note to put a check in it.
> >> 
> >> Which makes me think that we don't need the patch at all.
> > 
> > Well, without this patch, in case of dom0 EPT violation, dom0 will
> > not die gracefully printing gfn/mfn/etc.. info. But instead it will
> > show fault in vioapic_range. 
> > 
> > 
> > ept_handle_violation() 
> >        hvm_hap_nested_page_fault()
> >              -> handle_mmio() -----> vioapic_range() : KABOOM!!
> > 
> >        gdprintk(XENLOG_ERR, "EPT violation %#lx (%c%c%c/%c%c%c), "
> >                     "gpa %#"PRIpaddr", mfn %#lx, type %i.\n",
> >                                  qualification,  <=== NOT REACHED
> >           .......
> > 
> > I can submit it later too I guess. But without it, we'd not know
> > the ept violation crashes.
> 
> So we're moving in circles I'm afraid: You told us that I/O emulation
> is being handled by a separate path from HVM's, which means either
> handle_mmio() separates the cases itself, or doesn't even get
> called, only to then again show us the call sequence above. One

Correct, doesn't get called other than above path, and my initial
patch had put is_pvh check in hvm_hap_nested_page_fault which would 
avoid call to handle_mmio. But I understand adding too many is_pvh 
checks is undesirable.

I can't reproduce the crash now, so let's just drop this patch for
now.

thanks
mukesh