Re: [Xen-ia64-devel] PV-on-HVM driver for IPF

[Doi.Tsunehisa@xxxxxxxxxxxxxx]

Hi Tristan,

  Thank you for your comment.

You (Tristan.Gingold) said:
>>   I will post patches of PV-on-HVM for IPF.
>>
>>   We wrote the patch under this consideration:
>>
>>    * Expand hvm_op hypercall
>>      + Introduce HVMOP_setup_shared_info_page
>>        - A page allocated on HVM-guest OS is swapped original shared_info
>>          page with this hypercall.
>>        - In x86 code, original shared_info page is used after pv-on-hvm
>>          setup with remapping feature in arch depend HYPERVISOR_memory_op.
>>          But, we can't implement same feature for IPF, thus we select to
>>          implement with this method.
> Can you explain why you can't reuse the HYPERVISOR_memory_op hcall ?
> It isn't clear for me.

  In x86 code (xen/arch/x86/mm.c), it uses only virtual space of page frame
allocated by GuestOS, and remaps the vitual space to original share_info
page frame. But, we can't find same method for IPF.

  Can you suggest us about the remapping method ?

> About the patch:
> +static int
> +vmx_gnttab_setup_table(unsigned long frame_pa, unsigned long nr_frames)
> +{
> +    struct domain *d = current->domain;
> +    int rc = 0, i;
> +    unsigned long o_grant_shared, pgaddr;
> +
> +    if (nr_frames != NR_GRANT_FRAMES) {
> +        return -1;
> You'd better to return -EINVAL.

  I agree. I'll correct it.

> +    }
> +    o_grant_shared = (unsigned long)d->grant_table->shared;
> +    d->grant_table->shared = (struct grant_entry *)domain_mpa_to_imva(d,
> frame_pa);
> +
> +    /* Copy existing grant table shared into new page */
> +    if (o_grant_shared) {
> +        memcpy((void*)d->grant_table->shared,
> +                (void*)o_grant_shared, PAGE_SIZE * nr_frames);
> You should check the result of domain_mpa_to_imva, as it could fail.

  I agree. I'll try to correct it. It returns NULL if it fails, I think.
Is it correct ?

> +            if (likely(IS_XEN_HEAP_FRAME(virt_to_page(pgaddr)))) {
> +                free_domheap_page(virt_to_page(pgaddr));
> +                free_xenheap_page((void *)pgaddr);
> +            }
> +            else {
> +                put_page(virt_to_page(pgaddr));
> +            }
> May create a function to be called by gnttab_setup_table and 
> setup_shared_info_page.

  I think that these function are for only VT-i domain, thus I used
vmx_ prefix. What do you think about it ?

Thanks,
-- Tsunehisa Doi

_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel