Re: [Qemu-devel] [PATCH v1] highbank: validate register offset before access
Hello Philippe, +-- On Sun, 12 Nov 2017, Philippe Mathieu-Daudé wrote --+ | I'd rather use: | |"highbank: bad write offset 0x%" HWADDR_PRIx "\n", offset); Sent revised patch v2. Thank you. -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
Re: [Qemu-devel] [PATCH for 2.11 1/5] qom: move CPUClass.tcg_initialize to a global
On 11/10/2017 08:53 PM, Emilio G. Cota wrote:
> 55c3cee ("qom: Introduce CPUClass.tcg_initialize", 2017-10-24)
> introduces a per-CPUClass bool that we check so that the target CPU
> is initialized for TCG only once. This works well except when
> we end up creating more than one CPUClass, in which case we end
> up incorrectly initializing TCG more than once, i.e. once for
> each CPUClass.
... more than one CPUClass for a given translator.
Ideally, cortex-r5 and cortex-a53 would have a common parent class which would
hold all of the bits that are common between them, including the translator.
That said,
Reviewed-by: Richard Henderson
r~
Re: [Qemu-devel] kvm: virtio-net: saved image requires TUN_F_UFO support
On 11/11/2017 19:59, Stefan Priebe - Profihost AG wrote: > Hello, > > Am 10.11.2017 um 05:18 schrieb Jason Wang: >> >> >> On 2017年11月08日 19:22, Jason Wang wrote: >>> >>> >>> On 2017年11月08日 18:46, Paolo Bonzini wrote: On 08/11/2017 09:21, Jason Wang wrote: > > On 2017年11月08日 17:05, Stefan Priebe - Profihost AG wrote: >> Am 08.11.2017 um 08:54 schrieb Jason Wang: >>> On 2017年11月08日 15:41, Stefan Priebe - Profihost AG wrote: Hi Paolo, Am 06.11.2017 um 12:27 schrieb Paolo Bonzini: > On 06/11/2017 12:09, Stefan Priebe - Profihost AG wrote: >> HI Paolo, >> >> could this patchset be related? > Uh oh, yes it should. Jason, any ways to fix it? I suppose we > need to > disable UFO in the newest machine types, but do we also have to do > (software) UFO in vhost-net and QEMU for migration compatibility? Any news on this? Greets, >>> Since we probe UFO support, it will be disabled automatically on >>> device >>> startup. >>> >>> For the issue of migration, I think the only way is trying to fix >>> it in >>> kernel. >> But 4.14 is around the corner and the patchset is already included. >> Shouldn't this get fixed before 4.14 is released? > We will try to seek a solution as soon as possible. If we can't catch > 4.14, we will do it for stable for sure. Jason, can you write to netdev and Cc Linus and me? >>> >>> Paolo, see this https://www.spinics.net/lists/netdev/msg465454.html >>> >>> Just notice this today since I'm not on the cc list. >> >> An update: >> >> After some discussions on netdev, we will try to bring UFO back >> partially for just TAP. Willem promise to fix this. 4.14 is too late >> consider the fix is not trivial, it will go through stable tree. > > OK is it save to just revert the UFO patchset for my local branch? Yes, it is. Paolo
Re: [Qemu-devel] [PATCH v12 09/12] Move related hwpoison page function to accel/kvm/ folder
On 13/11/2017 02:45, gengdongjiu wrote:
> On 2017/11/10 19:32, Paolo Bonzini wrote:
>> On 10/11/2017 20:19, Dongjiu Geng wrote:
>>> +typedef struct HWPoisonPage {
>>> +ram_addr_t ram_addr;
>>> +QLIST_ENTRY(HWPoisonPage) list;
>>> +} HWPoisonPage;
>>> +
>>
>> Is this actually needed outside accel/kvm/kvm-all.c?
> Paolo,
>Thanks for the comments, this structure is added in the
> accel/kvm/kvm-all.c is also OK.
> My previous thought is that this is structure definition, so I move it to a
> header file.
> If you think this structure should be added in accel/kvm/kvm-all.c, I will
> move it.
It can be done later; but if you have to send a v13 series, I would be
grateful if you included this change as well.
Paolo
> thanks!
>
>>
>> Thanks,
>>
>> Paolo
>>
>> .
>>
>
Re: [Qemu-devel] [RESEND PATCH 2/6] memory: introduce AddressSpaceOps and IOMMUObject
On Mon, Nov 13, 2017 at 04:56:01PM +1100, David Gibson wrote: > On Fri, Nov 03, 2017 at 08:01:52PM +0800, Liu, Yi L wrote: > > From: Peter Xu > > > > AddressSpaceOps is similar to MemoryRegionOps, it's just for address > > spaces to store arch-specific hooks. > > > > The first hook I would like to introduce is iommu_get(). Return an > > IOMMUObject behind the AddressSpace. > > > > For systems that have IOMMUs, we will create a special address > > space per device which is different from system default address > > space for it (please refer to pci_device_iommu_address_space()). > > Normally when that happens, there will be one specific IOMMU (or > > say, translation unit) stands right behind that new address space. > > > > This iommu_get() fetches that guy behind the address space. Here, > > the guy is defined as IOMMUObject, which includes a notifier_list > > so far, may extend in future. Along with IOMMUObject, a new iommu > > notifier mechanism is introduced. It would be used for virt-svm. > > Also IOMMUObject can further have a IOMMUObjectOps which is similar > > to MemoryRegionOps. The difference is IOMMUObjectOps is not relied > > on MemoryRegion. > > > > Signed-off-by: Peter Xu > > Signed-off-by: Liu, Yi L > > Hi, sorry I didn't reply to the earlier postings of this after our > discussion in China. I've been sick several times and very busy. > > I still don't feel like there's an adequate explanation of exactly > what an IOMMUObject represents. Obviously it can represent more than > a single translation window - since that's represented by the > IOMMUMR. But what exactly do all the MRs - or whatever else - that > are represented by the IOMMUObject have in common, from a functional > point of view. > > Even understanding the SVM stuff better than I did, I don't really see > why an AddressSpace is an obvious unit to have an IOMMUObject > associated with it. Here's what I thought about it: IOMMUObject was planned to be the abstraction of the hardware translation unit, which is a higher level of the translated address spaces. Say, for each PCI device, it can have its own translated address space. However for multiple PCI devices, they can be sharing the same translation unit that handles the translation requests from different devices. That's the case for Intel platforms. We introduced this IOMMUObject because sometimes we want to do something with that translation unit rather than a specific device, in which we need a general IOMMU device handle. IIRC one issue left over during last time's discussion was that there could be more complicated IOMMU models. E.g., one device's DMA request can be translated nestedly by two or multiple IOMMUs, and current proposal cannot really handle that complicated hierachy. I'm just thinking whether we can start from a simple model (say, we don't allow nested IOMMUs, and actually we don't even allow multiple IOMMUs so far), then we can evolve from that point in the future. Also, I thought there were something you mentioned that this approach is not correct for Power systems, but I can't really remember the details... Anyways, I think this is not the only approach to solve the problem, and I believe any new better idea would be greatly welcomed as well. :) Thanks, > > > --- > > hw/core/Makefile.objs | 1 + > > hw/core/iommu.c | 58 +++ > > include/exec/memory.h | 22 +++ > > include/hw/core/iommu.h | 73 > > + > > memory.c| 10 +-- > > 5 files changed, 162 insertions(+), 2 deletions(-) > > create mode 100644 hw/core/iommu.c > > create mode 100644 include/hw/core/iommu.h > > > > diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs > > index f8d7a4a..d688412 100644 > > --- a/hw/core/Makefile.objs > > +++ b/hw/core/Makefile.objs > > @@ -5,6 +5,7 @@ common-obj-y += fw-path-provider.o > > # irq.o needed for qdev GPIO handling: > > common-obj-y += irq.o > > common-obj-y += hotplug.o > > +common-obj-y += iommu.o > > common-obj-y += nmi.o > > > > common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o > > diff --git a/hw/core/iommu.c b/hw/core/iommu.c > > new file mode 100644 > > index 000..7c4fcfe > > --- /dev/null > > +++ b/hw/core/iommu.c > > @@ -0,0 +1,58 @@ > > +/* > > + * QEMU emulation of IOMMU logic > > + * > > + * Copyright (C) 2017 Red Hat Inc. > > + * > > + * Authors: Peter Xu , > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License
Re: [Qemu-devel] [PATCH v12 09/12] Move related hwpoison page function to accel/kvm/ folder
On 2017/11/13 16:27, Paolo Bonzini wrote: >> If you think this structure should be added in accel/kvm/kvm-all.c, I will >> move it. > It can be done later; but if you have to send a v13 series, I would be > grateful if you included this change as well. Ok, got it, thanks Paolo. > > Paolo >
[Qemu-devel] [PATCH v2] virtio-pci: Don't force Subsystem Vendor ID = Vendor ID
The statement being removed doesn't change anything as virtio PCI devices
already
have Subsystem Vendor ID set to pci_default_sub_vendor_id (0x1af4), same as
Vendor
ID. And the Virtio spec does not require the two to be equal, either:
"The PCI Subsystem Vendor ID and the PCI Subsystem Device ID MAY reflect the
PCI
Vendor and Device ID of the environment (for informational purposes by the
driver)."
Background:
Following the recent virtio-win licensing change, several vendors are planning
to
ship their own certified version of Windows guest Virtio drivers, potentially
taking
advantage of Windows Update as a distribution channel. It is therefore critical
that
each vendor uses their own PCI Subsystem Vendor ID for Virtio devices to prevent
drivers from other vendors binding to it.
This would be trivially done by adding:
k->subsystem_vendor_id = ...
to virtio_pci_class_init(). Except for the problematic statement deleted by this
patch, which reverts the Subsystem Vendor ID back to 0x1af4 for legacy devices
for
no good reason.
Signed-off-by: Ladi Prosek
---
v1->v2:
* Added a comment about the default Subsystem Vendor ID.
hw/virtio/virtio-pci.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index e92837c42b..148621d9c7 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1588,9 +1588,11 @@ static void virtio_pci_device_plugged(DeviceState *d,
Error **errp)
"neither legacy nor transitional device.");
return ;
}
-/* legacy and transitional */
-pci_set_word(config + PCI_SUBSYSTEM_VENDOR_ID,
- pci_get_word(config + PCI_VENDOR_ID));
+/*
+ * Legacy and transitional devices use specific subsystem IDs.
+ * Note that the subsystem vendor ID (config + PCI_SUBSYSTEM_VENDOR_ID)
+ * is set to PCI_SUBVENDOR_ID_REDHAT_QUMRANET by default.
+ */
pci_set_word(config + PCI_SUBSYSTEM_ID, virtio_bus_get_vdev_id(bus));
} else {
/* pure virtio-1.0 */
--
2.13.5
Re: [Qemu-devel] [RFC PATCH 13/26] cpus: only take BQL for sleeping threads
> From: Paolo Bonzini [mailto:pbonz...@redhat.com] > > From: "David Hildenbrand" > > On 02.11.2017 12:08, Paolo Bonzini wrote: > > > On 31/10/2017 12:26, Pavel Dovgalyuk wrote: > > >> From: Alex Bennée > > >> > > >> Now the only real need to hold the BQL is for when we sleep on the > > >> cpu->halt conditional. The lock is actually dropped while the thread > > >> sleeps so the actual window for contention is pretty small. This also > > >> means we can remove the special case hack for exclusive work and > > >> simply declare that work no longer has an implicit BQL held. This > > >> isn't a major problem async work is generally only changing things in > > >> the context of its own vCPU. If it needs to work across vCPUs it > > >> should be using the exclusive mechanism or possibly taking the lock > > >> itself. > > >> > > >> Signed-off-by: Alex Bennée > > >> Tested-by: Pavel Dovgalyuk > > > > > > At least cpu_throttle_thread would fail with this patch. > > > > > > Also I am not sure if the s390 SIGP handlers are ready for this. > > > > > > > We have a global lock to the SIGP "facility". However we need the BQL in > > order to inject interrupts into CPUs (otherwise it would trigger an > > assert when injecting). > > > > We inject Restart and Stop interrupts from run_on_cpu. This requires the > > BQL. So Paolo should be right, this change would break s390x. > > I had some patches to access interrupt_request with the atomic builtins. If > Pavel can first extract the other changes to the icount mechanism, I can > update them. What changes do you mean here? I'm not sure that I understand clearly how threads interact with BQL. These patches were authored by Alex and we'll have to get him into the discussion. Pavel Dovgalyuk
Re: [Qemu-devel] [PATCH 2/7] s390x/pci: rework PCI STORE
On 09/11/2017 17:50, Cornelia Huck wrote:
On Tue, 7 Nov 2017 18:24:34 +0100
Pierre Morel wrote:
Enhance the fault detection, correction of the fault reporting.
Signed-off-by: Pierre Morel
Reviewed-by: Yi Min Zhao
---
hw/s390x/s390-pci-inst.c | 41 -
1 file changed, 24 insertions(+), 17 deletions(-)
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 8fcb02d..4a2f996 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -469,6 +469,12 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t
r2)
pcias = (env->regs[r2] >> 16) & 0xf;
len = env->regs[r2] & 0xf;
offset = env->regs[r2 + 1];
+data = env->regs[r1];
+
+if (!(fh & FH_MASK_ENABLE)) {
This covers the reserved/standby/disabled states, right?
yes it does, all these states have the FH_MASK_ENABLE set to 0
+setcc(cpu, ZPCI_PCI_LS_INVAL_HANDLE);
+return 0;
+}
pbdev = s390_pci_find_dev_by_fh(s390_get_phb(), fh);
if (!pbdev) {
@@ -478,12 +484,7 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t
r2)
}
switch (pbdev->state) {
-case ZPCI_FS_RESERVED:
-case ZPCI_FS_STANDBY:
-case ZPCI_FS_DISABLED:
case ZPCI_FS_PERMANENT_ERROR:
-setcc(cpu, ZPCI_PCI_LS_INVAL_HANDLE);
-return 0;
case ZPCI_FS_ERROR:
setcc(cpu, ZPCI_PCI_LS_ERR);
s390_set_status_code(env, r2, ZPCI_PCI_ST_BLOCKED);
@@ -492,9 +493,13 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t
r2)
break;
}
-data = env->regs[r1];
-if (pcias < 6) {
-if ((8 - (offset & 0x7)) < len) {
+/* Test that the pcias is a valid pcias and do the according operations */
s/according/appropriate/ ?
yes, better, thanks
+switch (pcias) {
+case 0 ... 5:
+/* Check length:
+ * A length of 0 is invalid and length should not cross a double word
+ */
+if (!len || (len > (8 - (offset & 0x7 {
program_interrupt(env, PGM_OPERAND, 4);
return 0;
}
@@ -512,21 +517,23 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t
r2)
program_interrupt(env, PGM_OPERAND, 4);
return 0;
}
-} else if (pcias == 15) {
-if ((4 - (offset & 0x3)) < len) {
-program_interrupt(env, PGM_OPERAND, 4);
-return 0;
-}
-
-if (zpci_endian_swap(&data, len)) {
+break;
+case 15:
+/* ZPCI uses the pseudo BAR number 15 as configuration space */
+/* possible access length are 1,2,4 and must not cross a word */
s/length/lengths/
yes, thanks
+if (!len || (len > (4 - (offset & 0x3))) || len == 3) {
program_interrupt(env, PGM_OPERAND, 4);
return 0;
}
-
+/* len = 1,2,4 so we do not need to test */
+zpci_endian_swap(&data, len);
pci_host_config_write_common(pbdev->pdev, offset,
pci_config_size(pbdev->pdev),
data, len);
-} else {
+break;
+case PCI_ROM_SLOT:
So, will this be filled in a later patch? (Reading from the top.)
No it will not it is only a reminder that we explicitly do not support it.
Since I do not know if we ever will support it do you think I better let
it fall?
+/* Fall through */
+default:
DPRINTF("pcistg invalid space\n");
setcc(cpu, ZPCI_PCI_LS_ERR);
s390_set_status_code(env, r2, ZPCI_PCI_ST_INVAL_AS);
On the whole, I think this improves readability, especially for folks
that do not have access to the spec.
--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany
Re: [Qemu-devel] [PATCH 3/7] s390x/pci: rework PCI LOAD
On 09/11/2017 17:51, Cornelia Huck wrote: On Tue, 7 Nov 2017 18:24:35 +0100 Pierre Morel wrote: Enhance the fault detection, correction of the fault reporting. Basically the same comments as for the previous patch (but looks good in general). thanks I will process it the same way as patch 2 then. Signed-off-by: Pierre Morel Reviewed-by: Yi Min Zhao --- hw/s390x/s390-pci-inst.c | 27 --- 1 file changed, 16 insertions(+), 11 deletions(-) -- Pierre Morel Linux/KVM/QEMU in Böblingen - Germany
Re: [Qemu-devel] [PATCH 6/7] s390x/pci: move the memory region write from pcistg
On 10/11/2017 10:51, Cornelia Huck wrote: On Fri, 10 Nov 2017 17:40:12 +0800 Yi Min Zhao wrote: 在 2017/11/10 上午3:23, Cornelia Huck 写道: On Tue, 7 Nov 2017 18:24:38 +0100 Pierre Morel wrote: Let's move the memory region write from pcistg into a dedicated function. This allows us to prepare a later patch searching for subregions inside of the memory region. OK, so here is the memory region write. Do we have any sleeping endianness bugs in there for when we wire up tcg? I'm not sure how this plays with the bswaps (see patch 1). But maybe I've just gotten lost somewhere. I think there's no error. For PCI bars' MRs, we got the little-endian data that is exactly fit to the byte ordering of pcilg instruction. For PCI config space, the data has been swapped according to the cpu byte ordering. Host or target cpu? So we use zpci_swap_endian() to swap the data back to the little-endian ordering. I do not see where we use the zpci_swap_endian() function in this patch or in the zpci_write_bar() function. That swap is unconditional. If we were running on a little-endian host, it would be wrong, wouldn't it? I think there is no problem here, we do not use this function but we use the memory_region_dispatch_write() to access a subregion of the PCI device which is defined as DEVICE_LITTLE_ENDIAN AFAIU The memory access process the endianness correctly. Signed-off-by: Pierre Morel Reviewed-by: Yi Min Zhao --- hw/s390x/s390-pci-inst.c | 27 +-- 1 file changed, 17 insertions(+), 10 deletions(-) -- Pierre Morel Linux/KVM/QEMU in Böblingen - Germany
[Qemu-devel] [PATCH] scripts/make-release: No need to delete pixman/.git anymore
The pixman submodule has been removed in commit c12b6d70e384c769ca372e1,
so there is no need anymore to delete pixman/.git while building a
release tarball.
Signed-off-by: Thomas Huth
---
scripts/make-release | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/make-release b/scripts/make-release
index fa6323f..70367ab 100755
--- a/scripts/make-release
+++ b/scripts/make-release
@@ -19,7 +19,7 @@ pushd ${destination}
git checkout "v${version}"
git submodule update --init
(cd roms/seabios && git describe --tags --long --dirty > .version)
-rm -rf .git roms/*/.git dtc/.git pixman/.git
+rm -rf .git roms/*/.git dtc/.git
popd
tar cfj ${destination}.tar.bz2 ${destination}
rm -rf ${destination}
--
1.8.3.1
Re: [Qemu-devel] [PATCH v2] virtio-pci: Don't force Subsystem Vendor ID = Vendor ID
On Mon, Nov 13, 2017 at 09:45:58AM +0100, Ladi Prosek wrote: > The statement being removed doesn't change anything as virtio PCI devices > already > have Subsystem Vendor ID set to pci_default_sub_vendor_id (0x1af4), same as > Vendor > ID. And the Virtio spec does not require the two to be equal, either: > > "The PCI Subsystem Vendor ID and the PCI Subsystem Device ID MAY reflect > the PCI > Vendor and Device ID of the environment (for informational purposes by the > driver)." > > Background: > > Following the recent virtio-win licensing change, several vendors are > planning to > ship their own certified version of Windows guest Virtio drivers, potentially > taking > advantage of Windows Update as a distribution channel. It is therefore > critical that > each vendor uses their own PCI Subsystem Vendor ID for Virtio devices to > prevent > drivers from other vendors binding to it. > > This would be trivially done by adding: > > k->subsystem_vendor_id = ... > > to virtio_pci_class_init(). Except for the problematic statement deleted by > this > patch, which reverts the Subsystem Vendor ID back to 0x1af4 for legacy > devices for > no good reason. Reviewed-by: Gerd Hoffmann cheers, Gerd
Re: [Qemu-devel] [PATCH 1/7] s390x/pci: factor out endianess conversion
On 09/11/2017 19:55, Philippe Mathieu-Daudé wrote:
On 11/09/2017 01:38 PM, Cornelia Huck wrote:
On Tue, 7 Nov 2017 18:24:33 +0100
Pierre Morel wrote:
There are two places where the same endianness conversion
is done.
Let's factor this out into a static function.
Signed-off-by: Pierre Morel
Reviewed-by: Yi Min Zhao
---
hw/s390x/s390-pci-inst.c | 58 ++--
1 file changed, 32 insertions(+), 26 deletions(-)
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 8e088f3..8fcb02d 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -314,6 +314,35 @@ out:
return 0;
}
+/**
+ * This function swaps the data at ptr according from one
+ * endianness to the other.
+ * valid data in the uint64_t data field.
I'm not sure what that line is supposed to mean?
+ * @ptr: a pointer to a uint64_t data field
+ * @len: the length of the valid data, must be 1,2,4 or 8
+ */
+static int zpci_endian_swap(uint64_t *ptr, uint8_t len)
+{
+uint64_t data = *ptr;
+switch (len) {
+case 1:
+break;
+case 2:
+data = bswap16(data);
+break;
+case 4:
+data = bswap32(data);
+break;
+case 8:
+data = bswap64(data);
+break;
+default:
+return -EINVAL;
+}
+*ptr = data;
+return 0;
+}
This is usually care taken by memory::adjust_endianness() ...
We are here intercepting an instruction with the data in a register.
That is what troubles me, but I will take a deeper look.
I was expecting more code to use a similar pattern, but it seems
surprisingly uncommon.
Which ring a bell for latent bug?
This remind me of a similar issue on ppc:
http://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg05121.html
...
http://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg05666.html
Thanks for the pointers.
--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany
Re: [Qemu-devel] [PATCH 1/7] s390x/pci: factor out endianess conversion
On 09/11/2017 19:55, Philippe Mathieu-Daudé wrote:
On 11/09/2017 01:38 PM, Cornelia Huck wrote:
On Tue, 7 Nov 2017 18:24:33 +0100
Pierre Morel wrote:
There are two places where the same endianness conversion
is done.
Let's factor this out into a static function.
Signed-off-by: Pierre Morel
Reviewed-by: Yi Min Zhao
---
hw/s390x/s390-pci-inst.c | 58 ++--
1 file changed, 32 insertions(+), 26 deletions(-)
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 8e088f3..8fcb02d 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -314,6 +314,35 @@ out:
return 0;
}
+/**
+ * This function swaps the data at ptr according from one
+ * endianness to the other.
+ * valid data in the uint64_t data field.
I'm not sure what that line is supposed to mean?
+ * @ptr: a pointer to a uint64_t data field
+ * @len: the length of the valid data, must be 1,2,4 or 8
+ */
+static int zpci_endian_swap(uint64_t *ptr, uint8_t len)
+{
+uint64_t data = *ptr;
+switch (len) {
+case 1:
+break;
+case 2:
+data = bswap16(data);
+break;
+case 4:
+data = bswap32(data);
+break;
+case 8:
+data = bswap64(data);
+break;
+default:
+return -EINVAL;
+}
+*ptr = data;
+return 0;
+}
This is usually care taken by memory::adjust_endianness() ...
We are here intercepting an instruction with the data in a register.
That is what troubles me, but I will take a deeper look.
I was expecting more code to use a similar pattern, but it seems
surprisingly uncommon.
Which ring a bell for latent bug?
This remind me of a similar issue on ppc:
http://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg05121.html
...
http://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg05666.html
Thanks for the pointers.
--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany
Re: [Qemu-devel] [PATCH 6/7] s390x/pci: move the memory region write from pcistg
On 10/11/2017 10:51, Cornelia Huck wrote: On Fri, 10 Nov 2017 17:40:12 +0800 Yi Min Zhao wrote: 在 2017/11/10 上午3:23, Cornelia Huck 写道: On Tue, 7 Nov 2017 18:24:38 +0100 Pierre Morel wrote: Let's move the memory region write from pcistg into a dedicated function. This allows us to prepare a later patch searching for subregions inside of the memory region. OK, so here is the memory region write. Do we have any sleeping endianness bugs in there for when we wire up tcg? I'm not sure how this plays with the bswaps (see patch 1). But maybe I've just gotten lost somewhere. I think there's no error. For PCI bars' MRs, we got the little-endian data that is exactly fit to the byte ordering of pcilg instruction. For PCI config space, the data has been swapped according to the cpu byte ordering. Host or target cpu? So we use zpci_swap_endian() to swap the data back to the little-endian ordering. I do not see where we use the zpci_swap_endian() function in this patch or in the zpci_write_bar() function. That swap is unconditional. If we were running on a little-endian host, it would be wrong, wouldn't it? I think there is no problem here, we do not use the swap function but we use the memory_region_dispatch_write() to access a subregion of the PCI device which is defined as DEVICE_LITTLE_ENDIAN AFAIU The memory access process the endianness correctly. Signed-off-by: Pierre Morel Reviewed-by: Yi Min Zhao --- hw/s390x/s390-pci-inst.c | 27 +-- 1 file changed, 17 insertions(+), 10 deletions(-) -- Pierre Morel Linux/KVM/QEMU in Böblingen - Germany
Re: [Qemu-devel] [PATCH 3/7] s390x/pci: rework PCI LOAD
On 09/11/2017 17:51, Cornelia Huck wrote: On Tue, 7 Nov 2017 18:24:35 +0100 Pierre Morel wrote: Enhance the fault detection, correction of the fault reporting. Basically the same comments as for the previous patch (but looks good in general). thanks I will process it the same way as patch 2/7 then. Signed-off-by: Pierre Morel Reviewed-by: Yi Min Zhao --- hw/s390x/s390-pci-inst.c | 27 --- 1 file changed, 16 insertions(+), 11 deletions(-) -- Pierre Morel Linux/KVM/QEMU in Böblingen - Germany
[Qemu-devel] [Bug 530077] Re: kvm: 16-bit code execution failure should be more friendly
Triaging old bug tickets... has this ever been fixed, thus could we close this ticket nowadays? Or is there something left to do here? ** Changed in: qemu Status: Confirmed => Incomplete ** Changed in: qemu Assignee: Anthony Liguori (anthony-codemonkey) => (unassigned) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/530077 Title: kvm: 16-bit code execution failure should be more friendly Status in QEMU: Incomplete Bug description: Today, when kvm fails at 16-bit code execution, we report: spirit:~/qemu> qemu-kvm ./hda-fedora.img kvm: unhandled exit 8021 kvm_run returned -22 There are three reasons exit reason 21 happens. The first is that a user is executing an image containing a workload that uses GFXBOOT or some other bootloader that exercises big real mode. On pre-Westmere Intel processors, VT could not handle big real mode. The second reason is that the guest's image is corrupted and we're executing random code. We accidentally fall into one of the unsupported modes for VT. Again, this is addressed on WSM. The third case is where there's an actual bug in KVM. This should be exceedingly rare at this stage. We should present a friendly error message explaining the possible causes and recommending corrective action. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/530077/+subscriptions
Re: [Qemu-devel] [PATCH] scripts/make-release: No need to delete pixman/.git anymore
On 13.11.2017 10:43, no-re...@patchew.org wrote: > Hi, > > This series failed automatic build test. Please find the testing commands and > their output below. If you have docker installed, you can probably reproduce > it > locally. > > Subject: [Qemu-devel] [PATCH] scripts/make-release: No need to delete > pixman/.git anymore > Type: series > Message-id: 1510564905-6185-1-git-send-email-th...@redhat.com [...] > QEMU -- "/tmp/qemu-test/build/x86_64-softmmu/qemu-system-x86_64" > -nodefaults -machine accel=qtest > QEMU_IMG -- "/tmp/qemu-test/build/qemu-img" > QEMU_IO -- "/tmp/qemu-test/build/qemu-io" --cache writeback -f raw > QEMU_NBD -- "/tmp/qemu-test/build/qemu-nbd" > IMGFMT-- raw > IMGPROTO -- file > PLATFORM -- Linux/x86_64 fe9889ba94a0 4.11.10-300.fc26.x86_64 > TEST_DIR -- /tmp/qemu-test > SOCKET_SCM_HELPER -- /tmp/qemu-test/build/tests/qemu-iotests/socket_scm_helper > > 001 > 002 > 004 > 005 > 008 > 009 > 010 > 011 > 012 > 017 [not run] not suitable for this image format: raw > 018 [not run] not suitable for this image format: raw > 019 [not run] not suitable for this image format: raw > 020 [not run] not suitable for this image format: raw > 021 > 024 [not run] not suitable for this image format: raw > 025 > 027 [not run] not suitable for this image format: raw > 028 [not run] not suitable for this image format: raw > 029 [not run] not suitable for this image format: raw > 031 [not run] not suitable for this image format: raw > 032 > 033 > 034 [not run] not suitable for this image format: raw > 035 [not run] not suitable for this image format: raw > 036 [not run] not suitable for this image format: raw > 037 [not run] not suitable for this image format: raw > 038 [not run] not suitable for this image format: raw > 039 [not run] not suitable for this image format: raw > 042 [not run] not suitable for this image format: raw > 045 > 046 [not run] not suitable for this image format: raw > 047 [not run] not suitable for this image format: raw > 048 > 050 [not run] not suitable for this image format: raw > 052 > 053 [not run] not suitable for this image format: raw > 054 [not run] not suitable for this image format: raw > 058 [not run] not suitable for this image format: raw > 059 [not run] not suitable for this image format: raw > 060 [not run] not suitable for this image format: raw > 062 [not run] not suitable for this image format: raw > 063 > 064 [not run] not suitable for this image format: raw > 065 [not run] not suitable for this image format: raw > 066 [not run] not suitable for this image format: raw > 067 [not run] not suitable for this image format: raw > 068 [not run] not suitable for this image format: raw > 069 [not run] not suitable for this image format: raw > 070 [not run] not suitable for this image format: raw > 071 [not run] not suitable for this image format: raw > 072 [not run] not suitable for this image format: raw > 073 [not run] not suitable for this image format: raw > 074 [not run] not suitable for this image format: raw > 075 [not run] not suitable for this image format: raw > 077 - output mismatch (see 077.out.bad) > --- /tmp/qemu-test/src/tests/qemu-iotests/077.out 2017-11-13 > 09:38:59.0 + > +++ /tmp/qemu-test/build/tests/qemu-iotests/077.out.bad 2017-11-13 > 09:41:20.601278693 + > @@ -56,9 +56,9 @@ > wrote XXX/XXX bytes at offset XXX > XXX bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) > blkdebug: Resuming request 'B' > +blkdebug: Resuming request 'A' > wrote XXX/XXX bytes at offset XXX > XXX bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) > -blkdebug: Resuming request 'A' > wrote XXX/XXX bytes at offset XXX > XXX bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) > wrote XXX/XXX bytes at offset XXX > 078 [not run] not suitable for this image format: raw > 081 > 082 [not run] not suitable for this image format: raw > 084 [not run] not suitable for this image format: raw > 086 > 087 [not run] not suitable for this image format: raw > 088 [not run] not suitable for this image format: raw > 089 [not run] not suitable for this image format: raw > 090 [not run] not suitable for this image format: raw > 092 [not run] not suitable for this image format: raw > 094 [not run] not suitable for this image protocol: file > 095 [not run] not suitable for this image format: raw > 096 [not run] not suitable for this image format: raw > 098
Re: [Qemu-devel] [PATCH for-2.12 v3 01/11] spapr: add pseries 2.12 machine type
On Mon, 13 Nov 2017 16:51:03 +1100
David Gibson wrote:
> On Fri, Nov 10, 2017 at 03:20:07PM +, Cédric Le Goater wrote:
> > Signed-off-by: Cédric Le Goater
> > ---
> > hw/ppc/spapr.c | 16 +++-
> > 1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index d682f013d422..a2dcbee07214 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -3687,6 +3687,20 @@ static const TypeInfo spapr_machine_info = {
> > type_init(spapr_machine_register_##suffix)
> >
> > /*
> > + * pseries-2.12
> > + */
> > +static void spapr_machine_2_12_instance_options(MachineState *machine)
> > +{
> > +}
> > +
> > +static void spapr_machine_2_12_class_options(MachineClass *mc)
> > +{
> > +/* Defaults for the latest behaviour inherited from the base class */
> > +}
> > +
> > +DEFINE_SPAPR_MACHINE(2_12, "2.12", true);
> > +
> > +/*
> > * pseries-2.11
> > */
> > static void spapr_machine_2_11_instance_options(MachineState *machine)
> > @@ -3698,7 +3712,7 @@ static void
> > spapr_machine_2_11_class_options(MachineClass *mc)
> > /* Defaults for the latest behaviour inherited from the base class */
> > }
> >
> > -DEFINE_SPAPR_MACHINE(2_11, "2.11", true);
> > +DEFINE_SPAPR_MACHINE(2_11, "2.11", false);
> >
> > /*
> > * pseries-2.10
>
> Uh.. not quite right. You also need to chain the 2.11 hooks onto the
Well, this happens in patch 5, but you're right it probably makes more
sense to consolidate in a single patch.
Also, this patch should have dropped the "Defaults for the latest..."
comment...
> new 2.12 ones. Never mind, we'll need it sooner or later, so I've put
> my own version into ppc-for-2.12.
>
... and so should your version I guess.
pgpukB2NDRr0Z.pgp
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] net/socket: fix coverity issue
On 13 November 2017 at 07:13, Jason Wang wrote:
>
>
> On 2017年11月06日 21:28, Jens Freimann wrote:
>>
>> This fixes coverity issue CID1005339.
>>
>> Make sure that saddr is not used uninitialized if the
>> mcast parameter is NULL.
>>
>> Cc: qemu-sta...@nongnu.org
>> Reported-by: Peter Maydell
>> Signed-off-by: Jens Freimann
>> ---
>> net/socket.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/socket.c b/net/socket.c
>> index e6b471c63d..51eaea67a0 100644
>> --- a/net/socket.c
>> +++ b/net/socket.c
>> @@ -332,7 +332,7 @@ static NetSocketState
>> *net_socket_fd_init_dgram(NetClientState *peer,
>> const char *mcast,
>> Error **errp)
>> {
>> -struct sockaddr_in saddr;
>> +struct sockaddr_in saddr = { 0 };
>> int newfd;
>> NetClientState *nc;
>> NetSocketState *s;
>> @@ -373,7 +373,7 @@ static NetSocketState
>> *net_socket_fd_init_dgram(NetClientState *peer,
>> net_socket_read_poll(s, true);
>> /* mcast: save bound address as dst */
>> -if (is_connected) {
>> +if (is_connected && mcast != NULL) {
>> s->dgram_dst = saddr;
>> snprintf(nc->info_str, sizeof(nc->info_str),
>>"socket: fd=%d (cloned mcast=%s:%d)",
>
>
> Applied, thanks.
Er, this version didn't pass code review and you should
apply v2 instead...
thanks
-- PMM
Re: [Qemu-devel] QEMU 3.0 ? (was: [PATCH for-2.12 v3 01/11] spapr: add pseries 2.12 machine type)
On 13 November 2017 at 07:14, Thomas Huth wrote: > By the way, before everybody now introduces "2.12" machine types ... is > there already a consensus that the next version will be "2.12" ? > > A couple of months ago, we discussed that we could maybe do a 3.0 after > 2.11, e.g. here: > > https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg05056.html > > I'd still like to see that happen... Peter, any thoughts on this? I don't see the point in declaring a 3.0 unless we have some sweeping change that merits it. I don't think we should do a sweeping change unless we have a well laid out and agreed on plan for how the transition works. So I would want to see the plan discussed and agreed first, and then we can say "ok, and we think we can do this in this timescale and so the version at $DATE will be 3.0". Changing the version number should be the last part of this process, not the first, in my view. thanks -- PMM
Re: [Qemu-devel] [PATCH v4] throttle-groups: drain before detaching ThrottleState
On Fri 10 Nov 2017 04:19:34 PM CET, Stefan Hajnoczi wrote:
> I/O requests hang after stop/cont commands at least since QEMU 2.10.0
> with -drive iops=100:
>
> (guest)$ dd if=/dev/zero of=/dev/vdb oflag=direct count=1000
> (qemu) stop
> (qemu) cont
> ...I/O is stuck...
>
> This happens because blk_set_aio_context() detaches the ThrottleState
> while requests may still be in flight:
>
> if (tgm->throttle_state) {
> throttle_group_detach_aio_context(tgm);
> throttle_group_attach_aio_context(tgm, new_context);
> }
>
> This patch encloses the detach/attach calls in a drained region so no
> I/O request is left hanging. Also add assertions so we don't make the
> same mistake again in the future.
>
> Reported-by: Yongxue Hong
> Signed-off-by: Stefan Hajnoczi
Reviewed-by: Alberto Garcia
Berto
Re: [Qemu-devel] [PATCH] net/socket: fix coverity issue
On 2017年11月13日 17:51, Peter Maydell wrote:
On 13 November 2017 at 07:13, Jason Wang wrote:
On 2017年11月06日 21:28, Jens Freimann wrote:
This fixes coverity issue CID1005339.
Make sure that saddr is not used uninitialized if the
mcast parameter is NULL.
Cc: qemu-sta...@nongnu.org
Reported-by: Peter Maydell
Signed-off-by: Jens Freimann
---
net/socket.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/socket.c b/net/socket.c
index e6b471c63d..51eaea67a0 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -332,7 +332,7 @@ static NetSocketState
*net_socket_fd_init_dgram(NetClientState *peer,
const char *mcast,
Error **errp)
{
-struct sockaddr_in saddr;
+struct sockaddr_in saddr = { 0 };
int newfd;
NetClientState *nc;
NetSocketState *s;
@@ -373,7 +373,7 @@ static NetSocketState
*net_socket_fd_init_dgram(NetClientState *peer,
net_socket_read_poll(s, true);
/* mcast: save bound address as dst */
-if (is_connected) {
+if (is_connected && mcast != NULL) {
s->dgram_dst = saddr;
snprintf(nc->info_str, sizeof(nc->info_str),
"socket: fd=%d (cloned mcast=%s:%d)",
Applied, thanks.
Er, this version didn't pass code review and you should
apply v2 instead...
thanks
-- PMM
Oops, indeed.
Apply V2.
Thanks
[Qemu-devel] [Bug 1731588] Re: qemu-system-arm black screen and keyboard not detected
"stm32-p103" is not a board model supported by upstream QEMU. Presumably you're using a fork of QEMU -- you should ask whoever is responsible for that fork about it. For the second command line -- is the binary you're trying to run built for the stellaris board model you're trying to run it on? If it's the same binary you tried to use with the stm32 board model then I wouldn't expect it to work. You need to build a binary that targets the board model you run with. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1731588 Title: qemu-system-arm black screen and keyboard not detected Status in QEMU: New Bug description: Hi guys, I try to emulate FreeRTOS with this guide : http://wiki.csie.ncku.edu.tw/embedded/Lab32 But, the keys on my keyboard are not taken into account. - Command line : qemu_stm32/arm-softmmu/qemu-system-arm -M stm32-p103 -monitor stdio -kernel build/main.bin -semihosting If I try to add usb flag with : qemu_stm32/arm-softmmu/qemu-system-arm -M stm32-p103 -monitor stdio -kernel build/main.bin -usb -device usb-host,hostbus=1,hostaddr=1 -show-curso I obtain this message "qemu-system-arm: -device usb-host,hostbus=1,hostaddr=1: 'usb-host' is not a valid device model name" My second option is try with the latest version of qemu with this command line : "qemu-system-arm -M lm3s6965evb -monitor stdio -kernel build/main.bin -semihosting" but I obtain a black screen. Could someone guide me on this problem ? To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1731588/+subscriptions
Re: [Qemu-devel] QEMU 3.0 ?
On 11/13/2017 10:53 AM, Peter Maydell wrote: > On 13 November 2017 at 07:14, Thomas Huth wrote: >> By the way, before everybody now introduces "2.12" machine types ... is >> there already a consensus that the next version will be "2.12" ? >> >> A couple of months ago, we discussed that we could maybe do a 3.0 after >> 2.11, e.g. here: >> >> https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg05056.html >> >> I'd still like to see that happen... Peter, any thoughts on this? > > I don't see the point in declaring a 3.0 unless we have some > sweeping change that merits it. I don't think we should do a > sweeping change unless we have a well laid out and agreed on > plan for how the transition works. So I would want to see the > plan discussed and agreed first, and then we can say "ok, and > we think we can do this in this timescale and so the version > at $DATE will be 3.0". Changing the version number should be > the last part of this process, not the first, in my view. One of the sweeping change for 3.0 could be to stop to maintaining migration compatibility with older versions (2.x). Even if the feature is really a must have in some cluster environment, the code (and the developer) is starting to suffer from it. Thanks, C.
Re: [Qemu-devel] [PULL for-2.11-rc1 v2 0/2] Block patches
On 8 November 2017 at 19:20, Stefan Hajnoczi wrote: > The following changes since commit b0fbe46ad82982b289a44ee2495b59b0bad8a842: > > Update version for v2.11.0-rc0 release (2017-11-07 16:05:28 +) > > are available in the git repository at: > > git://github.com/stefanha/qemu.git tags/block-pull-request > > for you to fetch changes up to ef6dada8b44e1e7c4bec5c1115903af9af415b50: > > util/async: use atomic_mb_set in qemu_bh_cancel (2017-11-08 19:09:15 +) > > > Pull request > > v2: > * v1 emails 2/3 and 3/3 weren't sent due to an email failure > * Included Sergio's updated wording in the commit description > > Applied, thanks. -- PMM
Re: [Qemu-devel] [RFC PATCH 13/26] cpus: only take BQL for sleeping threads
Pavel Dovgalyuk writes: >> From: Paolo Bonzini [mailto:pbonz...@redhat.com] >> > From: "David Hildenbrand" >> > On 02.11.2017 12:08, Paolo Bonzini wrote: >> > > On 31/10/2017 12:26, Pavel Dovgalyuk wrote: >> > >> From: Alex Bennée >> > >> >> > >> Now the only real need to hold the BQL is for when we sleep on the >> > >> cpu->halt conditional. The lock is actually dropped while the thread >> > >> sleeps so the actual window for contention is pretty small. This also >> > >> means we can remove the special case hack for exclusive work and >> > >> simply declare that work no longer has an implicit BQL held. This >> > >> isn't a major problem async work is generally only changing things in >> > >> the context of its own vCPU. If it needs to work across vCPUs it >> > >> should be using the exclusive mechanism or possibly taking the lock >> > >> itself. >> > >> >> > >> Signed-off-by: Alex Bennée >> > >> Tested-by: Pavel Dovgalyuk >> > > >> > > At least cpu_throttle_thread would fail with this patch. >> > > >> > > Also I am not sure if the s390 SIGP handlers are ready for this. >> > > >> > >> > We have a global lock to the SIGP "facility". However we need the BQL in >> > order to inject interrupts into CPUs (otherwise it would trigger an >> > assert when injecting). >> > >> > We inject Restart and Stop interrupts from run_on_cpu. This requires the >> > BQL. So Paolo should be right, this change would break s390x. >> >> I had some patches to access interrupt_request with the atomic builtins. If >> Pavel can first extract the other changes to the icount mechanism, I can >> update them. > > What changes do you mean here? > I'm not sure that I understand clearly how threads interact with BQL. > These patches were authored by Alex and we'll have to get him into the > discussion. Do you want me to re-spin my sub-set of the patches as a new base? -- Alex Bennée
Re: [Qemu-devel] [RESEND PATCH 2/6] memory: introduce AddressSpaceOps and IOMMUObject
On Mon, Nov 13, 2017 at 04:56:01PM +1100, David Gibson wrote:
> On Fri, Nov 03, 2017 at 08:01:52PM +0800, Liu, Yi L wrote:
> > From: Peter Xu
> >
> > AddressSpaceOps is similar to MemoryRegionOps, it's just for address
> > spaces to store arch-specific hooks.
> >
> > The first hook I would like to introduce is iommu_get(). Return an
> > IOMMUObject behind the AddressSpace.
> >
> > For systems that have IOMMUs, we will create a special address
> > space per device which is different from system default address
> > space for it (please refer to pci_device_iommu_address_space()).
> > Normally when that happens, there will be one specific IOMMU (or
> > say, translation unit) stands right behind that new address space.
> >
> > This iommu_get() fetches that guy behind the address space. Here,
> > the guy is defined as IOMMUObject, which includes a notifier_list
> > so far, may extend in future. Along with IOMMUObject, a new iommu
> > notifier mechanism is introduced. It would be used for virt-svm.
> > Also IOMMUObject can further have a IOMMUObjectOps which is similar
> > to MemoryRegionOps. The difference is IOMMUObjectOps is not relied
> > on MemoryRegion.
> >
> > Signed-off-by: Peter Xu
> > Signed-off-by: Liu, Yi L
>
> Hi, sorry I didn't reply to the earlier postings of this after our
> discussion in China. I've been sick several times and very busy.
Hi David,
Fully understood. I'll try my best to address your question. Also,
feel free to input further questions, anyhow, the more we discuss the
better work we done.
> I still don't feel like there's an adequate explanation of exactly
> what an IOMMUObject represents. Obviously it can represent more than
IOMMUObject is aimed to represent the iommu itself. e.g. the iommu
specific operations. One of the key purpose of IOMMUObject is to
introduce a notifier framework to let iommu emulator to be able to
do iommu operations other than MAP/UNMAP. As IOMMU grows more and
more feature, MAP/UNMAP is not the only operation iommu emulator needs
to deal. e.g. shared virtual memory. So far, as I know AMD/ARM also
has it. may correct me on it. As my cover letter mentioned, MR based
notifier framework doesn’t work for the newly added IOMMU operations.
Like bind guest pasid table pointer to host and propagate guest's
iotlb flush to host.
> a single translation window - since that's represented by the
> IOMMUMR. But what exactly do all the MRs - or whatever else - that
> are represented by the IOMMUObject have in common, from a functional
> point of view.
Let me take virt-SVM as an example. As far as I know, for virt-SVM,
the implementation of different vendors are similar. The key design
is to have a nested translation(aka. two stage translation). It is to
have guest maintain gVA->gPA mapping and hypervisor builds gPA->hPA
mapping. Similar to EPT based virt-MMU solution.
In Qemu, gPA->hPA mapping is done through MAP/UNMAP notifier, it can
keep going. But for gVA->gPA mapping, only guest knows it, so hypervisor
needs to trap specific guest iommu operation and pass the gVA->gPA
mapping knowledge to host through a notifier(newly added one). In VT-d,
it is called bind guest pasid table to host.
Also, for the gVA iotlb flushing, only guest knows it. So hypervisor
needs to propagate it to host. Here, MAP/UNMAP is not suitable since
this gVA iotlb flush here doesn’t require to modify host iommu
translation table. At the time gVA iotlb flush is issued, the gVA->gPA
mapping has already modified. Host iommu only needs to reference it when
performing address translation. But before host iommu perform translation,
it needs to flush the old gVA cache. In VT-d, it is called 1st level cache
flushing.
Both of the two notifiers(operations) has no direct relationship with MR,
instead they highly depends on virt-iommu itself. If virt-iommu exists,
then the two notfiers are needed, if not, then it's not.
> Even understanding the SVM stuff better than I did, I don't really see
> why an AddressSpace is an obvious unit to have an IOMMUObject
> associated with it.
This will benefit the notifier registration. As my comments above, the
IOMMUObject is to represent iommu. Associate an AddressSpace with an
IOMMUObject makes it easy to check if it is necessary to register the
notifiers. If no IOMMUObject, means no virt-iommu exposed to guest, then
no need to register notifiers. For this, I also considered to use the
MemoryRegion.iommu_ops. e.g. for VT-d, it can be a loop to check all the
subregions and register notfiers if it is an iommu MemoryRegion. Peter
mentioned it may not work for SPAR. So he proposed associating an
AddressSpace with an IOMMUObject. I think it wroks and easier, so I
didn’t object it.
https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg04931.html
+/* Check if vIOMMU exists */
+QTAILQ_FOREACH(subregion, &as->root->subregions, subregions_link) {
+if (memory_region_is_iommu(subregion)) {
+IOMMUNotifier n1;
+
+/*
+
Re: [Qemu-devel] QEMU 3.0 ?
On 13 November 2017 at 10:03, Cédric Le Goater wrote: > One of the sweeping change for 3.0 could be to stop to maintaining > migration compatibility with older versions (2.x). Even if the > feature is really a must have in some cluster environment, the > code (and the developer) is starting to suffer from it. We certainly can't just drop all migration-compat. The closest we might come would be to say we dropped migration-compat from versions older than $X for some value of X. But this kind of discussion is what I mean -- various people have different ideas about what we might or might not drop, but we need to come to a consensus about what we can actually do first. thanks -- PMM
Re: [Qemu-devel] [PATCH for-2.12 3/3] block: Deprecate "backing": ""
On Fri, Nov 10, 2017 at 04:21:05PM -0600, Eric Blake wrote: > On 11/10/2017 04:13 PM, Max Reitz wrote: > > We have a clear replacement, so let's deprecate it. > > > > Signed-off-by: Max Reitz > > --- > > qapi/block-core.json | 4 ++-- > > block.c | 4 > > qemu-doc.texi| 7 +++ > > qemu-options.hx | 4 ++-- > > 4 files changed, 15 insertions(+), 4 deletions(-) > > Dan has more details on the proper documentation to tweak when declaring > something deprecated. So this patch is incomplete, but what you have > makes sense. Its ok, Max has added to the relevant appendix in qemu-doc.texi Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] QEMU 3.0 ?
On 13.11.2017 10:53, Peter Maydell wrote: > On 13 November 2017 at 07:14, Thomas Huth wrote: >> By the way, before everybody now introduces "2.12" machine types ... is >> there already a consensus that the next version will be "2.12" ? >> >> A couple of months ago, we discussed that we could maybe do a 3.0 after >> 2.11, e.g. here: >> >> https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg05056.html >> >> I'd still like to see that happen... Peter, any thoughts on this? > > I don't see the point in declaring a 3.0 unless we have some > sweeping change that merits it. I don't think we should do a > sweeping change unless we have a well laid out and agreed on > plan for how the transition works. Since we declared a lot of interfaces / features as deprecated in QEMU 2.10, we could finally remove them in the release after 2.11. Looking at https://qemu.weilnetz.de/doc/qemu-doc.html#Deprecated-features that's quite a bit already. That's IMHO a good justification for a 3.0 already. > So I would want to see the > plan discussed and agreed first, and then we can say "ok, and > we think we can do this in this timescale and so the version > at $DATE will be 3.0". We could maybe also start a wiki page to collect ideas for what we want to do with "3.0" ... but I guess a lot of the possible changes will just be turned down again since somebody will cry "we need to stay compatible with older versions! Forever!". So I somehow doubt that this is worth the effort. > Changing the version number should be > the last part of this process, not the first, in my view. Yeah, but you know how this works in QEMU-Land: Once the 2.12 is established in the heads of various people, we'll have a hard time to bump the version number again, since there's always somebody complaining... So I guess we'll likely end up doing it rather the Linux kernel way one day - when we feel that the minor number got too big (three digits, maybe?), we'll switch the major number without any further justification ;-) Thomas
Re: [Qemu-devel] [PATCH v17 6/6] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_VQ
Ping for comments, thanks.
On 11/03/2017 04:13 PM, Wei Wang wrote:
Negotiation of the VIRTIO_BALLOON_F_FREE_PAGE_VQ feature indicates the
support of reporting hints of guest free pages to the host via
virtio-balloon. The host requests the guest to report the free pages by
sending commands via the virtio-balloon configuration registers.
When the guest starts to report, the first element added to the free page
vq is a sequence id of the start reporting command. The id is given by
the host, and it indicates whether the following free pages correspond
to the command. For example, the host may stop the report and start again
with a new command id. The obsolete pages for the previous start command
can be detected by the id dismatching on the host. The id is added to the
vq using an output buffer, and the free pages are added to the vq using
input buffer.
Here are some explainations about the added configuration registers:
- host2guest_cmd: a register used by the host to send commands to the
guest.
- guest2host_cmd: written by the guest to ACK to the host about the
commands that have been received. The host will clear the corresponding
bits on the host2guest_cmd register. The guest also uses this register
to send commands to the host (e.g. when finish free page reporting).
- free_page_cmd_id: the sequence id of the free page report command
given by the host.
Signed-off-by: Wei Wang
Signed-off-by: Liang Li
Cc: Michael S. Tsirkin
Cc: Michal Hocko
---
drivers/virtio/virtio_balloon.c | 234
include/uapi/linux/virtio_balloon.h | 11 ++
2 files changed, 223 insertions(+), 22 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index b31fc25..4087f04 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -55,7 +55,12 @@ static struct vfsmount *balloon_mnt;
struct virtio_balloon {
struct virtio_device *vdev;
- struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
+ struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *free_page_vq;
+
+ /* Balloon's own wq for cpu-intensive work items */
+ struct workqueue_struct *balloon_wq;
+ /* The free page reporting work item submitted to the balloon wq */
+ struct work_struct report_free_page_work;
/* The balloon servicing is delegated to a freezable workqueue. */
struct work_struct update_balloon_stats_work;
@@ -65,6 +70,10 @@ struct virtio_balloon {
spinlock_t stop_update_lock;
bool stop_update;
+ /* Stop reporting free pages */
+ bool report_free_page_stop;
+ uint32_t free_page_cmd_id;
+
/* Waiting for host to ack the pages we released. */
wait_queue_head_t acked;
@@ -191,6 +200,30 @@ static void send_balloon_page_sg(struct virtio_balloon *vb,
kick_and_wait(vq, vb->acked);
}
+static void send_free_page_sg(struct virtqueue *vq, void *addr, uint32_t size)
+{
+ int err = 0;
+ unsigned int len;
+
+ /* Detach all the used buffers from the vq */
+ while (virtqueue_get_buf(vq, &len))
+ ;
+
+ /*
+* Since this is an optimization feature, losing a couple of free
+* pages to report isn't important. We simply resturn without adding
+* the page if the vq is full.
+*/
+ if (vq->num_free) {
+ err = add_one_sg(vq, addr, size);
+ BUG_ON(err);
+ }
+
+ /* Batch till the vq is full */
+ if (!vq->num_free)
+ virtqueue_kick(vq);
+}
+
/*
* Send balloon pages in sgs to host. The balloon pages are recorded in the
* page xbitmap. Each bit in the bitmap corresponds to a page of PAGE_SIZE.
@@ -495,9 +528,8 @@ static void stats_handle_request(struct virtio_balloon *vb)
virtqueue_kick(vq);
}
-static void virtballoon_changed(struct virtio_device *vdev)
+static void virtballoon_cmd_balloon_memory(struct virtio_balloon *vb)
{
- struct virtio_balloon *vb = vdev->priv;
unsigned long flags;
spin_lock_irqsave(&vb->stop_update_lock, flags);
@@ -506,6 +538,50 @@ static void virtballoon_changed(struct virtio_device *vdev)
spin_unlock_irqrestore(&vb->stop_update_lock, flags);
}
+static void virtballoon_cmd_report_free_page_start(struct virtio_balloon *vb)
+{
+ unsigned long flags;
+
+ vb->report_free_page_stop = false;
+ spin_lock_irqsave(&vb->stop_update_lock, flags);
+ if (!vb->stop_update)
+ queue_work(vb->balloon_wq, &vb->report_free_page_work);
+ spin_unlock_irqrestore(&vb->stop_update_lock, flags);
+}
+
+static void virtballoon_changed(struct virtio_device *vdev)
+{
+ struct virtio_balloon *vb = vdev->priv;
+ u32 host2guest_cmd, guest2host_cmd = 0;
+
+ if (!virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ)) {
+ virtballoon_cmd_balloon_memory(vb);
+ return;
+
Re: [Qemu-devel] [PATCH] scripts/make-release: No need to delete pixman/.git anymore
On Mon, Nov 13, 2017 at 10:21:45AM +0100, Thomas Huth wrote: > The pixman submodule has been removed in commit c12b6d70e384c769ca372e1, > so there is no need anymore to delete pixman/.git while building a > release tarball. > > Signed-off-by: Thomas Huth Reviewed-by: Gerd Hoffmann
Re: [Qemu-devel] [PATCH V5] hw/pci-host: Fix x86 Host Bridges 64bit PCI hole
On 11/11/17 16:25, Marcel Apfelbaum wrote:
> Currently there is no MMIO range over 4G
> reserved for PCI hotplug. Since the 32bit PCI hole
> depends on the number of cold-plugged PCI devices
> and other factors, it is very possible is too small
> to hotplug PCI devices with large BARs.
>
> Fix it by reserving 2G for I4400FX chipset
> in order to comply with older Win32 Guest OSes
> and 32G for Q35 chipset.
>
> Even if the new defaults of pci-hole64-size will appear in
> "info qtree" also for older machines, the property was
> not implemented so no changes will be visible to guests.
>
> Note this is a regression since prev QEMU versions had
> some range reserved for 64bit PCI hotplug.
>
> Reviewed-by: Laszlo Ersek
> Reviewed-by: Gerd Hoffmann
> Signed-off-by: Marcel Apfelbaum
> ---
>
> V4 -> V5:
> - Renamed a local variable (Laszlo)
> - Added a comment to q35 props (Eduardo)
Looks good to me, thank you.
Laszlo
>
> V3 -> V4:
> - Addressed Laszlo's comments:
> - Added defines for pci-hole64 default size props.
> - Rounded the hole64_end to 1G
> - Moved some info to commit message
> - Addressed Michael's comments:
> - Added more comments.
> - I kept Gerd's "review-by" tag since no functional changes were made.
>
> V2 -> V3:
> - Addressed Gerd's and others comments and re-enabled the pci-hole64-size
>property defaulting it to 2G for I440FX and 32g for Q35.
> - Even if the new defaults of pci-hole64-size will appear in "info qtree"
>also for older machines, the property was not implemented so
>no changes will be visible to guests.
>
> V1 -> V2:
> Addressed Igor's comments:
> - aligned the hole64 start to 1Gb
> (I think all the computations took care of it already,
> but it can't hurt)
> - Init compat props to "off" instead of "false"
>
> hw/i386/pc.c | 22 ++
> hw/pci-host/piix.c| 32 ++--
> hw/pci-host/q35.c | 42 +++---
> include/hw/i386/pc.h | 10 +-
> include/hw/pci-host/q35.h | 1 +
> 5 files changed, 101 insertions(+), 6 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index e11a65b545..fafe5ba5cd 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1448,6 +1448,28 @@ void pc_memory_init(PCMachineState *pcms,
> pcms->ioapic_as = &address_space_memory;
> }
>
> +/*
> + * The 64bit pci hole starts after "above 4G RAM" and
> + * potentially the space reserved for memory hotplug.
> + */
> +uint64_t pc_pci_hole64_start(void)
> +{
> +PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> +PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
> +uint64_t hole64_start = 0;
> +
> +if (pcmc->has_reserved_memory && pcms->hotplug_memory.base) {
> +hole64_start = pcms->hotplug_memory.base;
> +if (!pcmc->broken_reserved_end) {
> +hole64_start += memory_region_size(&pcms->hotplug_memory.mr);
> +}
> +} else {
> +hole64_start = 0x1ULL + pcms->above_4g_mem_size;
> +}
> +
> +return ROUND_UP(hole64_start, 1ULL << 30);
> +}
> +
> qemu_irq pc_allocate_cpu_irq(void)
> {
> return qemu_allocate_irq(pic_irq_request, NULL, 0);
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index a7e2256870..a684a7cca9 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -50,6 +50,7 @@ typedef struct I440FXState {
> PCIHostState parent_obj;
> Range pci_hole;
> uint64_t pci_hole64_size;
> +bool pci_hole64_fix;
> uint32_t short_root_bus;
> } I440FXState;
>
> @@ -112,6 +113,9 @@ struct PCII440FXState {
> #define I440FX_PAM_SIZE 7
> #define I440FX_SMRAM0x72
>
> +/* Keep it 2G to comply with older win32 guests */
> +#define I440FX_PCI_HOST_HOLE64_SIZE_DEFAULT (1ULL << 31)
> +
> /* Older coreboot versions (4.0 and older) read a config register that
> doesn't
> * exist in real hardware, to get the RAM size from QEMU.
> */
> @@ -238,29 +242,52 @@ static void i440fx_pcihost_get_pci_hole_end(Object
> *obj, Visitor *v,
> visit_type_uint32(v, name, &value, errp);
> }
>
> +/*
> + * The 64bit PCI hole start is set by the Guest firmware
> + * as the address of the first 64bit PCI MEM resource.
> + * If no PCI device has resources on the 64bit area,
> + * the 64bit PCI hole will start after "over 4G RAM" and the
> + * reserved space for memory hotplug if any.
> + */
> static void i440fx_pcihost_get_pci_hole64_start(Object *obj, Visitor *v,
> const char *name,
> void *opaque, Error **errp)
> {
> PCIHostState *h = PCI_HOST_BRIDGE(obj);
> +I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj);
> Range w64;
> uint64_t value;
>
> pci_bus_get_w64_range(h->bus, &w64);
> value = range_is_empty(&w64) ? 0 : range_lob(&w64);
> +if (!value && s->pci_hole64_fix) {
> +value = p
Re: [Qemu-devel] [RFC PATCH 13/26] cpus: only take BQL for sleeping threads
On 13/11/2017 11:14, Alex Bennée wrote: > > Pavel Dovgalyuk writes: > >>> From: Paolo Bonzini [mailto:pbonz...@redhat.com] From: "David Hildenbrand" On 02.11.2017 12:08, Paolo Bonzini wrote: > On 31/10/2017 12:26, Pavel Dovgalyuk wrote: >> From: Alex Bennée >> >> Now the only real need to hold the BQL is for when we sleep on the >> cpu->halt conditional. The lock is actually dropped while the thread >> sleeps so the actual window for contention is pretty small. This also >> means we can remove the special case hack for exclusive work and >> simply declare that work no longer has an implicit BQL held. This >> isn't a major problem async work is generally only changing things in >> the context of its own vCPU. If it needs to work across vCPUs it >> should be using the exclusive mechanism or possibly taking the lock >> itself. >> >> Signed-off-by: Alex Bennée >> Tested-by: Pavel Dovgalyuk > > At least cpu_throttle_thread would fail with this patch. > > Also I am not sure if the s390 SIGP handlers are ready for this. > We have a global lock to the SIGP "facility". However we need the BQL in order to inject interrupts into CPUs (otherwise it would trigger an assert when injecting). We inject Restart and Stop interrupts from run_on_cpu. This requires the BQL. So Paolo should be right, this change would break s390x. >>> >>> I had some patches to access interrupt_request with the atomic builtins. If >>> Pavel can first extract the other changes to the icount mechanism, I can >>> update them. >> >> What changes do you mean here? >> I'm not sure that I understand clearly how threads interact with BQL. >> These patches were authored by Alex and we'll have to get him into the >> discussion. > > Do you want me to re-spin my sub-set of the patches as a new base? I think the first part to be merged is changes to cpu-exec.c and friends. These might even go into 2.11. Paolo
Re: [Qemu-devel] [Qemu-arm] [PATCH] arm/translate-a64: mark path as unreachable to eliminate warning
On 8 November 2017 at 12:37, Philippe Mathieu-Daudé wrote:
> On 11/07/2017 05:46 PM, Emilio G. Cota wrote:
>> Fixes the following warning when compiling with gcc 5.4.0 with -O1
>> optimizations and --enable-debug:
>>
>> target/arm/translate-a64.c: In function ‘aarch64_tr_translate_insn’:
>> target/arm/translate-a64.c:2361:8: error: ‘post_index’ may be used
>> uninitialized in this function [-Werror=maybe-uninitialized]
>> if (!post_index) {
>> ^
>> target/arm/translate-a64.c:2307:10: note: ‘post_index’ was declared here
>> bool post_index;
>> ^
>> target/arm/translate-a64.c:2386:8: error: ‘writeback’ may be used
>> uninitialized in this function [-Werror=maybe-uninitialized]
>> if (writeback) {
>> ^
>> target/arm/translate-a64.c:2308:10: note: ‘writeback’ was declared here
>> bool writeback;
>> ^
>>
>> Note that idx comes from selecting 2 bits, and therefore its value
>> can be at most 3.
>>
>> Signed-off-by: Emilio G. Cota
Applied to target-arm.next, thanks. (It's a bit sad that
gcc can't figure out that the result of x & 3 is constrained
to [0,3], but there you go.)
> Acked-by: Philippe Mathieu-Daudé
By the way, Philippe, what are you intending to convey with
an acked-by tag? Usually "acked-by" means "I'm the maintainer
for this area and I haven't actually reviewed this but I don't
object to it"...
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2] highbank: validate register offset before access
On 13 November 2017 at 06:26, P J P wrote: > From: Prasad J Pandit > > An 'offset' parameter sent to highbank register r/w functions > could be greater than number(NUM_REGS=0x200) of hb registers, > leading to an OOB access issue. Add check to avoid it. > > Reported-by: Moguofang (Dennis mo) > Signed-off-by: Prasad J Pandit > --- > hw/arm/highbank.c | 17 +++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > Update: use HWADDR_PRIx to print offset > -> https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg02116.html Applied to target-arm.next, thanks. -- PMM
Re: [Qemu-devel] [Qemu-arm] [PATCH v1 01/12] HACK: use objdump disas
Richard Henderson writes:
> ---
> disas.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/disas.c b/disas.c
> index d6a1eb9c8e..69069a85ca 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -231,7 +231,7 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong
> code,
> s.info.disassembler_options = (char *)"any";
> s.info.print_insn = print_insn_ppc;
> #endif
> -if (s.info.print_insn == NULL) {
> +if (1 || s.info.print_insn == NULL) {
> s.info.print_insn = print_insn_od_target;
> }
Because? Is this being droppped now for capstone?
--
Alex Bennée
Re: [Qemu-devel] [Qemu devel PATCH v2] MAINTAINERS: Add entries for Smartfusion2
On 13 November 2017 at 05:55, Subbaraya Sundeep wrote: > Voluntarily add myself as maintainer for Smartfusion2 > > Signed-off-by: Subbaraya Sundeep > Reviewed-by: Alistair Francis > Reviewed-by: Philippe Mathieu-Daudé > --- > v2: > reframed commit message as per Alistair's comment. > Renamed MSF2 SoC -> SmartFusion2 > MSF2 SOM -> Emcraft M2S-FG484 > as per Philippe's comments. Applied to target-arm.next, thanks. -- PMM
Re: [Qemu-devel] [Qemu-arm] [PATCH v1 02/12] target/arm: Add ARM_FEATURE_V8_1_SIMD
Richard Henderson writes:
"...and enable it for the 'any' CPUs used by linux-user"?
Otherwise:
Reviewed-by: Alex Bennée
> Signed-off-by: Richard Henderson
> ---
> target/arm/cpu.h | 1 +
> linux-user/elfload.c | 9 +
> target/arm/cpu.c | 1 +
> target/arm/cpu64.c | 1 +
> 4 files changed, 12 insertions(+)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 69cb49acc3..c5c9cef834 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -1312,6 +1312,7 @@ enum arm_features {
> ARM_FEATURE_VBAR, /* has cp15 VBAR */
> ARM_FEATURE_M_SECURITY, /* M profile Security Extension */
> ARM_FEATURE_JAZELLE, /* has (trivial) Jazelle implementation */
> +ARM_FEATURE_V8_1_SIMD, /* has ARMv8.1-SIMD */
> };
>
> static inline int arm_feature(CPUARMState *env, int feature)
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 79062882ba..003d9420b7 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -512,6 +512,14 @@ enum {
> ARM_HWCAP_A64_SHA1 = 1 << 5,
> ARM_HWCAP_A64_SHA2 = 1 << 6,
> ARM_HWCAP_A64_CRC32 = 1 << 7,
> +ARM_HWCAP_A64_ATOMICS = 1 << 8,
> +ARM_HWCAP_A64_FPHP = 1 << 9,
> +ARM_HWCAP_A64_ASIMDHP = 1 << 10,
> +ARM_HWCAP_A64_CPUID = 1 << 11,
> +ARM_HWCAP_A64_ASIMDRDM = 1 << 12,
> +ARM_HWCAP_A64_JSCVT = 1 << 13,
> +ARM_HWCAP_A64_FCMA = 1 << 14,
> +ARM_HWCAP_A64_LRCPC = 1 << 15,
> };
>
> #define ELF_HWCAP get_elf_hwcap()
> @@ -532,6 +540,7 @@ static uint32_t get_elf_hwcap(void)
> GET_FEATURE(ARM_FEATURE_V8_SHA1, ARM_HWCAP_A64_SHA1);
> GET_FEATURE(ARM_FEATURE_V8_SHA256, ARM_HWCAP_A64_SHA2);
> GET_FEATURE(ARM_FEATURE_CRC, ARM_HWCAP_A64_CRC32);
> +GET_FEATURE(ARM_FEATURE_V8_1_SIMD, ARM_HWCAP_A64_ASIMDRDM);
> #undef GET_FEATURE
>
> return hwcaps;
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 4300de66e2..276c996e9f 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1603,6 +1603,7 @@ static void arm_any_initfn(Object *obj)
> set_feature(&cpu->env, ARM_FEATURE_V8_SHA256);
> set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
> set_feature(&cpu->env, ARM_FEATURE_CRC);
> +set_feature(&cpu->env, ARM_FEATURE_V8_1_SIMD);
> cpu->midr = 0x;
> }
> #endif
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 670c07ab6e..b05c904ad2 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -226,6 +226,7 @@ static void aarch64_any_initfn(Object *obj)
> set_feature(&cpu->env, ARM_FEATURE_V8_SHA256);
> set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
> set_feature(&cpu->env, ARM_FEATURE_CRC);
> +set_feature(&cpu->env, ARM_FEATURE_V8_1_SIMD);
> cpu->ctr = 0x80038003; /* 32 byte I and D cacheline size, VIPT icache */
> cpu->dcz_blocksize = 7; /* 512 bytes */
> }
--
Alex Bennée
Re: [Qemu-devel] [PATCH for-2.11] nbd: Don't crash when server reports NBD_CMD_READ failure
12.11.2017 04:39, Eric Blake wrote:
If a server fails a read, for example with EIO, but the connection
is still live, then we would crash trying to print a non-existent
error message. Bug introduced in commit f140e300.
Signed-off-by: Eric Blake
---
block/nbd-client.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/nbd-client.c b/block/nbd-client.c
index b44d4d4a01..5f3375a970 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -78,7 +78,7 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
while (!s->quit) {
assert(s->reply.handle == 0);
ret = nbd_receive_reply(s->ioc, &s->reply, &local_err);
hmm.
/* nbd_receive_reply
* Returns 1 on success
* 0 on eof, when no data was read (errp is not set)
* negative errno on failure (errp is set)
*/
int nbd_receive_reply(QIOChannel *ioc, NBDReply *reply, Error **errp)
- looks like errp should be set if ret < 0. may be the function should
be fixed?
- don't you think that this function returns transferred server error?
it doesn't.
-if (ret < 0) {
+if (local_err) {
error_report_err(local_err);
}
if (ret <= 0) {
@@ -681,7 +681,7 @@ int nbd_client_co_preadv(BlockDriverState *bs, uint64_t
offset,
ret = nbd_co_receive_cmdread_reply(client, request.handle, offset, qiov,
&local_err);
-if (ret < 0) {
+if (local_err) {
error_report_err(local_err);
}
return ret;
but here, it looks like you are right. My first attempt was to store
server error to ret and
server error msg to errp (bad idea, as I can see now), but you proposed
to do not print every server error msg and
deleted this functionality. And now we have a set of functions which can
return ret < 0
and not set errp.. It looks very confusing, I think. Better solution is
to refactor this somehow,
may be not mixing server-error-reply with local normal errors..
For now, for second chunk:
Reviewed-by: Vladimir Sementsov-Ogievskiy
--
Best regards,
Vladimir
Re: [Qemu-devel] [PATCH for 2.11 0/5] TCG/ARM fixes for 2.11
On 10 November 2017 at 19:53, Emilio G. Cota wrote: > Some MachineClass changes to fix TCG initialization of some > ARM boards for 2.11. This was originally reported by Thomas Huth in [1], > where Peter suggested a way to fix it. Further discussion in > another thread [2] followed up on this. > > As a result of that follow-up discussion we also got some Zynq changes from > Alistair [3], which I'm including here since in order to test them > we need the first patch in this series. > > Thanks, > > Emilio > > [1] https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg00078.html > [2] https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg00502.html > [3] https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg01842.html > > Alistair Francis (2): > xlnx-zynqmp: Properly support the smp command line option > xlnx-zcu102: Add an info message deprecating the EP108 > > Emilio G. Cota (3): > qom: move CPUClass.tcg_initialize to a global > xlnx-zcu102: Specify the max number of CPUs for the EP108 > hw: add .min_cpus and .default_cpus fields to machine_class > > qemu-doc.texi | 7 +++ > include/hw/boards.h | 5 + > include/qom/cpu.h | 1 - > exec.c | 5 +++-- > hw/arm/exynos4_boards.c | 12 > hw/arm/raspi.c | 2 ++ > hw/arm/xlnx-zcu102.c| 9 - > hw/arm/xlnx-zynqmp.c| 26 -- > vl.c| 21 ++--- > 9 files changed, 63 insertions(+), 25 deletions(-) Since this is fixing Arm boards and I'm putting together an arm pullreq anyway, I'm applying this to target-arm.next. (Eduardo's reviewed the patches which touch the MachineClass and option parsing code.) thanks -- PMM
Re: [Qemu-devel] [PATCH v1] migration/ram.c: do not set 'postcopy_running' in POSTCOPY_INCOMING_END
Hi Peter,
On 11/13/2017 01:22 AM, Peter Xu wrote:
On Fri, Nov 10, 2017 at 06:35:16PM -0200, Daniel Henrique Barboza wrote:
When migrating a VM with 'migrate_set_capability postcopy-ram on'
a postcopy_state is set during the process, ending up with the
state POSTCOPY_INCOMING_END when the migration is over. This
postcopy_state is taken into account inside ram_load to check
how it will load the memory pages. This same ram_load is called when
in a loadvm command.
Inside ram_load, the logic to see if we're at postcopy_running state
is:
postcopy_running = postcopy_state_get() >= POSTCOPY_INCOMING_LISTENING
postcopy_state_get() returns this enum type:
typedef enum {
POSTCOPY_INCOMING_NONE = 0,
POSTCOPY_INCOMING_ADVISE,
POSTCOPY_INCOMING_DISCARD,
POSTCOPY_INCOMING_LISTENING,
POSTCOPY_INCOMING_RUNNING,
POSTCOPY_INCOMING_END
} PostcopyState;
In the case where ram_load is executed and postcopy_state is
POSTCOPY_INCOMING_END, postcopy_running will be set to 'true' and
ram_load will behave like a postcopy is in progress. This scenario isn't
achievable in a migration but it is reproducible when executing
savevm/loadvm after migrating with 'postcopy-ram on', causing loadvm
to fail with Error -22:
Source:
(qemu) migrate_set_capability postcopy-ram on
(qemu) migrate tcp:127.0.0.1:
Dest:
(qemu) migrate_set_capability postcopy-ram on
(qemu)
ubuntu1704-intel login:
Ubuntu 17.04 ubuntu1704-intel ttyS0
ubuntu1704-intel login: (qemu)
(qemu) savevm test1
(qemu) loadvm test1
Unknown combination of migration flags: 0x4 (postcopy mode)
error while loading state for instance 0x0 of device 'ram'
Error -22 while loading VM state
(qemu)
This patch fixes this problem by changing a bit the semantics
of postcopy_running inside ram_load, verifying first if
we're not in the POSTCOPY_INCOMING_END state. In this case,
postcopy_running is set to 'false'.
Signed-off-by: Daniel Henrique Barboza
---
migration/ram.c | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 8620aa400a..43ed719668 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2803,13 +2803,21 @@ static int ram_load(QEMUFile *f, void *opaque, int
version_id)
int flags = 0, ret = 0, invalid_flags = 0;
static uint64_t seq_iter;
int len = 0;
-/*
- * If system is running in postcopy mode, page inserts to host memory must
- * be atomic
- */
-bool postcopy_running = postcopy_state_get() >=
POSTCOPY_INCOMING_LISTENING;
-/* ADVISE is earlier, it shows the source has the postcopy capability on */
-bool postcopy_advised = postcopy_state_get() >= POSTCOPY_INCOMING_ADVISE;
+bool postcopy_advised = false, postcopy_running = false;
+uint8_t postcopy_state = postcopy_state_get();
+
+if (postcopy_state != POSTCOPY_INCOMING_END) {
+/*
+ * If system is running in postcopy mode, page inserts to host memory
+ * must be atomic
+ */
+postcopy_running = postcopy_state >= POSTCOPY_INCOMING_LISTENING;
+
+/* ADVISE is earlier, it shows the source has the postcopy
+ * capability on
+ */
+postcopy_advised = postcopy_state >= POSTCOPY_INCOMING_ADVISE;
+}
For me, this is not that clear. I would prefer something simpler like:
PostcopyState state = postcopy_state_get();
bool postcopy_running = state >= POSTCOPY_INCOMING_LISTENING && \
state < POSTCOPY_INCOMING_END;
Or we can introduce postcopy_is_running() helper. Same thing to
postcopy_advised. Thanks,
I like the idea of creating helpers to make the logic simpler. I'll make
this change in the next spin.
Thanks,
Daniel
seq_iter++;
--
2.13.6
Re: [Qemu-devel] [PULL for-2.11 0/2] Capstone updates
On 9 November 2017 at 07:52, Richard Henderson wrote: > One build fix for mingw cross-compiling, one feature regression fix. > > > r~ > > > The following changes since commit b0fbe46ad82982b289a44ee2495b59b0bad8a842: > > Update version for v2.11.0-rc0 release (2017-11-07 16:05:28 +) > > are available in the git repository at: > > git://github.com/rth7680/qemu.git tags/pull-cap-20171109 > > for you to fetch changes up to 9f81aeb5dadab588920b157a4610e890995ef4b9: > > Makefile: Capstone: Add support for cross compile ranlib (2017-11-09 > 08:47:14 +0100) > > > Capstone fixes for 2.11 > > > Alistair Francis (1): > Makefile: Capstone: Add support for cross compile ranlib > > Richard Henderson (1): > disas: Dump insn bytes along with capstone disassembly > Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH 2/7] s390x/pci: rework PCI STORE
On Mon, 13 Nov 2017 10:03:37 +0100 Pierre Morel wrote: > On 09/11/2017 17:50, Cornelia Huck wrote: > > On Tue, 7 Nov 2017 18:24:34 +0100 > > Pierre Morel wrote: > >> +case PCI_ROM_SLOT: > > > > So, will this be filled in a later patch? (Reading from the top.) > > No it will not it is only a reminder that we explicitly do not support it. > Since I do not know if we ever will support it do you think I better let > it fall? Either add a comment, or drop it. Otherwise I will ask again in the future :)
Re: [Qemu-devel] [PATCH 6/7] s390x/pci: move the memory region write from pcistg
On Mon, 13 Nov 2017 10:39:50 +0100 Pierre Morel wrote: > On 10/11/2017 10:51, Cornelia Huck wrote: > > On Fri, 10 Nov 2017 17:40:12 +0800 > > Yi Min Zhao wrote: > > > >> 在 2017/11/10 上午3:23, Cornelia Huck 写道: > >>> On Tue, 7 Nov 2017 18:24:38 +0100 > >>> Pierre Morel wrote: > >>> > Let's move the memory region write from pcistg into a dedicated > function. > This allows us to prepare a later patch searching for subregions > inside of the memory region. > >>> OK, so here is the memory region write. Do we have any sleeping > >>> endianness bugs in there for when we wire up tcg? I'm not sure how this > >>> plays with the bswaps (see patch 1). > >>> > >>> But maybe I've just gotten lost somewhere. > >> I think there's no error. For PCI bars' MRs, we got the little-endian data > >> that is exactly fit to the byte ordering of pcilg instruction. For PCI > >> config > >> space, the data has been swapped according to the cpu byte ordering. > > > > Host or target cpu? > > > >> So we use zpci_swap_endian() to swap the data back to the little-endian > >> ordering. > > > > > I do not see where we use the zpci_swap_endian() function in this patch > or in the zpci_write_bar() function. So, is that swap function only ever used to convert BE register contents to LE? > > > > > That swap is unconditional. If we were running on a little-endian host, > > it would be wrong, wouldn't it? > > I think there is no problem here, we do not use the swap function but we > use the memory_region_dispatch_write() to access a subregion of the PCI > device which is defined as DEVICE_LITTLE_ENDIAN > > AFAIU The memory access process the endianness correctly. OK, if this is all going through generic memory mechanisms, we should be fine. > > > > > >>> > Signed-off-by: Pierre Morel > Reviewed-by: Yi Min Zhao > --- > hw/s390x/s390-pci-inst.c | 27 +-- > 1 file changed, 17 insertions(+), 10 deletions(-) > > > >
Re: [Qemu-devel] [PATCH v2 00/43] Windbg supporting
On Wed, Nov 8, 2017 at 3:28 PM, Mihail Abakumov wrote: > Ladi Prosek писал 2017-11-08 16:43: > >> On Wed, Nov 8, 2017 at 12:54 PM, Mihail Abakumov >> wrote: >>> >>> Ladi Prosek писал 2017-11-06 18:15: >>> >>> In your case windbg does not send packets, more specifically, does not >>> continue to do it. What is your version of windbg. Also, windbg stub has >>> been supported only for windbg x64, yet. >> >> >> Ah, so that's a pretty important piece of information and not very >> intuitive given the "Only i386 is supported now." sentence in the >> cover email. >> >> Microsoft (R) Windows Debugger Version 10.0.15063.468 X86 >> Microsoft (R) Windows Debugger Version 10.0.15063.468 AMD64 >> Microsoft (R) Windows Debugger Version 10.0.16299.15 X86 >> Microsoft (R) Windows Debugger Version 10.0.16299.15 AMD64 >> >> are the versions I have tried. I don't see any difference between x86 >> and amd64, all versions never connect and crash after the second >> break. >> >> What guest OS are you running? Can you maybe zip up your QEMU binaries >> and share them with me? >> >> Thanks, >> Ladi > > > Oh, it looks like a problem in the versions. I use the Windbg from Windows > 7. > Windbg version: 6.12.0002.633 AMD64. I will try to test with your version. Looks like it's the -b switch making the difference here. It was removed in later versions of windbg. Windbg docs (older): -b (Kernel mode only) This option has two effects: 1. The debugger will break into the target computer immediately upon connection. 2. After a reboot, the debugger will break into the target computer once the kernel is initialized. See Crashing and Rebooting the Target Computer for details and for other methods of changing this status. Windbg docs (newer): -b This option is no longer supported. -b makes windbg send a break-in right after connecting. Apparently there is a short time window when the break-in will work. I can actually successfully connect with a Win10 windbg (so without -b) if I hit Ctrl+Break shortly after the initial handshake. This also explains why I was able to connect once during my initial testing. I was just lucky and hit Ctrl+Break soon enough after connecting. By initial handshake I mean this sequence (I have added a simple logging, should be clear what it means): Received control PACKET_TYPE_KD_RESET Sending data 7 Sending control 6 Newer debuggers without -b will stop here. Older debuggers with -b will continue with: Received RESULT_BREAKIN_BYTE Sending data 7 Received RESULT_BREAKIN_BYTE Sending data 7 Received data PACKET_TYPE_KD_STATE_MANIPULATE (12614) Sending control 4 Sending data 2 Received data PACKET_TYPE_KD_STATE_MANIPULATE (12592) Sending control 4 Sending data 2 Received data PACKET_TYPE_KD_STATE_MANIPULATE (12592) Sending control 4 Sending data 2 ... Next step, when I have time, is to find the differences between sending the break-in early and sending it later. Also interesting would be recording the exchange between windbg and the target when doing regular remote kernel debugging. I would still expect to see the "Connected to Windows 7 7601 x86 compatible target ..." output always, even when not breaking in immediately. The handshake is likely still missing something. Thanks, Ladi
[Qemu-devel] [RFC 3/5] hw/arm: add scattered RAM memory region support
From: Zhu Yijun
Dig out reserved memory holes and collect scattered RAM memory
regions by adding mem_list member in arm_boot_info struct.
Signed-off-by: Zhu Yijun
---
hw/arm/boot.c| 8
hw/arm/virt.c| 101 ++-
include/hw/arm/arm.h | 1 +
3 files changed, 108 insertions(+), 2 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index c2720c8..30438f4 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -836,6 +836,14 @@ static void arm_load_kernel_notify(Notifier *notifier,
void *data)
*/
assert(!(info->secure_board_setup && kvm_enabled()));
+/* If machine is not virt, the mem_list will empty. */
+if (QLIST_EMPTY(&vms->bootinfo.mem_list)) {
+RAMRegion *new = g_new(RAMRegion, 1);
+new->base = info->loader_start;
+new->size = info->ram_size;
+QLIST_INSERT_HEAD(&vms->bootinfo.mem_list, new, next);
+}
+
info->dtb_filename = qemu_opt_get(qemu_get_machine_opts(), "dtb");
/* Load the kernel. */
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ddde5e1..ff334c1 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -56,6 +56,7 @@
#include "hw/smbios/smbios.h"
#include "qapi/visitor.h"
#include "standard-headers/linux/input.h"
+#include "hw/vfio/vfio-common.h"
#define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
@@ -1225,6 +1226,98 @@ void virt_machine_done(Notifier *notifier, void *data)
virt_build_smbios(vms);
}
+static void handle_reserved_ram_region_overlap(void)
+{
+hwaddr cur_end, next_end;
+RAMRegion *reg, *next_reg, *tmp_reg;
+
+QLIST_FOREACH(reg, &reserved_ram_regions, next) {
+next_reg = QLIST_NEXT(reg, next);
+
+while (next_reg && next_reg->base <= (reg->base + reg->size)) {
+next_end = next_reg->base + next_reg->size;
+cur_end = reg->base + reg->size;
+if (next_end > cur_end) {
+reg->size += (next_end - cur_end);
+}
+
+tmp_reg = QLIST_NEXT(next_reg, next);
+QLIST_REMOVE(next_reg, next);
+g_free(next_reg);
+next_reg = tmp_reg;
+}
+}
+}
+
+static void update_memory_regions(VirtMachineState *vms, hwaddr ram_size)
+{
+
+RAMRegion *new, *reg, *last = NULL;
+hwaddr virt_start, virt_end;
+virt_start = vms->memmap[VIRT_MEM].base;
+virt_end = virt_start + ram_size - 1;
+
+handle_reserved_ram_region_overlap();
+
+QLIST_FOREACH(reg, &reserved_ram_regions, next) {
+if (reg->base >= virt_start && reg->base < virt_end) {
+if (reg->base == virt_start) {
+virt_start += reg->size;
+virt_end += reg->size;
+continue;
+} else {
+new = g_new(RAMRegion, 1);
+new->base = virt_start;
+new->size = reg->base - virt_start;
+virt_start = reg->base + reg->size;
+}
+
+if (QLIST_EMPTY(&vms->bootinfo.mem_list)) {
+QLIST_INSERT_HEAD(&vms->bootinfo.mem_list, new, next);
+} else {
+QLIST_INSERT_AFTER(last, new, next);
+}
+
+last = new;
+ram_size -= new->size;
+virt_end += reg->size;
+}
+}
+
+if (ram_size > 0) {
+new = g_new(RAMRegion, 1);
+new->base = virt_start;
+new->size = ram_size;
+
+if (QLIST_EMPTY(&vms->bootinfo.mem_list)) {
+QLIST_INSERT_HEAD(&vms->bootinfo.mem_list, new, next);
+} else {
+QLIST_INSERT_AFTER(last, new, next);
+}
+}
+}
+
+static void create_ram_alias(VirtMachineState *vms,
+ MemoryRegion *sysmem,
+ MemoryRegion *ram)
+{
+RAMRegion *reg;
+MemoryRegion *ram_memory;
+char *nodename;
+hwaddr sz = 0;
+
+QLIST_FOREACH(reg, &vms->bootinfo.mem_list, next) {
+nodename = g_strdup_printf("ram@%" PRIx64, reg->base);
+ram_memory = g_new(MemoryRegion, 1);
+memory_region_init_alias(ram_memory, NULL, nodename, ram, sz,
+ reg->size);
+memory_region_add_subregion(sysmem, reg->base, ram_memory);
+sz += reg->size;
+
+g_free(nodename);
+}
+}
+
static void virt_ram_memory_region_init(Notifier *notifier, void *data)
{
MachineState *machine = MACHINE(qdev_get_machine());
@@ -1232,10 +1325,15 @@ static void virt_ram_memory_region_init(Notifier
*notifier, void *data)
MemoryRegion *ram = g_new(MemoryRegion, 1);
VirtMachineState *vms = container_of(notifier, VirtMachineState,
ram_memory_region_init);
+RAMRegion *first_mem_reg;
memory_region_allocate_system_memory(ram, NULL, "mach-virt.ram",
machine->ram_size);
-memory_regi
[Qemu-devel] [RFC 0/5] arm: Exclude reserved memory regions of iommu to avoid
From: Zhu Yijun
With kernel 4.11, iommu/smmu will populate the MSI IOVA reserved window and
PCI reserved window which has to be excluded from Guest iova allocations.
And on certain HiSilicon platforms (hip06/hip07), the GIC ITS and PCIe RC
deviates from the standard implementation will reserve the hw msi regions in
the smmu-v3 driver which means these address regions will not be translated.
https://www.spinics.net/lists/linux-pci/msg65125.html
On such platforms, reserved memory regions will export like this:
root:~# cat /sys/kernel/iommu_groups/7/reserved_regions
0xa880 0xaffe reserved
0xc600 0xc601 msi
However, it falls within the Qemu default virtual memory address space.
Take a look at hw/arm/virt.c:
[VIRT_MEM] = { 0x4000, RAMLIMIT_BYTES },
. . .
memory_region_allocate_system_memory(ram, NULL, "mach-virt.ram",
machine->ram_size);
memory_region_add_subregion(sysmem, vms->memmap[VIRT_MEM].base, ram);
So suppose we allocate 4GB mem to a VM, there is a chance that the reserved
regions will get allocated for a Guest VF DMA iova and it will fail.
This patchset create holes in the Qemu RAM address space which exclude
the sysfs exported regions.
Zhu Yijun (5):
hw/vfio: Add function for getting reserved_region of device iommu
group
hw/arm/virt: Enable dynamic generation of guest RAM memory regions
hw/arm: add scattered RAM memory region support
hw/arm/boot: set fdt size cell of memory node from mem_list
hw/arm/virt-acpi-build: Build srat table according to mem_list
hw/arm/boot.c | 155 +++---
hw/arm/virt-acpi-build.c | 40 +--
hw/arm/virt.c | 120 ++--
hw/vfio/common.c | 67 ++
hw/vfio/pci.c | 2 +
hw/vfio/platform.c| 2 +
include/exec/memory.h | 7 ++
include/hw/arm/arm.h | 1 +
include/hw/arm/virt.h | 1 +
include/hw/vfio/vfio-common.h | 3 +
10 files changed, 347 insertions(+), 51 deletions(-)
--
1.8.3.1
[Qemu-devel] [RFC 1/5] hw/vfio: Add function for getting reserved_region of device iommu group
From: Zhu Yijun
With kernel 4.11, iommu/smmu will populate the MSI IOVA reserved window and
PCI reserved window which has to be excluded from Guest iova allocations.
However, If it falls within the Qemu default virtual memory address space,
then reserved regions may get allocated for a Guest VF DMA iova and it will
fail.
So get those reserved regions in this patch and create some holes in the Qemu
ram address in next patchset.
Signed-off-by: Zhu Yijun
---
hw/vfio/common.c | 67 +++
hw/vfio/pci.c | 2 ++
hw/vfio/platform.c| 2 ++
include/exec/memory.h | 7 +
include/hw/vfio/vfio-common.h | 3 ++
5 files changed, 81 insertions(+)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 7b2924c..01bdbbd 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -40,6 +40,8 @@ struct vfio_group_head vfio_group_list =
QLIST_HEAD_INITIALIZER(vfio_group_list);
struct vfio_as_head vfio_address_spaces =
QLIST_HEAD_INITIALIZER(vfio_address_spaces);
+struct reserved_ram_head reserved_ram_regions =
+QLIST_HEAD_INITIALIZER(reserved_ram_regions);
#ifdef CONFIG_KVM
/*
@@ -52,6 +54,71 @@ struct vfio_as_head vfio_address_spaces =
static int vfio_kvm_device_fd = -1;
#endif
+void update_reserved_regions(hwaddr addr, hwaddr size)
+{
+RAMRegion *reg, *new;
+
+new = g_new(RAMRegion, 1);
+new->base = addr;
+new->size = size;
+
+if (QLIST_EMPTY(&reserved_ram_regions)) {
+QLIST_INSERT_HEAD(&reserved_ram_regions, new, next);
+return;
+}
+
+/* make the base of reserved_ram_regions increase */
+QLIST_FOREACH(reg, &reserved_ram_regions, next) {
+if (addr > (reg->base + reg->size - 1)) {
+if (!QLIST_NEXT(reg, next)) {
+QLIST_INSERT_AFTER(reg, new, next);
+break;
+}
+continue;
+} else if (addr >= reg->base && addr <= (reg->base + reg->size - 1)) {
+/* discard the duplicate entry */
+if (addr == reg->base && size == reg->size) {
+g_free(new);
+break;
+} else {
+QLIST_INSERT_AFTER(reg, new, next);
+break;
+}
+} else {
+QLIST_INSERT_BEFORE(reg, new, next);
+break;
+}
+}
+}
+
+void vfio_get_iommu_group_reserved_region(char *group_path)
+{
+char *filename;
+FILE *fp;
+hwaddr start, end;
+char str[10];
+struct stat st;
+
+filename = g_strdup_printf("%s/iommu_group/reserved_regions", group_path);
+if (stat(filename, &st) < 0) {
+g_free(filename);
+return;
+}
+
+fp = fopen(filename, "r");
+if (!fp) {
+g_free(filename);
+return;
+}
+
+while (fscanf(fp, "%"PRIx64" %"PRIx64" %s", &start, &end, str) != EOF) {
+update_reserved_regions(start, (end - start + 1));
+}
+
+g_free(filename);
+fclose(fp);
+}
+
/*
* Common VFIO interrupt disable
*/
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index c977ee3..9bffb38 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2674,6 +2674,8 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
vdev->vbasedev.type = VFIO_DEVICE_TYPE_PCI;
vdev->vbasedev.dev = &vdev->pdev.qdev;
+vfio_get_iommu_group_reserved_region(vdev->vbasedev.sysfsdev);
+
tmp = g_strdup_printf("%s/iommu_group", vdev->vbasedev.sysfsdev);
len = readlink(tmp, group_path, sizeof(group_path));
g_free(tmp);
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index da84abf..d5bbc3a 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -578,6 +578,8 @@ static int vfio_base_device_init(VFIODevice *vbasedev,
Error **errp)
return -errno;
}
+vfio_get_iommu_group_reserved_region(vbasedev->sysfsdev);
+
tmp = g_strdup_printf("%s/iommu_group", vbasedev->sysfsdev);
len = readlink(tmp, group_path, sizeof(group_path));
g_free(tmp);
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 5ed4042..2bcc83b 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -46,6 +46,13 @@
OBJECT_GET_CLASS(IOMMUMemoryRegionClass, (obj), \
TYPE_IOMMU_MEMORY_REGION)
+/* Scattered RAM memory region struct */
+typedef struct RAMRegion {
+hwaddr base;
+hwaddr size;
+QLIST_ENTRY(RAMRegion) next;
+} RAMRegion;
+
typedef struct MemoryRegionOps MemoryRegionOps;
typedef struct MemoryRegionMmio MemoryRegionMmio;
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index f3a2ac9..3483ca6 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -161,10 +161,13 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as,
Error **errp);
void vfio_put_group(VFIOGroup *group);
int vfio_get_device(VFIOGroup *group, const char *name,
VFIODevice *vbasedev, Error **errp);
+void update_
Re: [Qemu-devel] QMP, HMP: introduce 'writeconfig' command
"Dr. David Alan Gilbert" writes: > * Vadim Galitsyn (vadim.galit...@profitbricks.com) wrote: >> Hi Guys, >> >> This thread is a continuation of discussion started in >> http://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg03182.html. >> >> This series introduces ‘writeconfig’ command support for QMP and HMP >> monitors. This functionality might be useful for live migration for >> cases when guest configuration was modified in runtime (for example >> as a result of hot- plug/unplug operations) and actual Qemu command >> line no longer reflects setup exposed to guest. >> >> Original series has ‘qemu_opts’ patch as well >> (http://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg03183.html) >> because HMP’s ‘object_add’ result was not reflected in ‘writeconfig’ >> output. Later I found that QMP’s ‘object-add’ has the same >> issue. Anyway, I don’t include ‘qemu_opts’ patches here because >> Markus mentioned (here >> http://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg03476.html) >> that this functionality is going to be reworked in some future and >> such patches might collide with the rework process. This is item 1 of my reply. Item 3 is more important: The accuracy of QemuOpts information is doubtful. Completeness: only certain kinds of configuration are done with QemuOpts. Incompleteness makes -writeconfig less useful than it could be, but it's still useful. Monitor command writeconfig could be similarly useful. Correctness: configuration gets stored in QemuOpts when we parse KEY=VALUE,... strings. It can also be constructed and updated manually. At certain points in time, bits from QemuOpts are used to actually configure stuff. Example: -device creates an entry in the "device" configuration group, which is later used to actually create and configure a device object. My point is: whenever we manipulate the actual objects, we may invalidate information stored in QemuOpts. We can try to keep it in sync, and we do at least sometimes. But this is a game we can only lose, except for the period(s) of time where QemuOpts is all there is, i.e. before actual objects get created. Note that -writeconfig runs before objects get created, so it's not affected by this issue. Out-of-sync QemuOpts is harmless unless something relies on it being accurate. I know we currently rely on QemuOpts IDs to catch duplicate IDs for some of the configuration groups. I doubt there's much else. If we add your monitor command, out-of-sync QemuOpts goes from harmless to serious bug. In other words, we'd create a new class of bugs, with an unknown number of existing instances that are probably hard to find and fix. Probably a perpetual source of new instances, too. Feels like a show stopper to me. Message-ID: <87k28qlca9@dusky.pond.sub.org> >> Markus, could you please post if you have an update on this topic? >> Current ‘master’ branch (9993c82dc2f5ce58b41d708b765e1a717ad4281d) >> still has the issue. There's my "[RFC PATCH 00/32] Command line QAPIfication" Message-Id: <20171002152552.27999-1-arm...@redhat.com> https://lists.nongnu.org/archive/html/qemu-devel/2017-10/msg00209.html There's also my KVM Forum presentation. Permanent links to slides and hopefully video should appear at http://www.linux-kvm.org/page/KVM_Forum_2017 "soon". Until then, you can also get my slides from https://events.linuxfoundation.org/sites/events/files/slides/armbru-qapi-cmdline_1.pdf >> Also, Markus mentioned that once configuration was changed during >> live migration -- it might be an issue because ‘writeconfig’ data >> became outdated (and might be make sense to think about to embed this >> data into migration stream itself). In the same time David said that >> this is another problem which is unrelated to this patch series. What >> is your current opinion on this topic? Can we consider these patches >> to be included into ‘master’ taking into account that not all >> configuration is dumped by ‘writeconfig’ (‘object_add’ problem), but >> this can be fixed later? > > I don't see anything wrong with having a 'writeconfig' command - we > already have the command line equivalent - that is assuming hotplug > etc all cause the values written to be correct. In the current state of things, this assumption is somewhere between reckless and wrong. See "this is a game we can only lose" above. > The data becoming outdated doesn't sound like a big issue to me; > it's the management layer that's doing any hotplugs, it can reissue > another 'writeconfig' command. It can, but how would it use the data? It already started the destination QEMU with configuration derived from the first writeconfig, plus -incoming. Source configuration may change until we're ready to pivot from to the destination. A second writeconfig only tells the management application what configuration it has to reach on the destination, but not how it could be don
Re: [Qemu-devel] [PATCH 1/8] sdl2: Fix broken display updating after the window is hidden
On Sun, Nov 12, 2017 at 08:30:25PM +0100, Jindrich Makovicka wrote: > With SDL 2.0.6, calling SDL_ShowWindow during SDL_WINDOWEVENT_HIDDEN > blocks all subsequent display updates. > > Instead of trying to override the change, just update the scon->hidden > flag. Has for me the side effect that sometimes I have to press ctrl-alt-2 twice. Showing window first time works. Hiding the window works too. Showing it the second time needs the double keypress. Added fprintfs to figure why [1]. Reason for that seems to be a suspious SDL_WINDOWEVENT_SHOWN event from SDL, so qemu thinks the window is visible even though it actually is hidden. Have a slightly older SDL version (2.0.3). Do you see that effect with 2.0.6 too? cheers, Gerd https://www.kraxel.org/cgit/qemu/commit/?h=testing/sdl&id=8235bfb66e7c82870ff1b1f47bdaaf4f522c5444
Re: [Qemu-devel] [PATCH 1/8] sdl2: Fix broken display updating after the window is hidden
On Mon, Nov 13, 2017 at 01:39:14PM +0100, Gerd Hoffmann wrote: > On Sun, Nov 12, 2017 at 08:30:25PM +0100, Jindrich Makovicka wrote: > > With SDL 2.0.6, calling SDL_ShowWindow during SDL_WINDOWEVENT_HIDDEN > > blocks all subsequent display updates. > > > > Instead of trying to override the change, just update the scon->hidden > > flag. > > Has for me the side effect that sometimes I have to press ctrl-alt-2 > twice. Showing window first time works. Hiding the window works too. > Showing it the second time needs the double keypress. > > Added fprintfs to figure why [1]. Reason for that seems to be a suspious > SDL_WINDOWEVENT_SHOWN event from SDL, so qemu thinks the window is > visible even though it actually is hidden. > > Have a slightly older SDL version (2.0.3). Do you see that effect with > 2.0.6 too? Oh, and I've just seen the current code has been added as attempt to workaround that bug, see commit d3f3a0f453ea590be529079ae214c200bb5ecc1a. Hmm. Seems we are trading one issue for another here, due to bugs in SDL :( cheers, Gerd
Re: [Qemu-devel] [PATCH v7 15/20] hw/arm/sysbus-fdt: Pass the VirtMachineState to the node creation functions
Hi Peter,
On 09/10/2017 19:47, Peter Maydell wrote:
> On 1 September 2017 at 18:21, Eric Auger wrote:
>> The VirtMachineState contains some dt phandles that will be used
>> in some node creation functions. For instance we plan to use the
>> PCI host controller phandle in the smmu node creation function. So
>> let's pass the VirtMachineState handle down to the node creation
>> functions by enhancing the involved datatypes.
>>
>> Signed-off-by: Eric Auger
>> ---
>> hw/arm/sysbus-fdt.c | 3 +++
>> hw/arm/virt.c | 1 +
>> include/hw/arm/sysbus-fdt.h | 2 ++
>> 3 files changed, 6 insertions(+)
>>
>> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
>> index d68e3dc..d92a983 100644
>> --- a/hw/arm/sysbus-fdt.c
>> +++ b/hw/arm/sysbus-fdt.c
>> @@ -36,6 +36,7 @@
>> #include "hw/vfio/vfio-platform.h"
>> #include "hw/vfio/vfio-calxeda-xgmac.h"
>> #include "hw/vfio/vfio-amd-xgbe.h"
>> +#include "hw/arm/virt.h"
>> #include "hw/arm/fdt.h"
>>
>> /*
>> @@ -47,6 +48,7 @@ typedef struct PlatformBusFDTData {
>> int irq_start; /* index of the first IRQ usable by platform bus devices
>> */
>> const char *pbus_node_name; /* name of the platform bus node */
>> PlatformBusDevice *pbus;
>> +VirtMachineState *vms;
>> } PlatformBusFDTData;
>
> sysbus-fdt isn't virt specific, so this doesn't belong here.
Correct. I plan to replace this by MachineState instead.
>
> More generally, why is sysbus-fdt involved in this at all?
> I expected that instantiating and wiring up the SMMU would
> be the job of hw/arm/virt.c, like any other device we
> might have on the board.
I wished to have the same type of option as for x86 where
"-device intel-iommu" is passed to the QEMU command line. smmuv3 device
being a SysBusDevice, a natural framework to handle its node creation
function is sysbus-fdt. Having a -device approach is practical to pass
other options to the device (this was typically the case for the
"caching-mode" option). On Intel there are caching-mode, passthrough
(pt) options.
Although the smmu caching-mode option may not survive in this form, we
can foresee other options to come. In the future we may pass the PCI bus
number we want to plug the smmu into.
Also the invocation method would be identical for virtio-iommu.
I explored the other (and simpler) alternative of passing an option to
virt machine but I think this approach is less flexible. Also as you
pointed out, by default, smmu would be on, if we stick to the existing
convention. Given the perf overhead I don't think this is what we want.
Thanks
Eric
>
> thanks
> -- PMM
>
Re: [Qemu-devel] [PATCH v13 2/6] qmp: Use ThrottleLimits structure
On 10/13/2017 4:26 PM, Eric Blake wrote:
[adding Markus, and block list]
On 10/13/2017 09:16 AM, Alberto Garcia wrote:
On Mon 02 Oct 2017 04:33:28 PM CEST, Pradeep Jagadeesh wrote:
This patch factors out code to use the ThrottleLimits
structure.
{ 'struct': 'BlockIOThrottle',
- 'data': { '*device': 'str', '*id': 'str', 'bps': 'int', 'bps_rd': 'int',
-'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
-'*bps_max': 'int', '*bps_rd_max': 'int',
-'*bps_wr_max': 'int', '*iops_max': 'int',
-'*iops_rd_max': 'int', '*iops_wr_max': 'int',
-'*bps_max_length': 'int', '*bps_rd_max_length': 'int',
-'*bps_wr_max_length': 'int', '*iops_max_length': 'int',
-'*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
-'*iops_size': 'int', '*group': 'str' } }
+ 'base': 'ThrottleLimits',
+ 'data': { '*device': 'str', '*id': 'str', '*group': 'str' } }
So BlockIOThrottle used to have parameters named bps_rd and iops_wr, and
after this patch they become bps-read and iops-write. This breaks the
API completely, as you can see if you run e.g. iotest 129:
AssertionError: failed path traversal for "return" in "{u'error': {u'class': u'GenericError',
u'desc': u"Parameter 'iops_rd' is unexpected"}}"
I just checked previous versions of the series and I see that Manos
already warned you of this in v11:
https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg04698.html
On the bright side, ThrottleLimits is marked 'since 2.11' (added in
commit 432d889e), meaning it has not yet been released, so we CAN fix
the naming in ThrottleLimits to be compatible with BlockIOThrottle if we
want to share the type, as long as we get it done before the 2.11
release. It does mean tweaking Manos' code to use compatible names
everywhere, but that may be a wise course of action (we tend to favor
'-' in new API names unless there is a strong reason to use '_'; but
sharing code for maximum back-compat would be a reason to use '_').
Could you please have a look at Manos reply and my reply also.
Please let me know what you think.
-Pradeep
Re: [Qemu-devel] [PATCH V5] hw/pci-host: Fix x86 Host Bridges 64bit PCI hole
On 11/11/2017 17:25, Marcel Apfelbaum wrote:
Currently there is no MMIO range over 4G
reserved for PCI hotplug. Since the 32bit PCI hole
depends on the number of cold-plugged PCI devices
and other factors, it is very possible is too small
to hotplug PCI devices with large BARs.
Fix it by reserving 2G for I4400FX chipset
in order to comply with older Win32 Guest OSes
and 32G for Q35 chipset.
Even if the new defaults of pci-hole64-size will appear in
"info qtree" also for older machines, the property was
not implemented so no changes will be visible to guests.
Note this is a regression since prev QEMU versions had
some range reserved for 64bit PCI hotplug.
Reviewed-by: Laszlo Ersek
Reviewed-by: Gerd Hoffmann
Signed-off-by: Marcel Apfelbaum
---
Hi Michael,
Can you please merge the patch for QEMU 2.11
if you have no further comments?
I think is an important fix and it worth
having it in 2.11 .
Thanks,
Marcel
V4 -> V5:
- Renamed a local variable (Laszlo)
- Added a comment to q35 props (Eduardo)
V3 -> V4:
- Addressed Laszlo's comments:
- Added defines for pci-hole64 default size props.
- Rounded the hole64_end to 1G
- Moved some info to commit message
- Addressed Michael's comments:
- Added more comments.
- I kept Gerd's "review-by" tag since no functional changes were made.
V2 -> V3:
- Addressed Gerd's and others comments and re-enabled the pci-hole64-size
property defaulting it to 2G for I440FX and 32g for Q35.
- Even if the new defaults of pci-hole64-size will appear in "info qtree"
also for older machines, the property was not implemented so
no changes will be visible to guests.
V1 -> V2:
Addressed Igor's comments:
- aligned the hole64 start to 1Gb
(I think all the computations took care of it already,
but it can't hurt)
- Init compat props to "off" instead of "false"
hw/i386/pc.c | 22 ++
hw/pci-host/piix.c| 32 ++--
hw/pci-host/q35.c | 42 +++---
include/hw/i386/pc.h | 10 +-
include/hw/pci-host/q35.h | 1 +
5 files changed, 101 insertions(+), 6 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e11a65b545..fafe5ba5cd 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1448,6 +1448,28 @@ void pc_memory_init(PCMachineState *pcms,
pcms->ioapic_as = &address_space_memory;
}
+/*
+ * The 64bit pci hole starts after "above 4G RAM" and
+ * potentially the space reserved for memory hotplug.
+ */
+uint64_t pc_pci_hole64_start(void)
+{
+PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
+PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
+uint64_t hole64_start = 0;
+
+if (pcmc->has_reserved_memory && pcms->hotplug_memory.base) {
+hole64_start = pcms->hotplug_memory.base;
+if (!pcmc->broken_reserved_end) {
+hole64_start += memory_region_size(&pcms->hotplug_memory.mr);
+}
+} else {
+hole64_start = 0x1ULL + pcms->above_4g_mem_size;
+}
+
+return ROUND_UP(hole64_start, 1ULL << 30);
+}
+
qemu_irq pc_allocate_cpu_irq(void)
{
return qemu_allocate_irq(pic_irq_request, NULL, 0);
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index a7e2256870..a684a7cca9 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -50,6 +50,7 @@ typedef struct I440FXState {
PCIHostState parent_obj;
Range pci_hole;
uint64_t pci_hole64_size;
+bool pci_hole64_fix;
uint32_t short_root_bus;
} I440FXState;
@@ -112,6 +113,9 @@ struct PCII440FXState {
#define I440FX_PAM_SIZE 7
#define I440FX_SMRAM0x72
+/* Keep it 2G to comply with older win32 guests */
+#define I440FX_PCI_HOST_HOLE64_SIZE_DEFAULT (1ULL << 31)
+
/* Older coreboot versions (4.0 and older) read a config register that doesn't
* exist in real hardware, to get the RAM size from QEMU.
*/
@@ -238,29 +242,52 @@ static void i440fx_pcihost_get_pci_hole_end(Object *obj,
Visitor *v,
visit_type_uint32(v, name, &value, errp);
}
+/*
+ * The 64bit PCI hole start is set by the Guest firmware
+ * as the address of the first 64bit PCI MEM resource.
+ * If no PCI device has resources on the 64bit area,
+ * the 64bit PCI hole will start after "over 4G RAM" and the
+ * reserved space for memory hotplug if any.
+ */
static void i440fx_pcihost_get_pci_hole64_start(Object *obj, Visitor *v,
const char *name,
void *opaque, Error **errp)
{
PCIHostState *h = PCI_HOST_BRIDGE(obj);
+I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj);
Range w64;
uint64_t value;
pci_bus_get_w64_range(h->bus, &w64);
value = range_is_empty(&w64) ? 0 : range_lob(&w64);
+if (!value && s->pci_hole64_fix) {
+value = pc_pci_hole64_start();
+}
visit_type_uint64(v, name, &value, errp);
Re: [Qemu-devel] [PATCH v7 15/20] hw/arm/sysbus-fdt: Pass the VirtMachineState to the node creation functions
On 13 November 2017 at 13:00, Auger Eric wrote: > On 09/10/2017 19:47, Peter Maydell wrote: >> More generally, why is sysbus-fdt involved in this at all? >> I expected that instantiating and wiring up the SMMU would >> be the job of hw/arm/virt.c, like any other device we >> might have on the board. > I wished to have the same type of option as for x86 where > "-device intel-iommu" is passed to the QEMU command line. smmuv3 device > being a SysBusDevice, a natural framework to handle its node creation > function is sysbus-fdt. Having a -device approach is practical to pass > other options to the device (this was typically the case for the > "caching-mode" option). On Intel there are caching-mode, passthrough > (pt) options. Not being able to conveniently wire up a sysbus device on the command line or pass it options are general problems. I don't think the SMMU is a special case that should work around these general issues by being created in a different way to everything else. If the "hard to pass options to the device" problem needs solving (which it does anyway if we want to drop '-net' for configuring embedded ethernet devices) we should solve it, not have some small set of sysbus devices be weirdly magic. thanks -- PMM
Re: [Qemu-devel] [virtio-dev] Re: [v21 1/2] virtio-crypto: Add virtio crypto device specification
On 11/13/2017 08:17 AM, Gonglei (Arei) wrote:
>>> +struct virtio_crypto_cipher_session_req {
>>> +/* Device-readable part */
>>> +struct virtio_crypto_cipher_session_para para;
>>> +/* The cipher key */
>>> +u8 cipher_key[keylen];
>>> +
>> Is there a limit to the size of chiper_key. I don't see one in your
>> kernel code. OTOH given that virtio_crypto_sym_create_session_req
>> is one flavor of virtio_crypto_op_ctrl_req.additional_para and that
>> the later is 56 bytes in case no mux mode is supported, I think
>> there must be a limit to the size of cipher_key!
>>
> Of course the size of cipher_key is limited, firstly the max length is defined
> in virtio crypto's configuration, see
>
> struct virtio_crypto_config {
> ... ...
> /* Maximum length of cipher key */
> uint32_t max_cipher_key_len;
> ... ...
> };
>
So for the current qemu implementation it's 64 bytes.
> Secondly the real cipher_key size for a specific request, is in struct
> virtio_crypto_cipher_session_para,
>
> struct virtio_crypto_cipher_session_para {
>... ...
> /* length of key */
> le32 keylen;
>... ...
> };
>
> That means a size of cipher_key is variable, which is assigned in each
> request.
Of course I understood that. There are two problems I was trying
to point out, and you ignored both.
1. The more important one I was explicit about. Sadly you ignored
that part of my mail. I will mark it as *Problem 1* down below.
2. If there is a limit to the size, then there should be a driver
normative statement ("Driver Requirements") that states this limit
MUST be respected. I didn't find this statement.
>
>> Please explain!
>>
>> Looking at the kernel code again, it seems to me that chiper_key
>> starts at offset 72 == sizeof(struct virtio_crypto_op_ctrl_req)
>> where struct virtio_crypto_op_ctrl_req is defined in
>> include/uapi/linux/virtio_crypto.h. That would mean that this
>> guy is *not a part of* virtio_crypto_op_ctrl_req but comes
>> after it and is of variable size.
*Problem 1*
Now consider the part where the whole request is described
"""
+The controlq request is composed of two parts:
+\begin{lstlisting}
+struct virtio_crypto_op_ctrl_req {
+struct virtio_crypto_ctrl_header header;
+
+/* additional paramenter */
+u8 additional_para[addl_para_len];
+};
+\end{lstlisting}
+
+The first is a general header (see above). And the second one, additional
+paramenter, contains an crypto-service-specific structure, which could be one
+of the following types:
+\begin{itemize*}
+\item struct virtio_crypto_sym_create_session_req
+\item struct virtio_crypto_hash_create_session_req
+\item struct virtio_crypto_mac_create_session_req
+\item struct virtio_crypto_aead_create_session_req
+\item virtio_crypto_destroy_session_req
+\end{itemize*}
+
+The size of the additional paramenter depends on the VIRTIO_CRYPTO_F_MUX_MODE
+feature bit:
+\item If the VIRTIO_CRYPTO_F_MUX_MODE feature bit is NOT negotiated, the
+size of additional paramenter is fixed to 56 bytes, the data of the unused
+part (if has) will be ingored.
+\item If the VIRTIO_CRYPTO_F_MUX_MODE feature bit is negotiated, the size of
+additional paramenter is flexible, which is the same as the
crypto-service-specific
+structure used.
"""
There it's said that the whole request is header + additional_para and that
if VIRTIO_CRYPTO_F_MUX_MODE feature bit is NOT negotiated additional para
is 56 bytes. Let's assume the key is part of the additional parameter.
But you can't put 64 bytes into 56 bytes. So as I say above
*the key is not part of virtio_crypto_op_ctrl_req* neiter as described
in this spec nor as defined in uapi/linux/virtio_crypto.h. That means
the communication protocol description (more precisely the message format
description) in the spec is broken. QED
In my opinion this is a big issue.
>>
>>> +/* Device-writable part */
>>
>> Now I'm interested on what 'offset' does the device writable
>> part start.
>>
>> Of course technically we don't need to know this, because we
>> have a device-read-only or device-write-only indication on each
>> descriptor. So virtio_crypto_session_input starts with the first
>> device write only descriptor.
>>
> You are right. We definitely don't need to know this. Pls see Qemu code:
> virtio_crypto_handle_request():
>
> /* We always touch the last byte, so just see how big in_iov is. */
> request->in_len = iov_size(in_iov, in_num);
> request->in = (void *)in_iov[in_num - 1].iov_base
> + in_iov[in_num - 1].iov_len
> - sizeof(struct virtio_crypto_inhdr);
> iov_discard_back(in_iov, &in_num, sizeof(struct virtio_crypto_inhdr));
>
> The above in_iov means device-writable iov.
>
I've seen that code. Thanks.
>>> +struct virtio_crypto_session_input input;
>>> +};
>>> +\end{lstlisting}
>>> +
>>> +Algorithm chaining requests are as follows:
>>> +
>>> +\begin{lstlisting}
>>> +struct virtio_crypto_alg_chain_session_para {
>>> +
Re: [Qemu-devel] [PULL for-2.11 0/2] s390x changes for 2.11-rc1
On 9 November 2017 at 15:32, Cornelia Huck wrote: > The following changes since commit b0fbe46ad82982b289a44ee2495b59b0bad8a842: > > Update version for v2.11.0-rc0 release (2017-11-07 16:05:28 +) > > are available in the git repository at: > > git://github.com/cohuck/qemu tags/s390x-20171109 > > for you to fetch changes up to fdaae351435147b9be6161d0f136ca7c40308059: > > target/s390x: Finish implementing RISBGN (2017-11-09 10:36:06 +0100) > > > s390x changes: let pci devices start out in a usable state, and make > RISBGN work in tcg. > > > > Christian Borntraeger (1): > s390x/pci: let pci devices start in configured mode > > Richard Henderson (1): > target/s390x: Finish implementing RISBGN > > hw/s390x/s390-pci-bus.c | 2 +- > target/s390x/translate.c | 9 +++-- > 2 files changed, 4 insertions(+), 7 deletions(-) Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH 1/8] sdl2: Fix broken display updating after the window is hidden
On Mon, Nov 13, 2017 at 1:59 PM, Gerd Hoffmann wrote: > On Mon, Nov 13, 2017 at 01:39:14PM +0100, Gerd Hoffmann wrote: >> On Sun, Nov 12, 2017 at 08:30:25PM +0100, Jindrich Makovicka wrote: >> > With SDL 2.0.6, calling SDL_ShowWindow during SDL_WINDOWEVENT_HIDDEN >> > blocks all subsequent display updates. >> > >> > Instead of trying to override the change, just update the scon->hidden >> > flag. >> >> Has for me the side effect that sometimes I have to press ctrl-alt-2 >> twice. Showing window first time works. Hiding the window works too. >> Showing it the second time needs the double keypress. >> >> Added fprintfs to figure why [1]. Reason for that seems to be a suspious >> SDL_WINDOWEVENT_SHOWN event from SDL, so qemu thinks the window is >> visible even though it actually is hidden. >> >> Have a slightly older SDL version (2.0.3). Do you see that effect with >> 2.0.6 too? > > Oh, and I've just seen the current code has been added as attempt to > workaround that bug, see commit d3f3a0f453ea590be529079ae214c200bb5ecc1a. > > Hmm. Seems we are trading one issue for another here, due to bugs in > SDL :( Does this happen for you with the whole series applied, or just the 1st patch? Especially [PATCH 8/8] sdl2: Ignore UI hotkeys after a focus change when GUI modifier is held can have an impact on the behavior you described. Regards, -- Jindřich Makovička
Re: [Qemu-devel] [PATCH 4/8] sdl2: Do not hide the cursor on auxilliary windows
Fails scripts/checkpatch.pl (trailing whitespace on several lines).
Re: [Qemu-devel] [PATCH 5/8] sdl2 uses surface relative coordinates
On Sun, Nov 12, 2017 at 08:30:29PM +0100, Jindrich Makovicka wrote: > This patch fixes mouse positioning with -device usb-tablet and fullscreen > or resized window. Fails checkpatch too (long lines). Also: can you add a "Fixes: 46522a82236ea0cf9011b89896d2d8f8ddaf2443" line (that is the commit which added the bug). thanks, Gerd
[Qemu-devel] [Bug 1563887] Re: qemu-system-ppc64 freezes on starting image on ppc64le
Thanks for the Detail Robin! -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1563887 Title: qemu-system-ppc64 freezes on starting image on ppc64le Status in QEMU: Invalid Status in linux package in Ubuntu: Invalid Status in livecd-rootfs package in Ubuntu: Invalid Status in qemu package in Ubuntu: Invalid Bug description: qemu-system-ppc64 running on Ubuntu 16.04 beta-2 fails to start an image as part of the certification process. This on an IBM ppc64le in PowerVM mode running Ubuntu 16.04 beta-2 deployed by MAAS 1.9.1. There is no error output. ubuntu@alpine01:~/kvm$ qemu-system-ppc64 -m 256 -display none -nographic -net nic -net user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine pseries -drive file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive file=seed.iso,if=virtio WARNING: Image format was not specified for 'seed.iso' and probing guessed raw. Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted. Specify the 'raw' format explicitly to remove the restrictions. SLOF ** QEMU Starting Build Date = Jan 29 2016 18:58:37 FW Version = buildd@ release 20151103 Press "s" to enter Open Firmware. Populating /vdevice methods Populating /vdevice/vty@7100 Populating /vdevice/nvram@7101 Populating /vdevice/l-lan@7102 Populating /vdevice/v-scsi@7103 SCSI: Looking for devices 8200 CD-ROM : "QEMU QEMU CD-ROM 2.5+" Populating /pci@8002000 00 1800 (D) : 1af4 1001virtio [ block ] 00 1000 (D) : 1af4 1001virtio [ block ] 00 0800 (D) : 106b 003fserial bus [ usb-ohci ] 00 (D) : 1234 qemu vga No NVRAM common partition, re-initializing... Installing QEMU fb Scanning USB OHCI: initializing USB Keyboard USB mouse No console specified using screen & keyboard Welcome to Open Firmware Copyright (c) 2004, 2011 IBM Corporation All rights reserved. This program and the accompanying materials are made available under the terms of the BSD License available at http://www.opensource.org/licenses/bsd-license.php Trying to load: from: /pci@8002000/scsi@3 ... E3404: Not a bootable device! Trying to load: from: /pci@8002000/scsi@2 ... Successfully loaded Linux ppc64le #31-Ubuntu SMP F ProblemType: Bug DistroRelease: Ubuntu 16.04 Package: qemu-system-ppc 1:2.5+dfsg-5ubuntu6 ProcVersionSignature: Ubuntu 4.4.0-16.32-generic 4.4.6 Uname: Linux 4.4.0-16-generic ppc64le ApportVersion: 2.20-0ubuntu3 Architecture: ppc64el Date: Wed Mar 30 14:10:01 2016 KvmCmdLine: COMMAND STAT EUID RUID PID PPID %CPU COMMAND kvm-irqfd-clean S< 0 0 1172 2 0.0 [kvm-irqfd-clean] qemu-nbdSsl 0 0 13467 1 0.0 qemu-nbd -c /dev/nbd0 xenial-server-cloudimg-ppc64el-disk1.img qemu-system-ppc Sl+ 1000 1000 18973 18896 101 qemu-system-ppc64 -m 256 -display none -nographic -net nic -net user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine pseries -drive file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive file=seed.iso,if=virtio Lsusb: Error: command ['lsusb'] failed with exit code 1: ProcEnviron: TERM=xterm PATH=(custom, no user) LANG=en_US.UTF-8 SHELL=/bin/bash ProcKernelCmdLine: BOOT_IMAGE=/boot/vmlinux-4.4.0-16-generic root=UUID=92d820c8-ab25-497b-9b1e-f1435992bbf3 ro ProcLoadAvg: 1.08 0.94 0.58 2/616 19571 ProcLocks: 1: POSIX ADVISORY WRITE 886 00:13:381 0 EOF 2: POSIX ADVISORY WRITE 1339 00:13:528 0 EOF 3: FLOCK ADVISORY WRITE 1284 00:13:522 0 EOF 4: POSIX ADVISORY WRITE 2281 00:13:563 0 EOF 5: POSIX ADVISORY WRITE 1331 00:13:536 0 EOF ProcSwaps: Filename TypeSizeUsedPriority /swap.img file 8388544 0 -1 ProcVersion: Linux version 4.4.0-16-generic (buildd@bos01-ppc64el-001) (gcc version 5.3.1 20160320 (Ubuntu/Linaro/IBM 5.3.1-12ubuntu4) ) #32-Ubuntu SMP Thu Mar 24 22:31:14 UTC 2016 SourcePackage: qemu UpgradeStatus: No upgrade log present (probably fresh install) bootlist: /pci@8002011/pci1014,034A@0/sas/disk@4068402c40 /pci@8002018/ethernet@0:speed=auto,duplex=auto,csarch,000.000.000.000,,000.000.000.000,000.000.000.000,5,5,000.000.000.000,512 /pci@8002018/ethernet@0,1:speed=auto,duplex=auto,csarch,000.000.000.000,,000.000.000.000,000.000.000.000,5,5,000.000.000.000,512 /pci@8002018/ethernet@0,2:speed=auto,duplex=auto,csarch,000.000.000.000,,000.000.000.000,000.000.000.000,5,5,000.000.
Re: [Qemu-devel] [PATCH v2 1/2] accel/tcg/translate-all: expand cpu_restore_state addr check
On 8 November 2017 at 15:32, Alex Bennée wrote: > We are still seeing signals during translation time when we walk over > a page protection boundary. This expands the check to ensure the host > PC is inside the code generation buffer. The original suggestion was > to check versus tcg_ctx.code_gen_ptr but as we now segment the > translation buffer we have to settle for just a general check for > being inside. > > I've also fixed up the declaration to make it clear it can deal with > invalid addresses. A later patch will fix up the call sites. > > Signed-off-by: Alex Bennée > Reported-by: Peter Maydell > Suggested-by: Paolo Bonzini > Cc: Richard Henderson Thanks; this fixes my test case. Patch 2 is just cleanup and looks like it needs rework, so I'm taking patch 1 into target-arm to put into master for rc1. thanks -- PMM
Re: [Qemu-devel] [PATCH] hw/arm/aspeed: Unlock SCU when running kernel
On Thu, Nov 9, 2017 at 5:54 PM, Cédric Le Goater wrote: > On 11/08/2017 04:24 AM, Joel Stanley wrote: >> --- a/hw/misc/aspeed_sdmc.c >> +++ b/hw/misc/aspeed_sdmc.c >> @@ -18,7 +18,6 @@ >> >> /* Protection Key Register */ >> #define R_PROT(0x00 / 4) >> -#define PROT_KEY_UNLOCK 0xFC600309 > > why are you using the memory controller magic value ? Oh. My bad. I think both ctags and myself became confused :) >> --- a/include/hw/misc/aspeed_sdmc.h >> +++ b/include/hw/misc/aspeed_sdmc.h >> @@ -16,6 +16,8 @@ >> >> #define ASPEED_SDMC_NR_REGS (0x8 >> 2) >> >> +#define PROT_KEY_UNLOCK 0xFC600309 > > Ditto. Should we be using the one from SCU ? > > Now that we are externalizing this value, adding a prefix would be > preferable : ASPEED_SCU_ ? I will grab the correct value from the SCU and rename it ASPEED_SCU_PROT_KEY for v2. Thanks for taking a look. Cheers, Joel
[Qemu-devel] [PATCH v2] hw/arm/aspeed: Unlock SCU when running kernel
The ASPEED hardware contains a lock register for the SCU that disables
any writes to the SCU when it is locked. The machine comes up with the
lock enabled, but on all known hardware u-boot will unlock it and leave
it unlocked when loading the kernel.
This means the kernel expects the SCU to be unlocked. When booting from
an emulated ROM the normal u-boot unlock path is executed. Things don't
go well when booting using the -kernel command line, as u-boot does not
run first.
Change behaviour so that when a kernel is passed to the machine, set the
reset value of the SCU to be unlocked.
Signed-off-by: Joel Stanley
---
v2:
- use the correct value for the key
- rename it ASPEED_SCU_PROT_KEY
---
hw/arm/aspeed.c | 10 ++
hw/arm/aspeed_soc.c | 2 ++
hw/misc/aspeed_scu.c | 5 +++--
include/hw/misc/aspeed_scu.h | 3 +++
4 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index ab895ad490af..754df8403273 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -36,6 +36,7 @@ typedef struct AspeedBoardState {
typedef struct AspeedBoardConfig {
const char *soc_name;
uint32_t hw_strap1;
+uint32_t hw_prot_key;
const char *fmc_model;
const char *spi_model;
uint32_t num_cs;
@@ -186,6 +187,15 @@ static void aspeed_board_init(MachineState *machine,
&error_abort);
object_property_set_int(OBJECT(&bmc->soc), cfg->num_cs, "num-cs",
&error_abort);
+if (machine->kernel_filename) {
+/*
+ * When booting with a -kernel command line there is no u-boot
+ * that runs to unlock the SCU. In this case set the default to
+ * be unlocked as the kernel expects
+ */
+object_property_set_int(OBJECT(&bmc->soc), ASPEED_SCU_PROT_KEY,
+"hw-prot-key", &error_abort);
+}
object_property_set_bool(OBJECT(&bmc->soc), true, "realized",
&error_abort);
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 5aa3d2ddd9cd..c83b7e207b86 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -154,6 +154,8 @@ static void aspeed_soc_init(Object *obj)
"hw-strap1", &error_abort);
object_property_add_alias(obj, "hw-strap2", OBJECT(&s->scu),
"hw-strap2", &error_abort);
+object_property_add_alias(obj, "hw-prot-key", OBJECT(&s->scu),
+ "hw-prot-key", &error_abort);
object_initialize(&s->fmc, sizeof(s->fmc), sc->info->fmc_typename);
object_property_add_child(obj, "fmc", OBJECT(&s->fmc), NULL);
diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index 95022d3607ad..74537ce9755d 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -85,7 +85,6 @@
#define BMC_REV TO_REG(0x19C)
#define BMC_DEV_ID TO_REG(0x1A4)
-#define PROT_KEY_UNLOCK 0x1688A8A8
#define SCU_IO_REGION_SIZE 0x1000
static const uint32_t ast2400_a0_resets[ASPEED_SCU_NR_REGS] = {
@@ -192,7 +191,7 @@ static void aspeed_scu_write(void *opaque, hwaddr offset,
uint64_t data,
}
if (reg > PROT_KEY && reg < CPU2_BASE_SEG1 &&
-s->regs[PROT_KEY] != PROT_KEY_UNLOCK) {
+s->regs[PROT_KEY] != ASPEED_SCU_PROT_KEY) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: SCU is locked!\n", __func__);
return;
}
@@ -246,6 +245,7 @@ static void aspeed_scu_reset(DeviceState *dev)
s->regs[SILICON_REV] = s->silicon_rev;
s->regs[HW_STRAP1] = s->hw_strap1;
s->regs[HW_STRAP2] = s->hw_strap2;
+s->regs[PROT_KEY] = s->hw_prot_key;
}
static uint32_t aspeed_silicon_revs[] = {
@@ -299,6 +299,7 @@ static Property aspeed_scu_properties[] = {
DEFINE_PROP_UINT32("silicon-rev", AspeedSCUState, silicon_rev, 0),
DEFINE_PROP_UINT32("hw-strap1", AspeedSCUState, hw_strap1, 0),
DEFINE_PROP_UINT32("hw-strap2", AspeedSCUState, hw_strap2, 0),
+DEFINE_PROP_UINT32("hw-prot-key", AspeedSCUState, hw_prot_key, 0),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/include/hw/misc/aspeed_scu.h b/include/hw/misc/aspeed_scu.h
index bd4ac013f997..d70cc0aeca61 100644
--- a/include/hw/misc/aspeed_scu.h
+++ b/include/hw/misc/aspeed_scu.h
@@ -29,6 +29,7 @@ typedef struct AspeedSCUState {
uint32_t silicon_rev;
uint32_t hw_strap1;
uint32_t hw_strap2;
+uint32_t hw_prot_key;
} AspeedSCUState;
#define AST2400_A0_SILICON_REV 0x02000303U
@@ -38,6 +39,8 @@ typedef struct AspeedSCUState {
extern bool is_supported_silicon_rev(uint32_t silicon_rev);
+#define ASPEED_SCU_PROT_KEY 0x1688A8A8
+
/*
* Extracted from Aspeed SDK v00.03.21. Fixes and extra definitions
* were added.
--
2.14.1
Re: [Qemu-devel] [PATCH 8/8] sdl2: Ignore UI hotkeys after a focus change when GUI modifier is held
Hi,
> if (!gui_grab && (qemu_input_is_absolute() || absolute_enabled)) {
> absolute_mouse_grab(scon);
> }
Can you please add a comment here describing why this is done?
>From the code alone it isn't obvious that this is a workaround
for a SDL bug.
> +scon->ignore_hotkeys = get_mod_state();
> break;
cheers,
Gerd
[Qemu-devel] [Bug 1727737] Re: qemu-arm stalls on a GCC sanitizer test since qemu-2.7
I looked a bit more at the sanitizers source code, to understand the
differences between arm and aarch64. And it turns out that on aarch64,
we have:
sanitizer_common/sanitizer_syscall_linux_aarch64.inc:
133 // Helper function used to avoid cobbler errno.
134 bool internal_iserror(uptr retval, int *rverrno) {
135 if (retval >= (uptr)-4095) {
but on arm, in the GCC version, we use:
sanitizer_common/sanitizer_syscall_generic.inc:
54 bool internal_iserror(uptr retval, int *rverrno) {
55if (retval == (uptr)-1) {
But recently (Nov 8th), the upstream sanitizer repo got a new file:
sanitizer_common/sanitizer_syscall_linux_arm.inc
133 // Helper function used to avoid cobbler errno.
134 bool internal_iserror(uptr retval, int *rverrno) {
135 if (retval >= (uptr)-4095) {
With that change, I now observe the same behaviour with qemu-aarch64 and
qemu-arm.
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1727737
Title:
qemu-arm stalls on a GCC sanitizer test since qemu-2.7
Status in QEMU:
Incomplete
Bug description:
Hi,
I have noticed that several GCC/sanitizer tests fail with timeout when
executed under QEMU.
After a bit of investigation, I have noticed that this worked with
qemu-2.7, and started failing with qemu-2.8, and still fails with
qemu-2.10.1
I'm attaching a tarball containing:
alloca_instruments_all_paddings.exe : the testcase, and the needed libs:
lib/librt.so.1
lib/libdl.so.2
lib/ld-linux-armhf.so.3
lib/libasan.so.5
lib/libc.so.6
lib/libgcc_s.so.1
lib/libpthread.so.0
lib/libm.so.6
To reproduce the problem:
$ qemu-arm -cpu any -R 0 -L $PWD $PWD/alloca_instruments_all_paddings.exe
returns in less than a second with qemu-2.7, and never with qemu-2.8
Using -d in_asm suggests that the program "almost" completes and qemu seems
to stall on:
0x40b6eb44: e08f4004 add r4, pc, r4
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1727737/+subscriptions
[Qemu-devel] [Bug 1727737] Re: qemu-arm stalls on a GCC sanitizer test since qemu-2.7
I also looked at QEMU's code, and I am suprised that do_syscall() returns the value of errno rather than the return code from the syscall. So for instance, if clone() fails, do_syscall() returns get_errno(do_fork(...)) instead of -1. I thought the target code expects -1 in case of failure, but I'm not familiar with QEMU sources, so I'm probably missing something. Looking at QEMU's linux-user/syscall.c:do_fork(), I noticed several places with return -TARGET_E: should this be: errno = TARGET_EXXX; return -1; instead? But given than most (if not all) syscalls in do_syscall actually use 'ret = get_errno()' I must be wrong :-) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1727737 Title: qemu-arm stalls on a GCC sanitizer test since qemu-2.7 Status in QEMU: Incomplete Bug description: Hi, I have noticed that several GCC/sanitizer tests fail with timeout when executed under QEMU. After a bit of investigation, I have noticed that this worked with qemu-2.7, and started failing with qemu-2.8, and still fails with qemu-2.10.1 I'm attaching a tarball containing: alloca_instruments_all_paddings.exe : the testcase, and the needed libs: lib/librt.so.1 lib/libdl.so.2 lib/ld-linux-armhf.so.3 lib/libasan.so.5 lib/libc.so.6 lib/libgcc_s.so.1 lib/libpthread.so.0 lib/libm.so.6 To reproduce the problem: $ qemu-arm -cpu any -R 0 -L $PWD $PWD/alloca_instruments_all_paddings.exe returns in less than a second with qemu-2.7, and never with qemu-2.8 Using -d in_asm suggests that the program "almost" completes and qemu seems to stall on: 0x40b6eb44: e08f4004 add r4, pc, r4 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1727737/+subscriptions
Re: [Qemu-devel] [PATCH v3] SDL2 various fixes
On Sun, Nov 12, 2017 at 08:30:24PM +0100, Jindrich Makovicka wrote: > Hi, > > here is an identical patchset with Signed-off-by. Please post a new version without reply-to. Queued up four patches: sdl2: Do not leave grab when fullscreen sdl2: Fix dead keyboard after fullsceen sdl2: Use the same pointer show/hide logic for absolute and relative mode sdl2: Do not quit the emulator when an auxilliary window is closed See: https://www.kraxel.org/cgit/qemu/log/?h=queue/ui The other ones need some clarification / testing / fixups, see individual replies. cheers, Gerd
Re: [Qemu-devel] [PATCH v7 15/20] hw/arm/sysbus-fdt: Pass the VirtMachineState to the node creation functions
Hi Peter, On 13/11/2017 14:08, Peter Maydell wrote: > On 13 November 2017 at 13:00, Auger Eric wrote: >> On 09/10/2017 19:47, Peter Maydell wrote: >>> More generally, why is sysbus-fdt involved in this at all? >>> I expected that instantiating and wiring up the SMMU would >>> be the job of hw/arm/virt.c, like any other device we >>> might have on the board. >> I wished to have the same type of option as for x86 where >> "-device intel-iommu" is passed to the QEMU command line. smmuv3 device >> being a SysBusDevice, a natural framework to handle its node creation >> function is sysbus-fdt. Having a -device approach is practical to pass >> other options to the device (this was typically the case for the >> "caching-mode" option). On Intel there are caching-mode, passthrough >> (pt) options. > > Not being able to conveniently wire up a sysbus device on the > command line or pass it options are general problems. I don't > think the SMMU is a special case that should work around these > general issues by being created in a different way to everything > else. If the "hard to pass options to the device" problem needs > solving (which it does anyway if we want to drop '-net' for > configuring embedded ethernet devices) we should solve it, > not have some small set of sysbus devices be weirdly magic. do you have examples of other SysbusDevices whose instantiation is made conditional with "-device" option and which address the dt node/ACPI table creation in a more standard manner? Or do you want me to drop the "-device" requirement. I may manage reaching my goal with yet another machine init done notifier that would create the dt node from virt code at fixed base address. But this solution still may be be virt specific. Is it the direction you want me to follow at the moment? Thanks Eric > > thanks > -- PMM >
Re: [Qemu-devel] [PATCH 1/8] sdl2: Fix broken display updating after the window is hidden
Hi, > >> Have a slightly older SDL version (2.0.3). Do you see that effect with > >> 2.0.6 too? > > > > Oh, and I've just seen the current code has been added as attempt to > > workaround that bug, see commit d3f3a0f453ea590be529079ae214c200bb5ecc1a. > > > > Hmm. Seems we are trading one issue for another here, due to bugs in > > SDL :( > > Does this happen for you with the whole series applied, or just the > 1st patch? Especially Same behavior here with only first patch and whole series applied. cheers, Gerd
Re: [Qemu-devel] [virtio-dev] Re: [v21 1/2] virtio-crypto: Add virtio crypto device specification
Hello Halil,
Thanks for your feedback.
>
> On 11/13/2017 08:17 AM, Gonglei (Arei) wrote:
> >>> +struct virtio_crypto_cipher_session_req {
> >>> +/* Device-readable part */
> >>> +struct virtio_crypto_cipher_session_para para;
> >>> +/* The cipher key */
> >>> +u8 cipher_key[keylen];
> >>> +
> >> Is there a limit to the size of chiper_key. I don't see one in your
> >> kernel code. OTOH given that virtio_crypto_sym_create_session_req
> >> is one flavor of virtio_crypto_op_ctrl_req.additional_para and that
> >> the later is 56 bytes in case no mux mode is supported, I think
> >> there must be a limit to the size of cipher_key!
> >>
> > Of course the size of cipher_key is limited, firstly the max length is
> > defined
> > in virtio crypto's configuration, see
> >
> > struct virtio_crypto_config {
> > ... ...
> > /* Maximum length of cipher key */
> > uint32_t max_cipher_key_len;
> > ... ...
> > };
> >
>
> So for the current qemu implementation it's 64 bytes.
>
> > Secondly the real cipher_key size for a specific request, is in struct
> virtio_crypto_cipher_session_para,
> >
> > struct virtio_crypto_cipher_session_para {
> >... ...
> > /* length of key */
> > le32 keylen;
> >... ...
> > };
> >
> > That means a size of cipher_key is variable, which is assigned in each
> > request.
>
> Of course I understood that. There are two problems I was trying
> to point out, and you ignored both.
>
> 1. The more important one I was explicit about. Sadly you ignored
> that part of my mail. I will mark it as *Problem 1* down below.
>
> 2. If there is a limit to the size, then there should be a driver
> normative statement ("Driver Requirements") that states this limit
> MUST be respected. I didn't find this statement.
We can add it.
> >
> >> Please explain!
> >>
> >> Looking at the kernel code again, it seems to me that chiper_key
> >> starts at offset 72 == sizeof(struct virtio_crypto_op_ctrl_req)
> >> where struct virtio_crypto_op_ctrl_req is defined in
> >> include/uapi/linux/virtio_crypto.h. That would mean that this
> >> guy is *not a part of* virtio_crypto_op_ctrl_req but comes
> >> after it and is of variable size.
>
> *Problem 1*
>
> Now consider the part where the whole request is described
>
> """
> +The controlq request is composed of two parts:
> +\begin{lstlisting}
> +struct virtio_crypto_op_ctrl_req {
> +struct virtio_crypto_ctrl_header header;
> +
> +/* additional paramenter */
> +u8 additional_para[addl_para_len];
> +};
> +\end{lstlisting}
> +
> +The first is a general header (see above). And the second one, additional
> +paramenter, contains an crypto-service-specific structure, which could be one
> +of the following types:
> +\begin{itemize*}
> +\item struct virtio_crypto_sym_create_session_req
> +\item struct virtio_crypto_hash_create_session_req
> +\item struct virtio_crypto_mac_create_session_req
> +\item struct virtio_crypto_aead_create_session_req
> +\item virtio_crypto_destroy_session_req
> +\end{itemize*}
> +
> +The size of the additional paramenter depends on the
> VIRTIO_CRYPTO_F_MUX_MODE
> +feature bit:
> +\item If the VIRTIO_CRYPTO_F_MUX_MODE feature bit is NOT negotiated,
> the
> +size of additional paramenter is fixed to 56 bytes, the data of the
> unused
> +part (if has) will be ingored.
> +\item If the VIRTIO_CRYPTO_F_MUX_MODE feature bit is negotiated, the
> size of
> +additional paramenter is flexible, which is the same as the
> crypto-service-specific
> +structure used.
> """
>
> There it's said that the whole request is header + additional_para and that
> if VIRTIO_CRYPTO_F_MUX_MODE feature bit is NOT negotiated additional
> para
> is 56 bytes. Let's assume the key is part of the additional parameter.
> But you can't put 64 bytes into 56 bytes. So as I say above
> *the key is not part of virtio_crypto_op_ctrl_req* neiter as described
> in this spec nor as defined in uapi/linux/virtio_crypto.h. That means
> the communication protocol description (more precisely the message format
> description) in the spec is broken. QED
>
> In my opinion this is a big issue.
>
OK, I get your point now. Sorry about that. :(
We should update the description about cipher_key and something like that.
The key is indeed not a part of virtio_crypto_op_ctrl_req in the realization, is
a separate entry in the descriptor table.
>
> >>
> >>> +/* Device-writable part */
> >>
> >> Now I'm interested on what 'offset' does the device writable
> >> part start.
> >>
> >> Of course technically we don't need to know this, because we
> >> have a device-read-only or device-write-only indication on each
> >> descriptor. So virtio_crypto_session_input starts with the first
> >> device write only descriptor.
> >>
> > You are right. We definitely don't need to know this. Pls see Qemu code:
> > virtio_crypto_handle_request():
> >
> > /* We always touch the last byte, so just see how big in_iov is. */
> > request->in_len = iov_
Re: [Qemu-devel] [PATCH v7 15/20] hw/arm/sysbus-fdt: Pass the VirtMachineState to the node creation functions
On 13 November 2017 at 13:37, Auger Eric wrote: > On 13/11/2017 14:08, Peter Maydell wrote: >> Not being able to conveniently wire up a sysbus device on the >> command line or pass it options are general problems. I don't >> think the SMMU is a special case that should work around these >> general issues by being created in a different way to everything >> else. If the "hard to pass options to the device" problem needs >> solving (which it does anyway if we want to drop '-net' for >> configuring embedded ethernet devices) we should solve it, >> not have some small set of sysbus devices be weirdly magic. > do you have examples of other SysbusDevices whose instantiation is made > conditional with "-device" option and which address the dt node/ACPI > table creation in a more standard manner? Or do you want me to drop the > "-device" requirement. I think that you just can't create sysbus devices with -device. They're hardwired into the machine model. > I may manage reaching my goal with yet another machine init done > notifier that would create the dt node from virt code at fixed base > address. But this solution still may be be virt specific. Is it the > direction you want me to follow at the moment? I still don't really understand why the SMMU has to be any different from the UART, or the PCI controller, or any of the other devices in the virt board model. None of those try to be creatable from the command line. thanks -- PMM
[Qemu-devel] [Bug 1727737] Re: qemu-arm stalls on a GCC sanitizer test since qemu-2.7
Hmm, the do_fork() code is a bit inconsistent there. Generally in linux-user/ functions should either: (1) return -1 with host errno set to a host errno; the caller then must use get_errno() to convert to the negative-target-errno that we need to return from do_syscall() (2) return negative-target-errno; the caller then need do nothing In this case do_fork() is supposed to be using approach 2, but some code paths are using approach 1 and the callers are all using get_errno(). This hybrid approach works OK as long as none of the negative-target- errno values returned are -1 (which happens to be TARGET_EPERM for all architectures, and which we only use once in linux-user, in the sigaltstack handling). In an ideal world we'd clean this up to consistently use approach 2, but I don't think the code as it stands is actually buggy. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1727737 Title: qemu-arm stalls on a GCC sanitizer test since qemu-2.7 Status in QEMU: Incomplete Bug description: Hi, I have noticed that several GCC/sanitizer tests fail with timeout when executed under QEMU. After a bit of investigation, I have noticed that this worked with qemu-2.7, and started failing with qemu-2.8, and still fails with qemu-2.10.1 I'm attaching a tarball containing: alloca_instruments_all_paddings.exe : the testcase, and the needed libs: lib/librt.so.1 lib/libdl.so.2 lib/ld-linux-armhf.so.3 lib/libasan.so.5 lib/libc.so.6 lib/libgcc_s.so.1 lib/libpthread.so.0 lib/libm.so.6 To reproduce the problem: $ qemu-arm -cpu any -R 0 -L $PWD $PWD/alloca_instruments_all_paddings.exe returns in less than a second with qemu-2.7, and never with qemu-2.8 Using -d in_asm suggests that the program "almost" completes and qemu seems to stall on: 0x40b6eb44: e08f4004 add r4, pc, r4 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1727737/+subscriptions
Re: [Qemu-devel] [PULL 0/8] NBD patches for 2.11-rc1
On 9 November 2017 at 16:59, Eric Blake wrote: > The following changes since commit b0fbe46ad82982b289a44ee2495b59b0bad8a842: > > Update version for v2.11.0-rc0 release (2017-11-07 16:05:28 +) > > are available in the git repository at: > > git://repo.or.cz/qemu/ericb.git tags/pull-nbd-2017-11-09 > > for you to fetch changes up to ef8c887ee01a4e4c8c5c28c86ea5b45162c51bcd: > > nbd/server: Fix structured read of length 0 (2017-11-09 10:25:11 -0600) > > > nbd patches for 2017-11-09 > > - Vladimir Sementsov-Ogievskiy: nbd/server: fix nbd_negotiate_handle_info > - Eric Blake: 0/7 various NBD fixes for 2.11 > > > Eric Blake (7): > nbd-client: Fix error message typos > nbd-client: Refuse read-only client with BDRV_O_RDWR > nbd/client: Nicer trace of structured reply > nbd: Fix struct name for structured reads > nbd-client: Short-circuit 0-length operations > nbd-client: Stricter enforcing of structured reply spec > nbd/server: Fix structured read of length 0 > > Vladimir Sementsov-Ogievskiy (1): > nbd/server: fix nbd_negotiate_handle_info > Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH v7 15/20] hw/arm/sysbus-fdt: Pass the VirtMachineState to the node creation functions
Hi, On 13/11/2017 14:44, Peter Maydell wrote: > On 13 November 2017 at 13:37, Auger Eric wrote: >> On 13/11/2017 14:08, Peter Maydell wrote: >>> Not being able to conveniently wire up a sysbus device on the >>> command line or pass it options are general problems. I don't >>> think the SMMU is a special case that should work around these >>> general issues by being created in a different way to everything >>> else. If the "hard to pass options to the device" problem needs >>> solving (which it does anyway if we want to drop '-net' for >>> configuring embedded ethernet devices) we should solve it, >>> not have some small set of sysbus devices be weirdly magic. >> do you have examples of other SysbusDevices whose instantiation is made >> conditional with "-device" option and which address the dt node/ACPI >> table creation in a more standard manner? Or do you want me to drop the >> "-device" requirement. > > I think that you just can't create sysbus devices with -device. > They're hardwired into the machine model. OK > >> I may manage reaching my goal with yet another machine init done >> notifier that would create the dt node from virt code at fixed base >> address. But this solution still may be be virt specific. Is it the >> direction you want me to follow at the moment? > > I still don't really understand why the SMMU has to be any > different from the UART, or the PCI controller, or any of > the other devices in the virt board model. None of those > try to be creatable from the command line. To me the difference is, if you don't want to use UART or PCI controller, you don't suffer performance downgrade if they are instantiated. On the opposite, if the SMMU is instantiated whereas you don't need his functionality you suffer performance downgrade. Thanks Eric > > thanks > -- PMM >
Re: [Qemu-devel] [PATCH v3 2/2] qdev: Check for the availability of a hotplug controller before adding a device
Thomas Huth writes:
> The qdev_unplug() function contains a g_assert(hotplug_ctrl) statement,
> so QEMU crashes when the user tries to device_add + device_del a device
> that does not have a corresponding hotplug controller. This could be
> provoked for a couple of devices in the past (see commit 4c93950659487c7ad
> or 84ebd3e8c7d4fe955 for example), and can currently for example also be
> triggered like this:
>
> $ s390x-softmmu/qemu-system-s390x -M none -nographic
> QEMU 2.10.50 monitor - type 'help' for more information
> (qemu) device_add qemu-s390x-cpu,id=x
> (qemu) device_del x
> **
> ERROR:qemu/qdev-monitor.c:872:qdev_unplug: assertion failed: (hotplug_ctrl)
> Aborted (core dumped)
>
> So devices clearly need a hotplug controller when they should be usable
> with device_add.
> The code in qdev_device_add() already checks whether the bus has a proper
> hotplug controller,
Where? Hmm, I guess it's this one:
if (qdev_hotplug && bus && !qbus_is_hotpluggable(bus)) {
error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name);
return NULL;
}
> but for devices that do not have a corresponding bus,
> there is no appropriate check available yet. In that case we should check
> whether the machine itself provides a suitable hotplug controller and
> refuse to plug the device if none is available.
>
> Reviewed-by: Igor Mammedov
> Signed-off-by: Thomas Huth
> ---
> hw/core/qdev.c | 28
> include/hw/qdev-core.h | 1 +
> qdev-monitor.c | 5 +
> 3 files changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 295..f739753 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -253,19 +253,31 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int
> alias_id,
> dev->alias_required_for_version = required_for_version;
> }
>
> +HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev)
> +{
> +MachineState *machine;
> +MachineClass *mc;
> +Object *m_obj = qdev_get_machine();
> +
> +if (object_dynamic_cast(m_obj, TYPE_MACHINE)) {
> +machine = MACHINE(m_obj);
> +mc = MACHINE_GET_CLASS(machine);
> +if (mc->get_hotplug_handler) {
> +return mc->get_hotplug_handler(machine, dev);
> +}
> +}
> +
> +return NULL;
> +}
> +
> HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
> {
> -HotplugHandler *hotplug_ctrl = NULL;
> +HotplugHandler *hotplug_ctrl;
>
> if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
> hotplug_ctrl = dev->parent_bus->hotplug_handler;
> -} else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) {
> -MachineState *machine = MACHINE(qdev_get_machine());
> -MachineClass *mc = MACHINE_GET_CLASS(machine);
> -
> -if (mc->get_hotplug_handler) {
> -hotplug_ctrl = mc->get_hotplug_handler(machine, dev);
> -}
> +} else {
> +hotplug_ctrl = qdev_get_machine_hotplug_handler(dev);
> }
> return hotplug_ctrl;
> }
qdev_get_machine_hotplug_handler() factored out of
qdev_get_hotplug_handler(). Okay. Announcing it in the commit message
could've saved me a few review brainwaves.
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 0a71bf8..51473ee 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -286,6 +286,7 @@ DeviceState *qdev_try_create(BusState *bus, const char
> *name);
> void qdev_init_nofail(DeviceState *dev);
> void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
> int required_for_version);
> +HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev);
> HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev);
> void qdev_unplug(DeviceState *dev, Error **errp);
> void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev,
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index 9188d20..38c0fc2 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -614,6 +614,11 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error
> **errp)
>
if (qdev_hotplug && bus && !qbus_is_hotpluggable(bus)) {
error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name);
return NULL;
}
if (!migration_is_idle()) {
error_setg(errp, "device_add not allowed while migrating");
return NULL;
}
/* create device */
dev = DEVICE(object_new(driver));
> if (bus) {
> qdev_set_parent_bus(dev, bus);
> +} else if (qdev_hotplug && !qdev_get_machine_hotplug_handler(dev)) {
> +/* No bus, no machine hotplug handler --> device is not hotpluggable
> */
Long line.
> +error_setg(&err, "Device '%s' can not be hotplugged on this machine",
> + driver);
> +goto err_del_dev;
> }
>
> qdev_set_id(dev, qemu_opts_id(opts));
Hmm. We need to check "can hotplug" in two separ
Re: [Qemu-devel] [PATCH] block: Close a BlockDriverState completely even when bs->drv is NULL
On Wed 08 Nov 2017 03:33:54 PM CET, Kevin Wolf wrote: >> This patch makes bdrv_close() do the full uninitialization process in >> all cases. This fixes the problem with corrupted images and still >> works fine with freshly created BDSs. >> >> Signed-off-by: Alberto Garcia > > This doesn't apply cleanly in the test case. Does it depend on your > qcow2 fixes? Ah, yes. It doesn't have any semantic dependency though, so as long as you add the new tests to qemu-iotests/060 it will work. I can resend it properly rebased. > Also, while I think the change makes sense, it's also clear that we're > trying to fix up an inconsistent state here. Maybe we could also > improve the state that block drivers leave behind when marking an > image as corrupt. Just setting bs->drv = NULL means that at least any > internal data structures will not get cleaned up. > > On the other hand, we can't just call bdrv_close() from a failing > request because closing requires that we drain the request > first. Maybe it would be possible to call drv->bdrv_close() with a BH > or something. I'm not sure if I'm following you here, where would you add the bottom-half and what kind of problem it would solve? Berto
Re: [Qemu-devel] [PATCH 3/3] qemu-iotests: Test I/O limits with removable media
On Fri 10 Nov 2017 11:21:27 PM CET, Max Reitz wrote: > By the way, I just noticed that this test tests that > x-blockdev-remove-medium and x-blockdev-insert-medium do not destroy > throttling information -- which is exactly why those commands had been > declared experimental in the first place. Oh, was that the reason? > So I guess this means we can drop the x- now. :-) Sure. Berto
[Qemu-devel] [PULL 8/9] hw: add .min_cpus and .default_cpus fields to machine_class
From: "Emilio G. Cota"
max_cpus needs to be an upper bound on the number of vCPUs
initialized; otherwise TCG region initialization breaks.
Some boards initialize a hard-coded number of vCPUs, which is not
captured by the global max_cpus and therefore breaks TCG initialization.
Fix it by adding the .min_cpus field to machine_class.
This commit also changes some user-facing behaviour: we now die if
-smp is below this hard-coded vCPU minimum instead of silently
ignoring the passed -smp value (sometimes announcing this by printing
a warning). However, the introduction of .default_cpus lessens the
likelihood that users will notice this: if -smp isn't set, we now
assign the value in .default_cpus to both smp_cpus and max_cpus. IOW,
if a user does not set -smp, they always get a correct number of vCPUs.
This change fixes 3468b59 ("tcg: enable multiple TCG contexts in
softmmu", 2017-10-24), which broke TCG initialization for some
ARM boards.
Fixes: 3468b59e18b179bc63c7ce934de912dfa9596122
Reported-by: Thomas Huth
Reviewed-by: Eduardo Habkost
Reviewed-by: Alistair Francis
Signed-off-by: Emilio G. Cota
Message-id: 1510343626-25861-6-git-send-email-c...@braap.org
Suggested-by: Peter Maydell
Signed-off-by: Emilio G. Cota
Signed-off-by: Peter Maydell
---
include/hw/boards.h | 5 +
hw/arm/exynos4_boards.c | 12
hw/arm/raspi.c | 2 ++
hw/arm/xlnx-zcu102.c| 2 ++
vl.c| 21 ++---
5 files changed, 31 insertions(+), 11 deletions(-)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 191a5b3..62f160e 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -102,6 +102,9 @@ typedef struct {
/**
* MachineClass:
+ * @max_cpus: maximum number of CPUs supported. Default: 1
+ * @min_cpus: minimum number of CPUs supported. Default: 1
+ * @default_cpus: number of CPUs instantiated if none are specified. Default: 1
* @get_hotplug_handler: this function is called during bus-less
*device hotplug. If defined it returns pointer to an instance
*of HotplugHandler object, which handles hotplug operation
@@ -167,6 +170,8 @@ struct MachineClass {
BlockInterfaceType block_default_type;
int units_per_default_bus;
int max_cpus;
+int min_cpus;
+int default_cpus;
unsigned int no_serial:1,
no_parallel:1,
use_virtcon:1,
diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
index f1441ec..750162c 100644
--- a/hw/arm/exynos4_boards.c
+++ b/hw/arm/exynos4_boards.c
@@ -27,7 +27,6 @@
#include "qemu-common.h"
#include "cpu.h"
#include "sysemu/sysemu.h"
-#include "sysemu/qtest.h"
#include "hw/sysbus.h"
#include "net/net.h"
#include "hw/arm/arm.h"
@@ -129,13 +128,6 @@ exynos4_boards_init_common(MachineState *machine,
Exynos4BoardType board_type)
{
Exynos4BoardState *s = g_new(Exynos4BoardState, 1);
-MachineClass *mc = MACHINE_GET_CLASS(machine);
-
-if (smp_cpus != EXYNOS4210_NCPUS && !qtest_enabled()) {
-error_report("%s board supports only %d CPU cores, ignoring smp_cpus"
- " value",
- mc->name, EXYNOS4210_NCPUS);
-}
exynos4_board_binfo.ram_size = exynos4_board_ram_size[board_type];
exynos4_board_binfo.board_id = exynos4_board_id[board_type];
@@ -189,6 +181,8 @@ static void nuri_class_init(ObjectClass *oc, void *data)
mc->desc = "Samsung NURI board (Exynos4210)";
mc->init = nuri_init;
mc->max_cpus = EXYNOS4210_NCPUS;
+mc->min_cpus = EXYNOS4210_NCPUS;
+mc->default_cpus = EXYNOS4210_NCPUS;
mc->ignore_memory_transaction_failures = true;
}
@@ -205,6 +199,8 @@ static void smdkc210_class_init(ObjectClass *oc, void *data)
mc->desc = "Samsung SMDKC210 board (Exynos4210)";
mc->init = smdkc210_init;
mc->max_cpus = EXYNOS4210_NCPUS;
+mc->min_cpus = EXYNOS4210_NCPUS;
+mc->default_cpus = EXYNOS4210_NCPUS;
mc->ignore_memory_transaction_failures = true;
}
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 5941c9f..cd5fa8c 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -167,6 +167,8 @@ static void raspi2_machine_init(MachineClass *mc)
mc->no_floppy = 1;
mc->no_cdrom = 1;
mc->max_cpus = BCM2836_NCPUS;
+mc->min_cpus = BCM2836_NCPUS;
+mc->default_cpus = BCM2836_NCPUS;
mc->default_ram_size = 1024 * 1024 * 1024;
mc->ignore_memory_transaction_failures = true;
};
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index 190eb69..9631a53 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -189,6 +189,7 @@ static void xlnx_ep108_machine_class_init(ObjectClass *oc,
void *data)
mc->units_per_default_bus = 1;
mc->ignore_memory_transaction_failures = true;
mc->max_cpus = XLNX_ZYNQMP_NUM_APU_CPUS + XLNX_ZYNQMP_NUM_RPU_CPUS;
+mc->default_cpus = XLNX_ZYNQMP_NUM_APU_CPUS;
}
static const TypeInfo xlnx_ep108_machine_init_typeinfo = {
@@ -246,6 +247,7 @@ static void xlnx
[Qemu-devel] [PULL 7/9] xlnx-zcu102: Specify the max number of CPUs for the EP108
From: "Emilio G. Cota"
Just like the zcu102, the ep108 can instantiate several CPUs.
Signed-off-by: Emilio G. Cota
Reviewed-by: Alistair Francis
Message-id: 1510343626-25861-5-git-send-email-c...@braap.org
Signed-off-by: Peter Maydell
---
hw/arm/xlnx-zcu102.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index a23..190eb69 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -188,6 +188,7 @@ static void xlnx_ep108_machine_class_init(ObjectClass *oc,
void *data)
mc->block_default_type = IF_IDE;
mc->units_per_default_bus = 1;
mc->ignore_memory_transaction_failures = true;
+mc->max_cpus = XLNX_ZYNQMP_NUM_APU_CPUS + XLNX_ZYNQMP_NUM_RPU_CPUS;
}
static const TypeInfo xlnx_ep108_machine_init_typeinfo = {
--
2.7.4
[Qemu-devel] [PULL 1/9] arm/translate-a64: mark path as unreachable to eliminate warning
From: "Emilio G. Cota"
Fixes the following warning when compiling with gcc 5.4.0 with -O1
optimizations and --enable-debug:
target/arm/translate-a64.c: In function ‘aarch64_tr_translate_insn’:
target/arm/translate-a64.c:2361:8: error: ‘post_index’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
if (!post_index) {
^
target/arm/translate-a64.c:2307:10: note: ‘post_index’ was declared here
bool post_index;
^
target/arm/translate-a64.c:2386:8: error: ‘writeback’ may be used uninitialized
in this function [-Werror=maybe-uninitialized]
if (writeback) {
^
target/arm/translate-a64.c:2308:10: note: ‘writeback’ was declared here
bool writeback;
^
Note that idx comes from selecting 2 bits, and therefore its value
can be at most 3.
Signed-off-by: Emilio G. Cota
Acked-by: Philippe Mathieu-Daudé
Message-id: 1510087611-1851-1-git-send-email-c...@braap.org
Reviewed-by: Peter Maydell
Signed-off-by: Peter Maydell
---
target/arm/translate-a64.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index caca05a..625ef2d 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -2351,6 +2351,8 @@ static void disas_ldst_reg_imm9(DisasContext *s, uint32_t
insn,
post_index = false;
writeback = true;
break;
+default:
+g_assert_not_reached();
}
if (rn == 31) {
--
2.7.4
[Qemu-devel] [PULL 6/9] xlnx-zcu102: Add an info message deprecating the EP108
From: Alistair Francis
The EP108 was an early access development board that is no longer used.
Add an info message to convert any users to the ZCU102 instead. On QEMU
they are both identical.
This patch also updated the qemu-doc.texi file to indicate that the
EP108 has been deprecated.
Signed-off-by: Alistair Francis
Reviewed-by: Emilio G. Cota
Message-id: 1510343626-25861-4-git-send-email-c...@braap.org
Signed-off-by: Peter Maydell
---
hw/arm/xlnx-zcu102.c | 3 +++
qemu-doc.texi| 7 +++
2 files changed, 10 insertions(+)
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index 7ec03da..a23 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -164,6 +164,9 @@ static void xlnx_ep108_init(MachineState *machine)
{
XlnxZCU102 *s = EP108_MACHINE(machine);
+info_report("The Xilinx EP108 machine is deprecated, please use the "
+"ZCU102 machine instead. It has the same features supported.");
+
xlnx_zynqmp_init(s, machine);
}
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 8c10956..d383ac4 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -2537,6 +2537,13 @@ or ``ivshmem-doorbell`` device types.
The ``spapr-pci-vfio-host-bridge'' device type is replaced by
the ``spapr-pci-host-bridge'' device type.
+@section System emulator machines
+
+@subsection Xilinx EP108 (since 2.11.0)
+
+The ``xlnx-ep108'' machine has been replaced by the ``xlnx-zcu102'' machine.
+The ``xlnx-zcu102'' machine has the same features and capabilites in QEMU.
+
@node License
@appendix License
--
2.7.4
[Qemu-devel] [PULL 2/9] highbank: validate register offset before access
From: Prasad J Pandit
An 'offset' parameter sent to highbank register r/w functions
could be greater than number(NUM_REGS=0x200) of hb registers,
leading to an OOB access issue. Add check to avoid it.
Reported-by: Moguofang (Dennis mo)
Signed-off-by: Prasad J Pandit
Message-id: 20171113062658.9697-1-ppan...@redhat.com
Reviewed-by: Peter Maydell
Signed-off-by: Peter Maydell
---
hw/arm/highbank.c | 17 +++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index 354c6b2..287392b 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -34,6 +34,7 @@
#include "hw/ide/ahci.h"
#include "hw/cpu/a9mpcore.h"
#include "hw/cpu/a15mpcore.h"
+#include "qemu/log.h"
#define SMP_BOOT_ADDR 0x100
#define SMP_BOOT_REG0x40
@@ -117,14 +118,26 @@ static void hb_regs_write(void *opaque, hwaddr offset,
}
}
-regs[offset/4] = value;
+if (offset / 4 >= NUM_REGS) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "highbank: bad write offset 0x%" HWADDR_PRIx "\n", offset);
+return;
+}
+regs[offset / 4] = value;
}
static uint64_t hb_regs_read(void *opaque, hwaddr offset,
unsigned size)
{
+uint32_t value;
uint32_t *regs = opaque;
-uint32_t value = regs[offset/4];
+
+if (offset / 4 >= NUM_REGS) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "highbank: bad read offset 0x%" HWADDR_PRIx "\n", offset);
+return 0;
+}
+value = regs[offset / 4];
if ((offset == 0x100) || (offset == 0x108) || (offset == 0x10C)) {
value |= 0x3000;
--
2.7.4
Re: [Qemu-devel] [PATCH v3 2/2] qdev: Check for the availability of a hotplug controller before adding a device
On 13.11.2017 15:00, Markus Armbruster wrote:
> Thomas Huth writes:
>
>> The qdev_unplug() function contains a g_assert(hotplug_ctrl) statement,
>> so QEMU crashes when the user tries to device_add + device_del a device
>> that does not have a corresponding hotplug controller. This could be
>> provoked for a couple of devices in the past (see commit 4c93950659487c7ad
>> or 84ebd3e8c7d4fe955 for example), and can currently for example also be
>> triggered like this:
>>
>> $ s390x-softmmu/qemu-system-s390x -M none -nographic
>> QEMU 2.10.50 monitor - type 'help' for more information
>> (qemu) device_add qemu-s390x-cpu,id=x
>> (qemu) device_del x
>> **
>> ERROR:qemu/qdev-monitor.c:872:qdev_unplug: assertion failed: (hotplug_ctrl)
>> Aborted (core dumped)
>>
>> So devices clearly need a hotplug controller when they should be usable
>> with device_add.
>> The code in qdev_device_add() already checks whether the bus has a proper
>> hotplug controller,
>
> Where? Hmm, I guess it's this one:
>
> if (qdev_hotplug && bus && !qbus_is_hotpluggable(bus)) {
> error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name);
> return NULL;
> }
Right.
>> but for devices that do not have a corresponding bus,
>> there is no appropriate check available yet. In that case we should check
>> whether the machine itself provides a suitable hotplug controller and
>> refuse to plug the device if none is available.
[...]
>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>> index 9188d20..38c0fc2 100644
>> --- a/qdev-monitor.c
>> +++ b/qdev-monitor.c
>> @@ -614,6 +614,11 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error
>> **errp)
>>
>if (qdev_hotplug && bus && !qbus_is_hotpluggable(bus)) {
>error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name);
>return NULL;
>}
>
>if (!migration_is_idle()) {
>error_setg(errp, "device_add not allowed while migrating");
>return NULL;
>}
>
>/* create device */
>dev = DEVICE(object_new(driver));
>
>> if (bus) {
>> qdev_set_parent_bus(dev, bus);
>> +} else if (qdev_hotplug && !qdev_get_machine_hotplug_handler(dev)) {
>> +/* No bus, no machine hotplug handler --> device is not
>> hotpluggable */
>
> Long line.
Less than 80 columns, so that's fine.
>> +error_setg(&err, "Device '%s' can not be hotplugged on this
>> machine",
>> + driver);
>> +goto err_del_dev;
>> }
>>
>> qdev_set_id(dev, qemu_opts_id(opts));
>
> Hmm. We need to check "can hotplug" in two separate ways, with bus and
> without bus. Can we keep the two ways on one place? Something like
>
>if (qdev_hotplug) {
>if (bus && !qbus_is_hotpluggable(bus)) {
>error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name);
>return NULL;
>}
>if (!bus && !qdev_get_machine_hotplug_handler(dev)) {
>error_setg(&err,
> "Machine doesn't support hot-plugging device '%s'"
> driver);
>return NULL;
>}
> }
We likely could ... but I don't like that change since in that case
you've got to always do "dev = DEVICE(object_new(driver))" (and the code
needs to be at the end of the function, so it would need "goto
err_del_dev" instead of "return NULL"). If we keep the checks separate,
the first check can bail out without creating the device first, so I'd
really prefer to keep my patch that way.
Thomas
[Qemu-devel] [PULL 9/9] accel/tcg/translate-all: expand cpu_restore_state addr check
From: Alex Bennée
We are still seeing signals during translation time when we walk over
a page protection boundary. This expands the check to ensure the host
PC is inside the code generation buffer. The original suggestion was
to check versus tcg_ctx.code_gen_ptr but as we now segment the
translation buffer we have to settle for just a general check for
being inside.
I've also fixed up the declaration to make it clear it can deal with
invalid addresses. A later patch will fix up the call sites.
Signed-off-by: Alex Bennée
Reported-by: Peter Maydell
Reviewed-by: Laurent Vivier
Reviewed-by: Richard Henderson
Message-id: 20171108153245.20740-2-alex.ben...@linaro.org
Suggested-by: Paolo Bonzini
Cc: Richard Henderson
Tested-by: Peter Maydell
Signed-off-by: Peter Maydell
---
include/exec/exec-all.h | 11 ++
accel/tcg/translate-all.c | 52 ++-
2 files changed, 40 insertions(+), 23 deletions(-)
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 923ece3..0f51c92 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -45,6 +45,17 @@ void restore_state_to_opc(CPUArchState *env, struct
TranslationBlock *tb,
target_ulong *data);
void cpu_gen_init(void);
+
+/**
+ * cpu_restore_state:
+ * @cpu: the vCPU state is to be restore to
+ * @searched_pc: the host PC the fault occurred at
+ * @return: true if state was restored, false otherwise
+ *
+ * Attempt to restore the state for a fault occurring in translated
+ * code. If the searched_pc is not in translated code no state is
+ * restored and the function returns false.
+ */
bool cpu_restore_state(CPUState *cpu, uintptr_t searched_pc);
void QEMU_NORETURN cpu_loop_exit_noexc(CPUState *cpu);
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 34c5e28..e7f0329 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -352,36 +352,42 @@ static int cpu_restore_state_from_tb(CPUState *cpu,
TranslationBlock *tb,
return 0;
}
-bool cpu_restore_state(CPUState *cpu, uintptr_t retaddr)
+bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc)
{
TranslationBlock *tb;
bool r = false;
+uintptr_t check_offset;
-/* A retaddr of zero is invalid so we really shouldn't have ended
- * up here. The target code has likely forgotten to check retaddr
- * != 0 before attempting to restore state. We return early to
- * avoid blowing up on a recursive tb_lock(). The target must have
- * previously survived a failed cpu_restore_state because
- * tb_find_pc(0) would have failed anyway. It still should be
- * fixed though.
+/* The host_pc has to be in the region of current code buffer. If
+ * it is not we will not be able to resolve it here. The two cases
+ * where host_pc will not be correct are:
+ *
+ * - fault during translation (instruction fetch)
+ * - fault from helper (not using GETPC() macro)
+ *
+ * Either way we need return early to avoid blowing up on a
+ * recursive tb_lock() as we can't resolve it here.
+ *
+ * We are using unsigned arithmetic so if host_pc <
+ * tcg_init_ctx.code_gen_buffer check_offset will wrap to way
+ * above the code_gen_buffer_size
*/
-
-if (!retaddr) {
-return r;
-}
-
-tb_lock();
-tb = tb_find_pc(retaddr);
-if (tb) {
-cpu_restore_state_from_tb(cpu, tb, retaddr);
-if (tb->cflags & CF_NOCACHE) {
-/* one-shot translation, invalidate it immediately */
-tb_phys_invalidate(tb, -1);
-tb_remove(tb);
+check_offset = host_pc - (uintptr_t) tcg_init_ctx.code_gen_buffer;
+
+if (check_offset < tcg_init_ctx.code_gen_buffer_size) {
+tb_lock();
+tb = tb_find_pc(host_pc);
+if (tb) {
+cpu_restore_state_from_tb(cpu, tb, host_pc);
+if (tb->cflags & CF_NOCACHE) {
+/* one-shot translation, invalidate it immediately */
+tb_phys_invalidate(tb, -1);
+tb_remove(tb);
+}
+r = true;
}
-r = true;
+tb_unlock();
}
-tb_unlock();
return r;
}
--
2.7.4
[Qemu-devel] [PULL 5/9] xlnx-zynqmp: Properly support the smp command line option
From: Alistair Francis
Allow the -smp command line option to control the number of CPUs we
create.
Signed-off-by: Alistair Francis
Reviewed-by: Eduardo Habkost
Reviewed-by: Emilio G. Cota
Tested-by: Emilio G. Cota
Message-id: 1510343626-25861-3-git-send-email-c...@braap.org
Signed-off-by: Peter Maydell
---
hw/arm/xlnx-zcu102.c | 3 ++-
hw/arm/xlnx-zynqmp.c | 26 --
2 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index e2d15a1..7ec03da 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -235,7 +235,8 @@ static void xlnx_zcu102_machine_class_init(ObjectClass *oc,
void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
-mc->desc = "Xilinx ZynqMP ZCU102 board";
+mc->desc = "Xilinx ZynqMP ZCU102 board with 4xA53s and 2xR5s based on " \
+ "the value of smp";
mc->init = xlnx_zcu102_init;
mc->block_default_type = IF_IDE;
mc->units_per_default_bus = 1;
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index d4b6560..c707c66 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -98,8 +98,9 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, const
char *boot_cpu,
{
Error *err = NULL;
int i;
+int num_rpus = MIN(smp_cpus - XLNX_ZYNQMP_NUM_APU_CPUS,
XLNX_ZYNQMP_NUM_RPU_CPUS);
-for (i = 0; i < XLNX_ZYNQMP_NUM_RPU_CPUS; i++) {
+for (i = 0; i < num_rpus; i++) {
char *name;
object_initialize(&s->rpu_cpu[i], sizeof(s->rpu_cpu[i]),
@@ -132,8 +133,9 @@ static void xlnx_zynqmp_init(Object *obj)
{
XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
int i;
+int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
-for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
+for (i = 0; i < num_apus; i++) {
object_initialize(&s->apu_cpu[i], sizeof(s->apu_cpu[i]),
"cortex-a53-" TYPE_ARM_CPU);
object_property_add_child(obj, "apu-cpu[*]", OBJECT(&s->apu_cpu[i]),
@@ -182,6 +184,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error
**errp)
MemoryRegion *system_memory = get_system_memory();
uint8_t i;
uint64_t ram_size;
+int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
const char *boot_cpu = s->boot_cpu ? s->boot_cpu : "apu-cpu[0]";
ram_addr_t ddr_low_size, ddr_high_size;
qemu_irq gic_spi[GIC_NUM_SPI_INTR];
@@ -233,10 +236,10 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error
**errp)
qdev_prop_set_uint32(DEVICE(&s->gic), "num-irq", GIC_NUM_SPI_INTR + 32);
qdev_prop_set_uint32(DEVICE(&s->gic), "revision", 2);
-qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", XLNX_ZYNQMP_NUM_APU_CPUS);
+qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", num_apus);
/* Realize APUs before realizing the GIC. KVM requires this. */
-for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
+for (i = 0; i < num_apus; i++) {
char *name;
object_property_set_int(OBJECT(&s->apu_cpu[i]), QEMU_PSCI_CONDUIT_SMC,
@@ -292,7 +295,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error
**errp)
}
}
-for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
+for (i = 0; i < num_apus; i++) {
qemu_irq irq;
sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i,
@@ -307,11 +310,14 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error
**errp)
}
if (s->has_rpu) {
-xlnx_zynqmp_create_rpu(s, boot_cpu, &err);
-if (err) {
-error_propagate(errp, err);
-return;
-}
+info_report("The 'has_rpu' property is no longer required, to use the "
+"RPUs just use -smp 6.");
+}
+
+xlnx_zynqmp_create_rpu(s, boot_cpu, &err);
+if (err) {
+error_propagate(errp, err);
+return;
}
if (!s->boot_cpu_ptr) {
--
2.7.4
[Qemu-devel] [PULL 3/9] MAINTAINERS: Add entries for Smartfusion2
From: Subbaraya Sundeep Voluntarily add myself as maintainer for Smartfusion2 Signed-off-by: Subbaraya Sundeep Reviewed-by: Alistair Francis Reviewed-by: Philippe Mathieu-Daudé Message-id: 1510552520-3566-1-git-send-email-sundeep.l...@gmail.com Signed-off-by: Peter Maydell --- MAINTAINERS | 17 + 1 file changed, 17 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 0cd4d02..ffd77b4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -564,6 +564,23 @@ M: Alistair Francis S: Maintained F: hw/arm/netduino2.c +SmartFusion2 +M: Subbaraya Sundeep +S: Maintained +F: hw/arm/msf2-soc.c +F: hw/misc/msf2-sysreg.c +F: hw/timer/mss-timer.c +F: hw/ssi/mss-spi.c +F: include/hw/arm/msf2-soc.h +F: include/hw/misc/msf2-sysreg.h +F: include/hw/timer/mss-timer.h +F: include/hw/ssi/mss-spi.h + +Emcraft M2S-FG484 +M: Subbaraya Sundeep +S: Maintained +F: hw/arm/msf2-som.c + CRIS Machines - Axis Dev88 -- 2.7.4
Re: [Qemu-devel] [PATCH v4] throttle-groups: drain before detaching ThrottleState
On Fri, Nov 10, 2017 at 03:19:34PM +, Stefan Hajnoczi wrote:
> I/O requests hang after stop/cont commands at least since QEMU 2.10.0
> with -drive iops=100:
>
> (guest)$ dd if=/dev/zero of=/dev/vdb oflag=direct count=1000
> (qemu) stop
> (qemu) cont
> ...I/O is stuck...
>
> This happens because blk_set_aio_context() detaches the ThrottleState
> while requests may still be in flight:
>
> if (tgm->throttle_state) {
> throttle_group_detach_aio_context(tgm);
> throttle_group_attach_aio_context(tgm, new_context);
> }
>
> This patch encloses the detach/attach calls in a drained region so no
> I/O request is left hanging. Also add assertions so we don't make the
> same mistake again in the future.
>
> Reported-by: Yongxue Hong
> Signed-off-by: Stefan Hajnoczi
> ---
> v4:
> * Simplified patch in response to Berto's review
> ---
> block/block-backend.c | 2 ++
> block/throttle-groups.c | 6 ++
> 2 files changed, 8 insertions(+)
Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block
Stefan
signature.asc
Description: PGP signature
[Qemu-devel] [PULL 0/9] target-arm queue
ARM bugfixes for rc1... The following changes since commit f291910db61b5812e68f1e76afb3ade41d567bea: Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2017-11-09' into staging (2017-11-13 13:13:12 +) are available in the git repository at: git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20171113 for you to fetch changes up to d25f2a72272b9ffe0d06710d6217d1169bc2cc7d: accel/tcg/translate-all: expand cpu_restore_state addr check (2017-11-13 13:55:27 +) target-arm queue: * translate-a64.c: silence gcc5 warning * highbank: validate register offset before access * MAINTAINERS: Add entries for Smartfusion2 * accel/tcg/translate-all: expand cpu_restore_state addr check (so usermode insn aborts don't crash with an assertion failure) * fix TCG initialization of some Arm boards by allowing them to specify min/default number of CPUs to create Alex Bennée (1): accel/tcg/translate-all: expand cpu_restore_state addr check Alistair Francis (2): xlnx-zynqmp: Properly support the smp command line option xlnx-zcu102: Add an info message deprecating the EP108 Emilio G. Cota (4): arm/translate-a64: mark path as unreachable to eliminate warning qom: move CPUClass.tcg_initialize to a global xlnx-zcu102: Specify the max number of CPUs for the EP108 hw: add .min_cpus and .default_cpus fields to machine_class Prasad J Pandit (1): highbank: validate register offset before access Subbaraya Sundeep (1): MAINTAINERS: Add entries for Smartfusion2 include/exec/exec-all.h| 11 ++ include/hw/boards.h| 5 + include/qom/cpu.h | 1 - accel/tcg/translate-all.c | 52 ++ exec.c | 5 +++-- hw/arm/exynos4_boards.c| 12 --- hw/arm/highbank.c | 17 +-- hw/arm/raspi.c | 2 ++ hw/arm/xlnx-zcu102.c | 9 +++- hw/arm/xlnx-zynqmp.c | 26 ++- target/arm/translate-a64.c | 2 ++ vl.c | 21 --- MAINTAINERS| 17 +++ qemu-doc.texi | 7 +++ 14 files changed, 137 insertions(+), 50 deletions(-)
Re: [Qemu-devel] NBD BLOCK_STATUS
On 11/10/2017 10:51 AM, Vladimir Sementsov-Ogievskiy wrote: > 10.11.2017 19:06, Eric Blake wrote: >> On 11/09/2017 06:42 AM, Vladimir Sementsov-Ogievskiy wrote: >>> Hi! >>> >>> Interesting fact: list/set_meta_context options are per-export, >>> so, in the server we should keep context selection per client per >>> export. >>> >>> And it is possible for client to set contexts for one export and than >>> proceed >>> to transmission phase with another one. >> However, we also documented in the spec that >> >> + A client MUST NOT send `NBD_CMD_BLOCK_STATUS` unless >> + within the negotiation phase it sent `NBD_OPT_SET_META_CONTEXT` >> + at least once, and the final time it was sent, it referred >> + to the export name that was ultimately selected, the server >> + responded without an error, and returned at least one metadata >> + context. > > I've missed this, then it's OK. > > maybe "ultimately selected for transmission phase" would be a bit better. Sure, I'll queue that up for my next round of doc tweaks. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PULL 4/9] qom: move CPUClass.tcg_initialize to a global
From: "Emilio G. Cota"
55c3cee ("qom: Introduce CPUClass.tcg_initialize", 2017-10-24)
introduces a per-CPUClass bool that we check so that the target CPU
is initialized for TCG only once. This works well except when
we end up creating more than one CPUClass, in which case we end
up incorrectly initializing TCG more than once, i.e. once for
each CPUClass.
This can be replicated with:
$ aarch64-softmmu/qemu-system-aarch64 -machine xlnx-zcu102 -smp 6 \
-global driver=xlnx,,zynqmp,property=has_rpu,value=on
In this case the class name of the "RPUs" is prefixed by "cortex-r5-",
whereas the "regular" CPUs are prefixed by "cortex-a53-". This
results in two CPUClass instances being created.
Fix it by introducing a static variable, so that only the first
target CPU being initialized will initialize the target-dependent
part of TCG, regardless of CPUClass instances.
Fixes: 55c3ceef61fcf06fc98ddc752b7cce788ce7680b
Signed-off-by: Emilio G. Cota
Reviewed-by: Eduardo Habkost
Reviewed-by: Alistair Francis
Reviewed-by: Richard Henderson
Tested-by: Alistair Francis
Message-id: 1510343626-25861-2-git-send-email-c...@braap.org
Signed-off-by: Peter Maydell
---
include/qom/cpu.h | 1 -
exec.c| 5 +++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index fa4b0c9..c2fa151 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -209,7 +209,6 @@ typedef struct CPUClass {
/* Keep non-pointer data at the end to minimize holes. */
int gdb_num_core_regs;
bool gdb_stop_before_watchpoint;
-bool tcg_initialized;
} CPUClass;
#ifdef HOST_WORDS_BIGENDIAN
diff --git a/exec.c b/exec.c
index 97a24a8..8b579c0 100644
--- a/exec.c
+++ b/exec.c
@@ -792,11 +792,12 @@ void cpu_exec_initfn(CPUState *cpu)
void cpu_exec_realizefn(CPUState *cpu, Error **errp)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
+static bool tcg_target_initialized;
cpu_list_add(cpu);
-if (tcg_enabled() && !cc->tcg_initialized) {
-cc->tcg_initialized = true;
+if (tcg_enabled() && !tcg_target_initialized) {
+tcg_target_initialized = true;
cc->tcg_initialize();
}
--
2.7.4
[Qemu-devel] [Bug 1727737] Re: qemu-arm stalls on a GCC sanitizer test since qemu-2.7
Thanks for the clarification. But how does the target get the actual syscall return code, if do_syscall() is supposed to return negative-target-errno? I mean, in general the target code will check if the syscall returned -1, and only then query errno? But if QEMU's do_syscall returns -errno, and put this value in r0 (for arm) how is the target code supposed to work? -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1727737 Title: qemu-arm stalls on a GCC sanitizer test since qemu-2.7 Status in QEMU: Incomplete Bug description: Hi, I have noticed that several GCC/sanitizer tests fail with timeout when executed under QEMU. After a bit of investigation, I have noticed that this worked with qemu-2.7, and started failing with qemu-2.8, and still fails with qemu-2.10.1 I'm attaching a tarball containing: alloca_instruments_all_paddings.exe : the testcase, and the needed libs: lib/librt.so.1 lib/libdl.so.2 lib/ld-linux-armhf.so.3 lib/libasan.so.5 lib/libc.so.6 lib/libgcc_s.so.1 lib/libpthread.so.0 lib/libm.so.6 To reproduce the problem: $ qemu-arm -cpu any -R 0 -L $PWD $PWD/alloca_instruments_all_paddings.exe returns in less than a second with qemu-2.7, and never with qemu-2.8 Using -d in_asm suggests that the program "almost" completes and qemu seems to stall on: 0x40b6eb44: e08f4004 add r4, pc, r4 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1727737/+subscriptions
Re: [Qemu-devel] [PATCH 1/3] ivshmem: Don't update non-existent MSI routes
Ladi Prosek writes:
> As of commit 660c97eef6f8 ("ivshmem: use kvm irqfd for msi notifications"),
> QEMU crashes with:
>
> kvm_irqchip_commit_routes: Assertion `ret == 0' failed.
>
> if the ivshmem device is configured with more vectors than what the server
> supports. This is caused by the ivshmem_vector_unmask() being called on
> vectors that have not been initialized by ivshmem_add_kvm_msi_virq().
>
> This commit fixes it by adding a simple check to the mask and unmask
> callbacks.
>
> Note that the opposite mismatch, if the server supplies more vectors than
> what the device is configured for, is already handled and leads to output
> like:
>
> Too many eventfd received, device has 1 vectors
>
> Fixes: 660c97eef6f8 ("ivshmem: use kvm irqfd for msi notifications")
> Signed-off-by: Ladi Prosek
I think I understand your description of what's wrong. Not obvious to
me is how it can happen. The cover letter mentions a Windows ivshmem
driver. Is this a device bug a driver can trigger? If yes, how?
Re: [Qemu-devel] [Qemu-block] [PATCH v2 0/1] Add 8-byte wide AMD flash support, partial interleaving
On 11/10/2017 06:12 PM, Paolo Bonzini wrote: On 10/11/2017 21:25, Mike Nawrocki wrote: This patch set does a few things. First, it switches the AMD CFI flash MMIO operations from the old MMIO API to the new one. Second, it enables 8-byte wide flash arrays. Finally, it adds flash interleaving using the "device-width" and "max-device-width" properties, using the same interface as pflash_cfi01.c. Much of the code was taken and adapted from that file. This unfortunately is not a patch set, it's a patch that does many things. You should split it in three parts, according to the three things you've mentioned in the message above. Paolo Sure, no problem. I'll get the split patch set out shortly. Thanks, Mike
[Qemu-devel] Command-line option to change ungrab key(s)
Would you accept a patch that allows the user to change the mouse ungrab key(s) in QEMU? It would look something like this: -ungrab or -ungrab ,,... If the user wanted the F19 key to be the ungrab key, this is what would be sent to QEMU: -ungrab F19 If the user wanted F16 and F17 held down to ungrab the mouse, this is what would be sent to QEMU: -ungrab F16,F17 Would this feature be something you would consider adding to QEMU?
Re: [Qemu-devel] [PATCH v4] throttle-groups: drain before detaching ThrottleState
On Fri 10 Nov 2017 04:19:34 PM CET, Stefan Hajnoczi wrote:
> I/O requests hang after stop/cont commands at least since QEMU 2.10.0
> with -drive iops=100:
>
> (guest)$ dd if=/dev/zero of=/dev/vdb oflag=direct count=1000
> (qemu) stop
> (qemu) cont
> ...I/O is stuck...
>
> This happens because blk_set_aio_context() detaches the ThrottleState
> while requests may still be in flight:
>
> if (tgm->throttle_state) {
> throttle_group_detach_aio_context(tgm);
> throttle_group_attach_aio_context(tgm, new_context);
> }
>
> This patch encloses the detach/attach calls in a drained region so no
> I/O request is left hanging. Also add assertions so we don't make the
> same mistake again in the future.
I'm wondering about the implications of this change... is it possible
now to bypass the I/O limits simply by stopping and quickly resuming the
VM? And is that a problem?
Berto
Re: [Qemu-devel] [PATCH v2 for-2.11] block: Make bdrv_next() keep strong references
On Fri, Nov 10, 2017 at 06:25:45PM +0100, Max Reitz wrote: > On one hand, it is a good idea for bdrv_next() to return a strong > reference because ideally nearly every pointer should be refcounted. > This fixes intermittent failure of iotest 194. > > On the other, it is absolutely necessary for bdrv_next() itself to keep > a strong reference to both the BB (in its first phase) and the BDS (at > least in the second phase) because when called the next time, it will > dereference those objects to get a link to the next one. Therefore, it > needs these objects to stay around until then. Just storing the pointer > to the next in the iterator is not really viable because that pointer > might become invalid as well. > > Both arguments taken together means we should probably just invoke > bdrv_ref() and blk_ref() in bdrv_next(). This means we have to assert > that bdrv_next() is always called from the main loop, but that was > probably necessary already before this patch and judging from the > callers, it also looks to actually be the case. > > Keeping these strong references means however that callers need to give > them up if they decide to abort the iteration early. They can do so > through the new bdrv_next_cleanup() function. > > Suggested-by: Kevin Wolf > Signed-off-by: Max Reitz > --- > v2: Instead of keeping the strong reference in bdrv_drain_all_*() only, > have them for all callers of bdrv_next() [Fam, Kevin] > (Completely different patch now, so no git-backport-diff included > here) > --- > include/block/block.h | 1 + > block.c | 3 +++ > block/block-backend.c | 48 ++-- > block/snapshot.c | 6 ++ > migration/block.c | 1 + > 5 files changed, 57 insertions(+), 2 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
