Re: [Qemu-devel] [PATCH 08/13] be-hci: use backend functions
On 26 May 2017 at 14:20, Marc-André Lureau wrote: > On Tue, May 9, 2017 at 3:42 PM Marc-André Lureau < > marcandre.lur...@redhat.com> wrote: >> Avoid accessing CharBackend directly, use qemu_chr_be_* methods instead. >> >> be->chr_read should exists if qemu_chr_be_can_write() is true. >> >> (use qemu_chr_be_write(), _impl() bypasses replay) >> >> Signed-off-by: Marc-André Lureau >> --- >> hw/bt/hci-csr.c | 9 +++-- >> > > No maintainer for this file. Andrzej, as author of the file, can you > review? This looks correct. Reviewed-by: Andrzej Zaborowski > > >> 1 file changed, 3 insertions(+), 6 deletions(-) >> >> diff --git a/hw/bt/hci-csr.c b/hw/bt/hci-csr.c >> index 0f2021086d..d13192b9b5 100644 >> --- a/hw/bt/hci-csr.c >> +++ b/hw/bt/hci-csr.c >> @@ -82,17 +82,14 @@ enum { >> >> static inline void csrhci_fifo_wake(struct csrhci_s *s) >> { >> -Chardev *chr = (Chardev *)s; >> -CharBackend *be = chr->be; >> +Chardev *chr = CHARDEV(s); >> >> if (!s->enable || !s->out_len) >> return; >> >> /* XXX: Should wait for s->modem_state & CHR_TIOCM_RTS? */ >> -if (be && be->chr_can_read && be->chr_can_read(be->opaque) && >> -be->chr_read) { >> -be->chr_read(be->opaque, >> - s->outfifo + s->out_start++, 1); >> +if (qemu_chr_be_can_write(chr)) { >> +qemu_chr_be_write(chr, s->outfifo + s->out_start++, 1); >> s->out_len--; >> if (s->out_start >= s->out_size) { >> s->out_start = 0; >> -- >> 2.13.0.rc1.16.gd80b50c3f >> >> >> -- > Marc-André Lureau
[Qemu-devel] [PATCH] nvme: Fix get/set number of queues feature, again
The number of queues that should be return by the admin command should: 1) Only mention the number of non-admin queues. 2) It is zero-based, meaning that '0 == one non-admin queue', '1 == two non-admin queues', and so forth. Because our `num_queues` means the number of queues _plus_ the admin queue, then the right calculation for the number returned from the admin command is `num_queues - 2`, combining the two requirements mentioned. The issue was discovered by reducing num_queues from 64 to 8 and running a Linux VM with an SMP parameter larger than that (e.g. 22). It tries to utilize all queues, and therefore fails with an invalid queue number when trying to queue I/Os on the last queue. Signed-off-by: Dan Aloni CC: Alex Friedman CC: Keith Busch CC: Stefan Hajnoczi --- hw/block/nvme.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 7428db9f0c91..08ddf3a39e2f 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -573,7 +573,7 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req) result = blk_enable_write_cache(n->conf.blk); break; case NVME_NUMBER_OF_QUEUES: -result = cpu_to_le32((n->num_queues - 1) | ((n->num_queues - 1) << 16)); +result = cpu_to_le32((n->num_queues - 2) | ((n->num_queues - 2) << 16)); break; default: return NVME_INVALID_FIELD | NVME_DNR; @@ -594,7 +594,7 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req) break; case NVME_NUMBER_OF_QUEUES: req->cqe.result = -cpu_to_le32((n->num_queues - 1) | ((n->num_queues - 1) << 16)); +cpu_to_le32((n->num_queues - 2) | ((n->num_queues - 2) << 16)); break; default: return NVME_INVALID_FIELD | NVME_DNR; -- 2.9.4
[Qemu-devel] [PULL 01/13] memory: tune last param of iommu_ops.translate()
From: Peter Xu This patch converts the old "is_write" bool into IOMMUAccessFlags. The difference is that "is_write" can only express either read/write, but sometimes what we really want is "none" here (neither read nor write). Replay is an good example - during replay, we should not check any RW permission bits since thats not an actual IO at all. CC: Paolo Bonzini CC: David Gibson Reviewed-by: David Gibson Acked-by: David Gibson Acked-by: Paolo Bonzini Signed-off-by: Peter Xu Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Jason Wang --- include/exec/memory.h| 10 -- exec.c | 3 ++- hw/alpha/typhoon.c | 2 +- hw/dma/rc4030.c | 2 +- hw/i386/amd_iommu.c | 4 ++-- hw/i386/intel_iommu.c| 4 ++-- hw/pci-host/apb.c| 2 +- hw/ppc/spapr_iommu.c | 2 +- hw/s390x/s390-pci-bus.c | 2 +- hw/s390x/s390-pci-inst.c | 2 +- memory.c | 3 ++- 11 files changed, 22 insertions(+), 14 deletions(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index 99e0f54..97fd0c2 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -185,8 +185,14 @@ struct MemoryRegionOps { typedef struct MemoryRegionIOMMUOps MemoryRegionIOMMUOps; struct MemoryRegionIOMMUOps { -/* Return a TLB entry that contains a given address. */ -IOMMUTLBEntry (*translate)(MemoryRegion *iommu, hwaddr addr, bool is_write); +/* + * Return a TLB entry that contains a given address. Flag should + * be the access permission of this translation operation. We can + * set flag to IOMMU_NONE to mean that we don't need any + * read/write permission checks, like, when for region replay. + */ +IOMMUTLBEntry (*translate)(MemoryRegion *iommu, hwaddr addr, + IOMMUAccessFlags flag); /* Returns minimum supported page size */ uint64_t (*get_min_page_size)(MemoryRegion *iommu); /* Called when IOMMU Notifier flag changed */ diff --git a/exec.c b/exec.c index ff16f04..b1db12f 100644 --- a/exec.c +++ b/exec.c @@ -486,7 +486,8 @@ static MemoryRegionSection address_space_do_translate(AddressSpace *as, break; } -iotlb = mr->iommu_ops->translate(mr, addr, is_write); +iotlb = mr->iommu_ops->translate(mr, addr, is_write ? + IOMMU_WO : IOMMU_RO); addr = ((iotlb.translated_addr & ~iotlb.addr_mask) | (addr & iotlb.addr_mask)); *plen = MIN(*plen, (addr | iotlb.addr_mask) - addr + 1); diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c index f50f5cf..c1cf780 100644 --- a/hw/alpha/typhoon.c +++ b/hw/alpha/typhoon.c @@ -664,7 +664,7 @@ static bool window_translate(TyphoonWindow *win, hwaddr addr, /* TODO: A translation failure here ought to set PCI error codes on the Pchip and generate a machine check interrupt. */ static IOMMUTLBEntry typhoon_translate_iommu(MemoryRegion *iommu, hwaddr addr, - bool is_write) + IOMMUAccessFlags flag) { TyphoonPchip *pchip = container_of(iommu, TyphoonPchip, iommu); IOMMUTLBEntry ret; diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c index 0080141..edf9432 100644 --- a/hw/dma/rc4030.c +++ b/hw/dma/rc4030.c @@ -489,7 +489,7 @@ static const MemoryRegionOps jazzio_ops = { }; static IOMMUTLBEntry rc4030_dma_translate(MemoryRegion *iommu, hwaddr addr, - bool is_write) + IOMMUAccessFlags flag) { rc4030State *s = container_of(iommu, rc4030State, dma_mr); IOMMUTLBEntry ret = { diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c index 329058d..7b6d4ea 100644 --- a/hw/i386/amd_iommu.c +++ b/hw/i386/amd_iommu.c @@ -988,7 +988,7 @@ static inline bool amdvi_is_interrupt_addr(hwaddr addr) } static IOMMUTLBEntry amdvi_translate(MemoryRegion *iommu, hwaddr addr, - bool is_write) + IOMMUAccessFlags flag) { AMDVIAddressSpace *as = container_of(iommu, AMDVIAddressSpace, iommu); AMDVIState *s = as->iommu_state; @@ -1017,7 +1017,7 @@ static IOMMUTLBEntry amdvi_translate(MemoryRegion *iommu, hwaddr addr, return ret; } -amdvi_do_translate(as, addr, is_write, &ret); +amdvi_do_translate(as, addr, flag & IOMMU_WO, &ret); trace_amdvi_translation_result(as->bus_num, PCI_SLOT(as->devfn), PCI_FUNC(as->devfn), addr, ret.translated_addr); return ret; diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 9ba2162..4a51df8 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -2221,7 +2221,7 @@ static void vtd_mem_write(void *opaque, hwaddr addr, } static IOMMUTLBEntry vtd_iommu_translate(MemoryRegion *iommu, hwaddr addr, - bool is_write) +
[Qemu-devel] [PULL 00/13] pci, virtio, vhost: fixes
The following changes since commit 9964e96dccf7f7c936ee854a795415d19b60: Merge remote-tracking branch 'jasowang/tags/net-pull-request' into staging (2017-05-23 15:01:31 +0100) are available in the git repository at: git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream for you to fetch changes up to 811bf15114bff4d46735acedbd72b906616ce365: acpi-test: update expected files (2017-05-29 03:07:57 +0300) pci, virtio, vhost: fixes A bunch of fixes all over the place. Most notably this fixes the new MTU feature when using vhost. Signed-off-by: Michael S. Tsirkin Ladi Prosek (1): pc: ACPI BIOS: use highest NUMA node for hotplug mem hole SRAT entry Maxime Coquelin (2): virtio_net: Bypass backends for MTU feature negotiation vhost-user: pass message as a pointer to process_message_reply() Michael S. Tsirkin (1): acpi-test: update expected files Peter Xu (9): memory: tune last param of iommu_ops.translate() memory: remove the last param in memory_region_iommu_replay() x86-iommu: use DeviceClass properties intel_iommu: renaming context entry helpers intel_iommu: provide vtd_ce_get_type() intel_iommu: use IOMMU_ACCESS_FLAG() intel_iommu: allow dev-iotlb context entry conditionally intel_iommu: support passthrough (PT) intel_iommu: turn off pt before 2.9 hw/i386/intel_iommu_internal.h | 1 + include/exec/memory.h | 15 +- include/hw/compat.h | 8 + include/hw/i386/x86-iommu.h | 1 + include/hw/virtio/virtio-net.h | 1 + include/hw/virtio/virtio.h | 1 + exec.c | 3 +- hw/alpha/typhoon.c | 2 +- hw/dma/rc4030.c | 2 +- hw/i386/acpi-build.c| 7 +- hw/i386/amd_iommu.c | 4 +- hw/i386/intel_iommu.c | 313 ++-- hw/i386/x86-iommu.c | 48 +- hw/net/virtio-net.c | 17 +- hw/pci-host/apb.c | 2 +- hw/ppc/spapr_iommu.c| 2 +- hw/s390x/s390-pci-bus.c | 2 +- hw/s390x/s390-pci-inst.c| 2 +- hw/vfio/common.c| 2 +- hw/virtio/vhost-user.c | 12 +- memory.c| 7 +- hw/i386/trace-events| 2 + tests/acpi-test-data/pc/SRAT.memhp | Bin 264 -> 264 bytes tests/acpi-test-data/q35/SRAT.memhp | Bin 264 -> 264 bytes 24 files changed, 299 insertions(+), 155 deletions(-)
[Qemu-devel] [PULL 02/13] memory: remove the last param in memory_region_iommu_replay()
From: Peter Xu We were always passing in that one as "false" to assume that's an read operation, and we also assume that IOMMU translation would always have that read permission. A better permission would be IOMMU_NONE since the replay is after all not a real read operation, but just a page table rebuilding process. CC: David Gibson CC: Paolo Bonzini Reviewed-by: David Gibson Acked-by: Paolo Bonzini Signed-off-by: Peter Xu Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Jason Wang --- include/exec/memory.h | 5 + hw/vfio/common.c | 2 +- memory.c | 8 +++- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index 97fd0c2..bfdc685 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -731,11 +731,8 @@ void memory_region_register_iommu_notifier(MemoryRegion *mr, * * @mr: the memory region to observe * @n: the notifier to which to replay iommu mappings - * @is_write: Whether to treat the replay as a translate "write" - * through the iommu */ -void memory_region_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n, -bool is_write); +void memory_region_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n); /** * memory_region_iommu_replay_all: replay existing IOMMU translations diff --git a/hw/vfio/common.c b/hw/vfio/common.c index a8f12ee..b9abe77 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -502,7 +502,7 @@ static void vfio_listener_region_add(MemoryListener *listener, QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next); memory_region_register_iommu_notifier(giommu->iommu, &giommu->n); -memory_region_iommu_replay(giommu->iommu, &giommu->n, false); +memory_region_iommu_replay(giommu->iommu, &giommu->n); return; } diff --git a/memory.c b/memory.c index 3f0aae8..0ddc4cc 100644 --- a/memory.c +++ b/memory.c @@ -1620,12 +1620,10 @@ uint64_t memory_region_iommu_get_min_page_size(MemoryRegion *mr) return TARGET_PAGE_SIZE; } -void memory_region_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n, -bool is_write) +void memory_region_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n) { hwaddr addr, granularity; IOMMUTLBEntry iotlb; -IOMMUAccessFlags flag = is_write ? IOMMU_WO : IOMMU_RO; /* If the IOMMU has its own replay callback, override */ if (mr->iommu_ops->replay) { @@ -1636,7 +1634,7 @@ void memory_region_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n, granularity = memory_region_iommu_get_min_page_size(mr); for (addr = 0; addr < memory_region_size(mr); addr += granularity) { -iotlb = mr->iommu_ops->translate(mr, addr, flag); +iotlb = mr->iommu_ops->translate(mr, addr, IOMMU_NONE); if (iotlb.perm != IOMMU_NONE) { n->notify(n, &iotlb); } @@ -1654,7 +1652,7 @@ void memory_region_iommu_replay_all(MemoryRegion *mr) IOMMUNotifier *notifier; IOMMU_NOTIFIER_FOREACH(notifier, mr) { -memory_region_iommu_replay(mr, notifier, false); +memory_region_iommu_replay(mr, notifier); } } -- MST
[Qemu-devel] [PULL 05/13] intel_iommu: provide vtd_ce_get_type()
From: Peter Xu Helper to fetch VT-d context entry type. Signed-off-by: Peter Xu Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Jason Wang --- hw/i386/intel_iommu.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index f06055f..b477143 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -595,6 +595,11 @@ static inline uint32_t vtd_ce_get_agaw(VTDContextEntry *ce) return 30 + (ce->hi & VTD_CONTEXT_ENTRY_AW) * 9; } +static inline uint32_t vtd_ce_get_type(VTDContextEntry *ce) +{ +return ce->lo & VTD_CONTEXT_ENTRY_TT; +} + static inline uint64_t vtd_iova_limit(VTDContextEntry *ce) { uint32_t ce_agaw = vtd_ce_get_agaw(ce); @@ -865,7 +870,7 @@ static int vtd_dev_to_context_entry(IntelIOMMUState *s, uint8_t bus_num, trace_vtd_ce_invalid(ce->hi, ce->lo); return -VTD_FR_CONTEXT_ENTRY_INV; } else { -switch (ce->lo & VTD_CONTEXT_ENTRY_TT) { +switch (vtd_ce_get_type(ce)) { case VTD_CONTEXT_TT_MULTI_LEVEL: /* fall through */ case VTD_CONTEXT_TT_DEV_IOTLB: -- MST
[Qemu-devel] [PULL 03/13] x86-iommu: use DeviceClass properties
From: Peter Xu No reason to keep tens of lines if we can do it actually far shorter. Signed-off-by: Peter Xu Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Jason Wang --- hw/i386/x86-iommu.c | 47 +++ 1 file changed, 7 insertions(+), 40 deletions(-) diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c index 23dcd3f..02b8825 100644 --- a/hw/i386/x86-iommu.c +++ b/hw/i386/x86-iommu.c @@ -88,55 +88,22 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp) x86_iommu_set_default(X86_IOMMU_DEVICE(dev)); } +static Property x86_iommu_properties[] = { +DEFINE_PROP_BOOL("intremap", X86IOMMUState, intr_supported, false), +DEFINE_PROP_BOOL("device-iotlb", X86IOMMUState, dt_supported, false), +DEFINE_PROP_END_OF_LIST(), +}; + static void x86_iommu_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); dc->realize = x86_iommu_realize; -} - -static bool x86_iommu_intremap_prop_get(Object *o, Error **errp) -{ -X86IOMMUState *s = X86_IOMMU_DEVICE(o); -return s->intr_supported; -} - -static void x86_iommu_intremap_prop_set(Object *o, bool value, Error **errp) -{ -X86IOMMUState *s = X86_IOMMU_DEVICE(o); -s->intr_supported = value; -} - -static bool x86_iommu_device_iotlb_prop_get(Object *o, Error **errp) -{ -X86IOMMUState *s = X86_IOMMU_DEVICE(o); -return s->dt_supported; -} - -static void x86_iommu_device_iotlb_prop_set(Object *o, bool value, Error **errp) -{ -X86IOMMUState *s = X86_IOMMU_DEVICE(o); -s->dt_supported = value; -} - -static void x86_iommu_instance_init(Object *o) -{ -X86IOMMUState *s = X86_IOMMU_DEVICE(o); - -/* By default, do not support IR */ -s->intr_supported = false; -object_property_add_bool(o, "intremap", x86_iommu_intremap_prop_get, - x86_iommu_intremap_prop_set, NULL); -s->dt_supported = false; -object_property_add_bool(o, "device-iotlb", - x86_iommu_device_iotlb_prop_get, - x86_iommu_device_iotlb_prop_set, - NULL); +dc->props = x86_iommu_properties; } static const TypeInfo x86_iommu_info = { .name = TYPE_X86_IOMMU_DEVICE, .parent= TYPE_SYS_BUS_DEVICE, -.instance_init = x86_iommu_instance_init, .instance_size = sizeof(X86IOMMUState), .class_init= x86_iommu_class_init, .class_size= sizeof(X86IOMMUClass), -- MST
[Qemu-devel] [PULL 04/13] intel_iommu: renaming context entry helpers
From: Peter Xu The old names are too long and less ordered. Let's start to use vtd_ce_*() as a pattern. Signed-off-by: Peter Xu Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Jason Wang --- hw/i386/intel_iommu.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 4a51df8..f06055f 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -512,7 +512,7 @@ static int vtd_get_root_entry(IntelIOMMUState *s, uint8_t index, return 0; } -static inline bool vtd_context_entry_present(VTDContextEntry *context) +static inline bool vtd_ce_present(VTDContextEntry *context) { return context->lo & VTD_CONTEXT_ENTRY_P; } @@ -533,7 +533,7 @@ static int vtd_get_context_entry_from_root(VTDRootEntry *root, uint8_t index, return 0; } -static inline dma_addr_t vtd_get_slpt_base_from_context(VTDContextEntry *ce) +static inline dma_addr_t vtd_ce_get_slpt_base(VTDContextEntry *ce) { return ce->lo & VTD_CONTEXT_ENTRY_SLPTPTR; } @@ -585,19 +585,19 @@ static inline bool vtd_is_level_supported(IntelIOMMUState *s, uint32_t level) /* Get the page-table level that hardware should use for the second-level * page-table walk from the Address Width field of context-entry. */ -static inline uint32_t vtd_get_level_from_context_entry(VTDContextEntry *ce) +static inline uint32_t vtd_ce_get_level(VTDContextEntry *ce) { return 2 + (ce->hi & VTD_CONTEXT_ENTRY_AW); } -static inline uint32_t vtd_get_agaw_from_context_entry(VTDContextEntry *ce) +static inline uint32_t vtd_ce_get_agaw(VTDContextEntry *ce) { return 30 + (ce->hi & VTD_CONTEXT_ENTRY_AW) * 9; } static inline uint64_t vtd_iova_limit(VTDContextEntry *ce) { -uint32_t ce_agaw = vtd_get_agaw_from_context_entry(ce); +uint32_t ce_agaw = vtd_ce_get_agaw(ce); return 1ULL << MIN(ce_agaw, VTD_MGAW); } @@ -642,8 +642,8 @@ static int vtd_iova_to_slpte(VTDContextEntry *ce, uint64_t iova, bool is_write, uint64_t *slptep, uint32_t *slpte_level, bool *reads, bool *writes) { -dma_addr_t addr = vtd_get_slpt_base_from_context(ce); -uint32_t level = vtd_get_level_from_context_entry(ce); +dma_addr_t addr = vtd_ce_get_slpt_base(ce); +uint32_t level = vtd_ce_get_level(ce); uint32_t offset; uint64_t slpte; uint64_t access_right_check; @@ -664,7 +664,7 @@ static int vtd_iova_to_slpte(VTDContextEntry *ce, uint64_t iova, bool is_write, VTD_DPRINTF(GENERAL, "error: fail to access second-level paging " "entry at level %"PRIu32 " for iova 0x%"PRIx64, level, iova); -if (level == vtd_get_level_from_context_entry(ce)) { +if (level == vtd_ce_get_level(ce)) { /* Invalid programming of context-entry */ return -VTD_FR_CONTEXT_ENTRY_INV; } else { @@ -809,8 +809,8 @@ static int vtd_page_walk(VTDContextEntry *ce, uint64_t start, uint64_t end, vtd_page_walk_hook hook_fn, void *private, bool notify_unmap) { -dma_addr_t addr = vtd_get_slpt_base_from_context(ce); -uint32_t level = vtd_get_level_from_context_entry(ce); +dma_addr_t addr = vtd_ce_get_slpt_base(ce); +uint32_t level = vtd_ce_get_level(ce); if (!vtd_iova_range_check(start, ce)) { return -VTD_FR_ADDR_BEYOND_MGAW; @@ -851,7 +851,7 @@ static int vtd_dev_to_context_entry(IntelIOMMUState *s, uint8_t bus_num, return ret_fr; } -if (!vtd_context_entry_present(ce)) { +if (!vtd_ce_present(ce)) { /* Not error - it's okay we don't have context entry. */ trace_vtd_ce_not_present(bus_num, devfn); return -VTD_FR_CONTEXT_ENTRY_P; @@ -861,7 +861,7 @@ static int vtd_dev_to_context_entry(IntelIOMMUState *s, uint8_t bus_num, return -VTD_FR_CONTEXT_ENTRY_RSVD; } /* Check if the programming of context-entry is valid */ -if (!vtd_is_level_supported(s, vtd_get_level_from_context_entry(ce))) { +if (!vtd_is_level_supported(s, vtd_ce_get_level(ce))) { trace_vtd_ce_invalid(ce->hi, ce->lo); return -VTD_FR_CONTEXT_ENTRY_INV; } else { -- MST
[Qemu-devel] [PULL 12/13] pc: ACPI BIOS: use highest NUMA node for hotplug mem hole SRAT entry
From: Ladi Prosek For reasons unknown, Windows won't online all memory, both at command line and hot-plugged later, unless the hotplug mem hole SRAT entry specifies a node greater than or equal to the ones where memory is added. Using the highest node on the machine makes recent versions of Windows happy. With this example command line: ... \ -m 1024,slots=4,maxmem=32G \ -numa node,nodeid=0 \ -numa node,nodeid=1 \ -numa node,nodeid=2 \ -numa node,nodeid=3 \ -object memory-backend-ram,size=1G,id=mem-mem1 \ -device pc-dimm,id=dimm-mem1,memdev=mem-mem1,node=1 Windows reports a total of 1G of RAM without this commit and the expected 2G with this commit. Signed-off-by: Ladi Prosek Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Igor Mammedov Acked-by: Laszlo Ersek --- hw/i386/acpi-build.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index afcadac..82bd44f 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -2404,14 +2404,17 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine) } /* - * Entry is required for Windows to enable memory hotplug in OS. + * Entry is required for Windows to enable memory hotplug in OS + * and for Linux to enable SWIOTLB when booted with less than + * 4G of RAM. Windows works better if the entry sets proximity + * to the highest NUMA node in the machine. * Memory devices may override proximity set by this entry, * providing _PXM method if necessary. */ if (hotplugabble_address_space_size) { numamem = acpi_data_push(table_data, sizeof *numamem); build_srat_memory(numamem, pcms->hotplug_memory.base, - hotplugabble_address_space_size, 0, + hotplugabble_address_space_size, pcms->numa_nodes - 1, MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED); } -- MST
[Qemu-devel] [PULL 08/13] intel_iommu: support passthrough (PT)
From: Peter Xu Hardware support for VT-d device passthrough. Although current Linux can live with iommu=pt even without this, but this is faster than when using software passthrough. Signed-off-by: Peter Xu Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Liu, Yi L Reviewed-by: Jason Wang --- hw/i386/intel_iommu_internal.h | 1 + include/hw/i386/x86-iommu.h| 1 + hw/i386/intel_iommu.c | 231 ++--- hw/i386/x86-iommu.c| 1 + hw/i386/trace-events | 2 + 5 files changed, 177 insertions(+), 59 deletions(-) diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h index 29d6707..0e73a65 100644 --- a/hw/i386/intel_iommu_internal.h +++ b/hw/i386/intel_iommu_internal.h @@ -187,6 +187,7 @@ /* Interrupt Remapping support */ #define VTD_ECAP_IR (1ULL << 3) #define VTD_ECAP_EIM(1ULL << 4) +#define VTD_ECAP_PT (1ULL << 6) #define VTD_ECAP_MHMV (15ULL << 20) /* CAP_REG */ diff --git a/include/hw/i386/x86-iommu.h b/include/hw/i386/x86-iommu.h index 361c07c..ef89c0c 100644 --- a/include/hw/i386/x86-iommu.h +++ b/include/hw/i386/x86-iommu.h @@ -74,6 +74,7 @@ struct X86IOMMUState { SysBusDevice busdev; bool intr_supported;/* Whether vIOMMU supports IR */ bool dt_supported; /* Whether vIOMMU supports DT */ +bool pt_supported; /* Whether vIOMMU supports pass-through */ IommuType type; /* IOMMU type - AMD/Intel */ QLIST_HEAD(, IEC_Notifier) iec_notifiers; /* IEC notify list */ }; diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index aac2cc7..15610b9 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -613,6 +613,11 @@ static inline bool vtd_ce_type_check(X86IOMMUState *x86_iommu, return false; } break; +case VTD_CONTEXT_TT_PASS_THROUGH: +if (!x86_iommu->pt_supported) { +return false; +} +break; default: /* Unknwon type */ return false; @@ -660,6 +665,29 @@ static bool vtd_slpte_nonzero_rsvd(uint64_t slpte, uint32_t level) } } +/* Find the VTD address space associated with a given bus number */ +static VTDBus *vtd_find_as_from_bus_num(IntelIOMMUState *s, uint8_t bus_num) +{ +VTDBus *vtd_bus = s->vtd_as_by_bus_num[bus_num]; +if (!vtd_bus) { +/* + * Iterate over the registered buses to find the one which + * currently hold this bus number, and update the bus_num + * lookup table: + */ +GHashTableIter iter; + +g_hash_table_iter_init(&iter, s->vtd_as_by_busptr); +while (g_hash_table_iter_next(&iter, NULL, (void **)&vtd_bus)) { +if (pci_bus_num(vtd_bus->bus) == bus_num) { +s->vtd_as_by_bus_num[bus_num] = vtd_bus; +return vtd_bus; +} +} +} +return vtd_bus; +} + /* Given the @iova, get relevant @slptep. @slpte_level will be the last level * of the translation, can be used for deciding the size of large page. */ @@ -906,6 +934,91 @@ static int vtd_dev_to_context_entry(IntelIOMMUState *s, uint8_t bus_num, return 0; } +/* + * Fetch translation type for specific device. Returns <0 if error + * happens, otherwise return the shifted type to check against + * VTD_CONTEXT_TT_*. + */ +static int vtd_dev_get_trans_type(VTDAddressSpace *as) +{ +IntelIOMMUState *s; +VTDContextEntry ce; +int ret; + +s = as->iommu_state; + +ret = vtd_dev_to_context_entry(s, pci_bus_num(as->bus), + as->devfn, &ce); +if (ret) { +return ret; +} + +return vtd_ce_get_type(&ce); +} + +static bool vtd_dev_pt_enabled(VTDAddressSpace *as) +{ +int ret; + +assert(as); + +ret = vtd_dev_get_trans_type(as); +if (ret < 0) { +/* + * Possibly failed to parse the context entry for some reason + * (e.g., during init, or any guest configuration errors on + * context entries). We should assume PT not enabled for + * safety. + */ +return false; +} + +return ret == VTD_CONTEXT_TT_PASS_THROUGH; +} + +/* Return whether the device is using IOMMU translation. */ +static bool vtd_switch_address_space(VTDAddressSpace *as) +{ +bool use_iommu; + +assert(as); + +use_iommu = as->iommu_state->dmar_enabled & !vtd_dev_pt_enabled(as); + +trace_vtd_switch_address_space(pci_bus_num(as->bus), + VTD_PCI_SLOT(as->devfn), + VTD_PCI_FUNC(as->devfn), + use_iommu); + +/* Turn off first then on the other */ +if (use_iommu) { +memory_region_set_enabled(&as->sys_alias, false); +memory_region_set_enabled(&as->iommu, true); +} else { +memory_region_set_enable
[Qemu-devel] [PULL 06/13] intel_iommu: use IOMMU_ACCESS_FLAG()
From: Peter Xu We have that now, so why not use it. Signed-off-by: Peter Xu Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Jason Wang --- hw/i386/intel_iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index b477143..3240e5d 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -1010,7 +1010,7 @@ out: entry->iova = addr & page_mask; entry->translated_addr = vtd_get_slpte_addr(slpte) & page_mask; entry->addr_mask = ~page_mask; -entry->perm = (writes ? 2 : 0) + (reads ? 1 : 0); +entry->perm = IOMMU_ACCESS_FLAG(reads, writes); } static void vtd_root_table_setup(IntelIOMMUState *s) -- MST
[Qemu-devel] [PULL 07/13] intel_iommu: allow dev-iotlb context entry conditionally
From: Peter Xu When device-iotlb is not specified, we should fail this check. A new function vtd_ce_type_check() is introduced. While I'm at it, clean up the vtd_dev_to_context_entry() a bit - replace many "else if" usage into direct if check. That'll make the logic more clear. Signed-off-by: Peter Xu Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Jason Wang --- hw/i386/intel_iommu.c | 49 - 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 3240e5d..aac2cc7 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -600,6 +600,26 @@ static inline uint32_t vtd_ce_get_type(VTDContextEntry *ce) return ce->lo & VTD_CONTEXT_ENTRY_TT; } +/* Return true if check passed, otherwise false */ +static inline bool vtd_ce_type_check(X86IOMMUState *x86_iommu, + VTDContextEntry *ce) +{ +switch (vtd_ce_get_type(ce)) { +case VTD_CONTEXT_TT_MULTI_LEVEL: +/* Always supported */ +break; +case VTD_CONTEXT_TT_DEV_IOTLB: +if (!x86_iommu->dt_supported) { +return false; +} +break; +default: +/* Unknwon type */ +return false; +} +return true; +} + static inline uint64_t vtd_iova_limit(VTDContextEntry *ce) { uint32_t ce_agaw = vtd_ce_get_agaw(ce); @@ -836,6 +856,7 @@ static int vtd_dev_to_context_entry(IntelIOMMUState *s, uint8_t bus_num, { VTDRootEntry re; int ret_fr; +X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s); ret_fr = vtd_get_root_entry(s, bus_num, &re); if (ret_fr) { @@ -846,7 +867,9 @@ static int vtd_dev_to_context_entry(IntelIOMMUState *s, uint8_t bus_num, /* Not error - it's okay we don't have root entry. */ trace_vtd_re_not_present(bus_num); return -VTD_FR_ROOT_ENTRY_P; -} else if (re.rsvd || (re.val & VTD_ROOT_ENTRY_RSVD)) { +} + +if (re.rsvd || (re.val & VTD_ROOT_ENTRY_RSVD)) { trace_vtd_re_invalid(re.rsvd, re.val); return -VTD_FR_ROOT_ENTRY_RSVD; } @@ -860,26 +883,26 @@ static int vtd_dev_to_context_entry(IntelIOMMUState *s, uint8_t bus_num, /* Not error - it's okay we don't have context entry. */ trace_vtd_ce_not_present(bus_num, devfn); return -VTD_FR_CONTEXT_ENTRY_P; -} else if ((ce->hi & VTD_CONTEXT_ENTRY_RSVD_HI) || - (ce->lo & VTD_CONTEXT_ENTRY_RSVD_LO)) { +} + +if ((ce->hi & VTD_CONTEXT_ENTRY_RSVD_HI) || +(ce->lo & VTD_CONTEXT_ENTRY_RSVD_LO)) { trace_vtd_ce_invalid(ce->hi, ce->lo); return -VTD_FR_CONTEXT_ENTRY_RSVD; } + /* Check if the programming of context-entry is valid */ if (!vtd_is_level_supported(s, vtd_ce_get_level(ce))) { trace_vtd_ce_invalid(ce->hi, ce->lo); return -VTD_FR_CONTEXT_ENTRY_INV; -} else { -switch (vtd_ce_get_type(ce)) { -case VTD_CONTEXT_TT_MULTI_LEVEL: -/* fall through */ -case VTD_CONTEXT_TT_DEV_IOTLB: -break; -default: -trace_vtd_ce_invalid(ce->hi, ce->lo); -return -VTD_FR_CONTEXT_ENTRY_INV; -} } + +/* Do translation type check */ +if (!vtd_ce_type_check(x86_iommu, ce)) { +trace_vtd_ce_invalid(ce->hi, ce->lo); +return -VTD_FR_CONTEXT_ENTRY_INV; +} + return 0; } -- MST
[Qemu-devel] [PULL 09/13] intel_iommu: turn off pt before 2.9
From: Peter Xu This is for compatibility. Signed-off-by: Peter Xu Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Jason Wang --- include/hw/compat.h | 4 1 file changed, 4 insertions(+) diff --git a/include/hw/compat.h b/include/hw/compat.h index 55b1765..4c53d60 100644 --- a/include/hw/compat.h +++ b/include/hw/compat.h @@ -6,6 +6,10 @@ .driver = "pci-bridge",\ .property = "shpc",\ .value= "off",\ +},{\ +.driver = "intel-iommu",\ +.property = "pt",\ +.value= "off",\ }, #define HW_COMPAT_2_8 \ -- MST
[Qemu-devel] [PULL 11/13] vhost-user: pass message as a pointer to process_message_reply()
From: Maxime Coquelin process_message_reply() was recently updated to get full message content instead of only its request field. There is no need to copy all the struct content into the stack, so just pass its pointer as const. Reviewed-by: Jens Freimann Reviewed-by: Zhiyong Yang Signed-off-by: Maxime Coquelin Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Marc-André Lureau --- hw/virtio/vhost-user.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index b87a176..dde094a 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -162,11 +162,11 @@ fail: } static int process_message_reply(struct vhost_dev *dev, - VhostUserMsg msg) + const VhostUserMsg *msg) { VhostUserMsg msg_reply; -if ((msg.flags & VHOST_USER_NEED_REPLY_MASK) == 0) { +if ((msg->flags & VHOST_USER_NEED_REPLY_MASK) == 0) { return 0; } @@ -174,10 +174,10 @@ static int process_message_reply(struct vhost_dev *dev, return -1; } -if (msg_reply.request != msg.request) { +if (msg_reply.request != msg->request) { error_report("Received unexpected msg type." "Expected %d received %d", - msg.request, msg_reply.request); + msg->request, msg_reply.request); return -1; } @@ -324,7 +324,7 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev, } if (reply_supported) { -return process_message_reply(dev, msg); +return process_message_reply(dev, &msg); } return 0; @@ -716,7 +716,7 @@ static int vhost_user_net_set_mtu(struct vhost_dev *dev, uint16_t mtu) /* If reply_ack supported, slave has to ack specified MTU is valid */ if (reply_supported) { -return process_message_reply(dev, msg); +return process_message_reply(dev, &msg); } return 0; -- MST
[Qemu-devel] [PULL 10/13] virtio_net: Bypass backends for MTU feature negotiation
From: Maxime Coquelin This patch adds a new internal "x-mtu-bypass-backend" property to bypass backends for MTU feature negotiation. When this property is set, the MTU feature is negotiated as soon as supported by the guest and a MTU value is set via the host_mtu parameter. In case the backend advertises the feature (e.g. DPDK's vhost-user backend), the feature negotiation is propagated down to the backend. When this property is not set, the backend has to support the MTU feature for its negotiation to succeed. For compatibility purpose, this property is disabled for machine types v2.9 and older. Cc: Aaron Conole Suggested-by: Michael S. Tsirkin Signed-off-by: Maxime Coquelin Reviewed-by: Vlad Yasevich Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- include/hw/compat.h| 4 include/hw/virtio/virtio-net.h | 1 + include/hw/virtio/virtio.h | 1 + hw/net/virtio-net.c| 17 - 4 files changed, 22 insertions(+), 1 deletion(-) diff --git a/include/hw/compat.h b/include/hw/compat.h index 4c53d60..400c64b 100644 --- a/include/hw/compat.h +++ b/include/hw/compat.h @@ -10,6 +10,10 @@ .driver = "intel-iommu",\ .property = "pt",\ .value= "off",\ +},{\ +.driver = "virtio-net-device",\ +.property = "x-mtu-bypass-backend",\ +.value= "off",\ }, #define HW_COMPAT_2_8 \ diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h index 1eec9a2..602b486 100644 --- a/include/hw/virtio/virtio-net.h +++ b/include/hw/virtio/virtio-net.h @@ -97,6 +97,7 @@ typedef struct VirtIONet { QEMUTimer *announce_timer; int announce_counter; bool needs_vnet_hdr_swap; +bool mtu_bypass_backend; } VirtIONet; void virtio_net_set_netclient_name(VirtIONet *n, const char *name, diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 7b6edba..80c45c3 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -79,6 +79,7 @@ struct VirtIODevice uint16_t queue_sel; uint64_t guest_features; uint64_t host_features; +uint64_t backend_features; size_t config_len; void *config; uint16_t config_vector; diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 98bd683..9a3d769 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -589,7 +589,15 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features, if (!get_vhost_net(nc->peer)) { return features; } -return vhost_net_get_features(get_vhost_net(nc->peer), features); +features = vhost_net_get_features(get_vhost_net(nc->peer), features); +vdev->backend_features = features; + +if (n->mtu_bypass_backend && +(n->host_features & 1ULL << VIRTIO_NET_F_MTU)) { +features |= (1ULL << VIRTIO_NET_F_MTU); +} + +return features; } static uint64_t virtio_net_bad_features(VirtIODevice *vdev) @@ -640,6 +648,11 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) VirtIONet *n = VIRTIO_NET(vdev); int i; +if (n->mtu_bypass_backend && +!virtio_has_feature(vdev->backend_features, VIRTIO_NET_F_MTU)) { +features &= ~(1ULL << VIRTIO_NET_F_MTU); +} + virtio_net_set_multiqueue(n, virtio_has_feature(features, VIRTIO_NET_F_MQ)); @@ -2093,6 +2106,8 @@ static Property virtio_net_properties[] = { DEFINE_PROP_UINT16("rx_queue_size", VirtIONet, net_conf.rx_queue_size, VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE), DEFINE_PROP_UINT16("host_mtu", VirtIONet, net_conf.mtu, 0), +DEFINE_PROP_BOOL("x-mtu-bypass-backend", VirtIONet, mtu_bypass_backend, + true), DEFINE_PROP_END_OF_LIST(), }; -- MST
[Qemu-devel] [PULL 13/13] acpi-test: update expected files
commit 1a8d61ddbf ("pc: ACPI BIOS: use highest NUMA node for hotplug mem hole SRAT entry") changed generated SRAT tables, update expected files accordingly. Signed-off-by: Michael S. Tsirkin --- tests/acpi-test-data/pc/SRAT.memhp | Bin 264 -> 264 bytes tests/acpi-test-data/q35/SRAT.memhp | Bin 264 -> 264 bytes 2 files changed, 0 insertions(+), 0 deletions(-) diff --git a/tests/acpi-test-data/pc/SRAT.memhp b/tests/acpi-test-data/pc/SRAT.memhp index a7dddf7760698193819e60a3a96f2a57cf367522..e508b4ae3cd9e3000209a4f9597913faa4206ec1 100644 GIT binary patch delta 23 ecmeBR>R{pu4ss0PU}RumTr`pE1|#Fd$EpB8iv|+_ delta 24 gcmeBR>R{pu4ss0PU}RumTs)EM#>8D*6W_=H08BjxX8-^I diff --git a/tests/acpi-test-data/q35/SRAT.memhp b/tests/acpi-test-data/q35/SRAT.memhp index a7dddf7760698193819e60a3a96f2a57cf367522..e508b4ae3cd9e3000209a4f9597913faa4206ec1 100644 GIT binary patch delta 23 ecmeBR>R{pu4ss0PU}RumTr`pE1|#Fd$EpB8iv|+_ delta 24 gcmeBR>R{pu4ss0PU}RumTs)EM#>8D*6W_=H08BjxX8-^I -- MST
[Qemu-devel] [PATCH] exec: fix address_space_get_iotlb_entry page mask
The IOTLB that it returned didn't guarantee that page_mask is indeed a so-called page mask. That won't affect current usage since now only vhost is using it (vhost API allows arbitary IOTLB range). However we have IOTLB scemantic and we should best follow it. This patch fixes this issue to make sure the page_mask is always a valid page mask. Fixes: a764040 ("exec: abstract address_space_do_translate()") Signed-off-by: Peter Xu --- exec.c | 20 +--- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/exec.c b/exec.c index ff16f04..7026c21 100644 --- a/exec.c +++ b/exec.c @@ -519,6 +519,15 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr, section = address_space_do_translate(as, addr, &xlat, &plen, is_write, false); +if (plen == (hwaddr)-1) { +/* If not specified during translation, use default mask */ +plen = TARGET_PAGE_MASK; +} else { +/* Make it a valid page mask */ +assert(plen); +plen = (1ULL << (63 - clz64(plen))) - 1; +} + /* Illegal translation */ if (section.mr == &io_mem_unassigned) { goto iotlb_fail; @@ -528,17 +537,6 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr, xlat += section.offset_within_address_space - section.offset_within_region; -if (plen == (hwaddr)-1) { -/* - * We use default page size here. Logically it only happens - * for identity mappings. - */ -plen = TARGET_PAGE_SIZE; -} - -/* Convert to address mask */ -plen -= 1; - return (IOMMUTLBEntry) { .target_as = section.address_space, .iova = addr & ~plen, -- 2.7.4
[Qemu-devel] [PATCH v2] exec: fix address_space_get_iotlb_entry page mask
The IOTLB that it returned didn't guarantee that page_mask is indeed a so-called page mask. That won't affect current usage since now only vhost is using it (vhost API allows arbitary IOTLB range). However we have IOTLB scemantic and we should best follow it. This patch fixes this issue to make sure the page_mask is always a valid page mask. Fixes: a764040 ("exec: abstract address_space_do_translate()") Signed-off-by: Peter Xu --- Sorry please use this one. The codes is merely the same, just avoided moving the codes around unnecessarily. --- exec.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/exec.c b/exec.c index ff16f04..3db247c 100644 --- a/exec.c +++ b/exec.c @@ -529,16 +529,14 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr, section.offset_within_region; if (plen == (hwaddr)-1) { -/* - * We use default page size here. Logically it only happens - * for identity mappings. - */ -plen = TARGET_PAGE_SIZE; +/* If not specified during translation, use default mask */ +plen = TARGET_PAGE_MASK; +} else { +/* Make it a valid page mask */ +assert(plen); +plen = (1ULL << (63 - clz64(plen))) - 1; } -/* Convert to address mask */ -plen -= 1; - return (IOMMUTLBEntry) { .target_as = section.address_space, .iova = addr & ~plen, -- 2.7.4
Re: [Qemu-devel] [PATCH] exec: fix address_space_get_iotlb_entry page mask
Hi, This series seems to have some coding style problems. See output below for more information: Type: series Subject: [Qemu-devel] [PATCH] exec: fix address_space_get_iotlb_entry page mask Message-id: 1496029936-6381-1-git-send-email-pet...@redhat.com === TEST SCRIPT BEGIN === #!/bin/bash BASE=base n=1 total=$(git log --oneline $BASE.. | wc -l) failed=0 git config --local diff.renamelimit 0 git config --local diff.renames True commits="$(git log --format=%H --reverse $BASE..)" for c in $commits; do echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..." if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then failed=1 echo fi n=$((n+1)) done exit $failed === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/1496029936-6381-1-git-send-email-pet...@redhat.com -> patchew/1496029936-6381-1-git-send-email-pet...@redhat.com Switched to a new branch 'test' 8778a42 exec: fix address_space_get_iotlb_entry page mask === OUTPUT BEGIN === Checking PATCH 1/1: exec: fix address_space_get_iotlb_entry page mask... ERROR: spaces required around that '-' (ctx:VxV) #24: FILE: exec.c:522: +if (plen == (hwaddr)-1) { ^ total: 1 errors, 0 warnings, 32 lines checked Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-de...@freelists.org
Re: [Qemu-devel] [PATCH v4 10/10] vhost: iommu: cache static mapping if there is
On Thu, May 25, 2017 at 09:14:55PM +0300, Michael S. Tsirkin wrote: > On Mon, May 22, 2017 at 10:42:00AM +0800, Peter Xu wrote: > > On Fri, May 19, 2017 at 07:55:26PM +0300, Michael S. Tsirkin wrote: > > > On Fri, May 19, 2017 at 11:19:49AM +0800, Peter Xu wrote: > > > > This patch pre-heat vhost iotlb cache when passthrough mode enabled. > > > > > > > > Sometimes, even if user specified iommu_platform for vhost devices, > > > > IOMMU might still be disabled. One case is passthrough mode in VT-d > > > > implementation. We can detect this by observing iommu_list. If it's > > > > empty, it means IOMMU translation is disabled, then we can actually > > > > pre-heat the translation (it'll be static mapping then) by first > > > > invalidating all IOTLB, then cache existing memory ranges into vhost > > > > backend iotlb using 1:1 mapping. > > > > > > > > Signed-off-by: Peter Xu > > > > > > I don't really understand. Is this a performance optimization? > > > Can you post some #s please? > > > > Yes, it is. Vhost can work even without this patch, but it should be > > faster when with this patch. > > You'll have to include perf testing numbers then. My mistake to not have compared the numbers before, since it's just so obvious to me that this patch should help. Though after some simple streaming tests, it shows that this patch didn't boost performance at all. I added some traces in vhost kernel to know what's happened, please see below. Without this patch, boot with iommu=pt, I see IOTLB cache insertion like this: vhost_process_iotlb_msg: iotlb new: 1 (0x17879b240-0x17fff) vhost_process_iotlb_msg: iotlb new: 2 (0x17879d240-0x17fff) vhost_process_iotlb_msg: iotlb new: 3 (0x17879a000-0x17fff) vhost_process_iotlb_msg: iotlb new: 4 (0x17857-0x17fff) vhost_process_iotlb_msg: iotlb new: 5 (0x178532606-0x17fff) vhost_process_iotlb_msg: iotlb new: 6 (0x177bad0e2-0x17fff) vhost_process_iotlb_msg: iotlb new: 7 (0x1768560e2-0x17fff) (Note: we can see the pattern is (ADDR-0x17fff), while ADDR is increasing, finally the range will cover all addresses that vhost needs for DMA, then we won't have cache miss, and 0x17fff is the upper limit for a 4G memory guest) While after this patch (this is expected): vhost_process_iotlb_msg: iotlb new: 1 (0x1-0x17fff) vhost_process_iotlb_msg: iotlb new: 2 (0x0-0x9) vhost_process_iotlb_msg: iotlb new: 3 (0xc-0x7fff) (Note: this is one entry per RAM memory region) So it explained well on why performance didn't really change even before applying this patch: currently when iommu=pt is on, address_space_get_iotlb_entry() can get IOTLB that is bigger than page size (if you see the code, plen decides the page mask, and plen is only limited by memory region sizes when PT is enabled). So until the 7th cache miss IOTLB request, the range is big enough to cover the rest of DMA addresses. My preference is that we still apply this patch even there is no performance gain on simple streaming test. Reasons: - the old code has good performance depending on implementation of address_space_get_iotlb_entry(), which may alter in the future - after apply the patch, we are 100% sure that we won't cache miss, while we cannot guarantee that without it. If not apply the patch, we may still encounter cache miss (e.g., access address <0x1768560e2 after the 7th cache miss in above test), which can introduce that cache-missed IO to be delayed. > > > As mentioned in the commit message and below comment [1], this patch > > pre-heat the cache for vhost. Currently the cache entries depends on > > the system memory ranges (dev->mem->nregions), and it should be far > > smaller than vhost's cache count (currently it is statically defined > > as max_iotlb_entries=2048 in kernel). If with current patch, these > > cache entries can cover the whole possible DMA ranges that PT mode > > would allow, so we won't have any cache miss then. > > > > For the comments, do you have any better suggestion besides commit > > message and [1]? > > > > > > > > Also, if it's PT, can't we bypass iommu altogether? That would be > > > even faster ... > > > > Yes, but I don't yet know a good way to do it... Any suggestion is > > welcomed as well. > > > > Btw, do you have any comment on other patches besides this one? Since > > this patch can really be isolated from the whole PT support series. > > > > Thanks, > > I've applied the rest of the series. Thank you very much. -- Peter Xu
Re: [Qemu-devel] [Qemu devel v5 PATCH 4/5] msf2: Add Smartfusion2 SoC.
Hi Alistair, On Sat, May 27, 2017 at 5:18 AM, Alistair Francis wrote: > On Tue, May 16, 2017 at 8:38 AM, Subbaraya Sundeep > wrote: > > Smartfusion2 SoC has hardened Microcontroller subsystem > > and flash based FPGA fabric. This patch adds support for > > Microcontroller subsystem in the SoC. > > > > Signed-off-by: Subbaraya Sundeep > > --- > > default-configs/arm-softmmu.mak | 1 + > > hw/arm/Makefile.objs| 1 + > > hw/arm/msf2-soc.c | 201 ++ > ++ > > include/hw/arm/msf2-soc.h | 69 ++ > > 4 files changed, 272 insertions(+) > > create mode 100644 hw/arm/msf2-soc.c > > create mode 100644 include/hw/arm/msf2-soc.h > > > > diff --git a/default-configs/arm-softmmu.mak > b/default-configs/arm-softmmu.mak > > index 78d7af0..7062512 100644 > > --- a/default-configs/arm-softmmu.mak > > +++ b/default-configs/arm-softmmu.mak > > @@ -122,3 +122,4 @@ CONFIG_ACPI=y > > CONFIG_SMBIOS=y > > CONFIG_ASPEED_SOC=y > > CONFIG_GPIO_KEY=y > > +CONFIG_MSF2=y > > diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs > > index 4c5c4ee..c828061 100644 > > --- a/hw/arm/Makefile.objs > > +++ b/hw/arm/Makefile.objs > > @@ -18,3 +18,4 @@ obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o > > obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o kzm.o > > obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o sabrelite.o > > obj-$(CONFIG_ASPEED_SOC) += aspeed_soc.o aspeed.o > > +obj-$(CONFIG_MSF2) += msf2-soc.o > > diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c > > new file mode 100644 > > index 000..329e30c > > --- /dev/null > > +++ b/hw/arm/msf2-soc.c > > @@ -0,0 +1,201 @@ > > +/* > > + * SmartFusion2 SoC emulation. > > + * > > + * Copyright (c) 2017 Subbaraya Sundeep > > + * > > + * Permission is hereby granted, free of charge, to any person > obtaining a copy > > + * of this software and associated documentation files (the > "Software"), to deal > > + * in the Software without restriction, including without limitation > the rights > > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or > sell > > + * copies of the Software, and to permit persons to whom the Software is > > + * furnished to do so, subject to the following conditions: > > + * > > + * The above copyright notice and this permission notice shall be > included in > > + * all copies or substantial portions of the Software. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > EXPRESS OR > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > MERCHANTABILITY, > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT > SHALL > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR > OTHER > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > ARISING FROM, > > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > DEALINGS IN > > + * THE SOFTWARE. > > + */ > > + > > +#include "qemu/osdep.h" > > +#include "qapi/error.h" > > +#include "qemu-common.h" > > +#include "hw/arm/arm.h" > > +#include "exec/address-spaces.h" > > +#include "hw/char/serial.h" > > +#include "hw/boards.h" > > +#include "sysemu/block-backend.h" > > +#include "hw/arm/msf2-soc.h" > > + > > +#define MSF2_TIMER_BASE 0x40004000 > > +#define MSF2_SYSREG_BASE0x40038000 > > + > > +#define MSF2_TIMER_IRQ0 14 > > +#define MSF2_TIMER_IRQ1 15 > > + > > +static const uint32_t spi_addr[MSF2_NUM_SPIS] = { 0x40001000 , > 0x40011000 }; > > +static const uint32_t uart_addr[MSF2_NUM_UARTS] = { 0x4000 , > 0x4001 }; > > + > > +static const int spi_irq[MSF2_NUM_SPIS] = { 2, 3 }; > > +static const int uart_irq[MSF2_NUM_UARTS] = { 10, 11 }; > > + > > +static void m2sxxx_soc_initfn(Object *obj) > > +{ > > +MSF2State *s = MSF2_SOC(obj); > > +int i; > > + > > +object_initialize(&s->armv7m, sizeof(s->armv7m), TYPE_ARMV7M); > > +qdev_set_parent_bus(DEVICE(&s->armv7m), sysbus_get_default()); > > + > > +object_initialize(&s->sysreg, sizeof(s->sysreg), TYPE_MSF2_SYSREG); > > +qdev_set_parent_bus(DEVICE(&s->sysreg), sysbus_get_default()); > > + > > +object_initialize(&s->timer, sizeof(s->timer), TYPE_MSS_TIMER); > > +qdev_set_parent_bus(DEVICE(&s->timer), sysbus_get_default()); > > + > > +for (i = 0; i < MSF2_NUM_SPIS; i++) { > > +object_initialize(&s->spi[i], sizeof(s->spi[i]), > > + TYPE_MSS_SPI); > > +qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default()); > > +} > > +} > > + > > +static void m2sxxx_soc_realize(DeviceState *dev_soc, Error **errp) > > +{ > > +MSF2State *s = MSF2_SOC(dev_soc); > > +DeviceState *dev, *armv7m; > > +SysBusDevice *busdev; > > +Error *err = NULL; > > +int i; > > + > > +MemoryRegion *system_memory = get_system_memory(); > > +MemoryRegion *nvm = g_new(MemoryRegion, 1); > > +MemoryRegion *nvm_alias = g_new(MemoryRegion, 1); > > +MemoryRegion *sram = g_new(MemoryRe
Re: [Qemu-devel] [Qemu devel v5 PATCH 5/5] msf2: Add Emcraft's Smartfusion2 SOM kit.
Hi Alistair, On Sat, May 27, 2017 at 5:30 AM, Alistair Francis wrote: > On Tue, May 16, 2017 at 8:38 AM, Subbaraya Sundeep > wrote: > > Emulated Emcraft's Smartfusion2 System On Module starter > > kit. > > > > Signed-off-by: Subbaraya Sundeep > > --- > > hw/arm/Makefile.objs | 1 + > > hw/arm/msf2-som.c| 89 ++ > ++ > > 2 files changed, 90 insertions(+) > > create mode 100644 hw/arm/msf2-som.c > > > > diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs > > index c828061..4b02093 100644 > > --- a/hw/arm/Makefile.objs > > +++ b/hw/arm/Makefile.objs > > @@ -5,6 +5,7 @@ obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o > > obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o > > obj-$(CONFIG_ACPI) += virt-acpi-build.o > > obj-y += netduino2.o > > +obj-y += msf2-som.o > > This should be obj-$(CONFIG_MSF2). > Ok will change it. > > > obj-y += sysbus-fdt.o > > > > obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o > > diff --git a/hw/arm/msf2-som.c b/hw/arm/msf2-som.c > > new file mode 100644 > > index 000..cd2b759 > > --- /dev/null > > +++ b/hw/arm/msf2-som.c > > @@ -0,0 +1,89 @@ > > +/* > > + * SmartFusion2 SOM starter kit(from Emcraft) emulation. > > + * > > + * Copyright (c) 2017 Subbaraya Sundeep > > + * > > + * Permission is hereby granted, free of charge, to any person > obtaining a copy > > + * of this software and associated documentation files (the > "Software"), to deal > > + * in the Software without restriction, including without limitation > the rights > > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or > sell > > + * copies of the Software, and to permit persons to whom the Software is > > + * furnished to do so, subject to the following conditions: > > + * > > + * The above copyright notice and this permission notice shall be > included in > > + * all copies or substantial portions of the Software. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > EXPRESS OR > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > MERCHANTABILITY, > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT > SHALL > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR > OTHER > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > ARISING FROM, > > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > DEALINGS IN > > + * THE SOFTWARE. > > + */ > > + > > +#include "qemu/osdep.h" > > +#include "qapi/error.h" > > +#include "hw/boards.h" > > +#include "hw/arm/msf2-soc.h" > > +#include "hw/arm/arm.h" > > +#include "exec/address-spaces.h" > > + > > +#define DDR_BASE_ADDRESS 0xA000 > > +#define DDR_SIZE (64 * M_BYTE) > > + > > +static void emcraft_sf2_init(MachineState *machine) > > +{ > > +DeviceState *dev; > > +DeviceState *spi_flash; > > +MSF2State *soc; > > +DriveInfo *dinfo = drive_get_next(IF_MTD); > > +qemu_irq cs_line; > > +SSIBus *spi_bus; > > +MemoryRegion *sysmem = get_system_memory(); > > +MemoryRegion *ddr = g_new(MemoryRegion, 1); > > + > > +memory_region_init_ram(ddr, NULL, "ddr-ram", DDR_SIZE, > > + &error_fatal); > > +vmstate_register_ram_global(ddr); > > +memory_region_add_subregion(sysmem, DDR_BASE_ADDRESS, ddr); > > The user can use -m to specify the amount of RAM to create in the > machine. Unless this board only ever includes 64MB of RAM you should > use that option (you will need to sanity check it though). If the > board only ever has 64MB it might be worth printing a warning to the > user if they specify an something. Although there might be a default > if they don't use -m, which makes it hard to print out a warning > message. > This -m confuses me. Why is it necessary for an embedded board? RAM chip is fixed and not extendable. Whereas normal PC may have extra RAM slots. If another board has more RAM then we would instantiate another machine for it with that RAM size. Please explain. Maybe I am thinking in wrong direction. Thanks, Sundeep > > > + > > +dev = qdev_create(NULL, TYPE_MSF2_SOC); > > +qdev_prop_set_string(dev, "part-name", "M2S010"); > > +qdev_prop_set_uint64(dev, "eNVM-size", M2S010_ENVM_SIZE); > > +qdev_prop_set_uint64(dev, "eSRAM-size", M2S010_ESRAM_SIZE); > > + > > +/* > > + * pclk0 and pclk1 are configurable in Libero. > > + * Emcraft's SoM kit comes with these settings by default. > > + */ > > +qdev_prop_set_uint32(dev, "pclk0", 71 * 100); > > +qdev_prop_set_uint32(dev, "pclk1", 71 * 100); > > + > > +object_property_set_bool(OBJECT(dev), true, "realized", > &error_fatal); > > + > > +soc = MSF2_SOC(dev); > > + > > +/* Attach SPI flash to SPI0 controller */ > > +spi_bus = (SSIBus *)qdev_get_child_bus(dev, "spi0"); > > +spi_flash = ssi_create_slave_no_init(spi_bus, "s25sl12801
Re: [Qemu-devel] [Qemu devel v5 PATCH 0/5] Add support for Smartfusion2 SoC
Hi Philippe, Any update on this? I will wait for your comments too and send next iteration fixing Alistair comments. Thanks, Sundeep On Wed, May 17, 2017 at 3:09 PM, sundeep subbaraya wrote: > Hi Philippe, > > On Wed, May 17, 2017 at 9:57 AM, Philippe Mathieu-Daudé > wrote: > >> Hi Sundeep, >> >> This patchset is way cleaner! >> I had a fast look and I like it, I'll try to make some time soon to >> review details and test it. > > > Thank you > >> > > >> Is your work interested on U-Boot or more focused in Linux kernel? >> > > I am interested more in kernel. I had to look into u-boot for first time > for Qemu only. > I worked only on FPGAs(load kernel with debugger) till now so never got a > chance to look into u-boot. > >> >> If you compile QEMU with libfdt support you can use the -dtb option to >> pass the blob to the kernel directly, bypassing the bootloader. >> >> Yeah for armv7m I could not find any thing like that in tree. > > >> If you need a bootloader you may give a look at coreboot which supports >> dts well, see how Vladimir Serbinenko used Linux's dt to boot a QEMU >> Versatile Express board: >> https://mail.coreboot.org/pipermail/coreboot-gerrit/2016-Feb >> ruary/040899.html >> >> Cool. I will look into it. > > Thanks, > Sundeep > > >> Regards, >> >> Phil. >> >> >> On 05/16/2017 12:38 PM, Subbaraya Sundeep wrote: >> >>> Hi Qemu-devel, >>> >>> I am trying to add Smartfusion2 SoC. >>> SoC is from Microsemi and System on Module(SOM) >>> board is from Emcraft systems. Smartfusion2 has hardened >>> Microcontroller(Cortex-M3)based Sub System and FPGA fabric. >>> At the moment only system timer, sysreg and SPI >>> controller are modelled. >>> >>> Testing: >>> ./arm-softmmu/qemu-system-arm -M smartfusion2-som -serial mon:stdio \ >>> -kernel u-boot.bin -display none -drive file=spi.bin,if=mtd,format=raw >>> >>> Binaries u-boot.bin and spi.bin are at: >>> https://github.com/Subbaraya-Sundeep/qemu-test-binaries.git >>> >>> U-boot is from Emcraft with modified >>> - SPI driver not to use PDMA. >>> - ugly hack to pass dtb to kernel in r1. >>> @ >>> https://github.com/Subbaraya-Sundeep/emcraft-uboot-sf2.git >>> >>> Linux is 4.5 linux with Smartfusion2 SoC dts and clocksource >>> driver added by myself @ >>> https://github.com/Subbaraya-Sundeep/linux.git >>> >>> v5 >>> As per Philippe comments: >>> Added abort in Sysreg if guest tries to remap memory >>> other than default mapping. >>> Use of CONFIG_MSF2 in Makefile for soc.c >>> Fixed incorrect logic in timer model. >>> Renamed msf2-timer.c -> mss-timer.c >>> msf2-spi.c -> mss-spi.c also type names >>> Renamed function msf2_init->emcraft_sf2_init in msf2-som.c >>> Added part-name,eNVM-size,eSRAM-size,pclk0 and pclk1 >>> properties to soc. >>> Pass soc part-name,memory size and clock rate properties from >>> som. >>> v4: >>> Fixed build failure by using PRIx macros. >>> v3: >>> Added SoC file and board file as per Alistair comments. >>> v2: >>> Added SPI controller so that u-boot loads kernel from spi flash. >>> v1: >>> Initial patch set with timer and sysreg >>> >>> Thanks, >>> Sundeep >>> >>> Subbaraya Sundeep (5): >>> msf2: Add Smartfusion2 System timer >>> msf2: Microsemi Smartfusion2 System Register block. >>> msf2: Add Smartfusion2 SPI controller >>> msf2: Add Smartfusion2 SoC. >>> msf2: Add Emcraft's Smartfusion2 SOM kit. >>> >>> default-configs/arm-softmmu.mak | 1 + >>> hw/arm/Makefile.objs| 2 + >>> hw/arm/msf2-soc.c | 201 + >>> hw/arm/msf2-som.c | 89 ++ >>> hw/misc/Makefile.objs | 1 + >>> hw/misc/msf2-sysreg.c | 161 + >>> hw/ssi/Makefile.objs| 1 + >>> hw/ssi/mss-spi.c| 378 ++ >>> ++ >>> hw/timer/Makefile.objs | 1 + >>> hw/timer/mss-timer.c| 249 ++ >>> include/hw/arm/msf2-soc.h | 69 >>> include/hw/misc/msf2-sysreg.h | 80 + >>> include/hw/ssi/mss-spi.h| 104 +++ >>> include/hw/timer/mss-timer.h| 80 + >>> 14 files changed, 1417 insertions(+) >>> create mode 100644 hw/arm/msf2-soc.c >>> create mode 100644 hw/arm/msf2-som.c >>> create mode 100644 hw/misc/msf2-sysreg.c >>> create mode 100644 hw/ssi/mss-spi.c >>> create mode 100644 hw/timer/mss-timer.c >>> create mode 100644 include/hw/arm/msf2-soc.h >>> create mode 100644 include/hw/misc/msf2-sysreg.h >>> create mode 100644 include/hw/ssi/mss-spi.h >>> create mode 100644 include/hw/timer/mss-timer.h >>> >>> >
Re: [Qemu-devel] [PATCH] msi: remove return code for msi_init()
On Mon, May 15, 2017 at 09:14:33PM +0800, Peter Xu wrote: > MSI should be supported by all interrupt controllers. Switching the old > check for msi_nonbroken into assertion. Do similar thing to > pci_add_capability2() below that. Then time to remove *errp. > > Since msi_init() won't fail now, touch up all the callers to avoid > checks against it. One side effect is that we fixed a possible leak in > current edu device. > > Reported-by: Markus Armbruster > Suggested-by: Paolo Bonzini > Signed-off-by: Peter Xu > --- > hw/audio/intel-hda.c | 18 +- > hw/i386/amd_iommu.c| 2 +- > hw/ide/ich.c | 6 +- > hw/misc/edu.c | 4 +--- > hw/net/e1000e.c| 6 +- > hw/net/trace-events| 1 - > hw/net/vmxnet3.c | 8 ++-- > hw/pci-bridge/ioh3420.c| 17 - > hw/pci-bridge/pci_bridge_dev.c | 19 +-- > hw/pci-bridge/xio3130_downstream.c | 11 +++ > hw/pci-bridge/xio3130_upstream.c | 11 +++ > hw/pci/msi.c | 25 ++--- > hw/scsi/megasas.c | 18 +- > hw/scsi/mptsas.c | 20 ++-- > hw/scsi/trace-events | 1 - > hw/scsi/vmw_pvscsi.c | 12 +++- > hw/usb/hcd-xhci.c | 16 +--- > hw/vfio/pci.c | 13 ++--- > include/hw/pci/msi.h | 6 +++--- > 19 files changed, 36 insertions(+), 178 deletions(-) Ping? Just to mention in case missed - this is also a bug fix for edu device. Also CC Markus since he's the reporter and I forgot to CC him in previous post. Sorry. Thanks, -- Peter Xu