Re: [Qemu-devel] [PATCH 4/6] ide: Update ide_drive_get to be HBA agnostic
John Snow writes: > On the subject of compatibly broken: > Is there any circumstance where the mapping has *any* effect on the > current working behavior? Since we do not support the shorthand syntax > at all currently, there is no code that USES this mapping to do > anything. > > Even if you specify -drive file=...,index=42; the -device that you > currently *must* add simply ignores any index/bus/unit mappings you've > already given the drive -- it doesn't seem to touch or use any > information within the DriveInfo structure at all. I didn't think this through, and I knew it, so I chose to err on the side of excessive caution by posing the bug-compatibility question. You're right: since -drive if=ide was ignored before, we can change the mapping. > That said, why not make this property retroactive? It won't affect anything. > > Or rather, the property alone won't. What *will* break older command > lines is the fact that we are now supporting the syntactic sugar at > all, but I think this is clearly a bug, and we should not go out of > our way to tolerate broken syntax. Agreed. > Based on the above, I think I will: > > (A) Implement the property for all versions Yes, please. > (B) Change the behavior of ide_drive_get to take the number of > elements in the array, and imply the number of buses instead. If we > decide to roll back older versions to be "compatibly broken," this > will in effect limit us to three drives instead of six (index 0, 2, > and 4) being mapped to ports 0, 1, and 2. > > This requires me touching the board init to adjust the call to > drive_get slightly. Thanks!
[Qemu-devel] [Bug 1373362] Re: qemu-2.1.1 i386-softmmu compile error: q35_dsdt_applesmc_sta undeclared
update: I found out that the latest version on the stable-1.7 branch builds fine, but all stable-2.0 and later fail. I did some binary search on all versions on the stable-2.0 branch and found out that the problem occurs after this commit: http://git.qemu.org/?p=qemu.git;a=commitdiff;h=15bce1b7c55c69f47e13c9eb2a4b80f41da26581 Unfortunately this is not so much surprising, as this is the first commit that introduced a reference to that symbol. So the question is: where is that symbol defined in *your* version? Just to track this down: could you please do a "grep" in your source to see where it is defined on your side? (grep -r q35_dsdt_applesmc_sta ../) I expect there is some slight difference to the hits that I get here (see my initial post) My guess is that it is defined in some "generated" file and that there is a problem with the generator/converter that produces it. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1373362 Title: qemu-2.1.1 i386-softmmu compile error: q35_dsdt_applesmc_sta undeclared Status in QEMU: New Status in “qemu” package in Gentoo Linux: New Bug description: I try to compile qemu-2.1.1 (Gentoo/x86), but the i386-softmmu fails to compile: CPP i386-softmmu/q35-acpi-dsdt.dsl.i.orig ACPI_PREPROCESS i386-softmmu/q35-acpi-dsdt.dsl.i IASL i386-softmmu/q35-acpi-dsdt.dsl.i ACPI_EXTRACT i386-softmmu/q35-acpi-dsdt.off CAT i386-softmmu/hw/i386/q35-acpi-dsdt.hex CCi386-softmmu/hw/i386/acpi-build.o /tmp/portage/app-emulation/qemu-2.1.1/work/qemu-2.1.1/hw/i386/acpi-build.c: In function 'acpi_get_dsdt': /tmp/portage/app-emulation/qemu-2.1.1/work/qemu-2.1.1/hw/i386/acpi-build.c:119:24: error: 'q35_dsdt_applesmc_sta' undeclared (first use in this function) applesmc_sta = q35_dsdt_applesmc_sta; ^ /tmp/portage/app-emulation/qemu-2.1.1/work/qemu-2.1.1/hw/i386/acpi-build.c:119:24: note: each undeclared identifier is reported only once for each function it appears in make[1]: *** [hw/i386/acpi-build.o] Error 1 make: *** [subdir-i386-softmmu] Error 2 Something seems to go wrong when generating the file i386-softmmu/hw/i386/q35-acpi-dsdt.hex: # grep -r q35_dsdt_applesmc_sta ../ ../softmmu-build/x86_64-softmmu/q35-acpi-dsdt.dsl.i:/* ACPI_EXTRACT_NAME_BYTE_CONST q35_dsdt_applesmc_sta */ ../softmmu-build/x86_64-softmmu/q35-acpi-dsdt.dsl.i.orig: ACPI_EXTRACT_NAME_BYTE_CONST q35_dsdt_applesmc_sta ../softmmu-build/i386-softmmu/q35-acpi-dsdt.dsl.i:/* ACPI_EXTRACT_NAME_BYTE_CONST q35_dsdt_applesmc_sta */ ../softmmu-build/i386-softmmu/q35-acpi-dsdt.dsl.i.orig: ACPI_EXTRACT_NAME_BYTE_CONST q35_dsdt_applesmc_sta ../hw/i386/acpi-build.c:applesmc_sta = q35_dsdt_applesmc_sta; ../hw/i386/q35-acpi-dsdt.dsl:#define DSDT_APPLESMC_STA q35_dsdt_applesmc_sta To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1373362/+subscriptions
Re: [Qemu-devel] [PATCH RESEND 0/9] virtio: fix virtio child recount in transports
> > > virtio-net: use aliases instead of duplicate qdev properties > > > virtio: fix virtio-net child refcount in transports > > > virtio/vhost scsi: use aliases instead of duplicate qdev properties > > > virtio/vhost-scsi: fix virtio-scsi/vhost-scsi child refcount in > > > transports > > > virtio-serial: use aliases instead of duplicate qdev properties > > > virtio-serial: fix virtio-serial child refcount in transports > > > virtio-rng: use aliases instead of duplicate qdev properties > > > virtio-rng: fix virtio-rng child refcount in transports > > > virtio-balloon: fix virtio-balloon child refcount in transports > > > > > > hw/s390x/s390-virtio-bus.c | 16 ++-- > > > hw/s390x/virtio-ccw.c | 18 +++--- > > > hw/virtio/virtio-pci.c | 18 +++--- > > > 3 files changed, 32 insertions(+), 20 deletions(-) > > > > One thing I noticed is that the various devices end up with similar > > code in the end: > > > > object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_WHATEVER); > > object_property_add_child(obj, "virtio-backend", > OBJECT(&dev->vdev), > > NULL); > > object_unref(OBJECT(&dev->vdev)); > > qdev_alias_all_properties(DEVICE(&dev->vdev), obj); > > > > Would it make sense to add a helper function for that? > Sorry, I'm afraid this is not helpful. Because dev and dev->vdev is different for different virtio devices, like VirtIOBlkPCI(and its vdev is VirtIOBlock), VirtIONetPCI(and its vdev is VirtIONet). They have no the same parameters for above code segment. :) Best regards, -Gonglei
Re: [Qemu-devel] [Bug 1373362] Re: qemu-2.1.1 i386-softmmu compile error: q35_dsdt_applesmc_sta undeclared
On 27 September 2014 10:28, Thomas Eschenbacher wrote: > update: I found out that the latest version on the stable-1.7 branch builds > fine, but all stable-2.0 and later fail. > I did some binary search on all versions on the stable-2.0 branch and found > out that the problem occurs after this commit: > > http://git.qemu.org/?p=qemu.git;a=commitdiff;h=15bce1b7c55c69f47e13c9eb2a4b80f41da26581 Do you also see this compile failure on git master, or is it only a problem on the stable branch? > Unfortunately this is not so much surprising, as this is the first > commit that introduced a reference to that symbol. So the question is: > where is that symbol defined in *your* version? > > Just to track this down: could you please do a "grep" in your source to see > where it is defined on your side? > (grep -r q35_dsdt_applesmc_sta ../) > I expect there is some slight difference to the hits that I get here (see my > initial post) > > My guess is that it is defined in some "generated" file and that there > is a problem with the generator/converter that produces it. Note that there are two different possible paths here: * if configure finds you have iasl it will build the q35-acpi-dsdt.hex files from the dsl source files * otherwise, we just copy hw/i386/q35-acpi-dsdt.hex.generated into the build tree as the .hex file (at least, this is how master is doing it.) Some things that might be useful for debugging: * if you add "V=1" to your make command line it will print the full commands being run rather than the pretty "IASL/CPP/CC" short progress info * you could look at whether the .hex file you've generated matches the .hex.generated file we ship at all My guess is that your system has a busted iasl of some kind, in which case you can work around this by passing '--iasl=none' to configure, but it would be good to identify what's actually going wrong here. thanks -- PMM
[Qemu-devel] [Bug 1373362] Re: qemu-2.1.1 i386-softmmu compile error: q35_dsdt_applesmc_sta undeclared
Hi Peter,
thanks for the hints! Indeed there was an outdated version of iasl on my
system, which I have manually installed in /usr/local/bin many years
ago... (on both machines, on the gentoo as well as the kubuntu box) -
sorry for that!!!
That produced an error message during build (see below), but for some
reason the make system did not recognize that and continued with broken
files (not only here, all other files were affected too).
iasl -vs -l -tc -p q35-acpi-dsdt q35-acpi-dsdt.dsl.i 2>&1
q35-acpi-dsdt.dsl.i48: If (LEqual(Arg0,
ToUUID("33DB4D5B-1FF7-401C-9657-7441C03DD766"))) {
Error1037 - ^ syntax
error, unexpected PARSEOP_NAMESEG, expecting ')'
q35-acpi-dsdt.dsl.i61: } Else {
Error1037 - ^ syntax error, unexpected
PARSEOP_ELSE
ASL Input: q35-acpi-dsdt.dsl.i - 308 lines, 22112 bytes, 47 keywords
Compilation complete. 2 Errors, 0 Warnings, 0 Remarks, 5 Optimizations
The bad old version of iasl that produced the trouble was:
---
Intel ACPI Component Architecture
ASL Optimizing Compiler / AML Disassembler version 20040715 [Oct 11 2004]
Copyright (C) 2000 - 2004 Intel Corporation
Supports ACPI Specification Revision 2.0c
---
Now I checked with iasl-20130117-32 (Gentoo) and iasl-20140214-64
(Kubuntu), both worked fine and both understand the command line
parameter "-v" to get their version (which is not the case for the old
one). Maybe it is worth writing some version check in the configure
script ?
IMO we can close this bug.
thanks a lot for your help,
Thomas
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1373362
Title:
qemu-2.1.1 i386-softmmu compile error: q35_dsdt_applesmc_sta
undeclared
Status in QEMU:
New
Status in “qemu” package in Gentoo Linux:
New
Bug description:
I try to compile qemu-2.1.1 (Gentoo/x86), but the i386-softmmu fails
to compile:
CPP i386-softmmu/q35-acpi-dsdt.dsl.i.orig
ACPI_PREPROCESS i386-softmmu/q35-acpi-dsdt.dsl.i
IASL i386-softmmu/q35-acpi-dsdt.dsl.i
ACPI_EXTRACT i386-softmmu/q35-acpi-dsdt.off
CAT i386-softmmu/hw/i386/q35-acpi-dsdt.hex
CCi386-softmmu/hw/i386/acpi-build.o
/tmp/portage/app-emulation/qemu-2.1.1/work/qemu-2.1.1/hw/i386/acpi-build.c:
In function 'acpi_get_dsdt':
/tmp/portage/app-emulation/qemu-2.1.1/work/qemu-2.1.1/hw/i386/acpi-build.c:119:24:
error: 'q35_dsdt_applesmc_sta' undeclared (first use in this function)
applesmc_sta = q35_dsdt_applesmc_sta;
^
/tmp/portage/app-emulation/qemu-2.1.1/work/qemu-2.1.1/hw/i386/acpi-build.c:119:24:
note: each undeclared identifier is reported only once for each function it
appears in
make[1]: *** [hw/i386/acpi-build.o] Error 1
make: *** [subdir-i386-softmmu] Error 2
Something seems to go wrong when generating the file
i386-softmmu/hw/i386/q35-acpi-dsdt.hex:
# grep -r q35_dsdt_applesmc_sta ../
../softmmu-build/x86_64-softmmu/q35-acpi-dsdt.dsl.i:/*
ACPI_EXTRACT_NAME_BYTE_CONST q35_dsdt_applesmc_sta */
../softmmu-build/x86_64-softmmu/q35-acpi-dsdt.dsl.i.orig:
ACPI_EXTRACT_NAME_BYTE_CONST q35_dsdt_applesmc_sta
../softmmu-build/i386-softmmu/q35-acpi-dsdt.dsl.i:/*
ACPI_EXTRACT_NAME_BYTE_CONST q35_dsdt_applesmc_sta */
../softmmu-build/i386-softmmu/q35-acpi-dsdt.dsl.i.orig:
ACPI_EXTRACT_NAME_BYTE_CONST q35_dsdt_applesmc_sta
../hw/i386/acpi-build.c:applesmc_sta = q35_dsdt_applesmc_sta;
../hw/i386/q35-acpi-dsdt.dsl:#define DSDT_APPLESMC_STA q35_dsdt_applesmc_sta
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1373362/+subscriptions
[Qemu-devel] [Bug 62244] Re: Debugging mode not enabled in Bochs.
** Also affects: qemu Importance: Undecided Status: New ** No longer affects: qemu -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/62244 Title: Debugging mode not enabled in Bochs. Status in “bochs” package in Ubuntu: Invalid Bug description: The Bochs package for Ubuntu doesn't include the debugger, which is one of Bochs' main use within communities who use it. (ref: http://wiki.cs.uiuc.edu/cs427/Bochs+Debugging+and+Instrumentation) It'd be helpful to the majority of Bochs users if it was included (--enable-debugger for configure). To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/bochs/+bug/62244/+subscriptions
[Qemu-devel] [PATCH 0/5] Add TriCore ABS, ABSB, B, BIT, BO instructions
Hi guys, here is the next round of TriCore patches. The first patch addresses a clang issue mentioned by Peter Maydell and some bugfixes. And the other four add instructions of the ABS, ABSB, B, BIT and BO opcode format. Thanks, Bastian Bastian Koppelmann (5): target-tricore: Cleanup and Bugfixes target-tricore: Add instructions of ABS, ABSB opcode format target-tricore: Add instructions of B opcode format target-tricore: Add instructions of BIT opcode format target-tricore: Add instructions of BO opcode format target-tricore/helper.h | 46 ++ target-tricore/op_helper.c | 575 - target-tricore/translate.c | 1261 ++ target-tricore/tricore-opcodes.h |4 +- 4 files changed, 1859 insertions(+), 27 deletions(-) -- 2.1.1
[Qemu-devel] [PATCH 1/5] target-tricore: Cleanup and Bugfixes
Move FCX loading of save_context_ to caller functions, for STLCX, STUCX insn to
use those functions.
Move FCX storing of restore_context_ to caller functions, for LDLCX, LDUCX insn
to use those functions.
Remove do_raise_exception function, which caused clang to emit a warning.
Fix: save_context_lower now saves a[11] instead of PSW.
Fix: MASK_OP_ABSB_BPOS starting at wrong offset.
Signed-off-by: Bastian Koppelmann
---
target-tricore/op_helper.c | 47 ++--
target-tricore/tricore-opcodes.h | 2 +-
2 files changed, 22 insertions(+), 27 deletions(-)
diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
index 6376f07..f2a5cbc 100644
--- a/target-tricore/op_helper.c
+++ b/target-tricore/op_helper.c
@@ -114,10 +114,8 @@ static bool cdc_zero(target_ulong *psw)
return count == 0;
}
-static void save_context_upper(CPUTriCoreState *env, int ea,
- target_ulong *new_FCX)
+static void save_context_upper(CPUTriCoreState *env, int ea)
{
-*new_FCX = cpu_ldl_data(env, ea);
cpu_stl_data(env, ea, env->PCXI);
cpu_stl_data(env, ea+4, env->PSW);
cpu_stl_data(env, ea+8, env->gpr_a[10]);
@@ -134,15 +132,12 @@ static void save_context_upper(CPUTriCoreState *env, int
ea,
cpu_stl_data(env, ea+52, env->gpr_d[13]);
cpu_stl_data(env, ea+56, env->gpr_d[14]);
cpu_stl_data(env, ea+60, env->gpr_d[15]);
-
}
-static void save_context_lower(CPUTriCoreState *env, int ea,
- target_ulong *new_FCX)
+static void save_context_lower(CPUTriCoreState *env, int ea)
{
-*new_FCX = cpu_ldl_data(env, ea);
cpu_stl_data(env, ea, env->PCXI);
-cpu_stl_data(env, ea+4, env->PSW);
+cpu_stl_data(env, ea+4, env->gpr_a[11]);
cpu_stl_data(env, ea+8, env->gpr_a[2]);
cpu_stl_data(env, ea+12, env->gpr_a[3]);
cpu_stl_data(env, ea+16, env->gpr_d[0]);
@@ -178,7 +173,6 @@ static void restore_context_upper(CPUTriCoreState *env, int
ea,
env->gpr_d[13] = cpu_ldl_data(env, ea+52);
env->gpr_d[14] = cpu_ldl_data(env, ea+56);
env->gpr_d[15] = cpu_ldl_data(env, ea+60);
-cpu_stl_data(env, ea, env->FCX);
}
void helper_call(CPUTriCoreState *env, uint32_t next_pc)
@@ -206,11 +200,12 @@ void helper_call(CPUTriCoreState *env, uint32_t next_pc)
/* EA = {FCX.FCXS, 6'b0, FCX.FCXO, 6'b0}; */
ea = ((env->FCX & MASK_FCX_FCXS) << 12) +
((env->FCX & MASK_FCX_FCXO) << 6);
-/* new_FCX = M(EA, word);
- M(EA, 16 * word) = {PCXI, PSW, A[10], A[11], D[8], D[9], D[10], D[11],
- A[12], A[13], A[14], A[15], D[12], D[13], D[14],
- D[15]}; */
-save_context_upper(env, ea, &new_FCX);
+/* new_FCX = M(EA, word); */
+new_FCX = cpu_ldl_data(env, ea);
+/* M(EA, 16 * word) = {PCXI, PSW, A[10], A[11], D[8], D[9], D[10], D[11],
+ A[12], A[13], A[14], A[15], D[12], D[13], D[14],
+ D[15]}; */
+save_context_upper(env, ea);
/* PCXI.PCPN = ICR.CCPN; */
env->PCXI = (env->PCXI & 0xff) +
@@ -263,9 +258,10 @@ void helper_ret(CPUTriCoreState *env)
ea = ((env->PCXI & MASK_PCXI_PCXS) << 12) +
((env->PCXI & MASK_PCXI_PCXO) << 6);
/* {new_PCXI, new_PSW, A[10], A[11], D[8], D[9], D[10], D[11], A[12],
-A[13], A[14], A[15], D[12], D[13], D[14], D[15]} = M(EA, 16 * word);
-M(EA, word) = FCX; */
+A[13], A[14], A[15], D[12], D[13], D[14], D[15]} = M(EA, 16 * word); */
restore_context_upper(env, ea, &new_PCXI, &new_PSW);
+/* M(EA, word) = FCX; */
+cpu_stl_data(env, ea, env->FCX);
/* FCX[19: 0] = PCXI[19: 0]; */
env->FCX = (env->FCX & 0xfff0) + (env->PCXI & 0x000f);
/* PCXI = new_PCXI; */
@@ -293,7 +289,12 @@ void helper_bisr(CPUTriCoreState *env, uint32_t const9)
tmp_FCX = env->FCX;
ea = ((env->FCX & 0xf) << 12) + ((env->FCX & 0x) << 6);
-save_context_lower(env, ea, &new_FCX);
+/* new_FCX = M(EA, word); */
+new_FCX = cpu_ldl_data(env, ea);
+/* M(EA, 16 * word) = {PCXI, A[11], A[2], A[3], D[0], D[1], D[2], D[3],
A[4]
+ , A[5], A[6], A[7], D[4], D[5], D[6], D[7]}; */
+save_context_lower(env, ea);
+
/* PCXI.PCPN = ICR.CCPN */
env->PCXI = (env->PCXI & 0xff) +
@@ -343,9 +344,10 @@ void helper_rfe(CPUTriCoreState *env)
ea = ((env->PCXI & MASK_PCXI_PCXS) << 12) +
((env->PCXI & MASK_PCXI_PCXO) << 6);
/*{new_PCXI, PSW, A[10], A[11], D[8], D[9], D[10], D[11], A[12],
- A[13], A[14], A[15], D[12], D[13], D[14], D[15]} = M(EA, 16 * word);
- M(EA, word) = FCX;*/
+ A[13], A[14], A[15], D[12], D[13], D[14], D[15]} = M(EA, 16 * word); */
restore_context_upper(env, ea, &new_PCXI, &new_PSW);
+/* M(EA, word) = FCX;*/
+cpu_stl_data(env, ea, env->FCX);
/* FCX[19: 0] = PCXI[19: 0]; */
env->FCX = (env->FCX & 0xfff0) + (env->PCXI & 0x000f);
/*
[Qemu-devel] [PATCH 4/5] target-tricore: Add instructions of BIT opcode format
Add instructions of BIT opcode format.
Add microcode generator functions gen_bit_1/2op to do 1/2 bit operations on the
last bit.
Signed-off-by: Bastian Koppelmann
---
target-tricore/translate.c | 349 +
1 file changed, 349 insertions(+)
diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 871c3cd..34375a9 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -427,6 +427,56 @@ static inline void gen_subs(TCGv ret, TCGv r1, TCGv r2)
gen_helper_sub_ssov(ret, cpu_env, r1, r2);
}
+/* D[c] = D[c][0] op1 (D[a][pos1] op2 D[b][pos2]);*/
+static inline void gen_bit_2op(TCGv ret, TCGv r1, TCGv r2, TCGv r3,
+ int pos1, int pos2,
+ void(*op1)(TCGv, TCGv, TCGv),
+ void(*op2)(TCGv, TCGv, TCGv))
+{
+TCGv temp1, temp2, temp3;
+
+temp1 = tcg_temp_new();
+temp2 = tcg_temp_new();
+temp3 = tcg_temp_new();
+
+tcg_gen_andi_tl(temp3, r3, 0x1);
+
+tcg_gen_andi_tl(temp2, r2 , (0x1u << pos2));
+tcg_gen_shri_tl(temp2, temp2, pos2);
+
+tcg_gen_andi_tl(temp1, r1, (0x1u << pos1));
+tcg_gen_shri_tl(temp1, temp1, pos1);
+
+(*op1)(temp1, temp1, temp2);
+(*op2)(ret , temp3, temp1);
+
+tcg_temp_free(temp1);
+tcg_temp_free(temp2);
+tcg_temp_free(temp3);
+}
+
+/* result = D[a][pos1] op1 D[b][pos2]; */
+static inline void gen_bit_1op(TCGv ret, TCGv r1, TCGv r2,
+ int pos1, int pos2,
+ void(*op1)(TCGv, TCGv, TCGv))
+{
+TCGv temp1, temp2;
+
+temp1 = tcg_temp_new();
+temp2 = tcg_temp_new();
+
+tcg_gen_andi_tl(temp2, r2, (0x1u << pos2));
+tcg_gen_shri_tl(temp2, temp2, pos2);
+
+tcg_gen_andi_tl(temp1, r1, (0x1u << pos1));
+tcg_gen_shri_tl(temp1, temp1, pos1);
+
+(*op1)(ret, temp1, temp2);
+
+tcg_temp_free(temp1);
+tcg_temp_free(temp2);
+}
+
/* helpers for generating program flow micro-ops */
static inline void gen_save_pc(target_ulong pc)
@@ -1347,6 +1397,283 @@ static void decode_abs_storeb_h(CPUTriCoreState *env,
DisasContext *ctx)
tcg_temp_free(temp);
}
+/* Bit-format */
+
+static void decode_bit_andacc(CPUTriCoreState *env, DisasContext *ctx)
+{
+uint32_t op2;
+int r1, r2, r3;
+int pos1, pos2;
+TCGv temp;
+
+r1 = MASK_OP_BIT_S1(ctx->opcode);
+r2 = MASK_OP_BIT_S2(ctx->opcode);
+r3 = MASK_OP_BIT_D(ctx->opcode);
+pos1 = MASK_OP_BIT_POS1(ctx->opcode);
+pos2 = MASK_OP_BIT_POS2(ctx->opcode);
+op2 = MASK_OP_BIT_OP2(ctx->opcode);
+
+temp = tcg_temp_new();
+
+switch (op2) {
+case OPC2_32_BIT_AND_AND_T:
+gen_bit_2op(temp, cpu_gpr_d[r1], cpu_gpr_d[r2], cpu_gpr_d[r3],
+pos1, pos2, &tcg_gen_and_tl, &tcg_gen_and_tl);
+break;
+case OPC2_32_BIT_AND_ANDN_T:
+gen_bit_2op(temp, cpu_gpr_d[r1], cpu_gpr_d[r2], cpu_gpr_d[r3],
+pos1, pos2, &tcg_gen_andc_tl, &tcg_gen_and_tl);
+break;
+case OPC2_32_BIT_AND_NOR_T:
+gen_bit_2op(temp, cpu_gpr_d[r1], cpu_gpr_d[r2], cpu_gpr_d[r3],
+pos1, pos2, &tcg_gen_or_tl, &tcg_gen_andc_tl);
+break;
+case OPC2_32_BIT_AND_OR_T:
+gen_bit_2op(temp, cpu_gpr_d[r1], cpu_gpr_d[r2], cpu_gpr_d[r3],
+pos1, pos2, &tcg_gen_or_tl, &tcg_gen_and_tl);
+break;
+}
+tcg_gen_andi_tl(temp, temp, 0x1);
+tcg_gen_andi_tl(cpu_gpr_d[r3], cpu_gpr_d[r3], 0xfffe);
+tcg_gen_add_tl(cpu_gpr_d[r3], cpu_gpr_d[r3], temp);
+tcg_temp_free(temp);
+}
+
+static void decode_bit_logical_t(CPUTriCoreState *env, DisasContext *ctx)
+{
+uint32_t op2;
+int r1, r2, r3;
+int pos1, pos2;
+r1 = MASK_OP_BIT_S1(ctx->opcode);
+r2 = MASK_OP_BIT_S2(ctx->opcode);
+r3 = MASK_OP_BIT_D(ctx->opcode);
+pos1 = MASK_OP_BIT_POS1(ctx->opcode);
+pos2 = MASK_OP_BIT_POS2(ctx->opcode);
+op2 = MASK_OP_BIT_OP2(ctx->opcode);
+
+switch (op2) {
+case OPC2_32_BIT_AND_T:
+gen_bit_1op(cpu_gpr_d[r3], cpu_gpr_d[r1], cpu_gpr_d[r2],
+pos1, pos2, &tcg_gen_and_tl);
+break;
+case OPC2_32_BIT_ANDN_T:
+gen_bit_1op(cpu_gpr_d[r3], cpu_gpr_d[r1], cpu_gpr_d[r2],
+pos1, pos2, &tcg_gen_andc_tl);
+break;
+case OPC2_32_BIT_NOR_T:
+gen_bit_1op(cpu_gpr_d[r3], cpu_gpr_d[r1], cpu_gpr_d[r2],
+pos1, pos2, &tcg_gen_nor_tl);
+tcg_gen_andi_tl(cpu_gpr_d[r3], cpu_gpr_d[r3], 0x1);
+break;
+case OPC2_32_BIT_OR_T:
+gen_bit_1op(cpu_gpr_d[r3], cpu_gpr_d[r1], cpu_gpr_d[r2],
+pos1, pos2, &tcg_gen_or_tl);
+break;
+}
+}
+
+static void decode_bit_insert(CPUTriCoreState *env, DisasContext *ctx)
+{
+uint32_t op2;
+int r1, r2, r3;
+int pos1, pos2;
+TCGv temp, temp2;
+op2 = MASK_OP_BIT_OP2(ctx->opcode);
+r1 = MASK_OP_BIT_S1(ctx->opcode);
+r2
[Qemu-devel] [PATCH 2/5] target-tricore: Add instructions of ABS, ABSB opcode format
Add instructions of ABS, ABSB opcode format.
Add microcode generator functions for ld/st of two 32bit reg as one 64bit value.
Add microcode generator functions for ldmst and swap.
Add helper ldlcx, lducx, stlcx and stucx.
Signed-off-by: Bastian Koppelmann
---
target-tricore/helper.h| 4 +
target-tricore/op_helper.c | 45 +++
target-tricore/translate.c | 303 +
3 files changed, 352 insertions(+)
diff --git a/target-tricore/helper.h b/target-tricore/helper.h
index 7b7d74b..fbabbd5 100644
--- a/target-tricore/helper.h
+++ b/target-tricore/helper.h
@@ -23,3 +23,7 @@ DEF_HELPER_2(call, void, env, i32)
DEF_HELPER_1(ret, void, env)
DEF_HELPER_2(bisr, void, env, i32)
DEF_HELPER_1(rfe, void, env)
+DEF_HELPER_2(ldlcx, void, env, i32)
+DEF_HELPER_2(lducx, void, env, i32)
+DEF_HELPER_2(stlcx, void, env, i32)
+DEF_HELPER_2(stucx, void, env, i32)
diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
index f2a5cbc..7a33afd 100644
--- a/target-tricore/op_helper.c
+++ b/target-tricore/op_helper.c
@@ -175,6 +175,27 @@ static void restore_context_upper(CPUTriCoreState *env,
int ea,
env->gpr_d[15] = cpu_ldl_data(env, ea+60);
}
+static void restore_context_lower(CPUTriCoreState *env, int ea,
+ target_ulong *ra, target_ulong *pcxi)
+{
+*pcxi = cpu_ldl_data(env, ea);
+*ra = cpu_ldl_data(env, ea+4);
+env->gpr_a[2] = cpu_ldl_data(env, ea+8);
+env->gpr_a[3] = cpu_ldl_data(env, ea+12);
+env->gpr_d[0] = cpu_ldl_data(env, ea+16);
+env->gpr_d[1] = cpu_ldl_data(env, ea+20);
+env->gpr_d[2] = cpu_ldl_data(env, ea+24);
+env->gpr_d[3] = cpu_ldl_data(env, ea+28);
+env->gpr_a[4] = cpu_ldl_data(env, ea+32);
+env->gpr_a[5] = cpu_ldl_data(env, ea+36);
+env->gpr_a[6] = cpu_ldl_data(env, ea+40);
+env->gpr_a[7] = cpu_ldl_data(env, ea+44);
+env->gpr_d[4] = cpu_ldl_data(env, ea+48);
+env->gpr_d[5] = cpu_ldl_data(env, ea+52);
+env->gpr_d[6] = cpu_ldl_data(env, ea+56);
+env->gpr_d[7] = cpu_ldl_data(env, ea+60);
+}
+
void helper_call(CPUTriCoreState *env, uint32_t next_pc)
{
target_ulong tmp_FCX;
@@ -356,6 +377,30 @@ void helper_rfe(CPUTriCoreState *env)
psw_write(env, new_PSW);
}
+void helper_ldlcx(CPUTriCoreState *env, uint32_t ea)
+{
+uint32_t dummy;
+/* insn doesn't load PCXI and RA */
+restore_context_lower(env, ea, &dummy, &dummy);
+}
+
+void helper_lducx(CPUTriCoreState *env, uint32_t ea)
+{
+uint32_t dummy;
+/* insn doesn't load PCXI and PSW */
+restore_context_upper(env, ea, &dummy, &dummy);
+}
+
+void helper_stlcx(CPUTriCoreState *env, uint32_t ea)
+{
+save_context_lower(env, ea);
+}
+
+void helper_stucx(CPUTriCoreState *env, uint32_t ea)
+{
+save_context_upper(env, ea);
+}
+
static inline void QEMU_NORETURN do_raise_exception_err(CPUTriCoreState *env,
uint32_t exception,
int error_code,
diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 4f654de..3ec5ca7 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -115,6 +115,8 @@ void tricore_cpu_dump_state(CPUState *cs, FILE *f,
tcg_temp_free_i32(helper_tmp);\
} while (0)
+#define EA_ABS_FORMAT(con) (((con & 0x3C000) << 14) + (con & 0x3FFF))
+
/* Functions for load/save to/from memory */
static inline void gen_offset_ld(DisasContext *ctx, TCGv r1, TCGv r2,
@@ -135,6 +137,64 @@ static inline void gen_offset_st(DisasContext *ctx, TCGv
r1, TCGv r2,
tcg_temp_free(temp);
}
+static void gen_st_2regs_64(TCGv rh, TCGv rl, TCGv address, DisasContext *ctx)
+{
+TCGv_i64 temp = tcg_temp_new_i64();
+
+tcg_gen_concat_i32_i64(temp, rl, rh);
+tcg_gen_qemu_st_i64(temp, address, ctx->mem_idx, MO_LEQ);
+
+tcg_temp_free_i64(temp);
+}
+
+static void gen_ld_2regs_64(TCGv rh, TCGv rl, TCGv address, DisasContext *ctx)
+{
+TCGv_i64 temp = tcg_temp_new_i64();
+
+tcg_gen_qemu_ld_i64(temp, address, ctx->mem_idx, MO_LEQ);
+/* write back to two 32 bit regs */
+tcg_gen_trunc_i64_i32(rl, temp);
+tcg_gen_shri_i64(temp, temp, 32);
+tcg_gen_trunc_i64_i32(rh, temp);
+
+tcg_temp_free_i64(temp);
+}
+
+/* M(EA, word) = (M(EA, word) & ~E[a][63:32]) | (E[a][31:0] & E[a][63:32]); */
+static void gen_ldmst(DisasContext *ctx, int ereg, TCGv ea)
+{
+TCGv temp = tcg_temp_new();
+TCGv temp2 = tcg_temp_new();
+
+/* temp = (M(EA, word) */
+tcg_gen_qemu_ld_tl(temp, ea, ctx->mem_idx, MO_LEUL);
+/* temp = temp & ~E[a][63:32]) */
+tcg_gen_andc_tl(temp, temp, cpu_gpr_d[ereg+1]);
+/* temp2 = (E[a][31:0] & E[a][63:32]); */
+tcg_gen_and_tl(temp2, cpu_gpr_d[ereg], cpu_gpr_d[ereg+1]);
+/* temp = temp | temp2; */
+tcg_gen_or_tl(temp, temp, temp2);
+/* M(EA, word) = temp; */
+tcg_gen_qemu_st_tl(temp, ea, ctx->mem_idx, MO_LEUL)
[Qemu-devel] [PATCH 3/5] target-tricore: Add instructions of B opcode format
Add instructions of B opcode format.
Signed-off-by: Bastian Koppelmann
---
target-tricore/translate.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 3ec5ca7..871c3cd 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -116,6 +116,8 @@ void tricore_cpu_dump_state(CPUState *cs, FILE *f,
} while (0)
#define EA_ABS_FORMAT(con) (((con & 0x3C000) << 14) + (con & 0x3FFF))
+#define EA_B_FORMAT(con) (((offset & 0xf0) << 8) | \
+ ((offset & 0x0f) << 1))
/* Functions for load/save to/from memory */
@@ -494,6 +496,7 @@ static void gen_compute_branch(DisasContext *ctx, uint32_t
opc, int r1,
case OPC1_32_B_J:
gen_goto_tb(ctx, 0, ctx->pc + offset * 2);
break;
+case OPC1_32_B_CALL:
case OPC1_16_SB_CALL:
gen_helper_1arg(call, ctx->next_pc);
gen_goto_tb(ctx, 0, ctx->pc + offset * 2);
@@ -569,6 +572,20 @@ static void gen_compute_branch(DisasContext *ctx, uint32_t
opc, int r1,
gen_helper_ret(cpu_env);
tcg_gen_exit_tb(0);
break;
+/* B-format */
+case OPC1_32_B_CALLA:
+gen_helper_1arg(call, ctx->next_pc);
+gen_goto_tb(ctx, 0, EA_B_FORMAT(offset));
+break;
+case OPC1_32_B_JLA:
+tcg_gen_movi_tl(cpu_gpr_a[11], ctx->next_pc);
+case OPC1_32_B_JA:
+gen_goto_tb(ctx, 0, EA_B_FORMAT(offset));
+break;
+case OPC1_32_B_JL:
+tcg_gen_movi_tl(cpu_gpr_a[11], ctx->next_pc);
+gen_goto_tb(ctx, 0, ctx->pc + offset * 2);
+break;
default:
printf("Branch Error at %x\n", ctx->pc);
}
@@ -1403,6 +1420,16 @@ static void decode_32Bit_opc(CPUTriCoreState *env,
DisasContext *ctx)
tcg_temp_free(temp);
tcg_temp_free(temp2);
break;
+/* B-format */
+case OPC1_32_B_CALL:
+case OPC1_32_B_CALLA:
+case OPC1_32_B_J:
+case OPC1_32_B_JA:
+case OPC1_32_B_JL:
+case OPC1_32_B_JLA:
+address = MASK_OP_B_DISP24(ctx->opcode);
+gen_compute_branch(ctx, op1, 0, 0, 0, address);
+break;
}
}
--
2.1.1
[Qemu-devel] [PATCH 5/5] target-tricore: Add instructions of BO opcode format
Add instructions of BO opcode format.
Add microcode generator functions gen_swap, gen_ldmst.
Add helper for loading/storing byte, halfword, upper halfword word, dword in
circular and bit reverse addr mode
Add sign extended bitmask for BO_OFF10 field.
Signed-off-by: Bastian Koppelmann
---
target-tricore/helper.h | 42 +++
target-tricore/op_helper.c | 483
target-tricore/translate.c | 582 +++
target-tricore/tricore-opcodes.h | 2 +
4 files changed, 1109 insertions(+)
diff --git a/target-tricore/helper.h b/target-tricore/helper.h
index fbabbd5..ee8c9a7 100644
--- a/target-tricore/helper.h
+++ b/target-tricore/helper.h
@@ -27,3 +27,45 @@ DEF_HELPER_2(ldlcx, void, env, i32)
DEF_HELPER_2(lducx, void, env, i32)
DEF_HELPER_2(stlcx, void, env, i32)
DEF_HELPER_2(stucx, void, env, i32)
+/* ld circ */
+DEF_HELPER_4(ld_b_circ, void, env, i32, i32, int)
+DEF_HELPER_4(ld_bu_circ, void, env, i32, i32, int)
+DEF_HELPER_4(ld_h_circ, void, env, i32, i32, int)
+DEF_HELPER_4(ld_hu_circ, void, env, i32, i32, int)
+DEF_HELPER_4(ld_q_circ, void, env, i32, i32, int)
+DEF_HELPER_4(ldmst_circ, void, env, i32, i32, int)
+DEF_HELPER_4(ld_a_circ, void, env, i32, i32, int)
+DEF_HELPER_4(ld_w_circ, void, env, i32, i32, int)
+DEF_HELPER_4(ld_d_circ, void, env, i32, i32, int)
+DEF_HELPER_4(ld_da_circ, void, env, i32, i32, int)
+/* st circ */
+DEF_HELPER_4(st_a_circ, void, env, i32, i32, int)
+DEF_HELPER_4(st_b_circ, void, env, i32, i32, int)
+DEF_HELPER_4(st_d_circ, void, env, i32, i32, int)
+DEF_HELPER_4(st_da_circ, void, env, i32, i32, int)
+DEF_HELPER_4(st_h_circ, void, env, i32, i32, int)
+DEF_HELPER_4(st_q_circ, void, env, i32, i32, int)
+DEF_HELPER_4(st_w_circ, void, env, i32, i32, int)
+DEF_HELPER_4(swap_circ, void, env, i32, i32, int)
+DEF_HELPER_3(empty_circ, void, env, i32, int)
+/* ld br */
+DEF_HELPER_3(ld_b_br, void, env, i32, i32)
+DEF_HELPER_3(ld_bu_br, void, env, i32, i32)
+DEF_HELPER_3(ld_h_br, void, env, i32, i32)
+DEF_HELPER_3(ld_hu_br, void, env, i32, i32)
+DEF_HELPER_3(ld_q_br, void, env, i32, i32)
+DEF_HELPER_3(ldmst_br, void, env, i32, i32)
+DEF_HELPER_3(ld_a_br, void, env, i32, i32)
+DEF_HELPER_3(ld_w_br, void, env, i32, i32)
+DEF_HELPER_3(ld_d_br, void, env, i32, i32)
+DEF_HELPER_3(ld_da_br, void, env, i32, i32)
+/* st br */
+DEF_HELPER_3(st_a_br, void, env, i32, i32)
+DEF_HELPER_3(st_b_br, void, env, i32, i32)
+DEF_HELPER_3(st_d_br, void, env, i32, i32)
+DEF_HELPER_3(st_da_br, void, env, i32, i32)
+DEF_HELPER_3(st_h_br, void, env, i32, i32)
+DEF_HELPER_3(st_q_br, void, env, i32, i32)
+DEF_HELPER_3(st_w_br, void, env, i32, i32)
+DEF_HELPER_3(swap_br, void, env, i32, i32)
+DEF_HELPER_2(empty_br, void, env, i32)
diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
index 7a33afd..c965a46 100644
--- a/target-tricore/op_helper.c
+++ b/target-tricore/op_helper.c
@@ -20,6 +20,489 @@
#include "exec/helper-proto.h"
#include "exec/cpu_ldst.h"
+/* Addressing mode helper */
+
+#define CIRC_BR_DEFINES(reg) \
+uint32_t index = env->gpr_a[reg+1] & 0x; \
+uint32_t length_incr = (env->gpr_a[reg+1] & 0x) >> 16; \
+
+static uint16_t reverse16(uint16_t val)
+{
+uint8_t high = (uint8_t)(val >> 8);
+uint8_t low = (uint8_t)(val & 0xff);
+
+uint16_t rh, rl;
+
+rl = (uint16_t)((high * 0x0202020202ULL & 0x010884422010ULL) % 1023);
+rh = (uint16_t)((low * 0x0202020202ULL & 0x010884422010ULL) % 1023);
+
+return (rh << 8) | rl;
+}
+
+#define BR_CALC_INDEX(reg, index, incr) do { \
+int32_t new_index = reverse16(reverse16(index) + reverse16(incr)); \
+env->gpr_a[reg+1] = (incr << 16) | new_index; \
+} while (0)
+
+#define CIRC_CALC_INDEX(reg, off, len) do { \
+int32_t new_index = index + off;\
+if (new_index < 0) {\
+new_index = new_index + len;\
+} else {\
+new_index = new_index % len;\
+} \
+env->gpr_a[reg+1] = (len << 16) | new_index; \
+} while (0)
+
+void helper_ld_b_circ(CPUTriCoreState *env, uint32_t r1, uint32_t r2, int
off10)
+{
+CIRC_BR_DEFINES(r2)
+
+uint32_t ea = env->gpr_a[r2] + index;
+
+env->gpr_d[r1] = cpu_ldsb_data(env, ea);
+
+CIRC_CALC_INDEX(r2, off10, length_incr);
+}
+
+void helper_ld_bu_circ(CPUTriCoreState *env, uint32_t r1, uint32_t r2,
+ int off10)
+{
+CIRC_BR_DEFINES(r2)
+
+uint32_t ea = env->gpr_a[r2] + index;
+
+env->gpr_d[r1] = cpu_ldub_data(env, ea);
+
+CIRC_CALC_INDEX(r2, off10, length_incr);
+}
+
+void helper_ld_h_circ(CPUTriCoreState *env, uint32_t r1, uint32_t r2, int
off10)
+{
+CIRC_BR_DEFINES(r2)
+
+uint32_t ea = env->gpr_a[r2] + index;
+
+env->gpr_d[r1] = cpu_ldsw_dat
Re: [Qemu-devel] [PATCH v6 0/2] dump: let dump_error return error reason to caller
Hi, Ping^2... Can anyone help reviewing this patch? Luiz? Eric? Markus? Thanks;) Best regards, zhanghailiang On 2014/9/24 8:39, zhanghailiang wrote: Hi, Ping... Thanks, zhanghailiang On 2014/9/19 14:43, zhanghailiang wrote: Hi, In original code, Function dump_error ignores its second parameter which contains error reason, it is better to return the error message to the caller. Here we use error_setg to return the error info to caller. And at the same time, we turn functions like write_elf64_note() to void, Because functions shouldn't return an error code _and_ an Error object. After this modification the code will be more clean. v5 -> v6 - Don't check *errp directly, if errp is NULL, there will be an error, Instead, check a local variable *local_err (Markus Armbruster) v4 -> v5: - Turn functions like write_elf64_note() to void (Luiz Capitulino) v3 -> v4: - Adjust the errp argument to the end - Remove trailing '.' in error messages v2 -> v3: - Drop the '\n' in the message when call dump_error(Eric Blake) v1 -> v2: - Return the error reason to the caller (Luiz Capitulino) zhanghailiang (2): dump: let dump_error return error info to caller dump: Don't return error code when return an Error object dump.c | 382 +++-- 1 file changed, 182 insertions(+), 200 deletions(-)
Re: [Qemu-devel] [PATCH 2/5] target-tricore: Add instructions of ABS, ABSB opcode format
On 09/27/2014 07:58 AM, Bastian Koppelmann wrote:
+env->gpr_a[2] = cpu_ldl_data(env, ea+8);
> +env->gpr_a[3] = cpu_ldl_data(env, ea+12);
> +env->gpr_d[0] = cpu_ldl_data(env, ea+16);
> +env->gpr_d[1] = cpu_ldl_data(env, ea+20);
> +env->gpr_d[2] = cpu_ldl_data(env, ea+24);
Watch the extra spaces.
> +static void gen_ld_2regs_64(TCGv rh, TCGv rl, TCGv address, DisasContext
> *ctx)
> +{
> +TCGv_i64 temp = tcg_temp_new_i64();
> +
> +tcg_gen_qemu_ld_i64(temp, address, ctx->mem_idx, MO_LEQ);
> +/* write back to two 32 bit regs */
> +tcg_gen_trunc_i64_i32(rl, temp);
> +tcg_gen_shri_i64(temp, temp, 32);
> +tcg_gen_trunc_i64_i32(rh, temp);
tcg_gen_extr_i64_i32(rl, rh, temp)
> +/* ABSB-format */
> +case OPC1_32_ABSB_ST_T:
> +address = MASK_OP_ABS_OFF18(ctx->opcode);
> +int8_t b = MASK_OP_ABSB_B(ctx->opcode);
> +int32_t bpos = MASK_OP_ABSB_BPOS(ctx->opcode);
You're not supposed to be relying on c99 mixed code and declarations.
r~
Re: [Qemu-devel] [PATCH 3/5] target-tricore: Add instructions of B opcode format
On 09/27/2014 07:58 AM, Bastian Koppelmann wrote: > Add instructions of B opcode format. > > Signed-off-by: Bastian Koppelmann > --- > target-tricore/translate.c | 27 +++ > 1 file changed, 27 insertions(+) Reviewed-by: Richard Henderson
Re: [Qemu-devel] [PATCH v3 6/7] scsi: Introduce scsi_req_cancel_async
On Thu, 09/25 10:51, Paolo Bonzini wrote:
> Il 25/09/2014 04:20, Fam Zheng ha scritto:
> > Devices will call this function to start an asynchronous cancellation. The
> > bus->info->cancel will be called after the request is canceled.
> >
> > Devices will probably need to track a separate TMF request that triggers
> > this
> > cancellation, and wait until the cancellation is done before completing it.
> > So
> > we store a notifier list in SCSIRequest and in scsi_req_cancel_complete we
> > notify them.
> >
> > Signed-off-by: Fam Zheng
> > ---
> > hw/scsi/scsi-bus.c | 23 +++
> > include/hw/scsi/scsi.h | 3 +++
> > 2 files changed, 26 insertions(+)
> >
> > diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> > index c91db63..df7585a 100644
> > --- a/hw/scsi/scsi-bus.c
> > +++ b/hw/scsi/scsi-bus.c
> > @@ -566,6 +566,7 @@ SCSIRequest *scsi_req_alloc(const SCSIReqOps *reqops,
> > SCSIDevice *d,
> > req->ops = reqops;
> > object_ref(OBJECT(d));
> > object_ref(OBJECT(qbus->parent));
> > +notifier_list_init(&req->cancel_notifiers);
> > trace_scsi_req_alloc(req->dev->id, req->lun, req->tag);
> > return req;
> > }
> > @@ -1725,9 +1726,31 @@ void scsi_req_cancel_complete(SCSIRequest *req)
> > if (req->bus->info->cancel) {
> > req->bus->info->cancel(req);
> > }
> > +notifier_list_notify(&req->cancel_notifiers, req);
>
> I think you also have to call notifier_list_notify from
> scsi_req_complete, because a cancelled request might end up being
> completed instead of cancelled.
Yes, will update the series.
>
> In fact, the next obvious step (enabled by your bdrv_aio_cancel cleanup)
> would be to _not_ call scsi_req_cancel_complete if we can report
> completion or if there was an I/O error. This can happen for
> scsi-generic, scsi_dma_complete_noio, etc. Basically, moving the
> io_canceled check from the beginning of the completion routine to just
> before bdrv_aio_* or dma_aio_* are called.
Okay.
Thanks,
Fam
[Qemu-devel] [PATCH v4 0/7] virtio-scsi: Asynchronous cancellation
v4: Call notifier_list_notify in scsi_req_complete. (Paolo) v3: Address Paolo's comments: - scsi_req_canceled -> scsi_req_cancel_complete - Drop unnecessary changes in scsi_req_alloc and scsi_req_unref. - Update comment of scsi_req_cancel_async. - Add notifier only if not null. - Use slice allocator for VirtIOSCSICancelNotifier and VirtIOSCSICancelTracker. - Use int return value (0/-EINPROGRESS) for virtio_scsi_do_tmf. This series changes VIRTIO_SCSI_T_TMF_ABORT_TASK and VIRTIO_SCSI_T_TMF_ABORT_TASK_SET emulation to asynchronous by making use of bdrv_aio_cancel_async. Before, when guest cancels a SCSI command, we use a nested poll loop to wait until the request is cancelled or completed before returning. This blocks the whole vm and makes the guest unresponsive if the backend block device takes time to complete it, possibly because of slow IO, throttling, network issue, etc.. Now we return to the guest to allow vcpus to run before completing the TMF, and only after all the requests have been canceled, we notify the guest about the completing of the TMF command. Fam Zheng (7): scsi: Drop scsi_req_abort scsi-generic: Handle canceled request in scsi_command_complete scsi-bus: Unify request unref in scsi_req_cancel scsi: Drop SCSIReqOps.cancel_io scsi: Introduce scsi_req_cancel_complete scsi: Introduce scsi_req_cancel_async virtio-scsi: Handle TMF request cancellation asynchronously hw/scsi/scsi-bus.c | 43 + hw/scsi/scsi-disk.c| 59 ++- hw/scsi/scsi-generic.c | 37 ++ hw/scsi/spapr_vscsi.c | 11 +-- hw/scsi/virtio-scsi.c | 85 +- include/hw/scsi/scsi.h | 5 ++- 6 files changed, 145 insertions(+), 95 deletions(-) -- 1.9.3
[Qemu-devel] [PATCH v4 6/7] scsi: Introduce scsi_req_cancel_async
Devices will call this function to start an asynchronous cancellation. The
bus->info->cancel will be called after the request is canceled.
Devices will probably need to track a separate TMF request that triggers this
cancellation, and wait until the cancellation is done before completing it. So
we store a notifier list in SCSIRequest and in scsi_req_cancel_complete we
notify them.
Signed-off-by: Fam Zheng
---
hw/scsi/scsi-bus.c | 26 ++
include/hw/scsi/scsi.h | 3 +++
2 files changed, 29 insertions(+)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index c91db63..0f3e039 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -566,6 +566,7 @@ SCSIRequest *scsi_req_alloc(const SCSIReqOps *reqops,
SCSIDevice *d,
req->ops = reqops;
object_ref(OBJECT(d));
object_ref(OBJECT(qbus->parent));
+notifier_list_init(&req->cancel_notifiers);
trace_scsi_req_alloc(req->dev->id, req->lun, req->tag);
return req;
}
@@ -1715,6 +1716,9 @@ void scsi_req_complete(SCSIRequest *req, int status)
scsi_req_ref(req);
scsi_req_dequeue(req);
req->bus->info->complete(req, req->status, req->resid);
+
+/* Cancelled requests might end up being completed instead of cancelled */
+notifier_list_notify(&req->cancel_notifiers, req);
scsi_req_unref(req);
}
@@ -1725,9 +1729,31 @@ void scsi_req_cancel_complete(SCSIRequest *req)
if (req->bus->info->cancel) {
req->bus->info->cancel(req);
}
+notifier_list_notify(&req->cancel_notifiers, req);
scsi_req_unref(req);
}
+/* Cancel @req asynchronously. @notifier is added to @req's cancellation
+ * notifier list, the bus will be notified the requests cancellation is
+ * completed.
+ * */
+void scsi_req_cancel_async(SCSIRequest *req, Notifier *notifier)
+{
+trace_scsi_req_cancel(req->dev->id, req->lun, req->tag);
+if (notifier) {
+notifier_list_add(&req->cancel_notifiers, notifier);
+}
+if (req->io_canceled) {
+return;
+}
+scsi_req_ref(req);
+scsi_req_dequeue(req);
+req->io_canceled = true;
+if (req->aiocb) {
+bdrv_aio_cancel_async(req->aiocb);
+}
+}
+
void scsi_req_cancel(SCSIRequest *req)
{
trace_scsi_req_cancel(req->dev->id, req->lun, req->tag);
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index a75a7c8..c47dc53 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -5,6 +5,7 @@
#include "block/block.h"
#include "hw/block/block.h"
#include "sysemu/sysemu.h"
+#include "qemu/notify.h"
#define MAX_SCSI_DEVS 255
@@ -53,6 +54,7 @@ struct SCSIRequest {
void *hba_private;
size_tresid;
SCSICommand cmd;
+NotifierList cancel_notifiers;
/* Note:
* - fields before sense are initialized by scsi_req_alloc;
@@ -267,6 +269,7 @@ int scsi_req_get_sense(SCSIRequest *req, uint8_t *buf, int
len);
void scsi_req_abort(SCSIRequest *req, int status);
void scsi_req_cancel_complete(SCSIRequest *req);
void scsi_req_cancel(SCSIRequest *req);
+void scsi_req_cancel_async(SCSIRequest *req, Notifier *notifier);
void scsi_req_retry(SCSIRequest *req);
void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense);
void scsi_device_set_ua(SCSIDevice *sdev, SCSISense sense);
--
1.9.3
[Qemu-devel] [PATCH v4 2/7] scsi-generic: Handle canceled request in scsi_command_complete
Now that we always called the cb in bdrv_aio_cancel, let's make scsi-generic
callbacks check io_canceled flag similarly to scsi-disk.
Signed-off-by: Fam Zheng
---
hw/scsi/scsi-generic.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 20587b4..2a73a43 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -93,6 +93,9 @@ static void scsi_command_complete(void *opaque, int ret)
SCSIGenericReq *r = (SCSIGenericReq *)opaque;
r->req.aiocb = NULL;
+if (r->req.io_canceled) {
+goto done;
+}
if (r->io_header.driver_status & SG_ERR_DRIVER_SENSE) {
r->req.sense_len = r->io_header.sb_len_wr;
}
@@ -133,6 +136,7 @@ static void scsi_command_complete(void *opaque, int ret)
r, r->req.tag, status);
scsi_req_complete(&r->req, status);
+done:
if (!r->req.io_canceled) {
scsi_req_unref(&r->req);
}
@@ -186,8 +190,7 @@ static void scsi_read_complete(void * opaque, int ret)
int len;
r->req.aiocb = NULL;
-if (ret) {
-DPRINTF("IO error ret %d\n", ret);
+if (ret || r->req.io_canceled) {
scsi_command_complete(r, ret);
return;
}
@@ -246,8 +249,7 @@ static void scsi_write_complete(void * opaque, int ret)
DPRINTF("scsi_write_complete() ret = %d\n", ret);
r->req.aiocb = NULL;
-if (ret) {
-DPRINTF("IO error\n");
+if (ret || r->req.io_canceled) {
scsi_command_complete(r, ret);
return;
}
--
1.9.3
[Qemu-devel] [PATCH v4 1/7] scsi: Drop scsi_req_abort
The only user of this function is spapr_vscsi.c. We can convert to
scsi_req_cancel plus adding a check in vscsi_request_cancelled.
Suggested-by: Paolo Bonzini
Signed-off-by: Fam Zheng
---
hw/scsi/scsi-bus.c| 15 ---
hw/scsi/spapr_vscsi.c | 11 ---
2 files changed, 8 insertions(+), 18 deletions(-)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index af293b5..f90a204 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1736,21 +1736,6 @@ void scsi_req_cancel(SCSIRequest *req)
scsi_req_unref(req);
}
-void scsi_req_abort(SCSIRequest *req, int status)
-{
-if (!req->enqueued) {
-return;
-}
-scsi_req_ref(req);
-scsi_req_dequeue(req);
-req->io_canceled = true;
-if (req->ops->cancel_io) {
-req->ops->cancel_io(req);
-}
-scsi_req_complete(req, status);
-scsi_req_unref(req);
-}
-
static int scsi_ua_precedence(SCSISense sense)
{
if (sense.key != UNIT_ATTENTION) {
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index 048cfc7..ec5dc0d 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -77,8 +77,9 @@ typedef struct vscsi_req {
SCSIRequest *sreq;
uint32_tqtag; /* qemu tag != srp tag */
boolactive;
-uint32_tdata_len;
boolwriting;
+booldma_error;
+uint32_tdata_len;
uint32_tsenselen;
uint8_t sense[SCSI_SENSE_BUF_SIZE];
@@ -536,8 +537,8 @@ static void vscsi_transfer_data(SCSIRequest *sreq, uint32_t
len)
}
if (rc < 0) {
fprintf(stderr, "VSCSI: RDMA error rc=%d!\n", rc);
-vscsi_makeup_sense(s, req, HARDWARE_ERROR, 0, 0);
-scsi_req_abort(req->sreq, CHECK_CONDITION);
+req->dma_error = true;
+scsi_req_cancel(req->sreq);
return;
}
@@ -591,6 +592,10 @@ static void vscsi_request_cancelled(SCSIRequest *sreq)
{
vscsi_req *req = sreq->hba_private;
+if (req->dma_error) {
+vscsi_makeup_sense(s, req, HARDWARE_ERROR, 0, 0);
+vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
+}
vscsi_put_req(req);
}
--
1.9.3
[Qemu-devel] [PATCH v4 5/7] scsi: Introduce scsi_req_cancel_complete
Let the aio cb do the clean up and notification job after scsi_req_cancel, in
preparation for asynchronous cancellation.
Signed-off-by: Fam Zheng
---
hw/scsi/scsi-bus.c | 14 ++
hw/scsi/scsi-disk.c| 8
hw/scsi/scsi-generic.c | 1 +
include/hw/scsi/scsi.h | 1 +
4 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 764f6cf..c91db63 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1718,6 +1718,16 @@ void scsi_req_complete(SCSIRequest *req, int status)
scsi_req_unref(req);
}
+/* Called by the devices when the request is canceled. */
+void scsi_req_cancel_complete(SCSIRequest *req)
+{
+assert(req->io_canceled);
+if (req->bus->info->cancel) {
+req->bus->info->cancel(req);
+}
+scsi_req_unref(req);
+}
+
void scsi_req_cancel(SCSIRequest *req)
{
trace_scsi_req_cancel(req->dev->id, req->lun, req->tag);
@@ -1730,10 +1740,6 @@ void scsi_req_cancel(SCSIRequest *req)
if (req->aiocb) {
bdrv_aio_cancel(req->aiocb);
}
-if (req->bus->info->cancel) {
-req->bus->info->cancel(req);
-}
-scsi_req_unref(req);
}
static int scsi_ua_precedence(SCSISense sense)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index ef13e66..7a7938a 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -168,6 +168,7 @@ static void scsi_aio_complete(void *opaque, int ret)
r->req.aiocb = NULL;
block_acct_done(bdrv_get_stats(s->qdev.conf.bs), &r->acct);
if (r->req.io_canceled) {
+scsi_req_cancel_complete(&r->req);
goto done;
}
@@ -214,6 +215,7 @@ static void scsi_write_do_fua(SCSIDiskReq *r)
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
if (r->req.io_canceled) {
+scsi_req_cancel_complete(&r->req);
goto done;
}
@@ -240,6 +242,7 @@ static void scsi_dma_complete_noio(void *opaque, int ret)
block_acct_done(bdrv_get_stats(s->qdev.conf.bs), &r->acct);
}
if (r->req.io_canceled) {
+scsi_req_cancel_complete(&r->req);
goto done;
}
@@ -280,6 +283,7 @@ static void scsi_read_complete(void * opaque, int ret)
r->req.aiocb = NULL;
block_acct_done(bdrv_get_stats(s->qdev.conf.bs), &r->acct);
if (r->req.io_canceled) {
+scsi_req_cancel_complete(&r->req);
goto done;
}
@@ -312,6 +316,7 @@ static void scsi_do_read(void *opaque, int ret)
block_acct_done(bdrv_get_stats(s->qdev.conf.bs), &r->acct);
}
if (r->req.io_canceled) {
+scsi_req_cancel_complete(&r->req);
goto done;
}
@@ -432,6 +437,7 @@ static void scsi_write_complete(void * opaque, int ret)
block_acct_done(bdrv_get_stats(s->qdev.conf.bs), &r->acct);
}
if (r->req.io_canceled) {
+scsi_req_cancel_complete(&r->req);
goto done;
}
@@ -1524,6 +1530,7 @@ static void scsi_unmap_complete(void *opaque, int ret)
r->req.aiocb = NULL;
if (r->req.io_canceled) {
+scsi_req_cancel_complete(&r->req);
goto done;
}
@@ -1623,6 +1630,7 @@ static void scsi_write_same_complete(void *opaque, int
ret)
r->req.aiocb = NULL;
block_acct_done(bdrv_get_stats(s->qdev.conf.bs), &r->acct);
if (r->req.io_canceled) {
+scsi_req_cancel_complete(&r->req);
goto done;
}
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 7e85047..01bca08 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -94,6 +94,7 @@ static void scsi_command_complete(void *opaque, int ret)
r->req.aiocb = NULL;
if (r->req.io_canceled) {
+scsi_req_cancel_complete(&r->req);
goto done;
}
if (r->io_header.driver_status & SG_ERR_DRIVER_SENSE) {
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 1118107..a75a7c8 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -265,6 +265,7 @@ void scsi_req_complete(SCSIRequest *req, int status);
uint8_t *scsi_req_get_buf(SCSIRequest *req);
int scsi_req_get_sense(SCSIRequest *req, uint8_t *buf, int len);
void scsi_req_abort(SCSIRequest *req, int status);
+void scsi_req_cancel_complete(SCSIRequest *req);
void scsi_req_cancel(SCSIRequest *req);
void scsi_req_retry(SCSIRequest *req);
void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense);
--
1.9.3
[Qemu-devel] [PATCH v4 4/7] scsi: Drop SCSIReqOps.cancel_io
The only two implementations are identical to each other, with nothing specific
to device: they only call bdrv_aio_cancel with the SCSIRequest.aiocb.
Let's move it to scsi-bus.
Signed-off-by: Fam Zheng
---
hw/scsi/scsi-bus.c | 4 ++--
hw/scsi/scsi-disk.c| 14 --
hw/scsi/scsi-generic.c | 13 -
include/hw/scsi/scsi.h | 1 -
4 files changed, 2 insertions(+), 30 deletions(-)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index f90a204..764f6cf 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1727,8 +1727,8 @@ void scsi_req_cancel(SCSIRequest *req)
scsi_req_ref(req);
scsi_req_dequeue(req);
req->io_canceled = true;
-if (req->ops->cancel_io) {
-req->ops->cancel_io(req);
+if (req->aiocb) {
+bdrv_aio_cancel(req->aiocb);
}
if (req->bus->info->cancel) {
req->bus->info->cancel(req);
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 2e45752..ef13e66 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -105,18 +105,6 @@ static void scsi_check_condition(SCSIDiskReq *r, SCSISense
sense)
scsi_req_complete(&r->req, CHECK_CONDITION);
}
-/* Cancel a pending data transfer. */
-static void scsi_cancel_io(SCSIRequest *req)
-{
-SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
-
-DPRINTF("Cancel tag=0x%x\n", req->tag);
-if (r->req.aiocb) {
-bdrv_aio_cancel(r->req.aiocb);
-}
-r->req.aiocb = NULL;
-}
-
static uint32_t scsi_init_iovec(SCSIDiskReq *r, size_t size)
{
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
@@ -2325,7 +2313,6 @@ static const SCSIReqOps scsi_disk_emulate_reqops = {
.send_command = scsi_disk_emulate_command,
.read_data= scsi_disk_emulate_read_data,
.write_data = scsi_disk_emulate_write_data,
-.cancel_io= scsi_cancel_io,
.get_buf = scsi_get_buf,
};
@@ -2335,7 +2322,6 @@ static const SCSIReqOps scsi_disk_dma_reqops = {
.send_command = scsi_disk_dma_command,
.read_data= scsi_read_data,
.write_data = scsi_write_data,
-.cancel_io= scsi_cancel_io,
.get_buf = scsi_get_buf,
.load_request = scsi_disk_load_request,
.save_request = scsi_disk_save_request,
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index e92b418..7e85047 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -140,18 +140,6 @@ done:
scsi_req_unref(&r->req);
}
-/* Cancel a pending data transfer. */
-static void scsi_cancel_io(SCSIRequest *req)
-{
-SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
-
-DPRINTF("Cancel tag=0x%x\n", req->tag);
-if (r->req.aiocb) {
-bdrv_aio_cancel(r->req.aiocb);
-}
-r->req.aiocb = NULL;
-}
-
static int execute_command(BlockDriverState *bdrv,
SCSIGenericReq *r, int direction,
BlockDriverCompletionFunc *complete)
@@ -458,7 +446,6 @@ const SCSIReqOps scsi_generic_req_ops = {
.send_command = scsi_send_command,
.read_data= scsi_read_data,
.write_data = scsi_write_data,
-.cancel_io= scsi_cancel_io,
.get_buf = scsi_get_buf,
.load_request = scsi_generic_load_request,
.save_request = scsi_generic_save_request,
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 6271ad3..1118107 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -130,7 +130,6 @@ struct SCSIReqOps {
int32_t (*send_command)(SCSIRequest *req, uint8_t *buf);
void (*read_data)(SCSIRequest *req);
void (*write_data)(SCSIRequest *req);
-void (*cancel_io)(SCSIRequest *req);
uint8_t *(*get_buf)(SCSIRequest *req);
void (*save_request)(QEMUFile *f, SCSIRequest *req);
--
1.9.3
[Qemu-devel] [PATCH v4 7/7] virtio-scsi: Handle TMF request cancellation asynchronously
For VIRTIO_SCSI_T_TMF_ABORT_TASK and VIRTIO_SCSI_T_TMF_ABORT_TASK_SET,
use scsi_req_cancel_async to start the cancellation.
Because each tmf command may cancel multiple requests, we need to use a
counter to track the number of remaining requests we still need to wait
for.
Signed-off-by: Fam Zheng
---
hw/scsi/virtio-scsi.c | 85 ++-
1 file changed, 78 insertions(+), 7 deletions(-)
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index fa36e23..7a6b71a 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -208,12 +208,40 @@ static void *virtio_scsi_load_request(QEMUFile *f,
SCSIRequest *sreq)
return req;
}
-static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
+typedef struct {
+VirtIOSCSIReq *tmf_req;
+intremaining;
+} VirtIOSCSICancelTracker;
+
+typedef struct {
+Notifier notifier;
+VirtIOSCSICancelTracker *tracker;
+} VirtIOSCSICancelNotifier;
+
+static void virtio_scsi_cancel_notify(Notifier *notifier, void *data)
+{
+VirtIOSCSICancelNotifier *n = container_of(notifier,
+ VirtIOSCSICancelNotifier,
+ notifier);
+
+if (--n->tracker->remaining == 0) {
+virtio_scsi_complete_req(n->tracker->tmf_req);
+g_slice_free(VirtIOSCSICancelTracker, n->tracker);
+}
+g_slice_free(VirtIOSCSICancelNotifier, n);
+}
+
+/* Return 0 if the request is ready to be completed and return to guest;
+ * -EINPROGRESS if the request is submitted and will be completed later, in the
+ * case of async cancellation. */
+static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
{
SCSIDevice *d = virtio_scsi_device_find(s, req->req.tmf.lun);
SCSIRequest *r, *next;
BusChild *kid;
int target;
+int ret = 0;
+int cancel_count;
if (s->dataplane_started && bdrv_get_aio_context(d->conf.bs) != s->ctx) {
aio_context_acquire(s->ctx);
@@ -251,7 +279,18 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s,
VirtIOSCSIReq *req)
*/
req->resp.tmf.response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED;
} else {
-scsi_req_cancel(r);
+VirtIOSCSICancelNotifier *notifier;
+VirtIOSCSICancelTracker *tracker;
+
+notifier = g_slice_new(VirtIOSCSICancelNotifier);
+notifier->notifier.notify
+ = virtio_scsi_cancel_notify;
+tracker= g_slice_new(VirtIOSCSICancelTracker);
+tracker->tmf_req = req;
+tracker->remaining = 1;
+notifier->tracker = tracker;
+scsi_req_cancel_async(r, ¬ifier->notifier);
+ret = -EINPROGRESS;
}
}
break;
@@ -277,6 +316,7 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq
*req)
if (d->lun != virtio_scsi_get_lun(req->req.tmf.lun)) {
goto incorrect_lun;
}
+cancel_count = 0;
QTAILQ_FOREACH_SAFE(r, &d->requests, next, next) {
if (r->hba_private) {
if (req->req.tmf.subtype == VIRTIO_SCSI_T_TMF_QUERY_TASK_SET) {
@@ -286,10 +326,35 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s,
VirtIOSCSIReq *req)
req->resp.tmf.response = VIRTIO_SCSI_S_FUNCTION_SUCCEEDED;
break;
} else {
-scsi_req_cancel(r);
+/* Before we actually cancel any requests in the next for
+ * loop, let's count them. This way, if the bus starts
+ * calling back to the notifier even before we finish the
+ * loop, the counter, which value is already seen in
+ * virtio_scsi_cancel_notify, will prevent us from
+ * completing the tmf too quickly. */
+cancel_count++;
}
}
}
+if (cancel_count) {
+VirtIOSCSICancelNotifier *notifier;
+VirtIOSCSICancelTracker *tracker;
+
+tracker= g_slice_new(VirtIOSCSICancelTracker);
+tracker->tmf_req = req;
+tracker->remaining = cancel_count;
+
+QTAILQ_FOREACH_SAFE(r, &d->requests, next, next) {
+if (r->hba_private) {
+notifier = g_slice_new(VirtIOSCSICancelNotifier);
+notifier->notifier.notify = virtio_scsi_cancel_notify;
+notifier->tracker = tracker;
+scsi_req_cancel_async(r, ¬ifier->notifier);
+}
+}
+ret = -EINPROGRESS;
+}
+
break;
case VIRTIO_SCSI_T_TMF_I_T_NEXUS_RESET:
@@ -310,20 +375,22 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s,
VirtIOSCSIReq *req)
br
[Qemu-devel] [PATCH v4 3/7] scsi-bus: Unify request unref in scsi_req_cancel
Before, scsi_req_cancel will take ownership of the canceled request and unref
it. We did this because we didn't know whether AIO CB will be called or not
during the cancelling, so we set the io_canceled flag before calling it, and
skip unref in the potentially called callbacks, which is not very nice.
Now, bdrv_aio_cancel has a stricter contract that the completion callbacks are
always called, so we can remove the checks of req->io_canceled and just unref
it in callbacks.
It will also make implementing asynchronous cancellation easier.
Signed-off-by: Fam Zheng
---
hw/scsi/scsi-disk.c| 37 -
hw/scsi/scsi-generic.c | 13 ++---
2 files changed, 10 insertions(+), 40 deletions(-)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 9645d01..2e45752 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -113,11 +113,6 @@ static void scsi_cancel_io(SCSIRequest *req)
DPRINTF("Cancel tag=0x%x\n", req->tag);
if (r->req.aiocb) {
bdrv_aio_cancel(r->req.aiocb);
-
-/* This reference was left in by scsi_*_data. We take ownership of
- * it the moment scsi_req_cancel is called, independent of whether
- * bdrv_aio_cancel completes the request or not. */
-scsi_req_unref(&r->req);
}
r->req.aiocb = NULL;
}
@@ -197,9 +192,7 @@ static void scsi_aio_complete(void *opaque, int ret)
scsi_req_complete(&r->req, GOOD);
done:
-if (!r->req.io_canceled) {
-scsi_req_unref(&r->req);
-}
+scsi_req_unref(&r->req);
}
static bool scsi_is_cmd_fua(SCSICommand *cmd)
@@ -246,9 +239,7 @@ static void scsi_write_do_fua(SCSIDiskReq *r)
scsi_req_complete(&r->req, GOOD);
done:
-if (!r->req.io_canceled) {
-scsi_req_unref(&r->req);
-}
+scsi_req_unref(&r->req);
}
static void scsi_dma_complete_noio(void *opaque, int ret)
@@ -280,9 +271,7 @@ static void scsi_dma_complete_noio(void *opaque, int ret)
}
done:
-if (!r->req.io_canceled) {
-scsi_req_unref(&r->req);
-}
+scsi_req_unref(&r->req);
}
static void scsi_dma_complete(void *opaque, int ret)
@@ -320,9 +309,7 @@ static void scsi_read_complete(void * opaque, int ret)
scsi_req_data(&r->req, r->qiov.size);
done:
-if (!r->req.io_canceled) {
-scsi_req_unref(&r->req);
-}
+scsi_req_unref(&r->req);
}
/* Actually issue a read to the block device. */
@@ -363,9 +350,7 @@ static void scsi_do_read(void *opaque, int ret)
}
done:
-if (!r->req.io_canceled) {
-scsi_req_unref(&r->req);
-}
+scsi_req_unref(&r->req);
}
/* Read more data from scsi device into buffer. */
@@ -481,9 +466,7 @@ static void scsi_write_complete(void * opaque, int ret)
}
done:
-if (!r->req.io_canceled) {
-scsi_req_unref(&r->req);
-}
+scsi_req_unref(&r->req);
}
static void scsi_write_data(SCSIRequest *req)
@@ -1582,9 +1565,7 @@ static void scsi_unmap_complete(void *opaque, int ret)
scsi_req_complete(&r->req, GOOD);
done:
-if (!r->req.io_canceled) {
-scsi_req_unref(&r->req);
-}
+scsi_req_unref(&r->req);
g_free(data);
}
@@ -1678,9 +1659,7 @@ static void scsi_write_same_complete(void *opaque, int
ret)
scsi_req_complete(&r->req, GOOD);
done:
-if (!r->req.io_canceled) {
-scsi_req_unref(&r->req);
-}
+scsi_req_unref(&r->req);
qemu_vfree(data->iov.iov_base);
g_free(data);
}
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 2a73a43..e92b418 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -137,9 +137,7 @@ static void scsi_command_complete(void *opaque, int ret)
scsi_req_complete(&r->req, status);
done:
-if (!r->req.io_canceled) {
-scsi_req_unref(&r->req);
-}
+scsi_req_unref(&r->req);
}
/* Cancel a pending data transfer. */
@@ -150,11 +148,6 @@ static void scsi_cancel_io(SCSIRequest *req)
DPRINTF("Cancel tag=0x%x\n", req->tag);
if (r->req.aiocb) {
bdrv_aio_cancel(r->req.aiocb);
-
-/* This reference was left in by scsi_*_data. We take ownership of
- * it independent of whether bdrv_aio_cancel completes the request
- * or not. */
-scsi_req_unref(&r->req);
}
r->req.aiocb = NULL;
}
@@ -214,9 +207,7 @@ static void scsi_read_complete(void * opaque, int ret)
bdrv_set_guest_block_size(s->conf.bs, s->blocksize);
scsi_req_data(&r->req, len);
-if (!r->req.io_canceled) {
-scsi_req_unref(&r->req);
-}
+scsi_req_unref(&r->req);
}
}
--
1.9.3
Re: [Qemu-devel] [Xen-devel] [PATCH 2/2] xen:i386:pc_piix: create isa bridge specific to IGD passthrough
On 2014/9/3 9:40, Kay, Allen M wrote:
-Original Message-
From: Chen, Tiejun
Sent: Monday, September 01, 2014 12:50 AM
To: Michael S. Tsirkin
Cc: xen-de...@lists.xensource.com; Kay, Allen M; qemu-devel@nongnu.org;
Konrad Rzeszutek Wilk
Subject: Re: [Qemu-devel] [Xen-devel] [PATCH 2/2] xen:i386:pc_piix: create
isa bridge specific to IGD passthrough
On 2014/9/1 14:05, Michael S. Tsirkin wrote:
On Mon, Sep 01, 2014 at 10:50:37AM +0800, Chen, Tiejun wrote:
On 2014/8/31 16:58, Michael S. Tsirkin wrote:
On Fri, Aug 29, 2014 at 09:28:50AM +0800, Chen, Tiejun wrote:
On 2014/8/28 8:56, Chen, Tiejun wrote:
+ */
+dev = pci_create_simple(bus, PCI_DEVFN(0x1f, 0),
+"xen-igd-passthrough-isa-bridge");
+if (dev) {
+r = xen_host_pci_device_get(&hdev, 0, 0,
+ PCI_DEVFN(0x1f,
0), 0);
+if (!r) {
+pci_config_set_vendor_id(dev->config,
hdev.vendor_id);
+pci_config_set_device_id(dev->config,
+ hdev.device_id);
Can you, instead, implement the reverse logic, probing the card
and supplying the correct device id for PCH?
Here what is your so-called reverse logic as I already asked you
previously? Do you mean I should list all PCHs with a combo
illustrated with the vendor/device id in advance? Then look up
if we can find a
Michael,
Ping.
Thanks
Tiejun
Could you explain this exactly? Then I can try follow-up your
idea ASAP if this is necessary and possible.
Michel,
Could you give us some explanation for your "reverse logic" when
you're free?
Thanks
Tiejun
So future drivers will look at device ID for the card and figure out
how things should work from there.
Old drivers still poke at device id of the chipset for this, but
maybe qemu can do what newer drivers do:
look at the card and figure out what guest should do, then present
the appropriate chipset id.
This is based on what Jesse said:
Practically speaking, we could probably assume specific GPU/PCH
combos,
as I don't think they're generally mixed across generations, though
SNB
and IVB did have compatible sockets, so there is the possibility of
mixing CPT and PPT PCHs, but those are register identical as far as the
graphics driver is concerned, so even that should be safe.
Michael,
Thanks for your explanation.
so the idea is to have a reverse table mapping GPU back to PCH.
Present to guest the ID that will let it assume the correct GPU.
I guess you mean we should create to maintain such a table:
[GPU Card: device_id(s), PCH: device_id]
Then each time, instead of exposing that real PCH device id directly,
qemu first can get the real GPU device id to lookup this table to
present a appropriate PCH's device id to the guest.
And looks here that appropriate PCH's device id is not always a that
real PCH's device id. Right? If I'm wrong please correct me.
Exactly: we don't really care what the PCH ID is - we only need it for
the guest driver to do the right thing.
Agreed.
I need to ask Allen to check if one given GPU card device id is always
corresponding to one given PCH on both HSW and BDW currently. If yes, I can
do this quickly. Otherwise I need some time to establish that sort
of connection.
Michael,
Sorry for this delay response but please keep in mind we really are in
this process.
You know this involve different GPU components we have to take some time
to communicate or even discuss internal.
Now I have a draft codes, could you take a look at this? I hope that
comment can help us to understand something, but if you have any
question, we can further respond inline.
---
typedef struct {
uint16_t gpu_device_id;
uint16_t pch_device_id;
} XenIGDDeviceIDInfo;
/* In real world different GPU should have different PCH. But actually
* the different PCH DIDs likely map to different PCH SKUs. We do the
* same thing for the GPU. For PCH, the different SKUs are going to be
* all the same silicon design and implementation, just different
* features turn on and off with fuses. The SW interfaces should be
* consistent across all SKUs in a given family (eg LPT). But just same
* features may not be supported.
*
* Most of these different PCH features probably don't matter to the
* Gfx driver, but obviously any difference in display port connections
* will so it should be fine with any PCH in case of passthrough.
*
* So currently use one PCH version, 0x8c4e, to cover all HSW scenarios,
* 0x9cc3 for BDW.
*/
static const XenIGDDeviceIDInfo xen_igd_combo_id_infos[] = {
/* HSW Classic */
{0x0402, 0x8c4e}, /* HSWGT1D, HSWD_w7 */
{0x0406, 0x8c4e}, /* HSWGT1M, HSWM_w7 */
{0x0412, 0x8c4e}, /* HSWGT2D, HSWD_w7 */
{0x0416, 0x8c4e}, /* HSWGT2M, HSWM_w7 */
{0x041E, 0x8c4e}, /* HSWGT15D, HSWD_w7 */
/* HSW ULT */
{0x0A06, 0x8c4e}, /* HSWGT1UT, HSWM_w7 */
{0x0A16, 0x8c4e}, /* HSWGT2UT, HSWM_w7 */
{0x0A26, 0x8c4e}, /* HSWGT3UT, HSWM_w7 */
{0x0A2E, 0x8c4e}, /* HSWG
Re: [Qemu-devel] [PATCH 4/5] target-tricore: Add instructions of BIT opcode format
On 09/27/2014 07:58 AM, Bastian Koppelmann wrote:
> +/* D[c] = D[c][0] op1 (D[a][pos1] op2 D[b][pos2]);*/
> +static inline void gen_bit_2op(TCGv ret, TCGv r1, TCGv r2, TCGv r3,
> + int pos1, int pos2,
> + void(*op1)(TCGv, TCGv, TCGv),
> + void(*op2)(TCGv, TCGv, TCGv))
> +{
> +TCGv temp1, temp2, temp3;
> +
> +temp1 = tcg_temp_new();
> +temp2 = tcg_temp_new();
> +temp3 = tcg_temp_new();
> +
> +tcg_gen_andi_tl(temp3, r3, 0x1);
> +
> +tcg_gen_andi_tl(temp2, r2 , (0x1u << pos2));
> +tcg_gen_shri_tl(temp2, temp2, pos2);
> +
> +tcg_gen_andi_tl(temp1, r1, (0x1u << pos1));
> +tcg_gen_shri_tl(temp1, temp1, pos1);
> +
> +(*op1)(temp1, temp1, temp2);
> +(*op2)(ret , temp3, temp1);
This incorrectly clobbers bits 1:31 of ret. You want
shri tmp1, r2, pos2
shri tmp1, r1, pos1
op1(tmp1, tmp1, tmp2)
op2(tmp1, r3, tmp1)
deposit ret, ret, tmp1, 0, 1
> +TCGv temp1, temp2;
> +
> +temp1 = tcg_temp_new();
> +temp2 = tcg_temp_new();
> +
> +tcg_gen_andi_tl(temp2, r2, (0x1u << pos2));
> +tcg_gen_shri_tl(temp2, temp2, pos2);
> +
> +tcg_gen_andi_tl(temp1, r1, (0x1u << pos1));
> +tcg_gen_shri_tl(temp1, temp1, pos1);
> +
> +(*op1)(ret, temp1, temp2);
This one, though, does get to set the whole register. That said,
I think you should *not* mask the two inputs, but instead mask the one output.
That saves one operation, and allows NOR to not need a special case.
> +case OPC2_32_BIT_AND_NOR_T:
> +gen_bit_2op(temp, cpu_gpr_d[r1], cpu_gpr_d[r2], cpu_gpr_d[r3],
> +pos1, pos2, &tcg_gen_or_tl, &tcg_gen_andc_tl);
> +break;
Without trying to take into account the properties of the tcg backend, this
seems less than ideal. Yes, it's correct, but so is
tcg_gen_nor_tl, tcg_gen_and_tl
which matches the name of the instruction.
If the tcg backend is sparc or ppc, it's more efficient too, since nor is
actually present in the isa. But of course, arm and even haswell x86 have andc
but don't have nor. This is stuff that a normal compiler optimizer could sort
out, but we don't have that for tcg.
I'd be willing to accept something conditionalized on TCG_TARGET_HAS_nor_i32,
if you like, but otherwise just match the instruction.
> +static void decode_bit_insert(CPUTriCoreState *env, DisasContext *ctx)
It's probably better to implement this with one right-shift and one deposit.
Certainly would be easier to read and follow.
> +case OPC2_32_BIT_XNOR_T:
> +gen_bit_1op(cpu_gpr_d[r3], cpu_gpr_d[r1], cpu_gpr_d[r2],
> +pos1, pos2, &tcg_gen_xor_tl);
tcg_gen_eqv_tl
> +case OPC2_32_BIT_OR_NOR_T:
> +gen_bit_2op(temp, cpu_gpr_d[r1], cpu_gpr_d[r2], cpu_gpr_d[r3],
> +pos1, pos2, &tcg_gen_or_tl, &tcg_gen_orc_tl);
> +break;
Again, probably better with nor + or, or conditionalization.
> +case OPC2_32_BIT_SH_XNOR_T:
> +gen_bit_1op(temp, cpu_gpr_d[r1], cpu_gpr_d[r2],
> +pos1, pos2, &tcg_gen_xor_tl);
> +tcg_gen_not_tl(temp, temp);
Again, eqv.
r~
Re: [Qemu-devel] [Xen-devel] [PATCH 2/2] xen:i386:pc_piix: create isa bridge specific to IGD passthrough
On 2014/9/28 10:59, Chen, Tiejun wrote:
On 2014/9/3 9:40, Kay, Allen M wrote:
-Original Message-
From: Chen, Tiejun
Sent: Monday, September 01, 2014 12:50 AM
To: Michael S. Tsirkin
Cc: xen-de...@lists.xensource.com; Kay, Allen M; qemu-devel@nongnu.org;
Konrad Rzeszutek Wilk
Subject: Re: [Qemu-devel] [Xen-devel] [PATCH 2/2] xen:i386:pc_piix:
create
isa bridge specific to IGD passthrough
On 2014/9/1 14:05, Michael S. Tsirkin wrote:
On Mon, Sep 01, 2014 at 10:50:37AM +0800, Chen, Tiejun wrote:
On 2014/8/31 16:58, Michael S. Tsirkin wrote:
On Fri, Aug 29, 2014 at 09:28:50AM +0800, Chen, Tiejun wrote:
On 2014/8/28 8:56, Chen, Tiejun wrote:
+ */
+dev = pci_create_simple(bus, PCI_DEVFN(0x1f, 0),
+
"xen-igd-passthrough-isa-bridge");
+if (dev) {
+r = xen_host_pci_device_get(&hdev, 0, 0,
+ PCI_DEVFN(0x1f,
0), 0);
+if (!r) {
+pci_config_set_vendor_id(dev->config,
hdev.vendor_id);
+pci_config_set_device_id(dev->config,
+ hdev.device_id);
Can you, instead, implement the reverse logic, probing the card
and supplying the correct device id for PCH?
Here what is your so-called reverse logic as I already asked you
previously? Do you mean I should list all PCHs with a combo
illustrated with the vendor/device id in advance? Then look up
if we can find a
Michael,
Ping.
Thanks
Tiejun
Could you explain this exactly? Then I can try follow-up your
idea ASAP if this is necessary and possible.
Michel,
Could you give us some explanation for your "reverse logic" when
you're free?
Thanks
Tiejun
So future drivers will look at device ID for the card and figure out
how things should work from there.
Old drivers still poke at device id of the chipset for this, but
maybe qemu can do what newer drivers do:
look at the card and figure out what guest should do, then present
the appropriate chipset id.
This is based on what Jesse said:
Practically speaking, we could probably assume specific GPU/PCH
combos,
as I don't think they're generally mixed across generations,
though
SNB
and IVB did have compatible sockets, so there is the
possibility of
mixing CPT and PPT PCHs, but those are register identical as
far as the
graphics driver is concerned, so even that should be safe.
Michael,
Thanks for your explanation.
so the idea is to have a reverse table mapping GPU back to PCH.
Present to guest the ID that will let it assume the correct GPU.
I guess you mean we should create to maintain such a table:
[GPU Card: device_id(s), PCH: device_id]
Then each time, instead of exposing that real PCH device id directly,
qemu first can get the real GPU device id to lookup this table to
present a appropriate PCH's device id to the guest.
And looks here that appropriate PCH's device id is not always a that
real PCH's device id. Right? If I'm wrong please correct me.
Exactly: we don't really care what the PCH ID is - we only need it for
the guest driver to do the right thing.
Agreed.
I need to ask Allen to check if one given GPU card device id is always
corresponding to one given PCH on both HSW and BDW currently. If yes,
I can
do this quickly. Otherwise I need some time to establish that sort
of connection.
Michael,
Sorry for this delay response but please keep in mind we really are in
this process.
You know this involve different GPU components we have to take some time
to communicate or even discuss internal.
Now I have a draft codes, could you take a look at this? I hope that
comment can help us to understand something, but if you have any
question, we can further respond inline.
---
typedef struct {
uint16_t gpu_device_id;
uint16_t pch_device_id;
} XenIGDDeviceIDInfo;
/* In real world different GPU should have different PCH. But actually
* the different PCH DIDs likely map to different PCH SKUs. We do the
* same thing for the GPU. For PCH, the different SKUs are going to be
* all the same silicon design and implementation, just different
* features turn on and off with fuses. The SW interfaces should be
* consistent across all SKUs in a given family (eg LPT). But just same
* features may not be supported.
*
* Most of these different PCH features probably don't matter to the
* Gfx driver, but obviously any difference in display port connections
* will so it should be fine with any PCH in case of passthrough.
*
* So currently use one PCH version, 0x8c4e, to cover all HSW scenarios,
* 0x9cc3 for BDW.
*/
static const XenIGDDeviceIDInfo xen_igd_combo_id_infos[] = {
/* HSW Classic */
{0x0402, 0x8c4e}, /* HSWGT1D, HSWD_w7 */
{0x0406, 0x8c4e}, /* HSWGT1M, HSWM_w7 */
{0x0412, 0x8c4e}, /* HSWGT2D, HSWD_w7 */
{0x0416, 0x8c4e}, /* HSWGT2M, HSWM_w7 */
{0x041E, 0x8c4e}, /* HSWGT15D, HSWD_w7 */
/* HSW ULT */
{0x0A06, 0x8c4e}, /* HSWGT1UT, HSWM_w7 */
{0x0A16, 0x8c4e}, /* HSWGT2UT, HSWM_w7 */
{0x0A26, 0x8c4e}, /* HSWGT3UT, HSWM_w7 */
{0x0A2E, 0x
Re: [Qemu-devel] [PATCH v2] vl: Adjust the place of calling mlockall to speedup VM's startup
On 2014/9/26 23:35, Paolo Bonzini wrote:
Il 26/09/2014 10:35, zhanghailiang ha scritto:
Hi,
This has been reviewed, Will anyone pick this up?;)
I think mst will. It's Jewish new year this week, so it will have to
wait for next week.
OK, Thanks;)
Best Regard,
zhanghailiang
Paolo
Thanks,
zhanghailiang
On 2014/9/23 18:42, zhanghailiang wrote:
If we configure mlock=on and memory policy=bind at the same time,
It will consume lots of time for system to treat with memory,
especially when call mbind behind mlockall.
Adjust the place of calling mlockall, calling mbind before mlockall
can remarkably reduce the time of VM's startup.
Acked-by: Michael S. Tsirkin
Signed-off-by: zhanghailiang
---
v2:
- Add Acked-by
- change 'int' to 'bool' (Thanks Hu Tao)
---
vl.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/vl.c b/vl.c
index dc792fe..35e5de6 100644
--- a/vl.c
+++ b/vl.c
@@ -134,6 +134,7 @@ const char* keyboard_layout = NULL;
ram_addr_t ram_size;
const char *mem_path = NULL;
int mem_prealloc = 0; /* force preallocation of physical target
memory */
+bool enable_mlock = false;
int nb_nics;
NICInfo nd_table[MAX_NICS];
int autostart;
@@ -1421,12 +1422,8 @@ static void smp_parse(QemuOpts *opts)
}
-static void configure_realtime(QemuOpts *opts)
+static void realtime_init(void)
{
-bool enable_mlock;
-
-enable_mlock = qemu_opt_get_bool(opts, "mlock", true);
-
if (enable_mlock) {
if (os_mlock() < 0) {
fprintf(stderr, "qemu: locking memory failed\n");
@@ -3973,7 +3970,7 @@ int main(int argc, char **argv, char **envp)
if (!opts) {
exit(1);
}
-configure_realtime(opts);
+enable_mlock = qemu_opt_get_bool(opts, "mlock", true);
break;
case QEMU_OPTION_msg:
opts = qemu_opts_parse(qemu_find_opts("msg"),
optarg, 0);
@@ -4441,6 +4438,8 @@ int main(int argc, char **argv, char **envp)
machine_class->init(current_machine);
+realtime_init();
+
audio_init();
cpu_synchronize_all_post_init();
.
Re: [Qemu-devel] [PATCH v2] qga: Rewrite code where using readdir_r
On 2014/9/26 23:40, Paolo Bonzini wrote:
Il 19/09/2014 05:09, zhanghailiang ha scritto:
If readdir_r fails, error_setg_errno will reference the freed
pointer *dirpath*.
Moreover, readdir_r may cause a buffer overflow, using readdir instead.
Signed-off-by: zhanghailiang
---
v2:
- Switch readdir_r to readdir (Comment of Eric Blake)
---
qga/commands-posix.c | 27 +++
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 7eed7f4..f6f3e3c 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -956,7 +956,7 @@ static void build_guest_fsinfo_for_virtual_device(char
const *syspath,
{
DIR *dir;
char *dirpath;
-struct dirent entry, *result;
+struct dirent *entry;
dirpath = g_strdup_printf("%s/slaves", syspath);
dir = opendir(dirpath);
@@ -965,22 +965,24 @@ static void build_guest_fsinfo_for_virtual_device(char
const *syspath,
g_free(dirpath);
return;
}
-g_free(dirpath);
for (;;) {
-if (readdir_r(dir, &entry, &result) != 0) {
-error_setg_errno(errp, errno, "readdir_r(\"%s\")", dirpath);
-break;
-}
-if (!result) {
+errno = 0;
+entry = readdir(dir);
+if (entry == NULL) {
+if (errno) {
+error_setg_errno(errp, errno, "readdir(\"%s\")", dirpath);
+}
break;
}
-if (entry.d_type == DT_LNK) {
-g_debug(" slave device '%s'", entry.d_name);
-dirpath = g_strdup_printf("%s/slaves/%s", syspath, entry.d_name);
-build_guest_fsinfo_for_device(dirpath, fs, errp);
-g_free(dirpath);
+if (entry->d_type == DT_LNK) {
+char *path;
+
+g_debug(" slave device '%s'", entry->d_name);
+path = g_strdup_printf("%s/slaves/%s", syspath, entry->d_name);
+build_guest_fsinfo_for_device(path, fs, errp);
+g_free(path);
if (*errp) {
break;
@@ -988,6 +990,7 @@ static void build_guest_fsinfo_for_virtual_device(char
const *syspath,
}
}
+g_free(dirpath);
closedir(dir);
}
Thanks,
Reviewed-by: Paolo Bonzini
Michael Roth will pick this up.
OK, Thanks!
Paolo
.
Re: [Qemu-devel] [PATCH 5/5] target-tricore: Add instructions of BO opcode format
On 09/27/2014 07:58 AM, Bastian Koppelmann wrote:
> +/* ld circ */
> +DEF_HELPER_4(ld_b_circ, void, env, i32, i32, int)
Avoid, whenever possible, performing memory operations within helpers. Doing
that complicates the generation of correct code for exact exceptions from the
memory operation. I'm not saying it can't be done, but what you're doing right
now certainly isn't correct, and it's easier to do the memory operation with
tcg ops.
So now we need to find an efficient way to do this. In both cases, I think one
helper each for bit-reverse and circular modes should suffice.
E.g.
tcg_gen_ext16u_tl(temp, A[b+1])
tcg_gen_add_tl(ea, A[b], temp)
tcg_gen_qemu_ld_tl(ret, ea, mmu_idx, MO_UB)
gen_helper_br_update(A[b+1], A[b+1])
and then
uint32_t helper_br_update(uint32_t reg)
{
uint32_t index = reg & 0x;
uint32_t incr = reg >> 16;
uint32_t new_index = reverse16(reverse16(index) + reverse16(incr));
return reg - index + new_index;
}
uint32_t helper_circ_update(uint32_t reg, uint32_t off)
{
uint32_t index = reg & 0x;
uint32_t length = reg >> 16;
int32_t new_index = index + off;
if (new_index < 0) {
new_index += length;
} else {
new_index %= length;
}
return reg - index + new_index;
}
> +void helper_ld_w_circ(CPUTriCoreState *env, uint32_t r1, uint32_t r2, int
> off10)
> +{
> +CIRC_BR_DEFINES(r2)
> +
> +uint32_t ea0 = env->gpr_a[r2] + index;
> +uint32_t ea2 = env->gpr_a[r2] + (index + 2 % length_incr);
> +
> +uint32_t hw_ea2 = cpu_lduw_data(env, ea2) << 16;
> +uint32_t lw_ea0 = cpu_lduw_data(env, ea0);
> +
> +env->gpr_d[r1] = hw_ea2 | lw_ea0;
> +
> +CIRC_CALC_INDEX(r2, off10, length_incr);
> +}
I know that the volume 2 pseudo code for LD.W says two halfword loads, but the
volume 1 text, section 2.5.5 says that the buffer end must be aligned to the
size of the access. As I read it, two memory operations should not be required.
r~
Re: [Qemu-devel] [PATCH 1/5] target-tricore: Cleanup and Bugfixes
On 09/27/2014 07:58 AM, Bastian Koppelmann wrote: > Move FCX loading of save_context_ to caller functions, for STLCX, STUCX insn > to use those functions. > Move FCX storing of restore_context_ to caller functions, for LDLCX, LDUCX > insn to use those functions. > Remove do_raise_exception function, which caused clang to emit a warning. > Fix: save_context_lower now saves a[11] instead of PSW. > Fix: MASK_OP_ABSB_BPOS starting at wrong offset. > > Signed-off-by: Bastian Koppelmann Reviewed-by: Richard Henderson r~
