Re: [Xen-devel] [PATCH 17/18 V2]: PVH xen: PVH dom0 creation...

["Jan Beulich" <JBeulich@xxxxxxxx>]

 >>> On 16.03.13 at 02:06, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> @@ -307,6 +309,65 @@ static void __init process_dom0_ioports_disable(void)
>      }
>  }
>  
> +/* 
> + * Set the 1:1 map for all non-RAM regions for dom 0. Thus, dom0 will have
> + * the entire io region mapped in the EPT/NPT.
> + */
> +static __init void  pvh_map_all_iomem(struct domain *d)
> +{
> +    unsigned long start = 0;
> +    const struct e820entry *entry;
> +    int rc, i, nump;
> +
> +    for (i = 0, entry = e820.map; i < e820.nr_map; i++, entry++) {
> +        unsigned long end = entry->addr + entry->size;
> +
> +        if (entry->type == E820_RAM || i == e820.nr_map - 1) {
> +            unsigned long start_pfn = PFN_DOWN(start);
> +            unsigned long end_pfn = PFN_UP(end);
> +
> +            if (entry->type == E820_RAM)
> +                end_pfn = PFN_UP(entry->addr);
> +
> +            if (start_pfn < end_pfn) {
> +                nump = end_pfn - start_pfn + 1;
> +                rc = domctl_memory_mapping(d, start_pfn, start_pfn, nump, 1);
> +                BUG_ON(rc);
> +            }
> +            start = end;
> +        }
> +    }

At least E820_UNUSABLE must be excluded here.

And as you're mapping the holes only - how do you deal with
the MMIO range past end of RAM? And perhaps even more
important - how do you deal with the split between RAM and
MMIO not being at the end of currently populated RAM, but
at the end of possible hotpluggable regions.

> +static __init void copy_pvh(char *dest, char *src, int bytes)
> +{
> +    /* raw_copy_to_guest() -> copy_to_user_hvm -> __hvm_copy needs curr 
> +     * to point to the hvm/pvh vcpu. Hence for PVH dom0 we can't use that.
> +     * So we just use dbg_rw_mem().
> +     */
> +    int rem = dbg_rw_mem((dbgva_t)dest, (unsigned char *)src, bytes, 0, 1, 
> 0);

Same comment as before: This is not acceptable for a submission
of a patch intended to be committed (i.e. non-RFC). You should
have worked out a suitable solution to this before posting.

> @@ -416,6 +478,13 @@ int __init construct_dom0(
>      {
>          printk("Kernel does not support Dom0 operation\n");
>          return -EINVAL;
> +
> +        if ( is_pvh_domain(d) && 
> +             !test_bit(XENFEAT_hvm_callback_vector, parms.f_supported) ) 
> +        {
> +            printk("Kernel does not support PVH mode\n");
> +            return -EINVAL;
> +        }

Adding dead code (after a return).

> @@ -621,16 +696,26 @@ int __init construct_dom0(
>          maddr_to_page(mpt_alloc)->u.inuse.type_info = PGT_l3_page_table;
>          l3start = __va(mpt_alloc); mpt_alloc += PAGE_SIZE;
>      }
> -    clear_page(l4tab);
> -    init_guest_l4_table(l4tab, d);
> -    v->arch.guest_table = pagetable_from_paddr(__pa(l4start));
> -    if ( is_pv_32on64_domain(d) )
> -        v->arch.guest_table_user = v->arch.guest_table;
> +    if ( is_pvh_domain(d) )
> +    {
> +        v->arch.guest_table = pagetable_from_paddr(vpt_start - v_start);

Am I understanding right that you're making v->arch.guest_table
store a guest physical address rather than a host physical one?
Can that really be done consistently across of uses of this field?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel