Re: [Qemu-devel] [PATCH v29 8/8] target/avr: Add tests
On 24/08/2019 20.46, Michael Rolnik wrote:
> 1. Avocado test
> The test is based on
> https://github.com/seharris/qemu-avr-tests/tree/master/free-rtos/Demo
> demo which. If working correctly, prints 'ABCDEFGHIJKLMNOPQRSTUVWX' out.
> it also demostrates that timer and IRQ are working
>
> 2. Boot serial test
> Prinit out 'T' through serial port
>
> Signed-off-by: Michael Rolnik
> Reviewed-by: Philippe Mathieu-Daudé
> Tested-by: Philippe Mathieu-Daudé
> ---
> tests/Makefile.include | 2 ++
> tests/acceptance/machine_avr6.py | 36
> tests/boot-serial-test.c | 10 +
> 3 files changed, 48 insertions(+)
> create mode 100644 tests/acceptance/machine_avr6.py
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 49684fd4f4..dbaeb87572 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -202,6 +202,8 @@ check-qtest-i386-y += tests/test-x86-cpuid-compat$(EXESUF)
> check-qtest-i386-y += tests/numa-test$(EXESUF)
> check-qtest-x86_64-y += $(check-qtest-i386-y)
>
> +check-qtest-avr-y += tests/boot-serial-test$(EXESUF)
> +
> check-qtest-alpha-y += tests/boot-serial-test$(EXESUF)
> check-qtest-alpha-$(CONFIG_VGA) += tests/display-vga-test$(EXESUF)
>
> diff --git a/tests/acceptance/machine_avr6.py
> b/tests/acceptance/machine_avr6.py
> new file mode 100644
> index 00..0601080b01
> --- /dev/null
> +++ b/tests/acceptance/machine_avr6.py
> @@ -0,0 +1,36 @@
> +import logging
> +import time
> +import distutils.spawn
> +
> +from avocado import skipUnless
> +from avocado_qemu import Test
> +from avocado.utils import process
> +
> +class AVR6Machine(Test):
> +timeout = 5
> +
> +def test_freertos(self):
> +"""
> +:avocado: tags=arch:avr
> +:avocado: tags=machine:sample
> +"""
> +"""
> +
> https://github.com/seharris/qemu-avr-tests/raw/master/free-rtos/Demo/AVR_ATMega2560_GCC/demo.elf
> +constantly prints out
> 'ABCDEFGHIJKLMNOPQRSTUVWXABCDEFGHIJKLMNOPQRSTUVWX'
> +"""
> +rom_url = 'https://github.com/seharris/qemu-avr-tests'
> +rom_url += '/raw/master/free-rtos/Demo/AVR_ATMega2560_GCC/demo.elf'
> +rom_hash = '7eb521f511ca8f2622e0a3c5e8dd686efbb911d4'
> +rom_path = self.fetch_asset(rom_url, asset_hash=rom_hash)
> +
> +self.vm.set_machine('sample')
> +self.vm.add_args('-bios', rom_path)
> +self.vm.add_args('-nographic')
> +self.vm.launch()
> +
> +time.sleep(2)
> +self.vm.shutdown()
> +
> +match = 'ABCDEFGHIJKLMNOPQRSTUVWXABCDEFGHIJKLMNOPQRSTUVWX'
> +
> +self.assertIn(match, self.vm.get_log())
> diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
> index 24852d4c7d..22cbaccc1b 100644
> --- a/tests/boot-serial-test.c
> +++ b/tests/boot-serial-test.c
> @@ -16,6 +16,15 @@
> #include "qemu/osdep.h"
> #include "libqtest.h"
>
> +static const uint8_t bios_avr[] = {
> +0x88, 0xe0, /* ldi r24, 0x08 */
> +0x80, 0x93, 0xc1, 0x00, /* sts 0x00C1, r24 ; Enable tx */
> +0x86, 0xe0, /* ldi r24, 0x06 */
> +0x80, 0x93, 0xc2, 0x00, /* sts 0x00C2, r24 ; Set the data bits to 8 */
> +0x84, 0xe5, /* ldi r24, 0x54 */
> +0x80, 0x93, 0xc6, 0x00, /* sts 0x00C6, r24 ; Output 'T' */
> +};
> +
> static const uint8_t kernel_mcf5208[] = {
> 0x41, 0xf9, 0xfc, 0x06, 0x00, 0x00, /* lea 0xfc06,%a0 */
> 0x10, 0x3c, 0x00, 0x54, /* move.b #'T',%d0 */
> @@ -92,6 +101,7 @@ typedef struct testdef {
>
> static testdef_t tests[] = {
> { "alpha", "clipper", "", "PCI:" },
> +{ "avr", "sample", "", "T", sizeof(bios_avr), NULL, bios_avr },
> { "ppc", "ppce500", "", "U-Boot" },
> { "ppc", "40p", "-vga none -boot d", "Trying cd:," },
> { "ppc", "g3beige", "", "PowerPC,750" },
>
Acked-by: Thomas Huth
[Qemu-devel] [PATCH qemu] spapr: Render full FDT on ibm, client-architecture-support
The ibm,client-architecture-support call is a way for the guest to
negotiate capabilities with a hypervisor. It is implemented as:
- the guest calls SLOF via client interface;
- SLOF calls QEMU (H_CAS hypercall) with an options vector from the guest;
- QEMU returns a device tree diff (which uses FDT format with
an additional header before it);
- SLOF walks through the partial diff tree and updates its internal tree
with the values from the diff.
This changes QEMU to simply re-render the entire tree and send it as
an update. SLOF can handle this already mostly, [1] is needed before this
can be applied.
The benefit is reduced code size as there is no need for another set of
DT rendering helpers such as spapr_fixup_cpu_dt().
The downside is that the updates are bigger now (as they include all
nodes and properties) but the difference on a '-smp 256,threads=1' system
before/after is 2.35s vs. 2.5s.
While at this, add a missing g_free(fdt) if the resulting tree is bigger
than the space allocated by SLOF. Also, store the resulting tree in
the spapr machine to have the latest valid FDT copy possible (this should
not matter much as H_UPDATE_DT happens right after that but nevertheless).
[1] https://patchwork.ozlabs.org/patch/1152915/
Signed-off-by: Alexey Kardashevskiy
---
hw/ppc/spapr.c | 90 ++
1 file changed, 10 insertions(+), 80 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index baedadf20b8c..6dea5947afbc 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -295,65 +295,6 @@ static void spapr_populate_pa_features(SpaprMachineState
*spapr,
_FDT((fdt_setprop(fdt, offset, "ibm,pa-features", pa_features, pa_size)));
}
-static int spapr_fixup_cpu_dt(void *fdt, SpaprMachineState *spapr)
-{
-MachineState *ms = MACHINE(spapr);
-int ret = 0, offset, cpus_offset;
-CPUState *cs;
-char cpu_model[32];
-uint32_t pft_size_prop[] = {0, cpu_to_be32(spapr->htab_shift)};
-
-CPU_FOREACH(cs) {
-PowerPCCPU *cpu = POWERPC_CPU(cs);
-DeviceClass *dc = DEVICE_GET_CLASS(cs);
-int index = spapr_get_vcpu_id(cpu);
-int compat_smt = MIN(ms->smp.threads, ppc_compat_max_vthreads(cpu));
-
-if (!spapr_is_thread0_in_vcore(spapr, cpu)) {
-continue;
-}
-
-snprintf(cpu_model, 32, "%s@%x", dc->fw_name, index);
-
-cpus_offset = fdt_path_offset(fdt, "/cpus");
-if (cpus_offset < 0) {
-cpus_offset = fdt_add_subnode(fdt, 0, "cpus");
-if (cpus_offset < 0) {
-return cpus_offset;
-}
-}
-offset = fdt_subnode_offset(fdt, cpus_offset, cpu_model);
-if (offset < 0) {
-offset = fdt_add_subnode(fdt, cpus_offset, cpu_model);
-if (offset < 0) {
-return offset;
-}
-}
-
-ret = fdt_setprop(fdt, offset, "ibm,pft-size",
- pft_size_prop, sizeof(pft_size_prop));
-if (ret < 0) {
-return ret;
-}
-
-if (nb_numa_nodes > 1) {
-ret = spapr_fixup_cpu_numa_dt(fdt, offset, cpu);
-if (ret < 0) {
-return ret;
-}
-}
-
-ret = spapr_fixup_cpu_smt_dt(fdt, offset, cpu, compat_smt);
-if (ret < 0) {
-return ret;
-}
-
-spapr_populate_pa_features(spapr, cpu, fdt, offset,
- spapr->cas_legacy_guest_workaround);
-}
-return ret;
-}
-
static hwaddr spapr_node0_size(MachineState *machine)
{
if (nb_numa_nodes) {
@@ -983,11 +924,13 @@ static bool spapr_hotplugged_dev_before_cas(void)
return false;
}
+static void *spapr_build_fdt(SpaprMachineState *spapr);
+
int spapr_h_cas_compose_response(SpaprMachineState *spapr,
target_ulong addr, target_ulong size,
SpaprOptionVector *ov5_updates)
{
-void *fdt, *fdt_skel;
+void *fdt;
SpaprDeviceTreeUpdateHeader hdr = { .version_id = 1 };
if (spapr_hotplugged_dev_before_cas()) {
@@ -1003,28 +946,11 @@ int spapr_h_cas_compose_response(SpaprMachineState
*spapr,
size -= sizeof(hdr);
-/* Create skeleton */
-fdt_skel = g_malloc0(size);
-_FDT((fdt_create(fdt_skel, size)));
-_FDT((fdt_finish_reservemap(fdt_skel)));
-_FDT((fdt_begin_node(fdt_skel, "")));
-_FDT((fdt_end_node(fdt_skel)));
-_FDT((fdt_finish(fdt_skel)));
-fdt = g_malloc0(size);
-_FDT((fdt_open_into(fdt_skel, fdt, size)));
-g_free(fdt_skel);
-
-/* Fixup cpu nodes */
-_FDT((spapr_fixup_cpu_dt(fdt, spapr)));
-
-if (spapr_dt_cas_updates(spapr, fdt, ov5_updates)) {
-return -1;
-}
-
-/* Pack resulting tree */
+fdt = spapr_build_fdt(spapr);
_FDT((fdt_pack(fdt)));
if (fdt_totalsize(fdt) + sizeof(hdr) > size) {
+g_free(fdt);
trace_spapr_cas_failed(size);
return -1;
}
Re: [Qemu-devel] [RFC PATCH qemu] qapi: Add query-memory-checksum
On 23/08/2019 21:41, Markus Armbruster wrote: Daniel P. Berrangé writes: On Fri, Aug 23, 2019 at 07:49:31AM +0200, Markus Armbruster wrote: Daniel P. Berrangé writes: On Thu, Aug 22, 2019 at 04:16:53PM +0200, Markus Armbruster wrote: Alexey Kardashevskiy writes: This returns MD5 checksum of all RAM blocks for migration debugging as this is way faster than saving the entire RAM to a file and checking that. Signed-off-by: Alexey Kardashevskiy Any particular reason for MD5? Have you measured the other choices offered by GLib? I understand you don't need crypto-strength here. Both MD5 and SHA-1 would be bad choices then. We have a tests/bench-crypto-hash test but its hardcoded for sha256. I hacked it to report all algorithms and got these results for varying input chunk sizes: /crypto/hash/md5/speed-512: 519.12 MB/sec OK /crypto/hash/md5/speed-1024: 560.39 MB/sec OK /crypto/hash/md5/speed-4096: 591.39 MB/sec OK /crypto/hash/md5/speed-16384: 576.46 MB/sec OK /crypto/hash/sha1/speed-512: 443.12 MB/sec OK /crypto/hash/sha1/speed-1024: 518.82 MB/sec OK /crypto/hash/sha1/speed-4096: 555.60 MB/sec OK /crypto/hash/sha1/speed-16384: 568.16 MB/sec OK /crypto/hash/sha224/speed-512: 221.90 MB/sec OK /crypto/hash/sha224/speed-1024: 239.79 MB/sec OK /crypto/hash/sha224/speed-4096: 269.37 MB/sec OK /crypto/hash/sha224/speed-16384: 274.87 MB/sec OK /crypto/hash/sha256/speed-512: 222.75 MB/sec OK /crypto/hash/sha256/speed-1024: 253.25 MB/sec OK /crypto/hash/sha256/speed-4096: 272.80 MB/sec OK /crypto/hash/sha256/speed-16384: 275.59 MB/sec OK /crypto/hash/sha384/speed-512: 322.73 MB/sec OK /crypto/hash/sha384/speed-1024: 369.84 MB/sec OK /crypto/hash/sha384/speed-4096: 406.71 MB/sec OK /crypto/hash/sha384/speed-16384: 417.87 MB/sec OK /crypto/hash/sha512/speed-512: 320.62 MB/sec OK /crypto/hash/sha512/speed-1024: 361.93 MB/sec OK /crypto/hash/sha512/speed-4096: 404.91 MB/sec OK /crypto/hash/sha512/speed-16384: 418.53 MB/sec OK /crypto/hash/ripemd160/speed-512: 226.45 MB/sec OK /crypto/hash/ripemd160/speed-1024: 239.25 MB/sec OK /crypto/hash/ripemd160/speed-4096: 251.31 MB/sec OK /crypto/hash/ripemd160/speed-16384: 255.01 MB/sec OK IOW, md5 is clearly the quickest, by a considerable margin over SHA256/512. SHA1 is slightly slower. Assuming that we document that this command is intentionally *not* trying to guarantee collision resistances we're ok. In fact we should not document what kind of checksum is reported by query-memory-checksum. The impl should be a black box from user's POV. If we're just aiming for debugging tool to detect accidental corruption, could we even just ignore cryptographic hashs entirely and do a crc32 - that'd be way faster than even md5. Good points. The doc strings should spell out "for debugging", like the commit message does, and both should spell out "weak collision resistance". I can't find CRC-32 in GLib, but zlib appears to provide it: http://refspecs.linuxbase.org/LSB_3.0.0/LSB-Core-generic/LSB-Core-generic/zlib-crc32-1.html Care to compare its speed to MD5? I hacked the code to use zlib's crc32 impl and got these for comparison: /crypto/hash/crc32/speed-512: 1089.18 MB/sec OK /crypto/hash/crc32/speed-1024: 1124.63 MB/sec OK /crypto/hash/crc32/speed-4096: 1162.73 MB/sec OK /crypto/hash/crc32/speed-16384: 1171.58 MB/sec OK /crypto/hash/crc32/speed-1048576: 1165.68 MB/sec OK /crypto/hash/md5/speed-512: 476.27 MB/sec OK /crypto/hash/md5/speed-1024: 517.16 MB/sec OK /crypto/hash/md5/speed-4096: 554.70 MB/sec OK /crypto/hash/md5/speed-16384: 564.44 MB/sec OK /crypto/hash/md5/speed-1048576: 566.78 MB/sec OK Twice as fast. Alexey, what do you think? This is even better. TBH I picked md5 as I could not spot crc32 helper in the first minute (I can see it now) and MD5 felt the fastest available from glibc :) I'll probably add start..end range(s) and repost. Thanks for all these numbers and reviews. -- Alexey
Re: [Qemu-devel] [PATCH v2 35/68] target/arm: Convert CPS (privileged)
On 8/25/19 6:10 PM, Richard Henderson wrote: > On 8/25/19 1:43 PM, Peter Maydell wrote: >> I'm still confused, I think. The hint space is >> + NOP 0011 1010 1000 >> (plus the more specific hint insns before that pattern with >> fixed values in the [7:0] bits). >> CPS falls into that space; but you've placed it with >> SMC and HVC which don't fall into the hint space, because >> they have 0111 in bits [27:24], not 0011. > > Oops. I see what you mean. So, I've moved the line up immediately following the hint space, and added a comment: +# If imod == '00' && M == '0' then SEE "Hint instructions", above. +CPS 0011 1010 1000 0 imod:2 M:1 A:1 I:1 F:1 mode:5 \ + The line *was* still within the same group (which is large), so it doesn't actually make a difference to the decode, but I do agree it makes more sense in the new position. r~
Re: [Qemu-devel] [PATCH v2 35/68] target/arm: Convert CPS (privileged)
On 8/25/19 1:43 PM, Peter Maydell wrote: > I'm still confused, I think. The hint space is > + NOP 0011 1010 1000 > (plus the more specific hint insns before that pattern with > fixed values in the [7:0] bits). > CPS falls into that space; but you've placed it with > SMC and HVC which don't fall into the hint space, because > they have 0111 in bits [27:24], not 0011. Oops. I see what you mean. r~
[Qemu-devel] [PATCH 3/3] audio: paaudio: ability to specify stream name
This can be used to identify stream in tools like pavucontrol when one
creates multiple -audiodevs or runs multiple qemu instances.
Signed-off-by: Kővágó, Zoltán
---
qapi/audio.json | 6 ++
audio/paaudio.c | 4 ++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/qapi/audio.json b/qapi/audio.json
index 9fefdf5186..a433b3c9d7 100644
--- a/qapi/audio.json
+++ b/qapi/audio.json
@@ -206,6 +206,11 @@
#
# @name: name of the sink/source to use
#
+# @stream-name: name of the PulseAudio stream created by qemu. Can be
+# used to identify the stream in PulseAudio when you
+# create multiple PulseAudio devices or run multiple qemu
+# instances (default "qemu", since 4.2)
+#
# @latency: latency you want PulseAudio to achieve in microseconds
# (default 15000)
#
@@ -215,6 +220,7 @@
'base': 'AudiodevPerDirectionOptions',
'data': {
'*name': 'str',
+'*stream-name': 'str',
'*latency': 'uint32' } }
##
diff --git a/audio/paaudio.c b/audio/paaudio.c
index 777b8e4718..827f442b6e 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -562,7 +562,7 @@ static int qpa_init_out(HWVoiceOut *hw, struct audsettings
*as,
pa->stream = qpa_simple_new (
c,
-"qemu",
+ppdo->has_stream_name ? ppdo->stream_name : "qemu",
PA_STREAM_PLAYBACK,
ppdo->has_name ? ppdo->name : NULL,
,
@@ -630,7 +630,7 @@ static int qpa_init_in(HWVoiceIn *hw, struct audsettings
*as, void *drv_opaque)
pa->stream = qpa_simple_new (
c,
-"qemu",
+ppdo->has_stream_name ? ppdo->stream_name : "qemu",
PA_STREAM_RECORD,
ppdo->has_name ? ppdo->name : NULL,
,
--
2.22.0
[Qemu-devel] [PATCH 2/3] audio: paaudio: fix client name
pa_context_new expects a client name, not a server socket path.
Signed-off-by: Kővágó, Zoltán
---
audio/paaudio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/audio/paaudio.c b/audio/paaudio.c
index bfef9acaad..777b8e4718 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -866,7 +866,7 @@ static void *qpa_conn_init(const char *server)
}
c->context = pa_context_new(pa_threaded_mainloop_get_api(c->mainloop),
-server);
+"qemu");
if (!c->context) {
goto fail;
}
--
2.22.0
[Qemu-devel] [PATCH 0/3] Audio: misc fixes for "Audio 20190821 patches"
Hi, This series fixes two problems reported by Maxim Levitsky in relation of my multiple audio backend patch series, and a small feature request. Unfortunately I don't really use PulseAudio nowadays, so I haven't tested it other than making sure it compiles and connects to pa. Regards, Zoltan Kővágó, Zoltán (3): audio: omitting audiodev= parameter is only deprecated audio: paaudio: fix client name audio: paaudio: ability to specify stream name qemu-deprecated.texi | 7 +++ qapi/audio.json | 6 ++ audio/audio.c| 8 audio/paaudio.c | 6 +++--- 4 files changed, 20 insertions(+), 7 deletions(-) -- 2.22.0
[Qemu-devel] [PATCH 1/3] audio: omitting audiodev= parameter is only deprecated
Unfortunately, changes introduced in af2041ed2d "audio: audiodev=
parameters no longer optional when -audiodev present" breaks backward
compatibility. This patch changes the error into a deprecation warning.
Signed-off-by: Kővágó, Zoltán
---
qemu-deprecated.texi | 7 +++
audio/audio.c| 8
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 00a4b6f350..9d74a1cfc0 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -72,6 +72,13 @@ backend settings instead of environment variables. To ease
migration to
the new format, the ``-audiodev-help'' option can be used to convert
the current values of the environment variables to ``-audiodev'' options.
+@subsection Creating sound card devices and vnc without audiodev= property
(since 4.2)
+
+When not using the deprecated legacy audio config, each sound card
+should specify an @code{audiodev=} property. Additionally, when using
+vnc, you should specify an @code{audiodev=} propery if you plan to
+transmit audio through the VNC protocol.
+
@subsection -mon ...,control=readline,pretty=on|off (since 4.1)
The @code{pretty=on|off} switch has no effect for HMP monitors, but is
diff --git a/audio/audio.c b/audio/audio.c
index 7d715332c9..e13addf922 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1412,8 +1412,9 @@ static AudioState *audio_init(Audiodev *dev, const char
*name)
drvname = AudiodevDriver_str(dev->driver);
} else if (!QTAILQ_EMPTY(_states)) {
if (!legacy_config) {
-dolog("You must specify an audiodev= for the device %s\n", name);
-exit(1);
+dolog("Device %s: audiodev default parameter is deprecated, please
"
+ "specify audiodev=%s\n", name,
+ QTAILQ_FIRST(_states)->dev->id);
}
return QTAILQ_FIRST(_states);
} else {
@@ -1548,8 +1549,7 @@ CaptureVoiceOut *AUD_add_capture(
if (!s) {
if (!legacy_config) {
-dolog("You must specify audiodev when trying to capture\n");
-return NULL;
+dolog("Capturing without setting an audiodev is deprecated\n");
}
s = audio_init(NULL, NULL);
}
--
2.22.0
Re: [Qemu-devel] [PULL 07/15] audio: audiodev= parameters no longer optional when -audiodev present
On 2019-08-26 00:15, Maxim Levitsky wrote: > On Sun, 2019-08-25 at 20:05 +0200, Zoltán Kővágó wrote: >> On 2019-08-25 11:44, Maxim Levitsky wrote: >>> On Wed, 2019-08-21 at 10:41 +0200, Gerd Hoffmann wrote: From: Kővágó, Zoltán This means you should probably stop using -soundhw (as it doesn't allow you to specify any options) and add the device manually with -device. The exception is pcspk, it's currently not possible to manually add it. To use it with audiodev, use something like this: -audiodev id=foo,... -global isa-pcspk.audiodev=foo -soundhw pcspk >>> >>> Hi! >> >> Hi, >> >>> There is one corner case this breaks. >>> In qemu 4.1.0, there is no way to specify audiodev for a sound device, >>> specifying it >>> fails with error. >>> So some of my machines have audiodev (which is miles better that using old >>> env variables) >>> but also have sound devices without audiodev reference since this wasn't >>> supported. >>> >>> >>> In what will be qemu 4.2, you must specify it, thus this kind of breaks >>> backward compatibility. >>> Maybe we can have audiodev reference optional for a version or two? >>> >>> This is just a minor itch, as otherwise the sound improvements are really >>> good. The days >>> of installing that old realtek driver are finally gone :-) >> >> Hmm, this is what happens when you split a patch series. We could >> either revert this patch, or alternatively turn the error messages into >> warnings about using deprecated behavior. > Warning would be great in this case! >> >>> Another thing I noted, that there is no way for pulseaudio audiodev to >>> specify the 'client name', >>> it always shows up in pavucontrl as the socket path to the server. >>> Thus if I added two PA audiodevs, I can't really distinguish between them. >>> The in|out.name= seems to specify the pulseaudio source/sink to connect to, >>> which is not the same. >> >> We currently supply the constant "qemu" as a name to pa_stream_new. >> While it's still not ideal, shouldn't this end up as a client name in >> pulseaudio instead of a socket path? > > Actually it seems that pulseaudio has two names supplied for each stream > Maybe stream name and application name? > > This is how chromium playback looks versus qemu in pavucontrol and in gnome > volume control. > > https://imgur.com/a/I8HZhgx > > I do notice that 'qemu' now, in pavucontrol though. I see. We currently pass the server socket to pa_context_new instead of the client name. I'll prepare a patch soon, thanks for the report! Regards, Zoltan
Re: [Qemu-devel] [Slirp] [PATCH 1/2] Do not reassemble fragments pointing outside of the original payload
Hello,
Philippe Mathieu-Daudé, le ven. 23 août 2019 17:15:32 +0200, a ecrit:
> > Did you make your test with commit 126c04acbabd ("Fix heap overflow in
> > ip_reass on big packet input") applied?
>
> Yes, unfortunately it doesn't fix the issue.
Ok.
Could you try the attached patch? There was a use-after-free. Without
it, I can indeed crash qemu with the given exploit. With it I don't
seem to be able to crash it (trying in a loop for several minutes).
Samuel
diff --git a/src/ip_input.c b/src/ip_input.c
index 7364ce0..aa514ae 100644
--- a/src/ip_input.c
+++ b/src/ip_input.c
@@ -292,6 +292,7 @@ static struct ip *ip_reass(Slirp *slirp, struct ip *ip,
struct ipq *fp)
*/
while (q != (struct ipasfrag *)>frag_link &&
ip->ip_off + ip->ip_len > q->ipf_off) {
+struct ipasfrag *prev;
i = (ip->ip_off + ip->ip_len) - q->ipf_off;
if (i < q->ipf_len) {
q->ipf_len -= i;
@@ -299,9 +300,10 @@ static struct ip *ip_reass(Slirp *slirp, struct ip *ip,
struct ipq *fp)
m_adj(dtom(slirp, q), i);
break;
}
+prev = q;
q = q->ipf_next;
-m_free(dtom(slirp, q->ipf_prev));
-ip_deq(q->ipf_prev);
+ip_deq(prev);
+m_free(dtom(slirp, prev));
}
insert:
Re: [Qemu-devel] [PATCH v2 2/2] iotests: Test allocate_first_block() with O_DIRECT
On Mon, Aug 26, 2019 at 1:03 AM Nir Soffer wrote:
> Using block_resize we can test allocate_first_block() with file
> descriptor opened with O_DIRECT, ensuring that it works for any size
> larger than 4096 bytes.
>
> Testing smaller sizes is tricky as the result depends on the filesystem
> used for testing. For example on NFS any size will work since O_DIRECT
> does not require any alignment.
>
Forgot to add:
Signed-off-by: Nir Soffer
---
> tests/qemu-iotests/175 | 25 +
> tests/qemu-iotests/175.out | 8
> 2 files changed, 33 insertions(+)
>
> diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175
> index d54cb43c39..60cc251eb2 100755
> --- a/tests/qemu-iotests/175
> +++ b/tests/qemu-iotests/175
> @@ -49,6 +49,23 @@ _filter_blocks()
> -e "s/blocks=$((extra_blocks + img_size /
> 512))\\(\$\\|[^0-9]\\)/max allocation/"
> }
>
> +# Resize image using block_resize.
> +# Parameter 1: image path
> +# Parameter 2: new size
> +_block_resize()
> +{
> +local path=$1
> +local size=$2
> +
> +$QEMU -qmp stdio -nographic -nodefaults \
> +-blockdev file,node-name=file,filename=$path,cache.direct=on \
> +< +{'execute': 'qmp_capabilities'}
> +{'execute': 'block_resize', 'arguments': {'node-name': 'file', 'size':
> $size}}
> +{'execute': 'quit'}
> +EOF
> +}
> +
> # get standard environment, filters and checks
> . ./common.rc
> . ./common.filter
> @@ -79,6 +96,14 @@ for mode in off full falloc; do
> stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks
> $min_blocks $size
> done
>
> +for new_size in 4096 1048576; do
> +echo
> +echo "== resize empty image with block_resize =="
> +_make_test_img 0 | _filter_imgfmt
> +_block_resize $TEST_IMG $new_size >/dev/null
> +stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks
> $min_blocks $new_size
> +done
> +
> # success, all done
> echo "*** done"
> rm -f $seq.full
> diff --git a/tests/qemu-iotests/175.out b/tests/qemu-iotests/175.out
> index 263e521262..39c2ee0f62 100644
> --- a/tests/qemu-iotests/175.out
> +++ b/tests/qemu-iotests/175.out
> @@ -15,4 +15,12 @@ size=1048576, max allocation
> == creating image with preallocation falloc ==
> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
> preallocation=falloc
> size=1048576, max allocation
> +
> +== resize empty image with block_resize ==
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=0
> +size=4096, min allocation
> +
> +== resize empty image with block_resize ==
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=0
> +size=1048576, min allocation
> *** done
> --
> 2.20.1
>
>
Re: [Qemu-devel] [PATCH v2 0/2] Optimize alignment probing
Patchew URL: https://patchew.org/QEMU/20190825220329.7942-1-nsof...@redhat.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Subject: [Qemu-devel] [PATCH v2 0/2] Optimize alignment probing Message-id: 20190825220329.7942-1-nsof...@redhat.com === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu - [tag update] patchew/20190819213755.26175-1-richard.hender...@linaro.org -> patchew/20190819213755.26175-1-richard.hender...@linaro.org * [new tag] patchew/20190825220329.7942-1-nsof...@redhat.com -> patchew/20190825220329.7942-1-nsof...@redhat.com Submodule 'capstone' (https://git.qemu.org/git/capstone.git) registered for path 'capstone' Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc' Submodule 'roms/QemuMacDrivers' (https://git.qemu.org/git/QemuMacDrivers.git) registered for path 'roms/QemuMacDrivers' Submodule 'roms/SLOF' (https://git.qemu.org/git/SLOF.git) registered for path 'roms/SLOF' Submodule 'roms/edk2' (https://git.qemu.org/git/edk2.git) registered for path 'roms/edk2' Submodule 'roms/ipxe' (https://git.qemu.org/git/ipxe.git) registered for path 'roms/ipxe' Submodule 'roms/openbios' (https://git.qemu.org/git/openbios.git) registered for path 'roms/openbios' Submodule 'roms/openhackware' (https://git.qemu.org/git/openhackware.git) registered for path 'roms/openhackware' Submodule 'roms/opensbi' (https://git.qemu.org/git/opensbi.git) registered for path 'roms/opensbi' Submodule 'roms/qemu-palcode' (https://git.qemu.org/git/qemu-palcode.git) registered for path 'roms/qemu-palcode' Submodule 'roms/seabios' (https://git.qemu.org/git/seabios.git/) registered for path 'roms/seabios' Submodule 'roms/seabios-hppa' (https://git.qemu.org/git/seabios-hppa.git) registered for path 'roms/seabios-hppa' Submodule 'roms/sgabios' (https://git.qemu.org/git/sgabios.git) registered for path 'roms/sgabios' Submodule 'roms/skiboot' (https://git.qemu.org/git/skiboot.git) registered for path 'roms/skiboot' Submodule 'roms/u-boot' (https://git.qemu.org/git/u-boot.git) registered for path 'roms/u-boot' Submodule 'roms/u-boot-sam460ex' (https://git.qemu.org/git/u-boot-sam460ex.git) registered for path 'roms/u-boot-sam460ex' Submodule 'slirp' (https://git.qemu.org/git/libslirp.git) registered for path 'slirp' Submodule 'tests/fp/berkeley-softfloat-3' (https://git.qemu.org/git/berkeley-softfloat-3.git) registered for path 'tests/fp/berkeley-softfloat-3' Submodule 'tests/fp/berkeley-testfloat-3' (https://git.qemu.org/git/berkeley-testfloat-3.git) registered for path 'tests/fp/berkeley-testfloat-3' Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) registered for path 'ui/keycodemapdb' Cloning into 'capstone'... Submodule path 'capstone': checked out '22ead3e0bfdb87516656453336160e0a37b066bf' Cloning into 'dtc'... Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536' Cloning into 'roms/QemuMacDrivers'... Submodule path 'roms/QemuMacDrivers': checked out '90c488d5f4a407342247b9ea869df1c2d9c8e266' Cloning into 'roms/SLOF'... Submodule path 'roms/SLOF': checked out '7bfe584e321946771692711ff83ad2b5850daca7' Cloning into 'roms/edk2'... Submodule path 'roms/edk2': checked out '20d2e5a125e34fc8501026613a71549b2a1a3e54' Submodule 'SoftFloat' (https://github.com/ucb-bar/berkeley-softfloat-3.git) registered for path 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3' Submodule 'CryptoPkg/Library/OpensslLib/openssl' (https://github.com/openssl/openssl) registered for path 'CryptoPkg/Library/OpensslLib/openssl' Cloning into 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3'... Submodule path 'roms/edk2/ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3': checked out 'b64af41c3276f97f0e181920400ee056b9c88037' Cloning into 'CryptoPkg/Library/OpensslLib/openssl'... Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl': checked out '50eaac9f3337667259de725451f201e784599687' Submodule 'boringssl' (https://boringssl.googlesource.com/boringssl) registered for path 'boringssl' Submodule 'krb5' (https://github.com/krb5/krb5) registered for path 'krb5' Submodule 'pyca.cryptography' (https://github.com/pyca/cryptography.git) registered for path 'pyca-cryptography' Cloning into 'boringssl'... Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/boringssl': checked out '2070f8ad9151dc8f3a73bffaa146b5e6937a583f' Cloning into 'krb5'... Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/krb5': checked out 'b9ad6c49505c96a088326b62a52568e3484f2168' Cloning into 'pyca-cryptography'... Submodule path
[Qemu-devel] [PATCH] target/xtensa: linux-user: add call0 ABI support
Xtensa binaries built for call0 ABI don't rotate register window on
function calls and returns. Invocation of signal handlers from the
kernel is therefore different in windowed and call0 ABIs.
There's currently no way to determine xtensa ELF binary ABI from the
binary itself. Provide an environment variable QEMU_XTENSA_ABI_CALL0 and
use it to initialize PS.WOE in xtensa_cpu_reset. Check this flag in
setup_rt_frame to determine how a signal should be delivered.
Signed-off-by: Max Filippov
---
linux-user/xtensa/signal.c | 25 +
target/xtensa/cpu.c| 22 ++
2 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/linux-user/xtensa/signal.c b/linux-user/xtensa/signal.c
index 8d54ef3ae34b..590f0313ffe9 100644
--- a/linux-user/xtensa/signal.c
+++ b/linux-user/xtensa/signal.c
@@ -134,6 +134,8 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
abi_ulong frame_addr;
struct target_rt_sigframe *frame;
uint32_t ra;
+bool abi_call0;
+unsigned base;
int i;
frame_addr = get_sigframe(ka, env, sizeof(*frame));
@@ -182,20 +184,27 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
__put_user(0x00, >retcode[5]);
#endif
}
-env->sregs[PS] = PS_UM | (3 << PS_RING_SHIFT);
-if (xtensa_option_enabled(env->config, XTENSA_OPTION_WINDOWED_REGISTER)) {
-env->sregs[PS] |= PS_WOE | (1 << PS_CALLINC_SHIFT);
-}
memset(env->regs, 0, sizeof(env->regs));
env->pc = ka->_sa_handler;
env->regs[1] = frame_addr;
env->sregs[WINDOW_BASE] = 0;
env->sregs[WINDOW_START] = 1;
-env->regs[4] = (ra & 0x3fff) | 0x4000;
-env->regs[6] = sig;
-env->regs[7] = frame_addr + offsetof(struct target_rt_sigframe, info);
-env->regs[8] = frame_addr + offsetof(struct target_rt_sigframe, uc);
+abi_call0 = (env->sregs[PS] & PS_WOE) == 0;
+env->sregs[PS] = PS_UM | (3 << PS_RING_SHIFT);
+
+if (abi_call0) {
+base = 0;
+env->regs[base] = ra;
+} else {
+env->sregs[PS] |= PS_WOE | (1 << PS_CALLINC_SHIFT);
+base = 4;
+env->regs[base] = (ra & 0x3fff) | 0x4000;
+}
+env->regs[base + 2] = sig;
+env->regs[base + 3] = frame_addr + offsetof(struct target_rt_sigframe,
+info);
+env->regs[base + 4] = frame_addr + offsetof(struct target_rt_sigframe, uc);
unlock_user_struct(frame, frame_addr, 1);
return;
diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
index 76db1741a796..791c061880e7 100644
--- a/target/xtensa/cpu.c
+++ b/target/xtensa/cpu.c
@@ -53,6 +53,18 @@ static bool xtensa_cpu_has_work(CPUState *cs)
#endif
}
+#ifdef CONFIG_USER_ONLY
+static int xtensa_abi_call0(void)
+{
+static int abi_call0 = -1;
+
+if (abi_call0 == -1) {
+abi_call0 = getenv("QEMU_XTENSA_ABI_CALL0") != NULL;
+}
+return abi_call0;
+}
+#endif
+
/* CPUClass::reset() */
static void xtensa_cpu_reset(CPUState *s)
{
@@ -70,10 +82,12 @@ static void xtensa_cpu_reset(CPUState *s)
XTENSA_OPTION_INTERRUPT) ? 0x1f : 0x10;
env->pending_irq_level = 0;
#else
-env->sregs[PS] =
-(xtensa_option_enabled(env->config,
- XTENSA_OPTION_WINDOWED_REGISTER) ? PS_WOE : 0) |
-PS_UM | (3 << PS_RING_SHIFT);
+env->sregs[PS] = PS_UM | (3 << PS_RING_SHIFT);
+if (xtensa_option_enabled(env->config,
+ XTENSA_OPTION_WINDOWED_REGISTER) &&
+!xtensa_abi_call0()) {
+env->sregs[PS] |= PS_WOE;
+}
#endif
env->sregs[VECBASE] = env->config->vecbase;
env->sregs[IBREAKENABLE] = 0;
--
2.11.0
Re: [Qemu-devel] [Qemu-block] [PATCH] block: posix: Always allocate the first block
On Sun, 2019-08-25 at 22:51 +0300, Nir Soffer wrote: > On Sun, Aug 25, 2019 at 10:44 AM Maxim Levitsky wrote: > > On Sat, 2019-08-17 at 00:21 +0300, Nir Soffer wrote: > > > When creating an image with preallocation "off" or "falloc", the first > > > block of the image is typically not allocated. When using Gluster > > > storage backed by XFS filesystem, reading this block using direct I/O > > > succeeds regardless of request length, fooling alignment detection. > > > > > > In this case we fallback to a safe value (4096) instead of the optimal > > > value (512), which may lead to unneeded data copying when aligning > > > requests. Allocating the first block avoids the fallback. > > > > > > When using preallocation=off, we always allocate at least one filesystem > > > block: > > > > > > $ ./qemu-img create -f raw test.raw 1g > > > Formatting 'test.raw', fmt=raw size=1073741824 > > > > > > $ ls -lhs test.raw > > > 4.0K -rw-r--r--. 1 nsoffer nsoffer 1.0G Aug 16 23:48 test.raw > > > > Are you sure about this? > > This is the new behaviour with this change... > > > [mlevitsk@maximlenovopc ~/work/test_area/posix-file 0]$ qemu-img create -f > > raw test.raw 1g -o preallocation=off > > Formatting 'test.raw', fmt=raw size=1073741824 preallocation=off > > [mlevitsk@maximlenovopc ~/work/test_area/posix-file 0]$ls -lhs ./test.raw > > 0 -rw-r--r--. 1 mlevitsk mlevitsk 1.0G Aug 25 10:38 ./test.raw > > > > ext4, tested on qemu-4.0.0 and qemu git master. > > And this is the old behavior. I guess the commit message does not make it > clear. Ah, thanks! > > From what I remember, the only case when posix-raw touches the first block > > is to zero it out > > when running on top of kernel block device, to erase whatever header might > > be there, and this > > is also kind of a backward compat hack which might be one day removed. > > This change is only for file, on block storage we use BLKSSZGET. > > > [...] > > > > Best regards, > > Maxim Levitsky > > > > Best regards, Maxim Levitsky
Re: [Qemu-devel] [PULL 07/15] audio: audiodev= parameters no longer optional when -audiodev present
On Sun, 2019-08-25 at 20:05 +0200, Zoltán Kővágó wrote: > On 2019-08-25 11:44, Maxim Levitsky wrote: > > On Wed, 2019-08-21 at 10:41 +0200, Gerd Hoffmann wrote: > > > From: Kővágó, Zoltán > > > > > > This means you should probably stop using -soundhw (as it doesn't allow > > > you to specify any options) and add the device manually with -device. > > > The exception is pcspk, it's currently not possible to manually add it. > > > To use it with audiodev, use something like this: > > > > > > -audiodev id=foo,... -global isa-pcspk.audiodev=foo -soundhw pcspk > > > > Hi! > > Hi, > > > There is one corner case this breaks. > > In qemu 4.1.0, there is no way to specify audiodev for a sound device, > > specifying it > > fails with error. > > So some of my machines have audiodev (which is miles better that using old > > env variables) > > but also have sound devices without audiodev reference since this wasn't > > supported. > > > > > > In what will be qemu 4.2, you must specify it, thus this kind of breaks > > backward compatibility. > > Maybe we can have audiodev reference optional for a version or two? > > > > This is just a minor itch, as otherwise the sound improvements are really > > good. The days > > of installing that old realtek driver are finally gone :-) > > Hmm, this is what happens when you split a patch series. We could > either revert this patch, or alternatively turn the error messages into > warnings about using deprecated behavior. Warning would be great in this case! > > > Another thing I noted, that there is no way for pulseaudio audiodev to > > specify the 'client name', > > it always shows up in pavucontrl as the socket path to the server. > > Thus if I added two PA audiodevs, I can't really distinguish between them. > > The in|out.name= seems to specify the pulseaudio source/sink to connect to, > > which is not the same. > > We currently supply the constant "qemu" as a name to pa_stream_new. > While it's still not ideal, shouldn't this end up as a client name in > pulseaudio instead of a socket path? Actually it seems that pulseaudio has two names supplied for each stream Maybe stream name and application name? This is how chromium playback looks versus qemu in pavucontrol and in gnome volume control. https://imgur.com/a/I8HZhgx I do notice that 'qemu' now, in pavucontrol though. Best regards, Maxim Levitsky > > Regards, > Zoltan
[Qemu-devel] [PATCH v2 2/2] iotests: Test allocate_first_block() with O_DIRECT
Using block_resize we can test allocate_first_block() with file
descriptor opened with O_DIRECT, ensuring that it works for any size
larger than 4096 bytes.
Testing smaller sizes is tricky as the result depends on the filesystem
used for testing. For example on NFS any size will work since O_DIRECT
does not require any alignment.
---
tests/qemu-iotests/175 | 25 +
tests/qemu-iotests/175.out | 8
2 files changed, 33 insertions(+)
diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175
index d54cb43c39..60cc251eb2 100755
--- a/tests/qemu-iotests/175
+++ b/tests/qemu-iotests/175
@@ -49,6 +49,23 @@ _filter_blocks()
-e "s/blocks=$((extra_blocks + img_size / 512))\\(\$\\|[^0-9]\\)/max
allocation/"
}
+# Resize image using block_resize.
+# Parameter 1: image path
+# Parameter 2: new size
+_block_resize()
+{
+local path=$1
+local size=$2
+
+$QEMU -qmp stdio -nographic -nodefaults \
+-blockdev file,node-name=file,filename=$path,cache.direct=on \
+<
[Qemu-devel] [PATCH v2 0/2] Optimize alignment probing
When probing unallocated area on XFS filesystem we cannot detect request
alignment and we fallback to safe value which may not be optimal. Avoid this
fallback by always allocating the first block when creating a new image or
resizing empty image.
I tested v1 only with -raw format, and missed some changes in qcow2
tests creating raw images during the tests. This time I tested with both
-raw and -qcow2.
Changes in v2:
- Support file descriptor opened with O_DIRECT (e.g. in block_resize) (Max)
- Remove unneeded change in 160 (Max)
- Fix block filter in 175 on filesystem allocating extra blocks (Max)
- Comment why we ignore errors in allocte_first_block() (Max)
- Comment why allocate_first_block() is needed in FALLOC mode (Max)
- Clarify commit message about user visible changes (Maxim)
- Fix 178.out.qcow2
- Fix 150.out with -qcow2 by spliting to 150.out.{raw,qcow2}
- Add test for allocate_first_block() with block_resize (Max)
- Drop provisioing tests results since I ran them only once
v1 was here:
https://lists.nongnu.org/archive/html/qemu-block/2019-08/msg00821.html
Nir Soffer (2):
block: posix: Always allocate the first block
iotests: Test allocate_first_block() with O_DIRECT
block/file-posix.c| 43 ++
tests/qemu-iotests/{150.out => 150.out.qcow2} | 0
tests/qemu-iotests/150.out.raw| 12 +
tests/qemu-iotests/175| 44 ---
tests/qemu-iotests/175.out| 16 +--
tests/qemu-iotests/178.out.qcow2 | 4 +-
tests/qemu-iotests/221.out| 12 +++--
tests/qemu-iotests/253.out| 12 +++--
8 files changed, 123 insertions(+), 20 deletions(-)
rename tests/qemu-iotests/{150.out => 150.out.qcow2} (100%)
create mode 100644 tests/qemu-iotests/150.out.raw
--
2.20.1
[Qemu-devel] [PATCH v2 1/2] block: posix: Always allocate the first block
When creating an image with preallocation "off" or "falloc", the first
block of the image is typically not allocated. When using Gluster
storage backed by XFS filesystem, reading this block using direct I/O
succeeds regardless of request length, fooling alignment detection.
In this case we fallback to a safe value (4096) instead of the optimal
value (512), which may lead to unneeded data copying when aligning
requests. Allocating the first block avoids the fallback.
Since we allocate the first block even with preallocation=off, we no
longer create images with zero disk size:
$ ./qemu-img create -f raw test.raw 1g
Formatting 'test.raw', fmt=raw size=1073741824
$ ls -lhs test.raw
4.0K -rw-r--r--. 1 nsoffer nsoffer 1.0G Aug 16 23:48 test.raw
And converting the image requires additional cluster:
$ ./qemu-img measure -f raw -O qcow2 test.raw
required size: 458752
fully allocated size: 1074135040
I did quick performance test for copying disks with qemu-img convert to
new raw target image to Gluster storage with sector size of 512 bytes:
for i in $(seq 10); do
rm -f dst.raw
sleep 10
time ./qemu-img convert -f raw -O raw -t none -T none src.raw dst.raw
done
Here is a table comparing the total time spent:
TypeBefore(s) After(s)Diff(%)
---
real 530.028469.123 -11.4
user 17.204 10.768 -37.4
sys17.881 7.011 -60.7
We can see very clear improvement in CPU usage.
Signed-off-by: Nir Soffer
---
block/file-posix.c| 43 +++
tests/qemu-iotests/{150.out => 150.out.qcow2} | 0
tests/qemu-iotests/150.out.raw| 12 ++
tests/qemu-iotests/175| 19 +---
tests/qemu-iotests/175.out| 8 ++--
tests/qemu-iotests/178.out.qcow2 | 4 +-
tests/qemu-iotests/221.out| 12 --
tests/qemu-iotests/253.out| 12 --
8 files changed, 90 insertions(+), 20 deletions(-)
rename tests/qemu-iotests/{150.out => 150.out.qcow2} (100%)
create mode 100644 tests/qemu-iotests/150.out.raw
diff --git a/block/file-posix.c b/block/file-posix.c
index fbeb0068db..51688ae3fc 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1749,6 +1749,39 @@ static int handle_aiocb_discard(void *opaque)
return ret;
}
+/*
+ * Help alignment probing by allocating the first block.
+ *
+ * When reading with direct I/O from unallocated area on Gluster backed by XFS,
+ * reading succeeds regardless of request length. In this case we fallback to
+ * safe alignment which is not optimal. Allocating the first block avoids this
+ * fallback.
+ *
+ * fd may be opened with O_DIRECT, but we don't know the buffer alignment or
+ * request alignment, so we use safe values.
+ *
+ * Returns: 0 on success, -errno on failure. Since this is an optimization,
+ * caller may ignore failures.
+ */
+static int allocate_first_block(int fd, size_t max_size)
+{
+size_t write_size = MIN(MAX_BLOCKSIZE, max_size);
+size_t max_align = MAX(MAX_BLOCKSIZE, getpagesize());
+void *buf;
+ssize_t n;
+
+buf = qemu_memalign(max_align, write_size);
+memset(buf, 0, write_size);
+
+do {
+n = pwrite(fd, buf, write_size, 0);
+} while (n == -1 && errno == EINTR);
+
+qemu_vfree(buf);
+
+return (n == -1) ? -errno : 0;
+}
+
static int handle_aiocb_truncate(void *opaque)
{
RawPosixAIOData *aiocb = opaque;
@@ -1788,6 +1821,13 @@ static int handle_aiocb_truncate(void *opaque)
/* posix_fallocate() doesn't set errno. */
error_setg_errno(errp, -result,
"Could not preallocate new data");
+} else if (current_length == 0) {
+/*
+ * Needed only if posix_fallocate() used fallocate(), but we
+ * don't have a way to detect that. Optimize future alignment
+ * probing; ignore failures.
+ */
+allocate_first_block(fd, offset);
}
} else {
result = 0;
@@ -1849,6 +1889,9 @@ static int handle_aiocb_truncate(void *opaque)
if (ftruncate(fd, offset) != 0) {
result = -errno;
error_setg_errno(errp, -result, "Could not resize file");
+} else if (current_length == 0 && offset > current_length) {
+/* Optimize future alignment probing; ignore failures. */
+allocate_first_block(fd, offset);
}
return result;
default:
diff --git a/tests/qemu-iotests/150.out b/tests/qemu-iotests/150.out.qcow2
similarity index 100%
rename from tests/qemu-iotests/150.out
rename to tests/qemu-iotests/150.out.qcow2
diff --git a/tests/qemu-iotests/150.out.raw b/tests/qemu-iotests/150.out.raw
new file mode 100644
index 00..3cdc7727a5
--- /dev/null
+++
Re: [Qemu-devel] [PATCH v2 53/68] target/arm: Convert T16 add, compare, move (two high registers)
On Mon, 19 Aug 2019 at 22:39, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/translate.c | 49 ++ > target/arm/t16.decode | 10 + > 2 files changed, 12 insertions(+), 47 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH v2 52/68] target/arm: Convert T16 branch and exchange
On Mon, 19 Aug 2019 at 22:39, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH v2 51/68] target/arm: Convert T16 one low register and immediate
On Mon, 19 Aug 2019 at 22:38, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH v2 50/68] target/arm: Convert T16 add/sub (3 low, 2 low and imm)
On Mon, 19 Aug 2019 at 22:38, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/translate.c | 26 ++ > target/arm/t16.decode | 16 > 2 files changed, 18 insertions(+), 24 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH v2 49/68] target/arm: Convert T16 load/store multiple
On Mon, 19 Aug 2019 at 22:38, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/translate.c | 48 -- > target/arm/t16.decode | 8 +++ > 2 files changed, 17 insertions(+), 39 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH v2 48/68] target/arm: Convert T16 add pc/sp (immediate)
On Mon, 19 Aug 2019 at 22:38, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/translate.c | 12 +--- > target/arm/t16.decode | 7 +++ > 2 files changed, 8 insertions(+), 11 deletions(-) > Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH v2 47/68] target/arm: Convert T16 load/store (immediate offset)
On Mon, 19 Aug 2019 at 22:38, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/translate.c | 94 +++--- > target/arm/t16.decode | 33 +++ > 2 files changed, 38 insertions(+), 89 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH v2 46/68] target/arm: Convert T16 load/store (register offset)
On Mon, 19 Aug 2019 at 22:38, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/translate.c | 51 ++ > target/arm/t16.decode | 15 + > 2 files changed, 17 insertions(+), 49 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH v2 45/68] target/arm: Convert T16 data-processing (two low regs)
On Mon, 19 Aug 2019 at 22:38, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/translate.c | 152 ++--- > target/arm/t16.decode | 36 ++ > 2 files changed, 43 insertions(+), 145 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH v2 35/68] target/arm: Convert CPS (privileged)
On Sun, 25 Aug 2019 at 18:28, Richard Henderson wrote: > > On 8/25/19 9:20 AM, Peter Maydell wrote: > > On Mon, 19 Aug 2019 at 22:38, Richard Henderson > > wrote: > >> > >> Signed-off-by: Richard Henderson > >> --- > >> target/arm/translate.c | 87 +++- > >> target/arm/a32-uncond.decode | 3 ++ > >> target/arm/t32.decode| 3 ++ > >> 3 files changed, 42 insertions(+), 51 deletions(-) > >> diff --git a/target/arm/t32.decode b/target/arm/t32.decode > >> index 18c268e712..354ad77fe6 100644 > >> --- a/target/arm/t32.decode > >> +++ b/target/arm/t32.decode > >> @@ -44,6 +44,7 @@ > >> !extern rd rn lsb msb > >> !extern rd rn satimm imm sh > >> !extern rd rn rm imm tb > >> + !extern mode imod M A I F > >> > >> # Data-processing (register) > >> > >> @@ -340,6 +341,8 @@ CLZ 1010 1011 1000 > >> @rdm > >> SMC 0111 imm:4 1000 > >> HVC 0111 1110 1000 \ > >>imm=%imm16_16_0 > >> +CPS 0011 1010 1000 0 imod:2 M:1 A:1 I:1 F:1 mode:5 > >> \ > >> + > > > > In T32 the CPS insn overlaps with the hint space (hint insns have > > bits [10:8] all-zeroes, whereas all the valid CPS insns have either > > M set or one of the imod bits set) -- why doesn't it need to be > > in the same insn group as the hints? If we're going to put it > > separated in the .decode file from the insns it overlaps with > > I guess a comment to that effect would help so it doesn't get > > inadvertently reordered with them. > > It is grouped. It's not immediately visible in the patch because there are a > *lot* of insns that overlap with the hints and 3 lines of context are > insufficient to see that. > > But the grouping is semi-visible in the indentation here. I'm still confused, I think. The hint space is + NOP 0011 1010 1000 (plus the more specific hint insns before that pattern with fixed values in the [7:0] bits). CPS falls into that space; but you've placed it with SMC and HVC which don't fall into the hint space, because they have 0111 in bits [27:24], not 0011. thanks -- PMM
Re: [Qemu-devel] [Qemu-block] [PATCH] block: posix: Always allocate the first block
On Sun, Aug 25, 2019 at 10:44 AM Maxim Levitsky wrote: > On Sat, 2019-08-17 at 00:21 +0300, Nir Soffer wrote: > > When creating an image with preallocation "off" or "falloc", the first > > block of the image is typically not allocated. When using Gluster > > storage backed by XFS filesystem, reading this block using direct I/O > > succeeds regardless of request length, fooling alignment detection. > > > > In this case we fallback to a safe value (4096) instead of the optimal > > value (512), which may lead to unneeded data copying when aligning > > requests. Allocating the first block avoids the fallback. > > > > When using preallocation=off, we always allocate at least one filesystem > > block: > > > > $ ./qemu-img create -f raw test.raw 1g > > Formatting 'test.raw', fmt=raw size=1073741824 > > > > $ ls -lhs test.raw > > 4.0K -rw-r--r--. 1 nsoffer nsoffer 1.0G Aug 16 23:48 test.raw > > Are you sure about this? > This is the new behaviour with this change... [mlevitsk@maximlenovopc ~/work/test_area/posix-file 0]$ qemu-img create -f > raw test.raw 1g -o preallocation=off > Formatting 'test.raw', fmt=raw size=1073741824 preallocation=off > [mlevitsk@maximlenovopc ~/work/test_area/posix-file 0]$ls -lhs ./test.raw > 0 -rw-r--r--. 1 mlevitsk mlevitsk 1.0G Aug 25 10:38 ./test.raw > > ext4, tested on qemu-4.0.0 and qemu git master. > And this is the old behavior. I guess the commit message does not make it clear. >From what I remember, the only case when posix-raw touches the first block > is to zero it out > when running on top of kernel block device, to erase whatever header might > be there, and this > is also kind of a backward compat hack which might be one day removed. > This change is only for file, on block storage we use BLKSSZGET. > > [...] > > Best regards, > Maxim Levitsky > > >
[Qemu-devel] [PULL 0/1] target/alpha queue
The following changes since commit 586f3dced9f2b354480c140c070a3d02a0c66a1e: Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20190822' into staging (2019-08-23 15:15:44 +0100) are available in the Git repository at: https://github.com/rth7680/qemu.git tags/pull-axp-20190825 for you to fetch changes up to cb1de55a83eaca9ee32be9c959dca99e11f2fea8: target/alpha: fix tlb_fill trap_arg2 value for instruction fetch (2019-08-25 12:30:48 -0700) Fix for alpha_cpu_tlb_fill Aurelien Jarno (1): target/alpha: fix tlb_fill trap_arg2 value for instruction fetch target/alpha/helper.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
[Qemu-devel] [PULL 1/1] target/alpha: fix tlb_fill trap_arg2 value for instruction fetch
From: Aurelien Jarno
Commit e41c94529740cc26 ("target/alpha: Convert to CPUClass::tlb_fill")
slightly changed the way the trap_arg2 value is computed in case of TLB
fill. The type of the variable used in the ternary operator has been
changed from an int to an enum. This causes the -1 value to not be
sign-extended to 64-bit in case of an instruction fetch. The trap_arg2
ends up with 0x instead of 0x. Fix that by
changing the -1 into -1LL.
This fixes the execution of user space processes in qemu-system-alpha.
Fixes: e41c94529740cc26
Cc: qemu-sta...@nongnu.org
Signed-off-by: Aurelien Jarno
[rth: Test MMU_DATA_LOAD and MMU_DATA_STORE instead of implying them.]
Signed-off-by: Richard Henderson
---
target/alpha/helper.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/target/alpha/helper.c b/target/alpha/helper.c
index c6998348df..19cda0a2db 100644
--- a/target/alpha/helper.c
+++ b/target/alpha/helper.c
@@ -283,7 +283,9 @@ bool alpha_cpu_tlb_fill(CPUState *cs, vaddr addr, int size,
cs->exception_index = EXCP_MMFAULT;
env->trap_arg0 = addr;
env->trap_arg1 = fail;
-env->trap_arg2 = (access_type == MMU_INST_FETCH ? -1 : access_type);
+env->trap_arg2 = (access_type == MMU_DATA_LOAD ? 0ull :
+ access_type == MMU_DATA_STORE ? 1ull :
+ /* access_type == MMU_INST_FETCH */ -1ull);
cpu_loop_exit_restore(cs, retaddr);
}
--
2.17.1
[Qemu-devel] [GSOC] Support for AVX within TCG: Work Product Submission
Hi folks, those of you who have been keeping up with Google Summer of Code this year might know that it's nearly over -- meaning that it's time for me to summarize all the work that I have done as a participant. Without further ado, you can find the summary attached below. Huge thanks to everyone who made this possible! Best, -Jan Bobek GSOC WORK PRODUCT SUBMISSION TITLE: Support for AVX within TCG DATE: 08/25/2019 AUTHOR: Jan Bobek MENTOR: Richard Henderson I. SUMMARY The goal of this GSoC project was to implement support for AVX instructions in the i386 TCG front-end. The project was effectively split up into two parts: - extending RISU [1] with the ability to generate and test x86 instruction streams (to properly test the new AVX implementation); - the actual work on AVX instructions in the i386 TCG front-end. II. RISU CHANGES There have been two patch series with changes to RISU: - The first one [2] adds support for testing x86 instruction sequences, and has been merged as early as June 7, 2019. - The second series [3] implements generation of x86 instruction streams using vector instructions up to AVX2; its latest iteration has not been merged yet due to several more or less minor issues raised during code review. Despite the second series not being merged yet, the implemented functionality was considered sufficient and a decision was made to proceed to the next stage of the project [4]. III. QEMU CHANGES All QEMU changes related to this project are included in a single extensive patch series [5]; previous iterations can be found at [6], [7] and [8]. The series features: - brand-new infrastructure for instruction decoding; - support for decoding vector instructions up to AVX2. Work that remains to be done includes: - rewrite of old ad-hoc helpers into gvec-style helpers; - implementation of helpers for previously unsupported AVX instructions (e.g. VGATHER et al.) IV. FINAL NOTES Even though I could not completely finish the project in the allotted time, I had a great time working on it, and I am planning to do the rest of the work in my free time over the coming weeks. Needless to say, I learned a lot in the process (especially about the x86 ISA), and I am very thankful to the GSoC program for making it all possible. Last but definitely not least, I would like to thank my mentor, Richard Henderson, for his support and assistance throughout this project. Your insights during code reviews have been indispensable, and I would not have been able to make as much progress on this project without your guidance. Thank you, Richard! REFERENCES 1. http://git.linaro.org/people/peter.maydell/risu.git/ 2. https://lists.nongnu.org/archive/html/qemu-devel/2019-05/msg05720.html 3. https://lists.nongnu.org/archive/html/qemu-devel/2019-07/msg02916.html 4. https://lists.nongnu.org/archive/html/qemu-devel/2019-07/msg04758.html 5. https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg04412.html 6. https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg02616.html 7. https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg01790.html 8. https://lists.nongnu.org/archive/html/qemu-devel/2019-07/msg07041.html signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH 22/25] paaudio: channel-map option
Add an option to change the channel map used by pulseaudio. If not
specified, falls back to an OSS compatible channel map.
Signed-off-by: Kővágó, Zoltán
---
qapi/audio.json | 7 +--
audio/paaudio.c | 18 ++
qemu-options.hx | 9 +
3 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/qapi/audio.json b/qapi/audio.json
index dc7f9cb1e2..e9e040a156 100644
--- a/qapi/audio.json
+++ b/qapi/audio.json
@@ -214,13 +214,16 @@
# @latency: latency you want PulseAudio to achieve in microseconds
# (default 15000)
#
+# @channel-map: channel map to use (default: OSS compatible map, since: 4.2)
+#
# Since: 4.0
##
{ 'struct': 'AudiodevPaPerDirectionOptions',
'base': 'AudiodevPerDirectionOptions',
'data': {
-'*name': 'str',
-'*latency': 'uint32' } }
+'*name':'str',
+'*latency': 'uint32',
+'*channel-map': 'str' } }
##
# @AudiodevPaOptions:
diff --git a/audio/paaudio.c b/audio/paaudio.c
index 796890a3a5..4ce4f03c72 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -338,17 +338,27 @@ static pa_stream *qpa_simple_new (
pa_stream_direction_t dir,
const char *dev,
const pa_sample_spec *ss,
-const pa_channel_map *map,
+const char *map,
const pa_buffer_attr *attr,
int *rerror)
{
int r;
pa_stream *stream;
pa_stream_flags_t flags;
+pa_channel_map pa_map;
pa_threaded_mainloop_lock(c->mainloop);
-stream = pa_stream_new(c->context, name, ss, map);
+if (map && !pa_channel_map_parse(_map, map)) {
+dolog("Invalid channel map specified: '%s'\n", map);
+map = NULL;
+}
+if (!map) {
+pa_channel_map_init_extend(_map, ss->channels,
+ PA_CHANNEL_MAP_OSS);
+}
+
+stream = pa_stream_new(c->context, name, ss, _map);
if (!stream) {
goto fail;
}
@@ -421,7 +431,7 @@ static int qpa_init_out(HWVoiceOut *hw, struct audsettings
*as,
PA_STREAM_PLAYBACK,
ppdo->has_name ? ppdo->name : NULL,
,
-NULL, /* channel map */
+ppdo->has_channel_map ? ppdo->channel_map : NULL,
,/* buffering attributes */
);
@@ -477,7 +487,7 @@ static int qpa_init_in(HWVoiceIn *hw, struct audsettings
*as, void *drv_opaque)
PA_STREAM_RECORD,
ppdo->has_name ? ppdo->name : NULL,
,
-NULL, /* channel map */
+ppdo->has_channel_map ? ppdo->channel_map : NULL,
,/* buffering attributes */
);
diff --git a/qemu-options.hx b/qemu-options.hx
index 8de6deb691..4eb4d6fe6d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -471,6 +471,7 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
"-audiodev pa,id=id[,prop[=value][,...]]\n"
"server= PulseAudio server address\n"
"in|out.name= source/sink device name\n"
+"in|out.channel-map= channel map to use\n"
#endif
#ifdef CONFIG_AUDIO_SDL
"-audiodev sdl,id=id[,prop[=value][,...]]\n"
@@ -636,6 +637,14 @@ Sets the PulseAudio @var{server} to connect to.
@item in|out.name=@var{sink}
Use the specified source/sink for recording/playback.
+@item in|out.channel-map=@var{map}
+Use the specified channel map. The default is an OSS compatible
+channel map. Do not forget to escape commas inside the map:
+
+@example
+-audiodev pa,id=example,sink.channel-map=front-left,,front-right
+@end example
+
@end table
@item -audiodev sdl,id=@var{id}[,@var{prop}[=@var{value}][,...]]
--
2.22.0
[Qemu-devel] [PATCH 19/25] audio: support more than two channels in volume setting
Signed-off-by: Kővágó, Zoltán
---
audio/audio.h | 10 ++
audio/audio_int.h | 4 ++--
audio/audio.c | 30 ++
audio/paaudio.c| 20
audio/spiceaudio.c | 14 --
5 files changed, 54 insertions(+), 24 deletions(-)
diff --git a/audio/audio.h b/audio/audio.h
index c74abb8c47..0db3c7dd5e 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -124,6 +124,16 @@ uint64_t AUD_get_elapsed_usec_out (SWVoiceOut *sw,
QEMUAudioTimeStamp *ts);
void AUD_set_volume_out (SWVoiceOut *sw, int mute, uint8_t lvol, uint8_t rvol);
void AUD_set_volume_in (SWVoiceIn *sw, int mute, uint8_t lvol, uint8_t rvol);
+#define AUDIO_MAX_CHANNELS 16
+typedef struct Volume {
+bool mute;
+int channels;
+uint8_t vol[AUDIO_MAX_CHANNELS];
+} Volume;
+
+void audio_set_volume_out(SWVoiceOut *sw, Volume *vol);
+void audio_set_volume_in(SWVoiceIn *sw, Volume *vol);
+
SWVoiceIn *AUD_open_in (
QEMUSoundCard *card,
SWVoiceIn *sw,
diff --git a/audio/audio_int.h b/audio/audio_int.h
index 383e3b8b06..e826734f8c 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -166,7 +166,7 @@ struct audio_pcm_ops {
*/
size_t (*put_buffer_out)(HWVoiceOut *hw, void *buf, size_t size);
void (*enable_out)(HWVoiceOut *hw, bool enable);
-void (*volume_out)(HWVoiceOut *hw, struct mixeng_volume *vol);
+void (*volume_out)(HWVoiceOut *hw, Volume *vol);
int(*init_in) (HWVoiceIn *hw, audsettings *as, void *drv_opaque);
void (*fini_in) (HWVoiceIn *hw);
@@ -175,7 +175,7 @@ struct audio_pcm_ops {
void *(*get_buffer_in)(HWVoiceIn *hw, size_t *size);
void (*put_buffer_in)(HWVoiceIn *hw, void *buf, size_t size);
void (*enable_in)(HWVoiceIn *hw, bool enable);
-void (*volume_in)(HWVoiceIn *hw, struct mixeng_volume *vol);
+void (*volume_in)(HWVoiceIn *hw, Volume *vol);
};
void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size);
diff --git a/audio/audio.c b/audio/audio.c
index 73ace0500e..60a1d16dea 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1884,31 +1884,45 @@ void AUD_del_capture (CaptureVoiceOut *cap, void
*cb_opaque)
}
void AUD_set_volume_out (SWVoiceOut *sw, int mute, uint8_t lvol, uint8_t rvol)
+{
+Volume vol = { .mute = mute, .channels = 2, .vol = { lvol, rvol } };
+audio_set_volume_out(sw, );
+}
+
+void audio_set_volume_out(SWVoiceOut *sw, Volume *vol)
{
if (sw) {
HWVoiceOut *hw = sw->hw;
-sw->vol.mute = mute;
-sw->vol.l = nominal_volume.l * lvol / 255;
-sw->vol.r = nominal_volume.r * rvol / 255;
+sw->vol.mute = vol->mute;
+sw->vol.l = nominal_volume.l * vol->vol[0] / 255;
+sw->vol.r = nominal_volume.l * vol->vol[vol->channels > 1 ? 1 : 0] /
+255;
if (hw->pcm_ops->volume_out) {
-hw->pcm_ops->volume_out(hw, >vol);
+hw->pcm_ops->volume_out(hw, vol);
}
}
}
void AUD_set_volume_in (SWVoiceIn *sw, int mute, uint8_t lvol, uint8_t rvol)
+{
+Volume vol = { .mute = mute, .channels = 2, .vol = { lvol, rvol } };
+audio_set_volume_in(sw, );
+}
+
+void audio_set_volume_in(SWVoiceIn *sw, Volume *vol)
{
if (sw) {
HWVoiceIn *hw = sw->hw;
-sw->vol.mute = mute;
-sw->vol.l = nominal_volume.l * lvol / 255;
-sw->vol.r = nominal_volume.r * rvol / 255;
+sw->vol.mute = vol->mute;
+sw->vol.l = nominal_volume.l * vol->vol[0] / 255;
+sw->vol.r = nominal_volume.r * vol->vol[vol->channels > 1 ? 1 : 0] /
+255;
if (hw->pcm_ops->volume_in) {
-hw->pcm_ops->volume_in(hw, >vol);
+hw->pcm_ops->volume_in(hw, vol);
}
}
}
diff --git a/audio/paaudio.c b/audio/paaudio.c
index a87061bbcf..796890a3a5 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -544,20 +544,22 @@ static void qpa_fini_in (HWVoiceIn *hw)
}
}
-static void qpa_volume_out(HWVoiceOut *hw, struct mixeng_volume *vol)
+static void qpa_volume_out(HWVoiceOut *hw, Volume *vol)
{
PAVoiceOut *pa = (PAVoiceOut *) hw;
pa_operation *op;
pa_cvolume v;
PAConnection *c = pa->g->conn;
+int i;
#ifdef PA_CHECK_VERSION/* macro is present in 0.9.16+ */
pa_cvolume_init (); /* function is present in 0.9.13+ */
#endif
-v.channels = 2;
-v.values[0] = ((PA_VOLUME_NORM - PA_VOLUME_MUTED) * vol->l) / UINT32_MAX;
-v.values[1] = ((PA_VOLUME_NORM - PA_VOLUME_MUTED) * vol->r) / UINT32_MAX;
+v.channels = vol->channels;
+for (i = 0; i < vol->channels; ++i) {
+v.values[i] = ((PA_VOLUME_NORM - PA_VOLUME_MUTED) * vol->vol[i]) / 255;
+}
pa_threaded_mainloop_lock(c->mainloop);
@@ -584,20 +586,22 @@ static void qpa_volume_out(HWVoiceOut *hw, struct
mixeng_volume *vol)
pa_threaded_mainloop_unlock(c->mainloop);
}
-static void qpa_volume_in(HWVoiceIn *hw, struct mixeng_volume *vol)
+static void qpa_volume_in(HWVoiceIn
[Qemu-devel] [PATCH 15/25] audio: split ctl_* functions into enable_* and volume_*
This way we no longer need vararg functions, improving compile time
error detection. Also now it's possible to check actually what commands
are supported, without needing to manually update ctl_caps.
Signed-off-by: Kővágó, Zoltán
---
audio/audio_int.h | 15 ++---
audio/audio_template.h | 1 -
audio/alsaaudio.c | 62
audio/audio.c | 45 +--
audio/coreaudio.c | 13 ++---
audio/dsoundaudio.c| 50 +++-
audio/noaudio.c| 14 ++---
audio/ossaudio.c | 79 ++---
audio/paaudio.c| 127 -
audio/sdlaudio.c | 17 +-
audio/spiceaudio.c | 102 ++---
audio/wavaudio.c | 7 +--
12 files changed, 217 insertions(+), 315 deletions(-)
diff --git a/audio/audio_int.h b/audio/audio_int.h
index 573a84c6e4..383e3b8b06 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -73,7 +73,6 @@ typedef struct HWVoiceOut {
QLIST_HEAD (sw_out_listhead, SWVoiceOut) sw_head;
QLIST_HEAD (sw_cap_listhead, SWVoiceCap) cap_head;
-int ctl_caps;
struct audio_pcm_ops *pcm_ops;
QLIST_ENTRY (HWVoiceOut) entries;
} HWVoiceOut;
@@ -94,7 +93,6 @@ typedef struct HWVoiceIn {
size_t pos_emul, pending_emul, size_emul;
QLIST_HEAD (sw_in_listhead, SWVoiceIn) sw_head;
-int ctl_caps;
struct audio_pcm_ops *pcm_ops;
QLIST_ENTRY (HWVoiceIn) entries;
} HWVoiceIn;
@@ -146,7 +144,6 @@ struct audio_driver {
int max_voices_in;
int voice_size_out;
int voice_size_in;
-int ctl_caps;
QLIST_ENTRY(audio_driver) next;
};
@@ -168,7 +165,8 @@ struct audio_pcm_ops {
* size may be smaller
*/
size_t (*put_buffer_out)(HWVoiceOut *hw, void *buf, size_t size);
-int(*ctl_out) (HWVoiceOut *hw, int cmd, ...);
+void (*enable_out)(HWVoiceOut *hw, bool enable);
+void (*volume_out)(HWVoiceOut *hw, struct mixeng_volume *vol);
int(*init_in) (HWVoiceIn *hw, audsettings *as, void *drv_opaque);
void (*fini_in) (HWVoiceIn *hw);
@@ -176,7 +174,8 @@ struct audio_pcm_ops {
size_t (*buffer_size_in)(HWVoiceIn *hw);
void *(*get_buffer_in)(HWVoiceIn *hw, size_t *size);
void (*put_buffer_in)(HWVoiceIn *hw, void *buf, size_t size);
-int(*ctl_in) (HWVoiceIn *hw, int cmd, ...);
+void (*enable_in)(HWVoiceIn *hw, bool enable);
+void (*volume_in)(HWVoiceIn *hw, struct mixeng_volume *vol);
};
void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size);
@@ -252,12 +251,6 @@ void audio_rate_start(RateCtl *rate);
size_t audio_rate_get_bytes(struct audio_pcm_info *info, RateCtl *rate,
size_t bytes_avail);
-#define VOICE_ENABLE 1
-#define VOICE_DISABLE 2
-#define VOICE_VOLUME 3
-
-#define VOICE_VOLUME_CAP (1 << VOICE_VOLUME)
-
static inline size_t audio_ring_dist(size_t dst, size_t src, size_t len)
{
return (dst >= src) ? (dst - src) : (len - src + dst);
diff --git a/audio/audio_template.h b/audio/audio_template.h
index 700732dc17..8d36ab91f8 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -267,7 +267,6 @@ static HW *glue(audio_pcm_hw_add_new_, TYPE)(AudioState *s,
hw->s = s;
hw->pcm_ops = drv->pcm_ops;
-hw->ctl_caps = drv->ctl_caps;
QLIST_INIT (>sw_head);
#ifdef DAC
diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index 4b16869a1d..95468bcf6d 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -739,34 +739,28 @@ static int alsa_voice_ctl (snd_pcm_t *handle, const char
*typ, int ctl)
return 0;
}
-static int alsa_ctl_out (HWVoiceOut *hw, int cmd, ...)
+static void alsa_enable_out(HWVoiceOut *hw, bool enable)
{
ALSAVoiceOut *alsa = (ALSAVoiceOut *) hw;
AudiodevAlsaPerDirectionOptions *apdo = alsa->dev->u.alsa.out;
-switch (cmd) {
-case VOICE_ENABLE:
-{
-bool poll_mode = apdo->try_poll;
+if (enable) {
+bool poll_mode = apdo->try_poll;
-ldebug ("enabling voice\n");
-if (poll_mode && alsa_poll_out (hw)) {
-poll_mode = 0;
-}
-hw->poll_mode = poll_mode;
-return alsa_voice_ctl (alsa->handle, "playback",
VOICE_CTL_PREPARE);
+ldebug("enabling voice\n");
+if (poll_mode && alsa_poll_out(hw)) {
+poll_mode = 0;
}
-
-case VOICE_DISABLE:
-ldebug ("disabling voice\n");
+hw->poll_mode = poll_mode;
+alsa_voice_ctl(alsa->handle, "playback", VOICE_CTL_PREPARE);
+} else {
+ldebug("disabling voice\n");
if (hw->poll_mode) {
hw->poll_mode = 0;
-alsa_fini_poll (>pollhlp);
+alsa_fini_poll(>pollhlp);
}
-return alsa_voice_ctl (alsa->handle, "playback", VOICE_CTL_PAUSE);
+alsa_voice_ctl(alsa->handle, "playback", VOICE_CTL_PAUSE);
}
-
-return -1;
}
static int
[Qemu-devel] [PATCH 25/25] usbaudio: change playback counters to 64 bit
With stereo playback, they need about 375 minutes of continuous audio
playback to overflow, which is usually not a problem (as stopping and
later resuming playback resets the counters). But with 7.1 audio, they
only need about 95 minutes to overflow.
After the overflow, the buf->prod % USBAUDIO_PACKET_SIZE(channels)
assertion no longer holds true, which will result in overflowing the
buffer. With 64 bit variables, it would take about 762000 years to
overflow.
Signed-off-by: Kővágó, Zoltán
---
hw/usb/dev-audio.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
index e42bdfbdc1..ea604bbb8e 100644
--- a/hw/usb/dev-audio.c
+++ b/hw/usb/dev-audio.c
@@ -578,9 +578,9 @@ static const USBDesc desc_audio_multi = {
struct streambuf {
uint8_t *data;
-uint32_t size;
-uint32_t prod;
-uint32_t cons;
+size_t size;
+uint64_t prod;
+uint64_t cons;
};
static void streambuf_init(struct streambuf *buf, uint32_t size,
@@ -601,7 +601,7 @@ static void streambuf_fini(struct streambuf *buf)
static int streambuf_put(struct streambuf *buf, USBPacket *p, uint32_t
channels)
{
-uint32_t free = buf->size - (buf->prod - buf->cons);
+int64_t free = buf->size - (buf->prod - buf->cons);
if (free < USBAUDIO_PACKET_SIZE(channels)) {
return 0;
@@ -610,6 +610,8 @@ static int streambuf_put(struct streambuf *buf, USBPacket
*p, uint32_t channels)
return 0;
}
+/* can happen if prod overflows */
+assert(buf->prod % USBAUDIO_PACKET_SIZE(channels) == 0);
usb_packet_copy(p, buf->data + (buf->prod % buf->size),
USBAUDIO_PACKET_SIZE(channels));
buf->prod += USBAUDIO_PACKET_SIZE(channels);
@@ -618,10 +620,10 @@ static int streambuf_put(struct streambuf *buf, USBPacket
*p, uint32_t channels)
static uint8_t *streambuf_get(struct streambuf *buf, size_t *len)
{
-uint32_t used = buf->prod - buf->cons;
+int64_t used = buf->prod - buf->cons;
uint8_t *data;
-if (!used) {
+if (used <= 0) {
*len = 0;
return NULL;
}
--
2.22.0
[Qemu-devel] [PATCH 21/25] audio: basic support for multichannel audio
Which currently only means removing some checks. Old code won't require
more than two channels, but new code will need it.
Signed-off-by: Kővágó, Zoltán
---
audio/alsaaudio.c | 7 ---
audio/audio.c | 2 +-
2 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index b3b21e07a2..b201cfc736 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -495,13 +495,6 @@ static int alsa_open(bool in, struct alsa_params_req *req,
goto err;
}
-if (nchannels != 1 && nchannels != 2) {
-alsa_logerr2 (err, typ,
- "Can not handle obtained number of channels %d\n",
- nchannels);
-goto err;
-}
-
if (apdo->buffer_length) {
int dir = 0;
unsigned int btime = apdo->buffer_length;
diff --git a/audio/audio.c b/audio/audio.c
index f46bd5dc3d..2fc5b0ff38 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -242,7 +242,7 @@ static int audio_validate_settings (struct audsettings *as)
{
int invalid;
-invalid = as->nchannels != 1 && as->nchannels != 2;
+invalid = as->nchannels < 1;
invalid |= as->endianness != 0 && as->endianness != 1;
switch (as->fmt) {
--
2.22.0
[Qemu-devel] [PATCH 16/25] audio: add mixeng option (documentation)
This will allow us to disable mixeng when we use a decent backend.
Disabling mixeng have a few advantages:
* we no longer convert the audio output from one format to another, when
the underlying audio system would just convert it to a third format.
We no longer convert, only the underlying system, when needed.
* the underlying system probably has better resampling and sample format
converting methods anyway...
* we may support formats that the mixeng currently does not support (S24
or float samples, more than two channels)
* when using an audio server (like pulseaudio) different sound card
outputs will show up as separate streams, even if we use only one
backend
Disadvantages:
* audio capturing no longer works (wavcapture, and vnc audio extension)
* some backends only support a single playback stream or very picky
about the audio format. In this case we can't disable mixeng.
However mixeng is not removed, only made optional, so this shouldn't be
a big concern.
Signed-off-by: Kővágó, Zoltán
---
qapi/audio.json | 5 +
qemu-options.hx | 6 ++
2 files changed, 11 insertions(+)
diff --git a/qapi/audio.json b/qapi/audio.json
index 9fefdf5186..dc7f9cb1e2 100644
--- a/qapi/audio.json
+++ b/qapi/audio.json
@@ -11,6 +11,10 @@
# General audio backend options that are used for both playback and
# recording.
#
+# @mixeng: use QEMU's mixing engine to mix all streams inside QEMU. When set to
+# off, fixed-settings must be also off. Not every backend compatible
+# with the off setting (default on, since 4.2)
+#
# @fixed-settings: use fixed settings for host input/output. When off,
# frequency, channels and format must not be
# specified (default true)
@@ -31,6 +35,7 @@
##
{ 'struct': 'AudiodevPerDirectionOptions',
'data': {
+'*mixeng': 'bool',
'*fixed-settings': 'bool',
'*frequency': 'uint32',
'*channels': 'uint32',
diff --git a/qemu-options.hx b/qemu-options.hx
index ea0638e92d..8de6deb691 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -433,6 +433,7 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
"specifies the audio backend to use\n"
"id= identifier of the backend\n"
"timer-period= timer period in microseconds\n"
+"in|out.mixeng= use mixeng to mix streams inside QEMU\n"
"in|out.fixed-settings= use fixed settings for host
audio\n"
"in|out.frequency= frequency to use with fixed settings\n"
"in|out.channels= number of channels to use with fixed
settings\n"
@@ -503,6 +504,11 @@ Identifies the audio backend.
Sets the timer @var{period} used by the audio subsystem in microseconds.
Default is 1 (10 ms).
+@item in|out.mixeng=on|off
+Use QEMU's mixing engine to mix all streams inside QEMU. When off,
+@var{fixed-settings} must be off too. Not every backend is fully
+compatible with the off setting. Default is on.
+
@item in|out.fixed-settings=on|off
Use fixed settings for host audio. When off, it will change based on
how the guest opens the sound card. In this case you must not specify
--
2.22.0
[Qemu-devel] [PATCH 23/25] usb-audio: do not count on avail bytes actually available
This assumption is no longer true when mixeng is turned off.
Signed-off-by: Kővágó, Zoltán
---
hw/usb/dev-audio.c | 30 ++
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
index ae42e5a2f1..74c99b1f12 100644
--- a/hw/usb/dev-audio.c
+++ b/hw/usb/dev-audio.c
@@ -319,30 +319,31 @@ static int streambuf_put(struct streambuf *buf, USBPacket
*p)
{
uint32_t free = buf->size - (buf->prod - buf->cons);
-if (!free) {
+if (free < USBAUDIO_PACKET_SIZE) {
return 0;
}
if (p->iov.size != USBAUDIO_PACKET_SIZE) {
return 0;
}
-assert(free >= USBAUDIO_PACKET_SIZE);
+
usb_packet_copy(p, buf->data + (buf->prod % buf->size),
USBAUDIO_PACKET_SIZE);
buf->prod += USBAUDIO_PACKET_SIZE;
return USBAUDIO_PACKET_SIZE;
}
-static uint8_t *streambuf_get(struct streambuf *buf)
+static uint8_t *streambuf_get(struct streambuf *buf, size_t *len)
{
uint32_t used = buf->prod - buf->cons;
uint8_t *data;
if (!used) {
+*len = 0;
return NULL;
}
-assert(used >= USBAUDIO_PACKET_SIZE);
data = buf->data + (buf->cons % buf->size);
-buf->cons += USBAUDIO_PACKET_SIZE;
+*len = MIN(buf->prod - buf->cons,
+ buf->size - (buf->cons % buf->size));
return data;
}
@@ -374,16 +375,21 @@ static void output_callback(void *opaque, int avail)
USBAudioState *s = opaque;
uint8_t *data;
-for (;;) {
-if (avail < USBAUDIO_PACKET_SIZE) {
-return;
-}
-data = streambuf_get(>out.buf);
+while (avail) {
+size_t written, len;
+
+data = streambuf_get(>out.buf, );
if (!data) {
return;
}
-AUD_write(s->out.voice, data, USBAUDIO_PACKET_SIZE);
-avail -= USBAUDIO_PACKET_SIZE;
+
+written = AUD_write(s->out.voice, data, len);
+avail -= written;
+s->out.buf.cons += written;
+
+if (written < len) {
+return;
+}
}
}
--
2.22.0
[Qemu-devel] [PATCH 13/25] audio: remove hw->samples, buffer_size_in/out pcm_ops
This patch removes the samples member from HWVoiceIn and HWVoiceOut.
Backends can specify buffer size via the newly added buffer_size_in and
buffer_size_out functions in audio_pcm_ops. They are optional, if not
defined qemu will fall back to some built-in constant.
Signed-off-by: Kővágó, Zoltán
---
Notes:
Not sure if this is necessary. At first it seemed like a good idea to
have a function so that backends can compute the size on demand when
needed and things like that, but currently it's just a burden. The only
good feature is that it allows a backend to not define a function and
let the audio subsystem choose a default value, but the same could be
achieved by specifying that hw->samples = 0 means use a default value.
So if you guys agree, I'll remove this patch. Maybe add an -audiodev
parameter to change it, overriding whatever the backends supplies.
audio/audio_int.h | 5 +++--
audio/audio_template.h | 24 +++-
audio/dsound_template.h | 8 +++-
audio/alsaaudio.c | 20 ++--
audio/audio.c | 2 --
audio/coreaudio.c | 10 +-
audio/dsoundaudio.c | 4
audio/noaudio.c | 2 --
audio/ossaudio.c| 22 +++---
audio/paaudio.c | 19 +--
audio/sdlaudio.c| 10 +-
audio/spiceaudio.c | 14 --
audio/wavaudio.c| 1 -
13 files changed, 113 insertions(+), 28 deletions(-)
diff --git a/audio/audio_int.h b/audio/audio_int.h
index 20021df9e8..65112b490a 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -71,7 +71,6 @@ typedef struct HWVoiceOut {
void *buf_emul;
size_t pos_emul, pending_emul, size_emul;
-size_t samples;
QLIST_HEAD (sw_out_listhead, SWVoiceOut) sw_head;
QLIST_HEAD (sw_cap_listhead, SWVoiceCap) cap_head;
int ctl_caps;
@@ -94,7 +93,6 @@ typedef struct HWVoiceIn {
void *buf_emul;
size_t pos_emul, pending_emul, size_emul;
-size_t samples;
QLIST_HEAD (sw_in_listhead, SWVoiceIn) sw_head;
int ctl_caps;
struct audio_pcm_ops *pcm_ops;
@@ -156,6 +154,8 @@ struct audio_pcm_ops {
int(*init_out)(HWVoiceOut *hw, audsettings *as, void *drv_opaque);
void (*fini_out)(HWVoiceOut *hw);
size_t (*write) (HWVoiceOut *hw, void *buf, size_t size);
+/* get the optimal buffer size in samples; optional */
+size_t (*buffer_size_out)(HWVoiceOut *hw);
/*
* get a buffer that after later can be passed to put_buffer_out; optional
* returns the buffer, and writes it's size to size (in bytes)
@@ -173,6 +173,7 @@ struct audio_pcm_ops {
int(*init_in) (HWVoiceIn *hw, audsettings *as, void *drv_opaque);
void (*fini_in) (HWVoiceIn *hw);
size_t (*read)(HWVoiceIn *hw, void *buf, size_t size);
+size_t (*buffer_size_in)(HWVoiceIn *hw);
void *(*get_buffer_in)(HWVoiceIn *hw, size_t *size);
void (*put_buffer_in)(HWVoiceIn *hw, void *buf, size_t size);
int(*ctl_in) (HWVoiceIn *hw, int cmd, ...);
diff --git a/audio/audio_template.h b/audio/audio_template.h
index 87c6d2d271..700732dc17 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -78,7 +78,20 @@ static void glue (audio_pcm_hw_free_resources_, TYPE) (HW
*hw)
static void glue(audio_pcm_hw_alloc_resources_, TYPE)(HW *hw)
{
-size_t samples = hw->samples;
+size_t samples;
+if (!hw->pcm_ops) {
+/*
+ * We should only end up here when using wavcapture hmp command (and
not
+ * the wavcapture audio backend).
+ * It needs a lot of samples, otherwise you'll end up with "Could not
+ * mix X bytes into a capture buffer" warnings and a garbled capture.
+ */
+samples = 4096 * 4;
+} else if (hw->pcm_ops->glue(buffer_size_, TYPE)) {
+samples = hw->pcm_ops->glue(buffer_size_, TYPE)(hw);
+} else {
+samples = 1024; /* todo better default */
+}
if (audio_bug(__func__, samples == 0)) {
dolog("Attempted to allocate empty buffer\n");
}
@@ -264,11 +277,6 @@ static HW *glue(audio_pcm_hw_add_new_, TYPE)(AudioState *s,
goto err0;
}
-if (audio_bug(__func__, hw->samples <= 0)) {
-dolog("hw->samples=%zd\n", hw->samples);
-goto err1;
-}
-
#ifdef DAC
hw->clip = mixeng_clip
#else
@@ -288,9 +296,7 @@ static HW *glue(audio_pcm_hw_add_new_, TYPE)(AudioState *s,
#endif
return hw;
- err1:
-glue (hw->pcm_ops->fini_, TYPE) (hw);
- err0:
+err0:
g_free (hw);
return NULL;
}
diff --git a/audio/dsound_template.h b/audio/dsound_template.h
index 9f10b688df..8cc5d69f36 100644
--- a/audio/dsound_template.h
+++ b/audio/dsound_template.h
@@ -254,7 +254,7 @@ static int dsound_init_out(HWVoiceOut *hw, struct
audsettings *as,
);
}
hw->size_emul = bc.dwBufferBytes;
-hw->samples = bc.dwBufferBytes >> hw->info.shift;
+ds->samples =
[Qemu-devel] [PATCH 14/25] audio: common rate control code for timer based outputs
This commit removes the ad-hoc rate-limiting code from noaudio and
wavaudio, and replaces them with a (slightly modified) code from
spiceaudio. This way multiple write calls (for example when the
circular buffer wraps around) do not cause problems.
Signed-off-by: Kővágó, Zoltán
---
audio/audio_int.h | 9 +
audio/audio.c | 30
audio/noaudio.c| 49 +-
audio/spiceaudio.c | 49 +++---
audio/wavaudio.c | 21 ++--
5 files changed, 78 insertions(+), 80 deletions(-)
diff --git a/audio/audio_int.h b/audio/audio_int.h
index 65112b490a..573a84c6e4 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -243,6 +243,15 @@ void *audio_calloc (const char *funcname, int nmemb,
size_t size);
void audio_run(AudioState *s, const char *msg);
+typedef struct RateCtl {
+int64_t start_ticks;
+int64_t bytes_sent;
+} RateCtl;
+
+void audio_rate_start(RateCtl *rate);
+size_t audio_rate_get_bytes(struct audio_pcm_info *info, RateCtl *rate,
+size_t bytes_avail);
+
#define VOICE_ENABLE 1
#define VOICE_DISABLE 2
#define VOICE_VOLUME 3
diff --git a/audio/audio.c b/audio/audio.c
index 2b77a87823..2cec12fa54 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -2044,3 +2044,33 @@ const char *audio_get_id(QEMUSoundCard *card)
return "";
}
}
+
+void audio_rate_start(RateCtl *rate)
+{
+memset(rate, 0, sizeof(RateCtl));
+rate->start_ticks = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+}
+
+size_t audio_rate_get_bytes(struct audio_pcm_info *info, RateCtl *rate,
+size_t bytes_avail)
+{
+int64_t now;
+int64_t ticks;
+int64_t bytes;
+int64_t samples;
+size_t ret;
+
+now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+ticks = now - rate->start_ticks;
+bytes = muldiv64(ticks, info->bytes_per_second, NANOSECONDS_PER_SECOND);
+samples = (bytes - rate->bytes_sent) >> info->shift;
+if (samples < 0 || samples > 65536) {
+AUD_log(NULL, "Resetting rate control (%" PRId64 " samples)", samples);
+audio_rate_start(rate);
+samples = 0;
+}
+
+ret = MIN(samples << info->shift, bytes_avail);
+rate->bytes_sent += ret;
+return ret;
+}
diff --git a/audio/noaudio.c b/audio/noaudio.c
index f4b9d09d84..dc1078471a 100644
--- a/audio/noaudio.c
+++ b/audio/noaudio.c
@@ -33,32 +33,26 @@
typedef struct NoVoiceOut {
HWVoiceOut hw;
-int64_t old_ticks;
+RateCtl rate;
} NoVoiceOut;
typedef struct NoVoiceIn {
HWVoiceIn hw;
-int64_t old_ticks;
+RateCtl rate;
} NoVoiceIn;
static size_t no_write(HWVoiceOut *hw, void *buf, size_t len)
{
NoVoiceOut *no = (NoVoiceOut *) hw;
-int64_t now;
-int64_t ticks;
-int64_t bytes;
-
-now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-ticks = now - no->old_ticks;
-bytes = muldiv64(ticks, hw->info.bytes_per_second, NANOSECONDS_PER_SECOND);
-
-no->old_ticks = now;
-return MIN(len, bytes);
+return audio_rate_get_bytes(>info, >rate, len);
}
static int no_init_out(HWVoiceOut *hw, struct audsettings *as, void
*drv_opaque)
{
+NoVoiceOut *no = (NoVoiceOut *) hw;
+
audio_pcm_init_info (>info, as);
+audio_rate_start(>rate);
return 0;
}
@@ -69,14 +63,20 @@ static void no_fini_out (HWVoiceOut *hw)
static int no_ctl_out (HWVoiceOut *hw, int cmd, ...)
{
-(void) hw;
-(void) cmd;
+NoVoiceOut *no = (NoVoiceOut *) hw;
+
+if (cmd == VOICE_ENABLE) {
+audio_rate_start(>rate);
+}
return 0;
}
static int no_init_in(HWVoiceIn *hw, struct audsettings *as, void *drv_opaque)
{
+NoVoiceIn *no = (NoVoiceIn *) hw;
+
audio_pcm_init_info (>info, as);
+audio_rate_start(>rate);
return 0;
}
@@ -87,25 +87,20 @@ static void no_fini_in (HWVoiceIn *hw)
static size_t no_read(HWVoiceIn *hw, void *buf, size_t size)
{
-size_t to_clear;
NoVoiceIn *no = (NoVoiceIn *) hw;
+int64_t bytes = audio_rate_get_bytes(>info, >rate, size);
-int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-int64_t ticks = now - no->old_ticks;
-int64_t bytes =
-muldiv64(ticks, hw->info.bytes_per_second, NANOSECONDS_PER_SECOND);
-
-no->old_ticks = now;
-to_clear = MIN(bytes, size);
-
-audio_pcm_info_clear_buf(>info, buf, to_clear >> hw->info.shift);
-return to_clear;
+audio_pcm_info_clear_buf(>info, buf, bytes >> hw->info.shift);
+return bytes;
}
static int no_ctl_in (HWVoiceIn *hw, int cmd, ...)
{
-(void) hw;
-(void) cmd;
+NoVoiceIn *no = (NoVoiceIn *) hw;
+
+if (cmd == VOICE_ENABLE) {
+audio_rate_start(>rate);
+}
return 0;
}
diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c
index 02e97095fc..a6b875ac36 100644
--- a/audio/spiceaudio.c
+++ b/audio/spiceaudio.c
@@ -40,15 +40,10 @@
#define LINE_IN_SAMPLES (256 * 4)
#endif
-typedef
[Qemu-devel] [PATCH 17/25] audio: make mixeng optional
Implementation of the previously added mixeng option.
Signed-off-by: Kővágó, Zoltán
---
audio/audio_template.h | 46 ---
audio/audio.c | 70 ++
2 files changed, 92 insertions(+), 24 deletions(-)
diff --git a/audio/audio_template.h b/audio/audio_template.h
index 8d36ab91f8..487e2a865f 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -78,26 +78,32 @@ static void glue (audio_pcm_hw_free_resources_, TYPE) (HW
*hw)
static void glue(audio_pcm_hw_alloc_resources_, TYPE)(HW *hw)
{
-size_t samples;
-if (!hw->pcm_ops) {
-/*
- * We should only end up here when using wavcapture hmp command (and
not
- * the wavcapture audio backend).
- * It needs a lot of samples, otherwise you'll end up with "Could not
- * mix X bytes into a capture buffer" warnings and a garbled capture.
- */
-samples = 4096 * 4;
-} else if (hw->pcm_ops->glue(buffer_size_, TYPE)) {
-samples = hw->pcm_ops->glue(buffer_size_, TYPE)(hw);
+if (glue(audio_get_pdo_, TYPE)(hw->s->dev)->mixeng) {
+size_t samples;
+if (!hw->pcm_ops) {
+/*
+ * We should only end up here when using wavcapture hmp command
(and
+ * not the wavcapture audio backend).
+ * It needs a lot of samples, otherwise you'll end up with "Could
+ * not mix X bytes into a capture buffer" warnings and a garbled
+ * capture.
+ */
+samples = 4096 * 4;
+} else if (hw->pcm_ops->glue(buffer_size_, TYPE)) {
+samples = hw->pcm_ops->glue(buffer_size_, TYPE)(hw);
+} else {
+samples = 1024; /* todo better default */
+}
+
+if (audio_bug(__func__, samples == 0)) {
+dolog("Attempted to allocate empty buffer\n");
+}
+
+HWBUF = g_malloc0(sizeof(STSampleBuffer) + sizeof(st_sample) *
samples);
+HWBUF->size = samples;
} else {
-samples = 1024; /* todo better default */
+HWBUF = NULL;
}
-if (audio_bug(__func__, samples == 0)) {
-dolog("Attempted to allocate empty buffer\n");
-}
-
-HWBUF = g_malloc0(sizeof(STSampleBuffer) + sizeof(st_sample) * samples);
-HWBUF->size = samples;
}
static void glue (audio_pcm_sw_free_resources_, TYPE) (SW *sw)
@@ -116,6 +122,10 @@ static int glue (audio_pcm_sw_alloc_resources_, TYPE) (SW
*sw)
{
int samples;
+if (!glue(audio_get_pdo_, TYPE)(sw->s->dev)->mixeng) {
+return 0;
+}
+
samples = ((int64_t) sw->HWBUF->size << 32) / sw->ratio;
sw->buf = audio_calloc(__func__, samples, sizeof(struct st_sample));
diff --git a/audio/audio.c b/audio/audio.c
index 84b78f604e..73ace0500e 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -838,32 +838,46 @@ static void audio_timer (void *opaque)
*/
size_t AUD_write(SWVoiceOut *sw, void *buf, size_t size)
{
+HWVoiceOut *hw;
+
if (!sw) {
/* XXX: Consider options */
return size;
}
+hw = sw->hw;
-if (!sw->hw->enabled) {
+if (!hw->enabled) {
dolog ("Writing to disabled voice %s\n", SW_NAME (sw));
return 0;
}
-return audio_pcm_sw_write(sw, buf, size);
+if (audio_get_pdo_out(hw->s->dev)->mixeng) {
+return audio_pcm_sw_write(sw, buf, size);
+} else {
+return hw->pcm_ops->write(hw, buf, size);
+}
}
size_t AUD_read(SWVoiceIn *sw, void *buf, size_t size)
{
+HWVoiceIn *hw;
+
if (!sw) {
/* XXX: Consider options */
return size;
}
+hw = sw->hw;
-if (!sw->hw->enabled) {
+if (!hw->enabled) {
dolog ("Reading from disabled voice %s\n", SW_NAME (sw));
return 0;
}
-return audio_pcm_sw_read(sw, buf, size);
+if (audio_get_pdo_in(hw->s->dev)->mixeng) {
+return audio_pcm_sw_read(sw, buf, size);
+} else {
+return hw->pcm_ops->read(hw, buf, size);
+}
}
int AUD_get_buffer_size_out (SWVoiceOut *sw)
@@ -1085,6 +1099,26 @@ static void audio_run_out (AudioState *s)
HWVoiceOut *hw = NULL;
SWVoiceOut *sw;
+if (!audio_get_pdo_out(s->dev)->mixeng) {
+while ((hw = audio_pcm_hw_find_any_enabled_out(s, hw))) {
+/* there is exactly 1 sw for each hw with no mixeng */
+sw = hw->sw_head.lh_first;
+
+if (hw->pending_disable) {
+hw->enabled = 0;
+hw->pending_disable = 0;
+if (hw->pcm_ops->enable_out) {
+hw->pcm_ops->enable_out(hw, false);
+}
+}
+
+if (sw->active) {
+sw->callback.fn(sw->callback.opaque, INT_MAX);
+}
+}
+return;
+}
+
while ((hw = audio_pcm_hw_find_any_enabled_out(s, hw))) {
size_t played, live, prev_rpos, free;
int nb_live, cleanup_required;
@@ -1222,6 +1256,17
[Qemu-devel] [PATCH 07/25] paaudio: port to the new audio backend api
Signed-off-by: Kővágó, Zoltán
---
configure| 5 -
audio/audio_pt_int.h | 22 ---
audio/audio_pt_int.c | 173
audio/paaudio.c | 372 ++-
audio/Makefile.objs | 1 -
5 files changed, 45 insertions(+), 528 deletions(-)
delete mode 100644 audio/audio_pt_int.h
delete mode 100644 audio/audio_pt_int.c
diff --git a/configure b/configure
index e44e454c43..75f5d87976 100755
--- a/configure
+++ b/configure
@@ -297,7 +297,6 @@ host_cc="cc"
libs_cpu=""
libs_softmmu=""
libs_tools=""
-audio_pt_int=""
audio_win_int=""
libs_qga=""
debug_info="yes"
@@ -3420,7 +3419,6 @@ for drv in $audio_drv_list; do
pa | try-pa)
if $pkg_config libpulse --exists; then
pulse_libs=$($pkg_config libpulse --libs)
-audio_pt_int="yes"
if test "$drv" = "try-pa"; then
audio_drv_list=$(echo "$audio_drv_list" | sed -e 's/try-pa/pa/')
fi
@@ -6654,9 +6652,6 @@ echo "PULSE_LIBS=$pulse_libs" >> $config_host_mak
echo "COREAUDIO_LIBS=$coreaudio_libs" >> $config_host_mak
echo "DSOUND_LIBS=$dsound_libs" >> $config_host_mak
echo "OSS_LIBS=$oss_libs" >> $config_host_mak
-if test "$audio_pt_int" = "yes" ; then
- echo "CONFIG_AUDIO_PT_INT=y" >> $config_host_mak
-fi
if test "$audio_win_int" = "yes" ; then
echo "CONFIG_AUDIO_WIN_INT=y" >> $config_host_mak
fi
diff --git a/audio/audio_pt_int.h b/audio/audio_pt_int.h
deleted file mode 100644
index 4c0c15b9af..00
--- a/audio/audio_pt_int.h
+++ /dev/null
@@ -1,22 +0,0 @@
-#ifndef QEMU_AUDIO_PT_INT_H
-#define QEMU_AUDIO_PT_INT_H
-
-#include
-
-struct audio_pt {
-const char *drv;
-pthread_t thread;
-pthread_cond_t cond;
-pthread_mutex_t mutex;
-};
-
-int audio_pt_init (struct audio_pt *, void *(*) (void *), void *,
- const char *, const char *);
-int audio_pt_fini (struct audio_pt *, const char *);
-int audio_pt_lock (struct audio_pt *, const char *);
-int audio_pt_unlock (struct audio_pt *, const char *);
-int audio_pt_wait (struct audio_pt *, const char *);
-int audio_pt_unlock_and_signal (struct audio_pt *, const char *);
-int audio_pt_join (struct audio_pt *, void **, const char *);
-
-#endif /* QEMU_AUDIO_PT_INT_H */
diff --git a/audio/audio_pt_int.c b/audio/audio_pt_int.c
deleted file mode 100644
index 9f9d44ad4a..00
--- a/audio/audio_pt_int.c
+++ /dev/null
@@ -1,173 +0,0 @@
-#include "qemu/osdep.h"
-#include "audio.h"
-
-#define AUDIO_CAP "audio-pt"
-
-#include "audio_int.h"
-#include "audio_pt_int.h"
-
-static void GCC_FMT_ATTR(3, 4) logerr (struct audio_pt *pt, int err,
- const char *fmt, ...)
-{
-va_list ap;
-
-va_start (ap, fmt);
-AUD_vlog (pt->drv, fmt, ap);
-va_end (ap);
-
-AUD_log (NULL, "\n");
-AUD_log (pt->drv, "Reason: %s\n", strerror (err));
-}
-
-int audio_pt_init (struct audio_pt *p, void *(*func) (void *),
- void *opaque, const char *drv, const char *cap)
-{
-int err, err2;
-const char *efunc;
-sigset_t set, old_set;
-
-p->drv = drv;
-
-err = sigfillset ();
-if (err) {
-logerr(p, errno, "%s(%s): sigfillset failed", cap, __func__);
-return -1;
-}
-
-err = pthread_mutex_init (>mutex, NULL);
-if (err) {
-efunc = "pthread_mutex_init";
-goto err0;
-}
-
-err = pthread_cond_init (>cond, NULL);
-if (err) {
-efunc = "pthread_cond_init";
-goto err1;
-}
-
-err = pthread_sigmask (SIG_BLOCK, , _set);
-if (err) {
-efunc = "pthread_sigmask";
-goto err2;
-}
-
-err = pthread_create (>thread, NULL, func, opaque);
-
-err2 = pthread_sigmask (SIG_SETMASK, _set, NULL);
-if (err2) {
-logerr(p, err2, "%s(%s): pthread_sigmask (restore) failed",
- cap, __func__);
-/* We have failed to restore original signal mask, all bets are off,
- so terminate the process */
-exit (EXIT_FAILURE);
-}
-
-if (err) {
-efunc = "pthread_create";
-goto err2;
-}
-
-return 0;
-
- err2:
-err2 = pthread_cond_destroy (>cond);
-if (err2) {
-logerr(p, err2, "%s(%s): pthread_cond_destroy failed", cap, __func__);
-}
-
- err1:
-err2 = pthread_mutex_destroy (>mutex);
-if (err2) {
-logerr(p, err2, "%s(%s): pthread_mutex_destroy failed", cap, __func__);
-}
-
- err0:
-logerr(p, err, "%s(%s): %s failed", cap, __func__, efunc);
-return -1;
-}
-
-int audio_pt_fini (struct audio_pt *p, const char *cap)
-{
-int err, ret = 0;
-
-err = pthread_cond_destroy (>cond);
-if (err) {
-logerr(p, err, "%s(%s): pthread_cond_destroy failed", cap, __func__);
-ret = -1;
-}
-
-err = pthread_mutex_destroy (>mutex);
-if (err) {
-logerr(p, err, "%s(%s): pthread_mutex_destroy failed", cap, __func__);
-ret = -1;
-}
-return ret;
-}
-
-int audio_pt_lock (struct audio_pt *p,
[Qemu-devel] [PATCH 12/25] audio: unify input and output mixeng buffer management
Usage notes: hw->samples became hw->{mix,conv}_buf->size, except before
initialization (audio_pcm_hw_alloc_resources_*), hw->samples gives the
initial size of the STSampleBuffer. The next commit tries to fix this
inconsistency.
Signed-off-by: Kővágó, Zoltán
---
audio/audio_int.h | 12 +++--
audio/audio_template.h | 19 +++
audio/audio.c | 120 +
audio/ossaudio.c | 3 +-
4 files changed, 79 insertions(+), 75 deletions(-)
diff --git a/audio/audio_int.h b/audio/audio_int.h
index c76d7c39e8..20021df9e8 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -52,6 +52,11 @@ struct audio_pcm_info {
typedef struct AudioState AudioState;
typedef struct SWVoiceCap SWVoiceCap;
+typedef struct STSampleBuffer {
+size_t pos, size;
+st_sample samples[];
+} STSampleBuffer;
+
typedef struct HWVoiceOut {
AudioState *s;
int enabled;
@@ -60,11 +65,9 @@ typedef struct HWVoiceOut {
struct audio_pcm_info info;
f_sample *clip;
-
-size_t rpos;
uint64_t ts_helper;
-struct st_sample *mix_buf;
+STSampleBuffer *mix_buf;
void *buf_emul;
size_t pos_emul, pending_emul, size_emul;
@@ -84,11 +87,10 @@ typedef struct HWVoiceIn {
t_sample *conv;
-size_t wpos;
size_t total_samples_captured;
uint64_t ts_helper;
-struct st_sample *conv_buf;
+STSampleBuffer *conv_buf;
void *buf_emul;
size_t pos_emul, pending_emul, size_emul;
diff --git a/audio/audio_template.h b/audio/audio_template.h
index ff4a173f18..87c6d2d271 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -76,16 +76,15 @@ static void glue (audio_pcm_hw_free_resources_, TYPE) (HW
*hw)
HWBUF = NULL;
}
-static bool glue(audio_pcm_hw_alloc_resources_, TYPE)(HW *hw)
+static void glue(audio_pcm_hw_alloc_resources_, TYPE)(HW *hw)
{
-HWBUF = audio_calloc(__func__, hw->samples, sizeof(struct st_sample));
-if (!HWBUF) {
-dolog("Could not allocate " NAME " buffer (%zu samples)\n",
- hw->samples);
-return false;
+size_t samples = hw->samples;
+if (audio_bug(__func__, samples == 0)) {
+dolog("Attempted to allocate empty buffer\n");
}
-return true;
+HWBUF = g_malloc0(sizeof(STSampleBuffer) + sizeof(st_sample) * samples);
+HWBUF->size = samples;
}
static void glue (audio_pcm_sw_free_resources_, TYPE) (SW *sw)
@@ -104,7 +103,7 @@ static int glue (audio_pcm_sw_alloc_resources_, TYPE) (SW
*sw)
{
int samples;
-samples = ((int64_t) sw->hw->samples << 32) / sw->ratio;
+samples = ((int64_t) sw->HWBUF->size << 32) / sw->ratio;
sw->buf = audio_calloc(__func__, samples, sizeof(struct st_sample));
if (!sw->buf) {
@@ -280,9 +279,7 @@ static HW *glue(audio_pcm_hw_add_new_, TYPE)(AudioState *s,
[hw->info.swap_endianness]
[audio_bits_to_index (hw->info.bits)];
-if (!glue(audio_pcm_hw_alloc_resources_, TYPE)(hw)) {
-goto err1;
-}
+glue(audio_pcm_hw_alloc_resources_, TYPE)(hw);
QLIST_INSERT_HEAD (>glue (hw_head_, TYPE), hw, entries);
glue (s->nb_hw_voices_, TYPE) -= 1;
diff --git a/audio/audio.c b/audio/audio.c
index d1c9ce855f..e571eba6a1 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -544,8 +544,8 @@ static size_t audio_pcm_hw_find_min_in (HWVoiceIn *hw)
static size_t audio_pcm_hw_get_live_in(HWVoiceIn *hw)
{
size_t live = hw->total_samples_captured - audio_pcm_hw_find_min_in (hw);
-if (audio_bug(__func__, live > hw->samples)) {
-dolog("live=%zu hw->samples=%zu\n", live, hw->samples);
+if (audio_bug(__func__, live > hw->conv_buf->size)) {
+dolog("live=%zu hw->conv_buf->size=%zu\n", live, hw->conv_buf->size);
return 0;
}
return live;
@@ -554,17 +554,17 @@ static size_t audio_pcm_hw_get_live_in(HWVoiceIn *hw)
static void audio_pcm_hw_clip_out(HWVoiceOut *hw, void *pcm_buf, size_t len)
{
size_t clipped = 0;
-size_t pos = hw->rpos;
+size_t pos = hw->mix_buf->pos;
while (len) {
-st_sample *src = hw->mix_buf + pos;
+st_sample *src = hw->mix_buf->samples + pos;
uint8_t *dst = advance(pcm_buf, clipped << hw->info.shift);
-size_t samples_till_end_of_buf = hw->samples - pos;
+size_t samples_till_end_of_buf = hw->mix_buf->size - pos;
size_t samples_to_clip = MIN(len, samples_till_end_of_buf);
hw->clip(dst, src, samples_to_clip);
-pos = (pos + samples_to_clip) % hw->samples;
+pos = (pos + samples_to_clip) % hw->mix_buf->size;
len -= samples_to_clip;
clipped += samples_to_clip;
}
@@ -579,17 +579,17 @@ static size_t audio_pcm_sw_get_rpos_in(SWVoiceIn *sw)
ssize_t live = hw->total_samples_captured - sw->total_hw_samples_acquired;
ssize_t rpos;
-if (audio_bug(__func__, live < 0 || live > hw->samples)) {
-dolog("live=%zu hw->samples=%zu\n", live, hw->samples);
+if
[Qemu-devel] [PATCH 11/25] audio: remove remains of the old backend api
Signed-off-by: Kővágó, Zoltán
---
audio/audio_int.h | 7 ---
audio/audio.c | 42 ++
2 files changed, 6 insertions(+), 43 deletions(-)
diff --git a/audio/audio_int.h b/audio/audio_int.h
index 8fb1ca8a8d..c76d7c39e8 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -153,7 +153,6 @@ struct audio_driver {
struct audio_pcm_ops {
int(*init_out)(HWVoiceOut *hw, audsettings *as, void *drv_opaque);
void (*fini_out)(HWVoiceOut *hw);
-size_t (*run_out)(HWVoiceOut *hw, size_t live);
size_t (*write) (HWVoiceOut *hw, void *buf, size_t size);
/*
* get a buffer that after later can be passed to put_buffer_out; optional
@@ -171,7 +170,6 @@ struct audio_pcm_ops {
int(*init_in) (HWVoiceIn *hw, audsettings *as, void *drv_opaque);
void (*fini_in) (HWVoiceIn *hw);
-size_t (*run_in)(HWVoiceIn *hw);
size_t (*read)(HWVoiceIn *hw, void *buf, size_t size);
void *(*get_buffer_in)(HWVoiceIn *hw, size_t *size);
void (*put_buffer_in)(HWVoiceIn *hw, void *buf, size_t size);
@@ -237,11 +235,6 @@ audio_driver *audio_driver_lookup(const char *name);
void audio_pcm_init_info (struct audio_pcm_info *info, struct audsettings *as);
void audio_pcm_info_clear_buf (struct audio_pcm_info *info, void *buf, int
len);
-size_t audio_pcm_hw_get_live_in(HWVoiceIn *hw);
-
-size_t audio_pcm_hw_clip_out(HWVoiceOut *hw, void *pcm_buf,
- size_t live, size_t pending);
-
int audio_bug (const char *funcname, int cond);
void *audio_calloc (const char *funcname, int nmemb, size_t size);
diff --git a/audio/audio.c b/audio/audio.c
index 0dfcfeaf06..d1c9ce855f 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -541,7 +541,7 @@ static size_t audio_pcm_hw_find_min_in (HWVoiceIn *hw)
return m;
}
-size_t audio_pcm_hw_get_live_in(HWVoiceIn *hw)
+static size_t audio_pcm_hw_get_live_in(HWVoiceIn *hw)
{
size_t live = hw->total_samples_captured - audio_pcm_hw_find_min_in (hw);
if (audio_bug(__func__, live > hw->samples)) {
@@ -551,29 +551,7 @@ size_t audio_pcm_hw_get_live_in(HWVoiceIn *hw)
return live;
}
-size_t audio_pcm_hw_clip_out(HWVoiceOut *hw, void *pcm_buf,
- size_t live, size_t pending)
-{
-size_t left = hw->samples - pending;
-size_t len = MIN (left, live);
-size_t clipped = 0;
-
-while (len) {
-struct st_sample *src = hw->mix_buf + hw->rpos;
-uint8_t *dst = advance (pcm_buf, hw->rpos << hw->info.shift);
-size_t samples_till_end_of_buf = hw->samples - hw->rpos;
-size_t samples_to_clip = MIN (len, samples_till_end_of_buf);
-
-hw->clip (dst, src, samples_to_clip);
-
-hw->rpos = (hw->rpos + samples_to_clip) % hw->samples;
-len -= samples_to_clip;
-clipped += samples_to_clip;
-}
-return clipped;
-}
-
-static void audio_pcm_hw_clip_out2(HWVoiceOut *hw, void *pcm_buf, size_t len)
+static void audio_pcm_hw_clip_out(HWVoiceOut *hw, void *pcm_buf, size_t len)
{
size_t clipped = 0;
size_t pos = hw->rpos;
@@ -1078,7 +1056,7 @@ static size_t audio_pcm_hw_run_out(HWVoiceOut *hw, size_t
live)
void *buf = hw->pcm_ops->get_buffer_out(hw, );
decr = MIN(size >> hw->info.shift, live);
-audio_pcm_hw_clip_out2(hw, buf, decr);
+audio_pcm_hw_clip_out(hw, buf, decr);
proc = hw->pcm_ops->put_buffer_out(hw, buf, decr << hw->info.shift) >>
hw->info.shift;
@@ -1141,11 +1119,7 @@ static void audio_run_out (AudioState *s)
}
prev_rpos = hw->rpos;
-if (hw->pcm_ops->run_out) {
-played = hw->pcm_ops->run_out(hw, live);
-} else {
-played = audio_pcm_hw_run_out(hw, live);
-}
+played = audio_pcm_hw_run_out(hw, live);
replay_audio_out();
if (audio_bug(__func__, hw->rpos >= hw->samples)) {
dolog("hw->rpos=%zu hw->samples=%zu played=%zu\n",
@@ -1242,12 +1216,8 @@ static void audio_run_in (AudioState *s)
size_t captured = 0, min;
if (replay_mode != REPLAY_MODE_PLAY) {
-if (hw->pcm_ops->run_in) {
-captured = hw->pcm_ops->run_in(hw);
-} else {
-captured = audio_pcm_hw_run_in(
-hw, hw->samples - audio_pcm_hw_get_live_in(hw));
-}
+captured = audio_pcm_hw_run_in(
+hw, hw->samples - audio_pcm_hw_get_live_in(hw));
}
replay_audio_in(, hw->conv_buf, >wpos, hw->samples);
--
2.22.0
[Qemu-devel] [PATCH 06/25] ossaudio: port to the new audio backend api
Signed-off-by: Kővágó, Zoltán
---
audio/ossaudio.c | 288 +--
1 file changed, 104 insertions(+), 184 deletions(-)
diff --git a/audio/ossaudio.c b/audio/ossaudio.c
index 1696933688..2782512706 100644
--- a/audio/ossaudio.c
+++ b/audio/ossaudio.c
@@ -40,19 +40,15 @@
typedef struct OSSVoiceOut {
HWVoiceOut hw;
-void *pcm_buf;
int fd;
-int wpos;
int nfrags;
int fragsize;
int mmapped;
-int pending;
Audiodev *dev;
} OSSVoiceOut;
typedef struct OSSVoiceIn {
HWVoiceIn hw;
-void *pcm_buf;
int fd;
int nfrags;
int fragsize;
@@ -371,98 +367,87 @@ static int oss_open(int in, struct oss_params *req,
audsettings *as,
return -1;
}
-static void oss_write_pending (OSSVoiceOut *oss)
+static size_t oss_get_available_bytes(OSSVoiceOut *oss)
{
-HWVoiceOut *hw = >hw;
+int err;
+struct count_info cntinfo;
+assert(oss->mmapped);
+err = ioctl(oss->fd, SNDCTL_DSP_GETOPTR, );
+if (err < 0) {
+oss_logerr(errno, "SNDCTL_DSP_GETOPTR failed\n");
+return 0;
+}
+
+return audio_ring_dist(cntinfo.ptr, oss->hw.pos_emul, oss->hw.size_emul);
+}
+
+static void *oss_get_buffer_out(HWVoiceOut *hw, size_t *size)
+{
+OSSVoiceOut *oss = (OSSVoiceOut *) hw;
+if (oss->mmapped) {
+*size = MIN(oss_get_available_bytes(oss), hw->size_emul -
hw->pos_emul);
+return hw->buf_emul + hw->pos_emul;
+} else {
+return audio_generic_get_buffer_out(hw, size);
+}
+}
+
+static size_t oss_put_buffer_out(HWVoiceOut *hw, void *buf, size_t size)
+{
+OSSVoiceOut *oss = (OSSVoiceOut *) hw;
if (oss->mmapped) {
-return;
+assert(buf == hw->buf_emul + hw->pos_emul && size < hw->size_emul);
+
+hw->pos_emul = (hw->pos_emul + size) % hw->size_emul;
+return size;
+} else {
+return audio_generic_put_buffer_out(hw, buf, size);
+}
+}
+
+static size_t oss_write(HWVoiceOut *hw, void *buf, size_t len)
+{
+OSSVoiceOut *oss = (OSSVoiceOut *) hw;
+size_t pos;
+
+if (oss->mmapped) {
+size_t total_len;
+len = MIN(len, oss_get_available_bytes(oss));
+
+total_len = len;
+while (len) {
+size_t to_copy = MIN(len, hw->size_emul - hw->pos_emul);
+memcpy(hw->buf_emul + hw->pos_emul, buf, to_copy);
+
+hw->pos_emul = (hw->pos_emul + to_copy) % hw->pos_emul;
+buf += to_copy;
+len -= to_copy;
+}
+return total_len;
}
-while (oss->pending) {
-int samples_written;
+pos = 0;
+while (len) {
ssize_t bytes_written;
-int samples_till_end = hw->samples - oss->wpos;
-int samples_to_write = MIN (oss->pending, samples_till_end);
-int bytes_to_write = samples_to_write << hw->info.shift;
-void *pcm = advance (oss->pcm_buf, oss->wpos << hw->info.shift);
+void *pcm = advance(buf, pos);
-bytes_written = write (oss->fd, pcm, bytes_to_write);
+bytes_written = write(oss->fd, pcm, len);
if (bytes_written < 0) {
if (errno != EAGAIN) {
-oss_logerr (errno, "failed to write %d bytes\n",
-bytes_to_write);
+oss_logerr(errno, "failed to write %zu bytes\n",
+ len);
}
-break;
-}
-
-if (bytes_written & hw->info.align) {
-dolog ("misaligned write asked for %d, but got %zd\n",
- bytes_to_write, bytes_written);
-return;
+return pos;
}
-samples_written = bytes_written >> hw->info.shift;
-oss->pending -= samples_written;
-oss->wpos = (oss->wpos + samples_written) % hw->samples;
-if (bytes_written - bytes_to_write) {
+pos += bytes_written;
+if (bytes_written < len) {
break;
}
+len -= bytes_written;
}
-}
-
-static size_t oss_run_out(HWVoiceOut *hw, size_t live)
-{
-OSSVoiceOut *oss = (OSSVoiceOut *) hw;
-int err;
-size_t decr;
-struct audio_buf_info abinfo;
-struct count_info cntinfo;
-size_t bufsize;
-
-bufsize = hw->samples << hw->info.shift;
-
-if (oss->mmapped) {
-int bytes, pos;
-
-err = ioctl (oss->fd, SNDCTL_DSP_GETOPTR, );
-if (err < 0) {
-oss_logerr (errno, "SNDCTL_DSP_GETOPTR failed\n");
-return 0;
-}
-
-pos = hw->rpos << hw->info.shift;
-bytes = audio_ring_dist (cntinfo.ptr, pos, bufsize);
-decr = MIN (bytes >> hw->info.shift, live);
-}
-else {
-err = ioctl (oss->fd, SNDCTL_DSP_GETOSPACE, );
-if (err < 0) {
-oss_logerr (errno, "SNDCTL_DSP_GETOPTR failed\n");
-return 0;
-}
-
-if (abinfo.bytes > bufsize) {
-trace_oss_invalid_available_size(abinfo.bytes, bufsize);
-
[Qemu-devel] [PATCH 04/25] dsoundaudio: port to the new audio backend api
Signed-off-by: Kővágó, Zoltán
---
audio/dsound_template.h | 47 +++---
audio/dsoundaudio.c | 329 ++--
2 files changed, 103 insertions(+), 273 deletions(-)
diff --git a/audio/dsound_template.h b/audio/dsound_template.h
index 8ece870c9e..9f10b688df 100644
--- a/audio/dsound_template.h
+++ b/audio/dsound_template.h
@@ -29,6 +29,8 @@
#define BUFPTR LPDIRECTSOUNDCAPTUREBUFFER
#define FIELD dsound_capture_buffer
#define FIELD2 dsound_capture
+#define HWVOICE HWVoiceIn
+#define DSOUNDVOICE DSoundVoiceIn
#else
#define NAME "playback buffer"
#define NAME2 "DirectSound"
@@ -37,6 +39,8 @@
#define BUFPTR LPDIRECTSOUNDBUFFER
#define FIELD dsound_buffer
#define FIELD2 dsound
+#define HWVOICE HWVoiceOut
+#define DSOUNDVOICE DSoundVoiceOut
#endif
static int glue (dsound_unlock_, TYPE) (
@@ -72,8 +76,6 @@ static int glue (dsound_lock_, TYPE) (
)
{
HRESULT hr;
-LPVOID p1 = NULL, p2 = NULL;
-DWORD blen1 = 0, blen2 = 0;
DWORD flag;
#ifdef DSBTYPE_IN
@@ -81,7 +83,7 @@ static int glue (dsound_lock_, TYPE) (
#else
flag = entire ? DSBLOCK_ENTIREBUFFER : 0;
#endif
-hr = glue(IFACE, _Lock)(buf, pos, len, , , , , flag);
+hr = glue(IFACE, _Lock)(buf, pos, len, p1p, blen1p, p2p, blen2p, flag);
if (FAILED (hr)) {
#ifndef DSBTYPE_IN
@@ -96,34 +98,34 @@ static int glue (dsound_lock_, TYPE) (
goto fail;
}
-if ((p1 && (blen1 & info->align)) || (p2 && (blen2 & info->align))) {
-dolog ("DirectSound returned misaligned buffer %ld %ld\n",
- blen1, blen2);
-glue (dsound_unlock_, TYPE) (buf, p1, p2, blen1, blen2);
+if ((p1p && *p1p && (*blen1p & info->align)) ||
+(p2p && *p2p && (*blen2p & info->align))) {
+dolog("DirectSound returned misaligned buffer %ld %ld\n",
+ *blen1p, *blen2p);
+glue(dsound_unlock_, TYPE)(buf, *p1p, p2p ? *p2p : NULL, *blen1p,
+ blen2p ? *blen2p : 0);
goto fail;
}
-if (!p1 && blen1) {
-dolog ("warning: !p1 && blen1=%ld\n", blen1);
-blen1 = 0;
+if (p1p && !*p1p && *blen1p) {
+dolog("warning: !p1 && blen1=%ld\n", *blen1p);
+*blen1p = 0;
}
-if (!p2 && blen2) {
-dolog ("warning: !p2 && blen2=%ld\n", blen2);
-blen2 = 0;
+if (p2p && !*p2p && *blen2p) {
+dolog("warning: !p2 && blen2=%ld\n", *blen2p);
+*blen2p = 0;
}
-*p1p = p1;
-*p2p = p2;
-*blen1p = blen1;
-*blen2p = blen2;
return 0;
fail:
*p1p = NULL - 1;
-*p2p = NULL - 1;
*blen1p = -1;
-*blen2p = -1;
+if (p2p) {
+*p2p = NULL - 1;
+*blen2p = -1;
+}
return -1;
}
@@ -242,7 +244,6 @@ static int dsound_init_out(HWVoiceOut *hw, struct
audsettings *as,
goto fail0;
}
-ds->first_time = 1;
obt_as.endianness = 0;
audio_pcm_init_info (>info, _as);
@@ -252,15 +253,13 @@ static int dsound_init_out(HWVoiceOut *hw, struct
audsettings *as,
bc.dwBufferBytes, hw->info.align + 1
);
}
+hw->size_emul = bc.dwBufferBytes;
hw->samples = bc.dwBufferBytes >> hw->info.shift;
ds->s = s;
#ifdef DEBUG_DSOUND
dolog ("caps %ld, desc %ld\n",
bc.dwBufferBytes, bd.dwBufferBytes);
-
-dolog ("bufsize %d, freq %d, chan %d, fmt %d\n",
- hw->bufsize, settings.freq, settings.nchannels, settings.fmt);
#endif
return 0;
@@ -276,3 +275,5 @@ static int dsound_init_out(HWVoiceOut *hw, struct
audsettings *as,
#undef BUFPTR
#undef FIELD
#undef FIELD2
+#undef HWVOICE
+#undef DSOUNDVOICE
diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c
index 2fc118b795..96693a97e5 100644
--- a/audio/dsoundaudio.c
+++ b/audio/dsoundaudio.c
@@ -53,19 +53,11 @@ typedef struct {
typedef struct {
HWVoiceOut hw;
LPDIRECTSOUNDBUFFER dsound_buffer;
-DWORD old_pos;
-int first_time;
dsound *s;
-#ifdef DEBUG_DSOUND
-DWORD old_ppos;
-DWORD played;
-DWORD mixed;
-#endif
} DSoundVoiceOut;
typedef struct {
HWVoiceIn hw;
-int first_time;
LPDIRECTSOUNDCAPTUREBUFFER dsound_capture_buffer;
dsound *s;
} DSoundVoiceIn;
@@ -243,11 +235,6 @@ static void GCC_FMT_ATTR (3, 4) dsound_logerr2 (
dsound_log_hresult (hr);
}
-static uint64_t usecs_to_bytes(struct audio_pcm_info *info, uint32_t usecs)
-{
-return muldiv64(usecs, info->bytes_per_second, 100);
-}
-
#ifdef DEBUG_DSOUND
static void print_wave_format (WAVEFORMATEX *wfx)
{
@@ -312,33 +299,6 @@ static int dsound_get_status_in
(LPDIRECTSOUNDCAPTUREBUFFER dscb,
return 0;
}
-static void dsound_write_sample (HWVoiceOut *hw, uint8_t *dst, int dst_len)
-{
-int src_len1 = dst_len;
-int src_len2 = 0;
-int pos = hw->rpos + dst_len;
-struct st_sample *src1 = hw->mix_buf + hw->rpos;
-struct st_sample *src2 = NULL;
-
-if (pos > hw->samples) {
-src_len1 = hw->samples -
[Qemu-devel] [PATCH 10/25] wavaudio: port to the new audio backend api
Signed-off-by: Kővágó, Zoltán
---
audio/wavaudio.c | 54
1 file changed, 9 insertions(+), 45 deletions(-)
diff --git a/audio/wavaudio.c b/audio/wavaudio.c
index b6eeeb4e26..7816097db8 100644
--- a/audio/wavaudio.c
+++ b/audio/wavaudio.c
@@ -36,52 +36,28 @@ typedef struct WAVVoiceOut {
HWVoiceOut hw;
FILE *f;
int64_t old_ticks;
-void *pcm_buf;
int total_samples;
} WAVVoiceOut;
-static size_t wav_run_out(HWVoiceOut *hw, size_t live)
+static size_t wav_write_out(HWVoiceOut *hw, void *buf, size_t len)
{
WAVVoiceOut *wav = (WAVVoiceOut *) hw;
-size_t rpos, decr, samples;
-uint8_t *dst;
-struct st_sample *src;
int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
int64_t ticks = now - wav->old_ticks;
int64_t bytes =
muldiv64(ticks, hw->info.bytes_per_second, NANOSECONDS_PER_SECOND);
-if (bytes > INT_MAX) {
-samples = INT_MAX >> hw->info.shift;
-}
-else {
-samples = bytes >> hw->info.shift;
-}
-
+bytes = MIN(bytes, len);
+bytes = bytes >> hw->info.shift << hw->info.shift;
wav->old_ticks = now;
-decr = MIN (live, samples);
-samples = decr;
-rpos = hw->rpos;
-while (samples) {
-int left_till_end_samples = hw->samples - rpos;
-int convert_samples = MIN (samples, left_till_end_samples);
-src = hw->mix_buf + rpos;
-dst = advance (wav->pcm_buf, rpos << hw->info.shift);
-
-hw->clip (dst, src, convert_samples);
-if (fwrite (dst, convert_samples << hw->info.shift, 1, wav->f) != 1) {
-dolog ("wav_run_out: fwrite of %d bytes failed\nReaons: %s\n",
- convert_samples << hw->info.shift, strerror (errno));
-}
-
-rpos = (rpos + convert_samples) % hw->samples;
-samples -= convert_samples;
-wav->total_samples += convert_samples;
+if (bytes && fwrite(buf, bytes, 1, wav->f) != 1) {
+dolog("wav_write_out: fwrite of %zu bytes failed\nReaons: %s\n",
+ bytes, strerror(errno));
}
-hw->rpos = rpos;
-return decr;
+wav->total_samples += bytes >> hw->info.shift;
+return bytes;
}
/* VICE code: Store number as little endian. */
@@ -137,13 +113,6 @@ static int wav_init_out(HWVoiceOut *hw, struct audsettings
*as,
audio_pcm_init_info (>info, _as);
hw->samples = 1024;
-wav->pcm_buf = audio_calloc(__func__, hw->samples, 1 << hw->info.shift);
-if (!wav->pcm_buf) {
-dolog("Could not allocate buffer (%zu bytes)\n",
- hw->samples << hw->info.shift);
-return -1;
-}
-
le_store (hdr + 22, hw->info.nchannels, 2);
le_store (hdr + 24, hw->info.freq, 4);
le_store (hdr + 28, hw->info.freq << (bits16 + stereo), 4);
@@ -153,8 +122,6 @@ static int wav_init_out(HWVoiceOut *hw, struct audsettings
*as,
if (!wav->f) {
dolog ("Failed to open wave file `%s'\nReason: %s\n",
wav_path, strerror(errno));
-g_free (wav->pcm_buf);
-wav->pcm_buf = NULL;
return -1;
}
@@ -208,9 +175,6 @@ static void wav_fini_out (HWVoiceOut *hw)
wav->f, strerror (errno));
}
wav->f = NULL;
-
-g_free (wav->pcm_buf);
-wav->pcm_buf = NULL;
}
static int wav_ctl_out (HWVoiceOut *hw, int cmd, ...)
@@ -234,7 +198,7 @@ static void wav_audio_fini (void *opaque)
static struct audio_pcm_ops wav_pcm_ops = {
.init_out = wav_init_out,
.fini_out = wav_fini_out,
-.run_out = wav_run_out,
+.write= wav_write_out,
.ctl_out = wav_ctl_out,
};
--
2.22.0
[Qemu-devel] [PATCH 20/25] audio: replace shift in audio_pcm_info with bytes_per_frame
The bit shifting trick worked because the number of bytes per frame was
always a power-of-two (since QEMU only supports mono, stereo and 8, 16
and 32 bit samples). But if we want to add support for surround sound,
this no longer holds true.
Signed-off-by: Kővágó, Zoltán
---
audio/audio_int.h | 3 +-
audio/dsound_template.h | 10 +++---
audio/alsaaudio.c | 11 +++---
audio/audio.c | 74 -
audio/coreaudio.c | 4 +--
audio/dsoundaudio.c | 4 +--
audio/noaudio.c | 2 +-
audio/ossaudio.c| 14
audio/spiceaudio.c | 5 +--
audio/wavaudio.c| 6 ++--
10 files changed, 67 insertions(+), 66 deletions(-)
diff --git a/audio/audio_int.h b/audio/audio_int.h
index e826734f8c..29fb0ededd 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -43,8 +43,7 @@ struct audio_pcm_info {
int sign;
int freq;
int nchannels;
-int align;
-int shift;
+int bytes_per_frame;
int bytes_per_second;
int swap_endianness;
};
diff --git a/audio/dsound_template.h b/audio/dsound_template.h
index 8cc5d69f36..ebc6ba04c1 100644
--- a/audio/dsound_template.h
+++ b/audio/dsound_template.h
@@ -98,8 +98,8 @@ static int glue (dsound_lock_, TYPE) (
goto fail;
}
-if ((p1p && *p1p && (*blen1p & info->align)) ||
-(p2p && *p2p && (*blen2p & info->align))) {
+if ((p1p && *p1p && (*blen1p % info->bytes_per_frame)) ||
+(p2p && *p2p && (*blen2p % info->bytes_per_frame))) {
dolog("DirectSound returned misaligned buffer %ld %ld\n",
*blen1p, *blen2p);
glue(dsound_unlock_, TYPE)(buf, *p1p, p2p ? *p2p : NULL, *blen1p,
@@ -247,14 +247,14 @@ static int dsound_init_out(HWVoiceOut *hw, struct
audsettings *as,
obt_as.endianness = 0;
audio_pcm_init_info (>info, _as);
-if (bc.dwBufferBytes & hw->info.align) {
+if (bc.dwBufferBytes % hw->info.bytes_per_frame) {
dolog (
"GetCaps returned misaligned buffer size %ld, alignment %d\n",
-bc.dwBufferBytes, hw->info.align + 1
+bc.dwBufferBytes, hw->info.bytes_per_frame
);
}
hw->size_emul = bc.dwBufferBytes;
-ds->samples = bc.dwBufferBytes >> hw->info.shift;
+ds->samples = bc.dwBufferBytes / hw->info.bytes_per_frame;
ds->s = s;
#ifdef DEBUG_DSOUND
diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index 95468bcf6d..b3b21e07a2 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -604,7 +604,7 @@ static size_t alsa_write(HWVoiceOut *hw, void *buf, size_t
len)
{
ALSAVoiceOut *alsa = (ALSAVoiceOut *) hw;
size_t pos = 0;
-size_t len_frames = len >> hw->info.shift;
+size_t len_frames = len / hw->info.bytes_per_frame;
while (len_frames) {
char *src = advance(buf, pos);
@@ -650,7 +650,7 @@ static size_t alsa_write(HWVoiceOut *hw, void *buf, size_t
len)
}
}
-pos += written << hw->info.shift;
+pos += written * hw->info.bytes_per_frame;
if (written < len_frames) {
break;
}
@@ -816,7 +816,8 @@ static size_t alsa_read(HWVoiceIn *hw, void *buf, size_t
len)
void *dst = advance(buf, pos);
snd_pcm_sframes_t nread;
-nread = snd_pcm_readi(alsa->handle, dst, len >> hw->info.shift);
+nread = snd_pcm_readi(
+alsa->handle, dst, len / hw->info.bytes_per_frame);
if (nread <= 0) {
switch (nread) {
@@ -842,8 +843,8 @@ static size_t alsa_read(HWVoiceIn *hw, void *buf, size_t
len)
}
}
-pos += nread << hw->info.shift;
-len -= nread << hw->info.shift;
+pos += nread * hw->info.bytes_per_frame;
+len -= nread * hw->info.bytes_per_frame;
}
return pos;
diff --git a/audio/audio.c b/audio/audio.c
index 60a1d16dea..f46bd5dc3d 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -299,12 +299,13 @@ static int audio_pcm_info_eq (struct audio_pcm_info
*info, struct audsettings *a
void audio_pcm_init_info (struct audio_pcm_info *info, struct audsettings *as)
{
-int bits = 8, sign = 0, shift = 0;
+int bits = 8, sign = 0, mul;
switch (as->fmt) {
case AUDIO_FORMAT_S8:
sign = 1;
case AUDIO_FORMAT_U8:
+mul = 1;
break;
case AUDIO_FORMAT_S16:
@@ -312,7 +313,7 @@ void audio_pcm_init_info (struct audio_pcm_info *info,
struct audsettings *as)
/* fall through */
case AUDIO_FORMAT_U16:
bits = 16;
-shift = 1;
+mul = 2;
break;
case AUDIO_FORMAT_S32:
@@ -320,7 +321,7 @@ void audio_pcm_init_info (struct audio_pcm_info *info,
struct audsettings *as)
/* fall through */
case AUDIO_FORMAT_U32:
bits = 32;
-shift = 2;
+mul = 4;
break;
default:
@@ -331,9 +332,8 @@ void audio_pcm_init_info (struct audio_pcm_info *info,
struct audsettings *as)
[Qemu-devel] [PATCH 03/25] coreaudio: port to the new audio backend api
Signed-off-by: Kővágó, Zoltán
---
audio/coreaudio.c | 130 --
1 file changed, 69 insertions(+), 61 deletions(-)
diff --git a/audio/coreaudio.c b/audio/coreaudio.c
index d1be58b40a..5cde42f982 100644
--- a/audio/coreaudio.c
+++ b/audio/coreaudio.c
@@ -43,9 +43,6 @@ typedef struct coreaudioVoiceOut {
UInt32 audioDevicePropertyBufferFrameSize;
AudioStreamBasicDescription outputStreamBasicDescription;
AudioDeviceIOProcID ioprocid;
-size_t live;
-size_t decr;
-size_t rpos;
} coreaudioVoiceOut;
#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_6
@@ -397,31 +394,29 @@ static int coreaudio_unlock (coreaudioVoiceOut *core,
const char *fn_name)
return 0;
}
-static size_t coreaudio_run_out(HWVoiceOut *hw, size_t live)
-{
-size_t decr;
-coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw;
-
-if (coreaudio_lock (core, "coreaudio_run_out")) {
-return 0;
+#define COREAUDIO_WRAPPER_FUNC(name, ret_type, args_decl, args) \
+static ret_type glue(coreaudio_, name)args_decl \
+{ \
+coreaudioVoiceOut *core = (coreaudioVoiceOut *) hw; \
+ret_type ret; \
+\
+if (coreaudio_lock(core, "coreaudio_" #name)) { \
+return 0; \
+} \
+\
+ret = glue(audio_generic_, name)args; \
+\
+coreaudio_unlock(core, "coreaudio_" #name); \
+return ret; \
}
-
-if (core->decr > live) {
-ldebug ("core->decr %d live %d core->live %d\n",
-core->decr,
-live,
-core->live);
-}
-
-decr = MIN (core->decr, live);
-core->decr -= decr;
-
-core->live = live - decr;
-hw->rpos = core->rpos;
-
-coreaudio_unlock (core, "coreaudio_run_out");
-return decr;
-}
+COREAUDIO_WRAPPER_FUNC(get_buffer_out, void *, (HWVoiceOut *hw, size_t *size),
+ (hw, size))
+COREAUDIO_WRAPPER_FUNC(put_buffer_out_nowrite, size_t,
+ (HWVoiceOut *hw, void *buf, size_t size),
+ (hw, buf, size))
+COREAUDIO_WRAPPER_FUNC(write, size_t, (HWVoiceOut *hw, void *buf, size_t size),
+ (hw, buf, size))
+#undef COREAUDIO_WRAPPER_FUNC
/* callback to feed audiooutput buffer */
static OSStatus audioDeviceIOProc(
@@ -433,19 +428,11 @@ static OSStatus audioDeviceIOProc(
const AudioTimeStamp* inOutputTime,
void* hwptr)
{
-UInt32 frame, frameCount;
-float *out = outOutputData->mBuffers[0].mData;
+UInt32 frameCount, pending_frames;
+void *out = outOutputData->mBuffers[0].mData;
HWVoiceOut *hw = hwptr;
coreaudioVoiceOut *core = (coreaudioVoiceOut *) hwptr;
-int rpos, live;
-struct st_sample *src;
-#ifndef FLOAT_MIXENG
-#ifdef RECIPROCAL
-const float scale = 1.f / UINT_MAX;
-#else
-const float scale = UINT_MAX;
-#endif
-#endif
+size_t len;
if (coreaudio_lock (core, "audioDeviceIOProc")) {
inInputTime = 0;
@@ -453,42 +440,51 @@ static OSStatus audioDeviceIOProc(
}
frameCount = core->audioDevicePropertyBufferFrameSize;
-live = core->live;
+pending_frames = hw->pending_emul >> hw->info.shift;
/* if there are not enough samples, set signal and return */
-if (live < frameCount) {
+if (pending_frames < frameCount) {
inInputTime = 0;
coreaudio_unlock (core, "audioDeviceIOProc(empty)");
return 0;
}
-rpos = core->rpos;
-src = hw->mix_buf + rpos;
+len = frameCount << hw->info.shift;
+while (len) {
+size_t write_len;
+ssize_t start = ((ssize_t) hw->pos_emul) - hw->pending_emul;
+if (start < 0) {
+start += hw->size_emul;
+}
+assert(start >= 0 && start < hw->size_emul);
-/* fill buffer */
-for (frame = 0; frame < frameCount; frame++) {
-#ifdef FLOAT_MIXENG
-*out++ = src[frame].l; /* left channel */
-*out++ = src[frame].r; /* right channel */
-#else
-#ifdef RECIPROCAL
-*out++ = src[frame].l * scale; /* left channel */
-*out++ = src[frame].r * scale; /* right channel */
-#else
-*out++ = src[frame].l / scale; /* left channel */
-*out++ = src[frame].r / scale; /* right channel */
-#endif
-#endif
+write_len = MIN(MIN(hw->pending_emul, len),
+hw->size_emul - start);
+
+memcpy(out, hw->buf_emul + start, write_len);
+hw->pending_emul -= write_len;
+len -= write_len;
+out
[Qemu-devel] [PATCH 02/25] alsaaudio: port to the new audio backend api
Signed-off-by: Kővágó, Zoltán
---
audio/alsaaudio.c | 308 +-
1 file changed, 83 insertions(+), 225 deletions(-)
diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index 591344dccd..19124d09d8 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -44,9 +44,6 @@ struct pollhlp {
typedef struct ALSAVoiceOut {
HWVoiceOut hw;
-int wpos;
-int pending;
-void *pcm_buf;
snd_pcm_t *handle;
struct pollhlp pollhlp;
Audiodev *dev;
@@ -55,7 +52,6 @@ typedef struct ALSAVoiceOut {
typedef struct ALSAVoiceIn {
HWVoiceIn hw;
snd_pcm_t *handle;
-void *pcm_buf;
struct pollhlp pollhlp;
Audiodev *dev;
} ALSAVoiceIn;
@@ -602,102 +598,64 @@ static int alsa_open(bool in, struct alsa_params_req
*req,
return -1;
}
-static snd_pcm_sframes_t alsa_get_avail (snd_pcm_t *handle)
-{
-snd_pcm_sframes_t avail;
-
-avail = snd_pcm_avail_update (handle);
-if (avail < 0) {
-if (avail == -EPIPE) {
-if (!alsa_recover (handle)) {
-avail = snd_pcm_avail_update (handle);
-}
-}
-
-if (avail < 0) {
-alsa_logerr (avail,
- "Could not obtain number of available frames\n");
-return -1;
-}
-}
-
-return avail;
-}
-
-static void alsa_write_pending (ALSAVoiceOut *alsa)
-{
-HWVoiceOut *hw = >hw;
-
-while (alsa->pending) {
-int left_till_end_samples = hw->samples - alsa->wpos;
-int len = MIN (alsa->pending, left_till_end_samples);
-char *src = advance (alsa->pcm_buf, alsa->wpos << hw->info.shift);
-
-while (len) {
-snd_pcm_sframes_t written;
-
-written = snd_pcm_writei (alsa->handle, src, len);
-
-if (written <= 0) {
-switch (written) {
-case 0:
-trace_alsa_wrote_zero(len);
-return;
-
-case -EPIPE:
-if (alsa_recover (alsa->handle)) {
-alsa_logerr (written, "Failed to write %d frames\n",
- len);
-return;
-}
-trace_alsa_xrun_out();
-continue;
-
-case -ESTRPIPE:
-/* stream is suspended and waiting for an
- application recovery */
-if (alsa_resume (alsa->handle)) {
-alsa_logerr (written, "Failed to write %d frames\n",
- len);
-return;
-}
-trace_alsa_resume_out();
-continue;
-
-case -EAGAIN:
-return;
-
-default:
-alsa_logerr (written, "Failed to write %d frames from
%p\n",
- len, src);
-return;
-}
-}
-
-alsa->wpos = (alsa->wpos + written) % hw->samples;
-alsa->pending -= written;
-len -= written;
-}
-}
-}
-
-static size_t alsa_run_out(HWVoiceOut *hw, size_t live)
+static size_t alsa_write(HWVoiceOut *hw, void *buf, size_t len)
{
ALSAVoiceOut *alsa = (ALSAVoiceOut *) hw;
-size_t decr;
-snd_pcm_sframes_t avail;
+size_t pos = 0;
+size_t len_frames = len >> hw->info.shift;
-avail = alsa_get_avail (alsa->handle);
-if (avail < 0) {
-dolog ("Could not get number of available playback frames\n");
-return 0;
+while (len_frames) {
+char *src = advance(buf, pos);
+snd_pcm_sframes_t written;
+
+written = snd_pcm_writei(alsa->handle, src, len_frames);
+
+if (written <= 0) {
+switch (written) {
+case 0:
+trace_alsa_wrote_zero(len_frames);
+return pos;
+
+case -EPIPE:
+if (alsa_recover(alsa->handle)) {
+alsa_logerr(written, "Failed to write %zu frames\n",
+len_frames);
+return pos;
+}
+trace_alsa_xrun_out();
+continue;
+
+case -ESTRPIPE:
+/*
+ * stream is suspended and waiting for an application
+ * recovery
+ */
+if (alsa_resume(alsa->handle)) {
+alsa_logerr(written, "Failed to write %zu frames\n",
+len_frames);
+return pos;
+}
+trace_alsa_resume_out();
+continue;
+
+case -EAGAIN:
+return pos;
+
+default:
+alsa_logerr(written, "Failed to write %zu frames from %p\n",
+len, src);
+return pos;
+
[Qemu-devel] [PATCH 08/25] sdlaudio: port to the new audio backend api
Signed-off-by: Kővágó, Zoltán
---
audio/sdlaudio.c | 87 +++-
1 file changed, 42 insertions(+), 45 deletions(-)
diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
index 14b11f0335..f7ac8cd101 100644
--- a/audio/sdlaudio.c
+++ b/audio/sdlaudio.c
@@ -41,8 +41,6 @@
typedef struct SDLVoiceOut {
HWVoiceOut hw;
-size_t live;
-size_t decr;
} SDLVoiceOut;
static struct SDLAudioState {
@@ -184,62 +182,59 @@ static void sdl_callback (void *opaque, Uint8 *buf, int
len)
SDLVoiceOut *sdl = opaque;
SDLAudioState *s = _sdl;
HWVoiceOut *hw = >hw;
-size_t samples = len >> hw->info.shift;
-size_t to_mix, decr;
-if (s->exit || !sdl->live) {
+if (s->exit) {
return;
}
/* dolog ("in callback samples=%zu live=%zu\n", samples, sdl->live); */
-to_mix = MIN(samples, sdl->live);
-decr = to_mix;
-while (to_mix) {
-size_t chunk = MIN(to_mix, hw->samples - hw->rpos);
-struct st_sample *src = hw->mix_buf + hw->rpos;
-
-/* dolog ("in callback to_mix %zu, chunk %zu\n", to_mix, chunk); */
-hw->clip(buf, src, chunk);
-hw->rpos = (hw->rpos + chunk) % hw->samples;
-to_mix -= chunk;
-buf += chunk << hw->info.shift;
+while (hw->pending_emul && len) {
+size_t write_len;
+ssize_t start = ((ssize_t) hw->pos_emul) - hw->pending_emul;
+if (start < 0) {
+start += hw->size_emul;
+}
+assert(start >= 0 && start < hw->size_emul);
+
+write_len = MIN(MIN(hw->pending_emul, len),
+hw->size_emul - start);
+
+memcpy(buf, hw->buf_emul + start, write_len);
+hw->pending_emul -= write_len;
+len -= write_len;
+buf += write_len;
}
-samples -= decr;
-sdl->live -= decr;
-sdl->decr += decr;
-
-/* dolog ("done len=%zu\n", len); */
-/* SDL2 does not clear the remaining buffer for us, so do it on our own */
-if (samples) {
-memset(buf, 0, samples << hw->info.shift);
+/* clear remaining buffer that we couldn't fill with data */
+if (len) {
+memset(buf, 0, len);
}
}
-static size_t sdl_run_out(HWVoiceOut *hw, size_t live)
-{
-size_t decr;
-SDLVoiceOut *sdl = (SDLVoiceOut *) hw;
-
-SDL_LockAudio();
-
-if (sdl->decr > live) {
-ldebug ("sdl->decr %d live %d sdl->live %d\n",
-sdl->decr,
-live,
-sdl->live);
+#define SDL_WRAPPER_FUNC(name, ret_type, args_decl, args, fail, unlock) \
+static ret_type glue(sdl_, name)args_decl \
+{ \
+ret_type ret; \
+\
+SDL_LockAudio();\
+\
+ret = glue(audio_generic_, name)args; \
+\
+SDL_UnlockAudio(); \
+return ret; \
}
-decr = MIN (sdl->decr, live);
-sdl->decr -= decr;
+SDL_WRAPPER_FUNC(get_buffer_out, void *, (HWVoiceOut *hw, size_t *size),
+ (hw, size), *size = 0, sdl_unlock)
+SDL_WRAPPER_FUNC(put_buffer_out_nowrite, size_t,
+ (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size),
+ /*nothing*/, sdl_unlock_and_post)
+SDL_WRAPPER_FUNC(write, size_t,
+ (HWVoiceOut *hw, void *buf, size_t size), (hw, buf, size),
+ /*nothing*/, sdl_unlock_and_post)
-sdl->live = live;
-
-SDL_UnlockAudio();
-
-return decr;
-}
+#undef SDL_WRAPPER_FUNC
static void sdl_fini_out (HWVoiceOut *hw)
{
@@ -336,7 +331,9 @@ static void sdl_audio_fini (void *opaque)
static struct audio_pcm_ops sdl_pcm_ops = {
.init_out = sdl_init_out,
.fini_out = sdl_fini_out,
-.run_out = sdl_run_out,
+.write= sdl_write,
+.get_buffer_out = sdl_get_buffer_out,
+.put_buffer_out = sdl_put_buffer_out_nowrite,
.ctl_out = sdl_ctl_out,
};
--
2.22.0
[Qemu-devel] [PATCH 01/25] audio: api for mixeng code free backends
Signed-off-by: Kővágó, Zoltán
---
audio/audio_int.h | 41 ++--
audio/audio_template.h | 1 +
audio/audio.c | 211 -
3 files changed, 245 insertions(+), 8 deletions(-)
diff --git a/audio/audio_int.h b/audio/audio_int.h
index a674c5374a..8fb1ca8a8d 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -65,6 +65,8 @@ typedef struct HWVoiceOut {
uint64_t ts_helper;
struct st_sample *mix_buf;
+void *buf_emul;
+size_t pos_emul, pending_emul, size_emul;
size_t samples;
QLIST_HEAD (sw_out_listhead, SWVoiceOut) sw_head;
@@ -87,6 +89,8 @@ typedef struct HWVoiceIn {
uint64_t ts_helper;
struct st_sample *conv_buf;
+void *buf_emul;
+size_t pos_emul, pending_emul, size_emul;
size_t samples;
QLIST_HEAD (sw_in_listhead, SWVoiceIn) sw_head;
@@ -147,17 +151,42 @@ struct audio_driver {
};
struct audio_pcm_ops {
-int (*init_out)(HWVoiceOut *hw, struct audsettings *as, void *drv_opaque);
-void (*fini_out)(HWVoiceOut *hw);
+int(*init_out)(HWVoiceOut *hw, audsettings *as, void *drv_opaque);
+void (*fini_out)(HWVoiceOut *hw);
size_t (*run_out)(HWVoiceOut *hw, size_t live);
-int (*ctl_out) (HWVoiceOut *hw, int cmd, ...);
+size_t (*write) (HWVoiceOut *hw, void *buf, size_t size);
+/*
+ * get a buffer that after later can be passed to put_buffer_out; optional
+ * returns the buffer, and writes it's size to size (in bytes)
+ * this is unrelated to the above buffer_size_out function
+ */
+void *(*get_buffer_out)(HWVoiceOut *hw, size_t *size);
+/*
+ * put back the buffer returned by get_buffer_out; optional
+ * buf must be equal the pointer returned by get_buffer_out,
+ * size may be smaller
+ */
+size_t (*put_buffer_out)(HWVoiceOut *hw, void *buf, size_t size);
+int(*ctl_out) (HWVoiceOut *hw, int cmd, ...);
-int (*init_in) (HWVoiceIn *hw, struct audsettings *as, void *drv_opaque);
-void (*fini_in) (HWVoiceIn *hw);
+int(*init_in) (HWVoiceIn *hw, audsettings *as, void *drv_opaque);
+void (*fini_in) (HWVoiceIn *hw);
size_t (*run_in)(HWVoiceIn *hw);
-int (*ctl_in) (HWVoiceIn *hw, int cmd, ...);
+size_t (*read)(HWVoiceIn *hw, void *buf, size_t size);
+void *(*get_buffer_in)(HWVoiceIn *hw, size_t *size);
+void (*put_buffer_in)(HWVoiceIn *hw, void *buf, size_t size);
+int(*ctl_in) (HWVoiceIn *hw, int cmd, ...);
};
+void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size);
+void audio_generic_put_buffer_in(HWVoiceIn *hw, void *buf, size_t size);
+void *audio_generic_get_buffer_out(HWVoiceOut *hw, size_t *size);
+size_t audio_generic_put_buffer_out(HWVoiceOut *hw, void *buf, size_t size);
+size_t audio_generic_put_buffer_out_nowrite(HWVoiceOut *hw, void *buf,
+size_t size);
+size_t audio_generic_write(HWVoiceOut *hw, void *buf, size_t size);
+size_t audio_generic_read(HWVoiceIn *hw, void *buf, size_t size);
+
struct capture_callback {
struct audio_capture_ops ops;
void *opaque;
diff --git a/audio/audio_template.h b/audio/audio_template.h
index 2562bf5f00..ff4a173f18 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -71,6 +71,7 @@ static void glue(audio_init_nb_voices_, TYPE)(AudioState *s,
static void glue (audio_pcm_hw_free_resources_, TYPE) (HW *hw)
{
+g_free(hw->buf_emul);
g_free (HWBUF);
HWBUF = NULL;
}
diff --git a/audio/audio.c b/audio/audio.c
index 7d715332c9..0dfcfeaf06 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -573,6 +573,25 @@ size_t audio_pcm_hw_clip_out(HWVoiceOut *hw, void *pcm_buf,
return clipped;
}
+static void audio_pcm_hw_clip_out2(HWVoiceOut *hw, void *pcm_buf, size_t len)
+{
+size_t clipped = 0;
+size_t pos = hw->rpos;
+
+while (len) {
+st_sample *src = hw->mix_buf + pos;
+uint8_t *dst = advance(pcm_buf, clipped << hw->info.shift);
+size_t samples_till_end_of_buf = hw->samples - pos;
+size_t samples_to_clip = MIN(len, samples_till_end_of_buf);
+
+hw->clip(dst, src, samples_to_clip);
+
+pos = (pos + samples_to_clip) % hw->samples;
+len -= samples_to_clip;
+clipped += samples_to_clip;
+}
+}
+
/*
* Soft voice (capture)
*/
@@ -1050,6 +1069,31 @@ static void audio_capture_mix_and_clear(HWVoiceOut *hw,
size_t rpos,
mixeng_clear(hw->mix_buf, samples - n);
}
+static size_t audio_pcm_hw_run_out(HWVoiceOut *hw, size_t live)
+{
+size_t clipped = 0;
+
+while (live) {
+size_t size, decr, proc;
+void *buf = hw->pcm_ops->get_buffer_out(hw, );
+
+decr = MIN(size >> hw->info.shift, live);
+audio_pcm_hw_clip_out2(hw, buf, decr);
+proc = hw->pcm_ops->put_buffer_out(hw, buf, decr << hw->info.shift) >>
+hw->info.shift;
+
+live -= proc;
+clipped += proc;
+hw->rpos =
[Qemu-devel] [PATCH 00/25] Audio: Mixeng-free 5.1/7.1 audio support
Hi, This is the final part of my audio patch series that finally makes mixeng optional and lifts the restriction of only supporting two channels of audio. It probably gained a bit of dust in the last few years, I've fixed most issues reported by checkpatch, but there might be other problems. Regards, Zoltan Kővágó, Zoltán (25): audio: api for mixeng code free backends alsaaudio: port to the new audio backend api coreaudio: port to the new audio backend api dsoundaudio: port to the new audio backend api noaudio: port to the new audio backend api ossaudio: port to the new audio backend api paaudio: port to the new audio backend api sdlaudio: port to the new audio backend api spiceaudio: port to the new audio backend api wavaudio: port to the new audio backend api audio: remove remains of the old backend api audio: unify input and output mixeng buffer management audio: remove hw->samples, buffer_size_in/out pcm_ops audio: common rate control code for timer based outputs audio: split ctl_* functions into enable_* and volume_* audio: add mixeng option (documentation) audio: make mixeng optional paaudio: get/put_buffer functions audio: support more than two channels in volume setting audio: replace shift in audio_pcm_info with bytes_per_frame audio: basic support for multichannel audio paaudio: channel-map option usb-audio: do not count on avail bytes actually available usb-audio: support more than two channels of audio usbaudio: change playback counters to 64 bit configure | 5 - qapi/audio.json | 12 +- audio/audio.h | 10 + audio/audio_int.h | 84 -- audio/audio_pt_int.h| 22 -- audio/audio_template.h | 55 ++-- audio/dsound_template.h | 59 +++-- audio/alsaaudio.c | 398 +--- audio/audio.c | 524 audio/audio_pt_int.c| 173 audio/coreaudio.c | 153 ++- audio/dsoundaudio.c | 387 --- audio/noaudio.c | 78 +++--- audio/ossaudio.c| 392 +++ audio/paaudio.c | 573 +--- audio/sdlaudio.c| 112 audio/spiceaudio.c | 270 +++ audio/wavaudio.c| 79 ++ hw/usb/dev-audio.c | 461 +++- audio/Makefile.objs | 1 - qemu-options.hx | 15 ++ 21 files changed, 1815 insertions(+), 2048 deletions(-) delete mode 100644 audio/audio_pt_int.h delete mode 100644 audio/audio_pt_int.c -- 2.22.0
[Qemu-devel] [PATCH 05/25] noaudio: port to the new audio backend api
Signed-off-by: Kővágó, Zoltán
---
audio/noaudio.c | 39 +++
1 file changed, 15 insertions(+), 24 deletions(-)
diff --git a/audio/noaudio.c b/audio/noaudio.c
index 0fb2629cf2..b054fd225b 100644
--- a/audio/noaudio.c
+++ b/audio/noaudio.c
@@ -41,10 +41,9 @@ typedef struct NoVoiceIn {
int64_t old_ticks;
} NoVoiceIn;
-static size_t no_run_out(HWVoiceOut *hw, size_t live)
+static size_t no_write(HWVoiceOut *hw, void *buf, size_t len)
{
NoVoiceOut *no = (NoVoiceOut *) hw;
-size_t decr, samples;
int64_t now;
int64_t ticks;
int64_t bytes;
@@ -52,13 +51,9 @@ static size_t no_run_out(HWVoiceOut *hw, size_t live)
now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
ticks = now - no->old_ticks;
bytes = muldiv64(ticks, hw->info.bytes_per_second, NANOSECONDS_PER_SECOND);
-bytes = MIN(bytes, SIZE_MAX);
-samples = bytes >> hw->info.shift;
no->old_ticks = now;
-decr = MIN (live, samples);
-hw->rpos = (hw->rpos + decr) % hw->samples;
-return decr;
+return MIN(len, bytes);
}
static int no_init_out(HWVoiceOut *hw, struct audsettings *as, void
*drv_opaque)
@@ -92,25 +87,21 @@ static void no_fini_in (HWVoiceIn *hw)
(void) hw;
}
-static size_t no_run_in(HWVoiceIn *hw)
+static size_t no_read(HWVoiceIn *hw, void *buf, size_t size)
{
+size_t to_clear;
NoVoiceIn *no = (NoVoiceIn *) hw;
-size_t live = audio_pcm_hw_get_live_in(hw);
-size_t dead = hw->samples - live;
-size_t samples = 0;
-if (dead) {
-int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-int64_t ticks = now - no->old_ticks;
-int64_t bytes =
-muldiv64(ticks, hw->info.bytes_per_second, NANOSECONDS_PER_SECOND);
+int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+int64_t ticks = now - no->old_ticks;
+int64_t bytes =
+muldiv64(ticks, hw->info.bytes_per_second, NANOSECONDS_PER_SECOND);
-no->old_ticks = now;
-bytes = MIN (bytes, SIZE_MAX);
-samples = bytes >> hw->info.shift;
-samples = MIN (samples, dead);
-}
-return samples;
+no->old_ticks = now;
+to_clear = MIN(bytes, size);
+
+audio_pcm_info_clear_buf(>info, buf, to_clear >> hw->info.shift);
+return to_clear;
}
static int no_ctl_in (HWVoiceIn *hw, int cmd, ...)
@@ -133,12 +124,12 @@ static void no_audio_fini (void *opaque)
static struct audio_pcm_ops no_pcm_ops = {
.init_out = no_init_out,
.fini_out = no_fini_out,
-.run_out = no_run_out,
+.write= no_write,
.ctl_out = no_ctl_out,
.init_in = no_init_in,
.fini_in = no_fini_in,
-.run_in = no_run_in,
+.read = no_read,
.ctl_in = no_ctl_in
};
--
2.22.0
Re: [Qemu-devel] [PULL 07/15] audio: audiodev= parameters no longer optional when -audiodev present
On 2019-08-25 11:44, Maxim Levitsky wrote: > On Wed, 2019-08-21 at 10:41 +0200, Gerd Hoffmann wrote: >> From: Kővágó, Zoltán >> >> This means you should probably stop using -soundhw (as it doesn't allow >> you to specify any options) and add the device manually with -device. >> The exception is pcspk, it's currently not possible to manually add it. >> To use it with audiodev, use something like this: >> >> -audiodev id=foo,... -global isa-pcspk.audiodev=foo -soundhw pcspk > > Hi! Hi, > There is one corner case this breaks. > In qemu 4.1.0, there is no way to specify audiodev for a sound device, > specifying it > fails with error. > So some of my machines have audiodev (which is miles better that using old > env variables) > but also have sound devices without audiodev reference since this wasn't > supported. > > > In what will be qemu 4.2, you must specify it, thus this kind of breaks > backward compatibility. > Maybe we can have audiodev reference optional for a version or two? > > This is just a minor itch, as otherwise the sound improvements are really > good. The days > of installing that old realtek driver are finally gone :-) Hmm, this is what happens when you split a patch series. We could either revert this patch, or alternatively turn the error messages into warnings about using deprecated behavior. > Another thing I noted, that there is no way for pulseaudio audiodev to > specify the 'client name', > it always shows up in pavucontrl as the socket path to the server. > Thus if I added two PA audiodevs, I can't really distinguish between them. > The in|out.name= seems to specify the pulseaudio source/sink to connect to, > which is not the same. We currently supply the constant "qemu" as a name to pa_stream_new. While it's still not ideal, shouldn't this end up as a client name in pulseaudio instead of a socket path? Regards, Zoltan
Re: [Qemu-devel] [PATCH v2 35/68] target/arm: Convert CPS (privileged)
On 8/25/19 10:28 AM, Richard Henderson wrote: >> CPS shouldn't exist at all for M-profile, but the legacy decoder >> got this wrong too, so we should put that on the todo list for >> fixing later (along, maybe, with UNDEFing on some of the >> unpredictable combinations of M/imod/etc for A profile?). > Fixing m-profile is just as easy as as commenting. > I'll leave a TODO for the unpredictable combinations. There's also a missing check for ARMv6 here. That got added later, with the T16 decode. I have just added the following to the T16 commit message: target/arm: Convert T16, Change processor state Add a check for ARMv6 in trans_CPS. We had this correct in the T16 path, but had previously forgotten the check on the A32 and T32 paths. r~
Re: [Qemu-devel] [PATCH v2 35/68] target/arm: Convert CPS (privileged)
On 8/25/19 9:20 AM, Peter Maydell wrote: > On Mon, 19 Aug 2019 at 22:38, Richard Henderson > wrote: >> >> Signed-off-by: Richard Henderson >> --- >> target/arm/translate.c | 87 +++- >> target/arm/a32-uncond.decode | 3 ++ >> target/arm/t32.decode| 3 ++ >> 3 files changed, 42 insertions(+), 51 deletions(-) >> diff --git a/target/arm/t32.decode b/target/arm/t32.decode >> index 18c268e712..354ad77fe6 100644 >> --- a/target/arm/t32.decode >> +++ b/target/arm/t32.decode >> @@ -44,6 +44,7 @@ >> !extern rd rn lsb msb >> !extern rd rn satimm imm sh >> !extern rd rn rm imm tb >> + !extern mode imod M A I F >> >> # Data-processing (register) >> >> @@ -340,6 +341,8 @@ CLZ 1010 1011 1000 >> @rdm >> SMC 0111 imm:4 1000 >> HVC 0111 1110 1000 \ >>imm=%imm16_16_0 >> +CPS 0011 1010 1000 0 imod:2 M:1 A:1 I:1 F:1 mode:5 \ >> + > > In T32 the CPS insn overlaps with the hint space (hint insns have > bits [10:8] all-zeroes, whereas all the valid CPS insns have either > M set or one of the imod bits set) -- why doesn't it need to be > in the same insn group as the hints? If we're going to put it > separated in the .decode file from the insns it overlaps with > I guess a comment to that effect would help so it doesn't get > inadvertently reordered with them. It is grouped. It's not immediately visible in the patch because there are a *lot* of insns that overlap with the hints and 3 lines of context are insufficient to see that. But the grouping is semi-visible in the indentation here. > CPS shouldn't exist at all for M-profile, but the legacy decoder > got this wrong too, so we should put that on the todo list for > fixing later (along, maybe, with UNDEFing on some of the > unpredictable combinations of M/imod/etc for A profile?). Fixing m-profile is just as easy as as commenting. I'll leave a TODO for the unpredictable combinations. r~
Re: [Qemu-devel] [PATCH 05/13] qcrypto-luks: clear the masterkey and password before freeing them always
On Sun, 2019-08-25 at 18:31 +0300, Maxim Levitsky wrote: > On Thu, 2019-08-22 at 13:56 +0300, Maxim Levitsky wrote: > > On Thu, 2019-08-22 at 11:49 +0100, Daniel P. Berrangé wrote: > > > On Tue, Aug 20, 2019 at 08:12:51PM +0200, Max Reitz wrote: > > > > On 14.08.19 22:22, Maxim Levitsky wrote: > > > > > While there are other places where these are still stored in memory, > > > > > this is still one less key material area that can be sniffed with > > > > > various side channel attacks > > > > > > > > > > > > > > > > > > > > > > > (Many empty lines here) > > > > > > > > > Signed-off-by: Maxim Levitsky > > > > > --- > > > > > crypto/block-luks.c | 52 > > > > > ++--- > > > > > 1 file changed, 44 insertions(+), 8 deletions(-) > > > > > > > > Wouldn’t it make sense to introduce a dedicated function for this? > > > > > > Yes, it would. > > > > > > In fact I have a series pending which bumps min glib and introduces > > > use of auto-free functions in this code. > > > > > > It would be desirable to have a autp-free func for memset+free > > > so we can just declare the variable > > > > > >q_autowipefree char *password = NULL; > > > > > > and have it result in memset+free > > > > > > > That is perfect. > > When do you think you could post the series so that I could rebase > > on top of it? > > > I am thinking that I will keep my patch as is, just so that code is > consistent in memsetting the secrets (even though as Nir pointed out, > that these will be probably optimized away anyway). > And then when you send your patch you will just remove all > of these memsets. > > Is this all right? I see that your series actually already got merged. Can I now implement the 'q_autowipefree', or do I need another glib version bump for that? Best regards, Maxim Levitsky > > Best regards, > Maxim Levitsky
Re: [Qemu-devel] [PATCH 00/13] RFC: luks/encrypted qcow2 key management
On Thu, 2019-08-22 at 12:35 +0100, Daniel P. Berrangé wrote: > On Wed, Aug 14, 2019 at 11:22:06PM +0300, Maxim Levitsky wrote: > > Hi! > > > > This patch series implements key management for luks based encryption > > It supports both raw luks images and qcow2 encrypted images. > > > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1731898 > > > > There are still several issues that need to be figured out, > > on which the feedback is very welcome, but other than that the code mostly > > works. > > > > The main issues are: > > > > 1. Instead of the proposed > > blockdev-update-encryption/blockdev-erase-encryption > > interface, it is probably better to implement 'blockdev-amend-options' in > > qmp, > > and use this both for offline and online key update (with some translation > > layer to convert the qemu-img 'options' to qmp structures) > > > > This interface already exists for offline qcow2 format options update/ > > > > This is an issue that was raised today on IRC with Kevin Wolf. Really thanks > > for the idea! > > > > We agreed that this new qmp interface should take the same options as > > blockdev-create does, however since we want to be able to edit the > > encryption > > slots separately, this implies that we sort of need to allow this on > > creation > > time as well. > > > > Also the BlockdevCreateOptions is a union, which is specialized by the > > driver name > > which is great for creation, but for update, the driver name is already > > known, > > and thus the user should not be forced to pass it again. > > However qmp doesn't seem to support union type guessing based on actual > > fields > > given (this might not be desired either), which complicates this somewhat. > > Given this design question around the integration into blockdev, I'd > suggest splitting the series into two parts. > > One series should do all the work in crypto/ code to support adding > and erasing key slots. > > One series should focus on block/ layer QMP/qemu-img integration. > > The block layer QAPI stuff shouldn't leak into the crypto/ code. > > So this will let us get on with reviewing & unit testing the > crypto code, while we discuss block layer design options in more > detail. > > Regards, > Daniel I think we need 3 series here. 1. All the re-factoring/preparation work I done in luks crypto driver, which can be merged now, pending minor changes from the review. I think that it at least doesn't make the code worse. 2. Common code for the block layer to support key management this way or another, can be even added with not a single driver implementing it. 1,2 don't depend on each other mostly. 3. Key management in LUKS, which needs both 1,2, but thankfully is mostly implemented, and won't need to change much from the current implementation. So I'll send 1 now, and I will star working on 2. Last week we (I and Daniel) defined a draft of amend interface, and if time permits we will work on that tomorrow to finalize the interface. Best regards, Maxim Levitsky
Re: [Qemu-devel] [PATCH 12/13] qemu-img: implement key management
On Thu, 2019-08-22 at 16:42 +0200, Max Reitz wrote: > On 22.08.19 13:32, Daniel P. Berrangé wrote: > > On Tue, Aug 20, 2019 at 08:29:55PM +0200, Max Reitz wrote: > > > On 14.08.19 22:22, Maxim Levitsky wrote: > > > > Signed-off-by: Maxim Levitsky > > > > --- > > > > block/crypto.c | 16 ++ > > > > block/crypto.h | 3 + > > > > qemu-img-cmds.hx | 13 + > > > > qemu-img.c | 140 +++ > > > > 4 files changed, 172 insertions(+) > > > > > > Yes, this seems a bit weird. Putting it under amend seems like the > > > natural thing if that works; if not, I think it should be a single > > > qemu-img subcommand instead of two. > > > > I'm not convinced by overloading two distinct operations on to one > > sub-command - doesn't seem to give an obvious benefit to overload > > them & IME experiance overloading results in harder to understand > > commands due to having distinct args to each command. > > Because it suits the qemu-img interface we currently have. For example, > we have a single subcommand for internal snapshot management (“qemu-img > snapshot”), so I think it makes sense to have a single subcommand for > encrypted image management. I personally don't care, other that I do thing that the best here is to use the amend interface. > > Max > Best regards, Maxim Levitsky
Re: [Qemu-devel] [PATCH 09/13] qcrypto-luks: implement the encryption key management
On Thu, 2019-08-22 at 12:27 +0100, Daniel P. Berrangé wrote:
> On Wed, Aug 14, 2019 at 11:22:15PM +0300, Maxim Levitsky wrote:
> > Signed-off-by: Maxim Levitsky
> > ---
> > crypto/block-luks.c | 374 +++-
> > 1 file changed, 373 insertions(+), 1 deletion(-)
> >
> > diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> > index 1997e92fe1..2c33643b52 100644
> > --- a/crypto/block-luks.c
> > +++ b/crypto/block-luks.c
> > @@ -72,6 +72,8 @@ typedef struct QCryptoBlockLUKSKeySlot
> > QCryptoBlockLUKSKeySlot;
> >
> > #define QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME 2000
> >
> > +#define QCRYPTO_BLOCK_LUKS_ERASE_ITERATIONS 40
> > +
> > static const char qcrypto_block_luks_magic[QCRYPTO_BLOCK_LUKS_MAGIC_LEN] =
> > {
> > 'L', 'U', 'K', 'S', 0xBA, 0xBE
> > };
> > @@ -221,6 +223,9 @@ struct QCryptoBlockLUKS {
> >
> > /* Hash algorithm used in pbkdf2 function */
> > QCryptoHashAlgorithm hash_alg;
> > +
> > +/* Name of the secret that was used to open the image */
> > +char *secret;
> > };
> >
> >
> > @@ -1121,6 +1126,194 @@ qcrypto_block_luks_find_key(QCryptoBlock *block,
> > }
> >
> >
> > +
> > +/*
> > + * Returns true if a slot i is marked as containing as active
>
> s/as containing//
>
> > + * (contains encrypted copy of the master key)
> > + */
> > +
> > +static bool
> > +qcrypto_block_luks_slot_active(QCryptoBlockLUKS *luks, int slot_idx)
> > +{
> > +uint32_t val = luks->header.key_slots[slot_idx].active;
> > +return val == QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED;
> > +}
> > +
> > +/*
> > + * Returns the number of slots that are marked as active
> > + * (contains encrypted copy of the master key)
> > + */
> > +
> > +static int
> > +qcrypto_block_luks_count_active_slots(QCryptoBlockLUKS *luks)
> > +{
> > +int i, ret = 0;
>
> I prefer to see 'size_t' for loop iterators
Done
>
> > +
> > +for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
> > +if (qcrypto_block_luks_slot_active(luks, i)) {
> > +ret++;
> > +}
> > +}
> > +return ret;
> > +}
> > +
> > +
> > +/*
> > + * Finds first key slot which is not active
> > + * Returns the key slot index, or -1 if doesn't exist
> > + */
> > +
> > +static int
> > +qcrypto_block_luks_find_free_keyslot(QCryptoBlockLUKS *luks)
> > +{
> > +uint i;
> > +
> > +for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
> > +if (!qcrypto_block_luks_slot_active(luks, i)) {
> > +return i;
> > +}
> > +}
> > +return -1;
> > +
> > +}
> > +
> > +/*
> > + * Erases an keyslot given its index
> > + *
> > + * Returns:
> > + *0 if the keyslot was erased successfully
> > + * -1 if a error occurred while erasing the keyslot
> > + *
> > + */
> > +
>
> Redundant blank line
Done
>
> > +static int
> > +qcrypto_block_luks_erase_key(QCryptoBlock *block,
> > + uint slot_idx,
> > + QCryptoBlockWriteFunc writefunc,
> > + void *opaque,
> > + Error **errp)
> > +{
> > +QCryptoBlockLUKS *luks = block->opaque;
> > +QCryptoBlockLUKSKeySlot *slot = >header.key_slots[slot_idx];
> > +uint8_t *garbagekey = NULL;
> > +size_t splitkeylen = masterkeylen(luks) * slot->stripes;
> > +int i;
> > +int ret = -1;
> > +
> > +assert(slot_idx < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS);
> > +assert(splitkeylen > 0);
> > +
> > +garbagekey = g_malloc0(splitkeylen);
> > +
> > +/* Reset the key slot header */
> > +memset(slot->salt, 0, QCRYPTO_BLOCK_LUKS_SALT_LEN);
> > +slot->iterations = 0;
> > +slot->active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
> > +
> > +qcrypto_block_luks_store_header(block, writefunc, opaque, errp);
> > +
> > +/*
> > + * Now try to erase the key material, even if the header
> > + * update failed
> > + */
> > +
>
> Redundant blank line
Fixed.
>
> > +for (i = 0 ; i < QCRYPTO_BLOCK_LUKS_ERASE_ITERATIONS ; i++) {
> > +if (qcrypto_random_bytes(garbagekey, splitkeylen, errp) < 0) {
> > +
>
> Again, many more times beelow.
>
> > +/*
> > + * If we failed to get the random data, still write
> > + * *something* to the key slot at least once
> > + */
>
> Specificially we write all-zeros, since garbagekey was allocated
> with g_malloc0
Clarified.
>
> > +
> > +if (i > 0) {
> > +goto cleanup;
> > +}
> > +}
> > +
> > +if (writefunc(block, slot->key_offset *
> > QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
> > + garbagekey,
> > + splitkeylen,
> > + opaque,
> > + errp) != splitkeylen) {
>
> Indent is off - align with '('
Fixed.
>
> > +goto cleanup;
> > +}
> > +}
> > +
> > +ret = 0;
> > +cleanup:
> > +g_free(garbagekey);
> > +
Re: [Qemu-devel] [PATCH 07/13] block: add manage-encryption command (qmp and blockdev)
On Thu, 2019-08-22 at 16:07 +0200, Markus Armbruster wrote:
> Maxim Levitsky writes:
>
> > On Wed, 2019-08-21 at 13:47 +0200, Markus Armbruster wrote:
> > > Maxim Levitsky writes:
> > >
> > > > This adds:
> > > >
> > > > * x-blockdev-update-encryption and x-blockdev-erase-encryption qmp
> > > > commands
> > > > Both commands take the QCryptoKeyManageOptions
> > > > the x-blockdev-update-encryption is meant for non destructive addition
> > > > of key slots / whatever the encryption driver supports in the future
> > > >
> > > > x-blockdev-erase-encryption is meant for destructive encryption key
> > > > erase,
> > > > in some cases even without way to recover the data.
> > > >
> > > >
> > > > * bdrv_setup_encryption callback in the block driver
> > > > This callback does both the above functions with 'action' parameter
> > > >
> > > > * QCryptoKeyManageOptions with set of options that drivers can use for
> > > > encryption managment
> > > > Currently it has all the options that LUKS needs, and later it can be
> > > > extended
> > > > (via union) to support more encryption drivers if needed
> > > >
> > > > * blk_setup_encryption / bdrv_setup_encryption - the usual block layer
> > > > wrappers.
> > > > Note that bdrv_setup_encryption takes BlockDriverState and not
> > > > BdrvChild,
> > > > for the ease of use from the qmp code. It is not expected that this
> > > > function
> > > > will be used by anything but qmp and qemu-img code
> > > >
> > > >
> > > > Signed-off-by: Maxim Levitsky
> > >
> > > [...]
> > > > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > > > index 0d43d4f37c..53ed411eed 100644
> > > > --- a/qapi/block-core.json
> > > > +++ b/qapi/block-core.json
> > > > @@ -5327,3 +5327,39 @@
> > > >'data' : { 'node-name': 'str',
> > > > 'iothread': 'StrOrNull',
> > > > '*force': 'bool' } }
> > > > +
> > > > +
> > > > +##
> > > > +# @x-blockdev-update-encryption:
> > > > +#
> > > > +# Update the encryption keys for an encrypted block device
> > > > +#
> > > > +# @node-name:Name of the blockdev to operate on
> > > > +# @force: Disable safety checks (use with care)
> > >
> > > What checks excactly are disabled?
> >
> > Ability to overwrite an used slot with a different password.
> > If overwrite fails, the image won't be recoverable.
> >
> > The safe way is to add a new slot, then erase the old
> > one, but this changes the slot where the password
> > is stored, unless this procedure is used twice
>
> Would this be a useful addition to the doc comment?
>
> > > > +# @options: Driver specific options
> > > > +#
> > > > +
> > > > +# Since: 4.2
> > > > +##
> > > > +{ 'command': 'x-blockdev-update-encryption',
> > > > + 'data': { 'node-name' : 'str',
> > > > +'*force' : 'bool',
> > > > +'options': 'QCryptoEncryptionSetupOptions' } }
> > > > +
> > > > +##
> > > > +# @x-blockdev-erase-encryption:
> > > > +#
> > > > +# Erase the encryption keys for an encrypted block device
> > > > +#
> > > > +# @node-name:Name of the blockdev to operate on
> > > > +# @force: Disable safety checks (use with care)
> > >
> > > Likewise.
> >
> > 1. Erase a slot which is already marked as
> > erased. Mostly harmless but pointless as well.
> >
> > 2. Erase last keyslot. This irreversibly destroys
> > any ability to read the data from the device,
> > unless a backup of the header and the key material is
> > done prior. Still can be useful when it is desired to
> > erase the data fast.
>
> Would this be a useful addition to the doc comment?
Yea, but since I'll will switch to the amend interface,
I'll leave it like that for now.
[...]
Best regards,
Maxim Levitsky
Re: [Qemu-devel] [PATCH v2 43/68] target/arm: Simplify disas_arm_insn
On Mon, 19 Aug 2019 at 22:38, Richard Henderson wrote: > > Fold away all of the cases that now just goto illegal_op, > because all of their internal bits are now in decodetree. > > Signed-off-by: Richard Henderson > --- > target/arm/translate.c | 69 ++ > 1 file changed, 16 insertions(+), 53 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH v2 42/68] target/arm: Simplify disas_thumb2_insn
On Mon, 19 Aug 2019 at 22:38, Richard Henderson wrote: > > Fold away all of the cases that now just goto illegal_op, > because all of their internal bits are now in decodetree. > > Signed-off-by: Richard Henderson > --- Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH v2 41/68] target/arm: Convert TT
On Mon, 19 Aug 2019 at 22:38, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/translate.c | 87 +- > target/arm/t32.decode | 5 ++- > 2 files changed, 31 insertions(+), 61 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH v2 40/68] target/arm: Convert SG
On Mon, 19 Aug 2019 at 22:38, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/translate.c | 51 -- > target/arm/t32.decode | 5 - > 2 files changed, 33 insertions(+), 23 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH v2 39/68] target/arm: Convert Table Branch
On Mon, 19 Aug 2019 at 22:38, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/translate.c | 57 +- > target/arm/t32.decode | 8 +- > 2 files changed, 41 insertions(+), 24 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH v2 35/68] target/arm: Convert CPS (privileged)
On Mon, 19 Aug 2019 at 22:38, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/translate.c | 87 +++- > target/arm/a32-uncond.decode | 3 ++ > target/arm/t32.decode| 3 ++ > 3 files changed, 42 insertions(+), 51 deletions(-) > diff --git a/target/arm/t32.decode b/target/arm/t32.decode > index 18c268e712..354ad77fe6 100644 > --- a/target/arm/t32.decode > +++ b/target/arm/t32.decode > @@ -44,6 +44,7 @@ > !extern rd rn lsb msb > !extern rd rn satimm imm sh > !extern rd rn rm imm tb > + !extern mode imod M A I F > > # Data-processing (register) > > @@ -340,6 +341,8 @@ CLZ 1010 1011 1000 > @rdm > SMC 0111 imm:4 1000 > HVC 0111 1110 1000 \ >imm=%imm16_16_0 > +CPS 0011 1010 1000 0 imod:2 M:1 A:1 I:1 F:1 mode:5 \ > + In T32 the CPS insn overlaps with the hint space (hint insns have bits [10:8] all-zeroes, whereas all the valid CPS insns have either M set or one of the imod bits set) -- why doesn't it need to be in the same insn group as the hints? If we're going to put it separated in the .decode file from the insns it overlaps with I guess a comment to that effect would help so it doesn't get inadvertently reordered with them. CPS shouldn't exist at all for M-profile, but the legacy decoder got this wrong too, so we should put that on the todo list for fixing later (along, maybe, with UNDEFing on some of the unpredictable combinations of M/imod/etc for A profile?). Otherwise Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH 06/13] qcrypto-luks: implement more rigorous header checking
On Sun, 2019-08-25 at 18:40 +0300, Maxim Levitsky wrote:
> On Thu, 2019-08-22 at 12:04 +0100, Daniel P. Berrangé wrote:
> > On Wed, Aug 14, 2019 at 11:22:12PM +0300, Maxim Levitsky wrote:
> > > Check that keyslots don't overlap with the data,
> > > and check that keyslots don't overlap with each other.
> > > (this is done using naive O(n^2) nested loops,
> > > but since there are just 8 keyslots, this doens't really matter.
> > >
> > > Signed-off-by: Maxim Levitsky
> > > ---
> > > crypto/block-luks.c | 42 ++
> > > 1 file changed, 42 insertions(+)
> > >
> > > diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> > > index 336e633df4..1997e92fe1 100644
> > > --- a/crypto/block-luks.c
> > > +++ b/crypto/block-luks.c
> > > @@ -551,6 +551,8 @@ static int
> > > qcrypto_block_luks_check_header(QCryptoBlockLUKS *luks, Error **errp)
> > > {
> > > int ret;
> > > +int i, j;
> > > +
> > >
> > > if (memcmp(luks->header.magic, qcrypto_block_luks_magic,
> > > QCRYPTO_BLOCK_LUKS_MAGIC_LEN) != 0) {
> > > @@ -566,6 +568,46 @@ qcrypto_block_luks_check_header(QCryptoBlockLUKS
> > > *luks, Error **errp)
> > > goto fail;
> > > }
> > >
> > > +/* Check all keyslots for corruption */
> > > +for (i = 0 ; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS ; i++) {
> > > +
> > > +QCryptoBlockLUKSKeySlot *slot1 = >header.key_slots[i];
> > > +uint start1 = slot1->key_offset;
> > > +uint len1 = splitkeylen_sectors(luks, slot1->stripes);
> >
> > Using 'uint' is not normal QEMU style.
> >
> > Either use 'unsigned int' or if a specific size is needed
> > then one of the 'guintNN' types from glib.
> >
> > This applies elsewhere in this patch series too, but
> > I'll only comment here & let you find the other cases.
>
> Fixed. Sorry for the noise.
>
> >
> > > +
> > > +if (slot1->stripes == 0 ||
> > > +(slot1->active != QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED &&
> > > +slot1->active != QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED)) {
> > > +
> >
> > Redundant blank line
>
> Fixed
> >
> > > +error_setg(errp, "Keyslot %i is corrupted", i);
> >
> > I'd do a separate check for stripes and active fields, and then give a
> > specific error message for each. That way if this does ever trigger
> > in practice will immediately understand which check failed.
> >
> > Also using '%d' rather than '%i' is more common convention
>
> Done.
Note that I switched i,j to be size_t since you said that you prefer this,
and to print this I apparently need %lu.
[...]
Best regards,
Maxim Levitsky
Re: [Qemu-devel] [PATCH v2 34/68] target/arm: Convert Clear-Exclusive, Barriers
On Mon, 19 Aug 2019 at 22:38, Richard Henderson
wrote:
>
> Signed-off-by: Richard Henderson
> ---
> target/arm/translate.c | 122 +++
> target/arm/a32-uncond.decode | 10 +++
> target/arm/t32.decode| 10 +++
> 3 files changed, 73 insertions(+), 69 deletions(-)
>
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index e268c5168d..6489bbc09c 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -10038,6 +10038,58 @@ static bool trans_SRS(DisasContext *s, arg_SRS *a)
> return true;
> }
>
> +/*
> + * Clear-Exclusive, Barriers
> + */
> +
> +static bool trans_CLREX(DisasContext *s, arg_CLREX *a)
> +{
> +if (!ENABLE_ARCH_6K) {
> +return false;
> +}
> +gen_clrex(s);
> +return true;
> +}
> +
> +static bool trans_DSB(DisasContext *s, arg_DSB *a)
> +{
> +if (!s->thumb && !ENABLE_ARCH_7) {
> +return false;
> +}
> +tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC);
> +return true;
> +}
> +
> +static bool trans_DMB(DisasContext *s, arg_DMB *a)
> +{
> +return trans_DSB(s, NULL);
> +}
> +
> +static bool trans_ISB(DisasContext *s, arg_ISB *a)
> +{
> +/*
> + * We need to break the TB after this insn to execute
> + * self-modifying code correctly and also to take
> + * any pending interrupts immediately.
> + */
> +gen_goto_tb(s, 0, s->base.pc_next);
> +return true;
> +}
The guard conditions on these don't look right for the
Thumb case -- the old Thumb decoder has them exist only if
we have feature V7 or feature M. Are they really equivalent?
> diff --git a/target/arm/t32.decode b/target/arm/t32.decode
> index c8a8aeceee..18c268e712 100644
> --- a/target/arm/t32.decode
> +++ b/target/arm/t32.decode
> @@ -305,6 +305,16 @@ CLZ 1010 1011 1000
> @rdm
># of the space is "reserved hint, behaves as nop".
>NOP 0011 1010 1000
> }
> +
> +# Miscelaneous control
"Miscellaneous"
> +{
> + CLREX 0011 1011 1000 0010
> + DSB 0011 1011 1000 0100
> + DMB 0011 1011 1000 0101
> + ISB 0011 1011 1000 0110
> + SB 0011 1011 1000 0111
> +}
Otherwise
Reviewed-by: Peter Maydell
thanks
-- PMM
Re: [Qemu-devel] [PATCH 06/13] qcrypto-luks: implement more rigorous header checking
On Thu, 2019-08-22 at 12:04 +0100, Daniel P. Berrangé wrote:
> On Wed, Aug 14, 2019 at 11:22:12PM +0300, Maxim Levitsky wrote:
> > Check that keyslots don't overlap with the data,
> > and check that keyslots don't overlap with each other.
> > (this is done using naive O(n^2) nested loops,
> > but since there are just 8 keyslots, this doens't really matter.
> >
> > Signed-off-by: Maxim Levitsky
> > ---
> > crypto/block-luks.c | 42 ++
> > 1 file changed, 42 insertions(+)
> >
> > diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> > index 336e633df4..1997e92fe1 100644
> > --- a/crypto/block-luks.c
> > +++ b/crypto/block-luks.c
> > @@ -551,6 +551,8 @@ static int
> > qcrypto_block_luks_check_header(QCryptoBlockLUKS *luks, Error **errp)
> > {
> > int ret;
> > +int i, j;
> > +
> >
> > if (memcmp(luks->header.magic, qcrypto_block_luks_magic,
> > QCRYPTO_BLOCK_LUKS_MAGIC_LEN) != 0) {
> > @@ -566,6 +568,46 @@ qcrypto_block_luks_check_header(QCryptoBlockLUKS
> > *luks, Error **errp)
> > goto fail;
> > }
> >
> > +/* Check all keyslots for corruption */
> > +for (i = 0 ; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS ; i++) {
> > +
> > +QCryptoBlockLUKSKeySlot *slot1 = >header.key_slots[i];
> > +uint start1 = slot1->key_offset;
> > +uint len1 = splitkeylen_sectors(luks, slot1->stripes);
>
> Using 'uint' is not normal QEMU style.
>
> Either use 'unsigned int' or if a specific size is needed
> then one of the 'guintNN' types from glib.
>
> This applies elsewhere in this patch series too, but
> I'll only comment here & let you find the other cases.
Fixed. Sorry for the noise.
>
> > +
> > +if (slot1->stripes == 0 ||
> > +(slot1->active != QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED &&
> > +slot1->active != QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED)) {
> > +
>
> Redundant blank line
Fixed
>
> > +error_setg(errp, "Keyslot %i is corrupted", i);
>
> I'd do a separate check for stripes and active fields, and then give a
> specific error message for each. That way if this does ever trigger
> in practice will immediately understand which check failed.
>
> Also using '%d' rather than '%i' is more common convention
Done.
>
>
> > +ret = -EINVAL;
> > +goto fail;
> > +}
> > +
> > +if (start1 + len1 > luks->header.payload_offset) {
> > +error_setg(errp,
> > + "Keyslot %i is overlapping with the encrypted
> > payload",
> > + i);
> > +ret = -EINVAL;
> > +goto fail;
> > +}
> > +
> > +for (j = i + 1 ; j < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS ; j++) {
> > +
>
> Redundant blank
>
> > +QCryptoBlockLUKSKeySlot *slot2 = >header.key_slots[j];
> > +uint start2 = slot2->key_offset;
> > +uint len2 = splitkeylen_sectors(luks, slot2->stripes);
> > +
> > +if (start1 + len1 > start2 && start2 + len2 > start1) {
> > +error_setg(errp,
> > + "Keyslots %i and %i are overlapping in the
> > header",
>
> %d
Fixed.
>
> > + i, j);
> > +ret = -EINVAL;
> > +goto fail;
> > +}
> > +}
> > +
> > +}
> > return 0;
> > fail:
> > return ret;
> > --
> > 2.17.2
> >
>
> Regards,
> Daniel
Best regards,
Maxim Levitsky
Re: [Qemu-devel] [PATCH 05/13] qcrypto-luks: clear the masterkey and password before freeing them always
On Thu, 2019-08-22 at 13:56 +0300, Maxim Levitsky wrote: > On Thu, 2019-08-22 at 11:49 +0100, Daniel P. Berrangé wrote: > > On Tue, Aug 20, 2019 at 08:12:51PM +0200, Max Reitz wrote: > > > On 14.08.19 22:22, Maxim Levitsky wrote: > > > > While there are other places where these are still stored in memory, > > > > this is still one less key material area that can be sniffed with > > > > various side channel attacks > > > > > > > > > > > > > > > > > > (Many empty lines here) > > > > > > > Signed-off-by: Maxim Levitsky > > > > --- > > > > crypto/block-luks.c | 52 ++--- > > > > 1 file changed, 44 insertions(+), 8 deletions(-) > > > > > > Wouldn’t it make sense to introduce a dedicated function for this? > > > > Yes, it would. > > > > In fact I have a series pending which bumps min glib and introduces > > use of auto-free functions in this code. > > > > It would be desirable to have a autp-free func for memset+free > > so we can just declare the variable > > > >q_autowipefree char *password = NULL; > > > > and have it result in memset+free > > > > That is perfect. > When do you think you could post the series so that I could rebase > on top of it? I am thinking that I will keep my patch as is, just so that code is consistent in memsetting the secrets (even though as Nir pointed out, that these will be probably optimized away anyway). And then when you send your patch you will just remove all of these memsets. Is this all right? Best regards, Maxim Levitsky
Re: [Qemu-devel] [PATCH v2 33/68] target/arm: Convert RFE and SRS
On Mon, 19 Aug 2019 at 22:38, Richard Henderson
wrote:
>
> Signed-off-by: Richard Henderson
> ---
> target/arm/translate.c | 150 ++-
> target/arm/a32-uncond.decode | 8 ++
> target/arm/t32.decode| 12 +++
> 3 files changed, 81 insertions(+), 89 deletions(-)
>
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index b6d8b7be8c..e268c5168d 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -9980,16 +9980,71 @@ static bool trans_SVC(DisasContext *s, arg_SVC *a)
> return true;
> }
>
> +/*
> + * Unconditional system instructions
> + */
> +
> +static bool trans_RFE(DisasContext *s, arg_RFE *a)
> +{
> +int32_t offset;
> +TCGv_i32 addr, t1, t2;
> +
> +if (IS_USER(s) || !ENABLE_ARCH_6) {
> +return false;
> +}
The legacy thumb decoder for RFE and SRS also has
"not if M profile", which we seem to be missing here ?
> diff --git a/target/arm/a32-uncond.decode b/target/arm/a32-uncond.decode
> index 573ac2cf8e..3b961233e5 100644
> --- a/target/arm/a32-uncond.decode
> +++ b/target/arm/a32-uncond.decode
> @@ -29,3 +29,11 @@
> %imm24h 0:s24 24:1 !function=times_2
>
> BLX_i 101 . imm=%imm24h
> +
> +# System Instructions
> +
> + rn w pu
> + mode w pu
> +
> +RFE 100 pu:2 0 w:1 1 rn:4 1010
> +SRS 110 pu:2 1 w:1 0 1101 0101 000 mode:5
Is this SRS encoding correct? The copy of the Arm ARM I'm looking at
thinks that it starts 100, the same as RFE.
Otherwise
Reviewed-by: Peter Maydell
thanks
-- PMM
Re: [Qemu-devel] [Qemu-block] [PATCH v5 1/6] iotests: allow Valgrind checking all QEMU processes
On 16/08/2019 01:49, John Snow wrote:
>
>
> On 7/19/19 12:30 PM, Andrey Shinkevich wrote:
>> With the '-valgrind' option, let all the QEMU processes be run under
>> the Valgrind tool. The Valgrind own parameters may be set with its
>> environment variable VALGRIND_OPTS, e.g.
>> VALGRIND_OPTS="--leak-check=yes" ./check -qcow2 -valgrind
>> or they may be listed in the Valgrind checked file ./.valgrindrc or
>> ~/.valgrindrc like
>> --memcheck:leak-check=no
>> --memcheck:track-origins=yes
>> When QEMU-IO process is being killed, the shell report refers to the
>> text of the command in _qemu_io_wrapper(), which was modified with this
>> patch. So, the benchmark output for the tests 039, 061 and 137 is to be
>> changed also.
>>
>
> Oh, weird. "VALGRIND_QEMU=y" actually has just meant ... valgrind
> qemu-io. OK.
>
>> Signed-off-by: Andrey Shinkevich
>> ---
>> tests/qemu-iotests/039.out | 30 ---
>> tests/qemu-iotests/061.out | 12 ++--
>> tests/qemu-iotests/137.out | 6 +---
>> tests/qemu-iotests/common.rc | 69
>>
>> 4 files changed, 59 insertions(+), 58 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out
>> index 724d7b2..972c6c0 100644
>> --- a/tests/qemu-iotests/039.out
>> +++ b/tests/qemu-iotests/039.out
>> @@ -11,11 +11,7 @@ No errors were found on the image.
>> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
>> wrote 512/512 bytes at offset 0
>> 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ];
>> then
>> -exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99
>> "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -else
>> -exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -fi )
>> +./common.rc: Killed ( _qemu_proc_wrapper
>> "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>> incompatible_features 0x1
>> ERROR cluster 5 refcount=0 reference=1
>> ERROR OFLAG_COPIED data cluster: l2_entry=8005 refcount=0
>> @@ -50,11 +46,7 @@ read 512/512 bytes at offset 0
>> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
>> wrote 512/512 bytes at offset 0
>> 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ];
>> then
>> -exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99
>> "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -else
>> -exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -fi )
>> +./common.rc: Killed ( _qemu_proc_wrapper
>> "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>> incompatible_features 0x1
>> ERROR cluster 5 refcount=0 reference=1
>> Rebuilding refcount structure
>> @@ -68,11 +60,7 @@ incompatible_features 0x0
>> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
>> wrote 512/512 bytes at offset 0
>> 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ];
>> then
>> -exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99
>> "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -else
>> -exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -fi )
>> +./common.rc: Killed ( _qemu_proc_wrapper
>> "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>> incompatible_features 0x0
>> No errors were found on the image.
>>
>> @@ -91,11 +79,7 @@ No errors were found on the image.
>> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
>> wrote 512/512 bytes at offset 0
>> 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ];
>> then
>> -exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99
>> "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -else
>> -exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -fi )
>> +./common.rc: Killed ( _qemu_proc_wrapper
>> "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>> incompatible_features 0x1
>> ERROR cluster 5 refcount=0 reference=1
>> ERROR OFLAG_COPIED data cluster: l2_entry=8005 refcount=0
>> @@ -105,11 +89,7 @@ Data may be corrupted, or further writes to the image
>> may corrupt it.
>> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
>> wrote 512/512 bytes at offset 0
>> 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> -./common.rc: Killed ( if [ "${VALGRIND_QEMU}" == "y" ];
>> then
>> -exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99
>> "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -else
>> -exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>> -fi )
>> +./common.rc: Killed ( _qemu_proc_wrapper
>> "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>> incompatible_features 0x0
>> No errors
Re: [Qemu-devel] [Qemu-block] [PATCH v5 4/6] iotests: Valgrind fails with nonexistent directory
On 16/08/2019 03:55, John Snow wrote:
>
>
> On 7/19/19 12:30 PM, Andrey Shinkevich wrote:
>> The Valgrind uses the exported variable TMPDIR and fails if the
>> directory does not exist. Let us exclude such a test case from
>> being run under the Valgrind and notify the user of it.
>>
>> Suggested-by: Kevin Wolf
>> Signed-off-by: Andrey Shinkevich
>> ---
>> tests/qemu-iotests/051 | 4
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
>> index ce942a5..f8141ca 100755
>> --- a/tests/qemu-iotests/051
>> +++ b/tests/qemu-iotests/051
>> @@ -377,6 +377,10 @@ printf %b "qemu-io $device_id \"write -P 0x33 0
>> 4k\"\ncommit $device_id\n" |
>> $QEMU_IO -c "read -P 0x33 0 4k" "$TEST_IMG" | _filter_qemu_io
>>
>> # Using snapshot=on with a non-existent TMPDIR
>> +if [ "${VALGRIND_QEMU}" == "y" ]; then
>> +_casenotrun "Valgrind needs a valid TMPDIR for itself"
>> +fi
>> +VALGRIND_QEMU="" \
>> TMPDIR=/nonexistent run_qemu -drive driver=null-co,snapshot=on
>>
>> # Using snapshot=on together with read-only=on
>>
>
> The only other way around this would be a complicated mechanism to set
> the TMPDIR for valgrind's sub-processes only, with e.g.
>
> valgrind ... env TMPDIR=/nonexistent qemu ...
>
> ... It's probably not worth trying to concoct such a thing; but I
> suppose it is possible. You'd have to set up a generic layer for setting
> environment variables, then in the qemu shim, you could either set them
> directly (non-valgrind invocation) or set them as part of the valgrind
> command-line.
>
> Or you could just take my R-B:
>
> Reviewed-by: John Snow
>
Thanks again John for your review and the advice.
Probably, it doesn't worth efforts to manage that case because QEMU
should fail anyway with the error message "Could not get temporary
filename: No such file or directory". So, we would not benefit much from
that run. We have other test cases that cover the main functionality.
It's just to check the QEMU error path for possible memory issues. Shall we?
Andrey
--
With the best regards,
Andrey Shinkevich
Re: [Qemu-devel] [PATCH] configure: more resilient Python version capture
On Sat, 24 Aug 2019 at 08:32, wrote: > > -echo "PYTHON_VERSION=$python_version" >> $config_host_mak > > +echo "PYTHON2=$python2" >> $config_host_mak > ... > > -ifneq ($(findstring v2,"v$(PYTHON_VERSION)"),v2) > > +ifneq ($(PYTHON2),y) > > Succinctly, if Python 3. > > We can further ween the world off Python 2 by replacing python2="y" for > python3="y" and PYTHON2 for PYTHON3. I don't think it's a big deal which way round we do this, because once we drop Python 2 support the if and the variable will just be deleted entirely. thanks -- PMM
Re: [Qemu-devel] [PATCH] i386/vmmouse: Properly reset state
On 21.07.19 10:58, Jan Kiszka wrote: From: Jan Kiszka nb_queue was not zeroed so that we no longer delivered events if a previous guest left the device in an overflow state. The state of absolute does not matter as the next vmmouse_update_handler call will align it again. Signed-off-by: Jan Kiszka --- hw/i386/vmmouse.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/i386/vmmouse.c b/hw/i386/vmmouse.c index 5d2d278be4..e335bd07da 100644 --- a/hw/i386/vmmouse.c +++ b/hw/i386/vmmouse.c @@ -257,6 +257,7 @@ static void vmmouse_reset(DeviceState *d) VMMouseState *s = VMMOUSE(d); s->queue_size = VMMOUSE_QUEUE_SIZE; +s->nb_queue = 0; vmmouse_disable(s); } -- 2.16.4 Ping - or who is looking after this? Jan
Re: [Qemu-devel] [PATCH 04/13] qcrypto-luks: refactoring: simplify the math used for keyslot locations
On Thu, 2019-08-22 at 11:47 +0100, Daniel P. Berrangé wrote:
> On Wed, Aug 14, 2019 at 11:22:10PM +0300, Maxim Levitsky wrote:
> > Signed-off-by: Maxim Levitsky
> > ---
> > crypto/block-luks.c | 64 +++--
> > 1 file changed, 38 insertions(+), 26 deletions(-)
> >
> > diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> > index 6bb369f3b4..e1a4df94b7 100644
> > --- a/crypto/block-luks.c
> > +++ b/crypto/block-luks.c
> > @@ -417,6 +417,33 @@ static int masterkeylen(QCryptoBlockLUKS *luks)
> > }
> >
> >
> > +/*
> > + * Returns number of sectors needed to store the key material
> > + * given number of anti forensic stripes
> > + */
> > +static int splitkeylen_sectors(QCryptoBlockLUKS *luks, int stripes)
>
> Needs a qcrypto_block_luks_ prefix on method name.
Done.
>
> I'd also put 'static int' on a separate line from method name
> to reduce too long lines.
Done.
>
> > +
> > +{
> > +/*
> > + * This calculation doesn't match that shown in the spec,
> > + * but instead follows the cryptsetup implementation.
> > + */
> > +
> > +size_t header_sectors = QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
> > + QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
>
> Following line indent should only be 4 spaces
I didn't knew that. Fixed.
>
> > +
> > +size_t splitkeylen = masterkeylen(luks) * stripes;
> > +
> > +/* First align the key material size to block size*/
> > +size_t splitkeylen_sectors =
> > +DIV_ROUND_UP(splitkeylen, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE);
>
> Again 4 space indent.
>
> > +
> > +/* Then also align the key material size to the size of the header */
> > +return ROUND_UP(splitkeylen_sectors, header_sectors);
> > +}
> > +
> > +
> > +
> > /*
> > * Stores the main LUKS header, taking care of endianess
> > */
> > @@ -1169,7 +1196,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
> > QCryptoBlockCreateOptionsLUKS luks_opts;
> > Error *local_err = NULL;
> > uint8_t *masterkey = NULL;
> > -size_t splitkeylen = 0;
> > +size_t next_sector;
> > size_t i;
> > char *password;
> > const char *cipher_alg;
> > @@ -1388,23 +1415,16 @@ qcrypto_block_luks_create(QCryptoBlock *block,
> > goto error;
> > }
> >
> > +/* start with the sector that follows the header*/
> > +next_sector = QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
> > + QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
>
> I'd suggest 'post_header_sector'
I called it now header_sectors, and each split key, split_key_sectors.
I hope that this is good enough.
>
> >
> > -/* Although LUKS has multiple key slots, we're just going
> > - * to use the first key slot */
> > -splitkeylen = luks->header.key_bytes * QCRYPTO_BLOCK_LUKS_STRIPES;
> > for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
> > -luks->header.key_slots[i].active =
> > QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
> > -luks->header.key_slots[i].stripes = QCRYPTO_BLOCK_LUKS_STRIPES;
> > -
> > -/* This calculation doesn't match that shown in the spec,
> > - * but instead follows the cryptsetup implementation.
> > - */
> > -luks->header.key_slots[i].key_offset =
> > -(QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
> > - QCRYPTO_BLOCK_LUKS_SECTOR_SIZE) +
> > -(ROUND_UP(DIV_ROUND_UP(splitkeylen,
> > QCRYPTO_BLOCK_LUKS_SECTOR_SIZE),
> > - (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
> > - QCRYPTO_BLOCK_LUKS_SECTOR_SIZE)) * i);
> > +QCryptoBlockLUKSKeySlot *slot = >header.key_slots[i];
> > +slot->active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
> > +slot->key_offset = next_sector;
> > +slot->stripes = QCRYPTO_BLOCK_LUKS_STRIPES;
> > +next_sector += splitkeylen_sectors(luks,
> > QCRYPTO_BLOCK_LUKS_STRIPES);
>
> I'm not a fan of the next_sector accumulator .
I actually think that accumulator is cleaner here,
but I won't argue about this. Fixed.
>
> I'd prefer to see the '* i' part done in splitkeylen_sectors, so that
> we have
>
> slot->key_offset = post_header_sector +
> splitkeylen_sectors(luks, QCRYPTO_BLOCK_LUKS_STRIPES, i);
>
> > @@ -1412,17 +1432,9 @@ qcrypto_block_luks_create(QCryptoBlock *block,
> > * slot headers, rounded up to the nearest sector, combined with
> > * the size of each master key material region, also rounded up
> > * to the nearest sector */
> > -luks->header.payload_offset =
> > -(QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
> > - QCRYPTO_BLOCK_LUKS_SECTOR_SIZE) +
> > -(ROUND_UP(DIV_ROUND_UP(splitkeylen,
> > QCRYPTO_BLOCK_LUKS_SECTOR_SIZE),
> > - (QCRYPTO_BLOCK_LUKS_KEY_SLOT_OFFSET /
> > - QCRYPTO_BLOCK_LUKS_SECTOR_SIZE)) *
> > - QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS);
> > -
> > +luks->header.payload_offset = next_sector;
>
> luks->header.payload_offset =
Re: [Qemu-devel] [PATCH 03/13] qcrypto-luks: refactoring: extract load/store/check/parse header functions
On Thu, 2019-08-22 at 11:34 +0100, Daniel P. Berrangé wrote: > On Thu, Aug 22, 2019 at 01:43:05AM +0300, Maxim Levitsky wrote: > > On Tue, 2019-08-20 at 20:01 +0200, Max Reitz wrote: > > > On 14.08.19 22:22, Maxim Levitsky wrote: > > > > With upcoming key management, the header will > > > > need to be stored after the image is created. > > > > > > > > Extracting load header isn't strictly needed, but > > > > do this anyway for the symmetry. > > > > > > > > Also I extracted a function that does basic sanity > > > > checks on the just read header, and a function > > > > which parses all the crypto format to make the > > > > code a bit more readable, plus now the code > > > > doesn't destruct the in-header cipher-mode string, > > > > so that the header now can be stored many times, > > > > which is needed for the key management. > > > > > > > > Also this allows to contain the endianess conversions > > > > in these functions alone > > > > > > > > The header is no longer endian swapped in place, > > > > to prevent (mostly theoretical races I think) > > > > races where someone could see the header in the > > > > process of beeing byteswapped. > > > > > > The formatting looks weird, it doesn’t look quite 72 characters wide... > > > (what commit messages normally use) > > > > Could you elaborate on that? I thought that code should not > > exceed 80 character limit. > > > > > > > > > Signed-off-by: Maxim Levitsky > > > > --- > > > > crypto/block-luks.c | 756 ++-- > > > > 1 file changed, 440 insertions(+), 316 deletions(-) > > > > > > Also, this commit is just too big. > > > > Yea, but it has no functional changes. > > I can split it further, but that won't help much IMHO. > > I'd find it easier to review if each newly introduced method was a > separate patch. It makes it easier to see which bit of removed > code was added to which method. > > Regards, > Daniel Done, patch is now split in several patches. Best regards, Maxim Levitsky
Re: [Qemu-devel] [PATCH 03/13] qcrypto-luks: refactoring: extract load/store/check/parse header functions
On Thu, 2019-08-22 at 11:38 +0100, Daniel P. Berrangé wrote:
> On Wed, Aug 14, 2019 at 11:22:09PM +0300, Maxim Levitsky wrote:
> > With upcoming key management, the header will
> > need to be stored after the image is created.
> >
> > Extracting load header isn't strictly needed, but
> > do this anyway for the symmetry.
> >
> > Also I extracted a function that does basic sanity
> > checks on the just read header, and a function
> > which parses all the crypto format to make the
> > code a bit more readable, plus now the code
> > doesn't destruct the in-header cipher-mode string,
> > so that the header now can be stored many times,
> > which is needed for the key management.
> >
> > Also this allows to contain the endianess conversions
> > in these functions alone
> >
> > The header is no longer endian swapped in place,
> > to prevent (mostly theoretical races I think)
> > races where someone could see the header in the
> > process of beeing byteswapped.
> >
> > Signed-off-by: Maxim Levitsky
> > ---
> > crypto/block-luks.c | 756 ++--
> > 1 file changed, 440 insertions(+), 316 deletions(-)
> > if (!(flags & QCRYPTO_BLOCK_OPEN_NO_IO)) {
> > /* Try to find which key slot our password is valid for
> > * and unlock the master key from that slot.
> > */
> > -
> > masterkey = g_new0(uint8_t, masterkeylen(luks));
> >
> > if (qcrypto_block_luks_find_key(block,
> > @@ -845,12 +1132,10 @@ qcrypto_block_luks_open(QCryptoBlock *block,
> > }
> >
> > block->sector_size = QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
> > -block->payload_offset = luks->header.payload_offset *
> > -block->sector_size;
> > +block->payload_offset = luks->header.payload_offset *
> > block->sector_size;
> >
> > g_free(masterkey);
> > g_free(password);
> > -
> > return 0;
>
> Smoe unrelated whitespace changes here.
>
>
> > +/* populate the slot 0 with the password encrypted master key*/
> > +/* This will also store the header */
> > +if (qcrypto_block_luks_store_key(block,
> > + 0,
> > + password,
> > + masterkey,
> > + luks_opts.iter_time,
> > + writefunc,
> > + opaque,
> > + errp)) {
> > goto error;
> > -}
> > + }
>
> Indent is off by 1
>
>
> Regards,
> Daniel
Fixed,
Best regards,
Maxim Levitsky
Re: [Qemu-devel] [QEMU] [PATCH v5 5/8] bootdevice: Gather LCHS from all relevant devices
> Only scsi-hd has the lchs properties, though, so what’s the purpose of > defining the unrealize function for all other classes? > > Max - shmuel.eider...@oracle.com + sam...@google.com The only purpose is to already have them mapped to the correct existing function, in case it will be used later on. I can resubmit without the unrealize for the other classes, WDYT? Sam
Re: [Qemu-devel] [QEMU] [PATCH v5 4/8] scsi: Propagate unrealize() callback to scsi-hd
> @@ -213,11 +221,18 @@ static void scsi_qdev_realize(DeviceState *qdev, Error
> **errp)
> static void scsi_qdev_unrealize(DeviceState *qdev, Error **errp)
> {
> SCSIDevice *dev = SCSI_DEVICE(qdev);
> +Error *local_err = NULL;
>
> if (dev->vmsentry) {
> qemu_del_vm_change_state_handler(dev->vmsentry);
> }
>
> +scsi_device_unrealize(dev, _err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
> +
> scsi_device_purge_requests(dev, SENSE_CODE(NO_SENSE));
(I see this code for the first time, but) I suppose I’d put the
scsi_device_unrealize() after scsi_device_purge_requests().
Max
> blockdev_mark_auto_del(dev->conf.blk);
> }
- shmuel.eider...@oracle.com
+ sam...@google.com
Sure, I'll resubmit
Sam
Re: [Qemu-devel] [Qemu-block] [PATCH v5 3/6] iotests: Add casenotrun report to bash tests
On 16/08/2019 23:33, Cleber Rosa wrote:
> On Thu, Aug 15, 2019 at 08:44:11PM -0400, John Snow wrote:
>>
>>
>> On 7/19/19 12:30 PM, Andrey Shinkevich wrote:
>>> The new function _casenotrun() is to be invoked if a test case cannot
>>> be run for some reason. The user will be notified by a message passed
>>> to the function.
>>>
>>
>> Oh, I assume this is a sub-test granularity; if we need to skip
>> individual items.
>>
>> I'm good with this, but we should CC Cleber Rosa, who has struggled
>> against this in the past, too.
>>
>
> The discussion I was involved in was not that much about skipping
> tests per se, but about how to determine if a test should be skipped
> or not. At that time, we proposed an integration with the build
> system, but the downside (and the reason for not pushing it forward)
> was the requirement to run the iotest outside of a build tree.
>
>>> Suggested-by: Kevin Wolf
>>> Signed-off-by: Andrey Shinkevich
>>> ---
>>> tests/qemu-iotests/common.rc | 7 +++
>>> 1 file changed, 7 insertions(+)
>>>
>>> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
>>> index 6e461a1..1089050 100644
>>> --- a/tests/qemu-iotests/common.rc
>>> +++ b/tests/qemu-iotests/common.rc
>>> @@ -428,6 +428,13 @@ _notrun()
>>> exit
>>> }
>>>
>>> +# bail out, setting up .casenotrun file
>>> +#
>>> +_casenotrun()
>>> +{
>>> +echo "[case not run] $*" >>"$OUTPUT_DIR/$seq.casenotrun"
>>> +}
>>> +
>>> # just plain bail out
>>> #
>>> _fail()
>>>
>>
>> seems fine to me otherwise.
>>
>> Reviewed-by: John Snow
>
> Yeah, this also LGTM.
>
> Reviewed-by: Cleber Rosa
>
Thank you very much for your reviews. Please note that the function
_casenotrun() works as a notifier only as it was done for the Python
based iotests. It is a caller responsibility to skip running a
particular test with all relevant logic. I will supply the comment in v6
and will keep your 'Reviewed-by' if there are no objections ))
Andrey
--
With the best regards,
Andrey Shinkevich
Re: [Qemu-devel] [PATCH 02/13] qcrypto-luks: misc refactoring
On Thu, 2019-08-22 at 16:32 +0200, Max Reitz wrote: > On 22.08.19 01:59, Maxim Levitsky wrote: > > On Tue, 2019-08-20 at 19:36 +0200, Max Reitz wrote: > > > On 14.08.19 22:22, Maxim Levitsky wrote: > > > > This is also a preparation for key read/write/erase functions > > > > > > > > * use master key len from the header > > > > * prefer to use crypto params in the QCryptoBlockLUKS > > > > over passing them as function arguments > > > > * define QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME > > > > * Add comments to various crypto parameters in the QCryptoBlockLUKS > > > > > > > > Signed-off-by: Maxim Levitsky > > > > --- > > > > crypto/block-luks.c | 213 ++-- > > > > 1 file changed, 105 insertions(+), 108 deletions(-) > > > [...] > > > > > @@ -410,21 +430,15 @@ > > > > qcrypto_block_luks_essiv_cipher(QCryptoCipherAlgorithm cipher, > > > > */ > > > > static int > > > > qcrypto_block_luks_load_key(QCryptoBlock *block, > > > > -QCryptoBlockLUKSKeySlot *slot, > > > > +uint slot_idx, > > > > > > Did you use uint on purpose or do you mean a plain “unsigned”? > > > > Well there are just 8 slots, but yea I don't mind to make this an unsigned > > int. > > My point was that “uint” is not a standard C type. There are lot of non standard types, like the u8/u8/u32/u64 I used to use in the kernel, so I kind of missed that. Won't use that type anymore :-) > > [...] > > > > > @@ -930,6 +922,17 @@ qcrypto_block_luks_create(QCryptoBlock *block, > > > > luks_opts.has_ivgen_hash_alg = true; > > > > } > > > > } > > > > + > > > > +luks = g_new0(QCryptoBlockLUKS, 1); > > > > +block->opaque = luks; > > > > + > > > > +luks->cipher_alg = luks_opts.cipher_alg; > > > > +luks->cipher_mode = luks_opts.cipher_mode; > > > > +luks->ivgen_alg = luks_opts.ivgen_alg; > > > > +luks->ivgen_hash_alg = luks_opts.ivgen_hash_alg; > > > > +luks->hash_alg = luks_opts.hash_alg; > > > > + > > > > + > > > > > > Why did you pull this up? Now @luks is leaked in both of the next error > > > paths. > > > > This is because the purpose of these fields changed. As Daniel explained to > > me > > they were initially added after the fact to serve as a cache of into to > > present in qemu-img info callback. > > But now I use these everywhere in the code so I won't them to be correct as > > soon as possible to minimize > > the risk of calling some function that uses these and would read garbage. > > I get that, but I was wondering why you pulled the allocation of @luks > up above the next two conditional blocks. Allocating and initializing > there should have worked just fine. Yea, I didn't have to, just thought that putting the initialization as above as possible is a good thing for future. > > Max > Best regards, Maxim Levitsky
Re: [Qemu-devel] [PATCH v5 0/6] Allow Valgrind checking all QEMU processes
On 16/08/2019 23:05, Cleber Rosa wrote: > On Fri, Jul 19, 2019 at 07:30:10PM +0300, Andrey Shinkevich wrote: >> In the current implementation of the QEMU bash iotests, only qemu-io >> processes may be run under the Valgrind, which is a useful tool for >> finding memory usage issues. Let's allow the common.rc bash script >> runing all the QEMU processes, such as qemu-kvm, qemu-img, qemu-ndb >> and qemu-vxhs, under the Valgrind tool. >> > > FIY, this looks very similar (in purpose) to: > > https://avocado-framework.readthedocs.io/en/71.0/WrapProcess.html > > And in fact Valgrind was one of the original motivations: > > > https://github.com/avocado-framework/avocado/blob/master/examples/wrappers/valgrind.sh > > Maybe this can be helpful for the Python based iotests. > > - Cleber. > Thank you Cleber for the advice. That is the way I actually ran Python iotests under Valgrind on my host and discovered some issues with them already. Andrey >> v5: >>01: The patch "block/nbd: NBDReply is used being uninitialized" was >> detached >>and taken into account in the patch "nbd: Initialize reply on failure" >>by Eric Blake. >> >> v4: >>01: The patch "iotests: Set read-zeroes on in null block driver for >> Valgrind" >>was extended with new cases and issued as a separate series. >>02: The new patch "block/nbd: NBDReply is used being uninitialized" was >>added to resolve the failure of the iotest 083 run under Valgrind. >> >> v3: >>01: The new function _casenotrun() was added to the common.rc bash >>script to notify the user of test cases dropped for some reason. >>Suggested by Kevin. >>Particularly, the notification about the nonexistent TMPDIR in >>the test 051 was added (noticed by Vladimir). >>02: The timeout in some test cases was extended for Valgrind because >>it differs when running on the ramdisk. >>03: Due to the common.nbd script has been changed with the commit >>b28f582c, the patch "iotests: amend QEMU NBD process synchronization" >>is actual no more. Note that QEMU_NBD is launched in the bash nested >>shell in the _qemu_nbd_wrapper() as it was before in common.rc. >>04: The patch "iotests: new file to suppress Valgrind errors" was dropped >>due to my superficial understanding of the work of the function >>blk_pread_unthrottled(). Special thanks to Kevin who shed the light >>on the null block driver involved. Now, the parameter 'read-zeroes=on' >>is passed to the null block driver to initialize the buffer in the >>function guess_disk_lchs() that the Valgrind was complaining to. >> >> v2: >>01: The patch 2/7 of v1 was merged into the patch 1/7, suggested by >> Daniel. >>02: Another patch 7/7 was added to introduce the Valgrind error >> suppression >>file into the QEMU project. >>Discussed in the email thread with the message ID: >><1560276131-683243-1-git-send-email-andrey.shinkev...@virtuozzo.com> >> >> Andrey Shinkevich (6): >>iotests: allow Valgrind checking all QEMU processes >>iotests: exclude killed processes from running under Valgrind >>iotests: Add casenotrun report to bash tests >>iotests: Valgrind fails with nonexistent directory >>iotests: extended timeout under Valgrind >>iotests: extend sleeping time under Valgrind >> >> tests/qemu-iotests/028 | 6 +++- >> tests/qemu-iotests/039 | 5 +++ >> tests/qemu-iotests/039.out | 30 +++-- >> tests/qemu-iotests/051 | 4 +++ >> tests/qemu-iotests/061 | 2 ++ >> tests/qemu-iotests/061.out | 12 ++- >> tests/qemu-iotests/137 | 1 + >> tests/qemu-iotests/137.out | 6 +--- >> tests/qemu-iotests/183 | 9 +- >> tests/qemu-iotests/192 | 6 +++- >> tests/qemu-iotests/247 | 6 +++- >> tests/qemu-iotests/common.rc | 76 >> +--- >> 12 files changed, 101 insertions(+), 62 deletions(-) >> >> -- >> 1.8.3.1 >> >> -- With the best regards, Andrey Shinkevich
Re: [Qemu-devel] [Qemu-block] [PATCH v5 6/6] iotests: extend sleeping time under Valgrind
On 16/08/2019 04:01, John Snow wrote:
>
>
> On 7/19/19 12:30 PM, Andrey Shinkevich wrote:
>> To synchronize the time when QEMU is running longer under the Valgrind,
>> increase the sleeping time in the test 247.
>>
>> Signed-off-by: Andrey Shinkevich
>> Reviewed-by: Vladimir Sementsov-Ogievskiy
>> ---
>> tests/qemu-iotests/247 | 6 +-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/qemu-iotests/247 b/tests/qemu-iotests/247
>> index 546a794..c853b73 100755
>> --- a/tests/qemu-iotests/247
>> +++ b/tests/qemu-iotests/247
>> @@ -57,7 +57,11 @@ TEST_IMG="$TEST_IMG.4" _make_test_img $size
>> {"execute":"block-commit",
>>"arguments":{"device":"format-4", "top-node": "format-2",
>> "base-node":"format-0", "job-id":"job0"}}
>> EOF
>> -sleep 1
>> +if [ "${VALGRIND_QEMU}" == "y" ]; then
>> +sleep 10
>> +else
>> +sleep 1
>> +fi
>> echo '{"execute":"quit"}'
>> ) | $QEMU -qmp stdio -nographic -nodefaults \
>> -blockdev file,node-name=file-0,filename=$TEST_IMG.0,auto-read-only=on
>> \
>>
>
> This makes me nervous, though. Won't this race terribly? (Wait, why
> doesn't it race already?)
>
It maybe better to rewrite this test in Python.
To let it work under Valgrind in the current implementation, I reserved
more seconds. We could have this tolerance just for the test.
Andrey
--
With the best regards,
Andrey Shinkevich
Re: [Qemu-devel] [PULL 07/15] audio: audiodev= parameters no longer optional when -audiodev present
On Wed, 2019-08-21 at 10:41 +0200, Gerd Hoffmann wrote: > From: Kővágó, Zoltán > > This means you should probably stop using -soundhw (as it doesn't allow > you to specify any options) and add the device manually with -device. > The exception is pcspk, it's currently not possible to manually add it. > To use it with audiodev, use something like this: > > -audiodev id=foo,... -global isa-pcspk.audiodev=foo -soundhw pcspk Hi! There is one corner case this breaks. In qemu 4.1.0, there is no way to specify audiodev for a sound device, specifying it fails with error. So some of my machines have audiodev (which is miles better that using old env variables) but also have sound devices without audiodev reference since this wasn't supported. In what will be qemu 4.2, you must specify it, thus this kind of breaks backward compatibility. Maybe we can have audiodev reference optional for a version or two? This is just a minor itch, as otherwise the sound improvements are really good. The days of installing that old realtek driver are finally gone :-) Another thing I noted, that there is no way for pulseaudio audiodev to specify the 'client name', it always shows up in pavucontrl as the socket path to the server. Thus if I added two PA audiodevs, I can't really distinguish between them. The in|out.name= seems to specify the pulseaudio source/sink to connect to, which is not the same. Best regards, Maxim Levitsky
Re: [Qemu-devel] [Qemu-block] [PATCH] block: posix: Always allocate the first block
On Sat, 2019-08-17 at 00:21 +0300, Nir Soffer wrote: > When creating an image with preallocation "off" or "falloc", the first > block of the image is typically not allocated. When using Gluster > storage backed by XFS filesystem, reading this block using direct I/O > succeeds regardless of request length, fooling alignment detection. > > In this case we fallback to a safe value (4096) instead of the optimal > value (512), which may lead to unneeded data copying when aligning > requests. Allocating the first block avoids the fallback. > > When using preallocation=off, we always allocate at least one filesystem > block: > > $ ./qemu-img create -f raw test.raw 1g > Formatting 'test.raw', fmt=raw size=1073741824 > > $ ls -lhs test.raw > 4.0K -rw-r--r--. 1 nsoffer nsoffer 1.0G Aug 16 23:48 test.raw Are you sure about this? [mlevitsk@maximlenovopc ~/work/test_area/posix-file 0]$ qemu-img create -f raw test.raw 1g -o preallocation=off Formatting 'test.raw', fmt=raw size=1073741824 preallocation=off [mlevitsk@maximlenovopc ~/work/test_area/posix-file 0]$ls -lhs ./test.raw 0 -rw-r--r--. 1 mlevitsk mlevitsk 1.0G Aug 25 10:38 ./test.raw ext4, tested on qemu-4.0.0 and qemu git master. >From what I remember, the only case when posix-raw touches the first block is >to zero it out when running on top of kernel block device, to erase whatever header might be there, and this is also kind of a backward compat hack which might be one day removed. [...] Best regards, Maxim Levitsky
[Qemu-devel] [PATCH 2/2] block/nvme: add support for discard
Signed-off-by: Maxim Levitsky
---
block/nvme.c | 83 ++
block/trace-events | 2 ++
2 files changed, 85 insertions(+)
diff --git a/block/nvme.c b/block/nvme.c
index f8bd11e19a..dd041f39c9 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -112,6 +112,7 @@ typedef struct {
bool plugged;
bool supports_write_zeros;
+bool supports_discard;
CoMutex dma_map_lock;
CoQueue dma_flush_queue;
@@ -463,6 +464,7 @@ static void nvme_identify(BlockDriverState *bs, int
namespace, Error **errp)
oncs = le16_to_cpu(idctrl->oncs);
s->supports_write_zeros = (oncs & NVME_ONCS_WRITE_ZEROS) != 0;
+s->supports_discard = (oncs & NVME_ONCS_DSM) != 0;
memset(resp, 0, 4096);
@@ -1153,6 +1155,86 @@ static coroutine_fn int
nvme_co_pwrite_zeroes(BlockDriverState *bs,
}
+static int coroutine_fn nvme_co_pdiscard(BlockDriverState *bs,
+ int64_t offset,
+ int bytes)
+{
+BDRVNVMeState *s = bs->opaque;
+NVMeQueuePair *ioq = s->queues[1];
+NVMeRequest *req;
+NvmeDsmRange *buf;
+QEMUIOVector local_qiov;
+int ret;
+
+NvmeCmd cmd = {
+.opcode = NVME_CMD_DSM,
+.nsid = cpu_to_le32(s->nsid),
+.cdw10 = cpu_to_le32(0), /*number of ranges - 0 based*/
+.cdw11 = cpu_to_le32(1 << 2), /*deallocate bit*/
+};
+
+NVMeCoData data = {
+.ctx = bdrv_get_aio_context(bs),
+.ret = -EINPROGRESS,
+};
+
+if (!s->supports_discard) {
+return -ENOTSUP;
+}
+
+assert(s->nr_queues > 1);
+
+buf = qemu_try_blockalign0(bs, s->page_size);
+if (!buf) {
+return -ENOMEM;
+}
+
+buf->nlb = cpu_to_le32(bytes >> s->blkshift);
+buf->slba = cpu_to_le64(offset >> s->blkshift);
+buf->cattr = 0;
+
+qemu_iovec_init(_qiov, 1);
+qemu_iovec_add(_qiov, buf, 4096);
+
+req = nvme_get_free_req(ioq);
+assert(req);
+
+qemu_co_mutex_lock(>dma_map_lock);
+ret = nvme_cmd_map_qiov(bs, , req, _qiov);
+qemu_co_mutex_unlock(>dma_map_lock);
+
+if (ret) {
+req->busy = false;
+goto out;
+}
+
+trace_nvme_dsm(s, offset, bytes);
+
+nvme_submit_command(s, ioq, req, , nvme_rw_cb, );
+
+data.co = qemu_coroutine_self();
+while (data.ret == -EINPROGRESS) {
+qemu_coroutine_yield();
+}
+
+qemu_co_mutex_lock(>dma_map_lock);
+ret = nvme_cmd_unmap_qiov(bs, _qiov);
+qemu_co_mutex_unlock(>dma_map_lock);
+
+if (ret) {
+goto out;
+}
+
+ret = data.ret;
+trace_nvme_dsm_done(s, offset, bytes, ret);
+out:
+qemu_iovec_destroy(_qiov);
+qemu_vfree(buf);
+return ret;
+
+}
+
+
static int nvme_reopen_prepare(BDRVReopenState *reopen_state,
BlockReopenQueue *queue, Error **errp)
{
@@ -1259,6 +1341,7 @@ static BlockDriver bdrv_nvme = {
.bdrv_co_pwritev = nvme_co_pwritev,
.bdrv_co_pwrite_zeroes= nvme_co_pwrite_zeroes,
+.bdrv_co_pdiscard = nvme_co_pdiscard,
.bdrv_co_flush_to_disk= nvme_co_flush,
.bdrv_reopen_prepare = nvme_reopen_prepare,
diff --git a/block/trace-events b/block/trace-events
index 8209fbd0c7..7d1d48b502 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -153,6 +153,8 @@ nvme_write_zeros(void *s, uint64_t offset, uint64_t bytes,
int flags) "s %p offs
nvme_qiov_unaligned(const void *qiov, int n, void *base, size_t size, int
align) "qiov %p n %d base %p size 0x%zx align 0x%x"
nvme_prw_buffered(void *s, uint64_t offset, uint64_t bytes, int niov, int
is_write) "s %p offset %"PRId64" bytes %"PRId64" niov %d is_write %d"
nvme_rw_done(void *s, int is_write, uint64_t offset, uint64_t bytes, int ret)
"s %p is_write %d offset %"PRId64" bytes %"PRId64" ret %d"
+nvme_dsm(void *s, uint64_t offset, uint64_t bytes) "s %p offset %"PRId64"
bytes %"PRId64""
+nvme_dsm_done(void *s, uint64_t offset, uint64_t bytes, int ret) "s %p offset
%"PRId64" bytes %"PRId64" ret %d"
nvme_dma_map_flush(void *s) "s %p"
nvme_free_req_queue_wait(void *q) "q %p"
nvme_cmd_map_qiov(void *s, void *cmd, void *req, void *qiov, int entries) "s
%p cmd %p req %p qiov %p entries %d"
--
2.17.2
[Qemu-devel] [PATCH 0/2] block/nvme: add support for write zeros and discard
This is the second part of the patches I prepared for this driver back when I worked on mdev-nvme. Best regards, Maxim Levitsky Maxim Levitsky (2): block/nvme: add support for write zeros block/nvme: add support for discard block/nvme.c | 155 ++- block/trace-events | 3 + include/block/nvme.h | 19 +- 3 files changed, 175 insertions(+), 2 deletions(-) -- 2.17.2
[Qemu-devel] [PATCH 1/2] block/nvme: add support for write zeros
Signed-off-by: Maxim Levitsky
---
block/nvme.c | 72 +++-
block/trace-events | 1 +
include/block/nvme.h | 19 +++-
3 files changed, 90 insertions(+), 2 deletions(-)
diff --git a/block/nvme.c b/block/nvme.c
index 5be3a39b63..f8bd11e19a 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -111,6 +111,8 @@ typedef struct {
uint64_t max_transfer;
bool plugged;
+bool supports_write_zeros;
+
CoMutex dma_map_lock;
CoQueue dma_flush_queue;
@@ -421,6 +423,7 @@ static void nvme_identify(BlockDriverState *bs, int
namespace, Error **errp)
NvmeIdNs *idns;
NvmeLBAF *lbaf;
uint8_t *resp;
+uint16_t oncs;
int r;
uint64_t iova;
NvmeCmd cmd = {
@@ -458,6 +461,9 @@ static void nvme_identify(BlockDriverState *bs, int
namespace, Error **errp)
s->max_transfer = MIN_NON_ZERO(s->max_transfer,
s->page_size / sizeof(uint64_t) * s->page_size);
+oncs = le16_to_cpu(idctrl->oncs);
+s->supports_write_zeros = (oncs & NVME_ONCS_WRITE_ZEROS) != 0;
+
memset(resp, 0, 4096);
cmd.cdw10 = 0;
@@ -470,6 +476,12 @@ static void nvme_identify(BlockDriverState *bs, int
namespace, Error **errp)
s->nsze = le64_to_cpu(idns->nsze);
lbaf = >lbaf[NVME_ID_NS_FLBAS_INDEX(idns->flbas)];
+if (NVME_ID_NS_DLFEAT_WRITE_ZEROS(idns->dlfeat) &&
+NVME_ID_NS_DLFEAT_READ_BEHAVIOR(idns->dlfeat) ==
+NVME_ID_NS_DLFEAT_READ_BEHAVIOR_ZEROS) {
+bs->supported_write_flags |= BDRV_REQ_MAY_UNMAP;
+}
+
if (lbaf->ms) {
error_setg(errp, "Namespaces with metadata are not yet supported");
goto out;
@@ -764,6 +776,8 @@ static int nvme_file_open(BlockDriverState *bs, QDict
*options, int flags,
int ret;
BDRVNVMeState *s = bs->opaque;
+bs->supported_write_flags = BDRV_REQ_FUA;
+
opts = qemu_opts_create(_opts, NULL, 0, _abort);
qemu_opts_absorb_qdict(opts, options, _abort);
device = qemu_opt_get(opts, NVME_BLOCK_OPT_DEVICE);
@@ -792,7 +806,6 @@ static int nvme_file_open(BlockDriverState *bs, QDict
*options, int flags,
goto fail;
}
}
-bs->supported_write_flags = BDRV_REQ_FUA;
return 0;
fail:
nvme_close(bs);
@@ -1086,6 +1099,60 @@ static coroutine_fn int nvme_co_flush(BlockDriverState
*bs)
}
+static coroutine_fn int nvme_co_pwrite_zeroes(BlockDriverState *bs,
+ int64_t offset,
+ int bytes,
+ BdrvRequestFlags flags)
+{
+BDRVNVMeState *s = bs->opaque;
+NVMeQueuePair *ioq = s->queues[1];
+NVMeRequest *req;
+
+uint32_t cdw12 = ((bytes >> s->blkshift) - 1) & 0x;
+
+if (!s->supports_write_zeros) {
+return -ENOTSUP;
+}
+
+NvmeCmd cmd = {
+.opcode = NVME_CMD_WRITE_ZEROS,
+.nsid = cpu_to_le32(s->nsid),
+.cdw10 = cpu_to_le32((offset >> s->blkshift) & 0x),
+.cdw11 = cpu_to_le32(((offset >> s->blkshift) >> 32) & 0x),
+};
+
+NVMeCoData data = {
+.ctx = bdrv_get_aio_context(bs),
+.ret = -EINPROGRESS,
+};
+
+if (flags & BDRV_REQ_MAY_UNMAP) {
+cdw12 |= (1 << 25);
+}
+
+if (flags & BDRV_REQ_FUA) {
+cdw12 |= (1 << 30);
+}
+
+cmd.cdw12 = cpu_to_le32(cdw12);
+
+trace_nvme_write_zeros(s, offset, bytes, flags);
+assert(s->nr_queues > 1);
+req = nvme_get_free_req(ioq);
+assert(req);
+
+nvme_submit_command(s, ioq, req, , nvme_rw_cb, );
+
+data.co = qemu_coroutine_self();
+while (data.ret == -EINPROGRESS) {
+qemu_coroutine_yield();
+}
+
+trace_nvme_rw_done(s, true, offset, bytes, data.ret);
+return data.ret;
+}
+
+
static int nvme_reopen_prepare(BDRVReopenState *reopen_state,
BlockReopenQueue *queue, Error **errp)
{
@@ -1190,6 +1257,9 @@ static BlockDriver bdrv_nvme = {
.bdrv_co_preadv = nvme_co_preadv,
.bdrv_co_pwritev = nvme_co_pwritev,
+
+.bdrv_co_pwrite_zeroes= nvme_co_pwrite_zeroes,
+
.bdrv_co_flush_to_disk= nvme_co_flush,
.bdrv_reopen_prepare = nvme_reopen_prepare,
diff --git a/block/trace-events b/block/trace-events
index 04209f058d..8209fbd0c7 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -149,6 +149,7 @@ nvme_submit_command_raw(int c0, int c1, int c2, int c3, int
c4, int c5, int c6,
nvme_handle_event(void *s) "s %p"
nvme_poll_cb(void *s) "s %p"
nvme_prw_aligned(void *s, int is_write, uint64_t offset, uint64_t bytes, int
flags, int niov) "s %p is_write %d offset %"PRId64" bytes %"PRId64" flags %d
niov %d"
+nvme_write_zeros(void *s, uint64_t offset, uint64_t bytes, int flags) "s %p
offset %"PRId64" bytes %"PRId64" flags %d"
nvme_qiov_unaligned(const void *qiov, int n, void *base, size_t size, int
align) "qiov %p n %d base %p size
