Re: [Qemu-devel] [PATCH] x86: Update VT-d Posted-Interrupts related information

[Wu, Feng]

> -----Original Message-----
> From: Alex Williamson [mailto:address@hidden
> Sent: Tuesday, November 11, 2014 5:58 AM
> To: Wu, Feng
> Cc: address@hidden; address@hidden; address@hidden;
> address@hidden; address@hidden
> Subject: Re: [PATCH] x86: Update VT-d Posted-Interrupts related information
> 
> On Mon, 2014-11-10 at 14:31 +0800, Feng Wu wrote:
> > VT-d Posted-Interrupts(PI) is an enhancement to CPU side Posted-Interrupt.
> > With VT-d Posted-Interrupts enabled, external interrupts from
> > direct-assigned devices can be delivered to guests without VMM
> > involvement when guest is running in non-root mode.
> >
> > If VT-d PI is supported by KVM, we need to update the IRTE with
> > the new guest interrtup configuration.
> >
> > Signed-off-by: Feng Wu <address@hidden>
> > ---
> >  hw/i386/kvm/pci-assign.c  |   24 ++++++++++++++++++++++++
> >  linux-headers/linux/kvm.h |    2 ++
> >  target-i386/kvm.c         |    5 +++++
> >  target-i386/kvm_i386.h    |    1 +
> >  4 files changed, 32 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
> > index bb206da..e55a99b 100644
> > --- a/hw/i386/kvm/pci-assign.c
> > +++ b/hw/i386/kvm/pci-assign.c
> > @@ -1005,6 +1005,12 @@ static void assigned_dev_update_msi(PCIDevice
> *pci_dev)
> >          assigned_dev->intx_route.mode = PCI_INTX_DISABLED;
> >          assigned_dev->intx_route.irq = -1;
> >          assigned_dev->assigned_irq_type = ASSIGNED_IRQ_MSI;
> > +
> > +        if (kvm_check_extension(kvm_state, KVM_CAP_PI)) {
> > +            if (kvm_device_pi_update(kvm_state, assigned_dev->dev_id)
> < 0) {
> > +                perror("assigned_dev_update_msi:
> kvm_device_pi_update");
> > +            }
> > +        }
> 
> 
> I don't really understand this ioctl, we need to call into KVM to set
> the new IRQ routing anyway, why can't KVM figure out that it needs to
> update the posted interrupt config at the same time?  Also, we'll need
> to support this through vfio-pci.  Thanks,
> 
> Alex

We need host irq information got from "struct kvm_assigned_dev_kernel *dev"
when updating IRTE for VT-d Posted-Interrtups. However, when guest tries to
update the msi message like the following: 

assigned_dev_update_msi_msg() --> kvm_irqchip_update_msi_route() --> 
kvm_update_routing_entry() -->
kvm_irqchip_commit_routes() --> kvm_irqchip_commit_routes()--> 
KVM_SET_GSI_ROUTING --> kvm_set_irq_routing()

It will finally go to kvm_set_irq_routing() in KVM, there are two problem:
1. It use RCU in this function, it is hard to find which entry in the irq 
routing table is being updated.
2. Even we find the updated entry, it is hard to find the associated assigned 
device with this irq routing entry in this function.

Do you have any ideas about this? If we can do all this inside KVM, that should 
be great!

Thanks,
Feng

> 
> >      } else {
> >          Error *local_err = NULL;
> >
> > @@ -1029,6 +1035,12 @@ static void
> assigned_dev_update_msi_msg(PCIDevice *pci_dev)
> >
> >      kvm_irqchip_update_msi_route(kvm_state,
> assigned_dev->msi_virq[0],
> >                                   msi_get_message(pci_dev, 0));
> > +
> > +    if (kvm_check_extension(kvm_state, KVM_CAP_PI)) {
> > +        if (kvm_device_pi_update(kvm_state, assigned_dev->dev_id) < 0) {
> > +            perror("assigned_dev_update_msi_msg:
> kvm_device_pi_update");
> > +        }
> > +    }
> >  }
> >
> >  static bool assigned_dev_msix_masked(MSIXTableEntry *entry)
> > @@ -1149,6 +1161,12 @@ static void assigned_dev_update_msix(PCIDevice
> *pci_dev)
> >                  perror("assigned_dev_enable_msix: assign irq");
> >                  return;
> >              }
> > +
> > +            if (kvm_check_extension(kvm_state, KVM_CAP_PI)) {
> > +                if (kvm_device_pi_update(kvm_state,
> assigned_dev->dev_id) < 0) {
> > +                    perror("assigned_dev_update_msix:
> kvm_device_pi_update");
> > +                }
> > +            }
> >          }
> >          assigned_dev->intx_route.mode = PCI_INTX_DISABLED;
> >          assigned_dev->intx_route.irq = -1;
> > @@ -1618,6 +1636,12 @@ static void assigned_dev_msix_mmio_write(void
> *opaque, hwaddr addr,
> >                  if (ret) {
> >                      error_report("Error updating irq routing entry
> (%d)", ret);
> >                  }
> > +                if (kvm_check_extension(kvm_state, KVM_CAP_PI)) {
> > +                    if (kvm_device_pi_update(kvm_state, adev->dev_id)
> < 0) {
> > +                        perror("assigned_dev_update_msi_msg: "
> > +                                    "kvm_device_pi_update");
> > +                    }
> > +                }
> >              }
> >          }
> >      }
> > diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> > index 2669938..b34f3c4 100644
> > --- a/linux-headers/linux/kvm.h
> > +++ b/linux-headers/linux/kvm.h
> > @@ -765,6 +765,7 @@ struct kvm_ppc_smmu_info {
> >  #define KVM_CAP_PPC_FIXUP_HCALL 103
> >  #define KVM_CAP_PPC_ENABLE_HCALL 104
> >  #define KVM_CAP_CHECK_EXTENSION_VM 105
> > +#define KVM_CAP_PI 106
> >
> >  #ifdef KVM_CAP_IRQ_ROUTING
> >
> > @@ -1020,6 +1021,7 @@ struct kvm_s390_ucas_mapping {
> >  #define KVM_XEN_HVM_CONFIG        _IOW(KVMIO,  0x7a, struct
> kvm_xen_hvm_config)
> >  #define KVM_SET_CLOCK             _IOW(KVMIO,  0x7b, struct
> kvm_clock_data)
> >  #define KVM_GET_CLOCK             _IOR(KVMIO,  0x7c, struct
> kvm_clock_data)
> > +#define KVM_ASSIGN_DEV_PI_UPDATE  _IOR(KVMIO,  0x7d, __u32)
> >  /* Available with KVM_CAP_PIT_STATE2 */
> >  #define KVM_GET_PIT2              _IOR(KVMIO,  0x9f, struct
> kvm_pit_state2)
> >  #define KVM_SET_PIT2              _IOW(KVMIO,  0xa0, struct
> kvm_pit_state2)
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index ccf36e8..3dd8e5e 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -2660,6 +2660,11 @@ int kvm_device_msi_assign(KVMState *s,
> uint32_t dev_id, int virq)
> >
> KVM_DEV_IRQ_GUEST_MSI, virq);
> >  }
> >
> > +int kvm_device_pi_update(KVMState *s, uint32_t dev_id)
> > +{
> > +    return kvm_vm_ioctl(s, KVM_ASSIGN_DEV_PI_UPDATE, &dev_id);
> > +}
> > +
> >  int kvm_device_msi_deassign(KVMState *s, uint32_t dev_id)
> >  {
> >      return kvm_deassign_irq_internal(s, dev_id,
> KVM_DEV_IRQ_GUEST_MSI |
> > diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
> > index cac30fd..c119b3e 100644
> > --- a/target-i386/kvm_i386.h
> > +++ b/target-i386/kvm_i386.h
> > @@ -37,4 +37,5 @@ int kvm_device_msix_set_vector(KVMState *s,
> uint32_t dev_id, uint32_t vector,
> >  int kvm_device_msix_assign(KVMState *s, uint32_t dev_id);
> >  int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id);
> >
> > +int kvm_device_pi_update(KVMState *s, uint32_t dev_id);
> >  #endif
> 
>