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