Re: [Qemu-devel] [PATCH] intel-iommu: Check IOAPIC's Trigger Mode against the one in IRTE

2016-09-28 Thread Wu, Feng


> -Original Message-
> From: Peter Xu [mailto:pet...@redhat.com]
> Sent: Wednesday, September 28, 2016 6:35 PM
> To: Paolo Bonzini <pbonz...@redhat.com>
> Cc: Wu, Feng <feng...@intel.com>; qemu-devel@nongnu.org; Michael S.
> Tsirkin <m...@redhat.com>
> Subject: Re: [PATCH] intel-iommu: Check IOAPIC's Trigger Mode against the one
> in IRTE
> 
> On Wed, Sep 28, 2016 at 12:21:37PM +0200, Paolo Bonzini wrote:
> >
> >
> > On 28/09/2016 12:05, Wu, Feng wrote:
> > > Hi Paolo,
> > >
> > >
> > >> -Original Message-
> > >> From: Wu, Feng
> > >> Sent: Thursday, September 22, 2016 12:12 AM
> > >> To: qemu-devel@nongnu.org
> > >> Cc: pbonz...@redhat.com; Wu, Feng <feng...@intel.com>
> > >> Subject: [PATCH] intel-iommu: Check IOAPIC's Trigger Mode against the
> one in
> > >> IRTE
> > >>
> > >> The Trigger Mode field of IOAPIC must match the Trigger Mode in
> > >> the IRTE according to VT-d Spec 5.1.5.1.
> > >
> > > Any comments about this patch? Thanks a lot!
> >
> > Hi, I am not the maintainer of the IOMMU emulation code.  CCing mst and
> > Peter.
> 
> I've provided my r-b already. :)

Okay, thanks to you guys!

Thanks,
Feng

> 
> -- peterx


Re: [Qemu-devel] [PATCH] intel-iommu: Check IOAPIC's Trigger Mode against the one in IRTE

2016-09-28 Thread Wu, Feng
Hi Paolo,


> -Original Message-
> From: Wu, Feng
> Sent: Thursday, September 22, 2016 12:12 AM
> To: qemu-devel@nongnu.org
> Cc: pbonz...@redhat.com; Wu, Feng <feng...@intel.com>
> Subject: [PATCH] intel-iommu: Check IOAPIC's Trigger Mode against the one in
> IRTE
> 
> The Trigger Mode field of IOAPIC must match the Trigger Mode in
> the IRTE according to VT-d Spec 5.1.5.1.

Any comments about this patch? Thanks a lot!

Thanks,
Feng

> 
> Signed-off-by: Feng Wu <feng...@intel.com>
> ---
>  hw/i386/intel_iommu.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 28c31a2..f32ad5c 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -27,6 +27,7 @@
>  #include "hw/pci/pci.h"
>  #include "hw/pci/pci_bus.h"
>  #include "hw/i386/pc.h"
> +#include "hw/i386/apic-msidef.h"
>  #include "hw/boards.h"
>  #include "hw/i386/x86-iommu.h"
>  #include "hw/pci-host/q35.h"
> @@ -2203,6 +2204,8 @@ static int vtd_interrupt_remap_msi(IntelIOMMUState
> *iommu,
>  }
>  } else {
>  uint8_t vector = origin->data & 0xff;
> +uint8_t trigger_mode = (origin->data >> MSI_DATA_TRIGGER_SHIFT) & 
> 0x1;
> +
>  VTD_DPRINTF(IR, "received IOAPIC interrupt");
>  /* IOAPIC entry vector should be aligned with IRTE vector
>   * (see vt-d spec 5.1.5.1). */
> @@ -2211,6 +2214,15 @@ static int
> vtd_interrupt_remap_msi(IntelIOMMUState *iommu,
>  "entry: %d, IRTE: %d, index: %d",
>  vector, irq.vector, index);
>  }
> +
> +/* The Trigger Mode field must match the Trigger Mode in the IRTE.
> + * (see vt-d spec 5.1.5.1). */
> +if (trigger_mode != irq.trigger_mode) {
> +VTD_DPRINTF(GENERAL, "IOAPIC trigger mode inconsistent: "
> +"entry: %u, IRTE: %u, index: %d",
> +trigger_mode, irq.trigger_mode, index);
> +}
> +
>  }
> 
>  /*
> --
> 1.9.1




Re: [Qemu-devel] [PATCH] intel-iommu: Check IOAPIC's Trigger Mode against the one in IRTE

2016-09-21 Thread Wu, Feng


> -Original Message-
> From: Peter Xu [mailto:pet...@redhat.com]
> Sent: Wednesday, September 21, 2016 2:12 PM
> To: Wu, Feng <feng...@intel.com>
> Cc: qemu-devel@nongnu.org; pbonz...@redhat.com
> Subject: Re: [Qemu-devel] [PATCH] intel-iommu: Check IOAPIC's Trigger Mode
> against the one in IRTE
> 
> On Wed, Sep 21, 2016 at 05:54:40AM +, Wu, Feng wrote:
> >
> >
> > > -Original Message-
> > > From: Peter Xu [mailto:pet...@redhat.com]
> > > Sent: Wednesday, September 21, 2016 1:45 PM
> > > To: Wu, Feng <feng...@intel.com>
> > > Cc: qemu-devel@nongnu.org; pbonz...@redhat.com
> > > Subject: Re: [Qemu-devel] [PATCH] intel-iommu: Check IOAPIC's Trigger
> Mode
> > > against the one in IRTE
> > >
> > > On Thu, Sep 22, 2016 at 12:12:17AM +0800, Feng Wu wrote:
> > > > The Trigger Mode field of IOAPIC must match the Trigger Mode in
> > > > the IRTE according to VT-d Spec 5.1.5.1.
> > > >
> > > > Signed-off-by: Feng Wu <feng...@intel.com>
> > >
> > > Reviewed-by: Peter Xu <pet...@redhat.com>
> > >
> > > Could I ask why we want this now? I know that both vector and trigger
> > > mode should not be aligned in current kernel.
> >
> > Oh, I don't aware of this. I was just looking at the code and found seems
> > the Spec says we need to check it. So I added the check. :)
> 
> Yeah, that's good enough a reason. :-)

Thanks for your review :)

Thanks,
Feng

> 
> -- peterx


Re: [Qemu-devel] [PATCH] intel-iommu: Check IOAPIC's Trigger Mode against the one in IRTE

2016-09-20 Thread Wu, Feng


> -Original Message-
> From: Peter Xu [mailto:pet...@redhat.com]
> Sent: Wednesday, September 21, 2016 1:45 PM
> To: Wu, Feng <feng...@intel.com>
> Cc: qemu-devel@nongnu.org; pbonz...@redhat.com
> Subject: Re: [Qemu-devel] [PATCH] intel-iommu: Check IOAPIC's Trigger Mode
> against the one in IRTE
> 
> On Thu, Sep 22, 2016 at 12:12:17AM +0800, Feng Wu wrote:
> > The Trigger Mode field of IOAPIC must match the Trigger Mode in
> > the IRTE according to VT-d Spec 5.1.5.1.
> >
> > Signed-off-by: Feng Wu <feng...@intel.com>
> 
> Reviewed-by: Peter Xu <pet...@redhat.com>
> 
> Could I ask why we want this now? I know that both vector and trigger
> mode should not be aligned in current kernel. 

Oh, I don't aware of this. I was just looking at the code and found seems
the Spec says we need to check it. So I added the check. :)

Thanks,
Feng

> However haven't go
> deeper yet.
> 
> Thanks,
> 
> -- peterx


Re: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size

2015-08-12 Thread Wu, Feng


 -Original Message-
 From: Jan Beulich [mailto:jbeul...@suse.com]
 Sent: Wednesday, August 12, 2015 4:43 PM
 To: Wu, Feng
 Cc: stefano.stabell...@eu.citrix.com; xen-de...@lists.xensource.com;
 qemu-devel@nongnu.org
 Subject: RE: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit
 bar with more than 4G size
 
  On 12.08.15 at 09:10, feng...@intel.com wrote:
 
 
  -Original Message-
  From: qemu-devel-bounces+feng.wu=intel@nongnu.org
  [mailto:qemu-devel-bounces+feng.wu=intel@nongnu.org] On Behalf Of
  Jan Beulich
  Sent: Wednesday, August 12, 2015 2:59 PM
  To: Wu, Feng
  Cc: xen-de...@lists.xensource.com; qemu-devel@nongnu.org;
  stefano.stabell...@eu.citrix.com
  Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle
 64-bit
  bar with more than 4G size
 
   On 05.08.15 at 04:02, feng...@intel.com wrote:
   @@ -491,8 +474,9 @@ static int
  xen_pt_bar_reg_write(XenPCIPassthroughState *s, XenPTReg *cfg_entry,
bar_ro_mask = XEN_PT_BAR_IO_RO_MASK | (r_size - 1);
break;
case XEN_PT_BAR_FLAG_UPPER:
   +r = d-io_regions[index-1];
 
  Perhaps worth an assert(index  0)?
 
  No problem, I will add it. BTW, do you have any other comments about this
  patch? If no, I am
  going to send out the new version with this changes.
 
 No - everything else looks to make sense (but continues to need
 testing).
 

I don't have such a device in hand. Can anybody who has such a device help to 
test this
patch? It would be highly appreciated!

Thanks,
Feng

 Jan




Re: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size

2015-08-12 Thread Wu, Feng


 -Original Message-
 From: qemu-devel-bounces+feng.wu=intel@nongnu.org
 [mailto:qemu-devel-bounces+feng.wu=intel@nongnu.org] On Behalf Of
 Jan Beulich
 Sent: Wednesday, August 12, 2015 2:59 PM
 To: Wu, Feng
 Cc: xen-de...@lists.xensource.com; qemu-devel@nongnu.org;
 stefano.stabell...@eu.citrix.com
 Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit
 bar with more than 4G size
 
  On 05.08.15 at 04:02, feng...@intel.com wrote:
  @@ -491,8 +474,9 @@ static int
 xen_pt_bar_reg_write(XenPCIPassthroughState *s, XenPTReg *cfg_entry,
   bar_ro_mask = XEN_PT_BAR_IO_RO_MASK | (r_size - 1);
   break;
   case XEN_PT_BAR_FLAG_UPPER:
  +r = d-io_regions[index-1];
 
 Perhaps worth an assert(index  0)?

No problem, I will add it. BTW, do you have any other comments about this 
patch? If no, I am
going to send out the new version with this changes.

Thanks,
Feng

 
 Jan
 




Re: [Qemu-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size

2015-08-06 Thread Wu, Feng


 -Original Message-
 From: Stefano Stabellini [mailto:stefano.stabell...@eu.citrix.com]
 Sent: Thursday, August 06, 2015 6:43 PM
 To: Wu, Feng
 Cc: stefano.stabell...@eu.citrix.com; xen-de...@lists.xensource.com;
 qemu-devel@nongnu.org
 Subject: Re: [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G
 size
 
 On Wed, 5 Aug 2015, Feng Wu wrote:
  This patch corrects a logic error when handling 64-bt bar with
  more than 4G size.
 
  With 64-bit Bar, it has two items in PCIDevice: io_regions[x]
  and io_regions[x+1], io_regions[x] has all the informations for
  this BAR, while io_regions[x+1] contains nothing, so we need to
  get the size from io_regions[x] when handling XEN_PT_BAR_FLAG_UPPER.
 
 That's because of the way io_regions are populated by
 xen_host_pci_get_resource, right?

Yes, xen_host_pci_get_resouce() is the source of the BAR information.

Thanks,
Feng

 
 
  Signed-off-by: Feng Wu feng...@intel.com
  ---
  I cannot test this patch sicne I don't have such a device, if
  someone have it, it would be highly appreicated if he can help
  to verfiy this patch.
 
 I would very much appreciate if somebody could properly validate this
 patch with the right device before I actually commit it
 
 
   hw/xen/xen_pt_config_init.c |   22 +++---
   1 files changed, 3 insertions(+), 19 deletions(-)
 
  diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
  index dd37be3..6fcef66 100644
  --- a/hw/xen/xen_pt_config_init.c
  +++ b/hw/xen/xen_pt_config_init.c
  @@ -326,23 +326,6 @@ static int
 xen_pt_cmd_reg_write(XenPCIPassthroughState *s, XenPTReg *cfg_entry,
   #define XEN_PT_BAR_IO_RO_MASK 0x0003  /* BAR ReadOnly
 mask(I/O) */
   #define XEN_PT_BAR_IO_EMU_MASK0xFFFC  /* BAR emul
 mask(I/O) */
 
  -static bool is_64bit_bar(PCIIORegion *r)
  -{
  -return !!(r-type  PCI_BASE_ADDRESS_MEM_TYPE_64);
  -}
  -
  -static uint64_t xen_pt_get_bar_size(PCIIORegion *r)
  -{
  -if (is_64bit_bar(r)) {
  -uint64_t size64;
  -size64 = (r + 1)-size;
  -size64 = 32;
  -size64 += r-size;
  -return size64;
  -}
  -return r-size;
  -}
  -
   static XenPTBarFlag xen_pt_bar_reg_parse(XenPCIPassthroughState *s,
int index)
   {
  @@ -365,7 +348,7 @@ static XenPTBarFlag
 xen_pt_bar_reg_parse(XenPCIPassthroughState *s,
 
   /* check unused BAR */
   r = d-io_regions[index];
  -if (!xen_pt_get_bar_size(r)) {
  +if (r-size == 0) {
   return XEN_PT_BAR_FLAG_UNUSED;
   }
  @@ -491,8 +474,9 @@ static int
 xen_pt_bar_reg_write(XenPCIPassthroughState *s, XenPTReg *cfg_entry,
   bar_ro_mask = XEN_PT_BAR_IO_RO_MASK | (r_size - 1);
   break;
   case XEN_PT_BAR_FLAG_UPPER:
  +r = d-io_regions[index-1];
   bar_emu_mask = XEN_PT_BAR_ALLF;
  -bar_ro_mask = r_size ? r_size - 1 : 0;
  +bar_ro_mask = (r-size - 1)  32;
   break;
   default:
   break;
 
 The changes look correct



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

2014-11-11 Thread Wu, Feng


 -Original Message-
 From: Alex Williamson [mailto:alex.william...@redhat.com]
 Sent: Tuesday, November 11, 2014 5:58 AM
 To: Wu, Feng
 Cc: pbonz...@redhat.com; r...@twiddle.net; mtosa...@redhat.com;
 qemu-devel@nongnu.org; k...@vger.kernel.org
 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 feng...@intel.com
  ---
   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

Re: [Qemu-devel] [PATCH] pc: piix4_pm: init legacy PCI hotplug when running on Xen

2014-11-03 Thread Wu, Feng
I still see this error in the latest QEMU. I find that this patch is not merged 
in. Anybody knows why?

Thanks,
Feng

 -Original Message-
 From: qemu-devel-bounces+feng.wu=intel@nongnu.org
 [mailto:qemu-devel-bounces+feng.wu=intel@nongnu.org] On Behalf Of
 Igor Mammedov
 Sent: Friday, May 23, 2014 8:04 PM
 To: qemu-devel@nongnu.org
 Cc: anthony.per...@citrix.com; Chen, Tiejun; stefano.stabell...@eu.citrix.com;
 aligu...@amazon.com; m...@redhat.com
 Subject: [Qemu-devel] [PATCH] pc: piix4_pm: init legacy PCI hotplug when
 running on Xen
 
 if user starts QEMU with -machine pc,accel=xen, then
 compat property in xenfv won't work and it would
 cause error:
 Unsupported bus. Bus doesn't have property 'acpi-pcihp-bsel' set
 when PCI device is added with -device on QEMU CLI.
 
 In case of Xen instead of using compat property,
 just use the fact that xen doesn't use QEMU's
 fw_cfg/acpi tables to switch piix4_pm into legacy
 PCI hotplug mode when Xen is enabled.
 
 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
  hw/acpi/piix4.c   |  3 +++
  hw/i386/pc_piix.c | 11 ---
  2 files changed, 3 insertions(+), 11 deletions(-)
 
 diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
 index 227ea30..12542c3 100644
 --- a/hw/acpi/piix4.c
 +++ b/hw/acpi/piix4.c
 @@ -501,6 +501,9 @@ I2CBus *piix4_pm_init(PCIBus *bus, int devfn,
 uint32_t smb_io_base,
  s-irq = sci_irq;
  s-smi_irq = smi_irq;
  s-kvm_enabled = kvm_enabled;
 +if (!fw_cfg) {
 +s-use_acpi_pci_hotplug = false;
 +}
 
  qdev_init_nofail(dev);
 
 diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
 index a13e8d6..067ff0c 100644
 --- a/hw/i386/pc_piix.c
 +++ b/hw/i386/pc_piix.c
 @@ -840,17 +840,6 @@ static QEMUMachine xenfv_machine = {
  .max_cpus = HVM_MAX_VCPUS,
  .default_machine_opts = accel=xen,
  .hot_add_cpu = pc_hot_add_cpu,
 -.compat_props = (GlobalProperty[]) {
 -/* xenfv has no fwcfg and so does not load acpi from QEMU.
 - * as such new acpi features don't work.
 - */
 -{
 -.driver   = PIIX4_PM,
 -.property = acpi-pci-hotplug-with-bridge-support,
 -.value= off,
 -},
 -{ /* end of list */ }
 -},
  };
  #endif
 
 --
 1.9.0