Re: [Qemu-devel] [PATCH] intel-iommu: Check IOAPIC's Trigger Mode against the one in IRTE
> -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
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
> -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
> -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
-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
-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
-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
-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
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