WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-ia64-devel

Re: [Xen-ia64-devel][PATCH][VTD]change ioports_permit_access interface

To: "Xu, Anthony" <anthony.xu@xxxxxxxxx>
Subject: Re: [Xen-ia64-devel][PATCH][VTD]change ioports_permit_access interface
From: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>
Date: Fri, 17 Oct 2008 15:55:07 +0900
Cc: xen-ia64-devel <xen-ia64-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Thu, 16 Oct 2008 23:55:12 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <F7C8A4D3A9905B45A80E4C194793FA6501AE5334A2@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-ia64-devel-request@lists.xensource.com?subject=help>
List-id: Discussion of the ia64 port of Xen <xen-ia64-devel.lists.xensource.com>
List-post: <mailto:xen-ia64-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-ia64-devel>, <mailto:xen-ia64-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-ia64-devel>, <mailto:xen-ia64-devel-request@lists.xensource.com?subject=unsubscribe>
References: <F7C8A4D3A9905B45A80E4C194793FA6501AE5334A2@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-ia64-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.6i
On Thu, Oct 16, 2008 at 07:37:51PM +0800, Xu, Anthony wrote:
> use VTD to assing device, guest port may not be equal to host port.
> Change ioports_permit_access interface
> add deassign_domain_mmio_page interface
> 
> Signed-off-by; Anthony Xu < anthony.xu@xxxxxxxxx >

Some comments.

- ioports_permit_access()
  x86 didn't change its prototype.
  Why does only ia64 need to change it?
  Or will x86 also change it?

  I suppose, it would be nice to map the port io area
  to arbitrary guest physical area.
  But I'm not sure how x86 will go with XEN_DOMCTL_ioport_mapping.

- deassign_domain_mmio_page()
  calling __assgin_domain_page() may result in page reference count
  leak because the corresponding p2m entry may be changed to another
  value.
  So you want to modify zap_domain_page_one() so that it accepts
  iomem page and call it from deassign_domain_mmio_page().

- probably it would better to split this patch into two ones.

thanks,

> use VTD to assing device, guest port may not be equal to host port.
> Change ioports_permit_access interface
> add deassign_domain_mmio_page interface
> 
> Signed-off-by; Anthony Xu < anthony.xu@xxxxxxxxx >
> 
> 
> 
> 
> 
> diff -r 42c7733c1a2a xen/arch/ia64/xen/dom0_ops.c
> --- a/xen/arch/ia64/xen/dom0_ops.c    Thu Oct 16 18:18:39 2008 +0800
> +++ b/xen/arch/ia64/xen/dom0_ops.c    Thu Oct 16 19:13:01 2008 +0800
> @@ -203,7 +203,7 @@
>              ret = 0;
>          else {
>              if (op->u.ioport_permission.allow_access)
> -                ret = ioports_permit_access(d, fp, lp);
> +                ret = ioports_permit_access(d, fp, fp, lp);
>              else
>                  ret = ioports_deny_access(d, fp, lp);
>          }
> @@ -522,7 +522,7 @@
>      fp = space_number << IO_SPACE_BITS;
>      lp = fp | 0xffff;
>  
> -    return ioports_permit_access(d, fp, lp);
> +    return ioports_permit_access(d, fp, fp, lp);
>  }
>  
>  unsigned long
> diff -r 42c7733c1a2a xen/arch/ia64/xen/domain.c
> --- a/xen/arch/ia64/xen/domain.c      Thu Oct 16 18:18:39 2008 +0800
> +++ b/xen/arch/ia64/xen/domain.c      Thu Oct 16 19:13:01 2008 +0800
> @@ -1995,7 +1995,7 @@
>               BUG();
>       if (irqs_permit_access(d, 0, NR_IRQS-1))
>               BUG();
> -     if (ioports_permit_access(d, 0, 0xffff))
> +     if (ioports_permit_access(d, 0, 0, 0xffff))
>               BUG();
>  }
>  
> diff -r 42c7733c1a2a xen/arch/ia64/xen/mm.c
> --- a/xen/arch/ia64/xen/mm.c  Thu Oct 16 18:18:39 2008 +0800
> +++ b/xen/arch/ia64/xen/mm.c  Thu Oct 16 19:13:01 2008 +0800
> @@ -984,15 +984,22 @@
>                                 ASSIGN_writable | ASSIGN_pgc_allocated);
>  }
>  
> +/* 
> + * Inpurt
> + * fgp: first guest port
> + * fmp: first machine port
> + * lmp: last machine port
> + */
>  int
> -ioports_permit_access(struct domain *d, unsigned int fp, unsigned int lp)
> +ioports_permit_access(struct domain *d, unsigned int fgp,
> +        unsigned int fmp, unsigned int lmp)
>  {
>      struct io_space *space;
> -    unsigned long mmio_start, mmio_end, mach_start;
> +    unsigned long mmio_start, mach_start, mach_end;
>      int ret;
>  
> -    if (IO_SPACE_NR(fp) >= num_io_spaces) {
> -        dprintk(XENLOG_WARNING, "Unknown I/O Port range 0x%x - 0x%x\n", fp, 
> lp);
> +    if (IO_SPACE_NR(fmp) >= num_io_spaces) {
> +        dprintk(XENLOG_WARNING, "Unknown I/O Port range 0x%x - 0x%x\n", fmp, 
> lmp);
>          return -EFAULT;
>      }
>  
> @@ -1006,42 +1013,44 @@
>       * I/O port spaces and thus will number port spaces differently.
>       * This is ok, they don't make use of this interface.
>       */
> -    ret = rangeset_add_range(d->arch.ioport_caps, fp, lp);
> +    ret = rangeset_add_range(d->arch.ioport_caps, fmp, lmp);
>      if (ret != 0)
>          return ret;
>  
> -    space = &io_space[IO_SPACE_NR(fp)];
> +    space = &io_space[IO_SPACE_NR(fmp)];
>  
>      /* Legacy I/O on dom0 is already setup */
>      if (d == dom0 && space == &io_space[0])
>          return 0;
>  
> -    fp = IO_SPACE_PORT(fp);
> -    lp = IO_SPACE_PORT(lp);
> +    fmp = IO_SPACE_PORT(fmp);
> +    lmp = IO_SPACE_PORT(lmp);
>  
>      if (space->sparse) {
> -        mmio_start = IO_SPACE_SPARSE_ENCODING(fp) & PAGE_MASK;
> -        mmio_end = PAGE_ALIGN(IO_SPACE_SPARSE_ENCODING(lp));
> +        mach_start = IO_SPACE_SPARSE_ENCODING(fmp) & PAGE_MASK;
> +        mach_end = PAGE_ALIGN(IO_SPACE_SPARSE_ENCODING(lmp));
>      } else {
> -        mmio_start = fp & PAGE_MASK;
> -        mmio_end = PAGE_ALIGN(lp);
> +        mach_start = fmp & PAGE_MASK;
> +        mach_end = PAGE_ALIGN(lmp);
>      }
>  
>      /*
>       * The "machine first port" is not necessarily identity mapped
>       * to the guest first port.  At least for the legacy range.
>       */
> -    mach_start = mmio_start | __pa(space->mmio_base);
> +    mach_start = mach_start | __pa(space->mmio_base);
> +    mach_end = mach_end | __pa(space->mmio_base);
>  
> -    if (space == &io_space[0]) {
> +    mmio_start = IO_SPACE_SPARSE_ENCODING(fgp) & PAGE_MASK;
> +
> +    if ( VMX_DOMAIN(d->vcpu[0]))
> +        mmio_start |= LEGACY_IO_START;
> +    else if (space == &io_space[0])
>          mmio_start |= IO_PORTS_PADDR;
> -        mmio_end |= IO_PORTS_PADDR;
> -    } else {
> +    else
>          mmio_start |= __pa(space->mmio_base);
> -        mmio_end |= __pa(space->mmio_base);
> -    }
>  
> -    while (mmio_start <= mmio_end) {
> +    while (mach_start < mach_end) {
>          __assign_domain_page(d, mmio_start,
>                  mach_start, ASSIGN_nocache | ASSIGN_direct_io); 
>          mmio_start += PAGE_SIZE;
> @@ -1091,7 +1100,9 @@
>          mmio_end = PAGE_ALIGN(lp_base);
>      }
>  
> -    if (space == &io_space[0] && d != dom0)
> +    if (VMX_DOMAIN(d->vcpu[0]))
> +        mmio_base = LEGACY_IO_START;
> +    else if (space == &io_space[0] && d != dom0)
>          mmio_base = IO_PORTS_PADDR;
>      else
>          mmio_base = __pa(space->mmio_base);
> @@ -1217,6 +1228,33 @@
>  
>      return mpaddr;
>  }
> +
> +int
> +deassign_domain_mmio_page(struct domain *d, unsigned long mpaddr,
> +        unsigned long phys_addr, unsigned long size )
> +{
> +    unsigned long addr = mpaddr & PAGE_MASK;
> +    unsigned long end = PAGE_ALIGN(mpaddr + size);
> +
> +    if (size == 0) {
> +        gdprintk(XENLOG_INFO, "%s: domain %p mpaddr 0x%lx size = 0x%lx\n",
> +                __func__, d, mpaddr, size);
> +    }
> +    if (!efi_mmio(phys_addr, size)) {
> +#ifndef NDEBUG
> +        gdprintk(XENLOG_INFO, "%s: domain %p mpaddr 0x%lx size = 0x%lx\n",
> +                __func__, d, mpaddr, size);
> +#endif
> +        return -EINVAL;
> +    }
> +
> +    for (; addr < end; addr += PAGE_SIZE ) {
> +        __assign_domain_page(d, addr, 0,
> +                ASSIGN_nocache | ASSIGN_direct_io);
> +    }
> +    return 0;
> +}
> +
>  
>  unsigned long
>  assign_domain_mach_page(struct domain *d,
> diff -r 42c7733c1a2a xen/include/asm-ia64/iocap.h
> --- a/xen/include/asm-ia64/iocap.h    Thu Oct 16 18:18:39 2008 +0800
> +++ b/xen/include/asm-ia64/iocap.h    Thu Oct 16 19:13:01 2008 +0800
> @@ -7,7 +7,7 @@
>  #ifndef __IA64_IOCAP_H__
>  #define __IA64_IOCAP_H__
>  
> -extern int ioports_permit_access(struct domain *d,
> +extern int ioports_permit_access(struct domain *d, unsigned int gs,
>                                unsigned int s, unsigned int e);
>  extern int ioports_deny_access(struct domain *d,
>                              unsigned int s, unsigned int e);
> 
> 

-- 
yamahata

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