diff mbox

[V3,18/29] VIOMMU: Add irq request callback to deal with irq remapping

Message ID 1506049330-11196-19-git-send-email-tianyu.lan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

lan,Tianyu Sept. 22, 2017, 3:01 a.m. UTC
This patch is to add irq request callback for platform implementation
to deal with irq remapping request.

Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 xen/common/viommu.c          | 15 +++++++++
 xen/include/asm-x86/viommu.h | 72 ++++++++++++++++++++++++++++++++++++++++++++
 xen/include/xen/viommu.h     | 11 +++++++
 3 files changed, 98 insertions(+)
 create mode 100644 xen/include/asm-x86/viommu.h

Comments

Roger Pau Monné Oct. 19, 2017, 3 p.m. UTC | #1
On Thu, Sep 21, 2017 at 11:01:59PM -0400, Lan Tianyu wrote:
> This patch is to add irq request callback for platform implementation
> to deal with irq remapping request.
> 
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
>  xen/common/viommu.c          | 15 +++++++++
>  xen/include/asm-x86/viommu.h | 72 ++++++++++++++++++++++++++++++++++++++++++++
>  xen/include/xen/viommu.h     | 11 +++++++
>  3 files changed, 98 insertions(+)
>  create mode 100644 xen/include/asm-x86/viommu.h
> 
> diff --git a/xen/common/viommu.c b/xen/common/viommu.c
> index 55feb5d..b517158 100644
> --- a/xen/common/viommu.c
> +++ b/xen/common/viommu.c
> @@ -163,6 +163,21 @@ int viommu_domctl(struct domain *d, struct xen_domctl_viommu_op *op,
>      return rc;
>  }
>  
> +int viommu_handle_irq_request(struct domain *d,
> +                              struct arch_irq_remapping_request *request)
> +{
> +    struct viommu *viommu = d->viommu;
> +
> +    if ( !viommu )
> +        return -EINVAL;

ENODEV

> +
> +    ASSERT(viommu->ops);
> +    if ( !viommu->ops->handle_irq_request )
> +        return -EINVAL;
> +
> +    return viommu->ops->handle_irq_request(d, request);
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/include/asm-x86/viommu.h b/xen/include/asm-x86/viommu.h
> new file mode 100644
> index 0000000..366fbb6
> --- /dev/null
> +++ b/xen/include/asm-x86/viommu.h
> @@ -0,0 +1,72 @@
> +/*
> + * include/asm-x86/viommu.h
> + *
> + * Copyright (c) 2017 Intel Corporation.
> + * Author: Lan Tianyu <tianyu.lan@intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program; If not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +#ifndef __ARCH_X86_VIOMMU_H__
> +#define __ARCH_X86_VIOMMU_H__
> +
> +/* IRQ request type */
> +#define VIOMMU_REQUEST_IRQ_MSI          0
> +#define VIOMMU_REQUEST_IRQ_APIC         1
> +
> +struct arch_irq_remapping_request

Oh, so you have been using arch_irq_remapping_request in previous
patches without it being introduced. This is becoming more and more
hard to review. I will try to finish reviewing the whole series but
please, in the future make sure that each patch compiles on it's
own.

It's impossible to properly review a series when you use a structure
that has not yet been introduced.

> +{
> +    union {
> +        /* MSI */
> +        struct {
> +            uint64_t addr;
> +            uint32_t data;
> +        } msi;
> +        /* Redirection Entry in IOAPIC */
> +        uint64_t rte;
> +    } msg;
> +    uint16_t source_id;
> +    uint8_t type;

Why don't you make this an enum?

> +};
> +
> +static inline void irq_request_ioapic_fill(struct arch_irq_remapping_request *req,
> +                                           uint32_t ioapic_id, uint64_t rte)
> +{
> +    ASSERT(req);
> +    req->type = VIOMMU_REQUEST_IRQ_APIC;
> +    req->source_id = ioapic_id;
> +    req->msg.rte = rte;
> +}
> +
> +static inline void irq_request_msi_fill(struct arch_irq_remapping_request *req,
> +                                        uint32_t source_id, uint64_t addr,
> +                                        uint32_t data)
> +{
> +    ASSERT(req);
> +    req->type = VIOMMU_REQUEST_IRQ_MSI;
> +    req->source_id = source_id;
> +    req->msg.msi.addr = addr;
> +    req->msg.msi.data = data;
> +}

You are introducing two functions here that are not used in this
patch. They should be added when they are used, or else it's very hard
to review.

Thanks, Roger.
diff mbox

Patch

diff --git a/xen/common/viommu.c b/xen/common/viommu.c
index 55feb5d..b517158 100644
--- a/xen/common/viommu.c
+++ b/xen/common/viommu.c
@@ -163,6 +163,21 @@  int viommu_domctl(struct domain *d, struct xen_domctl_viommu_op *op,
     return rc;
 }
 
+int viommu_handle_irq_request(struct domain *d,
+                              struct arch_irq_remapping_request *request)
+{
+    struct viommu *viommu = d->viommu;
+
+    if ( !viommu )
+        return -EINVAL;
+
+    ASSERT(viommu->ops);
+    if ( !viommu->ops->handle_irq_request )
+        return -EINVAL;
+
+    return viommu->ops->handle_irq_request(d, request);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/asm-x86/viommu.h b/xen/include/asm-x86/viommu.h
new file mode 100644
index 0000000..366fbb6
--- /dev/null
+++ b/xen/include/asm-x86/viommu.h
@@ -0,0 +1,72 @@ 
+/*
+ * include/asm-x86/viommu.h
+ *
+ * Copyright (c) 2017 Intel Corporation.
+ * Author: Lan Tianyu <tianyu.lan@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+#ifndef __ARCH_X86_VIOMMU_H__
+#define __ARCH_X86_VIOMMU_H__
+
+/* IRQ request type */
+#define VIOMMU_REQUEST_IRQ_MSI          0
+#define VIOMMU_REQUEST_IRQ_APIC         1
+
+struct arch_irq_remapping_request
+{
+    union {
+        /* MSI */
+        struct {
+            uint64_t addr;
+            uint32_t data;
+        } msi;
+        /* Redirection Entry in IOAPIC */
+        uint64_t rte;
+    } msg;
+    uint16_t source_id;
+    uint8_t type;
+};
+
+static inline void irq_request_ioapic_fill(struct arch_irq_remapping_request *req,
+                                           uint32_t ioapic_id, uint64_t rte)
+{
+    ASSERT(req);
+    req->type = VIOMMU_REQUEST_IRQ_APIC;
+    req->source_id = ioapic_id;
+    req->msg.rte = rte;
+}
+
+static inline void irq_request_msi_fill(struct arch_irq_remapping_request *req,
+                                        uint32_t source_id, uint64_t addr,
+                                        uint32_t data)
+{
+    ASSERT(req);
+    req->type = VIOMMU_REQUEST_IRQ_MSI;
+    req->source_id = source_id;
+    req->msg.msi.addr = addr;
+    req->msg.msi.data = data;
+}
+
+#endif /* __ARCH_X86_VIOMMU_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/xen/viommu.h b/xen/include/xen/viommu.h
index baa8ab7..230f6b1 100644
--- a/xen/include/xen/viommu.h
+++ b/xen/include/xen/viommu.h
@@ -21,10 +21,13 @@ 
 #define __XEN_VIOMMU_H__
 
 struct viommu;
+struct arch_irq_remapping_request;
 
 struct viommu_ops {
     int (*create)(struct domain *d, struct viommu *viommu);
     int (*destroy)(struct viommu *viommu);
+    int (*handle_irq_request)(struct domain *d,
+                              struct arch_irq_remapping_request *request);
 };
 
 struct viommu {
@@ -45,11 +48,19 @@  int viommu_register_type(uint64_t type, struct viommu_ops *ops);
 int viommu_destroy_domain(struct domain *d);
 int viommu_domctl(struct domain *d, struct xen_domctl_viommu_op *op,
                   bool_t *need_copy);
+int viommu_handle_irq_request(struct domain *d,
+                              struct arch_irq_remapping_request *request);
 #else
 static inline int viommu_register_type(uint64_t type, struct viommu_ops *ops)
 {
     return -EINVAL;
 }
+static inline int
+viommu_handle_irq_request(struct domain *d,
+                          struct arch_irq_remapping_request *request)
+{
+    return -EINVAL;
+}
 #endif
 
 #endif /* __XEN_VIOMMU_H__ */