[Qemu-devel] [Bug 1694998] Re: PPC: msgsnd instruction leads to assertion
Fix has now been included: http://git.qemu.org/?p=qemu.git;a=commitdiff;h=f1c29ebc51be77bd64178c8d ** Changed in: qemu Status: New => Fix Committed ** Changed in: qemu Assignee: (unassigned) => Thomas Huth (th-huth) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1694998 Title: PPC: msgsnd instruction leads to assertion Status in QEMU: Fix Committed Bug description: I tried to send doorbells (using msgsnd) between cores in guest OS. On QEMU v2.9.0 usage of msgsnd instruction leads to error: ERROR: <...>/qemu-new/translate-common.c:34:tcg_handle_interrupt: assertion failed: (qemu_mutex_iothread_locked()) QEMU v2.8.0 works fine. QEMU run options: qemu-system-ppc -serial stdio -M ppce500 -cpu e500mc -smp 2 -m 512M -kernel pok.elf pok.elf attached To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1694998/+subscriptions
Re: [Qemu-devel] [PATCH] s390: return unavailable features via query-cpu-definitions
On 30.06.2017 18:47, David Hildenbrand wrote:
> On 30.06.2017 15:25, Viktor Mihajlovski wrote:
>> The response for query-cpu-definitions didn't include the
>> unavailable-features field, which is used by libvirt to figure
>> out whether a certain cpu model is usable on the host.
>>
>> The unavailable features are now computed by obtaining the host CPU
>> model and comparing its feature bitmap with the feature bitmaps of
>> the known CPU models.
>>
>> I.e. the output of virsh domcapabilities would change from
>> ...
>>
>> z10EC-base
>> z9EC-base
>> z196.2-base
>> z900-base
>> z990
>> ...
>> to
>> ...
>>
>> z10EC-base
>> z9EC-base
>> z196.2-base
>> z900-base
>> z990
>> ...
>>
>> Signed-off-by: Viktor Mihajlovski
>> ---
>> target/s390x/cpu_models.c | 51
>> ++-
>> 1 file changed, 46 insertions(+), 5 deletions(-)
>>
>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>> index 63903c2..dc3371f 100644
>> --- a/target/s390x/cpu_models.c
>> +++ b/target/s390x/cpu_models.c
>> @@ -283,14 +283,43 @@ void s390_cpu_list(FILE *f, fprintf_function print)
>> }
>> }
>>
>> +static S390CPUModel *get_max_cpu_model(Error **errp);
>> +
>> #ifndef CONFIG_USER_ONLY
>> +static void list_add_feat(const char *name, void *opaque);
>> +
>> +static void check_unavailable_features(const S390CPUModel *max_model,
>> + const S390CPUModel *model,
>> + strList **unavailable)
>> +{
>> +S390FeatBitmap missing;
>> +
>> +/* detect missing features if any to properly report them */
>> +bitmap_andnot(missing, model->features, max_model->features,
>> + S390_FEAT_MAX);
>> +if (!bitmap_empty(missing, S390_FEAT_MAX)) {
>> +s390_feat_bitmap_to_ascii(missing,
>> + unavailable,
>> + list_add_feat);
>
> I remember discussing this with Eduardo when he introduced this.
>
> There is one additional case to handle here that might turn a CPU model
> not runnable. This can happen when trying to run a new CPU model on an
> old host, where the features are not the problem, but the model itself.
> E.g. running a GA2 version on a GA1 is not allowed. But this can happen
> more generally also between hardware generations.
>
> Therefore, whenever the model is never than the host model, we have to
> add here the special property "type" as missing feature. The
> documentation partly coveres what we had in mind.>
> qapi-schema.json:
> # @unavailable-features is a list of QOM property names that
>
> # represent CPU model attributes that prevent the CPU from running.
>
> # If the QOM property is read-only, that means there's no known
>
> # way to make the CPU model run in the current host. Implementations
>
> # that choose not to provide specific information return the
>
> # property name "type".
>
> We discussed that "type" should always be added if there is no way to
> make the model runnable (by simply removing features).
Thanks for the clarification.
I guess I was reading that too narrow-minded (no specific information vs
type), although I had the suspicion that features alone wouldn't
suffice. I will follow the query_cpu_model_comparison pattern and return
both "type" and the real features.
> > See check_compatibility() for details. For these cases, add "type" to
> the list. (you might be able to extend check_compatibility(), making
> e.g. the *errp and *unavailable parameters optional).
>
--
Mit freundlichen Grüßen/Kind Regards
Viktor Mihajlovski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
[Qemu-devel] Launching Standalone in QEMU-Microblaze
Hello!
I'm trying to run QEMU-Microblaze (Little-endian) with a standalone app in some
different ways, but none of them works for me:
1) I have created my own .DTB from my system design (.HDF), just a
microblaze connected to the Uartlite AXI, leds, interrupt controller, and a
gpio. Using the board support package I've made a simple c-based program (at
the end of the message):
Run it with ./qemu-system-microblazeel -M microblaze-fdt-plnx -dtb
system-top.dtb -kernel fibonacci.elf (-s -S) #(-s -S just to debug it)
Invalid MicroBlaze version number: (null) #Don't think it's a problem
Bar ram offset 9000528f
Aborted (core dumped)
2) I use a mb.dtb that I found on the internet. Same c-based program. Run
it with the debugger:
./qemu-system-microblazeel -M microblaze-fdt-plnx -dtb mb.dtb -kernel
fibonacci.elf (-s -S)
Lets my debug it (doesn't fail booting), but in the first step:
Bad ram pointer
I suppose this isn't my option cause the .dtb is created for another
microblaze-based design.
3) I use the same design (.HDF), but run it for the Spartan 3a dsp 1800
option:
./qemu-system-microblazeel -kernel fibonacci.elf (-s -S)
Runs "properly" -> I mean properly because I can follow on the debugger that
the steps are correctly made:
_start -> _start1 -> main -> fibonacci -> xil_printf -> xil_printf ... ->
xil_printf -> _exit
But doesn't print nothing. And doesn't write in memory as asked (*addrPtr =
0x150) :
In the qemu shell:
(qemu) x 0xC000
C000: 0x#When it should be 0x0150
4) Modifying my system design to be similar to the Spartan board design:
MEMORY_BASEADDR 0x9000
FLASH_BASEADDR 0xa000
INTC_BASEADDR 0x8180
TIMER_BASEADDR 0x83c0
UARTLITE_BASEADDR 0x8400
ETHLITE_BASEADDR 0x8100
I get exactly the same result as in the (3) case.
So here they go my questions:
- Am I doing it right? Is this the method of running a standalone
program over a microblaze?
- How can I make the program print something?
- Im not sure if the problem is that it doesn't write on memory, or I
am the one who fails reading it from the shell, because if I change the writing
position to (0x8408, uart status position) I get the error (qemu: hardware
error: write to UART STATUS?) -> that means im writing (or trying, at least).
How can I write on memory (and read after it, so I know it works)?
Thank you in advance! Really appreciate your Job! (I'm sorry if my problem it's
a simple one, I'm new at it)
#include
#include "platform.h"
#include "xil_printf.h"
void fibonacci(int d){
u32 a=1;
u32 b=0;
u32 em=0;
u32 *addrPtr = 0xC000;
for(int num=0; num
[Qemu-devel] [PATCH 0/1] virtio-scsi-ccw: fix iotest 068 on s390x
This commit fixes iotest 068 for s390x as s390x uses virtio-scsi-ccw. The related commit is c324fd0a39c for other platforms by Stefan Hajnoczi. Thanks! QingFeng Hao (1): virtio-scsi-ccw: use ioeventfd even when KVM is disabled hw/s390x/virtio-ccw.c | 2 +- target/s390x/kvm.c| 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) -- 2.11.2
Re: [Qemu-devel] [PATCH 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled
On 07/03/2017 09:38 AM, QingFeng Hao wrote:
> Do not check kvm_eventfds_enabled() when KVM is disabled since it
> always returns 0. Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
> ("memory: emulate ioeventfd") it has been possible to use ioeventfds in
> qtest or TCG mode.
>
> This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
> when KVM is disabled.
>
> I have tested that virtio-scsi-ccw works under tcg both with and without
> iothread.
>
> This patch fixes qemu-iotests 068, which was accidentally merged early
> despite the dependency on ioeventfd.
>
> Signed-off-by: QingFeng Hao
> Signed-off-by: Stefan Hajnoczi
cut'n'paste mistake of adding Stefans signoff?
Otherwise it looks good.
> ---
> hw/s390x/virtio-ccw.c | 2 +-
> target/s390x/kvm.c| 3 +++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 90d37cb9ff..35896eb007 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice
> *dev, Error **errp)
> sch->cssid, sch->ssid, sch->schid, sch->devno,
> ccw_dev->devno.valid ? "user-configured" : "auto-configured");
>
> -if (!kvm_eventfds_enabled()) {
> +if (kvm_enabled() && !kvm_eventfds_enabled()) {
> dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
> }
>
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index a3d00196f4..c37f9c3b9e 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -2220,6 +2220,9 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier
> *notifier, uint32_t sch,
> .addr = sch,
> .len = 8,
> };
> +if (!kvm_enabled()) {
> +return 0;
> +}
> if (!kvm_check_extension(kvm_state, KVM_CAP_IOEVENTFD)) {
> return -ENOSYS;
> }
>
Re: [Qemu-devel] [PATCH v1 2/3] arm: fix the armv7m reset state
On 06/30/2017 11:06 AM, Peter Maydell wrote: On 30 June 2017 at 09:24, KONRAD Frederic wrote: On 06/29/2017 06:45 PM, Peter Maydell wrote: It's the same thing, though, right? If the user's ELF file says "vector table is at 0x800" then we should either (a) say that's a user error, or (b) handle it right, whether we implemented the QEMU model with the flash at 0 and the alias at 0x800, or with the flash at 0x800 and the alias at 0. Fondamentaly yes, it is the same.. but it seems really strange to me to load the elf in the alias. The user creating the ELF file has no idea whether we modelled the flash at 0 and the alias at highmem or the other way around -- that is an implementation detail that should be completely invisible to the user. My two suggestions are based on that point -- either we should treat "ELF loaded at highmem" as always wrong, or we should always support it. If I choose (a) I'll need to objcpy all the elf to 0 or modify the build script which should work on the real board.. This seems not really a good option to me. I agree that I don't like this. If I choose (b) I won't be able to load it to SRAM and it is basically the same result I'll need to move or modify the config. I don't understand this, though. Option (b) is probably painful to implement (I don't have a good idea of how to do it) but it ought to mean that the ELF files that work on the board also work for QEMU (regardless of how the board model implemented the aliased flash). Yes that's exactly what I want. Basically the 0x alias can point to the SRAM or the ROM during the reset depending on some boot config. The ELF is directly loaded in the ROM or in the SRAM and my patch allows to fetch the two first words in the reset handler to make it work for any boot config. For example this devices p69: www.st.com/resource/en/reference_manual/DM00031020.pdf Thanks, Fred thanks -- PMM
[Qemu-devel] [PATCH 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled
Do not check kvm_eventfds_enabled() when KVM is disabled since it
always returns 0. Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
("memory: emulate ioeventfd") it has been possible to use ioeventfds in
qtest or TCG mode.
This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
when KVM is disabled.
I have tested that virtio-scsi-ccw works under tcg both with and without
iothread.
This patch fixes qemu-iotests 068, which was accidentally merged early
despite the dependency on ioeventfd.
Signed-off-by: QingFeng Hao
Signed-off-by: Stefan Hajnoczi
---
hw/s390x/virtio-ccw.c | 2 +-
target/s390x/kvm.c| 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 90d37cb9ff..35896eb007 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev,
Error **errp)
sch->cssid, sch->ssid, sch->schid, sch->devno,
ccw_dev->devno.valid ? "user-configured" : "auto-configured");
-if (!kvm_eventfds_enabled()) {
+if (kvm_enabled() && !kvm_eventfds_enabled()) {
dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
}
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index a3d00196f4..c37f9c3b9e 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2220,6 +2220,9 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier
*notifier, uint32_t sch,
.addr = sch,
.len = 8,
};
+if (!kvm_enabled()) {
+return 0;
+}
if (!kvm_check_extension(kvm_state, KVM_CAP_IOEVENTFD)) {
return -ENOSYS;
}
--
2.11.2
[Qemu-devel] [RFC v2 0/1] target/ppc/cpu-models: set POWER9_v1.0 as POWER9 DD1
This patch allows to use "-cpu POWER9" with a POWER9 DD1 host and KVM HV. With TCG, "-cpu POWER9" selects POWER9_v2.0 (POWER9 DD2). I post this patch as a RFC because kernel boots well with "-cpu host", but hangs with "-cpu POWER9" whereas I can see it selects correctly the POWER9_v1.0 CPU in both cases. Laurent Vivier (1): target/ppc/cpu-models: set POWER9_v1.0 as POWER9 DD1 hw/ppc/spapr_cpu_core.c | 5 - target/ppc/cpu-models.c | 6 -- target/ppc/cpu-models.h | 1 + 3 files changed, 9 insertions(+), 3 deletions(-) -- 2.9.4
[Qemu-devel] [RFC v2 1/1] target/ppc/cpu-models: set POWER9_v1.0 as POWER9 DD1
CPU_POWERPC_POWER9_DD1 is 0x004E0100, so this is the POWER9 v1.0.
When we run qemu on a POWER9 DD1 host, we must use either
"-cpu host" or "-cpu POWER9", but in the latter case it fails with
Unable to find sPAPR CPU Core definition
because POWER9 DD1 doesn't appear in the list of known CPUs.
This patch fixes this by defining POWER9_v1.0 with POWER9 DD1
PVR instead of CPU_POWERPC_POWER9_BASE. It also adds POWER_v2.0
with POWER9 DD2 PVR to avoid to trigger kernel POWER9 DD1 workaround
in TCG mode.
Signed-off-by: Laurent Vivier
---
hw/ppc/spapr_cpu_core.c | 5 -
target/ppc/cpu-models.c | 6 --
target/ppc/cpu-models.h | 1 +
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 9fb896b..00918a5 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -249,8 +249,11 @@ static const char *spapr_core_models[] = {
/* POWER8NVL */
"POWER8NVL_v1.0",
-/* POWER9 */
+/* POWER9 DD1 */
"POWER9_v1.0",
+
+/* POWER9 DD2 */
+"POWER9_v2.0",
};
static Property spapr_cpu_core_properties[] = {
diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
index 4d3e635..ac7e299 100644
--- a/target/ppc/cpu-models.c
+++ b/target/ppc/cpu-models.c
@@ -1144,8 +1144,10 @@
POWERPC_DEF("970_v2.2", CPU_POWERPC_970_v22,970,
"PowerPC 970 v2.2")
-POWERPC_DEF("POWER9_v1.0", CPU_POWERPC_POWER9_BASE,POWER9,
+POWERPC_DEF("POWER9_v1.0", CPU_POWERPC_POWER9_DD1, POWER9,
"POWER9 v1.0")
+POWERPC_DEF("POWER9_v2.0", CPU_POWERPC_POWER9_DD2, POWER9,
+"POWER9 v2.0")
POWERPC_DEF("970fx_v1.0",CPU_POWERPC_970FX_v10, 970,
"PowerPC 970FX v1.0 (G5)")
@@ -1391,7 +1393,7 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
{ "POWER8E", "POWER8E_v2.1" },
{ "POWER8", "POWER8_v2.0" },
{ "POWER8NVL", "POWER8NVL_v1.0" },
-{ "POWER9", "POWER9_v1.0" },
+{ "POWER9", "POWER9_v2.0" },
{ "970", "970_v2.2" },
{ "970fx", "970fx_v3.1" },
{ "970mp", "970mp_v1.1" },
diff --git a/target/ppc/cpu-models.h b/target/ppc/cpu-models.h
index b563c45..25045bd 100644
--- a/target/ppc/cpu-models.h
+++ b/target/ppc/cpu-models.h
@@ -562,6 +562,7 @@ enum {
CPU_POWERPC_POWER8NVL_v10 = 0x004C0100,
CPU_POWERPC_POWER9_BASE= 0x004E,
CPU_POWERPC_POWER9_DD1 = 0x004E0100,
+CPU_POWERPC_POWER9_DD2 = 0x004E1200,
CPU_POWERPC_970_v22= 0x00390202,
CPU_POWERPC_970FX_v10 = 0x00391100,
CPU_POWERPC_970FX_v20 = 0x003C0200,
--
2.9.4
[Qemu-devel] [PATCH v2] s390: return unavailable features via query-cpu-definitions
The response for query-cpu-definitions didn't include the
unavailable-features field, which is used by libvirt to figure
out whether a certain cpu model is usable on the host.
The unavailable features are now computed by obtaining the host CPU
model and comparing it against the known CPU models. The comparison
takes into account the generation, the GA level and the feature
bitmaps. In the case of a CPU generation/GA level mismatch
a feature called "type" is reported to be missing.
As a result, the output of virsh domcapabilities would change
from something like
...
z10EC-base
z9EC-base
z196.2-base
z900-base
z990
...
to
...
z10EC-base
z9EC-base
z196.2-base
z900-base
z990
...
Signed-off-by: Viktor Mihajlovski
---
target/s390x/cpu_models.c | 58 +++
1 file changed, 53 insertions(+), 5 deletions(-)
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 63903c2..51440ff 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -283,14 +283,50 @@ void s390_cpu_list(FILE *f, fprintf_function print)
}
}
+static S390CPUModel *get_max_cpu_model(Error **errp);
+
#ifndef CONFIG_USER_ONLY
+static void list_add_feat(const char *name, void *opaque);
+
+static void check_unavailable_features(const S390CPUModel *max_model,
+ const S390CPUModel *model,
+ strList **unavailable)
+{
+S390FeatBitmap missing;
+
+/* check general model compatibility */
+if (max_model->def->gen < model->def->gen ||
+(max_model->def->gen == model->def->gen &&
+ max_model->def->ec_ga < model->def->ec_ga)) {
+list_add_feat("type", unavailable);
+}
+
+/* detect missing features if any to properly report them */
+bitmap_andnot(missing, model->features, max_model->features,
+ S390_FEAT_MAX);
+if (!bitmap_empty(missing, S390_FEAT_MAX)) {
+s390_feat_bitmap_to_ascii(missing,
+ unavailable,
+ list_add_feat);
+}
+}
+
+struct CpuDefinitionInfoListData {
+CpuDefinitionInfoList *list;
+Error **errp;
+};
+
static void create_cpu_model_list(ObjectClass *klass, void *opaque)
{
-CpuDefinitionInfoList **cpu_list = opaque;
+struct CpuDefinitionInfoListData *cpu_list_data = opaque;
+CpuDefinitionInfoList **cpu_list = &cpu_list_data->list;
CpuDefinitionInfoList *entry;
CpuDefinitionInfo *info;
char *name = g_strdup(object_class_get_name(klass));
S390CPUClass *scc = S390_CPU_CLASS(klass);
+Object *obj;
+S390CPU *sc;
+S390CPUModel *scm;
/* strip off the -s390-cpu */
g_strrstr(name, "-" TYPE_S390_CPU)[0] = 0;
@@ -300,21 +336,33 @@ static void create_cpu_model_list(ObjectClass *klass,
void *opaque)
info->migration_safe = scc->is_migration_safe;
info->q_static = scc->is_static;
info->q_typename = g_strdup(object_class_get_name(klass));
-
+/* check for unavailable features */
+obj = object_new(object_class_get_name(klass));
+sc = S390_CPU(obj);
+scm = get_max_cpu_model(cpu_list_data->errp);
+if (scm && sc->model) {
+info->has_unavailable_features = true;
+check_unavailable_features(scm, sc->model,
&info->unavailable_features);
+}
entry = g_malloc0(sizeof(*entry));
entry->value = info;
entry->next = *cpu_list;
*cpu_list = entry;
+object_unref(obj);
}
CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
{
-CpuDefinitionInfoList *list = NULL;
+struct CpuDefinitionInfoListData list_data = {
+.list = NULL,
+.errp = errp,
+};
-object_class_foreach(create_cpu_model_list, TYPE_S390_CPU, false, &list);
+object_class_foreach(create_cpu_model_list, TYPE_S390_CPU, false,
+ &list_data);
-return list;
+return list_data.list;
}
static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info,
--
1.9.1
Re: [Qemu-devel] [Qemu-PPC] [PATCH 0/3] target/ppc: Implement radix mmu debug functions
On Mon, Jul 03, 2017 at 04:19:45PM +1000, Suraj Jitindar Singh wrote: > When the radix mmu emulation code was implemented the translation > debug functions were left as a todo. > > Implement the functions ppc_radix64_get_phys_page_debug() and > ppc_radix64_dump_level() to translate a single address and dump all the > address translations respectively. > > This functionality is already implemented for the hash mmu. > > A slight refactor of the radix mmu emulation code was necessary to enable > the debug code to call existing functions. I've applied 1&2 to ppc-for-2.10. I have some further comments on 3/3. > > Suraj Jitindar Singh (3): > target/ppc: Refactor tcg radix mmu code > target/ppc: Add debug function for radix mmu translation > target/ppc: Add debug function to dump radix mmu translations > > target/ppc/mmu-radix64.c | 116 > +-- > target/ppc/mmu-radix64.h | 2 + > target/ppc/mmu_helper.c | 5 +- > 3 files changed, 107 insertions(+), 16 deletions(-) > -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [Qemu-PPC] [PATCH 3/3] target/ppc: Add debug function to dump radix mmu translations
On Mon, Jul 03, 2017 at 04:19:48PM +1000, Suraj Jitindar Singh wrote:
> In target/ppc/mmu-hash64.c there already exists the function
> dump_slb() to dump the hash translation entries (for effective to
> virtual translation at least).
>
> Implement the function ppc_radix64_dump() to allow all the kernel
> effective to real address mappings and corresponding ptes to be dumped.
> This is called when "info tlb" is invoked in the qemu console. Previously
> this command had no output when invoked with a radix guest.
>
> Signed-off-by: Suraj Jitindar Singh
This doesn't really seem equivalent to the other dump_mmu() paths.
For HPT, it just dumps the SLB, which is 32 entries. The RPT is
likely to have thousands of PTEs.
Similarly the other embedded paths generally just dump a (software
loaded) TLB, which will have a fixed number of entries. 6xx dumps a
TLB and various metadata, but not its whole hash table.
In the other direction the info dumped for other platforms *is*
affected by the process context (or equivalent), whereas here you just
dump the RPT for the kernel (PID 0).
So instead of dumping the RPT itself, what I think you want is to
pretty print:
PID
LPID
partition table entry 0
partition table entry LPID
process table entry 0
process table entry PID
and maybe any other context relevant registers I've forgotten.
Obviously some of that will need to be omitted for the 'pseries' (vhyp
!= 0) case.
In fact most of the info above would make sense to always dump for a
v3.00 MMU, regardless of HPT vs RPT mode. Obviously dumping the SLB
only makes sense for HPT mode, though.
> ---
> target/ppc/mmu-radix64.c | 49
>
> target/ppc/mmu-radix64.h | 1 +
> target/ppc/mmu_helper.c | 2 +-
> 3 files changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
> index bbd37e3..f7eeead 100644
> --- a/target/ppc/mmu-radix64.c
> +++ b/target/ppc/mmu-radix64.c
> @@ -296,3 +296,52 @@ hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu,
> target_ulong eaddr)
>
> return raddr & TARGET_PAGE_MASK;
> }
> +
> +static void ppc_radix64_dump_level(FILE *f, fprintf_function cpu_fprintf,
> + PowerPCCPU *cpu, uint64_t eaddr,
> + uint64_t base_addr, uint64_t nls,
> + uint64_t psize, int *num)
> +{
> +CPUState *cs = CPU(cpu);
> +uint64_t i, pte;
> +
> +for (i = 0; i < (1 << nls); i++) {
> +eaddr &= ~((1ULL << psize) - 1); /* Clear the low bits */
> +eaddr |= (i << (psize - nls));
> +
> +pte = ldq_phys(cs->as, base_addr + (i * sizeof(pte)));
> +if (!(pte & R_PTE_VALID)) { /* Invalid Entry */
> +continue;
> +}
> +
> +if (pte & R_PTE_LEAF) {
> +uint64_t mask = (1ULL << (psize - nls)) - 1;
> +cpu_fprintf(f, "%d\t0x%.16" PRIx64 " -> 0x%.16" PRIx64
> + " pte: 0x%.16" PRIx64 "\n", (*num)++,
> + eaddr, pte & R_PTE_RPN & ~mask, pte);
> +} else {
> +ppc_radix64_dump_level(f, cpu_fprintf, cpu, eaddr, pte &
> R_PDE_NLB,
> + pte & R_PDE_NLS, psize - nls, num);
> +}
> +}
> +}
> +
> +void ppc_radix64_dump(FILE *f, fprintf_function cpu_fprintf, PowerPCCPU *cpu)
> +{
> +CPUState *cs = CPU(cpu);
> +PPCVirtualHypervisorClass *vhc =
> +PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
> +uint64_t patbe, prtbe0;
> +int num = 0;
> +
> +/* Get Process Table */
> +patbe = vhc->get_patbe(cpu->vhyp);
> +
> +/* Load the first entry -> Guest kernel mappings (all we dump for now) */
> +prtbe0 = ldq_phys(cs->as, patbe & PATBE1_R_PRTB);
> +
> +cpu_fprintf(f, "\tEADDR\t\t RADDR\n");
> +ppc_radix64_dump_level(f, cpu_fprintf, cpu, 0ULL, prtbe0 & PRTBE_R_RPDB,
> + prtbe0 & PRTBE_R_RPDS, PRTBE_R_GET_RTS(prtbe0),
> + &num);
> +}
> diff --git a/target/ppc/mmu-radix64.h b/target/ppc/mmu-radix64.h
> index 0ecf063..c6c22bd 100644
> --- a/target/ppc/mmu-radix64.h
> +++ b/target/ppc/mmu-radix64.h
> @@ -47,6 +47,7 @@
> int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
> int mmu_idx);
> hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
> +void ppc_radix64_dump(FILE *f, fprintf_function cpu_fprintf, PowerPCCPU
> *cpu);
>
> static inline int ppc_radix64_get_prot_eaa(uint64_t pte)
> {
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index b7b9088..9587b07 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -1287,7 +1287,7 @@ void dump_mmu(FILE *f, fprintf_function cpu_fprintf,
> CPUPPCState *env)
> break;
> case POWERPC_MMU_VER_3_00:
> if (ppc64_radix_gues
Re: [Qemu-devel] [PATCH 0/4] migration: fix iotest 055, only-migratable break
I tested the 2 patches and they can make iotest 055 passed on both x86 and s390x. thanks 在 2017/7/3 10:44, Peter Xu 写道: Two breakage introduced during the migration objectify series: one for --only-migratable, another one for iotest 055. First two patches fixes the breakages. Latter two are documentation updates suggested by Eduardo. Please review. Thanks. Peter Xu (4): migration: fix handling for --only-migratable vl: move global property, migrate init earlier doc: add item for "-M enforce-config-section" doc: update TYPE_MIGRATION documents include/migration/misc.h | 1 - migration/migration.c| 20 +--- qemu-options.hx | 8 vl.c | 26 +- 4 files changed, 30 insertions(+), 25 deletions(-) -- Regards QingFeng Hao
Re: [Qemu-devel] [PATCH 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled
在 2017/7/3 15:41, Christian Borntraeger 写道:
On 07/03/2017 09:38 AM, QingFeng Hao wrote:
Do not check kvm_eventfds_enabled() when KVM is disabled since it
always returns 0. Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
("memory: emulate ioeventfd") it has been possible to use ioeventfds in
qtest or TCG mode.
This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
when KVM is disabled.
I have tested that virtio-scsi-ccw works under tcg both with and without
iothread.
This patch fixes qemu-iotests 068, which was accidentally merged early
despite the dependency on ioeventfd.
Signed-off-by: QingFeng Hao
Signed-off-by: Stefan Hajnoczi
cut'n'paste mistake of adding Stefans signoff?
Otherwise it looks good.
I just want to mark that this patch is related with the former one from
Stefan.
Is that ok to add this sign-off? thanks!
---
hw/s390x/virtio-ccw.c | 2 +-
target/s390x/kvm.c| 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 90d37cb9ff..35896eb007 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev,
Error **errp)
sch->cssid, sch->ssid, sch->schid, sch->devno,
ccw_dev->devno.valid ? "user-configured" : "auto-configured");
-if (!kvm_eventfds_enabled()) {
+if (kvm_enabled() && !kvm_eventfds_enabled()) {
dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
}
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index a3d00196f4..c37f9c3b9e 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2220,6 +2220,9 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier
*notifier, uint32_t sch,
.addr = sch,
.len = 8,
};
+if (!kvm_enabled()) {
+return 0;
+}
if (!kvm_check_extension(kvm_state, KVM_CAP_IOEVENTFD)) {
return -ENOSYS;
}
--
Regards
QingFeng Hao
[Qemu-devel] [Bug 1701449] Re: high memory usage when using rbd with client caching
We are seeing pretty much the same issue with even small (1G mem) virtual instances using 2-3GB of RSS after running I/O intensive applications. Live migrating the instance to another machine pushes the memory usage back, but it will grow back again once I/O is back. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1701449 Title: high memory usage when using rbd with client caching Status in QEMU: New Bug description: Hi, we are experiencing a quite high memory usage of a single qemu (used with KVM) process when using RBD with client caching as a disk backend. We are testing with 3GB memory qemu virtual machines and 128MB RBD client cache. When running 'fio' in the virtual machine you can see that after some time the machine uses a lot more memory (RSS) on the hypervisor than she should. We have seen values (in real production machines, no artificially fio tests) of 250% memory overhead. I reproduced this with qemu version 2.9 as well. Here the contents of our ceph.conf on the hypervisor: """ [client] rbd cache writethrough until flush = False rbd cache max dirty = 100663296 rbd cache size = 134217728 rbd cache target dirty = 50331648 """ How to reproduce: * create a virtual machine with a RBD backed disk (100GB or so) * install a linux distribution on it (we are using Ubuntu) * install fio (apt-get install fio) * run fio multiple times with (e.g.) the following test file: """ # This job file tries to mimic the Intel IOMeter File Server Access Pattern [global] description=Emulation of Intel IOmeter File Server Access Pattern randrepeat=0 filename=/root/test.dat # IOMeter defines the server loads as the following: # iodepth=1 Linear # iodepth=4 Very Light # iodepth=8 Light # iodepth=64Moderate # iodepth=256 Heavy iodepth=8 size=80g direct=0 ioengine=libaio [iometer] stonewall bs=4M rw=randrw [iometer_just_write] stonewall bs=4M rw=write [iometer_just_read] stonewall bs=4M rw=read """ You can measure the virtual machine RSS usage on the hypervisor with: virsh dommemstat | grep rss or if you are not using libvirt: grep RSS /proc//status When switching off the RBD client cache, all is ok again, as the process does not use so much memory anymore. There is already a ticket on the ceph bug tracker for this ([1]). However I can reproduce that memory behaviour only when using qemu (maybe it is using librbd in a special way?). Running directly 'fio' with the rbd engine does not result in that high memory usage. [1] http://tracker.ceph.com/issues/20054 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1701449/+subscriptions
Re: [Qemu-devel] [PATCH 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled
On 07/03/2017 10:08 AM, QingFeng Hao wrote:
>
>
> 在 2017/7/3 15:41, Christian Borntraeger 写道:
>> On 07/03/2017 09:38 AM, QingFeng Hao wrote:
>>> Do not check kvm_eventfds_enabled() when KVM is disabled since it
>>> always returns 0. Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
>>> ("memory: emulate ioeventfd") it has been possible to use ioeventfds in
>>> qtest or TCG mode.
>>>
>>> This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
>>> when KVM is disabled.
>>>
>>> I have tested that virtio-scsi-ccw works under tcg both with and without
>>> iothread.
>>>
>>> This patch fixes qemu-iotests 068, which was accidentally merged early
>>> despite the dependency on ioeventfd.
>>>
>>> Signed-off-by: QingFeng Hao
>>> Signed-off-by: Stefan Hajnoczi
>> cut'n'paste mistake of adding Stefans signoff?
>>
>> Otherwise it looks good.
> I just want to mark that this patch is related with the former one from
> Stefan.
> Is that ok to add this sign-off? thanks!
No, sign-off indicates who passes the patch along for integration, so only
Stefan
is allowed to add that - if he actually takes the patch. It is very important
to
not mangle the sign-off-chain as it is actually used to track how a patch moved
from
the developer into the tree.
You can give credit to Stefan in the patch description - e.g. by saying in the
patch
description something like
based on a similar patch from Stefan Hajnoczi - commit c324fd0a39c ("
virtio-pci: use
ioeventfd even when KVM is disabled)
>>> ---
>>> hw/s390x/virtio-ccw.c | 2 +-
>>> target/s390x/kvm.c| 3 +++
>>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
>>> index 90d37cb9ff..35896eb007 100644
>>> --- a/hw/s390x/virtio-ccw.c
>>> +++ b/hw/s390x/virtio-ccw.c
>>> @@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice
>>> *dev, Error **errp)
>>> sch->cssid, sch->ssid, sch->schid, sch->devno,
>>> ccw_dev->devno.valid ? "user-configured" : "auto-configured");
>>>
>>> -if (!kvm_eventfds_enabled()) {
>>> +if (kvm_enabled() && !kvm_eventfds_enabled()) {
>>> dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
>>> }
>>>
>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>>> index a3d00196f4..c37f9c3b9e 100644
>>> --- a/target/s390x/kvm.c
>>> +++ b/target/s390x/kvm.c
>>> @@ -2220,6 +2220,9 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier
>>> *notifier, uint32_t sch,
>>> .addr = sch,
>>> .len = 8,
>>> };
>>> +if (!kvm_enabled()) {
>>> +return 0;
>>> +}
>>> if (!kvm_check_extension(kvm_state, KVM_CAP_IOEVENTFD)) {
>>> return -ENOSYS;
>>> }
>>>
>
Re: [Qemu-devel] [PATCH 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled
在 2017/7/3 16:21, Christian Borntraeger 写道:
On 07/03/2017 10:08 AM, QingFeng Hao wrote:
在 2017/7/3 15:41, Christian Borntraeger 写道:
On 07/03/2017 09:38 AM, QingFeng Hao wrote:
Do not check kvm_eventfds_enabled() when KVM is disabled since it
always returns 0. Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
("memory: emulate ioeventfd") it has been possible to use ioeventfds in
qtest or TCG mode.
This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
when KVM is disabled.
I have tested that virtio-scsi-ccw works under tcg both with and without
iothread.
This patch fixes qemu-iotests 068, which was accidentally merged early
despite the dependency on ioeventfd.
Signed-off-by: QingFeng Hao
Signed-off-by: Stefan Hajnoczi
cut'n'paste mistake of adding Stefans signoff?
Otherwise it looks good.
I just want to mark that this patch is related with the former one from Stefan.
Is that ok to add this sign-off? thanks!
No, sign-off indicates who passes the patch along for integration, so only
Stefan
is allowed to add that - if he actually takes the patch. It is very important to
not mangle the sign-off-chain as it is actually used to track how a patch moved
from
the developer into the tree.
You can give credit to Stefan in the patch description - e.g. by saying in the
patch
description something like
based on a similar patch from Stefan Hajnoczi - commit c324fd0a39c ("
virtio-pci: use
ioeventfd even when KVM is disabled)
Thanks for your good explanation and I'll change the commit message.
---
hw/s390x/virtio-ccw.c | 2 +-
target/s390x/kvm.c| 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 90d37cb9ff..35896eb007 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev,
Error **errp)
sch->cssid, sch->ssid, sch->schid, sch->devno,
ccw_dev->devno.valid ? "user-configured" : "auto-configured");
-if (!kvm_eventfds_enabled()) {
+if (kvm_enabled() && !kvm_eventfds_enabled()) {
dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
}
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index a3d00196f4..c37f9c3b9e 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2220,6 +2220,9 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier
*notifier, uint32_t sch,
.addr = sch,
.len = 8,
};
+if (!kvm_enabled()) {
+return 0;
+}
if (!kvm_check_extension(kvm_state, KVM_CAP_IOEVENTFD)) {
return -ENOSYS;
}
--
Regards
QingFeng Hao
Re: [Qemu-devel] [PATCH] Python3 Support for qmp.py
On Sat, Jul 01, 2017 at 12:39:41AM +0530, Ishani Chugh wrote:
> This patch intends to make qmp.py compatible with both python2 and python3.
Please identify the Python 2/3 compatibility issues addressed in the
patch like:
* Python 3 does not have dict.has_key(key), use key in dict instead
* Avoid line-based I/O since Python 2/3 have different character
encoding behavior. Explicitly encode/decode JSON UTF-8.
This explains why code changes are being made.
>
> Signed-off-by: Ishani Chugh
> ---
> scripts/qmp/qmp.py | 66
> +++---
> 1 file changed, 43 insertions(+), 23 deletions(-)
>
> diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
> index 62d3651..9926c36 100644
> --- a/scripts/qmp/qmp.py
> +++ b/scripts/qmp/qmp.py
> @@ -13,18 +13,23 @@ import errno
> import socket
> import sys
>
> +
> class QMPError(Exception):
> pass
Whitespace changes are generally discouraged because they perturb the
code (creating merge conflicts, adding noise to diffs, and obscuring
line change history).
I think you're making them for a good reason here - probably to conform
to Python PEP8 coding style? But I'm not sure because there is
explanation in the commit description.
If you want to reformat this file to meet the PEP8 standard, please do
it as a separate patch.
> @@ -42,6 +47,7 @@ class QEMUMonitorProtocol:
> self.__address = address
> self._debug = debug
> self.__sock = self.__get_sock()
> +self.data = b""
Please follow the variable naming convention: two underscores for
private member fields (i.e. self.__data).
> if server:
> self.__sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
> self.__sock.bind(self.__address)
> @@ -56,23 +62,35 @@ class QEMUMonitorProtocol:
>
> def __negotiate_capabilities(self):
> greeting = self.__json_read()
> -if greeting is None or not greeting.has_key('QMP'):
> +if greeting is None or 'QMP' not in greeting:
> raise QMPConnectError
> -# Greeting seems ok, negotiate capabilities
Why remove this comment?
> @@ -87,18 +105,18 @@ class QEMUMonitorProtocol:
> @param wait (bool): block until an event is available.
> @param wait (float): If wait is a float, treat it as a timeout value.
>
> -@raise QMPTimeoutError: If a timeout float is provided and the
> timeout
> -period elapses.
> -@raise QMPConnectError: If wait is True but no events could be
> retrieved
> -or if some other error occurred.
> +@raise QMPTimeoutError: If a timeout float is provided and the
> +timeout period elapses.
> +@raise QMPConnectError: If wait is True but no events could be
> +retrieved or if some other error occurred.
> """
>
> # Check for new events regardless and pull them into the cache:
> self.__sock.setblocking(0)
> try:
> -self.__json_read()
> +test = self.__json_read()
Is 'test' a debug variable that should be removed from the final patch?
> except socket.error as err:
> -if err[0] == errno.EAGAIN:
> +if err.errno == errno.EAGAIN:
> # No data available
> pass
Pre-existing bug: if the exceptin is not EAGAIN we need to raise the
exception again.
> self.__sock.setblocking(1)
> @@ -128,7 +146,7 @@ class QEMUMonitorProtocol:
> @raise QMPCapabilitiesError if fails to negotiate capabilities
> """
> self.__sock.connect(self.__address)
> -self.__sockfile = self.__sock.makefile()
> +self.__sockfile = self.__sock.makefile('rb')
self.__sockfile is no longer used, please delete it.
> if negotiate:
> return self.__negotiate_capabilities()
>
> @@ -143,7 +161,7 @@ class QEMUMonitorProtocol:
> """
> self.__sock.settimeout(15)
> self.__sock, _ = self.__sock.accept()
> -self.__sockfile = self.__sock.makefile()
> +self.__sockfile = self.__sock.makefile('rb')
Same here.
> @@ -245,3 +264,4 @@ class QEMUMonitorProtocol:
>
> def is_scm_available(self):
> return self.__sock.family == socket.AF_UNIX
> +
signature.asc
Description: PGP signature
[Qemu-devel] [PATCH v2 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled
This patch is based on a similar patch from Stefan Hajnoczi -
commit c324fd0a39c (" virtio-pci: use ioeventfd even when KVM is disabled)
Do not check kvm_eventfds_enabled() when KVM is disabled since it
always returns 0. Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
("memory: emulate ioeventfd") it has been possible to use ioeventfds in
qtest or TCG mode.
This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
when KVM is disabled.
I have tested that virtio-scsi-ccw works under tcg both with and without
iothread.
This patch fixes qemu-iotests 068, which was accidentally merged early
despite the dependency on ioeventfd.
Signed-off-by: QingFeng Hao
---
hw/s390x/virtio-ccw.c | 2 +-
target/s390x/kvm.c| 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 90d37cb9ff..35896eb007 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev,
Error **errp)
sch->cssid, sch->ssid, sch->schid, sch->devno,
ccw_dev->devno.valid ? "user-configured" : "auto-configured");
-if (!kvm_eventfds_enabled()) {
+if (kvm_enabled() && !kvm_eventfds_enabled()) {
dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
}
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index a3d00196f4..c37f9c3b9e 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2220,6 +2220,9 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier
*notifier, uint32_t sch,
.addr = sch,
.len = 8,
};
+if (!kvm_enabled()) {
+return 0;
+}
if (!kvm_check_extension(kvm_state, KVM_CAP_IOEVENTFD)) {
return -ENOSYS;
}
--
2.11.2
Re: [Qemu-devel] [PATCH v1 2/3] arm: fix the armv7m reset state
On 3 July 2017 at 08:31, KONRAD Frederic wrote: > On 06/30/2017 11:06 AM, Peter Maydell wrote: >> On 30 June 2017 at 09:24, KONRAD Frederic >> wrote: >>> If I choose (b) I won't be able to load it to SRAM and it is >>> basically the same result I'll need to move or modify the config. >> >> >> I don't understand this, though. Option (b) is probably painful >> to implement (I don't have a good idea of how to do it) but >> it ought to mean that the ELF files that work on the board >> also work for QEMU (regardless of how the board model >> implemented the aliased flash). >> > > Yes that's exactly what I want. > > Basically the 0x alias can point to the SRAM or the ROM > during the reset depending on some boot config. The ELF is > directly loaded in the ROM or in the SRAM and my patch allows to > fetch the two first words in the reset handler to make it work > for any boot config. Yes, but it only works if you implemented it that way round, and not for board implementations which put the real device at 0 and the alias at high memory. I'd like a fix which deals with all of this, not just with the particular arrangement your board implementation has. thanks -- PMM
[Qemu-devel] [PATCH v2 0/1] virtio-scsi-ccw: fix iotest 068 for s390x
This commit fixes iotest 068 for s390x as s390x uses virtio-scsi-ccw. It's based on commit c324fd0a39c by Stefan Hajnoczi. Thanks! Change history: v2: Remove Stefan from sign-off list and change the patch's commit message according to Christian Borntraeger's comment. QingFeng Hao (1): virtio-scsi-ccw: use ioeventfd even when KVM is disabled hw/s390x/virtio-ccw.c | 2 +- target/s390x/kvm.c| 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) -- 2.11.2
Re: [Qemu-devel] [PATCH 0/4] migration: fix iotest 055, only-migratable break
On Mon, Jul 03, 2017 at 03:54:38PM +0800, QingFeng Hao wrote: > I tested the 2 patches and they can make iotest 055 passed on both x86 and > s390x. Thanks for testing! -- Peter Xu
Re: [Qemu-devel] [PATCH v1 2/3] arm: fix the armv7m reset state
On 07/03/2017 10:51 AM, Peter Maydell wrote: On 3 July 2017 at 08:31, KONRAD Frederic wrote: On 06/30/2017 11:06 AM, Peter Maydell wrote: On 30 June 2017 at 09:24, KONRAD Frederic wrote: If I choose (b) I won't be able to load it to SRAM and it is basically the same result I'll need to move or modify the config. I don't understand this, though. Option (b) is probably painful to implement (I don't have a good idea of how to do it) but it ought to mean that the ELF files that work on the board also work for QEMU (regardless of how the board model implemented the aliased flash). Yes that's exactly what I want. Basically the 0x alias can point to the SRAM or the ROM during the reset depending on some boot config. The ELF is directly loaded in the ROM or in the SRAM and my patch allows to fetch the two first words in the reset handler to make it work for any boot config. Yes, but it only works if you implemented it that way round, and not for board implementations which put the real device at 0 and the alias at high memory. I'd like a fix which deals with all of this, not just with the particular arrangement your board implementation has. Ok got it, I'll check if I can do something clean which can handle both ways. Fred thanks -- PMM
Re: [Qemu-devel] [PATCH v1] target-s390x: fix risbg handling
On 01.07.2017 22:27, Richard Henderson wrote:
> On 06/25/2017 03:19 PM, Aurelien Jarno wrote:
>> On 2017-06-23 01:12, David Hildenbrand wrote:
>>> If we have for example: r3 contains 0x
>>> ec 33 3f bf 61 55 risbg %r3,%r3,63,191,97
>>>
>>> We want to rotate 33 to the left and only keep MSB bit 63 of that. So the
>>> result is then exactly 1 (we're reading the sign of the 32 bit value).
>>>
>>> Current code assumes that we can do that via an extract, which is not
>>> true (at least not that easy) and produces a 0.
>>
>> I think the mistake there is that the rotation is done to the left,
>> while in extract the "shift" is done to the right. The following patch
>> should be enough:
>>
>> --- a/target/s390x/translate.c
>> +++ b/target/s390x/translate.c
>> @@ -3441,8 +3441,8 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps
>> *o)
>> }
>>
>> /* In some cases we can implement this with extract. */
>> -if (imask == 0 && pos == 0 && len > 0 && rot + len <= 64) {
>> -tcg_gen_extract_i64(o->out, o->in2, rot, len);
>> +if (imask == 0 && pos == 0 && len > 0 && rot - len >= 0) {
>> +tcg_gen_extract_i64(o->out, o->in2, 64 - rot, len);
>> return NO_EXIT;
>
> Agreed. Included.
>
>
> r~
>
Was able to test it with your version and it works just fine!
Thanks!
--
Thanks,
David
Re: [Qemu-devel] Launching Standalone in QEMU-Microblaze
On Mon, Jul 03, 2017 at 07:23:23AM +, Ormaetxea Xabier wrote:
> Hello!
Hi!
>
> I'm trying to run QEMU-Microblaze (Little-endian) with a standalone app in
> some different ways, but none of them works for me:
>
>
> 1) I have created my own .DTB from my system design (.HDF), just a
> microblaze connected to the Uartlite AXI, leds, interrupt controller, and a
> gpio. Using the board support package I've made a simple c-based program (at
> the end of the message):
>
> Run it with ./qemu-system-microblazeel -M microblaze-fdt-plnx -dtb
> system-top.dtb -kernel fibonacci.elf (-s -S) #(-s -S just to debug it)
>
> Invalid MicroBlaze version number: (null) #Don't think it's a
> problem
> Bar ram offset 9000528f
> Aborted (core dumped)
This indicates that you've linked your standalone app to some RAM you expect
to exist at around 0x90005000, but there's none.
You'll need to check that you are linking your standalone program correctly.
The baremetal BSP needs to match the HW.
>
>
> 2) I use a mb.dtb that I found on the internet. Same c-based program.
> Run it with the debugger:
>
> ./qemu-system-microblazeel -M microblaze-fdt-plnx -dtb mb.dtb -kernel
> fibonacci.elf (-s -S)
>
> Lets my debug it (doesn't fail booting), but in the first step:
> Bad ram pointer
>
> I suppose this isn't my option cause the .dtb is created for another
> microblaze-based design.
>
>
> 3) I use the same design (.HDF), but run it for the Spartan 3a dsp 1800
> option:
>
> ./qemu-system-microblazeel -kernel fibonacci.elf (-s -S)
>
> Runs "properly" -> I mean properly because I can follow on the debugger that
> the steps are correctly made:
> _start -> _start1 -> main -> fibonacci -> xil_printf -> xil_printf ... ->
> xil_printf -> _exit
>
> But doesn't print nothing. And doesn't write in memory as asked (*addrPtr =
> 0x150) :
> In the qemu shell:
> (qemu) x 0xC000
> C000: 0x#When it should be 0x0150
Try adding the -serial stdio commandline option to QEMU.
>
>
> 4) Modifying my system design to be similar to the Spartan board design:
> MEMORY_BASEADDR 0x9000
> FLASH_BASEADDR 0xa000
> INTC_BASEADDR 0x8180
> TIMER_BASEADDR 0x83c0
> UARTLITE_BASEADDR 0x8400
> ETHLITE_BASEADDR 0x8100
> I get exactly the same result as in the (3) case.
>
> So here they go my questions:
>
> - Am I doing it right? Is this the method of running a standalone
> program over a microblaze?
Kind of but I get the impression that your bare-metal application is targeting
different hardware than what you are instructing QEMU to create.
>
> - How can I make the program print something?
>
> - Im not sure if the problem is that it doesn't write on memory, or
> I am the one who fails reading it from the shell, because if I change the
> writing position to (0x8408, uart status position) I get the error (qemu:
> hardware error: write to UART STATUS?) -> that means im writing (or trying,
> at least). How can I write on memory (and read after it, so I know it works)?
I didn't quite understand why you wrote to 0xC000?
The spartan design in QEMU has nothing at that address.
Try 0x9000 instead.
Best regards,
Edgar
>
> Thank you in advance! Really appreciate your Job! (I'm sorry if my problem
> it's a simple one, I'm new at it)
>
> #include
> #include "platform.h"
> #include "xil_printf.h"
>
> void fibonacci(int d){
>u32 a=1;
>u32 b=0;
>u32 em=0;
>u32 *addrPtr = 0xC000;
>
> for(int num=0; numfor(int i=0; i}
>for(int c=0; c}
>em = a+b;
>xil_printf(em);
>*addrPtr = 0x150;
>addrPtr+=1;
>b=a;
>a=em;
>}
> }
>
>
> int main()
> {
> init_platform();
> int i=35;
> fibonacci(i);
> cleanup_platform();
> return 0;
> }
Re: [Qemu-devel] [PATCH] target/ppc: Only set PCR in kvm if actually in a compat mode
On Mon, Jul 03, 2017 at 01:18:38PM +1000, Suraj Jitindar Singh wrote:
> On Fri, 2017-06-30 at 14:03 +1000, David Gibson wrote:
> > On Thu, Jun 29, 2017 at 02:59:39PM +1000, Suraj Jitindar Singh wrote:
> > > The Processor Compatibility Register (PCR) I used to set the
> > > compatibility mode of the processor using the SET_ONE_REG ioctl on
> > > KVM_REG_PPC_ARCH_COMPAT. Previously this was only called when a
> > > compat
> > > mode was actually in use, however a recent patch made it
> > > unconditional.
> > > Calling this in KVM_PR fails as there is no handler for that call
> > > and it
> > > is thus impossible to start a machine with KVM_PR.
> > >
> > > Change ppc_set_compat() so that the ioctl is only actually called
> > > if a
> > > compat mode is in use. This means that a KVM_PR guest can boot.
> > > Additionally the current behaviour for KVM_HV is preserved where a
> > > compat
> > > mode of 0 set pcr and arch_compat in the vcore struct to zero, both
> > > of
> > > which are initialised to zero anyway.
> > >
> > > Fixes: 37f516defa2e ("pseries: Reset CPU compatibility mode")
> > >
> > > Signed-off-by: Suraj Jitindar Singh
> >
> > This doesn't seem quite right. With this change, how would we ever
> > turn compatibility mode _off_ (which could happen on reset if nothing
>
> Oh yeah, didn't really think about that.
>
> > else). Really we should add this pseudo-register to KVM PR, although
> > I'm fine with also having a qemu workaround to let it work with older
> > PR versions.
>
> How do you feel about having a check and only calling the ioctl if the
> KVM in use is HV?
Don't really like it. For one thing, we want to avoid explicitly
checking for KVM PR - we should check specific capabilities instead.
For another, it means on PR we're silently ignoring the compatibility
mode which isn't really right.
I think the right approach here is to only call the ioctl() if the
compatibility mode has actually changed. That should make it work in
the cases the original patch did, which is.. actually very few, given
the new CAS logic.
Really the right fix is to implement the set compat mode ioctl() in
KVM PR.
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH v2] s390: return unavailable features via query-cpu-definitions
>
> +static S390CPUModel *get_max_cpu_model(Error **errp);
> +
> #ifndef CONFIG_USER_ONLY
> +static void list_add_feat(const char *name, void *opaque);
Wonder if we should declare all these prototypes at the beginning of the
file.
> +
> +static void check_unavailable_features(const S390CPUModel *max_model,
> + const S390CPUModel *model,
> + strList **unavailable)
> +{
> +S390FeatBitmap missing;
> +
> +/* check general model compatibility */
> +if (max_model->def->gen < model->def->gen ||
> +(max_model->def->gen == model->def->gen &&
> + max_model->def->ec_ga < model->def->ec_ga)) {
> +list_add_feat("type", unavailable);
> +}
> +
> +/* detect missing features if any to properly report them */
> +bitmap_andnot(missing, model->features, max_model->features,
> + S390_FEAT_MAX);
> +if (!bitmap_empty(missing, S390_FEAT_MAX)) {
> +s390_feat_bitmap_to_ascii(missing,
> + unavailable,
> + list_add_feat);
This certainly fits into one line.
> +}
> +}
> +
> +struct CpuDefinitionInfoListData {
> +CpuDefinitionInfoList *list;
> +Error **errp;
> +};
> +
> static void create_cpu_model_list(ObjectClass *klass, void *opaque)
> {
> -CpuDefinitionInfoList **cpu_list = opaque;
> +struct CpuDefinitionInfoListData *cpu_list_data = opaque;
> +CpuDefinitionInfoList **cpu_list = &cpu_list_data->list;
> CpuDefinitionInfoList *entry;
> CpuDefinitionInfo *info;
> char *name = g_strdup(object_class_get_name(klass));
> S390CPUClass *scc = S390_CPU_CLASS(klass);
> +Object *obj;
> +S390CPU *sc;
> +S390CPUModel *scm;
>
> /* strip off the -s390-cpu */
> g_strrstr(name, "-" TYPE_S390_CPU)[0] = 0;
> @@ -300,21 +336,33 @@ static void create_cpu_model_list(ObjectClass *klass,
> void *opaque)
> info->migration_safe = scc->is_migration_safe;
> info->q_static = scc->is_static;
> info->q_typename = g_strdup(object_class_get_name(klass));
> -
> +/* check for unavailable features */
> +obj = object_new(object_class_get_name(klass));
> +sc = S390_CPU(obj);
> +scm = get_max_cpu_model(cpu_list_data->errp);
H, if this function fails, we will create the same error multiple
times (as there is no way to stop this function from iterating). And we
will fail to create a cpu model list in case there is no host cpu model,
which is a change in behavior (as we will report an error).
Would it be better to simply get the max model in
arch_query_cpu_definitions() and pass it via CpuDefinitionInfoListData,
instead of the errp variable?
Then you could simply skip the checks and set
info->has_unavailable_features = false in case there is no max model
(get_max_cpu_model() returned NULL / an error). (same behavior as for now)
Errors from get_max_cpu_model() then should be ignored and not reported.
> +if (scm && sc->model) {
> +info->has_unavailable_features = true;
> +check_unavailable_features(scm, sc->model,
> &info->unavailable_features);
> +}
>
> entry = g_malloc0(sizeof(*entry));
> entry->value = info;
> entry->next = *cpu_list;
> *cpu_list = entry;
> +object_unref(obj);
> }
>
> CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
> {
> -CpuDefinitionInfoList *list = NULL;
> +struct CpuDefinitionInfoListData list_data = {
> +.list = NULL,
> +.errp = errp,
> +};
>
> -object_class_foreach(create_cpu_model_list, TYPE_S390_CPU, false, &list);
> +object_class_foreach(create_cpu_model_list, TYPE_S390_CPU, false,
> + &list_data);
>
> -return list;
> +return list_data.list;
> }
>
> static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo
> *info,
>
--
Thanks,
David
Re: [Qemu-devel] [RFC v2 1/1] target/ppc/cpu-models: set POWER9_v1.0 as POWER9 DD1
On 03.07.2017 09:45, Laurent Vivier wrote:
> CPU_POWERPC_POWER9_DD1 is 0x004E0100, so this is the POWER9 v1.0.
>
> When we run qemu on a POWER9 DD1 host, we must use either
> "-cpu host" or "-cpu POWER9", but in the latter case it fails with
>
> Unable to find sPAPR CPU Core definition
>
> because POWER9 DD1 doesn't appear in the list of known CPUs.
>
> This patch fixes this by defining POWER9_v1.0 with POWER9 DD1
> PVR instead of CPU_POWERPC_POWER9_BASE. It also adds POWER_v2.0
> with POWER9 DD2 PVR to avoid to trigger kernel POWER9 DD1 workaround
> in TCG mode.
>
> Signed-off-by: Laurent Vivier
> ---
> hw/ppc/spapr_cpu_core.c | 5 -
> target/ppc/cpu-models.c | 6 --
> target/ppc/cpu-models.h | 1 +
> 3 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 9fb896b..00918a5 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -249,8 +249,11 @@ static const char *spapr_core_models[] = {
> /* POWER8NVL */
> "POWER8NVL_v1.0",
>
> -/* POWER9 */
> +/* POWER9 DD1 */
> "POWER9_v1.0",
> +
> +/* POWER9 DD2 */
> +"POWER9_v2.0",
> };
Cosmetical nit: Do we really need two comments for this? I'd maybe list
both entries below a "/* POWER9 */" comment instead (similar to the "/*
970MP variants */" section in that file).
Apart from that, the patch looks good to me.
Thomas
Re: [Qemu-devel] [PATCHv7 5/6] fw_cfg: move qdev_init_nofail() from fw_cfg_init1() to callers
On Thu, 29 Jun 2017 15:07:19 +0100
Mark Cave-Ayland wrote:
> When looking to instantiate a TYPE_FW_CFG_MEM or TYPE_FW_CFG_IO device to be
> able to wire it up differently, it is much more convenient for the caller to
> instantiate the device and have the fw_cfg default files already preloaded
> during realize.
>
> Move fw_cfg_init1() to the end of both the fw_cfg_mem_realize() and
> fw_cfg_io_realize() functions so it no longer needs to be called manually
> when instantiating the device, and also rename it to fw_cfg_common_realize()
> which better describes its new purpose.
>
> Since it is now the responsibility of the machine to wire up the fw_cfg device
> it is necessary to introduce a object_property_add_child() call into
> fw_cfg_init_io() and fw_cfg_init_mem() to link the fw_cfg device to the root
> machine object as before.
>
> Finally we can now convert the asserts() preventing multiple fw_cfg devices
> and unparented fw_cfg devices being instantiated and replace them with proper
> error reporting at realize time. This allows us to remove FW_CFG_NAME and
> FW_CFG_PATH since they are no longer required.
>
> Signed-off-by: Mark Cave-Ayland
> ---
> hw/nvram/fw_cfg.c | 41 +
> 1 file changed, 29 insertions(+), 12 deletions(-)
>
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 2291121..31029ac 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -37,9 +37,6 @@
>
> #define FW_CFG_FILE_SLOTS_DFLT 0x20
>
> -#define FW_CFG_NAME "fw_cfg"
> -#define FW_CFG_PATH "/machine/" FW_CFG_NAME
> -
> #define TYPE_FW_CFG "fw_cfg"
> #define TYPE_FW_CFG_IO "fw_cfg_io"
> #define TYPE_FW_CFG_MEM "fw_cfg_mem"
> @@ -920,19 +917,22 @@ static int fw_cfg_unattached_at_realize(void)
> }
>
>
> -static void fw_cfg_init1(DeviceState *dev)
> +static void fw_cfg_common_realize(DeviceState *dev, Error **errp)
> {
> FWCfgState *s = FW_CFG(dev);
> MachineState *machine = MACHINE(qdev_get_machine());
> uint32_t version = FW_CFG_VERSION;
>
> -assert(!object_resolve_path(FW_CFG_PATH, NULL));
> -
> -object_property_add_child(OBJECT(machine), FW_CFG_NAME, OBJECT(s), NULL);
> -
> -qdev_init_nofail(dev);
> +if (!fw_cfg_find()) {
maybe add comment that here, that fw_cfg_find() will return NULL if more than
1 device is found.
> +error_setg(errp, "at most one %s device is permitted", TYPE_FW_CFG);
s/TYPE_FW_CFG/object_get_typename()/
so it would print leaf type name
> +return;
> +}
>
> -assert(!fw_cfg_unattached_at_realize());
> +if (fw_cfg_unattached_at_realize()) {
as I pointed out in v6, this condition will always be false,
I suggest to drop 4/6 patch and this hunk here so it won't to confuse
readers with assumption that condition might succeed.
> +error_setg(errp, "%s device must be added as a child device before "
> + "realize", TYPE_FW_CFG);
> +return;
> +}
>
> fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (char *)"QEMU", 4);
> fw_cfg_add_bytes(s, FW_CFG_UUID, &qemu_uuid, 16);
> @@ -965,7 +965,9 @@ FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t
> dma_iobase,
> qdev_prop_set_bit(dev, "dma_enabled", false);
> }
>
> -fw_cfg_init1(dev);
> +object_property_add_child(OBJECT(qdev_get_machine()), TYPE_FW_CFG,
> + OBJECT(dev), NULL);
> +qdev_init_nofail(dev);
>
> sbd = SYS_BUS_DEVICE(dev);
> ios = FW_CFG_IO(dev);
> @@ -1003,7 +1005,9 @@ FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr,
> qdev_prop_set_bit(dev, "dma_enabled", false);
> }
>
> -fw_cfg_init1(dev);
> +object_property_add_child(OBJECT(qdev_get_machine()), TYPE_FW_CFG,
> + OBJECT(dev), NULL);
> +qdev_init_nofail(dev);
>
> sbd = SYS_BUS_DEVICE(dev);
> sysbus_mmio_map(sbd, 0, ctl_addr);
> @@ -1033,6 +1037,7 @@ FWCfgState *fw_cfg_find(void)
> return FW_CFG(object_resolve_path_type("", TYPE_FW_CFG, NULL));
> }
>
> +
> static void fw_cfg_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -1104,6 +1109,12 @@ static void fw_cfg_io_realize(DeviceState *dev, Error
> **errp)
>&fw_cfg_dma_mem_ops, FW_CFG(s), "fwcfg.dma",
>sizeof(dma_addr_t));
> }
> +
> +fw_cfg_common_realize(dev, &local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
> }
>
> static void fw_cfg_io_class_init(ObjectClass *klass, void *data)
> @@ -1170,6 +1181,12 @@ static void fw_cfg_mem_realize(DeviceState *dev, Error
> **errp)
>sizeof(dma_addr_t));
> sysbus_init_mmio(sbd, &FW_CFG(s)->dma_iomem);
> }
> +
> +fw_cfg_common_realize(dev, &local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
> }
>
>
Re: [Qemu-devel] [PATCHv7 3/6] fw_cfg: switch fw_cfg_find() to locate the fw_cfg device by type rather than path
On Thu, 29 Jun 2017 15:07:17 +0100
Mark Cave-Ayland wrote:
> This will enable the fw_cfg device to be placed anywhere within the QOM tree
> regardless of its machine location.
>
> Signed-off-by: Mark Cave-Ayland
> ---
> hw/nvram/fw_cfg.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 99bdbc2..0fe7404 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -1017,7 +1017,7 @@ FWCfgState *fw_cfg_init_mem(hwaddr ctl_addr, hwaddr
> data_addr)
>
> FWCfgState *fw_cfg_find(void)
> {
> -return FW_CFG(object_resolve_path(FW_CFG_PATH, NULL));
> +return FW_CFG(object_resolve_path_type("", TYPE_FW_CFG, NULL));
I insist on using ambiguous argument
see why it's needed
https://www.mail-archive.com/qemu-devel@nongnu.org/msg460692.html
> }
>
> static void fw_cfg_class_init(ObjectClass *klass, void *data)
[Qemu-devel] handling emulation fine-grained memory protection
For the ARM v7M microcontrollers we currently treat their memory protection unit like a funny kind of MMU that only has a 1:1 address mapping. This basically works but it means that we can only support protection regions which are a multiple of 1K in size and on a 1K address boundary (because that's what we define as the "page size" for it). The real hardware lets you define protection regions on a granularity down to 64 bytes (both size and address). So far we've got away with this, but I think only because the payloads we've tested haven't really used the MPU much or at all. With v8M I expect the MPU (and its secure/non-secure cousin the Security Attribution Unit) to be much more heavily used, so it would be nice if we could lift this limitation somehow. Does anybody have any good ideas for how this ought to be done? We could wind down the "page size" for these CPUs (since we now have runtime-configurable-page-size for ARM CPUs this shouldn't compromise the A profile cores which can stick to 1K or 4K pages) but I don't think we can get down as low as 64 bytes due to all the things we keep in the low bits of TLB entries. I'm guessing we'd need to have "this page has fine grained protection regions" imply "take the slow path" and then do the protection check in the slow path. Alex Graf pointed out to me a while back that we already have a data structure for handling sub-page-sized things in the slow path (the subpage handling in the memory system), but can we easily (or otherwise) use it, or would it be simpler just to have a separate thing? There are probably some awkward corner cases too, for instance translate.c code would have to cope with the fact that it would now be possible for the first address in a page to be executable but later addresses in the same page not be executable... Any thoughts/ideas/suggestions? thanks -- PMM
Re: [Qemu-devel] [PULL 0/3] Queued TCG patches
On 30 June 2017 at 20:09, Richard Henderson wrote: > None of my TCGTemp patches for now; I'm still trying to understand > how they might (or might not) conflict with multi-threaded code gen > for TCG. > > > r~ > > > The following changes since commit 82d76dc7fc19a5eb9f731d7faed1792bb97214e0: > > Merge remote-tracking branch 'remotes/famz/tags/block-pull-request' into > staging (2017-06-30 16:29:51 +0100) > > are available in the git repository at: > > git://github.com/rth7680/qemu.git tags/pull-tcg-20170603 > > for you to fetch changes up to f3ced3c59287dabc253f83f0c70aa4934470c15e: > > tcg: consistently access cpu->tb_jmp_cache atomically (2017-06-30 11:40:59 > -0700) > > > Queued TCG patches > Applied, thanks. -- PMM
[Qemu-devel] [PATCH v2 04/15] tcg: change tcg_enabled()
Change the tcg_enabled() and make sure user build still enable tcg
even x86 softmmu disable tcg.
Signed-off-by: Yang Zhong
---
accel/tcg/tcg-all.c | 2 +-
accel/tcg/translate-all.c | 6 +-
bsd-user/main.c | 1 +
include/qemu-common.h | 7 ++-
linux-user/main.c | 2 +-
5 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index 1b13bc9..979b68a 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -33,7 +33,7 @@
#include "qemu/main-loop.h"
long tcg_tb_size;
-static bool tcg_allowed = true;
+bool tcg_allowed;
#ifndef CONFIG_USER_ONLY
/* mask must never be zero, except for A20 change call */
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index f6ad46b..bc75294 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -802,6 +802,7 @@ static void tb_htable_init(void)
size. */
void tcg_exec_init(unsigned long tb_size)
{
+tcg_allowed = true;
cpu_gen_init();
page_init();
tb_htable_init();
@@ -813,11 +814,6 @@ void tcg_exec_init(unsigned long tb_size)
#endif
}
-bool tcg_enabled(void)
-{
-return tcg_ctx.code_gen_buffer != NULL;
-}
-
/*
* Allocate a new translation block. Flush the translation buffer if
* too many translation blocks or too much generated code.
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 04f95dd..276aea7 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -35,6 +35,7 @@
#include "trace/control.h"
#include "glib-compat.h"
+bool tcg_allowed = true;
int singlestep;
unsigned long mmap_min_addr;
unsigned long guest_base;
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 387ef52..b5adbfa 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -76,8 +76,13 @@ int qemu_openpty_raw(int *aslave, char *pty_name);
sendto(sockfd, buf, len, flags, destaddr, addrlen)
#endif
+extern bool tcg_allowed;
void tcg_exec_init(unsigned long tb_size);
-bool tcg_enabled(void);
+#ifdef CONFIG_TCG
+#define tcg_enabled() (tcg_allowed)
+#else
+#define tcg_enabled() 0
+#endif
void cpu_exec_init_all(void);
void cpu_exec_step_atomic(CPUState *cpu);
diff --git a/linux-user/main.c b/linux-user/main.c
index ad03c9e..a11508a 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -38,7 +38,7 @@
#include "glib-compat.h"
char *exec_path;
-
+bool tcg_allowed = true;
int singlestep;
static const char *filename;
static const char *argv0;
--
1.9.1
[Qemu-devel] [PATCH v2 00/15] add disable-tcg option for x86 build
This patchset rebased from Paolo's below patchset, which was based on QEMU 2.0.50 version. https://github.com/bonzini/qemu/tree/disable-tcg Since qemu-system-x86_64 enabled kvm and TCG accelators by default, in fact, the TCG accelator is NOT useful in the system build of x86 platform. This patchset will disable TCG for x86 platform if --disable-tcg option is added into ./configure command. The new configure build command like below (1)./configure tcg is defaultly enabled (2)./configure --disable-tcg --target-list=x86_64-softmmu tcg is disabled in x86_64-softmmu (3)./configure --disable-tcg --target-list=i386-softmmu tcg is disabled in i386-softmmu How to verify disable-tcg option (1)./configure a)all user and softmmu build are okay. b) CONFIG_TCG=y is set in $config_target_mak in all user and softmmu directory c)kvm starting vm is okay ./qemu-system-x86_64 -enable-kvm -cpu host -m 2G -smp cpus=4,cores=4,threads=1,\ sockets=1 -drive format=raw,file=eywa.img,index=0,media=disk -nographic -serial \ stdio -nodefaults d)tcg starting vm is okay ./qemu-system-x86_64 -m 2G -smp cpus=4,cores=4,threads=1,sockets=1 -drive format=raw,\ file=eywa.img,index=0,media=disk -nographic -serial stdio -nodefaults (2)./configure --disable-tcg --target-list=x86_64-softmmu a) softmmu build is okay. b) CONFIG_TCG=y is not set in $config_target_mak in x86_64-softmmu directory c) kvm starting vm is okay ./qemu-system-x86_64 -enable-kvm -cpu host -m 2G -smp cpus=4,cores=4,threads=1,\ sockets=1 -drive format=raw,file=eywa.img,index=0,media=disk -nographic -serial \ stdio -nodefaults d) tcg starting vm is aborted The log as below: "tcg" accelerator not found. No accelerator found! (3)./configure --target-list=x86_64-softmmu,x86_64-linux-user ERROR: The current x86_64-linux-user can't support disable-tcg, only i386-softmmu|x86_64-softmmu support disable-tcg (4)./configure --disable-tcg --target-list=arm-softmmu ERROR: The current arm-softmmu can't support disable-tcg, only i386-softmmu|x86_64-softmmu support disable-tcg v2: * updated patch 01 reviewed by Richard Henderson,Thomas Huth and Paolo. "echo "CONFIG_TCG" >> $config_target_mak" is more clean. add the error configure check for --disable-tcg. new patch is easy to extend to other platform. * updated patch 02 reviewed by Paolo. remove CONFIG_TCG. add tcg_enabled() for tcg code. * updated patch 03 reviewed by Paolo. move tcg_handle_interrupt() from translate-common.c to tcg-all.c. * updated patch 09 reviewed by Paolo and Richard Henderson. moved the x86 related function out of stubs file. added patch 14 for x86 related tcg functions. * updated the flush_icache_range() reviewed by Paolo. deleted this patch and the compile is still okay. So there will not this patch in future release. * updated patch 12 reviewed by Richard Henderson. removed the CONFIG_TCG in CPUX86State struct. cpu-defs.h has disabled CPU_COMMON_TLB if tcg is disabled. Yang Zhong (15): configure: add the disable-tcg option vl: add tcg_enabled() for tcg related code tcg: tcg_handle_interrupt() function tcg: change tcg_enabled() tcg: move page_size_init() function kvmvapic: remove tcg related code tcg: move cpu_sync_bndcs_hflags() function tcg: make cpu_get_fp80()/cpu_set_fp80() static tcg: add the tcg-stub.c file into accel/stubs/ tcg: move tb related lock functions tcg: split cpu_set_mxcsr() and make cpu_set_fpuc() inline tcg: disable tcg in CPUX86State struct tcg: add the CONFIG_TCG for header tcg: add the tcg_enabled() in target/i386/ tcg: add the CONFIG_TCG into Makefiles Makefile.target | 4 +- accel/Makefile.objs | 2 +- accel/stubs/Makefile.objs| 1 + accel/stubs/tcg-stub.c | 94 accel/tcg/Makefile.objs | 2 +- accel/tcg/cpu-exec.c | 1 + accel/tcg/cputlb.c | 1 + accel/tcg/tcg-all.c | 36 - accel/tcg/translate-all.c| 24 +-- accel/tcg/translate-all.h| 3 ++ accel/tcg/translate-common.c | 56 -- bsd-user/main.c | 1 + configure| 43 +++- exec.c | 20 ++ hw/i386/kvmvapic.c | 24 --- include/exec/cpu-defs.h | 4 +- include/exec/cputlb.h| 2 +- include/exec/exec-all.h | 53 + include/exec/helper-proto.h | 2 + include/qemu-common.h| 7 +++- include/sysemu/accel.h | 2 +- linux-user/main.c| 2 +- qom/cpu.c| 2 + target/i386/Makefile.objs| 7 ++-- target/i386/bpt_helper.c | 26 +--- target/i386/cpu.c| 4 +- target/i386/cpu.h| 22 +-- target/i386/fpu_helper.c | 29 +- target/i386/helper.c | 3
[Qemu-devel] [PATCH v2 05/15] tcg: move page_size_init() function
translate-all.c will be disabled if tcg is disabled in the build,
so page_size_init() function and related variables will be moved
to exec.c file.
Signed-off-by: Yang Zhong
---
accel/tcg/translate-all.c | 18 --
exec.c| 20
2 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index bc75294..188f5df 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -112,9 +112,6 @@ typedef struct PageDesc {
#define V_L2_BITS 10
#define V_L2_SIZE (1 << V_L2_BITS)
-uintptr_t qemu_host_page_size;
-intptr_t qemu_host_page_mask;
-
/*
* L1 Mapping properties
*/
@@ -363,21 +360,6 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t retaddr)
return r;
}
-void page_size_init(void)
-{
-/* NOTE: we can always suppose that qemu_host_page_size >=
- TARGET_PAGE_SIZE */
-qemu_real_host_page_size = getpagesize();
-qemu_real_host_page_mask = -(intptr_t)qemu_real_host_page_size;
-if (qemu_host_page_size == 0) {
-qemu_host_page_size = qemu_real_host_page_size;
-}
-if (qemu_host_page_size < TARGET_PAGE_SIZE) {
-qemu_host_page_size = TARGET_PAGE_SIZE;
-}
-qemu_host_page_mask = -(intptr_t)qemu_host_page_size;
-}
-
static void page_init(void)
{
page_size_init();
diff --git a/exec.c b/exec.c
index 42ad1ea..ee61915 100644
--- a/exec.c
+++ b/exec.c
@@ -118,6 +118,11 @@ __thread CPUState *current_cpu;
2 = Adaptive rate instruction counting. */
int use_icount;
+uintptr_t qemu_host_page_size;
+intptr_t qemu_host_page_mask;
+uintptr_t qemu_real_host_page_size;
+intptr_t qemu_real_host_page_mask;
+
bool set_preferred_target_page_bits(int bits)
{
/* The target page size is the lowest common denominator for all
@@ -3590,3 +3595,18 @@ err:
}
#endif
+
+void page_size_init(void)
+{
+/* NOTE: we can always suppose that qemu_host_page_size >=
+ TARGET_PAGE_SIZE */
+qemu_real_host_page_size = getpagesize();
+qemu_real_host_page_mask = -(intptr_t)qemu_real_host_page_size;
+if (qemu_host_page_size == 0) {
+qemu_host_page_size = qemu_real_host_page_size;
+}
+if (qemu_host_page_size < TARGET_PAGE_SIZE) {
+qemu_host_page_size = TARGET_PAGE_SIZE;
+}
+qemu_host_page_mask = -(intptr_t)qemu_host_page_size;
+}
--
1.9.1
[Qemu-devel] [PATCH v2 01/15] configure: add the disable-tcg option
Add the disable-tcg option into configure and echo CONFIG_TCG=y into
$config_target_mak. The default tcg is enabled for all build, only i386
and x86_64 softmmu option can be disabled. This operation do not make
big change with the older build command.
The new configure build command like below
(1)./configure
tcg is defaultly enabled
(2)./configure --disable-tcg --target-list=x86_64-softmmu
tcg is disabled in x86_64-softmmu
(3)./configure --disable-tcg --target-list=i386-softmmu
tcg is disabled in i386-softmmu
If the --target-list include other softmmus or user options, the configure
command will report error and configure is aborted.
The error as:
ERROR: The current aarch64-softmmu can't support disable-tcg,
only i386-softmmu|x86_64-softmmu support disable-tcg
or
ERROR: The user build can't support disable-tcg,
only i386-softmmu|x86_64-softmmu support disable-tcg
Signed-off-by: Yang Zhong
---
configure | 43 ++-
1 file changed, 42 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index c571ad1..61ce514 100755
--- a/configure
+++ b/configure
@@ -224,6 +224,7 @@ cap_ng=""
attr=""
libattr=""
xfs=""
+tcg="yes"
vhost_net="no"
vhost_scsi="no"
@@ -953,6 +954,10 @@ for opt do
;;
--enable-hax) hax="yes"
;;
+ --disable-tcg) tcg="no"
+ ;;
+ --enable-tcg) tcg="yes"
+ ;;
--disable-tcg-interpreter) tcg_interpreter="no"
;;
--enable-tcg-interpreter) tcg_interpreter="yes"
@@ -1715,6 +1720,24 @@ case " $target_list " in
;;
esac
+if test "$tcg" = "no"; then
+ for target in $target_list; do
+ if test "$softmmu" = "yes"; then
+case $target in
+ i386-softmmu|x86_64-softmmu)
+ ;;
+*)
+ error_exit "The current $target can't support disable-tcg,"\
+ "only i386-softmmu|x86_64-softmmu support disable-tcg"
+ ;;
+esac
+ else
+error_exit "The user build can't support disable-tcg,"\
+ "only i386-softmmu|x86_64-softmmu support disable-tcg"
+ fi
+ done
+fi
+
feature_not_found() {
feature=$1
remedy=$2
@@ -5119,7 +5142,6 @@ echo "module support$modules"
echo "host CPU $cpu"
echo "host big endian $bigendian"
echo "target list $target_list"
-echo "tcg debug enabled $debug_tcg"
echo "gprof enabled $gprof"
echo "sparse enabled$sparse"
echo "strip binaries$strip_opt"
@@ -5173,6 +5195,11 @@ echo "Linux AIO support $linux_aio"
echo "ATTR/XATTR support $attr"
echo "Install blobs $blobs"
echo "KVM support $kvm"
+echo "TCG support $tcg"
+if test "$tcg" = "yes" ; then
+echo "TCG debug enabled $debug_tcg"
+echo "TCG interpreter $tcg_interpreter"
+fi
echo "HAX support $hax"
echo "RDMA support $rdma"
echo "TCG interpreter $tcg_interpreter"
@@ -6231,6 +6258,7 @@ fi
if test "$target_user_only" = "yes" ; then
echo "CONFIG_USER_ONLY=y" >> $config_target_mak
echo "CONFIG_QEMU_INTERP_PREFIX=\"$interp_prefix1\"" >> $config_target_mak
+ echo "CONFIG_TCG=y" >> $config_target_mak
fi
if test "$target_linux_user" = "yes" ; then
echo "CONFIG_LINUX_USER=y" >> $config_target_mak
@@ -6250,6 +6278,19 @@ if test "$target_bsd_user" = "yes" ; then
echo "CONFIG_BSD_USER=y" >> $config_target_mak
fi
+if test "$target_softmmu" = "yes" ; then
+ case "$target_name" in
+ i386|x86_64)
+ if test "$tcg" = "yes" ; then
+ echo "CONFIG_TCG=y" >> $config_target_mak
+ fi
+ ;;
+ *)
+ echo "CONFIG_TCG=y" >> $config_target_mak
+ ;;
+ esac
+fi
+
# generate QEMU_CFLAGS/LDFLAGS for targets
cflags=""
--
1.9.1
[Qemu-devel] [PATCH v2 08/15] tcg: make cpu_get_fp80()/cpu_set_fp80() static
Move cpu_get_fp80()/cpu_set_fp80() from fpu_helper.c to
machine.c because fpu_helper.c will be disabled if tcg is
disabled in the build.
Signed-off-by: Yang Zhong
---
target/i386/cpu.h| 2 --
target/i386/fpu_helper.c | 18 --
target/i386/machine.c| 18 ++
3 files changed, 18 insertions(+), 20 deletions(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index de0551f..8b3b535 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1418,8 +1418,6 @@ int cpu_x86_get_descr_debug(CPUX86State *env, unsigned
int selector,
/* op_helper.c */
/* used for debug or cpu save/restore */
-void cpu_get_fp80(uint64_t *pmant, uint16_t *pexp, floatx80 f);
-floatx80 cpu_set_fp80(uint64_t mant, uint16_t upper);
/* cpu-exec.c */
/* the following helpers are only usable in user mode simulation as
diff --git a/target/i386/fpu_helper.c b/target/i386/fpu_helper.c
index 69ea33a..34fb5fc 100644
--- a/target/i386/fpu_helper.c
+++ b/target/i386/fpu_helper.c
@@ -1539,24 +1539,6 @@ void helper_xsetbv(CPUX86State *env, uint32_t ecx,
uint64_t mask)
raise_exception_ra(env, EXCP0D_GPF, GETPC());
}
-void cpu_get_fp80(uint64_t *pmant, uint16_t *pexp, floatx80 f)
-{
-CPU_LDoubleU temp;
-
-temp.d = f;
-*pmant = temp.l.lower;
-*pexp = temp.l.upper;
-}
-
-floatx80 cpu_set_fp80(uint64_t mant, uint16_t upper)
-{
-CPU_LDoubleU temp;
-
-temp.l.upper = upper;
-temp.l.lower = mant;
-return temp.d;
-}
-
/* MMX/SSE */
/* XXX: optimize by storing fptt and fptags in the static cpu state */
diff --git a/target/i386/machine.c b/target/i386/machine.c
index 8c7a822..53587ae 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -142,6 +142,24 @@ typedef struct x86_FPReg_tmp {
uint16_t tmp_exp;
} x86_FPReg_tmp;
+static void cpu_get_fp80(uint64_t *pmant, uint16_t *pexp, floatx80 f)
+{
+CPU_LDoubleU temp;
+
+temp.d = f;
+*pmant = temp.l.lower;
+*pexp = temp.l.upper;
+}
+
+static floatx80 cpu_set_fp80(uint64_t mant, uint16_t upper)
+{
+CPU_LDoubleU temp;
+
+temp.l.upper = upper;
+temp.l.lower = mant;
+return temp.d;
+}
+
static void fpreg_pre_save(void *opaque)
{
x86_FPReg_tmp *tmp = opaque;
--
1.9.1
[Qemu-devel] [PATCH v2 02/15] vl: add tcg_enabled() for tcg related code
Need to disable the tcg related code in the vl.c if the
disable-tcg option is added into ./configure command.
Signed-off-by: Yang Zhong
---
accel/tcg/tcg-all.c| 2 +-
include/sysemu/accel.h | 2 +-
vl.c | 15 +++
3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index dba9931..b86c896 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -28,7 +28,7 @@
#include "sysemu/sysemu.h"
#include "qom/object.h"
-int tcg_tb_size;
+long tcg_tb_size;
static bool tcg_allowed = true;
static int tcg_init(MachineState *ms)
diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h
index ecc5c84..7b905b7 100644
--- a/include/sysemu/accel.h
+++ b/include/sysemu/accel.h
@@ -63,7 +63,7 @@ typedef struct AccelClass {
#define ACCEL_GET_CLASS(obj) \
OBJECT_GET_CLASS(AccelClass, (obj), TYPE_ACCEL)
-extern int tcg_tb_size;
+extern long tcg_tb_size;
void configure_accelerator(MachineState *ms);
/* Register accelerator specific global properties */
diff --git a/vl.c b/vl.c
index 36ff3f4..f28c1ac 100644
--- a/vl.c
+++ b/vl.c
@@ -3933,9 +3933,14 @@ int main(int argc, char **argv, char **envp)
configure_rtc(opts);
break;
case QEMU_OPTION_tb_size:
-tcg_tb_size = strtol(optarg, NULL, 0);
-if (tcg_tb_size < 0) {
-tcg_tb_size = 0;
+if (tcg_enabled()) {
+qemu_strtol(optarg, NULL, 0, &tcg_tb_size);
+if (tcg_tb_size < 0) {
+tcg_tb_size = 0;
+}
+} else {
+error_report("TCG is disabled");
+exit(1);
}
break;
case QEMU_OPTION_icount:
@@ -4481,7 +4486,9 @@ int main(int argc, char **argv, char **envp)
qemu_opts_del(icount_opts);
}
-qemu_tcg_configure(accel_opts, &error_fatal);
+if (tcg_enabled()) {
+qemu_tcg_configure(accel_opts, &error_fatal);
+}
if (default_net) {
QemuOptsList *net = qemu_find_opts("net");
--
1.9.1
[Qemu-devel] [PATCH v2 09/15] tcg: add the tcg-stub.c file into accel/stubs/
If tcg is disabled, the functions in tcg-stub.c file will be called.
This file is target-independent file, do not include any platform
related stub functions into this file.
Signed-off-by: Yang Zhong
---
accel/stubs/Makefile.objs | 1 +
accel/stubs/tcg-stub.c| 80 +++
2 files changed, 81 insertions(+)
create mode 100644 accel/stubs/tcg-stub.c
diff --git a/accel/stubs/Makefile.objs b/accel/stubs/Makefile.objs
index bd5794f..fdfbf73 100644
--- a/accel/stubs/Makefile.objs
+++ b/accel/stubs/Makefile.objs
@@ -1 +1,2 @@
obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o
+obj-$(call lnot,$(CONFIG_TCG)) += tcg-stub.o
diff --git a/accel/stubs/tcg-stub.c b/accel/stubs/tcg-stub.c
new file mode 100644
index 000..a9fe40a
--- /dev/null
+++ b/accel/stubs/tcg-stub.c
@@ -0,0 +1,80 @@
+/*
+ * QEMU TCG accelerator stub
+ *
+ * Copyright Red Hat, Inc. 2013
+ *
+ * Author: Paolo Bonzini
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "cpu.h"
+#include "tcg/tcg.h"
+#include "exec/cpu-common.h"
+#include "exec/exec-all.h"
+#include "translate-all.h"
+#include "exec/cpu-all.h"
+
+
+void tb_flush(CPUState *cpu)
+{
+}
+
+void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len)
+{
+}
+
+void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end)
+{
+}
+
+void tb_check_watchpoint(CPUState *cpu)
+{
+}
+
+TranslationBlock *tb_gen_code(CPUState *cpu,
+ target_ulong pc, target_ulong cs_base,
+ uint32_t flags, int cflags)
+{
+return NULL;
+}
+
+void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length)
+{
+}
+
+void tlb_set_dirty(CPUState *cpu, target_ulong vaddr)
+{
+}
+
+bool cpu_restore_state(CPUState *cpu, uintptr_t searched_pc)
+{
+return false;
+}
+
+void dump_exec_info(FILE *f, fprintf_function cpu_fprintf)
+{
+}
+
+void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf)
+{
+}
+
+void cpu_loop_exit(CPUState *cpu)
+{
+abort();
+}
+
+void cpu_loop_exit_noexc(CPUState *cpu)
+{
+abort();
+}
+
+int cpu_exec(CPUState *cpu)
+{
+abort();
+}
--
1.9.1
[Qemu-devel] [PATCH v2 06/15] kvmvapic: remove tcg related code
Since Paolo's below patch has fixed A20 issue
commit bbfa326fc8028e275eddf8c9965c2a1b59405b2e
target/i386: enable A20 automatically in system management mod
The tcg code in kvmvapic.c is NOT useful, those code need remove.
Signed-off-by: Yang Zhong
---
hw/i386/kvmvapic.c | 24
1 file changed, 24 deletions(-)
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 82a4955..5b7be5a 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -18,7 +18,6 @@
#include "sysemu/kvm.h"
#include "hw/i386/apic_internal.h"
#include "hw/sysbus.h"
-#include "tcg/tcg.h"
#define VAPIC_IO_PORT 0x7e
@@ -396,13 +395,9 @@ static void patch_call(VAPICROMState *s, X86CPU *cpu,
target_ulong ip,
static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
{
CPUState *cs = CPU(cpu);
-CPUX86State *env = &cpu->env;
VAPICHandlers *handlers;
uint8_t opcode[2];
uint32_t imm32 = 0;
-target_ulong current_pc = 0;
-target_ulong current_cs_base = 0;
-uint32_t current_flags = 0;
if (smp_cpus == 1) {
handlers = &s->rom_state.up;
@@ -410,17 +405,6 @@ static void patch_instruction(VAPICROMState *s, X86CPU
*cpu, target_ulong ip)
handlers = &s->rom_state.mp;
}
-if (!kvm_enabled()) {
-cpu_get_tb_cpu_state(env, ¤t_pc, ¤t_cs_base,
- ¤t_flags);
-/* Account this instruction, because we will exit the tb.
- This is the first instruction in the block. Therefore
- there is no need in restoring CPU state. */
-if (use_icount) {
---cs->icount_decr.u16.low;
-}
-}
-
pause_all_vcpus();
cpu_memory_rw_debug(cs, ip, opcode, sizeof(opcode), 0);
@@ -455,14 +439,6 @@ static void patch_instruction(VAPICROMState *s, X86CPU
*cpu, target_ulong ip)
}
resume_all_vcpus();
-
-if (!kvm_enabled()) {
-/* Both tb_lock and iothread_mutex will be reset when
- * longjmps back into the cpu_exec loop. */
-tb_lock();
-tb_gen_code(cs, current_pc, current_cs_base, current_flags, 1);
-cpu_loop_exit_noexc(cs);
-}
}
void vapic_report_tpr_access(DeviceState *dev, CPUState *cs, target_ulong ip,
--
1.9.1
[Qemu-devel] [PATCH v2 03/15] tcg: tcg_handle_interrupt() function
Move tcg_handle_interrupt() from translate-common.c to
accel/tcg/tcg-all.c.
Signed-off-by: Yang Zhong
---
accel/tcg/Makefile.objs | 2 +-
accel/tcg/tcg-all.c | 32 +
accel/tcg/translate-common.c | 56
qom/cpu.c| 2 ++
4 files changed, 35 insertions(+), 57 deletions(-)
delete mode 100644 accel/tcg/translate-common.c
diff --git a/accel/tcg/Makefile.objs b/accel/tcg/Makefile.objs
index f173cd5..70cd474 100644
--- a/accel/tcg/Makefile.objs
+++ b/accel/tcg/Makefile.objs
@@ -1,3 +1,3 @@
obj-$(CONFIG_SOFTMMU) += tcg-all.o
obj-$(CONFIG_SOFTMMU) += cputlb.o
-obj-y += cpu-exec.o cpu-exec-common.o translate-all.o translate-common.o
+obj-y += cpu-exec.o cpu-exec-common.o translate-all.o
diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index b86c896..1b13bc9 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -27,13 +27,45 @@
#include "sysemu/accel.h"
#include "sysemu/sysemu.h"
#include "qom/object.h"
+#include "qemu-common.h"
+#include "qom/cpu.h"
+#include "sysemu/cpus.h"
+#include "qemu/main-loop.h"
long tcg_tb_size;
static bool tcg_allowed = true;
+#ifndef CONFIG_USER_ONLY
+/* mask must never be zero, except for A20 change call */
+static void tcg_handle_interrupt(CPUState *cpu, int mask)
+{
+int old_mask;
+g_assert(qemu_mutex_iothread_locked());
+
+old_mask = cpu->interrupt_request;
+cpu->interrupt_request |= mask;
+
+/*
+ * If called from iothread context, wake the target cpu in
+ * case its halted.
+ */
+if (!qemu_cpu_is_self(cpu)) {
+qemu_cpu_kick(cpu);
+} else {
+cpu->icount_decr.u16.high = -1;
+if (use_icount &&
+!cpu->can_do_io
+&& (mask & ~old_mask) != 0) {
+cpu_abort(cpu, "Raised interrupt while not in I/O function");
+}
+}
+}
+#endif
+
static int tcg_init(MachineState *ms)
{
tcg_exec_init(tcg_tb_size * 1024 * 1024);
+cpu_interrupt_handler = tcg_handle_interrupt;
return 0;
}
diff --git a/accel/tcg/translate-common.c b/accel/tcg/translate-common.c
deleted file mode 100644
index 40fe5a1..000
--- a/accel/tcg/translate-common.c
+++ /dev/null
@@ -1,56 +0,0 @@
-/*
- * Host code generation common components
- *
- * Copyright (c) 2015 Peter Crosthwaite
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
- *
- * This library 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
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, see
[Qemu-devel] [PATCH v2 15/15] tcg: add the CONFIG_TCG into Makefiles
Add the CONFIG_TCG for frontend and backend's files in the related Makefiles. Signed-off-by: Yang Zhong --- Makefile.target | 4 ++-- accel/Makefile.objs | 2 +- target/i386/Makefile.objs | 7 --- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Makefile.target b/Makefile.target index 0066579..8e185d4 100644 --- a/Makefile.target +++ b/Makefile.target @@ -90,8 +90,8 @@ all: $(PROGS) stap # cpu emulator library obj-y += exec.o obj-y += accel/ -obj-y += tcg/tcg.o tcg/tcg-op.o tcg/optimize.o -obj-y += tcg/tcg-common.o tcg/tcg-runtime.o +obj-$(CONFIG_TCG) += tcg/tcg.o tcg/tcg-op.o tcg/optimize.o +obj-$(CONFIG_TCG) += tcg/tcg-common.o tcg/tcg-runtime.o obj-$(CONFIG_TCG_INTERPRETER) += tcg/tci.o obj-$(CONFIG_TCG_INTERPRETER) += disas/tci.o obj-y += fpu/softfloat.o diff --git a/accel/Makefile.objs b/accel/Makefile.objs index cd5702f..10666ed 100644 --- a/accel/Makefile.objs +++ b/accel/Makefile.objs @@ -1,4 +1,4 @@ obj-$(CONFIG_SOFTMMU) += accel.o obj-y += kvm/ -obj-y += tcg/ +obj-$(CONFIG_TCG) += tcg/ obj-y += stubs/ diff --git a/target/i386/Makefile.objs b/target/i386/Makefile.objs index 4fcb7f3..fee6c7e 100644 --- a/target/i386/Makefile.objs +++ b/target/i386/Makefile.objs @@ -1,6 +1,7 @@ -obj-y += translate.o helper.o cpu.o bpt_helper.o -obj-y += excp_helper.o fpu_helper.o cc_helper.o int_helper.o svm_helper.o -obj-y += smm_helper.o misc_helper.o mem_helper.o seg_helper.o mpx_helper.o +obj-y += helper.o cpu.o cc_helper.o bpt_helper.o +obj-$(CONFIG_TCG) += translate.o +obj-$(CONFIG_TCG) += excp_helper.o fpu_helper.o int_helper.o svm_helper.o +obj-$(CONFIG_TCG) += smm_helper.o misc_helper.o mem_helper.o seg_helper.o mpx_helper.o obj-y += gdbstub.o obj-$(CONFIG_SOFTMMU) += machine.o arch_memory_mapping.o arch_dump.o monitor.o obj-$(CONFIG_KVM) += kvm.o hyperv.o -- 1.9.1
[Qemu-devel] [PATCH v2 14/15] tcg: add the tcg_enabled() in target/i386/
Add the tcg_enabled() to disable tcg code in x86 target.
Signed-off-by: Yang Zhong
---
target/i386/bpt_helper.c | 26 --
target/i386/cpu.c| 4 +++-
target/i386/machine.c| 5 +++--
3 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/target/i386/bpt_helper.c b/target/i386/bpt_helper.c
index b3efdc7..083c712 100644
--- a/target/i386/bpt_helper.c
+++ b/target/i386/bpt_helper.c
@@ -216,7 +216,9 @@ void breakpoint_handler(CPUState *cs)
if (cs->watchpoint_hit->flags & BP_CPU) {
cs->watchpoint_hit = NULL;
if (check_hw_breakpoints(env, false)) {
-raise_exception(env, EXCP01_DB);
+if (tcg_enabled()) {
+raise_exception(env, EXCP01_DB);
+}
} else {
cpu_loop_exit_noexc(cs);
}
@@ -226,7 +228,9 @@ void breakpoint_handler(CPUState *cs)
if (bp->pc == env->eip) {
if (bp->flags & BP_CPU) {
check_hw_breakpoints(env, true);
-raise_exception(env, EXCP01_DB);
+if (tcg_enabled()) {
+raise_exception(env, EXCP01_DB);
+}
}
break;
}
@@ -241,7 +245,9 @@ void helper_single_step(CPUX86State *env)
check_hw_breakpoints(env, true);
env->dr[6] |= DR6_BS;
#endif
-raise_exception(env, EXCP01_DB);
+if (tcg_enabled()) {
+raise_exception(env, EXCP01_DB);
+}
}
void helper_rechecking_single_step(CPUX86State *env)
@@ -282,7 +288,9 @@ void helper_set_dr(CPUX86State *env, int reg, target_ulong
t0)
cpu_x86_update_dr7(env, t0);
return;
}
-raise_exception_err_ra(env, EXCP06_ILLOP, 0, GETPC());
+if (tcg_enabled()) {
+raise_exception_err_ra(env, EXCP06_ILLOP, 0, GETPC());
+}
#endif
}
@@ -304,7 +312,11 @@ target_ulong helper_get_dr(CPUX86State *env, int reg)
return env->dr[7];
}
}
-raise_exception_err_ra(env, EXCP06_ILLOP, 0, GETPC());
+if (tcg_enabled()) {
+raise_exception_err_ra(env, EXCP06_ILLOP, 0, GETPC());
+} else {
+return 0;
+}
}
/* Check if Port I/O is trapped by a breakpoint. */
@@ -329,7 +341,9 @@ void helper_bpt_io(CPUX86State *env, uint32_t port,
if (hit) {
env->dr[6] = (env->dr[6] & ~0xf) | hit;
env->eip = next_eip;
-raise_exception(env, EXCP01_DB);
+if (tcg_enabled()) {
+raise_exception(env, EXCP01_DB);
+}
}
#endif
}
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 642519a..c571772 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4040,8 +4040,10 @@ static void x86_cpu_common_class_init(ObjectClass *oc,
void *data)
cc->class_by_name = x86_cpu_class_by_name;
cc->parse_features = x86_cpu_parse_featurestr;
cc->has_work = x86_cpu_has_work;
+#ifdef CONFIG_TCG
cc->do_interrupt = x86_cpu_do_interrupt;
cc->cpu_exec_interrupt = x86_cpu_exec_interrupt;
+#endif
cc->dump_state = x86_cpu_dump_state;
cc->get_crash_info = x86_cpu_get_crash_info;
cc->set_pc = x86_cpu_set_pc;
@@ -4070,7 +4072,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc,
void *data)
cc->gdb_core_xml_file = "i386-32bit.xml";
cc->gdb_num_core_regs = 41;
#endif
-#ifndef CONFIG_USER_ONLY
+#if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY)
cc->debug_excp_handler = breakpoint_handler;
#endif
cc->cpu_exec_enter = x86_cpu_exec_enter;
diff --git a/target/i386/machine.c b/target/i386/machine.c
index 53587ae..b78b36c 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -280,8 +280,9 @@ static int cpu_post_load(void *opaque, int version_id)
for(i = 0; i < 8; i++) {
env->fptags[i] = (env->fptag_vmstate >> i) & 1;
}
-update_fp_status(env);
-
+if (tcg_enabled()) {
+update_fp_status(env);
+}
cpu_breakpoint_remove_all(cs, BP_CPU);
cpu_watchpoint_remove_all(cs, BP_CPU);
{
--
1.9.1
[Qemu-devel] [PATCH v2 11/15] tcg: split cpu_set_mxcsr() and make cpu_set_fpuc() inline
Split the cpu_set_mxcsr() and make cpu_set_fpuc() inline with specific
tcg code.
Signed-off-by: Yang Zhong
---
target/i386/cpu.h| 18 --
target/i386/fpu_helper.c | 11 ++-
2 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 8b3b535..67a6091 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1643,8 +1643,22 @@ static inline int32_t x86_get_a20_mask(CPUX86State *env)
}
/* fpu_helper.c */
-void cpu_set_mxcsr(CPUX86State *env, uint32_t val);
-void cpu_set_fpuc(CPUX86State *env, uint16_t val);
+void tcg_update_mxcsr(CPUX86State *env);
+static inline void cpu_set_mxcsr(CPUX86State *env, uint32_t mxcsr)
+{
+env->mxcsr = mxcsr;
+if (tcg_enabled()) {
+tcg_update_mxcsr(env);
+}
+}
+
+static inline void cpu_set_fpuc(CPUX86State *env, uint16_t fpuc)
+{
+ env->fpuc = fpuc;
+ if (tcg_enabled()) {
+update_fp_status(env);
+ }
+}
/* mem_helper.c */
void helper_lock_init(void);
diff --git a/target/i386/fpu_helper.c b/target/i386/fpu_helper.c
index 34fb5fc..f0facb9 100644
--- a/target/i386/fpu_helper.c
+++ b/target/i386/fpu_helper.c
@@ -1550,12 +1550,11 @@ void helper_xsetbv(CPUX86State *env, uint32_t ecx,
uint64_t mask)
#define SSE_RC_CHOP 0x6000
#define SSE_FZ 0x8000
-void cpu_set_mxcsr(CPUX86State *env, uint32_t mxcsr)
+void tcg_update_mxcsr(CPUX86State *env)
{
+uint32_t mxcsr = env->mxcsr;
int rnd_type;
-env->mxcsr = mxcsr;
-
/* set rounding mode */
switch (mxcsr & SSE_RC_MASK) {
default:
@@ -1581,12 +1580,6 @@ void cpu_set_mxcsr(CPUX86State *env, uint32_t mxcsr)
set_flush_to_zero((mxcsr & SSE_FZ) ? 1 : 0, &env->fp_status);
}
-void cpu_set_fpuc(CPUX86State *env, uint16_t val)
-{
-env->fpuc = val;
-update_fp_status(env);
-}
-
void helper_ldmxcsr(CPUX86State *env, uint32_t val)
{
cpu_set_mxcsr(env, val);
--
1.9.1
[Qemu-devel] [PATCH v2 07/15] tcg: move cpu_sync_bndcs_hflags() function
Move cpu_sync_bndcs_hflags() function from mpx_helper.c
to helper.c because mpx_helper.c need be disabled when
tcg is disabled.
Signed-off-by: Yang Zhong
---
target/i386/helper.c | 34 +-
target/i386/mpx_helper.c | 30 --
2 files changed, 33 insertions(+), 31 deletions(-)
diff --git a/target/i386/helper.c b/target/i386/helper.c
index ef05059..87fd705 100644
--- a/target/i386/helper.c
+++ b/target/i386/helper.c
@@ -29,6 +29,36 @@
#include "hw/i386/apic_internal.h"
#endif
+void cpu_sync_bndcs_hflags(CPUX86State *env)
+{
+uint32_t hflags = env->hflags;
+uint32_t hflags2 = env->hflags2;
+uint32_t bndcsr;
+
+if ((hflags & HF_CPL_MASK) == 3) {
+bndcsr = env->bndcs_regs.cfgu;
+} else {
+bndcsr = env->msr_bndcfgs;
+}
+
+if ((env->cr[4] & CR4_OSXSAVE_MASK)
+&& (env->xcr0 & XSTATE_BNDCSR_MASK)
+&& (bndcsr & BNDCFG_ENABLE)) {
+hflags |= HF_MPX_EN_MASK;
+} else {
+hflags &= ~HF_MPX_EN_MASK;
+}
+
+if (bndcsr & BNDCFG_BNDPRESERVE) {
+hflags2 |= HF2_MPX_PR_MASK;
+} else {
+hflags2 &= ~HF2_MPX_PR_MASK;
+}
+
+env->hflags = hflags;
+env->hflags2 = hflags2;
+}
+
static void cpu_x86_version(CPUX86State *env, int *family, int *model)
{
int cpuver = env->cpuid_version;
@@ -1302,10 +1332,12 @@ void cpu_report_tpr_access(CPUX86State *env, TPRAccess
access)
env->tpr_access_type = access;
cpu_interrupt(cs, CPU_INTERRUPT_TPR);
-} else {
+} else if (tcg_enabled()) {
cpu_restore_state(cs, cs->mem_io_pc);
apic_handle_tpr_access_report(cpu->apic_state, env->eip, access);
+} else {
+abort();
}
}
#endif /* !CONFIG_USER_ONLY */
diff --git a/target/i386/mpx_helper.c b/target/i386/mpx_helper.c
index 7e44820..ade5d24 100644
--- a/target/i386/mpx_helper.c
+++ b/target/i386/mpx_helper.c
@@ -24,36 +24,6 @@
#include "exec/exec-all.h"
-void cpu_sync_bndcs_hflags(CPUX86State *env)
-{
-uint32_t hflags = env->hflags;
-uint32_t hflags2 = env->hflags2;
-uint32_t bndcsr;
-
-if ((hflags & HF_CPL_MASK) == 3) {
-bndcsr = env->bndcs_regs.cfgu;
-} else {
-bndcsr = env->msr_bndcfgs;
-}
-
-if ((env->cr[4] & CR4_OSXSAVE_MASK)
-&& (env->xcr0 & XSTATE_BNDCSR_MASK)
-&& (bndcsr & BNDCFG_ENABLE)) {
-hflags |= HF_MPX_EN_MASK;
-} else {
-hflags &= ~HF_MPX_EN_MASK;
-}
-
-if (bndcsr & BNDCFG_BNDPRESERVE) {
-hflags2 |= HF2_MPX_PR_MASK;
-} else {
-hflags2 &= ~HF2_MPX_PR_MASK;
-}
-
-env->hflags = hflags;
-env->hflags2 = hflags2;
-}
-
void helper_bndck(CPUX86State *env, uint32_t fail)
{
if (unlikely(fail)) {
--
1.9.1
[Qemu-devel] what is the USB 'maxframes' parameter for and what is its maximum value?
Hi; I've been idly looking through our fairly small number of
remaining Coverity issues, and one of them is in hw/usb/hcd-ehci.c:
in ehci_work_bh() we do:
if (uframes > (ehci->maxframes * 8)) {
but if maxframes is very large then the multiply will overflow
a 32-bit integer. (CID 1375885)
We could shut up Coverity by forcing a 64-bit multiply, but that
doesn't really seem like the right approach -- maxframes is a
user-settable parameter whose default is 128 and values as big
as 2^30 seem like they probably aren't valid. So perhaps we should
handle this by having the device sanity-check the user-provided
value at startup.
Unfortunately there doesn't seem to be any documentation on what
the 'maxframes' property is for or what its valid values are.
Digging through git history this was added in commit 16a2dee6b98
whose commit message briefly says it's the upper limit for how many
frames we process at once, but I'm not familiar enough with
USB to be able to translate that into what the valid range
might be.
thanks
-- PMM
[Qemu-devel] [PATCH v2 10/15] tcg: move tb related lock functions
Move tb_lock()/tb_unlock()/tb_lock_reset() from tcg.h to
translate-all.h. tb_lock()/tb_unlock() need be impelemnted
in accel/stubs/tcg-stub.c.
Signed-off-by: Yang Zhong
---
accel/stubs/tcg-stub.c| 8
accel/tcg/cpu-exec.c | 1 +
accel/tcg/cputlb.c| 1 +
accel/tcg/translate-all.h | 3 +++
tcg/tcg.h | 4
trace/control-target.c| 1 -
6 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/accel/stubs/tcg-stub.c b/accel/stubs/tcg-stub.c
index a9fe40a..a84a6e6 100644
--- a/accel/stubs/tcg-stub.c
+++ b/accel/stubs/tcg-stub.c
@@ -20,6 +20,14 @@
#include "exec/cpu-all.h"
+void tb_lock(void)
+{
+}
+
+void tb_unlock(void)
+{
+}
+
void tb_flush(CPUState *cpu)
{
}
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 3581618..03b4ea2 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -21,6 +21,7 @@
#include "trace.h"
#include "disas/disas.h"
#include "exec/exec-all.h"
+#include "translate-all.h"
#include "tcg.h"
#include "qemu/atomic.h"
#include "sysemu/qtest.h"
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 1900936..c0efd74 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -21,6 +21,7 @@
#include "qemu/main-loop.h"
#include "cpu.h"
#include "exec/exec-all.h"
+#include "translate-all.h"
#include "exec/memory.h"
#include "exec/address-spaces.h"
#include "exec/cpu_ldst.h"
diff --git a/accel/tcg/translate-all.h b/accel/tcg/translate-all.h
index ba8e4d6..4774f78 100644
--- a/accel/tcg/translate-all.h
+++ b/accel/tcg/translate-all.h
@@ -23,6 +23,9 @@
/* translate-all.c */
+void tb_lock(void);
+void tb_unlock(void);
+void tb_lock_reset(void);
void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len);
void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
int is_cpu_write_access);
diff --git a/tcg/tcg.h b/tcg/tcg.h
index 9e37722..da78721 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -757,10 +757,6 @@ void *tcg_malloc_internal(TCGContext *s, int size);
void tcg_pool_reset(TCGContext *s);
TranslationBlock *tcg_tb_alloc(TCGContext *s);
-void tb_lock(void);
-void tb_unlock(void);
-void tb_lock_reset(void);
-
/* Called with tb_lock held. */
static inline void *tcg_malloc(int size)
{
diff --git a/trace/control-target.c b/trace/control-target.c
index 6266e63..2a5efc7 100644
--- a/trace/control-target.c
+++ b/trace/control-target.c
@@ -11,7 +11,6 @@
#include "cpu.h"
#include "trace-root.h"
#include "trace/control.h"
-#include "translate-all.h"
void trace_event_set_state_dynamic_init(TraceEvent *ev, bool state)
--
1.9.1
Re: [Qemu-devel] [PATCH v2 01/15] configure: add the disable-tcg option
On Mon, Jul 03, 2017 at 06:12:09PM +0800, Yang Zhong wrote: > Add the disable-tcg option into configure and echo CONFIG_TCG=y into > $config_target_mak. The default tcg is enabled for all build, only i386 > and x86_64 softmmu option can be disabled. This operation do not make > big change with the older build command. > > The new configure build command like below > (1)./configure >tcg is defaultly enabled > > (2)./configure --disable-tcg --target-list=x86_64-softmmu >tcg is disabled in x86_64-softmmu > > (3)./configure --disable-tcg --target-list=i386-softmmu >tcg is disabled in i386-softmmu > > If the --target-list include other softmmus or user options, the configure > command will report error and configure is aborted. > The error as: > ERROR: The current aarch64-softmmu can't support disable-tcg, > only i386-softmmu|x86_64-softmmu support disable-tcg > or > ERROR: The user build can't support disable-tcg, > only i386-softmmu|x86_64-softmmu support disable-tcg > > Signed-off-by: Yang Zhong > --- > configure | 43 ++- > 1 file changed, 42 insertions(+), 1 deletion(-) > > diff --git a/configure b/configure > index c571ad1..61ce514 100755 > --- a/configure > +++ b/configure > @@ -224,6 +224,7 @@ cap_ng="" > attr="" > libattr="" > xfs="" > +tcg="yes" > > vhost_net="no" > vhost_scsi="no" > @@ -953,6 +954,10 @@ for opt do >;; >--enable-hax) hax="yes" >;; > + --disable-tcg) tcg="no" > + ;; > + --enable-tcg) tcg="yes" > + ;; >--disable-tcg-interpreter) tcg_interpreter="no" >;; >--enable-tcg-interpreter) tcg_interpreter="yes" > @@ -1715,6 +1720,24 @@ case " $target_list " in >;; > esac > > +if test "$tcg" = "no"; then > + for target in $target_list; do > + if test "$softmmu" = "yes"; then > +case $target in > + i386-softmmu|x86_64-softmmu) > + ;; > +*) > + error_exit "The current $target can't support disable-tcg,"\ > + "only i386-softmmu|x86_64-softmmu support disable-tcg" > + ;; This looks too simplistic in its logic. You can disable TCG, if-and-only-if the system emulator supports KVM. KVM is supported on many architectures, not only x86-64 & i386. KVM is only supported if the guest emulator architecture matches the host build target architecture. ie if you are building an x86_64 system emulator on a PPC64 host, then you can't disable TCG. So this needs rewriting to *not* special case x86_64 / i386. Instead you need to compare & match build target / system emulator architectures, across all architectures supporting KVM. 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] [RFC PATCH 7/8] VFIO: Add new IOCTL for IOMMU TLB invalidate propagation
On Fri, May 12, 2017 at 01:11:02PM +0100, Jean-Philippe Brucker wrote:
Hi Jean,
As we've got a few discussions on it. I'd like to have a conclusion and
make it as a reference for future discussion.
Currently, we are inclined to have a hybrid format for the iommu tlb
invalidation from userspace(vIOMMU or userspace driver).
Based on the previous discussion, may the below work?
1. Add a IOCTL for iommu tlb invalidation.
VFIO_IOMMU_TLB_INVALIDATE
struct vfio_iommu_tlb_invalidate {
__u32 argsz;
__u32 length;
__u8data[];
};
comments from Alex William: is it more suitable to add a new flag bit on
vfio_device_svm(a structure defined in patch 5 of this patchset), the data
structure is so similar.
Personally, I'm ok with it. Pls let me know your thoughts. However, the
precondition is we accept the whole definition in this email. If not, the
vfio_iommu_tlb_invalidate would be defined differently.
2. Define a structure in include/uapi/linux/iommu.h(newly added header file)
struct iommu_tlb_invalidate {
__u32 scope;
/* pasid-selective invalidation described by @pasid */
#define IOMMU_INVALIDATE_PASID (1 << 0)
/* address-selevtive invalidation described by (@vaddr, @size) */
#define IOMMU_INVALIDATE_VADDR (1 << 1)
__u32 flags;
/* targets non-pasid mappings, @pasid is not valid */
#define IOMMU_INVALIDATE_NO_PASID (1 << 0)
/* indicating that the pIOMMU doesn't need to invalidate
all intermediate tables cached as part of the PTE for
vaddr, only the last-level entry (pte). This is a hint. */
#define IOMMU_INVALIDATE_VADDR_LEAF (1 << 1)
__u32 pasid;
__u64 vaddr;
__u64 size;
__u8data[];
};
For this part, the scope and flags are basically aligned with your previous
email. I renamed the prefix to be "IOMMU_". In my opinion, the scope and flags
would be filled by vIOMMU emulator and be parsed by underlying iommu driver,
it is much more suitable to be defined in a uapi header file.
Besides the reason above, I don't want VFIO engae too much on the data parsing.
If we move the scope,flags,pasid,vaddr,size fields to vfio_iommu_tlb_invalidate,
then both kernel space vfio and user space vfio needs to do much parsing. So I
may prefer the way above.
If you've got any other idea, pls feel free to post it. It's welcomed.
Thanks,
Yi L
> Hi Yi,
>
> On 26/04/17 11:12, Liu, Yi L wrote:
> > From: "Liu, Yi L"
> >
> > This patch adds VFIO_IOMMU_TLB_INVALIDATE to propagate IOMMU TLB
> > invalidate request from guest to host.
> >
> > In the case of SVM virtualization on VT-d, host IOMMU driver has
> > no knowledge of caching structure updates unless the guest
> > invalidation activities are passed down to the host. So a new
> > IOCTL is needed to propagate the guest cache invalidation through
> > VFIO.
> >
> > Signed-off-by: Liu, Yi L
> > ---
> > include/uapi/linux/vfio.h | 9 +
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> > index 6b97987..50c51f8 100644
> > --- a/include/uapi/linux/vfio.h
> > +++ b/include/uapi/linux/vfio.h
> > @@ -564,6 +564,15 @@ struct vfio_device_svm {
> >
> > #define VFIO_IOMMU_SVM_BIND_TASK _IO(VFIO_TYPE, VFIO_BASE + 22)
> >
> > +/* For IOMMU TLB Invalidation Propagation */
> > +struct vfio_iommu_tlb_invalidate {
> > + __u32 argsz;
> > + __u32 length;
> > + __u8data[];
> > +};
>
> We initially discussed something a little more generic than this, with
> most info explicitly described and only pIOMMU-specific quirks and hints
> in an opaque structure. Out of curiosity, why the change? I'm not against
> a fully opaque structure, but there seem to be a large overlap between TLB
> invalidations across architectures.
>
>
> For what it's worth, when prototyping the paravirtualized IOMMU I came up
> with the following.
>
> (From the paravirtualized POV, the SMMU also has to swizzle endianess
> after unpacking an opaque structure, since userspace doesn't know what's
> in it and guest might use a different endianess. So we need to force all
> opaque data to be e.g. little-endian.)
>
> struct vfio_iommu_tlb_invalidate {
> __u32 argsz;
> __u32 scope;
> __u32 flags;
> __u32 pasid;
> __u64 vaddr;
> __u64 size;
> __u8data[];
> };
>
> Scope is a bitfields restricting the invalidation scope. By default
> invalidate the whole container (all PASIDs and all VAs). @pasid, @vaddr
> and @size are unused.
>
> Adding VFIO_IOMMU_INVALIDATE_PASID (1 << 0) restricts the invalidation
> scope to the pasid described by @pasid.
> Adding VFIO_IOMMU_INVALIDATE_VADDR (1 << 1) restricts the invalidation
> scope to the address range described by (@vaddr, @size).
>
> So setting scope = VFIO_IOMMU_INVALIDATE_VADDR would invalidate the VA
> range for *all* pasids (as well as no_pasid). Setting scope =
> (VFIO_IOMMU_INVALIDATE_VADDR|VFIO_IOMMU_INVALIDATE_PASID) would invalidate
> the
[Qemu-devel] [PATCH v2 12/15] tcg: disable tcg in CPUX86State struct
Add the CONFIG_TCG for CPU_COMMON_TLB in the CPUX86State struct. Signed-off-by: Yang Zhong --- include/exec/cpu-defs.h | 4 +++- target/i386/cpu.h | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h index 5f4e303..bc8e7f8 100644 --- a/include/exec/cpu-defs.h +++ b/include/exec/cpu-defs.h @@ -25,7 +25,9 @@ #include "qemu/host-utils.h" #include "qemu/queue.h" +#ifdef CONFIG_TCG #include "tcg-target.h" +#endif #ifndef CONFIG_USER_ONLY #include "exec/hwaddr.h" #endif @@ -54,7 +56,7 @@ typedef uint64_t target_ulong; #error TARGET_LONG_SIZE undefined #endif -#if !defined(CONFIG_USER_ONLY) +#if !defined(CONFIG_USER_ONLY) && defined(CONFIG_TCG) /* use a fully associative victim tlb of 8 entries */ #define CPU_VTLB_SIZE 8 diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 67a6091..d4b10a3 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -52,7 +52,9 @@ #include "exec/cpu-defs.h" +#ifdef CONFIG_TCG #include "fpu/softfloat.h" +#endif #define R_EAX 0 #define R_ECX 1 -- 1.9.1
[Qemu-devel] [PATCH v2 13/15] tcg: add the CONFIG_TCG for header
Add the CONFIG_TCG for exec-all.h. Since function tlb_set_page_with_attrs()
is defined in ./accel/tcg/cputlb.c, which will be disabled if tcg is disabled.
This function need be implemented in accel/stubs/tcg-stub.c for disable-tcg.
Signed-off-by: Yang Zhong
---
accel/stubs/tcg-stub.c | 6 +
include/exec/cputlb.h | 2 +-
include/exec/exec-all.h | 53 -
include/exec/helper-proto.h | 2 ++
4 files changed, 37 insertions(+), 26 deletions(-)
diff --git a/accel/stubs/tcg-stub.c b/accel/stubs/tcg-stub.c
index a84a6e6..4e9f7fa 100644
--- a/accel/stubs/tcg-stub.c
+++ b/accel/stubs/tcg-stub.c
@@ -64,6 +64,12 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t searched_pc)
return false;
}
+void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr,
+ hwaddr paddr, MemTxAttrs attrs,
+ int prot, int mmu_idx, target_ulong size)
+{
+}
+
void dump_exec_info(FILE *f, fprintf_function cpu_fprintf)
{
}
diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h
index 3f94178..cf296d9 100644
--- a/include/exec/cputlb.h
+++ b/include/exec/cputlb.h
@@ -19,7 +19,7 @@
#ifndef CPUTLB_H
#define CPUTLB_H
-#if !defined(CONFIG_USER_ONLY)
+#if !defined(CONFIG_USER_ONLY) && defined(CONFIG_TCG)
/* cputlb.c */
void tlb_protect_code(ram_addr_t ram_addr);
void tlb_unprotect_code(ram_addr_t ram_addr);
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 724ec73..446c924 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -82,6 +82,34 @@ void cpu_reloading_memory_map(void);
* Note that with KVM only one address space is supported.
*/
void cpu_address_space_init(CPUState *cpu, AddressSpace *as, int asidx);
+/**
+ * tlb_set_page_with_attrs:
+ * @cpu: CPU to add this TLB entry for
+ * @vaddr: virtual address of page to add entry for
+ * @paddr: physical address of the page
+ * @attrs: memory transaction attributes
+ * @prot: access permissions (PAGE_READ/PAGE_WRITE/PAGE_EXEC bits)
+ * @mmu_idx: MMU index to insert TLB entry for
+ * @size: size of the page in bytes
+ *
+ * Add an entry to this CPU's TLB (a mapping from virtual address
+ * @vaddr to physical address @paddr) with the specified memory
+ * transaction attributes. This is generally called by the target CPU
+ * specific code after it has been called through the tlb_fill()
+ * entry point and performed a successful page table walk to find
+ * the physical address and attributes for the virtual address
+ * which provoked the TLB miss.
+ *
+ * At most one entry for a given virtual address is permitted. Only a
+ * single TARGET_PAGE_SIZE region is mapped; the supplied @size is only
+ * used by tlb_flush_page.
+ */
+void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr,
+ hwaddr paddr, MemTxAttrs attrs,
+ int prot, int mmu_idx, target_ulong size);
+#endif
+
+#if !defined(CONFIG_USER_ONLY) && defined(CONFIG_TCG)
/* cputlb.c */
/**
* tlb_flush_page:
@@ -205,31 +233,6 @@ void tlb_flush_by_mmuidx_all_cpus(CPUState *cpu, uint16_t
idxmap);
* depend on when the guests translation ends the TB.
*/
void tlb_flush_by_mmuidx_all_cpus_synced(CPUState *cpu, uint16_t idxmap);
-/**
- * tlb_set_page_with_attrs:
- * @cpu: CPU to add this TLB entry for
- * @vaddr: virtual address of page to add entry for
- * @paddr: physical address of the page
- * @attrs: memory transaction attributes
- * @prot: access permissions (PAGE_READ/PAGE_WRITE/PAGE_EXEC bits)
- * @mmu_idx: MMU index to insert TLB entry for
- * @size: size of the page in bytes
- *
- * Add an entry to this CPU's TLB (a mapping from virtual address
- * @vaddr to physical address @paddr) with the specified memory
- * transaction attributes. This is generally called by the target CPU
- * specific code after it has been called through the tlb_fill()
- * entry point and performed a successful page table walk to find
- * the physical address and attributes for the virtual address
- * which provoked the TLB miss.
- *
- * At most one entry for a given virtual address is permitted. Only a
- * single TARGET_PAGE_SIZE region is mapped; the supplied @size is only
- * used by tlb_flush_page.
- */
-void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr,
- hwaddr paddr, MemTxAttrs attrs,
- int prot, int mmu_idx, target_ulong size);
/* tlb_set_page:
*
* This function is equivalent to calling tlb_set_page_with_attrs()
diff --git a/include/exec/helper-proto.h b/include/exec/helper-proto.h
index 954bef8..417c7b0 100644
--- a/include/exec/helper-proto.h
+++ b/include/exec/helper-proto.h
@@ -28,7 +28,9 @@ dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2),
dh_ctype(t3), \
#include "helper.h"
#include "trace/generated-helpers.h"
+#ifdef CONFIG_TCG
#include "tcg-runtime.h"
+#endif
#undef DEF_HELPER_FLAGS_0
#undef DEF_HELPER_FLAGS_1
--
1
Re: [Qemu-devel] [PATCH v2 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled
On 07/03/2017 10:51 AM, QingFeng Hao wrote:
> This patch is based on a similar patch from Stefan Hajnoczi -
> commit c324fd0a39c (" virtio-pci: use ioeventfd even when KVM is disabled)
>
> Do not check kvm_eventfds_enabled() when KVM is disabled since it
> always returns 0. Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
> ("memory: emulate ioeventfd") it has been possible to use ioeventfds in
> qtest or TCG mode.
>
> This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
> when KVM is disabled.
>
> I have tested that virtio-scsi-ccw works under tcg both with and without
> iothread.
>
> This patch fixes qemu-iotests 068, which was accidentally merged early
> despite the dependency on ioeventfd.
>
> Signed-off-by: QingFeng Hao
> ---
> hw/s390x/virtio-ccw.c | 2 +-
> target/s390x/kvm.c| 3 +++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 90d37cb9ff..35896eb007 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice
> *dev, Error **errp)
> sch->cssid, sch->ssid, sch->schid, sch->devno,
> ccw_dev->devno.valid ? "user-configured" : "auto-configured");
>
> -if (!kvm_eventfds_enabled()) {
> +if (kvm_enabled() && !kvm_eventfds_enabled()) {
> dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
> }
>
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index a3d00196f4..c37f9c3b9e 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -2220,6 +2220,9 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier
> *notifier, uint32_t sch,
> .addr = sch,
> .len = 8,
> };
> +if (!kvm_enabled()) {
> +return 0;
> +}
thinking more about it. wouldnt it be better to do something like this instead
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 058ddad..cc47831 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -1240,7 +1240,11 @@ static inline int
s390_assign_subch_ioeventfd(EventNotifier *notifier,
uint32_t sch_id, int vq,
bool assign)
{
-return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+if (kvm_enabled()) {
+return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+} else {
+return 0;
+}
}
static inline void s390_crypto_reset(void)
FWIW, it seems that we (s390) do not have a functional equivalent function as
commit
8c56c1a592b5092d91da8d8943c1d6462a6f ("memory: emulate ioeventfd") , so we
will
not use the iothreads.
> if (!kvm_check_extension(kvm_state, KVM_CAP_IOEVENTFD)) {
> return -ENOSYS;
> }
>
Re: [Qemu-devel] [PATCH v2 01/15] configure: add the disable-tcg option
On 03/07/2017 12:20, Daniel P. Berrange wrote: >> >> +if test "$tcg" = "no"; then >> + for target in $target_list; do >> + if test "$softmmu" = "yes"; then >> +case $target in >> + i386-softmmu|x86_64-softmmu) >> + ;; >> +*) >> + error_exit "The current $target can't support disable-tcg,"\ >> + "only i386-softmmu|x86_64-softmmu support disable-tcg" >> + ;; > This looks too simplistic in its logic. > > You can disable TCG, if-and-only-if the system emulator supports KVM. > > KVM is supported on many architectures, not only x86-64 & i386. > > KVM is only supported if the guest emulator architecture matches the > host build target architecture. > > ie if you are building an x86_64 system emulator on a PPC64 host, > then you can't disable TCG. > > So this needs rewriting to *not* special case x86_64 / i386. Instead > you need to compare & match build target / system emulator architectures, > across all architectures supporting KVM. i386-softmmu and x86_64-softmmu are singled out here, because they're the only targets where --disable-tcg compiles. For the others, more work is needed (see patches 6-15 in Yang Zhong's series). The part that is missing in this patch is disabling non-hypervisor targets when --disable-tcg is specified. My original patch built only i386-softmmu and x86_64-softmmu if you specified --disable-tcg, see https://lists.nongnu.org/archive/html/qemu-devel/2012-09/msg02570.html https://lists.nongnu.org/archive/html/qemu-devel/2012-09/msg02571.html Paolo
Re: [Qemu-devel] [PATCH v2 01/15] configure: add the disable-tcg option
On Mon, Jul 03, 2017 at 12:25:36PM +0200, Paolo Bonzini wrote: > > > On 03/07/2017 12:20, Daniel P. Berrange wrote: > >> > >> +if test "$tcg" = "no"; then > >> + for target in $target_list; do > >> + if test "$softmmu" = "yes"; then > >> +case $target in > >> + i386-softmmu|x86_64-softmmu) > >> + ;; > >> +*) > >> + error_exit "The current $target can't support disable-tcg,"\ > >> + "only i386-softmmu|x86_64-softmmu support disable-tcg" > >> + ;; > > This looks too simplistic in its logic. > > > > You can disable TCG, if-and-only-if the system emulator supports KVM. > > > > KVM is supported on many architectures, not only x86-64 & i386. > > > > KVM is only supported if the guest emulator architecture matches the > > host build target architecture. > > > > ie if you are building an x86_64 system emulator on a PPC64 host, > > then you can't disable TCG. > > > > So this needs rewriting to *not* special case x86_64 / i386. Instead > > you need to compare & match build target / system emulator architectures, > > across all architectures supporting KVM. > > i386-softmmu and x86_64-softmmu are singled out here, because they're > the only targets where --disable-tcg compiles. For the others, more > work is needed (see patches 6-15 in Yang Zhong's series). Even with that, you still can't disable TCG if building on a non-x86 host, since that'd leave you with no available CPU at all. So the code still needs refactoring to check architectures properly. > The part that is missing in this patch is disabling non-hypervisor > targets when --disable-tcg is specified. My original patch built only > i386-softmmu and x86_64-softmmu if you specified --disable-tcg, see > > https://lists.nongnu.org/archive/html/qemu-devel/2012-09/msg02570.html > https://lists.nongnu.org/archive/html/qemu-devel/2012-09/msg02571.html 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] [PATCH v2] s390: return unavailable features via query-cpu-definitions
On 03.07.2017 11:25, David Hildenbrand wrote:
>
>>
>> +static S390CPUModel *get_max_cpu_model(Error **errp);
>> +
>> #ifndef CONFIG_USER_ONLY
>> +static void list_add_feat(const char *name, void *opaque);
>
> Wonder if we should declare all these prototypes at the beginning of the
> file.
>
Don't know either. Looking around in QEMU, forward declarations for
static functions seem to be used sparsely (only when needed). I could
have reordered the functions to get around without forward decls but
this would have obscured the change. Maybe defer and clean up in a
general cleanup/refactoring?
>> +
>> +static void check_unavailable_features(const S390CPUModel *max_model,
>> + const S390CPUModel *model,
>> + strList **unavailable)
>> +{
>> +S390FeatBitmap missing;
>> +
>> +/* check general model compatibility */
>> +if (max_model->def->gen < model->def->gen ||
>> +(max_model->def->gen == model->def->gen &&
>> + max_model->def->ec_ga < model->def->ec_ga)) {
>> +list_add_feat("type", unavailable);
>> +}
>> +
>> +/* detect missing features if any to properly report them */
>> +bitmap_andnot(missing, model->features, max_model->features,
>> + S390_FEAT_MAX);
>> +if (!bitmap_empty(missing, S390_FEAT_MAX)) {
>> +s390_feat_bitmap_to_ascii(missing,
>> + unavailable,
>> + list_add_feat);
>
> This certainly fits into one line.
True, will change.
>
>> +}
>> +}
>> +
>> +struct CpuDefinitionInfoListData {
>> +CpuDefinitionInfoList *list;
>> +Error **errp;
>> +};
>> +
>> static void create_cpu_model_list(ObjectClass *klass, void *opaque)
>> {
>> -CpuDefinitionInfoList **cpu_list = opaque;
>> +struct CpuDefinitionInfoListData *cpu_list_data = opaque;
>> +CpuDefinitionInfoList **cpu_list = &cpu_list_data->list;
>> CpuDefinitionInfoList *entry;
>> CpuDefinitionInfo *info;
>> char *name = g_strdup(object_class_get_name(klass));
>> S390CPUClass *scc = S390_CPU_CLASS(klass);
>> +Object *obj;
>> +S390CPU *sc;
>> +S390CPUModel *scm;
>>
>> /* strip off the -s390-cpu */
>> g_strrstr(name, "-" TYPE_S390_CPU)[0] = 0;
>> @@ -300,21 +336,33 @@ static void create_cpu_model_list(ObjectClass *klass,
>> void *opaque)
>> info->migration_safe = scc->is_migration_safe;
>> info->q_static = scc->is_static;
>> info->q_typename = g_strdup(object_class_get_name(klass));
>> -
>> +/* check for unavailable features */
>> +obj = object_new(object_class_get_name(klass));
>> +sc = S390_CPU(obj);
>> +scm = get_max_cpu_model(cpu_list_data->errp);
>
> H, if this function fails, we will create the same error multiple
> times (as there is no way to stop this function from iterating). And we
> will fail to create a cpu model list in case there is no host cpu model,
> which is a change in behavior (as we will report an error).
>
> Would it be better to simply get the max model in
> arch_query_cpu_definitions() and pass it via CpuDefinitionInfoListData,
> instead of the errp variable?Simplifies things, I like it.
>
> Then you could simply skip the checks and set
> info->has_unavailable_features = false in case there is no max model
> (get_max_cpu_model() returned NULL / an error). (same behavior as for now)
>
> Errors from get_max_cpu_model() then should be ignored and not reported.
>
Just to be sure: you suggest that I should call error_free() after
calling get_max_cpu_model, in order to prevent that the QMP command
query-cpu-definitions fails, right?
>
>
>> +if (scm && sc->model) {
>> +info->has_unavailable_features = true;
>> +check_unavailable_features(scm, sc->model,
>> &info->unavailable_features);
>> +}
>>
>> entry = g_malloc0(sizeof(*entry));
>> entry->value = info;
>> entry->next = *cpu_list;
>> *cpu_list = entry;
>> +object_unref(obj);
>> }
>>
>> CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
>> {
>> -CpuDefinitionInfoList *list = NULL;
>> +struct CpuDefinitionInfoListData list_data = {
>> +.list = NULL,
>> +.errp = errp,
>> +};
>>
>> -object_class_foreach(create_cpu_model_list, TYPE_S390_CPU, false,
>> &list);
>> +object_class_foreach(create_cpu_model_list, TYPE_S390_CPU, false,
>> + &list_data);
>>
>> -return list;
>> +return list_data.list;
>> }
>>
>> static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo
>> *info,
>>
>
>
--
Mit freundlichen Grüßen/Kind Regards
Viktor Mihajlovski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
Re: [Qemu-devel] [PATCH v2 01/15] configure: add the disable-tcg option
On 03/07/2017 12:33, Daniel P. Berrange wrote: >> i386-softmmu and x86_64-softmmu are singled out here, because they're >> the only targets where --disable-tcg compiles. For the others, more >> work is needed (see patches 6-15 in Yang Zhong's series). > > Even with that, you still can't disable TCG if building on a non-x86 > host, since that'd leave you with no available CPU at all. So the > code still needs refactoring to check architectures properly. It would leave you with a tools-only build; whether that's a good idea, it's another story. I think it's acceptable, but others may disagree. Paolo >> The part that is missing in this patch is disabling non-hypervisor >> targets when --disable-tcg is specified. My original patch built only >> i386-softmmu and x86_64-softmmu if you specified --disable-tcg, see >> >> https://lists.nongnu.org/archive/html/qemu-devel/2012-09/msg02570.html >> https://lists.nongnu.org/archive/html/qemu-devel/2012-09/msg02571.html
Re: [Qemu-devel] [RFC 00/29] postcopy+vhost-user/shared ram
Hi On Thu, Jun 29, 2017 at 8:56 PM Dr. David Alan Gilbert wrote: > * Dr. David Alan Gilbert (git) (dgilb...@redhat.com) wrote: > > From: "Dr. David Alan Gilbert" > > > > Hi, > > This is a RFC/WIP series that enables postcopy migration > > with shared memory to a vhost-user process. > > It's based off current-head + Juan's load_cleanup series, and > > Alexey's bitmap series (v4). It's very lightly tested and seems > > to work, but it's quite rough. > > Marc-André asked if I had a git with it all applied; so here we are: > https://github.com/dagrh/qemu/commits/vhost > g...@github.com:dagrh/qemu.git on the vhost branch > > I started looking at the series, but I am not familiar with ufd/postcopy. Could you update vhost-user.txt to describe the new messages? Otherwise, make check hangs in /x86_64/vhost-user/connect-fail (might be an unrelated regression?) Thanks -- Marc-André Lureau
Re: [Qemu-devel] [PATCH v1] s390x/cpumodel: allow to enable "idtes" feature for TCG
On 30.06.2017 21:22, Richard Henderson wrote: > On 06/29/2017 12:05 AM, Thomas Huth wrote: >> However, I'm not sure whether you can simply ignore the clearing-by-ASCE >> stuff in this case. For example, according to the PoP: >> >> "When the clearing-by-ASCE-option bit (bit 52 of gen- >> eral register R2 is one), the M4 field is ignored." >> >> And the idte helper function currently always takes the M4 field into >> account... > > I don't see that quote. Which version of the Principles of Operation are you using? I just checked, and that sentence seems to be available in version SA22-7832-10 (but it is not in SA22-7832-09 yet). Thomas
[Qemu-devel] [PATCH] ehci: add sanity check for maxframes
Reported-by: Peter Maydell
Signed-off-by: Gerd Hoffmann
---
hw/usb/hcd-ehci.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 73090e01ad..604912cb3e 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2483,6 +2483,11 @@ void usb_ehci_realize(EHCIState *s, DeviceState *dev,
Error **errp)
NB_PORTS);
return;
}
+if (s->maxframes < 8 || s->maxframes > 512) {
+error_setg(errp, "maxframes %d out if range (8 .. 512)",
+ s->maxframes);
+return;
+}
usb_bus_new(&s->bus, sizeof(s->bus), s->companion_enable ?
&ehci_bus_ops_companion : &ehci_bus_ops_standalone, dev);
--
2.9.3
Re: [Qemu-devel] [PATCH] ehci: add sanity check for maxframes
On 3 July 2017 at 12:15, Gerd Hoffmann wrote:
> Reported-by: Peter Maydell
> Signed-off-by: Gerd Hoffmann
> ---
> hw/usb/hcd-ehci.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
> index 73090e01ad..604912cb3e 100644
> --- a/hw/usb/hcd-ehci.c
> +++ b/hw/usb/hcd-ehci.c
> @@ -2483,6 +2483,11 @@ void usb_ehci_realize(EHCIState *s, DeviceState *dev,
> Error **errp)
> NB_PORTS);
> return;
> }
> +if (s->maxframes < 8 || s->maxframes > 512) {
> +error_setg(errp, "maxframes %d out if range (8 .. 512)",
> + s->maxframes);
Typo: should be "of".
> +return;
> +}
>
> usb_bus_new(&s->bus, sizeof(s->bus), s->companion_enable ?
> &ehci_bus_ops_companion : &ehci_bus_ops_standalone, dev);
> --
> 2.9.3
Otherwise
Reviewed-by: Peter Maydell
Do we want something similar for hcd-uhci just for consistency?
thanks
-- PMM
Re: [Qemu-devel] [PATCH 1/4] migration: fix handling for --only-migratable
Peter Xu wrote:
> MigrateState object is not ready at that time, so we'll get an
> assertion. Use qemu_global_option() instead.
>
> Reported-by: Eduardo Habkost
> Suggested-by: Eduardo Habkost
> Fixes: 3df663e ("migration: move only_migratable to MigrationState")
> Signed-off-by: Peter Xu
Reviewed-by: Juan Quintela
> bool migration_in_postcopy_after_devices(MigrationState *);
> -void migration_only_migratable_set(void);
> void migration_global_dump(Monitor *mon);
Nice, one less exported function.
> -migration_only_migratable_set();
> +qemu_global_option("migration.only-migratable=true");
> break;
> case QEMU_OPTION_nodefaults:
> has_defaults = 0;
aha, that is a much, much nicer way to set functions.
Later, Juan.
Re: [Qemu-devel] [PATCH 3/4] doc: add item for "-M enforce-config-section"
Peter Xu wrote: > It's never documented, and now we have one more parameter for it (which > means this one can be obsolete in the future). Document it properly. > > Although now when enforce-config-section is set, it'll override the > other "-global" parameter, that is not necessarily a rule. Forbid that > usage in the document. > > Suggested-by: Eduardo Habkost > Signed-off-by: Peter Xu > --- > qemu-options.hx | 8 > 1 file changed, 8 insertions(+) > > diff --git a/qemu-options.hx b/qemu-options.hx > index 297bd8a..927c51f 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -85,6 +85,14 @@ Enables or disables NVDIMM support. The default is off. > @item s390-squash-mcss=on|off > Enables or disables squashing subchannels into the default css. > The default is off. > +@item enforce-config-section=on|off > +Decides whether we will send the configuration section when doing > +migration. By default, it is turned on. We can set this to off to > +explicitly disable it. Note: this parameter will be obsolete soon, > +please use "-global migration.send-configuration=on|off" instead. > +"enforce-config-section" cannot be used together with "-global > +migration.send-configuration". If it happens, the behavior is > +undefined. > @end table > ETEXI Reviewed-by: Juan Quintela I will have been explicit that it is *already* obsolete, just that we don't have a way to say that. Later, Juan.
Re: [Qemu-devel] [PATCH 4/4] doc: update TYPE_MIGRATION documents
Peter Xu wrote: > [Peter collected Eduardo's patch comment and formatted into patch] > > Suggested-by: Eduardo Habkost > Signed-off-by: Peter Xu > --- Reviewed-by: Juan Quintela
Re: [Qemu-devel] [PATCH v6 3/3] test-qga: add test for guest-get-osinfo
Uh, sorry. I will resend in a minute.
Tomas
On Fri, 30 Jun 2017 13:48:41 +
Marc-André Lureau wrote:
> Hi,
>
> It's missing test-qga-os-release :)
>
> On Thu, Jun 29, 2017 at 11:27 PM Tomáš Golembiovský
> wrote:
>
> > Add test for guest-get-osinfo command.
> >
> > Qemu-ga was modified to accept QGA_OS_RELEASE environment variable. If
> > the variable is defined it is interpreted as path to the os-release file
> > and it is parsed instead of the default paths.
> >
> > Signed-off-by: Tomáš Golembiovský
> > ---
> > qga/commands-posix.c | 13 ++---
> > tests/test-qga.c | 53
> >
> > 2 files changed, 63 insertions(+), 3 deletions(-)
> >
> > diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> > index 2406518d47..f6ce8dd583 100644
> > --- a/qga/commands-posix.c
> > +++ b/qga/commands-posix.c
> > @@ -2683,9 +2683,16 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
> > info->kernel_release = g_strdup(kinfo.release);
> > info->machine_hardware = g_strdup(kinfo.machine);
> >
> > -GKeyFile *osrelease = ga_parse_osrelease("/etc/os-release");
> > -if (osrelease == NULL) {
> > -osrelease = ga_parse_osrelease("/usr/lib/os-release");
> > +GKeyFile *osrelease = NULL;
> > +
> > +const char *qga_os_release = g_getenv("QGA_OS_RELEASE");
> > +if (qga_os_release != NULL) {
> > +osrelease = ga_parse_osrelease(qga_os_release);
> > +} else {
> > +osrelease = ga_parse_osrelease("/etc/os-release");
> > +if (osrelease == NULL) {
> > +osrelease = ga_parse_osrelease("/usr/lib/os-release");
> > +}
> > }
> >
> > if (osrelease != NULL) {
> > diff --git a/tests/test-qga.c b/tests/test-qga.c
> > index 631b98639a..b9160708a0 100644
> > --- a/tests/test-qga.c
> > +++ b/tests/test-qga.c
> > @@ -936,6 +936,58 @@ static void test_qga_guest_exec_invalid(gconstpointer
> > fix)
> > QDECREF(ret);
> > }
> >
> > +static void test_qga_guest_get_osinfo(gconstpointer data)
> > +{
> > +TestFixture fixture;
> > +const gchar *str;
> > +gchar *cwd, *env[2];
> > +QDict *ret, *val;
> > +
> > +cwd = g_get_current_dir();
> > +env[0] =
> > g_strdup_printf("QGA_OS_RELEASE=%s%ctests%cdata%ctest-qga-os-release",
> > + cwd, G_DIR_SEPARATOR, G_DIR_SEPARATOR,
> > G_DIR_SEPARATOR);
> > +env[1] = NULL;
> > +g_free(cwd);
> > +fixture_setup(&fixture, NULL, env);
> > +
> > +ret = qmp_fd(fixture.fd, "{'execute': 'guest-get-osinfo'}");
> > +g_assert_nonnull(ret);
> > +qmp_assert_no_error(ret);
> > +
> > +val = qdict_get_qdict(ret, "return");
> > +
> > +str = qdict_get_try_str(val, "id");
> > +g_assert_nonnull(str);
> > +g_assert_cmpstr(str, ==, "qemu-ga-test");
> > +
> > +str = qdict_get_try_str(val, "name");
> > +g_assert_nonnull(str);
> > +g_assert_cmpstr(str, ==, "QEMU-GA");
> > +
> > +str = qdict_get_try_str(val, "pretty-name");
> > +g_assert_nonnull(str);
> > +g_assert_cmpstr(str, ==, "QEMU Guest Agent test");
> > +
> > +str = qdict_get_try_str(val, "version");
> > +g_assert_nonnull(str);
> > +g_assert_cmpstr(str, ==, "Test 1");
> > +
> > +str = qdict_get_try_str(val, "version-id");
> > +g_assert_nonnull(str);
> > +g_assert_cmpstr(str, ==, "1");
> > +
> > +str = qdict_get_try_str(val, "variant");
> > +g_assert_nonnull(str);
> > +g_assert_cmpstr(str, ==, "Unit test \"'$`\\ and etc.");
> > +
> > +str = qdict_get_try_str(val, "variant-id");
> > +g_assert_nonnull(str);
> > +g_assert_cmpstr(str, ==, "unit-test");
> > +
> > +QDECREF(ret);
> > +g_free(env[0]);
> > +}
> > +
> > int main(int argc, char **argv)
> > {
> > TestFixture fix;
> > @@ -972,6 +1024,7 @@ int main(int argc, char **argv)
> > g_test_add_data_func("/qga/guest-exec", &fix, test_qga_guest_exec);
> > g_test_add_data_func("/qga/guest-exec-invalid", &fix,
> > test_qga_guest_exec_invalid);
> > +g_test_add_data_func("/qga/guest-get-osinfo", &fix,
> > test_qga_guest_get_osinfo);
> >
> > if (g_getenv("QGA_TEST_SIDE_EFFECTING")) {
> > g_test_add_data_func("/qga/fsfreeze-and-thaw", &fix,
> > --
> > 2.13.1
> >
> > --
> Marc-André Lureau
--
Tomáš Golembiovský
Re: [Qemu-devel] [PATCH v3 0/3] Generic PCIe host bridge INTx determination for INTx routing
Hi, On 13/06/2017 15:31, Eric Auger wrote: > This series implements INTx to gsi routing for ARM VIRT/GPEX. This is > a respin of [1] which was lost in limbo. > > ARM virt uses GPEX PCIe bridge. This latter does not implement INTx > to GSI routing. PCIe/INTx assignment works but the consequence is > irqfd is not used along with INTx interrupts and VFIO INTx handlers > are executed on userspace leading to an important performance degradation. > > This issue is witnessed by the following messages; > > qemu-system-aarch64: -device vfio-pci,host=0006:90:00.0: PCI: Bug - > unimplemented PCI INTx routing (gpex-pcihost) > qemu-system-aarch64: PCI: Bug - unimplemented PCI INTx routing (gpex-pcihost) > qemu-system-aarch64: PCI: Bug - unimplemented PCI INTx routing (gpex-pcihost) > > So with this series, irqfd is set up for PCIe/INTx passthrough and we get > the optimal performance. Also we get rid of the above messages. Ping? any feedback on this series that fixes irqfd setup for PCIe passthrough/ARM with legacy interrupts? Thanks Eric > > This series can be found at: > https://github.com/eauger/qemu/tree/v2.9-gpex-intx-v3 > > References: > [1] Generic PCIe host bridge INTx determination for INTx routing >https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg04975.html > > Pranavkumar Sawargaonkar (3): > hw/pci-host/gpex: Set INTx index/gsi mapping > hw/arm/virt: Set INTx/gsi mapping > hw/pci-host/gpex: Implement PCI INTx routing > > hw/arm/virt.c | 4 > hw/pci-host/gpex.c | 22 ++ > include/hw/pci-host/gpex.h | 3 +++ > 3 files changed, 29 insertions(+) >
Re: [Qemu-devel] [PATCH v1] s390x/cpumodel: allow to enable "idtes" feature for TCG
On 03.07.2017 13:07, Thomas Huth wrote: > On 30.06.2017 21:22, Richard Henderson wrote: >> On 06/29/2017 12:05 AM, Thomas Huth wrote: >>> However, I'm not sure whether you can simply ignore the clearing-by-ASCE >>> stuff in this case. For example, according to the PoP: >>> >>> "When the clearing-by-ASCE-option bit (bit 52 of gen- >>> eral register R2 is one), the M4 field is ignored." >>> >>> And the idte helper function currently always takes the M4 field into >>> account... >> >> I don't see that quote. > > Which version of the Principles of Operation are you using? I just > checked, and that sentence seems to be available in version SA22-7832-10 > (but it is not in SA22-7832-09 yet). > > Thomas > > I think that part of the PoP is quite confusing. "When the local-TLB-clearing facility is not installed [...] the term “__specified__ CPU or CPUs” means all of the CPUs in the configuration." "When the local-TLB-clearing facility is installed [...] the term “__specified__ CPU or CPUs” means only the CPU executing the IDTE instruction (the local CPU)." "When the clearing-by-ASCE-option bit, bit 52 of gen- eral register R2, is zero [...] in the __specified__ CPU or CPUs in the configuration are cleared of (1) all TLB table entries of the designated [...]" "When the clearing-by-ASCE-option bit is one [...] but it does clear, from the TLBs in __all__ CPUs in the [...]" So one could assume, that local-tlb-clearing does not apply to clearing-by-ASCE-option. However: "When bit 52 of general register R2, the clearing-by- ASCE-option bit, is one, the clearing-by-ASCE oper- ation is specified. [...] The TLBs of the __specified__ CPU or CPUs in the configuration [...]" But: "When the clearing-by-ASCE-option bit (bit 52 of gen- eral register R2 is one), the M4 field is ignored." However also "Common Operation" section rather reads like local-TLB-clearing applies to any mode. Also the description of local-TLB-clearing is quite general: "Local-TLB-Clearing Facility The local-TLB-clearing facility may be available on a model implementing z/Architecture. The facility may provide performance improvements for the INVALI- DATE DAT TABLE ENTRY and INVALIDATE PAGE TABLE ENTRY instructions by allowing the program to specify whether TLB clearing should be done in all CPUs of the configuration or only in the CPU execut- ing the instruction." My interpretation: local-TLB-clearing was introduced after IDTES but before EDAT2 (M4 bit 2). I think "m4 is ignored" should rather mean "M4 bit 2 is ignored". Especially as this sentence was added later, I assume it is connected to EDAT2, not local-TLB-clearing. For now (!SMP) it should really matter. Opinions? -- Thanks, David
Re: [Qemu-devel] [Qemu-block] [PATCH 10/31] vvfat: use DIV_ROUND_UP
On 06/22/2017 07:41 AM, Marc-André Lureau wrote:
> I used the clang-tidy qemu-round check to generate the fix:
> https://github.com/elmarco/clang-tools-extra
>
> Signed-off-by: Marc-André Lureau
> ---
> block/vvfat.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/block/vvfat.c b/block/vvfat.c
> index 426ca70e35..877f71dcdc 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -433,7 +433,7 @@ static inline direntry_t*
> create_long_filename(BDRVVVFATState* s,const char* fil
> {
> char buffer[258];
> int length=short2long_name(buffer,filename),
> -number_of_entries=(length+25)/26,i;
> +number_of_entries=DIV_ROUND_UP(length, 26),i;
This formatting made me do a double take (at first, I thought it was a
comma expression, before realizing it was a declaration). While you are
touching it, can you please rewrite it into something more legible, such as:
int length = short2long_name(buffer, filename);
int number_of_entries = DIV_ROUND_UP(length, 26);
int i;
I don't mind declaring multiple variables in one declaration - provided
that we aren't also initializing them. But mixing in the un-initialized
declaration of 'i' with other initialized variables is just awkward.
--
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] [PATCH v7 2/3] test-qga: pass environemnt to qemu-ga
Modify fixture_setup() to pass environemnt variables to spawned qemu-ga
instance.
Signed-off-by: Tomáš Golembiovský
---
tests/test-qga.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/test-qga.c b/tests/test-qga.c
index c77f241036..631b98639a 100644
--- a/tests/test-qga.c
+++ b/tests/test-qga.c
@@ -46,7 +46,7 @@ static void qga_watch(GPid pid, gint status, gpointer
user_data)
}
static void
-fixture_setup(TestFixture *fixture, gconstpointer data)
+fixture_setup(TestFixture *fixture, gconstpointer data, gchar **envp)
{
const gchar *extra_arg = data;
GError *error = NULL;
@@ -67,7 +67,7 @@ fixture_setup(TestFixture *fixture, gconstpointer data)
g_shell_parse_argv(cmd, NULL, &argv, &error);
g_assert_no_error(error);
-g_spawn_async(fixture->test_dir, argv, NULL,
+g_spawn_async(fixture->test_dir, argv, envp,
G_SPAWN_SEARCH_PATH|G_SPAWN_DO_NOT_REAP_CHILD,
NULL, NULL, &fixture->pid, &error);
g_assert_no_error(error);
@@ -707,7 +707,7 @@ static void test_qga_blacklist(gconstpointer data)
QDict *ret, *error;
const gchar *class, *desc;
-fixture_setup(&fix, "-b guest-ping,guest-get-time");
+fixture_setup(&fix, "-b guest-ping,guest-get-time", NULL);
/* check blacklist */
ret = qmp_fd(fix.fd, "{'execute': 'guest-ping'}");
@@ -943,7 +943,7 @@ int main(int argc, char **argv)
setlocale (LC_ALL, "");
g_test_init(&argc, &argv, NULL);
-fixture_setup(&fix, NULL);
+fixture_setup(&fix, NULL, NULL);
g_test_add_data_func("/qga/sync-delimited", &fix, test_qga_sync_delimited);
g_test_add_data_func("/qga/sync", &fix, test_qga_sync);
--
2.13.1
[Qemu-devel] [PATCH v7 0/3] qemu-ga: add guest-get-osinfo command
v7:
- fixed incorrect error check in ga_get_win_version()
- added missing test-qga-os-release
- fixed coding style issues
v6:
- fixed the documentation comment in schema
- disguising os-release as key-value file (thanks Marc-André)
- dropped dependency on gio
- improved error handling
- added test
v5:
- fixed build failure with older glib
- fixed coding style issues
- fixed one log string
This is a continuation of the work started by Vinzenz Feenstra in the
threads:
https://lists.nongnu.org/archive/html/qemu-devel/2017-03/msg04154.html
https://lists.nongnu.org/archive/html/qemu-devel/2017-03/msg04302.html
https://lists.nongnu.org/archive/html/qemu-devel/2017-03/msg06262.html
The idea is to report some basic information from uname and from
os-release file, if it is present. On MS Windows, where neither uname
nor os-release exist we fill the values based on the information we can
get from the OS.
The example output on Fedora is:
{
"return": {
"kernel-version": "#1 SMP Mon May 8 18:46:06 UTC 2017",
"kernel-release": "4.10.15-200.fc25.x86_64",
"machine-hardware": "x86_64",
"id": "fedora",
"name": "Fedora",
"pretty-name": "Fedora 25 (Server Edition)",
"version": "25 (Server Edition)",
"variant": "Server Edition",
"version-id": "25",
"variant-id": "server"
}
}
The example output on MS Windows 10 is:
{
"return": {
"kernel-version": "10.0",
"kernel-release": "10240",
"machine-hardware": "x86_64",
"id": "mswindows",
"name": "Microsoft Windows",
"pretty-name": "Windows 10 Enterprise",
"version": "Microsoft Windows 10",
"version-id": "10",
"variant": "client",
"variant-id": "client"
}
}
Tomas Golembiovsky
Tomáš Golembiovský (3):
qemu-ga: add guest-get-osinfo command
test-qga: pass environemnt to qemu-ga
test-qga: add test for guest-get-osinfo
qga/commands-posix.c | 143 +++
qga/commands-win32.c | 185 +
qga/qapi-schema.json | 65 +++
tests/data/test-qga-os-release | 7 ++
tests/test-qga.c | 63 +-
5 files changed, 459 insertions(+), 4 deletions(-)
create mode 100644 tests/data/test-qga-os-release
--
2.13.1
Re: [Qemu-devel] [PATCH v2 01/15] configure: add the disable-tcg option
On 03.07.2017 12:55, Paolo Bonzini wrote: > > > On 03/07/2017 12:33, Daniel P. Berrange wrote: >>> i386-softmmu and x86_64-softmmu are singled out here, because they're >>> the only targets where --disable-tcg compiles. For the others, more >>> work is needed (see patches 6-15 in Yang Zhong's series). >> >> Even with that, you still can't disable TCG if building on a non-x86 >> host, since that'd leave you with no available CPU at all. So the >> code still needs refactoring to check architectures properly. > > It would leave you with a tools-only build; whether that's a good idea, > it's another story. I think it's acceptable, but others may disagree. I think this is OK for the first version. We still can refine the checks later if we then think that there is a need for it. Thomas
Re: [Qemu-devel] [PATCH v2] s390: return unavailable features via query-cpu-definitions
>> Wonder if we should declare all these prototypes at the beginning of the >> file. >> > Don't know either. Looking around in QEMU, forward declarations for > static functions seem to be used sparsely (only when needed). I could > have reordered the functions to get around without forward decls but > this would have obscured the change. Maybe defer and clean up in a > general cleanup/refactoring? Yes, fine with me! [...] >> >> H, if this function fails, we will create the same error multiple >> times (as there is no way to stop this function from iterating). And we >> will fail to create a cpu model list in case there is no host cpu model, >> which is a change in behavior (as we will report an error). >> >> Would it be better to simply get the max model in >> arch_query_cpu_definitions() and pass it via CpuDefinitionInfoListData, >> instead of the errp variable?Simplifies things, I like it. >> >> Then you could simply skip the checks and set >> info->has_unavailable_features = false in case there is no max model >> (get_max_cpu_model() returned NULL / an error). (same behavior as for now) >> >> Errors from get_max_cpu_model() then should be ignored and not reported. >> > Just to be sure: you suggest that I should call error_free() after > calling get_max_cpu_model, in order to prevent that the QMP command > query-cpu-definitions fails, right? That would be my suggestion simply don't provide runability information, if we can't tell (because the max model is not available - e.g. with old KVM versions without complete CPU model support), hiding the error. -- Thanks, David
[Qemu-devel] [PATCH v7 3/3] test-qga: add test for guest-get-osinfo
Add test for guest-get-osinfo command.
Qemu-ga was modified to accept QGA_OS_RELEASE environment variable. If
the variable is defined it is interpreted as path to the os-release file
and it is parsed instead of the default paths.
Signed-off-by: Tomáš Golembiovský
---
qga/commands-posix.c | 13 +++---
tests/data/test-qga-os-release | 7 ++
tests/test-qga.c | 55 ++
3 files changed, 72 insertions(+), 3 deletions(-)
create mode 100644 tests/data/test-qga-os-release
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 7a933a5f12..16b18c96ef 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2683,9 +2683,16 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
info->kernel_release = g_strdup(kinfo.release);
info->machine_hardware = g_strdup(kinfo.machine);
-GKeyFile *osrelease = ga_parse_osrelease("/etc/os-release");
-if (osrelease == NULL) {
-osrelease = ga_parse_osrelease("/usr/lib/os-release");
+GKeyFile *osrelease = NULL;
+
+const char *qga_os_release = g_getenv("QGA_OS_RELEASE");
+if (qga_os_release != NULL) {
+osrelease = ga_parse_osrelease(qga_os_release);
+} else {
+osrelease = ga_parse_osrelease("/etc/os-release");
+if (osrelease == NULL) {
+osrelease = ga_parse_osrelease("/usr/lib/os-release");
+}
}
if (osrelease != NULL) {
diff --git a/tests/data/test-qga-os-release b/tests/data/test-qga-os-release
new file mode 100644
index 00..70664eb6ec
--- /dev/null
+++ b/tests/data/test-qga-os-release
@@ -0,0 +1,7 @@
+ID=qemu-ga-test
+NAME=QEMU-GA
+PRETTY_NAME="QEMU Guest Agent test"
+VERSION="Test 1"
+VERSION_ID=1
+VARIANT="Unit test \"\'\$\`\\ and etc."
+VARIANT_ID=unit-test
diff --git a/tests/test-qga.c b/tests/test-qga.c
index 631b98639a..c151dfdc34 100644
--- a/tests/test-qga.c
+++ b/tests/test-qga.c
@@ -936,6 +936,59 @@ static void test_qga_guest_exec_invalid(gconstpointer fix)
QDECREF(ret);
}
+static void test_qga_guest_get_osinfo(gconstpointer data)
+{
+TestFixture fixture;
+const gchar *str;
+gchar *cwd, *env[2];
+QDict *ret, *val;
+
+cwd = g_get_current_dir();
+env[0] = g_strdup_printf(
+"QGA_OS_RELEASE=%s%ctests%cdata%ctest-qga-os-release",
+cwd, G_DIR_SEPARATOR, G_DIR_SEPARATOR, G_DIR_SEPARATOR);
+env[1] = NULL;
+g_free(cwd);
+fixture_setup(&fixture, NULL, env);
+
+ret = qmp_fd(fixture.fd, "{'execute': 'guest-get-osinfo'}");
+g_assert_nonnull(ret);
+qmp_assert_no_error(ret);
+
+val = qdict_get_qdict(ret, "return");
+
+str = qdict_get_try_str(val, "id");
+g_assert_nonnull(str);
+g_assert_cmpstr(str, ==, "qemu-ga-test");
+
+str = qdict_get_try_str(val, "name");
+g_assert_nonnull(str);
+g_assert_cmpstr(str, ==, "QEMU-GA");
+
+str = qdict_get_try_str(val, "pretty-name");
+g_assert_nonnull(str);
+g_assert_cmpstr(str, ==, "QEMU Guest Agent test");
+
+str = qdict_get_try_str(val, "version");
+g_assert_nonnull(str);
+g_assert_cmpstr(str, ==, "Test 1");
+
+str = qdict_get_try_str(val, "version-id");
+g_assert_nonnull(str);
+g_assert_cmpstr(str, ==, "1");
+
+str = qdict_get_try_str(val, "variant");
+g_assert_nonnull(str);
+g_assert_cmpstr(str, ==, "Unit test \"'$`\\ and etc.");
+
+str = qdict_get_try_str(val, "variant-id");
+g_assert_nonnull(str);
+g_assert_cmpstr(str, ==, "unit-test");
+
+QDECREF(ret);
+g_free(env[0]);
+}
+
int main(int argc, char **argv)
{
TestFixture fix;
@@ -972,6 +1025,8 @@ int main(int argc, char **argv)
g_test_add_data_func("/qga/guest-exec", &fix, test_qga_guest_exec);
g_test_add_data_func("/qga/guest-exec-invalid", &fix,
test_qga_guest_exec_invalid);
+g_test_add_data_func("/qga/guest-get-osinfo", &fix,
+ test_qga_guest_get_osinfo);
if (g_getenv("QGA_TEST_SIDE_EFFECTING")) {
g_test_add_data_func("/qga/fsfreeze-and-thaw", &fix,
--
2.13.1
[Qemu-devel] [PATCH v7 1/3] qemu-ga: add guest-get-osinfo command
Add a new 'guest-get-osinfo' command for reporting basic information of
the guest operating system. This includes machine architecture,
version and release of the kernel and several fields from os-release
file if it is present (as defined in [1]).
[1] https://www.freedesktop.org/software/systemd/man/os-release.html
Signed-off-by: Vinzenz Feenstra
Signed-off-by: Tomáš Golembiovský
---
qga/commands-posix.c | 136 +
qga/commands-win32.c | 185 +++
qga/qapi-schema.json | 65 ++
3 files changed, 386 insertions(+)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index d8e412275e..7a933a5f12 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -13,6 +13,7 @@
#include "qemu/osdep.h"
#include
+#include
#include
#include
#include
@@ -2577,3 +2578,138 @@ GuestUserList *qmp_guest_get_users(Error **err)
g_hash_table_destroy(cache);
return head;
}
+
+/* Replace escaped special characters with theire real values. The replacement
+ * is done in place -- returned value is in the original string.
+ */
+static void ga_osrelease_replace_special(gchar *value)
+{
+gchar *p, *p2, quote;
+
+/* Trim the string at first space or semicolon if it is not enclosed in
+ * single or double quotes. */
+if ((value[0] != '"') || (value[0] == '\'')) {
+p = strchr(value, ' ');
+if (p != NULL) {
+*p = 0;
+}
+p = strchr(value, ';');
+if (p != NULL) {
+*p = 0;
+}
+return;
+}
+
+quote = value[0];
+p2 = value;
+p = value + 1;
+while (*p != 0) {
+if (*p == '\\') {
+p++;
+switch (*p) {
+case '$':
+case '\'':
+case '"':
+case '\\':
+case '`':
+break;
+default:
+/* Keep literal backslash followed by whatever is there */
+p--;
+break;
+}
+} else if (*p == quote) {
+*p2 = 0;
+break;
+}
+*(p2++) = *(p++);
+}
+}
+
+static GKeyFile *ga_parse_osrelease(const char *fname)
+{
+gboolean ret;
+gchar *content = NULL;
+gchar *content2 = NULL;
+gsize len;
+GError *err = NULL;
+GKeyFile *keys = g_key_file_new();
+const char *group = "[os-release]\n";
+
+ret = g_file_get_contents(fname, &content, &len, &err);
+if (ret != TRUE) {
+slog("failed to read '%s', error: %s", fname, err->message);
+goto fail;
+}
+
+ret = g_utf8_validate(content, len, NULL);
+if (ret != TRUE) {
+slog("file is not utf-8 encoded: %s", fname);
+goto fail;
+}
+content2 = g_strdup_printf("%s%s", group, content);
+len += strlen(group);
+
+ret = g_key_file_load_from_data(keys, content2, len, G_KEY_FILE_NONE,
+&err);
+if (ret != TRUE) {
+slog("failed to parse file '%s', error: %s", fname, err->message);
+goto fail;
+}
+
+g_free(content);
+g_free(content2);
+return keys;
+
+fail:
+g_error_free(err);
+g_free(content);
+g_free(content2);
+g_key_file_free(keys);
+return NULL;
+}
+
+GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
+{
+GuestOSInfo *info = g_new0(GuestOSInfo, 1);
+
+struct utsname kinfo = {0};
+if (uname(&kinfo) != 0) {
+error_setg_errno(errp, errno, "uname failed");
+return NULL;
+}
+
+info->kernel_version = g_strdup(kinfo.version);
+info->kernel_release = g_strdup(kinfo.release);
+info->machine_hardware = g_strdup(kinfo.machine);
+
+GKeyFile *osrelease = ga_parse_osrelease("/etc/os-release");
+if (osrelease == NULL) {
+osrelease = ga_parse_osrelease("/usr/lib/os-release");
+}
+
+if (osrelease != NULL) {
+char *value;
+
+#define GET_FIELD(field, osfield) do { \
+value = g_key_file_get_value(osrelease, "os-release", osfield, NULL); \
+if (value != NULL) { \
+ga_osrelease_replace_special(value); \
+info->has_ ## field = true; \
+info->field = value; \
+} \
+} while (0)
+GET_FIELD(id, "ID");
+GET_FIELD(name, "NAME");
+GET_FIELD(pretty_name, "PRETTY_NAME");
+GET_FIELD(version, "VERSION");
+GET_FIELD(version_id, "VERSION_ID");
+GET_FIELD(variant, "VARIANT");
+GET_FIELD(variant_id, "VARIANT_ID");
+#undef GET_FIELD
+
+g_key_file_free(osrelease);
+}
+
+return info;
+}
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 439d229225..e471f5561b 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1639,3 +1639,188 @@ GuestUserList *qmp_guest_get_users(Error **err)
return NULL;
#endif
}
+
+typedef struct _ga_matrix_lookup_t {
+int major;
+int minor;
+char const *version;
+char const *version_id;
+} ga_matrix_lookup_t;
+
+static ga_m
[Qemu-devel] [PATCH v3] s390: return unavailable features via query-cpu-definitions
The response for query-cpu-definitions didn't include the
unavailable-features field, which is used by libvirt to figure
out whether a certain cpu model is usable on the host.
The unavailable features are now computed by obtaining the host CPU
model and comparing it against the known CPU models. The comparison
takes into account the generation, the GA level and the feature
bitmaps. In the case of a CPU generation/GA level mismatch
a feature called "type" is reported to be missing.
As a result, the output of virsh domcapabilities would change
from something like
...
z10EC-base
z9EC-base
z196.2-base
z900-base
z990
...
to
...
z10EC-base
z9EC-base
z196.2-base
z900-base
z990
...
Signed-off-by: Viktor Mihajlovski
---
v1 -> v2:
Account for model generation and GA level, not only on features.
v2 -> v3:
Prevent repetitive failures by calling get_max_cpu_model only once.
Ignore get_max_cpu_model failures in query_cpu_definitions in order
return CPU models even if the host CPU model is not available.
target/s390x/cpu_models.c | 62 +++
1 file changed, 57 insertions(+), 5 deletions(-)
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 63903c2..7cb55dc 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -283,10 +283,41 @@ void s390_cpu_list(FILE *f, fprintf_function print)
}
}
+static S390CPUModel *get_max_cpu_model(Error **errp);
+
#ifndef CONFIG_USER_ONLY
+static void list_add_feat(const char *name, void *opaque);
+
+static void check_unavailable_features(const S390CPUModel *max_model,
+ const S390CPUModel *model,
+ strList **unavailable)
+{
+S390FeatBitmap missing;
+
+/* check general model compatibility */
+if (max_model->def->gen < model->def->gen ||
+(max_model->def->gen == model->def->gen &&
+ max_model->def->ec_ga < model->def->ec_ga)) {
+list_add_feat("type", unavailable);
+}
+
+/* detect missing features if any to properly report them */
+bitmap_andnot(missing, model->features, max_model->features,
+ S390_FEAT_MAX);
+if (!bitmap_empty(missing, S390_FEAT_MAX)) {
+s390_feat_bitmap_to_ascii(missing, unavailable, list_add_feat);
+}
+}
+
+struct CpuDefinitionInfoListData {
+CpuDefinitionInfoList *list;
+S390CPUModel *model;
+};
+
static void create_cpu_model_list(ObjectClass *klass, void *opaque)
{
-CpuDefinitionInfoList **cpu_list = opaque;
+struct CpuDefinitionInfoListData *cpu_list_data = opaque;
+CpuDefinitionInfoList **cpu_list = &cpu_list_data->list;
CpuDefinitionInfoList *entry;
CpuDefinitionInfo *info;
char *name = g_strdup(object_class_get_name(klass));
@@ -300,7 +331,19 @@ static void create_cpu_model_list(ObjectClass *klass, void
*opaque)
info->migration_safe = scc->is_migration_safe;
info->q_static = scc->is_static;
info->q_typename = g_strdup(object_class_get_name(klass));
-
+/* check for unavailable features */
+if (cpu_list_data->model) {
+Object *obj;
+S390CPU *sc;
+obj = object_new(object_class_get_name(klass));
+sc = S390_CPU(obj);
+if (sc->model) {
+info->has_unavailable_features = true;
+check_unavailable_features(cpu_list_data->model, sc->model,
+ &info->unavailable_features);
+}
+object_unref(obj);
+}
entry = g_malloc0(sizeof(*entry));
entry->value = info;
@@ -310,11 +353,20 @@ static void create_cpu_model_list(ObjectClass *klass,
void *opaque)
CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
{
-CpuDefinitionInfoList *list = NULL;
+struct CpuDefinitionInfoListData list_data = {
+.list = NULL,
+};
+
+list_data.model = get_max_cpu_model(errp);
+if (*errp) {
+error_free(*errp);
+*errp = NULL;
+}
-object_class_foreach(create_cpu_model_list, TYPE_S390_CPU, false, &list);
+object_class_foreach(create_cpu_model_list, TYPE_S390_CPU, false,
+ &list_data);
-return list;
+return list_data.list;
}
static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info,
--
1.9.1
Re: [Qemu-devel] [RFC 00/29] postcopy+vhost-user/shared ram
* Marc-André Lureau (marcandre.lur...@gmail.com) wrote: > Hi > > On Thu, Jun 29, 2017 at 8:56 PM Dr. David Alan Gilbert > wrote: > > > * Dr. David Alan Gilbert (git) (dgilb...@redhat.com) wrote: > > > From: "Dr. David Alan Gilbert" > > > > > > Hi, > > > This is a RFC/WIP series that enables postcopy migration > > > with shared memory to a vhost-user process. > > > It's based off current-head + Juan's load_cleanup series, and > > > Alexey's bitmap series (v4). It's very lightly tested and seems > > > to work, but it's quite rough. > > > > Marc-André asked if I had a git with it all applied; so here we are: > > https://github.com/dagrh/qemu/commits/vhost > > g...@github.com:dagrh/qemu.git on the vhost branch > > > > > I started looking at the series, but I am not familiar with ufd/postcopy. I'm similarly unfamiliar with the vhost code when I started this (which probably shows!). The main thing about ufd is that a process registers with the ufd system and registers an area of memory with it; accesses to the memory block until the page is available, a message is sent down the ufd, and whoever receives that message may then respond by atomically copying a page into memory, or wakeing the process when it knows the page is there. This is the first time we've tried to use userfaultfd with shared memory and it does need a very recent kernel for it (4.11.0 or rhel 7.4 beta) > Could you update vhost-user.txt to describe the new messages? See below; I'll add that in. > Otherwise, > make check hangs in /x86_64/vhost-user/connect-fail (might be an unrelated > regression?) Thanks Entirely possible I broke it; I'll have a look - at the moment I'm more interested in comments on the structure of this set. Dave diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt index 481ab56e35..fec4cd0ffe 100644 --- a/docs/interop/vhost-user.txt +++ b/docs/interop/vhost-user.txt @@ -273,6 +273,14 @@ Once the source has finished migration, rings will be stopped by the source. No further update must be done before rings are restarted. +In postcopy migration the slave is started before all the memory has been +received from the source host, and care must be taken to avoid accessing pages +that have yet to be received. The slave opens a 'userfault'-fd and registers +the memory with it; this fd is then passed back over to the master. +The master services requests on the userfaultfd for pages that are accessed +and when the page is available it performs WAKE ioctl's on the userfaultfd +to wake the stalled slave. + IOMMU support - @@ -326,6 +334,7 @@ Protocol features #define VHOST_USER_PROTOCOL_F_REPLY_ACK 3 #define VHOST_USER_PROTOCOL_F_MTU4 #define VHOST_USER_PROTOCOL_F_SLAVE_REQ 5 +#define VHOST_USER_PROTOCOL_F_POSTCOPY 6 Master message types @@ -402,12 +411,17 @@ Master message types Id: 5 Equivalent ioctl: VHOST_SET_MEM_TABLE Master payload: memory regions description + Slave payload: (postcopy only) memory regions description Sets the memory map regions on the slave so it can translate the vring addresses. In the ancillary data there is an array of file descriptors for each memory mapped region. The size and ordering of the fds matches the number and ordering of memory regions. + When postcopy-listening has been received, SET_MEM_TABLE replies with + the bases of the memory mapped regions to the master. It must have mmap'd + the regions and enabled userfaultfd on them. + * VHOST_USER_SET_LOG_BASE Id: 6 @@ -580,6 +594,29 @@ Master message types This request should be send only when VIRTIO_F_IOMMU_PLATFORM feature has been successfully negotiated. + * VHOST_USER_POSTCOPY_ADVISE + Id: 23 + Master payload: N/A + Slave payload: userfault fd + u64 + + Master advises slave that a migration with postcopy enabled is underway, + the slave must open a userfaultfd for later use. + Note that at this stage the migration is still in precopy mode. + + * VHOST_USER_POSTCOPY_LISTEN + Id: 24 + Master payload: N/A + + Master advises slave that a transition to postcopy mode has happened. + + * VHOST_USER_POSTCOPY_END + Id: 25 + Slave payload: u64 + + Master advises that postcopy migration has now completed. The + slave must disable the userfaultfd. The response is an acknowledgement + only. + Slave message types --- > -- > Marc-André Lureau -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v4 11/13] virtio-console: chardev hotswap support
On 06/29/2017 01:02 PM, Marc-André Lureau wrote:
Hi
Looks good, but please write something in the commit message about what needs
to be done for be-change (what this patch does).
thanks
Hi,
thank you! I guess the description should look like
virtio-console: chardev hotswap support
In case of a backend change, the handler functions and the watch have
to be reset.
Also, avoid unsafe qemu_chr_fe_get_driver() usage even though the pointer
is not really stored.
Amit, have you had a chance to kindly look at this?
/Anton
- Original Message -
Signed-off-by: Anton Nefedov
Reviewed-by: Vladimir Sementsov-Ogievskiy
CC: Amit Shah
---
hw/char/virtio-console.c | 35 ++-
1 file changed, 30 insertions(+), 5 deletions(-)
diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
index afb4949..198b2a8 100644
--- a/hw/char/virtio-console.c
+++ b/hw/char/virtio-console.c
@@ -49,7 +49,7 @@ static ssize_t flush_buf(VirtIOSerialPort *port,
VirtConsole *vcon = VIRTIO_CONSOLE(port);
ssize_t ret;
-if (!qemu_chr_fe_get_driver(&vcon->chr)) {
+if (!qemu_chr_fe_backend_connected(&vcon->chr)) {
/* If there's no backend, we can just say we consumed all data. */
return len;
}
@@ -163,12 +163,35 @@ static void chr_event(void *opaque, int event)
}
}
+static int chr_be_change(void *opaque)
+{
+VirtConsole *vcon = opaque;
+VirtIOSerialPort *port = VIRTIO_SERIAL_PORT(vcon);
+VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_GET_CLASS(port);
+
+if (k->is_console) {
+qemu_chr_fe_set_handlers(&vcon->chr, chr_can_read, chr_read,
+ NULL, chr_be_change, vcon, NULL, true);
+} else {
+qemu_chr_fe_set_handlers(&vcon->chr, chr_can_read, chr_read,
+ chr_event, chr_be_change, vcon, NULL,
false);
+}
+
+if (vcon->watch) {
+g_source_remove(vcon->watch);
+vcon->watch = qemu_chr_fe_add_watch(&vcon->chr,
+G_IO_OUT | G_IO_HUP,
+chr_write_unblocked, vcon);
+}
+
+return 0;
+}
+
static void virtconsole_realize(DeviceState *dev, Error **errp)
{
VirtIOSerialPort *port = VIRTIO_SERIAL_PORT(dev);
VirtConsole *vcon = VIRTIO_CONSOLE(dev);
VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_GET_CLASS(dev);
-Chardev *chr = qemu_chr_fe_get_driver(&vcon->chr);
if (port->id == 0 && !k->is_console) {
error_setg(errp, "Port number 0 on virtio-serial devices reserved "
@@ -176,7 +199,7 @@ static void virtconsole_realize(DeviceState *dev, Error
**errp)
return;
}
-if (chr) {
+if (qemu_chr_fe_backend_connected(&vcon->chr)) {
/*
* For consoles we don't block guest data transfer just
* because nothing is connected - we'll just let it go
@@ -188,11 +211,13 @@ static void virtconsole_realize(DeviceState *dev, Error
**errp)
*/
if (k->is_console) {
qemu_chr_fe_set_handlers(&vcon->chr, chr_can_read, chr_read,
- NULL, NULL, vcon, NULL, true);
+ NULL, chr_be_change,
+ vcon, NULL, true);
virtio_serial_open(port);
} else {
qemu_chr_fe_set_handlers(&vcon->chr, chr_can_read, chr_read,
- chr_event, NULL, vcon, NULL, false);
+ chr_event, chr_be_change,
+ vcon, NULL, false);
}
}
}
--
2.7.4
--
/Anton
Re: [Qemu-devel] [RFC PATCH 7/8] VFIO: Add new IOCTL for IOMMU TLB invalidate propagation
Hi Yi,
On 02/07/17 11:06, Liu, Yi L wrote:
> On Fri, May 12, 2017 at 01:11:02PM +0100, Jean-Philippe Brucker wrote:
>
> Hi Jean,
>
> As we've got a few discussions on it. I'd like to have a conclusion and
> make it as a reference for future discussion.
>
> Currently, we are inclined to have a hybrid format for the iommu tlb
> invalidation from userspace(vIOMMU or userspace driver).
>
> Based on the previous discussion, may the below work?
>
> 1. Add a IOCTL for iommu tlb invalidation.
>
> VFIO_IOMMU_TLB_INVALIDATE
>
> struct vfio_iommu_tlb_invalidate {
>__u32 argsz;
>__u32 length;
Wouldn't argsz be exactly length + 8? Might be redundant in this case.
>__u8data[];
> };
>
> comments from Alex William: is it more suitable to add a new flag bit on
> vfio_device_svm(a structure defined in patch 5 of this patchset), the data
> structure is so similar.
>
> Personally, I'm ok with it. Pls let me know your thoughts. However, the
> precondition is we accept the whole definition in this email. If not, the
> vfio_iommu_tlb_invalidate would be defined differently.
With this proposal sharing the structure makes sense. As I understand it
we're keeping the VFIO_IOMMU_TLB_INVALIDATE ioctl? In which case adding a
flag bit would be redundant.
> 2. Define a structure in include/uapi/linux/iommu.h(newly added header file)
>
> struct iommu_tlb_invalidate {
> __u32 scope;
> /* pasid-selective invalidation described by @pasid */
> #define IOMMU_INVALIDATE_PASID(1 << 0)
> /* address-selevtive invalidation described by (@vaddr, @size) */
> #define IOMMU_INVALIDATE_VADDR(1 << 1)
> __u32 flags;
> /* targets non-pasid mappings, @pasid is not valid */
> #define IOMMU_INVALIDATE_NO_PASID (1 << 0)
Although it was my proposal, I don't like this flag. In ARM SMMU, we're
using a special mode where PASID 0 is reserved and any traffic without
PASID uses entry 0 of the PASID table. So I proposed the "NO_PASID" flag
to invalidate that special context explicitly. But this means that
invalidation packet targeted at that context will have "scope = PASID" and
"flags = NO_PASID", which is utterly confusing.
I now think that we should get rid of the IOMMU_INVALIDATE_NO_PASID flag
and just use PASID 0 to invalidate this context on ARM. I don't think
other architectures would use the NO_PASID flag anyway, but might be mistaken.
> /* indicating that the pIOMMU doesn't need to invalidate
>all intermediate tables cached as part of the PTE for
>vaddr, only the last-level entry (pte). This is a hint. */
> #define IOMMU_INVALIDATE_VADDR_LEAF (1 << 1)
> __u32 pasid;
> __u64 vaddr;
> __u64 size;
> __u8data[];
> };
>
> For this part, the scope and flags are basically aligned with your previous
> email. I renamed the prefix to be "IOMMU_". In my opinion, the scope and flags
> would be filled by vIOMMU emulator and be parsed by underlying iommu driver,
> it is much more suitable to be defined in a uapi header file.
I tend to agree, defining a single structure in a new IOMMU UAPI file is
better than having identical structures both in uapi/linux/vfio.h and
linux/iommu.h. This way we avoid VFIO having to copy the same structure
field by field. Arch-specific structures that go in
iommu_tlb_invalidate.data also ought to be defined in uapi/linux/iommu.h
> Besides the reason above, I don't want VFIO engae too much on the data
> parsing.
> If we move the scope,flags,pasid,vaddr,size fields to
> vfio_iommu_tlb_invalidate,
> then both kernel space vfio and user space vfio needs to do much parsing. So I
> may prefer the way above.
Would the entire validation of struct iommu_tlb_invalidate be offloaded to
the IOMMU subsystem? Checking the structure sizes, copying from user, and
validating the flags?
I guess it's mostly an implementation detail, but it might be better to
keep this code in VFIO as well, even for the validation of
iommu_tlb_invalidate.data (which would require VFIO to keep track of the
model used during bind). This way VFIO sanitizes what comes from
userspace, whilst the IOMMU subsystem only deals with valid kernel
structures, and another subsystem could easily reuse the
iommu_tlb_invalidate API.
The IOMMU subsystem would still validate the meaning of the fields, for
instance whether a given combination of flag is allowed or if the PASID
exists.
Thanks,
Jean
> If you've got any other idea, pls feel free to post it. It's welcomed.
>
> Thanks,
> Yi L
>
>> Hi Yi,
>>
>> On 26/04/17 11:12, Liu, Yi L wrote:
>>> From: "Liu, Yi L"
>>>
>>> This patch adds VFIO_IOMMU_TLB_INVALIDATE to propagate IOMMU TLB
>>> invalidate request from guest to host.
>>>
>>> In the case of SVM virtualization on VT-d, host IOMMU driver has
>>> no knowledge of caching structure updates unless the guest
>>> invalidation activities are passed down to the host. So a new
>>> IOCTL is needed to propagate the guest cache invalidation through
>>> VFIO.
>>>
>>> Signed-of
Re: [Qemu-devel] [PATCH RFC 1/6] hw/acpi: remove dead acpi code
On Fri, 30 Jun 2017 02:27:40 +0300
"Michael S. Tsirkin" wrote:
> On Fri, Jun 30, 2017 at 12:55:57AM +0300, Aleksandr Bezzubikov wrote:
> > Signed-off-by: Aleksandr Bezzubikov
>
>
> I agree it seems unused. I'm not sure why do we have this
> code. Igor?
it's leftovers of original ASL code from SeaBIOS,
my conversion only was 1:1 from ASL to AML to avoid regressions
and almost no cleanups has been done on result.
It should be safe to remove these objects as they are not used by
Q35 (no AML or hw users for these ports)
Reviewed-by: Igor Mammedov
>
> > ---
> > hw/i386/acpi-build.c | 10 --
> > 1 file changed, 10 deletions(-)
> >
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index afcadac..6fce967 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1912,16 +1912,6 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > build_piix4_pci0_int(dsdt);
> > } else {
> > sb_scope = aml_scope("_SB");
> > -aml_append(sb_scope,
> > -aml_operation_region("PCST", AML_SYSTEM_IO, aml_int(0xae00),
> > 0x0c));
> > -aml_append(sb_scope,
> > -aml_operation_region("PCSB", AML_SYSTEM_IO, aml_int(0xae0c),
> > 0x01));
> > -field = aml_field("PCSB", AML_ANY_ACC, AML_NOLOCK,
> > AML_WRITE_AS_ZEROS);
> > -aml_append(field, aml_named_field("PCIB", 8));
> > -aml_append(sb_scope, field);
> > -aml_append(dsdt, sb_scope);
> > -
> > -sb_scope = aml_scope("_SB");
> > dev = aml_device("PCI0");
> > aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
> > aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
> > --
> > 2.7.4
Re: [Qemu-devel] [PATCH v4 13/13] serial: chardev hotswap support
Hi Paolo, Michael,
Any feedback on this change? (and a cosmetic patch 12 of this series)
Thanks a lot in advance,
/Anton
On 06/26/2017 07:45 PM, Anton Nefedov wrote:
for a backend change, a number of ioctls has to be replayed to sync
the current setup of a frontend to a backend tty. This is hopefully
enough so we don't have to track, store and replay the whole original
control byte sequence.
Signed-off-by: Anton Nefedov
Reviewed-by: Vladimir Sementsov-Ogievskiy
CC: Michael S. Tsirkin
CC: Paolo Bonzini
---
hw/char/serial.c | 32 ++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/hw/char/serial.c b/hw/char/serial.c
index e47f0b6..9aec6c6 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -891,9 +891,37 @@ static void serial_reset(void *opaque)
s->msr &= ~UART_MSR_ANY_DELTA;
}
+static int serial_be_change(void *opaque)
+{
+SerialState *s = opaque;
+
+qemu_chr_fe_set_handlers(&s->chr, serial_can_receive1, serial_receive1,
+ serial_event, serial_be_change, s, NULL, true);
+
+serial_update_parameters(s);
+
+qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_BREAK,
+ &s->last_break_enable);
+
+s->poll_msl = (s->ier & UART_IER_MSI) ? 1 : 0;
+serial_update_msl(s);
+
+if (s->poll_msl >= 0 && !(s->mcr & UART_MCR_LOOP)) {
+serial_update_tiocm(s);
+}
+
+if (s->watch_tag > 0) {
+g_source_remove(s->watch_tag);
+s->watch_tag = qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
+ serial_watch_cb, s);
+}
+
+return 0;
+}
+
void serial_realize_core(SerialState *s, Error **errp)
{
-if (!qemu_chr_fe_get_driver(&s->chr)) {
+if (!qemu_chr_fe_backend_connected(&s->chr)) {
error_setg(errp, "Can't create serial device, empty char device");
return;
}
@@ -904,7 +932,7 @@ void serial_realize_core(SerialState *s, Error **errp)
qemu_register_reset(serial_reset, s);
qemu_chr_fe_set_handlers(&s->chr, serial_can_receive1, serial_receive1,
- serial_event, NULL, s, NULL, true);
+ serial_event, serial_be_change, s, NULL, true);
fifo8_create(&s->recv_fifo, UART_FIFO_LENGTH);
fifo8_create(&s->xmit_fifo, UART_FIFO_LENGTH);
serial_reset(s);
Re: [Qemu-devel] [PATCH v2 01/15] configure: add the disable-tcg option
On Mon, Jul 03, 2017 at 12:55:04PM +0200, Paolo Bonzini wrote: > > > On 03/07/2017 12:33, Daniel P. Berrange wrote: > >> i386-softmmu and x86_64-softmmu are singled out here, because they're > >> the only targets where --disable-tcg compiles. For the others, more > >> work is needed (see patches 6-15 in Yang Zhong's series). > > > > Even with that, you still can't disable TCG if building on a non-x86 > > host, since that'd leave you with no available CPU at all. So the > > code still needs refactoring to check architectures properly. > > It would leave you with a tools-only build; whether that's a good idea, > it's another story. I think it's acceptable, but others may disagree. If you do 'configure --target-list=x86_64-softmmu --disable-tcg' on a non-x86 host I would be surprised to find x86_64-softmmu not built. IMHO if you explicitly request system emulators and its not posisble to build then, an error should be report, rather than silently ignoring the request. 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] [PATCH v5 1/2] iotests: Use absolute paths for executables
On 07/02/2017 10:05 AM, Max Reitz wrote: > A user may specify a relative path for accessing qemu, qemu-img, etc. > through environment variables ($QEMU_PROG and friends) or a symlink. > > If a test decides to change its working directory, relative paths will > cease to work, however. Work around this by making all of the paths to > programs that should undergo testing absolute. Besides "realpath", we > also have to use "type -p" to support programs in $PATH. > > As a side effect, this fixes specifying these programs as symlinks for > out-of-tree builds: Before, you would have to create two symlinks, one > in the build and one in the source tree (the first one for common.config > to find, the second one for the iotest to use). Now it is sufficient to > create one in the build tree because common.config will resolve it. > > Reported-by: Kevin Wolf > Signed-off-by: Max Reitz > --- > tests/qemu-iotests/common.config | 11 +++ > 1 file changed, 11 insertions(+) > Reviewed-by: Eric Blake Tested-by: Eric Blake -- 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] [PATCH] spapr: fix memory hotplug error path
QEMU shouldn't abort if spapr_add_lmbs()->spapr_drc_attach() fails.
Let's propagate the error instead, like it is done everywhere else
where spapr_drc_attach() is called.
Signed-off-by: Greg Kurz
---
hw/ppc/spapr.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 70b3fd374e2b..e103be500189 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2601,6 +2601,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t
addr_start, uint64_t size,
int i, fdt_offset, fdt_size;
void *fdt;
uint64_t addr = addr_start;
+Error *local_err = NULL;
for (i = 0; i < nr_lmbs; i++) {
drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB,
@@ -2611,7 +2612,12 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t
addr_start, uint64_t size,
fdt_offset = spapr_populate_memory_node(fdt, node, addr,
SPAPR_MEMORY_BLOCK_SIZE);
-spapr_drc_attach(drc, dev, fdt, fdt_offset, errp);
+spapr_drc_attach(drc, dev, fdt, fdt_offset, &local_err);
+if (local_err) {
+g_free(fdt);
+error_propagate(errp, local_err);
+return;
+}
addr += SPAPR_MEMORY_BLOCK_SIZE;
}
/* send hotplug notification to the
@@ -2657,7 +2663,7 @@ static void spapr_memory_plug(HotplugHandler
*hotplug_dev, DeviceState *dev,
spapr_add_lmbs(dev, addr, size, node,
spapr_ovec_test(ms->ov5_cas, OV5_HP_EVT),
- &error_abort);
+ &local_err);
out:
error_propagate(errp, local_err);
[Qemu-devel] [PATCH v3 1/5] qapi/qnull: Add own header
Reviewed-by: Markus Armbruster
Signed-off-by: Max Reitz
---
include/qapi/qmp/qnull.h | 26 ++
include/qapi/qmp/qobject.h | 8
include/qapi/qmp/types.h | 1 +
qobject/qnull.c| 2 +-
tests/check-qnull.c| 2 +-
5 files changed, 29 insertions(+), 10 deletions(-)
create mode 100644 include/qapi/qmp/qnull.h
diff --git a/include/qapi/qmp/qnull.h b/include/qapi/qmp/qnull.h
new file mode 100644
index 000..48edad4
--- /dev/null
+++ b/include/qapi/qmp/qnull.h
@@ -0,0 +1,26 @@
+/*
+ * QNull
+ *
+ * Copyright (C) 2015 Red Hat, Inc.
+ *
+ * Authors:
+ * Markus Armbruster
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1
+ * or later. See the COPYING.LIB file in the top-level directory.
+ */
+
+#ifndef QNULL_H
+#define QNULL_H
+
+#include "qapi/qmp/qobject.h"
+
+extern QObject qnull_;
+
+static inline QObject *qnull(void)
+{
+qobject_incref(&qnull_);
+return &qnull_;
+}
+
+#endif /* QNULL_H */
diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
index b8ddbca..ef1d1a9 100644
--- a/include/qapi/qmp/qobject.h
+++ b/include/qapi/qmp/qobject.h
@@ -93,12 +93,4 @@ static inline QType qobject_type(const QObject *obj)
return obj->type;
}
-extern QObject qnull_;
-
-static inline QObject *qnull(void)
-{
-qobject_incref(&qnull_);
-return &qnull_;
-}
-
#endif /* QOBJECT_H */
diff --git a/include/qapi/qmp/types.h b/include/qapi/qmp/types.h
index a4bc662..749ac44 100644
--- a/include/qapi/qmp/types.h
+++ b/include/qapi/qmp/types.h
@@ -19,5 +19,6 @@
#include "qapi/qmp/qstring.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qlist.h"
+#include "qapi/qmp/qnull.h"
#endif /* QAPI_QMP_TYPES_H */
diff --git a/qobject/qnull.c b/qobject/qnull.c
index c124d05..43918f1 100644
--- a/qobject/qnull.c
+++ b/qobject/qnull.c
@@ -12,7 +12,7 @@
#include "qemu/osdep.h"
#include "qemu-common.h"
-#include "qapi/qmp/qobject.h"
+#include "qapi/qmp/qnull.h"
QObject qnull_ = {
.type = QTYPE_QNULL,
diff --git a/tests/check-qnull.c b/tests/check-qnull.c
index 8dd1c96..4a67b9a 100644
--- a/tests/check-qnull.c
+++ b/tests/check-qnull.c
@@ -8,7 +8,7 @@
*/
#include "qemu/osdep.h"
-#include "qapi/qmp/qobject.h"
+#include "qapi/qmp/qnull.h"
#include "qemu-common.h"
#include "qapi/qobject-input-visitor.h"
#include "qapi/qobject-output-visitor.h"
--
2.9.4
Re: [Qemu-devel] [PATCH v2] tests: Avoid non-portable 'echo -ARG'
On 07/02/2017 09:49 AM, Max Reitz wrote:
> On 2017-06-30 21:58, Eric Blake wrote:
>> POSIX says that backslashes in the arguments to 'echo', as well as
>> any use of 'echo -n' and 'echo -e', are non-portable; it recommends
>> people should favor 'printf' instead. This is definitely true where
>> we do not control which shell is running (such as in makefile snippets
>> or in documentation examples). But even for scripts where we
>> require bash (and therefore, where echo does what we want by default),
>> it is still possible to use 'shopt -s xpg_echo' to change bash's
>> behavior of echo. And setting a good example never hurts when we are
>> not sure if a snippet will be copied from a bash-only script to a
>> general shell script (although I don't change the use of non-portable
>> \e for ESC when we know the running shell is bash).
>>
>> Replace 'echo -n "..."' with 'printf %s "..."', and 'echo -e "..."'
>> with 'printf %b "...\n"', with the optimization that the %s/%b
>> argument can be omitted if the string being printed contains no
>> substitutions that could result in '%' (trivial if there is no
>> $ or `, but also possible when using things like $ret that are
>> known to be numeric given the assignment to ret just above).
>
> Well, yes, possible, but it's no longer trivial... And just using '%s'
> or '%b' in these cases would make reading the code simpler, in my opinion.
>
> Sure, omitting it makes sense for constant format strings, but for
> variable the cost outweighs the benefit, in my opinion.
>
> (And since this is a bit supposed to go through qemu-trivial, it should
> be trivial, right? :-))
I can spin up a v3 that adds more %s/%b anywhere a $ appears where the
variable being printed is not obviously assigned in the immediately
preceding context.
>> +++ b/tests/qemu-iotests/check
>
> [...]
>
>> @@ -281,9 +281,9 @@ do
>> rm -f $seq.out.bad
>> lasttime=`sed -n -e "/^$seq /s/.* //p" <$TIMESTAMP_FILE`
>> if [ "X$lasttime" != X ]; then
>> -echo -n " ${lasttime}s ..."
>> +printf " ${lasttime}s ..."
>
> You cannot prove this doesn't contain a %. In fact, I can very easily
> put a % into the timestamp file and let printf print it here.
>
> Sure, there shouldn't be one there, but on one hand it's still possible
> and on the other, finding out that there shouldn't be a % there is very
> much non-trivial.
Indeed, any variable whose contents are provided from the filesystem
rather than directly by the script are suspects for abuse. In a
testsuite, it's often feasible to assume that abuse is not happening,
but being robust doesn't hurt.
>> +++ b/tests/tcg/cris/Makefile
>> @@ -150,17 +150,17 @@ check_addcv17.tst: crtv10.o sysv10.o
>> build: $(CRT) $(SYS) $(TESTCASES)
>>
>> check: $(CRT) $(SYS) $(TESTCASES)
>> -@echo -e "\nQEMU simulator."
>> +@printf "\nQEMU simulator.\n"
>> for case in $(TESTCASES); do \
>> -echo -n "$$case "; \
>> +printf "$$case "; \
>
> This is another rather non-trivial case: Checking that this doesn't
> contain a % means reading through the whole list defining TESTCASES.
We'd be crazy to define TESTCASES with %, but I agree that the context
is not close, so being robust doesn't hurt.
Sounds like a v3 is coming up.
--
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] [PATCH v3 0/5] block: Don't compare strings in bdrv_reopen_prepare()
bdrv_reopen_prepare() assumes that all BDS options are strings, which is not necessarily correct. This series introduces a new qobject_is_equal() function which can be used to test whether any options have changed, independently of their type. v3: - Patch 1: - Fix copyright header [Markus] - Drop qobject.h include from qnull.c [Markus] - Replacing all QObject includes by qmp/types.h in target/i386/cpu.c became unnecessary due to 01b2ffcedd94ad7b42bc870e4c6936c87ad03429 (kept R-b anyway, because v2 contained exactly the same change as that commit) - Patch 2: - Move the code for QInt and QFloat to QNum - Write function descriptions in imperative mood [Markus] - Break comment lines after 70 characters [Markus] - Style changes [Markus] - Patch 3: Add a comment on what will go wrong when using e.g. -blockdev and the qemu-io reopen command together [Markus] - Patch 5: Added some test cases for qobject_is_equal() [Markus] git-backport-diff against v2: Key: [] : patches are identical [] : number of functional differences between upstream/downstream patch [down] : patch is downstream-only The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively 001/5:[0017] [FC] 'qapi/qnull: Add own header' 002/5:[0120] [FC] 'qapi: Add qobject_is_equal()' 003/5:[0018] [FC] 'block: qobject_is_equal() in bdrv_reopen_prepare()' 004/5:[] [--] 'iotests: Add test for non-string option reopening' 005/5:[down] 'tests: Add check-qobject for equality tests' Max Reitz (5): qapi/qnull: Add own header qapi: Add qobject_is_equal() block: qobject_is_equal() in bdrv_reopen_prepare() iotests: Add test for non-string option reopening tests: Add check-qobject for equality tests tests/Makefile.include | 4 +- include/qapi/qmp/qbool.h | 1 + include/qapi/qmp/qdict.h | 1 + include/qapi/qmp/qlist.h | 1 + include/qapi/qmp/qnull.h | 28 include/qapi/qmp/qnum.h| 1 + include/qapi/qmp/qobject.h | 17 +-- include/qapi/qmp/qstring.h | 1 + include/qapi/qmp/types.h | 1 + block.c| 31 +++-- qobject/qbool.c| 8 ++ qobject/qdict.c| 29 + qobject/qlist.c| 32 + qobject/qnull.c| 11 +- qobject/qnum.c | 53 qobject/qobject.c | 29 + qobject/qstring.c | 9 ++ tests/check-qnull.c| 2 +- tests/check-qobject.c | 312 + tests/qemu-iotests/133 | 9 ++ tests/qemu-iotests/133.out | 5 + 21 files changed, 561 insertions(+), 24 deletions(-) create mode 100644 include/qapi/qmp/qnull.h create mode 100644 tests/check-qobject.c -- 2.9.4
[Qemu-devel] [PATCH v3 5/5] tests: Add check-qobject for equality tests
Add a new test file (check-qobject.c) for unit tests that concern
QObjects as a whole.
Its only purpose for now is to test the qobject_is_equal() function.
Signed-off-by: Max Reitz
---
tests/Makefile.include | 4 +-
tests/check-qobject.c | 312 +
2 files changed, 315 insertions(+), 1 deletion(-)
create mode 100644 tests/check-qobject.c
diff --git a/tests/Makefile.include b/tests/Makefile.include
index c738e92..ff0022d 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -18,6 +18,7 @@ check-unit-y += tests/check-qlist$(EXESUF)
gcov-files-check-qlist-y = qobject/qlist.c
check-unit-y += tests/check-qnull$(EXESUF)
gcov-files-check-qnull-y = qobject/qnull.c
+check-unit-y += tests/check-qobject$(EXESUF)
check-unit-y += tests/check-qjson$(EXESUF)
gcov-files-check-qjson-y = qobject/qjson.c
check-unit-y += tests/test-qobject-output-visitor$(EXESUF)
@@ -507,7 +508,7 @@ GENERATED_FILES += tests/test-qapi-types.h
tests/test-qapi-visit.h \
tests/test-qmp-introspect.h
test-obj-y = tests/check-qnum.o tests/check-qstring.o tests/check-qdict.o \
- tests/check-qlist.o tests/check-qnull.o \
+ tests/check-qlist.o tests/check-qnull.o tests/check-qobject.o \
tests/check-qjson.o \
tests/test-coroutine.o tests/test-string-output-visitor.o \
tests/test-string-input-visitor.o tests/test-qobject-output-visitor.o \
@@ -540,6 +541,7 @@ tests/check-qstring$(EXESUF): tests/check-qstring.o
$(test-util-obj-y)
tests/check-qdict$(EXESUF): tests/check-qdict.o $(test-util-obj-y)
tests/check-qlist$(EXESUF): tests/check-qlist.o $(test-util-obj-y)
tests/check-qnull$(EXESUF): tests/check-qnull.o $(test-util-obj-y)
+tests/check-qobject$(EXESUF): tests/check-qobject.o $(test-util-obj-y)
tests/check-qjson$(EXESUF): tests/check-qjson.o $(test-util-obj-y)
tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o
$(test-qom-obj-y)
tests/check-qom-proplist$(EXESUF): tests/check-qom-proplist.o $(test-qom-obj-y)
diff --git a/tests/check-qobject.c b/tests/check-qobject.c
new file mode 100644
index 000..a68cb1e
--- /dev/null
+++ b/tests/check-qobject.c
@@ -0,0 +1,312 @@
+/*
+ * Generic QObject unit-tests.
+ *
+ * Copyright (C) 2017 Red Hat Inc.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ */
+#include "qemu/osdep.h"
+
+#include "qapi/qmp/types.h"
+#include "qemu-common.h"
+
+/* Marks the end of the test_equality() argument list.
+ * We cannot use NULL there because that is a valid argument. */
+static QObject _test_equality_end_of_arguments;
+
+/**
+ * Test whether all variadic QObject *arguments are equal (@expected
+ * is true) or whether they are all not equal (@expected is false).
+ * Every QObject is tested to be equal to itself (to test
+ * reflexivity), all tests are done both ways (to test symmetry), and
+ * transitivity is not assumed but checked (each object is compared to
+ * every other one).
+ *
+ * Note that qobject_is_equal() is not really an equivalence relation,
+ * so this function may not be used for all objects (reflexivity is
+ * not guaranteed).
+ */
+static void do_test_equality(bool expected, ...)
+{
+va_list ap_count, ap_extract;
+QObject **args;
+int arg_count = 0;
+int i, j;
+
+va_start(ap_count, expected);
+va_copy(ap_extract, ap_count);
+while (va_arg(ap_count, QObject *) != &_test_equality_end_of_arguments) {
+arg_count++;
+}
+va_end(ap_count);
+
+args = g_new(QObject *, arg_count);
+for (i = 0; i < arg_count; i++) {
+args[i] = va_arg(ap_extract, QObject *);
+}
+va_end(ap_extract);
+
+for (i = 0; i < arg_count; i++) {
+g_assert(qobject_is_equal(args[i], args[i]) == true);
+
+for (j = i + 1; j < arg_count; j++) {
+g_assert(qobject_is_equal(args[i], args[j]) == expected);
+}
+}
+}
+
+#define test_equality(expected, ...) \
+do_test_equality(expected, __VA_ARGS__, &_test_equality_end_of_arguments)
+
+static void do_free_all(int _, ...)
+{
+va_list ap;
+QObject *obj;
+
+va_start(ap, _);
+while ((obj = va_arg(ap, QObject *)) != NULL) {
+qobject_decref(obj);
+}
+va_end(ap);
+}
+
+#define free_all(...) \
+do_free_all(0, __VA_ARGS__, NULL)
+
+static void qobject_is_equal_null_test(void)
+{
+test_equality(false, qnull(), NULL);
+}
+
+static void qobject_is_equal_num_test(void)
+{
+QNum *u0, *i0, *d0, *d0p25, *dnan, *um42, *im42, *dm42;
+QString *s0, *s_empty;
+QBool *bfalse;
+
+u0 = qnum_from_uint(0u);
+i0 = qnum_from_int(0);
+d0 = qnum_from_double(0.0);
+d0p25 = qnum_from_double(0.25);
+dnan = qnum_from_double(0.0 / 0.0);
+um42 = qnum_from_uint((uint64_t)-42);
+im42 = qnum_from_int(-42);
+dm42 = qnum_from_int(-42.0);
+
+s0 = qstring_from_str("0");
+s_empty = qstring_new();
+bfalse = qbool_
[Qemu-devel] [PATCH v3 2/5] qapi: Add qobject_is_equal()
This generic function (along with its implementations for different
types) determines whether two QObjects are equal.
Signed-off-by: Max Reitz
---
include/qapi/qmp/qbool.h | 1 +
include/qapi/qmp/qdict.h | 1 +
include/qapi/qmp/qlist.h | 1 +
include/qapi/qmp/qnull.h | 2 ++
include/qapi/qmp/qnum.h| 1 +
include/qapi/qmp/qobject.h | 9
include/qapi/qmp/qstring.h | 1 +
qobject/qbool.c| 8 +++
qobject/qdict.c| 29 +
qobject/qlist.c| 32
qobject/qnull.c| 9
qobject/qnum.c | 53 ++
qobject/qobject.c | 29 +
qobject/qstring.c | 9
14 files changed, 185 insertions(+)
diff --git a/include/qapi/qmp/qbool.h b/include/qapi/qmp/qbool.h
index a4c..f77ea86 100644
--- a/include/qapi/qmp/qbool.h
+++ b/include/qapi/qmp/qbool.h
@@ -24,6 +24,7 @@ typedef struct QBool {
QBool *qbool_from_bool(bool value);
bool qbool_get_bool(const QBool *qb);
QBool *qobject_to_qbool(const QObject *obj);
+bool qbool_is_equal(const QObject *x, const QObject *y);
void qbool_destroy_obj(QObject *obj);
#endif /* QBOOL_H */
diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 363e431..84f8ea7 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -42,6 +42,7 @@ void qdict_del(QDict *qdict, const char *key);
int qdict_haskey(const QDict *qdict, const char *key);
QObject *qdict_get(const QDict *qdict, const char *key);
QDict *qobject_to_qdict(const QObject *obj);
+bool qdict_is_equal(const QObject *x, const QObject *y);
void qdict_iter(const QDict *qdict,
void (*iter)(const char *key, QObject *obj, void *opaque),
void *opaque);
diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
index c4b5fda..24e1e9f 100644
--- a/include/qapi/qmp/qlist.h
+++ b/include/qapi/qmp/qlist.h
@@ -58,6 +58,7 @@ QObject *qlist_peek(QList *qlist);
int qlist_empty(const QList *qlist);
size_t qlist_size(const QList *qlist);
QList *qobject_to_qlist(const QObject *obj);
+bool qlist_is_equal(const QObject *x, const QObject *y);
void qlist_destroy_obj(QObject *obj);
static inline const QListEntry *qlist_first(const QList *qlist)
diff --git a/include/qapi/qmp/qnull.h b/include/qapi/qmp/qnull.h
index 48edad4..f4fbcae 100644
--- a/include/qapi/qmp/qnull.h
+++ b/include/qapi/qmp/qnull.h
@@ -23,4 +23,6 @@ static inline QObject *qnull(void)
return &qnull_;
}
+bool qnull_is_equal(const QObject *x, const QObject *y);
+
#endif /* QNULL_H */
diff --git a/include/qapi/qmp/qnum.h b/include/qapi/qmp/qnum.h
index 09d745c..237d01b 100644
--- a/include/qapi/qmp/qnum.h
+++ b/include/qapi/qmp/qnum.h
@@ -48,6 +48,7 @@ double qnum_get_double(QNum *qn);
char *qnum_to_string(QNum *qn);
QNum *qobject_to_qnum(const QObject *obj);
+bool qnum_is_equal(const QObject *x, const QObject *y);
void qnum_destroy_obj(QObject *obj);
#endif /* QNUM_H */
diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
index ef1d1a9..38ac688 100644
--- a/include/qapi/qmp/qobject.h
+++ b/include/qapi/qmp/qobject.h
@@ -68,6 +68,15 @@ static inline void qobject_incref(QObject *obj)
}
/**
+ * qobject_is_equal(): Return whether the two objects are equal.
+ *
+ * Any of the pointers may be NULL; return true if both are. Always
+ * return false if only one is (therefore a QNull object is not
+ * considered equal to a NULL pointer).
+ */
+bool qobject_is_equal(const QObject *x, const QObject *y);
+
+/**
* qobject_destroy(): Free resources used by the object
*/
void qobject_destroy(QObject *obj);
diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index 10076b7..65c05a9 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -31,6 +31,7 @@ void qstring_append_int(QString *qstring, int64_t value);
void qstring_append(QString *qstring, const char *str);
void qstring_append_chr(QString *qstring, int c);
QString *qobject_to_qstring(const QObject *obj);
+bool qstring_is_equal(const QObject *x, const QObject *y);
void qstring_destroy_obj(QObject *obj);
#endif /* QSTRING_H */
diff --git a/qobject/qbool.c b/qobject/qbool.c
index 0606bbd..ac825fc 100644
--- a/qobject/qbool.c
+++ b/qobject/qbool.c
@@ -52,6 +52,14 @@ QBool *qobject_to_qbool(const QObject *obj)
}
/**
+ * qbool_is_equal(): Test whether the two QBools are equal
+ */
+bool qbool_is_equal(const QObject *x, const QObject *y)
+{
+return qobject_to_qbool(x)->value == qobject_to_qbool(y)->value;
+}
+
+/**
* qbool_destroy_obj(): Free all memory allocated by a
* QBool object
*/
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 576018e..e8f15f1 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -403,6 +403,35 @@ void qdict_del(QDict *qdict, const char *key)
}
/**
+ * qdict_is_equal(): Test whether the two QDicts are equal
+ *
+ * He
[Qemu-devel] [PATCH v3 3/5] block: qobject_is_equal() in bdrv_reopen_prepare()
Currently, bdrv_reopen_prepare() assumes that all BDS options are
strings. However, this is not the case if the BDS has been created
through the json: pseudo-protocol or blockdev-add.
Note that the user-invokable reopen command is an HMP command, so you
can only specify strings there. Therefore, specifying a non-string
option with the "same" value as it was when originally created will now
return an error because the values are supposedly similar (and there is
no way for the user to circumvent this but to just not specify the
option again -- however, this is still strictly better than just
crashing).
Signed-off-by: Max Reitz
---
block.c | 31 ++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/block.c b/block.c
index 913bb43..45eb248 100644
--- a/block.c
+++ b/block.c
@@ -2947,19 +2947,24 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
BlockReopenQueue *queue,
const QDictEntry *entry = qdict_first(reopen_state->options);
do {
-QString *new_obj = qobject_to_qstring(entry->value);
-const char *new = qstring_get_str(new_obj);
-/*
- * Caution: while qdict_get_try_str() is fine, getting
- * non-string types would require more care. When
- * bs->options come from -blockdev or blockdev_add, its
- * members are typed according to the QAPI schema, but
- * when they come from -drive, they're all QString.
- */
-const char *old = qdict_get_try_str(reopen_state->bs->options,
-entry->key);
-
-if (!old || strcmp(new, old)) {
+QObject *new = entry->value;
+QObject *old = qdict_get(reopen_state->bs->options, entry->key);
+
+/* TODO: When using -drive to specify blockdev options, all values
+ * will be strings; however, when using -blockdev, blockdev-add or
+ * filenames using the json:{} pseudo-protocol, they will be
+ * correctly typed.
+ * In contrast, reopening options are (currently) always strings
+ * (because you can only specify them through qemu-io; all other
+ * callers do not specify any options).
+ * Therefore, when using anything other than -drive to create a
BDS,
+ * this cannot detect non-string options as unchanged, because
+ * qobject_is_equal() always returns false for objects of different
+ * type. In the future, this should be remedied by correctly
typing
+ * all options. For now, this is not too big of an issue because
+ * the user simply can not specify options which cannot be changed
+ * anyway, so they will stay unchanged. */
+if (!qobject_is_equal(new, old)) {
error_setg(errp, "Cannot change the option '%s'", entry->key);
ret = -EINVAL;
goto error;
--
2.9.4
Re: [Qemu-devel] [PATCH RFC 0/6] q35: add acpi pci hotplug support
On Fri, 30 Jun 2017 10:25:05 +0300 Marcel Apfelbaum wrote: [...] > > So for the modern systems not supporting PCI ACPI hotplug > we don't need pci-bridges anyway, but for the older ones > the ACPI code of the pci-bridge will be loaded into the > ACPI namespace only if a pci-bridge is actually hot-plugged. just note that the set of 'older' guest OSes is limited to one that do not support SHPC (i.e. to EOLed WinXP & co) as for linux and more modern Windows SHPC hotplug should just work without our ACPI hack (which taxes low memory to keep acpi tables for bridges). So I'm in favor of Michael's suggestion to leave ACPI PCI only in PC machine for old WinXP guests and to keep Q35 clean, where linux or newer Windows guests could just use standard SHPC. [...]
[Qemu-devel] [PATCH v3 4/5] iotests: Add test for non-string option reopening
Reviewed-by: Kevin Wolf
Signed-off-by: Max Reitz
---
tests/qemu-iotests/133 | 9 +
tests/qemu-iotests/133.out | 5 +
2 files changed, 14 insertions(+)
diff --git a/tests/qemu-iotests/133 b/tests/qemu-iotests/133
index 9d35a6a..af6b3e1 100755
--- a/tests/qemu-iotests/133
+++ b/tests/qemu-iotests/133
@@ -83,6 +83,15 @@ $QEMU_IO -c 'reopen -o driver=qcow2' $TEST_IMG
$QEMU_IO -c 'reopen -o file.driver=file' $TEST_IMG
$QEMU_IO -c 'reopen -o backing.driver=qcow2' $TEST_IMG
+echo
+echo "=== Check that reopening works with non-string options ==="
+echo
+
+# Using the json: pseudo-protocol we can create non-string options
+# (Invoke 'info' just so we get some output afterwards)
+IMGOPTSSYNTAX=false $QEMU_IO -f null-co -c 'reopen' -c 'info' \
+"json:{'driver': 'null-co', 'size': 65536}"
+
# success, all done
echo "*** done"
rm -f $seq.full
diff --git a/tests/qemu-iotests/133.out b/tests/qemu-iotests/133.out
index cc86b94..f4a85ae 100644
--- a/tests/qemu-iotests/133.out
+++ b/tests/qemu-iotests/133.out
@@ -19,4 +19,9 @@ Cannot change the option 'driver'
=== Check that unchanged driver is okay ===
+
+=== Check that reopening works with non-string options ===
+
+format name: null-co
+format name: null-co
*** done
--
2.9.4
Re: [Qemu-devel] [PATCH v2 02/15] vl: add tcg_enabled() for tcg related code
On 03.07.2017 12:12, Yang Zhong wrote:
> Need to disable the tcg related code in the vl.c if the
> disable-tcg option is added into ./configure command.
>
> Signed-off-by: Yang Zhong
> ---
> accel/tcg/tcg-all.c| 2 +-
> include/sysemu/accel.h | 2 +-
> vl.c | 15 +++
> 3 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
> index dba9931..b86c896 100644
> --- a/accel/tcg/tcg-all.c
> +++ b/accel/tcg/tcg-all.c
> @@ -28,7 +28,7 @@
> #include "sysemu/sysemu.h"
> #include "qom/object.h"
>
> -int tcg_tb_size;
> +long tcg_tb_size;
This looks like a non-related change. I think you should either put it
into a separate patch, or drop it.
> static bool tcg_allowed = true;
>
> static int tcg_init(MachineState *ms)
> diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h
> index ecc5c84..7b905b7 100644
> --- a/include/sysemu/accel.h
> +++ b/include/sysemu/accel.h
> @@ -63,7 +63,7 @@ typedef struct AccelClass {
> #define ACCEL_GET_CLASS(obj) \
> OBJECT_GET_CLASS(AccelClass, (obj), TYPE_ACCEL)
>
> -extern int tcg_tb_size;
> +extern long tcg_tb_size;
>
> void configure_accelerator(MachineState *ms);
> /* Register accelerator specific global properties */
> diff --git a/vl.c b/vl.c
> index 36ff3f4..f28c1ac 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3933,9 +3933,14 @@ int main(int argc, char **argv, char **envp)
> configure_rtc(opts);
> break;
> case QEMU_OPTION_tb_size:
> -tcg_tb_size = strtol(optarg, NULL, 0);
> -if (tcg_tb_size < 0) {
> -tcg_tb_size = 0;
> +if (tcg_enabled()) {
> +qemu_strtol(optarg, NULL, 0, &tcg_tb_size);
> +if (tcg_tb_size < 0) {
> +tcg_tb_size = 0;
> +}
> +} else {
> +error_report("TCG is disabled");
> +exit(1);
> }
You could do this without an else-part, leaving the tcg_tb_size code
unmodified:
+ if (!tcg_enabled()) {
+ error_report("TCG is disabled");
+ exit(1);
+ }
tcg_tb_size = strtol(optarg, NULL, 0);
if (tcg_tb_size < 0) {
tcg_tb_size = 0;
> break;
> case QEMU_OPTION_icount:
> @@ -4481,7 +4486,9 @@ int main(int argc, char **argv, char **envp)
> qemu_opts_del(icount_opts);
> }
>
> -qemu_tcg_configure(accel_opts, &error_fatal);
> +if (tcg_enabled()) {
> +qemu_tcg_configure(accel_opts, &error_fatal);
> +}
>
> if (default_net) {
> QemuOptsList *net = qemu_find_opts("net");
>
Thomas
[Qemu-devel] [PATCH v3] tests: Avoid non-portable 'echo -ARG'
POSIX says that backslashes in the arguments to 'echo', as well as
any use of 'echo -n' and 'echo -e', are non-portable; it recommends
people should favor 'printf' instead. This is definitely true where
we do not control which shell is running (such as in makefile snippets
or in documentation examples). But even for scripts where we
require bash (and therefore, where echo does what we want by default),
it is still possible to use 'shopt -s xpg_echo' to change bash's
behavior of echo. And setting a good example never hurts when we are
not sure if a snippet will be copied from a bash-only script to a
general shell script (although I don't change the use of non-portable
\e for ESC when we know the running shell is bash).
Replace 'echo -n "..."' with 'printf %s "..."', and 'echo -e "..."'
with 'printf %b "...\n"', with the optimization that the %s/%b
argument can be omitted if the string being printed contains no
substitutions that could result in '%' (trivial if there are no
$ or `, but also possible when using things like $ret that are
known to be numeric given the assignment to ret just above).
In the qemu-iotests check script, fix unusual shell quoting
that would result in word-splitting if 'date' outputs a space.
In test 051, take an opportunity to shorten the line.
In test 068, get rid of a pointless second invocation of bash.
CC: qemu-triv...@nongnu.org
Signed-off-by: Eric Blake
---
v3: use 'printf %s' in a few more places that substitute [Max]
v2: be robust to potential % in substitutions
---
qemu-options.hx | 4 ++--
tests/multiboot/run_test.sh | 10 +-
tests/qemu-iotests/051 | 7 ---
tests/qemu-iotests/068 | 2 +-
tests/qemu-iotests/142 | 48 ++---
tests/qemu-iotests/171 | 14 ++---
tests/qemu-iotests/check| 18 -
tests/rocker/all| 10 +-
tests/tcg/cris/Makefile | 8
9 files changed, 61 insertions(+), 60 deletions(-)
diff --git a/qemu-options.hx b/qemu-options.hx
index 297bd8a..05a0474 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4363,7 +4363,7 @@ The simplest (insecure) usage is to provide the secret
inline
The simplest secure usage is to provide the secret via a file
- # echo -n "letmein" > mypasswd.txt
+ # printf "letmein" > mypasswd.txt
# $QEMU -object secret,id=sec0,file=mypasswd.txt,format=raw
For greater security, AES-256-CBC should be used. To illustrate usage,
@@ -4391,7 +4391,7 @@ telling openssl to base64 encode the result, but it could
be left
as raw bytes if desired.
@example
- # SECRET=$(echo -n "letmein" |
+ # SECRET=$(printf "letmein" |
openssl enc -aes-256-cbc -a -K $KEY -iv $IV)
@end example
diff --git a/tests/multiboot/run_test.sh b/tests/multiboot/run_test.sh
index 78d7edf..9a1cd59 100755
--- a/tests/multiboot/run_test.sh
+++ b/tests/multiboot/run_test.sh
@@ -26,7 +26,7 @@ run_qemu() {
local kernel=$1
shift
-echo -e "\n\n=== Running test case: $kernel $@ ===\n" >> test.log
+printf %b "\n\n=== Running test case: $kernel $@ ===\n\n" >> test.log
$QEMU \
-kernel $kernel \
@@ -68,21 +68,21 @@ for t in mmap modules; do
pass=1
if [ $debugexit != 1 ]; then
-echo -e "\e[31m ?? \e[0m $t (no debugexit used, exit code $ret)"
+printf "\e[31m ?? \e[0m $t (no debugexit used, exit code $ret)\n"
pass=0
elif [ $ret != 0 ]; then
-echo -e "\e[31mFAIL\e[0m $t (exit code $ret)"
+printf "\e[31mFAIL\e[0m $t (exit code $ret)\n"
pass=0
fi
if ! diff $t.out test.log > /dev/null 2>&1; then
-echo -e "\e[31mFAIL\e[0m $t (output difference)"
+printf "\e[31mFAIL\e[0m $t (output difference)\n"
diff -u $t.out test.log
pass=0
fi
if [ $pass == 1 ]; then
-echo -e "\e[32mPASS\e[0m $t"
+printf "\e[32mPASS\e[0m $t\n"
fi
done
diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index 26c29de..c3c08bf 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -217,7 +217,7 @@ run_qemu -drive driver=null-co,cache=invalid_value
# Test 142 checks the direct=on cases
for cache in writeback writethrough unsafe invalid_value; do
-echo -e "info block\ninfo block file\ninfo block backing\ninfo block
backing-file" | \
+printf "info block %s\n" '' file backing backing-file | \
run_qemu -drive
file="$TEST_IMG",cache=$cache,backing.file.filename="$TEST_IMG.base",backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=$device_id
-nodefaults
done
@@ -325,8 +325,9 @@ echo "qemu-io $device_id \"write -P 0x22 0 4k\"" | run_qemu
-drive file="$TEST_I
$QEMU_IO -c "read -P 0x22 0 4k" "$TEST_IMG" | _filter_qemu_io
-echo -e "qemu-io $device_id \"write -P 0x33 0 4k\"\ncommit $device_id" |
run_qemu -drive file="$TEST_IMG",snapshot=on,if=none,id=$device_id\
-
Re: [Qemu-devel] [PATCH v5 0/2] iotests: Add test for colon handling
On 2017-07-02 17:05, Max Reitz wrote: > v3 of v3 of "iotests: Add test for colon handling"; thus, basically, v5. > > This adds an iotest for the original series "block: Fix backing paths > for filenames with colons" and fixes common.config so it works if you > have specified the qemu binaries through relative paths. As a bonus, it > makes symlinked binaries work for out-of-tree builds. > > > v5: Don't try to resolve QEMU_VXHS_PROG if it is empty (which it may be) > [Eric] > > > git-backport-diff against v4: > > Key: > [] : patches are identical > [] : number of functional differences between upstream/downstream patch > [down] : patch is downstream-only > The flags [FC] indicate (F)unctional and (C)ontextual differences, > respectively > > 001/2:[0007] [FC] 'iotests: Use absolute paths for executables' > 002/2:[] [--] 'iotests: Add test for colon handling' > > > Max Reitz (2): > iotests: Use absolute paths for executables > iotests: Add test for colon handling > > tests/qemu-iotests/126 | 105 > +++ > tests/qemu-iotests/126.out | 23 + > tests/qemu-iotests/common.config | 11 > tests/qemu-iotests/group | 1 + > 4 files changed, 140 insertions(+) > create mode 100755 tests/qemu-iotests/126 > create mode 100644 tests/qemu-iotests/126.out Applied to my block tree: https://github.com/XanClic/qemu/commits/block (Let's hope it stays there this time O:-)) Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v3 1/5] qapi/qnull: Add own header
On 07/03/2017 07:25 AM, Max Reitz wrote: > Reviewed-by: Markus Armbruster > Signed-off-by: Max Reitz > --- > include/qapi/qmp/qnull.h | 26 ++ > include/qapi/qmp/qobject.h | 8 > include/qapi/qmp/types.h | 1 + > qobject/qnull.c| 2 +- > tests/check-qnull.c| 2 +- > 5 files changed, 29 insertions(+), 10 deletions(-) > create mode 100644 include/qapi/qmp/qnull.h > > diff --git a/include/qapi/qmp/qnull.h b/include/qapi/qmp/qnull.h > new file mode 100644 > index 000..48edad4 > --- /dev/null > +++ b/include/qapi/qmp/qnull.h > @@ -0,0 +1,26 @@ > +/* > + * QNull > + * > + * Copyright (C) 2015 Red Hat, Inc. The date makes sense based on when the bulk of the content for this file was first introduced elsewhere, but I'm okay if you also want to add 2017. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v2 03/15] tcg: tcg_handle_interrupt() function
On 03.07.2017 12:12, Yang Zhong wrote: > Move tcg_handle_interrupt() from translate-common.c to > accel/tcg/tcg-all.c. Why is this necessary / wanted? Could you please mention the reason in the patch description? Thanks, Thomas
Re: [Qemu-devel] [PATCH v3 2/5] qapi: Add qobject_is_equal()
On 07/03/2017 07:25 AM, Max Reitz wrote:
> This generic function (along with its implementations for different
> types) determines whether two QObjects are equal.
>
> Signed-off-by: Max Reitz
> ---
> +++ b/qobject/qnum.c
> +case QNUM_DOUBLE:
> +switch (num_y->kind) {
> +case QNUM_I64:
> +return qnum_is_equal(y, x);
> +case QNUM_U64:
> +return qnum_is_equal(y, x);
> +case QNUM_DOUBLE:
> +/* Comparison in native double type */
> +return num_x->u.dbl == num_y->u.dbl;
Which returns false if we allow NaN (in general, JSON does not, so I'm
not sure if we allow NaN in a QNum)
> +++ b/qobject/qobject.c
> +bool qobject_is_equal(const QObject *x, const QObject *y)
> +{
> +/* We cannot test x == y because an object does not need to be
> + * equal to itself (e.g. NaN floats are not). */
But this comment matches the code above in QNum.
> +
> +if (!x && !y) {
> +return true;
> +}
> +
> +if (!x || !y || x->type != y->type) {
> +return false;
> +}
I like that there is no implicit conversion between bool and int; the
only conversions that still allow equality are within the subtypes of QNum.
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v3 3/5] block: qobject_is_equal() in bdrv_reopen_prepare()
On 07/03/2017 07:25 AM, Max Reitz wrote:
> Currently, bdrv_reopen_prepare() assumes that all BDS options are
> strings. However, this is not the case if the BDS has been created
> through the json: pseudo-protocol or blockdev-add.
>
> Note that the user-invokable reopen command is an HMP command, so you
s/invokable/invocable/
> can only specify strings there. Therefore, specifying a non-string
> option with the "same" value as it was when originally created will now
> return an error because the values are supposedly similar (and there is
> no way for the user to circumvent this but to just not specify the
> option again -- however, this is still strictly better than just
> crashing).
>
> Signed-off-by: Max Reitz
> ---
> block.c | 31 ++-
> 1 file changed, 18 insertions(+), 13 deletions(-)
>
> +/* TODO: When using -drive to specify blockdev options, all
> values
> + * will be strings; however, when using -blockdev, blockdev-add
> or
> + * filenames using the json:{} pseudo-protocol, they will be
> + * correctly typed.
> + * In contrast, reopening options are (currently) always strings
> + * (because you can only specify them through qemu-io; all other
> + * callers do not specify any options).
> + * Therefore, when using anything other than -drive to create a
> BDS,
> + * this cannot detect non-string options as unchanged, because
> + * qobject_is_equal() always returns false for objects of
> different
> + * type. In the future, this should be remedied by correctly
> typing
> + * all options. For now, this is not too big of an issue because
> + * the user simply can not specify options which cannot be
> changed
Seeing "can not" usually looks wrong for "cannot"; but here, it is
grammatically correct. But better would be "the user can simply not
specify" or "the user can simply avoid specifying options".
> + * anyway, so they will stay unchanged. */
The grammar tweak is minor, so:
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [RFC 00/15] Error API: Flag errors in *errp even if errors are being ignored
Eduardo Habkost writes: > On Fri, Jun 30, 2017 at 01:40:58PM +0200, Markus Armbruster wrote: >> Eduardo Habkost writes: >> >> > On Thu, Jun 29, 2017 at 08:54:29AM +0200, Markus Armbruster wrote: >> >> Eduardo Habkost writes: >> >> >> >> > On Wed, Jun 28, 2017 at 11:05:26AM +0200, Markus Armbruster wrote: >> >> >> Eduardo Habkost writes: >> > [...] >> >> >> > I understand the reason we need to support errp==NULL, as it >> >> >> > makes life simpler for callers that don't want any extra error >> >> >> > information. However, this has the cost of making the functions >> >> >> > that report errors more complex and error-prone. >> >> >> > >> >> >> > (Evidence of that: the 34 ERR_IS_* cases handled by the "use >> >> >> > ERR_IS_* macros" patches in the series. Where existing code will >> >> >> > crash or behave differently if errp is NULL.) >> >> >> >> >> >> Which of them could *not* use a suitable return value instead of *errp? >> >> > >> >> > I don't know. But I'm not trying to improve those 34 ERR_IS_* cases. I >> >> > am trying to improve the 700+ functions that need the >> >> > local_err/error_propagate() boilerplate code today. This series already >> >> > handles 346 of them automatically (see patch 14/15). >> >> >> >> I agree the goal is reducing error_propagate() boilerplate. I latched >> >> onto the 34 ERR_IS_* cases only because you presented them as examples. >> > >> > The 34 ERR_IS_* cases were evidence of how easy it is to introduce >> > mistakes with the current API. Probably most of them are instances of >> > (1) and (2) below. >> >> The current interface can be abused, but how much abuse actually creeps >> in? I think we've been doing reasonably well there since we got rid of >> the bad examples and improved documentation. > > See the 30+ cases touched by patch 09/15. Except for the ones in > error.c, all of them look like bugs to me. > > I didn't investigate when each of them were introduced, though. > >> >> Moreover, the revised interface could also be abused. Nothing stops you >> from dereferencing errp before or after, the only thing that changes are >> the examples people see in code. I'm afraid the people who reinvent bad >> examples from scratch despite the documentation telling them not to will >> also bypass any macros the documentation tells them to use. >> >> *Especially* if we use macros only sometimes. ERR_IS_SET(&err) makes no >> sense, so we'd still test err directly there, wouldn't we? > > Any interface can be abused. But I still believe a simpler and easier > interface for propagating errors is less likely to be abused. Can you substantiate this claim with examples? I'm looking for arguments like "if existing code looks like $this, people could plausibly write $mistake, which is wrong. If it looks like $that, they could still write $related_mistake, but it seems far less likely." > But in either case, tools to detect abuse would be welcome. We can > write Coccinelle scripts to detect most abuse of the existing error API. checkpatch.pl would be nice, too. It's too stupid for rigorous checks. However, the ones you fix in PATCH 09 all involve *errp. Many of the *errp your patch doesn't touch look fishy to me. Should checkpatch.pl warn when it sees *errp in new code? > [...] >> >> > is fixed because ERR_IS_SET(errp) will work even if errp is NULL. >> >> > >> >> >> > TODO >> >> >> > >> >> >> > >> >> >> > * Simplify more cases of local_error/error_propagate() to use >> >> >> > errp directly. >> >> >> > * Update API documentation and code examples. >> >> >> > * Add a mechanism to ensure errp is never NULL. >> >> >> > >> >> >> > Git branch >> >> >> > -- >> >> >> > >> >> >> > This series depend on a few extra cleanups that I didn't submit >> >> >> > to qemu-devel yet. A git branch including this series is >> >> >> > available at: >> >> >> > >> >> >> > git://github.com/ehabkost/qemu-hacks.git >> >> >> > work/err-api-rework-ignore-ptr-v1 >> >> I doubt the macros make the bug fixing materially easier, and I doubt >> they can reduce future bugs of this kind. What they can do is letting >> us get rid of error_propagate() boilerplate with relative ease. >> >> If we switch to returning success/failure (which also gets rid of the >> boilerplate), then the macros may still let us get rid of boilerplate >> more quickly, for some additional churn. Worthwhile? Depends on how >> long the return value change takes us. > > My assumption is that it will take a very long time. "Very long time" would be bad, because unconverted code breeds more unconverted code by serving as bad example. Big "convert from old way to do things to new way" tasks should only be started with a reasonable idea on how to end them. We've started too many such tasks sanguinely, and have had the darndest time ending any of them. I'm investigating ways to automate parts of the job. >> I think the first order of business is to figure out whether we want to >> pursue retu
Re: [Qemu-devel] [PATCH v2 01/15] configure: add the disable-tcg option
On 03/07/2017 14:09, Daniel P. Berrange wrote: >> It would leave you with a tools-only build; whether that's a good idea, >> it's another story. I think it's acceptable, but others may disagree. > If you do 'configure --target-list=x86_64-softmmu --disable-tcg' on a > non-x86 host I would be surprised to find x86_64-softmmu not built. > IMHO if you explicitly request system emulators and its not posisble > to build then, an error should be report, rather than silently ignoring > the request. Technically you could build a qtest-only emulator, but I don't think _that_ is a good idea (and it's not what the patch does). Paolo
Re: [Qemu-devel] [PATCH v3] tests: Avoid non-portable 'echo -ARG'
On 2017-07-03 14:31, Eric Blake wrote: > POSIX says that backslashes in the arguments to 'echo', as well as > any use of 'echo -n' and 'echo -e', are non-portable; it recommends > people should favor 'printf' instead. This is definitely true where > we do not control which shell is running (such as in makefile snippets > or in documentation examples). But even for scripts where we > require bash (and therefore, where echo does what we want by default), > it is still possible to use 'shopt -s xpg_echo' to change bash's > behavior of echo. And setting a good example never hurts when we are > not sure if a snippet will be copied from a bash-only script to a > general shell script (although I don't change the use of non-portable > \e for ESC when we know the running shell is bash). > > Replace 'echo -n "..."' with 'printf %s "..."', and 'echo -e "..."' > with 'printf %b "...\n"', with the optimization that the %s/%b > argument can be omitted if the string being printed contains no > substitutions that could result in '%' (trivial if there are no > $ or `, but also possible when using things like $ret that are > known to be numeric given the assignment to ret just above). > > In the qemu-iotests check script, fix unusual shell quoting > that would result in word-splitting if 'date' outputs a space. > > In test 051, take an opportunity to shorten the line. > > In test 068, get rid of a pointless second invocation of bash. > > CC: qemu-triv...@nongnu.org > Signed-off-by: Eric Blake > > --- > v3: use 'printf %s' in a few more places that substitute [Max] > v2: be robust to potential % in substitutions > --- > qemu-options.hx | 4 ++-- > tests/multiboot/run_test.sh | 10 +- > tests/qemu-iotests/051 | 7 --- > tests/qemu-iotests/068 | 2 +- > tests/qemu-iotests/142 | 48 > ++--- > tests/qemu-iotests/171 | 14 ++--- > tests/qemu-iotests/check| 18 - > tests/rocker/all| 10 +- > tests/tcg/cris/Makefile | 8 > 9 files changed, 61 insertions(+), 60 deletions(-) [...] > diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check > index 4b1c674..30af0eb 100755 > --- a/tests/qemu-iotests/check > +++ b/tests/qemu-iotests/check [...] > @@ -314,21 +314,21 @@ do > > if [ -f core ] > then > -echo -n " [dumped core]" > +printf " [dumped core]" > mv core $seq.core > err=true > fi > > if [ -f $seq.notrun ] > then > -$timestamp || echo -n " [not run] " > -$timestamp && echo " [not run]" && echo -n "$seq -- " > +$timestamp || printf " [not run] " > +$timestamp && echo " [not run]" && printf "$seq -- " Everywhere else, $seq got its own %s, but not here. That's at least inconsistent. I don't quite know why you're not simply using %s and %b everywhere there is a non-constant format string. Yes, if it is an exit code, it is pretty clear that it won't contain a %. If it's a return value from "date +%T", it is kind of, too (but then you have to know what that does in order to review the change -- and I myself had to consult the man page, to be honest). If they are variables named "img_offset" and "img_size", then you'd hope they are numbers, but in order to check you still have to look at the rest of the code (especially since they are embedded as strings into the JSON object). If you simply used %s and %b everywhere there is a $, reviewing would be absolutely trivial. Yes, it seems a bit unnecessary, but it does make reviewing much easier and I think since this is also about setting a good example, that would be a good example, IMHO. (Sure, reviewing this already is easy enough, but there still is a difference between "very easy" and "trivial".) Although at this point I have to admit that reviewing a really trivial v4 would take me more time than to just write the following: With a %s added to the printf above: Reviewed-by: Max Reitz > cat $seq.notrun > notrun="$notrun $seq" > else > if [ $sts -ne 0 ] > then > -echo -n " [failed, exit status $sts]" > +printf " [failed, exit status $sts]" > err=true > fi > signature.asc Description: OpenPGP digital signature
