Re: [RFC PATCH 15/15] arm: xlnx-versal: Add emmc to versal
On Fri, Feb 12, 2021 at 01:37:18PM -0800, Alistair Francis wrote:
> On Thu, Feb 11, 2021 at 12:36 AM Sai Pavan Boddu
> wrote:
> >
> > Configuring SDHCI-0 to act as eMMC controller.
> >
> > Signed-off-by: Sai Pavan Boddu
>
> Reviewed-by: Alistair Francis
>
> Alistair
Hi Sai,
It would be great, if EMMC somehow could be made optional.
In any case, I think this is OK!
Reviewed-by: Edgar E. Iglesias
Could you please also add an example command-line in
docs/system/arm/xlnx-versal-virt.rst?
Thanks,
Edgar
>
> > ---
> > hw/arm/xlnx-versal-virt.c | 16 +++-
> > hw/arm/xlnx-versal.c | 14 --
> > 2 files changed, 23 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
> > index 8482cd6..18489e4 100644
> > --- a/hw/arm/xlnx-versal-virt.c
> > +++ b/hw/arm/xlnx-versal-virt.c
> > @@ -333,6 +333,13 @@ static void fdt_add_sd_nodes(VersalVirt *s)
> > qemu_fdt_setprop_sized_cells(s->fdt, name, "reg",
> > 2, addr, 2, MM_PMC_SD0_SIZE);
> > qemu_fdt_setprop(s->fdt, name, "compatible", compat,
> > sizeof(compat));
> > +/*
> > + * eMMC specific properties
> > + */
> > +if (i == 0) {
> > +qemu_fdt_setprop(s->fdt, name, "non-removable", NULL, 0);
> > +qemu_fdt_setprop_sized_cells(s->fdt, name, "bus-width", 1, 8);
> > +}
> > g_free(name);
> > }
> > }
> > @@ -512,7 +519,7 @@ static void create_virtio_regions(VersalVirt *s)
> > }
> > }
> >
> > -static void sd_plugin_card(SDHCIState *sd, DriveInfo *di)
> > +static void sd_plugin_card(SDHCIState *sd, DriveInfo *di, bool emmc)
> > {
> > BlockBackend *blk = di ? blk_by_legacy_dinfo(di) : NULL;
> > DeviceState *card;
> > @@ -520,6 +527,7 @@ static void sd_plugin_card(SDHCIState *sd, DriveInfo
> > *di)
> > card = qdev_new(TYPE_SD_CARD);
> > object_property_add_child(OBJECT(sd), "card[*]", OBJECT(card));
> > qdev_prop_set_drive_err(card, "drive", blk, _fatal);
> > +object_property_set_bool(OBJECT(card), "emmc", emmc, _fatal);
> > qdev_realize_and_unref(card, qdev_get_child_bus(DEVICE(sd), "sd-bus"),
> > _fatal);
> > }
> > @@ -528,7 +536,6 @@ static void versal_virt_init(MachineState *machine)
> > {
> > VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(machine);
> > int psci_conduit = QEMU_PSCI_CONDUIT_DISABLED;
> > -int i;
> >
> > /*
> > * If the user provides an Operating System to be loaded, we expect
> > them
> > @@ -581,10 +588,9 @@ static void versal_virt_init(MachineState *machine)
> > memory_region_add_subregion_overlap(get_system_memory(),
> > 0, >soc.fpd.apu.mr, 0);
> >
> > +sd_plugin_card(>soc.pmc.iou.sd[0], drive_get_next(IF_EMMC), true);
> > /* Plugin SD cards. */
> > -for (i = 0; i < ARRAY_SIZE(s->soc.pmc.iou.sd); i++) {
> > -sd_plugin_card(>soc.pmc.iou.sd[i], drive_get_next(IF_SD));
> > -}
> > +sd_plugin_card(>soc.pmc.iou.sd[1], drive_get_next(IF_SD), false);
> >
> > s->binfo.ram_size = machine->ram_size;
> > s->binfo.loader_start = 0x0;
> > diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
> > index b077716..3498dd9 100644
> > --- a/hw/arm/xlnx-versal.c
> > +++ b/hw/arm/xlnx-versal.c
> > @@ -230,9 +230,14 @@ static void versal_create_admas(Versal *s, qemu_irq
> > *pic)
> > }
> >
> > #define SDHCI_CAPABILITIES 0x280737ec6481 /* Same as on ZynqMP. */
> > +#define SDHCI0_CAPS ((SDHCI_CAPABILITIES & ~(3 << 30)) | \
> > + (1 << 30))
> > +#define SDHCI1_CAPS SDHCI_CAPABILITIES
> > +
> > static void versal_create_sds(Versal *s, qemu_irq *pic)
> > {
> > int i;
> > +uint64_t caps[] = {SDHCI0_CAPS, SDHCI1_CAPS};
> >
> > for (i = 0; i < ARRAY_SIZE(s->pmc.iou.sd); i++) {
> > DeviceState *dev;
> > @@ -244,9 +249,14 @@ static void versal_create_sds(Versal *s, qemu_irq *pic)
> >
> > object_property_set_uint(OBJECT(dev), "sd-spec-version", 3,
> > _fatal);
> > -object_property_set_uint(OBJECT(dev), "capareg",
> > SDHCI_CAPABILITIES,
> > +object_property_set_uint(OBJECT(dev), "capareg", caps[i],
> > _fatal);
> > -object_property_set_uint(OBJECT(dev), "uhs", UHS_I, _fatal);
> > +/*
> > + * UHS is not applicable for eMMC
> > + */
> > +if (i == 1) {
> > +object_property_set_uint(OBJECT(dev), "uhs", UHS_I,
> > _fatal);
> > +}
> > sysbus_realize(SYS_BUS_DEVICE(dev), _fatal);
> >
> > mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
> > --
> > 2.7.4
> >
> >
Re: Virgil 3D renderer on macOS
2021年2月13日(土) 12:20 Programmingkid : > I am also an M1 Mac owner. I don't know very much about graphics cards but I > would be happy to help if you need a tester. I don't need a tester now because I have not fixed even likely-obvious performance and compatibility problems which I can find out easily if I debug it or I run some serious performance/conformance test programs. But thank you for your proposal. > > I was doing a lot of thinking about implementing a 3D video card in QEMU. One > option was to port PCem's Voodoo2 card to QEMU. Another option was to > implement the ATI Rage 128 card. Drivers wouldn't probably be a problem since > they already exist for Mac OS and Windows. One issue users might encounter is > game support. Both these cards are older and probably are missing features > that newer games need. Then there's the problem of proprietary firmware files > these cards probably use. So I was wondering what your opinion on this issue. > Should we focus on emulating a real video card or focus on making Virgil 3D > compatible with more operating systems? The latter. I have no idea how complex such ancient accelerators are, but probably fewer people than those interested in Virgil are interested in them. The difference is quite important; you can reuse code written for different purposes if you hack an existing program, Virgil. In the case of my port, I made Virgil 3D renderer run on a macOS/CGL host where it was mainly written for Linux/DRM hosts. You can do the same, and port Virgil 3D drivers which already exist to other platforms although it should be much harder than my port, which is essentially fixes of some minor problems of programs written with standard APIs (OpenGL).
[PATCH] spice-app: avoid crash when core spice module doesn't loaded
When qemu is built with modules, but a given module doesn't load
qemu should handle that gracefully. When ui-spice-core.so isn't
able to be loaded and qemu is invoked with -display spice-app or
-spice, qemu will dereference a null pointer. With this change we
check the pointer before dereferencing and error out in a normal
way.
Signed-off-by: Bruce Rogers
---
ui/spice-app.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/ui/spice-app.c b/ui/spice-app.c
index 026124ef56..4325ac2d9c 100644
--- a/ui/spice-app.c
+++ b/ui/spice-app.c
@@ -129,6 +129,7 @@ static void spice_app_atexit(void)
static void spice_app_display_early_init(DisplayOptions *opts)
{
QemuOpts *qopts;
+QemuOptsList *list;
GError *err = NULL;
if (opts->has_full_screen) {
@@ -159,11 +160,16 @@ static void spice_app_display_early_init(DisplayOptions
*opts)
exit(1);
}
}
+list = qemu_find_opts("spice");
+if (list == NULL) {
+error_report("spice-app missing spice support");
+exit(1);
+}
type_register(_vc_type_info);
sock_path = g_strjoin("", app_dir, "/", "spice.sock", NULL);
-qopts = qemu_opts_create(qemu_find_opts("spice"), NULL, 0, _abort);
+qopts = qemu_opts_create(list, NULL, 0, _abort);
qemu_opt_set(qopts, "disable-ticketing", "on", _abort);
qemu_opt_set(qopts, "unix", "on", _abort);
qemu_opt_set(qopts, "addr", sock_path, _abort);
--
2.30.0
Re: Virgil 3D renderer on macOS
> On Feb 12, 2021, at 4:25 AM, qemu-devel-requ...@nongnu.org wrote:
>
> Message: 2
> Date: Fri, 12 Feb 2021 12:52:21 +0900
> From: 小田喜陽彦
> To: qemu-devel@nongnu.org, virglrenderer-de...@lists.freedesktop.org
> Subject: Virgil 3D renderer on macOS
> Message-ID:
>
> Content-Type: text/plain; charset="UTF-8"
>
> Hi,
>
> I would like to introduce my Virgil 3D renderer port to macOS.
>
> Some patches which are made in the process are useful even without
> Virgil 3D renderer, and already submitted to upstreams:
> - [PATCH] ui/cocoa: Support unique keys of JIS keyboards
> - [PATCH] ui/cocoa: Remove the uses of full screen APIs
> - [PATCH] ui/cocoa: Do not copy members of pixman image
> - [PATCH] ui/cocoa: Interpret left button down as is when command is pressed
> - Support ANGLE on macOS by akihikodaki · Pull Request #239 · anholt/libepoxy
> https://github.com/anholt/libepoxy/pull/239
>
> I will send other patches when I confirm they do not cause harm on
> Linux hosts, or changes they depend on get merged.
>
> The complete source code is available on GitHub:
> https://github.com/akihikodaki/libepoxy/tree/macos
> https://github.com/akihikodaki/qemu/tree/macos
> https://github.com/akihikodaki/virglrenderer/tree/macos
>
> The "cocoa" display of QEMU will provide OpenGL support to the guest
> on macOS hosts. "NSOpenGLContext" (which wraps "CGL") will be the
> backend for core profile. ANGLE (which also wraps CGL and provides
> compatibility improvements) will be the backend for ES profile. It is
> possible to build without ANGLE, but such a build will lose ES profile
> compatibility.
>
> Videos captured on M1 MacBook Air are available on YouTube:
> https://youtu.be/ezvQPREjN1s (The WebGL Aquarium on the host, for comparison)
> https://youtu.be/iOG9Dbn8VoE (QEMU with OpenGL Core)
> https://youtu.be/k0bVlVQU2JQ (QEMU with OpenGL ES)
>
> glmark2 gives 577 scores for gl=es and 151 scores for gl=off. The FPS
> of the WebGL aquarium with identical configurations was consistent
> with the display on the host (60 FPS), 15 FPS with gl=es, 8 FPS with
> gl=off.
>
> I have not ran a formal conformance tests, but gl=core (which uses
> NSOpenGLContext) had a few problems:
> - glmark2 fails with the following output:
>> vrend_compile_shader: context error reported 6 "glmark2" Illegal shader 0
>> shader failed to compile
>> ERROR: 0:2: '' : extension 'GL_ARB_fragment_coord_conventions' is not
>> supported
>
> - Mozilla Firefox opening "about:support" fails with the following
> output on the host (This one should be easy to fix but I rather not
> because I also have gl=es.):
>> No provider of glTexStorage2DMultisample found. Requires one of:
>>Desktop OpenGL 4.3
>>
>>GL_ARB_texture_storage_multisample
>>OpenGL ES 3.1
>
> In contrast, gl=es, backed with ANGLE, runs properly as far as I have seen.
>
> My motivation is to make Linux desktop usable on M1. Patches to add
> Hypervisor framework support on Aarch64 are already submitted ("hvf:
> Implement Apple Silicon Support") and they are indeed useful for
> various workloads and I also used them to port Virgil, but I also
> needed graphics acceleration for my purpose. Another attempt to get
> Linux work on M1 is Asahi Linux, which aims to run Linux bare-metal.
> Of course, this needs porting graphic stacks to M1 and is likely to
> take time. I am satisfied with my port for the purpose although there
> may be rooms for performance or compatibility improvements.
>
> There is nothing preventing that if anyone would like to use Virgil on
> Intel Macs. Also, some patches may benefit other OpenGL ES
> configurations and displays.
>
> Thanks,
I am also an M1 Mac owner. I don't know very much about graphics cards but I
would be happy to help if you need a tester.
I was doing a lot of thinking about implementing a 3D video card in QEMU. One
option was to port PCem's Voodoo2 card to QEMU. Another option was to implement
the ATI Rage 128 card. Drivers wouldn't probably be a problem since they
already exist for Mac OS and Windows. One issue users might encounter is game
support. Both these cards are older and probably are missing features that
newer games need. Then there's the problem of proprietary firmware files these
cards probably use. So I was wondering what your opinion on this issue. Should
we focus on emulating a real video card or focus on making Virgil 3D compatible
with more operating systems?
Re: [PATCH v2] target/mips/translate: Simplify PCPYH using deposit_i64()
On 2/12/21 4:26 PM, Philippe Mathieu-Daudé wrote:
> Simplify the PCPYH (Parallel Copy Halfword) instruction by using
> multiple calls to deposit_i64() which can be optimized by some
> TCG backends.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> v2: Send the Halfword version :)
> ---
> target/mips/translate.c | 36 ++--
> 1 file changed, 6 insertions(+), 30 deletions(-)
>
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index a5cf1742a8b..ddae26009dd 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -24786,36 +24786,12 @@ static void gen_mmi_pcpyh(DisasContext *ctx)
> tcg_gen_movi_i64(cpu_gpr[rd], 0);
> tcg_gen_movi_i64(cpu_mmr[rd], 0);
> } else {
> -TCGv_i64 t0 = tcg_temp_new();
> -TCGv_i64 t1 = tcg_temp_new();
> -uint64_t mask = (1ULL << 16) - 1;
> -
> -tcg_gen_andi_i64(t0, cpu_gpr[rt], mask);
> -tcg_gen_movi_i64(t1, 0);
> -tcg_gen_or_i64(t1, t0, t1);
> -tcg_gen_shli_i64(t0, t0, 16);
> -tcg_gen_or_i64(t1, t0, t1);
> -tcg_gen_shli_i64(t0, t0, 16);
> -tcg_gen_or_i64(t1, t0, t1);
> -tcg_gen_shli_i64(t0, t0, 16);
> -tcg_gen_or_i64(t1, t0, t1);
> -
> -tcg_gen_mov_i64(cpu_gpr[rd], t1);
> -
> -tcg_gen_andi_i64(t0, cpu_mmr[rt], mask);
> -tcg_gen_movi_i64(t1, 0);
> -tcg_gen_or_i64(t1, t0, t1);
> -tcg_gen_shli_i64(t0, t0, 16);
> -tcg_gen_or_i64(t1, t0, t1);
> -tcg_gen_shli_i64(t0, t0, 16);
> -tcg_gen_or_i64(t1, t0, t1);
> -tcg_gen_shli_i64(t0, t0, 16);
> -tcg_gen_or_i64(t1, t0, t1);
> -
> -tcg_gen_mov_i64(cpu_mmr[rd], t1);
> -
> -tcg_temp_free(t0);
> -tcg_temp_free(t1);
> +for (int i = 0; i < 4; i++) {
> +tcg_gen_deposit_i64(cpu_gpr[rd],
> +cpu_gpr[rd], cpu_gpr[rd], 16 * i, 16);
> +tcg_gen_deposit_i64(cpu_mmr[rd],
> +cpu_mmr[rd], cpu_mmr[rd], 16 * i, 16);
Missing rt in the replacement.
To make 4 identical copies, make use of previous inserts:
tcg_gen_deposit_i64(rd, rt, rt, 16, 48);
tcg_gen_deposit_i64(rd, rd, rd, 32, 32);
r~
Re: [PATCH v3 0/3] Add npcm7xx emc model
On Thu, Feb 11, 2021 at 11:48 AM Peter Maydell
wrote:
> On Thu, 11 Feb 2021 at 11:34, Peter Maydell
> wrote:
> >
> > On Tue, 9 Feb 2021 at 01:55, Doug Evans wrote:
> > >
> > > This is a 10/100 ethernet device that has several features.
> > > Only the ones needed by the Linux driver have been implemented.
> > > See npcm7xx_emc.c for a list of unimplemented features.
> > >
> > > Doug Evans (3):
> > > hw/net: Add npcm7xx emc model
> > > hw/arm: Add npcm7xx emc model
> > > tests/qtests: Add npcm7xx emc model test
> > >
> >
> >
> >
> > Applied to target-arm.next, thanks.
>
> Dropped again; the new tests fail on big-endian hosts (s390x, ppc64):
>
> MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
> QTEST_QEMU_IMG=./qemu-img
> G_TEST_DBUS_DAEMON=/home/ubuntu/qemu/tests/dbus-vmstate-daemon.sh
> QTEST_QEMU_BINARY=./qemu-system-arm tests/qtest/npcm7xx_emc-test --tap
> -k
> PASS 1 qtest-arm/npcm7xx_emc-test /arm/npcm7xx_emc/emc[0]/init
> **
> ERROR:../../tests/qtest/npcm7xx_emc-test.c:476:emc_send_verify1:
> assertion failed ((result_desc.status_and_length & expected_mask) ==
> expected_value): (0x == 0x0008)
> ERROR qtest-arm/npcm7xx_emc-test - Bail out!
> ERROR:../../tests/qtest/npcm7xx_emc-test.c:476:emc_send_verify1:
> assertion failed ((result_desc.status_and_length & expected_mask) ==
> expected_value): (0x == 0x0008)
>
Fixed in V4:
https://lists.nongnu.org/archive/html/qemu-devel/2021-02/msg04529.html
[PATCH v4 3/3] tests/qtests: Add npcm7xx emc model test
Reviewed-by: Hao Wu
Reviewed-by: Avi Fishman
Reviewed-by: Peter Maydell
Signed-off-by: Doug Evans
---
Differences from v3:
- handle big endian hosts, tested on sparc64
Differences from v2:
- remove use of C99 mixed decls/statements
tests/qtest/meson.build| 1 +
tests/qtest/npcm7xx_emc-test.c | 862 +
2 files changed, 863 insertions(+)
create mode 100644 tests/qtest/npcm7xx_emc-test.c
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index c83bc211b6..f7c369f3d5 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -136,6 +136,7 @@ qtests_sparc64 = \
qtests_npcm7xx = \
['npcm7xx_adc-test',
+ 'npcm7xx_emc-test',
'npcm7xx_gpio-test',
'npcm7xx_pwm-test',
'npcm7xx_rng-test',
diff --git a/tests/qtest/npcm7xx_emc-test.c b/tests/qtest/npcm7xx_emc-test.c
new file mode 100644
index 00..7a28173195
--- /dev/null
+++ b/tests/qtest/npcm7xx_emc-test.c
@@ -0,0 +1,862 @@
+/*
+ * QTests for Nuvoton NPCM7xx EMC Modules.
+ *
+ * Copyright 2020 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "libqos/libqos.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qnum.h"
+#include "qemu/bitops.h"
+#include "qemu/iov.h"
+
+/* Name of the emc device. */
+#define TYPE_NPCM7XX_EMC "npcm7xx-emc"
+
+/* Timeout for various operations, in seconds. */
+#define TIMEOUT_SECONDS 10
+
+/* Address in memory of the descriptor. */
+#define DESC_ADDR (1 << 20) /* 1 MiB */
+
+/* Address in memory of the data packet. */
+#define DATA_ADDR (DESC_ADDR + 4096)
+
+#define CRC_LENGTH 4
+
+#define NUM_TX_DESCRIPTORS 3
+#define NUM_RX_DESCRIPTORS 2
+
+/* Size of tx,rx test buffers. */
+#define TX_DATA_LEN 64
+#define RX_DATA_LEN 64
+
+#define TX_STEP_COUNT 1
+#define RX_STEP_COUNT 1
+
+/* 32-bit register indices. */
+typedef enum NPCM7xxPWMRegister {
+/* Control registers. */
+REG_CAMCMR,
+REG_CAMEN,
+
+/* There are 16 CAMn[ML] registers. */
+REG_CAMM_BASE,
+REG_CAML_BASE,
+
+REG_TXDLSA = 0x22,
+REG_RXDLSA,
+REG_MCMDR,
+REG_MIID,
+REG_MIIDA,
+REG_FFTCR,
+REG_TSDR,
+REG_RSDR,
+REG_DMARFC,
+REG_MIEN,
+
+/* Status registers. */
+REG_MISTA,
+REG_MGSTA,
+REG_MPCNT,
+REG_MRPC,
+REG_MRPCC,
+REG_MREPC,
+REG_DMARFS,
+REG_CTXDSA,
+REG_CTXBSA,
+REG_CRXDSA,
+REG_CRXBSA,
+
+NPCM7XX_NUM_EMC_REGS,
+} NPCM7xxPWMRegister;
+
+enum { NUM_CAMML_REGS = 16 };
+
+/* REG_CAMCMR fields */
+/* Enable CAM Compare */
+#define REG_CAMCMR_ECMP (1 << 4)
+/* Accept Unicast Packet */
+#define REG_CAMCMR_AUP (1 << 0)
+
+/* REG_MCMDR fields */
+/* Software Reset */
+#define REG_MCMDR_SWR (1 << 24)
+/* Frame Transmission On */
+#define REG_MCMDR_TXON (1 << 8)
+/* Accept Long Packet */
+#define REG_MCMDR_ALP (1 << 1)
+/* Frame Reception On */
+#define REG_MCMDR_RXON (1 << 0)
+
+/* REG_MIEN fields */
+/* Enable Transmit Completion Interrupt */
+#define REG_MIEN_ENTXCP (1 << 18)
+/* Enable Transmit Interrupt */
+#define REG_MIEN_ENTXINTR (1 << 16)
+/* Enable Receive Good Interrupt */
+#define REG_MIEN_ENRXGD (1 << 4)
+/* ENable Receive Interrupt */
+#define REG_MIEN_ENRXINTR (1 << 0)
+
+/* REG_MISTA fields */
+/* Transmit Bus Error Interrupt */
+#define REG_MISTA_TXBERR (1 << 24)
+/* Transmit Descriptor Unavailable Interrupt */
+#define REG_MISTA_TDU (1 << 23)
+/* Transmit Completion Interrupt */
+#define REG_MISTA_TXCP (1 << 18)
+/* Transmit Interrupt */
+#define REG_MISTA_TXINTR (1 << 16)
+/* Receive Bus Error Interrupt */
+#define REG_MISTA_RXBERR (1 << 11)
+/* Receive Descriptor Unavailable Interrupt */
+#define REG_MISTA_RDU (1 << 10)
+/* DMA Early Notification Interrupt */
+#define REG_MISTA_DENI (1 << 9)
+/* Maximum Frame Length Interrupt */
+#define REG_MISTA_DFOI (1 << 8)
+/* Receive Good Interrupt */
+#define REG_MISTA_RXGD (1 << 4)
+/* Packet Too Long Interrupt */
+#define REG_MISTA_PTLE (1 << 3)
+/* Receive Interrupt */
+#define REG_MISTA_RXINTR (1 << 0)
+
+typedef struct NPCM7xxEMCTxDesc NPCM7xxEMCTxDesc;
+typedef struct NPCM7xxEMCRxDesc NPCM7xxEMCRxDesc;
+
+struct NPCM7xxEMCTxDesc {
+uint32_t flags;
+uint32_t txbsa;
+uint32_t status_and_length;
+uint32_t ntxdsa;
+};
+
+struct NPCM7xxEMCRxDesc {
+uint32_t status_and_length;
+uint32_t rxbsa;
+uint32_t reserved;
+uint32_t nrxdsa;
+};
+
+/* NPCM7xxEMCTxDesc.flags values */
+/* Owner: 0 = cpu, 1 = emc */
+#define TX_DESC_FLAG_OWNER_MASK (1 <<
[PATCH v4 2/3] hw/arm: Add npcm7xx emc model
This is a 10/100 ethernet device that has several features.
Only the ones needed by the Linux driver have been implemented.
See npcm7xx_emc.c for a list of unimplemented features.
Reviewed-by: Hao Wu
Reviewed-by: Avi Fishman
Reviewed-by: Peter Maydell
Signed-off-by: Doug Evans
---
Differences from v3:
- no change
Differences from v2:
- none, patch ok as is
docs/system/arm/nuvoton.rst | 3 ++-
hw/arm/npcm7xx.c| 50 +++--
include/hw/arm/npcm7xx.h| 2 ++
3 files changed, 52 insertions(+), 3 deletions(-)
diff --git a/docs/system/arm/nuvoton.rst b/docs/system/arm/nuvoton.rst
index a1786342e2..c6e9a4c17e 100644
--- a/docs/system/arm/nuvoton.rst
+++ b/docs/system/arm/nuvoton.rst
@@ -43,6 +43,7 @@ Supported devices
* GPIO controller
* Analog to Digital Converter (ADC)
* Pulse Width Modulation (PWM)
+ * Ethernet controller (EMC)
Missing devices
---
@@ -56,7 +57,7 @@ Missing devices
* Shared memory (SHM)
* eSPI slave interface
- * Ethernet controllers (GMAC and EMC)
+ * Ethernet controller (GMAC)
* USB device (USBD)
* SMBus controller (SMBF)
* Peripheral SPI controller (PSPI)
diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
index 72040d4079..94b79ff4c0 100644
--- a/hw/arm/npcm7xx.c
+++ b/hw/arm/npcm7xx.c
@@ -82,6 +82,8 @@ enum NPCM7xxInterrupt {
NPCM7XX_UART1_IRQ,
NPCM7XX_UART2_IRQ,
NPCM7XX_UART3_IRQ,
+NPCM7XX_EMC1RX_IRQ = 15,
+NPCM7XX_EMC1TX_IRQ,
NPCM7XX_TIMER0_IRQ = 32, /* Timer Module 0 */
NPCM7XX_TIMER1_IRQ,
NPCM7XX_TIMER2_IRQ,
@@ -104,6 +106,8 @@ enum NPCM7xxInterrupt {
NPCM7XX_OHCI_IRQ= 62,
NPCM7XX_PWM0_IRQ= 93, /* PWM module 0 */
NPCM7XX_PWM1_IRQ, /* PWM module 1 */
+NPCM7XX_EMC2RX_IRQ = 114,
+NPCM7XX_EMC2TX_IRQ,
NPCM7XX_GPIO0_IRQ = 116,
NPCM7XX_GPIO1_IRQ,
NPCM7XX_GPIO2_IRQ,
@@ -152,6 +156,12 @@ static const hwaddr npcm7xx_pwm_addr[] = {
0xf0104000,
};
+/* Register base address for each EMC Module */
+static const hwaddr npcm7xx_emc_addr[] = {
+0xf0825000,
+0xf0826000,
+};
+
static const struct {
hwaddr regs_addr;
uint32_t unconnected_pins;
@@ -365,6 +375,10 @@ static void npcm7xx_init(Object *obj)
for (i = 0; i < ARRAY_SIZE(s->pwm); i++) {
object_initialize_child(obj, "pwm[*]", >pwm[i], TYPE_NPCM7XX_PWM);
}
+
+for (i = 0; i < ARRAY_SIZE(s->emc); i++) {
+object_initialize_child(obj, "emc[*]", >emc[i], TYPE_NPCM7XX_EMC);
+}
}
static void npcm7xx_realize(DeviceState *dev, Error **errp)
@@ -537,6 +551,40 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
sysbus_connect_irq(sbd, i, npcm7xx_irq(s, NPCM7XX_PWM0_IRQ + i));
}
+/*
+ * EMC Modules. Cannot fail.
+ * The mapping of the device to its netdev backend works as follows:
+ * emc[i] = nd_table[i]
+ * This works around the inability to specify the netdev property for the
+ * emc device: it's not pluggable and thus the -device option can't be
+ * used.
+ */
+QEMU_BUILD_BUG_ON(ARRAY_SIZE(npcm7xx_emc_addr) != ARRAY_SIZE(s->emc));
+QEMU_BUILD_BUG_ON(ARRAY_SIZE(s->emc) != 2);
+for (i = 0; i < ARRAY_SIZE(s->emc); i++) {
+s->emc[i].emc_num = i;
+SysBusDevice *sbd = SYS_BUS_DEVICE(>emc[i]);
+if (nd_table[i].used) {
+qemu_check_nic_model(_table[i], TYPE_NPCM7XX_EMC);
+qdev_set_nic_properties(DEVICE(sbd), _table[i]);
+}
+/*
+ * The device exists regardless of whether it's connected to a QEMU
+ * netdev backend. So always instantiate it even if there is no
+ * backend.
+ */
+sysbus_realize(sbd, _abort);
+sysbus_mmio_map(sbd, 0, npcm7xx_emc_addr[i]);
+int tx_irq = i == 0 ? NPCM7XX_EMC1TX_IRQ : NPCM7XX_EMC2TX_IRQ;
+int rx_irq = i == 0 ? NPCM7XX_EMC1RX_IRQ : NPCM7XX_EMC2RX_IRQ;
+/*
+ * N.B. The values for the second argument sysbus_connect_irq are
+ * chosen to match the registration order in npcm7xx_emc_realize.
+ */
+sysbus_connect_irq(sbd, 0, npcm7xx_irq(s, tx_irq));
+sysbus_connect_irq(sbd, 1, npcm7xx_irq(s, rx_irq));
+}
+
/*
* Flash Interface Unit (FIU). Can fail if incorrect number of chip selects
* specified, but this is a programming error.
@@ -621,8 +669,6 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
create_unimplemented_device("npcm7xx.vcd", 0xf081, 64 * KiB);
create_unimplemented_device("npcm7xx.ece", 0xf082, 8 * KiB);
create_unimplemented_device("npcm7xx.vdma", 0xf0822000, 8 * KiB);
-create_unimplemented_device("npcm7xx.emc1", 0xf0825000, 4 * KiB);
-create_unimplemented_device("npcm7xx.emc2", 0xf0826000, 4 * KiB);
create_unimplemented_device("npcm7xx.usbd[0]", 0xf083,
[PATCH v2] target/mips/translate: Simplify PCPYH using deposit_i64()
Simplify the PCPYH (Parallel Copy Halfword) instruction by using
multiple calls to deposit_i64() which can be optimized by some
TCG backends.
Signed-off-by: Philippe Mathieu-Daudé
---
v2: Send the Halfword version :)
---
target/mips/translate.c | 36 ++--
1 file changed, 6 insertions(+), 30 deletions(-)
diff --git a/target/mips/translate.c b/target/mips/translate.c
index a5cf1742a8b..ddae26009dd 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -24786,36 +24786,12 @@ static void gen_mmi_pcpyh(DisasContext *ctx)
tcg_gen_movi_i64(cpu_gpr[rd], 0);
tcg_gen_movi_i64(cpu_mmr[rd], 0);
} else {
-TCGv_i64 t0 = tcg_temp_new();
-TCGv_i64 t1 = tcg_temp_new();
-uint64_t mask = (1ULL << 16) - 1;
-
-tcg_gen_andi_i64(t0, cpu_gpr[rt], mask);
-tcg_gen_movi_i64(t1, 0);
-tcg_gen_or_i64(t1, t0, t1);
-tcg_gen_shli_i64(t0, t0, 16);
-tcg_gen_or_i64(t1, t0, t1);
-tcg_gen_shli_i64(t0, t0, 16);
-tcg_gen_or_i64(t1, t0, t1);
-tcg_gen_shli_i64(t0, t0, 16);
-tcg_gen_or_i64(t1, t0, t1);
-
-tcg_gen_mov_i64(cpu_gpr[rd], t1);
-
-tcg_gen_andi_i64(t0, cpu_mmr[rt], mask);
-tcg_gen_movi_i64(t1, 0);
-tcg_gen_or_i64(t1, t0, t1);
-tcg_gen_shli_i64(t0, t0, 16);
-tcg_gen_or_i64(t1, t0, t1);
-tcg_gen_shli_i64(t0, t0, 16);
-tcg_gen_or_i64(t1, t0, t1);
-tcg_gen_shli_i64(t0, t0, 16);
-tcg_gen_or_i64(t1, t0, t1);
-
-tcg_gen_mov_i64(cpu_mmr[rd], t1);
-
-tcg_temp_free(t0);
-tcg_temp_free(t1);
+for (int i = 0; i < 4; i++) {
+tcg_gen_deposit_i64(cpu_gpr[rd],
+cpu_gpr[rd], cpu_gpr[rd], 16 * i, 16);
+tcg_gen_deposit_i64(cpu_mmr[rd],
+cpu_mmr[rd], cpu_mmr[rd], 16 * i, 16);
+}
}
}
--
2.26.2
[PATCH v4 0/3] Add npcm7xx emc model
This is a 10/100 ethernet device that has several features. Only the ones needed by the Linux driver have been implemented. See npcm7xx_emc.c for a list of unimplemented features. Doug Evans (3): hw/net: Add npcm7xx emc model hw/arm: Add npcm7xx emc model tests/qtests: Add npcm7xx emc model test docs/system/arm/nuvoton.rst| 3 +- hw/arm/npcm7xx.c | 50 +- hw/net/meson.build | 1 + hw/net/npcm7xx_emc.c | 857 hw/net/trace-events| 17 + include/hw/arm/npcm7xx.h | 2 + include/hw/net/npcm7xx_emc.h | 286 +++ tests/qtest/meson.build| 1 + tests/qtest/npcm7xx_emc-test.c | 862 + 9 files changed, 2076 insertions(+), 3 deletions(-) create mode 100644 hw/net/npcm7xx_emc.c create mode 100644 include/hw/net/npcm7xx_emc.h create mode 100644 tests/qtest/npcm7xx_emc-test.c -- 2.30.0.478.g8a0d178c01-goog Differences from v3: 1/3 hw/net: Add npcm7xx emc model - no change 2/3 hw/arm: Add npcm7xx emc model - no change 3/3 tests/qtests: Add npcm7xx emc model test - handle big endian hosts, tested on sparc64 Differences from v2: 1/3 hw/net: Add npcm7xx emc model - move call to qemu_set_irq - remove use of C99 mixed decls/statements - add use of g_autofree 2/3 hw/arm: Add npcm7xx emc model - none, patch ok as is 3/3 tests/qtests: Add npcm7xx emc model test - remove use of C99 mixed decls/statements
[PATCH v4 1/3] hw/net: Add npcm7xx emc model
This is a 10/100 ethernet device that has several features.
Only the ones needed by the Linux driver have been implemented.
See npcm7xx_emc.c for a list of unimplemented features.
Reviewed-by: Hao Wu
Reviewed-by: Avi Fishman
Reviewed-by: Peter Maydell
Signed-off-by: Doug Evans
---
Differences from v3:
- no change
Differences from v2:
- move call to qemu_set_irq
- remove use of C99 mixed decls/statements
- add use of g_autofree
hw/net/meson.build | 1 +
hw/net/npcm7xx_emc.c | 857 +++
hw/net/trace-events | 17 +
include/hw/net/npcm7xx_emc.h | 286
4 files changed, 1161 insertions(+)
create mode 100644 hw/net/npcm7xx_emc.c
create mode 100644 include/hw/net/npcm7xx_emc.h
diff --git a/hw/net/meson.build b/hw/net/meson.build
index 4a7051b54a..af0749c42b 100644
--- a/hw/net/meson.build
+++ b/hw/net/meson.build
@@ -35,6 +35,7 @@ softmmu_ss.add(when: 'CONFIG_I82596_COMMON', if_true:
files('i82596.c'))
softmmu_ss.add(when: 'CONFIG_SUNHME', if_true: files('sunhme.c'))
softmmu_ss.add(when: 'CONFIG_FTGMAC100', if_true: files('ftgmac100.c'))
softmmu_ss.add(when: 'CONFIG_SUNGEM', if_true: files('sungem.c'))
+softmmu_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_emc.c'))
softmmu_ss.add(when: 'CONFIG_ETRAXFS', if_true: files('etraxfs_eth.c'))
softmmu_ss.add(when: 'CONFIG_COLDFIRE', if_true: files('mcf_fec.c'))
diff --git a/hw/net/npcm7xx_emc.c b/hw/net/npcm7xx_emc.c
new file mode 100644
index 00..714a742ba7
--- /dev/null
+++ b/hw/net/npcm7xx_emc.c
@@ -0,0 +1,857 @@
+/*
+ * Nuvoton NPCM7xx EMC Module
+ *
+ * Copyright 2020 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ *
+ * Unsupported/unimplemented features:
+ * - MCMDR.FDUP (full duplex) is ignored, half duplex is not supported
+ * - Only CAM0 is supported, CAM[1-15] are not
+ * - writes to CAMEN.[1-15] are ignored, these bits always read as zeroes
+ * - MII is not implemented, MIIDA.BUSY and MIID always return zero
+ * - MCMDR.LBK is not implemented
+ * - MCMDR.{OPMOD,ENSQE,AEP,ARP} are not supported
+ * - H/W FIFOs are not supported, MCMDR.FFTCR is ignored
+ * - MGSTA.SQE is not supported
+ * - pause and control frames are not implemented
+ * - MGSTA.CCNT is not supported
+ * - MPCNT, DMARFS are not implemented
+ */
+
+#include "qemu/osdep.h"
+
+/* For crc32 */
+#include
+
+#include "qemu-common.h"
+#include "hw/irq.h"
+#include "hw/qdev-clock.h"
+#include "hw/qdev-properties.h"
+#include "hw/net/npcm7xx_emc.h"
+#include "net/eth.h"
+#include "migration/vmstate.h"
+#include "qemu/bitops.h"
+#include "qemu/error-report.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "qemu/units.h"
+#include "sysemu/dma.h"
+#include "trace.h"
+
+#define CRC_LENGTH 4
+
+/*
+ * The maximum size of a (layer 2) ethernet frame as defined by 802.3.
+ * 1518 = 6(dest macaddr) + 6(src macaddr) + 2(proto) + 4(crc) + 1500(payload)
+ * This does not include an additional 4 for the vlan field (802.1q).
+ */
+#define MAX_ETH_FRAME_SIZE 1518
+
+static const char *emc_reg_name(int regno)
+{
+#define REG(name) case REG_ ## name: return #name;
+switch (regno) {
+REG(CAMCMR)
+REG(CAMEN)
+REG(TXDLSA)
+REG(RXDLSA)
+REG(MCMDR)
+REG(MIID)
+REG(MIIDA)
+REG(FFTCR)
+REG(TSDR)
+REG(RSDR)
+REG(DMARFC)
+REG(MIEN)
+REG(MISTA)
+REG(MGSTA)
+REG(MPCNT)
+REG(MRPC)
+REG(MRPCC)
+REG(MREPC)
+REG(DMARFS)
+REG(CTXDSA)
+REG(CTXBSA)
+REG(CRXDSA)
+REG(CRXBSA)
+case REG_CAMM_BASE + 0: return "CAM0M";
+case REG_CAML_BASE + 0: return "CAM0L";
+case REG_CAMM_BASE + 2 ... REG_CAMML_LAST:
+/* Only CAM0 is supported, fold the others into something simple. */
+if (regno & 1) {
+return "CAML";
+} else {
+return "CAMM";
+}
+default: return "UNKNOWN";
+}
+#undef REG
+}
+
+static void emc_reset(NPCM7xxEMCState *emc)
+{
+trace_npcm7xx_emc_reset(emc->emc_num);
+
+memset(>regs[0], 0, sizeof(emc->regs));
+
+/* These regs have non-zero reset values. */
+emc->regs[REG_TXDLSA] = 0xfffc;
+emc->regs[REG_RXDLSA] = 0xfffc;
+emc->regs[REG_MIIDA] = 0x0090;
+emc->regs[REG_FFTCR] = 0x0101;
+emc->regs[REG_DMARFC] = 0x0800;
+emc->regs[REG_MPCNT] = 0x7fff;
+
+emc->tx_active = false;
+emc->rx_active = false;
+}
+
+static void npcm7xx_emc_reset(DeviceState *dev)
+{
+NPCM7xxEMCState *emc = NPCM7XX_EMC(dev);
+emc_reset(emc);
+}
+
Re: [PATCH] target/mips/translate: Simplify PCPYH using deposit_i64()
On 2/13/21 1:19 AM, Philippe Mathieu-Daudé wrote:
> Simplify the PCPYH (Parallel Copy Halfword) instruction by using
> multiple calls to deposit_i64() which can be optimized by some
> TCG backends.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> target/mips/translate.c | 36 ++--
> 1 file changed, 6 insertions(+), 30 deletions(-)
>
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index a5cf1742a8b..5b31aa44f30 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -24786,36 +24786,12 @@ static void gen_mmi_pcpyh(DisasContext *ctx)
> tcg_gen_movi_i64(cpu_gpr[rd], 0);
> tcg_gen_movi_i64(cpu_mmr[rd], 0);
> } else {
> -TCGv_i64 t0 = tcg_temp_new();
> -TCGv_i64 t1 = tcg_temp_new();
> -uint64_t mask = (1ULL << 16) - 1;
> -
> -tcg_gen_andi_i64(t0, cpu_gpr[rt], mask);
> -tcg_gen_movi_i64(t1, 0);
> -tcg_gen_or_i64(t1, t0, t1);
> -tcg_gen_shli_i64(t0, t0, 16);
> -tcg_gen_or_i64(t1, t0, t1);
> -tcg_gen_shli_i64(t0, t0, 16);
> -tcg_gen_or_i64(t1, t0, t1);
> -tcg_gen_shli_i64(t0, t0, 16);
> -tcg_gen_or_i64(t1, t0, t1);
> -
> -tcg_gen_mov_i64(cpu_gpr[rd], t1);
> -
> -tcg_gen_andi_i64(t0, cpu_mmr[rt], mask);
> -tcg_gen_movi_i64(t1, 0);
> -tcg_gen_or_i64(t1, t0, t1);
> -tcg_gen_shli_i64(t0, t0, 16);
> -tcg_gen_or_i64(t1, t0, t1);
> -tcg_gen_shli_i64(t0, t0, 16);
> -tcg_gen_or_i64(t1, t0, t1);
> -tcg_gen_shli_i64(t0, t0, 16);
> -tcg_gen_or_i64(t1, t0, t1);
> -
> -tcg_gen_mov_i64(cpu_mmr[rd], t1);
> -
> -tcg_temp_free(t0);
> -tcg_temp_free(t1);
> +for (int i = 0; i < 4; i++) {
> +tcg_gen_deposit_i64(cpu_gpr[rd],
> +cpu_gpr[rd], cpu_gpr[rd], 8 * i, 8);
> +tcg_gen_deposit_i64(cpu_mmr[rd],
> +cpu_mmr[rd], cpu_mmr[rd], 8 * i, 8);
Oops sorry disregard this patch, wrong opcode.
[PATCH] target/mips/translate: Simplify PCPYH using deposit_i64()
Simplify the PCPYH (Parallel Copy Halfword) instruction by using
multiple calls to deposit_i64() which can be optimized by some
TCG backends.
Signed-off-by: Philippe Mathieu-Daudé
---
target/mips/translate.c | 36 ++--
1 file changed, 6 insertions(+), 30 deletions(-)
diff --git a/target/mips/translate.c b/target/mips/translate.c
index a5cf1742a8b..5b31aa44f30 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -24786,36 +24786,12 @@ static void gen_mmi_pcpyh(DisasContext *ctx)
tcg_gen_movi_i64(cpu_gpr[rd], 0);
tcg_gen_movi_i64(cpu_mmr[rd], 0);
} else {
-TCGv_i64 t0 = tcg_temp_new();
-TCGv_i64 t1 = tcg_temp_new();
-uint64_t mask = (1ULL << 16) - 1;
-
-tcg_gen_andi_i64(t0, cpu_gpr[rt], mask);
-tcg_gen_movi_i64(t1, 0);
-tcg_gen_or_i64(t1, t0, t1);
-tcg_gen_shli_i64(t0, t0, 16);
-tcg_gen_or_i64(t1, t0, t1);
-tcg_gen_shli_i64(t0, t0, 16);
-tcg_gen_or_i64(t1, t0, t1);
-tcg_gen_shli_i64(t0, t0, 16);
-tcg_gen_or_i64(t1, t0, t1);
-
-tcg_gen_mov_i64(cpu_gpr[rd], t1);
-
-tcg_gen_andi_i64(t0, cpu_mmr[rt], mask);
-tcg_gen_movi_i64(t1, 0);
-tcg_gen_or_i64(t1, t0, t1);
-tcg_gen_shli_i64(t0, t0, 16);
-tcg_gen_or_i64(t1, t0, t1);
-tcg_gen_shli_i64(t0, t0, 16);
-tcg_gen_or_i64(t1, t0, t1);
-tcg_gen_shli_i64(t0, t0, 16);
-tcg_gen_or_i64(t1, t0, t1);
-
-tcg_gen_mov_i64(cpu_mmr[rd], t1);
-
-tcg_temp_free(t0);
-tcg_temp_free(t1);
+for (int i = 0; i < 4; i++) {
+tcg_gen_deposit_i64(cpu_gpr[rd],
+cpu_gpr[rd], cpu_gpr[rd], 8 * i, 8);
+tcg_gen_deposit_i64(cpu_mmr[rd],
+cpu_mmr[rd], cpu_mmr[rd], 8 * i, 8);
+}
}
}
--
2.26.2
[PULL 3/5] qemu-iotests: 300: Add test case for modifying persistence of bitmap
From: Peter Krempa
Verify that the modification of the bitmap persistence over migration
which is controlled via BitmapMigrationBitmapAliasTransform works
properly.
Based on TestCrossAliasMigration
Signed-off-by: Peter Krempa
Message-Id:
Reviewed-by: Eric Blake
[eblake: Adjust test for explicit read_zeroes=False]
Signed-off-by: Eric Blake
---
tests/qemu-iotests/300 | 93 ++
tests/qemu-iotests/300.out | 4 +-
2 files changed, 95 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
index 43264d883d79..63036f6a6e13 100755
--- a/tests/qemu-iotests/300
+++ b/tests/qemu-iotests/300
@@ -600,6 +600,99 @@ class TestCrossAliasMigration(TestDirtyBitmapMigration):
self.verify_dest_has_all_bitmaps()
self.verify_dest_error(None)
+class TestAliasTransformMigration(TestDirtyBitmapMigration):
+"""
+Tests the 'transform' option which modifies bitmap persistence on
migration.
+"""
+
+src_node_name = 'node-a'
+dst_node_name = 'node-b'
+src_bmap_name = 'bmap-a'
+dst_bmap_name = 'bmap-b'
+
+def setUp(self) -> None:
+TestDirtyBitmapMigration.setUp(self)
+
+# Now create another block device and let both have two bitmaps each
+result = self.vm_a.qmp('blockdev-add',
+ node_name='node-b', driver='null-co',
+ read_zeroes=False)
+self.assert_qmp(result, 'return', {})
+
+result = self.vm_b.qmp('blockdev-add',
+ node_name='node-a', driver='null-co',
+ read_zeroes=False)
+self.assert_qmp(result, 'return', {})
+
+bmaps_to_add = (('node-a', 'bmap-b'),
+('node-b', 'bmap-a'),
+('node-b', 'bmap-b'))
+
+for (node, bmap) in bmaps_to_add:
+result = self.vm_a.qmp('block-dirty-bitmap-add',
+ node=node, name=bmap)
+self.assert_qmp(result, 'return', {})
+
+@staticmethod
+def transform_mapping() -> BlockBitmapMapping:
+return [
+{
+'node-name': 'node-a',
+'alias': 'node-a',
+'bitmaps': [
+{
+'name': 'bmap-a',
+'alias': 'bmap-a',
+'transform':
+{
+'persistent': True
+}
+},
+{
+'name': 'bmap-b',
+'alias': 'bmap-b'
+}
+]
+},
+{
+'node-name': 'node-b',
+'alias': 'node-b',
+'bitmaps': [
+{
+'name': 'bmap-a',
+'alias': 'bmap-a'
+},
+{
+'name': 'bmap-b',
+'alias': 'bmap-b'
+}
+]
+}
+]
+
+def verify_dest_bitmap_state(self) -> None:
+bitmaps = self.vm_b.query_bitmaps()
+
+for node in bitmaps:
+bitmaps[node] = sorted(((bmap['name'], bmap['persistent']) for
bmap in bitmaps[node]))
+
+self.assertEqual(bitmaps,
+ {'node-a': [('bmap-a', True), ('bmap-b', False)],
+ 'node-b': [('bmap-a', False), ('bmap-b', False)]})
+
+def test_transform_on_src(self) -> None:
+self.set_mapping(self.vm_a, self.transform_mapping())
+
+self.migrate()
+self.verify_dest_bitmap_state()
+self.verify_dest_error(None)
+
+def test_transform_on_dst(self) -> None:
+self.set_mapping(self.vm_b, self.transform_mapping())
+
+self.migrate()
+self.verify_dest_bitmap_state()
+self.verify_dest_error(None)
if __name__ == '__main__':
iotests.main(supported_protocols=['file'])
diff --git a/tests/qemu-iotests/300.out b/tests/qemu-iotests/300.out
index cafb8161f7b1..12e9ab7d571b 100644
--- a/tests/qemu-iotests/300.out
+++ b/tests/qemu-iotests/300.out
@@ -1,5 +1,5 @@
-.
+...
--
-Ran 37 tests
+Ran 39 tests
OK
--
2.30.1
[PULL 5/5] block: use return status of bdrv_append()
From: Vladimir Sementsov-Ogievskiy
Now bdrv_append returns status and we can drop all the local_err things
around it.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Alberto Garcia
Message-Id: <20210202124956.63146-3-vsement...@virtuozzo.com>
Signed-off-by: Eric Blake
---
block.c | 6 ++
block/backup-top.c | 23 +++
block/commit.c | 6 ++
block/mirror.c | 6 ++
blockdev.c | 6 +++---
tests/test-bdrv-graph-mod.c | 6 +++---
6 files changed, 23 insertions(+), 30 deletions(-)
diff --git a/block.c b/block.c
index fdb0261cb324..c682c3e3b96b 100644
--- a/block.c
+++ b/block.c
@@ -3118,7 +3118,6 @@ static BlockDriverState
*bdrv_append_temp_snapshot(BlockDriverState *bs,
int64_t total_size;
QemuOpts *opts = NULL;
BlockDriverState *bs_snapshot = NULL;
-Error *local_err = NULL;
int ret;
/* if snapshot, we create a temporary backing file and open it
@@ -3165,9 +3164,8 @@ static BlockDriverState
*bdrv_append_temp_snapshot(BlockDriverState *bs,
* order to be able to return one, we have to increase
* bs_snapshot's refcount here */
bdrv_ref(bs_snapshot);
-bdrv_append(bs_snapshot, bs, _err);
-if (local_err) {
-error_propagate(errp, local_err);
+ret = bdrv_append(bs_snapshot, bs, errp);
+if (ret < 0) {
bs_snapshot = NULL;
goto out;
}
diff --git a/block/backup-top.c b/block/backup-top.c
index 6e7e7bf34047..d1253e1aa63f 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -191,7 +191,8 @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState
*source,
BlockCopyState **bcs,
Error **errp)
{
-Error *local_err = NULL;
+ERRP_GUARD();
+int ret;
BDRVBackupTopState *state;
BlockDriverState *top;
bool appended = false;
@@ -224,9 +225,9 @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState
*source,
bdrv_drained_begin(source);
bdrv_ref(top);
-bdrv_append(top, source, _err);
-if (local_err) {
-error_prepend(_err, "Cannot append backup-top filter: ");
+ret = bdrv_append(top, source, errp);
+if (ret < 0) {
+error_prepend(errp, "Cannot append backup-top filter: ");
goto fail;
}
appended = true;
@@ -236,19 +237,18 @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState
*source,
* we want.
*/
state->active = true;
-bdrv_child_refresh_perms(top, top->backing, _err);
-if (local_err) {
-error_prepend(_err,
- "Cannot set permissions for backup-top filter: ");
+ret = bdrv_child_refresh_perms(top, top->backing, errp);
+if (ret < 0) {
+error_prepend(errp, "Cannot set permissions for backup-top filter: ");
goto fail;
}
state->cluster_size = cluster_size;
state->bcs = block_copy_state_new(top->backing, state->target,
cluster_size, perf->use_copy_range,
- write_flags, _err);
-if (local_err) {
-error_prepend(_err, "Cannot create block-copy-state: ");
+ write_flags, errp);
+if (!state->bcs) {
+error_prepend(errp, "Cannot create block-copy-state: ");
goto fail;
}
*bcs = state->bcs;
@@ -266,7 +266,6 @@ fail:
}
bdrv_drained_end(source);
-error_propagate(errp, local_err);
return NULL;
}
diff --git a/block/commit.c b/block/commit.c
index 71db7ba7472e..dd9ba87349e3 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -254,7 +254,6 @@ void commit_start(const char *job_id, BlockDriverState *bs,
BlockDriverState *iter;
BlockDriverState *commit_top_bs = NULL;
BlockDriverState *filtered_base;
-Error *local_err = NULL;
int64_t base_size, top_size;
uint64_t base_perms, iter_shared_perms;
int ret;
@@ -312,10 +311,9 @@ void commit_start(const char *job_id, BlockDriverState *bs,
commit_top_bs->total_sectors = top->total_sectors;
-bdrv_append(commit_top_bs, top, _err);
-if (local_err) {
+ret = bdrv_append(commit_top_bs, top, errp);
+if (ret < 0) {
commit_top_bs = NULL;
-error_propagate(errp, local_err);
goto fail;
}
diff --git a/block/mirror.c b/block/mirror.c
index 8e1ad6eceb57..fad270193888 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1560,7 +1560,6 @@ static BlockJob *mirror_start_job(
BlockDriverState *mirror_top_bs;
bool target_is_backing;
uint64_t target_perms, target_shared_perms;
-Error *local_err = NULL;
int ret;
if (granularity == 0) {
@@ -1609,12 +1608,11 @@ static BlockJob *mirror_start_job(
* it alive until block_job_create() succeeds even if bs has no parent. */
bdrv_ref(mirror_top_bs);
bdrv_drained_begin(bs);
-
[PULL 1/5] migration: dirty-bitmap: Use struct for alias map inner members
From: Peter Krempa
Currently the alias mapping hash stores just strings of the target
objects internally. In further patches we'll be adding another member
which will need to be stored in the map so pass a copy of the whole
BitmapMigrationBitmapAlias QAPI struct into the map.
Signed-off-by: Peter Krempa
Message-Id:
Reviewed-by: Eric Blake
[eblake: adjust long lines]
Signed-off-by: Eric Blake
---
migration/block-dirty-bitmap.c | 33 +
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index c61d382be87c..b39c13ce4ebe 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -75,6 +75,8 @@
#include "qemu/id.h"
#include "qapi/error.h"
#include "qapi/qapi-commands-migration.h"
+#include "qapi/qapi-visit-migration.h"
+#include "qapi/clone-visitor.h"
#include "trace.h"
#define CHUNK_SIZE (1 << 10)
@@ -224,6 +226,7 @@ static GHashTable *construct_alias_map(const
BitmapMigrationNodeAliasList *bbm,
AliasMapInnerNode *amin;
GHashTable *bitmaps_map;
const char *node_map_from, *node_map_to;
+GDestroyNotify gdn;
if (!id_wellformed(bmna->alias)) {
error_setg(errp, "The node alias '%s' is not well-formed",
@@ -263,8 +266,9 @@ static GHashTable *construct_alias_map(const
BitmapMigrationNodeAliasList *bbm,
node_map_to = bmna->node_name;
}
-bitmaps_map = g_hash_table_new_full(g_str_hash, g_str_equal,
-g_free, g_free);
+gdn = (GDestroyNotify) qapi_free_BitmapMigrationBitmapAlias;
+bitmaps_map = g_hash_table_new_full(g_str_hash, g_str_equal, g_free,
+gdn);
amin = g_new(AliasMapInnerNode, 1);
*amin = (AliasMapInnerNode){
@@ -276,7 +280,7 @@ static GHashTable *construct_alias_map(const
BitmapMigrationNodeAliasList *bbm,
for (bmbal = bmna->bitmaps; bmbal; bmbal = bmbal->next) {
const BitmapMigrationBitmapAlias *bmba = bmbal->value;
-const char *bmap_map_from, *bmap_map_to;
+const char *bmap_map_from;
if (strlen(bmba->alias) > UINT8_MAX) {
error_setg(errp,
@@ -293,7 +297,6 @@ static GHashTable *construct_alias_map(const
BitmapMigrationNodeAliasList *bbm,
if (name_to_alias) {
bmap_map_from = bmba->name;
-bmap_map_to = bmba->alias;
if (g_hash_table_contains(bitmaps_map, bmba->name)) {
error_setg(errp, "The bitmap '%s'/'%s' is mapped twice",
@@ -302,7 +305,6 @@ static GHashTable *construct_alias_map(const
BitmapMigrationNodeAliasList *bbm,
}
} else {
bmap_map_from = bmba->alias;
-bmap_map_to = bmba->name;
if (g_hash_table_contains(bitmaps_map, bmba->alias)) {
error_setg(errp, "The bitmap alias '%s'/'%s' is used
twice",
@@ -311,8 +313,8 @@ static GHashTable *construct_alias_map(const
BitmapMigrationNodeAliasList *bbm,
}
}
-g_hash_table_insert(bitmaps_map,
-g_strdup(bmap_map_from),
g_strdup(bmap_map_to));
+g_hash_table_insert(bitmaps_map, g_strdup(bmap_map_from),
+QAPI_CLONE(BitmapMigrationBitmapAlias, bmba));
}
}
@@ -538,11 +540,15 @@ static int add_bitmaps_to_list(DBMSaveState *s,
BlockDriverState *bs,
}
if (bitmap_aliases) {
-bitmap_alias = g_hash_table_lookup(bitmap_aliases, bitmap_name);
-if (!bitmap_alias) {
+BitmapMigrationBitmapAlias *bmap_inner;
+
+bmap_inner = g_hash_table_lookup(bitmap_aliases, bitmap_name);
+if (!bmap_inner) {
/* Skip bitmaps with no alias */
continue;
}
+
+bitmap_alias = bmap_inner->alias;
} else {
if (strlen(bitmap_name) > UINT8_MAX) {
error_report("Cannot migrate bitmap '%s' on node '%s': "
@@ -1074,13 +1080,16 @@ static int dirty_bitmap_load_header(QEMUFile *f,
DBMLoadState *s,
bitmap_name = s->bitmap_alias;
if (!s->cancelled && bitmap_alias_map) {
-bitmap_name = g_hash_table_lookup(bitmap_alias_map,
- s->bitmap_alias);
-if (!bitmap_name) {
+BitmapMigrationBitmapAlias *bmap_inner;
+
+bmap_inner = g_hash_table_lookup(bitmap_alias_map,
s->bitmap_alias);
+if (!bmap_inner) {
error_report("Error: Unknown bitmap alias '%s' on node "
"'%s' (alias '%s')", s->bitmap_alias,
s->bs->node_name, s->node_alias);
cancel_incoming_locked(s);
+} else {
+
[PULL 2/5] migration: dirty-bitmap: Allow control of bitmap persistence
From: Peter Krempa
Bitmap's source persistence is transported over the migration stream and
the destination mirrors it. In some cases the destination might want to
persist bitmaps which are not persistent on the source (e.g. the result
of merging bitmaps from a number of layers on the source when migrating
into a squashed image) but currently it would need to create another set
of persistent bitmaps and merge them.
This patch adds a 'transform' property to the alias map which allows
overriding the persistence of migrated bitmaps both on the source and
destination sides.
Signed-off-by: Peter Krempa
Message-Id:
Reviewed-by: Eric Blake
[eblake: grammar tweaks, drop dead conditional]
Signed-off-by: Eric Blake
---
qapi/migration.json| 19 ++-
migration/block-dirty-bitmap.c | 29 ++---
2 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/qapi/migration.json b/qapi/migration.json
index ce14d78071a5..6e5943fbb443 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -536,6 +536,19 @@
'data': [ 'none', 'zlib',
{ 'name': 'zstd', 'if': 'defined(CONFIG_ZSTD)' } ] }
+##
+# @BitmapMigrationBitmapAliasTransform:
+#
+# @persistent: If present, the bitmap will be made persistent
+# or transient depending on this parameter.
+#
+# Since: 6.0
+##
+{ 'struct': 'BitmapMigrationBitmapAliasTransform',
+ 'data': {
+ '*persistent': 'bool'
+ } }
+
##
# @BitmapMigrationBitmapAlias:
#
@@ -544,12 +557,16 @@
# @alias: An alias name for migration (for example the bitmap name on
# the opposite site).
#
+# @transform: Allows the modification of the migrated bitmap.
+# (since 6.0)
+#
# Since: 5.2
##
{ 'struct': 'BitmapMigrationBitmapAlias',
'data': {
'name': 'str',
- 'alias': 'str'
+ 'alias': 'str',
+ '*transform': 'BitmapMigrationBitmapAliasTransform'
} }
##
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index b39c13ce4ebe..975093610a41 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -150,6 +150,7 @@ typedef struct DBMLoadState {
BdrvDirtyBitmap *bitmap;
bool before_vm_start_handled; /* set in dirty_bitmap_mig_before_vm_start */
+BitmapMigrationBitmapAlias *bmap_inner;
/*
* cancelled
@@ -529,6 +530,7 @@ static int add_bitmaps_to_list(DBMSaveState *s,
BlockDriverState *bs,
}
FOR_EACH_DIRTY_BITMAP(bs, bitmap) {
+BitmapMigrationBitmapAliasTransform *bitmap_transform = NULL;
bitmap_name = bdrv_dirty_bitmap_name(bitmap);
if (!bitmap_name) {
continue;
@@ -549,6 +551,9 @@ static int add_bitmaps_to_list(DBMSaveState *s,
BlockDriverState *bs,
}
bitmap_alias = bmap_inner->alias;
+if (bmap_inner->has_transform) {
+bitmap_transform = bmap_inner->transform;
+}
} else {
if (strlen(bitmap_name) > UINT8_MAX) {
error_report("Cannot migrate bitmap '%s' on node '%s': "
@@ -574,8 +579,15 @@ static int add_bitmaps_to_list(DBMSaveState *s,
BlockDriverState *bs,
if (bdrv_dirty_bitmap_enabled(bitmap)) {
dbms->flags |= DIRTY_BITMAP_MIG_START_FLAG_ENABLED;
}
-if (bdrv_dirty_bitmap_get_persistence(bitmap)) {
-dbms->flags |= DIRTY_BITMAP_MIG_START_FLAG_PERSISTENT;
+if (bitmap_transform &&
+bitmap_transform->has_persistent) {
+if (bitmap_transform->persistent) {
+dbms->flags |= DIRTY_BITMAP_MIG_START_FLAG_PERSISTENT;
+}
+} else {
+if (bdrv_dirty_bitmap_get_persistence(bitmap)) {
+dbms->flags |= DIRTY_BITMAP_MIG_START_FLAG_PERSISTENT;
+}
}
QSIMPLEQ_INSERT_TAIL(>dbms_list, dbms, entry);
@@ -783,6 +795,7 @@ static int dirty_bitmap_load_start(QEMUFile *f,
DBMLoadState *s)
uint32_t granularity = qemu_get_be32(f);
uint8_t flags = qemu_get_byte(f);
LoadBitmapState *b;
+bool persistent;
if (s->cancelled) {
return 0;
@@ -807,7 +820,15 @@ static int dirty_bitmap_load_start(QEMUFile *f,
DBMLoadState *s)
return -EINVAL;
}
-if (flags & DIRTY_BITMAP_MIG_START_FLAG_PERSISTENT) {
+if (s->bmap_inner &&
+s->bmap_inner->has_transform &&
+s->bmap_inner->transform->has_persistent) {
+persistent = s->bmap_inner->transform->persistent;
+} else {
+persistent = flags & DIRTY_BITMAP_MIG_START_FLAG_PERSISTENT;
+}
+
+if (persistent) {
bdrv_dirty_bitmap_set_persistence(s->bitmap, true);
}
@@ -1091,6 +1112,8 @@ static int dirty_bitmap_load_header(QEMUFile *f,
DBMLoadState *s,
} else {
bitmap_name = bmap_inner->name;
}
+
+s->bmap_inner = bmap_inner;
}
if (!s->cancelled) {
--
2.30.1
[PULL 4/5] block: return status from bdrv_append and friends
From: Vladimir Sementsov-Ogievskiy
The recommended use of qemu error api assumes returning status together
with setting errp and avoid void functions with errp parameter. Let's
improve bdrv_append and some friends to reduce error-propagation
overhead in further patches.
Choose int return status, because bdrv_replace_node_common() has call
to bdrv_check_update_perm(), which reports int status, which seems
correct to propagate.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Message-Id: <20210202124956.63146-2-vsement...@virtuozzo.com>
Reviewed-by: Alberto Garcia
Signed-off-by: Eric Blake
---
include/block/block.h | 12 -
block.c | 58 +++
2 files changed, 43 insertions(+), 27 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index 0a9f2c187cdb..2c235a29e822 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -356,10 +356,10 @@ int bdrv_create(BlockDriver *drv, const char* filename,
int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp);
BlockDriverState *bdrv_new(void);
-void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
- Error **errp);
-void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
- Error **errp);
+int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
+Error **errp);
+int bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
+ Error **errp);
BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *node_options,
int flags, Error **errp);
@@ -373,8 +373,8 @@ BdrvChild *bdrv_open_child(const char *filename,
BdrvChildRole child_role,
bool allow_none, Error **errp);
BlockDriverState *bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp);
-void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
- Error **errp);
+int bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
+Error **errp);
int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
const char *bdref_key, Error **errp);
BlockDriverState *bdrv_open(const char *filename, const char *reference,
diff --git a/block.c b/block.c
index 4e52b1c588cd..fdb0261cb324 100644
--- a/block.c
+++ b/block.c
@@ -2827,14 +2827,15 @@ static BdrvChildRole bdrv_backing_role(BlockDriverState
*bs)
* Sets the bs->backing link of a BDS. A new reference is created; callers
* which don't need their own reference any more must call bdrv_unref().
*/
-void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
- Error **errp)
+int bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
+Error **errp)
{
+int ret = 0;
bool update_inherits_from = bdrv_chain_contains(bs, backing_hd) &&
bdrv_inherits_from_recursive(backing_hd, bs);
if (bdrv_is_backing_chain_frozen(bs, child_bs(bs->backing), errp)) {
-return;
+return -EPERM;
}
if (backing_hd) {
@@ -2853,15 +2854,22 @@ void bdrv_set_backing_hd(BlockDriverState *bs,
BlockDriverState *backing_hd,
bs->backing = bdrv_attach_child(bs, backing_hd, "backing", _of_bds,
bdrv_backing_role(bs), errp);
+if (!bs->backing) {
+ret = -EPERM;
+goto out;
+}
+
/* If backing_hd was already part of bs's backing chain, and
* inherits_from pointed recursively to bs then let's update it to
* point directly to bs (else it will become NULL). */
-if (bs->backing && update_inherits_from) {
+if (update_inherits_from) {
backing_hd->inherits_from = bs;
}
out:
bdrv_refresh_limits(bs, NULL);
+
+return ret;
}
/*
@@ -4532,9 +4540,9 @@ static bool should_update_child(BdrvChild *c,
BlockDriverState *to)
* With auto_skip=false the error is returned if from has a parent which should
* not be updated.
*/
-static void bdrv_replace_node_common(BlockDriverState *from,
- BlockDriverState *to,
- bool auto_skip, Error **errp)
+static int bdrv_replace_node_common(BlockDriverState *from,
+BlockDriverState *to,
+bool auto_skip, Error **errp)
{
BdrvChild *c, *next;
GSList *list = NULL, *p;
@@ -4556,11 +4564,13 @@ static void bdrv_replace_node_common(BlockDriverState
*from,
if (auto_skip) {
continue;
}
+ret = -EINVAL;
error_setg(errp, "Should not change '%s' link to '%s'",
c->name, from->node_name);
goto out;
}
if (c->frozen) {
+ret = -EPERM;
[PULL 0/5] bitmaps patches through 2021-02-12
The following changes since commit eac92d316351b855ba79eb374dd21cc367f1f9c1: Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210211-1' into staging (2021-02-11 19:57:50 +) are available in the Git repository at: https://repo.or.cz/qemu/ericb.git tags/pull-bitmaps-2021-02-12 for you to fetch changes up to 934aee14d36e67468260635af61c387227cdaf78: block: use return status of bdrv_append() (2021-02-12 15:39:44 -0600) bitmaps patches for 2021-02-12 - add 'transform' member to manipulate bitmaps across migration - work towards better error handling during bdrv_open Peter Krempa (3): migration: dirty-bitmap: Use struct for alias map inner members migration: dirty-bitmap: Allow control of bitmap persistence qemu-iotests: 300: Add test case for modifying persistence of bitmap Vladimir Sementsov-Ogievskiy (2): block: return status from bdrv_append and friends block: use return status of bdrv_append() qapi/migration.json| 19 - include/block/block.h | 12 +++--- block.c| 64 + block/backup-top.c | 23 +-- block/commit.c | 6 +-- block/mirror.c | 6 +-- blockdev.c | 6 +-- migration/block-dirty-bitmap.c | 62 +--- tests/test-bdrv-graph-mod.c| 6 +-- tests/qemu-iotests/300 | 93 ++ tests/qemu-iotests/300.out | 4 +- 11 files changed, 226 insertions(+), 75 deletions(-) -- 2.30.1
Re: [PATCH v7 03/14] block: check return value of bdrv_open_child and drop error propagation
On 2/2/21 6:49 AM, Vladimir Sementsov-Ogievskiy wrote:
> This patch is generated by cocci script:
>
> @@
> symbol bdrv_open_child, errp, local_err;
> expression file;
> @@
>
> file = bdrv_open_child(...,
> -_err
> +errp
> );
> - if (local_err)
> + if (!file)
> {
> ...
> - error_propagate(errp, local_err);
> ...
> }
>
> with command
>
> spatch --sp-file x.cocci --macro-file scripts/cocci-macro-file.h \
> --in-place --no-show-diff --max-width 80 --use-gitgrep block
With this patch applied, 'check unit-test' fails with:
Running test test-replication
Unexpected error in bdrv_open_driver() at ../block.c:1481:
Could not open '/tmp/p_local_disk.z1Ugyc': Invalid argument
ERROR test-replication - missing test plan
Directly reverting it has ripple effect on later patches in the series.
Running test-replication under gdb gives this backtrace:
Thread 1 "test-replicatio" received signal SIGABRT, Aborted.
0x76f6f9d5 in raise () from /lib64/libc.so.6
(gdb) bt
#0 0x76f6f9d5 in raise () from /lib64/libc.so.6
#1 0x76f588a4 in abort () from /lib64/libc.so.6
#2 0x556ad820 in error_handle_fatal (
errp=0x55790568 , err=0x55859010)
at ../util/error.c:40
#3 0x556ae3cf in error_propagate (
dst_errp=0x55790568 , local_err=0x55859010)
at ../util/error.c:286
#4 0x5558cc9e in bdrv_img_create (
filename=0x55822500 "/tmp/p_local_disk.DVFoWt",
fmt=0x556e809a "qcow2", base_filename=0x0, base_fmt=0x0,
options=0x0,
img_size=67108864, flags=2, quiet=true, errp=0x55790568
)
at ../block.c:6312
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> Reviewed-by: Greg Kurz
> Reviewed-by: Alberto Garcia
> ---
> block/blkdebug.c | 6 ++
> block/blklogwrites.c | 10 --
> block/blkreplay.c| 6 ++
> block/blkverify.c| 11 ---
> block/qcow2.c| 5 ++---
> block/quorum.c | 6 ++
> 6 files changed, 16 insertions(+), 28 deletions(-)
And this diffstat doesn't immediately tell me what ended up violating
the assumptions of error_abort. As such, at this point I'm temporarily
dropping the remainder of the series from my bitmaps queue, and only
including patches 1 and 2 in my next pull request. Looking forward to v8.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [RFC v2 2/2] Basic CXL DOE for CDAT and Compliance Mode
> On Feb 12, 2021, at 12:23 PM, Jonathan Cameron
> wrote:
>
> On Tue, 9 Feb 2021 15:36:03 -0500
> Chris Browy wrote:
>
> Split this into two patches for v3. CDAT in one, compliance mode in the
> other.
>
Compliance mode is an optional feature. We’ll split it out.
> I'd also move the actual elements out into the cxl components so that we
> can register only what makes sense for a given device. My guess
> is that for now that will be static const anyway.
>
> Coming together fine. Hopefully I'll start poking at the linux side of things
> next week. First job being simply providing a file to allow us to dump
> the whole CDAT table. Let me know if you get this loading an .aml file
> in the meantime as that'll make it easier to test (if not I'll hack it
> on top of these patches)
We can get the .aml loading by Thurs next week. Holiday next few days for
some of our folks.
>
> If needed I'll add it to iASL as well (may well be already in hand!)
>
> I think my version of this stuff did a useful job in improving my
> understanding
> of what we were trying to do, but that done I'm assuming we'll just abandon it
> as the disposable prototype it was :)
>
Thanks for focusing in on the area and uncovering problems with both our
versions!
Still lots of pieces need to come together and get working to be able to fully
enumerate
and configure the device!
> Jonathan
>
>
>> ---
>> hw/cxl/cxl-component-utils.c | 132 +++
>> hw/mem/cxl_type3.c | 172
>> include/hw/cxl/cxl_cdat.h | 120 +
>> include/hw/cxl/cxl_compl.h | 289
>> +
>> include/hw/cxl/cxl_component.h | 126 ++
>> include/hw/cxl/cxl_device.h| 3 +
>> include/hw/cxl/cxl_pci.h | 4 +
>> 7 files changed, 846 insertions(+)
>> create mode 100644 include/hw/cxl/cxl_cdat.h
>> create mode 100644 include/hw/cxl/cxl_compl.h
>>
>> diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
>> index e1bcee5..fc6c538 100644
>> --- a/hw/cxl/cxl-component-utils.c
>> +++ b/hw/cxl/cxl-component-utils.c
>> @@ -195,3 +195,135 @@ void cxl_component_create_dvsec(CXLComponentState
>> *cxl, uint16_t length,
>> range_init_nofail(>dvsecs[type], cxl->dvsec_offset, length);
>> cxl->dvsec_offset += length;
>> }
>> +
>> +/* Return the sum of bytes */
>> +static void cdat_ent_init(CDATStruct *cs, void *base, uint32_t len)
>> +{
>> +cs->base = base;
>> +cs->length = len;
>> +}
>> +
>> +void cxl_doe_cdat_init(CXLComponentState *cxl_cstate)
>> +{
>> +uint8_t sum = 0;
>> +uint32_t len = 0;
>> +int i, j;
>> +
>> +cxl_cstate->cdat_ent_len = 7;
>> +cxl_cstate->cdat_ent =
>> +g_malloc0(sizeof(CDATStruct) * cxl_cstate->cdat_ent_len);
>> +
>> +cdat_ent_init(_cstate->cdat_ent[0],
>> + _cstate->cdat_header,
>> sizeof(cxl_cstate->cdat_header));
>> +cdat_ent_init(_cstate->cdat_ent[1],
>> + _cstate->dsmas, sizeof(cxl_cstate->dsmas));
>> +cdat_ent_init(_cstate->cdat_ent[2],
>> + _cstate->dslbis, sizeof(cxl_cstate->dslbis));
>> +cdat_ent_init(_cstate->cdat_ent[3],
>> + _cstate->dsmscis, sizeof(cxl_cstate->dsmscis));
>> +cdat_ent_init(_cstate->cdat_ent[4],
>> + _cstate->dsis, sizeof(cxl_cstate->dsis));
>> +cdat_ent_init(_cstate->cdat_ent[5],
>> + _cstate->dsemts, sizeof(cxl_cstate->dsemts));
>> +cdat_ent_init(_cstate->cdat_ent[6],
>> + _cstate->sslbis, sizeof(cxl_cstate->sslbis));
>> +
>> +/* Set the DSMAS entry, ent = 1 */
>> +cxl_cstate->dsmas.header.type = CDAT_TYPE_DSMAS;
>> +cxl_cstate->dsmas.header.reserved = 0x0;
>> +cxl_cstate->dsmas.header.length = sizeof(cxl_cstate->dsmas);
>> +cxl_cstate->dsmas.DSMADhandle = 0x0;
>> +cxl_cstate->dsmas.flags = 0x0;
>> +cxl_cstate->dsmas.reserved2 = 0x0;
>> +cxl_cstate->dsmas.DPA_base = 0x0;
>> +cxl_cstate->dsmas.DPA_length = 0x4;
>
> Look to move the instances of these down into the memory device and expose
> cdat_ent_init() to there.
>
> That way, we can add whatever elements make sense for each type
> of component.
>
> Also have a cdat_ents_finalize() or similar to call at the end
> which calculates overall length + checksum.
OK
>
> Should also be easy enough to add a simple bit of code to call
> cdat_ent_init() for each element of a passed in CDAT.aml file.
>
We’ll address all the above when we add the CDAT.aml file support
which may only pass a subset of structures.
>> +
>> +/* Set the DSLBIS entry, ent = 2 */
>> +cxl_cstate->dslbis.header.type = CDAT_TYPE_DSLBIS;
>> +cxl_cstate->dslbis.header.reserved = 0;
>> +cxl_cstate->dslbis.header.length = sizeof(cxl_cstate->dslbis);
>> +cxl_cstate->dslbis.handle = 0;
>> +cxl_cstate->dslbis.flags = 0;
>> +cxl_cstate->dslbis.data_type = 0;
>> +
Re: [PATCH v7 13/14] block/qed: bdrv_qed_do_open: deal with errp
On 2/2/21 6:49 AM, Vladimir Sementsov-Ogievskiy wrote: > Set errp always on failure. Generic bdrv_open_driver supports driver > functions which can return negative value and forget to set errp. > That's a strange thing.. Let's improve bdrv_qed_do_open to not behave > this way. This allows to simplify code in > bdrv_qed_co_invalidate_cache(). Grammar tweak: Always set errp on failure. Generic bdrv_open_driver supports driver functions which can return a negative value but forget to set errp. That's a strange thing. Let's improve bdrv_qed_do_open to not behave this way. This allows the simplification of code in bdrv_qed_co_invalidate_cache(). > > Signed-off-by: Vladimir Sementsov-Ogievskiy > Reviewed-by: Alberto Garcia > Reviewed-by: Greg Kurz > --- > block/qed.c | 24 +++- > 1 file changed, 15 insertions(+), 9 deletions(-) > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [RFC PATCH 12/15] sd: emmc: Support boot area in emmc image
On Thu, Feb 11, 2021 at 12:15 AM Sai Pavan Boddu
wrote:
>
> From: Joel Stanley
>
> This assumes a specially constructued image:
>
> dd if=/dev/zero of=mmc-bootarea.img count=2 bs=1M
> dd if=u-boot-spl.bin of=mmc-bootarea.img conv=notrunc
> dd if=u-boot.bin of=mmc-bootarea.img conv=notrunc count=64 bs=1K
> cat mmc-bootarea.img obmc-phosphor-image.wic > mmc.img
> truncate --size 16GB mmc.img
> truncate --size 128MB mmc-bootarea.img
Could we document this somewhere user accessible?
Alistair
>
> Signed-off-by: Joel Stanley
> [clg: - changes on the definition names ]
> Signed-off-by: Cédric Le Goater
> [spb: use data_start property to access right emmc partition,
> Clean up PARTITION_ENABLE support as incomplete,
> Fix commit message to be generic.]
> Signed-off-by: Sai Pavan Boddu
> ---
> hw/sd/sd.c | 40
> 1 file changed, 40 insertions(+)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 54fba7b..55c1104 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1045,6 +1045,34 @@ static void sd_lock_command(SDState *sd)
> sd->card_status &= ~CARD_IS_LOCKED;
> }
>
> +/*
> + * This requires a disk image that has two boot partitions inserted at the
> + * beginning of it. The size of the boot partitions are configured in the
> + * ext_csd structure, which is hardcoded in qemu. They are currently set to
> + * 1MB each.
> + */
> +static uint32_t sd_bootpart_offset(SDState *sd)
> +{
> +unsigned int access = sd->ext_csd[EXT_CSD_PART_CONFIG] &
> +EXT_CSD_PART_CONFIG_ACC_MASK;
> +unsigned int boot_capacity = sd->ext_csd[EXT_CSD_BOOT_MULT] << 17;
> +
> +if (!sd->emmc) {
> +return 0;
> +}
> +
> +switch (access) {
> +case EXT_CSD_PART_CONFIG_ACC_DEFAULT:
> +return boot_capacity * 2;
> +case EXT_CSD_PART_CONFIG_ACC_BOOT0:
> +return 0;
> +case EXT_CSD_PART_CONFIG_ACC_BOOT0 + 1:
> +return boot_capacity * 1;
> +default:
> + g_assert_not_reached();
> +}
> +}
> +
> static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
> {
> uint32_t rca = 0x;
> @@ -1360,6 +1388,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
> SDRequest req)
> return sd_r1;
> }
>
> +if (sd->emmc) {
> +addr += sd_bootpart_offset(sd);
> +}
> sd->state = sd_sendingdata_state;
> sd->data_start = addr;
> sd->data_offset = 0;
> @@ -1379,6 +1410,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
> SDRequest req)
> return sd_r1;
> }
>
> +if (sd->emmc) {
> +addr += sd_bootpart_offset(sd);
> +}
> sd->state = sd_sendingdata_state;
> sd->data_start = addr;
> sd->data_offset = 0;
> @@ -1435,6 +1469,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
> SDRequest req)
> return sd_r1;
> }
>
> +if (sd->emmc) {
> +addr += sd_bootpart_offset(sd);
> +}
> sd->state = sd_receivingdata_state;
> sd->data_start = addr;
> sd->data_offset = 0;
> @@ -1465,6 +1502,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
> SDRequest req)
> return sd_r1;
> }
>
> +if (sd->emmc) {
> +addr += sd_bootpart_offset(sd);
> +}
> sd->state = sd_receivingdata_state;
> sd->data_start = addr;
> sd->data_offset = 0;
> --
> 2.7.4
>
>
Re: [RFC PATCH 14/15] sd: sdhci: Support eMMC devices
On Thu, Feb 11, 2021 at 12:34 AM Sai Pavan Boddu
wrote:
>
> Embedded device slots should be allowed as support of eMMC is available.
>
> Signed-off-by: Sai Pavan Boddu
Reviewed-by: Alistair Francis
Alistair
> ---
> hw/sd/sdhci.c | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 8ffa539..771212a 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -99,10 +99,6 @@ static void sdhci_check_capareg(SDHCIState *s, Error
> **errp)
> msk = FIELD_DP64(msk, SDHC_CAPAB, ASYNC_INT, 0);
>
> val = FIELD_EX64(s->capareg, SDHC_CAPAB, SLOT_TYPE);
> -if (val) {
> -error_setg(errp, "slot-type not supported");
> -return;
> -}
> trace_sdhci_capareg("slot type", val);
> msk = FIELD_DP64(msk, SDHC_CAPAB, SLOT_TYPE, 0);
>
> --
> 2.7.4
>
>
Re: [RFC PATCH 13/15] sd: emmc: Subtract bootarea size from blk
On Thu, Feb 11, 2021 at 12:25 AM Sai Pavan Boddu
wrote:
>
> From: Joel Stanley
>
> The userdata size is derived from the file the user passes on the
> command line, but we must take into account the boot areas.
>
> Signed-off-by: Joel Stanley
> Signed-off-by: Cédric Le Goater
Acked-by: Alistair Francis
Alistair
> ---
> hw/sd/sd.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 55c1104..a2f39c9 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -658,6 +658,11 @@ static void sd_reset(DeviceState *dev)
> }
> size = sect << 9;
>
> +if (sd->emmc) {
> +unsigned int boot_capacity = sd->ext_csd[EXT_CSD_BOOT_MULT] << 17;
> +size -= boot_capacity * 2;
> +}
> +
> sect = sd_addr_to_wpnum(size) + 1;
>
> sd->state = sd_idle_state;
> --
> 2.7.4
>
>
Re: [RFC PATCH 10/15] sd: emmc: Update CID structure for eMMC
On Thu, Feb 11, 2021 at 12:30 AM Sai Pavan Boddu
wrote:
>
> CID structure is little different for eMMC, w.r.t to product name and
> manufacturing date.
>
> Signed-off-by: Sai Pavan Boddu
> Signed-off-by: Edgar E. Iglesias
> ---
> hw/sd/sd.c | 52 +++-
> 1 file changed, 35 insertions(+), 17 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 7aab647..45311fa 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -345,23 +345,41 @@ static void sd_set_scr(SDState *sd)
>
> static void sd_set_cid(SDState *sd)
> {
> -sd->cid[0] = MID; /* Fake card manufacturer ID (MID) */
> -sd->cid[1] = OID[0]; /* OEM/Application ID (OID) */
> -sd->cid[2] = OID[1];
> -sd->cid[3] = PNM[0]; /* Fake product name (PNM) */
> -sd->cid[4] = PNM[1];
> -sd->cid[5] = PNM[2];
> -sd->cid[6] = PNM[3];
> -sd->cid[7] = PNM[4];
> -sd->cid[8] = PRV; /* Fake product revision (PRV) */
> -sd->cid[9] = 0xde; /* Fake serial number (PSN) */
> -sd->cid[10] = 0xad;
> -sd->cid[11] = 0xbe;
> -sd->cid[12] = 0xef;
> -sd->cid[13] = 0x00 | /* Manufacture date (MDT) */
> -((MDT_YR - 2000) / 10);
> -sd->cid[14] = ((MDT_YR % 10) << 4) | MDT_MON;
> -sd->cid[15] = (sd_crc7(sd->cid, 15) << 1) | 1;
> +if (sd->emmc) {
> +sd->cid[0] = MID;
> +sd->cid[1] = 0x1; /* CBX */
> +sd->cid[2] = OID[0];/* OEM/Application ID (OID) */
> +sd->cid[3] = PNM[0];/* Fake product name (PNM) 48bit */
> +sd->cid[4] = PNM[1];
> +sd->cid[5] = PNM[2];
> +sd->cid[6] = PNM[3];
> +sd->cid[7] = PNM[4];
Aren't the majority of these the same between the two cases? It's
probably cleaner to split them out then.
Alistair
> +sd->cid[8] = 0x0;
> +sd->cid[9] = PRV;/* Fake product revision (PRV) */
> +sd->cid[10] = 0xde; /* Fake serial number (PSN) */
> +sd->cid[11] = 0xad;
> +sd->cid[12] = 0xbe;
> +sd->cid[13] = 0xef;
> +sd->cid[14] = ((MDT_YR - 1997) % 0x10); /* MDT */
> +} else {
> +sd->cid[0] = MID; /* Fake card manufacturer ID (MID) */
> +sd->cid[1] = OID[0];/* OEM/Application ID (OID) */
> +sd->cid[2] = OID[1];
> +sd->cid[3] = PNM[0];/* Fake product name (PNM) 40bit */
> +sd->cid[4] = PNM[1];
> +sd->cid[5] = PNM[2];
> +sd->cid[6] = PNM[3];
> +sd->cid[7] = PNM[4];
> +sd->cid[8] = PRV; /* Fake product revision (PRV) */
> +sd->cid[9] = 0xde; /* Fake serial number (PSN) */
> +sd->cid[10] = 0xad;
> +sd->cid[11] = 0xbe;
> +sd->cid[12] = 0xef;
> +sd->cid[13] = 0x00 |/* Manufacture date (MDT) */
> +((MDT_YR - 2000) / 10);
> +sd->cid[14] = ((MDT_YR % 10) << 4) | MDT_MON;
> + }
> + sd->cid[15] = (sd_crc7(sd->cid, 15) << 1) | 1;
> }
>
> #define HWBLOCK_SHIFT 9 /* 512 bytes */
> --
> 2.7.4
>
>
Re: [RFC PATCH 09/15] sd: emmc: Add support for emmc erase
On Thu, Feb 11, 2021 at 12:19 AM Sai Pavan Boddu
wrote:
>
> Add CMD35 and CMD36 which sets the erase start and end.
>
> Signed-off-by: Sai Pavan Boddu
> Signed-off-by: Edgar E. Iglesias
> ---
> hw/sd/sd.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 236f2b8..7aab647 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1544,6 +1544,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
> SDRequest req)
>
> /* Erase commands (Class 5) */
> case 32: /* CMD32: ERASE_WR_BLK_START */
> +case 35:
Can you comment the CMD here?
> switch (sd->state) {
> case sd_transfer_state:
> sd->erase_start = req.arg;
> @@ -1555,6 +1556,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
> SDRequest req)
> break;
>
> case 33: /* CMD33: ERASE_WR_BLK_END */
> +case 36:
and here?
Alistair
> switch (sd->state) {
> case sd_transfer_state:
> sd->erase_end = req.arg;
> --
> 2.7.4
>
>
Re: [RFC PATCH 05/15] sd: emmc: support idle state in CMD2
On Thu, Feb 11, 2021 at 12:18 AM Sai Pavan Boddu
wrote:
>
> eMMC is expected to be in idle-state post CMD1. Ready state is an
> intermediate stage which we don't come across in Device identification
> mode.
>
> Signed-off-by: Sai Pavan Boddu
> Signed-off-by: Edgar E. Iglesias
Acked-by: Alistair Francis
Alistair
> ---
> hw/sd/sd.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index e3738b2..69289e0 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1051,6 +1051,10 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
> SDRequest req)
> if (sd->spi)
> goto bad_cmd;
> switch (sd->state) {
> +case sd_idle_state:
> +if (!sd->emmc) {
> +break;
> +}
> case sd_ready_state:
> sd->state = sd_identification_state;
> return sd_r2_i;
> --
> 2.7.4
>
>
Re: [RFC PATCH 04/15] sd: emmc: Update CMD1 definition for eMMC
On Thu, Feb 11, 2021 at 12:17 AM Sai Pavan Boddu
wrote:
>
> Add support to Power up the card and send response r3 in case of eMMC.
>
> Signed-off-by: Sai Pavan Boddu
> Signed-off-by: Edgar E. Iglesias
Acked-by: Alistair Francis
Alistair
> ---
> hw/sd/sd.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 57fff89..e3738b2 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1033,8 +1033,16 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
> SDRequest req)
> break;
>
> case 1:/* CMD1: SEND_OP_CMD */
> -if (!sd->spi)
> +/* MMC: Powerup & send r3
> + * SD: send r1 in spi mode
> + */
> +if (sd->emmc) {
> +sd_ocr_powerup(sd);
> +return sd->state == sd_idle_state ?
> + sd_r3 : sd_r0;
> +} else if (!sd->spi) {
> goto bad_cmd;
> +}
>
> sd->state = sd_transfer_state;
> return sd_r1;
> --
> 2.7.4
>
>
Re: [PATCH 0/4] hw/riscv: Clean-ups and map high mmio for PCIe of 'virt' machine
On Fri, Jan 22, 2021 at 4:32 AM Bin Meng wrote: > > From: Bin Meng > > This series does the following clean-ups: > - Drop 'struct MemmapEntry' > - virt: Drop the 'link_up' parameter of gpex_pcie_init() > > It also adds the following small enhancement to 'virt' machine: > - Limit RAM size in a 32-bit system > - Map high mmio for PCIe > > > Bin Meng (4): > hw/riscv: Drop 'struct MemmapEntry' > hw/riscv: virt: Drop the 'link_up' parameter of gpex_pcie_init() > hw/riscv: virt: Limit RAM size in a 32-bit system > hw/riscv: virt: Map high mmio for PCIe Thanks! Applied to riscv-to-apply.next Alistair > > hw/riscv/microchip_pfsoc.c | 9 ++--- > hw/riscv/opentitan.c | 9 ++--- > hw/riscv/sifive_e.c| 9 ++--- > hw/riscv/sifive_u.c| 11 +++--- > hw/riscv/spike.c | 9 ++--- > hw/riscv/virt.c| 72 ++ > 6 files changed, 73 insertions(+), 46 deletions(-) > > -- > 2.25.1 > >
Re: [RFC PATCH v2 1/2] Basic PCIe DOE support
> On Feb 12, 2021, at 11:24 AM, Jonathan Cameron
> wrote:
>
> On Tue, 9 Feb 2021 15:35:49 -0500
> Chris Browy wrote:
>
> Run ./scripts/checkpatch.pl over the patches and fix all the warnings before
> posting. It will save time by clearing out most of the minor formatting
> issues
> and similar that inevitably sneak in during development.
>
Excellent suggestion. We’re still newbies!
> The biggest issue I'm seeing in here is that the abstraction of
> multiple DOE capabiltiies accessing same protocols doesn't make sense.
>
> Each DOE ecap region and hence mailbox can have it's own set of
> (possibly overlapping) protocols.
>
> From the ECN:
> "It is permitted for a protocol using data object exchanges to require
> that a Function implement a unique instance of DOE for that specific
> protocol, and/or to allow sharing of a DOE instance to only a specific
> set of protocols using data object exchange, and/or to allow a Function
> to implement multiple instances of DOE supporting the specific protocol."
>
> Tightly couple the ECAP and DOE. If we are in the multiple instances
> of DOE supporting a specific protocol case, then register it separately
> for each one. The individual device emulation then needs to deal with
> any possible clashes etc.
Not sure how configurable we want to make the device. It is a simple type 3
device after all.
The DOE spec does leave it pretty arbitrary regarding N DOE instances (DOE
Extended Cap entry points) for M protocols, including where N>1 and M=1.
Currently we implement N=2 DOE caps (instances), one for CDAT, one for
Compliance Mode.
Maybe a more complex MLD device might have one or more DOE instances
for the CDAT protocol alone to define each HDM but currently we only have
one pmem (SLD) so we can’t really do much more than what’s supported.
Open to further suggestion though. Based on answer to above we’ll follow
the suggestion lower in the code review regarding
>
> Various comments inline, but mostly small stuff.
>
> Jonathan
>
>
>> ---
>> MAINTAINERS | 7 +
>> hw/pci/meson.build| 1 +
>> hw/pci/pcie.c | 2 +-
>> hw/pci/pcie_doe.c | 414
>> ++
>> include/hw/pci/pci_ids.h | 2 +
>> include/hw/pci/pcie.h | 1 +
>> include/hw/pci/pcie_doe.h | 166
>> include/hw/pci/pcie_regs.h| 4 +
>> include/standard-headers/linux/pci_regs.h | 3 +-
>> 9 files changed, 598 insertions(+), 2 deletions(-)
>> create mode 100644 hw/pci/pcie_doe.c
>> create mode 100644 include/hw/pci/pcie_doe.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 981dc92..4fb865e 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1655,6 +1655,13 @@ F: docs/pci*
>> F: docs/specs/*pci*
>> F: default-configs/pci.mak
>>
>> +PCIE DOE
>> +M: Huai-Cheng Kuo
>> +M: Chris Browy
>> +S: Supported
>> +F: include/hw/pci/pcie_doe.h
>> +F: hw/pci/pcie_doe.c
>> +
>> ACPI/SMBIOS
>> M: Michael S. Tsirkin
>> M: Igor Mammedov
>> diff --git a/hw/pci/meson.build b/hw/pci/meson.build
>> index 5c4bbac..115e502 100644
>> --- a/hw/pci/meson.build
>> +++ b/hw/pci/meson.build
>> @@ -12,6 +12,7 @@ pci_ss.add(files(
>> # allow plugging PCIe devices into PCI buses, include them even if
>> # CONFIG_PCI_EXPRESS=n.
>> pci_ss.add(files('pcie.c', 'pcie_aer.c'))
>> +pci_ss.add(files('pcie_doe.c'))
>> softmmu_ss.add(when: 'CONFIG_PCI_EXPRESS', if_true: files('pcie_port.c',
>> 'pcie_host.c'))
>> softmmu_ss.add_all(when: 'CONFIG_PCI', if_true: pci_ss)
>>
>
> ...
>
>> diff --git a/hw/pci/pcie_doe.c b/hw/pci/pcie_doe.c
>> new file mode 100644
>> index 000..df8e92e
>> --- /dev/null
>> +++ b/hw/pci/pcie_doe.c
>> @@ -0,0 +1,414 @@
>
> Add a copyright header / license etc before v3.
>
>> +#include "qemu/osdep.h"
>> +#include "qemu/log.h"
>> +#include "qemu/error-report.h"
>> +#include "qapi/error.h"
>> +#include "qemu/range.h"
>> +#include "hw/pci/pci.h"
>> +#include "hw/pci/pcie.h"
>> +#include "hw/pci/pcie_doe.h"
>> +#include "hw/pci/msi.h"
>> +#include "hw/pci/msix.h"
>> +
>> +/*
>> + * DOE Default Protocols (Discovery, CMA)
>> + */
>> +/* Discovery Request Object */
>> +struct doe_discovery {
>> +DOEHeader header;
>> +uint8_t index;
>> +uint8_t reserved[3];
>> +} QEMU_PACKED;
>> +
>> +/* Discovery Response Object */
>> +struct doe_discovery_rsp {
>> +DOEHeader header;
>> +uint16_t vendor_id;
>> +uint8_t doe_type;
>> +uint8_t next_index;
>> +} QEMU_PACKED;
>> +
>> +/* Callback for Discovery */
>> +static bool pcie_doe_discovery_rsp(DOECap *doe_cap)
>> +{
>> +PCIEDOE *doe = doe_cap->doe;
>> +struct doe_discovery *req = pcie_doe_get_req(doe_cap);
>> +uint8_t index = req->index;
>> +DOEProtocol *prot = NULL;
>> +
>> +/* Request length mismatch, discard */
>> +if (req->header.length < dwsizeof(struct doe_discovery)) {
>> +
Re: [PATCH] virtiofs_submounts.py test: Note on vmlinuz param
Max Reitz writes:
> From the cancel message, it is not entirely clear why this parameter is
> mandatory now, or that it will be optional in the future. Add such a
> more detailed explanation as a comment in the test source file.
>
> Suggested-by: Alex Bennée
> Signed-off-by: Max Reitz
Thanks!
Reviewed-by: Alex Bennée
> ---
> I’ve uploaded a build of Linux 5.10 here:
> https://xanclic.moe/linux-5.10
>
> But I’ve decided against mentioning it in this new comment or the cancel
> message, because, well, it’s my private server and I have limited
> bandwidth.
> ---
> tests/acceptance/virtiofs_submounts.py | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/tests/acceptance/virtiofs_submounts.py
> b/tests/acceptance/virtiofs_submounts.py
> index 949ca87a83..9a69b6b17b 100644
> --- a/tests/acceptance/virtiofs_submounts.py
> +++ b/tests/acceptance/virtiofs_submounts.py
> @@ -228,6 +228,18 @@ class VirtiofsSubmountsTest(BootLinux):
> def setUp(self):
> vmlinuz = self.params.get('vmlinuz')
> if vmlinuz is None:
> +"""
> +The Linux kernel supports FUSE auto-submounts only as of 5.10.
> +boot_linux.py currently provides Fedora 31, whose kernel is too
> +old, so this test cannot pass with the on-image kernel (you are
> +welcome to try, hence the option to force such a test with
> +-p vmlinuz=''). Therefore, for now the user must provide a
> +sufficiently new custom kernel, or effectively explicitly
> +request failure with -p vmlinuz=''.
> +Once an image with a sufficiently new kernel is available
> +(probably Fedora 34), we can make -p vmlinuz='' the default, so
> +that this parameter no longer needs to be specified.
> +"""
> self.cancel('vmlinuz parameter not set; you must point it to a '
> 'Linux kernel binary to test (to run this test with
> ' \
> 'the on-image kernel, set it to an empty string)')
--
Alex Bennée
Re: [PATCH 4/4] hw/riscv: virt: Map high mmio for PCIe
On Fri, Jan 22, 2021 at 4:35 AM Bin Meng wrote:
>
> From: Bin Meng
>
> Some peripherals require 64-bit PCI address, so let's map the high
> mmio space for PCIe.
>
> For RV32, the address is hardcoded to below 4 GiB from the highest
> accessible physical address. For RV64, the base address depends on
> top of RAM and is aligned to its size which is using 16 GiB for now.
>
> Signed-off-by: Bin Meng
Reviewed-by: Alistair Francis
Alistair
>
> ---
>
> hw/riscv/virt.c | 36 ++--
> 1 file changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 4f44509360..4ab3b35cc7 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -59,6 +59,15 @@ static const MemMapEntry virt_memmap[] = {
> [VIRT_DRAM] ={ 0x8000, 0x0 },
> };
>
> +/* PCIe high mmio is fixed for RV32 */
> +#define VIRT32_HIGH_PCIE_MMIO_BASE 0x3ULL
> +#define VIRT32_HIGH_PCIE_MMIO_SIZE (4 * GiB)
> +
> +/* PCIe high mmio for RV64, size is fixed but base depends on top of RAM */
> +#define VIRT64_HIGH_PCIE_MMIO_SIZE (16 * GiB)
> +
> +static MemMapEntry virt_high_pcie_memmap;
> +
> #define VIRT_FLASH_SECTOR_SIZE (256 * KiB)
>
> static PFlashCFI01 *virt_flash_create1(RISCVVirtState *s,
> @@ -371,7 +380,11 @@ static void create_fdt(RISCVVirtState *s, const
> MemMapEntry *memmap,
> 2, memmap[VIRT_PCIE_PIO].base, 2, memmap[VIRT_PCIE_PIO].size,
> 1, FDT_PCI_RANGE_MMIO,
> 2, memmap[VIRT_PCIE_MMIO].base,
> -2, memmap[VIRT_PCIE_MMIO].base, 2, memmap[VIRT_PCIE_MMIO].size);
> +2, memmap[VIRT_PCIE_MMIO].base, 2, memmap[VIRT_PCIE_MMIO].size,
> +1, FDT_PCI_RANGE_MMIO_64BIT,
> +2, virt_high_pcie_memmap.base,
> +2, virt_high_pcie_memmap.base, 2, virt_high_pcie_memmap.size);
> +
> create_pcie_irq_map(fdt, name, plic_pcie_phandle);
> g_free(name);
>
> @@ -448,12 +461,14 @@ update_bootargs:
> static inline DeviceState *gpex_pcie_init(MemoryRegion *sys_mem,
>hwaddr ecam_base, hwaddr ecam_size,
>hwaddr mmio_base, hwaddr mmio_size,
> + hwaddr high_mmio_base,
> + hwaddr high_mmio_size,
>hwaddr pio_base,
>DeviceState *plic)
> {
> DeviceState *dev;
> MemoryRegion *ecam_alias, *ecam_reg;
> -MemoryRegion *mmio_alias, *mmio_reg;
> +MemoryRegion *mmio_alias, *high_mmio_alias, *mmio_reg;
> qemu_irq irq;
> int i;
>
> @@ -473,6 +488,13 @@ static inline DeviceState *gpex_pcie_init(MemoryRegion
> *sys_mem,
> mmio_reg, mmio_base, mmio_size);
> memory_region_add_subregion(get_system_memory(), mmio_base, mmio_alias);
>
> +/* Map high MMIO space */
> +high_mmio_alias = g_new0(MemoryRegion, 1);
> +memory_region_init_alias(high_mmio_alias, OBJECT(dev), "pcie-mmio-high",
> + mmio_reg, high_mmio_base, high_mmio_size);
> +memory_region_add_subregion(get_system_memory(), high_mmio_base,
> +high_mmio_alias);
> +
> sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, pio_base);
>
> for (i = 0; i < GPEX_NUM_IRQS; i++) {
> @@ -601,6 +623,14 @@ static void virt_machine_init(MachineState *machine)
> machine->ram_size = 10 * GiB;
> error_report("Limitting RAM size to 10 GiB");
> }
> +
> +virt_high_pcie_memmap.base = VIRT32_HIGH_PCIE_MMIO_BASE;
> +virt_high_pcie_memmap.size = VIRT32_HIGH_PCIE_MMIO_SIZE;
> +} else {
> +virt_high_pcie_memmap.size = VIRT64_HIGH_PCIE_MMIO_SIZE;
> +virt_high_pcie_memmap.base = memmap[VIRT_DRAM].base +
> machine->ram_size;
> +virt_high_pcie_memmap.base =
> +ROUND_UP(virt_high_pcie_memmap.base, virt_high_pcie_memmap.size);
> }
>
> /* register system main memory (actual RAM) */
> @@ -686,6 +716,8 @@ static void virt_machine_init(MachineState *machine)
> memmap[VIRT_PCIE_ECAM].size,
> memmap[VIRT_PCIE_MMIO].base,
> memmap[VIRT_PCIE_MMIO].size,
> + virt_high_pcie_memmap.base,
> + virt_high_pcie_memmap.size,
> memmap[VIRT_PCIE_PIO].base,
> DEVICE(pcie_plic));
>
> --
> 2.25.1
>
>
Re: [PATCH v2 1/1] linux-user/signal: Decode waitid si_code
On Thu, Jan 21, 2021 at 7:15 AM Andreas K. Hüttel wrote:
>
> Am Mittwoch, 20. Januar 2021, 22:12:30 EET schrieb Andreas K. Hüttel:
> > > This patch just passes the waitid status directly back to the guest.
> >
> > This works at least as well as the previous versions, so ++ from me.
> >
> > Will do more testing over the next days to see if it maybe also improves the
> > additional oddities I observed.
> >
>
> So the patch is good since it clearly resolves the linked bug.
>
> However, something else is still broken (maybe related; unchanged compared to
> the previous patch version). I keep seeing hanging "qemu-riscv32 /bin/bash
> ..." processes using 100% cpu. If I attach strace (on the host arch x86-64) to
> qemu, I see an infinite loop:
>
> waitid(P_ALL, -1, {}, WNOHANG|WEXITED, NULL) = 0
> rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
> rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
> rt_sigprocmask(SIG_SETMASK, [CHLD], NULL, 8) = 0
> rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
> rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
> rt_sigprocmask(SIG_SETMASK, [CHLD], NULL, 8) = 0
> waitid(P_ALL, -1, {}, WNOHANG|WEXITED, NULL) = 0
> rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
> rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
> rt_sigprocmask(SIG_SETMASK, [CHLD], NULL, 8) = 0
> rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
> rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], NULL, 8) = 0
> rt_sigprocmask(SIG_SETMASK, [CHLD], NULL, 8) = 0
> ...
>
> Unfortunately I do not have a simple reproducer. This is somewhere in the
> middle of our build system...
>
> Otherwise, I can re-build glibc, gcc, binutils, make in the qemu chroot
> without obvious problems, with one striking strange detail - "make" refuses to
> run more than one concurrent process (even with MAKEOPTS="-j9"). Something is
> off there.
Ping!
Even though it seems like there are still issues with RV32, this is a
step in the right direction.
Alistair
>
> --
> Andreas K. Hüttel
> dilfri...@gentoo.org
> Gentoo Linux developer
> (council, qa, toolchain, base-system, perl, libreoffice)
Re: [PATCH v2 6/7] docs: add some documentation for the guest-loader
On Thu, Feb 11, 2021 at 9:20 AM Alex Bennée wrote: > > Signed-off-by: Alex Bennée > Message-Id: <20201105175153.30489-7-alex.ben...@linaro.org> Reviewed-by: Alistair Francis Alistair > > --- > v2 > - add docs and MAINTAINERS > --- > docs/system/guest-loader.rst | 54 > docs/system/index.rst| 1 + > MAINTAINERS | 1 + > 3 files changed, 56 insertions(+) > create mode 100644 docs/system/guest-loader.rst > > diff --git a/docs/system/guest-loader.rst b/docs/system/guest-loader.rst > new file mode 100644 > index 00..37d03cbd89 > --- /dev/null > +++ b/docs/system/guest-loader.rst > @@ -0,0 +1,54 @@ > +.. > + Copyright (c) 2020, Linaro > + > +Guest Loader > + > + > +The guest loader is similar to the `generic-loader` although it is > +aimed at a particular use case of loading hypervisor guests. This is > +useful for debugging hypervisors without having to jump through the > +hoops of firmware and boot-loaders. > + > +The guest loader does two things: > + > + - load blobs (kernels and initial ram disks) into memory > + - sets platform FDT data so hypervisors can find and boot them > + > +This is what is typically done by a boot-loader like grub using it's > +multi-boot capability. A typical example would look like: > + > +.. parsed-literal:: > + > + |qemu_system| -kernel ~/xen.git/xen/xen \ > +-append "dom0_mem=1G,max:1G loglvl=all guest_loglvl=all" \ > +-device > guest-loader,addr=0x4200,kernel=Image,bootargs="root=/dev/sda2 ro > console=hvc0 earlyprintk=xen" \ > +-device guest-loader,addr=0x4700,initrd=rootfs.cpio > + > +In the above example the Xen hypervisor is loaded by the -kernel > +parameter and passed it's boot arguments via -append. The Dom0 guest > +is loaded into the areas of memory. Each blob will get > +`/chosen/module@` entry in the FDT to indicate it's location and > +size. Additional information can be passed with by using additional > +arguments. > + > +Currently the only supported machines which use FDT data to boot are > +the ARM and RiscV `virt` machines. > + > +Arguments > +^ > + > +The full syntax of the guest-loader is:: > + > + -device > guest-loader,addr=[,kernel=,[bootargs=]][,initrd=] > + > +``addr=`` > + This is mandatory and indicates the start address of the blob. > + > +``kernel|initrd=`` > + Indicates the filename of the kernel or initrd blob. Both blobs will > + have the "multiboot,module" compatibility string as well as > + "multiboot,kernel" or "multiboot,ramdisk" as appropriate. > + > +``bootargs=`` > + This is an optional field for kernel blobs which will pass command > + like via the `/chosen/module@/bootargs` node. > diff --git a/docs/system/index.rst b/docs/system/index.rst > index cee1c83540..6ad9c93806 100644 > --- a/docs/system/index.rst > +++ b/docs/system/index.rst > @@ -26,6 +26,7 @@ Contents: > ivshmem > linuxboot > generic-loader > + guest-loader > vnc-security > tls > gdb > diff --git a/MAINTAINERS b/MAINTAINERS > index 774b3ca7a5..853f174fcf 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1997,6 +1997,7 @@ Guest Loader > M: Alex Bennée > S: Maintained > F: hw/core/guest-loader.c > +F: docs/system/guest-loader.rst > > Intel Hexadecimal Object File Loader > M: Su Hang > -- > 2.20.1 > >
Re: [PATCH v2 5/7] docs: move generic-loader documentation into the main manual
On Thu, Feb 11, 2021 at 9:21 AM Alex Bennée wrote: > > We might as well surface this useful information in the manual so > users can find it easily. It is a fairly simple conversion to rst with > the only textual fixes being QemuOps to QemuOpts. > > Signed-off-by: Alex Bennée > Message-Id: <20201105175153.30489-6-alex.ben...@linaro.org> Reviewed-by: Alistair Francis Alistair > > --- > v2 > - fix whitespace > - update MAINTAINERS > --- > docs/generic-loader.txt| 92 -- > docs/system/generic-loader.rst | 117 + > docs/system/index.rst | 1 + > MAINTAINERS| 2 +- > 4 files changed, 119 insertions(+), 93 deletions(-) > delete mode 100644 docs/generic-loader.txt > create mode 100644 docs/system/generic-loader.rst > > diff --git a/docs/generic-loader.txt b/docs/generic-loader.txt > deleted file mode 100644 > index a9603a2af7..00 > --- a/docs/generic-loader.txt > +++ /dev/null > @@ -1,92 +0,0 @@ > -Copyright (c) 2016 Xilinx Inc. > - > -This work is licensed under the terms of the GNU GPL, version 2 or later. > See > -the COPYING file in the top-level directory. > - > - > -The 'loader' device allows the user to load multiple images or values into > -QEMU at startup. > - > -Loading Data into Memory Values > > -The loader device allows memory values to be set from the command line. This > -can be done by following the syntax below: > - > - -device loader,addr=,data=,data-len= > - [,data-be=][,cpu-num=] > - > - - The address to store the data in. > - - The value to be written to the address. The maximum size of > - the data is 8 bytes. > - - The length of the data in bytes. This argument must be > - included if the data argument is. > - - Set to true if the data to be stored on the guest should be > - written as big endian data. The default is to write little > - endian data. > - - The number of the CPU's address space where the data should > - be loaded. If not specified the address space of the first > - CPU is used. > - > -All values are parsed using the standard QemuOps parsing. This allows the > user > -to specify any values in any format supported. By default the values > -will be parsed as decimal. To use hex values the user should prefix the > number > -with a '0x'. > - > -An example of loading value 0x800e to address 0xfd1a0104 is: > --device loader,addr=0xfd1a0104,data=0x800e,data-len=4 > - > -Setting a CPU's Program Counter > > -The loader device allows the CPU's PC to be set from the command line. This > -can be done by following the syntax below: > - > - -device loader,addr=,cpu-num= > - > - - The value to use as the CPU's PC. > - - The number of the CPU whose PC should be set to the > - specified value. > - > -All values are parsed using the standard QemuOps parsing. This allows the > user > -to specify any values in any format supported. By default the values > -will be parsed as decimal. To use hex values the user should prefix the > number > -with a '0x'. > - > -An example of setting CPU 0's PC to 0x8000 is: > --device loader,addr=0x8000,cpu-num=0 > - > -Loading Files > -- > -The loader device also allows files to be loaded into memory. It can load > ELF, > -U-Boot, and Intel HEX executable formats as well as raw images. The syntax > is > -shown below: > - > --device > loader,file=[,addr=][,cpu-num=][,force-raw=] > - > - - A file to be loaded into memory > - - The memory address where the file should be loaded. This is > - required for raw images and ignored for non-raw files. > - - This specifies the CPU that should be used. This is an > - optional argument and will cause the CPU's PC to be set to > - the memory address where the raw file is loaded or the > entry > - point specified in the executable format header. This > option > - should only be used for the boot image. > - This will also cause the image to be written to the > specified > - CPU's address space. If not specified, the default is CPU > 0. > - - Setting force-raw=on forces the file to be treated as a raw > - image. This can be used to load supported executable > formats > - as if they were raw. > - > -All values are parsed using the standard QemuOps parsing. This allows the > user > -to specify any values in any format supported. By default the values > -will be parsed as decimal. To use hex values the user should prefix the > number > -with a '0x'. > - > -An example of loading an ELF file which CPU0 will boot is shown below: > -
Re: [RFC PATCH 15/15] arm: xlnx-versal: Add emmc to versal
On Thu, Feb 11, 2021 at 12:36 AM Sai Pavan Boddu
wrote:
>
> Configuring SDHCI-0 to act as eMMC controller.
>
> Signed-off-by: Sai Pavan Boddu
Reviewed-by: Alistair Francis
Alistair
> ---
> hw/arm/xlnx-versal-virt.c | 16 +++-
> hw/arm/xlnx-versal.c | 14 --
> 2 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
> index 8482cd6..18489e4 100644
> --- a/hw/arm/xlnx-versal-virt.c
> +++ b/hw/arm/xlnx-versal-virt.c
> @@ -333,6 +333,13 @@ static void fdt_add_sd_nodes(VersalVirt *s)
> qemu_fdt_setprop_sized_cells(s->fdt, name, "reg",
> 2, addr, 2, MM_PMC_SD0_SIZE);
> qemu_fdt_setprop(s->fdt, name, "compatible", compat, sizeof(compat));
> +/*
> + * eMMC specific properties
> + */
> +if (i == 0) {
> +qemu_fdt_setprop(s->fdt, name, "non-removable", NULL, 0);
> +qemu_fdt_setprop_sized_cells(s->fdt, name, "bus-width", 1, 8);
> +}
> g_free(name);
> }
> }
> @@ -512,7 +519,7 @@ static void create_virtio_regions(VersalVirt *s)
> }
> }
>
> -static void sd_plugin_card(SDHCIState *sd, DriveInfo *di)
> +static void sd_plugin_card(SDHCIState *sd, DriveInfo *di, bool emmc)
> {
> BlockBackend *blk = di ? blk_by_legacy_dinfo(di) : NULL;
> DeviceState *card;
> @@ -520,6 +527,7 @@ static void sd_plugin_card(SDHCIState *sd, DriveInfo *di)
> card = qdev_new(TYPE_SD_CARD);
> object_property_add_child(OBJECT(sd), "card[*]", OBJECT(card));
> qdev_prop_set_drive_err(card, "drive", blk, _fatal);
> +object_property_set_bool(OBJECT(card), "emmc", emmc, _fatal);
> qdev_realize_and_unref(card, qdev_get_child_bus(DEVICE(sd), "sd-bus"),
> _fatal);
> }
> @@ -528,7 +536,6 @@ static void versal_virt_init(MachineState *machine)
> {
> VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(machine);
> int psci_conduit = QEMU_PSCI_CONDUIT_DISABLED;
> -int i;
>
> /*
> * If the user provides an Operating System to be loaded, we expect them
> @@ -581,10 +588,9 @@ static void versal_virt_init(MachineState *machine)
> memory_region_add_subregion_overlap(get_system_memory(),
> 0, >soc.fpd.apu.mr, 0);
>
> +sd_plugin_card(>soc.pmc.iou.sd[0], drive_get_next(IF_EMMC), true);
> /* Plugin SD cards. */
> -for (i = 0; i < ARRAY_SIZE(s->soc.pmc.iou.sd); i++) {
> -sd_plugin_card(>soc.pmc.iou.sd[i], drive_get_next(IF_SD));
> -}
> +sd_plugin_card(>soc.pmc.iou.sd[1], drive_get_next(IF_SD), false);
>
> s->binfo.ram_size = machine->ram_size;
> s->binfo.loader_start = 0x0;
> diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
> index b077716..3498dd9 100644
> --- a/hw/arm/xlnx-versal.c
> +++ b/hw/arm/xlnx-versal.c
> @@ -230,9 +230,14 @@ static void versal_create_admas(Versal *s, qemu_irq *pic)
> }
>
> #define SDHCI_CAPABILITIES 0x280737ec6481 /* Same as on ZynqMP. */
> +#define SDHCI0_CAPS ((SDHCI_CAPABILITIES & ~(3 << 30)) | \
> + (1 << 30))
> +#define SDHCI1_CAPS SDHCI_CAPABILITIES
> +
> static void versal_create_sds(Versal *s, qemu_irq *pic)
> {
> int i;
> +uint64_t caps[] = {SDHCI0_CAPS, SDHCI1_CAPS};
>
> for (i = 0; i < ARRAY_SIZE(s->pmc.iou.sd); i++) {
> DeviceState *dev;
> @@ -244,9 +249,14 @@ static void versal_create_sds(Versal *s, qemu_irq *pic)
>
> object_property_set_uint(OBJECT(dev), "sd-spec-version", 3,
> _fatal);
> -object_property_set_uint(OBJECT(dev), "capareg", SDHCI_CAPABILITIES,
> +object_property_set_uint(OBJECT(dev), "capareg", caps[i],
> _fatal);
> -object_property_set_uint(OBJECT(dev), "uhs", UHS_I, _fatal);
> +/*
> + * UHS is not applicable for eMMC
> + */
> +if (i == 1) {
> +object_property_set_uint(OBJECT(dev), "uhs", UHS_I,
> _fatal);
> +}
> sysbus_realize(SYS_BUS_DEVICE(dev), _fatal);
>
> mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
> --
> 2.7.4
>
>
Re: [RFC PATCH 03/15] sd: emmc: Dont not update CARD_CAPACITY for eMMC cards
On Thu, Feb 11, 2021 at 12:22 AM Sai Pavan Boddu
wrote:
>
> OCR.CARD_CAPACITY field is only valid for sd cards, So skip it for eMMC.
>
> Signed-off-by: Sai Pavan Boddu
> Signed-off-by: Edgar E. Iglesias
Reviewed-by: Alistair Francis
Alistair
> ---
> hw/sd/sd.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index a75fa1c..57fff89 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -308,7 +308,8 @@ static void sd_ocr_powerup(void *opaque)
> /* card power-up OK */
> sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_POWER_UP, 1);
>
> -if (sd->size > SDSC_MAX_CAPACITY) {
> +/* eMMC supports only Byte mode */
> +if (!sd->emmc && sd->size > SDSC_MAX_CAPACITY) {
> sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_CAPACITY, 1);
> }
> }
> --
> 2.7.4
>
>
Re: [RFC PATCH 01/15] block: add eMMC block device type
On Thu, Feb 11, 2021 at 12:15 AM Sai Pavan Boddu
wrote:
>
> From: Vincent Palatin
>
> Add new block device type.
>
> Signed-off-by: Vincent Palatin
> [SPB: Rebased over 5.1 version]
> Signed-off-by: Sai Pavan Boddu
> Signed-off-by: Joel Stanley
> Signed-off-by: Cédric Le Goater
Reviewed-by: Alistair Francis
Alistair
> ---
> blockdev.c| 1 +
> include/sysemu/blockdev.h | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/blockdev.c b/blockdev.c
> index b250b9b..593ce44 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
> [IF_SD] = "sd",
> [IF_VIRTIO] = "virtio",
> [IF_XEN] = "xen",
> +[IF_EMMC] = "emmc",
> };
>
> static int if_max_devs[IF_COUNT] = {
> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> index 3b5fcda..eefae9f 100644
> --- a/include/sysemu/blockdev.h
> +++ b/include/sysemu/blockdev.h
> @@ -24,6 +24,7 @@ typedef enum {
> */
> IF_NONE = 0,
> IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
> +IF_EMMC,
> IF_COUNT
> } BlockInterfaceType;
>
> --
> 2.7.4
>
>
[PULL 5/7] m68k: MOVEC insn. should generate exception if wrong CR is accessed
From: Lucien Murray-Pitts
Add CPU class detection for each CR type in the m68k_move_to/from helpers,
so that it throws and exception if an unsupported register is requested
for that CPU class.
Reclassified MOVEC insn. as only supported from 68010.
Signed-off-by: Lucien Murray-Pitts
Signed-off-by: BALATON Zoltan
Message-Id:
Signed-off-by: Laurent Vivier
---
target/m68k/cpu.h | 1 +
target/m68k/cpu.c | 1 +
target/m68k/helper.c| 188 ++--
target/m68k/translate.c | 2 +-
4 files changed, 146 insertions(+), 46 deletions(-)
diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index ae34c9461503..5d2cb012e510 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -497,6 +497,7 @@ enum m68k_features {
M68K_FEATURE_RTD, /* RTD insn. (680[12346]0, and CPU32) */
M68K_FEATURE_CHK2, /* CHK2 insn. (680[2346]0, and CPU32) */
M68K_FEATURE_MOVEP, /* MOVEP insn. (680[01234]0, and CPU32) */
+M68K_FEATURE_MOVEC, /* MOVEC insn. (from 68010) */
};
static inline int m68k_feature(CPUM68KState *env, int feature)
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index d0f8bd44339c..ff3c4c1c9802 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -132,6 +132,7 @@ static void m68010_cpu_initfn(Object *obj)
m68k_set_feature(env, M68K_FEATURE_M68010);
m68k_set_feature(env, M68K_FEATURE_RTD);
m68k_set_feature(env, M68K_FEATURE_BKPT);
+m68k_set_feature(env, M68K_FEATURE_MOVEC);
}
/*
diff --git a/target/m68k/helper.c b/target/m68k/helper.c
index 69acdc3b353c..1efd6e4f6555 100644
--- a/target/m68k/helper.c
+++ b/target/m68k/helper.c
@@ -184,6 +184,14 @@ void HELPER(cf_movec_to)(CPUM68KState *env, uint32_t reg,
uint32_t val)
}
}
+static void raise_exception_ra(CPUM68KState *env, int tt, uintptr_t raddr)
+{
+CPUState *cs = env_cpu(env);
+
+cs->exception_index = tt;
+cpu_loop_exit_restore(cs, raddr);
+}
+
void HELPER(m68k_movec_to)(CPUM68KState *env, uint32_t reg, uint32_t val)
{
switch (reg) {
@@ -209,61 +217,104 @@ void HELPER(m68k_movec_to)(CPUM68KState *env, uint32_t
reg, uint32_t val)
env->cacr = val & 0x80008000;
} else if (m68k_feature(env, M68K_FEATURE_M68060)) {
env->cacr = val & 0xf8e0e000;
+} else {
+break;
}
m68k_switch_sp(env);
return;
/* MC680[46]0 */
case M68K_CR_TC:
-env->mmu.tcr = val;
-return;
+if (m68k_feature(env, M68K_FEATURE_M68040)
+ || m68k_feature(env, M68K_FEATURE_M68060)) {
+env->mmu.tcr = val;
+return;
+}
+break;
/* MC68040 */
case M68K_CR_MMUSR:
-env->mmu.mmusr = val;
-return;
+if (m68k_feature(env, M68K_FEATURE_M68040)) {
+env->mmu.mmusr = val;
+return;
+}
+break;
/* MC680[46]0 */
case M68K_CR_SRP:
-env->mmu.srp = val;
-return;
-case M68K_CR_URP:
-env->mmu.urp = val;
-return;
+if (m68k_feature(env, M68K_FEATURE_M68040)
+ || m68k_feature(env, M68K_FEATURE_M68060)) {
+env->mmu.srp = val;
+return;
+}
+break;
/* MC680[46]0 */
+case M68K_CR_URP:
+if (m68k_feature(env, M68K_FEATURE_M68040)
+ || m68k_feature(env, M68K_FEATURE_M68060)) {
+env->mmu.urp = val;
+return;
+}
+break;
+/* MC680[12346]0 */
case M68K_CR_USP:
env->sp[M68K_USP] = val;
return;
/* MC680[234]0 */
case M68K_CR_MSP:
-env->sp[M68K_SSP] = val;
-return;
+if (m68k_feature(env, M68K_FEATURE_M68020)
+ || m68k_feature(env, M68K_FEATURE_M68030)
+ || m68k_feature(env, M68K_FEATURE_M68040)) {
+env->sp[M68K_SSP] = val;
+return;
+}
+break;
/* MC680[234]0 */
case M68K_CR_ISP:
-env->sp[M68K_ISP] = val;
-return;
+if (m68k_feature(env, M68K_FEATURE_M68020)
+ || m68k_feature(env, M68K_FEATURE_M68030)
+ || m68k_feature(env, M68K_FEATURE_M68040)) {
+env->sp[M68K_ISP] = val;
+return;
+}
+break;
/* MC68040/MC68LC040 */
-case M68K_CR_ITT0:
-env->mmu.ttr[M68K_ITTR0] = val;
-return;
+case M68K_CR_ITT0: /* MC68EC040 only: M68K_CR_IACR0 */
+if (m68k_feature(env, M68K_FEATURE_M68040)) {
+env->mmu.ttr[M68K_ITTR0] = val;
+return;
+}
+break;
/* MC68040/MC68LC040 */
-case M68K_CR_ITT1:
- env->mmu.ttr[M68K_ITTR1] = val;
-return;
+case M68K_CR_ITT1: /* MC68EC040 only: M68K_CR_IACR1 */
+if (m68k_feature(env, M68K_FEATURE_M68040)) {
+env->mmu.ttr[M68K_ITTR1] = val;
+return;
+}
+break;
/* MC68040/MC68LC040 */
-case M68K_CR_DTT0:
-env->mmu.ttr[M68K_DTTR0] = val;
-
[PULL 2/7] m68k: cascade m68k_features by m680xx_cpu_initfn() to improve readability
From: Lucien Murray-Pitts
The m680XX_cpu_initfn functions have been rearranged to cascade starting from
the base 68000, so that the 68010 then inherits from this, and so on until the
68060.
This makes it simpler to track features since in most cases the m68k were
product enhancements on each other, with only a few instructions being retired.
Because each cpu class inherits the previous CPU class, then for example
the 68020 also has the feature 68010, and 68000 and so on upto the 68060.
- Added 68010 cpu class, and moved correct features into 68000/68010.
- Added m68k_unset_feature to allow removing a feature in the inheritence
Signed-off-by: Lucien Murray-Pitts
Reviewed-by: Laurent Vivier
Signed-off-by: BALATON Zoltan
Message-Id:
Signed-off-by: Laurent Vivier
---
target/m68k/cpu.h | 1 +
target/m68k/cpu.c | 72 +--
2 files changed, 39 insertions(+), 34 deletions(-)
diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index 1d59cbb3f4ab..2b1cdf241bab 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -466,6 +466,7 @@ void do_m68k_semihosting(CPUM68KState *env, int nr);
enum m68k_features {
M68K_FEATURE_M68000, /* Base m68k instruction set */
+M68K_FEATURE_M68010,
M68K_FEATURE_M68020,
M68K_FEATURE_M68030,
M68K_FEATURE_M68040,
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index 5c72f2469471..d0f8bd44339c 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -41,6 +41,11 @@ static void m68k_set_feature(CPUM68KState *env, int feature)
env->features |= (1u << feature);
}
+static void m68k_unset_feature(CPUM68KState *env, int feature)
+{
+env->features &= (-1u - (1u << feature));
+}
+
static void m68k_cpu_reset(DeviceState *dev)
{
CPUState *s = CPU(dev);
@@ -115,25 +120,18 @@ static void m68000_cpu_initfn(Object *obj)
m68k_set_feature(env, M68K_FEATURE_MOVEP);
}
-/* common features for 68020, 68030 and 68040 */
-static void m680x0_cpu_common(CPUM68KState *env)
+/*
+ * Adds BKPT, MOVE-from-SR *now priv instr, and MOVEC, MOVES, RTD
+ */
+static void m68010_cpu_initfn(Object *obj)
{
-m68k_set_feature(env, M68K_FEATURE_M68000);
-m68k_set_feature(env, M68K_FEATURE_USP);
-m68k_set_feature(env, M68K_FEATURE_WORD_INDEX);
-m68k_set_feature(env, M68K_FEATURE_QUAD_MULDIV);
-m68k_set_feature(env, M68K_FEATURE_BRAL);
-m68k_set_feature(env, M68K_FEATURE_BCCL);
-m68k_set_feature(env, M68K_FEATURE_BITFIELD);
-m68k_set_feature(env, M68K_FEATURE_EXT_FULL);
-m68k_set_feature(env, M68K_FEATURE_SCALED_INDEX);
-m68k_set_feature(env, M68K_FEATURE_LONG_MULDIV);
-m68k_set_feature(env, M68K_FEATURE_FPU);
-m68k_set_feature(env, M68K_FEATURE_CAS);
-m68k_set_feature(env, M68K_FEATURE_BKPT);
+M68kCPU *cpu = M68K_CPU(obj);
+CPUM68KState *env = >env;
+
+m68000_cpu_initfn(obj);
+m68k_set_feature(env, M68K_FEATURE_M68010);
m68k_set_feature(env, M68K_FEATURE_RTD);
-m68k_set_feature(env, M68K_FEATURE_CHK2);
-m68k_set_feature(env, M68K_FEATURE_MOVEP);
+m68k_set_feature(env, M68K_FEATURE_BKPT);
}
/*
@@ -148,8 +146,19 @@ static void m68020_cpu_initfn(Object *obj)
M68kCPU *cpu = M68K_CPU(obj);
CPUM68KState *env = >env;
-m680x0_cpu_common(env);
+m68010_cpu_initfn(obj);
+m68k_unset_feature(env, M68K_FEATURE_M68010);
m68k_set_feature(env, M68K_FEATURE_M68020);
+m68k_set_feature(env, M68K_FEATURE_QUAD_MULDIV);
+m68k_set_feature(env, M68K_FEATURE_BRAL);
+m68k_set_feature(env, M68K_FEATURE_BCCL);
+m68k_set_feature(env, M68K_FEATURE_BITFIELD);
+m68k_set_feature(env, M68K_FEATURE_EXT_FULL);
+m68k_set_feature(env, M68K_FEATURE_SCALED_INDEX);
+m68k_set_feature(env, M68K_FEATURE_LONG_MULDIV);
+m68k_set_feature(env, M68K_FEATURE_FPU);
+m68k_set_feature(env, M68K_FEATURE_CAS);
+m68k_set_feature(env, M68K_FEATURE_CHK2);
}
/*
@@ -165,7 +174,8 @@ static void m68030_cpu_initfn(Object *obj)
M68kCPU *cpu = M68K_CPU(obj);
CPUM68KState *env = >env;
-m680x0_cpu_common(env);
+m68020_cpu_initfn(obj);
+m68k_unset_feature(env, M68K_FEATURE_M68020);
m68k_set_feature(env, M68K_FEATURE_M68030);
}
@@ -191,7 +201,8 @@ static void m68040_cpu_initfn(Object *obj)
M68kCPU *cpu = M68K_CPU(obj);
CPUM68KState *env = >env;
-m680x0_cpu_common(env);
+m68030_cpu_initfn(obj);
+m68k_unset_feature(env, M68K_FEATURE_M68030);
m68k_set_feature(env, M68K_FEATURE_M68040);
}
@@ -211,21 +222,13 @@ static void m68060_cpu_initfn(Object *obj)
M68kCPU *cpu = M68K_CPU(obj);
CPUM68KState *env = >env;
-m68k_set_feature(env, M68K_FEATURE_M68000);
-m68k_set_feature(env, M68K_FEATURE_USP);
-m68k_set_feature(env, M68K_FEATURE_WORD_INDEX);
-m68k_set_feature(env, M68K_FEATURE_BRAL);
-m68k_set_feature(env, M68K_FEATURE_BCCL);
-m68k_set_feature(env, M68K_FEATURE_BITFIELD);
-m68k_set_feature(env, M68K_FEATURE_EXT_FULL);
-
[PULL 6/7] m68k: add MSP detection support for stack pointer swap helpers
From: Lucien Murray-Pitts
On m68k there are two varities of stack pointers: USP with SSP or ISP/MSP.
Only the 68020/30/40 support the MSP register the stack swap helpers don't
support this feature.
This patch adds this support, as well as comments to CPUM68KState to
make it clear how stacks are handled
Signed-off-by: Lucien Murray-Pitts
Signed-off-by: BALATON Zoltan
Message-Id:
Signed-off-by: Laurent Vivier
---
target/m68k/cpu.h| 9 -
target/m68k/cpu.c| 1 +
target/m68k/helper.c | 3 ++-
3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index 5d2cb012e510..7c3feeaf8a64 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -85,7 +85,13 @@ typedef struct CPUM68KState {
uint32_t pc;
uint32_t sr;
-/* SSP and USP. The current_sp is stored in aregs[7], the other here. */
+/*
+ * The 68020/30/40 support two supervisor stacks, ISP and MSP.
+ * The 68000/10, Coldfire, and CPU32 only have USP/SSP.
+ *
+ * The current_sp is stored in aregs[7], the other here.
+ * The USP, SSP, and if used the additional ISP for 68020/30/40.
+ */
int current_sp;
uint32_t sp[3];
@@ -484,6 +490,7 @@ enum m68k_features {
M68K_FEATURE_CF_EMAC,
M68K_FEATURE_CF_EMAC_B, /* Revision B EMAC (dual accumulate). */
M68K_FEATURE_USP, /* User Stack Pointer. (680[012346]0, ISA A+, B or C).*/
+M68K_FEATURE_MSP, /* Master Stack Pointer. (680[234]0) */
M68K_FEATURE_EXT_FULL,/* 68020+ full extension word. */
M68K_FEATURE_WORD_INDEX, /* word sized address index registers. */
M68K_FEATURE_SCALED_INDEX, /* scaled address index registers. */
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index ff3c4c1c9802..37d2ed9dc79c 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -160,6 +160,7 @@ static void m68020_cpu_initfn(Object *obj)
m68k_set_feature(env, M68K_FEATURE_FPU);
m68k_set_feature(env, M68K_FEATURE_CAS);
m68k_set_feature(env, M68K_FEATURE_CHK2);
+m68k_set_feature(env, M68K_FEATURE_MSP);
}
/*
diff --git a/target/m68k/helper.c b/target/m68k/helper.c
index 1efd6e4f6555..4185ca94cefe 100644
--- a/target/m68k/helper.c
+++ b/target/m68k/helper.c
@@ -463,7 +463,8 @@ void m68k_switch_sp(CPUM68KState *env)
env->sp[env->current_sp] = env->aregs[7];
if (m68k_feature(env, M68K_FEATURE_M68000)) {
if (env->sr & SR_S) {
-if (env->sr & SR_M) {
+/* SR:Master-Mode bit unimplemented then ISP is not available */
+if (!m68k_feature(env, M68K_FEATURE_MSP) || env->sr & SR_M) {
new_sp = M68K_SSP;
} else {
new_sp = M68K_ISP;
--
2.29.2
[PULL 7/7] m68k: import bootinfo headers from linux
Copy bootinfo.h and bootinfo-mac.h from arch/m68k/include/uapi/asm/
to include/standard-headers/asm-m68k/
Imported from linux v5.9 but didn't change since v4.14 (header update)
and since v4.10 (content update).
Signed-off-by: Laurent Vivier
Message-Id: <20201220112615.933036-2-laur...@vivier.eu>
Signed-off-by: Laurent Vivier
---
hw/m68k/bootinfo.h| 55 --
.../standard-headers/asm-m68k/bootinfo-mac.h | 120 +
include/standard-headers/asm-m68k/bootinfo.h | 166 ++
hw/m68k/q800.c| 20 +--
MAINTAINERS | 2 +
5 files changed, 295 insertions(+), 68 deletions(-)
create mode 100644 include/standard-headers/asm-m68k/bootinfo-mac.h
create mode 100644 include/standard-headers/asm-m68k/bootinfo.h
diff --git a/hw/m68k/bootinfo.h b/hw/m68k/bootinfo.h
index c954270aad6c..adbf0c5521e5 100644
--- a/hw/m68k/bootinfo.h
+++ b/hw/m68k/bootinfo.h
@@ -11,61 +11,6 @@
#ifndef HW_M68K_BOOTINFO_H
#define HW_M68K_BOOTINFO_H
-struct bi_record {
-uint16_t tag;/* tag ID */
-uint16_t size; /* size of record */
-uint32_t data[]; /* data */
-};
-
-/* machine independent tags */
-
-#define BI_LAST 0x /* last record */
-#define BI_MACHTYPE 0x0001 /* machine type (u_long) */
-#define BI_CPUTYPE 0x0002 /* cpu type (u_long) */
-#define BI_FPUTYPE 0x0003 /* fpu type (u_long) */
-#define BI_MMUTYPE 0x0004 /* mmu type (u_long) */
-#define BI_MEMCHUNK 0x0005 /* memory chunk address and size */
- /* (struct mem_info) */
-#define BI_RAMDISK 0x0006 /* ramdisk address and size */
- /* (struct mem_info) */
-#define BI_COMMAND_LINE 0x0007 /* kernel command line parameters */
- /* (string) */
-
-/* Macintosh-specific tags (all u_long) */
-
-#define BI_MAC_MODEL0x8000 /* Mac Gestalt ID (model type) */
-#define BI_MAC_VADDR0x8001 /* Mac video base address */
-#define BI_MAC_VDEPTH 0x8002 /* Mac video depth */
-#define BI_MAC_VROW 0x8003 /* Mac video rowbytes */
-#define BI_MAC_VDIM 0x8004 /* Mac video dimensions */
-#define BI_MAC_VLOGICAL 0x8005 /* Mac video logical base */
-#define BI_MAC_SCCBASE 0x8006 /* Mac SCC base address */
-#define BI_MAC_BTIME0x8007 /* Mac boot time */
-#define BI_MAC_GMTBIAS 0x8008 /* Mac GMT timezone offset */
-#define BI_MAC_MEMSIZE 0x8009 /* Mac RAM size (sanity check) */
-#define BI_MAC_CPUID0x800a /* Mac CPU type (sanity check) */
-#define BI_MAC_ROMBASE 0x800b /* Mac system ROM base address */
-
-/* Macintosh hardware profile data */
-
-#define BI_MAC_VIA1BASE 0x8010 /* Mac VIA1 base address (always present) */
-#define BI_MAC_VIA2BASE 0x8011 /* Mac VIA2 base address (type varies) */
-#define BI_MAC_VIA2TYPE 0x8012 /* Mac VIA2 type (VIA, RBV, OSS) */
-#define BI_MAC_ADBTYPE 0x8013 /* Mac ADB interface type */
-#define BI_MAC_ASCBASE 0x8014 /* Mac Apple Sound Chip base address */
-#define BI_MAC_SCSI5380 0x8015 /* Mac NCR 5380 SCSI (base address, multi) */
-#define BI_MAC_SCSIDMA 0x8016 /* Mac SCSI DMA (base address) */
-#define BI_MAC_SCSI5396 0x8017 /* Mac NCR 53C96 SCSI (base address, multi) */
-#define BI_MAC_IDETYPE 0x8018 /* Mac IDE interface type */
-#define BI_MAC_IDEBASE 0x8019 /* Mac IDE interface base address */
-#define BI_MAC_NUBUS0x801a /* Mac Nubus type (none, regular, pseudo) */
-#define BI_MAC_SLOTMASK 0x801b /* Mac Nubus slots present */
-#define BI_MAC_SCCTYPE 0x801c /* Mac SCC serial type (normal, IOP) */
-#define BI_MAC_ETHTYPE 0x801d /* Mac builtin ethernet type (Sonic, MACE */
-#define BI_MAC_ETHBASE 0x801e /* Mac builtin ethernet base address */
-#define BI_MAC_PMU 0x801f /* Mac power management / poweroff hardware */
-#define BI_MAC_IOP_SWIM 0x8020 /* Mac SWIM floppy IOP */
-#define BI_MAC_IOP_ADB 0x8021 /* Mac ADB IOP */
#define BOOTINFO0(as, base, id) \
do { \
diff --git a/include/standard-headers/asm-m68k/bootinfo-mac.h
b/include/standard-headers/asm-m68k/bootinfo-mac.h
new file mode 100644
index ..449928cfcbf2
--- /dev/null
+++ b/include/standard-headers/asm-m68k/bootinfo-mac.h
@@ -0,0 +1,120 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+** asm/bootinfo-mac.h -- Macintosh-specific boot information definitions
+*/
+
+#ifndef _UAPI_ASM_M68K_BOOTINFO_MAC_H
+#define _UAPI_ASM_M68K_BOOTINFO_MAC_H
+
+
+/*
+ * Macintosh-specific tags (all __be32)
+ */
+
+#define BI_MAC_MODEL 0x8000 /* Mac Gestalt ID (model type) */
+#define BI_MAC_VADDR 0x8001 /* Mac video base address */
+#define BI_MAC_VDEPTH 0x8002 /* Mac video depth */
+#define BI_MAC_VROW0x8003 /* Mac video rowbytes */
+#define BI_MAC_VDIM0x8004 /* Mac video dimensions */
+#define BI_MAC_VLOGICAL0x8005 /* Mac video logical base */
[PULL 3/7] m68k: improve comments on m68k_move_to/from helpers
From: Lucien Murray-Pitts
Add more detailed comments to each case of m68k_move_to/from helpers to list
the supported CPUs for that CR as they were wrong in some cases, and
missing some cpu classes in other cases.
Signed-off-by: Lucien Murray-Pitts
Signed-off-by: BALATON Zoltan
Message-Id:
Signed-off-by: Laurent Vivier
---
target/m68k/helper.c | 39 ++-
1 file changed, 30 insertions(+), 9 deletions(-)
diff --git a/target/m68k/helper.c b/target/m68k/helper.c
index 3ff57657958c..9e81ee53ad8b 100644
--- a/target/m68k/helper.c
+++ b/target/m68k/helper.c
@@ -187,13 +187,15 @@ void HELPER(cf_movec_to)(CPUM68KState *env, uint32_t reg,
uint32_t val)
void HELPER(m68k_movec_to)(CPUM68KState *env, uint32_t reg, uint32_t val)
{
switch (reg) {
-/* MC680[1234]0 */
+/* MC680[12346]0 */
case M68K_CR_SFC:
env->sfc = val & 7;
return;
+/* MC680[12346]0 */
case M68K_CR_DFC:
env->dfc = val & 7;
return;
+/* MC680[12346]0 */
case M68K_CR_VBR:
env->vbr = val;
return;
@@ -210,25 +212,30 @@ void HELPER(m68k_movec_to)(CPUM68KState *env, uint32_t
reg, uint32_t val)
}
m68k_switch_sp(env);
return;
-/* MC680[34]0 */
+/* MC680[46]0 */
case M68K_CR_TC:
env->mmu.tcr = val;
return;
+/* MC68040 */
case M68K_CR_MMUSR:
env->mmu.mmusr = val;
return;
+/* MC680[46]0 */
case M68K_CR_SRP:
env->mmu.srp = val;
return;
case M68K_CR_URP:
env->mmu.urp = val;
return;
+/* MC680[46]0 */
case M68K_CR_USP:
env->sp[M68K_USP] = val;
return;
+/* MC680[234]0 */
case M68K_CR_MSP:
env->sp[M68K_SSP] = val;
return;
+/* MC680[234]0 */
case M68K_CR_ISP:
env->sp[M68K_ISP] = val;
return;
@@ -236,12 +243,15 @@ void HELPER(m68k_movec_to)(CPUM68KState *env, uint32_t
reg, uint32_t val)
case M68K_CR_ITT0:
env->mmu.ttr[M68K_ITTR0] = val;
return;
+/* MC68040/MC68LC040 */
case M68K_CR_ITT1:
env->mmu.ttr[M68K_ITTR1] = val;
return;
+/* MC68040/MC68LC040 */
case M68K_CR_DTT0:
env->mmu.ttr[M68K_DTTR0] = val;
return;
+/* MC68040/MC68LC040 */
case M68K_CR_DTT1:
env->mmu.ttr[M68K_DTTR1] = val;
return;
@@ -254,39 +264,50 @@ void HELPER(m68k_movec_to)(CPUM68KState *env, uint32_t
reg, uint32_t val)
uint32_t HELPER(m68k_movec_from)(CPUM68KState *env, uint32_t reg)
{
switch (reg) {
-/* MC680[1234]0 */
+/* MC680[12346]0 */
case M68K_CR_SFC:
return env->sfc;
+/* MC680[12346]0 */
case M68K_CR_DFC:
return env->dfc;
+/* MC680[12346]0 */
case M68K_CR_VBR:
return env->vbr;
-/* MC680[234]0 */
+/* MC680[2346]0 */
case M68K_CR_CACR:
return env->cacr;
-/* MC680[34]0 */
+/* MC680[46]0 */
case M68K_CR_TC:
return env->mmu.tcr;
+/* MC68040 */
case M68K_CR_MMUSR:
return env->mmu.mmusr;
+/* MC680[46]0 */
case M68K_CR_SRP:
return env->mmu.srp;
+/* MC680[46]0 */
case M68K_CR_USP:
return env->sp[M68K_USP];
+/* MC680[234]0 */
case M68K_CR_MSP:
return env->sp[M68K_SSP];
+/* MC680[234]0 */
case M68K_CR_ISP:
return env->sp[M68K_ISP];
/* MC68040/MC68LC040 */
case M68K_CR_URP:
return env->mmu.urp;
-case M68K_CR_ITT0:
+/* MC68040/MC68LC040 */
+case M68K_CR_ITT0: /* MC68EC040 only: M68K_CR_IACR0 */
return env->mmu.ttr[M68K_ITTR0];
-case M68K_CR_ITT1:
+/* MC68040/MC68LC040 */
+case M68K_CR_ITT1: /* MC68EC040 only: M68K_CR_IACR1 */
return env->mmu.ttr[M68K_ITTR1];
-case M68K_CR_DTT0:
+/* MC68040/MC68LC040 */
+case M68K_CR_DTT0: /* MC68EC040 only: M68K_CR_DACR0 */
return env->mmu.ttr[M68K_DTTR0];
-case M68K_CR_DTT1:
+/* MC68040/MC68LC040 */
+case M68K_CR_DTT1: /* MC68EC040 only: M68K_CR_DACR1 */
return env->mmu.ttr[M68K_DTTR1];
}
cpu_abort(env_cpu(env), "Unimplemented control register read 0x%x\n",
--
2.29.2
[PULL 1/7] m68k: improve cpu instantiation comments
From: Lucien Murray-Pitts
Improvement in comments for the instantiation functions.
This is to highlight what each cpu class, in the 68000 series, contains
in terms of instructions/features.
Signed-off-by: Lucien Murray-Pitts
Signed-off-by: BALATON Zoltan
Message-Id:
<2dfe32672ee6ddce4b54c6bcfce579d35abeaf51.1612137712.git.bala...@eik.bme.hu>
Signed-off-by: Laurent Vivier
---
target/m68k/cpu.h | 49 ---
target/m68k/cpu.c | 44 ++
2 files changed, 73 insertions(+), 20 deletions(-)
diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index de5b9875fea3..1d59cbb3f4ab 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -450,39 +450,48 @@ void m68k_switch_sp(CPUM68KState *env);
void do_m68k_semihosting(CPUM68KState *env, int nr);
/*
+ * The 68000 family is defined in six main CPU classes, the 680[012346]0.
+ * Generally each successive CPU adds enhanced data/stack/instructions.
+ * However, some features are only common to one, or a few classes.
+ * The features covers those subsets of instructons.
+ *
+ * CPU32/32+ are basically 680010 compatible with some 68020 class instructons,
+ * and some additional CPU32 instructions. Mostly Supervisor state differences.
+ *
+ * The ColdFire core ISA is a RISC-style reduction of the 68000 series cpu.
* There are 4 ColdFire core ISA revisions: A, A+, B and C.
* Each feature covers the subset of instructions common to the
* ISA revisions mentioned.
*/
enum m68k_features {
-M68K_FEATURE_M68000,
+M68K_FEATURE_M68000, /* Base m68k instruction set */
M68K_FEATURE_M68020,
M68K_FEATURE_M68030,
M68K_FEATURE_M68040,
M68K_FEATURE_M68060,
-M68K_FEATURE_CF_ISA_A,
-M68K_FEATURE_CF_ISA_B, /* (ISA B or C). */
-M68K_FEATURE_CF_ISA_APLUSC, /* BIT/BITREV, FF1, STRLDSR (ISA A+ or C). */
-M68K_FEATURE_BRAL, /* Long unconditional branch. (ISA A+ or B). */
+M68K_FEATURE_CF_ISA_A, /* Base Coldfire set Rev A. */
+M68K_FEATURE_CF_ISA_B, /* (ISA B or C). */
+M68K_FEATURE_CF_ISA_APLUSC, /* BIT/BITREV, FF1, STRLDSR (ISA A+ or C). */
+M68K_FEATURE_BRAL, /* BRA with Long branch. (680[2346]0, ISA A+ or B). */
M68K_FEATURE_CF_FPU,
M68K_FEATURE_CF_MAC,
M68K_FEATURE_CF_EMAC,
-M68K_FEATURE_CF_EMAC_B, /* Revision B EMAC (dual accumulate). */
-M68K_FEATURE_USP, /* User Stack Pointer. (ISA A+, B or C). */
-M68K_FEATURE_EXT_FULL, /* 68020+ full extension word. */
-M68K_FEATURE_WORD_INDEX, /* word sized address index registers. */
-M68K_FEATURE_SCALED_INDEX, /* scaled address index registers. */
-M68K_FEATURE_LONG_MULDIV, /* 32 bit multiply/divide. */
-M68K_FEATURE_QUAD_MULDIV, /* 64 bit multiply/divide. */
-M68K_FEATURE_BCCL, /* Long conditional branches. */
-M68K_FEATURE_BITFIELD, /* Bit field insns. */
-M68K_FEATURE_FPU,
-M68K_FEATURE_CAS,
-M68K_FEATURE_BKPT,
-M68K_FEATURE_RTD,
-M68K_FEATURE_CHK2,
-M68K_FEATURE_MOVEP,
+M68K_FEATURE_CF_EMAC_B, /* Revision B EMAC (dual accumulate). */
+M68K_FEATURE_USP, /* User Stack Pointer. (680[012346]0, ISA A+, B or C).*/
+M68K_FEATURE_EXT_FULL,/* 68020+ full extension word. */
+M68K_FEATURE_WORD_INDEX, /* word sized address index registers. */
+M68K_FEATURE_SCALED_INDEX, /* scaled address index registers. */
+M68K_FEATURE_LONG_MULDIV, /* 32 bit mul/div. (680[2346]0, and CPU32) */
+M68K_FEATURE_QUAD_MULDIV, /* 64 bit mul/div. (680[2346]0, and CPU32) */
+M68K_FEATURE_BCCL, /* Bcc with Long branches. (680[2346]0, and CPU32) */
+M68K_FEATURE_BITFIELD, /* BFxxx Bit field insns. (680[2346]0) */
+M68K_FEATURE_FPU, /* fpu insn. (680[46]0) */
+M68K_FEATURE_CAS, /* CAS/CAS2[WL] insns. (680[2346]0) */
+M68K_FEATURE_BKPT, /* BKPT insn. (680[12346]0, and CPU32) */
+M68K_FEATURE_RTD, /* RTD insn. (680[12346]0, and CPU32) */
+M68K_FEATURE_CHK2, /* CHK2 insn. (680[2346]0, and CPU32) */
+M68K_FEATURE_MOVEP, /* MOVEP insn. (680[01234]0, and CPU32) */
};
static inline int m68k_feature(CPUM68KState *env, int feature)
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index c6fde8132b2f..5c72f2469471 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -103,6 +103,7 @@ static void m5206_cpu_initfn(Object *obj)
m68k_set_feature(env, M68K_FEATURE_CF_ISA_A);
}
+/* Base feature set, including isns. for m68k family */
static void m68000_cpu_initfn(Object *obj)
{
M68kCPU *cpu = M68K_CPU(obj);
@@ -135,6 +136,13 @@ static void m680x0_cpu_common(CPUM68KState *env)
m68k_set_feature(env, M68K_FEATURE_MOVEP);
}
+/*
+ * Adds BFCHG, BFCLR, BFEXTS, BFEXTU, BFFFO, BFINS, BFSET, BFTST, CAS, CAS2,
+ * CHK2, CMP2, DIVSL, DIVUL, EXTB, PACK, TRAPcc, UNPK.
+ *
+ * 68020/30 only:
+ * CALLM, cpBcc, cpDBcc, cpGEN, cpRESTORE, cpSAVE, cpScc, cpTRAPcc
+ */
static void m68020_cpu_initfn(Object *obj)
{
M68kCPU *cpu = M68K_CPU(obj);
@@ -144,6
[PULL 4/7] m68k: add missing BUSCR/PCR CR defines, and BUSCR/PCR/CAAR CR to m68k_move_to/from
From: Lucien Murray-Pitts
The BUSCR/PCR CR defines were missing for 68060, and the move_to/from helper
functions were also missing a decode for the 68060 M68K_CR_CAAR CR register.
Added missing defines, and respective decodes for all three CR registers to
the helpers.
Although this patch defines them, the implementation is empty in this patch
and these registers will result in a cpu abort - which is the default prior
to this patch.
This patch aims to reach full coverage of all CR registers within the helpers.
Signed-off-by: Lucien Murray-Pitts
Reviewed-by: Laurent Vivier
Signed-off-by: BALATON Zoltan
Reviewed-by: Philippe Mathieu-Daudé
Message-Id:
<19e5c0fa8baed6479ed0502fd3deb132d19457fb.1612137712.git.bala...@eik.bme.hu>
Signed-off-by: Laurent Vivier
---
target/m68k/cpu.h| 4
target/m68k/helper.c | 10 ++
2 files changed, 14 insertions(+)
diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index 2b1cdf241bab..ae34c9461503 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -393,6 +393,10 @@ typedef enum {
#define M68K_CR_DACR00x006
#define M68K_CR_DACR10x007
+/* MC68060 */
+#define M68K_CR_BUSCR0x008
+#define M68K_CR_PCR 0x808
+
#define M68K_FPIAR_SHIFT 0
#define M68K_FPIAR(1 << M68K_FPIAR_SHIFT)
#define M68K_FPSR_SHIFT 1
diff --git a/target/m68k/helper.c b/target/m68k/helper.c
index 9e81ee53ad8b..69acdc3b353c 100644
--- a/target/m68k/helper.c
+++ b/target/m68k/helper.c
@@ -255,6 +255,11 @@ void HELPER(m68k_movec_to)(CPUM68KState *env, uint32_t
reg, uint32_t val)
case M68K_CR_DTT1:
env->mmu.ttr[M68K_DTTR1] = val;
return;
+/* Unimplemented Registers */
+case M68K_CR_CAAR:
+case M68K_CR_PCR:
+case M68K_CR_BUSCR:
+break;
}
cpu_abort(env_cpu(env),
"Unimplemented control register write 0x%x = 0x%x\n",
@@ -309,6 +314,11 @@ uint32_t HELPER(m68k_movec_from)(CPUM68KState *env,
uint32_t reg)
/* MC68040/MC68LC040 */
case M68K_CR_DTT1: /* MC68EC040 only: M68K_CR_DACR1 */
return env->mmu.ttr[M68K_DTTR1];
+/* Unimplemented Registers */
+case M68K_CR_CAAR:
+case M68K_CR_PCR:
+case M68K_CR_BUSCR:
+break;
}
cpu_abort(env_cpu(env), "Unimplemented control register read 0x%x\n",
reg);
--
2.29.2
[PULL 0/7] M68k for 6.0 patches
The following changes since commit 5b19cb63d9dfda41b412373b8c9fe14641bcab60: Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210205' in= to staging (2021-02-05 22:59:12 +) are available in the Git repository at: git://github.com/vivier/qemu-m68k.git tags/m68k-for-6.0-pull-request for you to fetch changes up to 382d71af7d61620ffb023962f83cc4786303c60d: m68k: import bootinfo headers from linux (2021-02-11 21:56:42 +0100) Pull request m68k-20210212 Move bootinfo headers to include/standard-headers/asm-m68k Add M68K_FEATURE_MSP, M68K_FEATURE_MOVEC, M68K_FEATURE_M68010 Add 68060 CR BUSCR and PCR (unimplemented) CPU types and features cleanup Laurent Vivier (1): m68k: import bootinfo headers from linux Lucien Murray-Pitts (6): m68k: improve cpu instantiation comments m68k: cascade m68k_features by m680xx_cpu_initfn() to improve readability m68k: improve comments on m68k_move_to/from helpers m68k: add missing BUSCR/PCR CR defines, and BUSCR/PCR/CAAR CR to m68k_move_to/from m68k: MOVEC insn. should generate exception if wrong CR is accessed m68k: add MSP detection support for stack pointer swap helpers hw/m68k/bootinfo.h| 55 .../standard-headers/asm-m68k/bootinfo-mac.h | 120 + include/standard-headers/asm-m68k/bootinfo.h | 166 + target/m68k/cpu.h | 64 +++-- hw/m68k/q800.c| 20 +- target/m68k/cpu.c | 116 ++--- target/m68k/helper.c | 234 ++ target/m68k/translate.c | 2 +- MAINTAINERS | 2 + 9 files changed, 604 insertions(+), 175 deletions(-) create mode 100644 include/standard-headers/asm-m68k/bootinfo-mac.h create mode 100644 include/standard-headers/asm-m68k/bootinfo.h --=20 2.29.2
Re: [PATCH] virtiofs_submounts.py test: Note on vmlinuz param
On Fri, Feb 12, 2021 at 12:20 PM Max Reitz wrote: > > From the cancel message, it is not entirely clear why this parameter is > mandatory now, or that it will be optional in the future. Add such a > more detailed explanation as a comment in the test source file. > > Suggested-by: Alex Bennée > Signed-off-by: Max Reitz > --- > I’ve uploaded a build of Linux 5.10 here: > https://xanclic.moe/linux-5.10 > > But I’ve decided against mentioning it in this new comment or the cancel > message, because, well, it’s my private server and I have limited > bandwidth. > --- > tests/acceptance/virtiofs_submounts.py | 12 > 1 file changed, 12 insertions(+) Reviewed-by: Willian Rampazzo
Re: [PATCH v3 07/10] hw/i386: declare ACPI mother board resource for MMCONFIG region
On Fri, Feb 12, 2021 at 04:40:38PM +0100,
Igor Mammedov wrote:
> On Wed, 10 Feb 2021 22:46:43 -0800
> Isaku Yamahata wrote:
>
> > Declare PNP0C01 device to reserve MMCONFIG region to conform to the
> > spec better and play nice with guest BIOSes/OSes.
> >
> > According to PCI Firmware Specification[0], MMCONFIG region must be
> > reserved by declaring a motherboard resource. It's optional to reserve
> > the region in memory map by Int 15 E820h or EFIGetMemoryMap.
> > Guest Linux checks if the MMCFG region is reserved by bios memory map
> > or ACPI resource. If it's not reserved, Linux falls back to legacy PCI
> > configuration access.
> >
> > TDVF [1] [2] doesn't reserve MMCONFIG the region in memory map.
> > On the other hand OVMF reserves it in memory map without declaring a
> > motherboard resource. With memory map reservation, linux guest uses
> > MMCONFIG region. However it doesn't comply to PCI Firmware
> > specification.
> >
> > [0] PCI Firmware specification Revision 3.2
> > 4.1.2 MCFG Table Description table 4-2 NOTE 2
> > If the operating system does not natively comprehend reserving the
> > MMCFG region, The MMCFG region must e reserved by firmware. ...
> > For most systems, the mortheroard resource would appear at the root
> > of the ACPI namespace (under \_SB)...
> > The resource can optionally be returned in Int15 E820h or
> > EFIGetMemoryMap as reserved memory but must always be reported
> > through ACPI as a motherboard resource
> >
> > [1] TDX: Intel Trust Domain Extension
> >
> > https://software.intel.com/content/www/us/en/develop/articles/intel-trust-domain-extensions.html
> > [2] TDX Virtual Firmware
> > https://github.com/tianocore/edk2-staging/tree/TDVF
> >
> > The change to DSDT is as follows.
> > @@ -68,32 +68,51 @@
> >
> > If ((CDW3 != Local0))
> > {
> > CDW1 |= 0x10
> > }
> >
> > CDW3 = Local0
> > }
> > Else
> > {
> > CDW1 |= 0x04
> > }
> >
> > Return (Arg3)
> > }
> > }
> > +
> > +Device (DRAC)
> > +{
> > +Name (_HID, "PNP0C01" /* System Board */) // _HID: Hardware ID
> > +Name (RBUF, ResourceTemplate ()
> > +{
> > +QWordMemory (ResourceProducer, PosDecode, MinFixed,
> > MaxFixed, NonCacheable, ReadWrite,
> > +0x, // Granularity
> > +0xB000, // Range Minimum
> > +0xBFFF, // Range Maximum
> > +0x, // Translation Offset
> > +0x1000, // Length
> > +,, , AddressRangeMemory, TypeStatic)
> > +})
> > +Method (_CRS, 0, Serialized) // _CRS: Current Resource
> > Settings
> > +{
> > +Return (RBUF) /* \_SB_.DRAC.RBUF */
> > +}
> > +}
> > }
> >
> > Scope (_SB)
> > {
> > Device (HPET)
> > {
> > Name (_HID, EisaId ("PNP0103") /* HPET System Timer */) //
> > _HID: Hardware ID
> > Name (_UID, Zero) // _UID: Unique ID
> > OperationRegion (HPTM, SystemMemory, 0xFED0, 0x0400)
> > Field (HPTM, DWordAcc, Lock, Preserve)
> > {
> > VEND, 32,
> > PRD,32
> > }
> >
> > Method (_STA, 0, NotSerialized) // _STA: Status
> >
> > Signed-off-by: Isaku Yamahata
> > ---
> > hw/i386/acpi-build.c | 46 +++-
> > 1 file changed, 45 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 00cc119362..e369908b1a 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1072,6 +1072,46 @@ static void build_q35_pci0_int(Aml *table)
> > aml_append(table, sb_scope);
> > }
> >
> > +static Aml *build_q35_dram_controller(void)
> > +{
> > +AcpiMcfgInfo mcfg;
> build_dsdt() already calls acpi_get_mcfg(),
> I suggest to cache it there and pass to build_q35_dram_controller
> as an argument.
Sure.
> > +Aml *dev;
> > +Aml *rbuf;
> > +Aml *resource_template;
> > +Aml *rbuf_name;
> > +Aml *crs;
> > +
> > +if (!acpi_get_mcfg()) {
> > +return NULL;
> > +}
> > +
> > +/* DRAM controller */
> > +dev = aml_device("DRAC");
> > +aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C01")));
> > +
> > +resource_template = aml_resource_template();
> > +aml_append(resource_template,
> > + aml_qword_memory(AML_POS_DECODE,
> > +AML_MIN_FIXED,
> > +AML_MAX_FIXED,
> > +AML_NON_CACHEABLE,
> > +
Re: [PATCH] virtiofs_submounts.py test: Note on vmlinuz param
Hi Cleber,
On 2/12/21 7:58 PM, Cleber Rosa wrote:
> On Fri, Feb 12, 2021 at 04:16:49PM +0100, Max Reitz wrote:
>> From the cancel message, it is not entirely clear why this parameter is
>> mandatory now, or that it will be optional in the future. Add such a
>> more detailed explanation as a comment in the test source file.
>>
>> Suggested-by: Alex Bennée
>> Signed-off-by: Max Reitz
>> ---
>> I’ve uploaded a build of Linux 5.10 here:
>> https://xanclic.moe/linux-5.10
>>
>> But I’ve decided against mentioning it in this new comment or the cancel
>> message, because, well, it’s my private server and I have limited
>> bandwidth.
>> ---
>> tests/acceptance/virtiofs_submounts.py | 12
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/tests/acceptance/virtiofs_submounts.py
>> b/tests/acceptance/virtiofs_submounts.py
>> index 949ca87a83..9a69b6b17b 100644
>> --- a/tests/acceptance/virtiofs_submounts.py
>> +++ b/tests/acceptance/virtiofs_submounts.py
>> @@ -228,6 +228,18 @@ class VirtiofsSubmountsTest(BootLinux):
>> def setUp(self):
>> vmlinuz = self.params.get('vmlinuz')
>> if vmlinuz is None:
>> +"""
>> +The Linux kernel supports FUSE auto-submounts only as of 5.10.
>> +boot_linux.py currently provides Fedora 31, whose kernel is too
>> +old, so this test cannot pass with the on-image kernel (you are
>> +welcome to try, hence the option to force such a test with
>> +-p vmlinuz=''). Therefore, for now the user must provide a
>> +sufficiently new custom kernel, or effectively explicitly
>> +request failure with -p vmlinuz=''.
>> +Once an image with a sufficiently new kernel is available
>> +(probably Fedora 34), we can make -p vmlinuz='' the default, so
>> +that this parameter no longer needs to be specified.
>> +"""
>> self.cancel('vmlinuz parameter not set; you must point it to a '
>> 'Linux kernel binary to test (to run this test with
>> ' \
>> 'the on-image kernel, set it to an empty string)')
>> --
>> 2.29.2
>>
>
> Hi Max,
>
> This looks good to me, and I've also tested your kernel build and
> works like a charm.
>
> As further work on top of this, it may be beneficial to have test
> documentation in a predictable place. The possibilities that come to
> my mind:
>
> * docs/devel/testing.rst
> * tests/acceptance/$test_file.py/data/README
>
> On a different topic, the "https://avocado-project.org/data/assets; has
> enough bandwidth and can be used to hold this type asset.
Can you define "this type asset" please?
> Alternatively,
> we can add a bit more automation to this test by letting people do something
> like:
>
> $ avocado assets register virtiofs-auto-submounts-vmlinuz /path/to/vmlinuz
>
> And on the test:
>
> vmlinuz = self.fetch_asset('virtiofs-auto-submounts-vmlinuz')
>
> And the test should cancel if that asset has not been previously registered.
Yay, great news!
Re: [PATCH v7 17/31] linux-user: Move lock_user et al out of line
On 2/12/21 7:48 PM, Richard Henderson wrote: > These functions are not small, except for unlock_user > without debugging enabled. Move them out of line, and > add missing braces on the way. > > Reviewed-by: Peter Maydell > Signed-off-by: Richard Henderson > --- > linux-user/qemu.h| 45 ++- > linux-user/uaccess.c | 46 > 2 files changed, 52 insertions(+), 39 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v7 18/31] linux-user: Fix types in uaccess.c
On 2/12/21 7:48 PM, Richard Henderson wrote: > For copy_*_user, only 0 and -TARGET_EFAULT are returned; no need > to involve abi_long. Use size_t for lengths. Use bool for the > lock_user copy argument. Use ssize_t for target_strlen, because > we can't overflow the host memory space. > > Reviewed-by: Peter Maydell > Signed-off-by: Richard Henderson > --- > linux-user/qemu.h| 14 ++ > linux-user/uaccess.c | 45 ++-- > 2 files changed, 29 insertions(+), 30 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 20/22] Acceptance Tests: add basic documentation on LinuxTest base class
On Wed, Feb 3, 2021 at 2:25 PM Cleber Rosa wrote: > > Signed-off-by: Cleber Rosa > --- > docs/devel/testing.rst | 26 ++ > 1 file changed, 26 insertions(+) > Reviewed-by: Willian Rampazzo
[PULL 13/14] block/backup: implement .cancel job handler
From: Vladimir Sementsov-Ogievskiy
Cancel in-flight io on target to not waste the time.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
Message-Id: <20210205163720.887197-10-vsement...@virtuozzo.com>
Signed-off-by: Eric Blake
---
block/backup.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/block/backup.c b/block/backup.c
index cc525d55445e..94e6dcd72e33 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -35,6 +35,7 @@ typedef struct BackupBlockJob {
BlockJob common;
BlockDriverState *backup_top;
BlockDriverState *source_bs;
+BlockDriverState *target_bs;
BdrvDirtyBitmap *sync_bitmap;
@@ -329,6 +330,13 @@ static void coroutine_fn backup_set_speed(BlockJob *job,
int64_t speed)
}
}
+static void backup_cancel(Job *job)
+{
+BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
+
+bdrv_cancel_in_flight(s->target_bs);
+}
+
static const BlockJobDriver backup_job_driver = {
.job_driver = {
.instance_size = sizeof(BackupBlockJob),
@@ -340,6 +348,7 @@ static const BlockJobDriver backup_job_driver = {
.abort = backup_abort,
.clean = backup_clean,
.pause = backup_pause,
+.cancel = backup_cancel,
},
.set_speed = backup_set_speed,
};
@@ -528,6 +537,7 @@ BlockJob *backup_job_create(const char *job_id,
BlockDriverState *bs,
job->backup_top = backup_top;
job->source_bs = bs;
+job->target_bs = target;
job->on_source_error = on_source_error;
job->on_target_error = on_target_error;
job->sync_mode = sync_mode;
--
2.30.1
Re: [PATCH 0/3] hw/display/sm501: Tidy up template header
On Fri, 12 Feb 2021, Peter Maydell wrote: For a long time now the UI layer has guaranteed that the console surface is always 32 bits per pixel RGB, but some older display device models still have the code to handle other formats. This patchset cleans up that dead code for the sm501 device, which allows us to remove the multiply-included sm501_template.h header entirely. There have been a few attempts at doing this cleanup on this device in the past by various people; rather than trying to resurrect those old patchsets and get them to apply to the current code in master, I just started from scratch. Tested with AROS ISO image on sam460ex. thanks -- PMM Peter Maydell (3): hw/display/sm501: Remove dead code for non-32-bit RGB surfaces hw/display/sm501: Expand out macros in template header hw/display/sm501: Inline template header into C file I've tried with AmigaOS and MorphOS and those also work. Unfortunately the drivers for sm501 on those are restricted to 16bit modes (maybe because real hardware is too slow otherwise or does not have enough memory) so every screen update in QEMU needs conversion which makes it quite slow. But this was like that before and unless we want to allow other than 32bit surfaces again we can't use the code removed here but that was the reason I've kept it and not removed so far in case we want to do this optimisation again. Otherwise, Acked-by: BALATON Zoltan The sm501 is also used on the SH4 r2d machine I think. Aurelien probably knows more about that. I've found some images for it here: https://people.debian.org/~aurel32/qemu/sh4/ https://lists.nongnu.org/archive/html/qemu-devel/2008-08/msg01308.html in case you want to test that too. Regards, BALATON Zoltan hw/display/sm501_template.h | 131 hw/display/sm501.c | 166 ++-- 2 files changed, 83 insertions(+), 214 deletions(-) delete mode 100644 hw/display/sm501_template.h
[PULL 07/14] block/raw-format: implement .bdrv_cancel_in_flight handler
From: Vladimir Sementsov-Ogievskiy
We are going to cancel in-flight requests on mirror nbd target on job
cancel. Still nbd is often used not directly but as raw-format child.
So, add pass-through handler here.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
Message-Id: <20210205163720.887197-4-vsement...@virtuozzo.com>
Signed-off-by: Eric Blake
---
block/raw-format.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/block/raw-format.c b/block/raw-format.c
index 42ec50802bcd..7717578ed6ab 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -575,6 +575,11 @@ static const char *const raw_strong_runtime_opts[] = {
NULL
};
+static void raw_cancel_in_flight(BlockDriverState *bs)
+{
+bdrv_cancel_in_flight(bs->file->bs);
+}
+
BlockDriver bdrv_raw = {
.format_name = "raw",
.instance_size= sizeof(BDRVRawState),
@@ -608,6 +613,7 @@ BlockDriver bdrv_raw = {
.bdrv_has_zero_init = _has_zero_init,
.strong_runtime_opts = raw_strong_runtime_opts,
.mutable_opts = mutable_opts,
+.bdrv_cancel_in_flight = raw_cancel_in_flight,
};
static void bdrv_raw_init(void)
--
2.30.1
Re: [PATCH 1/2] tests/acceptance: replace unstable apt.armbian.com URLs for orangepi-pc, cubieboard
On Thu, Feb 11, 2021 at 7:09 PM Niek Linnenbank wrote: > > Currently the automated acceptance tests for the Orange Pi PC and cubieboard > machines are disabled by default. The tests for both machines require > artifacts > that are stored on the apt.armbian.com domain. Unfortunately, some of these > artifacts > have been removed from apt.armbian.com and it is uncertain whether more will > be removed. > > This commit moves the artifacts previously stored on apt.armbian.com to a > different > domain that is maintainted by me and retrieves them using the path: > '/pub/qemu//'. > > Signed-off-by: Niek Linnenbank > --- > tests/acceptance/boot_linux_console.py | 46 +- > tests/acceptance/replay_kernel.py | 6 ++-- > 2 files changed, 18 insertions(+), 34 deletions(-) Reviewed-by: Willian Rampazzo
[PULL 14/14] iotests/264: add backup-cancel test-case
From: Vladimir Sementsov-Ogievskiy
Check that cancel doesn't wait for 10s of nbd reconnect timeout.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
Message-Id: <20210205163720.887197-11-vsement...@virtuozzo.com>
Signed-off-by: Eric Blake
---
tests/qemu-iotests/264 | 21 ++---
tests/qemu-iotests/264.out | 4 ++--
2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/tests/qemu-iotests/264 b/tests/qemu-iotests/264
index 347e53add51b..4f96825a22c2 100755
--- a/tests/qemu-iotests/264
+++ b/tests/qemu-iotests/264
@@ -94,20 +94,27 @@ class TestNbdReconnect(iotests.QMPTestCase):
result = self.vm.qmp('blockdev-del', node_name='backup0')
self.assert_qmp(result, 'return', {})
+def cancel_job(self):
+result = self.vm.qmp('block-job-cancel', device='drive0')
+self.assert_qmp(result, 'return', {})
+
+start_t = time.time()
+self.vm.event_wait('BLOCK_JOB_CANCELLED')
+delta_t = time.time() - start_t
+self.assertTrue(delta_t < 2.0)
+
def test_mirror_cancel(self):
# Mirror speed limit doesn't work well enough, it seems that mirror
# will run many parallel requests anyway. MAX_IN_FLIGHT is 16 and
# MAX_IO_BYTES is 1M in mirror.c, so let's use 20M disk.
self.init_vm(20 * 1024 * 1024)
self.start_job('blockdev-mirror')
+self.cancel_job()
-result = self.vm.qmp('block-job-cancel', device='drive0')
-self.assert_qmp(result, 'return', {})
-
-start_t = time.time()
-self.vm.event_wait('BLOCK_JOB_CANCELLED')
-delta_t = time.time() - start_t
-self.assertTrue(delta_t < 2.0)
+def test_backup_cancel(self):
+self.init_vm(5 * 1024 * 1024)
+self.start_job('blockdev-backup')
+self.cancel_job()
if __name__ == '__main__':
diff --git a/tests/qemu-iotests/264.out b/tests/qemu-iotests/264.out
index fbc63e62f885..8d7e99670093 100644
--- a/tests/qemu-iotests/264.out
+++ b/tests/qemu-iotests/264.out
@@ -1,5 +1,5 @@
-..
+...
--
-Ran 2 tests
+Ran 3 tests
OK
--
2.30.1
[PULL 05/14] block: add new BlockDriver handler: bdrv_cancel_in_flight
From: Vladimir Sementsov-Ogievskiy
It will be used to stop retrying NBD requests on mirror cancel.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
Message-Id: <20210205163720.887197-2-vsement...@virtuozzo.com>
Signed-off-by: Eric Blake
---
include/block/block.h | 3 +++
include/block/block_int.h | 9 +
block/io.c| 11 +++
3 files changed, 23 insertions(+)
diff --git a/include/block/block.h b/include/block/block.h
index 0a9f2c187cdb..2f2698074e30 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -849,4 +849,7 @@ int coroutine_fn bdrv_co_copy_range(BdrvChild *src, int64_t
src_offset,
BdrvChild *dst, int64_t dst_offset,
int64_t bytes, BdrvRequestFlags read_flags,
BdrvRequestFlags write_flags);
+
+void bdrv_cancel_in_flight(BlockDriverState *bs);
+
#endif
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 22a2789d3516..88e411193981 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -352,6 +352,15 @@ struct BlockDriver {
bool want_zero, int64_t offset, int64_t bytes, int64_t *pnum,
int64_t *map, BlockDriverState **file);
+/*
+ * This informs the driver that we are no longer interested in the result
+ * of in-flight requests, so don't waste the time if possible.
+ *
+ * One example usage is to avoid waiting for an nbd target node reconnect
+ * timeout during job-cancel.
+ */
+void (*bdrv_cancel_in_flight)(BlockDriverState *bs);
+
/*
* Invalidate any cached meta-data.
*/
diff --git a/block/io.c b/block/io.c
index b0435ed6707c..ca2dca30070e 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3460,3 +3460,14 @@ out:
return ret;
}
+
+void bdrv_cancel_in_flight(BlockDriverState *bs)
+{
+if (!bs || !bs->drv) {
+return;
+}
+
+if (bs->drv->bdrv_cancel_in_flight) {
+bs->drv->bdrv_cancel_in_flight(bs);
+}
+}
--
2.30.1
[PULL 12/14] iotests/264: add mirror-cancel test-case
From: Vladimir Sementsov-Ogievskiy
Check that cancel doesn't wait for 10s of nbd reconnect timeout.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
Message-Id: <20210205163720.887197-9-vsement...@virtuozzo.com>
Signed-off-by: Eric Blake
---
tests/qemu-iotests/264 | 38 ++
tests/qemu-iotests/264.out | 4 ++--
2 files changed, 32 insertions(+), 10 deletions(-)
diff --git a/tests/qemu-iotests/264 b/tests/qemu-iotests/264
index 6feeaa405632..347e53add51b 100755
--- a/tests/qemu-iotests/264
+++ b/tests/qemu-iotests/264
@@ -27,25 +27,26 @@ from iotests import qemu_img_create, file_path,
qemu_nbd_popen
disk_a, disk_b, nbd_sock = file_path('disk_a', 'disk_b', 'nbd-sock')
nbd_uri = 'nbd+unix:///?socket=' + nbd_sock
-size = 5 * 1024 * 1024
wait_limit = 3.0
wait_step = 0.2
class TestNbdReconnect(iotests.QMPTestCase):
-def setUp(self):
-qemu_img_create('-f', iotests.imgfmt, disk_a, str(size))
-qemu_img_create('-f', iotests.imgfmt, disk_b, str(size))
+def init_vm(self, disk_size):
+qemu_img_create('-f', iotests.imgfmt, disk_a, str(disk_size))
+qemu_img_create('-f', iotests.imgfmt, disk_b, str(disk_size))
self.vm = iotests.VM().add_drive(disk_a)
self.vm.launch()
-self.vm.hmp_qemu_io('drive0', 'write 0 {}'.format(size))
+self.vm.hmp_qemu_io('drive0', 'write 0 {}'.format(disk_size))
def tearDown(self):
self.vm.shutdown()
os.remove(disk_a)
os.remove(disk_b)
-def test(self):
+def start_job(self, job):
+"""Stat job with nbd target and kill the server"""
+assert job in ('blockdev-backup', 'blockdev-mirror')
with qemu_nbd_popen('-k', nbd_sock, '-f', iotests.imgfmt, disk_b):
result = self.vm.qmp('blockdev-add',
**{'node_name': 'backup0',
@@ -55,7 +56,7 @@ class TestNbdReconnect(iotests.QMPTestCase):
'path': nbd_sock},
'reconnect-delay': 10}})
self.assert_qmp(result, 'return', {})
-result = self.vm.qmp('blockdev-backup', device='drive0',
+result = self.vm.qmp(job, device='drive0',
sync='full', target='backup0',
speed=(1 * 1024 * 1024))
self.assert_qmp(result, 'return', {})
@@ -73,7 +74,8 @@ class TestNbdReconnect(iotests.QMPTestCase):
jobs = self.vm.qmp('query-block-jobs')['return']
# Check that job is still in progress
-self.assertTrue(jobs and jobs[0]['offset'] < jobs[0]['len'])
+self.assertTrue(jobs)
+self.assertTrue(jobs[0]['offset'] < jobs[0]['len'])
result = self.vm.qmp('block-job-set-speed', device='drive0', speed=0)
self.assert_qmp(result, 'return', {})
@@ -81,12 +83,32 @@ class TestNbdReconnect(iotests.QMPTestCase):
# Emulate server down time for 1 second
time.sleep(1)
+def test_backup(self):
+size = 5 * 1024 * 1024
+self.init_vm(size)
+self.start_job('blockdev-backup')
+
with qemu_nbd_popen('-k', nbd_sock, '-f', iotests.imgfmt, disk_b):
e = self.vm.event_wait('BLOCK_JOB_COMPLETED')
self.assertEqual(e['data']['offset'], size)
result = self.vm.qmp('blockdev-del', node_name='backup0')
self.assert_qmp(result, 'return', {})
+def test_mirror_cancel(self):
+# Mirror speed limit doesn't work well enough, it seems that mirror
+# will run many parallel requests anyway. MAX_IN_FLIGHT is 16 and
+# MAX_IO_BYTES is 1M in mirror.c, so let's use 20M disk.
+self.init_vm(20 * 1024 * 1024)
+self.start_job('blockdev-mirror')
+
+result = self.vm.qmp('block-job-cancel', device='drive0')
+self.assert_qmp(result, 'return', {})
+
+start_t = time.time()
+self.vm.event_wait('BLOCK_JOB_CANCELLED')
+delta_t = time.time() - start_t
+self.assertTrue(delta_t < 2.0)
+
if __name__ == '__main__':
iotests.main(supported_fmts=['qcow2'])
diff --git a/tests/qemu-iotests/264.out b/tests/qemu-iotests/264.out
index ae1213e6f863..fbc63e62f885 100644
--- a/tests/qemu-iotests/264.out
+++ b/tests/qemu-iotests/264.out
@@ -1,5 +1,5 @@
-.
+..
--
-Ran 1 tests
+Ran 2 tests
OK
--
2.30.1
[PULL 09/14] block/mirror: implement .cancel job handler
From: Vladimir Sementsov-Ogievskiy
Cancel in-flight io on target to not waste the time.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
Message-Id: <20210205163720.887197-6-vsement...@virtuozzo.com>
Signed-off-by: Eric Blake
---
block/mirror.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/block/mirror.c b/block/mirror.c
index 8e1ad6eceb57..9faffe470774 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1179,6 +1179,14 @@ static bool mirror_drained_poll(BlockJob *job)
return !!s->in_flight;
}
+static void mirror_cancel(Job *job)
+{
+MirrorBlockJob *s = container_of(job, MirrorBlockJob, common.job);
+BlockDriverState *target = blk_bs(s->target);
+
+bdrv_cancel_in_flight(target);
+}
+
static const BlockJobDriver mirror_job_driver = {
.job_driver = {
.instance_size = sizeof(MirrorBlockJob),
@@ -1190,6 +1198,7 @@ static const BlockJobDriver mirror_job_driver = {
.abort = mirror_abort,
.pause = mirror_pause,
.complete = mirror_complete,
+.cancel = mirror_cancel,
},
.drained_poll = mirror_drained_poll,
};
--
2.30.1
[PULL 10/14] iotests/264: move to python unittest
From: Vladimir Sementsov-Ogievskiy
We are going to add more test cases, so use the library supporting test
cases.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
Message-Id: <20210205163720.887197-7-vsement...@virtuozzo.com>
Signed-off-by: Eric Blake
---
tests/qemu-iotests/264 | 109 +
tests/qemu-iotests/264.out | 20 ++-
2 files changed, 66 insertions(+), 63 deletions(-)
diff --git a/tests/qemu-iotests/264 b/tests/qemu-iotests/264
index e725cefd47b5..6feeaa405632 100755
--- a/tests/qemu-iotests/264
+++ b/tests/qemu-iotests/264
@@ -20,13 +20,10 @@
#
import time
+import os
import iotests
-from iotests import qemu_img_create, file_path, qemu_nbd_popen, log
-
-iotests.script_initialize(
-supported_fmts=['qcow2'],
-)
+from iotests import qemu_img_create, file_path, qemu_nbd_popen
disk_a, disk_b, nbd_sock = file_path('disk_a', 'disk_b', 'nbd-sock')
nbd_uri = 'nbd+unix:///?socket=' + nbd_sock
@@ -34,46 +31,62 @@ size = 5 * 1024 * 1024
wait_limit = 3.0
wait_step = 0.2
-qemu_img_create('-f', iotests.imgfmt, disk_a, str(size))
-qemu_img_create('-f', iotests.imgfmt, disk_b, str(size))
-
-with qemu_nbd_popen('-k', nbd_sock, '-f', iotests.imgfmt, disk_b):
-vm = iotests.VM().add_drive(disk_a)
-vm.launch()
-vm.hmp_qemu_io('drive0', 'write 0 {}'.format(size))
-
-vm.qmp_log('blockdev-add', filters=[iotests.filter_qmp_testfiles],
- **{'node_name': 'backup0',
- 'driver': 'raw',
- 'file': {'driver': 'nbd',
- 'server': {'type': 'unix', 'path': nbd_sock},
- 'reconnect-delay': 10}})
-vm.qmp_log('blockdev-backup', device='drive0', sync='full',
- target='backup0', speed=(1 * 1024 * 1024))
-
-# Wait for some progress
-t = 0.0
-while t < wait_limit:
-jobs = vm.qmp('query-block-jobs')['return']
-if jobs and jobs[0]['offset'] > 0:
-break
-time.sleep(wait_step)
-t += wait_step
-
-if jobs and jobs[0]['offset'] > 0:
-log('Backup job is started')
-
-jobs = vm.qmp('query-block-jobs')['return']
-if jobs and jobs[0]['offset'] < jobs[0]['len']:
-log('Backup job is still in progress')
-
-vm.qmp_log('block-job-set-speed', device='drive0', speed=0)
-
-# Emulate server down time for 1 second
-time.sleep(1)
-
-with qemu_nbd_popen('-k', nbd_sock, '-f', iotests.imgfmt, disk_b):
-e = vm.event_wait('BLOCK_JOB_COMPLETED')
-log('Backup completed: {}'.format(e['data']['offset']))
-vm.qmp_log('blockdev-del', node_name='backup0')
-vm.shutdown()
+
+class TestNbdReconnect(iotests.QMPTestCase):
+def setUp(self):
+qemu_img_create('-f', iotests.imgfmt, disk_a, str(size))
+qemu_img_create('-f', iotests.imgfmt, disk_b, str(size))
+self.vm = iotests.VM().add_drive(disk_a)
+self.vm.launch()
+self.vm.hmp_qemu_io('drive0', 'write 0 {}'.format(size))
+
+def tearDown(self):
+self.vm.shutdown()
+os.remove(disk_a)
+os.remove(disk_b)
+
+def test(self):
+with qemu_nbd_popen('-k', nbd_sock, '-f', iotests.imgfmt, disk_b):
+result = self.vm.qmp('blockdev-add',
+ **{'node_name': 'backup0',
+'driver': 'raw',
+'file': {'driver': 'nbd',
+ 'server': {'type': 'unix',
+'path': nbd_sock},
+ 'reconnect-delay': 10}})
+self.assert_qmp(result, 'return', {})
+result = self.vm.qmp('blockdev-backup', device='drive0',
+ sync='full', target='backup0',
+ speed=(1 * 1024 * 1024))
+self.assert_qmp(result, 'return', {})
+
+# Wait for some progress
+t = 0.0
+while t < wait_limit:
+jobs = self.vm.qmp('query-block-jobs')['return']
+if jobs and jobs[0]['offset'] > 0:
+break
+time.sleep(wait_step)
+t += wait_step
+
+self.assertTrue(jobs and jobs[0]['offset'] > 0) # job started
+
+jobs = self.vm.qmp('query-block-jobs')['return']
+# Check that job is still in progress
+self.assertTrue(jobs and jobs[0]['offset'] < jobs[0]['len'])
+
+result = self.vm.qmp('block-job-set-speed', device='drive0', speed=0)
+self.assert_qmp(result, 'return', {})
+
+# Emulate server down time for 1 second
+time.sleep(1)
+
+with qemu_nbd_popen('-k', nbd_sock, '-f', iotests.imgfmt, disk_b):
+e = self.vm.event_wait('BLOCK_JOB_COMPLETED')
+self.assertEqual(e['data']['offset'], size)
+result = self.vm.qmp('blockdev-del', node_name='backup0')
+
[PULL 03/14] iotests/210: Fix reference output
From: Max Reitz
Commit 69b55e03f has changed an error message, adjust the reference
output to account for it.
Fixes: 69b55e03f7e65a36eb954d0b7d4698b258df2708
("block: refactor bdrv_check_request: add errp")
Signed-off-by: Max Reitz
Message-Id: <20210209181923.497688-1-mre...@redhat.com>
Reviewed-by: Eric Blake
Reviewed-by: Vladimir Sementsov-Ogievskiy
Signed-off-by: Eric Blake
---
tests/qemu-iotests/210.out | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/210.out b/tests/qemu-iotests/210.out
index dc1a3c9786ee..2e9fc596ebdf 100644
--- a/tests/qemu-iotests/210.out
+++ b/tests/qemu-iotests/210.out
@@ -182,7 +182,7 @@ Job failed: The requested file size is too large
=== Resize image with invalid sizes ===
{"execute": "block_resize", "arguments": {"node-name": "node1", "size":
9223372036854775296}}
-{"error": {"class": "GenericError", "desc": "Required too big image size, it
must be not greater than 9223372035781033984"}}
+{"error": {"class": "GenericError", "desc": "offset(9223372036854775296)
exceeds maximum(9223372035781033984)"}}
{"execute": "block_resize", "arguments": {"node-name": "node1", "size":
9223372036854775808}}
{"error": {"class": "GenericError", "desc": "Invalid parameter type for
'size', expected: integer"}}
{"execute": "block_resize", "arguments": {"node-name": "node1", "size":
18446744073709551104}}
--
2.30.1
[PULL 11/14] iotests.py: qemu_nbd_popen: remove pid file after use
From: Vladimir Sementsov-Ogievskiy
To not interfere with other qemu_nbd_popen() calls in same test.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
Message-Id: <20210205163720.887197-8-vsement...@virtuozzo.com>
Signed-off-by: Eric Blake
---
tests/qemu-iotests/iotests.py | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 00be68eca3e2..4e758308f247 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -296,7 +296,9 @@ def qemu_nbd_list_log(*args: str) -> str:
@contextmanager
def qemu_nbd_popen(*args):
'''Context manager running qemu-nbd within the context'''
-pid_file = file_path("pid")
+pid_file = file_path("qemu_nbd_popen-nbd-pid-file")
+
+assert not os.path.exists(pid_file)
cmd = list(qemu_nbd_args)
cmd.extend(('--persistent', '--pid-file', pid_file))
@@ -314,6 +316,8 @@ def qemu_nbd_popen(*args):
time.sleep(0.01)
yield
finally:
+if os.path.exists(pid_file):
+os.remove(pid_file)
log('Kill NBD server')
p.kill()
p.wait()
--
2.30.1
[PULL 00/14] NBD patches through 2021-02-12
The following changes since commit eac92d316351b855ba79eb374dd21cc367f1f9c1: Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210211-1' into staging (2021-02-11 19:57:50 +) are available in the Git repository at: https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2021-02-12 for you to fetch changes up to 594427fc56758cb944a85914eefe722cc2c667b8: iotests/264: add backup-cancel test-case (2021-02-12 12:19:46 -0600) nbd patches for 2021-02-12 - let qemu-nbd handle larger backlog of connecting clients - fix a few NBD-related iotest failures - add block cancellation hook for faster response to NBD failures Eric Blake (2): qemu-nbd: Use SOMAXCONN for socket listen() backlog qemu-nbd: Permit --shared=0 for unlimited clients Jagannathan Raman (1): io: error_prepend() in qio_channel_readv_full_all() causes segfault Max Reitz (1): iotests/210: Fix reference output Vladimir Sementsov-Ogievskiy (10): block: add new BlockDriver handler: bdrv_cancel_in_flight block/nbd: implement .bdrv_cancel_in_flight block/raw-format: implement .bdrv_cancel_in_flight handler job: add .cancel handler for the driver block/mirror: implement .cancel job handler iotests/264: move to python unittest iotests.py: qemu_nbd_popen: remove pid file after use iotests/264: add mirror-cancel test-case block/backup: implement .cancel job handler iotests/264: add backup-cancel test-case docs/tools/qemu-nbd.rst | 4 +- include/block/block.h | 3 + include/block/block_int.h | 9 +++ include/qemu/job.h| 5 ++ block/io.c| 11 block/backup.c| 10 +++ block/mirror.c| 9 +++ block/nbd.c | 15 + block/raw-format.c| 6 ++ blockdev-nbd.c| 7 ++- io/channel.c | 3 +- job.c | 3 + qemu-nbd.c| 14 - tests/qemu-iotests/210.out| 2 +- tests/qemu-iotests/264| 140 +++--- tests/qemu-iotests/264.out| 20 ++ tests/qemu-iotests/iotests.py | 6 +- 17 files changed, 193 insertions(+), 74 deletions(-) -- 2.30.1
[PULL 02/14] qemu-nbd: Permit --shared=0 for unlimited clients
This gives us better feature parity with QMP nbd-server-start, where
max-connections defaults to 0 for unlimited.
Signed-off-by: Eric Blake
Message-Id: <20210209152759.209074-3-ebl...@redhat.com>
Reviewed-by: Daniel P. Berrangé
---
docs/tools/qemu-nbd.rst | 4 ++--
qemu-nbd.c | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/docs/tools/qemu-nbd.rst b/docs/tools/qemu-nbd.rst
index fe41336dc550..ee862fa0bc02 100644
--- a/docs/tools/qemu-nbd.rst
+++ b/docs/tools/qemu-nbd.rst
@@ -136,8 +136,8 @@ driver options if ``--image-opts`` is specified.
.. option:: -e, --shared=NUM
Allow up to *NUM* clients to share the device (default
- ``1``). Safe for readers, but for now, consistency is not
- guaranteed between multiple writers.
+ ``1``), 0 for unlimited. Safe for readers, but for now,
+ consistency is not guaranteed between multiple writers.
.. option:: -t, --persistent
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 1a340ea4858d..b1b9430a8f54 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -328,7 +328,7 @@ static void *nbd_client_thread(void *arg)
static int nbd_can_accept(void)
{
-return state == RUNNING && nb_fds < shared;
+return state == RUNNING && (shared == 0 || nb_fds < shared);
}
static void nbd_update_server_watch(void);
@@ -707,7 +707,7 @@ int main(int argc, char **argv)
break;
case 'e':
if (qemu_strtoi(optarg, NULL, 0, ) < 0 ||
-shared < 1) {
+shared < 0) {
error_report("Invalid shared device number '%s'", optarg);
exit(EXIT_FAILURE);
}
@@ -966,7 +966,7 @@ int main(int argc, char **argv)
if (socket_activation == 0) {
int backlog;
-if (persistent) {
+if (persistent || shared == 0) {
backlog = SOMAXCONN;
} else {
backlog = MIN(shared, SOMAXCONN);
--
2.30.1
[PULL 06/14] block/nbd: implement .bdrv_cancel_in_flight
From: Vladimir Sementsov-Ogievskiy
Just stop waiting for connection in existing requests.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Eric Blake
Message-Id: <20210205163720.887197-3-vsement...@virtuozzo.com>
Signed-off-by: Eric Blake
---
block/nbd.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/block/nbd.c b/block/nbd.c
index b3cbbeb4b0cb..c26dc5a54f52 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -2458,6 +2458,18 @@ static const char *const nbd_strong_runtime_opts[] = {
NULL
};
+static void nbd_cancel_in_flight(BlockDriverState *bs)
+{
+BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
+
+reconnect_delay_timer_del(s);
+
+if (s->state == NBD_CLIENT_CONNECTING_WAIT) {
+s->state = NBD_CLIENT_CONNECTING_NOWAIT;
+qemu_co_queue_restart_all(>free_sema);
+}
+}
+
static BlockDriver bdrv_nbd = {
.format_name= "nbd",
.protocol_name = "nbd",
@@ -2484,6 +2496,7 @@ static BlockDriver bdrv_nbd = {
.bdrv_co_block_status = nbd_client_co_block_status,
.bdrv_dirname = nbd_dirname,
.strong_runtime_opts= nbd_strong_runtime_opts,
+.bdrv_cancel_in_flight = nbd_cancel_in_flight,
};
static BlockDriver bdrv_nbd_tcp = {
@@ -2512,6 +2525,7 @@ static BlockDriver bdrv_nbd_tcp = {
.bdrv_co_block_status = nbd_client_co_block_status,
.bdrv_dirname = nbd_dirname,
.strong_runtime_opts= nbd_strong_runtime_opts,
+.bdrv_cancel_in_flight = nbd_cancel_in_flight,
};
static BlockDriver bdrv_nbd_unix = {
@@ -2540,6 +2554,7 @@ static BlockDriver bdrv_nbd_unix = {
.bdrv_co_block_status = nbd_client_co_block_status,
.bdrv_dirname = nbd_dirname,
.strong_runtime_opts= nbd_strong_runtime_opts,
+.bdrv_cancel_in_flight = nbd_cancel_in_flight,
};
static void bdrv_nbd_init(void)
--
2.30.1
[PULL 04/14] io: error_prepend() in qio_channel_readv_full_all() causes segfault
From: Jagannathan Raman
Using error_prepend() in qio_channel_readv_full_all() causes a segfault
as errp is not set when ret is 0. This results in the failure of iotest
83. Replacing with error_setg() fixes the problem.
Additionally, removes a full stop at the end of error message
Reported-by: Max Reitz
Signed-off-by: Jagannathan Raman
Fixes: bebab91ebdfc591f8793a9a17370df1bfbe8b2ca
(io: add qio_channel_readv_full_all_eof & qio_channel_readv_full_all helpers)
Message-Id:
Reviewed-by: Daniel P. Berrangé
Signed-off-by: Eric Blake
---
io/channel.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/io/channel.c b/io/channel.c
index 4555021b6246..e8b019dc36e2 100644
--- a/io/channel.c
+++ b/io/channel.c
@@ -202,8 +202,7 @@ int qio_channel_readv_full_all(QIOChannel *ioc,
int ret = qio_channel_readv_full_all_eof(ioc, iov, niov, fds, nfds, errp);
if (ret == 0) {
-error_prepend(errp,
- "Unexpected end-of-file before all data were read.");
+error_setg(errp, "Unexpected end-of-file before all data were read");
return -1;
}
if (ret == 1) {
--
2.30.1
[PULL 08/14] job: add .cancel handler for the driver
From: Vladimir Sementsov-Ogievskiy
To be used in mirror in the following commit to cancel in-flight io on
target to not waste the time.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Message-Id: <20210205163720.887197-5-vsement...@virtuozzo.com>
Reviewed-by: Eric Blake
Signed-off-by: Eric Blake
---
include/qemu/job.h | 5 +
job.c | 3 +++
2 files changed, 8 insertions(+)
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 32aabb1c6000..efc6fa754498 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -251,6 +251,11 @@ struct JobDriver {
*/
void (*clean)(Job *job);
+/**
+ * If the callback is not NULL, it will be invoked in job_cancel_async
+ */
+void (*cancel)(Job *job);
+
/** Called when the job is freed */
void (*free)(Job *job);
diff --git a/job.c b/job.c
index 3aaaebafe2c8..289edee14374 100644
--- a/job.c
+++ b/job.c
@@ -715,6 +715,9 @@ static int job_finalize_single(Job *job)
static void job_cancel_async(Job *job, bool force)
{
+if (job->driver->cancel) {
+job->driver->cancel(job);
+}
if (job->user_paused) {
/* Do not call job_enter here, the caller will handle it. */
if (job->driver->user_resume) {
--
2.30.1
[PULL 01/14] qemu-nbd: Use SOMAXCONN for socket listen() backlog
Our default of a backlog of 1 connection is rather puny; it gets in
the way when we are explicitly allowing multiple clients (such as
qemu-nbd -e N [--shared], or nbd-server-start with its default
"max-connections":0 for unlimited), but is even a problem when we
stick to qemu-nbd's default of only 1 active client but use -t
[--persistent] where a second client can start using the server once
the first finishes. While the effects are less noticeable on TCP
sockets (since the client can poll() to learn when the server is ready
again), it is definitely observable on Unix sockets, where on Linux, a
client will fail with EAGAIN and no recourse but to sleep an arbitrary
amount of time before retrying if the server backlog is already full.
Since QMP nbd-server-start is always persistent, it now always
requests a backlog of SOMAXCONN; meanwhile, qemu-nbd will request
SOMAXCONN if persistent, otherwise its backlog should be based on the
expected number of clients.
See https://bugzilla.redhat.com/1925045 for a demonstration of where
our low backlog prevents libnbd from connecting as many parallel
clients as it wants.
Reported-by: Richard W.M. Jones
Signed-off-by: Eric Blake
CC: qemu-sta...@nongnu.org
Message-Id: <20210209152759.209074-2-ebl...@redhat.com>
Tested-by: Richard W.M. Jones
Reviewed-by: Daniel P. Berrangé
Signed-off-by: Eric Blake
---
blockdev-nbd.c | 7 ++-
qemu-nbd.c | 10 +-
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index d8443d235b73..b264620b98d8 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -134,7 +134,12 @@ void nbd_server_start(SocketAddress *addr, const char
*tls_creds,
qio_net_listener_set_name(nbd_server->listener,
"nbd-listener");
-if (qio_net_listener_open_sync(nbd_server->listener, addr, 1, errp) < 0) {
+/*
+ * Because this server is persistent, a backlog of SOMAXCONN is
+ * better than trying to size it to max_connections.
+ */
+if (qio_net_listener_open_sync(nbd_server->listener, addr, SOMAXCONN,
+ errp) < 0) {
goto error;
}
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 608c63e82a25..1a340ea4858d 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -964,8 +964,16 @@ int main(int argc, char **argv)
server = qio_net_listener_new();
if (socket_activation == 0) {
+int backlog;
+
+if (persistent) {
+backlog = SOMAXCONN;
+} else {
+backlog = MIN(shared, SOMAXCONN);
+}
saddr = nbd_build_socket_address(sockpath, bindto, port);
-if (qio_net_listener_open_sync(server, saddr, 1, _err) < 0) {
+if (qio_net_listener_open_sync(server, saddr, backlog,
+ _err) < 0) {
object_unref(OBJECT(server));
error_report_err(local_err);
exit(EXIT_FAILURE);
--
2.30.1
Re: [PATCH 1/1] Acceptance Tests: bump Avocado version requirement to 85.0
On Thu, Feb 11, 2021 at 8:28 PM Cleber Rosa wrote: > > This version (and 84.0) contain improvements that address specific > QEMU use cases, including: > > * Being able to download and use Fedora 31 images and thus >re-activate the "boot_linux.py" tests > > * Being able to register local assets via "avocado assets register" >and use them in tests > > Signed-off-by: Cleber Rosa > --- > tests/requirements.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Willian Rampazzo
Re: [PATCH v7 08/14] block/qcow2: qcow2_get_specific_info(): drop error propagation
On 2/5/21 5:52 AM, Vladimir Sementsov-Ogievskiy wrote:
> 05.02.2021 14:43, Alberto Garcia wrote:
>> On Tue 02 Feb 2021 01:49:50 PM CET, Vladimir Sementsov-Ogievskiy wrote:
>>> -Qcow2BitmapInfoList *qcow2_get_bitmap_info_list(BlockDriverState *bs,
>>> - Error **errp)
>>> +bool qcow2_get_bitmap_info_list(BlockDriverState *bs,
>>> + Qcow2BitmapInfoList **info_list,
>>> Error **errp)
>>> {
>>> BDRVQcow2State *s = bs->opaque;
>>> Qcow2BitmapList *bm_list;
>>> Qcow2Bitmap *bm;
>>> - Qcow2BitmapInfoList *list = NULL;
>>> - Qcow2BitmapInfoList **tail =
>>> if (s->nb_bitmaps == 0) {
>>> - return NULL;
>>> + *info_list = NULL;
>>> + return true;
>>> }
>>> bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
>>> s->bitmap_directory_size, errp);
>>> - if (bm_list == NULL) {
>>> - return NULL;
>>> + if (!bm_list) {
>>> + return false;
>>> }
>>> + *info_list = NULL;
>>> +
>>> QSIMPLEQ_FOREACH(bm, bm_list, entry) {
>>> Qcow2BitmapInfo *info = g_new0(Qcow2BitmapInfo, 1);
>>> info->granularity = 1U << bm->granularity_bits;
>>> info->name = g_strdup(bm->name);
>>> info->flags = get_bitmap_info_flags(bm->flags &
>>> ~BME_RESERVED_FLAGS);
>>> - QAPI_LIST_APPEND(tail, info);
>>> + QAPI_LIST_APPEND(info_list, info);
>>> }
>>> bitmap_list_free(bm_list);
>>> - return list;
>>> + return true;
>>> }
>>
>> Maybe I'm reading this wrong but...
>>
>> In the original code you had the head and tail of the list ('list' and
>> 'tail') then you would append items to the tail and finally return the
>> head.
>>
>> However the new code only uses and updates 'info_list' and it does not
>> keep the head anywhere, so what the caller gets is a pointer to the
>> tail.
>>
>
> No. *info_list is modified only on the first loop iteration. And than
> info_list is switched to &(*(info_list))->next, so on second iteration
> we will modify @next field of first element, not original *info_list.
Elsewhere when making these types of conversions, Markus suggested that
I keep a separate tail variable, initialized by the parameter info_list,
to make it more apparent. As in squashing the patch below:
With that, it looks this series is reviewed, so I'm planning on taking
it through my dirty-bitmaps tree (perhaps I'm stretching the fact that
patch 10/14 is definitely dirty-bitmaps into taking the whole series,
but I doubt I'll hear any complaints from other block maintainers)
diff --git i/block/qcow2-bitmap.c w/block/qcow2-bitmap.c
index e50da1ee7da3..f417f9ccb195 100644
--- i/block/qcow2-bitmap.c
+++ w/block/qcow2-bitmap.c
@@ -1103,6 +1103,7 @@ bool qcow2_get_bitmap_info_list(BlockDriverState *bs,
BDRVQcow2State *s = bs->opaque;
Qcow2BitmapList *bm_list;
Qcow2Bitmap *bm;
+Qcow2BitmapInfoList **tail;
if (s->nb_bitmaps == 0) {
*info_list = NULL;
@@ -1116,13 +1117,14 @@ bool qcow2_get_bitmap_info_list(BlockDriverState
*bs,
}
*info_list = NULL;
+tail = info_list;
QSIMPLEQ_FOREACH(bm, bm_list, entry) {
Qcow2BitmapInfo *info = g_new0(Qcow2BitmapInfo, 1);
info->granularity = 1U << bm->granularity_bits;
info->name = g_strdup(bm->name);
info->flags = get_bitmap_info_flags(bm->flags &
~BME_RESERVED_FLAGS);
-QAPI_LIST_APPEND(info_list, info);
+QAPI_LIST_APPEND(tail, info);
}
bitmap_list_free(bm_list);
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 1/2] block: Explicit null-co uses 'read-zeroes=false'
On 2/11/21 8:26 AM, Philippe Mathieu-Daudé wrote:
> We are going to switch the 'null-co' default 'read-zeroes' value
> from FALSE to TRUE in the next commit. First explicit the FALSE
> value when it is not set.
>
> Suggested-by: Eric Blake
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> +++ b/tests/qemu-iotests/300
> @@ -44,12 +44,12 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
> def setUp(self) -> None:
> self.vm_a = iotests.VM(path_suffix='-a')
> self.vm_a.add_blockdev(f'node-name={self.src_node_name},'
> - 'driver=null-co')
> + 'driver=null-co,read-zeroes=off')
> self.vm_a.launch()
>
> self.vm_b = iotests.VM(path_suffix='-b')
> self.vm_b.add_blockdev(f'node-name={self.dst_node_name},'
> - 'driver=null-co')
> + 'driver=null-co,read-zeroes=off')
> self.vm_b.add_incoming(f'unix:{mig_sock}')
> self.vm_b.launch()
>
>
Incomplete: 300 has a couple more lines that look like:
tests/qemu-iotests/300-result = self.vm_a.qmp('blockdev-add',
tests/qemu-iotests/300:
node_name='node-b', driver='null-co')
--
tests/qemu-iotests/300-result = self.vm_b.qmp('blockdev-add',
tests/qemu-iotests/300:
node_name='node-a', driver='null-co')
that could use the same treatment (noticed while reviewing Peter's patch
to add yet more uses in that test).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v7 00/31] target-arm: Implement ARMv8.5-MemTag, user mode
Patchew URL:
https://patchew.org/QEMU/20210212184902.1251044-1-richard.hender...@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210212184902.1251044-1-richard.hender...@linaro.org
Subject: [PATCH v7 00/31] target-arm: Implement ARMv8.5-MemTag, user mode
=== 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/20210205170019.25319-1-peter.mayd...@linaro.org ->
patchew/20210205170019.25319-1-peter.mayd...@linaro.org
- [tag update]
patchew/20210209193018.31339-1-mark.cave-ayl...@ilande.co.uk ->
patchew/20210209193018.31339-1-mark.cave-ayl...@ilande.co.uk
* [new tag]
patchew/20210212184902.1251044-1-richard.hender...@linaro.org ->
patchew/20210212184902.1251044-1-richard.hender...@linaro.org
- [tag update] patchew/cover.1613150869.git.pkre...@redhat.com ->
patchew/cover.1613150869.git.pkre...@redhat.com
Switched to a new branch 'test'
4cfeede tests/tcg/aarch64: Add mte smoke tests
0903d59 target/arm: Enable MTE for user-only
9df3d28 target/arm: Add allocation tag storage for user mode
3d09f6c linux-user/aarch64: Signal SEGV_MTEAERR for async tag check error
879da12 linux-user/aarch64: Signal SEGV_MTESERR for sync tag check fault
2dd5738 linux-user/aarch64: Pass syndrome to EXC_*_ABORT
b40490e target/arm: Split out syndrome.h from internals.h
63aaeca linux-user/aarch64: Implement PROT_MTE
900b086 linux-user/aarch64: Implement PR_MTE_TCF and PR_MTE_TAG
cb78c3c target/arm: Use the proper TBI settings for linux-user
78a9dec target/arm: Improve gen_top_byte_ignore
4a63561 linux-user/aarch64: Implement PR_TAGGED_ADDR_ENABLE
c66012b linux-user: Handle tags in lock_user/unlock_user
580506f linux-user: Fix types in uaccess.c
2195f03 linux-user: Move lock_user et al out of line
8397f7c linux-user: Use cpu_untagged_addr in access_ok; split out *_untagged
30d0a45 exec: Rename guest_{addr,range}_valid to *_untagged
a05e311 linux-user: Use guest_range_valid in access_ok
bdb38c8 linux-user: Explicitly untag memory management syscalls
42db09c exec: Use cpu_untagged_addr in g2h; split out g2h_untagged
0bd08b6 exec: Introduce cpu_untagged_addr
0800426 linux-user: Fix guest_addr_valid vs reserved_va
1fa4ffc linux-user: Do not use guest_addr_valid for h2g_valid
eb134c4 bsd-user: Tidy VERIFY_READ/VERIFY_WRITE
d58973b linux-user: Tidy VERIFY_READ/VERIFY_WRITE
0b39a7e linux-user: Check for overflow in access_ok
1a8ffb5 exec: Improve types for guest_addr_valid
4e09125 exec: Use uintptr_t in cpu_ldst.h
8ff95a1 exec: Use uintptr_t for guest_base
61d816a linux-user: Introduce PAGE_ANON
e4aaaca tcg: Introduce target-specific page data for user-only
=== OUTPUT BEGIN ===
1/31 Checking commit e4aaaca9e919 (tcg: Introduce target-specific page data for
user-only)
2/31 Checking commit 61d816a5eced (linux-user: Introduce PAGE_ANON)
3/31 Checking commit 8ff95a1e9714 (exec: Use uintptr_t for guest_base)
4/31 Checking commit 4e0912550e2c (exec: Use uintptr_t in cpu_ldst.h)
5/31 Checking commit 1a8ffb5feb79 (exec: Improve types for guest_addr_valid)
6/31 Checking commit 0b39a7ecec65 (linux-user: Check for overflow in access_ok)
7/31 Checking commit d58973b39d42 (linux-user: Tidy VERIFY_READ/VERIFY_WRITE)
8/31 Checking commit eb134c450d25 (bsd-user: Tidy VERIFY_READ/VERIFY_WRITE)
9/31 Checking commit 1fa4ffce22f6 (linux-user: Do not use guest_addr_valid for
h2g_valid)
10/31 Checking commit 0800426455c1 (linux-user: Fix guest_addr_valid vs
reserved_va)
11/31 Checking commit 0bd08b6bce82 (exec: Introduce cpu_untagged_addr)
12/31 Checking commit 42db09c16bdd (exec: Use cpu_untagged_addr in g2h; split
out g2h_untagged)
ERROR: braces {} are necessary for all arms of this statement
#446: FILE: bsd-user/qemu.h:379:
+if (host_ptr == g2h_untagged(guest_addr))
[...]
ERROR: braces {} are necessary for all arms of this statement
#703: FILE: linux-user/mmap.c:222:
+if (pread(fd, g2h_untagged(start), end - start, offset) == -1)
[...]
ERROR: braces {} are necessary for all arms of this statement
#865: FILE: linux-user/qemu.h:669:
+if (host_ptr == g2h_untagged(guest_addr))
[...]
total: 3 errors, 0 warnings, 946 lines checked
Patch 12/31 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
13/31 Checking commit bdb38c8bbd1d (linux-user: Explicitly untag memory
management syscalls)
14/31 Checking commit a05e311b4a85 (linux-user: Use guest_range_valid in
access_ok)
15/31 Checking commit 30d0a450a9fb (exec: Rename guest_{addr,range}_valid to
*_untagged)
16/31 Checking commit 8397f7cdd64e (linux-user: Use
Re: [PATCH] virtiofs_submounts.py test: Note on vmlinuz param
On Fri, Feb 12, 2021 at 04:16:49PM +0100, Max Reitz wrote:
> From the cancel message, it is not entirely clear why this parameter is
> mandatory now, or that it will be optional in the future. Add such a
> more detailed explanation as a comment in the test source file.
>
> Suggested-by: Alex Bennée
> Signed-off-by: Max Reitz
> ---
> I’ve uploaded a build of Linux 5.10 here:
> https://xanclic.moe/linux-5.10
>
> But I’ve decided against mentioning it in this new comment or the cancel
> message, because, well, it’s my private server and I have limited
> bandwidth.
> ---
> tests/acceptance/virtiofs_submounts.py | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/tests/acceptance/virtiofs_submounts.py
> b/tests/acceptance/virtiofs_submounts.py
> index 949ca87a83..9a69b6b17b 100644
> --- a/tests/acceptance/virtiofs_submounts.py
> +++ b/tests/acceptance/virtiofs_submounts.py
> @@ -228,6 +228,18 @@ class VirtiofsSubmountsTest(BootLinux):
> def setUp(self):
> vmlinuz = self.params.get('vmlinuz')
> if vmlinuz is None:
> +"""
> +The Linux kernel supports FUSE auto-submounts only as of 5.10.
> +boot_linux.py currently provides Fedora 31, whose kernel is too
> +old, so this test cannot pass with the on-image kernel (you are
> +welcome to try, hence the option to force such a test with
> +-p vmlinuz=''). Therefore, for now the user must provide a
> +sufficiently new custom kernel, or effectively explicitly
> +request failure with -p vmlinuz=''.
> +Once an image with a sufficiently new kernel is available
> +(probably Fedora 34), we can make -p vmlinuz='' the default, so
> +that this parameter no longer needs to be specified.
> +"""
> self.cancel('vmlinuz parameter not set; you must point it to a '
> 'Linux kernel binary to test (to run this test with
> ' \
> 'the on-image kernel, set it to an empty string)')
> --
> 2.29.2
>
Hi Max,
This looks good to me, and I've also tested your kernel build and
works like a charm.
As further work on top of this, it may be beneficial to have test
documentation in a predictable place. The possibilities that come to
my mind:
* docs/devel/testing.rst
* tests/acceptance/$test_file.py/data/README
On a different topic, the "https://avocado-project.org/data/assets; has
enough bandwidth and can be used to hold this type asset. Alternatively,
we can add a bit more automation to this test by letting people do something
like:
$ avocado assets register virtiofs-auto-submounts-vmlinuz /path/to/vmlinuz
And on the test:
vmlinuz = self.fetch_asset('virtiofs-auto-submounts-vmlinuz')
And the test should cancel if that asset has not been previously registered.
Anyway,
Reviewed-by: Cleber Rosa
signature.asc
Description: PGP signature
Re: [PATCH v7 03/31] exec: Use uintptr_t for guest_base
On 2/12/21 7:48 PM, Richard Henderson wrote: > This is more descriptive than 'unsigned long'. > No functional change, since these match on all linux+bsd hosts. > > Reviewed-by: Peter Maydell > Signed-off-by: Richard Henderson > --- > include/exec/cpu-all.h | 2 +- > bsd-user/main.c| 4 ++-- > linux-user/elfload.c | 4 ++-- > linux-user/main.c | 4 ++-- > 4 files changed, 7 insertions(+), 7 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v7 05/31] exec: Improve types for guest_addr_valid
On 2/12/21 7:48 PM, Richard Henderson wrote: > Return bool not int; pass abi_ulong not 'unsigned long'. > All callers use abi_ulong already, so the change in type > has no effect. > > Reviewed-by: Peter Maydell > Signed-off-by: Richard Henderson > --- > include/exec/cpu_ldst.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 03/38] target/riscv: Fixup saturate subtract function
On 2/12/21 7:02 AM, LIU Zhiwei wrote: > The overflow predication ((a - b) ^ a) & (a ^ b) & INT64_MIN is right. > However, when the predication is ture and a is 0, it should return maximum. > > Signed-off-by: LIU Zhiwei > --- > target/riscv/vector_helper.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Richard Henderson r~
[PATCH v7 26/31] linux-user/aarch64: Pass syndrome to EXC_*_ABORT
A proper syndrome is required to fill in the proper si_code.
Use page_get_flags to determine permission vs translation for user-only.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
linux-user/aarch64/cpu_loop.c | 24 +---
target/arm/tlb_helper.c | 15 +--
2 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
index 42b9c15f53..4e43906e66 100644
--- a/linux-user/aarch64/cpu_loop.c
+++ b/linux-user/aarch64/cpu_loop.c
@@ -23,6 +23,7 @@
#include "cpu_loop-common.h"
#include "qemu/guest-random.h"
#include "hw/semihosting/common-semi.h"
+#include "target/arm/syndrome.h"
#define get_user_code_u32(x, gaddr, env)\
({ abi_long __r = get_user_u32((x), (gaddr)); \
@@ -76,7 +77,7 @@
void cpu_loop(CPUARMState *env)
{
CPUState *cs = env_cpu(env);
-int trapnr;
+int trapnr, ec, fsc;
abi_long ret;
target_siginfo_t info;
@@ -117,9 +118,26 @@ void cpu_loop(CPUARMState *env)
case EXCP_DATA_ABORT:
info.si_signo = TARGET_SIGSEGV;
info.si_errno = 0;
-/* XXX: check env->error_code */
-info.si_code = TARGET_SEGV_MAPERR;
info._sifields._sigfault._addr = env->exception.vaddress;
+
+/* We should only arrive here with EC in {DATAABORT, INSNABORT}. */
+ec = syn_get_ec(env->exception.syndrome);
+assert(ec == EC_DATAABORT || ec == EC_INSNABORT);
+
+/* Both EC have the same format for FSC, or close enough. */
+fsc = extract32(env->exception.syndrome, 0, 6);
+switch (fsc) {
+case 0x04 ... 0x07: /* Translation fault, level {0-3} */
+info.si_code = TARGET_SEGV_MAPERR;
+break;
+case 0x09 ... 0x0b: /* Access flag fault, level {1-3} */
+case 0x0d ... 0x0f: /* Permission fault, level {1-3} */
+info.si_code = TARGET_SEGV_ACCERR;
+break;
+default:
+g_assert_not_reached();
+}
+
queue_signal(env, info.si_signo, QEMU_SI_FAULT, );
break;
case EXCP_DEBUG:
diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c
index df85079d9f..9609333cbd 100644
--- a/target/arm/tlb_helper.c
+++ b/target/arm/tlb_helper.c
@@ -154,21 +154,24 @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
bool probe, uintptr_t retaddr)
{
ARMCPU *cpu = ARM_CPU(cs);
+ARMMMUFaultInfo fi = {};
#ifdef CONFIG_USER_ONLY
-cpu->env.exception.vaddress = address;
-if (access_type == MMU_INST_FETCH) {
-cs->exception_index = EXCP_PREFETCH_ABORT;
+int flags = page_get_flags(useronly_clean_ptr(address));
+if (flags & PAGE_VALID) {
+fi.type = ARMFault_Permission;
} else {
-cs->exception_index = EXCP_DATA_ABORT;
+fi.type = ARMFault_Translation;
}
-cpu_loop_exit_restore(cs, retaddr);
+
+/* now we have a real cpu fault */
+cpu_restore_state(cs, retaddr, true);
+arm_deliver_fault(cpu, address, access_type, mmu_idx, );
#else
hwaddr phys_addr;
target_ulong page_size;
int prot, ret;
MemTxAttrs attrs = {};
-ARMMMUFaultInfo fi = {};
ARMCacheAttrs cacheattrs = {};
/*
--
2.25.1
[PATCH v7 31/31] tests/tcg/aarch64: Add mte smoke tests
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
tests/tcg/aarch64/mte.h | 60 +++
tests/tcg/aarch64/mte-1.c | 28 +++
tests/tcg/aarch64/mte-2.c | 45 +++
tests/tcg/aarch64/mte-3.c | 51 ++
tests/tcg/aarch64/mte-4.c | 45 +++
tests/tcg/aarch64/Makefile.target | 6
tests/tcg/configure.sh| 4 +++
7 files changed, 239 insertions(+)
create mode 100644 tests/tcg/aarch64/mte.h
create mode 100644 tests/tcg/aarch64/mte-1.c
create mode 100644 tests/tcg/aarch64/mte-2.c
create mode 100644 tests/tcg/aarch64/mte-3.c
create mode 100644 tests/tcg/aarch64/mte-4.c
diff --git a/tests/tcg/aarch64/mte.h b/tests/tcg/aarch64/mte.h
new file mode 100644
index 00..141cef522c
--- /dev/null
+++ b/tests/tcg/aarch64/mte.h
@@ -0,0 +1,60 @@
+/*
+ * Linux kernel fallback API definitions for MTE and test helpers.
+ *
+ * Copyright (c) 2021 Linaro Ltd
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#ifndef PR_SET_TAGGED_ADDR_CTRL
+# define PR_SET_TAGGED_ADDR_CTRL 55
+#endif
+#ifndef PR_TAGGED_ADDR_ENABLE
+# define PR_TAGGED_ADDR_ENABLE(1UL << 0)
+#endif
+#ifndef PR_MTE_TCF_SHIFT
+# define PR_MTE_TCF_SHIFT 1
+# define PR_MTE_TCF_NONE (0UL << PR_MTE_TCF_SHIFT)
+# define PR_MTE_TCF_SYNC (1UL << PR_MTE_TCF_SHIFT)
+# define PR_MTE_TCF_ASYNC (2UL << PR_MTE_TCF_SHIFT)
+# define PR_MTE_TAG_SHIFT 3
+#endif
+
+#ifndef PROT_MTE
+# define PROT_MTE 0x20
+#endif
+
+#ifndef SEGV_MTEAERR
+# define SEGV_MTEAERR8
+# define SEGV_MTESERR9
+#endif
+
+static void enable_mte(int tcf)
+{
+int r = prctl(PR_SET_TAGGED_ADDR_CTRL,
+ PR_TAGGED_ADDR_ENABLE | tcf | (0xfffe << PR_MTE_TAG_SHIFT),
+ 0, 0, 0);
+if (r < 0) {
+perror("PR_SET_TAGGED_ADDR_CTRL");
+exit(2);
+}
+}
+
+static void *alloc_mte_mem(size_t size)
+{
+void *p = mmap(NULL, size, PROT_READ | PROT_WRITE | PROT_MTE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+if (p == MAP_FAILED) {
+perror("mmap PROT_MTE");
+exit(2);
+}
+return p;
+}
diff --git a/tests/tcg/aarch64/mte-1.c b/tests/tcg/aarch64/mte-1.c
new file mode 100644
index 00..88dcd617ad
--- /dev/null
+++ b/tests/tcg/aarch64/mte-1.c
@@ -0,0 +1,28 @@
+/*
+ * Memory tagging, basic pass cases.
+ *
+ * Copyright (c) 2021 Linaro Ltd
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "mte.h"
+
+int main(int ac, char **av)
+{
+int *p0, *p1, *p2;
+long c;
+
+enable_mte(PR_MTE_TCF_NONE);
+p0 = alloc_mte_mem(sizeof(*p0));
+
+asm("irg %0,%1,%2" : "=r"(p1) : "r"(p0), "r"(1));
+assert(p1 != p0);
+asm("subp %0,%1,%2" : "=r"(c) : "r"(p0), "r"(p1));
+assert(c == 0);
+
+asm("stg %0, [%0]" : : "r"(p1));
+asm("ldg %0, [%1]" : "=r"(p2) : "r"(p0), "0"(p0));
+assert(p1 == p2);
+
+return 0;
+}
diff --git a/tests/tcg/aarch64/mte-2.c b/tests/tcg/aarch64/mte-2.c
new file mode 100644
index 00..a62278276a
--- /dev/null
+++ b/tests/tcg/aarch64/mte-2.c
@@ -0,0 +1,45 @@
+/*
+ * Memory tagging, basic fail cases, synchronous signals.
+ *
+ * Copyright (c) 2021 Linaro Ltd
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "mte.h"
+
+void pass(int sig, siginfo_t *info, void *uc)
+{
+assert(info->si_code == SEGV_MTESERR);
+exit(0);
+}
+
+int main(int ac, char **av)
+{
+struct sigaction sa;
+int *p0, *p1, *p2;
+long excl = 1;
+
+enable_mte(PR_MTE_TCF_SYNC);
+p0 = alloc_mte_mem(sizeof(*p0));
+
+/* Create two differently tagged pointers. */
+asm("irg %0,%1,%2" : "=r"(p1) : "r"(p0), "r"(excl));
+asm("gmi %0,%1,%0" : "+r"(excl) : "r" (p1));
+assert(excl != 1);
+asm("irg %0,%1,%2" : "=r"(p2) : "r"(p0), "r"(excl));
+assert(p1 != p2);
+
+/* Store the tag from the first pointer. */
+asm("stg %0, [%0]" : : "r"(p1));
+
+*p1 = 0;
+
+memset(, 0, sizeof(sa));
+sa.sa_sigaction = pass;
+sa.sa_flags = SA_SIGINFO;
+sigaction(SIGSEGV, , NULL);
+
+*p2 = 0;
+
+abort();
+}
diff --git a/tests/tcg/aarch64/mte-3.c b/tests/tcg/aarch64/mte-3.c
new file mode 100644
index 00..424ea685c2
--- /dev/null
+++ b/tests/tcg/aarch64/mte-3.c
@@ -0,0 +1,51 @@
+/*
+ * Memory tagging, basic fail cases, asynchronous signals.
+ *
+ * Copyright (c) 2021 Linaro Ltd
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "mte.h"
+
+void pass(int sig, siginfo_t *info, void *uc)
+{
+assert(info->si_code == SEGV_MTEAERR);
+exit(0);
+}
+
+int main(int ac, char **av)
+{
+struct sigaction sa;
+long *p0, *p1, *p2;
+long excl = 1;
+
+enable_mte(PR_MTE_TCF_ASYNC);
+p0 = alloc_mte_mem(sizeof(*p0));
+
+/* Create two differently tagged pointers. */
+asm("irg %0,%1,%2" :
Re: [PATCH v3 0/3] migration: dirty-bitmap: Allow control of bitmap persistence
On 2/12/21 11:34 AM, Peter Krempa wrote: > See 2/2 for explanation. > > Peter Krempa (3): > migration: dirty-bitmap: Convert alias map inner members to > BitmapMigrationBitmapAlias > migration: dirty-bitmap: Allow control of bitmap persistence > qemu-iotests: 300: Add test case for modifying persistence of bitmap > Queuing this through my dirty-bitmap tree > migration/block-dirty-bitmap.c | 60 -- > qapi/migration.json| 19 ++- > tests/qemu-iotests/300 | 91 ++ > tests/qemu-iotests/300.out | 4 +- > 4 files changed, 157 insertions(+), 17 deletions(-) > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
[PATCH v7 28/31] linux-user/aarch64: Signal SEGV_MTEAERR for async tag check error
The real kernel collects _TIF_MTE_ASYNC_FAULT into the current thread's
state on any kernel entry (interrupt, exception etc), and then delivers
the signal in advance of resuming the thread.
This means that while the signal won't be delivered immediately, it will
not be delayed forever -- at minimum it will be delivered after the next
clock interrupt.
We don't have a clock interrupt in linux-user, so we issue a cpu_kick
to signal a return to the main loop at the end of the current TB.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
linux-user/aarch64/target_signal.h | 1 +
linux-user/aarch64/cpu_loop.c | 11 +++
target/arm/mte_helper.c| 10 ++
3 files changed, 22 insertions(+)
diff --git a/linux-user/aarch64/target_signal.h
b/linux-user/aarch64/target_signal.h
index 777fb667fe..18013e1b23 100644
--- a/linux-user/aarch64/target_signal.h
+++ b/linux-user/aarch64/target_signal.h
@@ -21,6 +21,7 @@ typedef struct target_sigaltstack {
#include "../generic/signal.h"
+#define TARGET_SEGV_MTEAERR 8 /* Asynchronous ARM MTE error */
#define TARGET_SEGV_MTESERR 9 /* Synchronous ARM MTE exception */
#define TARGET_ARCH_HAS_SETUP_FRAME
diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
index b6a2e65593..7c42f65706 100644
--- a/linux-user/aarch64/cpu_loop.c
+++ b/linux-user/aarch64/cpu_loop.c
@@ -164,6 +164,17 @@ void cpu_loop(CPUARMState *env)
EXCP_DUMP(env, "qemu: unhandled CPU exception 0x%x - aborting\n",
trapnr);
abort();
}
+
+/* Check for MTE asynchronous faults */
+if (unlikely(env->cp15.tfsr_el[0])) {
+env->cp15.tfsr_el[0] = 0;
+info.si_signo = TARGET_SIGSEGV;
+info.si_errno = 0;
+info._sifields._sigfault._addr = 0;
+info.si_code = TARGET_SEGV_MTEAERR;
+queue_signal(env, info.si_signo, QEMU_SI_FAULT, );
+}
+
process_pending_signals(env);
/* Exception return on AArch64 always clears the exclusive monitor,
* so any return to running guest code implies this.
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 153bd1e9df..d55f8d1e1e 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -565,6 +565,16 @@ static void mte_check_fail(CPUARMState *env, uint32_t desc,
select = 0;
}
env->cp15.tfsr_el[el] |= 1 << select;
+#ifdef CONFIG_USER_ONLY
+/*
+ * Stand in for a timer irq, setting _TIF_MTE_ASYNC_FAULT,
+ * which then sends a SIGSEGV when the thread is next scheduled.
+ * This cpu will return to the main loop at the end of the TB,
+ * which is rather sooner than "normal". But the alternative
+ * is waiting until the next syscall.
+ */
+qemu_cpu_kick(env_cpu(env));
+#endif
break;
default:
--
2.25.1
[PATCH v7 30/31] target/arm: Enable MTE for user-only
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/cpu.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 70cfcbc918..b8bc89e71f 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -209,6 +209,21 @@ static void arm_cpu_reset(DeviceState *dev)
* Note that this must match useronly_clean_ptr.
*/
env->cp15.tcr_el[1].raw_tcr = (1ULL << 37);
+
+/* Enable MTE */
+if (cpu_isar_feature(aa64_mte, cpu)) {
+/* Enable tag access, but leave TCF0 as No Effect (0). */
+env->cp15.sctlr_el[1] |= SCTLR_ATA0;
+/*
+ * Exclude all tags, so that tag 0 is always used.
+ * This corresponds to Linux current->thread.gcr_incl = 0.
+ *
+ * Set RRND, so that helper_irg() will generate a seed later.
+ * Here in cpu_reset(), the crypto subsystem has not yet been
+ * initialized.
+ */
+env->cp15.gcr_el1 = 0x1;
+}
#else
/* Reset into the highest available EL */
if (arm_feature(env, ARM_FEATURE_EL3)) {
--
2.25.1
Re: [PATCH v3 3/3] qemu-iotests: 300: Add test case for modifying persistence of bitmap
On 2/12/21 11:34 AM, Peter Krempa wrote:
> Verify that the modification of the bitmap persistence over migration
> which is controlled via BitmapMigrationBitmapAliasTransform works
> properly.
>
> Based on TestCrossAliasMigration
>
> Signed-off-by: Peter Krempa
> ---
> tests/qemu-iotests/300 | 91 ++
> tests/qemu-iotests/300.out | 4 +-
> 2 files changed, 93 insertions(+), 2 deletions(-)
>
> +def setUp(self) -> None:
> +TestDirtyBitmapMigration.setUp(self)
> +
> +# Now create another block device and let both have two bitmaps each
> +result = self.vm_a.qmp('blockdev-add',
> + node_name='node-b', driver='null-co')
> +self.assert_qmp(result, 'return', {})
> +
> +result = self.vm_b.qmp('blockdev-add',
> + node_name='node-a', driver='null-co')
The use of null-co with no mention of the read-zeroes option here is a
(minor) semantic conflict with the proposal by Philippe to flip the
default of that option (minor, because all it impacts would be the
execution speed of the test)
https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg04027.html
Not your fault, so I don't mind touching up your additions in the same
manner Philippe proposed (which is safe regardless of whose patch lands
first).
diff --git i/tests/qemu-iotests/300 w/tests/qemu-iotests/300
index 9d4ec6a38195..63036f6a6e13 100755
--- i/tests/qemu-iotests/300
+++ w/tests/qemu-iotests/300
@@ -615,11 +615,13 @@ class
TestAliasTransformMigration(TestDirtyBitmapMigration):
# Now create another block device and let both have two bitmaps
each
result = self.vm_a.qmp('blockdev-add',
- node_name='node-b', driver='null-co')
+ node_name='node-b', driver='null-co',
+ read_zeroes=False)
self.assert_qmp(result, 'return', {})
result = self.vm_b.qmp('blockdev-add',
- node_name='node-a', driver='null-co')
+ node_name='node-a', driver='null-co',
+ read_zeroes=False)
self.assert_qmp(result, 'return', {})
bmaps_to_add = (('node-a', 'bmap-b'),
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[PATCH v7 19/31] linux-user: Handle tags in lock_user/unlock_user
Resolve the untagged address once, using thread_cpu.
Tidy the DEBUG_REMAP code using glib routines.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
linux-user/uaccess.c | 27 ++-
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/linux-user/uaccess.c b/linux-user/uaccess.c
index 76af6a92b1..c696913016 100644
--- a/linux-user/uaccess.c
+++ b/linux-user/uaccess.c
@@ -6,36 +6,37 @@
void *lock_user(int type, abi_ulong guest_addr, size_t len, bool copy)
{
+void *host_addr;
+
+guest_addr = cpu_untagged_addr(thread_cpu, guest_addr);
if (!access_ok_untagged(type, guest_addr, len)) {
return NULL;
}
+host_addr = g2h_untagged(guest_addr);
#ifdef DEBUG_REMAP
-{
-void *addr;
-addr = g_malloc(len);
-if (copy) {
-memcpy(addr, g2h(guest_addr), len);
-} else {
-memset(addr, 0, len);
-}
-return addr;
+if (copy) {
+host_addr = g_memdup(host_addr, len);
+} else {
+host_addr = g_malloc0(len);
}
-#else
-return g2h_untagged(guest_addr);
#endif
+return host_addr;
}
#ifdef DEBUG_REMAP
void unlock_user(void *host_ptr, abi_ulong guest_addr, size_t len);
{
+void *host_ptr_conv;
+
if (!host_ptr) {
return;
}
-if (host_ptr == g2h_untagged(guest_addr)) {
+host_ptr_conv = g2h(thread_cpu, guest_addr);
+if (host_ptr == host_ptr_conv) {
return;
}
if (len != 0) {
-memcpy(g2h_untagged(guest_addr), host_ptr, len);
+memcpy(host_ptr_conv, host_ptr, len);
}
g_free(host_ptr);
}
--
2.25.1
[PATCH v7 21/31] target/arm: Improve gen_top_byte_ignore
Use simple arithmetic instead of a conditional
move when tbi0 != tbi1.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/translate-a64.c | 25 ++---
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 1c4b8d02f3..b23a8975d5 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -183,17 +183,20 @@ static void gen_top_byte_ignore(DisasContext *s, TCGv_i64
dst,
/* Sign-extend from bit 55. */
tcg_gen_sextract_i64(dst, src, 0, 56);
-if (tbi != 3) {
-TCGv_i64 tcg_zero = tcg_const_i64(0);
-
-/*
- * The two TBI bits differ.
- * If tbi0, then !tbi1: only use the extension if positive.
- * if !tbi0, then tbi1: only use the extension if negative.
- */
-tcg_gen_movcond_i64(tbi == 1 ? TCG_COND_GE : TCG_COND_LT,
-dst, dst, tcg_zero, dst, src);
-tcg_temp_free_i64(tcg_zero);
+switch (tbi) {
+case 1:
+/* tbi0 but !tbi1: only use the extension if positive */
+tcg_gen_and_i64(dst, dst, src);
+break;
+case 2:
+/* !tbi0 but tbi1: only use the extension if negative */
+tcg_gen_or_i64(dst, dst, src);
+break;
+case 3:
+/* tbi0 and tbi1: always use the extension */
+break;
+default:
+g_assert_not_reached();
}
}
}
--
2.25.1
Re: [PATCH v7 04/31] exec: Use uintptr_t in cpu_ldst.h
On 2/12/21 7:48 PM, Richard Henderson wrote: > This is more descriptive than 'unsigned long'. > No functional change, since these match on all linux+bsd hosts. > > Reviewed-by: Peter Maydell > Signed-off-by: Richard Henderson > --- > include/exec/cpu_ldst.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
[PATCH v7 24/31] linux-user/aarch64: Implement PROT_MTE
Remember the PROT_MTE bit as PAGE_MTE/PAGE_TARGET_2.
Otherwise this does not yet have effect.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
include/exec/cpu-all.h| 1 +
linux-user/syscall_defs.h | 1 +
target/arm/cpu.h | 1 +
linux-user/mmap.c | 22 ++
4 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index d6ad774c01..09b9be845d 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -284,6 +284,7 @@ extern intptr_t qemu_host_page_mask;
#endif
/* Target-specific bits that will be used via page_get_flags(). */
#define PAGE_TARGET_1 0x0080
+#define PAGE_TARGET_2 0x0200
#if defined(CONFIG_USER_ONLY)
void page_dump(FILE *f);
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index f98c1c1c8d..46a960fccb 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -1311,6 +1311,7 @@ struct target_winsize {
#ifdef TARGET_AARCH64
#define TARGET_PROT_BTI 0x10
+#define TARGET_PROT_MTE 0x20
#endif
/* Common */
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 72a0819eb8..efa1618c4d 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3608,6 +3608,7 @@ static inline MemTxAttrs *typecheck_memtxattrs(MemTxAttrs
*x)
* AArch64 usage of the PAGE_TARGET_* bits for linux-user.
*/
#define PAGE_BTI PAGE_TARGET_1
+#define PAGE_MTE PAGE_TARGET_2
#ifdef TARGET_TAGGED_ADDRESSES
/**
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 6690384752..85e218ab1d 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -84,18 +84,24 @@ static int validate_prot_to_pageflags(int *host_prot, int
prot)
| (prot & PROT_EXEC ? PROT_READ : 0);
#ifdef TARGET_AARCH64
-/*
- * The PROT_BTI bit is only accepted if the cpu supports the feature.
- * Since this is the unusual case, don't bother checking unless
- * the bit has been requested. If set and valid, record the bit
- * within QEMU's page_flags.
- */
-if (prot & TARGET_PROT_BTI) {
+{
ARMCPU *cpu = ARM_CPU(thread_cpu);
-if (cpu_isar_feature(aa64_bti, cpu)) {
+
+/*
+ * The PROT_BTI bit is only accepted if the cpu supports the feature.
+ * Since this is the unusual case, don't bother checking unless
+ * the bit has been requested. If set and valid, record the bit
+ * within QEMU's page_flags.
+ */
+if ((prot & TARGET_PROT_BTI) && cpu_isar_feature(aa64_bti, cpu)) {
valid |= TARGET_PROT_BTI;
page_flags |= PAGE_BTI;
}
+/* Similarly for the PROT_MTE bit. */
+if ((prot & TARGET_PROT_MTE) && cpu_isar_feature(aa64_mte, cpu)) {
+valid |= TARGET_PROT_MTE;
+page_flags |= PAGE_MTE;
+}
}
#endif
--
2.25.1
[PATCH v7 29/31] target/arm: Add allocation tag storage for user mode
Use the now-saved PAGE_ANON and PAGE_MTE bits,
and the per-page saved data.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/mte_helper.c | 29 +++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index d55f8d1e1e..1c569336ea 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -78,8 +78,33 @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int
ptr_mmu_idx,
int tag_size, uintptr_t ra)
{
#ifdef CONFIG_USER_ONLY
-/* Tag storage not implemented. */
-return NULL;
+uint64_t clean_ptr = useronly_clean_ptr(ptr);
+int flags = page_get_flags(clean_ptr);
+uint8_t *tags;
+uintptr_t index;
+
+if (!(flags & (ptr_access == MMU_DATA_STORE ? PAGE_WRITE : PAGE_READ))) {
+/* SIGSEGV */
+arm_cpu_tlb_fill(env_cpu(env), ptr, ptr_size, ptr_access,
+ ptr_mmu_idx, false, ra);
+g_assert_not_reached();
+}
+
+/* Require both MAP_ANON and PROT_MTE for the page. */
+if (!(flags & PAGE_ANON) || !(flags & PAGE_MTE)) {
+return NULL;
+}
+
+tags = page_get_target_data(clean_ptr);
+if (tags == NULL) {
+size_t alloc_size = TARGET_PAGE_SIZE >> (LOG2_TAG_GRANULE + 1);
+tags = page_alloc_target_data(clean_ptr, alloc_size);
+assert(tags != NULL);
+}
+
+index = extract32(ptr, LOG2_TAG_GRANULE + 1,
+ TARGET_PAGE_BITS - LOG2_TAG_GRANULE - 1);
+return tags + index;
#else
uintptr_t index;
CPUIOTLBEntry *iotlbentry;
--
2.25.1
Re: [PATCH v7 25/31] target/arm: Split out syndrome.h from internals.h
On 2/12/21 7:48 PM, Richard Henderson wrote: > Move everything related to syndromes to a new file, > which can be shared with linux-user. > > Reviewed-by: Peter Maydell > Signed-off-by: Richard Henderson > --- > target/arm/internals.h | 245 +--- > target/arm/syndrome.h | 273 + > 2 files changed, 274 insertions(+), 244 deletions(-) > create mode 100644 target/arm/syndrome.h Reviewed-by: Philippe Mathieu-Daudé
[PATCH v7 18/31] linux-user: Fix types in uaccess.c
For copy_*_user, only 0 and -TARGET_EFAULT are returned; no need
to involve abi_long. Use size_t for lengths. Use bool for the
lock_user copy argument. Use ssize_t for target_strlen, because
we can't overflow the host memory space.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
linux-user/qemu.h| 14 ++
linux-user/uaccess.c | 45 ++--
2 files changed, 29 insertions(+), 30 deletions(-)
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 36b58bd840..d25a5dafc0 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -7,8 +7,6 @@
#include "exec/cpu_ldst.h"
#undef DEBUG_REMAP
-#ifdef DEBUG_REMAP
-#endif /* DEBUG_REMAP */
#include "exec/user/abitypes.h"
@@ -629,8 +627,8 @@ static inline bool access_ok(CPUState *cpu, int type,
* buffers between the target and host. These internally perform
* locking/unlocking of the memory.
*/
-abi_long copy_from_user(void *hptr, abi_ulong gaddr, size_t len);
-abi_long copy_to_user(abi_ulong gaddr, void *hptr, size_t len);
+int copy_from_user(void *hptr, abi_ulong gaddr, size_t len);
+int copy_to_user(abi_ulong gaddr, void *hptr, size_t len);
/* Functions for accessing guest memory. The tget and tput functions
read/write single values, byteswapping as necessary. The lock_user function
@@ -640,13 +638,13 @@ abi_long copy_to_user(abi_ulong gaddr, void *hptr, size_t
len);
/* Lock an area of guest memory into the host. If copy is true then the
host area will have the same contents as the guest. */
-void *lock_user(int type, abi_ulong guest_addr, long len, int copy);
+void *lock_user(int type, abi_ulong guest_addr, size_t len, bool copy);
/* Unlock an area of guest memory. The first LEN bytes must be
flushed back to guest memory. host_ptr = NULL is explicitly
allowed and does nothing. */
-#ifdef DEBUG_REMAP
-static inline void unlock_user(void *host_ptr, abi_ulong guest_addr, long len)
+#ifndef DEBUG_REMAP
+static inline void unlock_user(void *host_ptr, abi_ulong guest_addr, size_t
len)
{ }
#else
void unlock_user(void *host_ptr, abi_ulong guest_addr, long len);
@@ -654,7 +652,7 @@ void unlock_user(void *host_ptr, abi_ulong guest_addr, long
len);
/* Return the length of a string in target memory or -TARGET_EFAULT if
access error. */
-abi_long target_strlen(abi_ulong gaddr);
+ssize_t target_strlen(abi_ulong gaddr);
/* Like lock_user but for null terminated strings. */
void *lock_user_string(abi_ulong guest_addr);
diff --git a/linux-user/uaccess.c b/linux-user/uaccess.c
index bba012ed15..76af6a92b1 100644
--- a/linux-user/uaccess.c
+++ b/linux-user/uaccess.c
@@ -4,7 +4,7 @@
#include "qemu.h"
-void *lock_user(int type, abi_ulong guest_addr, long len, int copy)
+void *lock_user(int type, abi_ulong guest_addr, size_t len, bool copy)
{
if (!access_ok_untagged(type, guest_addr, len)) {
return NULL;
@@ -26,7 +26,7 @@ void *lock_user(int type, abi_ulong guest_addr, long len, int
copy)
}
#ifdef DEBUG_REMAP
-void unlock_user(void *host_ptr, abi_ulong guest_addr, long len);
+void unlock_user(void *host_ptr, abi_ulong guest_addr, size_t len);
{
if (!host_ptr) {
return;
@@ -34,7 +34,7 @@ void unlock_user(void *host_ptr, abi_ulong guest_addr, long
len);
if (host_ptr == g2h_untagged(guest_addr)) {
return;
}
-if (len > 0) {
+if (len != 0) {
memcpy(g2h_untagged(guest_addr), host_ptr, len);
}
g_free(host_ptr);
@@ -43,53 +43,53 @@ void unlock_user(void *host_ptr, abi_ulong guest_addr, long
len);
void *lock_user_string(abi_ulong guest_addr)
{
-abi_long len = target_strlen(guest_addr);
+ssize_t len = target_strlen(guest_addr);
if (len < 0) {
return NULL;
}
-return lock_user(VERIFY_READ, guest_addr, (long)(len + 1), 1);
+return lock_user(VERIFY_READ, guest_addr, (size_t)len + 1, 1);
}
/* copy_from_user() and copy_to_user() are usually used to copy data
* buffers between the target and host. These internally perform
* locking/unlocking of the memory.
*/
-abi_long copy_from_user(void *hptr, abi_ulong gaddr, size_t len)
+int copy_from_user(void *hptr, abi_ulong gaddr, size_t len)
{
-abi_long ret = 0;
-void *ghptr;
+int ret = 0;
+void *ghptr = lock_user(VERIFY_READ, gaddr, len, 1);
-if ((ghptr = lock_user(VERIFY_READ, gaddr, len, 1))) {
+if (ghptr) {
memcpy(hptr, ghptr, len);
unlock_user(ghptr, gaddr, 0);
-} else
+} else {
ret = -TARGET_EFAULT;
-
+}
return ret;
}
-
-abi_long copy_to_user(abi_ulong gaddr, void *hptr, size_t len)
+int copy_to_user(abi_ulong gaddr, void *hptr, size_t len)
{
-abi_long ret = 0;
-void *ghptr;
+int ret = 0;
+void *ghptr = lock_user(VERIFY_WRITE, gaddr, len, 0);
-if ((ghptr = lock_user(VERIFY_WRITE, gaddr, len, 0))) {
+if (ghptr) {
memcpy(ghptr, hptr, len);
unlock_user(ghptr, gaddr,
Re: [PATCH v3 2/3] migration: dirty-bitmap: Allow control of bitmap persistence
On 2/12/21 11:34 AM, Peter Krempa wrote:
> Bitmap's source persistence is transported over the migration stream and
> the destination mirrors it. In some cases the destination might want to
> persist bitmaps which are not persistent on the source (e.g. the result
> of merge of bitmaps from a number of layers on the source when migrating
Sorry for not proposing this grammar tweak earlier, but
s/merge of/merging/
> into a squashed image) but currently it would need to create another set
> of persistent bitmaps and merge them.
>
> This patch adds a 'transform' property to the alias map which allows to
> override the persistence of migrated bitmaps both on the source and
Once again, we encounter the non-idiomatic "allows to ${VERB}"; the
easiest solutions are "allows ${SUBJECT} to ${VERB}" or "allows
${VERB}ing". I'll go with the latter.
> destination sides.
>
> Signed-off-by: Peter Krempa
> ---
> migration/block-dirty-bitmap.c | 30 +++---
> qapi/migration.json| 19 ++-
> 2 files changed, 45 insertions(+), 4 deletions(-)
>
> @@ -806,7 +819,16 @@ static int dirty_bitmap_load_start(QEMUFile *f,
> DBMLoadState *s)
> return -EINVAL;
> }
>
> -if (flags & DIRTY_BITMAP_MIG_START_FLAG_PERSISTENT) {
> +if (s->bmap_inner &&
> +s->bmap_inner->has_transform &&
> +s->bmap_inner->transform &&
This leg of the conjunction is always true (if has_transform is set,
transform is necessarily non-NULL).
> +s->bmap_inner->transform->has_persistent) {
> +persistent = s->bmap_inner->transform->persistent;
> +} else {
> +persistent = flags & DIRTY_BITMAP_MIG_START_FLAG_PERSISTENT;
> +}
> +
> +if (persistent) {
> bdrv_dirty_bitmap_set_persistence(s->bitmap, true);
> }
>
> @@ -544,12 +557,16 @@
> # @alias: An alias name for migration (for example the bitmap name on
> # the opposite site).
> #
> +# @transform: Allows to modify properties of the migrated bitmap.
Allows the modification of
I can make those tweaks while queuing.
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[PATCH v7 23/31] linux-user/aarch64: Implement PR_MTE_TCF and PR_MTE_TAG
These prctl fields are required for the function of MTE.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
linux-user/aarch64/target_syscall.h | 9 ++
linux-user/syscall.c| 43 +
2 files changed, 52 insertions(+)
diff --git a/linux-user/aarch64/target_syscall.h
b/linux-user/aarch64/target_syscall.h
index 820601dfcc..76f6c3391d 100644
--- a/linux-user/aarch64/target_syscall.h
+++ b/linux-user/aarch64/target_syscall.h
@@ -33,5 +33,14 @@ struct target_pt_regs {
#define TARGET_PR_SET_TAGGED_ADDR_CTRL 55
#define TARGET_PR_GET_TAGGED_ADDR_CTRL 56
# define TARGET_PR_TAGGED_ADDR_ENABLE (1UL << 0)
+/* MTE tag check fault modes */
+# define TARGET_PR_MTE_TCF_SHIFT 1
+# define TARGET_PR_MTE_TCF_NONE(0UL << TARGET_PR_MTE_TCF_SHIFT)
+# define TARGET_PR_MTE_TCF_SYNC(1UL << TARGET_PR_MTE_TCF_SHIFT)
+# define TARGET_PR_MTE_TCF_ASYNC (2UL << TARGET_PR_MTE_TCF_SHIFT)
+# define TARGET_PR_MTE_TCF_MASK(3UL << TARGET_PR_MTE_TCF_SHIFT)
+/* MTE tag inclusion mask */
+# define TARGET_PR_MTE_TAG_SHIFT 3
+# define TARGET_PR_MTE_TAG_MASK(0xUL << TARGET_PR_MTE_TAG_SHIFT)
#endif /* AARCH64_TARGET_SYSCALL_H */
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ba4da7f8a6..61bf6148e7 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -10985,17 +10985,53 @@ static abi_long do_syscall1(void *cpu_env, int num,
abi_long arg1,
{
abi_ulong valid_mask = TARGET_PR_TAGGED_ADDR_ENABLE;
CPUARMState *env = cpu_env;
+ARMCPU *cpu = env_archcpu(env);
+
+if (cpu_isar_feature(aa64_mte, cpu)) {
+valid_mask |= TARGET_PR_MTE_TCF_MASK;
+valid_mask |= TARGET_PR_MTE_TAG_MASK;
+}
if ((arg2 & ~valid_mask) || arg3 || arg4 || arg5) {
return -TARGET_EINVAL;
}
env->tagged_addr_enable = arg2 & TARGET_PR_TAGGED_ADDR_ENABLE;
+
+if (cpu_isar_feature(aa64_mte, cpu)) {
+switch (arg2 & TARGET_PR_MTE_TCF_MASK) {
+case TARGET_PR_MTE_TCF_NONE:
+case TARGET_PR_MTE_TCF_SYNC:
+case TARGET_PR_MTE_TCF_ASYNC:
+break;
+default:
+return -EINVAL;
+}
+
+/*
+ * Write PR_MTE_TCF to SCTLR_EL1[TCF0].
+ * Note that the syscall values are consistent with hw.
+ */
+env->cp15.sctlr_el[1] =
+deposit64(env->cp15.sctlr_el[1], 38, 2,
+ arg2 >> TARGET_PR_MTE_TCF_SHIFT);
+
+/*
+ * Write PR_MTE_TAG to GCR_EL1[Exclude].
+ * Note that the syscall uses an include mask,
+ * and hardware uses an exclude mask -- invert.
+ */
+env->cp15.gcr_el1 =
+deposit64(env->cp15.gcr_el1, 0, 16,
+ ~arg2 >> TARGET_PR_MTE_TAG_SHIFT);
+arm_rebuild_hflags(env);
+}
return 0;
}
case TARGET_PR_GET_TAGGED_ADDR_CTRL:
{
abi_long ret = 0;
CPUARMState *env = cpu_env;
+ARMCPU *cpu = env_archcpu(env);
if (arg2 || arg3 || arg4 || arg5) {
return -TARGET_EINVAL;
@@ -11003,6 +11039,13 @@ static abi_long do_syscall1(void *cpu_env, int num,
abi_long arg1,
if (env->tagged_addr_enable) {
ret |= TARGET_PR_TAGGED_ADDR_ENABLE;
}
+if (cpu_isar_feature(aa64_mte, cpu)) {
+/* See above. */
+ret |= (extract64(env->cp15.sctlr_el[1], 38, 2)
+<< TARGET_PR_MTE_TCF_SHIFT);
+ret = deposit64(ret, TARGET_PR_MTE_TAG_SHIFT, 16,
+~env->cp15.gcr_el1);
+}
return ret;
}
#endif /* AARCH64 */
--
2.25.1
[PATCH v7 11/31] exec: Introduce cpu_untagged_addr
Provide an identity fallback for target that do not
use tagged addresses.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
include/exec/cpu_ldst.h | 7 +++
1 file changed, 7 insertions(+)
diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index e62f4fba00..d9dc1de414 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -69,6 +69,13 @@ typedef uint64_t abi_ptr;
#define TARGET_ABI_FMT_ptr "%"PRIx64
#endif
+#ifndef TARGET_TAGGED_ADDRESSES
+static inline abi_ptr cpu_untagged_addr(CPUState *cs, abi_ptr x)
+{
+return x;
+}
+#endif
+
/* All direct uses of g2h and h2g need to go away for usermode softmmu. */
#define g2h(x) ((void *)((uintptr_t)(abi_ptr)(x) + guest_base))
--
2.25.1
Re: [PATCH 04/38] target/riscv: 16-bit Addition & Subtraction Instructions
On 2/12/21 7:02 AM, LIU Zhiwei wrote:
> +static void tcg_gen_simd_add16(TCGv d, TCGv a, TCGv b)
> +{
> +TCGv t1 = tcg_temp_new();
> +TCGv t2 = tcg_temp_new();
> +
> +tcg_gen_andi_tl(t1, a, ~0x);
> +tcg_gen_add_tl(t2, a, b);
> +tcg_gen_add_tl(t1, t1, b);
> +tcg_gen_deposit_tl(d, t1, t2, 0, 16);
> +
> +tcg_temp_free(t1);
> +tcg_temp_free(t2);
> +}
I will note that there are some helper functions, e.g. tcg_gen_vec_add16_i64
(see the end of include/tcg/tcg-op-gvec.h), but those are explicitly i64, and
you'll still need these for rv32.
r~
[PATCH v7 22/31] target/arm: Use the proper TBI settings for linux-user
We were fudging TBI1 enabled to speed up the generated code.
Now that we've improved the code generation, remove this.
Also, tidy the comment to reflect the current code.
The pauth test was testing a kernel address (-1) and making
incorrect assumptions about TBI1; stick to userland addresses.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/internals.h | 4 ++--
target/arm/cpu.c| 10 +++---
tests/tcg/aarch64/pauth-2.c | 1 -
3 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index b251fe4450..112bbb14f0 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1425,9 +1425,9 @@ static inline bool tcma_check(uint32_t desc, int bit55,
int ptr_tag)
*/
static inline uint64_t useronly_clean_ptr(uint64_t ptr)
{
-/* TBI is known to be enabled. */
#ifdef CONFIG_USER_ONLY
-ptr = sextract64(ptr, 0, 56);
+/* TBI0 is known to be enabled, while TBI1 is disabled. */
+ptr &= sextract64(ptr, 0, 56);
#endif
return ptr;
}
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 5cf6c056c5..70cfcbc918 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -205,14 +205,10 @@ static void arm_cpu_reset(DeviceState *dev)
env->vfp.zcr_el[1] = MIN(cpu->sve_max_vq - 1, 3);
}
/*
- * Enable TBI0 and TBI1. While the real kernel only enables TBI0,
- * turning on both here will produce smaller code and otherwise
- * make no difference to the user-level emulation.
- *
- * In sve_probe_page, we assume that this is set.
- * Do not modify this without other changes.
+ * Enable TBI0 but not TBI1.
+ * Note that this must match useronly_clean_ptr.
*/
-env->cp15.tcr_el[1].raw_tcr = (3ULL << 37);
+env->cp15.tcr_el[1].raw_tcr = (1ULL << 37);
#else
/* Reset into the highest available EL */
if (arm_feature(env, ARM_FEATURE_EL3)) {
diff --git a/tests/tcg/aarch64/pauth-2.c b/tests/tcg/aarch64/pauth-2.c
index 9bba0beb63..978652ede3 100644
--- a/tests/tcg/aarch64/pauth-2.c
+++ b/tests/tcg/aarch64/pauth-2.c
@@ -53,7 +53,6 @@ void do_test(uint64_t value)
int main()
{
do_test(0);
-do_test(-1);
do_test(0xda004acedeadbeefull);
return 0;
}
--
2.25.1
[PATCH v7 20/31] linux-user/aarch64: Implement PR_TAGGED_ADDR_ENABLE
This is the prctl bit that controls whether syscalls accept tagged
addresses. See Documentation/arm64/tagged-address-abi.rst in the
linux kernel.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
linux-user/aarch64/target_syscall.h | 4
target/arm/cpu-param.h | 3 +++
target/arm/cpu.h| 31 +
linux-user/syscall.c| 24 ++
4 files changed, 62 insertions(+)
diff --git a/linux-user/aarch64/target_syscall.h
b/linux-user/aarch64/target_syscall.h
index 3194e6b009..820601dfcc 100644
--- a/linux-user/aarch64/target_syscall.h
+++ b/linux-user/aarch64/target_syscall.h
@@ -30,4 +30,8 @@ struct target_pt_regs {
# define TARGET_PR_PAC_APDBKEY (1 << 3)
# define TARGET_PR_PAC_APGAKEY (1 << 4)
+#define TARGET_PR_SET_TAGGED_ADDR_CTRL 55
+#define TARGET_PR_GET_TAGGED_ADDR_CTRL 56
+# define TARGET_PR_TAGGED_ADDR_ENABLE (1UL << 0)
+
#endif /* AARCH64_TARGET_SYSCALL_H */
diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h
index 00e7d9e937..7f38d33b8e 100644
--- a/target/arm/cpu-param.h
+++ b/target/arm/cpu-param.h
@@ -20,6 +20,9 @@
#ifdef CONFIG_USER_ONLY
#define TARGET_PAGE_BITS 12
+# ifdef TARGET_AARCH64
+# define TARGET_TAGGED_ADDRESSES
+# endif
#else
/*
* ARMv7 and later CPUs have 4K pages minimum, but ARMv5 and v6
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index f240275407..72a0819eb8 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -721,6 +721,11 @@ typedef struct CPUARMState {
const struct arm_boot_info *boot_info;
/* Store GICv3CPUState to access from this struct */
void *gicv3state;
+
+#ifdef TARGET_TAGGED_ADDRESSES
+/* Linux syscall tagged address support */
+bool tagged_addr_enable;
+#endif
} CPUARMState;
static inline void set_feature(CPUARMState *env, int feature)
@@ -3604,6 +3609,32 @@ static inline MemTxAttrs
*typecheck_memtxattrs(MemTxAttrs *x)
*/
#define PAGE_BTI PAGE_TARGET_1
+#ifdef TARGET_TAGGED_ADDRESSES
+/**
+ * cpu_untagged_addr:
+ * @cs: CPU context
+ * @x: tagged address
+ *
+ * Remove any address tag from @x. This is explicitly related to the
+ * linux syscall TIF_TAGGED_ADDR setting, not TBI in general.
+ *
+ * There should be a better place to put this, but we need this in
+ * include/exec/cpu_ldst.h, and not some place linux-user specific.
+ */
+static inline target_ulong cpu_untagged_addr(CPUState *cs, target_ulong x)
+{
+ARMCPU *cpu = ARM_CPU(cs);
+if (cpu->env.tagged_addr_enable) {
+/*
+ * TBI is enabled for userspace but not kernelspace addresses.
+ * Only clear the tag if bit 55 is clear.
+ */
+x &= sextract64(x, 0, 56);
+}
+return x;
+}
+#endif
+
/*
* Naming convention for isar_feature functions:
* Functions which test 32-bit ID registers should have _aa32_ in
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 24fc1daf02..ba4da7f8a6 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -10981,6 +10981,30 @@ static abi_long do_syscall1(void *cpu_env, int num,
abi_long arg1,
}
}
return -TARGET_EINVAL;
+case TARGET_PR_SET_TAGGED_ADDR_CTRL:
+{
+abi_ulong valid_mask = TARGET_PR_TAGGED_ADDR_ENABLE;
+CPUARMState *env = cpu_env;
+
+if ((arg2 & ~valid_mask) || arg3 || arg4 || arg5) {
+return -TARGET_EINVAL;
+}
+env->tagged_addr_enable = arg2 & TARGET_PR_TAGGED_ADDR_ENABLE;
+return 0;
+}
+case TARGET_PR_GET_TAGGED_ADDR_CTRL:
+{
+abi_long ret = 0;
+CPUARMState *env = cpu_env;
+
+if (arg2 || arg3 || arg4 || arg5) {
+return -TARGET_EINVAL;
+}
+if (env->tagged_addr_enable) {
+ret |= TARGET_PR_TAGGED_ADDR_ENABLE;
+}
+return ret;
+}
#endif /* AARCH64 */
case PR_GET_SECCOMP:
case PR_SET_SECCOMP:
--
2.25.1
[PATCH v7 07/31] linux-user: Tidy VERIFY_READ/VERIFY_WRITE
These constants are only ever used with access_ok, and friends.
Rather than translating them to PAGE_* bits, let them equal
the PAGE_* bits to begin.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
linux-user/qemu.h | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 441ba6a78b..9251337daf 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -488,8 +488,8 @@ extern unsigned long guest_stack_size;
/* user access */
-#define VERIFY_READ 0
-#define VERIFY_WRITE 1 /* implies read access */
+#define VERIFY_READ PAGE_READ
+#define VERIFY_WRITE (PAGE_READ | PAGE_WRITE)
static inline bool access_ok(int type, abi_ulong addr, abi_ulong size)
{
@@ -501,9 +501,7 @@ static inline bool access_ok(int type, abi_ulong addr,
abi_ulong size)
!guest_addr_valid(addr + size - 1))) {
return false;
}
-return page_check_range((target_ulong)addr, size,
-(type == VERIFY_READ) ? PAGE_READ :
-(PAGE_READ | PAGE_WRITE)) == 0;
+return page_check_range((target_ulong)addr, size, type) == 0;
}
/* NOTE __get_user and __put_user use host pointers and don't check access.
--
2.25.1
Re: [PATCH v2 30/42] esp: add 4 byte PDMA read and write transfers
On 2/9/21 8:30 PM, Mark Cave-Ayland wrote: > The MacOS toolbox ROM performs 4 byte reads/writes when transferring data to > and from the target. Since the SCSI bus is 16-bits wide, use the memory API > to split a 4 byte access into 2 x 2 byte accesses. > > Signed-off-by: Mark Cave-Ayland > --- > hw/scsi/esp.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daudé Out of curiosity, what is the bus used?
[PATCH v7 17/31] linux-user: Move lock_user et al out of line
These functions are not small, except for unlock_user
without debugging enabled. Move them out of line, and
add missing braces on the way.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
linux-user/qemu.h| 45 ++-
linux-user/uaccess.c | 46
2 files changed, 52 insertions(+), 39 deletions(-)
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 82eabb73f8..36b58bd840 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -640,57 +640,24 @@ abi_long copy_to_user(abi_ulong gaddr, void *hptr, size_t
len);
/* Lock an area of guest memory into the host. If copy is true then the
host area will have the same contents as the guest. */
-static inline void *lock_user(int type, abi_ulong guest_addr, long len, int
copy)
-{
-if (!access_ok_untagged(type, guest_addr, len)) {
-return NULL;
-}
-#ifdef DEBUG_REMAP
-{
-void *addr;
-addr = g_malloc(len);
-if (copy)
-memcpy(addr, g2h(guest_addr), len);
-else
-memset(addr, 0, len);
-return addr;
-}
-#else
-return g2h_untagged(guest_addr);
-#endif
-}
+void *lock_user(int type, abi_ulong guest_addr, long len, int copy);
/* Unlock an area of guest memory. The first LEN bytes must be
flushed back to guest memory. host_ptr = NULL is explicitly
allowed and does nothing. */
-static inline void unlock_user(void *host_ptr, abi_ulong guest_addr,
- long len)
-{
-
#ifdef DEBUG_REMAP
-if (!host_ptr)
-return;
-if (host_ptr == g2h_untagged(guest_addr))
-return;
-if (len > 0)
-memcpy(g2h_untagged(guest_addr), host_ptr, len);
-g_free(host_ptr);
+static inline void unlock_user(void *host_ptr, abi_ulong guest_addr, long len)
+{ }
+#else
+void unlock_user(void *host_ptr, abi_ulong guest_addr, long len);
#endif
-}
/* Return the length of a string in target memory or -TARGET_EFAULT if
access error. */
abi_long target_strlen(abi_ulong gaddr);
/* Like lock_user but for null terminated strings. */
-static inline void *lock_user_string(abi_ulong guest_addr)
-{
-abi_long len;
-len = target_strlen(guest_addr);
-if (len < 0)
-return NULL;
-return lock_user(VERIFY_READ, guest_addr, (long)(len + 1), 1);
-}
+void *lock_user_string(abi_ulong guest_addr);
/* Helper macros for locking/unlocking a target struct. */
#define lock_user_struct(type, host_ptr, guest_addr, copy) \
diff --git a/linux-user/uaccess.c b/linux-user/uaccess.c
index e215ecc2a6..bba012ed15 100644
--- a/linux-user/uaccess.c
+++ b/linux-user/uaccess.c
@@ -4,6 +4,52 @@
#include "qemu.h"
+void *lock_user(int type, abi_ulong guest_addr, long len, int copy)
+{
+if (!access_ok_untagged(type, guest_addr, len)) {
+return NULL;
+}
+#ifdef DEBUG_REMAP
+{
+void *addr;
+addr = g_malloc(len);
+if (copy) {
+memcpy(addr, g2h(guest_addr), len);
+} else {
+memset(addr, 0, len);
+}
+return addr;
+}
+#else
+return g2h_untagged(guest_addr);
+#endif
+}
+
+#ifdef DEBUG_REMAP
+void unlock_user(void *host_ptr, abi_ulong guest_addr, long len);
+{
+if (!host_ptr) {
+return;
+}
+if (host_ptr == g2h_untagged(guest_addr)) {
+return;
+}
+if (len > 0) {
+memcpy(g2h_untagged(guest_addr), host_ptr, len);
+}
+g_free(host_ptr);
+}
+#endif
+
+void *lock_user_string(abi_ulong guest_addr)
+{
+abi_long len = target_strlen(guest_addr);
+if (len < 0) {
+return NULL;
+}
+return lock_user(VERIFY_READ, guest_addr, (long)(len + 1), 1);
+}
+
/* copy_from_user() and copy_to_user() are usually used to copy data
* buffers between the target and host. These internally perform
* locking/unlocking of the memory.
--
2.25.1
