Re: [PATCH 16/18] PVH xen: Miscellaneous changes

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Fri, May 24, 2013 at 06:25:35PM -0700, Mukesh Rathor wrote:
> This patch contains misc changes like restricting iobitmap calls for PVH,
> restricting 32bit PVH guest, etc..

Could you please mention _why_ in the commit? And enumerate which
hypercalls are restricted. From the look of it is:

PHYSDEVOP_set_iopl, PHYSDEVOP_set_iobitmap and XEN_DOMCTL_getvcpucontext?


> 
> Changes in V6:
>   - clear out vcpu_guest_context struct in arch_get_info_guest.
> 
> Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>

I think besides the commit description which needs a bit more
explanation you can also attach:

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

> ---
>  xen/arch/x86/domain.c      |    7 +++++++
>  xen/arch/x86/domain_page.c |   10 +++++-----
>  xen/arch/x86/domctl.c      |    6 ++++++
>  xen/arch/x86/mm.c          |    2 +-
>  xen/arch/x86/physdev.c     |   13 +++++++++++++
>  xen/common/grant_table.c   |    4 ++--
>  6 files changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 9953f80..8cff7c9 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -339,6 +339,13 @@ int switch_compat(struct domain *d)
>  
>      if ( d == NULL )
>          return -EINVAL;
> +
> +    if ( is_pvh_domain(d) )
> +    {
> +        gdprintk(XENLOG_G_ERR,
> +                 "Xen does not currently support 32bit PVH guests\n");
> +        return -EINVAL;
> +    }
>      if ( !may_switch_mode(d) )
>          return -EACCES;
>      if ( is_pv_32on64_domain(d) )
> diff --git a/xen/arch/x86/domain_page.c b/xen/arch/x86/domain_page.c
> index efda6af..7685416 100644
> --- a/xen/arch/x86/domain_page.c
> +++ b/xen/arch/x86/domain_page.c
> @@ -34,7 +34,7 @@ static inline struct vcpu *mapcache_current_vcpu(void)
>       * then it means we are running on the idle domain's page table and must
>       * therefore use its mapcache.
>       */
> -    if ( unlikely(pagetable_is_null(v->arch.guest_table)) && !is_hvm_vcpu(v) )
> +    if ( unlikely(pagetable_is_null(v->arch.guest_table)) && is_pv_vcpu(v) )
>      {
>          /* If we really are idling, perform lazy context switch now. */
>          if ( (v = idle_vcpu[smp_processor_id()]) == current )
> @@ -71,7 +71,7 @@ void *map_domain_page(unsigned long mfn)
>  #endif
>  
>      v = mapcache_current_vcpu();
> -    if ( !v || is_hvm_vcpu(v) )
> +    if ( !v || !is_pv_vcpu(v) )
>          return mfn_to_virt(mfn);
>  
>      dcache = &v->domain->arch.pv_domain.mapcache;
> @@ -175,7 +175,7 @@ void unmap_domain_page(const void *ptr)
>      ASSERT(va >= MAPCACHE_VIRT_START && va < MAPCACHE_VIRT_END);
>  
>      v = mapcache_current_vcpu();
> -    ASSERT(v && !is_hvm_vcpu(v));
> +    ASSERT(v && is_pv_vcpu(v));
>  
>      dcache = &v->domain->arch.pv_domain.mapcache;
>      ASSERT(dcache->inuse);
> @@ -242,7 +242,7 @@ int mapcache_domain_init(struct domain *d)
>      struct mapcache_domain *dcache = &d->arch.pv_domain.mapcache;
>      unsigned int bitmap_pages;
>  
> -    if ( is_hvm_domain(d) || is_idle_domain(d) )
> +    if ( !is_pv_domain(d) || is_idle_domain(d) )
>          return 0;
>  
>  #ifdef NDEBUG
> @@ -273,7 +273,7 @@ int mapcache_vcpu_init(struct vcpu *v)
>      unsigned int ents = d->max_vcpus * MAPCACHE_VCPU_ENTRIES;
>      unsigned int nr = PFN_UP(BITS_TO_LONGS(ents) * sizeof(long));
>  
> -    if ( is_hvm_vcpu(v) || !dcache->inuse )
> +    if ( !is_pv_vcpu(v) || !dcache->inuse )
>          return 0;
>  
>      if ( ents > dcache->entries )
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index ce32245..8b44061 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -1305,6 +1305,12 @@ void arch_get_info_guest(struct vcpu *v, vcpu_guest_context_u c)
>              c.nat->gs_base_kernel = hvm_get_shadow_gs_base(v);
>          }
>      }
> +    else if ( is_pvh_vcpu(v) )
> +    {
> +        /* pvh fixme: punt it to phase II */
> +        printk(XENLOG_WARNING "PVH: fixme: arch_get_info_guest()\n");
> +        memset(c.nat, 0, sizeof(*c.nat));
> +    }
>      else
>      {
>          c(ldt_base = v->arch.pv_vcpu.ldt_base);
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index b190ad9..e992b4f 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -2805,7 +2805,7 @@ static struct domain *get_pg_owner(domid_t domid)
>          goto out;
>      }
>  
> -    if ( unlikely(paging_mode_translate(curr)) )
> +    if ( !is_pvh_domain(curr) && unlikely(paging_mode_translate(curr)) )
>      {
>          MEM_LOG("Cannot mix foreign mappings with translated domains");
>          goto out;
> diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
> index 3733c7a..2fc7ae6 100644
> --- a/xen/arch/x86/physdev.c
> +++ b/xen/arch/x86/physdev.c
> @@ -475,6 +475,13 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>  
>      case PHYSDEVOP_set_iopl: {
>          struct physdev_set_iopl set_iopl;
> +
> +        if ( is_pvh_vcpu(current) )
> +        {
> +            ret = -EINVAL;
> +            break;
> +        }
> +
>          ret = -EFAULT;
>          if ( copy_from_guest(&set_iopl, arg, 1) != 0 )
>              break;
> @@ -488,6 +495,12 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>  
>      case PHYSDEVOP_set_iobitmap: {
>          struct physdev_set_iobitmap set_iobitmap;
> +
> +        if ( is_pvh_vcpu(current) )
> +        {
> +            ret = -EINVAL;
> +            break;
> +        }
>          ret = -EFAULT;
>          if ( copy_from_guest(&set_iobitmap, arg, 1) != 0 )
>              break;
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index 3f97328..a2073d2 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -721,7 +721,7 @@ __gnttab_map_grant_ref(
>  
>      double_gt_lock(lgt, rgt);
>  
> -    if ( !is_hvm_domain(ld) && need_iommu(ld) )
> +    if ( is_pv_domain(ld) && need_iommu(ld) )
>      {
>          unsigned int wrc, rdc;
>          int err = 0;
> @@ -932,7 +932,7 @@ __gnttab_unmap_common(
>              act->pin -= GNTPIN_hstw_inc;
>      }
>  
> -    if ( !is_hvm_domain(ld) && need_iommu(ld) )
> +    if ( is_pv_domain(ld) && need_iommu(ld) )
>      {
>          unsigned int wrc, rdc;
>          int err = 0;
> -- 
> 1.7.2.3
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>