Re: [Xen-devel] [PATCH 25/34] x86/mm/shadow: make it build with !CONFIG_HVM

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Aug 21, 2018 at 02:27:40AM -0600, Jan Beulich wrote:
> 
> > --- a/xen/arch/x86/mm/shadow/multi.c
> > +++ b/xen/arch/x86/mm/shadow/multi.c
> > @@ -2926,18 +2926,25 @@ static int sh_page_fault(struct vcpu *v,
> >              }
> >              else
> >              {
> > +#if CONFIG_HVM
> >                  /* Magic MMIO marker: extract gfn for MMIO address */
> >                  ASSERT(sh_l1e_is_mmio(sl1e));
> > +                ASSERT(is_hvm_vcpu(v));
> >                  gpa = (((paddr_t)(gfn_x(sh_l1e_mmio_get_gfn(sl1e))))
> >                         << PAGE_SHIFT)
> >                      | (va & ~PAGE_MASK);
> > +                perfc_incr(shadow_fault_fast_mmio);
> > +                SHADOW_PRINTK("fast path mmio %#"PRIpaddr"\n", gpa);
> > +                sh_reset_early_unshadow(v);
> > +                trace_shadow_gen(TRC_SHADOW_FAST_MMIO, va);
> > +                return handle_mmio_with_translation(va, gpa >> PAGE_SHIFT,
> > +                                                    access)
> > +                    ? EXCRET_fault_fixed : 0;
> > +#else
> > +                /* When HVM is not enabled, there shouldn't be MMIO marker 
> > */
> > +                BUG();
> 
> At the example of this, while I agree we shouldn't reach here for PV,
> can this nevertheless be the less impactful domain_crash() please?
> 

Do you only want this BUG() to be replaced?

I think the two in shadonw_*_emulation should stay because you will only
ever get NULL pointer deref if you allow the code to continue.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel