>>> On 18.09.18 at 10:55, <[email protected]> wrote:
> @@ -399,7 +372,8 @@ static int __init pvh_setup_p2m(struct domain *d)
> } while ( preempted );
>
> /*
> - * Memory below 1MB is identity mapped.
> + * Memory below 1MB is identity mapped except RAM regions that are
> + * populated and copied below.
> * NB: this only makes sense when booted from legacy BIOS.
> */
> rc = modify_identity_mmio(d, 0, MB1_PAGES, true);
Mind making the comment even less ambiguous, e.g.
* Memory below 1MB is identity mapped initially. RAM regions are
* populated and copied below, replacing the respective mappings.
?
> @@ -420,16 +394,27 @@ static int __init pvh_setup_p2m(struct domain *d)
> addr = PFN_DOWN(d->arch.e820[i].addr);
> size = PFN_DOWN(d->arch.e820[i].size);
>
> - if ( addr >= MB1_PAGES )
> - rc = pvh_populate_memory_range(d, addr, size);
> - else
> - {
> - ASSERT(addr + size < MB1_PAGES);
> - pvh_steal_low_ram(d, addr, size);
> - }
> -
> + rc = pvh_populate_memory_range(d, addr, size);
> if ( rc )
> return rc;
> +
> + if ( addr < MB1_PAGES )
> + {
> + uint64_t end = min_t(uint64_t, MB(1),
> + d->arch.e820[i].addr +
> d->arch.e820[i].size);
I think min_t() and max_t() should only be used as a last resort, due
to their (hidden) casts. min(MB(1ULL), ...) ought to be fine here, I
would think.
> + enum hvm_translation_result res =
> + hvm_copy_to_guest_phys(mfn_to_maddr(_mfn(addr)),
> + mfn_to_virt(addr),
> + d->arch.e820[i].addr - end,
> + v);
> +
> + if ( res != HVMTRANS_okay )
> + {
> + printk("Failed to copy [%#lx, %#lx): %d\n",
> + addr, addr + size, res);
> + return -EFAULT;
I think this would better be logged only - there's no reason to believe
Dom0 wouldn't, in the common case, come up at least in a state
allowing investigation of a problem here. (Not that I think the copying
might fail in the first place, but anyway.)
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
