diff mbox

[RFC,15/23] X86/vioapic: Hook interrupt delivery of vIOAPIC

Message ID 1489750043-17260-16-git-send-email-tianyu.lan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

lan,Tianyu March 17, 2017, 11:27 a.m. UTC
From: Chao Gao <chao.gao@intel.com>

When irq remapping enabled, IOAPIC Redirection Entry maybe is in remapping
format. If that, generate a irq_remapping_request and send it to domain.

Signed-off-by: Chao Gao <chao.gao@intel.com>
Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 xen/arch/x86/Makefile                  |  1 +
 xen/arch/x86/hvm/vioapic.c             | 10 ++++++++++
 xen/arch/x86/viommu.c                  | 30 ++++++++++++++++++++++++++++++
 xen/include/asm-x86/viommu.h           |  3 +++
 xen/include/public/arch-x86/hvm/save.h |  1 +
 5 files changed, 45 insertions(+)
 create mode 100644 xen/arch/x86/viommu.c

Comments

Konrad Rzeszutek Wilk April 17, 2017, 2:43 p.m. UTC | #1
On Fri, Mar 17, 2017 at 07:27:15PM +0800, Lan Tianyu wrote:
> From: Chao Gao <chao.gao@intel.com>
> 
> When irq remapping enabled, IOAPIC Redirection Entry maybe is in remapping
> format. If that, generate a irq_remapping_request and send it to domain.
> 
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
>  xen/arch/x86/Makefile                  |  1 +
>  xen/arch/x86/hvm/vioapic.c             | 10 ++++++++++
>  xen/arch/x86/viommu.c                  | 30 ++++++++++++++++++++++++++++++
>  xen/include/asm-x86/viommu.h           |  3 +++
>  xen/include/public/arch-x86/hvm/save.h |  1 +
>  5 files changed, 45 insertions(+)
>  create mode 100644 xen/arch/x86/viommu.c
> 
> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> index f75eca0..d49f8c8 100644
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -66,6 +66,7 @@ obj-y += usercopy.o
>  obj-y += x86_emulate.o
>  obj-$(CONFIG_TBOOT) += tboot.o
>  obj-y += hpet.o
> +obj-y += viommu.o
>  obj-y += vm_event.o
>  obj-y += xstate.o
>  
> diff --git a/xen/arch/x86/hvm/vioapic.c b/xen/arch/x86/hvm/vioapic.c
> index fdbb21f..6a00644 100644
> --- a/xen/arch/x86/hvm/vioapic.c
> +++ b/xen/arch/x86/hvm/vioapic.c
> @@ -30,6 +30,7 @@
>  #include <xen/lib.h>
>  #include <xen/errno.h>
>  #include <xen/sched.h>
> +#include <xen/viommu.h>
>  #include <public/hvm/ioreq.h>
>  #include <asm/hvm/io.h>
>  #include <asm/hvm/vpic.h>
> @@ -285,9 +286,18 @@ static void vioapic_deliver(struct hvm_hw_vioapic *vioapic, int irq)
>      struct domain *d = vioapic_domain(vioapic);
>      struct vlapic *target;
>      struct vcpu *v;
> +    struct irq_remapping_request request;
>  
>      ASSERT(spin_is_locked(&d->arch.hvm_domain.irq_lock));
>  
> +    if ( vioapic->redirtbl[irq].ir.format )
> +    {
> +        irq_request_ioapic_fill(&request, vioapic->id,
> +                                vioapic->redirtbl[irq].bits);
> +        viommu_handle_irq_request(d, &request);
> +        return;
> +    }
> +
>      HVM_DBG_LOG(DBG_LEVEL_IOAPIC,
>                  "dest=%x dest_mode=%x delivery_mode=%x "
>                  "vector=%x trig_mode=%x",
> diff --git a/xen/arch/x86/viommu.c b/xen/arch/x86/viommu.c
> new file mode 100644
> index 0000000..ef78d3b
> --- /dev/null
> +++ b/xen/arch/x86/viommu.c
> @@ -0,0 +1,30 @@
> +/*
> + * viommu.c
> + *
> + * virtualize IOMMU.
> + *
> + * Copyright (C) 2017 Chao Gao, Intel Corporation.
> + *
> + * 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 that 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/>.
> + */
> +
> +#include <xen/viommu.h>
> +
> +void irq_request_ioapic_fill(struct 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;

Considering we get 'req' from the stack and it may have garbage, would
it be good to fill out the rest of the entries with sensible values? Or
is there no need for that?
> +}

This being a new file, you should probably include the nice
editor configuration block.

> diff --git a/xen/include/asm-x86/viommu.h b/xen/include/asm-x86/viommu.h
> index 0b25f34..fcf3c24 100644
> --- a/xen/include/asm-x86/viommu.h
> +++ b/xen/include/asm-x86/viommu.h
> @@ -49,6 +49,9 @@ struct irq_remapping_request
>      } msg;
>  };
>  
> +void irq_request_ioapic_fill(struct irq_remapping_request *req,
> +                             uint32_t ioapic_id, uint64_t rte);
> +
>  static inline const struct viommu_ops *viommu_get_ops(void)
>  {
>      /*
> diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h
> index 6127f89..06be4a5 100644
> --- a/xen/include/public/arch-x86/hvm/save.h
> +++ b/xen/include/public/arch-x86/hvm/save.h
> @@ -401,6 +401,7 @@ struct hvm_hw_vioapic {
>              uint8_t reserved[4];
>              uint8_t dest_id;
>          } fields;
> +        struct ir_ioapic_rte ir;
>      } redirtbl[VIOAPIC_NUM_PINS];
>  };
>  
> -- 
> 1.8.3.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
lan,Tianyu April 18, 2017, 8:34 a.m. UTC | #2
On 2017年04月17日 22:43, Konrad Rzeszutek Wilk wrote:
> On Fri, Mar 17, 2017 at 07:27:15PM +0800, Lan Tianyu wrote:
>> From: Chao Gao <chao.gao@intel.com>
>>
>> When irq remapping enabled, IOAPIC Redirection Entry maybe is in remapping
>> format. If that, generate a irq_remapping_request and send it to domain.
>>
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
>> ---
>>  xen/arch/x86/Makefile                  |  1 +
>>  xen/arch/x86/hvm/vioapic.c             | 10 ++++++++++
>>  xen/arch/x86/viommu.c                  | 30 ++++++++++++++++++++++++++++++
>>  xen/include/asm-x86/viommu.h           |  3 +++
>>  xen/include/public/arch-x86/hvm/save.h |  1 +
>>  5 files changed, 45 insertions(+)
>>  create mode 100644 xen/arch/x86/viommu.c
>>
>> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
>> index f75eca0..d49f8c8 100644
>> --- a/xen/arch/x86/Makefile
>> +++ b/xen/arch/x86/Makefile
>> @@ -66,6 +66,7 @@ obj-y += usercopy.o
>>  obj-y += x86_emulate.o
>>  obj-$(CONFIG_TBOOT) += tboot.o
>>  obj-y += hpet.o
>> +obj-y += viommu.o
>>  obj-y += vm_event.o
>>  obj-y += xstate.o
>>  
>> diff --git a/xen/arch/x86/hvm/vioapic.c b/xen/arch/x86/hvm/vioapic.c
>> index fdbb21f..6a00644 100644
>> --- a/xen/arch/x86/hvm/vioapic.c
>> +++ b/xen/arch/x86/hvm/vioapic.c
>> @@ -30,6 +30,7 @@
>>  #include <xen/lib.h>
>>  #include <xen/errno.h>
>>  #include <xen/sched.h>
>> +#include <xen/viommu.h>
>>  #include <public/hvm/ioreq.h>
>>  #include <asm/hvm/io.h>
>>  #include <asm/hvm/vpic.h>
>> @@ -285,9 +286,18 @@ static void vioapic_deliver(struct hvm_hw_vioapic *vioapic, int irq)
>>      struct domain *d = vioapic_domain(vioapic);
>>      struct vlapic *target;
>>      struct vcpu *v;
>> +    struct irq_remapping_request request;
>>  
>>      ASSERT(spin_is_locked(&d->arch.hvm_domain.irq_lock));
>>  
>> +    if ( vioapic->redirtbl[irq].ir.format )
>> +    {
>> +        irq_request_ioapic_fill(&request, vioapic->id,
>> +                                vioapic->redirtbl[irq].bits);
>> +        viommu_handle_irq_request(d, &request);
>> +        return;
>> +    }
>> +
>>      HVM_DBG_LOG(DBG_LEVEL_IOAPIC,
>>                  "dest=%x dest_mode=%x delivery_mode=%x "
>>                  "vector=%x trig_mode=%x",
>> diff --git a/xen/arch/x86/viommu.c b/xen/arch/x86/viommu.c
>> new file mode 100644
>> index 0000000..ef78d3b
>> --- /dev/null
>> +++ b/xen/arch/x86/viommu.c
>> @@ -0,0 +1,30 @@
>> +/*
>> + * viommu.c
>> + *
>> + * virtualize IOMMU.
>> + *
>> + * Copyright (C) 2017 Chao Gao, Intel Corporation.
>> + *
>> + * 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 that 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/>.
>> + */
>> +
>> +#include <xen/viommu.h>
>> +
>> +void irq_request_ioapic_fill(struct 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;
> 
> Considering we get 'req' from the stack and it may have garbage, would
> it be good to fill out the rest of the entries with sensible values? Or
> is there no need for that?

Both AMD and Intel will use the function to pass interrupt remapping
request. I am afraid different vendors may have different IOAPIC
remapping format. How about to parse and check remapping request data in
the vendor vIOMMU device module(E,G vvtd)? :)

>> +}
> 
> This being a new file, you should probably include the nice
> editor configuration block.

OK. Will add it.

> 
>> diff --git a/xen/include/asm-x86/viommu.h b/xen/include/asm-x86/viommu.h
>> index 0b25f34..fcf3c24 100644
>> --- a/xen/include/asm-x86/viommu.h
>> +++ b/xen/include/asm-x86/viommu.h
>> @@ -49,6 +49,9 @@ struct irq_remapping_request
>>      } msg;
>>  };
>>  
>> +void irq_request_ioapic_fill(struct irq_remapping_request *req,
>> +                             uint32_t ioapic_id, uint64_t rte);
>> +
>>  static inline const struct viommu_ops *viommu_get_ops(void)
>>  {
>>      /*
>> diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h
>> index 6127f89..06be4a5 100644
>> --- a/xen/include/public/arch-x86/hvm/save.h
>> +++ b/xen/include/public/arch-x86/hvm/save.h
>> @@ -401,6 +401,7 @@ struct hvm_hw_vioapic {
>>              uint8_t reserved[4];
>>              uint8_t dest_id;
>>          } fields;
>> +        struct ir_ioapic_rte ir;
>>      } redirtbl[VIOAPIC_NUM_PINS];
>>  };
>>  
>> -- 
>> 1.8.3.1
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> https://lists.xen.org/xen-devel
Konrad Rzeszutek Wilk April 18, 2017, 1:37 p.m. UTC | #3
On Tue, Apr 18, 2017 at 04:34:52PM +0800, Lan Tianyu wrote:
> On 2017年04月17日 22:43, Konrad Rzeszutek Wilk wrote:
> > On Fri, Mar 17, 2017 at 07:27:15PM +0800, Lan Tianyu wrote:
> >> From: Chao Gao <chao.gao@intel.com>
> >>
> >> When irq remapping enabled, IOAPIC Redirection Entry maybe is in remapping
> >> format. If that, generate a irq_remapping_request and send it to domain.
> >>
> >> Signed-off-by: Chao Gao <chao.gao@intel.com>
> >> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> >> ---
> >>  xen/arch/x86/Makefile                  |  1 +
> >>  xen/arch/x86/hvm/vioapic.c             | 10 ++++++++++
> >>  xen/arch/x86/viommu.c                  | 30 ++++++++++++++++++++++++++++++
> >>  xen/include/asm-x86/viommu.h           |  3 +++
> >>  xen/include/public/arch-x86/hvm/save.h |  1 +
> >>  5 files changed, 45 insertions(+)
> >>  create mode 100644 xen/arch/x86/viommu.c
> >>
> >> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> >> index f75eca0..d49f8c8 100644
> >> --- a/xen/arch/x86/Makefile
> >> +++ b/xen/arch/x86/Makefile
> >> @@ -66,6 +66,7 @@ obj-y += usercopy.o
> >>  obj-y += x86_emulate.o
> >>  obj-$(CONFIG_TBOOT) += tboot.o
> >>  obj-y += hpet.o
> >> +obj-y += viommu.o
> >>  obj-y += vm_event.o
> >>  obj-y += xstate.o
> >>  
> >> diff --git a/xen/arch/x86/hvm/vioapic.c b/xen/arch/x86/hvm/vioapic.c
> >> index fdbb21f..6a00644 100644
> >> --- a/xen/arch/x86/hvm/vioapic.c
> >> +++ b/xen/arch/x86/hvm/vioapic.c
> >> @@ -30,6 +30,7 @@
> >>  #include <xen/lib.h>
> >>  #include <xen/errno.h>
> >>  #include <xen/sched.h>
> >> +#include <xen/viommu.h>
> >>  #include <public/hvm/ioreq.h>
> >>  #include <asm/hvm/io.h>
> >>  #include <asm/hvm/vpic.h>
> >> @@ -285,9 +286,18 @@ static void vioapic_deliver(struct hvm_hw_vioapic *vioapic, int irq)
> >>      struct domain *d = vioapic_domain(vioapic);
> >>      struct vlapic *target;
> >>      struct vcpu *v;
> >> +    struct irq_remapping_request request;
> >>  
> >>      ASSERT(spin_is_locked(&d->arch.hvm_domain.irq_lock));
> >>  
> >> +    if ( vioapic->redirtbl[irq].ir.format )
> >> +    {
> >> +        irq_request_ioapic_fill(&request, vioapic->id,
> >> +                                vioapic->redirtbl[irq].bits);
> >> +        viommu_handle_irq_request(d, &request);
> >> +        return;
> >> +    }
> >> +
> >>      HVM_DBG_LOG(DBG_LEVEL_IOAPIC,
> >>                  "dest=%x dest_mode=%x delivery_mode=%x "
> >>                  "vector=%x trig_mode=%x",
> >> diff --git a/xen/arch/x86/viommu.c b/xen/arch/x86/viommu.c
> >> new file mode 100644
> >> index 0000000..ef78d3b
> >> --- /dev/null
> >> +++ b/xen/arch/x86/viommu.c
> >> @@ -0,0 +1,30 @@
> >> +/*
> >> + * viommu.c
> >> + *
> >> + * virtualize IOMMU.
> >> + *
> >> + * Copyright (C) 2017 Chao Gao, Intel Corporation.
> >> + *
> >> + * 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 that 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/>.
> >> + */
> >> +
> >> +#include <xen/viommu.h>
> >> +
> >> +void irq_request_ioapic_fill(struct 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;
> > 
> > Considering we get 'req' from the stack and it may have garbage, would
> > it be good to fill out the rest of the entries with sensible values? Or
> > is there no need for that?
> 
> Both AMD and Intel will use the function to pass interrupt remapping
> request. I am afraid different vendors may have different IOAPIC
> remapping format. How about to parse and check remapping request data in
> the vendor vIOMMU device module(E,G vvtd)? :)

Yeah that is fine. Just please put the comment in the code saying
that is the justification - or alternatively put it in the
commit description.
diff mbox

Patch

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index f75eca0..d49f8c8 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -66,6 +66,7 @@  obj-y += usercopy.o
 obj-y += x86_emulate.o
 obj-$(CONFIG_TBOOT) += tboot.o
 obj-y += hpet.o
+obj-y += viommu.o
 obj-y += vm_event.o
 obj-y += xstate.o
 
diff --git a/xen/arch/x86/hvm/vioapic.c b/xen/arch/x86/hvm/vioapic.c
index fdbb21f..6a00644 100644
--- a/xen/arch/x86/hvm/vioapic.c
+++ b/xen/arch/x86/hvm/vioapic.c
@@ -30,6 +30,7 @@ 
 #include <xen/lib.h>
 #include <xen/errno.h>
 #include <xen/sched.h>
+#include <xen/viommu.h>
 #include <public/hvm/ioreq.h>
 #include <asm/hvm/io.h>
 #include <asm/hvm/vpic.h>
@@ -285,9 +286,18 @@  static void vioapic_deliver(struct hvm_hw_vioapic *vioapic, int irq)
     struct domain *d = vioapic_domain(vioapic);
     struct vlapic *target;
     struct vcpu *v;
+    struct irq_remapping_request request;
 
     ASSERT(spin_is_locked(&d->arch.hvm_domain.irq_lock));
 
+    if ( vioapic->redirtbl[irq].ir.format )
+    {
+        irq_request_ioapic_fill(&request, vioapic->id,
+                                vioapic->redirtbl[irq].bits);
+        viommu_handle_irq_request(d, &request);
+        return;
+    }
+
     HVM_DBG_LOG(DBG_LEVEL_IOAPIC,
                 "dest=%x dest_mode=%x delivery_mode=%x "
                 "vector=%x trig_mode=%x",
diff --git a/xen/arch/x86/viommu.c b/xen/arch/x86/viommu.c
new file mode 100644
index 0000000..ef78d3b
--- /dev/null
+++ b/xen/arch/x86/viommu.c
@@ -0,0 +1,30 @@ 
+/*
+ * viommu.c
+ *
+ * virtualize IOMMU.
+ *
+ * Copyright (C) 2017 Chao Gao, Intel Corporation.
+ *
+ * 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 that 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/>.
+ */
+
+#include <xen/viommu.h>
+
+void irq_request_ioapic_fill(struct 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;
+}
diff --git a/xen/include/asm-x86/viommu.h b/xen/include/asm-x86/viommu.h
index 0b25f34..fcf3c24 100644
--- a/xen/include/asm-x86/viommu.h
+++ b/xen/include/asm-x86/viommu.h
@@ -49,6 +49,9 @@  struct irq_remapping_request
     } msg;
 };
 
+void irq_request_ioapic_fill(struct irq_remapping_request *req,
+                             uint32_t ioapic_id, uint64_t rte);
+
 static inline const struct viommu_ops *viommu_get_ops(void)
 {
     /*
diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h
index 6127f89..06be4a5 100644
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -401,6 +401,7 @@  struct hvm_hw_vioapic {
             uint8_t reserved[4];
             uint8_t dest_id;
         } fields;
+        struct ir_ioapic_rte ir;
     } redirtbl[VIOAPIC_NUM_PINS];
 };