Re: [Xen-devel] [RFC PATCH 15/23] X86/vioapic: Hook interrupt delivery of vIOAPIC

[Lan Tianyu <tianyu.lan@xxxxxxxxx>]

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@xxxxxxxxx>
>>
>> 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@xxxxxxxxx>
>> Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>
>> ---
>>  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@xxxxxxxxxxxxx
>> https://lists.xen.org/xen-devel


-- 
Best regards
Tianyu Lan

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