Re: [Xen-devel] [PATCH RFC 01/12] x86/paging: introduce paging_set_allocation

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 29/07/16 17:28, Roger Pau Monne wrote:
> diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
> index 107fc8c..1b270df 100644
> --- a/xen/arch/x86/mm/paging.c
> +++ b/xen/arch/x86/mm/paging.c
> @@ -953,6 +953,22 @@ void paging_write_p2m_entry(struct p2m_domain *p2m, 
> unsigned long gfn,
>          safe_write_pte(p, new);
>  }
>  
> +int paging_set_allocation(struct domain *d, unsigned long pages)
> +{
> +    int rc;
> +
> +    ASSERT(paging_mode_enabled(d));
> +
> +    paging_lock(d);
> +    if ( hap_enabled(d) )
> +        rc = hap_set_allocation(d, pages, NULL);
> +    else
> +        rc = sh_set_allocation(d, pages, NULL);

(without looking at the rest of the series) Your NMI is probably a
watchdog timeout from this call, as passing NULL means "non-preemptible".

As this is for the construction of dom0, it would be better to take a
preemptible pointer, loop in construct_dom0(), with a
process_pending_softirqs() call in between.

> +    paging_unlock(d);
> +
> +    return rc;
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
> index c22362f..452e22e 100644
> --- a/xen/arch/x86/mm/shadow/common.c
> +++ b/xen/arch/x86/mm/shadow/common.c
> @@ -1604,9 +1604,8 @@ shadow_free_p2m_page(struct domain *d, struct page_info 
> *pg)
>   * Input will be rounded up to at least shadow_min_acceptable_pages(),
>   * plus space for the p2m table.
>   * Returns 0 for success, non-zero for failure. */
> -static unsigned int sh_set_allocation(struct domain *d,
> -                                      unsigned int pages,
> -                                      int *preempted)
> +unsigned int sh_set_allocation(struct domain *d, unsigned int pages,
> +                               int *preempted)
>  {
>      struct page_info *sp;
>      unsigned int lower_bound;
> diff --git a/xen/include/asm-x86/hap.h b/xen/include/asm-x86/hap.h
> index c613836..e3c9c98 100644
> --- a/xen/include/asm-x86/hap.h
> +++ b/xen/include/asm-x86/hap.h
> @@ -46,7 +46,8 @@ int   hap_track_dirty_vram(struct domain *d,
>                             XEN_GUEST_HANDLE_64(uint8) dirty_bitmap);
>  
>  extern const struct paging_mode *hap_paging_get_mode(struct vcpu *);
> -void hap_set_alloc_for_pvh_dom0(struct domain *d, unsigned long num_pages);
> +unsigned int hap_set_allocation(struct domain *d, unsigned int pages,
> +                             int *preempted);

I also note from this change that there is an unsigned long => unsigned
int truncation in the internals of *_set_allocation().  This should
definitely be fixed, although possibly wants to be a separate change.

~Andrew

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