Re: [Qemu-devel] [for-2.6 PATCH 1/3] target-i386: Define structs for layout of xsave area
On 11/30/2015 03:18 AM, Paolo Bonzini wrote: Because this is always little endian, I would write it as uint8_t[16][16]. Maybe. That isn't altogether handy for TCG, since we'll be wanting to bswap these buffers (probably in uint64_t chunks). r~ -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [for-2.6 PATCH 1/3] target-i386: Define structs for layout of xsave area
On 12/01/2015 09:15 AM, Eduardo Habkost wrote: On Tue, Dec 01, 2015 at 09:09:47AM -0800, Richard Henderson wrote: On 11/30/2015 03:18 AM, Paolo Bonzini wrote: Because this is always little endian, I would write it as uint8_t[16][16]. Maybe. That isn't altogether handy for TCG, since we'll be wanting to bswap these buffers (probably in uint64_t chunks). X86XSaveArea will be used only when loading/saving state using xsave, not for executing regular instructions. ... like the regular instruction xsave? https://patchwork.ozlabs.org/patch/493318/ In X86CPU, the data is already stored as XMMReg unions (the one with the XMM_[BWDQ] helpers). Of course. But those unions are arranged to be in big-endian format on big-endian hosts. So we need to swap the data back to little-endian format for storage into guest memory. r~ -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [for-2.6 PATCH 1/3] target-i386: Define structs for layout of xsave area
On 12/01/2015 10:34 AM, Eduardo Habkost wrote:
BTW, if we are going to implement xsave in TCG, the
X86CPU<->xsave translation logic in kvm_{get,put}_xsave() could
be moved to generic code and reused by TCG instead of being
reimplemented.
That's not trivial.
In particular, stq_p isn't what the tcg helper needs to use, but rather
cpu_stq_data_ra. Given the differing parameters, we'd have to resort to some
sort of macro-ization. It's probably easiest to simply keep the two
implementations separate.
r~
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] target-i386: tcg: Accept clwb instruction
On 11/04/2015 10:24 PM, Eduardo Habkost wrote:
Accept the clwb instruction (66 0F AE /6) if its corresponding feature
flag is enabled on CPUID[7].
Signed-off-by: Eduardo Habkost <ehabk...@redhat.com>
---
target-i386/translate.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/target-i386/translate.c b/target-i386/translate.c
index b400d24..bac1685 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7716,10 +7716,21 @@ static target_ulong disas_insn(CPUX86State *env,
DisasContext *s,
}
break;
case 5: /* lfence */
-case 6: /* mfence */
if ((modrm & 0xc7) != 0xc0 || !(s->cpuid_features & CPUID_SSE2))
goto illegal_op;
break;
+case 6: /* mfence/clwb */
+if (s->prefix & PREFIX_DATA) {
+/* clwb */
+if (!(s->cpuid_7_0_ebx_features & CPUID_7_0_EBX_CLWB))
+goto illegal_op;
+gen_lea_modrm(env, s, modrm);
You should use gen_nop_modrm here, since we're not going to do anything with
the address. Otherwise,
Reviewed-by: Richard Henderson <r...@twiddle.net>
r~
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] target-i386: tcg: Check right CPUID bits for clflushopt/pcommit
On 11/04/2015 10:24 PM, Eduardo Habkost wrote: Detect the clflushopt and pcommit instructions and check their corresponding feature flags, instead of checking CPUID_SSE and CPUID_CLFLUSH. Signed-off-by: Eduardo Habkost <ehabk...@redhat.com> --- target-i386/translate.c | 28 1 file changed, 20 insertions(+), 8 deletions(-) Reviewed-by: Richard Henderson <r...@twiddle.net> r~ -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] target-i386: tcg: Accept clwb instruction
On 11/06/2015 02:59 PM, Eduardo Habkost wrote: Thanks! BTW, clflush uses gen_lea_modrm() too, does it do anything with the address somewhere else? Nope. It probably pre-dates the introduction of gen_nop_modrm. r~ -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH] target-i386: enable cflushopt/clwb/pcommit instructions
On 11/04/2015 08:35 PM, Eduardo Habkost wrote: On Fri, Oct 30, 2015 at 01:54:33PM -0700, Richard Henderson wrote: On 10/29/2015 12:31 AM, Xiao Guangrong wrote: These instructions are used by NVDIMM drivers and the specification locates at: https://software.intel.com/sites/default/files/managed/0d/53/319433-022.pdf There instructions are available on Skylake Server Signed-off-by: Xiao Guangrong <guangrong.x...@linux.intel.com> --- target-i386/cpu.c | 8 +--- target-i386/cpu.h | 3 +++ 2 files changed, 8 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson <r...@twiddle.net> Although it would be nice to update the comments in translate.c to include the new insns, since they overlap mfence and sfence. At present we only check for SSE enabled when accepting these; I suppose it's easiest to consider it invalid to specify +clwb,-sse? I assume you want to add the extra SSE requirement to TCG code, not to generic x86 code, then I have no objections. I don't really want to add any requirement, just point and laugh at anyone who reports an bug for the above condition. But in the case of clwb (/6 with a memory operand, modrm != 0xc0), we are not just requiring SSE2: we are rejecting the instruction unless modrm == 0xc0. That means TCG is rejecting the clwb instruction, so I believe we shouldn't add CLWB to TCG_7_0_EBX_FEATURES yet. Hmm, yes. I've cleaned up some of this code on a branch, but it didn't get enough testing or review this cycle, so it's going to wait for the next. I see you've posted a patch for this, which should be good enough until then. r~ -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH] target-i386: enable cflushopt/clwb/pcommit instructions
On 10/29/2015 12:31 AM, Xiao Guangrong wrote: > These instructions are used by NVDIMM drivers and the specification > locates at: > https://software.intel.com/sites/default/files/managed/0d/53/319433-022.pdf > > There instructions are available on Skylake Server > > Signed-off-by: Xiao Guangrong <guangrong.x...@linux.intel.com> > --- > target-i386/cpu.c | 8 +--- > target-i386/cpu.h | 3 +++ > 2 files changed, 8 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson <r...@twiddle.net> Although it would be nice to update the comments in translate.c to include the new insns, since they overlap mfence and sfence. At present we only check for SSE enabled when accepting these; I suppose it's easiest to consider it invalid to specify +clwb,-sse? r~ -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 10/15] cpu-model/s390: Add cpu class initialization routines
On 02/17/2015 06:24 AM, Michael Mueller wrote:
+/**
+ * s390_test_facility - test if given facility bit is set facility list
+ * of given cpu class
+ * @class: address of cpu class to test
+ * @nr: bit number to test
+ *
+ * Returns: true in case it is set
+ * false in case it is not set
+ */
+bool s390_test_facility(S390CPUClass *cc, unsigned long nr)
+{
+if (cc) {
+return test_facility(nr, cc-fac_list) ? true : false;
+}
+return false;
+}
Where do you see this being used?
r~
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 10/15] cpu-model/s390: Add cpu class initialization routines
On 02/17/2015 06:24 AM, Michael Mueller wrote:
+static inline uint64_t big_endian_bit(unsigned long nr)
+{
+return 1ul (BITS_PER_LONG - (nr % BITS_PER_LONG));
+};
This is buggy. NR=0 should map to 63, not 64.
+return !!(*ptr big_endian_bit(nr));
Personally I dislike !! as an idiom. Given that big_endian_bit isn't used
anywhere else, can we integrate it and change this to
static inline int test_facility(unsigned long nr, uint64_t *fac_list)
{
unsigned long word = nr / BITS_PER_LONG;
unsigned long be_bit = 63 - (nr % BITS_PER_LONG);
return (fac_list[word] be_bit) 1;
}
r~
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BUG: commit 50a2c6e breaks KVM/ARM (reset/init vcpu order)
On 05/26/2014 03:20 AM, Andreas Färber wrote:
Alpha is the main blocker for unifying CPU reset iirc. It does not
implement reset at all and thus is not calling it. The struct was not
designed for zero'ing things, so there's a mix of data fields and
pointers without clear separation to allow memset(), and I have neither
a working alpha test image nor the time to investigate this at the moment.
WIP here:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu-alpha
https://github.com/afaerber/qemu-cpu/commits/qom-cpu-reset
Doesn't compile anymore. I can fix that up with the attached, but we can't
actually test this without changes to the rom to actually support reset. At
the moment, the rom will tell qemu to poweroff whether the kernel signals for
poweroff or reset.
If this is good enough to unblock you in other qom cleanups, please go ahead.
One of these days I'll get around to filling out more complete roms...
r~
diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
index 6ab31a1..cbad6fa 100644
--- a/target-alpha/cpu.c
+++ b/target-alpha/cpu.c
@@ -49,19 +49,30 @@ static bool alpha_cpu_has_work(CPUState *cs)
/* CPUClass::reset() */
static void alpha_cpu_reset(CPUState *s)
{
+#ifdef CONFIG_SOFTMMU
AlphaCPU *cpu = ALPHA_CPU(s);
AlphaCPUClass *acc = ALPHA_CPU_GET_CLASS(cpu);
CPUAlphaState *env = cpu-env;
+uint64_t palbr;
if (qemu_loglevel_mask(CPU_LOG_RESET)) {
qemu_log(CPU Reset (CPU %d)\n, s-cpu_index);
-log_cpu_state(env, 0);
+log_cpu_state(s, 0);
}
acc-parent_reset(s);
-memset(env, 0, offsetof(CPUAlphaState, breakpoints));
-tlb_flush(env, 1);
+palbr = env-palbr;
+
+memset(env, 0, offsetof(CPUAlphaState, error_code));
+tlb_flush(s, 1);
+
+/* Reset vector goes to palbr + 0. */
+env-palbr = palbr;
+env-pc = palbr;
+#else
+abort();
+#endif
}
static void alpha_cpu_realizefn(DeviceState *dev, Error **errp)
Re: [Qemu-devel] [PATCH for-1.7] target-i386: Fix build by providing stub kvm_arch_get_supported_cpuid()
On 11/13/2013 03:04 AM, Anthony Liguori wrote: On Tue, Nov 12, 2013 at 8:08 AM, Peter Maydell peter.mayd...@linaro.org wrote: On 12 November 2013 15:58, Paolo Bonzini pbonz...@redhat.com wrote: I don't really see a reason why QEMU should give clang more weight than Windows or Mac OS X. I'm not asking for more weight (and actually my main reason for caring about clang is exactly MacOSX). I'm just asking that when a bug is reported whose underlying cause is we don't work on clang because we're relying on undocumented behaviour of gcc with an attached patch that fixes this by not relying on the undocumented behaviour, that we apply the patch rather than saying why do we care about clang... QEMU has always been intimately tied to GCC. Heck, it all started as a giant GCC hack relying on entirely undocumented behavior (dyngen's disassembly of functions). There's nothing intrinsically bad about being tied to GCC. If you were making argument that we could do it a different way and the result would be as nice or nicer, then it wouldn't be a discussion. But if supporting clang means we have to remove useful things, then it's always going to be an uphill battle. In this case, the whole discussion is a bit silly. Have you actually For what it's worth, I think BOTH of the patches that have been posted should be applied. That is, the patch that does (X || 1) - (1 || X), and the patch that adds the stub. Frankly I'd have thought this was obvious and I'm a bit dismayed about how long this thread has continued. As far as GCC is concerned, we consider trivial dead code elimination like this to be a quality of implementation issue. We would never remove it, even from -O0. We can't guarantee how successful we can be, but that's what bug reports and regression tests are for. r~ -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH for-1.7] target-i386: Fix build by providing stub kvm_arch_get_supported_cpuid()
On 11/13/2013 08:53 AM, Paolo Bonzini wrote: Il 12/11/2013 19:54, Richard Henderson ha scritto: For what it's worth, I think BOTH of the patches that have been posted should be applied. That is, the patch that does (X || 1) - (1 || X), and the patch that adds the stub. Frankly I'd have thought this was obvious It's not that obvious to me. If you add the stub, the patch that reorders operands is not necessary. If you reorder operands, the stub is not necessary. The patch that does (X || 1) - (1 || X) is unnecessary as a microoptimization, since this code basically runs once at startup. The code is also a little bit less clear with the reordered operands, but perhaps that's just me because I wrote the code that way. (Splitting the if in two would also make sense, and would not affect clarity). Why should both be applied? It's worth working around the clang missed optimization, if for nothing else than avoiding the noise of the bugs that would otherwise be filed against the release. I think it's also worthwhile to implement the kvm api in kvm-stub.c, unnecessary or not. If you really want compile-time feedback on those that ought to have been removed by optimization, you could elide them from the stub file depending on ifndef __OPTIMIZE__. r~ -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 8/8] s390: Add new channel I/O based virtio transport.
$SUBJECT reminds me that the tcg s390x port is now prepared to implement channel insns, but previously there were no channels worth talking about: /* I/O Instructions. For each we simply indicate non-operation. */ C(0xb276, XSCH,S, Z, 0, 0, 0, 0, subchannel, 0) C(0xb230, CSCH,S, Z, 0, 0, 0, 0, subchannel, 0) C(0xb231, HSCH,S, Z, 0, 0, 0, 0, subchannel, 0) C(0xb232, MSCH,S, Z, 0, 0, 0, 0, subchannel, 0) C(0xb23b, RCHP,S, Z, 0, 0, 0, 0, subchannel, 0) C(0xb238, RSCH,S, Z, 0, 0, 0, 0, subchannel, 0) C(0xb233, SSCH,S, Z, 0, 0, 0, 0, subchannel, 0) C(0xb234, STSCH, S, Z, 0, 0, 0, 0, subchannel, 0) C(0xb235, TSCH,S, Z, 0, 0, 0, 0, subchannel, 0) With a bit of help, I can make these emulated insns implement virtio as well. r~ -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [BUG] need to export variables in config.mak
On 09/30/2011 10:07 AM, Chris Friesen wrote: We've been playing a bit with kvm-kmod-3.0b. We use a cross compile environment, and one of my coworkers noticed that the variables in config.mak weren't actually exported and so didn't actually have any effect. Please elaborate on no effect, and what actual problems you are experiencing. I do cross-compilation builds all the time and have no such problems. r~ -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 21/24] isa: add isa_address_space()
On 08/08/2011 10:07 AM, Avi Kivity wrote:
A helper that returns the address space used by ISA devices. Useful
for getting rid of isa_mem_base, multiple ISA buses, or ISA buses behind
bridges.
Signed-off-by: Avi Kivity a...@redhat.com
---
hw/isa-bus.c |6 ++
hw/isa.h |1 +
2 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 2765543..1cb497f 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -20,6 +20,7 @@
#include monitor.h
#include sysbus.h
#include isa.h
+#include exec-memory.h
struct ISABus {
BusState qbus;
@@ -202,4 +203,9 @@ static char *isabus_get_fw_dev_path(DeviceState *dev)
return strdup(path);
}
+MemoryRegion *isa_address_space(ISADevice *dev)
+{
+return get_system_memory();
+}
+
This does not help get rid of isa_mem_base, as far as I can see.
All this is going to do is the equivalent of isa_mem_base == 0.
Did you have a plan beyond this?
The simplest replacement, as far as I can see, is to replace one
global variable with another:
MemoryRegion *isa_address_space;
Define that this variable *must* be set by any platform that
wants to use ISA. It can be set to the address_space_mem
parameter (i.e. the copy of get_system_memory() that the
platforms receive as a parameter).
For Alpha, I can set this to the sub-region that I pass off to
the PCI subsystem.
For those other non-x86 platforms that currently set isa_mem_base
to a non-zero value, they either re-use an otherwise existing
memory region or create a new sub-region properly placed.
Of course, as far as I can see, this variable is only used by
the VGA devices. Surely we can arrange to pass down some address
space during setup of the VGA?
r~
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 21/24] isa: add isa_address_space()
On 08/10/2011 09:24 AM, Richard Henderson wrote: Of course, as far as I can see, this variable is only used by the VGA devices. Surely we can arrange to pass down some address space during setup of the VGA? ... Which seems to be what you've done in patch 23. So what's the point of this patch? r~ -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] memory: use signed arithmetic
On 08/03/2011 01:56 AM, Avi Kivity wrote: When trying to map an alias of a ram region, where the alias starts at address A and we map it into address B, and A B, we had an arithmetic underflow. Because we use unsigned arithmetic, the underflow converted into a large number which failed addrrange_intersects() tests. The concrete example which triggered this was cirrus vga mapping the framebuffer at offsets 0xc-0xc7fff (relative to the start of the framebuffer) into offsets 0xa (relative to system addres space start). With our favorite analogy of a windowing system, this is equivalent to dragging a subwindow off the left edge of the screen, and failing to clip it into its parent window which is on screen. Fix by switching to signed arithmetic. Signed-off-by: Avi Kivity a...@redhat.com Signed-off-by: Richard Henderson r...@twiddle.net r~ -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH] memory: use signed arithmetic
On 08/02/2011 01:50 PM, Avi Kivity wrote:
struct AddrRange {
-uint64_t start;
-uint64_t size;
+int64_t start;
+int64_t size;
I'm must say I'm not keen on this. My primary objection is that
a range can no longer properly represent the entire address space.
Or, indeed, anything in the second half of it.
It sounds like your problem would be better solved by re-arranging
things such that you perform X Y comparisons rather than DELTA 0.
r~
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH] memory: use signed arithmetic
On 08/02/2011 03:06 PM, Avi Kivity wrote: I don't think there's any cpu which has a real 64-bit physical address space? Don't they all truncate it? I don't know. You're right that x86_64 does, at 48 bits. The alpha system I'm trying to emulate does, at 50 bits. I guess if IBM agrees wrt p-series and z-series emulation, then I'd be ok, so long as you add a comment above that structure that says no existing hw implementation actually uses 63 address bits. r~ -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 00/39] Memory API, batch 2: PCI devices
On 07/31/2011 10:57 AM, Avi Kivity wrote: This is a mostly mindless conversion of all QEMU PCI devices to the memory API. After this patchset is applied, it is no longer possible to create a PCI device using the old API. An immediate benefit is that PCI BARs that overlap each other are now handled correctly: currently, the sequence map BAR 0 map BAR 1 at an overlapping address unmap either BAR 0 or BAR 1 will leave a hole where the overlap exists. With the patchset, the memory map is restored correctly. Note that overlaps of PCI BARs with memory or non-PCI resources are still not resolved correctly; this will be fixed later on. The vga patches have ugly intermediate states; however the result is fairly clean. Also available from: git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory-region-b2 1-39 Reviewed-by: Richard Henderson r...@twiddle.net Nice cleanups. r~ -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 18/39] ide: convert to memory API
On 07/31/2011 10:57 AM, Avi Kivity wrote: +pci_register_bar_region(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, +d-cmd646_bar[2].cmd); Typo: cmd646_bar[1]. r~ -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 09/39] Integrate I/O memory regions into qemu
On 07/31/2011 10:57 AM, Avi Kivity wrote: +system_io = qemu_malloc(sizeof(*system_io)); +memory_region_init(system_memory, io, 65536); +set_system_io_map(system_io); Cut-paste error on that second line. r~ -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v2 04/23] memory: merge adjacent segments of a single memory region
On 07/26/2011 04:36 AM, Paolo Bonzini wrote:
On 07/26/2011 01:26 PM, Avi Kivity wrote:
+while (i view-nr) {
+j = i + 1;
+while (j view-nr
+can_merge(view-ranges[j-1], view-ranges[j])) {
+view-ranges[i].addr.size += view-ranges[j].addr.size;
+++j;
+}
+++i;
if (j != i) {
+memmove(view-ranges[i], view-ranges[j],
+(view-nr - j) * sizeof(view-ranges[j]));
+view-nr -= j - i;
+}
}
Err, j = i + 1, and increments. Your conditional will always be true.
r~
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v2] Register Linux dyntick timer as per-thread signal
On 06/17/2011 02:25 AM, Jan Kiszka wrote: Derived from kvm-tool patch http://thread.gmane.org/gmane.comp.emulators.kvm.devel/74309 Ingo Molnar pointed out that sending the timer signal to the whole process, just blocking it everywhere, is suboptimal with an increasing number of threads. QEMU is also using this pattern so far. Linux provides a (non-portable) way to restrict the signal to a single thread: We can use SIGEV_THREAD_ID unless we are forced to emulate signalfd via an additional thread. That case could theoretically be optimized as well, but it doesn't look worth bothering. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Reviewed-by: Richard Henderson r...@twiddle.net r~ -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [RFC][PATCH] Register Linux dyntick timer as per-thread signal
On 06/16/2011 02:31 AM, Jan Kiszka wrote:
ev.sigev_value.sival_int = 0;
-ev.sigev_notify = SIGEV_SIGNAL;
ev.sigev_signo = SIGALRM;
+#ifdef SIGEV_THREAD_ID
+if (qemu_signalfd_available()) {
+ev.sigev_notify = SIGEV_THREAD_ID;
+ev._sigev_un._tid = qemu_get_thread_id();
+} else
+#endif /* SIGEV_THREAD_ID */
+{
+ev.sigev_notify = SIGEV_SIGNAL;
+}
Rather than do the else-inside-ifdef thing, why not
leave the original setting of sigev_notify where it
was, and let the ifdef overwrite it?
r~
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/10] linux-user: remove unused variables
On 06/15/2011 01:35 AM, Alexander Graf wrote:
-abi_ulong arg5 = 0, arg6 = 0, arg7 = 0, arg8 = 0;
+abi_ulong arg5 = 0, arg6 = 0;
nb_args = mips_syscall_args[syscall_num];
sp_reg = env-active_tc.gpr[29];
switch (nb_args) {
/* these arguments are taken from the stack */
/* FIXME - what to do if get_user() fails? */
-case 8: get_user_ual(arg8, sp_reg + 28);
-case 7: get_user_ual(arg7, sp_reg + 24);
+case 8: /* get_user_ual(arg8, sp_reg + 28); */
+case 7: /* get_user_ual(arg7, sp_reg + 24); */
I'd prefer to see these and the respective variable definitions #if
0'd with a comment, stating that they're currently unused.
I'd prefer not to see if 0 code. Better, I think, to mark the
variables as __attribute__((unused)) with that same comment.
@@ -7058,18 +7056,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
case TARGET_NR_osf_sigprocmask:
{
abi_ulong mask;
-int how = arg1;
sigset_t set, oldset;
switch(arg1) {
case TARGET_SIG_BLOCK:
-how = SIG_BLOCK;
break;
case TARGET_SIG_UNBLOCK:
-how = SIG_UNBLOCK;
break;
case TARGET_SIG_SETMASK:
-how = SIG_SETMASK;
why go through the effort of setting how and then not using it? I'm
pretty sure this is a bug as well. A few lines down is the following
code:
sigprocmask(arg1, set, oldset);
which in TARGET_NR_sigprocmask would be:
ret = get_errno(sigprocmask(how, set, oldset));
So we end up sending guest masks to the host. Richard, this is Alpha
specific code. Mind to double-check?
I remember fixing this before. Perhaps it was in a patch tree that
never got pulled...
r~
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/10] alpha/translate: remve unused variables
On 06/14/2011 10:36 AM, Michael S. Tsirkin wrote: Signed-off-by: Michael S. Tsirkin m...@redhat.com --- target-alpha/translate.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) Acked-by: Richard Henderson r...@twiddle.net r~ -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] alpha: remove unused variable
On 06/14/2011 10:36 AM, Michael S. Tsirkin wrote: Signed-off-by: Michael S. Tsirkin m...@redhat.com --- target-alpha/translate.c |7 ++- 1 files changed, 6 insertions(+), 1 deletions(-) Acked-by: Richard Henderson r...@twiddle.net r~ -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [RFC PATCH 01/13] Generic DMA memory access interface
On 05/31/2011 06:38 PM, Eduard - Gabriel Munteanu wrote:
+static inline void dma_memory_rw(DMADevice *dev,
+ dma_addr_t addr,
+ void *buf,
+ dma_addr_t len,
+ int is_write)
I don't think this needs to be inline...
+{
+/*
+ * Fast-path non-iommu.
+ * More importantly, makes it obvious what this function does.
+ */
+if (!dev || !dev-mmu) {
+cpu_physical_memory_rw(addr, buf, len, is_write);
+return;
+}
... because you'll never be able to eliminate the if or the calls.
You might as well make the overall code smaller by taking the
entire function out of line.
+#define DEFINE_DMA_LD(prefix, suffix, devtype, dmafield, size)\
+static inline uint##size##_t \
+dma_ld##suffix(DMADevice *dev, dma_addr_t addr) \
+{ \
+int err; \
+dma_addr_t paddr, plen; \
+ \
+if (!dev || !dev-mmu) { \
+return ld##suffix##_phys(addr); \
+} \
Similarly for all the ld/st functions.
+#define DEFINE_DMA_MEMORY_RW(prefix, devtype, dmafield)
+#define DEFINE_DMA_MEMORY_READ(prefix, devtype, dmafield)
+#define DEFINE_DMA_MEMORY_WRITE(prefix, devtype, dmafield)
+
+#define DEFINE_DMA_OPS(prefix, devtype, dmafield) \
I think this is a bit over the top, really.
+err = dev-mmu-translate(dev, addr, paddr, plen, is_write);
I see you didn't take my suggestion for using an opaque callback pointer.
Really and truly, I won't be able to use this as-is for Alpha.
r~
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [RFC PATCH 01/13] Generic DMA memory access interface
On 06/01/2011 07:52 AM, Eduard - Gabriel Munteanu wrote:
The main selling point is there are more chances to screw up if every
bus layer implements these manually. And it's really convenient,
especially if we get to add another ld/st.
If we drop the ld/st, we're talking about 5 lines for every bus layer.
If I recall, there was just the one driver that actually uses the ld/st
interface; most used the read/write interface.
If I understand correctly you need some sort of shared state between
IOMMUs or units residing on different buses. Then you should be able to
get to it even with this API, just like I do with my AMD IOMMU state by
upcasting. It doesn't seem to matter whether you've got an opaque, that
opaque could very well be reachable by upcasting.
Did I get this wrong?
Can you honestly tell me that
+static int amd_iommu_translate(DMADevice *dev,
+ dma_addr_t addr,
+ dma_addr_t *paddr,
+ dma_addr_t *len,
+ int is_write)
+{
+PCIDevice *pci_dev = container_of(dev, PCIDevice, dma);
+PCIDevice *iommu_dev = DO_UPCAST(PCIDevice, qdev, dev-mmu-iommu);
+AMDIOMMUState *s = DO_UPCAST(AMDIOMMUState, dev, iommu_dev);
THREE (3) upcasts is a sane to write maintainable software?
The margin for error here is absolutely enormous.
If you had just passed in that AMDIOMMUState* as the opaque
value, it would be trivial to look at the initialization
statement and the callback function to verify that the right
value is being passed.
r~
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [RFC PATCH 01/13] Generic DMA memory access interface
On 06/01/2011 07:29 AM, Avi Kivity wrote: On 06/01/2011 05:01 PM, Richard Henderson wrote: +err = dev-mmu-translate(dev, addr,paddr,plen, is_write); I see you didn't take my suggestion for using an opaque callback pointer. Really and truly, I won't be able to use this as-is for Alpha. Rather than opaques, please pass the DMA engine itself and use container_of(). The dma engine object is currently sitting in the PCIBus structure. Which is private, and can't be extended by a host bridge implementation. The entire code could be re-arranged, true, but please suggest something reasonable. We should be removing opaques, not adding them. See my followup elsewhere. Opaques *can* be cleaner than upcasting, particularly if there are too many hoops through which to jump. r~ -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [RFC PATCH 01/13] Generic DMA memory access interface
On 06/01/2011 08:35 AM, Eduard - Gabriel Munteanu wrote: Maybe it's not nice, but you're missing the fact upcasting gives you some type safety. With opaques you have none. Lol. Do you understand what container_of does? This is not dynamic_cast with RTTI. You can put any type name in there that you like, so long as it has a field name to match. The type of the field you give doesn't even have to match the type of the pointer that you pass in. Type safety this is not. r~ -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [RFC PATCH 00/13] AMD IOMMU emulation patches, another try
On 05/31/2011 06:38 PM, Eduard - Gabriel Munteanu wrote: Hi, Again, sorry for taking so long, but I just don't send stuff without looking through it. This is meant to go into Michael's PCI branch, if it does. Some of the changes include: - some fixes (one thanks to David Gibson) and cleanups - macro magic for exporting clones of the DMA interface (e.g. pci_memory_read()); I hope it isn't too much a stretch - we use pci_memory_*() in most places where PCI devices are involved now - luckily we don't need unaligned accesses anymore - some attempt at signaling target aborts, but it doesn't seem like that stuff is completely implemented in the PCI layer / devices - PCI ids are defined in hw/amd_iommu.c until they get merged into Linux Also, I can't answer every request that the API is extended for doing this and that more comfortably. I understand there may be corner cases, but may I suggest merging it (maybe into a separate branch related to mst's pci) so that everybody can deal with it? This is still labeled RFC, but if you think it's ready it can be merged. I hope most of the important issues have been dealt with. I'll post the SeaBIOS patches soon (though I think you can give it a spin with the old ones, if you need). I'll also take care of submitting PCI ids to be merged into Linux. In any case, let me know what you think. I hope I didn't forget to Cc someone. In order to move the discussion along productively, please have a look at git://repo.or.cz/qemu/rth.git axp-iommu-1 which is based on your previous patch set. There's stuff in there that's not 100% relevant to the discussion, but these two commits: 0652a74 target-alpha: Implement iommu translation for Typhoon. db50b11 DMA: Use an void* opaque value, rather than upcasting from qdev. are exactly what I'm interested in discussing. r~ -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [RFC v1] Add declarations for hierarchical memory region API
On 05/20/2011 02:23 AM, Avi Kivity wrote:
On 05/19/2011 11:43 PM, Anthony Liguori wrote:
On 05/19/2011 09:12 AM, Avi Kivity wrote:
The memory API separates the attributes of a memory region (its size, how
reads or writes are handled, dirty logging, and coalescing) from where it
is mapped and whether it is enabled. This allows a device to configure
a memory region once, then hand it off to its parent bus to map it according
to the bus configuration.
Hierarchical registration also allows a device to compose a region out of
a number of sub-regions with different properties; for example some may be
RAM while others may be MMIO.
+struct {
+/* If nonzero, specify bounds on access sizes beyond which a
machine
+ * check is thrown.
+ */
+unsigned min_access_size;
+unsigned max_access_size;
+/* If true, unaligned accesses are supported. Otherwise unaligned
+ * accesses throw machine checks.
+ */
+ bool unaligned;
+} valid;
Under what circumstances would this be used?
The behavior of devices that receive non-natural accesses varies wildly.
For PCI devices, invalid accesses almost always return ~0. I can't think of
a device where an MCE would occur.
This was requested by Richard, so I'll let him comment.
Several alpha system chips MCE when accessed with incorrect sizes.
E.g. only 64-bit accesses are allowed.
Is this structure honestly any better than 4 function pointers?
I can't see that it is, myself.
r~
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [RFC v1] Add declarations for hierarchical memory region API
On 05/20/2011 07:31 AM, Anthony Liguori wrote: But is this a characteristic of devices or is this a characteristic of the chipset/CPU? Chipset. r~ -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [RFC PATCH 7/7] acpi: fix bug in acpi_checksum() caused by garbage in checksum field
On 03/30/2010 01:20 AM, Eduard - Gabriel Munteanu wrote: +/* Ignore preexisting garbage in checksum. */ +acpi_hdr = (struct acpi_table_header *) data; +sum -= acpi_hdr-checksum; + return (-sum) 0xff; Wouldn't it be cleaner to adjust the acpi_checksum definition to take and acpi_table_header operand instead of uint8_t? And given it's only usage, perhaps update the checksum instead of returning it? E.g. -((struct acpi_table_header*)p)-checksum = acpi_checksum((uint8_t*)p, off); +acpi_update_checksum((struct acpi_table_header *)p, off); r~ -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] qemu: jaso-parser: Output the content of invalid keyword
On 03/24/2010 08:12 AM, Amos Kong wrote:
When input some invialid word 'unknowcmd' through QMP port, qemu outputs this
error message:
parse error: invalid keyword `%s'
This patch makes qemu output the content of invalid keyword, like:
parse error: invalid keyword `unknowcmd'
Signed-off-by: Amos Kong ak...@redhat.com
Acked-by: Richard Henderson r...@redhat.com
---
json-parser.c |8 +++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/json-parser.c b/json-parser.c
index 579928f..b55d763 100644
--- a/json-parser.c
+++ b/json-parser.c
@@ -12,6 +12,7 @@
*/
#include stdbool.h
+#include stdarg.h
#include qemu-common.h
#include qstring.h
@@ -93,7 +94,12 @@ static int token_is_escape(QObject *obj, const char *value)
*/
static void parse_error(JSONParserContext *ctxt, QObject *token, const char
*msg, ...)
{
-fprintf(stderr, parse error: %s\n, msg);
+va_list ap;
+va_start(ap, msg);
+fprintf(stderr, parse error: );
+vfprintf(stderr, msg, ap);
+fprintf(stderr, \n);
+va_end(ap);
}
/**
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] Re: [PATCH 2/6] qemu-kvm: Modify and introduce wrapper functions to access phys_ram_dirty.
On 03/16/2010 01:10 PM, Blue Swirl wrote: Just a tangential note: a long time ago, I tried to disable self modifying code detection for Sparc. On most RISC architectures, SMC needs explicit flushing so in theory we need not track code memory writes. However, during exceptions the translator needs to access the original unmodified code that was used to generate the TB. But maybe there are other ways to avoid SMC tracking, on x86 it's still needed but I suppose SMC is pretty rare. True SMC is fairly rare, but the SMC checker triggers fairly often on the PLT update during dynamic linking. Nearly all cpus (x86 being the only exception I recall) needed to re-design their PLT format to avoid this code update in order to support SELinux. Where does the translator need access to this original code? I was just thinking about this problem today, wondering how much overhead there is with this SMC page protection thing. r~ -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
