Re: [Xen-devel] [PATCH] MSI / MSIX injection for Xen HVM

[Paolo Bonzini <pbonzini@xxxxxxxxxx>]

Il 01/03/2012 12:51, Wei Liu ha scritto:
> On Thu, 2012-03-01 at 11:22 +0000, Jan Kiszka wrote:
>> On 2012-03-01 11:20, Wei Liu wrote:
>>>> However, you know that you miss those (uncommon) messages that are
>>>> injected via DMA? They end up directly in apic_deliver_msi (where KVM
>>>> will once pick them up as well).
>>>>
>>>
>>> Thanks for pointing this out. However I cannot find apic_deliver_msi in
>>> qemu-kvm or upstream qemu tree. I can only find apic_send_msi and
>>> apic_deliver_irq, none of which seems to be KVM-specific. Can you please
>>> give me some pointer on this?
>>
>> Yes, apic_send_msi. I was accidentally looking at my msi refactoring tree.
>>
>> Jan
>>
> 
> New version of this patch.
> 
> ------8<------
> From d20edc78c71827da9f8be3308c0d473fec03b705 Mon Sep 17 00:00:00 2001
> From: Wei Liu <wei.liu2@xxxxxxxxxx>
> Date: Wed, 29 Feb 2012 16:46:41 +0000
> Subject: [PATCH] MSI / MSIX injection for Xen.
> 
> This is supposed to be used in conjunction with Xen's
> hypercall interface for emualted MSI / MSIX injection.
> 
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
>  hw/apic.c  |   10 ++++++++--
>  hw/msi.c   |    7 ++++++-
>  hw/msix.c  |    8 +++++++-
>  hw/xen.h   |    1 +
>  xen-all.c  |    5 +++++
>  xen-stub.c |    4 ++++
>  6 files changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/apic.c b/hw/apic.c
> index ff9d24e..9fc0b17 100644
> --- a/hw/apic.c
> +++ b/hw/apic.c
> @@ -22,6 +22,7 @@
>  #include "host-utils.h"
>  #include "trace.h"
>  #include "pc.h"
> +#include "xen.h"
>  
>  #define MAX_APIC_WORDS 8
>  
> @@ -641,8 +642,13 @@ static void apic_send_msi(target_phys_addr_t addr, 
> uint32_t data)
>      uint8_t dest_mode = (addr >> MSI_ADDR_DEST_MODE_SHIFT) & 0x1;
>      uint8_t trigger_mode = (data >> MSI_DATA_TRIGGER_SHIFT) & 0x1;
>      uint8_t delivery = (data >> MSI_DATA_DELIVERY_MODE_SHIFT) & 0x7;
> -    /* XXX: Ignore redirection hint. */
> -    apic_deliver_irq(dest, dest_mode, delivery, vector, trigger_mode);
> +
> +    if (xen_enabled()) {
> +        xen_hvm_inject_msi(addr, data);
> +    } else {
> +        /* XXX: Ignore redirection hint. */
> +        apic_deliver_irq(dest, dest_mode, delivery, vector, trigger_mode);
> +    }
>  }
>  
>  static void apic_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t 
> val)
> diff --git a/hw/msi.c b/hw/msi.c
> index 5d6ceb6..b11eeac 100644
> --- a/hw/msi.c
> +++ b/hw/msi.c
> @@ -20,6 +20,7 @@
>  
>  #include "msi.h"
>  #include "range.h"
> +#include "xen.h"
>  
>  /* Eventually those constants should go to Linux pci_regs.h */
>  #define PCI_MSI_PENDING_32      0x10
> @@ -257,7 +258,11 @@ void msi_notify(PCIDevice *dev, unsigned int vector)
>                     "notify vector 0x%x"
>                     " address: 0x%"PRIx64" data: 0x%"PRIx32"\n",
>                     vector, address, data);
> -    stl_le_phys(address, data);
> +    if (xen_enabled()) {
> +        xen_hvm_inject_msi(address, data);
> +    } else {
> +        stl_le_phys(address, data);
> +    }
>  }
>  
>  /* call this function after updating configs by pci_default_write_config(). 
> */
> diff --git a/hw/msix.c b/hw/msix.c
> index 3835eaa..cba302e 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -19,6 +19,7 @@
>  #include "msix.h"
>  #include "pci.h"
>  #include "range.h"
> +#include "xen.h"
>  
>  #define MSIX_CAP_LENGTH 12
>  
> @@ -365,7 +366,12 @@ void msix_notify(PCIDevice *dev, unsigned vector)
>  
>      address = pci_get_quad(table_entry + PCI_MSIX_ENTRY_LOWER_ADDR);
>      data = pci_get_long(table_entry + PCI_MSIX_ENTRY_DATA);
> -    stl_le_phys(address, data);
> +
> +    if (xen_enabled()) {
> +        xen_hvm_inject_msi(address, data);
> +    } else {
> +        stl_le_phys(address, data);
> +    }
>  }
>  
>  void msix_reset(PCIDevice *dev)
> diff --git a/hw/xen.h b/hw/xen.h
> index b46879c..e5926b7 100644
> --- a/hw/xen.h
> +++ b/hw/xen.h
> @@ -34,6 +34,7 @@ static inline int xen_enabled(void)
>  int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
>  void xen_piix3_set_irq(void *opaque, int irq_num, int level);
>  void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int 
> len);
> +void xen_hvm_inject_msi(uint64_t addr, uint32_t data);
>  void xen_cmos_set_s3_resume(void *opaque, int irq, int level);
>  
>  qemu_irq *xen_interrupt_controller_init(void);
> diff --git a/xen-all.c b/xen-all.c
> index b0ed1ed..78c6df3 100644
> --- a/xen-all.c
> +++ b/xen-all.c
> @@ -122,6 +122,11 @@ void xen_piix_pci_write_config_client(uint32_t address, 
> uint32_t val, int len)
>      }
>  }
>  
> +void xen_hvm_inject_msi(uint64_t addr, uint32_t data)
> +{
> +    xc_hvm_inject_msi(xen_xc, xen_domid, addr, data);
> +}
> +
>  static void xen_suspend_notifier(Notifier *notifier, void *data)
>  {
>      xc_set_hvm_param(xen_xc, xen_domid, HVM_PARAM_ACPI_S_STATE, 3);
> diff --git a/xen-stub.c b/xen-stub.c
> index 9ea02d4..8ff2b79 100644
> --- a/xen-stub.c
> +++ b/xen-stub.c
> @@ -29,6 +29,10 @@ void xen_piix_pci_write_config_client(uint32_t address, 
> uint32_t val, int len)
>  {
>  }
>  
> +void xen_hvm_inject_msi(uint64_t addr, uint32_t data)
> +{
> +}
> +
>  void xen_cmos_set_s3_resume(void *opaque, int irq, int level)
>  {
>  }

This is not a NACK, but I can't help asking.  Perhaps the fake Xen
interrupt controller is a bit too simplistic?  You can add a memory
region corresponding to the APICs and trap writes in that region.
Writes coming from QEMU are MSIs and can be injected to the hypervisor,
writes coming from the VM will be trapped by Xen before going out to QEMU.

Paolo

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