[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 25/34] x86/mm/shadow: make it build with !CONFIG_HVM
>>> On 26.08.18 at 13:04, <wei.liu2@xxxxxxxxxx> wrote: > 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. Did you perhaps remove too much context? From what's left I can't judge which others you refer to, or what NULL deref you talk about. Looking back at the full patch - I think I had already suggested that the two shadow_*_emulation() should altogether go inside #ifdef CONFIG_HVM, not just their bodies. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |