[PATCH v2 0/2] x86/io: rename dpci_ IO port handlers

[PATCH v2 0/2] x86/io: rename dpci_ IO port handlers

[Roger Pau Monne]

Hello,

The following series contain two cosmetic fixes for the g2m IO port handlers.
The first one renames them to use the g2m_ prefix instead of the dpci_ one,
since those handlers are not trapping the PCI config space in any way. The
second fix moves the list of ports from the domain_iommu struct to the
hvm_struct, where it fits more naturally.

Thanks, Roger.


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

[PATCH v2 1/2] x86/io: rename misleading dpci_ prefixed functions to g2m_

[Roger Pau Monne]

The dpci_ prefix used on those IO handlers is misleading, there's nothing PCI
specific in them, they simply map a guest IO port into a machine (physical) IO
port. They don't specifically trap the PCI IO port range in any way
(0xcf8/0xcfc).

Rename them to use the g2m_ prefix in order to avoid this confusion.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Paul Durrant <paul.durrant@citrix.com>
---
 xen/arch/x86/hvm/hvm.c       |  2 +-
 xen/arch/x86/hvm/io.c        | 28 ++++++++++++----------------
 xen/include/asm-x86/hvm/io.h |  6 +++++-
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 0282986..3a2d001 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -641,7 +641,7 @@ int hvm_domain_initialise(struct domain *d)
     else
         d->arch.hvm_domain.io_bitmap = hvm_io_bitmap;
 
-    register_dpci_portio_handler(d);
+    register_g2m_portio_handler(d);
 
     hvm_ioreq_init(d);
 
diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
index 2770ff4..3b3a600 100644
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -167,8 +167,8 @@ bool handle_pio(uint16_t port, unsigned int size, int dir)
     return true;
 }
 
-static bool_t dpci_portio_accept(const struct hvm_io_handler *handler,
-                                 const ioreq_t *p)
+static bool_t g2m_portio_accept(const struct hvm_io_handler *handler,
+                                const ioreq_t *p)
 {
     struct vcpu *curr = current;
     const struct domain_iommu *dio = dom_iommu(curr->domain);
@@ -190,10 +190,8 @@ static bool_t dpci_portio_accept(const struct hvm_io_handler *handler,
     return 0;
 }
 
-static int dpci_portio_read(const struct hvm_io_handler *handler,
-                            uint64_t addr,
-                            uint32_t size,
-                            uint64_t *data)
+static int g2m_portio_read(const struct hvm_io_handler *handler,
+                           uint64_t addr, uint32_t size, uint64_t *data)
 {
     struct hvm_vcpu_io *vio = &current->arch.hvm_vcpu.hvm_io;
     const struct g2m_ioport *g2m_ioport = vio->g2m_ioport;
@@ -217,10 +215,8 @@ static int dpci_portio_read(const struct hvm_io_handler *handler,
     return X86EMUL_OKAY;
 }
 
-static int dpci_portio_write(const struct hvm_io_handler *handler,
-                             uint64_t addr,
-                             uint32_t size,
-                             uint64_t data)
+static int g2m_portio_write(const struct hvm_io_handler *handler,
+                            uint64_t addr, uint32_t size, uint64_t data)
 {
     struct hvm_vcpu_io *vio = &current->arch.hvm_vcpu.hvm_io;
     const struct g2m_ioport *g2m_ioport = vio->g2m_ioport;
@@ -244,13 +240,13 @@ static int dpci_portio_write(const struct hvm_io_handler *handler,
     return X86EMUL_OKAY;
 }
 
-static const struct hvm_io_ops dpci_portio_ops = {
-    .accept = dpci_portio_accept,
-    .read = dpci_portio_read,
-    .write = dpci_portio_write
+static const struct hvm_io_ops g2m_portio_ops = {
+    .accept = g2m_portio_accept,
+    .read = g2m_portio_read,
+    .write = g2m_portio_write
 };
 
-void register_dpci_portio_handler(struct domain *d)
+void register_g2m_portio_handler(struct domain *d)
 {
     struct hvm_io_handler *handler = hvm_next_io_handler(d);
 
@@ -258,7 +254,7 @@ void register_dpci_portio_handler(struct domain *d)
         return;
 
     handler->type = IOREQ_TYPE_PIO;
-    handler->ops = &dpci_portio_ops;
+    handler->ops = &g2m_portio_ops;
 }
 
 /*
diff --git a/xen/include/asm-x86/hvm/io.h b/xen/include/asm-x86/hvm/io.h
index d6801c1..5ae9225 100644
--- a/xen/include/asm-x86/hvm/io.h
+++ b/xen/include/asm-x86/hvm/io.h
@@ -148,7 +148,11 @@ void stdvga_deinit(struct domain *d);
 
 extern void hvm_dpci_msi_eoi(struct domain *d, int vector);
 
-void register_dpci_portio_handler(struct domain *d);
+/*
+ * HVM port IO handler that performs forwarding of guest IO ports into machine
+ * IO ports.
+ */
+void register_g2m_portio_handler(struct domain *d);
 
 #endif /* __ASM_X86_HVM_IO_H__ */
 
-- 
2.10.1 (Apple Git-78)


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

Re: [PATCH v2 1/2] x86/io: rename misleading dpci_ prefixed functions to g2m_

[Paul Durrant]

> -----Original Message-----
> From: Roger Pau Monne
> Sent: 28 March 2017 14:12
> To: xen-devel@lists.xenproject.org
> Cc: Roger Pau Monne <roger.pau@citrix.com>; Jan Beulich
> <jbeulich@suse.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>; Paul
> Durrant <Paul.Durrant@citrix.com>
> Subject: [PATCH v2 1/2] x86/io: rename misleading dpci_ prefixed functions
> to g2m_
> 
> The dpci_ prefix used on those IO handlers is misleading, there's nothing PCI
> specific in them, they simply map a guest IO port into a machine (physical) IO
> port. They don't specifically trap the PCI IO port range in any way
> (0xcf8/0xcfc).
> 
> Rename them to use the g2m_ prefix in order to avoid this confusion.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Paul Durrant <paul.durrant@citrix.com>

> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Paul Durrant <paul.durrant@citrix.com>
> ---
>  xen/arch/x86/hvm/hvm.c       |  2 +-
>  xen/arch/x86/hvm/io.c        | 28 ++++++++++++----------------
>  xen/include/asm-x86/hvm/io.h |  6 +++++-
>  3 files changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 0282986..3a2d001 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -641,7 +641,7 @@ int hvm_domain_initialise(struct domain *d)
>      else
>          d->arch.hvm_domain.io_bitmap = hvm_io_bitmap;
> 
> -    register_dpci_portio_handler(d);
> +    register_g2m_portio_handler(d);
> 
>      hvm_ioreq_init(d);
> 
> diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
> index 2770ff4..3b3a600 100644
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -167,8 +167,8 @@ bool handle_pio(uint16_t port, unsigned int size, int
> dir)
>      return true;
>  }
> 
> -static bool_t dpci_portio_accept(const struct hvm_io_handler *handler,
> -                                 const ioreq_t *p)
> +static bool_t g2m_portio_accept(const struct hvm_io_handler *handler,
> +                                const ioreq_t *p)
>  {
>      struct vcpu *curr = current;
>      const struct domain_iommu *dio = dom_iommu(curr->domain);
> @@ -190,10 +190,8 @@ static bool_t dpci_portio_accept(const struct
> hvm_io_handler *handler,
>      return 0;
>  }
> 
> -static int dpci_portio_read(const struct hvm_io_handler *handler,
> -                            uint64_t addr,
> -                            uint32_t size,
> -                            uint64_t *data)
> +static int g2m_portio_read(const struct hvm_io_handler *handler,
> +                           uint64_t addr, uint32_t size, uint64_t *data)
>  {
>      struct hvm_vcpu_io *vio = &current->arch.hvm_vcpu.hvm_io;
>      const struct g2m_ioport *g2m_ioport = vio->g2m_ioport;
> @@ -217,10 +215,8 @@ static int dpci_portio_read(const struct
> hvm_io_handler *handler,
>      return X86EMUL_OKAY;
>  }
> 
> -static int dpci_portio_write(const struct hvm_io_handler *handler,
> -                             uint64_t addr,
> -                             uint32_t size,
> -                             uint64_t data)
> +static int g2m_portio_write(const struct hvm_io_handler *handler,
> +                            uint64_t addr, uint32_t size, uint64_t data)
>  {
>      struct hvm_vcpu_io *vio = &current->arch.hvm_vcpu.hvm_io;
>      const struct g2m_ioport *g2m_ioport = vio->g2m_ioport;
> @@ -244,13 +240,13 @@ static int dpci_portio_write(const struct
> hvm_io_handler *handler,
>      return X86EMUL_OKAY;
>  }
> 
> -static const struct hvm_io_ops dpci_portio_ops = {
> -    .accept = dpci_portio_accept,
> -    .read = dpci_portio_read,
> -    .write = dpci_portio_write
> +static const struct hvm_io_ops g2m_portio_ops = {
> +    .accept = g2m_portio_accept,
> +    .read = g2m_portio_read,
> +    .write = g2m_portio_write
>  };
> 
> -void register_dpci_portio_handler(struct domain *d)
> +void register_g2m_portio_handler(struct domain *d)
>  {
>      struct hvm_io_handler *handler = hvm_next_io_handler(d);
> 
> @@ -258,7 +254,7 @@ void register_dpci_portio_handler(struct domain *d)
>          return;
> 
>      handler->type = IOREQ_TYPE_PIO;
> -    handler->ops = &dpci_portio_ops;
> +    handler->ops = &g2m_portio_ops;
>  }
> 
>  /*
> diff --git a/xen/include/asm-x86/hvm/io.h b/xen/include/asm-x86/hvm/io.h
> index d6801c1..5ae9225 100644
> --- a/xen/include/asm-x86/hvm/io.h
> +++ b/xen/include/asm-x86/hvm/io.h
> @@ -148,7 +148,11 @@ void stdvga_deinit(struct domain *d);
> 
>  extern void hvm_dpci_msi_eoi(struct domain *d, int vector);
> 
> -void register_dpci_portio_handler(struct domain *d);
> +/*
> + * HVM port IO handler that performs forwarding of guest IO ports into
> machine
> + * IO ports.
> + */
> +void register_g2m_portio_handler(struct domain *d);
> 
>  #endif /* __ASM_X86_HVM_IO_H__ */
> 
> --
> 2.10.1 (Apple Git-78)

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

[PATCH v2 2/2] x86/io: move the list of guest to machine IO ports out of domain_iommu

[Roger Pau Monne]

There's no reason to store that list inside of the domain_iommu struct, the
forwarding of guest IO ports into machine IO ports is not tied to the presence
of an IOMMU.

Move it inside of the hvm_domain struct instead.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Paul Durrant <paul.durrant@citrix.com>
---
 xen/arch/x86/domctl.c               | 10 +++++-----
 xen/arch/x86/hvm/hvm.c              | 12 ++++++++++++
 xen/arch/x86/hvm/io.c               |  4 ++--
 xen/drivers/passthrough/x86/iommu.c | 11 -----------
 xen/include/asm-x86/hvm/domain.h    |  3 +++
 xen/include/asm-x86/iommu.h         |  1 -
 6 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 1220224..b7b8748 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -721,11 +721,11 @@ long arch_do_domctl(
 
     case XEN_DOMCTL_ioport_mapping:
     {
-        struct domain_iommu *hd;
         unsigned int fgp = domctl->u.ioport_mapping.first_gport;
         unsigned int fmp = domctl->u.ioport_mapping.first_mport;
         unsigned int np = domctl->u.ioport_mapping.nr_ports;
         unsigned int add = domctl->u.ioport_mapping.add_mapping;
+        struct hvm_domain *hvm_domain;
         struct g2m_ioport *g2m_ioport;
         int found = 0;
 
@@ -747,14 +747,14 @@ long arch_do_domctl(
         if ( ret )
             break;
 
-        hd = dom_iommu(d);
+        hvm_domain = &d->arch.hvm_domain;
         if ( add )
         {
             printk(XENLOG_G_INFO
                    "ioport_map:add: dom%d gport=%x mport=%x nr=%x\n",
                    d->domain_id, fgp, fmp, np);
 
-            list_for_each_entry(g2m_ioport, &hd->arch.g2m_ioport_list, list)
+            list_for_each_entry(g2m_ioport, &hvm_domain->g2m_ioport_list, list)
                 if (g2m_ioport->mport == fmp )
                 {
                     g2m_ioport->gport = fgp;
@@ -773,7 +773,7 @@ long arch_do_domctl(
                 g2m_ioport->gport = fgp;
                 g2m_ioport->mport = fmp;
                 g2m_ioport->np = np;
-                list_add_tail(&g2m_ioport->list, &hd->arch.g2m_ioport_list);
+                list_add_tail(&g2m_ioport->list, &hvm_domain->g2m_ioport_list);
             }
             if ( !ret )
                 ret = ioports_permit_access(d, fmp, fmp + np - 1);
@@ -788,7 +788,7 @@ long arch_do_domctl(
             printk(XENLOG_G_INFO
                    "ioport_map:remove: dom%d gport=%x mport=%x nr=%x\n",
                    d->domain_id, fgp, fmp, np);
-            list_for_each_entry(g2m_ioport, &hd->arch.g2m_ioport_list, list)
+            list_for_each_entry(g2m_ioport, &hvm_domain->g2m_ioport_list, list)
                 if ( g2m_ioport->mport == fmp )
                 {
                     list_del(&g2m_ioport->list);
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 3a2d001..b6c5c9b 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -608,6 +608,7 @@ int hvm_domain_initialise(struct domain *d)
     spin_lock_init(&d->arch.hvm_domain.uc_lock);
     spin_lock_init(&d->arch.hvm_domain.write_map.lock);
     INIT_LIST_HEAD(&d->arch.hvm_domain.write_map.list);
+    INIT_LIST_HEAD(&d->arch.hvm_domain.g2m_ioport_list);
 
     hvm_init_cacheattr_region_list(d);
 
@@ -707,6 +708,9 @@ void hvm_domain_relinquish_resources(struct domain *d)
 
 void hvm_domain_destroy(struct domain *d)
 {
+    struct list_head *ioport_list, *tmp;
+    struct g2m_ioport *ioport;
+
     xfree(d->arch.hvm_domain.io_handler);
     d->arch.hvm_domain.io_handler = NULL;
 
@@ -722,6 +726,14 @@ void hvm_domain_destroy(struct domain *d)
 
     xfree(d->arch.hvm_domain.pl_time);
     d->arch.hvm_domain.pl_time = NULL;
+
+    list_for_each_safe ( ioport_list, tmp,
+                         &d->arch.hvm_domain.g2m_ioport_list )
+    {
+        ioport = list_entry(ioport_list, struct g2m_ioport, list);
+        list_del(&ioport->list);
+        xfree(ioport);
+    }
 }
 
 static int hvm_save_tsc_adjust(struct domain *d, hvm_domain_context_t *h)
diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
index 3b3a600..9e00409 100644
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -171,12 +171,12 @@ static bool_t g2m_portio_accept(const struct hvm_io_handler *handler,
                                 const ioreq_t *p)
 {
     struct vcpu *curr = current;
-    const struct domain_iommu *dio = dom_iommu(curr->domain);
+    const struct hvm_domain *hvm_domain = &curr->domain->arch.hvm_domain;
     struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
     struct g2m_ioport *g2m_ioport;
     unsigned int start, end;
 
-    list_for_each_entry( g2m_ioport, &dio->arch.g2m_ioport_list, list )
+    list_for_each_entry( g2m_ioport, &hvm_domain->g2m_ioport_list, list )
     {
         start = g2m_ioport->gport;
         end = start + g2m_ioport->np;
diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
index 750c663..0253823 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -123,7 +123,6 @@ int arch_iommu_domain_init(struct domain *d)
     struct domain_iommu *hd = dom_iommu(d);
 
     spin_lock_init(&hd->arch.mapping_lock);
-    INIT_LIST_HEAD(&hd->arch.g2m_ioport_list);
     INIT_LIST_HEAD(&hd->arch.mapped_rmrrs);
 
     return 0;
@@ -131,16 +130,6 @@ int arch_iommu_domain_init(struct domain *d)
 
 void arch_iommu_domain_destroy(struct domain *d)
 {
-    const struct domain_iommu *hd = dom_iommu(d);
-    struct list_head *ioport_list, *tmp;
-    struct g2m_ioport *ioport;
-
-    list_for_each_safe ( ioport_list, tmp, &hd->arch.g2m_ioport_list )
-    {
-        ioport = list_entry(ioport_list, struct g2m_ioport, list);
-        list_del(&ioport->list);
-        xfree(ioport);
-    }
 }
 
 /*
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index 420cbdc..bdfc082 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -180,6 +180,9 @@ struct hvm_domain {
 
     unsigned long *io_bitmap;
 
+    /* List of guest to machine IO ports mapping. */
+    struct list_head g2m_ioport_list;
+
     /* List of permanently write-mapped pages. */
     struct {
         spinlock_t lock;
diff --git a/xen/include/asm-x86/iommu.h b/xen/include/asm-x86/iommu.h
index be95106..0008505 100644
--- a/xen/include/asm-x86/iommu.h
+++ b/xen/include/asm-x86/iommu.h
@@ -34,7 +34,6 @@ struct arch_iommu
     u64 pgd_maddr;                 /* io page directory machine address */
     spinlock_t mapping_lock;            /* io page table lock */
     int agaw;     /* adjusted guest address width, 0 is level 2 30-bit */
-    struct list_head g2m_ioport_list;   /* guest to machine ioport mapping */
     u64 iommu_bitmap;              /* bitmap of iommu(s) that the domain uses */
     struct list_head mapped_rmrrs;
 
-- 
2.10.1 (Apple Git-78)


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

Re: [PATCH v2 2/2] x86/io: move the list of guest to machine IO ports out of domain_iommu

[Paul Durrant]

> -----Original Message-----
> From: Roger Pau Monne
> Sent: 28 March 2017 14:12
> To: xen-devel@lists.xenproject.org
> Cc: Roger Pau Monne <roger.pau@citrix.com>; Jan Beulich
> <jbeulich@suse.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>; Paul
> Durrant <Paul.Durrant@citrix.com>
> Subject: [PATCH v2 2/2] x86/io: move the list of guest to machine IO ports
> out of domain_iommu
> 
> There's no reason to store that list inside of the domain_iommu struct, the
> forwarding of guest IO ports into machine IO ports is not tied to the presence
> of an IOMMU.
> 
> Move it inside of the hvm_domain struct instead.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Paul Durrant <paul.durrant@citrix.com>

> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Paul Durrant <paul.durrant@citrix.com>
> ---
>  xen/arch/x86/domctl.c               | 10 +++++-----
>  xen/arch/x86/hvm/hvm.c              | 12 ++++++++++++
>  xen/arch/x86/hvm/io.c               |  4 ++--
>  xen/drivers/passthrough/x86/iommu.c | 11 -----------
>  xen/include/asm-x86/hvm/domain.h    |  3 +++
>  xen/include/asm-x86/iommu.h         |  1 -
>  6 files changed, 22 insertions(+), 19 deletions(-)
> 
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index 1220224..b7b8748 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -721,11 +721,11 @@ long arch_do_domctl(
> 
>      case XEN_DOMCTL_ioport_mapping:
>      {
> -        struct domain_iommu *hd;
>          unsigned int fgp = domctl->u.ioport_mapping.first_gport;
>          unsigned int fmp = domctl->u.ioport_mapping.first_mport;
>          unsigned int np = domctl->u.ioport_mapping.nr_ports;
>          unsigned int add = domctl->u.ioport_mapping.add_mapping;
> +        struct hvm_domain *hvm_domain;
>          struct g2m_ioport *g2m_ioport;
>          int found = 0;
> 
> @@ -747,14 +747,14 @@ long arch_do_domctl(
>          if ( ret )
>              break;
> 
> -        hd = dom_iommu(d);
> +        hvm_domain = &d->arch.hvm_domain;
>          if ( add )
>          {
>              printk(XENLOG_G_INFO
>                     "ioport_map:add: dom%d gport=%x mport=%x nr=%x\n",
>                     d->domain_id, fgp, fmp, np);
> 
> -            list_for_each_entry(g2m_ioport, &hd->arch.g2m_ioport_list, list)
> +            list_for_each_entry(g2m_ioport, &hvm_domain->g2m_ioport_list,
> list)
>                  if (g2m_ioport->mport == fmp )
>                  {
>                      g2m_ioport->gport = fgp;
> @@ -773,7 +773,7 @@ long arch_do_domctl(
>                  g2m_ioport->gport = fgp;
>                  g2m_ioport->mport = fmp;
>                  g2m_ioport->np = np;
> -                list_add_tail(&g2m_ioport->list, &hd->arch.g2m_ioport_list);
> +                list_add_tail(&g2m_ioport->list, &hvm_domain->g2m_ioport_list);
>              }
>              if ( !ret )
>                  ret = ioports_permit_access(d, fmp, fmp + np - 1);
> @@ -788,7 +788,7 @@ long arch_do_domctl(
>              printk(XENLOG_G_INFO
>                     "ioport_map:remove: dom%d gport=%x mport=%x nr=%x\n",
>                     d->domain_id, fgp, fmp, np);
> -            list_for_each_entry(g2m_ioport, &hd->arch.g2m_ioport_list, list)
> +            list_for_each_entry(g2m_ioport, &hvm_domain->g2m_ioport_list,
> list)
>                  if ( g2m_ioport->mport == fmp )
>                  {
>                      list_del(&g2m_ioport->list);
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 3a2d001..b6c5c9b 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -608,6 +608,7 @@ int hvm_domain_initialise(struct domain *d)
>      spin_lock_init(&d->arch.hvm_domain.uc_lock);
>      spin_lock_init(&d->arch.hvm_domain.write_map.lock);
>      INIT_LIST_HEAD(&d->arch.hvm_domain.write_map.list);
> +    INIT_LIST_HEAD(&d->arch.hvm_domain.g2m_ioport_list);
> 
>      hvm_init_cacheattr_region_list(d);
> 
> @@ -707,6 +708,9 @@ void hvm_domain_relinquish_resources(struct
> domain *d)
> 
>  void hvm_domain_destroy(struct domain *d)
>  {
> +    struct list_head *ioport_list, *tmp;
> +    struct g2m_ioport *ioport;
> +
>      xfree(d->arch.hvm_domain.io_handler);
>      d->arch.hvm_domain.io_handler = NULL;
> 
> @@ -722,6 +726,14 @@ void hvm_domain_destroy(struct domain *d)
> 
>      xfree(d->arch.hvm_domain.pl_time);
>      d->arch.hvm_domain.pl_time = NULL;
> +
> +    list_for_each_safe ( ioport_list, tmp,
> +                         &d->arch.hvm_domain.g2m_ioport_list )
> +    {
> +        ioport = list_entry(ioport_list, struct g2m_ioport, list);
> +        list_del(&ioport->list);
> +        xfree(ioport);
> +    }
>  }
> 
>  static int hvm_save_tsc_adjust(struct domain *d, hvm_domain_context_t
> *h)
> diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
> index 3b3a600..9e00409 100644
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -171,12 +171,12 @@ static bool_t g2m_portio_accept(const struct
> hvm_io_handler *handler,
>                                  const ioreq_t *p)
>  {
>      struct vcpu *curr = current;
> -    const struct domain_iommu *dio = dom_iommu(curr->domain);
> +    const struct hvm_domain *hvm_domain = &curr->domain-
> >arch.hvm_domain;
>      struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
>      struct g2m_ioport *g2m_ioport;
>      unsigned int start, end;
> 
> -    list_for_each_entry( g2m_ioport, &dio->arch.g2m_ioport_list, list )
> +    list_for_each_entry( g2m_ioport, &hvm_domain->g2m_ioport_list, list )
>      {
>          start = g2m_ioport->gport;
>          end = start + g2m_ioport->np;
> diff --git a/xen/drivers/passthrough/x86/iommu.c
> b/xen/drivers/passthrough/x86/iommu.c
> index 750c663..0253823 100644
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -123,7 +123,6 @@ int arch_iommu_domain_init(struct domain *d)
>      struct domain_iommu *hd = dom_iommu(d);
> 
>      spin_lock_init(&hd->arch.mapping_lock);
> -    INIT_LIST_HEAD(&hd->arch.g2m_ioport_list);
>      INIT_LIST_HEAD(&hd->arch.mapped_rmrrs);
> 
>      return 0;
> @@ -131,16 +130,6 @@ int arch_iommu_domain_init(struct domain *d)
> 
>  void arch_iommu_domain_destroy(struct domain *d)
>  {
> -    const struct domain_iommu *hd = dom_iommu(d);
> -    struct list_head *ioport_list, *tmp;
> -    struct g2m_ioport *ioport;
> -
> -    list_for_each_safe ( ioport_list, tmp, &hd->arch.g2m_ioport_list )
> -    {
> -        ioport = list_entry(ioport_list, struct g2m_ioport, list);
> -        list_del(&ioport->list);
> -        xfree(ioport);
> -    }
>  }
> 
>  /*
> diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-
> x86/hvm/domain.h
> index 420cbdc..bdfc082 100644
> --- a/xen/include/asm-x86/hvm/domain.h
> +++ b/xen/include/asm-x86/hvm/domain.h
> @@ -180,6 +180,9 @@ struct hvm_domain {
> 
>      unsigned long *io_bitmap;
> 
> +    /* List of guest to machine IO ports mapping. */
> +    struct list_head g2m_ioport_list;
> +
>      /* List of permanently write-mapped pages. */
>      struct {
>          spinlock_t lock;
> diff --git a/xen/include/asm-x86/iommu.h b/xen/include/asm-x86/iommu.h
> index be95106..0008505 100644
> --- a/xen/include/asm-x86/iommu.h
> +++ b/xen/include/asm-x86/iommu.h
> @@ -34,7 +34,6 @@ struct arch_iommu
>      u64 pgd_maddr;                 /* io page directory machine address */
>      spinlock_t mapping_lock;            /* io page table lock */
>      int agaw;     /* adjusted guest address width, 0 is level 2 30-bit */
> -    struct list_head g2m_ioport_list;   /* guest to machine ioport mapping */
>      u64 iommu_bitmap;              /* bitmap of iommu(s) that the domain uses */
>      struct list_head mapped_rmrrs;
> 
> --
> 2.10.1 (Apple Git-78)

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

Re: [PATCH v2 2/2] x86/io: move the list of guest to machine IO ports out of domain_iommu

[Andrew Cooper]

On 28/03/17 14:12, Roger Pau Monne wrote:
> There's no reason to store that list inside of the domain_iommu struct, the
> forwarding of guest IO ports into machine IO ports is not tied to the presence
> of an IOMMU.
>
> Move it inside of the hvm_domain struct instead.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Actually, on second thoughts, I rescind my R-by.

This breaks PV guests, which must not use state in the hvm half of the
union.

~Andrew

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

Re: [PATCH v2 2/2] x86/io: move the list of guest to machine IO ports out of domain_iommu

[Roger Pau Monne]

On Wed, Mar 29, 2017 at 11:41:40AM +0100, Andrew Cooper wrote:
> On 28/03/17 14:12, Roger Pau Monne wrote:
> > There's no reason to store that list inside of the domain_iommu struct, the
> > forwarding of guest IO ports into machine IO ports is not tied to the presence
> > of an IOMMU.
> >
> > Move it inside of the hvm_domain struct instead.
> >
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Actually, on second thoughts, I rescind my R-by.
> 
> This breaks PV guests, which must not use state in the hvm half of the
> union.

I'm extremely confused now, AFAICT the g2m_ioport_list list is only used by the
g2m_portio_* handlers, and those handlers already make use of HVM only fields
(ie: hvm_vcpu_io).

Roger.

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

Re: [PATCH v2 2/2] x86/io: move the list of guest to machine IO ports out of domain_iommu

[Roger Pau Monne]

On Wed, Mar 29, 2017 at 11:49:53AM +0100, Roger Pau Monne wrote:
> On Wed, Mar 29, 2017 at 11:41:40AM +0100, Andrew Cooper wrote:
> > On 28/03/17 14:12, Roger Pau Monne wrote:
> > > There's no reason to store that list inside of the domain_iommu struct, the
> > > forwarding of guest IO ports into machine IO ports is not tied to the presence
> > > of an IOMMU.
> > >
> > > Move it inside of the hvm_domain struct instead.
> > >
> > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > 
> > Actually, on second thoughts, I rescind my R-by.
> > 
> > This breaks PV guests, which must not use state in the hvm half of the
> > union.
> 
> I'm extremely confused now, AFAICT the g2m_ioport_list list is only used by the
> g2m_portio_* handlers, and those handlers already make use of HVM only fields
> (ie: hvm_vcpu_io).

Can this be committed? I understand there's a previous regression here, but
this patches don't make it any worse AFAICT, and they clarify the nomenclature.

Thanks, Roger.

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

Re: [PATCH v2 2/2] x86/io: move the list of guest to machine IO ports out of domain_iommu

[Jan Beulich]

>>> On 04.04.17 at 13:06, <roger.pau@citrix.com> wrote:
> On Wed, Mar 29, 2017 at 11:49:53AM +0100, Roger Pau Monne wrote:
>> On Wed, Mar 29, 2017 at 11:41:40AM +0100, Andrew Cooper wrote:
>> > On 28/03/17 14:12, Roger Pau Monne wrote:
>> > > There's no reason to store that list inside of the domain_iommu struct, the
>> > > forwarding of guest IO ports into machine IO ports is not tied to the presence
>> > > of an IOMMU.
>> > >
>> > > Move it inside of the hvm_domain struct instead.
>> > >
>> > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> > 
>> > Actually, on second thoughts, I rescind my R-by.
>> > 
>> > This breaks PV guests, which must not use state in the hvm half of the
>> > union.
>> 
>> I'm extremely confused now, AFAICT the g2m_ioport_list list is only used by the
>> g2m_portio_* handlers, and those handlers already make use of HVM only fields
>> (ie: hvm_vcpu_io).
> 
> Can this be committed?

As said on irc, I think was posted after the last posting date, so you'd
need to obtain an ack from Julien.

> I understand there's a previous regression here, but
> this patches don't make it any worse AFAICT, and they clarify the 
> nomenclature.

I don't see the "previous regression": The handlers can be invoked
only for HVM guests. What is not HVM-specific is the domctl, and
there is where you introduce a regression.

Jan

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

Re: [PATCH v2 2/2] x86/io: move the list of guest to machine IO ports out of domain_iommu

[Andrew Cooper]

On 04/04/17 12:31, Jan Beulich wrote:
>>>> On 04.04.17 at 13:06, <roger.pau@citrix.com> wrote:
>> On Wed, Mar 29, 2017 at 11:49:53AM +0100, Roger Pau Monne wrote:
>>> On Wed, Mar 29, 2017 at 11:41:40AM +0100, Andrew Cooper wrote:
>>>> On 28/03/17 14:12, Roger Pau Monne wrote:
>>>>> There's no reason to store that list inside of the domain_iommu struct, the
>>>>> forwarding of guest IO ports into machine IO ports is not tied to the presence
>>>>> of an IOMMU.
>>>>>
>>>>> Move it inside of the hvm_domain struct instead.
>>>>>
>>>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>>> Actually, on second thoughts, I rescind my R-by.
>>>>
>>>> This breaks PV guests, which must not use state in the hvm half of the
>>>> union.
>>> I'm extremely confused now, AFAICT the g2m_ioport_list list is only used by the
>>> g2m_portio_* handlers, and those handlers already make use of HVM only fields
>>> (ie: hvm_vcpu_io).
>> Can this be committed?
> As said on irc, I think was posted after the last posting date, so you'd
> need to obtain an ack from Julien.
>
>> I understand there's a previous regression here, but
>> this patches don't make it any worse AFAICT, and they clarify the 
>> nomenclature.
> I don't see the "previous regression": The handlers can be invoked
> only for HVM guests. What is not HVM-specific is the domctl, and
> there is where you introduce a regression.

It is certainly the case that in the past, you could use this domctl to
pass ISA ports through to PV guests, and that definitely regressed as
some point in the past.  ISTR Ian (cc'd) fielded a question to this
effect from Debian.

This change definitely puts an HVM-only restriction in place, which is
wrong IMO.

~Andrew

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

Re: [PATCH v2 2/2] x86/io: move the list of guest to machine IO ports out of domain_iommu

[Jan Beulich]

>>> On 04.04.17 at 15:23, <andrew.cooper3@citrix.com> wrote:
> On 04/04/17 12:31, Jan Beulich wrote:
>>>>> On 04.04.17 at 13:06, <roger.pau@citrix.com> wrote:
>>> On Wed, Mar 29, 2017 at 11:49:53AM +0100, Roger Pau Monne wrote:
>>>> On Wed, Mar 29, 2017 at 11:41:40AM +0100, Andrew Cooper wrote:
>>>>> On 28/03/17 14:12, Roger Pau Monne wrote:
>>>>>> There's no reason to store that list inside of the domain_iommu struct, the
>>>>>> forwarding of guest IO ports into machine IO ports is not tied to the 
> presence
>>>>>> of an IOMMU.
>>>>>>
>>>>>> Move it inside of the hvm_domain struct instead.
>>>>>>
>>>>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>>>> Actually, on second thoughts, I rescind my R-by.
>>>>>
>>>>> This breaks PV guests, which must not use state in the hvm half of the
>>>>> union.
>>>> I'm extremely confused now, AFAICT the g2m_ioport_list list is only used by the
>>>> g2m_portio_* handlers, and those handlers already make use of HVM only fields
>>>> (ie: hvm_vcpu_io).
>>> Can this be committed?
>> As said on irc, I think was posted after the last posting date, so you'd
>> need to obtain an ack from Julien.
>>
>>> I understand there's a previous regression here, but
>>> this patches don't make it any worse AFAICT, and they clarify the 
>>> nomenclature.
>> I don't see the "previous regression": The handlers can be invoked
>> only for HVM guests. What is not HVM-specific is the domctl, and
>> there is where you introduce a regression.
> 
> It is certainly the case that in the past, you could use this domctl to
> pass ISA ports through to PV guests, and that definitely regressed as
> some point in the past.  ISTR Ian (cc'd) fielded a question to this
> effect from Debian.

How would that have worked? Making ports (in)accessible is done
via XEN_DOMCTL_ioport_permission. Going back to 3.2.3 I actually
find that what is now named dom_iommu() was domain_hvm_iommu()
back then, and it wasn't entirely mis-named (i.e. the field was in
arch.hvm_domain). So I think all that's needed is adding an
is_hvm_domain() to the domctl handling.

Jan

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

Re: [PATCH v2 2/2] x86/io: move the list of guest to machine IO ports out of domain_iommu

[Roger Pau Monne]

On Tue, Apr 04, 2017 at 07:39:29AM -0600, Jan Beulich wrote:
> >>> On 04.04.17 at 15:23, <andrew.cooper3@citrix.com> wrote:
> > On 04/04/17 12:31, Jan Beulich wrote:
> >>>>> On 04.04.17 at 13:06, <roger.pau@citrix.com> wrote:
> >>> On Wed, Mar 29, 2017 at 11:49:53AM +0100, Roger Pau Monne wrote:
> >>>> On Wed, Mar 29, 2017 at 11:41:40AM +0100, Andrew Cooper wrote:
> >>>>> On 28/03/17 14:12, Roger Pau Monne wrote:
> >>>>>> There's no reason to store that list inside of the domain_iommu struct, the
> >>>>>> forwarding of guest IO ports into machine IO ports is not tied to the 
> > presence
> >>>>>> of an IOMMU.
> >>>>>>
> >>>>>> Move it inside of the hvm_domain struct instead.
> >>>>>>
> >>>>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> >>>>> Actually, on second thoughts, I rescind my R-by.
> >>>>>
> >>>>> This breaks PV guests, which must not use state in the hvm half of the
> >>>>> union.
> >>>> I'm extremely confused now, AFAICT the g2m_ioport_list list is only used by the
> >>>> g2m_portio_* handlers, and those handlers already make use of HVM only fields
> >>>> (ie: hvm_vcpu_io).
> >>> Can this be committed?
> >> As said on irc, I think was posted after the last posting date, so you'd
> >> need to obtain an ack from Julien.
> >>
> >>> I understand there's a previous regression here, but
> >>> this patches don't make it any worse AFAICT, and they clarify the 
> >>> nomenclature.
> >> I don't see the "previous regression": The handlers can be invoked
> >> only for HVM guests. What is not HVM-specific is the domctl, and
> >> there is where you introduce a regression.
> > 
> > It is certainly the case that in the past, you could use this domctl to
> > pass ISA ports through to PV guests, and that definitely regressed as
> > some point in the past.  ISTR Ian (cc'd) fielded a question to this
> > effect from Debian.
> 
> How would that have worked? Making ports (in)accessible is done
> via XEN_DOMCTL_ioport_permission. Going back to 3.2.3 I actually
> find that what is now named dom_iommu() was domain_hvm_iommu()
> back then, and it wasn't entirely mis-named (i.e. the field was in
> arch.hvm_domain). So I think all that's needed is adding an
> is_hvm_domain() to the domctl handling.

I can either do that, or move the list to arch_domain instead of hvm_domain.
Either way it's not going to make it work for PV guests, but it's going to
prevent PV guests from writing to hvm_domain.

Roger.

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

Re: [PATCH v2 0/2] x86/io: rename dpci_ IO port handlers

[Andrew Cooper]

On 28/03/2017 14:12, Roger Pau Monne wrote:
> Hello,
>
> The following series contain two cosmetic fixes for the g2m IO port handlers.
> The first one renames them to use the g2m_ prefix instead of the dpci_ one,
> since those handlers are not trapping the PCI config space in any way. The
> second fix moves the list of ports from the domain_iommu struct to the
> hvm_struct, where it fits more naturally.

Both Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, although it
would be good to take the opportunity to s/bool_t/bool/ while committing.

~Andrew

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

Re: [PATCH v2 0/2] x86/io: rename dpci_ IO port handlers

[Roger Pau Monne]

On Tue, Mar 28, 2017 at 09:51:57PM +0100, Andrew Cooper wrote:
> On 28/03/2017 14:12, Roger Pau Monne wrote:
> > Hello,
> >
> > The following series contain two cosmetic fixes for the g2m IO port handlers.
> > The first one renames them to use the g2m_ prefix instead of the dpci_ one,
> > since those handlers are not trapping the PCI config space in any way. The
> > second fix moves the list of ports from the domain_iommu struct to the
> > hvm_struct, where it fits more naturally.
> 
> Both Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, although it
> would be good to take the opportunity to s/bool_t/bool/ while committing.

The accept handler (hvm_io_accept_t) expects a bool_t as return type. This can
be fixed also, but seems out of the scope of this patch.

Roger.

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